Skip to content

fix(packaging): keep Homebrew hooks declarative-compatible - #670

Open
piekstra wants to merge 2 commits into
mainfrom
fix/homebrew-postflight-steps
Open

piekstra wants to merge 2 commits into
mainfrom
fix/homebrew-postflight-steps

Conversation

@piekstra

@piekstra piekstra commented Oct 8, 2026

Copy link
Copy Markdown
Member

Homebrew 7 uses declarative postflight_steps; the generated cask still contains an assignment calling system_command and a conditional opoo. Loading the published cask fails with undefined method system_command for Homebrew::InstallSteps::DSL, so even brew list --versions fails.

Keep both hook calls direct, allowing the shared tap conversion to emit supported run steps. Catalog refresh remains nonblocking (must_succeed: false), and recovery guidance moves to the install caveats. Extend the existing rendered-hook check with a strict declarative DSL harness so an unsupported result assignment cannot pass the compatibility check.

Validation: goreleaser check; existing success/failure hook fixture checks plus the new declarative harness; git diff --check. The corresponding corrected published cask loads with the actual Homebrew 7.0.9 content loader and passes the tap's Homebrew style checks. A full snapshot build was not run for this packaging-only change.

monit-reviewer
monit-reviewer previously approved these changes Oct 8, 2026

@monit-reviewer monit-reviewer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated PR Review

Reviewed commit: eb90cbdd596f
Profile: claude-monit-reviewer - Posting as: monit-reviewer

Summary

Reviewer Findings
automation:ci-release 0
policies:conventions 0
automation:ci-deploy 0
harness-engineering:repo-health 1
harness-engineering:repo-health (1 finding)

Minor - scripts/verify-package-render.sh:106

StepsHarness invents its own run(command, args:, must_succeed:) method to stand in for Homebrew's real declarative postflight_steps DSL, and the conversion from the rendered system_command calls to that stub is done with a blind regex gsub rather than against anything from an actual Homebrew installation. The PR description says the real compatibility proof came from loading the published cask with the real Homebrew 7.0.9 content loader manually, outside this script. So the only mechanically-enforced, repeatable check is self-referential: it verifies the render against a name (run) and signature this script itself chose, not against Homebrew's actual DSL, and would not catch a future Homebrew DSL rename or signature change. Point to where the real DSL contract lives (e.g. a comment linking the Homebrew cask-dsl source/version this stub mirrors, or better, exercise brew style real Homebrew::InstallSteps::DSL if available in CI) so a future agent knows this stub can drift from upstream and where to re-verify it.

Reviewer Coverage

  • automation:ci-release — complete (broad); skipped: none; constraints: none
  • policies:conventions — complete (broad); skipped: none; constraints: Shared docs at ../cli-common/docs and ../.github were not present in the workbench; convention claims are limited to what is visible in the diff and repo-local .codereview/agents guidance.
  • automation:ci-deploy — complete (broad); skipped: none; constraints: none
  • harness-engineering:repo-health — complete (broad); inspected 1 assigned file (2 inspected across reviewers): scripts/verify-package-render.sh; skipped: none; constraints: none
Inspected files (2)
  • .goreleaser.yml
  • scripts/verify-package-render.sh

0 PR discussion threads considered. 0 summarized; 0 resolved.


Completed in 4m 00s | ~$1.40 (est.) | claude-sonnet-5 | cr 0.10.319
Field Value
Model claude-sonnet-5
Reviewers automation:ci-release, policies:conventions, automation:ci-deploy, harness-engineering:repo-health
Engine claude_cli · claude-sonnet-5
Reviewed by cr · monit-reviewer
Duration 4m 00s wall · 5m 51s compute
Cost ~$1.40 (est.)
Pricing basis anthropic-public-2026-09-23
Tokens 58 in / 15.6k out

Per-workstream usage

  • orchestrator-selection — claude-sonnet-5
    • In: 6
    • Out: 4.4k
    • Cache read: 89.7k
    • Cache create: 43.9k
    • Cost: ~$0.24 (est.)
    • Duration: 55s
  • automation:ci-release — claude-sonnet-5
    • In: 10
    • Out: 2.0k
    • Cache read: 212.1k
    • Cache create: 41.6k
    • Cost: ~$0.23 (est.)
    • Duration: 33s
  • policies:conventions — claude-sonnet-5
    • In: 16
    • Out: 2.6k
    • Cache read: 376.6k
    • Cache create: 39.7k
    • Cost: ~$0.26 (est.)
    • Duration: 1m 03s
  • automation:ci-deploy — claude-sonnet-5
    • In: 10
    • Out: 2.8k
    • Cache read: 214.7k
    • Cache create: 44.1k
    • Cost: ~$0.25 (est.)
    • Duration: 1m 59s
  • harness-engineering:repo-health — claude-sonnet-5
    • In: 10
    • Out: 3.3k
    • Cache read: 214.6k
    • Cache create: 44.4k
    • Cost: ~$0.25 (est.)
    • Duration: 1m 04s
  • orchestrator-rollup — claude-sonnet-5
    • In: 6
    • Out: 465
    • Cache read: 148.1k
    • Cache create: 34.8k
    • Cost: ~$0.17 (est.)
    • Duration: 15s

Comment thread scripts/verify-package-render.sh

@monit-reviewer monit-reviewer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated PR Review

Reviewed commit: c365e780e39d
Profile: claude-monit-reviewer - Posting as: monit-reviewer

Summary

Reviewer Findings
automation:ci-release 0
policies:conventions 0
structure:repo-health 0

Reviewer Coverage

Complete coverage details are retained in the local coverage.json artifact.

  • automation:ci-release — complete (broad); scope: 2 files; examples: .goreleaser.yml, scripts/verify-package-render.sh; body-inspected: 2 files; examples: .goreleaser.yml, scripts/verify-package-render.sh; skipped: none; constraints: Did not run goreleaser or the verify script; review based on diff reading only. Homebrew 7.0.9 InstallSteps::DSL behavior not independently verified against upstream.
  • policies:conventions — complete (broad); scope: 2 files; examples: .goreleaser.yml, scripts/verify-package-render.sh; body-inspected: 2 files; examples: .goreleaser.yml, scripts/verify-package-render.sh; skipped: none; constraints: Homebrew 7.0.9 InstallSteps::DSL behavior was not independently verified. Shared cli-common docs were not available in context; review limited to conventions visible in the diff and repo.
  • structure:repo-health — complete (broad); scope: 2 files; examples: .goreleaser.yml, scripts/verify-package-render.sh; body-inspected: 2 files; examples: .goreleaser.yml, scripts/verify-package-render.sh; skipped: none; constraints: Did not run the verify script or goreleaser; the StepsHarness double was not checked against upstream Homebrew. Reviewed the diff between base 999e59d and head c365e78 for the two assigned files only.

0 PR discussion threads considered. 0 summarized; 0 resolved.


Completed in 2m 53s | ~$1.25 (est.) | claude-sonnet-5-5 | cr 0.10.328
Field Value
Model claude-sonnet-5-5
Reviewers automation:ci-release, policies:conventions, structure:repo-health
Engine claude_cli · claude-sonnet-5-5
Reviewed by cr · monit-reviewer
Duration 2m 53s wall · 1m 50s compute
Cost ~$1.25 (est.)
Pricing basis baseline-2026-10-02/pricing
Tokens 34 in / 4.1k out

Per-workstream usage

  • orchestrator-selection — claude-sonnet-5-5
    • In: 4
    • Out: 619
    • Cache read: 7.0k
    • Cache create: 52.4k
    • Cost: ~$0.22 (est.)
    • Duration: 28s
  • automation:ci-release — claude-sonnet-5-5
    • In: 8
    • Out: 972
    • Cache read: 105.1k
    • Cache create: 50.8k
    • Cost: ~$0.23 (est.)
    • Duration: 20s
  • policies:conventions — claude-sonnet-5-5
    • In: 8
    • Out: 945
    • Cache read: 103.8k
    • Cache create: 50.1k
    • Cost: ~$0.23 (est.)
    • Duration: 28s
  • structure:repo-health — claude-sonnet-5-5
    • In: 8
    • Out: 1.1k
    • Cache read: 100.5k
    • Cache create: 54.9k
    • Cost: ~$0.25 (est.)
    • Duration: 20s
  • orchestrator-rollup — claude-sonnet-5-5
    • In: 6
    • Out: 424
    • Cache read: 129.7k
    • Cache create: 72.6k
    • Cost: ~$0.32 (est.)
    • Duration: 13s

@piekstra
piekstra marked this pull request as ready for review October 8, 2026 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants