fix(auto-reply): clear pending-final state before honoring post-send abort (#89115) by amknight · Pull Request #93201 · openclaw/openclaw

amknight · 2026-06-15T07:14:07Z

Summary

Fixes #89115 — agent "goes silent" after a delivered final reply.

In dispatchReplyFromConfig, the user-message success branch called
throwIfDispatchOperationAborted() before clearPendingFinalDeliveryAfterSuccess():

if (attemptedFinalDelivery && !finalDeliveryFailed) {
  throwIfDispatchOperationAborted();          // ← aborts here…
  await clearPendingFinalDeliveryAfterSuccess({ ... });  // …so this never runs
}

If stuck-session recovery aborts the embedded run in the narrow window between
the final reply shipping and the clear, the message is delivered but
pendingFinalDelivery stays true forever. The get-reply redelivery
short-circuit then keys off that flag and silently blocks every future inbound —
the agent appears to go silent until the session is manually reset.

The fix

Reorder — clear the durable pending-final bookkeeping first, then honor
the abort. The reply already shipped, so the clear must run; the abort is still
surfaced afterwards (the dispatch routes through finishReplyOperationAbortedDispatch()
and reports queuedFinal: false), so cancellation reporting is preserved.
Completeness — also clear the stranded pendingFinalDeliveryIntentId
field. agent-command.ts already clears it on its path; the success helper
did not, leaving stale state behind.

This is intentionally narrow: it does not change delivery-success detection,
add idle-wait calls, or touch the dispatcher outcome counts.

Why this supersedes #90229 and #89130

Both prior PRs had the right instinct; this one keeps the correct parts and drops
the risk:

fix(auto-reply): clear delivered pending finals #89130 deleted the abort check instead of moving it after the clear,
dropping the post-delivery cancellation reporting the codebase expects; it also
missed pendingFinalDeliveryIntentId and its test was mock-only.
fix(auto-reply): clear delivered pending final before abort #90229 had the correct reorder + the intentId cleanup, but bundled
unrelated changes onto a P1 hot path (waitForReplyDispatcherIdle, a
completedFinalDeliveryAttempt/final-outcome-counts rework whose synchronous
failed-count check is unreliable for async dispatchers) and had drifted from
main (the sendFinalPayload transcript-mirror helpers were renamed), so it
no longer applied cleanly.

Real behavior proof

Behavior or issue addressed: Race in dispatch-from-config: abort check before clearPendingFinalDeliveryAfterSuccess() leaves pendingFinalDelivery stuck after successful delivery #89115 — delivered final reply leaves
pendingFinalDelivery: true, silently blocking subsequent inbound messages.
Real environment tested: Not run against a live gateway in this change set.
Exact steps or command run after this patch:
node scripts/run-vitest.mjs src/auto-reply/reply/dispatch-from-config.reply-dispatch.test.ts
Evidence after fix: New regression test
clears pending final delivery when abort fires after a successful final send (#89115)
drives the real dispatchReplyFromConfig control flow (abort fired from inside
the dispatcher's sendFinalReply, in the post-send window). Result: the clear
runs, every pendingFinalDelivery* field (including pendingFinalDeliveryIntentId)
is cleared, and the run is still surfaced as aborted (queuedFinal: false).
Observed result after fix: 7/7 in the file pass; full reply-dispatch suite,
sessions.test.ts (44) and agent-command.live-model-switch.test.ts (62) pass.
Before evidence: Reverting only the source change makes the new test fail —
updateSessionStoreEntry is called 0 times (the abort threw before the
clear), i.e. the pending state is stranded exactly as reported.
What was not tested: A full live OpenClaw gateway repro with a real channel
and real stuck-session recovery timing.
Proof limitations or environment constraints: Validation here is the
deterministic regression test (fails on main, passes with the fix) plus
typecheck/lint/format; no live multi-process gateway run was performed in this
environment.

Tests and validation

Commands run:

node scripts/run-vitest.mjs src/auto-reply/reply/dispatch-from-config.reply-dispatch.test.ts — 7/7
node scripts/run-vitest.mjs src/config/sessions/sessions.test.ts src/agents/agent-command.live-model-switch.test.ts — 44 + 62
pnpm tsgo:core and pnpm tsgo:test:src — clean
oxfmt --check + oxlint on the changed files — clean

Regression coverage added: a test that fails before the fix (clear skipped,
state stranded) and passes after.

Risk checklist

Did user-visible behavior change? Yes — a delivered-then-aborted final no
longer strands the session; subsequent inbound messages are processed.
Did config, environment, or migration behavior change? No.

clawsweeper · 2026-06-15T07:15:34Z

Codex review: needs maintainer review before merge. Reviewed June 15, 2026, 3:39 AM ET / 07:39 UTC.

Summary
The PR clears pending-final delivery bookkeeping before honoring a late post-send abort and adds focused regression coverage for the delivered-final race.

PR surface: Source +5, Tests +50. Total +55 across 2 files.

Reproducibility: yes. at source level: current main can send a final reply and then check the abort signal before clearing pending-final state. I did not run a live gateway timing repro in this read-only review.

Review metrics: 1 noteworthy metric.

Pending-final cleanup field: 1 added. The PR adds pendingFinalDeliveryIntentId to the durable cleanup set, which matters because stale retry metadata can otherwise survive successful delivery.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
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:

none.

Risk before merge

[P1] This changes durable pending-final cleanup and post-send abort ordering on a hot auto-reply path, so a regression could strand session state or change final-delivery cancellation reporting.
[P1] The PR body reports deterministic regression and static validation, but no live external channel timing proof; maintainers must decide whether source-level proof is enough for this race.

Maintainer options:

Land With Focused Gates (recommended)
If the draft is maintainer-ready, merge after focused reply-dispatch/session validation and required checks pass for the current head while accepting the residual no-live-transport timing risk.
Request Live Timing Proof
Ask for redacted live gateway or disposable-channel evidence that a delivered final followed by a late abort clears pending-final fields before merge.
Keep Draft Until Owner Review
Leave the PR open in draft if maintainers want an owner to explicitly review the hot-path state and message-delivery tradeoff first.

Next step before merge

[P2] No repair lane is needed; the remaining action is maintainer draft readiness and merge-risk acceptance for a hot-path state/message-delivery change.

Security
Cleared: The diff only changes auto-reply runtime ordering and a colocated test; it adds no dependencies, workflows, permissions, downloaded code, or secret handling.

Review details

Best possible solution:

Land the narrow cleanup-before-abort fix once the draft is maintainer-ready and focused dispatch/session gates are green; keep the linked bug open until the PR merges.

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

Yes, at source level: current main can send a final reply and then check the abort signal before clearing pending-final state. I did not run a live gateway timing repro in this read-only review.

Is this the best way to solve the issue?

Yes; clearing durable pending-final state after accepted final delivery and then surfacing the abort is the narrow owner-path fix. Earlier closed branches were either less complete or bundled broader dispatcher changes.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 7a7165ad22c4.

Label changes

Label justifications:

P1: The PR fixes a user-visible race that can leave an agent session stuck after a delivered final reply and block later inbound messages.
merge-risk: 🚨 session-state: The diff changes when durable pendingFinalDelivery fields are cleared after final delivery.
merge-risk: 🚨 message-delivery: The diff changes final-dispatch abort timing and reporting on the auto-reply delivery path.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This is a member-authored maintainer PR, so the external contributor real-behavior proof gate does not apply; the body still provides deterministic after-fix regression output but no live gateway proof.

Evidence reviewed

PR surface:

Source +5, Tests +50. Total +55 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	6	1	+5
Tests	1	50	0	+50
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	56	1	+55

What I checked:

Repository policy read: Root AGENTS.md was read fully and applied; no scoped AGENTS.md exists under src/auto-reply, and src/agents/AGENTS.md was read because the review checked sibling agent-command cleanup behavior. (AGENTS.md:1, 7a7165ad22c4)
Live PR state: Live GitHub data shows this PR is open, draft, mergeable clean, member-authored by amknight, and labeled maintainer, so conservative cleanup must keep it open for explicit maintainer handling. (14bdcd522e11)
Current main still has the race window: Current main sends the final payload, records delivery success, then checks throwIfDispatchOperationAborted() before calling clearPendingFinalDeliveryAfterSuccess(), so a late abort can skip cleanup after a successful final send. (src/auto-reply/reply/dispatch-from-config.ts:3299, 7a7165ad22c4)
PR head reorders cleanup before abort: The PR head clears pending-final state first and then calls throwIfDispatchOperationAborted(), preserving late-abort reporting after durable cleanup has run. (src/auto-reply/reply/dispatch-from-config.ts:3300, 14bdcd522e11)
Intent cleanup aligns with sibling path: Current agent-command.ts already clears pendingFinalDeliveryIntentId with the other pending-final fields, and the PR adds the same field to the dispatch success cleanup helper. (src/agents/agent-command.ts:413, 7a7165ad22c4)
Abort result path is preserved: DispatchReplyOperationAbortedError is still caught and returned through finishReplyOperationAbortedDispatch(), which reports queuedFinal: false with dispatcher counts rather than swallowing the abort. (src/auto-reply/reply/dispatch-from-config.ts:1972, 7a7165ad22c4)

Likely related people:

MertBasar0: GitHub history for the dispatch regression tests shows commit c240e71 introduced the main-session durable delivery work that created the pending-final behavior being repaired. (role: original durable-delivery feature contributor; confidence: high; commits: c240e718e91e; files: src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/dispatch-from-config.reply-dispatch.test.ts)
steipete: Recent history shows repeated maintenance in the auto-reply dispatch tests, session helpers, and adjacent agent/session surfaces relevant to this fix. (role: recent area contributor; confidence: high; commits: 797bcd5bdb28, ea6d3a35ff01, cabb55380f84; files: src/auto-reply/reply/dispatch-from-config.reply-dispatch.test.ts, src/agents/agent-command.ts, src/logging/diagnostic-stuck-session-recovery.runtime.ts)
sandieman2: Commit c67dc59 recently changed final-delivery and queued-follow-up behavior in the same dispatch module and test file. (role: recent dispatch/message-delivery contributor; confidence: medium; commits: c67dc59b02b0; files: src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/dispatch-from-config.reply-dispatch.test.ts)
openperf: Recent stuck-session recovery commits touch the diagnostic abort source that makes this post-send abort window reachable. (role: adjacent stuck-recovery contributor; confidence: medium; commits: f194be19e261, 6f76d9f246a3; files: src/logging/diagnostic-stuck-session-recovery.runtime.ts)
Shakker: Local blame attributes the current abort-before-clear lines and helper shape in this checkout to commit a1b7f35, but broader GitHub history is a stronger routing signal than this boundary blame. (role: current-line author signal; confidence: low; commits: a1b7f3570ce3; files: src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/dispatch-from-config.reply-dispatch.test.ts)

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.

…abort (#89115) In dispatchReplyFromConfig the user-message success branch ran throwIfDispatchOperationAborted() *before* clearPendingFinalDeliveryAfterSuccess(). If stuck-session recovery aborted the run in the window between the final reply shipping and the clear, the message was delivered but pendingFinalDelivery stayed true forever — the get-reply redelivery short-circuit then silently blocked every future inbound and the agent "went silent" (#89115). Reorder so the durable pending-final bookkeeping is cleared first, then honor the abort afterwards (preserving abort reporting). Also clear the stranded pendingFinalDeliveryIntentId field — agent-command.ts already clears it but the success helper did not. Adds a regression test that fails on main (clear is skipped, state stranded) and passes with the fix. Supersedes #90229 and #89130.

…abort (openclaw#89115) (openclaw#93201) In dispatchReplyFromConfig the user-message success branch ran throwIfDispatchOperationAborted() *before* clearPendingFinalDeliveryAfterSuccess(). If stuck-session recovery aborted the run in the window between the final reply shipping and the clear, the message was delivered but pendingFinalDelivery stayed true forever — the get-reply redelivery short-circuit then silently blocked every future inbound and the agent "went silent" (openclaw#89115). Reorder so the durable pending-final bookkeeping is cleared first, then honor the abort afterwards (preserving abort reporting). Also clear the stranded pendingFinalDeliveryIntentId field — agent-command.ts already clears it but the success helper did not.

amknight mentioned this pull request Jun 15, 2026

fix(auto-reply): clear delivered pending final before abort #90229

Closed

openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels Jun 15, 2026

amknight force-pushed the ak/issue-89115-pending-final-abort-race branch from 31faa7f to 14bdcd5 Compare June 15, 2026 07:30

clawsweeper Bot mentioned this pull request Jun 15, 2026

Race in dispatch-from-config: abort check before clearPendingFinalDeliveryAfterSuccess() leaves pendingFinalDelivery stuck after successful delivery #89115

Closed

amknight marked this pull request as ready for review June 15, 2026 09:45

amknight merged commit eab22a9 into main Jun 15, 2026
199 of 200 checks passed

amknight deleted the ak/issue-89115-pending-final-abort-race branch June 15, 2026 09:46

clawsweeper Bot mentioned this pull request Jun 18, 2026

fix: dedupe duplicate non-streaming final replies #94294

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(auto-reply): clear pending-final state before honoring post-send abort (#89115)#93201

fix(auto-reply): clear pending-final state before honoring post-send abort (#89115)#93201
amknight merged 1 commit into
mainfrom
ak/issue-89115-pending-final-abort-race

amknight commented Jun 15, 2026

Uh oh!

clawsweeper Bot commented Jun 15, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

amknight commented Jun 15, 2026

Summary

The fix

Why this supersedes #90229 and #89130

Real behavior proof

Tests and validation

Risk checklist

Uh oh!

clawsweeper Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jun 15, 2026 •

edited

Loading