* fix(#2962): write npm-style gsd-sdk shim on Windows under --sdk install trySelfLinkGsdSdk previously contained `if (process.platform === 'win32') return null;` — a missed gap from #2775's POSIX self-link rather than an intentional design choice. As a result, `npx get-shit-done-cc@latest --claude --global --sdk` on Windows left `gsd-sdk` off PATH despite the installer reporting success, and the obvious recovery (`npm i -g @gsd-build/sdk`) lands the stale 0.1.0 publication that lacks the `query` subcommand the agents call ~40 times. This PR addresses the shim half. The npm-publish half (publishing @gsd-build/sdk at parity with the get-shit-done-cc version) requires maintainer credentials and is left for separate action. Changes: - bin/install.js: replace the unconditional Windows return-null with dispatch to a new trySelfLinkGsdSdkWindows() that: * resolves npm's global bin via `execFileSync('npm', ['prefix', '-g'])` (no shell interpolation; npm is the only PATH-resolved binary) * verifies write access with a probe before producing partial state * writes the standard npm shim triple to npm's global bin: - gsd-sdk.cmd (cmd.exe; CRLF line endings) - gsd-sdk.ps1 (PowerShell) - gsd-sdk (Bash wrapper for Cygwin/MSYS/Git-Bash) * each shim invokes `node "<absolute path to bin/gsd-sdk.js>"` with the passed args, decoupling shim location from SDK location — same logical structure as the POSIX wrapper-via-require() fallback above * unlinks any stale shims before writing so prior installs don't pin callers to a now-absent path * returns the .cmd path on success (handle the existing onPath check looks for) or null on any failure, falling through to the existing "gsd-sdk is not on your PATH" warning at line 8704 - tests/bug-2962-windows-sdk-shim.test.cjs (new): 5 tests exercising trySelfLinkGsdSdkWindows directly with cp.execFileSync mocked to redirect npm prefix to a temp dir. Asserts shim contents reference the absolute path, .cmd uses CRLF, stale shims are replaced not appended, and null is returned when `npm prefix -g` fails. - tests/no-unconditional-win32-skip.test.cjs (new): regression guard that fails CI if any future commit re-introduces `if (process.platform === 'win32') return null;` (or similar skip-only branches) in bin/install.js. Negative test verified by transiently re-introducing the bad pattern → guard fired → restored → passes. Out of scope: publishing @gsd-build/sdk@<current> to npm so the natural `npm i -g @gsd-build/sdk` recovery also lands a usable SDK. That requires maintainer credentials and is the second half of the issue. Closes #2962 * fix(#2962): address CodeRabbit findings — execSync for npm.cmd, behavior-based regression guard CR finding 1 (🟠 Major): Node's child_process docs explicitly call out that .cmd/.bat files cannot be spawned via execFile/execFileSync without a shell ("Spawning .bat and .cmd files on Windows" section). Since `npm` on Windows is `npm.cmd`, my use of execFileSync('npm', ['prefix', '-g'], { shell: false }) would have failed on the very platform this PR is meant to fix. Switched to cp.execSync('npm prefix -g', ...) — matching the existing convention at line ~8718 which makes the same lookup. Args are static literals so shell interpolation is not an injection vector. CR finding 2 (🟠 Major): the source-grep regression test in tests/no-unconditional-win32-skip.test.cjs violated the repo's no-source-grep testing standard (CONTRIBUTING.md). Replaced with a behavior-based test that: - overrides process.platform to 'win32' via Object.defineProperty - mocks cp.execSync to return a temp-dir as npm prefix - calls trySelfLinkGsdSdk(shimSrc) and asserts it returns non-null AND materializes gsd-sdk.cmd on disk The behavior guard is strictly stronger than the regex version: it would catch any equivalent skip pattern (e.g. os.platform() === 'win32', a typeof-based guard, etc.), not just literal `if (process.platform === 'win32')` text. Negative-tested by re-introducing the `return null` skip → test fails with maintainer-quoted diagnostic "trySelfLinkGsdSdk must not silently return null on Windows; a no-op skip is a missed-parity regression"; restored → passes. Test for Windows shim materialization (bug-2962-windows-sdk-shim.test.cjs) also updated to mock cp.execSync (matching the new production code path) instead of cp.execFileSync. Full suite: 6480/6480 pass. * test(#2962): make Windows shim tests self-contained per CR Each test now invokes trySelfLinkGsdSdkWindows() itself before reading the shim files, so they don't implicitly depend on the earlier test's side effects. Addresses CR's order-dependence finding. * test(#2962): structured shim parsing — eliminate substring source-grep CR found that even after the prior refactor, three tests in the suite still used .includes()/.startsWith() against shim file content (cmdContent.includes(\`@node ${jsonQuoted} %*\`) etc.). Substring matching on file text is the same anti-pattern the no-source-grep standard forbids — even when the file is one this test wrote — because it asserts a literal exists rather than that the structured shape is correct. Replace with three small parsers (parseCmdShim, parsePs1Invocation, parseBashInvocation) that split each shim into header + invocation tokens and assert via deepEqual on structured records. The assertions now check that the .cmd has @ECHO OFF / @SETLOCAL / @node <abs> %* in that order with exactly 3 meaningful lines, and that the .ps1 and bash wrappers split into the expected (call, nodeCmd, target, argToken) tuples. The stale-shim replacement test was hardened the same way: instead of proving the absence of a sentinel substring, it now proves the parsed target equals the new shimSrc and != the old path. Verified: scripts/lint-no-source-grep.cjs reports 0 violations across 348 test files. The 6-test windows-sdk-shim + win32-skip-guard suite all pass. * fix(#2962): expose pure shim IR + tests assert on typed fields, not rendered text Earlier "structured parser" approach (parseCmdShim / parsePs1Invocation / parseBashInvocation) was still raw-text manipulation behind a function wrapper — split('\\r\\n'), trim().split(/\\s+/), content.includes('\\r\\n'). Maintainer was right: hiding grep behind a parser is still grep. Real fix: refactor production code to expose the structured intermediate representation, and have tests assert on the IR fields directly. Production: - New buildWindowsShimTriple(shimSrc) — pure function, no fs/spawn. Returns { invocation: { interpreter, target }, eol: { cmd, ps1, sh }, fileNames: { cmd, ps1, sh }, render: { cmd: () => string, ... } }. The IR is the contract; rendered text is an implementation detail of the renderers. - trySelfLinkGsdSdkWindows now calls buildWindowsShimTriple, looks up filenames from triple.fileNames, and writes triple.render[kind]() to each target. Same observable behavior, structurally separated. - buildWindowsShimTriple added to test-mode exports. Tests (full rewrite — no shim file content is read at any point): - Layer 1: pure-IR tests assert on triple.invocation.target, triple.eol === { cmd: '\\r\\n', ps1: '\\n', sh: '\\n' }, triple.fileNames === { cmd: 'gsd-sdk.cmd', ... }, and the documented IR shape via Object.keys().sort() deepEqual. - Layer 2: fs/spawn driver tests assert filesystem FACTS: - return value equals expected path - all three target files exist as regular non-empty files - rendered file byte length === Buffer.byteLength of triple.render(kind) output (proves the writer writes what the renderer produces, no mutation, no truncation, no double-write — without comparing content) - mtime advances on rewrite (proves stale-replace behavior) - returns null when npm prefix -g throws No more split, .includes, .startsWith, .endsWith, or substring matching anywhere in the test suite. Lint clean. 10/10 tests pass.
151 lines
6.6 KiB
JavaScript
151 lines
6.6 KiB
JavaScript
'use strict';
|
|
|
|
process.env.GSD_TEST_MODE = '1';
|
|
|
|
/**
|
|
* Bug #2962: --sdk install flag on Windows leaves gsd-sdk un-shimmed.
|
|
*
|
|
* Tests are split into two layers, each at the right level of abstraction:
|
|
*
|
|
* 1. buildWindowsShimTriple — pure IR builder. Tests assert on TYPED
|
|
* FIELDS of the returned record (interpreter, target, eol, fileNames).
|
|
* No filesystem, no spawn, no text reads. This is the level where
|
|
* structural correctness lives.
|
|
*
|
|
* 2. trySelfLinkGsdSdkWindows — fs/spawn driver that calls the IR builder
|
|
* and writes the rendered shims to disk. Tests assert FILESYSTEM FACTS
|
|
* (file exists, file is non-empty, file mtime advanced after replace,
|
|
* function return value). No reads, no parsing, no substring matching.
|
|
*
|
|
* Per the repo's no-source-grep testing standard (CONTRIBUTING.md): the
|
|
* test must NEVER read shim file contents and pattern-match against them.
|
|
* The IR is the contract; the rendered text is an implementation detail of
|
|
* the renderer.
|
|
*/
|
|
|
|
const { test, describe, before, after } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const fs = require('node:fs');
|
|
const os = require('node:os');
|
|
const path = require('node:path');
|
|
const cp = require('node:child_process');
|
|
|
|
const ROOT = path.join(__dirname, '..');
|
|
const installModule = require(path.join(ROOT, 'bin', 'install.js'));
|
|
|
|
describe('Bug #2962: buildWindowsShimTriple — pure IR builder', () => {
|
|
test('resolves shimSrc to an absolute path on the invocation.target field', () => {
|
|
const shimSrc = path.join(ROOT, 'bin', 'gsd-sdk.js');
|
|
const triple = installModule.buildWindowsShimTriple(shimSrc);
|
|
assert.equal(triple.invocation.target, path.resolve(shimSrc));
|
|
assert.equal(triple.invocation.interpreter, 'node');
|
|
});
|
|
|
|
test('produces a structured IR with the documented shape', () => {
|
|
const triple = installModule.buildWindowsShimTriple(path.join(ROOT, 'bin', 'gsd-sdk.js'));
|
|
// Lock the public IR shape — adding/removing a key requires updating this assertion.
|
|
assert.deepEqual(Object.keys(triple).sort(), ['eol', 'fileNames', 'invocation', 'render']);
|
|
assert.deepEqual(Object.keys(triple.invocation).sort(), ['interpreter', 'target']);
|
|
assert.deepEqual(Object.keys(triple.eol).sort(), ['cmd', 'ps1', 'sh']);
|
|
assert.deepEqual(Object.keys(triple.fileNames).sort(), ['cmd', 'ps1', 'sh']);
|
|
assert.deepEqual(Object.keys(triple.render).sort(), ['cmd', 'ps1', 'sh']);
|
|
});
|
|
|
|
test('declares CRLF line endings on the .cmd file, LF on .ps1 and bash wrapper', () => {
|
|
const triple = installModule.buildWindowsShimTriple(path.join(ROOT, 'bin', 'gsd-sdk.js'));
|
|
assert.deepEqual(triple.eol, { cmd: '\r\n', ps1: '\n', sh: '\n' });
|
|
});
|
|
|
|
test('declares the standard npm-style filenames for the shim triple', () => {
|
|
const triple = installModule.buildWindowsShimTriple(path.join(ROOT, 'bin', 'gsd-sdk.js'));
|
|
assert.deepEqual(triple.fileNames, { cmd: 'gsd-sdk.cmd', ps1: 'gsd-sdk.ps1', sh: 'gsd-sdk' });
|
|
});
|
|
|
|
test('IR is purely a function of shimSrc — no fs / spawn side effects', () => {
|
|
// If buildWindowsShimTriple touched the filesystem, calling it twice with
|
|
// different shimSrc paths would leave two different artifacts. Asserting
|
|
// pure-function behavior structurally: same input → identical IR.
|
|
const shimSrc = path.join(ROOT, 'bin', 'gsd-sdk.js');
|
|
const a = installModule.buildWindowsShimTriple(shimSrc);
|
|
const b = installModule.buildWindowsShimTriple(shimSrc);
|
|
assert.deepEqual(a.invocation, b.invocation);
|
|
assert.deepEqual(a.eol, b.eol);
|
|
assert.deepEqual(a.fileNames, b.fileNames);
|
|
});
|
|
});
|
|
|
|
describe('Bug #2962: trySelfLinkGsdSdkWindows — fs/spawn driver', () => {
|
|
let tmpDir;
|
|
let origExecSync;
|
|
|
|
before(() => {
|
|
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2962-'));
|
|
origExecSync = cp.execSync;
|
|
cp.execSync = (cmd) => {
|
|
if (typeof cmd === 'string' && cmd.trim() === 'npm prefix -g') {
|
|
return tmpDir + '\n';
|
|
}
|
|
return origExecSync.call(cp, cmd);
|
|
};
|
|
});
|
|
|
|
after(() => {
|
|
cp.execSync = origExecSync;
|
|
fs.rmSync(tmpDir, { recursive: true, force: true });
|
|
});
|
|
|
|
test('returns the .cmd path on success and writes all three shim files', () => {
|
|
const shimSrc = path.join(ROOT, 'bin', 'gsd-sdk.js');
|
|
const triple = installModule.buildWindowsShimTriple(shimSrc);
|
|
const result = installModule.trySelfLinkGsdSdkWindows(shimSrc);
|
|
|
|
assert.equal(result, path.join(tmpDir, triple.fileNames.cmd));
|
|
for (const fileName of Object.values(triple.fileNames)) {
|
|
const target = path.join(tmpDir, fileName);
|
|
const stat = fs.statSync(target);
|
|
assert.ok(stat.isFile(), `${fileName} must be a regular file`);
|
|
assert.ok(stat.size > 0, `${fileName} must be non-empty`);
|
|
}
|
|
});
|
|
|
|
test('the rendered file size matches the IR renderer\'s output length (renderer drives the writer)', () => {
|
|
// Asserts the writer writes exactly what the renderer produces — no mutation,
|
|
// no double-write, no truncation. We compare BYTE LENGTHS, not contents:
|
|
// length is a structural property; content equality would re-introduce text matching.
|
|
const shimSrc = path.join(ROOT, 'bin', 'gsd-sdk.js');
|
|
const triple = installModule.buildWindowsShimTriple(shimSrc);
|
|
installModule.trySelfLinkGsdSdkWindows(shimSrc);
|
|
for (const kind of ['cmd', 'ps1', 'sh']) {
|
|
const target = path.join(tmpDir, triple.fileNames[kind]);
|
|
const expected = Buffer.byteLength(triple.render[kind](), 'utf8');
|
|
assert.equal(fs.statSync(target).size, expected, `${kind} byte length matches renderer`);
|
|
}
|
|
});
|
|
|
|
test('replaces stale shims atomically (mtime advances on rewrite)', () => {
|
|
const shimSrc = path.join(ROOT, 'bin', 'gsd-sdk.js');
|
|
installModule.trySelfLinkGsdSdkWindows(shimSrc);
|
|
const cmdPath = path.join(tmpDir, 'gsd-sdk.cmd');
|
|
const beforeMtime = fs.statSync(cmdPath).mtimeMs;
|
|
|
|
// Wait at least 10ms so mtime granularity (1ms on most fs, 1s on some) records the change.
|
|
const wait = Date.now() + 20;
|
|
while (Date.now() < wait) { /* busy-wait, intentional */ }
|
|
|
|
installModule.trySelfLinkGsdSdkWindows(shimSrc);
|
|
const afterMtime = fs.statSync(cmdPath).mtimeMs;
|
|
assert.ok(afterMtime > beforeMtime, `mtime must advance: before=${beforeMtime} after=${afterMtime}`);
|
|
});
|
|
|
|
test('returns null when npm prefix -g fails', () => {
|
|
const restore = cp.execSync;
|
|
cp.execSync = () => { throw new Error('npm not on PATH'); };
|
|
try {
|
|
const result = installModule.trySelfLinkGsdSdkWindows(path.join(ROOT, 'bin', 'gsd-sdk.js'));
|
|
assert.equal(result, null);
|
|
} finally {
|
|
cp.execSync = restore;
|
|
}
|
|
});
|
|
});
|