From eb90cbdd596f5ace3a62a08ce4d0746ab78fe73b Mon Sep 17 00:00:00 2001 From: piekstra Date: Thu, 8 Oct 2026 14:28:24 -0400 Subject: [PATCH 1/2] fix(packaging): keep cask hooks declarative-compatible --- .goreleaser.yml | 5 +++-- scripts/verify-package-render.sh | 28 +++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/.goreleaser.yml b/.goreleaser.yml index c11a9fbd..7cadaed6 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -115,12 +115,13 @@ homebrew_casks: post: install: | system_command "/usr/bin/xattr", args: ["-dr", "com.apple.quarantine", "#{staged_path}/cr"] - result = system_command "#{staged_path}/cr", args: ["catalog", "update"], must_succeed: false - opoo "cr catalog update failed; run `cr catalog update` to refresh the model catalog." unless result&.success? + system_command "#{staged_path}/cr", args: ["catalog", "update"], must_succeed: false caveats: | To configure cr, follow the setup examples in the README: https://github.com/open-cli-collective/codereview-cli#authentication-and-setup + If the catalog refresh fails, run `cr catalog update` after installation. + Secrets are stored in the OS credential store. They are never written to config.yml. diff --git a/scripts/verify-package-render.sh b/scripts/verify-package-render.sh index b5e2762f..63d1e53c 100755 --- a/scripts/verify-package-render.sh +++ b/scripts/verify-package-render.sh @@ -42,11 +42,13 @@ require_grep 'cr_v#{version}_darwin_arm64.tar.gz' "$cask" require_grep 'cr_v#{version}_darwin_amd64.tar.gz' "$cask" require_grep 'args: ["catalog", "update"]' "$cask" require_grep 'must_succeed: false' "$cask" -require_grep 'cr catalog update failed; run `cr catalog update`' "$cask" +require_grep 'If the catalog refresh fails, run `cr catalog update`' "$cask" # Exercise the rendered Ruby control flow as well as checking its source. The # updater is warning-only during package installation, so a failed refresh -# must call opoo without raising or skipping the command. +# must not raise or skip either command. Hooks use direct commands so the +# Homebrew tap can convert them to declarative postflight_steps without Ruby +# result assignments or conditionals that the declarative DSL cannot execute. ruby - "$cask" <<'RUBY' path = ARGV.fetch(0) source = File.read(path) @@ -96,8 +98,28 @@ raise "refresh command is not warning-only" unless success.calls.last[:must_succ failure = HookHarness.new(true) failure.run(body) raise "failure path did not invoke both commands" unless failure.calls.length == 2 -raise "failure path did not warn" unless failure.warnings.length == 1 +raise "hook must not require imperative warning branches" unless failure.warnings.empty? raise "failure path did not keep refresh warning-only" unless failure.calls.last[:must_succeed] == false +raise "hook must not assign command results" if body.match?(/\bresult\s*=/) +# Model the tap's direct-command conversion with a strict declarative DSL. +# A result assignment left behind would try to call unsupported system_command. +class StepsHarness + attr_reader :calls + def initialize + @calls = [] + end + def staged_path + "{{staged_path}}" + end + def run(command, args:, must_succeed: true) + @calls << {command: command, args: args, must_succeed: must_succeed} + nil + end +end +steps = StepsHarness.new +steps.instance_eval(body.gsub(/^(\s*)system_command /, '\\1run '), "declarative-hook", 1) +raise "declarative hook did not preserve both commands" unless steps.calls.length == 2 +raise "declarative catalog refresh must allow failure" unless steps.calls.last[:must_succeed] == false puts "rendered package hook behavior check OK" RUBY From c365e780e39d71f572fc022fe2180ab4bbbdd5d4 Mon Sep 17 00:00:00 2001 From: piekstra Date: Thu, 8 Oct 2026 14:36:45 -0400 Subject: [PATCH 2/2] docs(packaging): identify the Homebrew DSL contract --- scripts/verify-package-render.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/verify-package-render.sh b/scripts/verify-package-render.sh index 63d1e53c..083bb2d3 100755 --- a/scripts/verify-package-render.sh +++ b/scripts/verify-package-render.sh @@ -103,6 +103,10 @@ raise "failure path did not keep refresh warning-only" unless failure.calls.last raise "hook must not assign command results" if body.match?(/\bresult\s*=/) # Model the tap's direct-command conversion with a strict declarative DSL. # A result assignment left behind would try to call unsupported system_command. +# This narrow double mirrors Homebrew 7.0.9's InstallSteps::DSL#run contract: +# https://github.com/Homebrew/brew/blob/7.0.9/Library/Homebrew/install_steps.rb +# It is not an upstream compatibility test. Recheck the generated cask with +# Homebrew's content loader and brew style when that DSL changes. class StepsHarness attr_reader :calls def initialize