fix(queue): keep the correct follow-up review after publication - #1324
Conversation
Follow up on #1251 by reading direct-requeue authority from the saved lease decision. Preserve accepted and deduplicated receipt ownership, reject superseded completion authority, and keep newer command decisions intact. The original lost-completion fix is already upstream; retain its existing contributor credit.
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 automation
|
Codex review: needs maintainer review before merge. Reviewed August 31, 2026, 12:57 PM ET / 16:57 UTC. ClawSweeper reviewWhat this changesThe PR makes follow-up review requeues depend on the saved lease’s accepted or deduplicated direct-publication receipt, with regression tests, operational documentation, and an inspectable proof harness. Merge readinessThis PR remains necessary: current main predates its saved-lease authority fix, and the patch has no actionable correctness finding. Its supplied real workerd proof directly exercises the changed completion and reconciliation paths with the expected before/after results. Priority: P2 Review scores
Verification
How this fits togetherThe exact-review queue accepts publication results, keeps a lease for the original review, and later completes or reconciles that lease. This change decides whether that recovery creates one owed source-drift follow-up while preserving newer commands and the observer-only Bay status projection. flowchart LR
A[Artifact publication] --> B[Saved lease receipt]
B --> C[Completion callback]
B --> D[Lost-callback reconciliation]
C --> E{Accepted requeue owed?}
D --> E
E -->|Yes| F[One follow-up review]
E -->|No| G[Finish lease]
F --> H[Observer-only status projection]
G --> H
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Merge the focused saved-lease authority repair after the current review and check gates complete, then observe early production reconciliation and follow-up-review telemetry. Do we have a high-confidence way to reproduce the issue? Yes. The supplied exact-head AWS Crabbox trace exercises the signed Worker completion and reconciliation paths against workerd/SQLite and shows the baseline and candidate outcomes for the authority controls. Is this the best way to solve the issue? Yes. Reading authority and the producer decision from the immutable saved lease is the narrowest solution because it prevents mutable current queue metadata from adding, removing, or redirecting the old lease’s requeue obligation. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 0baa4dbb4d12. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
|
Merged with maintainer approval as aba9826. Validation applies to head CI, CodeQL, and the current ClawSweeper review completed; proof was accepted as sufficient, with no actionable finding or contributor-facing blocker. The explicit skipped checks were manual-only, bot-label exclusions or the inactive CodeSmith app—not counted as passes. The bounded queue-automation rollout risk is accepted for this landing. The main body and inspectable repro source retain the proof's limits: no deployed-edge, real GitHub run-lookup, full hosted-workflow or production-load claim. The existing queue/Bay telemetry remains the rollout observation surface; no synthetic production mutation is part of this verification. |
1 participant
Related: issue 1245
Follow-up to PR 1251
What Problem This Solves
Resolves a queue ownership problem where a follow-up review could be discarded or scheduled without an authoritative requeue receipt when the current publication decision and the saved publishing lease disagreed. A superseded receipt could also accept a direct requeue completion.
The baseline already contains the original lost-completion recovery and workflow failure classification from PR 1251. This PR tightens the remaining receipt-authority boundary; it does not claim that earlier fix as new work. The inconsistent-state cases below are controlled synthetic regressions, not claims of observed production incidents.
Why This Change Was Made
Completion and terminal-run reconciliation now share one interpretation of the saved lease's direct lifecycle. Only its accepted/deduplicated
requeuereceipt establishes an owed follow-up. The existing requeue path also reads the producer decision and old target from that same lease rather than mutable current metadata.There is no new recovery layer, endpoint, schema, retry policy, or model inference. The workflow implementation and failure gate are unchanged. Current-main terminal-outcome, error-correlation, and telemetry changes are preserved.
User Impact
An already-owed follow-up remains schedulable exactly once. Unsupported or superseded metadata cannot invent that obligation, later commands retain their prompt/marker/revision, and repeated or late callbacks do not create another review. Ordinary completion, failed/cancelled publication retry, parking, and incomplete-source behavior remain unchanged.
OpenClaw Bay Impact
No Bay schema, UI, or mutation control is added. The runtime proof exercises the existing public observer projection and SQLite Bay tables: the old fenced
requeuefact is retained without invented review success/failure, router, or acknowledgement receipts. The newer command's lifecycle is not overwritten.Documentation Impact
Active scheduler and state-storage documentation now state the saved-lease authority contract. The changelog records the follow-up and retains credit for the original contributor work.
The new public repro methodology and source is classified as proposed proof methodology, with its owner, source of truth, pinned baseline, requirements, and update triggers. It contains executable source, not raw logs, archives, credentials, or agent transcripts. Actual execution observations are recorded below.
Evidence
On the exact committed head:
pnpm run build:alland six focused suites: 445 passed, zero failed/skipped.pnpm run check: 4,187 passed, zero failed, eight skipped, plus 13/13 changed-coverage tests. Coverage: 82.83% lines, 75.80% branches, 88.06% functions.Real Behavior Proof
Claim and exercised surface: real workerd/SQLite Durable Objects, signed public Worker forwarding, publication receipts, terminal disposition, completion/reconciliation, and the existing public Bay read model. The realistic sequence records terminal
requeuebefore deliberately omitting/complete.Source identity:
659dee73d0706fee9776f7986d9861e82b897d24.26cd5760da7e4d49465f86857b788ec022569f5d.2f517572276a5a1857f832af3abec5cdc3029070; tracked working-tree changes:[].f1873d20690bbc76f53257949caf6f55301f47279a844c2e416a0ed637fafd07.3abab5ad0b52d366438f91b09de107eeb49f8acbaa2ae170737f144bee888906; source/index identity was reverified after validation.Environment and execution: AWS Crabbox, Linux,
c7a.8xlarge, eu-west-1, imageami-0461d919be7deb53c; leasecbx_b9877bf89798, runrun_1b578feaa825, 2026-08-31 16:37–16:40 UTC, overall exit 0. Node 24.20.0, pnpm 11.10.0, Wrangler 4.107.0. No Actions hydration or ambient credential forwarding; the IMDS IAM credential endpoint returned 404. Only a scanned explicit payload and one exact public source commit were used, not local Git history or unrelated artifacts. Authenticated portal links are not required to inspect this proof.Command (temporary directory prefixes normalized; the public driver contains the exact request sequence and assertions):
SAVED_LEASE_SOURCE_DIR="$PWD" SAVED_LEASE_OUTPUT=/tmp/saved-lease-results SAVED_LEASE_WRANGLER=/path/to/wrangler SAVED_LEASE_BASELINE_ARCHIVE=/tmp/saved-lease-baseline.tar bash docs/proof/saved-lease-authority/run-proof.shThe README gives the exact baseline archive command and requirements. Worker harness, scenario driver, and workflow annotation driver are inspectable at the executed commit.
Observed results: all 17 baseline + 17 candidate scenarios satisfied their explicit before/after expectations. The four distinguishing controls produced:
requeued: true, terminal rewritten torequeueinvalid_direct_lifecycle_requeue, terminal remainssupersededAccepted/deduplicated lost-callback controls already recover on the baseline and continue to recover on the candidate. Their observed reconciliation response is:
{"ok":true,"reconciled":1,"requeued":1,"completed":0}The resulting queue contains exactly one pending revision 5, with its lease cleared and source action
source_drift_requeue; the old revision 4 retains terminalrequeue. Delivered-completion controls reach the same pending state. Repeated reconciliation returns zero reconciled/requeued/completed, and late old completion returns 409. Newer commands retain their exact decision/marker/prompt/revision. Wrong attempt/generation, ambiguous ownership, unsigned requests, failed/cancelled retries, receipt collections, counters, and Bay projections are asserted. Worker outbound attempts and unexpected alarms remain 0.The unchanged workflow gate also passed 192 feasible Boolean contexts and 12 actual shell executions covering completion-only failure, typed deferral, exit-zero content failure, simultaneous failure, and review-only failure.
Limits: admission and malformed/canonical preconditions are seeded; terminal-run observations are supplied through the signed endpoint instead of real GitHub run lookup. Dispatch is suppressed only after pending-state inspection, and outbound Worker fetch is denied and counted. This proves the changed local runtime/persistence/forwarding boundary—not deployed-edge behavior, Actions admission, a complete hosted workflow, production load, or model inference. No live queue or GitHub mutation is performed by the repro.