fix(#111): replace deprecated Homebrew cask postflight stanza - #112
fullsend-ai-coder[bot] wants to merge 11 commits into
Conversation
|
🤖 Finished Review · ✅ Success · Started 8:34 AM UTC · Completed 8:50 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.19 |
|
Risk Assessment: moderate (2/5) DetailsRe-review: Tier 1 signals unchanged from prior run (FILES 8, LINES 372, PROTECTED_PATH_COUNT 2, bot author); composite (2.0x0.50 + 1.56x0.30 + 1.75x0.20 = 1.82) rounds to 2 -- PROTECTED_PATH_COUNT=2 is the primary elevation factor, offset by bot authorship, zero security sensitivity, no dependency changes, clear issue alignment, and zero churn across all changed files. Previous runRisk Assessment: moderate (2/5) DetailsRe-review: Tier 1 signals unchanged from prior run (FILES 8, LINES 377, PROTECTED_PATH_COUNT 2, bot author, no Go source); composite (1.75x0.50 + 1.75x0.30 + 2.0x0.20 = 1.80) rounds to 2 — PROTECTED_PATH_COUNT=2 is the primary elevation factor, offset by bot authorship, zero security sensitivity, no dependency changes, tight issue alignment, and low git churn across all changed files. Previous run (2)Risk Assessment: moderate (2/5) DetailsRe-review: Tier 1 signals essentially unchanged from prior run (FILES 7-8, LINES 369-377); composite (2.25x0.50 + 2.0x0.30 + 1.83x0.20 = 2.13) rounds to 2 — PROTECTED_PATH_COUNT and zero Go-test ratio elevate while bot authorship, zero security sensitivity, tight issue alignment, and predominantly low-risk spec documentation keep it at moderate. Previous run (3)Risk Assessment: moderate (2/5) DetailsRe-review: Tier 1 signals have shifted from the prior run (FILES_CHANGED now 7 vs 3, LINES_CHANGED now 369 vs 22) because spec artifacts were added to the PR, but the weighted composite (Tier1=2.25x0.50 + Tier2=2.0x0.30 + Tier3=1.5x0.20 = 1.93) still rounds to 2 — the formally elevated PROTECTED_PATH_COUNT and zero test ratio are offset by bot authorship, zero security sensitivity, no CI workflow changes, tight issue alignment, and the fact that the majority of new lines are low-risk spec documentation rather than executable code. Previous run (4)Risk Assessment: moderate (2/5) DetailsRe-review: Tier 1 signals are identical to the prior run (PROTECTED_PATH_COUNT=2, TEST_FILE_RATIO=0.00, FILES_CHANGED=3); the independent weighted composite (Tier1=2.0×0.50 + Tier2=2.14×0.30 + Tier3=1.5×0.20 = 1.94) also rounds to 2, confirming the prior score — this is a well-scoped 22-line deprecation fix with formally elevated protected-path and zero-test-ratio signals offset by small blast radius, bot authorship, tight issue alignment, and no unresolved discussions. Previous run (5)Risk Assessment: moderate (2/5) DetailsRe-review: Tier 1 signals are identical to the prior run (PROTECTED_PATH_COUNT=2, TEST_FILE_RATIO=0.00, FILES_CHANGED=3); Tier 2 and Tier 3 provide no articulable reason to change the score, so the prior score of 2/moderate is preserved — the weighted composite (Tier1=2.0x0.50 + Tier2=1.71x0.30 + Tier3=1.5x0.20 = 1.813) rounds to 2, confirming a fundamentally low-risk deprecation fix with formally elevated protected-path and zero-test-ratio signals. Previous run (6)Risk Assessment: moderate (2/5) DetailsRe-review: Tier 1 signals are identical to the prior run (PROTECTED_PATH_COUNT=2, TEST_FILE_RATIO=0.00, FILES_CHANGED=3); Tier 2 and Tier 3 provide no articulable reason to change the score, so the prior score of 2/moderate is preserved — the weighted composite (Tier1=2.0x0.50 + Tier2=1.71x0.30 + Tier3=1.5x0.20 = 1.813) rounds to 2, confirming a fundamentally low-risk deprecation fix with formally elevated protected-path and zero-test-ratio signals. Previous run (7)Risk Assessment: moderate (2/5) DetailsRe-review: protected_path_count rose from 1 to 2 and file count grew from 2 to 3; the weighted composite (Tier1=2.0x0.50 + Tier2=2.3x0.30 + Tier3=1.3x0.20 = 1.95) rounds to 2/moderate, reflecting formally elevated PROTECTED_PATH_COUNT and 0.00 TEST_FILE_RATIO signals despite the fundamentally low-risk deprecation fix. Previous run (8)Risk Assessment: low (1/5) DetailsSmall, targeted deprecation fix (2 files, 19 lines) replacing a deprecated Homebrew postflight stanza with its successor in goreleaser config and a test fixture; bot author, minimal blast radius, clear linked issue with well-covered acceptance criteria, and no security or CI pipeline concerns beyond the single protected goreleaser path. |
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Next steps:
Previous run (2)ReviewFindingsMedium
Low
Previous run (3)ReviewFindingsMedium
Low
Next steps:
Previous run (4)ReviewFindingsMedium
Previous run (5)ReviewFindingsMedium
Low
Next steps:
Previous run (6)ReviewFindingsMedium
Previous run (7)ReviewFindingsMedium
Low
Previous run (8)ReviewFindingsHigh
Medium
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 8:51 AM UTC · Completed 8:58 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.05 |
Replace {{staged_path}} with #{staged_path} in both the GoReleaser
custom_block and the testdata fixture. The {{}} syntax is not Ruby
string interpolation — Ruby uses #{} — so the previous change caused
xattr to target a nonexistent literal path, silently failing to
remove macOS quarantine.
Add a regression assertion to the cask test suite verifying that the
generated cask uses #{staged_path} (Ruby interpolation) rather than
{{staged_path}}.
Addresses #112
🔧 Fix agent — iteration 1 (bot-triggered)Fixed Ruby interpolation syntax in both .goreleaser.yaml and testdata fixture: replaced {{staged_path}} with #{staged_path}. The {{}} syntax is not Ruby interpolation and caused xattr to target a literal nonexistent path. Added a regression test assertion to catch this class of error. Fixed (3):
Disagreed (1):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 8:59 AM UTC · Completed 9:15 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.88 |
fullsend-ai-review
Bot
added
risk/moderate
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
requires-manual-review
|
/fs-fix the low finding, 1 iteration |
|
🤖 Finished Fix · ✅ Success · Started 10:46 AM UTC · Completed 10:52 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $0.87 |
🔧 Fix agent — iteration 2 (human-triggered)Fixed pattern inconsistency by combining the split run call in .goreleaser.yaml onto a single line to match the test fixture format Fixed (1):
Disagreed (1):
Tests: passed Decision points
Next steps:
|
Combine the split `run` call in .goreleaser.yaml onto a single line to match the test fixture format in testdata/replicator-v0.5.0.rb. Since GoReleaser injects custom_block verbatim, the generated cask now matches the fixture exactly. Addresses #112
|
🤖 Finished Review · ✅ Success · Started 10:53 AM UTC · Completed 11:07 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.69 |
yvonnedevlinrh
left a comment
yvonnedevlinrh
left a comment
There was a problem hiding this comment.
Requesting changes
CRITICAL: The non-trivial release configuration and test change has no preceding change-specific OpenSpec artifact. This violates the constitution’s Spec-First Development rule; the severity pack classifies constitutional violations as CRITICAL. Add and complete the required OpenSpec artifact with a coverage strategy before implementation.
HIGH: The regression test does not exercise GoReleaser output. .github/scripts/patch-homebrew-cask_test.sh:57-58 checks a hand-maintained fixture, so reverting .goreleaser.yaml:60-65 to legacy hooks.post.install would still pass. Add a deterministic rendered-cask test asserting postflight_steps do is present, postflight do is absent, and #{staged_path} survives rendering. This was suggested in issue #111 but not previously raised inline. Rules: TC-003, TC-006.
Replace {{staged_path}} with #{staged_path} in both the GoReleaser
custom_block and the testdata fixture. The {{}} syntax is not Ruby
string interpolation — Ruby uses #{} — so the previous change caused
xattr to target a nonexistent literal path, silently failing to
remove macOS quarantine.
Add a regression assertion to the cask test suite verifying that the
generated cask uses #{staged_path} (Ruby interpolation) rather than
{{staged_path}}.
Addresses #112
Combine the split `run` call in .goreleaser.yaml onto a single line to match the test fixture format in testdata/replicator-v0.5.0.rb. Since GoReleaser injects custom_block verbatim, the generated cask now matches the fixture exactly. Addresses #112
fcd92cc to
99a2f77
Compare
|
🤖 Finished Review · ✅ Success · Started 4:19 PM UTC · Completed 4:34 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.78 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
removed
the
requires-manual-review
|
🤖 Finished Fix · ✅ Success · Started 4:35 PM UTC · Completed 4:42 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.02 |
🔧 Fix agent — iteration 3 (bot-triggered)Addressed all three review findings: added GoReleaser DSL maintenance comment, rephrased test assertion message to be cause-neutral, and documented that protected-path human approval is a process step not a code fix. Fixed (2):
Disagreed (1):
Tests: passed
Decision points
Next steps:
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
removed
the
requires-manual-review
|
🤖 Finished Fix · ❌ Failure (running pre-script: exit status 1) · Started 1:01 PM UTC · Completed 1:01 PM UTC Commit: Effort: high |
|
/fs-fix |
|
🤖 Finished Fix · ✅ Success · Started 1:04 PM UTC · Completed 1:11 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.52 |
Remove unreachable else branch in awk custom_block extractor (the outer condition guarantees the inner condition is always true) and remove section-level PASS echo to match the file's convention of silent-on-success with a single suite-level PASS. Addresses #112
🔧 Fix agent — iteration 6 (human-triggered)Addressed two LOW findings: removed unreachable dead code in awk extractor and removed inconsistent section-level PASS echo. Disagreed on two findings: protected-path is a process step, and the grep assertion is a deliberate spec coverage canary. Fixed (2):
Disagreed (2):
Tests: passed
Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 1:13 PM UTC · Completed 1:27 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.76 |
| fi | ||
| else | ||
| # Fallback: check that 'postflight do' only appears as 'postflight_steps do' | ||
| POSTFLIGHT_LINES=$(echo "$CUSTOM_BLOCK" | grep 'postflight.*do' | grep -cv 'postflight_steps' || true) |
There was a problem hiding this comment.
[low] naming
POSTFLIGHT_LINES holds the numeric count returned by grep -c, not a collection of lines. The _LINES suffix implies a list or multi-line string. A _COUNT suffix (e.g. POSTFLIGHT_COUNT) would better signal that the value is an integer used in arithmetic comparison.
fullsend-ai-review
Bot
added
the
requires-manual-review
Replace {{staged_path}} with #{staged_path} in both the GoReleaser
custom_block and the testdata fixture. The {{}} syntax is not Ruby
string interpolation — Ruby uses #{} — so the previous change caused
xattr to target a nonexistent literal path, silently failing to
remove macOS quarantine.
Add a regression assertion to the cask test suite verifying that the
generated cask uses #{staged_path} (Ruby interpolation) rather than
{{staged_path}}.
Addresses #112
Combine the split `run` call in .goreleaser.yaml onto a single line to match the test fixture format in testdata/replicator-v0.5.0.rb. Since GoReleaser injects custom_block verbatim, the generated cask now matches the fixture exactly. Addresses #112
- Add maintenance comment above custom_block in .goreleaser.yaml noting that native postflight_steps support is not yet available in GoReleaser DSL (tracking goreleaser/goreleaser#6873) - Rephrase test assertion message in patch-homebrew-cask_test.sh to describe absence rather than presupposing substitution cause Addresses #112
1a26588 to
fe55e3a
Compare
…112 Add tactical OpenSpec change (fix-cask-postflight-steps) documenting the Homebrew postflight migration: proposal, design, tasks, and coverage strategy. Add a deterministic rendered-cask regression test that extracts the custom_block from .goreleaser.yaml and verifies postflight_steps is present, legacy postflight is absent, and #{staged_path} remains valid Ruby interpolation. The test requires no network access or goreleaser binary. Addresses #112
- Add Fixed entry under ## Unreleased in CHANGELOG.md for the Homebrew cask postflight_steps migration (DR-003). References #111. - Replace content-dependent Perl regex detection with a content- independent probe (`echo "probe" | grep -qP 'probe'`) so the grep -P availability check does not depend on the custom_block content shape. Addresses #112
Remove unreachable else branch in awk custom_block extractor (the outer condition guarantees the inner condition is always true) and remove section-level PASS echo to match the file's convention of silent-on-success with a single suite-level PASS. Addresses #112
Homebrew 7.0 deprecated the `postflight` block in favour of the declarative `postflight_steps` stanza. GoReleaser's `hooks.post.install` emits the deprecated form and does not yet support `postflight_steps` natively (planned for v2.19 via goreleaser/goreleaser#6873). Use GoReleaser's `custom_block` to inject the correct `postflight_steps` stanza directly, replacing `hooks.post.install`. The new stanza uses Homebrew's install-steps DSL: `on_macos do` replaces `if OS.mac?`, `run` replaces `system_command`, and `{{staged_path}}` replaces Ruby interpolation `#{staged_path}`. Updated the test fixture to match the new stanza syntax. All cask integrity regression tests pass. Closes #111
Replace {{staged_path}} with #{staged_path} in both the GoReleaser
custom_block and the testdata fixture. The {{}} syntax is not Ruby
string interpolation — Ruby uses #{} — so the previous change caused
xattr to target a nonexistent literal path, silently failing to
remove macOS quarantine.
Add a regression assertion to the cask test suite verifying that the
generated cask uses #{staged_path} (Ruby interpolation) rather than
{{staged_path}}.
Addresses #112
Combine the split `run` call in .goreleaser.yaml onto a single line to match the test fixture format in testdata/replicator-v0.5.0.rb. Since GoReleaser injects custom_block verbatim, the generated cask now matches the fixture exactly. Addresses #112
- Add maintenance comment above custom_block in .goreleaser.yaml noting that native postflight_steps support is not yet available in GoReleaser DSL (tracking goreleaser/goreleaser#6873) - Rephrase test assertion message in patch-homebrew-cask_test.sh to describe absence rather than presupposing substitution cause Addresses #112
…112 Add tactical OpenSpec change (fix-cask-postflight-steps) documenting the Homebrew postflight migration: proposal, design, tasks, and coverage strategy. Add a deterministic rendered-cask regression test that extracts the custom_block from .goreleaser.yaml and verifies postflight_steps is present, legacy postflight is absent, and #{staged_path} remains valid Ruby interpolation. The test requires no network access or goreleaser binary. Addresses #112
- Add Fixed entry under ## Unreleased in CHANGELOG.md for the Homebrew cask postflight_steps migration (DR-003). References #111. - Replace content-dependent Perl regex detection with a content- independent probe (`echo "probe" | grep -qP 'probe'`) so the grep -P availability check does not depend on the custom_block content shape. Addresses #112
Remove unreachable else branch in awk custom_block extractor (the outer condition guarantees the inner condition is always true) and remove section-level PASS echo to match the file's convention of silent-on-success with a single suite-level PASS. Addresses #112
fe55e3a to
d585147
Compare
The goreleaser-action with args: --version and install-only: false extracts the binary to a temp directory that does not persist on PATH. Subsequent steps fail with 'command not found'. Switch to install-only: true so the binary remains available for the snapshot render step. Update the regression assertion and spec artifacts to match.
Set persist-credentials: false on actions/checkout to prevent credential leakage through artifacts (artipacked). Disable Go module caching on actions/setup-go to eliminate the cache-poisoning vector flagged by zizmor when goreleaser-action is present in the same job.
3 participants
Summary
postflightcask stanza with the newpostflight_stepsstanza introduced in Homebrew 7.0.custom_blockto emitpostflight_stepsdirectly, since nativeinstall_stepssupport is not yet released (planned for GoReleaser v2.19 via feat(cask): add hooks install_steps, deprecate raw Ruby hooks goreleaser/goreleaser#6873).on_macos do,run,{{staged_path}}).Motivation
brew install unbound-force/tap/replicatoremits repeated deprecation warnings:The
postflightstanza will become an error after 2027-12-11, at which point the cask will fail to install entirely.Changes
.goreleaser.yamlhooks.post.install(which emits deprecatedpostflight) withcustom_blockcontainingpostflight_stepsusing Homebrew's declarative DSL.github/scripts/testdata/replicator-v0.5.0.rbpostflighttopostflight_steps,if OS.mac?toon_macos do,system_commandtorun,#{staged_path}to{{staged_path}}Test plan
patch-homebrew-cask_test.sh)go test ./... -count=1 -race)go vet ./...cleango buildsucceedspostflight_steps(no deprecation warning onbrew install)Closes #111
Post-script verification
agent/111-fix-cask-postflight-steps)3e527a578cf3957d9b8f72a3edaed7b773b526bf..HEAD)