fix(3663): preserve Hermes user-skill dirs under skills/gsd/ namespace

When kindPrefix === '' (Hermes: destSubpath=skills/gsd, no per-skill prefix),
startsWith('') always returns true so the prior removal loop would delete any
dir not in the staged set — including user-owned skill dirs.  Guard the entire
removal block behind kindPrefix !== '' so non-staged dirs are never pruned when
there is no prefix to distinguish GSD-owned from user-owned entries.

TDD: failing test added first asserting user-custom-skill is preserved through
a _syncGsdDir call with kindPrefix=''.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-17 00:40:19 -04:00
parent 9476ae863f
commit 8c16b1d338
2 changed files with 50 additions and 8 deletions

View File

@@ -256,14 +256,21 @@ function _syncGsdDir(stagedDir, destDir, kind) {
}
}
// 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 {}
// 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 {}
}
}
}
} else {

View File

@@ -220,4 +220,39 @@ describe('applySurface', () => {
fs.rmSync(base, { recursive: true, force: true });
}
});
test('_syncGsdDir skills kind (hermes): preserves non-GSD user dir under skills/gsd/ when kindPrefix is empty', () => {
const { _syncGsdDir } = require('../get-shit-done/bin/lib/surface.cjs');
const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-surface-hermes-'));
try {
const stagedDir = path.join(base, 'staged');
const destDir = path.join(base, 'dest');
fs.mkdirSync(destDir, { recursive: true });
// Staged contains a GSD skill named 'help' (no prefix under hermes skills/gsd/)
const stem1 = 'help';
fs.mkdirSync(path.join(stagedDir, stem1), { recursive: true });
fs.writeFileSync(path.join(stagedDir, stem1, 'SKILL.md'), '# help\n', 'utf8');
// Dest also has a user-owned custom skill dir (no gsd- prefix — Hermes namespace)
const userDir = path.join(destDir, 'user-custom-skill');
fs.mkdirSync(userDir, { recursive: true });
fs.writeFileSync(path.join(userDir, 'SKILL.md'), '# user custom\n', 'utf8');
// kindPrefix === '' simulates Hermes (destSubpath = skills/gsd, prefix = '')
const hermesKind = { kind: 'skills', destSubpath: 'skills/gsd', prefix: '', stage: () => stagedDir };
_syncGsdDir(stagedDir, destDir, hermesKind);
// The user's custom skill dir must be preserved — it's not in staged but should not be removed
// (Fix 4: when kindPrefix === '', skip the startsWith guard and preserve ALL non-staged dirs)
assert.ok(fs.existsSync(userDir), 'user-custom-skill dir must be preserved when kindPrefix is empty (Hermes)');
// The staged skill must still be copied
assert.ok(fs.existsSync(path.join(destDir, stem1, 'SKILL.md')), 'GSD help/SKILL.md must be copied');
} finally {
fs.rmSync(base, { recursive: true, force: true });
}
});
});