From ca302c6416e2dfae34af267c5fe69e63e59cd2a6 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Thu, 3 Sep 2026 00:03:05 -0700 Subject: [PATCH] fix(claude): retain unproven SDK exits --- .../claude-structured-session-acquisition.ts | 50 ++++++-------- .../claude-structured-session-adapter.test.ts | 43 ++++++++++++ .../claude-structured-session-adapter.ts | 69 ++++++++++++++----- .../claude/claude-structured-session-close.ts | 35 +++++++++- ...claude-structured-session-recovery.test.ts | 54 +++++++++++++++ .../claude/claude-structured-session-state.ts | 4 ++ 6 files changed, 203 insertions(+), 52 deletions(-) diff --git a/src/main/claude/claude-structured-session-acquisition.ts b/src/main/claude/claude-structured-session-acquisition.ts index 56942f1b3a2..e09f52c35b9 100644 --- a/src/main/claude/claude-structured-session-acquisition.ts +++ b/src/main/claude/claude-structured-session-acquisition.ts @@ -1,6 +1,5 @@ import { AgentSessionAcquisitionExitUnprovenError, - AgentSessionAcquisitionRootExitObservedError, AgentSessionPreSpawnError } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import type { @@ -8,10 +7,7 @@ import type { StructuredAgentSessionAcquireInput } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' -import { - openClaudeStreamJsonConnection, - type ClaudeStreamJsonConnection -} from './claude-stream-json-connection' +import { openClaudeStreamJsonConnection } from './claude-stream-json-connection' import { buildClaudePermissionCallbacks } from './claude-structured-inbound-control' import { resolveClaudeReplayWaiter } from './claude-structured-dispatch' import { @@ -45,7 +41,8 @@ import { } from './claude-structured-session-state' import { closeClaudePublishedSessionForDeps, - closeClaudeSession + closeClaudeSession, + claudeAcquisitionCleanupError } from './claude-structured-session-close' import { readClaudeTranscriptEntryUuid } from './claude-tui-exit' @@ -61,27 +58,6 @@ type AcquireCallbacks = { handleExit: (sessionId: string, attempt: ClaudeAcquisitionAttempt, error: Error) => void } -/** - * Which failure a Claude child that would not close cleanly actually is. - * - * The lease is keyed on the root's pid and start time, so a first-hand root exit - * releases it and the CLI's own diagnostic reaches the user. A descendant seen - * still alive, or a root Orca never saw leave, stays unproven and keeps the - * reservation — releasing there is the orphaning this proof exists to prevent. - */ -function claudeAcquisitionCleanupError( - connection: ClaudeStreamJsonConnection | null | undefined, - cause: unknown -): Error { - const verdict = connection?.exitVerdict - if (verdict?.root === 'processless') { - return new AgentSessionPreSpawnError(cause) - } - return verdict?.root === 'exited' && verdict.tree === 'unverifiable' - ? new AgentSessionAcquisitionRootExitObservedError(cause) - : new AgentSessionAcquisitionExitUnprovenError(cause) -} - export async function acquireClaudeSession({ input, deps, @@ -156,8 +132,19 @@ export async function acquireClaudeSession({ new Error(`claude session ${sessionId} could not be stopped`) ) } - // Any earlier session is closed or already gone: its exit says nothing about this start. - exits.delete(sessionId) + // A first-hand exit that has not yet proved its full tree still owns a cleanup + // obligation; never let a new acquisition hide that evidence by omission. + const retainedExit = exits.get(sessionId) + if (retainedExit) { + const firstProof = retainedExit.closePromise ? await retainedExit.closePromise : false + const proven = firstProof || (await retainedExit.connection.close().catch(() => false)) + if (!proven) { + throw claudeAcquisitionCleanupError(retainedExit.connection, retainedExit.error) + } + // The old child is superseded by this acquisition. It has a true proof, + // so discard its lifecycle evidence without publishing a stale recovery. + exits.delete(sessionId) + } acquisitions.assertCurrent(sessionId, attempt) // Closing persists the prior connection's final leaf, so launch validates that durable head. const launchIdentity = priorSession @@ -309,6 +296,7 @@ export async function releaseClaudeAcquisition(input: { sessions: Map acquisitions: ClaudeAcquisitionRegistry exits: Map + onExitProven?: (sessionId: string, exit: ClaudeSessionExit) => Promise persistHandle?: ClaudeStructuredSessionAdapterDeps['persistHandle'] onEvent?: ClaudeStructuredSessionAdapterDeps['onEvent'] }): Promise { @@ -319,7 +307,9 @@ export async function releaseClaudeAcquisition(input: { const firstProof = exit.closePromise ? await exit.closePromise : false // A failed exit-path proof is retained as evidence, not as a terminal result; // a release retry must drive a fresh tree verification on the same connection. - if (firstProof || (await exit.connection.close())) { + const retriedProof = firstProof || (await exit.connection.close()) + if (retriedProof) { + await input.onExitProven?.(input.sessionId, exit) // Keep the first-hand exit evidence indexed until the tree proof succeeds; // a failed close must be retryable and cannot look like an absent session. input.exits.delete(input.sessionId) diff --git a/src/main/claude/claude-structured-session-adapter.test.ts b/src/main/claude/claude-structured-session-adapter.test.ts index ce14381c10f..71b76a26183 100644 --- a/src/main/claude/claude-structured-session-adapter.test.ts +++ b/src/main/claude/claude-structured-session-adapter.test.ts @@ -676,6 +676,49 @@ describe('ClaudeStructuredSessionAdapter acquisition cleanup', () => { ) expect(connection.close).toHaveBeenCalledTimes(2) }) + + it('keeps shutdown pending until a retained unexpected-exit proof settles', async () => { + const claude = fakeClaude() + const adapter = await acquired(claude) + const connection = claude.connections[0] + const proof = Promise.withResolvers() + connection.close = vi + .fn<() => Promise>() + .mockImplementationOnce(() => proof.promise) + .mockResolvedValueOnce(true) as unknown as FakeConnection['close'] + + connection.handlers.onExit?.(new Error('crashed')) + await tick() + let settled = false + const closing = adapter.closeAll().then(() => { + settled = true + }) + await tick() + expect(settled).toBe(false) + + proof.resolve(false) + await expect(closing).resolves.toBeUndefined() + expect(connection.close).toHaveBeenCalledTimes(2) + }) + + it('does not claim shutdown success for a retained false exit proof', async () => { + const claude = fakeClaude() + const events: ClaudeStructuredSessionEvent[] = [] + const adapter = await acquired(claude, {}, events) + const connection = claude.connections[0] + connection.close = vi + .fn<() => Promise>() + .mockResolvedValue(false) as unknown as FakeConnection['close'] + + connection.handlers.onExit?.(new Error('crashed')) + await tick() + + await expect(adapter.closeAll()).rejects.toThrow( + 'claude structured session shutdown could not prove every child stopped' + ) + expect(events.filter((event) => event.type === 'ended')).toEqual([]) + expect(connection.close).toHaveBeenCalledTimes(4) + }) }) describe('ClaudeStructuredSessionAdapter prompts', () => { diff --git a/src/main/claude/claude-structured-session-adapter.ts b/src/main/claude/claude-structured-session-adapter.ts index ee7eb5ea3c6..e133bd3d788 100644 --- a/src/main/claude/claude-structured-session-adapter.ts +++ b/src/main/claude/claude-structured-session-adapter.ts @@ -81,24 +81,48 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda // An exit callback is root evidence only; the retained tree proof must run // before the host releases and reacquires this exact child. const closePromise = session.connection.close().catch(() => false) - this.exits.set(sessionId, { connection: session.connection, error, closePromise }) - // Persist the transcript-derived cursor before publishing the lifecycle - // event that lets the host release and reacquire this exact child. + const exit: ClaudeSessionExit = { + connection: session.connection, + session, + error, + closePromise + } + this.exits.set(sessionId, exit) void closePromise - .then(() => this.persistSessionHandle(sessionId, session)) + .then((proven) => (proven ? this.settleUnexpectedExit(sessionId, exit) : undefined)) .catch(() => undefined) - .then(() => { - const ended: ClaudeStructuredSessionEvent = { - type: 'ended', - sessionId, - reason: error.message, - cause: 'unexpected-exit', - fence: session.fence, - acquisitionGeneration: session.acquisitionGeneration - } - this.emit(session, session.events, ended) - settleClaudeExitedSession(session) - }) + } + + /** Lifecycle recovery is published only after the child tree proof is true. */ + private settleUnexpectedExit(sessionId: string, exit: ClaudeSessionExit): Promise { + exit.settlementPromise ??= (async () => { + if (this.exits.get(sessionId) !== exit) { + settleClaudeExitedSession(exit.session) + return + } + // Persist the transcript-derived cursor before publishing the lifecycle + // event that lets the host release and reacquire this exact child. + await this.persistSessionHandle(sessionId, exit.session).catch(() => undefined) + if (this.exits.get(sessionId) !== exit) { + settleClaudeExitedSession(exit.session) + return + } + this.exits.delete(sessionId) + const ended: ClaudeStructuredSessionEvent = { + type: 'ended', + sessionId, + reason: exit.error.message, + cause: 'unexpected-exit', + fence: exit.session.fence, + acquisitionGeneration: exit.session.acquisitionGeneration + } + try { + this.emit(exit.session, exit.session.events, ended) + } finally { + settleClaudeExitedSession(exit.session) + } + })() + return exit.settlementPromise } private async persistSessionHandle(sessionId: string, session: ClaudeSession): Promise { @@ -185,12 +209,16 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda sessions: this.sessions, acquisitions: this.acquisitions, exits: this.exits, + onExitProven: (sessionId, exit) => this.settleUnexpectedExit(sessionId, exit), ...(this.deps.persistHandle ? { persistHandle: this.deps.persistHandle } : {}), ...(this.deps.onEvent ? { onEvent: this.deps.onEvent } : {}) }) - closeSession = (sessionId: string): Promise => - closeClaudeSession({ + closeSession = (sessionId: string): Promise => { + if (this.exits.has(sessionId)) { + return this.releaseAcquisition({ sessionId }) + } + return closeClaudeSession({ sessionId, sessions: this.sessions, acquisitions: this.acquisitions, @@ -198,12 +226,15 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda ...(this.deps.readTranscriptLeaf ? { readTranscriptLeaf: this.deps.readTranscriptLeaf } : {}), ...(this.deps.onEvent ? { onEvent: this.deps.onEvent } : {}) }) + } closeAll = (): Promise => closeAllClaudeSessions({ sessions: this.sessions, acquisitions: this.acquisitions, - closeSession: this.closeSession + exits: this.exits, + closeSession: this.closeSession, + closeExit: (sessionId) => this.releaseAcquisition({ sessionId }) }) private session(sessionId: string): ClaudeSession { diff --git a/src/main/claude/claude-structured-session-close.ts b/src/main/claude/claude-structured-session-close.ts index c1b07bc24ca..b8f17c4ee97 100644 --- a/src/main/claude/claude-structured-session-close.ts +++ b/src/main/claude/claude-structured-session-close.ts @@ -1,12 +1,32 @@ import type { ClaudeAcquisitionRegistry, ClaudeSession, + ClaudeSessionExit, ClaudeStructuredSessionEvent } from './claude-structured-session-state' import { cancelClaudeAcquisitionAttempt } from './claude-structured-session-state' +import { + AgentSessionAcquisitionExitUnprovenError, + AgentSessionAcquisitionRootExitObservedError, + AgentSessionPreSpawnError +} from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import type { ClaudeStreamJsonConnection } from './claude-stream-json-connection' import { closeProcessRegistry } from '../../shared/child-process/close-process-registry' import { readClaudeTranscriptLeafWithReproof } from './claude-transcript-branch-proof' +export function claudeAcquisitionCleanupError( + connection: ClaudeStreamJsonConnection | null | undefined, + cause: unknown +): Error { + const verdict = connection?.exitVerdict + if (verdict?.root === 'processless') { + return new AgentSessionPreSpawnError(cause) + } + return verdict?.root === 'exited' && verdict.tree === 'unverifiable' + ? new AgentSessionAcquisitionRootExitObservedError(cause) + : new AgentSessionAcquisitionExitUnprovenError(cause) +} + export function settleClaudeDispatchWaiters(session: ClaudeSession): void { for (const waiter of session.dispatchWaiters.splice(0)) { clearTimeout(waiter.timer) @@ -152,14 +172,23 @@ export async function closeClaudeSession(input: { export async function closeAllClaudeSessions(input: { sessions: Map acquisitions: ClaudeAcquisitionRegistry + exits: Map closeSession: (sessionId: string) => Promise + closeExit: (sessionId: string) => Promise }): Promise { input.acquisitions.close() await closeProcessRegistry({ attempts: 3, - hasEntries: () => input.sessions.size > 0 || input.acquisitions.size > 0, - entryIds: () => new Set([...input.sessions.keys(), ...input.acquisitions.sessionIds()]), - closeEntry: input.closeSession, + hasEntries: () => + input.sessions.size > 0 || input.acquisitions.size > 0 || input.exits.size > 0, + entryIds: () => + new Set([ + ...input.sessions.keys(), + ...input.acquisitions.sessionIds(), + ...input.exits.keys() + ]), + closeEntry: async (sessionId) => + input.exits.has(sessionId) ? input.closeExit(sessionId) : input.closeSession(sessionId), failureMessage: 'claude structured session shutdown could not prove every child stopped' }) } diff --git a/src/main/claude/claude-structured-session-recovery.test.ts b/src/main/claude/claude-structured-session-recovery.test.ts index c2bbe9650a6..a3797dc4eae 100644 --- a/src/main/claude/claude-structured-session-recovery.test.ts +++ b/src/main/claude/claude-structured-session-recovery.test.ts @@ -5,6 +5,7 @@ import { adapterFor, fakeClaude, identityFor, + invokeCanUseTool, PROVIDER_SESSION_ID, tick } from './claude-structured-session-test-support' @@ -291,4 +292,57 @@ describe('ClaudeStructuredSessionAdapter transcript-derived recovery', () => { expect(close).toHaveBeenCalledOnce() expect(events.at(-1)).toMatchObject({ type: 'ended', cause: 'unexpected-exit' }) }) + + it('does not publish recovery while an unexpected-exit close proof is false', async () => { + const claude = fakeClaude() + const events: ClaudeStructuredSessionEvent[] = [] + const persistedHandles: unknown[] = [] + const adapter = adapterFor(claude, {}, events, persistedHandles) + await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9' }) + claude.connections[0].close = vi + .fn<() => Promise>() + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(true) as unknown as (typeof claude.connections)[0]['close'] + + claude.connections[0].handlers.onExit?.(new Error('crashed')) + await tick() + + expect(events.filter((event) => event.type === 'ended')).toEqual([]) + expect(persistedHandles).toEqual([]) + }) + + it('retains pending prompts while an unexpected-exit proof is unproven', async () => { + const claude = fakeClaude() + const events: ClaudeStructuredSessionEvent[] = [] + const adapter = adapterFor(claude, {}, events) + await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9' }) + const answered = invokeCanUseTool(claude.connections[0], 'Bash', 'permission-1', 'tool-1') + claude.connections[0].close = vi + .fn<() => Promise>() + .mockResolvedValue(false) as unknown as (typeof claude.connections)[0]['close'] + + claude.connections[0].handlers.onExit?.(new Error('crashed')) + await tick() + + expect(answered.settled()).toBe(false) + expect(events.filter((event) => event.type === 'ended')).toEqual([]) + }) + + it('publishes unexpected recovery exactly once after a retained proof retries successfully', async () => { + const claude = fakeClaude() + const events: ClaudeStructuredSessionEvent[] = [] + const adapter = adapterFor(claude, {}, events) + await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9' }) + claude.connections[0].close = vi + .fn<() => Promise>() + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(true) as unknown as (typeof claude.connections)[0]['close'] + + claude.connections[0].handlers.onExit?.(new Error('crashed')) + await tick() + await expect(adapter.releaseAcquisition({ sessionId: 'session-1' })).resolves.toBe(true) + await tick() + + expect(events.filter((event) => event.type === 'ended')).toHaveLength(1) + }) }) diff --git a/src/main/claude/claude-structured-session-state.ts b/src/main/claude/claude-structured-session-state.ts index 33017889bf0..e9567da8317 100644 --- a/src/main/claude/claude-structured-session-state.ts +++ b/src/main/claude/claude-structured-session-state.ts @@ -135,9 +135,13 @@ export function mintClaudeAcquisitionGeneration(deps: ClaudeStructuredSessionAda */ export type ClaudeSessionExit = { connection: ClaudeStreamJsonConnection + /** Full session identity retained until its child tree is proven gone. */ + session: ClaudeSession error: Error /** The exit path's first proof attempt; retries must observe this result. */ closePromise?: Promise + /** Shared lifecycle settlement for concurrent proof retries. */ + settlementPromise?: Promise } export type ClaudeAcquisitionAttempt = {