diff --git a/src/main/agent-hooks/hooks-json-read.ts b/src/main/agent-hooks/hooks-json-read.ts index e2c8743cab0..97ca220c354 100644 --- a/src/main/agent-hooks/hooks-json-read.ts +++ b/src/main/agent-hooks/hooks-json-read.ts @@ -1,4 +1,5 @@ -import { existsSync, readFileSync } from 'node:fs' +import { readFileSync } from 'node:fs' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' import type { HooksConfig } from './installer-utils' export function isPlainObject(value: unknown): value is Record { @@ -26,16 +27,17 @@ export function parseHooksJsonText(raw: string): HooksConfig | null { // bytes it was derived from; the raw snapshot and the parse must come from one // read or a concurrent save can slip between them unnoticed. export function readHooksJsonWithRaw(configPath: string): HooksJsonSnapshot { - if (!existsSync(configPath)) { - return { raw: null, config: {} } - } - let raw: string + // Why: the read arm below already separates "no hooks configured" from "could + // not read", but the `existsSync` arm in front of it did not — it returned a + // VALID EMPTY config for a file that merely could not be opened, and the + // installer then wrote generated hooks over it. One read classifies both, and + // closes the TOCTOU window between the two calls. try { - raw = readFileSync(configPath, 'utf-8') - } catch { - return { raw: null, config: null } + const raw = readFileSync(configPath, 'utf-8') + return { raw, config: parseHooksJsonText(raw) } + } catch (error) { + return isDefinitiveAbsence(error) ? { raw: null, config: {} } : { raw: null, config: null } } - return { raw, config: parseHooksJsonText(raw) } } export function readHooksJson(configPath: string): HooksConfig | null { diff --git a/src/main/codex/codex-pane-account-registry.ts b/src/main/codex/codex-pane-account-registry.ts index 39853471683..faa8dd87c0d 100644 --- a/src/main/codex/codex-pane-account-registry.ts +++ b/src/main/codex/codex-pane-account-registry.ts @@ -1,4 +1,5 @@ import { mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' import { dirname, join } from 'node:path' import { getOrcaUserDataPath } from './codex-home-paths' import type { @@ -51,22 +52,47 @@ function getRegistryPath(): string { return join(getOrcaUserDataPath(), 'codex-pane-accounts.json') } -function readRegistry(): RegistryFile { +/** + * `null` means the registry could not be READ. That is not the same as "no + * panes are attributed", and it must never be cached: the old `catch { return + * null }` collapsed both into an empty registry, so one unreadable read erased + * every pane's account attribution AND pinned that erasure in `cachedRegistry` + * for the rest of the process, surviving the file becoming readable again. + */ +function readRegistryOrNull(): RegistryFile | null { if (cachedRegistry) { return cachedRegistry } - cachedRegistry = parseRegistry(readRegistryFile()) + let rawRegistry: string + try { + rawRegistry = readFileSync(getRegistryPath(), 'utf-8') + } catch (error) { + if (!isDefinitiveAbsence(error)) { + return null + } + cachedRegistry = parseRegistry(null) + return cachedRegistry + } + // Why: a corrupt registry still degrades to empty and IS cached — rebuilding + // unparseable state is the intent, and re-reading it every call would only + // repeat the parse failure. + cachedRegistry = parseRegistry(parseRegistryJson(rawRegistry)) return cachedRegistry } -function readRegistryFile(): unknown { +function parseRegistryJson(rawRegistry: string): unknown { try { - return JSON.parse(readFileSync(getRegistryPath(), 'utf-8')) + return JSON.parse(rawRegistry) } catch { return null } } +/** Read-only callers: an unreadable registry reports no attribution, uncached. */ +function readRegistry(): RegistryFile { + return readRegistryOrNull() ?? { version: 2, panes: {} } +} + function parseRegistry(parsed: unknown): RegistryFile { const empty: RegistryFile = { version: 2, panes: {} } if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { @@ -162,7 +188,13 @@ function writeRegistry(registry: RegistryFile): void { * Records the account a PTY launched under. Pass null to forget a pinned pane. */ export function recordCodexPaneAccount(ptyId: string, record: CodexPaneAccountRecord | null): void { - const registry = readRegistry() + // Why: every write below persists the whole registry. Deriving it from an + // empty stand-in because the real one could not be read would drop every + // other pane's attribution on disk. Skip; the next record retries. + const registry = readRegistryOrNull() + if (!registry) { + return + } if (!record) { if (!(ptyId in registry.panes)) { return @@ -279,7 +311,13 @@ export function hasRecordedManagedHostCodexPane(): boolean { /** Drops records whose daemon PTYs are authoritatively absent. */ export function reconcileCodexPaneAccountsWithLivePtys(livePtyIds: readonly string[]): void { - const registry = readRegistry() + // Why: this deletes every pane not in the live list. Against an empty stand-in + // it is a no-op, but the write it guards would still persist that stand-in + // over the real file, so refuse rather than reconcile a registry nobody read. + const registry = readRegistryOrNull() + if (!registry) { + return + } const livePtyIdSet = new Set(livePtyIds) let changed = false for (const ptyId of Object.keys(registry.panes)) { diff --git a/src/main/codex/codex-trust-grant-ledger.ts b/src/main/codex/codex-trust-grant-ledger.ts index b1d20421bce..75a289db22f 100644 --- a/src/main/codex/codex-trust-grant-ledger.ts +++ b/src/main/codex/codex-trust-grant-ledger.ts @@ -1,4 +1,5 @@ -import { existsSync, mkdirSync, readFileSync, statSync, writeFileSync } from 'node:fs' +import { mkdirSync, readFileSync, statSync, writeFileSync } from 'node:fs' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' import { dirname, join } from 'node:path' import { getOrcaManagedCodexHomePath } from './codex-home-paths' import { normalizeCodexProjectPathForLookup } from './config-toml-trust' @@ -40,13 +41,22 @@ export function getCodexTrustGrantHomeKey(runtimeHomePath: string): string { return normalizeCodexProjectPathForLookup(runtimeHomePath) } -function readLedgerFile(ledgerPath: string): CodexTrustGrantLedgerFile { +/** + * `null` means the ledger could not be READ, which is different from an absent + * or corrupt one. A write derived from `empty` persists a file containing only + * the home being written, destroying every other home's grants — so the write + * paths refuse on `null` while the read path keeps degrading. + */ +function readLedgerFileOrNull(ledgerPath: string): CodexTrustGrantLedgerFile | null { const empty: CodexTrustGrantLedgerFile = { version: 1, homes: {} } - if (!existsSync(ledgerPath)) { - return empty + let rawLedger: string + try { + rawLedger = readFileSync(ledgerPath, 'utf-8') + } catch (error) { + return isDefinitiveAbsence(error) ? empty : null } try { - const parsed: unknown = JSON.parse(readFileSync(ledgerPath, 'utf-8')) + const parsed: unknown = JSON.parse(rawLedger) if ( !parsed || typeof parsed !== 'object' || @@ -67,6 +77,11 @@ function readLedgerFile(ledgerPath: string): CodexTrustGrantLedgerFile { } } +/** Corrupt or absent still degrades to empty — rebuilding those IS the intent. */ +function readLedgerFile(ledgerPath: string): CodexTrustGrantLedgerFile { + return readLedgerFileOrNull(ledgerPath) ?? { version: 1, homes: {} } +} + function persistLedgerFile(ledgerPath: string, file: CodexTrustGrantLedgerFile): void { mkdirSync(dirname(ledgerPath), { recursive: true, mode: 0o700 }) writeFileSync(ledgerPath, `${JSON.stringify(file, null, 2)}\n`, { @@ -94,7 +109,13 @@ export function writeCodexTrustGrantLedgerHome( home: CodexTrustGrantLedgerHome, ledgerPath = getCodexTrustGrantLedgerPath() ): void { - const file = readLedgerFile(ledgerPath) + const file = readLedgerFileOrNull(ledgerPath) + if (!file) { + // Why: writing a file derived from `empty` would drop every other home's + // grants, and those homes would then be re-prompted for trust they had + // already granted. Skip this write; the next grant retries. + return + } file.homes[getCodexTrustGrantHomeKey(runtimeHomePath)] = home persistLedgerFile(ledgerPath, file) } diff --git a/src/main/codex/config-settings-baseline.ts b/src/main/codex/config-settings-baseline.ts index 6959219c2d2..856352e81f9 100644 --- a/src/main/codex/config-settings-baseline.ts +++ b/src/main/codex/config-settings-baseline.ts @@ -1,4 +1,5 @@ import { existsSync, writeFileSync } from 'node:fs' +import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' import { join } from 'node:path' import { readAgentStateFileSync, readAgentStateJsonFileSync } from '../agent-state-file-reader' @@ -20,11 +21,39 @@ type StoredSettingsBaseline = { conflicts?: Record } -export function readCodexSettingsBaseline(runtimeHomePath: string): CodexSettingsBaseline | null { +/** + * Why callers need three answers, not two: the baseline records what Orca last + * mirrored, so `null` drives "there is nothing to compare against, bootstrap + * one". For a file that merely could not be READ, bootstrapping overwrites the + * record of a change the user made inside Codex, and the next mirror pass then + * writes the old value back over their edit. Absent and unparseable still map + * to `absent` — rebuilding those is the intent. + */ +export type CodexSettingsBaselineObservation = + | { kind: 'present'; baseline: CodexSettingsBaseline } + | { kind: 'absent' } + | { kind: 'indeterminate' } + +export function observeCodexSettingsBaseline( + runtimeHomePath: string +): CodexSettingsBaselineObservation { const baselinePath = getCodexSettingsBaselinePath(runtimeHomePath) - if (!existsSync(baselinePath)) { - return null + const baseline = readParsedCodexSettingsBaseline(baselinePath) + if (baseline === 'unreadable') { + return { kind: 'indeterminate' } } + return baseline ? { kind: 'present', baseline } : { kind: 'absent' } +} + +/** Absent and unreadable both collapse to `null`; use the observation to tell them apart. */ +export function readCodexSettingsBaseline(runtimeHomePath: string): CodexSettingsBaseline | null { + const observation = observeCodexSettingsBaseline(runtimeHomePath) + return observation.kind === 'present' ? observation.baseline : null +} + +function readParsedCodexSettingsBaseline( + baselinePath: string +): CodexSettingsBaseline | null | 'unreadable' { try { const parsed: unknown = readAgentStateJsonFileSync(baselinePath) if (!isStoredSettingsBaseline(parsed)) { @@ -46,11 +75,18 @@ export function readCodexSettingsBaseline(runtimeHomePath: string): CodexSetting } } return { settings, conflicts } - } catch { - return null + } catch (error) { + // Why: a malformed baseline is still `null` — resetting corrupt state is the + // intent, and only a read that FAILED must be preserved. + return isDefinitiveAbsence(error) || isMalformedBaselineError(error) ? null : 'unreadable' } } +/** Why: `readAgentStateJsonFileSync` throws for a parse failure and for a size cap alike. */ +function isMalformedBaselineError(error: unknown): boolean { + return error instanceof SyntaxError +} + export function writeCodexSettingsBaseline( runtimeHomePath: string, baseline: CodexSettingsBaseline diff --git a/src/main/codex/config-settings-promotion.ts b/src/main/codex/config-settings-promotion.ts index cd0b703960a..a8a9c2e541b 100644 --- a/src/main/codex/config-settings-promotion.ts +++ b/src/main/codex/config-settings-promotion.ts @@ -14,7 +14,7 @@ import { import { parseTomlKeyPath, parseTomlTableHeaderPath } from './config-toml-key-path' import { tuiStructuredKey, upsertPromotedSettingsInContent } from './codex-config-settings-upsert' import { - readCodexSettingsBaseline, + observeCodexSettingsBaseline, writeCodexSettingsBaseline, type CodexSettingsBaseline, type CodexSettingsConflict @@ -163,6 +163,13 @@ export function snapshotCodexRuntimeSettingsBaseline( runtimeHomePath = getOrcaManagedCodexHomePath(), conflicts: ReadonlyMap = new Map() ): void { + // Why: overwriting a baseline that could not be read records whatever the + // runtime config says now as "Orca wrote this", so an edit the user made + // inside Codex since the last pass is reclassified and never promoted again. + // Absent and malformed are still rebuilt — that is what this function is for. + if (observeCodexSettingsBaseline(runtimeHomePath).kind === 'indeterminate') { + return + } try { const runtimeTomlPath = join(runtimeHomePath, 'config.toml') // Why: record an empty baseline even for a missing runtime config, so Codex's first write still diffs and promotes. @@ -235,10 +242,17 @@ function promoteCodexRuntimeSettingsToSystemUnsafe( 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) { + const baselineObservation = observeCodexSettingsBaseline(runtimeHomePath) + if (baselineObservation.kind === 'indeterminate') { + // Why: an empty plan here lets the mirror proceed and write the system value + // back over an in-Codex edit this baseline would have identified. The caller + // turns a throw into the existing stall-and-retry null. + throw new Error('Codex settings baseline could not be read') + } + if (baselineObservation.kind === 'absent') { return emptyPromotionPlan() } + const baseline = baselineObservation.baseline const runtimeValues = readPromotedSettingValues(runtimeTomlPath) const systemValues = readPromotedSettingValues(systemTomlPath) const updates = new Map() diff --git a/src/main/codex/config-sync-stall.ts b/src/main/codex/config-sync-stall.ts index c03598d4a28..45cf35c78b2 100644 --- a/src/main/codex/config-sync-stall.ts +++ b/src/main/codex/config-sync-stall.ts @@ -1,6 +1,5 @@ -import { existsSync } from 'node:fs' import { join } from 'node:path' -import { readAgentStateFileSync } from '../agent-state-file-reader' +import { observeAgentStateFile, observeResolvedPathEntry } from './codex-path-observation' import { getOrcaManagedCodexHomePath, getSystemCodexHomePath } from './codex-home-paths' import type { CodexSettingsPromotionHomes } from './config-settings-promotion' import type { @@ -28,20 +27,28 @@ export function getCodexConfigSyncStatus( const runtimeConfigPath = join(homes.runtimeHomePath, 'config.toml') // Why: a stall only withholds settings once a managed runtime config exists; // without one the mirror seeds it and there is nothing yet to fall behind. - if (!existsSync(runtimeConfigPath)) { + // But `existsSync` said "no runtime config" for one that was merely locked, + // and this function then reported `synced` while the mirror was refusing — + // telling the user their edits had been applied when nothing had run. + const runtimeConfigObservation = observeResolvedPathEntry(runtimeConfigPath) + if (runtimeConfigObservation.kind === 'absent') { return { state: 'synced', reason: null, systemConfigPath } } - if (!existsSync(systemConfigPath)) { + if (runtimeConfigObservation.kind === 'indeterminate') { + // Why: the managed home is what could not be read, so say that rather than + // borrowing a source-side reason and blaming the wrong path. + return { state: 'stalled', reason: 'managed-home-unavailable', systemConfigPath } + } + const systemConfigObservation = observeAgentStateFile(systemConfigPath) + if (systemConfigObservation.kind === 'absent') { return { state: 'stalled', reason: 'missing-source', systemConfigPath } } - let rawSystemConfig: string - try { - rawSystemConfig = readAgentStateFileSync(systemConfigPath) - } catch { + if (systemConfigObservation.kind === 'indeterminate') { // Why: the mirror aborts on an unreadable source too, so report the stall // rather than claiming a sync that cannot happen. return { state: 'stalled', reason: 'unreadable-source', systemConfigPath } } + const rawSystemConfig = systemConfigObservation.value if (rawSystemConfig.trim() === '') { return { state: 'stalled', reason: 'blank-source', systemConfigPath } } diff --git a/src/main/codex/config-toml-trust.ts b/src/main/codex/config-toml-trust.ts index a4768730c5b..18d2c1cfd41 100644 --- a/src/main/codex/config-toml-trust.ts +++ b/src/main/codex/config-toml-trust.ts @@ -12,6 +12,7 @@ import { import { basename, dirname, join, posix as pathPosix, win32 as pathWin32 } from 'node:path' import { createHash, randomUUID } from 'node:crypto' import { renameFileWithWindowsRetry } from '../codex-accounts/fs-utils' +import { observe } from './codex-path-observation' import { foldWslUncPathCaseInsensitiveParts } from '../../shared/wsl-paths' import { writeRollingFileBackup } from '../rolling-file-backup' import { @@ -337,7 +338,17 @@ export function upsertHookTrustEntries( configPath: string, entries: readonly CodexTrustEntry[] ): void { - const existing = existsSync(configPath) ? readTomlFile(configPath) : '' + // Why: `existsSync` answered `false` for a locked config exactly as for an + // absent one, so `existing` became '' and the upsert below rebuilt the file + // from the trust entries alone — replacing the user's model, provider, MCP + // servers, approvals and comments with a trust-only stub. Only a definitive + // absence may seed from empty; every caller in hook-service already turns a + // throw here into "trust entries could not be written. Run /hooks in Codex". + const observation = observe(() => readTomlFile(configPath)) + if (observation.kind === 'indeterminate') { + throw observation.error + } + const existing = observation.kind === 'present' ? observation.value : '' const updated = upsertHookTrustEntriesInContent(existing, entries) if (updated === existing) { return diff --git a/src/main/codex/sta-4823-shared-state-survives-a-failed-read.test.ts b/src/main/codex/sta-4823-shared-state-survives-a-failed-read.test.ts new file mode 100644 index 00000000000..eb680ed7d18 --- /dev/null +++ b/src/main/codex/sta-4823-shared-state-survives-a-failed-read.test.ts @@ -0,0 +1,412 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as NodeFs from 'node:fs' +import type * as NodeOs from 'node:os' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +// STA-4823: six shared Codex state files were rebuilt, erased or reported as +// healthy after a read that had merely failed. Every one is reachable on the +// host lane; the WSL-specific half of the catalogue stays in STA-4606. + +const denials = vi.hoisted(() => { + const state = { + paths: new Set(), + readOnlyPaths: new Set(), + existenceDeniedPaths: new Set(), + deny(path: string): void { + state.paths.add(path) + }, + /** + * Content reads fail but `existsSync` still says the file is there — the + * shape a Windows sharing violation takes, as distinct from `deny`, which + * models an ACL denial where `existsSync` itself reports false. + */ + denyReads(path: string): void { + state.readOnlyPaths.add(path) + }, + /** + * `existsSync` reports false and content reads throw, but `statSync` and the + * atomic replace still work. This is the shape that actually loses data: the + * caller concludes "no file here", builds fresh content, and the rename + * lands because rename needs permission on the DIRECTORY, not the target. + * Guarding the write as well would let a test pass because the write failed + * rather than because the fix refused. + */ + denyExistence(path: string): void { + state.existenceDeniedPaths.add(path) + }, + release(path: string): void { + state.paths.delete(path) + }, + reset(): void { + state.paths.clear() + state.readOnlyPaths.clear() + state.existenceDeniedPaths.clear() + }, + check(target: unknown, syscall: string, readOnly = false): void { + if (typeof target !== 'string') { + return + } + const readDenied = + readOnly && (state.readOnlyPaths.has(target) || state.existenceDeniedPaths.has(target)) + if (!state.paths.has(target) && !readDenied) { + return + } + 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 +}) + +// One denial modelled coherently: a denied path throws from every read AND +// reports false from existsSync. Faulting one but not another lets a test pass +// against code that still consults the one left healthy. +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 => { + denials.check(args[0], syscall) + return original(...args) + } + return Object.assign(wrapped, original) + } + const patched: Record = { + ...actual, + readFileSync: Object.assign((...args: unknown[]): unknown => { + denials.check(args[0], 'read', true) + return (actual.readFileSync as (...a: unknown[]) => unknown)(...args) + }, actual.readFileSync), + // Why: the size-capped agent-state reader opens with `openSync`, so leaving + // it unguarded makes a denial invisible to every config/baseline read. Guard + // it for READ intent only — a write opens 'w'/'wx'/'a', and the atomic + // replace those writes perform must stay healthy (see `deny` above). + openSync: Object.assign((...args: unknown[]): unknown => { + const flags = args[1] + const isRead = flags === undefined || (typeof flags === 'string' && flags.startsWith('r')) + if (isRead) { + denials.check(args[0], 'open', true) + } + return (actual.openSync as (...a: unknown[]) => unknown)(...args) + }, actual.openSync), + statSync: guard(actual.statSync, 'stat'), + lstatSync: guard(actual.lstatSync, 'lstat'), + existsSync: Object.assign( + (...args: unknown[]): boolean => + typeof args[0] === 'string' && + (denials.paths.has(args[0]) || denials.existenceDeniedPaths.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 { upsertHookTrustEntries, readHookTrustEntries } = await import('./config-toml-trust') +const { + writeCodexTrustGrantLedgerHome, + readCodexTrustGrantLedgerHome, + getCodexTrustGrantLedgerPath +} = await import('./codex-trust-grant-ledger') +const { readHooksJson } = await import('../agent-hooks/hooks-json-read') +const { snapshotCodexRuntimeSettingsBaseline } = await import('./config-settings-promotion') +const { getCodexConfigSyncStatus } = await import('./config-sync-stall') +const paneRegistry = await import('./codex-pane-account-registry') + +let fakeHomeDir: string +let userDataDir: string +let runtimeHomePath: string +let previousUserDataPath: string | undefined + +const systemHome = (): string => join(fakeHomeDir, '.codex') +const baselinePath = (): string => join(runtimeHomePath, '.orca-config-settings-baseline.json') + +beforeEach(() => { + denials.reset() + fakeHomeDir = realFs.mkdtempSync(join(tmpdir(), 'orca-sta4823-home-')) + userDataDir = realFs.mkdtempSync(join(tmpdir(), 'orca-sta4823-data-')) + runtimeHomePath = join(userDataDir, 'codex-runtime-home', 'home') + 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(runtimeHomePath, { recursive: true }) + realFs.mkdirSync(systemHome(), { recursive: true }) + paneRegistry._internals.resetCache() +}) + +afterEach(() => { + denials.reset() + paneRegistry._internals.resetCache() + 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-4823 D29 — an unreadable config.toml must not become a trust-only stub', () => { + const USER_CONFIG = [ + 'model = "gpt-5.1-codex-max"', + 'model_provider = "openai"', + '', + '# my notes', + '[mcp_servers.local]', + 'command = "run-me"', + '' + ].join('\n') + const ENTRY = [ + { + sourcePath: '/tmp/hooks.json', + eventLabel: 'stop', + groupIndex: 0, + hookIndex: 0, + trustedHash: 'sha256:x', + enabled: true + } + ] + + it('refuses the write rather than rebuilding the file from the trust entries alone', () => { + const configPath = join(runtimeHomePath, 'config.toml') + realFs.writeFileSync(configPath, USER_CONFIG, 'utf-8') + denials.denyExistence(configPath) + + // Every hook-service caller already turns this throw into "trust entries + // could not be written. Run /hooks in Codex to approve." + expect(() => upsertHookTrustEntries(configPath, ENTRY as never)).toThrow('EPERM') + + // Before the fix, existsSync said the file was gone, `existing` became '', + // and the upsert wrote a config containing ONLY the trust table — the + // user's model, provider, MCP servers and comments all discarded. + expect(realFs.readFileSync(configPath, 'utf-8')).toBe(USER_CONFIG) + }) + + it('still seeds a config.toml that does not exist yet', () => { + const configPath = join(runtimeHomePath, 'config.toml') + + // Why: seeding from empty is correct for a genuine absence, and is how a + // fresh managed home gets its trust entries at all. + upsertHookTrustEntries(configPath, ENTRY as never) + + expect(readHookTrustEntries(configPath).size).toBeGreaterThan(0) + }) +}) + +describe('STA-4823 D30 — an unreadable trust-grant ledger must not be rewritten from empty', () => { + const OTHER_HOME = '/some/other/managed/home' + + function seedLedgerWithAnotherHome(): string { + const ledgerPath = getCodexTrustGrantLedgerPath() + writeCodexTrustGrantLedgerHome( + OTHER_HOME, + { entries: { 'a:b': { trustedHash: 'sha256:other' } } } as never, + ledgerPath + ) + return ledgerPath + } + + it('skips the write instead of dropping every other home', () => { + const ledgerPath = seedLedgerWithAnotherHome() + const before = realFs.readFileSync(ledgerPath, 'utf-8') + denials.deny(ledgerPath) + + writeCodexTrustGrantLedgerHome( + runtimeHomePath, + { entries: { 'c:d': { trustedHash: 'sha256:mine' } } } as never, + ledgerPath + ) + + // Before the fix the read degraded to an empty ledger and the write + // persisted a file holding only this home, so every other home lost its + // grants and would be re-prompted for trust it had already given. + expect(realFs.readFileSync(ledgerPath, 'utf-8')).toBe(before) + }) + + it('still records a home when the ledger is readable', () => { + const ledgerPath = seedLedgerWithAnotherHome() + + // Why: the anchor. Refusing whenever a read fails would stop the ledger + // ever recording a grant. + writeCodexTrustGrantLedgerHome( + runtimeHomePath, + { entries: { 'c:d': { trustedHash: 'sha256:mine' } } } as never, + ledgerPath + ) + + expect(readCodexTrustGrantLedgerHome(runtimeHomePath, ledgerPath)).not.toBeNull() + expect(readCodexTrustGrantLedgerHome(OTHER_HOME, ledgerPath)).not.toBeNull() + }) +}) + +describe('STA-4823 D33 — an unreadable hooks.json is not an empty hooks.json', () => { + it('reports a failed read as unknown rather than as no hooks configured', () => { + const hooksPath = join(runtimeHomePath, 'hooks.json') + realFs.writeFileSync(hooksPath, JSON.stringify({ hooks: { Stop: [] } }), 'utf-8') + denials.deny(hooksPath) + + // The read arm already returned null for a failed read; the existsSync arm + // in front of it returned a VALID EMPTY config, and the installer then + // wrote generated hooks over the user's file. + expect(readHooksJson(hooksPath)).toBeNull() + }) + + it('still reports a genuinely absent hooks.json as an empty config', () => { + // Why: absence really does mean "no hooks configured", and the installer + // depends on that to seed one. + expect(readHooksJson(join(runtimeHomePath, 'hooks.json'))).toEqual({}) + }) +}) + +describe('STA-4823 D26 — an unreadable settings baseline must not be overwritten', () => { + function seedBaseline(): string { + realFs.writeFileSync( + baselinePath(), + `${JSON.stringify({ version: 2, settings: { model: 'old-model' } })}\n`, + 'utf-8' + ) + return realFs.readFileSync(baselinePath(), 'utf-8') + } + + it('leaves the record alone so the pending edit can still be promoted', () => { + const before = seedBaseline() + realFs.writeFileSync( + join(runtimeHomePath, 'config.toml'), + 'model = "edited-in-codex"\n', + 'utf-8' + ) + denials.deny(baselinePath()) + + snapshotCodexRuntimeSettingsBaseline(runtimeHomePath) + + // Before the fix this recorded "edited-in-codex" as Orca's own write, so + // the user's in-Codex change was never promoted and the next mirror pass + // wrote the old value back over it. + expect(realFs.readFileSync(baselinePath(), 'utf-8')).toBe(before) + }) + + it('still replaces a baseline that is present but malformed', () => { + realFs.writeFileSync(baselinePath(), '{ not json', 'utf-8') + realFs.writeFileSync(join(runtimeHomePath, 'config.toml'), 'model = "m"\n', 'utf-8') + + // Why: a corrupt baseline carries no information and resetting it IS the + // intent. Only the unreadable case may be preserved, or a user is wedged + // on a broken file forever. + snapshotCodexRuntimeSettingsBaseline(runtimeHomePath) + + // Asserted against the file rather than the new observation API, so this + // anchor still means something when the fix is reverted. + expect(JSON.parse(realFs.readFileSync(baselinePath(), 'utf-8'))).toMatchObject({ version: 2 }) + }) +}) + +describe('STA-4823 D15 — the sync status must not report synced while the mirror refuses', () => { + it('reports the managed home as unavailable when its config cannot be read', () => { + const runtimeConfigPath = join(runtimeHomePath, 'config.toml') + realFs.writeFileSync(runtimeConfigPath, 'model = "m"\n', 'utf-8') + realFs.writeFileSync(join(systemHome(), 'config.toml'), 'model = "s"\n', 'utf-8') + denials.deny(runtimeConfigPath) + + const status = getCodexConfigSyncStatus({ runtimeHomePath, systemHomePath: systemHome() }) + + // Before the fix existsSync reported the locked runtime config as absent + // and this returned `synced` — telling the user their edits had been + // applied while nothing had run. + expect(status).toMatchObject({ state: 'stalled', reason: 'managed-home-unavailable' }) + }) + + it('still reports synced when no runtime config exists yet', () => { + realFs.writeFileSync(join(systemHome(), 'config.toml'), 'model = "s"\n', 'utf-8') + + // Why: with no managed config the mirror seeds one; there is nothing to + // have fallen behind, and warning here would fire on every fresh install. + expect( + getCodexConfigSyncStatus({ runtimeHomePath, systemHomePath: systemHome() }) + ).toMatchObject({ state: 'synced', reason: null }) + }) +}) + +describe('STA-4823 D31 — an unreadable pane registry must not erase every attribution', () => { + const PANE = 'pty-1' + const OTHER_PANE = 'pty-2' + const registryPath = (): string => join(userDataDir, 'codex-pane-accounts.json') + + function seedTwoAttributedPanes(): void { + paneRegistry.recordCodexPaneAccount(PANE, { + selectionKey: 'host', + accountId: 'account-1', + homeRoute: 'account-home' + } as never) + paneRegistry.recordCodexPaneAccount(OTHER_PANE, { + selectionKey: 'host', + accountId: 'account-2', + homeRoute: 'account-home' + } as never) + paneRegistry._internals.resetCache() + } + + it('does not persist an empty registry over the real one', () => { + seedTwoAttributedPanes() + const before = realFs.readFileSync(registryPath(), 'utf-8') + denials.deny(registryPath()) + + paneRegistry.recordCodexPaneAccount('pty-3', { + selectionKey: 'host', + accountId: 'account-3', + homeRoute: 'account-home' + } as never) + + // Before the fix the read degraded to an empty registry and this write + // persisted it, dropping every other pane's account attribution on disk. + expect(realFs.readFileSync(registryPath(), 'utf-8')).toBe(before) + }) + + it('does not cache the erasure, so attribution returns when the file does', () => { + seedTwoAttributedPanes() + denials.deny(registryPath()) + + expect(paneRegistry.getCodexPaneAccount(PANE)).toBeNull() + + denials.release(registryPath()) + + // THE CACHE is the second half of this defect: one unreadable read used to + // pin the empty registry in memory for the rest of the process, so the + // attribution never came back even after the file did. + expect(paneRegistry.getCodexPaneAccount(PANE)).toMatchObject({ accountId: 'account-1' }) + }) + + it('still reports no attribution when the registry genuinely does not exist', () => { + // Why: a fresh install has no registry, and that really is "no panes are + // attributed". Refusing here would make every first launch look degraded. + expect(paneRegistry.getCodexPaneAccount(PANE)).toBeNull() + }) +})