fix(codex): preserve resource copies on indeterminate reads

This commit is contained in:
Brennan Benson
2026-08-18 02:28:21 -07:00
parent d3598643b5
commit df93a7d4a5
3 changed files with 92 additions and 36 deletions
+12 -8
View File
@@ -1,6 +1,4 @@
import { existsSync } from 'node:fs'
import { dirname, join } from 'node:path'
import { readAgentStateFileSync } from '../agent-state-file-reader'
import { observeAgentStateFile } from './codex-path-observation'
import {
recoverInterruptedGuardedFileOperation,
@@ -118,18 +116,24 @@ export function syncSystemConfigIntoLegacySharedCodexHome(
const systemConfigPath = join(homes.systemHomePath, 'config.toml')
const runtimeConfigPath = join(homes.runtimeHomePath, 'config.toml')
recoverInterruptedGuardedFileOperation(runtimeConfigPath)
const rawSystemConfig = existsSync(systemConfigPath)
? readAgentStateFileSync(systemConfigPath)
: ''
const systemConfigObservation = observeAgentStateFile(systemConfigPath)
if (systemConfigObservation.kind === 'indeterminate') {
throw systemConfigObservation.error
}
const rawSystemConfig =
systemConfigObservation.kind === 'present' ? systemConfigObservation.value : ''
// Why: a missing cloud-synced source is not proof the user cleared config.
if (rawSystemConfig.trim() === '') {
return
}
const sourceConfigDir = resolveCodexConfigMirrorSourceDirectory(homes.systemHomePath)
const runtimeConfigBeforeMirror = existsSync(runtimeConfigPath)
? readAgentStateFileSync(runtimeConfigPath)
: null
const runtimeConfigObservation = observeAgentStateFile(runtimeConfigPath)
if (runtimeConfigObservation.kind === 'indeterminate') {
throw runtimeConfigObservation.error
}
const runtimeConfigBeforeMirror =
runtimeConfigObservation.kind === 'present' ? runtimeConfigObservation.value : null
const nextRuntimeConfig =
runtimeConfigBeforeMirror !== null
? mergeSystemCodexConfigIntoRuntime(
+25 -26
View File
@@ -17,7 +17,7 @@ import {
markCopiedResource,
targetIsOwnedFallbackCopy
} from './codex-managed-home-resource-copy-marker'
import { observeResolvedPathEntry } from './codex-path-observation'
import { observe, observeResolvedPathEntry } from './codex-path-observation'
const CODEX_GLOBAL_INSTRUCTIONS_ENTRY = 'AGENTS.md'
@@ -128,23 +128,28 @@ function linkSystemCodexResource(
return
}
}
const shouldRefreshFallbackCopy = targetIsOwnedFallbackCopy(
targetPath,
managedHomePath,
entryName,
sourcePath
)
if (pathEntryExists(targetPath) && !shouldRefreshFallbackCopy) {
// Why: an unreadable target is not a missing target; do not let the fallback
// copier remove it merely because existsSync/lstatSync collapsed the error.
const targetObservation = observe(() => lstatSync(targetPath))
if (targetObservation.kind === 'indeterminate') {
return
}
const shouldRefreshFallbackCopy =
targetObservation.kind === 'present' &&
targetIsOwnedFallbackCopy(targetPath, managedHomePath, entryName, sourcePath)
if (targetObservation.kind === 'present' && !shouldRefreshFallbackCopy) {
return
}
if (shouldRefreshFallbackCopy) {
// Why: WSL launch preparation runs before every Codex start. Avoid
// rewriting an unchanged file across the UNC boundary on every launch.
if (
entryName === CODEX_GLOBAL_INSTRUCTIONS_ENTRY &&
copiedFileContentsMatch(sourcePath, targetPath)
) {
return
if (entryName === CODEX_GLOBAL_INSTRUCTIONS_ENTRY) {
const contentsMatch = copiedFileContentsMatch(sourcePath, targetPath)
if (contentsMatch === 'match' || contentsMatch === 'indeterminate') {
// Why: a failed comparison is not permission to remove the only
// readable copy; leave it in place for the next launch.
return
}
}
rmSync(targetPath, { recursive: true, force: true })
}
@@ -214,25 +219,19 @@ function copySystemCodexResourceAsOwnedFallback(
}
}
function pathEntryExists(entryPath: string): boolean {
try {
lstatSync(entryPath)
return true
} catch {
return false
}
}
function copiedFileContentsMatch(sourcePath: string, targetPath: string): boolean {
function copiedFileContentsMatch(
sourcePath: string,
targetPath: string
): 'match' | 'different' | 'indeterminate' {
try {
// Why: reading a FIFO or device synchronously can block Codex launch.
// Follow source symlinks, but only compare two regular files.
if (!statSync(sourcePath).isFile() || !lstatSync(targetPath).isFile()) {
return false
return 'different'
}
return readFileSync(sourcePath).equals(readFileSync(targetPath))
return readFileSync(sourcePath).equals(readFileSync(targetPath)) ? 'match' : 'different'
} catch {
return false
return 'indeterminate'
}
}
@@ -18,11 +18,16 @@ import { join } from 'node:path'
const denials = vi.hoisted(() => {
const state = {
paths: new Set<string>(),
readOnlyPaths: new Set<string>(),
deny(path: string): void {
state.paths.add(path)
},
reset(): void {
state.paths.clear()
state.readOnlyPaths.clear()
},
denyReads(path: string): void {
state.readOnlyPaths.add(path)
},
has(target: unknown): boolean {
return typeof target === 'string' && state.paths.has(target)
@@ -46,7 +51,12 @@ vi.mock('node:fs', async (importOriginal) => {
const guard = (fn: unknown, syscall: string): unknown => {
const original = fn as (...args: unknown[]) => unknown
const wrapped = (...args: unknown[]): unknown => {
if (denials.has(args[0])) {
if (
denials.has(args[0]) ||
((syscall === 'read' || syscall === 'copy') &&
typeof args[0] === 'string' &&
denials.readOnlyPaths.has(args[0]))
) {
denials.fail(args[0] as string, syscall)
}
return original(...args)
@@ -56,6 +66,7 @@ vi.mock('node:fs', async (importOriginal) => {
const patched: Record<string, unknown> = {
...actual,
readFileSync: guard(actual.readFileSync, 'read'),
cpSync: guard(actual.cpSync, 'copy'),
statSync: guard(actual.statSync, 'stat'),
lstatSync: guard(actual.lstatSync, 'lstat'),
openSync: guard(actual.openSync, 'open'),
@@ -82,7 +93,8 @@ vi.mock('node:os', async () => {
const realFs = await vi.importActual<typeof NodeFs>('node:fs')
const { syncSystemConfigIntoManagedCodexHome } = await import('./codex-config-mirror')
const { syncSystemConfigIntoLegacySharedCodexHome, syncSystemConfigIntoManagedCodexHome } =
await import('./codex-config-mirror')
const { promoteCodexRuntimeSettingsToSystem, snapshotCodexRuntimeSettingsBaseline } =
await import('./config-settings-promotion')
const { syncCodexGlobalInstructionsIntoManagedHome } = await import('./codex-home-paths')
@@ -187,6 +199,20 @@ describe('STA-4737 the config mirror must not overwrite a runtime config it coul
// that refused here would break first launch for everyone.
expect(realFs.readFileSync(runtimeConfigPath(), 'utf-8')).toContain('model = "system-model"')
})
it('refuses the retained-pane mirror when its runtime config is denied', () => {
realFs.writeFileSync(systemConfigPath(), SYSTEM_CONFIG, 'utf-8')
realFs.writeFileSync(runtimeConfigPath(), RUNTIME_CONFIG, 'utf-8')
denials.deny(runtimeConfigPath())
expect(() =>
syncSystemConfigIntoLegacySharedCodexHome({
runtimeHomePath: runtimeHome(),
systemHomePath: systemHome()
})
).toThrow('EPERM')
expect(realFs.readFileSync(runtimeConfigPath(), 'utf-8')).toBe(RUNTIME_CONFIG)
})
})
describe('STA-4737 promotion must not rebuild a system config it could not read', () => {
@@ -261,6 +287,33 @@ describe('STA-4737 the resource sync must not delete a mirror whose source it co
expect(realFs.readFileSync(targetPath, 'utf-8')).toBe(MIRRORED)
})
it('keeps the mirrored copy when the source becomes unreadable after stat', () => {
const { sourcePath, targetPath } = seedOwnedMirrorCopy()
// A Windows sharing violation can leave metadata readable while content
// reads fail; stat success must not authorize deleting the fallback copy.
denials.denyReads(sourcePath)
syncCodexGlobalInstructionsIntoManagedHome({
systemHomePath: systemHome(),
managedHomePath: runtimeHome()
})
expect(realFs.readFileSync(targetPath, 'utf-8')).toBe(MIRRORED)
})
it('keeps an unreadable owned target instead of refreshing it', () => {
const { sourcePath, targetPath } = seedOwnedMirrorCopy()
realFs.writeFileSync(sourcePath, '# updated system instructions\n', 'utf-8')
denials.deny(targetPath)
syncCodexGlobalInstructionsIntoManagedHome({
systemHomePath: systemHome(),
managedHomePath: runtimeHome()
})
expect(realFs.readFileSync(targetPath, 'utf-8')).toBe(MIRRORED)
})
it('still removes the mirrored copy when the source is genuinely gone', () => {
const { sourcePath, targetPath } = seedOwnedMirrorCopy()
realFs.rmSync(sourcePath)