fix(homebrew): update bump workflow for cli layout#106
Conversation
- Checkout source code in homebrew-bump workflow - Copy formula template before stamping URL/SHA - Add test to ensure homebrew formula template tracks cli layout The homebrew bump workflow was failing because it was not checking out the source code, which prevented it from finding the formula template. Additionally, the formula template was not being copied before the URL and SHA were stamped, leading to incorrect formula generation. This change also adds a test to ensure that the homebrew formula template correctly reflects the new CLI layout, preventing future regressions.
📝 WalkthroughWalkthroughThe Homebrew bump workflow gains a new step that checks out the release source tag into a ChangesHomebrew Formula Template-Based Bump
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/workbranch-cli/tests/cases/release-config.sh (1)
84-90: ⚡ Quick winAssert workflow ordering, not just presence.
This test currently verifies both lines exist, but not that the template copy happens before stamping, so it can miss the regression described in the failure message.
Suggested patch
expected_workflow_lines = [ 'FORMULA_TEMPLATE="workbranch/packaging/homebrew/workbranch.rb"', 'cp "$FORMULA_TEMPLATE" "$FORMULA_PATH"', ] missing_workflow_lines = [line for line in expected_workflow_lines if line not in workflow] if missing_workflow_lines: raise SystemExit(f'Homebrew bump workflow must copy the formula template before stamping URL/SHA: {missing_workflow_lines}') + +copy_idx = workflow.find('cp "$FORMULA_TEMPLATE" "$FORMULA_PATH"') +stamp_idx = workflow.find("python3 - <<'PY'") +if copy_idx == -1 or stamp_idx == -1 or copy_idx > stamp_idx: + raise SystemExit('Homebrew bump workflow must copy formula template before URL/SHA stamping step')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/workbranch-cli/tests/cases/release-config.sh` around lines 84 - 90, The test currently only verifies that the FORMULA_TEMPLATE assignment and cp command lines exist somewhere in the workflow list, but does not enforce that they appear in the correct order before the stamping operations. Instead of using a simple membership check with the `missing_workflow_lines` list comprehension, find the actual indices of the template-related lines (FORMULA_TEMPLATE and cp) and any stamping operations in the workflow list, then verify that the template copy operations occur at earlier indices than the stamping operations to ensure the ordering constraint is satisfied.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/workbranch-cli/tests/cases/release-config.sh`:
- Around line 84-90: The test currently only verifies that the FORMULA_TEMPLATE
assignment and cp command lines exist somewhere in the workflow list, but does
not enforce that they appear in the correct order before the stamping
operations. Instead of using a simple membership check with the
`missing_workflow_lines` list comprehension, find the actual indices of the
template-related lines (FORMULA_TEMPLATE and cp) and any stamping operations in
the workflow list, then verify that the template copy operations occur at
earlier indices than the stamping operations to ensure the ordering constraint
is satisfied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0cc78824-99c9-47fe-8fee-41d05436c6d5
📒 Files selected for processing (2)
.github/workflows/homebrew-bump.ymlapps/workbranch-cli/tests/cases/release-config.sh
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b58b78dbe0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| PY | ||
| } | ||
|
|
||
| test_homebrew_formula_template_tracks_cli_layout() { |
There was a problem hiding this comment.
This regression test is only defined, but the test runner invokes cases via explicit run_test ... calls and there is no run_test test_homebrew_formula_template_tracks_cli_layout entry in apps/workbranch-cli/tests/run.sh. As a result, ./apps/workbranch-cli/tests/run.sh never executes the new Homebrew formula/workflow guard, so the intended regression coverage is inert until this test is added to main() with the other release-config checks.
Useful? React with 👍 / 👎.
1 participant
The homebrew bump workflow was failing because it was not checking out the source code, which prevented it from finding the formula template. Additionally, the formula template was not being copied before the URL and SHA were stamped, leading to incorrect formula generation.
This change also adds a test to ensure that the homebrew formula template correctly reflects the new CLI layout, preventing future regressions.
Summary by CodeRabbit
Tests
Chores