Skip to content

fix(queue): dedupe unchanged pull request edits - #857

Merged
brokemac79 merged 2 commits into
mainfrom
codex/csw-063-edited-semantic-dedupe
Jul 26, 2026
Merged

fix(queue): dedupe unchanged pull request edits#857
brokemac79 merged 2 commits into
mainfrom
codex/csw-063-edited-semantic-dedupe

Conversation

@brokemac79

@brokemac79 brokemac79 commented Jul 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Deduplicate semantically unchanged pull_request.edited exact-review ingress before a second durable review request is created.

Problem

Distinct GitHub pull_request.edited deliveries can represent the same review-relevant state. Delivery-ID idempotency alone retains both requests and creates a second durable review revision.

Implementation

  • Hash a review-relevant edited-PR tuple: target, head/base SHA, draft state, normalized title/body revision, and normalized command/request configuration.
  • Store only the tuple fingerprint in a seven-day durable semantic cursor; raw title, body, and prompt content are not persisted there.
  • Suppress only unchanged edited-PR inputs. Changed head/base, draft/ready state, content, or request configuration still enqueue.
  • Advance the source-authority watermark on a suppressed newer duplicate, including mixed-case repository spellings, so a delayed older edit cannot replace the current review.
  • Preserve redelivery idempotency for source-authority reservations written before these optional semantic fields existed.
  • Expose review.semantic_deduped_total and an explicit semantic_edited / unchanged_pull_request_edit response reason.
  • Preserve the same tuple through the documented local/repository-dispatch fallback and route valid PR payloads through the existing authenticated source-authority API. Older or malformed fallback payloads retain direct enqueue behavior.

This remains limited to same-route pull_request.edited semantic deduplication. It does not change #706’s cross-route ingress coalescing, terminalisation, publisher batching, or state-writer behavior.

Compatibility with current main

Rebased onto current origin/main at 2ea92d0bfc126b92d174e642fdbeabc3d392c9b3 after merged #855, #859, #863, #866, #867, #706, and #858. #855 and #863 do not overlap this ingress surface. #859/#866/#867 make Cloudflare/Durable Object canonical for exact-review records; this change continues to use the queue’s authenticated source-authority and SQLite DO interfaces only. It does not implement or alter the Cloudflare migration, canonical record projection, publication, or materializer paths.

Current-main reconciliation with #706

#706, now merged, coalesces one proven direct-webhook/target-dispatcher pair using a route-tagged opaque ingress fingerprint. Its durable receipt is distinct from this PR’s same-route edited-state cursor:

  • draft: coalesce matching exact-review ingress #706 continues to suppress only a matching admitted counterpart from the other ingress route.
  • This PR continues to suppress only an unchanged pull_request.edited review tuple for the same PR: head/base, draft state, normalized content/request configuration, and target.
  • Both durable tables, expiry windows, and metrics are retained. A target-dispatcher payload remains on its direct enqueue path; only a complete edited fallback tuple uses source authority.
  • The repository-dispatch tuple is nested under queue_claim, keeping the outer payload within GitHub’s ten-property limit. Incomplete or non-edited fallback deliveries keep the existing direct-enqueue behavior, so retries cannot obtain a newer source-authority sequence.

Why #859 does not supersede this PR

The fresh ClawSweeper comment proposes closing this PR as superseded by #859. I do not believe that conclusion matches the implementation boundary, so this section records the evidence for maintainer review. It is not a request to merge or close either PR.

  • feat: Codex-only runtime, OpenClaw fallback runner, direct Cloudflare publication #859 (4288d557fff1) is already an ancestor of this branch and current main; this PR was rebased on top of it.
  • feat: Codex-only runtime, OpenClaw fallback runner, direct Cloudflare publication #859 deduplicates Cloudflare/direct-publication plans by item key and already-created review revision. That protects result publication after review work exists; it does not suppress a duplicate pull_request.edited review request at ingress.
  • Current main still carries only pull-request head SHA, update time, and source-authority sequence. It has no base SHA, draft state, normalized content/request digest, semantic-edit cursor, or unchanged_pull_request_edit reason. A later source-authority sequence can therefore create a newer review revision for an otherwise unchanged edit.
  • This PR adds the missing ingress tuple, durable seven-day cursor, suppression metric/reason, and fallback propagation. Its focused Worker/DO proof observes one queued first edit, one semantically deduped repeat, and one queued changed-content edit.

Accordingly, #859 is a prerequisite architecture change, not a functional replacement for this PR’s acceptance requirement: exactly one durable request for a truly unchanged pull_request.edited event. The PR remains open pending a maintainer decision; no re-review, merge, or close is requested here.

Validation

pnpm exec oxfmt dashboard/worker.ts src/repair/comment-webhook.ts test/dashboard-worker.test.ts test/repair/comment-webhook.test.ts test/sweep-workflow.test.ts .github/workflows/sweep.yml
pnpm run build:repair
node --test test/dashboard-worker.test.ts
node --test test/repair/comment-webhook.test.ts
node --test --test-name-pattern "sweep event reviews and target fanout avoid storm amplification" test/sweep-workflow.test.ts
node --test --test-name-pattern "target dispatcher documents opt-in cross-route identity|sweep workflow executes only durable queue leases without runner-side admission" test/clawsweeper.test.ts
pnpm run build:dashboard
pnpm run lint:dashboard
pnpm run build:all
git diff --check
  • test/dashboard-worker.test.ts: 231 passed, 0 failed.
  • test/repair/comment-webhook.test.ts: 27 passed, 0 failed.
  • Focused test/sweep-workflow.test.ts assertion: 1 passed, 0 failed.
  • Focused cross-route workflow/documentation assertions: 2 passed, 0 failed.
  • actionlint is not installed on this host.

The unfiltered combined workflow-source suite has six unrelated native-Windows baseline failures because its Linux/Bash helper checks invoke unavailable /bin/bash through WSL. Per the platform-scope gate, that is an intentionally Linux-hosted integration (category 3); no platform, CI, shell, or workflow-policy change was made for it.

Redacted Worker / Durable Object runtime proof

Synthetic local Wrangler proof exercised the real local Worker, its signed internal routes, and the SQLite-backed Durable Object. It did not contact GitHub or production.

runtime: local Worker -> signed fallback/queue routes -> SQLite Durable Object
health: true
first edited tuple:             202 { queued: true }
unchanged edited tuple:         202 { deduped: true,
                                      dedupe_scope: "semantic_edited",
                                      dedupe_reason: "unchanged_pull_request_edit" }
changed content tuple:          202 { queued: true }
fallback source-authority path: 200 { source_authority_seq: 1 }

The initial local run also exercised changed base and draft inputs; both returned 202 { queued: true }. All payload data was synthetic; the durable metric contained one semantic dedupe and the latest changed state remained queued.

Risks and rollout

The semantic path fails open when the required edited-PR fields are absent or invalid, preserving existing enqueue behavior. Cursor retention matches delivery receipt retention (seven days). The durable cursor and metric contain fingerprints/counts only, not raw review content.

The fallback addition touches .github/workflows/sweep.yml, so GitHub OAuth/workflow-scope configuration remains a deployment risk. It uses the existing HMAC secret and source-authority API only; no gate, secret, permission, state-writer, or publication behavior changed.

Related work

Codex review closeout

The first post-rebase codex review --base origin/main found one accepted P2: the local/repository-dispatch fallback did not propagate the semantic tuple or reserve source authority. This PR now carries the tuple through that fallback and uses the existing authenticated source-authority route.

After the remedy and focused proof:

  • codex review --uncommitted: clean; no accepted/actionable findings.
  • codex review --base origin/main: clean; “The semantic edited-event deduplication, authority watermark handling, and fallback ingress changes are internally consistent and covered by focused tests.”

For the current-main rebase, the first dirty-patch review accepted two compatibility findings: the fallback tuple exceeded GitHub’s repository-dispatch top-level property limit, and non-edited retry deliveries could receive newer source authority. The tuple now travels in queue_claim, and source authority is limited to complete edited tuples. Focused proof was rerun after both fixes.

  • Current-main codex review --uncommitted: clean; no accepted/actionable findings.
  • Current-main codex review --base origin/main: clean; no actionable defects found.

@brokemac79

brokemac79 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Jul 25, 2026 •
edited
Loading

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.

Re-review progress:

@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 25, 2026
@clawsweeper

clawsweeper Bot commented Jul 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 26, 2026, 11:41 AM ET / 15:41 UTC.

ClawSweeper review

What this changes

This PR adds a seven-day durable fingerprint cursor that suppresses duplicate unchanged pull-request edit reviews while preserving enqueueing for changed review-relevant state and compatible fallback deliveries.

Merge readiness

⚠️ Ready for maintainer review - 4 items remain

Close this PR as superseded by #706.

Priority: P2
Reviewed head: a9e0ecd3f78909337f391872a12971ff57d8c742
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) Strong local runtime proof and focused coverage support a useful patch; owner confirmation of the workflow-to-Worker compatibility contract remains the main merge gate.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (live_output): The PR body provides redacted after-fix local runtime output from the real Worker, signed internal routes, and SQLite-backed Durable Object, showing first edit queued, unchanged repeat deduped, changed content queued, and fallback authority success.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body provides redacted after-fix local runtime output from the real Worker, signed internal routes, and SQLite-backed Durable Object, showing first edit queued, unchanged repeat deduped, changed content queued, and fallback authority success.
Evidence reviewed 3 items linked superseding PR: #706 (draft: coalesce matching exact-review ingress) is merged at 2026-07-26T15:11:33Z.
cluster evidence: the durable review links that PR in the work cluster or recommended risk path.
no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.
Findings None None.
Security None None.

How this fits together

Exact-review ingress receives GitHub pull-request events and turns them into durable queue requests for ClawSweeper reviews. This change sits before queue dispatch, using source-authority ordering and a Durable Object cursor to prevent duplicate review revisions without suppressing meaningful edits.

flowchart TD
  A[GitHub pull request edit] --> B[Webhook classification]
  B --> C[Review-relevant tuple]
  C --> D[Source-authority reservation]
  D --> E[Durable semantic cursor]
  E -->|unchanged| F[Deduped response and metric]
  E -->|changed or incomplete| G[Exact-review queue]
  G --> H[Durable review request]
Loading

Decision needed

Question Recommendation
Should ClawSweeper adopt same-route semantic deduplication for complete pull_request.edited fallback payloads, including reserving source authority before enqueue? Accept the semantic cursor: Approve the narrow idempotency change after confirming deployed fallback callers already satisfy the authenticated source-authority contract.

Why: The implementation is a focused compatibility change at the workflow-to-Worker admission boundary; source inspection from the supplied diff supports the mechanics, but accepting that runtime contract is an owner decision rather than a mechanical repair choice.

Before merge

  • Resolve merge risk (P2) - Merging changes the effective idempotency contract for edited pull-request events: a complete fallback tuple now reserves source authority before enqueue, so the deployed workflow caller and authenticated Worker contract need owner confirmation.
  • Resolve merge risk (P2) - The PR changes one GitHub Actions fallback path and Durable Object persistence behavior; green unit coverage reduces risk but does not independently prove the production workflow configuration is aligned.
  • Complete next step (P2) - No discrete repair finding remains; an exact-review owner must decide whether the fallback source-authority admission contract is acceptable for production.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface 7 files affected; 761 additions, 40 deletions The change spans the Worker, Durable Object queue, fallback workflow, and focused regression tests, so admission compatibility deserves cross-surface review.
Fallback integration 1 workflow route extended The repository-dispatch fallback now carries semantic edit state and may reserve source authority for complete edited tuples.

Merge-risk options

Maintainer options:

  1. Confirm fallback compatibility before merge (recommended)
    Verify that the deployed repository-dispatch fallback supplies the complete edited tuple only where it can safely use the authenticated source-authority reservation, then merge with the current fail-open behavior.
  2. Retain current duplicate behavior
    Pause or close this PR if maintainers do not want the fallback workflow to participate in semantic edited-event admission.

Technical review

Best possible solution:

Close this PR as superseded by #706.

Do we have a high-confidence way to reproduce the issue?

Yes in source and local runtime proof: repeated complete pull_request.edited tuples reach the exact-review queue, where delivery-ID idempotency alone permits a new revision. The supplied Worker/Durable Object run demonstrates an unchanged repeat is deduped while changed content still queues, but this review did not independently run that scenario.

Is this the best way to solve the issue?

Yes, conditionally: a bounded fingerprint cursor at ingress is the narrowest place to stop duplicate review requests while retaining meaningful edits and fail-open handling for incomplete payloads. The remaining question is whether the fallback source-authority reservation is the accepted production contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 2ea92d0bfc12.

Labels

Label justifications:

  • P2: This is a bounded exact-review reliability and efficiency change with a meaningful but non-emergency control-plane blast radius.
  • merge-risk: 🚨 compatibility: The patch changes when existing edited pull-request fallback inputs can enqueue and reserve source authority.
  • merge-risk: 🚨 automation: The patch changes the GitHub Actions fallback payload and its authenticated Worker admission path.
  • 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 provides redacted after-fix local runtime output from the real Worker, signed internal routes, and SQLite-backed Durable Object, showing first edit queued, unchanged repeat deduped, changed content queued, and fallback authority success.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides redacted after-fix local runtime output from the real Worker, signed internal routes, and SQLite-backed Durable Object, showing first edit queued, unchanged repeat deduped, changed content queued, and fallback authority success.

Evidence

What I checked:

  • linked superseding PR: draft: coalesce matching exact-review ingress #706 (draft: coalesce matching exact-review ingress) is merged at 2026-07-26T15:11:33Z.
  • cluster evidence: the durable review links that PR in the work cluster or recommended risk path.
  • no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

  • brokemac79: Authored the merged cross-route ingress coalescing work and this PR’s follow-up same-route semantic-deduplication implementation, giving them direct history across the exact-review ingress boundary. (role: merged adjacent ingress contributor; confidence: high; commits: 770bb31d7825, a9e0ecd3f789; files: dashboard/exact-review-queue.ts, .github/workflows/sweep.yml)
  • steipete: Merged direct-publication and runtime architecture work that this PR explicitly builds on, including the Durable Object and authenticated queue interfaces used by the fallback path. (role: recent exact-review architecture contributor; confidence: medium; commits: 4288d557fff1, 2ea92d0bfc12; files: dashboard/worker.ts, dashboard/exact-review-queue.ts)

Rank-up moves

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

  • Confirm the deployed repository-dispatch fallback and authenticated source-authority contract before merge.

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.

History

Review history (1 earlier review cycle)
  • reviewed 2026-07-26T10:33:51.698Z sha d417f28 :: needs maintainer review before merge. :: none

@brokemac79
brokemac79 force-pushed the codex/csw-063-edited-semantic-dedupe branch from b5f725a to d417f28 Compare July 26, 2026 08:42
@brokemac79
brokemac79 requested a review from a team as a code owner July 26, 2026 08:42
@brokemac79

brokemac79 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 26, 2026 •
edited
Loading

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.

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. and removed 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. labels Jul 26, 2026
@brokemac79
brokemac79 force-pushed the codex/csw-063-edited-semantic-dedupe branch from d417f28 to a9e0ecd Compare July 26, 2026 15:36
@brokemac79

brokemac79 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 26, 2026 •
edited
Loading

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.

Re-review progress:

@brokemac79
brokemac79 merged commit 439f425 into main Jul 26, 2026
12 checks passed
@brokemac79
brokemac79 deleted the codex/csw-063-edited-semantic-dedupe branch July 26, 2026 15:45
@brokemac79 brokemac79 mentioned this pull request Jul 27, 2026
Merged
This was referenced Jul 28, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@brokemac79