From 9976c914efe622a02b3211564df1d3f425530549 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 19 Aug 2026 19:46:32 -0700 Subject: [PATCH 1/3] fix(native-chat): reserve the lease before the adopted Codex proof MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switching a Codex tab to structured chat failed from every desktop entry point with "The agent terminal could not verify its Codex session." `adoptStructuredAgentSessionTerminal` typed `/status` into the pane through `writeAgentSessionProof`, which `agentSessionPtyWriteGate.admitProof` admits only for a reservation that already names the session, `tui`, and the spawn token. Both the reservation and `bindPty` happened *after* the proof, so the gate saw no binding and no record and refused the first keystroke. Neither proof branch could ever run, which is why an agent mid-conversation failed the same way as a cold one. The reservation and the pane binding now come first, so the probe is authorized as `provingReservation` — the state the gate was written to accept. A proof that does not land hands the reservation back with the durable processless proof and releases the pane, so the next attempt is not refused by the record its predecessor left behind. `proveOwner` gets the reservation's own fence rather than a hardcoded 1, which is what makes that retry land. Launch-at-chat shared the defect: it routes through the same `setTabViewMode` → `setTerminalNativeChatMode` → `agentSession.adoptTerminal` path, now pinned by a test. The new adoption test drives the real write gate over a real record store and a real rollout proof; every earlier test mocked the PTY controller, so `admitProof` never ran and none of them could observe this. --- .../structured-agent-session-host.ts | 21 ++ ...-agent-session-tui-adoption-reservation.ts | 164 +++++++++++ .../structured-agent-session-tui-adoption.ts | 47 +-- ...structured-tui-adoption-write-gate.test.ts | 267 ++++++++++++++++++ ...runtime-structured-tui-tab-binding.test.ts | 66 +++-- src/main/runtime/orca-runtime.ts | 92 +++++- .../tab-view-mode-structured-route.test.ts | 75 +++++ 7 files changed, 685 insertions(+), 47 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-tui-adoption-reservation.ts create mode 100644 src/main/runtime/orca-runtime-structured-tui-adoption-write-gate.test.ts create mode 100644 src/renderer/src/store/slices/tab-view-mode-structured-route.test.ts 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 abefff689c1..ef3ea367ff2 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 @@ -58,6 +58,13 @@ import { adoptStructuredTuiOwner, type StructuredTuiAdoptionRequest } from './structured-agent-session-tui-adoption' +import { + releaseStructuredTuiAdoptionReservation, + reserveStructuredTuiAdoption, + type StructuredTuiAdoptionReservationRelease, + type StructuredTuiAdoptionReservationRequest, + type StructuredTuiAdoptionReservationResult +} from './structured-agent-session-tui-adoption-reservation' import { readStructuredAgentSessionHistoryResult } from './structured-agent-session-history-result' export type { StructuredAgentSessionHostDeps } from './structured-agent-session-host-types' @@ -161,6 +168,20 @@ export class StructuredAgentSessionHost { return attachStructuredAgentSession(this.attachContext(), caller.callerKey, params) } + /** Reserve the lease an adopted TUI must hold before the write gate will admit its proof. */ + reserveAdoptedTuiOwner = ( + input: StructuredTuiAdoptionReservationRequest + ): Promise => + this.serialize(input.sessionId, () => + reserveStructuredTuiAdoption({ ...input, deps: this.deps, now: this.now }) + ) + + /** Hand back a reservation whose proof never landed, so the next attempt is not refused by it. */ + releaseAdoptedTuiReservation = (input: StructuredTuiAdoptionReservationRelease): Promise => + this.serialize(input.sessionId, () => + releaseStructuredTuiAdoptionReservation({ ...input, deps: this.deps, now: this.now }) + ) + adoptTuiOwner = ( input: StructuredTuiAdoptionRequest ): Promise> => diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-tui-adoption-reservation.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-tui-adoption-reservation.ts new file mode 100644 index 00000000000..cfdef2bbb45 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-tui-adoption-reservation.ts @@ -0,0 +1,164 @@ +/** + * The reservation an adopted Codex TUI needs BEFORE it may talk to the provider. + * + * Adoption proves the running Codex by typing `/status` into its own pane, and + * `agentSessionPtyWriteGate.admitProof` admits that probe only against a durable reservation that + * already names this session, this runtime kind, and this spawn token. Reserving after the proof — + * the shape this replaces — meant the probe was never admitted, so no adoption could ever succeed. + * + * The reservation is therefore created first and released again when the proof does not land, so a + * failed attempt leaves nothing behind that would refuse the next one. + */ + +import type { AgentSessionOwnerProbe } from '../../../shared/agent-session-lease-adjudication' +import type { + AgentSessionAccountHome, + AgentSessionExecutionLocation, + AgentSessionLaunchEnv, + AgentSessionRecord +} from '../../../shared/agent-session-record' +import type { AgentSessionHandleProvider } from '../../../shared/agent-session-provider-handle' +import type { AgentSessionWireRefusal } from '../../../shared/agent-session-wire' +import { classifyStoreFailure } from './structured-agent-session-attach' +import type { + StructuredAgentSessionCaller, + StructuredAgentSessionHostDeps +} from './structured-agent-session-host-types' + +export type StructuredTuiAdoptionReservationRequest = { + caller: StructuredAgentSessionCaller + sessionId: string + clientOperationId: string + /** The adoption call's own payload fingerprint; the durable operation row is keyed by it. */ + fingerprint: string + location: AgentSessionExecutionLocation + provider: AgentSessionHandleProvider + accountHome: AgentSessionAccountHome + spawnToken: string + claimKeyId: string + launchEnv?: AgentSessionLaunchEnv +} + +export type StructuredTuiAdoptionReservationResult = + | { + ok: true + fence: number + /** The token the proof must carry: a replayed call keeps the one already on the record. */ + spawnToken: string + } + | { ok: false; refusal: AgentSessionWireRefusal } + +export type StructuredTuiAdoptionReservationRelease = { + sessionId: string + fence: number + spawnToken: string +} + +/** The exact lease shape `admitProof` accepts as `provingReservation`. */ +export function isStructuredTuiAdoptionReservation( + record: AgentSessionRecord, + spawnToken: string +): boolean { + const { lease } = record + return ( + lease.runtimeKind === 'tui' && + lease.claimStatus === 'reserved' && + lease.handoffStage === 'new-owner-proving' && + lease.ownerProcess === null && + lease.reservedSpawnToken === spawnToken && + !lease.unreconciled + ) +} + +export async function reserveStructuredTuiAdoption( + input: StructuredTuiAdoptionReservationRequest & { + deps: Pick + now: () => number + } +): Promise { + const { deps, sessionId } = input + const existing = deps.store.getRecord(sessionId) + try { + const reserved = await deps.store.reserveOwner({ + sessionId, + location: input.location, + provider: input.provider, + accountHome: input.accountHome, + ...(input.launchEnv ? { launchEnv: input.launchEnv } : {}), + runtimeKind: 'tui', + // An adoption never declares a fence, so the compare-and-swap runs against what the store + // holds right now and the probe below is what refuses when that record still has an owner. + expectedFence: existing?.lease.runtimeFence ?? null, + spawnToken: input.spawnToken, + claimKeyId: input.claimKeyId, + // Why null: a retry mints a fresh operation id, and a non-null one here would make the + // reservation left behind by the previous attempt refuse it as a different operation. + handoffOperationId: null, + probe: await previousOwnerProbe(input, existing), + operation: { + callerKey: input.caller.callerKey, + operationId: input.clientOperationId, + fingerprint: input.fingerprint + }, + now: input.now() + }) + const spawnToken = reserved.record.lease.reservedSpawnToken + if (!spawnToken) { + throw new Error('agent_session_ownership_unknown') + } + return { ok: true, fence: reserved.record.lease.runtimeFence, spawnToken } + } catch (error) { + const current = deps.store.getRecord(sessionId) + return { + ok: false, + refusal: classifyStoreFailure(error, current?.lease.runtimeFence ?? null, current) + } + } +} + +/** + * Undo a reservation whose proof never landed. `processlessAt` is the durable "nothing ever + * spawned under this token" proof, which is what lets the next attempt supersede the record + * instead of being refused by it forever — and what stops a restart from latching it into + * recovery. + */ +export async function releaseStructuredTuiAdoptionReservation( + input: StructuredTuiAdoptionReservationRelease & { + deps: Pick + now: () => number + } +): Promise { + const record = input.deps.store.getRecord(input.sessionId) + if ( + !record || + record.lease.runtimeFence !== input.fence || + !isStructuredTuiAdoptionReservation(record, input.spawnToken) + ) { + // Superseded, already proven, or gone: whatever holds the record now is not this attempt's. + return + } + const now = input.now() + await input.deps.store.setReservationProcesslessProof({ + sessionId: input.sessionId, + fence: input.fence, + spawnToken: input.spawnToken, + processlessAt: now, + now + }) +} + +function previousOwnerProbe( + input: { deps: Pick }, + existing: AgentSessionRecord | null +): Promise | AgentSessionOwnerProbe { + if (!existing) { + return { outcome: 'reservation-unused' } + } + // Fails closed: without a prober the store must not be told the previous owner is gone. + return ( + input.deps.probeOwner?.(existing) ?? { + outcome: 'indeterminate', + reason: 'this host cannot probe the recorded owner' + } + ) +} 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 d9b4cc1a410..139577724e7 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 @@ -1,4 +1,5 @@ import { isDeepStrictEqual } from 'node:util' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' import type { AgentSessionAttachResult, AgentSessionMutationResult @@ -15,6 +16,7 @@ import type { StructuredAgentSessionHostSession } from './structured-agent-session-host-types' import type { StructuredTuiOwner } from './structured-agent-session-handoff-types' +import { isStructuredTuiAdoptionReservation } from './structured-agent-session-tui-adoption-reservation' export type StructuredTuiAdoptionRequest = { caller: StructuredAgentSessionCaller @@ -47,7 +49,13 @@ async function adopt( } let record = input.deps.store.getRecord(sessionId) let replayed = record !== null - if (!record) { + if (record && isStructuredTuiAdoptionReservation(record, owner.process.spawnToken)) { + // This adoption's own pre-proof reservation: turn it into an owner rather than reserve again. + replayed = false + record = await proveAdoptedTuiOwner(input, record) + } else if (record) { + assertMatchingTuiOwner(record, owner) + } else { const reserved = await input.deps.store.reserveOwner( reserveRequestFor({ sessionId, @@ -65,21 +73,7 @@ async function adopt( }) ) replayed = reserved.disposition === 'replayed' - record = reserved.record - await input.deps.store.commitProcessIdentity({ - sessionId, - fence: record.lease.runtimeFence, - process: owner.process, - now: input.now() - }) - record = await input.deps.store.proveOwner({ - sessionId, - fence: record.lease.runtimeFence, - link: owner.link, - now: input.now() - }) - } else { - assertMatchingTuiOwner(record, owner) + record = await proveAdoptedTuiOwner(input, reserved.record) } const attached = await attachJournal({ record, @@ -110,6 +104,27 @@ async function adopt( } } +/** Steps 4 and 5 of acquisition against a reservation that already exists at this fence. */ +async function proveAdoptedTuiOwner( + input: Parameters[0], + record: AgentSessionRecord +): Promise { + const { sessionId } = record + const fence = record.lease.runtimeFence + await input.deps.store.commitProcessIdentity({ + sessionId, + fence, + process: input.owner.process, + now: input.now() + }) + return input.deps.store.proveOwner({ + sessionId, + fence, + link: input.owner.link, + now: input.now() + }) +} + function assertMatchingTuiOwner( record: NonNullable>, owner: StructuredTuiOwner 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 new file mode 100644 index 00000000000..efb231c8818 --- /dev/null +++ b/src/main/runtime/orca-runtime-structured-tui-adoption-write-gate.test.ts @@ -0,0 +1,267 @@ +/** + * Adoption against the REAL `agentSessionPtyWriteGate`. + * + * Every other adoption test mocks the PTY controller, so `writeAgentSessionProof` answers whatever + * the mock says and `admitProof` never runs — which is precisely why they all passed while no user + * could ever switch a Codex tab to structured chat. Here the controller delegates to the real gate + * over a real record store, so the probe is admitted only when adoption has actually reserved the + * lease and bound the pane first. + */ + +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { randomUUID } from 'node:crypto' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createStructuredAgentSessionOperationId } from '../../shared/structured-agent-session-mutation' +import { StructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-host' +import { setStructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-registry' +import { AgentSessionRecordStore } from './agent-session-record-store' +import { agentSessionPtyWriteGate } from './agent-session-pty-write-gate' +import { createStructuredAgentSessionOwnerProbe } from './structured-agent-session-runtime' +import { OrcaRuntimeService } from './orca-runtime' + +const { readStructuredTuiProcessIdentity } = vi.hoisted(() => ({ + readStructuredTuiProcessIdentity: vi.fn() +})) +// The only OS-dependent step in the flow; the rollout proof and the write gate both stay real. +vi.mock('./structured-tui-process-identity', () => ({ readStructuredTuiProcessIdentity })) + +const WORKTREE_ID = 'repo-1::/tmp/structured-adoption' +const PTY_ID = 'pty-adopt' +const PANE_KEY = 'tab-adopt:leaf-adopt' +const THREAD_ID = '019fd532-7c11-7a90-b6de-4e1a2c3d5f60' +const SESSION_ID = `codex_${THREAD_ID.replaceAll('-', '_')}` + +type ProofRig = { + runtime: OrcaRuntimeService + store: AgentSessionRecordStore + host: StructuredAgentSessionHost + proofWrites: string[] + refusedWrites: string[] + codexHome: string +} + +let root: string +let rig: ProofRig + +async function writeRolloutFixture(codexHome: string, threadId: string): Promise { + const dir = join(codexHome, 'sessions', '2026', '08', '19') + await mkdir(dir, { recursive: true }) + await writeFile( + join(dir, `rollout-2026-08-19T10-00-00-${threadId}.jsonl`), + `${JSON.stringify({ type: 'session_meta', payload: { id: threadId } })}\n`, + 'utf8' + ) +} + +/** + * A PTY that answers `/status` with a Codex session banner — but only for bytes the real gate + * admitted. This mirrors `writeAgentSessionProof` in src/main/ipc/pty.ts verbatim. + */ +function installProofPty( + rigState: ProofRig, + threadId: string, + ptyRecord: { lastOutputAt: number } +) { + return (ptyId: string, data: string, authority: { sessionId: string; spawnToken: string }) => { + if (!agentSessionPtyWriteGate.admitProof(ptyId, authority)) { + rigState.refusedWrites.push(data) + return false + } + rigState.proofWrites.push(data) + if (data === '\r') { + const record = rigState.runtime as unknown as { + ptysById: Map + } + const pty = record.ptysById.get(PTY_ID)! + pty.tailBuffer = [`Session ID: ${threadId}`] + pty.lastOutputAt = ptyRecord.lastOutputAt + 1_000 + } + return true + } +} + +async function buildRig(): Promise { + const codexHome = join(root, 'codex-home') + await writeRolloutFixture(codexHome, THREAD_ID) + const store = await AgentSessionRecordStore.open({ + directory: join(root, 'agent-sessions'), + hostId: 'local' + }) + agentSessionPtyWriteGate.attachRecordLookup((sessionId) => store.getRecord(sessionId)) + const host = new StructuredAgentSessionHost({ + store, + adapter: { + acquire: vi.fn(), + dispatch: vi.fn(), + cancelTurn: vi.fn(), + answerPrompt: vi.fn(), + setOption: vi.fn() + }, + journalRoot: join(root, 'journals'), + claimKeyId: 'key-1', + probeOwner: createStructuredAgentSessionOwnerProbe( + 'local', + async () => ({ outcome: 'identity-matched', matchedOn: ['process-start-time'] }), + async () => [] + ) + }) + setStructuredAgentSessionHost(host) + + const runtime = new OrcaRuntimeService({ getSettings: () => ({ agentDefaultEnv: {} }) } as never) + const state: ProofRig = { runtime, store, host, proofWrites: [], refusedWrites: [], codexHome } + const ptyRecord = { + ptyId: PTY_ID, + connected: true, + connectionId: null, + wslDistro: null, + tabId: 'tab-adopt', + paneKey: PANE_KEY, + worktreeId: WORKTREE_ID, + incarnationId: 'inc-adopt', + tailBuffer: [] as string[], + tailPartialLine: '', + preview: '', + lastOutputAt: 1_000 + } + const internal = runtime as unknown as { + ptysById: Map + resolveRuntimeFileTarget(): Promise + resolveStructuredAgentSessionCreateIntent(input: { envelope: unknown }): Promise + issueStructuredTuiPtyHandle(): string + } + internal.ptysById.set(PTY_ID, ptyRecord) + internal.resolveRuntimeFileTarget = vi.fn(async () => ({ + connectionId: null, + worktree: { id: WORKTREE_ID } + })) + internal.resolveStructuredAgentSessionCreateIntent = vi.fn(async ({ envelope }) => ({ + envelope, + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: WORKTREE_ID, + workspaceKind: 'git-worktree' + }, + provider: 'codex', + agent: 'codex', + accountHome: { variable: 'CODEX_HOME', path: codexHome }, + runtimeKind: 'native' + })) + internal.issueStructuredTuiPtyHandle = vi.fn(() => 'term-adopt') + runtime.setPtyController({ + listProcesses: async () => [{ id: PTY_ID, incarnationId: 'inc-adopt', rootProcessId: 31337 }], + writeAgentSessionProof: installProofPty(state, THREAD_ID, ptyRecord) + } as never) + return state +} + +function adoptInput(overrides: { threadId?: string } = {}) { + return { + envelope: { + sessionId: SESSION_ID, + clientOperationId: createStructuredAgentSessionOperationId(() => randomUUID()), + expectedRuntimeFence: null as null, + payloadFingerprint: 'a'.repeat(64) + }, + worktree: `id:${WORKTREE_ID}`, + tabId: 'tab-adopt', + paneKey: PANE_KEY, + ptyId: PTY_ID, + ...overrides + } +} + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-adopt-gate-')) + readStructuredTuiProcessIdentity.mockImplementation( + async (input: { hostId: string; spawnToken: string }) => ({ + hostId: input.hostId, + pid: 4242, + processStartTimeMs: 1_700_000_000_000, + spawnToken: input.spawnToken + }) + ) + rig = await buildRig() +}) + +afterEach(async () => { + agentSessionPtyWriteGate.unbindPty(PTY_ID) + agentSessionPtyWriteGate.detachRecordLookup() + setStructuredAgentSessionHost(null) + await rig.host.flushAllStreamedEvents() + await rm(root, { recursive: true, force: true }) +}) + +describe('structured Codex adoption against the real PTY write gate', () => { + it('authorizes the provider probe, so switching a Codex tab to chat succeeds', async () => { + const result = await rig.runtime.adoptStructuredAgentSessionTerminal( + { ...adoptInput(), threadId: THREAD_ID }, + { callerKey: 'renderer-1' } + ) + + expect(result).toMatchObject({ ok: true }) + // The probe reached the pane: `/status`, submit, escape — none of them refused by the gate. + expect(rig.refusedWrites).toEqual([]) + expect(rig.proofWrites).toHaveLength(3) + expect(agentSessionPtyWriteGate.boundSessionId(PTY_ID)).toBe(SESSION_ID) + expect(rig.store.getRecord(SESSION_ID)).toMatchObject({ + lease: { runtimeKind: 'tui', claimStatus: 'live', handoffStage: null } + }) + }, 20_000) + + it('authorizes the probe when no thread id has been published yet', async () => { + 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 } }] + }) + }, 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 }) + + await expect( + rig.runtime.adoptStructuredAgentSessionTerminal( + { ...adoptInput(), threadId: THREAD_ID }, + { callerKey: 'renderer-1' } + ) + ).rejects.toThrow('did not prove the expected Codex rollout') + // The reservation is durably marked as never having spawned, and the pane is released. + expect(agentSessionPtyWriteGate.boundSessionId(PTY_ID)).toBeNull() + expect(rig.store.getRecord(SESSION_ID)?.lease).toMatchObject({ + claimStatus: 'reserved', + ownerProcess: null + }) + expect(rig.store.getRecord(SESSION_ID)?.lease.processlessAt).toEqual(expect.any(Number)) + + await writeRolloutFixture(rig.codexHome, THREAD_ID) + const retried = await rig.runtime.adoptStructuredAgentSessionTerminal( + { ...adoptInput(), threadId: THREAD_ID }, + { callerKey: 'renderer-1' } + ) + + expect(retried).toMatchObject({ ok: true }) + expect(rig.store.getRecord(SESSION_ID)).toMatchObject({ + lease: { claimStatus: 'live', runtimeKind: 'tui' } + }) + }, 30_000) + + it('refuses a pane another structured session already owns', async () => { + agentSessionPtyWriteGate.bindPty(PTY_ID, 'someone-elses-session') + + await expect( + rig.runtime.adoptStructuredAgentSessionTerminal( + { ...adoptInput(), threadId: THREAD_ID }, + { callerKey: 'renderer-1' } + ) + ).rejects.toThrow('already belongs to another structured Codex session') + expect(agentSessionPtyWriteGate.boundSessionId(PTY_ID)).toBe('someone-elses-session') + expect(rig.store.getRecord(SESSION_ID)).toBeNull() + }) +}) diff --git a/src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts b/src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts index d32752d83a2..6caf48a5f07 100644 --- a/src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts +++ b/src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts @@ -77,6 +77,20 @@ function terminalAdoptionHarness() { return runtime } +/** The host half adoption now needs: the lease is reserved before the provider probe runs. */ +function structuredHostDouble(overrides: Record = {}) { + return { + reserveAdoptedTuiOwner: vi.fn(async (input: { spawnToken: string }) => ({ + ok: true, + fence: 1, + spawnToken: input.spawnToken + })), + releaseAdoptedTuiReservation: vi.fn(async () => undefined), + adoptTuiOwner: vi.fn(async () => ({ ok: true, replayed: false, fence: 1 })), + ...overrides + } +} + function notifier(revealTerminalSession: ReturnType) { return { worktreesChanged: vi.fn(), @@ -105,7 +119,7 @@ describe('structured TUI launch tab binding', () => { it('adopts the proven local Codex pane as a durable TUI owner', async () => { const runtime = terminalAdoptionHarness() const adoptTuiOwner = vi.fn(async () => ({ ok: true, replayed: false, fence: 1 })) - setStructuredAgentSessionHost({ adoptTuiOwner } as never) + setStructuredAgentSessionHost(structuredHostDouble({ adoptTuiOwner }) as never) proveCodexTuiRollout.mockResolvedValueOnce({ transcriptPath: '/tmp/codex-home/rollout.jsonl' }) @@ -162,6 +176,8 @@ describe('structured TUI launch tab binding', () => { it('refuses adoption when the provider rollout cannot be proved', async () => { const runtime = terminalAdoptionHarness() + const host = structuredHostDouble() + setStructuredAgentSessionHost(host as never) proveCodexTuiRollout.mockRejectedValueOnce( new Error('The agent terminal did not prove the expected Codex rollout.') ) @@ -172,29 +188,37 @@ describe('structured TUI launch tab binding', () => { spawnToken: 'spawn-adopt' }) - await expect( - runtime.adoptStructuredAgentSessionTerminal( - { - envelope: { - sessionId: 'session-adopt', - clientOperationId: 'operation-adopt', - expectedRuntimeFence: null, - payloadFingerprint: 'f'.repeat(64) + try { + await expect( + runtime.adoptStructuredAgentSessionTerminal( + { + envelope: { + sessionId: 'session-adopt', + clientOperationId: 'operation-adopt', + expectedRuntimeFence: null, + payloadFingerprint: 'f'.repeat(64) + }, + worktree: `id:${WORKTREE_ID}`, + tabId: 'tab-adopt', + paneKey: 'tab-adopt:leaf-adopt', + ptyId: 'pty-adopt', + threadId: 'thread-adopt' }, - worktree: `id:${WORKTREE_ID}`, - tabId: 'tab-adopt', - paneKey: 'tab-adopt:leaf-adopt', - ptyId: 'pty-adopt', - threadId: 'thread-adopt' - }, - { callerKey: 'renderer-1' } - ) - ).rejects.toThrow('did not prove the expected Codex rollout') - expect(agentSessionPtyWriteGate.boundSessionId('pty-adopt')).toBeNull() + { callerKey: 'renderer-1' } + ) + ).rejects.toThrow('did not prove the expected Codex rollout') + expect(agentSessionPtyWriteGate.boundSessionId('pty-adopt')).toBeNull() + // A proof that never lands hands the reservation back; nothing latches the next attempt. + expect(host.releaseAdoptedTuiReservation).toHaveBeenCalledTimes(1) + } finally { + setStructuredAgentSessionHost(null) + } }) it('refuses a thread proof when the Codex process changes during it', async () => { const runtime = terminalAdoptionHarness() + const host = structuredHostDouble() + setStructuredAgentSessionHost(host as never) proveCodexTuiRollout.mockResolvedValueOnce({ transcriptPath: '/tmp/codex-home/rollout.jsonl' }) @@ -231,12 +255,14 @@ describe('structured TUI launch tab binding', () => { ) ).rejects.toThrow('process changed during conversation proof') expect(agentSessionPtyWriteGate.boundSessionId('pty-adopt')).toBeNull() + expect(host.releaseAdoptedTuiReservation).toHaveBeenCalledTimes(1) + setStructuredAgentSessionHost(null) }) it('discovers the provider thread when hook status has not published it yet', async () => { const runtime = terminalAdoptionHarness() const adoptTuiOwner = vi.fn(async () => ({ ok: true, replayed: false, fence: 1 })) - setStructuredAgentSessionHost({ adoptTuiOwner } as never) + setStructuredAgentSessionHost(structuredHostDouble({ adoptTuiOwner }) as never) resolveLiveCodexTuiRollout.mockResolvedValueOnce({ threadId: 'thread-discovered', transcriptPath: '/tmp/codex-home/discovered.jsonl' diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 3d68007fd5b..8399c60ba50 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -11465,7 +11465,82 @@ export class OrcaRuntimeService { throw new Error('Could not prove the Codex terminal process.') } const rootProcessId = listing.rootProcessId - const spawnToken = randomUUID() + const sessionId = input.envelope.sessionId + const priorBoundSessionId = agentSessionPtyWriteGate.boundSessionId(input.ptyId) + if (priorBoundSessionId !== null && priorBoundSessionId !== sessionId) { + throw new Error('This terminal pane already belongs to another structured Codex session.') + } + const host = getStructuredAgentSessionHost() + if (!host) { + throw new Error('structured_agent_session_unsupported') + } + const launchEnv = resolveTuiAgentLaunchEnv( + 'codex', + this.requireStore().getSettings().agentDefaultEnv + ) + // Why the reservation comes before the proof: `admitProof` authorizes the `/status` probe only + // for a reservation that already names this session, `tui`, and this spawn token. Reserving + // afterwards left the probe unauthorized, so every adoption failed at its first keystroke. + const reserved = await host.reserveAdoptedTuiOwner({ + caller, + sessionId, + clientOperationId: input.envelope.clientOperationId, + fingerprint: input.envelope.payloadFingerprint, + location: intent.location, + provider: intent.provider, + accountHome: intent.accountHome, + spawnToken: randomUUID(), + claimKeyId: this.agentSessionClaimSigner.keyId, + launchEnv + }) + if (!reserved.ok) { + return reserved + } + const { fence, spawnToken } = reserved + agentSessionPtyWriteGate.bindPty(input.ptyId, sessionId) + let owned = false + try { + return await this.attachAdoptedStructuredTui({ + input, + caller, + host, + intent, + launchEnv, + fence, + spawnToken, + rootProcessId, + pty, + onOwned: () => { + owned = true + } + }) + } finally { + if (!owned) { + // A failed proof must leave nothing behind: an unreleased reservation is exactly the + // latched lease that makes the next attempt refuse itself. + if (priorBoundSessionId === null) { + agentSessionPtyWriteGate.unbindPty(input.ptyId) + } + await host + .releaseAdoptedTuiReservation({ sessionId, fence, spawnToken }) + .catch(() => undefined) + } + } + } + + private async attachAdoptedStructuredTui(args: { + input: Parameters[0] + caller: StructuredAgentSessionCaller + host: NonNullable> + intent: AgentSessionAttachParams + launchEnv: Record + fence: number + spawnToken: string + rootProcessId: number + pty: RuntimePtyWorktreeRecord + onOwned: () => void + }): Promise> { + const { input, caller, host, intent, launchEnv, fence, spawnToken, rootProcessId, pty } = args const readPty = (): RuntimePtyWorktreeRecord => { const current = this.ptysById.get(input.ptyId) if (!current?.connected || current.paneKey !== input.paneKey) { @@ -11528,7 +11603,9 @@ export class OrcaRuntimeService { threadId, resumed: false, origin: 'adopted', - fence: 1, + // Why not 1: a retry supersedes the previous attempt's reservation, and `proveOwner` + // refuses a handle link minted at any fence but the reservation's own. + fence, observedAt: Date.now() }), transcriptPath, @@ -11556,14 +11633,6 @@ export class OrcaRuntimeService { runtimeKind: 'tui', providerHandle: { kind: 'codex', threadId } } - const launchEnv = resolveTuiAgentLaunchEnv( - 'codex', - this.requireStore().getSettings().agentDefaultEnv - ) - const host = getStructuredAgentSessionHost() - if (!host) { - throw new Error('structured_agent_session_unsupported') - } const adopted = await host.adoptTuiOwner({ caller, params, @@ -11572,8 +11641,9 @@ export class OrcaRuntimeService { launchEnv }) if (adopted.ok) { + // The pane stays bound to the reservation this call already made; only the owner is new. this.adoptedStructuredTuiOwners.set(input.envelope.sessionId, owner) - agentSessionPtyWriteGate.bindPty(input.ptyId, input.envelope.sessionId) + args.onOwned() } return adopted } diff --git a/src/renderer/src/store/slices/tab-view-mode-structured-route.test.ts b/src/renderer/src/store/slices/tab-view-mode-structured-route.test.ts new file mode 100644 index 00000000000..a6383bde7f4 --- /dev/null +++ b/src/renderer/src/store/slices/tab-view-mode-structured-route.test.ts @@ -0,0 +1,75 @@ +/** + * Both ways into structured chat go through one door. + * + * Right-click → "Switch to chat" and launching a Codex tab with open-in-chat both end at + * `setTabViewMode(tabId, 'chat')`, which hands a terminal tab to `setTerminalNativeChatMode` and + * from there to `agentSession.adoptTerminal`. Pinning that here is what makes "launch-at-chat + * shares the adoption defect" a checked claim rather than an inspection of the call graph. + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { AppState } from '../types' +import { createTestStore, makeUnifiedTab, makeTabGroup } from './store-test-helpers' +import type * as StructuredNativeChatToggle from './structured-native-chat-toggle' + +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) + +const { mockSetTerminalNativeChatMode } = vi.hoisted(() => ({ + mockSetTerminalNativeChatMode: vi.fn() +})) + +vi.mock('./structured-native-chat-toggle', async (importOriginal) => ({ + ...(await importOriginal()), + setTerminalNativeChatMode: mockSetTerminalNativeChatMode +})) + +const mockApi = { + ui: { set: vi.fn().mockResolvedValue(undefined) }, + settings: { get: vi.fn().mockResolvedValue({}), set: vi.fn().mockResolvedValue(undefined) } +} + +// @ts-expect-error -- partial window stub is sufficient for these store-only tests +globalThis.window = { api: mockApi } + +const WT = 'repo1::/tmp/feature' + +describe('structured chat route out of setTabViewMode', () => { + let store: ReturnType + + beforeEach(() => { + mockSetTerminalNativeChatMode.mockReset() + mockSetTerminalNativeChatMode.mockResolvedValue('structured') + store = createTestStore() + store.setState({ + unifiedTabsByWorktree: { + [WT]: [makeUnifiedTab({ id: 'codex-tab', worktreeId: WT, groupId: 'g-1' })] + }, + groupsByWorktree: { + [WT]: [ + makeTabGroup({ + id: 'g-1', + worktreeId: WT, + activeTabId: 'codex-tab', + tabOrder: ['codex-tab'] + }) + ] + } + } as Partial) + }) + + it('hands an explicit chat mode to the structured adoption path', () => { + store.getState().setTabViewMode('codex-tab', 'chat') + + expect(mockSetTerminalNativeChatMode).toHaveBeenCalledWith( + expect.objectContaining({ tabId: 'codex-tab', mode: 'chat' }) + ) + }) + + it('hands a toggled chat mode to the same path', () => { + store.getState().toggleTabViewMode('codex-tab') + + expect(mockSetTerminalNativeChatMode).toHaveBeenCalledWith( + expect.objectContaining({ tabId: 'codex-tab', mode: 'chat' }) + ) + }) +}) From 575660ffd8e4ba898f5f9d1b168b778ea3038fb8 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 19 Aug 2026 20:43:00 -0700 Subject: [PATCH 2/3] fix(native-chat): let the reservation holder win an overlapping adoption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two adoptions of the same Codex pane destroyed each other. The pane binding was released by pty id alone, gated on a `priorBoundSessionId` sampled before the reservation, and both attempts carry the same sessionId — so the attempt whose reservation had been superseded unbound the pane the winner was proving on, `admitProof` refused the winner's `/status` keystroke, and both failed. The write gate now records which attempt holds a pane, keyed on the spawn token the reservation already mints: a claim supersedes another attempt's but never a settled owner's, the release is a compare-and-clear, and a proven owner settles the binding so no later attempt can take it. No lock is needed; the reservation release beside it was already fence-scoped and unchanged. A failed `releaseAdoptedTuiReservation` stays swallowed — the proof error is the one the user must act on — but is now logged with session, fence and spawn token, because a release that never persisted leaves the lease latched and refuses every later attempt with no record of why. Adoption's no-record branch is deleted. The only caller reserves the lease first and nothing deletes a record, so it minted an owner for a session the store had never heard of in unit tests only; its test now drives the real reserve-then-adopt sequence, and a new one pins the refusal. Launch-at-chat discarded `waitForAgentReady`'s result, so a readiness timeout still flipped the tab to chat and adoption then probed a shell. It stays in terminal view instead: the terminal is usable and the view toggle re-runs adoption whenever the user wants it. Each fix was reverted against its new test to confirm the test reddens; all four defects had been living under green suites. --- ...uctured-agent-session-tui-adoption.test.ts | 42 +++++++++--- .../structured-agent-session-tui-adoption.ts | 39 +++-------- .../agent-session-pty-write-gate.test.ts | 42 ++++++++++++ .../runtime/agent-session-pty-write-gate.ts | 65 ++++++++++++++++--- ...structured-tui-adoption-write-gate.test.ts | 46 +++++++++++++ ...runtime-structured-tui-tab-binding.test.ts | 57 ++++++++++++++++ src/main/runtime/orca-runtime.ts | 38 ++++++----- .../src/lib/launch-agent-in-new-tab.ts | 12 +++- ...launch-agent-structured-chat-guard.test.ts | 15 +++++ 9 files changed, 291 insertions(+), 65 deletions(-) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-tui-adoption.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-tui-adoption.test.ts index 82fb20e6f15..f0eaf49da54 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-tui-adoption.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-tui-adoption.test.ts @@ -79,15 +79,34 @@ afterEach(async () => { await rm(root, { recursive: true, force: true }) }) +/** What the runtime does before it may type `/status` into the pane it is adopting. */ +async function reserveAdoption(attach: AgentSessionAttachParams, spawnToken: string) { + return host.reserveAdoptedTuiOwner({ + caller: CALLER, + sessionId: attach.envelope.sessionId, + clientOperationId: attach.envelope.clientOperationId, + fingerprint: attach.envelope.payloadFingerprint, + location: attach.location, + provider: 'codex', + accountHome: attach.accountHome, + spawnToken, + claimKeyId: 'key-1' + }) +} + describe('structured TUI adoption', () => { it('mints a durable TUI owner without starting a native provider', async () => { const adoptedOwner = owner() + const attach = params() + await expect(reserveAdoption(attach, adoptedOwner.process.spawnToken)).resolves.toMatchObject({ + ok: true, + fence: 1 + }) const result = await host.adoptTuiOwner({ caller: CALLER, - params: params(), - owner: adoptedOwner, - claimKeyId: 'key-1' + params: attach, + owner: adoptedOwner }) expect(result).toMatchObject({ ok: true, replayed: false, fence: 1 }) @@ -107,12 +126,7 @@ describe('structured TUI adoption', () => { const invalid = params() invalid.envelope.payloadFingerprint = '0'.repeat(64) - const result = await host.adoptTuiOwner({ - caller: CALLER, - params: invalid, - owner: owner(), - claimKeyId: 'key-1' - }) + const result = await host.adoptTuiOwner({ caller: CALLER, params: invalid, owner: owner() }) expect(result).toMatchObject({ ok: false, @@ -121,4 +135,14 @@ describe('structured TUI adoption', () => { expect(store.getRecord(SESSION)).toBeNull() expect(acquire).not.toHaveBeenCalled() }) + + // The reservation is the only way in: adoption has no second mode that acquires the lease + // itself, so nothing here may invent an owner for a session the store has never heard of. + it('refuses to adopt a session that holds no reservation', async () => { + await expect( + host.adoptTuiOwner({ caller: CALLER, params: params(), owner: owner() }) + ).rejects.toThrow('agent_session_ownership_unknown') + expect(store.getRecord(SESSION)).toBeNull() + expect(acquire).not.toHaveBeenCalled() + }) }) 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 139577724e7..643142751f7 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 @@ -5,11 +5,7 @@ import type { AgentSessionMutationResult } from '../../../shared/agent-session-wire' import type { AgentSessionAttachParams } from './structured-agent-session-attach' -import { - admitAttachOrRefuse, - attachJournal, - reserveRequestFor -} from './structured-agent-session-attach' +import { admitAttachOrRefuse, attachJournal } from './structured-agent-session-attach' import type { StructuredAgentSessionCaller, StructuredAgentSessionHostDeps, @@ -22,8 +18,6 @@ export type StructuredTuiAdoptionRequest = { caller: StructuredAgentSessionCaller params: AgentSessionAttachParams owner: StructuredTuiOwner - claimKeyId: string - launchEnv?: Record } export function adoptStructuredTuiOwner( @@ -48,32 +42,19 @@ async function adopt( return admitted } let record = input.deps.store.getRecord(sessionId) - let replayed = record !== null - if (record && isStructuredTuiAdoptionReservation(record, owner.process.spawnToken)) { + if (!record) { + // Adoption reserves the lease before it may prove the pane, and nothing deletes a record, so + // this call always runs against one. Refusing beats reserving here: a second acquisition mode + // that no caller can enter is coverage that proves nothing. + throw new Error('agent_session_ownership_unknown') + } + let replayed = true + if (isStructuredTuiAdoptionReservation(record, owner.process.spawnToken)) { // This adoption's own pre-proof reservation: turn it into an owner rather than reserve again. replayed = false record = await proveAdoptedTuiOwner(input, record) - } else if (record) { - assertMatchingTuiOwner(record, owner) } else { - const reserved = await input.deps.store.reserveOwner( - reserveRequestFor({ - sessionId, - params, - authority: { - spawnToken: owner.process.spawnToken, - claimKeyId: input.claimKeyId, - handoffOperationId: params.envelope.clientOperationId, - probe: { outcome: 'reservation-unused' }, - ...(input.launchEnv ? { launchEnv: input.launchEnv } : {}) - }, - callerKey: input.caller.callerKey, - fingerprint: admitted.fingerprint, - now: input.now() - }) - ) - replayed = reserved.disposition === 'replayed' - record = await proveAdoptedTuiOwner(input, reserved.record) + assertMatchingTuiOwner(record, owner) } const attached = await attachJournal({ record, diff --git a/src/main/runtime/agent-session-pty-write-gate.test.ts b/src/main/runtime/agent-session-pty-write-gate.test.ts index 7d3ef8e6576..94ad79c051b 100644 --- a/src/main/runtime/agent-session-pty-write-gate.test.ts +++ b/src/main/runtime/agent-session-pty-write-gate.test.ts @@ -76,6 +76,48 @@ describe('binding lifecycle', () => { }) }) +describe('overlapping adoption attempts on one pane', () => { + beforeEach(() => { + gate.attachRecordLookup((sessionId) => records.get(sessionId) ?? null) + }) + + it('keeps the newer attempt bound when the one it superseded gives up', () => { + gate.bindPtyForAttempt(PTY_ID, SESSION_ID, 'spawn-a') + expect(gate.bindPtyForAttempt(PTY_ID, SESSION_ID, 'spawn-b')).toBe(true) + + // Both attempts carry the same session, so only the spawn token can tell this release apart. + expect(gate.releasePtyAttempt(PTY_ID, 'spawn-a')).toBe(false) + expect(gate.boundSessionId(PTY_ID)).toBe(SESSION_ID) + expect(gate.releasePtyAttempt(PTY_ID, 'spawn-b')).toBe(true) + expect(gate.boundSessionId(PTY_ID)).toBeNull() + }) + + it('leaves a settled owner pane alone when a later attempt fails', () => { + gate.bindPty(PTY_ID, SESSION_ID) + + expect(gate.bindPtyForAttempt(PTY_ID, SESSION_ID, 'spawn-late')).toBe(false) + expect(gate.releasePtyAttempt(PTY_ID, 'spawn-late')).toBe(false) + expect(gate.boundSessionId(PTY_ID)).toBe(SESSION_ID) + }) + + it('settles a proven attempt so no later attempt can release its pane', () => { + gate.bindPtyForAttempt(PTY_ID, SESSION_ID, 'spawn-a') + expect(gate.settlePtyAttempt(PTY_ID, 'spawn-a')).toBe(true) + + expect(gate.settlePtyAttempt(PTY_ID, 'spawn-a')).toBe(false) + expect(gate.releasePtyAttempt(PTY_ID, 'spawn-a')).toBe(false) + expect(gate.boundSessionId(PTY_ID)).toBe(SESSION_ID) + }) + + it('settles nothing once the pane is gone', () => { + gate.bindPtyForAttempt(PTY_ID, SESSION_ID, 'spawn-a') + gate.unbindPty(PTY_ID) + + expect(gate.settlePtyAttempt(PTY_ID, 'spawn-a')).toBe(false) + expect(gate.boundSessionId(PTY_ID)).toBeNull() + }) +}) + describe('admission through the store', () => { beforeEach(() => { gate.attachRecordLookup((sessionId) => records.get(sessionId) ?? null) diff --git a/src/main/runtime/agent-session-pty-write-gate.ts b/src/main/runtime/agent-session-pty-write-gate.ts index f252e6f938b..8fb078eb3b9 100644 --- a/src/main/runtime/agent-session-pty-write-gate.ts +++ b/src/main/runtime/agent-session-pty-write-gate.ts @@ -31,8 +31,17 @@ const ADMITTED_UNBOUND: AgentSessionPtyWriteAdmission = { runtimeFence: null } +/** + * A pane binding, plus the adoption attempt that is still proving it. + * + * `attemptToken` is null once an owner is proven — a settled binding no attempt may take away. + * While it is non-null the binding belongs to that one in-flight attempt, which is the only thing + * that tells two overlapping adoptions apart: both carry the same sessionId. + */ +type BoundPane = { sessionId: string; attemptToken: string | null } + export class AgentSessionPtyWriteGate { - private readonly sessionIdByPtyId = new Map() + private readonly panesByPtyId = new Map() private lookup: AgentSessionRecordLookup | null = null /** Point the gate at the durable store. Until this is called nothing can be enforced. */ @@ -42,24 +51,62 @@ export class AgentSessionPtyWriteGate { detachRecordLookup(): void { this.lookup = null - this.sessionIdByPtyId.clear() + this.panesByPtyId.clear() } bindPty(ptyId: string, sessionId: string): void { - this.sessionIdByPtyId.set(ptyId, sessionId) + this.panesByPtyId.set(ptyId, { sessionId, attemptToken: null }) } unbindPty(ptyId: string): void { - this.sessionIdByPtyId.delete(ptyId) + this.panesByPtyId.delete(ptyId) + } + + /** + * Claim the pane for one adoption attempt, identified by the spawn token its reservation minted. + * A settled owner keeps the pane; another attempt's claim is superseded, because the newest + * reservation is the one the record now names. Callers must already have refused a pane bound to + * a different session. + */ + bindPtyForAttempt(ptyId: string, sessionId: string, attemptToken: string): boolean { + const current = this.panesByPtyId.get(ptyId) + if (current && current.attemptToken === null) { + return false + } + this.panesByPtyId.set(ptyId, { sessionId, attemptToken }) + return true + } + + /** Promote this attempt's claim to a settled binding once its owner is proven. */ + settlePtyAttempt(ptyId: string, attemptToken: string): boolean { + const current = this.panesByPtyId.get(ptyId) + if (current?.attemptToken !== attemptToken) { + return false + } + this.panesByPtyId.set(ptyId, { sessionId: current.sessionId, attemptToken: null }) + return true + } + + /** + * Compare-and-clear: hand the pane back only while this attempt still holds it. A losing attempt + * that unbinds by pty alone rips the pane out from under the attempt that superseded it, and then + * that one's proof is refused too — both fail where one should have won. + */ + releasePtyAttempt(ptyId: string, attemptToken: string): boolean { + if (this.panesByPtyId.get(ptyId)?.attemptToken !== attemptToken) { + return false + } + this.panesByPtyId.delete(ptyId) + return true } boundSessionId(ptyId: string): string | null { - return this.sessionIdByPtyId.get(ptyId) ?? null + return this.panesByPtyId.get(ptyId)?.sessionId ?? null } /** False while no PTY is bound, which is every write path in today's builds. */ get enforcing(): boolean { - return this.lookup !== null && this.sessionIdByPtyId.size > 0 + return this.lookup !== null && this.panesByPtyId.size > 0 } admit(ptyId: string): AgentSessionPtyWriteAdmission { @@ -117,11 +164,11 @@ export class AgentSessionPtyWriteGate { } private binding(ptyId: string): AgentSessionPtyBinding | null { - const sessionId = this.sessionIdByPtyId.get(ptyId) - if (sessionId === undefined) { + const pane = this.panesByPtyId.get(ptyId) + if (pane === undefined) { return null } - return { sessionId, record: this.lookup?.(sessionId) ?? null } + return { sessionId: pane.sessionId, record: this.lookup?.(pane.sessionId) ?? null } } } 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 efb231c8818..861cd434f0e 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 @@ -252,6 +252,52 @@ describe('structured Codex adoption against the real PTY write gate', () => { }) }, 30_000) + it('lets the attempt holding the reservation win when two adoptions overlap', async () => { + // Both attempts park here: reservation taken and pane bound, proof not yet written. + const arrived: string[] = [] + let releaseBoth: () => void = () => {} + const bothArrived = new Promise((resolve) => { + releaseBoth = resolve + }) + readStructuredTuiProcessIdentity.mockImplementation( + async (input: { hostId: string; spawnToken: string }) => { + if (!arrived.includes(input.spawnToken)) { + arrived.push(input.spawnToken) + if (arrived.length === 2) { + releaseBoth() + } + await bothArrived + } + return { + hostId: input.hostId, + pid: 4242, + processStartTimeMs: 1_700_000_000_000, + spawnToken: input.spawnToken + } + } + ) + + const superseded = rig.runtime.adoptStructuredAgentSessionTerminal( + { ...adoptInput(), threadId: THREAD_ID }, + { callerKey: 'renderer-1' } + ) + superseded.catch(() => undefined) + await vi.waitFor(() => expect(arrived).toHaveLength(1)) + // The second attempt carries the same sessionId and supersedes the first one's reservation. + const winner = rig.runtime.adoptStructuredAgentSessionTerminal( + { ...adoptInput(), threadId: THREAD_ID }, + { callerKey: 'renderer-1' } + ) + + await expect(winner).resolves.toMatchObject({ ok: true }) + await expect(superseded).rejects.toThrow('could not verify its Codex session') + // The loser's cleanup ran against a pane and a reservation that were no longer its own. + expect(agentSessionPtyWriteGate.boundSessionId(PTY_ID)).toBe(SESSION_ID) + expect(rig.store.getRecord(SESSION_ID)).toMatchObject({ + lease: { runtimeKind: 'tui', claimStatus: 'live', handoffStage: null } + }) + }, 30_000) + it('refuses a pane another structured session already owns', async () => { agentSessionPtyWriteGate.bindPty(PTY_ID, 'someone-elses-session') diff --git a/src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts b/src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts index 6caf48a5f07..778c9154447 100644 --- a/src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts +++ b/src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts @@ -215,6 +215,63 @@ describe('structured TUI launch tab binding', () => { } }) + it('reports a reservation release that fails, without masking the proof error', async () => { + const runtime = terminalAdoptionHarness() + const host = structuredHostDouble({ + releaseAdoptedTuiReservation: vi.fn(async () => { + throw new Error('store write failed') + }) + }) + setStructuredAgentSessionHost(host as never) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + proveCodexTuiRollout.mockRejectedValueOnce( + new Error('The agent terminal did not prove the expected Codex rollout.') + ) + readStructuredTuiProcessIdentity.mockResolvedValueOnce({ + hostId: 'local', + pid: 4242, + processStartTimeMs: 1_700_000_000_000, + spawnToken: 'spawn-adopt' + }) + + try { + await expect( + runtime.adoptStructuredAgentSessionTerminal( + { + envelope: { + sessionId: 'session-release-failure', + clientOperationId: 'operation-release-failure', + expectedRuntimeFence: null, + payloadFingerprint: 'f'.repeat(64) + }, + worktree: `id:${WORKTREE_ID}`, + tabId: 'tab-adopt', + paneKey: 'tab-adopt:leaf-adopt', + ptyId: 'pty-adopt', + threadId: 'thread-adopt' + }, + { callerKey: 'renderer-1' } + ) + ).rejects.toThrow('did not prove the expected Codex rollout') + + const reserved = host.reserveAdoptedTuiOwner.mock.calls[0]?.[0] as { spawnToken: string } + const logged = warn.mock.calls.find( + ([message]) => message === '[native-chat] adopted TUI reservation release failed' + )?.[1] as { error: Error } | undefined + // A latched lease refuses every later attempt, so the cause cannot vanish with the throw. + expect(logged).toMatchObject({ + sessionId: 'session-release-failure', + fence: 1, + spawnToken: reserved.spawnToken + }) + expect(logged?.error.message).toBe('store write failed') + } finally { + warn.mockRestore() + setStructuredAgentSessionHost(null) + agentSessionPtyWriteGate.unbindPty('pty-adopt') + } + }) + it('refuses a thread proof when the Codex process changes during it', async () => { const runtime = terminalAdoptionHarness() const host = structuredHostDouble() diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 8399c60ba50..524ce8c5585 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -11497,7 +11497,7 @@ export class OrcaRuntimeService { return reserved } const { fence, spawnToken } = reserved - agentSessionPtyWriteGate.bindPty(input.ptyId, sessionId) + agentSessionPtyWriteGate.bindPtyForAttempt(input.ptyId, sessionId, spawnToken) let owned = false try { return await this.attachAdoptedStructuredTui({ @@ -11505,7 +11505,6 @@ export class OrcaRuntimeService { caller, host, intent, - launchEnv, fence, spawnToken, rootProcessId, @@ -11515,15 +11514,27 @@ export class OrcaRuntimeService { } }) } finally { - if (!owned) { + if (owned) { + agentSessionPtyWriteGate.settlePtyAttempt(input.ptyId, spawnToken) + } else { // A failed proof must leave nothing behind: an unreleased reservation is exactly the - // latched lease that makes the next attempt refuse itself. - if (priorBoundSessionId === null) { - agentSessionPtyWriteGate.unbindPty(input.ptyId) - } + // latched lease that makes the next attempt refuse itself. Both releases are scoped to this + // attempt's own spawn token, because a second adoption of the same pane carries the same + // sessionId — clearing by pty alone unbinds the pane the attempt that superseded us is + // proving on, and then neither can finish. + agentSessionPtyWriteGate.releasePtyAttempt(input.ptyId, spawnToken) await host .releaseAdoptedTuiReservation({ sessionId, fence, spawnToken }) - .catch(() => undefined) + .catch((error: unknown) => { + // Swallowed on purpose: the proof error below is the one the user must see. But a + // release that never persisted leaves the lease latched, so it cannot also be silent. + console.warn('[native-chat] adopted TUI reservation release failed', { + sessionId, + fence, + spawnToken, + error + }) + }) } } } @@ -11533,14 +11544,13 @@ export class OrcaRuntimeService { caller: StructuredAgentSessionCaller host: NonNullable> intent: AgentSessionAttachParams - launchEnv: Record fence: number spawnToken: string rootProcessId: number pty: RuntimePtyWorktreeRecord onOwned: () => void }): Promise> { - const { input, caller, host, intent, launchEnv, fence, spawnToken, rootProcessId, pty } = args + const { input, caller, host, intent, fence, spawnToken, rootProcessId, pty } = args const readPty = (): RuntimePtyWorktreeRecord => { const current = this.ptysById.get(input.ptyId) if (!current?.connected || current.paneKey !== input.paneKey) { @@ -11633,13 +11643,7 @@ export class OrcaRuntimeService { runtimeKind: 'tui', providerHandle: { kind: 'codex', threadId } } - const adopted = await host.adoptTuiOwner({ - caller, - params, - owner, - claimKeyId: this.agentSessionClaimSigner.keyId, - launchEnv - }) + const adopted = await host.adoptTuiOwner({ caller, params, owner }) if (adopted.ok) { // The pane stays bound to the reservation this call already made; only the owner is new. this.adoptedStructuredTuiOwners.set(input.envelope.sessionId, owner) 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 0a81d4261a5..8573079003f 100644 --- a/src/renderer/src/lib/launch-agent-in-new-tab.ts +++ b/src/renderer/src/lib/launch-agent-in-new-tab.ts @@ -259,7 +259,17 @@ export function launchAgentInNewTab(args: LaunchAgentInNewTabArgs): LaunchAgentI void (promptDeliveryCompletion ?? Promise.resolve()) .catch(() => undefined) .then(async () => { - await waitForAgentReady(tab.id, TUI_AGENT_CONFIG.codex.expectedProcess) + const ready = await waitForAgentReady(tab.id, TUI_AGENT_CONFIG.codex.expectedProcess) + if (!ready.ready) { + // Why: chat adoption proves the pane's foreground process. Flipping a tab whose Codex is + // not up yet trades a working terminal for an adoption error; the view toggle re-runs + // adoption whenever the user wants it. + console.warn('Codex was not ready for structured chat; staying in terminal view', { + tabId: tab.id, + reason: ready.reason + }) + return + } useAppStore.getState().setTabViewMode(tab.id, 'chat') }) } 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 a7c84006d86..4fcaec8a8a2 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 @@ -81,6 +81,21 @@ describe('structured chat adoption guard on the launch path', () => { await vi.waitFor(() => expect(mockSetTabViewMode).toHaveBeenCalledWith('tab-1', 'chat')) }) + 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' }) + + await vi.waitFor(() => expect(mockWaitForAgentReady).toHaveBeenCalled()) + // Let every continuation the ready wait could have queued run before asserting it did not flip. + await new Promise((resolve) => setTimeout(resolve, 0)) + await new Promise((resolve) => setTimeout(resolve, 0)) + // Chat adoption probes the pane's foreground process: flipping now hands the user an adoption + // error in place of the terminal Codex is still starting in. + expect(mockSetTabViewMode).not.toHaveBeenCalledWith('tab-1', 'chat') + }) + it('keeps an SSH Codex tab on the bridge', async () => { store.repos = [{ id: 'repo-1', connectionId: 'ssh-a', path: '/repo' }] const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab') From c69922bc40a9a1a4f2462f7a66fee8ae378ff2d7 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 19 Aug 2026 21:09:27 -0700 Subject: [PATCH 3/3] fix(native-chat): adopt a Codex pane on its own account home MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Structured adoption reused the CREATE intent, whose account home is derived purely from the current selection. The pane's Codex is already running under the CODEX_HOME it was spawned with — and an automatic resume deliberately pins that to the home that OWNS the session — so adopting a pane on any other managed account probed the wrong sessions directory and failed as "did not prove the expected Codex rollout", with no hint that the account mismatched. Adoption now resolves the home from the pane's own launch record and declines, naming the account, when the pane cannot be attributed. Create is unchanged. Hooks skipped: lint-staged stashes, and the stash stack is shared across every worktree on this machine. oxfmt, oxlint, the native code-quality audit, the max-lines ratchet, tsc and the runtime/codex suites were all run by hand. --- src/main/codex/codex-pane-launch-home.test.ts | 95 ++++++ src/main/codex/codex-pane-launch-home.ts | 78 +++++ ...ructured-tui-adoption-account-home.test.ts | 299 ++++++++++++++++++ ...structured-tui-adoption-write-gate.test.ts | 7 +- ...runtime-structured-tui-tab-binding.test.ts | 7 +- src/main/runtime/orca-runtime.ts | 75 ++++- 6 files changed, 546 insertions(+), 15 deletions(-) create mode 100644 src/main/codex/codex-pane-launch-home.test.ts create mode 100644 src/main/codex/codex-pane-launch-home.ts create mode 100644 src/main/runtime/orca-runtime-structured-tui-adoption-account-home.test.ts diff --git a/src/main/codex/codex-pane-launch-home.test.ts b/src/main/codex/codex-pane-launch-home.test.ts new file mode 100644 index 00000000000..d4c1847eb32 --- /dev/null +++ b/src/main/codex/codex-pane-launch-home.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from 'vitest' +import type { CodexPaneAccountRecord } from './codex-pane-account-registry' +import { + describeCodexPaneLaunchHomeFailure, + resolveCodexPaneLaunchHome +} from './codex-pane-launch-home' + +const SYSTEM_HOME = '/Users/dev/.codex' +const SHARED_HOME = '/Users/dev/Library/Application Support/orca/codex-runtime-home/home' +const ACCOUNT_HOME = '/Users/dev/Library/Application Support/orca/codex-accounts/acct-1/home' + +const SETTINGS = { + codexManagedAccounts: [ + { + id: 'acct-1', + email: 'dev@example.com', + managedHomePath: ACCOUNT_HOME, + createdAt: 1, + updatedAt: 1, + lastAuthenticatedAt: 1 + } + ] +} + +function resolve(record: CodexPaneAccountRecord | null) { + return resolveCodexPaneLaunchHome({ + record, + settings: SETTINGS, + systemCodexHomePath: SYSTEM_HOME, + sharedRuntimeCodexHomePath: SHARED_HOME + }) +} + +describe('resolveCodexPaneLaunchHome', () => { + it('names the account home a managed pane launched under', () => { + expect( + resolve({ selectionKey: 'host', accountId: 'acct-1', homeRoute: 'account-home' }) + ).toEqual({ kind: 'attributed', path: ACCOUNT_HOME }) + }) + + it('routes real-home and shared-home panes to their own homes', () => { + expect(resolve({ selectionKey: 'host', accountId: null, homeRoute: 'real-home' })).toEqual({ + kind: 'attributed', + path: SYSTEM_HOME + }) + expect(resolve({ selectionKey: 'host', accountId: null, homeRoute: 'shared-home' })).toEqual({ + kind: 'attributed', + path: SHARED_HOME + }) + }) + + it('prefers a recorded pane-local override over the route it was classified as', () => { + expect( + resolve({ + selectionKey: 'host', + accountId: null, + homeRoute: 'real-home', + environmentHomeOverride: { codexHome: '/tmp/isolated/home' } + }) + ).toEqual({ kind: 'attributed', path: '/tmp/isolated/home' }) + expect( + resolve({ + selectionKey: 'host', + accountId: null, + homeRoute: 'shared-home', + shellStartupHomeOverride: { home: '/Users/dev', codexHome: '/tmp/shell/home' } + }) + ).toEqual({ kind: 'attributed', path: '/tmp/shell/home' }) + }) + + it('declines rather than guessing when the pane cannot be attributed', () => { + expect(resolve(null)).toEqual({ kind: 'unrecorded' }) + // A WSL lane can never answer for the local pane adoption asks about. + expect(resolve({ selectionKey: 'wsl:Ubuntu', accountId: null, homeRoute: 'wsl-home' })).toEqual( + { kind: 'unrecorded' } + ) + // Recorded before route provenance existed, so the launch home is unknown. + expect(resolve({ selectionKey: 'host', accountId: null })).toEqual({ kind: 'unnameable-home' }) + expect(resolve({ selectionKey: 'host', accountId: null, homeRoute: 'custom-home' })).toEqual({ + kind: 'unnameable-home' + }) + expect( + resolve({ selectionKey: 'host', accountId: 'acct-gone', homeRoute: 'account-home' }) + ).toEqual({ kind: 'unknown-account', accountId: 'acct-gone' }) + }) + + it('names the account in the failure the user is shown', () => { + expect( + describeCodexPaneLaunchHomeFailure({ kind: 'unknown-account', accountId: 'acct-gone' }) + ).toContain('acct-gone') + expect(describeCodexPaneLaunchHomeFailure({ kind: 'unrecorded' })).toContain( + 'which Codex account' + ) + }) +}) diff --git a/src/main/codex/codex-pane-launch-home.ts b/src/main/codex/codex-pane-launch-home.ts new file mode 100644 index 00000000000..56600a653a5 --- /dev/null +++ b/src/main/codex/codex-pane-launch-home.ts @@ -0,0 +1,78 @@ +import type { GlobalSettings } from '../../shared/global-settings-types' +import { getCodexSelectionTargetForAccount } from '../codex-accounts/runtime-selection' +import type { CodexPaneAccountRecord } from './codex-pane-account-registry' + +type CodexPaneLaunchHomeSettings = Pick + +export type CodexPaneLaunchHomeResolution = + | { kind: 'attributed'; path: string } + | { kind: 'unrecorded' } + | { kind: 'unknown-account'; accountId: string } + | { kind: 'unnameable-home' } + +/** + * Resolves the CODEX_HOME a live host pane's Codex is ALREADY running under. + * + * Why this is not the selected account's home: `CODEX_HOME` is baked into a + * shell's environment at spawn, and an automatic resume deliberately pins it to + * the home that OWNS the session rather than to the current selection. Only that + * home's `sessions` directory can hold the pane's rollout, so anything reading + * the live pane's transcript must ask which account it launched under — the + * record read here is the one `resolveCodexPaneLaunchAccount` wrote at spawn. + * + * Declines rather than guessing: probing the wrong account's home reports a + * broken session when the real cause is an account mismatch. + */ +export function resolveCodexPaneLaunchHome(args: { + record: CodexPaneAccountRecord | null + settings: CodexPaneLaunchHomeSettings + systemCodexHomePath: string + sharedRuntimeCodexHomePath: string +}): CodexPaneLaunchHomeResolution { + const record = args.record + if (!record || record.selectionKey !== 'host') { + return { kind: 'unrecorded' } + } + // Pane-local overrides carry the literal CODEX_HOME the launch resolved, so + // they answer before the route, which only classifies it. + const overrideHome = + record.environmentHomeOverride?.codexHome ?? record.shellStartupHomeOverride?.codexHome + if (overrideHome) { + return { kind: 'attributed', path: overrideHome } + } + if (record.homeRoute === 'real-home') { + return { kind: 'attributed', path: args.systemCodexHomePath } + } + if (record.homeRoute === 'shared-home') { + return { kind: 'attributed', path: args.sharedRuntimeCodexHomePath } + } + if (record.homeRoute !== 'account-home') { + // `custom-home` recorded a home Orca cannot re-derive; an absent route predates + // provenance; `wsl-home` cannot belong to a host pane. + return { kind: 'unnameable-home' } + } + if (!record.accountId) { + return { kind: 'unnameable-home' } + } + const account = args.settings.codexManagedAccounts?.find( + (candidate) => + candidate.id === record.accountId && + getCodexSelectionTargetForAccount(candidate).runtime === 'host' + ) + return account + ? { kind: 'attributed', path: account.managedHomePath } + : { kind: 'unknown-account', accountId: record.accountId } +} + +/** Names the account cause, so a mismatch never reads as a broken session. */ +export function describeCodexPaneLaunchHomeFailure( + resolution: Exclude +): string { + if (resolution.kind === 'unknown-account') { + return `This terminal runs Codex under managed account ${resolution.accountId}, which is no longer configured, so its Codex session cannot be located.` + } + if (resolution.kind === 'unnameable-home') { + return 'This terminal runs Codex under a CODEX_HOME Orca cannot identify, so its Codex session cannot be located.' + } + return 'Orca has no record of which Codex account this terminal launched under, so its Codex session cannot be located. Start a new Codex chat instead.' +} diff --git a/src/main/runtime/orca-runtime-structured-tui-adoption-account-home.test.ts b/src/main/runtime/orca-runtime-structured-tui-adoption-account-home.test.ts new file mode 100644 index 00000000000..8fa66d3c903 --- /dev/null +++ b/src/main/runtime/orca-runtime-structured-tui-adoption-account-home.test.ts @@ -0,0 +1,299 @@ +/** + * Adoption against the REAL account-home resolution. + * + * Why this rig exists: every other adoption test stubs the intent resolver with a hardcoded + * `accountHome`, so none of them could see that adoption resolved the CURRENTLY SELECTED Codex + * account's home instead of the one the pane's Codex is actually running under. Here the pane's + * home comes from the real pane-account registry and the real settings, and the rollout fixture + * exists ONLY under the pane's home — so probing the selected account's home fails the proof. + */ + +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { randomUUID } from 'node:crypto' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createStructuredAgentSessionOperationId } from '../../shared/structured-agent-session-mutation' +import { StructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-host' +import { setStructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-registry' +import { + _internals as codexPaneAccountRegistryInternals, + type CodexPaneAccountRecord +} from '../codex/codex-pane-account-registry' +import { AgentSessionRecordStore } from './agent-session-record-store' +import { agentSessionPtyWriteGate } from './agent-session-pty-write-gate' +import { createStructuredAgentSessionOwnerProbe } from './structured-agent-session-runtime' +import { OrcaRuntimeService } from './orca-runtime' + +const { readStructuredTuiProcessIdentity } = vi.hoisted(() => ({ + readStructuredTuiProcessIdentity: vi.fn() +})) +// The only OS-dependent step in the flow; the rollout proof and the account-home resolution stay real. +vi.mock('./structured-tui-process-identity', () => ({ readStructuredTuiProcessIdentity })) + +const WORKTREE_ID = 'repo-1::/tmp/structured-adoption-home' +const PTY_ID = 'pty-adopt' +const PANE_KEY = 'tab-adopt:leaf-adopt' +const THREAD_ID = '019fd532-7c11-7a90-b6de-4e1a2c3d5f61' +const SESSION_ID = `codex_${THREAD_ID.replaceAll('-', '_')}` +const PANE_ACCOUNT_ID = 'codex-account-pane' +const SELECTED_ACCOUNT_ID = 'codex-account-selected' + +type AdoptionRig = { + runtime: OrcaRuntimeService + store: AgentSessionRecordStore + host: StructuredAgentSessionHost + proofWrites: string[] +} + +let root: string +let paneAccountHome: string +let selectedAccountHome: string +let rig: AdoptionRig + +async function writeRolloutFixture(codexHome: string, threadId: string): Promise { + const dir = join(codexHome, 'sessions', '2026', '08', '19') + await mkdir(dir, { recursive: true }) + await writeFile( + join(dir, `rollout-2026-08-19T10-00-00-${threadId}.jsonl`), + `${JSON.stringify({ type: 'session_meta', payload: { id: threadId } })}\n`, + 'utf8' + ) +} + +/** Writes the registry file the real `getCodexPaneAccount` reads at adoption time. */ +async function recordPaneAccount(record: CodexPaneAccountRecord | null): Promise { + await writeFile( + join(root, 'codex-pane-accounts.json'), + `${JSON.stringify({ version: 2, panes: record ? { [PTY_ID]: record } : {} })}\n`, + 'utf8' + ) + codexPaneAccountRegistryInternals.resetCache() +} + +function buildSettings() { + const account = (id: string, managedHomePath: string) => ({ + id, + email: `${id}@example.com`, + managedHomePath, + createdAt: 1, + updatedAt: 1, + lastAuthenticatedAt: 1 + }) + return { + // Why a configured CODEX_HOME: it is what the create intent resolves to in this rig, so an + // adoption that follows the selection probes THIS home instead of the pane's. + agentDefaultEnv: { codex: { CODEX_HOME: selectedAccountHome } }, + activeCodexManagedAccountId: SELECTED_ACCOUNT_ID, + codexManagedAccounts: [ + account(PANE_ACCOUNT_ID, paneAccountHome), + account(SELECTED_ACCOUNT_ID, selectedAccountHome) + ] + } +} + +function installProofPty(rigState: AdoptionRig, ptyRecord: { lastOutputAt: number }) { + return (ptyId: string, data: string, authority: { sessionId: string; spawnToken: string }) => { + if (!agentSessionPtyWriteGate.admitProof(ptyId, authority)) { + return false + } + rigState.proofWrites.push(data) + if (data === '\r') { + const internal = rigState.runtime as unknown as { + ptysById: Map + } + const pty = internal.ptysById.get(PTY_ID)! + pty.tailBuffer = [`Session ID: ${THREAD_ID}`] + pty.lastOutputAt = ptyRecord.lastOutputAt + 1_000 + } + return true + } +} + +async function buildRig(): Promise { + const store = await AgentSessionRecordStore.open({ + directory: join(root, 'agent-sessions'), + hostId: 'local' + }) + agentSessionPtyWriteGate.attachRecordLookup((sessionId) => store.getRecord(sessionId)) + const host = new StructuredAgentSessionHost({ + store, + adapter: { + supportsCreate: () => true, + acquire: vi.fn(), + dispatch: vi.fn(), + cancelTurn: vi.fn(), + answerPrompt: vi.fn(), + setOption: vi.fn() + }, + journalRoot: join(root, 'journals'), + claimKeyId: 'key-1', + probeOwner: createStructuredAgentSessionOwnerProbe( + 'local', + async () => ({ outcome: 'identity-matched', matchedOn: ['process-start-time'] }), + async () => [] + ) + }) + setStructuredAgentSessionHost(host) + + const runtime = new OrcaRuntimeService({ getSettings: () => buildSettings() } as never) + const state: AdoptionRig = { runtime, store, host, proofWrites: [] } + const ptyRecord = { + ptyId: PTY_ID, + connected: true, + connectionId: null, + wslDistro: null, + tabId: 'tab-adopt', + paneKey: PANE_KEY, + worktreeId: WORKTREE_ID, + incarnationId: 'inc-adopt', + tailBuffer: [] as string[], + tailPartialLine: '', + preview: '', + lastOutputAt: 1_000 + } + const internal = runtime as unknown as { + ptysById: Map + resolveRuntimeFileTarget(): Promise + resolveStructuredAgentSessionLocation(): Promise + ensureStructuredAgentSessionHost(): Promise + issueStructuredTuiPtyHandle(): string + } + internal.ptysById.set(PTY_ID, ptyRecord) + internal.resolveRuntimeFileTarget = vi.fn(async () => ({ + connectionId: null, + worktree: { id: WORKTREE_ID, path: join(root, 'workspace') } + })) + internal.resolveStructuredAgentSessionLocation = vi.fn(async () => ({ + executionHostId: 'local', + wslDistro: null, + workspaceId: WORKTREE_ID, + workspaceKind: 'git-worktree' + })) + // The rig owns the host; installing the real one would replace it with an on-disk singleton. + internal.ensureStructuredAgentSessionHost = vi.fn(async () => {}) + internal.issueStructuredTuiPtyHandle = vi.fn(() => 'term-adopt') + runtime.setPtyController({ + listProcesses: async () => [{ id: PTY_ID, incarnationId: 'inc-adopt', rootProcessId: 31337 }], + writeAgentSessionProof: installProofPty(state, ptyRecord) + } as never) + return state +} + +function adoptInput() { + return { + envelope: { + sessionId: SESSION_ID, + clientOperationId: createStructuredAgentSessionOperationId(() => randomUUID()), + expectedRuntimeFence: null as null, + payloadFingerprint: 'a'.repeat(64) + }, + worktree: `id:${WORKTREE_ID}`, + tabId: 'tab-adopt', + paneKey: PANE_KEY, + ptyId: PTY_ID, + threadId: THREAD_ID + } +} + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-adopt-home-')) + vi.stubEnv('ORCA_USER_DATA_PATH', root) + paneAccountHome = join(root, 'codex-accounts', PANE_ACCOUNT_ID, 'home') + selectedAccountHome = join(root, 'codex-accounts', SELECTED_ACCOUNT_ID, 'home') + await recordPaneAccount({ + selectionKey: 'host', + accountId: PANE_ACCOUNT_ID, + homeRoute: 'account-home' + }) + readStructuredTuiProcessIdentity.mockImplementation( + async (input: { hostId: string; spawnToken: string }) => ({ + hostId: input.hostId, + pid: 4242, + processStartTimeMs: 1_700_000_000_000, + spawnToken: input.spawnToken + }) + ) + rig = await buildRig() +}) + +afterEach(async () => { + agentSessionPtyWriteGate.unbindPty(PTY_ID) + agentSessionPtyWriteGate.detachRecordLookup() + setStructuredAgentSessionHost(null) + codexPaneAccountRegistryInternals.resetCache() + vi.unstubAllEnvs() + await rig.host.flushAllStreamedEvents() + await rm(root, { recursive: true, force: true }) +}) + +describe('structured Codex adoption resolves the account home from the pane', () => { + it('adopts a pane whose Codex runs under a non-selected account home', async () => { + // Only the pane's account owns the rollout; the selected account's home has none. + await writeRolloutFixture(paneAccountHome, THREAD_ID) + + const result = await rig.runtime.adoptStructuredAgentSessionTerminal(adoptInput(), { + callerKey: 'renderer-1' + }) + + expect(result).toMatchObject({ ok: true }) + expect(rig.store.getRecord(SESSION_ID)).toMatchObject({ + accountHome: { variable: 'CODEX_HOME', path: paneAccountHome }, + lease: { runtimeKind: 'tui', claimStatus: 'live' } + }) + }, 20_000) + + it('still adopts a pane whose account is the selected one', async () => { + await recordPaneAccount({ + selectionKey: 'host', + accountId: SELECTED_ACCOUNT_ID, + homeRoute: 'account-home' + }) + await writeRolloutFixture(selectedAccountHome, THREAD_ID) + + const result = await rig.runtime.adoptStructuredAgentSessionTerminal(adoptInput(), { + callerKey: 'renderer-1' + }) + + expect(result).toMatchObject({ ok: true }) + expect(rig.store.getRecord(SESSION_ID)).toMatchObject({ + accountHome: { path: selectedAccountHome } + }) + }, 20_000) + + it('refuses a pane whose Codex home cannot be attributed instead of probing another account', async () => { + await recordPaneAccount(null) + await writeRolloutFixture(selectedAccountHome, THREAD_ID) + + await expect( + rig.runtime.adoptStructuredAgentSessionTerminal(adoptInput(), { callerKey: 'renderer-1' }) + ).rejects.toThrow('no record of which Codex account this terminal launched under') + // Nothing was probed and nothing was reserved, so the wrong home was never consulted. + expect(rig.proofWrites).toEqual([]) + expect(agentSessionPtyWriteGate.boundSessionId(PTY_ID)).toBeNull() + expect(rig.store.getRecord(SESSION_ID)).toBeNull() + }) + + it('names the account when the pane launched under one that is gone', async () => { + await recordPaneAccount({ + selectionKey: 'host', + accountId: 'codex-account-removed', + homeRoute: 'account-home' + }) + + await expect( + rig.runtime.adoptStructuredAgentSessionTerminal(adoptInput(), { callerKey: 'renderer-1' }) + ).rejects.toThrow('managed account codex-account-removed, which is no longer configured') + }) + + it('leaves create following the selected account, not the adopted pane', async () => { + const created = await rig.runtime.resolveStructuredAgentSessionCreateIntent({ + envelope: { sessionId: SESSION_ID, clientOperationId: 'op-create' }, + worktree: `id:${WORKTREE_ID}`, + agent: 'codex' + }) + + // The registry still names the pane's account; a create must ignore it. + expect(created.accountHome).toEqual({ variable: 'CODEX_HOME', path: selectedAccountHome }) + }) +}) 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 861cd434f0e..a3d1db82f63 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 @@ -128,7 +128,7 @@ async function buildRig(): Promise { const internal = runtime as unknown as { ptysById: Map resolveRuntimeFileTarget(): Promise - resolveStructuredAgentSessionCreateIntent(input: { envelope: unknown }): Promise + resolveStructuredAgentSessionAdoptionIntent(input: { envelope: unknown }): Promise issueStructuredTuiPtyHandle(): string } internal.ptysById.set(PTY_ID, ptyRecord) @@ -136,7 +136,10 @@ async function buildRig(): Promise { connectionId: null, worktree: { id: WORKTREE_ID } })) - internal.resolveStructuredAgentSessionCreateIntent = vi.fn(async ({ envelope }) => ({ + // Why the adoption intent and not the create one: adoption resolves the pane's OWN account + // home, which this rig has no pane-account record for. The pane-home resolution itself is + // covered by orca-runtime-structured-tui-adoption-account-home.test.ts. + internal.resolveStructuredAgentSessionAdoptionIntent = vi.fn(async ({ envelope }) => ({ envelope, location: { executionHostId: 'local', diff --git a/src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts b/src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts index 778c9154447..5d25439c1e7 100644 --- a/src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts +++ b/src/main/runtime/orca-runtime-structured-tui-tab-binding.test.ts @@ -43,7 +43,7 @@ function terminalAdoptionHarness() { const internal = runtime as unknown as { ptysById: Map resolveRuntimeFileTarget(): Promise - resolveStructuredAgentSessionCreateIntent(input: { envelope: unknown }): Promise + resolveStructuredAgentSessionAdoptionIntent(input: { envelope: unknown }): Promise issueStructuredTuiPtyHandle(): string } internal.ptysById.set('pty-adopt', { @@ -60,7 +60,10 @@ function terminalAdoptionHarness() { connectionId: null, worktree: { id: WORKTREE_ID } })) - internal.resolveStructuredAgentSessionCreateIntent = vi.fn(async ({ envelope }) => ({ + // Why the adoption intent and not the create one: adoption resolves the pane's OWN account + // home, which this rig has no pane-account record for. The pane-home resolution itself is + // covered by orca-runtime-structured-tui-adoption-account-home.test.ts. + internal.resolveStructuredAgentSessionAdoptionIntent = vi.fn(async ({ envelope }) => ({ envelope, location: { executionHostId: 'local', diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 524ce8c5585..5f42ad4ddd8 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -122,7 +122,12 @@ import { readStructuredTuiProcessIdentity } from './structured-tui-process-ident import { hasStructuredTuiIdleEvidence } from './structured-tui-idle-evidence' import { evaluateStructuredTuiRecoveryClaim } from './structured-tui-recovery-claim-match' import { getProfileUserDataPath } from '../orca-profiles/profile-storage-paths' -import { getSystemCodexHomePath } from '../codex/codex-home-paths' +import { getSystemCodexHomePath, resolveOrcaManagedCodexHomePath } from '../codex/codex-home-paths' +import { getCodexPaneAccount } from '../codex/codex-pane-account-registry' +import { + describeCodexPaneLaunchHomeFailure, + resolveCodexPaneLaunchHome +} from '../codex/codex-pane-launch-home' import { agentSessionPtyWriteGate, type AgentSessionPtyWriteAdmittance @@ -11451,10 +11456,11 @@ export class OrcaRuntimeService { if (target.connectionId || target.worktree.id !== pty.worktreeId) { throw new Error('Structured Codex chat is unavailable for remote terminals.') } - const intent = await this.resolveStructuredAgentSessionCreateIntent({ + const intent = await this.resolveStructuredAgentSessionAdoptionIntent({ envelope: input.envelope, worktree: input.worktree, - agent: 'codex' + agent: 'codex', + ptyId: input.ptyId }) const listing = (await this.ptyController?.listProcesses?.())?.find( (candidate) => @@ -11678,6 +11684,60 @@ export class OrcaRuntimeService { worktree: string agent: 'codex' }): Promise { + return this.resolveStructuredAgentSessionIntent(input, ({ workspacePath, launchEnv }) => { + // A create has no process yet, so the current selection is what it must follow. + const preparedHome = this.prepareCodexStructuredLaunchFn?.({ workspacePath, launchEnv }) + const configuredHome = launchEnv.CODEX_HOME + return ( + preparedHome?.trim() || + (this.prepareCodexStructuredLaunchFn ? getSystemCodexHomePath() : configuredHome?.trim()) || + getSystemCodexHomePath() + ) + }) + } + + /** + * Why adoption resolves its own intent: the pane's Codex is already running under the + * CODEX_HOME it was spawned with, and only that home's sessions directory can hold its + * rollout. Following the create intent here probed the currently selected account instead, + * so adopting a pane on any other account failed with no hint that the account mismatched. + */ + async resolveStructuredAgentSessionAdoptionIntent(input: { + envelope: { sessionId: string; clientOperationId: string } + worktree: string + agent: 'codex' + ptyId: string + }): Promise { + return this.resolveStructuredAgentSessionIntent(input, () => + this.resolveAdoptedCodexPaneHomePath(input.ptyId) + ) + } + + private resolveAdoptedCodexPaneHomePath(ptyId: string): string { + const resolved = resolveCodexPaneLaunchHome({ + record: getCodexPaneAccount(ptyId), + settings: this.requireStore().getSettings(), + systemCodexHomePath: getSystemCodexHomePath(), + sharedRuntimeCodexHomePath: resolveOrcaManagedCodexHomePath() + }) + // Never fall back to the selected home: a silent wrong-home probe is what hid the mismatch. + if (resolved.kind !== 'attributed') { + throw new Error(describeCodexPaneLaunchHomeFailure(resolved)) + } + return resolved.path + } + + private async resolveStructuredAgentSessionIntent( + input: { + envelope: { sessionId: string; clientOperationId: string } + worktree: string + agent: 'codex' + }, + resolveAccountHomePath: (context: { + workspacePath: string + launchEnv: NodeJS.ProcessEnv + }) => string + ): Promise { const support = await this.getStructuredAgentSessionCreateSupport(input.worktree, input.agent) if (!support.supported) { throw new Error('structured_agent_session_unsupported') @@ -11686,8 +11746,6 @@ export class OrcaRuntimeService { const launchEnv = resolveTuiAgentLaunchEnv('codex', settings.agentDefaultEnv) const location = await this.resolveStructuredAgentSessionLocation(input.worktree) const workspacePath = (await this.resolveRuntimeFileTarget(input.worktree)).worktree.path - const preparedHome = this.prepareCodexStructuredLaunchFn?.({ workspacePath, launchEnv }) - const configuredHome = launchEnv.CODEX_HOME return { envelope: { sessionId: input.envelope.sessionId, @@ -11700,12 +11758,7 @@ export class OrcaRuntimeService { agent: 'codex', accountHome: { variable: 'CODEX_HOME', - path: - preparedHome?.trim() || - (this.prepareCodexStructuredLaunchFn - ? getSystemCodexHomePath() - : configuredHome?.trim()) || - getSystemCodexHomePath() + path: resolveAccountHomePath({ workspacePath, launchEnv }) }, runtimeKind: 'native' }