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/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..3f20ecdd1f 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,29 @@ 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, claim: "x".repeat(70_000) }), + makeFinding({ id: "refuted", adjudication: { verdict: "refuted", reason: "No." } }), + makeFinding({ id: "advisory", category: "next-impact" }), + ], + }); + const payload = buildReviewPayload(review, anchors, { ...footer, reviewsComplete: false }); + + expect(payload.commit_id).toBe(footer.headSha); + expect(payload.comments).toHaveLength(1); + 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, + }); + }); + test("the injected models footer appears in the body verbatim", () => { const review = makeMergedReview({ findings: [] }); const payload = buildReviewPayload(review, anchors, footer); @@ -832,6 +860,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 +913,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 +1079,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( @@ -1208,17 +1242,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 6a7d1dbd1e..674ee84f2d 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; 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. */ @@ -619,7 +645,13 @@ export function renderReviewBody( ); const verdicts = computeVerdictCounts(review.findings); - const sections: string[] = [`## 🤖 AI Review\n\n${sanitizeModelText(review.summary)}`]; + // 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 + review.findings.filter(isAdvisoryNextImpact).length })}\n` + : ""; + 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( @@ -697,8 +729,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"), ); @@ -715,6 +745,7 @@ export interface InlineReviewComment { } export interface ReviewPayload { + commit_id?: string; event: "COMMENT"; body: string; comments: InlineReviewComment[]; @@ -763,7 +794,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 { + ...(footer.headSha ? { 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. */ @@ -903,11 +939,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), @@ -1138,6 +1175,8 @@ async function runPost(): Promise { const prDiffPath = requireEnv("PR_DIFF_PATH"); const claudeModel = requireEnv("CLAUDE_MODEL"); const codexModel = requireEnv("CODEX_MODEL"); + 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); @@ -1147,6 +1186,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/fast-forward.test.ts b/.github/scripts/fast-forward.test.ts index 008058b89e..c2d152cedc 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,36 @@ describe("runFastForward sync", () => { expect(() => repo.remoteTip("sync/main-into-develop")).toThrow(); }); + 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(); + 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 }; + }; + + 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([ + 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 () => { 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..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, @@ -199,9 +201,36 @@ 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; +} + +/** + * 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).values()].includes("CHANGES_REQUESTED"); +} + +function latestVerdicts(reviews: PullRequestReview[]): Map { + 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; +} + 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 +239,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 +479,21 @@ export async function runFastForward( if (pullRequest.draft) { return refuse(io, input, "pull request is a draft"); } + // 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( io, @@ -567,6 +613,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), @@ -600,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"); @@ -608,7 +676,12 @@ async function main(): Promise { if (!reviewCommitId && !dryRun) { throw new Error("REVIEW_COMMIT_ID is required"); } - const outcome = await runFastForward(io, { reviewCommitId, dryRun }); + // 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") { console.error(`::error::${outcome.reason}`); diff --git a/.github/scripts/sync-agent.test.ts b/.github/scripts/sync-agent.test.ts index cb8a51aa18..35fd2fa571 100644 --- a/.github/scripts/sync-agent.test.ts +++ b/.github/scripts/sync-agent.test.ts @@ -37,6 +37,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, failed, run(100, "Require fast-forward", "failure")], threads: [], @@ -50,6 +52,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, cancelled], threads: [], @@ -62,6 +66,8 @@ describe("planRepair", () => { const running = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, run(99, "CodeQL", null)], threads: [finding], @@ -69,6 +75,8 @@ describe("planRepair", () => { const missing = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: green.slice(1), threads: [finding], @@ -82,6 +90,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [ "@supabase/cli Claude could not resolve this merge: ", ], @@ -99,6 +109,8 @@ describe("planRepair", () => { runs: green, threads: [finding], reviewRunning: false, + aiReview: null, + changesRequested: false, }); expect(plan.action).toBe("skip"); @@ -108,18 +120,22 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + 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, + aiReview: null, + changesRequested: false, botComments: [RESOLVED, repaired(HEAD)], runs: [...green, run(99, "Check code quality", "failure")], }; @@ -132,6 +148,8 @@ describe("planRepair", () => { const state = { head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, runs: [...green, run(99, "Check code quality", "failure")], threads: [], }; @@ -144,6 +162,8 @@ describe("planRepair", () => { const plan = planRepair({ head: "d".repeat(40), reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED, repaired(HEAD), ""], runs: [...green, run(99, "Check code quality", "failure")], threads: [finding], @@ -156,6 +176,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: true, + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, run(99, "Check code quality", "failure")], threads: [], @@ -168,6 +190,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED, ``], runs: [...green, run(99, "Check code quality", "failure")], threads: [], @@ -175,6 +199,29 @@ describe("planRepair", () => { expect(plan.action).toBe("skip"); }); + + 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, + botComments: [RESOLVED], + runs: [...green, run(100, "Require fast-forward", "failure")], + threads: [], + changesRequested: false, + }; + + 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("buildSuggestions", () => { diff --git a/.github/scripts/sync-agent.ts b/.github/scripts/sync-agent.ts index b5427aa171..3d8f7c2ee8 100644 --- a/.github/scripts/sync-agent.ts +++ b/.github/scripts/sync-agent.ts @@ -6,8 +6,19 @@ 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 { type CheckRun, fetchPullRequestCheckRuns, REQUIRED_CHECKS } from "./fast-forward.ts"; +import { + AI_REVIEW_MARKER, + type AiReviewState, + isSuperseded, + parseAiReviewState, +} from "./ai-review/post-review.ts"; +import { + type CheckRun, + fetchPullRequestCheckRuns, + hasChangesRequested, + type PullRequestReview, + REQUIRED_CHECKS, +} from "./fast-forward.ts"; import { githubGraphql, githubRequest, @@ -50,6 +61,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 +105,10 @@ export function planRepair(state: { runs: CheckRun[]; threads: ReviewThread[]; reviewRunning: boolean; + /** 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))) { @@ -121,7 +137,7 @@ 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." }; + 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))); @@ -131,6 +147,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) => { @@ -522,7 +565,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 } } } } } @@ -544,7 +587,7 @@ interface ThreadsPage { path: string; line: number | null; author: { login: string } | null; - pullRequestReview: { body: string } | null; + pullRequestReview: { databaseId: number; body: string } | null; }[]; }; }[]; @@ -553,32 +596,38 @@ 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)), }, ]; }); @@ -691,11 +740,20 @@ 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( + 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) && + !isSuperseded(review.body), + ); + const aiReviewState = aiReview ? parseAiReviewState(aiReview.body) : null; + const aiThreads = await fetchAiReviewThreads(token, repository, pull.number); const plan = planRepair({ head: pull.head.sha, botComments: comments @@ -707,15 +765,25 @@ 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), + 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)); setOutput("head", pull.head.sha); - if (plan.action === "skip") { - console.log(plan.reason); + if (plan.action === "skip" || plan.action === "approve") { + console.log(plan.action === "skip" ? plan.reason : "Ready for approval."); return; } if (plan.action === "exhausted") { 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 54d3030fb5..80e8ed7e6d 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 }} @@ -126,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" \ @@ -137,8 +146,38 @@ 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 + + # 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 + 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)" + # 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 \ + -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 d089c98724..a2d25ca0d7 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 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 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. **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