diff --git a/.changeset/tidy-finches-jump.md b/.changeset/tidy-finches-jump.md new file mode 100644 index 000000000..531176f16 --- /dev/null +++ b/.changeset/tidy-finches-jump.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 822 +--- +**`/gsd:surface` no longer mis-names or orphans runtime command files** — re-surfacing now writes the same `gsd-`-prefixed command filenames as a fresh install for flat command dirs (Cursor, Augment, OpenCode, Kilo) and preserves user-authored command files instead of deleting them. diff --git a/src/surface.cts b/src/surface.cts index bd8cccc94..b3cf05bb5 100644 --- a/src/surface.cts +++ b/src/surface.cts @@ -410,23 +410,41 @@ function _syncGsdDir(stagedDir: string, destDir: string, kind: ArtifactKind | st // pruneSkillDirs() is the single point of truth for this logic. pruneSkillDirs(destDir, stagedDirs, kindPrefix, manifest); } else { - // commands / agents kind: work with .md files - const stagedFiles = new Set( - fs.readdirSync(stagedDir).filter(f => f.endsWith('.md')) - ); + // commands / agents kind: mirror installRuntimeArtifacts (_copyStaged / + // _removeGsdEntries in bin/install.js) so surface produces the SAME files as a + // fresh install (#816). Flat command dirs (opencode/cursor/augment/kilo) take + // the gsd- prefix on copy; namespaced command dirs (commands/gsd) and agents + // keep their staged names. Copying staged names verbatim previously diverged + // from install and orphaned the installed gsd-*.md files, and the unscoped + // prune deleted user-owned command files. + // + // NOTE: the destName rule below intentionally mirrors bin/install.js + // `_copyStaged` (the `namespacedByDir` decision). Keep them in sync. + const destLast = (typeof kind === 'object' && kind !== null && kind.destSubpath) + ? path.basename(kind.destSubpath) + : ''; + const prefixStem = kindPrefix ? kindPrefix.replace(/-$/, '') : ''; + const namespacedByDir = kindName === 'commands' && destLast === prefixStem; - // Copy files from staged to dest (overwrite to keep content current) + const stagedFiles = fs.readdirSync(stagedDir).filter(f => f.endsWith('.md')); + const stagedDestNames = new Set(); for (const file of stagedFiles) { - fs.copyFileSync(path.join(stagedDir, file), path.join(destDir, file)); + const destName = (kindName === 'agents' || namespacedByDir) + ? file + : `${kindPrefix}${file.slice(0, -3)}.md`; + fs.copyFileSync(path.join(stagedDir, file), path.join(destDir, destName)); + stagedDestNames.add(destName); } - // 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) { + // Prune stale GSD-owned files not in the staged set, preserving user-owned files + // (mirrors install's prefix-scoped _removeGsdEntries): + // - agents: only gsd-* are GSD-owned + // - flat command dirs: only `${kindPrefix}`-prefixed are GSD-owned + // - namespaced command dirs: the whole dir is GSD-owned + for (const file of fs.readdirSync(destDir).filter(f => f.endsWith('.md'))) { if (kindName === 'agents' && !file.startsWith('gsd-')) continue; - if (!stagedFiles.has(file)) { + if (kindName === 'commands' && !namespacedByDir && kindPrefix && !file.startsWith(kindPrefix)) continue; + if (!stagedDestNames.has(file)) { try { fs.unlinkSync(path.join(destDir, file)); } catch { /* ignore */ } } } diff --git a/tests/runtime-artifact-layout-surface.test.cjs b/tests/runtime-artifact-layout-surface.test.cjs index 58aebdd12..b48298c05 100644 --- a/tests/runtime-artifact-layout-surface.test.cjs +++ b/tests/runtime-artifact-layout-surface.test.cjs @@ -303,6 +303,144 @@ describe('applySurface', () => { assert.ok(fs.existsSync(path.join(destDir, stem1, 'SKILL.md')), 'GSD help/SKILL.md must be copied'); }); + // Regression guard for #816: applySurface must write gsd-prefixed command files + // (matching installRuntimeArtifacts/_copyStaged behaviour) and must NOT prune + // user-created command files that install would preserve. + // + // Affected runtimes have a FLAT command dir (opencode `command/`, cursor + // `commands/`, augment `commands/`, kilo `command/`) with kind.prefix='gsd-'. + // _copyStaged names files `gsd-.md` but the buggy _syncGsdDir copies + // them as `.md` (unprefixed) and also deletes ALL .md files not in the + // staged set, including user files. + test('applySurface writes gsd-prefixed command files matching install and preserves user commands (#816)', (t) => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-surface-816-')); + t.after(() => cleanup(configDir)); + + writeActiveProfile(configDir, 'standard'); + writeSurface(configDir, { + baseProfile: 'standard', + disabledClusters: [], + explicitAdds: [], + explicitRemoves: [], + }); + + // Determine the command dest dir for opencode: commandsKind destSubpath='command' + const layout = resolveRuntimeArtifactLayout('opencode', configDir, 'global'); + const commandsKind = layout.kinds.find(k => k.kind === 'commands'); + assert.ok(commandsKind, 'opencode layout must have a commands kind'); + const commandDir = path.join(configDir, commandsKind.destSubpath); + + // Pre-seed a user command file BEFORE applySurface — install would preserve it + fs.mkdirSync(commandDir, { recursive: true }); + fs.writeFileSync(path.join(commandDir, 'my-user-cmd.md'), '# user custom command\n', 'utf8'); + + const manifest = loadSkillsManifest(REAL_COMMANDS_DIR); + applySurface(configDir, layout, manifest, CLUSTERS); + + const files = fs.readdirSync(commandDir).filter(f => f.endsWith('.md')); + + // (a) At least one gsd-prefixed command file must exist — on buggy code only + // unprefixed files like 'help.md' are written, so this assertion fails. + assert.ok( + files.some(f => f.startsWith('gsd-') && f.endsWith('.md')), + '#816: applySurface must write at least one gsd-prefixed command file ' + + '(e.g. gsd-help.md) to match installRuntimeArtifacts/_copyStaged behaviour. ' + + `Actual files: [${files.join(', ')}]` + ); + + // (b) Every GSD-owned command file must be prefixed — no bare .md files + // allowed among GSD-owned output (excluding the user file). + const gsdFiles = files.filter(f => f !== 'my-user-cmd.md'); + const unprefixed = gsdFiles.filter(f => !f.startsWith('gsd-')); + assert.deepStrictEqual( + unprefixed, + [], + '#816: all GSD-owned command files must start with gsd- to match install. ' + + `Found unprefixed: [${unprefixed.join(', ')}]` + ); + + // (c) The pre-seeded user file must survive applySurface — on buggy code the + // commands-kind pruning loop deletes ALL .md files not in the staged set, + // which wipes user files that installRuntimeArtifacts would never touch. + assert.ok( + files.includes('my-user-cmd.md'), + '#816: applySurface must preserve user command file my-user-cmd.md that was ' + + 'present before sync — _syncGsdDir must not prune files not owned by GSD. ' + + `Actual files: [${files.join(', ')}]` + ); + }); + + // Parity regression guard for #816: applySurface command-dir filenames must + // match a fresh installRuntimeArtifacts for every command runtime. Guards + // against future drift between _syncGsdDir (surface) and _copyStaged (install) + // command-naming logic. + // + // Matrix: opencode/kilo = flat command/ + prefix gsd-; + // cursor/augment = flat commands/ + prefix gsd-; + // gemini = namespaced commands/gsd/ (no file prefix — dir is namespace). + // For each runtime we: run install into installDir, run applySurface into + // surfaceDir (same 'standard' profile both sides), then compare sorted .md + // filename sets in the commands dest dir. On a fresh dir (no superseded files) + // both paths must produce identical sets. + test('applySurface command-dir filenames match a fresh install for every command runtime (#816 parity)', async (t) => { + process.env.GSD_TEST_MODE = '1'; + const { installRuntimeArtifacts } = require('../bin/install.js'); + + const manifest = loadSkillsManifest(REAL_COMMANDS_DIR); + // Build the resolved profile once. Both install and surface sides must use + // the same skill set so any filename difference is purely a naming bug. + const resolvedProfile = resolveProfile({ modes: ['standard'], manifest }); + + const PARITY_RUNTIMES = ['opencode', 'cursor', 'augment', 'kilo', 'gemini']; + + for (const runtime of PARITY_RUNTIMES) { + // Create two independent temp dirs — one for install, one for surface. + const installDir = fs.mkdtempSync(path.join(os.tmpdir(), `gsd-816-install-${runtime}-`)); + const surfaceDir = fs.mkdtempSync(path.join(os.tmpdir(), `gsd-816-surface-${runtime}-`)); + t.after(() => { cleanup(installDir); cleanup(surfaceDir); }); + + // --- Install path --- + installRuntimeArtifacts(runtime, installDir, 'global', resolvedProfile); + + // --- Surface path --- + writeActiveProfile(surfaceDir, 'standard'); + writeSurface(surfaceDir, { + baseProfile: 'standard', + disabledClusters: [], + explicitAdds: [], + explicitRemoves: [], + }); + const layout = resolveRuntimeArtifactLayout(runtime, surfaceDir, 'global'); + applySurface(surfaceDir, layout, manifest, CLUSTERS); + + // --- Find commands kind --- + const cmdKind = layout.kinds.find(k => k.kind === 'commands'); + if (!cmdKind) { + // Runtime has no commands kind at global scope — skip gracefully. + continue; + } + + // --- Compare sorted .md filename sets --- + const installCmdDir = path.join(installDir, cmdKind.destSubpath); + const surfaceCmdDir = path.join(surfaceDir, cmdKind.destSubpath); + + const installFiles = fs.existsSync(installCmdDir) + ? fs.readdirSync(installCmdDir).filter(f => f.endsWith('.md')).sort() + : []; + const surfaceFiles = fs.existsSync(surfaceCmdDir) + ? fs.readdirSync(surfaceCmdDir).filter(f => f.endsWith('.md')).sort() + : []; + + assert.deepStrictEqual( + surfaceFiles, + installFiles, + `#816 parity: command filenames for ${runtime} (${cmdKind.destSubpath}) must match a fresh install.\n` + + ` install: [${installFiles.slice(0, 5).join(', ')}${installFiles.length > 5 ? '...' : ''}]\n` + + ` surface: [${surfaceFiles.slice(0, 5).join(', ')}${surfaceFiles.length > 5 ? '...' : ''}]` + ); + } + }); + // Regression test for #813: applySurface must apply per-runtime path rewrites // (applyRuntimeContentRewritesInPlace) just as installRuntimeArtifacts does. // Without the fix, skill bodies retain the converter's default ~/.claude/ paths