From 64939c99a8de96b70e6f3bf300109e38ba0cbbc0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 14:36:48 +0000 Subject: [PATCH 1/5] fix(bench): PRAGMAs are set in SQLiteStore.connect, not __init__ The first pilot session named connect and was scored as a miss; the key was wrong, the agent was right (sqlite_store.py:75-84 at the pinned sha). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01L7RD9DDX3xidUCfgBEtRDa --- benchmarks/agent_ab/tasks/cgis-control-sqlite-pragmas.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/benchmarks/agent_ab/tasks/cgis-control-sqlite-pragmas.yaml b/benchmarks/agent_ab/tasks/cgis-control-sqlite-pragmas.yaml index 71ee8eff..b8a9e4ca 100644 --- a/benchmarks/agent_ab/tasks/cgis-control-sqlite-pragmas.yaml +++ b/benchmarks/agent_ab/tasks/cgis-control-sqlite-pragmas.yaml @@ -8,13 +8,16 @@ question: | opens a database, and in which method? gold: symbols: - - SQLiteStore.__init__ + - SQLiteStore.connect files: - src/cgis/storage/sqlite_store.py facts: - WAL - "5000" + allowed_symbols: + - SQLiteStore notes: | Negative control: one file, one method, no cross-file structure. A graph should not help here; this measures what cgis costs when it is not needed. - sqlite_store.py:83-84. + sqlite_store.py:83-84, inside SQLiteStore.connect (line 75), not __init__: + the first pilot session caught a key that said __init__. From 0f9decc8c0fe3a8f7b3cb1dadd252eb643226bd0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 18:35:48 +0000 Subject: [PATCH 2/5] feat(bench): cgis-instructed arm and two multi-hop impact tasks The pilot's cgis arm made no cgis call in 12 of 12 sessions, and every task scored recall 1.0 in both arms. Add an arm that appends one system-prompt line telling the agent to query cgis first (a stand-in for #542's server instructions), and two impact tasks a single grep does not answer: a five-hop chain up to the entry points, and a chain whose first hop shares the target's method name. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01L7RD9DDX3xidUCfgBEtRDa --- benchmarks/agent_ab/README.md | 11 +++- .../tasks/cgis-impact-drift-same-name.yaml | 56 +++++++++++++++++++ .../cgis-impact-transitive-tsconfig.yaml | 45 +++++++++++++++ scripts/agent_ab.py | 25 +++++++-- tests/unit/test_agent_ab_script.py | 12 ++++ 5 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 benchmarks/agent_ab/tasks/cgis-impact-drift-same-name.yaml create mode 100644 benchmarks/agent_ab/tasks/cgis-impact-transitive-tsconfig.yaml diff --git a/benchmarks/agent_ab/README.md b/benchmarks/agent_ab/README.md index 046b7396..5050b8c3 100644 --- a/benchmarks/agent_ab/README.md +++ b/benchmarks/agent_ab/README.md @@ -40,8 +40,13 @@ resolver gets wrong. Each task's `notes` say how it was checked. |---|---|---|---|---| | `control` | none (`--strict-mcp-config`, empty config) | no | all deleted | no | | `cgis` | this checkout's `cgis-mcp` | yes (the plugin minus its `.mcp.json`) | `graph.db` built before the clock starts | no | +| `cgis-instructed` | as `cgis` | as `cgis` | as `cgis` | one line: query cgis first | -Both arms run under `cgis.bench.guard` as a PreToolUse hook, which refuses the +`cgis-instructed` exists because the first pilot's `cgis` arm made no cgis call in +12 of 12 sessions: with the server connected and the skill loaded, Sonnet still +went straight to Grep. It stands in for #542's MCP server instructions. + +All arms run under `cgis.bench.guard` as a PreToolUse hook, which refuses the cgis CLI, uv, sqlite3 and any read of `graph.db`/`graph.json` through Bash or the file tools. Without it the control arm is not a control: codegraph's own benchmark caught its control agent calling their CLI through Bash in 26 of 28 @@ -49,8 +54,8 @@ runs. The same predicate marks a finished run `contaminated` if a blocked call ever returned output; such runs are counted in the report and left out of the medians. -Both arms allow `Read`, `Grep`, `Glob` and `Bash` (plus `mcp__cgis` in the -treatment arm, which has no server in control) and refuse edits, web access and +All arms allow `Read`, `Grep`, `Glob` and `Bash` (plus `mcp__cgis` in the +treatment arms, which has no server in control) and refuse edits, web access and sub-agents, under `--permission-mode dontAsk`. Each session starts in a fresh detached worktree at the task's pinned commit, with `--no-session-persistence` and `--setting-sources project`, so user settings and earlier sessions do not diff --git a/benchmarks/agent_ab/tasks/cgis-impact-drift-same-name.yaml b/benchmarks/agent_ab/tasks/cgis-impact-drift-same-name.yaml new file mode 100644 index 00000000..fd8db4ba --- /dev/null +++ b/benchmarks/agent_ab/tasks/cgis-impact-drift-same-name.yaml @@ -0,0 +1,56 @@ +id: cgis-impact-drift-same-name +repo: cgis +sha: 715cd9a2ecee47949651344757edb4c7d09f64d7 +src_root: src +type: impact +question: | + In this repository, which code under `src/` can end up calling + `PatternCatalog.load_project_domains` in `src/cgis/query/drift/catalog.py`, directly + or transitively? Follow the chain up to the user-facing entry points (CLI commands + and MCP tools, and the Guardian review runner) and name every function or method + on the way. +gold: + symbols: + - DriftScorer.load_project_domains + - drift_service.analyze_drift + - GraphContextCollector.collect_drift + - ContextCollector.collect_all + - cli.drift + - mcp_server.cgis_drift + - id: guardian + any_of: [run_guardian, run_review_routed, GuardianReviewer.run_review, run_axis_review] + files: + - src/cgis/query/drift/drift.py + - src/cgis/query/drift/drift_service.py + - src/cgis/guardian/collector.py + - src/cgis/cli.py + - src/cgis/api/mcp_server.py + allowed_symbols: + - PatternCatalog + - PatternCatalog.load_project_domains + - DriftScorer + - GraphContextCollector + - ContextCollector + - GuardianReviewer + - run_guardian + - run_review_routed + - run_axis_review + - run_chunked_review + - chunked._single_pass + allowed_files: + - src/cgis/query/drift/catalog.py + - src/cgis/guardian/core.py + - src/cgis/guardian/axes.py + - src/cgis/guardian/chunked.py + - src/cgis/guardian/runner.py +notes: | + The first hop shares the target's name: DriftScorer.load_project_domains + (drift.py:58) wraps it, so a grep for the name finds both definitions and three + call sites that must be told apart. Read 2026-10-02 at the pinned sha: + DriftScorer.load_project_domains is called in analyze_drift (drift_service.py:162) + and GraphContextCollector.collect_drift (guardian/collector.py:249); analyze_drift + in cli.drift (cli.py:1213) and mcp_server.cgis_drift (mcp_server.py:636); + collect_drift in ContextCollector.collect_all (collector.py:353), which + GuardianReviewer.run_review (core.py:128) and run_axis_review (axes.py:68) call, + up through run_review_routed to run_guardian (runner.py:472). Any one Guardian + hop counts for the guardian item. diff --git a/benchmarks/agent_ab/tasks/cgis-impact-transitive-tsconfig.yaml b/benchmarks/agent_ab/tasks/cgis-impact-transitive-tsconfig.yaml new file mode 100644 index 00000000..34caefac --- /dev/null +++ b/benchmarks/agent_ab/tasks/cgis-impact-transitive-tsconfig.yaml @@ -0,0 +1,45 @@ +id: cgis-impact-transitive-tsconfig +repo: cgis +sha: 715cd9a2ecee47949651344757edb4c7d09f64d7 +src_root: src +type: impact +question: | + In this repository, which code under `src/` can end up executing the function + `_existing_json` in `src/cgis/tsconfig_paths.py`, directly or transitively? Follow + the chain all the way up to the user-facing entry points (CLI commands and MCP + tools) and name every function or method on the way. +gold: + symbols: + - tsconfig_paths._locate + - tsconfig_paths._options + - TsconfigPaths.aliases + - ImportNameCollector.collect + - IngestionPipeline.run + - cli.ingest + - mcp_server.cgis_ingest + - id: auto_refresh + any_of: [auto_refresh._refresh, auto_refresh.refresh_if_stale, auto_refresh.refreshes_graph] + files: + - src/cgis/tsconfig_paths.py + - src/cgis/import_names.py + - src/cgis/pipeline.py + - src/cgis/cli.py + - src/cgis/api/mcp_server.py + - src/cgis/api/auto_refresh.py + allowed_symbols: + - tsconfig_paths._existing_json + - TsconfigPaths + - ImportNameCollector + - IngestionPipeline + - auto_refresh + - api.mcp_server +notes: | + Multi-hop impact, five calls deep before the first entry point. Read 2026-10-02 at + the pinned sha: _existing_json is called only in _locate (tsconfig_paths.py:190, + :203); _locate only in _options (:153); _options in TsconfigPaths.aliases (:122) + and recursively in itself (:154); aliases only in ImportNameCollector.collect + (import_names.py:82); collect in IngestionPipeline.run (pipeline.py:193); run in + cli.ingest (cli.py:236, :243), mcp_server.cgis_ingest (mcp_server.py:354) and + auto_refresh._refresh (auto_refresh.py:82), which refresh_if_stale and the + refreshes_graph decorator on most MCP tools reach when CGIS_AUTO_REFRESH=1, so any + MCP tool name is allowed. The cgis graph agrees; the key was checked by grep. diff --git a/scripts/agent_ab.py b/scripts/agent_ab.py index 0a622002..7fa91be0 100644 --- a/scripts/agent_ab.py +++ b/scripts/agent_ab.py @@ -12,8 +12,12 @@ - `control`: no MCP servers; any committed or stray graph file is deleted. - `cgis`: the cgis MCP server from *this* checkout, the plugin's skills, and a graph built by this checkout's `cgis ingest` before the clock starts. +- `cgis-instructed`: `cgis` plus one appended system-prompt line telling the + agent to try cgis first. The pilot's `cgis` arm made no cgis call + in 12 of 12 sessions; this arm stands in for #542's server + instructions until they ship. -Both arms run under the same PreToolUse hook (`cgis.bench.guard`), the same +All arms run under the same PreToolUse hook (`cgis.bench.guard`), the same allowed tools (Read, Grep, Glob, Bash; no edits, no web, no sub-agents) and a PATH with no cgis or uv on it. Costs money: every non-dry run is a real session. Results append to `benchmarks/agent_ab/results.jsonl`, one line per @@ -44,8 +48,15 @@ from guardian_replay_skeptic import worktree_at -Arm = Literal["control", "cgis"] -ARMS: tuple[Arm, ...] = ("control", "cgis") +Arm = Literal["control", "cgis", "cgis-instructed"] +ARMS: tuple[Arm, ...] = ("control", "cgis", "cgis-instructed") + +#: Appended to the system prompt in the `cgis-instructed` arm only. +CGIS_INSTRUCTION = ( + "This repository has a code graph available through the cgis MCP tools " + "(mcp__cgis__*). For questions about callers, call chains, impact or code " + "structure, query cgis first, then read source only to confirm what it returns." +) _REPO_ROOT = Path(__file__).resolve().parent.parent _BENCH_DIR = _REPO_ROOT / "benchmarks" / "agent_ab" @@ -168,8 +179,10 @@ def build_command( ] if effort: cmd += ["--effort", effort] - if arm == "cgis": + if arm != "control": cmd += ["--plugin-dir", str(stage_plugin(config_dir / "plugin"))] + if arm == "cgis-instructed": + cmd += ["--append-system-prompt", CGIS_INSTRUCTION] return cmd @@ -272,7 +285,7 @@ def run_one(task: AgentTask, arm: Arm, run: int, repo: Path, args: argparse.Name """One session: worktree → (ingest) → claude -p → transcript → results line.""" with worktree_at(task.sha, repo) as wt, tempfile.TemporaryDirectory(prefix="ab-") as tmp: removed = remove_graph_files(wt) - ingest_s = ingest(wt, task.src_root) if arm == "cgis" else 0.0 + ingest_s = ingest(wt, task.src_root) if arm != "control" else 0.0 cmd = build_command( claude=args.claude, prompt=task.prompt(), @@ -390,7 +403,7 @@ def build_parser() -> argparse.ArgumentParser: run = sub.add_parser("run", help="run sessions and append results") run.add_argument("--tasks", type=Path, default=_BENCH_DIR / "tasks") run.add_argument("--task", action="append", default=[], help="task id; repeatable") - run.add_argument("--arm", action="append", choices=ARMS, help="repeatable; default both") + run.add_argument("--arm", action="append", choices=ARMS, help="repeatable; default all") run.add_argument("--runs", type=int, default=3) run.add_argument("--repo", action="append", default=[], help="NAME=PATH; repeatable") run.add_argument("--model", default=DEFAULT_MODEL) diff --git a/tests/unit/test_agent_ab_script.py b/tests/unit/test_agent_ab_script.py index f47de3d9..b8650dec 100644 --- a/tests/unit/test_agent_ab_script.py +++ b/tests/unit/test_agent_ab_script.py @@ -111,6 +111,18 @@ def test_cgis_command_uses_this_checkouts_server_and_the_plugin_without_mcp_json assert cmd[cmd.index("--effort") + 1] == "high" +def test_instructed_arm_is_the_cgis_arm_plus_one_system_prompt_line(tmp_path: Path) -> None: + (tmp_path / "a").mkdir() + (tmp_path / "b").mkdir() + plain = _command(tmp_path / "a", "cgis") + instructed = _command(tmp_path / "b", "cgis-instructed") + assert "--append-system-prompt" not in plain + assert instructed[instructed.index("--append-system-prompt") + 1] == ab.CGIS_INSTRUCTION + assert "--plugin-dir" in instructed + server = json.loads((tmp_path / "b" / "mcp.json").read_text())["mcpServers"]["cgis"] + assert server["command"].endswith("cgis-mcp") + + def test_repo_paths_parses_name_path_pairs(tmp_path: Path) -> None: repos = ab.repo_paths([f"owner-api={tmp_path}"]) assert repos["owner-api"] == tmp_path.resolve() From 666e44838b86f482e3b6e182b63f3706b0cf2a04 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 19:02:32 +0000 Subject: [PATCH 3/5] feat(bench): cgis-forced arm gates source tools behind one cgis call The cgis-instructed arm called cgis in 2 of 18 sessions. The new arm runs the guard with --cgis-first MARKER: Read, Grep, Glob and Bash are refused until an mcp__cgis__ call creates the marker, so the benchmark can measure what the graph adds once used, apart from whether the agent picks it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01L7RD9DDX3xidUCfgBEtRDa --- benchmarks/agent_ab/README.md | 5 +++- scripts/agent_ab.py | 22 ++++++++++----- src/cgis/bench/guard.py | 43 ++++++++++++++++++++++++++---- tests/unit/test_agent_ab_guard.py | 22 ++++++++++++++- tests/unit/test_agent_ab_script.py | 8 ++++++ 5 files changed, 87 insertions(+), 13 deletions(-) diff --git a/benchmarks/agent_ab/README.md b/benchmarks/agent_ab/README.md index 5050b8c3..152c475d 100644 --- a/benchmarks/agent_ab/README.md +++ b/benchmarks/agent_ab/README.md @@ -41,10 +41,13 @@ resolver gets wrong. Each task's `notes` say how it was checked. | `control` | none (`--strict-mcp-config`, empty config) | no | all deleted | no | | `cgis` | this checkout's `cgis-mcp` | yes (the plugin minus its `.mcp.json`) | `graph.db` built before the clock starts | no | | `cgis-instructed` | as `cgis` | as `cgis` | as `cgis` | one line: query cgis first | +| `cgis-forced` | as `cgis` | as `cgis` | as `cgis` | as `cgis-instructed`, and the guard refuses Read/Grep/Glob/Bash until one cgis call | `cgis-instructed` exists because the first pilot's `cgis` arm made no cgis call in 12 of 12 sessions: with the server connected and the skill loaded, Sonnet still -went straight to Grep. It stands in for #542's MCP server instructions. +went straight to Grep. It stands in for #542's MCP server instructions. It barely moved the agent (2 cgis +calls in 18 sessions), so `cgis-forced` makes the first graph query mandatory: it +measures what the graph adds once used, apart from whether the agent picks it. All arms run under `cgis.bench.guard` as a PreToolUse hook, which refuses the cgis CLI, uv, sqlite3 and any read of `graph.db`/`graph.json` through Bash or diff --git a/scripts/agent_ab.py b/scripts/agent_ab.py index 7fa91be0..bc73da1b 100644 --- a/scripts/agent_ab.py +++ b/scripts/agent_ab.py @@ -16,6 +16,9 @@ agent to try cgis first. The pilot's `cgis` arm made no cgis call in 12 of 12 sessions; this arm stands in for #542's server instructions until they ship. +- `cgis-forced`: `cgis-instructed`, and the guard refuses Read, Grep, Glob and + Bash until the session has made one cgis call. Measures what the + graph adds once used, separately from whether the agent picks it. All arms run under the same PreToolUse hook (`cgis.bench.guard`), the same allowed tools (Read, Grep, Glob, Bash; no edits, no web, no sub-agents) and a @@ -48,8 +51,9 @@ from guardian_replay_skeptic import worktree_at -Arm = Literal["control", "cgis", "cgis-instructed"] -ARMS: tuple[Arm, ...] = ("control", "cgis", "cgis-instructed") +Arm = Literal["control", "cgis", "cgis-instructed", "cgis-forced"] +ARMS: tuple[Arm, ...] = ("control", "cgis", "cgis-instructed", "cgis-forced") +_INSTRUCTED: tuple[Arm, ...] = ("cgis-instructed", "cgis-forced") #: Appended to the system prompt in the `cgis-instructed` arm only. CGIS_INSTRUCTION = ( @@ -116,9 +120,14 @@ def mcp_config(arm: Arm) -> dict[str, object]: return {"mcpServers": {"cgis": {"command": bin_path("cgis-mcp"), "args": []}}} -def hook_settings() -> dict[str, object]: - """The `--settings` document installing the guard hook on every tool call.""" +def hook_settings(cgis_first: Path | None = None) -> dict[str, object]: + """The `--settings` document installing the guard hook on every tool call. + + `cgis_first` is the marker file that switches the guard to `--cgis-first`. + """ command = f"{shlex.quote(sys.executable)} -m cgis.bench.guard" + if cgis_first is not None: + command += f" --cgis-first {shlex.quote(str(cgis_first))}" return { "hooks": { "PreToolUse": [{"matcher": ".*", "hooks": [{"type": "command", "command": command}]}] @@ -150,7 +159,8 @@ def build_command( mcp_path = config_dir / "mcp.json" settings_path = config_dir / "settings.json" mcp_path.write_text(json.dumps(mcp_config(arm)), encoding="utf-8") - settings_path.write_text(json.dumps(hook_settings()), encoding="utf-8") + cgis_first = config_dir / "cgis_used" if arm == "cgis-forced" else None + settings_path.write_text(json.dumps(hook_settings(cgis_first)), encoding="utf-8") cmd = [ claude, "-p", @@ -181,7 +191,7 @@ def build_command( cmd += ["--effort", effort] if arm != "control": cmd += ["--plugin-dir", str(stage_plugin(config_dir / "plugin"))] - if arm == "cgis-instructed": + if arm in _INSTRUCTED: cmd += ["--append-system-prompt", CGIS_INSTRUCTION] return cmd diff --git a/src/cgis/bench/guard.py b/src/cgis/bench/guard.py index 94753998..070a7e3f 100644 --- a/src/cgis/bench/guard.py +++ b/src/cgis/bench/guard.py @@ -9,12 +9,18 @@ JSON on stdin, and exit code 2 refuses it with stderr shown to the agent. The same predicate is applied to finished transcripts to flag contaminated runs, so "blocked" and "counted as contamination" cannot drift apart. + +With `--cgis-first MARKER` (the `cgis-forced` arm) the hook also refuses source +access until the session has made one cgis MCP call, recorded by creating MARKER. +That arm measures what the graph adds once it is used, not whether the agent +chooses to use it. """ import json import re import sys -from collections.abc import Mapping +from collections.abc import Mapping, Sequence +from pathlib import Path #: A cgis-adjacent executable in command position: start of the command, or after #: a separator, a subshell or a pipe, optionally with a directory in front. @@ -26,6 +32,14 @@ _PATH_KEYS = ("file_path", "path", "pattern", "notebook_path") +#: Tools that reach source without the graph; held back in `--cgis-first` mode. +_SOURCE_TOOLS = frozenset({"Read", "Grep", "Glob", "Bash"}) +_CGIS_PREFIX = "mcp__cgis__" +CGIS_FIRST_REASON = ( + "Query the code graph first: call one of the mcp__cgis__ tools before reading " + "or searching source." +) + def blocked_reason(tool_name: str, tool_input: Mapping[str, object]) -> str | None: """Why this tool call must be refused, or None when it may run.""" @@ -42,18 +56,37 @@ def blocked_reason(tool_name: str, tool_input: Mapping[str, object]) -> str | No return None -def main() -> int: +def cgis_first_reason(tool_name: str, marker: Path) -> str | None: + """Gate source tools behind one cgis call; a cgis call creates `marker`.""" + if tool_name.startswith(_CGIS_PREFIX): + marker.touch() + return None + if tool_name in _SOURCE_TOOLS and not marker.exists(): + return CGIS_FIRST_REASON + return None + + +def _marker(argv: Sequence[str]) -> Path | None: + """The `--cgis-first MARKER` path, when given.""" + if len(argv) >= 2 and argv[0] == "--cgis-first": + return Path(argv[1]) + return None + + +def main(argv: Sequence[str] | None = None) -> int: """Hook entry point: exit 2 with a reason to refuse the call, 0 to allow it.""" + marker = _marker(sys.argv[1:] if argv is None else argv) try: event = json.load(sys.stdin) except json.JSONDecodeError: return 0 if not isinstance(event, dict): return 0 + tool_name = str(event.get("tool_name", "")) tool_input = event.get("tool_input") - reason = blocked_reason( - str(event.get("tool_name", "")), tool_input if isinstance(tool_input, dict) else {} - ) + reason = blocked_reason(tool_name, tool_input if isinstance(tool_input, dict) else {}) + if reason is None and marker is not None: + reason = cgis_first_reason(tool_name, marker) if reason is None: return 0 print(reason, file=sys.stderr) diff --git a/tests/unit/test_agent_ab_guard.py b/tests/unit/test_agent_ab_guard.py index aa6c5e0a..ed1eb1eb 100644 --- a/tests/unit/test_agent_ab_guard.py +++ b/tests/unit/test_agent_ab_guard.py @@ -2,6 +2,7 @@ import io import json +from pathlib import Path import pytest @@ -66,7 +67,7 @@ def test_file_tools_on_source_are_allowed() -> None: def _run_main(monkeypatch: pytest.MonkeyPatch, stdin: str) -> int: monkeypatch.setattr("sys.stdin", io.StringIO(stdin)) - return guard.main() + return guard.main([]) def test_main_refuses_with_exit_2_and_a_reason( @@ -92,3 +93,22 @@ def test_main_lets_malformed_events_through(monkeypatch: pytest.MonkeyPatch, std def test_other_files_ending_in_graph_names_are_allowed(path: str) -> None: assert blocked_reason("Read", {"file_path": path}) is None assert blocked_reason("Bash", {"command": f"cat {path}"}) is None + + +def test_cgis_first_holds_source_tools_until_a_cgis_call(tmp_path: Path) -> None: + marker = tmp_path / "cgis_used" + assert guard.cgis_first_reason("Grep", marker) == guard.CGIS_FIRST_REASON + assert guard.cgis_first_reason("ToolSearch", marker) is None + assert guard.cgis_first_reason("mcp__cgis__cgis_analyze_impact", marker) is None + assert marker.exists() + assert guard.cgis_first_reason("Read", marker) is None + + +def test_main_applies_cgis_first_only_when_asked( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + event = json.dumps({"tool_name": "Read", "tool_input": {"file_path": "/w/src/a.py"}}) + monkeypatch.setattr("sys.stdin", io.StringIO(event)) + assert guard.main([]) == 0 + monkeypatch.setattr("sys.stdin", io.StringIO(event)) + assert guard.main(["--cgis-first", str(tmp_path / "m")]) == 2 diff --git a/tests/unit/test_agent_ab_script.py b/tests/unit/test_agent_ab_script.py index b8650dec..7c1bd294 100644 --- a/tests/unit/test_agent_ab_script.py +++ b/tests/unit/test_agent_ab_script.py @@ -123,6 +123,14 @@ def test_instructed_arm_is_the_cgis_arm_plus_one_system_prompt_line(tmp_path: Pa assert server["command"].endswith("cgis-mcp") +def test_forced_arm_runs_the_guard_in_cgis_first_mode(tmp_path: Path) -> None: + cmd = _command(tmp_path, "cgis-forced") + hook = json.loads((tmp_path / "settings.json").read_text())["hooks"]["PreToolUse"][0] + assert hook["hooks"][0]["command"].endswith(f"--cgis-first {tmp_path / 'cgis_used'}") + assert cmd[cmd.index("--append-system-prompt") + 1] == ab.CGIS_INSTRUCTION + assert "--plugin-dir" in cmd + + def test_repo_paths_parses_name_path_pairs(tmp_path: Path) -> None: repos = ab.repo_paths([f"owner-api={tmp_path}"]) assert repos["owner-api"] == tmp_path.resolve() From 2efeb7294c5261f9d2af9fe0bfe5d0e733e6cf34 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 19:18:12 +0000 Subject: [PATCH 4/5] feat(bench): two owner-api impact tasks A transitive chain from finance.service.credit_wallet to its HTTP routes and worker tasks, through a same-named method and an injected service, and a direct-caller question where another module defines a function with the same name. Keys were read at the pinned sha with an AST call index, not with cgis; the graph misses both routes of the first. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01L7RD9DDX3xidUCfgBEtRDa --- .../tasks/owner-impact-credit-wallet.yaml | 55 +++++++++++++++++++ ...wner-impact-occupied-window-same-name.yaml | 30 ++++++++++ 2 files changed, 85 insertions(+) create mode 100644 benchmarks/agent_ab/tasks/owner-impact-credit-wallet.yaml create mode 100644 benchmarks/agent_ab/tasks/owner-impact-occupied-window-same-name.yaml diff --git a/benchmarks/agent_ab/tasks/owner-impact-credit-wallet.yaml b/benchmarks/agent_ab/tasks/owner-impact-credit-wallet.yaml new file mode 100644 index 00000000..0e305729 --- /dev/null +++ b/benchmarks/agent_ab/tasks/owner-impact-credit-wallet.yaml @@ -0,0 +1,55 @@ +id: owner-impact-credit-wallet +repo: owner-api +sha: 8de22452b5d91242dd742a666ef2cd7b1eb7c1ff +src_root: ownima-backend/app +type: impact +question: | + In this repository, the module-level function `credit_wallet` in + `ownima-backend/app/domains/finance/service.py` is about to change. Which code under + `ownima-backend/app/` (not tests, not scripts) can end up calling it, directly or + transitively? Follow every chain up to its entry point (an HTTP route, a worker task, + or a function nothing else calls) and name every function or method on the way. +gold: + symbols: + - FinanceService.credit_wallet + - FinanceService.top_up + - TopUpService._credit + - TopUpService.fulfill + - TopUpService.get_status + - TopUpService.handle_webhook_event + - finance.routes.get_top_up + - finance.routes.stripe_webhook + - VehicleBonusCreditor._pay + - VehicleBonusCreditor.credit + - VehicleBonusWorker.credit + - vehicle_bonus.reconcile_vehicle_bonus + - VehicleBonusWorker.reconcile + files: + - ownima-backend/app/domains/finance/service.py + - ownima-backend/app/domains/finance/top_up_service.py + - ownima-backend/app/domains/finance/routes.py + - ownima-backend/app/domains/finance/vehicle_bonus_credit.py + - ownima-backend/app/worker/vehicle_bonus.py + allowed_symbols: + - finance.service.credit_wallet + - FinanceService + - TopUpService + - VehicleBonusCreditor + - VehicleBonusWorker + - worker.base + allowed_files: + - ownima-backend/app/worker/base.py +notes: | + Same-name trap plus DI: the module function is wrapped by the method + FinanceService.credit_wallet (service.py:332-344), and the top-up path reaches the + method through an injected `self._finance` (top_up_service.py:316). Read 2026-10-02 + at the pinned sha with an AST call index, not with cgis: credit_wallet is called in + FinanceService.credit_wallet (service.py:344) and VehicleBonusCreditor._pay + (vehicle_bonus_credit.py:202); the method in FinanceService.top_up (:367, no callers + of its own) and TopUpService._credit (top_up_service.py:316); _credit in fulfill + (:266); fulfill in get_status (:294) and handle_webhook_event (:305); those in the + routes get_top_up (routes.py:232) and stripe_webhook (:344). _pay in + VehicleBonusCreditor.credit (:142); that in VehicleBonusWorker.credit + (worker/vehicle_bonus.py:151) and reconcile_vehicle_bonus (:106); the latter in + VehicleBonusWorker.reconcile (:177). Both worker methods are registered as tasks in + worker/base.py:198-199. The cgis graph at this sha misses both routes. diff --git a/benchmarks/agent_ab/tasks/owner-impact-occupied-window-same-name.yaml b/benchmarks/agent_ab/tasks/owner-impact-occupied-window-same-name.yaml new file mode 100644 index 00000000..63cd701d --- /dev/null +++ b/benchmarks/agent_ab/tasks/owner-impact-occupied-window-same-name.yaml @@ -0,0 +1,30 @@ +id: owner-impact-occupied-window-same-name +repo: owner-api +sha: 8de22452b5d91242dd742a666ef2cd7b1eb7c1ff +src_root: ownima-backend/app +type: impact +question: | + In this repository, the function `occupied_window` in + `ownima-backend/app/services/reservation_conflict_service.py` is about to change its + signature. List every function or method under `ownima-backend/app/` (not tests, + not scripts) that calls it directly, so each call site can be updated. +gold: + symbols: + - ReservationConflictService._check_conflicts_internal + - ReservationConflictService._find_conflicting_groups + - ReservationConflictService._clusters + - ReservationConflictService.get_pending_conflicts_for_reservation + files: + - ownima-backend/app/services/reservation_conflict_service.py + allowed_symbols: + - reservation_conflict_service.occupied_window + - ReservationConflictService +notes: | + Same-name trap: domains/pricing/handover.py:230 defines another module-level + `occupied_window`, and HandoverSchedule.window_for (handover.py:195), + ReservationCreation.compute_actual_datetime_range (creation.py:716) and Rule.build + (validators.py:164) call that one (creation.py:21-28 and validators.py:18-25 import it + from app.domains.pricing.handover). Naming them costs precision. Read 2026-10-02 at + the pinned sha with an AST call index: the services function (line 836) is called at + reservation_conflict_service.py:241, :552, :661, :678, :736 and :753, inside the four + methods above. From 0e48c4dd28c2ff04a042cae2f7223ba8b11e4455 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 19:20:34 +0000 Subject: [PATCH 5/5] fix(bench): keep the cgis-first marker at a fixed path, quote for cmd.exe Sonar S8707 flagged the hook writing to a path taken from its argv. The guard now takes a bare --cgis-first flag and keeps its marker in its working directory, the run's fresh worktree, so a later run never sees an earlier run's marker and nothing the session passes picks the path. The hook command uses list2cmdline quoting on Windows, where POSIX single quotes mean nothing. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01L7RD9DDX3xidUCfgBEtRDa --- scripts/agent_ab.py | 24 +++++++++++++++--------- src/cgis/bench/guard.py | 20 ++++++++++---------- tests/unit/test_agent_ab_guard.py | 24 ++++++++++++++++++++---- tests/unit/test_agent_ab_script.py | 11 ++++++++++- 4 files changed, 55 insertions(+), 24 deletions(-) diff --git a/scripts/agent_ab.py b/scripts/agent_ab.py index bc73da1b..9fbcdda9 100644 --- a/scripts/agent_ab.py +++ b/scripts/agent_ab.py @@ -16,8 +16,8 @@ agent to try cgis first. The pilot's `cgis` arm made no cgis call in 12 of 12 sessions; this arm stands in for #542's server instructions until they ship. -- `cgis-forced`: `cgis-instructed`, and the guard refuses Read, Grep, Glob and - Bash until the session has made one cgis call. Measures what the +- `cgis-forced`: `cgis-instructed`, and the guard (`--cgis-first`) refuses + Read, Grep, Glob and Bash until the session has made one cgis call. Measures what the graph adds once used, separately from whether the agent picks it. All arms run under the same PreToolUse hook (`cgis.bench.guard`), the same @@ -45,6 +45,7 @@ from typing import Literal from cgis.bench.agent_task import AgentTask, extract_answer, load_tasks, score_answer +from cgis.bench.guard import CGIS_FIRST_FLAG from cgis.bench.transcript import parse_transcript, run_metrics sys.path.insert(0, str(Path(__file__).resolve().parent)) @@ -120,14 +121,19 @@ def mcp_config(arm: Arm) -> dict[str, object]: return {"mcpServers": {"cgis": {"command": bin_path("cgis-mcp"), "args": []}}} -def hook_settings(cgis_first: Path | None = None) -> dict[str, object]: +def _quote(arg: str) -> str: + """One shell word: cmd.exe quoting on Windows, POSIX quoting elsewhere.""" + return subprocess.list2cmdline([arg]) if sys.platform == "win32" else shlex.quote(arg) + + +def hook_settings(*, cgis_first: bool = False) -> dict[str, object]: """The `--settings` document installing the guard hook on every tool call. - `cgis_first` is the marker file that switches the guard to `--cgis-first`. + `cgis_first` runs the guard with `--cgis-first` (the `cgis-forced` arm). """ - command = f"{shlex.quote(sys.executable)} -m cgis.bench.guard" - if cgis_first is not None: - command += f" --cgis-first {shlex.quote(str(cgis_first))}" + command = f"{_quote(sys.executable)} -m cgis.bench.guard" + if cgis_first: + command += f" {CGIS_FIRST_FLAG}" return { "hooks": { "PreToolUse": [{"matcher": ".*", "hooks": [{"type": "command", "command": command}]}] @@ -159,8 +165,8 @@ def build_command( mcp_path = config_dir / "mcp.json" settings_path = config_dir / "settings.json" mcp_path.write_text(json.dumps(mcp_config(arm)), encoding="utf-8") - cgis_first = config_dir / "cgis_used" if arm == "cgis-forced" else None - settings_path.write_text(json.dumps(hook_settings(cgis_first)), encoding="utf-8") + settings = hook_settings(cgis_first=arm == "cgis-forced") + settings_path.write_text(json.dumps(settings), encoding="utf-8") cmd = [ claude, "-p", diff --git a/src/cgis/bench/guard.py b/src/cgis/bench/guard.py index 070a7e3f..84e8036d 100644 --- a/src/cgis/bench/guard.py +++ b/src/cgis/bench/guard.py @@ -10,8 +10,11 @@ same predicate is applied to finished transcripts to flag contaminated runs, so "blocked" and "counted as contamination" cannot drift apart. -With `--cgis-first MARKER` (the `cgis-forced` arm) the hook also refuses source -access until the session has made one cgis MCP call, recorded by creating MARKER. +With `--cgis-first` (the `cgis-forced` arm) the hook also refuses source access +until the session has made one cgis MCP call, recorded as a marker file in the +hook's working directory: the run's fresh worktree, deleted with it. The path is +fixed rather than passed in, so nothing the session controls picks what the +hook writes. That arm measures what the graph adds once it is used, not whether the agent chooses to use it. """ @@ -35,6 +38,9 @@ #: Tools that reach source without the graph; held back in `--cgis-first` mode. _SOURCE_TOOLS = frozenset({"Read", "Grep", "Glob", "Bash"}) _CGIS_PREFIX = "mcp__cgis__" +CGIS_FIRST_FLAG = "--cgis-first" +#: Created in the hook's working directory by the session's first cgis call. +MARKER_NAME = ".cgis-bench-used" CGIS_FIRST_REASON = ( "Query the code graph first: call one of the mcp__cgis__ tools before reading " "or searching source." @@ -66,16 +72,10 @@ def cgis_first_reason(tool_name: str, marker: Path) -> str | None: return None -def _marker(argv: Sequence[str]) -> Path | None: - """The `--cgis-first MARKER` path, when given.""" - if len(argv) >= 2 and argv[0] == "--cgis-first": - return Path(argv[1]) - return None - - def main(argv: Sequence[str] | None = None) -> int: """Hook entry point: exit 2 with a reason to refuse the call, 0 to allow it.""" - marker = _marker(sys.argv[1:] if argv is None else argv) + args = sys.argv[1:] if argv is None else argv + marker = Path.cwd() / MARKER_NAME if CGIS_FIRST_FLAG in args else None try: event = json.load(sys.stdin) except json.JSONDecodeError: diff --git a/tests/unit/test_agent_ab_guard.py b/tests/unit/test_agent_ab_guard.py index ed1eb1eb..bd84d9ba 100644 --- a/tests/unit/test_agent_ab_guard.py +++ b/tests/unit/test_agent_ab_guard.py @@ -104,11 +104,27 @@ def test_cgis_first_holds_source_tools_until_a_cgis_call(tmp_path: Path) -> None assert guard.cgis_first_reason("Read", marker) is None +def _event(tool_name: str) -> str: + return json.dumps({"tool_name": tool_name, "tool_input": {"file_path": "/w/src/a.py"}}) + + def test_main_applies_cgis_first_only_when_asked( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: - event = json.dumps({"tool_name": "Read", "tool_input": {"file_path": "/w/src/a.py"}}) - monkeypatch.setattr("sys.stdin", io.StringIO(event)) + monkeypatch.chdir(tmp_path) + monkeypatch.setattr("sys.stdin", io.StringIO(_event("Read"))) assert guard.main([]) == 0 - monkeypatch.setattr("sys.stdin", io.StringIO(event)) - assert guard.main(["--cgis-first", str(tmp_path / "m")]) == 2 + monkeypatch.setattr("sys.stdin", io.StringIO(_event("Read"))) + assert guard.main([guard.CGIS_FIRST_FLAG]) == 2 + + +def test_the_cgis_first_marker_lives_in_the_working_directory( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The run's worktree, so a later run never inherits an earlier run's marker.""" + monkeypatch.chdir(tmp_path) + monkeypatch.setattr("sys.stdin", io.StringIO(_event("mcp__cgis__cgis_context"))) + assert guard.main([guard.CGIS_FIRST_FLAG]) == 0 + assert (tmp_path / guard.MARKER_NAME).exists() + monkeypatch.setattr("sys.stdin", io.StringIO(_event("Read"))) + assert guard.main([guard.CGIS_FIRST_FLAG]) == 0 diff --git a/tests/unit/test_agent_ab_script.py b/tests/unit/test_agent_ab_script.py index 7c1bd294..2343e940 100644 --- a/tests/unit/test_agent_ab_script.py +++ b/tests/unit/test_agent_ab_script.py @@ -126,7 +126,7 @@ def test_instructed_arm_is_the_cgis_arm_plus_one_system_prompt_line(tmp_path: Pa def test_forced_arm_runs_the_guard_in_cgis_first_mode(tmp_path: Path) -> None: cmd = _command(tmp_path, "cgis-forced") hook = json.loads((tmp_path / "settings.json").read_text())["hooks"]["PreToolUse"][0] - assert hook["hooks"][0]["command"].endswith(f"--cgis-first {tmp_path / 'cgis_used'}") + assert hook["hooks"][0]["command"].endswith(" -m cgis.bench.guard --cgis-first") assert cmd[cmd.index("--append-system-prompt") + 1] == ab.CGIS_INSTRUCTION assert "--plugin-dir" in cmd @@ -300,6 +300,15 @@ def timeout(cmd: list[str], **_k: object) -> None: def test_the_hook_quotes_an_interpreter_path_with_spaces( monkeypatch: pytest.MonkeyPatch, ) -> None: + monkeypatch.setattr(ab.sys, "platform", "linux") monkeypatch.setattr(ab.sys, "executable", "/opt/my env/bin/python") hook = ab.hook_settings()["hooks"]["PreToolUse"][0]["hooks"][0] # type: ignore[index] assert hook["command"] == "'/opt/my env/bin/python' -m cgis.bench.guard" + + +def test_the_hook_uses_cmd_quoting_on_windows(monkeypatch: pytest.MonkeyPatch) -> None: + """POSIX single quotes mean nothing to cmd.exe.""" + monkeypatch.setattr(ab.sys, "platform", "win32") + monkeypatch.setattr(ab.sys, "executable", r"C:\Program Files\Python\python.exe") + hook = ab.hook_settings()["hooks"]["PreToolUse"][0]["hooks"][0] # type: ignore[index] + assert hook["command"] == r'"C:\Program Files\Python\python.exe" -m cgis.bench.guard'