diff --git a/.changeset/vivid-foxes-romp.md b/.changeset/vivid-foxes-romp.md index b04b1673b..f1357b255 100644 --- a/.changeset/vivid-foxes-romp.md +++ b/.changeset/vivid-foxes-romp.md @@ -2,4 +2,4 @@ type: Added pr: 3402 --- -Wire installer migrations into the normal install flow for every supported runtime with baseline scanning, action reporting, blocked-action protection, and install-state persistence before package materialization. +Wire installer migrations into the normal install flow for every supported runtime with baseline scanning, collapsed action reporting, blocked-action protection, transactional rollback, install-state persistence, and lock-guarded execution before package materialization. diff --git a/bin/install.js b/bin/install.js index 750b62ed6..007bc783c 100755 --- a/bin/install.js +++ b/bin/install.js @@ -77,6 +77,7 @@ const { stageSkillsForMode, } = require(path.join(_gsdLibDir, 'install-profiles.cjs')); const { + discoverInstallerMigrations, runInstallerMigrations, } = require(path.join(_gsdLibDir, 'installer-migrations.cjs')); const { @@ -7482,7 +7483,7 @@ function reportInstallerMigrationResult(result) { } } -function install(isGlobal, runtime = 'claude') { +function install(isGlobal, runtime = 'claude', options = {}) { const isOpencode = runtime === 'opencode'; const isGemini = runtime === 'gemini'; const isKilo = runtime === 'kilo'; @@ -7746,6 +7747,7 @@ function install(isGlobal, runtime = 'claude') { configDir: targetDir, runtime, scope: isGlobal ? 'global' : 'local', + migrations: options.installerMigrations, baselineScan: true, }); reportInstallerMigrationResult(installerMigrationResult); @@ -8377,6 +8379,12 @@ function install(isGlobal, runtime = 'claude') { } } catch (_earlyInstallErr) { + // Installer Migration Module Phase 4: docs/installer-migrations.md + // requires safe migrations to run before package materialization without + // leaving stale state behind when materialization fails. Roll migration + // actions back for every runtime; Codex then layers its broader runtime + // snapshot rollback on top. + rollbackInstallerMigrations(); // #3245 CR finding 2 — any throw in the pre-config install operations (skills copy, // agents copy, VERSION write, manifest write, etc.) triggers the Codex pre-config // rollback so the caller is never left in a partially-installed state. @@ -9112,6 +9120,7 @@ function install(isGlobal, runtime = 'claude') { updateBannerCommand, runtime, configDir: targetDir, + rollbackInstallerMigrations, }; } @@ -10573,40 +10582,69 @@ function trySelfLinkGsdSdkWindows(shimSrc) { */ function installAllRuntimes(runtimes, isGlobal, isInteractive) { const results = []; + const installerMigrations = discoverInstallerMigrations({ + migrationsDir: path.join(_gsdLibDir, 'installer-migrations'), + }); for (const runtime of runtimes) { - const result = install(isGlobal, runtime); + const result = install(isGlobal, runtime, { installerMigrations }); results.push(result); } const statuslineRuntimes = ['claude', 'gemini']; const primaryStatuslineResult = results.find(r => statuslineRuntimes.includes(r.runtime)); - const finalize = (shouldInstallStatusline, shouldInstallBanner) => { - // Verify sdk/dist/cli.js is present and executable. The dist is shipped - // prebuilt in the tarball (fix/2441-sdk-decouple); gsd-sdk reaches users via - // the parent package's bin/gsd-sdk.js shim, so no sub-install is needed. - // Skip with --no-sdk. Skip with isLocal (#2678 — local installs don't own global npm). - // #3033: pass forceSdk so --sdk overrides the local-install skip. - installSdkIfNeeded({ isLocal: !isGlobal, forceSdk: hasSdk }); - - const printSummaries = () => { - for (const result of results) { - const useStatusline = statuslineRuntimes.includes(result.runtime) && shouldInstallStatusline; - finishInstall( - result.settingsPath, - result.settings, - result.statuslineCommand, - useStatusline, - result.runtime, - isGlobal, - result.configDir, - { shouldInstallBanner: !!shouldInstallBanner, bannerCommand: result.updateBannerCommand } - ); + const rollbackFinalizedInstallerMigrations = (error) => { + const rollbackFailures = []; + for (const result of [...results].reverse()) { + if (!result || typeof result.rollbackInstallerMigrations !== 'function') continue; + try { + result.rollbackInstallerMigrations(); + } catch (rollbackError) { + rollbackFailures.push({ + runtime: result.runtime, + error: rollbackError.message, + }); } - }; + } + if (rollbackFailures.length > 0) { + error.installerMigrationRollbackFailures = rollbackFailures; + } + }; - printSummaries(); + const finalize = (shouldInstallStatusline, shouldInstallBanner) => { + try { + // Verify sdk/dist/cli.js is present and executable. The dist is shipped + // prebuilt in the tarball (fix/2441-sdk-decouple); gsd-sdk reaches users via + // the parent package's bin/gsd-sdk.js shim, so no sub-install is needed. + // Skip with --no-sdk. Skip with isLocal (#2678 — local installs don't own global npm). + // #3033: pass forceSdk so --sdk overrides the local-install skip. + installSdkIfNeeded({ isLocal: !isGlobal, forceSdk: hasSdk }); + + const printSummaries = () => { + for (const result of results) { + const useStatusline = statuslineRuntimes.includes(result.runtime) && shouldInstallStatusline; + finishInstall( + result.settingsPath, + result.settings, + result.statuslineCommand, + useStatusline, + result.runtime, + isGlobal, + result.configDir, + { shouldInstallBanner: !!shouldInstallBanner, bannerCommand: result.updateBannerCommand } + ); + } + }; + + printSummaries(); + } catch (error) { + // Phase 4 install/update integration requires safe migrations to roll + // back when later package/finalization materialization fails: + // docs/installer-migrations.md#phase-4-installupdate-integration. + rollbackFinalizedInstallerMigrations(error); + throw error; + } }; // Statusline first; if it won't actually be installed (declined, or local @@ -10681,6 +10719,7 @@ if (process.env.GSD_TEST_MODE) { readGsdRuntimeProfileResolver, readGsdEffectiveModelOverrides, install, + installAllRuntimes, uninstall, installSdkIfNeeded, buildSdkFailFastReport, diff --git a/get-shit-done/bin/lib/installer-migration-report.cjs b/get-shit-done/bin/lib/installer-migration-report.cjs index d0ff12275..ff175b968 100644 --- a/get-shit-done/bin/lib/installer-migration-report.cjs +++ b/get-shit-done/bin/lib/installer-migration-report.cjs @@ -19,21 +19,67 @@ function blockedInstallerMigrationActions(result) { return []; } +function baselineSummaryLabel(count, noun) { + return `${count} ${noun}${count === 1 ? '' : 's'}`; +} + +function baselineSummaryRow(type, actions) { + const count = actions.length; + if (type === 'record-baseline') { + return { + label: 'recorded', + relPath: baselineSummaryLabel(count, 'managed baseline file'), + reason: 'first-time baseline scan', + action: { type: 'record-baseline-summary', count, actions }, + }; + } + return { + label: 'preserved', + relPath: baselineSummaryLabel(count, 'user baseline file'), + reason: 'first-time baseline scan', + action: { type: 'baseline-preserve-user-summary', count, actions }, + }; +} + function summarizeInstallerMigrationResult(result) { const plan = result && result.plan; const actions = plan && Array.isArray(plan.actions) ? plan.actions : []; const blocked = blockedInstallerMigrationActions(result); const blockedSet = new Set(blocked); + const rows = []; + const baselineIndexes = new Map(); + const baselineActions = new Map(); - return { - hasReportableActions: actions.length > 0 || blocked.length > 0, - blocked, - rows: actions.map((action) => ({ + for (const action of actions) { + const type = action && action.type; + if (type === 'record-baseline' || type === 'baseline-preserve-user') { + if (!baselineActions.has(type)) { + baselineActions.set(type, []); + baselineIndexes.set(type, rows.length); + rows.push(null); + } + baselineActions.get(type).push(action); + continue; + } + + rows.push({ label: blockedSet.has(action) ? 'blocked' : installerMigrationActionLabel(action), relPath: action.relPath, reason: action.reason || '', action, - })), + }); + } + + // Phase 4 requires action reporting without flooding first-time baseline installs: + // docs/installer-migrations.md#phase-4-installupdate-integration. + for (const [type, baselineRows] of baselineActions) { + rows[baselineIndexes.get(type)] = baselineSummaryRow(type, baselineRows); + } + + return { + hasReportableActions: actions.length > 0 || blocked.length > 0, + blocked, + rows, }; } diff --git a/get-shit-done/bin/lib/installer-migrations.cjs b/get-shit-done/bin/lib/installer-migrations.cjs index b69272995..f497907b4 100644 --- a/get-shit-done/bin/lib/installer-migrations.cjs +++ b/get-shit-done/bin/lib/installer-migrations.cjs @@ -6,10 +6,25 @@ const crypto = require('crypto'); const MANIFEST_NAME = 'gsd-file-manifest.json'; const INSTALL_STATE_NAME = 'gsd-install-state.json'; +const INSTALL_MIGRATION_LOCK_NAME = 'gsd-install-migration.lock'; const DEFAULT_MIGRATIONS_DIR = path.join(__dirname, 'installer-migrations'); +const DEFAULT_LOCK_TIMEOUT_MS = 30_000; +const STRICT_JSON = Symbol('strict-json'); function sha256File(filePath) { - return crypto.createHash('sha256').update(fs.readFileSync(filePath)).digest('hex'); + const hash = crypto.createHash('sha256'); + const buffer = Buffer.allocUnsafe(1024 * 1024); + const fd = fs.openSync(filePath, 'r'); + try { + while (true) { + const bytesRead = fs.readSync(fd, buffer, 0, buffer.length, null); + if (bytesRead === 0) break; + hash.update(buffer.subarray(0, bytesRead)); + } + } finally { + fs.closeSync(fd); + } + return hash.digest('hex'); } function sha256Text(value) { @@ -20,7 +35,10 @@ function readJsonIfPresent(filePath, fallback) { if (!fs.existsSync(filePath)) return fallback; try { return JSON.parse(fs.readFileSync(filePath, 'utf8')); - } catch { + } catch (error) { + if (fallback === STRICT_JSON) { + throw new Error(`invalid installer migration state JSON: ${filePath}: ${error.message}`); + } return fallback; } } @@ -39,7 +57,7 @@ function readInstallManifest(configDir) { } function readInstallState(configDir) { - const state = readJsonIfPresent(path.join(configDir, INSTALL_STATE_NAME), null); + const state = readJsonIfPresent(path.join(configDir, INSTALL_STATE_NAME), STRICT_JSON); if (!state || typeof state !== 'object') { return { schemaVersion: 1, appliedMigrations: [] }; } @@ -114,7 +132,8 @@ function appliedMigrationEntries(state) { } function migrationChecksum(migration) { - if (typeof migration.checksum === 'string' && migration.checksum) return migration.checksum; + const checksum = migration.checksum; + if (typeof checksum === 'string' && checksum) return checksum; const serializable = { id: migration.id, title: migration.title || null, @@ -129,8 +148,7 @@ function migrationChecksum(migration) { return `sha256:${sha256Text(JSON.stringify(serializable))}`; } -function assertAppliedMigrationChecksums(state, migrations) { - const applied = appliedMigrationEntries(state); +function assertAppliedMigrationChecksums(applied, migrations) { for (const migration of migrations) { const entry = applied.get(migration.id); if (!entry || !entry.checksum) continue; @@ -186,6 +204,55 @@ function journalTimestamp(now) { return now().replace(/[:.]/g, '-'); } +function migrationRunId(appliedAt) { + return `${journalTimestamp(() => appliedAt)}-${crypto.randomBytes(8).toString('hex')}`; +} + +function sleepSync(ms) { + const buffer = new SharedArrayBuffer(4); + Atomics.wait(new Int32Array(buffer), 0, 0, ms); +} + +function acquireInstallMigrationLock(configDir, { timeoutMs = DEFAULT_LOCK_TIMEOUT_MS } = {}) { + fs.mkdirSync(configDir, { recursive: true }); + const lockPath = path.join(configDir, INSTALL_MIGRATION_LOCK_NAME); + const started = Date.now(); + + while (true) { + let fd = null; + try { + fd = fs.openSync(lockPath, 'wx'); + fs.writeFileSync(fd, JSON.stringify({ + pid: process.pid, + acquiredAt: new Date().toISOString(), + }) + '\n'); + return () => { + const failures = []; + try { fs.closeSync(fd); } catch (error) { failures.push(error); } + try { fs.rmSync(lockPath, { force: true }); } catch (error) { failures.push(error); } + if (failures.length > 0) { + const releaseError = new Error(`failed to release installer migration lock: ${lockPath}`); + releaseError.failures = failures; + throw releaseError; + } + }; + } catch (error) { + if (fd !== null) { + try { fs.closeSync(fd); } catch { /* best-effort */ } + try { fs.rmSync(lockPath, { force: true }); } catch { /* best-effort */ } + } + if (error && error.code === 'EEXIST') { + if (Date.now() - started >= timeoutMs) { + throw new Error(`installer migration lock is held: ${lockPath}`); + } + sleepSync(Math.min(50, Math.max(1, timeoutMs - (Date.now() - started)))); + continue; + } + throw error; + } + } +} + function ensureInsideConfig(configDir, relPath) { const normalized = normalizeRelPath(relPath); const fullPath = path.resolve(configDir, normalized); @@ -238,8 +305,8 @@ function planInstallerMigrations({ const scopedMigrations = migrations.filter((migration) => migration && migrationMatchesContext(migration, { runtime, scope }) ); - assertAppliedMigrationChecksums(state, scopedMigrations); - const applied = appliedMigrationIds(state); + const applied = appliedMigrationEntries(state); + assertAppliedMigrationChecksums(applied, scopedMigrations); const pending = scopedMigrations.filter((migration) => !applied.has(migration.id)); const actions = []; const blocked = []; @@ -273,6 +340,7 @@ function planInstallerMigrations({ if (!Array.isArray(plannedActions)) { throw new Error(`migration ${migration.id} plan must return an array`); } + const checksum = migrationChecksum(migration); for (const rawAction of plannedActions) { const relPath = normalizeRelPath(rawAction.relPath); const classification = rawAction.classification @@ -291,7 +359,7 @@ function planInstallerMigrations({ } const action = { migrationId: migration.id, - migrationChecksum: migrationChecksum(migration), + migrationChecksum: checksum, type: protectedType, relPath, reason: rawAction.reason || migration.description || '', @@ -303,7 +371,7 @@ function planInstallerMigrations({ action.requestedType = rawAction.type; } if (action.type === 'backup-and-remove') { - action.backupRelPath = path.posix.join('gsd-migration-backups', migration.id, relPath); + action.backupRelPath = null; } if (action.type === 'rewrite-json') { action.value = rawAction.value; @@ -339,7 +407,7 @@ function uniqueActionMigrationIds(actions) { return [...new Set(actions.map((action) => action.migrationId).filter(Boolean))]; } -function rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollbackRoot, previousInstallStateBytes }) { +function rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollbackRoot, backupRoot, previousInstallStateBytes }) { const failures = []; for (const action of [...journal.actions].reverse()) { if (!action.rollbackRelPath) continue; @@ -376,6 +444,7 @@ function rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollb try { fs.rmSync(journalPath, { force: true }); fs.rmSync(rollbackRoot, { recursive: true, force: true }); + fs.rmSync(backupRoot, { recursive: true, force: true }); } catch { // journal cleanup is best-effort; the rollback above is the safety-critical part } @@ -387,6 +456,12 @@ function rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollb } } +function cleanupMigrationRunArtifacts(journalPath, rollbackRoot, backupRoot) { + try { fs.rmSync(journalPath, { force: true }); } catch { /* best-effort */ } + try { fs.rmSync(rollbackRoot, { recursive: true, force: true }); } catch { /* best-effort */ } + try { fs.rmSync(backupRoot, { recursive: true, force: true }); } catch { /* best-effort */ } +} + function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().toISOString() }) { if (!configDir) throw new Error('configDir is required'); if (!plan || !Array.isArray(plan.actions)) throw new Error('plan with actions is required'); @@ -395,10 +470,13 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t } const appliedAt = now(); - const journalRelPath = path.posix.join('gsd-migration-journal', `${journalTimestamp(() => appliedAt)}.json`); + const runId = migrationRunId(appliedAt); + const journalRelPath = path.posix.join('gsd-migration-journal', `${runId}.json`); const journalPath = path.join(configDir, journalRelPath); - const rollbackRootRelPath = path.posix.join('gsd-migration-journal', `${journalTimestamp(() => appliedAt)}-rollback`); + const rollbackRootRelPath = path.posix.join('gsd-migration-journal', `${runId}-rollback`); const rollbackRoot = path.join(configDir, rollbackRootRelPath); + const backupRootRelPath = path.posix.join('gsd-migration-journal', `${runId}-backups`); + const backupRoot = path.join(configDir, backupRootRelPath); const journal = { schemaVersion: 1, appliedAt, @@ -455,7 +533,7 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t } if (action.type === 'backup-and-remove') { - const backupRelPath = action.backupRelPath || path.posix.join('gsd-migration-backups', action.migrationId, normalized); + const backupRelPath = action.backupRelPath || path.posix.join(backupRootRelPath, normalized); const backupPath = path.join(configDir, backupRelPath); fs.mkdirSync(path.dirname(backupPath), { recursive: true }); fs.copyFileSync(fullPath, backupPath); @@ -502,7 +580,7 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t return { appliedMigrationIds: journal.appliedMigrationIds, journalRelPath, - rollback: () => rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollbackRoot, previousInstallStateBytes }), + rollback: () => rollbackAppliedMigrationResult({ configDir, journal, journalPath, rollbackRoot, backupRoot, previousInstallStateBytes }), }; } catch (error) { const rollbackFailures = []; @@ -525,6 +603,7 @@ function applyInstallerMigrationPlan({ configDir, plan, now = () => new Date().t rollbackError.rollbackFailures = rollbackFailures; throw rollbackError; } + cleanupMigrationRunArtifacts(journalPath, rollbackRoot, backupRoot); throw error; } } @@ -537,29 +616,54 @@ function runInstallerMigrations({ migrations = discoverInstallerMigrations({ migrationsDir }), baselineScan = false, now = () => new Date().toISOString(), + lockTimeoutMs = DEFAULT_LOCK_TIMEOUT_MS, } = {}) { - const plan = planInstallerMigrations({ configDir, runtime, scope, migrations, baselineScan, now }); - if (plan.actions.length === 0) { - return { - appliedMigrationIds: [], - journalRelPath: null, - plan, - }; + const releaseLock = acquireInstallMigrationLock(configDir, { timeoutMs: lockTimeoutMs }); + let primaryError = null; + let completed = false; + try { + const plan = planInstallerMigrations({ configDir, runtime, scope, migrations, baselineScan, now }); + if (plan.actions.length === 0) { + completed = true; + return { + appliedMigrationIds: [], + journalRelPath: null, + plan, + }; + } + if (plan.blocked.length > 0) { + completed = true; + return { + appliedMigrationIds: [], + journalRelPath: null, + plan, + blocked: plan.blocked, + }; + } + const result = applyInstallerMigrationPlan({ configDir, plan, now }); + completed = true; + return { ...result, plan }; + } catch (error) { + primaryError = error; + throw error; + } finally { + try { + releaseLock(); + } catch (releaseError) { + if (primaryError) { + primaryError.suppressed = [...(primaryError.suppressed || []), releaseError]; + } else if (completed) { + throw releaseError; + } else { + throw releaseError; + } + } } - if (plan.blocked.length > 0) { - return { - appliedMigrationIds: [], - journalRelPath: null, - plan, - blocked: plan.blocked, - }; - } - const result = applyInstallerMigrationPlan({ configDir, plan, now }); - return { ...result, plan }; } module.exports = { DEFAULT_MIGRATIONS_DIR, + INSTALL_MIGRATION_LOCK_NAME, INSTALL_STATE_NAME, MANIFEST_NAME, applyInstallerMigrationPlan, diff --git a/get-shit-done/bin/lib/installer-migrations/000-first-time-baseline.cjs b/get-shit-done/bin/lib/installer-migrations/000-first-time-baseline.cjs index 8d03a5359..ddb93e4f4 100644 --- a/get-shit-done/bin/lib/installer-migrations/000-first-time-baseline.cjs +++ b/get-shit-done/bin/lib/installer-migrations/000-first-time-baseline.cjs @@ -56,8 +56,7 @@ function baselineInstallSurfaces(runtime) { function walkFiles(root, relDir, files) { const dir = path.join(root, relDir); if (!fs.existsSync(dir)) return; - const entries = fs.readdirSync(dir, { withFileTypes: true }) - .sort((a, b) => a.name.localeCompare(b.name)); + const entries = fs.readdirSync(dir, { withFileTypes: true }); for (const entry of entries) { const relPath = path.posix.join(relDir, entry.name); if (relDir === '' && INTERNAL_TOP_LEVEL_NAMES.has(entry.name)) continue; @@ -83,7 +82,7 @@ function scanBaselineFiles(configDir, runtime) { relPaths.add(normalized); } } - return [...relPaths].sort(); + return [...relPaths]; } function isUserOwnedBaselinePath(relPath) { diff --git a/tests/installer-migration-install-integration.test.cjs b/tests/installer-migration-install-integration.test.cjs index 4ff6b82aa..6e7b496ed 100644 --- a/tests/installer-migration-install-integration.test.cjs +++ b/tests/installer-migration-install-integration.test.cjs @@ -71,6 +71,21 @@ function captureConsole(fn) { } } +function withWriteFailure(matchPath, fn) { + const originalWriteFileSync = fs.writeFileSync; + fs.writeFileSync = (filePath, ...args) => { + if (path.resolve(String(filePath)) === path.resolve(matchPath)) { + throw new Error(`injected write failure for ${path.basename(matchPath)}`); + } + return originalWriteFileSync.call(fs, filePath, ...args); + }; + try { + return fn(); + } finally { + fs.writeFileSync = originalWriteFileSync; + } +} + function stripAnsi(value) { return value.replace(/\x1b\[[0-9;]*m/g, ''); } @@ -138,6 +153,56 @@ describe('installer migration install integration', { concurrency: false }, () = assert.equal(fs.existsSync(path.join(codexHome, 'get-shit-done', 'VERSION')), false); }); + test('rolls back applied migrations when package materialization fails for non-Codex installs', () => { + const claudeHome = path.join(tmpRoot, '.claude'); + fs.mkdirSync(claudeHome, { recursive: true }); + writeFile(claudeHome, 'hooks/statusline.js', 'legacy managed hook\n'); + writeManifest(claudeHome, { + 'hooks/statusline.js': sha256('legacy managed hook\n'), + }); + + assert.throws( + () => captureConsole(() => + withEnv('CLAUDE_CONFIG_DIR', claudeHome, () => + withWriteFailure(path.join(claudeHome, 'get-shit-done', 'VERSION'), () => install(true, 'claude')) + ) + ), + /injected write failure for VERSION/ + ); + + assert.equal( + fs.readFileSync(path.join(claudeHome, 'hooks/statusline.js'), 'utf8'), + 'legacy managed hook\n' + ); + assert.equal(fs.existsSync(path.join(claudeHome, 'gsd-install-state.json')), false); + }); + + test('rolls back applied migrations when multi-runtime finalization fails', () => { + const claudeHome = path.join(tmpRoot, '.claude'); + fs.mkdirSync(claudeHome, { recursive: true }); + writeFile(claudeHome, 'hooks/statusline.js', 'legacy managed hook\n'); + writeManifest(claudeHome, { + 'hooks/statusline.js': sha256('legacy managed hook\n'), + }); + + assert.throws( + () => captureConsole(() => + withEnv('CLAUDE_CONFIG_DIR', claudeHome, () => + withWriteFailure(path.join(claudeHome, 'settings.json'), () => + installModule.installAllRuntimes(['claude'], true, false) + ) + ) + ), + /injected write failure for settings\.json/ + ); + + assert.equal( + fs.readFileSync(path.join(claudeHome, 'hooks/statusline.js'), 'utf8'), + 'legacy managed hook\n' + ); + assert.equal(fs.existsSync(path.join(claudeHome, 'gsd-install-state.json')), false); + }); + for (const runtime of SUPPORTED_RUNTIMES) { test(`runs managed cleanup migrations for ${runtime}`, () => { const targetDir = path.join(tmpRoot, `.${runtime}-managed-cleanup`); diff --git a/tests/installer-migration-report.test.cjs b/tests/installer-migration-report.test.cjs index 0770c33ae..6ebcccfbe 100644 --- a/tests/installer-migration-report.test.cjs +++ b/tests/installer-migration-report.test.cjs @@ -62,8 +62,8 @@ test('summarizes every installer migration report category', () => { }, { label: 'preserved', - relPath: 'hooks/custom.js', - reason: 'user-owned hook', + relPath: '1 user baseline file', + reason: 'first-time baseline scan', }, { label: 'skipped', @@ -79,6 +79,72 @@ test('summarizes every installer migration report category', () => { ); }); +test('collapses first-time baseline report rows without hiding destructive actions', () => { + const blockedAction = { + type: 'prompt-user', + relPath: 'hooks/gsd-ambiguous.js', + reason: 'needs a user choice', + }; + const result = { + blocked: [blockedAction], + plan: { + actions: [ + { + type: 'record-baseline', + relPath: 'hooks/statusline.js', + reason: 'first-time baseline scan', + }, + { + type: 'record-baseline', + relPath: 'hooks/workflow-guard.js', + reason: 'first-time baseline scan', + }, + { + type: 'baseline-preserve-user', + relPath: 'hooks/custom.js', + reason: 'first-time baseline scan', + }, + { + type: 'remove-managed', + relPath: 'hooks/retired.js', + reason: 'retired hook', + }, + blockedAction, + ], + }, + }; + + assert.deepEqual( + summarizeInstallerMigrationResult(result).rows.map((row) => ({ + label: row.label, + relPath: row.relPath, + reason: row.reason, + })), + [ + { + label: 'recorded', + relPath: '2 managed baseline files', + reason: 'first-time baseline scan', + }, + { + label: 'preserved', + relPath: '1 user baseline file', + reason: 'first-time baseline scan', + }, + { + label: 'removed', + relPath: 'hooks/retired.js', + reason: 'retired hook', + }, + { + label: 'blocked', + relPath: 'hooks/gsd-ambiguous.js', + reason: 'needs a user choice', + }, + ] + ); +}); + test('throws when installer migrations require user choice', () => { assert.throws( () => assertInstallerMigrationsUnblocked({ diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index a0bd17c7c..5a112fd97 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -7,6 +7,7 @@ const crypto = require('crypto'); const { applyInstallerMigrationPlan, + classifyArtifact, discoverInstallerMigrations, INSTALL_STATE_NAME, planInstallerMigrations, @@ -334,7 +335,7 @@ test('plans backup before removal for a modified managed file', () => { assert.equal(plan.actions[0].classification, 'managed-modified'); assert.equal(plan.actions[0].originalHash, sha256('managed hook\n')); assert.equal(plan.actions[0].currentHash, sha256('user changed hook\n')); - assert.equal(plan.actions[0].backupRelPath, 'gsd-migration-backups/2026-05-11-remove-old-hook/hooks/old-hook.js'); + assert.equal(plan.actions[0].backupRelPath, null); } finally { cleanup(configDir); } @@ -374,6 +375,77 @@ test('blocks removal of unknown files by preserving them by default', () => { } }); +test('fails closed when install state JSON is malformed', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + fs.writeFileSync(path.join(configDir, INSTALL_STATE_NAME), '{ not json\n', 'utf8'); + + assert.throws( + () => readInstallState(configDir), + /invalid installer migration state JSON/ + ); +}); + +test('computes each migration checksum once per planned migration', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + writeFile(configDir, 'hooks/first.js', 'first hook\n'); + writeFile(configDir, 'hooks/second.js', 'second hook\n'); + writeManifest(configDir, { + 'hooks/first.js': sha256('first hook\n'), + 'hooks/second.js': sha256('second hook\n'), + }); + let checksumReads = 0; + const migration = { + id: '2026-05-11-remove-two-hooks', + description: 'Remove retired hooks', + get checksum() { + checksumReads += 1; + return 'sha256:precomputed'; + }, + plan: () => [ + { type: 'remove-managed', relPath: 'hooks/first.js', reason: 'retired hook' }, + { type: 'remove-managed', relPath: 'hooks/second.js', reason: 'retired hook' }, + ], + }; + + const plan = planInstallerMigrations({ + configDir, + migrations: [migration], + }); + + assert.equal(plan.actions.length, 2); + assert.equal(checksumReads, 1); +}); + +test('classifies large files without loading the whole file through readFileSync', (t) => { + const configDir = createTempInstall(); + const originalReadFileSync = fs.readFileSync; + t.after(() => { + fs.readFileSync = originalReadFileSync; + cleanup(configDir); + }); + + const relPath = 'skills/gsd-large/SKILL.md'; + const fullPath = path.join(configDir, relPath); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + fs.writeFileSync(fullPath, Buffer.alloc(1024 * 1024 + 1, 'a')); + + fs.readFileSync = (filePath, ...args) => { + if (path.resolve(String(filePath)) === path.resolve(fullPath)) { + throw new Error('large file should be streamed for hashing'); + } + return originalReadFileSync.call(fs, filePath, ...args); + }; + + const artifact = classifyArtifact(configDir, relPath, { files: {} }); + + assert.equal(artifact.classification, 'unknown'); + assert.match(artifact.currentHash, /^[0-9a-f]{64}$/); +}); + test('applies an unblocked plan with a journal and install-state update', () => { const configDir = createTempInstall(); try { @@ -408,7 +480,10 @@ test('applies an unblocked plan with a journal and install-state update', () => assert.equal(fs.existsSync(path.join(configDir, 'hooks/old-hook.js')), false); assert.deepEqual(result.appliedMigrationIds, ['2026-05-11-remove-old-hook']); - assert.equal(result.journalRelPath, 'gsd-migration-journal/2026-05-11T00-00-01-000Z.json'); + assert.match( + result.journalRelPath, + /^gsd-migration-journal\/2026-05-11T00-00-01-000Z-[0-9a-f]+\.json$/ + ); const journal = JSON.parse(fs.readFileSync(path.join(configDir, result.journalRelPath), 'utf8')); assert.deepEqual(journal.appliedMigrationIds, ['2026-05-11-remove-old-hook']); @@ -422,6 +497,174 @@ test('applies an unblocked plan with a journal and install-state update', () => } }); +test('uses unique journal paths for applies that share a timestamp', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + writeFile(configDir, 'hooks/first.js', 'first hook\n'); + writeFile(configDir, 'hooks/second.js', 'second hook\n'); + writeManifest(configDir, { + 'hooks/first.js': sha256('first hook\n'), + 'hooks/second.js': sha256('second hook\n'), + }); + const now = () => '2026-05-11T00:00:09.000Z'; + + const first = applyInstallerMigrationPlan({ + configDir, + plan: { + blocked: [], + actions: [{ + migrationId: 'first-migration', + migrationChecksum: 'sha256:first', + type: 'remove-managed', + relPath: 'hooks/first.js', + reason: 'first', + classification: 'managed-pristine', + originalHash: sha256('first hook\n'), + currentHash: sha256('first hook\n'), + }], + }, + now, + }); + const second = applyInstallerMigrationPlan({ + configDir, + plan: { + blocked: [], + actions: [{ + migrationId: 'second-migration', + migrationChecksum: 'sha256:second', + type: 'remove-managed', + relPath: 'hooks/second.js', + reason: 'second', + classification: 'managed-pristine', + originalHash: sha256('second hook\n'), + currentHash: sha256('second hook\n'), + }], + }, + now, + }); + + assert.notEqual(first.journalRelPath, second.journalRelPath); + assert.equal(fs.existsSync(path.join(configDir, first.journalRelPath)), true); + assert.equal(fs.existsSync(path.join(configDir, second.journalRelPath)), true); +}); + +test('stores modified-file backups under the unique migration run journal', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + writeFile(configDir, 'hooks/old-hook.js', 'user changed hook\n'); + writeManifest(configDir, { + 'hooks/old-hook.js': sha256('managed hook\n'), + }); + + const plan = planInstallerMigrations({ + configDir, + migrations: [ + { + id: '2026-05-11-remove-old-hook', + description: 'Remove retired hook', + plan: () => [{ + type: 'remove-managed', + relPath: 'hooks/old-hook.js', + reason: 'retired hook', + }], + }, + ], + now: () => '2026-05-11T00:00:00.000Z', + }); + + const result = applyInstallerMigrationPlan({ + configDir, + plan, + now: () => '2026-05-11T00:00:10.000Z', + }); + const journal = JSON.parse(fs.readFileSync(path.join(configDir, result.journalRelPath), 'utf8')); + const backupRelPath = journal.actions[0].backupRelPath; + + assert.match(backupRelPath, /^gsd-migration-journal\/2026-05-11T00-00-10-000Z-[0-9a-f]+-backups\/hooks\/old-hook\.js$/); + assert.equal(fs.readFileSync(path.join(configDir, backupRelPath), 'utf8'), 'user changed hook\n'); +}); + +test('successful migration rollback removes run-scoped backup directories', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + writeFile(configDir, 'hooks/old-hook.js', 'user changed hook\n'); + writeManifest(configDir, { + 'hooks/old-hook.js': sha256('managed hook\n'), + }); + + const plan = planInstallerMigrations({ + configDir, + migrations: [ + { + id: '2026-05-11-remove-old-hook', + description: 'Remove retired hook', + plan: () => [{ + type: 'remove-managed', + relPath: 'hooks/old-hook.js', + reason: 'retired hook', + }], + }, + ], + }); + + const result = applyInstallerMigrationPlan({ + configDir, + plan, + now: () => '2026-05-11T00:00:11.000Z', + }); + + result.rollback(); + + assert.equal(fs.readFileSync(path.join(configDir, 'hooks/old-hook.js'), 'utf8'), 'user changed hook\n'); + assert.equal(fs.existsSync(path.join(configDir, result.journalRelPath)), false); + assert.equal( + fs.readdirSync(path.join(configDir, 'gsd-migration-journal')).some((name) => name.includes('backups')), + false + ); +}); + +test('refuses to run migrations while another installer owns the migration lock', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + fs.writeFileSync(path.join(configDir, 'gsd-install-migration.lock'), 'held by test\n', 'utf8'); + + assert.throws( + () => runInstallerMigrations({ + configDir, + migrations: [], + lockTimeoutMs: 0, + }), + /installer migration lock is held/ + ); +}); + +test('reports lock release failures after migration work completes', (t) => { + const configDir = createTempInstall(); + const originalRmSync = fs.rmSync; + t.after(() => { + fs.rmSync = originalRmSync; + cleanup(configDir); + }); + + fs.rmSync = (targetPath, ...args) => { + if (path.basename(String(targetPath)) === 'gsd-install-migration.lock') { + throw new Error('simulated lock unlink failure'); + } + return originalRmSync.call(fs, targetPath, ...args); + }; + + assert.throws( + () => runInstallerMigrations({ + configDir, + migrations: [], + }), + /failed to release installer migration lock/ + ); +}); + test('rolls back touched files and leaves state unchanged when apply fails', () => { const configDir = createTempInstall(); try { @@ -466,12 +709,71 @@ test('rolls back touched files and leaves state unchanged when apply fails', () assert.equal(fs.readFileSync(path.join(configDir, 'hooks/old-hook.js'), 'utf8'), 'managed hook\n'); assert.deepEqual(readInstallState(configDir).appliedMigrations, []); - assert.equal(fs.existsSync(path.join(configDir, 'gsd-migration-journal', '2026-05-11T00-00-02-000Z.json')), false); + assert.equal( + fs.existsSync(path.join(configDir, 'gsd-migration-journal')) && + fs.readdirSync(path.join(configDir, 'gsd-migration-journal')).some((name) => + name.startsWith('2026-05-11T00-00-02-000Z') + ), + false + ); } finally { cleanup(configDir); } }); +test('cleans rollback and backup artifacts when migration apply fails', (t) => { + const configDir = createTempInstall(); + t.after(() => cleanup(configDir)); + + writeFile(configDir, 'hooks/old-hook.js', 'user changed hook\n'); + writeManifest(configDir, { + 'hooks/old-hook.js': sha256('managed hook\n'), + }); + const plan = { + blocked: [], + actions: [ + { + migrationId: '2026-05-11-remove-old-hook', + migrationChecksum: 'sha256:remove', + type: 'backup-and-remove', + relPath: 'hooks/old-hook.js', + reason: 'retired hook', + classification: 'managed-modified', + originalHash: sha256('managed hook\n'), + currentHash: sha256('user changed hook\n'), + }, + { + migrationId: '2026-05-11-remove-old-hook', + migrationChecksum: 'sha256:remove', + type: 'unsupported-test-action', + relPath: 'hooks/other.js', + reason: 'force failure', + classification: 'managed-pristine', + originalHash: null, + currentHash: null, + }, + ], + }; + + assert.throws( + () => applyInstallerMigrationPlan({ + configDir, + plan, + now: () => '2026-05-11T00:00:12.000Z', + }), + /unsupported migration action type/ + ); + + assert.equal(fs.readFileSync(path.join(configDir, 'hooks/old-hook.js'), 'utf8'), 'user changed hook\n'); + assert.equal( + fs.existsSync(path.join(configDir, 'gsd-migration-journal')) && + fs.readdirSync(path.join(configDir, 'gsd-migration-journal')).some((name) => + name.startsWith('2026-05-11T00-00-12-000Z') + ), + false + ); +}); + test('reports rollback restore failures instead of swallowing them', () => { const configDir = createTempInstall(); const originalCopyFileSync = fs.copyFileSync; @@ -506,7 +808,7 @@ test('reports rollback restore failures instead of swallowing them', () => { }; fs.copyFileSync = (src, dest) => { - if (String(src).includes('2026-05-11T00-00-04-000Z-rollback')) { + if (/2026-05-11T00-00-04-000Z-[0-9a-f]+-rollback/.test(String(src))) { throw new Error('simulated rollback copy failure'); } return originalCopyFileSync(src, dest);