Skip to content

draft: coalesce matching exact-review ingress#706

Draft
brokemac79 wants to merge 1 commit into
mainfrom
codex/csw-046-safe-ingress-dedupe
Draft

draft: coalesce matching exact-review ingress#706
brokemac79 wants to merge 1 commit into
mainfrom
codex/csw-046-safe-ingress-dedupe

Conversation

@brokemac79

@brokemac79 brokemac79 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

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_request
webhook and the compatibility pull_request_target dispatcher can now carry a
small, 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 a1e94e165a58a0ac86ec30744ddca8499937e3d3 at 10:19 UTC entered the
control 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

  • Direct pull_request intake hashes a versioned snapshot of repository,
    number, action, head SHA, update timestamp, body, and label.
  • The documented target-dispatcher template creates the same SHA-256 value and
    forwards it only as { route: "target_dispatcher", fingerprint }.
  • The durable queue stores the opaque seven-day receipt per route. It coalesces
    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.
  • The queue canonicalizes legacy pull_request_target source events to the
    same PR event family before accepting the optional envelope.
  • This is not a head-SHA-only dedupe: body/revision metadata changes produce a
    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:

  1. Approve the v1 identity field set and seven-day receipt retention.
  2. Approve adding the documented dispatcher fields to OpenClaw's target
    workflow; this PR does not itself alter OpenClaw's workflow.
  3. Sequence this after Review each selected PR in its own item shard #674, then rebase and repeat the proof against its
    generation/terminal protocol.
  4. Approve a canary that demonstrates direct-only, legacy-only, one delivery
    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

pnpm exec oxfmt .github/workflows/sweep.yml dashboard/exact-review-queue.ts dashboard/worker.ts docs/target-dispatcher.md test/clawsweeper.test.ts test/dashboard-worker.test.ts
git diff --check
pnpm run lint:dashboard
pnpm run build:all
node --test test/clawsweeper.test.ts test/dashboard-worker.test.ts

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.
actionlint is 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 2f5e742ac2 through the Worker HTTP boundary and
the real local Durable Object emulator:

  • Provider/lease: local-container / cbx_f1bbfee2887f using
    mcr.microsoft.com/playwright:v1.60.0-noble; one-shot cleanup completed.
  • Worker: wrangler dev --local with a synthetic webhook secret and no
    production endpoint, GitHub credential, target repository dispatch, or
    target-item mutation.
  • Route proof: the harness extracted and executed the current documented
    target-dispatcher run: block and current legacy-event-queue-intake
    run: block. A local fake gh transported only the synthetic repository
    dispatch between those two real scripts; the legacy block HMAC-signed the
    real local /internal/exact-review/enqueue request.
  • Direct-first and legacy-first both returned one queued admission followed by
    { deduped: true, dedupe_scope: "cross_route" }; each durable item stayed
    at revision 1.
  • A legacy-only delivery and a direct-only delivery each queued normally. A
    later body revision and a changed resolved default branch each queued a new
    revision (revision 2), so neither was cross-route suppressed.
  • A signed maintainer issue_comment reached the expected local app-credential
    boundary (503 github_app_not_configured) after command classification. No
    ingress identity applies to commands; this intentionally did not attempt a
    real app-authenticated GitHub dispatch.

Codex review closeout:

  • codex review --uncommitted initially identified target-branch divergence;
    that finding was accepted, fixed, and the focused proof rerun.
  • The repeated codex review --uncommitted was clean.
  • The initial codex review --base origin/main was clean for the original
    commit. After rebasing from current main, a second
    codex review --base origin/main was clean for 2f5e742ac2 against
    3beb5f044d25971c9b46f68521e5420674529874.

Risks and non-goals

The .github/workflows/sweep.yml change has normal GitHub OAuth/workflow-scope
rollout 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 review comment
has been requested while the maintainer decision is pending.

@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: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jul 19, 2026
@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 19, 2026, 2:20 PM ET / 18:20 UTC.

Summary
The PR adds SHA-256 ingress fingerprints to direct webhook and target-dispatcher pull-request deliveries so the durable exact-review queue coalesces only matching deliveries received through opposite routes.

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.

  • Patch surface: 6 files affected; 457 added, 6 removed. The change spans queue state, webhook intake, workflow dispatch, docs, and focused regression coverage, so the operational contract should be reviewed as one protocol.
  • Focused coverage: 243 test lines added in queue/worker tests. The core behavior is exercised for both route orders and for revisions that must remain distinct.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Obtain an explicit decision on the v1 identity field set, seven-day receipt retention, and target-workflow rollout.
  • After the accepted Review each selected PR in its own item shard #674 protocol lands, rebase this branch and repeat the paired-route canary against current main.

Risk before merge

  • [P1] Merging changes the durable queue’s admission contract and introduces seven-day receipt retention; maintainers need to explicitly accept the intended duplicate-versus-new-revision boundary.
  • [P1] The target-dispatcher documentation requires target repositories to opt into matching identity generation, so rollout needs a coordinated canary and upgrade communication rather than an implicit workflow behavior change.
  • [P1] The related per-item generation and terminal-reconciliation draft Review each selected PR in its own item shard #674 remains open, so merging this branch first could require a second compatibility revalidation after that protocol changes.

Maintainer options:

  1. Approve staged compatibility rollout (recommended)
    Accept the retained legacy-only fallback and merge only after the receipt contract, target-dispatcher adoption, and post-Review each selected PR in its own item shard #674 canary are explicitly approved.
  2. Retain current independent admission
    Do not merge this protocol if maintainers prefer duplicate queue admission over retaining opaque cross-route receipt state.
  3. Pause for a revised contract
    Keep the draft open while a narrower fingerprint field set or retention policy is designed and reviewed.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
After maintainer approval and the related queue-protocol merge, rebase onto current main and repeat the documented paired-route canary before enabling target-dispatcher adoption.

Next step before merge

  • [P2] A maintainer must choose the reliability contract and rollout sequence; no narrow automated repair can safely decide those policy boundaries.

Maintainer decision needed

  • Question: Should ClawSweeper adopt this opt-in cross-route fingerprint protocol, including seven-day opaque receipt retention, as the compatibility-preserving way to coalesce duplicate exact-review ingress?
  • Rationale: The patch is intentionally decision-gated: code cannot determine the acceptable retention window, semantic identity fields, target-workflow rollout responsibility, or sequencing relative to the still-open queue protocol work.
  • Likely owner: Peter — The discussion attributes the preceding fallback-route policy decision to Peter, and this PR asks for the corresponding reliability-preserving replacement decision.
  • Options:
    • Approve staged protocol rollout (recommended): Approve the v1 identity and seven-day receipt contract, merge only after the accepted Review each selected PR in its own item shard #674 protocol lands, then require a rebased paired-route canary before target workflow adoption.
    • Keep both routes without coalescing: Reject durable cross-route coalescing and retain the current duplicate admission behavior until a different reliability contract is designed.
    • Request a narrower contract: Ask for a revised identity or retention design before deciding whether target-dispatcher rollout should be supported.

Security
Cleared: The reviewed diff adds no new secret, permission, dependency, artifact-download, or untrusted-code execution surface beyond the existing webhook and workflow paths; its SHA-256 envelope is opaque and validated before queue use.

Review details

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

Label justifications:

  • P2: This is a bounded exact-review reliability improvement with no evidence of an active user-facing outage.
  • merge-risk: 🚨 compatibility: Existing target-dispatcher workflows and durable queue behavior must remain compatible when ingress metadata is absent or when the target branch changes.
  • merge-risk: 🚨 automation: The PR changes GitHub workflow ingestion and durable queue admission, so a mistaken identity contract could alter exact-review execution behavior after merge.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body supplies a dated Docker-backed local Durable Object canary that exercises both real documented ingress scripts, both arrival orders, distinct revisions, and command classification, with an observed improved queue result.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies a dated Docker-backed local Durable Object canary that exercises both real documented ingress scripts, both arrival orders, distinct revisions, and command classification, with an observed improved queue result.
Evidence reviewed

What I checked:

  • Bounded queue protocol: The proposed queue admission path accepts an optional ingress envelope and records route-specific fingerprints so only a matching delivery from the other route can be coalesced; malformed or absent metadata remains admitted. (dashboard/exact-review-queue.ts:319, 2f5e742ac2b0)
  • Direct-route identity: The direct GitHub webhook path derives a versioned fingerprint from repository, item number, action, head SHA, update timestamp, body, and label before enqueueing a pull-request review. (dashboard/worker.ts:1235, 2f5e742ac2b0)
  • Compatibility route retained: The workflow canonicalizes legacy pull_request_target source events to the pull-request family and forwards an ingress envelope only when the documented target dispatcher provides a valid fingerprint. (.github/workflows/sweep.yml:263, 2f5e742ac2b0)
  • Behavioral coverage and canary: The PR adds focused direct-first, legacy-first, legacy-only, changed-body, and changed-default-branch coverage, while the supplied Docker-backed Durable Object canary reports the intended admission and cross-route dedupe results. (test/dashboard-worker.test.ts:10392, 2f5e742ac2b0)
  • Related protocol dependency: The PR explicitly defers per-item generation and terminal reconciliation to the open draft Review each selected PR in its own item shard #674, so this ingress protocol should be decided and then rebased against that work rather than treated as an independent rollout. (6a039cfd0720)

Likely related people:

  • Peter: The PR discussion identifies Peter as the person who closed the preceding fallback-skipping proposal after deciding the documented legacy reliability route must remain available. (role: prior reliability-policy decision maker; confidence: medium; files: .github/workflows/sweep.yml, dashboard/exact-review-queue.ts)
  • hxy91819: The open related draft Review each selected PR in its own item shard #674 owns the generation, terminal reconciliation, and queue-protocol rollout work that this draft explicitly depends on. (role: adjacent queue-protocol contributor; confidence: medium; commits: 6a039cfd0720; files: dashboard/exact-review-queue.ts, dashboard/worker.ts, .github/workflows/sweep.yml)
  • brokemac79: This contributor also authored the earlier ingress proposal cited in the PR discussion, providing context on the observed duplicate-route behavior and the alternative compatibility-preserving design. (role: related ingress-proposal contributor; confidence: low; commits: 2f5e742ac2b0, 94791a34d89f; files: .github/workflows/sweep.yml, docs/target-dispatcher.md)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • 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.
Review history (3 earlier review cycles)
  • reviewed 2026-07-19T14:23:38.510Z sha 1dcd391 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-19T15:05:16.324Z sha 1dcd391 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-19T17:38:21.298Z sha 2f5e742 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 19, 2026
@brokemac79
brokemac79 force-pushed the codex/csw-046-safe-ingress-dedupe branch from 1dcd391 to 2f5e742 Compare July 19, 2026 17:35
@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 19, 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: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 19, 2026
@steipete

Copy link
Copy Markdown
Contributor

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 main and re-verify the admission path before requesting review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. 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. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants