Skip to content

fix(reply): retry stalled compacted direct sessions after reset#97099

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

fix(reply): retry stalled compacted direct sessions after reset#97099
849261680 wants to merge 7 commits into
openclaw:mainfrom
849261680:fix/90516-stalled-session-reset-retry

Conversation

@849261680

@849261680 849261680 commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #90516.

Long-lived direct chat sessions with compaction history can wedge during an embedded model call. OpenClaw already emits blocked lifecycle diagnostics, but the real Gateway path can surface the diagnostic abort as FailoverError(reason=timeout, rawError=terminated) and mark agent.wait as failed before the outer reset/retry guard gets a chance to rotate the hot session.

Why This Change Was Made

This keeps the recovery narrow: compacted direct sessions only, no visible output, no pending tool work, no fallback attempt, no prior reset, and no side-effect delivery evidence. When those guards match, the execution layer defers the terminal failure to the reply runner, the reply runner rotates the hot session once through the session lifecycle accessor, and retries the same inbound turn without duplicating user-message persistence.

The lifecycle backstop now exposes deferred terminal metadata so the execution layer can recognize the real diagnostic shape (livenessState: blocked plus timeout/terminated) without guessing from a generic timeout string. The reset helper no longer requires callers to expose a mutable whole-session store; if Gateway supplies only the active session entry, persistence still stays centralized in persistSessionResetLifecycle.

User Impact

A Telegram/WebChat/direct-chat user whose compacted session stalls should receive the recovered assistant reply after one automatic reset/retry instead of immediately seeing LLM request timed out. or LLM request failed.. Non-direct chats, non-compacted sessions, attempts with visible output, fallback attempts, pending tools, and pending block delivery keep the existing failure behavior.

Real behavior proof

Behavior addressed: A compacted direct session whose first embedded OpenAI Responses stream stalls silently is reset and retried once instead of surfacing the first run's timeout/error to the user.

Real environment tested: Local macOS source checkout, Node 24.15.0, branch fix/90516-stalled-session-reset-retry at a8442e4571b805f64bf7bdbfe46ce8304c4233e0. No live provider key or channel credential was used; the provider endpoint was a local OpenAI Responses SSE mock behind the real Gateway process.

Exact steps or command run after this patch:

  1. OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS=300000 OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-execution.test.ts
  2. OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS=300000 OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts
  3. pnpm exec oxfmt --check --threads=1 src/auto-reply/reply/agent-lifecycle-terminal.ts src/auto-reply/reply/agent-runner-execution.ts src/auto-reply/reply/agent-runner.ts src/auto-reply/reply/agent-runner-session-reset.ts src/auto-reply/reply/agent-runner-execution.test.ts src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts
  4. pnpm run lint:tmp:session-accessor-boundary
  5. pnpm exec oxlint src/auto-reply/reply/agent-runner-execution.ts src/auto-reply/reply/agent-runner.ts src/auto-reply/reply/agent-runner-session-reset.ts
  6. git diff --check
  7. pnpm tsgo:core:test
  8. .agents/skills/autoreview/scripts/autoreview --mode local
  9. Local Gateway proof: start a real openclaw gateway via createOpenClawTestInstance, configure diagnostics with stuckSessionWarnMs=1000 and stuckSessionAbortMs=1000, prewrite session store key agent:qa:webchat:direct:proof-90516 with sessionId=stalled-session, chatType=direct, channel=webchat, compactionCount=2, systemSent=true, and totalTokens=1000; point openai/gpt-5.5 at a local /v1/responses mock whose first SSE response never emits model output and whose second SSE response returns RESET_RETRY_OK; send chat.send over Gateway and wait with agent.wait.

Evidence after fix: The real Gateway proof returned chat.send -> {"runId":"proof-90516-run","status":"started"} and agent.wait -> {"runId":"proof-90516-run","status":"ok"}. The local provider mock received exactly 2 /v1/responses requests; the first silent SSE request was closed by diagnostic recovery, and the second returned RESET_RETRY_OK. The persisted session store changed from stalled-session to fd5c357b-5533-46d3-b436-713fff9de9c7, with systemSent:false, totalTokens:10, and compactionCount:2. Agent lifecycle events showed finishing with error:"LLM request timed out.", aborted:false, livenessState:"blocked"; then synthetic end with aborted:true, livenessState:"blocked"; then auto_session_reset_retry attempting; then the second run emitted RESET_RETRY_OK; then auto_session_reset_retry completed with previousSessionId:"stalled-session" and the new session id.

Observed result after fix: The user-visible run completed successfully after one reset/retry, no generic timeout payload was delivered as the final outcome, and the proof stderr did not contain the temporary RUN_THROW_RESET_DEBUG marker.

What was not tested: This proof does not use real Telegram credentials or a live OpenAI key. The covered behavior is the shared Gateway/direct-chat/reply-runner path with a local OpenAI Responses-compatible SSE provider mock.

@clawsweeper

clawsweeper Bot commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 7:00 PM ET / 23:00 UTC.

Summary
The branch adds a guarded one-shot reset/retry for compacted direct-chat stalled aborts, defers matching embedded failures, tracks pending block delivery, and updates focused reply-runner tests.

PR surface: Source +289, Tests +310. Total +599 across 8 files.

Reproducibility: yes. at source level: the linked issue has installed-Gateway incident logs, and current main has the abort/error path without a same-turn reset/retry branch. I did not replay the live multi-minute Telegram wedge in this read-only review.

Review metrics: 1 noteworthy metric.

  • Automatic replay path: 1 added. The PR adds one same-turn retry after hot-session rotation, so session ownership and duplicate-delivery semantics need maintainer attention before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #90516
Summary: This PR is the active candidate fix for the canonical stalled compacted direct-session reset/retry issue; adjacent recovery work overlaps diagnostics but not the direct-chat replay policy.

Members:

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

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

  • [P2] Get explicit maintainer acceptance of the one-shot reset/retry and repair-record semantics.
  • [P2] Use Mantis or equivalent Telegram proof if maintainers want transport-visible confirmation before merge.

Mantis proof suggestion
A live Telegram transcript would materially prove one recovered visible reply and no duplicate generic failure text. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

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

Risk before merge

  • [P1] The PR intentionally rotates hot session state and replays the same inbound user turn, so maintainers should explicitly accept the one-shot replay contract before merge.
  • [P1] The linked issue asked for operator-visible repair/audit semantics; the PR emits lifecycle events and persists reset lifecycle state, but whether that is the desired durable repair record remains a maintainer decision.
  • [P1] The supplied proof exercises the shared Gateway/direct-chat path with a local provider mock, not live Telegram credentials; a short Telegram proof would better validate the visible no-duplicate-reply behavior.

Maintainer options:

  1. Accept the guarded replay contract
    A maintainer can explicitly accept the one-shot compacted direct-session reset/retry semantics after reviewing the Gateway proof, lifecycle events, and reset persistence.
  2. Require visible transport proof first
    Ask for Mantis or equivalent Telegram proof showing exactly one recovered reply and no duplicate generic failure text before merge.
  3. Pause for repair-record policy
    Keep the PR open until maintainers decide whether lifecycle events plus persisted reset lifecycle are enough, or whether a separate durable operator repair record is required.

Next step before merge

  • [P2] The latest head has no narrow automated repair target, but the replay, session-state, and repair-record semantics need maintainer judgment before merge.

Security
Cleared: The diff changes reply/session runtime and tests only; I found no dependency, workflow, secret-handling, permission, package, or supply-chain change.

Review details

Best possible solution:

Land the narrow reply-runner recovery after maintainers accept the replay and repair-record contract, with optional Telegram-visible proof confirming one recovered reply and no duplicate generic failure text.

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

Yes, at source level: the linked issue has installed-Gateway incident logs, and current main has the abort/error path without a same-turn reset/retry branch. I did not replay the live multi-minute Telegram wedge in this read-only review.

Is this the best way to solve the issue?

Mostly yes: the reply runner is the right owner boundary, the previous accessor bypass is fixed, and focused tests cover the dangerous guards. The remaining question is maintainer acceptance of the replay and audit semantics, not a discrete code defect I can point to in the latest head.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a user-facing direct-chat failure where compacted sessions can repeatedly return generic LLM failures until manual reset.
  • merge-risk: 🚨 session-state: Merging this PR can rotate a live hot session, rebind the reply operation, and suppress prompt persistence during a same-turn retry.
  • merge-risk: 🚨 message-delivery: Merging this PR replays one inbound turn and relies on delivery evidence to avoid duplicate or missing visible replies.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live output from a real local Gateway/direct-chat run using a local Responses-compatible SSE mock that stalls once, rotates the compacted session, and succeeds on retry.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live output from a real local Gateway/direct-chat run using a local Responses-compatible SSE mock that stalls once, rotates the compacted session, and succeeds on retry.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes visible Telegram direct-chat recovery from generic failure text to a recovered reply, which a short Telegram proof can demonstrate.
Evidence reviewed

PR surface:

Source +289, Tests +310. Total +599 across 8 files.

View PR surface stats
Area Files Added Removed Net
Source 5 313 24 +289
Tests 3 310 0 +310
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 8 623 24 +599

What I checked:

Likely related people:

  • jalehman: Recent history shows session-accessor migration and lifecycle storage work in the affected reply/session reset surfaces. (role: recent area contributor; confidence: high; commits: 127e174c9e4d, 49e6f5a524bc, 7e2b2d2987b2; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/agent-runner-session-reset.ts, src/config/sessions/session-accessor.ts)
  • vincentkoc: Recent commits and co-authored/reviewed work touch the runner, fallback, and auto-reply execution path that this PR changes. (role: recent adjacent contributor and reviewer; confidence: medium; commits: 2b75806197ab, 14e448e0e13d, e998986889ec; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/agent-runner-execution.ts)
  • moeedahmed: Authored the recent auto-reply drain/restart-abort silence fix in the same run failure and delivery path. (role: recent adjacent contributor; confidence: medium; commits: e998986889ec; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/agent-runner-execution.ts)
  • neeravmakwana: Co-authored prior DM continuity work around silent session rotations, relevant to safe direct-chat reset behavior. (role: adjacent session-reset contributor; confidence: medium; commits: 3de5476f5145; files: src/auto-reply/reply/agent-runner-session-reset.ts, src/auto-reply/reply/session-transcript-replay.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.

@849261680

849261680 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

The previous ClawSweeper run reported a Codex infrastructure timeout rather than a patch-quality verdict. Please rerun on current head 51f675d625d30f687157e5d8e9d140cd61307edd.

@clawsweeper

clawsweeper Bot commented Jun 26, 2026 •
edited
Loading

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 added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. 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 26, 2026
@harjothkhara

harjothkhara commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Source/duplicate-lane review note, since the fresh ClawSweeper run timed out rather than producing a patch verdict.

I checked this against the closed predecessor PR #96512 and the current head 51f675d625d30f687157e5d8e9d140cd61307edd. This is the same #90516 lane and same five-file surface; the material delta over #96512 is the added retry-time prompt-persistence guard: after reset, runReplyAgent now sets followupRun.run.suppressNextUserMessagePersistence = true only around reply.run_agent_turn_after_session_reset, then restores/deletes it in finally (agent-runner.ts at this head). The regression test now asserts both the retry param and restoration, so that closes the duplicate-user-prompt persistence gap I would have worried about from the earlier patch.

I did not find a new source-level blocker in the reset guard itself. The duplicate-delivery guard is still the key merge risk: the current code gates on compacted direct sessions, blocked+aborted runs, no fallback attempts, no pending tools, no non-error visible payload, then flushes pending block delivery before deciding whether side-effect delivery happened (guard, pre-decision flush, pipeline send-chain/pending tracking). That matches the earlier review shape from #96512.

So I think the old #96512 maintainer gates still carry forward rather than being resolved by the re-opened lane: redacted live Telegram or equivalent provider proof should show exactly one recovered reply and no duplicate generic failure text, and maintainers should explicitly accept the one-shot hot-session reset/retry + diagnostic/repair-record semantics for #90516. The current patch emits auto_session_reset_retry lifecycle events, but I do not see a durable operator repair record beyond the normal reset lifecycle/session update, so if the issue’s requested audit trail is expected before merge, that contract still needs to be named.

@clawsweeper clawsweeper Bot added the mantis: telegram-visible-proof Mantis should capture Telegram visible proof. label Jun 26, 2026
@849261680

849261680 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated head: 1fed9d2d015d9565cd0e80b83997d84aade74515.

The PR body now includes a fresh real Gateway/direct-chat proof for #90516: first local OpenAI Responses SSE request stalls silently, diagnostic recovery closes it, persisted compacted direct session rotates from stalled-session, second provider request returns RESET_RETRY_OK, and agent.wait returns status: ok.

@clawsweeper

clawsweeper Bot commented Jun 26, 2026 •
edited
Loading

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 added proof: sufficient ClawSweeper judged the real behavior proof convincing. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 26, 2026
@849261680

849261680 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated head: a8442e4571b805f64bf7bdbfe46ce8304c4233e0.

I fixed the CI feedback from the previous head: the retry path no longer imports/calls legacy loadSessionStore from agent-runner.ts, lint:tmp:session-accessor-boundary passes, and the abort-message narrowing no longer trips no-base-to-string. The PR body has fresh proof for the current head, including a real Gateway/direct-chat run where the first local OpenAI Responses SSE request stalls silently, diagnostic recovery closes it, the compacted direct session rotates from stalled-session, the second provider request returns RESET_RETRY_OK, and agent.wait returns status: ok.

@clawsweeper

clawsweeper Bot commented Jun 26, 2026 •
edited
Loading

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 added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 26, 2026
@yetval yetval mentioned this pull request Jun 28, 2026
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jun 28, 2026
Merged
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 12, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 12, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 19, 2026

Copy link
Copy Markdown

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

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

Reviewers

No reviews

Assignees

No one assigned

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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: L stale Marked as stale due to inactivity status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

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

2 participants

@849261680 @harjothkhara