draft: coalesce matching exact-review ingress#706
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 19, 2026, 2:20 PM ET / 18:20 UTC. Summary Reproducibility: not applicable. as a conventional bug reproduction: the PR supplies a concrete paired-route control-plane canary, but the item is a proposed reliability protocol rather than a report of an unresolved current-main defect. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Approve or reject the v1 fingerprint field set and seven-day receipt policy, then land this only after the accepted generation/terminal protocol is known; rebase and repeat the paired-route canary against that resulting main branch before enabling the documented target-dispatcher rollout. Do we have a high-confidence way to reproduce the issue? Not applicable as a conventional bug reproduction: the PR supplies a concrete paired-route control-plane canary, but the item is a proposed reliability protocol rather than a report of an unresolved current-main defect. Is this the best way to solve the issue? Unclear pending maintainer direction: semantic cross-route coalescing is a substantially safer answer than the earlier route-skipping proposal, but the receipt lifetime, identity contents, and target-workflow rollout are product and operations choices. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3beb5f044d25. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
Review history (3 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 |
Status: draft - maintainer decision required
This is deliberately a decision-gated draft. It is not asking for merge, does
not change any ClawSweeper gate, and does not request an automated ClawSweeper
review until the protocol and rollout below are approved.
Summary
Preserve both exact-review reliability routes while coalescing only a proven
same-event delivery at the durable queue. A direct GitHub App
pull_requestwebhook and the compatibility
pull_request_targetdispatcher can now carry asmall, opaque cross-route identity envelope. The queue admits every delivery
unless it has already recorded the same identity from the other route with
the same resolved target branch.
Problem and prior work
For OpenClaw PR #110767,
the push
a1e94e165a58a0ac86ec30744ddca8499937e3d3at 10:19 UTC entered thecontrol plane through the direct
run 29683139308
and the legacy
run 29683141001.
The later run 29683229621
was lease-deferred, but still incurred redundant control-plane work.
Peter closed #703 because
skipping the fallback route would remove a documented reliability path. This
draft implements the safe shape from that review instead: semantic
cross-route coalescing, while retaining legacy-only admission.
Draft #674 owns the
separate per-item generation, live terminal-target, and reconciliation work.
This draft intentionally does not reimplement or claim terminal behavior for
the delayed projection observed on OpenClaw PR #111318. It must be rebased and
revalidated after #674's maintainer-approved merge.
Implementation
pull_requestintake hashes a versioned snapshot of repository,number, action, head SHA, update timestamp, body, and label.
forwards it only as
{ route: "target_dispatcher", fingerprint }.only a matching fingerprint from the opposite route and the same
resolved target branch. Invalid or absent ingress metadata is admitted;
legacy-only delivery remains the fallback.
pull_request_targetsource events to thesame PR event family before accepting the optional envelope.
new identity. A changed default branch also remains a new queue revision.
Maintainer decision and rollout contract
Please decide before merge or target-repository rollout:
workflow; this PR does not itself alter OpenClaw's workflow.
generation/terminal protocol.
through both routes in both arrival orders, a later body/revision update,
default-branch change, and maintainer-command preservation. Verify no
additional exact-review execution occurs for the paired delivery.
Validation
All commands passed: three TypeScript builds and 230 focused tests. The new
regression covers direct-first and legacy-first paired ingress, a legacy-only
delivery, a later PR body revision, and a changed target default branch.
Workflow YAML and the documented target-dispatcher YAML were parsed with the
repository's Node YAML parser; both embedded Bash programs passed
bash -n.actionlintis not installed on this Windows host.Docker-backed paired-route control-plane canary
On 2026-07-19, a disposable Docker-backed Crabbox local-container run proved
the rebased source tree at
2f5e742ac2through the Worker HTTP boundary andthe real local Durable Object emulator:
local-container/cbx_f1bbfee2887fusingmcr.microsoft.com/playwright:v1.60.0-noble; one-shot cleanup completed.wrangler dev --localwith a synthetic webhook secret and noproduction endpoint, GitHub credential, target repository dispatch, or
target-item mutation.
target-dispatcher
run:block and currentlegacy-event-queue-intakerun:block. A local fakeghtransported only the synthetic repositorydispatch between those two real scripts; the legacy block HMAC-signed the
real local
/internal/exact-review/enqueuerequest.{ deduped: true, dedupe_scope: "cross_route" }; each durable item stayedat revision 1.
later body revision and a changed resolved default branch each queued a new
revision (revision 2), so neither was cross-route suppressed.
issue_commentreached the expected local app-credentialboundary (
503 github_app_not_configured) after command classification. Noingress identity applies to commands; this intentionally did not attempt a
real app-authenticated GitHub dispatch.
Codex review closeout:
codex review --uncommittedinitially identified target-branch divergence;that finding was accepted, fixed, and the focused proof rerun.
codex review --uncommittedwas clean.codex review --base origin/mainwas clean for the originalcommit. After rebasing from current main, a second
codex review --base origin/mainwas clean for2f5e742ac2against3beb5f044d25971c9b46f68521e5420674529874.Risks and non-goals
The
.github/workflows/sweep.ymlchange has normal GitHub OAuth/workflow-scoperollout risk. The envelope is opt-in and fails open: without a valid target
dispatcher fingerprint, the existing legacy behavior is preserved. The receiver
stores only a SHA-256 receipt, not the PR body.
This draft does not hide terminal state in Bay, alter gates, dispatch Actions,
or fix #674's terminal reconciliation scope. No
@clawsweeper reviewcommenthas been requested while the maintainer decision is pending.