From 849b83f6c5d95629008e85715eda8aba212d91bb Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 23 Aug 2026 18:20:31 -0700 Subject: [PATCH] fix(runtime): preserve recovered store and cmd launch paths --- ...codex-structured-launch-resolution.test.ts | 27 ++++++ .../codex-structured-launch-resolution.ts | 6 +- .../agent-session-backup-recovery.test.ts | 6 +- .../agent-session-record-store-file.ts | 14 ++-- ...ent-session-recovery-publish-fault.test.ts | 83 +++++++++++++++++++ .../agent-session-store-transaction-queue.ts | 4 +- 6 files changed, 129 insertions(+), 11 deletions(-) create mode 100644 src/main/runtime/agent-session-recovery-publish-fault.test.ts diff --git a/src/main/codex/codex-structured-launch-resolution.test.ts b/src/main/codex/codex-structured-launch-resolution.test.ts index 3883930bdf9..da91e33e215 100644 --- a/src/main/codex/codex-structured-launch-resolution.test.ts +++ b/src/main/codex/codex-structured-launch-resolution.test.ts @@ -9,6 +9,16 @@ const IDENTITY = { sessionId: SESSION_ID } as Parameters< ReturnType >[0]['identity'] +async function withPlatform(platform: NodeJS.Platform, run: () => Promise): Promise { + const original = process.platform + Object.defineProperty(process, 'platform', { configurable: true, value: platform }) + try { + return await run() + } finally { + Object.defineProperty(process, 'platform', { configurable: true, value: original }) + } +} + function record(overrides: Partial = {}): AgentSessionRecord { return { sessionId: SESSION_ID, @@ -51,6 +61,23 @@ describe('codex structured launch resolution', () => { }) }) + it('passes a Windows .cmd path containing cmd syntax directly to the safe spawn layer', async () => { + const command = String.raw`C:\Users\r&d\npm-prefix\codex.cmd` + + await withPlatform('win32', async () => { + const resolveLaunch = createCodexStructuredLaunchResolver({ + store: { getRecord: () => record() } as unknown as AgentSessionRecordStore, + resolveWorkspacePath: async () => String.raw`C:\workspaces\orca`, + resolveCommand: () => command + }) + + await expect(resolveLaunch({ identity: IDENTITY })).resolves.toMatchObject({ + command, + args: ['app-server'] + }) + }) + }) + it('resumes the last thread this session actually proved, not one a caller names', async () => { const launch = await resolverFor( record({ diff --git a/src/main/codex/codex-structured-launch-resolution.ts b/src/main/codex/codex-structured-launch-resolution.ts index ae68be38d3f..7426c321a1c 100644 --- a/src/main/codex/codex-structured-launch-resolution.ts +++ b/src/main/codex/codex-structured-launch-resolution.ts @@ -11,7 +11,6 @@ import { agentSessionProviderHandleChainHead } from '../../shared/agent-session- import { LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host' import { resolveCodexCommand } from '../codex-cli/command' import type { AgentSessionRecordStore } from '../runtime/agent-session-record-store' -import { getSpawnArgsForWindows } from '../win32-utils' import type { CodexStructuredLaunch } from './codex-structured-session-adapter' import { resolvePinnedCodexRolloutProof } from './codex-tui-rollout-proof' @@ -59,12 +58,11 @@ export function createCodexStructuredLaunchResolver( pathEnv, ...(homePath ? { homePath } : {}) }) - const { spawnCmd, spawnArgs } = getSpawnArgsForWindows(command, CODEX_APP_SERVER_ARGS) const head = agentSessionProviderHandleChainHead(record.providerHandleChain) const resumeThreadId = head?.handle.provider === 'codex' ? head.handle.threadId : null return { - command: spawnCmd, - args: spawnArgs, + command, + args: CODEX_APP_SERVER_ARGS, cwd: await deps.resolveWorkspacePath(location.workspaceId), codexHome: accountHome.path, ...(environment ? { env: { ...environment } as Record } : {}), diff --git a/src/main/runtime/agent-session-backup-recovery.test.ts b/src/main/runtime/agent-session-backup-recovery.test.ts index a39d95e05df..18d1ec00bda 100644 --- a/src/main/runtime/agent-session-backup-recovery.test.ts +++ b/src/main/runtime/agent-session-backup-recovery.test.ts @@ -142,7 +142,11 @@ describe('crash-safe store writes', () => { const { mkdir } = await import('node:fs/promises') await mkdir(`${storePath}.bak`) - await expect(saveAgentSessionStore(storePath, loaded.state)).rejects.toBeTruthy() + await expect( + saveAgentSessionStore(storePath, loaded.state, { + primaryStatus: 'validated' + }) + ).rejects.toBeTruthy() expect(await readFile(storePath, 'utf-8')).toBe(committed) expect((await stat(`${storePath}.bak`)).isDirectory()).toBe(true) }) diff --git a/src/main/runtime/agent-session-record-store-file.ts b/src/main/runtime/agent-session-record-store-file.ts index 133a03afc4a..9068ae6a603 100644 --- a/src/main/runtime/agent-session-record-store-file.ts +++ b/src/main/runtime/agent-session-record-store-file.ts @@ -309,7 +309,8 @@ function serializeState(state: AgentSessionStoreState): string { /** * Commit the whole state. The live path is never absent: the new content is made durable in a temp - * file first, the old content is COPIED to the backup, and only then does the rename publish it. + * file first, a validated primary is COPIED to the backup, and only then does the rename publish it. + * Backup recovery keeps the known-good backup in place while publishing the repaired primary. * * The old ordering renamed the live file aside before writing the new one, so a death in that * window left the profile with a backup and no primary — which is exactly the state that wedged a @@ -317,7 +318,8 @@ function serializeState(state: AgentSessionStoreState): string { */ export async function saveAgentSessionStore( filePath: string, - state: AgentSessionStoreState + state: AgentSessionStoreState, + options: { primaryStatus: 'validated' | 'unusable-or-absent' } ): Promise { const directory = dirname(filePath) await mkdir(directory, { recursive: true, mode: 0o700 }) @@ -325,9 +327,11 @@ export async function saveAgentSessionStore( const tmpPath = durableWriteTempPath(filePath) try { await writeTempFileDurable(tmpPath, serializeState(state), 0o600) - // A failed rotation aborts the save. Recovery's fence floor assumes the backup is at most one - // committed generation behind; letting the primary advance past a stale backup breaks that. - await copyFileDurable(filePath, backupPath(filePath)) + // Only a primary parsed under the transaction lock may replace the backup. During recovery the + // primary is corrupt or absent, so the known-good backup must survive until publication. + if (options.primaryStatus === 'validated') { + await copyFileDurable(filePath, backupPath(filePath)) + } await renameDurable(tmpPath, filePath) } catch (error) { await rm(tmpPath, { force: true }).catch(() => {}) diff --git a/src/main/runtime/agent-session-recovery-publish-fault.test.ts b/src/main/runtime/agent-session-recovery-publish-fault.test.ts new file mode 100644 index 00000000000..10785bbad29 --- /dev/null +++ b/src/main/runtime/agent-session-recovery-publish-fault.test.ts @@ -0,0 +1,83 @@ +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as DurableFileWrite from '../durable-file-write' +import { + agentSessionStoreRevision, + loadAgentSessionStore, + saveAgentSessionStore, + type AgentSessionStoreState +} from './agent-session-record-store-file' +import { AgentSessionStoreTransactionQueue } from './agent-session-store-transaction-queue' + +const publishFault = vi.hoisted(() => ({ armed: false })) + +vi.mock('../durable-file-write', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + renameDurable: async (tmpPath: string, finalPath: string) => { + if (publishFault.armed) { + publishFault.armed = false + throw new Error('simulated death before primary publish') + } + return actual.renameDurable(tmpPath, finalPath) + } + } +}) + +let root: string +let storePath: string + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-store-recovery-publish-')) + storePath = join(root, 'agent-sessions.json') +}) + +afterEach(async () => { + publishFault.armed = false + await rm(root, { recursive: true, force: true }) +}) + +function state(generation: number): AgentSessionStoreState { + return { + schemaVersion: 2, + hostId: 'local', + records: new Map(), + operations: new Map(), + retiredClaimKeys: [{ keyId: `generation-${generation}`, retiredAt: generation }], + unreadableRecords: new Map() + } +} + +describe('backup recovery publication', () => { + it('keeps the valid backup when publication fails after a corrupt primary was recovered', async () => { + await saveAgentSessionStore(storePath, state(1), { primaryStatus: 'unusable-or-absent' }) + await saveAgentSessionStore(storePath, state(2), { + primaryStatus: 'validated' + }) + await writeFile(storePath, '{corrupt-primary', 'utf-8') + const knownGoodBackup = await readFile(`${storePath}.bak`, 'utf-8') + + const recovered = await loadAgentSessionStore(storePath, 'local') + expect(recovered.recoveredFromBackup).toBe(true) + expect(recovered.state.retiredClaimKeys[0]?.keyId).toBe('generation-1') + const queue = AgentSessionStoreTransactionQueue.fromLoadedStore( + storePath, + 'local', + recovered, + agentSessionStoreRevision(recovered.state) + ) + + publishFault.armed = true + await expect(queue.persistLoadedMigration()).rejects.toThrow( + 'simulated death before primary publish' + ) + + expect(await readFile(`${storePath}.bak`, 'utf-8')).toBe(knownGoodBackup) + const afterFault = await loadAgentSessionStore(storePath, 'local') + expect(afterFault.recoveredFromBackup).toBe(true) + expect(afterFault.state.retiredClaimKeys[0]?.keyId).toBe('generation-1') + }) +}) diff --git a/src/main/runtime/agent-session-store-transaction-queue.ts b/src/main/runtime/agent-session-store-transaction-queue.ts index 0da8dae355e..77e2fe5058a 100644 --- a/src/main/runtime/agent-session-store-transaction-queue.ts +++ b/src/main/runtime/agent-session-store-transaction-queue.ts @@ -116,7 +116,9 @@ export class AgentSessionStoreTransactionQueue { ) { return result } - await saveAgentSessionStore(this.filePath, this.state) + await saveAgentSessionStore(this.filePath, this.state, { + primaryStatus: this.diskStoreFound && !recovering ? 'validated' : 'unusable-or-absent' + }) this.state.schemaVersion = AGENT_SESSION_STORE_SCHEMA_VERSION this.diskRevision = agentSessionStoreRevision(this.state) this.diskRecoveredFromBackup = false