Skip to content

fix(plugin): suppress Codex auto-discovery of Claude hooks - #3363

Closed
worktrunk-bot wants to merge 1 commit into
mainfrom
fix/issue-3362-codex-hooks
Closed

fix(plugin): suppress Codex auto-discovery of Claude hooks#3363
worktrunk-bot wants to merge 1 commit into
mainfrom
fix/issue-3362-codex-hooks

Conversation

@worktrunk-bot

Copy link
Copy Markdown
Collaborator

Problem

The Codex manifest (.codex-plugin/plugin.json) omitted a hooks key on the assumption — documented in plugins/worktrunk/CLAUDE.md — that "Codex ships no hooks." That assumption doesn't hold: when a plugin manifest has no hooks key, Codex auto-discovers hooks/hooks.json from the plugin root by convention (DEFAULT_HOOKS_CONFIG_FILE, the None branch of load_plugin_hooks). Since Claude and Codex share one payload dir, Codex picked up Worktrunk's Claude hooks file and surfaced the Claude events it recognizes (PreToolUse, PermissionRequest, UserPromptSubmit, Stop) in a Codex session — the symptom in #3362.

Per @ofek's follow-up, seeing anything about Claude while using Codex is poor UX regardless of whether the hooks function, so this takes the suppress direction rather than shipping Codex-native hooks.

Solution

Set hooks: {} — an empty inline hooks object — in the Codex manifest. An inline object takes Codex's Some(Inline) branch, which skips empty entries, and therefore never reaches the None auto-discovery branch, so no hooks load.

An empty hooks: [] array would not work: an empty path list falls back to auto-discovery (resolve_manifest_hooks). The inline empty object is what actually overrides discovery.

The change touches only the Codex manifest; the Claude manifest and its hooks/hooks.json are untouched. Also updates the now-stale reasoning in plugins/worktrunk/CLAUDE.md and src/commands/config/codex.rs (the old note said Codex has no Stop/turn-end event — current Codex does; the real reasons to suppress are the shared-file collision plus the missing SessionEnd-clear and the Claude-branding UX).

Testing

  • Updated test_codex_plugin_metadata_is_valid_json to assert the manifest carries hooks: {} (encoding the correct belief — absence would re-open the bug) — passes locally.
  • Verified against real Codex source (codex-core-plugins @ 98d28aa): added a test exercising load_plugin_manifest + load_plugin_hooks with hooks: {} and a present hooks/hooks.json — it yields zero hook sources, while Codex's own load_plugin_hooks_discovers_default_hooks_file confirms an absent key does discover the file. That upstream test can't run in this repo's CI, so the guarantee is pinned by source-level verification rather than a worktrunk-side runtime test.
Verification test (run against the cloned Codex workspace)
#[test]
fn empty_inline_manifest_hooks_suppress_default_discovery() {
    let (_tmp, plugin_root) = plugin_root();
    write_manifest(&plugin_root, r#"{ "name": "demo-plugin", "hooks": {} }"#);
    // The shared Claude hooks file sits at the default discovery path.
    write_hook_file(&plugin_root, "hooks/hooks.json", "PreToolUse", "echo claude");

    let (sources, warnings) = load_sources(&plugin_root);

    assert_eq!(warnings, Vec::<String>::new());
    assert_eq!(sources, Vec::<PluginHookSource>::new()); // discovery suppressed
}
test loader::tests::empty_inline_manifest_hooks_suppress_default_discovery ... ok

Closes #3362 — automated triage. A maintainer should confirm against a live Codex session before merge.

The Codex manifest omitted a `hooks` key on the assumption that absence
means "no hooks". It does not: when a plugin manifest has no `hooks` key,
Codex auto-discovers `hooks/hooks.json` from the plugin root by convention
(`DEFAULT_HOOKS_CONFIG_FILE`). Because Claude and Codex share one payload
dir, Codex picked up Worktrunk's *Claude* hooks file and surfaced the
Claude events it recognizes (PreToolUse, PermissionRequest,
UserPromptSubmit, Stop) in a Codex session.

Set `hooks: {}` — an empty inline object — in the Codex manifest. This
takes Codex's `Some(Inline)` branch (skipped as empty) and never reaches
the `None` auto-discovery branch, so no hooks load. An empty `hooks: []`
array would not work: an empty path list falls back to discovery.

Verified against codex-core-plugins @ 98d28aa: an empty inline `hooks: {}`
plus a present `hooks/hooks.json` yields zero hook sources, whereas an
absent key discovers the file (existing upstream test).

Closes #3362

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core fix is correct and I verified it against Codex source at the pinned 98d28aa: resolve_manifest_hooks deserializes {} as RawPluginManifestHooks::InlineSome(Inline([empty])) (taking the Some branch, not None), and load_plugin_hooks's Some(Inline) arm hits if hooks_file.hooks.is_empty() { continue; }, so it emits zero sources and never reaches the None auto-discovery path. The hooks: [] caveat also checks out — an empty InlineList falls back through (!hooks.is_empty()).then_some(...) to None. Nice source-level pin.

One follow-up gap, though — the reasoning correction landed in plugins/worktrunk/CLAUDE.md and src/commands/config/codex.rs but two user-facing surfaces still carry the old "no turn-end event" claim this PR is retiring:

  • src/cli/config.rs (after_long_help for the codex plugin subcommand — a primary doc source): "Codex exposes no turn-end hook event, so the Codex plugin omits them until it does."
  • docs/content/claude-code.md ("Agent Integration"): "Codex omits activity tracking because its hooks have no turn-end event, so a 🤖 marker could never clear back to 💬."

I confirmed against codex-rs/hooks/src/schema.rs @ 98d28aa that HookEventNameWire now includes Stop (and SubagentStop), and the vocabulary has sessionStart but no session-end — exactly the "Stop exists, no SessionEnd to clear" story the updated CLAUDE.md tells. So these two strings are now not just stale but actively misleading: the second phrasing implies markers are omitted only until Codex ships a turn-end event, a condition that's now met while we deliberately keep omitting them.

The fix is the same rewrite already applied elsewhere (turn-end event exists; suppression stands for the shared-file + no-SessionEnd + Claude-branding reasons). Both are outside this diff, so no inline suggestion — editing after_long_help in src/cli/config.rs regenerates the help_config_plugins_codex snapshot and any command-page mirror, and docs/content/claude-code.md's edit auto-syncs to skills/worktrunk/reference/claude-code.md. I'm happy to push that as a follow-up commit if you'd like it folded into this PR rather than a separate one. (The CHANGELOG.md mention of "no turn-end hooks" is a historical release entry — left alone.)

Not approving since this is a self-authored PR; flagging for the maintainer who'll confirm against a live Codex session per the PR body.

@max-sixty

Copy link
Copy Markdown
Owner

@worktrunk-bot resolve conflicts

@worktrunk-bot

Copy link
Copy Markdown
Collaborator Author

I resolved the conflicts, but doing so surfaced that this PR is now fully superseded — the resolution collapses to zero net change against main.

While this PR was open, #3364 ("feat(plugin): ship Codex-native activity hooks", merged in 512a94de2 at 06:05Z) landed a superior fix for the same issue (#3362). Instead of suppressing hooks with hooks: {}, it ships Codex-native activity hooks inline in the manifest — which also takes Codex's Some(Inline) branch and blocks auto-discovery of the shared Claude hooks/hooks.json, so it closes #3362 the same way and adds the activity markers this PR deliberately omitted. It also already rewrote the user-facing "no turn-end event" strings I flagged in my earlier review (src/cli/config.rs, docs/content/claude-code.md).

All four conflicts were the two opposing approaches to the exact same lines (plugin.json hooks key, codex.rs install hint + comment, plugins/worktrunk/CLAUDE.md limitations section, config_show.rs assertions). The only coherent resolution is to take main's version everywhere — and once you do, the branch is byte-identical to main:

$ git diff origin/main --stat   # after resolving all 4 conflicts to main's side
$                               # (empty — nothing left)

So there's nothing to merge here: every change this PR made is already on main in a better form. I'm closing it as superseded by #3364 rather than pushing an empty merge commit. If you'd rather keep it open for any reason, just reopen and let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Codex plugin hooks should not rely on compatibility with Claude

2 participants