diff --git a/config/scripts/structured-lease-renewal-benchmark.ts b/config/scripts/structured-lease-renewal-benchmark.ts new file mode 100644 index 000000000000..a88ffb1c1036 --- /dev/null +++ b/config/scripts/structured-lease-renewal-benchmark.ts @@ -0,0 +1,108 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { performance } from 'node:perf_hooks' +import { AgentSessionRecordStore } from '../../src/main/runtime/agent-session-record-store' +import { readProcessStartTimesMs } from '../../src/main/runtime/agent-session-process-identity-probe' +import { StructuredAgentSessionLeaseRenewer } from '../../src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer' + +const RECORD_COUNT = 50 +const NOW = 1_800_000_000_000 + +async function seedStore(store: AgentSessionRecordStore, directory: string): Promise { + for (let index = 0; index < RECORD_COUNT; index += 1) { + const sessionId = `benchmark-session-${index}` + const spawnToken = `benchmark-spawn-${index}` + const reserved = await store.reserveOwner({ + sessionId, + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'benchmark-workspace', + workspaceKind: 'folder' + }, + provider: 'codex', + accountHome: { variable: 'CODEX_HOME', path: directory }, + runtimeKind: 'native', + expectedFence: null, + spawnToken, + claimKeyId: 'benchmark-key', + handoffOperationId: null, + probe: { outcome: 'reservation-unused' }, + operation: { + callerKey: 'benchmark', + operationId: `${NOW}-${index.toString(16).padStart(32, '0')}`, + fingerprint: `create-${index}` + }, + now: NOW + }) + const fence = reserved.record.lease.runtimeFence + await store.commitProcessIdentity({ + sessionId, + fence, + process: { + hostId: 'local', + pid: process.pid, + processStartTimeMs: null, + spawnToken + }, + now: NOW + }) + await store.proveOwner({ + sessionId, + fence, + link: { + linkId: `benchmark-link-${index}`, + handle: { provider: 'codex', threadId: `benchmark-thread-${index}` }, + origin: 'created', + mintedAtFence: fence, + observedAt: NOW + }, + now: NOW + }) + } +} + +async function main(): Promise { + const directory = await mkdtemp(join(tmpdir(), 'orca-lease-renewal-benchmark-')) + try { + const store = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + await seedStore(store, directory) + const renewer = new StructuredAgentSessionLeaseRenewer({ + store, + probe: async () => ({ outcome: 'identity-matched', matchedOn: ['spawn-token'] }), + now: () => NOW + 10_000 + }) + const startedAt = performance.now() + await renewer.renewNow() + const elapsedMs = performance.now() - startedAt + const renewed = store + .listRecords() + .filter((record) => record.lease.lastRenewedAt === NOW + 10_000).length + const processProbeStartedAt = performance.now() + const processStartTimes = await readProcessStartTimesMs( + Array.from({ length: RECORD_COUNT }, () => process.pid), + 'darwin' + ) + console.log( + JSON.stringify({ + records: RECORD_COUNT, + renewed, + elapsedMs, + processTableElapsedMs: performance.now() - processProbeStartedAt, + processTableMatches: + processStartTimes.get(process.pid) === null || + processStartTimes.get(process.pid) === undefined + ? 0 + : RECORD_COUNT + }) + ) + } finally { + await rm(directory, { recursive: true, force: true }) + } +} + +void main().catch((error: unknown) => { + console.error(error) + process.exitCode = 1 +}) diff --git a/mobile/src/session/mobile-structured-session-hold.test.ts b/mobile/src/session/mobile-structured-session-hold.test.ts new file mode 100644 index 000000000000..0a4c59717064 --- /dev/null +++ b/mobile/src/session/mobile-structured-session-hold.test.ts @@ -0,0 +1,63 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import { useMobileStructuredSessionHold } from './mobile-structured-session-hold' + +function renderHold(props: { client: RpcClient | null; sessionId: string | null }): { + renderer: ReactTestRenderer +} { + function Probe(): null { + useMobileStructuredSessionHold(props) + return null + } + let renderer!: ReactTestRenderer + act(() => { + renderer = create(createElement(Probe)) + }) + return { renderer } +} + +function client(sendRequest: ReturnType): RpcClient { + return { sendRequest } as unknown as RpcClient +} + +let rendered: ReactTestRenderer | null = null + +afterEach(() => { + act(() => rendered?.unmount()) + rendered = null +}) + +describe('a paired session view', () => { + it('holds the session while it is open and releases it when it closes', async () => { + const sendRequest = vi.fn(async () => ({ ok: true, result: {} })) + const { renderer } = renderHold({ client: client(sendRequest), sessionId: 'session-alpha' }) + + const hold = sendRequest.mock.calls.find(([method]) => method === 'agentSession.hold') + expect(hold?.[1]).toMatchObject({ sessionId: 'session-alpha' }) + + await act(async () => renderer.unmount()) + + const release = sendRequest.mock.calls.find(([method]) => method === 'agentSession.release') + expect(release?.[1]).toEqual(hold?.[1]) + }) + + it('asks for nothing before there is a session to hold', () => { + const sendRequest = vi.fn(async () => ({ ok: true, result: {} })) + rendered = renderHold({ client: client(sendRequest), sessionId: null }).renderer + + expect(sendRequest).not.toHaveBeenCalled() + }) + + // An older host answers method_not_found; a chat that cannot be held is still a chat that reads. + it('survives a host that does not know the method', () => { + const sendRequest = vi.fn(async () => { + throw new Error('method_not_found') + }) + + expect(() => { + rendered = renderHold({ client: client(sendRequest), sessionId: 'session-alpha' }).renderer + }).not.toThrow() + }) +}) diff --git a/mobile/src/session/mobile-structured-session-hold.ts b/mobile/src/session/mobile-structured-session-hold.ts new file mode 100644 index 000000000000..8dd165352f37 --- /dev/null +++ b/mobile/src/session/mobile-structured-session-hold.ts @@ -0,0 +1,37 @@ +// The paired session view telling the host it is looking at this chat. +// +// Same contract as the desktop chat's hold, over the mobile transport. It matters more here: a +// phone is the surface most likely to disappear without saying anything, so the host's own +// connection-close backstop is what usually ends up releasing this — the explicit release only +// covers the polite case of navigating away with the socket still up. +// +// A host that predates the method answers `method_not_found`; the session still reads. + +import { useEffect } from 'react' +import type { RpcClient } from '../transport/rpc-client' + +let holderOrdinal = 0 + +export function useMobileStructuredSessionHold(args: { + client: RpcClient | null + sessionId: string | null +}): void { + const { client, sessionId } = args + useEffect(() => { + if (!client || !sessionId) { + return + } + holderOrdinal += 1 + const holderId = `mobile-session:${holderOrdinal}` + // Chained, not raced: unmounting during the hold's round trip would otherwise release a hold + // that has not been taken yet. + const held = client + .sendRequest('agentSession.hold', { sessionId, holderId }) + .catch(() => undefined) + return () => { + void held.then(() => + client.sendRequest('agentSession.release', { sessionId, holderId }).catch(() => undefined) + ) + } + }, [client, sessionId]) +} diff --git a/mobile/src/session/use-mobile-structured-agent-session.ts b/mobile/src/session/use-mobile-structured-agent-session.ts index 7cabb9f4429e..95af02e18720 100644 --- a/mobile/src/session/use-mobile-structured-agent-session.ts +++ b/mobile/src/session/use-mobile-structured-agent-session.ts @@ -14,6 +14,7 @@ import { type StructuredAgentSessionState } from '../../../src/shared/structured-agent-session-reducer' import type { RpcClient } from '../transport/rpc-client' +import { useMobileStructuredSessionHold } from './mobile-structured-session-hold' import { createMobileStructuredReconnectState, noteStructuredBackground, @@ -58,6 +59,8 @@ export function useMobileStructuredAgentSession(args: { loadOlder: () => Promise } { const { client, sessionId } = args + // Declared before the read: the hold is what gives a restored session its provider child back. + useMobileStructuredSessionHold({ client, sessionId }) const [state, dispatch] = useReducer(reduceStructuredAgentSession, EMPTY_STRUCTURED_AGENT_SESSION) const stateRef = useRef(state) const [loadingOlder, setLoadingOlder] = useState(false) diff --git a/mobile/src/transport/mobile-e2ee-v2-physical-channel.test.ts b/mobile/src/transport/mobile-e2ee-v2-physical-channel.test.ts index a5c3db331227..8f02f8372d5a 100644 --- a/mobile/src/transport/mobile-e2ee-v2-physical-channel.test.ts +++ b/mobile/src/transport/mobile-e2ee-v2-physical-channel.test.ts @@ -143,7 +143,7 @@ describe('mobile E2EE v2 physical channel', () => { expect(JSON.parse(new TextDecoder().decode(capabilities!))).toEqual({ type: 'e2ee_client_capabilities', v: 1, - clientCapabilities: ['agent-session.structured.v1'] + clientCapabilities: ['agent-session.structured.v1', 'agent-session.structured.hold.v1'] }) expect(ctx.onAuthenticated).toHaveBeenCalledOnce() expect(ctx.onError).not.toHaveBeenCalled() diff --git a/mobile/src/transport/mobile-e2ee-v2-physical-channel.ts b/mobile/src/transport/mobile-e2ee-v2-physical-channel.ts index 40e0395cba0b..ce8f980bbff2 100644 --- a/mobile/src/transport/mobile-e2ee-v2-physical-channel.ts +++ b/mobile/src/transport/mobile-e2ee-v2-physical-channel.ts @@ -2,7 +2,10 @@ import { createWsOutboundBackpressureQueue, type WsOutboundBackpressureQueue } from '../../../src/shared/ws-outbound-backpressure-queue' -import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' +import { + STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY, + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY +} from '../../../src/shared/protocol-version' import type { MobileE2EEV2ClientSession } from './mobile-e2ee-v2-client-session' type ChannelState = 'awaiting-ready' | 'awaiting-authenticated' | 'ready' @@ -117,7 +120,10 @@ export class MobileE2EEV2PhysicalChannel { plaintext: JSON.stringify({ type: 'e2ee_client_capabilities', v: 1, - clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] + clientCapabilities: [ + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, + STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY + ] }) }) this.args.onAuthenticated() diff --git a/mobile/src/transport/rpc-client-contract.ts b/mobile/src/transport/rpc-client-contract.ts index 9461743950db..6104174cce70 100644 --- a/mobile/src/transport/rpc-client-contract.ts +++ b/mobile/src/transport/rpc-client-contract.ts @@ -36,6 +36,10 @@ export type RpcClient = { getState: () => ConnectionState getReconnectAttempt: () => number getLastConnectedAt: () => number | null + // Wall-clock stamp of the last inbound frame on the current session, or null when the + // transport can't vouch for one. Optional so older/foreign RpcClient shapes stay valid; + // callers must treat absent/null as "unknown" and fall back to a safe bound. + getLastInboundAt?: () => number | null onStateChange: (listener: (state: ConnectionState) => void) => () => void notifyForeground: (reason?: ForegroundNudgeReason) => void restartAfterStructuredBackground?: () => void diff --git a/mobile/src/transport/rpc-client-structured-session.test.ts b/mobile/src/transport/rpc-client-structured-session.test.ts index 857f50f42653..c4741a6ae6f1 100644 --- a/mobile/src/transport/rpc-client-structured-session.test.ts +++ b/mobile/src/transport/rpc-client-structured-session.test.ts @@ -99,7 +99,7 @@ describe('structured session RPC transport', () => { expect(auth).toEqual({ type: 'e2ee_auth', deviceToken: 'token', - clientCapabilities: ['agent-session.structured.v1'] + clientCapabilities: ['agent-session.structured.v1', 'agent-session.structured.hold.v1'] }) client.close() }) diff --git a/mobile/src/transport/rpc-client.ts b/mobile/src/transport/rpc-client.ts index 4800bc209ef9..c0a7ab838480 100644 --- a/mobile/src/transport/rpc-client.ts +++ b/mobile/src/transport/rpc-client.ts @@ -46,7 +46,10 @@ import { } from './rpc-session-liveness-watchdog' import { isStaleForegroundDial } from './rpc-stale-dial' import { websocketPayloadToUint8 } from './websocket-payload-bytes' -import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' +import { + STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY, + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY +} from '../../../src/shared/protocol-version' import type { RpcClient, SendRequestOptions, SubscribeOptions } from './rpc-client-contract' export type { RpcClient, SendRequestOptions } from './rpc-client-contract' @@ -374,7 +377,10 @@ export function connect( sendEncrypted({ type: 'e2ee_auth', deviceToken, - clientCapabilities: [STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY] + clientCapabilities: [ + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, + STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY + ] }) return } diff --git a/src/cli/handlers/orchestration-worker-cli.test.ts b/src/cli/handlers/orchestration-worker-cli.test.ts index d19666556c69..cd48e0f4c32f 100644 --- a/src/cli/handlers/orchestration-worker-cli.test.ts +++ b/src/cli/handlers/orchestration-worker-cli.test.ts @@ -164,6 +164,52 @@ describe('orchestration worker-start CLI contract', () => { expect(process.exitCode).toBe(1) }) + it('prints the Structured Chat recovery action for a refused worker start', async () => { + callMock.mockResolvedValue({ + result: { + taskId: 'task_1', + dispatchId: 'ctx_1', + state: 'failed', + failedStage: 'dispatch_input', + lastError: + 'The target terminal is in Structured Chat. Switch it to Terminal, then retry `orca orchestration worker-start`.', + effects: [], + residualResources: [] + } + }) + + await ORCHESTRATION_HANDLERS['orchestration worker-start']({ + flags: new Map([ + ['task', 'task_1'], + ['terminal', 'term_worker'], + ['from', 'term_coord'] + ]), + client: { call: callMock }, + cwd: '/tmp/repo', + json: false + } as never) + + const formatter = vi.mocked(printResult).mock.calls[0]?.[2] as + | ((result: { + taskId: string + dispatchId: string + state: string + failedStage?: string + lastError?: string + }) => string) + | undefined + expect( + formatter?.({ + taskId: 'task_1', + dispatchId: 'ctx_1', + state: 'failed', + failedStage: 'dispatch_input', + lastError: + 'The target terminal is in Structured Chat. Switch it to Terminal, then retry `orca orchestration worker-start`.' + }) + ).toMatch(/Structured Chat.*Switch it to Terminal.*orca orchestration worker-start/s) + }) + it('prints a reveal warning for a live background worker', async () => { callMock.mockResolvedValue({ result: { diff --git a/src/cli/handlers/terminal.test.ts b/src/cli/handlers/terminal.test.ts index 7d08658fc660..18832ba28a6f 100644 --- a/src/cli/handlers/terminal.test.ts +++ b/src/cli/handlers/terminal.test.ts @@ -5,6 +5,8 @@ import { printHelp } from '../help' import { COMMAND_SPECS } from '../specs' import { TERMINAL_HANDLERS } from './terminal' +const ORIGINAL_EXIT_CODE = process.exitCode + describe('terminal close CLI', () => { afterEach(() => { vi.restoreAllMocks() @@ -65,6 +67,7 @@ describe('terminal close CLI', () => { describe('terminal send CLI', () => { afterEach(() => { vi.restoreAllMocks() + process.exitCode = ORIGINAL_EXIT_CODE }) it('marks combined text and Enter as an agent prompt candidate', async () => { @@ -94,6 +97,44 @@ describe('terminal send CLI', () => { }) }) + it('explains that Structured Chat blocked a refused send and how to recover', async () => { + const call = vi.fn().mockResolvedValue({ + result: { + send: { + handle: 'term-1', + accepted: false, + bytesWritten: 0, + agentSessionRefusal: { + code: 'agent_session_conflict', + sessionId: 'session-1', + ownerRuntimeKind: 'native', + handoffStage: null, + ownerPid: 4242, + runtimeFence: 7 + } + } + } + }) + vi.spyOn(console, 'log').mockImplementation(() => {}) + process.exitCode = undefined + + await TERMINAL_HANDLERS['terminal send']({ + flags: new Map([ + ['terminal', 'term-1'], + ['text', 'review'], + ['enter', true] + ]), + client: { call } as unknown as RuntimeClient, + cwd: '/tmp/worktree', + json: false + }) + + expect(console.log).toHaveBeenCalledWith( + expect.stringMatching(/Structured Chat.*Switch it to Terminal.*orca terminal send/s) + ) + expect(process.exitCode).toBe(1) + }) + it('keeps text-only and bare Enter sends as direct terminal input', async () => { const call = vi.fn().mockResolvedValue({ result: { send: { handle: 'term-1', accepted: true, bytesWritten: 1 } } diff --git a/src/cli/handlers/terminal.ts b/src/cli/handlers/terminal.ts index 00c0a5cd8077..d0ced262a43b 100644 --- a/src/cli/handlers/terminal.ts +++ b/src/cli/handlers/terminal.ts @@ -115,6 +115,9 @@ export const TERMINAL_HANDLERS: Record = { client: { id: 'orca-cli', type: 'desktop' } }) printResult(result, json, formatTerminalSend) + if (!result.result.send.accepted) { + process.exitCode = 1 + } }, 'terminal wait': async ({ flags, client, cwd, json }) => { const timeoutMs = getOptionalPositiveIntegerFlag(flags, 'timeout-ms') diff --git a/src/cli/terminal-format.ts b/src/cli/terminal-format.ts index 806a39cb588b..017d2f3b923c 100644 --- a/src/cli/terminal-format.ts +++ b/src/cli/terminal-format.ts @@ -1,4 +1,5 @@ import { PTY_LIVE_NOTE, describeUnconfirmedStop } from '../shared/pty-liveness-verdict' +import { structuredChatPtyWriteRefusalCopy } from '../shared/agent-session-pty-write-refusal-copy' import type { RuntimeTerminalClose, RuntimeTerminalCreate, @@ -180,6 +181,12 @@ function formatTerminalReadLimitedWarning(terminal: RuntimeTerminalRead): string } export function formatTerminalSend(result: { send: RuntimeTerminalSend }): string { + if (result.send.agentSessionRefusal) { + const copy = structuredChatPtyWriteRefusalCopy(result.send.agentSessionRefusal, 'terminal-send') + if (copy) { + return copy + } + } return `Sent ${result.send.bytesWritten} bytes to ${result.send.handle}.` } diff --git a/src/main/codex/codex-app-server-connection.test.ts b/src/main/codex/codex-app-server-connection.test.ts index 8687830d0623..fecf816961cb 100644 --- a/src/main/codex/codex-app-server-connection.test.ts +++ b/src/main/codex/codex-app-server-connection.test.ts @@ -321,11 +321,10 @@ describe('openCodexAppServerConnection', () => { await vi.advanceTimersByTimeAsync(2_000) await closing - expect(child.kill).toHaveBeenCalledWith('SIGKILL') + await vi.waitFor(() => expect(child.kill).toHaveBeenCalledWith('SIGKILL')) }) it('reports unproven close when forced termination did not produce an exit event', async () => { - vi.useFakeTimers() const { child, spawnImpl } = stubChild({ exitOnStdinEnd: false }) answerInitialize(child) const connection = await openCodexAppServerConnection( @@ -334,11 +333,8 @@ describe('openCodexAppServerConnection', () => { spawnImpl ) - const closing = connection.close() - await vi.advanceTimersByTimeAsync(2_600) - - await expect(closing).resolves.toBe(false) - }) + await expect(connection.close()).resolves.toBe(false) + }, 10_000) it('ends the connection rather than buffering an oversized line', async () => { const { child, spawnImpl } = stubChild({ exitOnStdinEnd: false }) @@ -398,7 +394,7 @@ describe('openCodexAppServerConnection', () => { expect((await inFlight).message).toContain('structured sink failed') expect(exits).toEqual([expect.stringContaining('structured sink failed')]) expect(connection.closed).toBe(true) - expect(child.kill).toHaveBeenCalledWith('SIGKILL') + await vi.waitFor(() => expect(child.kill).toHaveBeenCalledWith('SIGKILL')) await connection.close() }) @@ -447,7 +443,7 @@ describe('openCodexAppServerConnection', () => { // A child nobody can write to is not a live session: the owner must see the // connection as gone rather than keep issuing calls that can only time out. expect(connection.closed).toBe(true) - expect(child.kill).toHaveBeenCalledWith('SIGKILL') + await vi.waitFor(() => expect(child.kill).toHaveBeenCalledWith('SIGKILL')) expect((await rejection(connection.request('turn/start'))).message).toContain('EPIPE') await connection.close() }) diff --git a/src/main/codex/codex-app-server-connection.ts b/src/main/codex/codex-app-server-connection.ts index b5c6980921c5..133a521ccee8 100644 --- a/src/main/codex/codex-app-server-connection.ts +++ b/src/main/codex/codex-app-server-connection.ts @@ -1,12 +1,14 @@ import { spawnProcess } from '../../shared/child-process/run-process' import { buildCodexAppServerExitError } from './codex-app-server-exit-error' import { isAppServerRecord, parseCodexAppServerJsonLine } from './codex-app-server-jsonl' +import { terminateCodexAppServerProcessTree } from './codex-app-server-process-teardown' +import { CodexAppServerRequestError } from './codex-app-server-request-error' +import { CODEX_SPAWN_TOKEN_ENV } from './codex-structured-owner-identity' import { waitForProcessExitUntil } from './codex-process-exit-deadline' import { CodexAppServerTimeoutError, CodexAppServerUnsupportedError, - isCodexMethodNotFoundError, - killCodexAppServerProcessTree + isCodexMethodNotFoundError } from './codex-app-server-session' import type { CodexAppServerConnection, @@ -18,27 +20,14 @@ export type { CodexAppServerConnectionHandlers, CodexAppServerServerRequest } from './codex-app-server-connection-types' +export { + CodexAppServerRequestError, + isCodexAppServerRequestError +} from './codex-app-server-request-error' // Structured chat needs a persistent bidirectional child and per-request deadlines; // the request-scoped app-server runner cannot carry approvals or streamed turns. -/** Codex answered the call and refused it. Distinct from a timeout or a dead - * child, which leave the call unsettled rather than declined. */ -export class CodexAppServerRequestError extends Error { - constructor( - readonly method: string, - readonly code: number | null, - message: string - ) { - super(message) - this.name = 'CodexAppServerRequestError' - } -} - -export function isCodexAppServerRequestError(error: unknown): error is CodexAppServerRequestError { - return error instanceof Error && error.name === 'CodexAppServerRequestError' -} - export type CodexAppServerLaunch = { command: string args: string[] @@ -82,6 +71,7 @@ export async function openCodexAppServerConnection( env: childEnv, cwd: process.cwd() }) + const spawnToken = launch.env?.[CODEX_SPAWN_TOKEN_ENV] const pending = new Map() let stderrTail = '' @@ -150,7 +140,7 @@ export async function openCodexAppServerConnection( failPending(error) return } - killCodexAppServerProcessTree(child) + void terminateCodexAppServerProcessTree(child, spawnToken) handleUnexpectedEnd(error) }) @@ -206,7 +196,7 @@ export async function openCodexAppServerConnection( stdoutBuffer += chunk if (Buffer.byteLength(stdoutBuffer) > STDOUT_LINE_MAX_BYTES) { child.stdout.destroy() - killCodexAppServerProcessTree(child) + void terminateCodexAppServerProcessTree(child, spawnToken) handleUnexpectedEnd(new Error('codex app-server emitted an oversized JSONL line')) return } @@ -226,7 +216,7 @@ export async function openCodexAppServerConnection( dispatchMessage(parsed) } catch (error) { child.stdout.destroy() - killCodexAppServerProcessTree(child) + void terminateCodexAppServerProcessTree(child, spawnToken) handleUnexpectedEnd(error instanceof Error ? error : new Error(String(error))) return } @@ -306,7 +296,11 @@ export async function openCodexAppServerConnection( if (!exited) { await waitForProcessExitUntil(exitPromise, GRACEFUL_EXIT_MS) if (!exited) { - killCodexAppServerProcessTree(child) + const treeExited = await terminateCodexAppServerProcessTree(child, spawnToken) + if (!treeExited) { + failPending(new Error('codex app-server process-tree exit was not proven')) + return false + } await waitForProcessExitUntil(exitPromise, FORCED_EXIT_MS) } } diff --git a/src/main/codex/codex-app-server-process-teardown.test.ts b/src/main/codex/codex-app-server-process-teardown.test.ts new file mode 100644 index 000000000000..aca0b9f5a62e --- /dev/null +++ b/src/main/codex/codex-app-server-process-teardown.test.ts @@ -0,0 +1,87 @@ +import type { ChildProcess } from 'node:child_process' +import { describe, expect, it, vi } from 'vitest' +import { terminateCodexAppServerProcessTree } from './codex-app-server-process-teardown' + +function child() { + return { + pid: 1234, + kill: vi.fn(() => true) as ChildProcess['kill'] + } +} + +describe('terminateCodexAppServerProcessTree', () => { + it('waits for the Windows tree kill before releasing the wrapper', async () => { + const target = child() + const release = Promise.withResolvers() + const terminateWindowsTree = vi.fn(() => release.promise) + + const teardown = terminateCodexAppServerProcessTree(target, undefined, { + platform: 'win32', + terminateWindowsTree + }) + expect(target.kill).not.toHaveBeenCalled() + release.resolve() + await teardown + + expect(terminateWindowsTree).toHaveBeenCalledWith(1234) + expect(target.kill).toHaveBeenCalledWith('SIGKILL') + }) + + it('kills exact Linux spawn-token PIDs before the recorded wrapper', async () => { + const target = child() + const findSpawnTokenProcesses = vi + .fn<() => Promise>() + .mockResolvedValueOnce([1234, 2345, 3456]) + .mockResolvedValueOnce([1234]) + .mockResolvedValueOnce([1234]) + const signalPid = vi.fn() + + await expect( + terminateCodexAppServerProcessTree(target, 'spawn-1', { + platform: 'linux', + findSpawnTokenProcesses, + signalPid, + isPidPresent: () => false, + wait: async () => undefined + }) + ).resolves.toBe(true) + + expect(signalPid.mock.calls).toEqual([ + [2345, 'SIGKILL'], + [3456, 'SIGKILL'] + ]) + expect(target.kill).toHaveBeenCalledTimes(1) + expect(target.kill).toHaveBeenCalledWith('SIGKILL') + }) + + it('keeps the wrapper reachable when Linux cannot prove descendant exit', async () => { + const target = child() + + await expect( + terminateCodexAppServerProcessTree(target, 'spawn-1', { + platform: 'linux', + findSpawnTokenProcesses: async () => null + }) + ).resolves.toBe(false) + + expect(target.kill).not.toHaveBeenCalled() + }) + + it('waits for an owned POSIX snapshot before killing the wrapper', async () => { + const target = child() + const snapshot = { rootPgid: 1234, descendants: [], capturedAtMs: 1 } + const release = Promise.withResolvers() + + const teardown = terminateCodexAppServerProcessTree(target, undefined, { + platform: 'darwin', + captureDescendants: async () => snapshot, + terminateDescendants: () => release.promise + }) + await vi.waitFor(() => expect(target.kill).toHaveBeenCalledWith('SIGSTOP')) + expect(target.kill).not.toHaveBeenCalledWith('SIGKILL') + release.resolve(true) + await teardown + + expect(target.kill).toHaveBeenLastCalledWith('SIGKILL') + }) +}) diff --git a/src/main/codex/codex-app-server-process-teardown.ts b/src/main/codex/codex-app-server-process-teardown.ts new file mode 100644 index 000000000000..c7c417326d74 --- /dev/null +++ b/src/main/codex/codex-app-server-process-teardown.ts @@ -0,0 +1,158 @@ +import type { ChildProcess } from 'node:child_process' +import { captureDescendantSnapshot, type DescendantSnapshot } from '../pty-descendant-termination' +import { terminateDescendantSnapshotAndWait } from '../pty-descendant-exit-verification' +import { findAgentSessionSpawnTokenProcesses } from '../runtime/agent-session-spawn-token-process-scan' +import { terminateWindowsProcessTree } from '../windows-process-tree-kill' + +const TOKEN_PROCESS_EXIT_TIMEOUT_MS = 3_500 +const TOKEN_PROCESS_POLL_MS = 25 +const activeTeardowns = new WeakMap>() + +type TeardownChild = Pick + +export type CodexAppServerProcessTeardownDeps = { + platform?: NodeJS.Platform + findSpawnTokenProcesses?: (spawnToken: string) => Promise + captureDescendants?: (rootPid: number) => Promise + terminateDescendants?: (snapshot: DescendantSnapshot) => Promise + terminateWindowsTree?: (rootPid: number) => Promise + signalPid?: (pid: number, signal: NodeJS.Signals) => void + isPidPresent?: (pid: number) => boolean + wait?: (ms: number) => Promise + now?: () => number +} + +function sendSignal(pid: number, signal: NodeJS.Signals): void { + try { + process.kill(pid, signal) + } catch { + // An already-gone exact PID is the desired outcome. + } +} + +function wait(ms: number): Promise { + return new Promise((resolve) => { + const timer = setTimeout(resolve, ms) + timer.unref?.() + }) +} + +function isPidPresent(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } catch (error) { + return (error as NodeJS.ErrnoException).code !== 'ESRCH' + } +} + +async function terminateSpawnTokenProcesses( + rootPid: number, + spawnToken: string, + deps: CodexAppServerProcessTeardownDeps +): Promise { + const find = deps.findSpawnTokenProcesses ?? findAgentSessionSpawnTokenProcesses + const signal = deps.signalPid ?? sendSignal + const pidPresent = deps.isPidPresent ?? isPidPresent + const delay = deps.wait ?? wait + const now = deps.now ?? Date.now + const deadline = now() + TOKEN_PROCESS_EXIT_TIMEOUT_MS + const signalled = new Set() + let emptyScans = 0 + + while (now() < deadline) { + const pids = await find(spawnToken).catch(() => null) + if (pids === null) { + return false + } + const descendants = pids.filter((pid) => pid !== rootPid) + for (const pid of descendants) { + signalled.add(pid) + signal(pid, 'SIGKILL') + } + const allSignalledPidsGone = [...signalled].every((pid) => !pidPresent(pid)) + emptyScans = descendants.length === 0 && allSignalledPidsGone ? emptyScans + 1 : 0 + if (emptyScans >= 2) { + return true + } + await delay(TOKEN_PROCESS_POLL_MS) + } + const remaining = await find(spawnToken).catch(() => null) + return ( + remaining !== null && + remaining.every((pid) => pid === rootPid) && + [...signalled].every((pid) => !pidPresent(pid)) + ) +} + +async function terminatePosixTree( + child: TeardownChild, + rootPid: number, + spawnToken: string | undefined, + deps: CodexAppServerProcessTeardownDeps +): Promise { + const platform = deps.platform ?? process.platform + if (platform === 'linux' && spawnToken) { + const descendantsExited = await terminateSpawnTokenProcesses(rootPid, spawnToken, deps) + if (descendantsExited) { + child.kill('SIGKILL') + } + return descendantsExited + } + child.kill('SIGSTOP') + const capture = deps.captureDescendants ?? captureDescendantSnapshot + const snapshot = await capture(rootPid).catch(() => null) + if (!snapshot) { + child.kill('SIGKILL') + return true + } + const terminate = deps.terminateDescendants ?? terminateDescendantSnapshotAndWait + const descendantsExited = await terminate(snapshot) + if (!descendantsExited) { + child.kill('SIGCONT') + return false + } + child.kill('SIGKILL') + return true +} + +/** Stops every process owned by one app-server launch before releasing its wrapper. */ +async function terminateOnce( + child: TeardownChild, + spawnToken: string | undefined, + deps: CodexAppServerProcessTeardownDeps +): Promise { + const rootPid = child.pid + if (!rootPid) { + child.kill('SIGKILL') + return false + } + if ((deps.platform ?? process.platform) === 'win32') { + const terminate = deps.terminateWindowsTree ?? terminateWindowsProcessTree + await terminate(rootPid) + // taskkill owns the tree; this preserves the prior direct-child fallback when it fails. + child.kill('SIGKILL') + return true + } + return terminatePosixTree(child, rootPid, spawnToken, deps) +} + +export function terminateCodexAppServerProcessTree( + child: TeardownChild, + spawnToken?: string, + deps: CodexAppServerProcessTeardownDeps = {} +): Promise { + const key = child as object + const active = activeTeardowns.get(key) + if (active) { + return active + } + const attempt = terminateOnce(child, spawnToken, deps).catch(() => false) + activeTeardowns.set(key, attempt) + void attempt.then(() => { + if (activeTeardowns.get(key) === attempt) { + activeTeardowns.delete(key) + } + }) + return attempt +} diff --git a/src/main/codex/codex-app-server-request-error.ts b/src/main/codex/codex-app-server-request-error.ts new file mode 100644 index 000000000000..0dbefaca9655 --- /dev/null +++ b/src/main/codex/codex-app-server-request-error.ts @@ -0,0 +1,15 @@ +/** Codex answered the call and refused it, rather than timing out or exiting. */ +export class CodexAppServerRequestError extends Error { + constructor( + readonly method: string, + readonly code: number | null, + message: string + ) { + super(message) + this.name = 'CodexAppServerRequestError' + } +} + +export function isCodexAppServerRequestError(error: unknown): error is CodexAppServerRequestError { + return error instanceof Error && error.name === 'CodexAppServerRequestError' +} diff --git a/src/main/codex/codex-app-server-teardown.integration.test.ts b/src/main/codex/codex-app-server-teardown.integration.test.ts new file mode 100644 index 000000000000..fa8cbf12b6c9 --- /dev/null +++ b/src/main/codex/codex-app-server-teardown.integration.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from 'vitest' +import { CODEX_SPAWN_TOKEN_ENV } from './codex-structured-owner-identity' +import { + openCodexAppServerConnection, + type CodexAppServerConnection +} from './codex-app-server-connection' + +const ITERATIONS = 40 + +const FORCE_KILL_APP_SERVER = String.raw` + const { spawn } = require('node:child_process') + const readline = require('node:readline') + const descendant = spawn(process.execPath, ['-e', 'setInterval(() => {}, 60000)'], { + stdio: 'ignore' + }) + const send = (payload) => process.stdout.write(JSON.stringify(payload) + '\n') + readline.createInterface({ input: process.stdin }).on('line', (line) => { + const message = JSON.parse(line) + if (message.method === 'initialize') return send({ id: message.id, result: {} }) + if (message.method === 'initialized') { + send({ method: 'test/descendant', params: { pid: descendant.pid } }) + } + }) + setInterval(() => {}, 60000) +` + +function processExists(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } catch { + return false + } +} + +type RunningServer = { + connection: CodexAppServerConnection + descendantPid: number +} + +async function openServer(iteration: number): Promise { + const descendant = Promise.withResolvers() + const connection = await openCodexAppServerConnection( + { + command: process.execPath, + args: ['-e', FORCE_KILL_APP_SERVER], + env: { [CODEX_SPAWN_TOKEN_ENV]: `teardown-test-${process.pid}-${iteration}` } + }, + { + onNotification: (method, params) => { + if (method === 'test/descendant') { + descendant.resolve((params as { pid: number }).pid) + } + } + } + ) + return { connection, descendantPid: await descendant.promise } +} + +describe.runIf(process.platform === 'linux')('Codex app-server process teardown', () => { + it('reaps the forced-close descendant in 40 consecutive launches', async () => { + const running: RunningServer[] = [] + try { + for (let iteration = 0; iteration < ITERATIONS; iteration += 1) { + running.push(await openServer(iteration)) + } + expect(running.every(({ descendantPid }) => processExists(descendantPid))).toBe(true) + + const closed = await Promise.all(running.map(({ connection }) => connection.close())) + + expect(closed).toEqual(Array.from({ length: ITERATIONS }, () => true)) + expect(running.filter(({ descendantPid }) => processExists(descendantPid))).toEqual([]) + } finally { + for (const { connection, descendantPid } of running) { + await connection.close().catch(() => false) + if (processExists(descendantPid)) { + process.kill(descendantPid, 'SIGKILL') + } + } + } + }, 30_000) +}) diff --git a/src/main/codex/codex-structured-launch-resolution.test.ts b/src/main/codex/codex-structured-launch-resolution.test.ts index 3883930bdf9f..da91e33e215a 100644 --- a/src/main/codex/codex-structured-launch-resolution.test.ts +++ b/src/main/codex/codex-structured-launch-resolution.test.ts @@ -9,6 +9,16 @@ const IDENTITY = { sessionId: SESSION_ID } as Parameters< ReturnType >[0]['identity'] +async function withPlatform(platform: NodeJS.Platform, run: () => Promise): Promise { + const original = process.platform + Object.defineProperty(process, 'platform', { configurable: true, value: platform }) + try { + return await run() + } finally { + Object.defineProperty(process, 'platform', { configurable: true, value: original }) + } +} + function record(overrides: Partial = {}): AgentSessionRecord { return { sessionId: SESSION_ID, @@ -51,6 +61,23 @@ describe('codex structured launch resolution', () => { }) }) + it('passes a Windows .cmd path containing cmd syntax directly to the safe spawn layer', async () => { + const command = String.raw`C:\Users\r&d\npm-prefix\codex.cmd` + + await withPlatform('win32', async () => { + const resolveLaunch = createCodexStructuredLaunchResolver({ + store: { getRecord: () => record() } as unknown as AgentSessionRecordStore, + resolveWorkspacePath: async () => String.raw`C:\workspaces\orca`, + resolveCommand: () => command + }) + + await expect(resolveLaunch({ identity: IDENTITY })).resolves.toMatchObject({ + command, + args: ['app-server'] + }) + }) + }) + it('resumes the last thread this session actually proved, not one a caller names', async () => { const launch = await resolverFor( record({ diff --git a/src/main/codex/codex-structured-launch-resolution.ts b/src/main/codex/codex-structured-launch-resolution.ts index ae68be38d3f3..7426c321a1c1 100644 --- a/src/main/codex/codex-structured-launch-resolution.ts +++ b/src/main/codex/codex-structured-launch-resolution.ts @@ -11,7 +11,6 @@ import { agentSessionProviderHandleChainHead } from '../../shared/agent-session- import { LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host' import { resolveCodexCommand } from '../codex-cli/command' import type { AgentSessionRecordStore } from '../runtime/agent-session-record-store' -import { getSpawnArgsForWindows } from '../win32-utils' import type { CodexStructuredLaunch } from './codex-structured-session-adapter' import { resolvePinnedCodexRolloutProof } from './codex-tui-rollout-proof' @@ -59,12 +58,11 @@ export function createCodexStructuredLaunchResolver( pathEnv, ...(homePath ? { homePath } : {}) }) - const { spawnCmd, spawnArgs } = getSpawnArgsForWindows(command, CODEX_APP_SERVER_ARGS) const head = agentSessionProviderHandleChainHead(record.providerHandleChain) const resumeThreadId = head?.handle.provider === 'codex' ? head.handle.threadId : null return { - command: spawnCmd, - args: spawnArgs, + command, + args: CODEX_APP_SERVER_ARGS, cwd: await deps.resolveWorkspacePath(location.workspaceId), codexHome: accountHome.path, ...(environment ? { env: { ...environment } as Record } : {}), diff --git a/src/main/codex/codex-tui-rollout-proof.test.ts b/src/main/codex/codex-tui-rollout-proof.test.ts index 957e835a655b..aa0588e7e7af 100644 --- a/src/main/codex/codex-tui-rollout-proof.test.ts +++ b/src/main/codex/codex-tui-rollout-proof.test.ts @@ -21,6 +21,17 @@ describe('Codex TUI rollout proof', () => { expect(parseCodexTuiStatusSessionId('Session: not-a-session')).toBeNull() }) + it('parses the Codex 0.148 status screen and semantic thread labels', () => { + const status = [ + '\u001b[2m│ >_ OpenAI Codex (v0.148.0) │', + '│ Model: gpt-5.6-sol (reasoning high, summaries auto) │', + `│ Session: \u001b[22m${THREAD}\u001b[2m │` + ].join('\n') + + expect(parseCodexTuiStatusSessionId(status)).toBe(THREAD) + expect(parseCodexTuiStatusSessionId(`Thread ID: ${OTHER_THREAD}`)).toBe(OTHER_THREAD) + }) + it('uses Kitty Enter only while the TUI negotiated Kitty input', () => { expect(codexTuiStatusProbeInput(0)).toEqual({ command: '\u001b[200~/status\u001b[201~', @@ -151,4 +162,33 @@ describe('Codex TUI rollout proof', () => { '\u001b' ]) }) + + it('accepts a proven blank Codex 0.148 session before its lazy rollout exists', async () => { + let reads = 0 + const write = vi.fn((_data: string) => true) + const resolveRollout = vi.fn(async () => null) + + await expect( + resolveLiveCodexTuiRollout({ + codexHome: '/pinned', + kittyKeyboardFlags: 1, + readOutput: () => ({ + text: + reads++ > 0 + ? `│ >_ OpenAI Codex (v0.148.0) │\n│ Session: \u001b[22m${THREAD}\u001b[2m │` + : '', + lastOutputAt: reads > 1 ? 2 : 1 + }), + write, + resolveRollout, + delay: async () => undefined + }) + ).resolves.toEqual({ threadId: THREAD }) + expect(resolveRollout).toHaveBeenCalledTimes(5) + expect(write.mock.calls.map(([data]) => data)).toEqual([ + '\u001b[200~/status\u001b[201~', + '\u001b[13u', + '\u001b' + ]) + }) }) diff --git a/src/main/codex/codex-tui-rollout-proof.ts b/src/main/codex/codex-tui-rollout-proof.ts index 4572b1d68ba3..9b303d104f92 100644 --- a/src/main/codex/codex-tui-rollout-proof.ts +++ b/src/main/codex/codex-tui-rollout-proof.ts @@ -5,7 +5,10 @@ import { relativePathInsideRoot } from '../../shared/cross-platform-path' import { listCodexSessionJsonlFilesIncrementally } from './codex-session-file-listing' const SESSION_ID_PATTERN = '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}' -const STATUS_SESSION_RE = new RegExp(`\\bSession(?: ID)?\\s*:\\s*(${SESSION_ID_PATTERN})\\b`, 'gi') +const STATUS_SESSION_RE = new RegExp( + `\\b(?:Session|Thread)(?:\\s+ID)?\\s*:\\s*(${SESSION_ID_PATTERN})\\b`, + 'gi' +) const ROLLOUT_READ_LIMIT = 64 * 1024 const STATUS_COMMAND_PASTE = '\u001b[200~/status\u001b[201~' const KITTY_ENTER = '\u001b[13u' @@ -125,7 +128,7 @@ export async function resolveLiveCodexTuiRollout(input: { timeoutMs?: number resolveRollout?: (codexHome: string, threadId: string) => Promise delay?: (ms: number) => Promise -}): Promise<{ threadId: string; transcriptPath: string }> { +}): Promise<{ threadId: string; transcriptPath?: string }> { const baselineOutputAt = input.readOutput().lastOutputAt const probe = codexTuiStatusProbeInput(input.kittyKeyboardFlags) if (!input.write(probe.command)) { @@ -144,14 +147,19 @@ export async function resolveLiveCodexTuiRollout(input: { const threadId = parseCodexTuiStatusSessionId(output.text) if (threadId) { const resolveRollout = input.resolveRollout ?? resolvePinnedCodexRolloutProof - const transcriptPath = await resolveRollout(input.codexHome, threadId) + let transcriptPath: string | null = null + // Codex 0.148 allocates a session before it writes a rollout; give a just-written + // file a short visibility window without rejecting a genuinely blank conversation. + for (let attempt = 0; attempt < 5 && !transcriptPath; attempt += 1) { + transcriptPath = await resolveRollout(input.codexHome, threadId) + if (!transcriptPath && attempt < 4) { + await delay(100) + } + } if (!input.write('\u001b')) { throw new Error('The agent terminal could not finish Codex session verification.') } - if (!transcriptPath) { - throw new Error('Could not find the rollout for this Codex conversation.') - } - return { threadId, transcriptPath } + return { threadId, ...(transcriptPath ? { transcriptPath } : {}) } } } await delay(100) @@ -178,12 +186,20 @@ async function readCodexRolloutSessionMetaId(filePath: string): Promise 0 ? id : null } catch { return null diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-context.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-context.ts index f7438d43ca6f..7113be8d54bc 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-context.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-context.ts @@ -1,19 +1,32 @@ -// What attach needs from the host, named so the host cannot grow attach's dependencies unnoticed. +// What attaching needs from the host, named explicitly. +// +// Passing the host itself would let this quietly grow new dependencies; an explicit context makes +// each one a deliberate addition and keeps the orchestration testable without constructing a host. import type { AgentSessionWireRefusal } from '../../../shared/agent-session-wire' -import type { AgentSessionSubscribers } from './structured-agent-session-subscribers' -import type { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' +import type { AgentJournalResetReason } from '../../../shared/agent-session-journal-types' +import type { AgentSessionJournal } from '../agent-session-journal/journal-store' import type { StructuredAgentSessionHostDeps, StructuredAgentSessionHostSession } from './structured-agent-session-host-types' +import type { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' import type { StructuredAgentSessionTaskQueue } from './structured-agent-session-task-queue' export type StructuredAgentSessionAttachContext = { deps: StructuredAgentSessionHostDeps runtimeState: StructuredAgentSessionHostRuntimeState sessions: Map - subscribers: AgentSessionSubscribers + subscribers: { + reset: ( + sessionId: string, + journal: AgentSessionJournal, + reset: AgentJournalResetReason, + fence: number + ) => void + snapshot: (sessionId: string, journal: AgentSessionJournal, fence: number) => void + publish: (sessionId: string, journal: AgentSessionJournal) => void + } tasks: StructuredAgentSessionTaskQueue reconcileLeases: (sessionId: string) => Promise serialize: (sessionId: string, task: () => Promise) => Promise diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts index 07da0ce0d5cd..f608f236c2cd 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts @@ -12,6 +12,7 @@ import type { } from '../../../shared/agent-session-wire' import type { AgentSessionAttachParams } from './structured-agent-session-attach' import { performAttach } from './structured-agent-session-attach-flow' +import { pinnedAgentSessionLaunchEnv } from './structured-agent-session-launch-env' import { refuseAgentSessionMutation } from './structured-agent-session-mutation-admission' import type { StructuredAgentSessionAttachContext } from './structured-agent-session-attach-context' @@ -42,7 +43,8 @@ export function attachStructuredAgentSession( spawnToken: context.deps.mintSpawnToken?.() ?? randomUUID(), claimKeyId: context.deps.claimKeyId, handoffOperationId: params.envelope.clientOperationId, - probe: await context.runtimeState.probeOwner(sessionId) + probe: await context.runtimeState.probeOwner(sessionId), + ...(await pinnedAgentSessionLaunchEnv(context.deps.resolveLaunchEnv, params)) }, callerKey, params, @@ -50,7 +52,12 @@ export function attachStructuredAgentSession( onAttached: (attached) => { const fence = context.deps.store.getRecord(sessionId)?.lease.runtimeFence ?? 0 const previousFence = context.sessions.get(sessionId)?.fence - context.sessions.set(sessionId, { journal: attached.journal, params, fence }) + context.sessions.set(sessionId, { + journal: attached.journal, + params, + fence, + hasProviderChild: true + }) if (attached.recovery) { context.subscribers.reset(sessionId, attached.journal, attached.recovery.reset, fence) } else if (previousFence !== undefined && previousFence !== fence) { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction-deadline.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction-deadline.ts new file mode 100644 index 000000000000..f34caba3593d --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction-deadline.ts @@ -0,0 +1,51 @@ +// A bound on how long teardown may take, expressed as a wrapper around the eviction steps rather +// than a change to them. +// +// The step list is ordered and abort-on-failure for reasons that have nothing to do with time, and +// a deadline must not disturb either. Wrapping each step's `run` keeps the order, and a timeout +// surfaces as that step failing — which is exactly the behavior wanted here: the rest of the +// eviction aborts, the session stays indexed, and the child stays LOADED. A stuck app-server that +// is still holding a conversation is a better outcome than one killed out from under it; the next +// close retries. + +import type { StructuredAgentSessionEvictionStep } from './structured-agent-session-eviction' + +export const STRUCTURED_AGENT_SESSION_EVICTION_STEP_TIMEOUT_MS = 10_000 + +export class StructuredAgentSessionEvictionTimeoutError extends Error { + constructor( + readonly step: string, + readonly timeoutMs: number + ) { + super(`agent session eviction step "${step}" did not finish within ${timeoutMs}ms`) + this.name = 'StructuredAgentSessionEvictionTimeoutError' + } +} + +export function withStructuredAgentSessionEvictionDeadline( + steps: readonly StructuredAgentSessionEvictionStep[], + timeoutMs = STRUCTURED_AGENT_SESSION_EVICTION_STEP_TIMEOUT_MS +): readonly StructuredAgentSessionEvictionStep[] { + return steps.map((step) => ({ + name: step.name, + run: async (context) => { + let timer: ReturnType | undefined + try { + await Promise.race([ + Promise.resolve(step.run(context)), + new Promise((_resolve, reject) => { + timer = setTimeout( + () => reject(new StructuredAgentSessionEvictionTimeoutError(step.name, timeoutMs)), + timeoutMs + ) + timer.unref?.() + }) + ]) + } finally { + if (timer) { + clearTimeout(timer) + } + } + } + })) +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts new file mode 100644 index 000000000000..3abe8299e06a --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts @@ -0,0 +1,152 @@ +import { describe, expect, it, vi } from 'vitest' +import { + evictStructuredAgentSession, + StructuredAgentSessionEvictionError, + STRUCTURED_AGENT_SESSION_EVICTION_STEPS, + type StructuredAgentSessionEvictionContext +} from './structured-agent-session-eviction' +import { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' + +function context(): StructuredAgentSessionEvictionContext & { order: string[] } { + const order: string[] = [] + return { + order, + sessionId: 'session-1', + eventSink: { + unbind: vi.fn(() => order.push('unbind')), + drained: vi.fn(async () => { + order.push('drained') + }), + close: vi.fn(() => order.push('close')) + } as unknown as StructuredAgentSessionEvictionContext['eventSink'], + adapter: { + closeSession: vi.fn(async () => { + order.push('closeSession') + }) + } as unknown as StructuredAgentSessionEvictionContext['adapter'], + forget: vi.fn(() => order.push('forget')), + discardSink: vi.fn(() => order.push('discardSink')), + releaseLease: vi.fn(async () => { + order.push('releaseLease') + }) + } +} + +function runtimeState(): StructuredAgentSessionHostRuntimeState { + return new StructuredAgentSessionHostRuntimeState({ + store: {} as never, + adapter: {} as never + } as never) +} + +describe('structured agent session eviction', () => { + it('stops the child before it lets the sink go, then forgets the session', async () => { + const ctx = context() + await evictStructuredAgentSession(ctx) + expect(ctx.order).toEqual([ + 'closeSession', + 'drained', + 'unbind', + 'close', + 'discardSink', + 'releaseLease', + 'forget' + ]) + }) + + it('names every step, so a half-finished eviction says which one failed', () => { + expect(STRUCTURED_AGENT_SESSION_EVICTION_STEPS.map((step) => step.name)).toEqual([ + 'stop-provider-child', + 'drain-published', + 'stop-publishing', + 'close-sink', + 'discard-sink', + 'release-lease', + 'forget-session' + ]) + }) +}) + +// Closing the codex child is not silent: the adapter emits its `ended` event and flushes coalesced +// text as it shuts down, and those rows are what clear the running-turn marker. If the sink is +// already closed the journal keeps claiming the agent is working, forever. +describe('rows the provider emits while closing', () => { + it('still reach the journal', async () => { + const state = runtimeState() + const sessionId = 'session-closing-rows' + const sink = state.eventSinkFor(sessionId) + const published: string[] = [] + sink.bind({ journal: {} as never, fence: 1, publish: () => published.push('final-flush') }) + + await evictStructuredAgentSession({ + sessionId, + eventSink: sink, + adapter: { + closeSession: async () => { + // What codex-structured-session-close does on its way out. + sink.sink.publish() + } + } as never, + forget: () => {}, + discardSink: () => state.discardEventSink(sessionId), + releaseLease: async () => {} + }) + + expect(published).toEqual(['final-flush']) + }) +}) + +// `closeSession` returning false means the adapter could not prove the child exited and has kept +// the session indexed on purpose so a retry can reach it. +describe('a child that will not stop', () => { + it('aborts without forgetting the session, so the next close is a real retry', async () => { + const ctx = context() + ctx.adapter.closeSession = vi.fn(async () => false) + + await expect(evictStructuredAgentSession(ctx)).rejects.toMatchObject({ + step: 'stop-provider-child' + }) + expect(ctx.forget).not.toHaveBeenCalled() + expect(ctx.discardSink).not.toHaveBeenCalled() + expect(ctx.order).toEqual([]) + }) + + it('reports the failing step and leaves the sink usable for the retry', async () => { + const ctx = context() + ctx.adapter.closeSession = vi.fn(async () => { + throw new Error('child would not stop') + }) + + await expect(evictStructuredAgentSession(ctx)).rejects.toBeInstanceOf( + StructuredAgentSessionEvictionError + ) + expect(ctx.eventSink.close).not.toHaveBeenCalled() + expect(ctx.forget).not.toHaveBeenCalled() + }) +}) + +// The runtime caches ONE sink per session id and hands the same instance to the next attach, so an +// eviction that closes without discarding leaves a reopened chat wired to a permanently closed +// sink — it accepts every provider event and publishes none. +describe('eviction against the real sink cache', () => { + it('lets the session publish again after it is evicted and reattached', async () => { + const state = runtimeState() + const sessionId = 'session-reattach' + await evictStructuredAgentSession({ + sessionId, + eventSink: state.eventSinkFor(sessionId), + adapter: { closeSession: async () => {} } as never, + forget: () => {}, + discardSink: () => state.discardEventSink(sessionId), + releaseLease: async () => {} + }) + + const published: string[] = [] + const reattached = state.eventSinkFor(sessionId) + reattached.bind({ journal: {} as never, fence: 2, publish: () => published.push('published') }) + reattached.sink.publish() + await reattached.drained() + + expect(published).toEqual(['published']) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.ts new file mode 100644 index 000000000000..da2d856bec90 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-eviction.ts @@ -0,0 +1,95 @@ +// Releasing one structured session's resources. +// +// Teardown is a DATA list, not a method body, for the reason this file exists at all: the host +// tracked which sessions were live in a map, and tore them down at three unrelated call sites +// (app quit, handoff to a TUI, and error cleanup). Closing a chat was never wired to any of them, +// so a provider child outlived the chat that owned it for the whole app session. +// +// ORDER. The provider child stops FIRST. Closing it is not silent: the codex adapter emits its +// `ended` event and flushes coalesced text as part of shutting down, and those are the rows that +// clear the running-turn marker. Draining or closing the sink ahead of that drops them, which +// leaves the durable journal claiming the agent is still working — a worse outcome than the leak +// this teardown exists to fix. So: stop the child, drain what it emitted on its way out, then let +// the sink go. +// +// FAILURE. A step that fails ABORTS the rest. `closeSession` returning false means the child's +// exit was not proven and the adapter has deliberately kept the session indexed so a retry can +// reach it; forgetting it anyway stranded the process forever and reported success. Leaving the +// session in place is what makes the next close a real retry instead of a no-op. + +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import type { DeferredStructuredAgentSessionEventSink } from './structured-agent-session-event-sink' + +export type StructuredAgentSessionEvictionContext = { + sessionId: string + eventSink: DeferredStructuredAgentSessionEventSink + adapter: StructuredAgentSessionAdapter + forget: () => void + /** Drops the cached sink so a later attach mints a fresh one. */ + discardSink: () => void + /** Hands the lease back now that this host's child is proven gone. No-ops when the record is + * not this host's to release. */ + releaseLease: () => Promise +} + +export type StructuredAgentSessionEvictionStep = { + name: string + run: (context: StructuredAgentSessionEvictionContext) => Promise | void +} + +export const STRUCTURED_AGENT_SESSION_EVICTION_STEPS: readonly StructuredAgentSessionEvictionStep[] = + [ + { + name: 'stop-provider-child', + run: async (context) => { + // An adapter with no close has nothing to stop; anything else must PROVE the exit. + const stopped = await context.adapter.closeSession?.(context.sessionId) + if (stopped === false) { + throw new Error('provider child exit was not proven') + } + } + }, + { name: 'drain-published', run: (context) => context.eventSink.drained() }, + { name: 'stop-publishing', run: (context) => context.eventSink.unbind() }, + { name: 'close-sink', run: (context) => context.eventSink.close() }, + // Why: the runtime caches one sink per session id and hands the SAME instance to the next + // attach. Closing without discarding leaves a reopened chat bound to a closed sink, which + // accepts every provider event and publishes none. Attach's own failure path already pairs + // these two; eviction has to as well. + { name: 'discard-sink', run: (context) => context.discardSink() }, + // Why here and not last: the durable lease still names a process this host just stopped, and a + // record left claiming a live owner is one nothing can resume — the next surface to open the + // chat would find a session it may not acquire. Placed BEFORE forget so a release that cannot + // be written aborts while the session is still indexed, which is what makes the retry real. + { name: 'release-lease', run: (context) => context.releaseLease() }, + { name: 'forget-session', run: (context) => context.forget() } + ] + +export class StructuredAgentSessionEvictionError extends Error { + constructor( + readonly step: string, + readonly sessionId: string, + override readonly cause: unknown + ) { + super(`agent session eviction failed at step "${step}" for ${sessionId}`) + this.name = 'StructuredAgentSessionEvictionError' + } +} + +/** + * Runs the eviction steps in order, stopping at the first failure. The step name travels with the + * error because the caller's only useful response is to retry, and a retry is only safe when the + * session is still indexed — which is exactly what aborting preserves. + */ +export async function evictStructuredAgentSession( + context: StructuredAgentSessionEvictionContext, + steps: readonly StructuredAgentSessionEvictionStep[] = STRUCTURED_AGENT_SESSION_EVICTION_STEPS +): Promise { + for (const step of steps) { + try { + await step.run(context) + } catch (error) { + throw new StructuredAgentSessionEvictionError(step.name, context.sessionId, error) + } + } +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-options.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-options.test.ts index d0adf884b894..1457793ee340 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-options.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-options.test.ts @@ -299,4 +299,27 @@ describe('structured session handoff options', () => { expect(launchedOptions).toEqual([]) expect(store.getRecord(SESSION)?.lease.runtimeKind).toBe('native') }) + + // Closing a chat used to leave its provider child resident for the whole app session: the host's + // session map had no delete and the only teardown was app quit. + it('stops the provider child and forgets the session when the chat closes', async () => { + const adapterCloseSession = (host as unknown as { deps: { adapter: { closeSession: Mock } } }) + .deps.adapter.closeSession + expect(host.hasSession(SESSION)).toBe(true) + + await host.close(SESSION) + + expect(adapterCloseSession).toHaveBeenCalledWith(SESSION) + expect(host.hasSession(SESSION)).toBe(false) + }) + + it('is a no-op for a session it does not hold', async () => { + await host.close(SESSION) + const adapterCloseSession = (host as unknown as { deps: { adapter: { closeSession: Mock } } }) + .deps.adapter.closeSession + adapterCloseSession.mockClear() + + await expect(host.close(SESSION)).resolves.toBeUndefined() + expect(adapterCloseSession).not.toHaveBeenCalled() + }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-restart.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-restart.ts index 68ec81fa5ea7..7d448ca755c3 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-restart.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-handoff-restart.ts @@ -99,6 +99,18 @@ export async function restoreStructuredAgentSessionHandoff( } async function restoreOnce(input: RestartAccess, record: AgentSessionRecord): Promise { + if ( + record.lease.handoffStage === null && + record.lease.claimStatus === 'released' && + record.lease.ownerProcess === null + ) { + // Reconcile already proved the owner gone: no transfer is in flight and there is no process to + // recover, whatever kind the last owner was. Falling through would latch manual recovery on a + // host with no TUI transport — a state a user then has to clear by hand, for nothing. (Startup + // used to reach this path only for sessions it had not eagerly resumed, which is why removing + // that resume is what made it visible.) + return + } if (canRestoreLiveTuiOwner(record)) { await restoreRecoverableLiveTui(input, record) return diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-hold-resume.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-hold-resume.ts new file mode 100644 index 000000000000..5ebdfed01145 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-hold-resume.ts @@ -0,0 +1,52 @@ +// Giving a held session its provider child back. +// +// This is the replacement for the startup resume, and the difference is only in WHO asks: the same +// eligibility rule, run when a surface binds instead of when the app launches. A write-capable hold +// must fail when acquisition is refused so the surface never mistakes a readable journal for a live +// provider child. + +import type { + AgentSessionAttachResult, + AgentSessionMutationResult +} from '../../../shared/agent-session-wire' +import type { AgentSessionAttachParams } from './structured-agent-session-attach' +import type { StructuredAgentSessionHostDeps } from './structured-agent-session-host-types' +import { adapterSupportsRecord } from './structured-agent-session-provider-support' +import { + structuredAgentSessionResumeOperationId, + structuredAgentSessionResumeParams +} from './structured-agent-session-resume-eligibility' + +export async function resumeHeldStructuredAgentSession(input: { + sessionId: string + deps: StructuredAgentSessionHostDeps + now: () => number + attach: ( + params: AgentSessionAttachParams + ) => Promise> +}): Promise { + const record = input.deps.store.getRecord(input.sessionId) + if (!record) { + throw new Error('agent_session_identity_required') + } + if (!adapterSupportsRecord(input.deps.adapter, record)) { + throw new Error('structured_agent_session_unsupported') + } + const params = structuredAgentSessionResumeParams( + record, + structuredAgentSessionResumeOperationId(input.now()) + ) + if (!params) { + throw new Error( + record.lease.unreconciled + ? 'execution_owner_reconciling' + : record.lease.claimStatus === 'conflicted' + ? 'agent_session_conflict' + : 'agent_session_ownership_unknown' + ) + } + const attached = await input.attach(params) + if (!attached.ok) { + throw new Error(attached.refusal.code) + } +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-holders.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-holders.ts new file mode 100644 index 000000000000..0771f288b657 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-holders.ts @@ -0,0 +1,49 @@ +// Who WANTS this session alive, as a set of ids rather than a count. +// +// A refcount is the obvious shape and the wrong one. Every path that decrements it — a chat tab +// closing, a transport dying, a client retrying a release it already sent — can fire twice or not +// at all, and an integer cannot tell those apart: a duplicate release evicts a session somebody is +// still looking at, and a lost one leaks the child forever. A set answers both idempotently, +// because it records WHICH surface holds the session, not how many do. + +export class StructuredAgentSessionHolders { + private readonly bySession = new Map>() + + /** True when the session gained its FIRST holder — the edge that ends a pending release. */ + add(sessionId: string, holderId: string): boolean { + const holders = this.bySession.get(sessionId) + if (!holders) { + this.bySession.set(sessionId, new Set([holderId])) + return true + } + holders.add(holderId) + return false + } + + /** True when the session lost its LAST holder — the edge that starts one. */ + remove(sessionId: string, holderId: string): boolean { + const holders = this.bySession.get(sessionId) + if (!holders?.delete(holderId) || holders.size > 0) { + return false + } + this.bySession.delete(sessionId) + return true + } + + isHeld(sessionId: string): boolean { + return (this.bySession.get(sessionId)?.size ?? 0) > 0 + } + + has(sessionId: string, holderId: string): boolean { + return this.bySession.get(sessionId)?.has(holderId) ?? false + } + + holderIds(sessionId: string): string[] { + return [...(this.bySession.get(sessionId) ?? [])] + } + + /** Drops every holder of one session without evaluating the edge, for a session that is gone. */ + forget(sessionId: string): void { + this.bySession.delete(sessionId) + } +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-holds.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-holds.test.ts new file mode 100644 index 000000000000..88f4079fc7fe --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-holds.test.ts @@ -0,0 +1,231 @@ +// The parts a session's lifetime is assembled from: the holder set, the release clock, and the +// deadline that keeps teardown from hanging. + +import { afterEach, describe, expect, it, vi } from 'vitest' +import { StructuredAgentSessionHolders } from './structured-agent-session-holders' +import { StructuredAgentSessionReleaseClock } from './structured-agent-session-release-clock' +import { StructuredAgentSessionHolds } from './structured-agent-session-holds' +import { + STRUCTURED_AGENT_SESSION_EVICTION_STEPS, + evictStructuredAgentSession +} from './structured-agent-session-eviction' +import { + StructuredAgentSessionEvictionTimeoutError, + withStructuredAgentSessionEvictionDeadline +} from './structured-agent-session-eviction-deadline' + +const clocks: StructuredAgentSessionReleaseClock[] = [] + +function clock(deps: { + isTurnActive?: () => boolean + isHeld?: () => boolean + evict: (sessionId: string) => Promise + onError?: (input: { sessionId: string; error: unknown }) => void +}): StructuredAgentSessionReleaseClock { + const created = new StructuredAgentSessionReleaseClock({ + isTurnActive: deps.isTurnActive ?? (() => false), + isHeld: deps.isHeld ?? (() => false), + evict: deps.evict, + ...(deps.onError ? { onError: deps.onError } : {}), + graceMs: 1 + }) + clocks.push(created) + return created +} + +afterEach(() => { + for (const created of clocks.splice(0)) { + created.dispose() + } +}) + +describe('the holder set', () => { + it('reports the first and last holder, and nothing in between', () => { + const holders = new StructuredAgentSessionHolders() + + expect(holders.add('session-1', 'a')).toBe(true) + expect(holders.add('session-1', 'b')).toBe(false) + expect(holders.remove('session-1', 'a')).toBe(false) + expect(holders.remove('session-1', 'b')).toBe(true) + expect(holders.isHeld('session-1')).toBe(false) + }) + + // The reason this is a set and not a count: every release path can fire twice or not at all. + it('absorbs a duplicate hold and a duplicate release', () => { + const holders = new StructuredAgentSessionHolders() + + holders.add('session-1', 'a') + holders.add('session-1', 'a') + expect(holders.remove('session-1', 'a')).toBe(true) + expect(holders.remove('session-1', 'a')).toBe(false) + expect(holders.holderIds('session-1')).toEqual([]) + }) + + it('keeps one session holders out of another session holders', () => { + const holders = new StructuredAgentSessionHolders() + + holders.add('session-1', 'a') + holders.add('session-2', 'a') + holders.remove('session-1', 'a') + + expect(holders.isHeld('session-1')).toBe(false) + expect(holders.isHeld('session-2')).toBe(true) + }) +}) + +describe('the release clock', () => { + it('waits out a running turn instead of evicting into it', async () => { + const evict = vi.fn(async () => {}) + let turnRunning = true + const releasing = clock({ isTurnActive: () => turnRunning, evict }) + + releasing.arm('session-1') + await new Promise((resolve) => setTimeout(resolve, 30)) + expect(evict).not.toHaveBeenCalled() + + turnRunning = false + await vi.waitFor(() => expect(evict).toHaveBeenCalledWith('session-1')) + }) + + it('stands down when a holder arrives during the wait', async () => { + const evict = vi.fn(async () => {}) + const releasing = clock({ isHeld: () => true, evict }) + + releasing.arm('session-1') + await new Promise((resolve) => setTimeout(resolve, 30)) + + expect(evict).not.toHaveBeenCalled() + }) + + it('reports a failed eviction rather than swallowing it', async () => { + const onError = vi.fn() + const releasing = clock({ + evict: async () => { + throw new Error('child would not stop') + }, + onError + }) + + releasing.arm('session-1') + + await vi.waitFor(() => + expect(onError).toHaveBeenCalledWith({ + sessionId: 'session-1', + error: expect.objectContaining({ message: 'child would not stop' }) + }) + ) + }) +}) + +describe('holds', () => { + it('resumes a session on its first hold and not on a retained one', async () => { + let child = false + const resume = vi.fn(async () => { + child = true + }) + const holds = new StructuredAgentSessionHolds({ + resume, + hasProviderChild: () => child, + isTurnActive: () => false, + evict: async () => {}, + graceMs: 1 + }) + + await holds.hold('session-1', 'stream-1', { resume: false }) + expect(resume).not.toHaveBeenCalled() + + await holds.hold('session-1', 'chat-1') + expect(resume).toHaveBeenCalledOnce() + + child = true + await holds.hold('session-1', 'chat-2') + expect(resume).toHaveBeenCalledOnce() + holds.dispose() + }) + + it('never arms the clock for a session with nothing to stop', async () => { + const evict = vi.fn(async () => {}) + const holds = new StructuredAgentSessionHolds({ + resume: async () => {}, + hasProviderChild: () => false, + isTurnActive: () => false, + evict, + graceMs: 1 + }) + + await holds.hold('session-1', 'chat-1', { resume: false }) + holds.release('session-1', 'chat-1') + await new Promise((resolve) => setTimeout(resolve, 20)) + + expect(evict).not.toHaveBeenCalled() + expect(holds.isReleasePending('session-1')).toBe(false) + holds.dispose() + }) + + it('fails a write-capable hold when resume proves no provider child', async () => { + const holds = new StructuredAgentSessionHolds({ + resume: async () => {}, + hasProviderChild: () => false, + isTurnActive: () => false, + evict: async () => {}, + graceMs: 1 + }) + + await expect(holds.hold('session-1', 'chat-1')).rejects.toThrow( + 'agent_session_ownership_unknown' + ) + expect(holds.isHeld('session-1')).toBe(false) + holds.dispose() + }) +}) + +describe('the teardown deadline', () => { + it('leaves the child loaded instead of forcing it, and keeps the session indexed', async () => { + const forget = vi.fn() + const releaseLease = vi.fn(async () => {}) + + await expect( + evictStructuredAgentSession( + { + sessionId: 'session-1', + eventSink: { + unbind: vi.fn(), + drained: vi.fn(async () => {}), + close: vi.fn() + } as never, + adapter: { closeSession: () => new Promise(() => {}) } as never, + forget, + discardSink: vi.fn(), + releaseLease + }, + withStructuredAgentSessionEvictionDeadline(STRUCTURED_AGENT_SESSION_EVICTION_STEPS, 5) + ) + ).rejects.toMatchObject({ step: 'stop-provider-child' }) + + expect(forget).not.toHaveBeenCalled() + expect(releaseLease).not.toHaveBeenCalled() + }) + + it('names the step that ran out of time', async () => { + const [step] = withStructuredAgentSessionEvictionDeadline( + [{ name: 'slow-step', run: () => new Promise(() => {}) }], + 5 + ) + + await expect(step?.run({} as never)).rejects.toBeInstanceOf( + StructuredAgentSessionEvictionTimeoutError + ) + }) + + it('does not delay a step that finishes', async () => { + const ran: string[] = [] + const steps = withStructuredAgentSessionEvictionDeadline( + [{ name: 'fast-step', run: () => void ran.push('fast-step') }], + 5_000 + ) + + await steps[0]?.run({} as never) + + expect(ran).toEqual(['fast-step']) + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-holds.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-holds.ts new file mode 100644 index 000000000000..eac3554783fc --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-holds.ts @@ -0,0 +1,103 @@ +// The lifetime of a structured session, tied to the surfaces that want one. +// +// Nothing used to tell the host that a chat WANTED a session, and nothing told it when a chat +// stopped wanting one. Both halves of that gap cost real processes: sessions nobody had opened got +// an app-server at every launch, and sessions the user closed kept theirs until the app quit. +// +// A surface takes a hold when it binds and drops it when it goes away. The first hold on a session +// with no child resumes it — that, and not the shape of a lease on disk, is what makes a provider +// process exist. The last hold leaving starts the release clock. Transport close is the BACKSTOP, +// not the mechanism: a client that vanishes mid-flight never sends its release, so the caller +// registers one against the connection and the holder set absorbs the duplicate. + +import { + StructuredAgentSessionReleaseClock, + type StructuredAgentSessionReleaseClockDeps +} from './structured-agent-session-release-clock' +import { StructuredAgentSessionHolders } from './structured-agent-session-holders' + +export type StructuredAgentSessionHoldsDeps = { + /** Acquires a provider child for a session that has none. A no-op when one is already live. */ + resume: (sessionId: string) => Promise + /** Whether evicting this session would actually free anything. */ + hasProviderChild: (sessionId: string) => boolean + isTurnActive: (sessionId: string) => boolean + evict: (sessionId: string) => Promise + onError?: (input: { sessionId: string; error: unknown }) => void + graceMs?: number +} + +export type StructuredAgentSessionHoldOptions = { + /** False for a hold that only RETAINS — a subscription stream, which must not make a child + * exist just by reading history. */ + resume?: boolean +} + +export class StructuredAgentSessionHolds { + private readonly holders = new StructuredAgentSessionHolders() + private readonly clock: StructuredAgentSessionReleaseClock + + constructor(private readonly deps: StructuredAgentSessionHoldsDeps) { + const clockDeps: StructuredAgentSessionReleaseClockDeps = { + isTurnActive: deps.isTurnActive, + isHeld: (sessionId) => this.holders.isHeld(sessionId), + evict: (sessionId) => this.deps.evict(sessionId), + ...(deps.onError ? { onError: deps.onError } : {}), + ...(deps.graceMs === undefined ? {} : { graceMs: deps.graceMs }) + } + this.clock = new StructuredAgentSessionReleaseClock(clockDeps) + } + + async hold( + sessionId: string, + holderId: string, + options: StructuredAgentSessionHoldOptions = {} + ): Promise { + const alreadyHeld = this.holders.has(sessionId, holderId) + this.holders.add(sessionId, holderId) + // Unconditional, not only on the first-holder edge: a second surface arriving during the grace + // window must cancel the pending release too. + this.clock.cancel(sessionId) + if (options.resume === false || this.deps.hasProviderChild(sessionId)) { + return + } + try { + await this.deps.resume(sessionId) + if (!this.deps.hasProviderChild(sessionId)) { + throw new Error('agent_session_ownership_unknown') + } + } catch (error) { + if (!alreadyHeld) { + this.holders.remove(sessionId, holderId) + } + throw error + } + } + + release(sessionId: string, holderId: string): void { + if (!this.holders.remove(sessionId, holderId)) { + return + } + if (this.deps.hasProviderChild(sessionId)) { + this.clock.arm(sessionId) + } + } + + /** Drops the holders of a session that is gone, whoever evicted it. */ + forget(sessionId: string): void { + this.clock.cancel(sessionId) + this.holders.forget(sessionId) + } + + isHeld(sessionId: string): boolean { + return this.holders.isHeld(sessionId) + } + + isReleasePending(sessionId: string): boolean { + return this.clock.isArmed(sessionId) + } + + dispose(): void { + this.clock.dispose() + } +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts new file mode 100644 index 000000000000..6f37f677e6f1 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts @@ -0,0 +1,86 @@ +// The host's half of a session's lifetime: what a close does, and what a hold is wired to. +// +// Lifted out of the host for the same reason attaching was — the host is a coordinator, and the +// sequence that stops a provider child and hands its lease back reads better next to the holder +// bookkeeping that decides when to run it than buried among the twenty other things a session can +// do. + +import { activeStructuredAgentSessionTurnId } from '../../../shared/structured-agent-session-projection' +import { + evictStructuredAgentSession, + STRUCTURED_AGENT_SESSION_EVICTION_STEPS, + type StructuredAgentSessionEvictionContext +} from './structured-agent-session-eviction' +import { withStructuredAgentSessionEvictionDeadline } from './structured-agent-session-eviction-deadline' +import { StructuredAgentSessionHolds } from './structured-agent-session-holds' +import type { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' +import type { + StructuredAgentSessionHostDeps, + StructuredAgentSessionHostSession +} from './structured-agent-session-host-types' +import { releaseStoredStructuredAgentSessionOwner } from './structured-agent-session-lease-release' + +export type StructuredAgentSessionLifetimeContext = { + deps: StructuredAgentSessionHostDeps + runtimeState: StructuredAgentSessionHostRuntimeState + sessions: Map + now: () => number +} + +function hasProviderChild( + context: StructuredAgentSessionLifetimeContext, + sessionId: string +): boolean { + return context.sessions.get(sessionId)?.hasProviderChild === true +} + +/** Runs the eviction steps under a deadline. A step that fails — or runs out of time — aborts the + * rest, which leaves the session indexed and the child loaded so the next close is a real retry. */ +export async function evictHeldStructuredAgentSession( + context: StructuredAgentSessionLifetimeContext, + sessionId: string +): Promise { + if (!context.sessions.has(sessionId)) { + return + } + const eviction: StructuredAgentSessionEvictionContext = { + sessionId, + eventSink: context.runtimeState.eventSinkFor(sessionId), + adapter: context.deps.adapter, + forget: () => context.sessions.delete(sessionId), + discardSink: () => context.runtimeState.discardEventSink(sessionId), + releaseLease: () => + releaseStoredStructuredAgentSessionOwner({ + store: context.deps.store, + sessionId, + hasProviderChild: hasProviderChild(context, sessionId), + now: context.now() + }) + } + await evictStructuredAgentSession( + eviction, + withStructuredAgentSessionEvictionDeadline(STRUCTURED_AGENT_SESSION_EVICTION_STEPS) + ) +} + +export function createStructuredAgentSessionHolds( + context: StructuredAgentSessionLifetimeContext, + input: { + resume: (sessionId: string) => Promise + evict: (sessionId: string) => Promise + } +): StructuredAgentSessionHolds { + return new StructuredAgentSessionHolds({ + resume: input.resume, + evict: input.evict, + hasProviderChild: (sessionId) => hasProviderChild(context, sessionId), + isTurnActive: (sessionId) => { + const session = context.sessions.get(sessionId) + return session + ? activeStructuredAgentSessionTurnId(session.journal.snapshot().items) !== null + : false + }, + onError: (error) => context.deps.onEventSinkError?.(error), + ...(context.deps.releaseGraceMs === undefined ? {} : { graceMs: context.deps.releaseGraceMs }) + }) +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts new file mode 100644 index 000000000000..c9afa06e5258 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts @@ -0,0 +1,115 @@ +// Everything a client can ask an ALREADY-ATTACHED session to do: send a turn, cancel one, answer a +// prompt, change an option, read the options back. +// +// They share one shape — admit the envelope against the lease, run a plan, publish the journal — so +// they share one path here rather than five copies in the host. The host keeps attach, holds and +// teardown; this is the surface that assumes those already happened. + +import type { AgentJournalMessageItem } from '../../../shared/agent-session-journal-types' +import type { + AgentSessionCancelResult, + AgentSessionMutationEnvelope, + AgentSessionMutationResult, + AgentSessionOptionResult, + AgentSessionOptionsResult, + AgentSessionPromptResult, + AgentSessionSendResult +} from '../../../shared/agent-session-wire' +import { admitAndRunAgentSessionMutation } from './structured-agent-session-mutation-admission' +import { + cancelPlan, + promptPlan, + sendPlan, + setOptionPlan, + type MutationPlan +} from './structured-agent-session-mutation-plans' +import type { + StructuredAgentSessionCaller, + StructuredAgentSessionHostDeps, + StructuredAgentSessionHostSession +} from './structured-agent-session-host-types' + +export type StructuredAgentSessionMutationContext = { + deps: StructuredAgentSessionHostDeps + sessions: Map + publish: (sessionId: string, journal: StructuredAgentSessionHostSession['journal']) => void + requireSession: (sessionId: string) => StructuredAgentSessionHostSession + serialize: (sessionId: string, task: () => Promise) => Promise + now: () => number +} + +function mutate( + context: StructuredAgentSessionMutationContext, + caller: StructuredAgentSessionCaller, + envelope: AgentSessionMutationEnvelope, + plan: MutationPlan +): Promise> { + return context.serialize(envelope.sessionId, () => + admitAndRunAgentSessionMutation({ + store: context.deps.store, + adapter: context.deps.adapter, + callerKey: caller.callerKey, + envelope, + plan, + journal: context.sessions.get(envelope.sessionId)?.journal, + publish: (journal) => context.publish(envelope.sessionId, journal), + now: () => context.now() + }) + ) +} + +export function sendStructuredAgentSessionTurn( + context: StructuredAgentSessionMutationContext, + caller: StructuredAgentSessionCaller, + params: { + envelope: AgentSessionMutationEnvelope + body: AgentJournalMessageItem + retryUnknown?: true + beforeRun?: () => void + } +): Promise> { + return mutate(context, caller, params.envelope, sendPlan(params)) +} + +export function cancelStructuredAgentSessionTurn( + context: StructuredAgentSessionMutationContext, + caller: StructuredAgentSessionCaller, + params: { envelope: AgentSessionMutationEnvelope; turnId: string } +): Promise> { + return mutate(context, caller, params.envelope, cancelPlan(params)) +} + +export function respondToStructuredAgentSessionPrompt( + context: StructuredAgentSessionMutationContext, + caller: StructuredAgentSessionCaller, + params: { + envelope: AgentSessionMutationEnvelope + kind: 'approval' | 'question' + itemId: string + expectedRevision: number + optionId: string + } +): Promise> { + return mutate(context, caller, params.envelope, promptPlan(params)) +} + +export function setStructuredAgentSessionOption( + context: StructuredAgentSessionMutationContext, + caller: StructuredAgentSessionCaller, + params: { envelope: AgentSessionMutationEnvelope; key: string; value: string } +): Promise> { + return mutate(context, caller, params.envelope, setOptionPlan(params)) +} + +export function readStructuredAgentSessionOptions( + context: StructuredAgentSessionMutationContext, + sessionId: string +): Promise { + return context.serialize(sessionId, async () => { + const session = context.requireSession(sessionId) + if (!context.deps.adapter.readOptions) { + throw new Error('structured_agent_session_options_unsupported') + } + return context.deps.adapter.readOptions({ sessionId, fence: session.fence }) + }) +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts index 6a595b8b8d61..db65f98624aa 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-runtime-state.ts @@ -21,6 +21,7 @@ export class StructuredAgentSessionHostRuntimeState { this.leaseRenewer = new StructuredAgentSessionLeaseRenewer({ store: deps.store, probe: (record) => this.probeRecord(record), + ...(deps.probeOwners ? { probeMany: deps.probeOwners } : {}), now: () => deps.now?.() ?? Date.now(), ...(onLeaseRenewed ? { onRenewed: onLeaseRenewed } : {}), ...(onDeadTuiOwner ? { onDeadTuiOwner } : {}), diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts index 1a6ef8a31805..85ddb2bbb621 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts @@ -13,6 +13,10 @@ export type StructuredAgentSessionHostSession = { journal: AgentSessionJournal params: AgentSessionAttachParams fence: number + /** Whether THIS host generation is running the provider process behind the session. A journal + * restored for reading has none, and neither has a session a TUI owns — so neither may be + * evicted to free a child, and neither may have its lease released as an observed exit. */ + hasProviderChild: boolean } export type StructuredAgentSessionHostDeps = { @@ -21,12 +25,20 @@ export type StructuredAgentSessionHostDeps = { journalRoot: string claimKeyId: string probeOwner?: (record: AgentSessionRecord) => Promise + probeOwners?: ( + records: readonly AgentSessionRecord[] + ) => Promise> /** Recovery-exit stop requests only; a lease moves only on a later proven-absent probe. */ stopOwnerProcess?: (pid: number, signal: 'SIGTERM' | 'SIGKILL') => void /** Host spawn-token process scan; null means the platform cannot enumerate, never "none". */ scanSpawnTokenProcesses?: () => Promise mintSpawnToken?: () => string + resolveLaunchEnv?: ( + provider: AgentSessionRecord['provider'] + ) => Promise | undefined> | Record | undefined now?: () => number + /** How long a session outlives its last surface. Tests drive this; production takes the default. */ + releaseGraceMs?: number onEventSinkError?: (input: { sessionId: string; error: unknown }) => void handoffTransport?: StructuredAgentSessionHandoffTransport } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts index 549b17023151..75eed906a3d0 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts @@ -709,7 +709,7 @@ describe('restart', () => { expect(listRecords).toHaveBeenCalledTimes(restoreReads) }) - it('clears stale TUI recovery when restart successfully reacquires the native owner', async () => { + it('clears stale TUI recovery at restart, and reacquires the native owner when a surface holds it', async () => { await attach() await store.transitionHandoff(SESSION, (record) => ({ ...record, @@ -723,6 +723,9 @@ describe('restart', () => { acquire.mockClear() await host.restoreReadableSessions() + // The recovery stage clears on evidence at startup; the child comes back only once a surface + // holds the session (see structured-agent-session-surface-lifetime.test.ts). + await host.hold(SESSION, 'surface-1') expect(acquire).toHaveBeenCalledOnce() expect(store.getRecord(SESSION)?.lease).toMatchObject({ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts index ef3ea367ff2e..81601480ff00 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts @@ -1,37 +1,21 @@ // Structured agent-session host: where the lease, journal, and provider adapter meet. // Mutations share one durable admission path and serialize per session. -import type { AgentJournalMessageItem } from '../../../shared/agent-session-journal-types' import type { AgentSessionExecutionLocation } from '../../../shared/agent-session-record' import type { AgentSessionAttachResult, - AgentSessionCancelResult, AgentSessionHistoryRequest, AgentSessionHistoryResult, AgentSessionHandoffRequest, AgentSessionHandoffResult, AgentSessionHandoffStatus, - AgentSessionMutationEnvelope, AgentSessionMutationResult, - AgentSessionOptionResult, AgentSessionOptionsResult, - AgentSessionPromptResult, - AgentSessionSendResult, AgentSessionWireRefusal } from '../../../shared/agent-session-wire' import type { AgentSessionAttachParams } from './structured-agent-session-attach' -import { - admitAndRunAgentSessionMutation, - AGENT_SESSION_NOT_ATTACHED -} from './structured-agent-session-mutation-admission' +import { AGENT_SESSION_NOT_ATTACHED } from './structured-agent-session-mutation-admission' import { createRestartReconciler } from './structured-agent-session-restart-reconcile' -import { - cancelPlan, - promptPlan, - sendPlan, - setOptionPlan, - type MutationPlan -} from './structured-agent-session-mutation-plans' import { AgentSessionSubscribers, type AgentSessionSubscribeInput @@ -45,10 +29,28 @@ import { type StructuredAgentSessionHostHandoff } from './structured-agent-session-host-handoff' import { StructuredAgentSessionHostRuntimeState } from './structured-agent-session-host-runtime-state' -import { listStructuredAgentSessionTabs } from './structured-agent-session-host-tabs' -import { StructuredAgentSessionReadableRestorer } from './structured-agent-session-readable-restorer' import { attachStructuredAgentSession } from './structured-agent-session-attach-orchestration' +import { + createStructuredAgentSessionHolds, + evictHeldStructuredAgentSession, + type StructuredAgentSessionLifetimeContext +} from './structured-agent-session-host-lifetime' +import type { + StructuredAgentSessionHolds, + StructuredAgentSessionHoldOptions +} from './structured-agent-session-holds' +import { resumeHeldStructuredAgentSession } from './structured-agent-session-hold-resume' import type { StructuredAgentSessionAttachContext } from './structured-agent-session-attach-context' +import { listStructuredAgentSessionTabs } from './structured-agent-session-host-tabs' +import { + cancelStructuredAgentSessionTurn, + readStructuredAgentSessionOptions, + respondToStructuredAgentSessionPrompt, + sendStructuredAgentSessionTurn, + setStructuredAgentSessionOption, + type StructuredAgentSessionMutationContext +} from './structured-agent-session-host-mutations' +import { StructuredAgentSessionReadableRestorer } from './structured-agent-session-readable-restorer' import type { StructuredAgentSessionCaller, StructuredAgentSessionHostDeps, @@ -77,6 +79,7 @@ export class StructuredAgentSessionHost { private readonly handoffs: StructuredAgentSessionHostHandoff private readonly readableRestorer: StructuredAgentSessionReadableRestorer private readonly restartRestore = new StructuredAgentSessionRestartRestoreGate() + private readonly holds: StructuredAgentSessionHolds constructor(readonly deps: StructuredAgentSessionHostDeps) { this.runtimeState = new StructuredAgentSessionHostRuntimeState( @@ -102,22 +105,21 @@ export class StructuredAgentSessionHost { subscribers: this.subscribers, now: this.now }) + this.holds = createStructuredAgentSessionHolds(this.lifetimeContext(), { + resume: (sessionId) => this.resumeForHold(sessionId), + evict: (sessionId) => this.close(sessionId) + }) this.readableRestorer = new StructuredAgentSessionReadableRestorer({ store: deps.store, journalRoot: deps.journalRoot, supportsRecord: (record) => providerSupport.adapterSupportsRecord(deps.adapter, record), reconcile: this.reconcileLeases, resolveRecovery: (sessionId) => this.runtimeState.resolveRecovery(sessionId), - resume: (params) => - this.attach({ callerKey: 'trusted-local:host-restart' }, params).then( - (result) => result.ok - ), serialize: (sessionId, task) => this.serialize(sessionId, task), hasSession: (sessionId) => this.sessions.has(sessionId), onReadable: (sessionId, restored) => this.sessions.set(sessionId, restored), restoreHandoff: (sessionId) => this.handoffs.restore(sessionId), - reapOrphanChildren: () => this.runtimeState.reapOrphanChildren(), - now: this.now + reapOrphanChildren: () => this.runtimeState.reapOrphanChildren() }) this.runtimeState.startLeaseRenewal() } @@ -126,6 +128,67 @@ export class StructuredAgentSessionHost { hasSession = (sessionId: string): boolean => this.sessions.has(sessionId) + /** A surface bound to this session and wants it live. The FIRST hold on a session with no + * provider child is what resumes one; a retained hold (a subscription) only keeps it. */ + hold = ( + sessionId: string, + holderId: string, + options?: StructuredAgentSessionHoldOptions + ): Promise => this.holds.hold(sessionId, holderId, options) + + /** That surface is gone. The child outlives it by the release grace, and by any running turn. */ + release = (sessionId: string, holderId: string): void => this.holds.release(sessionId, holderId) + + isHeld = (sessionId: string): boolean => this.holds.isHeld(sessionId) + + private async resumeForHold(sessionId: string): Promise { + const unreconciled = await this.reconcileLeases(sessionId) + if (unreconciled) { + throw new Error(unreconciled.code) + } + await this.runtimeState.resolveRecovery(sessionId) + await resumeHeldStructuredAgentSession({ + sessionId, + deps: this.deps, + now: () => this.now(), + attach: (params) => this.attach({ callerKey: 'trusted-local:surface-hold' }, params) + }) + } + + private lifetimeContext(): StructuredAgentSessionLifetimeContext { + return { + deps: this.deps, + runtimeState: this.runtimeState, + sessions: this.sessions, + now: () => this.now() + } + } + + /** The host's half of attaching, named so it cannot grow dependencies unnoticed. */ + private attachContext(): StructuredAgentSessionAttachContext { + return { + deps: this.deps, + runtimeState: this.runtimeState, + sessions: this.sessions, + subscribers: this.subscribers, + tasks: this.tasks, + reconcileLeases: (sessionId) => this.reconcileLeases(sessionId), + serialize: (sessionId, task) => this.serialize(sessionId, task), + now: () => this.now() + } + } + + /** Releases a session's resources without ending the conversation: the record and journal stay + * on disk, so the same session can be attached again. */ + close(sessionId: string): Promise { + return this.serialize(sessionId, async () => { + await evictHeldStructuredAgentSession(this.lifetimeContext(), sessionId) + // Whoever asked for the close, the surfaces that were holding this session are looking at a + // session that no longer exists. A failed eviction throws above and keeps them. + this.holds.forget(sessionId) + }) + } + supportsCreate = (location: AgentSessionExecutionLocation, agent: string): boolean => providerSupport.adapterSupportsCreate(this.deps.adapter, location, agent) @@ -147,20 +210,6 @@ export class StructuredAgentSessionHost { }) } - /** What attach needs from this host, named so its dependencies cannot grow unnoticed. */ - private attachContext(): StructuredAgentSessionAttachContext { - return { - deps: this.deps, - runtimeState: this.runtimeState, - sessions: this.sessions, - subscribers: this.subscribers, - tasks: this.tasks, - reconcileLeases: (sessionId) => this.reconcileLeases(sessionId), - serialize: (sessionId, task) => this.serialize(sessionId, task), - now: () => this.now() - } - } - attach( caller: StructuredAgentSessionCaller, params: AgentSessionAttachParams @@ -201,79 +250,50 @@ export class StructuredAgentSessionHost { this.runtimeState.flushEventSink(sessionId) async flushAllStreamedEvents(): Promise { + this.holds.dispose() this.runtimeState.stopLeaseRenewal() this.handoffs.stopTuiHistoryCatchup() await this.tasks.drainAttaches() await this.runtimeState.flushAllEventSinks() } - send( - caller: StructuredAgentSessionCaller, - params: { - envelope: AgentSessionMutationEnvelope - body: AgentJournalMessageItem - retryUnknown?: true - beforeRun?: () => void + private mutationContext(): StructuredAgentSessionMutationContext { + return { + deps: this.deps, + sessions: this.sessions, + publish: (sessionId, journal) => this.subscribers.publish(sessionId, journal), + requireSession: (sessionId) => this.requireSession(sessionId), + serialize: (sessionId, task) => this.serialize(sessionId, task), + now: () => this.now() } - ): Promise> { - return this.mutate(caller, params.envelope, sendPlan(params)) } - cancel( + send = ( caller: StructuredAgentSessionCaller, - params: { envelope: AgentSessionMutationEnvelope; turnId: string } - ): Promise> { - return this.mutate(caller, params.envelope, cancelPlan(params)) - } + params: Parameters[2] + ): ReturnType => + sendStructuredAgentSessionTurn(this.mutationContext(), caller, params) - respondToPrompt( + cancel = ( caller: StructuredAgentSessionCaller, - params: { - envelope: AgentSessionMutationEnvelope - kind: 'approval' | 'question' - itemId: string - expectedRevision: number - optionId: string - } - ): Promise> { - return this.mutate(caller, params.envelope, promptPlan(params)) - } + params: Parameters[2] + ): ReturnType => + cancelStructuredAgentSessionTurn(this.mutationContext(), caller, params) - setOption( + respondToPrompt = ( caller: StructuredAgentSessionCaller, - params: { envelope: AgentSessionMutationEnvelope; key: string; value: string } - ): Promise> { - return this.mutate(caller, params.envelope, setOptionPlan(params)) - } + params: Parameters[2] + ): ReturnType => + respondToStructuredAgentSessionPrompt(this.mutationContext(), caller, params) - readOptions(sessionId: string): Promise { - return this.serialize(sessionId, async () => { - const session = this.requireSession(sessionId) - if (!this.deps.adapter.readOptions) { - throw new Error('structured_agent_session_options_unsupported') - } - return this.deps.adapter.readOptions({ sessionId, fence: session.fence }) - }) - } - - private mutate( + setOption = ( caller: StructuredAgentSessionCaller, - envelope: AgentSessionMutationEnvelope, - plan: MutationPlan - ): Promise> { - return this.serialize(envelope.sessionId, () => - admitAndRunAgentSessionMutation({ - store: this.deps.store, - adapter: this.deps.adapter, - callerKey: caller.callerKey, - envelope, - plan, - journal: this.sessions.get(envelope.sessionId)?.journal, - publish: (journal) => this.subscribers.publish(envelope.sessionId, journal), - now: () => this.now() - }) - ) - } + params: Parameters[2] + ): ReturnType => + setStructuredAgentSessionOption(this.mutationContext(), caller, params) + + readOptions = (sessionId: string): Promise => + readStructuredAgentSessionOptions(this.mutationContext(), sessionId) requestHandoff( caller: StructuredAgentSessionCaller, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-launch-env.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-launch-env.ts new file mode 100644 index 000000000000..3efdf3ccc2d9 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-launch-env.ts @@ -0,0 +1,21 @@ +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import type { AgentSessionAttachParams } from './structured-agent-session-attach' + +type LaunchEnvResolver = ( + provider: AgentSessionRecord['provider'] +) => Promise | undefined> | Record | undefined + +export async function pinnedAgentSessionLaunchEnv( + resolver: LaunchEnvResolver | undefined, + params: AgentSessionAttachParams +): Promise<{ launchEnv: Record } | Record> { + if (!resolver) { + return {} + } + return { + launchEnv: { + ...(await resolver(params.provider)), + [params.accountHome.variable]: params.accountHome.path + } + } +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-release.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-release.ts new file mode 100644 index 000000000000..63d29754f61f --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-release.ts @@ -0,0 +1,32 @@ +// Handing the durable lease back after eviction stopped this host's child. +// +// Guarded on `hasProviderChild` for a reason that is not bookkeeping: a session restored only for +// reading, or one a TUI owns, names an owner process this host never started and may still be +// alive. Writing `exit-observed` against that record would release a lease out from under a running +// process and let a second writer in. + +import { + isSurfaceReleasableAgentSessionRecord, + releaseStoredAgentSessionOwnerAfterSurfaceClose +} from '../../runtime/agent-session-surface-release-transition' +import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' + +export async function releaseStoredStructuredAgentSessionOwner(input: { + store: AgentSessionRecordStore + sessionId: string + hasProviderChild: boolean + now: number +}): Promise { + if (!input.hasProviderChild) { + return + } + const record = input.store.getRecord(input.sessionId) + if (!record || !isSurfaceReleasableAgentSessionRecord(record)) { + return + } + await releaseStoredAgentSessionOwnerAfterSurfaceClose(input.store, { + sessionId: input.sessionId, + expectedFence: record.lease.runtimeFence, + now: input.now + }) +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.test.ts index 5a66b422070e..42607304e901 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.test.ts @@ -2,6 +2,10 @@ import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' +import { + agentSessionLeaseFixture, + agentSessionRecordFixture +} from '../../../shared/agent-session-record.test-fixture' import { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { StructuredAgentSessionLeaseRenewer } from './structured-agent-session-lease-renewer' @@ -66,6 +70,50 @@ afterEach(async () => { }) describe('structured agent-session lease renewal', () => { + it('commits one store transaction for the whole live-record sweep', async () => { + const records = ['a', 'b'].map((suffix, index) => { + const sessionId = `session-${suffix}` + return agentSessionRecordFixture( + agentSessionLeaseFixture({ + sessionId, + runtimeKind: 'native', + runtimeFence: index + 1, + ownerProcess: { + hostId: 'local', + pid: 4200 + index, + processStartTimeMs: NOW - 1_000, + spawnToken: `spawn-${suffix}` + }, + reservedSpawnToken: `spawn-${suffix}`, + lastRenewedAt: NOW, + leaseDeadlineAt: NOW + 30_000 + }) + ) + }) + const renewLeases = vi.fn(async (_renewals: readonly unknown[]) => records) + const probeMany = vi.fn( + async () => + new Map( + records.map((record) => [ + record.sessionId, + { outcome: 'identity-matched', matchedOn: ['spawn-token'] } as const + ]) + ) + ) + const renewer = new StructuredAgentSessionLeaseRenewer({ + store: { listRecords: () => records, renewLeases } as unknown as AgentSessionRecordStore, + probe: vi.fn(), + probeMany, + now: () => NOW + 10_000 + }) + + await renewer.renewNow() + + expect(probeMany).toHaveBeenCalledOnce() + expect(renewLeases).toHaveBeenCalledOnce() + expect(renewLeases.mock.calls[0]?.[0]).toHaveLength(2) + }) + it('drives renewal on the production interval', async () => { vi.useFakeTimers() const store = await liveStore() diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts index d4fdebc1ec47..7f3907815760 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-lease-renewer.ts @@ -18,6 +18,9 @@ export class StructuredAgentSessionLeaseRenewer { private readonly input: { store: AgentSessionRecordStore probe: (record: AgentSessionRecord) => Promise + probeMany?: ( + records: readonly AgentSessionRecord[] + ) => Promise> now: () => number onRenewed?: (record: AgentSessionRecord) => Promise onDeadTuiOwner?: (record: AgentSessionRecord, probe: AgentSessionOwnerProbe) => Promise @@ -47,49 +50,87 @@ export class StructuredAgentSessionLeaseRenewer { } this.running = true try { - await Promise.all( - this.input.store - .listRecords() - .filter( - (record) => - !record.lease.unreconciled && - record.lease.claimStatus === 'live' && - record.lease.ownerProcess !== null && - // A native record parked in recovery has no transport the host can vouch - // for; renewing it keeps an orphan pid's lease reading as a healthy owner. - !( - record.lease.runtimeKind === 'native' && - (record.lease.handoffStage === 'recovering' || - record.lease.handoffStage === 'manual-recovery') - ) + const records = this.input.store.listRecords().filter( + (record) => + !record.lease.unreconciled && + record.lease.claimStatus === 'live' && + record.lease.ownerProcess !== null && + // A native record parked in recovery has no transport the host can vouch + // for; renewing it keeps an orphan pid's lease reading as a healthy owner. + !( + record.lease.runtimeKind === 'native' && + (record.lease.handoffStage === 'recovering' || + record.lease.handoffStage === 'manual-recovery') ) - .map((record) => this.renewRecord(record)) ) + const probes = await this.probe(records) + const renewals: { + sessionId: string + fence: number + childProbe: AgentSessionOwnerProbe + now: number + }[] = [] + const now = this.input.now() + for (const record of records) { + const probe = probes.get(record.sessionId) + if (!probe) { + continue + } + if ( + record.lease.runtimeKind === 'tui' && + isProvenDeadProbe(probe) && + this.input.onDeadTuiOwner + ) { + try { + await this.input.onDeadTuiOwner(record, probe) + } catch (error) { + this.input.onError?.({ sessionId: record.sessionId, error }) + } + continue + } + renewals.push({ + sessionId: record.sessionId, + fence: record.lease.runtimeFence, + childProbe: probe, + now + }) + } + try { + const renewed = await this.input.store.renewLeases(renewals) + await Promise.all(renewed.map((record) => this.input.onRenewed?.(record))) + } catch (error) { + for (const renewal of renewals) { + this.input.onError?.({ sessionId: renewal.sessionId, error }) + } + } } finally { this.running = false } } - private async renewRecord(record: AgentSessionRecord): Promise { + private async probe( + records: readonly AgentSessionRecord[] + ): Promise> { try { - const probe = await this.input.probe(record) - if ( - record.lease.runtimeKind === 'tui' && - isProvenDeadProbe(probe) && - this.input.onDeadTuiOwner - ) { - await this.input.onDeadTuiOwner(record, probe) - return + if (this.input.probeMany) { + return await this.input.probeMany(records) } - const renewed = await this.input.store.renewLease({ - sessionId: record.sessionId, - fence: record.lease.runtimeFence, - childProbe: probe, - now: this.input.now() - }) - await this.input.onRenewed?.(renewed) + const settled = await Promise.allSettled(records.map((record) => this.input.probe(record))) + const probes = new Map() + for (const [index, result] of settled.entries()) { + const record = records[index] + if (result.status === 'fulfilled') { + probes.set(record.sessionId, result.value) + } else { + this.input.onError?.({ sessionId: record.sessionId, error: result.reason }) + } + } + return probes } catch (error) { - this.input.onError?.({ sessionId: record.sessionId, error }) + for (const record of records) { + this.input.onError?.({ sessionId: record.sessionId, error }) + } + return new Map() } } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-legacy-restart.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-legacy-restart.test.ts index 3bacb960b13c..13edd1f90289 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-legacy-restart.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-legacy-restart.test.ts @@ -4,6 +4,7 @@ import { LEGACY_AGENT_SESSION_ID } from '../../../shared/agent-session-record-legacy.test-fixture' import { loadAgentSessionRecord } from '../../../shared/agent-session-record-load' +import { isResumableStructuredAgentSessionRecord } from './structured-agent-session-resume-eligibility' import { restoreStructuredAgentSessionsOnRestart } from './structured-agent-session-restart-restore' describe('legacy structured session restart', () => { @@ -12,7 +13,7 @@ describe('legacy structured session restart', () => { if (loaded.status !== 'upgraded') { throw new Error(`expected upgrade, got ${loaded.status}`) } - const resume = vi.fn(async () => true) + const serialize = vi.fn(async (_sessionId: string, task: () => Promise) => task()) await restoreStructuredAgentSessionsOnRestart({ store: { getRecord: () => loaded.record } as never, @@ -20,9 +21,7 @@ describe('legacy structured session restart', () => { records: [loaded.record], reconcile: async () => null, resolveRecovery: async () => 'not-applicable', - operationId: () => 'legacy-upgrade-operation', - resume, - serialize: async () => undefined as never, + serialize: serialize as never, hasSession: () => false, onReadable: vi.fn(), restoreHandoff: vi.fn(async () => undefined) @@ -32,6 +31,8 @@ describe('legacy structured session restart', () => { sessionId: LEGACY_AGENT_SESSION_ID, lease: { claimStatus: 'conflicted', handoffStage: 'manual-recovery' } }) - expect(resume).not.toHaveBeenCalled() + // Restart no longer resumes ANYTHING, so the guard that matters now is the one a surface hits: + // a conflicted record under manual recovery is not a record a hold may acquire. + expect(isResumableStructuredAgentSessionRecord(loaded.record)).toBe(false) }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-read-restore.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-read-restore.ts index 7599fe134ce9..e8b065353292 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-read-restore.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-read-restore.ts @@ -20,6 +20,7 @@ export type RestoredStructuredAgentSessionRead = { journal: AgentSessionJournal params: AgentSessionAttachParams fence: number + hasProviderChild: false } export async function restoreStructuredAgentSessionRead( @@ -47,7 +48,8 @@ export async function restoreStructuredAgentSessionRead( identity: journalIdentityFor(record, params), journalDir }) - return { journal, params, fence: record.lease.runtimeFence } + // Read restore opens the journal and nothing else: no adapter call, so no provider child. + return { journal, params, fence: record.lease.runtimeFence, hasProviderChild: false } } export function attachParamsForRecord( diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.ts index 5ecd568a0900..7f929f7e5a63 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.ts @@ -1,8 +1,6 @@ -import { randomUUID } from 'node:crypto' import type { AgentSessionRecord } from '../../../shared/agent-session-record' import type { AgentSessionWireRefusal } from '../../../shared/agent-session-wire' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' -import type { AgentSessionAttachParams } from './structured-agent-session-attach' import type { RestoredStructuredAgentSessionRead } from './structured-agent-session-read-restore' import { restoreStructuredAgentSessionsOnRestart } from './structured-agent-session-restart-restore' @@ -16,14 +14,12 @@ export class StructuredAgentSessionReadableRestorer { supportsRecord: (record: AgentSessionRecord) => boolean reconcile: (sessionId: string) => Promise resolveRecovery: (sessionId: string) => Promise - resume: (params: AgentSessionAttachParams) => Promise serialize: (sessionId: string, task: () => Promise) => Promise hasSession: (sessionId: string) => boolean onReadable: (sessionId: string, restored: RestoredStructuredAgentSessionRead) => void restoreHandoff: (sessionId: string) => Promise /** Reaps children carrying a token no lease claims, before recovery grants a new owner. */ reapOrphanChildren?: () => Promise - now: () => number } ) {} @@ -41,9 +37,7 @@ export class StructuredAgentSessionReadableRestorer { await this.input.reapOrphanChildren?.() await restoreStructuredAgentSessionsOnRestart({ ...this.input, - records: this.input.store.listRecords().filter(this.input.supportsRecord), - operationId: () => - `${Math.trunc(this.input.now()).toString().padStart(13, '0')}-${randomUUID().replaceAll('-', '')}` + records: this.input.store.listRecords().filter(this.input.supportsRecord) }) } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-exits.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-exits.test.ts index 80c0d22db21f..3b18361518c7 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-exits.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-exits.test.ts @@ -4,7 +4,11 @@ import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import { spawnProcess } from '../../../shared/child-process/run-process' +import { CODEX_SPAWN_TOKEN_ENV } from '../../codex/codex-structured-owner-identity' import { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +import { readProcessStartTimeMs } from '../../runtime/agent-session-process-identity-probe' +import { createStructuredAgentSessionOwnerProbe } from '../../runtime/structured-agent-session-runtime' import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' import { StructuredAgentSessionHost } from './structured-agent-session-host' import type { StructuredAgentSessionHostDeps } from './structured-agent-session-host-types' @@ -22,6 +26,38 @@ let root: string let store: AgentSessionRecordStore let host: StructuredAgentSessionHost let acquire: Mock +const spawnedOwners = new Set>() +const supersededHosts = new Set() + +async function spawnOwner(spawnToken: string) { + const child = spawnProcess({ + program: process.execPath, + args: ['-e', 'setInterval(() => {}, 1_000)'], + env: { ...process.env, [CODEX_SPAWN_TOKEN_ENV]: spawnToken } + }) + spawnedOwners.add(child) + const pid = child.pid + if (!pid) { + throw new Error('owner process did not start') + } + const processStartTimeMs = await readProcessStartTimeMs(pid) + if (processStartTimeMs === null) { + throw new Error('owner process start time was unavailable') + } + return { + child, + process: { hostId: 'local', pid, processStartTimeMs, spawnToken } + } +} + +async function stopOwner(child: ReturnType): Promise { + if (child.exitCode === null && child.signalCode === null) { + const closed = new Promise((resolve) => child.once('close', () => resolve())) + child.kill('SIGTERM') + await closed + } + spawnedOwners.delete(child) +} function adapter(): StructuredAgentSessionAdapter { return { @@ -75,6 +111,9 @@ beforeEach(async () => { afterEach(async () => { await host.flushAllStreamedEvents() + await Promise.all([...supersededHosts].map((superseded) => superseded.flushAllStreamedEvents())) + supersededHosts.clear() + await Promise.all([...spawnedOwners].map((child) => stopOwner(child))) await rm(root, { recursive: true, force: true }) }) @@ -140,7 +179,7 @@ describe('recovery exits', () => { }) }) - it('heals a stranded native owner during startup restore without waiting for a client', async () => { + it('heals a stranded native owner during startup restore, and spawns nothing until a surface asks', async () => { expect((await host.attach(CALLER, hostTestAttachParams(null))).ok).toBe(true) await reopenStore() @@ -159,7 +198,18 @@ describe('recovery exits', () => { await host.restoreReadableSessions() + // Healing is startup's job; spawning is not. The orphan is stopped and the lease is free, but + // nothing has asked to look at this session, so no replacement child exists yet. expect(stopOwnerProcess).toHaveBeenCalledWith(4242, 'SIGTERM') + expect(acquire).toHaveBeenCalledTimes(1) + expect(store.getRecord(SESSION)?.lease).toMatchObject({ + claimStatus: 'released', + handoffStage: null, + ownerProcess: null + }) + + await host.hold(SESSION, 'surface-1') + expect(acquire).toHaveBeenCalledTimes(2) expect(store.getRecord(SESSION)?.lease).toMatchObject({ runtimeFence: 3, @@ -168,4 +218,73 @@ describe('recovery exits', () => { ownerProcess: { spawnToken: 'spawn-b' } }) }) + + it('recovers when the outgoing runtime writes after the replacement probes its dying owner', async () => { + const outgoing = await spawnOwner('spawn-a') + acquire.mockResolvedValueOnce({ + process: outgoing.process, + link: { + linkId: 'link-outgoing', + handle: { provider: 'codex', threadId: THREAD }, + origin: 'created', + mintedAtFence: 1, + observedAt: NOW + } + }) + expect((await host.attach(CALLER, hostTestAttachParams(null))).ok).toBe(true) + + const outgoingHost = host + const outgoingStore = store + supersededHosts.add(outgoingHost) + store = await AgentSessionRecordStore.open({ directory: join(root, 'store'), hostId: 'local' }) + const realProbe = createStructuredAgentSessionOwnerProbe('local') + let overlapDriven = false + openHost({ + mintSpawnToken: () => 'spawn-b', + probeOwner: async (record) => { + const probe = await realProbe(record) + if (!overlapDriven) { + overlapDriven = true + await outgoingStore.renewLease({ + sessionId: SESSION, + fence: 1, + childProbe: probe, + now: NOW + 1 + }) + await stopOwner(outgoing.child) + } + return probe + } + }) + + await host.restoreReadableSessions() + expect(store.getRecord(SESSION)?.lease).toMatchObject({ + runtimeFence: 2, + claimStatus: 'released', + ownerProcess: null + }) + expect(acquire).toHaveBeenCalledOnce() + + const replacement = await spawnOwner('spawn-b') + acquire.mockResolvedValueOnce({ + process: replacement.process, + link: { + linkId: 'link-replacement', + handle: { provider: 'codex', threadId: THREAD }, + origin: 'resumed', + mintedAtFence: 3, + observedAt: NOW + 2 + } + }) + + await host.hold(SESSION, 'surface-overlap') + + expect(store.getRecord(SESSION)?.lease).toMatchObject({ + runtimeFence: 3, + claimStatus: 'live', + handoffStage: null, + ownerProcess: { pid: replacement.process.pid, spawnToken: 'spawn-b' } + }) + expect(host.history({ sessionId: SESSION, direction: 'tail' }).ok).toBe(true) + }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-release-clock.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-release-clock.ts new file mode 100644 index 000000000000..452e6a6fae03 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-release-clock.ts @@ -0,0 +1,75 @@ +// The delay between "nothing holds this session" and "stop its provider child". +// +// TWO reasons it is not immediate. A surface that reconnects — a mobile socket dropping on a +// network switch, a renderer remounting a tab — releases and re-holds within a second, and killing +// an app-server in that window costs the user a respawn plus a resume for nothing. And a turn the +// user already asked for must finish: the provider is mid-answer, the journal has an open turn +// marker, and stopping the child there strands both. +// +// So the clock arms when the last holder leaves, and a tick that finds a turn still running RE-ARMS +// instead of evicting. That is what makes the wait start at the later of the two events rather than +// at whichever came first. + +export const STRUCTURED_AGENT_SESSION_RELEASE_GRACE_MS = 15_000 + +export type StructuredAgentSessionReleaseClockDeps = { + /** Never evict mid-turn; a true answer re-arms the clock instead. */ + isTurnActive: (sessionId: string) => boolean + /** Re-checked at fire time: a holder may have arrived while the timer ran. */ + isHeld: (sessionId: string) => boolean + evict: (sessionId: string) => Promise + onError?: (input: { sessionId: string; error: unknown }) => void + graceMs?: number +} + +export class StructuredAgentSessionReleaseClock { + private readonly timers = new Map>() + private readonly graceMs: number + + constructor(private readonly deps: StructuredAgentSessionReleaseClockDeps) { + this.graceMs = deps.graceMs ?? STRUCTURED_AGENT_SESSION_RELEASE_GRACE_MS + } + + arm(sessionId: string): void { + this.cancel(sessionId) + const timer = setTimeout(() => { + this.timers.delete(sessionId) + this.fire(sessionId) + }, this.graceMs) + // A pending release must never be the reason a process stays alive at quit. + timer.unref?.() + this.timers.set(sessionId, timer) + } + + cancel(sessionId: string): void { + const timer = this.timers.get(sessionId) + if (timer) { + clearTimeout(timer) + this.timers.delete(sessionId) + } + } + + isArmed(sessionId: string): boolean { + return this.timers.has(sessionId) + } + + dispose(): void { + for (const timer of this.timers.values()) { + clearTimeout(timer) + } + this.timers.clear() + } + + private fire(sessionId: string): void { + if (this.deps.isHeld(sessionId)) { + return + } + if (this.deps.isTurnActive(sessionId)) { + this.arm(sessionId) + return + } + void this.deps.evict(sessionId).catch((error: unknown) => { + this.deps.onError?.({ sessionId, error }) + }) + } +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-reconcile.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-reconcile.ts index d043c6c7d950..03b16c6b35bf 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-reconcile.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-reconcile.ts @@ -9,6 +9,8 @@ import type { AgentSessionWireRefusal } from '../../../shared/agent-session-wire import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { classifyStoreFailure } from './structured-agent-session-attach' +const MAX_RECONCILIATION_PASSES = 8 + /** Adjudicates leases loaded by this process or refreshed from another writer. * Answers with the refusal attach owes its caller, or null once settled. */ export function createRestartReconciler(deps: { @@ -22,9 +24,7 @@ export function createRestartReconciler(deps: { return null } if (!pending) { - const run = deps.store - .reconcileOnRestart({ probe: deps.probe, now: deps.now() }) - .then(() => undefined) + const run = reconcileCurrentLeases(deps) pending = run.finally(() => { pending = null }) @@ -41,3 +41,18 @@ export function createRestartReconciler(deps: { } } } + +async function reconcileCurrentLeases(deps: { + store: AgentSessionRecordStore + probe: (record: AgentSessionRecord) => Promise + now: () => number +}): Promise { + for (let pass = 0; pass < MAX_RECONCILIATION_PASSES; pass += 1) { + await deps.store.reconcileOnRestart({ probe: deps.probe, now: deps.now() }) + if (!deps.store.listRecords().some((record) => record.lease.unreconciled)) { + return + } + } + // An outgoing runtime can still be writing during restart; preserve the record and retry later. + throw new Error('execution_owner_reconciling') +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.ts index 721fe187cc44..7149552ead7b 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.ts @@ -1,9 +1,20 @@ +// What a restart owes a persisted session, and what it does NOT. +// +// It owes reconciliation — every lease loaded from disk names an owner from a process generation +// that no longer exists, and adjudicating that is startup's job. It owes an exit from any recovery +// stage the evidence now permits. And it owes a READABLE session: the journal open, history +// answerable, the tab restorable. +// +// It does not owe a provider child. This used to resume every record whose lease was `released` +// with no handoff in flight, which is the normal end state of a chat the user closed cleanly — so a +// healthy profile started an app-server per session it had ever used, in parallel, at every launch, +// with no client attached and nothing on screen. A child now exists because a surface asked for the +// session (see `structured-agent-session-holds`), not because a record survived on disk. + import type { AgentSessionRecord } from '../../../shared/agent-session-record' import type { AgentSessionWireRefusal } from '../../../shared/agent-session-wire' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' -import type { AgentSessionAttachParams } from './structured-agent-session-attach' import { - attachParamsForRecord, restoreStructuredAgentSessionRead, type RestoredStructuredAgentSessionRead } from './structured-agent-session-read-restore' @@ -14,8 +25,6 @@ export async function restoreStructuredAgentSessionsOnRestart(input: { records: AgentSessionRecord[] reconcile: (sessionId: string) => Promise resolveRecovery: (sessionId: string) => Promise - operationId: () => string - resume: (params: AgentSessionAttachParams) => Promise serialize: (sessionId: string, task: () => Promise) => Promise hasSession: (sessionId: string) => boolean onReadable: (sessionId: string, restored: RestoredStructuredAgentSessionRead) => void @@ -28,23 +37,9 @@ export async function restoreStructuredAgentSessionsOnRestart(input: { // A session latched in recovery exits here at startup, without waiting for a client. await input.resolveRecovery(sessionId) } - const current = input.store.getRecord(sessionId) - if ( - !unreconciled && - current?.lease.claimStatus === 'released' && - current.lease.handoffStage === null && - (await input.resume( - attachParamsForRecord(current, { - clientOperationId: input.operationId(), - expectedRuntimeFence: current.lease.runtimeFence, - runtimeKind: 'native' - }) - )) - ) { - return - } await input.serialize(sessionId, async () => { if (input.hasSession(sessionId)) { + // A surface that took a hold mid-restore already attached this one. await input.restoreHandoff(sessionId) return } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-resume-eligibility.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-resume-eligibility.ts new file mode 100644 index 000000000000..0baf6fe99527 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-resume-eligibility.ts @@ -0,0 +1,41 @@ +// When a record may be handed back a provider child. +// +// This used to be inline in the restart restore, which is what made it a STARTUP rule: every record +// whose lease looked like this got a child, at launch, whether or not anything was going to look at +// it. `released` + no handoff is the normal end state of a chat the user closed cleanly, so a +// healthy profile respawned everything it had ever opened. The predicate itself was never wrong — +// it answers "may this be resumed", not "should it be" — so it lives here now and the caller that +// knows a surface is asking is the only one that acts on it. + +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import { randomUUID } from 'node:crypto' +import type { AgentSessionAttachParams } from './structured-agent-session-attach' +import { attachParamsForRecord } from './structured-agent-session-read-restore' + +export function isResumableStructuredAgentSessionRecord(record: AgentSessionRecord): boolean { + return ( + !record.lease.unreconciled && + record.lease.claimStatus === 'released' && + record.lease.handoffStage === null + ) +} + +/** Attach params for a resume, or null when this record's lease is somebody else's problem. */ +export function structuredAgentSessionResumeParams( + record: AgentSessionRecord, + clientOperationId: string +): AgentSessionAttachParams | null { + if (!isResumableStructuredAgentSessionRecord(record)) { + return null + } + return attachParamsForRecord(record, { + clientOperationId, + expectedRuntimeFence: record.lease.runtimeFence, + runtimeKind: 'native' + }) +} + +/** `<13-digit ms>-<32 hex>`, the only operation-id shape the durable ledger admits. */ +export function structuredAgentSessionResumeOperationId(now: number): string { + return `${Math.trunc(now).toString().padStart(13, '0')}-${randomUUID().replaceAll('-', '')}` +} diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts new file mode 100644 index 000000000000..2cdf5e24ccb3 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts @@ -0,0 +1,246 @@ +// The lifetime of a provider child, from the surfaces that hold the session. +// +// Two leaks meet here and each has to be tested against the real host, not a double: a chat that +// closes without stopping its app-server, and a launch that starts one for every record on disk. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import type { AgentSessionOwnerProbe } from '../../../shared/agent-session-lease-adjudication' +import type { AgentSessionSubscribeEvent } from '../../../shared/agent-session-wire' +import { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' +import type { StructuredAgentSessionAdapter } from './structured-agent-session-adapter' +import type { StructuredAgentSessionEventSink } from './structured-agent-session-event-sink' +import { StructuredAgentSessionHost } from './structured-agent-session-host' +import { + HOST_TEST_NOW as NOW, + HOST_TEST_SESSION as SESSION, + HOST_TEST_THREAD as THREAD, + hostTestAttachParams, + resetHostTestOperationIds +} from './structured-agent-session-host-test-data' + +const CALLER = { callerKey: 'client-1' } +const SURFACE = 'desktop-chat:1' +/** Short enough to keep the suite fast, long enough that an eviction is a decision and not a race. */ +const GRACE_MS = 5 + +let root: string +let store: AgentSessionRecordStore +let host: StructuredAgentSessionHost +let acquire: Mock +let closeSession: Mock> +let sink: StructuredAgentSessionEventSink | null +let hostErrors: unknown[] + +function adapter(): StructuredAgentSessionAdapter { + return { + acquire, + closeSession, + releaseAcquisition: vi.fn(async () => undefined), + dispatch: vi.fn(async () => ({ state: 'rejected' as const, reason: 'unused' })), + cancelTurn: vi.fn(async () => ({ cancelled: false })), + answerPrompt: vi.fn(async () => undefined), + setOption: vi.fn(async () => undefined) + } +} + +function openHost(probeOwner?: (record: never) => Promise): void { + host = new StructuredAgentSessionHost({ + store, + adapter: adapter(), + journalRoot: root, + claimKeyId: 'key-1', + mintSpawnToken: () => `spawn-${acquire.mock.calls.length}`, + releaseGraceMs: GRACE_MS, + now: () => NOW, + onEventSinkError: ({ error }) => hostErrors.push(error), + ...(probeOwner ? { probeOwner: probeOwner as never } : {}) + }) +} + +/** A fresh app generation over the same durable store, with its owner proven gone. */ +async function reboot(): Promise { + await host.flushAllStreamedEvents() + store = await AgentSessionRecordStore.open({ directory: join(root, 'store'), hostId: 'local' }) + openHost(async () => ({ outcome: 'pid-absent' })) + acquire.mockClear() + closeSession.mockClear() +} + +async function attach(): Promise { + expect(await host.attach(CALLER, hostTestAttachParams(null))).toMatchObject({ ok: true }) +} + +function emitTurnLifecycle(state: 'running' | 'completed', ordinal: number): void { + sink?.appendItem( + { provider: 'codex', threadId: THREAD, turnId: 'turn-1', ordinal }, + { kind: 'status', text: state, turnLifecycle: { turnId: 'turn-1', state } } + ) +} + +/** Eviction is a sequence, not an event: the child stops first and the session is forgotten last. */ +function waitForEviction(): Promise { + return vi.waitFor(() => { + expect(closeSession).toHaveBeenCalledWith(SESSION) + expect(host.hasSession(SESSION)).toBe(false) + }) +} + +/** Long enough for several grace windows to elapse, so "not evicted" means the clock declined. */ +function waitOutSeveralGraceWindows(): Promise { + return new Promise((resolve) => setTimeout(resolve, GRACE_MS * 20)) +} + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-surface-lifetime-')) + resetHostTestOperationIds() + sink = null + hostErrors = [] + acquire = vi.fn(async ({ fence, spawnToken, events }) => { + sink = events ?? null + return { + process: { hostId: 'local', pid: 4242, processStartTimeMs: 1_700_000_000_000, spawnToken }, + link: { + linkId: `link-${fence}`, + handle: { provider: 'codex' as const, threadId: THREAD }, + origin: store.getRecord(SESSION)?.providerHandleChain.length + ? ('resumed' as const) + : ('created' as const), + mintedAtFence: fence, + observedAt: NOW + } + } + }) + closeSession = vi.fn(async () => undefined) + store = await AgentSessionRecordStore.open({ directory: join(root, 'store'), hostId: 'local' }) + openHost() +}) + +afterEach(async () => { + await host.flushAllStreamedEvents() + await rm(root, { recursive: true, force: true }) +}) + +describe('a chat that closes', () => { + it('releases the provider child it was holding', async () => { + await attach() + await host.hold(SESSION, SURFACE) + + host.release(SESSION, SURFACE) + + await waitForEviction() + expect(hostErrors).toEqual([]) + // The record and its journal stay; only the process and the claim on it go. + expect(store.getRecord(SESSION)?.lease).toMatchObject({ + claimStatus: 'released', + ownerProcess: null, + deathEvidence: { kind: 'exit-observed' } + }) + }) + + it('keeps the child while another surface still holds the session', async () => { + await attach() + await host.hold(SESSION, SURFACE) + await host.hold(SESSION, 'paired-phone:1') + + host.release(SESSION, SURFACE) + await waitOutSeveralGraceWindows() + + expect(closeSession).not.toHaveBeenCalled() + expect(host.hasSession(SESSION)).toBe(true) + }) + + it('does not lose the session to a release the client sent twice', async () => { + await attach() + await host.hold(SESSION, SURFACE) + await host.hold(SESSION, 'paired-phone:1') + + // A retried release must retire ONE holder, which is what a set gets right and a count does not. + host.release(SESSION, SURFACE) + host.release(SESSION, SURFACE) + await waitOutSeveralGraceWindows() + + expect(closeSession).not.toHaveBeenCalled() + expect(host.hasSession(SESSION)).toBe(true) + }) +}) + +describe('a session with a turn in flight', () => { + it('is not evicted while the turn runs, and is once it ends', async () => { + await attach() + await host.hold(SESSION, SURFACE) + emitTurnLifecycle('running', 1) + await host.flushStreamedEvents(SESSION) + + host.release(SESSION, SURFACE) + await waitOutSeveralGraceWindows() + + expect(closeSession).not.toHaveBeenCalled() + expect(host.hasSession(SESSION)).toBe(true) + + emitTurnLifecycle('completed', 2) + await host.flushStreamedEvents(SESSION) + + await waitForEviction() + }) +}) + +describe('startup', () => { + it('restores a session for reading without spawning a provider child', async () => { + await attach() + await reboot() + + await host.restoreReadableSessions() + + // The record is readable — the tab comes back, history answers — and nothing is running. + expect(acquire).not.toHaveBeenCalled() + expect(host.listSessionTabs()).toEqual([ + { sessionId: SESSION, workspaceId: 'workspace-1', agent: 'codex' } + ]) + expect(host.history({ sessionId: SESSION, direction: 'tail' }).ok).toBe(true) + expect(store.getRecord(SESSION)?.lease.claimStatus).toBe('released') + }) + + it('gives the child back to a chat a surface actually opens', async () => { + await attach() + await reboot() + await host.restoreReadableSessions() + + await host.hold(SESSION, SURFACE) + + expect(acquire).toHaveBeenCalledOnce() + expect(store.getRecord(SESSION)?.lease).toMatchObject({ + claimStatus: 'live', + runtimeKind: 'native', + ownerProcess: { pid: 4242 } + }) + }) +}) + +describe('a session evicted and opened again', () => { + it('publishes provider events to the reattached chat', async () => { + await attach() + await host.hold(SESSION, SURFACE) + host.release(SESSION, SURFACE) + await waitForEviction() + + await host.hold(SESSION, 'desktop-chat:2') + const events: AgentSessionSubscribeEvent[] = [] + const unsubscribe = host.subscribe({ + id: 'subscriber-1', + sessionId: SESSION, + emit: (event) => events.push(event) + }) + sink?.appendItem( + { provider: 'codex', threadId: THREAD, turnId: 'turn-2', ordinal: 1 }, + { kind: 'message', role: 'assistant', blocks: [{ type: 'text', text: 'back again' }] } + ) + sink?.publish() + await host.flushStreamedEvents(SESSION) + unsubscribe() + + expect(JSON.stringify(events)).toContain('back again') + }) +}) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-tui-adoption.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-tui-adoption.ts index f8aeacf3250f..5dc0cd4bb103 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-tui-adoption.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-tui-adoption.ts @@ -64,7 +64,13 @@ async function adopt( journalRoot: input.deps.journalRoot, adapter: input.deps.adapter }) - const session = { journal: attached.journal, params, fence: record.lease.runtimeFence } + // The TUI owns the process here; this host publishes the journal only. + const session = { + journal: attached.journal, + params, + fence: record.lease.runtimeFence, + hasProviderChild: false + } input.publish(sessionId, session) input.retain(sessionId, owner) input.snapshot(sessionId, session) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-wedged-profile-migration.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-wedged-profile-migration.test.ts index 67f008527aa3..e549ea017a6f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-wedged-profile-migration.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-wedged-profile-migration.test.ts @@ -4,11 +4,17 @@ // handoff restore, and no manual recovery can move, so the chat behind it never opens again. The // contract is that loading the record under this build makes it usable WITHOUT losing the // conversation — the journal, the provider handle chain, and the recorded evidence all survive. +// +// "Usable" means ACQUIRABLE, not acquired. Startup no longer resumes a provider child for a record +// nobody is looking at; a surface taking a hold is what spawns one. So the migration's job is to +// leave the lease in a state a hold can claim, and these tests prove that by adjudicating it rather +// than by reading fields off it. import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import { evaluateAgentSessionAcquisition } from '../../../shared/agent-session-lease-adjudication' import type { AgentSessionClaimStatus, AgentSessionHandoffStage, @@ -156,6 +162,18 @@ afterEach(async () => { await rm(root, { recursive: true, force: true }) }) +/** The property that matters: a hold taken now would be granted a lease. */ +function isAcquirable(lease: NonNullable>['lease']): boolean { + return ( + evaluateAgentSessionAcquisition({ + lease, + expectedFence: lease.runtimeFence, + handoffOperationId: null, + probe: { outcome: 'reservation-unused' } + }).decision === 'granted' + ) +} + describe('already-wedged profiles become usable on load', () => { it('re-adjudicates a conflicted manual-recovery record whose owner is provably gone', async () => { // The v1 -> v2 record upgrade stamps every migrated row `conflicted` + `manual-recovery`, and @@ -172,8 +190,9 @@ describe('already-wedged profiles become usable on load', () => { await host.restoreReadableSessions() - const lease = store.getRecord(SESSION)?.lease - expect(lease).toMatchObject({ claimStatus: 'live', handoffStage: null }) + const lease = store.getRecord(SESSION)!.lease + expect(lease).toMatchObject({ handoffStage: null, unreconciled: false }) + expect(isAcquirable(lease)).toBe(true) // A real re-adjudication, not a no-op: the eviction minted a new generation. expect(lease?.runtimeFence).toBeGreaterThan(13) // The conversation survived: the codex thread was resumed, not recreated. @@ -181,7 +200,9 @@ describe('already-wedged profiles become usable on load', () => { linkId: 'codex-13-link', handle: { threadId: THREAD } }) - expect(acquire).toHaveBeenCalledTimes(1) + // Why NOT acquired here: startup spawning a provider child for every recovered record is the + // accumulation this stack removed. Unlatching is the migration's job; spawning is a hold's. + expect(acquire).not.toHaveBeenCalled() }) it('leaves a conflicted record alone while its owner cannot be proven gone', async () => { @@ -213,10 +234,9 @@ describe('already-wedged profiles become usable on load', () => { await host.restoreReadableSessions() - expect(store.getRecord(SESSION)?.lease).toMatchObject({ - claimStatus: 'live', - handoffStage: null - }) + const lease = store.getRecord(SESSION)!.lease + expect(lease).toMatchObject({ handoffStage: null, unreconciled: false }) + expect(isAcquirable(lease)).toBe(true) }) it('exits a TUI reservation that crashed before its identity was committed', async () => { @@ -247,10 +267,9 @@ describe('already-wedged profiles become usable on load', () => { await host.restoreReadableSessions() - expect(store.getRecord(SESSION)?.lease).toMatchObject({ - claimStatus: 'live', - handoffStage: null - }) + const lease = store.getRecord(SESSION)!.lease + expect(lease).toMatchObject({ handoffStage: null, unreconciled: false }) + expect(isAcquirable(lease)).toBe(true) }) it("stops a child carrying a lost record's spawn token before recovery respawns", async () => { @@ -289,6 +308,11 @@ describe('already-wedged profiles become usable on load', () => { }) await host.restoreReadableSessions() + // Startup only reaps now; a hold is what asks for a child. The ordering property is unchanged + // and still the point: the orphan holding the lost token must be gone BEFORE anything is + // granted a new owner for that provider session, or two children race one conversation. + expect(order).toEqual(['stop:31337']) + await host.hold(SESSION, 'holder-1') expect(order).toEqual(['stop:31337', 'acquire']) }) diff --git a/src/main/native-chat/agent-session-wire/structured-tui-transcript-catchup.test.ts b/src/main/native-chat/agent-session-wire/structured-tui-transcript-catchup.test.ts index 8969ed0d68c9..b94d671fa9aa 100644 --- a/src/main/native-chat/agent-session-wire/structured-tui-transcript-catchup.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-tui-transcript-catchup.test.ts @@ -99,7 +99,12 @@ async function createCatchupFixture() { function createCatchup(input: Awaited>) { return new StructuredTuiTranscriptCatchup({ store, - session: () => ({ journal: input.journal, params: {} as never, fence: input.fence }), + session: () => ({ + hasProviderChild: false, + journal: input.journal, + params: {} as never, + fence: input.fence + }), schedule: async (_sessionId, task) => task(), publish: vi.fn(), reset: vi.fn() diff --git a/src/main/runtime/agent-session-backup-recovery.test.ts b/src/main/runtime/agent-session-backup-recovery.test.ts index a39d95e05dfc..18d1ec00bda4 100644 --- a/src/main/runtime/agent-session-backup-recovery.test.ts +++ b/src/main/runtime/agent-session-backup-recovery.test.ts @@ -142,7 +142,11 @@ describe('crash-safe store writes', () => { const { mkdir } = await import('node:fs/promises') await mkdir(`${storePath}.bak`) - await expect(saveAgentSessionStore(storePath, loaded.state)).rejects.toBeTruthy() + await expect( + saveAgentSessionStore(storePath, loaded.state, { + primaryStatus: 'validated' + }) + ).rejects.toBeTruthy() expect(await readFile(storePath, 'utf-8')).toBe(committed) expect((await stat(`${storePath}.bak`)).isDirectory()).toBe(true) }) diff --git a/src/main/runtime/agent-session-lease-renewal.test.ts b/src/main/runtime/agent-session-lease-renewal.test.ts new file mode 100644 index 000000000000..0c1c88db0031 --- /dev/null +++ b/src/main/runtime/agent-session-lease-renewal.test.ts @@ -0,0 +1,120 @@ +import { mkdtemp, readFile, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import type { AgentSessionRecord } from '../../shared/agent-session-record' +import { AgentSessionRecordStore } from './agent-session-record-store' +import { agentSessionStorePath } from './agent-session-record-store-file' + +const NOW = 1_800_000_000_000 +const MATCHED = { outcome: 'identity-matched', matchedOn: ['spawn-token'] } as const +const directories: string[] = [] + +async function establishOwner( + store: AgentSessionRecordStore, + directory: string, + suffix: string +): Promise { + const sessionId = `session-${suffix}` + const spawnToken = `spawn-${suffix}` + const reserved = await store.reserveOwner({ + sessionId, + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'workspace-1', + workspaceKind: 'folder' + }, + provider: 'codex', + accountHome: { variable: 'CODEX_HOME', path: directory }, + runtimeKind: 'native', + expectedFence: null, + spawnToken, + claimKeyId: 'key-1', + handoffOperationId: null, + probe: { outcome: 'reservation-unused' }, + operation: { + callerKey: 'renewal-test', + operationId: `${NOW}-${suffix.padStart(32, '0')}`, + fingerprint: `create-${suffix}` + }, + now: NOW + }) + const fence = reserved.record.lease.runtimeFence + await store.commitProcessIdentity({ + sessionId, + fence, + process: { hostId: 'local', pid: 4242, processStartTimeMs: NOW - 1_000, spawnToken }, + now: NOW + }) + return store.proveOwner({ + sessionId, + fence, + link: { + linkId: `link-${suffix}`, + handle: { provider: 'codex', threadId: `thread-${suffix}` }, + origin: 'created', + mintedAtFence: fence, + observedAt: NOW + }, + now: NOW + }) +} + +async function liveStore(): Promise<{ directory: string; store: AgentSessionRecordStore }> { + const directory = await mkdtemp(join(tmpdir(), 'orca-lease-renewal-batch-')) + directories.push(directory) + const store = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + await establishOwner(store, directory, 'a') + await establishOwner(store, directory, 'b') + return { directory, store } +} + +afterEach(async () => { + await Promise.all(directories.splice(0).map((path) => rm(path, { recursive: true, force: true }))) +}) + +describe('agent-session lease renewal batch', () => { + it('refuses a stale fence without changing the record', async () => { + const { store } = await liveStore() + + await expect( + store.renewLeases([ + { sessionId: 'session-a', fence: 0, childProbe: MATCHED, now: NOW + 10_000 } + ]) + ).rejects.toThrow('agent_session_checkpoint_stale') + expect(store.getRecord('session-a')?.lease.lastRenewedAt).toBe(NOW) + }) + + it('leaves every session durable when a later renewal was superseded', async () => { + const { directory, store } = await liveStore() + await store.evictProvenDeadOwner({ + sessionId: 'session-b', + expectedFence: 1, + probe: { outcome: 'pid-absent' }, + now: NOW + 5_000 + }) + const beforeDisk = await readFile(agentSessionStorePath(directory), 'utf-8') + const beforeFirst = store.getRecord('session-a') + + await expect( + store.renewLeases([ + { sessionId: 'session-a', fence: 1, childProbe: MATCHED, now: NOW + 10_000 }, + { sessionId: 'session-b', fence: 1, childProbe: MATCHED, now: NOW + 10_000 } + ]) + ).rejects.toThrow('agent_session_checkpoint_stale') + + expect(store.getRecord('session-a')).toEqual(beforeFirst) + expect(await readFile(agentSessionStorePath(directory), 'utf-8')).toBe(beforeDisk) + const reopened = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + expect( + reopened + .listRecords() + .map((record) => record.sessionId) + .sort() + ).toEqual(['session-a', 'session-b']) + expect(reopened.listRecords().every((record) => record.providerHandleChain.length > 0)).toBe( + true + ) + }) +}) diff --git a/src/main/runtime/agent-session-lease-renewal.ts b/src/main/runtime/agent-session-lease-renewal.ts new file mode 100644 index 000000000000..1a1f90dd7e1c --- /dev/null +++ b/src/main/runtime/agent-session-lease-renewal.ts @@ -0,0 +1,38 @@ +import type { AgentSessionOwnerProbe } from '../../shared/agent-session-lease-adjudication' +import type { AgentSessionRecord } from '../../shared/agent-session-record' +import { renewAgentSessionLease } from './agent-session-lease-transitions' +import type { AgentSessionStoreState } from './agent-session-record-store-file' + +export type AgentSessionLeaseRenewal = { + sessionId: string + fence: number + childProbe: AgentSessionOwnerProbe + now: number + leaseTtlMs?: number +} + +export function renewAgentSessionLeases( + state: AgentSessionStoreState, + renewals: readonly AgentSessionLeaseRenewal[], + defaultLeaseTtlMs: number +): AgentSessionRecord[] { + return renewals.map((args) => { + const record = state.records.get(args.sessionId) + if (!record) { + throw new Error( + state.unreadableRecords.has(args.sessionId) + ? 'execution_owner_reconciling' + : 'agent_session_identity_required' + ) + } + const renewed = renewAgentSessionLease({ + record, + fence: args.fence, + childProbe: args.childProbe, + now: args.now, + leaseTtlMs: args.leaseTtlMs ?? defaultLeaseTtlMs + }) + state.records.set(args.sessionId, renewed) + return renewed + }) +} diff --git a/src/main/runtime/agent-session-process-identity-probe.test.ts b/src/main/runtime/agent-session-process-identity-probe.test.ts index 5f310a2a6ee2..59cab6d41cdf 100644 --- a/src/main/runtime/agent-session-process-identity-probe.test.ts +++ b/src/main/runtime/agent-session-process-identity-probe.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import type { AgentSessionProcessIdentity } from '../../shared/agent-session-record' import { PROCESS_START_TIME_TOLERANCE_MS, + probeAgentSessionProcessIdentities, probeAgentSessionProcessIdentity, probeAgentSessionReservation, readProcessStartTimeMs, @@ -28,6 +29,32 @@ function deps(overrides: AgentSessionProcessProbeDeps = {}): AgentSessionProcess } describe('owner identity probe', () => { + it('shares one process-table read across a batch without weakening identity checks', async () => { + const readProcessStartTimes = vi.fn( + async () => + new Map([ + [4242, START_TIME], + [4243, START_TIME + PROCESS_START_TIME_TOLERANCE_MS + 1] + ]) + ) + const probes = await probeAgentSessionProcessIdentities({ + identities: [IDENTITY, { ...IDENTITY, pid: 4243, spawnToken: 'spawn-b' }], + deps: { + isPidPresent: () => true, + readEchoedSpawnToken: async () => null, + readProcessStartTimesMs: readProcessStartTimes, + platform: 'darwin' + } + }) + + expect(readProcessStartTimes).toHaveBeenCalledOnce() + expect(readProcessStartTimes).toHaveBeenCalledWith([4242, 4243], 'darwin') + expect(probes).toEqual([ + { outcome: 'identity-matched', matchedOn: ['process-start-time'] }, + { outcome: 'identity-mismatch', field: 'process-start-time' } + ]) + }) + it('reports an observed exit without touching the host', () => { return expect( probeAgentSessionProcessIdentity({ diff --git a/src/main/runtime/agent-session-process-identity-probe.ts b/src/main/runtime/agent-session-process-identity-probe.ts index 0ee9411dc667..749380583f41 100644 --- a/src/main/runtime/agent-session-process-identity-probe.ts +++ b/src/main/runtime/agent-session-process-identity-probe.ts @@ -76,6 +76,39 @@ async function readDarwinProcessStartTimeMs(pid: number): Promise } } +async function readDarwinProcessStartTimesMs( + pids: readonly number[] +): Promise> { + const observed = new Map() + if (pids.length === 0) { + return observed + } + try { + const result = await runProcess({ + program: 'ps', + args: ['-o', 'pid=,lstart=', '-p', pids.join(',')], + timeoutMs: PROCESS_START_TIME_TIMEOUT_MS + }) + if (result.timedOut || result.code !== 0) { + return observed + } + for (const line of result.stdout.split('\n')) { + const match = /^\s*(\d+)\s+(.+?)\s*$/.exec(line) + if (!match) { + continue + } + const pid = Number(match[1]) + const parsed = Date.parse(match[2]) + if (Number.isSafeInteger(pid) && Number.isFinite(parsed)) { + observed.set(pid, parsed) + } + } + } catch { + // A missing process table is unknown, never evidence that every owner exited. + } + return observed +} + async function readWindowsProcessStartTimeMs(pid: number): Promise { try { const script = @@ -116,6 +149,57 @@ export async function readProcessStartTimeMs( return null } +export async function readProcessStartTimesMs( + pids: readonly number[], + platform: NodeJS.Platform = process.platform +): Promise> { + const uniquePids = [...new Set(pids)] + if (platform === 'darwin') { + const table = await readDarwinProcessStartTimesMs(uniquePids) + return new Map(uniquePids.map((pid) => [pid, table.get(pid) ?? null])) + } + return new Map( + await Promise.all( + uniquePids.map(async (pid) => [pid, await readProcessStartTimeMs(pid, platform)] as const) + ) + ) +} + +export type AgentSessionProcessBatchProbeDeps = Omit< + AgentSessionProcessProbeDeps, + 'readProcessStartTimeMs' +> & { + readProcessStartTimesMs?: ( + pids: readonly number[], + platform?: NodeJS.Platform + ) => Promise> +} + +export async function probeAgentSessionProcessIdentities(args: { + identities: readonly AgentSessionProcessIdentity[] + deps?: AgentSessionProcessBatchProbeDeps +}): Promise { + const deps = args.deps ?? {} + const platform = deps.platform ?? process.platform + const pids = args.identities + .filter((identity) => identity.processStartTimeMs !== null) + .map((identity) => identity.pid) + const readStartTimes = deps.readProcessStartTimesMs ?? readProcessStartTimesMs + const startTimes = await readStartTimes(pids, platform).catch(() => new Map()) + return Promise.all( + args.identities.map((identity) => + probeAgentSessionProcessIdentity({ + identity, + deps: { + ...deps, + platform, + readProcessStartTimeMs: async (pid) => startTimes.get(pid) ?? null + } + }) + ) + ) +} + /** * Probe one recorded owner. `observedExit` short-circuits everything: Orca watching that exact * process exit is the strongest evidence available. diff --git a/src/main/runtime/agent-session-record-store-file.ts b/src/main/runtime/agent-session-record-store-file.ts index 133a03afc4a9..9068ae6a6035 100644 --- a/src/main/runtime/agent-session-record-store-file.ts +++ b/src/main/runtime/agent-session-record-store-file.ts @@ -309,7 +309,8 @@ function serializeState(state: AgentSessionStoreState): string { /** * Commit the whole state. The live path is never absent: the new content is made durable in a temp - * file first, the old content is COPIED to the backup, and only then does the rename publish it. + * file first, a validated primary is COPIED to the backup, and only then does the rename publish it. + * Backup recovery keeps the known-good backup in place while publishing the repaired primary. * * The old ordering renamed the live file aside before writing the new one, so a death in that * window left the profile with a backup and no primary — which is exactly the state that wedged a @@ -317,7 +318,8 @@ function serializeState(state: AgentSessionStoreState): string { */ export async function saveAgentSessionStore( filePath: string, - state: AgentSessionStoreState + state: AgentSessionStoreState, + options: { primaryStatus: 'validated' | 'unusable-or-absent' } ): Promise { const directory = dirname(filePath) await mkdir(directory, { recursive: true, mode: 0o700 }) @@ -325,9 +327,11 @@ export async function saveAgentSessionStore( const tmpPath = durableWriteTempPath(filePath) try { await writeTempFileDurable(tmpPath, serializeState(state), 0o600) - // A failed rotation aborts the save. Recovery's fence floor assumes the backup is at most one - // committed generation behind; letting the primary advance past a stale backup breaks that. - await copyFileDurable(filePath, backupPath(filePath)) + // Only a primary parsed under the transaction lock may replace the backup. During recovery the + // primary is corrupt or absent, so the known-good backup must survive until publication. + if (options.primaryStatus === 'validated') { + await copyFileDurable(filePath, backupPath(filePath)) + } await renameDurable(tmpPath, filePath) } catch (error) { await rm(tmpPath, { force: true }).catch(() => {}) diff --git a/src/main/runtime/agent-session-record-store.ts b/src/main/runtime/agent-session-record-store.ts index bf3f9a197517..bc341c912ad7 100644 --- a/src/main/runtime/agent-session-record-store.ts +++ b/src/main/runtime/agent-session-record-store.ts @@ -26,10 +26,13 @@ import { commitAgentSessionProcessIdentity, evictAgentSessionOwner, proveAgentSessionOwner, - renewAgentSessionLease, setAgentSessionJournalCheckpoint, type AgentSessionProcessIdentityCommit } from './agent-session-lease-transitions' +import { + renewAgentSessionLeases, + type AgentSessionLeaseRenewal +} from './agent-session-lease-renewal' import { applyAgentSessionRestartAdjudication } from './agent-session-restart-lease-transitions' import { agentSessionReconciliationTargetMatches } from './agent-session-reconciliation-target' import { replaceAgentSessionRecordOptions } from './agent-session-record-options' @@ -198,21 +201,14 @@ export class AgentSessionRecordStore { }) } - async renewLease(args: { - sessionId: string - fence: number - childProbe: AgentSessionOwnerProbe - now: number - leaseTtlMs?: number - }): Promise { - return this.mutate(args.sessionId, (record) => - renewAgentSessionLease({ - record, - fence: args.fence, - childProbe: args.childProbe, - now: args.now, - leaseTtlMs: args.leaseTtlMs ?? AGENT_SESSION_LEASE_TTL_MS - }) + async renewLease(args: AgentSessionLeaseRenewal): Promise { + const [renewed] = await this.renewLeases([args]) + return renewed + } + + async renewLeases(renewals: readonly AgentSessionLeaseRenewal[]): Promise { + return this.transact(() => + renewAgentSessionLeases(this.state, renewals, AGENT_SESSION_LEASE_TTL_MS) ) } diff --git a/src/main/runtime/agent-session-recovery-publish-fault.test.ts b/src/main/runtime/agent-session-recovery-publish-fault.test.ts new file mode 100644 index 000000000000..10785bbad290 --- /dev/null +++ b/src/main/runtime/agent-session-recovery-publish-fault.test.ts @@ -0,0 +1,83 @@ +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as DurableFileWrite from '../durable-file-write' +import { + agentSessionStoreRevision, + loadAgentSessionStore, + saveAgentSessionStore, + type AgentSessionStoreState +} from './agent-session-record-store-file' +import { AgentSessionStoreTransactionQueue } from './agent-session-store-transaction-queue' + +const publishFault = vi.hoisted(() => ({ armed: false })) + +vi.mock('../durable-file-write', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + renameDurable: async (tmpPath: string, finalPath: string) => { + if (publishFault.armed) { + publishFault.armed = false + throw new Error('simulated death before primary publish') + } + return actual.renameDurable(tmpPath, finalPath) + } + } +}) + +let root: string +let storePath: string + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-store-recovery-publish-')) + storePath = join(root, 'agent-sessions.json') +}) + +afterEach(async () => { + publishFault.armed = false + await rm(root, { recursive: true, force: true }) +}) + +function state(generation: number): AgentSessionStoreState { + return { + schemaVersion: 2, + hostId: 'local', + records: new Map(), + operations: new Map(), + retiredClaimKeys: [{ keyId: `generation-${generation}`, retiredAt: generation }], + unreadableRecords: new Map() + } +} + +describe('backup recovery publication', () => { + it('keeps the valid backup when publication fails after a corrupt primary was recovered', async () => { + await saveAgentSessionStore(storePath, state(1), { primaryStatus: 'unusable-or-absent' }) + await saveAgentSessionStore(storePath, state(2), { + primaryStatus: 'validated' + }) + await writeFile(storePath, '{corrupt-primary', 'utf-8') + const knownGoodBackup = await readFile(`${storePath}.bak`, 'utf-8') + + const recovered = await loadAgentSessionStore(storePath, 'local') + expect(recovered.recoveredFromBackup).toBe(true) + expect(recovered.state.retiredClaimKeys[0]?.keyId).toBe('generation-1') + const queue = AgentSessionStoreTransactionQueue.fromLoadedStore( + storePath, + 'local', + recovered, + agentSessionStoreRevision(recovered.state) + ) + + publishFault.armed = true + await expect(queue.persistLoadedMigration()).rejects.toThrow( + 'simulated death before primary publish' + ) + + expect(await readFile(`${storePath}.bak`, 'utf-8')).toBe(knownGoodBackup) + const afterFault = await loadAgentSessionStore(storePath, 'local') + expect(afterFault.recoveredFromBackup).toBe(true) + expect(afterFault.state.retiredClaimKeys[0]?.keyId).toBe('generation-1') + }) +}) diff --git a/src/main/runtime/agent-session-store-transaction-queue.ts b/src/main/runtime/agent-session-store-transaction-queue.ts index 0da8dae355e5..77e2fe5058ab 100644 --- a/src/main/runtime/agent-session-store-transaction-queue.ts +++ b/src/main/runtime/agent-session-store-transaction-queue.ts @@ -116,7 +116,9 @@ export class AgentSessionStoreTransactionQueue { ) { return result } - await saveAgentSessionStore(this.filePath, this.state) + await saveAgentSessionStore(this.filePath, this.state, { + primaryStatus: this.diskStoreFound && !recovering ? 'validated' : 'unusable-or-absent' + }) this.state.schemaVersion = AGENT_SESSION_STORE_SCHEMA_VERSION this.diskRevision = agentSessionStoreRevision(this.state) this.diskRecoveredFromBackup = false diff --git a/src/main/runtime/agent-session-surface-release-transition.ts b/src/main/runtime/agent-session-surface-release-transition.ts new file mode 100644 index 000000000000..d4da43f1933e --- /dev/null +++ b/src/main/runtime/agent-session-surface-release-transition.ts @@ -0,0 +1,60 @@ +// Releasing the lease when the LAST surface lets go of a session. +// +// Every other release in the wire needs a probe, because every other release is about a process +// somebody else started and nobody watched die. This one is different: the host stopped its own +// child through the adapter and the adapter proved the exit before this runs, so the evidence is +// `exit-observed` rather than an adjudicated absence. +// +// The fence still moves. A released lease at the old fence would let a mutation a client queued +// against the dead generation land on the next one. + +import type { AgentSessionRecord } from '../../shared/agent-session-record' +import { assertFence, withLease } from './agent-session-lease-transitions' +import type { AgentSessionRecordStore } from './agent-session-record-store' + +/** Whether this record is one THIS host may release on its own proof. A TUI owner, a session + * mid-handoff, and a lease nobody holds are all somebody else's transition. */ +export function isSurfaceReleasableAgentSessionRecord(record: AgentSessionRecord): boolean { + return ( + record.lease.runtimeKind === 'native' && + record.lease.claimStatus === 'live' && + record.lease.handoffStage === null && + record.lease.ownerProcess !== null + ) +} + +export function releaseAgentSessionOwnerAfterSurfaceClose(args: { + record: AgentSessionRecord + expectedFence: number + now: number +}): AgentSessionRecord { + const { record } = args + assertFence(record.lease, args.expectedFence) + if (!isSurfaceReleasableAgentSessionRecord(record)) { + throw new Error('agent_session_ownership_unknown') + } + return withLease(record, { + ...record.lease, + runtimeFence: record.lease.runtimeFence + 1, + ownerProcess: null, + reservedSpawnToken: null, + processlessAt: null, + claimStatus: 'released', + lastRenewedAt: args.now, + deathEvidence: { + kind: 'exit-observed', + detail: 'the last surface holding this session released it', + observedAt: args.now + } + }) +} + +/** Applied through the store's generic transition, the same way handoff records move. */ +export function releaseStoredAgentSessionOwnerAfterSurfaceClose( + store: AgentSessionRecordStore, + args: { sessionId: string; expectedFence: number; now: number } +): Promise { + return store.transitionHandoff(args.sessionId, (record) => + releaseAgentSessionOwnerAfterSurfaceClose({ ...args, record }) + ) +} diff --git a/src/main/runtime/orca-runtime-structured-session-restore.test.ts b/src/main/runtime/orca-runtime-structured-session-restore.test.ts index a5b2d7b68879..52db8dcf0df5 100644 --- a/src/main/runtime/orca-runtime-structured-session-restore.test.ts +++ b/src/main/runtime/orca-runtime-structured-session-restore.test.ts @@ -55,6 +55,8 @@ describe('structured session cold restoration', () => { it('normalizes a restored tab id and removes it when closed', async () => { const runtime = new OrcaRuntimeService() + const closeSessionTab = vi.fn(async () => undefined) + runtime.setNotifier({ closeSessionTab } as never) const internal = runtime as unknown as { getKnownWorkspaceSessionWorktreeIds(): Set hydrateHeadlessMobileSessionTabsFromWorkspaceSession(): Set @@ -146,6 +148,11 @@ describe('structured session cold restoration', () => { reason: 'user' }) + expect(closeSessionTab).toHaveBeenCalledWith( + 'structured-agent-session-restored-session', + 'workspace-1' + ) + const closed = await runtime.listMobileSessionTabs('id:workspace-1') expect(closed.tabs.map((tab) => tab.id)).toEqual([ 'terminal-tab::leaf-1', @@ -153,4 +160,26 @@ describe('structured session cold restoration', () => { ]) expect(closed.tabGroups?.[0]?.tabOrder).toEqual(['terminal-tab']) }) + + it('commits the host close when the renderer already removed the structured tab', async () => { + const runtime = new OrcaRuntimeService() + runtime.setNotifier({ + closeSessionTab: vi.fn(async () => { + throw new Error('session_tab_not_found') + }) + } as never) + runtime.publishStructuredAgentSessionTab({ + workspaceId: 'workspace-1', + sessionId: 'session-1', + agent: 'codex', + activate: true + }) + + await runtime.closeMobileSessionTab('id:workspace-1', 'agent-session:session-1', { + reason: 'user' + }) + + const snapshot = await runtime.listMobileSessionTabs('id:workspace-1') + expect(snapshot.tabs).toEqual([]) + }) }) diff --git a/src/main/runtime/orca-runtime-structured-tui-adoption-write-gate.test.ts b/src/main/runtime/orca-runtime-structured-tui-adoption-write-gate.test.ts index fab6a74542dc..c14e57b6e4b7 100644 --- a/src/main/runtime/orca-runtime-structured-tui-adoption-write-gate.test.ts +++ b/src/main/runtime/orca-runtime-structured-tui-adoption-write-gate.test.ts @@ -76,7 +76,10 @@ function installProofPty( ptysById: Map } const pty = record.ptysById.get(PTY_ID)! - pty.tailBuffer = [`Session ID: ${threadId}`] + pty.tailBuffer = [ + '\u001b[2m│ >_ OpenAI Codex (v0.148.0) │', + `│ Session: \u001b[22m${threadId}\u001b[2m │` + ] pty.lastOutputAt = ptyRecord.lastOutputAt + 1_000 } return true @@ -237,6 +240,21 @@ describe('structured Codex adoption against the real PTY write gate', () => { }) }, 20_000) + it('adopts a blank Codex 0.148 session before the lazy rollout exists', async () => { + await rm(join(rig.codexHome, 'sessions'), { recursive: true, force: true }) + + const result = await rig.runtime.adoptStructuredAgentSessionTerminal(adoptInput(), { + callerKey: 'renderer-1' + }) + + expect(result).toMatchObject({ ok: true }) + expect(rig.refusedWrites).toEqual([]) + expect(rig.store.getRecord(SESSION_ID)).toMatchObject({ + providerHandleChain: [{ origin: 'adopted', handle: { threadId: THREAD_ID } }], + lease: { runtimeKind: 'tui', claimStatus: 'live' } + }) + }, 20_000) + it('leaves nothing latched when the proof fails, so the next attempt still succeeds', async () => { await rm(join(rig.codexHome, 'sessions'), { recursive: true, force: true }) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 1a4e291d1600..4619d890fc94 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -94,6 +94,7 @@ import { type StructuredTuiOwner } from '../native-chat/agent-session-wire/structured-agent-session-handoff-types' import type { AgentSessionRecord } from '../../shared/agent-session-record' +import { SESSION_TAB_NOT_FOUND_ERROR } from '../../shared/session-tab-close' import { agentSessionOwnerBindingsEqual, cloneAgentSessionOwnerBinding, @@ -9471,7 +9472,19 @@ export class OrcaRuntimeService { // snapshot so paired clients stop showing it. await this.closeHeadlessMobileBrowserTab(worktreeId, snapshot, tab) } else if (tab.type === 'agent-session') { - this.notifier?.closeSessionTab?.(tab.id, worktreeId) + if (this.notifier?.closeSessionTab) { + try { + await this.notifier.closeSessionTab( + structuredAgentSessionTabId(tab.sessionId), + worktreeId + ) + } catch (error) { + // The renderer already having removed the tab is an idempotent close, not a veto. + if (!(error instanceof Error && error.message === SESSION_TAB_NOT_FOUND_ERROR)) { + throw error + } + } + } this.closeStructuredAgentSessionTab(worktreeId, snapshot, tab) } else { if (!this.notifier?.closeSessionTab) { @@ -11616,7 +11629,7 @@ export class OrcaRuntimeService { spawnToken }) ?? false } - let transcriptPath: string + let transcriptPath: string | undefined if (threadId) { const proof = await proveCodexTuiRollout({ ...proofInput, threadId }) transcriptPath = proof.transcriptPath @@ -11651,7 +11664,7 @@ export class OrcaRuntimeService { fence, observedAt: Date.now() }), - transcriptPath, + ...(transcriptPath ? { transcriptPath } : {}), historySource: 'provider-resume', adoptedTerminal: true } diff --git a/src/main/runtime/orchestration-structured-chat-lease.test.ts b/src/main/runtime/orchestration-structured-chat-lease.test.ts new file mode 100644 index 000000000000..c7e407b317f8 --- /dev/null +++ b/src/main/runtime/orchestration-structured-chat-lease.test.ts @@ -0,0 +1,372 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import { ORCHESTRATION_CONTRACT_VERSION } from '../../shared/protocol-version' +import type { AgentSessionExecutionLocation } from '../../shared/agent-session-record' +import { getDefaultWorkspaceSession } from '../../shared/constants' +import { agentSessionPtyWriteGate } from './agent-session-pty-write-gate' +import { AgentSessionRecordStore } from './agent-session-record-store' +import { OrcaRuntimeService } from './orca-runtime' +import { OrchestrationDb } from './orchestration/db' +import { RpcDispatcher } from './rpc/dispatcher' +import { ORCHESTRATION_METHODS } from './rpc/methods/orchestration' +import { TERMINAL_METHODS } from './rpc/methods/terminal' + +const WORKTREE_ID = 'repo-structured-chat::/tmp/structured-chat' +const SESSION_ID = 'session-structured-chat' +const COORDINATOR = { + handle: 'term_structured_coord', + tabId: '11111111-1111-4111-8111-111111111111', + leafId: 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa', + ptyId: 'pty-structured-coord' +} +const WORKER = { + handle: 'term_structured_worker', + tabId: '22222222-2222-4222-8222-222222222222', + leafId: 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb', + ptyId: 'pty-structured-worker' +} +const PLAIN = { + handle: 'term-plain', + tabId: '33333333-3333-4333-8333-333333333333', + leafId: 'cccccccc-cccc-4ccc-8ccc-cccccccccccc', + ptyId: 'pty-plain' +} +const LOCATION: AgentSessionExecutionLocation = { + executionHostId: 'local', + wslDistro: null, + workspaceId: WORKTREE_ID, + workspaceKind: 'git-worktree' +} + +type TestTerminal = typeof COORDINATOR + +function paneKey(terminal: TestTerminal): string { + return `${terminal.tabId}:${terminal.leafId}` +} + +function makeStore() { + const session = getDefaultWorkspaceSession() + const repo = { + id: 'repo-structured-chat', + path: '/tmp/structured-chat', + displayName: 'structured-chat', + badgeColor: '#000000', + addedAt: 0 + } + return { + getWorkspaceSession: vi.fn(() => session), + setWorkspaceSession: vi.fn(), + getRepos: vi.fn(() => [repo]), + getRepo: vi.fn(() => repo), + getAllWorktreeMeta: vi.fn(() => ({})), + getWorktreeMeta: vi.fn(() => undefined), + setWorktreeMeta: vi.fn(), + removeWorktreeMeta: vi.fn(), + getSettings: vi.fn(() => ({ workspaceDir: '/tmp/workspaces' })), + getProjects: vi.fn(() => []) + } +} + +describe('orchestration while Structured Chat owns an agent session', () => { + let directory: string + let recordStore: AgentSessionRecordStore + let db: OrchestrationDb + let runtime: OrcaRuntimeService + let dispatcher: RpcDispatcher + let writes: Mock<(ptyId: string, data: string) => void> + let operationSequence: number + + beforeEach(async () => { + directory = await mkdtemp(join(tmpdir(), 'orca-orchestration-structured-chat-')) + recordStore = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + db = new OrchestrationDb(':memory:') + runtime = new OrcaRuntimeService(makeStore() as never) + runtime.setOrchestrationDb(db) + vi.spyOn(runtime, 'showManagedTerminalWorkspace').mockResolvedValue({ + id: WORKTREE_ID, + repoId: 'repo-structured-chat' + } as never) + writes = vi.fn<(ptyId: string, data: string) => void>() + runtime.setPtyController({ + spawn: vi.fn(async () => ({ id: 'unused' })), + write: (ptyId: string, data: string) => { + agentSessionPtyWriteGate.assertAdmitted(ptyId) + writes(ptyId, data) + return true + }, + kill: vi.fn(() => true), + getForegroundProcess: vi.fn(async () => 'codex'), + listProcesses: vi.fn(async () => []), + hasPty: vi.fn(() => true) + } as never) + for (const terminal of [COORDINATOR, WORKER, PLAIN]) { + runtime.registerPty(terminal.ptyId, WORKTREE_ID, null, { + tabId: terminal.tabId, + leafId: terminal.leafId, + incarnationId: `${terminal.ptyId}-incarnation`, + agentLaunchAuthority: { launchToken: `${terminal.ptyId}-launch`, launchAgent: 'codex' } + }) + runtime.registerPreAllocatedHandleForPty(terminal.ptyId, terminal.handle) + } + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { + tabs: [COORDINATOR, WORKER, PLAIN].map((terminal) => ({ + tabId: terminal.tabId, + worktreeId: WORKTREE_ID, + title: 'Codex', + activeLeafId: terminal.leafId, + layout: null + })), + leaves: [COORDINATOR, WORKER, PLAIN].map((terminal, index) => ({ + tabId: terminal.tabId, + worktreeId: WORKTREE_ID, + leafId: terminal.leafId, + paneRuntimeId: index + 1, + ptyId: terminal.ptyId, + paneTitle: null, + title: 'Codex' + })) + }) + await runtime.listTerminals() + for (const terminal of [COORDINATOR, WORKER, PLAIN]) { + runtime.onPtyData(terminal.ptyId, '\x1b]0;Codex working\x07', 1) + runtime.onPtyData(terminal.ptyId, '\x1b]0;Codex done\x07', 2) + } + operationSequence = 0 + await establishOwner('native', 'spawn-native', null) + agentSessionPtyWriteGate.attachRecordLookup((sessionId) => recordStore.getRecord(sessionId)) + agentSessionPtyWriteGate.bindPty(WORKER.ptyId, SESSION_ID) + dispatcher = new RpcDispatcher({ + runtime, + methods: [...ORCHESTRATION_METHODS, ...TERMINAL_METHODS] + }) + }) + + afterEach(async () => { + vi.useRealTimers() + agentSessionPtyWriteGate.detachRecordLookup() + db.close() + await rm(directory, { recursive: true, force: true }) + }) + + function operation() { + operationSequence += 1 + return { + callerKey: 'structured-chat-test', + operationId: `1800000000000-${operationSequence.toString(16).padStart(32, '0')}`, + fingerprint: `structured-chat-${operationSequence}` + } + } + + async function establishOwner( + runtimeKind: 'native' | 'tui', + spawnToken: string, + expectedFence: number | null + ): Promise { + const now = 1_800_000_000_000 + operationSequence + const reserved = await recordStore.reserveOwner({ + sessionId: SESSION_ID, + location: LOCATION, + provider: 'codex', + accountHome: { variable: 'CODEX_HOME', path: '/tmp/codex-home' }, + runtimeKind, + expectedFence, + spawnToken, + claimKeyId: 'key-1', + handoffOperationId: null, + probe: + expectedFence === null + ? { outcome: 'indeterminate', reason: 'new session' } + : { outcome: 'pid-absent' }, + operation: operation(), + now + }) + const fence = reserved.record.lease.runtimeFence + await recordStore.commitProcessIdentity({ + sessionId: SESSION_ID, + fence, + process: { hostId: 'local', pid: 4242 + fence, processStartTimeMs: now, spawnToken }, + now + }) + await recordStore.proveOwner({ + sessionId: SESSION_ID, + fence, + link: { + linkId: `link-${fence}`, + handle: { provider: 'codex', threadId: 'thread-1' }, + origin: fence === 1 ? 'created' : 'resumed', + mintedAtFence: fence, + observedAt: now + }, + now + }) + } + + function createRun(coordinator = COORDINATOR) { + return db.createRun({ + objective: 'Structured Chat lease coverage', + coordinatorHandle: coordinator.handle, + coordinatorPaneKey: paneKey(coordinator) + }) + } + + function queueRunMessage(runId: string) { + return db.insertMessage({ + from: 'term_sender', + to: `run:${runId}`, + subject: 'Queued guidance', + type: 'status', + runId + }) + } + + async function rpc(method: string, params: Record, capability?: string) { + return dispatcher.dispatch({ + id: `request-${method}-${Math.random()}`, + authToken: 'test-token', + method, + params, + orchestrationContractVersion: ORCHESTRATION_CONTRACT_VERSION, + orchestrationRequestId: `mutation-${method}-${Math.random()}`, + orchestrationCapability: capability + }) + } + + it('retains a Run mailbox pointer while Structured Chat refuses the PTY write', () => { + const run = createRun(WORKER) + const message = queueRunMessage(run.id) + + runtime.deliverPendingMessagesForHandle(`run:${run.id}`) + + expect(db.getMessageById(message.id)?.delivered_at).toBeNull() + expect(writes.mock.calls.filter(([ptyId]) => ptyId === WORKER.ptyId)).toHaveLength(0) + }) + + it('redrives the same retained pointer after the lease returns to TUI', async () => { + vi.useFakeTimers() + const run = createRun(WORKER) + const message = queueRunMessage(run.id) + runtime.deliverPendingMessagesForHandle(`run:${run.id}`) + expect(db.getMessageById(message.id)?.delivered_at).toBeNull() + + await establishOwner('tui', 'spawn-tui', 1) + runtime.deliverPendingMessagesForHandle(`run:${run.id}`) + await vi.advanceTimersByTimeAsync(500) + + expect(db.getMessageById(message.id)?.delivered_at).not.toBeNull() + expect(writes).toHaveBeenCalledTimes(2) + expect(writes.mock.calls[0]?.[1]).toContain('orca orchestration check') + expect(writes.mock.calls[1]).toEqual([WORKER.ptyId, '\r']) + }) + + it('fails worker-start truthfully when its reused terminal is in Structured Chat', async () => { + const run = createRun() + const task = db.createTask({ spec: 'Run the queued work', runId: run.id }) + + const response = await rpc('orchestration.workerStart', { + task: task.id, + worktree: 'current', + terminal: WORKER.handle, + from: COORDINATOR.handle + }) + + if (!response.ok) { + throw new Error(response.error.message) + } + expect(response.ok).toBe(true) + expect(response.result).toMatchObject({ + state: 'failed', + failedStage: 'dispatch_input', + lastError: expect.stringMatching(/Structured Chat.*Switch it to Terminal/), + agentSessionRefusal: { code: 'agent_session_conflict', ownerRuntimeKind: 'native' } + }) + expect(db.getTask(task.id)?.status).toBe('failed') + expect(db.getDispatchContext(task.id)?.status).toBe('failed') + expect(writes.mock.calls.filter(([ptyId]) => ptyId === WORKER.ptyId)).toHaveLength(0) + }) + + it('reports a real gate refusal with typed metadata and zero bytes written', async () => { + const response = await rpc('terminal.send', { + terminal: WORKER.handle, + text: 'new prompt', + enter: true, + agentPrompt: true, + client: { id: 'test-client', type: 'desktop' } + }) + + expect(response.ok).toBe(true) + if (!response.ok) { + throw new Error(response.error.message) + } + expect(response.result).toEqual({ + send: { + handle: WORKER.handle, + accepted: false, + bytesWritten: 0, + agentSessionRefusal: expect.objectContaining({ + code: 'agent_session_conflict', + ownerRuntimeKind: 'native' + }) + } + }) + expect(writes).not.toHaveBeenCalled() + }) + + it('settles worker_done while its pane remains in Structured Chat', async () => { + const run = createRun() + const task = db.createTask({ spec: 'Finish from Structured Chat', runId: run.id }) + const dispatch = db.createDispatchContext( + task.id, + WORKER.handle, + paneKey(WORKER), + undefined, + runtime.getTerminalProcessIncarnation(WORKER.handle) ?? undefined + ) + const capability = db.mintDispatchCapability({ + dispatchId: dispatch.id, + paneKey: paneKey(WORKER), + processIncarnation: runtime.getTerminalProcessIncarnation(WORKER.handle)! + }) + + const response = await rpc( + 'orchestration.send', + { + from: WORKER.handle, + subject: 'Done', + body: 'Implemented the task. Verified the result. Nothing remains.', + type: 'worker_done', + payload: JSON.stringify({ + taskId: task.id, + dispatchId: dispatch.id, + outcome: 'succeeded' + }) + }, + capability + ) + + expect(response.ok).toBe(true) + expect(db.getTask(task.id)?.status).toBe('completed') + expect(db.getDispatchContextById(dispatch.id)?.status).toBe('completed') + expect(writes.mock.calls.filter(([ptyId]) => ptyId === WORKER.ptyId)).toHaveLength(0) + }) + + it('delivers normally to a never-adopted terminal', async () => { + vi.useFakeTimers() + const run = createRun(PLAIN) + const message = queueRunMessage(run.id) + + expect(agentSessionPtyWriteGate.admit(PLAIN.ptyId)).toEqual({ + admitted: true, + sessionId: null, + runtimeFence: null + }) + runtime.deliverPendingMessagesForHandle(`run:${run.id}`) + await vi.advanceTimersByTimeAsync(500) + + expect(db.getMessageById(message.id)?.delivered_at).not.toBeNull() + expect(writes).toHaveBeenCalledTimes(2) + expect(writes.mock.calls[1]).toEqual([PLAIN.ptyId, '\r']) + }) +}) diff --git a/src/main/runtime/rpc/methods/orchestration-worker-start-receipt.ts b/src/main/runtime/rpc/methods/orchestration-worker-start-receipt.ts index e58f9fff01ec..494cf1faccd1 100644 --- a/src/main/runtime/rpc/methods/orchestration-worker-start-receipt.ts +++ b/src/main/runtime/rpc/methods/orchestration-worker-start-receipt.ts @@ -4,6 +4,8 @@ import { type WorkerSetupReceipt } from './orchestration-worker-topology' import type { OrchestrationWorkerLaunchReceipt } from './orchestration-worker-launch-preferences' +import { isAgentSessionPtyWriteRefusedError } from '../../../../shared/agent-session-pty-write-admission' +import { structuredChatPtyWriteRefusalCopy } from '../../../../shared/agent-session-pty-write-refusal-copy' export function failWorkerStartWithReceipt(args: { db: OrchestrationDb @@ -15,7 +17,13 @@ export function failWorkerStartWithReceipt(args: { setup: WorkerSetupReceipt launch: OrchestrationWorkerLaunchReceipt }): unknown { - const reason = args.error instanceof Error ? args.error.message : String(args.error) + const agentSessionRefusal = isAgentSessionPtyWriteRefusedError(args.error) + ? args.error.refusal + : undefined + const reason = + (agentSessionRefusal && + structuredChatPtyWriteRefusalCopy(agentSessionRefusal, 'worker-start')) ?? + (args.error instanceof Error ? args.error.message : String(args.error)) const unknown = isUnknownWorkerStartOutcome(args.error, args.failedStage) const worker = unknown ? args.db.markWorkerStartUnknown(args.dispatchId, args.failedStage, reason) @@ -32,6 +40,7 @@ export function failWorkerStartWithReceipt(args: { launch: args.launch, effects: JSON.parse(worker.effects) as unknown[], residualResources: JSON.parse(worker.residual_resources) as unknown[], + ...(agentSessionRefusal ? { agentSessionRefusal } : {}), ...(unknown ? { nextCommands: [ diff --git a/src/main/runtime/rpc/methods/structured-agent-session-gate.ts b/src/main/runtime/rpc/methods/structured-agent-session-gate.ts new file mode 100644 index 000000000000..2dd317e08b0f --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-gate.ts @@ -0,0 +1,58 @@ +// Who may see `agentSession.*` at all. +// +// Shared by every structured method file so one gate governs the whole surface: a client that does +// not advertise `agent-session.structured.v1` is told the surface does not exist rather than being +// handed a session it cannot render or drive — and, just as importantly, cannot make the host EXIST +// by calling into it, which is an observable side effect. + +import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import { getStructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-registry' +import type { StructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-host' +import type { StructuredAgentSessionCaller } from '../../../native-chat/agent-session-wire/structured-agent-session-host-types' +import type { RpcContext } from '../core' + +/** + * In-process callers are the same build as the host, so they carry no negotiated + * capability list; every remote client must say it can read structured sessions. + */ +export function supportsStructuredSessions(ctx: RpcContext): boolean { + return ( + ctx.clientKind === undefined || + (ctx.clientCapabilities?.includes(STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY) ?? false) + ) +} + +export function requireStructuredCapability(ctx: RpcContext): void { + if (!supportsStructuredSessions(ctx)) { + throw new Error('structured_agent_session_unsupported') + } +} + +export function requireStructuredHost(ctx: RpcContext): StructuredAgentSessionHost { + requireStructuredCapability(ctx) + const host = getStructuredAgentSessionHost() + if (!host) { + throw new Error('structured_agent_session_unsupported') + } + return host +} + +/** Attach is the only way a session comes into being, so it is the only call + * that builds the host. Every other method addresses a session that must + * already be attached, and correctly reports absent when none is. */ +export async function ensureStructuredHostInstalled(ctx: RpcContext): Promise { + // Gated first: a client that cannot read structured sessions must not be able + // to make the host exist, which is an observable side effect of the surface. + if (!supportsStructuredSessions(ctx) || getStructuredAgentSessionHost()) { + return + } + await ctx.runtime.ensureStructuredAgentSessionHost() +} + +/** Mirrors the existing agent-session host-authority derivation so one client + * gets one operation namespace across both surfaces. */ +export function structuredCallerFor(ctx: RpcContext): StructuredAgentSessionCaller { + return { + callerKey: ctx.clientId?.trim() || `trusted-local:${ctx.clientKind ?? 'runtime'}` + } +} diff --git a/src/main/runtime/rpc/methods/structured-agent-session-hold.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-hold.test.ts new file mode 100644 index 000000000000..7a93aa66a6fa --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-hold.test.ts @@ -0,0 +1,262 @@ +// The wire half of a session's lifetime: who takes a hold, and what happens when they vanish. +// +// Run against the REAL subscription registry rather than a stub, because the backstop being tested +// IS that registry's connection sweep — a stubbed `registerSubscriptionCleanup` would prove that +// the handler called a function, which is not the claim. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import type { StructuredAgentSessionAdapter } from '../../../native-chat/agent-session-wire/structured-agent-session-adapter' +import { StructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-host' +import { + HOST_TEST_NOW as NOW, + HOST_TEST_SESSION as SESSION, + HOST_TEST_THREAD as THREAD, + hostTestAttachParams, + resetHostTestOperationIds +} from '../../../native-chat/agent-session-wire/structured-agent-session-host-test-data' +import { setStructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-registry' +import { + STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY, + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY +} from '../../../../shared/protocol-version' +import { AgentSessionRecordStore } from '../../agent-session-record-store' +import { OrcaRuntimeService } from '../../orca-runtime' +import type { RpcResponse } from '../core' +import { RpcDispatcher } from '../dispatcher' +import { STRUCTURED_AGENT_SESSION_METHODS } from './structured-agent-session' + +const CONNECTION = 'connection-1' +const GRACE_MS = 5 +const CLIENT = { + clientId: 'device-1', + clientKind: 'mobile' as const, + clientCapabilities: [ + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, + STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY + ], + connectionId: CONNECTION +} + +let root: string +let store: AgentSessionRecordStore +let host: StructuredAgentSessionHost +let runtime: OrcaRuntimeService +let dispatcher: RpcDispatcher +let closeSession: Mock> +let requests = 0 + +async function call(method: string, params: unknown): Promise { + const replies: RpcResponse[] = [] + requests += 1 + await dispatcher.dispatchStreaming( + { id: `request-${requests}`, authToken: 'token', method, params }, + (raw) => replies.push(JSON.parse(raw) as RpcResponse), + CLIENT + ) + return replies[0] as RpcResponse +} + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-hold-wire-')) + resetHostTestOperationIds() + requests = 0 + closeSession = vi.fn(async () => undefined) + store = await AgentSessionRecordStore.open({ directory: join(root, 'store'), hostId: 'local' }) + host = new StructuredAgentSessionHost({ + store, + adapter: { + acquire: async ({ fence, spawnToken }) => ({ + process: { hostId: 'local', pid: 4242, processStartTimeMs: 1_700_000_000_000, spawnToken }, + link: { + linkId: `link-${fence}`, + handle: { provider: 'codex', threadId: THREAD }, + origin: store.getRecord(SESSION)?.providerHandleChain.length ? 'resumed' : 'created', + mintedAtFence: fence, + observedAt: NOW + } + }), + closeSession, + dispatch: async () => ({ state: 'rejected', reason: 'unused' }), + cancelTurn: async () => ({ cancelled: false }), + answerPrompt: async () => undefined, + setOption: async () => undefined + }, + journalRoot: root, + claimKeyId: 'key-1', + mintSpawnToken: () => 'spawn-a', + releaseGraceMs: GRACE_MS, + now: () => NOW + }) + setStructuredAgentSessionHost(host) + runtime = new OrcaRuntimeService() + dispatcher = new RpcDispatcher({ runtime, methods: STRUCTURED_AGENT_SESSION_METHODS }) + expect(await host.attach({ callerKey: 'client-1' }, hostTestAttachParams(null))).toMatchObject({ + ok: true + }) +}) + +afterEach(async () => { + setStructuredAgentSessionHost(null) + await host.flushAllStreamedEvents() + await rm(root, { recursive: true, force: true }) +}) + +describe('a client that holds a session', () => { + it('keeps the provider child while the hold stands', async () => { + expect( + await call('agentSession.hold', { sessionId: SESSION, holderId: 'chat-1' }) + ).toMatchObject({ ok: true }) + + await new Promise((resolve) => setTimeout(resolve, GRACE_MS * 20)) + + expect(closeSession).not.toHaveBeenCalled() + expect(host.hasSession(SESSION)).toBe(true) + }) + + it('releases it when the client says so', async () => { + await call('agentSession.hold', { sessionId: SESSION, holderId: 'chat-1' }) + + expect( + await call('agentSession.release', { sessionId: SESSION, holderId: 'chat-1' }) + ).toMatchObject({ ok: true }) + + await vi.waitFor(() => expect(host.hasSession(SESSION)).toBe(false)) + expect(closeSession).toHaveBeenCalledWith(SESSION) + }) + + it('does not report success when no provider child can be acquired', async () => { + const response = await call('agentSession.hold', { + sessionId: 'session-missing', + holderId: 'chat-missing' + }) + + expect(response).toMatchObject({ + ok: false, + error: { code: 'agent_session_identity_required' } + }) + expect(host.isHeld('session-missing')).toBe(false) + }) +}) + +// A phone that loses signal mid-conversation never sends a release. Before this, the app-server it +// was talking to lived until the desktop quit. +describe('a client that disappears without cleanup', () => { + it('still releases the session when its transport closes', async () => { + await call('agentSession.hold', { sessionId: SESSION, holderId: 'chat-1' }) + + runtime.cleanupSubscriptionsForConnection(CONNECTION) + + await vi.waitFor(() => expect(host.hasSession(SESSION)).toBe(false)) + expect(closeSession).toHaveBeenCalledWith(SESSION) + }) + + it('does not release a hold another connection is still holding', async () => { + await call('agentSession.hold', { sessionId: SESSION, holderId: 'chat-1' }) + await dispatcher.dispatchStreaming( + { + id: 'request-other', + authToken: 'token', + method: 'agentSession.hold', + params: { sessionId: SESSION, holderId: 'chat-1' } + }, + () => {}, + { ...CLIENT, clientId: 'device-2', connectionId: 'connection-2' } + ) + + runtime.cleanupSubscriptionsForConnection(CONNECTION) + await new Promise((resolve) => setTimeout(resolve, GRACE_MS * 20)) + + expect(closeSession).not.toHaveBeenCalled() + expect(host.hasSession(SESSION)).toBe(true) + }) + + it('releases a desktop subscription when its renderer transport dies', async () => { + const transport = new AbortController() + await dispatcher.dispatchStreaming( + { + id: 'desktop-subscription', + authToken: 'token', + method: 'agentSession.subscribe', + params: { sessionId: SESSION } + }, + () => {}, + { + signal: transport.signal, + clientId: 'desktop-renderer', + clientKind: 'runtime', + clientCapabilities: CLIENT.clientCapabilities + } + ) + expect(host.isHeld(SESSION)).toBe(true) + + transport.abort() + + await vi.waitFor(() => expect(host.hasSession(SESSION)).toBe(false)) + expect(closeSession).toHaveBeenCalledWith(SESSION) + }) +}) + +// Paired builds ship on their own schedule, and the ones already in the field never learned to +// hold. Subscribing is the only thing they do to say a chat is open, and the host no longer resumes +// every record at launch on their behalf. +describe('a paired client that predates holds', () => { + /** Leaves the session where a restart leaves one: readable, with no provider child. */ + async function releaseAndRestoreForReading(): Promise { + await host.close(SESSION) + expect(host.hasSession(SESSION)).toBe(false) + await host.restoreReadableSessions() + expect(store.getRecord(SESSION)?.lease.claimStatus).toBe('released') + } + + async function subscribe(capabilities: string[]): Promise { + await dispatcher.dispatchStreaming( + { + id: 'request-subscribe', + authToken: 'token', + method: 'agentSession.subscribe', + params: { sessionId: SESSION } + }, + () => {}, + { ...CLIENT, clientCapabilities: capabilities } + ) + } + + it('keeps its session alive for as long as it is subscribed', async () => { + await subscribe([STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY]) + + await new Promise((resolve) => setTimeout(resolve, GRACE_MS * 20)) + + expect(host.isHeld(SESSION)).toBe(true) + expect(closeSession).not.toHaveBeenCalled() + }) + + it('releases it when that subscription connection goes away', async () => { + await subscribe([STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY]) + + runtime.cleanupSubscriptionsForConnection(CONNECTION) + + await vi.waitFor(() => expect(host.hasSession(SESSION)).toBe(false)) + }) + + it('gets its provider child back by subscribing to a session nothing is running', async () => { + await releaseAndRestoreForReading() + + await subscribe([STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY]) + + await vi.waitFor(() => expect(store.getRecord(SESSION)?.lease.claimStatus).toBe('live')) + }) + + it('does not let a current client stream alone resume a released session', async () => { + await releaseAndRestoreForReading() + + await subscribe([ + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, + STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY + ]) + + expect(store.getRecord(SESSION)?.lease.claimStatus).toBe('released') + }) +}) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-hold.ts b/src/main/runtime/rpc/methods/structured-agent-session-hold.ts new file mode 100644 index 000000000000..5bfd54112a7f --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-hold.ts @@ -0,0 +1,64 @@ +// `agentSession.hold` / `agentSession.release` — a surface saying it is bound to a session. +// +// The holder identity is scoped to the CONNECTION, not taken from the client verbatim: two clients +// are free to name their surfaces the same thing, and a hold that collides is one that a stranger +// can release. +// +// The registered cleanup is the backstop, and the ONLY thing that covers a client which vanishes — +// a phone that loses signal mid-turn never gets to send its release. Registering it before taking +// the hold is deliberate: re-registering an id runs the previous cleanup synchronously, so the +// stale release lands before this hold rather than after it. + +import { defineMethod, type RpcAnyMethod, type RpcContext } from '../core' +import { + ensureStructuredHostInstalled, + requireStructuredHost +} from './structured-agent-session-gate' +import { HoldParams } from './structured-agent-session-schemas' + +const HOLD_CLEANUP_PREFIX = 'agentSession.hold' + +function holderKeyFor(ctx: RpcContext, holderId: string): string { + const client = ctx.clientId?.trim() || (ctx.clientKind ?? 'runtime') + return `${ctx.connectionId ?? 'local'}:${client}:${holderId}` +} + +function holdCleanupIdFor(sessionId: string, holderKey: string): string { + return `${HOLD_CLEANUP_PREFIX}:${holderKey}:${sessionId}` +} + +export const STRUCTURED_AGENT_SESSION_HOLD_METHODS: RpcAnyMethod[] = [ + defineMethod({ + name: 'agentSession.hold', + params: HoldParams, + handler: async (params, ctx) => { + await ensureStructuredHostInstalled(ctx) + const host = requireStructuredHost(ctx) + const holderKey = holderKeyFor(ctx, params.holderId) + ctx.runtime.registerSubscriptionCleanup( + holdCleanupIdFor(params.sessionId, holderKey), + () => host.release(params.sessionId, holderKey), + ctx.connectionId + ) + try { + await host.hold(params.sessionId, holderKey) + } catch (error) { + ctx.runtime.cleanupSubscription(holdCleanupIdFor(params.sessionId, holderKey)) + throw error + } + return { held: true as const } + } + }), + defineMethod({ + name: 'agentSession.release', + params: HoldParams, + handler: async (params, ctx) => { + const host = requireStructuredHost(ctx) + const holderKey = holderKeyFor(ctx, params.holderId) + host.release(params.sessionId, holderKey) + // Retires the backstop too; its release is a no-op against a holder already gone. + ctx.runtime.cleanupSubscription(holdCleanupIdFor(params.sessionId, holderKey)) + return { released: true as const } + } + }) +] diff --git a/src/main/runtime/rpc/methods/structured-agent-session-schemas.ts b/src/main/runtime/rpc/methods/structured-agent-session-schemas.ts index 41f42e8805a0..0010c52fcd15 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-schemas.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-schemas.ts @@ -183,6 +183,13 @@ export const SetOptionParams = z export const OptionsParams = z.object({ sessionId: SessionId }).strict() +/** One surface's claim on one session. The id names the surface, not the client: a chat tab and a + * paired phone looking at the same session are two holders, and either leaving must not release + * the other's. */ +export const HoldParams = z + .object({ sessionId: SessionId, holderId: Identifier('Invalid holder id') }) + .strict() + export const HistoryParams = z .object({ sessionId: SessionId, diff --git a/src/main/runtime/rpc/methods/structured-agent-session.test.ts b/src/main/runtime/rpc/methods/structured-agent-session.test.ts index 93c5c4d24802..b1fffa8d31c8 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session.test.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session.test.ts @@ -192,7 +192,9 @@ describe('capability gating', () => { for (const method of STRUCTURED_AGENT_SESSION_METHODS) { expect(names).toContain(method.name) } - expect(STRUCTURED_AGENT_SESSION_METHODS).toHaveLength(15) + // Bump deliberately: the whole agentSession.* surface is behind the structured capability, + // so an additive method is invisible to old clients and needs no protocol bump. + expect(STRUCTURED_AGENT_SESSION_METHODS).toHaveLength(18) }) it('hides the surface from a declared client that did not advertise it', async () => { diff --git a/src/main/runtime/rpc/methods/structured-agent-session.ts b/src/main/runtime/rpc/methods/structured-agent-session.ts index 3663e203dfa0..58d34f72341b 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session.ts @@ -6,15 +6,20 @@ // is the whole visibility rule, because nothing else on the runtime publishes a // structured session. -import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import { STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' import { agentSessionFingerprintConflict, computeAgentSessionPayloadFingerprint } from '../../../../shared/agent-session-mutation-envelope' -import { getStructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-registry' -import type { StructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-host' -import type { StructuredAgentSessionCaller } from '../../../native-chat/agent-session-wire/structured-agent-session-host-types' import { defineMethod, defineStreamingMethod, type RpcAnyMethod, type RpcContext } from '../core' +import { + ensureStructuredHostInstalled as ensureHostInstalled, + requireStructuredCapability, + requireStructuredHost as requireHost, + structuredCallerFor as callerFor, + supportsStructuredSessions +} from './structured-agent-session-gate' +import { STRUCTURED_AGENT_SESSION_HOLD_METHODS } from './structured-agent-session-hold' import { AttachParams, AdoptTerminalParams, @@ -35,32 +40,6 @@ import { hasMobileClipboardImagePath } from '../mobile-clipboard-image-provenanc const SUBSCRIPTION_PREFIX = 'agentSession' -/** - * In-process callers are the same build as the host, so they carry no negotiated - * capability list; every remote client must say it can read structured sessions. - */ -function supportsStructuredSessions(ctx: RpcContext): boolean { - return ( - ctx.clientKind === undefined || - (ctx.clientCapabilities?.includes(STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY) ?? false) - ) -} - -function requireStructuredCapability(ctx: RpcContext): void { - if (!supportsStructuredSessions(ctx)) { - throw new Error('structured_agent_session_unsupported') - } -} - -function requireHost(ctx: RpcContext): StructuredAgentSessionHost { - requireStructuredCapability(ctx) - const host = getStructuredAgentSessionHost() - if (!host) { - throw new Error('structured_agent_session_unsupported') - } - return host -} - function assertMobileImageProvenance( ctx: RpcContext, body: { blocks: { type: string; path?: string; url?: string }[] } @@ -78,26 +57,6 @@ function assertMobileImageProvenance( } } -/** Attach is the only way a session comes into being, so it is the only call - * that builds the host. Every other method addresses a session that must - * already be attached, and correctly reports absent when none is. */ -async function ensureHostInstalled(ctx: RpcContext): Promise { - // Gated first: a client that cannot read structured sessions must not be able - // to make the host exist, which is an observable side effect of the surface. - if (!supportsStructuredSessions(ctx) || getStructuredAgentSessionHost()) { - return - } - await ctx.runtime.ensureStructuredAgentSessionHost() -} - -/** Mirrors the existing agent-session host-authority derivation so one client - * gets one operation namespace across both surfaces. */ -function callerFor(ctx: RpcContext): StructuredAgentSessionCaller { - return { - callerKey: ctx.clientId?.trim() || `trusted-local:${ctx.clientKind ?? 'runtime'}` - } -} - function subscriptionIdFor(ctx: RpcContext, sessionId: string): string { const base = `${SUBSCRIPTION_PREFIX}:${ctx.connectionId ?? 'local'}:${sessionId}` // Shared control multiplexes several streams over one socket; the frame id @@ -223,6 +182,16 @@ export const STRUCTURED_AGENT_SESSION_METHODS: RpcAnyMethod[] = [ params: CancelParams, handler: async (params, ctx) => requireHost(ctx).cancel(callerFor(ctx), params) }), + defineMethod({ + // Releasing a chat view, not ending a conversation: the record and journal stay on disk so the + // same session can be attached again. Only the provider child and the in-memory entry go. + name: 'agentSession.close', + params: OptionsParams, + handler: async (params, ctx) => { + await requireHost(ctx).close(params.sessionId) + return { ok: true as const } + } + }), defineMethod({ name: 'agentSession.respondToApproval', params: RespondParams, @@ -266,16 +235,40 @@ export const STRUCTURED_AGENT_SESSION_METHODS: RpcAnyMethod[] = [ handler: async (params, ctx, emit) => { const host = requireHost(ctx) const subscriptionId = subscriptionIdFor(ctx, params.sessionId) + // A live stream is a surface too. This is what keeps a client that never learned to hold — + // an older paired build — from having its session evicted while it is reading, and what + // releases one whose socket dies without a word. + // + // Retain-only for a client that holds its own surfaces: reading history must never be what + // starts a provider process. A paired build that predates holds gets the resuming kind, + // because subscribing IS the only thing it does to say a chat is open, and the host no longer + // resumes every record at launch on its behalf. + const streamHolder = `subscription:${subscriptionId}` + const streamResumes = + ctx.clientKind === 'mobile' && + !( + ctx.clientCapabilities?.includes(STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY) ?? + false + ) let closed = false let dispose = (): void => {} - ctx.runtime.registerSubscriptionCleanup( + let releaseTransportSubscription = (): void => {} + const onTransportAbort = (): void => releaseTransportSubscription() + const registration = ctx.runtime.registerOwnedSubscriptionCleanup( subscriptionId, () => { closed = true + ctx.signal?.removeEventListener('abort', onTransportAbort) dispose() + host.release(params.sessionId, streamHolder) }, ctx.connectionId ) + releaseTransportSubscription = registration.releaseIfCurrent + ctx.signal?.addEventListener('abort', onTransportAbort, { once: true }) + if (ctx.signal?.aborted) { + onTransportAbort() + } if (closed) { return } @@ -289,6 +282,14 @@ export const STRUCTURED_AGENT_SESSION_METHODS: RpcAnyMethod[] = [ }) if (closed) { dispose() + } else { + // Fire-and-forget, but never unhandled: a resume that refuses leaves the stream holding a + // readable session, which is exactly what the client sees anyway. + void host + .hold(params.sessionId, streamHolder, { resume: streamResumes }) + .catch((error: unknown) => + console.warn('[agent-session] stream hold failed', params.sessionId, error) + ) } } }), @@ -307,5 +308,6 @@ export const STRUCTURED_AGENT_SESSION_METHODS: RpcAnyMethod[] = [ ctx.runtime.cleanupSubscriptionsByPrefix(`${base}:`) return { unsubscribed: true } } - }) + }), + ...STRUCTURED_AGENT_SESSION_HOLD_METHODS ] diff --git a/src/main/runtime/runtime-rpc.ts b/src/main/runtime/runtime-rpc.ts index 38a05c16b9bc..81f749c98e66 100644 --- a/src/main/runtime/runtime-rpc.ts +++ b/src/main/runtime/runtime-rpc.ts @@ -191,8 +191,11 @@ const MOBILE_RPC_METHOD_ALLOWLIST = new Set([ 'agentSession.create', 'agentSession.createSupport', 'agentSession.cancel', + 'agentSession.close', 'agentSession.history', + 'agentSession.hold', 'agentSession.options', + 'agentSession.release', 'agentSession.respondToApproval', 'agentSession.respondToQuestion', 'agentSession.send', diff --git a/src/main/runtime/structured-agent-session-runtime.ts b/src/main/runtime/structured-agent-session-runtime.ts index d23f0766d207..d2800876f442 100644 --- a/src/main/runtime/structured-agent-session-runtime.ts +++ b/src/main/runtime/structured-agent-session-runtime.ts @@ -20,6 +20,7 @@ import type { StructuredAgentSessionHandoffTransport } from '../native-chat/agen import { setStructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-registry' import { AgentSessionRecordStore } from './agent-session-record-store' import { + probeAgentSessionProcessIdentities, probeAgentSessionProcessIdentity, probeAgentSessionReservation } from './agent-session-process-identity-probe' @@ -120,6 +121,7 @@ async function install(deps: StructuredAgentSessionRuntimeDeps): Promise deps.onError?.({ scope: `structured-agent-session-journal:${sessionId}`, error }), ...(deps.handoffTransport ? { handoffTransport: deps.handoffTransport } : {}) @@ -187,6 +189,38 @@ export function createStructuredAgentSessionOwnerProbe( } } +export function createStructuredAgentSessionOwnerProbes( + hostId: string +): (records: readonly AgentSessionRecord[]) => Promise> { + const probeOne = createStructuredAgentSessionOwnerProbe(hostId) + return async (records) => { + const results = new Map() + const localOwners: { + record: AgentSessionRecord + owner: NonNullable + }[] = [] + for (const record of records) { + const owner = record.lease.ownerProcess + if (owner?.hostId === hostId) { + localOwners.push({ record, owner }) + } else { + results.set(record.sessionId, await probeOne(record)) + } + } + const probes = await probeAgentSessionProcessIdentities({ + identities: localOwners.map(({ owner }) => owner), + deps: { readEchoedSpawnToken: readEchoedAgentSessionSpawnToken } + }) + for (const [index, { record }] of localOwners.entries()) { + results.set( + record.sessionId, + probes[index] ?? { outcome: 'indeterminate', reason: 'owner probe returned no result' } + ) + } + return results + } +} + /** * The only provider-side trace a reservation can leave in its own record: a handle link minted at * this fence. `proveAgentSessionOwner` refuses to append one before an identity is committed, so a diff --git a/src/renderer/src/components/native-chat/NativeChatOrchestrationPausedNotice.test.tsx b/src/renderer/src/components/native-chat/NativeChatOrchestrationPausedNotice.test.tsx new file mode 100644 index 000000000000..a13f672feb44 --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatOrchestrationPausedNotice.test.tsx @@ -0,0 +1,33 @@ +// @vitest-environment happy-dom + +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it } from 'vitest' +import { NativeChatOrchestrationPausedNotice } from './NativeChatOrchestrationPausedNotice' + +describe('NativeChatOrchestrationPausedNotice', () => { + afterEach(cleanup) + + it('stays hidden while dispatch state is loading or settled', () => { + const { rerender } = render() + + expect(screen.queryByRole('status')).toBeNull() + + rerender() + expect(screen.queryByRole('status')).toBeNull() + }) + + it.each(['pending', 'dispatched'] as const)( + 'persists recovery guidance for an active %s Dispatch', + (dispatchStatus) => { + render() + + const notice = screen.getByRole('status') + expect(notice.textContent).toContain('Orchestration paused') + expect(notice.textContent).toContain('Structured Chat blocks terminal prompts and sends') + expect(notice.textContent).toContain('Orchestration messages remain queued') + expect(notice.textContent).toContain( + 'switch to Terminal, then check the Orca inbox with orca orchestration check' + ) + } + ) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatOrchestrationPausedNotice.tsx b/src/renderer/src/components/native-chat/NativeChatOrchestrationPausedNotice.tsx new file mode 100644 index 000000000000..0e43f0cb1bbd --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatOrchestrationPausedNotice.tsx @@ -0,0 +1,36 @@ +import { PauseCircle } from 'lucide-react' +import type { AgentStatusOrchestrationContext } from '../../../../shared/agent-status-types' +import { Badge } from '@/components/ui/badge' +import { translate } from '@/i18n/i18n' + +export function NativeChatOrchestrationPausedNotice({ + dispatchStatus +}: { + dispatchStatus?: AgentStatusOrchestrationContext['dispatchStatus'] +}): React.JSX.Element | null { + if (dispatchStatus !== 'pending' && dispatchStatus !== 'dispatched') { + return null + } + + return ( +
+
+ ) +} diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx index ba6d46678829..8033aa4a19a3 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx @@ -106,6 +106,7 @@ describe('NativeChatStructuredSession', () => { it('wires local structured file links through the native chat opener', () => { render( { it('routes a bare model command to the native option picker', async () => { render( void + orchestrationDispatchStatus?: AgentStatusOrchestrationContext['dispatchStatus'] }): React.JSX.Element { const controller = useStructuredAgentSession(props) const [composerError, setComposerError] = useState(null) @@ -117,6 +121,7 @@ export function NativeChatStructuredSession(props: { tabIndex={-1} className="flex h-full min-h-0 w-full flex-col bg-background focus:outline-none" > +
{viewState.kind === 'loading' ? ( diff --git a/src/renderer/src/components/native-chat/NativeChatView.tsx b/src/renderer/src/components/native-chat/NativeChatView.tsx index a85f1e4b5121..3574ab90eb8e 100644 --- a/src/renderer/src/components/native-chat/NativeChatView.tsx +++ b/src/renderer/src/components/native-chat/NativeChatView.tsx @@ -53,6 +53,7 @@ import type { NativeChatResolvedViewProps, NativeChatViewProps } from './native- import { NativeChatStructuredSession } from './NativeChatStructuredSession' import { useNativeChatStatusEntry } from './use-native-chat-status-entry' import { useNativeChatFileLinkContext } from './use-native-chat-file-link-context' +import { NativeChatOrchestrationPausedNotice } from './NativeChatOrchestrationPausedNotice' export type { NativeChatViewProps } from './native-chat-view-types' @@ -73,7 +74,8 @@ function NativeChatBridgeView({ resolvedAgent, onSwitchToTerminal, readTerminalScreen, - contextMenuActions + contextMenuActions, + orchestrationDispatchStatus }: Exclude): React.JSX.Element { const { entry: agentStatusEntry, paneKey } = useNativeChatStatusEntry( terminalTabId, @@ -99,6 +101,7 @@ function NativeChatBridgeView({ onSwitchToTerminal={onSwitchToTerminal} readTerminalScreen={readTerminalScreen} contextMenuActions={contextMenuActions} + orchestrationDispatchStatus={orchestrationDispatchStatus} /> )} @@ -115,7 +118,8 @@ function NativeChatResolvedView({ terminalTabId, onSwitchToTerminal, readTerminalScreen, - contextMenuActions + contextMenuActions, + orchestrationDispatchStatus }: NativeChatResolvedViewProps): React.JSX.Element { // Primitive owner selection (no useShallow): routes the pane's read/subscribe to // the remote runtime host for a runtime-owned pane; null keeps the local path. @@ -394,6 +398,7 @@ function NativeChatResolvedView({ onContextMenuCapture={contextMenu.onContextMenuCapture} className="flex h-full min-h-0 w-full flex-col bg-background focus:outline-none" > +
{viewState.kind === 'loading' ? ( diff --git a/src/renderer/src/components/native-chat/native-chat-view-types.ts b/src/renderer/src/components/native-chat/native-chat-view-types.ts index d1a206be82bf..24be8a9d4d0f 100644 --- a/src/renderer/src/components/native-chat/native-chat-view-types.ts +++ b/src/renderer/src/components/native-chat/native-chat-view-types.ts @@ -1,10 +1,17 @@ -import type { AgentType } from '../../../../shared/agent-status-types' +import type { + AgentStatusOrchestrationContext, + AgentType +} from '../../../../shared/agent-status-types' import type { TuiAgent } from '../../../../shared/tui-agent' import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client' import type { NativeChatSession } from '../../../../shared/native-chat-types' import type { NativeChatContextMenuActions } from './use-native-chat-context-menu' -export type NativeChatBridgeViewProps = { +type NativeChatOrchestrationProps = { + orchestrationDispatchStatus?: AgentStatusOrchestrationContext['dispatchStatus'] +} + +export type NativeChatBridgeViewProps = NativeChatOrchestrationProps & { mode?: 'bridge' /** The terminal tab hosting the agent. paneKey is `${tabId}:${leafId}`. */ terminalTabId: string @@ -25,17 +32,18 @@ export type NativeChatBridgeViewProps = { contextMenuActions?: Omit } -export type NativeChatStructuredViewProps = { +export type NativeChatStructuredViewProps = NativeChatOrchestrationProps & { mode: 'structured' tabId: string sessionId: string target: RuntimeClientTarget agent: AgentType + isVisible: boolean allowFileUriLinks: boolean onSwitchToTerminal?: () => void } -export type NativeChatResolvedViewProps = { +export type NativeChatResolvedViewProps = NativeChatOrchestrationProps & { paneKey: string agent: NativeChatSession['agent'] sessionId: string | null diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-hold.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-hold.test.tsx new file mode 100644 index 000000000000..b915958a2661 --- /dev/null +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-hold.test.tsx @@ -0,0 +1,100 @@ +// @vitest-environment happy-dom + +// A structured chat is a view on a terminal tab, so closing the tab is an unmount and nothing else. +// If that unmount does not reach main, the codex app-server behind the chat has no other way to +// learn the chat is gone. + +import { renderHook, waitFor } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ call: vi.fn() })) + +vi.mock('@/runtime/structured-agent-session-client', () => ({ + callStructuredAgentSession: mocks.call, + subscribeStructuredAgentSession: vi.fn() +})) + +import { useStructuredAgentSessionHold } from './use-structured-agent-session-hold' + +const LOCAL_TARGET = { kind: 'local' } as const + +function callsTo(method: string): unknown[] { + return mocks.call.mock.calls.filter((call) => call[1] === method).map((call) => call[2]) +} + +beforeEach(() => { + mocks.call.mockReset() + mocks.call.mockResolvedValue(undefined) +}) + +describe('a mounted structured chat', () => { + it('holds the session while it is on screen and releases it on unmount', async () => { + const { unmount } = renderHook(() => + useStructuredAgentSessionHold({ + sessionId: 'session-alpha', + target: LOCAL_TARGET, + surface: 'desktop-chat' + }) + ) + + await waitFor(() => expect(callsTo('agentSession.hold')).toHaveLength(1)) + const held = callsTo('agentSession.hold')[0] as { sessionId: string; holderId: string } + expect(held.sessionId).toBe('session-alpha') + expect(callsTo('agentSession.release')).toHaveLength(0) + + unmount() + + await waitFor(() => + expect(callsTo('agentSession.release')).toEqual([ + { sessionId: 'session-alpha', holderId: held.holderId } + ]) + ) + }) + + it('does not release a hold that has not landed yet', async () => { + let settleHold = (): void => {} + mocks.call.mockImplementation((_target: unknown, method: string) => + method === 'agentSession.hold' + ? new Promise((resolve) => { + settleHold = resolve + }) + : Promise.resolve() + ) + const { unmount } = renderHook(() => + useStructuredAgentSessionHold({ + sessionId: 'session-alpha', + target: LOCAL_TARGET, + surface: 'desktop-chat' + }) + ) + await waitFor(() => expect(callsTo('agentSession.hold')).toHaveLength(1)) + + unmount() + // The hold is still in flight; releasing now would leave the late hold with nothing to undo it. + expect(callsTo('agentSession.release')).toHaveLength(0) + + settleHold() + + await waitFor(() => expect(callsTo('agentSession.release')).toHaveLength(1)) + }) + + it('keeps one hold across re-renders that rebuild the target object', async () => { + const { rerender, unmount } = renderHook( + (props: { sessionId: string }) => + useStructuredAgentSessionHold({ + sessionId: props.sessionId, + target: { kind: 'local' }, + surface: 'desktop-chat' + }), + { initialProps: { sessionId: 'session-alpha' } } + ) + await waitFor(() => expect(callsTo('agentSession.hold')).toHaveLength(1)) + + rerender({ sessionId: 'session-alpha' }) + rerender({ sessionId: 'session-alpha' }) + + expect(callsTo('agentSession.hold')).toHaveLength(1) + expect(callsTo('agentSession.release')).toHaveLength(0) + unmount() + }) +}) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-hold.ts b/src/renderer/src/components/native-chat/use-structured-agent-session-hold.ts new file mode 100644 index 000000000000..13fa5b1adf56 --- /dev/null +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-hold.ts @@ -0,0 +1,55 @@ +// The desktop chat telling main that this session is on screen. +// +// A structured chat is an in-place view on a terminal tab, so closing the tab unmounts this and +// nothing else in the close path knows a provider process is involved: `closeUnifiedTab` retires +// the PTY and drops the tab, main hears nothing, and a codex app-server outlives the chat for the +// rest of the app's life. Mount/unmount is the honest signal — it covers closing the tab, closing +// the window, and switching the pane back to the terminal, none of which share a code path. +// +// The release CHAINS off the hold rather than racing it: an unmount during the hold's round trip +// would otherwise release a hold that has not landed yet, and the late hold would never be undone. + +import { useEffect, useRef } from 'react' +import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client' +import { callStructuredAgentSession } from '@/runtime/structured-agent-session-client' + +let holderOrdinal = 0 + +export function structuredAgentSessionHolderId(surface: string): string { + holderOrdinal += 1 + return `${surface}:${holderOrdinal}` +} + +export function useStructuredAgentSessionHold(args: { + sessionId: string + target: RuntimeClientTarget + surface: string +}): void { + const { sessionId, surface, target } = args + // Keyed by VALUE, not identity: callers build the target inline, so an identity dependency would + // release and re-take the hold on every render of the pane. + const targetKey = target.kind === 'local' ? 'local' : `environment:${target.environmentId}` + const targetRef = useRef(target) + // Synced in an effect declared first (so it lands before the hold below) rather than in render: + // a render React discards must not leak its target into the next commit. + useEffect(() => { + targetRef.current = target + }, [target]) + useEffect(() => { + const runtimeTarget = targetRef.current + const holderId = structuredAgentSessionHolderId(surface) + const held = callStructuredAgentSession(runtimeTarget, 'agentSession.hold', { + sessionId, + holderId + // An older host has no such method; the session still reads, it just is not held. + }).catch(() => undefined) + return () => { + void held.then(() => + callStructuredAgentSession(runtimeTarget, 'agentSession.release', { + sessionId, + holderId + }).catch(() => undefined) + ) + } + }, [sessionId, surface, targetKey]) +} diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-read.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-read.test.tsx index 3942c40eaaa1..cb1126b0101b 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-read.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-read.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment happy-dom -import { act, renderHook, waitFor } from '@testing-library/react' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { act, cleanup, renderHook, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { AgentJournalCursor, AgentJournalRenderItem @@ -77,6 +77,8 @@ function page( } describe('useStructuredAgentSessionRead history window', () => { + afterEach(cleanup) + beforeEach(() => { vi.clearAllMocks() mocks.subscribe.mockResolvedValue({ unsubscribe: vi.fn() }) @@ -155,4 +157,36 @@ describe('useStructuredAgentSessionRead history window', () => { expect(result.current.state.items).toHaveLength(301) expect(result.current.state.items[0]?.itemId).toBe('oldest') }) + + it('refreshes only visible structured sessions when the app regains focus', async () => { + const hasFocus = vi.spyOn(document, 'hasFocus').mockReturnValue(true) + mocks.call.mockResolvedValue({ ok: true, page: page('tail', [], false) }) + const visible = renderHook(() => + useStructuredAgentSessionRead({ + sessionId: 'session-visible', + target: LOCAL_TARGET, + isVisible: true + }) + ) + const hidden = renderHook(() => + useStructuredAgentSessionRead({ + sessionId: 'session-hidden', + target: LOCAL_TARGET, + isVisible: false + }) + ) + await waitFor(() => expect(mocks.call).toHaveBeenCalledTimes(2)) + + act(() => window.dispatchEvent(new Event('focus'))) + + await waitFor(() => expect(mocks.call).toHaveBeenCalledTimes(3)) + expect(mocks.call).toHaveBeenLastCalledWith(LOCAL_TARGET, 'agentSession.history', { + sessionId: 'session-visible', + direction: 'tail', + limit: AGENT_SESSION_HISTORY_MAX_LIMIT + }) + visible.unmount() + hidden.unmount() + hasFocus.mockRestore() + }) }) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-read.ts b/src/renderer/src/components/native-chat/use-structured-agent-session-read.ts index a336bb917df1..caca77d60c31 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session-read.ts +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-read.ts @@ -49,12 +49,23 @@ function createReconnectScheduler(args: { shouldStop: () => boolean; reconnect: export function useStructuredAgentSessionRead(args: { sessionId: string target: RuntimeClientTarget + isVisible?: boolean }) { - const { sessionId, target } = args + const { sessionId, target, isVisible = true } = args const [state, dispatch] = useReducer(reduceStructuredAgentSession, EMPTY_STRUCTURED_AGENT_SESSION) const stateRef = useRef(state) const resumeCursorRef = useRef(state.cursor) const [loadingOlder, setLoadingOlder] = useState(false) + const refreshOnFocusRef = useRef<() => void>(() => {}) + + useEffect(() => { + if (!isVisible) { + return + } + const refresh = (): void => refreshOnFocusRef.current() + window.addEventListener('focus', refresh) + return () => window.removeEventListener('focus', refresh) + }, [isVisible]) useEffect(() => { stateRef.current = state @@ -198,7 +209,7 @@ export function useStructuredAgentSessionRead(args: { } }) } - window.addEventListener('focus', refreshOnFocus) + refreshOnFocusRef.current = refreshOnFocus void refreshTail() .then(() => open()) .catch((error) => { @@ -207,7 +218,9 @@ export function useStructuredAgentSessionRead(args: { }) return () => { stopped = true - window.removeEventListener('focus', refreshOnFocus) + if (refreshOnFocusRef.current === refreshOnFocus) { + refreshOnFocusRef.current = () => {} + } reconnectScheduler.dispose() coalescer.dispose() unsubscribe() diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx index 3090fc638c70..2e611e4c726c 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx @@ -97,7 +97,8 @@ describe('useStructuredAgentSession options', () => { useStructuredAgentSession({ sessionId: 'session-1', target: LOCAL_TARGET, - agent: 'codex' + agent: 'codex', + isVisible: true }) ) await waitFor(() => expect(result.current.optionSnapshot).toHaveLength(2)) @@ -128,7 +129,8 @@ describe('useStructuredAgentSession options', () => { useStructuredAgentSession({ sessionId: 'session-1', target: LOCAL_TARGET, - agent: 'codex' + agent: 'codex', + isVisible: true }) ) await waitFor(() => expect(result.current.optionSnapshot).toHaveLength(2)) @@ -146,8 +148,9 @@ describe('useStructuredAgentSession options', () => { it('mints a fresh operation when the same option is retried after a typed refusal', async () => { let attempts = 0 mocks.call.mockImplementation((_target, method) => { - if (method === 'agentSession.options') { - return Promise.resolve(OPTIONS) + if (method !== 'agentSession.setOption') { + // The hook also holds the session while it is mounted; only option writes are attempts. + return Promise.resolve(method === 'agentSession.options' ? OPTIONS : null) } attempts += 1 return Promise.resolve( @@ -173,7 +176,8 @@ describe('useStructuredAgentSession options', () => { useStructuredAgentSession({ sessionId: 'session-1', target: LOCAL_TARGET, - agent: 'codex' + agent: 'codex', + isVisible: true }) ) await waitFor(() => expect(result.current.optionSnapshot).toHaveLength(2)) @@ -197,8 +201,9 @@ describe('useStructuredAgentSession options', () => { it('reuses an option operation after a pending admission refusal', async () => { let attempts = 0 mocks.call.mockImplementation((_target, method) => { - if (method === 'agentSession.options') { - return Promise.resolve(OPTIONS) + if (method !== 'agentSession.setOption') { + // The hook also holds the session while it is mounted; only option writes are attempts. + return Promise.resolve(method === 'agentSession.options' ? OPTIONS : null) } attempts += 1 return Promise.resolve( @@ -226,7 +231,8 @@ describe('useStructuredAgentSession options', () => { useStructuredAgentSession({ sessionId: 'session-1', target: LOCAL_TARGET, - agent: 'codex' + agent: 'codex', + isVisible: true }) ) await waitFor(() => expect(result.current.optionSnapshot).toHaveLength(2)) @@ -271,7 +277,8 @@ describe('useStructuredAgentSession options', () => { useStructuredAgentSession({ sessionId: 'session-1', target: LOCAL_TARGET, - agent: 'codex' + agent: 'codex', + isVisible: true }) ) await waitFor(() => expect(result.current.optionSnapshot).toHaveLength(2)) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session.ts b/src/renderer/src/components/native-chat/use-structured-agent-session.ts index 94301dc0489e..66afda348da0 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session.ts +++ b/src/renderer/src/components/native-chat/use-structured-agent-session.ts @@ -25,6 +25,7 @@ import { structuredSessionOperationId, useStructuredAgentSessionOutbox } from './use-structured-agent-session-outbox' +import { useStructuredAgentSessionHold } from './use-structured-agent-session-hold' import { useStructuredAgentSessionRead } from './use-structured-agent-session-read' import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' @@ -36,9 +37,17 @@ export function useStructuredAgentSession(args: { sessionId: string target: RuntimeClientTarget agent: AgentType + isVisible: boolean }) { - const { agent, sessionId, target } = args - const { state, loadingOlder, loadOlder } = useStructuredAgentSessionRead({ sessionId, target }) + const { agent, isVisible, sessionId, target } = args + // Declared first: the hold is what gives a restored session its provider child back, and the + // read below is useless for sending until it lands. + useStructuredAgentSessionHold({ sessionId, target, surface: 'desktop-chat' }) + const { state, loadingOlder, loadOlder } = useStructuredAgentSessionRead({ + sessionId, + target, + isVisible + }) const stateRef = useRef(state) const [writeError, setWriteError] = useState(null) const operationIds = useRef(new Map()) diff --git a/src/renderer/src/components/tab-group/TabGroupPanel.tsx b/src/renderer/src/components/tab-group/TabGroupPanel.tsx index 0c1f90188d2d..8d1ed2ccee65 100644 --- a/src/renderer/src/components/tab-group/TabGroupPanel.tsx +++ b/src/renderer/src/components/tab-group/TabGroupPanel.tsx @@ -380,6 +380,7 @@ export default function TabGroupPanel({ tabId={activeTab.id} sessionId={activeTab.entityId} agent={activeTab.agentSessionAgent ?? 'codex'} + isVisible={isVisible} target={structuredRuntimeTarget} allowFileUriLinks={structuredFileLinksEnabled} /> diff --git a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts index 8caae122e7ba..f27be0a02c56 100644 --- a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts +++ b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts @@ -1,4 +1,5 @@ import { useCallback } from 'react' +import { toast } from 'sonner' import type { Tab } from '../../../../shared/tab-types' import { useAppStore } from '../../store' import { destroyWorkspaceWebviews } from '../../store/slices/browser-webview-cleanup' @@ -12,6 +13,17 @@ import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner import { browserWorkspaceHasRemoteOwner } from '@/runtime/remote-browser-tab-ownership' import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' import { toRuntimeWorktreeSelector } from '@/runtime/runtime-worktree-selector' +import { translate } from '@/i18n/i18n' + +function reportStructuredSessionCloseError(error: unknown): void { + toast.error( + translate( + 'components.native-chat.structuredSessionCloseFailed', + 'Could not close this Codex chat' + ), + { description: error instanceof Error ? error.message : String(error) } + ) +} export function useTabGroupTabCloseCommands({ worktreeId, @@ -87,10 +99,13 @@ export function useTabGroupTabCloseCommands({ tabId: `agent-session:${item.entityId}`, reason: 'user' }) - closeUnifiedTab(item.id) - if (!opts?.skipEmptyCheck) { - leaveWorktreeIfEmpty() - } + .then(() => { + closeUnifiedTab(item.id) + if (!opts?.skipEmptyCheck) { + leaveWorktreeIfEmpty() + } + }) + .catch(reportStructuredSessionCloseError) return } if (item.contentType === 'terminal') { @@ -164,7 +179,8 @@ export function useTabGroupTabCloseCommands({ tabId: `agent-session:${item.entityId}`, reason: 'user' }) - closeUnifiedTab(item.id) + .then(() => closeUnifiedTab(item.id)) + .catch(reportStructuredSessionCloseError) continue } if (item.contentType === 'terminal' && isWebRuntimeSessionActive(runtimeEnvironmentId)) { diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index dfbe7558dfed..27c131475509 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -534,6 +534,11 @@ function TerminalPane( ) const nativeChatEnabled = useAppStore((store) => store.settings?.experimentalNativeChat === true) const effectiveChatViewMode = nativeChatEnabled && isChatViewMode + const chatPaneDispatchStatus = useAppStore((store) => + chatLeafId + ? store.agentStatusByPaneKey[makePaneKey(tabId, chatLeafId)]?.orchestration?.dispatchStatus + : undefined + ) const unifiedTabLabel = useAppStore( (store) => getCachedUnifiedTerminalTabForWorktree(store.unifiedTabsByWorktree, worktreeId, tabId)?.label @@ -3170,8 +3175,10 @@ function TerminalPane( tabId={unifiedTabId ?? tabId} sessionId={structuredSessionId} agent="codex" + isVisible={isRendererVisible} target={{ kind: 'local' }} allowFileUriLinks + orchestrationDispatchStatus={chatPaneDispatchStatus} onSwitchToTerminal={switchNativeChatToTerminal} /> ) : ( @@ -3213,6 +3220,7 @@ function TerminalPane( canClosePane: managedPanes.length > 1, onClosePane: () => contextMenu.runForPane(chatPane.id, contextMenu.onClosePane) }} + orchestrationDispatchStatus={chatPaneDispatchStatus} /> )}
, diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 8ac9f0c159fd..09003f10f7ed 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -16109,7 +16109,13 @@ "allow": "Allow", "deny": "Deny" }, - "launchPromptNotDelivered": "Not delivered — check the terminal" + "launchPromptNotDelivered": "Not delivered — check the terminal", + "orchestrationPaused": { + "label": "Orchestration paused", + "message": "Structured Chat blocks terminal prompts and sends. Orchestration messages remain queued; switch to Terminal, then check the Orca inbox with" + }, + "structuredSessionCloseFailed": "Could not close this Codex chat", + "structuredSessionLaunchFailed": "Could not open Codex chat" }, "tab": { "bar": { diff --git a/src/renderer/src/lib/launch-agent-in-new-tab.ts b/src/renderer/src/lib/launch-agent-in-new-tab.ts index 8573079003f5..fd5d08054ae4 100644 --- a/src/renderer/src/lib/launch-agent-in-new-tab.ts +++ b/src/renderer/src/lib/launch-agent-in-new-tab.ts @@ -1,3 +1,4 @@ +import { toast } from 'sonner' import { useAppStore } from '@/store' import type { AgentStartupPlan } from '@/lib/tui-agent-startup' import { planLaunchAgentStartupPrompt } from '@/lib/launch-agent-startup-prompt-plan' @@ -31,6 +32,8 @@ import { resolveInitialNativeChatSessionOptions } from '@/components/native-chat import { seedNativeChatAppliedSessionOptions } from '@/components/native-chat/native-chat-session-option-cache' import { canUseStructuredNativeChat } from '@/lib/structured-native-chat-availability' import { waitForAgentReady } from '@/lib/agent-ready-wait' +import { launchStructuredCodexSession } from '@/lib/launch-structured-codex-session' +import { translate } from '@/i18n/i18n' export type LaunchAgentInNewTabArgs = { agent: TuiAgent @@ -176,6 +179,26 @@ export function launchAgentInNewTab(args: LaunchAgentInNewTabArgs): LaunchAgentI } } + const launchDirectStructuredChat = + agent === 'codex' && + !hasPrompt && + initialViewModeProps.viewMode === 'chat' && + canUseStructuredNativeChat(store, worktreeId) + if (launchDirectStructuredChat) { + void launchStructuredCodexSession(worktreeId).catch((error) => { + toast.error( + translate( + 'components.native-chat.structuredSessionLaunchFailed', + 'Could not open Codex chat' + ), + { + description: error instanceof Error ? error.message : String(error) + } + ) + }) + return { tabId: null, startupPlan, pasteDraftAfterLaunch: false } + } + // Why: queue startup BEFORE TerminalPane mounts — it snapshots pendingStartupByTabId in useState on first render. // Why: followup path pastes an unsubmitted draft, so gate the initial chat view like a draft launch, not auto-submit. const adoptStructuredChat = diff --git a/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts b/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts index 632127fa065c..a6a3f02a73e7 100644 --- a/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts +++ b/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts @@ -5,6 +5,7 @@ const mockSetTabViewMode = vi.fn() const mockWaitForAgentReady = vi.fn() const mockPasteDraftWhenAgentReady = vi.fn() const mockMarkNativeChatLaunchPromptFailed = vi.fn() +const mockLaunchStructuredCodexSession = vi.fn() const store = { activeRepoId: 'repo-1', @@ -61,6 +62,9 @@ vi.mock('@/runtime/web-runtime-session', () => ({ isWebRuntimeSessionActive: vi.fn(() => false), isWebTerminalSurfaceTabId: vi.fn(() => false) })) +vi.mock('@/lib/launch-structured-codex-session', () => ({ + launchStructuredCodexSession: mockLaunchStructuredCodexSession +})) /** Structured adoption creates the tab in terminal mode and flips it to chat once * Codex is ready; the bridge stamps `viewMode: 'chat'` on the tab up front. That @@ -73,24 +77,25 @@ describe('structured chat adoption guard on the launch path', () => { mockCreateTab.mockReturnValue({ id: 'tab-1' }) mockWaitForAgentReady.mockResolvedValue({ ready: true, reason: 'foreground-match' }) mockPasteDraftWhenAgentReady.mockResolvedValue(true) + mockLaunchStructuredCodexSession.mockResolvedValue('codex-session-1') }) - it('adopts a local Codex tab into the structured stack', async () => { + it('honors the persisted chat default by launching Codex directly as a structured session', async () => { const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab') - launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' }) + const result = launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' }) - expect(mockCreateTab).toHaveBeenCalledWith('wt-1', undefined, undefined, { - launchAgent: 'codex' - }) - await vi.waitFor(() => expect(mockSetTabViewMode).toHaveBeenCalledWith('tab-1', 'chat')) + expect(result).toMatchObject({ tabId: null, pasteDraftAfterLaunch: false }) + expect(mockLaunchStructuredCodexSession).toHaveBeenCalledWith('wt-1') + expect(mockCreateTab).not.toHaveBeenCalled() + expect(mockWaitForAgentReady).not.toHaveBeenCalled() }) it('stays in terminal view when Codex never became ready', async () => { mockWaitForAgentReady.mockResolvedValue({ ready: false, reason: 'timeout' }) const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab') - launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' }) + launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1', prompt: 'start this task' }) await vi.waitFor(() => expect(mockWaitForAgentReady).toHaveBeenCalled()) // Let every continuation the ready wait could have queued run before asserting it did not flip. diff --git a/src/renderer/src/lib/launch-structured-codex-session.test.ts b/src/renderer/src/lib/launch-structured-codex-session.test.ts new file mode 100644 index 000000000000..f8d82809363c --- /dev/null +++ b/src/renderer/src/lib/launch-structured-codex-session.test.ts @@ -0,0 +1,50 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { structuredAgentSessionPayloadFingerprint } from '../../../shared/structured-agent-session-mutation' +import { callStructuredAgentSession } from '@/runtime/structured-agent-session-client' +import { launchStructuredCodexSession } from './launch-structured-codex-session' + +vi.mock('@/runtime/structured-agent-session-client', () => ({ + callStructuredAgentSession: vi.fn() +})) + +describe('structured Codex launch', () => { + beforeEach(() => { + vi.mocked(callStructuredAgentSession).mockReset() + }) + + it('creates a native session with a host-verifiable launch intent', async () => { + vi.mocked(callStructuredAgentSession).mockImplementation(async (_target, _method, params) => ({ + ok: true, + replayed: false, + fence: 1, + cursor: { epoch: 'epoch-1', sequence: 0 }, + value: { + sessionId: (params as { envelope: { sessionId: string } }).envelope.sessionId, + fence: 1, + snapshot: { cursor: { epoch: 'epoch-1', sequence: 0 }, items: [] }, + unconfirmedClientMessageIds: [] + } + })) + + const sessionId = await launchStructuredCodexSession('workspace-1') + const params = vi.mocked(callStructuredAgentSession).mock.calls[0]?.[2] as { + envelope: { sessionId: string; payloadFingerprint: string } + worktree: string + agent: 'codex' + } + + expect(sessionId).toMatch(/^codex_[A-Za-z0-9_]{36}$/) + expect(callStructuredAgentSession).toHaveBeenCalledWith( + { kind: 'local' }, + 'agentSession.create', + expect.objectContaining({ worktree: 'id:workspace-1', agent: 'codex' }) + ) + expect(params.envelope.payloadFingerprint).toBe( + structuredAgentSessionPayloadFingerprint({ + method: 'agentSession.create', + sessionId: params.envelope.sessionId, + fields: { worktree: 'id:workspace-1', agent: 'codex' } + }) + ) + }) +}) diff --git a/src/renderer/src/lib/launch-structured-codex-session.ts b/src/renderer/src/lib/launch-structured-codex-session.ts new file mode 100644 index 000000000000..390187da124b --- /dev/null +++ b/src/renderer/src/lib/launch-structured-codex-session.ts @@ -0,0 +1,38 @@ +import type { + AgentSessionAttachResult, + AgentSessionMutationResult +} from '../../../shared/agent-session-wire' +import { + createStructuredAgentSessionOperationId, + structuredAgentSessionPayloadFingerprint +} from '../../../shared/structured-agent-session-mutation' +import { callStructuredAgentSession } from '@/runtime/structured-agent-session-client' +import { toRuntimeWorktreeSelector } from '@/runtime/runtime-worktree-selector' + +function newSessionId(): string { + return `codex_${crypto.randomUUID().replaceAll('-', '_')}` +} + +export async function launchStructuredCodexSession(worktreeId: string): Promise { + const sessionId = newSessionId() + const fields = { worktree: toRuntimeWorktreeSelector(worktreeId), agent: 'codex' as const } + const result = await callStructuredAgentSession< + AgentSessionMutationResult + >({ kind: 'local' }, 'agentSession.create', { + envelope: { + sessionId, + clientOperationId: createStructuredAgentSessionOperationId(() => crypto.randomUUID()), + expectedRuntimeFence: null, + payloadFingerprint: structuredAgentSessionPayloadFingerprint({ + method: 'agentSession.create', + sessionId, + fields + }) + }, + ...fields + }) + if (!result.ok) { + throw new Error(result.refusal.message) + } + return result.value.sessionId +} diff --git a/src/shared/agent-session-pty-write-refusal-copy.ts b/src/shared/agent-session-pty-write-refusal-copy.ts new file mode 100644 index 000000000000..6820b6ce44ba --- /dev/null +++ b/src/shared/agent-session-pty-write-refusal-copy.ts @@ -0,0 +1,13 @@ +import type { AgentSessionPtyWriteRefusal } from './agent-session-pty-write-admission' + +export function structuredChatPtyWriteRefusalCopy( + refusal: AgentSessionPtyWriteRefusal, + action: 'terminal-send' | 'worker-start' +): string | null { + if (refusal.ownerRuntimeKind !== 'native') { + return null + } + return action === 'worker-start' + ? 'The target terminal is in Structured Chat. Switch it to Terminal, then retry `orca orchestration worker-start`.' + : 'The target terminal is in Structured Chat. Switch it to Terminal, then retry `orca terminal send`.' +} diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index 1bd31958cc02..7e29681125fc 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -106,6 +106,13 @@ export const AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY = // can neither display nor drive. The host also refuses every agentSession.* // method from a connection that does not advertise this. export const STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY = 'agent-session.structured.v1' as const +// Why: a client that advertises this takes an explicit hold on every session surface it opens, so +// the host knows when the last surface is gone and may stop the provider child. A client WITHOUT it +// (every paired build that shipped before holds existed) is held for the life of its subscription +// instead, and that subscription is what resumes its session — which is how those builds keep being +// able to send after the host stopped resuming every record at launch. +export const STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY = + 'agent-session.structured.hold.v1' as const // Why: the first structured mobile client hardcoded Codex rendering, so hosts may publish Claude tabs only after this narrower capability is negotiated. export const CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY = 'agent-session.structured.claude.v1' as const @@ -157,6 +164,7 @@ export const RUNTIME_CAPABILITIES = [ AGENT_SESSION_HOST_AUTHORITY_RUNTIME_CAPABILITY, AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, + STRUCTURED_AGENT_SESSION_HOLD_RUNTIME_CAPABILITY, CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY, FILE_MUTATION_OWNERSHIP_RUNTIME_CAPABILITY, diff --git a/tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts b/tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts index ea10aa02bedb..7be0c5c734d9 100644 --- a/tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts +++ b/tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts @@ -43,12 +43,15 @@ const STRUCTURED_CALLS: { method: string; hostMethod: string | null }[] = [ { method: 'agentSession.ensure', hostMethod: 'attach' }, { method: 'agentSession.send', hostMethod: 'send' }, { method: 'agentSession.cancel', hostMethod: 'cancel' }, + { method: 'agentSession.close', hostMethod: 'close' }, { method: 'agentSession.respondToApproval', hostMethod: 'respondToPrompt' }, { method: 'agentSession.respondToQuestion', hostMethod: 'respondToPrompt' }, { method: 'agentSession.setOption', hostMethod: 'setOption' }, { method: 'agentSession.handoffStatus', hostMethod: 'handoffStatus' }, { method: 'agentSession.handoff', hostMethod: 'requestHandoff' }, { method: 'agentSession.options', hostMethod: 'readOptions' }, + { method: 'agentSession.hold', hostMethod: 'hold' }, + { method: 'agentSession.release', hostMethod: 'release' }, { method: 'agentSession.history', hostMethod: 'history' }, { method: 'agentSession.subscribe', hostMethod: 'subscribe' }, // Teardown runs through the runtime's subscription registry rather than the @@ -162,6 +165,9 @@ function paramsFor(method: string): unknown { } case 'agentSession.history': return { sessionId: SESSION, direction: 'tail' } + case 'agentSession.hold': + case 'agentSession.release': + return { sessionId: SESSION, holderId: 'surface-1' } default: return { sessionId: SESSION } } @@ -242,6 +248,9 @@ describe('cross-version structured agent sessions', () => { attach: vi.fn(async () => ({ ok: true, replayed: false, value: { sessionId: SESSION } })), send: vi.fn(async () => ({ ok: true, replayed: false })), cancel: vi.fn(async () => ({ ok: true, replayed: false })), + close: vi.fn(async () => undefined), + hold: vi.fn(async () => undefined), + release: vi.fn(() => undefined), respondToPrompt: vi.fn(async () => ({ ok: true, replayed: false })), setOption: vi.fn(async () => ({ ok: true, replayed: false })), requestHandoff: vi.fn(async () => ({ status: { owner: 'native' } })), @@ -627,6 +636,9 @@ describe('cross-version structured agent sessions', () => { const restarted = await bootHost('b') await restarted.restoreReadableSessions() + // Restart restores the session for READING. The chat the client still has open takes its + // hold, and that is what gives the session a provider child again. + await answer('agentSession.hold', { sessionId: SESSION, holderId: 'surface-1' }) const resumedFence = store.getRecord(SESSION)?.lease.runtimeFence ?? 0 expect(resumedFence).toBeGreaterThan(created.fence) const second = await answer('agentSession.send', sendParams('after restart', resumedFence))