diff --git a/README.md b/README.md index cada9528b4..a43b9e3b86 100644 --- a/README.md +++ b/README.md @@ -208,7 +208,7 @@ for example `graphify claude install --project` or `graphify codex install --pro > **Git hooks and uv tool / pipx:** `graphify hook install` embeds the current interpreter path directly into the hook scripts at install time, so the post-commit hook fires correctly even in GUI git clients and CI runners where `~/.local/bin` is not on PATH. If you reinstall or upgrade graphify, re-run `graphify hook install` to refresh the embedded path. -> **Strict mode (Claude Code):** `graphify install --project --strict` makes the assistant actually use the graph. The default install *nudges* it to run `graphify query` before reading files; strict mode *blocks* the first raw source read of a session and redirects it to the graph, then reverts to the nudge (so it fires at most once per session and never gets stuck). Toggle at runtime with `GRAPHIFY_HOOK_STRICT=1`/`0`; the default install is unchanged (soft nudge). +> **Strict mode (Claude Code):** `graphify install --project --strict` makes the assistant actually use the graph. The default install *nudges* it to run `graphify query` before reading files; strict mode *blocks* the first raw source read of a session and redirects it to the graph, then reverts to the nudge (so it fires at most once per session and never gets stuck). Toggle at runtime with `GRAPHIFY_HOOK_STRICT=1`/`0`; the default install is unchanged (soft nudge). The soft nudge itself is bounded per agent (each subagent has its own budget): the first one always fires, later ones are skipped while a recent `graphify query`/`explain`/`path` shows the assistant is already oriented, and it fires at most `GRAPHIFY_HOOK_NUDGE_CAP` times per agent (default 5; `0` disables the cap).
Pick your platform (20+ assistants, click to expand) diff --git a/graphify/cli.py b/graphify/cli.py index 458280d72d..0941cdd733 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -707,6 +707,73 @@ def _query_stamp_fresh() -> bool: return False +def _nudge_allowed(payload: dict) -> bool: + """Whether to emit a soft read/search nudge for this tool call (#3435). + + The nudge is advisory context that used to be re-injected on EVERY qualifying + Read/Glob/Grep/Bash call — thousands of times per session, far more tokens + than the graph queries it asks for. Budgets are per agent: the session id, + plus the ``agent_id`` Claude Code sends for subagents (which share the + parent's session id but start with an empty context). Per agent: + + * the first nudge always fires. The query stamp is project-wide and cannot + say which agent queried, so it must not silence a fresh subagent or session; + * after that, a fresh query stamp (query/explain/path within + GRAPHIFY_HOOK_STRICT_TTL) means someone oriented recently, so no nudge; + * at most GRAPHIFY_HOOK_NUDGE_CAP nudges (default 5; 0 or a non-number + disables the cap), counted in a marker next to the strict-mode ones. + + Calls without a session id cannot be counted: only the stamp applies. + Fail-open: on a marker error, only the stamp applies. + """ + from graphify.paths import out_path, write_text_atomic + import hashlib + stamp = _query_stamp_fresh() + try: + cap = int(os.environ.get("GRAPHIFY_HOOK_NUDGE_CAP", "5")) + except ValueError: + cap = 0 + key = re.sub(r"[^A-Za-z0-9_-]", "_", str(payload.get("session_id") or ""))[:64] + if not key: + return not stamp + agent = str(payload.get("agent_id") or "") + if agent: + # Hashed so long ids cannot truncate two agents into one key. + key += "-" + hashlib.sha256(agent.encode("utf-8")).hexdigest()[:16] + try: + d = out_path("cache", "hook_sessions") + d.mkdir(parents=True, exist_ok=True) + marker = d / f"{key}.nudges" + try: + count = int(marker.read_text(encoding="utf-8").strip() or "0") + except (OSError, ValueError): + count = 0 + if count and stamp: + return False + if 0 < cap <= count: + return False + write_text_atomic(marker, str(count + 1)) + if count == 0: + _gc_hook_session_markers(d) + return True + except Exception: + return not stamp + + +def _gc_hook_session_markers(d: "Path") -> None: + """Best-effort removal of per-session hook markers older than 24h.""" + try: + cutoff = time.time() - 86400 + for entry in os.scandir(d): + try: + if entry.stat().st_mtime < cutoff: + os.unlink(entry.path) + except OSError: + pass + except OSError: + pass + + def _mark_session_denied(session_id: str) -> bool: """Atomically claim a one-time strict block for this session. Returns True only on the FIRST call for a given session id (O_EXCL create wins once); every later @@ -721,16 +788,7 @@ def _mark_session_denied(session_id: str) -> bool: d.mkdir(parents=True, exist_ok=True) fd = os.open(str(d / f"{sid}.denied"), os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o644) os.close(fd) - try: - cutoff = time.time() - 86400 - for entry in os.scandir(d): - try: - if entry.stat().st_mtime < cutoff: - os.unlink(entry.path) - except OSError: - pass - except OSError: - pass + _gc_hook_session_markers(d) return True except FileExistsError: return False @@ -866,7 +924,8 @@ def _run_hook_guard(kind: str, strict: bool = False) -> None: # grep. Nudge-only, even in strict mode — see the docstring. is_grep_tool = not cmd_str and bool(t.get("pattern")) is_bash_search = bool(cmd_str) and _bash_invokes_search(cmd_str) - if (is_grep_tool or is_bash_search) and out_path("graph.json").is_file(): + if (is_grep_tool or is_bash_search) and out_path("graph.json").is_file() \ + and _nudge_allowed(d): sys.stdout.write(_SEARCH_NUDGE) elif kind == "read": vals = [str(t.get("file_path") or ""), str(t.get("pattern") or ""), str(t.get("path") or "")] @@ -926,7 +985,8 @@ def _run_hook_guard(kind: str, strict: bool = False) -> None: except Exception: pass if stale: - sys.stdout.write(_READ_NUDGE_STALE) + if _nudge_allowed(d): + sys.stdout.write(_READ_NUDGE_STALE) return # Strict block: Read tool only, first time per session, not recently # oriented, and the file is demonstrably indexed. @@ -937,7 +997,8 @@ def _run_hook_guard(kind: str, strict: bool = False) -> None: and _mark_session_denied(str(d.get("session_id") or "")): sys.stdout.write(_READ_DENY) return - sys.stdout.write(_READ_NUDGE) + if _nudge_allowed(d): + sys.stdout.write(_READ_NUDGE) except Exception: pass diff --git a/tests/test_hook_guard.py b/tests/test_hook_guard.py index 869f7ccb88..7bc8f06559 100644 --- a/tests/test_hook_guard.py +++ b/tests/test_hook_guard.py @@ -202,6 +202,96 @@ def _boom(*a, **k): assert out.strip() == "" +# --------------------------------------------------------------------------- # +# soft nudge bounds (#3435): fresh query stamp suppresses, per-session cap +# --------------------------------------------------------------------------- # +def _read_payload(sid="s1", agent=None): + p = {"session_id": sid, "tool_name": "Read", "tool_input": {"file_path": "src/app.py"}} + if agent: + p["agent_id"] = agent + return p + + +def _search_payload(sid="s1"): + return {"session_id": sid, "tool_input": {"command": "grep -rn foo ."}} + + +def _fresh_stamp(tmp_path): + stamp = tmp_path / "graphify-out" / "cache" / "last_query_stamp" + stamp.parent.mkdir(parents=True, exist_ok=True) + stamp.write_text("x", encoding="utf-8") + + +def test_fresh_query_stamp_silences_nudges_after_the_first(tmp_path, monkeypatch): + _fresh_stamp(tmp_path) + # The stamp is project-wide and cannot tell which agent queried, so each + # agent still gets its first nudge; later ones are silenced. + assert "MANDATORY" in _invoke("read", _read_payload(), tmp_path, monkeypatch) + assert _invoke("read", _read_payload(), tmp_path, monkeypatch) == "" + assert _invoke("search", _search_payload(), tmp_path, monkeypatch) == "" + + +def test_fresh_query_stamp_silences_nudges_without_session_id(tmp_path, monkeypatch): + _fresh_stamp(tmp_path) + payload = {"tool_name": "Read", "tool_input": {"file_path": "src/app.py"}} + assert _invoke("read", payload, tmp_path, monkeypatch) == "" + + +def test_subagents_get_their_own_nudge_budget(tmp_path, monkeypatch): + # Claude Code subagents share the parent's session_id and differ by agent_id. + monkeypatch.setenv("GRAPHIFY_HOOK_NUDGE_CAP", "1") + sid = "s" * 64 # long ids must not truncate distinct agents into one key + assert "MANDATORY" in _invoke("read", _read_payload(sid), tmp_path, monkeypatch) + assert _invoke("read", _read_payload(sid), tmp_path, monkeypatch) == "" + assert "MANDATORY" in _invoke("read", _read_payload(sid, "agent-a"), tmp_path, monkeypatch) + assert _invoke("read", _read_payload(sid, "agent-a"), tmp_path, monkeypatch) == "" + assert "MANDATORY" in _invoke("read", _read_payload(sid, "agent-b"), tmp_path, monkeypatch) + + +def test_subagent_first_nudge_survives_parent_query(tmp_path, monkeypatch): + assert "MANDATORY" in _invoke("read", _read_payload(), tmp_path, monkeypatch) + _fresh_stamp(tmp_path) # parent queried + assert _invoke("read", _read_payload(), tmp_path, monkeypatch) == "" + assert "MANDATORY" in _invoke("read", _read_payload(agent="agent-a"), tmp_path, monkeypatch) + + +def test_nudges_capped_per_session(tmp_path, monkeypatch): + monkeypatch.setenv("GRAPHIFY_HOOK_NUDGE_CAP", "3") + outs = [_invoke("read", _read_payload("cap"), tmp_path, monkeypatch) for _ in range(3)] + assert all("MANDATORY" in o for o in outs) + assert _invoke("read", _read_payload("cap"), tmp_path, monkeypatch) == "" + assert _invoke("search", _search_payload("cap"), tmp_path, monkeypatch) == "" + # Another session has its own budget. + assert "MANDATORY" in _invoke("read", _read_payload("other"), tmp_path, monkeypatch) + + +def test_read_and_search_nudges_share_the_session_budget(tmp_path, monkeypatch): + monkeypatch.setenv("GRAPHIFY_HOOK_NUDGE_CAP", "2") + assert "MANDATORY" in _invoke("search", _search_payload("mix"), tmp_path, monkeypatch) + assert "MANDATORY" in _invoke("read", _read_payload("mix"), tmp_path, monkeypatch) + assert _invoke("search", _search_payload("mix"), tmp_path, monkeypatch) == "" + + +def test_nudge_cap_default_is_five(tmp_path, monkeypatch): + monkeypatch.delenv("GRAPHIFY_HOOK_NUDGE_CAP", raising=False) + outs = [_invoke("read", _read_payload("d"), tmp_path, monkeypatch) for _ in range(6)] + assert [bool(o) for o in outs] == [True] * 5 + [False] + + +@pytest.mark.parametrize("value", ["0", "-1", "not-a-number"]) +def test_nudge_cap_disabled(value, tmp_path, monkeypatch): + monkeypatch.setenv("GRAPHIFY_HOOK_NUDGE_CAP", value) + outs = [_invoke("read", _read_payload("nocap"), tmp_path, monkeypatch) for _ in range(8)] + assert all("MANDATORY" in o for o in outs) + + +def test_nudges_without_session_id_are_not_counted(tmp_path, monkeypatch): + monkeypatch.setenv("GRAPHIFY_HOOK_NUDGE_CAP", "1") + payload = {"tool_name": "Read", "tool_input": {"file_path": "src/app.py"}} + outs = [_invoke("read", payload, tmp_path, monkeypatch) for _ in range(3)] + assert all("MANDATORY" in o for o in outs) + + # --------------------------------------------------------------------------- # # gemini: BeforeTool contract (always allow; nudge only when a graph exists) # --------------------------------------------------------------------------- # diff --git a/tests/test_hook_strict.py b/tests/test_hook_strict.py index 1b34d13f98..878206d082 100644 --- a/tests/test_hook_strict.py +++ b/tests/test_hook_strict.py @@ -86,7 +86,10 @@ def test_fresh_query_stamp_suppresses_deny(tmp_path, monkeypatch): stamp.parent.mkdir(parents=True, exist_ok=True) stamp.write_text(str(time.time()), encoding="utf-8") out = _invoke("read", _read(f), tmp_path, monkeypatch, strict=True) + # Recently oriented: no block. This session's first nudge still fires, since + # the project-wide stamp cannot tell which agent queried (#3435). assert not _is_deny(out) and "MANDATORY" in out + assert _invoke("read", _read(f), tmp_path, monkeypatch, strict=True) == "" def test_expired_query_stamp_still_denies(tmp_path, monkeypatch): @@ -140,6 +143,13 @@ def test_needs_update_flag_softens(tmp_path, monkeypatch): assert not _is_deny(out) and "stale" in out.lower() +def test_stale_notice_shares_the_session_nudge_budget(tmp_path, monkeypatch): + f = _fixture(tmp_path, fresh=False) + env = {"GRAPHIFY_HOOK_NUDGE_CAP": "1"} + assert "stale" in _invoke("read", _read(f), tmp_path, monkeypatch, env=env).lower() + assert _invoke("read", _read(f), tmp_path, monkeypatch, env=env) == "" + + def test_glob_never_denies(tmp_path, monkeypatch): _fixture(tmp_path) payload = {"session_id": "s1", "tool_name": "Glob",