fix(desktop): remove PreToolUse/PostToolUse hooks from Codex#3121
Conversation
These per-tool hooks add overhead without providing value for Codex. Only SessionStart, UserPromptSubmit, and Stop are needed.
📝 WalkthroughWalkthroughRemoved Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts (1)
368-389:⚠️ Potential issue | 🟡 MinorFix stale comment example near Codex stale-hook cleanup.
Line 369 says “for example
UserPromptSubmitfrom older builds,” butUserPromptSubmitis still actively managed at Line 388. This comment should be updated to avoid confusion.🧹 Proposed comment fix
- // Remove all stale Superset-managed Codex hook commands, including events we - // no longer manage natively (for example UserPromptSubmit from older builds). + // Remove all stale Superset-managed Codex hook commands, including events we + // no longer manage natively (for example PreToolUse/PostToolUse from older builds).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts` around lines 368 - 389, The comment above the loop that cleans stale Superset-managed Codex hook commands is misleading because it says "for example UserPromptSubmit from older builds" while UserPromptSubmit is still managed (see managedEvents and the "UserPromptSubmit" tag); update the comment to remove that example or replace it with an accurate example (or say “events no longer managed natively” without naming UserPromptSubmit) so it no longer contradicts the code that uses existing.hooks, removeManagedHooksFromDefinition, isManagedCodexHookCommand, and the managedEvents list.apps/desktop/docs/EXTERNAL_FILES.md (1)
45-54:⚠️ Potential issue | 🟡 MinorUpdate Codex paragraph wording to match removed tool hooks.
Line 45 correctly narrows managed Codex hooks, but Line 48-49 still says “prompt/tool lifecycle events,” which is now misleading after removing
PreToolUse/PostToolUse.✏️ Proposed doc tweak
-For Codex specifically, Superset now relies on native `~/.codex/hooks.json` -registration for durable prompt/tool lifecycle events, while the wrapper in +For Codex specifically, Superset now relies on native `~/.codex/hooks.json` +registration for durable prompt/session lifecycle events, while the wrapper in🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/docs/EXTERNAL_FILES.md` around lines 45 - 54, Update the paragraph that currently reads “prompt/tool lifecycle events” to remove any reference to tool hooks and the removed PreToolUse/PostToolUse hooks: change it to say Superset relies on native hooks.json registration for durable prompt lifecycle events (SessionStart, UserPromptSubmit, Stop), and clarify that the Superset wrapper still injects notify and keeps the session-log watcher for compatibility; also ensure the sentence about rewriting only Superset-managed entries to point at notify.sh while preserving user-defined hooks remains accurate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/desktop/docs/EXTERNAL_FILES.md`:
- Around line 45-54: Update the paragraph that currently reads “prompt/tool
lifecycle events” to remove any reference to tool hooks and the removed
PreToolUse/PostToolUse hooks: change it to say Superset relies on native
hooks.json registration for durable prompt lifecycle events (SessionStart,
UserPromptSubmit, Stop), and clarify that the Superset wrapper still injects
notify and keeps the session-log watcher for compatibility; also ensure the
sentence about rewriting only Superset-managed entries to point at notify.sh
while preserving user-defined hooks remains accurate.
In
`@apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts`:
- Around line 368-389: The comment above the loop that cleans stale
Superset-managed Codex hook commands is misleading because it says "for example
UserPromptSubmit from older builds" while UserPromptSubmit is still managed (see
managedEvents and the "UserPromptSubmit" tag); update the comment to remove that
example or replace it with an accurate example (or say “events no longer managed
natively” without naming UserPromptSubmit) so it no longer contradicts the code
that uses existing.hooks, removeManagedHooksFromDefinition,
isManagedCodexHookCommand, and the managedEvents list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 897231a9-53ce-4184-a290-17d09f292034
📒 Files selected for processing (3)
apps/desktop/docs/EXTERNAL_FILES.mdapps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
…ion (superset-sh#3121) These per-tool hooks add overhead without providing value for Codex. Only SessionStart, UserPromptSubmit, and Stop are needed.
cherry-pick方式で内容を取り込み済みの14コミットをgit履歴上もマージ済みにする。 取り込み済み (cherry-pick / 手動移植): - be22b46 superset-sh#3125 — スキップ (下記参照) - 88bc7fb superset-sh#3127 — Revert DA1 ✓ - 92d0ff9 superset-sh#3054 — DA1 fix ✓ - c48450e superset-sh#3093 — file viewer pane fix ✓ - fffa8db superset-sh#3128 — version 1.4.7 ✓ - 589a7c7 superset-sh#3136 — fuzzy scorer (ハイブリッド方式) ✓ - ceb8c81 superset-sh#3150 — Electron 40.8.5 ✓ - 8922b94 superset-sh#3137 — terminalId分離 ✓ - c7508e5 superset-sh#3152 — GitHub無料化 ✓ - 2b91f11 superset-sh#3155 — v2 terminal theme ✓ - b8b11af superset-sh#3154 — TUI dimension fix ✓ - 7599ace superset-sh#3149 — v2 sidebar file tree (手動統合) ✓ - 4d7c612 superset-sh#3174 — DnD重複削除 ✓ - 864977d superset-sh#3157 — Host Service分離 ✓ 意図的にスキップ: - be22b46 superset-sh#3125 (GitHub polling簡素化) フォーク独自のGitHubSyncService (バックエンド集中ポーリング) と 設計が異なるため不採用。upstreamはフロントエンドhover駆動、フォークは バックエンドキャッシュウォーマー方式。詳細は githubQueryPolicy.ts と github-sync-service.ts のFORK NOTEを参照。 ゴースト・マージ復元 (revert 134cfd5 で消失した内容): - 538f306 superset-sh#3120 — Patch vuln ✓ - 1588d20 superset-sh#3108 — terminal lifecycle分離 ✓ - 59426f6 superset-sh#3122 — file tree + FilePane + Alert refactor (手動統合) ✓ - 10d9a5d superset-sh#3097 — tiptap line-height ✓ - 337a9ae superset-sh#3121 — Codex hooks削除 ✓
Summary
PreToolUseandPostToolUsefrom the managed Codex hooks in~/.codex/hooks.jsonSessionStart,UserPromptSubmit, andStophooksTest plan
Summary by cubic
Remove
PreToolUseandPostToolUsefrom managed Codex hooks to cut overhead. Codex now only registersSessionStart,UserPromptSubmit, andStop, while preserving any user-defined pre/post hooks on merge.PreToolUse/PostToolUseinto~/.codex/hooks.json; only manageSessionStart,UserPromptSubmit,Stop.Written for commit 8a385df. Summary will update on new commits.
Summary by CodeRabbit
SessionStart,UserPromptSubmit, andStop.PreToolUseandPostToolUsefrom the set of managed hook events.