From 8a385dffe2638bac7726e89682cc1f179fbba848 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Thu, 2 Apr 2026 09:30:54 -0700 Subject: [PATCH] fix(desktop): remove PreToolUse/PostToolUse hooks from Codex integration These per-tool hooks add overhead without providing value for Codex. Only SessionStart, UserPromptSubmit, and Stop are needed. --- apps/desktop/docs/EXTERNAL_FILES.md | 2 +- .../agent-wrappers-claude-codex-opencode.ts | 21 +---- .../lib/agent-setup/agent-wrappers.test.ts | 85 ++++--------------- 3 files changed, 19 insertions(+), 89 deletions(-) diff --git a/apps/desktop/docs/EXTERNAL_FILES.md b/apps/desktop/docs/EXTERNAL_FILES.md index abcc47f5bb8..5cc01ad97cb 100644 --- a/apps/desktop/docs/EXTERNAL_FILES.md +++ b/apps/desktop/docs/EXTERNAL_FILES.md @@ -42,7 +42,7 @@ its hook entries into these files while preserving user-defined entries: | File | Purpose | |------|---------| | `~/.claude/settings.json` | Claude Code hook registration merge | -| `~/.codex/hooks.json` | Codex hook registration merge (`SessionStart`, `UserPromptSubmit`, `PreToolUse`, `PostToolUse`, `Stop`) | +| `~/.codex/hooks.json` | Codex hook registration merge (`SessionStart`, `UserPromptSubmit`, `Stop`) | | `~/.factory/settings.json` | Factory Droid hook registration (`UserPromptSubmit`, `Notification`, `PostToolUse`, `Stop`) | For Codex specifically, Superset now relies on native `~/.codex/hooks.json` diff --git a/apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts b/apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts index e3378f804f9..db889900cd5 100644 --- a/apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts +++ b/apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts @@ -385,12 +385,7 @@ export function getCodexGlobalHooksJsonContent( } const managedEvents: Array<{ - eventName: - | "SessionStart" - | "UserPromptSubmit" - | "PreToolUse" - | "PostToolUse" - | "Stop"; + eventName: "SessionStart" | "UserPromptSubmit" | "Stop"; definition: ClaudeHookDefinition; }> = [ { @@ -405,20 +400,6 @@ export function getCodexGlobalHooksJsonContent( hooks: [{ type: "command", command: notifyScriptPath }], }, }, - { - eventName: "PreToolUse", - definition: { - matcher: "*", - hooks: [{ type: "command", command: notifyScriptPath }], - }, - }, - { - eventName: "PostToolUse", - definition: { - matcher: "*", - hooks: [{ type: "command", command: notifyScriptPath }], - }, - }, { eventName: "Stop", definition: { diff --git a/apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts b/apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts index 9c968c4c7b6..9e96a3e4912 100644 --- a/apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts +++ b/apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts @@ -888,7 +888,7 @@ describe("agent-wrappers codex hooks.json", () => { rmSync(TEST_ROOT, { recursive: true, force: true }); }); - it("creates Codex hooks.json with prompt and tool lifecycle hooks when no file exists", () => { + it("creates Codex hooks.json with prompt and lifecycle hooks when no file exists", () => { const notifyPath = "/tmp/.superset/hooks/notify.sh"; const content = getCodexGlobalHooksJsonContent(notifyPath); expect(content).not.toBeNull(); @@ -907,8 +907,6 @@ describe("agent-wrappers codex hooks.json", () => { for (const eventName of [ "SessionStart", "UserPromptSubmit", - "PreToolUse", - "PostToolUse", "Stop", ] as const) { const hooks = parsed.hooks[eventName]; @@ -920,12 +918,8 @@ describe("agent-wrappers codex hooks.json", () => { ).toBe(true); } - expect(parsed.hooks.PreToolUse?.every((def) => def.matcher === "*")).toBe( - true, - ); - expect(parsed.hooks.PostToolUse?.every((def) => def.matcher === "*")).toBe( - true, - ); + expect(parsed.hooks.PreToolUse).toBeUndefined(); + expect(parsed.hooks.PostToolUse).toBeUndefined(); }); it("preserves user hooks when merging", () => { @@ -987,7 +981,7 @@ describe("agent-wrappers codex hooks.json", () => { const parsed = JSON.parse(content); - // Preserves user hook + // Preserves user hooks (including PreToolUse/PostToolUse which we don't manage) expect( parsed.hooks.Stop.some((def: { hooks: Array<{ command: string }> }) => def.hooks.some( @@ -1024,32 +1018,19 @@ describe("agent-wrappers codex hooks.json", () => { ), ).toBe(true); - // Adds managed hook - expect( - parsed.hooks.Stop.some((def: { hooks: Array<{ command: string }> }) => - def.hooks.some( - (hook: { command: string }) => hook.command === notifyPath, + // Adds managed hooks for SessionStart, UserPromptSubmit, Stop + for (const eventName of ["SessionStart", "UserPromptSubmit", "Stop"]) { + expect( + parsed.hooks[eventName].some( + (def: { hooks: Array<{ command: string }> }) => + def.hooks.some( + (hook: { command: string }) => hook.command === notifyPath, + ), ), - ), - ).toBe(true); + ).toBe(true); + } - // Also creates prompt + start hooks - expect( - parsed.hooks.SessionStart.some( - (def: { hooks: Array<{ command: string }> }) => - def.hooks.some( - (hook: { command: string }) => hook.command === notifyPath, - ), - ), - ).toBe(true); - expect( - parsed.hooks.UserPromptSubmit.some( - (def: { hooks: Array<{ command: string }> }) => - def.hooks.some( - (hook: { command: string }) => hook.command === notifyPath, - ), - ), - ).toBe(true); + // Does NOT inject managed hooks for PreToolUse/PostToolUse expect( parsed.hooks.PreToolUse.some( (def: { hooks: Array<{ command: string }> }) => @@ -1057,7 +1038,7 @@ describe("agent-wrappers codex hooks.json", () => { (hook: { command: string }) => hook.command === notifyPath, ), ), - ).toBe(true); + ).toBe(false); expect( parsed.hooks.PostToolUse.some( (def: { hooks: Array<{ command: string }> }) => @@ -1065,37 +1046,7 @@ describe("agent-wrappers codex hooks.json", () => { (hook: { command: string }) => hook.command === notifyPath, ), ), - ).toBe(true); - }); - - it("adds UserPromptSubmit, PreToolUse, and PostToolUse to the Codex hooks.json merge", () => { - const notifyPath = "/tmp/.superset/hooks/notify.sh"; - const content = getCodexGlobalHooksJsonContent(notifyPath); - expect(content).not.toBeNull(); - if (content === null) throw new Error("Expected content"); - - const parsed = JSON.parse(content) as { - hooks: Record< - string, - Array<{ - matcher?: string; - hooks: Array<{ type: string; command: string }>; - }> - >; - }; - - for (const eventName of [ - "UserPromptSubmit", - "PreToolUse", - "PostToolUse", - ] as const) { - expect(parsed.hooks[eventName]).toBeDefined(); - expect( - parsed.hooks[eventName]?.some((def) => - def.hooks.some((hook) => hook.command === notifyPath), - ), - ).toBe(true); - } + ).toBe(false); }); it("replaces stale Codex hook commands from old superset paths", () => { @@ -1150,8 +1101,6 @@ describe("agent-wrappers codex hooks.json", () => { for (const eventName of [ "SessionStart", "UserPromptSubmit", - "PreToolUse", - "PostToolUse", "Stop", ] as const) { const hooks = parsed.hooks[eventName];