diff --git a/.changeset/quick-voles-sprint.md b/.changeset/quick-voles-sprint.md new file mode 100644 index 000000000..88664d3c8 --- /dev/null +++ b/.changeset/quick-voles-sprint.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3249 +--- +**`✓ GSD SDK ready` no longer prints when no persistent `gsd-sdk` shim exists** — the installer now requires durable reachability (not just transient npx PATH) and replaces stale legacy symlinks pointing at deprecated `gsd-tools.cjs`. Falls back to an actionable warning when login-shell PATH probing fails. diff --git a/.gitignore b/.gitignore index 391bca1e2..89d9432e6 100644 --- a/.gitignore +++ b/.gitignore @@ -14,8 +14,8 @@ commands.html # Build artifacts (committed to npm, not git) hooks/dist/ -# Atomic-write staging dir used by scripts/build-hooks.js (see comment there) -hooks/.dist-staging/ +# Per-process atomic-write staging dirs used by scripts/build-hooks.js (see comment there) +hooks/.dist-staging-*/ # Coverage artifacts coverage/ diff --git a/bin/install.js b/bin/install.js index 888cd94f9..12b174dbf 100755 --- a/bin/install.js +++ b/bin/install.js @@ -9437,8 +9437,14 @@ function installSdkIfNeeded(opts) { // strictly weaker invariant than the one workflows depend on // (`command -v gsd-sdk` resolving), and led to a false ✓ in npx-cache // installs (issue #2775). + // + // #3231: strip transient npx-injected PATH segments before checking. The + // installer subprocess PATH includes `~/.npm/_npx//node_modules/.bin` + // which is ephemeral — it is NOT reachable from the user's interactive + // shell. A gsd-sdk found there must NOT count as "on PATH". const shimSrc = path.resolve(__dirname, 'gsd-sdk.js'); - let onPath = isGsdSdkOnPath(); + const persistentPath = filterNpxFromPath(process.env.PATH || ''); + let onPath = isGsdSdkOnPath(persistentPath); // Track WHERE we wrote the shim so the diagnostic can be specific even // when isGsdSdkOnPath() returns false because the write target isn't on @@ -9455,7 +9461,7 @@ function installSdkIfNeeded(opts) { const linked = trySelfLinkGsdSdk(shimSrc); if (linked) { shimDir = path.dirname(linked); - onPath = isGsdSdkOnPath(); + onPath = isGsdSdkOnPath(persistentPath); if (onPath) { console.log(` ${dim}↪ linked gsd-sdk → ${linked}${reset}`); } @@ -9470,13 +9476,23 @@ function installSdkIfNeeded(opts) { // require the shim to be reachable there too before claiming ✓. // POSIX-only probe; on Windows getUserShellPath() returns null and // we trust the existing check (Windows-specific fix is separate). + // + // #3231: when getUserShellPath() returns null (e.g. $SHELL unset on + // Linux, rc-file timeout), we cannot confirm persistent reachability. + // In that case, do NOT preserve a true onPath — require the initial + // check (on persistentPath) to have found the shim in a persistent + // location. Since we already filtered npx dirs above, onPath=true here + // means a non-transient dir has the shim, which is sufficient. const userShellPath = getUserShellPath(); if (onPath && userShellPath !== null) { - const userSees = isGsdSdkOnPath(userShellPath); + const persistentUserShellPath = filterNpxFromPath(userShellPath); + const userSees = isGsdSdkOnPath(persistentUserShellPath); if (!userSees) { onPath = false; } } + // If userShellPath is null (POSIX probe failed), onPath reflects + // the persistent-PATH check — that is the best available invariant. if (onPath) { console.log(` ${green}✓${reset} GSD SDK ready (sdk/dist/cli.js)`); @@ -9517,6 +9533,74 @@ function installSdkIfNeeded(opts) { } } +/** + * #3231 helper: detect whether a `gsd-sdk` binary is the legacy deprecated + * shim pointing at `gsd-tools.cjs`. + * + * Reads the first 512 bytes of the file and looks for the `@deprecated` + * marker alongside a `gsd-tools.cjs` reference — the fingerprint that + * distinguishes the old binary from the modern SDK. Treats any I/O error + * (missing file, EACCES) as "not legacy" so callers do not need to guard. + * + * This is intentionally a plain-text sniff of the file header, not a + * semantic parse — the marker is a stable, human-authored string that we + * own. Returns false conservatively (prefer false positives to false + * negatives: a non-legacy binary reported as legacy triggers a harmless + * replacement; a legacy binary reported as non-legacy would keep the broken + * shim in place). + */ +function isLegacyGsdSdkShim(filePath) { + const fs = require('fs'); + try { + const fd = fs.openSync(filePath, 'r'); + let header; + try { + const buf = Buffer.alloc(512); + const bytesRead = fs.readSync(fd, buf, 0, 512, 0); + header = buf.slice(0, bytesRead).toString('utf8'); + } finally { + try { fs.closeSync(fd); } catch {} + } + // The legacy binary contains "@deprecated" AND "gsd-tools.cjs" within + // its first 512 bytes. + return header.includes('@deprecated') && header.includes('gsd-tools.cjs'); + } catch { + return false; + } +} + +/** + * #3231 helper: strip transient npx-injected PATH segments. + * + * npm/npx injects `~/.npm/_npx//node_modules/.bin` (and equivalents) + * into the installer subprocess PATH. Those directories are ephemeral — they + * exist only for the duration of the `npx` run — and MUST NOT be treated as + * evidence that `gsd-sdk` is durably reachable. + * + * Strips any segment whose absolute form contains `/_npx/` or `\\_npx\\` + * as a proper path-component boundary. A user-named directory that merely + * contains the substring "npx" (e.g. `/home/user/my-npx-scripts/bin`) is + * preserved: we require the boundary characters (`/` or `\`) on both sides. + * + * Returns the filtered PATH string (may be empty if all segments were npx). + */ +function filterNpxFromPath(pathString) { + const path = require('path'); + const input = typeof pathString === 'string' ? pathString : (process.env.PATH || ''); + return input + .split(path.delimiter) + .filter((seg) => { + if (!seg) return false; + // Normalize to forward-slash form for the pattern check so both + // POSIX and Windows paths match a single expression. The sep-anchored + // pattern avoids matching "my-npx-scripts" etc. + const norm = seg.replace(/\\/g, '/'); + // Must have /_npx/ as a real path component, not just a substring. + return !norm.includes('/_npx/'); + }) + .join(path.delimiter); +} + /** * #2775 helper: check whether a callable `gsd-sdk` exists on a PATH. * @@ -9531,6 +9615,10 @@ function installSdkIfNeeded(opts) { * shims). Callers can pass the user-shell PATH from getUserShellPath() to * verify the shim is reachable from the runtime shell, not just the * install context. Zero-arg form preserves existing behavior. + * + * #3231: a candidate that passes the file/exec check is further tested via + * isLegacyGsdSdkShim — a symlink pointing at the deprecated gsd-tools.cjs + * binary must NOT be treated as "on PATH" even if it is executable. */ function isGsdSdkOnPath(pathString) { const path = require('path'); @@ -9547,8 +9635,15 @@ function isGsdSdkOnPath(pathString) { try { const st = fs.statSync(candidate); if (st.isFile()) { - if (process.platform === 'win32') return true; - if ((st.mode & 0o111) !== 0) return true; + if (process.platform === 'win32') { + if (!isLegacyGsdSdkShim(candidate)) return true; + } else if ((st.mode & 0o111) !== 0) { + // #3231: resolve symlink before sniffing, so we detect legacy + // through any level of indirection. + let target = candidate; + try { target = fs.realpathSync(candidate); } catch {} + if (!isLegacyGsdSdkShim(target)) return true; + } } } catch { // missing / EACCES on dir — keep scanning. @@ -10040,6 +10135,8 @@ if (process.env.GSD_TEST_MODE) { trySelfLinkGsdSdkWindows, buildWindowsShimTriple, formatSdkPathDiagnostic, + filterNpxFromPath, + isLegacyGsdSdkShim, isGsdSdkOnPath, getUserShellPath, homePathCoveredByRc, diff --git a/scripts/build-hooks.js b/scripts/build-hooks.js index e0563c3e5..47e5bc131 100644 --- a/scripts/build-hooks.js +++ b/scripts/build-hooks.js @@ -12,12 +12,15 @@ const vm = require('vm'); const HOOKS_DIR = path.join(__dirname, '..', 'hooks'); const DIST_DIR = path.join(HOOKS_DIR, 'dist'); -// Sibling directory used to stage atomic writes. Lives under hooks/ so it -// shares a filesystem with DIST_DIR (POSIX rename(2) is only atomic within -// the same filesystem) but is NOT inside DIST_DIR — so readers that -// readdirSync(DIST_DIR) (e.g. bin/install.js, install-hooks-copy tests) -// never observe a transient ".tmp" sibling file there. -const STAGE_DIR = path.join(HOOKS_DIR, '.dist-staging'); +// Per-process staging directory for atomic writes. Using process.pid in the +// name eliminates all contention between concurrent builders: each process +// owns its own staging dir and never races with another builder's cleanup. +// Lives under hooks/ so it shares a filesystem with DIST_DIR (POSIX +// rename(2) is only atomic within the same filesystem) but is NOT inside +// DIST_DIR — so readers that readdirSync(DIST_DIR) (e.g. bin/install.js, +// install-hooks-copy tests) never observe a transient ".tmp" sibling. +// The parent pattern hooks/.dist-staging-*/ is gitignored. +const STAGE_DIR = path.join(HOOKS_DIR, `.dist-staging-${process.pid}`); // Hooks to copy (pure Node.js, no bundling needed) const HOOKS_TO_COPY = [ @@ -143,7 +146,7 @@ function build() { } console.log(`\x1b[32m✓\x1b[0m Copying ${hook}...`); - // Atomic write: copy to a per-process staging file in the sibling + // Atomic write: copy to a per-process staging file in the per-PID sibling // STAGE_DIR (same filesystem as DIST_DIR so rename(2) is atomic), then // rename into place. Multiple test files invoke this script concurrently // from their before() hooks; fs.copyFileSync truncates then writes the @@ -154,7 +157,9 @@ function build() { // makes the swap atomic so readers see either the old file or the new // file. The staging file lives outside DIST_DIR so readdirSync(DIST_DIR) // (in install.js and tests) never observes a transient ".tmp" sibling. - const stagedDest = path.join(STAGE_DIR, `${hook}.${process.pid}.${Date.now()}`); + // Each process uses its own STAGE_DIR (keyed by PID) so concurrent + // builders never race on staging-dir creation or cleanup. + const stagedDest = path.join(STAGE_DIR, `${hook}.${Date.now()}`); fs.copyFileSync(src, stagedDest); // Preserve executable bit for shell scripts before rename so the // installed file is executable from the very first observation. @@ -164,22 +169,12 @@ function build() { renameAtomicWithRetry(stagedDest, dest, hook); } - // Best-effort cleanup of the staging dir. If concurrent builders are still - // running, their staged files will be left in STAGE_DIR and cleaned up by - // whichever builder calls fs.rmdirSync last. fs.rmdirSync throws ENOTEMPTY - // on a non-empty directory (it is NOT a silent no-op), so we first read - // the directory via fs.readdirSync(STAGE_DIR) -> leftovers and only call - // fs.rmdirSync(STAGE_DIR) when leftovers.length === 0. A TOCTOU window - // remains: another builder can drop a staged file between the readdirSync - // and the rmdirSync, in which case rmdirSync still throws ENOTEMPTY — the - // outer try/catch swallows that, plus ENOENT if the dir was already - // removed by a peer. Either way, build proceeds; cleanup is best-effort. + // Best-effort cleanup of this process's own staging dir. Since STAGE_DIR + // is per-PID (`.dist-staging-/`), no other builder touches it — so + // rmSync with recursive:true is safe and leaves no race window. try { - const leftovers = fs.readdirSync(STAGE_DIR); - if (leftovers.length === 0) { - fs.rmdirSync(STAGE_DIR); - } - } catch (e) { /* tolerate TOCTOU ENOTEMPTY or ENOENT from peer cleanup */ } + fs.rmSync(STAGE_DIR, { recursive: true, force: true }); + } catch (e) { /* tolerate ENOENT if the dir was never created (e.g. all hooks skipped) */ } if (hasErrors) { console.error('\n\x1b[31mBuild failed: fix syntax errors above before publishing.\x1b[0m'); diff --git a/tests/bug-3231-false-gsd-sdk-ready-linux.test.cjs b/tests/bug-3231-false-gsd-sdk-ready-linux.test.cjs new file mode 100644 index 000000000..4afffb7eb --- /dev/null +++ b/tests/bug-3231-false-gsd-sdk-ready-linux.test.cjs @@ -0,0 +1,420 @@ +/** + * Regression tests for bug #3231. + * + * `npx get-shit-done-cc@latest` prints `✓ GSD SDK ready (sdk/dist/cli.js)` on + * Linux but no persistent `gsd-sdk` shim is created. Two sub-bugs: + * + * 1. Transient npx PATH + null login-shell PATH → false success + * The initial isGsdSdkOnPath() call uses process.env.PATH, which includes + * `~/.npm/_npx//node_modules/.bin` — a transient dir npx injects. + * If that dir has a `gsd-sdk` entry, onPath = true and trySelfLinkGsdSdk + * is skipped (no persistent shim). Then getUserShellPath() returns null + * (Linux, slow rc files or unset $SHELL). The guard + * `onPath && userShellPath !== null` is FALSE, leaving onPath = true → + * false `✓ GSD SDK ready` is printed. + * + * 2. Stale legacy symlink → installer treats gsd-sdk as "on PATH" and skips + * materializing a modern SDK shim. The legacy binary (`gsd-tools.cjs`) has + * an `@deprecated` marker in its first bytes, lacks the `query` registry, + * and causes "Unknown command: query" for every workflow call. + * + * 3. Clean path: sdk/dist/cli.js present + gsd-sdk self-linked into a + * persistent PATH dir → installer DOES print success. + * + * All assertions use typed-IR / behavioral testing. No source-grep, no + * readFileSync on install.js. + */ + +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +const { describe, test, beforeEach, afterEach } = 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 installModule = require('../bin/install.js'); +const { + installSdkIfNeeded, + isGsdSdkOnPath, + filterNpxFromPath, + isLegacyGsdSdkShim, +} = installModule; + +// --------------------------------------------------------------------------- +// Console capture helper (no ANSI) +// --------------------------------------------------------------------------- +function captureConsole(fn) { + const stdout = []; + const stderr = []; + const origLog = console.log; + const origWarn = console.warn; + const origError = console.error; + console.log = (...a) => stdout.push(a.join(' ')); + console.warn = (...a) => stderr.push(a.join(' ')); + console.error = (...a) => stderr.push(a.join(' ')); + let threw = null; + try { + fn(); + } catch (e) { + threw = e; + } finally { + console.log = origLog; + console.warn = origWarn; + console.error = origError; + } + if (threw) throw threw; + const strip = (s) => s.replace(/\x1b\[[0-9;]*m/g, ''); + return { + stdout: stdout.map(strip).join('\n'), + stderr: stderr.map(strip).join('\n'), + }; +} + +// --------------------------------------------------------------------------- +// Shared fixture helpers +// --------------------------------------------------------------------------- +function makeSdkDir(root) { + const sdkDir = path.join(root, 'sdk'); + fs.mkdirSync(path.join(sdkDir, 'dist'), { recursive: true }); + fs.writeFileSync( + path.join(sdkDir, 'dist', 'cli.js'), + ['#!/usr/bin/env node', "console.log('0.0.0-test');", ''].join('\n'), + { mode: 0o755 }, + ); + return sdkDir; +} + +// --------------------------------------------------------------------------- +// Bug 1: transient npx PATH hit + null login-shell PATH → false "GSD SDK ready" +// --------------------------------------------------------------------------- +describe('bug #3231: transient npx PATH + null login-shell PATH', () => { + let tmpRoot; + let sdkDir; + let savedEnv; + + beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3231-a-')); + sdkDir = makeSdkDir(tmpRoot); + + // Simulate an npx-injected PATH: a transient _npx directory that happens + // to contain a gsd-sdk executable. This is NOT a persistent user location. + const npxBinDir = path.join(tmpRoot, '.npm', '_npx', 'abc123', 'node_modules', '.bin'); + fs.mkdirSync(npxBinDir, { recursive: true }); + const shimName = process.platform === 'win32' ? 'gsd-sdk.cmd' : 'gsd-sdk'; + const shimPath = path.join(npxBinDir, shimName); + fs.writeFileSync( + shimPath, + ['#!/bin/sh', 'exit 0', ''].join('\n'), + { mode: 0o755 }, + ); + + const homeDir = path.join(tmpRoot, 'home'); + fs.mkdirSync(homeDir, { recursive: true }); + + savedEnv = { + PATH: process.env.PATH, + HOME: process.env.HOME, + SHELL: process.env.SHELL, + }; + + // Install-subprocess PATH contains ONLY the npx transient dir — nothing + // persistent. $SHELL is unset to simulate getUserShellPath() → null. + process.env.PATH = npxBinDir; + process.env.HOME = homeDir; + delete process.env.SHELL; + }); + + afterEach(() => { + if (savedEnv.PATH == null) delete process.env.PATH; + else process.env.PATH = savedEnv.PATH; + if (savedEnv.HOME == null) delete process.env.HOME; + else process.env.HOME = savedEnv.HOME; + if (savedEnv.SHELL == null) delete process.env.SHELL; + else process.env.SHELL = savedEnv.SHELL; + try { fs.rmSync(tmpRoot, { recursive: true, force: true }); } catch {} + }); + + test('does NOT print "GSD SDK ready" when only a transient _npx PATH entry has gsd-sdk', () => { + // Pre-fix: isGsdSdkOnPath() finds gsd-sdk in the npx-injected dir, + // onPath = true, trySelfLinkGsdSdk is skipped, getUserShellPath() returns + // null (SHELL unset), the guard is short-circuited, and the false ✓ is + // printed. Post-fix: _npx dirs must be excluded from the initial check + // so the installer attempts self-link and re-probes. + const { stdout, stderr } = captureConsole(() => { + installSdkIfNeeded({ sdkDir }); + }); + const combined = `${stdout}\n${stderr}`; + + // Primary behavioral assertion: the installer must NOT falsely report + // "GSD SDK ready" when gsd-sdk is only reachable via a transient npx + // cache directory (not a persistent user PATH entry). + assert.ok( + !/GSD SDK ready/.test(combined), + 'installer must NOT print "GSD SDK ready" when only the transient _npx dir has gsd-sdk. Got: ' + combined, + ); + + // Secondary assertion: the installer must emit a warning or fallback + // diagnostic rather than silently succeeding. The warning path prints + // "GSD SDK files are present but gsd-sdk is not on your PATH" when + // self-link fails; a successful self-link into a non-PATH dir prints the + // same warning. Either way, some output must be produced. + assert.ok( + combined.trim().length > 0, + 'installer must emit a diagnostic (warning or fallback) instead of silent no-op. Got empty output.', + ); + }); + + test('filterNpxFromPath is exported and strips /_npx/ segments', () => { + // The fix adds a helper that removes any PATH segment whose absolute path + // contains /_npx/ (POSIX) or \\_npx\\ (Windows). + assert.equal(typeof filterNpxFromPath, 'function', 'filterNpxFromPath must be exported'); + + const npxDir = '/home/user/.npm/_npx/abc123/node_modules/.bin'; + const persistentDir = '/home/user/.local/bin'; + const unrelatedDir = '/usr/local/bin'; + const result = filterNpxFromPath( + [npxDir, persistentDir, unrelatedDir].join(path.delimiter), + ); + const segments = result.split(path.delimiter); + assert.ok(!segments.includes(npxDir), 'filtered PATH must not include the _npx dir'); + assert.ok(segments.includes(persistentDir), 'filtered PATH must keep persistent dirs'); + assert.ok(segments.includes(unrelatedDir), 'filtered PATH must keep unrelated dirs'); + }); + + test('filterNpxFromPath must not strip a user-named directory that merely contains "npx" as substring', () => { + // Containment guard: only strip when the segment truly contains /_npx/ + // (between separators), not when "npx" appears as part of a user dir name. + assert.equal(typeof filterNpxFromPath, 'function'); + const npxLikeUserDir = '/home/user/scripts/my-npx-wrapper/bin'; + const realNpxDir = '/home/user/.npm/_npx/abc/node_modules/.bin'; + const result = filterNpxFromPath( + [npxLikeUserDir, realNpxDir].join(path.delimiter), + ); + const segments = result.split(path.delimiter); + assert.ok( + segments.includes(npxLikeUserDir), + 'must not strip user dirs that merely contain "npx" as a substring', + ); + assert.ok(!segments.includes(realNpxDir), 'must strip real _npx dirs'); + }); +}); + +// --------------------------------------------------------------------------- +// Bug 2: stale legacy symlink pointing at gsd-tools.cjs (deprecated binary) +// --------------------------------------------------------------------------- +describe('bug #3231: stale legacy symlink to deprecated gsd-tools.cjs', () => { + let tmpRoot; + let sdkDir; + let savedEnv; + + beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3231-b-')); + sdkDir = makeSdkDir(tmpRoot); + + const homeDir = path.join(tmpRoot, 'home'); + fs.mkdirSync(homeDir, { recursive: true }); + + savedEnv = { + PATH: process.env.PATH, + HOME: process.env.HOME, + SHELL: process.env.SHELL, + }; + process.env.HOME = homeDir; + delete process.env.SHELL; + }); + + afterEach(() => { + if (savedEnv.PATH == null) delete process.env.PATH; + else process.env.PATH = savedEnv.PATH; + if (savedEnv.HOME == null) delete process.env.HOME; + else process.env.HOME = savedEnv.HOME; + if (savedEnv.SHELL == null) delete process.env.SHELL; + else process.env.SHELL = savedEnv.SHELL; + try { fs.rmSync(tmpRoot, { recursive: true, force: true }); } catch {} + }); + + test('isLegacyGsdSdkShim detects the deprecated gsd-tools.cjs marker', () => { + // The legacy binary starts with or contains the @deprecated marker + // referencing gsd-tools.cjs in the first 512 bytes. + assert.equal(typeof isLegacyGsdSdkShim, 'function', 'isLegacyGsdSdkShim must be exported'); + + const legacyFile = path.join(tmpRoot, 'gsd-sdk-legacy'); + fs.writeFileSync( + legacyFile, + [ + '#!/usr/bin/env node', + '// @deprecated — use gsd-tools.cjs directly', + "require('/usr/local/lib/gsd-tools.cjs');", + '', + ].join('\n'), + ); + + assert.equal(isLegacyGsdSdkShim(legacyFile), true, 'must detect legacy marker'); + }); + + test('isLegacyGsdSdkShim returns false for a modern SDK shim', () => { + assert.equal(typeof isLegacyGsdSdkShim, 'function'); + + const modernFile = path.join(tmpRoot, 'gsd-sdk-modern'); + fs.writeFileSync( + modernFile, + [ + '#!/usr/bin/env node', + "require('/usr/local/lib/node_modules/get-shit-done-cc/bin/gsd-sdk.js');", + '', + ].join('\n'), + ); + + assert.equal(isLegacyGsdSdkShim(modernFile), false, 'must not flag modern shims as legacy'); + }); + + test('isLegacyGsdSdkShim returns false for a non-existent file', () => { + assert.equal(typeof isLegacyGsdSdkShim, 'function'); + const missing = path.join(tmpRoot, 'does-not-exist'); + assert.equal(isLegacyGsdSdkShim(missing), false, 'missing file is not a legacy shim'); + }); + + test('installer replaces a stale legacy symlink and attempts self-link with modern SDK', () => { + // Set up: persistent PATH dir exists and contains a gsd-sdk symlink + // pointing at a fake "legacy" gsd-tools.cjs binary with the @deprecated + // marker. The installer must detect this, treat it as "not the right SDK", + // and replace it with a modern shim. + const persistentBin = path.join(tmpRoot, 'localbin'); + fs.mkdirSync(persistentBin, { recursive: true }); + + // Write a fake legacy binary + const legacyBin = path.join(tmpRoot, 'gsd-tools.cjs'); + fs.writeFileSync( + legacyBin, + [ + '#!/usr/bin/env node', + '// @deprecated — use gsd-tools.cjs directly', + "console.log('legacy');", + '', + ].join('\n'), + { mode: 0o755 }, + ); + + // Place a gsd-sdk symlink in the persistent dir pointing at the legacy binary. + const legacyShimPath = path.join(persistentBin, 'gsd-sdk'); + try { + fs.symlinkSync(legacyBin, legacyShimPath); + } catch { + // On Windows or symlink-hostile FS, write a file that mimics the legacy content + fs.writeFileSync( + legacyShimPath, + [ + '#!/usr/bin/env node', + '// @deprecated — use gsd-tools.cjs directly', + "console.log('legacy');", + '', + ].join('\n'), + { mode: 0o755 }, + ); + } + + process.env.PATH = persistentBin; + + const { stdout, stderr } = captureConsole(() => { + installSdkIfNeeded({ sdkDir }); + }); + const combined = `${stdout}\n${stderr}`; + + // After replacement the installer should succeed; if replacement fails (e.g. + // because the link dir is truly persistent), it must at minimum NOT report + // "GSD SDK ready" with the legacy binary still in place — it must warn. + const shimWasReplaced = !isLegacyGsdSdkShim(legacyShimPath); + if (shimWasReplaced) { + // Self-link succeeded: the shim is modern, so the installer must have + // reported readiness. + assert.ok( + stdout.length > 0, + 'installer must emit output after successful self-link', + ); + } else { + // Self-link failed or was skipped: the installer must NOT have falsely + // reported "GSD SDK ready" while the legacy binary is still in place. + assert.ok( + !/GSD SDK ready/.test(combined), + 'installer must NOT report ready while the legacy shim is still in place', + ); + // It must also have emitted a diagnostic (not silently swallowed). + assert.ok( + stdout.length > 0, + 'when self-link is skipped, installer should emit a PATH diagnostic', + ); + } + }); +}); + +// --------------------------------------------------------------------------- +// Test 3: clean install with gsd-sdk self-linked into a persistent PATH dir +// --------------------------------------------------------------------------- +describe('bug #3231: clean install — gsd-sdk self-linked into persistent PATH dir', () => { + let tmpRoot; + let sdkDir; + let savedEnv; + + beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3231-c-')); + sdkDir = makeSdkDir(tmpRoot); + const homeDir = path.join(tmpRoot, 'home'); + fs.mkdirSync(homeDir, { recursive: true }); + + savedEnv = { + PATH: process.env.PATH, + HOME: process.env.HOME, + SHELL: process.env.SHELL, + }; + process.env.HOME = homeDir; + delete process.env.SHELL; + }); + + afterEach(() => { + if (savedEnv.PATH == null) delete process.env.PATH; + else process.env.PATH = savedEnv.PATH; + if (savedEnv.HOME == null) delete process.env.HOME; + else process.env.HOME = savedEnv.HOME; + if (savedEnv.SHELL == null) delete process.env.SHELL; + else process.env.SHELL = savedEnv.SHELL; + try { fs.rmSync(tmpRoot, { recursive: true, force: true }); } catch {} + }); + + test('prints "GSD SDK ready" when gsd-sdk is self-linked into a persistent dir on PATH', () => { + const homeDir = process.env.HOME; + const localBin = path.join(homeDir, '.local', 'bin'); + fs.mkdirSync(localBin, { recursive: true }); + // PATH contains only the persistent localBin (no npx dirs) + process.env.PATH = localBin; + + const { stdout, stderr } = captureConsole(() => { + installSdkIfNeeded({ sdkDir }); + }); + const combined = `${stdout}\n${stderr}`; + + const shimPath = path.join(localBin, 'gsd-sdk'); + // Behavioral assertions: shim exists and is recognized as a modern (non-legacy) shim + // reachable from the persistent filtered PATH. + assert.ok( + fs.existsSync(shimPath), + 'installer must materialize gsd-sdk shim in the persistent PATH dir', + ); + assert.equal( + isGsdSdkOnPath(filterNpxFromPath(localBin)), + true, + 'installer must make gsd-sdk reachable on the persistent filtered PATH', + ); + + // Primary behavioral assertion: the installer MUST print "GSD SDK ready" + // after successfully self-linking into a persistent PATH dir. This is the + // positive counterpart to the bug #3231 fix — we confirm the success path + // works correctly, not just that the false-positive path is blocked. + assert.ok( + /GSD SDK ready/.test(stdout), + 'installer must print "GSD SDK ready" after a successful self-link into a persistent PATH dir. Got stdout: ' + stdout, + ); + }); +});