From 043cfd97b3099f4e1e7041b7fbb2e85c662b7a20 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 17 May 2026 01:00:54 -0400 Subject: [PATCH] fix(3663): applySurface creates missing dest dirs (Codex P1-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the fs.existsSync(dest) guard in applySurface so _syncGsdDir is always called. _syncGsdDir already does mkdirSync(..., { recursive: true }) so the destination is created when absent — recovering partially-initialized or user-deleted runtime config dirs. Also threads manifest through to _syncGsdDir as optional 4th arg (used by P1-2 Hermes fix). Co-Authored-By: Claude Sonnet 4.6 --- get-shit-done/bin/lib/surface.cjs | 54 ++++++++++++++++++++----------- tests/surface-apply.test.cjs | 25 ++++++++++++++ 2 files changed, 60 insertions(+), 19 deletions(-) diff --git a/get-shit-done/bin/lib/surface.cjs b/get-shit-done/bin/lib/surface.cjs index 08e88d9f6..af6ee17b3 100644 --- a/get-shit-done/bin/lib/surface.cjs +++ b/get-shit-done/bin/lib/surface.cjs @@ -208,9 +208,7 @@ function applySurface(runtimeConfigDir, layout, manifest, clusterMap) { 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); - } + _syncGsdDir(staged, dest, kind, manifest); } return resolved; } @@ -224,11 +222,16 @@ function applySurface(runtimeConfigDir, layout, manifest, clusterMap) { * by copying recursively; remove dirs not in staged set. Preserves dirs not matching * the prefix (user-owned skills). * + * For Hermes (empty prefix): uses manifest membership to discriminate GSD-owned vs + * user-owned dirs. GSD-owned = stem in manifest; removal targets = in manifest AND + * not in staged set. User-owned (not in manifest) are always preserved. + * * @param {string} stagedDir source (staged temp dir or original) * @param {string} destDir runtime destination * @param {import('./runtime-artifact-layout.cjs').ArtifactKind|'commands'|'agents'} kind + * @param {Map} [manifest] optional; required for Hermes empty-prefix removal */ -function _syncGsdDir(stagedDir, destDir, kind) { +function _syncGsdDir(stagedDir, destDir, kind, manifest) { if (!fs.existsSync(stagedDir)) return; fs.mkdirSync(destDir, { recursive: true }); @@ -256,22 +259,35 @@ function _syncGsdDir(stagedDir, destDir, kind) { } } - // Empty prefix = destSubpath is the GSD namespace (Hermes: skills/gsd/). - // With no prefix filter, we cannot safely distinguish GSD-owned from user-owned dirs, - // so we only remove dirs that match the prefix. When kindPrefix === '', - // startsWith('') is always true but we must guard: skip removal entirely if prefix is - // empty so user dirs under skills/gsd/ are preserved (Hermes user-skill safety). - if (kindPrefix !== '') { - // Remove prefix-matched dirs in dest that 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; // preserve user-owned dirs - if (!stagedDirs.has(entry)) { - try { fs.rmSync(entryPath, { recursive: true, force: true }); } catch {} - } + // Removal: discriminator depends on prefix shape. + // Non-empty prefix: GSD namespace IS the prefix; remove prefix-matching dirs not in staged set. + // Empty prefix (Hermes): GSD-owned = stem in manifest (i.e. canonically-shipped GSD skill). + // User-owned skills not in manifest are preserved. + // No manifest available: be conservative, don't remove anything. + const canonicalStems = manifest + ? new Set([...manifest.keys()].filter(k => !k.startsWith('_calls_agents_'))) + : null; + + const destEntries = fs.readdirSync(destDir); + for (const entry of destEntries) { + const entryPath = path.join(destDir, entry); + if (!fs.statSync(entryPath).isDirectory()) continue; + + let isGsdOwned; + if (kindPrefix !== '') { + isGsdOwned = entry.startsWith(kindPrefix); + } else if (canonicalStems) { + // Hermes: empty prefix, destSubpath is the namespace. + // GSD-owned iff the directory name (stem) appears in the canonical manifest. + isGsdOwned = canonicalStems.has(entry); + } else { + // No manifest available: be conservative, don't remove anything. + continue; } + + if (!isGsdOwned) continue; // preserve user-owned + if (stagedDirs.has(entry)) continue; // current GSD-owned, keep + try { fs.rmSync(entryPath, { recursive: true, force: true }); } catch {} } } else { // commands / agents kind: work with .md files diff --git a/tests/surface-apply.test.cjs b/tests/surface-apply.test.cjs index dd1f07629..7cd36143b 100644 --- a/tests/surface-apply.test.cjs +++ b/tests/surface-apply.test.cjs @@ -202,6 +202,31 @@ describe('applySurface', () => { assert.ok(fs.existsSync(foreignDir), 'my-custom-skill dir should be preserved'); }); + test('applySurface recreates missing destination directories', (t) => { + // Fixture: layout configDir exists but the dest subdirectory for the kinds does NOT. + const base = createTempDir('gsd-surface-missing-dest-'); + t.after(() => cleanup(base)); + const runtimeConfigDir = base; + // Do NOT pre-create commands/gsd or agents — they are intentionally absent. + writeActiveProfile(runtimeConfigDir, 'core'); + writeSurface(runtimeConfigDir, { + baseProfile: 'core', + disabledClusters: [], + explicitAdds: [], + explicitRemoves: [], + }); + const manifest = loadSkillsManifest(REAL_COMMANDS_DIR); + const layout = resolveRuntimeArtifactLayout('claude', runtimeConfigDir, 'local'); + applySurface(runtimeConfigDir, layout, manifest, CLUSTERS); + + // commands/gsd must have been created and populated + const commandsDir = path.join(runtimeConfigDir, 'commands', 'gsd'); + assert.ok(fs.existsSync(commandsDir), 'commands/gsd dir should be created even if initially absent'); + const files = fs.readdirSync(commandsDir).filter(f => f.endsWith('.md')); + assert.ok(files.length > 0, 'commands/gsd should contain staged skill files'); + assert.ok(files.includes('help.md'), 'help.md should be present after applySurface on missing dest'); + }); + test('_syncGsdDir skills kind (hermes): preserves non-GSD user dir under skills/gsd/ when kindPrefix is empty', (t) => { const { _syncGsdDir } = require('../get-shit-done/bin/lib/surface.cjs');