Repository navigation
ci(release): approve green, reviewed develop-into-next syncs as supabase-oss - #7124
Conversation
…ase-oss A Claude-resolved sync whose checks are green on a head the AI review saw, with every finding fixed or declined, now gets approved by supabase-oss so fast-forward.yml moves next without waiting for a maintainer. A head moved by a repair commit or a maintainer push still needs a person. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Coly010
left a comment
There was a problem hiding this comment.
a few gaps in what counts as "reviewed and answered" that i think need closing before the bot moves next on its own - inline below.
one that isn't in the diff: if either model job fails or times out, adjudicate swaps in empty findings and post-review still posts the marker (ai-review.yml:427-438), so a half review (or one model skimming a huge next...sync diff) counts as reviewed. could the auto-approve path require both models to have completed?
on your env question - auto-approve only allows develop, so the workflow_run/schedule runs deploy fine. rest looks good to me, and with those four sorted i'm happy to approve!
|
/ai-review |
…ered AI review The sync bot now requires both AI review models to finish, pins the review to the commit the models read, approves only when every unrefuted finding has an answered thread, and holds while a reviewer requests changes or after its approval on that head was dismissed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/ai-review |
There was a problem hiding this comment.
🤖 AI Review
Verified all six findings from both independent reviews; none overlap. Five are confirmed, and the force-push posting failure remains uncertain because its GitHub API trigger is not established by the repository code. The main issue is an approval race that can bypass a maintainer's change request. Other confirmed concerns are workflow compatibility, missing automatic promotion retries, external reviewers blocking automation, and contradictory release documentation.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟠 MAJOR | .github/workflows/sync-repair.yml:170 |
race-condition |
codex | The approval job can approve and promote a sync after a maintainer requests changes because it does not refresh the review eligibility decision. |
| 🟡 MINOR | .github/scripts/ai-review/post-review.ts:1176 |
compatibility |
claude | Older pull_request workflow copies fail to post reviews after develop receives this script because they do not provide the newly required HEAD_SHA and REVIEWS_COMPLETE variables. |
| 🟡 MINOR | .github/scripts/ai-review/post-review.ts:797 |
error-handling |
claude | Pinning commit_id can lose a completed review when GitHub rejects a reviewed commit removed by a force-push, because the 422 fallback retains the rejected SHA. |
| 🟡 MINOR | .github/workflows/sync-repair.yml:176 |
error-handling |
codex | An existing bot approval permanently suppresses automatic promotion retries for that head even when the original promotion failed or was refused. |
| 🟡 MINOR | .github/scripts/sync-agent.ts:630 |
robustness |
claude | A change-request review from an account without write access can indefinitely stop automatic approval of a sync PR. |
| 🟡 MINOR | apps/cli/docs/release-process.md:363 |
documentation |
codex | The release runbook retains a maintainer-approval guarantee for edits that the new automation can land without a person reviewing them. |
Stats
Claude findings: 3 · Codex findings: 3 · Confirmed: 5 · Refuted: 0 · Uncertain: 1
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
…-auto-approve # Conflicts: # .github/scripts/sync-agent.test.ts # .github/scripts/sync-agent.ts
…lands Fast-forward refuses supabase-oss's approval while a collaborator requests changes, so a change request between collect and approve still holds the sync. Older AI review workflow copies post without review state instead of failing, and a rejected pinned review is retried on the current head while its state line still names the reviewed commit. The runbook now describes when Claude's edits land without a maintainer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…is dismissed Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… into avallete/sync-next-auto-approve
jgoux
left a comment
There was a problem hiding this comment.
The maintainer hold on automatic approval can be bypassed (P1), and there are two paths where supabase-oss approves despite findings that were never addressed (P2). Details inline.
Any reviewer's change request now holds the sync, because author_association hides private org membership. Fast-forward reads the approver from the event payload, so sync PRs running an older workflow copy are still re-checked. The AI review marker leads the body so truncation cannot drop it, superseded reviews are ignored, and workflow suggestions are posted before the repair replies so a withheld fix never reads as answered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jgoux
left a comment
There was a problem hiding this comment.
Thanks for the fixes. All four earlier points hold up at this head: change requests count regardless of author_association, the marker leads the body and superseded reviews are skipped, suggestions are posted before replies, and the approver comes from the event payload. One minor test-coverage note inline.
…g markers Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… into avallete/sync-next-auto-approve
Summary
Claude-resolved
sync/develop-into-nextPRs now get approved without a maintainer when the merge is clean in practice:Require fast-forwardis green on the head,.github/workflow suggestions are waiting.sync-repairplans anapproveaction in that state, and a newapprovejob submits an approval for that commit assupabase-ossthrough theauto-approveenvironment. The approval runsfast-forward.ymlas usual, which re-checks the approver, the head and the checks before movingnext.A head moved past the reviewed commit, by a repair commit or a maintainer push, is never re-reviewed, so it still waits for a person. The job skips if the head moved or
supabase-ossalready approved that commit.This reverses the documented rule that the bot never approves sync heads;
release-process.mdrecords the new accepted decision. Reviewers should confirm theauto-approveenvironment's branch rule letssync-repairruns ondevelopdeploy.Linked issue
Checklist
🤖 Generated with Claude Code