Skip to content

Commit 76530ed

Browse files
draixdraix
committed
fix(runtime): discard poisoned resume id after FailoverError to break watchdog cascade (#79365)
When a resumed claude-cli live session was killed by the no-output watchdog the runtime kept offering the same persisted --resume <id> to every subsequent cold restart. Because the on-disk transcript was poisoned (corrupted tool-call state, hung MCP shutdown, etc.), each re-resumed turn hung again to the watchdog cap, producing the 5x 180s = ~15min Telegram-lane outage seen in the field until a manual gateway bounce. The historical recovery path only cleared the persisted CLI session binding for two stale-session classes: * session_expired - claude-cli reported "Conversation not found" * missing-transcript (#77030) - the on-disk .jsonl is gone Resumed-but-poisoned transcripts surfaced as FailoverError reason=timeout and left the binding untouched, hence the cascade. Fix: 1. cli-runner.ts (runPreparedCliAgent) Extend the in-runner cold-restart retry to also fire on reason=timeout when the provider is claude-cli and a resume sessionId was used. This mirrors the existing session_expired retry contract and keeps a single agent_end hook event per turn (codex review concern on #77385). The retry runs without --resume so the cold child generates a clean session id. 2. attempt-execution.ts Clear the persisted CLI session binding for the same condition so the next user turn also starts cold even when the in-runner retry was not taken (e.g. no sessionKey on the runner params). Both changes are gated on isClaudeCliProvider and the presence of a stored sessionId, so non-resumed timeouts (cold-start slow CLIs) and non-claude providers keep their previous behaviour. Tests: * cli-runner.reliability.test.ts - in-runner retry happens on poisoned-resume timeout, single agent_end success hook fires; no retry on cold-start timeout; no retry for non-claude providers. * attempt-execution.cli.test.ts - persisted binding is wiped after a poisoned-resume timeout and the retry uses a fresh session id; no retry / no clear on cold-start timeouts. Verified: pnpm test src/agents/cli-runner.spawn.test.ts \ src/agents/cli-runner.reliability.test.ts \ src/agents/command/session-store.test.ts \ src/agents/command/attempt-execution.cli.test.ts \ src/agents/command/attempt-execution.test.ts -> 5 files, 147 tests passed pnpm exec oxfmt --check --threads=1 (touched files): clean pnpm tsgo:core / tsgo:core:test: clean Fixes #79365
1 parent e6031fd commit 76530ed

5 files changed

Lines changed: 396 additions & 20 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ Docs: https://docs.openclaw.ai
184184

185185
### Fixes
186186

187+
- Agents/cli-runner: break the resume-watchdog cascade by discarding the persisted Claude CLI sessionId after a resumed live session is killed by the no-output watchdog, so the same poisoned transcript is not re-resumed on every following turn. The CLI runner now retries the in-flight attempt cold (no `--resume`) on `FailoverError reason=timeout` for resumed claude-cli sessions, mirroring the existing `session_expired` recovery contract and keeping a single `agent_end` hook event per turn; the user-driven attempt path additionally clears the stored binding so subsequent turns also start cold. Fixes #79365.
187188
- Cron/agents: recognize same-target `edit`↔`write` recovery in `isSameToolMutationAction`, so a successful `write` to a path clears an earlier failed `edit` on the same path. Stops cron from reporting fatal failures when an agent self-heals across `edit` and `write`, while preserving same-tool fingerprint matching, blocking different-target writes, and excluding tools (including `apply_patch`) whose real call args do not produce a stable `path` fingerprint segment. Fixes #79024. Thanks @RenzoMXD.
188189
- Agents/compaction: keep the recent tail after manual `/compact` when Pi returns an empty or no-op compaction summary, preventing blank checkpoints from replacing the live context.
189190
- fix(discord): gate user allowlist name resolution [AI]. (#79002) Thanks @pgondhi987.

‎src/agents/cli-runner.reliability.test.ts‎

Lines changed: 224 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,14 @@ vi.mock("../tts/tts.js", () => ({
3232
buildTtsSystemPromptHint: vi.fn(() => undefined),
3333
}));
3434

35+
// Stub the bundled-plugin facade so isClaudeCliProvider works in unit
36+
// tests without loading the real anthropic plugin assets. Required by the
37+
// in-runner poisoned-resume timeout retry path (#79365).
38+
vi.mock("../plugin-sdk/anthropic-cli.js", () => ({
39+
CLAUDE_CLI_BACKEND_ID: "claude-cli",
40+
isClaudeCliProvider: (providerId: string) => providerId === "claude-cli",
41+
}));
42+
3543
const mockGetGlobalHookRunner = vi.mocked(getGlobalHookRunner);
3644
const hookRunnerGlobalStateKey = Symbol.for("openclaw.plugins.hook-runner-global-state");
3745

@@ -108,25 +116,38 @@ function buildPreparedContext(params?: {
108116
runId?: string;
109117
lane?: string;
110118
openClawHistoryPrompt?: string;
119+
provider?: "codex-cli" | "claude-cli";
111120
}): PreparedCliRunContext {
112-
const backend = {
113-
command: "codex",
114-
args: ["exec", "--json"],
115-
output: "text" as const,
116-
input: "arg" as const,
117-
modelArg: "--model",
118-
sessionMode: "existing" as const,
119-
serialize: true,
120-
};
121+
const provider = params?.provider ?? "codex-cli";
122+
const backend =
123+
provider === "claude-cli"
124+
? {
125+
command: "claude",
126+
args: ["-p", "--output-format", "text"],
127+
output: "text" as const,
128+
input: "arg" as const,
129+
modelArg: "--model",
130+
sessionMode: "existing" as const,
131+
serialize: true,
132+
}
133+
: {
134+
command: "codex",
135+
args: ["exec", "--json"],
136+
output: "text" as const,
137+
input: "arg" as const,
138+
modelArg: "--model",
139+
sessionMode: "existing" as const,
140+
serialize: true,
141+
};
121142
return {
122143
params: {
123144
sessionId: "s1",
124145
sessionKey: params?.sessionKey,
125146
sessionFile: "/tmp/session.jsonl",
126147
workspaceDir: "/tmp",
127148
prompt: "hi",
128-
provider: "codex-cli",
129-
model: "gpt-5.4",
149+
provider,
150+
model: provider === "claude-cli" ? "opus" : "gpt-5.4",
130151
thinkLevel: "low",
131152
timeoutMs: 1_000,
132153
runId: params?.runId ?? "run-2",
@@ -135,18 +156,18 @@ function buildPreparedContext(params?: {
135156
started: Date.now(),
136157
workspaceDir: "/tmp",
137158
backendResolved: {
138-
id: "codex-cli",
159+
id: provider,
139160
config: backend,
140161
bundleMcp: false,
141-
pluginId: "openai",
162+
pluginId: provider === "claude-cli" ? "anthropic" : "openai",
142163
},
143164
preparedBackend: {
144165
backend,
145166
env: {},
146167
},
147168
reusableCliSession: params?.cliSessionId ? { sessionId: params.cliSessionId } : {},
148-
modelId: "gpt-5.4",
149-
normalizedModel: "gpt-5.4",
169+
modelId: provider === "claude-cli" ? "opus" : "gpt-5.4",
170+
normalizedModel: provider === "claude-cli" ? "opus" : "gpt-5.4",
150171
systemPrompt: "You are a helpful assistant.",
151172
systemPromptReport: {} as PreparedCliRunContext["systemPromptReport"],
152173
bootstrapPromptWarningLines: [],
@@ -356,6 +377,194 @@ describe("runCliAgent reliability", () => {
356377
}
357378
});
358379

380+
// Regression tests for #79365 — resume-watchdog cascade. Mirrors the
381+
// existing session_expired in-runner retry contract: when a resumed
382+
// claude-cli session is killed by the no-output watchdog (timeout
383+
// FailoverError) and we have a sessionKey + sessionId, retry the same
384+
// attempt cold (no --resume) so a single agent_end hook fires with the
385+
// final result. Hook ordering must stay consistent (no failed agent_end
386+
// followed by a successful return), which is why this lives in the
387+
// runner and not the outer attempt-execution layer.
388+
it("retries cold (no --resume) when a resumed claude-cli session hits the no-output watchdog (#79365)", async () => {
389+
const hookRunner = {
390+
hasHooks: vi.fn((hookName: string) => ["llm_input", "agent_end"].includes(hookName)),
391+
runLlmInput: vi.fn(async () => undefined),
392+
runLlmOutput: vi.fn(async () => undefined),
393+
runAgentEnd: vi.fn(async () => undefined),
394+
};
395+
setHookRunnerForTest(hookRunner);
396+
supervisorSpawnMock.mockClear();
397+
// First attempt: poisoned resume hangs and watchdog kills it.
398+
supervisorSpawnMock.mockResolvedValueOnce(
399+
createManagedRun({
400+
reason: "no-output-timeout",
401+
exitCode: null,
402+
exitSignal: "SIGKILL",
403+
durationMs: 200,
404+
stdout: "",
405+
stderr: "",
406+
timedOut: true,
407+
noOutputTimedOut: true,
408+
}),
409+
);
410+
// Second attempt (cold): clean recovery with a fresh sessionId.
411+
supervisorSpawnMock.mockResolvedValueOnce(
412+
createManagedRun({
413+
reason: "exit",
414+
exitCode: 0,
415+
exitSignal: null,
416+
durationMs: 50,
417+
stdout: "hello after cold restart",
418+
stderr: "",
419+
timedOut: false,
420+
noOutputTimedOut: false,
421+
}),
422+
);
423+
const { dir, sessionFile } = createSessionFile();
424+
425+
try {
426+
const result = await runPreparedCliAgent({
427+
...buildPreparedContext({
428+
provider: "claude-cli",
429+
sessionKey: "agent:main:subagent:resume-cascade",
430+
cliSessionId: "poisoned-cli-session",
431+
runId: "run-resume-cascade",
432+
}),
433+
params: {
434+
...buildPreparedContext({
435+
provider: "claude-cli",
436+
sessionKey: "agent:main:subagent:resume-cascade",
437+
cliSessionId: "poisoned-cli-session",
438+
runId: "run-resume-cascade",
439+
}).params,
440+
agentId: "main",
441+
sessionFile,
442+
workspaceDir: dir,
443+
},
444+
});
445+
446+
expect(result.payloads).toEqual([{ text: "hello after cold restart" }]);
447+
// Both attempts ran: first with --resume <id>, second cold.
448+
expect(supervisorSpawnMock).toHaveBeenCalledTimes(2);
449+
// Exactly one agent_end hook event fires — with success: true — so
450+
// hook consumers do not see a failed-then-succeeded sequence.
451+
await vi.waitFor(() => {
452+
expect(hookRunner.runAgentEnd).toHaveBeenCalledTimes(1);
453+
});
454+
expect(hookRunner.runAgentEnd).toHaveBeenCalledWith(
455+
expect.objectContaining({ success: true }),
456+
expect.any(Object),
457+
);
458+
} finally {
459+
fs.rmSync(dir, { recursive: true, force: true });
460+
}
461+
});
462+
463+
it("does not retry on no-output watchdog timeout when there is no resumed claude-cli session", async () => {
464+
const hookRunner = {
465+
hasHooks: vi.fn((hookName: string) => ["llm_input", "agent_end"].includes(hookName)),
466+
runLlmInput: vi.fn(async () => undefined),
467+
runLlmOutput: vi.fn(async () => undefined),
468+
runAgentEnd: vi.fn(async () => undefined),
469+
};
470+
setHookRunnerForTest(hookRunner);
471+
supervisorSpawnMock.mockClear();
472+
supervisorSpawnMock.mockResolvedValueOnce(
473+
createManagedRun({
474+
reason: "no-output-timeout",
475+
exitCode: null,
476+
exitSignal: "SIGKILL",
477+
durationMs: 200,
478+
stdout: "",
479+
stderr: "",
480+
timedOut: true,
481+
noOutputTimedOut: true,
482+
}),
483+
);
484+
const { dir, sessionFile } = createSessionFile();
485+
486+
try {
487+
await expect(
488+
runPreparedCliAgent({
489+
...buildPreparedContext({
490+
provider: "claude-cli",
491+
sessionKey: "agent:main:subagent:cold-timeout",
492+
// No cliSessionId — cold start.
493+
runId: "run-cold-timeout",
494+
}),
495+
params: {
496+
...buildPreparedContext({
497+
provider: "claude-cli",
498+
sessionKey: "agent:main:subagent:cold-timeout",
499+
runId: "run-cold-timeout",
500+
}).params,
501+
agentId: "main",
502+
sessionFile,
503+
workspaceDir: dir,
504+
},
505+
}),
506+
).rejects.toThrow("produced no output");
507+
// Only one attempt — there was no poisoned resume id to discard.
508+
expect(supervisorSpawnMock).toHaveBeenCalledTimes(1);
509+
} finally {
510+
fs.rmSync(dir, { recursive: true, force: true });
511+
}
512+
});
513+
514+
it("does not retry non-claude-cli providers on watchdog timeout, even with a resume id", async () => {
515+
// The resume cascade fix is scoped to claude-cli; a codex-cli
516+
// (or other) provider that times out with a resume id should not
517+
// get the cold-restart retry, preserving the historical contract.
518+
const hookRunner = {
519+
hasHooks: vi.fn((hookName: string) => ["llm_input", "agent_end"].includes(hookName)),
520+
runLlmInput: vi.fn(async () => undefined),
521+
runLlmOutput: vi.fn(async () => undefined),
522+
runAgentEnd: vi.fn(async () => undefined),
523+
};
524+
setHookRunnerForTest(hookRunner);
525+
supervisorSpawnMock.mockClear();
526+
supervisorSpawnMock.mockResolvedValueOnce(
527+
createManagedRun({
528+
reason: "no-output-timeout",
529+
exitCode: null,
530+
exitSignal: "SIGKILL",
531+
durationMs: 200,
532+
stdout: "",
533+
stderr: "",
534+
timedOut: true,
535+
noOutputTimedOut: true,
536+
}),
537+
);
538+
const { dir, sessionFile } = createSessionFile();
539+
540+
try {
541+
await expect(
542+
runPreparedCliAgent({
543+
...buildPreparedContext({
544+
provider: "codex-cli",
545+
sessionKey: "agent:main:subagent:codex-timeout",
546+
cliSessionId: "codex-thread-123",
547+
runId: "run-codex-timeout",
548+
}),
549+
params: {
550+
...buildPreparedContext({
551+
provider: "codex-cli",
552+
sessionKey: "agent:main:subagent:codex-timeout",
553+
cliSessionId: "codex-thread-123",
554+
runId: "run-codex-timeout",
555+
}).params,
556+
agentId: "main",
557+
sessionFile,
558+
workspaceDir: dir,
559+
},
560+
}),
561+
).rejects.toThrow("produced no output");
562+
expect(supervisorSpawnMock).toHaveBeenCalledTimes(1);
563+
} finally {
564+
fs.rmSync(dir, { recursive: true, force: true });
565+
}
566+
});
567+
359568
it("returns the assembled CLI prompt in meta for raw trace consumers", async () => {
360569
supervisorSpawnMock.mockResolvedValueOnce(
361570
createManagedRun({

‎src/agents/cli-runner.ts‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { ReplyPayload } from "../auto-reply/reply-payload.js";
33
import { SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js";
44
import { formatErrorMessage } from "../infra/errors.js";
55
import { createSubsystemLogger } from "../logging/subsystem.js";
6+
import { isClaudeCliProvider } from "../plugin-sdk/anthropic-cli.js";
67
import { buildAgentHookContextChannelFields } from "../plugins/hook-agent-context.js";
78
import { resolveBlockMessage } from "../plugins/hook-decision-types.js";
89
import { getGlobalHookRunner } from "../plugins/hook-runner-global.js";
@@ -474,13 +475,37 @@ export async function runPreparedCliAgent(
474475
} catch (err) {
475476
if (isFailoverError(err)) {
476477
const retryableSessionId = context.reusableCliSession.sessionId ?? params.cliSessionId;
477-
// Check if this is a session expired error and we have a session to clear
478-
if (err.reason === "session_expired" && retryableSessionId && params.sessionKey) {
479-
// Clear the expired session ID from the session entry
478+
// Decide whether to recover in-runner with a cold restart (no --resume).
479+
//
480+
// session_expired is the clean-error case ("Conversation not found");
481+
// we have always retried it here.
482+
//
483+
// timeout on a resumed claude-cli session is the silent-hang case: the
484+
// on-disk transcript is poisoned (corrupted tool-call state, stuck MCP
485+
// call, etc.), the watchdog killed the child after no output, and re-
486+
// resuming the same sessionId on the next turn just hangs again. The
487+
// historical behaviour was a 5-attempt cascade burning ~15 min of the
488+
// lane before a manual gateway bounce (#79365). Retry once here without
489+
// --resume so the cold child starts a fresh session and the post-run
490+
// flow persists a clean sessionId via setCliSessionBinding. Hooks stay
491+
// consistent (single agent_end), matching the existing session_expired
492+
// retry's contract.
493+
const isPoisonedResumeTimeout =
494+
err.reason === "timeout" &&
495+
isClaudeCliProvider(params.provider) &&
496+
Boolean(context.reusableCliSession.sessionId);
497+
const recoverableReason = err.reason === "session_expired" || isPoisonedResumeTimeout;
498+
if (recoverableReason && retryableSessionId && params.sessionKey) {
499+
// Clear the expired or poisoned session ID from the session entry
480500
// This requires access to the session store, which we don't have here
481501
// We'll need to modify the caller to handle this case
482502

483503
// For now, retry without the session ID to create a new session
504+
if (isPoisonedResumeTimeout) {
505+
log.warn(
506+
`claude-cli resume timeout, retrying without --resume: provider=${params.provider} sessionId=${retryableSessionId}`,
507+
);
508+
}
484509
try {
485510
const { output, lastAssistant } = await executeCliAttempt(undefined);
486511
const effectiveCliSessionId = output.sessionId;

0 commit comments

Comments
 (0)