shard-jobs: validate every sub-shard summary, bounded JUnit read, stricter --ci-verify records, unique count paths, no serial launch without wall

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

scripts/lib/shard-jobs.ts

@@ -26,6 +26,7 @@
  * (the console stream is byte-identical with or without it). Counts are
  * evidence for k > 1 comparisons; they never decide a verdict.
  */
+import * as fs from 'node:fs';
 import * as os from 'node:os';
 import type { FreeCiPlan, FreeCiResult, FreeShardOutcome } from '../test-free-shards';
 import { guardFreeHome, sharedFreeHome, type FreeHomeGuard, type FreeHomeGuardFactory } from './free-home-guard';
@@ -107,7 +108,8 @@ export function parseJunitCounts(xml: string | null, planned: readonly string[],
   const canonical = (printed: string) => {
     const stripped = normalizeRelativePath(printed).replace(/^(?:\.{1,2}\/)+/, '');
     if (plannedSet.has(stripped)) return stripped;
-    return planned.find(file => stripped.endsWith(`/${file}`) || file.endsWith(`/${stripped}`)) ?? stripped;
+    const matches = planned.filter(file => stripped.endsWith(`/${file}`) || file.endsWith(`/${stripped}`));
+    return matches.length === 1 ? matches[0] : stripped;
   };
   let current: { file: string; state: 'pass' | 'fail' | 'skip' | 'todo'; expects: number } | null = null;
   const finish = () => {
@@ -139,6 +141,18 @@ export function parseJunitCounts(xml: string | null, planned: readonly string[],
   return result(true);
 }
 
+const JUNIT_MAX_BYTES = 64 * 1024 * 1024;
+
+/** The child-written report, only when it is a regular file of bounded size (never a link, FIFO or device). */
+export function readJunitReport(file: string): string | null {
+  let fd: number;
+  try { fd = fs.openSync(file, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW ?? 0) | (fs.constants.O_NONBLOCK ?? 0)); } catch { return null; }
+  try {
+    const stat = fs.fstatSync(fd);
+    return stat.isFile() && stat.size <= JUNIT_MAX_BYTES ? fs.readFileSync(fd, 'utf8') : null;
+  } catch { return null; } finally { fs.closeSync(fd); }
+}
+
 /** One shard's counts from its sub-shards' counts; complete only when every part is and no file repeats. */
 export function mergeTestCounts(parts: Array<ShardTestCounts | undefined>): ShardTestCounts | undefined {
   if (parts.every(part => !part)) return undefined;
@@ -199,7 +213,9 @@ export function subShardProblems(settled: SubShardSettlement): string[] {
   const problems: string[] = [];
   if (JSON.stringify(outcome.files) !== JSON.stringify(assignment.files)) problems.push(`${assignment.id} reported another file set`);
   const summary = outcome.summary;
-  if (outcome.status === 'passed' && (!summary || summary.filesRan !== assignment.files.length || summary.sawTerminalSummary !== true
+  if (!summary || (summary.sawTerminalSummary && summary.filesRan !== assignment.files.length)) {
+    problems.push(`${assignment.id} reported ${summary?.filesRan ?? 'no'} executed file(s) for ${assignment.files.length} assigned`);
+  } else if (outcome.status === 'passed' && (summary.filesRan !== assignment.files.length || summary.sawTerminalSummary !== true
     || outcome.exitCode !== 0 || outcome.failingFiles.length || outcome.unattributedFailures)) {
     problems.push(`${assignment.id} passed without a complete execution summary`);
   }
@@ -275,8 +291,15 @@ export function subShardRecordProblems(result: FreeCiResult): string[] {
   const problems: string[] = [];
   if (new Set(files).size !== files.length || files.length !== result.outcome.files.length
     || !result.outcome.files.every(file => files.includes(file))) problems.push('sub-shard records do not cover the shard exactly once');
-  if (result.outcome.status === 'passed' && records.some(record => record.status !== 'passed' || record.summary?.filesRan !== record.files.length
-    || (record.files.length > 0 && record.summary?.sawTerminalSummary !== true))) problems.push('a passing shard holds a failed or incomplete sub-shard');
+  const prefix = `${result.planId.slice(0, 12)}:${result.outcome.shard}.`;
+  if (records.some((record, index) => record.id !== `${prefix}${index + 1}`)
+    || records.filter(record => record.phase === 'parallel').length > result.jobs!
+    || records.some((record, index) => record.phase === 'serial' ? index !== records.length - 1 : record.phase !== 'parallel')) {
+    problems.push('sub-shard ids or phases do not match the plan and --jobs');
+  }
+  if (records.some(record => record.status === 'missing' || record.summary?.filesRan !== record.files.length
+    || (record.files.length > 0 && record.summary?.sawTerminalSummary !== true))) problems.push('a sub-shard did not run every assigned file');
+  if (result.outcome.status === 'passed' && records.some(record => record.status !== 'passed')) problems.push('a passing shard holds a failed sub-shard');
   return problems;
 }
 
@@ -297,12 +320,15 @@ export function subShardTimeoutText(
 ): string[] {
   const suspects = inFlight.filter(file => record.files.includes(file));
   const files = suspects.length ? suspects : record.files;
+  const fix = record.phase === 'serial'
+    ? 'this file already runs alone after the parallel sub-shards; fix or split it, never raise the timeout'
+    : `if it passes serially, add it to SLOW_UNDER_LOAD in scripts/lib/shard-jobs.ts with run id ${context.runId}; never raise the timeout`;
   return [
     `[test:free] ${FREE_SUBSHARD_TIMEOUT}: CI shard ${shard} sub-shard ${record.id} (${record.files.length} files) hit its ${Math.round(wallTimeoutMs / 1000)}s wall at --jobs ${jobs} on ${context.runner}, run ${context.runId}.`,
     `[test:free]   in flight: ${suspects.length ? suspects.join(', ') : 'unknown (no open file chunk; see the sub-shard log)'}`,
     `[test:free]   reproduce serially: printf '%s\\n' ${files.map(quote).join(' ')} > files.txt && GSTACK_FREE_JOBS=1 bun run scripts/test-free-shards.ts --files-from files.txt`,
     `[test:free]   or the whole CI shard at k=1: bun run scripts/test-free-shards.ts --ci-run ${quote(context.planPath)} --shard ${shard} --result shard-${shard}.json --jobs 1`,
-    `[test:free]   if it passes serially, add it to SLOW_UNDER_LOAD in scripts/lib/shard-jobs.ts with run id ${context.runId}; never raise the timeout. See ${DOCS}`,
+    `[test:free]   ${fix}. See ${DOCS}`,
   ];
 }
 
@@ -344,9 +370,12 @@ export async function runCiShard(input: CiShardInput, deps: CiShardDeps): Promis
   const total = assignments.length;
   log(`[test:free] CI shard ${shard.shard}/${plan.shards.length} at --jobs ${jobs}: ${assignments
     .map(entry => `sub-shard ${entry.id} = runner shard ${entry.index}/${total} (${entry.phase}, ${entry.files.length} files)`).join('; ')}`);
-  const run = (entry: SubShardAssignment, homeGuard?: FreeHomeGuardFactory, wall = wallTimeoutMs): Promise<SubShardSettlement> =>
-    deps.runShard(entry.files, entry.index, total, { wallTimeoutMs: wall, verbose, testCounts: true, ...(homeGuard ? { homeGuard } : {}) })
+  const walls = new Map<string, number>();
+  const run = (entry: SubShardAssignment, homeGuard?: FreeHomeGuardFactory, wall = wallTimeoutMs): Promise<SubShardSettlement> => {
+    walls.set(entry.id, wall);
+    return deps.runShard(entry.files, entry.index, total, { wallTimeoutMs: wall, verbose, testCounts: true, ...(homeGuard ? { homeGuard } : {}) })
       .then(outcome => ({ assignment: entry, outcome }), (error: unknown) => ({ assignment: entry, error: error instanceof Error ? error.message : String(error) }));
+  };
   const parallel = assignments.filter(entry => entry.phase === 'parallel');
   const serial = assignments.filter(entry => entry.phase === 'serial');
   const phaseProblems: string[] = [];
@@ -359,13 +388,14 @@ export async function runCiShard(input: CiShardInput, deps: CiShardDeps): Promis
     if (homeChange) phaseProblems.push(homeChange);
   }
   for (const entry of serial) {
+    const left = startedAt + wallTimeoutMs - Date.now();
     settled.push(deps.terminationRequested() ? { assignment: entry, error: 'cancelled before the serial phase' }
-      : await run(entry, undefined, Math.max(1, startedAt + wallTimeoutMs - Date.now())));
+      : left <= 0 ? { assignment: entry, error: 'no wall left after the parallel phase' } : await run(entry, undefined, left));
   }
   const merged = mergeSubShardOutcomes(shard, settled, Date.now() - startedAt, phaseProblems);
   for (const problem of merged.problems) console.error(`[test:free] ${FREE_SUBSHARD_REJECTED}: CI shard ${shard.shard}: ${problem}`);
   for (const record of merged.records.filter(entry => entry.status === 'timed-out')) {
-    for (const line of subShardTimeoutText(shard.shard, jobs, record, merged.outcome.inFlight ?? [], wallTimeoutMs, input.context)) console.error(line);
+    for (const line of subShardTimeoutText(shard.shard, jobs, record, merged.outcome.inFlight ?? [], walls.get(record.id) ?? wallTimeoutMs, input.context)) console.error(line);
   }
   log(`[test:free] CI shard ${shard.shard}: ${merged.outcome.status} across ${total} sub-shards in ${Math.round(merged.outcome.elapsedMs / 1000)}s`);
   const retried = await deps.retry([merged.outcome]);
@@ -376,8 +406,8 @@ export async function runCiShard(input: CiShardInput, deps: CiShardDeps): Promis
 }
 
 /** The duration seed takes only one-file children's walls, never a sub-shard's or a shard's. */
-export function assertSoloSeedSample(file: string, outcome: Pick<FreeShardOutcome, 'files'>): void {
-  if (outcome.files.length !== 1 || outcome.files[0] !== file) {
+export function assertSoloSeedSample(file: string, outcome: Pick<FreeShardOutcome, 'files' | 'summary'>): void {
+  if (outcome.files.length !== 1 || outcome.files[0] !== file || (outcome.summary?.filesRan ?? 0) > 1) {
     throw new Error(`Duration seed sample for ${file} came from a ${outcome.files.length}-file child; seeds are recorded only from one-file serial children.`);
   }
 }

scripts/test-free-shards.ts

@@ -97,7 +97,7 @@ import { attributeFreeHomeWriters, guardFreeHome, sharedFreeHome, type FreeHomeG
 import { appendStepSummary, ciHealthSummary, SEED_REFRESH_COMMAND, unseededWarning, windowsCurationLine } from './lib/free-ci-health';
 import { curateWindowsSafe, type CurationResult } from './lib/windows-curation';
 import {
-  assertSoloSeedSample, ciRunContext, parseJobsFlag, parseJunitCounts, rejectJobsWithoutCiRun, runCiShard, SLOW_UNDER_LOAD, subShardRecordProblems,
+  assertSoloSeedSample, ciRunContext, parseJobsFlag, parseJunitCounts, readJunitReport, rejectJobsWithoutCiRun, runCiShard, SLOW_UNDER_LOAD, subShardRecordProblems,
   validateJobsForShard, type ShardTestCounts, type SubShardRecord,
 } from './lib/shard-jobs';
 export { curateWindowsSafe, detectWindowsFragility, KNOWN_WINDOWS_INCOMPATIBLE, type CurationResult } from './lib/windows-curation';
@@ -1730,9 +1730,7 @@ export async function runFreeShard(
       shardLog.write(diagnostic + '\n');
     }
     await new Promise<void>((resolve) => shardLog.stream.end(() => resolve()));
-    if (junitPath) {
-      try { junitXml = fs.readFileSync(junitPath, 'utf8'); } catch { /* counts record the missing report */ }
-    }
+    if (junitPath) junitXml = readJunitReport(junitPath);
     try {
       if (!cleanupError) fs.rmSync(stateDir, { recursive: true, force: true });
     } catch {

test/shard-jobs.test.ts

@@ -15,7 +15,7 @@ import {
 } from '../scripts/test-free-shards';
 import {
   assertSoloSeedSample, assignSubShards, ciRunContext, FREE_JOBS_INVALID, mergeSubShardOutcomes, parseJobsFlag, parseJunitCounts,
-  runCiShard, SLOW_UNDER_LOAD, subShardTimeoutText, validateJobsForShard, type CiShardDeps, type RunShardOptions,
+  readJunitReport, runCiShard, SLOW_UNDER_LOAD, subShardRecordProblems, subShardTimeoutText, validateJobsForShard, type CiShardDeps, type RunShardOptions,
 } from '../scripts/lib/shard-jobs';
 
 const ROOT = path.resolve(import.meta.dir, '..');
@@ -199,6 +199,36 @@ describe('k > 1: merge, rejection and one retry pass per CI shard', () => {
     } finally { errors.mockRestore(); }
   });
 
+  test('a failed sub-shard that ran too few or too many files is rejected, so counts cannot cancel and no retry repairs it', async () => {
+    const errors = spyOn(console, 'error').mockImplementation(() => {});
+    try {
+      const h = harness(files.slice(0, 4), runFiles => ({ ...failed(runFiles, [runFiles[0]]),
+        summary: { testsRan: 2, filesRan: runFiles.includes(files[0]) ? 1 : 3, sawTerminalSummary: true } }));
+      const { result } = await h.run(2);
+      expect(result.outcome.summary!.filesRan).toBe(4);
+      expect(result.outcome.unattributedFailures).toBeGreaterThanOrEqual(2);
+      expect(eligibleFreeRetryFiles([result.outcome])).toBeNull();
+      const forged: FreeCiResult = { ...result, outcome: { ...result.outcome, unattributedFailures: 0 },
+        retry: { ...passed([files[0], files[1]]), shard: 2 } };
+      expect(() => verifyFreeCiResults({ ...h.plan, revision: 'revision-a' } as never, [forged])).toThrow(/did not run every assigned file|identity/);
+      expect(subShardRecordProblems(forged)).toContain('a sub-shard did not run every assigned file');
+    } finally { errors.mockRestore(); }
+  });
+
+  test('the serial sub-shard gets only what is left of the wall, and none left means it is not launched', async () => {
+    const errors = spyOn(console, 'error').mockImplementation(() => {});
+    try {
+      const h = harness([...files, EXCLUSIVE], async runFiles => {
+        if (!runFiles.includes(EXCLUSIVE)) await new Promise(resolve => setTimeout(resolve, 60));
+        return passed(runFiles);
+      });
+      const { result } = await runCiShard({ plan: h.plan, shard: h.shard, revision: 'revision-a', jobs: 2, wallTimeoutMs: 50, verbose: false, context }, h.deps);
+      expect(h.calls.some(call => call.files.includes(EXCLUSIVE))).toBe(false);
+      expect(result.subShards!.at(-1)).toMatchObject({ status: 'missing', phase: 'serial' });
+      expect(result.outcome.status).toBe('failed');
+    } finally { errors.mockRestore(); }
+  });
+
   test('a real-home change in the concurrent phase and a cancelled serial phase both fail the shard', async () => {
     const errors = spyOn(console, 'error').mockImplementation(() => {});
     try {
@@ -239,12 +269,19 @@ describe('k > 1: merge, rejection and one retry pass per CI shard', () => {
     const merged = mergeSubShardOutcomes(shard, assignments.map(assignment => ({ assignment, outcome: passed(assignment.files) })), 10);
     const result: FreeCiResult = { planId: plan.id, revision: 'revision-a', outcome: merged.outcome, retry: null, jobs: 2, subShards: merged.records };
     expect(() => verifyFreeCiResults(plan, [result])).not.toThrow();
+    const renamed = structuredClone(result);
+    renamed.subShards![0].id = 'other:1.1';
+    expect(() => verifyFreeCiResults(plan, [renamed])).toThrow(/ids or phases/);
+    const tooMany = structuredClone(result);
+    tooMany.jobs = 1;
+    expect(subShardRecordProblems({ ...tooMany, jobs: 2, subShards: [...tooMany.subShards!, { ...tooMany.subShards![0], id: `${plan.id.slice(0, 12)}:1.3`, files: [] }] }))
+      .toContain('sub-shard ids or phases do not match the plan and --jobs');
     const overlap = structuredClone(result);
     overlap.subShards![1].files = [...overlap.subShards![1].files, overlap.subShards![0].files[0]];
     expect(() => verifyFreeCiResults(plan, [overlap])).toThrow(/sub-shards rejected/);
     const hidden = structuredClone(result);
     hidden.subShards![0].status = 'failed';
-    expect(() => verifyFreeCiResults(plan, [hidden])).toThrow(/failed or incomplete sub-shard/);
+    expect(() => verifyFreeCiResults(plan, [hidden])).toThrow(/a passing shard holds a failed sub-shard/);
     expect(() => verifyFreeCiResults(plan, [{ ...result, jobs: 1 }])).toThrow(/valid --jobs/);
   });
 });
@@ -268,6 +305,27 @@ describe('per-file test and expect() counts from bun JUnit', () => {
       totals: { tests: 4, pass: 1, fail: 1, skip: 1, todo: 1, expects: 9 } });
   });
 
+  test('an ambiguous short path is never credited to one of two planned files that share it', () => {
+    const counts = parseJunitCounts(xml('a.test.ts'), ['test/a.test.ts', 'browse/test/a.test.ts'], 4);
+    expect(counts.complete).toBe(false);
+    expect(counts.problem).toContain('unplanned');
+  });
+
+  test('the report is read only as a bounded regular file, never through a link or a FIFO', () => {
+    const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-junit-'));
+    try {
+      fs.writeFileSync(path.join(dir, 'real.xml'), '<testsuites/>');
+      expect(readJunitReport(path.join(dir, 'real.xml'))).toBe('<testsuites/>');
+      expect(readJunitReport(path.join(dir, 'absent.xml'))).toBeNull();
+      if (process.platform !== 'win32') {
+        fs.symlinkSync(path.join(dir, 'real.xml'), path.join(dir, 'link.xml'));
+        expect(readJunitReport(path.join(dir, 'link.xml'))).toBeNull();
+        const fifo = Bun.spawnSync(['mkfifo', path.join(dir, 'fifo.xml')], { timeout: 5_000 });
+        if (fifo.exitCode === 0) expect(readJunitReport(path.join(dir, 'fifo.xml'))).toBeNull();
+      }
+    } finally { fs.rmSync(dir, { recursive: true, force: true }); }
+  });
+
   test('a missing report, an unplanned file or a count that disagrees with bun summary is incomplete, never silently complete', () => {
     expect(parseJunitCounts(null, ['test/a.test.ts'], 4)).toMatchObject({ complete: false, problem: 'bun wrote no JUnit report' });
     expect(parseJunitCounts(xml('test/z.test.ts'), ['test/a.test.ts'], 4)).toMatchObject({ complete: false, problem: expect.stringContaining('unplanned') });
@@ -376,6 +434,7 @@ describe('serial lists and the duration seed', () => {
   test('seeds take only one-file children; sub-shard runs have no path to the seed writer', () => {
     expect(() => assertSoloSeedSample('test/a.test.ts', { files: ['test/a.test.ts'] })).not.toThrow();
     expect(() => assertSoloSeedSample('test/a.test.ts', { files: ['test/a.test.ts', 'test/b.test.ts'] })).toThrow(/one-file serial children/);
+    expect(() => assertSoloSeedSample('test/a.test.ts', { files: ['test/a.test.ts'], summary: { testsRan: 9, filesRan: 2, sawTerminalSummary: true } })).toThrow(/one-file serial children/);
     expect(fs.readFileSync(path.join(ROOT, 'scripts/lib/shard-jobs.ts'), 'utf8')).not.toContain('writeDurationSeed');
     const runner = fs.readFileSync(path.join(ROOT, 'scripts/test-free-shards.ts'), 'utf8');
     const recorder = runner.slice(runner.indexOf('async function recordFreeTestDurations('), runner.indexOf('async function retryFailedFreeFiles('));