ci: Activate V8 E2E impact map filter in PR CI (#31757)

Co-authored-by: n8n-cat-bot[bot] <n8n-cat-bot[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

packages/testing/janitor/src/cli.ts

@@ -45,14 +45,7 @@ import {
 	formatBaselineInfo,
 	getBaselinePath,
 } from './core/baseline.js';
-import {
-	type ImpactMap,
-	type InternedImpactMap,
-	decodeImpactMap,
-	encodeImpactMap,
-	mergeCoverage,
-	resolveImpact,
-} from './core/coverage-map.js';
+import { encodeImpactMap, mergeCoverage } from './core/coverage-map.js';
 import { extractDiffs } from './core/extract-diffs.js';
 import {
 	ImpactAnalyzer,
@@ -83,6 +76,7 @@ import { createProject } from './core/project-loader.js';
 import { toJSON, toConsole } from './core/reporter.js';
 import { filterToFailedSpecs } from './core/retry-filter.js';
 import { computeScope, formatScope } from './core/scope-analyzer.js';
+import { selectE2e } from './core/select-e2e.js';
 import { TcrExecutor, formatTcrResultConsole, formatTcrResultJSON } from './core/tcr-executor.js';
 import { TestDiscoveryAnalyzer } from './core/test-discovery-analyzer.js';
 import { runTestScoped } from './core/test-scoped-runner.js';
@@ -539,6 +533,24 @@ async function runOrchestrate(options: CliOptions): Promise<void> {
 		}
 	}
 
+	// Composable allowlist filter. distribute-tests.mjs pre-computes the union
+	// of AST + V8 selection and writes it here; orchestrate then balances shards
+	// against that subset instead of the full discovered set.
+	if (options.includeSpecsFile) {
+		const includeRaw = fs.readFileSync(options.includeSpecsFile, 'utf-8');
+		const include = new Set(
+			includeRaw
+				.split(/[\n,]+/)
+				.map((s) => s.trim())
+				.filter(Boolean),
+		);
+		const totalBefore = specs.length;
+		specs = specs.filter((s) => include.has(s.path));
+		console.error(
+			`Include: ${specs.length}/${totalBefore} specs after applying allowlist (${include.size} entries)`,
+		);
+	}
+
 	const metrics: Record<string, number> = {};
 	if (config.orchestration.metricsPath) {
 		const metricsPath = path.isAbsolute(config.orchestration.metricsPath)
@@ -669,45 +681,15 @@ function runMergeCoverage(options: CliOptions): void {
 	);
 }
 
-/** select-e2e: changed files + impact map → spec list (JSON).
- *
- *  Two layers of safety, both biased to OVER-select (never miss a regression):
- *   - FAIL-OPEN on the map source: a missing/unreadable/corrupt map → broad
- *     (run everything). This is what makes swapping the committed file for a
- *     remote webhook safe — a fetch failure degrades to running the full suite,
- *     never to skipping tests.
- *   - DEFAULT-BROAD on content: any changed file absent from a loaded map → broad.
- */
+/** select-e2e: changed files + impact map → spec list (JSON). I/O wrapper
+ *  around {@link selectE2e}, where the fail-open safety contract lives. */
 function runSelectE2e(options: CliOptions): void {
-	const changed = (readChangedFiles(options) ?? []).map((file) => ({ file }));
-	const allSpecs = options.allSpecsFile
-		? fs
-				.readFileSync(options.allSpecsFile, 'utf8')
-				.split(/[\n,]+/)
-				.map((s) => s.trim())
-				.filter(Boolean)
-		: undefined;
-
-	let map: ImpactMap = {};
-	let failOpen: string | undefined;
-	if (options.mapFile && fs.existsSync(options.mapFile)) {
-		try {
-			const parsed: unknown = JSON.parse(fs.readFileSync(options.mapFile, 'utf8'));
-			// Interned form ({specs, files}) is decoded; a plain ImpactMap is used as-is.
-			const isInterned =
-				typeof parsed === 'object' && parsed !== null && 'specs' in parsed && 'files' in parsed;
-			map = isInterned ? decodeImpactMap(parsed as InternedImpactMap) : (parsed as ImpactMap);
-		} catch (error) {
-			failOpen = `unreadable map: ${String(error)}`;
-		}
-	} else {
-		failOpen = options.mapFile ? `map not found: ${options.mapFile}` : 'no --map provided';
-	}
-
-	// With an empty map every changed file is "unmapped" → resolveImpact returns
-	// broad, so fail-open falls out of the same code path — no special-casing.
-	const result = resolveImpact(changed, map, { allSpecs });
-	console.log(JSON.stringify({ ...result, failOpen }));
+	const result = selectE2e({
+		changedFiles: readChangedFiles(options) ?? [],
+		mapFile: options.mapFile,
+		allSpecsFile: options.allSpecsFile,
+	});
+	console.log(JSON.stringify(result));
 }
 
 async function main(): Promise<void> {

packages/testing/janitor/src/cli/arg-parser.ts

@@ -69,6 +69,8 @@ export interface CliOptions {
 	outMap?: string;
 	mapFile?: string;
 	allSpecsFile?: string;
+	/** Path to a newline-separated allowlist of spec paths (orchestrate). */
+	includeSpecsFile?: string;
 }
 
 const SUBCOMMANDS: Record<string, Command> = {
@@ -224,6 +226,9 @@ const VALUE_FLAG_HANDLERS: Record<string, (options: CliOptions, value: string) =
 	'--all-specs=': (opts, value) => {
 		opts.allSpecsFile = value;
 	},
+	'--include-specs-file=': (opts, value) => {
+		opts.includeSpecsFile = value;
+	},
 };
 
 function createDefaultOptions(): CliOptions {

packages/testing/janitor/src/core/select-e2e.test.ts

@@ -0,0 +1,126 @@
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import * as path from 'node:path';
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+
+import { type ImpactMap, type InternedImpactMap, encodeImpactMap } from './coverage-map.js';
+import { selectE2e } from './select-e2e.js';
+
+// The handler's fail-open contract is its safety guarantee: every failure mode
+// of the map source must degrade to mode 'broad', never to an empty spec set
+// (which would silently skip real tests). These cases pin that contract so a
+// future refactor of resolveImpact / loadMap can't quietly invert it.
+
+describe('selectE2e — fail-open contract', () => {
+	let tempDir: string;
+
+	beforeEach(() => {
+		tempDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'select-e2e-')));
+	});
+
+	afterEach(() => {
+		fs.rmSync(tempDir, { recursive: true, force: true });
+	});
+
+	const ALL_SPECS = ['tests/e2e/a.spec.ts', 'tests/e2e/b.spec.ts'];
+
+	const writeAllSpecs = (content: string): string => {
+		const p = path.join(tempDir, 'all-specs.txt');
+		fs.writeFileSync(p, content);
+		return p;
+	};
+
+	it('no --map provided → broad with failOpen reason', () => {
+		const result = selectE2e({
+			changedFiles: ['packages/cli/src/x.ts'],
+			allSpecsFile: writeAllSpecs(ALL_SPECS.join('\n')),
+		});
+		expect(result.mode).toBe('broad');
+		expect(result.failOpen).toBe('no --map provided');
+		expect(result.specs).toEqual([...ALL_SPECS].sort());
+	});
+
+	it("map path that doesn't exist → broad with failOpen reason", () => {
+		const missing = path.join(tempDir, 'never-existed.json');
+		const result = selectE2e({
+			changedFiles: ['packages/cli/src/x.ts'],
+			mapFile: missing,
+			allSpecsFile: writeAllSpecs(ALL_SPECS.join('\n')),
+		});
+		expect(result.mode).toBe('broad');
+		expect(result.failOpen).toBe(`map not found: ${missing}`);
+		expect(result.specs).toEqual([...ALL_SPECS].sort());
+	});
+
+	it('corrupt / non-JSON map → broad with failOpen reason', () => {
+		const mapPath = path.join(tempDir, 'corrupt.json');
+		fs.writeFileSync(mapPath, '{not valid json,,,');
+		const result = selectE2e({
+			changedFiles: ['packages/cli/src/x.ts'],
+			mapFile: mapPath,
+			allSpecsFile: writeAllSpecs(ALL_SPECS.join('\n')),
+		});
+		expect(result.mode).toBe('broad');
+		expect(result.failOpen).toMatch(/^unreadable map:/);
+		expect(result.specs).toEqual([...ALL_SPECS].sort());
+	});
+
+	// THE COLLAPSE: empty map → every changed file is unmapped → resolveImpact
+	// returns broad. If a refactor ever makes resolveImpact({}, …) return scoped/
+	// empty, every other test still passes — this one fails. That is the point.
+	it('empty map {} → broad (the fail-open collapse)', () => {
+		const mapPath = path.join(tempDir, 'empty.json');
+		fs.writeFileSync(mapPath, '{}');
+		const result = selectE2e({
+			changedFiles: ['packages/cli/src/x.ts'],
+			mapFile: mapPath,
+			allSpecsFile: writeAllSpecs(ALL_SPECS.join('\n')),
+		});
+		expect(result.mode).toBe('broad');
+		// File was loaded successfully — no failOpen reason. The broad comes from
+		// resolveImpact's DEFAULT-BROAD rule (changed file absent from the map).
+		expect(result.failOpen).toBeUndefined();
+		expect(result.unmapped).toEqual(['packages/cli/src/x.ts']);
+		expect(result.specs).toEqual([...ALL_SPECS].sort());
+	});
+
+	it('interned {specs, files} map → decoded correctly, resolves scoped', () => {
+		const map: ImpactMap = {
+			'packages/cli/src/x.ts': { '10': ['tests/e2e/a.spec.ts'] },
+		};
+		const interned: InternedImpactMap = encodeImpactMap(map);
+		const mapPath = path.join(tempDir, 'interned.json');
+		fs.writeFileSync(mapPath, JSON.stringify(interned));
+
+		const result = selectE2e({
+			changedFiles: ['packages/cli/src/x.ts'],
+			mapFile: mapPath,
+		});
+		expect(result.mode).toBe('scoped');
+		expect(result.failOpen).toBeUndefined();
+		expect(result.specs).toEqual(['tests/e2e/a.spec.ts']);
+	});
+
+	describe('--all-specs parsing', () => {
+		const triggerBroad = (allSpecsFile: string) =>
+			selectE2e({
+				changedFiles: ['packages/never/covered.ts'],
+				allSpecsFile,
+			});
+
+		it('parses a newline-separated list', () => {
+			const file = writeAllSpecs('tests/e2e/a.spec.ts\ntests/e2e/b.spec.ts\n');
+			expect(triggerBroad(file).specs).toEqual(['tests/e2e/a.spec.ts', 'tests/e2e/b.spec.ts']);
+		});
+
+		it('parses a comma-separated list', () => {
+			const file = writeAllSpecs('tests/e2e/a.spec.ts,tests/e2e/b.spec.ts');
+			expect(triggerBroad(file).specs).toEqual(['tests/e2e/a.spec.ts', 'tests/e2e/b.spec.ts']);
+		});
+
+		it('parses mixed newline + comma + trims empties', () => {
+			const file = writeAllSpecs('tests/e2e/a.spec.ts,\n,tests/e2e/b.spec.ts,\n\n');
+			expect(triggerBroad(file).specs).toEqual(['tests/e2e/a.spec.ts', 'tests/e2e/b.spec.ts']);
+		});
+	});
+});

packages/testing/janitor/src/core/select-e2e.ts

@@ -0,0 +1,72 @@
+/**
+ * `select-e2e` handler: changed files + impact map → spec list.
+ *
+ * The file-system-aware wrapper around the pure {@link resolveImpact} resolver.
+ * This is where the FAIL-OPEN safety contract lives — every failure mode
+ * (missing map, unreadable map, corrupt JSON, empty map) must degrade to
+ * `mode: 'broad'` so the caller runs the full suite, never an empty one.
+ *
+ * Extracted from {@link runSelectE2e} in `cli.ts` so the contract can be
+ * exhaustively unit-tested without spawning a subprocess.
+ */
+
+import * as fs from 'node:fs';
+
+import {
+	type ImpactMap,
+	type InternedImpactMap,
+	type ResolveResult,
+	decodeImpactMap,
+	resolveImpact,
+} from './coverage-map.js';
+
+export interface SelectE2eInput {
+	/** Changed files (file paths). */
+	changedFiles: string[];
+	/** Path to the impact map JSON. Missing/unreadable → fail-open broad. */
+	mapFile?: string;
+	/** Path to a newline/comma separated list of every known spec. */
+	allSpecsFile?: string;
+}
+
+export interface SelectE2eResult extends ResolveResult {
+	/** Set when the map could not be loaded; the result is broad as a safety. */
+	failOpen?: string;
+}
+
+function parseSpecList(raw: string): string[] {
+	return raw
+		.split(/[\n,]+/)
+		.map((s) => s.trim())
+		.filter(Boolean);
+}
+
+function loadMap(mapFile: string | undefined): { map: ImpactMap; failOpen?: string } {
+	if (!mapFile) return { map: {}, failOpen: 'no --map provided' };
+	if (!fs.existsSync(mapFile)) return { map: {}, failOpen: `map not found: ${mapFile}` };
+	try {
+		const parsed: unknown = JSON.parse(fs.readFileSync(mapFile, 'utf8'));
+		// Interned form ({specs, files}) is decoded; a plain ImpactMap is used as-is.
+		const isInterned =
+			typeof parsed === 'object' && parsed !== null && 'specs' in parsed && 'files' in parsed;
+		const map = isInterned ? decodeImpactMap(parsed as InternedImpactMap) : (parsed as ImpactMap);
+		return { map };
+	} catch (error) {
+		return { map: {}, failOpen: `unreadable map: ${String(error)}` };
+	}
+}
+
+/**
+ * Resolve changed files against the impact map, biased to OVER-select. With an
+ * empty/missing map every changed file is "unmapped" → {@link resolveImpact}
+ * returns `mode: 'broad'`, so fail-open falls out of the same code path.
+ */
+export function selectE2e(input: SelectE2eInput): SelectE2eResult {
+	const allSpecs = input.allSpecsFile
+		? parseSpecList(fs.readFileSync(input.allSpecsFile, 'utf8'))
+		: undefined;
+	const { map, failOpen } = loadMap(input.mapFile);
+	const changed = input.changedFiles.map((file) => ({ file }));
+	const result = resolveImpact(changed, map, { allSpecs });
+	return { ...result, failOpen };
+}

packages/testing/playwright/scripts/distribute-tests.mjs

@@ -7,6 +7,18 @@
  * Thin wrapper that calls `janitor orchestrate` for generic shard distribution,
  * then maps capabilities to n8n-specific Docker images for the CI matrix.
  *
+ * Impact scoping is a domain-partitioned UNION of two analyzers (DEVP-364):
+ *   - app-source partition (changes outside packages/testing/playwright/)
+ *     → V8 coverage map via `janitor select-e2e` (see select-affected-e2e.mjs)
+ *   - playwright-internal partition (changes inside the playwright package)
+ *     → AST walk via `janitor impact --test-list`
+ * Union the two spec sets, write to a temp file, then `orchestrate
+ * --include-specs-file=<that>` balances shards against the union.
+ *
+ * FAIL-OPEN at every layer: if EITHER analyzer bails to broad (V8 map missing,
+ * AST throws, etc.), broad wins — `orchestrate` runs without the allowlist and
+ * the full suite ships. Never skip a test on uncertainty.
+ *
  * Usage:
  *   node distribute-tests.mjs --matrix <shards> --orchestrate  # GitHub Actions matrix with images
  *   node distribute-tests.mjs --matrix <shards>                # Simple matrix (no distribution)
@@ -14,6 +26,8 @@
  */
 
 import { execFileSync } from 'child_process';
+import { mkdtempSync, writeFileSync, rmSync } from 'node:fs';
+import { tmpdir } from 'node:os';
 import path from 'path';
 import { fileURLToPath } from 'url';
 
@@ -21,6 +35,7 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url));
 const PLAYWRIGHT_DIR = path.resolve(__dirname, '..');
 const REPO_ROOT = path.resolve(__dirname, '..', '..', '..', '..');
 const JANITOR_CLI = path.resolve(__dirname, '..', '..', 'janitor', 'dist', 'cli.js');
+const SELECT_AFFECTED_E2E = path.resolve(__dirname, 'select-affected-e2e.mjs');
 const PLAYWRIGHT_PREFIX = path.relative(REPO_ROOT, PLAYWRIGHT_DIR) + path.sep;
 const CONTAINER_STARTUP_TIME = 22_500; // 22.5s average per fixture
 
@@ -53,37 +68,115 @@ function getRequiredImages(capabilities) {
 	return [...images].sort();
 }
 
-function hasExternalFiles(files) {
-	const externalFiles = files.split(',').filter((f) => !f.startsWith(PLAYWRIGHT_PREFIX));
-	if (externalFiles.length === 0) return false;
+/**
+ * Partition CSV of changed files into (a) inside the playwright package,
+ * (b) everywhere else. The AST analyzer can only trace (a); the V8 coverage
+ * map keys on (b). Empty CSVs short-circuit to empty arrays.
+ */
+function partitionFiles(filesCsv) {
+	const internal = [];
+	const external = [];
+	for (const raw of filesCsv.split(',')) {
+		const f = raw.trim();
+		if (!f) continue;
+		if (f.startsWith(PLAYWRIGHT_PREFIX)) internal.push(f);
+		else external.push(f);
+	}
+	return { internal, external };
+}
 
-	// Files outside the playwright project can't be traced by the impact analyzer
-	console.error(
-		`Impact: ${externalFiles.length} file(s) outside playwright project — running all tests`,
+/**
+ * Query the V8 coverage map for specs affected by app-source changes.
+ *
+ * Returns `{ broad: true, reason }` whenever the underlying selection is broad
+ * (map missing/stale/corrupt, or any unmapped change file). Broad propagates
+ * up the union as "broad wins" — orchestrate runs without the allowlist.
+ *
+ * Any thrown error from the wrapper is caught and treated as broad — fail-open.
+ */
+function selectV8Specs(externalFiles) {
+	if (externalFiles.length === 0)
+		return { broad: false, specs: new Set(), reason: 'no app-source changes' };
+	try {
+		const out = execFileSync('node', [SELECT_AFFECTED_E2E, ...externalFiles], {
+			encoding: 'utf-8',
+			stdio: ['pipe', 'pipe', 'inherit'],
+		});
+		const parsed = JSON.parse(out);
+		if (parsed.mode === 'broad') {
+			return {
+				broad: true,
+				reason: parsed.failOpen ?? `unmapped: ${(parsed.unmapped ?? []).join(', ')}`,
+			};
+		}
+		return { broad: false, specs: new Set(parsed.specs ?? []) };
+	} catch (error) {
+		return { broad: true, reason: `select-e2e failed: ${String(error)}` };
+	}
+}
+
+/**
+ * Query the AST walker for specs affected by playwright-internal changes.
+ *
+ * Returns a Set of spec paths (playwright-root-relative) or `{ broad: true }`
+ * if the analyzer fails. The AST walk only sees relationships expressed in
+ * source — a change to a non-test file the walker doesn't recognise is
+ * silently dropped, but that is the AST's own concern; we only fail-open on
+ * thrown errors, matching the V8 path.
+ */
+function selectAstSpecs(internalFiles, base) {
+	if (internalFiles.length === 0) return { broad: false, specs: new Set() };
+	// Strip the playwright prefix — janitor runs with cwd=PLAYWRIGHT_DIR.
+	const normalized = internalFiles.map((f) =>
+		f.startsWith(PLAYWRIGHT_PREFIX) ? f.slice(PLAYWRIGHT_PREFIX.length) : f,
 	);
-	for (const f of externalFiles) console.error(`  - ${f}`);
-	return true;
+	const cliArgs = ['impact', '--test-list', `--files=${normalized.join(',')}`];
+	if (base) cliArgs.push(`--base=${base}`);
+	try {
+		const out = execFileSync('node', [JANITOR_CLI, ...cliArgs], {
+			cwd: PLAYWRIGHT_DIR,
+			encoding: 'utf-8',
+			stdio: ['pipe', 'pipe', 'inherit'],
+		});
+		const specs = out
+			.split('\n')
+			.map((s) => s.trim())
+			.filter(Boolean);
+		return { broad: false, specs: new Set(specs) };
+	} catch (error) {
+		return { broad: true, reason: `ast impact failed: ${String(error)}` };
+	}
+}
+
+function logSelectionDecision(decision) {
+	console.error('\n🎯 Impact selection:');
+	console.error(`  mode: ${decision.mode}`);
+	if (decision.v8) {
+		const v8 = decision.v8;
+		console.error(
+			`  v8 (app-source): ${v8.broad ? `broad (${v8.reason})` : `${v8.specs.size} specs`}`,
+		);
+	} else {
+		console.error('  v8 (app-source): skipped (no app-source changes)');
+	}
+	if (decision.ast) {
+		const ast = decision.ast;
+		console.error(
+			`  ast (playwright): ${ast.broad ? `broad (${ast.reason})` : `${ast.specs.size} specs`}`,
+		);
+	} else {
+		console.error('  ast (playwright): skipped (no playwright changes)');
+	}
+	if (decision.unionSize !== undefined) {
+		console.error(`  union: ${decision.unionSize} specs`);
+	}
+	console.error('');
 }
 
 function getOrchestration(numShards, options = {}) {
 	const cliArgs = ['orchestrate', `--shards=${numShards}`];
-	// Disable impact when explicit files include non-playwright paths the analyzer can't trace.
-	// When no files are provided, janitor auto-detects via git — impact stays enabled.
-	const useImpact = options.impact && !(options.files && hasExternalFiles(options.files));
-
-	if (useImpact) {
-		cliArgs.push('--impact');
-		if (options.base) cliArgs.push(`--base=${options.base}`);
-	}
-	if (useImpact && options.files) {
-		// Normalize repo-root-relative paths to playwright-root-relative
-		// git diff gives 'packages/testing/playwright/foo.ts', janitor expects 'foo.ts'
-		const normalized = options.files
-			.split(',')
-			.map((f) => (f.startsWith(PLAYWRIGHT_PREFIX) ? f.slice(PLAYWRIGHT_PREFIX.length) : f))
-			.join(',');
-		cliArgs.push(`--files=${normalized}`);
-	}
+	const includeFile = options.includeSpecsFile;
+	if (includeFile) cliArgs.push(`--include-specs-file=${includeFile}`);
 	const output = execFileSync('node', [JANITOR_CLI, ...cliArgs], {
 		cwd: PLAYWRIGHT_DIR,
 		encoding: 'utf-8',
@@ -92,6 +185,42 @@ function getOrchestration(numShards, options = {}) {
 	return JSON.parse(output);
 }
 
+/**
+ * Resolve the impact decision for a CI run: union AST + V8 selection over the
+ * changed-files partition, OR broad if either analyzer bails. Writes the union
+ * to a temp file and returns its path (the caller passes it to orchestrate).
+ *
+ * Returns `null` when broad — caller orchestrates without the allowlist.
+ */
+function resolveImpactAllowlist(filesCsv, base) {
+	const { internal, external } = partitionFiles(filesCsv);
+	const v8 = selectV8Specs(external);
+	const ast = selectAstSpecs(internal, base);
+
+	// Fail-open: either bails → broad wins. Run everything.
+	if (v8.broad || ast.broad) {
+		logSelectionDecision({
+			mode: 'broad',
+			v8: external.length > 0 ? v8 : null,
+			ast: internal.length > 0 ? ast : null,
+		});
+		return null;
+	}
+
+	const union = new Set([...(v8.specs ?? []), ...(ast.specs ?? [])]);
+	logSelectionDecision({
+		mode: 'scoped',
+		v8: external.length > 0 ? v8 : null,
+		ast: internal.length > 0 ? ast : null,
+		unionSize: union.size,
+	});
+
+	const tmp = mkdtempSync(path.join(tmpdir(), 'distribute-tests-'));
+	const allowPath = path.join(tmp, 'include-specs.txt');
+	writeFileSync(allowPath, [...union].join('\n'));
+	return allowPath;
+}
+
 const args = process.argv.slice(2);
 const matrixMode = args.includes('--matrix');
 const orchestrateMode = args.includes('--orchestrate');
@@ -106,6 +235,25 @@ if (!shards || shards < 1) {
 	process.exit(1);
 }
 
+let includeSpecsFile;
+let cleanupPaths = [];
+if (impactMode && filesArg) {
+	includeSpecsFile = resolveImpactAllowlist(filesArg, baseArg);
+	if (includeSpecsFile) cleanupPaths.push(path.dirname(includeSpecsFile));
+} else if (impactMode) {
+	console.error('Impact: no --files provided — running full suite');
+}
+
+function cleanup() {
+	for (const p of cleanupPaths) {
+		try {
+			rmSync(p, { recursive: true, force: true });
+		} catch {
+			// best-effort
+		}
+	}
+}
+
 if (matrixMode) {
 	if (!orchestrateMode) {
 		const matrix = Array.from({ length: shards }, (_, i) => ({
@@ -115,7 +263,7 @@ if (matrixMode) {
 		}));
 		console.log(JSON.stringify(matrix));
 	} else {
-		const result = getOrchestration(shards, { impact: impactMode, files: filesArg, base: baseArg });
+		const result = getOrchestration(shards, { includeSpecsFile });
 
 		if (result.shards.length === 0) {
 			console.error('\n⏭️  No specs to run — all filtered out by discovery/impact. Skipping.\n');
@@ -152,11 +300,14 @@ if (matrixMode) {
 	const index = parseInt(args[1]);
 	if (isNaN(index) || index < 0 || index >= shards) {
 		console.error(`Index must be between 0 and ${shards - 1}`);
+		cleanup();
 		process.exit(1);
 	}
-	const result = getOrchestration(shards, { impact: impactMode, files: filesArg, base: baseArg });
+	const result = getOrchestration(shards, { includeSpecsFile });
 	const shard = result.shards[index];
 	if (shard) {
 		console.log(shard.specs.filter((s) => !QUARANTINE.has(s)).join('\n'));
 	}
 }
+
+cleanup();

packages/testing/playwright/scripts/select-affected-e2e.mjs

@@ -17,30 +17,61 @@
  */
 
 import { execFileSync } from 'node:child_process';
-import { existsSync } from 'node:fs';
+import { existsSync, accessSync, constants as fsConstants } from 'node:fs';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 
-const __dirname = path.dirname(fileURLToPath(import.meta.url));
+const __filename = fileURLToPath(import.meta.url);
+const __dirname = path.dirname(__filename);
 const REPO_ROOT = path.resolve(__dirname, '..', '..', '..', '..');
 const JANITOR_CLI = path.resolve(__dirname, '..', '..', 'janitor', 'dist', 'cli.js');
 const COMMITTED_MAP = path.join(REPO_ROOT, '.github', 'test-metrics', 'e2e-impact-map.json');
 
-/** @returns {string | null} path to a readable impact map, or null → fail-open broad. */
-function resolveMapPath() {
+/**
+ * @param {{ mapPath?: string }} [opts]
+ * @returns {string | null} path to a readable impact map, or null → fail-open broad.
+ *
+ * Wrapped in try/catch so any I/O failure (race deletion, perms, fs error) is
+ * swallowed into a null return — never throws. The caller then omits --map,
+ * which makes select-e2e fail open to broad (run the full suite).
+ */
+export function resolveMapPath(opts = {}) {
 	// FUTURE: fetch a remote webhook to a temp file here and return that path;
-	// wrap in try/catch and return null on failure (fail-open).
-	return existsSync(COMMITTED_MAP) ? COMMITTED_MAP : null;
+	// keep the try/catch — return null on failure (fail-open).
+	const target = opts.mapPath ?? COMMITTED_MAP;
+	try {
+		if (!existsSync(target)) return null;
+		accessSync(target, fsConstants.R_OK);
+		return target;
+	} catch {
+		return null;
+	}
 }
 
-const changedFiles = process.argv.slice(2).join(',') || process.env.CHANGED_FILES || '';
-const mapPath = resolveMapPath();
+/**
+ * Build the janitor argv for `select-e2e`. Pure (no I/O) so the wrapper's glue
+ * can be tested without spawning a subprocess. If `mapPath` is null we omit
+ * `--map`, which makes select-e2e fail open to broad.
+ *
+ * @param {{ changedFiles: string, mapPath: string | null, allSpecs?: string }} input
+ * @returns {string[]}
+ */
+export function buildArgs({ changedFiles, mapPath, allSpecs }) {
+	const args = ['select-e2e', `--changed-files=${changedFiles}`];
+	if (mapPath) args.push(`--map=${mapPath}`);
+	if (allSpecs) args.push(`--all-specs=${allSpecs}`);
+	return args;
+}
 
-const args = ['select-e2e', `--changed-files=${changedFiles}`];
-// Omitting --map (or pointing at a missing file) makes select-e2e fail open to broad.
-if (mapPath) args.push(`--map=${mapPath}`);
-const allSpecs = process.env.ALL_SPECS_FILE;
-if (allSpecs) args.push(`--all-specs=${allSpecs}`);
+function runAsScript() {
+	const changedFiles = process.argv.slice(2).join(',') || process.env.CHANGED_FILES || '';
+	const mapPath = resolveMapPath();
+	const args = buildArgs({ changedFiles, mapPath, allSpecs: process.env.ALL_SPECS_FILE });
+	const out = execFileSync('node', [JANITOR_CLI, ...args], { encoding: 'utf-8' });
+	process.stdout.write(out);
+}
 
-const out = execFileSync('node', [JANITOR_CLI, ...args], { encoding: 'utf-8' });
-process.stdout.write(out);
+// Only execute when invoked directly — keeps the module importable for tests.
+if (process.argv[1] && path.resolve(process.argv[1]) === __filename) {
+	runAsScript();
+}

packages/testing/playwright/scripts/select-affected-e2e.test.ts

@@ -0,0 +1,72 @@
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import * as path from 'node:path';
+
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+
+// @ts-expect-error — JS module without types; runtime export is what we test.
+import { buildArgs, resolveMapPath } from './select-affected-e2e.mjs';
+
+// The wrapper is the bridge between CI (raw changed-files list) and janitor
+// select-e2e. Its only job is to keep selection FAIL-OPEN: any failure in
+// locating the map must degrade to broad — never throw, never hide selection.
+
+describe('select-affected-e2e wrapper — fail-open contract', () => {
+	let tempDir: string;
+
+	beforeEach(() => {
+		tempDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'select-wrapper-')));
+	});
+
+	afterEach(() => {
+		fs.rmSync(tempDir, { recursive: true, force: true });
+	});
+
+	describe('resolveMapPath()', () => {
+		it('returns null when the committed map is missing (wrapper degrades to broad)', () => {
+			const missing = path.join(tempDir, 'never-existed.json');
+			expect(resolveMapPath({ mapPath: missing })).toBeNull();
+		});
+
+		it('returns the path when the map exists and is readable', () => {
+			const p = path.join(tempDir, 'present.json');
+			fs.writeFileSync(p, '{}');
+			expect(resolveMapPath({ mapPath: p })).toBe(p);
+		});
+
+		it('returns null on an unreadable map (e.g. perms), never throws', () => {
+			// On non-root environments chmod 000 makes accessSync(R_OK) throw.
+			// Skip the assertion on root (CI sometimes runs as root and can read anything).
+			if (process.getuid?.() === 0) return;
+			const p = path.join(tempDir, 'unreadable.json');
+			fs.writeFileSync(p, '{}');
+			fs.chmodSync(p, 0o000);
+			try {
+				expect(resolveMapPath({ mapPath: p })).toBeNull();
+			} finally {
+				fs.chmodSync(p, 0o644);
+			}
+		});
+	});
+
+	describe('buildArgs()', () => {
+		// Omitting --map is exactly how the wrapper signals fail-open broad to
+		// the janitor CLI — assert by absence, not by a placeholder value.
+		it('omits --map when mapPath is null (fail-open broad)', () => {
+			const args = buildArgs({ changedFiles: 'a.ts,b.ts', mapPath: null });
+			expect(args).toEqual(['select-e2e', '--changed-files=a.ts,b.ts']);
+			expect(args.some((a: string) => a.startsWith('--map='))).toBe(false);
+		});
+
+		it('passes --map and --all-specs through when provided', () => {
+			expect(
+				buildArgs({ changedFiles: 'a.ts', mapPath: '/tmp/m.json', allSpecs: '/tmp/s.txt' }),
+			).toEqual([
+				'select-e2e',
+				'--changed-files=a.ts',
+				'--map=/tmp/m.json',
+				'--all-specs=/tmp/s.txt',
+			]);
+		});
+	});
+});