From e15425a64377beb25790149e2e2ea8858f6f812a Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 15 Jul 2026 02:04:49 -0700 Subject: [PATCH] fix(codex): install cross-volume session backfill copies atomically On a real Codex home whose filesystem supports no hardlinks (exFAT/FAT, some network mounts), the staged cross-volume copy was installed with a non-atomic copyFile(..., COPYFILE_EXCL) straight into the final rollout-*.jsonl name. An install interrupted mid-copy (app quit, crash, ENOSPC during the deferred run) could strand a truncated rollout that the next run then skips as already-present, defeating the staging design's own guarantee that a failed copy never leaves a partial session behind. Install the fully-staged copy with an atomic rename instead, guarded by an existence re-check so it keeps the never-overwrite contract (and the rename source is the same immutable managed rollout, so any clobber would be byte-identical). Cover the no-hardlink-support target and an interrupted install that must leave no partial in the user's sessions tree. --- src/main/codex/codex-session-backfill-copy.ts | 33 ++++++++-- src/main/codex/codex-session-backfill.test.ts | 66 +++++++++++++++++++ 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/src/main/codex/codex-session-backfill-copy.ts b/src/main/codex/codex-session-backfill-copy.ts index 0455eb4b0f0..bb2f7621058 100644 --- a/src/main/codex/codex-session-backfill-copy.ts +++ b/src/main/codex/codex-session-backfill-copy.ts @@ -1,6 +1,5 @@ import { randomUUID } from 'node:crypto' -import { constants } from 'node:fs' -import { copyFile, link, rm, writeFile } from 'node:fs/promises' +import { copyFile, link, lstat, rename, rm, writeFile } from 'node:fs/promises' import { dirname, join } from 'node:path' export async function copySessionFileWithoutOverwrite( @@ -21,9 +20,16 @@ export async function copySessionFileWithoutOverwrite( if (isExistsError(installLinkError)) { throw installLinkError } - // Some target filesystems do not support hardlinks at all. COPYFILE_EXCL - // preserves the collision contract while retaining the staged snapshot. - await copyFile(temporaryPath, targetPath, constants.COPYFILE_EXCL) + // Why: some target filesystems support no hardlinks at all. Install the + // fully-staged copy with an atomic rename — never a raw copy into the + // rollout filename — so an interrupted install cannot strand a truncated + // session that a later run skips as already-present. Re-check existence + // first because rename, unlike the hardlink above, would clobber a target + // that appeared mid-run. + if (await pathEntryExists(targetPath)) { + throw makeTargetExistsError(targetPath) + } + await rename(temporaryPath, targetPath) } } finally { try { @@ -36,6 +42,23 @@ export async function copySessionFileWithoutOverwrite( } } +/** Existence via lstat so a broken symlink at the target still counts as taken. */ +async function pathEntryExists(entryPath: string): Promise { + try { + await lstat(entryPath) + return true + } catch { + return false + } +} + function isExistsError(error: unknown): boolean { return (error as NodeJS.ErrnoException | null)?.code === 'EEXIST' } + +/** EEXIST so a rename-install collision routes to the skip path, not a failure. */ +function makeTargetExistsError(targetPath: string): NodeJS.ErrnoException { + const error = new Error(`EEXIST: backfill target already exists: ${targetPath}`) + ;(error as NodeJS.ErrnoException).code = 'EEXIST' + return error +} diff --git a/src/main/codex/codex-session-backfill.test.ts b/src/main/codex/codex-session-backfill.test.ts index f62296f678a..126f134502e 100644 --- a/src/main/codex/codex-session-backfill.test.ts +++ b/src/main/codex/codex-session-backfill.test.ts @@ -23,6 +23,8 @@ const { homedirMock } = vi.hoisted(() => ({ const { fsMockState } = vi.hoisted(() => ({ fsMockState: { failLink: false, + failInstallLink: false, + failInstallRename: false, failCopy: false, failDirectoryPath: null as string | null, failLstatPath: null as string | null @@ -60,8 +62,25 @@ vi.mock('node:fs/promises', async () => { error.code = 'EXDEV' throw error } + // Simulate a target filesystem with no hardlink support: even the + // same-volume staged-copy install link (.orca-backfill-*.tmp) fails. + if (fsMockState.failInstallLink && String(args[0]).includes('.orca-backfill-')) { + const error = new Error('EPERM: hardlinks unsupported') as NodeJS.ErrnoException + error.code = 'EPERM' + throw error + } return actual.link(...args) }, + rename: (...args: Parameters) => { + // Simulate an interrupted atomic install (crash/ENOSPC mid-rename) on a + // hardlink-less target, exercising the no-partial-stranded guarantee. + if (fsMockState.failInstallRename && String(args[0]).includes('.orca-backfill-')) { + const error = new Error('EIO: rename interrupted') as NodeJS.ErrnoException + error.code = 'EIO' + throw error + } + return actual.rename(...args) + }, copyFile: async (...args: Parameters) => { if (fsMockState.failCopy) { // Simulate a copy that fails after opening its destination, which is @@ -134,6 +153,8 @@ function readAuditActions(): string[] { beforeEach(() => { fsMockState.failLink = false + fsMockState.failInstallLink = false + fsMockState.failInstallRename = false fsMockState.failCopy = false fsMockState.failDirectoryPath = null fsMockState.failLstatPath = null @@ -289,6 +310,51 @@ describe('backfillManagedCodexSessionsIntoSystemHome', () => { expect(readAuditActions()).toEqual(['copy', 'run-summary']) }) + it('installs via atomic rename when the target volume has no hardlink support', async () => { + // Why: exFAT/FAT and some network targets support no hardlinks, so even the + // staged-copy install link fails and the atomic-rename fallback must run. + fsMockState.failLink = true + fsMockState.failInstallLink = true + const managedPath = writeManagedSession( + join('2026', '05', '26', 'rollout-a.jsonl'), + '{"id":"a"}\n' + ) + + const summary = await backfillManagedCodexSessionsIntoSystemHome( + resolveCodexSessionBackfillPaths() + ) + + expect(summary).toMatchObject({ linkedFiles: 0, copiedFiles: 1, failedFiles: 0 }) + const targetPath = join(getSystemSessionsRoot(), '2026', '05', '26', 'rollout-a.jsonl') + expect(readFileSync(targetPath, 'utf-8')).toBe(readFileSync(managedPath, 'utf-8')) + // Staged temp file is renamed into place, not stranded in the sessions tree. + expect( + readdirSync(dirname(targetPath)).filter((name) => name.includes('.orca-backfill-')) + ).toEqual([]) + expect(readAuditActions()).toEqual(['copy', 'run-summary']) + }) + + it('never strands a partial rollout when the atomic install is interrupted', async () => { + // Why: a hardlink-less target plus an interrupted install must leave the + // real rollout filename absent (not truncated), so the next run retries + // instead of skipping a partial session as already-present. + fsMockState.failLink = true + fsMockState.failInstallLink = true + fsMockState.failInstallRename = true + writeManagedSession(join('2026', '05', '26', 'rollout-a.jsonl'), '{"id":"a"}\n') + + const summary = await backfillManagedCodexSessionsIntoSystemHome( + resolveCodexSessionBackfillPaths() + ) + + expect(summary).toMatchObject({ failedFiles: 1, copiedFiles: 0, linkedFiles: 0 }) + const targetPath = join(getSystemSessionsRoot(), '2026', '05', '26', 'rollout-a.jsonl') + expect(existsSync(targetPath)).toBe(false) + // No partial rollout and no leftover staging file in the user's tree. + expect(readdirSync(dirname(targetPath))).toEqual([]) + expect(readAuditActions()).toEqual(['failed', 'run-summary']) + }) + it('records per-file failures without aborting the run', async () => { fsMockState.failLink = true fsMockState.failCopy = true