ci(release): validate agents before publishing binary - #6986
Conversation
PR Summary by QodoValidate agents before publishing release binaries
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Preflight agents against the release tag. Guard the release tag against unexpected movement. Signed-off-by: Ralph Bean <rbean@redhat.com>
b3b80c6 to
8766e4d
Compare
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 7:42 PM UTC · Completed 8:01 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.62 |
|
Risk Assessment: moderate (2/5) DetailsSingle CI workflow file with one protected path and CI workflow flag active, but small change size; high multi-author contention (8 authors in 90d) and recent fix/revert history (4 commits) elevate Tier 2, producing a moderate composite. |
ReviewFindingsHigh
Low
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Findings from a two-model review pass (Codex gpt-5.6-sol + Grok 4.6), verified against the pinned callee before posting.
Posting one finding inline. The tag-verification TOCTOU and the release-runbook contradiction are both already covered by the existing review comments on this PR, so I have not duplicated them.
For reference, the lower-severity items I did not post: the notify-agents-sync-failure if: now has two unreachable clauses (release can no longer be success when validate-agents/resolve-agents fail); release now also blocks on resolve-agents' cross-repo "Resolve agents main" API call, so an unrelated agents-repo blip stops the binary (splitting the tag-verify into its own job would fix both that and the added serialisation); runner-image.yml / sandbox-images.yml still publish GHCR images ungated on the same tag push; and annotated tags are peeled only one level (fails closed).
One thing worth recording since it came up and is not a problem: a reviewer flagged that github.sha might be the tag-object SHA on an annotated-tag push, which would make the new check fail every release. That was checked against this repo's history and is not the case — v0.40.0, v0.39.0 and v0.38.0 are all annotated, and in each the Release run's head_sha is the peeled commit (v0.40.0: tag object 389315ee, peeled 2a103497, run head_sha 2a103497). The one-sided peel in the new step is correct. The job.workflow_sha may be a tag object SHA precedent elsewhere in this repo refers to a different context value.
Moving agents validation ahead of GoReleaser makes publication depend on that gate, which raises the bar for what the gate has to guarantee. Three gaps and their fixes: The tag is verified only before validation. validate-agents clones fullsend by tag name — the called workflow rejects anything but `main` or a `v*` ref and uses `git clone --branch`, so it cannot be pinned to a SHA from this side. A tag moved during the ~45 minutes of functional tests would therefore leave the gate validating one commit while GoReleaser publishes another. Add a `recheck-tag` job between the gate and `release` that verifies the tag again, so publication requires the tag to have been stable across the whole gate. It is a job rather than a step in `release` so that a tag moved mid-gate stays a pre-publication failure with its own notification. Both verify jobs check out `github.sha` rather than the tag, so the guard never runs a copy of itself fetched through the ref it is checking. validate-agents can report success having run no tests. In the pinned callee an empty E2E_GCP_WIF_PROVIDER makes its "Check for secrets" step log a warning, skip GCP auth and every test step, and still exit 0; the roll-up then reports success. Before this change that only cost the agents tag, but it now ships an unvalidated binary. Fail resolve-agents when the secret is missing rather than degrading silently. A blocked release is silent on Slack. notify-agents-sync-failure fires only when `release` succeeded, and `release` can no longer succeed when validate-agents or resolve-agents fail — so those two clauses became unreachable and the pre-publication path lost its notification. Split the two cases: notify-release-blocked for failures before publishing, notify-agents-sync-failure for the one failure that can still happen after. Both now fail loudly on a missing SLACK_WEBHOOK_URL instead of on a bare curl error, matching notify-vouch-slack.yml. The tag check lives in scripts/verify-release-tag.sh so both jobs run the same code. It peels nested annotated tags rather than one level, shape-checks each SHA before it goes back into an API path, requires the resolved object to be a commit, and sanitises `::` in its workflow commands — the two low-severity review findings. `release` also gains the `timeout-minutes` every other job in the file already has (20, on a job that has historically taken ~2). The cutting-releases skill and its runbooks still described the old order, telling the cutter that a validation failure ships the binary. Rewritten to match, including what to do when the release is blocked (re-run the failed jobs; never move the tag). Validation: - `bash scripts/verify-release-tag-test.sh` (13 cases) - `actionlint .github/workflows/release.yml` - `shellcheck scripts/verify-release-tag*.sh` - `make lint` Assisted-by: Claude (fix), Grok (review), Codex (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
|
@ralphbean I took this over and pushed Tag verification now brackets the gate. The qodo HIGH is real but its suggested fix is not available: at the pinned callee The gate could pass having run zero tests. The callee's "Check for secrets" step ( A blocked release was silent on Slack. Supporting changes. The check lives in Docs. One thing worth recording as not a problem, since a reviewer raised it: Validated locally: Happy to back any of this out if you'd rather keep the PR minimal and file the hardening separately. |
|
🤖 Review · ❌ Terminated · Started 7:23 PM UTC · Ended 8:05 PM UTC Commit: |
|
🤖 Finished Retro · ✅ Success · Started 7:50 PM UTC · Completed 8:01 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.55 |
Retro: PR #6986 — ci(release): validate agents before publishing binaryPR #6986 reordered the fullsend-ai/fullsend release dependency graph so agents functional tests must pass before GoReleaser publishes the binary. Three review sources caught legitimate issues: qodo (tag TOCTOU), fullsend-ai-review ($3.62, claude-opus-4-6 at high effort — command injection, SHA validation, protected-path governance), and waynesun09 (vacuous test gate via multi-model review with Codex + Grok). waynesun09 then pushed a comprehensive hardening commit (+419/−31 across 7 files) addressing all findings. The PR was merged ~26 hours after creation. Autonomy readinessNot a candidate for increased autonomy. The most impactful finding — that the Process observations
Evidence for existing issues (no new proposals)
|
|
🤖 Finished Review · ❌ Failure (validation failed after 2 iteration(s)) · Started 7:23 PM UTC · Completed 8:05 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
|
Follow-ups from this PR are now filed:
|
3 participants
Releasing fullsend currently publishes the binary before the agents functional tests finish. That leaves a window where users can receive the new binary while the matching agents tag is still missing.
This changes the release dependency graph so that:
The functional tests already build fullsend from the
fullsend_refinput, so they validate the unreleased source tag rather than a previous published binary.Validation:
actionlint .github/workflows/release.ymlmake lint