diff --git a/src/main/codex/codex-real-home-hook-install.test.ts b/src/main/codex/codex-real-home-hook-install.test.ts index b242a24cc0b..b108a0b5250 100644 --- a/src/main/codex/codex-real-home-hook-install.test.ts +++ b/src/main/codex/codex-real-home-hook-install.test.ts @@ -372,6 +372,23 @@ describe('ensureRealHomeCodexHookState (install)', () => { }) describe('ensureRealHomeCodexHookState (opt-out sweep)', () => { + it('keeps the managed lane when hooks.json cannot be read', () => { + mkdirSync(getRealHooksJsonPath()) + + expect(ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir })).toBe( + 'unavailable' + ) + }) + + it('keeps the managed lane when hooks.json is malformed', () => { + writeFileSync(getRealHooksJsonPath(), '{ not json', 'utf-8') + + expect(ensureRealHomeCodexHookState({ hooksEnabled: false, userDataPath: userDataDir })).toBe( + 'unavailable' + ) + expect(readFileSync(getRealHooksJsonPath(), 'utf-8')).toBe('{ not json') + }) + it('rebases trust when a user appended hooks after Orca installed', () => { grantSucceeds() const before = { type: 'command', command: 'before.sh' } diff --git a/src/main/codex/codex-real-home-hook-install.ts b/src/main/codex/codex-real-home-hook-install.ts index 1d59c17ac14..678526e4e60 100644 --- a/src/main/codex/codex-real-home-hook-install.ts +++ b/src/main/codex/codex-real-home-hook-install.ts @@ -274,7 +274,12 @@ function sweepRealHomeCodexHook(): RealHomeCodexHookLane { // Why: single read — the pre-write generation guard must compare against // the exact bytes this sweep's parse came from. const { raw: previousRaw, config } = readHooksJsonWithRaw(hooksJsonPath) - if (!config?.hooks || previousRaw === null) { + if (!config) { + // Why: a failed or malformed read proves no cleanup; keep the managed lane + // until a later pass can inspect and remove the real-home entry. + return 'unavailable' + } + if (!config.hooks || previousRaw === null) { return 'removed' } const isManagedCommand = createManagedCommandMatcher(getCodexManagedScriptFileName()) diff --git a/src/main/codex/hook-service-user-hook-mirroring.test.ts b/src/main/codex/hook-service-user-hook-mirroring.test.ts index e25d8e709f7..d06fec0b3a7 100644 --- a/src/main/codex/hook-service-user-hook-mirroring.test.ts +++ b/src/main/codex/hook-service-user-hook-mirroring.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { mkdirSync, readFileSync, writeFileSync } from 'node:fs' +import { mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import type * as Os from 'node:os' import { join } from 'node:path' import { upsertHookTrustEntriesInContent } from './config-toml-trust' @@ -32,6 +32,36 @@ import { CodexHookService } from './hook-service' const homes = setupCodexHookHomes(homedirMock, getPathMock) +function seedSystemUserHook(command: string): { + systemHooksPath: string + managedHooksPath: string +} { + const systemCodexHome = join(homes.tmpHome, '.codex') + const systemHooksPath = join(systemCodexHome, 'hooks.json') + mkdirSync(systemCodexHome, { recursive: true }) + writeFileSync( + systemHooksPath, + `${JSON.stringify({ hooks: { Stop: [{ hooks: [{ type: 'command', command }] }] } })}\n`, + 'utf-8' + ) + writeFileSync(join(systemCodexHome, 'config.toml'), 'model = "system-model"\n', 'utf-8') + return { + systemHooksPath, + managedHooksPath: join(homes.userDataDir, 'codex-runtime-home', 'home', 'hooks.json') + } +} + +function readRuntimeHookCommands(managedHooksPath: string): string[] { + const runtime = JSON.parse(readFileSync(managedHooksPath, 'utf-8')) as { + hooks: Record + } + return Object.values(runtime.hooks).flatMap((definitions) => + definitions.flatMap( + (definition) => definition.hooks?.flatMap((hook) => hook.command ?? []) ?? [] + ) + ) +} + function markHookTrustDisabled(toml: string, header: string): string { const headerIndex = toml.indexOf(header) expect(headerIndex).not.toBe(-1) @@ -43,6 +73,48 @@ function markHookTrustDisabled(toml: string, header: string): string { } describe('CodexHookService', () => { + it('preserves mirrored user hooks when the system hooks file cannot be read', () => { + const service = new CodexHookService() + const { systemHooksPath, managedHooksPath } = seedSystemUserHook('user-hook') + expect(service.install().state).toBe('installed') + const systemBefore = readFileSync(systemHooksPath, 'utf-8') + const before = readFileSync(managedHooksPath, 'utf-8') + + rmSync(systemHooksPath) + mkdirSync(systemHooksPath) + + for (const retry of [() => service.install(), () => service.refreshRuntimeUserHooks()]) { + expect(retry()).toMatchObject({ + state: 'error', + detail: 'Could not read system Codex hooks.json' + }) + expect(readFileSync(managedHooksPath, 'utf-8')).toBe(before) + } + + rmSync(systemHooksPath, { recursive: true }) + writeFileSync(systemHooksPath, systemBefore, 'utf-8') + expect(service.install().state).toBe('installed') + expect(readRuntimeHookCommands(managedHooksPath)).toContain('user-hook') + }) + + it.each(['absent', 'malformed'] as const)( + 'rebuilds mirrored user hooks when the system source is %s', + (sourceState) => { + const service = new CodexHookService() + const { systemHooksPath, managedHooksPath } = seedSystemUserHook('stale-user-hook') + expect(service.install().state).toBe('installed') + + if (sourceState === 'absent') { + rmSync(systemHooksPath) + } else { + writeFileSync(systemHooksPath, '{ not json', 'utf-8') + } + + expect(service.install().state).toBe('installed') + expect(readRuntimeHookCommands(managedHooksPath)).not.toContain('stale-user-hook') + } + ) + it('mirrors trusted system user hook approvals into the runtime CODEX_HOME', () => { const systemCodexHome = join(homes.tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index 3bf087b14d9..a0e30c5941c 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -299,13 +299,18 @@ function getRuntimeHooksWithSystemUserHooks( ): { hooks: Record trustEntries: MirroredRuntimeUserHookTrustEntry[] -} { +} | null { const systemConfigPath = getSystemConfigPath() if (systemConfigPath === runtimeConfigPath) { return { hooks: { ...runtimeHooks }, trustEntries: [] } } - const systemConfig = readHooksJson(systemConfigPath) + const { raw: systemRaw, config: systemConfig } = readHooksJsonWithRaw(systemConfigPath) + if (!systemConfig && systemRaw === null) { + // Why: rebuilding from an unreadable source drops the last-known user hooks + // from the managed runtime; let launch prep retry without changing it. + return null + } if (!systemConfig?.hooks) { return { hooks: {}, trustEntries: [] } } @@ -1284,6 +1289,15 @@ export class CodexHookService { const isManagedCommand = createManagedCommandMatcher(getCodexManagedScriptFileName()) const command = getManagedCommand(scriptPath) const hookPlan = getRuntimeHooksWithSystemUserHooks(config.hooks, isManagedCommand, configPath) + if (!hookPlan) { + return { + agent: 'codex', + state: 'error', + configPath, + managedHooksPresent: false, + detail: 'Could not read system Codex hooks.json' + } + } const nextHooks = hookPlan.hooks const managedEvents = new Set(CODEX_EVENTS) @@ -1534,6 +1548,15 @@ export class CodexHookService { const isManagedCommand = createManagedCommandMatcher(getCodexManagedScriptFileName()) const hookPlan = getRuntimeHooksWithSystemUserHooks(config.hooks, isManagedCommand, configPath) + if (!hookPlan) { + return { + agent: 'codex', + state: 'error', + configPath, + managedHooksPresent: false, + detail: 'Could not read system Codex hooks.json' + } + } config.hooks = hookPlan.hooks writeCodexHooksJson(configPath, hookPlan.hooks)