From 6ba01b3f1ac5872a58c6e4fe212632be03d9aafd Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 12 Oct 2026 13:55:52 -0700 Subject: [PATCH 01/11] Publish Pi at spawn and settle signed-out starts once --- .../scripts/vitest-sqlite-runtime-files.mjs | 3 + src/main/jsonl-rpc/agent-connection.ts | 13 +- .../structured-agent-session-host-delivery.ts | 4 +- ...red-agent-session-signed-out-child.test.ts | 22 +- ...ructured-agent-session-signed-out-child.ts | 10 +- src/main/pi/pi-scripted-child.test-fixture.ts | 261 ++++++++++++++ src/main/pi/rpc-configured-default.test.ts | 25 +- src/main/pi/rpc-options.test.ts | 4 +- src/main/pi/rpc-options.ts | 7 +- src/main/pi/rpc-runtime-registration.ts | 12 +- .../pi/rpc-session-adapter.test-fixture.ts | 221 ++++++++++++ src/main/pi/rpc-session-adapter.test.ts | 222 ++---------- src/main/pi/rpc-session-adapter.ts | 18 +- src/main/pi/rpc-session-startup.test.ts | 248 ++++++++++++++ src/main/pi/rpc-session-startup.ts | 174 ++++++++++ src/main/pi/rpc-session.ts | 109 +++--- ...-agent-session-pi-signed-out-start.test.ts | 94 +++++ ...tructured-agent-session-pi-startup.test.ts | 188 ++++++++++ ...provider-operations-while-starting.test.ts | 323 ++++++++++++++++++ ...tured-agent-session-transport-overrides.ts | 3 + 20 files changed, 1683 insertions(+), 278 deletions(-) create mode 100644 src/main/pi/pi-scripted-child.test-fixture.ts create mode 100644 src/main/pi/rpc-session-adapter.test-fixture.ts create mode 100644 src/main/pi/rpc-session-startup.test.ts create mode 100644 src/main/pi/rpc-session-startup.ts create mode 100644 src/main/runtime/structured-agent-session-pi-signed-out-start.test.ts create mode 100644 src/main/runtime/structured-agent-session-pi-startup.test.ts create mode 100644 src/main/runtime/structured-agent-session-provider-operations-while-starting.test.ts diff --git a/config/scripts/vitest-sqlite-runtime-files.mjs b/config/scripts/vitest-sqlite-runtime-files.mjs index 4c2e4878a847..dd21b83a3451 100644 --- a/config/scripts/vitest-sqlite-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-runtime-files.mjs @@ -242,6 +242,9 @@ export const SQLITE_RUNTIME_INCLUDE = [ 'src/main/runtime/runtime-client-agent-launch-settings-durability.test.ts', 'src/main/runtime/structured-agent-session-codex-scripted-child.test.ts', 'src/main/runtime/structured-agent-session-codex-start-deadline.test.ts', + 'src/main/runtime/structured-agent-session-pi-startup.test.ts', + 'src/main/runtime/structured-agent-session-pi-signed-out-start.test.ts', + 'src/main/runtime/structured-agent-session-provider-operations-while-starting.test.ts', 'src/main/runtime/structured-agent-session-codex-settlement-order.test.ts', 'src/main/runtime/structured-agent-session-integration-replay.test.ts', 'src/main/runtime/structured-agent-session-integration.test.ts', diff --git a/src/main/jsonl-rpc/agent-connection.ts b/src/main/jsonl-rpc/agent-connection.ts index 551beb40dec6..a194660f7b70 100644 --- a/src/main/jsonl-rpc/agent-connection.ts +++ b/src/main/jsonl-rpc/agent-connection.ts @@ -1,5 +1,6 @@ import type { PipedProcessSpawner } from '@orca/process-host/process-spec' import { spawnProcess } from '@orca/process-host' +import { providerDiagnostic, withProviderDiagnostic } from '../../shared/agent-session-failure' import { spawnManagedProviderProcess, type ManagedProviderProcess, @@ -81,10 +82,14 @@ export class JsonlRpcAgentConnection { managed.child.stderr.on('error', this.onError) managed.onExit((exit) => { clearTimeout(this.streamExitTimer) - const error = new Error( - exit.processless - ? 'Agent process could not start' - : `Agent process exited (code ${exit.code ?? 'none'}, signal ${exit.signal ?? 'none'})` + // The agent's last stderr lines are what a person can act on when it dies. + const error = withProviderDiagnostic( + new Error( + exit.processless + ? 'Agent process could not start' + : `Agent process exited (code ${exit.code ?? 'none'}, signal ${exit.signal ?? 'none'})` + ), + providerDiagnostic(managed.stderrTail(), 'person') ) this.peer.finishOnInputEnd(error) const stdout = managed.child.stdout diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts index 9ff8543d514e..bd9102421ed3 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts @@ -76,7 +76,7 @@ export function createStructuredAgentSessionConversationDelivery(input: { const session = input.clientDelivery.readStopping(sessionId) ? undefined : sessions.get(sessionId) - await retireSignedOutStructuredAgentSessionChild(sessionId, session, { + await retireSignedOutStructuredAgentSessionChild(sessionId, session, startedFor, { work: { childWork: () => input.clientDelivery.readChildWork(sessionId), hasOpenDispatch: () => { @@ -189,7 +189,7 @@ export function createStructuredAgentSessionConversationDelivery(input: { * A rewind found prepared when the conversation opens was started under a child this process no * longer has — the open runs only when none is indexed — so nothing will finish it, and left alone * it refuses every send until a view attaches. Settled here instead of by a start inside - * acceptance. A Codex rewind only its provider can prove stays for the attach. + * acceptance. A Codex rewind only its provider can prove stays for the next proven start. */ async function settleInterruptedCommands( deps: StructuredAgentSessionHostDeps, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-signed-out-child.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-signed-out-child.test.ts index ae09e13d8782..13fa794a4eb1 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-signed-out-child.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-signed-out-child.test.ts @@ -205,10 +205,11 @@ describe('retireSignedOutStructuredAgentSessionChild', () => { const retire = async ( input: Parameters[0], work: Partial = {}, - stopAgent = vi.fn(async () => undefined) + stopAgent = vi.fn(async () => undefined), + startedFor = 'next-send' ) => { const warn = vi.fn() - await retireSignedOutStructuredAgentSessionChild('session', conversation(input), { + await retireSignedOutStructuredAgentSessionChild('session', conversation(input), startedFor, { work: { ...idle, ...work }, stopAgent, logger: { warn, error: vi.fn() } @@ -220,6 +221,23 @@ describe('retireSignedOutStructuredAgentSessionChild', () => { expect((await retire(signedOut)).stopAgent).toHaveBeenCalledWith('session') }) + it('keeps a child started for the send it is about to take', async () => { + const own = await retire( + { ...signedOut, child: childAt({ startedFor: 'next-send' }) }, + {}, + undefined, + 'next-send' + ) + expect(own.stopAgent).not.toHaveBeenCalled() + const earlier = await retire( + { ...signedOut, child: childAt({ startedFor: 'earlier-send' }) }, + {}, + undefined, + 'next-send' + ) + expect(earlier.stopAgent).toHaveBeenCalledWith('session') + }) + it('keeps it while it owes work the idle sweep also protects', async () => { // A pending prompt is the shared check's too; the idle sweep's tests cover that case. for (const kept of [ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-signed-out-child.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-signed-out-child.ts index 40ea59a69eb8..c9c891bc3791 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-signed-out-child.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-signed-out-child.ts @@ -78,12 +78,15 @@ function owesWakeUp( ) } -/** For a caller inside the session's serialize, before it hands the next send to the child. Work - * the child still owes keeps it, as it keeps an idle one from the sweep. A stop that fails leaves - * the close begun, which the start joins. */ +/** For a caller inside the session's serialize, before it hands the next send (`startedFor`) to + * the child. A child started for that same send keeps it: the send has not reached any child yet, + * and the agent's own refusal settles it once, where a new child would refuse it again forever. + * Work the child still owes keeps it, as it keeps an idle one from the sweep. A stop that fails + * leaves the close begun, which the start joins. */ export async function retireSignedOutStructuredAgentSessionChild( sessionId: string, session: SignedOutReading | undefined, + startedFor: string, deps: { work: StructuredAgentSessionChildWorkReads startUnavailable?: () => AgentSessionUnavailable | undefined @@ -93,6 +96,7 @@ export async function retireSignedOutStructuredAgentSessionChild( ): Promise { if ( !session || + session.child?.startedFor === startedFor || !structuredAgentSessionChildReportedSignedOut(session, deps.startUnavailable?.()) || structuredAgentSessionChildHasOpenWork(session.journal, deps.work) || owesWakeUp(session.journal, deps.work.childWork()) diff --git a/src/main/pi/pi-scripted-child.test-fixture.ts b/src/main/pi/pi-scripted-child.test-fixture.ts new file mode 100644 index 000000000000..635c30bc8a75 --- /dev/null +++ b/src/main/pi/pi-scripted-child.test-fixture.ts @@ -0,0 +1,261 @@ +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { providerDiagnostic, withProviderDiagnostic } from '../../shared/agent-session-failure' +import type { JsonlRpcAgentConnectionOptions } from '../jsonl-rpc/agent-connection' +import { JsonlRpcResponseError, type JsonlRpcRecord } from '../jsonl-rpc/peer' +import type { ProviderProcessLaunch } from '../provider-process/provider-process-launch' +import type { ScriptedAgentChild } from '../runtime/structured-agent-scripted-child.test-fixture' +import type { PiRpcConnection } from './rpc-session' + +class ScriptedPiConnection implements PiRpcConnection { + closed = false + rootVerdict: PiRpcConnection['rootVerdict'] = 'live' + processless = false + lastCloseResult: PiRpcConnection['lastCloseResult'] = null + readonly pid: number + readonly file: string + private readonly handshake = Promise.withResolvers() + private readonly exitListeners = new Set<() => void>() + private firstStateRead = true + private streaming = false + private model = { provider: 'scripted', id: 'model-1', reasoning: true } + private effort = 'medium' + + constructor( + launch: ProviderProcessLaunch, + private readonly handlers: JsonlRpcAgentConnectionOptions, + private readonly script: ScriptedAgentChild, + private readonly received: string[], + private readonly wire: string[], + private readonly onClosed: () => void, + number: number, + /** A Pi with no login: it lists no model and fails every prompt as not signed in. */ + private readonly signedOut = false + ) { + this.pid = 42_000 + number + const sessionIndex = launch.args.indexOf('--session') + this.file = + sessionIndex !== -1 + ? launch.args[sessionIndex + 1] + : join(tmpdir(), 'orca-scripted-pi', `session-${number}.jsonl`) + void this.handshake.promise.catch(() => undefined) + } + + async request(command: string, params: Record = {}): Promise { + if (this.closed) { + throw new Error('Scripted Pi connection closed') + } + this.wire.push(command) + if (command === 'get_state') { + if (this.firstStateRead) { + this.firstStateRead = false + if (!this.script.holdHandshakes) { + this.releaseHandshake() + } + return this.handshake.promise + } + return this.state() + } + if (command === 'get_available_models') { + return { models: this.signedOut ? [] : [this.model] } + } + if (command === 'get_commands') { + return { commands: [] } + } + if ( + command === 'set_model' && + typeof params.provider === 'string' && + typeof params.modelId === 'string' + ) { + this.model = { ...this.model, provider: params.provider, id: params.modelId } + return this.model + } + if (command === 'set_thinking_level' && typeof params.level === 'string') { + this.effort = params.level + return {} + } + if (command === 'abort') { + this.endTurn('aborted') + return {} + } + throw new Error(`Unscripted Pi request: ${command}`) + } + + async send(frame: JsonlRpcRecord): Promise { + if (this.closed || frame.type !== 'prompt' || typeof frame.message !== 'string') { + throw new Error('Scripted Pi requires a live prompt') + } + this.received.push(frame.message) + this.wire.push(`prompt:${frame.message}`) + if (this.signedOut) { + this.emit({ + type: 'response', + command: 'prompt', + success: false, + error: 'No API key found for scripted' + }) + return + } + if (!this.streaming) { + this.streaming = true + this.emit({ type: 'agent_start' }) + } + this.emit({ type: 'message_end', message: { role: 'user', content: frame.message } }) + this.emit({ + type: 'response', + command: 'prompt', + success: true, + data: { disposition: 'started' } + }) + if (this.script.completeTurns) { + this.endTurn('stop') + } + } + + releaseHandshake(): void { + if (!this.closed) { + this.handshake.resolve(this.state()) + } + } + + failHandshake(message: string): void { + const error = withProviderDiagnostic( + new JsonlRpcResponseError('get_state', message), + providerDiagnostic(message, 'person') + ) + this.handshake.reject(error) + this.exit(error) + } + + async close(): Promise>> { + if (!this.closed) { + this.onClosed() + const error = new Error('Scripted Pi closed') + this.handshake.reject(error) + this.exit(error) + } + return { root: 'exited', tree: 'exited' } + } + + onExit(listener: () => void): void { + if (this.rootVerdict === 'exited') { + listener() + } else { + this.exitListeners.add(listener) + } + } + + pauseReading(): void {} + resumeReading(): void {} + + private exit(error: Error): void { + if (this.closed) { + return + } + this.closed = true + this.rootVerdict = 'exited' + this.lastCloseResult = { root: 'exited', tree: 'exited' } + this.exitListeners.forEach((listener) => listener()) + this.exitListeners.clear() + this.handlers.onExit?.(error, { + expected: false, + exit: { code: 0, signal: null, processless: false } + }) + } + + private endTurn(stopReason: string): void { + if (!this.streaming) { + return + } + this.streaming = false + this.emit({ + type: 'message_end', + message: { role: 'assistant', content: 'Scripted Pi reply', stopReason } + }) + this.emit({ type: 'agent_end' }) + this.emit({ type: 'agent_settled' }) + } + + private emit(frame: JsonlRpcRecord): void { + this.handlers.onRecord?.(frame) + } + + private state() { + return { + sessionFile: this.file, + model: this.signedOut ? null : this.model, + thinkingLevel: this.effort, + isStreaming: this.streaming, + isCompacting: false, + pendingMessageCount: 0 + } + } +} + +export type ScriptedPiChild = ScriptedAgentChild & { + /** Every request command and prompt any spawn received, in order. */ + wire(): readonly string[] +} + +export const piScriptedChild = (options: { signedOut?: boolean } = {}): ScriptedPiChild => { + const children: ScriptedPiConnection[] = [] + const received: string[] = [] + const wire: string[] = [] + let resumed = 0 + let closed = 0 + const script: ScriptedPiChild = { + holdHandshakes: true, + completeTurns: true, + deps: { + resolvePiLaunch: async ({ identity }) => { + const handle = identity.providerHandle + if (handle && (handle.transport !== 'jsonl-rpc' || handle.agent !== 'pi')) { + throw new Error('Scripted Pi received another provider handle') + } + return { + command: join(tmpdir(), 'orca-scripted-pi', 'fake-pi'), + cwd: tmpdir(), + fullAccess: true, + previous: handle + ? { + linkId: 'scripted-previous', + handle, + origin: 'created', + mintedAtFence: 0, + observedAt: Date.now() + } + : null, + ...(handle ? { sessionFile: handle.nativeId } : {}) + } + }, + openPiConnection: (launch, handlers) => { + if (launch.args.includes('--session')) { + resumed += 1 + } + const child = new ScriptedPiConnection( + launch, + handlers, + script, + received, + wire, + () => { + closed += 1 + }, + children.length + 1, + options.signedOut + ) + children.push(child) + return child + }, + readProcessStartTime: async () => 1_700_000_000_000 + }, + releaseHandshake: () => children.at(-1)?.releaseHandshake(), + failHandshake: (message) => children.at(-1)?.failHandshake(message), + prompts: () => received, + spawns: () => children.length, + resumes: () => resumed, + closes: () => closed, + wire: () => wire + } + return script +} diff --git a/src/main/pi/rpc-configured-default.test.ts b/src/main/pi/rpc-configured-default.test.ts index 467417c8eadd..7ba6289084a4 100644 --- a/src/main/pi/rpc-configured-default.test.ts +++ b/src/main/pi/rpc-configured-default.test.ts @@ -28,6 +28,10 @@ import { agentModelCatalogFingerprint } from '../native-chat/agent-model-catalog import { createPiModelCatalogProbe, piModelCatalogFromListing } from './rpc-model-catalog-probe' import type { PiRpcConnection } from './rpc-session' import { PiRpcSessionAdapter } from './rpc-session-adapter' +import type { + StructuredAgentSessionLifecycleEvent, + StructuredAgentSessionStartedEvent +} from '../native-chat/agent-session-wire/structured-agent-session-adapter' const PI_HOME = { variable: 'PI_CODING_AGENT_DIR', path: '/homes/pi' } // What `pi --list-models` prints: the first row is not the model Pi starts on. @@ -176,6 +180,7 @@ async function piHost() { } } as AgentSessionRecord) const rig = await openProviderTimelineRig({ agent: 'pi', sessionId }) + const onLifecycle = vi.fn<(event: StructuredAgentSessionLifecycleEvent) => void>() const adapter = new PiRpcSessionAdapter({ resolveLaunch: async () => ({ command: '/opt/pi/bin/pi', @@ -191,13 +196,13 @@ async function piHost() { child = piChild(opts.available ?? AVAILABLE, opts.thinking ?? 'high', opts.startsOn ?? '') return child.connection }, - onLifecycle: vi.fn(), + onLifecycle, onSettled: vi.fn(), onIdle: vi.fn(), logger: { warn: vi.fn(), error: vi.fn() } }) adapters.push(adapter) - const acquired = await adapter.acquire({ + await adapter.acquire({ identity: { sessionId, workspaceId: 'ws-1', @@ -210,10 +215,20 @@ async function piHost() { options: opts.saved ?? {}, events: rig.eventSink }) - if (!acquired.catalogListing) { + // The acquire answers at spawn; the start hands the host its listing once it is proven. + const started = await vi.waitFor(() => { + const event = onLifecycle.mock.calls + .map(([call]) => call) + .find((call): call is StructuredAgentSessionStartedEvent => call.type === 'started') + if (!event) { + throw new Error('Pi has not started') + } + return event + }) + if (!started.catalogListing) { throw new Error('a Pi start hands the host its listing') } - catalog.recordLiveListing(sessionId, acquired.catalogListing) + catalog.recordLiveListing(sessionId, started.catalogListing) if (opts.afterStart && child) { await opts.afterStart({ child, adapter, sessionId }) } @@ -224,7 +239,7 @@ async function piHost() { } } await read() - return { listing: acquired.catalogListing, read } + return { listing: started.catalogListing, read } } return { catalog, chat, store, clock } } diff --git a/src/main/pi/rpc-options.test.ts b/src/main/pi/rpc-options.test.ts index 5beaefb54ca1..d15c602cf550 100644 --- a/src/main/pi/rpc-options.test.ts +++ b/src/main/pi/rpc-options.test.ts @@ -70,8 +70,8 @@ describe('Pi RPC session options', () => { effort: 'high' }) expect(rpc.request.mock.calls).toEqual([ - ['set_model', { provider: 'openai-codex', modelId: 'gpt-6.1-sol' }], - ['set_thinking_level', { level: 'high' }] + ['set_model', { provider: 'openai-codex', modelId: 'gpt-6.1-sol' }, {}], + ['set_thinking_level', { level: 'high' }, {}] ]) expect(() => parsePiModelOptionId('openai-codex')).toThrow() await expect(applyPiRpcSessionOption(rpc, selected, 'model', '/gpt-6.1-sol')).rejects.toThrow() diff --git a/src/main/pi/rpc-options.ts b/src/main/pi/rpc-options.ts index 4f75e064c6e1..b881d31f1729 100644 --- a/src/main/pi/rpc-options.ts +++ b/src/main/pi/rpc-options.ts @@ -92,17 +92,18 @@ export async function applyPiRpcSessionOption( rpc: PiRpcRequester, selected: Map, key: string, - value: string + value: string, + options: { timeoutMs?: number | null } = {} ): Promise>> { if (key === 'model') { const model = parsePiModelOptionId(value) modelSchema.parse({ provider: model.provider, id: model.modelId }) - await rpc.request('set_model', model) + await rpc.request('set_model', model, options) } else if (key === 'effort') { if (!THINKING_LEVELS.some((level) => level === value)) { throw new Error(`Unsupported Pi thinking level: ${value}`) } - await rpc.request('set_thinking_level', { level: value }) + await rpc.request('set_thinking_level', { level: value }, options) } else { throw new Error(`Unsupported Pi option: ${key}`) } diff --git a/src/main/pi/rpc-runtime-registration.ts b/src/main/pi/rpc-runtime-registration.ts index a8e97e5e3081..8bb3c1a91f47 100644 --- a/src/main/pi/rpc-runtime-registration.ts +++ b/src/main/pi/rpc-runtime-registration.ts @@ -24,11 +24,13 @@ import { createPiModelCatalogProbe } from './rpc-model-catalog-probe' function createPiRpcAdapter(context: StructuredAgentAdapterContext): StructuredAgentRuntimeAdapter { const { deps } = context return new PiRpcSessionAdapter({ - resolveLaunch: composeStructuredLaunch( - PI_RPC_AGENT, - structuredAgentLaunchSources(context), - piRpcLaunchPart({}) - ), + resolveLaunch: + deps.resolvePiLaunch ?? + composeStructuredLaunch( + PI_RPC_AGENT, + structuredAgentLaunchSources(context), + piRpcLaunchPart({}) + ), ...(deps.openPiConnection ? { openConnection: deps.openPiConnection } : {}), ...(deps.readProcessStartTime ? { readProcessStartTime: deps.readProcessStartTime } : {}), onLifecycle: context.deliverLifecycle, diff --git a/src/main/pi/rpc-session-adapter.test-fixture.ts b/src/main/pi/rpc-session-adapter.test-fixture.ts new file mode 100644 index 000000000000..573d28d9a678 --- /dev/null +++ b/src/main/pi/rpc-session-adapter.test-fixture.ts @@ -0,0 +1,221 @@ +import { expect, vi, type Mock } from 'vitest' +import type { AgentSessionProcessIdentity } from '../../shared/agent-session-record' +import type { StructuredAgentSessionAcquireInput } from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import { + closeProviderTimelineRigs, + openProviderTimelineRig +} from '../native-chat/agent-session-timeline/provider-timeline-assembler-test-support' +import type { JsonlRpcAgentConnectionOptions } from '../jsonl-rpc/agent-connection' +import type { JsonlRpcRecord } from '../jsonl-rpc/peer' +import { JsonlRpcResponseError } from '../jsonl-rpc/peer' +import type { PiRpcConnection } from './rpc-session' +import type { ProviderProcessLaunch } from '../provider-process/provider-process-launch' +import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' +import { PiRpcSessionAdapter, type PiRpcSessionAdapterDeps } from './rpc-session-adapter' +import type { PiRpcResolvedLaunch } from './rpc-launch-resolution' + +export const sessionId = 'session-timeline' +export const file = '/host/account/sessions/session.jsonl' +export const state = { + sessionFile: file, + isStreaming: false, + isCompacting: false, + model: { provider: 'anthropic', id: 'model-1' } +} +const cleanups: (() => Promise)[] = [] +export async function closePiRpcAdapterFixtures(): Promise { + for (const cleanup of cleanups.splice(0)) { + await cleanup() + } + await closeProviderTimelineRigs() +} + +export class FakeConnection implements PiRpcConnection { + pid = 4123 + closed = false + rootVerdict: 'live' | 'unverifiable' | 'exited' = 'live' + processless = false + deferExit = false + closeError?: Error + lastCloseResult: PiRpcConnection['lastCloseResult'] = null + closeResult: Awaited> = { root: 'exited', tree: 'exited' } + readonly sent: JsonlRpcRecord[] = [] + readonly requests: string[] = [] + private readonly exitListeners: (() => void)[] = [] + requestOverride?: (command: string) => Promise | undefined + abortRequest?: () => void + constructor(readonly handlers: JsonlRpcAgentConnectionOptions) {} + async request(command: string): Promise { + this.requests.push(command) + const override = this.requestOverride?.(command) + if (override) { + return override + } + if (command === 'get_state') { + return state + } + if (command === 'get_available_models') { + return { models: [state.model] } + } + if (command === 'get_commands') { + return { commands: [{ name: 'help' }] } + } + if (command === 'set_thinking_level') { + throw new JsonlRpcResponseError(command, 'saved effort unavailable') + } + return {} + } + async send(frame: JsonlRpcRecord): Promise { + this.sent.push(frame) + } + async close(): Promise>> { + this.abortRequest?.() + this.closed = true + this.rootVerdict = this.closeResult.root + this.lastCloseResult = this.closeResult + if (this.closeResult.root === 'exited' && !this.deferExit) { + this.exit(this.closeError) + } + return this.closeResult + } + pauseReading(): void {} + resumeReading(): void {} + onExit(listener: () => void): void { + if (this.rootVerdict === 'exited') { + listener() + } else { + this.exitListeners.push(listener) + } + } + receive(frame: JsonlRpcRecord): void { + this.handlers.onRecord?.(frame) + } + exit(error = new Error('child exited')): void { + this.closed = true + this.rootVerdict = 'exited' + for (const listener of this.exitListeners.splice(0)) { + listener() + } + this.handlers.onExit?.(error, { + expected: false, + exit: { code: 1, signal: null, processless: false } + }) + } +} + +type PiLifecycleEvent = Parameters[0] + +/** The first `type` event the adapter reported; a missing one fails the test. */ +export function reportedEvent( + lifecycle: Mock, + type: T +): Extract { + const isType = (event: PiLifecycleEvent): event is Extract => + event.type === type + const found = lifecycle.mock.calls.map(([event]) => event).find(isType) + if (!found) { + throw new Error(`no ${type} event was reported`) + } + return found +} + +export async function setup( + options: Readonly> = {}, + eventSink?: (sink: StructuredAgentSessionEventSink) => StructuredAgentSessionEventSink +) { + const rig = await openProviderTimelineRig({ agent: 'pi', sessionId }) + const connections: FakeConnection[] = [] + const lifecycle = vi.fn() + const settled = vi.fn() + const idle = vi.fn() + const onSpawned = vi.fn(async (_process: AgentSessionProcessIdentity) => { + expect(connections.at(-1)?.requests).toEqual([]) + }) + const input: StructuredAgentSessionAcquireInput = { + identity: { + sessionId, + workspaceId: 'folder-1', + hostId: 'local', + agent: 'pi', + providerHandle: null + }, + fence: 7, + spawnToken: 'spawn-token', + options, + events: eventSink?.(rig.eventSink) ?? rig.eventSink, + onSpawned + } + const resolveLaunch = vi.fn(async (): Promise => ({ + command: '/host/bin/pi', + cwd: '/host/folder', + fullAccess: true, + previous: null + })) + const openConnection = vi.fn( + (_launch: ProviderProcessLaunch, handlers: JsonlRpcAgentConnectionOptions) => { + const connection = new FakeConnection(handlers) + connections.push(connection) + return connection + } + ) + const adapter = new PiRpcSessionAdapter({ + resolveLaunch, + readProcessStartTime: async () => 12345, + openConnection, + onLifecycle: lifecycle, + onSettled: settled, + onIdle: idle, + logger: { warn: vi.fn(), error: vi.fn() } + }) + const acquired = await adapter.acquire(input) + // Commands are read after the start; waiting for them leaves nothing of it in flight. + await vi.waitFor(() => expect(adapter.readCommands(sessionId)).toBeDefined()) + const started = reportedEvent(lifecycle, 'started') + lifecycle.mockClear() + const connection = connections[0] + if (!connection) { + throw new Error('connection missing') + } + cleanups.push(async () => { + await adapter.closeAll().catch(() => {}) + await adapter.drainObservedExits() + adapter.acknowledgeSessionRelease(sessionId) + }) + return { + adapter, + connection, + rig, + acquired, + started, + onSpawned, + lifecycle, + settled, + idle, + input, + resolveLaunch, + openConnection, + connections + } +} + +export async function restart( + h: Awaited>, + configure: (connection: FakeConnection) => void, + input: Partial = {} +) { + await h.adapter.closeSession(sessionId) + await h.adapter.drainObservedExits() + h.lifecycle.mockClear() + h.openConnection.mockImplementationOnce((_launch, handlers) => { + const connection = new FakeConnection(handlers) + configure(connection) + h.connections.push(connection) + return connection + }) + const acquired = await h.adapter.acquire({ ...h.input, fence: 8, ...input }) + const child = h.connections.at(-1) + if (!child) { + throw new Error('child missing') + } + return { acquired, child } +} diff --git a/src/main/pi/rpc-session-adapter.test.ts b/src/main/pi/rpc-session-adapter.test.ts index 8c74e65eea1c..48a0b9bc5bce 100644 --- a/src/main/pi/rpc-session-adapter.test.ts +++ b/src/main/pi/rpc-session-adapter.test.ts @@ -1,185 +1,18 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { agentJournalItemKey } from '../../shared/agent-session-journal-item-key' -import type { AgentSessionProcessIdentity } from '../../shared/agent-session-record' import { agentJournalTurnBody } from '../../shared/agent-session-turn-record' -import { - AgentSessionAcquisitionRootExitObservedError, - type StructuredAgentSessionAcquireInput -} from '../native-chat/agent-session-wire/structured-agent-session-adapter' -import { - closeProviderTimelineRigs, - openProviderTimelineRig -} from '../native-chat/agent-session-timeline/provider-timeline-assembler-test-support' -import type { JsonlRpcAgentConnectionOptions } from '../jsonl-rpc/agent-connection' -import type { JsonlRpcRecord } from '../jsonl-rpc/peer' +import { AgentSessionAcquisitionRootExitObservedError } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import { JsonlRpcResponseError } from '../jsonl-rpc/peer' -import type { PiRpcConnection } from './rpc-session' -import type { ProviderProcessLaunch } from '../provider-process/provider-process-launch' -import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' +import { + closePiRpcAdapterFixtures, + FakeConnection, + file, + sessionId, + setup +} from './rpc-session-adapter.test-fixture' import { agentSessionLaunchFolderMissing } from '../runtime/agent-session-launch-directory' -import { PiRpcSessionAdapter } from './rpc-session-adapter' - -const sessionId = 'session-timeline' -const file = '/host/account/sessions/session.jsonl' -const state = { - sessionFile: file, - isStreaming: false, - isCompacting: false, - model: { provider: 'anthropic', id: 'model-1' } -} -const cleanups: (() => Promise)[] = [] -afterEach(async () => { - for (const cleanup of cleanups.splice(0)) { - await cleanup() - } - await closeProviderTimelineRigs() -}) - -class FakeConnection implements PiRpcConnection { - pid = 4123 - closed = false - rootVerdict: 'live' | 'unverifiable' | 'exited' = 'live' - processless = false - deferExit = false - lastCloseResult: PiRpcConnection['lastCloseResult'] = null - closeResult: Awaited> = { root: 'exited', tree: 'exited' } - readonly sent: JsonlRpcRecord[] = [] - readonly requests: string[] = [] - private readonly exitListeners: (() => void)[] = [] - requestOverride?: (command: string) => Promise | undefined - abortRequest?: () => void - constructor(readonly handlers: JsonlRpcAgentConnectionOptions) {} - async request(command: string): Promise { - this.requests.push(command) - const override = this.requestOverride?.(command) - if (override) { - return override - } - if (command === 'get_state') { - return state - } - if (command === 'get_available_models') { - return { models: [state.model] } - } - if (command === 'get_commands') { - return { commands: [{ name: 'help' }] } - } - if (command === 'set_thinking_level') { - throw new Error('saved effort unavailable') - } - return {} - } - async send(frame: JsonlRpcRecord): Promise { - this.sent.push(frame) - } - async close(): Promise>> { - this.abortRequest?.() - this.closed = true - this.rootVerdict = this.closeResult.root - this.lastCloseResult = this.closeResult - if (this.closeResult.root === 'exited' && !this.deferExit) { - this.exit() - } - return this.closeResult - } - pauseReading(): void {} - resumeReading(): void {} - onExit(listener: () => void): void { - if (this.rootVerdict === 'exited') { - listener() - } else { - this.exitListeners.push(listener) - } - } - receive(frame: JsonlRpcRecord): void { - this.handlers.onRecord?.(frame) - } - exit(error = new Error('child exited')): void { - this.closed = true - this.rootVerdict = 'exited' - for (const listener of this.exitListeners.splice(0)) { - listener() - } - this.handlers.onExit?.(error, { - expected: false, - exit: { code: 1, signal: null, processless: false } - }) - } -} -async function setup( - options: Readonly> = {}, - eventSink?: (sink: StructuredAgentSessionEventSink) => StructuredAgentSessionEventSink -) { - const rig = await openProviderTimelineRig({ agent: 'pi', sessionId }) - const connections: FakeConnection[] = [] - const lifecycle = vi.fn(), - settled = vi.fn(), - idle = vi.fn() - const onSpawned = vi.fn(async (_process: AgentSessionProcessIdentity) => { - expect(connections.at(-1)?.requests).toEqual([]) - }) - const input: StructuredAgentSessionAcquireInput = { - identity: { - sessionId, - workspaceId: 'folder-1', - hostId: 'local', - agent: 'pi', - providerHandle: null - }, - fence: 7, - spawnToken: 'spawn-token', - options, - events: eventSink?.(rig.eventSink) ?? rig.eventSink, - onSpawned - } - const resolveLaunch = vi.fn(async () => ({ - command: '/host/bin/pi', - cwd: '/host/folder', - fullAccess: true, - previous: null - })) - const openConnection = vi.fn( - (_launch: ProviderProcessLaunch, handlers: JsonlRpcAgentConnectionOptions) => { - const connection = new FakeConnection(handlers) - connections.push(connection) - return connection - } - ) - const adapter = new PiRpcSessionAdapter({ - resolveLaunch, - readProcessStartTime: async () => 12345, - openConnection, - onLifecycle: lifecycle, - onSettled: settled, - onIdle: idle, - logger: { warn: vi.fn(), error: vi.fn() } - }) - const acquired = await adapter.acquire(input) - const connection = connections[0] - if (!connection) { - throw new Error('connection missing') - } - cleanups.push(async () => { - await adapter.closeAll().catch(() => {}) - await adapter.drainObservedExits() - adapter.acknowledgeSessionRelease(sessionId) - }) - return { - adapter, - connection, - rig, - acquired, - onSpawned, - lifecycle, - settled, - idle, - input, - resolveLaunch, - openConnection, - connections - } -} +afterEach(closePiRpcAdapterFixtures) describe('Pi RPC session ownership and delivery', () => { it('refuses a floating chat whose folder is gone with the reason the person reads', async () => { @@ -206,17 +39,16 @@ describe('Pi RPC session ownership and delivery', () => { await expect(started).rejects.toThrow('closed while starting') expect(h.connections).toHaveLength(1) await expect(h.adapter.acquire({ ...h.input, fence: 9 })).resolves.toMatchObject({ - link: { origin: 'created' } + process: { pid: 4123 } }) resolving.resolve(launch!) await Promise.resolve() expect(h.connections).toHaveLength(2) }) - it('kills the child and rejects a stalled startup when the host aborts it', async () => { + it('closes a published child during a stalled handshake without reporting started', async () => { const h = await setup() await h.adapter.closeSession(sessionId) - const controller = new AbortController() const opened = Promise.withResolvers() const reply = Promise.withResolvers() h.openConnection.mockImplementationOnce((_launch, handlers) => { @@ -228,11 +60,26 @@ describe('Pi RPC session ownership and delivery', () => { opened.resolve(connection) return connection }) - const started = h.adapter.acquire({ ...h.input, fence: 8, signal: controller.signal }) + const acquired = h.adapter.acquire({ ...h.input, fence: 8 }) const child = await opened.promise await Promise.resolve() - controller.abort(new Error('Pi closed while starting')) - await expect(started).rejects.toThrow('closed while starting') + await expect(acquired).resolves.toMatchObject({ process: { pid: 4123 } }) + h.lifecycle.mockClear() + expect(h.adapter.readCommands(sessionId)).toBeUndefined() + expect(h.adapter.holdsDispatch(sessionId)).toBe(false) + await h.adapter.closeSession(sessionId) + await h.adapter.drainObservedExits() + expect(h.lifecycle).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'ended', + startupUnproven: true, + startupUnanswered: true, + cause: 'requested-close' + }) + ) + expect(h.lifecycle).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'started', fence: 8 }) + ) expect(child.rootVerdict).toBe('exited') }) @@ -278,7 +125,7 @@ describe('Pi RPC session ownership and delivery', () => { await h.adapter.closeSession(sessionId) h.adapter.acknowledgeSessionRelease(sessionId) await expect(h.adapter.acquire({ ...h.input, fence: 8 })).resolves.toMatchObject({ - link: { origin: 'created' } + process: { pid: 4123 } }) let drained = false const delivery = h.adapter.drainObservedExits().then(() => { @@ -310,12 +157,13 @@ describe('Pi RPC session ownership and delivery', () => { spawnToken: 'spawn-token' }) expect(h.acquired.process).toEqual(h.onSpawned.mock.calls[0]?.[0]) - expect(h.acquired.link).toMatchObject({ + expect(h.acquired.link).toBeUndefined() + expect(h.started.link).toMatchObject({ handle: { transport: 'jsonl-rpc', agent: 'pi', nativeId: file }, origin: 'created', mintedAtFence: 7 }) - expect(h.adapter.readOptionRestoreFailures(sessionId)).toEqual(['effort', 'unknown']) + expect(h.started.restoreSkippedOptions).toEqual(['unknown', 'effort']) expect(h.connection.requests).toContain('get_commands') }) @@ -587,6 +435,10 @@ describe('Pi RPC start without a model', () => { return connection }) await h.adapter.acquire({ ...h.input, fence: 8 }) + // Known by the time the start is proven: the listing is part of Pi's start. + await vi.waitFor(() => + expect(h.lifecycle).toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) + ) expect(h.adapter.startUnavailable(sessionId)).toEqual({ reason: 'notSignedIn' }) // Its root gone while its output still drains: that child says nothing more. const child = h.connections.at(-1)! diff --git a/src/main/pi/rpc-session-adapter.ts b/src/main/pi/rpc-session-adapter.ts index 139268fe0a78..181cd802d7ce 100644 --- a/src/main/pi/rpc-session-adapter.ts +++ b/src/main/pi/rpc-session-adapter.ts @@ -16,7 +16,7 @@ import { providerSpawnedProcessIdentity } from '../provider-process/provider-spa import { ProviderAcquisitionStarts } from '../provider-process/provider-acquisition-starts' import { compactPiRpcSession } from './rpc-compaction' import { buildPiRpcLaunch } from './rpc-launch' -import { piRpcProviderLink, type PiRpcResolvedLaunch } from './rpc-launch-resolution' +import type { PiRpcResolvedLaunch } from './rpc-launch-resolution' import { PiRpcSession, type PiRpcSessionDeps, type PiRpcConnection } from './rpc-session' import { PiRpcPromptError, preparePiRpcPrompt } from './rpc-prompt' import { applyPiRpcSessionOption } from './rpc-options' @@ -93,15 +93,15 @@ export class PiRpcSessionAdapter implements StructuredAgentSessionAdapter { await spawned.onSpawned(session.connection.pid) } const process = await spawned.read(session.connection.pid) - const file = await waitForPromiseWithSignal(session.start(), attempt.signal) if (session.connection.closed || attempt.signal.aborted) { throw new Error('Pi exited while starting') } + const child = session + void child.start(launch, () => this.starts.retainFailed(id, child.connection)) return { process, acquisitionGeneration: session.generation, - link: piRpcProviderLink(launch, file, input.fence, randomUUID(), Date.now()), - ...(session.startListing ? { catalogListing: session.startListing } : {}) + providerChildPhase: 'starting' } } catch (error) { if (!session) { @@ -227,10 +227,10 @@ export class PiRpcSessionAdapter implements StructuredAgentSessionAdapter { } readOptions: NonNullable = async (input) => this.session(input.sessionId, input.fence).readOptions() - readCommands = (id: string) => this.sessions.get(id)?.commands readOptionRestoreFailures(id: string): readonly string[] { - return this.sessions.get(id)?.skipped ?? [] + return this.sessions.get(id)?.startup.skipped ?? [] } + readCommands = (id: string) => this.sessions.get(id)?.startup.commands holdsDispatch(id: string): boolean { return this.sessions.get(id)?.turns.holdsDispatch ?? false } @@ -241,10 +241,14 @@ export class PiRpcSessionAdapter implements StructuredAgentSessionAdapter { return session && !session.connection.closed && session.connection.rootVerdict !== 'exited' && - session.options?.models.length === 0 + session.startup.options?.models.length === 0 ? { reason: 'notSignedIn' } : undefined } + holdsLiveProviderProcess(id: string, generation: string): boolean { + const session = this.sessions.get(id) + return session?.generation === generation && !session.connection.closed + } async closeSession(id: string, requested = true): Promise { const session = this.sessions.get(id) diff --git a/src/main/pi/rpc-session-startup.test.ts b/src/main/pi/rpc-session-startup.test.ts new file mode 100644 index 000000000000..e463ed9cb621 --- /dev/null +++ b/src/main/pi/rpc-session-startup.test.ts @@ -0,0 +1,248 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { providerDiagnostic, withProviderDiagnostic } from '../../shared/agent-session-failure' +import { AgentSessionAcquisitionExitUnprovenError } from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import { JsonlRpcResponseError } from '../jsonl-rpc/peer' +import { + closePiRpcAdapterFixtures, + file, + restart, + reportedEvent, + sessionId, + setup, + state +} from './rpc-session-adapter.test-fixture' + +afterEach(closePiRpcAdapterFixtures) + +describe('Pi background startup', () => { + it('publishes at spawn, restores model before effort, lists models in its start and reads commands after', async () => { + const h = await setup() + const handshake = Promise.withResolvers() + const listing = Promise.withResolvers() + const commands = Promise.withResolvers() + let revision = 3 + const { acquired, child } = await restart( + h, + (connection) => { + connection.requestOverride = (command) => { + if ( + command === 'get_state' && + !h.lifecycle.mock.calls.some(([event]) => event.type === 'started') + ) { + // The listing's own state read sees what the restore set. + return connection.requests.includes('get_available_models') + ? Promise.resolve({ + ...state, + model: { provider: 'anthropic', id: 'model-2' }, + thinkingLevel: 'high' + }) + : handshake.promise + } + if (command === 'get_available_models') { + return listing.promise + } + if (command === 'get_commands') { + return commands.promise + } + return command === 'set_thinking_level' ? Promise.resolve({}) : undefined + } + }, + { options: { effort: 'high', model: 'anthropic/model-2' }, optionRevision: () => revision } + ) + expect(acquired.link).toBeUndefined() + expect(child.requests).toEqual(['get_state']) + expect(h.lifecycle).not.toHaveBeenCalled() + if (!acquired.acquisitionGeneration) { + throw new Error('acquisition generation missing') + } + expect(h.adapter.holdsLiveProviderProcess(sessionId, acquired.acquisitionGeneration)).toBe(true) + revision = 5 + handshake.resolve(state) + await vi.waitFor(() => expect(child.requests).toContain('get_available_models')) + expect(child.requests.slice(0, 4)).toEqual([ + 'get_state', + 'set_model', + 'set_thinking_level', + 'get_available_models' + ]) + // The listing is part of the start: no `started` before it answers. + expect(h.lifecycle).not.toHaveBeenCalled() + listing.resolve({ models: [{ provider: 'anthropic', id: 'model-2', reasoning: true }] }) + await vi.waitFor(() => + expect(h.lifecycle).toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) + ) + expect(h.lifecycle.mock.calls[0]?.[0]).toMatchObject({ + type: 'started', + fence: 8, + acquisitionGeneration: acquired.acquisitionGeneration, + link: { handle: { nativeId: file }, origin: 'created' }, + reportedOptions: { + model: 'anthropic/model-2', + effort: 'high', + confirmed: ['model', 'effort'] + }, + restoreSkippedOptions: [], + optionRevision: 3 + }) + // Commands never hold the start. + expect(h.adapter.readCommands(sessionId)).toBeUndefined() + commands.resolve({ commands: [{ name: 'help' }] }) + await vi.waitFor(() => expect(h.adapter.readCommands(sessionId)).toBeDefined()) + expect(h.lifecycle).toHaveBeenCalledTimes(1) + }) + + it.each(['closeSession', 'disposeSession', 'forceCloseSession', 'releaseAcquisition'] as const)( + '%s stops a held handshake without a late started event', + async (method) => { + const h = await setup() + const handshake = Promise.withResolvers() + const { child } = await restart(h, (connection) => { + connection.requestOverride = (command) => + command === 'get_state' ? handshake.promise : undefined + }) + await (method === 'releaseAcquisition' + ? h.adapter.releaseAcquisition({ sessionId }) + : h.adapter[method](sessionId)) + handshake.resolve(state) + await h.adapter.drainObservedExits() + await Promise.resolve() + expect(child.rootVerdict).toBe('exited') + expect(h.lifecycle).toHaveBeenCalledWith( + expect.objectContaining({ type: 'ended', startupUnproven: true, startupUnanswered: true }) + ) + expect(h.lifecycle).not.toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) + } + ) + + it.each([ + ['get_state', new JsonlRpcResponseError('get_state', 'Pi is not signed in'), true], + ['set_model', new Error('Agent RPC request timed out: set_model'), false], + ['set_model', new Error('Agent JSON-lines RPC stream closed'), false] + ] as const)( + 'fails startup on %s failure %s and keeps its diagnostic', + async (command, error, unanswered) => { + const h = await setup() + const { child } = await restart( + h, + (connection) => { + connection.requestOverride = (name) => + name === command ? Promise.reject(error) : undefined + }, + { options: { model: 'anthropic/model-2' } } + ) + await vi.waitFor(() => + expect(h.lifecycle).toHaveBeenCalledWith(expect.objectContaining({ type: 'ended' })) + ) + expect(child.rootVerdict).toBe('exited') + const ended = reportedEvent(h.lifecycle, 'ended') + expect(ended).toMatchObject({ + cause: 'unexpected-exit', + startupUnproven: true, + reason: error.message + }) + expect(ended.startupUnanswered).toBe(unanswered ? true : undefined) + if (command === 'get_state') { + expect(ended.failure?.detail).toEqual({ text: 'Pi is not signed in', audience: 'person' }) + } + expect(h.lifecycle).not.toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) + } + ) + + it('does not retry acquisition while a failed start still owns an unproven process', async () => { + const h = await setup() + const { child } = await restart(h, (connection) => { + connection.closeResult = { root: 'unverifiable', tree: null } + connection.requestOverride = () => Promise.reject(new Error('get_state failed')) + }) + await vi.waitFor(() => expect(child.closed).toBe(true)) + await expect(h.adapter.acquire({ ...h.input, fence: 9 })).rejects.toBeInstanceOf( + AgentSessionAcquisitionExitUnprovenError + ) + expect(h.connections).toHaveLength(2) + child.closeResult = { root: 'exited', tree: 'exited' } + await expect(h.adapter.acquire({ ...h.input, fence: 10 })).resolves.toMatchObject({ + process: { pid: 4123 } + }) + }) + + it('preserves the provider exit diagnostic when transport closure failed startup first', async () => { + const h = await setup() + await restart(h, (connection) => { + connection.closeError = withProviderDiagnostic( + new Error('provider exited'), + providerDiagnostic('Pi is not signed in', 'log') + ) + connection.requestOverride = () => + Promise.reject(new Error('Agent JSON-lines RPC stream closed')) + }) + await vi.waitFor(() => + expect(h.lifecycle).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'ended', + startupUnproven: true, + failure: { + kind: 'providerStartFailed', + detail: { text: 'Pi is not signed in', audience: 'log' } + } + }) + ) + ) + }) + + it('ends a child that exits while its handshake is held', async () => { + const h = await setup() + const { child } = await restart(h, (connection) => { + connection.requestOverride = () => new Promise(() => {}) + }) + child.exit(new Error('provider died during get_state')) + await h.adapter.drainObservedExits() + expect(h.lifecycle).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'ended', + cause: 'unexpected-exit', + startupUnproven: true, + startupUnanswered: true, + reason: 'provider died during get_state' + }) + ) + expect(h.lifecycle).not.toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) + }) + + it('rejects an invalid session file and reports a proved resumed file from the launch', async () => { + const h = await setup() + const { child } = await restart(h, (connection) => { + connection.requestOverride = (name) => + name === 'get_state' + ? Promise.resolve({ ...state, sessionFile: 'relative.jsonl' }) + : undefined + }) + await vi.waitFor(() => + expect(h.lifecycle).toHaveBeenCalledWith( + expect.objectContaining({ type: 'ended', startupUnproven: true }) + ) + ) + expect(child.rootVerdict).toBe('exited') + const launch = await h.resolveLaunch.mock.results[0]?.value + if (!launch) { + throw new Error('launch missing') + } + h.resolveLaunch.mockResolvedValueOnce({ + ...launch, + sessionFile: file, + previous: h.started.link + }) + await restart(h, () => {}) + await vi.waitFor(() => + expect(h.lifecycle).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'started', + link: expect.objectContaining({ origin: 'resumed' }) + }) + ) + ) + expect(h.openConnection.mock.calls.at(-1)?.[0].args).toEqual( + expect.arrayContaining(['--session', file]) + ) + expect(reportedEvent(h.lifecycle, 'started').link?.handle.nativeId).toBe(file) + }) +}) diff --git a/src/main/pi/rpc-session-startup.ts b/src/main/pi/rpc-session-startup.ts new file mode 100644 index 000000000000..d24f5d5691ae --- /dev/null +++ b/src/main/pi/rpc-session-startup.ts @@ -0,0 +1,174 @@ +import { randomUUID } from 'node:crypto' +import { waitForPromiseWithSignal } from '../../shared/abort-signal-reason' +import type { + AgentSessionOptionsResult, + AgentSessionSlashCommand +} from '../../shared/agent-session-wire' +import { JsonlRpcResponseError } from '../jsonl-rpc/peer' +import type { StructuredAgentSessionAcquireInput } from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import { + unpickedSessionConfiguredChoice, + withLiveCatalogListing, + type AgentModelCatalogLiveListing +} from '../native-chat/agent-model-catalog/agent-model-catalog-entry' +import { piRpcProviderLink, type PiRpcResolvedLaunch } from './rpc-launch-resolution' +import { + applyPiRpcSessionOption, + piModelOptionId, + readPiRpcCommands, + readPiRpcSessionOptions +} from './rpc-options' +import { piRpcStateSchema, type PiRpcState } from './rpc-protocol' +import type { PiRpcConnection, PiRpcSessionDeps } from './rpc-session' +import { STRUCTURED_AGENT_SESSION_STARTUP_CEILING_MS } from '../native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract' + +/** What the start cannot go on without waits as long as the host's startup attempt does, its only + * deadline; the optional reads keep their own bound and never fail the start. */ +const START_REQUEST = { timeoutMs: STRUCTURED_AGENT_SESSION_STARTUP_CEILING_MS } + +export class PiRpcSessionStartup { + started = false + answered = false + skipped: readonly string[] = [] + commands?: AgentSessionSlashCommand[] + /** What the start listed; a signed-out Pi lists no model. */ + options?: AgentSessionOptionsResult + private readonly controller = new AbortController() + + constructor( + private readonly input: StructuredAgentSessionAcquireInput, + private readonly generation: string, + private readonly connection: PiRpcConnection, + private readonly selected: Map, + private readonly deps: PiRpcSessionDeps, + /** A new Pi session starts on its config's model; a resumed or forked one keeps its own. */ + private readonly resolvesConfig: boolean, + private readonly setState: (state: PiRpcState) => void + ) {} + + stop(): void { + this.controller.abort(new Error('Pi closed while starting')) + } + + async run(launch: PiRpcResolvedLaunch): Promise { + const optionRevision = this.input.optionRevision?.() ?? 0 + const answer = await this.wait(this.connection.request('get_state', {}, START_REQUEST)) + this.answered = true + this.assertLive() + const state = piRpcStateSchema.parse(answer) + const link = piRpcProviderLink( + launch, + state.sessionFile, + this.input.fence, + randomUUID(), + Date.now() + ) + this.setState(state) + const current: AgentSessionOptionsResult['current'] = { + model: state.model ? piModelOptionId(state.model.provider, state.model.id) : '', + ...(state.thinkingLevel ? { effort: state.thinkingLevel } : {}) + } + const saved = this.input.options ?? {} + const skipped = Object.keys(saved).filter((key) => key !== 'model' && key !== 'effort') + // Keys a restore sent, whether or not Pi took them. + const picked = new Set() + for (const key of ['model', 'effort'] as const) { + const value = saved[key] + if (value === undefined) { + continue + } + picked.add(key) + try { + this.assertLive() + await this.wait( + applyPiRpcSessionOption(this.connection, this.selected, key, value, START_REQUEST) + ) + current[key] = value + } catch (error) { + if (!(error instanceof JsonlRpcResponseError)) { + throw error + } + skipped.push(key) + this.warn('Pi rejected a saved option', 'pi-option-restore', error, { key }) + } + } + this.assertLive() + current.confirmed = [...(current.model ? ['model'] : []), ...(current.effort ? ['effort'] : [])] + // Part of the start, as other agents' handshakes answer with theirs: a Pi that lists no model + // is signed out, and what it started on names its config's default. + const options = await this.readListing() + this.assertLive() + // What it started on names its config's default once; a later switch teaches nothing. + let catalogListing: AgentModelCatalogLiveListing | undefined + if (options) { + this.options = options + catalogListing = withLiveCatalogListing( + options, + unpickedSessionConfiguredChoice({ resolvesConfig: this.resolvesConfig, picked, ...options }) + ).catalogListing + } + this.skipped = skipped + this.started = true + this.deps.onLifecycle({ + type: 'started', + ...this.identity(), + link, + reportedOptions: options?.current ?? current, + restoreSkippedOptions: skipped, + optionRevision, + ...(catalogListing ? { catalogListing } : {}) + }) + await this.readCommands() + } + + /** Never fails the start: without it the chat only lacks the listing and the sign-in verdict. */ + private async readListing(): Promise { + try { + return await this.wait(readPiRpcSessionOptions(this.connection)) + } catch (error) { + this.assertLive() + this.warn('Pi options could not be read', 'pi-options', error) + return undefined + } + } + + private async readCommands(): Promise { + try { + this.assertLive() + const result = await this.wait(readPiRpcCommands(this.connection)) + this.assertLive() + this.commands = result.commands + } catch (error) { + this.warn('Pi commands could not be read', 'pi-commands', error) + } + } + + private assertLive(): void { + if (this.controller.signal.aborted || this.connection.closed) { + throw new Error('Pi closed while starting') + } + } + + private wait(promise: Promise): Promise { + return waitForPromiseWithSignal(promise, this.controller.signal) + } + + private identity() { + return { + sessionId: this.input.identity.sessionId, + fence: this.input.fence, + acquisitionGeneration: this.generation + } + } + + private warn(message: string, scope: string, error: unknown, fields = {}): void { + if (!this.controller.signal.aborted) { + this.deps.logger.warn(message, { + scope, + sessionId: this.input.identity.sessionId, + error, + ...fields + }) + } + } +} diff --git a/src/main/pi/rpc-session.ts b/src/main/pi/rpc-session.ts index ae693653df4c..925a1bbf727e 100644 --- a/src/main/pi/rpc-session.ts +++ b/src/main/pi/rpc-session.ts @@ -1,7 +1,8 @@ -import type { - AgentSessionOptionsResult, - AgentSessionSlashCommand -} from '../../shared/agent-session-wire' +import { + agentSessionFailureFact, + providerDiagnostic, + providerDiagnosticOf +} from '../../shared/agent-session-failure' import type { StructuredAgentSessionAcquireInput, StructuredAgentSessionLifecycleEvent, @@ -15,15 +16,13 @@ import { type JsonlRpcAgentConnectionOptions } from '../jsonl-rpc/agent-connection' import type { ProviderProcessLaunch } from '../provider-process/provider-process-launch' +import { JsonlRpcResponseError } from '../jsonl-rpc/peer' import { PiRpcTurns } from './rpc-turns' import { PiRpcDialogCallbacks } from './rpc-dialog-callbacks' -import { piRpcStateSchema } from './rpc-protocol' -import { applyPiRpcSessionOption, readPiRpcCommands, readPiRpcSessionOptions } from './rpc-options' -import { - unpickedSessionConfiguredChoice, - withLiveCatalogListing, - type AgentModelCatalogLiveListing -} from '../native-chat/agent-model-catalog/agent-model-catalog-entry' +import { readPiRpcSessionOptions } from './rpc-options' +import { withLiveCatalogListing } from '../native-chat/agent-model-catalog/agent-model-catalog-entry' +import type { PiRpcResolvedLaunch } from './rpc-launch-resolution' +import { PiRpcSessionStartup } from './rpc-session-startup' export type PiRpcConnection = Pick< JsonlRpcAgentConnection, @@ -62,10 +61,7 @@ export class PiRpcSession { readonly turns: PiRpcTurns readonly dialogs: PiRpcDialogCallbacks readonly selected = new Map() - private startCatalogListing?: AgentModelCatalogLiveListing - readonly skipped: string[] = [] - commands?: AgentSessionSlashCommand[] - options?: AgentSessionOptionsResult + readonly startup: PiRpcSessionStartup requestedClose = false private publishedExit = false private failedCause?: Error @@ -145,55 +141,31 @@ export class PiRpcSession { .finally(() => this.releaseAfterExit?.()) } }) + this.startup = new PiRpcSessionStartup( + input, + generation, + this.connection, + this.selected, + deps, + resolvesConfig, + (state) => { + this.lane.apply(this.turns.context.setModel(state.model ?? undefined, Date.now())) + } + ) } - async start(): Promise { - const state = piRpcStateSchema.parse(await this.connection.request('get_state')) - this.lane.apply(this.turns.context.setModel(state.model ?? undefined, Date.now())) - // Keys a restore sent, whether or not Pi took them. - const picked = new Set() - for (const [key, value] of Object.entries(this.input.options ?? {})) { - if (!['model', 'effort'].includes(key)) { - this.skipped.push(key) - continue - } - picked.add(key) - try { - await applyPiRpcSessionOption(this.connection, this.selected, key, value) - } catch (error) { - this.skipped.push(key) - this.deps.logger.warn('Pi rejected a saved option', { - scope: 'pi-option-restore', - sessionId: this.input.identity.sessionId, - key, - error - }) - } - } - this.options = await readPiRpcSessionOptions(this.connection) - this.startCatalogListing = withLiveCatalogListing( - this.options, - unpickedSessionConfiguredChoice({ - resolvesConfig: this.resolvesConfig, - picked, - ...this.options - }) - ).catalogListing + async start(launch: PiRpcResolvedLaunch, retainFailed: () => void): Promise { try { - this.commands = (await readPiRpcCommands(this.connection)).commands + await this.startup.run(launch) } catch (error) { - this.deps.logger.warn('Pi commands could not be read', { - scope: 'pi-commands', - sessionId: this.input.identity.sessionId, - error - }) + if (!this.requestedClose) { + this.failedCause ??= error instanceof Error ? error : new Error(String(error)) + } + const result = await this.close(false).catch(() => null) + if (result?.root !== 'exited') { + retainFailed() + } } - return state.sessionFile - } - - /** What this child listed at its start, with what its config resolved; the host saves it once. */ - get startListing(): AgentModelCatalogLiveListing | undefined { - return this.startCatalogListing } /** What the child reports now; only the start says what its config resolved. */ @@ -203,6 +175,7 @@ export class PiRpcSession { async close(requested = true): ReturnType { this.requestedClose ||= requested + this.startup.stop() this.dialogs.cancelAll() return this.connection.close() } @@ -256,6 +229,8 @@ export class PiRpcSession { return } this.publishedExit = true + this.failedCause ??= error + this.startup?.stop() this.dialogs.cancelAll() this.turns.end() this.lane.finalize() @@ -281,6 +256,7 @@ export class PiRpcSession { } finally { clearTimeout(timer) } + const cause = this.failedCause ?? error this.deps.onLifecycle({ type: 'ended', sessionId: this.input.identity.sessionId, @@ -288,7 +264,20 @@ export class PiRpcSession { acquisitionGeneration: this.generation, observedAt: Date.now(), cause: this.requestedClose ? 'requested-close' : 'unexpected-exit', - reason: (this.failedCause ?? error).message + reason: cause.message, + failure: agentSessionFailureFact( + this.startup.started ? 'providerExited' : 'providerStartFailed', + { + detail: + providerDiagnosticOf(cause) ?? + providerDiagnosticOf(error) ?? + (cause instanceof JsonlRpcResponseError + ? providerDiagnostic(cause.message, 'person') + : undefined) + } + ), + ...(this.startup.started ? {} : { startupUnproven: true }), + ...(this.startup.answered ? {} : { startupUnanswered: true }) }) } diff --git a/src/main/runtime/structured-agent-session-pi-signed-out-start.test.ts b/src/main/runtime/structured-agent-session-pi-signed-out-start.test.ts new file mode 100644 index 000000000000..f35cd7b4657c --- /dev/null +++ b/src/main/runtime/structured-agent-session-pi-signed-out-start.test.ts @@ -0,0 +1,94 @@ +// A message that starts a signed-out Pi reaches it once and is settled by Pi's own refusal; the +// start made for it is never replaced as a signed-out child, which would start Pi for it forever. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, expect, it, vi } from 'vitest' +import { readWholeAgentSessionFailureFact } from '../../shared/agent-session-failure' +import { + hostTestMessage, + HOST_TEST_SESSION as SESSION +} from '../native-chat/agent-session-wire/structured-agent-session-host-test-data' +import { createTestParams } from '../native-chat/agent-session-wire/structured-agent-session-create-test-fixture' +import { createStructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' +import { piScriptedChild } from '../pi/pi-scripted-child.test-fixture' +import { + ensureStructuredAgentSessionHost, + stopStructuredAgentSessionRuntime +} from './structured-agent-session-runtime' + +let directory: string | undefined +let operations = 0 +const operationId = () => `${Date.now()}-${(++operations).toString(16).padStart(32, '0')}` +const caller = { callerKey: 'pi-signed-out-start-test' } + +afterEach(async () => { + await stopStructuredAgentSessionRuntime() + if (directory) { + await rm(directory, { recursive: true, force: true }) + directory = undefined + } +}) + +it('settles the first message of a signed-out Pi as not signed in, and starts Pi once', async () => { + directory = await mkdtemp(join(tmpdir(), 'orca-pi-signed-out-')) + const root = directory + const script = piScriptedChild({ signedOut: true }) + const host = await ensureStructuredAgentSessionHost({ + logger: createStructuredAgentSessionLogger(), + stateDirectory: root, + hostId: 'local', + claimKeyId: 'key-1', + resolveWorkspacePath: async () => root, + resolveClaudeAuthPolicy: () => ({ account: 'managed' }), + resolveLaunchArgs: () => [], + resolveEnvironment: async () => ({}), + ...script.deps + }) + const firstMessage = { + clientMessageId: operationId(), + body: hostTestMessage('hello, signed-out Pi') + } + // As a new chat sends its first message: the start is made for it. + const params = createTestParams(firstMessage, { + provider: 'pi', + agent: 'pi', + accountHome: { variable: 'PI_CODING_AGENT_DIR', path: root }, + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'folder-1', + workspaceKind: 'folder' + } + }) + params.envelope.clientOperationId = operationId() + const created = await host.create(caller, params, { firstMessage }) + if (!created.ok) { + throw new Error(JSON.stringify(created.refusal)) + } + // Pi answers its start after the host has published it, as a real Pi does. + await vi.waitFor(() => + expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('starting') + ) + await new Promise((resolve) => setTimeout(resolve, 50)) + script.releaseHandshake() + const submission = async () => { + await host.flushStreamedEvents(SESSION) + return (await host.journalSnapshot(SESSION)).submissions.find( + (entry) => entry.clientMessageId === firstMessage.clientMessageId + ) + } + + await vi.waitFor(async () => expect((await submission())?.dispatchState).toBe('rejected'), { + timeout: 5_000 + }) + expect(readWholeAgentSessionFailureFact((await submission())?.rejection)?.kind).toBe( + 'notSignedIn' + ) + // Nothing starts Pi again for a message it already refused. + await new Promise((resolve) => setTimeout(resolve, 500)) + expect(script.spawns()).toBe(1) + expect(script.closes()).toBe(0) + expect(new Set(script.prompts())).toEqual(new Set(['hello, signed-out Pi'])) +}) diff --git a/src/main/runtime/structured-agent-session-pi-startup.test.ts b/src/main/runtime/structured-agent-session-pi-startup.test.ts new file mode 100644 index 000000000000..19403cfa5aa8 --- /dev/null +++ b/src/main/runtime/structured-agent-session-pi-startup.test.ts @@ -0,0 +1,188 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, expect, it, vi } from 'vitest' +import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' +import { + hostTestAttachParams, + hostTestMessage, + HOST_TEST_SESSION +} from '../native-chat/agent-session-wire/structured-agent-session-host-test-data' +import { createStructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' +import { piScriptedChild } from '../pi/pi-scripted-child.test-fixture' +import { peekOpenedAgentSessionRecordStore } from './agent-session-record-store-slot' +import { + ensureStructuredAgentSessionHost, + stopStructuredAgentSessionRuntime +} from './structured-agent-session-runtime' + +let directory: string | undefined +let operations = 0 +const operationId = () => `${Date.now()}-${(++operations).toString(16).padStart(32, '0')}` + +afterEach(async () => { + await stopStructuredAgentSessionRuntime() + if (directory) { + await rm(directory, { recursive: true, force: true }) + directory = undefined + } +}) + +async function setup() { + directory = await mkdtemp(join(tmpdir(), 'orca-pi-startup-')) + const root = directory + const script = piScriptedChild() + const host = await ensureStructuredAgentSessionHost({ + logger: createStructuredAgentSessionLogger(), + stateDirectory: root, + hostId: 'local', + claimKeyId: 'key-1', + resolveWorkspacePath: async () => root, + resolveClaudeAuthPolicy: () => ({ account: 'managed' }), + resolveLaunchArgs: () => [], + resolveEnvironment: async () => ({}), + ...script.deps + }) + const caller = { callerKey: 'pi-startup-test' } + const params = hostTestAttachParams(null, { + provider: 'pi', + agent: 'pi', + providerHandle: undefined, + options: { unknown: 'unavailable-saved-option' }, + accountHome: { variable: 'PI_CODING_AGENT_DIR', path: root }, + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'folder-1', + workspaceKind: 'folder' + } + }) + params.envelope.clientOperationId = operationId() + const attached = await host.attach(caller, params) + expect(attached.ok).toBe(true) + if (!attached.ok) { + throw new Error(JSON.stringify(attached.refusal)) + } + return { script, host, caller, fence: attached.value.fence } +} + +function envelope(method: string, fields: Record) { + return { + sessionId: HOST_TEST_SESSION, + clientOperationId: operationId(), + expectedRuntimeFence: + peekOpenedAgentSessionRecordStore()?.getRecord(HOST_TEST_SESSION)?.lease.runtimeFence ?? null, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method, + sessionId: HOST_TEST_SESSION, + fields + }) + } +} + +it('attaches Pi at spawn and holds the opening send until the provider answers its handshake', async () => { + const { script, host, caller, fence } = await setup() + expect(script.spawns()).toBe(1) + const body = hostTestMessage('hello while Pi is starting') + const sent = await host.send(caller, { + envelope: { + sessionId: HOST_TEST_SESSION, + clientOperationId: operationId(), + expectedRuntimeFence: fence, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.send', + sessionId: HOST_TEST_SESSION, + fields: { body } + }) + }, + body + }) + expect(sent.ok).toBe(true) + expect(script.prompts()).toEqual([]) + script.releaseHandshake() + await vi.waitFor(() => expect(script.prompts()).toEqual(['hello while Pi is starting'])) + await vi.waitFor(() => + expect(peekOpenedAgentSessionRecordStore()?.getRecord(HOST_TEST_SESSION)?.options).toEqual({ + model: 'scripted/model-1', + effort: 'medium' + }) + ) + await stopStructuredAgentSessionRuntime() + expect(script.closes()).toBe(1) +}) + +it('keeps a model picked during startup when the optional state read predates applying the pick', async () => { + const { script, host, caller, fence } = await setup() + const fields = { key: 'model', value: 'scripted/model-2' } + const picked = await host.setOption(caller, { + envelope: { + sessionId: HOST_TEST_SESSION, + clientOperationId: operationId(), + expectedRuntimeFence: fence, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.setOption', + sessionId: HOST_TEST_SESSION, + fields + }) + }, + ...fields + }) + expect(picked.ok).toBe(true) + const body = hostTestMessage('first message') + const sent = await host.send(caller, { + envelope: envelope('agentSession.send', { body }), + body + }) + expect(sent.ok).toBe(true) + script.releaseHandshake() + // The pick reaches Pi before the message held for its start. + await vi.waitFor(() => expect(script.prompts()).toEqual(['first message'])) + const wire = script.wire() + expect(wire.indexOf('set_model')).toBeGreaterThan(-1) + expect(wire.indexOf('set_model')).toBeLessThan(wire.indexOf('prompt:first message')) + const { runtimeState, sessions, serialize } = host.collaboratorsForTests() + await vi.waitFor(() => expect(sessions.get(HOST_TEST_SESSION)?.child?.phase).toBe('ready')) + await vi.waitFor(() => + expect(runtimeState.optionRevisions.current(HOST_TEST_SESSION)).toBeGreaterThan(1) + ) + await serialize(HOST_TEST_SESSION, async () => {}) + expect(peekOpenedAgentSessionRecordStore()?.getRecord(HOST_TEST_SESSION)?.options?.model).toBe( + fields.value + ) +}) + +it('ends Pi on Stop and resumes its real session file when the next send starts a child', async () => { + const { script, host, caller } = await setup() + script.completeTurns = false + script.releaseHandshake() + const first = hostTestMessage('start a long turn') + expect( + ( + await host.send(caller, { + envelope: envelope('agentSession.send', { body: first }), + body: first + }) + ).ok + ).toBe(true) + await vi.waitFor(() => expect(script.prompts()).toEqual(['start a long turn'])) + expect((await host.cancel(caller, { envelope: envelope('agentSession.cancel', {}) })).ok).toBe( + true + ) + await vi.waitFor(() => expect(script.closes()).toBe(1)) + const second = hostTestMessage('resume after Stop') + expect( + ( + await host.send(caller, { + envelope: envelope('agentSession.send', { body: second }), + body: second + }) + ).ok + ).toBe(true) + await vi.waitFor(() => expect(script.spawns()).toBe(2)) + expect(script.resumes()).toBe(1) + expect(script.prompts()).toEqual(['start a long turn']) + script.releaseHandshake() + await vi.waitFor(() => + expect(script.prompts()).toEqual(['start a long turn', 'resume after Stop']) + ) +}) diff --git a/src/main/runtime/structured-agent-session-provider-operations-while-starting.test.ts b/src/main/runtime/structured-agent-session-provider-operations-while-starting.test.ts new file mode 100644 index 000000000000..66c7fd735e72 --- /dev/null +++ b/src/main/runtime/structured-agent-session-provider-operations-while-starting.test.ts @@ -0,0 +1,323 @@ +// An operation only the agent's protocol session can perform (a rewind, a goal, the recovery of a +// rewind left in doubt) never reaches a child still proving its start, for any agent. Driven through +// the real Codex and ACP adapters with only the provider child scripted. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, expect, it, vi } from 'vitest' +import { agentJournalSubmissionKey } from '../../shared/agent-session-journal-item-key' +import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' +import type { AgentSessionMutationEnvelope } from '../../shared/agent-session-wire' +import { ACP_LAUNCH_SPECS } from '../acp/acp-launch-specs' +import { acpScriptedChild } from '../acp/acp-scripted-child.test-fixture' +import { acpStructuredAgentDefinition } from '../acp/acp-structured-agent-definitions' +import { CODEX_STRUCTURED_AGENT } from '../codex/codex-structured-agent-definition' +import type { StructuredAgentDefinition } from '../native-chat/agent-session-wire/structured-agent-definition' +import { + CODEX_SCRIPTED_THREAD, + codexScriptedChild +} from '../codex/codex-scripted-child.test-fixture' +import type { StructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-host' +import { + HOST_TEST_SESSION as SESSION, + hostTestAttachParams, + hostTestMessage +} from '../native-chat/agent-session-wire/structured-agent-session-host-test-data' +import { createStructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' +import type { ScriptedAgentChild } from './structured-agent-scripted-child.test-fixture' +import { + ensureStructuredAgentSessionHost, + stopStructuredAgentSessionRuntime +} from './structured-agent-session-runtime' + +const CALLER = { callerKey: 'provider-operations-test' } +let root: string | undefined +let operations = 0 +/** The attach that opened the chat; a reconnecting view replays it. */ +let openedWith: Parameters[1] | undefined + +afterEach(async () => { + await stopStructuredAgentSessionRuntime() + if (root) { + await rm(root, { recursive: true, force: true }) + root = undefined + } +}) + +const operationId = (): string => `${Date.now()}-${(++operations).toString(16).padStart(32, '0')}` + +const eventually = (assertion: () => void | Promise) => + vi.waitFor(assertion, { timeout: 10_000 }) + +function envelope( + host: StructuredAgentSessionHost, + method: string, + fields: Record +): AgentSessionMutationEnvelope { + return { + sessionId: SESSION, + clientOperationId: operationId(), + expectedRuntimeFence: host.deps.store.getRecord(SESSION)?.lease.runtimeFence ?? 0, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method, + sessionId: SESSION, + fields + }) + } +} + +async function openChat( + child: ScriptedAgentChild, + definition: StructuredAgentDefinition = CODEX_STRUCTURED_AGENT +): Promise { + const { agent } = definition + root = await mkdtemp(join(tmpdir(), `orca-provider-operations-${agent}-`)) + const directory = root + const host = await ensureStructuredAgentSessionHost({ + logger: createStructuredAgentSessionLogger(), + stateDirectory: directory, + hostId: 'local', + claimKeyId: 'key-1', + resolveWorkspacePath: async () => directory, + resolveLaunchArgs: () => [], + resolveClaudeAuthPolicy: () => ({ account: 'managed' }), + ...child.deps + }) + const params = hostTestAttachParams(null, { + provider: agent, + agent, + accountHome: + definition.accountLocatorKind === 'opencode' + ? { kind: 'opencode', locator: { kind: 'unmanaged' } } + : { variable: definition.accountHomeVariable, path: join(directory, agent) }, + providerHandle: undefined + }) + // The runtime admits operation ids against its own clock. + params.envelope.clientOperationId = operationId() + const attached = await host.attach(CALLER, params) + expect(attached, JSON.stringify(attached)).toMatchObject({ ok: true }) + openedWith = params + return host +} + +async function send(host: StructuredAgentSessionHost, text: string) { + const body = hostTestMessage(text) + return host.send(CALLER, { envelope: envelope(host, 'agentSession.send', { body }), body }) +} + +/** A first turn the scripted Codex recorded, then the chat put to rest with a turn to target. */ +async function codexChatAtRestWithATurn(child: ReturnType) { + child.holdHandshakes = false + const host = await openChat(child) + child.releaseHandshake() + const first = await send(host, 'first turn') + if (!first.ok) { + throw new Error(JSON.stringify(first.refusal)) + } + const { clientMessageId } = first.value + let providerItemId: string | null = null + await eventually(async () => { + const submission = (await host.journalSnapshot(SESSION)).submissions.find( + (entry) => entry.clientMessageId === clientMessageId + ) + expect(submission?.providerItemId).toEqual(expect.any(String)) + providerItemId = submission?.providerItemId ?? null + }) + await host.close(SESSION, 'evict') + expect(child.closes()).toBe(1) + child.holdHandshakes = true + return { + host, + itemId: agentJournalSubmissionKey(clientMessageId), + providerItemId: providerItemId! + } +} + +/** Resolves to whether `pending` settled within a short wait. */ +async function settlesSoon(pending: Promise): Promise { + let settled = false + void pending.finally(() => (settled = true)) + await new Promise((resolve) => setTimeout(resolve, 50)) + return settled +} + +it('recovers a Codex rewind left in doubt once the restarted thread is open, then delivers the send that started it', async () => { + const child = codexScriptedChild({ + 'thread/read': () => ({ thread: { id: CODEX_SCRIPTED_THREAD, status: { type: 'idle' } } }), + // The rewound turn is gone and nothing earlier is retained: the provider applied the rewind. + 'thread/turns/list': () => ({ data: [], nextCursor: null }), + 'thread/items/list': () => ({ data: [], nextCursor: null }) + }) + const { host, itemId, providerItemId } = await codexChatAtRestWithATurn(child) + const { store } = host.deps + const expectedEpoch = (await host.journalSnapshot(SESSION)).cursor.epoch + const request = envelope(host, 'agentSession.rewind', { itemId, expectedEpoch }) + await store.admitMutationOperation({ + callerKey: CALLER.callerKey, + envelope: request, + hostFingerprint: request.payloadFingerprint, + now: Date.now() + }) + await store.transitionHandoff(SESSION, (record) => ({ + ...record, + rewind: { + operationId: request.clientOperationId, + callerKey: CALLER.callerKey, + itemId, + providerItemId, + expectedEpoch, + phase: 'prepared', + retained: [] + } + })) + + const sent = send(host, 'after the rewind') + await eventually(() => expect(child.spawns()).toBe(2)) + expect(await settlesSoon(sent)).toBe(false) + child.releaseHandshake() + + expect(await sent).toMatchObject({ ok: true }) + expect(store.getRecord(SESSION)?.rewind?.phase).toBe('completed') + await eventually(() => expect(child.prompts()).toEqual(['first turn', 'after the rewind'])) +}) + +it('settles a rewind that goes in doubt while a send waits in the queue, by waiting out the start the send needs', async () => { + const child = codexScriptedChild({ + 'thread/read': () => ({ thread: { id: CODEX_SCRIPTED_THREAD, status: { type: 'idle' } } }), + 'thread/turns/list': () => ({ data: [], nextCursor: null }), + 'thread/items/list': () => ({ data: [], nextCursor: null }) + }) + const { host, itemId, providerItemId } = await codexChatAtRestWithATurn(child) + const { store } = host.deps + const expectedEpoch = (await host.journalSnapshot(SESSION)).cursor.epoch + const queueHeld = Promise.withResolvers() + void host.collaboratorsForTests().serialize(SESSION, () => queueHeld.promise) + + // Nothing is in doubt when the send arrives; it waits in the queue. + const sent = send(host, 'after the rewind') + expect(await settlesSoon(sent)).toBe(false) + const request = envelope(host, 'agentSession.rewind', { itemId, expectedEpoch }) + await store.admitMutationOperation({ + callerKey: CALLER.callerKey, + envelope: request, + hostFingerprint: request.payloadFingerprint, + now: Date.now() + }) + await store.transitionHandoff(SESSION, (record) => ({ + ...record, + rewind: { + operationId: request.clientOperationId, + callerKey: CALLER.callerKey, + itemId, + providerItemId, + expectedEpoch, + phase: 'prepared', + retained: [] + } + })) + queueHeld.resolve() + + // The send starts the agent to settle the rewind, and waits that start out. + await eventually(() => expect(child.spawns()).toBe(2)) + expect(await settlesSoon(sent)).toBe(false) + child.releaseHandshake() + + expect(await sent).toMatchObject({ ok: true }) + expect(store.getRecord(SESSION)?.rewind?.phase).toBe('completed') + await eventually(() => expect(child.prompts()).toEqual(['first turn', 'after the rewind'])) +}) + +it('answers a rewind asked of a resting Codex chat from the thread it opens, leaving nothing in doubt', async () => { + const child = codexScriptedChild({ + 'thread/resume': (params) => ({ + thread: { id: params?.threadId, historyMode: 'legacy' }, + model: 'scripted-model' + }) + }) + const { host, itemId } = await codexChatAtRestWithATurn(child) + const expectedEpoch = (await host.journalSnapshot(SESSION)).cursor.epoch + + const rewound = host.rewind(CALLER, { + itemId, + expectedEpoch, + envelope: envelope(host, 'agentSession.rewind', { itemId, expectedEpoch }) + }) + await eventually(() => expect(child.spawns()).toBe(2)) + expect(await settlesSoon(rewound)).toBe(false) + child.releaseHandshake() + + // Only the opened thread says its history cannot be rewound. + expect(await rewound).toMatchObject({ + ok: false, + refusal: { rewindReason: 'history-not-paginated' } + }) + expect(host.deps.store.getRecord(SESSION)?.rewind).toBeUndefined() + expect(await send(host, 'still sends')).toMatchObject({ ok: true }) + await eventually(() => expect(child.prompts()).toEqual(['first turn', 'still sends'])) +}) + +it('sets a goal asked of a starting Codex chat on the thread once it is open', async () => { + const child = codexScriptedChild() + const host = await openChat(child) + const change = { kind: 'set' as const, objective: 'Ship the release' } + + const goal = host.changeThreadGoal(CALLER, { + envelope: envelope(host, 'agentSession.threadGoal', { change }), + change + }) + expect(await settlesSoon(goal)).toBe(false) + child.releaseHandshake() + + expect(await goal).toMatchObject({ ok: true }) + expect(child.requests().filter(({ method }) => method.startsWith('thread/goal'))).toEqual([ + { + method: 'thread/goal/set', + params: { threadId: CODEX_SCRIPTED_THREAD, objective: 'Ship the release', status: 'active' } + } + ]) +}) + +it('keeps a goal waiting on a starting Codex chat through a re-attach, then sets it once the thread is open', async () => { + const child = codexScriptedChild() + const host = await openChat(child) + const change = { kind: 'set' as const, objective: 'Ship the release' } + + const goal = host.changeThreadGoal(CALLER, { + envelope: envelope(host, 'agentSession.threadGoal', { change }), + change + }) + expect(await settlesSoon(goal)).toBe(false) + // A view reconnects while the chat is still starting: its replayed attach keeps the live child. + if (!openedWith) { + throw new Error('the chat was not opened') + } + expect(await host.attach(CALLER, openedWith)).toMatchObject({ ok: true, replayed: true }) + expect(child.spawns()).toBe(1) + expect(await settlesSoon(goal)).toBe(false) + child.releaseHandshake() + + expect(await goal).toMatchObject({ ok: true }) + expect(child.requests().some(({ method }) => method === 'thread/goal/set')).toBe(true) +}) + +it('answers a goal asked of an ACP agent, which has none, without waiting out its start', async () => { + const spec = ACP_LAUNCH_SPECS[0]! + const child = acpScriptedChild(spec.agent)() + const host = await openChat(child, acpStructuredAgentDefinition(spec)) + const change = { kind: 'set' as const, objective: 'Ship the release' } + + const goal = host.changeThreadGoal(CALLER, { + envelope: envelope(host, 'agentSession.threadGoal', { change }), + change + }) + expect(await settlesSoon(goal)).toBe(true) + expect(await goal).toMatchObject({ + ok: false, + refusal: { details: { reason: 'goalsUnsupported' } } + }) + child.releaseHandshake() + + expect(await send(host, 'after the goal')).toMatchObject({ ok: true }) + await eventually(() => expect(child.prompts()).toEqual(['after the goal'])) +}) diff --git a/src/main/runtime/structured-agent-session-transport-overrides.ts b/src/main/runtime/structured-agent-session-transport-overrides.ts index 550760f812c8..249f5057ebfb 100644 --- a/src/main/runtime/structured-agent-session-transport-overrides.ts +++ b/src/main/runtime/structured-agent-session-transport-overrides.ts @@ -4,10 +4,13 @@ import type { ClaudeStructuredSessionAdapterDeps } from '../claude/claude-struct import type { CodexStructuredSessionAdapterDeps } from '../codex/codex-structured-session-adapter' import type { StructuredAgentSessionStartupLimits } from '../native-chat/agent-session-wire/structured-agent-session-startup-attempt-contract' import type { PiRpcSessionDeps } from '../pi/rpc-session' +import type { PiRpcSessionAdapterDeps } from '../pi/rpc-session-adapter' /** Provider transports are overridden only to drive the runtime against scripted children. */ export type StructuredAgentSessionTransportOverrides = { openPiConnection?: PiRpcSessionDeps['openConnection'] + /** Scripted Pi children resolve their launch here, so no test reaches a real Pi binary. */ + resolvePiLaunch?: PiRpcSessionAdapterDeps['resolveLaunch'] openCodexConnection?: CodexStructuredSessionAdapterDeps['openConnection'] openAcpConnection?: AcpStructuredSessionAdapterDeps['connect'] /** Answers an ACP launch's command lookup and version probe for a scripted child; every launch From 39eb5270c5c9151a3d22c51301efebad30af8fa4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 12 Oct 2026 21:05:46 -0700 Subject: [PATCH 02/11] Await Pi open and fail signed-out construction --- .../structured-agent-session-host-delivery.ts | 3 +- ...tured-agent-session-outgrown-child.test.ts | 231 +++---------- ...structured-agent-session-outgrown-child.ts | 69 +--- src/main/pi/pi-scripted-child.test-fixture.ts | 11 +- .../pi/rpc-session-adapter.test-fixture.ts | 9 +- src/main/pi/rpc-session-adapter.test.ts | 37 ++- src/main/pi/rpc-session-adapter.ts | 36 +- src/main/pi/rpc-session-startup.test.ts | 308 ++++++------------ src/main/pi/rpc-session-startup.ts | 102 +++--- src/main/pi/rpc-session.ts | 48 ++- ...-agent-session-pi-signed-out-start.test.ts | 37 ++- 11 files changed, 313 insertions(+), 578 deletions(-) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts index 3f5e70fc7871..8ea0b209246e 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts @@ -83,7 +83,7 @@ export function createStructuredAgentSessionConversationDelivery(input: { const session = input.clientDelivery.readStopping(sessionId) ? undefined : sessions.get(sessionId) - await retireOutgrownStructuredAgentSessionChild(sessionId, session, startedFor, { + await retireOutgrownStructuredAgentSessionChild(sessionId, session, { work: { childWork: () => input.clientDelivery.readChildWork(sessionId), hasOpenDispatch: () => { @@ -92,7 +92,6 @@ export function createStructuredAgentSessionConversationDelivery(input: { }, providerHoldsDispatch: () => deps.adapter.holdsDispatch?.(sessionId) === true }, - startUnavailable: () => deps.adapter.startUnavailable?.(sessionId), launchPermission: async () => { const record = deps.store.getRecord(sessionId) return record ? structuredAgentSessionLaunchPermission(deps, record) : null diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.test.ts index df042438c2eb..8c8d4fdb960b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.test.ts @@ -1,23 +1,9 @@ -// A child the chat outgrew is replaced before the next send: one that reported it is not signed in, -// so a sign-in made since reaches a new child, and one launched with a permission mode the chat no -// longer has, so the new mode reaches a new child. +// An idle child launched under a different permission mode is replaced before the next send. -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { agentSessionFailureFact } from '../../../shared/agent-session-failure' import { agentSessionFailureWords } from '../../../shared/agent-session-failure-words' -import { - acceptedDispatch, - createRestTestRig, - foundRestTestChat, - REST_TEST_CALLER as CALLER, - REST_TEST_SESSION as SESSION, - restTestSend, - type RestTestRig -} from './structured-agent-session-rest-test-rig' -import { - retireOutgrownStructuredAgentSessionChild, - structuredAgentSessionChildReportedSignedOut -} from './structured-agent-session-outgrown-child' +import { retireOutgrownStructuredAgentSessionChild } from './structured-agent-session-outgrown-child' import { claudeAndCodexDeclared } from './structured-agent-session-adapter-router-test-support' import { structuredAgentSessionLaunchPermission } from './structured-agent-session-launch-permission' import type { AgentChatPermissionMode } from '../../../shared/agent-chat-permission-mode' @@ -28,109 +14,23 @@ import type { AgentJournalTurnLifecycle } from '../../../shared/agent-session-journal-types' import type { AgentChildWorkView } from '../../../shared/agent-status-child-work-view' -import type { AgentSessionUnavailable } from '../../../shared/agent-session-availability' -import type { JournalItemLinkageVisitor } from '../agent-session-journal/journal-store-contracts' import type { StructuredAgentSessionChildWorkReads } from './structured-agent-session-idle-sweep' -describe('a send after the agent said it is not signed in', () => { - let rig: RestTestRig - - beforeEach(async () => { - rig = await createRestTestRig() - }) - - afterEach(async () => { - await rig.dispose() - }) - - const fence = () => rig.store.getRecord(SESSION)?.lease.runtimeFence ?? 1 - - it('goes to a new agent, and the new agent is kept for the sends after it', async () => { - await foundRestTestChat(rig) - rig.adapter.dispatch.mockImplementationOnce(async () => ({ - state: 'rejected', - ...agentSessionFailureWords(agentSessionFailureFact('notSignedIn'), { - surface: 'rejection' - }) - })) - expect((await rig.host.send(CALLER, restTestSend('signed out', fence()))).ok).toBe(true) - await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(2)) - expect(rig.adapter.acquire).toHaveBeenCalledTimes(1) - rig.adapter.closeSession.mockClear() - - expect((await rig.host.send(CALLER, restTestSend('signed in now', fence()))).ok).toBe(true) - await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(3)) - expect(rig.adapter.closeSession).toHaveBeenCalledWith(SESSION) - expect(rig.adapter.acquire).toHaveBeenCalledTimes(2) - - // The earlier child's report does not follow the new one. - expect((await rig.host.send(CALLER, restTestSend('and again', fence()))).ok).toBe(true) - await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(4)) - expect(rig.adapter.acquire).toHaveBeenCalledTimes(2) - }) - - // A Pi that started with no model keeps a placeholder: a sign-in made since reaches a new Pi only. - it('goes to a new agent when the running one started signed out', async () => { - await foundRestTestChat(rig) - rig.adapter.startUnavailable.mockReturnValueOnce({ reason: 'notSignedIn' }) - rig.adapter.closeSession.mockClear() - expect((await rig.host.send(CALLER, restTestSend('signed in since', fence()))).ok).toBe(true) - await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(2)) - expect(rig.adapter.closeSession).toHaveBeenCalledWith(SESSION) - expect(rig.adapter.acquire).toHaveBeenCalledTimes(2) - }) - - it('keeps the agent after any other rejection', async () => { - await foundRestTestChat(rig) - rig.adapter.dispatch.mockImplementationOnce(async () => ({ - state: 'rejected', - ...agentSessionFailureWords(agentSessionFailureFact('providerRejected'), { - surface: 'rejection' - }) - })) - expect((await rig.host.send(CALLER, restTestSend('refused', fence()))).ok).toBe(true) - await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(2)) - rig.adapter.dispatch.mockImplementation(async () => acceptedDispatch()) - - expect((await rig.host.send(CALLER, restTestSend('next', fence()))).ok).toBe(true) - await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(3)) - expect(rig.adapter.acquire).toHaveBeenCalledTimes(1) - }) -}) - type Row = { itemId: string; fence: number; agentId?: string; body: AgentJournalItemBody } -const statusRow = ( - fence: number, - kind: 'notSignedIn' | 'providerExited', - agentId?: string -): Row => ({ - itemId: `status-${fence}-${kind}-${agentId ?? 'root'}`, - fence, - ...(agentId ? { agentId } : {}), - body: { - kind: 'status', - ...agentSessionFailureWords(agentSessionFailureFact(kind), { surface: 'row' }) - } -}) -const rejected = (fence: number) => ({ - fence, - dispatchState: 'rejected' as const, - rejection: agentSessionFailureFact('notSignedIn') -}) const childAt = ( overrides: Partial = {} ): StructuredAgentSessionProviderChild => ({ generation: 'generation-2', fence: 2, phase: 'ready', + permissionMode: 'ask', ...overrides }) const conversation = ( input: { child?: StructuredAgentSessionProviderChild | null items?: Row[] - submissions?: ReturnType[] activeTurnId?: string newestTurnEndedAt?: number } = {} @@ -139,14 +39,8 @@ const conversation = ( return { child: input.child === undefined ? childAt() : input.child, journal: { - submissions: () => input.submissions ?? [], visitItems: (visit: (itemId: string, sequence: number, body: AgentJournalItemBody) => void) => items.forEach((item, index) => visit(item.itemId, index, item.body)), - visitItemsWithLinkage: (visit: JournalItemLinkageVisitor) => - items.forEach((item, index) => - visit(item.itemId, index, item.body, item.agentId ? { agentId: item.agentId } : {}) - ), - itemFence: (itemId: string) => items.find((item) => item.itemId === itemId)?.fence, activeTurnId: () => input.activeTurnId ?? null, newestTurn: (): AgentJournalTurnLifecycle | null => input.newestTurnEndedAt === undefined @@ -156,41 +50,39 @@ const conversation = ( } } -describe('structuredAgentSessionChildReportedSignedOut', () => { - const reported = (input: Parameters[0]) => - structuredAgentSessionChildReportedSignedOut(conversation(input)) - - it('reads a not-signed-in row the running child wrote', () => { - expect(reported({ items: [statusRow(2, 'notSignedIn')] })).toBe(true) - }) - - it('ignores one an earlier child or a subagent wrote, and any other failure', () => { - expect(reported({ items: [statusRow(1, 'notSignedIn')] })).toBe(false) - expect(reported({ items: [statusRow(2, 'notSignedIn', 'subagent-1')] })).toBe(false) - expect(reported({ items: [statusRow(2, 'providerExited')] })).toBe(false) - }) - - it('reads a send rejected as not signed in at this child, not at an earlier one', () => { - expect(reported({ submissions: [rejected(2)] })).toBe(true) - expect(reported({ submissions: [rejected(1)] })).toBe(false) - }) - - it('leaves a child that is starting, closing, or absent', () => { - const items = [statusRow(2, 'notSignedIn')] - expect(reported({ items, child: childAt({ phase: 'starting' }) })).toBe(false) - expect(reported({ items, child: null })).toBe(false) - }) - - it("reads the running child's own start, and only a signed-out one", () => { - const fromStart = (startUnavailable: AgentSessionUnavailable, child = childAt({})) => - structuredAgentSessionChildReportedSignedOut(conversation({ child }), startUnavailable) - expect(fromStart({ reason: 'notSignedIn' })).toBe(true) - expect(fromStart({ reason: 'cliMissing' })).toBe(false) - expect(fromStart({ reason: 'notSignedIn' }, childAt({ phase: 'starting' }))).toBe(false) - }) -}) - describe('retireOutgrownStructuredAgentSessionChild', () => { + it('keeps a signed-out child until normal failure settlement closes it', async () => { + const stopAgent = vi.fn(async () => undefined) + await retireOutgrownStructuredAgentSessionChild( + 'session', + conversation({ + child: childAt({ permissionMode: undefined }), + items: [ + { + itemId: 'signed-out', + fence: 2, + body: { + kind: 'status', + ...agentSessionFailureWords(agentSessionFailureFact('notSignedIn'), { + surface: 'row' + }) + } + } + ] + }), + { + work: { + childWork: () => [], + hasOpenDispatch: () => false, + providerHoldsDispatch: () => false + }, + launchPermission: async () => 'full-access', + stopAgent, + logger: { warn: vi.fn(), error: vi.fn() } + } + ) + expect(stopAgent).not.toHaveBeenCalled() + }) const liveChildWork: AgentChildWorkView = { id: 'subagent-1', kind: 'agent', @@ -206,54 +98,30 @@ describe('retireOutgrownStructuredAgentSessionChild', () => { hasOpenDispatch: () => false, providerHoldsDispatch: () => false } - const signedOut = { submissions: [rejected(2)] } + const outgrown = { child: childAt({ permissionMode: 'ask' }) } const retire = async ( input: Parameters[0], work: Partial = {}, stopAgent = vi.fn(async () => undefined), - launchPermission?: () => Promise, - startedFor = 'next-send' + launchPermission: () => Promise = async () => 'full-access' ) => { const warn = vi.fn() - await retireOutgrownStructuredAgentSessionChild('session', conversation(input), startedFor, { + await retireOutgrownStructuredAgentSessionChild('session', conversation(input), { work: { ...idle, ...work }, - ...(launchPermission ? { launchPermission } : {}), + launchPermission, stopAgent, logger: { warn, error: vi.fn() } }) return { stopAgent, warn } } - it('stops a signed-out child that owes nothing', async () => { - expect((await retire(signedOut)).stopAgent).toHaveBeenCalledWith('session') - }) - - it('keeps a child started for the send it is about to take', async () => { - const own = await retire( - { ...signedOut, child: childAt({ startedFor: 'next-send' }) }, - {}, - undefined, - undefined, - 'next-send' - ) - expect(own.stopAgent).not.toHaveBeenCalled() - const earlier = await retire( - { ...signedOut, child: childAt({ startedFor: 'earlier-send' }) }, - {}, - undefined, - undefined, - 'next-send' - ) - expect(earlier.stopAgent).toHaveBeenCalledWith('session') - }) - it('keeps it while it owes work the idle sweep also protects', async () => { // A pending prompt is the shared check's too; the idle sweep's tests cover that case. for (const kept of [ - await retire({ ...signedOut, activeTurnId: 'turn-1' }), - await retire(signedOut, { childWork: () => [liveChildWork] }), - await retire(signedOut, { hasOpenDispatch: () => true }), - await retire(signedOut, { providerHoldsDispatch: () => true }) + await retire({ ...outgrown, activeTurnId: 'turn-1' }), + await retire(outgrown, { childWork: () => [liveChildWork] }), + await retire(outgrown, { hasOpenDispatch: () => true }), + await retire(outgrown, { providerHoldsDispatch: () => true }) ]) { expect(kept.stopAgent).not.toHaveBeenCalled() } @@ -268,12 +136,12 @@ describe('retireOutgrownStructuredAgentSessionChild', () => { settledAt: 200 } const after = await retire( - { ...signedOut, newestTurnEndedAt: 100 }, + { ...outgrown, newestTurnEndedAt: 100 }, { childWork: () => [settled] } ) expect(after.stopAgent).not.toHaveBeenCalled() const before = await retire( - { ...signedOut, newestTurnEndedAt: 300 }, + { ...outgrown, newestTurnEndedAt: 300 }, { childWork: () => [settled] } ) expect(before.stopAgent).toHaveBeenCalledWith('session') @@ -281,7 +149,7 @@ describe('retireOutgrownStructuredAgentSessionChild', () => { it('logs a stop that fails and lets the send go on', async () => { const { warn } = await retire( - signedOut, + outgrown, {}, vi.fn(async () => { throw new Error('exit not proven') @@ -400,7 +268,10 @@ describe('retireOutgrownStructuredAgentSessionChild', () => { it('never reads the mode for a child launched with none', async () => { const current = now('full-access') - expect((await retire({}, {}, undefined, current)).stopAgent).not.toHaveBeenCalled() + expect( + (await retire({ child: childAt({ permissionMode: undefined }) }, {}, undefined, current)) + .stopAgent + ).not.toHaveBeenCalled() expect(current).not.toHaveBeenCalled() }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.ts index 10e3fad53df3..3c0d7fff55e1 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.ts @@ -1,10 +1,5 @@ -// A child the chat outgrew (signed out, or launched with another permission mode) is replaced before -// the next send: agents read their login and permission mode only when they start. +// An idle child launched with another permission mode is replaced before the next send. -import type { AgentSessionUnavailable } from '../../../shared/agent-session-availability' -import { readWholeAgentSessionFailureFact } from '../../../shared/agent-session-failure' -import { isRootAgentJournalItem } from '../../../shared/agent-session-journal-producer' -import type { AgentJournalSubmission } from '../../../shared/agent-session-journal-types' import type { AgentChildWorkView } from '../../../shared/agent-status-child-work-view' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' @@ -17,53 +12,7 @@ import type { AgentChatPermissionMode } from '../../../shared/agent-chat-permiss /** What the check reads of a conversation; every read skips building a snapshot. */ type OutgrownChildReading = Pick & { - journal: Pick< - AgentSessionJournal, - 'visitItems' | 'visitItemsWithLinkage' | 'itemFence' | 'activeTurnId' | 'newestTurn' - > & { - submissions(): readonly Pick[] - } -} - -const signedOut = (failure: unknown): boolean => - readWholeAgentSessionFailureFact(failure)?.kind === 'notSignedIn' - -/** The running child itself said it is not signed in: its start, a send it rejected, or a row it - * wrote. Each child holds its own fence, so matching it scopes both to this child; a new one has - * nothing. `startUnavailable` is the adapter's read of that child's start. */ -export function structuredAgentSessionChildReportedSignedOut( - session: OutgrownChildReading, - startUnavailable?: AgentSessionUnavailable -): boolean { - const { child, journal } = session - if (!child || child.phase === 'starting' || child.close) { - return false - } - if (startUnavailable?.reason === 'notSignedIn') { - return true - } - if ( - journal - .submissions() - .some( - (submission) => - submission.dispatchState === 'rejected' && - submission.fence === child.fence && - signedOut(submission.rejection) - ) - ) { - return true - } - // The session's own agent only: a subagent's sign-in failure says nothing about its parent. - let found = false - journal.visitItemsWithLinkage((itemId, _sequence, body, attribution) => { - found ||= - body.kind === 'status' && - isRootAgentJournalItem(attribution) && - signedOut(body.failure) && - journal.itemFence(itemId) === child.fence - }) - return found + journal: Pick } /** Child work that settled after the lead's newest turn ended: the lead may still owe the turn @@ -91,18 +40,14 @@ async function launchedWithOtherPermissionMode( return mode !== null && mode !== child.permissionMode } -/** For a caller inside the session's serialize, before it hands the next send (`startedFor`) to - * the child. A child started for that same send keeps it: the send has not reached any child yet, - * and the agent's own refusal settles it once, where a new child would refuse it again forever. - * Work the child still owes keeps it, as it keeps an idle one from the sweep. A stop that fails +/** For a caller inside the session's serialize, before it hands the next send to + * the child. Work the child still owes keeps it, as it keeps an idle one from the sweep. A stop that fails * leaves the close begun, which the start joins. */ export async function retireOutgrownStructuredAgentSessionChild( sessionId: string, session: OutgrownChildReading | undefined, - startedFor: string, deps: { work: StructuredAgentSessionChildWorkReads - startUnavailable?: () => AgentSessionUnavailable | undefined /** The mode a start of the chat would launch with now. */ launchPermission?: () => Promise stopAgent: (sessionId: string) => Promise @@ -111,11 +56,7 @@ export async function retireOutgrownStructuredAgentSessionChild( ): Promise { if ( !session || - !( - (session.child?.startedFor !== startedFor && - structuredAgentSessionChildReportedSignedOut(session, deps.startUnavailable?.())) || - (await launchedWithOtherPermissionMode(session.child, deps.launchPermission)) - ) || + !(await launchedWithOtherPermissionMode(session.child, deps.launchPermission)) || structuredAgentSessionChildHasOpenWork(session.journal, deps.work) || owesWakeUp(session.journal, deps.work.childWork()) ) { diff --git a/src/main/pi/pi-scripted-child.test-fixture.ts b/src/main/pi/pi-scripted-child.test-fixture.ts index 635c30bc8a75..5c6c4d38dec1 100644 --- a/src/main/pi/pi-scripted-child.test-fixture.ts +++ b/src/main/pi/pi-scripted-child.test-fixture.ts @@ -29,7 +29,7 @@ class ScriptedPiConnection implements PiRpcConnection { private readonly wire: string[], private readonly onClosed: () => void, number: number, - /** A Pi with no login: it lists no model and fails every prompt as not signed in. */ + /** A Pi with no login lists no model. */ private readonly signedOut = false ) { this.pid = 42_000 + number @@ -195,6 +195,7 @@ class ScriptedPiConnection implements PiRpcConnection { export type ScriptedPiChild = ScriptedAgentChild & { /** Every request command and prompt any spawn received, in order. */ wire(): readonly string[] + signIn(): void } export const piScriptedChild = (options: { signedOut?: boolean } = {}): ScriptedPiChild => { @@ -203,6 +204,7 @@ export const piScriptedChild = (options: { signedOut?: boolean } = {}): Scripted const wire: string[] = [] let resumed = 0 let closed = 0 + let signedOut = options.signedOut ?? false const script: ScriptedPiChild = { holdHandshakes: true, completeTurns: true, @@ -242,7 +244,7 @@ export const piScriptedChild = (options: { signedOut?: boolean } = {}): Scripted closed += 1 }, children.length + 1, - options.signedOut + signedOut ) children.push(child) return child @@ -255,7 +257,10 @@ export const piScriptedChild = (options: { signedOut?: boolean } = {}): Scripted spawns: () => children.length, resumes: () => resumed, closes: () => closed, - wire: () => wire + wire: () => wire, + signIn: () => { + signedOut = false + } } return script } diff --git a/src/main/pi/rpc-session-adapter.test-fixture.ts b/src/main/pi/rpc-session-adapter.test-fixture.ts index 573d28d9a678..10810ec4abf0 100644 --- a/src/main/pi/rpc-session-adapter.test-fixture.ts +++ b/src/main/pi/rpc-session-adapter.test-fixture.ts @@ -168,9 +168,12 @@ export async function setup( logger: { warn: vi.fn(), error: vi.fn() } }) const acquired = await adapter.acquire(input) - // Commands are read after the start; waiting for them leaves nothing of it in flight. - await vi.waitFor(() => expect(adapter.readCommands(sessionId)).toBeDefined()) - const started = reportedEvent(lifecycle, 'started') + const started = await adapter.open({ + sessionId, + fence: 7, + acquisitionGeneration: acquired.acquisitionGeneration ?? null, + signal: new AbortController().signal + }) lifecycle.mockClear() const connection = connections[0] if (!connection) { diff --git a/src/main/pi/rpc-session-adapter.test.ts b/src/main/pi/rpc-session-adapter.test.ts index 48a0b9bc5bce..e401cbed2096 100644 --- a/src/main/pi/rpc-session-adapter.test.ts +++ b/src/main/pi/rpc-session-adapter.test.ts @@ -63,7 +63,16 @@ describe('Pi RPC session ownership and delivery', () => { const acquired = h.adapter.acquire({ ...h.input, fence: 8 }) const child = await opened.promise await Promise.resolve() - await expect(acquired).resolves.toMatchObject({ process: { pid: 4123 } }) + const owned = await acquired + expect(owned).toMatchObject({ process: { pid: 4123 } }) + const opening = h.adapter + .open({ + sessionId, + fence: 8, + acquisitionGeneration: owned.acquisitionGeneration ?? null, + signal: new AbortController().signal + }) + .catch((error: unknown) => error) h.lifecycle.mockClear() expect(h.adapter.readCommands(sessionId)).toBeUndefined() expect(h.adapter.holdsDispatch(sessionId)).toBe(false) @@ -73,7 +82,6 @@ describe('Pi RPC session ownership and delivery', () => { expect.objectContaining({ type: 'ended', startupUnproven: true, - startupUnanswered: true, cause: 'requested-close' }) ) @@ -81,6 +89,7 @@ describe('Pi RPC session ownership and delivery', () => { expect.objectContaining({ type: 'started', fence: 8 }) ) expect(child.rootVerdict).toBe('exited') + expect(await opening).toBeInstanceOf(Error) }) it('retains final events when exit publication times out under journal backpressure', async () => { @@ -422,9 +431,8 @@ describe('Pi RPC session ownership and delivery', () => { }) describe('Pi RPC start without a model', () => { - it('reports a start that listed no model as signed out, for that child only', async () => { + it('refuses a successfully empty listing and leaves cleanup to the host', async () => { const h = await setup() - expect(h.adapter.startUnavailable(sessionId)).toBeUndefined() await h.adapter.closeSession(sessionId) h.openConnection.mockImplementationOnce((_launch, handlers) => { const connection = new FakeConnection(handlers) @@ -434,19 +442,18 @@ describe('Pi RPC start without a model', () => { h.connections.push(connection) return connection }) - await h.adapter.acquire({ ...h.input, fence: 8 }) - // Known by the time the start is proven: the listing is part of Pi's start. - await vi.waitFor(() => - expect(h.lifecycle).toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) - ) - expect(h.adapter.startUnavailable(sessionId)).toEqual({ reason: 'notSignedIn' }) - // Its root gone while its output still drains: that child says nothing more. + const acquired = await h.adapter.acquire({ ...h.input, fence: 8 }) + await expect( + h.adapter.open({ + sessionId, + fence: 8, + acquisitionGeneration: acquired.acquisitionGeneration ?? null, + signal: new AbortController().signal + }) + ).rejects.toMatchObject({ reason: 'notSignedIn' }) const child = h.connections.at(-1)! - child.rootVerdict = 'exited' expect(child.closed).toBe(false) - expect(h.adapter.startUnavailable(sessionId)).toBeUndefined() - child.rootVerdict = 'live' await h.adapter.closeSession(sessionId) - expect(h.adapter.startUnavailable(sessionId)).toBeUndefined() + expect(child.closed).toBe(true) }) }) diff --git a/src/main/pi/rpc-session-adapter.ts b/src/main/pi/rpc-session-adapter.ts index 181cd802d7ce..6dc725194c29 100644 --- a/src/main/pi/rpc-session-adapter.ts +++ b/src/main/pi/rpc-session-adapter.ts @@ -1,6 +1,5 @@ import { randomUUID } from 'node:crypto' import { waitForPromiseWithSignal } from '../../shared/abort-signal-reason' -import type { AgentSessionUnavailable } from '../../shared/agent-session-availability' import { agentSessionFailureFact } from '../../shared/agent-session-failure' import { agentSessionFailureWords } from '../../shared/agent-session-failure-words' import { @@ -81,7 +80,7 @@ export class PiRpcSessionAdapter implements StructuredAgentSessionAdapter { } const spec = buildPiRpcLaunch(launch) const fresh = !launch.sessionFile && !launch.forkFile - session = new PiRpcSession(input, randomUUID(), spec, this.deps, fresh) + session = new PiRpcSession(input, randomUUID(), spec, this.deps, launch, fresh) this.sessions.set(id, session) this.starts.track(attempt, session.connection) const spawned = providerSpawnedProcessIdentity( @@ -96,8 +95,6 @@ export class PiRpcSessionAdapter implements StructuredAgentSessionAdapter { if (session.connection.closed || attempt.signal.aborted) { throw new Error('Pi exited while starting') } - const child = session - void child.start(launch, () => this.starts.retainFailed(id, child.connection)) return { process, acquisitionGeneration: session.generation, @@ -125,6 +122,19 @@ export class PiRpcSessionAdapter implements StructuredAgentSessionAdapter { } } + open: NonNullable = async (input) => { + const session = this.sessions.get(input.sessionId) + if ( + !session || + session.input.fence !== input.fence || + session.generation !== input.acquisitionGeneration || + session.connection.closed + ) { + throw new Error('Pi child changed before it opened') + } + return session.open(input.signal) + } + dispatch: StructuredAgentSessionAdapter['dispatch'] = async (input) => { const session = this.session(input.sessionId, input.fence) let prompt @@ -234,17 +244,6 @@ export class PiRpcSessionAdapter implements StructuredAgentSessionAdapter { holdsDispatch(id: string): boolean { return this.sessions.get(id)?.turns.holdsDispatch ?? false } - /** Started with no model listed: Pi keeps a placeholder, so every prompt this child takes fails - * as not signed in, even after a sign-in, which only a new Pi reads. */ - startUnavailable(id: string): AgentSessionUnavailable | undefined { - const session = this.sessions.get(id) - return session && - !session.connection.closed && - session.connection.rootVerdict !== 'exited' && - session.startup.options?.models.length === 0 - ? { reason: 'notSignedIn' } - : undefined - } holdsLiveProviderProcess(id: string, generation: string): boolean { const session = this.sessions.get(id) return session?.generation === generation && !session.connection.closed @@ -295,7 +294,12 @@ export class PiRpcSessionAdapter implements StructuredAgentSessionAdapter { } private session(id: string, fence: number): PiRpcSession { const session = this.sessions.get(id) - if (!session || session.input.fence !== fence || session.connection.closed) { + if ( + !session || + session.input.fence !== fence || + session.connection.closed || + !session.openedLink + ) { throw new Error('Pi session is not live under this fence') } return session diff --git a/src/main/pi/rpc-session-startup.test.ts b/src/main/pi/rpc-session-startup.test.ts index e463ed9cb621..eb426dd06b2e 100644 --- a/src/main/pi/rpc-session-startup.test.ts +++ b/src/main/pi/rpc-session-startup.test.ts @@ -1,12 +1,8 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' -import { providerDiagnostic, withProviderDiagnostic } from '../../shared/agent-session-failure' -import { AgentSessionAcquisitionExitUnprovenError } from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import { afterEach, describe, expect, it } from 'vitest' import { JsonlRpcResponseError } from '../jsonl-rpc/peer' import { closePiRpcAdapterFixtures, - file, restart, - reportedEvent, sessionId, setup, state @@ -14,235 +10,129 @@ import { afterEach(closePiRpcAdapterFixtures) -describe('Pi background startup', () => { - it('publishes at spawn, restores model before effort, lists models in its start and reads commands after', async () => { +describe('Pi host awaited open', () => { + it('owns the process before RPC and restores model before effort during open', async () => { const h = await setup() - const handshake = Promise.withResolvers() - const listing = Promise.withResolvers() - const commands = Promise.withResolvers() - let revision = 3 + const stateReply = Promise.withResolvers() const { acquired, child } = await restart( h, (connection) => { - connection.requestOverride = (command) => { - if ( - command === 'get_state' && - !h.lifecycle.mock.calls.some(([event]) => event.type === 'started') - ) { - // The listing's own state read sees what the restore set. - return connection.requests.includes('get_available_models') - ? Promise.resolve({ - ...state, - model: { provider: 'anthropic', id: 'model-2' }, - thinkingLevel: 'high' - }) - : handshake.promise - } - if (command === 'get_available_models') { - return listing.promise - } - if (command === 'get_commands') { - return commands.promise - } - return command === 'set_thinking_level' ? Promise.resolve({}) : undefined - } + connection.requestOverride = (command) => + command === 'get_state' && connection.requests.length === 1 + ? stateReply.promise + : command === 'set_thinking_level' + ? Promise.resolve({}) + : undefined }, - { options: { effort: 'high', model: 'anthropic/model-2' }, optionRevision: () => revision } - ) - expect(acquired.link).toBeUndefined() - expect(child.requests).toEqual(['get_state']) - expect(h.lifecycle).not.toHaveBeenCalled() - if (!acquired.acquisitionGeneration) { - throw new Error('acquisition generation missing') - } - expect(h.adapter.holdsLiveProviderProcess(sessionId, acquired.acquisitionGeneration)).toBe(true) - revision = 5 - handshake.resolve(state) - await vi.waitFor(() => expect(child.requests).toContain('get_available_models')) - expect(child.requests.slice(0, 4)).toEqual([ - 'get_state', - 'set_model', - 'set_thinking_level', - 'get_available_models' - ]) - // The listing is part of the start: no `started` before it answers. - expect(h.lifecycle).not.toHaveBeenCalled() - listing.resolve({ models: [{ provider: 'anthropic', id: 'model-2', reasoning: true }] }) - await vi.waitFor(() => - expect(h.lifecycle).toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) + { options: { model: 'anthropic/model-2', effort: 'high' } } ) - expect(h.lifecycle.mock.calls[0]?.[0]).toMatchObject({ - type: 'started', + expect(child.requests).toEqual([]) + expect(acquired.process.pid).toBe(4123) + const opening = h.adapter.open({ + sessionId, fence: 8, - acquisitionGeneration: acquired.acquisitionGeneration, - link: { handle: { nativeId: file }, origin: 'created' }, - reportedOptions: { - model: 'anthropic/model-2', - effort: 'high', - confirmed: ['model', 'effort'] - }, - restoreSkippedOptions: [], - optionRevision: 3 + acquisitionGeneration: acquired.acquisitionGeneration ?? null, + signal: new AbortController().signal }) - // Commands never hold the start. - expect(h.adapter.readCommands(sessionId)).toBeUndefined() - commands.resolve({ commands: [{ name: 'help' }] }) - await vi.waitFor(() => expect(h.adapter.readCommands(sessionId)).toBeDefined()) - expect(h.lifecycle).toHaveBeenCalledTimes(1) + expect(child.requests).toEqual(['get_state']) + stateReply.resolve(state) + await expect(opening).resolves.toMatchObject({ + reportedOptions: { model: 'anthropic/model-1' } + }) + expect(child.requests.indexOf('set_model')).toBeLessThan( + child.requests.indexOf('set_thinking_level') + ) + expect(h.lifecycle).not.toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) }) - it.each(['closeSession', 'disposeSession', 'forceCloseSession', 'releaseAcquisition'] as const)( - '%s stops a held handshake without a late started event', - async (method) => { - const h = await setup() - const handshake = Promise.withResolvers() - const { child } = await restart(h, (connection) => { - connection.requestOverride = (command) => - command === 'get_state' ? handshake.promise : undefined - }) - await (method === 'releaseAcquisition' - ? h.adapter.releaseAcquisition({ sessionId }) - : h.adapter[method](sessionId)) - handshake.resolve(state) - await h.adapter.drainObservedExits() - await Promise.resolve() - expect(child.rootVerdict).toBe('exited') - expect(h.lifecycle).toHaveBeenCalledWith( - expect.objectContaining({ type: 'ended', startupUnproven: true, startupUnanswered: true }) - ) - expect(h.lifecycle).not.toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) - } - ) - - it.each([ - ['get_state', new JsonlRpcResponseError('get_state', 'Pi is not signed in'), true], - ['set_model', new Error('Agent RPC request timed out: set_model'), false], - ['set_model', new Error('Agent JSON-lines RPC stream closed'), false] - ] as const)( - 'fails startup on %s failure %s and keeps its diagnostic', - async (command, error, unanswered) => { - const h = await setup() - const { child } = await restart( - h, - (connection) => { - connection.requestOverride = (name) => - name === command ? Promise.reject(error) : undefined - }, - { options: { model: 'anthropic/model-2' } } - ) - await vi.waitFor(() => - expect(h.lifecycle).toHaveBeenCalledWith(expect.objectContaining({ type: 'ended' })) - ) - expect(child.rootVerdict).toBe('exited') - const ended = reportedEvent(h.lifecycle, 'ended') - expect(ended).toMatchObject({ - cause: 'unexpected-exit', - startupUnproven: true, - reason: error.message - }) - expect(ended.startupUnanswered).toBe(unanswered ? true : undefined) - if (command === 'get_state') { - expect(ended.failure?.detail).toEqual({ text: 'Pi is not signed in', audience: 'person' }) - } - expect(h.lifecycle).not.toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) - } - ) - - it('does not retry acquisition while a failed start still owns an unproven process', async () => { + it('rejects a known empty listing once, then opens a fresh child after sign-in', async () => { const h = await setup() - const { child } = await restart(h, (connection) => { - connection.closeResult = { root: 'unverifiable', tree: null } - connection.requestOverride = () => Promise.reject(new Error('get_state failed')) - }) - await vi.waitFor(() => expect(child.closed).toBe(true)) - await expect(h.adapter.acquire({ ...h.input, fence: 9 })).rejects.toBeInstanceOf( - AgentSessionAcquisitionExitUnprovenError - ) - expect(h.connections).toHaveLength(2) - child.closeResult = { root: 'exited', tree: 'exited' } - await expect(h.adapter.acquire({ ...h.input, fence: 10 })).resolves.toMatchObject({ - process: { pid: 4123 } + const { acquired, child } = await restart(h, (connection) => { + connection.requestOverride = (command) => + command === 'get_available_models' ? Promise.resolve({ models: [] }) : undefined }) + await expect( + h.adapter.open({ + sessionId, + fence: 8, + acquisitionGeneration: acquired.acquisitionGeneration ?? null, + signal: new AbortController().signal + }) + ).rejects.toMatchObject({ reason: 'notSignedIn' }) + expect(child.closed).toBe(false) + expect(child.sent).toEqual([]) + await h.adapter.closeSession(sessionId) + const again = await h.adapter.acquire({ ...h.input, fence: 9 }) + await expect( + h.adapter.open({ + sessionId, + fence: 9, + acquisitionGeneration: again.acquisitionGeneration ?? null, + signal: new AbortController().signal + }) + ).resolves.toMatchObject({ reportedOptions: { model: 'anthropic/model-1' } }) + expect(h.connections).toHaveLength(3) }) - it('preserves the provider exit diagnostic when transport closure failed startup first', async () => { + it('treats an unknown provider as signed out even when listing succeeds', async () => { const h = await setup() - await restart(h, (connection) => { - connection.closeError = withProviderDiagnostic( - new Error('provider exited'), - providerDiagnostic('Pi is not signed in', 'log') - ) - connection.requestOverride = () => - Promise.reject(new Error('Agent JSON-lines RPC stream closed')) + const { acquired, child } = await restart(h, (connection) => { + connection.requestOverride = (command) => + command === 'get_state' + ? Promise.resolve({ ...state, model: { provider: 'unknown', id: 'unknown' } }) + : undefined }) - await vi.waitFor(() => - expect(h.lifecycle).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'ended', - startupUnproven: true, - failure: { - kind: 'providerStartFailed', - detail: { text: 'Pi is not signed in', audience: 'log' } - } - }) - ) - ) + await expect( + h.adapter.open({ + sessionId, + fence: 8, + acquisitionGeneration: acquired.acquisitionGeneration ?? null, + signal: new AbortController().signal + }) + ).rejects.toMatchObject({ reason: 'notSignedIn' }) + expect(child.requests).toEqual(['get_state']) }) - it('ends a child that exits while its handshake is held', async () => { + it('keeps a failed optional listing distinct from a known empty list', async () => { const h = await setup() - const { child } = await restart(h, (connection) => { - connection.requestOverride = () => new Promise(() => {}) + const { acquired } = await restart(h, (connection) => { + connection.requestOverride = (command) => + command === 'get_available_models' + ? Promise.reject(new JsonlRpcResponseError(command, 'listing unavailable')) + : undefined }) - child.exit(new Error('provider died during get_state')) - await h.adapter.drainObservedExits() - expect(h.lifecycle).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'ended', - cause: 'unexpected-exit', - startupUnproven: true, - startupUnanswered: true, - reason: 'provider died during get_state' + await expect( + h.adapter.open({ + sessionId, + fence: 8, + acquisitionGeneration: acquired.acquisitionGeneration ?? null, + signal: new AbortController().signal }) - ) - expect(h.lifecycle).not.toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) + ).resolves.toMatchObject({ reportedOptions: { model: 'anthropic/model-1' } }) }) - it('rejects an invalid session file and reports a proved resumed file from the launch', async () => { + it('lets the host close a child held in the state request without a late open', async () => { const h = await setup() - const { child } = await restart(h, (connection) => { - connection.requestOverride = (name) => - name === 'get_state' - ? Promise.resolve({ ...state, sessionFile: 'relative.jsonl' }) - : undefined + const stateReply = Promise.withResolvers() + const { acquired, child } = await restart(h, (connection) => { + connection.requestOverride = (command) => + command === 'get_state' ? stateReply.promise : undefined + connection.abortRequest = () => stateReply.reject(new Error('closed while starting')) }) - await vi.waitFor(() => - expect(h.lifecycle).toHaveBeenCalledWith( - expect.objectContaining({ type: 'ended', startupUnproven: true }) - ) - ) + const controller = new AbortController() + const opening = h.adapter + .open({ + sessionId, + fence: 8, + acquisitionGeneration: acquired.acquisitionGeneration ?? null, + signal: controller.signal + }) + .catch((error: unknown) => error) + expect(child.requests).toEqual(['get_state']) + controller.abort() + await h.adapter.closeSession(sessionId) + expect(await opening).toBeInstanceOf(Error) expect(child.rootVerdict).toBe('exited') - const launch = await h.resolveLaunch.mock.results[0]?.value - if (!launch) { - throw new Error('launch missing') - } - h.resolveLaunch.mockResolvedValueOnce({ - ...launch, - sessionFile: file, - previous: h.started.link - }) - await restart(h, () => {}) - await vi.waitFor(() => - expect(h.lifecycle).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'started', - link: expect.objectContaining({ origin: 'resumed' }) - }) - ) - ) - expect(h.openConnection.mock.calls.at(-1)?.[0].args).toEqual( - expect.arrayContaining(['--session', file]) - ) - expect(reportedEvent(h.lifecycle, 'started').link?.handle.nativeId).toBe(file) + expect(h.lifecycle).not.toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) }) }) diff --git a/src/main/pi/rpc-session-startup.ts b/src/main/pi/rpc-session-startup.ts index d24f5d5691ae..af2d5f3a10b9 100644 --- a/src/main/pi/rpc-session-startup.ts +++ b/src/main/pi/rpc-session-startup.ts @@ -1,11 +1,16 @@ import { randomUUID } from 'node:crypto' import { waitForPromiseWithSignal } from '../../shared/abort-signal-reason' +import { providerDiagnostic, withProviderDiagnostic } from '../../shared/agent-session-failure' import type { AgentSessionOptionsResult, AgentSessionSlashCommand } from '../../shared/agent-session-wire' import { JsonlRpcResponseError } from '../jsonl-rpc/peer' -import type { StructuredAgentSessionAcquireInput } from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import { + AgentSessionAcquisitionRefusal, + type StructuredAgentSessionAcquireInput +} from '../native-chat/agent-session-wire/structured-agent-session-adapter' +import type { StructuredAgentSessionProviderOpenResult } from '../native-chat/agent-session-wire/structured-agent-session-provider-open-contract' import { unpickedSessionConfiguredChoice, withLiveCatalogListing, @@ -27,17 +32,13 @@ import { STRUCTURED_AGENT_SESSION_STARTUP_CEILING_MS } from '../native-chat/agen const START_REQUEST = { timeoutMs: STRUCTURED_AGENT_SESSION_STARTUP_CEILING_MS } export class PiRpcSessionStartup { - started = false - answered = false skipped: readonly string[] = [] commands?: AgentSessionSlashCommand[] /** What the start listed; a signed-out Pi lists no model. */ options?: AgentSessionOptionsResult - private readonly controller = new AbortController() constructor( private readonly input: StructuredAgentSessionAcquireInput, - private readonly generation: string, private readonly connection: PiRpcConnection, private readonly selected: Map, private readonly deps: PiRpcSessionDeps, @@ -46,16 +47,17 @@ export class PiRpcSessionStartup { private readonly setState: (state: PiRpcState) => void ) {} - stop(): void { - this.controller.abort(new Error('Pi closed while starting')) - } - - async run(launch: PiRpcResolvedLaunch): Promise { + async run( + launch: PiRpcResolvedLaunch, + signal: AbortSignal + ): Promise { const optionRevision = this.input.optionRevision?.() ?? 0 - const answer = await this.wait(this.connection.request('get_state', {}, START_REQUEST)) - this.answered = true - this.assertLive() + const answer = await this.wait(this.connection.request('get_state', {}, START_REQUEST), signal) + this.assertLive(signal) const state = piRpcStateSchema.parse(answer) + if (state.model?.provider === 'unknown') { + throw this.notSignedIn() + } const link = piRpcProviderLink( launch, state.sessionFile, @@ -79,9 +81,10 @@ export class PiRpcSessionStartup { } picked.add(key) try { - this.assertLive() + this.assertLive(signal) await this.wait( - applyPiRpcSessionOption(this.connection, this.selected, key, value, START_REQUEST) + applyPiRpcSessionOption(this.connection, this.selected, key, value, START_REQUEST), + signal ) current[key] = value } catch (error) { @@ -92,12 +95,15 @@ export class PiRpcSessionStartup { this.warn('Pi rejected a saved option', 'pi-option-restore', error, { key }) } } - this.assertLive() + this.assertLive(signal) current.confirmed = [...(current.model ? ['model'] : []), ...(current.effort ? ['effort'] : [])] // Part of the start, as other agents' handshakes answer with theirs: a Pi that lists no model // is signed out, and what it started on names its config's default. - const options = await this.readListing() - this.assertLive() + const options = await this.readListing(signal) + this.assertLive(signal) + if (options && options.models.length === 0) { + throw this.notSignedIn() + } // What it started on names its config's default once; a later switch teaches nothing. let catalogListing: AgentModelCatalogLiveListing | undefined if (options) { @@ -108,67 +114,65 @@ export class PiRpcSessionStartup { ).catalogListing } this.skipped = skipped - this.started = true - this.deps.onLifecycle({ - type: 'started', - ...this.identity(), + await this.readCommands(signal) + this.assertLive(signal) + return { link, reportedOptions: options?.current ?? current, restoreSkippedOptions: skipped, optionRevision, ...(catalogListing ? { catalogListing } : {}) - }) - await this.readCommands() + } } /** Never fails the start: without it the chat only lacks the listing and the sign-in verdict. */ - private async readListing(): Promise { + private async readListing(signal: AbortSignal): Promise { try { - return await this.wait(readPiRpcSessionOptions(this.connection)) + return await this.wait(readPiRpcSessionOptions(this.connection), signal) } catch (error) { - this.assertLive() + this.assertLive(signal) this.warn('Pi options could not be read', 'pi-options', error) return undefined } } - private async readCommands(): Promise { + private async readCommands(signal: AbortSignal): Promise { try { - this.assertLive() - const result = await this.wait(readPiRpcCommands(this.connection)) - this.assertLive() + this.assertLive(signal) + const result = await this.wait(readPiRpcCommands(this.connection), signal) + this.assertLive(signal) this.commands = result.commands } catch (error) { this.warn('Pi commands could not be read', 'pi-commands', error) } } - private assertLive(): void { - if (this.controller.signal.aborted || this.connection.closed) { + private assertLive(signal: AbortSignal): void { + if (signal.aborted || this.connection.closed) { throw new Error('Pi closed while starting') } } - private wait(promise: Promise): Promise { - return waitForPromiseWithSignal(promise, this.controller.signal) + private wait(promise: Promise, signal: AbortSignal): Promise { + return waitForPromiseWithSignal(promise, signal) } - private identity() { - return { - sessionId: this.input.identity.sessionId, - fence: this.input.fence, - acquisitionGeneration: this.generation - } + private notSignedIn(): AgentSessionAcquisitionRefusal { + return withProviderDiagnostic( + new AgentSessionAcquisitionRefusal( + 'Pi has no authenticated model provider available.', + 'notSignedIn' + ), + providerDiagnostic('Pi has no authenticated model provider available.', 'person') + ) } private warn(message: string, scope: string, error: unknown, fields = {}): void { - if (!this.controller.signal.aborted) { - this.deps.logger.warn(message, { - scope, - sessionId: this.input.identity.sessionId, - error, - ...fields - }) - } + this.deps.logger.warn(message, { + scope, + sessionId: this.input.identity.sessionId, + error, + ...fields + }) } } diff --git a/src/main/pi/rpc-session.ts b/src/main/pi/rpc-session.ts index 925a1bbf727e..917b12eff7bd 100644 --- a/src/main/pi/rpc-session.ts +++ b/src/main/pi/rpc-session.ts @@ -22,6 +22,8 @@ import { PiRpcDialogCallbacks } from './rpc-dialog-callbacks' import { readPiRpcSessionOptions } from './rpc-options' import { withLiveCatalogListing } from '../native-chat/agent-model-catalog/agent-model-catalog-entry' import type { PiRpcResolvedLaunch } from './rpc-launch-resolution' +import type { StructuredAgentSessionProviderOpenResult } from '../native-chat/agent-session-wire/structured-agent-session-provider-open-contract' +import type { AgentSessionProviderHandleLink } from '../../shared/agent-session-provider-handle' import { PiRpcSessionStartup } from './rpc-session-startup' export type PiRpcConnection = Pick< @@ -62,6 +64,7 @@ export class PiRpcSession { readonly dialogs: PiRpcDialogCallbacks readonly selected = new Map() readonly startup: PiRpcSessionStartup + openedLink: AgentSessionProviderHandleLink | null = null requestedClose = false private publishedExit = false private failedCause?: Error @@ -73,6 +76,7 @@ export class PiRpcSession { readonly generation: string, launch: ProviderProcessLaunch, private readonly deps: PiRpcSessionDeps, + private readonly resolvedLaunch: PiRpcResolvedLaunch, /** A new Pi session starts on its config's model; a resumed or forked one keeps its own. */ readonly resolvesConfig = false ) { @@ -143,7 +147,6 @@ export class PiRpcSession { }) this.startup = new PiRpcSessionStartup( input, - generation, this.connection, this.selected, deps, @@ -154,18 +157,10 @@ export class PiRpcSession { ) } - async start(launch: PiRpcResolvedLaunch, retainFailed: () => void): Promise { - try { - await this.startup.run(launch) - } catch (error) { - if (!this.requestedClose) { - this.failedCause ??= error instanceof Error ? error : new Error(String(error)) - } - const result = await this.close(false).catch(() => null) - if (result?.root !== 'exited') { - retainFailed() - } - } + async open(signal: AbortSignal): Promise { + const result = await this.startup.run(this.resolvedLaunch, signal) + this.openedLink = result.link ?? null + return result } /** What the child reports now; only the start says what its config resolved. */ @@ -175,7 +170,6 @@ export class PiRpcSession { async close(requested = true): ReturnType { this.requestedClose ||= requested - this.startup.stop() this.dialogs.cancelAll() return this.connection.close() } @@ -187,6 +181,9 @@ export class PiRpcSession { sessionId: this.input.identity.sessionId, error }) + if (!this.openedLink) { + return + } void this.close(false) .then((result) => { if (result.root !== 'exited') { @@ -230,7 +227,6 @@ export class PiRpcSession { } this.publishedExit = true this.failedCause ??= error - this.startup?.stop() this.dialogs.cancelAll() this.turns.end() this.lane.finalize() @@ -265,19 +261,15 @@ export class PiRpcSession { observedAt: Date.now(), cause: this.requestedClose ? 'requested-close' : 'unexpected-exit', reason: cause.message, - failure: agentSessionFailureFact( - this.startup.started ? 'providerExited' : 'providerStartFailed', - { - detail: - providerDiagnosticOf(cause) ?? - providerDiagnosticOf(error) ?? - (cause instanceof JsonlRpcResponseError - ? providerDiagnostic(cause.message, 'person') - : undefined) - } - ), - ...(this.startup.started ? {} : { startupUnproven: true }), - ...(this.startup.answered ? {} : { startupUnanswered: true }) + failure: agentSessionFailureFact(this.openedLink ? 'providerExited' : 'providerStartFailed', { + detail: + providerDiagnosticOf(cause) ?? + providerDiagnosticOf(error) ?? + (cause instanceof JsonlRpcResponseError + ? providerDiagnostic(cause.message, 'person') + : undefined) + }), + ...(this.openedLink ? {} : { startupUnproven: true }) }) } diff --git a/src/main/runtime/structured-agent-session-pi-signed-out-start.test.ts b/src/main/runtime/structured-agent-session-pi-signed-out-start.test.ts index f35cd7b4657c..0d5faf66d5f6 100644 --- a/src/main/runtime/structured-agent-session-pi-signed-out-start.test.ts +++ b/src/main/runtime/structured-agent-session-pi-signed-out-start.test.ts @@ -1,11 +1,11 @@ -// A message that starts a signed-out Pi reaches it once and is settled by Pi's own refusal; the -// start made for it is never replaced as a signed-out child, which would start Pi for it forever. +// A signed-out Pi fails construction once; a later send after sign-in opens a new child. import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, expect, it, vi } from 'vitest' import { readWholeAgentSessionFailureFact } from '../../shared/agent-session-failure' +import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' import { hostTestMessage, HOST_TEST_SESSION as SESSION @@ -13,6 +13,7 @@ import { import { createTestParams } from '../native-chat/agent-session-wire/structured-agent-session-create-test-fixture' import { createStructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' import { piScriptedChild } from '../pi/pi-scripted-child.test-fixture' +import { peekOpenedAgentSessionRecordStore } from './agent-session-record-store-slot' import { ensureStructuredAgentSessionHost, stopStructuredAgentSessionRuntime @@ -31,7 +32,7 @@ afterEach(async () => { } }) -it('settles the first message of a signed-out Pi as not signed in, and starts Pi once', async () => { +it('settles the held message once, closes Pi, and starts afresh for a later send after sign-in', async () => { directory = await mkdtemp(join(tmpdir(), 'orca-pi-signed-out-')) const root = directory const script = piScriptedChild({ signedOut: true }) @@ -67,11 +68,10 @@ it('settles the first message of a signed-out Pi as not signed in, and starts Pi if (!created.ok) { throw new Error(JSON.stringify(created.refusal)) } - // Pi answers its start after the host has published it, as a real Pi does. + // Pi answers its start after the host has published it. await vi.waitFor(() => expect(host.collaboratorsForTests().sessions.get(SESSION)?.child?.phase).toBe('starting') ) - await new Promise((resolve) => setTimeout(resolve, 50)) script.releaseHandshake() const submission = async () => { await host.flushStreamedEvents(SESSION) @@ -86,9 +86,28 @@ it('settles the first message of a signed-out Pi as not signed in, and starts Pi expect(readWholeAgentSessionFailureFact((await submission())?.rejection)?.kind).toBe( 'notSignedIn' ) - // Nothing starts Pi again for a message it already refused. - await new Promise((resolve) => setTimeout(resolve, 500)) + await vi.waitFor(() => expect(script.closes()).toBe(1)) expect(script.spawns()).toBe(1) - expect(script.closes()).toBe(0) - expect(new Set(script.prompts())).toEqual(new Set(['hello, signed-out Pi'])) + expect(script.prompts()).toEqual([]) + script.signIn() + const body = hostTestMessage('hello after sign-in') + const fields = { body } + const sent = await host.send(caller, { + envelope: { + sessionId: SESSION, + clientOperationId: operationId(), + expectedRuntimeFence: + peekOpenedAgentSessionRecordStore()?.getRecord(SESSION)?.lease.runtimeFence ?? null, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.send', + sessionId: SESSION, + fields + }) + }, + body + }) + expect(sent.ok).toBe(true) + await vi.waitFor(() => expect(script.spawns()).toBe(2)) + script.releaseHandshake() + await vi.waitFor(() => expect(script.prompts()).toEqual(['hello after sign-in'])) }) From 50d36b04d24d921a8693b8cc57d47798baff9c9b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 12 Oct 2026 21:44:34 -0700 Subject: [PATCH 03/11] Read Pi catalog facts from awaited opening --- src/main/pi/rpc-configured-default.test.ts | 33 +++++++++------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/main/pi/rpc-configured-default.test.ts b/src/main/pi/rpc-configured-default.test.ts index 7ba6289084a4..2e22a3811e70 100644 --- a/src/main/pi/rpc-configured-default.test.ts +++ b/src/main/pi/rpc-configured-default.test.ts @@ -28,10 +28,6 @@ import { agentModelCatalogFingerprint } from '../native-chat/agent-model-catalog import { createPiModelCatalogProbe, piModelCatalogFromListing } from './rpc-model-catalog-probe' import type { PiRpcConnection } from './rpc-session' import { PiRpcSessionAdapter } from './rpc-session-adapter' -import type { - StructuredAgentSessionLifecycleEvent, - StructuredAgentSessionStartedEvent -} from '../native-chat/agent-session-wire/structured-agent-session-adapter' const PI_HOME = { variable: 'PI_CODING_AGENT_DIR', path: '/homes/pi' } // What `pi --list-models` prints: the first row is not the model Pi starts on. @@ -180,7 +176,6 @@ async function piHost() { } } as AgentSessionRecord) const rig = await openProviderTimelineRig({ agent: 'pi', sessionId }) - const onLifecycle = vi.fn<(event: StructuredAgentSessionLifecycleEvent) => void>() const adapter = new PiRpcSessionAdapter({ resolveLaunch: async () => ({ command: '/opt/pi/bin/pi', @@ -196,13 +191,12 @@ async function piHost() { child = piChild(opts.available ?? AVAILABLE, opts.thinking ?? 'high', opts.startsOn ?? '') return child.connection }, - onLifecycle, onSettled: vi.fn(), onIdle: vi.fn(), logger: { warn: vi.fn(), error: vi.fn() } }) adapters.push(adapter) - await adapter.acquire({ + const acquired = await adapter.acquire({ identity: { sessionId, workspaceId: 'ws-1', @@ -215,16 +209,17 @@ async function piHost() { options: opts.saved ?? {}, events: rig.eventSink }) - // The acquire answers at spawn; the start hands the host its listing once it is proven. - const started = await vi.waitFor(() => { - const event = onLifecycle.mock.calls - .map(([call]) => call) - .find((call): call is StructuredAgentSessionStartedEvent => call.type === 'started') - if (!event) { - throw new Error('Pi has not started') - } - return event - }) + const started = await adapter + .open({ + sessionId, + fence: 1, + acquisitionGeneration: acquired.acquisitionGeneration ?? null, + signal: new AbortController().signal + }) + .catch(async (error: unknown) => { + await adapter.closeSession(sessionId) + throw error + }) if (!started.catalogListing) { throw new Error('a Pi start hands the host its listing') } @@ -316,12 +311,12 @@ describe('Pi’s default comes from what a chat with no pick runs', () => { const { catalog, chat } = await piHost() const clean = workspace('clean') await catalog.read({ agent: 'pi', waitForListing: true }) - expect((await chat(clean, { available: [] })).listing).not.toHaveProperty('configuredDefault') + await expect(chat(clean, { available: [] })).rejects.toMatchObject({ reason: 'notSignedIn' }) expect(firstFrame(await catalog.read({ agent: 'pi', workspacePath: clean }))).toEqual(NOTHING) // Nor does it forget one a signed-in chat taught. await chat(clean) - await chat(clean, { available: [] }) + await expect(chat(clean, { available: [] })).rejects.toMatchObject({ reason: 'notSignedIn' }) expect(firstFrame(await catalog.read({ agent: 'pi', workspacePath: clean }))).toEqual({ model: 'openai/gpt-6', effort: 'high' From 90db80bac7b2d1eecde40515fd29e1c8fcc426c1 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 12 Oct 2026 21:47:14 -0700 Subject: [PATCH 04/11] Keep the Pi fixture exit observer --- src/main/pi/rpc-configured-default.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/pi/rpc-configured-default.test.ts b/src/main/pi/rpc-configured-default.test.ts index 2e22a3811e70..3091f3096f7b 100644 --- a/src/main/pi/rpc-configured-default.test.ts +++ b/src/main/pi/rpc-configured-default.test.ts @@ -191,6 +191,7 @@ async function piHost() { child = piChild(opts.available ?? AVAILABLE, opts.thinking ?? 'high', opts.startsOn ?? '') return child.connection }, + onLifecycle: vi.fn(), onSettled: vi.fn(), onIdle: vi.fn(), logger: { warn: vi.fn(), error: vi.fn() } From 0a282e3c11aa3287ad9955c73621e3e95c69fc62 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 13 Oct 2026 01:47:33 -0700 Subject: [PATCH 05/11] Restore signed-out restart and fail Pi startup faults --- .../scripts/vitest-sqlite-runtime-files.mjs | 1 + ...structured-agent-session-adapter-router.ts | 3 - .../structured-agent-session-adapter.ts | 8 +- ...tured-agent-session-outgrown-child.test.ts | 189 +++++++++++++----- ...structured-agent-session-outgrown-child.ts | 60 +++++- .../structured-agent-session-rest-test-rig.ts | 2 - src/main/pi/rpc-session-adapter.test.ts | 1 - src/main/pi/rpc-session-startup.test.ts | 18 ++ src/main/pi/rpc-session-startup.ts | 7 +- src/main/pi/rpc-session.ts | 8 +- ...uctured-agent-session-options-read.test.ts | 71 ------- .../structured-agent-session-options-read.ts | 8 +- ...gent-session-adapter-start-failure.test.ts | 128 ++++++++++++ 13 files changed, 352 insertions(+), 152 deletions(-) create mode 100644 src/main/runtime/structured-agent-session-adapter-start-failure.test.ts diff --git a/config/scripts/vitest-sqlite-runtime-files.mjs b/config/scripts/vitest-sqlite-runtime-files.mjs index b0c01a5ec51b..de59ab40f710 100644 --- a/config/scripts/vitest-sqlite-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-runtime-files.mjs @@ -251,6 +251,7 @@ export const SQLITE_RUNTIME_INCLUDE = [ 'src/main/runtime/structured-agent-session-codex-start-deadline.test.ts', 'src/main/runtime/structured-agent-session-pi-startup.test.ts', 'src/main/runtime/structured-agent-session-pi-signed-out-start.test.ts', + 'src/main/runtime/structured-agent-session-adapter-start-failure.test.ts', 'src/main/runtime/structured-agent-session-provider-operations-while-starting.test.ts', 'src/main/runtime/structured-agent-session-codex-settlement-order.test.ts', 'src/main/runtime/structured-agent-session-integration-replay.test.ts', diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts index ad17ad5e5e0f..34b3b6d4009b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts @@ -136,9 +136,6 @@ export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessi this.liveOwnerOrNull(sessionId)?.holdsLiveProviderProcess?.(sessionId, acquisitionGeneration) ?? false - startUnavailable: NonNullable = (sessionId) => - this.liveOwnerOrNull(sessionId)?.startUnavailable?.(sessionId) - stopEndsSession = (sessionId: string): boolean => this.liveOwnerOrNull(sessionId)?.stopEndsSession?.(sessionId) ?? false 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 cd8c7e125eb8..cbfa0c9b790d 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 @@ -1,7 +1,4 @@ -import type { - AgentSessionAccountKind, - AgentSessionUnavailable -} from '../../../shared/agent-session-availability' +import type { AgentSessionAccountKind } from '../../../shared/agent-session-availability' import type { AgentSessionRewindReason, AgentSessionRewindSupport @@ -381,9 +378,6 @@ export type StructuredAgentSessionAdapter = StructuredAgentSessionAdapterStop & /** The adapter's own child for this exact acquisition has a pid and its root exit has not been * seen: first-hand proof of life for lease renewal. Absent or false falls back to a PID probe. */ holdsLiveProviderProcess?(sessionId: string, acquisitionGeneration: string): boolean - /** Why the session's running child can take no turn, as its own start found; undefined when - * unknown or no child runs. Lives and dies with that child. */ - startUnavailable?(sessionId: string): AgentSessionUnavailable | undefined /** The `/` surface the running provider reports for itself. Undefined when the * provider never reports one, which is what keeps the client on its catalog. */ readCommands?(sessionId: string): AgentSessionSlashCommand[] | undefined diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.test.ts index 8c8d4fdb960b..216c6fa72371 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.test.ts @@ -1,9 +1,23 @@ -// An idle child launched under a different permission mode is replaced before the next send. +// A child the chat outgrew is replaced before the next send: one that reported it is not signed in, +// so a sign-in made since reaches a new child, and one launched with a permission mode the chat no +// longer has, so the new mode reaches a new child. -import { describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { agentSessionFailureFact } from '../../../shared/agent-session-failure' import { agentSessionFailureWords } from '../../../shared/agent-session-failure-words' -import { retireOutgrownStructuredAgentSessionChild } from './structured-agent-session-outgrown-child' +import { + acceptedDispatch, + createRestTestRig, + foundRestTestChat, + REST_TEST_CALLER as CALLER, + REST_TEST_SESSION as SESSION, + restTestSend, + type RestTestRig +} from './structured-agent-session-rest-test-rig' +import { + retireOutgrownStructuredAgentSessionChild, + structuredAgentSessionChildReportedSignedOut +} from './structured-agent-session-outgrown-child' import { claudeAndCodexDeclared } from './structured-agent-session-adapter-router-test-support' import { structuredAgentSessionLaunchPermission } from './structured-agent-session-launch-permission' import type { AgentChatPermissionMode } from '../../../shared/agent-chat-permission-mode' @@ -14,23 +28,97 @@ import type { AgentJournalTurnLifecycle } from '../../../shared/agent-session-journal-types' import type { AgentChildWorkView } from '../../../shared/agent-status-child-work-view' +import type { JournalItemLinkageVisitor } from '../agent-session-journal/journal-store-contracts' import type { StructuredAgentSessionChildWorkReads } from './structured-agent-session-idle-sweep' +describe('a send after the agent said it is not signed in', () => { + let rig: RestTestRig + + beforeEach(async () => { + rig = await createRestTestRig() + }) + + afterEach(async () => { + await rig.dispose() + }) + + const fence = () => rig.store.getRecord(SESSION)?.lease.runtimeFence ?? 1 + + it('goes to a new agent, and the new agent is kept for the sends after it', async () => { + await foundRestTestChat(rig) + rig.adapter.dispatch.mockImplementationOnce(async () => ({ + state: 'rejected', + ...agentSessionFailureWords(agentSessionFailureFact('notSignedIn'), { + surface: 'rejection' + }) + })) + expect((await rig.host.send(CALLER, restTestSend('signed out', fence()))).ok).toBe(true) + await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(2)) + expect(rig.adapter.acquire).toHaveBeenCalledTimes(1) + rig.adapter.closeSession.mockClear() + + expect((await rig.host.send(CALLER, restTestSend('signed in now', fence()))).ok).toBe(true) + await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(3)) + expect(rig.adapter.closeSession).toHaveBeenCalledWith(SESSION) + expect(rig.adapter.acquire).toHaveBeenCalledTimes(2) + + // The earlier child's report does not follow the new one. + expect((await rig.host.send(CALLER, restTestSend('and again', fence()))).ok).toBe(true) + await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(4)) + expect(rig.adapter.acquire).toHaveBeenCalledTimes(2) + }) + + it('keeps the agent after any other rejection', async () => { + await foundRestTestChat(rig) + rig.adapter.dispatch.mockImplementationOnce(async () => ({ + state: 'rejected', + ...agentSessionFailureWords(agentSessionFailureFact('providerRejected'), { + surface: 'rejection' + }) + })) + expect((await rig.host.send(CALLER, restTestSend('refused', fence()))).ok).toBe(true) + await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(2)) + rig.adapter.dispatch.mockImplementation(async () => acceptedDispatch()) + + expect((await rig.host.send(CALLER, restTestSend('next', fence()))).ok).toBe(true) + await vi.waitFor(() => expect(rig.adapter.dispatch).toHaveBeenCalledTimes(3)) + expect(rig.adapter.acquire).toHaveBeenCalledTimes(1) + }) +}) + type Row = { itemId: string; fence: number; agentId?: string; body: AgentJournalItemBody } +const statusRow = ( + fence: number, + kind: 'notSignedIn' | 'providerExited', + agentId?: string +): Row => ({ + itemId: `status-${fence}-${kind}-${agentId ?? 'root'}`, + fence, + ...(agentId ? { agentId } : {}), + body: { + kind: 'status', + ...agentSessionFailureWords(agentSessionFailureFact(kind), { surface: 'row' }) + } +}) +const rejected = (fence: number) => ({ + fence, + dispatchState: 'rejected' as const, + rejection: agentSessionFailureFact('notSignedIn') +}) const childAt = ( overrides: Partial = {} ): StructuredAgentSessionProviderChild => ({ generation: 'generation-2', fence: 2, phase: 'ready', - permissionMode: 'ask', ...overrides }) const conversation = ( input: { child?: StructuredAgentSessionProviderChild | null items?: Row[] + submissions?: ReturnType[] activeTurnId?: string newestTurnEndedAt?: number } = {} @@ -39,8 +127,14 @@ const conversation = ( return { child: input.child === undefined ? childAt() : input.child, journal: { + submissions: () => input.submissions ?? [], visitItems: (visit: (itemId: string, sequence: number, body: AgentJournalItemBody) => void) => items.forEach((item, index) => visit(item.itemId, index, item.body)), + visitItemsWithLinkage: (visit: JournalItemLinkageVisitor) => + items.forEach((item, index) => + visit(item.itemId, index, item.body, item.agentId ? { agentId: item.agentId } : {}) + ), + itemFence: (itemId: string) => items.find((item) => item.itemId === itemId)?.fence, activeTurnId: () => input.activeTurnId ?? null, newestTurn: (): AgentJournalTurnLifecycle | null => input.newestTurnEndedAt === undefined @@ -50,39 +144,33 @@ const conversation = ( } } -describe('retireOutgrownStructuredAgentSessionChild', () => { - it('keeps a signed-out child until normal failure settlement closes it', async () => { - const stopAgent = vi.fn(async () => undefined) - await retireOutgrownStructuredAgentSessionChild( - 'session', - conversation({ - child: childAt({ permissionMode: undefined }), - items: [ - { - itemId: 'signed-out', - fence: 2, - body: { - kind: 'status', - ...agentSessionFailureWords(agentSessionFailureFact('notSignedIn'), { - surface: 'row' - }) - } - } - ] - }), - { - work: { - childWork: () => [], - hasOpenDispatch: () => false, - providerHoldsDispatch: () => false - }, - launchPermission: async () => 'full-access', - stopAgent, - logger: { warn: vi.fn(), error: vi.fn() } - } - ) - expect(stopAgent).not.toHaveBeenCalled() +describe('structuredAgentSessionChildReportedSignedOut', () => { + const reported = (input: Parameters[0]) => + structuredAgentSessionChildReportedSignedOut(conversation(input)) + + it('reads a not-signed-in row the running child wrote', () => { + expect(reported({ items: [statusRow(2, 'notSignedIn')] })).toBe(true) + }) + + it('ignores one an earlier child or a subagent wrote, and any other failure', () => { + expect(reported({ items: [statusRow(1, 'notSignedIn')] })).toBe(false) + expect(reported({ items: [statusRow(2, 'notSignedIn', 'subagent-1')] })).toBe(false) + expect(reported({ items: [statusRow(2, 'providerExited')] })).toBe(false) + }) + + it('reads a send rejected as not signed in at this child, not at an earlier one', () => { + expect(reported({ submissions: [rejected(2)] })).toBe(true) + expect(reported({ submissions: [rejected(1)] })).toBe(false) }) + + it('leaves a child that is starting, closing, or absent', () => { + const items = [statusRow(2, 'notSignedIn')] + expect(reported({ items, child: childAt({ phase: 'starting' }) })).toBe(false) + expect(reported({ items, child: null })).toBe(false) + }) +}) + +describe('retireOutgrownStructuredAgentSessionChild', () => { const liveChildWork: AgentChildWorkView = { id: 'subagent-1', kind: 'agent', @@ -98,30 +186,34 @@ describe('retireOutgrownStructuredAgentSessionChild', () => { hasOpenDispatch: () => false, providerHoldsDispatch: () => false } - const outgrown = { child: childAt({ permissionMode: 'ask' }) } + const signedOut = { submissions: [rejected(2)] } const retire = async ( input: Parameters[0], work: Partial = {}, stopAgent = vi.fn(async () => undefined), - launchPermission: () => Promise = async () => 'full-access' + launchPermission?: () => Promise ) => { const warn = vi.fn() await retireOutgrownStructuredAgentSessionChild('session', conversation(input), { work: { ...idle, ...work }, - launchPermission, + ...(launchPermission ? { launchPermission } : {}), stopAgent, logger: { warn, error: vi.fn() } }) return { stopAgent, warn } } + it('stops a signed-out child that owes nothing', async () => { + expect((await retire(signedOut)).stopAgent).toHaveBeenCalledWith('session') + }) + it('keeps it while it owes work the idle sweep also protects', async () => { // A pending prompt is the shared check's too; the idle sweep's tests cover that case. for (const kept of [ - await retire({ ...outgrown, activeTurnId: 'turn-1' }), - await retire(outgrown, { childWork: () => [liveChildWork] }), - await retire(outgrown, { hasOpenDispatch: () => true }), - await retire(outgrown, { providerHoldsDispatch: () => true }) + await retire({ ...signedOut, activeTurnId: 'turn-1' }), + await retire(signedOut, { childWork: () => [liveChildWork] }), + await retire(signedOut, { hasOpenDispatch: () => true }), + await retire(signedOut, { providerHoldsDispatch: () => true }) ]) { expect(kept.stopAgent).not.toHaveBeenCalled() } @@ -136,12 +228,12 @@ describe('retireOutgrownStructuredAgentSessionChild', () => { settledAt: 200 } const after = await retire( - { ...outgrown, newestTurnEndedAt: 100 }, + { ...signedOut, newestTurnEndedAt: 100 }, { childWork: () => [settled] } ) expect(after.stopAgent).not.toHaveBeenCalled() const before = await retire( - { ...outgrown, newestTurnEndedAt: 300 }, + { ...signedOut, newestTurnEndedAt: 300 }, { childWork: () => [settled] } ) expect(before.stopAgent).toHaveBeenCalledWith('session') @@ -149,7 +241,7 @@ describe('retireOutgrownStructuredAgentSessionChild', () => { it('logs a stop that fails and lets the send go on', async () => { const { warn } = await retire( - outgrown, + signedOut, {}, vi.fn(async () => { throw new Error('exit not proven') @@ -268,10 +360,7 @@ describe('retireOutgrownStructuredAgentSessionChild', () => { it('never reads the mode for a child launched with none', async () => { const current = now('full-access') - expect( - (await retire({ child: childAt({ permissionMode: undefined }) }, {}, undefined, current)) - .stopAgent - ).not.toHaveBeenCalled() + expect((await retire({}, {}, undefined, current)).stopAgent).not.toHaveBeenCalled() expect(current).not.toHaveBeenCalled() }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.ts index 3c0d7fff55e1..03f3a32e0c70 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.ts @@ -1,5 +1,9 @@ -// An idle child launched with another permission mode is replaced before the next send. +// A child the chat outgrew (signed out, or launched with another permission mode) is replaced before +// the next send: agents read their login and permission mode only when they start. +import { readWholeAgentSessionFailureFact } from '../../../shared/agent-session-failure' +import { isRootAgentJournalItem } from '../../../shared/agent-session-journal-producer' +import type { AgentJournalSubmission } from '../../../shared/agent-session-journal-types' import type { AgentChildWorkView } from '../../../shared/agent-status-child-work-view' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' @@ -12,7 +16,48 @@ import type { AgentChatPermissionMode } from '../../../shared/agent-chat-permiss /** What the check reads of a conversation; every read skips building a snapshot. */ type OutgrownChildReading = Pick & { - journal: Pick + journal: Pick< + AgentSessionJournal, + 'visitItems' | 'visitItemsWithLinkage' | 'itemFence' | 'activeTurnId' | 'newestTurn' + > & { + submissions(): readonly Pick[] + } +} + +const signedOut = (failure: unknown): boolean => + readWholeAgentSessionFailureFact(failure)?.kind === 'notSignedIn' + +/** The running child itself said it is not signed in through a rejected send or a row it wrote. + * Its fence scopes both to this child, so a new one has nothing. */ +export function structuredAgentSessionChildReportedSignedOut( + session: OutgrownChildReading +): boolean { + const { child, journal } = session + if (!child || child.phase === 'starting' || child.close) { + return false + } + if ( + journal + .submissions() + .some( + (submission) => + submission.dispatchState === 'rejected' && + submission.fence === child.fence && + signedOut(submission.rejection) + ) + ) { + return true + } + // The session's own agent only: a subagent's sign-in failure says nothing about its parent. + let found = false + journal.visitItemsWithLinkage((itemId, _sequence, body, attribution) => { + found ||= + body.kind === 'status' && + isRootAgentJournalItem(attribution) && + signedOut(body.failure) && + journal.itemFence(itemId) === child.fence + }) + return found } /** Child work that settled after the lead's newest turn ended: the lead may still owe the turn @@ -40,9 +85,9 @@ async function launchedWithOtherPermissionMode( return mode !== null && mode !== child.permissionMode } -/** For a caller inside the session's serialize, before it hands the next send to - * the child. Work the child still owes keeps it, as it keeps an idle one from the sweep. A stop that fails - * leaves the close begun, which the start joins. */ +/** For a caller inside the session's serialize, before it hands the next send to the child. Work + * the child still owes keeps it, as it keeps an idle one from the sweep. A stop that fails leaves + * the close begun, which the start joins. */ export async function retireOutgrownStructuredAgentSessionChild( sessionId: string, session: OutgrownChildReading | undefined, @@ -56,7 +101,10 @@ export async function retireOutgrownStructuredAgentSessionChild( ): Promise { if ( !session || - !(await launchedWithOtherPermissionMode(session.child, deps.launchPermission)) || + !( + structuredAgentSessionChildReportedSignedOut(session) || + (await launchedWithOtherPermissionMode(session.child, deps.launchPermission)) + ) || structuredAgentSessionChildHasOpenWork(session.journal, deps.work) || owesWakeUp(session.journal, deps.work.childWork()) ) { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-rest-test-rig.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-rest-test-rig.ts index 21e35eebb9a8..37e13a2c71e4 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-rest-test-rig.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-rest-test-rig.ts @@ -50,7 +50,6 @@ export type RestTestAdapter = { NonNullable > holdsDispatch: Mock> - startUnavailable: Mock> readOptions: Mock> } @@ -166,7 +165,6 @@ export async function createRestTestRig( dispatch: vi.fn(async () => acceptedDispatch()), acknowledgeSessionRelease: vi.fn(), holdsDispatch: vi.fn(() => false), - startUnavailable: vi.fn(() => undefined), readOptions: vi.fn(async () => ({ models: [], current: { model: 'gpt-live' } })) } const starts = structuredAgentSessionHeldStarts(() => rig.host) diff --git a/src/main/pi/rpc-session-adapter.test.ts b/src/main/pi/rpc-session-adapter.test.ts index e401cbed2096..7d08cec2014d 100644 --- a/src/main/pi/rpc-session-adapter.test.ts +++ b/src/main/pi/rpc-session-adapter.test.ts @@ -81,7 +81,6 @@ describe('Pi RPC session ownership and delivery', () => { expect(h.lifecycle).toHaveBeenCalledWith( expect.objectContaining({ type: 'ended', - startupUnproven: true, cause: 'requested-close' }) ) diff --git a/src/main/pi/rpc-session-startup.test.ts b/src/main/pi/rpc-session-startup.test.ts index eb426dd06b2e..cf54e06637a1 100644 --- a/src/main/pi/rpc-session-startup.test.ts +++ b/src/main/pi/rpc-session-startup.test.ts @@ -135,4 +135,22 @@ describe('Pi host awaited open', () => { expect(child.rootVerdict).toBe('exited') expect(h.lifecycle).not.toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) }) + + it('fails an open if its lane fails while the state request is pending', async () => { + const h = await setup() + const stateReply = Promise.withResolvers() + const { acquired, child } = await restart(h, (connection) => { + connection.requestOverride = (command) => + command === 'get_state' ? stateReply.promise : undefined + }) + const opening = h.adapter.open({ + sessionId, + fence: 8, + acquisitionGeneration: acquired.acquisitionGeneration ?? null, + signal: new AbortController().signal + }) + child.handlers.onClose?.(new Error('lane failed during startup')) + stateReply.resolve(state) + await expect(opening).rejects.toThrow('lane failed during startup') + }) }) diff --git a/src/main/pi/rpc-session-startup.ts b/src/main/pi/rpc-session-startup.ts index af2d5f3a10b9..d7dd34baadf3 100644 --- a/src/main/pi/rpc-session-startup.ts +++ b/src/main/pi/rpc-session-startup.ts @@ -44,7 +44,8 @@ export class PiRpcSessionStartup { private readonly deps: PiRpcSessionDeps, /** A new Pi session starts on its config's model; a resumed or forked one keeps its own. */ private readonly resolvesConfig: boolean, - private readonly setState: (state: PiRpcState) => void + private readonly setState: (state: PiRpcState) => void, + private readonly failedCause: () => Error | undefined ) {} async run( @@ -148,6 +149,10 @@ export class PiRpcSessionStartup { } private assertLive(signal: AbortSignal): void { + const failure = this.failedCause() + if (failure) { + throw failure + } if (signal.aborted || this.connection.closed) { throw new Error('Pi closed while starting') } diff --git a/src/main/pi/rpc-session.ts b/src/main/pi/rpc-session.ts index 917b12eff7bd..406a7cfb08df 100644 --- a/src/main/pi/rpc-session.ts +++ b/src/main/pi/rpc-session.ts @@ -153,7 +153,8 @@ export class PiRpcSession { resolvesConfig, (state) => { this.lane.apply(this.turns.context.setModel(state.model ?? undefined, Date.now())) - } + }, + () => this.failedCause ) } @@ -261,15 +262,14 @@ export class PiRpcSession { observedAt: Date.now(), cause: this.requestedClose ? 'requested-close' : 'unexpected-exit', reason: cause.message, - failure: agentSessionFailureFact(this.openedLink ? 'providerExited' : 'providerStartFailed', { + failure: agentSessionFailureFact('providerExited', { detail: providerDiagnosticOf(cause) ?? providerDiagnosticOf(error) ?? (cause instanceof JsonlRpcResponseError ? providerDiagnostic(cause.message, 'person') : undefined) - }), - ...(this.openedLink ? {} : { startupUnproven: true }) + }) }) } diff --git a/src/main/runtime/rpc/methods/structured-agent-session-options-read.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-options-read.test.ts index 56db442ce2c2..2efcd0e0d000 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-options-read.test.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-options-read.test.ts @@ -2,15 +2,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { setStructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-registry' -import type { AgentSessionUnavailable } from '../../../../shared/agent-session-availability' import type { AgentSessionRecord } from '../../../../shared/agent-session-record' -import type { AgentSessionModelCatalogResult } from '../../../../shared/agent-session-wire' import { agentSessionRecordFixture } from '../../../../shared/agent-session-record.test-fixture' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' -import { - PI_STRUCTURED_DIALOGS_RUNTIME_CAPABILITY, - STRUCTURED_AGENT_SESSION_REGISTERED_AGENTS_RUNTIME_CAPABILITY -} from '../../../../shared/protocol-version' import { call, clearStructuredHostStub, @@ -166,68 +160,3 @@ describe('agentSession.modelCatalog before anything built the host', () => { expect(reply).toMatchObject({ ok: false }) }) }) - -const PI_CLIENT = { - ...STRUCTURED_CLIENT, - clientCapabilities: [ - ...STRUCTURED_CLIENT.clientCapabilities, - STRUCTURED_AGENT_SESSION_REGISTERED_AGENTS_RUNTIME_CAPABILITY, - PI_STRUCTURED_DIALOGS_RUNTIME_CAPABILITY - ] -} - -describe("agentSession.modelCatalog with what the running agent's start said", () => { - const read = vi.fn(async (): Promise => ({ origin: 'unknown' })) - const startUnavailable = vi.fn((_sessionId: string): AgentSessionUnavailable | undefined => ({ - reason: 'notSignedIn' - })) - const record: AgentSessionRecord = { ...agentSessionRecordFixture(), provider: 'pi' } - const readFor = (agent: string) => - call('agentSession.modelCatalog', { agent, sessionId: record.sessionId }, PI_CLIENT) - - beforeEach(() => { - read.mockReset() - read.mockResolvedValue({ origin: 'unknown' }) - startUnavailable.mockClear() - setStructuredAgentSessionHost( - Object.assign(hostStub(), { - deps: { - modelCatalog: { read }, - store: { getRecord: () => record }, - adapter: { startUnavailable } - } - }) - ) - }) - - it("answers a Pi chat's catalog with its signed-out start", async () => { - const reply = await readFor('pi') - expect(startUnavailable).toHaveBeenCalledWith(record.sessionId) - expect(reply).toMatchObject({ - ok: true, - result: { origin: 'unknown', unavailable: { reason: 'notSignedIn' } } - }) - }) - - it('keeps the reason the catalog itself found', async () => { - read.mockResolvedValue({ origin: 'unknown', unavailable: { reason: 'cliMissing' } }) - expect(await readFor('pi')).toMatchObject({ - ok: true, - result: { unavailable: { reason: 'cliMissing' } } - }) - }) - - it("never lends one agent's start to another agent's read", async () => { - const reply = await readFor('codex') - expect(startUnavailable).not.toHaveBeenCalled() - expect(reply).toMatchObject({ ok: true, result: { origin: 'unknown' } }) - expect(reply).not.toMatchObject({ result: { unavailable: expect.anything() } }) - }) - - it('stays quiet while the start said nothing', async () => { - startUnavailable.mockReturnValueOnce(undefined) - const reply = await readFor('pi') - expect(reply).toMatchObject({ ok: true }) - expect(reply).not.toMatchObject({ result: { unavailable: expect.anything() } }) - }) -}) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-options-read.ts b/src/main/runtime/rpc/methods/structured-agent-session-options-read.ts index 346b17ee8f0f..3d18c9b9dc31 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-options-read.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-options-read.ts @@ -88,13 +88,7 @@ export const STRUCTURED_AGENT_SESSION_OPTIONS_READ_METHODS = [ .catch(() => null) return catalog.read({ ...params, workspacePath }) } - const answer = await read() - // The chat's running agent can say what no listing does: a Pi that started with no model. - const started = - record?.provider === params.agent - ? host.deps.adapter.startUnavailable?.(record.sessionId) - : undefined - return started && !answer.unavailable ? { ...answer, unavailable: started } : answer + return read() } }) ] diff --git a/src/main/runtime/structured-agent-session-adapter-start-failure.test.ts b/src/main/runtime/structured-agent-session-adapter-start-failure.test.ts new file mode 100644 index 000000000000..fd2a38dab5da --- /dev/null +++ b/src/main/runtime/structured-agent-session-adapter-start-failure.test.ts @@ -0,0 +1,128 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, expect, it, vi } from 'vitest' +import { readWholeAgentSessionFailureFact } from '../../shared/agent-session-failure' +import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' +import { + hostTestAttachParams, + hostTestMessage, + HOST_TEST_SESSION as SESSION +} from '../native-chat/agent-session-wire/structured-agent-session-host-test-data' +import { createStructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' +import { piScriptedChild } from '../pi/pi-scripted-child.test-fixture' +import { codexScriptedChild } from '../codex/codex-scripted-child.test-fixture' +import { acpScriptedChild } from '../acp/acp-scripted-child.test-fixture' +import type { ScriptedAgentChild } from './structured-agent-scripted-child.test-fixture' +import { + ensureStructuredAgentSessionHost, + stopStructuredAgentSessionRuntime +} from './structured-agent-session-runtime' + +let directory: string | undefined +let operations = 0 +const operationId = () => `${Date.now()}-${(++operations).toString(16).padStart(32, '0')}` +const caller = { callerKey: 'adapter-start-failure-test' } + +afterEach(async () => { + await stopStructuredAgentSessionRuntime() + if (directory) { + await rm(directory, { recursive: true, force: true }) + directory = undefined + } +}) + +async function failedStart(agent: 'pi' | 'codex' | 'grok', script: ScriptedAgentChild) { + directory = await mkdtemp(join(tmpdir(), 'orca-adapter-start-failure-')) + const root = directory + const host = await ensureStructuredAgentSessionHost({ + logger: createStructuredAgentSessionLogger(), + stateDirectory: root, + hostId: 'local', + claimKeyId: 'key-1', + resolveWorkspacePath: async () => root, + resolveClaudeAuthPolicy: () => ({ account: 'managed' }), + resolveLaunchArgs: () => [], + resolveEnvironment: async () => ({ PATH: '' }), + ...script.deps + }) + const params = hostTestAttachParams( + null, + agent === 'codex' + ? { providerHandle: undefined } + : { + provider: agent, + agent, + providerHandle: undefined, + accountHome: + agent === 'pi' + ? { variable: 'PI_CODING_AGENT_DIR', path: root } + : { variable: 'GROK_HOME', path: root }, + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'folder-1', + workspaceKind: 'folder' + } + } + ) + params.envelope.clientOperationId = operationId() + const attached = await host.attach(caller, params) + if (!attached.ok) { + throw new Error(JSON.stringify(attached.refusal)) + } + const body = hostTestMessage('held during start') + const sent = await host.send(caller, { + envelope: { + sessionId: SESSION, + clientOperationId: operationId(), + expectedRuntimeFence: attached.value.fence, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.send', + sessionId: SESSION, + fields: { body } + }) + }, + body + }) + if (!sent.ok) { + throw new Error(JSON.stringify(sent.refusal)) + } + await vi.waitFor(() => expect(script.spawns()).toBe(1)) + script.failHandshake?.('provider config is invalid: bad key') + const submission = async () => { + await host.flushStreamedEvents(SESSION) + return (await host.journalSnapshot(SESSION)).submissions.find( + (entry) => entry.clientMessageId === sent.value.clientMessageId + ) + } + await vi.waitFor(async () => expect((await submission())?.dispatchState).toBe('rejected'), { + timeout: 5_000 + }) + const settled = await submission() + const snapshot = await host.journalSnapshot(SESSION) + const failureKind = agent === 'codex' ? 'startFailed' : 'providerStartFailed' + expect(readWholeAgentSessionFailureFact(settled?.rejection)).toMatchObject({ + kind: failureKind, + detail: { text: 'provider config is invalid: bad key' } + }) + expect( + snapshot.items.filter( + (item) => + item.body.kind === 'status' && + readWholeAgentSessionFailureFact(item.body.failure)?.kind === failureKind + ) + ).toHaveLength(1) + expect(script.prompts()).toEqual([]) + expect(script.spawns()).toBe(1) +} + +it('Pi records one failure without dispatch after a failed start', async () => { + await failedStart('pi', piScriptedChild()) +}) +it('Codex records one failure without dispatch after a failed start', async () => { + await failedStart('codex', codexScriptedChild()) +}) +it('ACP records one failure without dispatch after a failed start', async () => { + await failedStart('grok', acpScriptedChild('grok')()) +}) From 6ff10a68530f2896b6d3bdc74da8ef6abee98fbf Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 13 Oct 2026 02:24:33 -0700 Subject: [PATCH 06/11] Run Pi opening catalog fixtures with Node SQLite --- config/scripts/vitest-sqlite-runtime-files.mjs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/scripts/vitest-sqlite-runtime-files.mjs b/config/scripts/vitest-sqlite-runtime-files.mjs index de59ab40f710..130cf64d0229 100644 --- a/config/scripts/vitest-sqlite-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-runtime-files.mjs @@ -2,6 +2,8 @@ import { SQLITE_DATABASE_RUNTIME_INCLUDE } from './vitest-sqlite-database-runtim // Real SQLite fixtures keep Node publication and close semantics. export const SQLITE_RUNTIME_INCLUDE = [ + 'src/main/pi/rpc-session-adapter.test.ts', + 'src/main/pi/rpc-session-startup.test.ts', 'src/main/acp/acp-structured-publish-at-spawn.test.ts', ...SQLITE_DATABASE_RUNTIME_INCLUDE, 'src/main/acp/acp-structured-host-live-listing.test.ts', From f1571b8119712652461fb2d775635a269d7924b0 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 13 Oct 2026 04:10:04 -0700 Subject: [PATCH 07/11] Remove retired Pi startup revision and match saved-pick behavior --- src/main/pi/rpc-session-startup.ts | 2 -- .../structured-agent-session-pi-startup.test.ts | 15 +++++---------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/main/pi/rpc-session-startup.ts b/src/main/pi/rpc-session-startup.ts index d7dd34baadf3..8a7c6a39b79c 100644 --- a/src/main/pi/rpc-session-startup.ts +++ b/src/main/pi/rpc-session-startup.ts @@ -52,7 +52,6 @@ export class PiRpcSessionStartup { launch: PiRpcResolvedLaunch, signal: AbortSignal ): Promise { - const optionRevision = this.input.optionRevision?.() ?? 0 const answer = await this.wait(this.connection.request('get_state', {}, START_REQUEST), signal) this.assertLive(signal) const state = piRpcStateSchema.parse(answer) @@ -121,7 +120,6 @@ export class PiRpcSessionStartup { link, reportedOptions: options?.current ?? current, restoreSkippedOptions: skipped, - optionRevision, ...(catalogListing ? { catalogListing } : {}) } } diff --git a/src/main/runtime/structured-agent-session-pi-startup.test.ts b/src/main/runtime/structured-agent-session-pi-startup.test.ts index 19403cfa5aa8..da1c11bb1c1e 100644 --- a/src/main/runtime/structured-agent-session-pi-startup.test.ts +++ b/src/main/runtime/structured-agent-session-pi-startup.test.ts @@ -103,15 +103,14 @@ it('attaches Pi at spawn and holds the opening send until the provider answers i await vi.waitFor(() => expect(script.prompts()).toEqual(['hello while Pi is starting'])) await vi.waitFor(() => expect(peekOpenedAgentSessionRecordStore()?.getRecord(HOST_TEST_SESSION)?.options).toEqual({ - model: 'scripted/model-1', - effort: 'medium' + unknown: 'unavailable-saved-option' }) ) await stopStructuredAgentSessionRuntime() expect(script.closes()).toBe(1) }) -it('keeps a model picked during startup when the optional state read predates applying the pick', async () => { +it('saves a startup model pick without writing it to the starting Pi child', async () => { const { script, host, caller, fence } = await setup() const fields = { key: 'model', value: 'scripted/model-2' } const picked = await host.setOption(caller, { @@ -135,16 +134,12 @@ it('keeps a model picked during startup when the optional state read predates ap }) expect(sent.ok).toBe(true) script.releaseHandshake() - // The pick reaches Pi before the message held for its start. + // A startup pick is saved for a later start at this piece. await vi.waitFor(() => expect(script.prompts()).toEqual(['first message'])) const wire = script.wire() - expect(wire.indexOf('set_model')).toBeGreaterThan(-1) - expect(wire.indexOf('set_model')).toBeLessThan(wire.indexOf('prompt:first message')) - const { runtimeState, sessions, serialize } = host.collaboratorsForTests() + expect(wire).not.toContain('set_model') + const { sessions, serialize } = host.collaboratorsForTests() await vi.waitFor(() => expect(sessions.get(HOST_TEST_SESSION)?.child?.phase).toBe('ready')) - await vi.waitFor(() => - expect(runtimeState.optionRevisions.current(HOST_TEST_SESSION)).toBeGreaterThan(1) - ) await serialize(HOST_TEST_SESSION, async () => {}) expect(peekOpenedAgentSessionRecordStore()?.getRecord(HOST_TEST_SESSION)?.options?.model).toBe( fields.value From 0b4f30e09dc1168fd507dce4314eaa7b06f82963 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 13 Oct 2026 05:18:14 -0700 Subject: [PATCH 08/11] Apply Pi startup picks after readiness and remove unused open facts [skip ci] --- src/main/pi/rpc-session-adapter.test.ts | 2 +- src/main/pi/rpc-session-startup.ts | 1 - .../structured-agent-session-pi-startup.test.ts | 13 +++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/pi/rpc-session-adapter.test.ts b/src/main/pi/rpc-session-adapter.test.ts index 7d08cec2014d..d4d2e4c4bf11 100644 --- a/src/main/pi/rpc-session-adapter.test.ts +++ b/src/main/pi/rpc-session-adapter.test.ts @@ -171,7 +171,7 @@ describe('Pi RPC session ownership and delivery', () => { origin: 'created', mintedAtFence: 7 }) - expect(h.started.restoreSkippedOptions).toEqual(['unknown', 'effort']) + expect(h.adapter.readOptionRestoreFailures(sessionId)).toEqual(['unknown', 'effort']) expect(h.connection.requests).toContain('get_commands') }) diff --git a/src/main/pi/rpc-session-startup.ts b/src/main/pi/rpc-session-startup.ts index 8a7c6a39b79c..b246ea9b9aab 100644 --- a/src/main/pi/rpc-session-startup.ts +++ b/src/main/pi/rpc-session-startup.ts @@ -119,7 +119,6 @@ export class PiRpcSessionStartup { return { link, reportedOptions: options?.current ?? current, - restoreSkippedOptions: skipped, ...(catalogListing ? { catalogListing } : {}) } } diff --git a/src/main/runtime/structured-agent-session-pi-startup.test.ts b/src/main/runtime/structured-agent-session-pi-startup.test.ts index da1c11bb1c1e..61dbba5ff0be 100644 --- a/src/main/runtime/structured-agent-session-pi-startup.test.ts +++ b/src/main/runtime/structured-agent-session-pi-startup.test.ts @@ -110,10 +110,10 @@ it('attaches Pi at spawn and holds the opening send until the provider answers i expect(script.closes()).toBe(1) }) -it('saves a startup model pick without writing it to the starting Pi child', async () => { +it('writes a startup model pick once the Pi child is ready', async () => { const { script, host, caller, fence } = await setup() const fields = { key: 'model', value: 'scripted/model-2' } - const picked = await host.setOption(caller, { + const picked = host.setOption(caller, { envelope: { sessionId: HOST_TEST_SESSION, clientOperationId: operationId(), @@ -126,18 +126,19 @@ it('saves a startup model pick without writing it to the starting Pi child', asy }, ...fields }) - expect(picked.ok).toBe(true) + expect(script.wire()).not.toContain('set_model') + script.releaseHandshake() + expect((await picked).ok).toBe(true) + expect(script.prompts()).toEqual([]) const body = hostTestMessage('first message') const sent = await host.send(caller, { envelope: envelope('agentSession.send', { body }), body }) expect(sent.ok).toBe(true) - script.releaseHandshake() - // A startup pick is saved for a later start at this piece. await vi.waitFor(() => expect(script.prompts()).toEqual(['first message'])) const wire = script.wire() - expect(wire).not.toContain('set_model') + expect(wire).toContain('set_model') const { sessions, serialize } = host.collaboratorsForTests() await vi.waitFor(() => expect(sessions.get(HOST_TEST_SESSION)?.child?.phase).toBe('ready')) await serialize(HOST_TEST_SESSION, async () => {}) From b85ccadc250052a4223f867d09f3357fb3299b4f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 13 Oct 2026 11:11:50 -0700 Subject: [PATCH 09/11] Keep startup recovery flows and remove duplicate Pi handshake cases --- src/main/pi/rpc-session-adapter.test.ts | 74 --------------- src/main/pi/rpc-session-startup.test.ts | 67 -------------- ...provider-operations-while-starting.test.ts | 89 +------------------ 3 files changed, 1 insertion(+), 229 deletions(-) diff --git a/src/main/pi/rpc-session-adapter.test.ts b/src/main/pi/rpc-session-adapter.test.ts index d4d2e4c4bf11..004a21b5e6be 100644 --- a/src/main/pi/rpc-session-adapter.test.ts +++ b/src/main/pi/rpc-session-adapter.test.ts @@ -5,7 +5,6 @@ import { AgentSessionAcquisitionRootExitObservedError } from '../native-chat/age import { JsonlRpcResponseError } from '../jsonl-rpc/peer' import { closePiRpcAdapterFixtures, - FakeConnection, file, sessionId, setup @@ -46,51 +45,6 @@ describe('Pi RPC session ownership and delivery', () => { expect(h.connections).toHaveLength(2) }) - it('closes a published child during a stalled handshake without reporting started', async () => { - const h = await setup() - await h.adapter.closeSession(sessionId) - const opened = Promise.withResolvers() - const reply = Promise.withResolvers() - h.openConnection.mockImplementationOnce((_launch, handlers) => { - const connection = new FakeConnection(handlers) - connection.requestOverride = (command) => - command === 'get_state' ? reply.promise : undefined - connection.abortRequest = () => reply.reject(new Error('closed while starting')) - h.connections.push(connection) - opened.resolve(connection) - return connection - }) - const acquired = h.adapter.acquire({ ...h.input, fence: 8 }) - const child = await opened.promise - await Promise.resolve() - const owned = await acquired - expect(owned).toMatchObject({ process: { pid: 4123 } }) - const opening = h.adapter - .open({ - sessionId, - fence: 8, - acquisitionGeneration: owned.acquisitionGeneration ?? null, - signal: new AbortController().signal - }) - .catch((error: unknown) => error) - h.lifecycle.mockClear() - expect(h.adapter.readCommands(sessionId)).toBeUndefined() - expect(h.adapter.holdsDispatch(sessionId)).toBe(false) - await h.adapter.closeSession(sessionId) - await h.adapter.drainObservedExits() - expect(h.lifecycle).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'ended', - cause: 'requested-close' - }) - ) - expect(h.lifecycle).not.toHaveBeenCalledWith( - expect.objectContaining({ type: 'started', fence: 8 }) - ) - expect(child.rootVerdict).toBe('exited') - expect(await opening).toBeInstanceOf(Error) - }) - it('retains final events when exit publication times out under journal backpressure', async () => { let blocked = true const h = await setup({}, (sink) => ({ @@ -428,31 +382,3 @@ describe('Pi RPC session ownership and delivery', () => { expect(await other.adapter.closeSession(sessionId)).toBe(false) }) }) - -describe('Pi RPC start without a model', () => { - it('refuses a successfully empty listing and leaves cleanup to the host', async () => { - const h = await setup() - await h.adapter.closeSession(sessionId) - h.openConnection.mockImplementationOnce((_launch, handlers) => { - const connection = new FakeConnection(handlers) - // What a signed-out Pi lists (captured from Pi 1.0.4 in __fixtures__/signed-out.jsonl). - connection.requestOverride = (command) => - command === 'get_available_models' ? Promise.resolve({ models: [] }) : undefined - h.connections.push(connection) - return connection - }) - const acquired = await h.adapter.acquire({ ...h.input, fence: 8 }) - await expect( - h.adapter.open({ - sessionId, - fence: 8, - acquisitionGeneration: acquired.acquisitionGeneration ?? null, - signal: new AbortController().signal - }) - ).rejects.toMatchObject({ reason: 'notSignedIn' }) - const child = h.connections.at(-1)! - expect(child.closed).toBe(false) - await h.adapter.closeSession(sessionId) - expect(child.closed).toBe(true) - }) -}) diff --git a/src/main/pi/rpc-session-startup.test.ts b/src/main/pi/rpc-session-startup.test.ts index cf54e06637a1..e381e875d18b 100644 --- a/src/main/pi/rpc-session-startup.test.ts +++ b/src/main/pi/rpc-session-startup.test.ts @@ -1,5 +1,4 @@ import { afterEach, describe, expect, it } from 'vitest' -import { JsonlRpcResponseError } from '../jsonl-rpc/peer' import { closePiRpcAdapterFixtures, restart, @@ -45,72 +44,6 @@ describe('Pi host awaited open', () => { expect(h.lifecycle).not.toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) }) - it('rejects a known empty listing once, then opens a fresh child after sign-in', async () => { - const h = await setup() - const { acquired, child } = await restart(h, (connection) => { - connection.requestOverride = (command) => - command === 'get_available_models' ? Promise.resolve({ models: [] }) : undefined - }) - await expect( - h.adapter.open({ - sessionId, - fence: 8, - acquisitionGeneration: acquired.acquisitionGeneration ?? null, - signal: new AbortController().signal - }) - ).rejects.toMatchObject({ reason: 'notSignedIn' }) - expect(child.closed).toBe(false) - expect(child.sent).toEqual([]) - await h.adapter.closeSession(sessionId) - const again = await h.adapter.acquire({ ...h.input, fence: 9 }) - await expect( - h.adapter.open({ - sessionId, - fence: 9, - acquisitionGeneration: again.acquisitionGeneration ?? null, - signal: new AbortController().signal - }) - ).resolves.toMatchObject({ reportedOptions: { model: 'anthropic/model-1' } }) - expect(h.connections).toHaveLength(3) - }) - - it('treats an unknown provider as signed out even when listing succeeds', async () => { - const h = await setup() - const { acquired, child } = await restart(h, (connection) => { - connection.requestOverride = (command) => - command === 'get_state' - ? Promise.resolve({ ...state, model: { provider: 'unknown', id: 'unknown' } }) - : undefined - }) - await expect( - h.adapter.open({ - sessionId, - fence: 8, - acquisitionGeneration: acquired.acquisitionGeneration ?? null, - signal: new AbortController().signal - }) - ).rejects.toMatchObject({ reason: 'notSignedIn' }) - expect(child.requests).toEqual(['get_state']) - }) - - it('keeps a failed optional listing distinct from a known empty list', async () => { - const h = await setup() - const { acquired } = await restart(h, (connection) => { - connection.requestOverride = (command) => - command === 'get_available_models' - ? Promise.reject(new JsonlRpcResponseError(command, 'listing unavailable')) - : undefined - }) - await expect( - h.adapter.open({ - sessionId, - fence: 8, - acquisitionGeneration: acquired.acquisitionGeneration ?? null, - signal: new AbortController().signal - }) - ).resolves.toMatchObject({ reportedOptions: { model: 'anthropic/model-1' } }) - }) - it('lets the host close a child held in the state request without a late open', async () => { const h = await setup() const stateReply = Promise.withResolvers() diff --git a/src/main/runtime/structured-agent-session-provider-operations-while-starting.test.ts b/src/main/runtime/structured-agent-session-provider-operations-while-starting.test.ts index 66c7fd735e72..e6cd3762a512 100644 --- a/src/main/runtime/structured-agent-session-provider-operations-while-starting.test.ts +++ b/src/main/runtime/structured-agent-session-provider-operations-while-starting.test.ts @@ -1,6 +1,4 @@ -// An operation only the agent's protocol session can perform (a rewind, a goal, the recovery of a -// rewind left in doubt) never reaches a child still proving its start, for any agent. Driven through -// the real Codex and ACP adapters with only the provider child scripted. +// Protocol operations wait for the real Codex adapter to open; only its child is scripted. import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' @@ -9,9 +7,6 @@ import { afterEach, expect, it, vi } from 'vitest' import { agentJournalSubmissionKey } from '../../shared/agent-session-journal-item-key' import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' import type { AgentSessionMutationEnvelope } from '../../shared/agent-session-wire' -import { ACP_LAUNCH_SPECS } from '../acp/acp-launch-specs' -import { acpScriptedChild } from '../acp/acp-scripted-child.test-fixture' -import { acpStructuredAgentDefinition } from '../acp/acp-structured-agent-definitions' import { CODEX_STRUCTURED_AGENT } from '../codex/codex-structured-agent-definition' import type { StructuredAgentDefinition } from '../native-chat/agent-session-wire/structured-agent-definition' import { @@ -142,46 +137,6 @@ async function settlesSoon(pending: Promise): Promise { return settled } -it('recovers a Codex rewind left in doubt once the restarted thread is open, then delivers the send that started it', async () => { - const child = codexScriptedChild({ - 'thread/read': () => ({ thread: { id: CODEX_SCRIPTED_THREAD, status: { type: 'idle' } } }), - // The rewound turn is gone and nothing earlier is retained: the provider applied the rewind. - 'thread/turns/list': () => ({ data: [], nextCursor: null }), - 'thread/items/list': () => ({ data: [], nextCursor: null }) - }) - const { host, itemId, providerItemId } = await codexChatAtRestWithATurn(child) - const { store } = host.deps - const expectedEpoch = (await host.journalSnapshot(SESSION)).cursor.epoch - const request = envelope(host, 'agentSession.rewind', { itemId, expectedEpoch }) - await store.admitMutationOperation({ - callerKey: CALLER.callerKey, - envelope: request, - hostFingerprint: request.payloadFingerprint, - now: Date.now() - }) - await store.transitionHandoff(SESSION, (record) => ({ - ...record, - rewind: { - operationId: request.clientOperationId, - callerKey: CALLER.callerKey, - itemId, - providerItemId, - expectedEpoch, - phase: 'prepared', - retained: [] - } - })) - - const sent = send(host, 'after the rewind') - await eventually(() => expect(child.spawns()).toBe(2)) - expect(await settlesSoon(sent)).toBe(false) - child.releaseHandshake() - - expect(await sent).toMatchObject({ ok: true }) - expect(store.getRecord(SESSION)?.rewind?.phase).toBe('completed') - await eventually(() => expect(child.prompts()).toEqual(['first turn', 'after the rewind'])) -}) - it('settles a rewind that goes in doubt while a send waits in the queue, by waiting out the start the send needs', async () => { const child = codexScriptedChild({ 'thread/read': () => ({ thread: { id: CODEX_SCRIPTED_THREAD, status: { type: 'idle' } } }), @@ -257,27 +212,6 @@ it('answers a rewind asked of a resting Codex chat from the thread it opens, lea await eventually(() => expect(child.prompts()).toEqual(['first turn', 'still sends'])) }) -it('sets a goal asked of a starting Codex chat on the thread once it is open', async () => { - const child = codexScriptedChild() - const host = await openChat(child) - const change = { kind: 'set' as const, objective: 'Ship the release' } - - const goal = host.changeThreadGoal(CALLER, { - envelope: envelope(host, 'agentSession.threadGoal', { change }), - change - }) - expect(await settlesSoon(goal)).toBe(false) - child.releaseHandshake() - - expect(await goal).toMatchObject({ ok: true }) - expect(child.requests().filter(({ method }) => method.startsWith('thread/goal'))).toEqual([ - { - method: 'thread/goal/set', - params: { threadId: CODEX_SCRIPTED_THREAD, objective: 'Ship the release', status: 'active' } - } - ]) -}) - it('keeps a goal waiting on a starting Codex chat through a re-attach, then sets it once the thread is open', async () => { const child = codexScriptedChild() const host = await openChat(child) @@ -300,24 +234,3 @@ it('keeps a goal waiting on a starting Codex chat through a re-attach, then sets expect(await goal).toMatchObject({ ok: true }) expect(child.requests().some(({ method }) => method === 'thread/goal/set')).toBe(true) }) - -it('answers a goal asked of an ACP agent, which has none, without waiting out its start', async () => { - const spec = ACP_LAUNCH_SPECS[0]! - const child = acpScriptedChild(spec.agent)() - const host = await openChat(child, acpStructuredAgentDefinition(spec)) - const change = { kind: 'set' as const, objective: 'Ship the release' } - - const goal = host.changeThreadGoal(CALLER, { - envelope: envelope(host, 'agentSession.threadGoal', { change }), - change - }) - expect(await settlesSoon(goal)).toBe(true) - expect(await goal).toMatchObject({ - ok: false, - refusal: { details: { reason: 'goalsUnsupported' } } - }) - child.releaseHandshake() - - expect(await send(host, 'after the goal')).toMatchObject({ ok: true }) - await eventually(() => expect(child.prompts()).toEqual(['after the goal'])) -}) From b9e16a8d9efc56a52739fdbc79d87b81fc964e2a Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 13 Oct 2026 11:18:09 -0700 Subject: [PATCH 10/11] Cover both Pi startup choices in the delivery flow --- src/main/pi/rpc-session-startup.test.ts | 34 ----------- ...tructured-agent-session-pi-startup.test.ts | 56 +++++++++---------- 2 files changed, 25 insertions(+), 65 deletions(-) diff --git a/src/main/pi/rpc-session-startup.test.ts b/src/main/pi/rpc-session-startup.test.ts index e381e875d18b..e39c84ec3457 100644 --- a/src/main/pi/rpc-session-startup.test.ts +++ b/src/main/pi/rpc-session-startup.test.ts @@ -10,40 +10,6 @@ import { afterEach(closePiRpcAdapterFixtures) describe('Pi host awaited open', () => { - it('owns the process before RPC and restores model before effort during open', async () => { - const h = await setup() - const stateReply = Promise.withResolvers() - const { acquired, child } = await restart( - h, - (connection) => { - connection.requestOverride = (command) => - command === 'get_state' && connection.requests.length === 1 - ? stateReply.promise - : command === 'set_thinking_level' - ? Promise.resolve({}) - : undefined - }, - { options: { model: 'anthropic/model-2', effort: 'high' } } - ) - expect(child.requests).toEqual([]) - expect(acquired.process.pid).toBe(4123) - const opening = h.adapter.open({ - sessionId, - fence: 8, - acquisitionGeneration: acquired.acquisitionGeneration ?? null, - signal: new AbortController().signal - }) - expect(child.requests).toEqual(['get_state']) - stateReply.resolve(state) - await expect(opening).resolves.toMatchObject({ - reportedOptions: { model: 'anthropic/model-1' } - }) - expect(child.requests.indexOf('set_model')).toBeLessThan( - child.requests.indexOf('set_thinking_level') - ) - expect(h.lifecycle).not.toHaveBeenCalledWith(expect.objectContaining({ type: 'started' })) - }) - it('lets the host close a child held in the state request without a late open', async () => { const h = await setup() const stateReply = Promise.withResolvers() diff --git a/src/main/runtime/structured-agent-session-pi-startup.test.ts b/src/main/runtime/structured-agent-session-pi-startup.test.ts index 61dbba5ff0be..ca1250b3041a 100644 --- a/src/main/runtime/structured-agent-session-pi-startup.test.ts +++ b/src/main/runtime/structured-agent-session-pi-startup.test.ts @@ -110,41 +110,35 @@ it('attaches Pi at spawn and holds the opening send until the provider answers i expect(script.closes()).toBe(1) }) -it('writes a startup model pick once the Pi child is ready', async () => { - const { script, host, caller, fence } = await setup() - const fields = { key: 'model', value: 'scripted/model-2' } - const picked = host.setOption(caller, { - envelope: { - sessionId: HOST_TEST_SESSION, - clientOperationId: operationId(), - expectedRuntimeFence: fence, - payloadFingerprint: computeAgentSessionPayloadFingerprint({ - method: 'agentSession.setOption', - sessionId: HOST_TEST_SESSION, - fields - }) - }, - ...fields - }) +it('writes startup model and effort picks before a message accepted after them', async () => { + const { script, host, caller } = await setup() + const picked = ( + [ + ['model', 'scripted/model-2'], + ['effort', 'low'] + ] as const + ).map(([key, value]) => + host.setOption(caller, { + envelope: envelope('agentSession.setOption', { key, value }), + key, + value + }) + ) + const body = hostTestMessage('first message') + const sent = host.send(caller, { envelope: envelope('agentSession.send', { body }), body }) expect(script.wire()).not.toContain('set_model') script.releaseHandshake() - expect((await picked).ok).toBe(true) - expect(script.prompts()).toEqual([]) - const body = hostTestMessage('first message') - const sent = await host.send(caller, { - envelope: envelope('agentSession.send', { body }), - body - }) - expect(sent.ok).toBe(true) + expect((await Promise.all(picked)).every((result) => result.ok)).toBe(true) + expect((await sent).ok).toBe(true) await vi.waitFor(() => expect(script.prompts()).toEqual(['first message'])) const wire = script.wire() - expect(wire).toContain('set_model') - const { sessions, serialize } = host.collaboratorsForTests() - await vi.waitFor(() => expect(sessions.get(HOST_TEST_SESSION)?.child?.phase).toBe('ready')) - await serialize(HOST_TEST_SESSION, async () => {}) - expect(peekOpenedAgentSessionRecordStore()?.getRecord(HOST_TEST_SESSION)?.options?.model).toBe( - fields.value - ) + expect(wire.indexOf('set_model')).toBeGreaterThanOrEqual(0) + expect(wire.indexOf('set_thinking_level')).toBeGreaterThan(wire.indexOf('set_model')) + expect(wire.indexOf('prompt')).toBeGreaterThan(wire.indexOf('set_thinking_level')) + expect(peekOpenedAgentSessionRecordStore()?.getRecord(HOST_TEST_SESSION)?.options).toMatchObject({ + model: 'scripted/model-2', + effort: 'low' + }) }) it('ends Pi on Stop and resumes its real session file when the next send starts a child', async () => { From a826ea9311dfd18ef8e197e1634eac0e53718b02 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 13 Oct 2026 11:21:12 -0700 Subject: [PATCH 11/11] Match the Pi prompt entry recorded by the scripted wire --- src/main/runtime/structured-agent-session-pi-startup.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/runtime/structured-agent-session-pi-startup.test.ts b/src/main/runtime/structured-agent-session-pi-startup.test.ts index ca1250b3041a..ce452f7367a0 100644 --- a/src/main/runtime/structured-agent-session-pi-startup.test.ts +++ b/src/main/runtime/structured-agent-session-pi-startup.test.ts @@ -134,7 +134,7 @@ it('writes startup model and effort picks before a message accepted after them', const wire = script.wire() expect(wire.indexOf('set_model')).toBeGreaterThanOrEqual(0) expect(wire.indexOf('set_thinking_level')).toBeGreaterThan(wire.indexOf('set_model')) - expect(wire.indexOf('prompt')).toBeGreaterThan(wire.indexOf('set_thinking_level')) + expect(wire.indexOf('prompt:first message')).toBeGreaterThan(wire.indexOf('set_thinking_level')) expect(peekOpenedAgentSessionRecordStore()?.getRecord(HOST_TEST_SESSION)?.options).toMatchObject({ model: 'scripted/model-2', effort: 'low'