the recovery could never have fired — fix its condition
Shipped e6422052 an hour ago claiming a chat that cannot be resumed would recover on
the next message. It would not have. Caught before it was pulled, by tracing the two
error strings in a screenshot back to their sources instead of trusting the change.
`sawOutput` was set on ANY SDK message. But a turn that cannot load its conversation
still emits a terminal `result` with `is_error` and no text — that is exactly what
reaches the user as "Claude Code returned an error", one of the two lines in the
report. So the single event that proves the resume failed was being counted as proof
it had worked, and `clearResume` could never be true in the one situation it exists
for.
The flag now ignores `error` and counts everything else, as `countsAsOutput` — a named
function with tests rather than an inline `!==`, because this is the second time this
rule has been wrong and both times it was wrong silently.
`stopped` deliberately counts: an interrupted turn was running, so its conversation
loaded, and discarding a transcript because somebody pressed stop quickly would be a
new bug of the same shape.
Five more tests, including the reported failure end to end (`['error']` -> give up on
the resume) and its opposite (`['assistant:text', 'error']` -> keep it).
Still does not explain WHY the resume fails. Also worth recording: my evidence that
"resume works by hand" may be worth nothing — `claude --resume` with an id it cannot
find may simply start a fresh session and exit 0, which would look identical to
success in both tests I ran. The new spawn log is what will settle it.
tsgo clean, 950 pass / 7 fail unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -6,7 +6,7 @@ import { query, type Query, type SpawnOptions, type SpawnedProcess } from '@anth
|
||||
import type { ChatEvent, PromptImage } from '../../api/chat/types';
|
||||
import type { ClaudeSpawnParams, ClaudeSpawnStreamingParams, ClaudeCodeResult, LiveClaudeSession } from '../protocol';
|
||||
import { setClaudeSession, clearClaudeSession, getClaudeSession } from './state';
|
||||
import { diagnoseExit, stderrReason } from './exit-diagnosis';
|
||||
import { countsAsOutput, diagnoseExit, stderrReason } from './exit-diagnosis';
|
||||
import { createParseState, processMessage } from './stream-parser';
|
||||
import { spawnClaudeAsMember } from './spawn-as-member';
|
||||
import { claudeBinIn } from '@@/os-user-claude';
|
||||
@@ -620,13 +620,12 @@ function createSession(params: ClaudeSpawnStreamingParams, onEvent: (event: Chat
|
||||
// Any event at all is a sign of life, so push the stall deadline back. Placed after the branch
|
||||
// above so a terminal event disarms rather than re-arms it.
|
||||
armStall(session);
|
||||
// See `countsAsOutput`: an error is the one event that does not prove the harness was working.
|
||||
if (countsAsOutput(event.type)) session.sawOutput = true;
|
||||
session.emit(event);
|
||||
};
|
||||
try {
|
||||
for await (const msg of q as AsyncGenerator<Record<string, unknown>>) {
|
||||
// One message — any message — means the child got far enough to talk. That distinguishes a resume
|
||||
// that could not even load from a turn that broke halfway, which want opposite handling.
|
||||
session.sawOutput = true;
|
||||
processMessage(msg, state, {
|
||||
onEvent: emit,
|
||||
onSessionId: (id: string) => setClaudeSession(sessionKey, id, params.userId),
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { diagnoseExit, stderrReason } from './exit-diagnosis';
|
||||
import { countsAsOutput, diagnoseExit, stderrReason } from './exit-diagnosis';
|
||||
|
||||
// The bug these encode: a member's chat answered "Claude Code process exited with code 1" four times in a
|
||||
// row and could never recover, because every message retried the same `--resume` id and nothing ever
|
||||
@@ -55,3 +55,39 @@ describe('stderrReason', () => {
|
||||
expect(stderrReason('x'.repeat(5000)).length).toBeLessThanOrEqual(300);
|
||||
});
|
||||
});
|
||||
|
||||
// ── What counts as the harness having worked ──
|
||||
//
|
||||
// These exist because the first version of this rule was wrong in the only case it was written for, and
|
||||
// nothing failed: a broken resume emits a terminal error result, that was counted as output, and the
|
||||
// recovery it was supposed to trigger could never fire.
|
||||
|
||||
describe('countsAsOutput', () => {
|
||||
test('an error does NOT count — it is what a failed resume emits', () => {
|
||||
expect(countsAsOutput('error')).toBe(false);
|
||||
});
|
||||
|
||||
test('real work counts', () => {
|
||||
for (const type of ['assistant:text', 'assistant:delta', 'tool:start', 'tool:result', 'result']) {
|
||||
expect(countsAsOutput(type)).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
test('a stopped turn counts — it was running, so the conversation loaded', () => {
|
||||
// Clearing someone's transcript because they pressed stop quickly would be its own bug.
|
||||
expect(countsAsOutput('stopped')).toBe(true);
|
||||
});
|
||||
|
||||
// The end-to-end shape of the reported failure: no real output, an error result, a resuming turn.
|
||||
test('the reported failure now resolves to "give up on the resume"', () => {
|
||||
const events = ['error'];
|
||||
const sawOutput = events.some(countsAsOutput);
|
||||
expect(diagnoseExit({ sawOutput, resumeId: '49527b33', stderrTail: '' }).clearResume).toBe(true);
|
||||
});
|
||||
|
||||
test('and a turn that spoke before failing does not', () => {
|
||||
const events = ['assistant:text', 'error'];
|
||||
const sawOutput = events.some(countsAsOutput);
|
||||
expect(diagnoseExit({ sawOutput, resumeId: '49527b33', stderrTail: '' }).clearResume).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -17,6 +17,24 @@
|
||||
* turn that broke later. Clearing the resume on the second would trade one bad turn for a lost
|
||||
* conversation, which is the wrong direction.
|
||||
*/
|
||||
/**
|
||||
* Whether an event proves the harness was actually working.
|
||||
*
|
||||
* This is the flag's definition and the first version got it backwards in exactly the case it exists for.
|
||||
* It counted ANY SDK message as output — but a turn that cannot load its conversation still emits a
|
||||
* terminal `result` with `is_error` and no text, which is what reaches the user as "Claude Code returned an
|
||||
* error". So the very event that proves the resume failed was being read as proof it succeeded, and the
|
||||
* recovery could never fire. Caught by tracing the two error strings in a screenshot to their sources
|
||||
* rather than by testing, which is luck; hence this being a named function with tests rather than an
|
||||
* inline `!==`.
|
||||
*
|
||||
* `stopped` DOES count. A turn someone interrupted was running, which means the conversation loaded — and
|
||||
* discarding the transcript because somebody pressed stop quickly would be its own bug.
|
||||
*/
|
||||
export function countsAsOutput(eventType: string): boolean {
|
||||
return eventType !== 'error';
|
||||
}
|
||||
|
||||
export type ExitDiagnosis = {
|
||||
/** Forget the stored transcript id, so the next message starts a new session instead of failing again. */
|
||||
clearResume: boolean;
|
||||
|
||||
Reference in New Issue
Block a user