fix(#2649): installer fail-fast when sdk/dist missing in npx cache (#2667)

Root cause shared with #2647: a broken 1.38.3 tarball shipped without
sdk/dist/. The pre-#2441-decouple installer reacted by running
spawnSync('npm.cmd', ['install'], { cwd: sdkDir }) inside the npx cache
on Windows, where the cache is read-only, producing the misleading
"Failed to npm install in sdk/" error.

Defensive changes here (user-facing behavior only; packaging fix lives
in the sibling PR for #2647):

- Classify the install context (classifySdkInstall): detect npx cache
  paths, node_modules-based installs, and dev clones via path heuristics
  plus a side-effect-free write probe. Exported for test.
- Rewrite the dist-missing error to branch on context:
    tarball + npxCache -> "don't touch npx cache; npm i -g ...@latest"
    tarball (other)    -> upgrade path + clone-build escape hatch
    dev-clone          -> keep existing cd sdk && npm install && npm run build
- Preserve the invariant that the installer never shells out to
  npm install itself — users always drive that.
- Add tests/bug-2649-sdk-fail-fast.test.cjs covering the classifier and
  both failure messages, with spawnSync/execSync interceptors that
  assert no nested npm install is attempted.

Cross-ref: #2647 (packaging).

Fixes #2649

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-04-24 18:05:04 -04:00
committed by GitHub
parent a72bebb379
commit 8caa7d4c3a
2 changed files with 277 additions and 6 deletions

View File

@@ -7023,8 +7023,72 @@ function maybeSuggestPathExport(globalBin, homeDir) {
* --no-sdk skips the check entirely (back-compat).
* --sdk forces the check even if it would otherwise be skipped.
*/
function installSdkIfNeeded() {
if (hasNoSdk) {
/**
* Classify the install context for the SDK directory.
*
* Distinguishes three shapes the installer must handle differently when
* `sdk/dist/` is missing:
*
* - `tarball` + `npxCache: true`
* User ran `npx get-shit-done-cc@latest`. sdk/ lives under
* `<npm-cache>/_npx/<hash>/node_modules/get-shit-done-cc/sdk` which
* is treated as read-only by npm/npx on Windows (#2649). We MUST
* NOT attempt a nested `npm install` there — it will fail with
* EACCES/EPERM and produce the misleading "Failed to npm install
* in sdk/" error the user reported. Point at the global upgrade.
*
* - `tarball` + `npxCache: false`
* User ran a global install (`npm i -g get-shit-done-cc`). sdk/dist
* ships in the published tarball; if it's missing, the published
* artifact itself is broken (see #2647). Same user-facing fix:
* upgrade to latest.
*
* - `dev-clone`
* Developer running from a git clone. Keep the existing "cd sdk &&
* npm install && npm run build" hint — the user is expected to run
* that themselves. The installer itself never shells out to npm.
*
* Detection heuristics are path-based and side-effect-free: we look for
* `_npx` and `node_modules` segments that indicate a packaged install,
* and for a `.git` directory nearby that indicates a clone. A best-effort
* write probe detects read-only filesystems (tmpfile create + unlink);
* probe failures are treated as read-only.
*/
function classifySdkInstall(sdkDir) {
const path = require('path');
const fs = require('fs');
const segments = sdkDir.split(/[\\/]+/);
const npxCache = segments.includes('_npx');
const inNodeModules = segments.includes('node_modules');
const parent = path.dirname(sdkDir);
const hasGitNearby = fs.existsSync(path.join(parent, '.git'));
let mode;
if (hasGitNearby && !npxCache && !inNodeModules) {
mode = 'dev-clone';
} else if (npxCache || inNodeModules) {
mode = 'tarball';
} else {
mode = 'dev-clone';
}
let readOnly = npxCache; // assume true for npx cache
if (!readOnly) {
try {
const probe = path.join(sdkDir, `.gsd-write-probe-${process.pid}`);
fs.writeFileSync(probe, '');
fs.unlinkSync(probe);
} catch {
readOnly = true;
}
}
return { mode, npxCache, readOnly };
}
function installSdkIfNeeded(opts) {
opts = opts || {};
if (hasNoSdk && !opts.sdkDir) {
console.log(`\n ${dim}Skipping GSD SDK check (--no-sdk)${reset}`);
return;
}
@@ -7032,9 +7096,11 @@ function installSdkIfNeeded() {
const path = require('path');
const fs = require('fs');
const sdkCliPath = path.resolve(__dirname, '..', 'sdk', 'dist', 'cli.js');
const sdkDir = opts.sdkDir || path.resolve(__dirname, '..', 'sdk');
const sdkCliPath = path.join(sdkDir, 'dist', 'cli.js');
if (!fs.existsSync(sdkCliPath)) {
const ctx = classifySdkInstall(sdkDir);
const bar = '━'.repeat(72);
const redBold = `${red}${bold}`;
console.error('');
@@ -7043,9 +7109,33 @@ function installSdkIfNeeded() {
console.error(`${redBold}${bar}${reset}`);
console.error(` ${red}Reason:${reset} sdk/dist/cli.js not found at ${sdkCliPath}`);
console.error('');
console.error(` This should not happen with a published tarball install.`);
console.error(` If you are running from a git clone, build the SDK first:`);
console.error(` ${cyan}cd sdk && npm install && npm run build${reset}`);
if (ctx.mode === 'tarball') {
// User install (including `npx get-shit-done-cc@latest`, which stages
// a read-only tarball under the npx cache). The sdk/dist/ artifact
// should ship in the published tarball. If it's missing, the only
// sane fix from the user's side is a fresh global install of a
// version that includes dist/. Do NOT attempt a nested `npm install`
// inside the (read-only) npx cache — that's the #2649 failure mode.
if (ctx.npxCache) {
console.error(` Detected read-only npx cache install (${dim}${sdkDir}${reset}).`);
console.error(` The installer will ${bold}not${reset} attempt \`npm install\` inside the npx cache.`);
console.error('');
} else {
console.error(` The published tarball appears to be missing sdk/dist/ (see #2647).`);
console.error('');
}
console.error(` Fix: install a version that ships sdk/dist/ globally:`);
console.error(` ${cyan}npm install -g get-shit-done-cc@latest${reset}`);
console.error(` Or, if you prefer a one-shot run, clear the npx cache first:`);
console.error(` ${cyan}npx --yes get-shit-done-cc@latest${reset}`);
console.error(` Or build from source (git clone):`);
console.error(` ${cyan}git clone https://github.com/gsd-build/get-shit-done && cd get-shit-done/sdk && npm install && npm run build${reset}`);
} else {
// Dev clone: keep the existing build-from-source hint.
console.error(` Running from a git clone — build the SDK first:`);
console.error(` ${cyan}cd sdk && npm install && npm run build${reset}`);
}
console.error(`${redBold}${bar}${reset}`);
console.error('');
process.exit(1);
@@ -7146,6 +7236,8 @@ if (process.env.GSD_TEST_MODE) {
readGsdEffectiveModelOverrides,
install,
uninstall,
installSdkIfNeeded,
classifySdkInstall,
convertClaudeCommandToCodexSkill,
convertClaudeToOpencodeFrontmatter,
convertClaudeToKiloFrontmatter,