diff --git a/.changeset/install-shell-path-probe.md b/.changeset/install-shell-path-probe.md new file mode 100644 index 000000000..b67213f13 --- /dev/null +++ b/.changeset/install-shell-path-probe.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3028 +--- +**Installer no longer prints `✓ GSD SDK ready` when the shim is unreachable from the user's runtime shells.** The previous check used `process.env.PATH` from the install subprocess, which often differs from the user's later interactive shells (POSIX `~/.local/bin` not in login shell, node-version-manager PATH shims). Added `getUserShellPath()` helper that probes `$SHELL -lc 'printf %s "$PATH"'` and `isGsdSdkOnPath(pathString?)` overload that accepts an explicit PATH; the install-time check now downgrades to the actionable `⚠` diagnostic from PR #3014 when install-PATH and user-shell-PATH disagree. Windows cross-shell support tracked separately. See #3020. diff --git a/bin/install.js b/bin/install.js index 0e9e98d96..604c86915 100755 --- a/bin/install.js +++ b/bin/install.js @@ -9215,6 +9215,22 @@ function installSdkIfNeeded(opts) { } } + // #3020: cross-shell PATH verification. Even when the install-time + // process.env.PATH walk found the shim, the user's later interactive + // shells may have a different PATH — Windows cross-shell .cmd/no-ext + // mismatch, POSIX ~/.local/bin missing from login shell, or node- + // version-manager PATH shims. Probe the user's login shell PATH and + // 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). + const userShellPath = getUserShellPath(); + if (onPath && userShellPath !== null) { + const userSees = isGsdSdkOnPath(userShellPath); + if (!userSees) { + onPath = false; + } + } + if (onPath) { console.log(` ${green}✓${reset} GSD SDK ready (sdk/dist/cli.js)`); } else { @@ -9255,17 +9271,27 @@ function installSdkIfNeeded(opts) { } /** - * #2775 helper: check whether a callable `gsd-sdk` exists on the current PATH. + * #2775 helper: check whether a callable `gsd-sdk` exists on a PATH. * * Pure PATH walk (no spawn) — we look for a regular file or symlink named * `gsd-sdk` (or `gsd-sdk.cmd`/`.exe` on Windows) in any directory on PATH and * verify it carries the execute bit on POSIX. Avoids paying spawn cost and * avoids the chicken-and-egg of needing to run the not-yet-installed binary. + * + * #3020: accepts an optional explicit PATH string. The install subprocess's + * process.env.PATH is not the same set the user's later interactive shells + * see (Windows cross-shell, POSIX ~/.local/bin, node-version-manager + * 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. */ -function isGsdSdkOnPath() { +function isGsdSdkOnPath(pathString) { const path = require('path'); const fs = require('fs'); - const pathEnv = process.env.PATH || ''; + // Type-guard the explicit input (#3028 CR): callers may pass null + // (getUserShellPath() can return null), and `null.split()` throws. + // Only honor pathString when it's a string; fall back otherwise. + const pathEnv = typeof pathString === 'string' ? pathString : (process.env.PATH || ''); const exts = process.platform === 'win32' ? ['.cmd', '.exe', '.bat', ''] : ['']; for (const seg of pathEnv.split(path.delimiter)) { if (!seg) continue; @@ -9285,6 +9311,54 @@ function isGsdSdkOnPath() { return false; } +/** + * #3020: probe the user's login shell to learn the PATH that will be + * visible at workflow runtime. + * + * The install subprocess inherits process.env.PATH from npm/npx, which + * may include directories the user's interactive shells do not (e.g. + * ~/.local/bin auto-injected by npm-prefix tooling, or nvm-shimmed + * paths). Asserting `gsd-sdk` is on the install-subprocess PATH is a + * weaker invariant than the runtime contract — workflows shell out via + * `bash -c "gsd-sdk …"`, and that bash inherits PATH from the user's + * login shell. + * + * Uses `$SHELL -lc 'printf %s "$PATH"'` on POSIX. Returns null on Windows + * (cross-shell PATH probing requires a different strategy — Git Bash + * vs PowerShell vs cmd.exe each read PATH from different sources, and + * a future revision can build a Windows-aware probe). Returns null + * when $SHELL is unset, when the spawn fails, or when the result is + * empty — callers must fall back to process.env.PATH in those cases. + * + * Synchronous so it can be called from the existing post-install check + * without restructuring the whole flow as async. + */ +function getUserShellPath() { + if (process.platform === 'win32') return null; + const shellEnv = typeof process.env.SHELL === 'string' ? process.env.SHELL : ''; + if (!shellEnv) return null; + const cp = require('child_process'); + try { + const out = cp.execFileSync(shellEnv, ['-lc', 'printf %s "$PATH"'], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + // 2-second cap so a misconfigured rc file (e.g. interactive prompt) + // can't hang the install. The probe is best-effort — null on timeout + // is the safe fallback. + timeout: 2000, + }); + // #3028 CR: login startup scripts can print banners / motd / stale + // log lines BEFORE the printf, polluting stdout. Take the LAST + // non-empty line as the PATH candidate so noise doesn't flip the + // cross-shell check to false. PATH itself is single-line. + const lines = String(out || '').split(/\r?\n/).map((s) => s.trim()).filter(Boolean); + const candidate = lines.length > 0 ? lines[lines.length - 1] : ''; + return candidate.length > 0 ? candidate : null; + } catch { + return null; + } +} + /** * #2775 helper: attempt to materialize the `gsd-sdk` shim at a user-writable * PATH location. Returns the absolute path created on success, or null if no @@ -9679,6 +9753,7 @@ if (process.env.GSD_TEST_MODE) { buildWindowsShimTriple, formatSdkPathDiagnostic, isGsdSdkOnPath, + getUserShellPath, homePathCoveredByRc, maybeSuggestPathExport, runtimeMap, diff --git a/tests/bug-3020-install-shell-path-probe.test.cjs b/tests/bug-3020-install-shell-path-probe.test.cjs new file mode 100644 index 000000000..96bf6229f --- /dev/null +++ b/tests/bug-3020-install-shell-path-probe.test.cjs @@ -0,0 +1,169 @@ +/** + * Regression test for bug #3020. + * + * The installer prints `✓ GSD SDK ready (sdk/dist/cli.js)` whenever + * isGsdSdkOnPath() — which reads process.env.PATH from the install + * subprocess — finds the shim. That set is not the same as the user's + * later interactive shell PATH: + * + * - Windows cross-shell: gsd-sdk.cmd resolves under PowerShell/cmd + * (PATHEXT) but bare `gsd-sdk` does not resolve under Git Bash / + * MSYS / WSL bash. + * - POSIX ~/.local/bin: install subprocess inherits npm/npx-injected + * PATH containing ~/.local/bin; user's login shell may not. + * - Node version managers (nvm/fnm/volta) shim PATH per-shell. + * + * Result: green ✓ at install time, "command not found" at workflow + * runtime (#3011 originals + @x0rk + @stefanoginella). + * + * Fix: introduce two helpers and use them at install time. + * + * isGsdSdkOnPath(pathString?: string) + * - Now accepts an optional explicit PATH string. When omitted, + * falls back to process.env.PATH (preserves existing behavior). + * - Pure: no spawn, no I/O beyond fs.statSync on candidates. + * + * getUserShellPath() → string | null + * - Probes the user's login shell ($SHELL -lc 'printf %s "$PATH"') on + * POSIX so we can predict the runtime shell PATH. + * - Returns null on Windows or when the probe fails (caller falls + * back to process.env.PATH). + * + * Tests are typed-IR / structural — no console capture, no source grep. + */ + +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +const { test, describe } = 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 INSTALL = require(path.join(__dirname, '..', 'bin', 'install.js')); +const { isGsdSdkOnPath, getUserShellPath } = INSTALL; + +describe('bug #3020: isGsdSdkOnPath accepts an explicit PATH string', () => { + test('exported as a function', () => { + assert.equal(typeof isGsdSdkOnPath, 'function'); + }); + + test('returns true when an executable gsd-sdk exists in the supplied PATH', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3020-')); + try { + // Create a fake `gsd-sdk` shim with the executable bit set. + const shimName = process.platform === 'win32' ? 'gsd-sdk.cmd' : 'gsd-sdk'; + const shimPath = path.join(tmp, shimName); + fs.writeFileSync(shimPath, process.platform === 'win32' ? '@echo off\nexit 0\n' : '#!/bin/sh\nexit 0\n'); + if (process.platform !== 'win32') fs.chmodSync(shimPath, 0o755); + const result = isGsdSdkOnPath(tmp); + assert.equal(result, true, `expected true for PATH=${tmp}, got ${result}`); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('returns false when the supplied PATH has no gsd-sdk', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3020-')); + try { + const result = isGsdSdkOnPath(tmp); + assert.equal(result, false); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('zero-arg form preserves existing behavior (reads process.env.PATH)', () => { + // Just call it — it shouldn't throw and should return a boolean. + const result = isGsdSdkOnPath(); + assert.equal(typeof result, 'boolean'); + }); + + test('treats an empty PATH string as no segments to scan', () => { + const result = isGsdSdkOnPath(''); + assert.equal(result, false); + }); + + test('null pathString is type-guarded — falls back to process.env.PATH (#3028 CR)', () => { + // Pre-fix: isGsdSdkOnPath(null) threw "Cannot read properties of null + // (reading 'split')". Post-fix: typeof check falls back to process.env.PATH. + let threw = null; + let result; + try { + result = isGsdSdkOnPath(null); + } catch (e) { + threw = e; + } + assert.equal(threw, null, `must not throw on null input, got: ${threw && threw.message}`); + assert.equal(typeof result, 'boolean', 'must return a boolean'); + }); + + test('non-string pathString (number, object) falls back to process.env.PATH (#3028 CR)', () => { + // Defensive: any non-string argument should fall back, not throw. + assert.equal(typeof isGsdSdkOnPath(0), 'boolean'); + assert.equal(typeof isGsdSdkOnPath({}), 'boolean'); + assert.equal(typeof isGsdSdkOnPath([]), 'boolean'); + }); +}); + +describe('bug #3020: getUserShellPath probes the user login shell PATH', () => { + test('exported as a function', () => { + assert.equal(typeof getUserShellPath, 'function'); + }); + + test('returns a string with at least one segment OR null', () => { + const result = getUserShellPath(); + if (result === null) { + // Acceptable on Windows or when probing fails — caller must fall back. + return; + } + assert.equal(typeof result, 'string'); + // PATH must have segments separated by the platform delimiter. + assert.ok(result.length > 0, 'non-null result must be non-empty'); + }); + + test('returns null on Windows (POSIX shell probe is not portable)', () => { + if (process.platform !== 'win32') return; + const result = getUserShellPath(); + assert.equal(result, null); + }); + + test('returns null when SHELL env var is unset', () => { + if (process.platform === 'win32') return; + const original = process.env.SHELL; + delete process.env.SHELL; + try { + const result = getUserShellPath(); + assert.equal(result, null, 'must return null when $SHELL is unset (POSIX caller falls back to process.env.PATH)'); + } finally { + if (original !== undefined) process.env.SHELL = original; + } + }); +}); + +describe('bug #3020: cross-shell PATH mismatch is detectable via the new helpers', () => { + test('install-time PATH has shim, user-shell PATH does not → mismatch detected', () => { + const installDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3020-install-')); + const userDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3020-user-')); + try { + const shimName = process.platform === 'win32' ? 'gsd-sdk.cmd' : 'gsd-sdk'; + const shimPath = path.join(installDir, shimName); + fs.writeFileSync(shimPath, process.platform === 'win32' ? '@echo off\nexit 0\n' : '#!/bin/sh\nexit 0\n'); + if (process.platform !== 'win32') fs.chmodSync(shimPath, 0o755); + + const installSees = isGsdSdkOnPath(installDir); + const userSees = isGsdSdkOnPath(userDir); + + assert.equal(installSees, true, 'install-time PATH sees the shim'); + assert.equal(userSees, false, 'user-shell PATH does not see the shim'); + // The mismatch is what the post-install check must detect to avoid + // the false ✓. + assert.notEqual(installSees, userSees, 'shim presence differs between install-time PATH and user-shell PATH'); + } finally { + fs.rmSync(installDir, { recursive: true, force: true }); + fs.rmSync(userDir, { recursive: true, force: true }); + } + }); +});