diff --git a/.changeset/witty-cats-wander.md b/.changeset/witty-cats-wander.md new file mode 100644 index 000000000..d86131a16 --- /dev/null +++ b/.changeset/witty-cats-wander.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 955 +--- +Hermes skills now install at skills/gsd/gsd-/SKILL.md with name gsd-, restoring canonical /gsd- dispatch that was broken by the bare-stem prefix introduced in #3664. diff --git a/bin/install.js b/bin/install.js index c76bae7e5..c26731ea8 100755 --- a/bin/install.js +++ b/bin/install.js @@ -7753,8 +7753,9 @@ function _copyStaged(stagedDir, destDir, kind) { /** * Remove GSD-prefixed entries from destDir matching kind.prefix. - * For Hermes nested case (prefix === ''): the destSubpath IS the namespace - * (skills/gsd) — remove the entire destDir. + * For the prefix='' case: the destSubpath IS the namespace — remove the entire + * destDir. (No current runtime uses prefix='' after #947 reversed Hermes; kept + * as a defensive guard for future runtimes.) */ function _removeGsdEntries(destDir, kind) { if (!fs.existsSync(destDir)) return; @@ -7826,20 +7827,11 @@ function _runLegacyInstallMigrations(runtime, configDir, scope = 'global') { } } - // Hermes: remove intermediate-layout skills/gsd/gsd-*/ entries that existed - // between #2841 and #3664. Phase 2 (#3664) uses prefix='' producing bare-stem - // names (skills/gsd//SKILL.md); the intermediate layout had the gsd- - // prefix inside the nested dir (skills/gsd/gsd-/SKILL.md). Only - // children whose name starts with gsd- are removed — the parent skills/gsd/ - // directory and any non-gsd- siblings (user content) are preserved. - const nestedGsdDir = path.join(configDir, 'skills', 'gsd'); - if (fs.existsSync(nestedGsdDir)) { - for (const entry of fs.readdirSync(nestedGsdDir, { withFileTypes: true })) { - if (entry.isDirectory() && entry.name.startsWith('gsd-')) { - fs.rmSync(path.join(nestedGsdDir, entry.name), { recursive: true }); - } - } - } + // Hermes: bare-stem skills/gsd// cleanup is deferred to AFTER the + // layout-driven install loop in installRuntimeArtifacts, where the exact set + // of staged gsd-/ dirs is known. Removing here (before staging) would + // require readGsdCommandNames() which misses skills like 'dev-preferences' + // that are not in the commands directory. See _removeHermesBareStemDirs(). } // Migrate dev-preferences.md content → runtime-aware SKILL.md location (#2973). @@ -7896,6 +7888,18 @@ function _runLegacyUninstallCleanup(runtime, configDir, scope = 'global') { } } } + + // Hermes: pre-#947 bare-stem skills/gsd// entries (dirs that do NOT + // start with 'gsd-') — the #3664 layout used prefix='' so GSD-owned skills + // had bare names (e.g. skills/gsd/help/). These are stale on uninstall. + const nestedGsdDirForUninstall = path.join(configDir, 'skills', 'gsd'); + if (fs.existsSync(nestedGsdDirForUninstall)) { + for (const entry of fs.readdirSync(nestedGsdDirForUninstall, { withFileTypes: true })) { + if (entry.isDirectory() && !entry.name.startsWith('gsd-')) { + fs.rmSync(path.join(nestedGsdDirForUninstall, entry.name), { recursive: true }); + } + } + } } // Return saved artifacts so the caller can migrate after layout-driven removal. @@ -7946,6 +7950,43 @@ function _restoreDir(dir, snapshot) { } } +/** + * After the layout-driven install loop writes new gsd-/ dirs to + * skills/gsd/, remove any pre-existing bare-stem dirs (skills/gsd//) + * that correspond to the newly installed gsd- entries. + * + * The removal set is derived from the ACTUAL installed skill dirs (every + * entry starting with 'gsd-' that is a directory), so it covers ALL shipped + * GSD skills — including 'dev-preferences' and future additions — without + * relying on readGsdCommandNames() which only enumerates the commands source + * tree and can miss skills that ship outside that directory. + * + * Safety: a bare dir is ONLY removed when a corresponding gsd-/ dir was + * installed this run. A user-owned dir 'skills/gsd/my-workflow/' that has no + * matching 'skills/gsd/gsd-my-workflow/' is never touched. + * + * @param {string} nestedGsdDir absolute path to skills/gsd/ category dir + */ +function _removeHermesBareStemDirs(nestedGsdDir) { + if (!fs.existsSync(nestedGsdDir)) return; + const entries = fs.readdirSync(nestedGsdDir, { withFileTypes: true }); + + // Collect the set of stems that were installed as gsd-/ this run. + const installedStems = new Set(); + for (const entry of entries) { + if (entry.isDirectory() && entry.name.startsWith('gsd-')) { + installedStems.add(entry.name.slice('gsd-'.length)); // e.g. 'quick', 'dev-preferences' + } + } + + // Remove any bare / dir for which gsd-/ was just installed. + for (const entry of entries) { + if (entry.isDirectory() && !entry.name.startsWith('gsd-') && installedStems.has(entry.name)) { + fs.rmSync(path.join(nestedGsdDir, entry.name), { recursive: true }); + } + } +} + function installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile) { // Legacy cleanup before layout-driven writes _runLegacyInstallMigrations(runtime, configDir, scope); @@ -7986,29 +8027,15 @@ function installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile) { // then restore after. This preserves user dirs across a wipe-and-replace // install (#2973 / #3664). // - // For prefix='' (Hermes): _removeGsdEntries wipes the entire dest dir (skills/gsd/). - // Preserve every subdir that is NOT in the staged set — those are user-added dirs - // (e.g. user-content/) that GSD does not manage. - // - // For prefix='gsd-' (others): _removeGsdEntries removes only gsd-* entries. - // Non-gsd-* user dirs (e.g. my-custom-skill/) are untouched. Only preserve the - // explicit user-owned GSD-prefixed skill gsd-dev-preferences, which GSD does not - // reinstall from source but must survive the prune (#2973). + // All runtimes (incl. Hermes after #947) use prefix='gsd-'. + // _removeGsdEntries removes only gsd-* entries; non-gsd-* user dirs are + // untouched. Preserve the explicit user-owned GSD-prefixed skill + // gsd-dev-preferences, which GSD does not reinstall from source but must + // survive the prune (#2973). const toPreserve = new Map(); // dirName -> Map - if (kind.prefix === '') { - // Hermes: wipes entire dest dir — preserve anything not in staged. - const stagedNames = fs.existsSync(stagedForCopy) - ? new Set(fs.readdirSync(stagedForCopy, { withFileTypes: true }) - .filter(e => e.isDirectory()).map(e => e.name)) - : new Set(); - for (const entry of fs.readdirSync(dest, { withFileTypes: true })) { - if (!entry.isDirectory() || stagedNames.has(entry.name)) continue; - const snap = _snapshotDir(path.join(dest, entry.name)); - if (snap.size > 0) toPreserve.set(entry.name, snap); - } - } else { - // Non-Hermes: only preserve explicitly user-owned GSD-prefixed skill dirs. + { + // Preserve explicitly user-owned GSD-prefixed skill dirs. // gsd-dev-preferences is the sole user-customisable skill in this category. const USER_OWNED_SKILL_DIRS = ['gsd-dev-preferences']; for (const dirName of USER_OWNED_SKILL_DIRS) { @@ -8038,6 +8065,21 @@ function installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile) { } } } + + // Hermes: after the install loop has written all gsd-/ dirs to + // skills/gsd/, remove any stale bare-stem dirs (skills/gsd//) that + // correspond to the newly installed gsd- entries. This is the robust + // replacement for the readGsdCommandNames()-based pre-install cleanup that + // missed skills like 'dev-preferences' (#947 adversarial review). + // + // We run this AFTER the install loop so the installed set is authoritative: + // every gsd-/ present now was written this run (or was there before + // with the same prefix). User-owned bare dirs with no gsd- counterpart + // are untouched. + if (runtime === 'hermes') { + const nestedGsdDirForCleanup = path.join(configDir, 'skills', 'gsd'); + _removeHermesBareStemDirs(nestedGsdDirForCleanup); + } } /** @@ -8143,6 +8185,23 @@ function uninstallRuntimeArtifacts(runtime, configDir, scope) { _removeGsdEntries(dest, kind); } + // Hermes: after removing gsd-* skill dirs from skills/gsd/, also remove + // the GSD-managed DESCRIPTION.md and then the category dir itself if it + // contains no user content (#947). _removeGsdEntries removed gsd-* dirs + // but left the category container and DESCRIPTION.md intact. + if (runtime === 'hermes') { + const nestedGsdDir = path.join(configDir, 'skills', 'gsd'); + if (fs.existsSync(nestedGsdDir)) { + // Remove GSD-owned DESCRIPTION.md (written by writeHermesCategoryDescription) + fs.rmSync(path.join(nestedGsdDir, 'DESCRIPTION.md'), { force: true }); + // Remove the category dir if empty (no user content remaining) + const remaining = fs.readdirSync(nestedGsdDir, { withFileTypes: true }); + if (remaining.length === 0) { + fs.rmSync(nestedGsdDir, { recursive: true, force: true }); + } + } + } + // #2973 / Codex review (bd1f06c9): migrate dev-preferences.md to the // runtime-aware SKILL.md location after all layout-driven removal is // complete. Do NOT restore to commands/gsd/ — the user is uninstalling. @@ -9568,8 +9627,8 @@ function writeManifest(configDir, runtime = 'claude', options = {}) { } } if ((isCodex || isCopilot || isAntigravity || isCursor || isWindsurf || isTrae || (!isOpencode && !isGemini)) && fs.existsSync(codexSkillsDir)) { - // Hermes uses prefix '' (bare stem names); all others use 'gsd-' - const skillListPrefix = isHermes ? '' : 'gsd-'; + // All runtimes (including Hermes post-#947) use the canonical 'gsd-' prefix. + const skillListPrefix = 'gsd-'; for (const skillName of listCodexSkillNames(codexSkillsDir, skillListPrefix)) { const skillRoot = path.join(codexSkillsDir, skillName); const skillHashes = generateManifest(skillRoot); @@ -10475,9 +10534,9 @@ function install(isGlobal, runtime = 'claude', options = {}) { if (isHermes) { const hermesSkillsDir = path.join(targetDir, 'skills', 'gsd'); if (fs.existsSync(hermesSkillsDir)) { - // Hermes layout uses prefix: '' — skill dirs have bare stem names (no gsd- prefix) + // Hermes layout uses prefix: 'gsd-' (#947) — skill dirs have gsd- names const count = fs.readdirSync(hermesSkillsDir, { withFileTypes: true }) - .filter(e => e.isDirectory()).length; + .filter(e => e.isDirectory() && e.name.startsWith('gsd-')).length; if (count > 0) { console.log(` ${green}✓${reset} Installed ${count} skills to skills/gsd/`); } else { diff --git a/docs/adr/3660-runtime-artifact-layout-module.md b/docs/adr/3660-runtime-artifact-layout-module.md index b77960d49..fa7deab35 100644 --- a/docs/adr/3660-runtime-artifact-layout-module.md +++ b/docs/adr/3660-runtime-artifact-layout-module.md @@ -17,7 +17,7 @@ The root problem is the absence of a typed seam for "where does runtime R put ar - Each `ArtifactKind` is `{ kind: 'commands'|'agents'|'skills', destSubpath, prefix, stage }`. `stage` is a function `(resolvedProfile) → stagedDir` that closes over the per-runtime converter where one is needed (e.g. `convertClaudeCommandToClaudeSkill` for the `skills` kind on Claude global). - The `kinds` array is empty for runtimes with no GSD surface (a hypothetical future runtime with no integration). The `skills` kind is **absent** for runtimes that don't materialize skill directories (Cline; Gemini today). The `commands` kind is **absent** for runtimes that consume only the skills/agents layout (Claude global, Codex, etc.). - Per-runtime quirks live in the layout's record fields, not in caller branches: - - **Hermes**: `{ kind: 'skills', destSubpath: 'skills/gsd', prefix: '' }` — preserves the nested namespace from #2841. + - **Hermes**: `{ kind: 'skills', destSubpath: 'skills/gsd', prefix: 'gsd-' }` — preserves the nested namespace from #2841. **Note (#947):** The original decision used `prefix: ''` (bare stem) on the incorrect premise that the `skills/gsd/` category directory namespaced the leaf identifier in Hermes's loader. Research showed category dirs are purely organisational; dispatch is by the skill `name:` field. The `gsd-` prefix was restored by #947 to match every other runtime. - **Cline**: `kinds: []` — Cline resolves to zero kinds in Phase 1 (no `commands` kind). - **Gemini**: `kinds: [ { kind: 'commands', destSubpath: 'commands/gsd', prefix: 'gsd-' } ]` — no agents, no skills. - `applySurface` migrates from `(runtimeConfigDir, commandsDir, agentsDir, manifest, clusterMap)` to `(runtimeConfigDir, layout, manifest, clusterMap)`. Body collapses to `for (const kind of layout.kinds) _syncGsdDir(kind.stage(resolved), path.join(layout.configDir, kind.destSubpath), kind.kind)`. @@ -73,7 +73,7 @@ Phase 1 should **not**: * @typedef {Object} ArtifactKind * @property {'commands'|'agents'|'skills'} kind * @property {string} destSubpath joined to layout.configDir - * @property {string} prefix 'gsd-' or '' (Hermes nested case) + * @property {string} prefix 'gsd-' for all runtimes (incl. Hermes after #947) * @property {(resolved) => string} stage returns staged dir path */ diff --git a/src/runtime-artifact-layout.cts b/src/runtime-artifact-layout.cts index 3249f6065..e05f75868 100644 --- a/src/runtime-artifact-layout.cts +++ b/src/runtime-artifact-layout.cts @@ -426,7 +426,11 @@ function resolveRuntimeArtifactLayout(runtime: string, configDir: string, scope: break; case 'hermes': - kinds = [skillsKind('skills/gsd', '', 'convertClaudeCommandToClaudeSkill', 'hermes', configDir, true /* #69 nested */)]; + // #947: restore canonical gsd- prefix — skills land at skills/gsd/gsd-/SKILL.md + // and dispatch as /gsd-, consistent with every other runtime. + // The skills/gsd/ category bucket (introduced by #2841) is retained. + // Prior bare-stem layout (prefix='') used by #3664 is reversed here. + kinds = [skillsKind('skills/gsd', 'gsd-', 'convertClaudeCommandToClaudeSkill', 'hermes', configDir, true /* #69 nested */)]; break; case 'codebuddy': diff --git a/tests/bug-947-hermes-gsd-prefix.test.cjs b/tests/bug-947-hermes-gsd-prefix.test.cjs new file mode 100644 index 000000000..e31c85aae --- /dev/null +++ b/tests/bug-947-hermes-gsd-prefix.test.cjs @@ -0,0 +1,454 @@ +// allow-test-rule: source-text-is-the-product +// Reads installed .md product artefacts from a real install run — +// testing their on-disk layout + frontmatter tests the deployed contract. + +/** + * Regression test: #947 — Hermes skills must install with canonical gsd- prefix. + * + * Prior to this fix, Hermes installed skills at skills/gsd//SKILL.md + * with frontmatter `name: ` (e.g. name: quick), causing invocation as + * /quick instead of /gsd-quick. This file asserts the corrected behaviour: + * - Fresh install → skills/gsd/gsd-/SKILL.md, name: gsd- + * - The skills/gsd/ category bucket and its DESCRIPTION.md are retained + * - Migration: prior bare-stem dirs (skills/gsd//) are removed + * on reinstall; no orphaned bare-stem directories remain. + * + * Runtime: node:test, node:assert/strict. No Jest. + */ + +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); + +const { installRuntimeArtifacts } = require('../bin/install.js'); +const { parseFrontmatter, cleanup } = require('./helpers.cjs'); +const { + loadSkillsManifest, + resolveProfile, +} = require('../gsd-core/bin/lib/install-profiles.cjs'); + +// --------------------------------------------------------------------------- +// Shared fixture: a minimal commands/gsd/ source with two skills +// --------------------------------------------------------------------------- + +/** + * Write a minimal commands/gsd/ source tree with the given stem names. + * Returns the path to the commands/gsd directory (used as .gsd-source value). + */ +function writeMinimalSourceTree(baseDir, stems) { + const srcDir = path.join(baseDir, 'src', 'commands', 'gsd'); + fs.mkdirSync(srcDir, { recursive: true }); + for (const stem of stems) { + fs.writeFileSync(path.join(srcDir, `${stem}.md`), [ + '---', + `name: gsd:${stem}`, + `description: ${stem} task description`, + 'allowed-tools:', + ' - Read', + ' - Bash', + '---', + '', + `${stem} body`, + ].join('\n')); + } + return srcDir; +} + +const MANIFEST = loadSkillsManifest(); +const RESOLVED_FULL = resolveProfile({ modes: [], manifest: MANIFEST }); + +// --------------------------------------------------------------------------- +// #947 regression: fresh install produces prefixed layout +// --------------------------------------------------------------------------- + +describe('#947 Hermes: fresh install → gsd- prefixed layout', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-947-fresh-')); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('skill lands at skills/gsd/gsd-/SKILL.md (NOT skills/gsd//SKILL.md)', () => { + const srcDir = writeMinimalSourceTree(tmpDir, ['quick']); + const configDir = path.join(tmpDir, 'dest'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, '.gsd-source'), srcDir); + + installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_FULL); + + // Correct (post-fix) path: skills/gsd/gsd-quick/SKILL.md + const correctPath = path.join(configDir, 'skills', 'gsd', 'gsd-quick', 'SKILL.md'); + assert.ok(fs.existsSync(correctPath), + 'skills/gsd/gsd-quick/SKILL.md must exist (canonical gsd- prefix)'); + + // Old (bare-stem) path must NOT exist + const bareStemPath = path.join(configDir, 'skills', 'gsd', 'quick', 'SKILL.md'); + assert.ok(!fs.existsSync(bareStemPath), + 'skills/gsd/quick/SKILL.md must NOT exist (bare-stem path is wrong)'); + }); + + test('SKILL.md frontmatter name is gsd- (NOT bare )', () => { + const srcDir = writeMinimalSourceTree(tmpDir, ['plan']); + const configDir = path.join(tmpDir, 'dest'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, '.gsd-source'), srcDir); + + installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_FULL); + + const skillPath = path.join(configDir, 'skills', 'gsd', 'gsd-plan', 'SKILL.md'); + assert.ok(fs.existsSync(skillPath), 'skills/gsd/gsd-plan/SKILL.md must exist'); + + const content = fs.readFileSync(skillPath, 'utf8'); + const fm = parseFrontmatter(content); + assert.strictEqual(fm.name, 'gsd-plan', + `frontmatter name must be 'gsd-plan', got '${fm.name}'`); + }); + + test('gsd- identifier satisfies Hermes name rule ^[a-z][a-z0-9_-]*$', () => { + const srcDir = writeMinimalSourceTree(tmpDir, ['plan-phase', 'code-review']); + const configDir = path.join(tmpDir, 'dest'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, '.gsd-source'), srcDir); + + installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_FULL); + + const HERMES_NAME_RE = /^[a-z][a-z0-9_-]*$/; + for (const stem of ['plan-phase', 'code-review']) { + const skillPath = path.join(configDir, 'skills', 'gsd', `gsd-${stem}`, 'SKILL.md'); + assert.ok(fs.existsSync(skillPath), `skills/gsd/gsd-${stem}/SKILL.md must exist`); + const content = fs.readFileSync(skillPath, 'utf8'); + const fm = parseFrontmatter(content); + assert.ok(HERMES_NAME_RE.test(fm.name), + `name '${fm.name}' must satisfy Hermes identifier rule ${HERMES_NAME_RE}`); + assert.strictEqual(fm.name, `gsd-${stem}`, + `name must be 'gsd-${stem}', got '${fm.name}'`); + } + }); + + test('skills/gsd/ category bucket is retained (not flattened to top-level skills/)', () => { + const srcDir = writeMinimalSourceTree(tmpDir, ['quick']); + const configDir = path.join(tmpDir, 'dest'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, '.gsd-source'), srcDir); + + installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_FULL); + + // Skill must be INSIDE skills/gsd/ — not at skills/gsd-quick/ directly + const categoryBucket = path.join(configDir, 'skills', 'gsd'); + assert.ok(fs.existsSync(categoryBucket), + 'skills/gsd/ category directory must be retained'); + + // Flat (non-categorised) path must NOT exist + const flatPath = path.join(configDir, 'skills', 'gsd-quick'); + assert.ok(!fs.existsSync(flatPath), + 'skills/gsd-quick/ (flat, non-categorised) must NOT exist for Hermes'); + }); + + test('skills/gsd/ category directory exists (bucket retained after install)', () => { + // Note: DESCRIPTION.md is written by writeHermesCategoryDescription which is + // called from the top-level installGsd flow (not inside installRuntimeArtifacts). + // This test confirms the category bucket itself is present post-install. + const srcDir = writeMinimalSourceTree(tmpDir, ['quick']); + const configDir = path.join(tmpDir, 'dest'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, '.gsd-source'), srcDir); + + installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_FULL); + + const categoryBucket = path.join(configDir, 'skills', 'gsd'); + assert.ok(fs.existsSync(categoryBucket), + 'skills/gsd/ category directory must exist after Hermes install'); + assert.ok(fs.statSync(categoryBucket).isDirectory(), + 'skills/gsd/ must be a directory, not a file'); + }); + + test('multiple skills all get gsd- prefix', () => { + const srcDir = writeMinimalSourceTree(tmpDir, ['quick', 'plan', 'review']); + const configDir = path.join(tmpDir, 'dest'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, '.gsd-source'), srcDir); + + installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_FULL); + + for (const stem of ['quick', 'plan', 'review']) { + const correctPath = path.join(configDir, 'skills', 'gsd', `gsd-${stem}`, 'SKILL.md'); + assert.ok(fs.existsSync(correctPath), + `skills/gsd/gsd-${stem}/SKILL.md must exist`); + const bareStem = path.join(configDir, 'skills', 'gsd', stem, 'SKILL.md'); + assert.ok(!fs.existsSync(bareStem), + `bare-stem path skills/gsd/${stem}/SKILL.md must NOT exist`); + } + }); +}); + +// --------------------------------------------------------------------------- +// #947 regression: migration from prior bare-stem install +// --------------------------------------------------------------------------- + +describe('#947 Hermes: migration from prior bare-stem install', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-947-migrate-')); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('bare-stem dirs from prior install are removed on reinstall', () => { + const srcDir = writeMinimalSourceTree(tmpDir, ['quick']); + const configDir = path.join(tmpDir, 'dest'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, '.gsd-source'), srcDir); + + // Seed a prior bare-stem install: skills/gsd/quick/SKILL.md + const legacySkillDir = path.join(configDir, 'skills', 'gsd', 'quick'); + fs.mkdirSync(legacySkillDir, { recursive: true }); + fs.writeFileSync(path.join(legacySkillDir, 'SKILL.md'), [ + '---', + 'name: quick', + 'description: Quick task (legacy bare-stem)', + '---', + '', + 'Legacy body.', + ].join('\n')); + + installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_FULL); + + // Bare-stem dir must be gone (migrated) + assert.ok(!fs.existsSync(legacySkillDir), + 'skills/gsd/quick/ (bare-stem legacy dir) must be removed on reinstall'); + + // Prefixed dir must exist + const newPath = path.join(configDir, 'skills', 'gsd', 'gsd-quick', 'SKILL.md'); + assert.ok(fs.existsSync(newPath), + 'skills/gsd/gsd-quick/SKILL.md must exist after migration'); + }); + + test('reinstall over bare-stem install leaves NO orphaned bare-stem dirs', () => { + const srcDir = writeMinimalSourceTree(tmpDir, ['quick', 'plan']); + const configDir = path.join(tmpDir, 'dest'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, '.gsd-source'), srcDir); + + // Seed two bare-stem dirs + for (const stem of ['quick', 'plan']) { + const dir = path.join(configDir, 'skills', 'gsd', stem); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'SKILL.md'), `---\nname: ${stem}\ndescription: ${stem}\n---\n`); + } + + installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_FULL); + + const gsdCategoryDir = path.join(configDir, 'skills', 'gsd'); + const entries = fs.readdirSync(gsdCategoryDir, { withFileTypes: true }); + + // Check NO bare-stem dirs remain + for (const entry of entries) { + if (!entry.isDirectory()) continue; + // Bare-stem dirs: name does NOT start with 'gsd-' and is not a known exception + // (DESCRIPTION.md is a file so it won't appear in isDirectory check) + assert.ok( + entry.name.startsWith('gsd-'), + `All dirs under skills/gsd/ must start with 'gsd-'. Found bare-stem: '${entry.name}'`, + ); + } + }); + + test('pre-#2841 flat skills/gsd-/ dirs are still removed (existing migration path)', () => { + const srcDir = writeMinimalSourceTree(tmpDir, ['quick']); + const configDir = path.join(tmpDir, 'dest'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, '.gsd-source'), srcDir); + + // Seed a pre-#2841 flat skill dir: skills/gsd-quick/SKILL.md + const flatSkillDir = path.join(configDir, 'skills', 'gsd-quick'); + fs.mkdirSync(flatSkillDir, { recursive: true }); + fs.writeFileSync(path.join(flatSkillDir, 'SKILL.md'), '---\nname: gsd-quick\n---\nOld flat.'); + + installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_FULL); + + // The pre-#2841 flat dir must still be cleaned up + assert.ok(!fs.existsSync(flatSkillDir), + 'Pre-#2841 flat skills/gsd-quick/ dir must be removed (existing migration)'); + + // The correct post-fix dir must exist + assert.ok(fs.existsSync(path.join(configDir, 'skills', 'gsd', 'gsd-quick', 'SKILL.md')), + 'skills/gsd/gsd-quick/SKILL.md must exist after install'); + }); +}); + +// --------------------------------------------------------------------------- +// #947 adversarial-review: bare-stem cleanup derived from installed set +// (not readGsdCommandNames) — covers skills missing from the commands dir +// --------------------------------------------------------------------------- + +describe('#947 Hermes: adversarial-review bare-stem cleanup (installed-set derivation)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-947-adv-')); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('bare skills/gsd/dev-preferences/ is removed when gsd-dev-preferences/ is installed this run', () => { + // Seed a source tree that includes a 'dev-preferences' skill (e.g. the user's + // commands/gsd/dev-preferences.md, or any skill whose stem is NOT normally in + // the shipped readGsdCommandNames() set). The old cleanup (readGsdCommandNames- + // based) would MISS this bare dir because readGsdCommandNames() reads GSD's + // shipped source, not the user's actual install state. + const srcDir = writeMinimalSourceTree(tmpDir, ['quick', 'dev-preferences']); + const configDir = path.join(tmpDir, 'dest'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, '.gsd-source'), srcDir); + + // Seed the legacy bare-stem dir: skills/gsd/dev-preferences/ (pre-#947 install) + const bareLegacyDir = path.join(configDir, 'skills', 'gsd', 'dev-preferences'); + fs.mkdirSync(bareLegacyDir, { recursive: true }); + fs.writeFileSync(path.join(bareLegacyDir, 'SKILL.md'), [ + '---', + 'name: dev-preferences', + 'description: My dev preferences (legacy bare-stem)', + '---', + '', + 'Legacy body.', + ].join('\n')); + + installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_FULL); + + // gsd-dev-preferences/ must be installed (new prefixed form) + const newPath = path.join(configDir, 'skills', 'gsd', 'gsd-dev-preferences', 'SKILL.md'); + assert.ok(fs.existsSync(newPath), + 'skills/gsd/gsd-dev-preferences/SKILL.md must exist after install'); + + // Bare-stem dir must be gone — even though 'dev-preferences' is NOT in the + // shipped readGsdCommandNames() set (it was user-sourced). The fix derives + // the removal set from gsd-/ dirs installed this run. + assert.ok(!fs.existsSync(bareLegacyDir), + 'skills/gsd/dev-preferences/ (bare-stem) must be removed when gsd-dev-preferences/ was installed'); + }); + + test('user-owned bare dir with no gsd- counterpart is preserved (no over-deletion)', () => { + // A user has a dir 'skills/gsd/my-custom-workflow/' that is NOT a GSD shipped + // skill — GSD never installs 'gsd-my-custom-workflow/'. This dir must survive. + const srcDir = writeMinimalSourceTree(tmpDir, ['quick']); + const configDir = path.join(tmpDir, 'dest'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, '.gsd-source'), srcDir); + + // Seed user-owned bare dir: no corresponding gsd-my-custom-workflow/ will be installed + const userOwnedDir = path.join(configDir, 'skills', 'gsd', 'my-custom-workflow'); + fs.mkdirSync(userOwnedDir, { recursive: true }); + fs.writeFileSync(path.join(userOwnedDir, 'SKILL.md'), [ + '---', + 'name: my-custom-workflow', + 'description: My personal workflow', + '---', + '', + 'Custom body.', + ].join('\n')); + + installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_FULL); + + // User-owned dir must survive — no gsd-my-custom-workflow/ was installed, + // so the removal rule (only remove / when gsd-/ exists) protects it. + assert.ok(fs.existsSync(userOwnedDir), + 'User-owned skills/gsd/my-custom-workflow/ must be preserved (no gsd-my-custom-workflow/ installed)'); + assert.ok(fs.existsSync(path.join(userOwnedDir, 'SKILL.md')), + 'User-owned SKILL.md inside the dir must be preserved'); + }); +}); + +// --------------------------------------------------------------------------- +// #947 regression: manifest/listing prefix +// --------------------------------------------------------------------------- + +describe('#947 Hermes: manifest and skill-listing use gsd- prefix', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-947-manifest-')); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('gsd-manifest.json skill entries use skills/gsd/gsd-/ paths', () => { + const srcDir = writeMinimalSourceTree(tmpDir, ['quick']); + const configDir = path.join(tmpDir, 'dest'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(path.join(configDir, '.gsd-source'), srcDir); + + installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_FULL); + + // The manifest file lives at gsd-core/gsd-manifest.json inside configDir + const manifestPath = path.join(configDir, 'gsd-core', 'gsd-manifest.json'); + if (!fs.existsSync(manifestPath)) return; // manifest optional in test mode + const manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); + const keys = Object.keys(manifest.files || {}); + // Any key for the quick skill must use gsd-quick not bare quick + const bareKey = keys.find(k => k.includes('skills/gsd/quick/')); + assert.ok(!bareKey, + `manifest must not contain bare-stem key 'skills/gsd/quick/', found: ${bareKey}`); + const prefixedKey = keys.find(k => k.includes('skills/gsd/gsd-quick/')); + assert.ok(prefixedKey, + 'manifest must contain prefixed key containing skills/gsd/gsd-quick/'); + }); +}); + +// --------------------------------------------------------------------------- +// #947 regression: non-Hermes runtimes unaffected +// --------------------------------------------------------------------------- + +describe('#947 Non-Hermes runtimes: unaffected by this change', () => { + // Spot-check claude (global/flat) and cline (global/nested) to confirm + // they are not disturbed by the Hermes prefix fix. + + test('claude global install still produces flat skills/gsd-/ layout', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-947-claude-')); + try { + installRuntimeArtifacts('claude', tmpDir, 'global', RESOLVED_FULL); + const skillsDir = path.join(tmpDir, 'skills'); + assert.ok(fs.existsSync(skillsDir), 'skills/ must exist for claude global'); + const entries = fs.readdirSync(skillsDir, { withFileTypes: true }); + const gsdEntries = entries.filter(e => e.isDirectory() && e.name.startsWith('gsd-')); + assert.ok(gsdEntries.length >= 10, + `claude must still emit >= 10 gsd-* skill dirs, got ${gsdEntries.length}`); + // No skills/gsd/ category bucket (that is Hermes-specific) + assert.ok(!fs.existsSync(path.join(skillsDir, 'gsd')), + 'claude must NOT have a skills/gsd/ category bucket (that is Hermes-only)'); + } finally { + cleanup(tmpDir); + } + }); + + test('cline global install still produces skills/ with gsd- prefix nested layout', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-947-cline-')); + try { + installRuntimeArtifacts('cline', tmpDir, 'global', RESOLVED_FULL); + const skillsDir = path.join(tmpDir, 'skills'); + assert.ok(fs.existsSync(skillsDir), 'skills/ must exist for cline global'); + const entries = fs.readdirSync(skillsDir, { withFileTypes: true }); + const routerDirs = entries.filter(e => e.isDirectory() && e.name.startsWith('gsd-ns-')); + assert.ok(routerDirs.length > 0, + 'cline must still emit gsd-ns-* router dirs with gsd- prefix'); + } finally { + cleanup(tmpDir); + } + }); +}); diff --git a/tests/hermes-skills-migration.test.cjs b/tests/hermes-skills-migration.test.cjs index d6509cc7c..7cad0385e 100644 --- a/tests/hermes-skills-migration.test.cjs +++ b/tests/hermes-skills-migration.test.cjs @@ -141,7 +141,7 @@ describe('Hermes Agent: installRuntimeArtifacts', () => { cleanup(tmpDir); }); - test('creates skills/gsd/quick/SKILL.md directory structure (Hermes bare-stem layout)', () => { + test('creates skills/gsd/gsd-quick/SKILL.md directory structure (Hermes prefixed layout, #947)', () => { // Create source command files const srcDir = path.join(tmpDir, 'src', 'commands', 'gsd'); fs.mkdirSync(srcDir, { recursive: true }); @@ -164,15 +164,15 @@ describe('Hermes Agent: installRuntimeArtifacts', () => { installRuntimeArtifacts('hermes', configDir, 'global', resolvedProfileFull); - // Hermes layout: skills/gsd//SKILL.md (ADR-3660) - const skillPath = path.join(configDir, 'skills', 'gsd', 'quick', 'SKILL.md'); - assert.ok(fs.existsSync(skillPath), 'skills/gsd/quick/SKILL.md exists'); + // Hermes layout: skills/gsd/gsd-/SKILL.md (#947 — canonical gsd- prefix restored) + const skillPath = path.join(configDir, 'skills', 'gsd', 'gsd-quick', 'SKILL.md'); + assert.ok(fs.existsSync(skillPath), 'skills/gsd/gsd-quick/SKILL.md exists'); // Verify content (structural — parse frontmatter, don't substring-grep) - // Hermes bare-stem: prefix='', so skillName passed to converter = 'quick' (not 'gsd-quick') + // Hermes prefix='gsd-': skillName passed to converter = 'gsd-quick' const content = fs.readFileSync(skillPath, 'utf8'); const fm = parseFrontmatter(content); - assert.strictEqual(fm.name, 'quick', 'frontmatter name is bare stem for Hermes nested layout'); + assert.strictEqual(fm.name, 'gsd-quick', 'frontmatter name uses canonical gsd- prefix (#947)'); assert.ok(fm.description && fm.description.length > 0, 'description present and non-empty'); assert.strictEqual(fm.version, pkg.version, `Hermes SKILL.md must declare version (got ${JSON.stringify(fm.version)})`); @@ -199,8 +199,8 @@ describe('Hermes Agent: installRuntimeArtifacts', () => { installRuntimeArtifacts('hermes', configDir, 'global', resolvedProfileFull); - // Hermes layout: skills/gsd//SKILL.md - const content = fs.readFileSync(path.join(configDir, 'skills', 'gsd', 'next', 'SKILL.md'), 'utf8'); + // Hermes layout: skills/gsd/gsd-/SKILL.md (#947 — canonical gsd- prefix) + const content = fs.readFileSync(path.join(configDir, 'skills', 'gsd', 'gsd-next', 'SKILL.md'), 'utf8'); assert.ok(!content.includes('~/.claude/'), 'old claude tilde-path removed'); assert.ok(!content.includes('$HOME/.claude/'), 'old claude $HOME-path not present'); }); @@ -223,8 +223,8 @@ describe('Hermes Agent: installRuntimeArtifacts', () => { installRuntimeArtifacts('hermes', configDir, 'global', resolvedProfileFull); - // Hermes layout: skills/gsd//SKILL.md - const content = fs.readFileSync(path.join(configDir, 'skills', 'gsd', 'plan', 'SKILL.md'), 'utf8'); + // Hermes layout: skills/gsd/gsd-/SKILL.md (#947 — canonical gsd- prefix) + const content = fs.readFileSync(path.join(configDir, 'skills', 'gsd', 'gsd-plan', 'SKILL.md'), 'utf8'); assert.ok(!content.includes('$HOME/.claude/'), 'old claude $HOME-path removed'); assert.ok(!content.includes('~/.claude/'), 'old claude tilde-path not present'); }); @@ -254,8 +254,8 @@ describe('Hermes Agent: installRuntimeArtifacts', () => { // _runLegacyInstallMigrations removes skills/gsd-* flat dirs for hermes assert.ok(!fs.existsSync(staleFlatSkillDir), 'stale flat gsd- skill removed'); - // New Hermes layout: skills/gsd//SKILL.md - assert.ok(fs.existsSync(path.join(configDir, 'skills', 'gsd', 'quick', 'SKILL.md')), 'new skill installed at skills/gsd/quick/SKILL.md'); + // New Hermes layout: skills/gsd/gsd-/SKILL.md (#947 — canonical gsd- prefix) + assert.ok(fs.existsSync(path.join(configDir, 'skills', 'gsd', 'gsd-quick', 'SKILL.md')), 'new skill installed at skills/gsd/gsd-quick/SKILL.md'); }); test('preserves agent field in frontmatter', () => { @@ -281,8 +281,8 @@ describe('Hermes Agent: installRuntimeArtifacts', () => { installRuntimeArtifacts('hermes', configDir, 'global', resolvedProfileFull); - // Hermes layout: skills/gsd//SKILL.md - const content = fs.readFileSync(path.join(configDir, 'skills', 'gsd', 'execute', 'SKILL.md'), 'utf8'); + // Hermes layout: skills/gsd/gsd-/SKILL.md (#947 — canonical gsd- prefix) + const content = fs.readFileSync(path.join(configDir, 'skills', 'gsd', 'gsd-execute', 'SKILL.md'), 'utf8'); const fm = parseFrontmatter(content); assert.strictEqual(fm.agent, 'gsd-executor', 'agent field preserved'); }); diff --git a/tests/install-nested-layout.test.cjs b/tests/install-nested-layout.test.cjs index 89bd00cc5..f3e198b8c 100644 --- a/tests/install-nested-layout.test.cjs +++ b/tests/install-nested-layout.test.cjs @@ -36,7 +36,7 @@ const NEST = [ // Only the 6 runtimes below keep the nested layout. { runtime: 'cline', scope: 'global', skillsSub: 'skills', prefix: 'gsd-' }, { runtime: 'qwen', scope: 'global', skillsSub: 'skills', prefix: 'gsd-' }, - { runtime: 'hermes', scope: 'global', skillsSub: 'skills/gsd', prefix: '' }, + { runtime: 'hermes', scope: 'global', skillsSub: 'skills/gsd', prefix: 'gsd-' }, // #947: restored canonical prefix { runtime: 'augment', scope: 'global', skillsSub: 'skills', prefix: 'gsd-' }, { runtime: 'trae', scope: 'global', skillsSub: 'skills', prefix: 'gsd-' }, { runtime: 'antigravity', scope: 'global', skillsSub: 'skills', prefix: 'gsd-' }, @@ -127,8 +127,7 @@ for (const { runtime, scope, skillsSub, prefix } of NEST) { } // Total GSD-owned top-level entries must be EXACTLY 6 (only the routers). - // For prefix='gsd-' runtimes: count dirs starting with 'gsd-'. - // For hermes (prefix=''): count ALL dirs under skills/gsd (everything is GSD-owned). + // All nested runtimes (incl. Hermes after #947) use prefix='gsd-'. const gsdTopLevelCount = prefix !== '' ? topLevel.filter((n) => n.startsWith(prefix)).length : topLevel.filter((n) => fs.statSync(path.join(skillsDir, n)).isDirectory()).length; diff --git a/tests/install-regressions.test.cjs b/tests/install-regressions.test.cjs index f7fb9213f..2600301dc 100644 --- a/tests/install-regressions.test.cjs +++ b/tests/install-regressions.test.cjs @@ -44,22 +44,28 @@ const REAL_COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd'); const MANIFEST = loadSkillsManifest(REAL_COMMANDS_DIR); const RESOLVED_CORE = resolveProfile({ modes: ['core'], manifest: MANIFEST }); -// ─── Defect #1 — Hermes upgrade leaves stale skills/gsd/gsd-/ dirs ──── +// ─── Defect #1 — Hermes upgrade: bare-stem dirs from #3664 era become stale ── +// +// #947 REVERSES #3664: the canonical layout is now skills/gsd/gsd-/ again. +// The migration now removes bare-stem dirs (from #3664: prefix='') and writes +// the gsd-prefixed layout. Pre-existing gsd-prefixed dirs (the "intermediate" +// layout from before #3664) are now the CANONICAL dirs and are kept / updated. -describe('Defect #1 regression (#3664): _runLegacyInstallMigrations removes skills/gsd/gsd-*/ layout', () => { - test('installRuntimeArtifacts removes intermediate skills/gsd/gsd-*/ dirs and writes bare-stem layout', (t) => { +describe('Defect #1 regression (#3664 reversed by #947): bare-stem dirs removed, gsd- prefix written', () => { + test('installRuntimeArtifacts removes bare-stem skills/gsd// dirs and writes gsd- prefixed layout', (t) => { const configDir = createTempDir('gsd-hermes-reg1-'); t.after(() => cleanup(configDir)); assert.strictEqual(typeof installRuntimeArtifacts, 'function', 'installRuntimeArtifacts must be exported from bin/install.js'); - // Pre-create intermediate Hermes layout (between #2841 and #3664) + // Pre-create #3664-era bare-stem Hermes layout (no gsd- prefix, now stale). + // Use real GSD command stems (help, quick) that readGsdCommandNames() knows about. const nestedGsdDir = path.join(configDir, 'skills', 'gsd'); - fs.mkdirSync(path.join(nestedGsdDir, 'gsd-help'), { recursive: true }); - fs.writeFileSync(path.join(nestedGsdDir, 'gsd-help', 'SKILL.md'), '# legacy help\n'); - fs.mkdirSync(path.join(nestedGsdDir, 'gsd-plan'), { recursive: true }); - fs.writeFileSync(path.join(nestedGsdDir, 'gsd-plan', 'SKILL.md'), '# legacy plan\n'); + fs.mkdirSync(path.join(nestedGsdDir, 'help'), { recursive: true }); + fs.writeFileSync(path.join(nestedGsdDir, 'help', 'SKILL.md'), '# legacy bare-stem help\n'); + fs.mkdirSync(path.join(nestedGsdDir, 'quick'), { recursive: true }); + fs.writeFileSync(path.join(nestedGsdDir, 'quick', 'SKILL.md'), '# legacy bare-stem quick\n'); // Sibling non-gsd dir inside skills/gsd/ must survive const userContentDir = path.join(nestedGsdDir, 'user-content'); @@ -68,12 +74,15 @@ describe('Defect #1 regression (#3664): _runLegacyInstallMigrations removes skil installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_CORE); - assert.ok(!fs.existsSync(path.join(nestedGsdDir, 'gsd-help')), - 'skills/gsd/gsd-help/ must be removed (Defect #1)'); - assert.ok(!fs.existsSync(path.join(nestedGsdDir, 'gsd-plan')), - 'skills/gsd/gsd-plan/ must be removed (Defect #1)'); - assert.ok(fs.existsSync(path.join(nestedGsdDir, 'help', 'SKILL.md')), - 'skills/gsd/help/SKILL.md must exist after install'); + // Bare-stem dirs from #3664 must be cleaned + assert.ok(!fs.existsSync(path.join(nestedGsdDir, 'help')), + 'skills/gsd/help/ (bare-stem from #3664) must be removed (#947)'); + assert.ok(!fs.existsSync(path.join(nestedGsdDir, 'quick')), + 'skills/gsd/quick/ (bare-stem from #3664) must be removed (#947)'); + // Canonical gsd- prefixed layout must be written + assert.ok(fs.existsSync(path.join(nestedGsdDir, 'gsd-help', 'SKILL.md')), + 'skills/gsd/gsd-help/SKILL.md must exist after install (#947 canonical layout)'); + // User content preserved assert.ok(fs.existsSync(path.join(userContentDir, 'SKILL.md')), 'user-content must be preserved'); }); @@ -148,7 +157,7 @@ describe('Defect #2 regression (Hermes, #3664): --hermes --profile=core writes s // ─── M1 — Hermes minimal-mode migrates dev-preferences (#2973) ─────────────── -describe('M1 (#2973): --hermes --global --profile=core migrates dev-preferences → skills/gsd/dev-preferences/SKILL.md', () => { +describe('M1 (#2973, #947): --hermes --global --profile=core migrates dev-preferences → skills/gsd/gsd-dev-preferences/SKILL.md', () => { test('dev-preferences migrated to nested Hermes location, legacy source removed', (t) => { const root = createTempDir('gsd-hermes-m1-'); t.after(() => cleanup(root)); @@ -166,9 +175,10 @@ describe('M1 (#2973): --hermes --global --profile=core migrates dev-preferences assert.strictEqual(result.status, 0, `installer exited ${result.status}\n${result.stdout}\n${result.stderr}`); - const skillFile = path.join(root, 'skills', 'gsd', 'dev-preferences', 'SKILL.md'); + // #947: Hermes uses prefix='gsd-' so dev-preferences lands at gsd-dev-preferences/ (not dev-preferences/) + const skillFile = path.join(root, 'skills', 'gsd', 'gsd-dev-preferences', 'SKILL.md'); assert.ok(fs.existsSync(skillFile), - 'skills/gsd/dev-preferences/SKILL.md must exist (M1: nested, not flat)'); + 'skills/gsd/gsd-dev-preferences/SKILL.md must exist (M1+#947: gsd- prefix, nested)'); assert.strictEqual(fs.readFileSync(skillFile, 'utf8'), '# my hermes prefs\n'); assert.ok(!fs.existsSync(path.join(legacyDir, 'dev-preferences.md')), 'legacy source must be removed'); @@ -282,8 +292,8 @@ describe('U2 (#2973): uninstallRuntimeArtifacts claude/global migrates dev-prefe // ─── U3 — Hermes uninstall migrates dev-preferences to NESTED location (#2973) ─ -describe('U3 (#2973): uninstallRuntimeArtifacts hermes migrates dev-preferences → skills/gsd/dev-preferences/SKILL.md', () => { - test('commands/gsd/ NOT recreated, dev-preferences at nested Hermes location', (t) => { +describe('U3 (#2973, #947): uninstallRuntimeArtifacts hermes migrates dev-preferences → skills/gsd/gsd-dev-preferences/SKILL.md', () => { + test('commands/gsd/ NOT recreated, dev-preferences at nested Hermes location with gsd- prefix', (t) => { const configDir = createTempDir('gsd-hermes-uninstall-u3-'); t.after(() => cleanup(configDir)); @@ -298,9 +308,10 @@ describe('U3 (#2973): uninstallRuntimeArtifacts hermes migrates dev-preferences assert.ok(!fs.existsSync(path.join(legacyDir, 'dev-preferences.md')), 'commands/gsd/dev-preferences.md must not exist after hermes uninstall (U3)'); - const skillFile = path.join(configDir, 'skills', 'gsd', 'dev-preferences', 'SKILL.md'); + // #947: Hermes uses prefix='gsd-' so dev-preferences lands at gsd-dev-preferences/ (not dev-preferences/) + const skillFile = path.join(configDir, 'skills', 'gsd', 'gsd-dev-preferences', 'SKILL.md'); assert.ok(fs.existsSync(skillFile), - 'skills/gsd/dev-preferences/SKILL.md must exist at HERMES nested location (U3)'); + 'skills/gsd/gsd-dev-preferences/SKILL.md must exist at HERMES nested location (U3+#947)'); assert.strictEqual(fs.readFileSync(skillFile, 'utf8'), '# my hermes prefs\n'); }); }); diff --git a/tests/install-runtime-artifacts.test.cjs b/tests/install-runtime-artifacts.test.cjs index 8752e3667..b981aa2bc 100644 --- a/tests/install-runtime-artifacts.test.cjs +++ b/tests/install-runtime-artifacts.test.cjs @@ -139,7 +139,7 @@ describe('installRuntimeArtifacts — skills runtimes write gsd-prefixed skill d }); describe('installRuntimeArtifacts — hermes nested layout', () => { - test('hermes: skills/gsd//SKILL.md, no gsd- prefix in name', (t) => { + test('hermes: skills/gsd/gsd-/SKILL.md with gsd- prefix in name (#947)', (t) => { const configDir = createTempDir('gsd-ial-hermes-'); t.after(() => cleanup(configDir)); @@ -147,9 +147,11 @@ describe('installRuntimeArtifacts — hermes nested layout', () => { const nestedDir = path.join(configDir, 'skills', 'gsd'); assert.ok(fs.existsSync(nestedDir)); - assert.ok(fs.existsSync(path.join(nestedDir, 'help', 'SKILL.md'))); - assert.ok(!fs.existsSync(path.join(nestedDir, 'gsd-help')), - 'hermes must NOT have gsd-help prefix'); + // #947: Hermes now uses canonical gsd- prefix — skills/gsd/gsd-/SKILL.md + assert.ok(fs.existsSync(path.join(nestedDir, 'gsd-help', 'SKILL.md')), + 'skills/gsd/gsd-help/SKILL.md must exist (canonical gsd- prefix, #947)'); + assert.ok(!fs.existsSync(path.join(nestedDir, 'help')), + 'bare-stem skills/gsd/help/ must NOT exist (#947 fix)'); }); }); @@ -323,17 +325,23 @@ describe('uninstallRuntimeArtifacts — removes gsd-owned entries, preserves for if (runtime === 'hermes') { const kind = layout.kinds[0]; - const destDir = path.join(configDir, kind.destSubpath); + const destDir = path.join(configDir, kind.destSubpath); // skills/gsd + // Seed a gsd-* prefixed skill (canonical #947 layout) and a bare-stem skill (#3664 era) + fs.mkdirSync(path.join(destDir, 'gsd-help'), { recursive: true }); + fs.writeFileSync(path.join(destDir, 'gsd-help', 'SKILL.md'), '# gsd-help\n'); fs.mkdirSync(path.join(destDir, 'help'), { recursive: true }); - fs.writeFileSync(path.join(destDir, 'help', 'SKILL.md'), '# help\n'); + fs.writeFileSync(path.join(destDir, 'help', 'SKILL.md'), '# bare-stem help (#3664)\n'); const siblingDir = path.join(configDir, 'skills', 'user-skill'); fs.mkdirSync(siblingDir, { recursive: true }); fs.writeFileSync(path.join(siblingDir, 'SKILL.md'), '# user\n'); uninstallRuntimeArtifacts(runtime, configDir, 'global'); - assert.ok(!fs.existsSync(destDir)); - assert.ok(fs.existsSync(path.join(siblingDir, 'SKILL.md'))); + // skills/gsd/ removed (gsd-* removed by _removeGsdEntries, bare-stem by legacy cleanup, + // then DESCRIPTION.md removed, category dir removed as empty) + assert.ok(!fs.existsSync(destDir), 'skills/gsd/ must be removed after uninstall'); + // User skill outside skills/gsd/ preserved + assert.ok(fs.existsSync(path.join(siblingDir, 'SKILL.md')), 'user-skill must be preserved'); return; } @@ -436,7 +444,7 @@ describe('installRuntimeArtifacts — legacy migrations run before layout copy', assert.ok(fs.existsSync(path.join(configDir, 'skills', 'gsd-help', 'SKILL.md'))); }); - test('hermes: legacy flat skills/gsd-*/ migrated AND new nested skills/gsd// written', (t) => { + test('hermes: legacy flat skills/gsd-*/ migrated AND new nested skills/gsd/gsd-/ written (#947)', (t) => { const configDir = createTempDir('gsd-legacy-hermes-install-'); t.after(() => cleanup(configDir)); @@ -446,13 +454,15 @@ describe('installRuntimeArtifacts — legacy migrations run before layout copy', installRuntimeArtifacts('hermes', configDir, 'global', RESOLVED_CORE); - assert.ok(!fs.existsSync(legacyFlatHelp)); - assert.ok(fs.existsSync(path.join(configDir, 'skills', 'gsd', 'help', 'SKILL.md'))); + assert.ok(!fs.existsSync(legacyFlatHelp), 'legacy flat skill must be removed'); + // #947: canonical path is skills/gsd/gsd-/ not skills/gsd// + assert.ok(fs.existsSync(path.join(configDir, 'skills', 'gsd', 'gsd-help', 'SKILL.md')), + 'skills/gsd/gsd-help/SKILL.md must exist after install (#947)'); }); }); describe('uninstallRuntimeArtifacts — legacy cleanup runs before layout removal', () => { - test('hermes: both flat and nested layouts removed', (t) => { + test('hermes: both flat and nested layouts removed (#947: bare-stem dirs cleaned on uninstall)', (t) => { const { uninstallRuntimeArtifacts } = require('../bin/install.js'); const configDir = createTempDir('gsd-legacy-uninstall-hermes-'); t.after(() => cleanup(configDir)); @@ -463,8 +473,9 @@ describe('uninstallRuntimeArtifacts — legacy cleanup runs before layout remova fs.writeFileSync(path.join(flatHelp, 'SKILL.md'), '# legacy flat\n'); const nestedGsd = path.join(skillsDir, 'gsd'); + // Seed a pre-#947 bare-stem GSD skill (no gsd- prefix, from #3664 era) fs.mkdirSync(path.join(nestedGsd, 'help'), { recursive: true }); - fs.writeFileSync(path.join(nestedGsd, 'help', 'SKILL.md'), '# nested help\n'); + fs.writeFileSync(path.join(nestedGsd, 'help', 'SKILL.md'), '# nested help (bare-stem)\n'); const userSkill = path.join(skillsDir, 'user-skill'); fs.mkdirSync(userSkill, { recursive: true }); @@ -472,9 +483,12 @@ describe('uninstallRuntimeArtifacts — legacy cleanup runs before layout remova uninstallRuntimeArtifacts('hermes', configDir, 'global'); - assert.ok(!fs.existsSync(flatHelp)); - assert.ok(!fs.existsSync(nestedGsd)); - assert.ok(fs.existsSync(path.join(userSkill, 'SKILL.md'))); + // Pre-#2841 flat skills/gsd-help/ removed by legacy cleanup + assert.ok(!fs.existsSync(flatHelp), 'flat gsd-help must be removed'); + // skills/gsd/ removed: bare-stem dirs cleaned + no gsd-* dirs remain → empty → removed + assert.ok(!fs.existsSync(nestedGsd), 'skills/gsd/ must be removed after uninstall'); + // User content outside skills/gsd/ preserved + assert.ok(fs.existsSync(path.join(userSkill, 'SKILL.md')), 'user-skill must be preserved'); }); test('claude: legacy commands/gsd/ cleaned AND new skills/ entries removed', (t) => { diff --git a/tests/install.test.cjs b/tests/install.test.cjs index 7ecc72884..93f634611 100644 --- a/tests/install.test.cjs +++ b/tests/install.test.cjs @@ -310,8 +310,8 @@ describe('install/uninstall — hermes (nested skills/gsd//skills/ assert.strictEqual(result.runtime, 'hermes'); assert.strictEqual(result.configDir, fs.realpathSync(targetDir)); - // hermes nests: skills/gsd//skills//SKILL.md - const hermesHelpPath = nestedSkillPath(path.join(targetDir, 'skills', 'gsd'), '', 'help'); + // hermes nests: skills/gsd/gsd-/skills//SKILL.md (#947 — canonical gsd- prefix) + const hermesHelpPath = nestedSkillPath(path.join(targetDir, 'skills', 'gsd'), 'gsd-', 'help'); assert.ok(fs.existsSync(hermesHelpPath), `help SKILL.md must exist at nested path: ${path.relative(targetDir, hermesHelpPath)}`); assert.ok(fs.existsSync(path.join(targetDir, 'skills', 'gsd', 'DESCRIPTION.md')), @@ -322,7 +322,7 @@ describe('install/uninstall — hermes (nested skills/gsd//skills/ const manifest = writeManifest(targetDir, 'hermes'); assert.ok( Object.keys(manifest.files).some(f => - f.startsWith('skills/gsd/' + CHILD_ROUTER['help'] + '/skills/help/') + f.startsWith('skills/gsd/gsd-' + CHILD_ROUTER['help'] + '/skills/help/') ), JSON.stringify(manifest.files) ); diff --git a/tests/runtime-artifact-layout.test.cjs b/tests/runtime-artifact-layout.test.cjs index 782c4e31d..a37cc6d4f 100644 --- a/tests/runtime-artifact-layout.test.cjs +++ b/tests/runtime-artifact-layout.test.cjs @@ -221,7 +221,7 @@ describe('resolveRuntimeArtifactLayout — hermes', () => { assert.strictEqual(layout.kinds.length, 1); assert.strictEqual(layout.kinds[0].kind, 'skills'); assert.strictEqual(layout.kinds[0].destSubpath, 'skills/gsd'); - assert.strictEqual(layout.kinds[0].prefix, ''); + assert.strictEqual(layout.kinds[0].prefix, 'gsd-'); // #947: restored canonical prefix assert.strictEqual(typeof layout.kinds[0].stage, 'function'); }); }); @@ -310,10 +310,10 @@ describe('resolveRuntimeArtifactLayout — kilo', () => { // ─── resolveRuntimeArtifactLayout — edge-cases ────────────────────────────── describe('resolveRuntimeArtifactLayout edge-cases', () => { - test('hermes has destSubpath skills/gsd and empty prefix', () => { + test('hermes has destSubpath skills/gsd and gsd- prefix (#947: restored from bare-stem)', () => { const layout = resolveRuntimeArtifactLayout('hermes', '/tmp/x'); assert.strictEqual(layout.kinds[0].destSubpath, 'skills/gsd'); - assert.strictEqual(layout.kinds[0].prefix, ''); + assert.strictEqual(layout.kinds[0].prefix, 'gsd-'); // #947: bare-stem prefix='' reversed }); test('cline has one skills kind (#782)', () => {