fix(commitments): preserve extraction batch on transient failure#89817
Merged
vincentkoc merged 1 commit intoJun 29, 2026
Merged
Conversation
Contributor
|
Codex review: needs maintainer review before merge. Reviewed June 28, 2026, 9:05 PM ET / 01:05 UTC. Summary PR surface: Source +22, Tests +309. Total +331 across 2 files. Reproducibility: yes. Current main has a source-reproducible path where drainCommitmentExtractionQueue removes a batch before extractor execution and rethrows a non-terminal error before restoration or persistence. Review metrics: none identified. 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:
Next step before merge
Security Review detailsBest possible solution: Land the narrow restore-and-rearm fix after maintainer review and green required checks, while leaving durable queue persistence as a separate product decision. Do we have a high-confidence way to reproduce the issue? Yes. Current main has a source-reproducible path where drainCommitmentExtractionQueue removes a batch before extractor execution and rethrows a non-terminal error before restoration or persistence. Is this the best way to solve the issue? Yes. Restoring the original batch at the extraction failure boundary and reusing the existing single-slot debounce is the narrowest maintainable fix; durable retry storage would be a larger follow-up design. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 081f8fd1fa68. Label changesLabel justifications:
Evidence reviewedPR surface: Source +22, Tests +309. Total +331 across 2 files. View PR surface stats
What 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
|
masatohoshino
force-pushed
the
fix/commitment-extraction-batch-loss
branch
from
June 20, 2026 06:31
d6360b1 to
993d49a
Compare
A drained extraction batch is spliced off the queue before the extractor runs. On a non-terminal failure the batch was never restored, so those items were silently lost and never retried. Restore the batch to the front of the queue (original order) on non-terminal errors and rethrow so the caller still logs; terminal model/auth errors keep the existing cooldown drop/stop behavior. Also ensure the single-slot debounce schedules a drain from the overflow branch, so a queue left full by a restored batch still gets retried instead of being stuck. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vincentkoc
force-pushed
the
fix/commitment-extraction-batch-loss
branch
from
June 29, 2026 01:01
993d49a to
183579b
Compare
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Jun 29, 2026
…nclaw#89817) A drained extraction batch is spliced off the queue before the extractor runs. On a non-terminal failure the batch was never restored, so those items were silently lost and never retried. Restore the batch to the front of the queue (original order) on non-terminal errors and rethrow so the caller still logs; terminal model/auth errors keep the existing cooldown drop/stop behavior. Also ensure the single-slot debounce schedules a drain from the overflow branch, so a queue left full by a restored batch still gets retried instead of being stuck. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
QiuYuang
pushed a commit
to QiuYuang/openclaw
that referenced
this pull request
Jul 1, 2026
…nclaw#89817) A drained extraction batch is spliced off the queue before the extractor runs. On a non-terminal failure the batch was never restored, so those items were silently lost and never retried. Restore the batch to the front of the queue (original order) on non-terminal errors and rethrow so the caller still logs; terminal model/auth errors keep the existing cooldown drop/stop behavior. Also ensure the single-slot debounce schedules a drain from the overflow branch, so a queue left full by a restored batch still gets retried instead of being stuck. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
chenyangjun-xy
pushed a commit
to chenyangjun-xy/openclaw
that referenced
this pull request
Jul 1, 2026
…nclaw#89817) A drained extraction batch is spliced off the queue before the extractor runs. On a non-terminal failure the batch was never restored, so those items were silently lost and never retried. Restore the batch to the front of the queue (original order) on non-terminal errors and rethrow so the caller still logs; terminal model/auth errors keep the existing cooldown drop/stop behavior. Also ensure the single-slot debounce schedules a drain from the overflow branch, so a queue left full by a restored batch still gets retried instead of being stuck. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
chenyangjun-xy
pushed a commit
to chenyangjun-xy/openclaw
that referenced
this pull request
Jul 3, 2026
…nclaw#89817) A drained extraction batch is spliced off the queue before the extractor runs. On a non-terminal failure the batch was never restored, so those items were silently lost and never retried. Restore the batch to the front of the queue (original order) on non-terminal errors and rethrow so the caller still logs; terminal model/auth errors keep the existing cooldown drop/stop behavior. Also ensure the single-slot debounce schedules a drain from the overflow branch, so a queue left full by a restored batch still gets retried instead of being stuck. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Rorqualx
pushed a commit
to Rorqualx/cortex
that referenced
this pull request
Jul 4, 2026
…nclaw#89817) A drained extraction batch is spliced off the queue before the extractor runs. On a non-terminal failure the batch was never restored, so those items were silently lost and never retried. Restore the batch to the front of the queue (original order) on non-terminal errors and rethrow so the caller still logs; terminal model/auth errors keep the existing cooldown drop/stop behavior. Also ensure the single-slot debounce schedules a drain from the overflow branch, so a queue left full by a restored batch still gets retried instead of being stuck. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit be0c40b)
Rorqualx
pushed a commit
to Rorqualx/cortex
that referenced
this pull request
Jul 8, 2026
…nclaw#89817) A drained extraction batch is spliced off the queue before the extractor runs. On a non-terminal failure the batch was never restored, so those items were silently lost and never retried. Restore the batch to the front of the queue (original order) on non-terminal errors and rethrow so the caller still logs; terminal model/auth errors keep the existing cooldown drop/stop behavior. Also ensure the single-slot debounce schedules a drain from the overflow branch, so a queue left full by a restored batch still gets retried instead of being stuck. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit be0c40b)
wheakerd
pushed a commit
to wheakerd/clawdbot
that referenced
this pull request
Jul 15, 2026
…nclaw#89817) A drained extraction batch is spliced off the queue before the extractor runs. On a non-terminal failure the batch was never restored, so those items were silently lost and never retried. Restore the batch to the front of the queue (original order) on non-terminal errors and rethrow so the caller still logs; terminal model/auth errors keep the existing cooldown drop/stop behavior. Also ensure the single-slot debounce schedules a drain from the overflow branch, so a queue left full by a restored batch still gets retried instead of being stuck. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit be0c40b)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Problem This Solves
drainCommitmentExtractionQueuetakes a batch off the queue withqueue.splice(0, batchMaxItems)and runs the extractor. On a non-terminal extractor error (a transient model or network error), thecatchonly re-threw — the already-dequeued batch was never put back, so those items were silently lost and never retried.A follow-up review found the first fix was incomplete: a drain started by the debounce timer clears the pending timer before running, so the restore-only fix left the recovered batch queued with no scheduled drain. It would then not retry until some later enqueue happened to reschedule one, and would be lost if the process exited first.
Why This Change Was Made
queue.unshift(...batch)) and the error is rethrown, so the existing caller still logs it.scheduleDrainSoon), so a timer-fired transient failure retries on its own without depending on a later enqueue.isTerminalExtractionError) are unchanged: they still cool down and drop the agent's queued items viaopenTerminalFailureCooldown.scheduleDrainSoon(debounceMs)helper and reused by enqueue, the overflow branch, and the restore path — mechanical reuse, not new scheduling behavior. Theif (timer) returnguard keeps at most one pending drain timer, so retries fire at the existing debounce cadence (default 15s), not in a loop.User Impact
Inferred follow-up commitments are no longer silently dropped when extraction hits a transient error; the batch is retried and persisted instead of lost. No config, API, or channel surface changes; terminal auth or model failures behave exactly as before.
Evidence
pnpm test src/commitments/runtime.test.ts— focused regression suite, 12 tests passing. Highest-value cases:restores and reprocesses a batch after a non-terminal extractor failure(restore plus retry, no duplicate persistence);restores a failed batch to the front, preserving order across batches(ordering);re-arms the drain after a timer-fired non-terminal failure with no later enqueue(the completeness fix; fails if the re-arm is removed).Terminal behavior is held unchanged by
keeps the existing drop/stop behavior on terminal extraction failures. Typecheck (pnpm tsgo:core,pnpm tsgo:core:test),oxfmt, andoxlintpass on the touched files.Production change is limited to
src/commitments/runtime.ts; the rest is tests insrc/commitments/runtime.test.ts. A local, uncommitted Node script additionally drove the production queue, persistence, dedupe, and cooldown paths (only theextractBatchcall injected to throw) and confirmed: the transient failure retained the batch with nothing persisted, the retry reprocessed exactly the two items once, a third drain did nothing, and a terminalNo API key foundfailure dropped and did not retry. No live channel or provider run was performed.Scope
This change is limited to the non-terminal extractor-failure path in
drainCommitmentExtractionQueue(src/commitments/runtime.ts) and its colocated test. NoCHANGELOG.mdedit: this is an internal queue bug fix and release notes are generated at release time.