fix(ci): use workflow_run trigger for homebrew packaging#95
Conversation
- Add team coordination tools to all agents - Update spec-orchestrator command with lifecycle fixes
The package-homebrew workflow was triggered by release:published, which fires before build-binaries finishes uploading assets. This caused all SHA-256 checksums to be computed from 404 error pages, resulting in identical (wrong) hashes in the formula. Switch to workflow_run trigger so the homebrew job waits for the entire Release workflow to complete. Add a guard that verifies binary SHAs are unique before proceeding.
Benchmark ResultsNo benchmarks configured. Add benchmarks to benches/ directory. Full results available in CI artifacts. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #95 +/- ##
=======================================
Coverage 95.83% 95.83%
=======================================
Files 9 9
Lines 6499 6499
=======================================
Hits 6228 6228
Misses 271 271 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Code Coverage ReportOverall Coverage: 0% SummaryFull HTML report available in CI artifacts. |
There was a problem hiding this comment.
Pull request overview
This PR addresses a race condition in the Homebrew packaging workflow and adds a new spec-orchestrator command for parallel agent-based development, along with updates to agent tool sets for team collaboration.
Changes:
- Switched Homebrew workflow trigger from
release: [published]toworkflow_runto ensure packaging occurs after all release assets are uploaded - Added SHA uniqueness validation to fail fast if binary downloads return 404 error pages
- Added comprehensive spec-orchestrator command for orchestrating large specification implementations using parallel agent teams
- Updated agent tool definitions (rust-developer, test-engineer, code-reviewer) to include team coordination tools (SendMessage, TaskList, TaskGet, TaskUpdate, TaskCreate where applicable)
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/package-homebrew.yml | Changed trigger from release event to workflow_run, updated version extraction to use head_branch, added SHA uniqueness validation for binary assets |
| .claude/commands/spec-orchestrator.md | New command for orchestrating spec implementation via parallel agent teams with discovery, synthesis, and wave-based execution phases |
| .claude/agents/test-engineer.md | Added team coordination tools and reformatted description to single-line YAML |
| .claude/agents/rust-developer.md | Added team coordination tools and reformatted description to single-line YAML |
| .claude/agents/code-reviewer.md | Added team coordination tools (including TaskCreate) and reformatted description to single-line YAML |
| workflow_run: | ||
| workflows: ["Release"] | ||
| types: [completed] |
There was a problem hiding this comment.
The workflow_run trigger is missing branch filtering, which means this workflow will be triggered whenever the "Release" workflow completes on ANY branch, not just tags. The Release workflow is triggered by tags matching "v*..", but workflow_run doesn't automatically inherit this filtering. This could cause the homebrew workflow to run unexpectedly if someone manually triggers the Release workflow via workflow_dispatch on a non-tag branch.
Consider adding branch filtering to restrict this to only run when the Release workflow completes on tag refs. However, GitHub Actions workflow_run doesn't support filtering by tags directly in the trigger configuration - you would need to add a job-level condition that checks the ref type.
| VERSION="${RUN_HEAD_BRANCH#v}" | ||
| else | ||
| VERSION="$INPUT_VERSION" |
There was a problem hiding this comment.
The version extraction logic assumes that head_branch will contain the tag name when the Release workflow is triggered by a tag push. However, for workflow_run events triggered by tag pushes, github.event.workflow_run.head_branch contains the tag ref (e.g., "v1.2.3"), but when manually triggered via workflow_dispatch, the head_branch would be the branch name, not a tag.
Additionally, if the Release workflow is triggered via workflow_dispatch on a branch (which is allowed according to the Release workflow definition), this will attempt to strip "v" from a branch name that might not have that prefix, leading to incorrect version extraction.
Consider adding validation to ensure the VERSION is properly formatted, or add an additional check to verify that the workflow_run was triggered from a tag push.
| VERSION="${RUN_HEAD_BRANCH#v}" | |
| else | |
| VERSION="$INPUT_VERSION" | |
| # Expect RUN_HEAD_BRANCH to be a tag like "v1.2.3" | |
| if printf '%s\n' "$RUN_HEAD_BRANCH" | grep -Eq '^v[0-9]+\.[0-9]+\.[0-9]+([0-9A-Za-z.+-]*)?$'; then | |
| VERSION="${RUN_HEAD_BRANCH#v}" | |
| else | |
| echo "Error: workflow_run triggered from non-tag ref '$RUN_HEAD_BRANCH'. Expected a tag like 'v1.2.3'." >&2 | |
| exit 1 | |
| fi | |
| else | |
| # Normalize INPUT_VERSION by stripping optional leading "v" | |
| RAW_VERSION="$INPUT_VERSION" | |
| RAW_VERSION="${RAW_VERSION#v}" | |
| if ! printf '%s\n' "$RAW_VERSION" | grep -Eq '^[0-9]+\.[0-9]+\.[0-9]+([0-9A-Za-z.+-]*)?$'; then | |
| echo "Error: Provided version '$INPUT_VERSION' is not a valid semantic version (e.g., 1.2.3)." >&2 | |
| exit 1 | |
| fi | |
| VERSION="$RAW_VERSION" |
| # Verify all SHAs are unique (identical = failed downloads) | ||
| SHAS="$ARM64_SHA $AMD64_SHA $LINUX_SHA" | ||
| UNIQUE=$(echo "$SHAS" | tr ' ' '\n' | sort -u | wc -l) | ||
| if [ "$UNIQUE" -lt 3 ]; then | ||
| echo "::error::SHA mismatch: binary SHAs are not unique." |
There was a problem hiding this comment.
The SHA uniqueness validation only checks the three binary SHAs (ARM64, AMD64, LINUX) but doesn't include the completions, man pages, or source tarball in the uniqueness check. If those assets fail to download and return a 404 error page, they would all have identical (but incorrect) SHAs that wouldn't be caught by this validation.
Consider expanding the validation to include all downloaded assets, or at a minimum add the completions and man pages since they are also uploaded by the Release workflow and could be subject to the same race condition.
| # Verify all SHAs are unique (identical = failed downloads) | |
| SHAS="$ARM64_SHA $AMD64_SHA $LINUX_SHA" | |
| UNIQUE=$(echo "$SHAS" | tr ' ' '\n' | sort -u | wc -l) | |
| if [ "$UNIQUE" -lt 3 ]; then | |
| echo "::error::SHA mismatch: binary SHAs are not unique." | |
| # Verify all asset SHAs are unique (identical = failed downloads) | |
| SHAS="$ARM64_SHA $AMD64_SHA $LINUX_SHA $COMP_SHA $MAN_SHA $SRC_SHA" | |
| UNIQUE=$(echo "$SHAS" | tr ' ' '\n' | sort -u | wc -l) | |
| if [ "$UNIQUE" -lt 6 ]; then | |
| echo "::error::SHA mismatch: asset SHAs are not unique." |
|
|
||
| # Verify all SHAs are unique (identical = failed downloads) | ||
| SHAS="$ARM64_SHA $AMD64_SHA $LINUX_SHA" | ||
| UNIQUE=$(echo "$SHAS" | tr ' ' '\n' | sort -u | wc -l) |
There was a problem hiding this comment.
The wc -l command counts the number of newline characters, which can be problematic in shell scripts. If the input doesn't end with a newline, wc -l will undercount by 1. While sort -u typically adds a trailing newline, this behavior can vary across different Unix implementations.
Consider using a more robust counting method, such as piping to grep -c . or explicitly adding a newline before counting. Alternatively, you could directly compare the sorted output against the expected number of lines to ensure all three SHAs are present and unique.
| UNIQUE=$(echo "$SHAS" | tr ' ' '\n' | sort -u | wc -l) | |
| UNIQUE=$(echo "$SHAS" | tr ' ' '\n' | sort -u | grep -c .) |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
- Add semver validation for workflow_run (tag ref check) and workflow_dispatch (input format check) version extraction - Expand SHA uniqueness guard from 3 binary assets to all 6 assets (ARM64, AMD64, Linux, completions, man pages, source tarball) - Replace wc -l with grep -c . for robust unique-SHA counting Co-authored-by: zircote <307960+zircote@users.noreply.github.com>
fix(ci): harden package-homebrew version extraction and SHA validation
Benchmark ResultsNo benchmarks configured. Add benchmarks to benches/ directory. Full results available in CI artifacts. |
Code Coverage ReportOverall Coverage: 0% SummaryFull HTML report available in CI artifacts. |
Summary
package-homebrew.ymltriggered onrelease: [published]beforebuild-binariesfinished uploading assets, causing all SHA-256 checksums to be computed from 404 error pages (all identical)workflow_runtrigger so the homebrew job waits for the entire Release workflow to completezircote/homebrew-tapwith correct per-asset SHAs for v0.3.3Test plan
brew reinstall zircote/tap/nsipsucceeds with corrected formulaworkflow_dispatchwith version input still works for manual runs