fix(agent-eval): drop harvested queries whose project cannot be read (#532)

* fix(agent-eval): drop harvested queries whose project cannot be read

A code-mode call that passed projectPath as a variable or expression was
credited to the session's starting directory, so a query about another
project could be kept under a public repository. A relative projectPath
was resolved against the harvester's directory instead of the session's.
Such calls are now resolved against the session directory or dropped.

Also: --public accepts a remote URL or a name ending in .git, and the
script's entry check works through symlinks and paths with spaces.

* test(agent-eval): run the harvester end to end on an unreadable projectPath

Author: bompus

__tests__/query-replay.test.ts

@@ -1,8 +1,10 @@
-import { readdirSync, readFileSync } from 'node:fs';
+import { spawnSync, execFileSync } from 'node:child_process';
+import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
 import { join, resolve } from 'node:path';
 import { describe, expect, it } from 'vitest';
 // @ts-expect-error untyped ESM script
-import { callsInLine, repoSlug } from '../scripts/agent-eval/harvest-queries.mjs';
+import { callDirectory, callsInLine, repoSlug } from '../scripts/agent-eval/harvest-queries.mjs';
 // @ts-expect-error untyped ESM script
 import { compareRow } from '../scripts/agent-eval/replay-queries.mjs';
 
@@ -60,6 +62,39 @@ describe('query harvest', () => {
       .toEqual([{ query: 'Q', projectPath: undefined, maxFiles: 3 }]);
   });
 
+  it('never credits a call with an unreadable or relative project to the wrong repository', () => {
+    const call = (input: string) => callsInLine({ payload: { type: 'custom_tool_call', input } })[0];
+    const variable = call('const p = "/private"; codegraph_explore({query:"Q", projectPath: p})');
+    expect(variable).toEqual({ query: 'Q', projectPath: null });
+    expect(callDirectory(variable, '/public')).toBeNull();
+    expect(callDirectory(call('codegraph_explore({query:"Q", projectPath: join(a, "b")})'), '/public')).toBeNull();
+    expect(callDirectory(call('codegraph_explore({query:"Q", projectPath: `${root}/x`})'), '/public')).toBeNull();
+    expect(callDirectory({ query: 'Q', projectPath: 'sub/dir' }, '/session')).toBe(resolve('/session', 'sub/dir'));
+    expect(callDirectory({ query: 'Q', projectPath: 'sub/dir' }, undefined)).toBeNull();
+    expect(callDirectory({ query: 'Q', projectPath: 7 }, '/public')).toBeNull();
+    expect(callDirectory({ query: 'Q', projectPath: undefined }, '/session')).toBe('/session');
+    expect(callDirectory({ query: 'Q', projectPath: '/abs' }, '/session')).toBe('/abs');
+  });
+  it('harvests only calls whose project it can read', () => {
+    const root = mkdtempSync(join(tmpdir(), 'harvest-'));
+    try {
+      const repo = join(root, 'repo');
+      execFileSync('git', ['init', '-q', repo]);
+      execFileSync('git', ['-C', repo, 'remote', 'add', 'origin', 'https://github.com/pub/repo.git']);
+      const explore = (input: string) => JSON.stringify({ cwd: repo, payload: { type: 'custom_tool_call', input } });
+      writeFileSync(join(root, 's.jsonl'), [
+        explore('codegraph_explore({query:"kept"})'),
+        explore('codegraph_explore({query:"unreadable", projectPath: p})'),
+      ].join('\n'));
+      const script = resolve(import.meta.dirname, '../scripts/agent-eval/harvest-queries.mjs');
+      const run = spawnSync(process.execPath, [script, '--public', 'https://github.com/pub/repo', join(root, 's.jsonl')], { encoding: 'utf8' });
+      expect(run.status).toBe(0);
+      expect(JSON.parse(run.stdout).repos['pub/repo'].tasks.map((t: { prompt: string }) => t.prompt)).toEqual(['kept']);
+      expect(run.stderr).toContain('kept 1, duplicates 0, dropped 1');
+    } finally {
+      rmSync(root, { recursive: true, force: true });
+    }
+  });
   it('maps GitHub remotes to owner/repo', () => {
     expect(repoSlug('git@github.com:Owner/Repo.git\n')).toBe('owner/repo');
     expect(repoSlug('https://github.com/owner/repo')).toBe('owner/repo');

scripts/agent-eval/harvest-queries.mjs

@@ -16,7 +16,8 @@
 import { execFileSync } from 'node:child_process';
 import { createReadStream, readdirSync, statSync } from 'node:fs';
 import { createInterface } from 'node:readline';
-import { join } from 'node:path';
+import { isAbsolute, join, resolve } from 'node:path';
+import { fileURLToPath } from 'node:url';
 import { createHash } from 'node:crypto';
 
 /** Explore calls in one transcript line: [{ query, projectPath?, maxFiles? }]. */
@@ -51,8 +52,9 @@ const ESCAPES = { n: '\n', t: '\t', r: '\r', b: '\b', f: '\f', v: '\v', 0: '\0'
 /**
  * The top-level string and number fields of the JavaScript object literal that
  * opens at `src[open]` ('{'). Code-mode calls are JavaScript, not JSON: keys may
- * be bare, strings may use any quote and may contain braces. Values that are
- * not plain literals (expressions, template strings with `${`) are skipped.
+ * be bare, strings may use any quote and may contain braces. A field whose
+ * value is not a plain literal (a variable, an expression, a template string
+ * with `${`) is null, so a caller can tell it was present but unreadable.
  */
 export function literalFields(src, open) {
   const fields = {};
@@ -68,9 +70,10 @@ export function literalFields(src, open) {
       }
       i = j;
       if (depth === 1 && expectKey) { key = s; expectKey = false; }
-      else if (depth === 1 && expectValue && key !== undefined && literal) fields[key] = s;
+      else if (depth === 1 && expectValue && key !== undefined) fields[key] = literal ? s : null;
       expectValue = false;
     } else if (ch === '{' || ch === '[' || ch === '(') {
+      if (depth === 1 && expectValue && key !== undefined) fields[key] = null;
       depth++;
       if (depth === 1) expectKey = true;
       expectValue = false;
@@ -84,13 +87,27 @@ export function literalFields(src, open) {
       const word = /^[A-Za-z_$0-9.]+/.exec(src.slice(i))[0];
       i += word.length - 1;
       if (expectKey) { key = word; expectKey = false; }
-      else if (expectValue && key !== undefined && /^\d+$/.test(word)) fields[key] = Number(word);
+      else if (expectValue && key !== undefined) fields[key] = /^\d+$/.test(word) ? Number(word) : null;
       expectValue = false;
     }
   }
   return fields;
 }
 
+/**
+ * The directory a call asked about: its projectPath, resolved against the
+ * session's working directory when relative, or that directory when the call
+ * named none. Null when it cannot be known (a projectPath that is not a plain
+ * string, or a relative one with no session directory): such a call is
+ * dropped, never credited to the directory the session started in.
+ */
+export function callDirectory(call, cwd) {
+  if (call.projectPath === undefined) return cwd ?? null;
+  if (typeof call.projectPath !== 'string' || call.projectPath === '') return null;
+  if (isAbsolute(call.projectPath)) return call.projectPath;
+  return cwd ? resolve(cwd, call.projectPath) : null;
+}
+
 /** `owner/repo` from a remote on github.com itself (HTTPS, SSH URL or scp form), or null. */
 export function repoSlug(url) {
   const text = url.trim();
@@ -115,7 +132,10 @@ async function main(argv) {
   const inputs = [];
   let since = '';
   for (let i = 0; i < argv.length; i++) {
-    if (argv[i] === '--public') pub.add(argv[++i].toLowerCase());
+    if (argv[i] === '--public') {
+      const name = argv[++i];
+      pub.add(repoSlug(name) ?? name.toLowerCase().replace(/\.git$/, ''));
+    }
     else if (argv[i] === '--since') since = argv[++i];
     else inputs.push(argv[i]);
   }
@@ -148,7 +168,7 @@ async function main(argv) {
         cwd = entry.cwd ?? entry.payload?.cwd ?? cwd;
         if (since && (entry.timestamp ?? '') < since) continue;
         for (const call of callsInLine(entry)) {
-          const dir = call.projectPath ?? cwd;
+          const dir = callDirectory(call, cwd);
           const slug = dir ? slugFor(dir) : null;
           if (!slug || !pub.has(slug)) { dropped++; continue; }
           const prompt = call.query.trim().replace(/\s+/g, ' ');
@@ -166,8 +186,10 @@ async function main(argv) {
     }
   }
   console.log(JSON.stringify({ repos }, null, 2));
-  console.error(`kept ${kept}, duplicates ${duplicate}, dropped ${dropped} (not on a --public repository)`);
+  console.error(`kept ${kept}, duplicates ${duplicate}, dropped ${dropped} (project unknown or not on a --public repository)`);
   return 0;
 }
 
-if (import.meta.url === `file://${process.argv[1]}`) main(process.argv.slice(2)).then((code) => process.exit(code));
+if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) {
+  main(process.argv.slice(2)).then((code) => process.exit(code));
+}