diff --git a/src/main/codex/codex-structured-launch-resolution.test.ts b/src/main/codex/codex-structured-launch-resolution.test.ts index da3af41d79d0..60aab5ed1db3 100644 --- a/src/main/codex/codex-structured-launch-resolution.test.ts +++ b/src/main/codex/codex-structured-launch-resolution.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest' import type { AgentSessionRecord } from '../../shared/agent-session-record' +import type { AgentSessionProviderHandleLink } from '../../shared/agent-session-provider-handle' import { LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host' import type { AgentSessionRecordStore } from '../runtime/agent-session-record-store' import { createCodexStructuredLaunchResolver } from './codex-structured-launch-resolution' @@ -114,6 +115,35 @@ describe('codex structured launch resolution', () => { expect(launch.resumeThreadId).toBe('thread-current') }) + it('lets only a thread this session created be superseded when Codex never saved it', async () => { + const link = ( + origin: AgentSessionProviderHandleLink['origin'], + mintedAtFence: number + ): AgentSessionProviderHandleLink => ({ + linkId: `link-${mintedAtFence}`, + handle: { provider: 'codex', threadId: 't' }, + origin, + mintedAtFence, + observedAt: 1 + }) + const chainFor = (origin: 'created' | 'resumed' | 'adopted') => + origin === 'resumed' ? [link('created', 1), link('resumed', 2)] : [link(origin, 1)] + + const created = await resolverFor(record({ providerHandleChain: chainFor('created') }))({ + identity: IDENTITY + }) + expect(created).toMatchObject({ resumeThreadId: 't', supersedeIfUnsaved: true }) + for (const origin of ['resumed', 'adopted'] as const) { + const launch = await resolverFor(record({ providerHandleChain: chainFor(origin) }))({ + identity: IDENTITY + }) + expect(launch.resumeThreadId).toBe('t') + expect(launch).not.toHaveProperty('supersedeIfUnsaved') + } + const fresh = await resolverFor(record())({ identity: IDENTITY }) + expect(fresh).not.toHaveProperty('supersedeIfUnsaved') + }) + // Agent Permissions is the only thing derived from the arguments field. app-server owns it on // the thread RPC rather than through the interactive CLI's process flags. it('resolves the bypass posture as app-server thread policy', async () => { diff --git a/src/main/codex/codex-structured-launch-resolution.ts b/src/main/codex/codex-structured-launch-resolution.ts index ca2e23573e14..e60d814471e0 100644 --- a/src/main/codex/codex-structured-launch-resolution.ts +++ b/src/main/codex/codex-structured-launch-resolution.ts @@ -84,6 +84,9 @@ export function createCodexStructuredLaunchResolver( // An empty chain is a session that has never proved a thread, so it // starts one; anything else resumes the last link this session proved. resumeThreadId, + // Only a thread this session created may still be one Codex never saved: a resumed, + // forked or adopted head names a conversation Codex held. + ...(resumeThreadId && head?.origin === 'created' ? { supersedeIfUnsaved: true } : {}), ...(permissionPolicy ? { permissionPolicy } : {}), ...(resumeThreadId ? { diff --git a/src/main/codex/codex-structured-owner-identity.test.ts b/src/main/codex/codex-structured-owner-identity.test.ts index c9a7b6822181..c86885171e47 100644 --- a/src/main/codex/codex-structured-owner-identity.test.ts +++ b/src/main/codex/codex-structured-owner-identity.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { codexProcessIdentity } from './codex-structured-owner-identity' +import { codexProcessIdentity, codexProviderHandleLink } from './codex-structured-owner-identity' const IDENTITY = { sessionId: 'session-identity', @@ -46,3 +46,22 @@ describe('codex process identity', () => { expect(readStartTime).toHaveBeenCalledTimes(3) }) }) + +describe('codex provider handle link', () => { + it('names the unsaved thread a creation superseded, and nothing else can', () => { + expect( + codexProviderHandleLink({ + threadId: 'thread-new', + resumed: false, + supersedesThreadId: 'thread-unsaved', + fence: 3, + observedAt: 1 + }) + ).toMatchObject({ origin: 'created', supersedesKey: 'codex:"thread-unsaved"' }) + const base = { threadId: 'thread-new', fence: 3, observedAt: 1 } + // @ts-expect-error an adopted conversation was never an unsaved creation + codexProviderHandleLink({ ...base, resumed: false, origin: 'adopted', supersedesThreadId: 't' }) + // @ts-expect-error a resume proved the thread it named, so it replaces nothing + codexProviderHandleLink({ ...base, resumed: true, supersedesThreadId: 't' }) + }) +}) diff --git a/src/main/codex/codex-structured-owner-identity.ts b/src/main/codex/codex-structured-owner-identity.ts index 2259303a4b81..44f25ea47247 100644 --- a/src/main/codex/codex-structured-owner-identity.ts +++ b/src/main/codex/codex-structured-owner-identity.ts @@ -1,5 +1,8 @@ import type { AgentSessionJournalIdentity } from '../../shared/agent-session-journal-types' -import type { AgentSessionProviderHandleLink } from '../../shared/agent-session-provider-handle' +import { + agentSessionProviderHandleKey, + type AgentSessionProviderHandleLink +} from '../../shared/agent-session-provider-handle' import type { AgentSessionProcessIdentity } from '../../shared/agent-session-record' import { readProcessStartTimeMs } from '../runtime/agent-session-process-identity-probe' @@ -46,19 +49,33 @@ export async function codexProcessIdentity( } } -export function codexProviderHandleLink(input: { +type CodexProviderHandleLinkInput = { threadId: string - resumed: boolean - origin?: 'adopted' fence: number linkId?: string observedAt: number -}): AgentSessionProviderHandleLink { +} & ( + | { origin?: 'adopted'; resumed: boolean; supersedesThreadId?: never } + /** A new thread started in place of this unsaved one; only a creation can supersede. */ + | { origin?: never; resumed: false; supersedesThreadId: string } +) + +export function codexProviderHandleLink( + input: CodexProviderHandleLinkInput +): AgentSessionProviderHandleLink { return { linkId: input.linkId ?? `codex-${input.fence}-${input.threadId}`.slice(0, 128), handle: { provider: 'codex', threadId: input.threadId }, origin: input.origin ?? (input.resumed ? 'resumed' : 'created'), mintedAtFence: input.fence, - observedAt: input.observedAt + observedAt: input.observedAt, + ...(input.supersedesThreadId + ? { + supersedesKey: agentSessionProviderHandleKey({ + provider: 'codex', + threadId: input.supersedesThreadId + }) + } + : {}) } } diff --git a/src/main/codex/codex-structured-session-acquire.ts b/src/main/codex/codex-structured-session-acquire.ts index 8a7aa5dd8277..94b64e5b1e17 100644 --- a/src/main/codex/codex-structured-session-acquire.ts +++ b/src/main/codex/codex-structured-session-acquire.ts @@ -210,7 +210,9 @@ export async function acquireCodexStructuredSession(input: { process, link: codexProviderHandleLink({ threadId: opened.threadId, - resumed: launch.resumeThreadId !== null, + ...(opened.supersededThreadId + ? { resumed: false, supersedesThreadId: opened.supersededThreadId } + : { resumed: launch.resumeThreadId !== null }), fence: acquireInput.fence, linkId: deps.mintLinkId?.(), observedAt: deps.now?.() ?? Date.now() diff --git a/src/main/codex/codex-structured-session-adapter.test.ts b/src/main/codex/codex-structured-session-adapter.test.ts index 020a29ea33b8..f17003f7e40a 100644 --- a/src/main/codex/codex-structured-session-adapter.test.ts +++ b/src/main/codex/codex-structured-session-adapter.test.ts @@ -83,6 +83,42 @@ describe('CodexStructuredSessionAdapter.acquire', () => { expect(acquisition.link.handle).toEqual({ provider: 'codex', threadId: 'thread-proven' }) }) + it('starts a thread in place of a creation Codex never saved, and says which it replaced', async () => { + const codex = fakeCodex({ + 'thread/resume': () => { + throw new CodexAppServerRequestError( + 'thread/resume', + -32600, + 'codex app-server thread/resume failed: no rollout found for thread id thread-unsaved' + ) + } + }) + const adapter = adapterFor(codex, { + resumeThreadId: 'thread-unsaved', + supersedeIfUnsaved: true + }) + + const acquisition = await adapter.acquire({ + identity: identityFor('session-1'), + fence: 9, + spawnToken: 'spawn-9' + }) + + expect(codex.connections[0].calls.map((call) => call.method)).toEqual([ + 'thread/resume', + 'thread/start' + ]) + expect(acquisition.link).toEqual({ + linkId: `codex-9-${THREAD_ID}`, + handle: { provider: 'codex', threadId: THREAD_ID }, + origin: 'created', + supersedesKey: 'codex:"thread-unsaved"', + mintedAtFence: 9, + observedAt: 1_700_000_000_500 + }) + expect(codex.connections[0].closeCount).toBe(0) + }) + it('refuses a resume that lands on a different thread and reaps the child', async () => { const codex = fakeCodex({ 'thread/resume': () => ({ thread: { id: 'thread-other' } }) }) const adapter = adapterFor(codex, { resumeThreadId: 'thread-proven' }) diff --git a/src/main/codex/codex-structured-session-state.ts b/src/main/codex/codex-structured-session-state.ts index c890df0be582..fa62d2cc0a49 100644 --- a/src/main/codex/codex-structured-session-state.ts +++ b/src/main/codex/codex-structured-session-state.ts @@ -24,6 +24,9 @@ export type CodexStructuredLaunch = { codexHome: string | null resumeThreadId: string | null resumePath?: string | null + /** The resumed thread is this session's own creation: when Codex answers that it holds no + * rollout for it, start a new thread in its place. Never set for a thread a resume proved. */ + supersedeIfUnsaved?: boolean permissionPolicy?: CodexStructuredPermissionPolicy env?: Record } diff --git a/src/main/codex/codex-structured-thread-open.test.ts b/src/main/codex/codex-structured-thread-open.test.ts index 0dec79de10af..b819a0dc6b21 100644 --- a/src/main/codex/codex-structured-thread-open.test.ts +++ b/src/main/codex/codex-structured-thread-open.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import { CodexAppServerFrameSizeError, CodexAppServerRequestError, + openCodexAppServerConnection, type CodexAppServerConnection } from './codex-app-server-connection' import { codexStructuredPermissionPolicyForSettings } from './codex-structured-permission-policy' @@ -216,4 +217,123 @@ describe('openCodexThread', () => { ).resolves.toMatchObject({ threadId: 'thread-1' }) expect(request).toHaveBeenCalledTimes(2) }) + + describe('a thread Codex never saved', () => { + const noRollout = (threadId: string, code = -32600) => + new CodexAppServerRequestError( + 'thread/resume', + code, + `codex app-server thread/resume failed: no rollout found for thread id ${threadId}` + ) + function codexWithoutRollout(error: Error = noRollout('thread-unsaved')) { + return vi.fn(async (method: string, _params?: Record) => { + if (method === 'thread/resume') { + throw error + } + return { thread: { id: 'thread-new' }, model: 'gpt-live' } + }) + } + + it('starts a new thread in its place when Codex proves it holds no rollout', async () => { + const request = codexWithoutRollout() + + await expect( + openCodexThread( + connectionFor(request), + { cwd: '/workspace', resumeThreadId: 'thread-unsaved', supersedeIfUnsaved: true }, + 2_000 + ) + ).resolves.toMatchObject({ + threadId: 'thread-new', + supersededThreadId: 'thread-unsaved', + model: 'gpt-live' + }) + expect(request.mock.calls.map(([method]) => method)).toEqual([ + 'thread/resume', + 'thread/start' + ]) + expect(request).toHaveBeenLastCalledWith( + 'thread/start', + { cwd: '/workspace' }, + { + timeoutMs: 2_000 + } + ) + }) + + it('keeps the resume failure for a thread a resume already proved', async () => { + const request = codexWithoutRollout() + + await expect( + openCodexThread( + connectionFor(request), + { cwd: '/workspace', resumeThreadId: 'thread-unsaved' }, + 2_000 + ) + ).rejects.toThrow('no rollout found for thread id thread-unsaved') + expect(request).toHaveBeenCalledOnce() + }) + + it('treats no other resume failure as proof that nothing was saved', async () => { + const failures = [ + noRollout('thread-other'), + noRollout('thread-unsaved', -32603), + new CodexAppServerRequestError( + 'thread/resume', + -32600, + 'codex app-server thread/resume failed: thread not found' + ), + new Error('codex app-server exited') + ] + for (const failure of failures) { + const request = codexWithoutRollout(failure) + await expect( + openCodexThread( + connectionFor(request), + { cwd: '/workspace', resumeThreadId: 'thread-unsaved', supersedeIfUnsaved: true }, + 2_000 + ) + ).rejects.toBe(failure) + expect(request).toHaveBeenCalledOnce() + } + }) + + // Every other test here builds the error itself; this one sends Codex's raw JSON-RPC frame + // through the real connection, so a change to Orca's own error wording cannot hide the proof. + it('recognizes the raw frame Codex sends, through the real connection', async () => { + const fakeAppServer = String.raw` + const readline = require('node:readline') + 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 === 'thread/resume') { + const threadId = message.params.threadId + return send({ + id: message.id, + error: { code: -32600, message: 'no rollout found for thread id ' + threadId } + }) + } + if (message.method === 'thread/start') { + return send({ id: message.id, result: { thread: { id: 'thread-new' } } }) + } + }) + ` + const connection = await openCodexAppServerConnection({ + command: process.execPath, + args: ['-e', fakeAppServer] + }) + try { + await expect( + openCodexThread( + connection, + { cwd: '/workspace', resumeThreadId: 'thread-unsaved', supersedeIfUnsaved: true }, + 5_000 + ) + ).resolves.toMatchObject({ threadId: 'thread-new', supersededThreadId: 'thread-unsaved' }) + } finally { + await connection.close() + } + }) + }) }) diff --git a/src/main/codex/codex-structured-thread-open.ts b/src/main/codex/codex-structured-thread-open.ts index 2dd5cbee59cb..83cd710e1360 100644 --- a/src/main/codex/codex-structured-thread-open.ts +++ b/src/main/codex/codex-structured-thread-open.ts @@ -14,6 +14,8 @@ import { readCodexThreadId, readCodexThreadPath } from './codex-structured-threa export type CodexOpenedThread = { threadId: string + /** The unsaved thread this new one was started in place of. */ + supersededThreadId?: string thread?: Record /** Rollout file Codex named, when it named one. */ historyPath: string | null @@ -63,37 +65,68 @@ async function resumeCodexThread( } } +/** + * Codex's own answer that it holds no rollout for this exact thread: the thread was started but + * never given input, so there is no conversation to lose. Codex matches the same exact text + * internally; any other resume failure, including a broader "not found", is not this proof. + * Orca's own wrapper prefix is deliberately not part of the match. + * Codex also uses this text for an archived thread read active-only; resume reads archived threads + * and answers "is archived" instead, so here the text means no rollout exists at all. + */ +function isCodexNoRolloutError(error: unknown, threadId: string): boolean { + return ( + isCodexAppServerRequestError(error) && + error.method === 'thread/resume' && + error.code === -32600 && + error.message.endsWith(`no rollout found for thread id ${threadId}`) + ) +} + export async function openCodexThread( connection: Pick, launch: { cwd: string resumeThreadId: string | null resumePath?: string | null + supersedeIfUnsaved?: boolean permissionPolicy?: CodexStructuredPermissionPolicy }, timeoutMs: number | undefined ): Promise { - const resumeParams = launch.resumeThreadId - ? { - threadId: launch.resumeThreadId, - cwd: launch.cwd, - ...launch.permissionPolicy, - ...(launch.resumePath ? { path: launch.resumePath } : {}) + const resumeThreadId = launch.resumeThreadId + const startThread = (): Promise => + connection.request( + 'thread/start', + { cwd: launch.cwd, ...launch.permissionPolicy }, + { timeoutMs } + ) + let supersededThreadId: string | undefined + let opened: unknown + if (!resumeThreadId) { + opened = await startThread() + } else { + const resumeParams = { + threadId: resumeThreadId, + cwd: launch.cwd, + ...launch.permissionPolicy, + ...(launch.resumePath ? { path: launch.resumePath } : {}) + } + try { + opened = await resumeCodexThread(connection, resumeParams, timeoutMs) + } catch (error) { + if (!launch.supersedeIfUnsaved || !isCodexNoRolloutError(error, resumeThreadId)) { + throw error } - : null - const opened = resumeParams - ? await resumeCodexThread(connection, resumeParams, timeoutMs) - : await connection.request( - 'thread/start', - { cwd: launch.cwd, ...launch.permissionPolicy }, - { timeoutMs } - ) + supersededThreadId = resumeThreadId + opened = await startThread() + } + } const threadId = readCodexThreadId(opened) if (!threadId) { throw new Error('codex app-server did not name the thread it opened') } - if (launch.resumeThreadId && threadId !== launch.resumeThreadId) { - throw new Error(`codex app-server resumed ${threadId} instead of ${launch.resumeThreadId}`) + if (supersededThreadId === undefined && resumeThreadId && threadId !== resumeThreadId) { + throw new Error(`codex app-server resumed ${threadId} instead of ${resumeThreadId}`) } const result = opened as Record const thread = @@ -106,6 +139,7 @@ export async function openCodexThread( const serviceTier = nonEmptyString(result.serviceTier) return { threadId, + ...(supersededThreadId === undefined ? {} : { supersededThreadId }), thread, historyPath: readCodexThreadPath(opened), ...(thread.historyMode === 'legacy' || thread.historyMode === 'paginated' diff --git a/src/main/runtime/agent-session-unsaved-creation-supersession.test.ts b/src/main/runtime/agent-session-unsaved-creation-supersession.test.ts new file mode 100644 index 000000000000..ec2a1e82e92b --- /dev/null +++ b/src/main/runtime/agent-session-unsaved-creation-supersession.test.ts @@ -0,0 +1,160 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import type { AgentSessionProviderHandleLink } from '../../shared/agent-session-provider-handle' +import { codexProviderHandleLink } from '../codex/codex-structured-owner-identity' +import { AgentSessionRecordStore } from './agent-session-record-store' +import type { AgentSessionReserveRequest } from './agent-session-reservation-admission' + +const NOW = 1_800_000_000_000 +const SESSION = 'session-codex' +let directory: string +let operations = 0 + +function reserveRequest( + overrides: Partial = {} +): AgentSessionReserveRequest { + operations += 1 + return { + sessionId: SESSION, + location: { + executionHostId: 'local', + wslDistro: null, + workspaceId: 'workspace-1', + workspaceKind: 'folder' + }, + provider: 'codex', + accountHome: { variable: 'CODEX_HOME', path: '/home/dev/.codex' }, + runtimeKind: 'native', + expectedFence: null, + spawnToken: `spawn-${operations}`, + claimKeyId: 'key-1', + handoffOperationId: null, + probe: { outcome: 'reservation-unused' }, + operation: { + callerKey: 'client-1', + operationId: `${NOW}-${String(operations).padStart(32, '0')}`, + fingerprint: `fp-${operations}` + }, + now: NOW, + ...overrides + } +} + +/** Reserve, observe the spawn and prove the link the adapter reported, as the host does. */ +async function prove( + store: AgentSessionRecordStore, + request: AgentSessionReserveRequest, + link: (fence: number) => AgentSessionProviderHandleLink +) { + const { record } = await store.reserveOwner(request) + const fence = record.lease.runtimeFence + await store.commitProcessIdentity({ + sessionId: SESSION, + fence, + process: { + hostId: 'local', + pid: 4242 + fence, + processStartTimeMs: NOW, + spawnToken: record.lease.reservedSpawnToken ?? '' + }, + now: NOW + }) + return store.proveOwner({ sessionId: SESSION, fence, link: link(fence), now: NOW }) +} + +/** A restart: the previous owner is gone and the reopened store adjudicates it. */ +async function restart(store: AgentSessionRecordStore) { + const fence = store.getRecord(SESSION)?.lease.runtimeFence ?? 0 + await store.evictProvenDeadOwner({ + sessionId: SESSION, + expectedFence: fence, + probe: { outcome: 'exit-observed' }, + now: NOW + }) + const reopened = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + await reopened.reconcileOnRestart({ + probe: async () => ({ outcome: 'reservation-unused' }), + now: NOW + }) + return reopened +} + +beforeEach(async () => { + directory = await mkdtemp(join(tmpdir(), 'orca-agent-session-supersession-')) +}) + +afterEach(async () => { + await rm(directory, { recursive: true, force: true }) +}) + +describe('a Codex thread started in place of one Codex never saved', () => { + it('becomes the session identity and survives the next restart', async () => { + const first = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + await prove(first, reserveRequest(), (fence) => + codexProviderHandleLink({ + threadId: 'thread-unsaved', + resumed: false, + fence, + observedAt: NOW + }) + ) + + const second = await restart(first) + const expectedFence = second.getRecord(SESSION)?.lease.runtimeFence ?? 0 + const proved = await prove(second, reserveRequest({ expectedFence }), (fence) => + codexProviderHandleLink({ + threadId: 'thread-new', + resumed: false, + supersedesThreadId: 'thread-unsaved', + fence, + observedAt: NOW + }) + ) + + expect(proved.lease.claimStatus).toBe('live') + expect(proved.providerHandleChain).toEqual([ + expect.objectContaining({ + handle: { provider: 'codex', threadId: 'thread-new' }, + origin: 'created', + supersedesKey: 'codex:"thread-unsaved"' + }) + ]) + expect(proved.lease.provenHandleLinkId).toBe(proved.providerHandleChain[0]?.linkId) + + const third = await restart(second) + expect(third.getRecord(SESSION)?.providerHandleChain).toEqual(proved.providerHandleChain) + }) + + it('is refused once a resume has proved the conversation Codex saved', async () => { + const first = await AgentSessionRecordStore.open({ directory, hostId: 'local' }) + await prove(first, reserveRequest(), (fence) => + codexProviderHandleLink({ threadId: 'thread-saved', resumed: false, fence, observedAt: NOW }) + ) + const second = await restart(first) + await prove( + second, + reserveRequest({ expectedFence: second.getRecord(SESSION)?.lease.runtimeFence ?? 0 }), + (fence) => + codexProviderHandleLink({ threadId: 'thread-saved', resumed: true, fence, observedAt: NOW }) + ) + + const third = await restart(second) + await expect( + prove( + third, + reserveRequest({ expectedFence: third.getRecord(SESSION)?.lease.runtimeFence ?? 0 }), + (fence) => + codexProviderHandleLink({ + threadId: 'thread-new', + resumed: false, + supersedesThreadId: 'thread-saved', + fence, + observedAt: NOW + }) + ) + ).rejects.toThrow('agent_session_provider_handle_invalid') + expect(third.getRecord(SESSION)?.providerHandleChain).toHaveLength(2) + }) +}) diff --git a/src/shared/agent-session-provider-handle.test.ts b/src/shared/agent-session-provider-handle.test.ts index e2830c010292..7474c751cbe0 100644 --- a/src/shared/agent-session-provider-handle.test.ts +++ b/src/shared/agent-session-provider-handle.test.ts @@ -391,3 +391,79 @@ describe('adopted chain heads', () => { expect(isAgentSessionProviderHandleChain([adopted()])).toBe(true) }) }) + +describe('superseding a creation the provider never saved', () => { + const unsaved: AgentSessionProviderHandle = { provider: 'codex', threadId: 'thread-unsaved' } + const created = link({ linkId: 'codex-1-thread-unsaved', handle: unsaved }) + function replacement(overrides: Partial = {}) { + return link({ + linkId: 'codex-3-thread-new', + handle: { provider: 'codex', threadId: 'thread-new' }, + mintedAtFence: 3, + supersedesKey: agentSessionProviderHandleKey(unsaved), + ...overrides + }) + } + + it('replaces the unsaved creation instead of standing beside it', () => { + const chain = appendAgentSessionProviderHandleLink([created], replacement()) + expect(chain).toEqual([replacement()]) + expect(isAgentSessionProviderHandleChain(chain)).toBe(true) + // An unused chat reopened across many restarts stays one link long. + const again = appendAgentSessionProviderHandleLink( + chain, + replacement({ + linkId: 'codex-5-thread-newer', + handle: { provider: 'codex', threadId: 'thread-newer' }, + mintedAtFence: 5, + supersedesKey: agentSessionProviderHandleKey({ provider: 'codex', threadId: 'thread-new' }) + }) + ) + expect(again).toHaveLength(1) + expect(again[0]?.handle).toEqual({ provider: 'codex', threadId: 'thread-newer' }) + }) + + it('never supersedes a conversation a resume, fork or adoption proved', () => { + const resumed = link({ linkId: 'codex-2-thread-unsaved', origin: 'resumed', handle: unsaved }) + expect(() => appendAgentSessionProviderHandleLink([created, resumed], replacement())).toThrow( + 'agent_session_provider_handle_invalid' + ) + expect(() => + appendAgentSessionProviderHandleLink([{ ...created, origin: 'adopted' }], replacement()) + ).toThrow('agent_session_provider_handle_invalid') + }) + + it('names exactly the head it replaces, on a new root, under a current fence', () => { + expect(() => + appendAgentSessionProviderHandleLink( + [created], + replacement({ supersedesKey: 'codex:"thread-other"' }) + ) + ).toThrow('agent_session_provider_handle_invalid') + expect(() => + appendAgentSessionProviderHandleLink([created], replacement({ handle: unsaved })) + ).toThrow('agent_session_provider_handle_invalid') + expect(() => + appendAgentSessionProviderHandleLink([created], replacement({ linkId: created.linkId })) + ).toThrow('agent_session_provider_handle_invalid') + expect(() => + appendAgentSessionProviderHandleLink( + [{ ...created, mintedAtFence: 4 }], + replacement({ mintedAtFence: 3 }) + ) + ).toThrow('agent_session_provider_handle_stale_fence') + }) + + it('carries supersession only on a creation, and never as a persisted second link', () => { + expect( + appendAgentSessionProviderHandleLink([], replacement({ origin: 'created' })) + ).toHaveLength(1) + expect(() => + appendAgentSessionProviderHandleLink( + [created], + replacement({ origin: 'resumed', handle: unsaved }) + ) + ).toThrow('agent_session_provider_handle_invalid') + expect(isAgentSessionProviderHandleChain([created, replacement()])).toBe(false) + }) +}) diff --git a/src/shared/agent-session-provider-handle.ts b/src/shared/agent-session-provider-handle.ts index 67d6d7f7ce21..ea789dcdfe40 100644 --- a/src/shared/agent-session-provider-handle.ts +++ b/src/shared/agent-session-provider-handle.ts @@ -5,6 +5,8 @@ * identifies a conversation. Claude's session id is the identity root and its leaf uuid is a * branch cursor; Codex's thread id is the whole key. Resumes extend the chain, forks start a new * identity root, and the chain records which is which so a fork is never presented as a resume. + * A creation the provider never saved can be superseded by a new creation, which takes its place + * instead of standing beside it: the unsaved handle was never a conversation to continue. */ export const AGENT_SESSION_PROVIDER_HANDLE_PROVIDERS = ['claude', 'codex'] as const @@ -32,6 +34,8 @@ export type AgentSessionProviderHandleLink = { observedAt: number /** Key of the link a fork was seeded from. Only set when `origin` is `forked`. */ forkedFromKey?: string + /** Key of the unsaved creation this creation replaced. Only set when `origin` is `created`. */ + supersedesKey?: string } export type AgentSessionProviderHandleChain = readonly AgentSessionProviderHandleLink[] @@ -124,7 +128,9 @@ export function isAgentSessionProviderHandleLink( Number.isSafeInteger(link.observedAt) && (link.origin === 'forked' ? isHandleField(link.forkedFromKey) - : link.forkedFromKey === undefined) + : link.forkedFromKey === undefined) && + (link.supersedesKey === undefined || + (link.origin === 'created' && isHandleField(link.supersedesKey))) ) } @@ -177,6 +183,9 @@ export function appendAgentSessionProviderHandleLink( if (link.mintedAtFence < head.mintedAtFence) { throw new Error('agent_session_provider_handle_stale_fence') } + if (link.origin === 'created' && link.supersedesKey !== undefined) { + return supersedeUnsavedCreation(head, link) + } if (link.origin === 'created' || link.origin === 'adopted') { throw new Error('agent_session_provider_handle_invalid') } @@ -214,3 +223,25 @@ export function appendAgentSessionProviderHandleLink( } return [...chain, link] } + +/** + * Replace the chain's only link, a creation the provider proved it never saved, with the creation + * that took its place. Every other head names a conversation the provider held (a resume or fork + * proved it, an adoption imported it), so only a `created` head can be superseded, and only by a + * new identity root that names it. + */ +function supersedeUnsavedCreation( + head: AgentSessionProviderHandleLink, + link: AgentSessionProviderHandleLink +): AgentSessionProviderHandleLink[] { + if ( + head.origin !== 'created' || + link.supersedesKey !== agentSessionProviderHandleKey(head.handle) || + agentSessionProviderHandleRoot(link.handle) === agentSessionProviderHandleRoot(head.handle) || + link.linkId === head.linkId + ) { + throw new Error('agent_session_provider_handle_invalid') + } + // Why: in place, so a chat reopened unused across many restarts never grows toward the cap. + return [link] +}