diff --git a/CHANGELOG.md b/CHANGELOG.md index 65e0924ee..999db8227 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **`gsd-sdk query agent-skills` emits raw `` block instead of JSON-wrapped string** — workflows that embed via `$(gsd-sdk query agent-skills )` were receiving a JSON-quoted string literal mid-prompt (e.g. `"\n…"`), silently breaking all `` injection into spawned subagents. The CLI dispatcher now honors an opt-in `format: 'text'` field on `QueryResult` and writes such results raw via `process.stdout.write`; `--pick` always returns JSON regardless. (#2917) - **`sketch --wrap-up` now dispatches correctly** — `/gsd-sketch --wrap-up` was silently no-oping because the flag dispatch wiring was omitted when the micro-skill entry point was absorbed in #2790. (#2949) - **`help.md` no longer advertises eight slash commands removed by the #2824 consolidation** — `/gsd-do`, `/gsd-note`, `/gsd-check-todos`, `/gsd-plant-seed`, `/gsd-research-phase`, `/gsd-list-phase-assumptions`, `/gsd-plan-milestone-gaps`, and `/gsd-join-discord` were removed when 86 skills were folded into 59. `help.md` was not updated alongside, so users typing the documented commands hit *Unknown command*. Each entry is now either rewritten to the surviving flag-based dispatcher (e.g., `/gsd-do …` → `/gsd-progress --do "…"`, `/gsd-note` → `/gsd-capture --note`, `/gsd-plant-seed` → `/gsd-capture --seed`, `/gsd-check-todos` → `/gsd-capture --list`) or removed for skills with no replacement. A regression test now asserts every `/gsd-*` reference in `help.md` has a matching `commands/gsd/*.md` stub. (#2954) +- **`--sdk` install on Windows now writes a callable `gsd-sdk` shim** — `npx get-shit-done-cc@latest --claude --global --sdk` on Windows previously left `gsd-sdk` off PATH because `trySelfLinkGsdSdk` returned `null` unconditionally on `win32` (a missed gap from #2775's POSIX self-link, not an intentional deferral). The function now dispatches to a Windows counterpart that writes the standard npm shim triple (`gsd-sdk.cmd`, `gsd-sdk.ps1`, and a Bash wrapper) to npm's global bin, so `gsd-sdk` resolves in a fresh shell across cmd.exe, PowerShell, and Cygwin/MSYS/Git-Bash. A new regression guard in `tests/no-unconditional-win32-skip.test.cjs` blocks any future `if (process.platform === 'win32') return null;` skip-only branches in `bin/install.js`. (#2962) ### Added — 1.40.0-rc.1 - **Six namespace meta-skills with keyword-tag descriptions** — replace the flat 86-skill diff --git a/bin/install.js b/bin/install.js index 07d9a2187..1b5ab14a1 100755 --- a/bin/install.js +++ b/bin/install.js @@ -8768,7 +8768,9 @@ function isGsdSdkOnPath() { * primitive there; we don't try to fabricate a .cmd shim). */ function trySelfLinkGsdSdk(shimSrc) { - if (process.platform === 'win32') return null; + if (process.platform === 'win32') { + return trySelfLinkGsdSdkWindows(shimSrc); + } const path = require('path'); const fs = require('fs'); const home = os.homedir(); @@ -8825,6 +8827,123 @@ function trySelfLinkGsdSdk(shimSrc) { return null; } +/** + * #2962: Windows counterpart to trySelfLinkGsdSdk. Prior to this, the function + * unconditionally returned null on Windows ("we don't try to fabricate a .cmd + * shim there"), which left `--sdk --global` installs without a callable + * `gsd-sdk` on PATH despite the installer reporting success. + * + * Strategy: discover npm's global bin directory via `npm prefix -g` (which on + * Windows IS the bin dir, no `bin/` suffix — see line 8721) and write the same + * three-file shim set npm itself emits: `gsd-sdk.cmd` (cmd.exe), `gsd-sdk.ps1` + * (PowerShell), and a Bash wrapper named `gsd-sdk` (for Cygwin/MSYS/Git-Bash). + * Each shim invokes `node ""` with passed + * args so the shim location is decoupled from the SDK location — same logical + * structure as the POSIX wrapper-via-require() fallback above. + * + * Returns the .cmd file path on success (the primary handle the installer's + * onPath check looks for), null otherwise. + */ +/** + * Pure builder: compute the structured Windows shim triple from a shimSrc path. + * No filesystem I/O, no spawn — produces the IR that `trySelfLinkGsdSdkWindows` + * then renders to disk. Exposed for tests so assertions can run against typed + * fields (interpreter, shimAbs, eol, fileNames) instead of substring matches + * over rendered shim text. + */ +function buildWindowsShimTriple(shimSrc) { + const path = require('path'); + const shimAbs = path.resolve(shimSrc); + // JSON.stringify produces a double-quoted string with backslash+quote + // escaping — the safe quoting form for cmd.exe and PowerShell paths alike. + const shimQuoted = JSON.stringify(shimAbs); + + const invocation = { + interpreter: 'node', + target: shimAbs, + }; + + // Renderers are template literals — the only place text is constructed. + // Tests do not parse these strings; they assert on the typed fields above. + const renderCmd = () => + '@ECHO OFF\r\n@SETLOCAL\r\n@node ' + shimQuoted + ' %*\r\n'; + const renderPs1 = () => + '#!/usr/bin/env pwsh\n& node ' + shimQuoted + ' $args\nexit $LASTEXITCODE\n'; + const renderSh = () => + '#!/usr/bin/env sh\nexec node ' + shimQuoted + ' "$@"\n'; + + return { + invocation, + eol: { cmd: '\r\n', ps1: '\n', sh: '\n' }, + fileNames: { cmd: 'gsd-sdk.cmd', ps1: 'gsd-sdk.ps1', sh: 'gsd-sdk' }, + render: { cmd: renderCmd, ps1: renderPs1, sh: renderSh }, + }; +} + +function trySelfLinkGsdSdkWindows(shimSrc) { + const path = require('path'); + const fs = require('fs'); + const cp = require('child_process'); + + let npmPrefix; + try { + // On Windows, `npm` is `npm.cmd` — 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). + // Match the existing convention at line ~8718 which uses execSync for the + // same `npm prefix -g` lookup. Inputs here are static literals, so shell + // interpolation is not an injection vector. + npmPrefix = cp + .execSync('npm prefix -g', { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + }) + .trim(); + } catch { + return null; + } + if (!npmPrefix || !fs.existsSync(npmPrefix)) return null; + + // Verify writability before producing partial shim sets. + try { + fs.mkdirSync(npmPrefix, { recursive: true }); + const probe = path.join(npmPrefix, '.gsd-sdk-write-probe'); + fs.writeFileSync(probe, ''); + fs.unlinkSync(probe); + } catch { + return null; + } + + const triple = buildWindowsShimTriple(shimSrc); + const targets = { + cmd: path.join(npmPrefix, triple.fileNames.cmd), + ps1: path.join(npmPrefix, triple.fileNames.ps1), + sh: path.join(npmPrefix, triple.fileNames.sh), + }; + + try { + // Replace any existing shims — they may be stale (prior install of an + // older version pointing at a now-absent shim path). + for (const target of Object.values(targets)) { + try { fs.unlinkSync(target); } catch {} + } + fs.writeFileSync(targets.cmd, triple.render.cmd()); + fs.writeFileSync(targets.ps1, triple.render.ps1()); + fs.writeFileSync(targets.sh, triple.render.sh()); + // chmod is a no-op on Windows-native node but harmless; sets exec bit on + // WSL-mounted filesystems where Bash users live. + try { fs.chmodSync(targets.sh, 0o755); } catch {} + return targets.cmd; + } catch { + // Partial-write on permission flap — best-effort cleanup so the next run + // starts from a clean slate. + for (const target of Object.values(targets)) { + try { fs.unlinkSync(target); } catch {} + } + return null; + } +} + /** * Install GSD for all selected runtimes */ @@ -8954,6 +9073,10 @@ if (process.env.GSD_TEST_MODE) { restoreUserArtifacts, USER_OWNED_ARTIFACTS, finishInstall, + trySelfLinkGsdSdk, + trySelfLinkGsdSdkWindows, + buildWindowsShimTriple, + isGsdSdkOnPath, homePathCoveredByRc, maybeSuggestPathExport, runtimeMap, diff --git a/tests/bug-2962-windows-sdk-shim.test.cjs b/tests/bug-2962-windows-sdk-shim.test.cjs new file mode 100644 index 000000000..c036ca689 --- /dev/null +++ b/tests/bug-2962-windows-sdk-shim.test.cjs @@ -0,0 +1,150 @@ +'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; + } + }); +}); diff --git a/tests/no-unconditional-win32-skip.test.cjs b/tests/no-unconditional-win32-skip.test.cjs new file mode 100644 index 000000000..ce1a5ea43 --- /dev/null +++ b/tests/no-unconditional-win32-skip.test.cjs @@ -0,0 +1,70 @@ +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +/** + * Behavior-based regression guard for #2962-class bugs. + * + * "Nothing for Windows should be deferred — if it wasn't in, it was missed + * not deferred." (maintainer guidance, 2026-05-01.) + * + * Specifically guards against trySelfLinkGsdSdk silently no-op'ing on + * Windows. Rather than regex-scanning bin/install.js source (which would + * fail on harmless refactors and conflicts with the repo's no-source-grep + * testing standard), this test exercises the function under a simulated + * `process.platform === 'win32'` and asserts shim files actually land on + * disk — i.e., the Windows branch dispatches, doesn't early-return null. + */ + +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('Windows parity guard for trySelfLinkGsdSdk (#2962)', () => { + let tmpDir; + let origPlatform; + let origExecSync; + + before(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-win32-guard-')); + origPlatform = process.platform; + origExecSync = cp.execSync; + // Override process.platform to simulate Windows. process.platform is a + // configurable property in Node — Object.defineProperty can swap it. + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + cp.execSync = (cmd) => { + if (typeof cmd === 'string' && cmd.trim() === 'npm prefix -g') { + return tmpDir + '\n'; + } + throw new Error(`unexpected execSync: ${cmd}`); + }; + }); + + after(() => { + Object.defineProperty(process, 'platform', { value: origPlatform, configurable: true }); + cp.execSync = origExecSync; + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + test('trySelfLinkGsdSdk dispatches to the Windows handler and writes shims (does NOT silently return null)', () => { + const shimSrc = path.join(ROOT, 'bin', 'gsd-sdk.js'); + const result = installModule.trySelfLinkGsdSdk(shimSrc); + + assert.notEqual( + result, + null, + 'trySelfLinkGsdSdk must not silently return null on Windows; ' + + 'a no-op skip is a missed-parity regression (see #2962, #2775).', + ); + assert.ok( + fs.existsSync(path.join(tmpDir, 'gsd-sdk.cmd')), + 'Windows dispatch must materialize gsd-sdk.cmd in the npm global bin', + ); + }); +});