draft: coalesce matching exact-review ingress - #706
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 26, 2026, 10:55 AM ET / 14:55 UTC. ClawSweeper reviewWhat this changesThis PR adds a shared SHA-256 ingress identity so matching direct-webhook and target-dispatcher pull-request deliveries collapse to one durable exact-review queue admission while independent updates remain eligible. Merge readinessKeep this PR open for normal maintainer review. The supplied discussion records approved direction to retain both ingress routes while coalescing only proven same-event deliveries, and the branch includes focused runtime proof and clean checks; no discrete patch defect is established from the reviewed evidence. Priority: P2 Review scores
Verification
How this fits togetherExact review receives pull-request events through both the direct GitHub webhook and an optional target-repository dispatcher. Both routes feed the Durable Object queue, which authorizes, deduplicates, leases, and later dispatches review work to the sweep workflow. flowchart LR
A[Direct GitHub webhook] --> C[Exact-review Worker]
B[Target dispatcher] --> C
C --> D[Route-aware ingress identity]
D --> E[Durable exact-review queue]
E --> F[Source-authority and live-head checks]
F --> G[Leased review run]
G --> H[Review publication]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Merge only after maintainer review confirms the identity tuple remains the single documented contract across both routes, then roll it out opt-in and observe paired delivery counts while preserving legacy-only fallback behavior. Do we have a high-confidence way to reproduce the issue? No fresh current-main reproduction was run during this read-only review. The supplied incident identifies the duplicated dual-route delivery path, and the branch's local Worker/Durable Object proof exercises that source-level path in both delivery orders. Is this the best way to solve the issue? Yes. The approved design keeps the reliability fallback and coalesces only a matching, versioned identity at the durable queue, which is narrower and safer than disabling one ingress route outright. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 5196f34e7b69. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (5 earlier review cycles)
|
1dcd391 to
2f5e742
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Maintainer decision (relayed from @steipete): direction approved. Keep both delivery routes and coalesce proven same-event deliveries at the durable queue via the opaque cross-route identity envelope as proposed. Please proceed out of draft; one coordination note — the queue DO has moved substantially since Jul 19 (state-append window, supersession handling in #749's lane, batch publication), so rebase against current |
2f5e742 to
b8ffe55
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Summary
Rebased and revalidated
b8ffe5580b901f37a836f26d8abd2f948a21f557on currentmain(3ecc41e30f9da8aa4dfda1b2561c4883cab68cfd). This preserves bothexact-review ingress routes while allowing the durable queue to collapse one
proven same-event delivery before dispatch.
Peter approved this safe shape and requested a rebase/reverification in
his review.
This PR is ready for maintainer look, not a merge request or gate change.
Problem
For OpenClaw PR #110767,
push
a1e94e165a58a0ac86ec30744ddca8499937e3d3entered the durable controlplane both through the direct
pull_request run
and the legacy
pull_request_target run.
The later lease-deferred run
did not invoke Codex, but still consumed queue, runner, cache, Actions, and
Bay control-plane work.
Closing #703 correctly
preserved the reliability fallback. This patch keeps it; it canonicalizes only
the paired delivery, never a head SHA alone.
Implementation
pull_requestintake and the documented target dispatcher computethe same versioned SHA-256 identity from repository, PR number, action, head
SHA, update timestamp, body, and label.
only a matching admitted counterpart on the same resolved target branch.
verified direct delivery. Delayed counterparts cannot replace a newer
revision, completed review, or legacy-only update.
Receipt migration preserves pre-schema direct admissions and the supported
rollback/re-upgrade path; current unadmitted direct receipts use a distinct
sentinel until queue acceptance.
verification retry. Maintainer commands and later body/revision updates are
not suppressed.
Validation and proof
actionlintis unavailable on this Windows host. The focused workflow/parsertests passed, and the extracted dispatcher/legacy Bash blocks passed
bash -n.Docker Desktop
29.5.2/ Crabbox local-container proof passed on the committedworktree: lease
cbx_fe569c2c4dcb, imagemcr.microsoft.com/playwright:v1.60.0-noble. It runs the actual Worker withwrangler dev --local, the Durable Object emulator, and the current extracteddispatcher/legacy workflow blocks. It proves direct-then-legacy and
legacy-then-direct yield one queue admission; direct-only and legacy-only stay
admissible; a body revision becomes revision 2; a default-branch change does
not cross-route-coalesce; and a maintainer command reaches its expected local
app-credential boundary. It has no production credentials or GitHub mutation.
The container receives a raw synced workspace, so its diagnostic Git HEAD is
empty; the committed source SHA above is the force-pushed PR head.
Codex review closeout:
codex review --uncommitted: accepted and tested the delayed legacy-onlyidentity edge and rollback/re-upgrade receipt migration; final run clean.
codex review --base origin/main: clean — no actionable bugs against3ecc41e30f9da8aa4dfda1b2561c4883cab68cfd.#674 and terminal behavior
This preserves and credits #674:
it proposed separate per-item generation, live terminal-target checks, and
terminal reconciliation. #674 is closed without merge. This focused ingress
patch deliberately does not reimplement that terminal/reconciliation work or
hide terminal state in Bay; any remaining terminal behavior stays a separate
maintainer decision and follow-up.
Risk and rollout
The
.github/workflows/sweep.ymlchange carries normal GitHub OAuth/workflowscope risk. The target-dispatcher envelope is opt-in and fails open: absent or
invalid metadata preserves legacy admission. Only a SHA-256 receipt is stored,
not PR content. Roll out the documented dispatcher fields to a target only
after this PR lands and observe paired delivery counts; do not enable a gate,
dispatch Actions manually, or merge automatically.