Skip to content

fix: retry stalled direct chats after session reset#96512

Closed
849261680 wants to merge 4 commits into
openclaw:mainfrom
849261680:fix/90516-stalled-session-reset-retry
Closed

fix: retry stalled direct chats after session reset#96512
849261680 wants to merge 4 commits into
openclaw:mainfrom
849261680:fix/90516-stalled-session-reset-retry

Conversation

@849261680

@849261680 849261680 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Fixes #90516

What Problem This Solves

Fixes an issue where users in direct chat could receive a generic LLM request failed. response when a compacted hot session became wedged and the active model call was diagnostically aborted without producing visible output.

Why This Change Was Made

When a compacted direct-session run aborts with no visible payloads, no provider fallback, no pending tool work, and no committed source/stream/tool side effects, the reply runner now rotates the hot session with the existing reply-run session reset path and retries the original turn once. The retry updates the active reply operation's session id before re-entering the runner so stop, steering, and recovery controls target the fresh session. Buffered block-stream output is treated as already-visible work, so the runner does not replay a turn that has output waiting to flush.

Codex contract checked: ../codex/sdk/typescript/src/thread.ts, ../codex/sdk/typescript/src/events.ts, ../codex/sdk/typescript/tests/abort.test.ts, ../codex/codex-rs/mcp-server/src/codex_tool_runner.rs, and ../codex/codex-rs/mcp-server/src/message_processor.rs. Those paths confirm abort/error completion is surfaced through the Codex turn/tool-call stream; this PR keeps the repair in OpenClaw's reply/session layer.

User Impact

Direct chat users get the same message retried on a fresh hot session instead of having to manually run sessions.reset and resend after the wedged-session abort pattern. Existing sessions with visible output, buffered stream output, tool side effects, fallback attempts, heartbeat turns, non-direct chats, or non-compacted sessions are not auto-replayed.

Evidence

Behavior addressed: compacted direct chat run aborts without visible output and previously surfaced LLM request failed. instead of resetting and retrying once.

Real environment tested: local OpenClaw source worktree on Node/pnpm with mocked embedded-runner behavior for the exact reply-runner session-state edge.

Exact steps or command run after this patch:

  • pnpm install --frozen-lockfile
  • node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts
  • node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-session-reset.test.ts
  • git diff --check
  • pnpm exec oxfmt --check --threads=1 src/auto-reply/reply/agent-runner.ts src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts
  • pnpm exec oxlint --threads=1 src/auto-reply/reply/agent-runner.ts src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts
  • node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core.tsbuildinfo
  • node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core-test.tsbuildinfo
  • .agents/skills/autoreview/scripts/autoreview --mode local

Evidence after fix: regression test covers reset + one retry, active reply operation rebinding to the fresh session, persisted session rotation, queue refresh, and a buffered block-stream non-reset case.

Observed result after fix: focused Vitest passed with 50/50 tests in agent-runner.misc.runreplyagent.test.ts; agent-runner-session-reset.test.ts passed 2/2; final autoreview reported autoreview clean: no accepted/actionable findings reported.

What was not tested: live Telegram delivery and a live Codex provider hang were not exercised; the proof uses the existing mocked embedded runner to deterministically model the stalled-abort state from #90516 without requiring a real wedged provider session.

@clawsweeper

clawsweeper Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 3:00 PM ET / 19:00 UTC.

Summary
The PR adds a guarded one-shot hot-session reset and retry path for compacted direct-chat aborted runs, plus pending block-delivery checks and focused regression tests.

PR surface: Source +186, Tests +268. Total +454 across 5 files.

Reproducibility: yes. at source level: the linked installed-gateway report plus current main show the blocked aborted-run path can surface the generic failure without a same-turn reset/retry, but I did not replay the live Telegram incident.

Review metrics: 1 noteworthy metric.

  • Automatic replay paths: 1 added. The PR adds a same-turn retry after hot session rotation, so duplicate-delivery and active-session ownership semantics matter before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #90516
Summary: PR 96512 is the current candidate fix for the canonical stalled direct-chat hot-session reset/retry issue.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦞 diamond lobster
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Add redacted live Telegram or equivalent provider recovery proof showing one recovered reply and no duplicate failure text.
  • [P1] Get maintainer acceptance for the one-shot session reset/retry and repair-record semantics.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body and comments list local mocked embedded-runner tests and explicitly say live Telegram delivery and live Codex provider recovery were not exercised. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A live Telegram transcript would materially prove that reset/retry delivers one recovered reply instead of a generic failure or duplicate message. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram live: verify the compacted direct chat stalled-abort path resets once, retries the user turn, and delivers one recovered reply without duplicate failure text.

Risk before merge

  • [P1] Automatic reset/retry rotates hot session state and rebinds the active reply operation during an in-flight user turn, so maintainers need to accept that contract.
  • [P1] If the delivery guard misses a real transport side effect, replaying the same user turn can duplicate or misrepresent a visible reply.
  • [P1] Contributor proof is still mocked embedded-runner proof; live Telegram or equivalent provider recovery proof is missing for the behavior users will experience.

Maintainer options:

  1. Require live recovery proof before merge (recommended)
    Add redacted Telegram or equivalent live provider proof that the stalled-abort path resets once, retries the same turn, and delivers one recovered reply without duplicate failure text.
  2. Accept the replay contract explicitly
    Maintainers can accept the automatic replay and session-rotation semantics after proof, including the diagnostic record shape for retry success or failure.
  3. Pause until proof is available
    Keep the PR open but unmerged if no live transport or live provider scenario can demonstrate the user-visible recovery path.

Next step before merge

  • [P1] The remaining blocker is human review of live recovery proof and replay semantics, not a concrete automatable code repair.

Security
Cleared: The diff changes TypeScript reply-runner logic and tests only; no dependency, workflow, secret-handling, permission, or supply-chain surface was added.

Review details

Best possible solution:

Land only after live Telegram or equivalent provider proof shows one recovered reply without duplicate failure text and maintainers accept the one-shot replay/session-rotation semantics.

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

Yes at source level: the linked installed-gateway report plus current main show the blocked aborted-run path can surface the generic failure without a same-turn reset/retry, but I did not replay the live Telegram incident.

Is this the best way to solve the issue?

Yes, conditionally: the reply runner is the right layer because it has the original turn, session entry, delivery evidence, and reply operation, but merge still needs live proof and maintainer acceptance of replay semantics.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a real user-facing Telegram direct-chat workflow where users can receive repeated generic LLM failures until manual session reset.
  • merge-risk: 🚨 message-delivery: The reset/retry path replays the same user turn and could duplicate or misrepresent visible replies if real delivery side effects are missed.
  • merge-risk: 🚨 session-state: The diff rotates hot session state and rebinds the active reply operation during an in-flight reply path.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦞 diamond lobster.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body and comments list local mocked embedded-runner tests and explicitly say live Telegram delivery and live Codex provider recovery were not exercised. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes user-visible Telegram direct-chat behavior from a generic failure to a recovered reply, so a short Telegram proof run would materially help review.
Evidence reviewed

PR surface:

Source +186, Tests +268. Total +454 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 2 191 5 +186
Tests 3 268 0 +268
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 459 5 +454

Acceptance criteria:

  • [P1] Redacted live Telegram or equivalent provider proof shows one recovered reply and no duplicate failure text.
  • [P1] Maintainer accepts the session reset/retry diagnostic and repair-record semantics.
  • [P1] Focused regression tests for reset, retry, queued followup refresh, and pending block delivery remain green.

What I checked:

  • Live PR state: PR 96512 is open, cleanly mergeable, and still labeled for proof plus message-delivery and session-state merge risk at head 91be47a. (91be47aac8b5)
  • Linked issue evidence: Issue 90516 reports an installed Telegram direct-chat session where a compacted hot session aborted with no content, delivered LLM request failed, and recovered after manual sessions.reset.
  • Current main behavior: Current main runs runAgentTurnWithFallback and proceeds directly to final/payload handling without a hot-session reset/retry branch after the first run outcome. (src/auto-reply/reply/agent-runner.ts:1723, b6bc3ed0dbd8)
  • Latest release behavior: Tag v2026.6.10 contains the same direct run/final flow and does not include this PR's reset/retry gate. (src/auto-reply/reply/agent-runner.ts:1715, aa69b12d0086)
  • PR reset gate: The PR gates auto replay on direct chat, compaction history, blocked aborted run state, no fallback attempts, no pending tools, no non-error payloads, and no committed side-effect delivery. (src/auto-reply/reply/agent-runner.ts:345, 91be47aac8b5)
  • Pending delivery guard: The PR flushes pending block delivery before deciding that the original turn had no visible delivery, reducing duplicate-message risk from in-flight sends. (src/auto-reply/reply/agent-runner.ts:1864, 91be47aac8b5)

Likely related people:

  • vincentkoc: Recent squash commits in the same reply-runner and block-streaming surfaces list this handle as reviewer/co-author, and those paths share the delivery and replay invariants reviewed here. (role: recent reviewer and adjacent owner; confidence: high; commits: e998986889ec, 9549545dd0a0; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/block-reply-pipeline.ts, src/auto-reply/reply/agent-runner-execution.ts)
  • moeedahmed: Authored the recent drain/restart-abort reply path change in the same runner and execution surfaces. (role: adjacent abort/recovery contributor; confidence: medium; commits: e998986889ec; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/agent-runner-execution.ts)
  • yetval: Authored the recent multi-message block-streaming suppression change in the block pipeline affected by this PR's pending-delivery guard. (role: recent block-streaming contributor; confidence: medium; commits: 9549545dd0a0; files: src/auto-reply/reply/block-reply-pipeline.ts)
  • jalehman: Recent merged work added lifecycle storage seams and Codex reply execution changes that are adjacent to this retry path's session rotation and active operation rebinding. (role: recent session seam contributor; confidence: high; commits: 49e6f5a524bc, b6bc3ed0dbd8; files: src/auto-reply/reply/agent-runner-session-reset.ts, src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/agent-runner-execution.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.

@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. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 24, 2026
@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. labels Jun 24, 2026
@849261680

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated head 8758227 addresses the prior review findings:

  • Pending block output is now treated as visible delivery evidence before reset/retry, with a buffered block-output regression test.
  • The added test lint errors are fixed: reset lifecycle write is awaited and synchronous session-store load no longer uses await.
  • Hosted checks on 8758227 currently show Real behavior proof success, check-lint success, check-test-types success, and Critical Quality agent-runtime-boundary success.

Local verification on the latest head:

  • pnpm exec oxlint --threads=1 --type-aware src/auto-reply/reply/agent-runner.ts src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts
  • node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts -t "stalled compacted direct session|buffered block output"
  • node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core-test.tsbuildinfo
  • git diff --check

I attempted to dispatch the Mantis Telegram Desktop Proof workflow for this PR, but GitHub rejected workflow_dispatch with HTTP 403 requiring admin rights. Maintainer-side Mantis desktop proof is still the remaining path if you require live Telegram visual proof before merge.

@clawsweeper

clawsweeper Bot commented Jun 24, 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.

@clawsweeper

clawsweeper Bot commented Jun 24, 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.

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

Labels

mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Stalled direct chat model call surfaces LLM request failed instead of auto-resetting/retrying wedged session

1 participant