From 062169016d23a53706689d87817c6a44578e62a9 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 7 Oct 2026 20:22:03 -0700 Subject: [PATCH 1/5] feat(native-chat): host-owned startup attempt and hold-and-deliver barrier The host now mints one startup attempt before any adapter acquires (lease identity, spawn token, pinned launch context, cancellation, a 60 s deadline from the mint that activity never renews, option intent, and the event and child-work sinks), and the shared delivery loop hands nothing to a child still proving its start. A send is still accepted and journaled at once; readiness (the normalized `started` settlement) wakes delivery, which re-checks the child, fence, close and Stop before handover. A start that fails, crashes, or misses its deadline leaves held messages definitely unsent; a dispatch that throws after handover is `unknown`, never inferred unsent from the child's phase. --- ...-agent-session-accept-then-deliver.test.ts | 23 +- ...structured-agent-session-acquire-aborts.ts | 8 +- .../structured-agent-session-acquisition.ts | 15 +- .../structured-agent-session-adapter.ts | 38 +- .../structured-agent-session-agent-start.ts | 15 +- .../structured-agent-session-attach-flow.ts | 27 +- ...ured-agent-session-attach-orchestration.ts | 33 +- ...ent-session-claude-hung-start-stop.test.ts | 41 +- .../structured-agent-session-command-turn.ts | 23 +- ...d-agent-session-conversation-close.test.ts | 12 +- ...red-agent-session-conversation-lifetime.ts | 27 ++ ...ctured-agent-session-delivery-loop.test.ts | 2 +- .../structured-agent-session-delivery-loop.ts | 47 +-- ...tured-agent-session-ended-child-failure.ts | 41 ++ ...ctured-agent-session-host-runtime-state.ts | 23 +- .../structured-agent-session-host-types.ts | 2 + .../structured-agent-session-host.ts | 7 +- ...ctured-agent-session-mutation-admission.ts | 1 - ...ructured-agent-session-mutation-context.ts | 1 - ...red-agent-session-mutation-turn-context.ts | 1 - ...ed-agent-session-owed-work-release.test.ts | 30 +- ...gent-session-provider-child-record.test.ts | 56 +-- ...d-agent-session-provider-history-window.ts | 28 ++ ...ructured-agent-session-provider-started.ts | 25 +- ...session-queued-message-rig.test-fixture.ts | 31 +- ...session-send-restarts-failed-start.test.ts | 58 ++- ...agent-session-start-failure-writer.test.ts | 8 +- ...agent-session-startup-attempt-host.test.ts | 354 ++++++++++++++++++ ...sion-startup-attempt-registrations.test.ts | 192 ++++++++++ ...ured-agent-session-startup-attempt.test.ts | 127 +++++++ ...tructured-agent-session-startup-attempt.ts | 190 ++++++++++ ...d-agent-session-stop-event-binding.test.ts | 3 +- ...tructured-agent-session-stop-event.test.ts | 9 +- .../structured-agent-session-turns.ts | 20 +- ...ured-agent-session-unrun-send-hold.test.ts | 37 +- ...ude-structured-send-during-startup.test.ts | 40 +- ...-send-restart-dies-before-dispatch.test.ts | 9 +- ...tured-startup-fault-is-not-an-exit.test.ts | 8 +- 38 files changed, 1337 insertions(+), 275 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-ended-child-failure.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-provider-history-window.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts 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 263de102f532..24526b1ee9ca 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 @@ -53,6 +53,8 @@ import { NO_STRUCTURED_AGENTS } from './structured-agent-session-adapter-router- const CALLER = { callerKey: 'client-1' } +const childPhase = () => host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase + function eventually(assertion: () => void | Promise): Promise { return vi.waitFor(assertion, { timeout: 10_000 }) } @@ -624,11 +626,9 @@ describe('a child that exits before its message is handed over', () => { }) }) -describe('a start that fails after it was handed messages', () => { +describe('a start that fails while messages wait on it', () => { it('keeps one row in the words its rejected messages carry', async () => { - // A starting child takes the first at once; the second waits until that turn opens, which it - // never does. Neither is answered. - dispatch.mockImplementation(async () => ({ state: 'admitted' as const })) + // Both are held for a starting child that never proves its start. acquire.mockImplementation(async (input) => ({ ...(await spawnChild(input)), providerChildPhase: 'starting' as const @@ -638,7 +638,7 @@ describe('a start that fails after it was handed messages', () => { const first = await accept('first') const second = await accept('second') - await eventually(() => expect(dispatch).toHaveBeenCalledTimes(1)) + await eventually(() => expect(childPhase()).toBe('starting')) await host.handleAdapterEvent({ type: 'ended', sessionId: SESSION, @@ -653,7 +653,7 @@ describe('a start that fails after it was handed messages', () => { }) await host.flushStreamedEvents(SESSION) - // The held message is rejected by the delivery loop's next step, after the start's own. + // Both are rejected by the delivery loop's next step, never having been handed over. await eventually(async () => expect((await submission(second))?.dispatchState).toBe('rejected')) const rows = (await host.journalSnapshot(SESSION)).items.filter((item) => item.itemId.includes('start-failure') @@ -671,6 +671,7 @@ describe('a start that fails after it was handed messages', () => { rejection: row.kind === 'status' ? row.failure : null }) } + expect(dispatch).not.toHaveBeenCalled() }) }) @@ -724,7 +725,7 @@ describe('Stop withdraws what is queued', () => { expect(dispatch).not.toHaveBeenCalled() }) - it('stops a child still proving its start, rejecting what it was handed, and the next send starts another (W17c)', async () => { + it('stops a child still proving its start, withdrawing what was held for it, and the next send starts another (W17c)', async () => { const closeSession = vi.fn(async () => true) adapterExtras = { closeSession } await host.close(SESSION, 'evict') @@ -733,10 +734,10 @@ describe('Stop withdraws what is queued', () => { ...(await spawnChild(input)), providerChildPhase: 'starting' as const })) - // Written to the starting child at once; it never answers. - dispatch.mockResolvedValueOnce({ state: 'admitted' }) + // Held for the starting child, which never proves its start. const id = await accept('hello') - await eventually(async () => expect((await submission(id))?.handedOverAt).toBeDefined()) + await eventually(() => expect(childPhase()).toBe('starting')) + expect((await submission(id))?.handedOverAt).toBeUndefined() expect(await stop()).toMatchObject({ ok: true, value: { cancelled: true } }) @@ -747,7 +748,7 @@ describe('Stop withdraws what is queued', () => { expect(closeSession).toHaveBeenCalled() const next = await accept('after stop') await eventually(async () => expect((await submission(next))?.dispatchState).toBe('accepted')) - expect(dispatch).toHaveBeenCalledTimes(2) + expect(dispatch).toHaveBeenCalledTimes(1) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-acquire-aborts.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-acquire-aborts.ts index a3eabd09f0e5..80fb60851324 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-acquire-aborts.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-acquire-aborts.ts @@ -1,7 +1,7 @@ // The acquire each session has in flight, owned by the host that runs it, or another provider wait // its queue is on (an option write). The queue runs one step at a time, so a session has at most -// one; a close, a Stop admitted now, or quit aborts it from outside the queue instead of waiting -// behind a provider that may never answer. +// one; a close, a Stop admitted now, quit, or the start's deadline aborts it from outside the queue +// instead of waiting behind a provider that may never answer. export class StructuredAgentSessionAcquireAborts { private readonly inFlight = new Map() @@ -26,8 +26,8 @@ export class StructuredAgentSessionAcquireAborts { } /** A no-op when the session has nothing in flight. */ - abort(sessionId: string, reason: string): void { - this.inFlight.get(sessionId)?.abort(new Error(reason)) + abort(sessionId: string, reason: string | Error): void { + this.inFlight.get(sessionId)?.abort(typeof reason === 'string' ? new Error(reason) : reason) } /** Quit: every start under way stops, and so does any the attach drain still runs. */ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts index 70019d6da3e5..159c99d2cc04 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts @@ -13,6 +13,7 @@ import { journalIdentityFor } from './structured-agent-session-attach' import type { AttachFlowInput } from './structured-agent-session-attach-flow' import { readNativeSessionOptions } from './structured-agent-session-option-restoration' import { withAgentSessionCreatePhase } from '../../observability/agent-session-instrumentation' +import { mintStructuredAgentSessionStartupAttempt } from './structured-agent-session-startup-attempt' /** The same process, whatever Orca runtime the store stamped on its record (`runtime`): that stamp * is about who holds the process, not which process it is. */ @@ -48,15 +49,21 @@ export async function acquireOwner( } catch (error) { throw new AgentSessionPreSpawnError(error) } - const acquired = await input.adapter.acquire({ + // Minted before the adapter runs anything, so the deadline bounds the whole start. + const attempt = mintStructuredAgentSessionStartupAttempt({ + record, identity: journalIdentityFor(record, input.params), - fence, // Retries must recover the original reservation, not mint a second child. spawnToken, - ...(record.options ? { options: record.options } : {}), ...(input.eventSink ? { events: input.eventSink } : {}), - ...(input.recordPhase ? { recordPhase: input.recordPhase } : {}), ...(input.acquireSignal ? { signal: input.acquireSignal } : {}), + now: input.now(), + ...(input.startupDeadlineMs === undefined ? {} : { deadlineMs: input.startupDeadlineMs }) + }) + input.onStartupAttempt?.(attempt) + const acquired = await input.adapter.acquire({ + ...attempt, + ...(input.recordPhase ? { recordPhase: input.recordPhase } : {}), onSpawned: async (process) => { record = await input.store.commitProcessIdentity({ sessionId: record.sessionId, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts index fb26aeeb7562..e257e5984f01 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts @@ -46,7 +46,7 @@ export type { import type { AgentJournalDispatchRejection } from '../../../shared/agent-session-failure-words' import type { AgentSessionPromptResponse } from '../../../shared/agent-session-question-answer' import type { ProviderHistoryWindow } from '../agent-session-journal/journal-submission-reconciler' -import type { StructuredAgentSessionEventSink } from './structured-agent-session-event-sink' +import type { StructuredAgentSessionStartupAttempt } from './structured-agent-session-startup-attempt' import type { AgentSessionCreatePhaseRecorder } from '../../observability/agent-session-instrumentation' export class AgentSessionAcquisitionRefusal extends Error { @@ -122,7 +122,9 @@ export type AgentSessionAcquisition = { /** Host-local identity for this exact provider child, distinct even when the durable fence is * reused by a superseding acquisition. */ acquisitionGeneration?: string - /** Absent means `ready`: the adapter proved startup before answering. */ + /** `starting`: published at spawn, proven later by a `started` event. Absent means `ready`, an + * acquire that ran the whole handshake before answering; that bridge ends once every adapter + * publishes at spawn. Either way the host holds input until the child is `ready`. */ providerChildPhase?: StructuredAgentSessionProviderChildPhase } @@ -246,26 +248,22 @@ export type StructuredAgentSessionLifecycleEvent = | StructuredAgentSessionOptionsSkippedEvent /** Whether the provider child behind an acquisition has proven its start. A publish-first - * acquire hands over a `starting` child, which already takes input, and the `started` lifecycle - * event flips it. */ + * acquire hands over a `starting` child, which the host gives no input until the `started` + * lifecycle event flips it. */ export type StructuredAgentSessionProviderChildPhase = 'starting' | 'ready' -export type StructuredAgentSessionAcquireInput = { - identity: AgentSessionJournalIdentity - fence: number - spawnToken: string - options?: Readonly> - /** Provider events may begin before acquisition returns. */ - events?: StructuredAgentSessionEventSink - recordPhase?: AgentSessionCreatePhaseRecorder - /** Durably records the child's identity the moment it exists, before any handshake, so a crash - * mid-start leaves an owner recovery can stop. The acquisition's `process` must match it. */ - onSpawned?: (process: AgentSessionProcessIdentity) => Promise - /** Aborted by a close, or by a Stop admitted now, that must not wait behind this acquire: the - * adapter stops what it started and the acquire fails. An adapter whose acquire never waits on - * the provider's handshake may ignore it. */ - signal?: AbortSignal -} +/** The host's startup attempt (`structured-agent-session-startup-attempt`), which the host always + * passes whole. Only the fields every caller has are required, so an adapter test may pass less. */ +export type StructuredAgentSessionAcquireInput = Pick< + StructuredAgentSessionStartupAttempt, + 'identity' | 'fence' | 'spawnToken' +> & + Partial> & { + recordPhase?: AgentSessionCreatePhaseRecorder + /** Durably records the child's identity the moment it exists, before any handshake, so a crash + * mid-start leaves an owner recovery can stop. The acquisition's `process` must match it. */ + onSpawned?: (process: AgentSessionProcessIdentity) => Promise + } export type StructuredAgentSessionSetOptionInput = { sessionId: string 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 9046e40dbbe8..557e1569a418 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 @@ -52,6 +52,8 @@ export type StructuredAgentSessionResumeOutcome = diagnostic?: ProviderDiagnostic /** The host's own close, Stop or quit aborted the start, so it failed nothing it was for. */ aborted?: true + /** The start's deadline passed before it proved itself: Orca stopped it. */ + expired?: true argumentProblem?: AgentSessionArgumentProblem } @@ -168,6 +170,7 @@ async function startStructuredAgentSessionAgent( let attached: AgentSessionMutationResult let acquisitionError: unknown let aborted = false + let expired = false try { attached = await attachStructuredAgentSessionUnderSerialize(context, callerKey, params, { ...(startedFor === undefined ? {} : { startedFor }), @@ -176,6 +179,9 @@ async function startStructuredAgentSessionAgent( }, onAborted: () => { aborted = true + }, + onStartupExpired: () => { + expired = true } }) } catch (error) { @@ -190,17 +196,19 @@ async function startStructuredAgentSessionAgent( error ) if (settled) { - return withDiagnostic(settled.refusal, error, aborted) + return withDiagnostic(settled.refusal, error, { aborted, expired }) } throw error } - return attached.ok ? { ok: true } : withDiagnostic(attached.refusal, acquisitionError, aborted) + return attached.ok + ? { ok: true } + : withDiagnostic(attached.refusal, acquisitionError, { aborted, expired }) } function withDiagnostic( refusal: AgentSessionWireRefusal, error: unknown, - aborted: boolean + { aborted, expired }: { aborted: boolean; expired: boolean } ): StructuredAgentSessionResumeOutcome { const diagnostic = providerDiagnosticOf(error) const argumentProblem = argumentProblemOf(error) @@ -209,6 +217,7 @@ function withDiagnostic( refusal, ...(diagnostic ? { diagnostic } : {}), ...(aborted ? { aborted: true as const } : {}), + ...(expired ? { expired: true as const } : {}), ...(argumentProblem ? { argumentProblem } : {}) } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts index 9c287da62e2c..41b7017aecdb 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts @@ -16,7 +16,6 @@ import type { AgentSessionMutationResult } from '../../../shared/agent-session-wire' import { agentSessionLeaseAdmitsWriter } from '../../../shared/agent-session-lease-adjudication' -import type { AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' import type { AgentSessionRecord } from '../../../shared/agent-session-record' import { admitAttachOrRefuse, @@ -47,6 +46,8 @@ import { type AgentSessionCreatePhaseRecorder } from '../../observability/agent-session-instrumentation' import type { ProviderHistoryWindow } from '../agent-session-journal/journal-submission-reconciler' +import { readProviderHistoryWindow } from './structured-agent-session-provider-history-window' +import type { StructuredAgentSessionStartupAttempt } from './structured-agent-session-startup-attempt' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { StructuredAgentSessionLogger } from './structured-agent-session-logger' @@ -63,6 +64,9 @@ export type AttachFlowInput = { recordPhase?: AgentSessionCreatePhaseRecorder /** Aborted when a close, or a Stop admitted now, must not wait behind this attach's acquire. */ acquireSignal?: AbortSignal + /** The attempt this attach's acquire runs under, minted before the adapter is called. */ + onStartupAttempt?: (attempt: StructuredAgentSessionStartupAttempt) => void + startupDeadlineMs?: number /** Publishes the journal before clients can send against the new owner. `acquiredOwner` is * true only when this attach spawned the provider child, so a re-attach to a live one is not * mistaken for a cold acquire. */ @@ -279,27 +283,6 @@ export async function performAttach( } } -async function readProviderHistoryWindow(input: { - adapter: StructuredAgentSessionAdapter - identity: AgentSessionJournalIdentity - accountHome: AgentSessionRecord['accountHome'] - ownerAlreadyAdmitted: boolean -}): Promise { - const read = input.adapter.providerHistoryWindow - if (!read) { - return null - } - let history: ProviderHistoryWindow | null - try { - history = await read({ identity: input.identity, accountHome: input.accountHome }) - } catch { - return null - } - // A lease that was already live may belong to a provider child this process - // has not indexed yet. Preserve the safe unknown outcome in that case. - return history && input.ownerAlreadyAdmitted ? { ...history, turnInFlight: true } : history -} - async function settleUnsupportedReservation( input: AttachFlowInput, record: AgentSessionRecord diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts index e801a99042da..d54460ef50aa 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts @@ -36,6 +36,10 @@ import { } from './structured-agent-session-provider-child' import type { DeferredStructuredAgentSessionEventSink } from './structured-agent-session-event-sink' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' +import { + isStructuredAgentSessionStartupExpired, + type StructuredAgentSessionStartupAttempt +} from './structured-agent-session-startup-attempt' import { addAgentSessionCreatePhaseAttributes, withAgentSessionCreatePhase, @@ -51,6 +55,8 @@ export type StructuredAgentSessionAttachOptions = { startedFor?: string /** A close, an admitted Stop or quit aborted this attach: its refusal is that abort's. */ onAborted?: () => void + /** The start's deadline aborted this attach's acquire: it failed, nobody stopped it. */ + onStartupExpired?: () => void } /** @@ -114,7 +120,9 @@ async function runAttach( return await runAttachUnderAbort(context, callerKey, params, options, acquire.signal) } finally { acquire.end() - if (acquire.signal.aborted) { + if (isStructuredAgentSessionStartupExpired(acquire.signal.reason)) { + options.onStartupExpired?.() + } else if (acquire.signal.aborted) { options.onAborted?.() } } @@ -161,10 +169,12 @@ async function runAttachUnderAbort( // attach makes the child and its sink the session's; any other exit closes the sink with // whatever the child queued, and leaves the conversation's child as it was. const attemptSink = context.runtimeState.mintEventSink(sessionId) - const attempt: { candidate: AttachCandidate | null; committed: boolean } = { - candidate: null, - committed: false - } + const attempt: { + candidate: AttachCandidate | null + committed: boolean + startup: StructuredAgentSessionStartupAttempt | null + } = { candidate: null, committed: false, startup: null } + const { startupAttempts } = context.runtimeState try { const attached = await performAttach({ store: context.deps.store, @@ -193,6 +203,13 @@ async function runAttachUnderAbort( now: () => context.now(), recordPhase, acquireSignal, + ...(context.deps.startupDeadlineMs === undefined + ? {} + : { startupDeadlineMs: context.deps.startupDeadlineMs }), + onStartupAttempt: (startup) => { + attempt.startup = startup + startupAttempts.track(sessionId, startup) + }, ...(options.onAcquisitionFailed ? { onAcquisitionFailed: options.onAcquisitionFailed } : {}), openConversation: async (record) => { const conversation = await context.openConversation(record.sessionId, { @@ -259,12 +276,18 @@ async function runAttachUnderAbort( context.runtimeState.adoptEventSink(sessionId, candidate.sink) attempt.committed = candidate.sink === attemptSink indexProviderChild(conversation, candidate.child) + if (attempt.startup) { + startupAttempts.published(sessionId, attempt.startup.attemptId, candidate.child) + } context.publishStatus?.(sessionId) } return stampFailedCreateOwnerVerdict(context.deps.store, callerKey, params.envelope, attached) } finally { if (!attempt.committed) { attemptSink.close() + if (attempt.startup) { + startupAttempts.abandon(sessionId, attempt.startup.attemptId) + } } } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-hung-start-stop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-hung-start-stop.test.ts index 595e18177541..347de6cf6643 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-claude-hung-start-stop.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-claude-hung-start-stop.test.ts @@ -1,5 +1,6 @@ -// A Claude CLI that never answers initialize still takes the chat's message at once, and the -// chat's Stop still ends it. On the shipping adapter and host, against a CLI that echoes nothing. +// A Claude CLI that never answers initialize is handed nothing: the host accepts the chat's message +// at once and holds it until the start proves itself, and the chat's Stop still ends the start. On +// the shipping adapter and host, against a CLI that echoes nothing. import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' @@ -130,7 +131,15 @@ async function readsWorking(): Promise { ) } -it('writes the message to a start that never answers, and a Stop ends it: stopped, nothing working', async () => { +/** Whether the CLI was written `hello`, once the delivery loop has settled what it does with it. */ +async function helloWritten(): Promise { + await vi.waitFor(() => + expect(host.collaboratorsForTests().conversationDelivery.loop.isRunning(SESSION)).toBe(false) + ) + return claude.connections[0]!.sent.some((message) => JSON.stringify(message).includes('hello')) +} + +it('holds the message from a start that never answers, and a Stop ends it: stopped, nothing working', async () => { expect(host['sessions'].get(SESSION)?.child?.phase).toBe('starting') const body = hostTestMessage('hello') const sent = await host.send(CALLER, { envelope: envelope('agentSession.send', { body }), body }) @@ -140,10 +149,8 @@ it('writes the message to a start that never answers, and a Stop ends it: stoppe const id = sent.value.clientMessageId const [connection] = claude.connections - // Handed over and written while initialize is still unanswered. - await vi.waitFor(() => - expect(connection!.sent.some((message) => JSON.stringify(message).includes('hello'))).toBe(true) - ) + // Accepted, and held while initialize is still unanswered. + expect(await helloWritten()).toBe(false) expect(connection!.calls).toEqual([{ subtype: 'initialize' }]) expect(host['sessions'].get(SESSION)?.child?.phase).toBe('starting') expect(await readsWorking()).toBe(true) @@ -153,7 +160,7 @@ it('writes the message to a start that never answers, and a Stop ends it: stoppe expect(stopped).toMatchObject({ ok: true, value: { cancelled: true } }) await vi.waitFor(() => expect(host['sessions'].get(SESSION)?.child).toBeNull()) expect(connection!.closeCount).toBeGreaterThan(0) - // Stopped, not in doubt: the child it was written to is gone and never echoed it. + // Withdrawn, not in doubt: it was never handed over. await vi.waitFor(async () => expect( (await host.journalSnapshot(SESSION)).submissions.find( @@ -164,7 +171,7 @@ it('writes the message to a start that never answers, and a Stop ends it: stoppe expect(await readsWorking()).toBe(false) }) -async function sendHello(): Promise { +async function sendHello(written: boolean): Promise { const body = hostTestMessage('hello') // A person's send, which a close keeps as a held card. const sent = await host.send(CALLER, { @@ -175,11 +182,7 @@ async function sendHello(): Promise { if (!sent.ok) { throw new Error('send refused') } - await vi.waitFor(() => - expect( - claude.connections[0]!.sent.some((message) => JSON.stringify(message).includes('hello')) - ).toBe(true) - ) + await vi.waitFor(async () => expect(await helloWritten()).toBe(written)) return sent.value.clientMessageId } @@ -189,12 +192,12 @@ async function submission(id: string) { ) } -// A CLI that never answered initialize ran nothing it was handed: a close settles it as it does a -// queued send, keeping the person's words as a held card. +// A CLI that never answered initialize was handed nothing: a close settles the held message as it +// does any queued send, keeping the person's words as a held card. it.each(['user-close', 'evict'] as const)( - 'keeps a message written to a start that never answered as a held card when a %s ends it', + 'keeps a message held behind a start that never answered as a held card when a %s ends it', async (cause) => { - const id = await sendHello() + const id = await sendHello(false) await host.close(SESSION, cause) @@ -213,7 +216,7 @@ it.each(['user-close', 'evict'] as const)( it('leaves a message in doubt when the start answered and the chat closes before the echo', async () => { answerInitialize() await adapter['sessions'].get(SESSION)?.startup.settled - const id = await sendHello() + const id = await sendHello(true) await host.close(SESSION, 'user-close') diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts index e0130423d5e2..5a1bbeab5120 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts @@ -39,11 +39,9 @@ import type { JournalLifecycleMutationInput } from '../agent-session-journal/jou import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { AgentSessionCommandAdmission, - StructuredAgentSessionAdapter, - StructuredAgentSessionProviderChildPhase + StructuredAgentSessionAdapter } from './structured-agent-session-adapter' import type { StructuredAgentRegistry } from './structured-agent-registry' -import { structuredAgentSessionStartFailure } from './structured-agent-session-failure-text' import { conversationCommandBlocked } from './structured-conversation-command-admission' export const STRUCTURED_AGENT_SESSION_COMPACT_COMMAND = 'compact' @@ -155,7 +153,6 @@ export type StructuredAgentSessionCommandHandoverContext = { fence: number adapter: StructuredAgentSessionAdapter agents: StructuredAgentRegistry - providerChildPhase?: () => StructuredAgentSessionProviderChildPhase | undefined /** Who a failure the handover meets names, as the start's own row does. */ failureTextContext?: AgentSessionFailureWordsContext record: () => AgentSessionRecord | null @@ -210,19 +207,11 @@ export async function handOverStructuredAgentSessionCommand( command: { clientMessageId, ...turn, running } }) } catch (error) { - // A child still starting throws only for a start that failed before the write, so the command - // provably did not run. Any other throw is a lost reply: the command may have run. - const unsent = - ctx.providerChildPhase?.() === 'starting' - ? { - state: 'rejected' as const, - ...structuredAgentSessionStartFailure({ error }, ctx.failureTextContext) - } - : { - state: 'unknown' as const, - reason: error instanceof Error ? error.message : String(error) - } - await settleUnsentCommand(ctx, clientMessageId, unsent) + // Handed over only to a child that proved its start, so a throw is a lost reply: it may have run. + await settleUnsentCommand(ctx, clientMessageId, { + state: 'unknown', + reason: error instanceof Error ? error.message : String(error) + }) return } if (admission.state === 'rejected') { 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 e19127fde7cd..4e6b575ef1f1 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 @@ -221,22 +221,23 @@ describe('the sweep and the lease (P2-20)', () => { }) describe('a start that never finishes (P2-15)', () => { - it('is stopped by the sweep, and the message it was handed carries its one row and rejection', async () => { + it('is stopped by the sweep, and the message held for it carries its one row and rejection', async () => { const stopReason = 'Codex never finished starting, so Orca stopped it.' const spawn = rig.adapter.acquire.getMockImplementation()! rig.adapter.acquire.mockImplementationOnce(async (input) => ({ ...(await spawn(input)), providerChildPhase: 'starting' as const })) - // Written to the starting child at once; it never answers. - rig.adapter.dispatch.mockResolvedValueOnce({ state: 'admitted' }) + // Held for the starting child, which never proves its start. const reader = collectSubscriber() const attached = await rig.host.attach(CALLER, hostTestAttachParams(null)) expect(attached.ok).toBe(true) await rig.host.subscribe({ id: 'reader', sessionId: SESSION, emit: reader.emit }) const sent = await rig.host.send(CALLER, restTestSend('stuck behind the start', fence())) expect(sent.ok).toBe(true) - await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledOnce()) + await vi.waitFor(() => + expect(rig.host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('starting') + ) rig.clock.now += IDLE_MS + 1 await sweepOnce(rig.host) @@ -259,7 +260,8 @@ describe('a start that never finishes (P2-15)', () => { const again = await rig.host.send(CALLER, restTestSend('try again', fence())) expect(again.ok).toBe(true) - await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(2), COLD_START) + expect(rig.adapter.dispatch).not.toHaveBeenCalled() + await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(1), COLD_START) await expect(sweepOnce(rig.host)).resolves.toBeUndefined() }) }) 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 1100c0abf81b..e2b206595049 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 @@ -25,6 +25,8 @@ import type { StructuredAgentSessionHostSession } from './structured-agent-sessi import { StructuredAgentSessionIdleSweep } from './structured-agent-session-idle-sweep' import { AGENT_SESSION_NOT_ATTACHED } from './structured-agent-session-mutation-admission' import { deferredStructuredAgentSessionLogger } from './structured-agent-session-logger' +import { sameProviderChild } from './structured-agent-session-provider-child' +import type { StructuredAgentSessionExpiredStartup } from './structured-agent-session-startup-attempt' export type StructuredAgentSessionConversationLifetime = ReturnType< typeof createStructuredAgentSessionConversationLifetime @@ -100,10 +102,35 @@ export function createStructuredAgentSessionConversationLifetime(host: { return { idleSweep, stopAgent, + /** A host stop of the child the expired attempt published, if it is still that one and still + * starting: its end fails what is queued, as the idle sweep's stop of a start does. */ + expireStartup: ({ sessionId, child: expired }: StructuredAgentSessionExpiredStartup): void => { + void serialize(sessionId, async () => { + const child = sessions.get(sessionId)?.child + if ( + !expired || + !child || + child.phase !== 'starting' || + !sameProviderChild(child, expired) + ) { + return + } + await stopStructuredAgentSessionAgentUnderSerialize(host.context(), sessionId, { + cause: 'host-stop' + }) + }).catch((error: unknown) => + deps().logger.warn('stopping an agent past its start deadline failed', { + scope: 'startup-deadline', + sessionId, + error + }) + ) + }, /** Quit has begun: nothing opens a conversation or sweeps one after this. */ dispose: (): void => { disposed = true idleSweep.dispose() + host.context().runtimeState.startupAttempts.dispose() }, /** * The only way any code reaches a session. An open conversation answers without the lock, so diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.test.ts index 242daf74faf6..61f596359165 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' import { agentSessionFailureFact } from '../../../shared/agent-session-failure' -import { structuredAgentSessionEndedChildFailure } from './structured-agent-session-delivery-loop' +import { structuredAgentSessionEndedChildFailure } from './structured-agent-session-ended-child-failure' import type { StructuredAgentSessionChildEndCause, StructuredAgentSessionEndedChild 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 380742f0e235..d8d6d9a1af8c 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 @@ -25,8 +25,6 @@ import { } from './structured-agent-session-failure-text' import type { StructuredAgentSessionResumeOutcome } from './structured-agent-session-agent-start' import type { - StructuredAgentSessionChildEndCause, - StructuredAgentSessionEndedChild, StructuredAgentSessionHostSession, StructuredAgentSessionProviderChildIdentity } from './structured-agent-session-host-types' @@ -40,6 +38,7 @@ import { structuredAgentSessionNextHandover } from './structured-agent-session-o import { structuredAgentSessionCommandRunning } from './structured-agent-session-command-turn' import type { StructuredAgentSessionLogger } from './structured-agent-session-logger' import { holdRestartedStructuredAgentSessionSends } from './structured-agent-session-host-lifetime' +import { structuredAgentSessionEndedChildFailure } from './structured-agent-session-ended-child-failure' export type StructuredAgentSessionDeliveryLoopDeps = { sessions: ReadonlyMap @@ -218,7 +217,6 @@ export class StructuredAgentSessionDeliveryLoop { child && awaited && child.generation === awaited.generation && child.fence === awaited.fence ? child : null - // A child still starting takes input: the provider queues it behind its own start. if (!awaitedChild) { // The child started for this message is gone or replaced by another. const ended = awaitedChild ? undefined : session.lastEndedChild @@ -238,7 +236,9 @@ export class StructuredAgentSessionDeliveryLoop { // Never steer into a turn a person's Stop is ending: the message runs after it, as its own // turn, and the turn's end is a commit that wakes the loop again. Read here, at the handover, // because a Stop can take the lane between the step that judged the send and this one. - if (this.deps.stopping(sessionId)) { + // Nor hand anything to a child still proving its start: nothing is written until it is + // `ready`, whose settlement wakes the loop, so a start that fails leaves the message unsent. + if (this.deps.stopping(sessionId) || awaitedChild.phase === 'starting') { return this.stop(sessionId) } // None while a turn is still opening: joining it would record the message ahead of it. @@ -253,7 +253,6 @@ export class StructuredAgentSessionDeliveryLoop { fence: awaitedChild.fence, adapter: this.deps.adapter, agents: this.deps.agents, - providerChildPhase: () => this.deps.sessions.get(sessionId)?.child?.phase, failureTextContext: this.deps.failureTextContext(sessionId), record: () => this.deps.record(sessionId), childWork: () => this.deps.readChildWork(sessionId), @@ -267,8 +266,11 @@ export class StructuredAgentSessionDeliveryLoop { /** A start the session refused, as the failure every queued message it was for is rejected with. */ private refusedStart( sessionId: string, - { refusal, diagnostic, argumentProblem }: RefusedStart + { refusal, diagnostic, argumentProblem, expired }: RefusedStart ): StartFailure { + if (expired) { + return { startKey: null, cause: { failure: agentSessionFailureFact('hostStopped') } } + } // 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 { @@ -345,36 +347,3 @@ function startThatFailedWhileQueued( const cause = structuredAgentSessionEndedChildFailure(ended) return cause ? { startKey: ended.generation, cause } : null } - -function providerEndFailure( - ended: StructuredAgentSessionEndedChild -): StructuredAgentSessionStartFailureCause { - if (ended.duringStartup) { - return { exit: ended.failure } - } - return { failure: ended.failure ?? agentSessionFailureFact('providerExited') } -} - -// Every end cause, so a new one does not compile until it says whether it fails what is queued. -const ENDED_CHILD_FAILURE = { - 'user-stop': () => null, - // The user closing this chat closes what was queued before it; see `closeWhatTheUserClosed`. - 'user-close': () => null, - // The host stopping the child is Orca's cause, never the provider's: a start that never finished. - 'host-stop': () => ({ failure: agentSessionFailureFact('hostStopped') }), - exit: providerEndFailure, - // The attach records its own fault as the end's failure. - 'attach-failed': providerEndFailure, - // Reached only when an eviction's stop landed and a later step failed, leaving the conversation. - evict: providerEndFailure -} satisfies Record< - StructuredAgentSessionChildEndCause, - (ended: StructuredAgentSessionEndedChild) => StructuredAgentSessionStartFailureCause | null -> - -/** Why a queued message the child never took is rejected; null when its end fails nothing. */ -export function structuredAgentSessionEndedChildFailure( - ended: StructuredAgentSessionEndedChild -): StructuredAgentSessionStartFailureCause | null { - return ENDED_CHILD_FAILURE[ended.cause](ended) -} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-ended-child-failure.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-ended-child-failure.ts new file mode 100644 index 000000000000..2ae9a10a4631 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-ended-child-failure.ts @@ -0,0 +1,41 @@ +// Why a queued message the child never took is rejected, by how that child ended. + +import { agentSessionFailureFact } from '../../../shared/agent-session-failure' +import type { StructuredAgentSessionStartFailureCause } from './structured-agent-session-failure-text' +import type { + StructuredAgentSessionChildEndCause, + StructuredAgentSessionEndedChild +} from './structured-agent-session-host-types' + +function providerEndFailure( + ended: StructuredAgentSessionEndedChild +): StructuredAgentSessionStartFailureCause { + if (ended.duringStartup) { + return { exit: ended.failure } + } + return { failure: ended.failure ?? agentSessionFailureFact('providerExited') } +} + +// Every end cause, so a new one does not compile until it says whether it fails what is queued. +const ENDED_CHILD_FAILURE = { + 'user-stop': () => null, + // The user closing this chat closes what was queued before it; see `closeWhatTheUserClosed`. + 'user-close': () => null, + // The host stopping the child is Orca's cause, never the provider's: a start that never finished. + 'host-stop': () => ({ failure: agentSessionFailureFact('hostStopped') }), + exit: providerEndFailure, + // The attach records its own fault as the end's failure. + 'attach-failed': providerEndFailure, + // Reached only when an eviction's stop landed and a later step failed, leaving the conversation. + evict: providerEndFailure +} satisfies Record< + StructuredAgentSessionChildEndCause, + (ended: StructuredAgentSessionEndedChild) => StructuredAgentSessionStartFailureCause | null +> + +/** Why a queued message the child never took is rejected; null when its end fails nothing. */ +export function structuredAgentSessionEndedChildFailure( + ended: StructuredAgentSessionEndedChild +): StructuredAgentSessionStartFailureCause | null { + return ENDED_CHILD_FAILURE[ended.cause](ended) +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts index cc5be644cc13..e8b90cc2da18 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts @@ -13,10 +13,16 @@ import { type ProviderChildSessions } from './structured-agent-session-provider-child' import { resolveStructuredSessionRecovery } from './structured-agent-session-recovery-resolution' +import { + StructuredAgentSessionStartupAttempts, + StructuredAgentSessionStartupExpiredError, + type StructuredAgentSessionExpiredStartup +} from './structured-agent-session-startup-attempt' export class StructuredAgentSessionHostRuntimeState { private readonly eventSinks = new Map() readonly acquireAborts = new StructuredAgentSessionAcquireAborts() + readonly startupAttempts: StructuredAgentSessionStartupAttempts private readonly leaseRenewer: StructuredAgentSessionLeaseRenewer private readonly onEventSinkFailure?: (sessionId: string, error: unknown) => void @@ -24,9 +30,24 @@ export class StructuredAgentSessionHostRuntimeState { private readonly deps: StructuredAgentSessionHostDeps, /** Required: a child held here renews its lease without a PID probe. */ sessions: ProviderChildSessions, - onEventSinkFailure?: (sessionId: string, error: unknown) => void + onEventSinkFailure?: (sessionId: string, error: unknown) => void, + /** A published child's start ran past its deadline; one still acquiring is aborted here. */ + onStartupExpired?: (expired: StructuredAgentSessionExpiredStartup) => void ) { this.onEventSinkFailure = onEventSinkFailure + this.startupAttempts = new StructuredAgentSessionStartupAttempts({ + now: () => deps.now?.() ?? Date.now(), + expire: (expired) => { + if (expired.child === null) { + this.acquireAborts.abort( + expired.sessionId, + new StructuredAgentSessionStartupExpiredError() + ) + return + } + onStartupExpired?.(expired) + } + }) this.leaseRenewer = new StructuredAgentSessionLeaseRenewer({ store: deps.store, probe: (record) => this.probeRecord(record), 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 2230efa15c9b..8552c76d2289 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 @@ -141,6 +141,8 @@ export type StructuredAgentSessionHostDeps = { now?: () => number /** The idle sweep's period and window. Tests drive these; production takes the defaults. */ idleSweep?: { intervalMs?: number; idleMs?: number } + /** How long a start may take to prove itself. Tests drive this; production takes the default. */ + startupDeadlineMs?: number /** Whether an orchestration dispatch still owns this session's worker; absent answers no. */ hasOpenDispatch?: (record: AgentSessionRecord) => boolean /** A chat tab left the screen: closed, or its workspace removed. Advisory; a throw is logged. */ 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 e0f193ba0b49..3e8f87a3a9fc 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 @@ -104,8 +104,11 @@ export class StructuredAgentSessionHost { (sessionId) => this.lifetime.conversation(sessionId), this.clientDelivery.readChildWork ) - this.runtimeState = new StructuredAgentSessionHostRuntimeState(deps, this.sessions, (id, e) => - this.eventRecovery.recoverAfterSinkFailure(id, e) + this.runtimeState = new StructuredAgentSessionHostRuntimeState( + deps, + this.sessions, + (id, e) => this.eventRecovery.recoverAfterSinkFailure(id, e), + (expired) => this.lifetime.expireStartup(expired) ) this.reconcileLeases = createRestartReconciler({ store: deps.store, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-admission.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-admission.ts index 3cb9762cf39f..3fac8f4d8709 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-admission.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-admission.ts @@ -89,7 +89,6 @@ export type AgentSessionMutationRequest = { record: AgentSessionRecord ) => Promise publish: (journal: AgentSessionJournal) => void - providerChildPhase?: AgentSessionTurnContext['providerChildPhase'] now: () => number } 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 099b356cb5e3..e704dbba5707 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 @@ -71,7 +71,6 @@ export function mutateStructuredAgentSession( journal: () => context.sessions.get(envelope.sessionId)?.journal, prepareSession, publish: (journal) => context.publish(envelope.sessionId, journal), - providerChildPhase: () => context.sessions.get(envelope.sessionId)?.child?.phase, now: () => context.now() }) ) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-turn-context.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-turn-context.ts index cae4399cb654..776937662eb0 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-turn-context.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-turn-context.ts @@ -32,7 +32,6 @@ export function mutationTurnContext( .then(() => undefined), resolvedBy: request.callerKey, publish: () => request.publish(journal), - ...(request.providerChildPhase ? { providerChildPhase: request.providerChildPhase } : {}), now: () => request.now() } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-owed-work-release.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-owed-work-release.test.ts index c906785c5418..6d188fb3b385 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-owed-work-release.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-owed-work-release.test.ts @@ -175,15 +175,21 @@ function waitOutSeveralSweeps(): Promise { return new Promise((resolve) => setTimeout(resolve, SWEEP_MS * 20)) } +function deliverySettled(): Promise { + return vi.waitFor(() => + expect(host.collaboratorsForTests().conversationDelivery.loop.isRunning(SESSION)).toBe(false) + ) +} + describe('a Claude chat whose CLI is still starting', () => { - it('writes a message at once, and keeps the agent inside the idle window while startup is still out', async () => { + it('holds a message until startup lands, and keeps the agent inside the idle window meanwhile', async () => { await attachStarting() const sent = await send('sent while starting') - // Written before initialize answers: the CLI queues it behind its own start. - await untilSent(claude.connections[0]) - expect(claude.connections[0].sent).toEqual([expect.objectContaining({ type: 'user' })]) - await vi.waitFor(async () => expect(await dispatchState(sent)).toBe('accepted')) + // Held, never written, while initialize is unanswered. + await deliverySettled() + expect(claude.connections[0].sent).toEqual([]) + expect(await dispatchState(sent)).toBe('pending') clock += IDLE_MS - 1 await waitOutSeveralSweeps() @@ -192,10 +198,13 @@ describe('a Claude chat whose CLI is still starting', () => { landInit() await claudeStartupSettled(adapter, SESSION) + await untilSent(claude.connections[0]) + expect(claude.connections[0].sent).toEqual([expect.objectContaining({ type: 'user' })]) + await vi.waitFor(async () => expect(await dispatchState(sent)).toBe('accepted')) expect(host.hasSession(SESSION)).toBe(true) }) - it('gives a message written while starting a full idle window after startup lands to open its turn', async () => { + it('gives a message held while starting a full idle window after startup lands to open its turn', async () => { await attachStarting() const connection = claude.connections[0] // Claude echoes a prompt only when it starts that turn, which a loaded machine delays. @@ -203,16 +212,19 @@ describe('a Claude chat whose CLI is still starting', () => { connection.sent.push(message) } const held = await send('sent while starting') - await untilSent(connection) - expect(connection.sent).toEqual([expect.objectContaining({ type: 'user' })]) + await deliverySettled() + expect(connection.sent).toEqual([]) clock += IDLE_MS - 1 await waitOutSeveralSweeps() expect(host.hasSession(SESSION)).toBe(true) - // Startup lands just before the window closes; the agent stays while its message waits. + // Startup lands just before the window closes and the message is handed over; the agent stays + // while its message waits. landInit() await claudeStartupSettled(adapter, SESSION) await Promise.all(lifecycle) + await untilSent(connection) + expect(connection.sent).toEqual([expect.objectContaining({ type: 'user' })]) clock += IDLE_MS - 1 await waitOutSeveralSweeps() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts index 8c98a685c7d2..8ab957f0e900 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts @@ -215,6 +215,18 @@ function currentChild() { return { sessionId: SESSION, fence: child.fence, acquisitionGeneration: child.generation } } +/** The current child proves its start, as a publish-first provider's `started` event does. */ +function prove(): Promise { + const { acquisitionGeneration, ...child } = currentChild() + return host.handleAdapterEvent({ + type: 'started', + ...child, + acquisitionGeneration, + reportedOptions: { model: 'default' }, + restoreSkippedOptions: [] + }) +} + function exit(child: ReturnType, reason: string, startupUnproven?: true) { return host.handleAdapterEvent({ type: 'ended', @@ -278,14 +290,13 @@ describe('Stop on a child still proving its start', () => { adapterExtras = { closeSession: vi.fn(async () => true) } await restartHost() acquire.mockImplementationOnce(spawnStartingChild) - // Written to the starting child at once; it never answers. - dispatch.mockResolvedValueOnce({ state: 'admitted' }) + // Held for the starting child, which never proves its start. const first = await accept('hello') const journal = conversation()?.journal const events = await subscribe() const frames = watchStatus() await eventually(() => expect(conversation()?.child?.phase).toBe('starting')) - await eventually(async () => expect((await submission(first))?.handedOverAt).toBeDefined()) + expect((await submission(first))?.handedOverAt).toBeUndefined() expect(await stop()).toMatchObject({ ok: true, value: { cancelled: true } }) @@ -306,7 +317,7 @@ describe('Stop on a child still proving its start', () => { const next = await accept('after stop') await eventually(async () => expect((await submission(next))?.dispatchState).toBe('accepted')) expect(conversation()?.journal).toBe(journal) - expect(dispatch.mock.calls.map(([input]) => input.clientMessageId)).toEqual([first, next]) + expect(dispatch.mock.calls.map(([input]) => input.clientMessageId)).toEqual([next]) // The reader opened before the Stop saw the next message delivered on the same stream. expect( events.some( @@ -383,16 +394,14 @@ describe('a published child that dies while it proves its start', () => { const EXIT = START_EXIT const TEXT = START_TEXT - it('leaves one error row keyed by the start, and every message it was handed rejected with it (R2)', async () => { + it('leaves one error row keyed by the start, and every message held for it rejected with it (R2)', async () => { await restartHost() acquire.mockImplementation(spawnStartingChild) - // The first is written to the starting child at once; it never answers, so its turn never - // opens and the second waits behind it. - dispatch.mockImplementation(async () => ({ state: 'admitted' as const })) + // Both are held for the starting child, which never proves its start. const first = await accept('first') const events = await subscribe() const second = await accept('second') - await eventually(() => expect(dispatch).toHaveBeenCalledTimes(1)) + await eventually(() => expect(conversation()?.child?.phase).toBe('starting')) const child = currentChild() await exit(child, EXIT, true) @@ -415,6 +424,7 @@ describe('a published child that dies while it proves its start', () => { } expect(rejectedIn(events, second)).toBe(true) expect(acquire).toHaveBeenCalledTimes(2) + expect(dispatch).not.toHaveBeenCalled() }) }) @@ -508,6 +518,7 @@ describe('a start another operation made that dies while a sent message waits on await exited await held const id = params.envelope.clientOperationId + await prove() await eventually(async () => expect((await submission(id))?.dispatchState).not.toBe('pending')) expect((await submission(id))?.dispatchState).toBe('accepted') @@ -564,14 +575,13 @@ describe('a child that ends before its message is handed over', () => { }) describe('another child indexed after a Stop ended the starting one', () => { - it('hands the next message to the child now there, still starting (R2)', async () => { + it('hands the next message to the child now there once it proves its start (R2)', async () => { adapterExtras = { closeSession: vi.fn(async () => true) } await restartHost() acquire.mockImplementation(spawnStartingChild) - // The starting child takes the first message at once and never answers it. - dispatch.mockResolvedValueOnce({ state: 'admitted' }) + // Held for the starting child, which never proves its start. const first = await accept('first') - await eventually(() => expect(dispatch).toHaveBeenCalledTimes(1)) + await eventually(() => expect(conversation()?.child?.phase).toBe('starting')) const stopped = currentChild() expect(await stop()).toMatchObject({ ok: true }) // An operation that needs the agent starts its own child before the second message is sent. @@ -581,15 +591,19 @@ describe('another child indexed after a Stop ended the starting one', () => { const replacement = currentChild() expect(replacement.acquisitionGeneration).not.toBe(stopped.acquisitionGeneration) const secondId = await accept('second') + await settleLoop() + expect((await submission(secondId))?.handedOverAt).toBeUndefined() + + await prove() await eventually(async () => expect((await submission(secondId))?.dispatchState).toBe('accepted') ) expect(conversation()?.child).toMatchObject({ generation: replacement.acquisitionGeneration, - phase: 'starting' + phase: 'ready' }) - expect(dispatch.mock.calls.map(([input]) => input.clientMessageId)).toEqual([first, secondId]) + expect(dispatch.mock.calls.map(([input]) => input.clientMessageId)).toEqual([secondId]) expect(acquire).toHaveBeenCalledTimes(3) expect(await submission(first)).toMatchObject({ reason: DISPATCH_REJECTED_CANCELLED }) }) @@ -703,15 +717,14 @@ async function settleLoop(): Promise { } describe('how a stopped child ends the start its loop was waiting on', () => { - /** A starting child that took the first message and never answered it, then the stop below; - * the message sent after the stop is the next delivery's. */ + /** A starting child the first message is held for, then the stop below; the message sent after + * the stop is the next delivery's. */ async function stoppedWhileStarting(stop: () => Promise) { adapterExtras = { closeSession: vi.fn(async () => true) } await restartHost() acquire.mockImplementationOnce(spawnStartingChild) - dispatch.mockResolvedValueOnce({ state: 'admitted' }) await accept('first') - await eventually(() => expect(dispatch).toHaveBeenCalledTimes(1)) + await eventually(() => expect(conversation()?.child?.phase).toBe('starting')) await stop() return accept('second') } @@ -865,9 +878,8 @@ describe('how a stopped child ends the start its loop was waiting on', () => { adapterExtras = { closeSession: vi.fn(async () => true) } await restartHost() acquire.mockImplementationOnce(spawnStartingChild) - dispatch.mockResolvedValueOnce({ state: 'admitted' }) const first = await accept('first') - await eventually(() => expect(dispatch).toHaveBeenCalledTimes(1)) + await eventually(() => expect(conversation()?.child?.phase).toBe('starting')) await host['serialize'](SESSION, () => stopStructuredAgentSessionAgentUnderSerialize(host['lifetimeContext'](), SESSION, { cause: 'host-stop', @@ -875,7 +887,7 @@ describe('how a stopped child ends the start its loop was waiting on', () => { }) ) - // What the stopped start was handed fails with it, said once; a later send starts afresh. + // What was held for the stopped start fails with it, said once; a later send starts afresh. await eventually(async () => expect((await submission(first))?.dispatchState).toBe('rejected')) // The sentence is the constructor's, not the reason the stop was given. const text = 'Codex never finished starting, so Orca stopped it.' diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-history-window.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-history-window.ts new file mode 100644 index 000000000000..4d090e3ef066 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-history-window.ts @@ -0,0 +1,28 @@ +// What the provider recorded after the journal's last committed item, sampled before an attach +// acquires a new child: once one is running, an absent prompt no longer proves non-delivery. + +import type { AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import type { ProviderHistoryWindow } from '../agent-session-journal/journal-submission-reconciler' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' + +export async function readProviderHistoryWindow(input: { + adapter: StructuredAgentSessionAdapter + identity: AgentSessionJournalIdentity + accountHome: AgentSessionRecord['accountHome'] + ownerAlreadyAdmitted: boolean +}): Promise { + const read = input.adapter.providerHistoryWindow + if (!read) { + return null + } + let history: ProviderHistoryWindow | null + try { + history = await read({ identity: input.identity, accountHome: input.accountHome }) + } catch { + return null + } + // A lease that was already live may belong to a provider child this process + // has not indexed yet. Preserve the safe unknown outcome in that case. + return history && input.ownerAlreadyAdmitted ? { ...history, turnInFlight: true } : history +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts index 589bf37c2616..9c4778d0efe5 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts @@ -1,10 +1,11 @@ // The host's half of a provider child proving its start. // // A publish-first acquire hands the host a child that has answered nothing yet, so the record -// keeps only the saved options the reservation carried. This is where the host learns the start -// landed, flips the session to `ready`, and persists what the child now reports as fact through -// the same record write a user's option change takes. Bookkeeping never gates the user: a failed -// write is reported and the session stays usable. +// keeps only the saved options the reservation carried, and the delivery loop hands it nothing. +// This is where the host learns the start landed: the child turns `ready`, its startup attempt +// ends, the loop wakes to hand over what was queued meanwhile, and what the child now reports is +// persisted as fact through the same record write a user's option change takes. Bookkeeping never +// gates the user: a failed write is reported, and delivery has already been woken. // // This runs under the session's own serialized step, which its close and sends wait on, so it // asks the provider nothing: the event carries what the child proved. @@ -20,6 +21,7 @@ import type { } from './structured-agent-session-host-types' import { nativeSessionOptionsFromReport } from './structured-agent-session-option-restoration' import { markProviderChildStarted } from './structured-agent-session-provider-child' +import type { StructuredAgentSessionStartupAttempts } from './structured-agent-session-startup-attempt' export type StructuredAgentSessionProviderStartedContext = { deps: StructuredAgentSessionHostDeps @@ -27,6 +29,9 @@ export type StructuredAgentSessionProviderStartedContext = { serialize: (sessionId: string, task: () => Promise) => Promise now: () => number publishStatus?: (sessionId: string) => void + runtimeState: { startupAttempts: Pick } + /** The barrier lifts: what was accepted while the child started is handed over now. */ + wakeDelivery: (sessionId: string) => void } export function settleStructuredAgentSessionProviderStarted( @@ -36,15 +41,13 @@ export function settleStructuredAgentSessionProviderStarted( // Serialized behind the attach that published this child, so the lease it proved is committed. return context.serialize(event.sessionId, async () => { const session = context.sessions.get(event.sessionId) - if ( - !session || - !markProviderChildStarted(session, { - generation: event.acquisitionGeneration, - fence: event.fence - }) - ) { + const child = { generation: event.acquisitionGeneration, fence: event.fence } + // A stale child's proof starts nothing: the barrier stays on the child the host holds. + if (!session || !markProviderChildStarted(session, child)) { return } + context.runtimeState.startupAttempts.ready(event.sessionId, child) + context.wakeDelivery(event.sessionId) try { await persistStartedOptions(context, event) } catch (error) { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts index fd3294ddc7d6..69a762908e12 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts @@ -169,11 +169,17 @@ export async function createQueuedMessageTestRig( })) } - /** A first send that keeps the session working until the test settles it. */ + /** A first send that keeps the session working until the test settles it. A starting child + * proves its start first, as the host hands it nothing before. */ async function workingSend(): Promise { const { id, result } = send('work on this') await result - await eventually(async () => expect((await submission(id))?.handedOverAt).toBeDefined()) + await eventually(async () => { + if (host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase === 'starting') { + await emitStarted() + } + expect((await submission(id))?.handedOverAt).toBeDefined() + }) return id } @@ -241,6 +247,26 @@ export async function createQueuedMessageTestRig( }) } + /** The starting child proves its start, as a publish-first provider's `started` event does: the + * host hands it what it held. */ + async function proveStart(): Promise { + await eventually(() => + expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('starting') + ) + await emitStarted() + } + + function emitStarted(): Promise { + return host.handleAdapterEvent({ + type: 'started', + sessionId: SESSION, + fence: store.getRecord(SESSION)!.lease.runtimeFence, + acquisitionGeneration: 'generation-1', + reportedOptions: { model: 'default' }, + restoreSkippedOptions: [] + }) + } + async function dispose(): Promise { await host.flushAllStreamedEvents() await rm(root, { recursive: true, force: true }) @@ -278,6 +304,7 @@ export async function createQueuedMessageTestRig( queuePause, restartOffers, resume, + proveStart, dispose } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts index 592f82653750..62bc4e65519f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts @@ -3,9 +3,9 @@ // child back: the host accepts the message, and its delivery restarts the child and hands the // message to the new owner, instead of parking it behind a lease nothing would ever re-acquire. // -// A child is published before it has proven its start, and it owns the send from that moment: the -// message is handed to it. When the child exits first, the exit settlement rejects the message and -// writes the cause into the chat, once, and the next send is a fresh restart. +// A child is published before it has proven its start, and the host holds the send until it does, +// then hands it over. When the child exits first, the message was never handed over: it is rejected +// with the cause, written into the chat once, and the next send is a fresh restart. import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' @@ -64,7 +64,8 @@ function sendEnvelope( } } -/** A send is accepted at once and handed to the child delivery finds or starts. */ +/** A send is accepted at once; delivery finds or starts its child and holds it until that child + * proves its start. */ async function send( text: string, fence = store.getRecord(SESSION)?.lease.runtimeFence ?? 0 @@ -77,10 +78,16 @@ async function send( value: { submission: { dispatchState: 'pending', handoverRecorded: true } } }) const clientMessageId = sent.ok ? sent.value.clientMessageId : '' + await eventually(() => + expect(host.collaboratorsForTests().conversationDelivery.loop.isRunning(SESSION)).toBe(false) + ) + return clientMessageId +} + +async function handedOver(clientMessageId: string): Promise { await eventually(async () => expect((await submission(clientMessageId))?.handedOverAt).toBeDefined() ) - return clientMessageId } /** The child of the current acquisition, as the adapter would identify it in a lifecycle event. */ @@ -184,27 +191,31 @@ describe('a send into a published session whose child ended before startup', () expect(acquire).toHaveBeenCalledOnce() }) - it('restarts the child and admits the message against it before it has proven its start', async () => { + it('restarts the child and holds the message for it until it proves its start', async () => { const releasedFence = store.getRecord(SESSION)?.lease.runtimeFence ?? 0 const first = await send('hello again', releasedFence) - // Accepted at the lost owner's fence and handed to the child delivery started, once. + // Accepted at the lost owner's fence, held for the child delivery started. expect(acquire).toHaveBeenCalledTimes(2) expect(store.getRecord(SESSION)?.lease.claimStatus).toBe('live') - expect(dispatch).toHaveBeenCalledOnce() + expect(dispatch).not.toHaveBeenCalled() const current = store.getRecord(SESSION)?.lease.runtimeFence ?? 0 expect(current).toBeGreaterThan(releasedFence) - // Once proven, the next send meets a live owner and restarts nothing. The child echoes the first - // message, as a live one does, so the next is not held behind a turn still opening. + // Proven, the child is handed the message, once. await proveStarted() + await handedOver(first) + expect(dispatch).toHaveBeenCalledOnce() + + // The next send meets a live owner and restarts nothing. The child echoes the first message, + // as a live one does, so the next is not held behind a turn still opening. await host.settleLateDispatch({ sessionId: SESSION, clientMessageId: first, providerIdentity: { provider: 'codex', threadId: THREAD, turnId: 'turn-1', ordinal: 0 } }) - await send('and again', current) + await handedOver(await send('and again', current)) expect(acquire).toHaveBeenCalledTimes(2) expect(dispatch).toHaveBeenCalledTimes(2) }) @@ -216,8 +227,10 @@ describe('a send into a published session whose child ended before startup', () const held = await send('still not signed in', releasedFence) await exitBeforeProof() - // The child never proved its start, so it accepted nothing: the exit rejects the message this - // host admitted, so nothing pins the session and Retry stays offered, and one row names the cause. + // The child never proved its start, so it was handed nothing: the message is rejected, so + // nothing pins the session and Retry stays offered, and one row names the cause. + await eventually(async () => expect((await submission(held))?.dispatchState).toBe('rejected')) + expect((await submission(held))?.handedOverAt).toBeUndefined() expect(await submission(held)).toMatchObject({ dispatchState: 'rejected', reason: 'Codex stopped before it finished starting. Send your message to try again.', @@ -245,23 +258,27 @@ describe('a send into a published session whose child ended before startup', () // One spawn per user action: nothing restarted it a second time. expect(acquire).toHaveBeenCalledTimes(2) - // Retry is a fresh action: it restarts once and is handed to the new child. - await send('signed in now') + // Retry is a fresh action: it restarts once and is handed to the new child once it proves itself. + const retry = await send('signed in now') expect(acquire).toHaveBeenCalledTimes(3) - expect(dispatch).toHaveBeenCalledTimes(2) + await proveStarted() + await handedOver(retry) + expect(dispatch).toHaveBeenCalledOnce() expect((await journalStatuses()).slice(rowsBefore)).toHaveLength(1) }) }) describe('a send while the child of the first start is still proving itself', () => { - it('is handed to the starting child, and nothing restarts it', async () => { - await send('hello') + it('is held for the starting child, handed over once it proves its start, and restarts nothing', async () => { + const id = await send('hello') - expect(dispatch).toHaveBeenCalledOnce() + expect(dispatch).not.toHaveBeenCalled() expect(acquire).toHaveBeenCalledOnce() await proveStarted() + await handedOver(id) + expect(dispatch).toHaveBeenCalledOnce() expect(acquire).toHaveBeenCalledOnce() expect(await journalStatuses()).toEqual([]) }) @@ -272,6 +289,7 @@ describe('a send while the child of the first start is still proving itself', () await exitBeforeProof() + await eventually(async () => expect((await submission(held))?.dispatchState).toBe('rejected')) expect(await submission(held)).toMatchObject({ dispatchState: 'rejected', reason: 'Codex stopped before it finished starting. Send your message to try again.', @@ -288,7 +306,7 @@ describe('a send while the child of the first start is still proving itself', () it('leaves a send against a proven child alone', async () => { await proveStarted() - await send('hello') + await handedOver(await send('hello')) expect(acquire).toHaveBeenCalledOnce() expect(dispatch).toHaveBeenCalledOnce() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts index 6cef8ef9a1f1..1d2c47beb5f4 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts @@ -175,10 +175,11 @@ afterEach(async () => { }) describe('a start that fails before it proves itself', () => { - it("writes one row in the exit's words for a message the child was handed", async () => { + it("writes one row in the exit's words for a message held for the starting child", async () => { const sent = await send('hello') - // A starting child takes the message at once. - await eventually(() => expect(dispatch).toHaveBeenCalledOnce()) + await eventually(() => + expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('starting') + ) await exitBeforeProof() await eventually(async () => @@ -188,6 +189,7 @@ describe('a start that fails before it proves itself', () => { expect(await startRows()).toEqual([EXIT_TEXT]) expect(publishedStartRows()).toEqual([EXIT_TEXT]) + expect(dispatch).not.toHaveBeenCalled() }) it('leaves the row to the loop when the exit lands while the message still waits', async () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts new file mode 100644 index 000000000000..8bd4f3676f2c --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts @@ -0,0 +1,354 @@ +// The host's startup attempt and its hold on delivery, against the real host, store and journal +// with a scripted adapter. A send to a chat whose agent is still starting is accepted at once and +// handed over only once the start proves itself; every way a start ends settles what was held. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import { agentSessionFailureFact } from '../../../shared/agent-session-failure' +import { agentSessionFailureWords } from '../../../shared/agent-session-failure-words' +import type { AgentJournalSubmission } from '../../../shared/agent-session-journal-types' +import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' +import { codexProviderHandle } from '../../../shared/agent-session-provider-handle-encoding' +import { DISPATCH_REJECTED_CANCELLED } from '../../../shared/structured-agent-session-dispatch-rejection' +import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' +import { openTestJournalHostDatabase } from '../agent-session-journal/journal-host-database-test-support' +import type { + AgentSessionAcquisition, + StructuredAgentSessionAcquireInput, + StructuredAgentSessionAdapter +} from './structured-agent-session-adapter' +import { NO_STRUCTURED_AGENTS } from './structured-agent-session-adapter-router-test-support' +import { StructuredAgentSessionHost } from './structured-agent-session-host' +import { + HOST_TEST_NOW as NOW, + HOST_TEST_SESSION as SESSION, + HOST_TEST_THREAD as THREAD, + hostTestAttachParams, + hostTestMessage, + hostTestOperationId, + resetHostTestOperationIds +} from './structured-agent-session-host-test-data' +import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' +import { STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS } from './structured-agent-session-startup-attempt' + +const CALLER = { callerKey: 'client-1' } + +const eventually = (assertion: () => void | Promise): Promise => + vi.waitFor(assertion, { timeout: 10_000 }) + +let root: string +let store: AgentSessionRecordStore +let host: StructuredAgentSessionHost +let acquire: Mock +let dispatch: Mock +let closeSession: Mock> +let startupDeadlineMs: number | undefined + +function acquisition(input: StructuredAgentSessionAcquireInput): AgentSessionAcquisition { + return { + process: { + hostId: 'local', + pid: 4242, + processStartTimeMs: 1_700_000_000_000, + spawnToken: input.spawnToken + }, + acquisitionGeneration: `generation-${acquire.mock.calls.length}`, + link: { + linkId: `link-${input.fence}`, + handle: codexProviderHandle(THREAD), + origin: store.getRecord(SESSION)?.providerHandleChain.length ? 'resumed' : 'created', + mintedAtFence: input.fence, + observedAt: NOW + } + } +} + +/** An acquire that publishes at spawn: the child proves its start later, with `started`. */ +const publishFirst: StructuredAgentSessionAdapter['acquire'] = async (input) => ({ + ...acquisition(input), + providerChildPhase: 'starting' +}) + +async function startHost(): Promise { + host = new StructuredAgentSessionHost({ + agents: NO_STRUCTURED_AGENTS, + logger: createStructuredAgentSessionLogger(), + store, + adapter: { + acquire, + dispatch, + closeSession, + releaseAcquisition: vi.fn(async () => true), + cancelTurn: vi.fn(async () => ({ cancelled: false })), + answerPrompt: vi.fn(async () => undefined), + setOption: vi.fn(async () => undefined) + }, + journalDatabase: openTestJournalHostDatabase(root), + claimKeyId: 'key-1', + mintSpawnToken: () => `spawn-${acquire.mock.calls.length}`, + idleSweep: { intervalMs: 60 * 60_000, idleMs: 60 * 60_000 }, + ...(startupDeadlineMs === undefined ? {} : { startupDeadlineMs }), + now: () => NOW + }) +} + +/** A fresh host with no child, so the next send starts one through `acquire`. */ +async function restartWith(next: StructuredAgentSessionAdapter['acquire']): Promise { + await host.close(SESSION, 'evict') + await host.flushAllStreamedEvents() + acquire.mockImplementation(next) + closeSession.mockClear() + await startHost() +} + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-startup-attempt-')) + resetHostTestOperationIds() + startupDeadlineMs = undefined + acquire = vi.fn(async (input) => acquisition(input)) + dispatch = vi.fn(async () => ({ + state: 'accepted' as const, + providerIdentity: { + provider: 'codex' as const, + threadId: THREAD, + turnId: `turn-${dispatch.mock.calls.length}`, + ordinal: dispatch.mock.calls.length + } + })) + closeSession = vi.fn(async () => true) + store = await openTestAgentSessionRecordStore(root) + await startHost() + expect(await host.attach(CALLER, hostTestAttachParams(null))).toMatchObject({ ok: true }) +}) + +afterEach(async () => { + await host.flushAllStreamedEvents() + await rm(root, { recursive: true, force: true }) +}) + +function sendParams(text: string, clientOperationId = hostTestOperationId()) { + const body = hostTestMessage(text) + return { + envelope: { + sessionId: SESSION, + clientOperationId, + expectedRuntimeFence: 1, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.send', + sessionId: SESSION, + fields: { body } + }) + }, + body + } +} + +async function accept(text: string, clientOperationId?: string): Promise { + const params = sendParams(text, clientOperationId) + expect(await host.send(CALLER, params)).toMatchObject({ + ok: true, + value: { submission: { dispatchState: 'pending', handoverRecorded: true } } + }) + return params.envelope.clientOperationId +} + +async function submission(id: string): Promise { + return (await host.journalSnapshot(SESSION)).submissions.find( + (entry) => entry.clientMessageId === id + ) +} + +function startedEvent(generation = `generation-${acquire.mock.calls.length}`) { + return { + type: 'started' as const, + sessionId: SESSION, + fence: store.getRecord(SESSION)!.lease.runtimeFence, + acquisitionGeneration: generation, + reportedOptions: { model: 'default' }, + restoreSkippedOptions: [] + } +} + +/** The send's start has published its child, still starting; the host holds the send. */ +async function heldBehindStart(text: string): Promise { + const id = await accept(text) + await eventually(async () => + expect((await host.readStatusSummary(SESSION))?.hostExecutionPhase).toBe('starting') + ) + return id +} + +function stop() { + return host.cancel(CALLER, { + envelope: { + sessionId: SESSION, + clientOperationId: hostTestOperationId(), + expectedRuntimeFence: null, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.cancel', + sessionId: SESSION, + fields: { turnId: 'turn-none' } + }) + }, + turnId: 'turn-none' + }) +} + +const HOST_STOPPED = agentSessionFailureWords(agentSessionFailureFact('hostStopped'), { + agentName: 'Codex', + surface: 'rejection' +}) + +describe('a send while the agent starts', () => { + it('is accepted at once and handed over only once the start proves itself', async () => { + await restartWith(publishFirst) + const id = await heldBehindStart('hello') + + expect(await submission(id)).toMatchObject({ dispatchState: 'pending' }) + expect((await submission(id))?.handedOverAt).toBeUndefined() + expect(dispatch).not.toHaveBeenCalled() + + await host.handleAdapterEvent(startedEvent()) + + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('accepted')) + expect(dispatch).toHaveBeenCalledTimes(1) + }) + + it('hands every message held behind the start over, in order, once', async () => { + await restartWith(publishFirst) + const first = await heldBehindStart('first') + const second = await accept('second') + // The same id again is the same message: it is recorded and handed over once. + await accept('first', first) + + await host.handleAdapterEvent(startedEvent()) + + await eventually(async () => expect((await submission(second))?.dispatchState).toBe('accepted')) + expect(dispatch.mock.calls.map(([input]) => input.clientMessageId)).toEqual([first, second]) + }) + + it('ignores a start proof from a child it no longer holds', async () => { + await restartWith(publishFirst) + const id = await heldBehindStart('hello') + + await host.handleAdapterEvent(startedEvent('generation-stale')) + await host.handleAdapterEvent({ ...startedEvent(), fence: startedEvent().fence + 1 }) + await host.flushStreamedEvents(SESSION) + + expect(dispatch).not.toHaveBeenCalled() + expect((await host.readStatusSummary(SESSION))?.hostExecutionPhase).toBe('starting') + await host.handleAdapterEvent(startedEvent()) + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('accepted')) + }) +}) + +describe('a start that ends before it proves itself', () => { + it('settles what it held as never sent when the agent exits during startup', async () => { + await restartWith(publishFirst) + const id = await heldBehindStart('hello') + + await host.handleAdapterEvent({ + type: 'ended', + sessionId: SESSION, + fence: store.getRecord(SESSION)!.lease.runtimeFence, + acquisitionGeneration: `generation-${acquire.mock.calls.length}`, + reason: 'exited with code 1', + cause: 'unexpected-exit', + startupUnproven: true + }) + + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) + expect((await submission(id))?.handedOverAt).toBeUndefined() + expect(dispatch).not.toHaveBeenCalled() + }) + + it('withdraws what it held on a Stop, without waiting on the handshake', async () => { + await restartWith(publishFirst) + const id = await heldBehindStart('hello') + + expect(await stop()).toMatchObject({ ok: true, value: { cancelled: true } }) + + expect(await submission(id)).toMatchObject({ + dispatchState: 'rejected', + reason: DISPATCH_REJECTED_CANCELLED + }) + expect((await submission(id))?.handedOverAt).toBeUndefined() + expect(closeSession).toHaveBeenCalled() + expect(dispatch).not.toHaveBeenCalled() + }) + + it('keeps what it held for the chat on a close, without waiting on the handshake', async () => { + await restartWith(publishFirst) + const id = await heldBehindStart('hello') + + await host.close(SESSION, 'user-close') + + await host.revealSession(SESSION) + expect(await submission(id)).toMatchObject({ + dispatchState: 'rejected', + ...agentSessionFailureWords(agentSessionFailureFact('chatClosed'), { surface: 'rejection' }) + }) + expect((await submission(id))?.handedOverAt).toBeUndefined() + expect(dispatch).not.toHaveBeenCalled() + }) + + it('stops a published start that misses its deadline and fails what it held', async () => { + startupDeadlineMs = 50 + await restartWith(publishFirst) + const id = await heldBehindStart('hello') + + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) + expect(await submission(id)).toMatchObject(HOST_STOPPED) + expect((await submission(id))?.handedOverAt).toBeUndefined() + expect(closeSession).toHaveBeenCalled() + expect(dispatch).not.toHaveBeenCalled() + }) + + it('aborts an acquire still in its handshake at the deadline and fails what it held', async () => { + startupDeadlineMs = 50 + await restartWith( + (input) => + new Promise((_resolve, reject) => { + input.signal?.addEventListener('abort', () => reject(input.signal?.reason)) + }) + ) + const id = await accept('hello') + + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) + expect(await submission(id)).toMatchObject(HOST_STOPPED) + expect(dispatch).not.toHaveBeenCalled() + }) + + it('is not ended by the deadline once it proved itself', async () => { + startupDeadlineMs = 50 + await restartWith(publishFirst) + const id = await heldBehindStart('hello') + await host.handleAdapterEvent(startedEvent()) + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('accepted')) + + await new Promise((resolve) => setTimeout(resolve, 100)) + + expect(closeSession).not.toHaveBeenCalled() + expect((await host.readStatusSummary(SESSION))?.hostExecutionPhase).toBe('ready') + }) +}) + +describe('the attempt an acquire runs under', () => { + it('is minted by the host with the lease, the pinned launch and a fixed deadline', async () => { + await restartWith(publishFirst) + await heldBehindStart('hello') + + const record = store.getRecord(SESSION)! + const [input] = acquire.mock.calls.at(-1)! + expect(input).toMatchObject({ + fence: record.lease.runtimeFence, + launch: { location: record.location, accountHome: record.accountHome }, + deadlineAt: NOW + STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS + }) + expect(input.attemptId).toEqual(expect.any(String)) + expect(input.signal).toBeInstanceOf(AbortSignal) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts new file mode 100644 index 000000000000..4c29d6f8339c --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts @@ -0,0 +1,192 @@ +// Every agent this build registers starts under the host's one startup attempt and its hold on +// delivery. Iterates the runtime's real registration list, so an agent registered later is covered +// without being named here. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi, type Mock } from 'vitest' +import type { AgentSessionAccountHome } from '../../../shared/agent-session-account-home' +import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' +import type { AgentSessionProviderHandle } from '../../../shared/agent-session-provider-handle' +import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' +import { STRUCTURED_AGENT_RUNTIME_REGISTRATIONS } from '../../runtime/structured-agent-runtime-registrations' +import { openTestJournalHostDatabase } from '../agent-session-journal/journal-host-database-test-support' +import type { StructuredAgentDefinition } from './structured-agent-definition' +import { StructuredAgentRegistry } from './structured-agent-registry' +import type { + StructuredAgentSessionAcquireInput, + StructuredAgentSessionAdapter +} from './structured-agent-session-adapter' +import { StructuredAgentSessionHost } from './structured-agent-session-host' +import { + HOST_TEST_NOW as NOW, + HOST_TEST_SESSION as SESSION, + hostTestAttachParams, + hostTestMessage, + hostTestOperationId, + resetHostTestOperationIds +} from './structured-agent-session-host-test-data' +import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' +import { STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS } from './structured-agent-session-startup-attempt' + +const CALLER = { callerKey: 'client-1' } + +// Adapters whose own acquire already publishes at spawn, with the test that drives the real one. +// Every other registration still runs its handshake inside acquire (the temporary bridge) until +// its adapter moves to publish at spawn, which is when it joins this map. +const REAL_ADAPTER_PUBLISHES_AT_SPAWN: Readonly> = { + claude: 'structured-agent-session-claude-hung-start-stop.test.ts' +} + +let root: string | null = null + +afterEach(async () => { + if (root) { + await rm(root, { recursive: true, force: true }) + root = null + } +}) + +function accountHome(definition: StructuredAgentDefinition): AgentSessionAccountHome { + return definition.accountLocatorKind === 'opencode' + ? { kind: 'opencode', locator: { kind: 'unmanaged' } } + : { variable: definition.accountHomeVariable, path: `/home/dev/.${definition.agent}` } +} + +function handle(definition: StructuredAgentDefinition): AgentSessionProviderHandle { + return { transport: definition.handleTransport, agent: definition.agent, nativeId: 'native-1' } +} + +/** A host driving `definition` over a scripted adapter that publishes its child at spawn. */ +async function hostFor(definition: StructuredAgentDefinition): Promise<{ + host: StructuredAgentSessionHost + store: AgentSessionRecordStore + acquire: Mock + dispatch: Mock +}> { + root = await mkdtemp(join(tmpdir(), 'orca-startup-registrations-')) + resetHostTestOperationIds() + const store = await openTestAgentSessionRecordStore(root) + const acquire: Mock = vi.fn( + async (input: StructuredAgentSessionAcquireInput) => ({ + process: { + hostId: 'local', + pid: 4242, + processStartTimeMs: 1_700_000_000_000, + spawnToken: input.spawnToken + }, + acquisitionGeneration: `generation-${acquire.mock.calls.length}`, + providerChildPhase: 'starting' as const, + link: { + linkId: `link-${input.fence}`, + handle: handle(definition), + origin: 'created' as const, + mintedAtFence: input.fence, + observedAt: NOW + } + }) + ) + const dispatch: Mock = vi.fn(async () => ({ + state: 'admitted' as const + })) + const adapter: StructuredAgentSessionAdapter = { + acquire, + dispatch, + closeSession: vi.fn(async () => true), + releaseAcquisition: vi.fn(async () => true), + cancelTurn: vi.fn(async () => ({ cancelled: false })), + answerPrompt: vi.fn(async () => undefined), + setOption: vi.fn(async () => undefined) + } + const declared: StructuredAgentDefinition = { + ...definition, + // The double implements none of the optional capabilities' methods. + capabilities: { ...definition.capabilities, rewind: false, compact: false, threadGoal: false } + } + const host = new StructuredAgentSessionHost({ + agents: new StructuredAgentRegistry([{ definition: declared, adapter }]), + logger: createStructuredAgentSessionLogger(), + store, + adapter, + journalDatabase: openTestJournalHostDatabase(root), + claimKeyId: 'key-1', + mintSpawnToken: () => `spawn-${acquire.mock.calls.length}`, + now: () => NOW + }) + return { host, store, acquire, dispatch } +} + +describe.each( + STRUCTURED_AGENT_RUNTIME_REGISTRATIONS.map( + ({ definition }) => [definition.agent, definition] as const + ) +)('%s starts under the host attempt', (agent, definition) => { + it('acquires under a host-minted attempt and is handed nothing until it proves its start', async () => { + const { host, store, acquire, dispatch } = await hostFor(definition) + try { + const attached = await host.attach( + CALLER, + hostTestAttachParams(null, { + provider: agent, + agent, + accountHome: accountHome(definition), + // A new chat: the provider mints its handle at acquire. + providerHandle: undefined + }) + ) + expect(attached, JSON.stringify(attached)).toMatchObject({ ok: true }) + const record = store.getRecord(SESSION)! + expect(acquire.mock.calls[0]?.[0]).toMatchObject({ + attemptId: expect.any(String), + fence: record.lease.runtimeFence, + launch: { location: record.location, accountHome: record.accountHome }, + deadlineAt: NOW + STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS + }) + + const body = hostTestMessage('hello') + const sent = await host.send(CALLER, { + envelope: { + sessionId: SESSION, + clientOperationId: hostTestOperationId(), + expectedRuntimeFence: record.lease.runtimeFence, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.send', + sessionId: SESSION, + fields: { body } + }) + }, + body + }) + expect(sent).toMatchObject({ ok: true }) + await vi.waitFor(() => + expect(host.collaboratorsForTests().conversationDelivery.loop.isRunning(SESSION)).toBe( + false + ) + ) + expect(dispatch).not.toHaveBeenCalled() + + await host.handleAdapterEvent({ + type: 'started', + sessionId: SESSION, + fence: record.lease.runtimeFence, + acquisitionGeneration: 'generation-1', + reportedOptions: { model: 'default' }, + restoreSkippedOptions: [] + }) + + await vi.waitFor(() => expect(dispatch).toHaveBeenCalledOnce()) + } finally { + await host.flushAllStreamedEvents() + } + }) +}) + +describe.each( + STRUCTURED_AGENT_RUNTIME_REGISTRATIONS.map(({ definition }) => definition.agent).filter( + (agent) => !REAL_ADAPTER_PUBLISHES_AT_SPAWN[agent] + ) +)('%s with its own adapter', () => { + it.todo('publishes at spawn instead of finishing its handshake first') +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts new file mode 100644 index 000000000000..ab5f9ff4bfe8 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts @@ -0,0 +1,127 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import { + STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS, + StructuredAgentSessionStartupAttempts, + mintStructuredAgentSessionStartupAttempt, + type StructuredAgentSessionExpiredStartup +} from './structured-agent-session-startup-attempt' + +const SESSION = 'session-alpha' +const CHILD = { generation: 'generation-1', fence: 3 } + +let expired: StructuredAgentSessionExpiredStartup[] +let attempts: StructuredAgentSessionStartupAttempts + +function mint() { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the mint reads only the lease fence, location, account home, options and updatedAt this literal sets. + const record = { + lease: { runtimeFence: CHILD.fence }, + location: { executionHostId: 'local', workspaceId: 'workspace-1' }, + accountHome: { variable: 'CODEX_HOME', path: '/home/dev/.codex' }, + updatedAt: 1 + } as unknown as AgentSessionRecord + return mintStructuredAgentSessionStartupAttempt({ + record, + identity: { + sessionId: SESSION, + workspaceId: 'workspace-1', + hostId: 'local', + agent: 'codex', + providerHandle: null + }, + spawnToken: 'spawn-1', + now: Date.now() + }) +} + +beforeEach(() => { + vi.useFakeTimers() + expired = [] + attempts = new StructuredAgentSessionStartupAttempts({ + now: () => Date.now(), + expire: (startup) => expired.push(startup) + }) +}) + +afterEach(() => { + attempts.dispose() + vi.useRealTimers() +}) + +describe('a startup attempt deadline', () => { + it('counts from the mint and fires once, naming the acquire still in flight', () => { + const attempt = mint() + attempts.track(SESSION, attempt) + + vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS - 1) + expect(expired).toEqual([]) + vi.advanceTimersByTime(1) + + expect(expired).toEqual([{ sessionId: SESSION, attemptId: attempt.attemptId, child: null }]) + vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + expect(expired).toHaveLength(1) + }) + + it('names the starting child an acquire published', () => { + const attempt = mint() + attempts.track(SESSION, attempt) + attempts.published(SESSION, attempt.attemptId, { ...CHILD, phase: 'starting' }) + + vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + + expect(expired).toEqual([{ sessionId: SESSION, attemptId: attempt.attemptId, child: CHILD }]) + }) + + it('ends with the start its child proved, and ignores a stale child’s proof', () => { + const attempt = mint() + attempts.track(SESSION, attempt) + attempts.published(SESSION, attempt.attemptId, { ...CHILD, phase: 'starting' }) + + attempts.ready(SESSION, { ...CHILD, generation: 'generation-0' }) + vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS / 2) + attempts.ready(SESSION, CHILD) + vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + + expect(expired).toEqual([]) + }) + + it('ends with an acquire that answered ready, or an attach that failed', () => { + const ready = mint() + attempts.track(SESSION, ready) + attempts.published(SESSION, ready.attemptId, { ...CHILD, phase: 'ready' }) + const failed = mint() + attempts.track('session-beta', failed) + attempts.abandon('session-beta', failed.attemptId) + + vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + + expect(expired).toEqual([]) + }) + + it('expires a child published after its deadline passed, once it is published', () => { + const attempt = mint() + attempts.track(SESSION, attempt) + vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + + attempts.published(SESSION, attempt.attemptId, { ...CHILD, phase: 'starting' }) + + expect(expired.map((startup) => startup.child)).toEqual([null, CHILD]) + }) + + it('is replaced by the next attempt, and stops at quit', () => { + const first = mint() + attempts.track(SESSION, first) + vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS / 2) + const second = mint() + attempts.track(SESSION, second) + attempts.abandon(SESSION, first.attemptId) + + vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS / 2) + expect(expired).toEqual([]) + attempts.dispose() + vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + + expect(expired).toEqual([]) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts new file mode 100644 index 000000000000..fcbab3c90338 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts @@ -0,0 +1,190 @@ +// One start of a conversation's provider child, minted by the host before any adapter acquires. +// +// Whatever a start means as policy belongs to the host and is the same for every agent: which +// conversation and lease it is for, what cancels it, when it gives up, and when its child may take +// input. An adapter only runs its protocol inside the attempt it is handed. +// +// The deadline counts from the mint and nothing renews it: a start that has not proved itself by +// then is not coming. Expiry re-derives what to end from the host's own state, so a timer that +// outlives its attempt ends nothing. + +import { randomUUID } from 'node:crypto' +import type { AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import type { AgentChildWorkEvidence } from '../../../shared/agent-status-child-work-evidence' +import type { StructuredAgentSessionEventSink } from './structured-agent-session-event-sink' +import type { StructuredAgentSessionProviderChildIdentity } from './structured-agent-session-host-types' + +export const STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS = 60_000 + +export type StructuredAgentSessionStartupAttempt = { + /** The host's generation for this start; never reused, even when the lease fence is. */ + readonly attemptId: string + readonly identity: AgentSessionJournalIdentity + readonly fence: number + readonly spawnToken: string + /** Where and as whom the record pins the start; an adapter never re-resolves either. */ + readonly launch: Pick + /** The saved options the start launches with, as intent; never a catalog guess. */ + readonly options?: Readonly> + /** The record write `options` was read from: a pick after it is newer intent to reconcile. */ + readonly optionsAsOf: number + /** Provider events may begin before acquisition returns. */ + readonly events?: StructuredAgentSessionEventSink + /** Background work this start's child reports, scoped to the attempt. Unset until the host routes + * child work by attempt; adapters report through their registration until then. */ + readonly childWork?: (evidence: AgentChildWorkEvidence[]) => void + /** Aborted by a close, a Stop admitted now, quit, or the deadline: the adapter stops what it + * started and the acquire fails. */ + readonly signal?: AbortSignal + /** Host clock. */ + readonly deadlineAt: number +} + +export function mintStructuredAgentSessionStartupAttempt(input: { + record: AgentSessionRecord + identity: AgentSessionJournalIdentity + spawnToken: string + events?: StructuredAgentSessionEventSink + signal?: AbortSignal + now: number + deadlineMs?: number +}): StructuredAgentSessionStartupAttempt { + const { record } = input + return { + attemptId: randomUUID(), + identity: input.identity, + fence: record.lease.runtimeFence, + spawnToken: input.spawnToken, + launch: { + location: record.location, + accountHome: record.accountHome, + ...(record.launchDirectory === undefined ? {} : { launchDirectory: record.launchDirectory }) + }, + ...(record.options ? { options: record.options } : {}), + optionsAsOf: record.updatedAt, + ...(input.events ? { events: input.events } : {}), + ...(input.signal ? { signal: input.signal } : {}), + deadlineAt: input.now + (input.deadlineMs ?? STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + } +} + +/** The deadline's own reason, so an expired start is told from a close, Stop or quit. */ +export class StructuredAgentSessionStartupExpiredError extends Error { + constructor() { + super('the agent did not finish starting before its deadline') + this.name = 'StructuredAgentSessionStartupExpiredError' + } +} + +export function isStructuredAgentSessionStartupExpired(reason: unknown): boolean { + return reason instanceof Error && reason.name === 'StructuredAgentSessionStartupExpiredError' +} + +/** Where an expired attempt stood: still inside its acquire, or published as a starting child. */ +export type StructuredAgentSessionExpiredStartup = { + sessionId: string + attemptId: string + /** Null while the acquire has not returned. */ + child: StructuredAgentSessionProviderChildIdentity | null +} + +type Tracked = { + attempt: StructuredAgentSessionStartupAttempt + child: StructuredAgentSessionProviderChildIdentity | null + timer: ReturnType | null + expired: boolean +} + +/** Each session's open attempt and its deadline. A session starts one child at a time, so a new + * attempt replaces any older one. */ +export class StructuredAgentSessionStartupAttempts { + private readonly open = new Map() + private disposed = false + + constructor( + private readonly deps: { + now: () => number + /** Runs outside the session's queue: an acquire may hold it for the whole handshake. */ + expire: (expired: StructuredAgentSessionExpiredStartup) => void + } + ) {} + + track(sessionId: string, attempt: StructuredAgentSessionStartupAttempt): void { + this.end(sessionId) + const tracked: Tracked = { attempt, child: null, timer: null, expired: false } + this.open.set(sessionId, tracked) + if (this.disposed) { + return + } + tracked.timer = setTimeout( + () => this.expire(sessionId, tracked), + Math.max(0, attempt.deadlineAt - this.deps.now()) + ) + // A start's deadline must never be the reason a process stays alive at quit. + tracked.timer.unref?.() + } + + /** The acquire returned: a `starting` child stays on the deadline until it proves its start, and + * one published after its deadline passed is expired now. */ + published( + sessionId: string, + attemptId: string, + child: StructuredAgentSessionProviderChildIdentity & { phase: 'starting' | 'ready' } + ): void { + const tracked = this.open.get(sessionId) + if (tracked?.attempt.attemptId !== attemptId) { + return + } + if (child.phase === 'ready') { + this.end(sessionId) + return + } + tracked.child = { generation: child.generation, fence: child.fence } + if (tracked.expired) { + this.expire(sessionId, tracked) + } + } + + /** The child proved its start: its attempt is over. A stale child's proof ends nothing. */ + ready(sessionId: string, child: StructuredAgentSessionProviderChildIdentity): void { + const tracked = this.open.get(sessionId) + if (tracked?.child?.generation === child.generation && tracked.child.fence === child.fence) { + this.end(sessionId) + } + } + + /** The attempt failed, or its attach did: nothing it started is left to time out. */ + abandon(sessionId: string, attemptId: string): void { + if (this.open.get(sessionId)?.attempt.attemptId === attemptId) { + this.end(sessionId) + } + } + + /** Quit: no deadline fires after this. */ + dispose(): void { + this.disposed = true + for (const tracked of this.open.values()) { + if (tracked.timer) { + clearTimeout(tracked.timer) + } + } + this.open.clear() + } + + private expire(sessionId: string, tracked: Tracked): void { + if (this.open.get(sessionId) !== tracked || this.disposed) { + return + } + tracked.expired = true + this.deps.expire({ sessionId, attemptId: tracked.attempt.attemptId, child: tracked.child }) + } + + private end(sessionId: string): void { + const tracked = this.open.get(sessionId) + if (tracked?.timer) { + clearTimeout(tracked.timer) + } + this.open.delete(sessionId) + } +} 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 9c9722de00bb..c09d2eba456c 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 @@ -108,7 +108,7 @@ async function queuedDraft(text: string): Promise { * behind the start, which the Stop holds. */ async function stopOfStart(options: { held?: true } = {}): Promise { rig = await createQueuedMessageTestRig({ starting: true, restartable: true }) - // Handed over at once to a child that never proves its start; nothing echoes it. + // Held for a child that never proves its start, so it is never handed over. rig.send('work on this') await eventually(() => expect(childPhase()).toBe('starting')) const held = options.held ? await queuedDraft('queued behind the start') : undefined @@ -124,6 +124,7 @@ async function mailTurn(): Promise { const handedOver = rig.dispatch.mock.calls.length const mail = rig.send('mail for the worker') await mail.result + await rig.proveStart() await eventually(() => expect(rig.dispatch).toHaveBeenCalledTimes(handedOver + 1)) await turnOpenedBy(mail.id) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event.test.ts index 1e29b5ad1113..ea0dae59199e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-stop-event.test.ts @@ -126,9 +126,13 @@ describe("a Stop's event", () => { it('at an agent still starting, reaches the journal before the start is ended, and holds a card queued before it', async () => { rig = await createQueuedMessageTestRig({ starting: true, restartable: true }) - // A starting agent takes the send at once; it never proves its start. + // Held for a starting agent that never proves its start. rig.send('work on this') - await eventually(() => expect(rig.dispatch).toHaveBeenCalledTimes(1)) + await eventually(() => + expect(rig.host.collaboratorsForTests().sessions.get(HOST_TEST_SESSION)?.child?.phase).toBe( + 'starting' + ) + ) const held = await queuedDraft('queued while it starts') let atEnd: JournalStopEvent[] | undefined rig.closeSession.mockImplementationOnce(async () => { @@ -140,6 +144,7 @@ describe("a Stop's event", () => { expect(await rig.handoff(held)).toBeUndefined() expect(await rig.queuePause()).toEqual({ reason: 'stopped' }) expect(rig.cancelTurn).not.toHaveBeenCalled() + expect(rig.dispatch).not.toHaveBeenCalled() expect(rig.closeSession).toHaveBeenCalledTimes(1) expect(atEnd).toEqual([ { reason: 'user-stop', caller: QUEUED_RIG_CALLER.callerKey, at: expect.any(Number) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts index 200ce5a338ab..17ccba7938ef 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts @@ -28,10 +28,8 @@ import type { AgentSessionJournal } from '../agent-session-journal/journal-store import type { JournalOperationReceipt } from '../agent-session-journal/journal-row-writer' import type { AgentSessionDispatchOutcome, - StructuredAgentSessionAdapter, - StructuredAgentSessionProviderChildPhase + StructuredAgentSessionAdapter } from './structured-agent-session-adapter' -import { structuredAgentSessionStartFailure } from './structured-agent-session-failure-text' import { agentJournalSubmissionKey } from '../../../shared/agent-session-journal-item-key' import { handOverStructuredAgentSessionCommand, @@ -67,8 +65,6 @@ export type AgentSessionTurnContext = { /** Republishes state kept outside the journal, such as the record's options or rewind phase. * Journal appends reach readers on their own. */ publish: () => void - /** What the host holds about the child this dispatch is for, read at the moment it is needed. */ - providerChildPhase?: () => StructuredAgentSessionProviderChildPhase | undefined /** Who a Stop's refusal row names. */ failureTextContext?: AgentSessionFailureWordsContext /** The operation's success, committed with the row that accepts it (`MutationPlan.settlesWithWrite`). */ @@ -98,10 +94,10 @@ export function agentSessionAttachmentExpiredRefusal(): { ) } -/** A thrown adapter error is indistinguishable from a lost reply, so it settles as `unknown` - * rather than as a rejection — unless the child had not proven its start. A dispatch to such a - * child throws only when the start failed before the write (a write's own failure is an outcome, - * not a throw), so it is provably unwritten and is rejected with the cause the adapter gave. */ +/** A thrown adapter error is indistinguishable from a lost reply, so it settles as `unknown`, never + * replayed. Only a child that proved its start is handed anything, so a start that fails leaves its + * messages unsent instead (the delivery loop's barrier); an adapter that knows a write never + * happened answers `rejected`. */ async function dispatchSafely( ctx: AgentSessionHandoverContext, clientMessageId: string, @@ -117,12 +113,6 @@ async function dispatchSafely( requestedAt }) } catch (error) { - if (ctx.providerChildPhase?.() === 'starting') { - return { - state: 'rejected', - ...structuredAgentSessionStartFailure({ error }, ctx.failureTextContext) - } - } return { state: 'unknown', reason: error instanceof Error ? error.message : String(error) } } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unrun-send-hold.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unrun-send-hold.test.ts index fe6a29cdcae6..e4de581bf067 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-unrun-send-hold.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-unrun-send-hold.test.ts @@ -1,9 +1,8 @@ -// A message handed to an agent whose start never answers provably never ran: its CLI takes no -// message before it answers initialize. So when the chat ends then, the message is settled as a +// A message sent while the agent's start never answers provably never ran: the host hands a child +// nothing until it proves its start. So when the chat ends then, the message is settled as a // queued one is for the same end (`journal-unsent-send-hold.ts`): a quit or a close keeps a // person's words as an ordinary card that waits for the chat's next turn, a person's Stop -// withdraws it. Against the real host, store and -// journal, with an agent that stays starting. +// withdraws it. Against the real host, store and journal, with an agent that stays starting. import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { agentSessionFailureFact } from '../../../shared/agent-session-failure' @@ -46,20 +45,23 @@ afterEach(async () => { await rig.dispose() }) -/** Sent to the starting agent and handed over to it, never echoed. */ -async function handedToHungStart(text: string): Promise { +/** Sent while the agent starts, and held: never handed over. */ +async function heldBehindHungStart(text: string): Promise { // No child, so this send starts one that never answers. await rig.host.close(SESSION, 'evict') const sent = rig.send(text) await sent.result - await eventually(async () => expect((await rig.submission(sent.id))?.handedOverAt).toBeDefined()) - expect(rig.host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('starting') + await eventually(() => + expect(rig.host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('starting') + ) + expect((await rig.submission(sent.id))?.handedOverAt).toBeUndefined() + expect(rig.dispatch).not.toHaveBeenCalled() return sent.id } -describe('a message handed to a start that never answered, then the chat ends', () => { +describe('a message held behind a start that never answered, then the chat ends', () => { it('is a card after a quit, rejected as a restart, held by the reopen until a turn', async () => { - const id = await handedToHungStart('kept through quit') + const id = await heldBehindHungStart('kept through quit') await rig.quitRestartHostProcess() expect(await rig.submission(id)).toMatchObject({ @@ -76,7 +78,7 @@ describe('a message handed to a start that never answered, then the chat ends', expect(await rig.restartOffers()).toEqual([]) }) - it('is offered for resume after a quit once the start answered, as the control', async () => { + it('is offered for resume after a quit once the start proved itself, as the control', async () => { await rig.dispose() rig = await createQueuedMessageTestRig({ restartable: true, @@ -84,7 +86,14 @@ describe('a message handed to a start that never answered, then the chat ends', stopEndsSession: true, recoveryCapsule: true }) - const id = await handedToHungStart('may have run') + await rig.host.close(SESSION, 'evict') + const sent = rig.send('may have run') + await sent.result + await rig.proveStart() + await eventually(async () => + expect((await rig.submission(sent.id))?.handedOverAt).toBeDefined() + ) + const id = sent.id await rig.quitRestartHostProcess() expect(await rig.restartOffers()).toEqual([ @@ -95,7 +104,7 @@ describe('a message handed to a start that never answered, then the chat ends', it.each(['user-close', 'evict'] as const)( 'is a card after a %s, rejected as closed, held by the reopen until a turn', async (cause) => { - const id = await handedToHungStart('kept at close') + const id = await heldBehindHungStart('kept at close') await rig.host.close(SESSION, cause) rig.crashRestartHostProcess() @@ -110,7 +119,7 @@ describe('a message handed to a start that never answered, then the chat ends', ) it("is withdrawn by a person's Stop, as a queued one is, and kept as no card", async () => { - const id = await handedToHungStart('stopped') + const id = await heldBehindHungStart('stopped') expect(await rig.stop()).toMatchObject({ ok: true, value: { cancelled: true } }) diff --git a/src/main/runtime/claude-structured-send-during-startup.test.ts b/src/main/runtime/claude-structured-send-during-startup.test.ts index a50615fbed4a..494d012de462 100644 --- a/src/main/runtime/claude-structured-send-during-startup.test.ts +++ b/src/main/runtime/claude-structured-send-during-startup.test.ts @@ -1,10 +1,10 @@ // A Claude chat is published the moment its child spawns, before the CLI has answered initialize, // and launched with the chat's saved options. A send in that window — into a fresh start, or into -// the restart the delivery loop makes for a send after a start that failed — is written to the CLI -// at once, which takes it behind its own start. When the CLI dies before answering, the message is -// rejected with the CLI's own diagnostic, the chat shows the cause once, and nothing is left as a -// delivery nobody can confirm. Against the production runtime, adapter, record store and host, with -// only the CLI process scripted. +// the restart the delivery loop makes for a send after a start that failed — is accepted at once and +// held by the host until the start proves itself, then written once. When the CLI dies before +// answering, the never-written message is rejected with the CLI's own diagnostic, the chat shows the +// cause once, and nothing is left as a delivery nobody can confirm. Against the production runtime, +// adapter, record store and host, with only the CLI process scripted. import { afterEach, describe, expect, it, vi } from 'vitest' import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' @@ -57,6 +57,14 @@ function fence(host: StructuredAgentSessionHost): number { return host.deps.store.getRecord(SESSION)?.lease.runtimeFence ?? 0 } +/** Whether the CLI was written a message, once the delivery loop has settled what it does. */ +async function written(host: StructuredAgentSessionHost): Promise { + await vi.waitFor(() => + expect(host.collaboratorsForTests().conversationDelivery.loop.isRunning(SESSION)).toBe(false) + ) + return claude.child(SESSION).calls.includes('send') +} + async function statusRows(host: StructuredAgentSessionHost): Promise { return (await host.journalSnapshot(SESSION)).items.flatMap((item) => item.body.kind === 'status' ? [item.body.text] : [] @@ -80,7 +88,7 @@ async function failLatestStart(host: StructuredAgentSessionHost, count: number): } describe('a send into a Claude chat whose CLI keeps failing at startup', () => { - it('restarts once, rejects the message it wrote with the diagnostic when that start dies too, then delivers once the CLI is healthy', async () => { + it('restarts once, rejects the message held for it with the diagnostic when that start dies too, then delivers once the CLI is healthy', async () => { claude.behave(SESSION, { initHangs: true }) const host = await claude.install() await expect(host.attach(CALLER, claude.attachParams(SESSION, null))).resolves.toMatchObject({ @@ -90,14 +98,14 @@ describe('a send into a Claude chat whose CLI keeps failing at startup', () => { expect(await statusRows(host)).toEqual([STARTUP_TEXT]) const releasedFence = fence(host) - // The delivery loop asks for the child back and writes the message to it at once; the CLI - // dies again before it answers initialize. + // The delivery loop asks for the child back and holds the message for it; the CLI dies again + // before it answers initialize, so it was never written. const held = await send(host, 'hello?') await vi.waitFor(() => expect(claude.children(SESSION)).toHaveLength(2)) - await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) await vi.waitFor(() => expect(host.deps.store.getRecord(SESSION)?.lease.claimStatus).toBe('live') ) + expect(await written(host)).toBe(false) await failLatestStart(host, 2) // Rejected with the cause, not left in doubt; one row for this attempt names it. @@ -128,21 +136,22 @@ describe('a send into a Claude chat whose CLI keeps failing at startup', () => { }) describe('a send while the first Claude start is still answering initialize', () => { - it('is written to the CLI at once, before it answers', async () => { + it('is held from the CLI until it answers, then written once', async () => { claude.behave(SESSION, { initHangs: true }) const host = await claude.install() await host.attach(CALLER, claude.attachParams(SESSION, null)) const sent = await send(host, 'hello') - await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) + expect(await written(host)).toBe(false) expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('starting') - // The CLI answers: the start lands under the message already written, and nothing fails. + // The CLI answers: the start lands, and the held message is written to it, once. claude.child(SESSION).answerInit() await vi.waitFor(() => expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('ready') ) + await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) expect(claude.child(SESSION).calls.filter((call) => call === 'send')).toHaveLength(1) expect((await submission(host, sent))?.dispatchState).not.toBe('rejected') expect(claude.children(SESSION)).toHaveLength(1) @@ -151,18 +160,19 @@ describe('a send while the first Claude start is still answering initialize', () // Only a SessionStart hook makes the CLI name its session before the first turn, and that hook // comes from status hooks a user can turn off; the initialize answer alone starts the chat. - it('is written at once, and the start lands, with no start frame before the first turn', async () => { + it('lands its start and is written, with no start frame before the first turn', async () => { claude.behave(SESSION, { initHangs: true, sendsNoStartFrame: true }) const host = await claude.install() await host.attach(CALLER, claude.attachParams(SESSION, null)) await send(host, 'hello') - await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) + expect(await written(host)).toBe(false) claude.child(SESSION).answerInit() await vi.waitFor(() => expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('ready') ) + await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) expect(claude.children(SESSION)).toHaveLength(1) expect(await statusRows(host)).toEqual([]) }) @@ -174,7 +184,7 @@ describe('a send while the first Claude start is still answering initialize', () const startedFence = fence(host) const held = await send(host, 'hello') - await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) + expect(await written(host)).toBe(false) await failLatestStart(host, 1) await vi.waitFor(async () => diff --git a/src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts b/src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts index 3eb815bca50e..a05905e9599c 100644 --- a/src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts +++ b/src/main/runtime/claude-structured-send-restart-dies-before-dispatch.test.ts @@ -1,6 +1,6 @@ // A send is accepted into a chat whose Claude child is gone, and its delivery restarts the child -// and writes the message to it at once. When that child dies before it proves its start, it never -// ran the message, so the send settles `rejected` with the child's own diagnostic, never as a +// and holds the message until that child proves its start. When it dies first, it was never handed +// the message, so the send settles `rejected` with the child's own diagnostic, never as a // delivery nobody can confirm, and a client that was subscribed the whole time receives the // failure row and the rejected submission over the wire. Against the production runtime, adapter, // record store and host, with only the CLI scripted. @@ -141,7 +141,10 @@ describe('a send whose restarted Claude child dies before it proves its start', // Accepted: the answer comes before the restart it needs. expect(answered.get(sent)).toBe('pending') await eventually(() => expect(claude.children(SESSION)).toHaveLength(2)) - await eventually(() => expect(claude.child(SESSION).calls).toContain('send')) + await eventually(() => + expect(host.collaboratorsForTests().conversationDelivery.loop.isRunning(SESSION)).toBe(false) + ) + expect(claude.child(SESSION).calls).not.toContain('send') await failLatestStart(host, 2) // Provably not delivered, with the cause; not "unconfirmed". diff --git a/src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts b/src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts index 65215d609966..81b3b9c19535 100644 --- a/src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts +++ b/src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts @@ -94,14 +94,17 @@ describe('a Claude start that Orca fails while the CLI is still running', () => expect(await failureRows(host)).toEqual([]) }) - it('rejects a message it wrote as a start that could not happen when init names another session', async () => { + it('rejects a message held for it as a start that could not happen when init names another session', async () => { claude.behave(SESSION, { initHangs: true, initNamesForeignSession: true }) const host = await claude.install() await expect(host.attach(CALLER, claude.attachParams(SESSION, null))).resolves.toMatchObject({ ok: true }) const held = await send(host, 'hello') - await vi.waitFor(() => expect(claude.child(SESSION).calls).toContain('send')) + await vi.waitFor(() => + expect(host.collaboratorsForTests().conversationDelivery.loop.isRunning(SESSION)).toBe(false) + ) + expect(claude.child(SESSION).calls).not.toContain('send') claude.child(SESSION).answerInit() await released(host) @@ -116,6 +119,7 @@ describe('a Claude start that Orca fails while the CLI is still running', () => expect(await failureRows(host)).toEqual([ { text: expect.stringMatching(/^Claude couldn't start\./), kind: 'startFailed' } ]) + expect(claude.child(SESSION).calls).not.toContain('send') }) it('still says Claude stopped when the CLI exits on its own before its start lands', async () => { From a6d591fa08b2ead3be988e8be37b73c372467746 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 7 Oct 2026 22:10:36 -0700 Subject: [PATCH 2/5] fix(native-chat): time agent starts from spawn by progress, and release Claude on initialize Review round 1 on the host-owned startup attempt. - The startup clock now starts when the process exists and fails a start only after 60 s with no stdout frame or stderr line, or 10 minutes after spawn however chatty it is. Every agent's child reports output through the one managed-process spawn, so a slow migration or history replay is no longer killed mid-way. Each start's spawn-to-outcome time is recorded as an `agentSession.startup` span for tuning. - Claude reports `started` on its initialize answer; the settings read follows and is applied and persisted as a later `options-reported` update, so messages no longer wait on it. - A started child's reported options are persisted in their own step after the held messages are handed over, and only if no pick changed the record's options since. - An attempt now ends whenever its child ends or an expiry stopped it. - Removed `optionsAsOf`, which was not an option revision, and comments that described the old publish-and-write-at-once behaviour. --- src/main/acp/acp-agent-connection.ts | 8 +- src/main/acp/acp-structured-acquire.ts | 1 + .../claude/claude-agent-sdk-process-spawn.ts | 6 +- .../claude/claude-stream-json-connection.ts | 4 +- .../claude-structured-applied-options.test.ts | 9 +- .../claude/claude-structured-real-cli.test.ts | 61 +++++ .../claude-structured-session-acquisition.ts | 7 +- .../claude-structured-session-adapter.test.ts | 2 +- ...claude-structured-session-startup-state.ts | 5 +- .../claude-structured-session-startup.test.ts | 32 +++ .../claude-structured-session-startup.ts | 203 ++++++++++------ .../claude/claude-structured-session-state.ts | 4 +- .../codex-app-server-connection-types.ts | 2 + src/main/codex/codex-app-server-connection.ts | 3 +- .../codex/codex-structured-session-acquire.ts | 1 + src/main/jsonl-rpc/agent-connection.ts | 5 +- ...structured-agent-session-acquire-aborts.ts | 2 +- .../structured-agent-session-acquisition.ts | 9 +- .../structured-agent-session-adapter.ts | 8 + .../structured-agent-session-agent-start.ts | 2 +- .../structured-agent-session-attach-flow.ts | 13 +- ...ured-agent-session-attach-orchestration.ts | 10 +- .../structured-agent-session-child-exit.ts | 4 + ...red-agent-session-conversation-lifetime.ts | 4 +- .../structured-agent-session-delivery-loop.ts | 21 +- ...structured-agent-session-event-recovery.ts | 7 +- ...ctured-agent-session-host-runtime-state.ts | 6 +- .../structured-agent-session-host-types.ts | 6 +- ...ructured-agent-session-provider-started.ts | 132 +++++++---- ...agent-session-startup-attempt-host.test.ts | 111 ++++++++- ...sion-startup-attempt-registrations.test.ts | 9 +- ...ured-agent-session-startup-attempt.test.ts | 190 +++++++++++---- ...tructured-agent-session-startup-attempt.ts | 224 ++++++++++++++---- .../agent-session-instrumentation.ts | 19 +- src/main/pi/rpc-session.ts | 1 + .../managed-provider-process-output.test.ts | 58 +++++ .../managed-provider-process.ts | 23 ++ .../structured-agent-session-at-rest.test.ts | 2 +- .../structured-claude-runtime-adapter.ts | 6 +- 39 files changed, 956 insertions(+), 264 deletions(-) create mode 100644 src/main/provider-process/managed-provider-process-output.test.ts diff --git a/src/main/acp/acp-agent-connection.ts b/src/main/acp/acp-agent-connection.ts index d4f7eaf2ec25..b7a20520941c 100644 --- a/src/main/acp/acp-agent-connection.ts +++ b/src/main/acp/acp-agent-connection.ts @@ -13,6 +13,8 @@ export type AcpAgentConnectionOptions = Omit & peer?: Omit /** Process exit evidence, including expected closes and processless spawn failures. */ onExit?: (error: Error, context: { expected: boolean; exit: ProviderProcessExit }) => void + /** Any stdout or stderr chunk from the child. */ + onOutput?: () => void } /** The execution host owns the child and protocol lifetime; the adapter owns turns and Stop. */ @@ -37,7 +39,11 @@ export class AcpAgentConnection extends AcpSessionRuntime { ) { // Validate before spawning so invalid limits cannot leave an unowned child. const peer = resolveAcpPeerOptions({ ...options.peer, closeOnInputEnd: false }) - const managed = spawnManagedProviderProcess(launch, { spawnImpl, site: 'acp-agent-teardown' }) + const managed = spawnManagedProviderProcess(launch, { + spawnImpl, + site: 'acp-agent-teardown', + ...(options.onOutput ? { onOutput: options.onOutput } : {}) + }) const lifecycle: { closing: boolean; error?: Error } = { closing: false } const diagnose = (message: string): void => { try { diff --git a/src/main/acp/acp-structured-acquire.ts b/src/main/acp/acp-structured-acquire.ts index b9c8b41d209f..45097b006ee6 100644 --- a/src/main/acp/acp-structured-acquire.ts +++ b/src/main/acp/acp-structured-acquire.ts @@ -128,6 +128,7 @@ export async function acquireAcpStructuredSession(input: { }, { clientInfo: { name: 'orca', version: '1' }, + ...(acquire.onOutput ? { onOutput: acquire.onOutput } : {}), onPermission: (request, context) => { if (!session?.turns.acceptsRequests) { // No prompt of Orca's runs (a turn the agent began itself included), or a Stop or steer diff --git a/src/main/claude/claude-agent-sdk-process-spawn.ts b/src/main/claude/claude-agent-sdk-process-spawn.ts index 23bbe5950a0c..c117ee09b665 100644 --- a/src/main/claude/claude-agent-sdk-process-spawn.ts +++ b/src/main/claude/claude-agent-sdk-process-spawn.ts @@ -46,7 +46,8 @@ function definedEnv(env: Record): Record void ): ClaudeCodeProcessSpawn { let managed: ManagedProviderProcess | null = null return { @@ -64,7 +65,8 @@ export function createClaudeCodeProcessSpawn( inheritedEnv: definedEnv(options.env), site: 'claude-stream-json-teardown', policy: (supervised) => claudeChildClosePolicy(supervised, platform), - acceptClose: claudeChildCloseProven + acceptClose: claudeChildCloseProven, + ...(onOutput ? { onOutput } : {}) } ) // The SDK drains stderr only for its own local spawn; the managed process drains it here. diff --git a/src/main/claude/claude-stream-json-connection.ts b/src/main/claude/claude-stream-json-connection.ts index bbf0db1c115d..56f18013d7a9 100644 --- a/src/main/claude/claude-stream-json-connection.ts +++ b/src/main/claude/claude-stream-json-connection.ts @@ -70,6 +70,8 @@ export type ClaudeStreamJsonConnectionHandlers = { /** 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 + /** Any stdout or stderr chunk from the child. */ + onOutput?: () => void } /** @@ -124,7 +126,7 @@ export async function openClaudeStreamJsonConnection( queryImpl?: typeof ClaudeAgentSdk.query ): Promise { const { query } = await loadClaudeAgentSdk() - const spawner = createClaudeCodeProcessSpawn(spawnImpl) + const spawner = createClaudeCodeProcessSpawn(spawnImpl, process.platform, handlers.onOutput) const inbox = createClaudeUserMessageQueue() const session = (queryImpl ?? query)({ prompt: inbox.messages, diff --git a/src/main/claude/claude-structured-applied-options.test.ts b/src/main/claude/claude-structured-applied-options.test.ts index 1c3d0be57742..f4043c1fd489 100644 --- a/src/main/claude/claude-structured-applied-options.test.ts +++ b/src/main/claude/claude-structured-applied-options.test.ts @@ -125,8 +125,9 @@ describe('Claude model before the first turn', () => { const events: ClaudeStructuredSessionEvent[] = [] await acquired(startedWithoutATurn(SETTINGS.settingsHaiku), {}, events) - const started = events.find((event) => event.type === 'started') - expect(started).toMatchObject({ reportedOptions: { model: 'haiku' } }) + // The settings read follows `started`; what it shows is persisted as the later report. + const reported = events.find((event) => event.type === 'options-reported') + expect(reported).toMatchObject({ reportedOptions: { model: 'haiku' } }) }) it('shows the applied effort but never persists it as the session options', async () => { @@ -139,6 +140,10 @@ describe('Claude model before the first turn', () => { expect(started?.type === 'started' ? started.reportedOptions : null).not.toHaveProperty( 'effort' ) + const reported = events.find((event) => event.type === 'options-reported') + expect( + reported?.type === 'options-reported' ? reported.reportedOptions : null + ).not.toHaveProperty('effort') const { current } = await adapter.readOptions({ sessionId: 'session-1', fence: 7 }) expect(current.effort).toBe('medium') }) diff --git a/src/main/claude/claude-structured-real-cli.test.ts b/src/main/claude/claude-structured-real-cli.test.ts index 0e9345537bb1..96a8c7a1803d 100644 --- a/src/main/claude/claude-structured-real-cli.test.ts +++ b/src/main/claude/claude-structured-real-cli.test.ts @@ -338,6 +338,67 @@ describe.skipIf(!realClaudeAvailable)(suiteTitle, () => { 90_000 ) + // The host hands a Claude child nothing until `started`, which now follows the initialize answer + // alone. Measures what that wait costs against the live binary, so the startup limits are tuned + // from data: spawn to `started` with nothing sent, then a send to its first output. + it.skipIf(!realClaudeAuthenticated)( + 'reports `started` with nothing sent, then answers a send handed over after it', + async () => { + const providerSessionId = randomUUID() + const claudeConfigDir = process.env.CLAUDE_CONFIG_DIR?.trim() || join(homedir(), '.claude') + const events: ClaudeStructuredSessionEvent[] = [] + const adapter = realAdapter( + providerSessionId, + claudeConfigDir, + events, + process.cwd(), + undefined, + realClaudeLaunchHome().env, + false + ) + const until = async (done: () => boolean, timeoutMs: number): Promise => { + const deadline = Date.now() + timeoutMs + while (!done() && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 20)) + } + return done() + } + try { + const acquireAt = Date.now() + await adapter.acquire({ + identity: identity(providerSessionId), + fence: 1, + spawnToken: 'real-cli-started-timing' + }) + const publishedAt = Date.now() + expect(await until(() => events.some((event) => event.type === 'started'), 30_000)).toBe( + true + ) + const startedAt = Date.now() + const before = events.length + await adapter.dispatch({ + sessionId: 'real-cli-handshake', + clientMessageId: 'real-cli-started-timing-1', + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'Say ok' }] }, + fence: 1 + }) + expect( + await until(() => events.slice(before).some((event) => event.type === 'message'), 60_000) + ).toBe(true) + const firstOutputAt = Date.now() + // Reported for tuning; the assertions above are the contract. + console.info( + `[real-cli startup timing] acquire->published ${publishedAt - acquireAt} ms, ` + + `published->started ${startedAt - publishedAt} ms, ` + + `dispatch->first output ${firstOutputAt - startedAt} ms` + ) + } finally { + await adapter.closeAll() + } + }, + 120_000 + ) + // The contract a chat's first message depends on: saved options ride the launch, and the // message is written as soon as the child is published, before the CLI answers initialize. it.skipIf(!realClaudeAuthenticated)( diff --git a/src/main/claude/claude-structured-session-acquisition.ts b/src/main/claude/claude-structured-session-acquisition.ts index b10af618808b..98defecd713f 100644 --- a/src/main/claude/claude-structured-session-acquisition.ts +++ b/src/main/claude/claude-structured-session-acquisition.ts @@ -24,6 +24,7 @@ import { settleClaudeSessionStartup } from './claude-structured-session-startup' import { createClaudeSessionPublication } from './claude-structured-session-publication' +import { readClaudeStructuredSessionSettings } from './claude-structured-session-acquisition-options' import { mintClaudeAcquisitionGeneration, type ClaudeAcquisitionRegistry, @@ -183,6 +184,7 @@ export async function acquireClaudeSession({ onMessage, canUseTool, onUserDialog, + ...(input.onOutput ? { onOutput: input.onOutput } : {}), onFault: (error) => { childEnded ??= error initProof.reject(error) @@ -275,8 +277,10 @@ export async function acquireClaudeSession({ requestTimeoutMs: deps.requestTimeoutMs, emit }), + readSettings: () => readClaudeStructuredSessionSettings(connection, deps.requestTimeoutMs), isCurrent: () => sessions.get(sessionId) === session, fault: (error) => callbacks.handleExit(sessionId, attempt, error), + diagnose: (diagnostic) => emit({ type: 'auth-diagnostic', sessionId, diagnostic }), report: (event) => emit({ ...event, @@ -293,8 +297,7 @@ export async function acquireClaudeSession({ exits.get(sessionId)?.error ?? new Error('claude session ended before acquisition returned') ) } - // The start reads its facts only after publish, so the child is `starting` until `started` - // says otherwise; it already takes input. + // The start reads its facts only after publish, so the child is `starting` until `started`. return { ...publication.acquisition, providerChildPhase: 'starting' } } catch (error) { unbindReadingControl?.() diff --git a/src/main/claude/claude-structured-session-adapter.test.ts b/src/main/claude/claude-structured-session-adapter.test.ts index cc59dfde3017..0897917718da 100644 --- a/src/main/claude/claude-structured-session-adapter.test.ts +++ b/src/main/claude/claude-structured-session-adapter.test.ts @@ -130,7 +130,7 @@ describe('ClaudeStructuredSessionAdapter.acquire', () => { ).toEqual([]) // Left out, not refused: the record takes the provider's own Fast, as before. expect(adapter.readOptionRestoreFailures('session-1')).toEqual([]) - expect(events.find((event) => event.type === 'started')).toMatchObject({ + expect(events.find((event) => event.type === 'options-reported')).toMatchObject({ reportedOptions: { fastMode: false } }) }) diff --git a/src/main/claude/claude-structured-session-startup-state.ts b/src/main/claude/claude-structured-session-startup-state.ts index ba4107fa7f4c..b922efd32805 100644 --- a/src/main/claude/claude-structured-session-startup-state.ts +++ b/src/main/claude/claude-structured-session-startup-state.ts @@ -1,7 +1,6 @@ // Where a Claude start stands. A session is published once its child is spawned, launched with the -// chat's saved options, before the CLI has answered initialize. Messages are written to it at once -// (the CLI queues them behind its start); only an option write, a control request initialize must -// answer first, waits for startup to land. +// chat's saved options, before the CLI has answered initialize. The host hands it no message until +// `started`; an option write, a control request initialize must answer first, waits here too. import type { SubmissionRejectionFact } from '../../shared/agent-session-failure' import { providerStartupFailureFact } from '../native-chat/agent-session-wire/structured-agent-session-failure-text' diff --git a/src/main/claude/claude-structured-session-startup.test.ts b/src/main/claude/claude-structured-session-startup.test.ts index 58a81a2ca6f4..da81c01a33cd 100644 --- a/src/main/claude/claude-structured-session-startup.test.ts +++ b/src/main/claude/claude-structured-session-startup.test.ts @@ -69,6 +69,38 @@ describe('Claude structured session publishes before the CLI answers initialize' await adapter.closeAll() }) + it('reports `started` on the initialize answer, never waiting on the settings read', async () => { + const SLOW_SETTINGS_MS = 25_000 + const claude = fakeClaude({ + initDelayMs: SLOW_INIT_MS, + // The fake answers `get_settings` with whatever this resolves to, when it resolves. + settings: new Promise((resolve) => + setTimeout( + () => resolve({ effective: { fastMode: true }, applied: { model: 'claude-haiku-4-5' } }), + SLOW_INIT_MS + SLOW_SETTINGS_MS + ) + ) + }) + const { adapter, events } = startingAdapter(claude) + await adapter.acquire(ACQUIRE) + + await vi.advanceTimersByTimeAsync(SLOW_INIT_MS) + + expect(claude.connections[0].calls.map((call) => call.subtype)).toContain('get_settings') + expect(events.filter((event) => event.type === 'started')).toHaveLength(1) + expect(events.some((event) => event.type === 'options-reported')).toBe(false) + expect(events.some((event) => event.type === 'auth-diagnostic')).toBe(false) + + await vi.advanceTimersByTimeAsync(SLOW_SETTINGS_MS) + await claudeStartupSettled(adapter, 'session-1') + + expect(events.find((event) => event.type === 'options-reported')).toMatchObject({ + reportedOptions: { model: 'claude-haiku-4-5', fastMode: true } + }) + expect(events.some((event) => event.type === 'auth-diagnostic')).toBe(true) + await adapter.closeAll() + }) + it('reports `started` with the options its child was launched with, having written none', async () => { const claude = fakeClaude({ initDelayMs: SLOW_INIT_MS, initModel: 'claude-opus-9' }) const { adapter, events } = startingAdapter(claude) diff --git a/src/main/claude/claude-structured-session-startup.ts b/src/main/claude/claude-structured-session-startup.ts index e7e0c12f338e..5a50ddf220dc 100644 --- a/src/main/claude/claude-structured-session-startup.ts +++ b/src/main/claude/claude-structured-session-startup.ts @@ -1,7 +1,8 @@ -// What Claude reports at initialize, read in the background once the session is published. None of -// it gates the create or a message: the child was launched with the chat's saved options and takes -// input at once. Every way the start can fail (exit, auth, a foreign session id) faults the -// published session through its exit path; a CLI that never answers is ended by a Stop or a close. +// What Claude reports at initialize, read once the session is published. The initialize answer is +// the start: `started` follows it, and the host hands the child nothing before. The settings read +// after it is optional and never delays that; what it reports is applied as a later options update. +// Every way the start can fail (exit, auth, a foreign session id) faults the published session +// through its exit path; one that stops making progress is ended by the host's startup limit. import type { StructuredAgentSessionOptionsSkippedEvent, @@ -18,20 +19,24 @@ import { } from './claude-structured-init-proof' import { claudeStructuredSessionPublicationOptions, - prepareClaudeStructuredSessionAcquisitionOptions, - readClaudeStructuredSessionSettings + prepareClaudeStructuredSessionAcquisitionOptions } from './claude-structured-session-acquisition-options' import { claudeRetiredOptions } from './claude-structured-retired-model' import { claudeStructuredSessionOptionsFrom, observeClaudeSettingsApplied, + readClaudeFastModeFacts, readClaudeSettingsEffort } from './claude-structured-session-options' import { failClaudeStartup, type ClaudeSessionStartup } from './claude-structured-session-startup-state' -import type { ClaudeSession, ClaudeStructuredSessionEvent } from './claude-structured-session-state' +import type { + ClaudeAuthDiagnostic, + ClaudeSession, + ClaudeStructuredSessionEvent +} from './claude-structured-session-state' import { admitClaudeStartFastMode, applyClaudeStartFastMode @@ -99,19 +104,26 @@ export type StructuredAgentSessionStartedOptions = Pick< /** A lifecycle event the start reports, before the session's identity is stamped on it. */ export type ClaudeStartupReport = - | ({ type: 'started' } & StructuredAgentSessionStartedOptions) + | ({ + type: 'started' | 'options-reported' + } & StructuredAgentSessionStartedOptions) | Pick -export type ClaudeStartupFacts = { +/** What the initialize answer proved: enough to start. */ +export type ClaudeInitializeFacts = { init: ClaudeInitObservation | null initProof: ClaudeInitProof initialization: unknown - settings: unknown - prepared: ReturnType resumesTranscript: boolean requestTimeoutMs: number | undefined } +/** The initialize facts and the optional settings read that followed them. */ +export type ClaudeStartupFacts = ClaudeInitializeFacts & { + settings: unknown + prepared: ReturnType +} + /** Settles on the CLI's initialize answer, or on its exit or a refused proof. */ export async function readClaudeStartupFacts(input: { connection: ClaudeStreamJsonConnection @@ -122,7 +134,7 @@ export async function readClaudeStartupFacts(input: { resumesTranscript: boolean requestTimeoutMs: number | undefined emit: (event: ClaudeStructuredSessionEvent) => void -}): Promise { +}): Promise { // The CLI's first answer has no request deadline of its own; the reads after it do. const initialization = await Promise.race([ input.connection.initializationResult().then((result) => { @@ -143,39 +155,50 @@ export async function readClaudeStartupFacts(input: { sessionId: input.sessionId, models: readClaudeModels(initialization) }) - input.initProof.seen() - const settings = await readClaudeStructuredSessionSettings( - input.connection, - input.requestTimeoutMs - ) - // Read again: a frame naming another session may have come during the settings read. - const init = input.initProof.seen() - input.emit({ - type: 'auth-diagnostic', - sessionId: input.sessionId, - diagnostic: claudeAuthDiagnostic(initialization, init, settings) - }) return { - init, + init: input.initProof.seen(), initProof: input.initProof, initialization, - settings, - prepared: prepareClaudeStructuredSessionAcquisitionOptions({ settings, initialization }), resumesTranscript: input.resumesTranscript, requestTimeoutMs: input.requestTimeoutMs } } -function applyClaudeStartupFacts(session: ClaudeSession, facts: ClaudeStartupFacts): void { - const { init, initialization, settings, prepared } = facts - const effort = readClaudeSettingsEffort(settings) - const published = claudeStructuredSessionPublicationOptions(prepared) +function applyClaudeInitializeFacts(session: ClaudeSession, facts: ClaudeInitializeFacts): void { + const { init, initialization } = facts // A turn's own init frame may already have reported the running model. if (init?.model && session.reportedOptions.model === undefined) { session.reportedOptions.model = init.model session.reportedModelMutation = session.optionMutationSequence } + const fastModeFacts = readClaudeFastModeFacts(initialization) + session.fastModeState ??= fastModeFacts.state + session.fastModeDisabledReason ??= fastModeFacts.disabledReason + session.capabilities = readClaudeCapabilities(session.capabilities, initialization, init?.message) + // A catalog frame that streamed in after publish is newer than the initialize answer. + if (session.commands.commands === undefined) { + session.commands = new ClaudeSlashCommandCatalog(init?.message, initialization) + } + session.events?.publish() +} + +/** Settings read after `started`. Values a write made since the read began own are left alone. */ +function applyClaudeSettingsFacts( + session: ClaudeSession, + facts: ClaudeStartupFacts, + writtenSinceRead: boolean +): void { + const { settings, prepared } = facts + const effort = readClaudeSettingsEffort(settings) + const published = claudeStructuredSessionPublicationOptions(prepared) observeClaudeSettingsApplied(session, settings) + if (published.fastModePerSessionOptIn !== null) { + session.fastModePerSessionOptIn = published.fastModePerSessionOptIn + } + if (writtenSinceRead) { + session.events?.publish() + return + } // The readback vouches for a value the child was launched with only when it reports that value. const agrees = (key: string, reported: string): boolean => !session.options.has(key) || session.options.get(key) === reported @@ -196,16 +219,6 @@ function applyClaudeStartupFacts(session: ClaudeSession, facts: ClaudeStartupFac session.confirmedOptions.add('fastMode') } } - if (published.fastModePerSessionOptIn !== null) { - session.fastModePerSessionOptIn = published.fastModePerSessionOptIn - } - session.fastModeState ??= published.fastModeState - session.fastModeDisabledReason ??= published.fastModeDisabledReason - session.capabilities = readClaudeCapabilities(session.capabilities, initialization, init?.message) - // A catalog frame that streamed in after publish is newer than the initialize answer. - if (session.commands.commands === undefined) { - session.commands = new ClaudeSlashCommandCatalog(init?.message, initialization) - } session.events?.publish() } @@ -213,9 +226,10 @@ function applyClaudeStartupFacts(session: ClaudeSession, facts: ClaudeStartupFac * it would pin an effort nobody chose on every reopen, past a later settings change. */ function claudeStartedReportedOptions( session: ClaudeSession, - catalog: unknown[] + catalog: unknown[], + readMutationSequence = session.optionMutationSequence ): StructuredAgentSessionStartedOptions['reportedOptions'] { - const { current } = claudeStructuredSessionOptionsFrom(session, catalog) + const { current } = claudeStructuredSessionOptionsFrom(session, catalog, readMutationSequence) if (session.options.has('effort') || session.reportedOptions.effort !== undefined) { return current } @@ -223,15 +237,19 @@ function claudeStartedReportedOptions( return persisted } -/** Applies startup facts to the published session, which already takes input. Any failure faults - * the session so the user sees why it never started. */ +/** Applies the initialize answer and reports `started`, so the host hands the child what it holds; + * then reads the optional settings and reports what they add. A failure before `started` faults + * the session so the user sees why it never started; the settings read can fail nothing. */ export async function settleClaudeSessionStartup(input: { session: ClaudeSession - facts: Promise + facts: Promise + /** Never rejects: an unreadable settings answer reads as null. */ + readSettings: () => Promise isCurrent: () => boolean fault: (error: Error) => void + diagnose: (diagnostic: ClaudeAuthDiagnostic) => void /** `started` once startup has proven, with what the child now reports, snapshotted from memory; - * then any saved option the child showed it cannot run. */ + * then what the settings add, and any saved option the child showed it cannot run. */ report: (event: ClaudeStartupReport) => void }): Promise { const { session } = input @@ -242,41 +260,35 @@ export async function settleClaudeSessionStartup(input: { failClaudeStartup(session, new Error('claude session closed before startup completed')) return true } + let initialized: ClaudeInitializeFacts try { - const facts = await input.facts + initialized = await input.facts if (superseded()) { return } // From here no read of the proof is pending, so a frame naming another session ends it. - facts.initProof.onRefusal = (error) => { + initialized.initProof.onRefusal = (error) => { failClaudeStartup(session, error) if (input.isCurrent()) { input.fault(error) } } - applyClaudeStartupFacts(session, facts) - const startFastMode = admitClaudeStartFastMode(session, facts) - if (!superseded()) { - input.report({ - type: 'started', - // `list_models` is answered from this same initialize result, so nothing is re-read. - reportedOptions: claudeStartedReportedOptions( - session, - readClaudeModels(facts.initialization) - ), - restoreSkippedOptions: [...session.restoreSkippedOptions], - ...claudeRetiredOptions(session) - }) - if (session.startup.state === 'pending') { - session.startup.state = 'proven' - } - if (startFastMode !== null) { - void applyClaudeStartFastMode(session, facts, startFastMode, (event) => { - if (input.isCurrent()) { - input.report(event) - } - }) - } + applyClaudeInitializeFacts(session, initialized) + input.report({ + type: 'started', + // `list_models` is answered from this same initialize result, so nothing is re-read. A + // saved Fast the launch left out is decided once settings are read, so this read settles + // none of it (an older sequence never rewrites an option). + reportedOptions: claudeStartedReportedOptions( + session, + readClaudeModels(initialized.initialization), + session.fastModeAtStart ? session.optionMutationSequence - 1 : undefined + ), + restoreSkippedOptions: [...session.restoreSkippedOptions], + ...claudeRetiredOptions(session) + }) + if (session.startup.state === 'pending') { + session.startup.state = 'proven' } } catch (caught) { const error = caught instanceof Error ? caught : new Error(String(caught)) @@ -286,5 +298,52 @@ export async function settleClaudeSessionStartup(input: { if (!endedElsewhere && input.isCurrent()) { input.fault(error) } + return + } + await settleClaudeStartupSettings(input, initialized) +} + +async function settleClaudeStartupSettings( + input: Parameters[0], + initialized: ClaudeInitializeFacts +): Promise { + const { session } = input + const sequence = session.optionMutationSequence + const settings = await input.readSettings() + if (!input.isCurrent()) { + return + } + let init: ClaudeInitObservation | null + try { + // Read again: a frame naming the session may have come since; a refusal is already ending it. + init = initialized.initProof.seen() + } catch { + return + } + const facts: ClaudeStartupFacts = { + ...initialized, + init, + settings, + prepared: prepareClaudeStructuredSessionAcquisitionOptions({ + settings, + initialization: initialized.initialization + }) + } + input.diagnose(claudeAuthDiagnostic(facts.initialization, facts.init, settings)) + const writtenSinceRead = sequence !== session.optionMutationSequence + applyClaudeSettingsFacts(session, facts, writtenSinceRead) + const startFastMode = writtenSinceRead ? null : admitClaudeStartFastMode(session, facts) + input.report({ + type: 'options-reported', + reportedOptions: claudeStartedReportedOptions(session, readClaudeModels(facts.initialization)), + restoreSkippedOptions: [...session.restoreSkippedOptions], + ...claudeRetiredOptions(session) + }) + if (startFastMode !== null) { + void applyClaudeStartFastMode(session, facts, startFastMode, (event) => { + if (input.isCurrent()) { + input.report(event) + } + }) } } diff --git a/src/main/claude/claude-structured-session-state.ts b/src/main/claude/claude-structured-session-state.ts index e327e2316219..096a3a4c6724 100644 --- a/src/main/claude/claude-structured-session-state.ts +++ b/src/main/claude/claude-structured-session-state.ts @@ -7,6 +7,7 @@ import type { } from '../../shared/agent-session-journal-types' import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import type { + StructuredAgentSessionOptionsReportedEvent, StructuredAgentSessionOptionsSkippedEvent, StructuredAgentSessionStartedEvent } from '../native-chat/agent-session-wire/structured-agent-session-adapter' @@ -68,8 +69,9 @@ export type ClaudeStructuredSessionEvent = fence: number } | { type: 'auth-diagnostic'; sessionId: string; diagnostic: ClaudeAuthDiagnostic } - /** Startup facts applied; the child already took any message written to it. */ + /** The CLI answered initialize: the host may hand the child input from here. */ | StructuredAgentSessionStartedEvent + | StructuredAgentSessionOptionsReportedEvent | StructuredAgentSessionOptionsSkippedEvent | { type: 'ended' diff --git a/src/main/codex/codex-app-server-connection-types.ts b/src/main/codex/codex-app-server-connection-types.ts index bf33bc5d0b80..9e50a5233133 100644 --- a/src/main/codex/codex-app-server-connection-types.ts +++ b/src/main/codex/codex-app-server-connection-types.ts @@ -13,6 +13,8 @@ export type CodexAppServerConnectionHandlers = { 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 + /** Any stdout or stderr chunk from the child. */ + onOutput?: () => void } export type CodexAppServerConnection = { diff --git a/src/main/codex/codex-app-server-connection.ts b/src/main/codex/codex-app-server-connection.ts index 0739ca65af69..172e0d5577d5 100644 --- a/src/main/codex/codex-app-server-connection.ts +++ b/src/main/codex/codex-app-server-connection.ts @@ -46,7 +46,8 @@ export async function openCodexAppServerConnection( ): Promise { const managed = spawnManagedProviderProcess(launch, { spawnImpl, - site: 'codex-app-server-teardown' + site: 'codex-app-server-teardown', + ...(handlers.onOutput ? { onOutput: handlers.onOutput } : {}) }) const { child, terminateTree: terminateProcessTree } = managed diff --git a/src/main/codex/codex-structured-session-acquire.ts b/src/main/codex/codex-structured-session-acquire.ts index 1dbdde663b41..5171e92a1a0f 100644 --- a/src/main/codex/codex-structured-session-acquire.ts +++ b/src/main/codex/codex-structured-session-acquire.ts @@ -166,6 +166,7 @@ export async function acquireCodexStructuredSession(input: { Buffer.byteLength(JSON.stringify(payload ?? null), 'utf8') ), onSpawned: spawnIdentity.onSpawned, + ...(acquireInput.onOutput ? { onOutput: acquireInput.onOutput } : {}), onExit: (error, exit) => { try { handleCodexSessionExit({ diff --git a/src/main/jsonl-rpc/agent-connection.ts b/src/main/jsonl-rpc/agent-connection.ts index 65bd6cfc9ad3..89be90dc6e4c 100644 --- a/src/main/jsonl-rpc/agent-connection.ts +++ b/src/main/jsonl-rpc/agent-connection.ts @@ -17,6 +17,8 @@ import { resolveJsonlRpcPeerOptions, type JsonlRpcPeerOptions } from './peer-lim export type JsonlRpcAgentConnectionOptions = JsonlRpcPeerHandlers & { peer?: JsonlRpcPeerOptions onExit?: (error: Error, context: { expected: boolean; exit: ProviderProcessExit }) => void + /** Any stdout or stderr chunk from the child. */ + onOutput?: () => void } /** The owning runtime keeps process exit evidence separate from transport closure. */ @@ -35,7 +37,8 @@ export class JsonlRpcAgentConnection { const peerOptions = resolveJsonlRpcPeerOptions(options.peer) this.managed = spawnManagedProviderProcess(launch, { spawnImpl, - site: 'jsonl-rpc-agent-teardown' + site: 'jsonl-rpc-agent-teardown', + ...(options.onOutput ? { onOutput: options.onOutput } : {}) }) const managed = this.managed this.peer = new JsonlRpcPeer( diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-acquire-aborts.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-acquire-aborts.ts index 80fb60851324..9fdc5e9a72b4 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-acquire-aborts.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-acquire-aborts.ts @@ -1,6 +1,6 @@ // The acquire each session has in flight, owned by the host that runs it, or another provider wait // its queue is on (an option write). The queue runs one step at a time, so a session has at most -// one; a close, a Stop admitted now, quit, or the start's deadline aborts it from outside the queue +// one; a close, a Stop admitted now, quit, or the startup limit aborts it from outside the queue // instead of waiting behind a provider that may never answer. export class StructuredAgentSessionAcquireAborts { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts index 159c99d2cc04..1614785712a0 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts @@ -49,22 +49,21 @@ export async function acquireOwner( } catch (error) { throw new AgentSessionPreSpawnError(error) } - // Minted before the adapter runs anything, so the deadline bounds the whole start. const attempt = mintStructuredAgentSessionStartupAttempt({ record, identity: journalIdentityFor(record, input.params), // Retries must recover the original reservation, not mint a second child. spawnToken, ...(input.eventSink ? { events: input.eventSink } : {}), - ...(input.acquireSignal ? { signal: input.acquireSignal } : {}), - now: input.now(), - ...(input.startupDeadlineMs === undefined ? {} : { deadlineMs: input.startupDeadlineMs }) + ...(input.acquireSignal ? { signal: input.acquireSignal } : {}) }) - input.onStartupAttempt?.(attempt) + const progress = input.onStartupAttempt?.(attempt) const acquired = await input.adapter.acquire({ ...attempt, + ...(progress ? { onOutput: progress.output } : {}), ...(input.recordPhase ? { recordPhase: input.recordPhase } : {}), onSpawned: async (process) => { + progress?.spawned() record = await input.store.commitProcessIdentity({ sessionId: record.sessionId, fence, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts index e257e5984f01..6396fe17fae5 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts @@ -242,9 +242,17 @@ export type StructuredAgentSessionOptionsSkippedEvent = { options: Readonly> } +/** What a child that already proved its start reports later, such as an optional read that came + * after `started`: persisted as the start's report is. Never delays a start. */ +export type StructuredAgentSessionOptionsReportedEvent = Omit< + StructuredAgentSessionStartedEvent, + 'type' +> & { type: 'options-reported' } + export type StructuredAgentSessionLifecycleEvent = | StructuredAgentSessionEndedEvent | StructuredAgentSessionStartedEvent + | StructuredAgentSessionOptionsReportedEvent | StructuredAgentSessionOptionsSkippedEvent /** Whether the provider child behind an acquisition has proven its start. A publish-first 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 557e1569a418..2c180c121b33 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 @@ -52,7 +52,7 @@ export type StructuredAgentSessionResumeOutcome = diagnostic?: ProviderDiagnostic /** The host's own close, Stop or quit aborted the start, so it failed nothing it was for. */ aborted?: true - /** The start's deadline passed before it proved itself: Orca stopped it. */ + /** The start went silent or hit its ceiling before it proved itself: Orca stopped it. */ expired?: true argumentProblem?: AgentSessionArgumentProblem } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts index 41b7017aecdb..6514e4361df0 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts @@ -47,7 +47,10 @@ import { } from '../../observability/agent-session-instrumentation' import type { ProviderHistoryWindow } from '../agent-session-journal/journal-submission-reconciler' import { readProviderHistoryWindow } from './structured-agent-session-provider-history-window' -import type { StructuredAgentSessionStartupAttempt } from './structured-agent-session-startup-attempt' +import type { + StructuredAgentSessionStartupAttempt, + StructuredAgentSessionStartupProgress +} from './structured-agent-session-startup-attempt' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { StructuredAgentSessionLogger } from './structured-agent-session-logger' @@ -64,9 +67,11 @@ export type AttachFlowInput = { recordPhase?: AgentSessionCreatePhaseRecorder /** Aborted when a close, or a Stop admitted now, must not wait behind this attach's acquire. */ acquireSignal?: AbortSignal - /** The attempt this attach's acquire runs under, minted before the adapter is called. */ - onStartupAttempt?: (attempt: StructuredAgentSessionStartupAttempt) => void - startupDeadlineMs?: number + /** The attempt this attach's acquire runs under, minted before the adapter is called; what it + * answers learns of the child's spawn and output. */ + onStartupAttempt?: ( + attempt: StructuredAgentSessionStartupAttempt + ) => StructuredAgentSessionStartupProgress /** Publishes the journal before clients can send against the new owner. `acquiredOwner` is * true only when this attach spawned the provider child, so a re-attach to a live one is not * mistaken for a cold acquire. */ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts index d54460ef50aa..18662e44566e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts @@ -55,7 +55,7 @@ export type StructuredAgentSessionAttachOptions = { startedFor?: string /** A close, an admitted Stop or quit aborted this attach: its refusal is that abort's. */ onAborted?: () => void - /** The start's deadline aborted this attach's acquire: it failed, nobody stopped it. */ + /** The startup limit aborted this attach's acquire: it failed, nobody stopped it. */ onStartupExpired?: () => void } @@ -203,12 +203,9 @@ async function runAttachUnderAbort( now: () => context.now(), recordPhase, acquireSignal, - ...(context.deps.startupDeadlineMs === undefined - ? {} - : { startupDeadlineMs: context.deps.startupDeadlineMs }), onStartupAttempt: (startup) => { attempt.startup = startup - startupAttempts.track(sessionId, startup) + return startupAttempts.track(sessionId, startup) }, ...(options.onAcquisitionFailed ? { onAcquisitionFailed: options.onAcquisitionFailed } : {}), openConversation: async (record) => { @@ -305,6 +302,9 @@ function endReleasedChild( ): void { const session = context.sessions.get(sessionId) const child = session?.child + if (child) { + context.runtimeState.startupAttempts.childEnded(sessionId, child) + } if ( !session || !child || 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 index a983753e2a99..e9ac5b09bd9e 100644 --- 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 @@ -30,6 +30,7 @@ import { 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' +import type { StructuredAgentSessionStartupAttempts } from './structured-agent-session-startup-attempt' /** How the child's root went: the close this host asked for, or a death of its own. */ export type StructuredAgentSessionChildExit = { @@ -58,6 +59,8 @@ export type StructuredAgentSessionChildExitContext< publishStatus?: (sessionId: string) => void /** The delivery loop hands over whatever is queued once the child is off the record. */ wakeDelivery?: (sessionId: string) => void + /** An ended child's start, if still open, has nothing left to time. */ + startupAttempts?: Pick serialize: (sessionId: string, task: () => Promise) => Promise now: () => number logger: StructuredAgentSessionLogger @@ -142,6 +145,7 @@ export async function endExitedStructuredAgentSessionChildUnderSerialize< : ('chatClosed' as const) : undefined const endChild = (): void => { + context.startupAttempts?.childEnded(sessionId, child) endProviderChild(session, { generation: child.generation, fence: child.fence, 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 e2b206595049..772f57879d20 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 @@ -119,8 +119,8 @@ export function createStructuredAgentSessionConversationLifetime(host: { cause: 'host-stop' }) }).catch((error: unknown) => - deps().logger.warn('stopping an agent past its start deadline failed', { - scope: 'startup-deadline', + deps().logger.warn('stopping an agent past its startup limit failed', { + scope: 'startup-limit', sessionId, error }) 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 d8d6d9a1af8c..1858eaeddf8d 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 @@ -84,6 +84,7 @@ type StartFailure = { startKey: string | null; cause: StructuredAgentSessionStar export class StructuredAgentSessionDeliveryLoop { private readonly running = new Set() + private readonly runs = new Map>() private disposed = false constructor(private readonly deps: StructuredAgentSessionDeliveryLoopDeps) {} @@ -98,13 +99,23 @@ export class StructuredAgentSessionDeliveryLoop { } /** From inside the session's serialize, after a message was accepted or the conversation - * opened. A loop already running re-reads the journal on its next step. */ - wake(sessionId: string): void { - if (this.disposed || this.running.has(sessionId)) { - return + * opened. A loop already running re-reads the journal on its next step. Settles once the loop + * has handed over all it can; never rejects. */ + wake(sessionId: string): Promise { + if (this.disposed) { + return Promise.resolve() + } + if (this.running.has(sessionId)) { + return this.runs.get(sessionId) ?? Promise.resolve() } this.running.add(sessionId) - void this.run(sessionId) + const run = this.run(sessionId).finally(() => { + if (this.runs.get(sessionId) === run) { + this.runs.delete(sessionId) + } + }) + this.runs.set(sessionId, run) + return run } private async run(sessionId: string): Promise { 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 bf4c74bd3086..51894a9ff8d7 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 @@ -10,6 +10,7 @@ import type { } from './structured-agent-session-host-types' import type { StructuredAgentSessionSinkBarrier } from './structured-agent-session-event-sink' import { + settleStructuredAgentSessionOptionsReported, settleStructuredAgentSessionOptionsSkipped, settleStructuredAgentSessionProviderStarted } from './structured-agent-session-provider-started' @@ -35,7 +36,7 @@ export class StructuredAgentSessionEventRecovery { serialize: (sessionId: string, task: () => Promise) => Promise now: () => number runtimeState: StructuredAgentSessionHostRuntimeState - wakeDelivery: (sessionId: string) => void + wakeDelivery: (sessionId: string) => Promise } ) {} @@ -44,6 +45,7 @@ export class StructuredAgentSessionEventRecovery { return { ...this.context, logger: deps.logger, + startupAttempts: runtimeState.startupAttempts, holdUnrunSends: async (sessionId, fence, cause) => { const journal = this.context.sessions.get(sessionId)?.journal if (journal) { @@ -111,6 +113,9 @@ export class StructuredAgentSessionEventRecovery { if (event.type === 'started') { return settleStructuredAgentSessionProviderStarted(this.context, event) } + if (event.type === 'options-reported') { + return settleStructuredAgentSessionOptionsReported(this.context, event) + } if (event.type === 'options-skipped') { return settleStructuredAgentSessionOptionsSkipped(this.context, event) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts index e8b90cc2da18..3757a2f5345b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts @@ -13,6 +13,7 @@ import { type ProviderChildSessions } from './structured-agent-session-provider-child' import { resolveStructuredSessionRecovery } from './structured-agent-session-recovery-resolution' +import { recordAgentSessionStartup } from '../../observability/agent-session-instrumentation' import { StructuredAgentSessionStartupAttempts, StructuredAgentSessionStartupExpiredError, @@ -31,12 +32,13 @@ export class StructuredAgentSessionHostRuntimeState { /** Required: a child held here renews its lease without a PID probe. */ sessions: ProviderChildSessions, onEventSinkFailure?: (sessionId: string, error: unknown) => void, - /** A published child's start ran past its deadline; one still acquiring is aborted here. */ + /** A published child's start passed its startup limit; one still acquiring is aborted here. */ onStartupExpired?: (expired: StructuredAgentSessionExpiredStartup) => void ) { this.onEventSinkFailure = onEventSinkFailure this.startupAttempts = new StructuredAgentSessionStartupAttempts({ - now: () => deps.now?.() ?? Date.now(), + ...(deps.startupLimits ? { limits: deps.startupLimits } : {}), + settled: recordAgentSessionStartup, expire: (expired) => { if (expired.child === null) { this.acquireAborts.abort( 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 8552c76d2289..cd670d7f8bf3 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 @@ -25,6 +25,7 @@ import type { AgentModelCatalogService } from '../agent-model-catalog/agent-mode import type { StructuredAgentSessionLogger } from './structured-agent-session-logger' import type { StructuredAgentId } from '../../../shared/agent-session-provider-handle' import type { StructuredAgentRegistry } from './structured-agent-registry' +import type { StructuredAgentSessionStartupLimits } from './structured-agent-session-startup-attempt' export type StructuredAgentSessionCaller = { callerKey: string } @@ -141,8 +142,9 @@ export type StructuredAgentSessionHostDeps = { now?: () => number /** The idle sweep's period and window. Tests drive these; production takes the defaults. */ idleSweep?: { intervalMs?: number; idleMs?: number } - /** How long a start may take to prove itself. Tests drive this; production takes the default. */ - startupDeadlineMs?: number + /** How long a start may stay silent, and its ceiling. Tests drive these; production takes the + * host constants. */ + startupLimits?: Partial /** Whether an orchestration dispatch still owns this session's worker; absent answers no. */ hasOpenDispatch?: (record: AgentSessionRecord) => boolean /** A chat tab left the screen: closed, or its workspace removed. Advisory; a throw is logged. */ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts index 9c4778d0efe5..92f474c28966 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts @@ -3,15 +3,17 @@ // A publish-first acquire hands the host a child that has answered nothing yet, so the record // keeps only the saved options the reservation carried, and the delivery loop hands it nothing. // This is where the host learns the start landed: the child turns `ready`, its startup attempt -// ends, the loop wakes to hand over what was queued meanwhile, and what the child now reports is -// persisted as fact through the same record write a user's option change takes. Bookkeeping never -// gates the user: a failed write is reported, and delivery has already been woken. +// ends, and the loop wakes to hand over what was queued meanwhile. Only once that handover is done +// is what the child reports persisted, in a step of its own, so bookkeeping never sits between a +// ready child and the user's first message; a failed write is reported, never thrown. // -// This runs under the session's own serialized step, which its close and sends wait on, so it -// asks the provider nothing: the event carries what the child proved. +// These run under the session's own serialized steps, which its close and sends wait on, so they +// ask the provider nothing: the event carries what the child proved. +import { isDeepStrictEqual } from 'node:util' import { agentSessionLeaseAdmitsWriter } from '../../../shared/agent-session-lease-adjudication' import type { + StructuredAgentSessionOptionsReportedEvent, StructuredAgentSessionOptionsSkippedEvent, StructuredAgentSessionStartedEvent } from './structured-agent-session-adapter' @@ -20,7 +22,10 @@ import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' import { nativeSessionOptionsFromReport } from './structured-agent-session-option-restoration' -import { markProviderChildStarted } from './structured-agent-session-provider-child' +import { + markProviderChildStarted, + sameProviderChild +} from './structured-agent-session-provider-child' import type { StructuredAgentSessionStartupAttempts } from './structured-agent-session-startup-attempt' export type StructuredAgentSessionProviderStartedContext = { @@ -30,62 +35,99 @@ export type StructuredAgentSessionProviderStartedContext = { now: () => number publishStatus?: (sessionId: string) => void runtimeState: { startupAttempts: Pick } - /** The barrier lifts: what was accepted while the child started is handed over now. */ - wakeDelivery: (sessionId: string) => void + /** The barrier lifts: what was accepted while the child started is handed over now. Settles once + * the loop has handed over all it can. */ + wakeDelivery: (sessionId: string) => Promise } -export function settleStructuredAgentSessionProviderStarted( +type ReportedOptions = Pick< + StructuredAgentSessionOptionsReportedEvent, + 'sessionId' | 'fence' | 'acquisitionGeneration' | 'reportedOptions' | 'restoreSkippedOptions' +> & + Pick + +export async function settleStructuredAgentSessionProviderStarted( context: StructuredAgentSessionProviderStartedContext, event: StructuredAgentSessionStartedEvent ): Promise { // Serialized behind the attach that published this child, so the lease it proved is committed. - return context.serialize(event.sessionId, async () => { + const started = await context.serialize(event.sessionId, async () => { const session = context.sessions.get(event.sessionId) const child = { generation: event.acquisitionGeneration, fence: event.fence } // A stale child's proof starts nothing: the barrier stays on the child the host holds. if (!session || !markProviderChildStarted(session, child)) { - return + return null } context.runtimeState.startupAttempts.ready(event.sessionId, child) - context.wakeDelivery(event.sessionId) - try { - await persistStartedOptions(context, event) - } catch (error) { - context.deps.logger.warn('recording what a started provider reported failed', { - scope: 'provider-started-options', - sessionId: event.sessionId, - error - }) - } finally { - context.publishStatus?.(event.sessionId) - } + const delivered = context.wakeDelivery(event.sessionId) + context.publishStatus?.(event.sessionId) + return { delivered, optionsAtStart: context.deps.store.getRecord(event.sessionId)?.options } }) + if (!started) { + return + } + // Not awaited: the adapter's next event may be what the handover itself waits on. + void started.delivered.then(() => + persistReportedOptions(context, event, { optionsAtStart: started.optionsAtStart }) + ) } -async function persistStartedOptions( +/** What a ready child reports later, such as a read that came after its start: persisted as the + * start's report is, never ahead of a send. */ +export function settleStructuredAgentSessionOptionsReported( context: StructuredAgentSessionProviderStartedContext, - event: StructuredAgentSessionStartedEvent + event: StructuredAgentSessionOptionsReportedEvent ): Promise { - const { store } = context.deps - const record = store.getRecord(event.sessionId) - if ( - !record || - record.lease.runtimeFence !== event.fence || - !agentSessionLeaseAdmitsWriter(record.lease) - ) { - return - } - await store.replaceSessionOptions({ - sessionId: event.sessionId, - fence: event.fence, - options: nativeSessionOptionsFromReport({ - reported: event.reportedOptions, - restoreSkipped: event.restoreSkippedOptions, - ...(event.retiredOptions ? { retired: event.retiredOptions } : {}), - ...(record.options ? { priorOptions: record.options } : {}) - }), - now: context.now() - }) + return persistReportedOptions(context, event, null) +} + +/** Non-fatal, and only for the child that reported, with the record as it stood when it did: a pick + * made since is the user's and newer than the report. */ +function persistReportedOptions( + context: StructuredAgentSessionProviderStartedContext, + event: ReportedOptions, + snapshot: { optionsAtStart: Readonly> | undefined } | null +): Promise { + return context + .serialize(event.sessionId, async () => { + const child = context.sessions.get(event.sessionId)?.child + const record = context.deps.store.getRecord(event.sessionId) + if ( + !child || + !sameProviderChild(child, { + generation: event.acquisitionGeneration, + fence: event.fence + }) || + !record || + record.lease.runtimeFence !== event.fence || + !agentSessionLeaseAdmitsWriter(record.lease) || + (snapshot && !isDeepStrictEqual(record.options, snapshot.optionsAtStart)) + ) { + return + } + try { + await context.deps.store.replaceSessionOptions({ + sessionId: event.sessionId, + fence: event.fence, + options: nativeSessionOptionsFromReport({ + reported: event.reportedOptions, + restoreSkipped: event.restoreSkippedOptions, + ...(event.retiredOptions ? { retired: event.retiredOptions } : {}), + ...(record.options ? { priorOptions: record.options } : {}) + }), + now: context.now() + }) + } finally { + context.publishStatus?.(event.sessionId) + } + }) + .catch((error: unknown) => { + context.deps.logger.warn('recording what a started provider reported failed', { + scope: 'provider-started-options', + sessionId: event.sessionId, + error + }) + }) } /** A running child showed saved options it cannot run: the record drops them, as a start that diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts index 8bd4f3676f2c..d856ecaf50ba 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts @@ -32,7 +32,7 @@ import { resetHostTestOperationIds } from './structured-agent-session-host-test-data' import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' -import { STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS } from './structured-agent-session-startup-attempt' +import type { StructuredAgentSessionStartupLimits } from './structured-agent-session-startup-attempt' const CALLER = { callerKey: 'client-1' } @@ -45,7 +45,7 @@ let host: StructuredAgentSessionHost let acquire: Mock let dispatch: Mock let closeSession: Mock> -let startupDeadlineMs: number | undefined +let startupLimits: Partial | undefined function acquisition(input: StructuredAgentSessionAcquireInput): AgentSessionAcquisition { return { @@ -90,7 +90,7 @@ async function startHost(): Promise { claimKeyId: 'key-1', mintSpawnToken: () => `spawn-${acquire.mock.calls.length}`, idleSweep: { intervalMs: 60 * 60_000, idleMs: 60 * 60_000 }, - ...(startupDeadlineMs === undefined ? {} : { startupDeadlineMs }), + ...(startupLimits === undefined ? {} : { startupLimits }), now: () => NOW }) } @@ -107,7 +107,7 @@ async function restartWith(next: StructuredAgentSessionAdapter['acquire']): Prom beforeEach(async () => { root = await mkdtemp(join(tmpdir(), 'orca-startup-attempt-')) resetHostTestOperationIds() - startupDeadlineMs = undefined + startupLimits = undefined acquire = vi.fn(async (input) => acquisition(input)) dispatch = vi.fn(async () => ({ state: 'accepted' as const, @@ -263,6 +263,7 @@ describe('a start that ends before it proves itself', () => { await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) expect((await submission(id))?.handedOverAt).toBeUndefined() expect(dispatch).not.toHaveBeenCalled() + expect(startupAttemptOpen()).toBe(false) }) it('withdraws what it held on a Stop, without waiting on the handshake', async () => { @@ -278,6 +279,7 @@ describe('a start that ends before it proves itself', () => { expect((await submission(id))?.handedOverAt).toBeUndefined() expect(closeSession).toHaveBeenCalled() expect(dispatch).not.toHaveBeenCalled() + expect(startupAttemptOpen()).toBe(false) }) it('keeps what it held for the chat on a close, without waiting on the handshake', async () => { @@ -293,10 +295,11 @@ describe('a start that ends before it proves itself', () => { }) expect((await submission(id))?.handedOverAt).toBeUndefined() expect(dispatch).not.toHaveBeenCalled() + expect(startupAttemptOpen()).toBe(false) }) - it('stops a published start that misses its deadline and fails what it held', async () => { - startupDeadlineMs = 50 + it('stops a published start that goes silent and fails what it held', async () => { + startupLimits = { silenceMs: 50 } await restartWith(publishFirst) const id = await heldBehindStart('hello') @@ -305,13 +308,37 @@ describe('a start that ends before it proves itself', () => { expect((await submission(id))?.handedOverAt).toBeUndefined() expect(closeSession).toHaveBeenCalled() expect(dispatch).not.toHaveBeenCalled() + expect(startupAttemptOpen()).toBe(false) }) - it('aborts an acquire still in its handshake at the deadline and fails what it held', async () => { - startupDeadlineMs = 50 + it('keeps a slow start that is still talking, and stops one that never readies at the ceiling', async () => { + startupLimits = { silenceMs: 50, ceilingMs: 400 } + const chatter: ReturnType[] = [] + await restartWith(async (input) => { + await input.onSpawned?.(acquisition(input).process) + chatter.push(setInterval(() => input.onOutput?.(), 10)) + return { ...acquisition(input), providerChildPhase: 'starting' } + }) + try { + const id = await heldBehindStart('hello') + await new Promise((resolve) => setTimeout(resolve, 200)) + expect(closeSession).not.toHaveBeenCalled() + expect(startupAttemptOpen()).toBe(true) + + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) + expect(await submission(id)).toMatchObject(HOST_STOPPED) + expect(dispatch).not.toHaveBeenCalled() + } finally { + chatter.forEach(clearInterval) + } + }) + + it('aborts an acquire still in its handshake once it goes silent and fails what it held', async () => { + startupLimits = { silenceMs: 50 } await restartWith( (input) => new Promise((_resolve, reject) => { + void input.onSpawned?.(acquisition(input).process) input.signal?.addEventListener('abort', () => reject(input.signal?.reason)) }) ) @@ -320,10 +347,11 @@ describe('a start that ends before it proves itself', () => { await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) expect(await submission(id)).toMatchObject(HOST_STOPPED) expect(dispatch).not.toHaveBeenCalled() + await eventually(() => expect(startupAttemptOpen()).toBe(false)) }) - it('is not ended by the deadline once it proved itself', async () => { - startupDeadlineMs = 50 + it('is not ended by the limit once it proved itself', async () => { + startupLimits = { silenceMs: 50 } await restartWith(publishFirst) const id = await heldBehindStart('hello') await host.handleAdapterEvent(startedEvent()) @@ -333,11 +361,68 @@ describe('a start that ends before it proves itself', () => { expect(closeSession).not.toHaveBeenCalled() expect((await host.readStatusSummary(SESSION))?.hostExecutionPhase).toBe('ready') + expect(startupAttemptOpen()).toBe(false) }) }) +describe('what a started child reports', () => { + it('is persisted only after what was held is handed over', async () => { + await restartWith(publishFirst) + await heldBehindStart('hello') + const order: string[] = [] + dispatch.mockImplementationOnce(async (input) => { + order.push('dispatch') + return { + state: 'accepted' as const, + providerIdentity: { + provider: 'codex' as const, + threadId: THREAD, + turnId: `turn-${input.clientMessageId}`, + ordinal: 1 + } + } + }) + const replace = store.replaceSessionOptions.bind(store) + vi.spyOn(store, 'replaceSessionOptions').mockImplementation(async (args) => { + order.push('persist') + return replace(args) + }) + + await host.handleAdapterEvent({ ...startedEvent(), reportedOptions: { model: 'reported' } }) + + await eventually(() => expect(order).toEqual(['dispatch', 'persist'])) + expect(store.getRecord(SESSION)?.options).toMatchObject({ model: 'reported' }) + }) + + it('never overwrites a pick made after the child reported', async () => { + await restartWith(publishFirst) + await heldBehindStart('hello') + dispatch.mockImplementationOnce(async () => { + // A pick lands while the first handover is still running. + await store.replaceSessionOptions({ + sessionId: SESSION, + fence: store.getRecord(SESSION)!.lease.runtimeFence, + options: { model: 'picked' }, + now: NOW + }) + return { state: 'admitted' as const } + }) + + await host.handleAdapterEvent({ ...startedEvent(), reportedOptions: { model: 'reported' } }) + await eventually(() => expect(dispatch).toHaveBeenCalled()) + await new Promise((resolve) => setTimeout(resolve, 50)) + await host.flushStreamedEvents(SESSION) + + expect(store.getRecord(SESSION)?.options).toEqual({ model: 'picked' }) + }) +}) + +function startupAttemptOpen(): boolean { + return host.collaboratorsForTests().runtimeState.startupAttempts.isOpen(SESSION) +} + describe('the attempt an acquire runs under', () => { - it('is minted by the host with the lease, the pinned launch and a fixed deadline', async () => { + it('is minted by the host with the lease, the pinned launch and its progress report', async () => { await restartWith(publishFirst) await heldBehindStart('hello') @@ -345,9 +430,9 @@ describe('the attempt an acquire runs under', () => { const [input] = acquire.mock.calls.at(-1)! expect(input).toMatchObject({ fence: record.lease.runtimeFence, - launch: { location: record.location, accountHome: record.accountHome }, - deadlineAt: NOW + STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS + launch: { location: record.location, accountHome: record.accountHome } }) + expect(input.onOutput).toEqual(expect.any(Function)) expect(input.attemptId).toEqual(expect.any(String)) expect(input.signal).toBeInstanceOf(AbortSignal) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts index 4c29d6f8339c..eed7303b6667 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts @@ -29,13 +29,12 @@ import { resetHostTestOperationIds } from './structured-agent-session-host-test-data' import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' -import { STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS } from './structured-agent-session-startup-attempt' const CALLER = { callerKey: 'client-1' } -// Adapters whose own acquire already publishes at spawn, with the test that drives the real one. -// Every other registration still runs its handshake inside acquire (the temporary bridge) until -// its adapter moves to publish at spawn, which is when it joins this map. +// Temporary, removed when every adapter publishes at spawn: adapters whose own acquire already does, +// with the test that drives the real one. Every other registration still runs its handshake inside +// acquire (the bridge) until then. const REAL_ADAPTER_PUBLISHES_AT_SPAWN: Readonly> = { claude: 'structured-agent-session-claude-hung-start-stop.test.ts' } @@ -142,7 +141,7 @@ describe.each( attemptId: expect.any(String), fence: record.lease.runtimeFence, launch: { location: record.location, accountHome: record.accountHome }, - deadlineAt: NOW + STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS + onOutput: expect.any(Function) }) const body = hostTestMessage('hello') diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts index ab5f9ff4bfe8..331ec1424b7e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts @@ -1,25 +1,27 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { AgentSessionRecord } from '../../../shared/agent-session-record' import { - STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS, + STRUCTURED_AGENT_SESSION_STARTUP_CEILING_MS as CEILING_MS, + STRUCTURED_AGENT_SESSION_STARTUP_SILENCE_MS as SILENCE_MS, StructuredAgentSessionStartupAttempts, mintStructuredAgentSessionStartupAttempt, - type StructuredAgentSessionExpiredStartup + type StructuredAgentSessionExpiredStartup, + type StructuredAgentSessionStartupSettled } from './structured-agent-session-startup-attempt' const SESSION = 'session-alpha' const CHILD = { generation: 'generation-1', fence: 3 } let expired: StructuredAgentSessionExpiredStartup[] +let settled: StructuredAgentSessionStartupSettled[] let attempts: StructuredAgentSessionStartupAttempts function mint() { - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the mint reads only the lease fence, location, account home, options and updatedAt this literal sets. + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the mint reads only the lease fence, location, account home and options this literal sets. const record = { lease: { runtimeFence: CHILD.fence }, location: { executionHostId: 'local', workspaceId: 'workspace-1' }, - accountHome: { variable: 'CODEX_HOME', path: '/home/dev/.codex' }, - updatedAt: 1 + accountHome: { variable: 'CODEX_HOME', path: '/home/dev/.codex' } } as unknown as AgentSessionRecord return mintStructuredAgentSessionStartupAttempt({ record, @@ -30,17 +32,25 @@ function mint() { agent: 'codex', providerHandle: null }, - spawnToken: 'spawn-1', - now: Date.now() + spawnToken: 'spawn-1' }) } +/** Output every `everyMs` for `forMs`. */ +function chatter(output: () => void, forMs: number, everyMs = 10_000): void { + for (let elapsed = 0; elapsed < forMs; elapsed += everyMs) { + vi.advanceTimersByTime(everyMs) + output() + } +} + beforeEach(() => { vi.useFakeTimers() expired = [] + settled = [] attempts = new StructuredAgentSessionStartupAttempts({ - now: () => Date.now(), - expire: (startup) => expired.push(startup) + expire: (startup) => expired.push(startup), + settled: (startup) => settled.push(startup) }) }) @@ -49,79 +59,175 @@ afterEach(() => { vi.useRealTimers() }) -describe('a startup attempt deadline', () => { - it('counts from the mint and fires once, naming the acquire still in flight', () => { +describe('the startup clock', () => { + it('does not run before the spawn: resolving a launch is not starting', () => { + attempts.track(SESSION, mint()) + + vi.advanceTimersByTime(CEILING_MS * 2) + + expect(expired).toEqual([]) + }) + + it('fails a spawned agent that says nothing for the silence limit, naming the acquire', () => { const attempt = mint() - attempts.track(SESSION, attempt) + attempts.track(SESSION, attempt).spawned() - vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS - 1) + vi.advanceTimersByTime(SILENCE_MS - 1) expect(expired).toEqual([]) vi.advanceTimersByTime(1) expect(expired).toEqual([{ sessionId: SESSION, attemptId: attempt.attemptId, child: null }]) - vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + expect(settled).toEqual([{ agent: 'codex', outcome: 'silent', durationMs: SILENCE_MS }]) + vi.advanceTimersByTime(CEILING_MS) expect(expired).toHaveLength(1) }) - it('names the starting child an acquire published', () => { + it('keeps a slow start that is still talking past the silence limit', () => { const attempt = mint() - attempts.track(SESSION, attempt) + const progress = attempts.track(SESSION, attempt) + progress.spawned() attempts.published(SESSION, attempt.attemptId, { ...CHILD, phase: 'starting' }) - vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + chatter(progress.output, SILENCE_MS * 3) + attempts.ready(SESSION, CHILD) + + expect(expired).toEqual([]) + expect(settled).toEqual([{ agent: 'codex', outcome: 'ready', durationMs: SILENCE_MS * 3 }]) + expect(attempts.isOpen(SESSION)).toBe(false) + }) + + it('times silence from the last output, not the spawn', () => { + const progress = attempts.track(SESSION, mint()) + progress.spawned() + vi.advanceTimersByTime(SILENCE_MS - 1_000) + progress.output() + + vi.advanceTimersByTime(SILENCE_MS - 1) + expect(expired).toEqual([]) + vi.advanceTimersByTime(1) + + expect(expired).toHaveLength(1) + }) + + it('fails a chatty agent that never becomes ready at the ceiling', () => { + const attempt = mint() + const progress = attempts.track(SESSION, attempt) + progress.spawned() + attempts.published(SESSION, attempt.attemptId, { ...CHILD, phase: 'starting' }) + + chatter(progress.output, CEILING_MS - 10_000) + expect(expired).toEqual([]) + chatter(progress.output, 10_000) expect(expired).toEqual([{ sessionId: SESSION, attemptId: attempt.attemptId, child: CHILD }]) + expect(settled).toEqual([{ agent: 'codex', outcome: 'ceiling', durationMs: CEILING_MS }]) + }) + + it('starts on output or publication when no spawn was reported', () => { + const byOutput = mint() + attempts.track(SESSION, byOutput).output() + const byPublish = mint() + attempts.track('session-beta', byPublish) + attempts.published('session-beta', byPublish.attemptId, { ...CHILD, phase: 'starting' }) + + vi.advanceTimersByTime(SILENCE_MS) + + expect(expired.map((startup) => startup.attemptId).sort()).toEqual( + [byOutput.attemptId, byPublish.attemptId].sort() + ) }) +}) - it('ends with the start its child proved, and ignores a stale child’s proof', () => { +describe('an attempt ends', () => { + it('with the start its child proved, and ignores a stale child’s proof', () => { const attempt = mint() - attempts.track(SESSION, attempt) + attempts.track(SESSION, attempt).spawned() attempts.published(SESSION, attempt.attemptId, { ...CHILD, phase: 'starting' }) attempts.ready(SESSION, { ...CHILD, generation: 'generation-0' }) - vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS / 2) + expect(attempts.isOpen(SESSION)).toBe(true) attempts.ready(SESSION, CHILD) - vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + vi.advanceTimersByTime(CEILING_MS) expect(expired).toEqual([]) + expect(attempts.isOpen(SESSION)).toBe(false) }) - it('ends with an acquire that answered ready, or an attach that failed', () => { - const ready = mint() - attempts.track(SESSION, ready) - attempts.published(SESSION, ready.attemptId, { ...CHILD, phase: 'ready' }) - const failed = mint() - attempts.track('session-beta', failed) - attempts.abandon('session-beta', failed.attemptId) + it('with its child, however the child ended, leaving no entry or timer', () => { + const attempt = mint() + attempts.track(SESSION, attempt).spawned() + attempts.published(SESSION, attempt.attemptId, { ...CHILD, phase: 'starting' }) - vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + attempts.childEnded(SESSION, { ...CHILD, generation: 'generation-0' }) + expect(attempts.isOpen(SESSION)).toBe(true) + attempts.childEnded(SESSION, CHILD) - expect(expired).toEqual([]) + expect(attempts.isOpen(SESSION)).toBe(false) + expect(vi.getTimerCount()).toBe(0) + expect(settled.map((startup) => startup.outcome)).toEqual(['ended']) }) - it('expires a child published after its deadline passed, once it is published', () => { + it('once an expiry has stopped its published child', () => { const attempt = mint() - attempts.track(SESSION, attempt) - vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) - + attempts.track(SESSION, attempt).spawned() attempts.published(SESSION, attempt.attemptId, { ...CHILD, phase: 'starting' }) - expect(expired.map((startup) => startup.child)).toEqual([null, CHILD]) + vi.advanceTimersByTime(SILENCE_MS) + + expect(expired).toHaveLength(1) + expect(attempts.isOpen(SESSION)).toBe(false) + expect(vi.getTimerCount()).toBe(0) }) - it('is replaced by the next attempt, and stops at quit', () => { + it('when an acquire the expiry aborted is abandoned, or publishes late and is expired then', () => { + const abandoned = mint() + attempts.track(SESSION, abandoned).spawned() + vi.advanceTimersByTime(SILENCE_MS) + expect(attempts.isOpen(SESSION)).toBe(true) + attempts.abandon(SESSION, abandoned.attemptId) + expect(attempts.isOpen(SESSION)).toBe(false) + + const late = mint() + attempts.track(SESSION, late).spawned() + vi.advanceTimersByTime(SILENCE_MS) + attempts.published(SESSION, late.attemptId, { ...CHILD, phase: 'starting' }) + + expect(expired.map((startup) => startup.child)).toEqual([null, null, CHILD]) + expect(attempts.isOpen(SESSION)).toBe(false) + expect(vi.getTimerCount()).toBe(0) + // Each start is measured once, at the limit that ended it. + expect(settled.map((startup) => startup.outcome)).toEqual(['silent', 'silent']) + }) + + it('with an acquire that answered ready, or an attach that failed', () => { + const ready = mint() + attempts.track(SESSION, ready).spawned() + attempts.published(SESSION, ready.attemptId, { ...CHILD, phase: 'ready' }) + const failed = mint() + attempts.track('session-beta', failed).spawned() + attempts.abandon('session-beta', failed.attemptId) + + vi.advanceTimersByTime(CEILING_MS) + + expect(expired).toEqual([]) + expect(vi.getTimerCount()).toBe(0) + }) + + it('when the next attempt replaces it, and every clock stops at quit', () => { const first = mint() - attempts.track(SESSION, first) - vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS / 2) + const firstProgress = attempts.track(SESSION, first) + firstProgress.spawned() const second = mint() - attempts.track(SESSION, second) + attempts.track(SESSION, second).spawned() + // The replaced attempt's reports reach nothing. + firstProgress.output() attempts.abandon(SESSION, first.attemptId) + expect(attempts.isOpen(SESSION)).toBe(true) - vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS / 2) - expect(expired).toEqual([]) attempts.dispose() - vi.advanceTimersByTime(STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + vi.advanceTimersByTime(CEILING_MS) expect(expired).toEqual([]) + expect(attempts.isOpen(SESSION)).toBe(false) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts index fcbab3c90338..4b21fe477a0a 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts @@ -4,9 +4,11 @@ // conversation and lease it is for, what cancels it, when it gives up, and when its child may take // input. An adapter only runs its protocol inside the attempt it is handed. // -// The deadline counts from the mint and nothing renews it: a start that has not proved itself by -// then is not coming. Expiry re-derives what to end from the host's own state, so a timer that -// outlives its attempt ends nothing. +// The clock starts when the process exists, never before (resolving a launch is not starting it). +// A start that says nothing for a minute is wedged; one still talking is given up only at the +// ceiling, so a slow migration or history replay is not killed mid-way while every start still +// ends. Expiry re-derives what to end from the host's own state, so a timer that outlives its +// attempt ends nothing. import { randomUUID } from 'node:crypto' import type { AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' @@ -14,8 +16,19 @@ import type { AgentSessionRecord } from '../../../shared/agent-session-record' import type { AgentChildWorkEvidence } from '../../../shared/agent-status-child-work-evidence' import type { StructuredAgentSessionEventSink } from './structured-agent-session-event-sink' import type { StructuredAgentSessionProviderChildIdentity } from './structured-agent-session-host-types' +import { sameProviderChild } from './structured-agent-session-provider-child' -export const STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS = 60_000 +/** A spawned child with no stdout frame or stderr line for this long has stopped starting. */ +export const STRUCTURED_AGENT_SESSION_STARTUP_SILENCE_MS = 60_000 +/** No start, however chatty, outlives this from its spawn. */ +export const STRUCTURED_AGENT_SESSION_STARTUP_CEILING_MS = 10 * 60_000 + +export type StructuredAgentSessionStartupLimits = { silenceMs: number; ceilingMs: number } + +export const STRUCTURED_AGENT_SESSION_STARTUP_LIMITS: StructuredAgentSessionStartupLimits = { + silenceMs: STRUCTURED_AGENT_SESSION_STARTUP_SILENCE_MS, + ceilingMs: STRUCTURED_AGENT_SESSION_STARTUP_CEILING_MS +} export type StructuredAgentSessionStartupAttempt = { /** The host's generation for this start; never reused, even when the lease fence is. */ @@ -27,18 +40,16 @@ export type StructuredAgentSessionStartupAttempt = { readonly launch: Pick /** The saved options the start launches with, as intent; never a catalog guess. */ readonly options?: Readonly> - /** The record write `options` was read from: a pick after it is newer intent to reconcile. */ - readonly optionsAsOf: number /** Provider events may begin before acquisition returns. */ readonly events?: StructuredAgentSessionEventSink /** Background work this start's child reports, scoped to the attempt. Unset until the host routes * child work by attempt; adapters report through their registration until then. */ readonly childWork?: (evidence: AgentChildWorkEvidence[]) => void - /** Aborted by a close, a Stop admitted now, quit, or the deadline: the adapter stops what it + /** Aborted by a close, a Stop admitted now, quit, or the startup limit: the adapter stops what it * started and the acquire fails. */ readonly signal?: AbortSignal - /** Host clock. */ - readonly deadlineAt: number + /** Any stdout frame or stderr line from the child: proof the start is still moving. */ + readonly onOutput?: () => void } export function mintStructuredAgentSessionStartupAttempt(input: { @@ -47,8 +58,6 @@ export function mintStructuredAgentSessionStartupAttempt(input: { spawnToken: string events?: StructuredAgentSessionEventSink signal?: AbortSignal - now: number - deadlineMs?: number }): StructuredAgentSessionStartupAttempt { const { record } = input return { @@ -62,17 +71,15 @@ export function mintStructuredAgentSessionStartupAttempt(input: { ...(record.launchDirectory === undefined ? {} : { launchDirectory: record.launchDirectory }) }, ...(record.options ? { options: record.options } : {}), - optionsAsOf: record.updatedAt, ...(input.events ? { events: input.events } : {}), - ...(input.signal ? { signal: input.signal } : {}), - deadlineAt: input.now + (input.deadlineMs ?? STRUCTURED_AGENT_SESSION_STARTUP_DEADLINE_MS) + ...(input.signal ? { signal: input.signal } : {}) } } -/** The deadline's own reason, so an expired start is told from a close, Stop or quit. */ +/** The startup limit's own reason, so an expired start is told from a close, Stop or quit. */ export class StructuredAgentSessionStartupExpiredError extends Error { constructor() { - super('the agent did not finish starting before its deadline') + super('the agent stopped making progress before it finished starting') this.name = 'StructuredAgentSessionStartupExpiredError' } } @@ -89,44 +96,91 @@ export type StructuredAgentSessionExpiredStartup = { child: StructuredAgentSessionProviderChildIdentity | null } +/** How one spawned start ended, for tuning the limits from what agents really take. */ +export type StructuredAgentSessionStartupSettled = { + agent: AgentSessionJournalIdentity['agent'] + outcome: 'ready' | 'silent' | 'ceiling' | 'ended' + /** From spawn to the outcome. */ + durationMs: number +} + +/** What the host learns about a start while its acquire runs. */ +export type StructuredAgentSessionStartupProgress = { + /** The process exists: the clock starts. */ + spawned: () => void + /** Any output; output before a spawn report also starts the clock. */ + output: () => void +} + type Tracked = { attempt: StructuredAgentSessionStartupAttempt child: StructuredAgentSessionProviderChildIdentity | null + spawnedAt: number | null + lastOutputAt: number timer: ReturnType | null expired: boolean } -/** Each session's open attempt and its deadline. A session starts one child at a time, so a new - * attempt replaces any older one. */ +/** Each session's open attempt and its startup clock. A session starts one child at a time, so a + * new attempt replaces any older one. */ export class StructuredAgentSessionStartupAttempts { private readonly open = new Map() private disposed = false + private readonly limits: StructuredAgentSessionStartupLimits + private readonly now: () => number constructor( private readonly deps: { - now: () => number /** Runs outside the session's queue: an acquire may hold it for the whole handshake. */ expire: (expired: StructuredAgentSessionExpiredStartup) => void + /** Never throws back into the clock. */ + settled?: (settled: StructuredAgentSessionStartupSettled) => void + limits?: Partial + /** Real elapsed time: the host's own `now` may be pinned. */ + now?: () => number } - ) {} + ) { + this.limits = { ...STRUCTURED_AGENT_SESSION_STARTUP_LIMITS, ...deps.limits } + this.now = deps.now ?? Date.now + } - track(sessionId: string, attempt: StructuredAgentSessionStartupAttempt): void { + track( + sessionId: string, + attempt: StructuredAgentSessionStartupAttempt + ): StructuredAgentSessionStartupProgress { this.end(sessionId) - const tracked: Tracked = { attempt, child: null, timer: null, expired: false } + const tracked: Tracked = { + attempt, + child: null, + spawnedAt: null, + lastOutputAt: 0, + timer: null, + expired: false + } this.open.set(sessionId, tracked) - if (this.disposed) { - return + const current = (): boolean => this.open.get(sessionId) === tracked && !this.disposed + return { + spawned: () => { + if (current()) { + this.startClock(sessionId, tracked) + } + }, + output: () => { + if (!current()) { + return + } + if (tracked.spawnedAt === null) { + this.startClock(sessionId, tracked) + return + } + // Read when the timer fires, so a chatty child costs no timer churn. + tracked.lastOutputAt = this.now() + } } - tracked.timer = setTimeout( - () => this.expire(sessionId, tracked), - Math.max(0, attempt.deadlineAt - this.deps.now()) - ) - // A start's deadline must never be the reason a process stays alive at quit. - tracked.timer.unref?.() } - /** The acquire returned: a `starting` child stays on the deadline until it proves its start, and - * one published after its deadline passed is expired now. */ + /** The acquire returned: a `starting` child stays on the clock until it proves its start, and + * one published after the limit passed is expired now. */ published( sessionId: string, attemptId: string, @@ -137,31 +191,50 @@ export class StructuredAgentSessionStartupAttempts { return } if (child.phase === 'ready') { - this.end(sessionId) + this.settle(sessionId, tracked, 'ready') return } tracked.child = { generation: child.generation, fence: child.fence } if (tracked.expired) { - this.expire(sessionId, tracked) + this.expire(sessionId, tracked, null) + return + } + // A child published without a spawn report has existed since at least now. + if (tracked.spawnedAt === null && !this.disposed) { + this.startClock(sessionId, tracked) } } /** The child proved its start: its attempt is over. A stale child's proof ends nothing. */ ready(sessionId: string, child: StructuredAgentSessionProviderChildIdentity): void { const tracked = this.open.get(sessionId) - if (tracked?.child?.generation === child.generation && tracked.child.fence === child.fence) { - this.end(sessionId) + if (tracked?.child && sameProviderChild(tracked.child, child)) { + this.settle(sessionId, tracked, 'ready') + } + } + + /** The child ended (exit, crash, Stop, close): nothing is left for its attempt to time. */ + childEnded(sessionId: string, child: StructuredAgentSessionProviderChildIdentity): void { + const tracked = this.open.get(sessionId) + if (tracked?.child && sameProviderChild(tracked.child, child)) { + this.settle(sessionId, tracked, 'ended') } } /** The attempt failed, or its attach did: nothing it started is left to time out. */ abandon(sessionId: string, attemptId: string): void { - if (this.open.get(sessionId)?.attempt.attemptId === attemptId) { - this.end(sessionId) + const tracked = this.open.get(sessionId) + if (tracked?.attempt.attemptId === attemptId) { + this.settle(sessionId, tracked, 'ended') } } - /** Quit: no deadline fires after this. */ + /** Whether the session still has an attempt open; for tests and diagnostics. */ + isOpen(sessionId: string): boolean { + return this.open.has(sessionId) + } + + /** Quit: no limit fires after this. */ dispose(): void { this.disposed = true for (const tracked of this.open.values()) { @@ -172,12 +245,81 @@ export class StructuredAgentSessionStartupAttempts { this.open.clear() } - private expire(sessionId: string, tracked: Tracked): void { + private startClock(sessionId: string, tracked: Tracked): void { + if (tracked.spawnedAt !== null) { + return + } + tracked.spawnedAt = this.now() + tracked.lastOutputAt = tracked.spawnedAt + this.schedule(sessionId, tracked) + } + + private schedule(sessionId: string, tracked: Tracked): void { + if (tracked.spawnedAt === null) { + return + } + const due = Math.min( + tracked.lastOutputAt + this.limits.silenceMs, + tracked.spawnedAt + this.limits.ceilingMs + ) + tracked.timer = setTimeout(() => this.check(sessionId, tracked), Math.max(0, due - this.now())) + // A start's clock must never be the reason a process stays alive at quit. + tracked.timer.unref?.() + } + + private check(sessionId: string, tracked: Tracked): void { + tracked.timer = null + if (this.open.get(sessionId) !== tracked || this.disposed || tracked.spawnedAt === null) { + return + } + const now = this.now() + if (now - tracked.spawnedAt >= this.limits.ceilingMs) { + this.expire(sessionId, tracked, 'ceiling') + } else if (now - tracked.lastOutputAt >= this.limits.silenceMs) { + this.expire(sessionId, tracked, 'silent') + } else { + this.schedule(sessionId, tracked) + } + } + + /** `reason` null: a start whose limit already passed inside its acquire was published now. */ + private expire(sessionId: string, tracked: Tracked, reason: 'silent' | 'ceiling' | null): void { if (this.open.get(sessionId) !== tracked || this.disposed) { return } + if (reason) { + this.report(tracked, reason) + } tracked.expired = true - this.deps.expire({ sessionId, attemptId: tracked.attempt.attemptId, child: tracked.child }) + const { child } = tracked + // A published child's stop is under way and ends nothing a newer attempt owns; one still + // acquiring stays tracked until its aborted acquire is abandoned or published. + if (child) { + this.end(sessionId) + } + this.deps.expire({ sessionId, attemptId: tracked.attempt.attemptId, child }) + } + + private settle(sessionId: string, tracked: Tracked, outcome: 'ready' | 'ended'): void { + if (!tracked.expired) { + this.report(tracked, outcome) + } + this.end(sessionId) + } + + private report(tracked: Tracked, outcome: StructuredAgentSessionStartupSettled['outcome']): void { + if (tracked.spawnedAt === null || this.disposed) { + return + } + try { + this.deps.settled?.({ + agent: tracked.attempt.identity.agent, + outcome, + durationMs: Math.max(0, this.now() - tracked.spawnedAt) + }) + } catch { + // Measurement is bookkeeping: it never decides a start. + } } private end(sessionId: string): void { diff --git a/src/main/observability/agent-session-instrumentation.ts b/src/main/observability/agent-session-instrumentation.ts index 693d96a28c20..980f0d61c054 100644 --- a/src/main/observability/agent-session-instrumentation.ts +++ b/src/main/observability/agent-session-instrumentation.ts @@ -1,4 +1,4 @@ -import { withSpan, type ActiveSpan } from './tracer' +import { startSpan, withSpan, type ActiveSpan } from './tracer' export type AgentSessionCreatePhase = | 'reconcile_leases' @@ -24,6 +24,23 @@ export async function withAgentSessionSpan(fn: (span: ActiveSpan) => Promise< return withSpan('agentSession.create', fn, { attributes: { kind: 'agent-session' } }) } +/** One spawned start's outcome and its spawn-to-outcome time, so the startup limits are tuned from + * what each agent really takes. Agent id and numbers only. */ +export function recordAgentSessionStartup(startup: { + agent: string + outcome: string + durationMs: number +}): void { + startSpan('agentSession.startup', { + attributes: { + kind: 'agent-session', + 'agent_session.agent': startup.agent, + 'agent_session.startup.outcome': startup.outcome, + 'agent_session.startup.spawn_to_outcome_ms': Math.round(startup.durationMs) + } + }).end() +} + export async function withAgentSessionCreatePhase( phase: AgentSessionCreatePhase, record: AgentSessionCreatePhaseRecorder | undefined, diff --git a/src/main/pi/rpc-session.ts b/src/main/pi/rpc-session.ts index 7796ae3a1024..ec6427a3f0c9 100644 --- a/src/main/pi/rpc-session.ts +++ b/src/main/pi/rpc-session.ts @@ -117,6 +117,7 @@ export class PiRpcSession { this.connection = ( deps.openConnection ?? ((spec, handlers) => new JsonlRpcAgentConnection(spec, handlers)) )(launch, { + ...(input.onOutput ? { onOutput: input.onOutput } : {}), onRecord: (frame) => frame.type === 'extension_ui_request' ? this.dialogs.receive(frame) diff --git a/src/main/provider-process/managed-provider-process-output.test.ts b/src/main/provider-process/managed-provider-process-output.test.ts new file mode 100644 index 000000000000..1973d83918dc --- /dev/null +++ b/src/main/provider-process/managed-provider-process-output.test.ts @@ -0,0 +1,58 @@ +import { EventEmitter } from 'node:events' +import { PassThrough } from 'node:stream' +import { describe, expect, it, vi } from 'vitest' +import type { spawnProcess } from '../../shared/child-process/run-process' +import { spawnManagedProviderProcess } from './managed-provider-process' + +function launchWithOutput(onOutput: () => void) { + const child = Object.assign(new EventEmitter(), { + pid: 9_999_999, + stdin: new PassThrough(), + stdout: new PassThrough(), + stderr: new PassThrough(), + kill: vi.fn(() => true) + }) + const managed = spawnManagedProviderProcess( + { command: 'fixture-provider', args: [] }, + { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The managed lifecycle reads only events, pid, streams and kill from this fixture. + spawnImpl: () => child as unknown as ReturnType, + platform: 'win32', + site: 'fixture-provider-teardown', + onOutput + } + ) + return { child, managed } +} + +const tick = (): Promise => new Promise((resolve) => setImmediate(resolve)) + +describe('managed provider process output', () => { + it('reports stderr and stdout chunks as output', async () => { + const onOutput = vi.fn() + const { child } = launchWithOutput(onOutput) + child.stdout.on('data', () => {}) + await tick() + + child.stderr.write('migrating\n') + child.stdout.write('{"id":1}\n') + await tick() + + expect(onOutput).toHaveBeenCalledTimes(2) + }) + + it('never starts stdout before its reader subscribes, so a late reader loses nothing', async () => { + const onOutput = vi.fn() + const { child } = launchWithOutput(onOutput) + child.stdout.write('early frame\n') + await tick() + expect(onOutput).not.toHaveBeenCalled() + + const read: string[] = [] + child.stdout.setEncoding('utf8').on('data', (chunk: string) => read.push(chunk)) + await tick() + + expect(read.join('')).toBe('early frame\n') + expect(onOutput).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/provider-process/managed-provider-process.ts b/src/main/provider-process/managed-provider-process.ts index c256aeda270f..c7b230f01b02 100644 --- a/src/main/provider-process/managed-provider-process.ts +++ b/src/main/provider-process/managed-provider-process.ts @@ -36,6 +36,8 @@ type ManagedProviderProcessOptions = { inheritedEnv?: NodeJS.ProcessEnv /** Defaults to "the root is gone". */ acceptClose?: (result: ProviderProcessCloseResult) => boolean + /** Any stdout or stderr chunk: the child is doing something. */ + onOutput?: () => void } export type ManagedProviderProcess = { @@ -91,7 +93,11 @@ export function spawnManagedProviderProcess( // An undrained stderr pipe blocks the child once it fills. child.stderr.setEncoding('utf8').on('data', (chunk: string) => { stderrTail = (stderrTail + chunk).slice(-STDERR_TAIL_MAX_CHARS) + options.onOutput?.() }) + if (options.onOutput) { + observeReaderOutput(child.stdout, options.onOutput) + } let resolveExit = (): void => {} const exitPromise = new Promise((resolve) => { resolveExit = resolve @@ -170,3 +176,20 @@ export function spawnManagedProviderProcess( } } } + +/** Watches stdout only once its reader subscribes: a listener of our own would start the stream + * flowing and drop whatever arrived before a reader that subscribes late. */ +function observeReaderOutput( + stdout: Pick, + onOutput: () => void +): void { + const onSubscribe = (event: string | symbol): void => { + if (event !== 'data' && event !== 'readable') { + return + } + stdout.removeListener('newListener', onSubscribe) + // After the reader's own listener lands; no chunk can arrive before a microtask runs. + queueMicrotask(() => stdout.on('data', onOutput)) + } + stdout.on('newListener', onSubscribe) +} diff --git a/src/main/runtime/rpc/methods/structured-agent-session-at-rest.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-at-rest.test.ts index 1e5bb7e6bc37..f866eedfacc4 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-at-rest.test.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-at-rest.test.ts @@ -473,7 +473,7 @@ describe('every close withdraws what is queued (P2-29)', () => { await restingChat() // The delivery loop has not reached its first start yet. const { loop } = rig.host.collaboratorsForTests().conversationDelivery - vi.spyOn(loop, 'wake').mockImplementation(() => undefined) + vi.spyOn(loop, 'wake').mockImplementation(() => Promise.resolve()) const reader: unknown[] = [] await rig.host.subscribe({ id: 'reader', diff --git a/src/main/runtime/structured-claude-runtime-adapter.ts b/src/main/runtime/structured-claude-runtime-adapter.ts index d9abf1cf3885..acb5cb9e207a 100644 --- a/src/main/runtime/structured-claude-runtime-adapter.ts +++ b/src/main/runtime/structured-claude-runtime-adapter.ts @@ -55,7 +55,11 @@ export type StructuredClaudeRuntimeAdapterDeps = { export function structuredClaudeLifecycleEvent( event: ClaudeStructuredSessionEvent ): StructuredAgentSessionLifecycleEvent | null { - if (event.type === 'started' || event.type === 'options-skipped') { + if ( + event.type === 'started' || + event.type === 'options-reported' || + event.type === 'options-skipped' + ) { return event } // Every exit of a child with an identity, expected or not: the host ends that child's record. From 93ac98be80bbaaad79ee7398154f8707aaf1d9b4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 7 Oct 2026 22:22:44 -0700 Subject: [PATCH 3/5] fix(native-chat): keep the startup attempt's contract free of host and process code The adapter contract imported the attempt type from the module that also runs its clock and imports the host types, which pulled the model catalog and process-scan code into the mobile test typecheck. The attempt's shape and limits now live in a dependency-free contract module. --- .../structured-agent-session-adapter.ts | 2 +- .../structured-agent-session-attach-flow.ts | 6 +-- ...ured-agent-session-attach-orchestration.ts | 6 +-- .../structured-agent-session-host-types.ts | 2 +- ...-agent-session-startup-attempt-contract.ts | 41 +++++++++++++++++++ ...agent-session-startup-attempt-host.test.ts | 2 +- ...ured-agent-session-startup-attempt.test.ts | 4 +- ...tructured-agent-session-startup-attempt.ts | 40 +++--------------- 8 files changed, 56 insertions(+), 47 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts index 6396fe17fae5..2374be60fd36 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts @@ -46,7 +46,7 @@ export type { import type { AgentJournalDispatchRejection } from '../../../shared/agent-session-failure-words' import type { AgentSessionPromptResponse } from '../../../shared/agent-session-question-answer' import type { ProviderHistoryWindow } from '../agent-session-journal/journal-submission-reconciler' -import type { StructuredAgentSessionStartupAttempt } from './structured-agent-session-startup-attempt' +import type { StructuredAgentSessionStartupAttempt } from './structured-agent-session-startup-attempt-contract' import type { AgentSessionCreatePhaseRecorder } from '../../observability/agent-session-instrumentation' export class AgentSessionAcquisitionRefusal extends Error { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts index 6514e4361df0..4a69f4a37c89 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts @@ -47,10 +47,8 @@ import { } from '../../observability/agent-session-instrumentation' import type { ProviderHistoryWindow } from '../agent-session-journal/journal-submission-reconciler' import { readProviderHistoryWindow } from './structured-agent-session-provider-history-window' -import type { - StructuredAgentSessionStartupAttempt, - StructuredAgentSessionStartupProgress -} from './structured-agent-session-startup-attempt' +import type { StructuredAgentSessionStartupAttempt } from './structured-agent-session-startup-attempt-contract' +import type { StructuredAgentSessionStartupProgress } from './structured-agent-session-startup-attempt' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { StructuredAgentSessionLogger } from './structured-agent-session-logger' diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts index 18662e44566e..e6d3a55e5fad 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts @@ -36,10 +36,8 @@ import { } from './structured-agent-session-provider-child' import type { DeferredStructuredAgentSessionEventSink } from './structured-agent-session-event-sink' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' -import { - isStructuredAgentSessionStartupExpired, - type StructuredAgentSessionStartupAttempt -} from './structured-agent-session-startup-attempt' +import { isStructuredAgentSessionStartupExpired } from './structured-agent-session-startup-attempt' +import type { StructuredAgentSessionStartupAttempt } from './structured-agent-session-startup-attempt-contract' import { addAgentSessionCreatePhaseAttributes, withAgentSessionCreatePhase, 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 cd670d7f8bf3..e0062f8050c0 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 @@ -25,7 +25,7 @@ import type { AgentModelCatalogService } from '../agent-model-catalog/agent-mode import type { StructuredAgentSessionLogger } from './structured-agent-session-logger' import type { StructuredAgentId } from '../../../shared/agent-session-provider-handle' import type { StructuredAgentRegistry } from './structured-agent-registry' -import type { StructuredAgentSessionStartupLimits } from './structured-agent-session-startup-attempt' +import type { StructuredAgentSessionStartupLimits } from './structured-agent-session-startup-attempt-contract' export type StructuredAgentSessionCaller = { callerKey: string } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts new file mode 100644 index 000000000000..95517a463d78 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts @@ -0,0 +1,41 @@ +// The startup attempt's shape and limits, apart from its clock: the adapter contract reaches this +// module, so it must not pull in the host or Node-only process code. + +import type { AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import type { AgentChildWorkEvidence } from '../../../shared/agent-status-child-work-evidence' +import type { StructuredAgentSessionEventSink } from './structured-agent-session-event-sink' + +/** A spawned child with no stdout frame or stderr line for this long has stopped starting. */ +export const STRUCTURED_AGENT_SESSION_STARTUP_SILENCE_MS = 60_000 +/** No start, however chatty, outlives this from its spawn. */ +export const STRUCTURED_AGENT_SESSION_STARTUP_CEILING_MS = 10 * 60_000 + +export type StructuredAgentSessionStartupLimits = { silenceMs: number; ceilingMs: number } + +export const STRUCTURED_AGENT_SESSION_STARTUP_LIMITS: StructuredAgentSessionStartupLimits = { + silenceMs: STRUCTURED_AGENT_SESSION_STARTUP_SILENCE_MS, + ceilingMs: STRUCTURED_AGENT_SESSION_STARTUP_CEILING_MS +} + +export type StructuredAgentSessionStartupAttempt = { + /** The host's generation for this start; never reused, even when the lease fence is. */ + readonly attemptId: string + readonly identity: AgentSessionJournalIdentity + readonly fence: number + readonly spawnToken: string + /** Where and as whom the record pins the start; an adapter never re-resolves either. */ + readonly launch: Pick + /** The saved options the start launches with, as intent; never a catalog guess. */ + readonly options?: Readonly> + /** Provider events may begin before acquisition returns. */ + readonly events?: StructuredAgentSessionEventSink + /** Background work this start's child reports, scoped to the attempt. Unset until the host routes + * child work by attempt; adapters report through their registration until then. */ + readonly childWork?: (evidence: AgentChildWorkEvidence[]) => void + /** Aborted by a close, a Stop admitted now, quit, or the startup limit: the adapter stops what it + * started and the acquire fails. */ + readonly signal?: AbortSignal + /** Any stdout frame or stderr line from the child: proof the start is still moving. */ + readonly onOutput?: () => void +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts index d856ecaf50ba..9de2bb8ec53e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts @@ -32,7 +32,7 @@ import { resetHostTestOperationIds } from './structured-agent-session-host-test-data' import { createStructuredAgentSessionLogger } from './structured-agent-session-logger' -import type { StructuredAgentSessionStartupLimits } from './structured-agent-session-startup-attempt' +import type { StructuredAgentSessionStartupLimits } from './structured-agent-session-startup-attempt-contract' const CALLER = { callerKey: 'client-1' } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts index 331ec1424b7e..52e38e13a000 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts @@ -2,7 +2,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { AgentSessionRecord } from '../../../shared/agent-session-record' import { STRUCTURED_AGENT_SESSION_STARTUP_CEILING_MS as CEILING_MS, - STRUCTURED_AGENT_SESSION_STARTUP_SILENCE_MS as SILENCE_MS, + STRUCTURED_AGENT_SESSION_STARTUP_SILENCE_MS as SILENCE_MS +} from './structured-agent-session-startup-attempt-contract' +import { StructuredAgentSessionStartupAttempts, mintStructuredAgentSessionStartupAttempt, type StructuredAgentSessionExpiredStartup, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts index 4b21fe477a0a..2af55be05982 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts @@ -13,44 +13,14 @@ import { randomUUID } from 'node:crypto' import type { AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' import type { AgentSessionRecord } from '../../../shared/agent-session-record' -import type { AgentChildWorkEvidence } from '../../../shared/agent-status-child-work-evidence' import type { StructuredAgentSessionEventSink } from './structured-agent-session-event-sink' import type { StructuredAgentSessionProviderChildIdentity } from './structured-agent-session-host-types' import { sameProviderChild } from './structured-agent-session-provider-child' - -/** A spawned child with no stdout frame or stderr line for this long has stopped starting. */ -export const STRUCTURED_AGENT_SESSION_STARTUP_SILENCE_MS = 60_000 -/** No start, however chatty, outlives this from its spawn. */ -export const STRUCTURED_AGENT_SESSION_STARTUP_CEILING_MS = 10 * 60_000 - -export type StructuredAgentSessionStartupLimits = { silenceMs: number; ceilingMs: number } - -export const STRUCTURED_AGENT_SESSION_STARTUP_LIMITS: StructuredAgentSessionStartupLimits = { - silenceMs: STRUCTURED_AGENT_SESSION_STARTUP_SILENCE_MS, - ceilingMs: STRUCTURED_AGENT_SESSION_STARTUP_CEILING_MS -} - -export type StructuredAgentSessionStartupAttempt = { - /** The host's generation for this start; never reused, even when the lease fence is. */ - readonly attemptId: string - readonly identity: AgentSessionJournalIdentity - readonly fence: number - readonly spawnToken: string - /** Where and as whom the record pins the start; an adapter never re-resolves either. */ - readonly launch: Pick - /** The saved options the start launches with, as intent; never a catalog guess. */ - readonly options?: Readonly> - /** Provider events may begin before acquisition returns. */ - readonly events?: StructuredAgentSessionEventSink - /** Background work this start's child reports, scoped to the attempt. Unset until the host routes - * child work by attempt; adapters report through their registration until then. */ - readonly childWork?: (evidence: AgentChildWorkEvidence[]) => void - /** Aborted by a close, a Stop admitted now, quit, or the startup limit: the adapter stops what it - * started and the acquire fails. */ - readonly signal?: AbortSignal - /** Any stdout frame or stderr line from the child: proof the start is still moving. */ - readonly onOutput?: () => void -} +import { + STRUCTURED_AGENT_SESSION_STARTUP_LIMITS, + type StructuredAgentSessionStartupAttempt, + type StructuredAgentSessionStartupLimits +} from './structured-agent-session-startup-attempt-contract' export function mintStructuredAgentSessionStartupAttempt(input: { record: AgentSessionRecord From 0abb820f7eb0dd9783e5efb7bcf437c26f8358f4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 8 Oct 2026 00:32:36 -0700 Subject: [PATCH 4/5] fix(native-chat): never let a stale option report undo a later pick A report of the child's options is now persisted only if no pick, no model-not-found heal and no newer report reached the chat since its read began. The host keeps one option revision per chat; the startup attempt hands the adapter a reader for it, and each report carries the revision it was read at. This replaces the record-snapshot check `started` had and covers the settings report, which had none, so a settings read answered just after a pick can no longer write the older model back. Also: the option read a ready child does inside its start now ends with the start (limit, Stop or close); a start that turns ready as the limit fires is recorded as ready; the startup-attempt host tests run under the Node SQLite runtime; and the retired-model test follows the new startup order. --- .../scripts/vitest-sqlite-runtime-files.mjs | 2 + .../claude/claude-structured-init-proof.ts | 55 +++++++ .../claude-structured-session-acquisition.ts | 3 +- ...aude-structured-session-journal-control.ts | 2 +- .../claude-structured-session-startup.test.ts | 4 +- .../claude-structured-session-startup.ts | 69 ++------- ...-agent-session-accept-then-deliver.test.ts | 5 +- .../structured-agent-session-acquisition.ts | 34 +++-- .../structured-agent-session-adapter.ts | 3 + .../structured-agent-session-attach-flow.ts | 2 + ...ured-agent-session-attach-orchestration.ts | 1 + ...red-agent-session-conversation-lifetime.ts | 6 +- ...structured-agent-session-host-mutations.ts | 2 + ...ctured-agent-session-host-runtime-state.ts | 2 + .../structured-agent-session-host.ts | 1 + ...ructured-agent-session-mutation-context.ts | 3 + ...ructured-agent-session-option-revisions.ts | 31 ++++ ...gent-session-provider-child-record.test.ts | 3 +- ...ructured-agent-session-provider-started.ts | 49 +++--- ...session-queued-message-rig.test-fixture.ts | 3 +- ...session-send-restarts-failed-start.test.ts | 3 +- ...-agent-session-startup-attempt-contract.ts | 2 + ...agent-session-startup-attempt-host.test.ts | 142 ++++++++++++++++-- ...sion-startup-attempt-registrations.test.ts | 3 +- ...ured-agent-session-startup-attempt.test.ts | 16 +- ...tructured-agent-session-startup-attempt.ts | 42 ++++-- ...ude-structured-retired-saved-model.test.ts | 16 +- ...ured-agent-session-failure-logging.test.ts | 3 +- 28 files changed, 374 insertions(+), 133 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-option-revisions.ts diff --git a/config/scripts/vitest-sqlite-runtime-files.mjs b/config/scripts/vitest-sqlite-runtime-files.mjs index f1820fd89065..a8d18883e6a4 100644 --- a/config/scripts/vitest-sqlite-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-runtime-files.mjs @@ -125,6 +125,8 @@ export const SQLITE_RUNTIME_INCLUDE = [ 'src/main/native-chat/agent-session-wire/structured-agent-session-shutdown-cut-row.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-stale-turn-verdict.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-start-failure-writer.test.ts', + 'src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts', + 'src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-startup-reconcile-failure.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-status-feed-child-work.test.ts', 'src/main/native-chat/agent-session-wire/structured-agent-session-status-feed-child.test.ts', diff --git a/src/main/claude/claude-structured-init-proof.ts b/src/main/claude/claude-structured-init-proof.ts index 869bc4535776..b0a357295010 100644 --- a/src/main/claude/claude-structured-init-proof.ts +++ b/src/main/claude/claude-structured-init-proof.ts @@ -105,3 +105,58 @@ export function claudeAuthDiagnostic( settingSources: CLAUDE_DEFAULT_SETTING_SOURCES } } + +/** The CLI's own frame naming the session it runs (system/init or a SessionStart hook). Only a + * SessionStart hook sends one before the first turn, so startup takes it when it came and never + * waits for it. */ +export type ClaudeInitProof = { + promise: Promise + resolve: (init: ClaudeInitObservation) => void + reject: (error: Error) => void + /** A frame named another provider session. */ + refuse: () => void + /** The proof seen so far, or null; throws when it was refused or the child failed first. */ + seen: () => ClaudeInitObservation | null + /** Set once startup has read the proof: a later refusal ends the session. */ + onRefusal: ((error: Error) => void) | null +} + +export function createClaudeInitProof(): ClaudeInitProof { + let resolvePromise = (_init: ClaudeInitObservation): void => {} + let rejectPromise = (_error: Error): void => {} + const promise = new Promise((resolve, reject) => { + resolvePromise = resolve + rejectPromise = reject + }) + void promise.catch(() => {}) + let outcome: { init: ClaudeInitObservation } | { error: Error } | null = null + const reject = (error: Error): void => { + outcome ??= { error } + rejectPromise(error) + } + const proof: ClaudeInitProof = { + promise, + resolve: (init) => { + outcome ??= { init } + resolvePromise(init) + }, + reject, + refuse: () => { + // A session already proven by its own frame keeps that proof. + if (outcome && 'init' in outcome) { + return + } + const error = new Error('claude provider session expected') + reject(error) + proof.onRefusal?.(error) + }, + seen: () => { + if (outcome && 'error' in outcome) { + throw outcome.error + } + return outcome?.init ?? null + }, + onRefusal: null + } + return proof +} diff --git a/src/main/claude/claude-structured-session-acquisition.ts b/src/main/claude/claude-structured-session-acquisition.ts index 98defecd713f..206bf525b66d 100644 --- a/src/main/claude/claude-structured-session-acquisition.ts +++ b/src/main/claude/claude-structured-session-acquisition.ts @@ -8,6 +8,7 @@ import { resolveClaudeReplayTurn } from './claude-replay-turn-resolution' import { claudeSessionStateEndsTurn } from './claude-session-state-turn-over' import { settleClaudeTurnEndWaiters } from './claude-request-end-wait' import { + createClaudeInitProof, readClaudeCapabilities, readClaudeFrameString, readClaudeInit @@ -19,7 +20,6 @@ import { adoptClaudeStructuredSpawnOptions } from './claude-structured-spawn-opt import { createClaudeSessionJournalTranslator } from './claude-structured-journal-translation' import { observeClaudeFastModeFacts } from './claude-structured-session-options' import { - createClaudeInitProof, readClaudeStartupFacts, settleClaudeSessionStartup } from './claude-structured-session-startup' @@ -281,6 +281,7 @@ export async function acquireClaudeSession({ isCurrent: () => sessions.get(sessionId) === session, fault: (error) => callbacks.handleExit(sessionId, attempt, error), diagnose: (diagnostic) => emit({ type: 'auth-diagnostic', sessionId, diagnostic }), + optionRevision: () => input.optionRevision?.() ?? 0, report: (event) => emit({ ...event, diff --git a/src/main/claude/claude-structured-session-journal-control.ts b/src/main/claude/claude-structured-session-journal-control.ts index 9e3ee0289cf4..796e75c35f95 100644 --- a/src/main/claude/claude-structured-session-journal-control.ts +++ b/src/main/claude/claude-structured-session-journal-control.ts @@ -5,7 +5,7 @@ import { import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import type { ClaudeStreamJsonConnection } from './claude-stream-json-connection' import type { ClaudeJournalTranslator } from './claude-journal-translator-contract' -import type { ClaudeInitProof } from './claude-structured-session-startup' +import type { ClaudeInitProof } from './claude-structured-init-proof' import type { ClaudeAcquisitionAttempt, ClaudeAcquireCallbacks diff --git a/src/main/claude/claude-structured-session-startup.test.ts b/src/main/claude/claude-structured-session-startup.test.ts index da81c01a33cd..0c01bcd51f0f 100644 --- a/src/main/claude/claude-structured-session-startup.test.ts +++ b/src/main/claude/claude-structured-session-startup.test.ts @@ -119,7 +119,9 @@ describe('Claude structured session publishes before the CLI answers initialize' acquisitionGeneration: expect.any(String), // What the child was launched with, carried so the host never asks the CLI again. reportedOptions: expect.objectContaining({ model: 'opus' }), - restoreSkippedOptions: [] + restoreSkippedOptions: [], + // No host revision handed in: nothing has moved it. + optionRevision: 0 }) expect(claude.connections[0].calls.map(({ subtype }) => subtype)).toEqual([ 'initialize', diff --git a/src/main/claude/claude-structured-session-startup.ts b/src/main/claude/claude-structured-session-startup.ts index 5a50ddf220dc..1930f254faf4 100644 --- a/src/main/claude/claude-structured-session-startup.ts +++ b/src/main/claude/claude-structured-session-startup.ts @@ -15,7 +15,8 @@ import { claudeInitializationAuthError, readClaudeCapabilities, readClaudeModels, - type ClaudeInitObservation + type ClaudeInitObservation, + type ClaudeInitProof } from './claude-structured-init-proof' import { claudeStructuredSessionPublicationOptions, @@ -42,64 +43,9 @@ import { applyClaudeStartFastMode } from './claude-structured-start-fast-mode' -/** The CLI's own frame naming the session it runs (system/init or a SessionStart hook). Only a - * SessionStart hook sends one before the first turn, so startup takes it when it came and never - * waits for it. */ -export type ClaudeInitProof = { - promise: Promise - resolve: (init: ClaudeInitObservation) => void - reject: (error: Error) => void - /** A frame named another provider session. */ - refuse: () => void - /** The proof seen so far, or null; throws when it was refused or the child failed first. */ - seen: () => ClaudeInitObservation | null - /** Set once startup has read the proof: a later refusal ends the session. */ - onRefusal: ((error: Error) => void) | null -} - -export function createClaudeInitProof(): ClaudeInitProof { - let resolvePromise = (_init: ClaudeInitObservation): void => {} - let rejectPromise = (_error: Error): void => {} - const promise = new Promise((resolve, reject) => { - resolvePromise = resolve - rejectPromise = reject - }) - void promise.catch(() => {}) - let outcome: { init: ClaudeInitObservation } | { error: Error } | null = null - const reject = (error: Error): void => { - outcome ??= { error } - rejectPromise(error) - } - const proof: ClaudeInitProof = { - promise, - resolve: (init) => { - outcome ??= { init } - resolvePromise(init) - }, - reject, - refuse: () => { - // A session already proven by its own frame keeps that proof. - if (outcome && 'init' in outcome) { - return - } - const error = new Error('claude provider session expected') - reject(error) - proof.onRefusal?.(error) - }, - seen: () => { - if (outcome && 'error' in outcome) { - throw outcome.error - } - return outcome?.init ?? null - }, - onRefusal: null - } - return proof -} - export type StructuredAgentSessionStartedOptions = Pick< StructuredAgentSessionStartedEvent, - 'reportedOptions' | 'restoreSkippedOptions' | 'retiredOptions' + 'reportedOptions' | 'restoreSkippedOptions' | 'retiredOptions' | 'optionRevision' > /** A lifecycle event the start reports, before the session's identity is stamped on it. */ @@ -248,6 +194,8 @@ export async function settleClaudeSessionStartup(input: { isCurrent: () => boolean fault: (error: Error) => void diagnose: (diagnostic: ClaudeAuthDiagnostic) => void + /** The host's option revision now, stamped on each report as its read begins. */ + optionRevision: () => number /** `started` once startup has proven, with what the child now reports, snapshotted from memory; * then what the settings add, and any saved option the child showed it cannot run. */ report: (event: ClaudeStartupReport) => void @@ -285,7 +233,8 @@ export async function settleClaudeSessionStartup(input: { session.fastModeAtStart ? session.optionMutationSequence - 1 : undefined ), restoreSkippedOptions: [...session.restoreSkippedOptions], - ...claudeRetiredOptions(session) + ...claudeRetiredOptions(session), + optionRevision: input.optionRevision() }) if (session.startup.state === 'pending') { session.startup.state = 'proven' @@ -309,6 +258,7 @@ async function settleClaudeStartupSettings( ): Promise { const { session } = input const sequence = session.optionMutationSequence + const optionRevision = input.optionRevision() const settings = await input.readSettings() if (!input.isCurrent()) { return @@ -337,7 +287,8 @@ async function settleClaudeStartupSettings( type: 'options-reported', reportedOptions: claudeStartedReportedOptions(session, readClaudeModels(facts.initialization)), restoreSkippedOptions: [...session.restoreSkippedOptions], - ...claudeRetiredOptions(session) + ...claudeRetiredOptions(session), + optionRevision }) if (startFastMode !== null) { void applyClaudeStartFastMode(session, facts, startFastMode, (event) => { 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 24526b1ee9ca..29a033a4e7b9 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 @@ -828,10 +828,7 @@ describe('a close that stops the child and then a wind-down step fails', () => { evict: () => host.close(SESSION, 'evict') } satisfies Partial Promise>> - it.each([ - { end: 'evict', starting: true, kind: 'chatClosed', verdict: null }, - { end: 'evict', starting: false, kind: 'chatClosed', verdict: null } - ] as const)( + it.each([{ end: 'evict', starting: true, kind: 'chatClosed', verdict: null }] as const)( 'rejects what is queued as $kind after a $end (during startup: $starting)', async ({ end, starting, kind, verdict }) => { const started = deferred() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts index 1614785712a0..6f05dbf01674 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts @@ -1,4 +1,5 @@ import { isDeepStrictEqual } from 'node:util' +import { waitForPromiseWithSignal } from '../../../shared/abort-signal-reason' import type { AgentSessionProcessIdentity, AgentSessionRecord @@ -55,7 +56,8 @@ export async function acquireOwner( // Retries must recover the original reservation, not mint a second child. spawnToken, ...(input.eventSink ? { events: input.eventSink } : {}), - ...(input.acquireSignal ? { signal: input.acquireSignal } : {}) + ...(input.acquireSignal ? { signal: input.acquireSignal } : {}), + ...(input.optionRevision ? { optionRevision: input.optionRevision } : {}) }) const progress = input.onStartupAttempt?.(attempt) const acquired = await input.adapter.acquire({ @@ -78,20 +80,22 @@ export async function acquireOwner( const options = providerChildPhase === 'starting' ? undefined - : await withAgentSessionCreatePhase('restore_options', input.recordPhase, async () => - input.adapter.readAcquisitionOptions - ? input.adapter.readAcquisitionOptions({ - sessionId: record.sessionId, - fence, - ...(record.options ? { priorOptions: record.options } : {}) - }) - : readNativeSessionOptions({ - adapter: input.adapter, - sessionId: record.sessionId, - fence, - ...(record.options ? { priorOptions: record.options } : {}) - }) - ) + : await withAgentSessionCreatePhase('restore_options', input.recordPhase, async () => { + const read = { + sessionId: record.sessionId, + fence, + ...(record.options ? { priorOptions: record.options } : {}) + } + // Still inside the start, so the limit or a Stop ends a read the provider never answers. + return waitForPromiseWithSignal( + Promise.resolve( + input.adapter.readAcquisitionOptions + ? input.adapter.readAcquisitionOptions(read) + : readNativeSessionOptions({ adapter: input.adapter, ...read }) + ), + input.acquireSignal + ) + }) if (record.lease.ownerProcess === null) { await input.store.commitProcessIdentity({ sessionId: record.sessionId, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts index 2374be60fd36..27b3bfa41e41 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts @@ -229,6 +229,9 @@ export type StructuredAgentSessionStartedEvent = { restoreSkippedOptions: readonly string[] /** Values the child showed it cannot run: a report naming the same value is not persisted. */ retiredOptions?: Readonly> + /** The attempt's `optionRevision()` as the read behind this report began: a pick or report the + * host took since makes it out of date. */ + optionRevision: number } /** A running child showed saved options it cannot run, as a model the provider reports missing. diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts index 4a69f4a37c89..e042afb98098 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts @@ -70,6 +70,8 @@ export type AttachFlowInput = { onStartupAttempt?: ( attempt: StructuredAgentSessionStartupAttempt ) => StructuredAgentSessionStartupProgress + /** The conversation's option revision, which the child's reports are stamped with. */ + optionRevision?: () => number /** Publishes the journal before clients can send against the new owner. `acquiredOwner` is * true only when this attach spawned the provider child, so a re-attach to a live one is not * mistaken for a cold acquire. */ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts index e6d3a55e5fad..c5722ad1483e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts @@ -205,6 +205,7 @@ async function runAttachUnderAbort( attempt.startup = startup return startupAttempts.track(sessionId, startup) }, + optionRevision: () => context.runtimeState.optionRevisions.current(sessionId), ...(options.onAcquisitionFailed ? { onAcquisitionFailed: options.onAcquisitionFailed } : {}), openConversation: async (record) => { const conversation = await context.openConversation(record.sessionId, { 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 772f57879d20..431ca9e761de 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 @@ -59,7 +59,7 @@ export function createStructuredAgentSessionConversationLifetime(host: { if (await releaseLeaseOfEndedStructuredAgentSessionChild(host.context(), sessionId)) { return false } - return closeStructuredAgentSessionConversationUnderSerialize( + const closed = await closeStructuredAgentSessionConversationUnderSerialize( { sessions, closeStatus: (id) => { @@ -71,6 +71,10 @@ export function createStructuredAgentSessionConversationLifetime(host: { sessionId, atRest ) + if (closed) { + host.context().runtimeState.optionRevisions.forget(sessionId) + } + return closed } const idleSweep = new StructuredAgentSessionIdleSweep({ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts index 5a0537e5a5ac..cb9cd10fea68 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts @@ -199,6 +199,8 @@ export async function setStructuredAgentSessionOption( return await performSetOption(ctx, params, wait.signal) } finally { wait.end() + // Whatever the write's outcome, the child may hold what it asked for now. + context.optionRevisions.advance(params.envelope.sessionId) } } }, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts index 3757a2f5345b..d604b805eced 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts @@ -12,6 +12,7 @@ import { heldProviderChildReader, type ProviderChildSessions } from './structured-agent-session-provider-child' +import { StructuredAgentSessionOptionRevisions } from './structured-agent-session-option-revisions' import { resolveStructuredSessionRecovery } from './structured-agent-session-recovery-resolution' import { recordAgentSessionStartup } from '../../observability/agent-session-instrumentation' import { @@ -24,6 +25,7 @@ export class StructuredAgentSessionHostRuntimeState { private readonly eventSinks = new Map() readonly acquireAborts = new StructuredAgentSessionAcquireAborts() readonly startupAttempts: StructuredAgentSessionStartupAttempts + readonly optionRevisions = new StructuredAgentSessionOptionRevisions() private readonly leaseRenewer: StructuredAgentSessionLeaseRenewer private readonly onEventSinkFailure?: (sessionId: string, error: unknown) => void 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 3e8f87a3a9fc..2876aaed39d7 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 @@ -294,6 +294,7 @@ export class StructuredAgentSessionHost { stopAgent: (sessionId, ending) => this.lifetime.stopAgent(sessionId, ending), wakeQueuedDrain: (sessionId) => this.queued.drain.schedule(sessionId), acquireAborts: this.runtimeState.acquireAborts, + optionRevisions: this.runtimeState.optionRevisions, now: () => this.now() } } 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 e704dbba5707..1f543cbf19f0 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 @@ -14,6 +14,7 @@ import { import type { MutationPlan } from './structured-agent-session-mutation-plans' import type { StructuredAgentSessionStopEnding } from './structured-agent-session-host-lifetime' import type { StructuredAgentSessionAcquireAborts } from './structured-agent-session-acquire-aborts' +import type { StructuredAgentSessionOptionRevisions } from './structured-agent-session-option-revisions' import type { StructuredAgentSessionCaller, StructuredAgentSessionHostDeps, @@ -48,6 +49,8 @@ export type StructuredAgentSessionMutationContext = { /** The provider wait each session's serialize is on (a start, an option write), which a caller * outside that serialize aborts. */ acquireAborts: Pick + /** Moved by every pick a running child takes, so a report it read before is never persisted. */ + optionRevisions: Pick now: () => number } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-option-revisions.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-option-revisions.ts new file mode 100644 index 000000000000..dc4fa7bbe39f --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-option-revisions.ts @@ -0,0 +1,31 @@ +// The order of what a conversation's record learns about its child's options. +// +// A pick and a child's report both write the record's options, but a report describes the child as +// of its read, which can predate a pick still in flight, a saved value the child showed it cannot +// run, or a newer report. Each of those moves the revision. A report carries the revision it was +// read at and is persisted only if nothing moved it since, so no read undoes what followed it. + +export class StructuredAgentSessionOptionRevisions { + private readonly revisions = new Map() + + current(sessionId: string): number { + return this.revisions.get(sessionId) ?? 0 + } + + /** What any report read before now says is out of date. */ + advance(sessionId: string): number { + const next = this.current(sessionId) + 1 + this.revisions.set(sessionId, next) + return next + } + + /** A report read at `readAt` becomes the newest word, or null when a pick or report came since. */ + admitReport(sessionId: string, readAt: number): number | null { + return readAt === this.current(sessionId) ? this.advance(sessionId) : null + } + + /** The conversation closed with no child: nothing it reported is still to be persisted. */ + forget(sessionId: string): void { + this.revisions.delete(sessionId) + } +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts index 8ab957f0e900..c7a9997117e3 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-child-record.test.ts @@ -223,7 +223,8 @@ function prove(): Promise { ...child, acquisitionGeneration, reportedOptions: { model: 'default' }, - restoreSkippedOptions: [] + restoreSkippedOptions: [], + optionRevision: host.collaboratorsForTests().runtimeState.optionRevisions.current(SESSION) }) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts index 92f474c28966..1663d258f787 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts @@ -5,12 +5,12 @@ // This is where the host learns the start landed: the child turns `ready`, its startup attempt // ends, and the loop wakes to hand over what was queued meanwhile. Only once that handover is done // is what the child reports persisted, in a step of its own, so bookkeeping never sits between a -// ready child and the user's first message; a failed write is reported, never thrown. +// ready child and the user's first message; a failed write is reported, never thrown. Every report +// is persisted only if no pick or later report came after its read (`option-revisions`). // // These run under the session's own serialized steps, which its close and sends wait on, so they // ask the provider nothing: the event carries what the child proved. -import { isDeepStrictEqual } from 'node:util' import { agentSessionLeaseAdmitsWriter } from '../../../shared/agent-session-lease-adjudication' import type { StructuredAgentSessionOptionsReportedEvent, @@ -26,6 +26,7 @@ import { markProviderChildStarted, sameProviderChild } from './structured-agent-session-provider-child' +import type { StructuredAgentSessionOptionRevisions } from './structured-agent-session-option-revisions' import type { StructuredAgentSessionStartupAttempts } from './structured-agent-session-startup-attempt' export type StructuredAgentSessionProviderStartedContext = { @@ -34,22 +35,26 @@ export type StructuredAgentSessionProviderStartedContext = { serialize: (sessionId: string, task: () => Promise) => Promise now: () => number publishStatus?: (sessionId: string) => void - runtimeState: { startupAttempts: Pick } + runtimeState: { + startupAttempts: Pick + optionRevisions: Pick< + StructuredAgentSessionOptionRevisions, + 'admitReport' | 'advance' | 'current' + > + } /** The barrier lifts: what was accepted while the child started is handed over now. Settles once * the loop has handed over all it can. */ wakeDelivery: (sessionId: string) => Promise } -type ReportedOptions = Pick< - StructuredAgentSessionOptionsReportedEvent, - 'sessionId' | 'fence' | 'acquisitionGeneration' | 'reportedOptions' | 'restoreSkippedOptions' -> & - Pick +type ReportedOptions = Omit export async function settleStructuredAgentSessionProviderStarted( context: StructuredAgentSessionProviderStartedContext, event: StructuredAgentSessionStartedEvent ): Promise { + // On arrival, before any wait: reports are admitted in the order the child made them. + const admitted = admitReportedOptions(context, event) // Serialized behind the attach that published this child, so the lease it proved is committed. const started = await context.serialize(event.sessionId, async () => { const session = context.sessions.get(event.sessionId) @@ -61,15 +66,13 @@ export async function settleStructuredAgentSessionProviderStarted( context.runtimeState.startupAttempts.ready(event.sessionId, child) const delivered = context.wakeDelivery(event.sessionId) context.publishStatus?.(event.sessionId) - return { delivered, optionsAtStart: context.deps.store.getRecord(event.sessionId)?.options } + return { delivered } }) if (!started) { return } // Not awaited: the adapter's next event may be what the handover itself waits on. - void started.delivered.then(() => - persistReportedOptions(context, event, { optionsAtStart: started.optionsAtStart }) - ) + void started.delivered.then(() => persistReportedOptions(context, event, admitted)) } /** What a ready child reports later, such as a read that came after its start: persisted as the @@ -78,16 +81,26 @@ export function settleStructuredAgentSessionOptionsReported( context: StructuredAgentSessionProviderStartedContext, event: StructuredAgentSessionOptionsReportedEvent ): Promise { - return persistReportedOptions(context, event, null) + return persistReportedOptions(context, event, admitReportedOptions(context, event)) +} + +function admitReportedOptions( + context: StructuredAgentSessionProviderStartedContext, + event: ReportedOptions +): number | null { + return context.runtimeState.optionRevisions.admitReport(event.sessionId, event.optionRevision) } -/** Non-fatal, and only for the child that reported, with the record as it stood when it did: a pick - * made since is the user's and newer than the report. */ +/** Non-fatal, and only for the child that reported, while its report is still the newest word: a + * pick or a later report since it was admitted is newer than what it read. */ function persistReportedOptions( context: StructuredAgentSessionProviderStartedContext, event: ReportedOptions, - snapshot: { optionsAtStart: Readonly> | undefined } | null + admitted: number | null ): Promise { + if (admitted === null) { + return Promise.resolve() + } return context .serialize(event.sessionId, async () => { const child = context.sessions.get(event.sessionId)?.child @@ -101,7 +114,7 @@ function persistReportedOptions( !record || record.lease.runtimeFence !== event.fence || !agentSessionLeaseAdmitsWriter(record.lease) || - (snapshot && !isDeepStrictEqual(record.options, snapshot.optionsAtStart)) + context.runtimeState.optionRevisions.current(event.sessionId) !== admitted ) { return } @@ -136,6 +149,8 @@ export function settleStructuredAgentSessionOptionsSkipped( context: StructuredAgentSessionProviderStartedContext, event: StructuredAgentSessionOptionsSkippedEvent ): Promise { + // A report read before the child showed this is out of date: it would write the value back. + context.runtimeState.optionRevisions.advance(event.sessionId) return context.serialize(event.sessionId, async () => { const { store } = context.deps const record = store.getRecord(event.sessionId) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts index 69a762908e12..90f2a96acba3 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-queued-message-rig.test-fixture.ts @@ -263,7 +263,8 @@ export async function createQueuedMessageTestRig( fence: store.getRecord(SESSION)!.lease.runtimeFence, acquisitionGeneration: 'generation-1', reportedOptions: { model: 'default' }, - restoreSkippedOptions: [] + restoreSkippedOptions: [], + optionRevision: host.collaboratorsForTests().runtimeState.optionRevisions.current(SESSION) }) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts index 62bc4e65519f..fb2f9beae728 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-send-restarts-failed-start.test.ts @@ -104,7 +104,8 @@ function proveStarted(): Promise { type: 'started', ...currentChild(), reportedOptions: { model: 'sonnet' }, - restoreSkippedOptions: [] + restoreSkippedOptions: [], + optionRevision: host.collaboratorsForTests().runtimeState.optionRevisions.current(SESSION) }) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts index 95517a463d78..da4f40dabcbf 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts @@ -38,4 +38,6 @@ export type StructuredAgentSessionStartupAttempt = { readonly signal?: AbortSignal /** Any stdout frame or stderr line from the child: proof the start is still moving. */ readonly onOutput?: () => void + /** The conversation's option revision now; a report stamps it as its read begins. */ + readonly optionRevision?: () => number } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts index 9de2bb8ec53e..d52e831d3f6c 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts @@ -45,6 +45,8 @@ let host: StructuredAgentSessionHost let acquire: Mock let dispatch: Mock let closeSession: Mock> +let setOption: Mock +let readAcquisitionOptions: StructuredAgentSessionAdapter['readAcquisitionOptions'] let startupLimits: Partial | undefined function acquisition(input: StructuredAgentSessionAcquireInput): AgentSessionAcquisition { @@ -84,7 +86,8 @@ async function startHost(): Promise { releaseAcquisition: vi.fn(async () => true), cancelTurn: vi.fn(async () => ({ cancelled: false })), answerPrompt: vi.fn(async () => undefined), - setOption: vi.fn(async () => undefined) + setOption, + ...(readAcquisitionOptions ? { readAcquisitionOptions } : {}) }, journalDatabase: openTestJournalHostDatabase(root), claimKeyId: 'key-1', @@ -119,6 +122,8 @@ beforeEach(async () => { } })) closeSession = vi.fn(async () => true) + setOption = vi.fn(async () => undefined) + readAcquisitionOptions = undefined store = await openTestAgentSessionRecordStore(root) await startHost() expect(await host.attach(CALLER, hostTestAttachParams(null))).toMatchObject({ ok: true }) @@ -161,6 +166,12 @@ async function submission(id: string): Promise { const id = await accept(text) @@ -350,6 +379,42 @@ describe('a start that ends before it proves itself', () => { await eventually(() => expect(startupAttemptOpen()).toBe(false)) }) + it('ends a ready child’s option read that never answers, and fails what it held', async () => { + startupLimits = { silenceMs: 50 } + readAcquisitionOptions = () => new Promise(() => {}) + await restartWith(async (input) => { + await input.onSpawned?.(acquisition(input).process) + return acquisition(input) + }) + const id = await accept('hello') + + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) + expect(await submission(id)).toMatchObject(HOST_STOPPED) + expect(dispatch).not.toHaveBeenCalled() + await eventually(() => expect(startupAttemptOpen()).toBe(false)) + }) + + it('ends a ready child inside its start when the chat closes before its options are read', async () => { + let land = (): void => {} + const landed = new Promise((resolve) => { + land = resolve + }) + await restartWith(async (input) => { + await landed + return acquisition(input) + }) + const id = await accept('hello') + await eventually(() => expect(acquire).toHaveBeenCalled()) + + const closing = host.close(SESSION, 'evict') + land() + await expect(closing).resolves.toBeUndefined() + + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('rejected')) + expect((await submission(id))?.rejection).toMatchObject({ kind: 'chatClosed' }) + expect(dispatch).not.toHaveBeenCalled() + }) + it('is not ended by the limit once it proved itself', async () => { startupLimits = { silenceMs: 50 } await restartWith(publishFirst) @@ -397,24 +462,80 @@ describe('what a started child reports', () => { it('never overwrites a pick made after the child reported', async () => { await restartWith(publishFirst) await heldBehindStart('hello') + const report = { ...startedEvent(), reportedOptions: { model: 'reported' } } dispatch.mockImplementationOnce(async () => { // A pick lands while the first handover is still running. - await store.replaceSessionOptions({ - sessionId: SESSION, - fence: store.getRecord(SESSION)!.lease.runtimeFence, - options: { model: 'picked' }, - now: NOW - }) + void pick('picked') return { state: 'admitted' as const } }) - await host.handleAdapterEvent({ ...startedEvent(), reportedOptions: { model: 'reported' } }) - await eventually(() => expect(dispatch).toHaveBeenCalled()) + await host.handleAdapterEvent(report) + await eventually(() => expect(store.getRecord(SESSION)?.options).toEqual({ model: 'picked' })) await new Promise((resolve) => setTimeout(resolve, 50)) await host.flushStreamedEvents(SESSION) expect(store.getRecord(SESSION)?.options).toEqual({ model: 'picked' }) }) + + it('never lets a settings read made before a pick undo it, even queued behind the pick', async () => { + await restartWith(publishFirst) + const id = await heldBehindStart('hello') + await host.handleAdapterEvent(startedEvent()) + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('accepted')) + await eventually(() => expect(store.getRecord(SESSION)?.options).toEqual({ model: 'default' })) + // The settings read starts, then the user picks while it is out. + const readAt = optionRevisionNow() + let landPick = (): void => {} + setOption.mockImplementationOnce( + () => + new Promise((resolve) => { + landPick = () => resolve(undefined) + }) + ) + const picked = pick('picked') + await eventually(() => expect(setOption).toHaveBeenCalled()) + + // The read answers with what the child ran before the pick, while the pick is still in flight. + const reported = host.handleAdapterEvent({ + ...startedEvent(), + type: 'options-reported', + reportedOptions: { model: 'default' }, + optionRevision: readAt + }) + landPick() + expect(await picked).toMatchObject({ ok: true }) + await reported + await host.flushStreamedEvents(SESSION) + + expect(store.getRecord(SESSION)?.options).toEqual({ model: 'picked' }) + }) + + it('never lands the start’s report after a newer settings report', async () => { + await restartWith(publishFirst) + await heldBehindStart('hello') + let landHandover = (): void => {} + dispatch.mockImplementationOnce( + () => + new Promise((resolve) => { + landHandover = () => resolve({ state: 'admitted' as const }) + }) + ) + // The start's report waits on the handover; the settings report that follows it does not. + await host.handleAdapterEvent({ ...startedEvent(), reportedOptions: { model: 'at-start' } }) + await eventually(() => expect(dispatch).toHaveBeenCalled()) + const reported = host.handleAdapterEvent({ + ...startedEvent(), + type: 'options-reported', + reportedOptions: { model: 'from-settings' } + }) + + landHandover() + await reported + await new Promise((resolve) => setTimeout(resolve, 50)) + await host.flushStreamedEvents(SESSION) + + expect(store.getRecord(SESSION)?.options).toEqual({ model: 'from-settings' }) + }) }) function startupAttemptOpen(): boolean { @@ -435,5 +556,6 @@ describe('the attempt an acquire runs under', () => { expect(input.onOutput).toEqual(expect.any(Function)) expect(input.attemptId).toEqual(expect.any(String)) expect(input.signal).toBeInstanceOf(AbortSignal) + expect(input.optionRevision).toEqual(expect.any(Function)) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts index eed7303b6667..64c681c9f9ed 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-registrations.test.ts @@ -172,7 +172,8 @@ describe.each( fence: record.lease.runtimeFence, acquisitionGeneration: 'generation-1', reportedOptions: { model: 'default' }, - restoreSkippedOptions: [] + restoreSkippedOptions: [], + optionRevision: host.collaboratorsForTests().runtimeState.optionRevisions.current(SESSION) }) await vi.waitFor(() => expect(dispatch).toHaveBeenCalledOnce()) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts index 52e38e13a000..841b35825589 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts @@ -79,9 +79,23 @@ describe('the startup clock', () => { vi.advanceTimersByTime(1) expect(expired).toEqual([{ sessionId: SESSION, attemptId: attempt.attemptId, child: null }]) - expect(settled).toEqual([{ agent: 'codex', outcome: 'silent', durationMs: SILENCE_MS }]) vi.advanceTimersByTime(CEILING_MS) expect(expired).toHaveLength(1) + // Measured at the limit, recorded once the aborted acquire settles. + expect(settled).toEqual([]) + attempts.abandon(SESSION, attempt.attemptId) + expect(settled).toEqual([{ agent: 'codex', outcome: 'silent', durationMs: SILENCE_MS }]) + }) + + it('records a start that turned ready as the limit fired as ready', () => { + const attempt = mint() + attempts.track(SESSION, attempt).spawned() + vi.advanceTimersByTime(SILENCE_MS) + expect(expired).toHaveLength(1) + + attempts.published(SESSION, attempt.attemptId, { ...CHILD, phase: 'ready' }) + + expect(settled).toEqual([{ agent: 'codex', outcome: 'ready', durationMs: SILENCE_MS }]) }) it('keeps a slow start that is still talking past the silence limit', () => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts index 2af55be05982..369a7defa26d 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts @@ -28,6 +28,7 @@ export function mintStructuredAgentSessionStartupAttempt(input: { spawnToken: string events?: StructuredAgentSessionEventSink signal?: AbortSignal + optionRevision?: () => number }): StructuredAgentSessionStartupAttempt { const { record } = input return { @@ -42,7 +43,8 @@ export function mintStructuredAgentSessionStartupAttempt(input: { }, ...(record.options ? { options: record.options } : {}), ...(input.events ? { events: input.events } : {}), - ...(input.signal ? { signal: input.signal } : {}) + ...(input.signal ? { signal: input.signal } : {}), + ...(input.optionRevision ? { optionRevision: input.optionRevision } : {}) } } @@ -89,6 +91,9 @@ type Tracked = { lastOutputAt: number timer: ReturnType | null expired: boolean + /** The limit's measure, held until an acquire it aborted settles: one that is ready anyway + * reports that instead. */ + expiry: StructuredAgentSessionStartupSettled | null } /** Each session's open attempt and its startup clock. A session starts one child at a time, so a @@ -125,7 +130,8 @@ export class StructuredAgentSessionStartupAttempts { spawnedAt: null, lastOutputAt: 0, timer: null, - expired: false + expired: false, + expiry: null } this.open.set(sessionId, tracked) const current = (): boolean => this.open.get(sessionId) === tracked && !this.disposed @@ -258,35 +264,45 @@ export class StructuredAgentSessionStartupAttempts { return } if (reason) { - this.report(tracked, reason) + tracked.expiry = this.measure(tracked, reason) } tracked.expired = true const { child } = tracked // A published child's stop is under way and ends nothing a newer attempt owns; one still // acquiring stays tracked until its aborted acquire is abandoned or published. if (child) { + this.report(tracked.expiry) this.end(sessionId) } this.deps.expire({ sessionId, attemptId: tracked.attempt.attemptId, child }) } private settle(sessionId: string, tracked: Tracked, outcome: 'ready' | 'ended'): void { - if (!tracked.expired) { - this.report(tracked, outcome) - } + this.report( + tracked.expired && outcome === 'ended' ? tracked.expiry : this.measure(tracked, outcome) + ) this.end(sessionId) } - private report(tracked: Tracked, outcome: StructuredAgentSessionStartupSettled['outcome']): void { - if (tracked.spawnedAt === null || this.disposed) { + private measure( + tracked: Tracked, + outcome: StructuredAgentSessionStartupSettled['outcome'] + ): StructuredAgentSessionStartupSettled | null { + return tracked.spawnedAt === null + ? null + : { + agent: tracked.attempt.identity.agent, + outcome, + durationMs: Math.max(0, this.now() - tracked.spawnedAt) + } + } + + private report(settled: StructuredAgentSessionStartupSettled | null): void { + if (!settled || this.disposed) { return } try { - this.deps.settled?.({ - agent: tracked.attempt.identity.agent, - outcome, - durationMs: Math.max(0, this.now() - tracked.spawnedAt) - }) + this.deps.settled?.(settled) } catch { // Measurement is bookkeeping: it never decides a start. } diff --git a/src/main/runtime/claude-structured-retired-saved-model.test.ts b/src/main/runtime/claude-structured-retired-saved-model.test.ts index 138a240984dc..d4743996f63a 100644 --- a/src/main/runtime/claude-structured-retired-saved-model.test.ts +++ b/src/main/runtime/claude-structured-retired-saved-model.test.ts @@ -109,20 +109,21 @@ describe("a Claude chat whose saved model the CLI says doesn't exist", () => { expect(record()?.options?.model).toBe('opus') }) - // A slow settings readback lets the turn's error land first; `started` then reports the CLI's - // own model, which is still the retired one, and must not write it back. - it('does not let a start that lands after the heal write the retired model back', async () => { + // A slow settings readback lets the turn's error land first; the report that follows it still + // names the CLI's own model, which is the retired one, and must not write it back. + it('does not let a settings report that lands after the heal write the retired model back', async () => { claude.behave(SESSION, { sendsNoStartFrame: true, startupSettingsReadHangs: true, controlTimeoutMs: 300 }) const host = await claude.install() + const reports = vi.spyOn(host, 'handleAdapterEvent') await expect( host.attach(CALLER, claude.attachParams(SESSION, null, { options: { model: RETIRED } })) ).resolves.toMatchObject({ ok: true }) await vi.waitFor(() => - expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('starting') + expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('ready') ) claude.child(SESSION).handlers.onMessage?.({ type: 'system', @@ -131,11 +132,16 @@ describe("a Claude chat whose saved model the CLI says doesn't exist", () => { model: RETIRED }) claude.child(SESSION).handlers.onMessage?.(modelNotFoundReply('reply-1')) + await vi.waitFor(() => + expect(host.deps.store.getRecord(SESSION)?.options).not.toHaveProperty('model') + ) + // The settings read gives up at its deadline and reports what the child showed. await vi.waitFor( - () => expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('ready'), + () => expect(reports.mock.calls.map(([event]) => event.type)).toContain('options-reported'), { timeout: 5_000 } ) + await host.flushStreamedEvents(SESSION) expect(host.deps.store.getRecord(SESSION)?.options).not.toHaveProperty('model') }) }) diff --git a/src/main/runtime/structured-agent-session-failure-logging.test.ts b/src/main/runtime/structured-agent-session-failure-logging.test.ts index 9825a2b7aee6..e343a45ab211 100644 --- a/src/main/runtime/structured-agent-session-failure-logging.test.ts +++ b/src/main/runtime/structured-agent-session-failure-logging.test.ts @@ -111,7 +111,8 @@ describe('failures the desktop host used to drop', () => { fence: 1, acquisitionGeneration: 'generation-1', reportedOptions: { model: 'gpt-5' }, - restoreSkippedOptions: [] + restoreSkippedOptions: [], + optionRevision: 0 }) lifecycle.deliver({ type: 'ended', From 12eb0f4c3a1fc1bcb528c5bac244a2d73f93a005 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 8 Oct 2026 02:11:19 -0700 Subject: [PATCH 5/5] fix(native-chat): require the option revision and order reports by arrival The host can no longer mint a startup attempt without the conversation's option revision, so no host path can stamp a report at a stale zero and drop every report after the first pick. Adapter tests may still omit it. A report is now dropped only when a pick or a value the child showed it cannot run came after its read. Reports are ordered among themselves by when the host takes them, and only the newest admitted one is persisted. The settings read's stamp therefore holds however late the host takes the start's report, instead of relying on that report being handled synchronously before the read begins. Tests: a settings read stamped before the host took the start's report is kept; a report read after a failed pick is persisted; the minted attempt's revision reader answers the host's current revision. --- .../claude-structured-session-acquisition.ts | 1 + .../claude-structured-session-startup.ts | 1 + ...-agent-session-acquisition-options.test.ts | 12 ++++++ ...-session-acquisition-runtime-stamp.test.ts | 1 + .../structured-agent-session-acquisition.ts | 2 +- .../structured-agent-session-adapter.ts | 4 +- ...tured-agent-session-adopted-import.test.ts | 1 + .../structured-agent-session-attach-flow.ts | 2 +- ...ructured-agent-session-drivability.test.ts | 1 + ...ructured-agent-session-option-revisions.ts | 23 ++++++++-- ...ent-session-pre-spawn-first-answer.test.ts | 1 + ...nt-session-processless-reservation.test.ts | 5 +++ ...ructured-agent-session-provider-started.ts | 8 +++- ...-agent-session-startup-attempt-contract.ts | 2 +- ...agent-session-startup-attempt-host.test.ts | 43 ++++++++++++++++++- ...ured-agent-session-startup-attempt.test.ts | 3 +- ...tructured-agent-session-startup-attempt.ts | 4 +- 17 files changed, 99 insertions(+), 15 deletions(-) diff --git a/src/main/claude/claude-structured-session-acquisition.ts b/src/main/claude/claude-structured-session-acquisition.ts index 206bf525b66d..d3e02367daf1 100644 --- a/src/main/claude/claude-structured-session-acquisition.ts +++ b/src/main/claude/claude-structured-session-acquisition.ts @@ -281,6 +281,7 @@ export async function acquireClaudeSession({ isCurrent: () => sessions.get(sessionId) === session, fault: (error) => callbacks.handleExit(sessionId, attempt, error), diagnose: (diagnostic) => emit({ type: 'auth-diagnostic', sessionId, diagnostic }), + // The host's minted attempt always carries it; only adapter tests that pass less omit it. optionRevision: () => input.optionRevision?.() ?? 0, report: (event) => emit({ diff --git a/src/main/claude/claude-structured-session-startup.ts b/src/main/claude/claude-structured-session-startup.ts index 1930f254faf4..a1261a81ddc4 100644 --- a/src/main/claude/claude-structured-session-startup.ts +++ b/src/main/claude/claude-structured-session-startup.ts @@ -258,6 +258,7 @@ async function settleClaudeStartupSettings( ): Promise { const { session } = input const sequence = session.optionMutationSequence + // Only a pick moves it, so this stamp holds however late the host takes `started`. const optionRevision = input.optionRevision() const settings = await input.readSettings() if (!input.isCurrent()) { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition-options.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition-options.test.ts index ffb50f0ef99b..495b7c629989 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition-options.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition-options.test.ts @@ -166,6 +166,7 @@ describe('structured session acquisition options', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(CREATE_OPERATION, null), now: () => NOW, onAttached: (attached) => { @@ -205,6 +206,7 @@ describe('structured session acquisition options', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(RESUME_OPERATION, releasedFence), now: () => NOW + 1, onAttached: () => {} @@ -238,6 +240,7 @@ describe('structured session acquisition options', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(CREATE_OPERATION, null, options), now: () => NOW, recordPhase, @@ -269,6 +272,7 @@ describe('structured session acquisition options', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(CREATE_OPERATION, null, options), now: () => NOW, onAttached: () => {} @@ -301,6 +305,7 @@ describe('structured session acquisition options', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(CREATE_OPERATION, null), now: () => NOW, onAttached: () => {} @@ -339,6 +344,7 @@ describe('structured session acquisition options', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(RESUME_OPERATION, releasedFence), now: () => NOW + 1, onAttached: () => {} @@ -377,6 +383,7 @@ describe('structured session acquisition options', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(CREATE_OPERATION, null, { model: 'gpt-standard', fastMode: 'true' @@ -413,6 +420,7 @@ describe('structured session acquisition options', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(CREATE_OPERATION, null, { model: 'gpt-saved', effort: 'medium' }), now: () => NOW, onAttached: () => {} @@ -449,6 +457,7 @@ describe('structured session acquisition options', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(CREATE_OPERATION, null, options), now: () => NOW, onAttached: () => {} @@ -485,6 +494,7 @@ describe('structured session acquisition options', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(CREATE_OPERATION, null), now: () => NOW, onAttached: () => {} @@ -580,6 +590,7 @@ describe('structured session acquisition options', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(operationId, fence), now: () => NOW, onAttached: () => {} @@ -680,6 +691,7 @@ describe('the tab a create reserves', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, // Beside the fingerprinted fields, like `options`: which tab shows the chat is not which // conversation this attaches to. params: { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition-runtime-stamp.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition-runtime-stamp.test.ts index 90c94306db1d..d47f8650abca 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition-runtime-stamp.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition-runtime-stamp.test.ts @@ -114,6 +114,7 @@ it('starts a chat whose recorded owner carries the runtime stamp', async () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(), now: () => NOW, onAttached: () => {} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts index 6f05dbf01674..6b87efc267d1 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts @@ -57,7 +57,7 @@ export async function acquireOwner( spawnToken, ...(input.eventSink ? { events: input.eventSink } : {}), ...(input.acquireSignal ? { signal: input.acquireSignal } : {}), - ...(input.optionRevision ? { optionRevision: input.optionRevision } : {}) + optionRevision: input.optionRevision }) const progress = input.onStartupAttempt?.(attempt) const acquired = await input.adapter.acquire({ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts index 27b3bfa41e41..518aae086231 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts @@ -229,8 +229,8 @@ export type StructuredAgentSessionStartedEvent = { restoreSkippedOptions: readonly string[] /** Values the child showed it cannot run: a report naming the same value is not persisted. */ retiredOptions?: Readonly> - /** The attempt's `optionRevision()` as the read behind this report began: a pick or report the - * host took since makes it out of date. */ + /** The attempt's `optionRevision()` as the read behind this report began: a pick the host took + * since makes it out of date. An earlier report never does, whenever the host took it. */ optionRevision: number } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adopted-import.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adopted-import.test.ts index 93bfc64bae2b..79d5c5d44923 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adopted-import.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adopted-import.test.ts @@ -137,6 +137,7 @@ async function attach( probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(transcriptPath), now: () => NOW, onAttached diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts index e042afb98098..1563146f7e4a 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts @@ -71,7 +71,7 @@ export type AttachFlowInput = { attempt: StructuredAgentSessionStartupAttempt ) => StructuredAgentSessionStartupProgress /** The conversation's option revision, which the child's reports are stamped with. */ - optionRevision?: () => number + optionRevision: () => number /** Publishes the journal before clients can send against the new owner. `acquiredOwner` is * true only when this attach spawned the provider child, so a re-attach to a live one is not * mistaken for a cold acquire. */ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-drivability.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-drivability.test.ts index bbcadedabfef..2814638e0413 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-drivability.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-drivability.test.ts @@ -143,6 +143,7 @@ async function attach(input: { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: input.params, now: () => NOW, onAttached: async (attached) => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-option-revisions.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-option-revisions.ts index dc4fa7bbe39f..0c20a84c1f59 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-option-revisions.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-option-revisions.ts @@ -2,11 +2,15 @@ // // A pick and a child's report both write the record's options, but a report describes the child as // of its read, which can predate a pick still in flight, a saved value the child showed it cannot -// run, or a newer report. Each of those moves the revision. A report carries the revision it was -// read at and is persisted only if nothing moved it since, so no read undoes what followed it. +// run, or a newer report. A pick or such a value moves the revision; a report carries the revision +// it was read at and is admitted only if nothing moved it since. Reports are ordered among +// themselves by arrival, so a read's stamp never depends on when an earlier report was taken, and +// the newest admitted report is the only one persisted. export class StructuredAgentSessionOptionRevisions { private readonly revisions = new Map() + private readonly newestReports = new Map() + private reports = 0 current(sessionId: string): number { return this.revisions.get(sessionId) ?? 0 @@ -19,13 +23,24 @@ export class StructuredAgentSessionOptionRevisions { return next } - /** A report read at `readAt` becomes the newest word, or null when a pick or report came since. */ + /** A report read at `readAt` becomes the newest word, or null when a pick came since its read. */ admitReport(sessionId: string, readAt: number): number | null { - return readAt === this.current(sessionId) ? this.advance(sessionId) : null + if (readAt !== this.current(sessionId)) { + return null + } + const admitted = ++this.reports + this.newestReports.set(sessionId, admitted) + return admitted + } + + /** Whether the report admitted as `admitted`, read at `readAt`, is still the newest word. */ + isNewest(sessionId: string, readAt: number, admitted: number): boolean { + return readAt === this.current(sessionId) && this.newestReports.get(sessionId) === admitted } /** The conversation closed with no child: nothing it reported is still to be persisted. */ forget(sessionId: string): void { this.revisions.delete(sessionId) + this.newestReports.delete(sessionId) } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-pre-spawn-first-answer.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-pre-spawn-first-answer.test.ts index 0a04d6cddfa5..b1b0833ad9c9 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-pre-spawn-first-answer.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-pre-spawn-first-answer.test.ts @@ -100,6 +100,7 @@ async function firstAnswerAndReplay(thrown: AgentSessionPreSpawnError) { probe: { outcome: 'reservation-unused' as const } }, callerKey: 'client-1', + optionRevision: () => 0, params: createParams(), now: () => NOW, onAttached: () => {} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-processless-reservation.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-processless-reservation.test.ts index 543abc1e2537..bebf68bd2f34 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-processless-reservation.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-processless-reservation.test.ts @@ -97,6 +97,7 @@ describe('processless structured session reservation', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(), now: () => NOW, onAttached: () => {} @@ -145,6 +146,7 @@ describe('processless structured session reservation', () => { probe: { outcome: 'reservation-unused' as const } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(), now: () => NOW, onAttached: () => {} @@ -184,6 +186,7 @@ describe('processless structured session reservation', () => { probe: { outcome: 'reservation-unused' as const } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(), now: () => NOW, onAttached: () => {} @@ -237,6 +240,7 @@ describe('processless structured session reservation', () => { probe: { outcome: 'reservation-unused' } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(), now: () => NOW, onAttached: () => {} @@ -306,6 +310,7 @@ describe('processless structured session reservation', () => { probe: { outcome: 'reservation-unused' as const } }, callerKey: 'client-1', + optionRevision: () => 0, params: attachParams(), now: () => NOW, onAttached: () => {} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts index 1663d258f787..b56781fa6a6b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-provider-started.ts @@ -39,7 +39,7 @@ export type StructuredAgentSessionProviderStartedContext = { startupAttempts: Pick optionRevisions: Pick< StructuredAgentSessionOptionRevisions, - 'admitReport' | 'advance' | 'current' + 'admitReport' | 'advance' | 'isNewest' > } /** The barrier lifts: what was accepted while the child started is handed over now. Settles once @@ -114,7 +114,11 @@ function persistReportedOptions( !record || record.lease.runtimeFence !== event.fence || !agentSessionLeaseAdmitsWriter(record.lease) || - context.runtimeState.optionRevisions.current(event.sessionId) !== admitted + !context.runtimeState.optionRevisions.isNewest( + event.sessionId, + event.optionRevision, + admitted + ) ) { return } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts index da4f40dabcbf..ec239cf3a304 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract.ts @@ -39,5 +39,5 @@ export type StructuredAgentSessionStartupAttempt = { /** Any stdout frame or stderr line from the child: proof the start is still moving. */ readonly onOutput?: () => void /** The conversation's option revision now; a report stamps it as its read begins. */ - readonly optionRevision?: () => number + readonly optionRevision: () => number } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts index d52e831d3f6c..154e3e9d11e4 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt-host.test.ts @@ -536,6 +536,45 @@ describe('what a started child reports', () => { expect(store.getRecord(SESSION)?.options).toEqual({ model: 'from-settings' }) }) + + it('keeps a settings read stamped before the host took the start’s report', async () => { + await restartWith(publishFirst) + await heldBehindStart('hello') + // Both reads are stamped before the host takes either report, as a delivery that lags would. + const started = { ...startedEvent(), reportedOptions: { model: 'at-start' } } + const settings = { + ...started, + type: 'options-reported' as const, + reportedOptions: { model: 'from-settings' } + } + + await host.handleAdapterEvent(started) + await host.handleAdapterEvent(settings) + await new Promise((resolve) => setTimeout(resolve, 50)) + await host.flushStreamedEvents(SESSION) + + expect(store.getRecord(SESSION)?.options).toEqual({ model: 'from-settings' }) + }) + + it('takes a report read after a pick that failed', async () => { + await restartWith(publishFirst) + const id = await heldBehindStart('hello') + await host.handleAdapterEvent(startedEvent()) + await eventually(async () => expect((await submission(id))?.dispatchState).toBe('accepted')) + await eventually(() => expect(store.getRecord(SESSION)?.options).toEqual({ model: 'default' })) + setOption.mockRejectedValueOnce(new Error('the provider refused the model')) + await expect(pick('refused')).rejects.toThrow('the provider refused the model') + + await host.handleAdapterEvent({ + ...startedEvent(), + type: 'options-reported', + reportedOptions: { model: 'from-settings' } + }) + + await eventually(() => + expect(store.getRecord(SESSION)?.options).toEqual({ model: 'from-settings' }) + ) + }) }) function startupAttemptOpen(): boolean { @@ -556,6 +595,8 @@ describe('the attempt an acquire runs under', () => { expect(input.onOutput).toEqual(expect.any(Function)) expect(input.attemptId).toEqual(expect.any(String)) expect(input.signal).toBeInstanceOf(AbortSignal) - expect(input.optionRevision).toEqual(expect.any(Function)) + expect(input.optionRevision!()).toBe( + host.collaboratorsForTests().runtimeState.optionRevisions.current(SESSION) + ) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts index 841b35825589..ddf04b4b0e73 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.test.ts @@ -34,7 +34,8 @@ function mint() { agent: 'codex', providerHandle: null }, - spawnToken: 'spawn-1' + spawnToken: 'spawn-1', + optionRevision: () => 0 }) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts index 369a7defa26d..efecd7f77376 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-startup-attempt.ts @@ -28,7 +28,7 @@ export function mintStructuredAgentSessionStartupAttempt(input: { spawnToken: string events?: StructuredAgentSessionEventSink signal?: AbortSignal - optionRevision?: () => number + optionRevision: () => number }): StructuredAgentSessionStartupAttempt { const { record } = input return { @@ -44,7 +44,7 @@ export function mintStructuredAgentSessionStartupAttempt(input: { ...(record.options ? { options: record.options } : {}), ...(input.events ? { events: input.events } : {}), ...(input.signal ? { signal: input.signal } : {}), - ...(input.optionRevision ? { optionRevision: input.optionRevision } : {}) + optionRevision: input.optionRevision } }