From 8750d33ef0d05f7f9a684d54820543c14b3e7c46 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 1 Nov 2026 15:25:21 +0000 Subject: [PATCH] fix: graph checkout ignores the tree's export attributes; the tracer runs the free runner's own command and sandbox Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- docs/test-impact.md | 8 ++++---- lib/test-impact/key.ts | 23 +++++++++++++---------- scripts/test-impact.ts | 21 +++++++++++++-------- test/test-impact-key.test.ts | 17 +++++++++++++++-- 4 files changed, 45 insertions(+), 24 deletions(-) diff --git a/docs/test-impact.md b/docs/test-impact.md index c65a05807..b41d07e82 100644 --- a/docs/test-impact.md +++ b/docs/test-impact.md @@ -6,7 +6,7 @@ GST-167 is the plan to skip a free test on a pull request only when main already `scripts/test-impact.ts` is a Bun CLI: -- `trace [--out ] ...` runs each file with `bun test` under strace and writes its read set to `` (default `.context/test-impact/traces`). It needs Linux with strace installed. +- `trace [--out ] ...` runs each file under strace with the free runner's own `bun test` arguments and shard sandbox, and writes its read set to `` (default `.context/test-impact/traces`). It needs Linux with strace installed. - `key --tree [--base ] [--traces ] [--json] ...` prints each file's key at ``, or why it has none. - `explain [--tree ] [--base ] [--traces ] ` prints every input of one file's key and why it is or isn't eligible. @@ -18,12 +18,12 @@ The recorder runs `strace -f -y -qq -v` with `openat, open, execve, execveat, ge ## What the key covers -`lib/test-impact/key.ts` reads everything at the given tree through git, never from the working tree. The key is a sha256 over: +`lib/test-impact/key.ts` reads everything at the given tree through git, never from the working tree. The graph runs over a checkout of that tree made through a private index, so the tree's own `.gitattributes` can't hide files the way `export-ignore` would in `git archive`. The key is a sha256 over: - each traced path's bytes (read with `git cat-file --batch`), or that it is a directory, or that it is absent; - the names in each traced directory listing, at the tree; - the test's forward import-graph closure from `lib/pregate/import-graph.ts`, up to 12 hops; -- `bun.lock`, every `tsconfig*.json` and the `dependencies` in `.gstack/pregate.json`; +- `bun.lock`, `bunfig.toml`, every `tsconfig*.json` and the `dependencies` in `.gstack/pregate.json`; - the key code itself: the tracer, the key, the CLI, the free-test runner and planner (`scripts/test-free-shards.ts`) with every repo module it imports (shard engine, HOME guard, CI health, Windows curation, the paid/free split, the state root), the import graph and `lib/pregate/lane-selection.ts`; - the bytes of each executable outside the repo the trace ran (git, bash, tar and so on), read on the machine computing the key; - the runtime (platform, architecture, Bun version) and the values of `CI`, `GITHUB_ACTIONS`, `TZ`, `LANG`, `LC_ALL`, `NODE_ENV`, `EVALS` and `EVALS_TIER`. @@ -39,4 +39,4 @@ A file is ineligible, so it always runs, when: - its import-graph walk reaches the 12-hop cap. The walk fails closed: a deeper chain makes the test ineligible instead of cutting the closure short; - the tree changes any key-code file against the base. Then every file runs. -Limits: strace can't see which environment variables a test reads, so only the allowlist above is in the key, and a test that depends on another variable has to read it through a file the trace sees or stay off the reuse path. The recorder runs `bun test` directly, not inside the free runner's per-shard sandbox, so PR 3 should record traces through the runner. Timing assertions, flaky history and the shard's execution grouping are for PR 3 and PR 4, which have the receipts to check them against. Windows has no tracer, so the Windows lane stays full. +Limits: strace can't see which environment variables a test reads, so only the allowlist above is in the key, and a test that depends on another variable has to read it through a file the trace sees or stay off the reuse path. The recorder reuses the runner's arguments and sandbox but not its home-write guard, which only watches and changes nothing a test reads. Timing assertions, flaky history and the shard's execution grouping are for PR 3 and PR 4, which have the receipts to check them against. Windows has no tracer, so the Windows lane stays full. diff --git a/lib/test-impact/key.ts b/lib/test-impact/key.ts index a17fafcdd..2380e8329 100644 --- a/lib/test-impact/key.ts +++ b/lib/test-impact/key.ts @@ -8,7 +8,7 @@ * - listed: the sorted entry names of every repo directory the trace listed; * - graph: the forward import-graph closure of the test (lib/pregate/import-graph.ts), * capped at MAX_HOPS hops; a closure that reaches the cap is ineligible, never truncated; - * - global: bun.lock, every tsconfig*.json and the pregate `dependencies`; + * - global: bun.lock, bunfig.toml, every tsconfig*.json and the pregate `dependencies`; * - code: KEY_CODE_FILES, the runner, planner, shard engine, graph and key code; * - tool: the bytes of each executable outside the repo the trace ran (git, bash, tar…), read on this machine; * - runtime and env: the runtime manifest and ENV_ALLOWLIST values. @@ -58,8 +58,8 @@ export interface KeyOptions { now?: number; runtime?: Record; env?: Record; } -function git(repoRoot: string, args: string[], input?: Buffer | string): Buffer { - const r = spawnSync('git', args, { cwd: repoRoot, input, timeout: 300_000, maxBuffer: 1024 * 1024 * 1024 }); +function git(repoRoot: string, args: string[], input?: Buffer | string, env?: NodeJS.ProcessEnv): Buffer { + const r = spawnSync('git', args, { cwd: repoRoot, input, env, timeout: 300_000, maxBuffer: 1024 * 1024 * 1024 }); if (r.status !== 0) throw new Error(`git ${args.join(' ')} failed: ${r.error?.message ?? r.stderr?.toString().trim() ?? r.status}`); return r.stdout; } @@ -104,19 +104,22 @@ export function catText(repoRoot: string, treeish: string, rel: string): string return r.status === 0 ? r.stdout : null; } -/** A tree-ish exported to a fresh temp directory with every file staged, so the import graph (git ls-files) sees exactly that tree. */ +/** + * A tree-ish checked out into a fresh temp directory with every file staged, so the import graph (git ls-files) sees exactly + * that tree. It goes through a private index and checkout-index, never `git archive`, whose export-ignore and export-subst + * attributes would let the tree itself hide files from the graph. + */ export function materializeTree(repoRoot: string, treeish: string): string { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-test-impact-tree-')); - const tar = `${dir}.tar`; + const env = { ...process.env, GIT_INDEX_FILE: `${dir}.index` }; try { - git(repoRoot, ['archive', '--format=tar', '-o', tar, treeish]); - const x = spawnSync('tar', ['-xf', tar, '-C', dir], { timeout: 300_000 }); - if (x.status !== 0) throw new Error(`tar -xf failed: ${x.stderr?.toString().trim()}`); + git(repoRoot, ['read-tree', treeish], undefined, env); + git(repoRoot, ['checkout-index', '-a', '-f', `--prefix=${dir}/`], undefined, env); git(dir, ['init', '-q']); git(dir, ['add', '-A', '-f']); return dir; } catch (error) { fs.rmSync(dir, { recursive: true, force: true }); throw error; } - finally { fs.rmSync(tar, { force: true }); } + finally { fs.rmSync(env.GIT_INDEX_FILE, { force: true }); } } /** The runtime manifest a key binds: platform, architecture and Bun version. */ @@ -187,7 +190,7 @@ export function computeKeys(opts: KeyOptions): KeyReport { const envNow = envDigests(opts.env ?? process.env); const runtime = opts.runtime ?? runtimeManifest(); const shared: KeyInput[] = [ - ...[...treeIdx.entries.keys()].filter(p => p === 'bun.lock' || TSCONFIG.test(p)).map(p => ({ kind: 'global' as const, path: p, digest: '' })), + ...[...treeIdx.entries.keys()].filter(p => p === 'bun.lock' || p === 'bunfig.toml' || TSCONFIG.test(p)).map(p => ({ kind: 'global' as const, path: p, digest: '' })), { kind: 'global', path: `${PREGATE_CONFIG_PATH}#dependencies`, digest: sha(JSON.stringify(dependencies)) }, ...KEY_CODE_FILES.map(p => ({ kind: 'code' as const, path: p, digest: '' })), ...Object.entries(runtime).sort().map(([k, v]) => ({ kind: 'runtime' as const, path: k, digest: v })), diff --git a/scripts/test-impact.ts b/scripts/test-impact.ts index eedee008d..d392e192b 100755 --- a/scripts/test-impact.ts +++ b/scripts/test-impact.ts @@ -3,7 +3,8 @@ * test-impact — trace free tests and compute their reuse keys (GST-167 PR 2; docs/test-impact.md). * * scripts/test-impact.ts trace [--out ] ... - * Run each file with `bun test` under strace (Linux) and write its read set to + * Run each file under strace (Linux) exactly as the free runner does (its bun test arguments and shard + * sandbox) and write its read set to * (default .context/test-impact/traces). * scripts/test-impact.ts key --tree [--base ] [--traces ] [--json] ... * Print each file's key at , or why it is ineligible. With --base, the key code is @@ -17,7 +18,9 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { bunIsolationArgs } from '../lib/bun-isolation'; +import { randomUUID } from 'node:crypto'; +import { createShardSandbox } from './lib/shard-engine'; +import { buildShardArgs, disableGitAutoMaintenance } from './test-free-shards'; import * as localKey from '../lib/test-impact/key'; import type { KeyOptions, KeyReport } from '../lib/test-impact/key'; import { STRACE_FLAGS, TRACE_VERSION, envDigests, parseStrace, tracePath, type TraceRecord } from '../lib/test-impact/trace'; @@ -55,20 +58,22 @@ function trace(root: string, argv: string[]): number { const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: root, encoding: 'utf8', timeout: 30_000 }); let failed = 0; for (const file of files.map(f => relTo(root, f))) { - const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-test-impact-run-')); + const sandbox = createShardSandbox('gstack-test-impact-run-', process.env, { realpath: true }); + const env = { ...sandbox.env, BROWSE_STATE_FILE: path.join(sandbox.stateDir, '.gstack', 'browse.json'), GSTACK_FREE_SHARD_ID: randomUUID(), BUN_RUNTIME_TRANSPILER_CACHE_PATH: '0' }; + disableGitAutoMaintenance(env); const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-test-impact-log-')); try { const log = path.join(logDir, 'strace.txt'); - const r = spawnSync('strace', [...STRACE_FLAGS, '-o', log, '--', process.execPath, ...bunIsolationArgs(), 'test', path.join(root, file), '--timeout=30000', '--max-concurrency=1'], - { cwd: root, env: { ...process.env, TMPDIR: tmp, TMP: tmp, TEMP: tmp, BUN_RUNTIME_TRANSPILER_CACHE_PATH: '0' }, timeout: 600_000, maxBuffer: 256 * 1024 * 1024, stdio: ['ignore', 'pipe', 'pipe'] }); - const set = parseStrace(fs.existsSync(log) ? fs.readFileSync(log, 'utf8') : '', { repoRoot: root, home: os.homedir(), tmpDir: tmp, pathDirs: (process.env.PATH ?? '').split(path.delimiter) }); - const record: TraceRecord = { version: TRACE_VERSION, test: file, tracedAt: new Date().toISOString(), commit: head.status === 0 ? head.stdout.trim() : null, exitCode: r.status, runtime: localKey.runtimeManifest(), env: envDigests(process.env), ...set }; + const r = spawnSync('strace', [...STRACE_FLAGS, '-o', log, '--', process.execPath, ...buildShardArgs([file], { rootDir: root })], + { cwd: root, env, timeout: 600_000, maxBuffer: 256 * 1024 * 1024, stdio: ['ignore', 'pipe', 'pipe'] }); + const set = parseStrace(fs.existsSync(log) ? fs.readFileSync(log, 'utf8') : '', { repoRoot: root, home: os.homedir(), tmpDir: sandbox.stateDir, pathDirs: (process.env.PATH ?? '').split(path.delimiter) }); + const record: TraceRecord = { version: TRACE_VERSION, test: file, tracedAt: new Date().toISOString(), commit: head.status === 0 ? head.stdout.trim() : null, exitCode: r.status, runtime: localKey.runtimeManifest(), env: envDigests(env), ...set }; fs.writeFileSync(tracePath(out, file), JSON.stringify(record, null, 2) + '\n'); if (r.status !== 0) failed++; const verdict = set.ineligible.length ? `ineligible: ${set.ineligible.join('; ')}` : 'eligible'; console.log(`traced ${file}: exit ${r.status ?? r.signal}, ${set.paths.length} paths, ${Object.keys(set.dirs).length} listed dirs, ${verdict} → ${path.relative(root, tracePath(out, file))}`); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + fs.rmSync(sandbox.stateDir, { recursive: true, force: true }); fs.rmSync(logDir, { recursive: true, force: true }); } } diff --git a/test/test-impact-key.test.ts b/test/test-impact-key.test.ts index 58b7ae45c..b772c8572 100644 --- a/test/test-impact-key.test.ts +++ b/test/test-impact-key.test.ts @@ -80,6 +80,17 @@ describe('test-impact key: what changes it', () => { expect(keyAt(repo, 'HEAD', t).key).not.toBe(before.key); }); + test('a branch .gitattributes export-ignore cannot hide a graph input', () => { + const t = 'test/a.test.ts'; + const repo = fixture({ 'lib/hidden.ts': 'export const x = 1;\n', [t]: "import { x } from '../lib/hidden';\nimport { test } from 'bun:test';\ntest('a', () => x);\n" }); + traceFrom(repo, t); + const before = keyAt(repo, 'main', t); + commit(repo, { '.gitattributes': 'lib/hidden.ts export-ignore\n', 'lib/hidden.ts': 'export const x = 2;\n' }); + const after = keyAt(repo, 'HEAD', t); + expect(after.inputs).toContainEqual(expect.objectContaining({ kind: 'graph', path: 'lib/hidden.ts' })); + expect(after.key).not.toBe(before.key); + }); + test('a file added under a directory the test listed changes the key', () => { const t = 'test/listing.test.js'; const repo = fixture({ 'test/data/a.txt': 'a\n', 'test/data/b.txt': 'b\n', [t]: `${NODE_TEST}test('lists', () => require('fs').readdirSync(require('path').join(__dirname, 'da' + 'ta')));\n` }); @@ -221,11 +232,13 @@ describe('scripts/test-impact.ts', () => { }); const hasStrace = process.platform === 'linux' && spawnSync('strace', ['-V'], { timeout: 10_000 }).status === 0; - test.if(hasStrace)('trace (real strace): a spawned bin that sources a sibling records the sibling, and editing it changes the key', () => { + test.if(hasStrace)('trace (real strace, the runner\'s own command): a spawned bin that sources a sibling and the bunfig preload are recorded, and editing the sibling changes the key', () => { const t = 'test/tool2.test.ts'; const repo = fixture({ 'bin/tool2': '#!/bin/bash\ncd "$(dirname "$0")"\nsource ./tool2-lib.sh\ngreet\n', 'bin/tool2-lib.sh': 'greet() { echo hi; }\n', + 'bunfig.toml': '[test]\npreload = ["./test/setup.ts"]\n', + 'test/setup.ts': 'export {};\n', [t]: "import { expect, test } from 'bun:test';\nimport { spawnSync } from 'node:child_process';\nimport * as path from 'node:path';\ntest('greets', () => expect(spawnSync(path.join(import.meta.dir, '..', 'bin', 'tool2'), { encoding: 'utf8', timeout: 10000 }).stdout.trim()).toBe('hi'));\n", }); fs.chmodSync(path.join(repo.work, 'bin/tool2'), 0o755); @@ -237,7 +250,7 @@ describe('scripts/test-impact.ts', () => { expect(r.status).toBe(0); expect(r.out).toContain(`traced ${t}: exit 0`); const rec = readTrace(traces, t)!; - expect(rec.paths).toEqual(expect.arrayContaining([t, 'bin/tool2', 'bin/tool2-lib.sh'])); + expect(rec.paths).toEqual(expect.arrayContaining([t, 'bin/tool2', 'bin/tool2-lib.sh', 'bunfig.toml', 'test/setup.ts'])); expect(rec.ineligible).toEqual([]); const env = { ...process.env, ...repo.env }; const runtime = { platform: process.platform, arch: process.arch, bun: Bun.version };