diff --git a/src/main/claude/claude-stream-json-connection-close.test.ts b/src/main/claude/claude-stream-json-connection-close.test.ts index 22076f3600e..348601bbd48 100644 --- a/src/main/claude/claude-stream-json-connection-close.test.ts +++ b/src/main/claude/claude-stream-json-connection-close.test.ts @@ -4,6 +4,7 @@ import type { ChildProcessWithoutNullStreams } from 'node:child_process' import { describe, expect, it, vi } from 'vitest' import type { query } from '@anthropic-ai/claude-agent-sdk' import { + CLAUDE_READER_DRAIN_AFTER_EXIT_MS, openClaudeStreamJsonConnection, type ClaudeStreamJsonLaunch } from './claude-stream-json-connection' @@ -162,6 +163,123 @@ describe('Claude stream-json close ordering', () => { expect(next).toHaveBeenCalledOnce() }) + // The ladder (stdin end, SIGTERM, SIGKILL) runs again for the next ask; nothing reuses the root. + it('runs the stop ladder again on a close after an unproven attempt, refusing input meanwhile', async () => { + mocks.refresh.mockReset() + mocks.proveClaudeChildExit.mockReset() + mocks.refresh.mockResolvedValue(undefined) + mocks.proveClaudeChildExit.mockResolvedValueOnce(false).mockResolvedValueOnce(true) + const child = fakeChild() + const next = vi.fn(async (): Promise>> => ({ + value: undefined, + done: true + })) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: This injected query exercises only the async iterator used by the connection. + const queryImpl = ((params: Parameters[0]) => { + params.options?.spawnClaudeCodeProcess?.({ + command: 'claude', + args: [], + env: {}, + signal: new AbortController().signal + }) + return { + [Symbol.asyncIterator]: () => ({ next }) + } + }) as unknown as typeof query + const connection = await openClaudeStreamJsonConnection( + { pathToClaudeCodeExecutable: 'claude', options: {}, cwd: '/work/repo' }, + {}, + () => child, + queryImpl + ) + + await expect(connection.close()).resolves.toBe(false) + await expect(connection.send({ type: 'user' })).rejects.toThrow() + await expect(connection.close()).resolves.toBe(true) + expect(mocks.proveClaudeChildExit).toHaveBeenCalledTimes(2) + }) + + // Whatever holds the output open past a proven exit (a process that escaped the tree) must not + // keep every send and Stop that joins this close waiting. + it('resolves a proven close though the output never ends', async () => { + mocks.refresh.mockReset() + mocks.proveClaudeChildExit.mockReset() + mocks.refresh.mockResolvedValue(undefined) + mocks.proveClaudeChildExit.mockResolvedValue(true) + const child = fakeChild() + const next = vi.fn(() => new Promise>>(() => {})) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: This injected query exercises only the async iterator used by the connection. + const queryImpl = ((params: Parameters[0]) => { + params.options?.spawnClaudeCodeProcess?.({ + command: 'claude', + args: [], + env: {}, + signal: new AbortController().signal + }) + return { + [Symbol.asyncIterator]: () => ({ next }) + } + }) as unknown as typeof query + const connection = await openClaudeStreamJsonConnection( + { pathToClaudeCodeExecutable: 'claude', options: {}, cwd: '/work/repo' }, + {}, + () => child, + queryImpl + ) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + vi.useFakeTimers() + try { + let closed: boolean | undefined + void connection.close().then((value) => { + closed = value + }) + await vi.advanceTimersByTimeAsync(CLAUDE_READER_DRAIN_AFTER_EXIT_MS - 1) + expect(closed).toBeUndefined() + await vi.advanceTimersByTimeAsync(1) + expect(closed).toBe(true) + expect(warn).toHaveBeenCalledWith( + '[claude-stream-json] output still open after the proven exit:', + expect.anything() + ) + } finally { + vi.useRealTimers() + warn.mockRestore() + } + }) + + it('reports a root that exits after its close came back unproven as that close ending', async () => { + mocks.refresh.mockReset() + mocks.proveClaudeChildExit.mockReset() + mocks.refresh.mockResolvedValue(undefined) + mocks.proveClaudeChildExit.mockResolvedValue(false) + const child = fakeChild() + const next = vi.fn(() => new Promise>>(() => {})) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: This injected query exercises only the async iterator used by the connection. + const queryImpl = ((params: Parameters[0]) => { + params.options?.spawnClaudeCodeProcess?.({ + command: 'claude', + args: [], + env: {}, + signal: new AbortController().signal + }) + return { + [Symbol.asyncIterator]: () => ({ next }) + } + }) as unknown as typeof query + const exits: (boolean | undefined)[] = [] + const connection = await openClaudeStreamJsonConnection( + { pathToClaudeCodeExecutable: 'claude', options: {}, cwd: '/work/repo' }, + { onExit: (_error, exit) => exits.push(exit?.expected) }, + () => child, + queryImpl + ) + await expect(connection.close()).resolves.toBe(false) + + child.emit('exit', 0, 'SIGTERM') + + expect(exits).toEqual([true]) + }) + it('waits for the live tree refresh before ending stdin', async () => { const refreshDone = Promise.withResolvers() mocks.refresh.mockReturnValueOnce(refreshDone.promise) diff --git a/src/main/claude/claude-stream-json-connection.ts b/src/main/claude/claude-stream-json-connection.ts index 6f67e04b36e..d6511a99863 100644 --- a/src/main/claude/claude-stream-json-connection.ts +++ b/src/main/claude/claude-stream-json-connection.ts @@ -15,6 +15,7 @@ import { type ClaudeControlSurface } from './claude-agent-sdk-control-requests' import { createClaudeChildTreeReaper, proveClaudeChildExit } from './claude-agent-sdk-exit-proof' +import { withTimeout } from '../../shared/promise-timeout-fallback' import type { DescendantTreeVerdict } from '../pty-descendant-exit-verification' import { createClaudeCodeProcessSpawn } from './claude-agent-sdk-process-spawn' import { @@ -25,6 +26,10 @@ import type { ClaudeStructuredSdkOptions } from './claude-structured-launch-reso export { ClaudeControlRequestError } +/** How long a proven close waits for messages already written before the exit; whatever still + * holds the output open past it is no reason to keep the close unresolved. */ +export const CLAUDE_READER_DRAIN_AFTER_EXIT_MS = 2_000 + /** * The SDK is loaded at the structured-Claude boundary rather than by this module's * import. The ordinary runtime's class graph statically reaches this file, and the @@ -33,6 +38,7 @@ export { ClaudeControlRequestError } * path never opted into, and a missing SDK would fail runtime startup. Memoized, * so a session pays the import once per process rather than once per connection. */ + let claudeAgentSdk: Promise | null = null function loadClaudeAgentSdk(): Promise { @@ -65,7 +71,9 @@ export type ClaudeStreamJsonConnectionHandlers = { onUserDialog?: OnUserDialog /** A transport/process fault that is not itself first-hand root exit proof. */ onFault?: (error: Error) => void - onExit?: (error: Error) => void + /** The root process exited, reported once. `expected`: a close had begun, so it is that close's + * end, even one that ran out of its own escalation first and came back unproven. */ + onExit?: (error: Error, exit?: { expected: boolean }) => void } /** @@ -222,9 +230,9 @@ export async function openClaudeStreamJsonConnection( faultReported = true handlers.onFault?.(terminalError) } - if (!closing && exited && !exitReported) { + if (exited && !exitReported) { exitReported = true - handlers.onExit?.(terminalError) + handlers.onExit?.(terminalError, { expected: closing }) } } @@ -330,7 +338,16 @@ export async function openClaudeStreamJsonConnection( closePromise = null return false } - await readerDone + const drained = await withTimeout( + readerDone.then(() => true), + CLAUDE_READER_DRAIN_AFTER_EXIT_MS, + false + ) + if (!drained) { + console.warn('[claude-stream-json] output still open after the proven exit:', { + pid: spawner.pid + }) + } return true })() return closePromise diff --git a/src/main/claude/claude-structured-control-actions.ts b/src/main/claude/claude-structured-control-actions.ts index 071cff0430c..03648b4bbb6 100644 --- a/src/main/claude/claude-structured-control-actions.ts +++ b/src/main/claude/claude-structured-control-actions.ts @@ -97,6 +97,32 @@ export async function stopClaudeBackgroundTasks( return { cancelled } } +/** Stops the tasks while `session` is still the one the host asked about, then publishes its child + * work so the host's records follow every acknowledged stop. */ +export async function stopCurrentClaudeBackgroundTasks(input: { + sessions: ReadonlyMap + session: ClaudeSession + sessionId: string + fence: number + taskIds: readonly string[] + timeoutMs: number | undefined + publishChildWork: (session: ClaudeSession) => void +}): Promise<{ cancelled: boolean }> { + const { sessions, session, sessionId, fence } = input + const acquisitionGeneration = session.acquisitionGeneration + const isCurrent = () => + sessions.get(sessionId) === session && + session.fence === fence && + session.acquisitionGeneration === acquisitionGeneration + try { + return await stopClaudeBackgroundTasks(session, input.timeoutMs, isCurrent, input.taskIds) + } finally { + if (isCurrent()) { + input.publishChildWork(session) + } + } +} + export async function answerClaudePrompt( session: ClaudeSession, claim: ClaudePromptClaim, diff --git a/src/main/claude/claude-structured-prompt-ownership.ts b/src/main/claude/claude-structured-prompt-ownership.ts index 7f68b583865..07e3960c6ed 100644 --- a/src/main/claude/claude-structured-prompt-ownership.ts +++ b/src/main/claude/claude-structured-prompt-ownership.ts @@ -209,3 +209,17 @@ export async function dismissClaudeStructuredPrompt(input: { session.prompts.releaseClaim(claim) } } + +/** An answered or dismissed request frees the child it blocked before the host records the card, + * so no row reads the child waiting beside a closed card; no provider frame says so first. */ +export function settleClaudePromptFreeingChild Promise }>( + input: { request: R; sessions: Map; free: () => void }, + settle: (input: { request: R; sessions: Map }) => Promise +): Promise { + const { request, sessions, free } = input + const commit = async (): Promise => { + free() + await request.commit() + } + return settle({ request: { ...request, commit }, sessions }).finally(free) +} diff --git a/src/main/claude/claude-structured-requested-stop.test.ts b/src/main/claude/claude-structured-requested-stop.test.ts index 993109f4a37..edcaff847bb 100644 --- a/src/main/claude/claude-structured-requested-stop.test.ts +++ b/src/main/claude/claude-structured-requested-stop.test.ts @@ -85,6 +85,7 @@ describe('a requested stop of a structured Claude chat', () => { expect(turns.at(-1)).toMatchObject({ turnId: 'turn-1', state: 'interrupted' }) const ended = events.filter((event) => event.type === 'ended') expect(ended).toEqual([expect.objectContaining({ reason: 'claude session closed' })]) - expect(ended[0]).not.toHaveProperty('cause') + // The host ends the child's record on it, as the close Orca asked for, never as a crash. + expect(ended[0]).toMatchObject({ cause: 'requested-close' }) }) }) diff --git a/src/main/claude/claude-structured-session-acquisition.ts b/src/main/claude/claude-structured-session-acquisition.ts index 404787620e9..39d2e0cdbe2 100644 --- a/src/main/claude/claude-structured-session-acquisition.ts +++ b/src/main/claude/claude-structured-session-acquisition.ts @@ -153,12 +153,9 @@ export async function acquireClaudeSession({ settle() } } - const { canUseTool, onUserDialog } = buildClaudePermissionCallbacks({ - sessionId, - prompts, - emit: (event) => - callbacks.deliver(attempt, sessionId, () => callbacks.emit(liveSession, input.events, event)) - }) + const emit = (event: Parameters[2]): void => + callbacks.deliver(attempt, sessionId, () => callbacks.emit(liveSession, input.events, event)) + const { canUseTool, onUserDialog } = buildClaudePermissionCallbacks({ sessionId, prompts, emit }) try { const launch = await resolveClaudeAcquisitionLaunch({ @@ -200,7 +197,12 @@ export async function acquireClaudeSession({ childEnded ??= error initProof.reject(error) }, - onExit: (error) => { + onExit: (error, exit) => { + if (exit?.expected) { + // The end of a close Orca began; that close settles it, or finishes it now. + callbacks.finishClose(sessionId, attempt) + return + } // The child exited on its own; marked in place, as the fault report may hold this error. withObservedProviderExit(error) childEnded ??= error @@ -218,8 +220,6 @@ export async function acquireClaudeSession({ deps.now ? { now: deps.now } : {} ) acquisitions.assertCurrent(sessionId, attempt) - const emit = (event: Parameters[2]): void => - callbacks.deliver(attempt, sessionId, () => callbacks.emit(liveSession, input.events, event)) if (connection.pid === undefined) { // A pid-less spawn always reports its error next; surface that, not the missing pid. await initProof.promise diff --git a/src/main/claude/claude-structured-session-adapter.ts b/src/main/claude/claude-structured-session-adapter.ts index 7dae79b408f..0e3176a1813 100644 --- a/src/main/claude/claude-structured-session-adapter.ts +++ b/src/main/claude/claude-structured-session-adapter.ts @@ -5,7 +5,7 @@ import type { StructuredAgentSessionAcquireInput, StructuredAgentSessionAdapter } from '../native-chat/agent-session-wire/structured-agent-session-adapter' -import { stopClaudeBackgroundTasks } from './claude-structured-control-actions' +import { stopCurrentClaudeBackgroundTasks } from './claude-structured-control-actions' import { dispatchClaudeTurn } from './claude-structured-dispatch' import { claudeHoldsDispatch } from './claude-command-lifecycle' import { releaseClaudeAcquisition } from './claude-structured-acquisition-release' @@ -26,7 +26,11 @@ import { type ClaudeStructuredSessionAdapterDeps, type ClaudeStructuredSessionEvent } from './claude-structured-session-state' -import { closeAllClaudeSessions, closeClaudeSession } from './claude-structured-session-close' +import { + closeAllClaudeSessions, + closeClaudeSession, + finishClaudeCloseAfterExit +} from './claude-structured-session-close' import { claudeStoppedRequestEndWait } from './claude-request-end-wait' import { drainClaudeObservedExits, @@ -40,7 +44,8 @@ import { claudePromptCardWritten, drainClaudeChildWork } from './claude-child-wo import { answerClaudeStructuredPrompt, cancelClaudeStructuredTurn, - dismissClaudeStructuredPrompt + dismissClaudeStructuredPrompt, + settleClaudePromptFreeingChild } from './claude-structured-prompt-ownership' import { claudePromptCancelRoute } from './claude-structured-prompt-replies' @@ -95,6 +100,14 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda emit: (session, _events, event) => this.emit(session, event), handleExit: (sessionId, attempt, error) => observeClaudeSessionExit(this.exitLifecycle, sessionId, attempt, error), + finishClose: (sessionId, attempt) => + finishClaudeCloseAfterExit({ + sessions: this.sessions, + sessionId, + connection: attempt.connection, + deps: this.deps, + afterClose: (close) => this.afterClose(sessionId, close) + }), settleExit: (sessionId, exit) => settleClaudeUnexpectedExit(this.exitLifecycle, sessionId, exit) } @@ -204,44 +217,23 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda awaitStoppedRequestEnd = claudeStoppedRequestEndWait(this.sessions) routePromptCancel = claudePromptCancelRoute dismissPrompt: NonNullable = (request) => - this.freeingAsker(request, (freeing) => - dismissClaudeStructuredPrompt({ request: freeing, sessions: this.sessions }) - ) - /** An answered or dismissed request frees the child it blocked before the host records the card, - * so no row reads the child waiting beside a closed card; no provider frame says so first. */ - private freeingAsker = Promise }>( - request: R, - settle: (request: R) => Promise - ): Promise => { - const free = () => this.publishChildWork(request.sessionId) - const commit = async (): Promise => { - free() - await request.commit() - } - return settle({ ...request, commit }).finally(free) - } + settleClaudePromptFreeingChild(this.asker(request), dismissClaudeStructuredPrompt) + /** The request with what frees the child it blocked (`settleClaudePromptFreeingChild`). */ + private asker = (request: R) => ({ + request, + sessions: this.sessions, + free: () => this.publishChildWork(request.sessionId) + }) stopBackgroundTasks: NonNullable = async ( input - ) => { - const session = this.session(input.sessionId) - const acquisitionGeneration = session.acquisitionGeneration - const isCurrent = () => - this.sessions.get(input.sessionId) === session && - session.fence === input.fence && - session.acquisitionGeneration === acquisitionGeneration - try { - return await stopClaudeBackgroundTasks( - session, - this.deps.requestTimeoutMs, - isCurrent, - input.taskIds - ) - } finally { - if (isCurrent()) { - this.publishChildWork(input.sessionId, session) - } - } - } + ) => + stopCurrentClaudeBackgroundTasks({ + ...input, + sessions: this.sessions, + session: this.session(input.sessionId), + timeoutMs: this.deps.requestTimeoutMs, + publishChildWork: (session) => this.publishChildWork(input.sessionId, session) + }) /** The tracker's own roster, for the tests that compare it with the host's child records. No * production code reads it: what runs, what a Stop reaches and what blocks a command are all * read from the host's child records. */ @@ -261,9 +253,7 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda return session ? claudeHoldsDispatch(session) : false } answerPrompt: StructuredAgentSessionAdapter['answerPrompt'] = (request) => - this.freeingAsker(request, (freeing) => - answerClaudeStructuredPrompt({ request: freeing, sessions: this.sessions }) - ) + settleClaudePromptFreeingChild(this.asker(request), answerClaudeStructuredPrompt) setOption: StructuredAgentSessionAdapter['setOption'] = (input) => setClaudeStructuredSessionOption( this.session(input.sessionId), @@ -327,7 +317,8 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda sessions: this.sessions, acquisitions: this.acquisitions, ...(this.deps.persistHandle ? { persistHandle: this.deps.persistHandle } : {}), - ...(this.deps.onEvent ? { onEvent: this.deps.onEvent } : {}) + ...(this.deps.onEvent ? { onEvent: this.deps.onEvent } : {}), + ...(this.deps.logger ? { logger: this.deps.logger } : {}) }) closeAll = (): Promise => diff --git a/src/main/claude/claude-structured-session-close.test.ts b/src/main/claude/claude-structured-session-close.test.ts index 8881c6ae74b..3d61bbb7357 100644 --- a/src/main/claude/claude-structured-session-close.test.ts +++ b/src/main/claude/claude-structured-session-close.test.ts @@ -126,7 +126,9 @@ describe('Claude published session close lifecycle', () => { ).sessions.get('session-1') const disposeTranslator = vi.spyOn(session!.translator!, 'dispose') - await expect(adapter.closeSession('session-1')).rejects.toBe(persistenceError) + // The exit is proven; the failed cursor write after it never reads as an unproven exit. + await expect(adapter.closeSession('session-1')).resolves.toBe(true) + await vi.waitFor(() => expect(persistHandle).toHaveBeenCalledOnce()) // The child is provably dead; a failed cursor write may not suppress the end. expect(events.filter((event) => event.type === 'ended')).toHaveLength(1) expect(events.filter((event) => event.type === 'handle')).toHaveLength(0) @@ -134,10 +136,9 @@ describe('Claude published session close lifecycle', () => { // A close Orca asked for stops the child still running, then the host hears the session end. expect(childWork).toEqual(['live', 'ended', 'session-ended']) + // Nothing of the dead child stays indexed for a retry. await expect(adapter.closeSession('session-1')).resolves.toBe(true) - expect(persistHandle).toHaveBeenCalledTimes(2) - // The retry persists the same cursor without a second lifecycle end. - expect(events.filter((event) => event.type === 'handle')).toHaveLength(1) + expect(persistHandle).toHaveBeenCalledOnce() expect(events.filter((event) => event.type === 'ended')).toHaveLength(1) expect(disposeTranslator).toHaveBeenCalledOnce() }) diff --git a/src/main/claude/claude-structured-session-close.ts b/src/main/claude/claude-structured-session-close.ts index 68f3dbbae1f..903f49f052e 100644 --- a/src/main/claude/claude-structured-session-close.ts +++ b/src/main/claude/claude-structured-session-close.ts @@ -7,6 +7,7 @@ import type { } from './claude-structured-session-state' import { cancelClaudeAcquisitionAttempt } from './claude-structured-session-state' import { + AgentSessionAcquisitionExitProvenError, AgentSessionAcquisitionExitUnprovenError, AgentSessionAcquisitionRootExitObservedError, AgentSessionPreSpawnError @@ -18,6 +19,7 @@ import { closeProcessRegistry } from '../../shared/child-process/close-process-r import { retireClaudeDispatchWaiters } from './claude-structured-dispatch' import { settledClaudeTurnEndLeaf } from './claude-structured-resume-point' import { settleClaudeTurnEndWaiters } from './claude-request-end-wait' +import type { StructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' /** The root's own exit was seen first-hand. The lease follows the root, so a descendant * left unverified or seen alive does not hold it. */ @@ -85,6 +87,7 @@ type CloseClaudePublishedSessionInput = { fence: number }) => Promise onEvent?: (event: ClaudeStructuredSessionEvent) => void + logger?: StructuredAgentSessionLogger } async function finalizeClaudePublishedSession( @@ -122,23 +125,10 @@ async function finalizeClaudePublishedSession( } session.childWork.clear() session.backgroundTasks.clear() - const leafUuid = await settledClaudeTurnEndLeaf(session) - const persistence = - session.closePersistence ?? - (session.closePersistence = (async () => { - await input.persistHandle?.({ - sessionId: input.sessionId, - providerSessionId: session.providerSessionId, - leafUuid, - fence: session.fence - }) - })()) - const ended = { - type: 'ended', - sessionId: input.sessionId, - reason: 'claude session closed', - observedAt: Date.now() - } as const + // The exit is proven, so the session ends now. Saving its resume point is bookkeeping that + // follows, reported on failure; it never holds the close or reads as an unproven exit. + session.closeFinalized = true + input.sessions.delete(input.sessionId) let callbackError: unknown let callbackThrew = false const deliver = (event: ClaudeStructuredSessionEvent): void => { @@ -149,31 +139,18 @@ async function finalizeClaudePublishedSession( callbackError ??= error } } - let persistenceError: unknown - try { - await persistence - session.closeFinalized = true - input.sessions.delete(input.sessionId) - deliver({ - type: 'handle', - sessionId: input.sessionId, - providerSessionId: session.providerSessionId, - leafUuid, - fence: session.fence - }) - } catch (error) { - // Keep the closed session indexed so a retry can persist the same cursor. - // Removing it first would turn a durable-write failure into a no-op retry. - if (session.closePersistence === persistence) { - session.closePersistence = undefined - } - persistenceError = error - } - // The connection already proved the child dead, so the session has ended - // whatever the durable write did: withholding it would strand the renderer on - // a session nothing re-drives. Emitted once, so a retry only re-persists. if (!session.closeEnded) { session.closeEnded = true + const ended = { + type: 'ended', + sessionId: input.sessionId, + reason: 'claude session closed', + // The host ends the child's record on it, whoever was still waiting on the close. + cause: 'requested-close', + fence: session.fence, + acquisitionGeneration: session.acquisitionGeneration, + observedAt: Date.now() + } as const try { try { session.translator?.handle(ended) @@ -186,18 +163,41 @@ async function finalizeClaudePublishedSession( session.translator?.dispose() } } - if (persistenceError) { - throw persistenceError - } - if (callbackThrew) { - throw callbackError - } + session.closePersistence ??= persistClosedClaudeSession(input, session) if (rootExitVerdict) { throw rootExitVerdict } + if (callbackThrew) { + // The exit is proven; only what followed it failed, which the caller reports. + throw new AgentSessionAcquisitionExitProvenError(callbackError) + } return true } +/** The resume point a closed session leaves for the next start, after the close already ended. */ +async function persistClosedClaudeSession( + input: CloseClaudePublishedSessionInput, + session: ClaudeSession +): Promise { + try { + const leafUuid = await settledClaudeTurnEndLeaf(session) + const handle = { + sessionId: input.sessionId, + providerSessionId: session.providerSessionId, + leafUuid, + fence: session.fence + } + await input.persistHandle?.(handle) + input.onEvent?.({ type: 'handle', ...handle }) + } catch (error) { + input.logger?.warn("saving a closed Claude session's resume point failed", { + scope: 'claude-close-resume-point', + sessionId: input.sessionId, + error + }) + } +} + export async function closeClaudePublishedSession( input: CloseClaudePublishedSessionInput ): Promise { @@ -233,11 +233,37 @@ export function closeClaudePublishedSessionForDeps( fence: number }) => Promise onEvent?: (event: ClaudeStructuredSessionEvent) => void + logger?: StructuredAgentSessionLogger } ): Promise { return closeClaudePublishedSession({ sessions, sessionId, ...deps }) } +/** The root exited after a close came back unproven: joins a close still running, or finishes that + * one for this exact child, through `afterClose`, which publishes the session's child work like + * any close. What failed after the exit is reported. */ +export function finishClaudeCloseAfterExit(input: { + sessions: Map + sessionId: string + connection: ClaudeStreamJsonConnection | null + deps: Parameters[2] + afterClose: (close: () => Promise) => Promise +}): void { + const { sessions, sessionId, deps } = input + if (sessions.get(sessionId)?.connection !== input.connection) { + return + } + void input + .afterClose(() => closeClaudePublishedSessionForDeps(sessions, sessionId, deps)) + .catch((error: unknown) => + deps.logger?.warn('finishing a Claude close after its process exited reported', { + scope: 'claude-close-after-exit', + sessionId, + error + }) + ) +} + export async function closeClaudeSession(input: { sessionId: string sessions: Map @@ -249,6 +275,7 @@ export async function closeClaudeSession(input: { fence: number }) => Promise onEvent?: (event: ClaudeStructuredSessionEvent) => void + logger?: StructuredAgentSessionLogger }): Promise { const attempt = input.acquisitions.get(input.sessionId) if (!(await cancelClaudeAcquisitionAttempt(attempt))) { diff --git a/src/main/claude/claude-structured-session-recovery.test.ts b/src/main/claude/claude-structured-session-recovery.test.ts index 71a7ac9ce31..a91b1d7c193 100644 --- a/src/main/claude/claude-structured-session-recovery.test.ts +++ b/src/main/claude/claude-structured-session-recovery.test.ts @@ -95,13 +95,16 @@ describe('ClaudeStructuredSessionAdapter close and exit recovery', () => { ).sessions.get('session-1') const disposeTranslator = vi.spyOn(session!.translator!, 'dispose') - await expect(adapter.closeSession('session-1')).rejects.toBe(callbackError) - expect(events.filter((event) => event.type === 'handle')).toHaveLength(1) + // The handle follows the proven close as bookkeeping; its callback's failure is reported. + await expect(adapter.closeSession('session-1')).resolves.toBe(true) + await vi.waitFor(() => + expect(events.filter((event) => event.type === 'handle')).toHaveLength(1) + ) expect(events.filter((event) => event.type === 'ended')).toHaveLength(1) expect(disposeTranslator).toHaveBeenCalledOnce() }) - it('retains a closed session until its durable cursor persistence succeeds', async () => { + it('does not keep a dead child indexed over a failed resume-point write', async () => { const claude = fakeClaude() const persistenceError = new Error('store unavailable') const persistHandle = vi @@ -111,10 +114,11 @@ describe('ClaudeStructuredSessionAdapter close and exit recovery', () => { const adapter = adapterFor(claude, {}, [], [], undefined, persistHandle) await adapter.acquire({ identity: identityFor(), fence: 7, spawnToken: 'spawn-9' }) - await expect(adapter.closeSession('session-1')).rejects.toBe(persistenceError) - expect(persistHandle).toHaveBeenCalledTimes(1) + // The exit is proven, so the close ends the session; the write after it is bookkeeping. await expect(adapter.closeSession('session-1')).resolves.toBe(true) - expect(persistHandle).toHaveBeenCalledTimes(2) + await vi.waitFor(() => expect(persistHandle).toHaveBeenCalledOnce()) + await expect(adapter.closeSession('session-1')).resolves.toBe(true) + expect(persistHandle).toHaveBeenCalledOnce() }) it('persists the last completed turn message before graceful close', async () => { @@ -141,6 +145,7 @@ describe('ClaudeStructuredSessionAdapter close and exit recovery', () => { await adapter.closeSession('session-1') + await vi.waitFor(() => expect(persistedHandles).toHaveLength(1)) expect(persistedHandles).toEqual([ { sessionId: 'session-1', @@ -149,7 +154,9 @@ describe('ClaudeStructuredSessionAdapter close and exit recovery', () => { fence: 7 } ]) - expect(events.at(-2)).toEqual({ + // Written after the close already ended the session: its end comes first. + expect(events.at(-2)).toMatchObject({ type: 'ended', cause: 'requested-close' }) + expect(events.at(-1)).toEqual({ type: 'handle', sessionId: 'session-1', providerSessionId: PROVIDER_SESSION_ID, diff --git a/src/main/claude/claude-structured-session-state.ts b/src/main/claude/claude-structured-session-state.ts index d61df04ac75..876090c07e4 100644 --- a/src/main/claude/claude-structured-session-state.ts +++ b/src/main/claude/claude-structured-session-state.ts @@ -1,3 +1,4 @@ +import type { StructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' import type { AgentJournalDispatchRejection } from '../../shared/agent-session-failure-words' import type { SubmissionRejectionFact } from '../../shared/agent-session-failure' import type { @@ -115,6 +116,8 @@ export type ClaudeStructuredSessionAdapterDeps = { leafUuid: string | null fence: number }) => Promise + /** Where bookkeeping a close or exit does after the child is gone reports a failure. */ + logger?: StructuredAgentSessionLogger /** Advance the durable resume point in place at a turn end; bookkeeping, never a turn failure. */ persistResumePoint?: (input: { sessionId: string @@ -345,5 +348,7 @@ export type ClaudeAcquireCallbacks = { event: ClaudeStructuredSessionEvent ) => void handleExit: (sessionId: string, attempt: ClaudeAcquisitionAttempt, error: Error) => void + /** The root exited during a close Orca began: finishes that close for this exact child. */ + finishClose: (sessionId: string, attempt: ClaudeAcquisitionAttempt) => void settleExit: (sessionId: string, exit: ClaudeSessionExit) => Promise } diff --git a/src/main/codex/codex-app-server-connection-types.ts b/src/main/codex/codex-app-server-connection-types.ts index df3f40ac39a..bf33bc5d0b8 100644 --- a/src/main/codex/codex-app-server-connection-types.ts +++ b/src/main/codex/codex-app-server-connection-types.ts @@ -8,7 +8,9 @@ export type CodexAppServerConnectionHandlers = { onNotification?: (method: string, params: unknown) => void onServerRequest?: (request: CodexAppServerServerRequest) => void onUnhandledFrame?: (kind: string, payload: unknown) => void - onExit?: (error: Error) => void + /** The root process exited, reported once. `expected`: a close had begun, so it is that close's + * end, even one that gave up first and came back unproven. */ + onExit?: (error: Error, exit?: { expected: boolean }) => void /** Awaited once the child has a pid and before the handshake; a rejection reaps the child. */ onSpawned?: (pid: number) => Promise } @@ -30,4 +32,6 @@ export type CodexAppServerConnection = { resumeReading?: () => void /** Resolves true only after the child emitted `exit` or `close`; false is unproven. */ close: () => Promise + /** The root exited, but the forced kill of its process tree could not prove the tree gone. */ + readonly processTreeUnproven?: boolean } diff --git a/src/main/codex/codex-app-server-connection.test.ts b/src/main/codex/codex-app-server-connection.test.ts index d36cfac47a4..797dd838eaf 100644 --- a/src/main/codex/codex-app-server-connection.test.ts +++ b/src/main/codex/codex-app-server-connection.test.ts @@ -542,6 +542,33 @@ describe('openCodexAppServerConnection', () => { await expect(connection.close()).resolves.toBe(true) }) + // A root that outlived one kill is killed again by the next ask, which then proves it gone. + it('kills the root again on a close after an unproven attempt', async () => { + vi.useFakeTimers() + const { child, spawnImpl } = stubChild({ exitOnStdinEnd: false }) + answerInitialize(child) + const connection = await openCodexAppServerConnection( + { command: 'codex', args: ['app-server'] }, + {}, + spawnImpl + ) + + const first = connection.close() + await vi.advanceTimersByTimeAsync(GRACEFUL_EXIT_MS + 3_500) + await expect(first).resolves.toBe(false) + child.kill.mockImplementation((signal) => { + if (signal === 'SIGKILL') { + setTimeout(() => child.emit('exit', null, 'SIGKILL'), 10) + } + return true + }) + + const second = connection.close() + await vi.advanceTimersByTimeAsync(GRACEFUL_EXIT_MS + 3_500) + await expect(second).resolves.toBe(true) + expect(child.kill.mock.calls.filter(([signal]) => signal === 'SIGKILL')).toHaveLength(2) + }) + it.each([1_090_188, 2_900_090])( 'accepts a realistic %i-byte escaped command completion and keeps processing', async (frameBytes) => { @@ -828,14 +855,14 @@ describe('openCodexAppServerConnection', () => { await connection.close() }) - it('keeps a graceful close quiet when stdin breaks during the reap', async () => { + it('reports a graceful close as expected when stdin breaks during the reap', async () => { vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) const { child, spawnImpl } = stubChild({ exitOnStdinEnd: false }) answerInitialize(child) - const exits: string[] = [] + const exits: (boolean | undefined)[] = [] const connection = await openCodexAppServerConnection( { command: 'codex', args: ['app-server'] }, - { onExit: (error) => exits.push(error.message) }, + { onExit: (_error, exit) => exits.push(exit?.expected) }, spawnImpl ) child.stdin.on('finish', () => child.stdin.emit('error', new Error('write EPIPE'))) @@ -858,7 +885,8 @@ describe('openCodexAppServerConnection', () => { await expect(closing).resolves.toBe(true) expect((await inFlight).message).toContain('EPIPE') - expect(exits).toHaveLength(0) + // The root's exit is the close's own end, never an unexpected death. + expect(exits).toEqual([true]) expect(vi.getTimerCount()).toBe(0) }) }) diff --git a/src/main/codex/codex-app-server-connection.ts b/src/main/codex/codex-app-server-connection.ts index 3e7d16ea1bb..070d3e3c301 100644 --- a/src/main/codex/codex-app-server-connection.ts +++ b/src/main/codex/codex-app-server-connection.ts @@ -79,6 +79,7 @@ export async function openCodexAppServerConnection( let exitObserved = false let closing = false let exitReported = false + let processTreeUnproven = false const exitProof = new RetryableProcessExitProof() /** First terminal cause, or null while the transport is still usable. Set once: * a child that dies reaches us through several listeners, and the specific @@ -126,9 +127,9 @@ export async function openCodexAppServerConnection( // Transport/protocol failures make the connection unusable immediately so // callers do not hang, but recovery must not treat that as a child exit // until the execution host has observed `exit`/`close`. - if (exitObserved && !closing && !exitReported) { + if (exitObserved && !exitReported) { exitReported = true - handlers.onExit?.(terminalError) + handlers.onExit?.(terminalError, { expected: closing }) } } @@ -257,11 +258,10 @@ export async function openCodexAppServerConnection( ) if (!exited) { const treeExited = await terminateProcessTree() - if (!treeExited) { - dispatcher.failPending(new Error('codex app-server process-tree exit was not proven')) - return false - } await waitForProcessExitUntil(exitPromise, FORCED_EXIT_MS) + // The lease follows the root, which is gone: a child left behind is reported by the + // owner, and blocks nothing. + processTreeUnproven = !treeExited && exitObserved } } dispatcher.failPending(new Error('codex app-server connection closed')) @@ -276,6 +276,9 @@ export async function openCodexAppServerConnection( get closed() { return closing || exited || terminalError !== null }, + get processTreeUnproven() { + return processTreeUnproven + }, request, notify, respond: (id, result) => writeResponse({ id, result }), diff --git a/src/main/codex/codex-close-turn-end.test.ts b/src/main/codex/codex-close-turn-end.test.ts index 98ae89a7347..dae441a3526 100644 --- a/src/main/codex/codex-close-turn-end.test.ts +++ b/src/main/codex/codex-close-turn-end.test.ts @@ -58,7 +58,6 @@ function sessionWithRunningTurn() { }, backgroundTasks: new CodexBackgroundTaskTracker('thread-1'), ended: false, - requestedClose: false, fence: 7, acquisitionGeneration: 'generation-1', threadId: 'thread-1', diff --git a/src/main/codex/codex-requested-close-turn-timing.test.ts b/src/main/codex/codex-requested-close-turn-timing.test.ts index 454c3ccd4a0..8652919924f 100644 --- a/src/main/codex/codex-requested-close-turn-timing.test.ts +++ b/src/main/codex/codex-requested-close-turn-timing.test.ts @@ -1,7 +1,6 @@ import { createCodexDispatchEchoes } from './codex-structured-dispatch-echo' import { createCodexTurnOpenWaits } from './codex-structured-turn-open-wait' import { afterEach, describe, expect, it, vi } from 'vitest' -import type { AgentJournalItemBody } from '../../shared/agent-session-journal-types' import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import { CodexBackgroundTaskTracker } from './codex-background-task-tracker' import { createCodexJournalTranslator } from './codex-structured-journal-translation' @@ -12,28 +11,19 @@ import type { CodexSession } from './codex-structured-session-state' afterEach(() => vi.useRealTimers()) describe('requested-close durable turn timing', () => { + // The exit is observed, so a refused terminal row is the host's to settle, never a reason to + // keep the dead child indexed for a retry. it.each([true, false])( - 'keeps the first exit receipt when retry requestedClose=%s', + 'ends the session with its exit receipt though its terminal row is refused (requestedClose=%s)', async (requestedClose) => { vi.useFakeTimers() vi.setSystemTime(1_000) - const terminalBodies: AgentJournalItemBody[] = [] - let refuseSettlement = true + // Backpressure refuses the terminal row the close's end would write. const sink: StructuredAgentSessionEventSink = { appendItem: () => {}, appendTombstone: () => {}, publish: () => {}, - tryAppendLifecycleBatch: (_id, mutations) => { - if (refuseSettlement) { - return { accepted: false, reason: 'backpressure' } - } - for (const mutation of mutations) { - if (mutation.kind === 'item') { - terminalBodies.push(mutation.body) - } - } - return { accepted: true } - } + tryAppendLifecycleBatch: () => ({ accepted: false, reason: 'backpressure' }) } const translator = createCodexJournalTranslator({ sink, @@ -63,7 +53,6 @@ describe('requested-close durable turn timing', () => { }, backgroundTasks: new CodexBackgroundTaskTracker('thread-1'), ended: false, - requestedClose: false, fence: 7, acquisitionGeneration: 'generation-1', threadId: 'thread-1', @@ -79,22 +68,11 @@ describe('requested-close durable turn timing', () => { const onEvent = vi.fn() vi.setSystemTime(2_000) - await expect(closeCodexPublishedSession(sessions, 'session-1')).resolves.toBe(false) - expect(sessions.get('session-1')).toBe(session) - expect(session.ended).toBe(false) - - refuseSettlement = false - vi.setSystemTime(60_000) - await expect( - closeCodexPublishedSession(sessions, 'session-1', onEvent, { - expectedAcquisitionGeneration: 'replacement-generation' - }) - ).resolves.toBe(false) - expect(onEvent).not.toHaveBeenCalled() await expect( closeCodexPublishedSession(sessions, 'session-1', onEvent, { requestedClose }) ).resolves.toBe(true) expect(sessions.has('session-1')).toBe(false) + expect(session.ended).toBe(true) expect(onEvent).toHaveBeenCalledWith( expect.objectContaining({ cause: requestedClose ? 'requested-close' : 'unexpected-exit', @@ -102,12 +80,6 @@ describe('requested-close durable turn timing', () => { observedAt: 2_000 }) ) - expect(terminalBodies.find((body) => body.kind === 'turn')).toMatchObject({ - kind: 'turn', - state: 'interrupted', - startedAt: 1_000, - completedAt: 2_000 - }) } ) }) diff --git a/src/main/codex/codex-structured-session-acquire.ts b/src/main/codex/codex-structured-session-acquire.ts index ae965795f7d..1b704fa3946 100644 --- a/src/main/codex/codex-structured-session-acquire.ts +++ b/src/main/codex/codex-structured-session-acquire.ts @@ -111,7 +111,14 @@ export async function acquireCodexStructuredSession(input: { previous: previousAttempt }) acquisitions.assertCurrent(sessionId, attempt) - if (!(await closeCodexPublishedSession(sessions, sessionId, deps.onEvent))) { + if ( + !(await closeCodexPublishedSession( + sessions, + sessionId, + deps.onEvent, + deps.logger ? { logger: deps.logger } : {} + )) + ) { throw new Error(`codex app-server for session ${sessionId} could not be stopped`) } acquisitions.assertCurrent(sessionId, attempt) @@ -163,13 +170,16 @@ export async function acquireCodexStructuredSession(input: { Buffer.byteLength(JSON.stringify(payload ?? null), 'utf8') ), onSpawned: spawnIdentity.onSpawned, - onExit: (error) => { + onExit: (error, exit) => { try { handleCodexSessionExit({ sessions, sessionId, connection: acquisition.connection, error, + // The end of a close Orca began, even one that came back unproven before it. + ...(exit?.expected ? { closedByOrca: true as const } : {}), + ...(deps.logger ? { logger: deps.logger } : {}), prompts: acquisition.prompts, ...(deps.onEvent ? { onEvent: deps.onEvent } : {}) }) diff --git a/src/main/codex/codex-structured-session-adapter-fixture.ts b/src/main/codex/codex-structured-session-adapter-fixture.ts index f2b4e102c35..37d434ef1a4 100644 --- a/src/main/codex/codex-structured-session-adapter-fixture.ts +++ b/src/main/codex/codex-structured-session-adapter-fixture.ts @@ -50,7 +50,7 @@ export function fakeCodex(routes: Record = {}): { routes: Record } { const connections: FakeConnection[] = [] - const openConnection = (async (launch, handlers = {}) => { + const openConnection: typeof openCodexAppServerConnection = async (launch, handlers = {}) => { const connection: FakeConnection = { launch, handlers, @@ -67,15 +67,21 @@ export function fakeCodex(routes: Record = {}): { notify: () => {}, respond: (id, result) => connection.replies.push({ id, result }), respondWithError: (id, code, message) => connection.replies.push({ id, code, message }), + // As the real connection: the root's exit is reported, once, inside the close that ends it. close: async () => { connection.closeCount += 1 - connection.closed = true + if (!connection.closed) { + connection.closed = true + handlers.onExit?.(new Error('codex app-server connection ended: killed'), { + expected: true + }) + } return true } } connections.push(connection) return connection - }) as typeof openCodexAppServerConnection + } routes['thread/start'] ??= () => ({ thread: { id: THREAD_ID, path: '/rollouts/abc.jsonl' }, model: 'gpt-live', diff --git a/src/main/codex/codex-structured-session-adapter.ts b/src/main/codex/codex-structured-session-adapter.ts index 6f67b68f8e6..288e08b9e34 100644 --- a/src/main/codex/codex-structured-session-adapter.ts +++ b/src/main/codex/codex-structured-session-adapter.ts @@ -73,6 +73,7 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap sessions: this.sessions, acquisitions: this.acquisitions, ...(deps.onEvent ? { onEvent: deps.onEvent } : {}), + ...(deps.logger ? { logger: deps.logger } : {}), forgetNotificationRetries: (sessionId) => this.notificationRetries.clear(sessionId, null) }) } diff --git a/src/main/codex/codex-structured-session-close.test.ts b/src/main/codex/codex-structured-session-close.test.ts index 4906b45a529..54cebde991e 100644 --- a/src/main/codex/codex-structured-session-close.test.ts +++ b/src/main/codex/codex-structured-session-close.test.ts @@ -96,7 +96,7 @@ describe('Codex structured session close lifecycle', () => { connection, backgroundTasks: new CodexBackgroundTaskTracker('thread-1'), ended: false, - requestedClose, + ...(requestedClose ? { orcaClose: { requested: true, reason: new Error('closed') } } : {}), fence: 7, acquisitionGeneration: 'generation-1', threadId: THREAD, @@ -134,7 +134,7 @@ describe('Codex structured session close lifecycle', () => { expect(translator.handle).toHaveBeenCalledOnce() }) - it("ends a Stop's wait for its turn to open when a requested close cannot publish its end yet", async () => { + it("ends the session and a Stop's wait for its turn to open when a requested close cannot publish its end", async () => { const { connection, prompts, session, sessions } = backpressuredSession(true) let released = false void session.turnOpenWaits.wait('turn-1', 60_000).then(() => { @@ -150,10 +150,10 @@ describe('Codex structured session close lifecycle', () => { closedByOrca: true, prompts }) - ).toBe(false) + ).toBe(true) await Promise.resolve() - // Left for the retry, but the child is gone: nothing waits on a turn it would open. - expect(session.ended).toBe(false) + // The exit is observed: the refused row is the host's to settle, and nothing waits on a turn. + expect(session.ended).toBe(true) expect(released).toBe(true) }) diff --git a/src/main/codex/codex-structured-session-close.ts b/src/main/codex/codex-structured-session-close.ts index fe6c0d67673..465d7801fec 100644 --- a/src/main/codex/codex-structured-session-close.ts +++ b/src/main/codex/codex-structured-session-close.ts @@ -8,18 +8,18 @@ import { type CodexStructuredSessionEvent } from './codex-structured-session-state' import type { StructuredAgentSessionEndedEvent } from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import type { StructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' export function handleCodexSessionExit(input: { sessions: Map sessionId: string connection: CodexAppServerConnection | null error: Error - /** Set by Orca's own close. Absent only from the connection's onExit, which the connection - * withholds while Orca is closing the child. */ + /** Set for the end of a close Orca began: by that close, or by the root's own exit report. */ closedByOrca?: true prompts?: CodexSession['prompts'] - allowFailedSettlement?: boolean onEvent?: (event: CodexStructuredSessionEvent) => void + logger?: StructuredAgentSessionLogger }): boolean { const session = input.sessions.get(input.sessionId) if (!session || session.connection !== input.connection || session.ended) { @@ -32,23 +32,27 @@ export function handleCodexSessionExit(input: { const event: StructuredAgentSessionEndedEvent = { type: 'ended', sessionId: input.sessionId, - reason: input.error.message, + // The connection reports the exit inside the close it ends; the close's own reason is the why. + reason: ((input.closedByOrca && session.orcaClose?.reason) || input.error).message, // Only the child's own exit blames Codex; a close Orca made, for any reason, is Orca's. failure: input.closedByOrca ? agentSessionFailureFact('hostFault') : agentSessionFailureFact('providerExited', { detail: providerDiagnosticOf(input.error) }), - cause: session.requestedClose ? 'requested-close' : 'unexpected-exit', + cause: session.orcaClose?.requested ? 'requested-close' : 'unexpected-exit', fence: session.fence, acquisitionGeneration: session.acquisitionGeneration, observedAt: session.exitObservedAt } as const // A synchronous sink rejection (usually backpressure) leaves the terminal rows to the host's - // exit settlement, which writes its own bounded fallback. + // exit settlement, which writes its own bounded fallback. The exit itself is observed, so the + // session ends either way: holding a dead child as unproven over a refused row would strand it. const admission = session.translator?.handle(event) ?? { accepted: true } - // The connection invokes onExit exactly once, so an unexpected exit is forwarded even when - // admission is backpressured; waiting for a second callback would strand the lease. - if (!admission.accepted && event.cause !== 'unexpected-exit' && !input.allowFailedSettlement) { - return false + if (!admission.accepted) { + input.logger?.warn("Codex's final rows were refused; the host settles the turn instead", { + scope: 'codex-exit-rows', + sessionId: input.sessionId, + reason: admission.reason + }) } session.ended = true // Nothing can echo for this child any more; the journal's pending-submission @@ -69,7 +73,7 @@ export async function closeCodexPublishedSession( sessionId: string, onEvent?: (event: CodexStructuredSessionEvent) => void, options?: { - allowFailedSettlement?: boolean + logger?: StructuredAgentSessionLogger requestedClose?: boolean expectedFence?: number expectedAcquisitionGeneration?: string @@ -89,7 +93,10 @@ export async function closeCodexPublishedSession( } // Sink-failure recovery force-closes the child but must preserve the // observed-exit cause so host lease settlement runs as an unexpected death. - session.requestedClose = options?.requestedClose ?? true + session.orcaClose = { + requested: options?.requestedClose ?? true, + reason: options?.unexpectedReason ?? new Error('codex session closed') + } // Keep the session indexed until the child exit is observed. A timeout or // failed kill must leave the live connection available for a safe retry. const exited = await session.connection.close() @@ -97,21 +104,16 @@ export async function closeCodexPublishedSession( return false } if (!session.ended) { - const handled = handleCodexSessionExit({ + handleCodexSessionExit({ sessions, sessionId, connection: session.connection, - error: options?.unexpectedReason ?? new Error('codex session closed'), + error: session.orcaClose.reason, closedByOrca: true, prompts: session.prompts, - ...(options?.allowFailedSettlement ? { allowFailedSettlement: true } : {}), - ...(onEvent ? { onEvent } : {}) + ...(onEvent ? { onEvent } : {}), + ...(options?.logger ? { logger: options.logger } : {}) }) - // Keep the closed session indexed when terminal settlement admission was - // rejected; a later close attempt retries the same stable lifecycle event. - if (!handled) { - return false - } } sessions.delete(sessionId) return true @@ -121,7 +123,8 @@ export async function closeCodexSession( sessionId: string, sessions: Map, acquisitions: CodexAcquisitionRegistry, - onEvent?: (event: CodexStructuredSessionEvent) => void + onEvent?: (event: CodexStructuredSessionEvent) => void, + logger?: StructuredAgentSessionLogger ): Promise { const attempt = acquisitions.get(sessionId) if (!(await cancelCodexAcquisitionAttempt(attempt))) { @@ -130,7 +133,7 @@ export async function closeCodexSession( if (attempt) { acquisitions.deleteIfCurrent(sessionId, attempt) } - return closeCodexPublishedSession(sessions, sessionId, onEvent) + return closeCodexPublishedSession(sessions, sessionId, onEvent, logger ? { logger } : {}) } export async function closeAllCodexSessions( diff --git a/src/main/codex/codex-structured-session-options-catalog.test.ts b/src/main/codex/codex-structured-session-options-catalog.test.ts index ad4935e5d4b..a5f0c1fa69d 100644 --- a/src/main/codex/codex-structured-session-options-catalog.test.ts +++ b/src/main/codex/codex-structured-session-options-catalog.test.ts @@ -53,7 +53,6 @@ function storeSession( }, backgroundTasks: new CodexBackgroundTaskTracker('thread-1'), ended: false, - requestedClose: false, fence: 1, acquisitionGeneration: 'generation-1', threadId: 'thread-1', diff --git a/src/main/codex/codex-structured-session-options.test.ts b/src/main/codex/codex-structured-session-options.test.ts index 4df25a9477c..fe30a2f42f9 100644 --- a/src/main/codex/codex-structured-session-options.test.ts +++ b/src/main/codex/codex-structured-session-options.test.ts @@ -27,7 +27,6 @@ function optionSession(request: CodexAppServerConnection['request']): CodexSessi }, backgroundTasks: new CodexBackgroundTaskTracker('thread-1'), ended: false, - requestedClose: false, fence: 1, acquisitionGeneration: 'generation-1', threadId: 'thread-1', diff --git a/src/main/codex/codex-structured-session-state.ts b/src/main/codex/codex-structured-session-state.ts index 6ddc11ac1f6..f70517668b5 100644 --- a/src/main/codex/codex-structured-session-state.ts +++ b/src/main/codex/codex-structured-session-state.ts @@ -1,3 +1,4 @@ +import type { StructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' import type { AgentJournalItemIdentity, AgentSessionJournalIdentity @@ -77,6 +78,8 @@ export type CodexStructuredSessionAdapterDeps = { /** Host capability seam; production uses the native Windows process table. */ isWindowsProcessStartTimeAvailable?: () => boolean onEvent?: (event: CodexStructuredSessionEvent) => void + /** Where bookkeeping a close or exit does after the child is gone reports a failure. */ + logger?: StructuredAgentSessionLogger /** What the session's child work did, delivered after the journal handled the frame. */ onChildWorkEvidence?: (sessionId: string, evidence: AgentChildWorkEvidence[]) => void /** A send admitted earlier: its identity once Codex echoes it, or its rejection when the turn @@ -105,7 +108,9 @@ export type CodexSession = { ended: boolean /** First observed child exit survives rejected settlement admission. */ exitObservedAt?: number - requestedClose: boolean + /** The close Orca began for this child: asked for, or forced as a death, and why. Whatever ends + * the child after it (that close, or the exit the connection reports meanwhile) keeps this. */ + orcaClose?: { requested: boolean; reason: Error } fence: number acquisitionGeneration: string threadId: string @@ -145,13 +150,9 @@ export function mintCodexAcquisitionGeneration(deps: CodexStructuredSessionAdapt export function codexSessionLifecycle( fence: number, acquisitionGeneration: string -): Pick< - CodexSession, - 'ended' | 'requestedClose' | 'fence' | 'acquisitionGeneration' | 'turnOpenWaits' -> { +): Pick { return { ended: false, - requestedClose: false, fence, acquisitionGeneration, turnOpenWaits: createCodexTurnOpenWaits() diff --git a/src/main/codex/codex-structured-session-teardown.ts b/src/main/codex/codex-structured-session-teardown.ts index 80b06ab1c36..e7395fb3c84 100644 --- a/src/main/codex/codex-structured-session-teardown.ts +++ b/src/main/codex/codex-structured-session-teardown.ts @@ -4,6 +4,8 @@ // session owns are cleared exactly once, and only when the child was actually // proven stopped — a refused close leaves the session indexed for a retry. +import { AgentSessionAcquisitionRootExitObservedError } from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import type { StructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' import { closeAllCodexSessions, closeCodexPublishedSession, @@ -19,6 +21,7 @@ export type CodexStructuredSessionTeardownDeps = { sessions: Map acquisitions: CodexAcquisitionRegistry onEvent?: (event: CodexStructuredSessionEvent) => void + logger?: StructuredAgentSessionLogger forgetNotificationRetries: (sessionId: string) => void } @@ -26,13 +29,24 @@ export class CodexStructuredSessionTeardown { constructor(private readonly deps: CodexStructuredSessionTeardownDeps) {} close = async (sessionId: string): Promise => { - const closed = await closeCodexSession( + const connection = this.deps.sessions.get(sessionId)?.connection + const closed = this.settled( sessionId, - this.deps.sessions, - this.deps.acquisitions, - this.deps.onEvent + await closeCodexSession( + sessionId, + this.deps.sessions, + this.deps.acquisitions, + this.deps.onEvent, + this.deps.logger + ) ) - return this.settled(sessionId, closed) + if (closed && connection?.processTreeUnproven) { + // The root exited, so the close is proven; its owner reports the children left unconfirmed. + throw new AgentSessionAcquisitionRootExitObservedError( + new Error('codex app-server exited, but its process tree was not proven gone') + ) + } + return closed } forceClose = async (sessionId: string): Promise => { @@ -40,7 +54,7 @@ export class CodexStructuredSessionTeardown { this.deps.sessions, sessionId, this.deps.onEvent, - { allowFailedSettlement: true, requestedClose: false } + { requestedClose: false } ) return this.settled(sessionId, closed) } @@ -63,7 +77,6 @@ export class CodexStructuredSessionTeardown { return Promise.resolve(false) } return closeCodexPublishedSession(this.deps.sessions, sessionId, this.deps.onEvent, { - allowFailedSettlement: true, requestedClose: false, expectedFence: fence, expectedAcquisitionGeneration: acquisitionGeneration, diff --git a/src/main/native-chat/agent-session-journal/journal-host-database.test.ts b/src/main/native-chat/agent-session-journal/journal-host-database.test.ts index 84ff9775022..2b4e4a0819a 100644 --- a/src/main/native-chat/agent-session-journal/journal-host-database.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-host-database.test.ts @@ -232,8 +232,9 @@ describe('quit', () => { const journal = await journals.open({ identity: IDENTITY, stateDirectory: root }) const database = openTestJournalHostDatabase(root) const installed = { - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: teardown calls only `flushAllStreamedEvents` on the host. + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: teardown calls only `stopDelivery` and `flushAllStreamedEvents` on the host. host: { + stopDelivery: () => {}, flushAllStreamedEvents: async () => { // A child's last row, delivered while quit is draining its sink. await new Promise((resolve) => setTimeout(resolve, 10)) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts index b3f91aea700..1b4a4e95b8c 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-accept-then-deliver.test.ts @@ -773,9 +773,10 @@ describe('an eviction between acceptance and handover', () => { }) }) -// A close abandons what is queued before it stops the child, so a release that then fails still -// leaves every queued message rejected as closed, never blamed on the provider. -describe('a close that stops the child and then fails', () => { +// A close abandons what is queued before it stops the child, so a wind-down step that then fails +// (reported, never the close's failure) still leaves every queued message rejected as closed, +// never blamed on the provider. +describe('a close that stops the child and then a wind-down step fails', () => { const END_CHILD = { evict: () => host.close(SESSION, 'evict') } satisfies Partial Promise>> @@ -803,8 +804,7 @@ describe('a close that stops the child and then fails', () => { const id = await accept('hello') await eventually(() => expect(acquire).toHaveBeenCalledTimes(2)) - await expect(END_CHILD[end]()).rejects.toThrow() - expect(host.hasSession(SESSION)).toBe(true) + await expect(END_CHILD[end]()).resolves.toBeUndefined() started.resolve() await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-agent-start.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-agent-start.ts index d2ef237aa1b..ec1289d401d 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-agent-start.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-agent-start.ts @@ -29,9 +29,10 @@ import { attachStructuredAgentSessionUnderSerialize } from './structured-agent-s import { failedCreateRefusal } from './structured-agent-session-failed-create-refusal' import { adapterSupportsRecord } from './structured-agent-session-provider-support' import { - finishOwedStructuredAgentSessionWindDownUnderSerialize, - type StructuredAgentSessionLifetimeContext -} from './structured-agent-session-host-lifetime' + joinClosingStructuredAgentSessionChild, + releaseLeaseOfEndedStructuredAgentSessionChild +} from './structured-agent-session-child-close' +export { joinClosingStructuredAgentSessionChild } import { structuredAgentSessionResumeOperationId, structuredAgentSessionResumeParams @@ -52,48 +53,6 @@ export type StructuredAgentSessionResumeOutcome = /** The attach's caller key: the ledger row a start settles is Orca's own. */ const AGENT_START_CALLER_KEY = 'trusted-local:agent-start' -/** - * Every operation that reaches the provider finishes a stop an earlier attempt left owed first: the - * child that stop could not prove gone takes no input, and none may start beside it. Still - * unproven, the operation is refused with the exit `unverifiable`, never assumed `exited`. - */ -export async function finishOwedStructuredAgentSessionStop( - context: StructuredAgentSessionLifetimeContext, - sessionId: string -): Promise { - if (await finishOwedStructuredAgentSessionWindDownUnderSerialize(context, sessionId)) { - return { ok: true } - } - return { - ok: false, - refusal: refuse( - 'agent_session_ownership_unknown', - { reason: 'previousExitUnverifiable', ownerVerdict: 'unverifiable' }, - "Orca could not prove this chat's previous agent process exited." - ) - } -} - -/** The same, for an operation the running child performs, which starts none: only an exit still - * unproven, its child still on record, holds it back. A proven exit owes only bookkeeping, which - * gates no such operation: its retry was reported, and the operation goes on as with none owed. */ -export async function finishOwedStructuredAgentSessionStopForProviderWrite( - context: StructuredAgentSessionLifetimeContext, - sessionId: string -): Promise { - const settled = await finishOwedStructuredAgentSessionStop(context, sessionId) - return settled.ok || context.sessions.get(sessionId)?.child ? settled : { ok: true } -} - -export function isStructuredAgentSessionPreviousExitUnverifiable( - refusal: AgentSessionWireRefusal -): boolean { - return ( - refusal.code === 'agent_session_ownership_unknown' && - refusal.details?.reason === 'previousExitUnverifiable' - ) -} - /** * Gives the session a provider child if it has none, for a caller inside its serialize — which is * what makes "if it has none" exact: two askers run this in turn, and the second finds the first @@ -104,9 +63,11 @@ export async function ensureStructuredAgentSessionAgent( sessionId: string, startedFor?: string ): Promise { - const settled = await finishOwedStructuredAgentSessionStop(context, sessionId) - if (!settled.ok) { - return settled + // A child a stop began closing takes no input, and none may start beside it: the start waits on + // that close, and is refused while its exit stays unverifiable. + const closed = await joinClosingStructuredAgentSessionChild(context, sessionId) + if (!closed.ok) { + return closed } if (context.sessions.get(sessionId)?.child) { return { ok: true } @@ -158,6 +119,9 @@ async function startStructuredAgentSessionAgent( return { ok: false, refusal: unreconciled } } await context.runtimeState.resolveRecovery(sessionId) + // A start needs the lease released; a release the exit's own wind-down could not write is + // re-derived from this host's proof of that exit, never refused on. + await releaseLeaseOfEndedStructuredAgentSessionChild(context, sessionId) const record = context.deps.store.getRecord(sessionId) if (!record) { return refuseResume( diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-context.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-context.ts index fe99ad90bc3..12c2f54a179 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-context.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-context.ts @@ -13,6 +13,7 @@ import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' import type { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' +import type { StructuredAgentSessionLifetimeContext } from './structured-agent-session-host-lifetime' import type { StructuredAgentSessionTaskQueue } from './structured-agent-session-task-queue' import type { StructuredAgentSessionConversationOpenOptions } from './structured-agent-session-conversation-open' @@ -33,6 +34,9 @@ export type StructuredAgentSessionAttachContext = { serialize: (sessionId: string, task: () => Promise) => Promise now: () => number publishStatus: (sessionId: string) => void + /** Joining a stop's close ends the child's record through the one exit handler. */ + endExitedChild: StructuredAgentSessionLifetimeContext['endExitedChild'] + wakeDelivery?: StructuredAgentSessionLifetimeContext['wakeDelivery'] /** The conversation's one open journal, opened when closed; see `conversation-open`. */ openConversation: ( sessionId: string, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts index b55ebbda2d7..834d4f569fa 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts @@ -85,6 +85,21 @@ export function mutateWithChatStop( ) ) const child = context.sessions.get(ctx.sessionId)?.child + if (child?.close) { + // A close an earlier stop began: this Stop joins it, retrying the exit's proof, rather + // than asking a child that takes no input to stop again. Its event, issued ahead of that + // retry, records only the withdrawal. + const effect = hadQueued ? tookEffect() : Promise.resolve() + await stopChild().catch((error: unknown) => + context.deps.logger.warn('ending the agent process on Stop failed', { + scope: 'stop-child', + sessionId, + error + }) + ) + await effect + return { ok: true, value: { ...named, cancelled: await withdrew } } + } if (child?.phase === 'starting') { // A start that may never land is the one thing here Stop has to end; the chat stays. // The event is issued first and lands behind the withdrawal, in the journal's queue order. diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-child-close.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-child-close.ts new file mode 100644 index 00000000000..0022df7c156 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-child-close.ts @@ -0,0 +1,137 @@ +// A provider child's close, which every stop, start and provider write that meets it joins. +// +// The close lives on the child (`child.close`) and ends with it, so nothing outlives the process +// it is about. Joining it is asking the adapter to close: the adapter runs one close per child at +// a time and bounds it by its own kill escalation, so every caller waits on the same attempt, and +// a caller that comes after an attempt ended unproven runs it again. The verdict is the root's +// exit alone. Proven, the one exit handler ends the record; a proof that lands with no caller +// waiting reaches that handler as the adapter's report of the exit. + +import { refuse } from '../../../shared/agent-session-wire-refusals' +import type { AgentSessionWireRefusal } from '../../../shared/agent-session-wire' +import type { StructuredAgentSessionLifetimeContext } from './structured-agent-session-host-lifetime' +import type { StructuredAgentSessionProviderChild } from './structured-agent-session-host-types' +import { stopAgentSessionProviderRoot } from './structured-agent-session-provider-exit-proof' +import { releaseStoredStructuredAgentSessionOwnerAfterExit } from './structured-agent-session-lease-release' +import { isSurfaceReleasableAgentSessionRecord } from '../../runtime/agent-session-surface-release-transition' + +/** What a caller learned about the child's exit: proven, or not. A root still there after the + * close's kill reads `unverifiable`, never `exited`. */ +export type StructuredAgentSessionChildCloseVerdict = 'exited' | 'unverifiable' + +/** Joins the child's close and, once its root's exit is proven, ends the record. */ +export async function joinStructuredAgentSessionChildClose( + context: StructuredAgentSessionLifetimeContext, + sessionId: string, + child: StructuredAgentSessionProviderChild +): Promise { + if (!(await closeProviderRoot(context, sessionId))) { + return 'unverifiable' + } + await context.endExitedChild(sessionId, child, { expected: true, reason: 'closed by Orca' }) + context.restartWitness?.stopped(sessionId) + return 'exited' +} + +/** The refusal of an operation that met a child whose close is still unproven. */ +export function previousExitUnverifiableRefusal(): AgentSessionWireRefusal { + return refuse( + 'agent_session_ownership_unknown', + { reason: 'previousExitUnverifiable', ownerVerdict: 'unverifiable' }, + "Orca could not prove this chat's previous agent process exited." + ) +} + +/** For an operation that reaches the provider: a child a stop began closing takes no input and + * none may start beside it, so the operation joins that close and is refused while it is still + * unproven. */ +export async function joinClosingStructuredAgentSessionChild( + context: StructuredAgentSessionLifetimeContext, + sessionId: string +): Promise<{ ok: true } | { ok: false; refusal: AgentSessionWireRefusal }> { + const child = context.sessions.get(sessionId)?.child + if (!child?.close) { + return { ok: true } + } + if ((await joinStructuredAgentSessionChildClose(context, sessionId, child)) === 'exited') { + return { ok: true } + } + // A stop reports this through its own failure; here the refusal is the only trace. + context.deps.logger.warn("the agent's process did not exit after Orca stopped and killed it", { + scope: 'provider-close-unproven', + sessionId + }) + return { ok: false, refusal: previousExitUnverifiableRefusal() } +} + +function closeProviderRoot( + context: StructuredAgentSessionLifetimeContext, + sessionId: string +): Promise { + const { adapter, logger } = context.deps + // An adapter with no close has nothing to stop; anything else must PROVE the exit. + const stop = adapter.disposeSession ?? adapter.closeSession + if (!stop) { + return Promise.resolve(true) + } + return stopAgentSessionProviderRoot( + () => stop.call(adapter, sessionId), + // The exit is proven; a child process left behind, or a write after it, is reported only. + (error) => + logger.warn("the agent's process exited, but its close did not finish cleanly", { + scope: 'provider-close-after-exit', + sessionId, + error + }) + ).catch((error: unknown) => { + logger.warn("closing the agent's process did not prove it exited", { + scope: 'provider-close', + sessionId, + error + }) + return false + }) +} + +/** Whether the lease still names the child this host last proved gone, unreleased: only that + * in-memory proof lets this host release it without a probe, so the handle carrying it stays. */ +export function structuredAgentSessionEndedChildHoldsLease( + context: Pick, + sessionId: string +): boolean { + const ended = context.sessions.get(sessionId)?.lastEndedChild + const record = context.deps.store.getRecord(sessionId) + return ( + ended?.rootGone === true && + record !== null && + isSurfaceReleasableAgentSessionRecord(record) && + record.lease.runtimeFence === ended.fence + ) +} + +/** Writes the release a proven exit allows when the exit handler could not: a start and the + * handle's close re-derive it, and a failure is reported, never anyone's refusal. Resolves + * whether the lease still names that child. */ +export async function releaseLeaseOfEndedStructuredAgentSessionChild( + context: Pick, + sessionId: string +): Promise { + const ended = context.sessions.get(sessionId)?.lastEndedChild + if (!ended || !structuredAgentSessionEndedChildHoldsLease(context, sessionId)) { + return false + } + await releaseStoredStructuredAgentSessionOwnerAfterExit({ + store: context.deps.store, + sessionId, + expectedFence: ended.fence, + now: context.now(), + ...(ended.reason ? { exitReason: ended.reason } : {}) + }).catch((error: unknown) => + context.deps.logger.warn("releasing an exited agent's lease failed", { + scope: 'ended-child-lease-release', + sessionId, + error + }) + ) + return structuredAgentSessionEndedChildHoldsLease(context, sessionId) +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts new file mode 100644 index 00000000000..c606a73e310 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-child-exit.ts @@ -0,0 +1,219 @@ +// The one handler that ends a provider child's record, whether its exit was expected (a close this +// host asked for) or not (the child died on its own). The exit was seen or proven first-hand, so +// every step after it is bookkeeping: each is attempted and reported, none keeps the child on +// record, and the record ends in `finally`. `expected` changes only what the chat is told. + +import type { SubmissionRejectionFact } from '../../../shared/agent-session-failure' +import { PROVIDER_EXIT_ROW_PREFIX } from '../../../shared/agent-session-stop-row-identity' +import { structuredAgentSessionFailureWordsContext } from './structured-agent-session-send-preparation' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import type { StructuredAgentSessionEndedEvent } from './structured-agent-session-adapter' +import type { + StructuredAgentSessionHostSession, + StructuredAgentSessionProviderChild +} from './structured-agent-session-host-types' +import { endProviderChild } from './structured-agent-session-provider-child' +import { + releaseStoredStructuredAgentSessionOwnerAfterExit, + type StructuredAgentSessionLeaseStore +} from './structured-agent-session-lease-release' +import type { StructuredAgentSessionSinkBarrier } from './structured-agent-session-event-sink' +import { + captureUnfinishedStructuredAgentSessionWork, + settleStructuredAgentSessionDeadGeneration, + type DeadGenerationJournal, + unfinishedStructuredAgentSessionWorkWasInterrupted +} from './structured-agent-session-dead-generation-settlement' +import type { StructuredAgentSessionLogger } from './structured-agent-session-logger' +import { evictStructuredAgentSession } from './structured-agent-session-eviction' +import type { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' + +/** How the child's root went: the close this host asked for, or a death of its own. */ +export type StructuredAgentSessionChildExit = { + expected: boolean + /** Log text only; the chat's words come from `failure`. */ + reason: string + failure?: SubmissionRejectionFact + /** Host receipt of the exit: the end time of a turn it interrupted. */ + observedAt?: number + startupUnproven?: true +} + +export type StructuredAgentSessionChildExitSession = Pick< + StructuredAgentSessionHostSession, + 'child' | 'lastEndedChild' +> & { journal: DeadGenerationJournal & Pick } + +export type StructuredAgentSessionChildExitContext< + TSession extends StructuredAgentSessionChildExitSession = StructuredAgentSessionHostSession +> = { + store: StructuredAgentSessionLeaseStore + sessions: Map + flushLifecycle: (sessionId: string) => Promise + publishFence: (sessionId: string, session: TSession) => void + publishStatus?: (sessionId: string) => void + /** The delivery loop hands over whatever is queued once the child is off the record. */ + wakeDelivery?: (sessionId: string) => void + serialize: (sessionId: string, task: () => Promise) => Promise + now: () => number + logger: StructuredAgentSessionLogger + /** Lets the child's sink and the adapter's route for it go; absent leaves both to the next attach. */ + route?: { + runtimeState: Pick + acknowledgeRelease: (sessionId: string) => Promise | void + } +} + +/** An adapter's report of a child's exit, for the child still on record with its identity. */ +export function settleStructuredAgentSessionChildExit< + TSession extends StructuredAgentSessionChildExitSession +>( + context: StructuredAgentSessionChildExitContext, + event: StructuredAgentSessionEndedEvent +): Promise { + return context.serialize(event.sessionId, async () => { + const child = context.sessions.get(event.sessionId)?.child + if (!child || child.fence !== event.fence || child.generation !== event.acquisitionGeneration) { + return + } + await endExitedStructuredAgentSessionChildUnderSerialize(context, event.sessionId, child, { + expected: event.cause === 'requested-close', + reason: event.reason, + ...(event.failure ? { failure: event.failure } : {}), + ...(event.observedAt === undefined ? {} : { observedAt: event.observedAt }), + ...(event.startupUnproven ? { startupUnproven: event.startupUnproven } : {}) + }) + }) +} + +/** Ends the record of a child whose exit this host saw or proved; a no-op once it has left. */ +export async function endExitedStructuredAgentSessionChildUnderSerialize< + TSession extends StructuredAgentSessionChildExitSession +>( + context: StructuredAgentSessionChildExitContext, + sessionId: string, + child: StructuredAgentSessionProviderChild, + exit: StructuredAgentSessionChildExit +): Promise { + const session = context.sessions.get(sessionId) + if (!session || session.child !== child) { + return + } + const { expected } = exit + const close = child.close + // Receipt of the exit is the one end time the host may record for a running turn. + const observedAt = exit.observedAt ?? context.now() + // The host's own phase decides, so a provider that omits the flag still gets a start that + // failed told as one: the row says so. + const exitedDuringStartup = exit.startupUnproven === true || child.phase === 'starting' + const endChild = (): void => { + endProviderChild(session, { + generation: child.generation, + fence: child.fence, + // A close keeps the cause of the stop that asked for it, and ends where it was asked. + cause: expected ? (close?.cause ?? 'evict') : 'exit', + reason: expected ? (close?.reason ?? null) : exit.reason, + ...(!expected && exit.failure ? { failure: exit.failure } : {}), + duringStartup: exitedDuringStartup, + // The adapter publishes an exit only once it saw the root go, first-hand or proven. + rootGone: true, + ...(expected && close ? { endedAt: close.requestedAt } : {}) + }) + context.publishStatus?.(sessionId) + } + const record = context.store.getRecord(sessionId) + if (!record || record.lease.handoffStage !== null) { + // An acquisition or recovery already owns this lease's transition. + endChild() + context.wakeDelivery?.(sessionId) + return + } + try { + // The exited child's own writes land first: its dead generation is settled from all of them. + try { + const barrier = await context.flushLifecycle(sessionId) + if (!barrier.ok) { + logExitFailure(context, sessionId, 'exit-lifecycle-barrier', barrier.error) + } + } catch (error) { + logExitFailure(context, sessionId, 'exit-lifecycle-barrier', error) + } + const unfinishedWork = captureUnfinishedStructuredAgentSessionWork(session.journal) + // Folded before the fallback's end is built, so the end reads it (`turnEndAfterStop`). + await close?.recorded + const generation = child.generation ?? 'unknown' + const settled = await settleStructuredAgentSessionDeadGeneration({ + journal: session.journal, + sessionId, + fence: child.fence, + settlementId: `${expected ? 'expected-close:' : PROVIDER_EXIT_ROW_PREFIX}${sessionId}:${child.fence}:${generation}`, + pendingSubmissionReason: expected + ? 'provider_closed_before_acknowledgement' + : 'provider_exited_before_acknowledgement', + // Only a turn no adapter settled. Whether a close ended a person's turn is the Stop event's to + // say (`turnEndAfterStop`); a death of its own interrupted it. + verdict: { state: 'interrupted', completedAt: expected ? context.now() : observedAt }, + failureTextContext: structuredAgentSessionFailureWordsContext(record, session.journal), + // A failed start always says why: no response was running to carry the reason. + showUnexpectedExitOutcome: + !expected && + (exitedDuringStartup || + unfinishedStructuredAgentSessionWorkWasInterrupted( + unfinishedWork, + session.journal, + observedAt + )), + ...(!expected && exit.failure ? { exitFailure: exit.failure } : {}), + ...(!expected && exitedDuringStartup && child.generation + ? { exitedDuringStartup: { generation: child.generation } } + : {}) + }) + if (!settled.ok) { + logExitFailure(context, sessionId, 'exit-settlement', settled.error) + } + } finally { + // The root's exit was observed, so the owner is released even when terminal settlement could + // not be durably accepted. Bare cause: whatever this settlement could not write is settled from + // it later (the settle recording it queues, or the next open or acquire). + let released = false + try { + await releaseStoredStructuredAgentSessionOwnerAfterExit({ + store: context.store, + sessionId, + expectedFence: child.fence, + now: context.now(), + exitObservedAt: observedAt, + exitReason: exit.reason + }) + released = true + } catch (error) { + logExitFailure(context, sessionId, 'exit-owner-release', error) + } + if (context.route) { + const { runtimeState, acknowledgeRelease } = context.route + await evictStructuredAgentSession({ + sessionId, + eventSink: runtimeState.eventSinkFor(sessionId), + logger: context.logger, + discardSink: () => runtimeState.discardEventSink(sessionId), + acknowledgeRelease: () => acknowledgeRelease(sessionId) + }) + } + endChild() + // A reader re-baselines on a death of the child's own; a close Orca asked for moves no fence a + // reader holds, as a client resends a message when its fence moves. + if (released && !expected) { + context.publishFence(sessionId, session) + } + context.wakeDelivery?.(sessionId) + } +} + +function logExitFailure( + context: Pick, + sessionId: string, + scope: string, + error: unknown +): void { + context.logger.warn('settling a provider exit did not finish', { scope, sessionId, error }) +} 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 f09fe672b5e..bf473bfc6df 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 @@ -15,6 +15,7 @@ import { } from '../agent-session-journal/journal-host-database-test-support' import type { AgentSessionAttachParams } from './structured-agent-session-attach' import { stopStructuredAgentSessionAgentUnderSerialize } from './structured-agent-session-host-lifetime' +import { endExitedStructuredAgentSessionChildUnderSerialize } from './structured-agent-session-child-exit' import { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' @@ -135,7 +136,23 @@ describe('Claude root-exit stop', () => { runtimeState, sessions, now: () => NOW + 30 * 60_000, - publishStatus + publishStatus, + endExitedChild: (sessionId, child, exit) => + endExitedStructuredAgentSessionChildUnderSerialize( + { + store, + sessions, + flushLifecycle: (id) => runtimeState.lifecycleBarrier(id), + publishFence: () => undefined, + publishStatus, + serialize: (_sessionId, task) => task(), + now: () => NOW + 30 * 60_000, + logger: deps.logger + }, + sessionId, + child, + exit + ) }, 'session-1', { cause: 'evict' } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.ts new file mode 100644 index 00000000000..cfc2c27ca6b --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-stop-exit-ends-record.test.ts @@ -0,0 +1,617 @@ +// A Claude child's close and its exit, on the shipping adapter and host. A Stop begins the child's +// close; every later stop, start and provider write joins it; the exit, whenever it is proven, ends +// the child's record; and a close still unverifiable fails what needed the child, holding nothing. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' +import { activeStructuredAgentSessionTurnId } from '../../../shared/structured-agent-session-live-turn' +import { claudeUnwrittenUserMessageError } from '../../claude/claude-agent-sdk-user-message-queue' +import { ClaudeStructuredSessionAdapter } from '../../claude/claude-structured-session-adapter' +import { + fakeClaude, + PROVIDER_SESSION_ID, + type FakeConnection +} from '../../claude/claude-structured-session-test-support' +import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' +import { structuredClaudeLifecycleEvent } from '../../runtime/structured-claude-runtime-adapter' +import { openTestJournalHostDatabase } from '../agent-session-journal/journal-host-database-test-support' +import { StructuredAgentSessionHost } from './structured-agent-session-host' +import { recordingStructuredAgentSessionLogger } from './structured-agent-session-logger-test-support' +import { + HOST_TEST_NOW as NOW, + HOST_TEST_SESSION as SESSION, + hostTestAttachParams, + hostTestMessage, + hostTestOperationId, + resetHostTestOperationIds +} from './structured-agent-session-host-test-data' + +const CALLER = { callerKey: 'client-1' } +const CAPABILITIES = ['interrupt_receipt_v1', 'interrupt_cancel_queued_v1', 'msg_lifecycle_v1'] + +let root: string +let host: StructuredAgentSessionHost +let adapter: ClaudeStructuredSessionAdapter +let store: AgentSessionRecordStore +let claude: ReturnType +let log: ReturnType +let persistHandle: ReturnType Promise>> +let childWork: string[] + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-claude-stop-exit-ends-record-')) + resetHostTestOperationIds() + log = recordingStructuredAgentSessionLogger() + persistHandle = vi.fn(async () => undefined) + childWork = [] + claude = fakeClaude({ + replayUuid: null, + routes: { interrupt: () => ({ still_queued: [], cancelled: [] }) } + }) + const lifecycle: Promise[] = [] + adapter = new ClaudeStructuredSessionAdapter({ + resolveLaunch: async () => ({ + pathToClaudeCodeExecutable: 'claude', + options: {}, + cwd: root, + claudeConfigDir: join(root, 'claude-home'), + providerSessionId: PROVIDER_SESSION_ID, + resumeLeafUuid: null, + resumesTranscript: (store.getRecord(SESSION)?.providerHandleChain.length ?? 0) > 0, + continuesChain: (store.getRecord(SESSION)?.providerHandleChain.length ?? 0) > 0 + }), + onEvent: (event) => { + const mapped = structuredClaudeLifecycleEvent(event) + if (mapped) { + lifecycle.push(host.handleAdapterEvent(mapped)) + } + }, + onDispatchSettledLate: (settlement) => void host.settleLateDispatch(settlement), + persistHandle: () => persistHandle(), + logger: log.logger, + onChildWorkEvidence: (sessionId, evidence) => { + childWork.push(...evidence.map((edge) => edge.type)) + host.publishChildWorkEvidence(sessionId, evidence) + }, + openConnection: claude.openConnection, + readProcessStartTime: async () => 1_700_000_000_000, + now: () => NOW + }) + store = await openTestAgentSessionRecordStore(root) + host = new StructuredAgentSessionHost({ + store, + adapter: Object.assign(adapter, { supportsCreate: () => true }), + journalDatabase: openTestJournalHostDatabase(root), + claimKeyId: 'key-1', + mintSpawnToken: () => 'spawn-a', + logger: log.logger, + // Ticked by hand only, and with no idle window: a tick reaps whatever is at rest. + idleSweep: { intervalMs: 3_600_000, idleMs: 0 }, + now: () => NOW + }) + const params = hostTestAttachParams(null, { + provider: 'claude', + agent: 'claude', + accountHome: { variable: 'CLAUDE_CONFIG_DIR', path: join(root, 'claude-home') }, + providerHandle: { kind: 'claude', sessionId: PROVIDER_SESSION_ID, leafUuid: null } + }) + expect(await host.attach(CALLER, params)).toMatchObject({ ok: true }) + await adapter.awaitStarted(SESSION) + await Promise.all(lifecycle) +}) + +afterEach(async () => { + await adapter.closeAll() + await host.flushAllStreamedEvents() + await rm(root, { recursive: true, force: true }) +}) + +function eventually(assertion: () => T | Promise): Promise { + return vi.waitFor(assertion, { timeout: 10_000 }) +} + +function envelope( + method: + | 'agentSession.send' + | 'agentSession.cancel' + | 'agentSession.setOption' + | 'agentSession.queuedMessageSend', + // The fingerprint's own field shape, as the sibling host tests type it. + fields: Parameters[0]['fields'] +) { + return { + sessionId: SESSION, + clientOperationId: hostTestOperationId(), + expectedRuntimeFence: store.getRecord(SESSION)!.lease.runtimeFence, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method, + sessionId: SESSION, + fields + }) + } +} + +async function send(text: string): Promise { + const body = hostTestMessage(text) + const sent = await host.send(CALLER, { envelope: envelope('agentSession.send', { body }), body }) + if (!sent.ok) { + throw new Error(`send refused: ${JSON.stringify(sent.refusal)}`) + } + return sent.value.clientMessageId +} + +async function submission(clientMessageId: string) { + await host.flushStreamedEvents(SESSION) + return (await host.journalSnapshot(SESSION)).submissions.find( + (entry) => entry.clientMessageId === clientMessageId + ) +} + +function frame(connection: FakeConnection, message: Record): void { + connection.handlers.onMessage?.({ session_id: PROVIDER_SESSION_ID, ...message }) +} + +function wrote(connection: FakeConnection, text: string): boolean { + return connection.sent.some((message) => JSON.stringify(message).includes(text)) +} + +async function openTurn(connection: FakeConnection): Promise { + const text = 'Write a long reply.' + const clientMessageId = await send(text) + await eventually(() => expect(wrote(connection, text)).toBe(true)) + frame(connection, { + type: 'system', + subtype: 'init', + uuid: 'init-1', + model: 'claude-sonnet-5', + capabilities: CAPABILITIES + }) + const written = connection.sent.at(-1)! + frame(connection, { ...written, uuid: written.uuid }) + frame(connection, { + type: 'assistant', + uuid: 'stopped-turn-leaf', + parent_tool_use_id: null, + message: { id: 'msg-1', role: 'assistant', content: [{ type: 'text', text: 'Working on' }] } + }) + await eventually(async () => + expect((await submission(clientMessageId))?.dispatchState).toBe('accepted') + ) + expect( + activeStructuredAgentSessionTurnId((await host.journalSnapshot(SESSION)).items) + ).not.toBeNull() +} + +function stop() { + return host.cancel(CALLER, { envelope: envelope('agentSession.cancel', {}) }) +} + +function laneDrained(): Promise { + return host['tasks'].serialize(SESSION, async () => {}) +} + +/** As the real connection: once a close begins it refuses every write, proven or not. */ +function closeUnprovenFor(connection: FakeConnection, failures: number): void { + const close = connection.close + let left = failures + connection.close = async () => { + if (left === 0) { + return close() + } + left -= 1 + connection.closeCount += 1 + connection.closed = true + return false + } + refuseWritesOnceClosed(connection) +} + +function refuseWritesOnceClosed(connection: FakeConnection): void { + const write = connection.send + connection.send = (message, beforeDispatch) => + connection.closed + ? Promise.reject( + claudeUnwrittenUserMessageError(new Error('claude stream-json connection is closed')) + ) + : write(message, beforeDispatch) +} + +const INTERRUPTED_RESULT = { + type: 'result', + subtype: 'error_during_execution', + is_error: true, + terminal_reason: 'aborted_streaming', + uuid: 'interrupted-result' +} + +function child() { + return host['sessions'].get(SESSION)?.child ?? null +} + +function lease() { + return store.getRecord(SESSION)?.lease +} + +/** Stops a running turn; its child's close cannot prove the exit `failures` times. */ +async function stopWithUnprovenClose(failures: number): Promise { + const connection = claude.connections[0]! + await openTurn(connection) + closeUnprovenFor(connection, failures) + await expect(stop()).resolves.toMatchObject({ ok: true, value: { cancelled: true } }) + frame(connection, INTERRUPTED_RESULT) + await laneDrained() + expect(connection.closeCount).toBe(1) + // The child stays on record with its close begun, never as a stored obligation beside it. + expect(child()?.close).toMatchObject({ cause: 'user-stop' }) + return connection +} + +async function resumedWith(connection: FakeConnection, text: string): Promise { + return eventually(() => { + const started = claude.connections.at(-1)! + expect(started).not.toBe(connection) + expect(wrote(started, text)).toBe(true) + return started + }) +} + +function setModel(model: string) { + const fields = { key: 'model', value: model } + return host.setOption(CALLER, { envelope: envelope('agentSession.setOption', fields), ...fields }) +} + +function deferred() { + let resolve: (value: T) => void = () => undefined + const promise = new Promise((settle) => { + resolve = settle + }) + return { promise, resolve } +} + +function scopes(): unknown[] { + return log.entries.map((entry) => entry.fields.scope) +} + +it('joins the close a Stop could not prove before the next message, then sends it to a resumed child', async () => { + const connection = await stopWithUnprovenClose(1) + + await send('Carry on.') + await resumedWith(connection, 'Carry on.') + + expect(connection.closeCount).toBe(2) + expect(wrote(connection, 'Carry on.')).toBe(false) + expect(host['sessions'].get(SESSION)?.lastEndedChild).toMatchObject({ + cause: 'user-stop', + rootGone: true + }) +}) + +it('has a second Stop join the close the first could not prove, retrying its kill', async () => { + const connection = await stopWithUnprovenClose(1) + + // Nothing was queued, so it withdrew nothing and records no event of its own. + await expect(stop()).resolves.toMatchObject({ ok: true, value: { cancelled: false } }) + expect(connection.closeCount).toBe(2) + expect(child()).toBeNull() + expect(host['sessions'].get(SESSION)?.lastEndedChild).toMatchObject({ + cause: 'user-stop', + rootGone: true + }) + expect(connection.calls.filter((call) => call.subtype === 'interrupt')).toHaveLength(1) +}) + +it('has a send made while the close still runs wait for that close, not start another', async () => { + const connection = claude.connections[0]! + await openTurn(connection) + const proof = deferred() + const close = connection.close + let attempts = 0 + connection.close = async () => { + attempts += 1 + connection.closed = true + await proof.promise + return close() + } + refuseWritesOnceClosed(connection) + await expect(stop()).resolves.toMatchObject({ ok: true, value: { cancelled: true } }) + frame(connection, INTERRUPTED_RESULT) + + // Accepted on the chat's queue behind the Stop's close, which nothing abandons meanwhile. + const sent = send('Carry on.') + await eventually(() => expect(attempts).toBe(1)) + expect(claude.connections).toHaveLength(1) + proof.resolve() + await sent + + await resumedWith(connection, 'Carry on.') + // One close of the old child, which the send waited on rather than starting its own. + expect(attempts).toBe(1) + expect(connection.closeCount).toBe(1) +}) + +// The verdict is the root's exit; the resume point written after it is bookkeeping. +it('ends the record at the proven exit though the resume-point write after it never settles', async () => { + const connection = claude.connections[0]! + await openTurn(connection) + refuseWritesOnceClosed(connection) + persistHandle.mockImplementationOnce(() => new Promise(() => {})) + await expect(stop()).resolves.toMatchObject({ ok: true, value: { cancelled: true } }) + frame(connection, INTERRUPTED_RESULT) + + await eventually(() => expect(child()).toBeNull()) + expect(host['sessions'].get(SESSION)?.lastEndedChild).toMatchObject({ cause: 'user-stop' }) + await send('Carry on.') + await resumedWith(connection, 'Carry on.') +}) + +it('ends the record when the root exits after its close gave up, with no one asking again', async () => { + const connection = claude.connections[0]! + // A background task the old agent was running when the Stop's close came back unproven. + frame(connection, { + type: 'system', + subtype: 'task_started', + uuid: 'task-start', + task_id: 'background-1', + task_type: 'local_agent', + is_backgrounded: true + }) + await stopWithUnprovenClose(1) + childWork.length = 0 + + // The connection reports the root's exit as the end of the close Orca began. + connection.handlers.onExit?.(new Error('claude exited'), { expected: true }) + + await eventually(() => expect(child()).toBeNull()) + await eventually(() => expect(lease()?.claimStatus).toBe('released')) + expect(host['sessions'].get(SESSION)?.lastEndedChild).toMatchObject({ cause: 'user-stop' }) + expect(connection.closeCount).toBe(2) + expect(claude.connections).toHaveLength(1) + // The child work ends with the session, so nothing is left shown running for a dead agent. + await eventually(() => expect(childWork).toContain('session-ended')) +}) + +it('sends after a proven exit whose resume-point write and lease release both failed', async () => { + const connection = claude.connections[0]! + await openTurn(connection) + refuseWritesOnceClosed(connection) + persistHandle.mockRejectedValueOnce(new Error('resume point not written')) + const transition = store.transitionHandoff.bind(store) + vi.spyOn(store, 'transitionHandoff') + .mockImplementationOnce(async () => { + throw new Error('store unavailable') + }) + .mockImplementation(transition) + await expect(stop()).resolves.toMatchObject({ ok: true, value: { cancelled: true } }) + frame(connection, INTERRUPTED_RESULT) + await eventually(() => expect(child()).toBeNull()) + // Both reported; neither kept the dead child on record. + await eventually(() => + expect(scopes()).toEqual( + expect.arrayContaining(['claude-close-resume-point', 'exit-owner-release']) + ) + ) + expect(lease()?.claimStatus).toBe('live') + + // The start writes the release from this host's proof of that exit. + await send('Carry on.') + await resumedWith(connection, 'Carry on.') +}) + +// A crash's reason can carry kilobytes of stderr; the release a start re-derives must still land. +it('sends after a crash with a long reason whose own lease release failed', async () => { + const connection = claude.connections[0]! + const transition = store.transitionHandoff.bind(store) + vi.spyOn(store, 'transitionHandoff') + .mockImplementationOnce(async () => { + throw new Error('store unavailable') + }) + .mockImplementation(transition) + + connection.handlers.onExit?.( + new Error(`claude stream-json exited (code 1): ${'stack frame\n'.repeat(700)}`) + ) + await eventually(() => expect(child()).toBeNull()) + await eventually(() => expect(scopes()).toContain('exit-owner-release')) + expect(lease()?.claimStatus).toBe('live') + + await send('Carry on.') + await resumedWith(connection, 'Carry on.') + expect(scopes()).not.toContain('ended-child-lease-release') +}) + +it('rejects a message whose start meets a close still unverifiable, and starts nothing beside it', async () => { + const connection = await stopWithUnprovenClose(2) + + const next = await send('Carry on.') + + await eventually(async () => expect((await submission(next))?.dispatchState).toBe('rejected')) + const rejected = (await submission(next))! + expect(rejected.reason).toBe( + "Couldn't stop Claude from before. Send your message again to try once more." + ) + expect(rejected.rejection).toMatchObject({ + kind: 'restartFailed', + refusal: { details: { reason: 'previousExitUnverifiable' } } + }) + // Rejected, never held: no waiting note, and the old child still on record. + const notes = (await host.journalSnapshot(SESSION)).items.filter( + (item) => item.body.kind === 'status' && item.body.failure?.kind === 'previousExitUnverifiable' + ) + expect(notes).toEqual([]) + expect(connection.closeCount).toBe(2) + expect(claude.connections).toHaveLength(1) + expect(wrote(connection, 'Carry on.')).toBe(false) + expect(child()?.close).toBeDefined() + expect(scopes()).toContain('provider-close-unproven') +}) + +it('refuses an option change while the close stays unverifiable, never writing to the old child', async () => { + const connection = await stopWithUnprovenClose(2) + + await expect(setModel('claude-opus-5')).resolves.toMatchObject({ + ok: false, + refusal: { + code: 'agent_session_ownership_unknown', + details: { reason: 'previousExitUnverifiable', ownerVerdict: 'unverifiable' } + } + }) + expect(connection.closeCount).toBe(2) + expect(connection.calls.some((call) => call.subtype === 'set_model')).toBe(false) + + // Joined again once the exit is proven, the pick is the chat's at rest, for the next start. + await expect(setModel('claude-opus-5')).resolves.toMatchObject({ ok: true }) + expect(child()).toBeNull() + expect(store.getRecord(SESSION)?.options).toMatchObject({ model: 'claude-opus-5' }) +}) + +it('keeps an option change at rest when the Stop proved the exit and only its drain failed', async () => { + const connection = claude.connections[0]! + await openTurn(connection) + // The Stop reads the journal without a drain, so the exit's wind-down is the only drain to fail. + vi.spyOn(host['runtimeState'].eventSinkFor(SESSION), 'lifecycleBarrier').mockResolvedValueOnce({ + ok: false, + error: new Error('drain barrier lost') + }) + await expect(stop()).resolves.toMatchObject({ ok: true, value: { cancelled: true } }) + frame(connection, INTERRUPTED_RESULT) + await laneDrained() + await eventually(() => expect(child()).toBeNull()) + expect(scopes().filter((scope) => scope === 'exit-lifecycle-barrier')).toHaveLength(1) + + await expect(setModel('claude-opus-5')).resolves.toMatchObject({ ok: true }) + expect(store.getRecord(SESSION)?.options).toMatchObject({ model: 'claude-opus-5' }) + expect(connection.closeCount).toBe(1) + expect(claude.connections).toHaveLength(1) +}) + +it('reports a descendant left behind by a proven root exit, and blocks nothing', async () => { + const connection = claude.connections[0]! + await openTurn(connection) + refuseWritesOnceClosed(connection) + connection.close = async () => { + connection.closeCount += 1 + connection.closed = true + connection.exitVerdict = { root: 'exited', tree: 'live' } + return false + } + await expect(stop()).resolves.toMatchObject({ ok: true, value: { cancelled: true } }) + frame(connection, INTERRUPTED_RESULT) + + await eventually(() => expect(child()).toBeNull()) + expect(scopes()).toContain('provider-close-after-exit') + await send('Carry on.') + await resumedWith(connection, 'Carry on.') +}) + +it('has quit join a close still unverifiable, and hand the lease back once it proves', async () => { + const connection = await stopWithUnprovenClose(1) + + await host.flushAllStreamedEvents() + + expect(connection.closeCount).toBe(2) + expect(lease()).toMatchObject({ claimStatus: 'released', ownerProcess: null }) +}) + +it('has the idle reaper join a close still unverifiable, ending the record with the Stop it finishes', async () => { + const connection = await stopWithUnprovenClose(1) + + await host['lifetime'].idleSweep.tick() + + expect(connection.closeCount).toBe(2) + expect(lease()).toMatchObject({ claimStatus: 'released', ownerProcess: null }) + expect(host.hasSession(SESSION)).toBe(false) +}) + +it("ends a Claude journal-sink failure as Orca's own fault, never a quiet rest", async () => { + const connection = claude.connections[0]! + await openTurn(connection) + + host['eventRecovery'].recoverAfterSinkFailure(SESSION, new Error('journal write failed')) + + await eventually(() => expect(child()).toBeNull()) + expect(host['sessions'].get(SESSION)?.lastEndedChild).toMatchObject({ + cause: 'exit', + failure: { kind: 'hostFault' } + }) + expect(connection.closeCount).toBe(1) +}) + +it('delivers a message accepted after a tab close was asked for, when the late exit lands first', async () => { + const connection = claude.connections[0]! + closeUnprovenFor(connection, 1) + await expect(host.close(SESSION, 'user-close')).rejects.toThrow() + const closing = child()! + expect(closing.close).toMatchObject({ cause: 'user-close' }) + + // The message is accepted, then the exit report ends the child, before the delivery step runs. + const held = deferred() + const holding = host['tasks'].serialize(SESSION, () => held.promise) + const sent = send('Carry on.') + const ended = host['tasks'].serialize(SESSION, () => + host['eventRecovery'].endExitedChildUnderSerialize(SESSION, closing, { + expected: true, + reason: 'claude session closed' + }) + ) + held.resolve() + await holding + const next = await sent + await ended + + // The close was asked for before the message, so its end closes nothing the message carries. + await resumedWith(connection, 'Carry on.') + expect(await submission(next)).not.toMatchObject({ dispatchState: 'rejected' }) +}) + +it('fails only the message its own refusal is about, never one accepted while that start was refused', async () => { + const connection = claude.connections[0]! + await openTurn(connection) + // The Stop's close and the first send's join both come back unproven; the second's proves. + const close = connection.close + const joined = deferred() + let attempts = 0 + connection.close = async () => { + attempts += 1 + connection.closeCount += 1 + connection.closed = true + if (attempts === 1) { + return false + } + return attempts === 2 ? joined.promise : close() + } + refuseWritesOnceClosed(connection) + await expect(stop()).resolves.toMatchObject({ ok: true, value: { cancelled: true } }) + frame(connection, INTERRUPTED_RESULT) + await laneDrained() + + const first = await send('First.') + await eventually(() => expect(attempts).toBe(2)) + // Accepted while the first send's join still runs, then that join comes back unproven. + const second = send('Second.') + joined.resolve(false) + + const secondId = await second + await eventually(async () => expect((await submission(first))?.dispatchState).toBe('rejected')) + await resumedWith(connection, 'Second.') + expect(await submission(secondId)).not.toMatchObject({ dispatchState: 'rejected' }) +}) + +it('runs the stop again for every ask after a close that came back unproven', async () => { + const connection = await stopWithUnprovenClose(3) + + await send('First.') + await eventually(() => expect(connection.closeCount).toBe(2)) + await laneDrained() + await send('Second.') + await eventually(() => expect(connection.closeCount).toBe(3)) + await laneDrained() + + // Each ask asked again; the third proves the exit. + await send('Third.') + await resumedWith(connection, 'Third.') + expect(connection.closeCount).toBe(4) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-unproven-stop-send.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-unproven-stop-send.test.ts deleted file mode 100644 index 65dcfa42073..00000000000 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-unproven-stop-send.test.ts +++ /dev/null @@ -1,569 +0,0 @@ -// A Claude Stop whose close could not prove the child gone, then the user's next message, on the -// shipping adapter and host. That child takes no input, so the send retries the stop first; still -// unproven, the message waits with its reason until a later retry proves the exit. - -import { mkdtemp, rm } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, beforeEach, expect, it, vi } from 'vitest' -import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' -import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../shared/agent-session-journal-types' -import { activeStructuredAgentSessionTurnId } from '../../../shared/structured-agent-session-live-turn' -import { claudeUnwrittenUserMessageError } from '../../claude/claude-agent-sdk-user-message-queue' -import { ClaudeStructuredSessionAdapter } from '../../claude/claude-structured-session-adapter' -import { - fakeClaude, - PROVIDER_SESSION_ID, - type FakeConnection -} from '../../claude/claude-structured-session-test-support' -import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' -import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' -import { structuredClaudeLifecycleEvent } from '../../runtime/structured-claude-runtime-adapter' -import { openTestJournalHostDatabase } from '../agent-session-journal/journal-host-database-test-support' -import { StructuredAgentSessionHost } from './structured-agent-session-host' -import { recordingStructuredAgentSessionLogger } from './structured-agent-session-logger-test-support' -import { - HOST_TEST_NOW as NOW, - HOST_TEST_SESSION as SESSION, - hostTestAttachParams, - hostTestMessage, - hostTestOperationId, - resetHostTestOperationIds -} from './structured-agent-session-host-test-data' - -const CALLER = { callerKey: 'client-1' } -const CAPABILITIES = ['interrupt_receipt_v1', 'interrupt_cancel_queued_v1', 'msg_lifecycle_v1'] - -let root: string -let host: StructuredAgentSessionHost -let adapter: ClaudeStructuredSessionAdapter -let store: AgentSessionRecordStore -let claude: ReturnType -let log: ReturnType - -beforeEach(async () => { - root = await mkdtemp(join(tmpdir(), 'orca-claude-unproven-stop-send-')) - resetHostTestOperationIds() - log = recordingStructuredAgentSessionLogger() - claude = fakeClaude({ - replayUuid: null, - routes: { interrupt: () => ({ still_queued: [], cancelled: [] }) } - }) - const lifecycle: Promise[] = [] - adapter = new ClaudeStructuredSessionAdapter({ - resolveLaunch: async () => ({ - pathToClaudeCodeExecutable: 'claude', - options: {}, - cwd: root, - claudeConfigDir: join(root, 'claude-home'), - providerSessionId: PROVIDER_SESSION_ID, - resumeLeafUuid: null, - resumesTranscript: (store.getRecord(SESSION)?.providerHandleChain.length ?? 0) > 0, - continuesChain: (store.getRecord(SESSION)?.providerHandleChain.length ?? 0) > 0 - }), - onEvent: (event) => { - const mapped = structuredClaudeLifecycleEvent(event) - if (mapped) { - lifecycle.push(host.handleAdapterEvent(mapped)) - } - }, - onDispatchSettledLate: (settlement) => void host.settleLateDispatch(settlement), - openConnection: claude.openConnection, - readProcessStartTime: async () => 1_700_000_000_000, - now: () => NOW - }) - store = await openTestAgentSessionRecordStore(root) - host = new StructuredAgentSessionHost({ - store, - adapter: Object.assign(adapter, { supportsCreate: () => true }), - journalDatabase: openTestJournalHostDatabase(root), - claimKeyId: 'key-1', - mintSpawnToken: () => 'spawn-a', - logger: log.logger, - now: () => NOW - }) - const params = hostTestAttachParams(null, { - provider: 'claude', - agent: 'claude', - accountHome: { variable: 'CLAUDE_CONFIG_DIR', path: join(root, 'claude-home') }, - providerHandle: { kind: 'claude', sessionId: PROVIDER_SESSION_ID, leafUuid: null } - }) - expect(await host.attach(CALLER, params)).toMatchObject({ ok: true }) - await adapter.awaitStarted(SESSION) - await Promise.all(lifecycle) -}) - -afterEach(async () => { - await adapter.closeAll() - await host.flushAllStreamedEvents() - await rm(root, { recursive: true, force: true }) -}) - -function eventually(assertion: () => T | Promise): Promise { - return vi.waitFor(assertion, { timeout: 10_000 }) -} - -function envelope( - method: - | 'agentSession.send' - | 'agentSession.cancel' - | 'agentSession.setOption' - | 'agentSession.queuedMessageSend', - // The fingerprint's own field shape, as the sibling host tests type it. - fields: Parameters[0]['fields'] -) { - return { - sessionId: SESSION, - clientOperationId: hostTestOperationId(), - expectedRuntimeFence: store.getRecord(SESSION)!.lease.runtimeFence, - payloadFingerprint: computeAgentSessionPayloadFingerprint({ - method, - sessionId: SESSION, - fields - }) - } -} - -async function send(text: string): Promise { - const body = hostTestMessage(text) - const sent = await host.send(CALLER, { envelope: envelope('agentSession.send', { body }), body }) - if (!sent.ok) { - throw new Error(`send refused: ${JSON.stringify(sent.refusal)}`) - } - return sent.value.clientMessageId -} - -async function submission(clientMessageId: string) { - await host.flushStreamedEvents(SESSION) - return (await host.journalSnapshot(SESSION)).submissions.find( - (entry) => entry.clientMessageId === clientMessageId - ) -} - -function frame(connection: FakeConnection, message: Record): void { - connection.handlers.onMessage?.({ session_id: PROVIDER_SESSION_ID, ...message }) -} - -function wrote(connection: FakeConnection, text: string): boolean { - return connection.sent.some((message) => JSON.stringify(message).includes(text)) -} - -async function openTurn(connection: FakeConnection): Promise { - const text = 'Write a long reply.' - const clientMessageId = await send(text) - await eventually(() => expect(wrote(connection, text)).toBe(true)) - frame(connection, { - type: 'system', - subtype: 'init', - uuid: 'init-1', - model: 'claude-sonnet-5', - capabilities: CAPABILITIES - }) - const written = connection.sent.at(-1)! - frame(connection, { ...written, uuid: written.uuid }) - frame(connection, { - type: 'assistant', - uuid: 'stopped-turn-leaf', - parent_tool_use_id: null, - message: { id: 'msg-1', role: 'assistant', content: [{ type: 'text', text: 'Working on' }] } - }) - await eventually(async () => - expect((await submission(clientMessageId))?.dispatchState).toBe('accepted') - ) - expect( - activeStructuredAgentSessionTurnId((await host.journalSnapshot(SESSION)).items) - ).not.toBeNull() -} - -function stop() { - return host.cancel(CALLER, { envelope: envelope('agentSession.cancel', {}) }) -} - -function laneDrained(): Promise { - return host['tasks'].serialize(SESSION, async () => {}) -} - -/** A commit queues a serialized wake, and the wake queues its delivery step behind that. */ -async function commitSettled(): Promise { - await laneDrained() - await laneDrained() -} - -/** As the real connection: once a close begins it refuses every write, proven or not. */ -function closeUnprovenFor(connection: FakeConnection, failures: number): void { - const close = connection.close - let left = failures - connection.close = async () => { - if (left === 0) { - return close() - } - left -= 1 - connection.closeCount += 1 - connection.closed = true - return false - } - const write = connection.send - connection.send = (message, beforeDispatch) => - connection.closed - ? Promise.reject( - claudeUnwrittenUserMessageError(new Error('claude stream-json connection is closed')) - ) - : write(message, beforeDispatch) -} - -const INTERRUPTED_RESULT = { - type: 'result', - subtype: 'error_during_execution', - is_error: true, - terminal_reason: 'aborted_streaming', - uuid: 'interrupted-result' -} - -/** Stops a turn whose child's close cannot prove the exit `failures` times, and lets the Stop's - * second step fail on it. */ -async function stopWithUnprovenClose(failures: number): Promise { - const connection = claude.connections[0]! - await openTurn(connection) - closeUnprovenFor(connection, failures) - await expect(stop()).resolves.toMatchObject({ ok: true, value: { cancelled: true } }) - frame(connection, INTERRUPTED_RESULT) - await laneDrained() - expect(connection.closeCount).toBe(1) - expect(owedWindDown()).toBeDefined() - return connection -} - -function owedWindDown() { - return host['sessions'].get(SESSION)?.owesProviderChildWindDown -} - -async function waitRows() { - await host.flushStreamedEvents(SESSION) - return (await host.journalSnapshot(SESSION)).items.flatMap((item) => - item.body.kind === 'status' && item.body.failure?.kind === 'previousExitUnverifiable' - ? [item.body] - : [] - ) -} - -async function resumedWith(connection: FakeConnection, text: string): Promise { - return eventually(() => { - const started = claude.connections.at(-1)! - expect(started).not.toBe(connection) - expect(wrote(started, text)).toBe(true) - return started - }) -} - -it('retries the close a Stop could not prove before the next message, then sends it to a resumed child', async () => { - const connection = await stopWithUnprovenClose(1) - - const next = await send('Carry on.') - const resumed = await resumedWith(connection, 'Carry on.') - - expect(connection.closeCount).toBe(2) - expect(wrote(connection, 'Carry on.')).toBe(false) - expect(resumed.closed).toBe(false) - expect(owedWindDown()).toBeUndefined() - expect((await submission(next))?.dispatchState).toBe('pending') - expect(await waitRows()).toEqual([]) -}) - -it('holds the message with its reason while the exit stays unverifiable, and sends it once a later retry proves it', async () => { - const connection = await stopWithUnprovenClose(3) - - // Never refused: the send is accepted and returns at once. - const next = await send('Carry on.') - await eventually(async () => expect(await waitRows()).toHaveLength(1)) - await laneDrained() - expect(await waitRows()).toEqual([ - { - kind: 'status', - tone: 'warning', - text: 'Claude from before may still be running. Your messages will send once it stops.', - failure: { kind: 'previousExitUnverifiable' } - } - ]) - // Still queued, drawn below the chat; never written to the child the Stop could not end. - expect(await submission(next)).toMatchObject({ dispatchState: 'pending' }) - expect((await submission(next))?.handedOverAt).toBeUndefined() - expect(claude.connections).toHaveLength(1) - expect(wrote(connection, 'Carry on.')).toBe(false) - expect(owedWindDown()).toBeDefined() - // The Stop's attempt and the send's one retry: the row's own commit retries nothing. - expect(connection.closeCount).toBe(2) - - // A second message retries once more, and waits under the same row. - const second = await send('And this.') - await eventually(() => expect(connection.closeCount).toBe(3)) - await laneDrained() - expect(await submission(second)).toMatchObject({ dispatchState: 'pending' }) - expect(await waitRows()).toHaveLength(1) - - // The sweep's next tick retries the stop; the exit is proven and both messages go out. - await host['lifetime'].idleSweep.tick() - const resumed = await resumedWith(connection, 'Carry on.') - await eventually(() => expect(wrote(resumed, 'And this.')).toBe(true)) - expect(connection.closeCount).toBe(4) - expect(owedWindDown()).toBeUndefined() - expect(await waitRows()).toHaveLength(1) -}) - -it('lets a held message be withdrawn with Stop, and the owed stop still ends on the next retry', async () => { - const connection = await stopWithUnprovenClose(2) - const next = await send('Carry on.') - await eventually(async () => expect(await waitRows()).toHaveLength(1)) - await laneDrained() - - await expect(stop()).resolves.toMatchObject({ ok: true, value: { cancelled: true } }) - expect(await submission(next)).toMatchObject({ dispatchState: 'rejected' }) - expect(owedWindDown()).toBeDefined() - - await host['lifetime'].idleSweep.tick() - expect(connection.closeCount).toBe(3) - expect(owedWindDown()).toBeUndefined() - expect(store.getRecord(SESSION)?.lease.claimStatus).toBe('released') - // Nothing was waiting, so no child starts. - expect(claude.connections).toHaveLength(1) - expect(wrote(connection, 'Carry on.')).toBe(false) -}) - -function setModel(model: string) { - const fields = { key: 'model', value: model } - return host.setOption(CALLER, { envelope: envelope('agentSession.setOption', fields), ...fields }) -} - -it('refuses an option change with the exit unverifiable after retrying the stop, never writing to the old child', async () => { - const connection = await stopWithUnprovenClose(2) - - await expect(setModel('claude-opus-5')).resolves.toMatchObject({ - ok: false, - refusal: { - code: 'agent_session_ownership_unknown', - details: { reason: 'previousExitUnverifiable', ownerVerdict: 'unverifiable' } - } - }) - expect(connection.closeCount).toBe(2) - expect(connection.calls.some((call) => call.subtype === 'set_model')).toBe(false) - expect(owedWindDown()).toBeDefined() - - // Trying again retries the stop; proven, the pick is the chat's at rest, for the next start. - await expect(setModel('claude-opus-5')).resolves.toMatchObject({ ok: true }) - expect(connection.closeCount).toBe(3) - expect(owedWindDown()).toBeUndefined() - expect(connection.calls.some((call) => call.subtype === 'set_model')).toBe(false) - expect(store.getRecord(SESSION)?.options).toMatchObject({ model: 'claude-opus-5' }) - expect(claude.connections).toHaveLength(1) -}) - -function stopBackgroundTasks() { - const fields = { scope: 'background-tasks' as const } - return host.cancel(CALLER, { envelope: envelope('agentSession.cancel', fields), ...fields }) -} - -/** Holds the user's next message on a Stop whose exit stays unproven through the send's retry. */ -async function heldAfterStop(): Promise { - const connection = await stopWithUnprovenClose(2) - await send('Carry on.') - await eventually(async () => expect(await waitRows()).toHaveLength(1)) - await laneDrained() - expect(connection.closeCount).toBe(2) - return connection -} - -it('sends a held message once an option change proves the exit: the stop that lands hands it over', async () => { - const connection = await heldAfterStop() - - await expect(setModel('claude-opus-5')).resolves.toMatchObject({ ok: true }) - expect(connection.closeCount).toBe(3) - const resumed = await resumedWith(connection, 'Carry on.') - expect(resumed.closed).toBe(false) - expect(owedWindDown()).toBeUndefined() - expect(store.getRecord(SESSION)?.options).toMatchObject({ model: 'claude-opus-5' }) -}) - -it('sends a held message once a background-task stop proves the exit, though that stop finds no agent', async () => { - const connection = await heldAfterStop() - - // Proven gone, the old agent has no tasks left to stop, and the message still goes out. - await expect(stopBackgroundTasks()).resolves.toMatchObject({ - ok: false, - refusal: { details: { reason: 'noLiveOwner' } } - }) - expect(connection.closeCount).toBe(3) - await resumedWith(connection, 'Carry on.') - expect(owedWindDown()).toBeUndefined() -}) - -it('sends a message held after a tab close whose exit was unproven, rather than rejecting it as closed', async () => { - const connection = claude.connections[0]! - closeUnprovenFor(connection, 2) - await expect(host.close(SESSION, 'user-close')).rejects.toThrow() - expect(owedWindDown()).toMatchObject({ cause: 'user-close' }) - - // The chat stays open on the host; the user sends again, and the message waits on that close. - const next = await send('Carry on.') - await eventually(async () => expect(await waitRows()).toHaveLength(1)) - await laneDrained() - expect(connection.closeCount).toBe(2) - - // The close was asked for before this message, so the retry that lands ends nothing it waited on. - await host['lifetime'].idleSweep.tick() - await laneDrained() - expect(await submission(next)).not.toMatchObject({ dispatchState: 'rejected' }) - await resumedWith(connection, 'Carry on.') - expect(owedWindDown()).toBeUndefined() -}) - -it('never stops a live child for a wind-down another, earlier child still owes', async () => { - const connection = claude.connections[0]! - const session = host['sessions'].get(SESSION)! - session.owesProviderChildWindDown = { - generation: 'an-earlier-child', - fence: session.child!.fence, - cause: 'user-stop', - requestedAt: session.journal.cursor() - } - - await send('Carry on.') - await eventually(() => expect(wrote(connection, 'Carry on.')).toBe(true)) - await laneDrained() - await host['lifetime'].idleSweep.tick() - expect(connection.closeCount).toBe(0) - expect(claude.connections).toHaveLength(1) -}) - -it('retries once per new message: commits after a second waiting message retry nothing', async () => { - const connection = await stopWithUnprovenClose(6) - await send('Carry on.') - await eventually(async () => expect(await waitRows()).toHaveLength(1)) - await laneDrained() - await send('And this.') - await eventually(() => expect(connection.closeCount).toBe(3)) - await laneDrained() - - // Any journal commit wakes the delivery loop while a message is queued. - const session = host['sessions'].get(SESSION)! - for (const n of [1, 2, 3]) { - await session.journal.appendItem( - { provider: 'orca', clientMessageId: `unrelated-${n}` }, - { kind: 'status', text: `Unrelated ${n}.` }, - { fence: store.getRecord(SESSION)!.lease.runtimeFence, turnScope: AGENT_JOURNAL_THREAD_SCOPE } - ) - await commitSettled() - } - expect(connection.closeCount).toBe(3) - expect(claude.connections).toHaveLength(1) - - // The sweep still retries, one pass a tick, until the exit is proven; then both go out. - for (const _tick of [1, 2, 3, 4]) { - await host['lifetime'].idleSweep.tick() - } - expect(connection.closeCount).toBe(7) - const resumed = await resumedWith(connection, 'Carry on.') - await eventually(() => expect(wrote(resumed, 'And this.')).toBe(true)) -}) - -it('queues a follow-up as a draft by default while a message waits, and Steer retries the stop once for it', async () => { - const connection = await stopWithUnprovenClose(3) - await send('Carry on.') - await eventually(async () => expect(await waitRows()).toHaveLength(1)) - await laneDrained() - - // Queueing follow-ups is the default: the waiting message reads as working, so this is a draft. - const body = hostTestMessage('And this.') - const delivery = 'queue-if-active' as const - const queued = await host.send(CALLER, { - envelope: envelope('agentSession.send', { body, delivery }), - body, - delivery - }) - expect(queued).toMatchObject({ ok: true, value: { queued: { state: 'waiting' } } }) - await commitSettled() - expect(connection.closeCount).toBe(2) - - // Steer makes it a waiting message, which retries once and waits under the same note. - const messageId = queued.ok && 'queued' in queued.value ? queued.value.queued.messageId : '' - await expect( - host.queuedMessageSend(CALLER, { - envelope: envelope('agentSession.queuedMessageSend', { messageId }), - messageId - }) - ).resolves.toMatchObject({ ok: true }) - await eventually(() => expect(connection.closeCount).toBe(3)) - await commitSettled() - expect(connection.closeCount).toBe(3) - expect(claude.connections).toHaveLength(1) - expect(await waitRows()).toHaveLength(1) - - await host['lifetime'].idleSweep.tick() - const resumed = await resumedWith(connection, 'Carry on.') - await eventually(() => expect(wrote(resumed, 'And this.')).toBe(true)) -}) - -it('rejects as closed a message a second tab close closed, though that close could not reject it itself', async () => { - const connection = claude.connections[0]! - closeUnprovenFor(connection, 3) - await expect(host.close(SESSION, 'user-close')).rejects.toThrow() - const next = await send('Carry on.') - await eventually(async () => expect(await waitRows()).toHaveLength(1)) - await laneDrained() - expect(connection.closeCount).toBe(2) - - // The second close's own rejection of what is queued fails; its stop is a new ask all the same. - const session = host['sessions'].get(SESSION)! - vi.spyOn(session.journal, 'rejectQueuedSubmissions').mockRejectedValueOnce(new Error('disk full')) - await expect(host.close(SESSION, 'user-close')).rejects.toThrow() - expect(connection.closeCount).toBe(3) - expect(await submission(next)).toMatchObject({ dispatchState: 'pending' }) - - // The retry that proves the exit closes what that second close closed. - await host['lifetime'].idleSweep.tick() - await commitSettled() - expect(connection.closeCount).toBe(4) - expect(await submission(next)).toMatchObject({ dispatchState: 'rejected' }) - expect(claude.connections).toHaveLength(1) -}) - -it('notes why a message waits when another operation failed its retry before the message was delivered', async () => { - const connection = await stopWithUnprovenClose(2) - - // The send is accepted, then the option change's failed retry runs before the send's delivery. - const sent = send('Carry on.') - const option = setModel('claude-opus-5') - await expect(option).resolves.toMatchObject({ - ok: false, - refusal: { details: { reason: 'previousExitUnverifiable' } } - }) - await sent - await commitSettled() - expect(connection.closeCount).toBe(2) - expect(await waitRows()).toHaveLength(1) - expect(claude.connections).toHaveLength(1) -}) - -it('keeps an option change at rest when the Stop proved the exit and only its bookkeeping keeps failing', async () => { - const connection = claude.connections[0]! - await openTurn(connection) - // The close proves the exit; draining what the old agent wrote fails for the Stop. The Stop reads - // the journal without a drain, so its close is the first and only drain to fail here. - const barrierLost = { ok: false as const, error: new Error('drain barrier lost') } - const drained = vi - .spyOn(host['runtimeState'].eventSinkFor(SESSION), 'drained') - .mockResolvedValueOnce(barrierLost) - await expect(stop()).resolves.toMatchObject({ ok: true, value: { cancelled: true } }) - frame(connection, INTERRUPTED_RESULT) - await laneDrained() - expect(connection.closeCount).toBe(1) - expect(host['sessions'].get(SESSION)?.child).toBeNull() - expect(owedWindDown()).toBeDefined() - - // The option change's retry of that bookkeeping fails again: reported, and the pick still lands. - drained.mockResolvedValueOnce(barrierLost) - await expect(setModel('claude-opus-5')).resolves.toMatchObject({ ok: true }) - expect(owedWindDown()).toBeDefined() - expect(log.scopes().filter((scope) => scope === 'owed-stop-retry')).toHaveLength(1) - expect(store.getRecord(SESSION)?.options).toMatchObject({ model: 'claude-opus-5' }) - expect(connection.closeCount).toBe(1) - expect(claude.connections).toHaveLength(1) -}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-close-verdict.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-close-verdict.test.ts index b1922b913ed..4ac8e1e35ad 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-close-verdict.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-close-verdict.test.ts @@ -263,7 +263,7 @@ describe('a turn cut short by closing its provider', () => { expect(notices).toEqual(ONE_NOTICE) }) - it("keeps the user's cancellation when a close aborts after the provider settled, then retries", async () => { + it("keeps the user's cancellation when a close's drain fails after the provider settled", async () => { await runningTurn() const sink = host['runtimeState'].eventSinkFor(SESSION) const drained = sink.drained.bind(sink) @@ -275,10 +275,9 @@ describe('a turn cut short by closing its provider', () => { } return drained() }) - // The adapter settled the turn, then the close aborted at the drain after it. - await expect(host.close(SESSION, 'user-close')).rejects.toThrow() + // The adapter settled the turn, then the drain after it failed: reported, and the close ends. + await expect(host.close(SESSION, 'user-close')).resolves.toBeUndefined() expect(closeCalls).toBe(1) - await host.close(SESSION, 'user-close') const { turn } = await settledTurn() expect(turn).toMatchObject({ state: 'interrupted', outcome: 'cancellation' }) }) @@ -287,7 +286,7 @@ describe('a turn cut short by closing its provider', () => { ['user-close', { outcome: 'cancellation' }], ['evict', { outcome: undefined }] ] as const)( - "keeps a %s's cause when the idle sweep finishes a wind-down it could not", + "keeps a %s's cause when the close's drain fails before the host settles its turn", async (cause, verdict) => { providerEnd = null await runningTurn() @@ -301,10 +300,8 @@ describe('a turn cut short by closing its provider', () => { } return drained() }) - // The provider is proven gone, then the close aborts before the host settles its turn. - await expect(host.close(SESSION, cause)).rejects.toThrow() - - await host.collaboratorsForTests().lifetime.idleSweep.tick() + // The provider is proven gone, then the drain fails: the host still settles the turn. + await expect(host.close(SESSION, cause)).resolves.toBeUndefined() expect(closeCalls).toBe(1) const { turn, notices } = await settledTurn() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-codex-stop-row.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-codex-stop-row.test.ts index 7f828a48468..1dbbac5282d 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-codex-stop-row.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-codex-stop-row.test.ts @@ -27,7 +27,7 @@ import { hostTestOperationId, resetHostTestOperationIds } from './structured-agent-session-host-test-data' -import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' +import { recordingStructuredAgentSessionLogger } from './structured-agent-session-logger-test-support' const CALLER = { callerKey: 'client-1' } @@ -38,6 +38,7 @@ let codex: ReturnType let notify: (method: string, params: unknown) => void /** Read at each start, so a test can say what the next start resumes. */ let launch: { resumeThreadId?: string | null } +let log: ReturnType let disposeSession: MockInstance> beforeEach(async () => { @@ -58,19 +59,19 @@ beforeEach(async () => { return {} } const store = await openTestAgentSessionRecordStore(root) - // The runtime's wiring: an echo accepts its send, and an exit reaches the host. + // The runtime's wiring: an echo accepts its send, and every exit reaches the host. launch = {} const adapter = adapterFor(codex, launch, [], { onDispatchSettledLate: (settlement) => void host.settleLateDispatch(settlement), onEvent: (event) => { - if (event.type === 'ended' && 'cause' in event && event.cause === 'unexpected-exit') { + if (event.type === 'ended' && 'cause' in event) { void host.handleAdapterEvent(event) } } }) disposeSession = vi.spyOn(adapter, 'disposeSession') host = new StructuredAgentSessionHost({ - logger: createStructuredAgentSessionLogger(), + logger: (log = recordingStructuredAgentSessionLogger()).logger, store, adapter: Object.assign(adapter, { supportsCreate: () => true }), journalDatabase: openTestJournalHostDatabase(root), @@ -432,16 +433,17 @@ describe('a Codex Stop whose interrupt failed', () => { }) describe('a message after a Codex Stop whose exit was unproven', () => { - it('retries that stop first, then goes to a fresh Codex, never to the old one', async () => { + /** A Stop whose interrupt fails ends the child, whose close cannot prove the exit `failures` + * times. As the real connection, it refuses every request once a close begins. */ + async function stopWithUnprovenClose(failures: number) { await runningTurn() codex.routes['turn/interrupt'] = () => { throw interruptFailure('internal error') } - // As the real connection: once a close begins it refuses every request, proven or not. const old = codex.connections.at(-1)! const close = old.close const request = old.request - let unproven = 1 + let unproven = failures old.close = async () => { old.closed = true if (unproven === 0) { @@ -454,28 +456,81 @@ describe('a message after a Codex Stop whose exit was unproven', () => { old.closed ? Promise.reject(new Error('codex app-server is closing')) : request(method, params) - await stop() await host.flushStreamedEvents(SESSION) - expect(host['sessions'].get(SESSION)?.owesProviderChildWindDown).toBeDefined() + return old + } + + const startedWith = (connection: (typeof codex.connections)[number], text: string) => + connection.calls.some( + (call) => call.method === 'turn/start' && JSON.stringify(call.params).includes(text) + ) + + it('rejects the message while the close stays unverifiable, and starts no Codex beside it', async () => { + const old = await stopWithUnprovenClose(2) + + const sent = await send('carry on') + if (!sent.ok || !('submission' in sent.value)) { + throw new Error(JSON.stringify(sent)) + } + const id = sent.value.submission.clientMessageId + await vi.waitFor(async () => + expect( + (await host.journalSnapshot(SESSION)).submissions.find( + (entry) => entry.clientMessageId === id + ) + ).toMatchObject({ + dispatchState: 'rejected', + reason: "Couldn't stop Codex from before. Send your message again to try once more." + }) + ) + expect(codex.connections).toHaveLength(1) + expect(startedWith(old, 'carry on')).toBe(false) + expect(host['sessions'].get(SESSION)?.child?.close).toBeDefined() + }) + + it('ends the record when the root exits after its close gave up, with no one asking again', async () => { + const old = await stopWithUnprovenClose(1) + const handled = vi.spyOn(host, 'handleAdapterEvent') + + old.handlers.onExit?.(new Error('codex app-server exited'), { expected: true }) + + // The close Orca began, ended: Orca's, never blamed on Codex as a crash. + expect(handled).toHaveBeenCalledWith( + expect.objectContaining({ cause: 'requested-close', failure: { kind: 'hostFault' } }) + ) + await vi.waitFor(() => expect(host['sessions'].get(SESSION)?.child).toBeNull()) + expect(host['sessions'].get(SESSION)?.lastEndedChild).toMatchObject({ cause: 'user-stop' }) + expect(codex.connections).toHaveLength(1) + }) + + it('joins that close first, then goes to a fresh Codex, never to the old one', async () => { + const old = await stopWithUnprovenClose(1) + expect(host['sessions'].get(SESSION)?.child?.close).toMatchObject({ cause: 'user-stop' }) // The next start resumes the chat's thread, as the runtime's launch resolves it from the record. launch.resumeThreadId = THREAD const sent = await send('carry on') expect(sent).toMatchObject({ ok: true }) await vi.waitFor(() => expect(codex.connections).toHaveLength(2)) - await vi.waitFor(() => - expect( - codex.connections[1]!.calls.some( - (call) => call.method === 'turn/start' && JSON.stringify(call.params).includes('carry on') - ) - ).toBe(true) - ) - expect( - old.calls.some( - (call) => call.method === 'turn/start' && JSON.stringify(call.params).includes('carry on') - ) - ).toBe(false) - expect(host['sessions'].get(SESSION)?.owesProviderChildWindDown).toBeUndefined() + await vi.waitFor(() => expect(startedWith(codex.connections[1]!, 'carry on')).toBe(true)) + expect(startedWith(old, 'carry on')).toBe(false) + expect(host['sessions'].get(SESSION)?.lastEndedChild).toMatchObject({ cause: 'user-stop' }) + }) + + it('reports a process tree left unproven by a proven root exit, and ends the record anyway', async () => { + await runningTurn() + codex.routes['turn/interrupt'] = () => { + throw interruptFailure('internal error') + } + const old = codex.connections.at(-1)! + // The forced kill saw the root exit but could not prove the rest of the tree gone. + Object.defineProperty(old, 'processTreeUnproven', { get: () => old.closed }) + + await stop() + + await vi.waitFor(() => expect(host['sessions'].get(SESSION)?.child).toBeNull()) + expect(log.entries.map((entry) => entry.fields.scope)).toContain('provider-close-after-exit') + expect(host['sessions'].get(SESSION)?.lastEndedChild).toMatchObject({ rootGone: true }) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts index 2dac4cacb7d..0133fbc9fed 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-close.test.ts @@ -19,7 +19,6 @@ import { sweepOnce, type RestTestRig } from './structured-agent-session-rest-test-rig' -import { StructuredAgentSessionIdleSweep } from './structured-agent-session-idle-sweep' import { hostTestAttachParams } from './structured-agent-session-host-test-data' let rig: RestTestRig @@ -57,9 +56,8 @@ describe('a stop that fails', () => { rig.clock.now += IDLE_MS + 1 await sweepOnce(rig.host) - expect(openSession()?.owesProviderChildWindDown).toMatchObject({ - generation: expect.any(String) - }) + // The child stays on record with its close unproven, and the next tick joins it. + expect(openSession()?.child?.close).toMatchObject({ cause: 'evict' }) await sweepOnce(rig.host) expect(rig.adapter.closeSession).toHaveBeenCalledTimes(2) @@ -67,23 +65,21 @@ describe('a stop that fails', () => { expect(statusRows().at(-1)?.hostExecutionOwned).toBeUndefined() }) - it('finishes its wind-down on the next tick, whatever its own writes did to the clock (P2-11 b)', async () => { + it('re-derives a release its wind-down could not write before the handle closes (P2-11 b)', async () => { await foundRestTestChat(rig) rig.clock.now += IDLE_MS + 1 - // The child is proven gone and its work settled, then handing the lease back fails. + // The child is proven gone and its work settled, then handing the lease back fails once. const transition = vi .spyOn(rig.store, 'transitionHandoff') .mockRejectedValueOnce(new Error('store unavailable')) await sweepOnce(rig.host) - expect(transition).toHaveBeenCalledOnce() - expect(rig.store.getRecord(SESSION)?.lease.claimStatus).toBe('live') - // Five minutes later, not thirty. - rig.clock.now += 5 * 60_000 - await sweepOnce(rig.host) + // Reported, never retried as a stop: the handle's close writes the release from the proof. + expect(transition).toHaveBeenCalledTimes(2) expect(rig.store.getRecord(SESSION)?.lease.claimStatus).toBe('released') expect(rig.adapter.closeSession).toHaveBeenCalledOnce() + expect(rig.host.hasSession(SESSION)).toBe(false) }) }) @@ -287,50 +283,3 @@ describe('a start that never finishes (P2-15)', () => { await expect(sweepOnce(rig.host)).resolves.toBeUndefined() }) }) - -describe('the wind-down retry with a message queued (P2-31)', () => { - it('waits for the delivery rather than rejecting a message accepted after the failed stop', async () => { - const queued = { clientMessageId: 'm', dispatchState: 'pending', handoverRecorded: true } - const session = { - journal: { - submissions: () => [queued], - pendingSubmissions: () => [queued], - snapshot: () => ({ items: [] }) - }, - child: null, - owesProviderChildWindDown: { generation: 'generation-1', fence: 1 } - } - const stopAgent = vi.fn(async () => undefined) - const finishOwedWindDown = vi.fn(async () => true) - const sweep = new StructuredAgentSessionIdleSweep({ - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a session fixture carrying only the journal and child facts the sweep reads. - sessions: Object.assign(new Map([[SESSION, session as never]]), { - lastActivityAt: () => 0, - touch: () => undefined - }), - serialize: (_id, task) => task(), - now: () => IDLE_MS + 1, - isDisposed: () => false, - deliveryActive: () => true, - childWork: () => undefined, - hasOpenDispatch: () => false, - providerHoldsDispatch: () => false, - stopAgent, - stopStartingAgent: stopAgent, - finishOwedWindDown, - closeConversation: vi.fn(async () => false), - // A failed step fails the test. - logger: { - warn: (_message, fields) => { - throw fields.error - }, - error: (_message, fields) => { - throw fields.error - } - } - }) - await sweep.tick() - expect(stopAgent).not.toHaveBeenCalled() - expect(finishOwedWindDown).not.toHaveBeenCalled() - }) -}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts index 19af6d048d4..3a35b4b138a 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts @@ -12,10 +12,10 @@ import { import type { AgentChildWorkView } from '../../../shared/agent-status-child-work-view' import { createJournalOpenReadRefusals } from '../agent-session-journal/journal-open-failure' import type { StructuredAgentSessionConversations } from './structured-agent-session-conversations' +import { releaseLeaseOfEndedStructuredAgentSessionChild } from './structured-agent-session-child-close' import { abandonQueuedStructuredAgentSessionMessages, closeStructuredAgentSessionConversationUnderSerialize, - finishOwedStructuredAgentSessionWindDownUnderSerialize, stopStructuredAgentSessionAgentUnderSerialize, type StructuredAgentSessionCloseCause, type StructuredAgentSessionLifetimeContext, @@ -57,11 +57,13 @@ export function createStructuredAgentSessionConversationLifetime(host: { }) const stopAgent = (sessionId: string, ending: StructuredAgentSessionStopEnding) => stopStructuredAgentSessionAgentUnderSerialize(host.context(), sessionId, ending) - const finishOwedWindDown = (sessionId: string) => - finishOwedStructuredAgentSessionWindDownUnderSerialize(host.context(), sessionId) - const closeConversation = (sessionId: string): Promise => - closeStructuredAgentSessionConversationUnderSerialize( + const closeConversation = async (sessionId: string): Promise => { + // The handle carries the proof that releases a lease its child's wind-down could not. + if (await releaseLeaseOfEndedStructuredAgentSessionChild(host.context(), sessionId)) { + return false + } + return closeStructuredAgentSessionConversationUnderSerialize( { sessions, closeStatus: (id) => { @@ -72,6 +74,7 @@ export function createStructuredAgentSessionConversationLifetime(host: { }, sessionId ) + } const idleSweep = new StructuredAgentSessionIdleSweep({ sessions, @@ -87,7 +90,6 @@ export function createStructuredAgentSessionConversationLifetime(host: { providerHoldsDispatch: (sessionId) => deps().adapter.holdsDispatch?.(sessionId) === true, // The host puts an idle agent to rest: a turn it cuts short is news, not the user's Stop. stopAgent: (sessionId) => stopAgent(sessionId, { cause: 'evict', resting: true }), - finishOwedWindDown, // A host stop: the delivery loop waiting on this child writes the one error row and rejects // what is queued with it, both worded from the hostStopped fact. stopStartingAgent: (sessionId) => diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts index 6f03cc1345c..4f8679578cf 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts @@ -311,9 +311,10 @@ describe('a Stop that names no turn', () => { expect(await stop()).toMatchObject({ ok: true, value: { cancelled: true } }) expect(closeSession).toHaveBeenCalledExactlyOnceWith(SESSION) + // Bookkeeping after the proven exit: reported, never the Stop's failure. expect(log.entries).toContainEqual( expect.objectContaining({ - fields: expect.objectContaining({ scope: 'stop-child', sessionId: SESSION }) + fields: expect.objectContaining({ scope: 'exit-wind-down', sessionId: SESSION }) }) ) expect((await submission(id))?.dispatchState).toBe('rejected') diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts index 83ab0e8e6fe..b9601ed1a8d 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.ts @@ -26,10 +26,7 @@ import { structuredAgentSessionStartFailure, type StructuredAgentSessionStartFailureCause } from './structured-agent-session-failure-text' -import { - isStructuredAgentSessionPreviousExitUnverifiable, - type StructuredAgentSessionResumeOutcome -} from './structured-agent-session-agent-start' +import type { StructuredAgentSessionResumeOutcome } from './structured-agent-session-agent-start' import type { StructuredAgentSessionChildEndCause, StructuredAgentSessionEndedChild, @@ -42,10 +39,6 @@ import { } from './structured-agent-session-start-failure-row' import { failedProviderChildStart } from './structured-agent-session-provider-child' import { handOverSubmission } from './structured-agent-session-turns' -import { - recordStructuredAgentSessionWindDownWait, - structuredAgentSessionWindDownWaitHolds -} from './structured-agent-session-wind-down-wait-row' import { structuredAgentSessionCommandRunning } from './structured-agent-session-command-turn' import type { StructuredAgentSessionLogger } from './structured-agent-session-logger' @@ -121,16 +114,8 @@ export class StructuredAgentSessionDeliveryLoop { return } if (!prepared.ok) { - const { refusal, diagnostic } = prepared - // A conversation no agent ever ran, such as a cleared chat's, failed to start, not restart. - const newSession = this.deps.record(sessionId)?.providerHandleChain.length === 0 - const cause = { - refusal, - ...(diagnostic ? { diagnostic } : {}), - ...(newSession ? { newSession: true as const } : {}) - } await this.deps.serialize(sessionId, () => - this.fail(sessionId, { startKey: null, cause }) + this.fail(sessionId, this.refusedStart(sessionId, prepared)) ) return } @@ -188,23 +173,15 @@ export class StructuredAgentSessionDeliveryLoop { if (!oldest || (session.child && structuredAgentSessionCommandRunning(session.journal))) { return this.stop(sessionId) } - // Already waiting on a stop that could not prove its child gone: a new message retries it, and - // any other retry that lands wakes this loop itself, so the waiting row's own commit does not. - // Another operation's retry may have failed first, so the row is made sure of here too. - if (structuredAgentSessionWindDownWaitHolds(session)) { - await recordStructuredAgentSessionWindDownWait(session, sessionId, this.deps) - return this.stop(sessionId) - } const failedStart = startThatFailedWhileQueued(session, oldest) if (failedStart) { return this.fail(sessionId, failedStart) } const ready = await this.deps.ensureProviderChild(sessionId, oldest.clientMessageId) - if (!ready.ok && isStructuredAgentSessionPreviousExitUnverifiable(ready.refusal)) { - // The start retried that stop first and still could not prove the exit: the message waits, - // saying why, rather than being refused. The row is written once per unproven child. - await recordStructuredAgentSessionWindDownWait(session, sessionId, this.deps) - return this.stop(sessionId) + if (!ready.ok && ready.refusal.details?.reason === 'previousExitUnverifiable') { + // Failed in the step that was refused: a message accepted, or an exit proven, after it must + // not be failed for a verdict that no longer holds. + return this.fail(sessionId, this.refusedStart(sessionId, ready)) } if (!ready.ok) { return ready @@ -272,6 +249,23 @@ export class StructuredAgentSessionDeliveryLoop { return 'continue' } + /** A start the session refused, as the failure every queued message it was for is rejected with. */ + private refusedStart( + sessionId: string, + { refusal, diagnostic }: Extract + ): StartFailure { + // A conversation no agent ever ran, such as a cleared chat's, failed to start, not restart. + const newSession = this.deps.record(sessionId)?.providerHandleChain.length === 0 + return { + startKey: null, + cause: { + refusal, + ...(diagnostic ? { diagnostic } : {}), + ...(newSession ? { newSession: true as const } : {}) + } + } + } + private async fail(sessionId: string, failure: StartFailure): Promise<'stop'> { const session = this.deps.sessions.get(sessionId) if (session) { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.ts index 2b9340859f4..398d04873d4 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.ts @@ -3,11 +3,18 @@ import type { StructuredAgentSessionLifecycleEvent } from './structured-agent-se import { stopAgentSessionProviderRoot } from './structured-agent-session-provider-exit-proof' import type { StructuredAgentSessionHostDeps, - StructuredAgentSessionHostSession + StructuredAgentSessionHostSession, + StructuredAgentSessionProviderChild } from './structured-agent-session-host-types' import type { StructuredAgentSessionSinkBarrier } from './structured-agent-session-event-sink' import { settleStructuredAgentSessionProviderStarted } from './structured-agent-session-provider-started' -import { settleUnexpectedStructuredAgentSessionExit } from './structured-agent-session-unexpected-exit' +import { + endExitedStructuredAgentSessionChildUnderSerialize, + settleStructuredAgentSessionChildExit, + type StructuredAgentSessionChildExit, + type StructuredAgentSessionChildExitContext +} from './structured-agent-session-child-exit' +import type { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' export class StructuredAgentSessionEventRecovery { private readonly sinkFailures = new Set() @@ -22,13 +29,31 @@ export class StructuredAgentSessionEventRecovery { publishStatus?: (sessionId: string) => void serialize: (sessionId: string, task: () => Promise) => Promise now: () => number + runtimeState: StructuredAgentSessionHostRuntimeState + wakeDelivery: (sessionId: string) => void } ) {} - private get exitContext() { - return { ...this.context, logger: this.context.deps.logger } + private get exitContext(): StructuredAgentSessionChildExitContext { + const { deps, runtimeState } = this.context + return { + ...this.context, + logger: deps.logger, + route: { + runtimeState, + acknowledgeRelease: (sessionId) => deps.adapter.acknowledgeSessionRelease?.(sessionId) + } + } } + /** The one exit handler, for a caller inside the session's serialize that proved the exit. */ + endExitedChildUnderSerialize = ( + sessionId: string, + child: StructuredAgentSessionProviderChild, + exit: StructuredAgentSessionChildExit + ): Promise => + endExitedStructuredAgentSessionChildUnderSerialize(this.exitContext, sessionId, child, exit) + recoverAfterSinkFailure(sessionId: string, error: unknown): void { if (this.sinkFailures.has(sessionId)) { return @@ -39,26 +64,17 @@ export class StructuredAgentSessionEventRecovery { const child = this.context.sessions.get(sessionId)?.child const stop = this.context.deps.adapter.forceCloseSession ?? this.context.deps.adapter.closeSession - if (!child || !stop) { - return null + if (!child || !stop || !(await stopAgentSessionProviderRoot(() => stop(sessionId)))) { + return } - const { fence, generation: acquisitionGeneration } = child - const stopped = await stopAgentSessionProviderRoot(() => stop(sessionId)) - if (!stopped || !acquisitionGeneration) { - return null - } - return { - type: 'ended', - sessionId, + // Ended in the same step as the stop, so no report of that close can end it first as a + // quiet rest: Orca stopped the provider because its own journal failed. + await this.endExitedChildUnderSerialize(sessionId, child, { + expected: false, reason: `journal sink failure: ${error instanceof Error ? error.message : String(error)}`, - // Orca stopped the provider because its own journal failed. - failure: agentSessionFailureFact('hostFault'), - cause: 'unexpected-exit', - fence, - acquisitionGeneration - } as const + failure: agentSessionFailureFact('hostFault') + }) }) - .then((event) => (event ? this.handle(event) : undefined)) .catch((error: unknown) => this.context.deps.logger.warn( 'stopping a provider after its journal failed did not finish', @@ -72,12 +88,13 @@ export class StructuredAgentSessionEventRecovery { .finally(() => this.sinkFailures.delete(sessionId)) } - /** An exit is settled and shown; nothing restarts the child. The next send does, through the - * delivery loop, which also owns any message still queued. */ + /** Every child's exit ends its record here, expected or not. An exit is settled and shown; + * nothing restarts the child. The next send does, through the delivery loop, which also owns any + * message still queued. */ async handle(event: StructuredAgentSessionLifecycleEvent): Promise { if (event.type === 'started') { return settleStructuredAgentSessionProviderStarted(this.context, event) } - await settleUnexpectedStructuredAgentSessionExit(this.exitContext, event) + await settleStructuredAgentSessionChildExit(this.exitContext, event) } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction-deadline.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction-deadline.ts deleted file mode 100644 index f34caba3593..00000000000 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction-deadline.ts +++ /dev/null @@ -1,51 +0,0 @@ -// A bound on how long teardown may take, expressed as a wrapper around the eviction steps rather -// than a change to them. -// -// The step list is ordered and abort-on-failure for reasons that have nothing to do with time, and -// a deadline must not disturb either. Wrapping each step's `run` keeps the order, and a timeout -// surfaces as that step failing — which is exactly the behavior wanted here: the rest of the -// eviction aborts, the session stays indexed, and the child stays LOADED. A stuck app-server that -// is still holding a conversation is a better outcome than one killed out from under it; the next -// close retries. - -import type { StructuredAgentSessionEvictionStep } from './structured-agent-session-eviction' - -export const STRUCTURED_AGENT_SESSION_EVICTION_STEP_TIMEOUT_MS = 10_000 - -export class StructuredAgentSessionEvictionTimeoutError extends Error { - constructor( - readonly step: string, - readonly timeoutMs: number - ) { - super(`agent session eviction step "${step}" did not finish within ${timeoutMs}ms`) - this.name = 'StructuredAgentSessionEvictionTimeoutError' - } -} - -export function withStructuredAgentSessionEvictionDeadline( - steps: readonly StructuredAgentSessionEvictionStep[], - timeoutMs = STRUCTURED_AGENT_SESSION_EVICTION_STEP_TIMEOUT_MS -): readonly StructuredAgentSessionEvictionStep[] { - return steps.map((step) => ({ - name: step.name, - run: async (context) => { - let timer: ReturnType | undefined - try { - await Promise.race([ - Promise.resolve(step.run(context)), - new Promise((_resolve, reject) => { - timer = setTimeout( - () => reject(new StructuredAgentSessionEvictionTimeoutError(step.name, timeoutMs)), - timeoutMs - ) - timer.unref?.() - }) - ]) - } finally { - if (timer) { - clearTimeout(timer) - } - } - } - })) -} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts index 6cd06bbd61e..4da10e213f6 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts @@ -5,16 +5,12 @@ import { STRUCTURED_AGENT_SESSION_EVICTION_STEPS, type StructuredAgentSessionEvictionContext } from './structured-agent-session-eviction' -import { - AgentSessionAcquisitionRootExitObservedError, - AgentSessionPreSpawnError -} from './structured-agent-session-adapter' import { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' import { withJournalQueueMembers } from './structured-agent-session-journal-double-test-support' import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' import { recordingStructuredAgentSessionLogger } from './structured-agent-session-logger-test-support' -function context(): StructuredAgentSessionEvictionContext & { order: string[] } { +function context(closeError?: Error): StructuredAgentSessionEvictionContext & { order: string[] } { const order: string[] = [] return { order, @@ -24,26 +20,19 @@ function context(): StructuredAgentSessionEvictionContext & { order: string[] } unbind: vi.fn(() => order.push('unbind')), drained: vi.fn(async () => { order.push('drained') - return { ok: true } + return { ok: true as const } }), - close: vi.fn(() => order.push('close')) - } as unknown as StructuredAgentSessionEvictionContext['eventSink'], - adapter: { - closeSession: vi.fn(async () => { - order.push('closeSession') - return true + close: vi.fn(() => { + order.push('close') + if (closeError) { + throw closeError + } }) - } as unknown as StructuredAgentSessionEvictionContext['adapter'], + }, acknowledgeRelease: vi.fn(() => { order.push('acknowledgeRelease') }), - discardSink: vi.fn(() => order.push('discardSink')), - settleWork: vi.fn(async () => { - order.push('settleWork') - }), - releaseLease: vi.fn(async () => { - order.push('releaseLease') - }) + discardSink: vi.fn(() => order.push('discardSink')) } } @@ -56,177 +45,34 @@ function runtimeState(): StructuredAgentSessionHostRuntimeState { } as never) } -describe('structured agent session eviction', () => { - it('stops the child before it lets the sink go, then acknowledges the release', async () => { +describe("the route release after a child's exit", () => { + it('lets the sink go before it acknowledges the release', async () => { const ctx = context() await evictStructuredAgentSession(ctx) - expect(ctx.order).toEqual([ - 'closeSession', - 'drained', - 'settleWork', - 'unbind', - 'close', - 'discardSink', - 'releaseLease', - 'acknowledgeRelease' - ]) + expect(ctx.order).toEqual(['unbind', 'close', 'discardSink', 'acknowledgeRelease']) }) - it('uses disposal rather than handoff close when the chat is removed', async () => { - const ctx = context() - const closeSession = vi.fn(async () => { - throw new Error('resume cursor unavailable') - }) - const disposeSession = vi.fn(async () => true) - ctx.adapter = { ...ctx.adapter, closeSession, disposeSession } - - await evictStructuredAgentSession(ctx) - - expect(disposeSession).toHaveBeenCalledWith('session-1') - expect(closeSession).not.toHaveBeenCalled() - }) - - it('names every step, so a half-finished eviction says which one failed', () => { + it('names every step, so a failure says which one it was', () => { expect(STRUCTURED_AGENT_SESSION_EVICTION_STEPS.map((step) => step.name)).toEqual([ - 'snapshot-before-stop', - 'stop-provider-child', - 'drain-published', - 'settle-dead-generation', 'stop-publishing', 'close-sink', 'discard-sink', - 'release-lease', 'acknowledge-release' ]) }) - it('still stops the child when the pre-stop snapshot cannot drain the sink', async () => { - vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) - try { - const ctx = context() - const snapshot = vi.fn() - ctx.beforeProviderChildStop = snapshot - // A journal write that never settles, then a healthy sink once the child is stopped. - vi.mocked(ctx.eventSink.drained).mockReturnValueOnce(new Promise(() => {})) + // Bookkeeping for a process already gone: none of it may keep the child on record. + it('reports a failed step with its name and still runs every step after it', async () => { + const recorded = recordingStructuredAgentSessionLogger() + const ctx = { ...context(new Error('sink already gone')), logger: recorded.logger } - const eviction = evictStructuredAgentSession(ctx) - await vi.advanceTimersByTimeAsync(1_000) + await expect(evictStructuredAgentSession(ctx)).resolves.toBeUndefined() - expect(snapshot).toHaveBeenCalledOnce() - expect(ctx.adapter.closeSession).toHaveBeenCalledOnce() - await eviction - } finally { - vi.useRealTimers() - } - }) - - it('aborts after a failed drain barrier without unbinding or acknowledging the release', async () => { - const ctx = context() - ctx.eventSink.drained = vi.fn(async () => { - ctx.order.push('drained') - return { ok: false, error: new Error('append failed') } - }) as unknown as StructuredAgentSessionEvictionContext['eventSink']['drained'] - - await expect(evictStructuredAgentSession(ctx)).rejects.toMatchObject({ - step: 'drain-published' - }) - expect(ctx.eventSink.unbind).not.toHaveBeenCalled() - expect(ctx.eventSink.close).not.toHaveBeenCalled() - expect(ctx.discardSink).not.toHaveBeenCalled() - expect(ctx.releaseLease).not.toHaveBeenCalled() - expect(ctx.acknowledgeRelease).not.toHaveBeenCalled() - expect(ctx.order).toEqual(['closeSession', 'drained']) - }) -}) - -// Closing the codex child is not silent: the adapter emits its `ended` event and flushes coalesced -// text as it shuts down, and those rows are what clear the running-turn marker. If the sink is -// already closed the journal keeps claiming the agent is working, forever. -describe('rows the provider emits while closing', () => { - it('still reach the journal', async () => { - const state = runtimeState() - const sessionId = 'session-closing-rows' - const sink = state.eventSinkFor(sessionId) - const published: string[] = [] - sink.bind({ - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: these rows are publications only, which reach nothing on the journal but its in-order read. - journal: withJournalQueueMembers({ appendItem: async () => ({}) }) as never, - fence: 1, - publish: () => published.push('final-flush') - }) - - await evictStructuredAgentSession({ - sessionId, - logger: recordingStructuredAgentSessionLogger().logger, - eventSink: sink, - adapter: { - closeSession: async () => { - // What codex-structured-session-close does on its way out. - sink.sink.publish() - return true - } - } as never, - discardSink: () => state.discardEventSink(sessionId), - releaseLease: async () => {}, - acknowledgeRelease: () => {} - }) - - expect(published).toEqual(['final-flush']) - }) -}) - -// `closeSession` returning false means the adapter could not prove the child exited and has kept -// the session indexed on purpose so a retry can reach it. -describe('a child that will not stop', () => { - it.each([ - new AgentSessionAcquisitionRootExitObservedError(new Error('root exited')), - new AgentSessionPreSpawnError(new Error('spawn failed')) - ])('continues eviction after an actionable provider verdict', async (error) => { - const ctx = context() - ctx.adapter.closeSession = vi.fn(async () => { - throw error - }) - const stopped = vi.fn() - ctx.onProviderChildStopped = stopped - - await evictStructuredAgentSession(ctx) - - // The host ends its child on the one reading of the verdict, not a second one of its own. - expect(stopped).toHaveBeenCalledWith({ rootGone: true }) - expect(ctx.order).toEqual([ - 'drained', - 'settleWork', - 'unbind', - 'close', - 'discardSink', - 'releaseLease', - 'acknowledgeRelease' + expect(ctx.order).toEqual(['unbind', 'close', 'discardSink', 'acknowledgeRelease']) + expect(recorded.entries.map((entry) => entry.fields.error)).toEqual([ + expect.objectContaining({ step: 'close-sink' }) ]) - }) - - it('aborts without acknowledging the release, so the next stop is a real retry', async () => { - const ctx = context() - ctx.adapter.closeSession = vi.fn(async () => false) - - await expect(evictStructuredAgentSession(ctx)).rejects.toMatchObject({ - step: 'stop-provider-child' - }) - expect(ctx.acknowledgeRelease).not.toHaveBeenCalled() - expect(ctx.discardSink).not.toHaveBeenCalled() - expect(ctx.order).toEqual([]) - }) - - it('reports the failing step and leaves the sink usable for the retry', async () => { - const ctx = context() - ctx.adapter.closeSession = vi.fn(async () => { - throw new Error('child would not stop') - }) - - await expect(evictStructuredAgentSession(ctx)).rejects.toBeInstanceOf( - StructuredAgentSessionEvictionError - ) - expect(ctx.eventSink.close).not.toHaveBeenCalled() - expect(ctx.acknowledgeRelease).not.toHaveBeenCalled() + expect(recorded.entries[0]?.fields.error).toBeInstanceOf(StructuredAgentSessionEvictionError) }) }) @@ -241,9 +87,7 @@ describe('eviction against the real sink cache', () => { sessionId, logger: recordingStructuredAgentSessionLogger().logger, eventSink: state.eventSinkFor(sessionId), - adapter: { closeSession: async () => true } as never, discardSink: () => state.discardEventSink(sessionId), - releaseLease: async () => {}, acknowledgeRelease: () => {} }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.ts index dcc31c451d2..2ecd885242c 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.ts @@ -1,54 +1,28 @@ -// Stopping one structured session's provider child and handing its lease back. +// What a host does with a provider child's route once its exit has ended the child's record: the +// sink it wrote through and the adapter's index of it. Settling its work and handing the lease back +// are the exit handler's (`structured-agent-session-child-exit`); these come after. // // Teardown is a DATA list, not a method body, for the reason this file exists at all: the host // tracked which sessions were live in a map, and tore them down at three unrelated call sites // (app quit, handoff to a TUI, and error cleanup). Closing a chat was never wired to any of them, // so a provider child outlived the chat that owned it for the whole app session. // -// ORDER. The provider child stops FIRST. Closing it is not silent: the codex adapter emits its -// `ended` event and flushes coalesced text as part of shutting down, and those are the rows that -// clear the running-turn marker. Draining or closing the sink ahead of that drops them, which -// leaves the durable journal claiming the agent is still working — a worse outcome than the leak -// this teardown exists to fix. So: stop the child, drain what it emitted on its way out, then let -// the sink go. The one step ahead of the stop only reads, for quit's resume offer. -// -// FAILURE. A step that fails ABORTS the rest. `closeSession` returning false means the child's -// exit was not proven and the adapter has deliberately kept the session indexed so a retry can -// reach it; forgetting it anyway stranded the process forever and reported success. Leaving the -// session in place is what makes the next close a real retry instead of a no-op. +// FAILURE. Each step is bookkeeping for a process already gone, so a step that fails is reported +// and the rest still run: none of them may keep a dead child on record or gate the next send. -import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' -import { stopAgentSessionProviderRoot } from './structured-agent-session-provider-exit-proof' import type { DeferredStructuredAgentSessionEventSink } from './structured-agent-session-event-sink' -import type { StructuredAgentSessionStopVerdict } from './structured-agent-session-host-types' import { withTimeout } from '../../../shared/promise-timeout-fallback' import type { StructuredAgentSessionLogger } from './structured-agent-session-logger' export type StructuredAgentSessionEvictionContext = { sessionId: string - hasProviderChild?: boolean - eventSink: DeferredStructuredAgentSessionEventSink - adapter: StructuredAgentSessionAdapter + eventSink: Pick logger: StructuredAgentSessionLogger /** Tells the adapter the released lease is done with, so it drops this child's route and index. * The conversation stays: stopping the agent never closes its journal. */ acknowledgeRelease: () => Promise | void /** Drops the cached sink so a later attach mints a fresh one. */ discardSink: () => void - /** Fires right before the stop, while the child's turn and background roster are still live. A - * throw is logged, never allowed to abort the stop. */ - beforeProviderChildStop?: () => void - /** Fires with the stop's verdict once `stopAgentSessionProviderRoot` read the root gone, so host - * bookkeeping stops claiming a child. */ - onProviderChildStopped?: (verdict: StructuredAgentSessionStopVerdict) => void - /** Whether this host still owes the child's wind-down. Distinct from `hasProviderChild`, which a - * proven exit retires mid-run: the two disagree for exactly the steps a retry has to repeat. */ - owesProviderChildWindDown?: boolean - /** Settles work owned by the child after its final callbacks have drained. */ - settleWork?: () => Promise - /** Hands the lease back now that this host's child is proven gone. No-ops when the record is - * not this host's to release. */ - releaseLease: () => Promise } /** The resume offer is advisory; a stalled sink must not hold the child's stop behind it. */ @@ -59,58 +33,25 @@ export type StructuredAgentSessionEvictionStep = { run: (context: StructuredAgentSessionEvictionContext) => Promise | void } +/** Quit's resume offer: what the sidebar shows, read while the child is still running. Events the + * provider already delivered are part of what it showed at the stop. A throw is logged. */ +export async function snapshotBeforeStructuredAgentSessionStop( + context: Pick, + snapshot: () => void +): Promise { + await withTimeout(context.eventSink.drained(), SNAPSHOT_DRAIN_TIMEOUT_MS, null) + try { + snapshot() + } catch { + context.logger.warn('capturing a recovery witness before a stop failed', { + scope: 'recovery-witness', + sessionId: context.sessionId + }) + } +} + export const STRUCTURED_AGENT_SESSION_EVICTION_STEPS: readonly StructuredAgentSessionEvictionStep[] = [ - { - // Quit's resume offer: what the sidebar shows, read while the child is still running. - name: 'snapshot-before-stop', - run: async (context) => { - if (context.hasProviderChild === false || !context.beforeProviderChildStop) { - return - } - // Events the provider already delivered are part of what the sidebar showed at the stop. - await withTimeout(context.eventSink.drained(), SNAPSHOT_DRAIN_TIMEOUT_MS, null) - try { - context.beforeProviderChildStop() - } catch { - context.logger.warn('capturing a recovery witness before a stop failed', { - scope: 'recovery-witness', - sessionId: context.sessionId - }) - } - } - }, - { - name: 'stop-provider-child', - run: async (context) => { - if (context.hasProviderChild === false) { - return - } - // An adapter with no close has nothing to stop; anything else must PROVE the exit. - const stop = context.adapter.disposeSession ?? context.adapter.closeSession - const rootGone = stop - ? await stopAgentSessionProviderRoot(() => stop.call(context.adapter, context.sessionId)) - : true - if (!rootGone) { - throw new Error('provider child exit was not proven') - } - context.onProviderChildStopped?.({ rootGone }) - } - }, - { - name: 'drain-published', - run: async (context) => { - const barrier = await context.eventSink.drained() - if (!barrier.ok) { - throw barrier.error - } - } - }, - { - name: 'settle-dead-generation', - run: (context) => - context.owesProviderChildWindDown === false ? undefined : context.settleWork?.() - }, { name: 'stop-publishing', run: (context) => context.eventSink.unbind() }, { name: 'close-sink', run: (context) => context.eventSink.close() }, // Why: the runtime caches one sink per session id and hands the SAME instance to the next @@ -118,11 +59,6 @@ export const STRUCTURED_AGENT_SESSION_EVICTION_STEPS: readonly StructuredAgentSe // accepts every provider event and publishes none. Attach's own failure path already pairs // these two; eviction has to as well. { name: 'discard-sink', run: (context) => context.discardSink() }, - // Why here and not last: the durable lease still names a process this host just stopped, and a - // record left claiming a live owner is one nothing can resume — the next send would find a - // session it may not acquire. Placed BEFORE the acknowledgement so a release that cannot be - // written aborts while the adapter still routes the session, which is what makes the retry real. - { name: 'release-lease', run: (context) => context.releaseLease() }, { name: 'acknowledge-release', run: (context) => context.acknowledgeRelease() } ] @@ -137,11 +73,8 @@ export class StructuredAgentSessionEvictionError extends Error { } } -/** - * Runs the eviction steps in order, stopping at the first failure. The step name travels with the - * error because the caller's only useful response is to retry, and a retry is only safe when the - * session is still indexed — which is exactly what aborting preserves. - */ +/** Runs every wind-down step in order. A failure is reported with the step that failed, and the + * rest still run. */ export async function evictStructuredAgentSession( context: StructuredAgentSessionEvictionContext, steps: readonly StructuredAgentSessionEvictionStep[] = STRUCTURED_AGENT_SESSION_EVICTION_STEPS @@ -150,7 +83,11 @@ export async function evictStructuredAgentSession( try { await step.run(context) } catch (error) { - throw new StructuredAgentSessionEvictionError(step.name, context.sessionId, error) + context.logger.warn('a wind-down step after the agent exited failed', { + scope: 'exit-wind-down', + sessionId: context.sessionId, + error: new StructuredAgentSessionEvictionError(step.name, context.sessionId, error) + }) } } } 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 2c312ef17e8..5e8d7e2097f 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 @@ -13,26 +13,18 @@ import type { AgentJournalSubmission } from '../../../shared/agent-session-journ import { agentSessionFailureFact } from '../../../shared/agent-session-failure' import { agentSessionFailureWords } from '../../../shared/agent-session-failure-words' import { - evictStructuredAgentSession, - STRUCTURED_AGENT_SESSION_EVICTION_STEPS, - type StructuredAgentSessionEvictionContext + snapshotBeforeStructuredAgentSessionStop, + StructuredAgentSessionEvictionError } from './structured-agent-session-eviction' -import { withStructuredAgentSessionEvictionDeadline } from './structured-agent-session-eviction-deadline' +import { joinStructuredAgentSessionChildClose } from './structured-agent-session-child-close' import type { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' import type { StructuredAgentSessionHostDeps, StructuredAgentSessionHostSession, - StructuredAgentSessionOwedWindDown, - StructuredAgentSessionProviderChildIdentity + StructuredAgentSessionProviderChild } from './structured-agent-session-host-types' -import { - endProviderChild, - pendingProviderChildWindDown, - sameProviderChild, - structuredAgentSessionConversationFence -} from './structured-agent-session-provider-child' -import { releaseStoredStructuredAgentSessionOwner } from './structured-agent-session-lease-release' -import { settleStructuredAgentSessionDeadGeneration } from './structured-agent-session-dead-generation-settlement' +import type { StructuredAgentSessionChildExit } from './structured-agent-session-child-exit' +import { structuredAgentSessionConversationFence } from './structured-agent-session-provider-child' import type { StructuredAgentSessionStopCause } from './structured-agent-session-adapter' export type { StructuredAgentSessionStopEnding } from './structured-agent-session-host-stop-event' import { @@ -50,6 +42,13 @@ export type StructuredAgentSessionLifetimeContext = { publishStatus?: (sessionId: string) => void /** Hands the delivery loop what is queued; for a caller inside the session's serialize. */ wakeDelivery?: (sessionId: string) => void + /** The one exit handler (`structured-agent-session-child-exit`), for a caller inside the + * session's serialize that proved its child's exit. */ + endExitedChild: ( + sessionId: string, + child: StructuredAgentSessionProviderChild, + exit: StructuredAgentSessionChildExit + ) => Promise /** Quit-only snapshot taken immediately before the provider child is stopped. */ restartWitness?: { beforeStop: (sessionId: string) => void @@ -90,43 +89,12 @@ export async function abandonQueuedStructuredAgentSessionMessages( ) } -/** The wind-down this host owes for the session's child. A live child always owes one, whatever a - * previous childless eviction recorded: a remembered tombstone must never outrank the child in - * front of it. */ -function owedProviderChildWindDown( - session: StructuredAgentSessionHostSession -): StructuredAgentSessionProviderChildIdentity | undefined { - return session.child - ? { generation: session.child.generation, fence: session.child.fence } - : session.owesProviderChildWindDown -} - -/** The stop this pass owes. A retry continues the one already asked for, keeping where it was asked; - * any other stop is a new ask, even one with the same cause: a second close closes what came since. */ -function owedStop( - session: StructuredAgentSessionHostSession, - cause: StructuredAgentSessionStopCause, - retry: boolean -): StructuredAgentSessionOwedWindDown | undefined { - const owed = owedProviderChildWindDown(session) - if (!owed) { - return undefined - } - const asked = session.owesProviderChildWindDown - const continues = retry && asked !== undefined && sameProviderChild(asked, owed) - return { - generation: owed.generation, - fence: owed.fence, - cause, - requestedAt: continues ? asked.requestedAt : session.journal.cursor() - } -} - /** - * The agent goes to rest; the conversation stays. Runs the eviction steps under a deadline. A step - * that fails — or runs out of time — aborts the rest and leaves the wind-down owed, so the next - * stop is a real retry. `ending` is how the child's end is told: a user's Stop, the host stopping it - * for a cause (with its text), or an eviction the conversation's close follows. + * The agent goes to rest; the conversation stays. Begins the child's close, or joins the one + * already begun, and waits for the exit's proof as long as a caller may. Still unproven, it throws + * and the close keeps running: a later proof, or the next asker's attempt, ends the record. + * `ending` is how the child's end is told: a user's Stop, the host stopping it for a cause (with + * its text), or an eviction the conversation's close follows. The first stop's ending decides. */ export async function stopStructuredAgentSessionAgentUnderSerialize( context: StructuredAgentSessionLifetimeContext, @@ -134,159 +102,69 @@ export async function stopStructuredAgentSessionAgentUnderSerialize( ending: StructuredAgentSessionStopEnding ): Promise { const session = context.sessions.get(sessionId) - if (!session) { + const child = session?.child + if (!session || !child) { return } - // Judged before the kill: a stop that ends nothing writes nothing. - const recorded = (await stopEndsWork(context, sessionId, session, ending)) - ? recordStopEvent(context, sessionId, session, ending) - : Promise.resolve(null) - // The obligation OUTLIVES the child. `child` is ended the instant the adapter proves the exit, - // so a step that aborts after that point would otherwise leave the retry reading "no child - // here" and skipping the settlement and the lease release it still owes. - const asked = 'recorded' in ending ? ending.recorded : ending.cause - // A retry finishes the stop that ended the child, so the child's end keeps that stop's cause. - const cause = session.child ? asked : (session.owesProviderChildWindDown?.cause ?? asked) - const owed = owedStop(session, cause, ending.retry === true) - session.owesProviderChildWindDown = owed - const stopping = session.child - const eviction: StructuredAgentSessionEvictionContext = { - sessionId, - // The retry must not re-stop a child the adapter already proved gone, so this stays honest. - hasProviderChild: stopping !== null, - owesProviderChildWindDown: owed !== undefined, - eventSink: context.runtimeState.eventSinkFor(sessionId), - adapter: context.deps.adapter, - logger: context.deps.logger, - ...(context.restartWitness - ? { beforeProviderChildStop: () => context.restartWitness?.beforeStop(sessionId) } - : {}), - // Host state must not disagree with the adapter for the steps in between. - onProviderChildStopped: (verdict) => { - if (stopping) { - endProviderChild(session, { - generation: stopping.generation, - fence: stopping.fence, - cause, - reason: ('reason' in ending ? ending.reason : undefined) ?? null, - duringStartup: stopping.phase === 'starting', - // A later retry that proves the exit still ends the child at the Stop it finishes. - ...(owed ? { endedAt: owed.requestedAt } : {}), - ...verdict - }) - } - context.restartWitness?.stopped(sessionId) - }, - acknowledgeRelease: () => context.deps.adapter.acknowledgeSessionRelease?.(sessionId), - discardSink: () => context.runtimeState.discardEventSink(sessionId), - settleWork: async () => { - // Folded before the fallback's end is built, so the end reads it (`turnEndAfterStop`). - await recorded - const fence = - owed?.fence ?? structuredAgentSessionConversationFence(context.deps.store, sessionId) - const settled = await settleStructuredAgentSessionDeadGeneration({ - journal: session.journal, - sessionId, - fence, - settlementId: `expected-close:${sessionId}:${fence}:${owed?.generation ?? 'unknown'}`, - pendingSubmissionReason: 'provider_closed_before_acknowledgement', - // Only a turn no adapter settled: one with no close, or whose settle threw. Whether it was - // a person's Stop is its event's to say (`turnEndAfterStop`). - verdict: { state: 'interrupted', completedAt: context.now() }, - showUnexpectedExitOutcome: false - }) - if (!settled.ok) { - context.deps.logger.warn("settling a closed agent's work failed", { - scope: 'close-settlement', - sessionId, - error: settled.error - }) - // Without the cause the log names the step and nothing else. - throw new Error('dead generation work settlement failed', { cause: settled.error }) - } - }, - releaseLease: async () => { - if (owed) { - await releaseStoredStructuredAgentSessionOwner({ - store: context.deps.store, - sessionId, - hasProviderChild: true, - expectedFence: owed.fence, - now: context.now() - }) - } - session.owesProviderChildWindDown = undefined - // Whatever ended the child, the row belongs to the conversation: it shows not-running, and - // only the conversation's close forgets it. - context.publishStatus?.(sessionId) - // The stop's own end hands over what waited on it, whichever caller's retry landed. - context.wakeDelivery?.(sessionId) + const cause = 'recorded' in ending ? ending.recorded : ending.cause + if (!child.close) { + // Judged before the kill: a stop that ends nothing writes nothing. Its event is issued before + // the kill and never awaited by it; the journal writes rows in order. + const recorded = (await stopEndsWork(context, sessionId, session, ending)) + ? recordStopEvent(context, sessionId, session, ending) + : Promise.resolve(null) + child.close = { + cause, + reason: ('reason' in ending ? ending.reason : undefined) ?? null, + recorded, + requestedAt: session.journal.cursor() } - } - try { - await evictStructuredAgentSession( - eviction, - withStructuredAgentSessionEvictionDeadline(STRUCTURED_AGENT_SESSION_EVICTION_STEPS) + } else if (child.close.cause === cause) { + // The same stop asked again, such as a second close of the chat, closes what came since, and + // binds again what its child's end cuts. + child.close.requestedAt = session.journal.cursor() + child.close.recorded = child.close.recorded.then((settle) => + settle ? session.journal.stopMarks.beginSettle() : null ) - } catch (error) { - if (session.owesProviderChildWindDown === owed && owed) { - session.owesProviderChildWindDown = { ...owed, failedAt: session.journal.cursor() } + } + const { close } = child + try { + if (context.restartWitness) { + await snapshotBeforeStructuredAgentSessionStop( + { + sessionId, + eventSink: context.runtimeState.eventSinkFor(sessionId), + logger: context.deps.logger + }, + () => context.restartWitness?.beforeStop(sessionId) + ) + } + if ((await joinStructuredAgentSessionChildClose(context, sessionId, child)) !== 'exited') { + throw new StructuredAgentSessionEvictionError( + 'stop-provider-child', + sessionId, + new Error('provider child exit was not proven') + ) } - throw error } finally { // A person's close binds what its child's end cut; done, proven or not, it binds no more. - void recorded.then((settle) => session.journal.stopMarks.settled(settle)) + void close?.recorded.then((settle) => session.journal.stopMarks.settled(settle)) } } -/** - * Retries the wind-down an earlier stop left owed, with that stop's own cause: a child it could - * not prove gone takes no input, so nothing may write to it or start beside it until this lands. - * One pass of the stop, bounded by its own step deadline (10 s): a shorter bound would cut a - * supervised Claude's exit proof (up to about 7 s) short. Resolves whether nothing is owed now; a - * failure is reported, never thrown, and leaves the exit unverifiable, never exited. Landing, the - * stop itself hands over what waited on it. - */ -export async function finishOwedStructuredAgentSessionWindDownUnderSerialize( - context: StructuredAgentSessionLifetimeContext, - sessionId: string -): Promise { - const session = context.sessions.get(sessionId) - const owed = session && pendingProviderChildWindDown(session) - if (!owed) { - return true - } - try { - await stopStructuredAgentSessionAgentUnderSerialize( - context, - sessionId, - owed.cause === 'user-stop' - ? { recorded: 'user-stop', retry: true } - : { cause: owed.cause, retry: true } - ) - } catch (error) { - context.deps.logger.warn('retrying an unfinished agent stop failed', { - scope: 'owed-stop-retry', - sessionId, - error - }) - } - return context.sessions.get(sessionId)?.owesProviderChildWindDown === undefined -} - /** A close's cause: the user closing this chat, or the host evicting it (quit, idle, teardown). */ export type StructuredAgentSessionCloseCause = Extract< StructuredAgentSessionStopCause, 'user-close' | 'evict' > -/** Whether the conversation's handle is only a cache now: no child, no wind-down owed, and nothing - * queued or waiting on the provider. */ +/** Whether the conversation's handle is only a cache now: no child, and nothing queued or waiting + * on the provider. */ export function structuredAgentSessionConversationClosable( session: StructuredAgentSessionHostSession ): boolean { return ( - owedProviderChildWindDown(session) === undefined && + session.child === null && !session.journal.submissions().some(isQueuedAgentJournalSubmission) && session.journal.pendingSubmissions().length === 0 ) @@ -315,9 +193,7 @@ export async function closeStructuredAgentSessionConversationUnderSerialize( return true } -/** Stops every provider child owned by this host while keeping failed evictions reachable. A - * session whose child is already stopped but whose wind-down aborted is still in scope — that is - * the retry. */ +/** Stops every provider child owned by this host while keeping failed evictions reachable. */ export async function evictOwnedStructuredAgentSessions( context: StructuredAgentSessionLifetimeContext & { serialize: (sessionId: string, task: () => Promise) => Promise @@ -325,7 +201,7 @@ export async function evictOwnedStructuredAgentSessions( retainOnFailure: Set ): Promise { const ownedSessionIds = [...context.sessions] - .filter(([, session]) => owedProviderChildWindDown(session) !== undefined) + .filter(([, session]) => session.child !== null) .map(([sessionId]) => sessionId) // Retained up front and cleared only once a stop settles: the quit phase is bounded, and a // timeout leaves these still running. Closing their journals underneath them is the one outcome diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-stop-event.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-stop-event.ts index 70cfd07bb7a..1c9e2720aca 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-stop-event.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-stop-event.ts @@ -13,7 +13,7 @@ import { sentSinceStop } from './structured-agent-session-queued-stop' /** How a stop ends the child, and why (`lastEndedChild`). A person's Stop wrote its event in its * own step (`recorded` names its reason); any other stop names the reason its event records, with * the host's text for it. Quit writes none: its resume marker's trigger records why. */ -export type StructuredAgentSessionStopEnding = ( +export type StructuredAgentSessionStopEnding = | { recorded: 'user-stop' } | { cause: Exclude @@ -23,10 +23,6 @@ export type StructuredAgentSessionStopEnding = ( * no work its event records. */ resting?: true } -) & { - /** The retry of a stop already owed, set only by that retry: its event, if any, is written. */ - retry?: true -} /** How long a host stop waits for the session's sink before it judges whether the stop ends work. */ const STOP_EVENT_DRAIN_TIMEOUT_MS = 1_000 @@ -34,8 +30,8 @@ const STOP_EVENT_DRAIN_TIMEOUT_MS = 1_000 /** * Whether this stop ends work its event must record: a running turn or an unanswered send, a start's * own included, read once the sink drained what the provider already said. A start that carries - * no send ends nothing. A person's Stop wrote its own event, and quit, the idle sweep's rest and a - * retry of a stop already owed write none. + * no send ends nothing. A person's Stop wrote its own event, and quit and the idle sweep's rest + * write none; a stop that joins a close already begun writes none either. */ export async function stopEndsWork( context: StructuredAgentSessionLifetimeContext, @@ -44,7 +40,7 @@ export async function stopEndsWork( ending: StructuredAgentSessionStopEnding ): Promise { const { child, journal } = session - if ('recorded' in ending || ending.quit || ending.resting || ending.retry || !child) { + if ('recorded' in ending || ending.quit || ending.resting || !child) { return false } // A failed drain has nothing more to deliver, so the journal's read as it stands holds. One diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts index ae75f476591..7016e91e17c 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts @@ -11,6 +11,7 @@ import type { AgentSessionRecoveryCapsule } from '../../runtime/agent-session-re import type { AgentSessionSpawnTokenScan } from '../../runtime/agent-session-spawn-token-process-scan' import type { JournalHostDatabase } from '../agent-session-journal/journal-host-database' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import type { JournalStopSettle } from '../agent-session-journal/queued-message-pause' import type { StructuredAgentSessionAdapter, StructuredAgentSessionChildEndCause, @@ -43,13 +44,18 @@ export type StructuredAgentSessionProviderChildIdentity = { readonly fence: number } -/** A wind-down still owed, with the stop that owes it: a retry finishes that stop. */ -export type StructuredAgentSessionOwedWindDown = StructuredAgentSessionProviderChildIdentity & { +/** The close a stop began for its child. It lives on the child and ends with it: once begun, the + * child takes no input again, and every later stop, start or provider write joins it. */ +export type StructuredAgentSessionChildClose = { + /** The first stop's, which the child's end keeps however many asks join it. */ readonly cause: StructuredAgentSessionStopCause - /** Where the journal stood when the stop was asked for; the child's end is ordered there. */ - readonly requestedAt: AgentJournalCursor - /** Where it stood once the newest pass failed: a message accepted by then waited through a retry. */ - readonly failedAt?: AgentJournalCursor + readonly reason: string | null + /** The Stop event that stop wrote, folded before the work it ends is settled, with the settle a + * person's close that named no turn opens; a repeated ask reopens it. */ + recorded: Promise + /** Where the journal stood when that stop was asked for: the child's end is ordered there, so a + * message accepted while the exit was being proven came after it. A repeated ask moves it. */ + requestedAt: AgentJournalCursor } /** The provider process behind a conversation. Written only in @@ -61,6 +67,7 @@ export type StructuredAgentSessionProviderChild = StructuredAgentSessionProvider /** The queued message whose delivery started this child, fixed when the start is made; absent * for any other start. In memory only: it tells a restart offer its own start from another. */ readonly startedFor?: string + close?: StructuredAgentSessionChildClose } /** What ending a child established about its provider root. A stop's comes only from @@ -83,8 +90,7 @@ export type StructuredAgentSessionEndedChild = StructuredAgentSessionProviderChi duringStartup: boolean startedFor?: string /** Where the conversation's journal stood when the child ended, to order the end against a - * message's acceptance. A stop's end stands where it was asked for: a message accepted while - * retries proved the exit waited on it, and came after it. */ + * message's acceptance. A close's end stands where its stop was asked for. */ endedAt: AgentJournalCursor } @@ -97,10 +103,6 @@ export type StructuredAgentSessionHostSession = { * has none — so it may not be evicted to free a child, nor have its lease released as an * observed exit. */ child: StructuredAgentSessionProviderChild | null - /** The wind-down this host still owes for a child it started: settling that generation's work - * and handing the lease back. Outlives `child`, which ends the moment the adapter proves the - * exit — an eviction that aborts after that point must still finish it on the next close. */ - owesProviderChildWindDown?: StructuredAgentSessionOwedWindDown lastEndedChild?: StructuredAgentSessionEndedChild } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts index 8256f90e53c..59599c5adfe 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts @@ -151,7 +151,9 @@ export class StructuredAgentSessionHost { ), publishStatus: this.clientDelivery.publishStatusAndSettlement, serialize: (sessionId, task) => this.tasks.trackAttach(this.serialize(sessionId, task)), - now: () => this.now() + now: () => this.now(), + runtimeState: this.runtimeState, + wakeDelivery: (sessionId) => this.conversationDelivery.loop.wake(sessionId) }) this.restartResume = createStructuredAgentSessionRestartResume(deps, this.sessions, { ...structuredAgentSessionRestartResumeSurfaces(this, this.now), @@ -186,7 +188,8 @@ export class StructuredAgentSessionHost { sessions: this.sessions, now: () => this.now(), publishStatus: this.clientDelivery.publishStatus, - wakeDelivery: (sessionId: string) => this.conversationDelivery.loop.wake(sessionId) + wakeDelivery: (sessionId: string) => this.conversationDelivery.loop.wake(sessionId), + endExitedChild: this.eventRecovery.endExitedChildUnderSerialize } satisfies StructuredAgentSessionLifetimeContext } @@ -249,8 +252,11 @@ export class StructuredAgentSessionHost { // Trigger inlined rather than imported: `AgentSessionResumeTrigger` in shared is the canonical // type, and this file has no line budget left for the import. + /** Quit: no exit or recovery settled after this starts a child or hands a message over. */ + stopDelivery = (): void => this.conversationDelivery.dispose() + async flushAllStreamedEvents(options?: { trigger?: 'quit' | 'update' }): Promise { - this.conversationDelivery.dispose() + this.stopDelivery() await flushStructuredAgentSessionHost({ ...this.lifetimeContext(), idleSweep: this.lifetime, @@ -272,11 +278,8 @@ export class StructuredAgentSessionHost { openConversation: this.conversationDelivery.open, ensureAgent: (sessionId) => agentStart.ensureStructuredAgentSessionAgentForOperation(this.attachContext(), sessionId), - finishOwedStop: (sessionId) => - agentStart.finishOwedStructuredAgentSessionStopForProviderWrite( - this.attachContext(), - sessionId - ), + joinChildClose: (sessionId) => + agentStart.joinClosingStructuredAgentSessionChild(this.attachContext(), sessionId), wakeDelivery: (sessionId) => this.conversationDelivery.loop.wake(sessionId), stopAgent: (sessionId, ending) => this.lifetime.stopAgent(sessionId, ending), wakeQueuedDrain: (sessionId) => this.queued.drain.schedule(sessionId), diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.test.ts index 7823f65b535..9cb6dbab4ef 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.test.ts @@ -319,7 +319,6 @@ describe('the idle sweep with no child running (P2-22 ii)', () => { providerHoldsDispatch: () => false, stopAgent, stopStartingAgent: stopAgent, - finishOwedWindDown: vi.fn(async () => true), closeConversation, // A failed step fails the test. logger: { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.ts index 96a5f59cf84..1b67f259ed9 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.ts @@ -15,7 +15,6 @@ import type { AgentJournalRenderItem } from '../../../shared/agent-session-journ import type { AgentChildWorkView } from '../../../shared/agent-status-child-work-view' import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' import type { StructuredAgentSessionLogger } from './structured-agent-session-logger' -import { pendingProviderChildWindDown } from './structured-agent-session-provider-child' export const STRUCTURED_AGENT_SESSION_IDLE_SWEEP_INTERVAL_MS = 5 * 60_000 export const STRUCTURED_AGENT_SESSION_IDLE_MS = 30 * 60_000 @@ -37,8 +36,6 @@ export type StructuredAgentSessionIdleSweepDeps = { providerHoldsDispatch: (sessionId: string) => boolean /** Each of these runs inside the session's serialize and never takes it again. */ stopAgent: (sessionId: string) => Promise - /** Retries a stop that did not finish; landing, it hands over what waited on it. */ - finishOwedWindDown: (sessionId: string) => Promise stopStartingAgent: (sessionId: string) => Promise closeConversation: (sessionId: string) => Promise logger: StructuredAgentSessionLogger @@ -111,12 +108,6 @@ export class StructuredAgentSessionIdleSweep { if (!session || this.deps.isDisposed()) { return } - // A stop that did not finish: retry it now, before the idle test, so the rows its settlement - // wrote cannot push the retry out. A running delivery step retries it itself. - if (pendingProviderChildWindDown(session) && !this.deps.deliveryActive(sessionId)) { - await this.deps.finishOwedWindDown(sessionId) - return - } // Owed work is activity, read every tick, so the agent gets a full window once it ends: a child // can read done before the lead's wake-up turn writes anything. if (session.child && session.child.phase !== 'starting' && this.owesWork(sessionId, session)) { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-release.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-release.ts index fe584a50fce..63e3add288e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-release.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-release.ts @@ -1,9 +1,9 @@ -// Handing the durable lease back after eviction stopped this host's child. +// Handing the durable lease back once this host's own provider child is gone. // -// Guarded on `hasProviderChild` for a reason that is not bookkeeping: a session restored only for -// reading, or one a TUI owns, names an owner process this host never started and may still be -// alive. Writing `exit-observed` against that record would release a lease out from under a running -// process and let a second writer in. +// Only an exit this host saw or proved, of the child it started at this fence, may release: a +// session restored only for reading, or one a TUI owns, names an owner process this host never +// started and may still be alive. Writing `exit-observed` against that record would release a lease +// out from under a running process and let a second writer in. import { isSurfaceReleasableAgentSessionRecord, @@ -17,45 +17,16 @@ export type StructuredAgentSessionLeaseStore = Pick< 'getRecord' | 'transitionHandoff' > -export async function releaseStoredStructuredAgentSessionOwner(input: { - store: StructuredAgentSessionLeaseStore - sessionId: string - hasProviderChild: boolean - expectedFence: number - now: number -}): Promise { - if (!input.hasProviderChild) { - return - } - const record = input.store.getRecord(input.sessionId) - if ( - !record || - record.lease.runtimeFence !== input.expectedFence || - !isSurfaceReleasableAgentSessionRecord(record) - ) { - return - } - await releaseStoredAgentSessionOwnerAfterSurfaceClose(input.store, { - sessionId: input.sessionId, - expectedFence: input.expectedFence, - now: input.now - }) -} - -/** Releases only the exact provider child whose exit the adapter positively observed. */ -export async function releaseStoredStructuredAgentSessionOwnerAfterUnexpectedExit(input: { +/** Releases the lease of the child whose exit this host observed, with that exit's evidence. + * Throws `agent_session_checkpoint_stale` when the record no longer names that child. */ +export async function releaseStoredStructuredAgentSessionOwnerAfterExit(input: { store: StructuredAgentSessionLeaseStore sessionId: string expectedFence: number - expectedAcquisitionGeneration: string - acquisitionGeneration: string | null now: number exitObservedAt?: number exitReason?: string }): Promise { - if (input.acquisitionGeneration !== input.expectedAcquisitionGeneration) { - throw new Error('agent_session_checkpoint_stale') - } const record = input.store.getRecord(input.sessionId) if ( !record || @@ -68,7 +39,7 @@ export async function releaseStoredStructuredAgentSessionOwnerAfterUnexpectedExi sessionId: input.sessionId, expectedFence: input.expectedFence, now: input.now, - exitObservedAt: input.exitObservedAt, + ...(input.exitObservedAt === undefined ? {} : { exitObservedAt: input.exitObservedAt }), ...(input.exitReason ? { exitReason: input.exitReason } : {}) }) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts index e77975dc5c2..216a952f830 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-context.ts @@ -32,9 +32,9 @@ export type StructuredAgentSessionMutationContext = { openConversation: (sessionId: string) => Promise /** Gives the session a provider child; inside the caller's serialize. */ ensureAgent: (sessionId: string) => Promise - /** Finishes a stop an earlier attempt left owed, for an operation that starts no child; inside - * the caller's serialize. */ - finishOwedStop: (sessionId: string) => Promise + /** Joins a close a stop began on the session's child, for an operation that starts no child; + * inside the caller's serialize. */ + joinChildClose: (sessionId: string) => Promise /** A message was accepted: the session's delivery loop hands it over. */ wakeDelivery: (sessionId: string) => void /** Stops the session's provider child, keeping its conversation; inside the caller's serialize. diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts index 5785cfc53e1..d4236f395da 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child.ts @@ -11,7 +11,6 @@ import type { AgentSessionJournal } from '../agent-session-journal/journal-store import type { StructuredAgentSessionEndedChild, StructuredAgentSessionHostSession, - StructuredAgentSessionOwedWindDown, StructuredAgentSessionProviderChild, StructuredAgentSessionProviderChildIdentity } from './structured-agent-session-host-types' @@ -48,7 +47,7 @@ export function markProviderChildStarted( return child !== null } -/** `endedAt` is a stop's ask; an exit ends where the journal stands. */ +/** `endedAt` is a close's ask; an exit of the child's own ends where the journal stands. */ export function endProviderChild( session: ChildBearer, ended: Omit & { @@ -77,15 +76,6 @@ export function failedProviderChildStart( return !session.child && ended?.duringStartup && ended.cause !== 'user-stop' ? ended : null } -/** The owed wind-down an operation reaching the provider finishes first. One owed for another child - * never outranks the child in front of it, which that child's own stop finishes. */ -export function pendingProviderChildWindDown( - session: Pick -): StructuredAgentSessionOwedWindDown | undefined { - const { child, owesProviderChildWindDown: owed } = session - return owed && (!child || sameProviderChild(child, owed)) ? owed : undefined -} - export function sameProviderChild( a: StructuredAgentSessionProviderChildIdentity, b: StructuredAgentSessionProviderChildIdentity diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-exit-proof.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-exit-proof.ts index 9d27e570db4..1545a8fa618 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-exit-proof.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-exit-proof.ts @@ -50,16 +50,24 @@ function provenExitAcquisitionFailure(cause: unknown): unknown { } /** Whether a stop left the provider root gone. The lease follows the root, so a first-hand root - * exit or a processless child ends the session whatever its descendants did; any other - * failure still throws. */ -export async function stopAgentSessionProviderRoot(stop: () => Promise): Promise { + * exit or a processless child ends the session whatever its descendants did. A descendant left + * unconfirmed, or bookkeeping that failed after the proven exit, is only `report`ed. Any other + * failure throws. */ +export async function stopAgentSessionProviderRoot( + stop: () => Promise, + report?: (error: Error) => void +): Promise { try { return (await stop()) === true } catch (error) { if ( error instanceof AgentSessionAcquisitionRootExitObservedError || - isAgentSessionPreSpawnError(error) + error instanceof AgentSessionAcquisitionExitProvenError ) { + report?.(error) + return true + } + if (isAgentSessionPreSpawnError(error)) { return true } throw error diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.ts index e74947054e4..62ac5fc644e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.ts @@ -100,11 +100,12 @@ export function openForWrite( } /** For an operation the running child performs, which starts none: the conversation, then any - * stop an earlier attempt left owed, so it never reaches a child that takes no input. */ + * close a stop began on that child, which it joins, so it never reaches a child that takes no + * input. */ export function openForProviderWrite( context: Pick< StructuredAgentSessionMutationContext, - 'openConversation' | 'finishOwedStop' | 'deps' + 'openConversation' | 'joinChildClose' | 'deps' >, envelope: AgentSessionMutationEnvelope ): () => Promise { @@ -114,7 +115,7 @@ export function openForProviderWrite( envelope, context.deps.logger ) - return opened.ok ? context.finishOwedStop(envelope.sessionId) : opened + return opened.ok ? context.joinChildClose(envelope.sessionId) : opened } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-failure-exit.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-failure-exit.test.ts index 8235d0d6705..07f54bc2ab4 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-failure-exit.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-failure-exit.test.ts @@ -6,10 +6,10 @@ import { } from '../../../shared/agent-session-record.test-fixture' import { structuredAgentSessionCompactBody } from './structured-agent-session-command-turn' import { - settleUnexpectedStructuredAgentSessionExit, - type StructuredAgentSessionUnexpectedExitContext, - type StructuredAgentSessionUnexpectedExitSession -} from './structured-agent-session-unexpected-exit' + settleStructuredAgentSessionChildExit, + type StructuredAgentSessionChildExitContext, + type StructuredAgentSessionChildExitSession +} from './structured-agent-session-child-exit' import { recordingStructuredAgentSessionLogger } from './structured-agent-session-logger-test-support' const SESSION = 'session-1' @@ -17,7 +17,7 @@ const GENERATION = 'generation-1' const REASON = 'claude stream-json exited (code 1): session limit reached' const STARTUP_TEXT = 'Claude stopped before it finished starting. Send your message to try again.' -function startedSession(): StructuredAgentSessionUnexpectedExitSession & { +function startedSession(): StructuredAgentSessionChildExitSession & { journal: { appendLifecycleBatch: ReturnType } } { return { @@ -34,7 +34,7 @@ function startedSession(): StructuredAgentSessionUnexpectedExitSession & { } } -function contextFor(session: StructuredAgentSessionUnexpectedExitSession) { +function contextFor(session: StructuredAgentSessionChildExitSession) { let record: AgentSessionRecord = agentSessionRecordFixture( agentSessionLeaseFixture({ sessionId: SESSION, @@ -47,7 +47,7 @@ function contextFor(session: StructuredAgentSessionUnexpectedExitSession) { unreconciled: false }) ) - const context: StructuredAgentSessionUnexpectedExitContext = { + const context: StructuredAgentSessionChildExitContext = { logger: recordingStructuredAgentSessionLogger().logger, store: { getRecord: () => record, @@ -78,7 +78,7 @@ describe('a provider that ends before it finished starting', () => { it('tells the user why, even with no response in progress', async () => { const session = startedSession() - await settleUnexpectedStructuredAgentSessionExit(contextFor(session), { + await settleStructuredAgentSessionChildExit(contextFor(session), { ...ended, // The adapter typed the start's own failure; the host keeps it rather than reword it. failure: { kind: 'notSignedIn' }, @@ -106,7 +106,7 @@ describe('a provider that ends before it finished starting', () => { it('keeps an ordinary idle exit silent', async () => { const session = startedSession() - await settleUnexpectedStructuredAgentSessionExit(contextFor(session), ended) + await settleStructuredAgentSessionChildExit(contextFor(session), ended) expect(session.child).toBeNull() expect(session.journal.appendLifecycleBatch).not.toHaveBeenCalled() @@ -118,7 +118,7 @@ describe('a provider that ends before it finished starting', () => { child: { generation: GENERATION, fence: 7, phase: 'starting' as const } } - await settleUnexpectedStructuredAgentSessionExit(contextFor(session), { + await settleStructuredAgentSessionChildExit(contextFor(session), { ...ended, failure: { kind: 'providerExited', detail: { text: REASON, audience: 'log' } } }) @@ -157,7 +157,7 @@ describe('a provider that ends before it finished starting', () => { } } - await settleUnexpectedStructuredAgentSessionExit(contextFor(session), ended) + await settleStructuredAgentSessionChildExit(contextFor(session), ended) const text = 'Claude stopped before it finished starting. Run /compact again.' expect(session.journal.rejectPendingSubmissions).toHaveBeenCalledWith( diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-binding.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-binding.test.ts index a91ed7c8cbd..65da28ca6b2 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-binding.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-binding.test.ts @@ -131,6 +131,9 @@ async function mailTurn(): Promise { async function evictedAt(): Promise { let atClose: JournalStopEvent[] = [] rig.closeSession.mockImplementationOnce(async () => { + // The event is issued before the kill, never awaited by it: the journal writes it ahead of + // anything the kill makes the child write. + await new Promise((resolve) => setImmediate(resolve)) atClose = stopEvents() return true }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-entries.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-entries.test.ts index dd0acf82b37..1d7781c9886 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-entries.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-entries.test.ts @@ -45,6 +45,9 @@ function stopEvents(): JournalStopEvent[] { function stopEventsAtClose(): { events: JournalStopEvent[] | null } { const seen: { events: JournalStopEvent[] | null } = { events: null } rig.closeSession.mockImplementationOnce(async () => { + // The event is issued before the kill, never awaited by it: the journal writes it ahead of + // anything the kill makes the child write. + await new Promise((resolve) => setImmediate(resolve)) seen.events = stopEvents() return true }) @@ -286,9 +289,9 @@ describe('every Stop entry writes its event, with its reason, before it ends the expect(atClose.events?.map((event) => event.reason)).toEqual(['user-stop']) }) - // The idle sweep finishes a stop whose exit was unproven: the same stop, so its event stands alone. + // A later stop joins a close whose exit was unproven: the same close, so its event stands alone. it.each([['user-close' as const], ['evict' as const]])( - 'writes one event for a close (%s) whose exit was unproven, and none for its retry', + 'writes one event for a close (%s) whose exit was unproven, and none for a stop that joins it', async (cause) => { rig = await createQueuedMessageTestRig({ idleSweep: MANUAL_IDLE_SWEEP }) await runningTurn() @@ -296,13 +299,14 @@ describe('every Stop entry writes its event, with its reason, before it ends the const session = () => rig.host.collaboratorsForTests().sessions.get(HOST_TEST_SESSION)! await rig.host.close(HOST_TEST_SESSION, cause).catch(() => undefined) - expect(session().child).not.toBeNull() - expect(session().owesProviderChildWindDown).toMatchObject({ cause }) + expect(session().child?.close).toMatchObject({ cause }) expect(stopEvents()).toEqual([{ reason: cause, turnId: 'turn-1', at: expect.any(Number) }]) - await idleSweep().tick() + await rig.host['tasks'].serialize(HOST_TEST_SESSION, () => + rig.host['lifetime'].stopAgent(HOST_TEST_SESSION, { cause: 'host-stop' }) + ) - expect(session().owesProviderChildWindDown).toBeUndefined() + expect(session().child).toBeNull() expect(stopEvents().map((event) => event.reason)).toEqual([cause]) expect(session().lastEndedChild?.cause).toBe(cause) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-settle-binding.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-settle-binding.test.ts index 14e62854bbc..a3fee6b2ed4 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-settle-binding.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-settle-binding.test.ts @@ -194,4 +194,22 @@ describe("a person's close pressed before its send's turn showed", () => { expect(later).toMatchObject({ state: 'interrupted' }) expect(later).not.toHaveProperty('outcome') }) + + // The first close's kill is unproven and its settle closes with it; asking again re-kills, so a + // turn that ask's end cuts is the person's again. + it('binds the turn a repeated close cuts after a first close that came back unproven', async () => { + rig = await createQueuedMessageTestRig() + const sent = await rig.workingSend() + const close = () => rig.host['lifetime'].stopAgent(HOST_TEST_SESSION, { cause: 'user-close' }) + rig.closeSession.mockResolvedValueOnce(false) + await expect(close()).rejects.toThrow() + rig.closeSession.mockImplementationOnce(async () => { + await turnOpens(sent) + return true + }) + + await close() + + expect(openedTurn()).toMatchObject({ state: 'interrupted', outcome: 'cancellation' }) + }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts index 5a1ad69f613..c681369ce27 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts @@ -337,7 +337,7 @@ describe('a chat that closes', () => { expect(closeSession).toHaveBeenCalledOnce() }) - it('settles and releases on the retry when a step after the child stopped aborts', async () => { + it('reports a failed drain after the child stopped, and still settles and releases', async () => { await attach() dispatch.mockResolvedValueOnce({ state: 'admitted' }) const body = hostTestMessage('pending across an aborted eviction') @@ -351,12 +351,9 @@ describe('a chat that closes', () => { failEvictionDrain() const settled = captureSettledSubmissions() - await expect(host.close(SESSION, 'evict')).rejects.toMatchObject({ step: 'drain-published' }) - // The child is proven gone, but the wind-down it owes is not done: nothing settled, no release. - expect(session!.child).toBeNull() - expect(store.getRecord(SESSION)?.lease.claimStatus).not.toBe('released') - + // The child is proven gone: the failed drain is bookkeeping, reported, and the rest still runs. await expect(host.close(SESSION, 'evict')).resolves.toBeUndefined() + expect(session!.child).toBeNull() expect(closeSession).toHaveBeenCalledOnce() expect(store.getRecord(SESSION)?.lease).toMatchObject({ claimStatus: 'released', @@ -675,7 +672,7 @@ describe('an unexpected provider exit', () => { unsubscribe() }) - it('keeps a requested close out of recovery', async () => { + it('ends the record of a requested close as a close, never as a crash to recover from', async () => { await attach() const fence = store.getRecord(SESSION)?.lease.runtimeFence ?? 0 @@ -689,7 +686,9 @@ describe('an unexpected provider exit', () => { }) expect(acquire).toHaveBeenCalledOnce() - expect(store.getRecord(SESSION)?.lease.claimStatus).toBe('live') + expect(host['sessions'].get(SESSION)?.child).toBeNull() + expect(host['sessions'].get(SESSION)?.lastEndedChild).not.toHaveProperty('failure') + expect(store.getRecord(SESSION)?.lease.claimStatus).toBe('released') }) it('recovers after a failed lifecycle barrier and dispatches a distinct next message', async () => { @@ -842,19 +841,16 @@ describe('an unexpected provider exit', () => { }) }) -describe('a quit over an eviction that never got its retry', () => { - // Nothing calls `close` a second time when the user quits instead of reopening the chat, so the - // quit sweep is the last thing that can hand the lease back — and it only reaches the session if - // it still counts a stopped child's unfinished wind-down as owed. - it('finishes the wind-down the aborted close left behind', async () => { +describe('a quit after a close whose drain failed', () => { + // The close's own wind-down hands the lease back past a failed drain, so quit finds nothing owed. + it('has nothing of that child left to finish', async () => { await attach() - await sendPending('pending across an abandoned eviction') + await sendPending('pending across a failed drain') const settled = captureSettledSubmissions() failEvictionDrain() - await expect(host.close(SESSION, 'evict')).rejects.toMatchObject({ step: 'drain-published' }) - expect(host['sessions'].get(SESSION)?.child).toBeNull() - expect(store.getRecord(SESSION)?.lease.claimStatus).not.toBe('released') + await expect(host.close(SESSION, 'evict')).resolves.toBeUndefined() + expect(host['sessions'].get(SESSION)?.child ?? null).toBeNull() await host.flushAllStreamedEvents() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts index 6337d65f553..3a0e43d2328 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts @@ -101,9 +101,9 @@ export type StructuredAgentSessionStopWindDown = { /** * A session-ending Stop's second step, queued behind its first in the same tick so nothing sent - * meanwhile reaches the child it ends. The Stop has answered: a failure here is reported. The next - * operation that reaches the agent retries the wind-down it leaves owed, and so does the idle - * sweep's next tick. + * meanwhile reaches the child it ends. The Stop has answered: a failure here is reported. A close + * it could not prove keeps the child on record, and the next operation that reaches the agent + * joins that close. */ export async function endStoppedStructuredAgentSession( ctx: Pick, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts index 4f4e414657d..e5222eae1b6 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts @@ -9,10 +9,10 @@ import { import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' import { settleStaleStructuredAgentSessionState } from './structured-agent-session-dead-generation-settlement' import { - settleUnexpectedStructuredAgentSessionExit, - type StructuredAgentSessionUnexpectedExitContext, - type StructuredAgentSessionUnexpectedExitSession -} from './structured-agent-session-unexpected-exit' + settleStructuredAgentSessionChildExit, + type StructuredAgentSessionChildExitContext, + type StructuredAgentSessionChildExitSession +} from './structured-agent-session-child-exit' import { recordingStructuredAgentSessionLogger } from './structured-agent-session-logger-test-support' const exitOutcome = (agent: string): string => @@ -105,7 +105,7 @@ describe('provider-exit settlement', () => { } } as unknown as StructuredAgentSessionHostSession - await settleUnexpectedStructuredAgentSessionExit( + await settleStructuredAgentSessionChildExit( // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the exit settlement reads only these members of its context; the store double is partial. { logger: recordingStructuredAgentSessionLogger().logger, @@ -190,7 +190,7 @@ describe('provider-exit settlement', () => { transitionHandoff: async () => ({ lease: { runtimeFence: 8 } }) } - await settleUnexpectedStructuredAgentSessionExit( + await settleStructuredAgentSessionChildExit( { store, sessions: new Map([[SESSION, session]]), @@ -285,7 +285,7 @@ describe('provider-exit settlement', () => { epoch: 'epoch-1', sequence: 3 })) - const session: StructuredAgentSessionUnexpectedExitSession = { + const session: StructuredAgentSessionChildExitSession = { child: { generation: GENERATION, fence: 7, phase: 'ready' }, journal: { cursor: () => ({ epoch: 'epoch-1', sequence: 0 }), @@ -298,7 +298,7 @@ describe('provider-exit settlement', () => { } const { store } = mutableStore() - const context: StructuredAgentSessionUnexpectedExitContext = { + const context: StructuredAgentSessionChildExitContext = { logger: recordingStructuredAgentSessionLogger().logger, store, sessions: new Map([[SESSION, session]]), @@ -316,7 +316,7 @@ describe('provider-exit settlement', () => { serialize: async (_sessionId: string, task: () => Promise) => task(), now: () => 1_234 } - await settleUnexpectedStructuredAgentSessionExit(context, { + await settleStructuredAgentSessionChildExit(context, { type: 'ended', sessionId: SESSION, reason: 'provider exited after completing the turn', @@ -344,7 +344,7 @@ describe('provider-exit settlement', () => { it('settles a submission the dead child never acknowledged', async () => { const markPendingSubmissionsUnknown = vi.fn(async () => ['client-1']) - const session: StructuredAgentSessionUnexpectedExitSession = { + const session: StructuredAgentSessionChildExitSession = { child: { generation: GENERATION, fence: 7, phase: 'ready' }, journal: { cursor: () => ({ epoch: 'epoch-1', sequence: 0 }), @@ -358,7 +358,7 @@ describe('provider-exit settlement', () => { } const { store } = mutableStore() - const context: StructuredAgentSessionUnexpectedExitContext = { + const context: StructuredAgentSessionChildExitContext = { logger: recordingStructuredAgentSessionLogger().logger, store, sessions: new Map([[SESSION, session]]), @@ -367,7 +367,7 @@ describe('provider-exit settlement', () => { serialize: async (_sessionId: string, task: () => Promise) => task(), now: () => 1 } - await settleUnexpectedStructuredAgentSessionExit(context, { + await settleStructuredAgentSessionChildExit(context, { type: 'ended', sessionId: SESSION, reason: 'provider exited', @@ -397,7 +397,7 @@ describe('provider-exit settlement', () => { }) it('releases without offering a restart while terminal settlement is failing', async () => { - const session: StructuredAgentSessionUnexpectedExitSession = { + const session: StructuredAgentSessionChildExitSession = { child: { generation: GENERATION, fence: 7, phase: 'ready' }, journal: { cursor: () => ({ epoch: 'epoch-1', sequence: 0 }), @@ -423,7 +423,7 @@ describe('provider-exit settlement', () => { acquisitionGeneration: GENERATION } const { store } = mutableStore() - const context: StructuredAgentSessionUnexpectedExitContext = { + const context: StructuredAgentSessionChildExitContext = { store, sessions: new Map([[SESSION, session]]), flushLifecycle: async () => ({ ok: false, error: new Error('sink failed') }), @@ -432,7 +432,7 @@ describe('provider-exit settlement', () => { now: () => 1, logger: log.logger } - await settleUnexpectedStructuredAgentSessionExit(context, event) + await settleStructuredAgentSessionChildExit(context, event) expect(session.child).toBeNull() expect(publishFence).toHaveBeenCalledTimes(1) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts deleted file mode 100644 index 5a0b2f6de30..00000000000 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts +++ /dev/null @@ -1,198 +0,0 @@ -import type { AgentSessionFailureWordsContext } from '../../../shared/agent-session-failure-words' -import { PROVIDER_EXIT_ROW_PREFIX } from '../../../shared/agent-session-stop-row-identity' -import { structuredAgentSessionFailureWordsContext } from './structured-agent-session-send-preparation' -import type { AgentSessionJournal } from '../agent-session-journal/journal-store' -import type { StructuredAgentSessionEndedEvent } from './structured-agent-session-adapter' -import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' -import { endProviderChild } from './structured-agent-session-provider-child' -import { - releaseStoredStructuredAgentSessionOwnerAfterUnexpectedExit, - type StructuredAgentSessionLeaseStore -} from './structured-agent-session-lease-release' -import type { StructuredAgentSessionSinkBarrier } from './structured-agent-session-event-sink' -import { - captureUnfinishedStructuredAgentSessionWork, - MAX_UNEXPECTED_EXIT_REASON_CHARS, - settleStructuredAgentSessionDeadGeneration, - type DeadGenerationJournal, - unfinishedStructuredAgentSessionWorkWasInterrupted -} from './structured-agent-session-dead-generation-settlement' -import type { StructuredAgentSessionTurnVerdict } from './structured-agent-session-stale-turn-verdict' -import type { StructuredAgentSessionLogger } from './structured-agent-session-logger' - -type UnexpectedExitLifecycleEvent = StructuredAgentSessionEndedEvent & { - cause: 'unexpected-exit' -} - -export type StructuredAgentSessionUnexpectedExitSession = Pick< - StructuredAgentSessionHostSession, - 'child' | 'lastEndedChild' -> & { journal: DeadGenerationJournal & Pick } - -export type StructuredAgentSessionUnexpectedExitContext< - TSession extends StructuredAgentSessionUnexpectedExitSession = StructuredAgentSessionHostSession -> = { - store: StructuredAgentSessionLeaseStore - sessions: Map - flushLifecycle: (sessionId: string) => Promise - publishFence: (sessionId: string, session: TSession) => void - publishStatus?: (sessionId: string) => void - serialize: (sessionId: string, task: () => Promise) => Promise - now: () => number - logger: StructuredAgentSessionLogger -} - -export async function settleUnexpectedStructuredAgentSessionExit< - TSession extends StructuredAgentSessionUnexpectedExitSession ->( - context: StructuredAgentSessionUnexpectedExitContext, - event: StructuredAgentSessionEndedEvent -): Promise { - if (event.cause !== 'unexpected-exit') { - return - } - const unexpectedEvent = event as UnexpectedExitLifecycleEvent - // Receipt of the exit is the one end time the host may record for a running turn. - const observedAt = event.observedAt ?? context.now() - return context.serialize(unexpectedEvent.sessionId, async () => { - const session = context.sessions.get(unexpectedEvent.sessionId) - const child = session?.child - if ( - !session || - !child || - child.fence !== unexpectedEvent.fence || - child.generation !== unexpectedEvent.acquisitionGeneration - ) { - return - } - // The host's own phase decides, so a provider that omits the flag still gets a start that - // failed told as one: the row says so. - const exitedDuringStartup = - unexpectedEvent.startupUnproven === true || child.phase === 'starting' - const endChild = (): void => { - endProviderChild(session, { - generation: child.generation, - fence: child.fence, - cause: 'exit', - reason: unexpectedEvent.reason, - ...(unexpectedEvent.failure ? { failure: unexpectedEvent.failure } : {}), - duringStartup: exitedDuringStartup, - // The adapter publishes an exit only once it saw the root go, first-hand or proven. - rootGone: true - }) - context.publishStatus?.(unexpectedEvent.sessionId) - } - const record = context.store.getRecord(unexpectedEvent.sessionId) - if (!record || record.lease.handoffStage !== null) { - // An acquisition or recovery already owns this lease's transition. - endChild() - return - } - - const stableSettlementId = providerExitSettlementId(unexpectedEvent) - try { - // The exited child's own writes land first: its dead generation is settled from all of them. - try { - const barrier = await context.flushLifecycle(unexpectedEvent.sessionId) - if (!barrier.ok) { - logExitFailure(context, unexpectedEvent, 'exit-lifecycle-barrier', barrier.error) - } - } catch (error) { - logExitFailure(context, unexpectedEvent, 'exit-lifecycle-barrier', error) - } - const unfinishedWork = captureUnfinishedStructuredAgentSessionWork(session.journal) - await retryUnexpectedExitSettlement({ - context, - event: unexpectedEvent, - journal: session.journal, - fence: child.fence, - stableSettlementId, - verdict: { state: 'interrupted', completedAt: observedAt }, - exitedDuringStartup, - failureTextContext: structuredAgentSessionFailureWordsContext(record, session.journal), - // A failed start always says why: no response was running to carry the reason. - showUnexpectedExitOutcome: - exitedDuringStartup || - unfinishedStructuredAgentSessionWorkWasInterrupted( - unfinishedWork, - session.journal, - observedAt - ) - }) - } finally { - // Provider exit was positively observed, so release the owner even when - // terminal settlement could not be durably accepted. - let released: Awaited< - ReturnType - > | null = null - try { - released = await releaseStoredStructuredAgentSessionOwnerAfterUnexpectedExit({ - store: context.store, - sessionId: unexpectedEvent.sessionId, - expectedFence: unexpectedEvent.fence, - expectedAcquisitionGeneration: unexpectedEvent.acquisitionGeneration, - acquisitionGeneration: child.generation, - now: context.now(), - exitObservedAt: observedAt, - // Bare cause: whatever this settlement could not write is settled from it later (the - // settle recording it queues, or the next open or acquire); `exit-observed` says the rest. - exitReason: unexpectedEvent.reason.slice(0, MAX_UNEXPECTED_EXIT_REASON_CHARS) - }) - } catch (error) { - logExitFailure(context, unexpectedEvent, 'exit-owner-release', error) - } finally { - endChild() - if (released) { - context.publishFence(unexpectedEvent.sessionId, session) - } - } - } - }) -} - -async function retryUnexpectedExitSettlement(input: { - context: Pick - event: UnexpectedExitLifecycleEvent - journal: DeadGenerationJournal - fence: number - stableSettlementId: string - verdict: StructuredAgentSessionTurnVerdict - exitedDuringStartup: boolean - failureTextContext: AgentSessionFailureWordsContext - showUnexpectedExitOutcome?: boolean -}): Promise { - const settled = await settleStructuredAgentSessionDeadGeneration({ - journal: input.journal, - sessionId: input.event.sessionId, - fence: input.fence, - settlementId: input.stableSettlementId, - verdict: input.verdict, - pendingSubmissionReason: 'provider_exited_before_acknowledgement', - showUnexpectedExitOutcome: input.showUnexpectedExitOutcome, - ...(input.event.failure ? { exitFailure: input.event.failure } : {}), - failureTextContext: input.failureTextContext, - ...(input.exitedDuringStartup - ? { exitedDuringStartup: { generation: input.event.acquisitionGeneration } } - : {}) - }) - if (!settled.ok) { - logExitFailure(input.context, input.event, 'exit-settlement', settled.error) - } -} - -function logExitFailure( - context: Pick, - event: UnexpectedExitLifecycleEvent, - scope: string, - error: unknown -): void { - context.logger.warn('settling a provider exit did not finish', { - scope, - sessionId: event.sessionId, - error - }) -} - -function providerExitSettlementId(event: UnexpectedExitLifecycleEvent): string { - return `${PROVIDER_EXIT_ROW_PREFIX}${event.sessionId}:${event.fence}:${event.acquisitionGeneration}` -} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-wind-down-wait-row.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-wind-down-wait-row.ts deleted file mode 100644 index 5f1d03a7ead..00000000000 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-wind-down-wait-row.ts +++ /dev/null @@ -1,83 +0,0 @@ -// The row a message waiting on an unfinished stop leaves in the chat: why it has not gone out. - -import { agentSessionFailureFact } from '../../../shared/agent-session-failure' -import { - agentSessionFailureWords, - type AgentSessionFailureWordsContext -} from '../../../shared/agent-session-failure-words' -import { agentJournalItemKey } from '../../../shared/agent-session-journal-item-key' -import { isQueuedAgentJournalSubmission } from '../../../shared/agent-session-queued-submission' -import { - AGENT_JOURNAL_THREAD_SCOPE, - type AgentJournalItemIdentity -} from '../../../shared/agent-session-journal-types' -import type { - StructuredAgentSessionHostSession, - StructuredAgentSessionProviderChildIdentity -} from './structured-agent-session-host-types' -import { pendingProviderChildWindDown } from './structured-agent-session-provider-child' - -/** Keyed by the child the stop could not prove gone, so every send that waits on it shares a row. */ -export function structuredAgentSessionWindDownWaitIdentity( - owed: StructuredAgentSessionProviderChildIdentity -): AgentJournalItemIdentity { - return { - provider: 'orca', - clientMessageId: `wind-down-wait:${owed.fence}:${owed.generation ?? 'unknown'}` - } -} - -type WaitingSession = Pick< - StructuredAgentSessionHostSession, - 'journal' | 'child' | 'owesProviderChildWindDown' -> - -function windDownWaitRow( - session: WaitingSession, - owed: StructuredAgentSessionProviderChildIdentity -) { - const itemId = agentJournalItemKey(structuredAgentSessionWindDownWaitIdentity(owed)) - return session.journal.snapshot().items.find((item) => item.itemId === itemId) -} - -/** Every queued message already waited through a retry of the owed stop: it was accepted before the - * newest pass failed. Only a new message retries again, and a retry that lands wakes the loop - * itself, so no other commit, each of which wakes the delivery loop, retries. */ -export function structuredAgentSessionWindDownWaitHolds(session: WaitingSession): boolean { - const failedAt = pendingProviderChildWindDown(session)?.failedAt - if (!failedAt || failedAt.epoch !== session.journal.cursor().epoch) { - return false - } - return session.journal - .submissions() - .every( - (submission) => - !isQueuedAgentJournalSubmission(submission) || - (submission.acceptedSequence ?? 0) <= failedAt.sequence - ) -} - -/** Written once per child, so every message that waits on it shares the row. */ -export async function recordStructuredAgentSessionWindDownWait( - session: WaitingSession, - sessionId: string, - input: { - conversationFence: (sessionId: string) => number - failureTextContext: (sessionId: string) => AgentSessionFailureWordsContext - } -): Promise { - const owed = pendingProviderChildWindDown(session) - if (!owed || windDownWaitRow(session, owed)) { - return - } - const identity = structuredAgentSessionWindDownWaitIdentity(owed) - const words = agentSessionFailureWords(agentSessionFailureFact('previousExitUnverifiable'), { - ...input.failureTextContext(sessionId), - surface: 'row' - }) - await session.journal.appendItem( - identity, - { kind: 'status', tone: 'warning', ...words }, - { fence: input.conversationFence(sessionId), turnScope: AGENT_JOURNAL_THREAD_SCOPE } - ) -} diff --git a/src/main/runtime/agent-session-surface-release-transition.ts b/src/main/runtime/agent-session-surface-release-transition.ts index fe98a870524..1099e8d6c04 100644 --- a/src/main/runtime/agent-session-surface-release-transition.ts +++ b/src/main/runtime/agent-session-surface-release-transition.ts @@ -9,7 +9,10 @@ // holding the stopped owner's fence is refused as stale rather than acting on its successor. import { agentSessionRefusalError } from '../../shared/agent-session-wire-refusals' -import type { AgentSessionRecord } from '../../shared/agent-session-record' +import { + MAX_AGENT_SESSION_DEATH_DETAIL_CHARS, + type AgentSessionRecord +} from '../../shared/agent-session-record' import { nextAgentSessionFence } from '../../shared/agent-session-next-fence' import { assertFence, withLease } from './agent-session-lease-transitions' import type { AgentSessionRecordStore } from './agent-session-record-store' @@ -50,7 +53,10 @@ export function releaseAgentSessionOwnerAfterSurfaceClose(args: { lastRenewedAt: args.now, deathEvidence: { kind: 'exit-observed', - detail: args.exitReason ?? 'the last surface holding this session released it', + // A provider's exit reason can carry kilobytes of stderr; a longer detail fails the write. + detail: args.exitReason + ? args.exitReason.slice(0, MAX_AGENT_SESSION_DEATH_DETAIL_CHARS) + : 'the last surface holding this session released it', observedAt: args.exitObservedAt ?? args.now, ownerFence: record.lease.runtimeFence } diff --git a/src/main/runtime/structured-agent-session-lifecycle-delivery.test.ts b/src/main/runtime/structured-agent-session-lifecycle-delivery.test.ts new file mode 100644 index 00000000000..b236c81d594 --- /dev/null +++ b/src/main/runtime/structured-agent-session-lifecycle-delivery.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from 'vitest' +import type { StructuredAgentSessionLifecycleEvent } from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import { recordingStructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger-test-support' +import { createStructuredAgentSessionLifecycleDelivery } from './structured-agent-session-lifecycle-delivery' + +function ended( + sessionId: string, + cause: 'unexpected-exit' | 'requested-close' +): StructuredAgentSessionLifecycleEvent { + return { + type: 'ended', + sessionId, + reason: 'exited', + cause, + fence: 1, + acquisitionGeneration: 'generation-1' + } +} + +describe('provider exits reaching the host', () => { + // A close the host asked for ends on its own session's lane; another chat's crash recovery, + // which can run a whole reacquisition, must not hold it up. + it('ends a requested close without waiting behind another chat in recovery', async () => { + const handled: string[] = [] + let releaseRecovery = (): void => {} + const recovery = new Promise((resolve) => { + releaseRecovery = resolve + }) + const delivery = createStructuredAgentSessionLifecycleDelivery({ + handle: async (event) => { + if (event.type === 'ended' && event.cause === 'unexpected-exit') { + await recovery + } + handled.push(event.sessionId) + }, + logger: recordingStructuredAgentSessionLogger().logger, + drainObservedExits: async () => {} + }) + + delivery.deliver(ended('crashed', 'unexpected-exit')) + delivery.deliver(ended('closed', 'requested-close')) + await new Promise((resolve) => setImmediate(resolve)) + + expect(handled).toEqual(['closed']) + releaseRecovery() + await delivery.drain() + expect(handled).toEqual(['closed', 'crashed']) + }) +}) diff --git a/src/main/runtime/structured-agent-session-lifecycle-delivery.ts b/src/main/runtime/structured-agent-session-lifecycle-delivery.ts index ccf026de359..ba307c2cfb0 100644 --- a/src/main/runtime/structured-agent-session-lifecycle-delivery.ts +++ b/src/main/runtime/structured-agent-session-lifecycle-delivery.ts @@ -3,8 +3,9 @@ // Exit recovery runs on one chain so teardown can drain it: exit callbacks arrive from child // process tasks, and a fire-and-forget one could otherwise append after the host flushed and // removed its journal directory. That chain orders nothing across sessions, and a recovery on it -// can run a whole reacquisition, so `started` stays off it: it takes only its own session's -// serialized step, and is tracked here so the same drain still waits for it. +// can run a whole reacquisition, so `started` and the end of a close the host asked for stay off +// it: each takes only its own session's serialized step, and is tracked here so the same drain +// still waits for it. import type { StructuredAgentSessionLifecycleEvent } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import type { StructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' @@ -34,8 +35,8 @@ export function createStructuredAgentSessionLifecycleDelivery(input: { } return { deliver: (event) => { - if (event.type === 'started') { - // Called now, so the step is queued on its session ahead of any later exit of that child. + if (event.type === 'started' || event.cause === 'requested-close') { + // Called now, so the step is queued on its own session's lane, behind nothing of another's. const settling = settle(event) settlingStarts.add(settling) void settling.finally(() => settlingStarts.delete(settling)) diff --git a/src/main/runtime/structured-agent-session-runtime-teardown.test.ts b/src/main/runtime/structured-agent-session-runtime-teardown.test.ts new file mode 100644 index 00000000000..767d0716c44 --- /dev/null +++ b/src/main/runtime/structured-agent-session-runtime-teardown.test.ts @@ -0,0 +1,27 @@ +import { expect, it } from 'vitest' +import { tearDownRuntime, type InstalledRuntime } from './structured-agent-session-runtime-teardown' + +// An exit that settles while teardown drains recovery wakes delivery; a fresh child started then +// would only be killed by the teardown after it. +it('stops delivery before it drains the recovery of exits already observed', async () => { + const order: string[] = [] + const installed: InstalledRuntime = { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: teardown calls only `stopDelivery` and `flushAllStreamedEvents` on the host. + host: { + stopDelivery: () => order.push('stop-delivery'), + flushAllStreamedEvents: async () => { + order.push('flush') + } + } as never, + adapter: { closeAll: async () => undefined }, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: teardown calls only `close` on the database. + journalDatabase: { close: () => order.push('close-database') } as never, + waitForRecovery: async () => { + order.push('wait-for-recovery') + } + } + + await tearDownRuntime(installed, 'quit') + + expect(order.slice(0, 2)).toEqual(['stop-delivery', 'wait-for-recovery']) +}) diff --git a/src/main/runtime/structured-agent-session-runtime-teardown.ts b/src/main/runtime/structured-agent-session-runtime-teardown.ts index 47d93e02830..3275939a4fa 100644 --- a/src/main/runtime/structured-agent-session-runtime-teardown.ts +++ b/src/main/runtime/structured-agent-session-runtime-teardown.ts @@ -35,6 +35,9 @@ export async function tearDownRuntime( installed: InstalledRuntime, trigger: AgentSessionResumeTrigger ): Promise { + // An exit settled while recovery drains wakes delivery, which would start a fresh child for + // teardown to kill; queued messages wait for the next launch instead. + installed.host.stopDelivery() // Drain an in-flight recovery before stopping children; recovery may still // be writing lifecycle rows or acquiring a replacement child. await installed.waitForRecovery() diff --git a/src/main/runtime/structured-agent-session-runtime.ts b/src/main/runtime/structured-agent-session-runtime.ts index 3785fdeaf5c..b5efd11efad 100644 --- a/src/main/runtime/structured-agent-session-runtime.ts +++ b/src/main/runtime/structured-agent-session-runtime.ts @@ -270,8 +270,10 @@ async function installOnJournal( host?.publishChildWorkEvidence(sessionId, evidence), onDispatchSettledLate, onPrimaryThreadStoppedRunning: releaseUnansweredDispatches, + logger: deps.logger, onEvent: (event) => { - if (event.type === 'ended' && 'cause' in event && event.cause === 'unexpected-exit') { + // Every exit, expected or not: the host ends that child's record. + if (event.type === 'ended' && 'cause' in event) { lifecycle.deliver(event) } } @@ -293,6 +295,7 @@ async function installOnJournal( } : {}), onLifecycleEvent: (event) => lifecycle.deliver(event), + logger: deps.logger, onChildWorkEvidence: (sessionId, evidence) => host?.publishChildWorkEvidence(sessionId, evidence), onDispatchSettledLate, diff --git a/src/main/runtime/structured-claude-runtime-adapter.ts b/src/main/runtime/structured-claude-runtime-adapter.ts index 9024c65b503..732515cdde6 100644 --- a/src/main/runtime/structured-claude-runtime-adapter.ts +++ b/src/main/runtime/structured-claude-runtime-adapter.ts @@ -38,6 +38,7 @@ export type StructuredClaudeRuntimeAdapterDeps = { onDispatchSettledLate?: ClaudeStructuredSessionAdapterDeps['onDispatchSettledLate'] onSessionIdle?: ClaudeStructuredSessionAdapterDeps['onSessionIdle'] onChildWorkEvidence?: ClaudeStructuredSessionAdapterDeps['onChildWorkEvidence'] + logger?: ClaudeStructuredSessionAdapterDeps['logger'] } /** The adapter events the host's lifecycle handler consumes, in the host's vocabulary. */ @@ -47,9 +48,10 @@ export function structuredClaudeLifecycleEvent( if (event.type === 'started') { return event } + // Every exit of a child with an identity, expected or not: the host ends that child's record. if ( event.type === 'ended' && - event.cause === 'unexpected-exit' && + event.cause !== undefined && event.fence !== undefined && event.acquisitionGeneration ) { @@ -131,6 +133,7 @@ export function createStructuredClaudeRuntimeAdapter( ...(deps.onDispatchSettledLate ? { onDispatchSettledLate: deps.onDispatchSettledLate } : {}), ...(deps.onSessionIdle ? { onSessionIdle: deps.onSessionIdle } : {}), ...(deps.onChildWorkEvidence ? { onChildWorkEvidence: deps.onChildWorkEvidence } : {}), + ...(deps.logger ? { logger: deps.logger } : {}), ...(deps.openClaudeConnection ? { openConnection: deps.openClaudeConnection } : {}), ...(deps.readProcessStartTime ? { readProcessStartTime: deps.readProcessStartTime } : {}), ...(deps.modelCatalog ? { modelCatalog: deps.modelCatalog } : {}) diff --git a/src/renderer/src/components/native-chat/agent-session-failure-words-text.ts b/src/renderer/src/components/native-chat/agent-session-failure-words-text.ts index 34dab4ab94f..59c4947c8b4 100644 --- a/src/renderer/src/components/native-chat/agent-session-failure-words-text.ts +++ b/src/renderer/src/components/native-chat/agent-session-failure-words-text.ts @@ -27,6 +27,11 @@ const PIECES: Record translate('components.native-chat.failureWords.sendToTryAgain', COPY.sendToTryAgain), + sendAgainToTryOnceMore: () => + translate( + 'components.native-chat.failureWords.sendAgainToTryOnceMore', + COPY.sendAgainToTryOnceMore + ), couldNotStart: (values) => translate('components.native-chat.failureWords.couldNotStart', COPY.couldNotStart, values), couldNotRestart: (values) => diff --git a/src/renderer/src/i18n/en-runtime-required.json b/src/renderer/src/i18n/en-runtime-required.json index 3cc1ed29e59..ff122b73905 100644 --- a/src/renderer/src/i18n/en-runtime-required.json +++ b/src/renderer/src/i18n/en-runtime-required.json @@ -2785,7 +2785,7 @@ "notDelivered": "This message was not delivered.", "notDeliveredSendAgain": "This message was not delivered. Send it again to continue.", "notSignedIn": "{{agent}} is not signed in for the selected account.", - "previousExitUnverifiable": "{{agent}} from before may still be running. Your messages will send once it stops.", + "previousExitUnverifiable": "Couldn't stop {{agent}} from before.", "providerExitedRejection": "{{agent}} stopped before this message was sent.", "providerExitedRow": "{{agent}} stopped while this response was in progress. You can continue in this conversation.", "providerRateLimited": "{{agent}} is rate-limited and retrying.", @@ -2797,6 +2797,7 @@ "queueFull": "Too many messages were waiting for the agent, so this one was not sent.", "runCommandAgain": "Run /{{command}} again.", "sendToTryAgain": "Send your message to try again.", + "sendAgainToTryOnceMore": "Send your message again to try once more.", "signInFirst": "Sign in first.", "signInThenRunCommand": "Sign in, then run /{{command}} again.", "signInThenSend": "Sign in, then send your message again.", diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 25087a71ea2..40c50ff3b5c 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -17706,6 +17706,7 @@ "providerStartFailed": "{{agent}} stopped before it finished starting.", "runCommandAgain": "Run /{{command}} again.", "sendToTryAgain": "Send your message to try again.", + "sendAgainToTryOnceMore": "Send your message again to try once more.", "couldNotStart": "{{agent}} couldn't start.", "couldNotRestart": "{{agent}} couldn't restart.", "notSignedIn": "{{agent}} is not signed in for the selected account.", @@ -17759,7 +17760,7 @@ "providerRateLimited": "{{agent}} is rate-limited and retrying.", "providerRetrying": "{{agent}} hit a temporary problem and is retrying.", "providerRetryingQuoted": "{{agent}} is retrying: {{detail}}.", - "previousExitUnverifiable": "{{agent}} from before may still be running. Your messages will send once it stops." + "previousExitUnverifiable": "Couldn't stop {{agent}} from before." }, "writeNotice": { "notDoneReadHistory": "This chat's history couldn't be loaded.", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index ec2d25bbb34..0d836852f59 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -17484,6 +17484,7 @@ "providerStartFailed": "{{agent}} se detuvo antes de terminar de iniciarse.", "runCommandAgain": "Vuelva a ejecutar /{{command}}.", "sendToTryAgain": "Envíe su mensaje para volver a intentarlo.", + "sendAgainToTryOnceMore": "Vuelva a enviar su mensaje para intentarlo de nuevo.", "couldNotStart": "{{agent}} no pudo iniciarse.", "couldNotRestart": "{{agent}} no pudo reiniciarse.", "notSignedIn": "{{agent}} no tiene la sesión iniciada con la cuenta seleccionada.", @@ -17537,7 +17538,7 @@ "providerRateLimited": "{{agent}} alcanzó un límite de solicitudes y está reintentando.", "providerRetrying": "{{agent}} tuvo un problema temporal y está reintentando.", "providerRetryingQuoted": "{{agent}} está reintentando: {{detail}}.", - "previousExitUnverifiable": "Es posible que {{agent}} siga en ejecución desde antes. Sus mensajes se enviarán cuando se detenga." + "previousExitUnverifiable": "No se pudo detener la ejecución anterior de {{agent}}." }, "writeNotice": { "notDoneReadHistory": "No se pudo cargar el historial de este chat.", diff --git a/src/renderer/src/i18n/locales/fr.json b/src/renderer/src/i18n/locales/fr.json index 333d821d346..0b7490532bb 100644 --- a/src/renderer/src/i18n/locales/fr.json +++ b/src/renderer/src/i18n/locales/fr.json @@ -17583,6 +17583,7 @@ "providerStartFailed": "{{agent}} s'est arrêté avant d'avoir fini de démarrer.", "runCommandAgain": "Relancez /{{command}}.", "sendToTryAgain": "Envoyez votre message pour réessayer.", + "sendAgainToTryOnceMore": "Renvoyez votre message pour réessayer.", "couldNotStart": "{{agent}} n'a pas pu démarrer.", "couldNotRestart": "{{agent}} n'a pas pu redémarrer.", "notSignedIn": "{{agent}} n'est pas connecté avec le compte sélectionné.", @@ -17636,7 +17637,7 @@ "providerRateLimited": "{{agent}} est limité en débit et réessaie.", "providerRetrying": "{{agent}} a rencontré un problème temporaire et réessaie.", "providerRetryingQuoted": "{{agent}} réessaie : {{detail}}.", - "previousExitUnverifiable": "L'exécution précédente de {{agent}} est peut-être toujours en cours. Vos messages seront envoyés dès qu'elle s'arrêtera." + "previousExitUnverifiable": "Impossible d'arrêter l'exécution précédente de {{agent}}." }, "writeNotice": { "notDoneReadHistory": "L'historique de ce chat n'a pas pu être chargé.", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 5d1678b46d0..f83c99f8972 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -17519,6 +17519,7 @@ "providerStartFailed": "{{agent}} は起動が完了する前に停止しました。", "runCommandAgain": "/{{command}} をもう一度実行してください。", "sendToTryAgain": "もう一度試すには、メッセージを送信してください。", + "sendAgainToTryOnceMore": "メッセージをもう一度送信して、再度お試しください。", "couldNotStart": "{{agent}} を起動できませんでした。", "couldNotRestart": "{{agent}} を再起動できませんでした。", "notSignedIn": "{{agent}} は選択したアカウントでサインインしていません。", @@ -17572,7 +17573,7 @@ "providerRateLimited": "{{agent}} はレート制限を受けているため、再試行しています。", "providerRetrying": "{{agent}} で一時的な問題が発生したため、再試行しています。", "providerRetryingQuoted": "{{agent}} は再試行しています: {{detail}}。", - "previousExitUnverifiable": "以前の {{agent}} がまだ実行中の可能性があります。停止するとメッセージが送信されます。" + "previousExitUnverifiable": "以前の {{agent}} を停止できませんでした。" }, "writeNotice": { "notDoneReadHistory": "このチャットの履歴を読み込めませんでした。", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 5cd0371e7be..27247862e4b 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -17519,6 +17519,7 @@ "providerStartFailed": "{{agent}}이(가) 시작을 마치기 전에 중지되었습니다.", "runCommandAgain": "/{{command}}을(를) 다시 실행하세요.", "sendToTryAgain": "다시 시도하려면 메시지를 보내세요.", + "sendAgainToTryOnceMore": "메시지를 다시 보내 한 번 더 시도하세요.", "couldNotStart": "{{agent}}을(를) 시작하지 못했습니다.", "couldNotRestart": "{{agent}}을(를) 다시 시작하지 못했습니다.", "notSignedIn": "{{agent}}이(가) 선택한 계정으로 로그인되어 있지 않습니다.", @@ -17572,7 +17573,7 @@ "providerRateLimited": "{{agent}}이(가) 속도 제한에 걸려 다시 시도하고 있습니다.", "providerRetrying": "{{agent}}에 일시적인 문제가 발생하여 다시 시도하고 있습니다.", "providerRetryingQuoted": "{{agent}}이(가) 다시 시도하고 있습니다: {{detail}}.", - "previousExitUnverifiable": "이전에 실행된 {{agent}}이(가) 아직 실행 중일 수 있습니다. 중지되면 메시지가 전송됩니다." + "previousExitUnverifiable": "이전에 실행된 {{agent}}을(를) 중지하지 못했습니다." }, "writeNotice": { "notDoneReadHistory": "이 채팅의 기록을 불러오지 못했습니다.", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 3ce2dbdb7ba..63b7188d2a1 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -17484,6 +17484,7 @@ "providerStartFailed": "{{agent}} 在完成启动前已停止。", "runCommandAgain": "请重新运行 /{{command}}。", "sendToTryAgain": "请发送您的消息以重试。", + "sendAgainToTryOnceMore": "请重新发送您的消息，再试一次。", "couldNotStart": "{{agent}} 无法启动。", "couldNotRestart": "{{agent}} 无法重新启动。", "notSignedIn": "{{agent}} 未使用所选账户登录。", @@ -17537,7 +17538,7 @@ "providerRateLimited": "{{agent}} 已被限流，正在重试。", "providerRetrying": "{{agent}} 遇到临时问题，正在重试。", "providerRetryingQuoted": "{{agent}} 正在重试：{{detail}}。", - "previousExitUnverifiable": "之前的 {{agent}} 可能仍在运行。它停止后，您的消息就会发送。" + "previousExitUnverifiable": "无法停止之前的 {{agent}}。" }, "writeNotice": { "notDoneReadHistory": "无法加载此聊天的历史记录。", diff --git a/src/shared/agent-session-failure-copy.ts b/src/shared/agent-session-failure-copy.ts index 1f3a58f3bad..1e555c676d6 100644 --- a/src/shared/agent-session-failure-copy.ts +++ b/src/shared/agent-session-failure-copy.ts @@ -14,6 +14,7 @@ export const AGENT_SESSION_FAILURE_COPY = { providerStartFailed: '{{agent}} stopped before it finished starting.', runCommandAgain: 'Run /{{command}} again.', sendToTryAgain: 'Send your message to try again.', + sendAgainToTryOnceMore: 'Send your message again to try once more.', couldNotStart: "{{agent}} couldn't start.", couldNotRestart: "{{agent}} couldn't restart.", terminalAgentHoldsChat: TERMINAL_AGENT_HOLDS_CHAT, @@ -81,8 +82,7 @@ export const AGENT_SESSION_FAILURE_COPY = { providerRateLimited: '{{agent}} is rate-limited and retrying.', providerRetrying: '{{agent}} hit a temporary problem and is retrying.', providerRetryingQuoted: '{{agent}} is retrying: {{detail}}.', - previousExitUnverifiable: - '{{agent}} from before may still be running. Your messages will send once it stops.' + previousExitUnverifiable: "Couldn't stop {{agent}} from before." } as const export type AgentSessionFailureCopyId = keyof typeof AGENT_SESSION_FAILURE_COPY diff --git a/src/shared/agent-session-failure-words.test.ts b/src/shared/agent-session-failure-words.test.ts index 98273628214..4e6b81b265a 100644 --- a/src/shared/agent-session-failure-words.test.ts +++ b/src/shared/agent-session-failure-words.test.ts @@ -151,6 +151,28 @@ describe('the words written beside a failure fact', () => { ).toBe("Codex couldn't start. Start a new chat to continue.") }) + it('says a start refused beside a process Orca could not stop in its own words', () => { + const refused = (context: { command?: 'compact'; retryControl?: boolean } = {}) => + agentSessionFailureSentence( + { + kind: 'restartFailed', + refusal: { + code: 'agent_session_ownership_unknown', + details: { reason: 'previousExitUnverifiable' } + } + }, + 'rejection', + { agentName: 'Claude', ...context } + ) + expect(refused()).toBe( + "Couldn't stop Claude from before. Send your message again to try once more." + ) + expect(refused({ command: 'compact' })).toBe( + "Couldn't stop Claude from before. Run /compact again." + ) + expect(refused({ retryControl: true })).toBe("Couldn't stop Claude from before.") + }) + it('names the exit a row reports differently from the message it left unsent', () => { expect( agentSessionFailureWords({ kind: 'providerExited' }, { surface: 'row', agentName: 'Claude' }) diff --git a/src/shared/agent-session-failure-words.ts b/src/shared/agent-session-failure-words.ts index f64ba5cb335..74e6e543b53 100644 --- a/src/shared/agent-session-failure-words.ts +++ b/src/shared/agent-session-failure-words.ts @@ -130,12 +130,13 @@ function withRetryCause(sentence: string, cause: string | undefined): string { /** The next step after a start or restart that failed: the command, or the message, again. */ function startRetry( say: AgentSessionFailureSay, - { command, retryControl }: AgentSessionFailureWordsContext + { command, retryControl }: AgentSessionFailureWordsContext, + sendAgain: 'sendToTryAgain' | 'sendAgainToTryOnceMore' = 'sendToTryAgain' ): string[] { if (retryControl) { return [] } - return [command ? say('runCommandAgain', { command }) : say('sendToTryAgain')] + return [command ? say('runCommandAgain', { command }) : say(sendAgain)] } function couldNot(verb: 'couldNotStart' | 'couldNotRestart'): Sentence { @@ -145,6 +146,13 @@ function couldNot(verb: 'couldNotStart' | 'couldNotRestart'): Sentence { if (fact.refusal?.details?.reason === 'claimConflicted') { return joinSentences([failed, say('terminalAgentHoldsChat'), say('quitTerminalAgent')]) } + // The previous process may still run, so nothing started: that, never that it exited. + if (fact.refusal?.details?.reason === 'previousExitUnverifiable') { + return joinSentences([ + say('previousExitUnverifiable', agent(say, context)), + ...startRetry(say, context, 'sendAgainToTryOnceMore') + ]) + } const code = fact.refusal?.code return joinSentences( code && !START_REFUSAL_RESUMABLE[code] diff --git a/src/shared/agent-session-failure.ts b/src/shared/agent-session-failure.ts index b08c5250a15..a3c97850a1e 100644 --- a/src/shared/agent-session-failure.ts +++ b/src/shared/agent-session-failure.ts @@ -48,7 +48,7 @@ export const AGENT_SESSION_FAILURE_KINDS = [ 'hostStopped', /** The provider is retrying a request its API refused; not a failure yet. */ 'providerRetrying', - /** A message waits on a child a Stop could not prove gone: its exit is unverifiable. */ + /** A child a Stop could not prove gone: its exit is unverifiable. Kept for rows hosts wrote. */ 'previousExitUnverifiable' ] as const export type AgentSessionFailureKind = (typeof AGENT_SESSION_FAILURE_KINDS)[number] diff --git a/src/shared/agent-session-record.ts b/src/shared/agent-session-record.ts index b8280cf332d..a9d563392f0 100644 --- a/src/shared/agent-session-record.ts +++ b/src/shared/agent-session-record.ts @@ -157,6 +157,8 @@ export type AgentSessionOptionsReplacement = { } const MAX_ID_LENGTH = 512 +/** A death evidence's `detail` past this fails a load, so whoever writes one cuts it here. */ +export const MAX_AGENT_SESSION_DEATH_DETAIL_CHARS = MAX_ID_LENGTH const MAX_PATH_LENGTH = 4096 const MAX_LAUNCH_ENV_ENTRIES = 256 const MAX_LAUNCH_ENV_VALUE_LENGTH = 65_536 @@ -290,7 +292,7 @@ function isAgentSessionDeathEvidence(value: unknown): value is AgentSessionDeath (evidence.kind === 'exit-observed' || evidence.kind === 'pid-absent' || evidence.kind === 'identity-mismatch') && - isBoundedString(evidence.detail, MAX_ID_LENGTH) && + isBoundedString(evidence.detail, MAX_AGENT_SESSION_DEATH_DETAIL_CHARS) && typeof observedAt === 'number' && Number.isSafeInteger(observedAt) && observedAt >= 0 && diff --git a/src/shared/agent-session-refusal-details.ts b/src/shared/agent-session-refusal-details.ts index 70c6121d03f..b56ca5fbeaa 100644 --- a/src/shared/agent-session-refusal-details.ts +++ b/src/shared/agent-session-refusal-details.ts @@ -67,8 +67,8 @@ export const AGENT_SESSION_REFUSAL_REASONS = { 'notResumable', 'noProviderChild', 'conversationHeldElsewhere', - /** A Stop could not prove its child gone, and a retry could not either: that child takes no - * input and none starts beside it. Sent with `ownerVerdict: 'unverifiable'`. */ + /** The close a stop began could not prove its child gone: that child takes no input and none + * starts beside it. Sent with `ownerVerdict: 'unverifiable'`. */ 'previousExitUnverifiable' ], agent_session_conflict: [ diff --git a/src/shared/agent-session-stop-row-identity.ts b/src/shared/agent-session-stop-row-identity.ts index d314003a3f9..8d9fac96f11 100644 --- a/src/shared/agent-session-stop-row-identity.ts +++ b/src/shared/agent-session-stop-row-identity.ts @@ -4,7 +4,7 @@ // Contract: a new row that explains a stop must carry the providerExited fact or one of these // prefixes, or a client derives a second notice beside it. -/** The exit row the unexpected-exit settle writes, keyed by the child that exited. */ +/** The exit row the child-exit settle writes for an unexpected exit, keyed by the child that exited. */ export const PROVIDER_EXIT_ROW_PREFIX = 'provider-exit:' /** The exit row a reopen or acquire writes for a generation found dead. */ export const STALE_SESSION_ROW_PREFIX = 'stale-session:'