From 0fbe754ebfbe6cf4a096b3c26ebc83a0ef9f67ea Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Wed, 2 Sep 2026 03:16:16 -0700 Subject: [PATCH] fix(claude): classify cleanup after a first-hand exit as a root exit instead of a proven tree When the CLI died between a successful acquire and the host's commit or proof of the lease, handleExit had already removed the session, so releaseAcquisition found nothing and reported true. The attach flow then settled exit-proven with deathEvidence claiming cleanup proved no provider child remains, though the tree was never verified. The adapter now keeps the exit that removed a published session until the session is acquired again; acquisition cleanup runs that connection's close ladder and classifies its verdict exactly as a start-time failure would be, so the record reads root-exit-observed. The wire helper keeps that typed classification and its provider diagnostic instead of wrapping it as unproven, and the router gives up its owner even when the release throws. Claude-Session: https://claude.ai/code/session_01HfdhsvSJucLw4cTZxzg2CP --- .../claude-structured-session-acquisition.ts | 35 ++++++++++++- .../claude-structured-session-adapter.test.ts | 51 +++++++++++++++++++ .../claude-structured-session-adapter.ts | 18 ++++++- .../claude/claude-structured-session-state.ts | 10 ++++ ...tured-agent-session-adapter-router.test.ts | 33 ++++++++++++ ...structured-agent-session-adapter-router.ts | 8 +-- .../structured-agent-session-adapter.test.ts | 22 ++++++++ .../structured-agent-session-adapter.ts | 16 ++++-- 8 files changed, 183 insertions(+), 10 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.test.ts diff --git a/src/main/claude/claude-structured-session-acquisition.ts b/src/main/claude/claude-structured-session-acquisition.ts index 47d226a9831..1748b4c0507 100644 --- a/src/main/claude/claude-structured-session-acquisition.ts +++ b/src/main/claude/claude-structured-session-acquisition.ts @@ -38,10 +38,14 @@ import { type ClaudeAcquisitionRegistry, type ClaudeAcquisitionAttempt, type ClaudeSession, + type ClaudeSessionExit, type ClaudeStructuredSessionAdapterDeps, type ClaudeStructuredSessionEvent } from './claude-structured-session-state' -import { closeClaudePublishedSessionForDeps } from './claude-structured-session-close' +import { + closeClaudePublishedSessionForDeps, + closeClaudeSession +} from './claude-structured-session-close' export const CLAUDE_STRUCTURED_INIT_TIMEOUT_MS = 10_000 @@ -78,12 +82,14 @@ export async function acquireClaudeSession({ deps, sessions, acquisitions, + exits, callbacks }: { input: StructuredAgentSessionAcquireInput deps: ClaudeStructuredSessionAdapterDeps sessions: Map acquisitions: ClaudeAcquisitionRegistry + exits: Map callbacks: AcquireCallbacks }): Promise { const sessionId = input.identity.sessionId @@ -133,6 +139,8 @@ 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) acquisitions.assertCurrent(sessionId, attempt) const launch = await deps.resolveLaunch({ identity: input.identity }) observedLeafUuid = launch.resumeLeafUuid @@ -246,3 +254,28 @@ export async function acquireClaudeSession({ attempt.finish() } } + +/** + * Cleanup for an acquisition the host could not commit or prove. A session that + * a first-hand exit already removed is not an absence to report as proven: the + * ladder on its connection still answers, and that answer is classified exactly + * as a start-time failure would be. + */ +export async function releaseClaudeAcquisition(input: { + sessionId: string + sessions: Map + acquisitions: ClaudeAcquisitionRegistry + exits: Map + persistHandle?: ClaudeStructuredSessionAdapterDeps['persistHandle'] + onEvent?: ClaudeStructuredSessionAdapterDeps['onEvent'] +}): Promise { + const exit = input.exits.get(input.sessionId) + if (!exit || input.sessions.has(input.sessionId) || input.acquisitions.get(input.sessionId)) { + return closeClaudeSession(input) + } + input.exits.delete(input.sessionId) + if (await exit.connection.close()) { + return true + } + throw claudeAcquisitionCleanupError(exit.connection, exit.error) +} diff --git a/src/main/claude/claude-structured-session-adapter.test.ts b/src/main/claude/claude-structured-session-adapter.test.ts index 2a2e00e8134..a0f3b7f4900 100644 --- a/src/main/claude/claude-structured-session-adapter.test.ts +++ b/src/main/claude/claude-structured-session-adapter.test.ts @@ -665,6 +665,57 @@ describe('ClaudeStructuredSessionAdapter acquisition cleanup', () => { expect(error).not.toBeInstanceOf(AgentSessionAcquisitionRootExitObservedError) }) + /** A published session whose CLI then exits first-hand, with the verdict its ladder holds. */ + async function exitedAfterPublish( + exitVerdict: ClaudeStreamJsonConnection['exitVerdict'] + ): Promise<{ adapter: ClaudeStructuredSessionAdapter; connection: FakeConnection }> { + const claude = fakeClaude({ unprovenCloseVerdict: exitVerdict }) + const adapter = await acquired(claude) + const connection = claude.connections[0] + connection.handlers.onExit?.(new Error('claude stream-json exited (code 1): crashed')) + return { adapter, connection } + } + + it('classifies cleanup after a first-hand exit removed the session as a root exit, never as proven', async () => { + // The host may still be committing or proving the lease when the child dies; + // its cleanup must find the exit the ladder observed, not an absence. + const { adapter, connection } = await exitedAfterPublish({ + root: 'exited', + tree: 'unverifiable' + }) + const error = await adapter.releaseAcquisition({ sessionId: 'session-1' }).catch((e) => e) + + expect(error).toBeInstanceOf(AgentSessionAcquisitionRootExitObservedError) + expect((error as Error).message).toBe('claude stream-json exited (code 1): crashed') + expect(connection.closeCount).toBe(1) + }) + + it('never releases after an exit that left a descendant observed alive', async () => { + const { adapter } = await exitedAfterPublish({ root: 'exited', tree: 'live' }) + const error = await adapter.releaseAcquisition({ sessionId: 'session-1' }).catch((e) => e) + + expect(error).toBeInstanceOf(AgentSessionAcquisitionExitUnprovenError) + expect(error).not.toBeInstanceOf(AgentSessionAcquisitionRootExitObservedError) + }) + + it('forgets a retained exit once the session is acquired again', async () => { + const options: Parameters[0] = {} + const claude = fakeClaude(options) + const adapter = await acquired(claude) + const first = claude.connections[0] + first.handlers.onExit?.(new Error('claude stream-json exited (code 1): crashed')) + first.exitVerdict = { root: 'exited', tree: 'unverifiable' } + first.close = async () => false + options.exitBeforeInit = 'claude stream-json exited (code 1): not logged in' + + await expect( + adapter.acquire({ identity: identityFor(), fence: 8, spawnToken: 'spawn-10' }) + ).rejects.toThrow('not logged in') + // The second start's own proven close is the answer; the first exit is stale. + await expect(adapter.releaseAcquisition({ sessionId: 'session-1' })).resolves.toBe(true) + expect(first.closeCount).toBe(0) + }) + it('reports unproven published-session cleanup so callers can retry safely', async () => { const claude = fakeClaude() const adapter = await acquired(claude) diff --git a/src/main/claude/claude-structured-session-adapter.ts b/src/main/claude/claude-structured-session-adapter.ts index 36d3c94f9e8..1e8a8176df6 100644 --- a/src/main/claude/claude-structured-session-adapter.ts +++ b/src/main/claude/claude-structured-session-adapter.ts @@ -6,7 +6,10 @@ import type { import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import { answerClaudePrompt, cancelClaudeTurn } from './claude-structured-control-actions' import { dispatchClaudeTurn } from './claude-structured-dispatch' -import { acquireClaudeSession } from './claude-structured-session-acquisition' +import { + acquireClaudeSession, + releaseClaudeAcquisition +} from './claude-structured-session-acquisition' export { CLAUDE_STRUCTURED_INIT_TIMEOUT_MS } from './claude-structured-session-acquisition' import { supportsClaudeStructuredLocation } from './claude-structured-location-support' import { setClaudeStructuredOption } from './claude-structured-options' @@ -15,6 +18,7 @@ import { ClaudeAcquisitionRegistry, type ClaudeAcquisitionAttempt, type ClaudeSession, + type ClaudeSessionExit, type ClaudeStructuredSessionAdapterDeps, type ClaudeStructuredSessionEvent } from './claude-structured-session-state' @@ -36,6 +40,7 @@ const DISPATCH_ACK_TIMEOUT_MS = 10_000 export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAdapter { private readonly sessions = new Map() private readonly acquisitions = new ClaudeAcquisitionRegistry() + private readonly exits = new Map() constructor(private readonly deps: ClaudeStructuredSessionAdapterDeps) {} @@ -47,6 +52,7 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda deps: this.deps, sessions: this.sessions, acquisitions: this.acquisitions, + exits: this.exits, callbacks: { deliver: (attempt, sessionId, event) => this.deliver(attempt, sessionId, event), emit: (session, events, event) => this.emit(session, events, event), @@ -70,6 +76,7 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda return } this.sessions.delete(sessionId) + this.exits.set(sessionId, { connection: session.connection, error }) this.emit(session, session.events, { type: 'ended', sessionId, reason: error.message }) settleClaudeExitedSession(session) } @@ -109,7 +116,14 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda readClaudeStructuredSessionOptions(this.session(input.sessionId), this.deps.requestTimeoutMs) releaseAcquisition = (input: { sessionId: string }): Promise => - this.closeSession(input.sessionId) + releaseClaudeAcquisition({ + sessionId: input.sessionId, + sessions: this.sessions, + acquisitions: this.acquisitions, + exits: this.exits, + ...(this.deps.persistHandle ? { persistHandle: this.deps.persistHandle } : {}), + ...(this.deps.onEvent ? { onEvent: this.deps.onEvent } : {}) + }) closeSession = (sessionId: string): Promise => closeClaudeSession({ diff --git a/src/main/claude/claude-structured-session-state.ts b/src/main/claude/claude-structured-session-state.ts index d6d234ab34c..b6cbf4dff11 100644 --- a/src/main/claude/claude-structured-session-state.ts +++ b/src/main/claude/claude-structured-session-state.ts @@ -74,6 +74,16 @@ export type ClaudeSession = { events: StructuredAgentSessionEventSink | undefined } +/** + * The first-hand exit that removed a published session. Kept until the session + * is acquired again so acquisition cleanup that arrives after the exit finds + * what the ladder observed, not an absence it would otherwise report as proven. + */ +export type ClaudeSessionExit = { + connection: ClaudeStreamJsonConnection + error: Error +} + export type ClaudeAcquisitionAttempt = { connection: ClaudeStreamJsonConnection | null prompts: ClaudePromptRegistry diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.test.ts new file mode 100644 index 00000000000..db842cae3b0 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it, vi } from 'vitest' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import { StructuredAgentSessionAdapterRouter } from './structured-agent-session-adapter-router' + +function adapterOf( + releaseAcquisition: StructuredAgentSessionAdapter['releaseAcquisition'] +): StructuredAgentSessionAdapter { + return { + acquire: vi.fn(async () => ({ process: { pid: 1 } }) as never), + releaseAcquisition, + dispatch: vi.fn(), + cancelTurn: vi.fn(), + answerPrompt: vi.fn(), + setOption: vi.fn() + } as unknown as StructuredAgentSessionAdapter +} + +describe('StructuredAgentSessionAdapterRouter.releaseAcquisition', () => { + it('drops the owner even when its release reports a typed failure', async () => { + const failure = new Error('root exited') + const claude = adapterOf(vi.fn().mockRejectedValueOnce(failure).mockResolvedValue(false)) + const codex = adapterOf(vi.fn(async () => false)) + const router = new StructuredAgentSessionAdapterRouter({ claude, codex }, async () => {}) + const identity = { sessionId: 'session-1', agent: 'claude' } as never + await router.acquire({ identity, fence: 1, spawnToken: 'spawn-1' }) + + await expect(router.releaseAcquisition({ sessionId: 'session-1' })).rejects.toBe(failure) + // With no owner left, a later release asks every adapter instead of the stale one. + await expect(router.releaseAcquisition({ sessionId: 'session-1' })).resolves.toBe(false) + expect(claude.releaseAcquisition).toHaveBeenCalledTimes(2) + expect(codex.releaseAcquisition).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts index d5968bbadef..88695e58e61 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts @@ -30,9 +30,11 @@ export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessi async releaseAcquisition(input: { sessionId: string }): Promise { const adapter = this.owners.get(input.sessionId) if (adapter) { - const released = await adapter.releaseAcquisition?.(input) - this.owners.delete(input.sessionId) - return released === true + try { + return (await adapter.releaseAcquisition?.(input)) === true + } finally { + this.owners.delete(input.sessionId) + } } let released = false for (const candidate of Object.values(this.adapters)) { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.test.ts index 5f67240c1c6..77cce9153f5 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import { AgentSessionAcquisitionExitUnprovenError, + AgentSessionAcquisitionRootExitObservedError, rethrowAfterAgentSessionAcquisitionCleanup } from './structured-agent-session-adapter' @@ -28,6 +29,27 @@ describe('failed agent-session acquisition cleanup', () => { ).rejects.toBeInstanceOf(AgentSessionAcquisitionExitUnprovenError) }) + it('keeps a first-hand root exit that cleanup observed, with the provider diagnostic', async () => { + const cause = new Error('proof failed') + const exit = new AgentSessionAcquisitionRootExitObservedError( + new Error('claude stream-json exited (code 1): crashed') + ) + const error = await rethrowAfterAgentSessionAcquisitionCleanup( + { + releaseAcquisition: vi.fn(async () => { + throw exit + }) + }, + 'session-1', + cause + ).catch((thrown: unknown) => thrown) + + expect(error).toBeInstanceOf(AgentSessionAcquisitionRootExitObservedError) + expect(error).not.toBeInstanceOf(AgentSessionAcquisitionExitUnprovenError) + expect((error as Error).message).toBe('claude stream-json exited (code 1): crashed') + expect((error as Error).cause).toMatchObject({ errors: [cause, exit] }) + }) + it('reports unproven exit when cleanup throws', async () => { const error = await rethrowAfterAgentSessionAcquisitionCleanup( { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts index 3a6ea86ae3a..0a7022b7da1 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts @@ -119,7 +119,9 @@ export type StructuredAgentSessionAdapter = { * at — the store rejects a link minted at any other fence. */ acquire(input: StructuredAgentSessionAcquireInput): Promise /** Reaps an acquired provider when the host cannot commit or prove its lease. - * Returns true only after provider child exit is proven. */ + * Returns true only after provider child exit is proven. Throws + * `AgentSessionAcquisitionRootExitObservedError` when the provider root's own + * exit was observed first-hand but its descendants could not be verified. */ releaseAcquisition?(input: { sessionId: string }): Promise dispatch(input: { sessionId: string @@ -168,9 +170,15 @@ export async function rethrowAfterAgentSessionAcquisitionCleanup( try { released = (await adapter.releaseAcquisition?.({ sessionId })) === true } catch (cleanupError) { - throw new AgentSessionAcquisitionExitUnprovenError( - new AggregateError([cause, cleanupError], 'agent session acquisition cleanup failed') - ) + // A root exit the cleanup observed first-hand keeps its classification and its + // provider diagnostic; the failure that triggered cleanup rides along as cause. + throw cleanupError instanceof AgentSessionAcquisitionRootExitObservedError + ? new AgentSessionAcquisitionRootExitObservedError( + new AggregateError([cause, cleanupError], cleanupError.message) + ) + : new AgentSessionAcquisitionExitUnprovenError( + new AggregateError([cause, cleanupError], 'agent session acquisition cleanup failed') + ) } if (released) { throw cause