a dead claude process no longer hangs the chat forever
the sdk runs two independent tasks per session: the consumer loop (`for await (const msg of q)`) and an input pump that writes the queue to the child's stdin. the consumer loop's `finally` is what removes a session from the map — but when the CHILD dies it is the input pump that fails, with `ProcessTransport is not ready for writing`, and that rejection neither ends the consumer loop nor is caught anywhere. so the loop stayed parked on a stream with no writer, `finally` never ran, the session stayed in the map, and spawnClaudeStreaming handed every later turn to the same corpse. each one pushed a message onto a queue nobody drained: no error, no result, no timeout. the client spun forever and the only trace was one unhandledRejection line in the sidecar log. observed on the host today; the only cure was pm2 restart officer-claude-code. a member's turn already supplied its own spawn function because it has to go through setpriv. the owner had none, and therefore no place to observe the child — which is exactly why its death was invisible. so give the owner one too, and wrap both in watchChild: on exit or error, drop the session from the map and, if a turn was in flight, tell the client. emitting only while generating is deliberate. a child that exits between turns is invisible to the user, and an error bubble arriving in a chat nobody is looking at would be noise — dropping the map entry is the whole repair there, because the next turn builds a fresh session and resumes the transcript by id. the stall timer now tears the session down as well. it used to keep it — "it may still be working, and the next turn resumes it" — which is right for a slow agent and wrong for a wedged one: the session stayed broken, so every later turn hung the same way and "send again to continue" was a lie. sessions with background tasks outstanding are still left alone, since a job can be silent far longer than ten minutes and still land its notification. verified live against the real manager: killed the child mid-turn, saw the error surface and the next turn rebuild the session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,7 +1,8 @@
|
||||
import { existsSync } from 'node:fs';
|
||||
import { spawn } from 'node:child_process';
|
||||
import { homedir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { query, type Query } from '@anthropic-ai/claude-agent-sdk';
|
||||
import { query, type Query, type SpawnOptions, type SpawnedProcess } from '@anthropic-ai/claude-agent-sdk';
|
||||
import type { ChatEvent, PromptImage } from '../../api/chat/types';
|
||||
import type { ClaudeSpawnParams, ClaudeSpawnStreamingParams, ClaudeCodeResult, LiveClaudeSession } from '../protocol';
|
||||
import { setClaudeSession, clearClaudeSession, getClaudeSession } from './state';
|
||||
@@ -216,6 +217,79 @@ const COMPACT_STALL_TIMEOUT_MS = 20 * 60 * 1000;
|
||||
|
||||
const sessions = new Map<string, PersistentSession>();
|
||||
|
||||
/**
|
||||
* The owner's spawn — what the SDK would do by default, written out so it can be wrapped by `watchChild`.
|
||||
*
|
||||
* A member's turn already supplies its own (`spawnClaudeAsMember`) because it has to go through `setpriv`.
|
||||
* The owner had no such function and therefore no place to observe the child, which is precisely why its
|
||||
* death was invisible. The existence check mirrors the SDK's default: without it a bad `CLAUDE_BIN` fails
|
||||
* as a write to a closed pipe several seconds later, naming nothing useful.
|
||||
*/
|
||||
function spawnClaudeAsOwner({ command, args, cwd, env, signal }: SpawnOptions): SpawnedProcess {
|
||||
if (!existsSync(command)) throw new Error(`claude CLI not found at ${command}`);
|
||||
const child = spawn(command, args, { cwd, env, signal, stdio: ['pipe', 'pipe', 'pipe'] });
|
||||
// Non-null by construction: 'pipe' on all three. Mirrors the cast in `spawn-as-member.ts`.
|
||||
return child as unknown as SpawnedProcess;
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop a session whose `claude` process is gone, and tell whoever was waiting.
|
||||
*
|
||||
* This closes the hole that made a dead agent look like a slow one **forever**. The SDK runs two
|
||||
* independent tasks per session: the consumer loop (`for await (const msg of q)`) and an input pump that
|
||||
* writes the queue to the child's stdin. The consumer loop's `finally` is what removes a session from the
|
||||
* map — but when the CHILD dies, it is the input pump that fails, with `ProcessTransport is not ready for
|
||||
* writing`, and that rejection neither ends the consumer loop nor is caught anywhere. So the loop stayed
|
||||
* parked on a stream with no writer, `finally` never ran, the session stayed in the map, and
|
||||
* `spawnClaudeStreaming` handed every later turn to the same corpse. Each one pushed a message onto a
|
||||
* queue nobody drained: no error, no result, no timeout. The client spun forever, and the only trace was
|
||||
* an `unhandledRejection` line in the sidecar log.
|
||||
*
|
||||
* Observed on 2026-08-14; the session had to be cleared with `pm2 restart officer-claude-code`.
|
||||
*
|
||||
* Emitting only while a turn is in flight is deliberate. A child that exits between turns is invisible to
|
||||
* the user, and an error bubble arriving in a chat nobody is looking at would be noise — dropping the map
|
||||
* entry is the whole repair there, because the next turn then builds a fresh session and resumes the
|
||||
* transcript by id.
|
||||
*/
|
||||
function dropDeadSession(session: PersistentSession, detail: string): void {
|
||||
// Identity, not key: a later turn may have already replaced this entry, and killing its session because
|
||||
// its predecessor's process exited would break the live conversation instead of a dead one.
|
||||
if (sessions.get(session.sessionKey) !== session) return;
|
||||
sessions.delete(session.sessionKey);
|
||||
if (session.idleTimer) clearTimeout(session.idleTimer);
|
||||
if (session.stallTimer) clearTimeout(session.stallTimer);
|
||||
session.idleTimer = undefined;
|
||||
session.stallTimer = undefined;
|
||||
|
||||
const wasGenerating = session.isGenerating;
|
||||
session.isGenerating = false;
|
||||
// A deliberate teardown (kill / idle GC) aborts first, and its exit is not news.
|
||||
if (session.abort.signal.aborted) return;
|
||||
|
||||
console.error(`[claude:exit:${session.sessionKey}] ${detail}`);
|
||||
if (!wasGenerating) return;
|
||||
session.emit({
|
||||
type: 'error',
|
||||
message: 'The agent process exited unexpectedly. Your conversation is safe — send again to continue.',
|
||||
});
|
||||
}
|
||||
|
||||
/** Wrap a spawn so the session self-heals when its child goes away. */
|
||||
function watchChild(
|
||||
inner: (options: SpawnOptions) => SpawnedProcess,
|
||||
session: PersistentSession,
|
||||
): (options: SpawnOptions) => SpawnedProcess {
|
||||
return (options: SpawnOptions): SpawnedProcess => {
|
||||
const child = inner(options);
|
||||
child.on('exit', (code, signal) =>
|
||||
dropDeadSession(session, `claude exited (code=${code ?? 'null'} signal=${signal ?? 'null'})`),
|
||||
);
|
||||
child.on('error', (err: Error) => dropDeadSession(session, `claude failed to start: ${err.message}`));
|
||||
return child;
|
||||
};
|
||||
}
|
||||
|
||||
/** A hand-rolled async iterable we can push turns onto and close on teardown. */
|
||||
function makeInputQueue() {
|
||||
const buf: SdkUserMessage[] = [];
|
||||
@@ -273,13 +347,25 @@ function armStall(session: PersistentSession): void {
|
||||
session.isGenerating = false;
|
||||
session.compactStartedAt = undefined;
|
||||
session.interrupted = false;
|
||||
if (session.pendingTasks.size === 0) armIdle(session);
|
||||
session.emit({
|
||||
type: 'error',
|
||||
message: compacting
|
||||
? `Compaction has produced nothing for ${waited} minutes and has been given up on. Your conversation is safe — send again to continue.`
|
||||
: `The agent has produced nothing for ${waited} minutes and has been given up on. Your conversation is safe — send again to continue.`,
|
||||
});
|
||||
// A background job can be silent for far longer than this and still land its `task_notification`, so
|
||||
// a stall with tasks outstanding keeps the old behaviour and leaves the session alone.
|
||||
if (session.pendingTasks.size > 0) return;
|
||||
|
||||
// Otherwise tear it down rather than leaving it armed for the next turn.
|
||||
//
|
||||
// This used to keep the session — "it may still be working, and the next turn resumes it" — which is
|
||||
// the right instinct for a SLOW agent and exactly wrong for a wedged one: a session that has said
|
||||
// nothing for ten minutes because its transport is broken stays broken, so every later turn hangs
|
||||
// the same way and the message above ("send again to continue") is a lie. Killing costs a resume,
|
||||
// which is what the message already promises; the transcript id survives in `claudeSessions`, so the
|
||||
// next turn continues the same conversation.
|
||||
killClaudeSession(session.sessionKey, session.userId);
|
||||
},
|
||||
compacting ? COMPACT_STALL_TIMEOUT_MS : STALL_TIMEOUT_MS,
|
||||
);
|
||||
@@ -361,12 +447,18 @@ function createSession(params: ClaudeSpawnStreamingParams, onEvent: (event: Chat
|
||||
// authoritative for settings, and `~` is decided by the HOME the process gets. Pointing the SDK at a
|
||||
// member's binary while spawning as the service user would read the OWNER'S settings and credential
|
||||
// while executing the member's code — the worst of both, and it would look like it worked.
|
||||
//
|
||||
// Both branches go through `watchChild`: the owner's spawn exists only so there is something to
|
||||
// wrap (see `spawnClaudeAsOwner`), because a child nobody watches is a session that can die silently.
|
||||
...(params.member
|
||||
? {
|
||||
pathToClaudeCodeExecutable: claudeBinIn(params.member.home),
|
||||
spawnClaudeCodeProcess: spawnClaudeAsMember(params.member),
|
||||
spawnClaudeCodeProcess: watchChild(spawnClaudeAsMember(params.member), session),
|
||||
}
|
||||
: { pathToClaudeCodeExecutable: CLAUDE_BIN }),
|
||||
: {
|
||||
pathToClaudeCodeExecutable: CLAUDE_BIN,
|
||||
spawnClaudeCodeProcess: watchChild(spawnClaudeAsOwner, session),
|
||||
}),
|
||||
settingSources: ['user', 'project', 'local'],
|
||||
env: cleanEnv as Record<string, string>,
|
||||
stderr: (d: string) => {
|
||||
|
||||
Reference in New Issue
Block a user