merge queue: checking #747 on main (282a0f8)

engine/hooks/hook-freshness/README.md

@@ -11,9 +11,10 @@ with `CATSTACK_HOOKS_REPO`), reads `git branch --show-current` and
 turn's context when the checkout is off `main` or behind it. Once per
 session, keyed by transcript path.
 
-Advisory only — never blocks. No network by default; set
-`CATSTACK_HOOK_FRESHNESS_FETCH=1` to allow a 3-second `git fetch` first, so
-the count is not itself stale. `CATSTACK_HOOK_FRESHNESS=0` silences it.
+Advisory only — never blocks. `CATSTACK_HOOK_FRESHNESS` picks the mode:
+`local` (the default) compares against the last `origin/main` you fetched,
+`fetch` first runs a 3-second `git fetch` so the count is not itself stale,
+and `off` silences it.
 Fails open on every error: no symlink, no git, a detached HEAD, a timeout.
 
 ## Files
@@ -28,6 +29,10 @@ Fails open on every error: no symlink, no git, a detached HEAD, a timeout.
 | Var | Effect |
 |-----|--------|
 | `CATSTACK_HOOKS_REPO` | Use this checkout instead of resolving the symlink. |
-| `CATSTACK_HOOK_FRESHNESS_FETCH=1` | Allow a short `git fetch origin main` first. |
-| `CATSTACK_HOOK_FRESHNESS=0` | Silence the advisory. |
+| `CATSTACK_HOOK_FRESHNESS=local` | Default. Count against the local `origin/main`, no network. |
+| `CATSTACK_HOOK_FRESHNESS=fetch` | Run a short `git fetch origin main` first. |
+| `CATSTACK_HOOK_FRESHNESS=off` | Silence the advisory (`0` also works). |
+
+`CATSTACK_HOOK_FRESHNESS_FETCH` is retired: the hook ignores it and says so
+in its advisory. An unknown value falls back to `local` with a note.
 | `HOOK_FRESHNESS_STATE_DIR` | Once-per-session marker directory. |

engine/hooks/hook-freshness/detect.py

@@ -7,7 +7,7 @@
 from `origin/main`, and returns one advisory line for the turn.
 
 Advisory only: no block, no LLM, no network unless
-CATSTACK_HOOK_FRESHNESS_FETCH=1. Fails open on every error.
+CATSTACK_HOOK_FRESHNESS=fetch. Fails open on every error.
 """
 from __future__ import annotations
 
@@ -24,6 +24,11 @@
 ANCHOR_LINK = os.path.join(os.path.expanduser("~"), ".claude", "hooks", "diu-stop")
 TRUNK = "origin/main"
 FETCH_TIMEOUT_SECS = 3
+
+MODE_FLAG = "CATSTACK_HOOK_FRESHNESS"
+RETIRED_FETCH_FLAG = "CATSTACK_HOOK_FRESHNESS_FETCH"
+OFF_VALUES = frozenset({"off", "0", "false", "no"})
+LOCAL_VALUES = frozenset({"", "local", "1", "true", "yes", "on"})
 GIT_TIMEOUT_SECS = 5
 
 MESSAGE = (
@@ -62,11 +67,35 @@ def resolve_repo(env=None, realpath=os.path.realpath, isdir=os.path.isdir):
     return repo if isdir(os.path.join(repo, ".git")) else None
 
 
+def freshness_mode(env):
+    """(mode, note). mode is off, local, or fetch; note names a value this
+    hook could not use, or None."""
+    raw = env.get(MODE_FLAG, "").strip().lower()
+    notes = []
+    if RETIRED_FETCH_FLAG in env:
+        notes.append(
+            f"hook-freshness: {RETIRED_FETCH_FLAG} is retired and ignored; "
+            f"set {MODE_FLAG}=fetch instead."
+        )
+    if raw in OFF_VALUES:
+        mode = "off"
+    elif raw == "fetch":
+        mode = "fetch"
+    elif raw in LOCAL_VALUES:
+        mode = "local"
+    else:
+        mode = "local"
+        notes.append(
+            f"hook-freshness: {MODE_FLAG}={env.get(MODE_FLAG)} is not off, local, or fetch; using local."
+        )
+    return mode, "\n".join(notes) or None
+
+
 def repo_state(repo, env=None, run=_run_git):
     """(branch, commits behind trunk) for the checkout, or (None, None)."""
     env = env if env is not None else os.environ
     try:
-        if env.get("CATSTACK_HOOK_FRESHNESS_FETCH") == "1":
+        if freshness_mode(env)[0] == "fetch":
             run(["fetch", "--quiet", "origin", "main"], repo, FETCH_TIMEOUT_SECS)
         branch = run(["branch", "--show-current"], repo)
         behind_raw = run(["rev-list", "--count", f"HEAD..{TRUNK}"], repo)
@@ -216,13 +245,14 @@ def decide(
 ):
     """Advisory context for this prompt, or None. Once per session."""
     env = env if env is not None else os.environ
-    if env.get("CATSTACK_HOOK_FRESHNESS") == "0":
+    mode, mode_note = freshness_mode(env)
+    if mode == "off":
         return None
     key = payload.get("transcript_path") or payload.get("transcriptPath") or ""
     if state and already_advised(key):
         return None
     missing, unreadable = unresolvable_hooks(settings_path=settings_path, load=load, exists=exists)
-    lines = [ln for ln in [unresolvable_advisory(missing, unreadable)] if ln]
+    lines = [ln for ln in [mode_note, unresolvable_advisory(missing, unreadable)] if ln]
     repo = resolve_repo(env=env)
     if repo:
         branch, behind = repo_state(repo, env=env, run=run)

engine/hooks/hook-freshness/tests/test_hooks.py

@@ -133,6 +133,41 @@ def test_missing_settings_file_reports_unchecked_through_decide(self):
     def test_no_hit_when_disabled_by_env(self):
         self.assertIsNone(detect.decide({}, env={"CATSTACK_HOOK_FRESHNESS": "0"}))
 
+    def test_no_hit_when_set_to_off(self):
+        settings = {"hooks": {"Stop": [{"hooks": [{"command": "python3 /nope/missing.py"}]}]}}
+
+        def run(value):
+            env = {"CATSTACK_HOOKS_REPO": "/nope/not/a/repo"}
+            if value is not None:
+                env["CATSTACK_HOOK_FRESHNESS"] = value
+            return detect.decide({}, env=env, state=False, load=lambda _p: settings)
+
+        self.assertIsNotNone(run(None))
+        self.assertIsNone(run("off"))
+
+    def test_mode_values(self):
+        self.assertEqual(detect.freshness_mode({}), ("local", None))
+        self.assertEqual(detect.freshness_mode({"CATSTACK_HOOK_FRESHNESS": " Fetch "}), ("fetch", None))
+        self.assertEqual(detect.freshness_mode({"CATSTACK_HOOK_FRESHNESS": "local"}), ("local", None))
+        self.assertEqual(detect.freshness_mode({"CATSTACK_HOOK_FRESHNESS": "false"})[0], "off")
+        mode, note = detect.freshness_mode({"CATSTACK_HOOK_FRESHNESS": "sometimes"})
+        self.assertEqual(mode, "local")
+        self.assertIn("CATSTACK_HOOK_FRESHNESS=sometimes", note)
+
+    def test_retired_fetch_flag_is_named_and_ignored(self):
+        mode, note = detect.freshness_mode({"CATSTACK_HOOK_FRESHNESS_FETCH": "1"})
+        self.assertEqual(mode, "local")
+        self.assertIn("CATSTACK_HOOK_FRESHNESS_FETCH is retired", note)
+        with tempfile.TemporaryDirectory() as tmp:
+            line = detect.decide(
+                {},
+                env={"CATSTACK_HOOKS_REPO": "/nope/not/a/repo", "CATSTACK_HOOK_FRESHNESS_FETCH": "1"},
+                state=False,
+                settings_path=os.path.join(tmp, "settings.json"),
+                load=lambda _p: {"hooks": {}},
+            )
+        self.assertIn("CATSTACK_HOOK_FRESHNESS_FETCH is retired", line)
+
     def test_fails_open_when_git_errors(self):
         with tempfile.TemporaryDirectory() as tmp:
             repo = os.path.join(tmp, "catstack")
@@ -148,12 +183,20 @@ def test_no_fetch_unless_opted_in(self):
             detect.repo_state(repo, env={}, run=fake_git(record=calls))
         self.assertNotIn("fetch", [c[0] for c in calls])
 
-    def test_fetch_when_opted_in(self):
+    def test_retired_fetch_flag_does_not_fetch(self):
         calls = []
         with tempfile.TemporaryDirectory() as tmp:
             repo = os.path.join(tmp, "catstack")
             os.makedirs(os.path.join(repo, ".git"))
             detect.repo_state(repo, env={"CATSTACK_HOOK_FRESHNESS_FETCH": "1"}, run=fake_git(record=calls))
+        self.assertNotIn("fetch", [c[0] for c in calls])
+
+    def test_fetch_when_opted_in(self):
+        calls = []
+        with tempfile.TemporaryDirectory() as tmp:
+            repo = os.path.join(tmp, "catstack")
+            os.makedirs(os.path.join(repo, ".git"))
+            detect.repo_state(repo, env={"CATSTACK_HOOK_FRESHNESS": "fetch"}, run=fake_git(record=calls))
         self.assertIn("fetch", [c[0] for c in calls])
 
     def test_fails_open_on_garbage_stdin(self):