fix(#3011): actionable SDK-not-on-PATH diagnostic with shim location and shell-specific commands (#3014)
* fix(#3011): actionable SDK-not-on-PATH diagnostic with shim location and shell-specific commands The previous diagnostic was a generic 'GSD SDK files are present but gsd-sdk is not on your PATH' message with no concrete path or shell-specific PATH-export command. Windows users reported that they couldn't find where the shim was written and didn't know how to add it to PATH for each shell (PowerShell vs cmd.exe vs Git Bash vs WSL all read PATH from different sources). New formatSdkPathDiagnostic({ shimDir, platform, runDir }) helper returns a typed IR: - shimLocationLine: explicit 'Shim written to: <path>' - actionLines: platform-specific PATH-export commands - Windows: 3 lines (PowerShell, cmd.exe, Git Bash with backslash->/ translation for bash compatibility) - POSIX: 1 line (export PATH=...) - npxNoteLines: 'you're running via npx ... npm install -g instead' when runDir is under an _npx cache segment (where the shim may be written to a temp dir that won't persist for the user's interactive shell) - isNpx, isWin32: structured booleans for assertions Renderer in install.js just emits each line. Tests assert on the typed IR fields directly (no source-grep, no console-output parsing). Tests: 12 cases across 5 suites covering Windows shell flavors (PowerShell preserves backslashes, Git Bash translates to forward), POSIX exports, null-shimDir fallback to npm install -g advice, npx detection on both path-separator conventions, and IR shape contract. Closes #3011 * fix(#3011): cmd.exe guidance uses powershell -Command, not setx CodeRabbit flagged the cmd.exe action line as a Major Windows correctness bug: setx PATH "${shimDir}; %PATH%" Two failure modes: 1. setx silently truncates the registry value above 1024 chars, permanently storing the truncated PATH and breaking applications until restored from the registry backup or fixed manually. 2. %PATH% expands to its current literal value at the moment setx runs, and the result is written as REG_SZ instead of REG_EXPAND_SZ. Lazy references like %SystemRoot% are baked in as literals, so future changes to those variables stop propagating. Replace with the same SetEnvironmentVariable call already used for the PowerShell line, invoked through `powershell -Command` so cmd.exe users get a safe command without us recommending two different APIs. * fix(#3011): escape shimDir for PowerShell, bash, and POSIX export CodeRabbit (Minor): a Windows username with a single quote (e.g. "C:\Users\O'Neil\AppData\Roaming\npm") would interpolate raw into the suggested commands, producing unparseable shell input the user can't fix without understanding the bug. Each shell context needs a different escape: - PowerShell single-quoted strings: '' is the literal-quote escape. Apply to both the PowerShell line and the cmd.exe line (which delegates to PowerShell). - Git Bash, where the path lives inside an outer single-quoted echo: '\'' (close-quote, escaped-quote, reopen-quote) embeds a literal single quote. The slash-conversion (\\ → /) still applies first. - POSIX export (Linux/macOS) inside double quotes: escape \, $, ", and backtick so the path is copied verbatim. $PATH lives outside the escape and still expands at paste time. Regression test: bug-3011-sdk-path-diagnostic.test.cjs locks in the expected escape sequence for all three shell flavors.
This commit is contained in:
Tom Boucher
@@ -9078,6 +9078,12 @@ function installSdkIfNeeded(opts) {
|
||||
const shimSrc = path.resolve(__dirname, 'gsd-sdk.js');
|
||||
let onPath = isGsdSdkOnPath();
|
||||
|
||||
// Track WHERE we wrote the shim so the diagnostic can be specific even
|
||||
// when isGsdSdkOnPath() returns false because the write target isn't on
|
||||
// PATH (#3011: Windows users hit this when npm's global bin dir is
|
||||
// populated but not on every shell's PATH — Git Bash vs PowerShell vs
|
||||
// cmd.exe each read PATH from different sources).
|
||||
let shimDir = null;
|
||||
if (!onPath) {
|
||||
// Try to materialize the shim into a user-writable PATH location so the
|
||||
// installer can deliver on the success message without requiring the user
|
||||
@@ -9086,6 +9092,7 @@ function installSdkIfNeeded(opts) {
|
||||
// it's not on PATH (then a follow-up suggestion is printed).
|
||||
const linked = trySelfLinkGsdSdk(shimSrc);
|
||||
if (linked) {
|
||||
shimDir = path.dirname(linked);
|
||||
onPath = isGsdSdkOnPath();
|
||||
if (onPath) {
|
||||
console.log(` ${dim}↪ linked gsd-sdk → ${linked}${reset}`);
|
||||
@@ -9096,12 +9103,23 @@ function installSdkIfNeeded(opts) {
|
||||
if (onPath) {
|
||||
console.log(` ${green}✓${reset} GSD SDK ready (sdk/dist/cli.js)`);
|
||||
} else {
|
||||
// #3011: actionable diagnostic. The previous shape printed a generic
|
||||
// "not on your PATH" message that didn't tell the user where to look.
|
||||
// formatSdkPathDiagnostic produces a typed IR that we then render to
|
||||
// stdout; tests assert on the IR (no source-grep, no console capture).
|
||||
const ir = formatSdkPathDiagnostic({
|
||||
shimDir,
|
||||
platform: process.platform,
|
||||
runDir: __dirname,
|
||||
});
|
||||
console.log('');
|
||||
console.log(` ${yellow}⚠${reset} GSD SDK files are present but ${bold}gsd-sdk${reset} is not on your PATH.`);
|
||||
console.log(` Workflows that call ${cyan}gsd-sdk query …${reset} will fail with "command not found".`);
|
||||
console.log(` Install globally to materialize the bin symlink:`);
|
||||
console.log(` ${cyan}npm install -g get-shit-done-cc${reset}`);
|
||||
console.log(` Or add a directory containing the shim to your PATH manually.`);
|
||||
if (ir.shimLocationLine) console.log(` ${ir.shimLocationLine}`);
|
||||
for (const line of ir.actionLines) console.log(` ${line}`);
|
||||
if (ir.npxNoteLines.length > 0) {
|
||||
for (const line of ir.npxNoteLines) console.log(` ${line}`);
|
||||
}
|
||||
console.log('');
|
||||
}
|
||||
|
||||
@@ -9275,6 +9293,75 @@ function buildWindowsShimTriple(shimSrc) {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* #3011: pure builder for the SDK-not-on-PATH diagnostic. Takes the
|
||||
* resolved shim directory (or null if write failed), the current platform,
|
||||
* and the install.js __dirname (used to detect npx-cache invocation).
|
||||
* Returns a typed IR with:
|
||||
* - shimLocationLine: prose mentioning where the shim is (or empty if no
|
||||
* write happened)
|
||||
* - actionLines: ordered list of commands the user can run to add the
|
||||
* shim dir to their PATH (platform-specific shells), or fallback to
|
||||
* `npm install -g` advice when no shim was written
|
||||
* - npxNoteLines: ordered list of lines warning about npx persistence
|
||||
* when runDir is under an `_npx` cache segment
|
||||
*
|
||||
* Tests assert on the typed fields (paths/commands), not on rendered
|
||||
* console output. Pure function — no fs, no spawn, no console.
|
||||
*/
|
||||
function formatSdkPathDiagnostic({ shimDir, platform, runDir }) {
|
||||
const path = require('path');
|
||||
const isWin32 = platform === 'win32';
|
||||
// Detect either path separator — the test fixtures pass Windows-style
|
||||
// paths while running on POSIX, and real users hit either depending on
|
||||
// their npm/npx setup. Anchor on `_npx` between separators.
|
||||
const isNpx = typeof runDir === 'string' &&
|
||||
(runDir.includes('/_npx/') || runDir.includes('\\_npx\\'));
|
||||
|
||||
const shimLocationLine = shimDir ? `Shim written to: ${shimDir}` : '';
|
||||
const actionLines = [];
|
||||
|
||||
if (shimDir) {
|
||||
// Escape shimDir for each shell context. A path containing a single
|
||||
// quote (e.g. C:\Users\O'Neil\AppData\...) would otherwise generate
|
||||
// broken commands the user can't paste:
|
||||
// - PowerShell single-quoted string: '' escapes a literal single quote
|
||||
// - bash inside outer single quotes: '\'' (close, escaped quote, reopen)
|
||||
// - POSIX export inside double quotes: escape \ $ " ` so the path is
|
||||
// copied verbatim and $PATH (which is OUTSIDE the escaped substring)
|
||||
// still expands at paste time.
|
||||
const psShimDir = shimDir.replace(/'/g, "''");
|
||||
const bashShimDir = shimDir.replace(/\\/g, '/').replace(/'/g, "'\\''");
|
||||
const posixShimDir = shimDir.replace(/[\\$"`]/g, '\\$&');
|
||||
actionLines.push('Add that directory to your PATH and restart your shell.');
|
||||
if (isWin32) {
|
||||
actionLines.push(`PowerShell: [Environment]::SetEnvironmentVariable('PATH', '${psShimDir};' + [Environment]::GetEnvironmentVariable('PATH', 'User'), 'User')`);
|
||||
// setx PATH "...;%PATH%" silently truncates above 1024 chars and
|
||||
// expands %PATH% / %SystemRoot% to literals (turning REG_EXPAND_SZ
|
||||
// into REG_SZ), permanently breaking lazy variable references.
|
||||
// Invoke PowerShell from cmd.exe with the same SetEnvironmentVariable
|
||||
// call as the PowerShell line so cmd.exe users get a safe command.
|
||||
actionLines.push(`cmd.exe : powershell -Command "[Environment]::SetEnvironmentVariable('PATH', '${psShimDir};' + [Environment]::GetEnvironmentVariable('PATH', 'User'), 'User')"`);
|
||||
actionLines.push(`Git Bash : echo 'export PATH="${bashShimDir}:$PATH"' >> ~/.bashrc`);
|
||||
} else {
|
||||
actionLines.push(`export PATH="${posixShimDir}:$PATH"`);
|
||||
}
|
||||
} else {
|
||||
actionLines.push('Could not locate a writable PATH directory to install the shim.');
|
||||
actionLines.push('Install globally to materialize the bin symlink:');
|
||||
actionLines.push('npm install -g get-shit-done-cc');
|
||||
}
|
||||
|
||||
const npxNoteLines = isNpx
|
||||
? [
|
||||
"Note: you're running via npx. For a persistent shim,",
|
||||
'install globally instead: npm install -g get-shit-done-cc',
|
||||
]
|
||||
: [];
|
||||
|
||||
return { shimLocationLine, actionLines, npxNoteLines, isNpx, isWin32 };
|
||||
}
|
||||
|
||||
function trySelfLinkGsdSdkWindows(shimSrc) {
|
||||
const path = require('path');
|
||||
const fs = require('fs');
|
||||
@@ -9475,6 +9562,7 @@ if (process.env.GSD_TEST_MODE) {
|
||||
trySelfLinkGsdSdk,
|
||||
trySelfLinkGsdSdkWindows,
|
||||
buildWindowsShimTriple,
|
||||
formatSdkPathDiagnostic,
|
||||
isGsdSdkOnPath,
|
||||
homePathCoveredByRc,
|
||||
maybeSuggestPathExport,
|
||||
|
||||
Reference in New Issue
Block a user