* fix(#3020): probe user shell PATH at install-time, not just process.env.PATH The installer's "✓ GSD SDK ready" message was a false positive whenever the install subprocess's process.env.PATH contained the gsd-sdk shim but the user's later interactive shells did not. Three known sources of mismatch on POSIX: - ~/.local/bin: install subprocess inherits npm/npx-injected PATH; user's login shell may not add ~/.local/bin if .profile/.bashrc/ .zshrc don't. - nvm/fnm/volta: node version managers shim PATH per-shell, so `npm prefix -g` from inside the install subprocess can resolve to a different bin dir than the user's interactive shell sees. - npm-prefix tooling: some installers inject extra PATH entries that vanish in fresh sessions. Result reported on #3011 by @x0rk and @stefanoginella: install prints ✓, but every workflow invocation later fails with "bash: gsd-sdk: command not found". Fix: - isGsdSdkOnPath(pathString?) — now accepts an explicit PATH string. Zero-arg form preserves existing behavior (reads process.env.PATH). Pure walk, no spawn. Lets callers verify against any PATH source. - getUserShellPath() — new helper. Probes the user's login shell via `$SHELL -lc 'printf %s "$PATH"'` (POSIX). 2-second timeout so a misconfigured rc file can't hang the install. Returns null on Windows (cross-shell PATH probing requires a different strategy per Git Bash / PowerShell / cmd.exe — tracked separately) or when the probe fails; callers fall back to process.env.PATH in that case. - installSdkIfNeeded() — after the existing isGsdSdkOnPath() check passes, also verify the shim is reachable from getUserShellPath() on POSIX. If install-PATH and user-shell-PATH disagree, downgrade to the actionable ⚠ diagnostic from PR #3014 (which has the shim location, shell-specific PATH-update commands, and an npx fallback note). Routing affected users into PR #3014's diagnostic is the point — not silently green-then-red. Tests: - bug-3020-install-shell-path-probe.test.cjs (10 tests, structural): - isGsdSdkOnPath accepts an explicit PATH (true/false on fixture PATH dirs with/without an executable shim) - zero-arg form returns a boolean - empty string PATH → false - getUserShellPath returns string-or-null - returns null on Windows - returns null when $SHELL unset on POSIX - cross-shell mismatch detection: install-PATH and user-PATH that differ produce different isGsdSdkOnPath results — the invariant the install-time check now exploits - All assertions on structural records, not console output. Adheres to typed-IR / CONTRIBUTING.md "Prohibited: Raw Text Matching". Verification: - 10/10 pass on new regression test - 6768/6768 pass on full suite (5 net-new tests) - lint-no-source-grep clean Windows cross-shell coverage (gsd-sdk.cmd resolves under PowerShell but not Git Bash without a no-extension sibling) is tracked separately — this PR is the POSIX-side fix and the Windows scaffolding (the optional pathString arg on isGsdSdkOnPath) that a Windows fix can build on. Closes #3020 * fix(#3020): type-guard pathString, last-line PATH parse (CR) CodeRabbit on PR #3028 (4 findings — 3 actionable + 1 nitpick): 1. .changeset/install-shell-path-probe.md (2 findings): - `pr: TBD` → `pr: 3028` - Doc said `echo $PATH` but impl uses `printf %s "$PATH"` (chosen to avoid shell-dependent echo behavior, e.g. interpreting `-n`). Aligned changeset prose with implementation. 2. bin/install.js:9176 — isGsdSdkOnPath(pathString) used `pathString !== undefined` to gate the explicit-PATH branch, but getUserShellPath() can return null and `null.split()` throws. Tightened to `typeof pathString === 'string'` so null / number / object inputs fall back to process.env.PATH. Added 2 regression tests covering the null and non-string cases. 3. bin/install.js:9232 — getUserShellPath trimmed entire stdout. A misconfigured rc file that prints a banner / motd / log line BEFORE the printf would pollute the result and incorrectly flip the cross-shell check to false. Take the LAST non-empty line (PATH itself is single-line) so noise can't hijack the probe. 4. Nitpick: the changeset PR placeholder — covered by (1). Verification: 12/12 pass on regression test (10 original + 2 new type-guard tests), 6770/6770 full suite, lint clean. * docs(#3020): JSDoc references printf %s "$PATH", not echo $PATH (CR) CodeRabbit caught two stale JSDoc references that still said `$SHELL -lc 'echo $PATH'` while the implementation uses `$SHELL -lc 'printf %s "$PATH"'`. echo is undesirable here because: - POSIX echo's behavior with `-n` / backslash escapes varies across shells (bash builtin vs /bin/echo vs zsh) and can introduce trailing-newline pollution that the per-line trim now papers over. - printf is portable and emits exactly the bytes given. Synced both stale doc strings: - bin/install.js:9211 (getUserShellPath JSDoc) - tests/bug-3020-install-shell-path-probe.test.cjs:27 (header) No behavior change — implementation already uses printf.
This commit is contained in:
5
.changeset/install-shell-path-probe.md
Normal file
5
.changeset/install-shell-path-probe.md
Normal file
@@ -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.
|
||||
@@ -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,
|
||||
|
||||
169
tests/bug-3020-install-shell-path-probe.test.cjs
Normal file
169
tests/bug-3020-install-shell-path-probe.test.cjs
Normal file
@@ -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 });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user