Repository navigation
fix(homebrew-alias): keep staged paths absolute in install-step args - #47
Merged
Merged
Conversation
base: resolves only a run step's command path, not its args, so the
rewritten quarantine step ran xattr on a bare binary name in the wrong
working directory and every cask upgrade failed and reverted. Rewrite
#{staged_path} to the {{staged_path}} install-step template token
instead, which Homebrew expands inside args.
monit-reviewer
approved these changes
Sep 23, 2026
monit-reviewer
left a comment
monit-reviewer
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: 647107833d56
Profile: claude-monit-reviewer - Posting as: monit-reviewer
Summary
| Reviewer | Findings |
|---|---|
| automation:ci-deploy | 0 |
| security:code-auditor | 0 |
Reviewer Coverage
automation:ci-deploy— complete (broad); skipped: none; constraints: nonesecurity:code-auditor— complete (broad); inspected 1 assigned file (2 inspected across reviewers):actions/homebrew-alias/modernize.sh; skipped: none; constraints: none
Inspected files (2)
actions/homebrew-alias/modernize.shactions/homebrew-alias/test_modernize.sh
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 2m 16s | ~$1.43 (est.) | claude-sonnet-5 | cr 0.10.311
| Field | Value |
|---|---|
| Model | claude-sonnet-5 |
| Reviewers | automation:ci-deploy, security:code-auditor |
| Engine | claude_cli · claude-sonnet-5 |
| Reviewed by | cr · monit-reviewer |
| Duration | 2m 16s wall · 2m 34s compute |
| Cost | ~$1.43 (est.) |
| Pricing basis | anthropic-public-2026-09-02 |
| Tokens | 42 in / 6.2k out |
Per-workstream usage
orchestrator-selection— claude-sonnet-5- In: 6
- Out: 2.0k
- Cache read: 102.7k
- Cache create: 73.5k
- Cost: ~$0.33 (est.)
- Duration: 32s
automation:ci-deploy— claude-sonnet-5- In: 22
- Out: 3.1k
- Cache read: 680.5k
- Cache create: 77.7k
- Cost: ~$0.48 (est.)
- Duration: 1m 24s
security:code-auditor— claude-sonnet-5- In: 8
- Out: 784
- Cache read: 184.4k
- Cache create: 71.9k
- Cost: ~$0.33 (est.)
- Duration: 24s
orchestrator-rollup— claude-sonnet-5- In: 6
- Out: 357
- Cache read: 143.7k
- Cache create: 63.1k
- Cost: ~$0.28 (est.)
- Duration: 13s
piekstra
marked this pull request as ready for review
September 23, 2026 17:25
piekstra
added a commit
to open-cli-collective/homebrew-tap
that referenced
this pull request
Sep 23, 2026
## Summary
- The quarantine steps used `base: :staged_path` (or `base: :appdir`)
with a bare name in `args`. `base:` resolves only the `run` command
path, so xattr ran on a relative name, exited 1, and `brew upgrade`
reverted every affected cask (`xattr: No such file: gro`).
- Args now use the `{{staged_path}}` and `{{appdir}}` install-step
template tokens, which Homebrew expands inside `args`.
- `codereview-cli` was re-published in the legacy `postflight do` form
after the earlier migration, which fails this repo's CI style check on
main; it is converted to the same token form.
The generator fix is open-cli-collective/.github#47; until `v1` includes
it, the next release of a CLI re-publishes the broken form for that
cask.
## Verification
- `brew upgrade --cask google-readonly` 1.2.7 -> 2.0.11 succeeds with
the token form on Homebrew 7.0.6 and fails and reverts with the current
cask.
- Both tokens expand against a real cask context: `{{staged_path}}/gro`
-> `/opt/homebrew/Caskroom/google-readonly/2.0.11/gro`,
`{{appdir}}/X.app` -> `/Applications/X.app`.
- `brew style --only-cops Cask/InstallSteps,Homebrew/OSDependsOn
Casks/*.rb`: no offenses (main has one, in codereview-cli).
piekstra
added a commit
to open-cli-collective/homebrew-tap
that referenced
this pull request
Sep 23, 2026
…23) The google-cli 2.0.12 release started before `v1` included open-cli-collective/.github#47, so its casks (`gro`, `grw`, `google-readonly`, `google-readwrite`) came out with the bare-name quarantine step that fails and reverts `brew upgrade`. This applies the same `{{staged_path}}` form as #22. Later releases publish the corrected form from `v1`. `brew style --only-cops Cask/InstallSteps,Homebrew/OSDependsOn Casks/*.rb`: no offenses.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
modernize.shrewrite turnedinto
In Homebrew install steps,
base:resolves only theruncommand path;argsare passed through with template tokens expanded and no path resolution. xattr then runs on a baregrooutside the staged directory, exits 1 (xattr: No such file: gro), andbrew upgradereverts the upgrade. Every cask this action publishes is affected.The rewrite now converts
#{staged_path}to the{{staged_path}}template token, which Homebrew expands insideargs:Verification
test_modernize.shpasses, with the assertion changed to the token form and a new check thatbase: :staged_pathis not emitted.google-readonlycask upgraded 1.2.7 -> 2.0.11 withbrew upgrade --caskon Homebrew 7.0.6; the same upgrade with the current cask fails and reverts.brew style --only-cops Cask/InstallStepsaccepts the token form.Rollout
After merge, advance the movable
v1tag to the merge commit so the next release of each CLI publishes the corrected step. The tap casks already published are fixed separately in homebrew-tap.