diff --git a/.changeset/phase-five-installer-migration-guardrails.md b/.changeset/phase-five-installer-migration-guardrails.md new file mode 100644 index 000000000..a2bef8770 --- /dev/null +++ b/.changeset/phase-five-installer-migration-guardrails.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 3404 +--- +**Installer migrations now enforce authoring guardrails** — migration records require explicit metadata, scopes, ownership evidence, and runtime-contract citations before planned cleanup can run. diff --git a/CONTEXT.md b/CONTEXT.md index d3c780b97..ea0f8ece3 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -64,6 +64,9 @@ Module owning SDK-to-`get-shit-done-cc` compatibility policy: legacy asset disco ### Runtime-Global Skills Policy Module Module owning runtime-aware global skills directory policy for SDK query surfaces. Resolves runtime-global skills bases/skill paths from runtime + env precedence, renders display paths for warnings/manifests, and reports unsupported runtimes with no skills directory. +### Installer Migration Authoring Guard Module +Module owning validation for Installer Migration Module records and planned actions. It enforces migration metadata, explicit install scopes, ownership evidence for destructive/config actions, and runtime contract citations for runtime config rewrites before a migration can enter planning or apply. + ### MVP Mode Phase-level planning mode that frames work as a vertical slice (UI → API → DB) of one user-visible capability instead of horizontal layers. Resolved at workflow init via the precedence chain: `--mvp` CLI flag → ROADMAP.md `**Mode:** mvp` field → `workflow.mvp_mode` config → false. All-or-nothing per phase (PRD #2826 Q1). Surfaced as `MVP_MODE=true|false` to the planner, executor, verifier, and discovery surfaces (progress, stats, graphify). Canonical parser: `roadmap.cjs` `**Mode:**` field; canonical resolution chain documented in `workflows/plan-phase.md`. Concept index: `references/mvp-concepts.md`. diff --git a/bin/install.js b/bin/install.js index 592353078..b711f2aaa 100755 --- a/bin/install.js +++ b/bin/install.js @@ -7772,6 +7772,14 @@ function install(isGlobal, runtime = 'claude', options = {}) { // every supported runtime uses this same planner/apply/report path, while // individual migration records decide whether a runtime-specific config // rewrite is allowed by that runtime's documented ownership boundary. + // #3245 CR finding 2 — wrap the pre-config install operations in a try/catch so + // that ANY throw between snapshot capture and the Codex config block triggers rollback. + // Non-Codex paths are unaffected (_codexPreConfigRollback is null for them). + // + // agentsSrc is declared here (let, not const) because installCodexConfig() inside the + // Codex config block below also references it, and that block is outside the try scope. + let agentsSrc = path.join(src, 'agents'); + try { installerMigrationResult = runInstallerMigrations({ configDir: targetDir, runtime, @@ -7782,15 +7790,6 @@ function install(isGlobal, runtime = 'claude', options = {}) { reportInstallerMigrationResult(installerMigrationResult); assertInstallerMigrationsUnblocked(installerMigrationResult); - // #3245 CR finding 2 — wrap the pre-config install operations in a try/catch so - // that ANY throw between snapshot capture and the Codex config block triggers rollback. - // Non-Codex paths are unaffected (_codexPreConfigRollback is null for them). - // - // agentsSrc is declared here (let, not const) because installCodexConfig() inside the - // Codex config block below also references it, and that block is outside the try scope. - let agentsSrc = path.join(src, 'agents'); - try { - // OpenCode/Kilo use command/ (flat), Codex uses skills/, Claude/Gemini use commands/gsd/ if (isOpencode || isKilo) { // OpenCode/Kilo: flat structure in command/ directory diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index c2bcdbc84..78300673d 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -277,6 +277,7 @@ "init-command-router.cjs", "init.cjs", "install-profiles.cjs", + "installer-migration-authoring.cjs", "installer-migration-report.cjs", "installer-migrations.cjs", "intel.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 9a50be43a..783bc0d09 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -359,7 +359,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t --- -## CLI Modules (52 shipped) +## CLI Modules (53 shipped) Full listing: `get-shit-done/bin/lib/*.cjs`. @@ -385,6 +385,7 @@ Full listing: `get-shit-done/bin/lib/*.cjs`. | `init-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools init` | | `init.cjs` | Compound context loading for each workflow type | | `install-profiles.cjs` | Install profile allowlist + skill staging for `--minimal` install (#2762); single source of truth for which `gsd-*` skills/agents land in runtime config dirs | +| `installer-migration-authoring.cjs` | Installer migration authoring guardrails for record metadata, explicit scopes, ownership evidence, and runtime contract citations | | `installer-migration-report.cjs` | Installer migration report projection and blocked-action guard for install/update integration | | `installer-migrations.cjs` | Installer migration planning, artifact classification, install-state persistence, journaled apply, and rollback helpers | | `intel.cjs` | Codebase intel store backing `/gsd-map-codebase --query` and `gsd-intel-updater` | diff --git a/docs/adr/0008-installer-migration-module.md b/docs/adr/0008-installer-migration-module.md index 25509d37a..f6b569f4a 100644 --- a/docs/adr/0008-installer-migration-module.md +++ b/docs/adr/0008-installer-migration-module.md @@ -48,3 +48,12 @@ snapshot before migration work proceeds. The first implementation should extract manifest/user-owned helpers, add install-state persistence, add migration planning, and port one existing orphan cleanup into the migration runner. It should not rewrite every runtime installer branch in the first pass. The detailed module contract lives in `docs/installer-migrations.md`. + +## Amendment (2026-05-11): Authoring guard enforcement + +The Installer Migration Authoring Guard Module validates migration records and +planned actions before planning can proceed. Records must declare title, +description, introduction version, explicit install scopes, destructive status, +and a plan function. Destructive or config-rewrite actions must include +ownership evidence, and runtime config rewrites must cite the runtime +configuration contract registry. diff --git a/docs/installer-migrations.md b/docs/installer-migrations.md index 6289bfc78..57422a380 100644 --- a/docs/installer-migrations.md +++ b/docs/installer-migrations.md @@ -135,6 +135,7 @@ Required fields: module.exports = { id: '2026-05-11-runtime-layout-example', title: 'Move legacy commands into runtime skills', + description: 'Move legacy runtime command files into the generated skill layout.', introducedIn: '1.50.0', runtimes: ['claude', 'codex', 'gemini'], scopes: ['global', 'local'], @@ -145,6 +146,12 @@ module.exports = { }; ``` +The Installer Migration Authoring Guard Module rejects records that omit `id`, +`title`, `description`, `introducedIn`, `scopes`, `destructive`, or `plan`. +`runtimes` remains optional only for migrations intentionally shared by every +runtime, but scope must always be explicit so an author cannot accidentally +broaden local/global behavior. + The `plan(ctx)` function receives an install context with runtime, scope, target directory, previous manifest, install state, package manifest, and filesystem helpers. It returns actions. It must not mutate disk. @@ -169,6 +176,10 @@ Remove a path only when it is known to be GSD-managed and unchanged from the previous manifest, or when the migration provides a purpose-built detector for an old GSD-owned shape. +Authoring guardrail: every `remove-managed` action must include +`ownershipEvidence` explaining the manifest entry, generated marker, or +purpose-built detector that proves GSD ownership. + Use for retired hooks, old generated agents, deprecated command files, and stale runtime-specific generated artifacts. @@ -203,6 +214,10 @@ Use this for legacy JSON config cleanup such as Codex `hooks.json`, where GSD can prove ownership of individual generated hook commands but not the whole file. +Authoring guardrail: every `rewrite-json` action must include +`ownershipEvidence`, and the migration record must include `runtimeContract` +citing `docs/installer-migrations.md#runtime-configuration-contract-registry`. + ### preserve-user Declare that a path is user-owned and must survive surrounding directory diff --git a/get-shit-done/bin/lib/installer-migration-authoring.cjs b/get-shit-done/bin/lib/installer-migration-authoring.cjs new file mode 100644 index 000000000..d6fb5c500 --- /dev/null +++ b/get-shit-done/bin/lib/installer-migration-authoring.cjs @@ -0,0 +1,117 @@ +'use strict'; + +const path = require('path'); + +function requireNonEmptyString(record, field, source) { + if (typeof record[field] !== 'string' || record[field].trim() === '') { + throw new Error(`migration record must include a non-empty ${field}: ${source}`); + } +} + +function validateStringArray(record, field, source) { + if (record[field] === undefined) return; + if ( + !Array.isArray(record[field]) || + record[field].length === 0 || + record[field].some((value) => typeof value !== 'string' || value.trim() === '') + ) { + throw new Error(`migration record ${field} must be a non-empty string array when provided: ${source}`); + } +} + +function requireStringArray(record, field, source) { + if ( + !Array.isArray(record[field]) || + record[field].length === 0 || + record[field].some((value) => typeof value !== 'string' || value.trim() === '') + ) { + throw new Error(`migration record ${field} must be a non-empty string array: ${source}`); + } +} + +function recordSource(record, fallback) { + return fallback || (record && typeof record.id === 'string' && record.id.trim() ? record.id : ''); +} + +function validateInstallerMigrationRecord(record, source) { + const displaySource = recordSource(record, source); + if (!record || typeof record !== 'object') { + throw new Error(`migration record must export an object: ${displaySource}`); + } + + // Authoring contract follows docs/installer-migrations.md#authoring-workflow + // and docs/adr/0008-installer-migration-module.md#decision. + requireNonEmptyString(record, 'id', displaySource); + requireNonEmptyString(record, 'title', displaySource); + requireNonEmptyString(record, 'description', displaySource); + requireNonEmptyString(record, 'introducedIn', displaySource); + if (typeof record.destructive !== 'boolean') { + throw new Error(`migration record must declare destructive as a boolean: ${displaySource}`); + } + validateStringArray(record, 'runtimes', displaySource); + requireStringArray(record, 'scopes', displaySource); + if (typeof record.plan !== 'function') { + throw new Error(`migration record must include a plan function: ${displaySource}`); + } + + return record; +} + +function actionSource(migration, action) { + const migrationId = migration && typeof migration.id === 'string' ? migration.id : ''; + const relPath = action && typeof action.relPath === 'string' ? action.relPath : ''; + return `${migrationId} ${relPath}`; +} + +function requireActionEvidence(action, field, migration) { + if (typeof action[field] !== 'string' || action[field].trim() === '') { + throw new Error(`migration action ${action.type} must include ${field}: ${actionSource(migration, action)}`); + } +} + +function validateSafeRelPath(relPath, migration, actionType) { + const source = actionSource(migration, { relPath }); + const normalized = relPath.replace(/\\/g, '/'); + if (path.isAbsolute(normalized) || path.win32.isAbsolute(normalized)) { + throw new Error(`migration action ${actionType} relPath must stay inside configDir: ${source}`); + } + const segments = normalized.split('/'); + if (segments.some((segment) => segment === '' || segment === '.' || segment === '..')) { + throw new Error(`migration action ${actionType} relPath must stay inside configDir: ${source}`); + } +} + +function validateInstallerMigrationActions(actions, migration) { + if (!Array.isArray(actions)) { + throw new Error(`migration ${migration.id} plan must return an array`); + } + + for (const action of actions) { + if (!action || typeof action !== 'object') { + throw new Error(`migration action must be an object: ${migration.id}`); + } + if (typeof action.type !== 'string' || action.type.trim() === '') { + throw new Error(`migration action must include a non-empty type: ${migration.id}`); + } + if (typeof action.relPath !== 'string' || action.relPath.trim() === '') { + throw new Error(`migration action ${action.type} must include a non-empty relPath: ${migration.id}`); + } + validateSafeRelPath(action.relPath, migration, action.type); + // Ownership and runtime-contract evidence are required by + // docs/installer-migrations.md#action-types and + // docs/adr/0008-installer-migration-module.md#runtime-contract-decision. + if (action.type === 'remove-managed' || action.type === 'rewrite-json') { + requireActionEvidence(action, 'ownershipEvidence', migration); + } + if (action.type === 'rewrite-json' && (typeof migration.runtimeContract !== 'string' || migration.runtimeContract.trim() === '')) { + throw new Error(`migration action rewrite-json requires migration runtimeContract: ${actionSource(migration, action)}`); + } + } + + return actions; +} + +module.exports = { + validateInstallerMigrationActions, + validateInstallerMigrationRecord, +}; diff --git a/get-shit-done/bin/lib/installer-migrations.cjs b/get-shit-done/bin/lib/installer-migrations.cjs index 8aaa3eeb9..c9d552984 100644 --- a/get-shit-done/bin/lib/installer-migrations.cjs +++ b/get-shit-done/bin/lib/installer-migrations.cjs @@ -3,6 +3,10 @@ const fs = require('fs'); const path = require('path'); const crypto = require('crypto'); +const { + validateInstallerMigrationActions, + validateInstallerMigrationRecord, +} = require('./installer-migration-authoring.cjs'); const MANIFEST_NAME = 'gsd-file-manifest.json'; const INSTALL_STATE_NAME = 'gsd-install-state.json'; @@ -175,19 +179,6 @@ function migrationMatchesContext(migration, { runtime, scope }) { return true; } -function validateMigrationRecord(record, source) { - if (!record || typeof record !== 'object') { - throw new Error(`migration record must export an object: ${source}`); - } - if (typeof record.id !== 'string' || record.id.trim() === '') { - throw new Error(`migration record must include a non-empty id: ${source}`); - } - if (typeof record.plan !== 'function') { - throw new Error(`migration record must include a plan function: ${source}`); - } - return record; -} - function discoverInstallerMigrations({ migrationsDir }) { if (!migrationsDir || !fs.existsSync(migrationsDir)) return []; return fs.readdirSync(migrationsDir, { withFileTypes: true }) @@ -199,7 +190,7 @@ function discoverInstallerMigrations({ migrationsDir }) { delete require.cache[require.resolve(source)]; const exported = require(source); const records = Array.isArray(exported) ? exported : [exported]; - return records.map((record) => validateMigrationRecord(record, source)); + return records.map((record) => validateInstallerMigrationRecord(record, source)); }); } @@ -305,8 +296,11 @@ function planInstallerMigrations({ const manifest = readInstallManifest(configDir); const state = readInstallState(configDir); - const scopedMigrations = migrations.filter((migration) => - migration && migrationMatchesContext(migration, { runtime, scope }) + const validatedMigrations = migrations.map((migration) => + validateInstallerMigrationRecord(migration) + ); + const scopedMigrations = validatedMigrations.filter((migration) => + migrationMatchesContext(migration, { runtime, scope }) ); const applied = appliedMigrationEntries(state); assertAppliedMigrationChecksums(applied, scopedMigrations); @@ -323,12 +317,6 @@ function planInstallerMigrations({ }; for (const migration of pending) { - if (typeof migration.id !== 'string' || migration.id.trim() === '') { - throw new Error('migration id must be a non-empty string'); - } - if (typeof migration.plan !== 'function') { - throw new Error(`migration ${migration.id} must provide a plan function`); - } const plannedActions = migration.plan({ configDir, runtime, @@ -340,9 +328,7 @@ function planInstallerMigrations({ classifyArtifact: classify, readJson: (relPath) => readJson(configDir, relPath), }); - if (!Array.isArray(plannedActions)) { - throw new Error(`migration ${migration.id} plan must return an array`); - } + validateInstallerMigrationActions(plannedActions, migration); const checksum = migrationChecksum(migration); for (const rawAction of plannedActions) { const relPath = normalizeRelPath(rawAction.relPath); 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 ddb93e4f4..cbc31d6c4 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 @@ -151,6 +151,21 @@ module.exports = { const actions = []; for (const relPath of scanBaselineFiles(configDir, runtime)) { + // docs/installer-migrations.md#baseline-preserve-user keeps user-owned + // artifacts out of destructive migration flow; classify later only when + // ownership is not already known. + if (isUserOwnedBaselinePath(relPath)) { + actions.push({ + type: 'baseline-preserve-user', + relPath, + reason: 'known user-owned artifact preserved by first-time migration baseline', + classification: 'user-owned', + originalHash: null, + currentHash: null, + }); + continue; + } + const artifact = classifyArtifact(relPath); if (artifact.classification === 'managed-pristine' || artifact.classification === 'managed-modified') { actions.push({ @@ -174,18 +189,6 @@ module.exports = { continue; } - if (isUserOwnedBaselinePath(relPath)) { - actions.push({ - type: 'baseline-preserve-user', - relPath, - reason: 'known user-owned artifact preserved by first-time migration baseline', - classification: 'user-owned', - originalHash: null, - currentHash, - }); - continue; - } - if (isStaleGsdLookingPath(relPath)) { actions.push({ type: 'prompt-user', diff --git a/get-shit-done/bin/lib/installer-migrations/001-legacy-orphan-files.cjs b/get-shit-done/bin/lib/installer-migrations/001-legacy-orphan-files.cjs index b8868288a..26d263198 100644 --- a/get-shit-done/bin/lib/installer-migrations/001-legacy-orphan-files.cjs +++ b/get-shit-done/bin/lib/installer-migrations/001-legacy-orphan-files.cjs @@ -20,11 +20,19 @@ module.exports = { const actions = []; for (const relPath of LEGACY_ORPHAN_FILES) { const artifact = classifyArtifact(relPath); - if (artifact.classification === 'managed-pristine' || artifact.classification === 'managed-modified') { + if (artifact.classification === 'managed-pristine') { actions.push({ type: 'remove-managed', relPath, reason: 'legacy orphan hook file retired by installer migration', + ownershipEvidence: 'legacy hook path is manifest-managed in gsd-file-manifest.json', + }); + } else if (artifact.classification === 'managed-modified') { + actions.push({ + type: 'backup-and-remove', + relPath, + reason: 'legacy orphan hook file retired by installer migration', + ownershipEvidence: 'legacy hook path is manifest-managed in gsd-file-manifest.json', }); } } diff --git a/get-shit-done/bin/lib/installer-migrations/002-codex-legacy-hooks-json.cjs b/get-shit-done/bin/lib/installer-migrations/002-codex-legacy-hooks-json.cjs index 6a7d9fe1d..38a06479a 100644 --- a/get-shit-done/bin/lib/installer-migrations/002-codex-legacy-hooks-json.cjs +++ b/get-shit-done/bin/lib/installer-migrations/002-codex-legacy-hooks-json.cjs @@ -26,8 +26,8 @@ function pruneLegacyCodexHooksJsonValue(value, configDir) { for (const item of value) { const pruned = pruneLegacyCodexHooksJsonValue(item, configDir); if (pruned.changed) changed = true; - if (!isStructurallyEmpty(pruned.value)) next.push(pruned.value); - else changed = true; + if (pruned.changed && isStructurallyEmpty(pruned.value)) changed = true; + else next.push(pruned.value); } return { value: next, changed }; } @@ -42,8 +42,8 @@ function pruneLegacyCodexHooksJsonValue(value, configDir) { for (const [key, child] of Object.entries(value)) { const pruned = pruneLegacyCodexHooksJsonValue(child, configDir); if (pruned.changed) changed = true; - if (!isStructurallyEmpty(pruned.value)) next[key] = pruned.value; - else changed = true; + if (pruned.changed && isStructurallyEmpty(pruned.value)) changed = true; + else next[key] = pruned.value; } return { value: next, changed }; } @@ -74,6 +74,7 @@ module.exports = { value: pruned.value, deleteIfEmpty: true, reason: 'legacy Codex hooks.json GSD registration retired by installer migration', + ownershipEvidence: 'pruned command matches generated GSD hook command under the install hooks directory', }, ]; }, diff --git a/tests/installer-migration-authoring.test.cjs b/tests/installer-migration-authoring.test.cjs new file mode 100644 index 000000000..9b30c64c1 --- /dev/null +++ b/tests/installer-migration-authoring.test.cjs @@ -0,0 +1,215 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const { + discoverInstallerMigrations, + planInstallerMigrations, +} = require('../get-shit-done/bin/lib/installer-migrations.cjs'); +const { cleanup, createTempDir } = require('./helpers.cjs'); + +function writeMigration(dir, fileName, source) { + fs.writeFileSync(path.join(dir, fileName), source, 'utf8'); +} + +function completeMigrationRecord(overrides = {}) { + return { + id: '2026-05-11-authoring-guard-test', + title: 'Authoring guard test migration', + description: 'Exercise installer migration authoring guardrails.', + introducedIn: '1.50.0', + scopes: ['global', 'local'], + destructive: true, + plan: () => [], + ...overrides, + }; +} + +test('rejects discovered migration records missing required authoring metadata', (t) => { + const migrationsDir = createTempDir('gsd-migration-authoring-'); + t.after(() => cleanup(migrationsDir)); + + writeMigration( + migrationsDir, + '001-missing-title.cjs', + `'use strict'; +module.exports = { + id: '2026-05-11-missing-title', + description: 'Incomplete migration record.', + introducedIn: '1.50.0', + scopes: ['global', 'local'], + destructive: false, + plan: () => [], +}; +` + ); + + assert.throws( + () => discoverInstallerMigrations({ migrationsDir }), + /migration record must include a non-empty title: .*001-missing-title\.cjs/ + ); +}); + +test('rejects direct migration records missing required authoring metadata during planning', (t) => { + const configDir = createTempDir('gsd-migration-authoring-plan-'); + t.after(() => cleanup(configDir)); + + fs.writeFileSync( + path.join(configDir, 'gsd-file-manifest.json'), + JSON.stringify({ version: '1.50.0', timestamp: '2026-05-11T00:00:00.000Z', mode: 'full', files: {} }), + 'utf8' + ); + + assert.throws( + () => planInstallerMigrations({ + configDir, + migrations: [ + { + id: '2026-05-11-incomplete-direct-record', + title: 'Incomplete direct migration record', + introducedIn: '1.50.0', + scopes: ['global', 'local'], + destructive: false, + plan: () => [], + }, + ], + }), + /migration record must include a non-empty description: 2026-05-11-incomplete-direct-record/ + ); +}); + +test('rejects migration records without explicit install scopes', (t) => { + const configDir = createTempDir('gsd-migration-authoring-scope-'); + t.after(() => cleanup(configDir)); + + fs.writeFileSync( + path.join(configDir, 'gsd-file-manifest.json'), + JSON.stringify({ version: '1.50.0', timestamp: '2026-05-11T00:00:00.000Z', mode: 'full', files: {} }), + 'utf8' + ); + + assert.throws( + () => planInstallerMigrations({ + configDir, + migrations: [ + { + id: '2026-05-11-missing-scopes', + title: 'Missing scopes', + description: 'Missing explicit install scope.', + introducedIn: '1.50.0', + destructive: false, + plan: () => [], + }, + ], + }), + /migration record scopes must be a non-empty string array: 2026-05-11-missing-scopes/ + ); +}); + +test('rejects destructive migration actions without ownership evidence', (t) => { + const configDir = createTempDir('gsd-migration-authoring-action-'); + t.after(() => cleanup(configDir)); + + fs.writeFileSync( + path.join(configDir, 'gsd-file-manifest.json'), + JSON.stringify({ + version: '1.50.0', + timestamp: '2026-05-11T00:00:00.000Z', + mode: 'full', + files: {}, + }), + 'utf8' + ); + + assert.throws( + () => planInstallerMigrations({ + configDir, + migrations: [ + completeMigrationRecord({ + plan: () => [ + { + type: 'remove-managed', + relPath: 'hooks/old-hook.js', + reason: 'retired hook', + }, + ], + }), + ], + scope: 'global', + }), + /migration action remove-managed must include ownershipEvidence: 2026-05-11-authoring-guard-test hooks\/old-hook\.js/ + ); +}); + +test('rejects migration actions with absolute or traversal relPaths', (t) => { + const configDir = createTempDir('gsd-migration-authoring-relpath-'); + t.after(() => cleanup(configDir)); + + fs.writeFileSync( + path.join(configDir, 'gsd-file-manifest.json'), + JSON.stringify({ version: '1.50.0', timestamp: '2026-05-11T00:00:00.000Z', mode: 'full', files: {} }), + 'utf8' + ); + + for (const relPath of ['/tmp/outside.js', 'hooks/../outside.js', 'hooks/..', '.']) { + assert.throws( + () => planInstallerMigrations({ + configDir, + migrations: [ + completeMigrationRecord({ + plan: () => [ + { + type: 'remove-managed', + relPath, + reason: 'bad path', + ownershipEvidence: 'test fixture manifest-managed hook', + }, + ], + }), + ], + scope: 'global', + }), + /relPath must stay inside configDir/ + ); + } +}); + +test('rejects runtime config rewrites without a runtime contract citation', (t) => { + const configDir = createTempDir('gsd-migration-authoring-runtime-'); + t.after(() => cleanup(configDir)); + + fs.writeFileSync( + path.join(configDir, 'gsd-file-manifest.json'), + JSON.stringify({ version: '1.50.0', timestamp: '2026-05-11T00:00:00.000Z', mode: 'full', files: {} }), + 'utf8' + ); + fs.writeFileSync(path.join(configDir, 'hooks.json'), '{"hooks":[]}\n', 'utf8'); + + assert.throws( + () => planInstallerMigrations({ + configDir, + migrations: [ + completeMigrationRecord({ + runtimes: ['codex'], + scopes: ['global', 'local'], + plan: () => [ + { + type: 'rewrite-json', + relPath: 'hooks.json', + value: {}, + deleteIfEmpty: true, + reason: 'retired generated Codex hook registration', + ownershipEvidence: 'matches generated GSD hook command path', + }, + ], + }), + ], + runtime: 'codex', + scope: 'global', + }), + /migration action rewrite-json requires migration runtimeContract: 2026-05-11-authoring-guard-test hooks\.json/ + ); +}); diff --git a/tests/installer-migration-install-integration.test.cjs b/tests/installer-migration-install-integration.test.cjs index 9f831fe81..1b2565725 100644 --- a/tests/installer-migration-install-integration.test.cjs +++ b/tests/installer-migration-install-integration.test.cjs @@ -17,11 +17,29 @@ const path = require('node:path'); const crypto = require('node:crypto'); const installModule = require('../bin/install.js'); +const pkg = require('../package.json'); const { install } = installModule; const { createTempDir, cleanup } = require('./helpers.cjs'); const installScript = path.join(__dirname, '..', 'bin', 'install.js'); const SUPPORTED_RUNTIMES = installModule.allRuntimes; +const RUNTIME_INSTALL_CONTRACTS = { + claude: { surface: 'flat-skills', settings: true, packageJson: true }, + antigravity: { surface: 'flat-skills', settings: true, packageJson: true }, + augment: { surface: 'flat-skills', settings: true, packageJson: true }, + cline: { surface: 'clinerules', settings: false, packageJson: false }, + codebuddy: { surface: 'flat-skills', settings: true, packageJson: true }, + codex: { surface: 'flat-skills', settings: false, packageJson: false, codexConfig: true }, + copilot: { surface: 'flat-skills', settings: false, packageJson: false, copilotInstructions: true }, + cursor: { surface: 'flat-skills', settings: false, packageJson: false }, + gemini: { surface: 'commands-gsd', settings: true, packageJson: true }, + hermes: { surface: 'hermes-skills', settings: true, packageJson: true }, + kilo: { surface: 'flat-command', settings: false, packageJson: true }, + opencode: { surface: 'flat-command', settings: true, packageJson: true }, + qwen: { surface: 'flat-skills', settings: true, packageJson: true }, + trae: { surface: 'flat-skills', settings: false, packageJson: false }, + windsurf: { surface: 'flat-skills', settings: false, packageJson: false }, +}; function sha256(content) { return crypto.createHash('sha256').update(content).digest('hex'); @@ -118,13 +136,24 @@ function stripAnsi(value) { return value.replace(/\x1b\[[0-9;]*m/g, ''); } -function runInstallerCli(runtime, targetDir) { +function runInstallerCli(runtime, targetDir, options = {}) { + const { minimal = true } = options; const env = { ...process.env }; delete env.GSD_TEST_MODE; + env.HOME = path.join(path.dirname(targetDir), 'home'); + env.USERPROFILE = env.HOME; return spawnSync( process.execPath, - [installScript, `--${runtime}`, '--global', '--config-dir', targetDir, '--minimal', '--no-sdk'], + [ + installScript, + `--${runtime}`, + '--global', + '--config-dir', + targetDir, + ...(minimal ? ['--minimal'] : []), + '--no-sdk', + ], { encoding: 'utf8', env, @@ -132,6 +161,104 @@ function runInstallerCli(runtime, targetDir) { ); } +function listDirNames(root, relPath) { + const dir = path.join(root, relPath); + if (!fs.existsSync(dir)) return []; + return fs.readdirSync(dir, { withFileTypes: true }).map((entry) => entry.name); +} + +function assertHasGsdDirectory(root, relPath) { + assert.ok( + listDirNames(root, relPath).some((name) => name.startsWith('gsd-')), + `${relPath} should contain generated GSD entries` + ); +} + +function assertFreshInstallContract(runtime, targetDir) { + const contract = RUNTIME_INSTALL_CONTRACTS[runtime]; + assert.ok(contract, `missing runtime install contract for ${runtime}`); + + assert.equal( + fs.readFileSync(path.join(targetDir, 'get-shit-done', 'VERSION'), 'utf8'), + pkg.version, + `${runtime} should install the package VERSION` + ); + assert.ok( + fs.existsSync(path.join(targetDir, 'get-shit-done', 'bin', 'gsd-tools.cjs')), + `${runtime} should install the GSD tool payload` + ); + assert.ok( + fs.existsSync(path.join(targetDir, 'gsd-file-manifest.json')), + `${runtime} should write the install manifest` + ); + + const manifest = JSON.parse(fs.readFileSync(path.join(targetDir, 'gsd-file-manifest.json'), 'utf8')); + assert.equal(manifest.version, pkg.version, `${runtime} manifest should record the package version`); + assert.equal(manifest.mode, 'full', `${runtime} manifest should record a full install`); + assert.ok( + manifest.files['get-shit-done/VERSION'], + `${runtime} manifest should track the installed VERSION file` + ); + + if (contract.surface === 'flat-skills') { + assertHasGsdDirectory(targetDir, 'skills'); + } else if (contract.surface === 'hermes-skills') { + assertHasGsdDirectory(targetDir, path.join('skills', 'gsd')); + assert.ok( + fs.existsSync(path.join(targetDir, 'skills', 'gsd', 'DESCRIPTION.md')), + 'Hermes should install the nested GSD category description' + ); + } else if (contract.surface === 'flat-command') { + assert.ok( + listDirNames(targetDir, 'command').some((name) => name.startsWith('gsd-') && name.endsWith('.md')), + `${runtime} should install flattened command markdown files` + ); + } else if (contract.surface === 'commands-gsd') { + assert.ok( + listDirNames(targetDir, path.join('commands', 'gsd')).length > 0, + `${runtime} should install commands/gsd entries` + ); + } else if (contract.surface === 'clinerules') { + assert.match( + fs.readFileSync(path.join(targetDir, '.clinerules'), 'utf8'), + /GSD workflows live in `get-shit-done\/workflows\/`/, + 'Cline should install root .clinerules guidance' + ); + } + + assert.ok( + listDirNames(targetDir, 'agents').some((name) => name.startsWith('gsd-')), + `${runtime} full install should install agents` + ); + + assert.equal( + fs.existsSync(path.join(targetDir, 'settings.json')), + contract.settings, + `${runtime} settings.json presence should match the runtime contract` + ); + assert.equal( + fs.existsSync(path.join(targetDir, 'package.json')), + contract.packageJson, + `${runtime} package.json presence should match the runtime contract` + ); + + if (contract.codexConfig) { + assert.match( + fs.readFileSync(path.join(targetDir, 'config.toml'), 'utf8'), + /GSD Agent Configuration/, + 'Codex should install config.toml with the GSD marker' + ); + } + + if (contract.copilotInstructions) { + assert.match( + fs.readFileSync(path.join(targetDir, 'copilot-instructions.md'), 'utf8'), + /GSD Configuration/, + 'Copilot should install managed copilot instructions' + ); + } +} + describe('installer migration install integration', { concurrency: false }, () => { let tmpRoot; let codexHome; @@ -272,6 +399,33 @@ describe('installer migration install integration', { concurrency: false }, () = }); for (const runtime of SUPPORTED_RUNTIMES) { + test(`runs a full end-to-end install for ${runtime}`, () => { + const targetDir = path.join(tmpRoot, `.${runtime}-full-install`); + fs.mkdirSync(targetDir, { recursive: true }); + writeFile(targetDir, 'hooks/statusline.js', 'legacy managed hook\n'); + writeManifest(targetDir, { + 'hooks/statusline.js': sha256('legacy managed hook\n'), + }); + + const result = runInstallerCli(runtime, targetDir, { minimal: false }); + + assert.equal(result.status, 0, result.stderr || result.stdout); + const output = stripAnsi(`${result.stdout}\n${result.stderr}`); + assert.match(output, /Installing for /); + assert.match(output, /Installer migrations/); + assert.match(output, /removed\s+hooks\/statusline\.js/); + assert.match(output, /Installed get-shit-done/); + assert.match(output, /Done!/); + assert.equal(fs.existsSync(path.join(targetDir, 'hooks/statusline.js')), false); + + const installState = JSON.parse(fs.readFileSync(path.join(targetDir, 'gsd-install-state.json'), 'utf8')); + assert.ok( + installState.appliedMigrations.some((entry) => entry.id === '2026-05-11-legacy-orphan-files'), + `${runtime} should track the applied cleanup migration in install state` + ); + assertFreshInstallContract(runtime, targetDir); + }); + test(`runs managed cleanup migrations for ${runtime}`, () => { const targetDir = path.join(tmpRoot, `.${runtime}-managed-cleanup`); fs.mkdirSync(targetDir, { recursive: true }); diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index fde9cdc3e..6d081261d 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -48,6 +48,26 @@ function writeManifest(root, files) { ); } +function migrationRecord(overrides = {}) { + return { + id: '2026-05-11-remove-old-hook', + title: 'Remove retired hook', + description: 'Remove retired hook', + introducedIn: '1.50.0', + scopes: ['global', 'local'], + destructive: true, + plan: () => [ + { + type: 'remove-managed', + relPath: 'hooks/old-hook.js', + reason: 'retired hook', + ownershipEvidence: 'test fixture manifest-managed hook', + }, + ], + ...overrides, + }; +} + function legacyCodexHook(configDir) { return { hooks: [ @@ -158,6 +178,51 @@ test('preserves unknown files discovered in known install surfaces by default', } }); +test('preserves user-owned skill files during baseline without hashing their content', (t) => { + const configDir = createTempInstall(); + const originalOpenSync = fs.openSync; + t.after(() => { + fs.openSync = originalOpenSync; + cleanup(configDir); + }); + + writeFile(configDir, 'skills/custom-user-skill/SKILL.md', 'user skill\n'); + writeManifest(configDir, {}); + const userSkillPath = path.join(configDir, 'skills/custom-user-skill/SKILL.md'); + fs.openSync = (filePath, ...args) => { + if (path.resolve(String(filePath)) === path.resolve(userSkillPath)) { + throw new Error('user-owned skill content should not be hashed during baseline'); + } + return originalOpenSync.call(fs, filePath, ...args); + }; + + const result = runInstallerMigrations({ + configDir, + runtime: 'claude', + scope: 'global', + migrations: [firstTimeBaselineMigration], + baselineScan: true, + now: () => '2026-05-11T00:00:01.000Z', + }); + + assert.deepEqual( + result.plan.actions.map((action) => ({ + type: action.type, + relPath: action.relPath, + classification: action.classification, + currentHash: action.currentHash, + })), + [ + { + type: 'baseline-preserve-user', + relPath: 'skills/custom-user-skill/SKILL.md', + classification: 'user-owned', + currentHash: null, + }, + ] + ); +}); + test('blocks stale GSD-looking baseline artifacts for explicit user choice', () => { const configDir = createTempInstall(); try { @@ -259,18 +324,9 @@ test('plans a pending migration against an unchanged managed file', () => { 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', - }, - ], - }, + migrationRecord(), ], + scope: 'global', now: () => '2026-05-11T00:00:00.000Z', }); @@ -314,18 +370,9 @@ test('plans backup before removal for a modified managed file', () => { 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', - }, - ], - }, + migrationRecord(), ], + scope: 'global', now: () => '2026-05-11T00:00:00.000Z', }); @@ -350,18 +397,18 @@ test('blocks removal of unknown files by preserving them by default', () => { const plan = planInstallerMigrations({ configDir, migrations: [ - { - id: '2026-05-11-remove-old-hook', - description: 'Remove retired hook', + migrationRecord({ plan: () => [ { type: 'remove-managed', relPath: 'hooks/custom-user-hook.js', reason: 'retired hook', + ownershipEvidence: 'test fixture asks to retire a matching hook path', }, ], - }, + }), ], + scope: 'global', now: () => '2026-05-11T00:00:00.000Z', }); @@ -399,21 +446,35 @@ test('computes each migration checksum once per planned migration', (t) => { }); let checksumReads = 0; const migration = { - id: '2026-05-11-remove-two-hooks', - description: 'Remove retired hooks', + ...migrationRecord({ + id: '2026-05-11-remove-two-hooks', + title: 'Remove two retired hooks', + description: 'Remove retired hooks', + plan: () => [ + { + type: 'remove-managed', + relPath: 'hooks/first.js', + reason: 'retired hook', + ownershipEvidence: 'test fixture manifest-managed hook', + }, + { + type: 'remove-managed', + relPath: 'hooks/second.js', + reason: 'retired hook', + ownershipEvidence: 'test fixture manifest-managed hook', + }, + ], + }), 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], + scope: 'global', }); assert.equal(plan.actions.length, 2); @@ -457,18 +518,9 @@ test('applies an unblocked plan with a journal and install-state update', () => 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', - }, - ], - }, + migrationRecord(), ], + scope: 'global', now: () => '2026-05-11T00:00:00.000Z', }); @@ -561,16 +613,9 @@ test('stores modified-file backups under the unique migration run journal', (t) 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', - }], - }, + migrationRecord(), ], + scope: 'global', now: () => '2026-05-11T00:00:00.000Z', }); @@ -598,16 +643,9 @@ test('successful migration rollback removes run-scoped backup directories', (t) 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', - }], - }, + migrationRecord(), ], + scope: 'global', }); const result = applyInstallerMigrationPlan({ @@ -686,18 +724,24 @@ test('rollback handle restores files and install state after a successful apply' const plan = planInstallerMigrations({ configDir, migrations: [ - { + migrationRecord({ id: '2026-05-11-remove-old-hook', + title: 'Remove retired hook', description: 'Remove retired hook', + introducedIn: '1.50.0', + scopes: ['global'], + destructive: true, plan: () => [ { type: 'remove-managed', relPath: 'hooks/old-hook.js', reason: 'retired hook', + ownershipEvidence: 'test fixture manifest-managed hook', }, ], - }, + }), ], + scope: 'global', now: () => '2026-05-11T00:00:00.000Z', }); @@ -967,14 +1011,13 @@ test('skips migration records already present in install state', () => { const plan = planInstallerMigrations({ configDir, migrations: [ - { - id: '2026-05-11-remove-old-hook', - description: 'Remove retired hook', + migrationRecord({ plan: () => { throw new Error('already-applied migration planner must not run'); }, - }, + }), ], + scope: 'global', now: () => '2026-05-11T00:00:03.000Z', }); @@ -994,12 +1037,15 @@ test('marks zero-action pending migrations as applied', () => { const result = runInstallerMigrations({ configDir, migrations: [ - { + migrationRecord({ id: '2026-05-11-noop-cleanup', + title: 'No-op cleanup', description: 'No-op cleanup', + destructive: false, plan: () => [], - }, + }), ], + scope: 'global', now: () => '2026-05-11T00:00:06.000Z', }); @@ -1033,13 +1079,12 @@ test('refuses to plan an already-applied migration whose checksum changed', () = () => planInstallerMigrations({ configDir, migrations: [ - { - id: '2026-05-11-remove-old-hook', + migrationRecord({ checksum: 'sha256:new-definition', - description: 'Remove retired hook', plan: () => [], - }, + }), ], + scope: 'global', }), /applied migration checksum changed/ ); @@ -1069,16 +1114,17 @@ test('ignores checksum drift for applied migrations outside the active runtime s runtime: 'claude', scope: 'global', migrations: [ - { + migrationRecord({ id: '2026-05-11-codex-only', + title: 'Codex-only migration', + description: 'Codex-only migration', checksum: 'sha256:new-definition', runtimes: ['codex'], scopes: ['global'], - description: 'Codex-only migration', plan: () => { throw new Error('out-of-scope migration planner must not run'); }, - }, + }), ], }); @@ -1097,12 +1143,12 @@ test('discovers migration records from a directory in filename order', () => { fs.mkdirSync(migrationsDir, { recursive: true }); fs.writeFileSync( path.join(migrationsDir, '002-second.cjs'), - "module.exports = { id: 'second', description: 'second', plan: () => [] };\n", + "module.exports = { id: 'second', title: 'Second', description: 'second', introducedIn: '1.50.0', scopes: ['global', 'local'], destructive: false, plan: () => [] };\n", 'utf8' ); fs.writeFileSync( path.join(migrationsDir, '001-first.cjs'), - "module.exports = { id: 'first', description: 'first', plan: () => [] };\n", + "module.exports = { id: 'first', title: 'First', description: 'first', introducedIn: '1.50.0', scopes: ['global', 'local'], destructive: false, plan: () => [] };\n", 'utf8' ); @@ -1123,18 +1169,21 @@ test('rejects migration actions that escape the install root', () => { () => planInstallerMigrations({ configDir, migrations: [ - { + migrationRecord({ id: '2026-05-11-bad-path', + title: 'Bad path', description: 'Bad path', plan: () => [ { type: 'remove-managed', relPath: 'hooks/../../outside.js', reason: 'bad path', + ownershipEvidence: 'test fixture manifest-managed hook', }, ], - }, + }), ], + scope: 'global', }), /relPath must stay inside configDir/ ); @@ -1153,18 +1202,21 @@ test('rejects migration actions that normalize to the install root', () => { () => planInstallerMigrations({ configDir, migrations: [ - { + migrationRecord({ id: `2026-05-11-bad-path-${relPath.replace(/[^a-z0-9]/gi, '-')}`, + title: 'Bad path', description: 'Bad path', plan: () => [ { type: 'remove-managed', relPath, reason: 'bad path', + ownershipEvidence: 'test fixture manifest-managed hook', }, ], - }, + }), ], + scope: 'global', }), /relPath must stay inside configDir/ ); @@ -1198,6 +1250,41 @@ test('runs discovered installer migrations against manifest-managed legacy orpha } }); +test('backs up modified legacy orphan files before removing them', () => { + const configDir = createTempInstall(); + try { + writeFile(configDir, 'hooks/statusline.js', 'user modified legacy hook\n'); + writeManifest(configDir, { + 'hooks/statusline.js': sha256('legacy managed hook\n'), + }); + + const plan = planInstallerMigrations({ + configDir, + migrations: discoverInstallerMigrations({ + migrationsDir: path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'installer-migrations'), + }), + scope: 'global', + now: () => '2026-05-11T00:00:05.000Z', + }); + const action = plan.actions.find((item) => item.relPath === 'hooks/statusline.js'); + + assert.equal(action.type, 'backup-and-remove'); + + const result = runInstallerMigrations({ + configDir, + scope: 'global', + now: () => '2026-05-11T00:00:05.000Z', + }); + const journal = JSON.parse(fs.readFileSync(path.join(configDir, result.journalRelPath), 'utf8')); + const backupRelPath = journal.actions.find((item) => item.relPath === 'hooks/statusline.js').backupRelPath; + + assert.equal(fs.existsSync(path.join(configDir, 'hooks/statusline.js')), false); + assert.equal(fs.readFileSync(path.join(configDir, backupRelPath), 'utf8'), 'user modified legacy hook\n'); + } finally { + cleanup(configDir); + } +}); + test('runs a Codex legacy hooks.json cleanup migration without removing user hooks', () => { const configDir = createTempInstall(); try { @@ -1234,6 +1321,40 @@ test('runs a Codex legacy hooks.json cleanup migration without removing user hoo } }); +test('preserves unrelated empty hooks.json structure while pruning legacy Codex hooks', () => { + const configDir = createTempInstall(); + try { + writeFile( + configDir, + 'hooks.json', + JSON.stringify({ + SessionStart: [ + legacyCodexHook(configDir), + { hooks: [] }, + { metadata: null }, + ], + }, null, 2) + ); + writeManifest(configDir, {}); + + runInstallerMigrations({ + configDir, + runtime: 'codex', + scope: 'global', + now: () => '2026-05-11T00:00:06.000Z', + }); + + const hooksJson = JSON.parse(fs.readFileSync(path.join(configDir, 'hooks.json'), 'utf8')); + + assert.deepEqual(hooksJson.SessionStart, [ + { hooks: [] }, + { metadata: null }, + ]); + } finally { + cleanup(configDir); + } +}); + test('skips runtime-specific migration records for other runtimes', () => { const configDir = createTempInstall(); try {