Skip to content

fix(core): recover review head drift cleanly (BRIX-1768)#112

Open
quay-worker[bot] wants to merge 9 commits into
devfrom
quay/umbrella/BRIX-1768
Open

fix(core): recover review head drift cleanly (BRIX-1768)#112
quay-worker[bot] wants to merge 9 commits into
devfrom
quay/umbrella/BRIX-1768

Conversation

@quay-worker

@quay-worker quay-worker Bot commented Jun 22, 2026

Copy link
Copy Markdown

Quay Umbrella Final PR

Umbrella external ref: BRIX-1768
Umbrella title: Quay: make PR review tasks recover cleanly from head/worktree drift
Linear ticket: BRIX-1768
Source branch: quay/umbrella/BRIX-1768
Target branch: dev

Quay opened this PR after all expected umbrella subtasks were accounted for.

Expected Subtasks

Review update

  • Bind review scheduling to CI evidence from the same PR head SHA; mismatched snapshots stay pending instead of creating a reviewer attempt.
  • Add a regression test for a force-push race between prView and prSnapshotByNumber.

Deployment Steps

✅ No manual deployment steps needed, will be fully operational after automated CI deployment.

Test Plan

Pre-merge checklist

  • bun run typecheck:cli
  • bun test packages/cli/tests/review/pr_review_entry.test.ts
  • bun run test:cli

Post-deployment verification

  • Verify synthetic review requests remain pending when PR head changes between review scheduling reads, then schedule after the next green same-head CI snapshot.

@linear

linear Bot commented Jun 22, 2026

Copy link
Copy Markdown

BRIX-1768

@quay-reviewer quay-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Findings

Blocking

🔴 [1] enterReview can still schedule a reviewer for a stale PR head
enterReview now takes the attempt SHA from prView at pr_review.ts:239, but it later classifies CI from a separate prSnapshotByNumber read at pr_review.ts:305 and then writes/schedules the original headSha at pr_review.ts:510-L515. If the PR is force-pushed after prView returns SHA A but before prSnapshotByNumber returns SHA B with green checks, classifyCi sees a self-consistent snapshot for B and returns pass, while Quay inserts a review attempt for A. The spawn path will usually retarget before launch, but if that guard is missed or races again, the finalization path records the posted review against task.head_sha; synthetic review approvals and requested-changes outcomes are not protected by the Quay-owned approved-review CI guard. This reintroduces the head/worktree drift class this PR is trying to eliminate. Treat a snapshot whose headSha does not equal the selected review headSha as stale or re-read prView after the CI snapshot and only schedule when both reads agree.

Review scheduling must bind the selected commit and CI evidence to the same live PR head before creating an attempt, because independent GitHub reads can cross a force-push boundary.

Non-blocking

None.

@quay-worker quay-worker Bot changed the title Quay: make PR review tasks recover cleanly from head/worktree drift (BRIX-1768) fix(core): recover review head drift cleanly (BRIX-1768) Jun 22, 2026

@quay-reviewer quay-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lgtm!

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.

0 participants