From 560bfcca787130d93a94cdd6b443126d4bc54611 Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 13 Oct 2026 15:21:30 +0200 Subject: [PATCH 1/9] ci(release): approve green, reviewed develop-into-next syncs as supabase-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 --- .github/CODEOWNERS | 2 +- .github/scripts/sync-agent.test.ts | 25 ++++++++++++++++++++ .github/scripts/sync-agent.ts | 26 +++++++++++++++----- .github/workflows/sync-repair.yml | 38 ++++++++++++++++++++++++++++++ apps/cli/docs/release-process.md | 4 ++-- 5 files changed, 86 insertions(+), 9 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index adbacd4653..e2ad6c2359 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -2,7 +2,7 @@ * @supabase/cli # Files that bots update. Either owner satisfies code-owner review; `supabase-oss` approves only -# bot-opened PRs, through bot-pr-auto-merge.yml. +# bot-opened PRs, through bot-pr-auto-merge.yml and sync-repair.yml. /.github/workflows/*.yml @supabase/cli @supabase-oss /package.json @supabase/cli @supabase-oss /apps/*/package.json @supabase/cli @supabase-oss diff --git a/.github/scripts/sync-agent.test.ts b/.github/scripts/sync-agent.test.ts index 8db92aa61c..2bc9bbbd50 100644 --- a/.github/scripts/sync-agent.test.ts +++ b/.github/scripts/sync-agent.test.ts @@ -36,6 +36,7 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + reviewedHeads: [], botComments: [RESOLVED], runs: [...green, failed, run(100, "Require fast-forward", "failure")], threads: [], @@ -49,6 +50,7 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + reviewedHeads: [], botComments: [RESOLVED], runs: [...green, cancelled], threads: [], @@ -61,6 +63,7 @@ describe("planRepair", () => { const running = planRepair({ head: HEAD, reviewRunning: false, + reviewedHeads: [], botComments: [RESOLVED], runs: [...green, run(99, "CodeQL", null)], threads: [finding], @@ -68,6 +71,7 @@ describe("planRepair", () => { const missing = planRepair({ head: HEAD, reviewRunning: false, + reviewedHeads: [], botComments: [RESOLVED], runs: green.slice(1), threads: [finding], @@ -81,6 +85,7 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + reviewedHeads: [], botComments: [ "@supabase/cli Claude could not resolve this merge: ", ], @@ -98,6 +103,7 @@ describe("planRepair", () => { runs: green, threads: [finding], reviewRunning: false, + reviewedHeads: [], }); expect(plan.action).toBe("skip"); @@ -107,6 +113,7 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + reviewedHeads: [], botComments: [RESOLVED], runs: [...green, run(-1, "Check code quality", "failure")], threads: [], @@ -119,6 +126,7 @@ describe("planRepair", () => { const state = { head: HEAD, reviewRunning: false, + reviewedHeads: [], botComments: [RESOLVED, repaired(HEAD)], runs: [...green, run(99, "Check code quality", "failure")], }; @@ -131,6 +139,7 @@ describe("planRepair", () => { const state = { head: HEAD, reviewRunning: false, + reviewedHeads: [], runs: [...green, run(99, "Check code quality", "failure")], threads: [], }; @@ -143,6 +152,7 @@ describe("planRepair", () => { const plan = planRepair({ head: "d".repeat(40), reviewRunning: false, + reviewedHeads: [], botComments: [RESOLVED, repaired(HEAD), ""], runs: [...green, run(99, "Check code quality", "failure")], threads: [finding], @@ -155,6 +165,7 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: true, + reviewedHeads: [], botComments: [RESOLVED], runs: [...green, run(99, "Check code quality", "failure")], threads: [], @@ -167,6 +178,7 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + reviewedHeads: [], botComments: [RESOLVED, ``], runs: [...green, run(99, "Check code quality", "failure")], threads: [], @@ -174,6 +186,19 @@ describe("planRepair", () => { expect(plan.action).toBe("skip"); }); + + test("approves a green head the AI review saw, but not one a repair commit moved past it", () => { + const state = { + reviewRunning: false, + reviewedHeads: [HEAD], + botComments: [RESOLVED, repaired(HEAD)], + runs: [...green, run(100, "Require fast-forward", "failure")], + threads: [], + }; + + expect(planRepair({ ...state, head: HEAD })).toEqual({ action: "approve" }); + expect(planRepair({ ...state, head: "e".repeat(40) }).action).toBe("skip"); + }); }); describe("buildSuggestions", () => { diff --git a/.github/scripts/sync-agent.ts b/.github/scripts/sync-agent.ts index 77fe12a382..7c4029941a 100644 --- a/.github/scripts/sync-agent.ts +++ b/.github/scripts/sync-agent.ts @@ -50,6 +50,7 @@ export interface ReviewThread { export type RepairPlan = | { action: "skip"; reason: string } | { action: "exhausted" } + | { action: "approve" } | { action: "repair"; failures: CheckRun[]; threads: ReviewThread[] }; function repairMarker(head: string): string { @@ -93,6 +94,8 @@ export function planRepair(state: { runs: CheckRun[]; threads: ReviewThread[]; reviewRunning: boolean; + /** Commits the AI review posted a review on. */ + reviewedHeads: string[]; }): RepairPlan { const { head, botComments, threads } = state; if (!botComments.some((body) => body.startsWith(RESOLVE_MARKER))) { @@ -121,7 +124,10 @@ export function planRepair(state: { (run) => !["success", "neutral", "skipped"].includes(run.conclusion ?? ""), ); if (threads.length === 0 && failures.length === 0) { - return { action: "skip", reason: "Nothing to repair." }; + // A head the AI review saw has no repair commit on top, so only Claude's resolution and declined findings remain. + return state.reviewedHeads.includes(head) + ? { action: "approve" } + : { action: "skip", reason: "Nothing to repair." }; } const rounds = botComments.filter((body) => body.startsWith(REPAIR_MARKER)).length; const headRepaired = botComments.some((body) => body.startsWith(repairMarker(head))); @@ -648,11 +654,13 @@ async function collect(outputDir: string): Promise { token, `/repos/${repository}/issues/${pull.number}/comments`, )), - ...(await listAll<{ user: { login: string } | null; body: string }>( - token, - `/repos/${repository}/pulls/${pull.number}/reviews`, - )), ]; + const reviews = await listAll<{ + user: { login: string } | null; + body: string; + commit_id: string; + }>(token, `/repos/${repository}/pulls/${pull.number}/reviews`); + comments.push(...reviews); const plan = planRepair({ head: pull.head.sha, botComments: comments @@ -666,12 +674,18 @@ async function collect(outputDir: string): Promise { ), threads: await fetchOpenFindings(token, repository, pull.number), reviewRunning: await isReviewRunning(token, repository, pull.number), + reviewedHeads: reviews + .filter( + (review) => + isLogin(review.user?.login, AI_REVIEW_LOGIN) && review.body.includes(AI_REVIEW_MARKER), + ) + .map((review) => review.commit_id), }); setOutput("action", plan.action); setOutput("pr", String(pull.number)); setOutput("head", pull.head.sha); - if (plan.action === "skip") { + if (plan.action === "skip" || plan.action === "approve") { console.log(plan.reason); return; } diff --git a/.github/workflows/sync-repair.yml b/.github/workflows/sync-repair.yml index 54d3030fb5..44348d9516 100644 --- a/.github/workflows/sync-repair.yml +++ b/.github/workflows/sync-repair.yml @@ -40,6 +40,10 @@ jobs: pull-requests: read checks: read actions: read + outputs: + action: ${{ steps.collect.outputs.action }} + pr: ${{ steps.collect.outputs.pr }} + head: ${{ steps.collect.outputs.head }} env: OUTPUT_DIR: ${{ github.workspace }}/sync-agent RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} @@ -142,3 +146,37 @@ jobs: REPO_DIR="$GITHUB_WORKSPACE/work" HEAD_SHA="$(git -C "$GITHUB_WORKSPACE/work" rev-parse HEAD)" \ bun .github/scripts/sync-agent.ts suggest fi + + # A green head the AI review saw carries only Claude's resolution and its declined findings. The approval runs + # fast-forward.yml, which re-checks the head and its checks before moving `next`. + approve: + name: Approve the reviewed sync + needs: repair + if: needs.repair.outputs.action == 'approve' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: {} + # Holds `AUTO_APPROVE_PR_PAT`; the environment only deploys from the default branch. + environment: auto-approve + steps: + - name: Approve + env: + GH_TOKEN: ${{ secrets.AUTO_APPROVE_PR_PAT }} + PR: ${{ needs.repair.outputs.pr }} + HEAD_SHA: ${{ needs.repair.outputs.head }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + if [ "$(gh pr view "$PR" --repo "$GITHUB_REPOSITORY" --json headRefOid -q .headRefOid)" != "$HEAD_SHA" ]; then + echo "::notice::The sync branch moved since the checks settled; skipping." + exit 0 + fi + login="$(gh api user --jq .login)" + approved="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${PR}/reviews" \ + --jq ".[] | select(.user.login == \"$login\" and .state == \"APPROVED\" and .commit_id == \"$HEAD_SHA\") | .id")" + if [ -n "$approved" ]; then + echo "::notice::${login} already approved ${HEAD_SHA}." + exit 0 + fi + gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/reviews" -f commit_id="$HEAD_SHA" -f event=APPROVE \ + -f body="CI is green and the AI review raised nothing the merge introduced, so this approves Claude's resolution of the sync (${RUN_URL})." > /dev/null diff --git a/apps/cli/docs/release-process.md b/apps/cli/docs/release-process.md index 753cef6099..d9257f12f0 100644 --- a/apps/cli/docs/release-process.md +++ b/apps/cli/docs/release-process.md @@ -356,11 +356,11 @@ git push origin sync/develop-into-next A `develop-into-next` conflict PR is resolved by Claude (the `claude.opus` model in `.github/ai-models.json`) in the same run. The `Resolve conflicts with Claude` job merges `next` into the sync branch and runs Claude Code with file read, search, and edit tools only: no shell, no web tools, no `.git` edits, and no GitHub write token. A `pnpm-lock.yaml` conflict keeps `next`'s lockfile, which the next job regenerates. `Run quick checks on the resolution` formats the merged tree and runs `check:all` and unit tests in a job with no secrets, and `Fix the quick checks with Claude` fixes what fails once, without re-running them. This catches code on one side that still uses something the other renamed or removed; integration and end-to-end failures are left to the PR's CI and the repair loop below. The jobs holding the Anthropic key never execute the merged tree, and the tree passes between jobs as a git bundle. The `Publish the resolution` job pushes the latest finished stage from a fresh checkout with the release App token, comments the resolution and any check that failed, and starts the AI review. Choices between `develop` and `next` behavior are listed as decisions for `@supabase/cli`. If Claude cannot resolve the merge, it comments why and the PR waits for a person as above. -[`sync-repair.yml`](../../../.github/workflows/sync-repair.yml) (`Sync repair`) runs when a CI workflow or the AI review finishes, and hourly. Once every check on the PR head has finished and no `AI Review` run for the PR is still going, it gives Claude the failed jobs' log tails and the AI review's unanswered threads. Claude fixes what the merge broke and replies on each finding, declining findings the merge did not introduce. The fix is pushed as a `chore(repo)` commit, fixed threads are resolved, and a round comment is posted. A head gets another round only for new review findings. When a round leaves its head failing, or after two rounds, the bot asks `@supabase/cli` to take over and stops repairing that PR. The repair loop reads only CI output and the AI review's own threads, never other people's comments, and only acts on PRs Claude resolved. +[`sync-repair.yml`](../../../.github/workflows/sync-repair.yml) (`Sync repair`) runs when a CI workflow or the AI review finishes, and hourly. Once every check on the PR head has finished and no `AI Review` run for the PR is still going, it gives Claude the failed jobs' log tails and the AI review's unanswered threads. Claude fixes what the merge broke and replies on each finding, declining findings the merge did not introduce. The fix is pushed as a `chore(repo)` commit, fixed threads are resolved, and a round comment is posted. A head gets another round only for new review findings. When a round leaves its head failing, or after two rounds, the bot asks `@supabase/cli` to take over and stops repairing that PR. When every check except `Require fast-forward` is green on a head the AI review saw, and Claude fixed or declined every finding, `supabase-oss` approves that head, which fast-forwards `next`. A head moved by a repair commit or a maintainer push is not re-reviewed, so it waits for a maintainer's approval. The repair loop reads only CI output and the AI review's own threads, never other people's comments, and only acts on PRs Claude resolved. Workflows on the sync branch run with repository secrets, so Claude never pushes a change under `.github/`. Claude resolves and fixes workflow files like any other, but those edits are posted as a review of suggestions, with a diff for file additions and deletions; a conflicted workflow keeps `develop`'s side until then. A maintainer reviews them and uses **Commit suggestions**, so the workflow change runs only once a person commits it, and repair waits for that. The publish job refuses to push when the merge's `.github/` differs from git's own merge (with `develop`'s side on conflicts) or a later commit touches `.github/`, and the repair token has no `workflows` permission. Claude's other edits run in the sync PR's CI before anyone reviews them, and land only through a maintainer's approval like any sync PR. -The PR is authored by the release App, so the person who resolved it can approve it. This is an accepted decision: sync resolvers are trusted maintainers with write access, and the resolution content is not independently reviewed. `bot-pr-auto-merge.yml` excludes both sync heads so the bot never approves or auto-merges them. **Approving the PR fast-forwards the target to the PR head** and deletes the sync branch; do not use the merge button (the repo is squash-only and squashing destroys the merge ancestry the sync relies on). The `Require fast-forward` check fails on these PRs to enforce that. +The PR is authored by the release App, so the person who resolved it can approve it. This is an accepted decision: sync resolvers are trusted maintainers with write access, and the resolution content is not independently reviewed. `bot-pr-auto-merge.yml` excludes both sync heads so the bot never approves or auto-merges them; only `Sync repair` approves, on the conditions above. This is an accepted decision: Claude's resolution and its own judgement of which findings the merge introduced reach `next` without a person reviewing them. **Approving the PR fast-forwards the target to the PR head** and deletes the sync branch; do not use the merge button (the repo is squash-only and squashing destroys the merge ancestry the sync relies on). The `Require fast-forward` check fails on these PRs to enforce that. #### Fast-forward on approval From eab1eaa8623648cea360c7bd3f5b193feed239f1 Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 13 Oct 2026 15:36:45 +0200 Subject: [PATCH 2/9] ci(release): log the approve plan without a skip reason Co-Authored-By: Claude Opus 5.5 --- .github/scripts/sync-agent.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/sync-agent.ts b/.github/scripts/sync-agent.ts index 7c4029941a..66c63053e1 100644 --- a/.github/scripts/sync-agent.ts +++ b/.github/scripts/sync-agent.ts @@ -686,7 +686,7 @@ async function collect(outputDir: string): Promise { setOutput("head", pull.head.sha); if (plan.action === "skip" || plan.action === "approve") { - console.log(plan.reason); + console.log(plan.action === "skip" ? plan.reason : "Ready for approval."); return; } if (plan.action === "exhausted") { From 0d3d0eb6764c98982ae15e8cd9ba266b55e50b63 Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 13 Oct 2026 17:27:09 +0200 Subject: [PATCH 3/9] ci(release): approve a sync only after a complete, pinned, fully answered 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 --- .../ai-review/generate-pr-diff.test.ts | 27 ++++ .github/scripts/ai-review/generate-pr-diff.ts | 16 ++- .github/scripts/ai-review/post-review.test.ts | 32 +++++ .github/scripts/ai-review/post-review.ts | 43 ++++++- .github/scripts/ai-review/resolve.test.ts | 1 + .github/scripts/ai-review/resolve.ts | 17 ++- .github/scripts/sync-agent.test.ts | 78 +++++++++--- .github/scripts/sync-agent.ts | 120 +++++++++++++----- .github/workflows/ai-review.yml | 19 ++- .github/workflows/sync-repair.yml | 13 +- apps/cli/docs/release-process.md | 2 +- 11 files changed, 296 insertions(+), 72 deletions(-) diff --git a/.github/scripts/ai-review/generate-pr-diff.test.ts b/.github/scripts/ai-review/generate-pr-diff.test.ts index 471b00691c..bf58f72002 100644 --- a/.github/scripts/ai-review/generate-pr-diff.test.ts +++ b/.github/scripts/ai-review/generate-pr-diff.test.ts @@ -87,4 +87,31 @@ describe("generatePrDiff", () => { expect(diff).toContain("+feature"); expect(diff).not.toContain("base-only.txt"); }); + + test("diffs the pinned head even after the PR branch moves", () => { + const { seed, checkout } = setupRepository(); + git(seed, "switch", "-c", "feature"); + writeFileSync(join(seed, "reviewed.txt"), "reviewed\n"); + git(seed, "add", "reviewed.txt"); + git(seed, "commit", "-m", "reviewed change"); + publishPullRequest(seed, 88); + const headSha = git(seed, "rev-parse", "HEAD"); + writeFileSync(join(seed, "late.txt"), "pushed during the review\n"); + git(seed, "add", "late.txt"); + git(seed, "commit", "-m", "late change"); + publishPullRequest(seed, 88); + + const outputPath = join(checkout, "pr.diff"); + generatePrDiff({ + repositoryPath: checkout, + prNumber: 88, + baseRef: "develop", + outputPath, + headSha, + }); + + const diff = readFileSync(outputPath, "utf8"); + expect(diff).toContain("+reviewed"); + expect(diff).not.toContain("late.txt"); + }); }); diff --git a/.github/scripts/ai-review/generate-pr-diff.ts b/.github/scripts/ai-review/generate-pr-diff.ts index dc6b84d54f..9d7d78f571 100644 --- a/.github/scripts/ai-review/generate-pr-diff.ts +++ b/.github/scripts/ai-review/generate-pr-diff.ts @@ -7,6 +7,8 @@ export interface GeneratePrDiffOptions { prNumber: number; baseRef: string; outputPath: string; + /** Commit to diff instead of the PR's current head, so a push during the review is not reviewed unseen. */ + headSha?: string; } function runGit(repositoryPath: string, args: string[]): void { @@ -22,19 +24,22 @@ function runGit(repositoryPath: string, args: string[]): void { } } -function validateInputs(prNumber: number, baseRef: string): void { +function validateInputs(prNumber: number, baseRef: string, headSha?: string): void { if (!Number.isSafeInteger(prNumber) || prNumber <= 0) { throw new Error(`Invalid PR number: ${prNumber}`); } if (baseRef.length === 0) { throw new Error("Base ref must not be empty"); } + if (headSha !== undefined && !/^[0-9a-f]{40}$/.test(headSha)) { + throw new Error(`Invalid head SHA: ${headSha}`); + } } export function generatePrDiff(options: GeneratePrDiffOptions): void { const repositoryPath = resolve(options.repositoryPath); const outputPath = resolve(options.outputPath); - validateInputs(options.prNumber, options.baseRef); + validateInputs(options.prNumber, options.baseRef, options.headSha); runGit(repositoryPath, ["check-ref-format", `refs/heads/${options.baseRef}`]); const baseRef = "refs/ai-review/base"; @@ -45,7 +50,7 @@ export function generatePrDiff(options: GeneratePrDiffOptions): void { "--no-tags", "origin", `+refs/heads/${options.baseRef}:${baseRef}`, - `+refs/pull/${options.prNumber}/head:${headRef}`, + `+${options.headSha ?? `refs/pull/${options.prNumber}/head`}:${headRef}`, ]); mkdirSync(dirname(outputPath), { recursive: true }); @@ -78,14 +83,15 @@ export function generatePrDiff(options: GeneratePrDiffOptions): void { } function parseArguments(args: string[]): GeneratePrDiffOptions { - if (args.length !== 2) { - throw new Error("Usage: generate-pr-diff.ts "); + if (args.length < 2 || args.length > 3) { + throw new Error("Usage: generate-pr-diff.ts [head-sha]"); } return { repositoryPath: process.cwd(), prNumber: Number(args[0]), baseRef: args[1] ?? "", outputPath: "/tmp/ai-review/pr.diff", + headSha: args[2] || undefined, }; } diff --git a/.github/scripts/ai-review/post-review.test.ts b/.github/scripts/ai-review/post-review.test.ts index be1442b79e..f17a5623bb 100644 --- a/.github/scripts/ai-review/post-review.test.ts +++ b/.github/scripts/ai-review/post-review.test.ts @@ -10,6 +10,7 @@ import { type MarkedEntry, type MergedFinding, type MergedReview, + parseAiReviewState, parseDiffAnchors, partitionFindings, postConsolidatedReview, @@ -535,6 +536,8 @@ describe("renderReviewBody", () => { trigger: "auto", runUrl: "https://example.com/run/9", modelsFooter: "`claude-fable-5` + `gpt-5.6-sol`", + headSha: "a".repeat(40), + reviewsComplete: true, }; test("shows 'No issues found.' when nothing was posted", () => { @@ -672,6 +675,8 @@ describe("buildReviewPayload", () => { trigger: "manual", runUrl: "https://example.com/run/1", modelsFooter: "`claude-fable-5` + `gpt-5.6-sol`", + headSha: "a".repeat(40), + reviewsComplete: true, }; test("event is always COMMENT", () => { @@ -686,6 +691,27 @@ describe("buildReviewPayload", () => { expect(payload.body).toContain(AI_REVIEW_MARKER); }); + test("posts on the reviewed head and records every unrefuted finding, inline or not", () => { + const review = makeMergedReview({ + findings: [ + makeFinding({ id: "inline", file: "file.ts", line: 10 }), + makeFinding({ id: "outside", file: "file.ts", line: 99 }), + makeFinding({ id: "refuted", adjudication: { verdict: "refuted", reason: "No." } }), + ], + }); + const payload = buildReviewPayload(review, anchors, { ...footer, reviewsComplete: false }); + + expect(payload.commit_id).toBe(footer.headSha); + expect(payload.comments).toHaveLength(1); + expect( + parseAiReviewState(truncateReviewBody(`${payload.body}${"x".repeat(70_000)}`, footer.runUrl)), + ).toEqual({ + head: footer.headSha, + complete: false, + findings: 2, + }); + }); + test("the injected models footer appears in the body verbatim", () => { const review = makeMergedReview({ findings: [] }); const payload = buildReviewPayload(review, anchors, footer); @@ -832,6 +858,8 @@ describe("next-impact findings", () => { trigger: "auto", runUrl: "https://example.com/run/2", modelsFooter: "`claude-fable-5` + `gpt-5.6-sol`", + headSha: "a".repeat(40), + reviewsComplete: true, }; const anchors = parseDiffAnchors(SINGLE_HUNK_DIFF); @@ -883,6 +911,8 @@ describe("foldInlineCommentsIntoBody", () => { trigger: "manual", runUrl: "https://example.com/run/1", modelsFooter: "`claude-fable-5` + `gpt-5.6-sol`", + headSha: "a".repeat(40), + reviewsComplete: true, }; test("returns the same payload unchanged when there are no inline comments", () => { @@ -1047,6 +1077,8 @@ describe("post flow via injected ReviewIo", () => { trigger: "manual", runUrl: "https://example.com/run/1", modelsFooter: "`claude-fable-5` + `gpt-5.6-sol`", + headSha: "a".repeat(40), + reviewsComplete: true, }; function makeReviewIo( diff --git a/.github/scripts/ai-review/post-review.ts b/.github/scripts/ai-review/post-review.ts index 6a7d1dbd1e..e7ff9252ac 100644 --- a/.github/scripts/ai-review/post-review.ts +++ b/.github/scripts/ai-review/post-review.ts @@ -24,6 +24,29 @@ */ export const AI_REVIEW_MARKER = ""; +const AI_REVIEW_STATE_PATTERN = + /^$/m; + +/** What a posted AI review covered, read from the hidden state line at the top of its body. */ +export interface AiReviewState { + head: string; + /** Whether both the Claude and the Codex review finished. */ + complete: boolean; + /** Findings posted that were not refuted, inline or in the body. */ + findings: number; +} + +export function renderAiReviewState(state: AiReviewState): string { + return ``; +} + +export function parseAiReviewState(body: string): AiReviewState | null { + const match = AI_REVIEW_STATE_PATTERN.exec(body); + if (!match?.[1] || !match[2] || !match[3]) { + return null; + } + return { head: match[1], complete: match[2] === "true", findings: Number(match[3]) }; +} const SUPERSEDED_SUMMARY = "Superseded by a newer AI review"; /** Hidden marker `isSuperseded` checks for. Kept out of the human-readable * `SUPERSEDED_SUMMARY` text, and broken by `sanitizeModelText`, so a model @@ -606,6 +629,9 @@ export interface ReviewFooterInfo { /** e.g. `` `claude-fable-5` + `gpt-5.6-sol` ``. Sourced from * `.github/ai-models.json` through the workflow's model env vars. */ modelsFooter: string; + /** Commit the models reviewed; the review is posted on it. */ + headSha: string; + reviewsComplete: boolean; } /** Renders the full review body: summary, findings table, out-of-diff section, refuted details, stats, and footer. */ @@ -619,7 +645,10 @@ export function renderReviewBody( ); const verdicts = computeVerdictCounts(review.findings); - const sections: string[] = [`## 🤖 AI Review\n\n${sanitizeModelText(review.summary)}`]; + // The state line leads the body so truncation never drops it. + const sections: string[] = [ + `${renderAiReviewState({ head: footer.headSha, complete: footer.reviewsComplete, findings: posted.length })}\n## 🤖 AI Review\n\n${sanitizeModelText(review.summary)}`, + ]; if (posted.length > 0) { const rows = posted.map( @@ -715,6 +744,7 @@ export interface InlineReviewComment { } export interface ReviewPayload { + commit_id: string; event: "COMMENT"; body: string; comments: InlineReviewComment[]; @@ -763,7 +793,12 @@ export function buildReviewPayload( const body = renderReviewBody(review, partitioned, footer); // A body-only review has no 422 fold-retry to truncate it later, so // truncate the first payload too rather than posting an oversized body. - return { event: "COMMENT", body: truncateReviewBody(body, footer.runUrl), comments }; + return { + commit_id: footer.headSha, + event: "COMMENT", + body: truncateReviewBody(body, footer.runUrl), + comments, + }; } /** Folds every inline comment into the review body, for the 422-retry path when GitHub rejects an anchor. */ @@ -1138,6 +1173,8 @@ async function runPost(): Promise { const prDiffPath = requireEnv("PR_DIFF_PATH"); const claudeModel = requireEnv("CLAUDE_MODEL"); const codexModel = requireEnv("CODEX_MODEL"); + const headSha = requireEnv("HEAD_SHA"); + const reviewsComplete = requireEnv("REVIEWS_COMPLETE") === "true"; const raw: unknown = JSON.parse(await Bun.file(mergedReviewPath).text()); assertMergedReview(raw); @@ -1147,6 +1184,8 @@ async function runPost(): Promise { trigger, runUrl, modelsFooter: `\`${claudeModel}\` + \`${codexModel}\``, + headSha, + reviewsComplete, }); console.log(`Posted AI review on PR #${prNumber} (${raw.findings.length} finding(s)).`); } diff --git a/.github/scripts/ai-review/resolve.test.ts b/.github/scripts/ai-review/resolve.test.ts index 07453c1c32..f4d77aea58 100644 --- a/.github/scripts/ai-review/resolve.test.ts +++ b/.github/scripts/ai-review/resolve.test.ts @@ -24,6 +24,7 @@ function makePr(overrides: Partial = {}): PrDetails { authorLogin: PR_AUTHOR, headRepoFullName: REPO, baseRepoFullName: REPO, + headSha: "a".repeat(40), ...overrides, }; } diff --git a/.github/scripts/ai-review/resolve.ts b/.github/scripts/ai-review/resolve.ts index 58df368df1..0cfaafc071 100644 --- a/.github/scripts/ai-review/resolve.ts +++ b/.github/scripts/ai-review/resolve.ts @@ -61,6 +61,8 @@ export interface PrDetails { headRepoFullName: string; /** `owner/name` of the repository the PR targets. */ baseRepoFullName: string; + /** Head commit when the run resolved; every job reviews this commit even if the branch moves. */ + headSha: string; } /** A prior review or issue comment, checked for the dedup marker. */ @@ -85,13 +87,15 @@ export interface ResolveResult { /** Human-readable explanation, present whenever `shouldRun` is false. */ skipReason?: string; trigger: Trigger; + /** Commit every job reviews, present whenever `shouldRun` is true. */ + headSha?: string; } /** No size gate: Claude and Codex review agentically over many turns, so any * PR that clears the draft/bot/fork/dedup checks is reviewed regardless of * size, best-effort within the model's context/turn budget. */ -function decideForPr(trigger: Trigger): ResolveResult { - return { shouldRun: true, trigger }; +function decideForPr(trigger: Trigger, pr: PrDetails): ResolveResult { + return { shouldRun: true, trigger, headSha: pr.headSha }; } /** @@ -156,7 +160,7 @@ export async function resolveDecision(input: ResolveInput, io: ResolveIo): Promi } // A maintainer explicitly asked, so the marker/dedup guard and the // draft/fork/bot skips below don't apply. - return decideForPr(trigger); + return decideForPr(trigger, pr); } if (pr.draft) { @@ -206,7 +210,7 @@ export async function resolveDecision(input: ResolveInput, io: ResolveIo): Promi }; } - return decideForPr(trigger); + return decideForPr(trigger, pr); } function requireEnv(name: string): string { @@ -243,7 +247,7 @@ interface RestPullRequest { state: "open" | "closed"; draft: boolean; user: { login: string; type: string } | null; - head: { repo: { full_name: string } | null }; + head: { sha: string; repo: { full_name: string } | null }; base: { repo: { full_name: string } }; } @@ -274,6 +278,7 @@ function assertRestPullRequest(value: unknown): asserts value is RestPullRequest typeof value.user.type === "string") ) || !isRecordEntry(value.head) || + typeof value.head.sha !== "string" || !( value.head.repo === null || (isRecordEntry(value.head.repo) && typeof value.head.repo.full_name === "string") @@ -314,6 +319,7 @@ async function fetchPullRequest(token: string, base: string, prNumber: number): authorLogin: pr.user?.login ?? "", headRepoFullName: pr.head.repo?.full_name ?? "", baseRepoFullName: pr.base.repo.full_name, + headSha: pr.head.sha, }; } @@ -369,6 +375,7 @@ function writeOutputs(result: ResolveResult, prNumber: number): void { should_run: String(result.shouldRun), pr_number: String(prNumber), head_ref: `refs/pull/${prNumber}/head`, + head_sha: result.headSha ?? "", trigger: result.trigger, }; const lines = Object.entries(entries).map(([name, value]) => { diff --git a/.github/scripts/sync-agent.test.ts b/.github/scripts/sync-agent.test.ts index 2bc9bbbd50..77738616da 100644 --- a/.github/scripts/sync-agent.test.ts +++ b/.github/scripts/sync-agent.test.ts @@ -7,6 +7,7 @@ import { type CheckRun, REQUIRED_CHECKS } from "./fast-forward.ts"; import { createTestRepo, git, type TestRepo } from "./promotion-test-repo.ts"; import { buildSuggestions, + hasChangesRequested, leftoverConflictMarkers, planRepair, type ReviewThread, @@ -36,7 +37,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, - reviewedHeads: [], + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, failed, run(100, "Require fast-forward", "failure")], threads: [], @@ -50,7 +52,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, - reviewedHeads: [], + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, cancelled], threads: [], @@ -63,7 +66,8 @@ describe("planRepair", () => { const running = planRepair({ head: HEAD, reviewRunning: false, - reviewedHeads: [], + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, run(99, "CodeQL", null)], threads: [finding], @@ -71,7 +75,8 @@ describe("planRepair", () => { const missing = planRepair({ head: HEAD, reviewRunning: false, - reviewedHeads: [], + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: green.slice(1), threads: [finding], @@ -85,7 +90,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, - reviewedHeads: [], + aiReview: null, + changesRequested: false, botComments: [ "@supabase/cli Claude could not resolve this merge: ", ], @@ -103,7 +109,8 @@ describe("planRepair", () => { runs: green, threads: [finding], reviewRunning: false, - reviewedHeads: [], + aiReview: null, + changesRequested: false, }); expect(plan.action).toBe("skip"); @@ -113,20 +120,22 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, - reviewedHeads: [], + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, run(-1, "Check code quality", "failure")], threads: [], }); - expect(plan).toEqual({ action: "skip", reason: "Nothing to repair." }); + expect(plan.action).toBe("skip"); }); test("hands a head a round left failing to a maintainer unless new findings arrive", () => { const state = { head: HEAD, reviewRunning: false, - reviewedHeads: [], + aiReview: null, + changesRequested: false, botComments: [RESOLVED, repaired(HEAD)], runs: [...green, run(99, "Check code quality", "failure")], }; @@ -139,7 +148,8 @@ describe("planRepair", () => { const state = { head: HEAD, reviewRunning: false, - reviewedHeads: [], + aiReview: null, + changesRequested: false, runs: [...green, run(99, "Check code quality", "failure")], threads: [], }; @@ -152,7 +162,8 @@ describe("planRepair", () => { const plan = planRepair({ head: "d".repeat(40), reviewRunning: false, - reviewedHeads: [], + aiReview: null, + changesRequested: false, botComments: [RESOLVED, repaired(HEAD), ""], runs: [...green, run(99, "Check code quality", "failure")], threads: [finding], @@ -165,7 +176,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: true, - reviewedHeads: [], + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, run(99, "Check code quality", "failure")], threads: [], @@ -178,7 +190,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, - reviewedHeads: [], + aiReview: null, + changesRequested: false, botComments: [RESOLVED, ``], runs: [...green, run(99, "Check code quality", "failure")], threads: [], @@ -187,17 +200,46 @@ describe("planRepair", () => { expect(plan.action).toBe("skip"); }); - test("approves a green head the AI review saw, but not one a repair commit moved past it", () => { + test("approves a green head only when both models reviewed it and every finding has an answered thread", () => { + const reviewed = { head: HEAD, complete: true, findings: 2, answeredThreads: 2 }; const state = { + head: HEAD, reviewRunning: false, - reviewedHeads: [HEAD], - botComments: [RESOLVED, repaired(HEAD)], + botComments: [RESOLVED], runs: [...green, run(100, "Require fast-forward", "failure")], threads: [], + changesRequested: false, }; - expect(planRepair({ ...state, head: HEAD })).toEqual({ action: "approve" }); - expect(planRepair({ ...state, head: "e".repeat(40) }).action).toBe("skip"); + expect(planRepair({ ...state, aiReview: reviewed })).toEqual({ action: "approve" }); + for (const plan of [ + planRepair({ ...state, aiReview: reviewed, head: "e".repeat(40) }), + planRepair({ ...state, aiReview: { ...reviewed, complete: false } }), + planRepair({ ...state, aiReview: { ...reviewed, answeredThreads: 1 } }), + planRepair({ ...state, aiReview: reviewed, changesRequested: true }), + planRepair({ ...state, aiReview: null }), + ]) { + expect(plan.action).toBe("skip"); + } + }); +}); + +describe("hasChangesRequested", () => { + const review = (login: string, state: string) => ({ user: { login }, state }); + + test("holds while a reviewer's latest verdict requests changes, ignoring later comments", () => { + expect( + hasChangesRequested([ + review("maintainer", "CHANGES_REQUESTED"), + review("maintainer", "COMMENTED"), + ]), + ).toBe(true); + expect( + hasChangesRequested([ + review("maintainer", "CHANGES_REQUESTED"), + review("maintainer", "APPROVED"), + ]), + ).toBe(false); }); }); diff --git a/.github/scripts/sync-agent.ts b/.github/scripts/sync-agent.ts index 66c63053e1..afae42d6d4 100644 --- a/.github/scripts/sync-agent.ts +++ b/.github/scripts/sync-agent.ts @@ -6,7 +6,11 @@ import { spawnSync } from "node:child_process"; import { appendFileSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; -import { AI_REVIEW_MARKER } from "./ai-review/post-review.ts"; +import { + AI_REVIEW_MARKER, + type AiReviewState, + parseAiReviewState, +} from "./ai-review/post-review.ts"; import { type CheckRun, fetchPullRequestCheckRuns, REQUIRED_CHECKS } from "./fast-forward.ts"; import { githubGraphql, @@ -94,8 +98,10 @@ export function planRepair(state: { runs: CheckRun[]; threads: ReviewThread[]; reviewRunning: boolean; - /** Commits the AI review posted a review on. */ - reviewedHeads: string[]; + /** The latest AI review, with how many of its inline threads are resolved or answered by the release bot. */ + aiReview: (AiReviewState & { answeredThreads: number }) | null; + /** Whether any reviewer's latest verdict is "Request changes". */ + changesRequested: boolean; }): RepairPlan { const { head, botComments, threads } = state; if (!botComments.some((body) => body.startsWith(RESOLVE_MARKER))) { @@ -124,10 +130,7 @@ export function planRepair(state: { (run) => !["success", "neutral", "skipped"].includes(run.conclusion ?? ""), ); if (threads.length === 0 && failures.length === 0) { - // A head the AI review saw has no repair commit on top, so only Claude's resolution and declined findings remain. - return state.reviewedHeads.includes(head) - ? { action: "approve" } - : { action: "skip", reason: "Nothing to repair." }; + return planApproval(head, state.aiReview, state.changesRequested); } const rounds = botComments.filter((body) => body.startsWith(REPAIR_MARKER)).length; const headRepaired = botComments.some((body) => body.startsWith(repairMarker(head))); @@ -137,6 +140,33 @@ export function planRepair(state: { return { action: "repair", failures, threads }; } +/** + * Approves only a head both models reviewed, where every finding they did not refute has a thread Claude fixed or + * declined. Findings only in the review body have no thread, so they wait for a maintainer. + */ +function planApproval( + head: string, + review: (AiReviewState & { answeredThreads: number }) | null, + changesRequested: boolean, +): RepairPlan { + if (changesRequested) { + return { action: "skip", reason: "A reviewer requested changes." }; + } + if (review?.head !== head) { + return { action: "skip", reason: "Nothing to repair; the AI review did not see this head." }; + } + if (!review.complete) { + return { action: "skip", reason: "Only one model finished the AI review." }; + } + if (review.answeredThreads < review.findings) { + return { + action: "skip", + reason: "AI review findings without an answered thread need a maintainer.", + }; + } + return { action: "approve" }; +} + /** Lines that still carry a conflict marker, as `path:line`. */ export function leftoverConflictMarkers(workDir: string, paths: string[]): string[] { return paths.flatMap((path) => { @@ -528,7 +558,7 @@ query($owner: String!, $name: String!, $number: Int!) { id isResolved comments(first: 50) { - nodes { databaseId body path line author { login } pullRequestReview { body } } + nodes { databaseId body path line author { login } pullRequestReview { databaseId body } } } } } @@ -550,7 +580,7 @@ interface ThreadsPage { path: string; line: number | null; author: { login: string } | null; - pullRequestReview: { body: string } | null; + pullRequestReview: { databaseId: number; body: string } | null; }[]; }; }[]; @@ -559,37 +589,56 @@ interface ThreadsPage { } | null; } -/** Unresolved AI review threads the release bot has not answered; nobody else's comments reach the agent. */ -async function fetchOpenFindings( +/** + * AI review threads, answered when resolved or replied to by the release bot. Only the AI review's own comments are + * read, so nobody else's text reaches the agent. + */ +async function fetchAiReviewThreads( token: string, repository: string, number: number, -): Promise { +): Promise<{ thread: ReviewThread; reviewId: number; answered: boolean }[]> { const [owner, name] = repository.split("/"); const page = await githubGraphql(token, THREADS_QUERY, { owner, name, number }); return (page.repository?.pullRequest?.reviewThreads.nodes ?? []).flatMap((thread) => { const [first, ...rest] = thread.comments.nodes; if ( - thread.isResolved || - !first || - !isLogin(first.author?.login, AI_REVIEW_LOGIN) || - !first.pullRequestReview?.body.includes(AI_REVIEW_MARKER) || - rest.some((comment) => isLogin(comment.author?.login, RELEASE_BOT_LOGIN)) + !first?.pullRequestReview?.body.includes(AI_REVIEW_MARKER) || + !isLogin(first.author?.login, AI_REVIEW_LOGIN) ) { return []; } return [ { - threadId: thread.id, - commentId: first.databaseId, - path: first.path, - line: first.line, - body: first.body, + thread: { + threadId: thread.id, + commentId: first.databaseId, + path: first.path, + line: first.line, + body: first.body, + }, + reviewId: first.pullRequestReview.databaseId, + answered: + thread.isResolved || + rest.some((comment) => isLogin(comment.author?.login, RELEASE_BOT_LOGIN)), }, ]; }); } +/** Whether any reviewer's latest approving or blocking review requests changes; comments do not change a verdict. */ +export function hasChangesRequested( + reviews: { user: { login: string } | null; state: string }[], +): boolean { + const verdicts = new Map(); + for (const review of reviews) { + if (review.user && ["APPROVED", "CHANGES_REQUESTED", "DISMISSED"].includes(review.state)) { + verdicts.set(review.user.login, review.state); + } + } + return [...verdicts.values()].includes("CHANGES_REQUESTED"); +} + async function logTail(token: string, repository: string, jobId: number): Promise { const response = await fetch( `https://api.github.com/repos/${repository}/actions/jobs/${jobId}/logs`, @@ -656,11 +705,18 @@ async function collect(outputDir: string): Promise { )), ]; const reviews = await listAll<{ + id: number; user: { login: string } | null; body: string; - commit_id: string; + state: string; }>(token, `/repos/${repository}/pulls/${pull.number}/reviews`); comments.push(...reviews); + const aiReview = reviews.findLast( + (review) => + isLogin(review.user?.login, AI_REVIEW_LOGIN) && review.body.includes(AI_REVIEW_MARKER), + ); + const aiReviewState = aiReview ? parseAiReviewState(aiReview.body) : null; + const aiThreads = await fetchAiReviewThreads(token, repository, pull.number); const plan = planRepair({ head: pull.head.sha, botComments: comments @@ -672,14 +728,18 @@ async function collect(outputDir: string): Promise { pull.number, pull.head.sha, ), - threads: await fetchOpenFindings(token, repository, pull.number), + threads: aiThreads.filter((entry) => !entry.answered).map((entry) => entry.thread), reviewRunning: await isReviewRunning(token, repository, pull.number), - reviewedHeads: reviews - .filter( - (review) => - isLogin(review.user?.login, AI_REVIEW_LOGIN) && review.body.includes(AI_REVIEW_MARKER), - ) - .map((review) => review.commit_id), + aiReview: + aiReview && aiReviewState + ? { + ...aiReviewState, + answeredThreads: aiThreads.filter( + (entry) => entry.reviewId === aiReview.id && entry.answered, + ).length, + } + : null, + changesRequested: hasChangesRequested(reviews), }); setOutput("action", plan.action); setOutput("pr", String(pull.number)); diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml index 5140c4142b..582874175f 100644 --- a/.github/workflows/ai-review.yml +++ b/.github/workflows/ai-review.yml @@ -66,6 +66,8 @@ jobs: should_run: ${{ steps.resolve.outputs.should_run }} pr_number: ${{ steps.resolve.outputs.pr_number }} head_ref: ${{ steps.resolve.outputs.head_ref }} + # Every job reviews this commit, so a push during the run is never reported as reviewed. + head_sha: ${{ steps.resolve.outputs.head_sha }} trigger: ${{ steps.resolve.outputs.trigger }} # Read once here so every job in the run uses the same models, even if the file changes mid-run. claude_model: ${{ steps.models.outputs.claude_model }} @@ -116,7 +118,7 @@ jobs: - name: Checkout PR head (untrusted; review subject matter only) uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ needs.resolve.outputs.head_ref }} + ref: ${{ needs.resolve.outputs.head_sha }} path: pr fetch-depth: 1 persist-credentials: false @@ -143,13 +145,14 @@ jobs: env: GH_TOKEN: ${{ github.token }} PR: ${{ needs.resolve.outputs.pr_number }} + HEAD_SHA: ${{ needs.resolve.outputs.head_sha }} run: | mkdir -p /tmp/ai-review gh pr view "$PR" --repo "$GITHUB_REPOSITORY" \ --json number,title,body,baseRefName,headRefName,additions,deletions,changedFiles \ > /tmp/ai-review/pr.json base_ref=$(jq -r '.baseRefName' /tmp/ai-review/pr.json) - bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" + bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" "$HEAD_SHA" if [ -f .github/scripts/ai-review/generate-next-context.ts ]; then bun .github/scripts/ai-review/generate-next-context.ts "$base_ref" fi @@ -287,10 +290,11 @@ jobs: env: GH_TOKEN: ${{ github.token }} PR: ${{ needs.resolve.outputs.pr_number }} + HEAD_SHA: ${{ needs.resolve.outputs.head_sha }} run: | mkdir -p /tmp/ai-review base_ref=$(gh pr view "$PR" --repo "$GITHUB_REPOSITORY" --json baseRefName --jq '.baseRefName') - bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" + bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" "$HEAD_SHA" if [ -f .github/scripts/ai-review/generate-next-context.ts ]; then bun .github/scripts/ai-review/generate-next-context.ts "$base_ref" fi @@ -381,7 +385,7 @@ jobs: - name: Checkout PR head (untrusted; read-only, for verify-by-reading) uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ needs.resolve.outputs.head_ref }} + ref: ${{ needs.resolve.outputs.head_sha }} path: pr fetch-depth: 1 persist-credentials: false @@ -442,10 +446,11 @@ jobs: env: GH_TOKEN: ${{ github.token }} PR: ${{ needs.resolve.outputs.pr_number }} + HEAD_SHA: ${{ needs.resolve.outputs.head_sha }} run: | mkdir -p /tmp/ai-review base_ref=$(gh pr view "$PR" --repo "$GITHUB_REPOSITORY" --json baseRefName --jq '.baseRefName') - bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" + bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" "$HEAD_SHA" if [ -f .github/scripts/ai-review/generate-next-context.ts ]; then bun .github/scripts/ai-review/generate-next-context.ts "$base_ref" fi @@ -522,6 +527,8 @@ jobs: name: Post review needs: - resolve + - claude-review + - codex-review - adjudicate # Runs only when adjudication succeeded (it produced the merged review this job posts). # `!cancelled()` is required since an explicit `if` replaces the default "all needed jobs @@ -571,6 +578,8 @@ jobs: MERGED_REVIEW_PATH: /tmp/ai-review/merged-review.json PR_DIFF_PATH: /tmp/ai-review/pr.diff TRIGGER: ${{ needs.resolve.outputs.trigger }} + HEAD_SHA: ${{ needs.resolve.outputs.head_sha }} + REVIEWS_COMPLETE: ${{ needs.claude-review.result == 'success' && needs.codex-review.result == 'success' }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} # CLAUDE_MODEL / CODEX_MODEL are the job-level values from `resolve`, the same ones the # review jobs ran with, so the footer never drifts from what actually ran. diff --git a/.github/workflows/sync-repair.yml b/.github/workflows/sync-repair.yml index 44348d9516..b6080f0888 100644 --- a/.github/workflows/sync-repair.yml +++ b/.github/workflows/sync-repair.yml @@ -147,8 +147,8 @@ jobs: bun .github/scripts/sync-agent.ts suggest fi - # A green head the AI review saw carries only Claude's resolution and its declined findings. The approval runs - # fast-forward.yml, which re-checks the head and its checks before moving `next`. + # A green head both AI review models saw, with every finding fixed or declined and no reviewer requesting changes. + # The approval runs fast-forward.yml, which re-checks the head and its checks before moving `next`. approve: name: Approve the reviewed sync needs: repair @@ -172,10 +172,11 @@ jobs: exit 0 fi login="$(gh api user --jq .login)" - approved="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${PR}/reviews" \ - --jq ".[] | select(.user.login == \"$login\" and .state == \"APPROVED\" and .commit_id == \"$HEAD_SHA\") | .id")" - if [ -n "$approved" ]; then - echo "::notice::${login} already approved ${HEAD_SHA}." + # A dismissed approval on this head means a maintainer took it back, so it is never posted again. + previous="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${PR}/reviews" \ + --jq ".[] | select(.user.login == \"$login\" and .commit_id == \"$HEAD_SHA\" and (.state == \"APPROVED\" or .state == \"DISMISSED\")) | .state")" + if [ -n "$previous" ]; then + echo "::notice::${login} already approved ${HEAD_SHA} (${previous//$'\n'/, })." exit 0 fi gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/reviews" -f commit_id="$HEAD_SHA" -f event=APPROVE \ diff --git a/apps/cli/docs/release-process.md b/apps/cli/docs/release-process.md index d9257f12f0..2aaef8bc7e 100644 --- a/apps/cli/docs/release-process.md +++ b/apps/cli/docs/release-process.md @@ -356,7 +356,7 @@ git push origin sync/develop-into-next A `develop-into-next` conflict PR is resolved by Claude (the `claude.opus` model in `.github/ai-models.json`) in the same run. The `Resolve conflicts with Claude` job merges `next` into the sync branch and runs Claude Code with file read, search, and edit tools only: no shell, no web tools, no `.git` edits, and no GitHub write token. A `pnpm-lock.yaml` conflict keeps `next`'s lockfile, which the next job regenerates. `Run quick checks on the resolution` formats the merged tree and runs `check:all` and unit tests in a job with no secrets, and `Fix the quick checks with Claude` fixes what fails once, without re-running them. This catches code on one side that still uses something the other renamed or removed; integration and end-to-end failures are left to the PR's CI and the repair loop below. The jobs holding the Anthropic key never execute the merged tree, and the tree passes between jobs as a git bundle. The `Publish the resolution` job pushes the latest finished stage from a fresh checkout with the release App token, comments the resolution and any check that failed, and starts the AI review. Choices between `develop` and `next` behavior are listed as decisions for `@supabase/cli`. If Claude cannot resolve the merge, it comments why and the PR waits for a person as above. -[`sync-repair.yml`](../../../.github/workflows/sync-repair.yml) (`Sync repair`) runs when a CI workflow or the AI review finishes, and hourly. Once every check on the PR head has finished and no `AI Review` run for the PR is still going, it gives Claude the failed jobs' log tails and the AI review's unanswered threads. Claude fixes what the merge broke and replies on each finding, declining findings the merge did not introduce. The fix is pushed as a `chore(repo)` commit, fixed threads are resolved, and a round comment is posted. A head gets another round only for new review findings. When a round leaves its head failing, or after two rounds, the bot asks `@supabase/cli` to take over and stops repairing that PR. When every check except `Require fast-forward` is green on a head the AI review saw, and Claude fixed or declined every finding, `supabase-oss` approves that head, which fast-forwards `next`. A head moved by a repair commit or a maintainer push is not re-reviewed, so it waits for a maintainer's approval. The repair loop reads only CI output and the AI review's own threads, never other people's comments, and only acts on PRs Claude resolved. +[`sync-repair.yml`](../../../.github/workflows/sync-repair.yml) (`Sync repair`) runs when a CI workflow or the AI review finishes, and hourly. Once every check on the PR head has finished and no `AI Review` run for the PR is still going, it gives Claude the failed jobs' log tails and the AI review's unanswered threads. Claude fixes what the merge broke and replies on each finding, declining findings the merge did not introduce. The fix is pushed as a `chore(repo)` commit, fixed threads are resolved, and a round comment is posted. A head gets another round only for new review findings. When a round leaves its head failing, or after two rounds, the bot asks `@supabase/cli` to take over and stops repairing that PR. When every check except `Require fast-forward` is green on a head both AI review models finished reviewing, and every finding they did not refute has a thread Claude fixed or declined, `supabase-oss` approves that head, which fast-forwards `next`. A head moved by a repair commit or a maintainer push is not re-reviewed, and findings outside the diff have no thread, so both wait for a maintainer's approval. To hold a sync, mark the PR as a draft or request changes; dismissing `supabase-oss`'s approval also stops it approving that head again. The repair loop reads only CI output and the AI review's own threads, never other people's comments, and only acts on PRs Claude resolved. Workflows on the sync branch run with repository secrets, so Claude never pushes a change under `.github/`. Claude resolves and fixes workflow files like any other, but those edits are posted as a review of suggestions, with a diff for file additions and deletions; a conflicted workflow keeps `develop`'s side until then. A maintainer reviews them and uses **Commit suggestions**, so the workflow change runs only once a person commits it, and repair waits for that. The publish job refuses to push when the merge's `.github/` differs from git's own merge (with `develop`'s side on conflicts) or a later commit touches `.github/`, and the repair token has no `workflows` permission. Claude's other edits run in the sync PR's CI before anyone reviews them, and land only through a maintainer's approval like any sync PR. From 8c68094b22e12b4c5d4b0e9063cc620b16862a54 Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 13 Oct 2026 17:42:00 +0200 Subject: [PATCH 4/9] ci(release): re-check change requests before the sync bot's approval 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 --- .github/scripts/ai-review/post-review.test.ts | 27 ++++++--- .github/scripts/ai-review/post-review.ts | 26 +++++---- .github/scripts/fast-forward.test.ts | 26 +++++++++ .github/scripts/fast-forward.ts | 55 ++++++++++++++++++- .github/scripts/sync-agent.test.ts | 20 ------- .github/scripts/sync-agent.ts | 31 ++++------- .github/workflows/fast-forward.yml | 1 + apps/cli/docs/release-process.md | 4 +- 8 files changed, 128 insertions(+), 62 deletions(-) diff --git a/.github/scripts/ai-review/post-review.test.ts b/.github/scripts/ai-review/post-review.test.ts index f17a5623bb..1fc5d2b90c 100644 --- a/.github/scripts/ai-review/post-review.test.ts +++ b/.github/scripts/ai-review/post-review.test.ts @@ -706,7 +706,7 @@ describe("buildReviewPayload", () => { expect( parseAiReviewState(truncateReviewBody(`${payload.body}${"x".repeat(70_000)}`, footer.runUrl)), ).toEqual({ - head: footer.headSha, + head: "a".repeat(40), complete: false, findings: 2, }); @@ -1240,17 +1240,28 @@ describe("post flow via injected ReviewIo", () => { expect(postedReviews[1]?.body).toContain("Inline comments (GitHub rejected"); }); - test("never retries with the fold when there were no inline comments to fold", async () => { + test("retries a rejected pinned review on the current head, still naming the reviewed commit", async () => { const finding = makeFinding({ file: "file.ts", line: 999 }); // not anchorable -> body-only const review = makeMergedReview({ findings: [finding] }); - const { io, postedReviews } = makeReviewIo({ - postReviewStatuses: [422], - }); + const { io, postedReviews } = makeReviewIo({ postReviewStatuses: [422, 200] }); - await expect(postConsolidatedReview(io, 42, review, SINGLE_HUNK_DIFF, footer)).rejects.toThrow( - /Review POST failed \(status 422\)/, - ); + await postConsolidatedReview(io, 42, review, SINGLE_HUNK_DIFF, footer); + + expect(postedReviews.map((posted) => posted.commit_id)).toEqual([footer.headSha, undefined]); + expect(parseAiReviewState(postedReviews[1]?.body ?? "")?.head).toBe(footer.headSha); + }); + + test("an older workflow's review carries no state, so it never retries a body-only 422", async () => { + const finding = makeFinding({ file: "file.ts", line: 999 }); + const review = makeMergedReview({ findings: [finding] }); + const { io, postedReviews } = makeReviewIo({ postReviewStatuses: [422] }); + const unpinned = { ...footer, headSha: undefined, reviewsComplete: undefined }; + + await expect( + postConsolidatedReview(io, 42, review, SINGLE_HUNK_DIFF, unpinned), + ).rejects.toThrow(/Review POST failed \(status 422\)/); expect(postedReviews).toHaveLength(1); + expect(parseAiReviewState(postedReviews[0]?.body ?? "")).toBeNull(); }); test("throws with GitHub's response body when the retry also 422s, instead of swallowing it", async () => { diff --git a/.github/scripts/ai-review/post-review.ts b/.github/scripts/ai-review/post-review.ts index e7ff9252ac..5f7a47b0eb 100644 --- a/.github/scripts/ai-review/post-review.ts +++ b/.github/scripts/ai-review/post-review.ts @@ -629,9 +629,9 @@ export interface ReviewFooterInfo { /** e.g. `` `claude-fable-5` + `gpt-5.6-sol` ``. Sourced from * `.github/ai-models.json` through the workflow's model env vars. */ modelsFooter: string; - /** Commit the models reviewed; the review is posted on it. */ - headSha: string; - reviewsComplete: boolean; + /** Commit the models reviewed; absent from older workflow copies, whose reviews carry no state and never approve. */ + headSha?: string; + reviewsComplete?: boolean; } /** Renders the full review body: summary, findings table, out-of-diff section, refuted details, stats, and footer. */ @@ -646,8 +646,11 @@ export function renderReviewBody( const verdicts = computeVerdictCounts(review.findings); // The state line leads the body so truncation never drops it. + const state = footer.headSha + ? `${renderAiReviewState({ head: footer.headSha, complete: footer.reviewsComplete === true, findings: posted.length })}\n` + : ""; const sections: string[] = [ - `${renderAiReviewState({ head: footer.headSha, complete: footer.reviewsComplete, findings: posted.length })}\n## 🤖 AI Review\n\n${sanitizeModelText(review.summary)}`, + `${state}## 🤖 AI Review\n\n${sanitizeModelText(review.summary)}`, ]; if (posted.length > 0) { @@ -744,7 +747,7 @@ export interface InlineReviewComment { } export interface ReviewPayload { - commit_id: string; + commit_id?: string; event: "COMMENT"; body: string; comments: InlineReviewComment[]; @@ -794,7 +797,7 @@ export function buildReviewPayload( // A body-only review has no 422 fold-retry to truncate it later, so // truncate the first payload too rather than posting an oversized body. return { - commit_id: footer.headSha, + ...(footer.headSha ? { commit_id: footer.headSha } : {}), event: "COMMENT", body: truncateReviewBody(body, footer.runUrl), comments, @@ -938,11 +941,12 @@ export async function postConsolidatedReview( const prior = await listPriorRunsBestEffort(io, prNumber); const result = await io.postReview(prNumber, payload); - if (result.status === 422 && payload.comments.length > 0) { + if (result.status === 422 && (payload.comments.length > 0 || payload.commit_id)) { console.warn( - "Review POST rejected an inline anchor (422); retrying once with comments folded into the body.", + "Review POST rejected (422); retrying once on the current head with comments folded into the body.", ); - const folded = foldInlineCommentsIntoBody(payload); + // A force-push can drop the reviewed commit; the state line still names it, so the review cannot vouch for the new head. + const { commit_id: _rejected, ...folded } = foldInlineCommentsIntoBody(payload); const retryResult = await io.postReview(prNumber, { ...folded, body: truncateReviewBody(folded.body, footer.runUrl), @@ -1173,8 +1177,8 @@ async function runPost(): Promise { const prDiffPath = requireEnv("PR_DIFF_PATH"); const claudeModel = requireEnv("CLAUDE_MODEL"); const codexModel = requireEnv("CODEX_MODEL"); - const headSha = requireEnv("HEAD_SHA"); - const reviewsComplete = requireEnv("REVIEWS_COMPLETE") === "true"; + const headSha = process.env.HEAD_SHA || undefined; + const reviewsComplete = process.env.REVIEWS_COMPLETE === "true"; const raw: unknown = JSON.parse(await Bun.file(mergedReviewPath).text()); assertMergedReview(raw); diff --git a/.github/scripts/fast-forward.test.ts b/.github/scripts/fast-forward.test.ts index 008058b89e..50efe99836 100644 --- a/.github/scripts/fast-forward.test.ts +++ b/.github/scripts/fast-forward.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, test } from "bun:test"; import { type CheckRun, + AUTO_APPROVER_LOGIN, type FastForwardIo, type GraphqlQuery, type Promotion, @@ -427,6 +428,7 @@ function scenarioIo( const io: FastForwardIo = { git: makeGit(checkout), getPullRequest: async () => pullRequest, + listReviews: async () => [], listCheckRuns: async () => checks, findOpenPullRequest: async (base, head) => { lookups.push(`${base}<-${head}`); @@ -733,6 +735,30 @@ describe("runFastForward sync", () => { expect(() => repo.remoteTip("sync/main-into-develop")).toThrow(); }); + test("refuses the sync bot's approval while a collaborator requests changes, not an outsider", async () => { + const { repo, head, io, comments } = resolvedSyncScenario(); + const before = repo.remoteTip("develop"); + const review = (author_association: string) => ({ + user: { login: author_association.toLowerCase() }, + state: "CHANGES_REQUESTED", + author_association, + }); + + const held = await runFastForward( + { ...io, listReviews: async () => [review("COLLABORATOR")] }, + { reviewCommitId: head, approver: AUTO_APPROVER_LOGIN }, + ); + expect(held.status).toBe("refused"); + expect(comments[0]).toContain("requested changes"); + expect(repo.remoteTip("develop")).toBe(before); + + const landed = await runFastForward( + { ...io, listReviews: async () => [review("NONE")] }, + { reviewCommitId: head, approver: AUTO_APPROVER_LOGIN }, + ); + expect(landed.status).toBe("fast-forwarded"); + }); + test("lands the approved head merged with a target move without pulling in the moved source", async () => { const { repo, head, io, comments } = resolvedSyncScenario(); git(repo.seed, "switch", "main"); diff --git a/.github/scripts/fast-forward.ts b/.github/scripts/fast-forward.ts index 77b9ff82cd..e989115c40 100644 --- a/.github/scripts/fast-forward.ts +++ b/.github/scripts/fast-forward.ts @@ -199,9 +199,37 @@ export type CheckEvaluation = failing: { name: string; conclusion: string }[]; }; +/** Account `Sync repair` approves eligible sync pull requests as. */ +export const AUTO_APPROVER_LOGIN = "supabase-oss"; + +export interface PullRequestReview { + user: { login: string } | null; + state: string; + author_association: string; +} + +/** + * Whether a collaborator's latest approving or blocking review requests changes; comments do not change a verdict, + * and reviews from people outside the repository never hold automation. + */ +export function hasChangesRequested(reviews: PullRequestReview[]): boolean { + const verdicts = new Map(); + for (const review of reviews) { + if ( + review.user && + ["OWNER", "MEMBER", "COLLABORATOR"].includes(review.author_association) && + ["APPROVED", "CHANGES_REQUESTED", "DISMISSED"].includes(review.state) + ) { + verdicts.set(review.user.login, review.state); + } + } + return [...verdicts.values()].includes("CHANGES_REQUESTED"); +} + export interface FastForwardIo { git: GitRunner; getPullRequest(): Promise; + listReviews(): Promise; listCheckRuns(sha: string): Promise; findOpenPullRequest(base: string, head: string): Promise; comment(body: string): Promise; @@ -210,6 +238,8 @@ export interface FastForwardIo { export interface FastForwardInput { /** The commit the approving review was submitted against; a local dry run defaults to the live head. */ reviewCommitId?: string; + /** Login of the approving reviewer. */ + approver?: string; dryRun?: boolean; } @@ -448,6 +478,15 @@ export async function runFastForward( if (pullRequest.draft) { return refuse(io, input, "pull request is a draft"); } + // Sync repair decides to approve before posting, so a change request can land in between. + if (input.approver === AUTO_APPROVER_LOGIN && hasChangesRequested(await io.listReviews())) { + return refuse( + io, + input, + "a reviewer requested changes", + `Not fast-forwarding \`${promotion.target}\`: a reviewer requested changes after \`${AUTO_APPROVER_LOGIN}\` decided to approve. A maintainer approves once they are addressed.`, + ); + } if (approvedSha !== headSha) { return refuse( io, @@ -567,6 +606,19 @@ function makeIo(token: string, repository: string, prNumber: number): FastForwar git: makeGit(process.cwd()), getPullRequest: () => githubRequest(token, `/repos/${repository}/pulls/${prNumber}`), + async listReviews() { + const reviews: PullRequestReview[] = []; + for (let page = 1; ; page += 1) { + const batch = await githubRequest( + token, + `/repos/${repository}/pulls/${prNumber}/reviews?per_page=100&page=${page}`, + ); + reviews.push(...batch); + if (batch.length < 100) { + return reviews; + } + } + }, listCheckRuns: (sha) => fetchPullRequestCheckRuns( (query, variables) => githubGraphql(token, query, variables), @@ -608,7 +660,8 @@ async function main(): Promise { if (!reviewCommitId && !dryRun) { throw new Error("REVIEW_COMMIT_ID is required"); } - const outcome = await runFastForward(io, { reviewCommitId, dryRun }); + const approver = process.env.REVIEWER_LOGIN || undefined; + const outcome = await runFastForward(io, { reviewCommitId, approver, dryRun }); console.log(JSON.stringify(outcome)); if (outcome.status === "refused") { console.error(`::error::${outcome.reason}`); diff --git a/.github/scripts/sync-agent.test.ts b/.github/scripts/sync-agent.test.ts index dc40ac8e30..35fd2fa571 100644 --- a/.github/scripts/sync-agent.test.ts +++ b/.github/scripts/sync-agent.test.ts @@ -8,7 +8,6 @@ import { createTestRepo, git, type TestRepo } from "./promotion-test-repo.ts"; import { buildSuggestions, failureExcerpt, - hasChangesRequested, leftoverConflictMarkers, planRepair, type ReviewThread, @@ -225,25 +224,6 @@ describe("planRepair", () => { }); }); -describe("hasChangesRequested", () => { - const review = (login: string, state: string) => ({ user: { login }, state }); - - test("holds while a reviewer's latest verdict requests changes, ignoring later comments", () => { - expect( - hasChangesRequested([ - review("maintainer", "CHANGES_REQUESTED"), - review("maintainer", "COMMENTED"), - ]), - ).toBe(true); - expect( - hasChangesRequested([ - review("maintainer", "CHANGES_REQUESTED"), - review("maintainer", "APPROVED"), - ]), - ).toBe(false); - }); -}); - describe("buildSuggestions", () => { const diff = [ "diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml", diff --git a/.github/scripts/sync-agent.ts b/.github/scripts/sync-agent.ts index 801053dd77..9215f7f44f 100644 --- a/.github/scripts/sync-agent.ts +++ b/.github/scripts/sync-agent.ts @@ -11,7 +11,13 @@ import { type AiReviewState, parseAiReviewState, } from "./ai-review/post-review.ts"; -import { type CheckRun, fetchPullRequestCheckRuns, REQUIRED_CHECKS } from "./fast-forward.ts"; +import { + type CheckRun, + fetchPullRequestCheckRuns, + hasChangesRequested, + type PullRequestReview, + REQUIRED_CHECKS, +} from "./fast-forward.ts"; import { githubGraphql, githubRequest, @@ -626,19 +632,6 @@ async function fetchAiReviewThreads( }); } -/** Whether any reviewer's latest approving or blocking review requests changes; comments do not change a verdict. */ -export function hasChangesRequested( - reviews: { user: { login: string } | null; state: string }[], -): boolean { - const verdicts = new Map(); - for (const review of reviews) { - if (review.user && ["APPROVED", "CHANGES_REQUESTED", "DISMISSED"].includes(review.state)) { - verdicts.set(review.user.login, review.state); - } - } - return [...verdicts.values()].includes("CHANGES_REQUESTED"); -} - async function jobFailureExcerpt( token: string, repository: string, @@ -747,12 +740,10 @@ async function collect(outputDir: string): Promise { `/repos/${repository}/issues/${pull.number}/comments`, )), ]; - const reviews = await listAll<{ - id: number; - user: { login: string } | null; - body: string; - state: string; - }>(token, `/repos/${repository}/pulls/${pull.number}/reviews`); + const reviews = await listAll( + token, + `/repos/${repository}/pulls/${pull.number}/reviews`, + ); comments.push(...reviews); const aiReview = reviews.findLast( (review) => diff --git a/.github/workflows/fast-forward.yml b/.github/workflows/fast-forward.yml index 12d108bbae..91df7c1250 100644 --- a/.github/workflows/fast-forward.yml +++ b/.github/workflows/fast-forward.yml @@ -88,6 +88,7 @@ jobs: REPOSITORY: ${{ github.repository }} PR_NUMBER: ${{ github.event.pull_request.number }} REVIEW_COMMIT_ID: ${{ github.event.review.commit_id }} + REVIEWER_LOGIN: ${{ github.event.review.user.login }} run: | git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" diff --git a/apps/cli/docs/release-process.md b/apps/cli/docs/release-process.md index d770cde9fd..5bf414b238 100644 --- a/apps/cli/docs/release-process.md +++ b/apps/cli/docs/release-process.md @@ -356,9 +356,9 @@ git push origin sync/develop-into-next A `develop-into-next` conflict PR is resolved by Claude (the `claude.opus` model in `.github/ai-models.json`) in the same run. The `Resolve conflicts with Claude` job merges `next` into the sync branch and runs Claude Code with file read, search, and edit tools only: no shell, no web tools, no `.git` edits, and no GitHub write token. A `pnpm-lock.yaml` conflict keeps `next`'s lockfile, which the next job regenerates. `Run quick checks on the resolution` formats the merged tree and runs `check:all` and unit tests in a job with no secrets, and `Fix the quick checks with Claude` fixes what fails once, without re-running them. This catches code on one side that still uses something the other renamed or removed; integration and end-to-end failures are left to the PR's CI and the repair loop below. The jobs holding the Anthropic key never execute the merged tree, and the tree passes between jobs as a git bundle. The `Publish the resolution` job pushes the latest finished stage from a fresh checkout with the release App token, comments the resolution and any check that failed, and starts the AI review. Choices between `develop` and `next` behavior are listed as decisions for `@supabase/cli`. If Claude cannot resolve the merge, it comments why and the PR waits for a person as above. -[`sync-repair.yml`](../../../.github/workflows/sync-repair.yml) (`Sync repair`) runs when a CI workflow or the AI review finishes, and hourly. Once every check on the PR head has finished and no `AI Review` run for the PR is still going, it gives Claude the failed jobs' log tails and the AI review's unanswered threads. Claude fixes what the merge broke and replies on each finding, declining findings the merge did not introduce. The fix is pushed as a `chore(repo)` commit, fixed threads are resolved, and a round comment is posted. A head gets another round only for new review findings. When a round leaves its head failing, or after two rounds, the bot asks `@supabase/cli` to take over and stops repairing that PR. When every check except `Require fast-forward` is green on a head both AI review models finished reviewing, and every finding they did not refute has a thread Claude fixed or declined, `supabase-oss` approves that head, which fast-forwards `next`. A head moved by a repair commit or a maintainer push is not re-reviewed, and findings outside the diff have no thread, so both wait for a maintainer's approval. To hold a sync, mark the PR as a draft or request changes; dismissing `supabase-oss`'s approval also stops it approving that head again. The repair loop reads only CI output and the AI review's own threads, never other people's comments, and only acts on PRs Claude resolved. +[`sync-repair.yml`](../../../.github/workflows/sync-repair.yml) (`Sync repair`) runs when a CI workflow or the AI review finishes, and hourly. Once every check on the PR head has finished and no `AI Review` run for the PR is still going, it gives Claude the failed jobs' log tails and the AI review's unanswered threads. Claude fixes what the merge broke and replies on each finding, declining findings the merge did not introduce. The fix is pushed as a `chore(repo)` commit, fixed threads are resolved, and a round comment is posted. A head gets another round only for new review findings. When a round leaves its head failing, or after two rounds, the bot asks `@supabase/cli` to take over and stops repairing that PR. When every check except `Require fast-forward` is green on a head both AI review models finished reviewing, and every finding they did not refute has a thread Claude fixed or declined, `supabase-oss` approves that head, which fast-forwards `next`. A head moved by a repair commit or a maintainer push is not re-reviewed, and findings outside the diff have no thread, so both wait for a maintainer's approval. To hold a sync, mark the PR as a draft or request changes as a repository collaborator; dismissing `supabase-oss`'s approval also stops it approving that head again. `Fast-forward on approval` re-checks change requests before moving `next` on `supabase-oss`'s approval. When it refuses, `supabase-oss` does not approve that head again, so a maintainer approves. The repair loop reads only CI output and the AI review's own threads, never other people's comments, and only acts on PRs Claude resolved. -Workflows on the sync branch run with repository secrets, so Claude never pushes a change under `.github/`. Claude resolves and fixes workflow files like any other, but those edits are posted as a review of suggestions, with a diff for file additions and deletions; a conflicted workflow keeps `develop`'s side until then. A maintainer reviews them and uses **Commit suggestions**, so the workflow change runs only once a person commits it, and repair waits for that. The publish job refuses to push when the merge's `.github/` differs from git's own merge (with `develop`'s side on conflicts) or a later commit touches `.github/`, and the repair token has no `workflows` permission. Claude's other edits run in the sync PR's CI before anyone reviews them, and land only through a maintainer's approval like any sync PR. +Workflows on the sync branch run with repository secrets, so Claude never pushes a change under `.github/`. Claude resolves and fixes workflow files like any other, but those edits are posted as a review of suggestions, with a diff for file additions and deletions; a conflicted workflow keeps `develop`'s side until then. A maintainer reviews them and uses **Commit suggestions**, so the workflow change runs only once a person commits it, and repair waits for that. The publish job refuses to push when the merge's `.github/` differs from git's own merge (with `develop`'s side on conflicts) or a later commit touches `.github/`, and the repair token has no `workflows` permission. Claude's other edits run in the sync PR's CI before anyone reviews them, and land through a maintainer's approval or `Sync repair`'s approval under the conditions above. The PR is authored by the release App, so the person who resolved it can approve it. This is an accepted decision: sync resolvers are trusted maintainers with write access, and the resolution content is not independently reviewed. `bot-pr-auto-merge.yml` excludes both sync heads so the bot never approves or auto-merges them; only `Sync repair` approves, on the conditions above. This is an accepted decision: Claude's resolution and its own judgement of which findings the merge introduced reach `next` without a person reviewing them. **Approving the PR fast-forwards the target to the PR head** and deletes the sync branch; do not use the merge button (the repo is squash-only and squashing destroys the merge ancestry the sync relies on). The `Require fast-forward` check fails on these PRs to enforce that. From ffe6f9d4ff6b7fb918ff6e526fbe4196426d50eb Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 13 Oct 2026 17:47:58 +0200 Subject: [PATCH 5/9] chore(ci): format post-review state line Co-Authored-By: Claude Opus 5.5 --- .github/scripts/ai-review/post-review.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/scripts/ai-review/post-review.ts b/.github/scripts/ai-review/post-review.ts index 5f7a47b0eb..cf10788f2e 100644 --- a/.github/scripts/ai-review/post-review.ts +++ b/.github/scripts/ai-review/post-review.ts @@ -649,9 +649,7 @@ export function renderReviewBody( const state = footer.headSha ? `${renderAiReviewState({ head: footer.headSha, complete: footer.reviewsComplete === true, findings: posted.length })}\n` : ""; - const sections: string[] = [ - `${state}## 🤖 AI Review\n\n${sanitizeModelText(review.summary)}`, - ]; + const sections: string[] = [`${state}## 🤖 AI Review\n\n${sanitizeModelText(review.summary)}`]; if (posted.length > 0) { const rows = posted.map( From ca1a1b5f8f00033b91fa88ddb3aea4f2bdc3ce3b Mon Sep 17 00:00:00 2001 From: avallete Date: Wed, 14 Oct 2026 15:41:09 +0200 Subject: [PATCH 6/9] ci(release): refuse a sync fast-forward once supabase-oss's approval is dismissed Co-Authored-By: Claude Opus 5.5 --- .github/scripts/fast-forward.test.ts | 45 ++++++++++++++++------------ .github/scripts/fast-forward.ts | 40 ++++++++++++++++--------- 2 files changed, 52 insertions(+), 33 deletions(-) diff --git a/.github/scripts/fast-forward.test.ts b/.github/scripts/fast-forward.test.ts index 50efe99836..2a26152fd6 100644 --- a/.github/scripts/fast-forward.test.ts +++ b/.github/scripts/fast-forward.test.ts @@ -735,28 +735,35 @@ describe("runFastForward sync", () => { expect(() => repo.remoteTip("sync/main-into-develop")).toThrow(); }); - test("refuses the sync bot's approval while a collaborator requests changes, not an outsider", async () => { - const { repo, head, io, comments } = resolvedSyncScenario(); - const before = repo.remoteTip("develop"); - const review = (author_association: string) => ({ - user: { login: author_association.toLowerCase() }, - state: "CHANGES_REQUESTED", + test("lands the sync bot's approval only while it is active and no collaborator requests changes", async () => { + const review = (login: string, state: string, author_association = "COLLABORATOR") => ({ + user: { login }, + state, author_association, }); + const approved = review(AUTO_APPROVER_LOGIN, "APPROVED"); + const attempt = async (reviews: ReturnType[]) => { + const { repo, head, io } = resolvedSyncScenario(); + const before = repo.remoteTip("develop"); + const outcome = await runFastForward( + { ...io, listReviews: async () => reviews }, + { reviewCommitId: head, approver: AUTO_APPROVER_LOGIN }, + ); + return { status: outcome.status, moved: repo.remoteTip("develop") !== before }; + }; - const held = await runFastForward( - { ...io, listReviews: async () => [review("COLLABORATOR")] }, - { reviewCommitId: head, approver: AUTO_APPROVER_LOGIN }, - ); - expect(held.status).toBe("refused"); - expect(comments[0]).toContain("requested changes"); - expect(repo.remoteTip("develop")).toBe(before); - - const landed = await runFastForward( - { ...io, listReviews: async () => [review("NONE")] }, - { reviewCommitId: head, approver: AUTO_APPROVER_LOGIN }, - ); - expect(landed.status).toBe("fast-forwarded"); + expect(await attempt([approved, review("maintainer", "CHANGES_REQUESTED")])).toEqual({ + status: "refused", + moved: false, + }); + expect(await attempt([approved, review(AUTO_APPROVER_LOGIN, "DISMISSED")])).toEqual({ + status: "refused", + moved: false, + }); + expect(await attempt([approved, review("outsider", "CHANGES_REQUESTED", "NONE")])).toEqual({ + status: "fast-forwarded", + moved: true, + }); }); test("lands the approved head merged with a target move without pulling in the moved source", async () => { diff --git a/.github/scripts/fast-forward.ts b/.github/scripts/fast-forward.ts index e989115c40..e47eecb73e 100644 --- a/.github/scripts/fast-forward.ts +++ b/.github/scripts/fast-forward.ts @@ -213,17 +213,23 @@ export interface PullRequestReview { * and reviews from people outside the repository never hold automation. */ export function hasChangesRequested(reviews: PullRequestReview[]): boolean { + return [ + ...latestVerdicts( + reviews.filter((review) => + ["OWNER", "MEMBER", "COLLABORATOR"].includes(review.author_association), + ), + ).values(), + ].includes("CHANGES_REQUESTED"); +} + +function latestVerdicts(reviews: PullRequestReview[]): Map { const verdicts = new Map(); for (const review of reviews) { - if ( - review.user && - ["OWNER", "MEMBER", "COLLABORATOR"].includes(review.author_association) && - ["APPROVED", "CHANGES_REQUESTED", "DISMISSED"].includes(review.state) - ) { + if (review.user && ["APPROVED", "CHANGES_REQUESTED", "DISMISSED"].includes(review.state)) { verdicts.set(review.user.login, review.state); } } - return [...verdicts.values()].includes("CHANGES_REQUESTED"); + return verdicts; } export interface FastForwardIo { @@ -478,14 +484,20 @@ export async function runFastForward( if (pullRequest.draft) { return refuse(io, input, "pull request is a draft"); } - // Sync repair decides to approve before posting, so a change request can land in between. - if (input.approver === AUTO_APPROVER_LOGIN && hasChangesRequested(await io.listReviews())) { - return refuse( - io, - input, - "a reviewer requested changes", - `Not fast-forwarding \`${promotion.target}\`: a reviewer requested changes after \`${AUTO_APPROVER_LOGIN}\` decided to approve. A maintainer approves once they are addressed.`, - ); + // A change request or a dismissal can land between Sync repair's decision and this run. + if (input.approver === AUTO_APPROVER_LOGIN) { + const reviews = await io.listReviews(); + if ( + hasChangesRequested(reviews) || + latestVerdicts(reviews).get(AUTO_APPROVER_LOGIN) !== "APPROVED" + ) { + return refuse( + io, + input, + "a reviewer requested changes or the approval was dismissed", + `Not fast-forwarding \`${promotion.target}\`: a reviewer requested changes or dismissed \`${AUTO_APPROVER_LOGIN}\`'s approval. A maintainer approves once that is addressed.`, + ); + } } if (approvedSha !== headSha) { return refuse( From e3a2f735b65a5a028ac3a48898008a6cb333530b Mon Sep 17 00:00:00 2001 From: avallete Date: Wed, 14 Oct 2026 15:53:46 +0200 Subject: [PATCH 7/9] ci(release): close the remaining gaps in the sync auto-approval 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 --- .github/scripts/ai-review/post-review.test.ts | 6 ++-- .github/scripts/ai-review/post-review.ts | 7 ++--- .github/scripts/fast-forward.test.ts | 19 ++++++------ .github/scripts/fast-forward.ts | 30 ++++++++++++------- .github/scripts/sync-agent.ts | 5 +++- .github/workflows/fast-forward.yml | 1 - .github/workflows/sync-repair.yml | 12 ++++---- apps/cli/docs/release-process.md | 2 +- 8 files changed, 45 insertions(+), 37 deletions(-) diff --git a/.github/scripts/ai-review/post-review.test.ts b/.github/scripts/ai-review/post-review.test.ts index 1fc5d2b90c..29959610ad 100644 --- a/.github/scripts/ai-review/post-review.test.ts +++ b/.github/scripts/ai-review/post-review.test.ts @@ -703,9 +703,9 @@ describe("buildReviewPayload", () => { expect(payload.commit_id).toBe(footer.headSha); expect(payload.comments).toHaveLength(1); - expect( - parseAiReviewState(truncateReviewBody(`${payload.body}${"x".repeat(70_000)}`, footer.runUrl)), - ).toEqual({ + const truncated = truncateReviewBody(`${payload.body}${"x".repeat(70_000)}`, footer.runUrl); + expect(truncated).toContain(AI_REVIEW_MARKER); + expect(parseAiReviewState(truncated)).toEqual({ head: "a".repeat(40), complete: false, findings: 2, diff --git a/.github/scripts/ai-review/post-review.ts b/.github/scripts/ai-review/post-review.ts index cf10788f2e..5eb5eab9e0 100644 --- a/.github/scripts/ai-review/post-review.ts +++ b/.github/scripts/ai-review/post-review.ts @@ -645,11 +645,12 @@ export function renderReviewBody( ); const verdicts = computeVerdictCounts(review.findings); - // The state line leads the body so truncation never drops it. + // The markers lead the body so truncation never drops them. const state = footer.headSha ? `${renderAiReviewState({ head: footer.headSha, complete: footer.reviewsComplete === true, findings: posted.length })}\n` : ""; - const sections: string[] = [`${state}## 🤖 AI Review\n\n${sanitizeModelText(review.summary)}`]; + const header = `${AI_REVIEW_MARKER}\n${state}`; + const sections: string[] = [`${header}## 🤖 AI Review\n\n${sanitizeModelText(review.summary)}`]; if (posted.length > 0) { const rows = posted.map( @@ -727,8 +728,6 @@ export function renderReviewBody( `Models: ${footer.modelsFooter} · Trigger: \`${footer.trigger}\` · [Workflow run](${footer.runUrl})`, "", "This review runs once per PR. A maintainer can request another with a `/ai-review` comment.", - "", - AI_REVIEW_MARKER, ].join("\n"), ); diff --git a/.github/scripts/fast-forward.test.ts b/.github/scripts/fast-forward.test.ts index 2a26152fd6..c2d152cedc 100644 --- a/.github/scripts/fast-forward.test.ts +++ b/.github/scripts/fast-forward.test.ts @@ -735,12 +735,8 @@ describe("runFastForward sync", () => { expect(() => repo.remoteTip("sync/main-into-develop")).toThrow(); }); - test("lands the sync bot's approval only while it is active and no collaborator requests changes", async () => { - const review = (login: string, state: string, author_association = "COLLABORATOR") => ({ - user: { login }, - state, - author_association, - }); + test("lands the sync bot's approval only while it is active and nobody requests changes", async () => { + const review = (login: string, state: string) => ({ user: { login }, state }); const approved = review(AUTO_APPROVER_LOGIN, "APPROVED"); const attempt = async (reviews: ReturnType[]) => { const { repo, head, io } = resolvedSyncScenario(); @@ -760,10 +756,13 @@ describe("runFastForward sync", () => { status: "refused", moved: false, }); - expect(await attempt([approved, review("outsider", "CHANGES_REQUESTED", "NONE")])).toEqual({ - status: "fast-forwarded", - moved: true, - }); + expect( + await attempt([ + review("maintainer", "CHANGES_REQUESTED"), + review("maintainer", "APPROVED"), + approved, + ]), + ).toEqual({ status: "fast-forwarded", moved: true }); }); test("lands the approved head merged with a target move without pulling in the moved source", async () => { diff --git a/.github/scripts/fast-forward.ts b/.github/scripts/fast-forward.ts index e47eecb73e..5e1bcf8f6c 100644 --- a/.github/scripts/fast-forward.ts +++ b/.github/scripts/fast-forward.ts @@ -1,3 +1,5 @@ +import { readFileSync } from "node:fs"; + import { analyzeCommits } from "../../apps/cli/scripts/analyze-commits-title.js"; import { type GitRunner, @@ -205,21 +207,14 @@ export const AUTO_APPROVER_LOGIN = "supabase-oss"; export interface PullRequestReview { user: { login: string } | null; state: string; - author_association: string; } /** - * Whether a collaborator's latest approving or blocking review requests changes; comments do not change a verdict, - * and reviews from people outside the repository never hold automation. + * Whether anyone's latest approving or blocking review requests changes; comments do not change a verdict. Every + * reviewer counts because `author_association` hides private org membership; a maintainer dismisses a stray one. */ export function hasChangesRequested(reviews: PullRequestReview[]): boolean { - return [ - ...latestVerdicts( - reviews.filter((review) => - ["OWNER", "MEMBER", "COLLABORATOR"].includes(review.author_association), - ), - ).values(), - ].includes("CHANGES_REQUESTED"); + return [...latestVerdicts(reviews).values()].includes("CHANGES_REQUESTED"); } function latestVerdicts(reviews: PullRequestReview[]): Map { @@ -664,6 +659,15 @@ function parseArguments(args: string[]): { prNumber: number; dryRun: boolean } { return { prNumber, dryRun }; } +function reviewerLogin(event: unknown): string | undefined { + const isRecord = (value: unknown): value is Record => + typeof value === "object" && value !== null; + const review = isRecord(event) ? event.review : undefined; + const user = isRecord(review) ? review.user : undefined; + const login = isRecord(user) ? user.login : undefined; + return typeof login === "string" ? login : undefined; +} + async function main(): Promise { const { prNumber, dryRun } = parseArguments(process.argv.slice(2)); const repository = process.env.REPOSITORY ?? requireEnv("GITHUB_REPOSITORY"); @@ -672,7 +676,11 @@ async function main(): Promise { if (!reviewCommitId && !dryRun) { throw new Error("REVIEW_COMMIT_ID is required"); } - const approver = process.env.REVIEWER_LOGIN || undefined; + // Read from the event so a sync PR running an older copy of the workflow still identifies the approver. + const eventPath = process.env.GITHUB_EVENT_PATH; + const approver = eventPath + ? reviewerLogin(JSON.parse(readFileSync(eventPath, "utf8"))) + : undefined; const outcome = await runFastForward(io, { reviewCommitId, approver, dryRun }); console.log(JSON.stringify(outcome)); if (outcome.status === "refused") { diff --git a/.github/scripts/sync-agent.ts b/.github/scripts/sync-agent.ts index 9215f7f44f..3d8f7c2ee8 100644 --- a/.github/scripts/sync-agent.ts +++ b/.github/scripts/sync-agent.ts @@ -9,6 +9,7 @@ import { join } from "node:path"; import { AI_REVIEW_MARKER, type AiReviewState, + isSuperseded, parseAiReviewState, } from "./ai-review/post-review.ts"; import { @@ -747,7 +748,9 @@ async function collect(outputDir: string): Promise { comments.push(...reviews); const aiReview = reviews.findLast( (review) => - isLogin(review.user?.login, AI_REVIEW_LOGIN) && review.body.includes(AI_REVIEW_MARKER), + isLogin(review.user?.login, AI_REVIEW_LOGIN) && + review.body.includes(AI_REVIEW_MARKER) && + !isSuperseded(review.body), ); const aiReviewState = aiReview ? parseAiReviewState(aiReview.body) : null; const aiThreads = await fetchAiReviewThreads(token, repository, pull.number); diff --git a/.github/workflows/fast-forward.yml b/.github/workflows/fast-forward.yml index 91df7c1250..12d108bbae 100644 --- a/.github/workflows/fast-forward.yml +++ b/.github/workflows/fast-forward.yml @@ -88,7 +88,6 @@ jobs: REPOSITORY: ${{ github.repository }} PR_NUMBER: ${{ github.event.pull_request.number }} REVIEW_COMMIT_ID: ${{ github.event.review.commit_id }} - REVIEWER_LOGIN: ${{ github.event.review.user.login }} run: | git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" diff --git a/.github/workflows/sync-repair.yml b/.github/workflows/sync-repair.yml index b6080f0888..80e8ed7e6d 100644 --- a/.github/workflows/sync-repair.yml +++ b/.github/workflows/sync-repair.yml @@ -130,6 +130,11 @@ jobs: --force-with-lease="refs/heads/sync/develop-into-next:$HEAD_SHA" \ "https://x-access-token:${GH_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" HEAD:refs/heads/sync/develop-into-next fi + # The workflow suggestions hold the head before any reply counts a withheld fix as answered. + if [ "$CHANGED" != "" ]; then + (cd trusted && REPO_DIR="$GITHUB_WORKSPACE/work" HEAD_SHA="$(git -C "$GITHUB_WORKSPACE/work" rev-parse HEAD)" \ + bun .github/scripts/sync-agent.ts suggest) + fi if [ -f "$OUTPUT_DIR/replies.json" ]; then jq -c '.[]' "$OUTPUT_DIR/replies.json" | while read -r reply; do gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/comments/$(jq -r .commentId <<<"$reply")/replies" \ @@ -141,13 +146,8 @@ jobs: fi done fi - if [ "$CHANGED" != "" ]; then - cd trusted - REPO_DIR="$GITHUB_WORKSPACE/work" HEAD_SHA="$(git -C "$GITHUB_WORKSPACE/work" rev-parse HEAD)" \ - bun .github/scripts/sync-agent.ts suggest - fi - # A green head both AI review models saw, with every finding fixed or declined and no reviewer requesting changes. + # A green head both AI review models saw, with every finding fixed or declined and no change request. # The approval runs fast-forward.yml, which re-checks the head and its checks before moving `next`. approve: name: Approve the reviewed sync diff --git a/apps/cli/docs/release-process.md b/apps/cli/docs/release-process.md index 5bf414b238..a2d25ca0d7 100644 --- a/apps/cli/docs/release-process.md +++ b/apps/cli/docs/release-process.md @@ -356,7 +356,7 @@ git push origin sync/develop-into-next A `develop-into-next` conflict PR is resolved by Claude (the `claude.opus` model in `.github/ai-models.json`) in the same run. The `Resolve conflicts with Claude` job merges `next` into the sync branch and runs Claude Code with file read, search, and edit tools only: no shell, no web tools, no `.git` edits, and no GitHub write token. A `pnpm-lock.yaml` conflict keeps `next`'s lockfile, which the next job regenerates. `Run quick checks on the resolution` formats the merged tree and runs `check:all` and unit tests in a job with no secrets, and `Fix the quick checks with Claude` fixes what fails once, without re-running them. This catches code on one side that still uses something the other renamed or removed; integration and end-to-end failures are left to the PR's CI and the repair loop below. The jobs holding the Anthropic key never execute the merged tree, and the tree passes between jobs as a git bundle. The `Publish the resolution` job pushes the latest finished stage from a fresh checkout with the release App token, comments the resolution and any check that failed, and starts the AI review. Choices between `develop` and `next` behavior are listed as decisions for `@supabase/cli`. If Claude cannot resolve the merge, it comments why and the PR waits for a person as above. -[`sync-repair.yml`](../../../.github/workflows/sync-repair.yml) (`Sync repair`) runs when a CI workflow or the AI review finishes, and hourly. Once every check on the PR head has finished and no `AI Review` run for the PR is still going, it gives Claude the failed jobs' log tails and the AI review's unanswered threads. Claude fixes what the merge broke and replies on each finding, declining findings the merge did not introduce. The fix is pushed as a `chore(repo)` commit, fixed threads are resolved, and a round comment is posted. A head gets another round only for new review findings. When a round leaves its head failing, or after two rounds, the bot asks `@supabase/cli` to take over and stops repairing that PR. When every check except `Require fast-forward` is green on a head both AI review models finished reviewing, and every finding they did not refute has a thread Claude fixed or declined, `supabase-oss` approves that head, which fast-forwards `next`. A head moved by a repair commit or a maintainer push is not re-reviewed, and findings outside the diff have no thread, so both wait for a maintainer's approval. To hold a sync, mark the PR as a draft or request changes as a repository collaborator; dismissing `supabase-oss`'s approval also stops it approving that head again. `Fast-forward on approval` re-checks change requests before moving `next` on `supabase-oss`'s approval. When it refuses, `supabase-oss` does not approve that head again, so a maintainer approves. The repair loop reads only CI output and the AI review's own threads, never other people's comments, and only acts on PRs Claude resolved. +[`sync-repair.yml`](../../../.github/workflows/sync-repair.yml) (`Sync repair`) runs when a CI workflow or the AI review finishes, and hourly. Once every check on the PR head has finished and no `AI Review` run for the PR is still going, it gives Claude the failed jobs' log tails and the AI review's unanswered threads. Claude fixes what the merge broke and replies on each finding, declining findings the merge did not introduce. The fix is pushed as a `chore(repo)` commit, fixed threads are resolved, and a round comment is posted. A head gets another round only for new review findings. When a round leaves its head failing, or after two rounds, the bot asks `@supabase/cli` to take over and stops repairing that PR. When every check except `Require fast-forward` is green on a head both AI review models finished reviewing, and every finding they did not refute has a thread Claude fixed or declined, `supabase-oss` approves that head, which fast-forwards `next`. A head moved by a repair commit or a maintainer push is not re-reviewed, and findings outside the diff have no thread, so both wait for a maintainer's approval. To hold a sync, mark the PR as a draft or request changes (any reviewer's change request holds it, so dismiss a stray one); dismissing `supabase-oss`'s approval also stops it approving that head again. `Fast-forward on approval` re-checks change requests before moving `next` on `supabase-oss`'s approval. When it refuses, `supabase-oss` does not approve that head again, so a maintainer approves. The repair loop reads only CI output and the AI review's own threads, never other people's comments, and only acts on PRs Claude resolved. Workflows on the sync branch run with repository secrets, so Claude never pushes a change under `.github/`. Claude resolves and fixes workflow files like any other, but those edits are posted as a review of suggestions, with a diff for file additions and deletions; a conflicted workflow keeps `develop`'s side until then. A maintainer reviews them and uses **Commit suggestions**, so the workflow change runs only once a person commits it, and repair waits for that. The publish job refuses to push when the merge's `.github/` differs from git's own merge (with `develop`'s side on conflicts) or a later commit touches `.github/`, and the repair token has no `workflows` permission. Claude's other edits run in the sync PR's CI before anyone reviews them, and land through a maintainer's approval or `Sync repair`'s approval under the conditions above. From 94ca24de25307529328deb4e023b7f5b4c04790a Mon Sep 17 00:00:00 2001 From: avallete Date: Wed, 14 Oct 2026 16:06:06 +0200 Subject: [PATCH 8/9] ci(release): count advisory next-impact findings in the AI review state Co-Authored-By: Claude Opus 5.5 --- .github/scripts/ai-review/post-review.test.ts | 3 ++- .github/scripts/ai-review/post-review.ts | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/scripts/ai-review/post-review.test.ts b/.github/scripts/ai-review/post-review.test.ts index 29959610ad..0b7656abc6 100644 --- a/.github/scripts/ai-review/post-review.test.ts +++ b/.github/scripts/ai-review/post-review.test.ts @@ -697,6 +697,7 @@ describe("buildReviewPayload", () => { makeFinding({ id: "inline", file: "file.ts", line: 10 }), makeFinding({ id: "outside", file: "file.ts", line: 99 }), makeFinding({ id: "refuted", adjudication: { verdict: "refuted", reason: "No." } }), + makeFinding({ id: "advisory", category: "next-impact" }), ], }); const payload = buildReviewPayload(review, anchors, { ...footer, reviewsComplete: false }); @@ -708,7 +709,7 @@ describe("buildReviewPayload", () => { expect(parseAiReviewState(truncated)).toEqual({ head: "a".repeat(40), complete: false, - findings: 2, + findings: 3, }); }); diff --git a/.github/scripts/ai-review/post-review.ts b/.github/scripts/ai-review/post-review.ts index 5eb5eab9e0..674ee84f2d 100644 --- a/.github/scripts/ai-review/post-review.ts +++ b/.github/scripts/ai-review/post-review.ts @@ -645,9 +645,10 @@ export function renderReviewBody( ); const verdicts = computeVerdictCounts(review.findings); - // The markers lead the body so truncation never drops them. + // The markers lead the body so truncation never drops them. Advisory next-impact findings have no thread, so + // counting them keeps any review that raised one from approving a sync. const state = footer.headSha - ? `${renderAiReviewState({ head: footer.headSha, complete: footer.reviewsComplete === true, findings: posted.length })}\n` + ? `${renderAiReviewState({ head: footer.headSha, complete: footer.reviewsComplete === true, findings: posted.length + review.findings.filter(isAdvisoryNextImpact).length })}\n` : ""; const header = `${AI_REVIEW_MARKER}\n${state}`; const sections: string[] = [`${header}## 🤖 AI Review\n\n${sanitizeModelText(review.summary)}`]; From 3e39ba666adc4d816f09c5ee0937da9cf5a6df4e Mon Sep 17 00:00:00 2001 From: avallete Date: Wed, 14 Oct 2026 17:51:35 +0200 Subject: [PATCH 9/9] test(ci): truncate the review content itself when checking the leading markers Co-Authored-By: Claude Opus 5.5 --- .github/scripts/ai-review/post-review.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/scripts/ai-review/post-review.test.ts b/.github/scripts/ai-review/post-review.test.ts index 0b7656abc6..3f20ecdd1f 100644 --- a/.github/scripts/ai-review/post-review.test.ts +++ b/.github/scripts/ai-review/post-review.test.ts @@ -695,7 +695,7 @@ describe("buildReviewPayload", () => { const review = makeMergedReview({ findings: [ makeFinding({ id: "inline", file: "file.ts", line: 10 }), - makeFinding({ id: "outside", file: "file.ts", line: 99 }), + makeFinding({ id: "outside", file: "file.ts", line: 99, claim: "x".repeat(70_000) }), makeFinding({ id: "refuted", adjudication: { verdict: "refuted", reason: "No." } }), makeFinding({ id: "advisory", category: "next-impact" }), ], @@ -704,9 +704,10 @@ describe("buildReviewPayload", () => { expect(payload.commit_id).toBe(footer.headSha); expect(payload.comments).toHaveLength(1); - const truncated = truncateReviewBody(`${payload.body}${"x".repeat(70_000)}`, footer.runUrl); - expect(truncated).toContain(AI_REVIEW_MARKER); - expect(parseAiReviewState(truncated)).toEqual({ + expect(payload.body).toContain("truncated"); + expect(payload.body).not.toContain(footer.modelsFooter); + expect(payload.body).toContain(AI_REVIEW_MARKER); + expect(parseAiReviewState(payload.body)).toEqual({ head: "a".repeat(40), complete: false, findings: 3,