From d3c01ce8ed7c9707bbec34099fca2fc6508833eb Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Tue, 4 Aug 2026 15:30:28 -0500 Subject: [PATCH] test(agent-eval): stop the arms reaching codegraph through the shell (CG-7) The without-arm had no MCP server but still had Bash, and the target repo carries the .codegraph/ index the with-arm needs. Agents found it: 14 of 15 without-arm runs in a 7-repo pass ran `codegraph explore` through Bash, one of them via `ls .codegraph && codegraph explore ...`. That arm was measuring codegraph-over-CLI, not codegraph-absent, so every without-arm number it produced was wrong. It bit the with-arm too -- output arriving through Bash is attributed to Bash, understating what codegraph itself occupies (1 of 15 runs). Both arms now run on a PATH where the CLI is hidden, so the MCP server is the only way to reach codegraph and stays the single variable. The binary shares a directory with tools the run needs -- claude itself sits next to it -- so the directory is substituted in place by one of symlinks to every entry except codegraph, preserving PATH order and precedence. The run aborts if claude or node did not survive the substitution. Prevention alone would fail silently the next time the CLI lands somewhere new, so parse-run.mjs flags any Bash command naming codegraph and parse-bench-readme drops contaminated without-arm runs from the aggregate (CG_INCLUDE_CONTAMINATED=1 keeps them). CG_ARMS re-runs one arm without redoing the other. --- scripts/agent-eval/parse-bench-readme.mjs | 13 +++++- scripts/agent-eval/parse-run.mjs | 13 ++++-- scripts/agent-eval/run-all.sh | 48 +++++++++++++++++++++-- 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/scripts/agent-eval/parse-bench-readme.mjs b/scripts/agent-eval/parse-bench-readme.mjs index 4c438d0..7b11e02 100644 --- a/scripts/agent-eval/parse-bench-readme.mjs +++ b/scripts/agent-eval/parse-bench-readme.mjs @@ -42,7 +42,7 @@ function parse(dir, label) { const o = s.occupancy; return { dur: s.dur, tools: s.tools, reads: s.reads, grep: s.grep, cg: s.cg, - bash: s.counts.Bash || 0, + bash: s.counts.Bash || 0, cliCalls: s.cliCalls, tokens: s.processed, cost: s.cost, raced: s.raced, turns: s.turns, segments: files.length, ctx: o.ctxFinal, @@ -67,7 +67,11 @@ const pct = (w, wo) => wo > 0 ? Math.round((1 - w / wo) * 100) : 0; // race, not steady-state value. `CG_INCLUDE_RACED=1` keeps them (to see the raw // distribution). The WITHOUT arm has no MCP, so it's never raced. const includeRaced = process.env.CG_INCLUDE_RACED === '1'; +// A without-arm run that shelled out to the codegraph CLI measured +// codegraph-over-CLI, not codegraph-absent. Drop it unless asked otherwise. +const includeContaminated = process.env.CG_INCLUDE_CONTAMINATED === '1'; const rows = []; +let contaminated = 0; for (const repo of REPOS) { const dir = join(ROOT, repo); const runDirs = existsSync(dir) ? readdirSync(dir).filter(d => /^run\d+$/.test(d)).sort() : []; @@ -75,10 +79,15 @@ for (const repo of REPOS) { for (const rd of runDirs) { const w = parse(join(dir, rd), 'headless-with'); if (w) { if (w.raced && !includeRaced) racedExcluded++; else W.push(w); } - const wo = parse(join(dir, rd), 'headless-without'); if (wo) WO.push(wo); + const wo = parse(join(dir, rd), 'headless-without'); + if (wo) { + if (wo.cliCalls && !includeContaminated) { contaminated++; console.error(`[excluded] ${repo}/${rd} without-arm ran the codegraph CLI ${wo.cliCalls}x`); } + else WO.push(wo); + } } rows.push({ repo, W, WO, racedExcluded }); } +if (contaminated) console.error(`[excluded] ${contaminated} contaminated without-arm run(s); CG_INCLUDE_CONTAMINATED=1 keeps them\n`); // ---- Table 1: the existing throughput view. -------------------------------- console.log('repo n(w/wo) time WITH→WITHOUT tools W→WO tokens W→WO (saved) cost W→WO (saved)'); diff --git a/scripts/agent-eval/parse-run.mjs b/scripts/agent-eval/parse-run.mjs index cf53dff..ea69b76 100644 --- a/scripts/agent-eval/parse-run.mjs +++ b/scripts/agent-eval/parse-run.mjs @@ -90,7 +90,7 @@ export function parseSession(files) { const toolCalls = []; // display sequence const nameById = new Map(); // tool_use_id -> tool name const counts = {}; // tool name -> calls - let initTools = null, result = null, raced = false; + let initTools = null, result = null, raced = false, cliCalls = 0; const results = []; // one `result` event per session segment (multi-turn) let compactions = 0; @@ -126,7 +126,13 @@ export function parseSession(files) { let detail = ''; if (b.name === 'Task') detail = ` [subagent_type=${b.input?.subagent_type ?? '?'}] ${(b.input?.description ?? '').slice(0, 40)}`; else if (/codegraph/.test(b.name)) detail = ` ${JSON.stringify(b.input?.query ?? b.input?.task ?? b.input?.symbol ?? '').slice(0, 60)}`; - else if (b.name === 'Bash') detail = ` ${(b.input?.command ?? '').slice(0, 50)}`; + else if (b.name === 'Bash') { + detail = ` ${(b.input?.command ?? '').slice(0, 50)}`; + // An arm with no codegraph MCP can still shell out to the CLI — the + // target repo carries the .codegraph/ index and the binary is on + // PATH. That silently turns a "without" arm into codegraph-over-CLI. + if (/\bcodegraph\b/.test(b.input?.command ?? '')) cliCalls++; + } else if (b.name === 'Read') detail = ` ${(b.input?.file_path ?? '').split('/').slice(-1)[0]}`; toolCalls.push(`${b.name}${detail}`); } @@ -268,7 +274,7 @@ export function parseSession(files) { + sumUsage('cache_creation_input_tokens') + sumUsage('output_tokens'); return { - files, toolCalls, counts, initTools, result, results, raced, + files, toolCalls, counts, initTools, result, results, raced, cliCalls, ok: results.length > 0 && results.every((r) => r.subtype === 'success'), turns: reqIdx.length, tools: toolCalls.filter((t) => !t.startsWith('ToolSearch')).length, @@ -450,6 +456,7 @@ if (isMain) { console.log(`\n=== ${files.map((f) => f.split('/').pop()).join(' + ')} ===`); console.log(`codegraph tools exposed: ${s.initTools ? s.initTools.length : '?'}${s.raced ? ' [MCP COLD-START RACE — tool call hit "No such tool available"]' : ''}`); + if (s.cliCalls) console.log(`!! ${s.cliCalls} Bash call${s.cliCalls === 1 ? '' : 's'} invoked the codegraph CLI — if this is a without-arm, the run is CONTAMINATED`); console.log(`\nTool calls (${s.toolCalls.length}):`); console.log(' by type:', JSON.stringify(s.counts)); s.toolCalls.forEach((tc, i) => console.log(` ${i + 1}. ${tc}`)); diff --git a/scripts/agent-eval/run-all.sh b/scripts/agent-eval/run-all.sh index 551cfb6..349818b 100755 --- a/scripts/agent-eval/run-all.sh +++ b/scripts/agent-eval/run-all.sh @@ -43,6 +43,46 @@ mkdir -p "$OUT" # The A/B's only variable must be the MCP server wired below. export CODEGRAPH_NO_PROMPT_HOOK=1 +# Hide the codegraph CLI from BOTH arms, so the only way to reach codegraph is +# the MCP server wired below — which is what makes it the A/B's single variable. +# +# Both arms have Bash, and the target repo carries the .codegraph/ index the +# with-arm needs. Agents FIND that: 14 of 15 without-arm runs in one 7-repo pass +# ran `codegraph explore` through Bash (one via `ls .codegraph && codegraph +# explore …`), so that arm was measuring codegraph-over-CLI, not +# codegraph-absent. It matters in the with-arm too — output that arrives through +# Bash is attributed to Bash, understating what codegraph itself occupies. +# +# The binary usually shares a directory with tools the run needs (claude itself +# lives next to it here), so dropping the whole directory is not an option. +# Substitute an equivalent directory IN PLACE: symlinks to every entry except +# codegraph, keeping PATH order and precedence intact. +SHIM_BIN="$OUT/nocg-bin" +rm -rf "$SHIM_BIN"; mkdir -p "$SHIM_BIN" +sanitized_path() { + local out="" d e + local IFS=: + for d in $PATH; do + [ -n "$d" ] || continue + if [ -x "$d/codegraph" ]; then + for e in "$d"/*; do + [ "$(basename "$e")" = codegraph ] && continue + ln -sf "$e" "$SHIM_BIN/" 2>/dev/null + done + d="$SHIM_BIN" + fi + out="${out:+$out:}$d" + done + printf '%s' "$out" +} +ARM_PATH="$(sanitized_path)" +if PATH="$ARM_PATH" command -v codegraph >/dev/null 2>&1; then + echo "WARNING: 'codegraph' is still on the arm PATH — runs will be contaminated" +fi +for t in claude node; do + PATH="$ARM_PATH" command -v "$t" >/dev/null || { echo "sanitized PATH lost '$t' — refusing to run"; exit 1; } +done + [ -n "$CG_BIN" ] || { echo "no codegraph binary on PATH (set CG_BIN)"; exit 1; } [ -d "$REPO/.codegraph" ] || { echo "no .codegraph index at $REPO — index it first"; exit 1; } case "$MODE" in headless|tmux|all) ;; *) echo "mode must be headless|tmux|all (got '$MODE')"; exit 1;; esac @@ -83,7 +123,7 @@ headless() { [ "$seg" -gt 1 ] && out="$OUT/run-$label.t$seg.jsonl" local resume=() [ -n "$sid" ] && resume=(--resume "$sid") - ( cd "$REPO" && claude -p "$q" \ + ( cd "$REPO" && PATH="$ARM_PATH" claude -p "$q" \ --output-format stream-json --verbose \ --permission-mode bypassPermissions \ --model "${MODEL:-sonnet}" --effort "${EFFORT:-high}" \ @@ -104,9 +144,11 @@ headless() { echo } +# CG_ARMS=with|without|both — re-run one arm without redoing the other. +ARMS="${CG_ARMS:-both}" if [ "$MODE" = headless ] || [ "$MODE" = all ]; then - headless "headless-with" "$OUT/mcp-codegraph.json" - headless "headless-without" "$OUT/mcp-empty.json" + case "$ARMS" in both|with) headless "headless-with" "$OUT/mcp-codegraph.json";; esac + case "$ARMS" in both|without) headless "headless-without" "$OUT/mcp-empty.json";; esac fi if [ "$MODE" = tmux ] || [ "$MODE" = all ]; then