From 93f4e2bb67805504313505bfa5a68053014e0b90 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 31 Oct 2026 16:00:52 -0700 Subject: [PATCH 1/3] fix(cli): name a hung claude turn stuck, not started on retries (TASK-710) A claude spawn counted as started on any stdout or stderr byte or any transcript growth. A provider that only retries writes system events (init, api_retry) and warnings, so it looked alive until the run timeout. Run claude with stream-json. In stream mode the watchdog counts only model events (non-system stream-json lines) as life. Stderr and transcript growth no longer count there. A spawn with no model event inside the window is killed as "stuck: no model output after the first transcript row". The reply is the result event's text (claudeResultText). Live walk (claude 2.1.314, stub provider that only retries, window 8s): 3 of 3 named stuck and killed at ~8.4s. Positive control (12s turn after its first model event, window 8s) is not killed and returns its text. Mutant with the signal dropped: the stall is never named. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: dart Agent-Model: claude/claude-haiku-5-5/default --- cli/__tests__/adapters.claude.test.mjs | 2 +- cli/__tests__/first-output-watchdog.test.mjs | 98 +++++++++++++++++++- cli/src/lib/adapters/claude.js | 30 +++++- cli/src/lib/first-output-watchdog.js | 42 ++++++++- 4 files changed, 161 insertions(+), 11 deletions(-) diff --git a/cli/__tests__/adapters.claude.test.mjs b/cli/__tests__/adapters.claude.test.mjs index e2da4b7c39..29c5d1f65f 100644 --- a/cli/__tests__/adapters.claude.test.mjs +++ b/cli/__tests__/adapters.claude.test.mjs @@ -120,7 +120,7 @@ describe('claude adapter — spawn()', () => { expect(calls[0].args[0]).toBe('-p'); expect(calls[0].args[1]).toContain('hello world'); expect(calls[0].args.slice(2)).toEqual( - ['--output-format', 'text', '--resume', 'sid-123', '--model', 'test-model'], + ['--output-format', 'stream-json', '--verbose', '--include-partial-messages', '--resume', 'sid-123', '--model', 'test-model'], ); // Explicit: must NOT use --session-id on resume path — the whole point. expect(calls[0].args).not.toContain('--session-id'); diff --git a/cli/__tests__/first-output-watchdog.test.mjs b/cli/__tests__/first-output-watchdog.test.mjs index 6f38aed0b9..9d1d722612 100644 --- a/cli/__tests__/first-output-watchdog.test.mjs +++ b/cli/__tests__/first-output-watchdog.test.mjs @@ -38,8 +38,10 @@ const stubAdapter = (await import('../src/lib/adapters/stub.js')).default; const claude = (await import('../src/lib/adapters/claude.js')).default; const { consoleLocked, spawnStalledError, watchFirstOutput, firstOutputWindowMs, DEFAULT_FIRST_OUTPUT_WINDOW_MS, physicalPath, - killStallSurvivors, + killStallSurvivors, isModelProgressLine, } = await import('../src/lib/first-output-watchdog.js'); +const { claudeResultText } = await import('../src/lib/adapters/claude.js'); +const watchStream = (args) => watchFirstOutput({ ...args, modelStream: true }); const { classifySpawnFailure, spawnRetryPolicy, SPAWN_FAILURE_CLASS } = await import('../src/lib/spawn-retry.js'); afterAll(() => fs.rmSync(sessionsTmpDir, { recursive: true, force: true })); @@ -247,8 +249,9 @@ describe('the claude adapter under the first-output watchdog', () => { test('a slow spawn that is talking is not killed', async () => { const { value, error, child } = await spawnClaude((proc) => { proc.stderr.emit('data', Buffer.from('warming up')); + proc.stdout.emit('data', Buffer.from('{"type":"assistant","message":{}}\n')); setTimeout(() => { - proc.stdout.emit('data', Buffer.from('the answer')); + proc.stdout.emit('data', Buffer.from('{"type":"result","result":"the answer"}\n')); proc.emit('close', 0); }, 150); }, { windowMs: 50 }); @@ -457,3 +460,94 @@ describe('performRun — a stalled turn', () => { } }); }); + +describe('the model-stream watchdog (stream-json, TASK-710)', () => { + const transcriptWithFirstRow = () => { + const file = path.join(sessionsTmpDir, `transcript-${Math.random().toString(36).slice(2)}.jsonl`); + fs.writeFileSync(file, '{"type":"user"}\n'); + return file; + }; + const streamProc = () => { + const proc = new EventEmitter(); + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + return proc; + }; + + test('isModelProgressLine counts model events and not system events or non-JSON', () => { + expect(isModelProgressLine('{"type":"assistant","message":{}}')).toBe(true); + expect(isModelProgressLine('{"type":"stream_event","event":{}}')).toBe(true); + expect(isModelProgressLine('{"type":"system","subtype":"init"}')).toBe(false); + expect(isModelProgressLine('{"type":"system","subtype":"api_retry","attempt":1}')).toBe(false); + expect(isModelProgressLine('not json')).toBe(false); + }); + + test('a provider that only retries is named stuck after the first transcript row, not left to the run timeout', async () => { + const proc = streamProc(); + const transcriptPath = transcriptWithFirstRow(); + const onStall = jest.fn(); + watchStream({ proc, windowMs: 50, transcriptPath, modelStream: true, onStall, lockProbe: async () => false }); + proc.stdout.emit('data', Buffer.from('{"type":"system","subtype":"init"}\n')); + proc.stdout.emit('data', Buffer.from('{"type":"system","subtype":"api_retry","attempt":1}\n')); + // Transcript growth alone must not count as life in stream mode. + fs.appendFileSync(transcriptPath, '{"type":"user","retry":true}\n'); + await new Promise((resolve) => setTimeout(resolve, 120)); + expect(onStall).toHaveBeenCalledTimes(1); + expect(onStall.mock.calls[0][0]).toBe('stuck: no model output after the first transcript row'); + }); + + test('a stderr warning at spawn is not life in stream mode', async () => { + const proc = streamProc(); + const transcriptPath = transcriptWithFirstRow(); + const onStall = jest.fn(); + watchStream({ proc, windowMs: 50, transcriptPath, onStall, lockProbe: async () => false }); + proc.stderr.emit('data', Buffer.from('[claude-code:unrecognized_model] {"model":"x"}\n')); + await new Promise((resolve) => setTimeout(resolve, 120)); + expect(onStall).toHaveBeenCalledWith('stuck: no model output after the first transcript row'); + }); + + test('a model event split across chunks still counts as life', async () => { + const proc = streamProc(); + const transcriptPath = transcriptWithFirstRow(); + const onStall = jest.fn(); + const w = watchStream({ proc, windowMs: 50, transcriptPath, modelStream: true, onStall, lockProbe: async () => false }); + proc.stdout.emit('data', Buffer.from('{"type":"assi')); + proc.stdout.emit('data', Buffer.from('stant","message":{}}\n')); + await new Promise((resolve) => setTimeout(resolve, 120)); + w.stop(); + expect(onStall).not.toHaveBeenCalled(); + }); + + test('a locked console is still named as such in stream mode', async () => { + const proc = streamProc(); + const transcriptPath = transcriptWithFirstRow(); + const onStall = jest.fn(); + watchStream({ proc, windowMs: 50, transcriptPath, modelStream: true, onStall, lockProbe: async () => true }); + await new Promise((resolve) => setTimeout(resolve, 120)); + expect(onStall).toHaveBeenCalledWith('console locked'); + }); + + test('a missing transcript alone does not kill in stream mode either', async () => { + const proc = streamProc(); + const onStall = jest.fn(); + const w = watchStream({ proc, windowMs: 50, transcriptPath: path.join(sessionsTmpDir, 'absent.jsonl'), modelStream: true, onStall, lockProbe: async () => false }); + await new Promise((resolve) => setTimeout(resolve, 120)); + w.stop(); + expect(onStall).not.toHaveBeenCalled(); + }); +}); + +describe('claudeResultText', () => { + test('returns the result event text from stream-json output', () => { + const stdout = [ + '{"type":"system","subtype":"init"}', + '{"type":"assistant","message":{"content":[{"type":"text","text":"partial"}]}}', + '{"type":"result","subtype":"success","result":"final answer\\n","is_error":false}', + ].join('\n'); + expect(claudeResultText(stdout)).toBe('final answer'); + }); + + test('returns plain text output as it arrived when there is no result event', () => { + expect(claudeResultText(' done quietly \n')).toBe('done quietly'); + }); +}); diff --git a/cli/src/lib/adapters/claude.js b/cli/src/lib/adapters/claude.js index b72a21b543..7c0b198fbc 100644 --- a/cli/src/lib/adapters/claude.js +++ b/cli/src/lib/adapters/claude.js @@ -494,10 +494,29 @@ const publicSkillPromptArgs = async (skillSources) => { // `ownGroup` is set only under the run supervisor, which kills the child's // whole group; unsupervised, the spawn options are exactly what they were. +// Stream-json so each model event is a sign of life for the watchdog. The +// partial messages put thinking deltas on the stream too, so a long think is +// visible while it runs. A hung provider writes only `system` retry events, +// and the watchdog does not count those. +const STREAM_ARGS = ['--output-format', 'stream-json', '--verbose', '--include-partial-messages']; + +// The reply is the `result` event's text. Output with no result event is +// returned as it arrived, so a turn that never reaches one still gets its text. +export const claudeResultText = (stdout) => { + const lines = String(stdout || '').split('\n'); + for (let i = lines.length - 1; i >= 0; i -= 1) { + try { + const event = JSON.parse(lines[i]); + if (event?.type === 'result') return String(event.result ?? '').trim(); + } catch { /* not an event line */ } + } + return String(stdout || '').trim(); +}; + const runClaude = ({ cmd, args, cwd, env, timeoutMs, credentials = [], spawnImpl = childSpawn, onSpawn, controls = {}, ownGroup = false, transcriptPath = null, firstOutputMs = firstOutputWindowMs(), lockProbe, stallKillGraceMs = STALL_KILL_GRACE_MS, - processTable = readProcessTable, survivorIo, + processTable = readProcessTable, survivorIo, modelStream = true, }) => new Promise((resolve, reject) => { const proc = spawnInOwnGroup(spawnImpl, cmd, args, { cwd, @@ -529,6 +548,7 @@ const runClaude = ({ proc, windowMs: firstOutputMs, transcriptPath, + modelStream, lockProbe, onStall: (reason) => { stalled = spawnStalledError({ label: 'claude', windowMs: firstOutputMs, reason }); @@ -1161,7 +1181,7 @@ export default { ...(seatModel ? ['--model', seatModel] : []), ...(ctx.environment?.effort ? ['--effort', String(ctx.environment.effort)] : []), ]; - const baseArgs = ['-p', fullPrompt, '--output-format', 'text', sessionFlag, sessionId, ...modelArgs]; + const baseArgs = ['-p', fullPrompt, ...STREAM_ARGS, sessionFlag, sessionId, ...modelArgs]; let mountedSkills = []; let skillMount = { mounted: [], conflicted: [] }; @@ -1274,7 +1294,7 @@ export default { const contextTokens = claudeContextTokens({ cwd: physicalPath(ctx.cwd), sessionId, configDir: claudeConfigDirFor({ ...env, HOME: childHome || env?.HOME }), }); - return { text: stdout.trim(), newSessionId: sessionId, contextTokens }; + return { text: claudeResultText(stdout), newSessionId: sessionId, contextTokens }; } catch (err) { // A public seat's refusal is not a session-store failure, whatever its // message happens to contain: it must reach the run loop, never the @@ -1295,7 +1315,7 @@ export default { memoryWithheld: ctx.memoryWithheld === true, }); const retryBase = ['-p', freshPrompt, - '--output-format', 'text', '--session-id', freshId, ...modelArgs]; + ...STREAM_ARGS, '--session-id', freshId, ...modelArgs]; const retryCtx = { ...ctx, mcpConfigPath: mcpConfig?.file || null, @@ -1337,7 +1357,7 @@ export default { cwd: physicalPath(ctx.cwd), sessionId: freshId, configDir: claudeConfigDirFor({ ...retry.env, HOME: retry.childHome || retry.env?.HOME }), }); - return { text: stdout.trim(), newSessionId: freshId, contextTokens }; + return { text: claudeResultText(stdout), newSessionId: freshId, contextTokens }; } throw err; } finally { diff --git a/cli/src/lib/first-output-watchdog.js b/cli/src/lib/first-output-watchdog.js index 2677bf88d2..9b6ce55d4d 100644 --- a/cli/src/lib/first-output-watchdog.js +++ b/cli/src/lib/first-output-watchdog.js @@ -16,6 +16,9 @@ * A spawn that does neither inside the window is killed with a reason. When * the transcript is missing altogether, the path may simply be wrong, so the * spawn is killed only if the console is also locked. + * + * That is the rule for a plain-text spawn. A stream-json spawn (TASK-710) counts + * only model events on stdout; see `modelStream` on watchFirstOutput. */ import { realpathSync, statSync } from 'fs'; import { readProcess } from '@commonlyai/seats/process-group'; @@ -99,15 +102,35 @@ const sizeOf = (file) => { } }; +// A stream-json line is model progress when it is an event other than `system`. +// The `system` events are `init` (written before any model call) and +// `api_retry` (a provider failing and retrying). Counting either would keep a +// hung provider looking alive until the run timeout. +export const isModelProgressLine = (line) => { + try { + const event = JSON.parse(line); + return !!event && typeof event === 'object' && event.type !== 'system'; + } catch { + return false; + } +}; + /** * Watch `proc` for its first sign of life. Returns `{ stop }`; call it when the * process closes. `onStall(reason)` fires at most once, after the caller should * kill the process. + * + * With `modelStream`, the spawn writes stream-json events. Only model output + * counts as life there. A transcript row does not, because claude writes its + * first row about a second after spawn, before any model reply. Stderr does + * not either: claude prints warnings at spawn (an unrecognised model, a + * connector notice) whatever the provider does next. */ export const watchFirstOutput = ({ proc, windowMs, transcriptPath = null, + modelStream = false, onStall, lockProbe = consoleLocked, setTimeoutImpl = setTimeout, @@ -116,19 +139,32 @@ export const watchFirstOutput = ({ if (!windowMs) return { stop: () => {} }; let started = false; let stopped = false; + let partial = ''; const markStarted = () => { started = true; }; - proc.stdout?.on('data', markStarted); - proc.stderr?.on('data', markStarted); + if (modelStream) { + proc.stdout?.on('data', (chunk) => { + const lines = (partial + chunk.toString()).split('\n'); + partial = lines.pop(); + if (lines.some(isModelProgressLine)) markStarted(); + }); + } else { + proc.stdout?.on('data', markStarted); + proc.stderr?.on('data', markStarted); + } // A missing file counts as empty, so creating an empty one is not growth. const baseline = Math.max(0, sizeOf(transcriptPath)); const timer = setTimeoutImpl(async () => { const size = sizeOf(transcriptPath); - if (started || size > baseline) return; + if (started || (!modelStream && size > baseline)) return; const locked = await lockProbe().catch(() => null); if (stopped) return; // No transcript where one was expected may only mean the path is wrong, so // that alone never kills a turn: it takes a locked console as well. if (size < 0 && locked !== true) return; + if (modelStream) { + onStall(locked ? 'console locked' : 'stuck: no model output after the first transcript row'); + return; + } onStall(locked ? 'console locked' : 'no transcript, stdout or stderr'); }, windowMs); timer?.unref?.(); From 034df3d44fd63cd624a5e2916f78c0e37a6f40eb Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 31 Oct 2026 16:04:31 -0700 Subject: [PATCH 2/3] fix(cli): keep the failure tail and the reply text readable in stream mode (TASK-710) Review of 93f4e2bb6 found four real defects in the stream-json change: - A non-zero exit put multi-line NDJSON into the 2000-char failure detail, so the head cut dropped the terminal error. claudeFailureTail keeps the result text, or the lines that are not system or stream events. - claudeResultText returned raw NDJSON for a turn with no result event. It now returns the assistant text, and raw text only when there is no JSON. - The watchdog did not recheck `started` after the lock probe, so output arriving during the probe could still be killed as stuck. - A missing transcript path still suppressed the stall in stream mode, though the stream signal never reads the transcript. The guard now applies only to plain-text spawns. Live walk after the fix: 3 of 3 stuck at ~8.4s; positive control not killed. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: dart Agent-Model: claude/claude-haiku-5-5/default --- cli/__tests__/first-output-watchdog.test.mjs | 59 +++++++++++++++++--- cli/src/lib/adapters/claude.js | 52 +++++++++++++---- cli/src/lib/first-output-watchdog.js | 13 +++-- 3 files changed, 100 insertions(+), 24 deletions(-) diff --git a/cli/__tests__/first-output-watchdog.test.mjs b/cli/__tests__/first-output-watchdog.test.mjs index 9d1d722612..309140b090 100644 --- a/cli/__tests__/first-output-watchdog.test.mjs +++ b/cli/__tests__/first-output-watchdog.test.mjs @@ -40,7 +40,7 @@ const { consoleLocked, spawnStalledError, watchFirstOutput, firstOutputWindowMs, DEFAULT_FIRST_OUTPUT_WINDOW_MS, physicalPath, killStallSurvivors, isModelProgressLine, } = await import('../src/lib/first-output-watchdog.js'); -const { claudeResultText } = await import('../src/lib/adapters/claude.js'); +const { claudeResultText, claudeFailureTail } = await import('../src/lib/adapters/claude.js'); const watchStream = (args) => watchFirstOutput({ ...args, modelStream: true }); const { classifySpawnFailure, spawnRetryPolicy, SPAWN_FAILURE_CLASS } = await import('../src/lib/spawn-retry.js'); @@ -88,10 +88,11 @@ describe('the claude adapter under the first-output watchdog', () => { expect(classifySpawnFailure(error)).toBe(SPAWN_FAILURE_CLASS.STALLED); }); - test('a missing transcript alone does not kill: the path may be wrong, so it takes a locked console', async () => { + test('a model reply is not killed when the transcript path is wrong', async () => { const { value, error, child } = await spawnClaude((proc) => { + proc.stdout.emit('data', Buffer.from('{"type":"assistant","message":{}}\n')); setTimeout(() => { - proc.stdout.emit('data', Buffer.from('done quietly')); + proc.stdout.emit('data', Buffer.from('{"type":"result","result":"done quietly"}\n')); proc.emit('close', 0); }, 150); }, { windowMs: 50, locked: false }); @@ -493,7 +494,7 @@ describe('the model-stream watchdog (stream-json, TASK-710)', () => { fs.appendFileSync(transcriptPath, '{"type":"user","retry":true}\n'); await new Promise((resolve) => setTimeout(resolve, 120)); expect(onStall).toHaveBeenCalledTimes(1); - expect(onStall.mock.calls[0][0]).toBe('stuck: no model output after the first transcript row'); + expect(onStall.mock.calls[0][0]).toBe('stuck: no model output inside the window'); }); test('a stderr warning at spawn is not life in stream mode', async () => { @@ -503,7 +504,7 @@ describe('the model-stream watchdog (stream-json, TASK-710)', () => { watchStream({ proc, windowMs: 50, transcriptPath, onStall, lockProbe: async () => false }); proc.stderr.emit('data', Buffer.from('[claude-code:unrecognized_model] {"model":"x"}\n')); await new Promise((resolve) => setTimeout(resolve, 120)); - expect(onStall).toHaveBeenCalledWith('stuck: no model output after the first transcript row'); + expect(onStall).toHaveBeenCalledWith('stuck: no model output inside the window'); }); test('a model event split across chunks still counts as life', async () => { @@ -527,12 +528,25 @@ describe('the model-stream watchdog (stream-json, TASK-710)', () => { expect(onStall).toHaveBeenCalledWith('console locked'); }); - test('a missing transcript alone does not kill in stream mode either', async () => { + test('a missing transcript does not hide a silent provider in stream mode', async () => { const proc = streamProc(); const onStall = jest.fn(); - const w = watchStream({ proc, windowMs: 50, transcriptPath: path.join(sessionsTmpDir, 'absent.jsonl'), modelStream: true, onStall, lockProbe: async () => false }); + watchStream({ proc, windowMs: 50, transcriptPath: path.join(sessionsTmpDir, 'absent.jsonl'), onStall, lockProbe: async () => false }); await new Promise((resolve) => setTimeout(resolve, 120)); - w.stop(); + expect(onStall).toHaveBeenCalledWith('stuck: no model output inside the window'); + }); + + test('model output that arrives while the lock probe runs still counts as life', async () => { + const proc = streamProc(); + const transcriptPath = transcriptWithFirstRow(); + const onStall = jest.fn(); + let releaseProbe; + const lockProbe = () => new Promise((resolve) => { releaseProbe = () => resolve(false); }); + watchStream({ proc, windowMs: 50, transcriptPath, onStall, lockProbe }); + await new Promise((resolve) => setTimeout(resolve, 60)); + proc.stdout.emit('data', Buffer.from('{"type":"assistant","message":{}}\n')); + releaseProbe(); + await new Promise((resolve) => setTimeout(resolve, 20)); expect(onStall).not.toHaveBeenCalled(); }); }); @@ -547,7 +561,34 @@ describe('claudeResultText', () => { expect(claudeResultText(stdout)).toBe('final answer'); }); - test('returns plain text output as it arrived when there is no result event', () => { + test('returns plain text output as it arrived when there is no JSON at all', () => { expect(claudeResultText(' done quietly \n')).toBe('done quietly'); }); + + test('a turn with no result event returns its assistant text, never the raw NDJSON', () => { + const stdout = [ + '{"type":"system","subtype":"init"}', + '{"type":"assistant","message":{"content":[{"type":"text","text":"half "}]}}', + '{"type":"assistant","message":{"content":[{"type":"text","text":"an answer"}]}}', + ].join('\n'); + expect(claudeResultText(stdout)).toBe('half an answer'); + expect(claudeResultText('{"type":"system","subtype":"init"}')).toBe(''); + }); +}); + +describe('claudeFailureTail', () => { + test('keeps the terminal error past the init events that precede it', () => { + const init = `{"type":"system","subtype":"init","tools":["${'x'.repeat(5000)}"]}`; + const stdout = [init, '{"type":"stream_event","event":{}}', '{"type":"result","is_error":true,"result":"Claude AI usage limit reached"}'].join('\n'); + expect(claudeFailureTail(stdout)).toBe('Claude AI usage limit reached'); + }); + + test('without a result event, keeps the non-system lines and drops init and stream deltas', () => { + const stdout = [ + '{"type":"system","subtype":"init"}', + '{"type":"stream_event","event":{}}', + 'Error: usage limit reached', + ].join('\n'); + expect(claudeFailureTail(stdout)).toBe('Error: usage limit reached'); + }); }); diff --git a/cli/src/lib/adapters/claude.js b/cli/src/lib/adapters/claude.js index 7c0b198fbc..1d7b3d9ffe 100644 --- a/cli/src/lib/adapters/claude.js +++ b/cli/src/lib/adapters/claude.js @@ -500,17 +500,49 @@ const publicSkillPromptArgs = async (skillSources) => { // and the watchdog does not count those. const STREAM_ARGS = ['--output-format', 'stream-json', '--verbose', '--include-partial-messages']; -// The reply is the `result` event's text. Output with no result event is -// returned as it arrived, so a turn that never reaches one still gets its text. +const parseEvent = (line) => { + try { + const event = JSON.parse(line); + return event && typeof event === 'object' ? event : null; + } catch { + return null; + } +}; + +// The reply is the `result` event's text. A turn that reaches no result event +// (killed mid-stream, exit 0) gets the text of its assistant events, never the +// raw NDJSON. Output with no JSON lines at all is plain text and returned as it +// arrived. export const claudeResultText = (stdout) => { - const lines = String(stdout || '').split('\n'); - for (let i = lines.length - 1; i >= 0; i -= 1) { - try { - const event = JSON.parse(lines[i]); - if (event?.type === 'result') return String(event.result ?? '').trim(); - } catch { /* not an event line */ } + const text = String(stdout || ''); + const events = text.split('\n').map(parseEvent); + if (!events.some(Boolean)) return text.trim(); + for (let i = events.length - 1; i >= 0; i -= 1) { + if (events[i]?.type === 'result') return String(events[i].result ?? '').trim(); } - return String(stdout || '').trim(); + return events + .filter((event) => event?.type === 'assistant') + .flatMap((event) => event.message?.content || []) + .filter((block) => block?.type === 'text') + .map((block) => block.text) + .join('') + .trim(); +}; + +// The failure tail for a non-zero exit. Stream events are mostly system init +// and status lines, which would push the terminal error past the 2000-char cut, +// so keep the result text, or the lines that are not system or stream events. +export const claudeFailureTail = (stdout) => { + const text = String(stdout || ''); + const lines = text.split('\n'); + const events = lines.map(parseEvent); + if (!events.some(Boolean)) return text.trim(); + const result = [...events].reverse().find((event) => event?.type === 'result'); + if (String(result?.result ?? '').trim()) return String(result.result).trim(); + return lines + .filter((_, i) => !['system', 'stream_event'].includes(events[i]?.type)) + .join('\n') + .trim(); }; const runClaude = ({ @@ -604,7 +636,7 @@ const runClaude = ({ // reduced to one named field. The status, when the tail names one, is // attached as `error.status` so a refusal classifies by field rather than // by regex. - const detail = [stderr.trim(), stdout.trim()].filter(Boolean).join(' | '); + const detail = [stderr.trim(), claudeFailureTail(stdout)].filter(Boolean).join(' | '); return reject(adapterFailure('claude', detail, { credentials, exitCode: code, diff --git a/cli/src/lib/first-output-watchdog.js b/cli/src/lib/first-output-watchdog.js index 9b6ce55d4d..06c3b1ab52 100644 --- a/cli/src/lib/first-output-watchdog.js +++ b/cli/src/lib/first-output-watchdog.js @@ -157,14 +157,17 @@ export const watchFirstOutput = ({ const size = sizeOf(transcriptPath); if (started || (!modelStream && size > baseline)) return; const locked = await lockProbe().catch(() => null); - if (stopped) return; - // No transcript where one was expected may only mean the path is wrong, so - // that alone never kills a turn: it takes a locked console as well. - if (size < 0 && locked !== true) return; + // Output can arrive while the probe runs (ps or ioreg may take a second). + if (stopped || started) return; if (modelStream) { - onStall(locked ? 'console locked' : 'stuck: no model output after the first transcript row'); + // The model signal does not read the transcript, so a missing path cannot + // hide a hung provider here. + onStall(locked ? 'console locked' : 'stuck: no model output inside the window'); return; } + // No transcript where one was expected may only mean the path is wrong, so + // that alone never kills a turn: it takes a locked console as well. + if (size < 0 && locked !== true) return; onStall(locked ? 'console locked' : 'no transcript, stdout or stderr'); }, windowMs); timer?.unref?.(); From 8f7d340adb168ea38dbb7ddb0beff6f4a4703121 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 31 Oct 2026 16:12:33 -0700 Subject: [PATCH 3/3] fix(cli): raise a result flagged is_error as a failed turn, not a reply (TASK-710) CodeRabbit on #3108: claudeResultText returned the text of a result event even when is_error was set, so a failed turn that exits 0 posted its error text as the seat's reply. The result path now throws adapterFailure when is_error is true, outside the parse guard. Both the first turn and the session-recovery retry pass their spawn credentials, so the text is scrubbed like the exit-path failure tail. Tests cover the first turn, the recovery turn, and the unit function. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: dart Agent-Model: claude/claude-haiku-5-5/default --- cli/__tests__/adapters.claude.test.mjs | 22 ++++++++++++++++++++ cli/__tests__/first-output-watchdog.test.mjs | 5 +++++ cli/src/lib/adapters/claude.js | 14 ++++++++----- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/cli/__tests__/adapters.claude.test.mjs b/cli/__tests__/adapters.claude.test.mjs index 29c5d1f65f..4975f31ebf 100644 --- a/cli/__tests__/adapters.claude.test.mjs +++ b/cli/__tests__/adapters.claude.test.mjs @@ -177,6 +177,28 @@ describe('claude adapter — spawn()', () => { expect(res.text).toBe('recovered'); }); + test('a first turn whose result is an error is a failure, not a reply, even at exit 0', async () => { + const errorResult = '{"type":"result","subtype":"success","is_error":true,"result":"API Error: 500 upstream down"}\n'; + const { impl, calls } = makeSpawnImpl({ stdout: errorResult, code: 0 }); + await expect( + claude.spawn('x', { environment: { model: 'test-model' }, sessionId: 'sid-err', _spawnImpl: impl }), + ).rejects.toThrow(/API Error: 500 upstream down/); + expect(calls).toHaveLength(1); + }); + + test('a recovery turn whose result is an error is a failure, not a reply', async () => { + let call = 0; + const impl = () => { + call += 1; + if (call === 1) return fakeChild({ stderr: 'Error: Session ID abc-123 is already in use.', code: 1 }); + return fakeChild({ stdout: '{"type":"result","is_error":true,"result":"API Error: 529 overloaded"}\n', code: 0 }); + }; + await expect( + claude.spawn('x', { environment: { model: 'test-model' }, sessionId: 'abc-123', _spawnImpl: impl }), + ).rejects.toThrow(/API Error: 529 overloaded/); + expect(call).toBe(2); + }); + test('does NOT self-heal on generic spawn errors — only session-ID poison-pills', async () => { const { impl, calls } = makeSpawnImpl({ stderr: 'some other error', code: 1 }); await expect( diff --git a/cli/__tests__/first-output-watchdog.test.mjs b/cli/__tests__/first-output-watchdog.test.mjs index 309140b090..a4c3a1ee01 100644 --- a/cli/__tests__/first-output-watchdog.test.mjs +++ b/cli/__tests__/first-output-watchdog.test.mjs @@ -561,6 +561,11 @@ describe('claudeResultText', () => { expect(claudeResultText(stdout)).toBe('final answer'); }); + test('a result flagged is_error throws instead of returning its text as the reply', () => { + const stdout = '{"type":"result","subtype":"success","is_error":true,"result":"API Error: 500"}'; + expect(() => claudeResultText(stdout)).toThrow(/API Error: 500/); + }); + test('returns plain text output as it arrived when there is no JSON at all', () => { expect(claudeResultText(' done quietly \n')).toBe('done quietly'); }); diff --git a/cli/src/lib/adapters/claude.js b/cli/src/lib/adapters/claude.js index 1d7b3d9ffe..e426c14a10 100644 --- a/cli/src/lib/adapters/claude.js +++ b/cli/src/lib/adapters/claude.js @@ -512,13 +512,17 @@ const parseEvent = (line) => { // The reply is the `result` event's text. A turn that reaches no result event // (killed mid-stream, exit 0) gets the text of its assistant events, never the // raw NDJSON. Output with no JSON lines at all is plain text and returned as it -// arrived. -export const claudeResultText = (stdout) => { +// arrived. A result flagged `is_error` is a failed turn, whatever its exit code, +// so it is raised here, outside the parse guard, and never returned as a reply. +export const claudeResultText = (stdout, { credentials = [] } = {}) => { const text = String(stdout || ''); const events = text.split('\n').map(parseEvent); if (!events.some(Boolean)) return text.trim(); for (let i = events.length - 1; i >= 0; i -= 1) { - if (events[i]?.type === 'result') return String(events[i].result ?? '').trim(); + if (events[i]?.type !== 'result') continue; + const reply = String(events[i].result ?? '').trim(); + if (events[i].is_error === true) throw adapterFailure('claude', reply, { credentials }); + return reply; } return events .filter((event) => event?.type === 'assistant') @@ -1326,7 +1330,7 @@ export default { const contextTokens = claudeContextTokens({ cwd: physicalPath(ctx.cwd), sessionId, configDir: claudeConfigDirFor({ ...env, HOME: childHome || env?.HOME }), }); - return { text: claudeResultText(stdout), newSessionId: sessionId, contextTokens }; + return { text: claudeResultText(stdout, { credentials: spawnCredentials(spawnCtx) }), newSessionId: sessionId, contextTokens }; } catch (err) { // A public seat's refusal is not a session-store failure, whatever its // message happens to contain: it must reach the run loop, never the @@ -1389,7 +1393,7 @@ export default { cwd: physicalPath(ctx.cwd), sessionId: freshId, configDir: claudeConfigDirFor({ ...retry.env, HOME: retry.childHome || retry.env?.HOME }), }); - return { text: claudeResultText(stdout), newSessionId: freshId, contextTokens }; + return { text: claudeResultText(stdout, { credentials: spawnCredentials(retryCtx) }), newSessionId: freshId, contextTokens }; } throw err; } finally {