hooks: 🐛 Keep primitive fire limits stable across releases

CHANGELOG.md

@@ -125,6 +125,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Primitive hooks keep their fire limits across capt-hook releases.**
+  `nudge()`, `gate()`, and other primitives used the framework's versioned install path
+  as their state identity, resetting `max_fires` on each release. They now key on the
+  declaring pack or repo module. Loader hooks use `loader.py`, and review FIX routing,
+  `capt-hook list`, and hook lint see the correct hook files. Existing counters restart
+  once after upgrading; state stored under the old keys is not migrated.
 - **`capt-hook helper install` reports each step as it runs.** It used to capture every
   step and print nothing until the end, so a multi-minute `package-install` looked like
   a hang. The reason a step failed went only to the update log, and the error said just

captain_hook/app.py

@@ -10,7 +10,7 @@
 from typing import TYPE_CHECKING, Any, get_args
 
 from captain_hook.conditions import matches_conditions
-from captain_hook.state import caller_file, hook_name
+from captain_hook.state import caller_file, framework_frame, hook_name
 from captain_hook.types import (
     TOOL_EVENTS,
     CustomCondition,
@@ -300,6 +300,7 @@ def on(
     mandatory: bool = False,
     transcript_events: int | None = None,
     on_incomplete: str | None = None,
+    source_file: str | None = None,
 ) -> Callable[[HookHandler], HookHandler]:
     reject_async_decision(events, async_)
     reject_mandatory_misuse(events, async_, mandatory)
@@ -324,12 +325,13 @@ def on(
 
     def decorator(fn: HookHandler) -> HookHandler:
         validate_handler_signature(fn)
+        defined = fn.__code__.co_filename
         _state.hooks.append(
             RegisteredHook(
                 spec=spec,
                 handler=fn,
                 name=fn.__name__,
-                source_file=fn.__code__.co_filename,
+                source_file=source_file or (caller_file() if framework_frame(defined) else defined),
             )
         )
         return fn

captain_hook/cli.py

@@ -31,6 +31,7 @@
     register_fault_announcements,
     register_pr_announcements,
     register_resource_provisioning,
+    stamp_identity,
 )
 from captain_hook.log import setup_logging
 from captain_hook.packs import manager, plugins
@@ -122,12 +123,14 @@ def discover(self, *, scope: DiscoveryScope = "all") -> list[manager.ResolvedPac
         for pack_ in packs:
             discover_pack(pack_.name, pack_.path)
         register_pack_tools(packs)
+        before = len(_state.hooks)
         # Resource provisioning is one shared SessionStart hook; register it once with the union.
         if resources := list(dict.fromkeys(r for pack_ in packs for r in pack_.descriptor.resources)):
             register_resource_provisioning(resources)
         # The PR announcer's gating all lives in collect_announcements, so it registers unconditionally.
         register_pr_announcements()
         register_fault_announcements()
+        stamp_identity(before, root=Path(__file__).parent)
         return packs
 
 

captain_hook/loader.py

@@ -131,7 +131,7 @@ def register_resource_provisioning(resources: Sequence[str]) -> None:
     so both event dispatch and settings generation see the async hook.
     """
 
-    @on(Event.SessionStart, async_=True)
+    @on(Event.SessionStart, async_=True, source_file=__file__)
     def provision_pack_resources(evt: BaseHookEvent) -> None:
         from captain_hook.util.model_cache import provision_resources
 
@@ -147,7 +147,7 @@ def register_pr_announcements() -> None:
     start tells the user what happened.
     """
 
-    @on(Event.SessionStart, max_fires=1)
+    @on(Event.SessionStart, max_fires=1, source_file=__file__)
     def announce_pr_status(evt: BaseHookEvent) -> HookResult | None:
         from captain_hook.review.announce import collect_announcements
         from captain_hook.types import Action, HookResult
@@ -166,7 +166,7 @@ def register_fault_announcements() -> None:
     reaches a person rather than only the daemon log.
     """
 
-    @on(Event.SessionStart, max_fires=1)
+    @on(Event.SessionStart, max_fires=1, source_file=__file__)
     def announce_faults(evt: BaseHookEvent) -> HookResult | None:
         from captain_hook.faults import drain
         from captain_hook.types import Action, HookResult

captain_hook/review/fix.py

@@ -48,8 +48,7 @@
 target verbatim (``repo`` ``None`` — fixed in place), but an installed-wheel or plugin-pack
 ``source_file`` carries no repo path, so the real hook comes from the decision ``kind``'s
 module prefix routed through the scan's :class:`~captain_hook.review.routing.PackIndex`. A
-``nudge()``/``gate()`` fire records the primitive file, and a ``hook()`` bundled in a pack
-records the wheel or plugin file — all of these route through the ``kind``: a
+hook bundled in a pack records the wheel or plugin file, so it routes through the ``kind``: a
 ``<pack>.<module>`` prefix naming a module a builtin pack ships targets the pack source inside
 captain-hook itself (``captain_hook/builtin_packs/<pack>/hooks/<module>.py``, repo captain-hook);
 a prefix naming a pack an enabled Claude Code plugin ships targets that plugin's ``plugin.json``
@@ -394,23 +393,23 @@ async def fire_message_target(
 def user_repo_source(source_file: str) -> bool:
     """A hook file living in the watched repo, not the installed wheel.
 
-    Installed-wheel source (a ``nudge()``/``gate()`` primitive fire, or a ``hook()`` bundled in a
-    builtin pack) carries ``captain_hook/`` in its path, so it resolves through the decision ``kind``'s
-    module prefix instead of being returned verbatim. Plugin-pack source is routed earlier.
+    Installed-wheel source (a hook bundled in a builtin pack) carries ``captain_hook/`` in its path,
+    so it resolves through the decision ``kind``'s module prefix instead of being returned verbatim.
+    Plugin-pack source is routed earlier.
     """
     return f"{CAPTAIN_HOOK_ROOT}/" not in source_file
 
 
-def plugin_rel_path(source_file: str, install_root: str) -> str | None:
-    """A plugin-pack ``source_file``'s path relative to its plugin install root, or ``None``.
+def root_rel_path(source_file: str, root: str) -> str | None:
+    """A ``source_file``'s path relative to a plugin install root or the watched repo root, or ``None``.
 
     Best effort: a plugin rooted below its repo root (a ``plugin/`` subdir) carries that offset in the
     result, which the fix PR resolves against the plugin's repo.
     """
     if not source_file:
         return None
-    source, root = Path(source_file).resolve(), Path(install_root).resolve()
-    return str(source.relative_to(root)) if source.is_relative_to(root) else None
+    source, base = Path(source_file).resolve(), Path(root).resolve()
+    return str(source.relative_to(base)) if source.is_relative_to(base) else None
 
 
 def plugin_route_for(decision: Decision, module: str, sep: str, index: PackIndex) -> PluginRoute | None:
@@ -438,10 +437,11 @@ def resolve_target(decision: Decision, index: PackIndex) -> Target | None:
         if route.repo is None:
             logger.bind(hook=decision.kind).debug("dropped plugin-pack misfire; plugin declares no repository")
             return None
-        rel = plugin_rel_path(decision.source_file, route.root)
+        rel = root_rel_path(decision.source_file, route.root)
         return Target(rel or decision.source_file, decision.kind, route.repo, module.split(".")[0] if sep else None)
     if user_repo_source(decision.source_file):
-        return Target(decision.source_file, decision.kind, repo=None, pack=None)
+        rel = root_rel_path(decision.source_file, str(index.root)) if index.root else None
+        return Target(rel or decision.source_file, decision.kind, repo=None, pack=None)
     if not sep or not module:
         return None
     match module.split("."):

captain_hook/review/routing.py

@@ -74,11 +74,13 @@ class PackIndex:
             to its :class:`PluginRoute`. Membership marks a kind as a plugin-pack misfire.
         plugin_dirs: The ``hooks/`` dir of each plugin pack mapped to its route — the source arm for a
             hook fired under a bare ``@on`` name (no ``<pack>.`` prefix in its ``kind``).
+        root: The watched repo's root, which a repo-local hook's ``source_file`` relativizes against.
     """
 
     builtins: Mapping[str, Path]
     plugin_prefixes: Mapping[str, PluginRoute] = field(default_factory=dict)
     plugin_dirs: Mapping[str, PluginRoute] = field(default_factory=dict)
+    root: Path | None = None
 
     @classmethod
     def load(cls, root: Path | None) -> PackIndex:
@@ -107,4 +109,4 @@ def load(cls, root: Path | None) -> PackIndex:
             route = PluginRoute(RepoKey(normalize_origin(repository)) if repository else None, plugin.root)
             prefixes[manager.pack_module_name(plugin.id)] = route
             dirs[str(plugin_discovery.plugin_pack_root(plugin) / manager.HOOKS_DIRNAME)] = route
-        return cls(builtins=builtins, plugin_prefixes=prefixes, plugin_dirs=dirs)
+        return cls(builtins=builtins, plugin_prefixes=prefixes, plugin_dirs=dirs, root=root)

tests/test_decisions_writer.py

@@ -161,7 +161,7 @@ def test_primitive_built_hook_keeps_source_pair(self, db_path: Path, tmp_path: P
 
         (row,) = rows(db_path)
         assert row.kind.split(":")[-1].startswith("nudge")
-        assert row.source_file.endswith("captain_hook/primitives/nudge.py")
+        assert row.source_file == __file__
         assert (row.event, row.action, row.message) == ("PreToolUse", "warn", "Remember to run tests")
         assert row.tool_name == "Edit"
         assert row.tool_digest == evt.tool_digest

tests/test_pack_imports.py

@@ -1,17 +1,39 @@
 from __future__ import annotations
 
+import json
+import os
+import subprocess
 import sys
 from pathlib import Path
 
 import pytest
 
+import captain_hook
 from captain_hook import app
 from captain_hook.loader import discover_pack
 
 RELATIVE_IMPORT_SRC = (
     "from ._common import SHARED\nfrom captain_hook import Event, hook\nhook(Event.PreToolUse, message=str(SHARED))\n"
 )
 ON_HANDLER_SRC = "from captain_hook import Event, on\n\n\n@on(Event.PostToolUse)\ndef check(evt):\n    return None\n"
+PRIMITIVES_SRC = (
+    "from captain_hook import gate, llm_nudge, nudge, rewrite_command, set_tool_input\n"
+    "nudge('Remember the release notes')\n"
+    "gate('Run the suite before stopping')\n"
+    "llm_nudge('Is the agent guessing?', message='Observe first')\n"
+    "set_tool_input('model', 'sonnet', tool='Agent')\n"
+    "rewrite_command('cat $$$ARGS', 'bat $$$ARGS')\n"
+)
+INSTALLED_PROBE = (
+    "import json, sys\n"
+    "from pathlib import Path\n"
+    "import captain_hook\n"
+    "from captain_hook.app import _state\n"
+    "from captain_hook.loader import discover_pack\n"
+    "discover_pack('ccx', Path(sys.argv[1]))\n"
+    "print(json.dumps({'package': captain_hook.__file__,"
+    " 'hooks': [[h.state_key, h._state_identity, h.source_file] for h in _state.hooks]}))\n"
+)
 
 
 @pytest.fixture
@@ -91,6 +113,58 @@ def test_state_key_stable_across_pack_root_moves(tmp_path: Path, isolate_modules
     assert key_a == key_b
 
 
+def installed_state_keys(install: Path, pack: Path) -> list[list[str]]:
+    install.mkdir()
+    (install / "captain_hook").symlink_to(Path(captain_hook.__file__).parent, target_is_directory=True)
+    probe = subprocess.run(
+        [sys.executable, "-c", INSTALLED_PROBE, str(pack)],
+        cwd=install,
+        env={**os.environ, "PYTHONPATH": str(install)},
+        capture_output=True,
+        text=True,
+        check=True,
+    )
+    loaded = json.loads(probe.stdout)
+    assert loaded["package"] == str(install / "captain_hook" / "__init__.py")
+    return loaded["hooks"]
+
+
+def test_primitive_state_keys_stable_across_capt_hook_installs(tmp_path: Path) -> None:
+    pack = _pack_root(tmp_path / "pack", PRIMITIVES_SRC)
+
+    first = installed_state_keys(tmp_path / "v1", pack)
+    second = installed_state_keys(tmp_path / "v2", pack)
+
+    assert len(first) == 5
+    assert first == second
+    assert {(identity, source) for _, identity, source in first} == {("ccx\0guard.py", str(pack / "guard.py"))}
+
+
+def test_framework_hooks_key_on_their_package_relative_path(tmp_path: Path, isolate_modules: None) -> None:
+    from captain_hook.cli import CliState
+
+    (hooks := tmp_path / ".claude" / "hooks").mkdir(parents=True)
+    CliState(root=tmp_path, hooks=str(hooks)).discover(scope="all")
+
+    framework = {"announce_pr_status", "announce_faults"}
+    assert {h._state_identity for h in app._state.hooks if h.name in framework} == {"loader.py"}
+
+
+def test_shared_decorator_keeps_each_handler_file(tmp_path: Path, isolate_modules: None) -> None:
+    pack = tmp_path / "pack"
+    pack.mkdir()
+    (pack / "_shared.py").write_text(
+        "from captain_hook import Event, on\n\npost = on(Event.PostToolUse, max_fires=1)\n"
+    )
+    for stem in ("alpha", "beta"):
+        (pack / f"{stem}.py").write_text("from ._shared import post\n\n\n@post\ndef check(evt):\n    return None\n")
+
+    app.reset()
+    discover_pack("ccx", pack)
+
+    assert sorted(h._state_identity for h in app._state.hooks) == ["ccx\0alpha.py", "ccx\0beta.py"]
+
+
 def test_state_key_differs_for_two_packs_same_hook_name(tmp_path: Path, isolate_modules: None) -> None:
     # Two packs each register an @on handler named "check"; the pack name in the identity keeps
     # their state keys (and thus max_fires counters) distinct.

tests/test_review_fix.py

@@ -454,6 +454,11 @@ class TestResolveTarget:
     def test_resolve_target(self, source_file: str, kind: str, expected: Target | None) -> None:
         assert resolve_target(make_decision(kind=kind, source_file=source_file), INDEX) == expected
 
+    def test_repo_hook_relativizes_against_the_watched_repo(self, tmp_path: Path) -> None:
+        index = PackIndex(builtins=INDEX.builtins, root=tmp_path)
+        decision = make_decision(kind="guard:nudge_deadbeef", source_file=str(tmp_path / ".claude/hooks/guard.py"))
+        assert resolve_target(decision, index) == Target(".claude/hooks/guard.py", "guard:nudge_deadbeef", None, None)
+
     def test_plugin_pack_routes_to_its_repository(self) -> None:
         index = PackIndex(builtins=INDEX.builtins, plugin_prefixes={NOTIFY_PREFIX: NOTIFY_ROUTE})
         target = resolve_target(make_decision(kind=NOTIFY_KIND, source_file=NOTIFY_SOURCE), index)