* test(#947): add regression tests and update stale Hermes assertions - Add bug-947-hermes-gsd-prefix.test.cjs: 12 TDD tests covering fresh install canonical layout, bare-stem migration, manifest key format, and non-Hermes runtime isolation - Update hermes-skills-migration.test.cjs: bare-stem → gsd-prefixed path and name assertions (#947 canonical layout) - Update install-nested-layout.test.cjs: Hermes NEST matrix prefix '' → 'gsd-' - Update install-regressions.test.cjs: Defect #1 now seeds bare-stem dirs (help/, quick/) and asserts gsd-help/ canonical output; use real GSD stems so readGsdCommandNames() migration finds them - Update install-runtime-artifacts.test.cjs: Hermes nested layout and legacy migration assertions align with gsd- prefix - Update install.test.cjs: Hermes install test uses gsd- prefixed paths - Update runtime-artifact-layout.test.cjs: prefix '' → 'gsd-' Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(#947): restore gsd- prefix on Hermes skills for canonical dispatch Hermes skills were installing under bare-stem paths (skills/gsd/<stem>/SKILL.md, name: <stem>) due to prefix: '' set in ADR-3660 / #3664. This broke /gsd-<stem> dispatch and forced users to invoke skills without the gsd- namespace prefix. - src/runtime-artifact-layout.cts: change Hermes skillsKind prefix from '' to 'gsd-'; skills now land at skills/gsd/gsd-<stem>/SKILL.md with name: gsd-<stem> - bin/install.js _runLegacyInstallMigrations: invert the #3664 migration — remove stale bare-stem dirs (using readGsdCommandNames() to distinguish GSD-owned stems from user content), keep gsd-* dirs which are now canonical - bin/install.js _runLegacyUninstallCleanup: also remove bare-stem dirs on uninstall for clean teardown - bin/install.js uninstallRuntimeArtifacts: post-cleanup removes DESCRIPTION.md and empty skills/gsd/ category dir on Hermes - bin/install.js: remove skillListPrefix Hermes exception (now uses shared 'gsd-' path) - docs/adr/3660-runtime-artifact-layout-module.md: document #947 reversal of the bare-stem sub-decision Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: add changeset for #947 fix (#955) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(#947): remove ALL pre-migration bare-stem Hermes skills on reinstall (adversarial review) Replace readGsdCommandNames()-based bare-stem cleanup (which missed skills not in the commands source tree, e.g. dev-preferences) with _removeHermesBareStemDirs(), called AFTER the install loop when the exact set of installed gsd-<stem>/ dirs is authoritative. For every gsd-<stem>/ written this run, the corresponding bare skills/gsd/<stem>/ is removed. User-owned bare dirs with no gsd-<stem> counterpart are preserved. Add two adversarial-review regression tests that FAIL on old code: - bare skills/gsd/dev-preferences/ removed when gsd-dev-preferences/ installed - user-owned bare dir with no gsd-<stem> counterpart is preserved (no over-deletion) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
141
bin/install.js
141
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/<stem>/SKILL.md); the intermediate layout had the gsd-
|
||||
// prefix inside the nested dir (skills/gsd/gsd-<stem>/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/<stem>/ cleanup is deferred to AFTER the
|
||||
// layout-driven install loop in installRuntimeArtifacts, where the exact set
|
||||
// of staged gsd-<stem>/ 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/<stem>/ 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-<stem>/ dirs to
|
||||
* skills/gsd/, remove any pre-existing bare-stem dirs (skills/gsd/<stem>/)
|
||||
* that correspond to the newly installed gsd-<stem> 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-<stem>/ 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-<stem>/ 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 <stem>/ dir for which gsd-<stem>/ 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<relPath, Buffer>
|
||||
|
||||
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-<stem>/ dirs to
|
||||
// skills/gsd/, remove any stale bare-stem dirs (skills/gsd/<stem>/) that
|
||||
// correspond to the newly installed gsd-<stem> 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-<stem>/ present now was written this run (or was there before
|
||||
// with the same prefix). User-owned bare dirs with no gsd-<stem> 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-<stem> 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 {
|
||||
|
||||
Reference in New Issue
Block a user