Skip to content

ci(release): re-review repaired syncs and re-resolve them when next moves - #7205

Merged
avallete merged 3 commits into
developfrom
avallete/supabase-oss-auto-approval-debug-a3589b
Oct 16, 2026
Merged

avallete merged 3 commits into
developfrom
avallete/supabase-oss-auto-approval-debug-a3589b

Conversation

@avallete

@avallete avallete commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Summary

TL;DR: Since #7124, no Claude-resolved sync/develop-into-next PR has been auto-approved by supabase-oss. Claude's repair commit moves the head past the only AI review, so the approval gate (review head must equal PR head) never passes. This adds one bounded AI re-review after a repair, and when next moves and conflicts at fast-forward time, Claude re-resolves the same PR instead of asking a maintainer.

Before

flowchart LR
  A[AI review on X] --> B[Claude repair pushes Y]
  B --> C{"review head == Y?"}
  C -- no --> D[skip forever]
  D --> E[human approves]
  F[next moved + conflict] --> G[ask human to resolve]
Loading

After

flowchart LR
  A[AI review on X] --> B[Claude repair pushes Y]
  B --> C[one AI re-review on Y]
  C -- "clean or all answered" --> D[supabase-oss approves]
  C -- "needs a fix / CI red" --> E[maintainer]
  F[next moved + conflict] --> G["Claude re-resolves same PR<br/>(max 2 resyncs)"]
  G --> A
Loading

Why

On #7158, the AI review saw 39034946 and the repair pushed b064c720. Every later sync-repair run then logged "Nothing to repair; the AI review did not see this head." until a maintainer approved it. On #7112, next moved after approval and the resync conflicted, which left the PR waiting on a person.

What changed

  • A resolution is everything after the latest resolve comment (from the first Claude resolve or a resync). Repair rounds count per resolution, using release-bot comments and reviews sorted by time.
  • After a repair push, sync-repair posts a re-review marker, and a separate actions: write job dispatches the AI review once. The job that runs Claude never gets that permission. If the dispatch fails, it tags maintainers.
  • After the re-review, rounds can only reply to findings. Claude declines repeats by pointing at its earlier replies. If a finding needs a fix or CI fails, the PR goes to a maintainer. Nothing is pushed in these rounds.
  • fast-forward: if next moved and merging it into an approved develop→next sync conflicts, it dispatches sync-branches.yml with a new resync_pr input. That re-runs the resolve, check, fix and publish steps on the same branch and PR. The limit is 3 resolutions. A PR resolved by hand counts as one. If the dispatch fails, or the limit is reached, it falls back to today's "resolve by hand" message.
  • resync_pr only accepts an open same-repository sync/develop-into-next → next PR.
  • On a sync/develop-into-next PR, the AI review gets sync-resolution.diff: each sync merge as --remerge-diff, plus the commits made on the sync branch. Both prompts and the adjudicator limit findings to what that resolution introduced, or to code on one side that uses something the other side removed. Previously the review re-checked all of develop since the last sync, and most of its findings were dismissed as "not from the merge".
  • release-process.md documents the new rules.

Linked issue

  • I'm a Supabase maintainer.

Checklist

  • The PR title follows Conventional Commits.
  • Tests added or updated for the change.

🤖 Generated with Claude Code

…e them when next moves

A repair commit moved the sync head past the only AI review, so supabase-oss never approved a
Claude-resolved sync. Each resolution now gets one AI re-review after a repair push, later rounds
only answer findings, and a fast-forward conflict with a moved next hands the same pull request
back to Claude's resolution (at most two resyncs).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@avallete
avallete requested review from a team and supabase-oss as code owners October 16, 2026 13:47
@avallete

Copy link
Copy Markdown
Member Author

/ai-review

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superseded by a newer AI review

🤖 AI Review

Both independent reviews completed. All four distinct findings are confirmed: two major logic bugs, one minor resync-limit error, and one nit-level test-coverage gap.

Findings

Severity Location Category Sources Claim
🟠 MAJOR .github/scripts/sync-agent.ts:128 logic claude An exhausted marker from an earlier resolution permanently blocks repair and automatic approval after a new resync resolution.
🟠 MAJOR .github/sync-branches/repair-prompt.md:37 correctness codex The answer-only prompt instructs Claude to decline repeated findings even when an earlier claimed fix was incomplete, allowing a still-valid issue to bypass maintainer handoff.
🟡 MINOR .github/scripts/fast-forward.ts:434 correctness codex A manually resolved sync PR can receive three automatic resyncs instead of the documented maximum of two.
⚪ NIT .github/scripts/sync-agent.ts:877 test-coverage claude The new answer-only handoff, fixed-reply suppression, and conditional re-review marker behavior have no direct test coverage.

Stats

Claude findings: 2 · Codex findings: 2 · Confirmed: 4 · Refuted: 0 · Uncertain: 0


Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: auto · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.

Comment thread .github/scripts/sync-agent.ts
Comment thread .github/sync-branches/repair-prompt.md Outdated
Comment thread .github/scripts/fast-forward.ts Outdated
Comment thread .github/scripts/sync-agent.ts

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI Review

Verified all six findings against the surrounding code and trusted conventions. Four are confirmed: skipped re-review after a publish failure, unconditional dismissal of repeated findings, missing conflict fallback when listing comments fails, and an extra resync after a manual resolution. Two are refuted because the two-round limit and permanent maintainer handoff are documented policies. No findings overlap.

Findings

Severity Location Category Sources Claim
🟠 MAJOR .github/workflows/sync-repair.yml:160 error-handling claude A Publish failure after posting the re-review marker skips the rereview job, leaving the repaired head without its AI review or the dispatch-failure maintainer notification.
🟠 MAJOR .github/sync-branches/repair-prompt.md:37 correctness codex The unconditional instruction to decline repeated findings can dismiss a valid report that an earlier repair was ineffective, bypassing maintainer handoff and permitting automatic approval.
🟡 MINOR .github/scripts/fast-forward.ts:432 error-handling claude Failure to list PR comments aborts the conflicting resync path without posting the conflict list or manual-resolution fallback.
🟡 MINOR .github/scripts/fast-forward.ts:433 correctness codex A manually resolved initial PR receives three automatic resyncs rather than the documented maximum of two because its initial resolution disappears from the counter once automated markers exist.
Refuted findings (kept for transparency, not posted as review comments)
  • .github/scripts/sync-agent.ts:157 (correctness): A first repair commit made in round two triggers a re-review whose findings cannot receive an automatic answer-only round because the two-round budget is exhausted.
    Refuted: The exhaustion behavior is real but follows the existing hard two-round policy documented in trusted/apps/cli/docs/release-process.md:359. The re-review remains useful: .github/scripts/sync-agent.ts:150-151 evaluates approval before enforcing the repair budget, so a clean review can approve the repaired head; outstanding findings instead receive the maintainer handoff at lines 813-817. Read-only execution verified both outcomes. The comment restricts any later rounds without promising an additional round.
  • .github/scripts/sync-agent.ts:128 (correctness): The exhausted marker applies across the entire PR, preventing repair and automatic approval after a later resolution even though repair-round counters reset per resolution.
    Refuted: Permanent handoff is already documented in trusted/apps/cli/docs/release-process.md:359: after exhaustion the bot stops repairing that PR. The existing test in trusted/.github/scripts/sync-agent.test.ts:161-172 explicitly preserves handoff across a maintainer's new head; pr/.github/scripts/sync-agent.test.ts:196-207 retains it. Resetting a resolution's round counter does not revoke the separate PR-wide handoff, and the PR's release documentation retains that policy.

Stats

Claude findings: 4 · Codex findings: 2 · Confirmed: 4 · Refuted: 2 · Uncertain: 0


Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.

Comment thread .github/workflows/sync-repair.yml Outdated
Comment thread .github/scripts/fast-forward.ts Outdated
Comment thread .github/sync-branches/repair-prompt.md Outdated
Comment thread .github/scripts/fast-forward.ts Outdated
avallete and others added 2 commits October 16, 2026 15:58
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@avallete
avallete added this pull request to the merge queue Oct 16, 2026
Merged via the queue into develop with commit f01410e Oct 16, 2026
44 checks passed
@avallete
avallete deleted the avallete/supabase-oss-auto-approval-debug-a3589b branch October 16, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants