Skip to content

fix(exact-review): supersede stale PR heads before dispatch#842

Merged
brokemac79 merged 2 commits into
mainfrom
codex/csw-055-stale-head-supersession
Jul 25, 2026
Merged

fix(exact-review): supersede stale PR heads before dispatch#842
brokemac79 merged 2 commits into
mainfrom
codex/csw-055-stale-head-supersession

Conversation

@brokemac79

Copy link
Copy Markdown
Contributor

Summary

  • Supersede an obsolete queued pull-request head before it can reserve a workflow lease or dispatch.
  • Preserve explicit command/status work by rebinding it to the authoritative current head instead of dropping it.
  • Record an atomic, durable supersession audit with a rollback-compatible schema migration.

Incident timeline and source attribution

  • #840 merged at 2026-07-25T00:22:42Z (8006cf0bcf7ff810761c11a3cf0609b33b03475b).
  • For openclaw/openclaw#113453, the old head 26ec173dd5e9b53a36ce1e3f72795a3f20715be8 was superseded by 97eac4a7486d5f0c94ae91382ebb81f9e2bf9e2f; the durable current-head verdict already existed.
  • A later queue revision nevertheless admitted the obsolete 26ec head, dispatched it twice, and parked it as review_retry_exhausted after 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 sourceHeadSha was 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

  • #749 introduced the source-authority and active stale-run protections this path relies on. Its queued-admission coverage did not supersede a surviving legacy event whose PR remained open.
  • #840 added terminal admission for closed/missing targets, but deliberately had no open-PR head comparison; this is the direct missing case.
  • #838 bounded retry attempts, which made the missed supersession visible as 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:

  • An ordinary queued revision whose head differs from the live open PR head is removed locally as live_head_advanced; it records a supersession audit and increments completed/superseded metrics without dispatch or attempt consumption.
  • An explicit command/status revision is retained, advanced to the live head, and has stale retry and dispatch-failure state cleared. Its first admission pass does not dispatch the obsolete head; the next pass dispatches the refreshed current-head command.
  • Headless PR-body/status commands are not globally deduplicated by SHA. Existing terminal-command completion semantics remain unchanged.

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.
  • Focused regressions cover 26ec -> 97e style 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 check was 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.

@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. labels Jul 25, 2026
@clawsweeper

clawsweeper Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 24, 2026, 10:55 PM ET / July 25, 2026, 02:55 UTC.

ClawSweeper review

What this changes

This 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
Reviewed head: 21f465f05a7d382256de1b6c7a16abaf4d042cae

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch is focused and plausibly correct, but its external-PR merge gate is held by mock-boundary proof rather than independently demonstrated live admission behavior.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The PR includes useful copied before/after output from the real queue class and alarm loop, but its GitHub interaction is mocked at the HTTP boundary; add redacted live-admission output or an equivalent inspectable runtime artifact before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR includes useful copied before/after output from the real queue class and alarm loop, but its GitHub interaction is mocked at the HTTP boundary; add redacted live-admission output or an equivalent inspectable runtime artifact before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 4 items Distinct remaining admission case: The PR body and related merged PR #840 distinguish terminal-target admission from this PR's missing open-target head comparison. This proposal therefore is not superseded by the current terminal admission behavior.
Related source-authority provenance: Merged PR #749 established the surrounding stale-run/source-authority protections; the supplied incident description identifies a queued legacy event that survives those protections until admission.
Focused branch scope: The branch changes the queue implementation and its focused durable-object tests, adding a supersession audit identity and reason code plus regression coverage for stale ordinary work, explicit command refresh, and legacy schema behavior.
Findings None None.
Security None None.

How this fits together

Exact 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
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR includes useful copied before/after output from the real queue class and alarm loop, but its GitHub interaction is mocked at the HTTP boundary; add redacted live-admission output or an equivalent inspectable runtime artifact before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - The new admission path depends on a live GitHub pull-request read; a misclassified transient, rate-limit, or access failure could delay valid review work or incorrectly treat it as superseded.
  • Resolve merge risk (P1) - The supersession audit-table migration changes durable queue metadata, so upgrade and rollback compatibility need review in addition to the normal queue behavior.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Focused surface 2 files affected; 441 additions, 20 deletions The patch is concentrated in the durable queue implementation and its queue-level regression tests, making admission and schema behavior the central review surface.
Durable audit change 1 supersession audit schema expanded The added audit identity and reason code require upgrade and rollback scrutiny beyond ordinary dispatch behavior.

Root-cause cluster

Relationship: canonical
Canonical: #842
Summary: This PR is a distinct follow-up for the stale queued-head admission gap after related merged protections.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Add real admission-path proof before merge (recommended)
    Provide redacted terminal or runtime evidence showing a queued stale head is compared with a live open pull-request head, then superseded or refreshed without dispatching the obsolete revision.
  2. Accept the focused-harness evidence
    Merge based on the existing durable-object tests only if maintainers independently accept the remaining uncertainty around the live GitHub admission boundary and rollback path.
  3. Pause for a narrower follow-up
    Pause this change if the required live-read behavior cannot be demonstrated safely, because queue admission is an availability-sensitive control point.

Technical review

Best 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.

Labels

Label changes:

  • add P2: This is a bounded queue-reliability improvement that prevents unnecessary review workflow dispatches without evidence of a current user-facing outage.
  • add merge-risk: 🚨 compatibility: The patch migrates durable supersession-audit storage and must preserve existing rows and older-worker inserts.
  • add merge-risk: 🚨 availability: Admission gains a live GitHub read, so incorrect error handling could stall or mishandle valid exact-review work.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR includes useful copied before/after output from the real queue class and alarm loop, but its GitHub interaction is mocked at the HTTP boundary; add redacted live-admission output or an equivalent inspectable runtime artifact before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a bounded queue-reliability improvement that prevents unnecessary review workflow dispatches without evidence of a current user-facing outage.
  • merge-risk: 🚨 availability: Admission gains a live GitHub read, so incorrect error handling could stall or mishandle valid exact-review work.
  • merge-risk: 🚨 compatibility: The patch migrates durable supersession-audit storage and must preserve existing rows and older-worker inserts.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR includes useful copied before/after output from the real queue class and alarm loop, but its GitHub interaction is mocked at the HTTP boundary; add redacted live-admission output or an equivalent inspectable runtime artifact before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

Likely related people:

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add redacted terminal or runtime proof of the stale-head versus live-head admission result; remove private repository, endpoint, and credential details.
  • After adding proof, update the PR body to trigger a fresh ClawSweeper review; if it does not rerun, ask a maintainer to comment @clawsweeper re-review.
  • Complete review of the live-read error classifications and the durable audit migration on the submitted head.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@brokemac79
brokemac79 merged commit 4bfea92 into main Jul 25, 2026
8 checks passed
@brokemac79
brokemac79 deleted the codex/csw-055-stale-head-supersession branch July 25, 2026 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant