diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 9d987a8..1efddcf 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -12,6 +12,6 @@ "license": "AGPL-3.0-or-later", "keywords": ["mcp", "colgrep", "semantic-search", "code-search", "agents"], "mcpServers": "./.claude-plugin/mcp.json", - "hooks": ["./hooks/hooks.json", "./hooks/claude-code.json"], + "hooks": "./hooks/worktree-remove.json", "skills": "./skills/" } diff --git a/AGENTS.md b/AGENTS.md index f0d771b..22b0ea0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,7 +57,7 @@ claude plugin install colgrep-mcp-dev@cracking-shells # from the repo's market | `server/pyproject.toml` | the only version source, dependencies, `[tool.commitizen]`, `[tool.ruff]`, pytest config | `cz bump` for the version; humans/agents for the rest | | `plugin.json`, `mcp.json`, `.claude-plugin/`, `.codex-plugin/`, `.agents/` | product plugin manifests for the three ecosystems; each MCP config (`.claude-plugin/mcp.json`, `.codex-plugin/mcp.json`, `mcp.json`) launches `uvx colgrep-mcp==`; `version` fields and the pins are written by `cz bump` only | launcher/packaging changes | | `skills/colgrep-search/SKILL.md` | the end-user skill that teaches agents when to use the tools | when tool semantics change | -| `hooks/` | the plugin hooks: `hooks.json` (events every hook-capable harness knows), `claude-code.json` (Claude-only events), `colgrep_policy.py` (one stdlib script serving all events); pinned by `server/tests/test_hooks.py` | when the search policy or the harness wiring changes | +| `hooks/` | the plugin hooks: `hooks.json` (events every hook-capable harness knows), one `.json` per event not every harness knows (`worktree-remove.json`), `colgrep_policy.py` (one stdlib script serving all events); pinned by `server/tests/test_hooks.py` | when the search policy or the harness wiring changes | | `dev/` | the `colgrep-mcp-dev` plugin: `skills//` (SKILL.md, `references/`, `scripts/`), `evals/-triggers/case.yaml`; versioned by `cz bump` with the product | maintainers, when a cycle learns something | | `CONTRIBUTING.md` | the commit vocabulary `cz check` enforces and the release recipe; the rest points at `landing-and-release` | with the commitizen config, never alone | | `CHANGELOG.md` | Keep-a-Changelog; sections since 0.1.0 are generated by `cz bump` | `cz bump` | diff --git a/README.md b/README.md index ba2e759..2f1f553 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,7 @@ The plugin also ships harness hooks (`hooks/`), so the policy the skill teaches | `PreToolUse` on `Grep` and `Bash` | Denies the built-in Grep tool and shell corpus searches (`grep -r`, `rg`, `find -exec grep`, `xargs grep`) inside a source corpus, with a reason naming `search`, `find_files` and `expand`. Single-file grep, `cmd \| grep`, `grep -c`/`-v`/`-o`, `rg --files` and file-name lookup stay allowed. Targets that are machine state (hidden directories, `~/Library`, temp directories outside a git work tree) are never gated. Prefix `COLGREP_BYPASS=1` to a command colgrep cannot serve. | | `WorktreeRemove` (Claude Code only) | Clears the colgrep index a removed worktree owned, never one it was folded into. | -`hooks/hooks.json` holds only events that Claude Code, Codex and Cursor all understand; `hooks/claude-code.json` holds the Claude-only event and is named by the Claude Code manifest alone. Codex loads a plugin's `hooks/hooks.json` and sets `CLAUDE_PLUGIN_ROOT` for it, but skips the hooks until you trust them once in `/hooks`. Cursor imports Claude Code hooks from `settings.json` files, not from plugins, so a Cursor project copies the three `hooks.json` entries into its `.claude/settings.json`. Agent Plugins 1.0 defines no hooks component and ignores the directory. +`hooks/hooks.json` holds only events that Claude Code, Codex and Cursor all understand; every other event gets its own file named after it, so `hooks/worktree-remove.json` holds `WorktreeRemove`. The two loaders read the manifest's `hooks` field with opposite semantics. Claude Code always loads `hooks/hooks.json` and treats the field as additional files, so the Claude Code manifest names just `hooks/worktree-remove.json`: a manifest that lists the default file too fails to load at install time with "Duplicate hooks file detected". Codex discovers `hooks/hooks.json` only when the manifest defines no `hooks` and an explicit value replaces that discovery, so the Codex manifest names `hooks/hooks.json`; Codex sets `CLAUDE_PLUGIN_ROOT` for it but skips the hooks until you trust them once in `/hooks`. Cursor imports Claude Code hooks from `settings.json` files, not from plugins, so a Cursor project copies the three `hooks.json` entries into its `.claude/settings.json`. Agent Plugins 1.0 defines no hooks component and ignores the directory. ## Configuration diff --git a/dev/skills/maintainer-policy/references/drift-tests.md b/dev/skills/maintainer-policy/references/drift-tests.md index d8a8d01..543471c 100644 --- a/dev/skills/maintainer-policy/references/drift-tests.md +++ b/dev/skills/maintainer-policy/references/drift-tests.md @@ -17,7 +17,7 @@ in a leaf or a PR template. | `tests/test_changelog.py` | Every `## ` version heading in `CHANGELOG.md` is shaped so commitizen's incremental changelog mode can parse it — a hand-written Keep-a-Changelog heading in the wrong shape would make `cz bump --changelog` duplicate that section instead of appending to it. | | `tests/test_readme.py` | The tool names listed in the Tools table of `README.md` and of `server/README.md` (the PyPI page) match the tools the server actually registers — a renamed or removed tool that isn't updated in a README fails here instead of shipping stale docs. | | `tests/test_packaging.py` | `server/LICENSE` is a byte copy of the repository `LICENSE` (hatchling only packages files under `server/`); `[project.urls]` point at the repository; no tracked text file carries a path from a maintainer's machine. | -| `tests/test_hooks.py` | The hook script's decisions for each event, fed the harness's JSON by hand; `hooks/hooks.json` carries only events every hook-capable harness knows and `hooks/claude-code.json` the Claude-only one; the Claude Code manifest names both files, the Codex manifest only the portable one. | +| `tests/test_hooks.py` | The hook script's decisions for each event, fed the harness's JSON by hand; `hooks/hooks.json` carries only events every hook-capable harness knows and every other file exactly one event, named after it (`hooks/worktree-remove.json`); the Claude Code manifest names only the extra files (it auto-loads `hooks.json`), the Codex manifest only the portable one (its field replaces default discovery). | | `tests/test_dev_plugin.py` | `AGENTS.md` stays under its line cap and names every skill under `dev/skills/`; every `dev/skills/*/SKILL.md` has front matter whose `name` matches its directory and a description long enough to state when to load it; the dev plugin manifest is the versioned skills plugin (no `mcpServers`); the marketplace lists both plugins from disjoint sources; the product plugin's skills path never resolves inside `dev/`. | ## Recipe for a new one diff --git a/dev/skills/stack-traps/SKILL.md b/dev/skills/stack-traps/SKILL.md index b28f7dc..557f86b 100644 --- a/dev/skills/stack-traps/SKILL.md +++ b/dev/skills/stack-traps/SKILL.md @@ -28,6 +28,7 @@ linked section, don't re-derive the trap from scratch. | `claude -p` or `claude plugin eval` fails with an OAuth error | claude-code | `references/claude-code.md#oauth` | | `claude --plugin-dir . mcp list` says Connected but your edit is not in the server, or the plugin fails right after a bump | claude-code | `references/claude-code.md#uvx-pin` | | the plugin's hooks don't fire, or still run the old text, after an edit or an update; Codex lists them but never runs them | claude-code | `references/claude-code.md#plugin-hooks` | +| `claude plugin list` shows the plugin `failed to load` with `Duplicate hooks file detected`, while `--plugin-dir` lists every hook | claude-code | `references/claude-code.md#hooks-manifest-duplicate` | | an implementer reports "leaf file missing", or its branch is based on `main` instead of the campaign branch | claude-code | `references/claude-code.md#agent-worktree` | | a subagent said it was watching CI or would follow up, and nothing happened | claude-code | `references/claude-code.md#worker-turn` | | a Bash command is blocked, including inside a heredoc that only mentions the search pattern | machine | `references/machine.md#shell-hook` | @@ -61,7 +62,9 @@ linked section, don't re-derive the trap from scratch. - `references/claude-code.md` — root `.mcp.json` vs plugin-scope `.claude-plugin/mcp.json`, marketplace vs plugin namespaces, `claude -p` on an expired OAuth session, the `uvx colgrep-mcp==` pin, plugin - hooks loading (reload, Codex trust, the portable/Claude-only split), the + hooks loading (reload, Codex trust, the portable/per-event file split, the + auto-loaded `hooks/hooks.json` a Claude manifest must not name again and a + Codex manifest must), the Agent tool's worktree base and the end of a subagent's turn. - `references/machine.md` — the plugin's shell-search hook and `COLGREP_BYPASS=1`, Windows CI's two known causes, the detached-worktree trick for `main`, the diff --git a/dev/skills/stack-traps/references/claude-code.md b/dev/skills/stack-traps/references/claude-code.md index f3a573f..b734e86 100644 --- a/dev/skills/stack-traps/references/claude-code.md +++ b/dev/skills/stack-traps/references/claude-code.md @@ -131,9 +131,53 @@ Code; in Codex open `/hooks` and trust; for Cursor copy the three script without any harness by piping it the event JSON — `server/tests/test_hooks.py` does exactly that through `sys.executable`, so `uv run pytest tests/test_hooks.py` is the fastest check. Keep -Claude-only events (`WorktreeRemove`) in `hooks/claude-code.json`, never in -the portable `hooks/hooks.json` a Codex parser also reads; -`test_hooks.py` pins the split. +Claude-only events (`WorktreeRemove`) in their own per-event file +(`hooks/worktree-remove.json`), never in the portable `hooks/hooks.json` a +Codex parser also reads, and name only the per-event files from the Claude +Code manifest (`#hooks-manifest-duplicate`); `test_hooks.py` pins the split. + +## `claude plugin list` says "failed to load: Duplicate hooks file detected" {#hooks-manifest-duplicate} + +**Symptom**: after `claude plugin install` or `claude plugin update`, the +plugin shows `✘ failed to load` with `Hook load failed: Duplicate hooks file +detected: ./hooks/hooks.json resolves to already-loaded file +~/.claude/plugins/cache//colgrep-mcp//hooks/hooks.json`, +and nothing of the plugin — hooks, MCP server, skill — is available; yet +`claude --plugin-dir . plugin details colgrep-mcp` on the very same tree lists +all four hooks and `claude plugin validate .` passes. + +**Cause**: Claude Code loads `hooks/hooks.json` automatically and reads the +manifest's `hooks` field as *additional* files only (the plugins reference +documents the field with the example `"./my-extra-hooks.json"` and files +hooks under "own merge rules"; the error text states the rule outright). A +manifest listing `./hooks/hooks.json` names the default twice, and the +marketplace loader refuses the whole plugin — observed with Claude Code +2.1.270 on the 0.4.0 and 0.5.0 installs. Neither `--plugin-dir`, `plugin +details` nor `plugin validate` runs that check, so every tree-level gate in +`AGENTS.md` stayed green on a manifest the install path rejected. harness_wiring +R01 §C2 designed the manifest as an array of both files before the rule was +documented; the manifest carried it from 0.4.0 until this fix. + +Codex reads the same field the other way round: it discovers +`hooks/hooks.json` only when the manifest defines no `hooks`, and an explicit +value *replaces* that discovery instead of adding to it (Codex plugin docs, +"Build a plugin"). No single manifest value serves both loaders, which is why +the two manifests differ. + +**What to do**: the Claude Code manifest's `hooks` names only the per-event +files — today the single string `"./hooks/worktree-remove.json"` — and never +`./hooks/hooks.json`; the Codex manifest names `./hooks/hooks.json` and never +a per-event file whose event it may not know (R01 risk 1). +`test_hooks.py::test_manifests_name_the_hook_files_per_ecosystem` pins both. +Name every non-portable hook file after the one event it holds +(`WorktreeRemove` → `worktree-remove.json`): `hooks.json` is the name both +loaders claim by default, and a second generic name beside it (the former +`claude-code.json`) is the kind a loader could claim next; the per-event test +enforces the stem. To check a manifest the way an install does, register a scratch directory marketplace (a `marketplace.json` with a +throwaway `name` whose plugin `source` is a copy of the tree without `.git` +and `.venv`), `claude plugin install colgrep-mcp@`, read +`claude plugin list`, then uninstall and `claude plugin marketplace remove` it. +`--plugin-dir` is not that check. ## An implementer's worktree is based on `main`, not the campaign branch {#agent-worktree} diff --git a/hooks/claude-code.json b/hooks/claude-code.json deleted file mode 100644 index c5a4749..0000000 --- a/hooks/claude-code.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "description": "colgrep-mcp: Claude Code-only events. Named from .claude-plugin/plugin.json only, never from the Codex manifest, so a parser that knows no WorktreeRemove never sees it. Clears the colgrep index a removed worktree owned (never one it was folded into).", - "hooks": { - "WorktreeRemove": [ - { - "hooks": [ - { - "type": "command", - "command": "uv run --no-project --quiet python \"${CLAUDE_PLUGIN_ROOT}/hooks/colgrep_policy.py\"", - "timeout": 90 - } - ] - } - ] - } -} diff --git a/hooks/hooks.json b/hooks/hooks.json index 206b96d..509a2d9 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -1,5 +1,5 @@ { - "description": "colgrep-mcp: state the search policy at session and subagent start; deny the built-in Grep tool and shell corpus searches in favour of the colgrep MCP tools. Only events every hook-capable harness (Claude Code, Codex, Cursor) understands live here; Claude-only events are in claude-code.json.", + "description": "colgrep-mcp: state the search policy at session and subagent start; deny the built-in Grep tool and shell corpus searches in favour of the colgrep MCP tools. Only events every hook-capable harness (Claude Code, Codex, Cursor) understands live here, in the one file both Claude Code and Codex load by default; an event not every harness knows gets its own file named after it (worktree-remove.json).", "hooks": { "SessionStart": [ { diff --git a/hooks/worktree-remove.json b/hooks/worktree-remove.json new file mode 100644 index 0000000..2465e58 --- /dev/null +++ b/hooks/worktree-remove.json @@ -0,0 +1,16 @@ +{ + "description": "colgrep-mcp: the WorktreeRemove event, which only Claude Code fires. One file per event that not every hook-capable harness knows, named after the event; named from .claude-plugin/plugin.json only, never from the Codex manifest, so a parser that knows no WorktreeRemove never sees it. Clears the colgrep index a removed worktree owned (never one it was folded into).", + "hooks": { + "WorktreeRemove": [ + { + "hooks": [ + { + "type": "command", + "command": "uv run --no-project --quiet python \"${CLAUDE_PLUGIN_ROOT}/hooks/colgrep_policy.py\"", + "timeout": 90 + } + ] + } + ] + } +} diff --git a/server/tests/test_hooks.py b/server/tests/test_hooks.py index 6596c11..a05b052 100644 --- a/server/tests/test_hooks.py +++ b/server/tests/test_hooks.py @@ -5,11 +5,14 @@ the tests, so the Windows CI job is the portability oracle for it as it is for `fake_colgrep.py` (harness_wiring R01 §Validation). The drift guards pin what no ecosystem's loader checks for us: that the portable file names only -events every hook-capable harness understands (R01 §C1), that the Claude-only -file is disjoint from it and named by the Claude Code manifest alone (R01 -§C2), that every command launches the one script through the one launcher -(R01 §C3), and that the injected policy stays under Codex's context cap -(R01 §C4). +events every hook-capable harness understands (R01 §C1); that every other +file holds exactly one event not every harness knows and is named after it +(`worktree-remove.json`), so no file name sits near a loader default that +could claim it later (R01 §C2, amended); that the Claude Code manifest names +exactly those files and never the auto-loaded `hooks/hooks.json`, which fails +the install (stack-traps `claude-code.md#hooks-manifest-duplicate`); that +every command launches the one script through the one launcher (R01 §C3); +and that the injected policy stays under Codex's context cap (R01 §C4). """ from __future__ import annotations @@ -18,6 +21,7 @@ import importlib.util import json import os +import re import subprocess import sys import tempfile @@ -29,12 +33,13 @@ HOOKS_DIR = REPO_ROOT / "hooks" SCRIPT = HOOKS_DIR / "colgrep_policy.py" PORTABLE_FILE = HOOKS_DIR / "hooks.json" -CLAUDE_ONLY_FILE = HOOKS_DIR / "claude-code.json" +#: One file per event that not every harness knows, named after the event. +EXTRA_FILES = sorted(p for p in HOOKS_DIR.glob("*.json") if p != PORTABLE_FILE) #: Events documented by all three hook-capable ecosystems: Claude Code's hooks #: reference, Codex's "Hooks" doc (§Hooks) and Cursor's third-party-hooks -#: mapping table (R01 §C1). Anything outside this set belongs in the -#: Claude-only file. +#: mapping table (R01 §C1). Anything outside this set gets its own file, +#: named after the event. PORTABLE_EVENTS = { "SessionStart", "SessionEnd", @@ -324,12 +329,22 @@ def test_portable_file_names_only_events_every_harness_understands(): assert {"SessionStart", "SubagentStart", "PreToolUse"} <= events -def test_claude_only_file_is_disjoint_from_the_portable_file(): +def _kebab(event: str) -> str: + return re.sub(r"(?= {"worktree-remove"} def test_pre_tool_use_matches_grep_and_bash_only(): @@ -338,7 +353,7 @@ def test_pre_tool_use_matches_grep_and_bash_only(): def test_every_handler_launches_the_one_script_through_the_one_launcher(): - for path in (PORTABLE_FILE, CLAUDE_ONLY_FILE): + for path in (PORTABLE_FILE, *EXTRA_FILES): for handler in _commands(_load(path)): assert handler["type"] == "command", path.name command = handler["command"] @@ -350,12 +365,24 @@ def test_every_handler_launches_the_one_script_through_the_one_launcher(): def test_manifests_name_the_hook_files_per_ecosystem(): + """The two loaders read the same field with opposite semantics. Claude Code always loads + `hooks/hooks.json` and treats `hooks` as *additional* files: naming the default again fails + the whole plugin at install time ("Duplicate hooks file detected", Claude Code 2.1.270) while + `--plugin-dir` accepts it silently. Codex discovers `hooks/hooks.json` only when the manifest + defines no `hooks`, and an explicit value *replaces* that discovery. So the Claude manifest + names exactly the extra files (today all Claude-only) and the Codex manifest the portable + file (stack-traps `claude-code.md#hooks-manifest-duplicate`).""" claude = _load(REPO_ROOT / ".claude-plugin" / "plugin.json") codex = _load(REPO_ROOT / ".codex-plugin" / "plugin.json") agent = _load(REPO_ROOT / "plugin.json") - assert claude["hooks"] == ["./hooks/hooks.json", "./hooks/claude-code.json"] - assert codex["hooks"] == "./hooks/hooks.json", "Codex must never be pointed at the Claude-only file" + named = [claude["hooks"]] if isinstance(claude["hooks"], str) else list(claude["hooks"]) + assert "./hooks/hooks.json" not in named, ( + "Claude Code auto-loads hooks/hooks.json; naming it in the manifest fails the plugin" + ) + assert set(named) == {f"./hooks/{p.name}" for p in EXTRA_FILES}, "every extra file is Claude-only today" + assert claude["hooks"] == "./hooks/worktree-remove.json" + assert codex["hooks"] == "./hooks/hooks.json", "Codex: the field replaces default discovery, so name hooks.json" assert "hooks" not in agent, "Agent Plugins 1.0 defines no hooks component"