From 4d578394eb335f2c8d1e1f4c133ce0a23bb6afa8 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 17 May 2026 00:24:04 -0400 Subject: [PATCH] refactor(3663): migrate applySurface to layout-driven loop; add skills kind to _syncGsdDir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - applySurface(runtimeConfigDir, commandsDir, agentsDir, manifest, clusterMap) → applySurface(runtimeConfigDir, layout, manifest, clusterMap) - _syncGsdDir extended to handle skills kind (dirs not files, prefix-gated removal) - _findInstallSource/_findAgentsSource deleted; listSurface now uses findInstallSourceRoot() from runtime-artifact-layout.cjs - findInstallSourceRoot exported from runtime-artifact-layout.cjs - test(3663): update surface-apply.test.cjs to new layout-passing shape + add skills kind test Co-Authored-By: Claude Sonnet 4.6 --- .../bin/lib/runtime-artifact-layout.cjs | 2 +- get-shit-done/bin/lib/surface.cjs | 195 +++++++----------- tests/surface-apply.test.cjs | 86 ++++++-- 3 files changed, 147 insertions(+), 136 deletions(-) diff --git a/get-shit-done/bin/lib/runtime-artifact-layout.cjs b/get-shit-done/bin/lib/runtime-artifact-layout.cjs index 25412b62c..2aac59b93 100644 --- a/get-shit-done/bin/lib/runtime-artifact-layout.cjs +++ b/get-shit-done/bin/lib/runtime-artifact-layout.cjs @@ -228,4 +228,4 @@ function resolveRuntimeArtifactLayout(runtime, configDir, scope = 'global') { return { runtime, configDir, kinds }; } -module.exports = { resolveRuntimeArtifactLayout }; +module.exports = { resolveRuntimeArtifactLayout, findInstallSourceRoot }; diff --git a/get-shit-done/bin/lib/surface.cjs b/get-shit-done/bin/lib/surface.cjs index 00168c5ce..5fc22057c 100644 --- a/get-shit-done/bin/lib/surface.cjs +++ b/get-shit-done/bin/lib/surface.cjs @@ -13,8 +13,8 @@ * readSurface(runtimeConfigDir) * writeSurface(runtimeConfigDir, surfaceState) * resolveSurface(runtimeConfigDir, manifest, clusterMap) - * applySurface(runtimeConfigDir, commandsDir, agentsDir, manifest, clusterMap) - * listSurface(runtimeConfigDir, manifest, clusterMap) + * applySurface(runtimeConfigDir, layout, manifest, clusterMap) + * listSurface(runtimeConfigDir, layout, manifest, clusterMap) */ const fs = require('fs'); @@ -31,6 +31,7 @@ const { PROFILES, } = require('./install-profiles.cjs'); const { CLUSTERS, allClusteredSkills } = require('./clusters.cjs'); +const { findInstallSourceRoot } = require('./runtime-artifact-layout.cjs'); const SURFACE_FILE_NAME = '.gsd-surface.json'; @@ -194,146 +195,99 @@ function resolveSurface(runtimeConfigDir, manifest, clusterMap) { // --------------------------------------------------------------------------- /** - * Re-stage the active surface to commandsDir and agentsDir in-place. - * Only touches files matching `gsd-` prefix or `*.md` in commandsDir. - * Never touches non-`gsd-*` files. - * - * Steps: - * 1. Resolve surface → active skill/agent sets - * 2. Stage to temp dirs via stageSkillsForProfile / stageAgentsForProfile - * 3. Find the install source (where skill files live) - * 4. Sync: copy missing, delete superseded (gsd-only) + * Re-stage the active surface using the resolved layout. + * Iterates layout.kinds and syncs each artifact kind to its destination. * * @param {string} runtimeConfigDir - * @param {string} commandsDir runtime commands/gsd dir (resolved per-runtime by callers) - * @param {string} agentsDir runtime agents dir (resolved per-runtime by callers) + * @param {import('./runtime-artifact-layout.cjs').Layout} layout * @param {Map} manifest * @param {Object} [clusterMap] */ -function applySurface(runtimeConfigDir, commandsDir, agentsDir, manifest, clusterMap) { +function applySurface(runtimeConfigDir, layout, manifest, clusterMap) { const resolved = resolveSurface(runtimeConfigDir, manifest, clusterMap); - - // Find install source - const srcCommandsDir = _findInstallSource(runtimeConfigDir); - - // Stage skills - const stagedSkills = stageSkillsForProfile(srcCommandsDir, resolved); - - // Sync commandsDir from stagedSkills - _syncGsdDir(stagedSkills, commandsDir, 'commands'); - - // Stage and sync agents - if (agentsDir && fs.existsSync(agentsDir)) { - const srcAgentsDir = _findAgentsSource(runtimeConfigDir); - if (srcAgentsDir) { - const stagedAgents = stageAgentsForProfile(srcAgentsDir, resolved); - _syncGsdDir(stagedAgents, agentsDir, 'agents'); + for (const kind of layout.kinds) { + const staged = kind.stage(resolved); + const dest = path.join(layout.configDir, kind.destSubpath); + if (fs.existsSync(dest)) { + _syncGsdDir(staged, dest, kind); } } + return resolved; } /** * Sync destination directory from staged source. - * Adds files present in staged but missing in dest. - * Removes gsd-prefixed .md files in dest not present in staged. - * Never touches non-gsd files. + * + * For 'commands' kind: iterate *.md files in destDir, remove if not in staged set. + * For 'agents' kind: same, but only remove files starting with 'gsd-' prefix. + * For 'skills' kind: iterate directories in destDir matching kind.prefix; add missing + * by copying recursively; remove dirs not in staged set. Preserves dirs not matching + * the prefix (user-owned skills). * * @param {string} stagedDir source (staged temp dir or original) * @param {string} destDir runtime destination - * @param {'commands'|'agents'} context + * @param {import('./runtime-artifact-layout.cjs').ArtifactKind|'commands'|'agents'} kind */ -function _syncGsdDir(stagedDir, destDir, context) { +function _syncGsdDir(stagedDir, destDir, kind) { if (!fs.existsSync(stagedDir)) return; fs.mkdirSync(destDir, { recursive: true }); - const stagedFiles = new Set( - fs.readdirSync(stagedDir).filter(f => f.endsWith('.md')) - ); + // Normalize: allow legacy string context for backward-compat with internal callers + const kindName = (typeof kind === 'string') ? kind : kind.kind; + const kindPrefix = (typeof kind === 'object' && kind !== null) ? kind.prefix : 'gsd-'; - // Copy missing files from staged to dest - for (const file of stagedFiles) { - const destFile = path.join(destDir, file); - if (!fs.existsSync(destFile)) { - fs.copyFileSync(path.join(stagedDir, file), destFile); - } else { - // Overwrite to ensure content is current - fs.copyFileSync(path.join(stagedDir, file), destFile); - } - } + if (kindName === 'skills') { + // Skills kind: work with directories, not files. + // Each staged entry is a directory named ${prefix}${stem}. + const stagedDirs = new Set( + fs.readdirSync(stagedDir).filter(entry => { + return fs.statSync(path.join(stagedDir, entry)).isDirectory(); + }) + ); - // Remove gsd-only files from dest that aren't in staged set - // For commands dir: all .md files are gsd skills - // For agents dir: only gsd-* files - const destEntries = fs.readdirSync(destDir).filter(f => f.endsWith('.md')); - for (const file of destEntries) { - if (context === 'agents' && !file.startsWith('gsd-')) continue; - if (!stagedFiles.has(file)) { - try { fs.unlinkSync(path.join(destDir, file)); } catch {} - } - } -} - -/** - * Find the install source commands/gsd directory. - * Checks the runtime's `.gsd-source` marker (sibling of the surface state file), - * then walks up from __dirname to find the installed package source. - * - * @param {string} runtimeConfigDir - * @returns {string} path to install source commands/gsd - */ -function _findInstallSource(runtimeConfigDir) { - // Check for .gsd-source marker - const sourceMarker = path.join(runtimeConfigDir, '.gsd-source'); - if (fs.existsSync(sourceMarker)) { - try { - const src = fs.readFileSync(sourceMarker, 'utf8').trim(); - if (src && fs.existsSync(src)) return src; - } catch {} - } - - // Walk up from this module's dir to find commands/gsd - let dir = __dirname; - for (let i = 0; i < 6; i++) { - const candidate = path.join(dir, 'commands', 'gsd'); - if (fs.existsSync(candidate)) return candidate; - const parent = path.dirname(dir); - if (parent === dir) break; - dir = parent; - } - - // Fallback: the runtimeConfigDir itself - return path.join(runtimeConfigDir, '..', 'commands', 'gsd'); -} - -/** - * Find the install source agents directory. - * - * @param {string} runtimeConfigDir - * @returns {string|null} - */ -function _findAgentsSource(runtimeConfigDir) { - // Prefer .gsd-source sibling marker (commands/gsd) and derive agents from it. - const sourceMarker = path.join(runtimeConfigDir, '.gsd-source'); - if (fs.existsSync(sourceMarker)) { - try { - const commandsSrc = fs.readFileSync(sourceMarker, 'utf8').trim(); - if (commandsSrc && fs.existsSync(commandsSrc)) { - const commandsParent = path.dirname(commandsSrc); // .../commands - const candidate = path.resolve(commandsParent, '..', 'agents'); - if (fs.existsSync(candidate)) return candidate; + // Copy missing dirs from staged to dest + for (const dirName of stagedDirs) { + const destSubDir = path.join(destDir, dirName); + if (!fs.existsSync(destSubDir)) { + fs.cpSync(path.join(stagedDir, dirName), destSubDir, { recursive: true }); + } else { + // Overwrite to ensure content is current + fs.cpSync(path.join(stagedDir, dirName), destSubDir, { recursive: true }); } - } catch {} - } + } - let dir = __dirname; - for (let i = 0; i < 6; i++) { - const candidate = path.join(dir, 'agents'); - if (fs.existsSync(candidate)) return candidate; - const parent = path.dirname(dir); - if (parent === dir) break; - dir = parent; + // Remove dirs in dest that match the prefix but are not in staged set + const destEntries = fs.readdirSync(destDir); + for (const entry of destEntries) { + const entryPath = path.join(destDir, entry); + if (!fs.statSync(entryPath).isDirectory()) continue; + if (!entry.startsWith(kindPrefix)) continue; // only touch prefix-matched dirs + if (!stagedDirs.has(entry)) { + try { fs.rmSync(entryPath, { recursive: true, force: true }); } catch {} + } + } + } else { + // commands / agents kind: work with .md files + const stagedFiles = new Set( + fs.readdirSync(stagedDir).filter(f => f.endsWith('.md')) + ); + + // Copy files from staged to dest (overwrite to keep content current) + for (const file of stagedFiles) { + fs.copyFileSync(path.join(stagedDir, file), path.join(destDir, file)); + } + + // Remove gsd-only files from dest that aren't in staged set + // For commands dir: all .md files are gsd skills + // For agents dir: only gsd-* files + const destEntries = fs.readdirSync(destDir).filter(f => f.endsWith('.md')); + for (const file of destEntries) { + if (kindName === 'agents' && !file.startsWith('gsd-')) continue; + if (!stagedFiles.has(file)) { + try { fs.unlinkSync(path.join(destDir, file)); } catch {} + } + } } - return null; } // --------------------------------------------------------------------------- @@ -344,7 +298,7 @@ function _findAgentsSource(runtimeConfigDir) { * List the currently enabled and disabled skills with token cost. * * Token cost = sum of description lengths ÷ 4 (mirrors audit script). - * Descriptions are read from the installed commandsDir skill files. + * Descriptions are read from the install source (findInstallSourceRoot). * * @param {string} runtimeConfigDir * @param {Map} manifest @@ -366,7 +320,7 @@ function listSurface(runtimeConfigDir, manifest, clusterMap) { const disabled = allStems.filter(s => !enabledSet.has(s)).sort(); // Compute token cost by reading descriptions from the install source - const srcCommandsDir = _findInstallSource(runtimeConfigDir); + const srcCommandsDir = findInstallSourceRoot(); let tokenCost = 0; for (const stem of enabled) { const filePath = path.join(srcCommandsDir, `${stem}.md`); @@ -393,6 +347,5 @@ module.exports = { applySurface, listSurface, // Exported for testing - _findInstallSource, _syncGsdDir, }; diff --git a/tests/surface-apply.test.cjs b/tests/surface-apply.test.cjs index 05152c6c7..da148c8d1 100644 --- a/tests/surface-apply.test.cjs +++ b/tests/surface-apply.test.cjs @@ -12,6 +12,7 @@ const os = require('os'); const { writeSurface, applySurface } = require('../get-shit-done/bin/lib/surface.cjs'); const { loadSkillsManifest, writeActiveProfile } = require('../get-shit-done/bin/lib/install-profiles.cjs'); const { CLUSTERS } = require('../get-shit-done/bin/lib/clusters.cjs'); +const { resolveRuntimeArtifactLayout } = require('../get-shit-done/bin/lib/runtime-artifact-layout.cjs'); const REAL_COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd'); const REAL_AGENTS_DIR = path.join(__dirname, '..', 'agents'); @@ -21,20 +22,18 @@ function tmpDir() { } /** - * Create a minimal fixture install dir structure. - * Returns { runtimeConfigDir, commandsDir, agentsDir }. - * runtimeConfigDir has a .gsd-source marker pointing to REAL_COMMANDS_DIR. + * Create a minimal fixture install dir structure for claude/local layout. + * runtimeConfigDir is the layout configDir. + * commandsDir = runtimeConfigDir/commands/gsd + * agentsDir = runtimeConfigDir/agents */ function createFixtureRuntime() { const base = tmpDir(); - const runtimeConfigDir = path.join(base, 'config'); - const commandsDir = path.join(base, 'commands', 'gsd'); - const agentsDir = path.join(base, 'agents'); - fs.mkdirSync(runtimeConfigDir, { recursive: true }); + const runtimeConfigDir = base; + const commandsDir = path.join(runtimeConfigDir, 'commands', 'gsd'); + const agentsDir = path.join(runtimeConfigDir, 'agents'); fs.mkdirSync(commandsDir, { recursive: true }); fs.mkdirSync(agentsDir, { recursive: true }); - // Write source marker so surface.cjs can find the install source - fs.writeFileSync(path.join(runtimeConfigDir, '.gsd-source'), REAL_COMMANDS_DIR, 'utf8'); return { base, runtimeConfigDir, commandsDir, agentsDir }; } @@ -50,7 +49,8 @@ describe('applySurface', () => { explicitRemoves: [], }); const manifest = loadSkillsManifest(REAL_COMMANDS_DIR); - applySurface(runtimeConfigDir, commandsDir, agentsDir, manifest, CLUSTERS); + const layout = resolveRuntimeArtifactLayout('claude', runtimeConfigDir, 'local'); + applySurface(runtimeConfigDir, layout, manifest, CLUSTERS); const files = fs.readdirSync(commandsDir).filter(f => f.endsWith('.md')); // Every file should be a real stem we know about @@ -79,7 +79,8 @@ describe('applySurface', () => { explicitRemoves: [], }); const manifest = loadSkillsManifest(REAL_COMMANDS_DIR); - applySurface(runtimeConfigDir, commandsDir, agentsDir, manifest, CLUSTERS); + const layout = resolveRuntimeArtifactLayout('claude', runtimeConfigDir, 'local'); + applySurface(runtimeConfigDir, layout, manifest, CLUSTERS); const afterStandard = new Set(fs.readdirSync(commandsDir).filter(f => f.endsWith('.md'))); @@ -90,7 +91,7 @@ describe('applySurface', () => { explicitAdds: [], explicitRemoves: [], }); - applySurface(runtimeConfigDir, commandsDir, agentsDir, manifest, CLUSTERS); + applySurface(runtimeConfigDir, layout, manifest, CLUSTERS); const afterCore = new Set(fs.readdirSync(commandsDir).filter(f => f.endsWith('.md'))); @@ -126,7 +127,8 @@ describe('applySurface', () => { explicitRemoves: [], }); const manifest = loadSkillsManifest(REAL_COMMANDS_DIR); - applySurface(runtimeConfigDir, commandsDir, agentsDir, manifest, CLUSTERS); + const layout = resolveRuntimeArtifactLayout('claude', runtimeConfigDir, 'local'); + applySurface(runtimeConfigDir, layout, manifest, CLUSTERS); // Non-gsd file should still be there assert.ok(fs.existsSync(foreignAgent), 'non-gsd agent file should not be touched'); @@ -147,7 +149,8 @@ describe('applySurface', () => { explicitRemoves: [], }); const manifest = loadSkillsManifest(REAL_COMMANDS_DIR); - applySurface(runtimeConfigDir, commandsDir, agentsDir, manifest, CLUSTERS); + const layout = resolveRuntimeArtifactLayout('claude', runtimeConfigDir, 'local'); + applySurface(runtimeConfigDir, layout, manifest, CLUSTERS); // Core skills should now be present assert.ok( @@ -162,4 +165,59 @@ describe('applySurface', () => { fs.rmSync(base, { recursive: true, force: true }); } }); + + test('_syncGsdDir skills kind: adds missing skill dirs, removes stale prefix-matched dirs, preserves foreign dirs', () => { + const { _syncGsdDir } = require('../get-shit-done/bin/lib/surface.cjs'); + const { stageSkillsForRuntimeAsSkills } = require('../get-shit-done/bin/lib/install-profiles.cjs'); + const { findInstallSourceRoot } = require('../get-shit-done/bin/lib/runtime-artifact-layout.cjs'); + // Minimal converter that produces SKILL.md with given stem + function converter(stem, content) { + return [ + '---', + `name: ${stem}`, + '---', + content, + ].join('\n'); + } + + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-surface-skills-')); + try { + const stagedDir = path.join(base, 'staged'); + const destDir = path.join(base, 'dest'); + fs.mkdirSync(destDir, { recursive: true }); + + // Build a staged dir manually: gsd-help/SKILL.md and gsd-update/SKILL.md + const stem1 = 'gsd-help'; + const stem2 = 'gsd-update'; + fs.mkdirSync(path.join(stagedDir, stem1), { recursive: true }); + fs.writeFileSync(path.join(stagedDir, stem1, 'SKILL.md'), '# help\n', 'utf8'); + fs.mkdirSync(path.join(stagedDir, stem2), { recursive: true }); + fs.writeFileSync(path.join(stagedDir, stem2, 'SKILL.md'), '# update\n', 'utf8'); + + // In destDir: stale gsd- dir + foreign user dir + const staleDir = path.join(destDir, 'gsd-old-skill'); + fs.mkdirSync(staleDir, { recursive: true }); + fs.writeFileSync(path.join(staleDir, 'SKILL.md'), '# old\n', 'utf8'); + + const foreignDir = path.join(destDir, 'my-custom-skill'); + fs.mkdirSync(foreignDir, { recursive: true }); + fs.writeFileSync(path.join(foreignDir, 'SKILL.md'), '# custom\n', 'utf8'); + + const skillsKind = { kind: 'skills', destSubpath: 'skills', prefix: 'gsd-', stage: () => stagedDir }; + + _syncGsdDir(stagedDir, destDir, skillsKind); + + // staged dirs copied + assert.ok(fs.existsSync(path.join(destDir, stem1, 'SKILL.md')), 'gsd-help/SKILL.md should be copied'); + assert.ok(fs.existsSync(path.join(destDir, stem2, 'SKILL.md')), 'gsd-update/SKILL.md should be copied'); + + // stale gsd- dir removed + assert.ok(!fs.existsSync(staleDir), 'stale gsd-old-skill dir should be removed'); + + // foreign dir preserved + assert.ok(fs.existsSync(foreignDir), 'my-custom-skill dir should be preserved'); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }); });