fix(exact-review): supersede stale PR heads before dispatch#842
Conversation
|
@clawsweeper review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review: needs real behavior proof before merge. Reviewed July 24, 2026, 10:55 PM ET / July 25, 2026, 02:55 UTC. ClawSweeper reviewWhat this changesThis PR makes the exact-review queue compare source-head-bound work with the live open pull-request head before dispatch, superseding obsolete ordinary work and refreshing explicit command work to the current head. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 3 items remain This PR addresses a distinct exact-review admission gap that remains after the merged stale-run and terminal-target protections: an open pull request can retain queued work for an older head until after workflow capacity is consumed. The patch is focused and its stated behavior is plausible, but the external-PR proof gate is not met because the reported before/after run uses mocked GitHub responses rather than an inspectable live admission response or equivalent redacted runtime evidence. Priority: P2 Review scores
Verification
How this fits togetherExact Review turns pull-request events and explicit review commands into durable queue items, then dispatches workflows and publishes review status. This patch changes the queue-admission step so obsolete pull-request revisions are handled before they reserve a lease or trigger a workflow. flowchart LR
A[Pull-request event or review command] --> B[Durable exact-review queue]
B --> C[Admission pass]
C --> D[Read live pull-request head]
D --> E{Queued head matches?}
E -->|Ordinary stale work| F[Supersede and audit locally]
E -->|Explicit stale command| G[Refresh command to live head]
E -->|Current head| H[Reserve lease and dispatch]
F --> I[Queue metrics and status]
G --> I
H --> I
Before merge
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the pre-dispatch live-head reconciliation narrow, retain explicit command-status work on the authoritative head, and add redacted runtime evidence that demonstrates the live-head admission interaction plus legacy-schema compatibility. Do we have a high-confidence way to reproduce the issue? Yes at source level: the described failure path is specific—a queued source head can remain older than the live head while the pull request is still open, so terminal admission does not stop dispatch. The supplied runtime output exercises that queue behavior, but the live GitHub response itself is mocked. Is this the best way to solve the issue? Yes, provisionally: reconciling source-head-bound work at the existing bounded admission stage addresses the wasted dispatch before lease reservation without changing queue capacity or retry policy. The safest merge path still needs proof of live-read failure handling and durable migration behavior. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8006cf0bcf7f. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
Summary
Incident timeline and source attribution
2026-07-25T00:22:42Z(8006cf0bcf7ff810761c11a3cf0609b33b03475b).26ec173dd5e9b53a36ce1e3f72795a3f20715be8was superseded by97eac4a7486d5f0c94ae91382ebb81f9e2bf9e2f; the durable current-head verdict already existed.26echead, dispatched it twice, and parked it asreview_retry_exhaustedafter eight attempts. The lease correctly rejected the stale source tuple, but only after workflow consumption.This repair is based on that observed stale-head incident. Validation uses only local durable storage and mocked GitHub responses; it made no production queue, target, comment, workflow, Actions, or gate mutation.
Root cause
Admission checked only the issue's open/terminal state. An open PR whose queued
sourceHeadShawas older than GitHub's live PR head remained dispatchable, so the lease-level source validation happened too late.Causal boundary and related post–21 July work
review_retry_exhausted; it did not create the stale source condition.These are scoped causal links, not a blanket attribution: this PR closes the interaction gap between their protections.
Behavior change and command boundary
Before workflow dispatch, a source-head-bound pull request is read through the authoritative PR endpoint:
live_head_advanced; it records a supersession audit and increments completed/superseded metrics without dispatch or attempt consumption.Audit writes occur in the same durable transaction as state and metrics. The audit-table upgrade preserves old rows and gives rollback-era inserts a generated default audit ID, avoiding loss if an older worker temporarily writes the prior column set.
Validation
node --test test/dashboard-worker.test.ts— 206 passing.pnpm run build:dashboard— passed.pnpm exec oxfmt --check dashboard/exact-review-queue.ts test/dashboard-worker.test.ts— passed.git diff --check— passed.26ec -> 97estyle stale ordinary work (zero dispatch/attempts), stale command refresh and status preservation, stale dispatch diagnostic clearing, repeat audit identities, and legacy-schema/rollback insert compatibility.pnpm run checkwas also evaluated: active-surface, dashboard-boundary, and limits checks pass; its formatter stage reports the established 488 unrelated current-main files, while both changed files pass the focused formatter check above.Review closeout
codex review --uncommitted— final dirty-patch review clean after accepted audit-identity, atomic-write, atomic-migration, rollback-compatibility, and refreshed-command diagnostic findings were fixed and retested.codex review --base origin/main— final branch review clean: no actionable correctness issues.Risk and rollout
The change adds a read-only PR-head check only for source-head-bound PR decisions during the existing bounded admission pass. It avoids workflow and queue-control changes, does not alter capacity or retry limits, and keeps explicit command completion intact. No changelog or workflow change is included.