fix(codex): preserve hooks after failed source reads

This commit is contained in:
Brennan Benson
2026-08-23 11:41:36 -07:00
parent a93272cd7c
commit 7ee3be40e1
4 changed files with 121 additions and 4 deletions
@@ -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' }
@@ -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())
@@ -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<string, { hooks?: { command?: string }[] }[]>
}
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')
+25 -2
View File
@@ -299,13 +299,18 @@ function getRuntimeHooksWithSystemUserHooks(
): {
hooks: Record<string, HookDefinition[]>
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<string>(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)