Files
get-shit-done/tests/install-path-detection.test.cjs
Tom Boucher 807db75d55 fix(#2620): detect HOME-relative PATH entries before suggesting absolute export (#2625)
* fix(#2620): detect HOME-relative PATH entries before suggesting absolute export

When the installer reported `gsd-sdk` not on PATH and suggested
appending an absolute `export PATH="/home/user/.npm-global/bin:$PATH"`
line to the user's rc file, a user who had the equivalent
`export PATH="$HOME/.npm-global/bin:$PATH"` already in their shell
profile would get a duplicate entry — the installer only compared the
absolute form.

Add `homePathCoveredByRc(globalBin, homeDir, rcFileNames?)` to
`bin/install.js` and export it for test-mode callers. The helper scans
`~/.zshrc`, `~/.bashrc`, `~/.bash_profile`, `~/.profile`, grepping each
file for `export PATH=` / bare `PATH=` lines and substituting the
common HOME forms (\$HOME, \${HOME}, leading ~/) with the real home
directory before comparing each resolved PATH segment against
globalBin. Trailing slashes are normalised so `.npm-global/bin/`
matches `.npm-global/bin`. Missing / unreadable / malformed rc files
are swallowed — the caller falls back to the existing absolute
suggestion.

Tests cover $HOME, \${HOME}, and ~/ forms, absolute match,
trailing-slash match, commented-out lines, missing rc files, and
unreadable rc files (directory where a file is expected).

Closes #2620

* fix(#2620): skip relative PATH segments in homePathCoveredByRc

CodeRabbit flagged that the helper unconditionally resolved every
non-$-containing segment against homeAbs via path.resolve(homeAbs, …),
which silently turns a bare relative segment like `bin` or
`node_modules/.bin` into `$HOME/bin` / `$HOME/node_modules/.bin`. That
is wrong: bare PATH segments depend on the shell's cwd at lookup time,
not on $HOME — so the helper was returning true for rc files that do
not actually cover globalBin.

Guard the compare with path.isAbsolute(expanded) after HOME expansion.
Only segments that are absolute on their own (or that became absolute
via $HOME / \${HOME} / ~ substitution) are compared against targetAbs.
Relative segments are skipped.

Add two regression tests covering a bare `bin` segment and a nested
`node_modules/.bin` segment; both previously returned true when home
happened to contain a matching subdirectory and now correctly return
false.

Closes #2620 (CodeRabbit follow-up)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(#2620): wire homePathCoveredByRc into installer suggestion path

CodeRabbit flagged that homePathCoveredByRc was added in the previous
commit but never called from the installer, so the user-facing PATH
warning stayed unchanged — users with `export PATH="$HOME/.npm-global/bin:$PATH"`
in their rc would still get a duplicate absolute-path suggestion.

Add `maybeSuggestPathExport(globalBin, homeDir)` that:
- skips silently when globalBin is already on process.env.PATH;
- prints a "try reopening your shell" diagnostic when homePathCoveredByRc
  returns true (the directory IS on PATH via an rc entry — just not in
  the current shell);
- otherwise falls through to the absolute-path
  `echo 'export PATH="…:$PATH"' >> ~/.zshrc` suggestion.

Call it from installSdkIfNeeded after the sdk/dist check succeeds,
resolving globalBin via `npm prefix -g` (plus `/bin` on POSIX). Swallow
any exec failure so the installer keeps working when npm is weird.

Export maybeSuggestPathExport for tests. Add three new regression tests
(installer-flow coverage per CodeRabbit nitpick):
- rc covers globalBin via $HOME form → no absolute suggestion emitted
- rc covers only an unrelated directory → absolute suggestion emitted
- globalBin already on process.env.PATH → no output at all

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-23 11:53:51 -04:00

293 lines
8.9 KiB
JavaScript

/**
* Regression test for #2620 — installer should not suggest adding an absolute
* PATH export when the user's rc file already contains a HOME-relative entry
* that covers the same directory.
*
* Covers `homePathCoveredByRc(globalBin, homeDir, rcFileNames?)` which parses
* each rc file's `export PATH=` lines, substitutes `$HOME` / `${HOME}` / `~`,
* and returns true when any resolved PATH entry equals globalBin.
*/
'use strict';
const { test, describe, before, after } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const os = require('os');
const path = require('path');
const INSTALL_PATH = path.join(__dirname, '..', 'bin', 'install.js');
function loadInstaller() {
process.env.GSD_TEST_MODE = '1';
delete require.cache[require.resolve(INSTALL_PATH)];
return require(INSTALL_PATH);
}
function createTempHome() {
return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-home-'));
}
function cleanup(dir) {
fs.rmSync(dir, { recursive: true, force: true });
}
describe('installer HOME-relative PATH detection (#2620)', () => {
let installer;
before(() => {
installer = loadInstaller();
});
test('homePathCoveredByRc is exported', () => {
assert.strictEqual(
typeof installer.homePathCoveredByRc,
'function',
'bin/install.js must export homePathCoveredByRc for #2620',
);
});
test('detects $HOME/.npm-global/bin pattern', () => {
const home = createTempHome();
try {
fs.writeFileSync(
path.join(home, '.zshrc'),
'export PATH="$HOME/.npm-global/bin:$PATH"\n',
);
const globalBin = path.join(home, '.npm-global', 'bin');
assert.strictEqual(installer.homePathCoveredByRc(globalBin, home), true);
} finally {
cleanup(home);
}
});
test('detects ${HOME}/.npm-global/bin pattern', () => {
const home = createTempHome();
try {
fs.writeFileSync(
path.join(home, '.bashrc'),
'export PATH="${HOME}/.npm-global/bin:$PATH"\n',
);
const globalBin = path.join(home, '.npm-global', 'bin');
assert.strictEqual(installer.homePathCoveredByRc(globalBin, home), true);
} finally {
cleanup(home);
}
});
test('detects ~/.npm-global/bin tilde form', () => {
const home = createTempHome();
try {
fs.writeFileSync(
path.join(home, '.profile'),
'export PATH=~/.npm-global/bin:$PATH\n',
);
const globalBin = path.join(home, '.npm-global', 'bin');
assert.strictEqual(installer.homePathCoveredByRc(globalBin, home), true);
} finally {
cleanup(home);
}
});
test('detects absolute path that exactly matches globalBin', () => {
const home = createTempHome();
try {
const globalBin = path.join(home, '.npm-global', 'bin');
fs.writeFileSync(
path.join(home, '.zshrc'),
`export PATH="${globalBin}:$PATH"\n`,
);
assert.strictEqual(installer.homePathCoveredByRc(globalBin, home), true);
} finally {
cleanup(home);
}
});
test('returns false when rc files exist but do not cover globalBin', () => {
const home = createTempHome();
try {
fs.writeFileSync(
path.join(home, '.zshrc'),
'export PATH="$HOME/.cargo/bin:$PATH"\nexport FOO=bar\n',
);
const globalBin = path.join(home, '.npm-global', 'bin');
assert.strictEqual(installer.homePathCoveredByRc(globalBin, home), false);
} finally {
cleanup(home);
}
});
test('returns false when no rc files exist', () => {
const home = createTempHome();
try {
const globalBin = path.join(home, '.npm-global', 'bin');
assert.strictEqual(installer.homePathCoveredByRc(globalBin, home), false);
} finally {
cleanup(home);
}
});
test('swallows unreadable rc files without throwing', () => {
const home = createTempHome();
try {
const rc = path.join(home, '.zshrc');
fs.mkdirSync(rc); // directory where a file is expected — reading throws
const globalBin = path.join(home, '.npm-global', 'bin');
assert.doesNotThrow(() => installer.homePathCoveredByRc(globalBin, home));
assert.strictEqual(installer.homePathCoveredByRc(globalBin, home), false);
} finally {
cleanup(home);
}
});
test('ignores commented-out export PATH lines', () => {
const home = createTempHome();
try {
fs.writeFileSync(
path.join(home, '.zshrc'),
'# export PATH="$HOME/.npm-global/bin:$PATH"\n',
);
const globalBin = path.join(home, '.npm-global', 'bin');
assert.strictEqual(installer.homePathCoveredByRc(globalBin, home), false);
} finally {
cleanup(home);
}
});
test('matches globalBin regardless of trailing slash', () => {
const home = createTempHome();
try {
fs.writeFileSync(
path.join(home, '.zshrc'),
'export PATH="$HOME/.npm-global/bin/:$PATH"\n',
);
const globalBin = path.join(home, '.npm-global', 'bin');
assert.strictEqual(installer.homePathCoveredByRc(globalBin, home), true);
} finally {
cleanup(home);
}
});
// CodeRabbit finding: bare relative PATH segments (e.g. `bin`) must not be
// resolved against $HOME. Relative segments depend on the shell's cwd at
// lookup time and are unrelated to $HOME/bin.
test('does not treat bare relative PATH segment as HOME-relative', () => {
const home = createTempHome();
try {
fs.writeFileSync(
path.join(home, '.zshrc'),
'export PATH="bin:$PATH"\n',
);
const globalBin = path.join(home, 'bin');
assert.strictEqual(
installer.homePathCoveredByRc(globalBin, home),
false,
'relative PATH segments must not be resolved against $HOME',
);
} finally {
cleanup(home);
}
});
test('does not treat nested relative PATH segment as HOME-relative', () => {
const home = createTempHome();
try {
fs.writeFileSync(
path.join(home, '.zshrc'),
'export PATH="node_modules/.bin:$PATH"\n',
);
const globalBin = path.join(home, 'node_modules', '.bin');
assert.strictEqual(
installer.homePathCoveredByRc(globalBin, home),
false,
);
} finally {
cleanup(home);
}
});
// CodeRabbit actionable 1 + nitpick: the installer's PATH-export
// suggestion banner must be suppressed when an rc file already covers
// globalBin via a HOME-relative entry.
test('maybeSuggestPathExport suppresses suggestion when rc covers globalBin', () => {
const home = createTempHome();
try {
const globalBin = path.join(home, '.npm-global', 'bin');
fs.mkdirSync(globalBin, { recursive: true });
fs.writeFileSync(
path.join(home, '.zshrc'),
'export PATH="$HOME/.npm-global/bin:$PATH"\n',
);
const logs = [];
const origLog = console.log;
console.log = (...args) => { logs.push(args.join(' ')); };
try {
installer.maybeSuggestPathExport(globalBin, home);
} finally {
console.log = origLog;
}
const joined = logs.join('\n');
assert.ok(
!/echo 'export PATH=/.test(joined),
`installer should not emit absolute export suggestion; got:\n${joined}`,
);
} finally {
cleanup(home);
}
});
test('maybeSuggestPathExport emits suggestion when rc does not cover globalBin', () => {
const home = createTempHome();
try {
const globalBin = path.join(home, '.npm-global', 'bin');
fs.mkdirSync(globalBin, { recursive: true });
fs.writeFileSync(
path.join(home, '.zshrc'),
'export PATH="$HOME/.cargo/bin:$PATH"\n',
);
const logs = [];
const origLog = console.log;
console.log = (...args) => { logs.push(args.join(' ')); };
try {
installer.maybeSuggestPathExport(globalBin, home);
} finally {
console.log = origLog;
}
const joined = logs.join('\n');
assert.ok(
/echo 'export PATH=/.test(joined),
`installer should emit absolute export suggestion when rc does not cover globalBin; got:\n${joined}`,
);
} finally {
cleanup(home);
}
});
test('maybeSuggestPathExport is a no-op when globalBin already on process.env.PATH', () => {
const home = createTempHome();
const origPath = process.env.PATH;
try {
const globalBin = path.join(home, '.npm-global', 'bin');
fs.mkdirSync(globalBin, { recursive: true });
process.env.PATH = `${globalBin}${path.delimiter}${origPath || ''}`;
const logs = [];
const origLog = console.log;
console.log = (...args) => { logs.push(args.join(' ')); };
try {
installer.maybeSuggestPathExport(globalBin, home);
} finally {
console.log = origLog;
}
assert.strictEqual(logs.length, 0, `expected no output when on PATH; got:\n${logs.join('\n')}`);
} finally {
if (origPath === undefined) delete process.env.PATH; else process.env.PATH = origPath;
cleanup(home);
}
});
});