diff --git a/.changeset/3541-installer-migration-prompt-user-resolution.md b/.changeset/3541-installer-migration-prompt-user-resolution.md new file mode 100644 index 000000000..15cd84baa --- /dev/null +++ b/.changeset/3541-installer-migration-prompt-user-resolution.md @@ -0,0 +1,6 @@ +--- +type: Fixed +pr: 0 +--- + +**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. (#0) diff --git a/bin/install.js b/bin/install.js index b5e373fd8..4cf3a5839 100755 --- a/bin/install.js +++ b/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); diff --git a/get-shit-done/bin/lib/installer-migration-report.cjs b/get-shit-done/bin/lib/installer-migration-report.cjs index ff175b968..528ebdb14 100644 --- a/get-shit-done/bin/lib/installer-migration-report.cjs +++ b/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,179 @@ 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/-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 }; +} + +// 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 resolutions = []; + const unresolved = []; + + for (const action of blocked) { + if (action && action.type === 'prompt-user') { + const classification = classifyPromptUserAction(action); + if (classification) { + const resolved = materializeResolution(action, classification.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: classification.category, + choice: classification.choice, + reason: action.reason, + resolvedActionType: resolved.type, + source: 'non-tty-default', + }); + 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}= ` + + `(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, }; diff --git a/tests/bug-3541-installer-migration-prompt-user-resolution.test.cjs b/tests/bug-3541-installer-migration-prompt-user-resolution.test.cjs new file mode 100644 index 000000000..4a44a42ba --- /dev/null +++ b/tests/bug-3541-installer-migration-prompt-user-resolution.test.cjs @@ -0,0 +1,201 @@ +/** + * 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); + }); +}); diff --git a/tests/installer-migration-report.test.cjs b/tests/installer-migration-report.test.cjs index 6ebcccfbe..6a6d90e1c 100644 --- a/tests/installer-migration-report.test.cjs +++ b/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'); });