fix(cli-runner): drop stale claude-cli sessionId when transcript missing#77030
Conversation
|
Codex review: needs maintainer review before merge. Summary Reproducibility: yes. at source level. Current main checks missing Claude CLI transcripts only in Next step before merge Security Review detailsBest possible solution: Land the shared prepare-layer missing-transcript guard after normal maintainer, CI, and mergeability checks, while leaving the broader update.run and shell-only cron work tracked in #77011. Do we have a high-confidence way to reproduce the issue? Yes, at source level. Current main checks missing Claude CLI transcripts only in Is this the best way to solve the issue? Yes. Moving the provider-gated transcript probe into What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 111df161df38. Re-review progress:
|
ab863a2 to
dd7b934
Compare
…ing (openclaw#77011) Probe ~/.claude/projects/.../<sid>.jsonl in prepareCliRunContext before emitting `claude --resume <sid>`. When the on-disk transcript no longer exists (e.g. after a half-installed update.run, manual prune, or Claude CLI reinstall), drop the saved cliSessionBinding so this turn starts a fresh session instead of timing out on a dead resume target. The post-run session-store flow then writes the new sessionId back, ending the loop.
dd7b934 to
e9919ad
Compare
|
Landed via temp rebase onto main.
Thanks @openperf! |
Summary
Problem: After a half-installed
update.run(Gateway update.run can leave half-installed package, killing live session transcripts #77011), live Telegram-direct sessions stop responding indefinitely. The Claude CLI live session repeatedly invokesclaude --resume <stale-sid>against a transcript that no longer exists at~/.claude/projects/<project>/<sessionId>.jsonl. The first attempt hangs to the 5-minute hard timeout (durationMs=300011 error=FailoverError); every following turn fast-fails (durationMs=384 error=FailoverError) without ever clearing the dead binding. Concretely,src/agents/cli-runner/prepare.ts:259-271only validates auth-profile / auth-epoch / system-prompt / mcp hashes viaresolveCliSessionReuse(src/agents/cli-session.ts:127); it never checks that the transcript file the persistedcliSessionBindings.claude-cli.sessionIdpoints at is still on disk.Root Cause: A transcript-existence probe (
claudeCliSessionTranscriptHasContentinsrc/agents/command/attempt-execution.helpers.ts:80) is already wired into the user-driven path atsrc/agents/command/attempt-execution.ts:443-467to clear stale bindings before invokingrunCliAgent. The auto-reply / followup / Telegram-direct path takes a different entrypoint —src/auto-reply/reply/agent-runner-execution.ts:1305callsrunCliAgentdirectly and does not go throughrunAgentAttempt, so the existing probe is not consulted on this code path. Thesession_expiredretry insidesrc/agents/cli-runner.ts:331does not compensate either, becauseclaude --resumeagainst a missing transcript surfaces asreason=timeout, notreason=session_expired; the retry branch never fires and the persistedcliSessionBindingis never overwritten with a fresh sessionId. Every subsequent turn re-reads the same dead sessionId until the operator explicitly runsopenclaw sessions cleanup --enforce --fix-missing. The cron isolated-agent path (src/cron/isolated-agent/run-execution.runtime.ts) is in the same category — also entersrunCliAgentdirectly.Fix: Push the transcript-existence pre-flight down into
prepareCliRunContext, the single common entry point for all CLI-run callers (attempt-execution.ts,agent-runner-execution.ts,cron/isolated-agent/run-execution.runtime.ts). When the candidate sessionId is non-empty, the provider isclaude-cli, and the on-disk Claude CLI transcript has no assistant message (i.e. resume target is dead), drop the binding for this turn and return aCliReusableSessionwithinvalidatedReason: "missing-transcript". The current turn then runs as a freshclaudesession (no--resume), and the existing post-run flow insrc/agents/command/session-store.ts:163-168writes the brand-newcliSessionBindingback to the session store, replacing the dead one. The loop is broken at the per-turn level — no manual cleanup needed, no schema migration, no protocol bump. The check is gated onisClaudeCliProvider(params.provider)so non-claude providers pay zero cost. The helper is exposed via the existingprepareDepsinjection seam so tests stay hermetic and don't touch real~/.claude/projects/.What changed:
src/agents/cli-runner/prepare.ts— importisClaudeCliProviderandclaudeCliSessionTranscriptHasContent; expose the latter throughprepareDepsfor test injection; before computingreusableCliSession, run the claude-cli transcript probe and short-circuit to{ invalidatedReason: "missing-transcript" }when the resume target is dead.src/agents/cli-runner/types.ts— extendCliReusableSession.invalidatedReasonunion with the new"missing-transcript"case.src/agents/cli-runner/prepare.test.ts— add avi.mockfor theplugin-sdk/anthropic-cli.jsfacade (so the test runs without bundled-plugin runtime), plus three behavior tests: drop on missing transcript, keep on present transcript, no probe for non-claude providers.CHANGELOG.md— single Fixes line under Unreleased referencing the issue.What did NOT change (scope boundary):
update.runorchestration / atomic-swap logic insrc/infra/update-runner.tsandsrc/infra/package-update-steps.tsis untouched (the npm path already stages and atomically swaps).attempt-execution.ts:443-467is intentionally left in place. It still has the value of clearing the persisted binding synchronously for that path (so a follow-up read inside the same request sees the cleared entry); this PR's pre-flight is a cross-caller safety net, not a replacement. Both call the same helper, so the two layers cannot diverge.session_expiredretry path atcli-runner.ts:331-358is unchanged. The two failure modes are complementary:session_expiredis the clean-error case (Claude returns "Conversation not found"), already handled in-process by the retry;timeoutis the silent-hang case (Claude blocks until the per-turn deadline), addressed here at the preparation layer above.Reproduction
cliSessionBindings.claude-cli.sessionId(any successful turn does this).update.runoutcome by removing the corresponding~/.claude/projects/<project>/<sessionId>.jsonlfile (or replace its contents with an empty file). The persisted session entry still references the now-missing sessionId.claude live session turn failed: ... durationMs=300011 error=FailoverError(5-minute resume hang) andcli session resetis not logged. Every subsequent turn fails immediately with the same FailoverError; the binding never refreshes; user is stuck.cli session reset: provider=claude-cli reason=missing-transcript ...once at the start of the next turn, the run completes against a fresh Claude session, and the new sessionId is persisted by the existing post-run flow. Subsequent turns succeed.Risk / Mitigation
claudeCliSessionTranscriptHasContentreads up toSESSION_FILE_MAX_RECORDS=500lines and returnstrueonce it sees an assistant message. A binding is only persisted bysession-store.ts:setCliSessionBindingafter a successful run, which by definition has flushed at least one assistant turn — so a freshly-written binding always passes the probe. The probe walks~/.claude/projects/*(already done inattempt-execution.helpers.ts), so home-dir / project-prefix mismatches behave the same as in the existing user-driven check.Mitigation: covered by the new "keeps the claude-cli sessionId when the on-disk transcript is present" test, plus the existing
claudeCliSessionTranscriptHasContentsuite inattempt-execution.test.ts:366-432(already validates symlink rejection, path-traversal rejection, and assistant-message detection).isClaudeCliProvider(params.provider), so any non-claude provider follows the unmodified existing branch.Mitigation: new "does not probe the transcript for non-claude-cli providers" test asserts
transcriptCheckis never called and the existing{ sessionId }flow is preserved.fs.readdirplus a small boundedfs.open+readlineper claude-cli turn. The same helper is already on the user-driven hot path viaattempt-execution.ts:447; we are not introducing a new I/O class, only widening its coverage. Worst-case is bounded bySESSION_FILE_MAX_RECORDS=500lines and short-circuits on the first assistant message (typically line 1-2).Mitigation: the helper itself is unchanged and already production-tested.
claudeCliSessionTranscriptHasContentcrosses fromagents/cli-runner/intoagents/command/, andisClaudeCliProvideris loaded through theplugin-sdk/anthropic-cli.jsfacade.Mitigation: both directions are already used in core (
prepare.tsalready imports../command/types.js;attempt-execution.tsalready loads the same facade). No new architectural seam, no cycle (helpers.ts does not import back into cli-runner). Test file mocks the facade locally so the unit suite stays hermetic.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Refs #77011 — addresses the missing-transcript auto-recovery scenario described in the issue. The other two items in the same issue (
update.runatomicity and shell-only cronsystemEvent runsession rows) are independent failure modes; they remain open and out of scope here so they can be tracked and shipped separately.