fix(codex): fail closed on unsafe backfill install

This commit is contained in:
Brennan Benson
2026-07-16 00:53:31 -07:00
parent 8ce03b4652
commit 9ecaee715e
3 changed files with 87 additions and 66 deletions
+32 -24
View File
@@ -1,7 +1,9 @@
import { randomUUID } from 'node:crypto'
import { copyFile, link, lstat, rename, rm, writeFile } from 'node:fs/promises'
import { copyFile, link, rm, writeFile } from 'node:fs/promises'
import { dirname, join } from 'node:path'
const ATOMIC_NO_REPLACE_UNSUPPORTED_CODE = 'ORCA_ATOMIC_NO_REPLACE_UNSUPPORTED'
export async function copySessionFileWithoutOverwrite(
sourcePath: string,
targetPath: string
@@ -20,16 +22,12 @@ export async function copySessionFileWithoutOverwrite(
if (isExistsError(installLinkError)) {
throw installLinkError
}
// 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)
if (!isHardlinkUnsupportedError(installLinkError)) {
throw installLinkError
}
await rename(temporaryPath, targetPath)
// Why: Node has no portable atomic rename-if-absent. Fail closed on a
// hardlink-less target instead of risking replacement of a concurrent file.
throw makeAtomicNoReplaceUnsupportedError(targetPath, installLinkError)
}
} finally {
try {
@@ -42,23 +40,33 @@ export async function copySessionFileWithoutOverwrite(
}
}
/** Existence via lstat so a broken symlink at the target still counts as taken. */
async function pathEntryExists(entryPath: string): Promise<boolean> {
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'
function isHardlinkUnsupportedError(error: unknown): boolean {
const code = (error as NodeJS.ErrnoException | null)?.code
return (
code === 'EPERM' ||
code === 'EACCES' ||
code === 'ENOTSUP' ||
code === 'EOPNOTSUPP' ||
code === 'ENOSYS'
)
}
function makeAtomicNoReplaceUnsupportedError(
targetPath: string,
cause: unknown
): NodeJS.ErrnoException {
const error = new Error(
`Cannot atomically install backfill without overwrite on this filesystem: ${targetPath}`,
{ cause }
) as NodeJS.ErrnoException
error.code = ATOMIC_NO_REPLACE_UNSUPPORTED_CODE
return error
}
export function isAtomicNoReplaceUnsupportedError(error: unknown): boolean {
return (error as NodeJS.ErrnoException | null)?.code === ATOMIC_NO_REPLACE_UNSUPPORTED_CODE
}
+40 -41
View File
@@ -24,7 +24,7 @@ const { fsMockState } = vi.hoisted(() => ({
fsMockState: {
failLink: false,
failInstallLink: false,
failInstallRename: false,
failInstallLinkTransiently: false,
failCopy: false,
failAuditMkdirOnce: false,
failDirectoryPath: null as string | null,
@@ -81,17 +81,12 @@ vi.mock('node:fs/promises', async () => {
error.code = 'EPERM'
throw error
}
return actual.link(...args)
},
rename: (...args: Parameters<typeof actual.rename>) => {
// 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
if (fsMockState.failInstallLinkTransiently && String(args[0]).includes('.orca-backfill-')) {
const error = new Error('EIO: transient install failure') as NodeJS.ErrnoException
error.code = 'EIO'
throw error
}
return actual.rename(...args)
return actual.link(...args)
},
copyFile: async (...args: Parameters<typeof actual.copyFile>) => {
if (fsMockState.failCopy) {
@@ -166,7 +161,7 @@ function readAuditActions(): string[] {
beforeEach(() => {
fsMockState.failLink = false
fsMockState.failInstallLink = false
fsMockState.failInstallRename = false
fsMockState.failInstallLinkTransiently = false
fsMockState.failCopy = false
fsMockState.failAuditMkdirOnce = false
fsMockState.failDirectoryPath = null
@@ -335,48 +330,40 @@ 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.
it('fails closed when the target filesystem cannot install without overwrite', async () => {
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 })
expect(summary).toMatchObject({
linkedFiles: 0,
copiedFiles: 0,
skippedUnsupportedFilesystemFiles: 1,
failedFiles: 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(['copy-unsupported', 'run-summary'])
})
it('keeps transient install failures retryable', async () => {
fsMockState.failLink = true
fsMockState.failInstallLinkTransiently = true
writeManagedSession(join('2026', '05', '26', 'rollout-a.jsonl'), '{"id":"a"}\n')
const summary = await backfillManagedCodexSessionsIntoSystemHome(
resolveCodexSessionBackfillPaths()
)
expect(summary).toMatchObject({
skippedUnsupportedFilesystemFiles: 0,
failedFiles: 1
})
expect(readAuditActions()).toEqual(['failed', 'run-summary'])
})
@@ -423,6 +410,18 @@ describe('startCodexSessionBackfillInBackground', () => {
).toBe(false)
})
it('does not retry a stable hardlink-less filesystem limitation', async () => {
fsMockState.failLink = true
fsMockState.failInstallLink = true
writeManagedSession(join('2026', '05', '26', 'rollout-a.jsonl'), '{"id":"a"}\n')
const first = await startCodexSessionBackfillInBackground()
expect(first).toMatchObject({ skippedUnsupportedFilesystemFiles: 1, failedFiles: 0 })
expect(existsSync(getMarkerPath())).toBe(true)
expect(await startCodexSessionBackfillInBackground()).toBeNull()
})
it('leaves the marker unset when any file fails so the next startup retries', async () => {
fsMockState.failLink = true
fsMockState.failCopy = true
+15 -1
View File
@@ -11,7 +11,10 @@ import {
createCodexSessionBackfillAuditWriter,
type CodexSessionBackfillAuditWriter
} from './codex-session-backfill-audit'
import { copySessionFileWithoutOverwrite } from './codex-session-backfill-copy'
import {
copySessionFileWithoutOverwrite,
isAtomicNoReplaceUnsupportedError
} from './codex-session-backfill-copy'
import { listCodexSessionJsonlFilesIncrementally } from './codex-session-file-listing'
import type { CodexSessionBridgeIncrementalOptions } from './codex-session-file-listing'
@@ -26,6 +29,7 @@ export type CodexSessionBackfillSummary = {
skippedExistingFiles: number
skippedUnexpectedFiles: number
skippedSymlinkFiles: number
skippedUnsupportedFilesystemFiles: number
failedDirectories: number
failedFiles: number
}
@@ -122,6 +126,7 @@ export async function backfillManagedCodexSessionsIntoSystemHome(
skippedExistingFiles: 0,
skippedUnexpectedFiles: 0,
skippedSymlinkFiles: 0,
skippedUnsupportedFilesystemFiles: 0,
failedDirectories: 0,
failedFiles: 0
}
@@ -263,6 +268,15 @@ async function backfillOneManagedSessionFile(
summary.skippedExistingFiles += 1
return
}
if (isAtomicNoReplaceUnsupportedError(copyError)) {
summary.skippedUnsupportedFilesystemFiles += 1
await appendAuditRecord({
action: 'copy-unsupported',
source: managedSessionFilePath,
target: systemSessionFilePath
})
return
}
summary.failedFiles += 1
await appendAuditRecord({
action: 'failed',