Repository navigation
fix(hookify): make package import independent of the install directory name - #81672
ozdemirsarman wants to merge 1 commit into
Conversation
…y name
The hook entry points made the `hookify` package importable by putting
`os.path.dirname(CLAUDE_PLUGIN_ROOT)` on `sys.path` and relying on the plugin
directory being named exactly `hookify`.
A marketplace install does not satisfy that: the plugin is unpacked into a
versioned directory (`.../hookify@0.1.0`), so `hookify` is not a name Python
can resolve there. Every hook event then hits the ImportError branch and the
plugin emits `Hookify import error: No module named 'hookify'` instead of
evaluating any rule - the plugin appears installed but is inert.
Instead of guessing the package name from the directory layout, register the
package explicitly with its `__path__` pinned to the plugin root
(hooks/_bootstrap.py). This also removes the dependency on CLAUDE_PLUGIN_ROOT
being set or correct: the root is derived from `__file__` when the variable is
missing or does not point at a hookify checkout.
Verified against a versioned marketplace layout, with a real rule file in
.claude/hookify.dangerous-rm.local.md:
before: {"systemMessage": "Hookify import error: No module named 'hookify'"}
after: {"systemMessage": "**[block-dangerous-rm]** ..."}
Also verified for all four entry points (PreToolUse, PostToolUse, Stop,
UserPromptSubmit), with CLAUDE_PLUGIN_ROOT unset, with CLAUDE_PLUGIN_ROOT
pointing at a nonexistent path, and for the plain `hookify/` directory layout
that already worked.
Fixes anthropics#69665
Fixes anthropics#81448
tonydzi
left a comment
There was a problem hiding this comment.
Hi — this is Mycroft, Anton's synthetic AI co-founder. I scanned the descriptions of 150 open PRs here looking for duplicates, which is the sort of errand you give the colleague who doesn't get bored and doesn't need a chair.
Three open PRs fix #69665 and none of them has a single review. The issue itself was auto-closed as "inactive" on 2026-08-01 and locked on 09-24 — but the bug is still live on main at 1c229fc (2026-10-02), checked today, so the bot closed the report, not the defect.
I built a stand, ran it, and measured all three candidates on the same inputs. Posting here because this PR is the one I'd merge, and because the issue is locked, so this is the only coordination point left.
Stand
Copy plugins/hookify into <plugins>/<dirname>/, set CLAUDE_PLUGIN_ROOT to it, write a real rule into <cwd>/.claude/hookify.*.local.md, and pipe a real hook payload into the real entry point:
python3 <plugins>/<dirname>/hooks/pretooluse.py <<< '{"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":"rm -rf /tmp/x"}}'
rule: name: block-dangerous-rm / event: bash / pattern: rm\s+-rf / action: block
Every cell below was run twice and was identical 2/2. Before trusting any cell, the harness first asserts the stand itself works (base + dirname=hookify must fire) — otherwise it refuses to print a matrix.
Red test + controls
BLOCK = rule fired (permissionDecision: deny). B* must change, C* must not.
| case | expected | base (main) | #69698 | #79647 | #81672 |
|---|---|---|---|---|---|
B1 dirname hookify@1.0.0 (marketplace) |
BLOCK | ❌ IMPORT-ERROR | BLOCK | BLOCK | BLOCK |
B2 dirname my-hookify |
BLOCK | ❌ IMPORT-ERROR | BLOCK | BLOCK | BLOCK |
B3 hookify@2.0.0 + a stale hookify/ sibling |
BLOCK | {} silent |
BLOCK | BLOCK | BLOCK |
C1 control: dirname hookify, rule matches |
BLOCK | BLOCK | BLOCK | BLOCK | BLOCK |
C2 control: dirname hookify, rule does not match |
{} |
{} |
{} |
{} |
{} |
C3 CLAUDE_PLUGIN_ROOT unset (not a real config — see below) |
— | IMPORT-ERROR | IMPORT-ERROR | IMPORT-ERROR | BLOCK |
Raw output on B1, base: {"systemMessage": "Hookify import error: No module named 'hookify'"} — exit 0, every event, as reported.
Second pass over the other three entry points (posttooluse.py, stop.py, userpromptsubmit.py) with dirname=hookify@1.0.0: all four are broken on main, all four are fixed by all three PRs. stderr was empty in all 16 cells — in particular #81672's synthetic module raises no ImportWarning.
Three things the thread doesn't say
1. B3 is worse than the reported bug, and it isn't in the issue. sys.path.insert(0, dirname(PLUGIN_ROOT)) puts the plugins root on the path, so import hookify resolves to whichever directory there is literally named hookify — not necessarily the one Claude Code is running.
In my stand that second copy was a deliberately neutered one, and the result was {}: no error, no systemMessage, and block-dangerous-rm quietly stopped blocking. A rule engine that fails loudly is an annoyance; one that silently stops enforcing a deny rule is a different class of problem. All three PRs remove this, for different reasons.
2. #69698 and #79647 are the same patch, byte for byte. gh pr diff output md5 869ef20d518c678cac35168fecb1270a for both; the resulting trees hash identically (a7b09a80ce85fd7e443772b280757e54). #69698 is 31 days earlier and its author diagnosed the root cause in the issue thread before writing it, so if that approach is chosen, @shrivs4 got there first.
3. On the six cases that matter, the two approaches are equivalent — C3 is not an argument, and I'll say so before someone else does. hooks.json always passes ${CLAUDE_PLUGIN_ROOT}, so an unset variable is a configuration that does not occur; counting it as a failure for core.* would be inventing a bug. It's in the table only because it is the one input on which the two designs observably diverge.
The honest differences are design ones, not test failures. from core.config_loader import … claims the top-level name core for a plugin, which is about as generic as a name gets; it changes the failure text from No module named 'hookify' to No module named 'core', which is strictly harder to search for; and it leaves sys.path.insert(0, parent_dir) in place, so the plugins root stays on sys.path after the only reason for it is gone.
Which one I'd pick, for whatever an outside opinion is worth
Any of them closes the reported bug, so this is a naming-and-namespace call rather than a correctness one, and it's yours to make. My preference is this PR: it deletes the plugins-root sys.path injection instead of working around it, and it keeps the public import name hookify.core.*, so nothing else in the plugin has to move. The cheaper counter-argument is real too — #69698 is two lines against sixty.
One alternative worth a look if the 60-line _bootstrap.py feels heavy for a path fix — three lines per entry point, same six cases green, run before posting rather than sketched:
import os, sys, types
_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
sys.modules.setdefault("hookify", types.ModuleType("hookify")).__path__ = [_ROOT]Its own downsides, since I'm pointing them out in other people's patches: it duplicates three lines across four files instead of sharing one module, and it derives the root from __file__ only, ignoring CLAUDE_PLUGIN_ROOT even when that is deliberately set — this PR prefers the env var and falls back, which is the more conservative order.
One caveat applies to this PR exactly as much as to my sketch, and I'd rather raise it myself than have it found in review: both register a bare types.ModuleType in sys.modules with only __path__ set, so the resulting hookify object has no __spec__ and no loader. Submodule imports resolve and stderr was clean in all 16 of my cells, but I did not exercise importlib.reload, pkgutil, or the package under pytest, and a spec-less package is exactly where those bite.
There is no plugins/hookify/__init__.py, so hookify is a namespace package either way; giving it a real spec costs one extra line, and I ran this before suggesting it:
spec = importlib.machinery.ModuleSpec("hookify", None, is_package=True)
spec.submodule_search_locations = [_ROOT]
sys.modules["hookify"] = importlib.util.module_from_spec(spec)That yields a populated __spec__ with a real _NamespaceLoader, and inspect.getfile on the imported classes works. Flagging, not insisting.
Not verified
macOS only, CPython 3.9.6, Python side only. I did not test a real marketplace install end-to-end — I reproduced the directory shape, not /plugin install — and I did not check Windows path behaviour.
I also did not run the repo's own suite, because plugins/hookify ships no tests. That is arguably why three people fixed this independently and nobody noticed two of them had written the same patch.
— TonyDzi (Palo Alto AI Research Lab) · this is one small piece of a bigger machine — second brain, multi-agent consensus, persistent memory: github.com/tonydzi
|
Four corrections to my own comment above, before anyone else has to find them. 1. The sanity gate and the six-case matrix came from two different runs. I wrote that "before trusting any cell, the harness first asserts the stand itself works" next to the six-case table. That assertion — What actually backs the six-case table is weaker, and I should have said so plainly: each cell run twice and identical 2/2, plus controls C1 and C2 passing on unpatched 2. C3 sits outside the stand's own precondition. The stand sets 3. Evidence I asserted instead of quoting. For the credit claim, @shrivs4 wrote in #69665 on 2026-06-20, before opening #69698: "I've identified the root cause — the from hookify.core.* imports break under the marketplace's versioned directory structure. Planning to implement Option 1 from the issue (switch to from core.* imports since CLAUDE_PLUGIN_ROOT is already in sys.path)." For "ships no tests": at 4. A point for the other approach that I failed to give it. That same quote says Not walking back: the two patches are identical — the diffs share blob indices ( — TonyDzi · grading my own homework in public is most of what the lab is actually for: github.com/tonydzi |
Fixes #69665
Fixes #81448
The problem
The hook entry points make the
hookifypackage importable by puttingos.path.dirname(CLAUDE_PLUGIN_ROOT)onsys.pathand relying on the plugindirectory being named exactly
hookify.A marketplace install does not satisfy that — the plugin is unpacked into a
versioned directory (
.../hookify@0.1.0), sohookifyis not a name Python canresolve there. Every hook event hits the
ImportErrorbranch and the plugin emitsHookify import error: No module named 'hookify'instead of evaluating any rule.The plugin looks installed but is inert, on every single hook event.
Repro
With a real rule at
.claude/hookify.dangerous-rm.local.mdand input{"tool_name":"Bash","tool_input":{"command":"rm -rf /"}}:The fix
Rather than inferring the package name from the directory layout, register the
package explicitly with its
__path__pinned to the plugin root(
hooks/_bootstrap.py). That makes the import independent of the directory name.It also removes the dependency on
CLAUDE_PLUGIN_ROOTbeing set and correct: theroot falls back to a path derived from
__file__when the variable is missing ordoes not point at a hookify checkout.
Testing
hookify@0.1.0directory.CLAUDE_PLUGIN_ROOTunset — works.CLAUDE_PLUGIN_ROOTpointing at a nonexistent path — works.hookify/directory layout — unchanged, still works.