From e5102d59dc923714861ebb1f269be47972cafdf3 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 24 Sep 2026 03:53:44 -0700 Subject: [PATCH 1/3] fix(claude): let the lease release close the adapter's session so an evicted chat resumes Eviction releases a Claude lease once the provider root has exited, even when its descendants are unverifiable. The Claude adapter kept that session indexed until the whole tree was proven gone, and every later acquire re-ran the same close first, so a hidden chat (or an orchestration mail wake) could not resume until the app restarted. - The host's lease release is now the adapter's close decision: the router forwards acknowledgeSessionRelease to the owning adapter, and the Claude adapter drops and settles the session and any retained exit. - An acquire at a later fence than an indexed child retires it instead of closing it again; a child whose root was never seen to exit still goes through the close ladder. - Unproven descendants move to a bounded, connection-keyed cleanup that no acquire consults. It retries the close ladder on a fixed schedule, then reports the last verdict instead of claiming the tree gone. - An unexpected exit whose root exited with an unverifiable tree now publishes `ended`, so the host releases and recovers the session. The exit stays indexed as evidence until the release retires it, so acquisition cleanup keeps reporting a root exit and never a proven one. --- .../claude-released-child-cleanup.test.ts | 90 +++++++ .../claude/claude-released-child-cleanup.ts | 129 ++++++++++ .../claude-structured-acquisition-launch.ts | 8 +- .../claude-structured-session-adapter.ts | 62 ++++- ...laude-structured-session-exit-lifecycle.ts | 26 +- .../claude-structured-session-release.test.ts | 197 ++++++++++++++ .../claude-structured-session-retirement.ts | 60 +++++ .../claude/claude-structured-session-state.ts | 12 +- ...tured-agent-session-adapter-router.test.ts | 17 ++ ...structured-agent-session-adapter-router.ts | 5 +- .../structured-agent-session-adapter.ts | 3 +- ...red-agent-session-claude-root-exit.test.ts | 240 ++++++++++-------- 12 files changed, 718 insertions(+), 131 deletions(-) create mode 100644 src/main/claude/claude-released-child-cleanup.test.ts create mode 100644 src/main/claude/claude-released-child-cleanup.ts create mode 100644 src/main/claude/claude-structured-session-release.test.ts create mode 100644 src/main/claude/claude-structured-session-retirement.ts diff --git a/src/main/claude/claude-released-child-cleanup.test.ts b/src/main/claude/claude-released-child-cleanup.test.ts new file mode 100644 index 00000000000..97de3a96dcc --- /dev/null +++ b/src/main/claude/claude-released-child-cleanup.test.ts @@ -0,0 +1,90 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ClaudeStreamJsonConnection } from './claude-stream-json-connection' +import { + ClaudeReleasedChildCleanup, + type ClaudeReleasedChildCleanupReport +} from './claude-released-child-cleanup' +import { fakeClaude } from './claude-structured-session-test-support' + +async function releasedChild( + closeResults: boolean[], + verdict: ClaudeStreamJsonConnection['exitVerdict'] = { root: 'exited', tree: 'unverifiable' } +) { + const connection = await fakeClaude({ unprovenCloseVerdict: verdict }).openConnection({ + pathToClaudeCodeExecutable: 'claude', + options: {}, + cwd: '/work/repo' + }) + const close = vi.fn(async () => closeResults.shift() ?? false) + connection.close = close + return { connection, close } +} + +describe('ClaudeReleasedChildCleanup', () => { + let reports: ClaudeReleasedChildCleanupReport[] + let cleanup: ClaudeReleasedChildCleanup + + beforeEach(() => { + vi.useFakeTimers() + reports = [] + cleanup = new ClaudeReleasedChildCleanup({ + retryDelaysMs: [10, 20], + report: (report) => reports.push(report) + }) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('gives up after its schedule and reports the verdict it last saw', async () => { + const { connection, close } = await releasedChild([]) + cleanup.adopt('session-1', connection) + + await vi.advanceTimersByTimeAsync(10) + expect(close).toHaveBeenCalledTimes(1) + expect(reports).toEqual([]) + await vi.advanceTimersByTimeAsync(20) + expect(close).toHaveBeenCalledTimes(2) + + expect(reports).toEqual([ + { sessionId: 'session-1', pid: 4321, verdict: { root: 'exited', tree: 'unverifiable' } } + ]) + expect(cleanup.size).toBe(0) + await vi.advanceTimersByTimeAsync(1_000) + expect(close).toHaveBeenCalledTimes(2) + }) + + it('stops as soon as a retry proves the tree gone', async () => { + const { connection, close } = await releasedChild([true]) + cleanup.adopt('session-1', connection) + + await vi.advanceTimersByTimeAsync(100) + + expect(close).toHaveBeenCalledTimes(1) + expect(reports).toEqual([]) + expect(cleanup.size).toBe(0) + }) + + it('never adopts a child whose tree is already proven', async () => { + const { connection } = await releasedChild([], { root: 'exited', tree: 'exited' }) + cleanup.adopt('session-1', connection) + + expect(cleanup.size).toBe(0) + }) + + it('makes one final attempt at shutdown and reports without throwing', async () => { + const unproven = await releasedChild([false]) + const proven = await releasedChild([true]) + cleanup.adopt('session-1', unproven.connection) + cleanup.adopt('session-2', proven.connection) + + await expect(cleanup.closeAll()).resolves.toBeUndefined() + + expect(unproven.close).toHaveBeenCalledOnce() + expect(proven.close).toHaveBeenCalledOnce() + expect(reports.map((report) => report.sessionId)).toEqual(['session-1']) + await vi.advanceTimersByTimeAsync(1_000) + expect(unproven.close).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/claude/claude-released-child-cleanup.ts b/src/main/claude/claude-released-child-cleanup.ts new file mode 100644 index 00000000000..df379b8a9a0 --- /dev/null +++ b/src/main/claude/claude-released-child-cleanup.ts @@ -0,0 +1,129 @@ +import type { + ClaudeChildExitVerdict, + ClaudeStreamJsonConnection +} from './claude-stream-json-connection' + +/** Each retry re-runs the connection's own close ladder, which re-verifies its snapshot. */ +export const CLAUDE_RELEASED_CHILD_RETRY_DELAYS_MS: readonly number[] = [5_000, 30_000, 120_000] + +export type ClaudeReleasedChildCleanupReport = { + sessionId: string + pid: number | undefined + verdict: ClaudeChildExitVerdict +} + +type PendingCleanup = { + sessionId: string + attempts: number + timer: ReturnType | undefined +} + +function reportUnverifiedChild(report: ClaudeReleasedChildCleanupReport): void { + console.warn('[claude-structured-session] released child tree was not verified gone', report) +} + +function treeProven(verdict: ClaudeChildExitVerdict): boolean { + return verdict.root === 'processless' || (verdict.root === 'exited' && verdict.tree === 'exited') +} + +/** + * Descendant verification for children whose lease the host already released. It is keyed by + * connection and nothing consults it before acquiring, so it can never gate a resume. It gives up + * after a fixed schedule and reports what it last observed, never claiming the tree gone. + */ +export class ClaudeReleasedChildCleanup { + private readonly pending = new Map() + private closed = false + private readonly retryDelaysMs: readonly number[] + private readonly report: (report: ClaudeReleasedChildCleanupReport) => void + + constructor( + options: { + retryDelaysMs?: readonly number[] + report?: (report: ClaudeReleasedChildCleanupReport) => void + } = {} + ) { + this.retryDelaysMs = options.retryDelaysMs ?? CLAUDE_RELEASED_CHILD_RETRY_DELAYS_MS + this.report = options.report ?? reportUnverifiedChild + } + + get size(): number { + return this.pending.size + } + + adopt(sessionId: string, connection: ClaudeStreamJsonConnection): void { + if (this.pending.has(connection) || treeProven(connection.exitVerdict)) { + return + } + const entry: PendingCleanup = { sessionId, attempts: 0, timer: undefined } + if (this.closed) { + // Shutdown already ran its final pass; this child still gets exactly one. + void this.finalAttempt(connection, entry) + return + } + this.pending.set(connection, entry) + this.schedule(connection, entry) + } + + /** One last bounded attempt per child, then report whatever stays unverified. Never throws. */ + async closeAll(): Promise { + this.closed = true + const entries = [...this.pending] + this.pending.clear() + await Promise.all( + entries.map(([connection, entry]) => { + clearTimeout(entry.timer) + return this.finalAttempt(connection, entry) + }) + ) + } + + private schedule(connection: ClaudeStreamJsonConnection, entry: PendingCleanup): void { + const delay = this.retryDelaysMs[entry.attempts] + if (delay === undefined) { + this.pending.delete(connection) + this.reportUnverified(connection, entry) + return + } + entry.timer = setTimeout(() => void this.attempt(connection, entry), delay) + entry.timer.unref?.() + } + + private async attempt( + connection: ClaudeStreamJsonConnection, + entry: PendingCleanup + ): Promise { + entry.timer = undefined + entry.attempts += 1 + const proven = await connection.close().catch(() => false) + if (this.pending.get(connection) !== entry) { + return + } + if (proven) { + this.pending.delete(connection) + return + } + this.schedule(connection, entry) + } + + private async finalAttempt( + connection: ClaudeStreamJsonConnection, + entry: PendingCleanup + ): Promise { + if (!(await connection.close().catch(() => false))) { + this.reportUnverified(connection, entry) + } + } + + private reportUnverified(connection: ClaudeStreamJsonConnection, entry: PendingCleanup): void { + try { + this.report({ + sessionId: entry.sessionId, + pid: connection.pid, + verdict: connection.exitVerdict + }) + } catch { + // Reporting is bookkeeping; a failed report must not surface as a close failure. + } + } +} diff --git a/src/main/claude/claude-structured-acquisition-launch.ts b/src/main/claude/claude-structured-acquisition-launch.ts index 7b7f3d3203b..ce7d9762a53 100644 --- a/src/main/claude/claude-structured-acquisition-launch.ts +++ b/src/main/claude/claude-structured-acquisition-launch.ts @@ -39,7 +39,10 @@ export async function resolveClaudeAcquisitionLaunch(args: { ) } acquisitions.assertCurrent(sessionId, attempt) - let resumeSession = sessions.get(sessionId) + // A child whose lease the host already released is retired, never closed again: its tree + // proof is cleanup, not a precondition for this resume. + let resumeSession = + callbacks.retireSuperseded(sessionId, input.fence) ?? sessions.get(sessionId) if (!(await closeClaudePublishedSessionForDeps(sessions, sessionId, deps))) { throw new AgentSessionAcquisitionExitUnprovenError( new Error(`claude session ${sessionId} could not be stopped`) @@ -54,6 +57,9 @@ export async function resolveClaudeAcquisitionLaunch(args: { } // The superseded child must settle before its durable resume identity is reused. await callbacks.settleExit(sessionId, retainedExit) + if (exits.get(sessionId) === retainedExit) { + exits.delete(sessionId) + } resumeSession ??= retainedExit.session } acquisitions.assertCurrent(sessionId, attempt) diff --git a/src/main/claude/claude-structured-session-adapter.ts b/src/main/claude/claude-structured-session-adapter.ts index d28b07dc16d..0cd025c655f 100644 --- a/src/main/claude/claude-structured-session-adapter.ts +++ b/src/main/claude/claude-structured-session-adapter.ts @@ -21,6 +21,12 @@ import { type ClaudeStructuredSessionEvent } from './claude-structured-session-state' import { closeAllClaudeSessions, closeClaudeSession } from './claude-structured-session-close' +import { ClaudeReleasedChildCleanup } from './claude-released-child-cleanup' +import { + retireClaudeReleasedSession, + retireClaudeSessionSupersededByFence, + type ClaudeSessionRetirementInput +} from './claude-structured-session-retirement' import { drainClaudeObservedExits, observeClaudeSessionExit, @@ -54,8 +60,10 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda private readonly exits = new Map() private readonly settledExitErrors = new Map() private readonly exitLifecycle: ClaudeExitLifecycle + private readonly releasedChildren: ClaudeReleasedChildCleanup constructor(private readonly deps: ClaudeStructuredSessionAdapterDeps) { + this.releasedChildren = deps.releasedChildCleanup ?? new ClaudeReleasedChildCleanup() this.exitLifecycle = { sessions: this.sessions, exits: this.exits, @@ -87,11 +95,30 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda handleExit: (sessionId, attempt, error) => observeClaudeSessionExit(this.exitLifecycle, sessionId, attempt, error), settleExit: (sessionId, exit) => - settleClaudeUnexpectedExit(this.exitLifecycle, sessionId, exit) + settleClaudeUnexpectedExit(this.exitLifecycle, sessionId, exit), + retireSuperseded: (sessionId, fence) => + retireClaudeSessionSupersededByFence({ ...this.retirement(sessionId), fence }) } }) } + private retirement(sessionId: string): ClaudeSessionRetirementInput { + return { + sessionId, + sessions: this.sessions, + exits: this.exits, + cleanup: this.releasedChildren, + ...(this.deps.onBackgroundTasksChanged + ? { onBackgroundTasksChanged: this.deps.onBackgroundTasksChanged } + : {}) + } + } + + /** The host released this session's lease: drop it from the index and settle what it owned. */ + acknowledgeSessionRelease = (sessionId: string): void => { + retireClaudeReleasedSession(this.retirement(sessionId)) + } + private deliver(attempt: ClaudeAcquisitionAttempt, sessionId: string, event: () => void): void { if (!attempt.published) { attempt.buffered.push(event) @@ -106,7 +133,7 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda } /** Resolves once every first-hand exit observed so far has published its - * lifecycle event — or has failed its tree proof and stayed indexed for a + * lifecycle event — or has seen a descendant alive and stayed indexed for a * retry. Publication trails observation by the close ladder and the * transcript cursor write, so nothing outside can otherwise tell the two * apart without guessing at wall-clock. */ @@ -119,13 +146,17 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda /** Restart reconciliation reads the transcript a resume replays; these maps track liveness. */ providerHistoryWindow: NonNullable = ( input - ) => - resolveClaudeProviderHistoryWindow({ + ) => { + const exit = this.exits.get(input.identity.sessionId) + return resolveClaudeProviderHistoryWindow({ identity: input.identity, accountHomePath: input.accountHome.path, + // An exit that already published `ended` has no turn left to run. hasLiveSession: - this.sessions.has(input.identity.sessionId) || this.exits.has(input.identity.sessionId) + this.sessions.has(input.identity.sessionId) || + (exit !== undefined && exit.endedWithTreeUnproven !== true) }) + } private emit(session: ClaudeSession | null, event: ClaudeStructuredSessionEvent): void { const backgroundTasksChanged = @@ -256,14 +287,19 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda }) } - closeAll = (): Promise => - closeAllClaudeSessions({ - sessions: this.sessions, - acquisitions: this.acquisitions, - exits: this.exits, - closeSession: this.closeSession, - closeExit: (sessionId) => this.releaseAcquisition({ sessionId }) - }) + closeAll = async (): Promise => { + try { + await closeAllClaudeSessions({ + sessions: this.sessions, + acquisitions: this.acquisitions, + exits: this.exits, + closeSession: this.closeSession, + closeExit: (sessionId) => this.releaseAcquisition({ sessionId }) + }) + } finally { + await this.releasedChildren.closeAll() + } + } private session(sessionId: string): ClaudeSession { const session = this.sessions.get(sessionId) diff --git a/src/main/claude/claude-structured-session-exit-lifecycle.ts b/src/main/claude/claude-structured-session-exit-lifecycle.ts index fb1ab1943b4..2401c6e38cf 100644 --- a/src/main/claude/claude-structured-session-exit-lifecycle.ts +++ b/src/main/claude/claude-structured-session-exit-lifecycle.ts @@ -33,9 +33,8 @@ export function observeClaudeSessionExit( } lifecycle.sessions.delete(sessionId) failClaudeStartupGate(session, error) - // Re-enter the provider's close ladder before publishing lifecycle recovery. - // An exit callback is root evidence only; the retained tree proof must run - // before the host releases and reacquires this exact child. + // Re-enter the provider's close ladder before publishing lifecycle recovery: + // an exit callback is root evidence only. const closePromise = session.connection.close().catch(() => false) const exit: ClaudeSessionExit = { connection: session.connection, @@ -46,17 +45,21 @@ export function observeClaudeSessionExit( lifecycle.exits.set(sessionId, exit) exit.publication = closePromise .then((proven) => { - // A failed startup keeps the failed-create bar: a first-hand root exit releases it. - const startupFailed = session.startup.state === 'failed' - if (!proven && !(startupFailed && claudeRootExitObserved(session.connection))) { - return undefined + if (proven) { + return settleClaudeUnexpectedExit(lifecycle, sessionId, exit) } - return settleClaudeUnexpectedExit(lifecycle, sessionId, exit) + // The lease follows the root, so a first-hand root exit publishes `ended` even while the + // tree is unverifiable; only a descendant seen alive withholds it. + if (claudeRootExitObserved(session.connection)) { + exit.endedWithTreeUnproven = true + return settleClaudeUnexpectedExit(lifecycle, sessionId, exit) + } + return undefined }) .catch(() => undefined) } -/** Lifecycle recovery is published only after the child tree proof is true. */ +/** Persists the last completed turn, then publishes the `ended` the host releases the lease on. */ export function settleClaudeUnexpectedExit( lifecycle: ClaudeExitLifecycle, sessionId: string, @@ -79,7 +82,10 @@ export function settleClaudeUnexpectedExit( settleClaudeExitedSession(exit.session) return } - exits.delete(sessionId) + // Unproven descendants stay indexed as evidence until the host's release retires them. + if (!exit.endedWithTreeUnproven) { + exits.delete(sessionId) + } lifecycle.settledExitErrors.set(sessionId, exit.error) const ended: ClaudeStructuredSessionEvent = { type: 'ended', diff --git a/src/main/claude/claude-structured-session-release.test.ts b/src/main/claude/claude-structured-session-release.test.ts new file mode 100644 index 00000000000..3c3d19e57b8 --- /dev/null +++ b/src/main/claude/claude-structured-session-release.test.ts @@ -0,0 +1,197 @@ +// The host's lease release is the one decision that a Claude child is closed. These pin how the +// adapter's own session index follows that decision instead of outliving it. + +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { + AgentSessionAcquisitionExitUnprovenError, + AgentSessionAcquisitionRootExitObservedError +} from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import type { ClaudeStreamJsonConnection } from './claude-stream-json-connection' +import { + ClaudeReleasedChildCleanup, + type ClaudeReleasedChildCleanupReport +} from './claude-released-child-cleanup' +import { + ClaudeStructuredSessionAdapter, + type ClaudeStructuredSessionEvent +} from './claude-structured-session-adapter' +import { + PROVIDER_SESSION_ID, + fakeClaude, + identityFor, + recordingJournalSink +} from './claude-structured-session-test-support' + +const ROOT_EXITED_TREE_UNVERIFIABLE = { root: 'exited', tree: 'unverifiable' } as const + +async function acquiredWithCleanup( + closeVerdict?: ClaudeStreamJsonConnection['exitVerdict'], + claudeConfigDir = '/accounts/claude' +) { + const claude = fakeClaude(closeVerdict ? { unprovenCloseVerdict: closeVerdict } : {}) + const events: ClaudeStructuredSessionEvent[] = [] + const unverified: ClaudeReleasedChildCleanupReport[] = [] + const cleanup = new ClaudeReleasedChildCleanup({ + retryDelaysMs: [], + report: (report) => unverified.push(report) + }) + const adapter = new ClaudeStructuredSessionAdapter({ + resolveLaunch: async () => ({ + pathToClaudeCodeExecutable: 'claude', + options: {}, + cwd: '/work/repo', + claudeConfigDir, + providerSessionId: PROVIDER_SESSION_ID, + resumeLeafUuid: null, + resumesTranscript: false, + continuesChain: false + }), + onEvent: (event) => events.push(event), + openConnection: claude.openConnection, + readProcessStartTime: async () => 1_700_000_000_000, + now: () => 1_700_000_000_500, + persistHandle: async () => {}, + releasedChildCleanup: cleanup + }) + await adapter.acquire({ + identity: identityFor(), + fence: 7, + spawnToken: 'spawn-7', + events: recordingJournalSink() + }) + await adapter.drainStartup(identityFor().sessionId) + return { adapter, claude, events, cleanup, unverified } +} + +function endedEvents(events: ClaudeStructuredSessionEvent[]) { + return events.filter((event) => event.type === 'ended') +} + +describe('Claude unexpected exit with an unverifiable tree', () => { + it('publishes ended so the host can release the lease the root held', async () => { + const { adapter, claude, events } = await acquiredWithCleanup(ROOT_EXITED_TREE_UNVERIFIABLE) + + claude.connections[0]!.handlers.onExit?.(new Error('claude stream-json exited (code 1)')) + await adapter.drainObservedExits() + + expect(endedEvents(events)).toEqual([ + expect.objectContaining({ cause: 'unexpected-exit', fence: 7 }) + ]) + // Publishing is not a tree claim: cleanup for this child still reports only the root's exit. + await expect(adapter.releaseAcquisition({ sessionId: 'session-1' })).rejects.toBeInstanceOf( + AgentSessionAcquisitionRootExitObservedError + ) + }) + + it('still withholds ended while a descendant was seen alive', async () => { + const { adapter, claude, events } = await acquiredWithCleanup({ root: 'exited', tree: 'live' }) + + claude.connections[0]!.handlers.onExit?.(new Error('claude stream-json exited (code 1)')) + await adapter.drainObservedExits() + + expect(endedEvents(events)).toEqual([]) + await expect(adapter.releaseAcquisition({ sessionId: 'session-1' })).rejects.toBeInstanceOf( + AgentSessionAcquisitionExitUnprovenError + ) + }) + + it('stops reporting a turn in flight once ended is out', async () => { + const accountHome = await mkdtemp(join(tmpdir(), 'orca-claude-release-history-')) + try { + const { adapter, claude } = await acquiredWithCleanup( + ROOT_EXITED_TREE_UNVERIFIABLE, + accountHome + ) + await mkdir(join(accountHome, 'projects', 'work'), { recursive: true }) + const rows = [ + { type: 'user', uuid: 'anchor', parentUuid: null, sessionId: PROVIDER_SESSION_ID }, + { type: 'last-prompt', sessionId: PROVIDER_SESSION_ID, leafUuid: 'anchor' } + ] + await writeFile( + join(accountHome, 'projects', 'work', `${PROVIDER_SESSION_ID}.jsonl`), + `${rows.map((row) => JSON.stringify(row)).join('\n')}\n` + ) + + claude.connections[0]!.handlers.onExit?.(new Error('claude stream-json exited (code 1)')) + await adapter.drainObservedExits() + + const window = await adapter.providerHistoryWindow({ + identity: { + ...identityFor(), + providerHandle: { kind: 'claude', sessionId: PROVIDER_SESSION_ID, leafUuid: 'anchor' } + }, + accountHome: { variable: 'CLAUDE_CONFIG_DIR', path: accountHome } + }) + expect(window?.turnInFlight).toBe(false) + } finally { + await rm(accountHome, { recursive: true, force: true }) + } + }) +}) + +describe('Claude acknowledged session release', () => { + it('forgets a session whose close saw only the root leave, and hands its tree to cleanup', async () => { + const { adapter, claude, cleanup, unverified } = await acquiredWithCleanup( + ROOT_EXITED_TREE_UNVERIFIABLE + ) + await expect(adapter.closeSession('session-1')).rejects.toBeInstanceOf( + AgentSessionAcquisitionRootExitObservedError + ) + + adapter.acknowledgeSessionRelease('session-1') + + expect(adapter.recordsContextUsage('session-1')).toBe(false) + await expect(adapter.closeSession('session-1')).resolves.toBe(true) + expect(claude.connections[0]!.closeCount).toBe(1) + await cleanup.closeAll() + expect(unverified).toEqual([ + { sessionId: 'session-1', pid: 4321, verdict: ROOT_EXITED_TREE_UNVERIFIABLE } + ]) + }) + + it('forgets an exit whose ended already went out', async () => { + const { adapter, claude, cleanup, unverified } = await acquiredWithCleanup( + ROOT_EXITED_TREE_UNVERIFIABLE + ) + claude.connections[0]!.handlers.onExit?.(new Error('claude stream-json exited (code 1)')) + await adapter.drainObservedExits() + + adapter.acknowledgeSessionRelease('session-1') + + await expect(adapter.releaseAcquisition({ sessionId: 'session-1' })).resolves.toBe(true) + await cleanup.closeAll() + expect(unverified).toHaveLength(1) + }) +}) + +describe('Claude resume past a released child', () => { + it('does not close a child again once a later fence proves its lease released', async () => { + const { adapter, claude, cleanup, unverified } = await acquiredWithCleanup( + ROOT_EXITED_TREE_UNVERIFIABLE + ) + // No acknowledgement: an acquisition-cleanup release reaches the store, not the adapter. + await expect(adapter.closeSession('session-1')).rejects.toBeInstanceOf( + AgentSessionAcquisitionRootExitObservedError + ) + + await adapter.acquire({ identity: identityFor(), fence: 8, spawnToken: 'spawn-8' }) + + expect(claude.connections).toHaveLength(2) + expect(claude.connections[0]!.closeCount).toBe(1) + await cleanup.closeAll() + expect(unverified.map((report) => report.pid)).toEqual([4321]) + }) + + it('still stops a live child first, since a later fence is not evidence it died', async () => { + const { adapter, claude } = await acquiredWithCleanup() + const first = claude.connections[0]! + + await adapter.acquire({ identity: identityFor(), fence: 8, spawnToken: 'spawn-8' }) + + expect(first.closeCount).toBe(1) + expect(claude.connections).toHaveLength(2) + }) +}) diff --git a/src/main/claude/claude-structured-session-retirement.ts b/src/main/claude/claude-structured-session-retirement.ts new file mode 100644 index 00000000000..0f2e562f212 --- /dev/null +++ b/src/main/claude/claude-structured-session-retirement.ts @@ -0,0 +1,60 @@ +import type { ClaudeReleasedChildCleanup } from './claude-released-child-cleanup' +import { settleClaudeExitedSession } from './claude-structured-session-close' +import type { + ClaudeSession, + ClaudeSessionExit, + ClaudeStructuredSessionAdapterDeps +} from './claude-structured-session-state' + +export type ClaudeSessionRetirementInput = { + sessionId: string + sessions: Map + exits: Map + cleanup: ClaudeReleasedChildCleanup + onBackgroundTasksChanged?: ClaudeStructuredSessionAdapterDeps['onBackgroundTasksChanged'] +} + +function retire(input: ClaudeSessionRetirementInput, session: ClaudeSession): void { + session.unbindReadingControl?.() + settleClaudeExitedSession(session) + if (session.backgroundTasks.clear()) { + input.onBackgroundTasksChanged?.(input.sessionId, null) + } + input.cleanup.adopt(input.sessionId, session.connection) +} + +/** + * The host's lease release is the one decision that a child is closed. The session leaves the + * index here, whatever its tree proof said, and any unproven descendants become cleanup that no + * acquisition waits on. Returns the retired session so a resume can reuse its provider handle. + */ +export function retireClaudeReleasedSession( + input: ClaudeSessionRetirementInput +): ClaudeSession | undefined { + const session = input.sessions.get(input.sessionId) + if (session) { + input.sessions.delete(input.sessionId) + retire(input, session) + } + const exit = input.exits.get(input.sessionId) + if (exit) { + input.exits.delete(input.sessionId) + retire(input, exit.session) + } + return session ?? exit?.session +} + +/** + * An acquisition at a later fence than the indexed child's proves the host already released that + * child's lease, so its tree proof is no longer a precondition. A root not yet seen to exit still + * goes through the close ladder: a newer fence is not evidence that a process died. + */ +export function retireClaudeSessionSupersededByFence( + input: ClaudeSessionRetirementInput & { fence: number } +): ClaudeSession | undefined { + const indexed = input.sessions.get(input.sessionId) ?? input.exits.get(input.sessionId)?.session + if (!indexed || indexed.fence >= input.fence || indexed.connection.exitVerdict.root === 'live') { + return undefined + } + return retireClaudeReleasedSession(input) +} diff --git a/src/main/claude/claude-structured-session-state.ts b/src/main/claude/claude-structured-session-state.ts index 3c6a8413a91..5d878e7e2f7 100644 --- a/src/main/claude/claude-structured-session-state.ts +++ b/src/main/claude/claude-structured-session-state.ts @@ -20,6 +20,7 @@ import type { import type { ClaudeBackgroundTaskTracker } from './claude-background-task-tracker' import type { ClaudeSlashCommandCatalog } from './claude-slash-command-catalog' import type { ClaudeSessionStartupGate } from './claude-structured-session-startup-gate' +import type { ClaudeReleasedChildCleanup } from './claude-released-child-cleanup' export type ClaudeAuthDiagnostic = { apiKeySourceConfigured: boolean @@ -108,6 +109,8 @@ export type ClaudeStructuredSessionAdapterDeps = { leafUuid: string fence: number }) => Promise + /** Bounded tree verification for children whose lease was released; injectable for tests. */ + releasedChildCleanup?: ClaudeReleasedChildCleanup } export type ClaudeDispatchWaiter = { @@ -188,8 +191,9 @@ export function mintClaudeAcquisitionGeneration(deps: ClaudeStructuredSessionAda /** * 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. + * is acquired again or its lease release is acknowledged, 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 @@ -203,6 +207,8 @@ export type ClaudeSessionExit = { /** The whole ladder-then-settle tail, retained so a barrier can await an exit * that is observed but not yet published. Never rejects. */ publication?: Promise + /** `ended` went out on the root's exit while the tree stayed unproven; kept only as evidence. */ + endedWithTreeUnproven?: boolean } export type ClaudeAcquisitionAttempt = { @@ -321,4 +327,6 @@ export type ClaudeAcquireCallbacks = { ) => void handleExit: (sessionId: string, attempt: ClaudeAcquisitionAttempt, error: Error) => void settleExit: (sessionId: string, exit: ClaudeSessionExit) => Promise + /** Retires an indexed child whose lease a later fence proves released; returns it for resume. */ + retireSuperseded: (sessionId: string, fence: number) => ClaudeSession | undefined } 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 index e08c289c87a..a3370f783ca 100644 --- 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 @@ -42,6 +42,23 @@ function adapterOf( } } +describe('StructuredAgentSessionAdapterRouter.acknowledgeSessionRelease', () => { + it('hands the host release to the owning adapter only', async () => { + const claude = adapterOf(vi.fn(async () => true)) + const codex = adapterOf(vi.fn(async () => true)) + claude.acknowledgeSessionRelease = vi.fn() + codex.acknowledgeSessionRelease = vi.fn() + const router = new StructuredAgentSessionAdapterRouter({ claude, codex }, async () => {}) + await router.acquire({ identity: claudeIdentity('session-1'), fence: 1, spawnToken: 's-1' }) + + router.acknowledgeSessionRelease('session-1') + router.acknowledgeSessionRelease('session-1') + + expect(claude.acknowledgeSessionRelease).toHaveBeenCalledExactlyOnceWith('session-1') + expect(codex.acknowledgeSessionRelease).not.toHaveBeenCalled() + }) +}) + describe('StructuredAgentSessionAdapterRouter.releaseAcquisition', () => { it('drops the owner even when its release reports a typed failure', async () => { const failure = new Error('root exited') 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 a41eb92e56d..6eb0d626a53 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 @@ -198,9 +198,12 @@ export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessi return this.closePromise } - /** Drops a per-session stop receipt after the host releases its durable owner. */ + /** Drops a per-session stop receipt after the host releases its durable owner, and hands that + * release to the owning provider so its own session index cannot outlive the lease. */ acknowledgeSessionRelease = (sessionId: string): void => { + const route = this.routes.get(sessionId) this.routes.delete(sessionId) + route?.adapter.acknowledgeSessionRelease?.(sessionId) } private owner(sessionId: string): StructuredAgentSessionAdapter { 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 d8407cd2ad3..9377fb549e8 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 @@ -298,7 +298,8 @@ export type StructuredAgentSessionAdapter = { forceCloseSession?(sessionId: string): Promise /** Stops a provider child for teardown without requiring a future-resume cursor. */ disposeSession?(sessionId: string): Promise - /** Host acknowledgement that the proven-dead child, lease and journal owner are released. */ + /** Host acknowledgement that the lease and journal owner are released. This is the close + * decision: the root is gone, though its descendants may still be unverified. */ acknowledgeSessionRelease?(sessionId: string): void } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-root-exit.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-root-exit.test.ts index d83ca49a79c..bfb2335792b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-root-exit.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-root-exit.test.ts @@ -2,9 +2,10 @@ import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' +import { ClaudeReleasedChildCleanup } from '../../claude/claude-released-child-cleanup' +import { ClaudeStructuredSessionAdapter } from '../../claude/claude-structured-session-adapter' import { PROVIDER_SESSION_ID, - adapterFor, fakeClaude, identityFor } from '../../claude/claude-structured-session-test-support' @@ -24,109 +25,118 @@ afterEach(async () => { await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) }) -describe('Claude root-exit eviction', () => { - it('releases a captured live claim after the provider root exits', async () => { - const root = await mkdtemp(join(tmpdir(), 'orca-claude-root-exit-')) - roots.push(root) - const store = await AgentSessionRecordStore.open({ directory: root, hostId: 'local' }) - const claude = fakeClaude({ - unprovenCloseVerdict: { root: 'exited', tree: 'unverifiable' } - }) - const adapter = adapterFor(claude) - const reservation = await store.reserveOwner({ - sessionId: 'session-1', - location: { - executionHostId: 'local', - workspaceId: 'folder-1', - workspaceKind: 'folder', - wslDistro: null - }, - provider: 'claude', - accountHome: { variable: 'CLAUDE_CONFIG_DIR', path: root }, - runtimeKind: 'native', - expectedFence: null, - spawnToken: 'spawn-1', - claimKeyId: 'key-1', - handoffOperationId: null, - probe: { outcome: 'reservation-unused' }, - operation: { - callerKey: 'test', - operationId: `${NOW}-00000000000000000000000000000001`, - fingerprint: 'create' - }, - now: NOW - }) - const fence = reservation.record.lease.runtimeFence - const acquisition = await adapter.acquire({ - identity: { ...identityFor(), hostId: 'local', workspaceId: 'folder-1' }, - fence, - spawnToken: 'spawn-1' - }) - await store.commitProcessIdentity({ - sessionId: 'session-1', - fence, - process: acquisition.process, - now: NOW - }) - await store.proveOwner({ +const LOCATION = { + executionHostId: 'local', + workspaceId: 'folder-1', + workspaceKind: 'folder', + wslDistro: null +} as const + +const IDENTITY = { ...identityFor(), hostId: 'local', workspaceId: 'folder-1' } + +/** A live Claude claim evicted while its close could only observe the root leave. */ +async function evictAfterRootExit(options: { exitFirst: boolean }) { + const root = await mkdtemp(join(tmpdir(), 'orca-claude-root-exit-')) + roots.push(root) + const store = await AgentSessionRecordStore.open({ directory: root, hostId: 'local' }) + const claude = fakeClaude({ + unprovenCloseVerdict: { root: 'exited', tree: 'unverifiable' } + }) + const unverified: unknown[] = [] + const cleanup = new ClaudeReleasedChildCleanup({ + retryDelaysMs: [], + report: (report) => unverified.push(report) + }) + const adapter = new ClaudeStructuredSessionAdapter({ + resolveLaunch: async () => ({ + pathToClaudeCodeExecutable: 'claude', + options: {}, + cwd: '/work/repo', + claudeConfigDir: root, + providerSessionId: PROVIDER_SESSION_ID, + resumeLeafUuid: null, + resumesTranscript: false, + continuesChain: false + }), + openConnection: claude.openConnection, + readProcessStartTime: async () => 1_700_000_000_000, + now: () => 1_700_000_000_500, + persistHandle: async () => {}, + releasedChildCleanup: cleanup + }) + const reservation = await store.reserveOwner({ + sessionId: 'session-1', + location: LOCATION, + provider: 'claude', + accountHome: { variable: 'CLAUDE_CONFIG_DIR', path: root }, + runtimeKind: 'native', + expectedFence: null, + spawnToken: 'spawn-1', + claimKeyId: 'key-1', + handoffOperationId: null, + probe: { outcome: 'reservation-unused' }, + operation: { + callerKey: 'test', + operationId: `${NOW}-00000000000000000000000000000001`, + fingerprint: 'create' + }, + now: NOW + }) + const fence = reservation.record.lease.runtimeFence + const acquisition = await adapter.acquire({ identity: IDENTITY, fence, spawnToken: 'spawn-1' }) + await adapter.drainStartup(IDENTITY.sessionId) + await store.commitProcessIdentity({ + sessionId: 'session-1', + fence, + process: acquisition.process, + now: NOW + }) + await store.proveOwner({ sessionId: 'session-1', fence, link: acquisition.link, now: NOW }) + const journal = await journals.open({ identity: IDENTITY, journalDir: join(root, 'journal') }) + const close = vi.spyOn(journal, 'close') + const params: AgentSessionAttachParams = { + envelope: { sessionId: 'session-1', - fence, - link: acquisition.link, - now: NOW - }) - const journal = await journals.open({ - identity: { ...identityFor(), hostId: 'local', workspaceId: 'folder-1' }, - journalDir: join(root, 'journal') - }) - const close = vi.spyOn(journal, 'close') - const params: AgentSessionAttachParams = { - envelope: { - sessionId: 'session-1', - clientOperationId: `${NOW}-00000000000000000000000000000001`, - expectedRuntimeFence: fence, - payloadFingerprint: 'create' - }, - location: { - executionHostId: 'local', - workspaceId: 'folder-1', - workspaceKind: 'folder', - wslDistro: null - }, - provider: 'claude', - agent: 'claude', - accountHome: { variable: 'CLAUDE_CONFIG_DIR', path: root }, - runtimeKind: 'native', - providerHandle: { kind: 'claude', sessionId: PROVIDER_SESSION_ID, leafUuid: null } - } - const sessions = new Map([ - [ - 'session-1', - { - journal, - params, - fence, - hasProviderChild: true, - providerChildPhase: 'ready', - acquisitionGeneration: acquisition.acquisitionGeneration ?? null - } - ] - ]) - const deps = { store, adapter, journalRoot: root, claimKeyId: 'key-1' } - const runtimeState = new StructuredAgentSessionHostRuntimeState(deps) + clientOperationId: `${NOW}-00000000000000000000000000000001`, + expectedRuntimeFence: fence, + payloadFingerprint: 'create' + }, + location: LOCATION, + provider: 'claude', + agent: 'claude', + accountHome: { variable: 'CLAUDE_CONFIG_DIR', path: root }, + runtimeKind: 'native', + providerHandle: { kind: 'claude', sessionId: PROVIDER_SESSION_ID, leafUuid: null } + } + const sessions = new Map([ + [ + 'session-1', + { + journal, + params, + fence, + hasProviderChild: true, + providerChildPhase: 'ready', + acquisitionGeneration: acquisition.acquisitionGeneration ?? null + } + ] + ]) + const deps = { store, adapter, journalRoot: root, claimKeyId: 'key-1' } + const runtimeState = new StructuredAgentSessionHostRuntimeState(deps) + if (options.exitFirst) { claude.connections[0]!.handlers.onExit?.(new Error('provider exited')) - await expect( - evictHeldStructuredAgentSession( - { - deps, - runtimeState, - sessions, - now: () => NOW + 30 * 60_000, - forgetStatus: vi.fn() - }, - 'session-1' - ) - ).resolves.toBeUndefined() + } + await evictHeldStructuredAgentSession( + { deps, runtimeState, sessions, now: () => NOW + 30 * 60_000, forgetStatus: vi.fn() }, + 'session-1' + ) + return { adapter, claude, store, sessions, close, cleanup, unverified } +} + +describe('Claude root-exit eviction', () => { + it('releases a captured live claim after the provider root exits', async () => { + const { adapter, store, sessions, close } = await evictAfterRootExit({ exitFirst: true }) expect(store.getRecord('session-1')?.lease).toMatchObject({ claimStatus: 'released', @@ -135,7 +145,31 @@ describe('Claude root-exit eviction', () => { }) expect(sessions.size).toBe(0) expect(close).toHaveBeenCalledOnce() - // Why: releasing the root-owned lease does not claim unverifiable descendants stopped. - await expect(adapter.closeSession('session-1')).rejects.toThrow('provider exited') + // The release is the close decision: the adapter keeps no session for a close to retry. + await expect(adapter.closeSession('session-1')).resolves.toBe(true) + }) + + it.each([ + ['an orderly close', false], + ['a provider that exited first', true] + ])('resumes in the same run after %s left the tree unverifiable', async (_label, exitFirst) => { + const { adapter, claude, store, cleanup, unverified } = await evictAfterRootExit({ exitFirst }) + const released = store.getRecord('session-1')!.lease + expect(released.claimStatus).toBe('released') + + await expect( + adapter.acquire({ + identity: IDENTITY, + fence: released.runtimeFence + 1, + spawnToken: 'spawn-2' + }) + ).resolves.toMatchObject({ link: { handle: { sessionId: PROVIDER_SESSION_ID } } }) + expect(claude.connections).toHaveLength(2) + + // The old tree is reported unverified, never claimed gone, and nothing waited on it. + await cleanup.closeAll() + expect(unverified).toEqual([ + { sessionId: 'session-1', pid: 4321, verdict: { root: 'exited', tree: 'unverifiable' } } + ]) }) }) From fcddff74f6c104e8deef24257c3f7219dd255d80 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 24 Sep 2026 04:36:49 -0700 Subject: [PATCH 2/3] fix(claude): fence the acknowledged release so a late one cannot retire a newer child The host's release acknowledgement now carries the fence it released. The router and the Claude adapter both retire only a child at or below that fence, through the same fence rule an acquisition uses, so a stale acknowledgement cannot reach a child acquired since. The rule's comment now names both ways the lease grants a later fence. --- .../claude-structured-session-adapter.ts | 12 ++++---- .../claude-structured-session-release.test.ts | 21 ++++++++++++-- .../claude-structured-session-retirement.ts | 19 ++++++++---- ...tured-agent-session-adapter-router.test.ts | 29 +++++++++++++++---- ...structured-agent-session-adapter-router.ts | 16 +++++++--- .../structured-agent-session-adapter.ts | 2 +- ...tructured-agent-session-handoff-forward.ts | 5 ++-- .../structured-agent-session-handoff-types.ts | 2 +- .../structured-agent-session-handoff.test.ts | 3 +- .../structured-agent-session-host-handoff.ts | 3 +- .../structured-agent-session-host-lifetime.ts | 2 +- .../structured-agent-session-host-teardown.ts | 12 ++++---- ...ured-agent-session-journal-handles.test.ts | 7 +++-- 13 files changed, 96 insertions(+), 37 deletions(-) diff --git a/src/main/claude/claude-structured-session-adapter.ts b/src/main/claude/claude-structured-session-adapter.ts index 0cd025c655f..ecb02ba86d7 100644 --- a/src/main/claude/claude-structured-session-adapter.ts +++ b/src/main/claude/claude-structured-session-adapter.ts @@ -23,8 +23,7 @@ import { import { closeAllClaudeSessions, closeClaudeSession } from './claude-structured-session-close' import { ClaudeReleasedChildCleanup } from './claude-released-child-cleanup' import { - retireClaudeReleasedSession, - retireClaudeSessionSupersededByFence, + retireClaudeSessionReleasedThrough, type ClaudeSessionRetirementInput } from './claude-structured-session-retirement' import { @@ -97,7 +96,10 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda settleExit: (sessionId, exit) => settleClaudeUnexpectedExit(this.exitLifecycle, sessionId, exit), retireSuperseded: (sessionId, fence) => - retireClaudeSessionSupersededByFence({ ...this.retirement(sessionId), fence }) + retireClaudeSessionReleasedThrough({ + ...this.retirement(sessionId), + releasedFence: fence - 1 + }) } }) } @@ -115,8 +117,8 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda } /** The host released this session's lease: drop it from the index and settle what it owned. */ - acknowledgeSessionRelease = (sessionId: string): void => { - retireClaudeReleasedSession(this.retirement(sessionId)) + acknowledgeSessionRelease = (sessionId: string, releasedFence: number): void => { + retireClaudeSessionReleasedThrough({ ...this.retirement(sessionId), releasedFence }) } private deliver(attempt: ClaudeAcquisitionAttempt, sessionId: string, event: () => void): void { diff --git a/src/main/claude/claude-structured-session-release.test.ts b/src/main/claude/claude-structured-session-release.test.ts index 3c3d19e57b8..93c19050a3a 100644 --- a/src/main/claude/claude-structured-session-release.test.ts +++ b/src/main/claude/claude-structured-session-release.test.ts @@ -141,7 +141,7 @@ describe('Claude acknowledged session release', () => { AgentSessionAcquisitionRootExitObservedError ) - adapter.acknowledgeSessionRelease('session-1') + adapter.acknowledgeSessionRelease('session-1', 7) expect(adapter.recordsContextUsage('session-1')).toBe(false) await expect(adapter.closeSession('session-1')).resolves.toBe(true) @@ -159,12 +159,29 @@ describe('Claude acknowledged session release', () => { claude.connections[0]!.handlers.onExit?.(new Error('claude stream-json exited (code 1)')) await adapter.drainObservedExits() - adapter.acknowledgeSessionRelease('session-1') + adapter.acknowledgeSessionRelease('session-1', 7) await expect(adapter.releaseAcquisition({ sessionId: 'session-1' })).resolves.toBe(true) await cleanup.closeAll() expect(unverified).toHaveLength(1) }) + it('leaves a child acquired since alone when a stale release arrives late', async () => { + const { adapter, claude, cleanup, unverified } = await acquiredWithCleanup( + ROOT_EXITED_TREE_UNVERIFIABLE + ) + await expect(adapter.closeSession('session-1')).rejects.toBeInstanceOf( + AgentSessionAcquisitionRootExitObservedError + ) + await adapter.acquire({ identity: identityFor(), fence: 8, spawnToken: 'spawn-8' }) + const current = claude.connections[1]! + + adapter.acknowledgeSessionRelease('session-1', 7) + + expect(adapter.recordsContextUsage('session-1')).toBe(true) + await cleanup.closeAll() + expect(current.closeCount).toBe(0) + expect(unverified).toHaveLength(1) + }) }) describe('Claude resume past a released child', () => { diff --git a/src/main/claude/claude-structured-session-retirement.ts b/src/main/claude/claude-structured-session-retirement.ts index 0f2e562f212..e666504bca5 100644 --- a/src/main/claude/claude-structured-session-retirement.ts +++ b/src/main/claude/claude-structured-session-retirement.ts @@ -45,15 +45,22 @@ export function retireClaudeReleasedSession( } /** - * An acquisition at a later fence than the indexed child's proves the host already released that - * child's lease, so its tree proof is no longer a precondition. A root not yet seen to exit still - * goes through the close ladder: a newer fence is not evidence that a process died. + * Retires the indexed child only when its fence is at or below the one the host no longer grants to + * it. Two facts deliver that: an acknowledged release names its fence, and an acquisition at fence + * F means F-1 is gone. The lease grants a later fence either after a release or over an unreleased + * lease whose owner probe proved the recorded process dead, so neither is evidence about THIS + * connection: a root not yet seen to exit here still goes through the close ladder. A stale + * acknowledgement can never reach a child acquired at a newer fence. */ -export function retireClaudeSessionSupersededByFence( - input: ClaudeSessionRetirementInput & { fence: number } +export function retireClaudeSessionReleasedThrough( + input: ClaudeSessionRetirementInput & { releasedFence: number } ): ClaudeSession | undefined { const indexed = input.sessions.get(input.sessionId) ?? input.exits.get(input.sessionId)?.session - if (!indexed || indexed.fence >= input.fence || indexed.connection.exitVerdict.root === 'live') { + if ( + !indexed || + indexed.fence > input.releasedFence || + indexed.connection.exitVerdict.root === 'live' + ) { return undefined } return retireClaudeReleasedSession(input) 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 index a3370f783ca..c17713ab294 100644 --- 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 @@ -51,12 +51,31 @@ describe('StructuredAgentSessionAdapterRouter.acknowledgeSessionRelease', () => const router = new StructuredAgentSessionAdapterRouter({ claude, codex }, async () => {}) await router.acquire({ identity: claudeIdentity('session-1'), fence: 1, spawnToken: 's-1' }) - router.acknowledgeSessionRelease('session-1') - router.acknowledgeSessionRelease('session-1') + router.acknowledgeSessionRelease('session-1', 1) + router.acknowledgeSessionRelease('session-1', 1) - expect(claude.acknowledgeSessionRelease).toHaveBeenCalledExactlyOnceWith('session-1') + expect(claude.acknowledgeSessionRelease).toHaveBeenCalledExactlyOnceWith('session-1', 1) expect(codex.acknowledgeSessionRelease).not.toHaveBeenCalled() }) + + it('ignores a release for an older fence than the route acquired since', async () => { + const closeSession = vi.fn(async () => true) + const claude = adapterOf(vi.fn(async () => true)) + claude.closeSession = closeSession + claude.acknowledgeSessionRelease = vi.fn() + const router = new StructuredAgentSessionAdapterRouter( + { claude, codex: adapterOf(vi.fn(async () => true)) }, + async () => {} + ) + await router.acquire({ identity: claudeIdentity('session-1'), fence: 1, spawnToken: 's-1' }) + await router.acquire({ identity: claudeIdentity('session-1'), fence: 3, spawnToken: 's-3' }) + + router.acknowledgeSessionRelease('session-1', 1) + + expect(claude.acknowledgeSessionRelease).not.toHaveBeenCalled() + await expect(router.closeSession('session-1')).resolves.toBe(true) + expect(closeSession).toHaveBeenCalledOnce() + }) }) describe('StructuredAgentSessionAdapterRouter.releaseAcquisition', () => { @@ -120,7 +139,7 @@ describe('StructuredAgentSessionAdapterRouter.closeSession', () => { await expect(closeJournal()).rejects.toThrow('journal close failed') await expect(router.closeSession('session-1')).resolves.toBe(true) expect(closeSession).toHaveBeenCalledOnce() - router.acknowledgeSessionRelease('session-1') + router.acknowledgeSessionRelease('session-1', 1) await expect(router.closeSession('session-1')).resolves.toBe(false) await router.acquire({ identity, fence: 2, spawnToken: 'spawn-2' }) @@ -223,7 +242,7 @@ describe('StructuredAgentSessionAdapterRouter.closeAll', () => { // router has no record of, and an absent record is not a stop it can report. await expect(router.closeSession('session-1')).resolves.toBe(true) await expect(router.closeSession('never-routed')).resolves.toBe(false) - router.acknowledgeSessionRelease('session-1') + router.acknowledgeSessionRelease('session-1', 1) await expect(router.closeSession('session-1')).resolves.toBe(false) await router.closeAll() expect(closeAdapters).toHaveBeenCalledOnce() 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 6eb0d626a53..8f5af7d992b 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 @@ -6,7 +6,11 @@ import type { import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' type RoutedAgent = 'claude' | 'codex' -type SessionRoute = { adapter: StructuredAgentSessionAdapter; state: 'live' | 'stopped' } +type SessionRoute = { + adapter: StructuredAgentSessionAdapter + state: 'live' | 'stopped' + fence: number +} export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessionAdapter { private readonly routes = new Map() @@ -38,7 +42,7 @@ export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessi if (this.allAdaptersClosed) { throw new Error('structured session adapter router is closed') } - this.routes.set(input.identity.sessionId, { adapter, state: 'live' }) + this.routes.set(input.identity.sessionId, { adapter, state: 'live', fence: input.fence }) return acquired } @@ -200,10 +204,14 @@ export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessi /** Drops a per-session stop receipt after the host releases its durable owner, and hands that * release to the owning provider so its own session index cannot outlive the lease. */ - acknowledgeSessionRelease = (sessionId: string): void => { + acknowledgeSessionRelease = (sessionId: string, releasedFence: number): void => { const route = this.routes.get(sessionId) + // A release for an older fence says nothing about a route acquired since. + if (!route || route.fence > releasedFence) { + return + } this.routes.delete(sessionId) - route?.adapter.acknowledgeSessionRelease?.(sessionId) + route.adapter.acknowledgeSessionRelease?.(sessionId, releasedFence) } private owner(sessionId: string): StructuredAgentSessionAdapter { 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 9377fb549e8..877ee72f946 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 @@ -300,7 +300,7 @@ export type StructuredAgentSessionAdapter = { disposeSession?(sessionId: string): Promise /** Host acknowledgement that the lease and journal owner are released. This is the close * decision: the root is gone, though its descendants may still be unverified. */ - acknowledgeSessionRelease?(sessionId: string): void + acknowledgeSessionRelease?(sessionId: string, releasedFence: number): void } export async function rethrowAfterAgentSessionAcquisitionCleanup( diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-forward.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-forward.ts index b9559868f4b..7af08108745 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-forward.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-forward.ts @@ -52,13 +52,14 @@ export async function handoffStructuredSessionToTui( await rollbackPreparingNativeOwner(context, sessionId, operationId) throw new Error('agent_session_owner_exit_unproven') } + const nativeFence = record.lease.runtimeFence record = await stopStoredAgentSessionOwnerForHandoff(deps.store, { sessionId, - expectedFence: record.lease.runtimeFence, + expectedFence: nativeFence, operationId, now: deps.now() }) - deps.acknowledgeNativeRelease?.(sessionId) + deps.acknowledgeNativeRelease?.(sessionId, nativeFence) context.publishStage(record, 'to-tui') if (nativeSuspend.state === 'stopped-cleanup-failed') { await markStructuredHandoffManualRecovery(context, sessionId, operationId) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-types.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-types.ts index c1115f33a2b..a58f070fed5 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-types.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-types.ts @@ -78,7 +78,7 @@ export type StructuredAgentSessionHandoffDeps = { session: (sessionId: string) => { journal: AgentSessionJournal; fence: number } suspendNative: (sessionId: string) => Promise /** Consumes the router's stop proof after `old-owner-stopped` is durable. */ - acknowledgeNativeRelease?: (sessionId: string) => void + acknowledgeNativeRelease?: (sessionId: string, releasedFence: number) => void acquireNative: (input: { sessionId: string fence: number diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff.test.ts index 3c6277601af..367e7a755ce 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff.test.ts @@ -302,6 +302,7 @@ describe('structured session handoff failure handling', () => { } }) + const nativeFence = store.getRecord(SESSION)!.lease.runtimeFence await expect(handoffStructuredSessionToTui(context, request(operation), false)).rejects.toBe( cleanupError ) @@ -309,7 +310,7 @@ describe('structured session handoff failure handling', () => { expect(launchTui).not.toHaveBeenCalled() expect(retainOwner).not.toHaveBeenCalled() expect(releaseOwner).not.toHaveBeenCalled() - expect(acknowledgeNativeRelease).toHaveBeenCalledExactlyOnceWith(SESSION) + expect(acknowledgeNativeRelease).toHaveBeenCalledExactlyOnceWith(SESSION, nativeFence) expect(store.getRecord(SESSION)?.lease).toMatchObject({ runtimeKind: 'native', claimStatus: 'released', diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts index 41e2d8c61e3..c75e40618a6 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts @@ -101,7 +101,8 @@ export function createStructuredAgentSessionHostHandoff( return { state: 'stopped-cleanup-failed', error } } }, - acknowledgeNativeRelease: (sessionId) => deps.adapter.acknowledgeSessionRelease?.(sessionId), + acknowledgeNativeRelease: (sessionId, releasedFence) => + deps.adapter.acknowledgeSessionRelease?.(sessionId, releasedFence), acquireNative: (input) => acquireNativeHandoffOwner(deps, host, input), acquireNativeStop: (sessionId, turnId, fence) => stopNativeHandoffTurn(deps.adapter, host.session(sessionId), { sessionId, turnId, fence }), diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts index aee8a882576..23f4e043d17 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts @@ -88,7 +88,7 @@ export async function evictHeldStructuredAgentSession( }, forget: async () => { await forgetStructuredAgentSession(context, sessionId) - context.deps.adapter.acknowledgeSessionRelease?.(sessionId) + context.deps.adapter.acknowledgeSessionRelease?.(sessionId, session.fence) }, discardSink: () => context.runtimeState.discardEventSink(sessionId), settleWork: async () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts index 604edbe63c4..62c3c524107 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts @@ -107,7 +107,7 @@ export async function tearDownStructuredAgentSessionHost(input: { phases: readonly StructuredAgentSessionTeardownPhase[] sessions: Map retainSessionIds?: ReadonlySet - acknowledgeSessionRelease?: (sessionId: string) => void + acknowledgeSessionRelease?: (sessionId: string, releasedFence: number) => void }): Promise { const failures: unknown[] = [] for (const phase of input.phases) { @@ -124,13 +124,13 @@ export async function tearDownStructuredAgentSessionHost(input: { // `allSettled`, so one rejected close cannot skip the others. const closed = await Promise.allSettled(entries.map(([, session]) => session.journal.close())) closed.forEach((result, index) => { - const sessionId = entries[index]?.[0] + const [sessionId, session] = entries[index] ?? [] if (result.status === 'fulfilled') { // Only a FULFILLED close drops the entry. One that rejected stays indexed, // which is what makes a later close a real retry rather than a no-op. - if (sessionId !== undefined) { + if (sessionId !== undefined && session !== undefined) { input.sessions.delete(sessionId) - input.acknowledgeSessionRelease?.(sessionId) + input.acknowledgeSessionRelease?.(sessionId, session.fence) } return } @@ -171,7 +171,7 @@ export async function flushStructuredAgentSessionHost( }), sessions: context.sessions, retainSessionIds, - acknowledgeSessionRelease: (sessionId) => - context.deps.adapter.acknowledgeSessionRelease?.(sessionId) + acknowledgeSessionRelease: (sessionId, releasedFence) => + context.deps.adapter.acknowledgeSessionRelease?.(sessionId, releasedFence) }) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-journal-handles.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-journal-handles.test.ts index 1c9f271004c..bcd32316eed 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-journal-handles.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-journal-handles.test.ts @@ -206,7 +206,10 @@ describe('site 11: host teardown is failure-complete', () => { }) expect(sessions.size).toBe(0) - expect(acknowledgeSessionRelease.mock.calls).toEqual([[SESSION], [`${SESSION}-b`]]) + expect(acknowledgeSessionRelease.mock.calls).toEqual([ + [SESSION, 1], + [`${SESSION}-b`, 1] + ]) await expectNothingHoldsTheDirectory(journalDir) await expectNothingHoldsTheDirectory(join(root, 'journal-b')) }) @@ -253,7 +256,7 @@ describe('site 11: host teardown is failure-complete', () => { // Only the failure stays indexed — `status === 'fulfilled'`, not "settled". expect([...sessions.keys()]).toEqual([SESSION]) - expect(acknowledgeSessionRelease).toHaveBeenCalledExactlyOnceWith(`${SESSION}-b`) + expect(acknowledgeSessionRelease).toHaveBeenCalledExactlyOnceWith(`${SESSION}-b`, 1) await expectNothingHoldsTheDirectory(join(root, 'journal-b')) }) }) From f9fad0b2d3945a07dc32dadef13e544b905d8a87 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 24 Sep 2026 11:37:24 -0700 Subject: [PATCH 3/3] fix(claude): re-check a withheld exit so a surviving descendant cannot wedge the chat An unexpected exit whose close saw a descendant alive withheld `ended` with nothing re-checking it, so the host kept a dead child and the visible chat stayed wedged until restart. The withheld exit now joins the bounded released-child cleanup: a retry that proves the tree publishes `ended`, and giving up publishes it with the live verdict reported. The exit's publication is modelled as a state instead of a boolean, and the fence rule moved to a provider-neutral module so Codex can reuse it. --- .../claude-released-child-cleanup.test.ts | 18 ++++++ .../claude/claude-released-child-cleanup.ts | 28 ++++++++- .../claude-structured-session-adapter.ts | 7 ++- ...laude-structured-session-exit-lifecycle.ts | 21 ++++++- .../claude-structured-session-release.test.ts | 60 +++++++++++++++++-- .../claude-structured-session-retirement.ts | 15 ++--- .../claude/claude-structured-session-state.ts | 6 +- ...ructured-agent-session-fence-retirement.ts | 13 ++++ 8 files changed, 146 insertions(+), 22 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-fence-retirement.ts diff --git a/src/main/claude/claude-released-child-cleanup.test.ts b/src/main/claude/claude-released-child-cleanup.test.ts index 97de3a96dcc..1abb39291db 100644 --- a/src/main/claude/claude-released-child-cleanup.test.ts +++ b/src/main/claude/claude-released-child-cleanup.test.ts @@ -66,6 +66,24 @@ describe('ClaudeReleasedChildCleanup', () => { expect(cleanup.size).toBe(0) }) + it('tells its owner once whether a retry proved the tree or the schedule gave up', async () => { + const provenChild = await releasedChild([true]) + const stuckChild = await releasedChild([]) + const settled: [string, boolean][] = [] + cleanup.adopt('session-1', provenChild.connection, (proven) => settled.push(['s1', proven])) + cleanup.adopt('session-2', stuckChild.connection, (proven) => settled.push(['s2', proven])) + + await vi.advanceTimersByTimeAsync(10) + expect(settled).toEqual([['s1', true]]) + await vi.advanceTimersByTimeAsync(20) + + expect(settled).toEqual([ + ['s1', true], + ['s2', false] + ]) + expect(reports.map((report) => report.sessionId)).toEqual(['session-2']) + }) + it('never adopts a child whose tree is already proven', async () => { const { connection } = await releasedChild([], { root: 'exited', tree: 'exited' }) cleanup.adopt('session-1', connection) diff --git a/src/main/claude/claude-released-child-cleanup.ts b/src/main/claude/claude-released-child-cleanup.ts index df379b8a9a0..79ee1569bd8 100644 --- a/src/main/claude/claude-released-child-cleanup.ts +++ b/src/main/claude/claude-released-child-cleanup.ts @@ -12,10 +12,14 @@ export type ClaudeReleasedChildCleanupReport = { verdict: ClaudeChildExitVerdict } +/** Told once whether a retry proved the tree gone (`true`) or the schedule gave up (`false`). */ +export type ClaudeReleasedChildSettled = (treeProven: boolean) => void + type PendingCleanup = { sessionId: string attempts: number timer: ReturnType | undefined + onSettled: ClaudeReleasedChildSettled | undefined } function reportUnverifiedChild(report: ClaudeReleasedChildCleanupReport): void { @@ -51,11 +55,15 @@ export class ClaudeReleasedChildCleanup { return this.pending.size } - adopt(sessionId: string, connection: ClaudeStreamJsonConnection): void { + adopt( + sessionId: string, + connection: ClaudeStreamJsonConnection, + onSettled?: ClaudeReleasedChildSettled + ): void { if (this.pending.has(connection) || treeProven(connection.exitVerdict)) { return } - const entry: PendingCleanup = { sessionId, attempts: 0, timer: undefined } + const entry: PendingCleanup = { sessionId, attempts: 0, timer: undefined, onSettled } if (this.closed) { // Shutdown already ran its final pass; this child still gets exactly one. void this.finalAttempt(connection, entry) @@ -83,6 +91,7 @@ export class ClaudeReleasedChildCleanup { if (delay === undefined) { this.pending.delete(connection) this.reportUnverified(connection, entry) + this.settle(entry, false) return } entry.timer = setTimeout(() => void this.attempt(connection, entry), delay) @@ -101,6 +110,7 @@ export class ClaudeReleasedChildCleanup { } if (proven) { this.pending.delete(connection) + this.settle(entry, true) return } this.schedule(connection, entry) @@ -110,9 +120,21 @@ export class ClaudeReleasedChildCleanup { connection: ClaudeStreamJsonConnection, entry: PendingCleanup ): Promise { - if (!(await connection.close().catch(() => false))) { + const proven = await connection.close().catch(() => false) + if (!proven) { this.reportUnverified(connection, entry) } + this.settle(entry, proven) + } + + private settle(entry: PendingCleanup, treeProven: boolean): void { + const onSettled = entry.onSettled + entry.onSettled = undefined + try { + onSettled?.(treeProven) + } catch { + // The owner's settlement is its own; cleanup still ends here. + } } private reportUnverified(connection: ClaudeStreamJsonConnection, entry: PendingCleanup): void { diff --git a/src/main/claude/claude-structured-session-adapter.ts b/src/main/claude/claude-structured-session-adapter.ts index ecb02ba86d7..99acae541d5 100644 --- a/src/main/claude/claude-structured-session-adapter.ts +++ b/src/main/claude/claude-structured-session-adapter.ts @@ -67,6 +67,7 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda sessions: this.sessions, exits: this.exits, settledExitErrors: this.settledExitErrors, + cleanup: this.releasedChildren, deps, emit: (session, event) => this.emit(session, event) } @@ -135,8 +136,8 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda } /** Resolves once every first-hand exit observed so far has published its - * lifecycle event — or has seen a descendant alive and stayed indexed for a - * retry. Publication trails observation by the close ladder and the + * lifecycle event — or has seen a descendant alive and handed the withheld + * `ended` to the bounded cleanup. Publication trails observation by the close ladder and the * transcript cursor write, so nothing outside can otherwise tell the two * apart without guessing at wall-clock. */ drainObservedExits = (): Promise => drainClaudeObservedExits(this.exits) @@ -156,7 +157,7 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda // An exit that already published `ended` has no turn left to run. hasLiveSession: this.sessions.has(input.identity.sessionId) || - (exit !== undefined && exit.endedWithTreeUnproven !== true) + (exit !== undefined && exit.ended !== 'published') }) } diff --git a/src/main/claude/claude-structured-session-exit-lifecycle.ts b/src/main/claude/claude-structured-session-exit-lifecycle.ts index 2401c6e38cf..d0b8ce21d35 100644 --- a/src/main/claude/claude-structured-session-exit-lifecycle.ts +++ b/src/main/claude/claude-structured-session-exit-lifecycle.ts @@ -1,4 +1,5 @@ import { settledClaudeTurnEndLeaf } from './claude-structured-resume-point' +import type { ClaudeReleasedChildCleanup } from './claude-released-child-cleanup' import { claudeRootExitObserved, settleClaudeExitedSession @@ -17,6 +18,8 @@ export type ClaudeExitLifecycle = { exits: Map /** A settled exit's diagnostic, kept for a send admitted before the host heard of the exit. */ settledExitErrors: Map + /** Re-checks an exit whose close saw a descendant alive, so its `ended` is never withheld for good. */ + cleanup: ClaudeReleasedChildCleanup deps: Pick emit: (session: ClaudeSession, event: ClaudeStructuredSessionEvent) => void } @@ -49,11 +52,23 @@ export function observeClaudeSessionExit( return settleClaudeUnexpectedExit(lifecycle, sessionId, exit) } // The lease follows the root, so a first-hand root exit publishes `ended` even while the - // tree is unverifiable; only a descendant seen alive withholds it. + // tree is unverifiable. A descendant seen alive only defers it: the bounded cleanup re-runs + // the ladder and publishes on proof, or at give-up with the last verdict reported. if (claudeRootExitObserved(session.connection)) { - exit.endedWithTreeUnproven = true + exit.ended = 'published' return settleClaudeUnexpectedExit(lifecycle, sessionId, exit) } + const verdict = session.connection.exitVerdict + if (verdict.root !== 'exited' || verdict.tree !== 'live') { + return undefined + } + exit.ended = 'withheld' + lifecycle.cleanup.adopt(sessionId, session.connection, (treeProven) => { + if (!treeProven) { + exit.ended = 'published' + } + void settleClaudeUnexpectedExit(lifecycle, sessionId, exit).catch(() => undefined) + }) return undefined }) .catch(() => undefined) @@ -83,7 +98,7 @@ export function settleClaudeUnexpectedExit( return } // Unproven descendants stay indexed as evidence until the host's release retires them. - if (!exit.endedWithTreeUnproven) { + if (exit.ended !== 'published') { exits.delete(sessionId) } lifecycle.settledExitErrors.set(sessionId, exit.error) diff --git a/src/main/claude/claude-structured-session-release.test.ts b/src/main/claude/claude-structured-session-release.test.ts index 93c19050a3a..2c233fa5b57 100644 --- a/src/main/claude/claude-structured-session-release.test.ts +++ b/src/main/claude/claude-structured-session-release.test.ts @@ -4,7 +4,7 @@ import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { AgentSessionAcquisitionExitUnprovenError, AgentSessionAcquisitionRootExitObservedError @@ -26,16 +26,18 @@ import { } from './claude-structured-session-test-support' const ROOT_EXITED_TREE_UNVERIFIABLE = { root: 'exited', tree: 'unverifiable' } as const +const ROOT_EXITED_TREE_LIVE = { root: 'exited', tree: 'live' } as const async function acquiredWithCleanup( closeVerdict?: ClaudeStreamJsonConnection['exitVerdict'], - claudeConfigDir = '/accounts/claude' + claudeConfigDir = '/accounts/claude', + retryDelaysMs: readonly number[] = [] ) { const claude = fakeClaude(closeVerdict ? { unprovenCloseVerdict: closeVerdict } : {}) const events: ClaudeStructuredSessionEvent[] = [] const unverified: ClaudeReleasedChildCleanupReport[] = [] const cleanup = new ClaudeReleasedChildCleanup({ - retryDelaysMs: [], + retryDelaysMs, report: (report) => unverified.push(report) }) const adapter = new ClaudeStructuredSessionAdapter({ @@ -87,7 +89,11 @@ describe('Claude unexpected exit with an unverifiable tree', () => { }) it('still withholds ended while a descendant was seen alive', async () => { - const { adapter, claude, events } = await acquiredWithCleanup({ root: 'exited', tree: 'live' }) + const { adapter, claude, events } = await acquiredWithCleanup( + ROOT_EXITED_TREE_LIVE, + undefined, + [60_000] + ) claude.connections[0]!.handlers.onExit?.(new Error('claude stream-json exited (code 1)')) await adapter.drainObservedExits() @@ -132,6 +138,52 @@ describe('Claude unexpected exit with an unverifiable tree', () => { }) }) +describe('Claude unexpected exit while a descendant was seen alive', () => { + it('publishes the withheld ended once a cleanup retry proves the tree gone', async () => { + const { adapter, claude, events, cleanup, unverified } = await acquiredWithCleanup( + ROOT_EXITED_TREE_LIVE, + undefined, + [1, 60_000] + ) + const connection = claude.connections[0]! + connection.handlers.onExit?.(new Error('claude stream-json exited (code 1)')) + await adapter.drainObservedExits() + expect(endedEvents(events)).toEqual([]) + expect(cleanup.size).toBe(1) + + // The surviving descendant exits; the next scheduled ladder run can now prove the tree. + connection.exitVerdict = { root: 'exited', tree: 'exited' } + connection.close = async () => true + + await vi.waitFor(() => + expect(endedEvents(events)).toEqual([ + expect.objectContaining({ cause: 'unexpected-exit', fence: 7 }) + ]) + ) + expect(cleanup.size).toBe(0) + expect(unverified).toEqual([]) + await expect(adapter.releaseAcquisition({ sessionId: 'session-1' })).resolves.toBe(true) + }) + + it('publishes at give-up and reports the descendant as live, never gone', async () => { + const { adapter, claude, events, unverified } = await acquiredWithCleanup( + ROOT_EXITED_TREE_LIVE, + undefined, + [1] + ) + claude.connections[0]!.handlers.onExit?.(new Error('claude stream-json exited (code 1)')) + + await vi.waitFor(() => expect(endedEvents(events)).toHaveLength(1)) + expect(unverified).toEqual([ + { sessionId: 'session-1', pid: 4321, verdict: ROOT_EXITED_TREE_LIVE } + ]) + // The exit stays as evidence: cleanup still reports the descendant unproven. + await expect(adapter.releaseAcquisition({ sessionId: 'session-1' })).rejects.toBeInstanceOf( + AgentSessionAcquisitionExitUnprovenError + ) + }) +}) + describe('Claude acknowledged session release', () => { it('forgets a session whose close saw only the root leave, and hands its tree to cleanup', async () => { const { adapter, claude, cleanup, unverified } = await acquiredWithCleanup( diff --git a/src/main/claude/claude-structured-session-retirement.ts b/src/main/claude/claude-structured-session-retirement.ts index e666504bca5..a8a9258716b 100644 --- a/src/main/claude/claude-structured-session-retirement.ts +++ b/src/main/claude/claude-structured-session-retirement.ts @@ -1,3 +1,4 @@ +import { isAgentSessionChildReleasedThroughFence } from '../native-chat/agent-session-wire/structured-agent-session-fence-retirement' import type { ClaudeReleasedChildCleanup } from './claude-released-child-cleanup' import { settleClaudeExitedSession } from './claude-structured-session-close' import type { @@ -45,11 +46,8 @@ export function retireClaudeReleasedSession( } /** - * Retires the indexed child only when its fence is at or below the one the host no longer grants to - * it. Two facts deliver that: an acknowledged release names its fence, and an acquisition at fence - * F means F-1 is gone. The lease grants a later fence either after a release or over an unreleased - * lease whose owner probe proved the recorded process dead, so neither is evidence about THIS - * connection: a root not yet seen to exit here still goes through the close ladder. A stale + * Retires the indexed child only once it is past its lease. Two facts deliver the released fence: + * an acknowledged release names it, and an acquisition at fence F means F-1 is gone. A stale * acknowledgement can never reach a child acquired at a newer fence. */ export function retireClaudeSessionReleasedThrough( @@ -58,8 +56,11 @@ export function retireClaudeSessionReleasedThrough( const indexed = input.sessions.get(input.sessionId) ?? input.exits.get(input.sessionId)?.session if ( !indexed || - indexed.fence > input.releasedFence || - indexed.connection.exitVerdict.root === 'live' + !isAgentSessionChildReleasedThroughFence({ + childFence: indexed.fence, + releasedFence: input.releasedFence, + rootSeenLive: indexed.connection.exitVerdict.root === 'live' + }) ) { return undefined } diff --git a/src/main/claude/claude-structured-session-state.ts b/src/main/claude/claude-structured-session-state.ts index 5d878e7e2f7..29642662a21 100644 --- a/src/main/claude/claude-structured-session-state.ts +++ b/src/main/claude/claude-structured-session-state.ts @@ -207,8 +207,10 @@ export type ClaudeSessionExit = { /** The whole ladder-then-settle tail, retained so a barrier can await an exit * that is observed but not yet published. Never rejects. */ publication?: Promise - /** `ended` went out on the root's exit while the tree stayed unproven; kept only as evidence. */ - endedWithTreeUnproven?: boolean + /** Where `ended` stands for this exit. `withheld`: a descendant was seen alive and the bounded + * cleanup is re-checking. `published`: `ended` went out while the tree stayed unproven, so the + * exit is kept only as evidence until a release retires it. A proven exit leaves the map. */ + ended?: 'withheld' | 'published' } export type ClaudeAcquisitionAttempt = { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-fence-retirement.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-fence-retirement.ts new file mode 100644 index 00000000000..ec0ae85e93f --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-fence-retirement.ts @@ -0,0 +1,13 @@ +/** + * Whether a provider child indexed at `childFence` is past its lease. The lease grants a later + * fence after a release, or over an unreleased lease whose owner probe proved the recorded process + * dead; either way a fence at or below `releasedFence` no longer owns the session. Neither grant + * is evidence about the child's own process, so a root not yet seen to exit is never past it. + */ +export function isAgentSessionChildReleasedThroughFence(input: { + childFence: number + releasedFence: number + rootSeenLive: boolean +}): boolean { + return input.childFence <= input.releasedFence && !input.rootSeenLive +}