fix: retry stalled direct chats after session reset#96512
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 3:00 PM ET / 19:00 UTC. Summary 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.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest 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 changesLabel justifications:
Evidence reviewedPR surface: Source +186, Tests +268. Total +454 across 5 files. View PR surface stats
Acceptance criteria:
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
|
|
@clawsweeper re-review Updated head 8758227 addresses the prior review findings:
Local verification on the latest head:
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. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
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.resetand 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-lockfilenode scripts/run-vitest.mjs src/auto-reply/reply/agent-runner.misc.runreplyagent.test.tsnode scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-session-reset.test.tsgit diff --checkpnpm exec oxfmt --check --threads=1 src/auto-reply/reply/agent-runner.ts src/auto-reply/reply/agent-runner.misc.runreplyagent.test.tspnpm exec oxlint --threads=1 src/auto-reply/reply/agent-runner.ts src/auto-reply/reply/agent-runner.misc.runreplyagent.test.tsnode scripts/run-tsgo.mjs -p tsconfig.core.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core.tsbuildinfonode 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 localEvidence 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.tspassed 2/2; final autoreview reportedautoreview 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.