Browse Source

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.
Colby McHenry 1 tháng trước cách đây
mục cha
commit
d3c01ce8ed

+ 11 - 2
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)');

+ 10 - 3
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}`));

+ 45 - 3
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