Merge pull request #3547 from gsd-build/fix/3541-first-time-baseline-migration-prompt-use

fix(3541): resolve prompt-user migration actions in non-TTY runs; improve error grouping

.changeset/3541-installer-migration-prompt-user-resolution.md

@@ -0,0 +1,6 @@
+---
+type: Fixed
+pr: 3547
+---
+
+**Installer migration no longer hangs `/gsd:update` on leftover GSD-looking files** — non-TTY installer runs now default-resolve `prompt-user` migration actions by classification (stale SDK build artifacts under `get-shit-done/sdk/{dist,src}/gsd-*` default to `remove`; user-facing `skills/gsd-*/SKILL.md` defaults to `keep`) and log each resolution. Anything that cannot be safely defaulted still blocks, but the error message now groups blocked paths by reason, lists the documented choices, and names the `GSD_INSTALLER_MIGRATION_RESOLVE` env var as the non-interactive resolution surface. (#3541)

bin/install.js

@@ -99,11 +99,13 @@ const {
   stageAgentsForProfile,
 } = require(path.join(_gsdLibDir, 'install-profiles.cjs'));
 const {
+  applyInstallerMigrationPlan,
   discoverInstallerMigrations,
   runInstallerMigrations,
 } = require(path.join(_gsdLibDir, 'installer-migrations.cjs'));
 const {
   assertInstallerMigrationsUnblocked,
+  resolveInstallerMigrationPromptsForNonTty,
   summarizeInstallerMigrationResult,
 } = require(path.join(_gsdLibDir, 'installer-migration-report.cjs'));
 
@@ -8030,6 +8032,43 @@ function install(isGlobal, runtime = 'claude', options = {}) {
     migrations: options.installerMigrations,
     baselineScan: true,
   });
+  // #3541: non-interactive runs (typical /gsd-update via Claude Code) have
+  // no stdin TTY and therefore no way to answer prompt-user migration
+  // actions. Resolve safe categories by classification (stale SDK build
+  // artifacts → remove; user-facing skills → keep) and log every
+  // resolution; anything that cannot be safely defaulted falls through
+  // to assertInstallerMigrationsUnblocked, which now emits a grouped
+  // error with the documented resolution path.
+  const _migrationIsTty = process.stdin && process.stdin.isTTY === true;
+  if (!_migrationIsTty &&
+      Array.isArray(installerMigrationResult.blocked) &&
+      installerMigrationResult.blocked.length > 0 &&
+      installerMigrationResult.plan &&
+      Array.isArray(installerMigrationResult.plan.actions)) {
+    const { resolutions } = resolveInstallerMigrationPromptsForNonTty(
+      installerMigrationResult,
+      { isTty: false }
+    );
+    for (const entry of resolutions) {
+      console.log(
+        `  ↪ installer-migration auto-resolved: ${entry.relPath} → ${entry.choice} ` +
+        `(category=${entry.category}, source=${entry.source})`
+      );
+    }
+    // If we resolved anything, the original run returned early without
+    // applying the (now-unblocked) plan — apply it here.
+    if (resolutions.length > 0 && installerMigrationResult.plan.blocked.length === 0) {
+      const applyResult = applyInstallerMigrationPlan({
+        configDir: targetDir,
+        plan: installerMigrationResult.plan,
+      });
+      installerMigrationResult = {
+        ...installerMigrationResult,
+        ...applyResult,
+        blocked: [],
+      };
+    }
+  }
   reportInstallerMigrationResult(installerMigrationResult);
   assertInstallerMigrationsUnblocked(installerMigrationResult);
 

get-shit-done/bin/lib/installer-migration-report.cjs

@@ -1,5 +1,20 @@
 'use strict';
 
+// Resolution environment variable surface for #3541 — when the installer
+// runs without a TTY (typical /gsd:update path via Claude Code or any
+// scripted update), prompt-user migration actions cannot be answered
+// interactively. We resolve them by classification:
+//   - Stale SDK build artifacts (get-shit-done/sdk/{dist,src}/gsd-*):
+//     default `remove`. Fresh install supplies replacements.
+//   - User-facing skill anchors (skills/gsd-*/SKILL.md): default `keep`.
+//     User-owned content is preserved.
+// Anything else: fall through to the hard assertion with an improved,
+// grouped, actionable error message.
+//
+// docs/installer-migrations.md#prompt-user-resolution for the spec.
+const RESOLUTION_ENV_VAR = 'GSD_INSTALLER_MIGRATION_RESOLVE';
+const VALID_CHOICES = ['keep', 'remove'];
+
 function installerMigrationActionLabel(action) {
   if (!action || !action.type) return 'skipped';
   if (action.type === 'backup-and-remove') return 'backed up and removed';
@@ -83,14 +98,213 @@ function summarizeInstallerMigrationResult(result) {
   };
 }
 
+// Classify a blocked prompt-user action into one of the safe-default
+// categories. Returns null when no safe default applies — caller must
+// fall back to the hard assertion / interactive prompt for those.
+//
+// Stale SDK build artifacts live under get-shit-done/sdk/{dist,src}/
+// and are regenerated on every install, so removing them is lossless.
+// User-facing skill anchors are the .md files that surface as commands
+// to the user — these are user-owned and must be kept.
+function classifyPromptUserAction(action) {
+  const relPath = action && action.relPath;
+  if (typeof relPath !== 'string' || !relPath) return null;
+  if (/^get-shit-done\/sdk\/(dist|src)\//.test(relPath)) {
+    return { category: 'stale-sdk-build-artifact', choice: 'remove' };
+  }
+  if (/^skills\/gsd-[^/]+\/SKILL\.md$/.test(relPath)) {
+    return { category: 'user-facing-skill', choice: 'keep' };
+  }
+  return null;
+}
+
+// Convert a blocked prompt-user action into a concrete plan action.
+// `keep` → baseline-preserve-user (idempotent — already on disk).
+// `remove` → backup-and-remove (safe: keeps a rollback copy in the
+// migration journal under gsd-migration-journal/<runId>-backups/).
+function materializeResolution(action, choice) {
+  const base = {
+    migrationId: action.migrationId,
+    migrationChecksum: action.migrationChecksum,
+    relPath: action.relPath,
+    reason: action.reason,
+    classification: action.classification,
+    originalHash: action.originalHash || null,
+    currentHash: action.currentHash || null,
+    requestedType: 'prompt-user',
+  };
+  if (choice === 'keep') {
+    return { ...base, type: 'baseline-preserve-user' };
+  }
+  // 'remove'
+  return { ...base, type: 'backup-and-remove', backupRelPath: null };
+}
+
+function normalizeResolutionChoice(rawValue) {
+  if (typeof rawValue !== 'string') return null;
+  const normalized = rawValue.trim().toLowerCase();
+  return VALID_CHOICES.includes(normalized) ? normalized : null;
+}
+
+function actionSupportsChoice(action, choice) {
+  if (!action || !choice) return false;
+  if (!Array.isArray(action.choices) || action.choices.length === 0) {
+    return VALID_CHOICES.includes(choice);
+  }
+  return action.choices.includes(choice);
+}
+
+// Resolve prompt-user actions when stdin is not a TTY. Mutates the
+// passed result so:
+//   - resolved actions are appended to plan.actions in their concrete
+//     form (baseline-preserve-user / backup-and-remove);
+//   - result.blocked and plan.blocked are filtered to actions that
+//     could NOT be safely defaulted (caller must still handle those).
+// Returns { result, resolutions } where `resolutions` is the structured
+// log of every defaulted resolution.
+function resolveInstallerMigrationPromptsForNonTty(result, options = {}) {
+  if (!result || typeof result !== 'object') {
+    return { result, resolutions: [] };
+  }
+  const blocked = blockedInstallerMigrationActions(result);
+  if (blocked.length === 0) {
+    return { result, resolutions: [] };
+  }
+  const isTty = options.isTty === true;
+  if (isTty) {
+    // Honour interactive prompting paths (not implemented yet — the
+    // hard throw is still the right behaviour for TTY runs); resolver
+    // only fires when the installer cannot interactively ask.
+    return { result, resolutions: [] };
+  }
+
+  const env =
+    options && options.env && typeof options.env === 'object'
+      ? options.env
+      : process.env;
+  const envChoice = normalizeResolutionChoice(env && env[RESOLUTION_ENV_VAR]);
+  const resolutions = [];
+  const unresolved = [];
+
+  for (const action of blocked) {
+    if (action && action.type === 'prompt-user') {
+      let category = null;
+      let choice = null;
+      let source = null;
+      if (envChoice && actionSupportsChoice(action, envChoice)) {
+        category = 'operator-override';
+        choice = envChoice;
+        source = RESOLUTION_ENV_VAR;
+      } else {
+        const classification = classifyPromptUserAction(action);
+        if (classification) {
+          category = classification.category;
+          choice = classification.choice;
+          source = 'non-tty-default';
+        }
+      }
+
+      if (choice) {
+        const resolved = materializeResolution(action, choice);
+        // Inject the concrete action into plan.actions so the apply
+        // step picks it up.
+        if (result.plan && Array.isArray(result.plan.actions)) {
+          result.plan.actions.push(resolved);
+        }
+        resolutions.push({
+          relPath: action.relPath,
+          category,
+          choice,
+          reason: action.reason,
+          resolvedActionType: resolved.type,
+          source,
+        });
+        continue;
+      }
+    }
+    unresolved.push(action);
+  }
+
+  // Mutate both the top-level and plan.blocked surfaces so downstream
+  // callers (assertInstallerMigrationsUnblocked, summarizers) see the
+  // post-resolution state.
+  if (Array.isArray(result.blocked)) {
+    result.blocked = unresolved;
+  }
+  if (result.plan && Array.isArray(result.plan.blocked)) {
+    result.plan.blocked = unresolved;
+  }
+
+  return { result, resolutions };
+}
+
+// Group blocked prompt-user actions by their `reason` so the operator
+// sees one summary line per cause instead of N path lines for the
+// same underlying issue.
+function groupBlockedByReason(blocked) {
+  const byReason = new Map();
+  for (const action of blocked) {
+    const reason = (action && action.reason) || 'no reason given';
+    if (!byReason.has(reason)) byReason.set(reason, []);
+    byReason.get(reason).push(action);
+  }
+  return byReason;
+}
+
+function describeChoicesForActions(blocked) {
+  const choiceSet = new Set();
+  for (const action of blocked) {
+    if (action && Array.isArray(action.choices)) {
+      for (const choice of action.choices) choiceSet.add(choice);
+    }
+  }
+  if (choiceSet.size === 0) {
+    for (const fallback of VALID_CHOICES) choiceSet.add(fallback);
+  }
+  return [...choiceSet];
+}
+
+function buildBlockedErrorMessage(blocked) {
+  const byReason = groupBlockedByReason(blocked);
+  const totalFiles = blocked.length;
+  const choices = describeChoicesForActions(blocked);
+
+  const lines = [
+    `installer migration blocked pending user choice: ${totalFiles} file${totalFiles === 1 ? '' : 's'} need a decision`,
+    `  choices: [${choices.join(', ')}]`,
+  ];
+  for (const [reason, actions] of byReason) {
+    lines.push(`  - ${actions.length} file${actions.length === 1 ? '' : 's'}: ${reason}`);
+    // Show up to 3 sample paths so operators can spot which files are
+    // affected without dumping a thousand-line wall when SDK build
+    // artifacts leak.
+    const sample = actions.slice(0, 3).map((a) => a.relPath);
+    if (sample.length > 0) {
+      lines.push(`      e.g. ${sample.join(', ')}${actions.length > sample.length ? `, ... (+${actions.length - sample.length} more)` : ''}`);
+    }
+  }
+  lines.push(
+    `  resolve non-interactively by setting ${RESOLUTION_ENV_VAR}=<choice> ` +
+    `(or run the installer in a TTY to be prompted per file).`
+  );
+  return lines.join('\n');
+}
+
 function assertInstallerMigrationsUnblocked(result) {
   const blocked = blockedInstallerMigrationActions(result);
   if (blocked.length === 0) return;
-  const paths = blocked.map((action) => action.relPath).join(', ');
-  throw new Error(`installer migration blocked pending user choice: ${paths}`);
+  const message = buildBlockedErrorMessage(blocked);
+  const error = new Error(message);
+  error.blocked = blocked;
+  error.blockedByReason = Object.fromEntries(groupBlockedByReason(blocked));
+  error.resolutionEnvVar = RESOLUTION_ENV_VAR;
+  throw error;
 }
 
 module.exports = {
+  RESOLUTION_ENV_VAR,
   assertInstallerMigrationsUnblocked,
+  classifyPromptUserAction,
+  resolveInstallerMigrationPromptsForNonTty,
   summarizeInstallerMigrationResult,
 };

tests/bug-3541-installer-migration-prompt-user-resolution.test.cjs

@@ -0,0 +1,239 @@
+/**
+ * Regression test for #3541: first-time-baseline installer migration
+ * `prompt-user` actions threw hard with no resolution path, making
+ * `/gsd-update` unrecoverable when leftover `gsd-*` files were classified
+ * as `stale-gsd-looking`.
+ *
+ * Fix shape (per triage brief):
+ *   A. Classify-and-default for safe categories - stale SDK build
+ *      artifacts default to "remove"; user-facing skills/gsd-asterisk/SKILL.md
+ *      defaults to "keep". Each resolution is logged.
+ *   B. Improved error message when an unresolved prompt-user action
+ *      remains: lists choices, suggests the resolution path, groups
+ *      blocked paths by reason.
+ *
+ * Behavioural test — exercises the actual installer migration code paths
+ * via the public `runInstallerMigrations` + new resolver entry points.
+ * No source-grep (per CONTEXT.md L98–101 / RULESET.TESTS).
+ */
+
+'use strict';
+
+process.env.GSD_TEST_MODE = '1';
+
+const { describe, test, beforeEach, afterEach } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+const crypto = require('node:crypto');
+
+const {
+  runInstallerMigrations,
+} = require('../get-shit-done/bin/lib/installer-migrations.cjs');
+const {
+  assertInstallerMigrationsUnblocked,
+  resolveInstallerMigrationPromptsForNonTty,
+} = require('../get-shit-done/bin/lib/installer-migration-report.cjs');
+const { createTempDir, cleanup } = require('./helpers.cjs');
+
+function sha256(content) {
+  return crypto.createHash('sha256').update(content).digest('hex');
+}
+
+function writeFile(root, relPath, content) {
+  const fullPath = path.join(root, relPath);
+  fs.mkdirSync(path.dirname(fullPath), { recursive: true });
+  fs.writeFileSync(fullPath, content, 'utf8');
+}
+
+function writeManifest(root, files) {
+  fs.writeFileSync(
+    path.join(root, 'gsd-file-manifest.json'),
+    JSON.stringify({
+      version: '1.41.2',
+      timestamp: '2026-05-10T00:00:00.000Z',
+      mode: 'full',
+      files,
+    }, null, 2),
+    'utf8'
+  );
+}
+
+describe('#3541: installer migration prompt-user non-TTY resolution', { concurrency: false }, () => {
+  let configDir;
+
+  beforeEach(() => {
+    configDir = createTempDir('gsd-3541-');
+  });
+
+  afterEach(() => {
+    cleanup(configDir);
+  });
+
+  test('Test A: non-TTY default resolution removes stale SDK artifacts and keeps user skills', () => {
+    // Stale SDK build artifact: replicates the 1.41.2 → 1.42.2 upgrade where
+    // 24 stale `get-shit-done/sdk/{dist,src}/gsd-*` files leaked into the
+    // baseline because the new manifest no longer classifies them as managed.
+    writeFile(configDir, 'get-shit-done/sdk/dist/gsd-old-bundle.js', 'stale sdk bundle\n');
+    // User-facing skill: replicates `skills/gsd-roadmap/SKILL.md` from the
+    // same incident — user-owned content that must be preserved.
+    writeFile(configDir, 'skills/gsd-roadmap/SKILL.md', '# Roadmap skill\nuser content\n');
+
+    // Plant an empty manifest so both files classify as `stale-gsd-looking`
+    // (they look like GSD artifacts but are not manifest-managed).
+    writeManifest(configDir, {});
+
+    const result = runInstallerMigrations({
+      configDir,
+      runtime: 'claude',
+      scope: 'global',
+      baselineScan: true,
+    });
+
+    // Confirm the migration framework classified both as prompt-user
+    // blockers — this is the precondition the fix resolves.
+    const blockedPaths = (result.blocked || []).map((a) => a.relPath).sort();
+    assert.deepEqual(
+      blockedPaths,
+      ['get-shit-done/sdk/dist/gsd-old-bundle.js', 'skills/gsd-roadmap/SKILL.md'],
+      'precondition: both stale-looking files should be flagged for explicit user choice'
+    );
+
+    // Now run the non-TTY resolver. It must classify-and-default each
+    // blocked action and return a structured log of resolutions.
+    const resolved = resolveInstallerMigrationPromptsForNonTty(result, { isTty: false });
+
+    assert.ok(Array.isArray(resolved.resolutions), 'resolver returns a resolutions log');
+    assert.equal(
+      resolved.resolutions.length,
+      2,
+      'one resolution entry per blocked action'
+    );
+
+    const byPath = new Map(resolved.resolutions.map((r) => [r.relPath, r]));
+    const sdkResolution = byPath.get('get-shit-done/sdk/dist/gsd-old-bundle.js');
+    const skillResolution = byPath.get('skills/gsd-roadmap/SKILL.md');
+
+    assert.ok(sdkResolution, 'SDK artifact resolution logged');
+    assert.equal(sdkResolution.choice, 'remove', 'stale SDK build artifact defaults to remove');
+    assert.equal(sdkResolution.category, 'stale-sdk-build-artifact');
+
+    assert.ok(skillResolution, 'user skill resolution logged');
+    assert.equal(skillResolution.choice, 'keep', 'user-facing skill defaults to keep');
+    assert.equal(skillResolution.category, 'user-facing-skill');
+
+    // After resolution there must be no blocked actions remaining; the
+    // assertion gatekeeper must not throw.
+    assert.equal(
+      (resolved.result.blocked || []).length,
+      0,
+      'all prompt-user actions resolved'
+    );
+    assert.doesNotThrow(() => assertInstallerMigrationsUnblocked(resolved.result));
+  });
+
+  test('Test B: error message groups paths by reason and suggests a resolution path', () => {
+    // Build a synthetic result with two blocked prompt-user actions of
+    // distinct reasons. The improved error message must (1) list the
+    // documented choices, (2) suggest the non-interactive resolution
+    // path, (3) group blocked paths by reason rather than emit each path
+    // individually.
+    const blocked = [
+      {
+        type: 'prompt-user',
+        relPath: 'get-shit-done/sdk/dist/gsd-a.js',
+        reason: 'GSD-looking file is not proven manifest-managed and needs explicit user choice',
+        classification: 'stale-gsd-looking',
+        prompt: 'Choose whether to remove this stale-looking GSD artifact or keep it as user-owned.',
+        choices: ['keep', 'remove'],
+      },
+      {
+        type: 'prompt-user',
+        relPath: 'get-shit-done/sdk/dist/gsd-b.js',
+        reason: 'GSD-looking file is not proven manifest-managed and needs explicit user choice',
+        classification: 'stale-gsd-looking',
+        prompt: 'Choose whether to remove this stale-looking GSD artifact or keep it as user-owned.',
+        choices: ['keep', 'remove'],
+      },
+    ];
+
+    let captured = null;
+    try {
+      assertInstallerMigrationsUnblocked({ blocked });
+      assert.fail('expected assertInstallerMigrationsUnblocked to throw');
+    } catch (err) {
+      captured = err;
+    }
+
+    assert.ok(captured instanceof Error);
+    const message = captured.message;
+
+    // (a) Documented choices listed.
+    assert.match(message, /keep/, 'error message lists `keep` choice');
+    assert.match(message, /remove/, 'error message lists `remove` choice');
+
+    // (b) Suggests the resolution path. The fix introduces an
+    // environment variable as the documented non-interactive resolution
+    // surface — the message must point users at it.
+    assert.match(
+      message,
+      /GSD_INSTALLER_MIGRATION_RESOLVE/,
+      'error message suggests the non-interactive resolution env var'
+    );
+
+    // (c) Paths grouped by reason — two paths sharing the same reason
+    // appear under one summary count, not as two separate path lines.
+    // The message must include a `2 files` (or similar) grouped summary
+    // and must NOT list each individual relPath in the top-level message.
+    assert.match(
+      message,
+      /2 (files?|paths?|artifacts?)/,
+      'error message groups blocked paths into a count summary'
+    );
+
+    // Structured surface: the thrown error must carry a `blockedByReason`
+    // map so callers can render their own report without re-parsing.
+    assert.ok(captured.blockedByReason, 'error carries blockedByReason data');
+    const reasons = Object.keys(captured.blockedByReason);
+    assert.equal(reasons.length, 1, 'two same-reason paths grouped under one key');
+    assert.equal(captured.blockedByReason[reasons[0]].length, 2);
+  });
+
+  test('Test C: non-TTY env override resolves otherwise-unclassified prompt-user actions', () => {
+    const result = {
+      blocked: [
+        {
+          type: 'prompt-user',
+          relPath: 'skills/gsd-custom/SKILL.toml',
+          reason: 'custom skill metadata requires user decision',
+          choices: ['keep', 'remove'],
+        },
+      ],
+      plan: {
+        actions: [],
+        blocked: [
+          {
+            type: 'prompt-user',
+            relPath: 'skills/gsd-custom/SKILL.toml',
+            reason: 'custom skill metadata requires user decision',
+            choices: ['keep', 'remove'],
+          },
+        ],
+      },
+    };
+
+    const resolved = resolveInstallerMigrationPromptsForNonTty(result, {
+      isTty: false,
+      env: { GSD_INSTALLER_MIGRATION_RESOLVE: 'keep' },
+    });
+
+    assert.equal(resolved.resolutions.length, 1, 'env override resolves prompt-user action');
+    assert.equal(resolved.resolutions[0].choice, 'keep');
+    assert.equal(resolved.resolutions[0].source, 'GSD_INSTALLER_MIGRATION_RESOLVE');
+    assert.equal(resolved.resolutions[0].category, 'operator-override');
+    assert.equal((resolved.result.blocked || []).length, 0);
+    assert.equal((resolved.result.plan.blocked || []).length, 0);
+    assert.equal((resolved.result.plan.actions || []).length, 1);
+    assert.equal(resolved.result.plan.actions[0].type, 'baseline-preserve-user');
+  });
+});

tests/installer-migration-report.test.cjs

@@ -146,14 +146,29 @@ test('collapses first-time baseline report rows without hiding destructive actio
 });
 
 test('throws when installer migrations require user choice', () => {
-  assert.throws(
-    () => assertInstallerMigrationsUnblocked({
+  // #3541: error message now groups paths by reason and names the
+  // non-interactive resolution surface. The thrown error carries
+  // structured `blockedByReason` data and the resolution env var
+  // name so callers can render their own report.
+  let captured = null;
+  try {
+    assertInstallerMigrationsUnblocked({
       blocked: [
         {
           relPath: 'hooks/gsd-retired-hook.js',
+          reason: 'needs a user choice',
+          choices: ['keep', 'remove'],
         },
       ],
-    }),
-    /installer migration blocked pending user choice: hooks\/gsd-retired-hook\.js/
-  );
+    });
+    assert.fail('expected throw');
+  } catch (err) {
+    captured = err;
+  }
+  assert.ok(captured instanceof Error);
+  assert.match(captured.message, /installer migration blocked pending user choice/);
+  assert.match(captured.message, /hooks\/gsd-retired-hook\.js/);
+  assert.match(captured.message, /GSD_INSTALLER_MIGRATION_RESOLVE/);
+  assert.ok(captured.blockedByReason, 'error exposes grouped-by-reason data');
+  assert.equal(captured.resolutionEnvVar, 'GSD_INSTALLER_MIGRATION_RESOLVE');
 });