ci(release): gate stable publishing on live e2e tests - #6547
Merged
Conversation
Contributor
There was a problem hiding this comment.
🤖 AI Review
The stable-release gate is fail-closed, with no critical or major issue found. Confirmed concerns center on notification gaps, noisy or stale failure alerts, an expensive/error-fragile recovery scan, overly broad secret inheritance, and several CI maintainability nits. The arbitrary-ref and smoke-test permission findings are refuted by the actual caller structure and nested-workflow requirements; the operational impact of concurrent suites remains uncertain.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | .github/workflows/live-e2e-notify.yml:42 |
observability |
claude | A failed workflow run with no job matching the expected suite name exits without sending a Slack notification. |
| 🟡 MINOR | .github/workflows/live-e2e.yml:21 |
security |
claude | The reusable-suite callers use secrets: inherit even though the suite requires only the staging access token, unnecessarily broadening the called workflows' secret context. |
| 🟡 MINOR | .github/workflows/live-e2e-notify.yml:53 |
error-handling |
claude | Recovery detection may fetch every prior workflow-run page and then request jobs for many candidates, while any individual API error aborts the notification step. |
| 🟡 MINOR | .github/workflows/live-e2e-notify.yml:70 |
correctness |
claude+codex | Failure notifications have no deduplication or ordering guard, allowing repeated outage alerts, duplicate stable-release alerts, and a slower older run to post a stale failure after a newer run succeeds. |
| ⚪ NIT | .github/workflows/live-e2e-suite.yml:29 |
maintainability |
claude | The gate and notifier rely on the exact Live e2e job name, while the gate additionally relies on the Run live e2e step name; renaming either silently changes reuse or alerting behavior. |
| ⚪ NIT | .github/workflows/live-e2e-notify.yml:26 |
code-quality |
claude | WORKFLOW_NAME and CONCLUSION are unused, and notifications do not identify whether they came from the routine Live E2E workflow or a release-blocking gate. |
| ⚪ NIT | .github/workflows/live-e2e-suite.yml:30 |
ci |
claude | The new suite, gate, and notifier jobs lack job-level timeouts, allowing non-test steps to run until GitHub's default job timeout. |
| ⚪ NIT | .github/workflows/live-e2e-gate.yml:66 |
ci |
claude | The result job uses always(), so cancellation can still execute its failure branch and emit a misleading live-e2e error. |
| ⚪ NIT | .github/workflows/live-e2e.yml:19 |
ci |
claude | Scheduled, manual, push, and stable-gate suites can overlap against the same staging organization because their concurrency groups do not serialize across those run types. |
| ⚪ NIT | .github/workflows/live-e2e-gate.yml:33 |
ci |
claude | Reuse is coupled to the develop branch and live-e2e.yml filename, so commits without a matching develop run—or a future workflow rename—silently fall back to rerunning the suite. |
Refuted findings (kept for transparency, not posted as review comments)
.github/workflows/live-e2e-suite.yml:5(security): The reusable workflow's ref input permits checkout of an arbitrary ref, recreating the previously documented risk of running selected code with the staging token.
Refuted: The reusable input is not exposed as a workflow_dispatch input. The only real callers pass github.sha or a gate input derived from that release SHA, so the current code does not let an operator or external PR select an arbitrary checkout ref..github/workflows/release-smoke-test.yml:27(security): The smoke-test workflow unnecessarily grants pull-requests: write even though its dry-run execution does not create a pull request.
Refuted: propose-release-notes.yml:50-52 declares pull-requests: write. GitHub validates nested reusable-workflow permissions even when the dry-run condition later skips that job, exactly as release-smoke-test.yml:22-23 explains, so the caller must grant the scope at startup.
Stats
Claude findings: 12 · Codex findings: 1 · Confirmed: 9 · Refuted: 2 · Uncertain: 1
Models: claude-opus-5 + gpt-5.6-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Coly010
approved these changes
Sep 10, 2026
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.
Live staging tests currently run independently of stable publication, so a stable release can ship without a passing live suite for its commit.
Run the suite after every push to
developand every night, with superseded push runs cancelled independently of scheduled and manual runs. Stable publication, including manual releases, requires a passing suite for the exact release commit: reuse a verified successful run ondevelop, or run the suite before publishing. Beta releases retain their existing gates.Send staging startup failures and failure/recovery transitions through the existing Slack release webhook, linking the commit and logs. A bounded history lookup suppresses repeated outcomes and notifications superseded by newer runs or attempts. Stable-gate failures use the existing release-failure notification to avoid duplicate alerts; notification delivery remains independent of publication.
Forward only the staging access token through the live workflow calls, bound job runtimes, and document the reuse and concurrency policy in the maintainers guide. Existing opt-in PR Supabox coverage is unchanged.