From 02a36d3db9c8d129ce6c96c8c73f2016f2d3107a Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 18 Aug 2026 22:08:52 -0400 Subject: [PATCH] fix(3618): update the Windows fallow assertions to the behavior #3618 chose (#3654) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit full test (windows-latest, 24, shard 1/3) has been RED on next since ac1b6d679 and blocks every PR. Three assertions fail, Windows-only, and all three are stale TESTS rather than resolver defects. Two of them create a BARE extensionless 'fallow' and assert it resolves. #3618 deliberately stopped resolving that on Windows, and said so in its own commit message: the extensionless file is npm's POSIX sh shim, which CreateProcess cannot run (#3275). The code did what it meant to; the tests were never updated to match. Both now assert BOTH platform contracts rather than skipping a lane — the precedent #3618 set when it fixed its own F4 ('a t.skip on one lane would have been green and would have left the win32 carve-out unpinned'). POSIX keeps the original bare+chmod fixture verbatim. win32 creates the artifact npm actually writes there, fallow.cmd, and asserts resolution finds it; each also pins that a bare extensionless fallow ALONE still resolves to null on win32, so the carve-out is pinned rather than merely stepped around. The precedence test keeps testing precedence: on win32 both node_modules/.bin and the PATH dir get a fallow.cmd, and .bin must still win. The third failure was case, not logic: the test wrote fallow.cmd and asserted exact string equality, but candidates are built by appending PATHEXT entries, and PATHEXT is uppercase. Confirmed at the source — DEFAULT_PATHEXT = '.EXE;.CMD;.BAT;.COM' (src/shell-command-projection.cts:651), read verbatim and never lowercased, with the candidate returned as-is. So the resolver returned fallow.CMD, which is the same file on case-insensitive NTFS. Fixed the assertion to compare case-insensitively on win32; the resolver is untouched, because its return value has to stay usable verbatim. Not verifiable here: this repo's remote runner matrix is Linux-only and the host is macOS, so the win32 branches are reasoned from the resolver source and proven only by GitHub CI. The POSIX lanes were verified by execution (bare+chmod in .bin resolves, .bin beats PATH, non-executable in PATH yields null). No changeset: changeset-lint's USER_FACING_PREFIXES does not include tests/, so this is OK_NO_USER_FACING_CHANGES rather than a missing fragment. Refs #3618 Co-authored-by: sim --- tests/config-schema.property.test.cjs | 108 ++++++++++++++++++++------ 1 file changed, 86 insertions(+), 22 deletions(-) diff --git a/tests/config-schema.property.test.cjs b/tests/config-schema.property.test.cjs index 9cf19da86..4efed9f2e 100644 --- a/tests/config-schema.property.test.cjs +++ b/tests/config-schema.property.test.cjs @@ -743,17 +743,56 @@ describe('feat-3210: fallow integration module', () => { const { resolveFallowBinary } = require('../gsd-core/bin/lib/fallow-runner.cjs'); // N2: use shared helper const baseTmp = getWritableTmp(); - const tmp = fs.mkdtempSync(path.join(baseTmp, 'gsd-fallow-bin-')); - const binDir = path.join(tmp, 'node_modules', '.bin'); - fs.mkdirSync(binDir, { recursive: true }); - const fallowPath = path.join(binDir, 'fallow'); - fs.writeFileSync(fallowPath, '#!/usr/bin/env sh\n'); - if (process.platform !== 'win32') fs.chmodSync(fallowPath, 0o755); - const resolved = resolveFallowBinary({ cwd: tmp, envPath: '' }); - assert.strictEqual(resolved, fallowPath); + if (process.platform === 'win32') { + // #3618/#3275: npm's Windows install drops `fallow.cmd` in node_modules/.bin + // (plus a bare extensionless POSIX `sh` shim beside it — npm's shim for + // non-Windows shells, which CreateProcess cannot execute; the seam + // deliberately never tries a bare name on win32). Assert BOTH contracts: + // the .cmd resolves, and the bare shim ALONE (no .cmd sibling) resolves + // to null, so the deliberate win32 carve-out stays pinned. + const tmp = fs.mkdtempSync(path.join(baseTmp, 'gsd-fallow-bin-')); + try { + const binDir = path.join(tmp, 'node_modules', '.bin'); + fs.mkdirSync(binDir, { recursive: true }); + const cmdPath = path.join(binDir, 'fallow.cmd'); + fs.writeFileSync(cmdPath, '@echo off\r\n'); - cleanup(tmp); + const resolved = resolveFallowBinary({ cwd: tmp, envPath: '' }); + // Case-insensitive: PATHEXT casing (ambient env or DEFAULT_PATHEXT) is + // conventionally uppercase, so the resolver constructs `fallow.CMD` — + // the same file as `fallow.cmd` on case-insensitive NTFS, just a + // different string. + assert.strictEqual(String(resolved).toLowerCase(), cmdPath.toLowerCase()); + } finally { + cleanup(tmp); + } + + const tmpBare = fs.mkdtempSync(path.join(baseTmp, 'gsd-fallow-bin-bare-')); + try { + const binDir = path.join(tmpBare, 'node_modules', '.bin'); + fs.mkdirSync(binDir, { recursive: true }); + const bareOnly = path.join(binDir, 'fallow'); + fs.writeFileSync(bareOnly, '#!/usr/bin/env sh\n'); + + const resolvedBare = resolveFallowBinary({ cwd: tmpBare, envPath: '' }); + assert.strictEqual(resolvedBare, null, 'win32: bare extensionless fallow alone must not resolve (#3275)'); + } finally { + cleanup(tmpBare); + } + } else { + const tmp = fs.mkdtempSync(path.join(baseTmp, 'gsd-fallow-bin-')); + const binDir = path.join(tmp, 'node_modules', '.bin'); + fs.mkdirSync(binDir, { recursive: true }); + const fallowPath = path.join(binDir, 'fallow'); + fs.writeFileSync(fallowPath, '#!/usr/bin/env sh\n'); + fs.chmodSync(fallowPath, 0o755); + + const resolved = resolveFallowBinary({ cwd: tmp, envPath: '' }); + assert.strictEqual(resolved, fallowPath); + + cleanup(tmp); + } }); // H6: replaced wholesale win32 skip with platform-adapted assertion @@ -773,9 +812,14 @@ describe('feat-3210: fallow integration module', () => { fs.writeFileSync(bareFile, '@echo off\r\n'); fs.writeFileSync(cmdFile, '@echo off\r\n'); const resolved = resolveFallowBinary({ cwd: tmp, envPath: pathDir }); + // Case-insensitive: PATHEXT casing (ambient env or DEFAULT_PATHEXT) is + // conventionally uppercase, so the resolver constructs `fallow.CMD` — + // the same file as `fallow.cmd` on case-insensitive NTFS, just a + // different string. Comparing exact case here would be over-specifying + // a filesystem-level identity as a string identity. assert.strictEqual( - resolved, - cmdFile, + String(resolved).toLowerCase(), + cmdFile.toLowerCase(), 'Windows: .cmd candidate must be preferred over bare extensionless file', ); } finally { @@ -915,22 +959,42 @@ describe('feat-3210: M2 - node_modules/.bin resolution order', () => { const baseTmp = getWritableTmp(); const tmp = fs.mkdtempSync(path.join(baseTmp, 'gsd-fallow-order-')); try { - // local node_modules/.bin/fallow const binDir = path.join(tmp, 'node_modules', '.bin'); fs.mkdirSync(binDir, { recursive: true }); - const localFallow = path.join(binDir, 'fallow'); - fs.writeFileSync(localFallow, '#!/usr/bin/env sh\necho local\n'); - if (process.platform !== 'win32') fs.chmodSync(localFallow, 0o755); - - // PATH fallow (a different file) const pathDir = path.join(tmp, 'pathbin'); fs.mkdirSync(pathDir, { recursive: true }); - const pathFallow = path.join(pathDir, 'fallow'); - fs.writeFileSync(pathFallow, '#!/usr/bin/env sh\necho path\n'); - if (process.platform !== 'win32') fs.chmodSync(pathFallow, 0o755); - const resolved = resolveFallowBinary({ cwd: tmp, envPath: pathDir }); - assert.strictEqual(resolved, localFallow, 'node_modules/.bin/fallow must win over PATH fallow'); + if (process.platform === 'win32') { + // #3618/#3275: exercise precedence against the real Windows candidate + // shape (npm's `fallow.cmd`), not the bare extensionless POSIX shim the + // resolver deliberately no longer matches on win32. + const localFallow = path.join(binDir, 'fallow.cmd'); + fs.writeFileSync(localFallow, '@echo off\r\necho local\r\n'); + const pathFallow = path.join(pathDir, 'fallow.cmd'); + fs.writeFileSync(pathFallow, '@echo off\r\necho path\r\n'); + + const resolved = resolveFallowBinary({ cwd: tmp, envPath: pathDir }); + // Case-insensitive for the same PATHEXT-casing reason as the sibling + // tests above (resolver constructs `fallow.CMD`, not `fallow.cmd`). + assert.strictEqual( + String(resolved).toLowerCase(), + localFallow.toLowerCase(), + 'node_modules/.bin/fallow.cmd must win over PATH fallow.cmd', + ); + } else { + // local node_modules/.bin/fallow + const localFallow = path.join(binDir, 'fallow'); + fs.writeFileSync(localFallow, '#!/usr/bin/env sh\necho local\n'); + fs.chmodSync(localFallow, 0o755); + + // PATH fallow (a different file) + const pathFallow = path.join(pathDir, 'fallow'); + fs.writeFileSync(pathFallow, '#!/usr/bin/env sh\necho path\n'); + fs.chmodSync(pathFallow, 0o755); + + const resolved = resolveFallowBinary({ cwd: tmp, envPath: pathDir }); + assert.strictEqual(resolved, localFallow, 'node_modules/.bin/fallow must win over PATH fallow'); + } } finally { cleanup(tmp); }