From 3416dda9d4cb488630449d5d42bca75f8ba768f6 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 21 Jun 2026 21:15:11 -0400 Subject: [PATCH] refactor(#1556): wire installRuntimeArtifacts to install plan (#1563) --- bin/install.js | 58 +++++++---------- tests/install-runtime-artifacts.test.cjs | 83 ++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 33 deletions(-) diff --git a/bin/install.js b/bin/install.js index 0fc5504e7..b62071f65 100755 --- a/bin/install.js +++ b/bin/install.js @@ -363,6 +363,9 @@ const { const { resolveRuntimeArtifactLayout, } = require(path.join(_gsdLibDir, 'runtime-artifact-layout.cjs')); +const { + createRuntimeArtifactInstallPlan, +} = require(path.join(_gsdLibDir, 'runtime-artifact-install-plan.cjs')); const { planLegacyCleanup, applyLegacyCleanup, @@ -7008,36 +7011,25 @@ function installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile) { _runLegacyInstallMigrations(runtime, configDir, scope); const layout = resolveRuntimeArtifactLayout(runtime, configDir, scope); - - // Compute pathPrefix once for the rewrite step (same derivation as the - // top-level install() function). - const _resolvedTarget = path.resolve(configDir).replace(/\\/g, '/'); - const _homeDir = os.homedir().replace(/\\/g, '/'); - const pathPrefix = computePathPrefix({ - isGlobal: scope === 'global', - isOpencode: runtime === 'opencode', - isWindowsHost: process.platform === 'win32', - resolvedTarget: _resolvedTarget, - homeDir: _homeDir, + const planResult = createRuntimeArtifactInstallPlan({ + layout, + resolvedProfile, + homedir: () => os.homedir(), + platform: process.platform, + resolveAttribution: getCommitAttribution, }); - for (const kind of layout.kinds) { - const staged = kind.stage(resolvedProfile); - // stagedForCopy: the directory to copy from (may differ from staged if rewrites - // produce a temp copy — see applyRuntimeContentRewritesForCommandsInPlace). - let stagedForCopy = staged; - const isGlobal = scope === 'global'; - if (kind.kind === 'skills' || kind.kind === 'kimi-agents') { - applyRuntimeContentRewritesInPlace(staged, runtime, pathPrefix, isGlobal, getCommitAttribution(runtime)); - } else if (kind.kind === 'commands') { - // Returns a temp dir with rewritten content so source files are never mutated. - stagedForCopy = applyRuntimeContentRewritesForCommandsInPlace(staged, runtime, pathPrefix, isGlobal, getCommitAttribution(runtime)); + const cleanupDirs = planResult.ok ? planResult.plan.cleanupDirs : planResult.cleanupDirs; + try { + if (!planResult.ok) { + throw new Error(planResult.message); } - // applyRuntimeContentRewritesForCommandsInPlace() returns a fresh mkdtemp dir under - // os.tmpdir() (gsd-cmd-rewrites-*); remove it once copied so it does not accumulate (#856). - const tempToClean = stagedForCopy !== staged ? stagedForCopy : null; - try { - const dest = path.join(layout.configDir, kind.destSubpath); + + const kindsByName = new Map(layout.kinds.map((kind) => [kind.kind, kind])); + for (const item of planResult.plan.items) { + const kind = kindsByName.get(item.kind); + if (!kind) throw new Error(`Install plan returned unknown artifact kind: ${item.kind}`); + const dest = item.destDir; fs.mkdirSync(dest, { recursive: true }); if (kind.kind === 'skills' && fs.existsSync(dest)) { // Pre-prune: snapshot user-owned content before _removeGsdEntries wipes it, @@ -7064,7 +7056,7 @@ function installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile) { } _removeGsdEntries(dest, kind); - _copyStaged(stagedForCopy, dest, kind); + _copyStaged(item.sourceDir, dest, kind); // Restore user-owned dirs after the prune+copy for (const [dirName, snap] of toPreserve) { @@ -7074,13 +7066,13 @@ function installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile) { // For non-skills kinds (commands, agents): no user content to preserve; // just prune stale gsd-* entries and copy new ones. _removeGsdEntries(dest, kind); - _copyStaged(stagedForCopy, dest, kind); - } - } finally { - if (tempToClean) { - try { fs.rmSync(tempToClean, { recursive: true, force: true }); } catch { /* best-effort */ } + _copyStaged(item.sourceDir, dest, kind); } } + } finally { + for (const dir of cleanupDirs) { + try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* best-effort */ } + } } // Hermes: after the install loop has written all gsd-/ dirs to diff --git a/tests/install-runtime-artifacts.test.cjs b/tests/install-runtime-artifacts.test.cjs index 0ba9776b1..6573070f3 100644 --- a/tests/install-runtime-artifacts.test.cjs +++ b/tests/install-runtime-artifacts.test.cjs @@ -46,8 +46,91 @@ const REAL_COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd'); const MANIFEST = loadSkillsManifest(REAL_COMMANDS_DIR); const RESOLVED_CORE = resolveProfile({ modes: ['core'], manifest: MANIFEST }); +function loadFreshInstallerWithInstallPlanStub(stub) { + const installPath = require.resolve('../bin/install.js'); + const planPath = require.resolve('../gsd-core/bin/lib/runtime-artifact-install-plan.cjs'); + const planModule = require(planPath); + const original = planModule.createRuntimeArtifactInstallPlan; + planModule.createRuntimeArtifactInstallPlan = stub; + delete require.cache[installPath]; + const installer = require('../bin/install.js'); + + return { + installer, + restore() { + planModule.createRuntimeArtifactInstallPlan = original; + delete require.cache[installPath]; + }, + }; +} + // ─── Section 6: installRuntimeArtifacts — parameterised layout loop ────────── +describe('installRuntimeArtifacts — consumes Runtime Artifact Install Plan Module', () => { + test('executes returned copy items and cleanup obligations', (t) => { + const configDir = createTempDir('gsd-install-plan-adapter-'); + const sourceDir = createTempDir('gsd-install-plan-source-'); + const cleanupDir = createTempDir('gsd-install-plan-cleanup-'); + t.after(() => { + cleanup(configDir); + cleanup(sourceDir); + cleanup(cleanupDir); + }); + + fs.writeFileSync(path.join(sourceDir, 'proof.md'), '# proof\n'); + fs.writeFileSync(path.join(cleanupDir, 'temp.md'), '# cleanup\n'); + let planArgs; + const { installer, restore } = loadFreshInstallerWithInstallPlanStub((args) => { + planArgs = args; + return { + ok: true, + plan: { + cleanupDirs: [cleanupDir], + items: [ + { kind: 'commands', sourceDir, destDir: path.join(configDir, 'commands', 'gsd') }, + ], + }, + }; + }); + t.after(restore); + + installer.installRuntimeArtifacts('gemini', configDir, 'global', RESOLVED_CORE); + + assert.strictEqual(planArgs.layout.runtime, 'gemini'); + assert.strictEqual(planArgs.layout.configDir, configDir); + assert.strictEqual(planArgs.layout.scope, 'global'); + assert.strictEqual(planArgs.resolvedProfile, RESOLVED_CORE); + assert.strictEqual(planArgs.resolveAttribution('gemini'), undefined); + assert.ok(fs.existsSync(path.join(configDir, 'commands', 'gsd', 'proof.md'))); + assert.ok(!fs.existsSync(cleanupDir), 'returned cleanup dir must be removed after copy'); + }); + + test('cleans returned obligations when planning fails', (t) => { + const configDir = createTempDir('gsd-install-plan-fail-'); + const cleanupDir = createTempDir('gsd-install-plan-fail-cleanup-'); + t.after(() => { + cleanup(configDir); + cleanup(cleanupDir); + }); + + fs.writeFileSync(path.join(cleanupDir, 'temp.md'), '# cleanup\n'); + const { installer, restore } = loadFreshInstallerWithInstallPlanStub(() => ({ + ok: false, + kind: 'rewrite_failed', + failedKind: 'commands', + message: 'planned failure', + cleanupDirs: [cleanupDir], + })); + t.after(restore); + + assert.throws( + () => installer.installRuntimeArtifacts('gemini', configDir, 'global', RESOLVED_CORE), + /planned failure/, + ); + assert.ok(!fs.existsSync(cleanupDir), 'failure cleanup dir must be removed'); + }); +}); + const SKILLS_RUNTIMES_LAYOUT = [ 'claude', 'cursor', 'codex', 'copilot', 'antigravity', 'windsurf', 'augment', 'trae', 'qwen', 'kimi', 'codebuddy',