diff --git a/CHANGELOG.md b/CHANGELOG.md index bf1269873d..0e6a9647ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ ### Fixed +- **Claude plugin hooks load again**: The Claude activity (🤖/💬) and `WorktreeCreate`/`WorktreeRemove` hooks stopped loading after the hooks file was renamed to `claude-hooks.json` in [#3382](https://github.com/max-sixty/worktrunk/pull/3382). Claude Code discovers plugin hooks by convention at `hooks/hooks.json` and does not honor `plugin.json`'s string-path `hooks` override for plugin loads, so the renamed file was never read — `/hooks` showed no worktrunk handlers and the markers silently stopped updating. The file is restored to the conventional `hooks/hooks.json`; the #3362 Codex collision the rename guarded against stays closed because the Codex manifest defines its hooks inline, which overrides Codex's convention discovery independently of the filename. ([#3417](https://github.com/max-sixty/worktrunk/issues/3417), thanks @avdi for reporting) + - **`wt switch` no longer crashes on ragged tables in PR comments**: A PR/MR comment table with more cells in a data row than in its header could panic termimad's column fitter at narrow widths, and because the comments preview renders on a background worker, the panic aborted the whole picker. The table render is now contained; the preview falls back to the table's plain text. ([#3408](https://github.com/max-sixty/worktrunk/pull/3408), closes [#3407](https://github.com/max-sixty/worktrunk/issues/3407), thanks @ortonomy for reporting) - **`wt config show` no longer suspends on the zsh completion probe**: The interactive zsh probe that detects a missing `compinit` claims the terminal foreground when job control is on; a slow or prompting zsh startup could hit the probe's kill-on-timeout before the foreground was restored, leaving `wt` in a background process group and its pager suspended with `suspended (tty output)`. Both interactive probes now run with job control disabled (`zsh +m`), so a timed-out probe can't strand the terminal. ([#3327](https://github.com/max-sixty/worktrunk/pull/3327), closes [#3322](https://github.com/max-sixty/worktrunk/issues/3322), thanks @karmeleon for reporting) @@ -32,8 +34,6 @@ ### Internal -- **Claude plugin hooks are Claude-scoped**: The plugin's hooks file is renamed `hooks.json` → `claude-hooks.json` so Codex's convention discovery can't pick up Claude's hooks, hardening the Codex scoping shipped in 0.66.0. ([#3382](https://github.com/max-sixty/worktrunk/pull/3382), thanks @ofek for the suggestion) - - **skim 5.0**: The picker's fuzzy-finder library moves from 4.10 to 5.0, dropping roughly 1,000 lines of transitive dependencies from the lockfile. ([#3378](https://github.com/max-sixty/worktrunk/pull/3378)) - **Prune benchmarks**: A rust-repo-scale prune fixture, Criterion benches, and trace spans over the removal path. ([#3401](https://github.com/max-sixty/worktrunk/pull/3401)) diff --git a/plugins/worktrunk/CLAUDE.md b/plugins/worktrunk/CLAUDE.md index a9d7e41544..4a0d6ad962 100644 --- a/plugins/worktrunk/CLAUDE.md +++ b/plugins/worktrunk/CLAUDE.md @@ -19,9 +19,11 @@ worktrunk/ ← repo root = marketplace root ├── plugin.json ← Claude manifest (NO .claude-plugin/ wrapper — │ the wrapper is marketplace-root-only) ├── .codex-plugin/plugin.json ← Codex manifest (Codex's required wrapper) - ├── hooks/claude-hooks.json ← Claude activity + WorktreeCreate/Remove hooks - │ (Claude-scoped name — a plain hooks/hooks.json - │ would be auto-discovered by Codex, see #3362) + ├── hooks/hooks.json ← Claude activity + WorktreeCreate/Remove hooks + │ (the conventional path Claude Code's loader + │ discovers — a Claude-scoped filename is NOT + │ honored by the string-path override, see #3417; + │ Codex is kept off it by its inline manifest, #3362) ├── hooks/wt.sh ← canonical hook shim; Claude reaches it via │ $CLAUDE_PLUGIN_ROOT, Codex via $PLUGIN_ROOT, │ Gemini via @@ -37,9 +39,13 @@ Path resolution differs by tool, all verified end-to-end against the real CLIs: - **Claude**: `.claude-plugin/marketplace.json` `source: "./plugins/worktrunk"`. Claude reads `plugins/worktrunk/plugin.json` (at the plugin root, *not* a - `.claude-plugin/` subdir). `hooks` and `skills` paths in `plugin.json` resolve - from the plugin root, so `./skills/worktrunk` follows the `skills` symlink to - the repo-root `skills/worktrunk`. `$CLAUDE_PLUGIN_ROOT` is the plugin root. + `.claude-plugin/` subdir). `skills` paths in `plugin.json` resolve from the + plugin root, so `./skills/worktrunk` follows the `skills` symlink to the + repo-root `skills/worktrunk`. Hooks are different: Claude's loader discovers + them by **convention** at `hooks/hooks.json` and does not honor the + string-path `hooks` override for plugin loads, so the file must sit at that + conventional path (#3417) even though `plugin.json` still names it. + `$CLAUDE_PLUGIN_ROOT` is the plugin root. - **Codex**: `.agents/plugins/marketplace.json` `source` object `{ "source": "local", "path": "./plugins/worktrunk" }`. Codex reads `plugins/worktrunk/.codex-plugin/plugin.json`. `skills: "./skills/"` resolves @@ -71,9 +77,10 @@ The 💬 transitions overlap deliberately: `Notification` covers the documented ### Codex activity hooks (marker persists after session end) -The Claude manifest carries `hooks: "./hooks/claude-hooks.json"` (a path); the Codex manifest carries `hooks` as an **inline object**, `{ "hooks": { … } }`, embedding a Codex-tailored hooks file directly. The distinction is deliberate: +The Claude manifest carries `hooks: "./hooks/hooks.json"` (a path); the Codex manifest carries `hooks` as an **inline object**, `{ "hooks": { … } }`, embedding a Codex-tailored hooks file directly. The distinction is deliberate: -- **Why inline, not a path or an absent key.** Claude and Codex share one payload dir, and Codex auto-discovers a `hooks/hooks.json` at the plugin root by convention (`DEFAULT_HOOKS_CONFIG_FILE`, the `None` branch of `load_plugin_hooks`). Historically the Claude hooks lived at exactly that path, so a Codex session surfaced Worktrunk's *Claude* events ([#3362](https://github.com/max-sixty/worktrunk/issues/3362)). Two independent things now keep a Codex session clean: (1) the Claude file is **Claude-scoped** — `hooks/claude-hooks.json`, not the conventional `hooks/hooks.json` — so Codex's discovery lookup finds nothing at the plugin root; and (2) the Codex manifest carries its own hooks **inline**, taking Codex's `Some(Inline)` branch (`resolve_manifest_hooks` in `codex-rs/core-plugins/src/manifest.rs`). The inline object is the functional definition of the Codex-native events *and* overrides discovery, so even if a `hooks/hooks.json` ever reappeared at the plugin root it would not be loaded. The Claude-scoped filename makes the #3362 collision structurally impossible; the inline manifest is what actually drives Codex's markers. +- **Why the Claude file sits at the conventional `hooks/hooks.json`.** Claude Code's loader discovers plugin hooks by convention at `hooks/hooks.json`; it does **not** honor the string-path `hooks` override in `plugin.json` for plugin loads. A Claude-scoped filename (`hooks/claude-hooks.json`) therefore loads *nothing* — `/hooks` shows no worktrunk handlers and the 🤖/💬 markers stop updating, silently ([#3417](https://github.com/max-sixty/worktrunk/issues/3417)). So the file must live at the conventional path. +- **Why inline for Codex, not a path or an absent key.** Claude and Codex share one payload dir, and Codex *also* auto-discovers `hooks/hooks.json` at the plugin root by convention (`DEFAULT_HOOKS_CONFIG_FILE`, the `None` branch of `load_plugin_hooks`) — which once surfaced Worktrunk's *Claude* events in a Codex session ([#3362](https://github.com/max-sixty/worktrunk/issues/3362)). The Codex manifest carries its own hooks **inline**, taking Codex's `Some(Inline)` branch (`resolve_manifest_hooks` in `codex-rs/core-plugins/src/manifest.rs`), which **overrides** convention discovery. The inline object is both the functional definition of the Codex-native events and the thing that keeps Codex off the shared `hooks/hooks.json`, so the two toolchains coexist on one file: Claude discovers it, Codex ignores it. (An earlier revision Claude-scoped the filename as belt-and-suspenders against #3362, but that broke Claude's discovery — #3417 — and the inline override already made it redundant.) - **Why `$PLUGIN_ROOT`, not `$CLAUDE_PLUGIN_ROOT`.** Codex exports both to hook commands (`PLUGIN_ROOT` native, `CLAUDE_PLUGIN_ROOT` as an OOTB-compat alias — `codex-rs/hooks/src/engine/discovery.rs`). The Codex file uses the native `$PLUGIN_ROOT` so nothing Claude-branded appears in a Codex session. The events (Codex's `HookEventsToml` vocabulary, verified against `codex-rs/config/src/hook_config.rs`): diff --git a/plugins/worktrunk/hooks/claude-hooks.json b/plugins/worktrunk/hooks/hooks.json similarity index 100% rename from plugins/worktrunk/hooks/claude-hooks.json rename to plugins/worktrunk/hooks/hooks.json diff --git a/plugins/worktrunk/plugin.json b/plugins/worktrunk/plugin.json index e05663bfd0..9c74683d78 100644 --- a/plugins/worktrunk/plugin.json +++ b/plugins/worktrunk/plugin.json @@ -4,7 +4,7 @@ "author": { "name": "Worktrunk" }, - "hooks": "./hooks/claude-hooks.json", + "hooks": "./hooks/hooks.json", "skills": [ "./skills/worktrunk", "./skills/wt-switch-create" diff --git a/tests/integration_tests/config_show.rs b/tests/integration_tests/config_show.rs index a978702eaf..d774cbd4d2 100644 --- a/tests/integration_tests/config_show.rs +++ b/tests/integration_tests/config_show.rs @@ -3917,13 +3917,12 @@ fn test_codex_plugin_metadata_is_valid_json() { // (`hooks` object, not a path). This is the functional definition of its // Codex-native events, and it also overrides Codex's convention discovery // (an absent `hooks` key makes Codex auto-discover a `hooks/hooks.json` at - // the plugin root). The Claude hooks file is Claude-scoped - // (`hooks/claude-hooks.json`), so that discovery lookup finds nothing — the - // #3362 collision (Codex surfacing the *Claude* file's events) is gone - // structurally, and inline still overrides discovery should any file - // reappear at the conventional path. Codex added a `Stop` turn-end event - // (post codex-cli 0.130.0), so 🤖 returns to 💬 at turn end. See CLAUDE.md - // → "Plugin Layout". + // the plugin root). The Claude hooks file sits at that same conventional + // `hooks/hooks.json` (Claude's loader discovers it there — #3417), but the + // inline Codex override means Codex never loads it, so the #3362 collision + // (Codex surfacing the *Claude* file's events) stays closed. Codex added a + // `Stop` turn-end event (post codex-cli 0.130.0), so 🤖 returns to 💬 at + // turn end. See CLAUDE.md → "Plugin Layout". let codex_hooks = plugin .get("hooks") .and_then(|h| h.get("hooks")) @@ -3950,7 +3949,8 @@ fn test_codex_plugin_metadata_is_valid_json() { "Codex hooks must not reference the Claude-branded $CLAUDE_PLUGIN_ROOT alias" ); // Hooks are inline, so the wrapper dir still holds only plugin.json — no - // separate hooks file to collide with the Claude `hooks/claude-hooks.json`. + // separate hooks file, and the inline override keeps Codex off the shared + // Claude `hooks/hooks.json`. assert!( !project_root .join("plugins/worktrunk/.codex-plugin/hooks") @@ -4009,26 +4009,33 @@ fn test_plugin_layout_is_consolidated() { // Claude manifest at the plugin root (no wrapper); hooks relative to it. let claude = json("plugins/worktrunk/plugin.json"); - assert_eq!(claude["hooks"], "./hooks/claude-hooks.json"); + assert_eq!(claude["hooks"], "./hooks/hooks.json"); assert!( - root.join("plugins/worktrunk/hooks/claude-hooks.json") - .exists() + root.join("plugins/worktrunk/hooks/hooks.json").exists() && root.join("plugins/worktrunk/hooks/wt.sh").exists(), "Claude hooks must live at the plugin root's hooks/" ); - // The Claude hooks file is Claude-scoped (claude-hooks.json), NOT the - // conventional hooks/hooks.json. A file at that conventional plugin-root - // path would be auto-discovered by Codex's convention loader and resurface - // the #3362 collision; the Claude-scoped name makes that impossible - // structurally rather than only overriding it inline in the Codex manifest. + // The Claude hooks file must sit at the conventional hooks/hooks.json. + // Claude Code discovers plugin hooks by that convention path and does NOT + // honor plugin.json's string-path `hooks` override for plugin loads, so a + // Claude-scoped name (hooks/claude-hooks.json) silently stops the hooks + // from loading — /hooks shows nothing and the 🤖/💬 markers never update + // (#3417). Sitting at the conventional path does NOT resurface the #3362 + // Codex collision: the Codex manifest defines its hooks inline (verified + // above), taking Codex's Some(Inline) branch, which overrides convention + // discovery — so Codex never loads this file even though it is now at the + // discoverable path. Structural scoping via the filename is what broke + // Claude; the inline Codex manifest is what actually keeps Codex clean. assert!( - !root.join("plugins/worktrunk/hooks/hooks.json").exists(), - "the Claude hooks file must be Claude-scoped (hooks/claude-hooks.json), not the \ - conventional hooks/hooks.json that Codex would auto-discover (#3362)" + !root + .join("plugins/worktrunk/hooks/claude-hooks.json") + .exists(), + "the Claude hooks file must sit at the conventional hooks/hooks.json that Claude \ + Code's loader discovers, not a Claude-scoped name the string-path override can't rescue (#3417)" ); assert!( - !read("plugins/worktrunk/hooks/claude-hooks.json").contains(".claude-plugin/hooks/"), - "claude-hooks.json must reference $CLAUDE_PLUGIN_ROOT/hooks/wt.sh, not the old wrapper path" + !read("plugins/worktrunk/hooks/hooks.json").contains(".claude-plugin/hooks/"), + "hooks.json must reference $CLAUDE_PLUGIN_ROOT/hooks/wt.sh, not the old wrapper path" ); // The description is duplicated across the Claude marketplace pointer and @@ -4104,8 +4111,8 @@ fn test_plugin_layout_is_consolidated() { // `--force-delete` silently discards committed-but-unpushed work — the only // recovery path is `git fsck`. The safe default retains unmerged branches // and prints a `wt remove -D ` hint for the user to act on. - let hooks = read("plugins/worktrunk/hooks/claude-hooks.json"); - let hooks_json = json("plugins/worktrunk/hooks/claude-hooks.json"); + let hooks = read("plugins/worktrunk/hooks/hooks.json"); + let hooks_json = json("plugins/worktrunk/hooks/hooks.json"); // Claude hands each hook command to the user's LOGIN shell before `bash` // launches, so the outer command line must parse under fish/zsh/bash. fish @@ -4183,7 +4190,7 @@ fn test_plugin_layout_is_consolidated() { fn test_claude_hook_commands_parse_in_all_shells() { let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")); let hooks_json: serde_json::Value = serde_json::from_str( - &fs::read_to_string(root.join("plugins/worktrunk/hooks/claude-hooks.json")).unwrap(), + &fs::read_to_string(root.join("plugins/worktrunk/hooks/hooks.json")).unwrap(), ) .unwrap();