fix(#4220): terminate the Windows temp-sweep ancestor walk (and two bugs it unmasked) (#4245)

* fix(#4220): terminate the temp-sweep ancestor walk with a fixed-point check

scripts/run-tests.cjs's sweepProtectSet block walked each selected test
file's ancestor directories, stopping on `cur !== runTempRoot &&
cur.length > 1` — a POSIX-only sentinel. path.posix.dirname('/') === '/'
(length 1) correctly stops, but path.win32.dirname('C:\\') === 'C:\\'
(length 3) never satisfies the length check, so the walk spun forever on
Windows whenever a selected file lived outside runTempRoot (the common
case). This has hung every Windows CI shard since #4207.

Extract the walk into a pure, exported computeSweepProtectSet(selected,
runTempRoot, dirnameImpl) helper and replace the length sentinel with a
fixed-point check (stop when dirnameImpl(cur) === cur), which terminates
correctly on POSIX, Windows drive roots, and UNC roots alike with no
platform branch.

* fix(#4220): repoint TEMP/TMP alongside TMPDIR in run-tests-temp-root test child env

Node's os.tmpdir() on Windows never reads TMPDIR, only TEMP/TMP. The
test's runNode child-process env override only set TMPDIR, so on a
real Windows runner nested inside a run-tests invocation the child
inherited the outer process's already-repointed TEMP/TMP and its
mkdtempSync(os.tmpdir()) landed under the outer run's temp root
instead of the test's intended `outer` directory. This was masked on
gsd-test's benches and locally because Windows CI always died in the
#4220 infinite loop before reaching this test.

* fix(#4220): stop the ancestor walk from protecting the filesystem root itself

computeSweepProtectSet added `cur` to the protect set before checking
whether dirname(cur) === cur, so on the terminating iteration it
protected the filesystem root (posix `/`, and analogously a win32
drive root) instead of stopping before adding it. Caught by the
existing posix-parity regression assertion
(`!protectSet.has('/')`) on the linux-node24 gsd-test bench. Reorder
to compute the parent and check the fixed point before adding.

* fix(#4220): backfill changeset pr number to 4245

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-09-03 13:42:14 -04:00
committed by GitHub
parent 515191f07d
commit 456136659d
3 changed files with 160 additions and 14 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 4245
---
**Windows CI test runs no longer hang indefinitely.** The test runner's temp-sweep protection walk used a POSIX-only termination check that never fired on a Windows drive root, spinning forever and timing out every Windows CI shard. A second, previously-masked bug in the temp-root regression test's own child-process env override (only `TMPDIR`, not `TEMP`/`TMP`) is also fixed, since Windows never reads `TMPDIR`.

View File

@@ -957,6 +957,34 @@ function rankChunkFilesByWeight(files, weightOf, timingsTable) {
});
}
// #4020 / #4220: pure helper extracted from main()'s temp-sweep protection
// block. Walks each selected file's ancestor directories, protecting every
// one from a later temp-sweep, stopping either at runTempRoot or at the
// filesystem root. Termination uses a FIXED-POINT check (stop when
// dirnameImpl(cur) === cur) instead of a POSIX-only length sentinel
// (`cur.length > 1`): path.posix.dirname('/') === '/' (length 1) correctly
// stops, but path.win32.dirname('C:\\') === 'C:\\' (length 3) never
// satisfies a length check, so the old logic spun forever on Windows
// whenever a selected file lived outside runTempRoot — the common case,
// since most selected test files live in the repo checkout, not the temp
// root (root-caused every Windows CI shard timing out since #4207). The
// injectable dirnameImpl lets tests exercise path.win32/path.posix behavior
// directly without a real Windows/POSIX runner.
function computeSweepProtectSet(selectedFiles, runTempRoot, dirnameImpl = require('path').dirname) {
const protectSet = new Set();
for (const f of selectedFiles) {
let cur = f;
while (cur && cur !== runTempRoot) {
const parent = dirnameImpl(cur);
if (parent === cur) break; // cur IS the filesystem root — never protect it, just stop
protectSet.add(cur);
cur = parent;
}
if (cur === runTempRoot) protectSet.add(f); // exact-file case
}
return protectSet;
}
function main() {
const args = process.argv.slice(2);
const parsed = parseArgs(args);
@@ -1135,18 +1163,7 @@ function main() {
// chunk still runs — a harness may stage synthetic test files under the run
// root (tests/run-tests-harness.test.cjs's 30-file chunking fixture). Protect
// every ancestor of every selected file that lies INSIDE the run root.
const sweepProtectSet = new Set();
{
const { dirname } = require('path');
for (const f of selected) {
let cur = f;
while (cur && cur !== runTempRoot && cur.length > 1) {
sweepProtectSet.add(cur);
cur = dirname(cur);
}
if (cur === runTempRoot) sweepProtectSet.add(f); // exact-file case
}
}
const sweepProtectSet = computeSweepProtectSet(selected, runTempRoot);
// Sandbox the overlay home so the loader's global scan ($GSD_HOME/.gsd/capabilities)
// cannot read a developer's real installed capabilities during tests (ADR-1244 D2).
@@ -1643,4 +1660,8 @@ module.exports = {
setupRunTempRoot,
sweepRunTempRoot,
assertTempRootBounded,
// #4220: extracted for in-process unit coverage of the ancestor-walk
// termination logic (Windows drive-root hang fix) without a real
// Windows/POSIX runner.
computeSweepProtectSet,
};

View File

@@ -42,7 +42,10 @@ describe('#4020 — run-tests temp root', () => {
const outer = createTempDir('gsd-4020-outer-');
t.after(() => cleanup(outer));
const r = runNode(['-e', setupProbe], { timeoutMs: 30_000, env: { ...process.env, TMPDIR: outer } });
const r = runNode(['-e', setupProbe], {
timeoutMs: 30_000,
env: { ...process.env, TMPDIR: outer, TEMP: outer, TMP: outer },
});
assert.equal(r.exitCode, 0, `probe failed: ${r.stderr.slice(-300)}`);
const out = JSON.parse(r.stdout.trim().split(/\n/).pop());
assert.ok(out.base.startsWith('gsd-test-run-'),
@@ -64,7 +67,10 @@ describe('#4020 — run-tests temp root', () => {
const outer = createTempDir('gsd-4020-idem-');
t.after(() => cleanup(outer));
const r = runNode(['-e', probe], { timeoutMs: 30_000, env: { ...process.env, TMPDIR: outer } });
const r = runNode(['-e', probe], {
timeoutMs: 30_000,
env: { ...process.env, TMPDIR: outer, TEMP: outer, TMP: outer },
});
assert.equal(r.exitCode, 0, `probe failed: ${r.stderr.slice(-300)}`);
assert.ok(JSON.parse(r.stderr.trim().split('\n').pop()).reuse === true,
'a second invocation with the root active reuses it');
@@ -175,4 +181,118 @@ describe('#4020 — run-tests temp root', () => {
assert.ok(fs.existsSync(sibling),
'a nested runner never sweeps the shared root — sibling fixtures survive');
});
describe('#4220 — computeSweepProtectSet ancestor-walk termination', () => {
// The original inline block stopped the ancestor walk on
// `cur !== runTempRoot && cur.length > 1`, a POSIX-only sentinel:
// path.posix.dirname('/') === '/' (length 1) correctly stops, but
// path.win32.dirname('C:\\') === 'C:\\' (length 3) never satisfies a
// length check — dirname returns the SAME string forever, so the loop
// never terminates on Windows whenever a selected file lives outside
// runTempRoot (the common case: most selected test files are in the repo
// checkout, not the temp root). This bounds every assertion below with a
// hard iteration cap so a genuinely-hanging implementation fails FAST in
// this test run rather than wedging the suite.
const HARD_ITERATION_CAP = 10_000;
// A dirnameImpl wrapper that throws once it has been called more times
// than a real ancestor walk could ever need, turning an infinite loop
// into a fast, loud test failure instead of a hang.
function boundedDirname(realDirnameImpl) {
let calls = 0;
return (p) => {
calls++;
if (calls > HARD_ITERATION_CAP) {
throw new Error(
`computeSweepProtectSet: dirname called >${HARD_ITERATION_CAP} times — ` +
`the ancestor walk did not terminate (regression of #4220)`,
);
}
return realDirnameImpl(p);
};
}
test('terminates and protects ancestors on win32 paths outside runTempRoot', () => {
const runner = require('../scripts/run-tests.cjs');
assert.equal(typeof runner.computeSweepProtectSet, 'function',
'the runner must export computeSweepProtectSet for in-process verification');
const win32 = require('node:path').win32;
// A file living entirely outside runTempRoot (the common case — most
// selected test files are in the repo checkout), walked with the
// Windows dirname implementation whose drive-root is NOT length 1.
const selected = ['C:\\repo\\tests\\a\\b.test.cjs'];
const runTempRoot = 'C:\\Users\\ci\\AppData\\Local\\Temp\\gsd-test-run-xyz';
const protectSet = runner.computeSweepProtectSet(
selected, runTempRoot, boundedDirname(win32.dirname),
);
assert.ok(protectSet.has('C:\\repo\\tests\\a\\b.test.cjs'), 'the file itself is protected');
assert.ok(protectSet.has('C:\\repo\\tests\\a'), 'immediate parent is protected');
assert.ok(protectSet.has('C:\\repo\\tests'), 'grandparent is protected');
assert.ok(protectSet.has('C:\\repo'), 'walk reaches up to the drive-relative root');
// The walk must have stopped — it must NOT contain the drive root
// itself looping forever, and the protect set must be finite/small.
assert.ok(protectSet.size < 20, 'the walk terminates with a small, bounded protect set');
});
test('terminates and protects ancestors on win32 paths INSIDE runTempRoot (exact-file case)', () => {
const runner = require('../scripts/run-tests.cjs');
const win32 = require('node:path').win32;
const runTempRoot = 'C:\\Users\\ci\\AppData\\Local\\Temp\\gsd-test-run-xyz';
const selected = [`${runTempRoot}\\fixture\\nested\\c.test.cjs`];
const protectSet = runner.computeSweepProtectSet(
selected, runTempRoot, boundedDirname(win32.dirname),
);
assert.ok(protectSet.has(selected[0]), 'the file itself is protected');
assert.ok(protectSet.has(`${runTempRoot}\\fixture\\nested`), 'immediate parent is protected');
assert.ok(protectSet.has(`${runTempRoot}\\fixture`), 'grandparent is protected');
assert.ok(!protectSet.has(win32.dirname(runTempRoot)),
'the walk stops at runTempRoot and never protects its parent');
});
test('terminates and protects ancestors on posix paths (parity with win32)', () => {
const runner = require('../scripts/run-tests.cjs');
const posix = require('node:path').posix;
const selected = ['/repo/tests/a/b.test.cjs'];
const runTempRoot = '/synthetic-root/gsd-test-run-xyz';
const protectSet = runner.computeSweepProtectSet(
selected, runTempRoot, boundedDirname(posix.dirname),
);
assert.ok(protectSet.has('/repo/tests/a/b.test.cjs'), 'the file itself is protected');
assert.ok(protectSet.has('/repo/tests/a'), 'immediate parent is protected');
assert.ok(protectSet.has('/repo/tests'), 'grandparent is protected');
assert.ok(protectSet.has('/repo'), 'walk reaches up toward the root');
assert.ok(!protectSet.has('/'),
'the walk stops before adding the filesystem root itself, mirroring win32 behavior');
});
test('exact-file case on posix: selected file lies directly under runTempRoot', () => {
const runner = require('../scripts/run-tests.cjs');
const posix = require('node:path').posix;
const runTempRoot = '/synthetic-root/gsd-test-run-xyz';
const selected = [runTempRoot];
const protectSet = runner.computeSweepProtectSet(
selected, runTempRoot, boundedDirname(posix.dirname),
);
assert.ok(protectSet.has(runTempRoot), 'the exact-file case protects runTempRoot itself');
});
test('defaults dirnameImpl to the real platform path.dirname when not injected', () => {
const runner = require('../scripts/run-tests.cjs');
const path = require('node:path');
const os = require('node:os');
const runTempRoot = path.join(os.tmpdir(), 'gsd-test-run-default');
const selected = [path.join(runTempRoot, 'fixture', 'd.test.cjs')];
const protectSet = runner.computeSweepProtectSet(selected, runTempRoot);
assert.ok(protectSet.has(selected[0]), 'default dirnameImpl still protects the selected file');
assert.ok(protectSet.has(path.join(runTempRoot, 'fixture')), 'default dirnameImpl walks ancestors');
});
});
});