diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 855d1e988b0..42b3d0772d0 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -40,6 +40,7 @@ "../src/main/codex/codex-config-settings-upsert.ts", "../src/main/codex/codex-home-paths.ts", "../src/main/codex/codex-managed-home-resource-copy-marker.ts", + "../src/main/codex/codex-path-observation.ts", "../src/main/codex/codex-hook-identity.ts", "../src/main/codex/codex-hook-trust-grant.ts", "../src/main/codex/codex-managed-trust-reconciliation.ts", @@ -55,6 +56,7 @@ "../src/main/codex/config-settings-baseline.ts", "../src/main/codex/config-settings-conflict-resolution.ts", "../src/main/codex/config-settings-promotion.ts", + "../src/main/codex/config-settings-promotion-write-target.ts", "../src/main/codex/config-sync-stall.ts", "../src/main/codex/config-toml-deprecated-hook-flag.ts", "../src/main/codex/config-toml-key-path.ts", diff --git a/src/main/codex-accounts/host-codex-managed-home-ownership.ts b/src/main/codex-accounts/host-codex-managed-home-ownership.ts index 8dfebe8df1d..1820325247c 100644 --- a/src/main/codex-accounts/host-codex-managed-home-ownership.ts +++ b/src/main/codex-accounts/host-codex-managed-home-ownership.ts @@ -1,5 +1,6 @@ import { lstatSync, readFileSync, realpathSync, statSync } from 'node:fs' import { isAbsolute, join, relative, resolve, sep } from 'node:path' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' type HostCodexManagedHomeOwnershipOptions = { candidatePath: string @@ -40,11 +41,6 @@ export class ManagedCodexHomeTemporarilyUnavailableError extends Error { } } -function isDefinitiveAbsence(error: unknown): boolean { - const code = (error as NodeJS.ErrnoException | null)?.code - return code === 'ENOENT' || code === 'ENOTDIR' -} - function pathsEqual(left: string, right: string): boolean { const resolvedLeft = resolve(left) const resolvedRight = resolve(right) diff --git a/src/main/codex/codex-config-mirror.ts b/src/main/codex/codex-config-mirror.ts index 10ede4d9cca..4e8f5ccc197 100644 --- a/src/main/codex/codex-config-mirror.ts +++ b/src/main/codex/codex-config-mirror.ts @@ -1,6 +1,7 @@ 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, writeFileAtomically, @@ -67,6 +68,17 @@ export function syncSystemConfigIntoManagedCodexHome( reportCodexConfigSyncOutcome(homes.runtimeHomePath, getCodexConfigSyncStatus(homes), error) return } + if (mirrorResult.status === 'refused-indeterminate') { + // Why: no mirror ran, so this must behave exactly like the throwing path + // above — surface the reason and advance nothing. Advancing the baseline + // here would record an unmirrored runtime change as promoted and strand it. + reportCodexConfigSyncOutcome( + homes.runtimeHomePath, + getCodexConfigSyncStatus(homes), + mirrorResult.error + ) + return + } // Why: report from the same pass that decided, so the surfaced status can // never disagree with what the mirror actually did. reportCodexConfigSyncOutcome(homes.runtimeHomePath, getCodexConfigSyncStatus(homes)) @@ -135,6 +147,7 @@ export function syncSystemConfigIntoLegacySharedCodexHome( type CodexConfigMirrorResult = | { status: 'skipped-missing-source' } + | { status: 'refused-indeterminate'; error: unknown } | { status: 'mirrored'; preservedConflictKeys: ReadonlySet } function syncSystemConfigIntoManagedCodexHomeUnsafe( @@ -143,9 +156,21 @@ function syncSystemConfigIntoManagedCodexHomeUnsafe( ): CodexConfigMirrorResult { const systemConfigPath = join(systemHomePath, 'config.toml') const runtimeConfigPath = join(runtimeHomePath, 'config.toml') - const systemConfigExists = existsSync(systemConfigPath) - const runtimeConfigExists = existsSync(runtimeConfigPath) - const rawSystemConfig = systemConfigExists ? readAgentStateFileSync(systemConfigPath) : '' + // Why: `existsSync` answered `false` for a locked file exactly as for an absent + // one, so a held handle on the RUNTIME config read as "no runtime config yet" + // and the fresh-mirror branch below overwrote it wholesale. Neither side may + // be acted on unless it was actually observed. + const systemConfigObservation = observeAgentStateFile(systemConfigPath) + if (systemConfigObservation.kind === 'indeterminate') { + return { status: 'refused-indeterminate', error: systemConfigObservation.error } + } + const runtimeConfigObservation = observeAgentStateFile(runtimeConfigPath) + if (runtimeConfigObservation.kind === 'indeterminate') { + return { status: 'refused-indeterminate', error: runtimeConfigObservation.error } + } + const runtimeConfigExists = runtimeConfigObservation.kind === 'present' + const rawSystemConfig = + systemConfigObservation.kind === 'present' ? systemConfigObservation.value : '' // Why: a missing or blank source is not an authoritative empty config. Merging // it would erase every ordinary setting from an existing managed runtime, and // a 0-byte file is what a half-written or unhydrated cloud-synced home shows. @@ -165,7 +190,9 @@ function syncSystemConfigIntoManagedCodexHomeUnsafe( } const systemConfig = prepareSystemConfigForRuntimeMirror(rawSystemConfig, sourceConfigDir) - const runtimeConfig = readAgentStateFileSync(runtimeConfigPath) + // Why: reuse the bytes already observed above rather than re-reading. A second + // read could succeed where the first failed and re-open the gap this closes. + const runtimeConfig = runtimeConfigObservation.value const preserved = preserveRuntimeConflictValues( mergeSystemCodexConfigIntoRuntime(runtimeConfig, systemConfig), promotionPlan.runtimeValuesToPreserve diff --git a/src/main/codex/codex-home-paths.ts b/src/main/codex/codex-home-paths.ts index 5402a316d0e..e7fe51c42fc 100644 --- a/src/main/codex/codex-home-paths.ts +++ b/src/main/codex/codex-home-paths.ts @@ -1,6 +1,5 @@ import { cpSync, - existsSync, lstatSync, mkdirSync, readFileSync, @@ -18,6 +17,7 @@ import { markCopiedResource, targetIsOwnedFallbackCopy } from './codex-managed-home-resource-copy-marker' +import { observeResolvedPathEntry } from './codex-path-observation' const CODEX_GLOBAL_INSTRUCTIONS_ENTRY = 'AGENTS.md' @@ -103,11 +103,20 @@ function linkSystemCodexResource( ): void { const sourcePath = join(systemHomePath, entryName) const targetPath = join(managedHomePath, entryName) - if (!existsSync(sourcePath)) { + // Why: both branches below DELETE Orca's mirrored copy because the system + // resource "is not there". `existsSync` and the old `catch { return false }` + // both reported that for a source we merely could not read, so one denied + // read on ~/.codex/AGENTS.md removed the managed copy on the next launch. + // One resolved stat now answers reachability and regular-file-ness together. + const sourceObservation = observeResolvedPathEntry(sourcePath) + if (sourceObservation.kind === 'indeterminate') { + return + } + if (sourceObservation.kind === 'absent') { removeCopiedResourceIfOwned(targetPath, managedHomePath, entryName, sourcePath) return } - if (entryName === CODEX_GLOBAL_INSTRUCTIONS_ENTRY && !systemResourceIsRegularFile(sourcePath)) { + if (entryName === CODEX_GLOBAL_INSTRUCTIONS_ENTRY && !sourceObservation.value.isFile()) { removeCopiedResourceIfOwned(targetPath, managedHomePath, entryName, sourcePath) console.warn('[codex-home] Ignoring non-file system Codex resource:', entryName) return @@ -205,14 +214,6 @@ function copySystemCodexResourceAsOwnedFallback( } } -function systemResourceIsRegularFile(sourcePath: string): boolean { - try { - return statSync(sourcePath).isFile() - } catch { - return false - } -} - function pathEntryExists(entryPath: string): boolean { try { lstatSync(entryPath) diff --git a/src/main/codex/codex-path-observation.ts b/src/main/codex/codex-path-observation.ts new file mode 100644 index 00000000000..5613dd9c336 --- /dev/null +++ b/src/main/codex/codex-path-observation.ts @@ -0,0 +1,43 @@ +import { statSync, type Stats } from 'node:fs' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' +import { readAgentStateFileSync } from '../agent-state-file-reader' + +/** + * A filesystem read either answered, definitively answered "not there", or + * failed to answer at all. + * + * `existsSync` collapses the last two into `false`, and a `catch` that returns a + * default collapses them into the default. That is how a held lock comes to + * authorise an overwrite or a delete: the caller reads "this file is not there" + * from evidence that says only "I could not look". + */ +export type CodexPathObservation = + | { kind: 'present'; value: T } + | { kind: 'absent' } + | { kind: 'indeterminate'; error: unknown } + +/** + * Classify any read. Only `ENOENT`/`ENOTDIR` are absence — every other errno, + * including unrecognised ones, is indeterminate. Mapping an unknown errno to a + * verdict is the category error this module exists to prevent. + */ +export function observe(read: () => T): CodexPathObservation { + try { + return { kind: 'present', value: read() } + } catch (error) { + return isDefinitiveAbsence(error) ? { kind: 'absent' } : { kind: 'indeterminate', error } + } +} + +/** Why: one call replaces the `existsSync` + read pair, closing its TOCTOU window too. */ +export function observeAgentStateFile(filePath: string): CodexPathObservation { + return observe(() => readAgentStateFileSync(filePath)) +} + +/** + * `stat`, so a symlink resolves to its target — the same reachability question + * `existsSync` answers, but with the failure kept distinct from the absence. + */ +export function observeResolvedPathEntry(entryPath: string): CodexPathObservation { + return observe(() => statSync(entryPath)) +} diff --git a/src/main/codex/config-settings-promotion-write-target.ts b/src/main/codex/config-settings-promotion-write-target.ts new file mode 100644 index 00000000000..8759aaa65a0 --- /dev/null +++ b/src/main/codex/config-settings-promotion-write-target.ts @@ -0,0 +1,46 @@ +import { lstatSync, readlinkSync, realpathSync, statSync } from 'node:fs' +import { dirname, resolve } from 'node:path' + +/** + * Where a promotion write actually lands. `~/.codex/config.toml` is commonly a + * dotfile-manager symlink, so the write has to follow it — including when the + * link is dangling, which `realpathSync` cannot resolve. + */ +export function resolvePromotionWriteTarget(systemTomlPath: string): { + path: string + mode: number +} { + try { + const realPath = realpathSync(systemTomlPath) + return { path: realPath, mode: statSync(realPath).mode & 0o777 } + } catch { + // Continue below: realpath also fails for a valid dangling dotfile link. + } + try { + if (lstatSync(systemTomlPath).isSymbolicLink()) { + const targetPath = resolveDanglingSymlinkTarget(systemTomlPath) + return { path: targetPath, mode: 0o600 } + } + } catch { + // Missing non-link targets are created owner-only at the requested path. + } + return { path: systemTomlPath, mode: 0o600 } +} + +function resolveDanglingSymlinkTarget(linkPath: string): string { + let currentPath = linkPath + const visited = new Set() + while (!visited.has(currentPath)) { + visited.add(currentPath) + try { + if (!lstatSync(currentPath).isSymbolicLink()) { + return currentPath + } + currentPath = resolve(dirname(currentPath), readlinkSync(currentPath)) + } catch { + return currentPath + } + } + // Why: replacing any link in a cycle would destroy dotfile-manager state; abort instead. + throw new Error(`Codex config symlink cycle at ${linkPath}`) +} diff --git a/src/main/codex/config-settings-promotion.ts b/src/main/codex/config-settings-promotion.ts index 76785369fb5..cd0b703960a 100644 --- a/src/main/codex/config-settings-promotion.ts +++ b/src/main/codex/config-settings-promotion.ts @@ -1,14 +1,7 @@ -import { - existsSync, - lstatSync, - mkdirSync, - readlinkSync, - realpathSync, - statSync, - writeFileSync -} from 'node:fs' +import { mkdirSync, writeFileSync } from 'node:fs' import { dirname, join, resolve } from 'node:path' -import { readAgentStateFileSync } from '../agent-state-file-reader' +import { observeAgentStateFile } from './codex-path-observation' +import { resolvePromotionWriteTarget } from './config-settings-promotion-write-target' import { writeFileAtomically } from '../codex-accounts/fs-utils' import { parseWslUncPath } from '../../shared/wsl-paths' import { getOrcaManagedCodexHomePath, getSystemCodexHomePath } from './codex-home-paths' @@ -112,10 +105,17 @@ function matchPromotedStructuredKey( // the [tui] body, and [profiles.*]/other tables are still ignored. function readPromotedSettingValues(configPath: string): Map { const result = new Map() - if (!existsSync(configPath)) { + // Why: an unreadable config held no settings only in the sense that we could + // not read them. Returning an empty map says the user cleared every promoted + // value, and the write below then acts on that. + const observation = observeAgentStateFile(configPath) + if (observation.kind === 'absent') { return result } - const lines = readAgentStateFileSync(configPath).split('\n') + if (observation.kind === 'indeterminate') { + throw observation.error + } + const lines = observation.value.split('\n') let state = createTomlLineScanState() let inPreamble = true let tuiTableSeen = false @@ -224,9 +224,16 @@ function promoteCodexRuntimeSettingsToSystemUnsafe( if (resolve(runtimeTomlPath) === resolve(systemTomlPath)) { return emptyPromotionPlan() } - if (!existsSync(runtimeTomlPath)) { + const runtimeTomlObservation = observeAgentStateFile(runtimeTomlPath) + if (runtimeTomlObservation.kind === 'absent') { return emptyPromotionPlan() } + if (runtimeTomlObservation.kind === 'indeterminate') { + // Why: the caller turns a throw into the existing "stall and retry" null. An + // empty plan here would instead let the mirror proceed against a runtime + // config nobody read. + throw runtimeTomlObservation.error + } // Why: without a baseline, a stale runtime value looks like a fresh in-Codex change; skip until the mirror writes one. const baseline = readCodexSettingsBaseline(runtimeHomePath) if (!baseline) { @@ -253,14 +260,27 @@ function promoteCodexRuntimeSettingsToSystemUnsafe( const writeTarget = resolvePromotionWriteTarget(systemTomlPath) // Why: a dangling symlink may target an unmade dir tree; create its real parent so the atomic temp write has a home. mkdirSync(dirname(writeTarget.path), { recursive: true, mode: 0o700 }) - const targetExists = existsSync(writeTarget.path) + // Why: this is the user's real ~/.codex/config.toml, and `existsSync` reading + // `false` for a locked file sent it down the reconstruct branch below, which + // replaces the canonical config with settings derived from Orca's runtime + // copy. One read replaces the old existsSync + read pair and its TOCTOU gap. + // The indeterminate arm is a backstop rather than the live guard: an + // unreadable system config already refused in readPromotedSettingValues, + // because `writeTarget.path` always resolves to the same file as + // `systemTomlPath` (its realpath, its dangling-link target, or itself). + const writeTargetObservation = observeAgentStateFile(writeTarget.path) + if (writeTargetObservation.kind === 'indeterminate') { + throw writeTargetObservation.error + } + const targetExists = writeTargetObservation.kind === 'present' // Why: seeding a brand-new ~/.codex/config.toml from the promoted keys alone // would leave a skeleton the next mirror treats as authoritative, deleting // every other runtime setting (mcp_servers, features). With no system config // the runtime IS the user's config, so carry its ordinary settings across. - const systemContent = targetExists - ? readAgentStateFileSync(writeTarget.path) - : extractOrdinaryCodexSettings(readAgentStateFileSync(runtimeTomlPath)) + const systemContent = + writeTargetObservation.kind === 'present' + ? writeTargetObservation.value + : extractOrdinaryCodexSettings(runtimeTomlObservation.value) const nextContent = upsertPromotedSettingsInContent(systemContent, updates) if (nextContent === systemContent) { return { conflicts, runtimeValuesToPreserve } @@ -329,38 +349,3 @@ function emptyPromotionPlan(): CodexSettingsPromotionPlan { } // Why: follow an existing dotfile-manager symlink and carry its mode forward so an atomic write can't widen a 0600 config. -function resolvePromotionWriteTarget(systemTomlPath: string): { path: string; mode: number } { - try { - const realPath = realpathSync(systemTomlPath) - return { path: realPath, mode: statSync(realPath).mode & 0o777 } - } catch { - // Continue below: realpath also fails for a valid dangling dotfile link. - } - try { - if (lstatSync(systemTomlPath).isSymbolicLink()) { - const targetPath = resolveDanglingSymlinkTarget(systemTomlPath) - return { path: targetPath, mode: 0o600 } - } - } catch { - // Missing non-link targets are created owner-only at the requested path. - } - return { path: systemTomlPath, mode: 0o600 } -} - -function resolveDanglingSymlinkTarget(linkPath: string): string { - let currentPath = linkPath - const visited = new Set() - while (!visited.has(currentPath)) { - visited.add(currentPath) - try { - if (!lstatSync(currentPath).isSymbolicLink()) { - return currentPath - } - currentPath = resolve(dirname(currentPath), readlinkSync(currentPath)) - } catch { - return currentPath - } - } - // Why: replacing any link in a cycle would destroy dotfile-manager state; abort instead. - throw new Error(`Codex config symlink cycle at ${linkPath}`) -} diff --git a/src/main/codex/sta-4737-unreadable-is-not-absent.test.ts b/src/main/codex/sta-4737-unreadable-is-not-absent.test.ts new file mode 100644 index 00000000000..794c13de893 --- /dev/null +++ b/src/main/codex/sta-4737-unreadable-is-not-absent.test.ts @@ -0,0 +1,277 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as NodeFs from 'node:fs' +import type * as CodexSettingsPromotion from './config-settings-promotion' +import type * as NodeOs from 'node:os' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +// STA-4737: three shared Codex modules decided a file was absent from a read +// that had merely failed, and then overwrote or deleted it. These are the paths +// where the collapse costs the user data, not just a retry. + +/** + * One denial, modelled coherently across every call that can observe it. A path + * under an ACL denial throws from `readFileSync`/`statSync` AND reports `false` + * from `existsSync`. Faulting one call but not another lets a test pass against + * code that still consults the one left healthy. + */ +const denials = vi.hoisted(() => { + const state = { + paths: new Set(), + deny(path: string): void { + state.paths.add(path) + }, + reset(): void { + state.paths.clear() + }, + has(target: unknown): boolean { + return typeof target === 'string' && state.paths.has(target) + }, + fail(target: string, syscall: string): never { + const error: NodeJS.ErrnoException = new Error( + `EPERM: operation not permitted, ${syscall} '${target}'` + ) + error.code = 'EPERM' + error.errno = -4048 + error.syscall = syscall + error.path = target + throw error + } + } + return state +}) + +vi.mock('node:fs', async (importOriginal) => { + const actual = await 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])) { + denials.fail(args[0] as string, syscall) + } + return original(...args) + } + return Object.assign(wrapped, original) + } + const patched: Record = { + ...actual, + readFileSync: guard(actual.readFileSync, 'read'), + statSync: guard(actual.statSync, 'stat'), + lstatSync: guard(actual.lstatSync, 'lstat'), + openSync: guard(actual.openSync, 'open'), + existsSync: Object.assign( + (...args: unknown[]): boolean => + denials.has(args[0]) ? false : actual.existsSync(args[0] as string), + actual.existsSync + ) + } + return { ...patched, default: patched } +}) + +const { getPathMock, homedirMock } = vi.hoisted(() => ({ + getPathMock: vi.fn<(name: string) => string>(), + homedirMock: vi.fn<() => string>() +})) + +vi.mock('electron', () => ({ app: { getPath: getPathMock } })) + +vi.mock('node:os', async () => { + const actual = await vi.importActual('node:os') + return { ...actual, homedir: homedirMock } +}) + +const realFs = await vi.importActual('node:fs') + +const { syncSystemConfigIntoManagedCodexHome } = await import('./codex-config-mirror') +const { promoteCodexRuntimeSettingsToSystem, snapshotCodexRuntimeSettingsBaseline } = + await import('./config-settings-promotion') +const { syncCodexGlobalInstructionsIntoManagedHome } = await import('./codex-home-paths') +const { markCopiedResource } = await import('./codex-managed-home-resource-copy-marker') + +let fakeHomeDir: string +let userDataDir: string +let previousUserDataPath: string | undefined + +const systemHome = (): string => join(fakeHomeDir, '.codex') +const systemConfigPath = (): string => join(systemHome(), 'config.toml') +const runtimeHome = (): string => join(userDataDir, 'codex-runtime-home', 'home') +const runtimeConfigPath = (): string => join(runtimeHome(), 'config.toml') + +beforeEach(() => { + denials.reset() + fakeHomeDir = realFs.mkdtempSync(join(tmpdir(), 'orca-sta4737-home-')) + userDataDir = realFs.mkdtempSync(join(tmpdir(), 'orca-sta4737-data-')) + previousUserDataPath = process.env.ORCA_USER_DATA_PATH + process.env.ORCA_USER_DATA_PATH = userDataDir + homedirMock.mockReturnValue(fakeHomeDir) + getPathMock.mockImplementation((name: string) => { + if (name === 'userData') { + return userDataDir + } + throw new Error(`unexpected app.getPath(${name})`) + }) + realFs.mkdirSync(systemHome(), { recursive: true }) + realFs.mkdirSync(runtimeHome(), { recursive: true }) +}) + +afterEach(() => { + denials.reset() + realFs.rmSync(fakeHomeDir, { recursive: true, force: true }) + realFs.rmSync(userDataDir, { recursive: true, force: true }) + if (previousUserDataPath === undefined) { + delete process.env.ORCA_USER_DATA_PATH + } else { + process.env.ORCA_USER_DATA_PATH = previousUserDataPath + } + vi.clearAllMocks() +}) + +describe('STA-4737 the config mirror must not overwrite a runtime config it could not read', () => { + const SYSTEM_CONFIG = 'model = "system-model"\n' + const RUNTIME_CONFIG = 'model = "user-picked"\n\n[mcp_servers.local]\ncommand = "run-me"\n' + + // Why: with no baseline, promotion returns an empty plan without reading the + // runtime config at all, so the mirror is the only thing standing between a + // denied read and an overwrite. This is the ordinary state for a runtime home + // seeded outside the mirror. + it('leaves a denied runtime config exactly as it found it', () => { + realFs.writeFileSync(systemConfigPath(), SYSTEM_CONFIG, 'utf-8') + realFs.writeFileSync(runtimeConfigPath(), RUNTIME_CONFIG, 'utf-8') + denials.deny(runtimeConfigPath()) + + syncSystemConfigIntoManagedCodexHome() + + // Before the fix the mirror read "no runtime config yet" and replaced this + // whole file with a fresh copy of the system one. + expect(realFs.readFileSync(runtimeConfigPath(), 'utf-8')).toBe(RUNTIME_CONFIG) + }) + + // Why: promotion runs first and refuses on the same denied file, so it alone + // covers the case above. This drives the mirror's OWN guard, which is what + // stands between a write and a file that became unreadable in the window + // between promotion reading it and the mirror rewriting it — the exact shape + // of a scanner taking the file mid-pass. + it('refuses its own write when the runtime config is denied after promotion succeeded', async () => { + realFs.writeFileSync(systemConfigPath(), SYSTEM_CONFIG, 'utf-8') + realFs.writeFileSync(runtimeConfigPath(), RUNTIME_CONFIG, 'utf-8') + + vi.resetModules() + vi.doMock('./config-settings-promotion', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + // Promotion already read the file successfully and had nothing to do. + promoteCodexRuntimeSettingsToSystem: () => ({ + conflicts: new Map(), + runtimeValuesToPreserve: new Map() + }) + } + }) + try { + const mirror = await import('./codex-config-mirror') + denials.deny(runtimeConfigPath()) + mirror.syncSystemConfigIntoManagedCodexHome() + expect(realFs.readFileSync(runtimeConfigPath(), 'utf-8')).toBe(RUNTIME_CONFIG) + } finally { + vi.doUnmock('./config-settings-promotion') + vi.resetModules() + } + }) + + it('still seeds a runtime config that is genuinely absent', () => { + realFs.writeFileSync(systemConfigPath(), SYSTEM_CONFIG, 'utf-8') + + syncSystemConfigIntoManagedCodexHome() + + // Why: absence is a real answer, and seeding it is the mirror's job. A fix + // that refused here would break first launch for everyone. + expect(realFs.readFileSync(runtimeConfigPath(), 'utf-8')).toContain('model = "system-model"') + }) +}) + +describe('STA-4737 promotion must not rebuild a system config it could not read', () => { + function seedPromotableRuntimeChange(): void { + realFs.writeFileSync(systemConfigPath(), 'model = "old-model"\n', 'utf-8') + // The baseline records what Orca last mirrored; the later edit then reads as + // an in-Codex change that must be promoted back to ~/.codex. + realFs.writeFileSync(runtimeConfigPath(), 'model = "old-model"\n', 'utf-8') + snapshotCodexRuntimeSettingsBaseline(runtimeHome()) + realFs.writeFileSync(runtimeConfigPath(), 'model = "new-model"\n', 'utf-8') + } + + it('leaves a denied system config exactly as it found it', () => { + seedPromotableRuntimeChange() + const before = realFs.readFileSync(systemConfigPath(), 'utf-8') + denials.deny(systemConfigPath()) + + // Why: the caller turns a refusal into its existing "stall and retry" null. + expect( + promoteCodexRuntimeSettingsToSystem({ + runtimeHomePath: runtimeHome(), + systemHomePath: systemHome() + }) + ).toBeNull() + + // Before the fix, the unreadable config counted as absent and was replaced + // by a reconstruction built from Orca's runtime copy. + expect(realFs.readFileSync(systemConfigPath(), 'utf-8')).toBe(before) + }) + + it('still promotes into a readable system config', () => { + seedPromotableRuntimeChange() + + const plan = promoteCodexRuntimeSettingsToSystem({ + runtimeHomePath: runtimeHome(), + systemHomePath: systemHome() + }) + + // Why: the anchor. Without it, "returns null and writes nothing" would pass + // against a promotion that had simply stopped working. + expect(plan).not.toBeNull() + expect(realFs.readFileSync(systemConfigPath(), 'utf-8')).toContain('model = "new-model"') + }) +}) + +describe('STA-4737 the resource sync must not delete a mirror whose source it could not read', () => { + const AGENTS_ENTRY = 'AGENTS.md' + const MIRRORED = '# instructions Orca copied for the distro\n' + + function seedOwnedMirrorCopy(): { sourcePath: string; targetPath: string } { + const sourcePath = join(systemHome(), AGENTS_ENTRY) + const targetPath = join(runtimeHome(), AGENTS_ENTRY) + realFs.writeFileSync(sourcePath, MIRRORED, 'utf-8') + realFs.writeFileSync(targetPath, MIRRORED, 'utf-8') + // Orca owns this copy, which is what entitles the sync to remove it. + markCopiedResource(runtimeHome(), AGENTS_ENTRY, sourcePath) + return { sourcePath, targetPath } + } + + it('keeps the mirrored copy when the source is denied', () => { + const { sourcePath, targetPath } = seedOwnedMirrorCopy() + denials.deny(sourcePath) + + syncCodexGlobalInstructionsIntoManagedHome({ + systemHomePath: systemHome(), + managedHomePath: runtimeHome() + }) + + // Before the fix, `existsSync` reported the denied source as gone and the + // managed copy was deleted on the next launch. + expect(realFs.existsSync(targetPath)).toBe(true) + 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) + + syncCodexGlobalInstructionsIntoManagedHome({ + systemHomePath: systemHome(), + managedHomePath: runtimeHome() + }) + + // Why: removing Orca's own copy of a resource the user deleted is the + // point of this path. Refusing here would strand stale instructions. + expect(realFs.existsSync(targetPath)).toBe(false) + }) +}) diff --git a/src/shared/definitive-filesystem-absence.ts b/src/shared/definitive-filesystem-absence.ts new file mode 100644 index 00000000000..8140d0588e3 --- /dev/null +++ b/src/shared/definitive-filesystem-absence.ts @@ -0,0 +1,17 @@ +/** + * The single errno allowlist for "this path is definitively not there". + * + * `existsSync` returns `false` for `ENOENT` and for `EPERM`, `EACCES`, `EBUSY`, + * `EIO`, `UNKNOWN` and every unrecognised code alike, and a `catch` returning a + * default does the same. Callers that act on absence — deleting a mirror, + * overwriting a config, clearing a credential — must be able to tell the two + * apart, and they must all agree on where the line is, so this lives in one + * place rather than being re-derived per lane. + * + * An unknown code is never absence. Mapping the unknown to a verdict is the + * category error this predicate exists to prevent. + */ +export function isDefinitiveAbsence(error: unknown): boolean { + const code = (error as NodeJS.ErrnoException | null)?.code + return code === 'ENOENT' || code === 'ENOTDIR' +}