fix(codex): sanitize hooks config writes (#6247)

Co-authored-by: Orca <help@stably.ai>
Co-authored-by: brennanb2025 <brennankbenson@gmail.com>

src/main/agent-hooks/remote-hook-service-installers.test.ts

@@ -192,7 +192,16 @@ describe('remote hook service installers', () => {
   })
 
   it('installs remote Codex hooks with matching trust entries', async () => {
-    const { sftp, fs } = createFakeSftp()
+    const { sftp, fs } = createFakeSftp({
+      '/home/dev/.codex/hooks.json': `${JSON.stringify({
+        hooks: {},
+        _managed: {
+          'external-manager': {
+            Stop: [0]
+          }
+        }
+      })}\n`
+    })
 
     const status = await new CodexHookService().installRemote(sftp, '/home/dev/')
 
@@ -200,7 +209,9 @@ describe('remote hook service installers', () => {
     expect(status.configPath).toBe('/home/dev/.codex/hooks.json')
     const hooks = JSON.parse(fs.files.get('/home/dev/.codex/hooks.json')!) as {
       hooks: Record<string, { hooks: { command: string }[] }[]>
+      _managed?: unknown
     }
+    expect(hooks._managed).toEqual({ 'external-manager': { Stop: [0] } })
     for (const eventName of [
       'SessionStart',
       'UserPromptSubmit',

src/main/codex/hook-service.test.ts

@@ -133,6 +133,32 @@ describe('CodexHookService', () => {
     expect(trustConfig).toContain(':permission_request:0:0')
   })
 
+  it('drops plugin manager metadata from runtime hooks.json during install', () => {
+    const managedCodexHome = join(userDataDir, 'codex-runtime-home', 'home')
+    mkdirSync(managedCodexHome, { recursive: true })
+    writeFileSync(
+      join(managedCodexHome, 'hooks.json'),
+      `${JSON.stringify({
+        hooks: {},
+        _managed: {
+          'compound-engineering': {
+            Stop: [0]
+          }
+        }
+      })}\n`,
+      'utf-8'
+    )
+
+    expect(new CodexHookService().install().state).toBe('installed')
+
+    const hooksConfig = JSON.parse(readFileSync(join(managedCodexHome, 'hooks.json'), 'utf-8')) as {
+      hooks: Record<string, unknown>
+      _managed?: unknown
+    }
+    expect(hooksConfig._managed).toBeUndefined()
+    expect(Object.keys(hooksConfig)).toEqual(['hooks'])
+  })
+
   // Why: #6078 — a Windows user profile path like `C:\Users\Jane Doe` used to
   // be written verbatim as the hook command, so Codex split it at the space and
   // the hook exited with code 1. The managed command uses an encoded launcher
@@ -720,6 +746,11 @@ describe('CodexHookService', () => {
               { hooks: [{ type: 'command', command: legacyCommand }] }
             ],
             SessionStart: [{ hooks: [{ type: 'command', command: legacyCommand }] }]
+          },
+          _managed: {
+            'external-manager': {
+              Stop: [0]
+            }
           }
         },
         null,
@@ -752,9 +783,11 @@ describe('CodexHookService', () => {
 
     const systemHooks = JSON.parse(readFileSync(systemHooksPath, 'utf-8')) as {
       hooks: Record<string, { hooks?: { command?: string }[] }[]>
+      _managed?: unknown
     }
     expect(systemHooks.hooks.Stop).toEqual([{ hooks: [{ type: 'command', command: 'user-hook' }] }])
     expect(systemHooks.hooks.SessionStart).toBeUndefined()
+    expect(systemHooks._managed).toEqual({ 'external-manager': { Stop: [0] } })
     const systemToml = readFileSync(join(systemCodexHome, 'config.toml'), 'utf-8')
     expect(systemToml).toContain('model = "system-model"')
     expect(systemToml).not.toContain(':stop:1:0')
@@ -913,6 +946,41 @@ describe('CodexHookService', () => {
     expect(existsSync(profilePath)).toBe(false)
   })
 
+  it('sanitizes runtime hooks.json metadata during remove even without managed hooks', () => {
+    const managedCodexHome = join(userDataDir, 'codex-runtime-home', 'home')
+    const managedHooksPath = join(managedCodexHome, 'hooks.json')
+    mkdirSync(managedCodexHome, { recursive: true })
+    writeFileSync(
+      managedHooksPath,
+      `${JSON.stringify(
+        {
+          hooks: {
+            Stop: [{ hooks: [{ type: 'command', command: 'user-hook' }] }]
+          },
+          _managed: {
+            'compound-engineering': {
+              Stop: [0]
+            }
+          }
+        },
+        null,
+        2
+      )}\n`,
+      'utf-8'
+    )
+
+    const status = new CodexHookService().remove()
+
+    expect(status.state).toBe('not_installed')
+    const hooksConfig = JSON.parse(readFileSync(managedHooksPath, 'utf-8')) as {
+      hooks: Record<string, unknown>
+      _managed?: unknown
+    }
+    expect(hooksConfig._managed).toBeUndefined()
+    expect(Object.keys(hooksConfig)).toEqual(['hooks'])
+    expect(hooksConfig.hooks.Stop).toEqual([{ hooks: [{ type: 'command', command: 'user-hook' }] }])
+  })
+
   it('cleans duplicate Codex hook representations while keeping status hooks in runtime CODEX_HOME', () => {
     const systemCodexHome = join(tmpHome, '.codex')
     const systemHooksPath = join(systemCodexHome, 'hooks.json')

src/main/codex/hook-service.ts

@@ -62,6 +62,12 @@ function getConfigPath(): string {
   return join(getOrcaManagedCodexHomePath(), 'hooks.json')
 }
 
+function writeCodexHooksJson(configPath: string, hooks: Record<string, HookDefinition[]>): void {
+  // Why: Codex rejects unknown top-level hooks.json fields, so plugin manager
+  // bookkeeping such as `_managed` must not survive Orca's rewrite.
+  writeHooksJson(configPath, { hooks })
+}
+
 function getCodexConfigTomlPath(): string {
   return join(getOrcaManagedCodexHomePath(), 'config.toml')
 }
@@ -532,6 +538,8 @@ function cleanupLegacySystemManagedHooks(): void {
   // Why: Codex hooks moved to Orca's managed CODEX_HOME; old entries in
   // ~/.codex would keep external Codex sessions reporting into Orca.
   if (removedManagedHook) {
+    // Why: this is the user's system hooks file, not Orca's runtime copy.
+    // Remove only stale Orca hook entries and preserve other managers' metadata.
     writeHooksJson(legacyConfigPath, { ...config, hooks: nextHooks })
   }
   removeMatchingTrustEntries(getSystemCodexConfigTomlPath(), trustEntries)
@@ -889,7 +897,7 @@ export class CodexHookService {
 
     config.hooks = nextHooks
     writeManagedScript(scriptPath, getManagedScript())
-    writeHooksJson(configPath, config)
+    writeCodexHooksJson(configPath, nextHooks)
     // Why: trust entries write last so a half-write can't leave a hash
     // pointing at a hook that doesn't exist. Surface failures — without this,
     // getStatus would report green for a hook Codex won't actually fire.
@@ -979,7 +987,9 @@ export class CodexHookService {
       // Why: SSH remotes use POSIX `.sh` hook paths even when Orca itself is
       // running on Windows; never derive remote script syntax from local OS.
       await writeManagedScriptRemote(sftp, remoteScriptPath, getManagedScript('posix'))
-      await writeHooksJsonRemote(sftp, remoteConfigPath, config)
+      // Why: SSH installs edit the user's remote ~/.codex/hooks.json directly.
+      // Preserve non-Orca top-level metadata while replacing the hooks tree.
+      await writeHooksJsonRemote(sftp, remoteConfigPath, { ...config, hooks: nextHooks })
       try {
         const existingToml = (await readTextFileRemote(sftp, remoteTomlPath)) ?? ''
         const updatedToml = upsertHookTrustEntriesInContent(existingToml, trustEntries)
@@ -1035,7 +1045,7 @@ export class CodexHookService {
     const isManagedCommand = createManagedCommandMatcher(getManagedScriptFileName())
     const hookPlan = getRuntimeHooksWithSystemUserHooks(config.hooks, isManagedCommand)
     config.hooks = hookPlan.hooks
-    writeHooksJson(configPath, config)
+    writeCodexHooksJson(configPath, hookPlan.hooks)
 
     try {
       const tomlPath = getCodexConfigTomlPath()
@@ -1080,7 +1090,6 @@ export class CodexHookService {
     }
 
     const nextHooks = { ...config.hooks }
-    let removedManagedHooks = false
     // Why: same broad matcher as install(), so remove() also cleans up stale
     // entries from older builds even if the current scriptPath has moved.
     const isManagedCommand = createManagedCommandMatcher(getManagedScriptFileName())
@@ -1092,18 +1101,16 @@ export class CodexHookService {
         continue
       }
       const cleaned = removeManagedCommands(definitions, isManagedCommand)
-      if (JSON.stringify(cleaned) !== JSON.stringify(definitions)) {
-        removedManagedHooks = true
-      }
       if (cleaned.length === 0) {
         delete nextHooks[eventName]
       } else {
         nextHooks[eventName] = cleaned
       }
     }
-    if (configExists && removedManagedHooks) {
-      config.hooks = nextHooks
-      writeHooksJson(configPath, config)
+    if (configExists) {
+      // Why: remove() can be the only repair path for a parseable runtime file
+      // whose top-level plugin metadata makes Codex reject hooks.json.
+      writeCodexHooksJson(configPath, nextHooks)
     }
 
     // Why: also drop our trust entries so config.toml doesn't accumulate dead