diff --git a/src/main/codex/codex-config-mirror.ts b/src/main/codex/codex-config-mirror.ts index 4e8f5ccc197..dcb51c814da 100644 --- a/src/main/codex/codex-config-mirror.ts +++ b/src/main/codex/codex-config-mirror.ts @@ -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( diff --git a/src/main/codex/codex-home-paths.ts b/src/main/codex/codex-home-paths.ts index e7fe51c42fc..b265245f92d 100644 --- a/src/main/codex/codex-home-paths.ts +++ b/src/main/codex/codex-home-paths.ts @@ -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' } } 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 index 794c13de893..23549990a3c 100644 --- a/src/main/codex/sta-4737-unreadable-is-not-absent.test.ts +++ b/src/main/codex/sta-4737-unreadable-is-not-absent.test.ts @@ -18,11 +18,16 @@ import { join } from 'node:path' const denials = vi.hoisted(() => { const state = { paths: new Set(), + readOnlyPaths: new Set(), 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 = { ...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('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)