diff --git a/.changeset/fix-3406-detect-stale-sdk-shadow.md b/.changeset/fix-3406-detect-stale-sdk-shadow.md new file mode 100644 index 000000000..989a9fbd7 --- /dev/null +++ b/.changeset/fix-3406-detect-stale-sdk-shadow.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3641 +--- +**Install-time warning when a stale `@gsd-build/sdk` shadows the bundled `gsd-sdk` shim** — global installs now run `npm ls -g @gsd-build/sdk` and, if the standalone 0.1.0 package is present (it never received `query` subcommand support), print a clear remediation block before the install completes. Detection is fail-closed: any npm/exec error silently returns no-stale. Gated by `GSD_SKIP_STALE_SDK_CHECK=1` for CI/test environments. Resolves #3406. diff --git a/bin/install.js b/bin/install.js index 9c4d5107f..f1ea21d91 100755 --- a/bin/install.js +++ b/bin/install.js @@ -7855,6 +7855,45 @@ function install(isGlobal, runtime = 'claude', options = {}) { ? targetDir.replace(os.homedir(), '~') : targetDir.replace(process.cwd(), '.'); + // #3406: warn if a stale standalone `@gsd-build/sdk` is globally installed + // and shadows the `gsd-sdk` shim this installer wires up. Only meaningful + // for global installs (the shim collision lives in the global node_modules + // bin dir). Guarded by GSD_SKIP_STALE_SDK_CHECK so CI/tests can silence it. + // #3406 CR: opt-out only on explicit "1" / "true" / "yes" rather than any + // non-empty value. Without this guard `GSD_SKIP_STALE_SDK_CHECK=0` and + // `GSD_SKIP_STALE_SDK_CHECK=false` would silently disable the check. + const skipRaw = process.env.GSD_SKIP_STALE_SDK_CHECK; + const skipStaleCheck = skipRaw === '1' || skipRaw === 'true' || skipRaw === 'yes'; + if (isGlobal && !skipStaleCheck) { + try { + const { execFileSync } = require('child_process'); + const npmCmd = process.platform === 'win32' ? 'npm.cmd' : 'npm'; + const staleInfo = detectStaleStandaloneSdk(() => { + try { + return execFileSync( + npmCmd, + ['ls', '-g', '@gsd-build/sdk', '--json', '--depth=0'], + { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'ignore'], timeout: 10_000 } + ); + } catch (e) { + // `npm ls -g ` exits 1 with the JSON still on stdout when + // the package is absent. execFileSync throws on non-zero exit but + // attaches stdout to the error. Recover the JSON in that case so + // the detector classifies "absent" correctly. + if (e && typeof e.stdout !== 'undefined') { + return Buffer.isBuffer(e.stdout) ? e.stdout.toString('utf-8') : String(e.stdout); + } + throw e; + } + }); + if (staleInfo.stale) { + console.warn(`\n${yellow}${formatStaleStandaloneSdkWarning(staleInfo)}${reset}\n`); + } + } catch { + // Detection is best-effort; never block install on its failure. + } + } + // Path prefix for file references in markdown content (e.g. gsd-tools.cjs). // Replaces $HOME/.claude/ or ~/.claude/ so the result is get-shit-done/bin/... // For global installs: use $HOME/ so paths expand correctly inside double-quoted @@ -10597,6 +10636,80 @@ function installSdkIfNeeded(opts) { } } +/** + * #3406 helper: detect a stale globally-installed `@gsd-build/sdk` package + * shadowing the `gsd-sdk` shim that `get-shit-done-cc` installs. + * + * Background: `@gsd-build/sdk@0.1.0` was published once and never updated + * (the SDK now ships embedded in `get-shit-done-cc`). When a user has the + * 0.1.0 standalone package installed globally, its `gsd-sdk` bin shadows + * the one `get-shit-done-cc` provides — and the 0.1.0 binary only knows + * `run | auto | init` (no `query`), so every `gsd-sdk query ` + * call from skills/hooks fails until the user runs + * `npm uninstall -g @gsd-build/sdk`. + * + * Pure function: takes an injected `runNpmLs` executor that returns + * `npm ls -g @gsd-build/sdk --json --depth=0` stdout. Returns: + * `{ stale: true, version }` when the package is present. + * `{ stale: false }` for every other input — including: + * - executor throws (npm missing / EACCES / network), + * - executor returns null/undefined/non-string, + * - stdout is not parseable JSON, + * - the JSON has no `.dependencies['@gsd-build/sdk']` field. + * + * Fail-closed conservative: we'd rather miss a detection than fire a + * false-positive warning that confuses users who have a fine install. + */ +function detectStaleStandaloneSdk(runNpmLs) { + if (typeof runNpmLs !== 'function') return { stale: false }; + let out; + try { + out = runNpmLs(); + } catch { + return { stale: false }; + } + if (typeof out !== 'string' || out.length === 0) return { stale: false }; + let parsed; + try { + parsed = JSON.parse(out); + } catch { + return { stale: false }; + } + const deps = parsed && typeof parsed === 'object' ? parsed.dependencies : null; + if (!deps || typeof deps !== 'object') return { stale: false }; + const entry = deps['@gsd-build/sdk']; + if (!entry || typeof entry !== 'object') return { stale: false }; + const version = typeof entry.version === 'string' ? entry.version : '(unknown)'; + // #3406 CR: scope stale detection to the known-bad version (0.1.0). Any + // newer @gsd-build/sdk version is an intentional install (or a future + // republish) and should not be flagged as a shim shadow. Without this + // narrowing, a maintainer's local-link or a legitimate future publish + // would trigger a misleading "stale shadow" warning on every install. + if (version !== '0.1.0') return { stale: false }; + return { stale: true, version }; +} + +/** + * #3406 helper: format the install-time warning emitted when + * `detectStaleStandaloneSdk` reports a stale shadow. Separated from the + * detection so the message contract is testable independently of npm. + */ +function formatStaleStandaloneSdkWarning(info) { + const version = info && info.version ? info.version : '(unknown)'; + return [ + '⚠ A stale globally-installed @gsd-build/sdk@' + version + ' is shadowing the', + ' `gsd-sdk` shim that get-shit-done-cc provides. The standalone package', + ' only knows `run | auto | init` — every `gsd-sdk query ` call from', + ' skills and hooks will fail until you remove it.', + '', + ' Remediation:', + ' npm uninstall -g @gsd-build/sdk', + ' npx -y get-shit-done-cc@latest -- --global', + '', + ' Tracking: #3406 — https://github.com/gsd-build/get-shit-done/issues/3406', + ].join('\n'); +} + /** * #3231 helper: detect whether a `gsd-sdk` binary is the legacy deprecated * shim pointing at `gsd-tools.cjs`. @@ -11215,6 +11328,8 @@ if (process.env.GSD_TEST_MODE) { installAllRuntimes, uninstall, installSdkIfNeeded, + detectStaleStandaloneSdk, + formatStaleStandaloneSdkWarning, buildSdkFailFastReport, renderSdkFailFastReport, classifySdkInstall, diff --git a/tests/bug-3406-stale-sdk-shadow-detect.test.cjs b/tests/bug-3406-stale-sdk-shadow-detect.test.cjs new file mode 100644 index 000000000..3f836a02c --- /dev/null +++ b/tests/bug-3406-stale-sdk-shadow-detect.test.cjs @@ -0,0 +1,169 @@ +'use strict'; + +/** + * Regression tests for #3406 — stale globally-installed `@gsd-build/sdk@0.1.0` + * shadows the `gsd-sdk` shim that `get-shit-done-cc` installs. The standalone + * 0.1.0 binary only knows `run | auto | init` (no `query` subcommand), so + * every workflow that calls `gsd-sdk query ` fails until the user + * runs `npm uninstall -g @gsd-build/sdk`. + * + * Maintainer decision (per triage): option 2 — detect-and-warn during + * install. This test pins the pure detection helper so the install-time + * warning fires on the right input and stays silent otherwise. + * + * Test surface is the exported helper `detectStaleStandaloneSdk(runNpmLs)`. + * `runNpmLs` is an injected executor: in production it spawns + * `npm ls -g @gsd-build/sdk --json --depth=0`; in tests we hand it a stub + * that returns canned stdout / throws, so the test never touches the host + * npm state. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); + +process.env.GSD_TEST_MODE = '1'; +const installer = require('../bin/install.js'); +const { detectStaleStandaloneSdk } = installer; + +describe('#3406: detectStaleStandaloneSdk', () => { + test('is exported from bin/install.js under GSD_TEST_MODE', () => { + assert.strictEqual( + typeof detectStaleStandaloneSdk, + 'function', + 'detectStaleStandaloneSdk must be exported for install-time wiring + tests' + ); + }); + + test('returns { stale: false } when npm ls reports the package is not installed', () => { + // `npm ls -g @gsd-build/sdk --json --depth=0` exit code 1 with this + // JSON shape is the standard "not present" signal. + const stub = () => JSON.stringify({ + name: 'lib', + dependencies: {}, + }); + const result = detectStaleStandaloneSdk(stub); + assert.deepStrictEqual(result, { stale: false }); + }); + + test('returns { stale: true, version, path? } when @gsd-build/sdk is present', () => { + const stub = () => JSON.stringify({ + name: 'lib', + dependencies: { + '@gsd-build/sdk': { + version: '0.1.0', + resolved: 'file:/Users/REDACTED/.nvm/versions/node/v24.15.0/lib/node_modules/@gsd-build/sdk', + }, + }, + }); + const result = detectStaleStandaloneSdk(stub); + assert.strictEqual(result.stale, true); + assert.strictEqual(result.version, '0.1.0'); + }); + + test('returns { stale: false } when runNpmLs throws (npm missing / EACCES)', () => { + const stub = () => { throw new Error('npm: command not found'); }; + const result = detectStaleStandaloneSdk(stub); + assert.deepStrictEqual(result, { stale: false }); + }); + + test('returns { stale: false } when runNpmLs returns malformed JSON', () => { + const stub = () => 'not-json-at-all'; + const result = detectStaleStandaloneSdk(stub); + assert.deepStrictEqual(result, { stale: false }); + }); + + test('returns { stale: false } when the JSON has no dependencies field', () => { + const stub = () => JSON.stringify({ name: 'lib' }); + const result = detectStaleStandaloneSdk(stub); + assert.deepStrictEqual(result, { stale: false }); + }); + + test('returns { stale: false } when runNpmLs returns null/undefined', () => { + const resultNull = detectStaleStandaloneSdk(() => null); + const resultUndef = detectStaleStandaloneSdk(() => undefined); + assert.deepStrictEqual(resultNull, { stale: false }); + assert.deepStrictEqual(resultUndef, { stale: false }); + }); + + test('returns { stale: false } for non-0.1.0 versions (CR #3406)', () => { + // Only 0.1.0 is the known-bad shadow. Any newer or unrelated published + // version is an intentional install (or a future republish) and must + // NOT be flagged. Without this gate, every maintainer with a local-link + // or any future publish would trigger a misleading warning on install. + const stubNewer = () => JSON.stringify({ dependencies: { '@gsd-build/sdk': { version: '1.50.0-canary.0' } } }); + const stubFuture = () => JSON.stringify({ dependencies: { '@gsd-build/sdk': { version: '2.0.0' } } }); + assert.deepStrictEqual(detectStaleStandaloneSdk(stubNewer), { stale: false }); + assert.deepStrictEqual(detectStaleStandaloneSdk(stubFuture), { stale: false }); + }); +}); + +describe('#3406: install-time wiring stays silent when no stale package is found', () => { + // We can't easily stub npm inside a spawned install subprocess without + // shelling around it, so the install-side coverage here verifies the + // negative case: when @gsd-build/sdk is NOT installed globally (the npm + // dependency tree on CI is irrelevant — we use a doctored PATH that points + // npm at an empty prefix), the install run prints NO #3406 warning. The + // positive case is exhaustively covered by detectStaleStandaloneSdk above. + const path = require('node:path'); + const fs = require('node:fs'); + const os = require('node:os'); + const { execFileSync } = require('node:child_process'); + + test('install does not emit the stale-SDK warning on a clean npm prefix', () => { + const installScript = path.resolve(__dirname, '..', 'bin', 'install.js'); + const tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3406-home-')); + const tmpPrefix = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3406-npm-')); + try { + const stdout = execFileSync( + process.execPath, + [installScript, '--claude', '--global', '--yes', '--no-sdk'], + { + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + env: { + ...process.env, + CLAUDE_CONFIG_DIR: tmpHome, + npm_config_prefix: tmpPrefix, + // Detect-and-warn must execute, just find nothing — so do NOT set + // GSD_SKIP_STALE_SDK_CHECK here. + }, + timeout: 60_000, + } + ); + assert.ok( + !stdout.includes('@gsd-build/sdk'), + 'install output must not mention @gsd-build/sdk when the package is absent' + ); + assert.ok( + !stdout.includes('#3406'), + 'install output must not reference #3406 when no stale shadow is present' + ); + } finally { + try { fs.rmSync(tmpHome, { recursive: true, force: true }); } catch { /* ignore */ } + try { fs.rmSync(tmpPrefix, { recursive: true, force: true }); } catch { /* ignore */ } + } + }); +}); + +describe('#3406: formatStaleStandaloneSdkWarning', () => { + const { formatStaleStandaloneSdkWarning } = installer; + + test('is exported from bin/install.js under GSD_TEST_MODE', () => { + assert.strictEqual( + typeof formatStaleStandaloneSdkWarning, + 'function', + 'formatStaleStandaloneSdkWarning must be exported for tests' + ); + }); + + test('message names the stale package, the version, and the uninstall command', () => { + const out = formatStaleStandaloneSdkWarning({ stale: true, version: '0.1.0' }); + assert.ok(out.includes('@gsd-build/sdk'), 'must name the shadowing package'); + assert.ok(out.includes('0.1.0'), 'must show the stale version'); + assert.ok( + out.includes('npm uninstall -g @gsd-build/sdk'), + 'must include the remediation command verbatim' + ); + assert.ok(out.includes('#3406'), 'must reference the issue for traceability'); + }); +});