fix(3659): applySurface prunes skill dirs on cluster disable (#3766)
* test(3659): add regression tests for applySurface skill-dir pruning on cluster disable Tests that applySurface with claude global scope correctly prunes ~/.claude/skills/gsd-STEM/ dirs for disabled clusters, preserves gsd-STEM dirs in enabled clusters, leaves non-gsd user dirs untouched, and is idempotent across two consecutive calls. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(3659): applySurface now prunes ~/.claude/skills/gsd-STEM/ on cluster disable Root cause: surface.md directed the AI to use RUNTIME_CONFIG_DIR=~/.claude/skills (the skills sub-directory) instead of the base Claude config dir (~/.claude). When runtimeConfigDir=~/.claude/skills and scope=global, the layout computes dest=~/.claude/skills/skills — the wrong target — so pruning never reached the actual gsd-STEM dirs in ~/.claude/skills/. Fix: - surface.md: correct RUNTIME_CONFIG_DIR to use the base config dir (~/.claude), add explicit SCOPE=global, and update all path references in execution_context. Surface state file moves from ~/.claude/skills/.gsd-surface.json to ~/.claude/.gsd-surface.json, matching install/uninstall conventions. - surface.cjs: extract pruneSkillDirs() as a shared helper (single point of truth for gsd-STEM dir removal). _syncGsdDir now delegates to it instead of having the ownership/prune logic inline. Export pruneSkillDirs for callers that need stand-alone pruning without a full applySurface pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(3659): update changeset to reference PR #3766 * fix(3659): manifest-membership gate on pruneSkillDirs prevents user gsd-* dir data loss Finding 1 (CRITICAL): the prefixed branch previously deleted any on-disk dir that matched the 'gsd-' prefix and was not in retainedNames. A user-created gsd-mything/ would be silently destroyed. Fix: deletion now requires BOTH prefix match AND manifest membership (stem present in manifest). Dirs that match the prefix but are not manifest-known are preserved with a process.stderr warning so the user knows the dir was kept. Finding 2 (type guard): the Hermes (empty-prefix) branch passed manifest directly to new Set([...manifest.keys()]) without verifying it is actually a Map. A truthy non-Map would throw. Fix: safeManifest = (manifest instanceof Map) ? manifest : null, used in both branches. Non-Map manifest triggers the same conservative no-deletions path already used when manifest is absent. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(3659): counter-test for all-clusters-disabled + user gsd-* dir preservation Finding 3: add test (e) that disables every cluster (Object.keys(CLUSTERS)) and asserts three things: 1. All GSD-owned skill dirs (gsd-explore/, gsd-help/) are removed. 2. Non-gsd user dir (my-custom-skill/) is preserved. 3. User-created gsd-mything/ (prefix match, not in manifest) is preserved — this is the critical regression guard for the Finding 1 data-loss fix. Also update the existing _syncGsdDir skills-kind test in surface-apply.test.cjs to pass a manifest that declares old-skill as GSD-owned. Without a manifest the new conservative path correctly preserves all unknown gsd-* dirs, which broke the pre-existing no-manifest assertion; supplying the manifest restores the expected pruning behavior and documents the required calling contract. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(3659): address pr-review-toolkit + codex review findings - Collapse redundant if/else in _syncGsdDir - Collapse duplicate canonicalStems branches in pruneSkillDirs - Update stale module-header comment (config-dir root) - Clarify dead isGsdOwned guard comment - Log rmSync failures to stderr - Add pruneSkillDirs to module-header Exports JSDoc - Remove unused imports in bug-3659 test file Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/wise-pumas-glide.md
Normal file
5
.changeset/wise-pumas-glide.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3766
|
||||
---
|
||||
**`applySurface` now prunes `~/.claude/skills/gsd-STEM/` directories on cluster disable** — matches the install/uninstall behavior; disabled clusters were leaving stale skill dirs on disk because the surface.md spec directed the AI to use the skills sub-directory as `runtimeConfigDir` instead of the base config dir. Extracts `pruneSkillDirs()` as the single point of truth for skill-dir removal.
|
||||
@@ -10,8 +10,9 @@ requires: [config, update]
|
||||
---
|
||||
|
||||
<objective>
|
||||
Manage the runtime skill surface without reinstall. Reads/writes `~/.claude/skills/.gsd-surface.json`
|
||||
(sibling to `.gsd-profile`) and re-stages the active commands/gsd directory in place.
|
||||
Manage the runtime skill surface without reinstall. Reads/writes `~/.claude/.gsd-surface.json`
|
||||
(sibling to `~/.claude/.gsd-profile`) and re-stages the active skills directory in place.
|
||||
Skill dirs live at `~/.claude/skills/gsd-*/`.
|
||||
|
||||
Sub-commands: list · status · profile · disable · enable · reset
|
||||
</objective>
|
||||
@@ -114,15 +115,27 @@ Valid cluster names: `core_loop`, `audit_review`, `milestone`, `research_ideate`
|
||||
|
||||
## runtimeConfigDir resolution
|
||||
|
||||
The `runtimeConfigDir` for `applySurface` is the **base Claude config directory**
|
||||
(`~/.claude`), NOT the skills sub-directory (`~/.claude/skills`).
|
||||
|
||||
This matches `installRuntimeArtifacts` and `uninstallRuntimeArtifacts`, which also
|
||||
receive `~/.claude` as `configDir`. The skill dirs themselves live at
|
||||
`~/.claude/skills/gsd-*/` because the `claude global` layout has `destSubpath =
|
||||
'skills'` — they are derived from `configDir`, not the root for it.
|
||||
|
||||
```bash
|
||||
# Claude Code
|
||||
RUNTIME_CONFIG_DIR=~/.claude/skills
|
||||
# Claude Code — global install
|
||||
RUNTIME_CONFIG_DIR="${CLAUDE_CONFIG_DIR:-$HOME/.claude}"
|
||||
SCOPE="global"
|
||||
|
||||
# Artifact destinations are derived from runtime layout
|
||||
# via resolveRuntimeArtifactLayout(runtime, RUNTIME_CONFIG_DIR, scope)
|
||||
# via resolveRuntimeArtifactLayout(runtime, RUNTIME_CONFIG_DIR, SCOPE)
|
||||
# then applySurface(RUNTIME_CONFIG_DIR, layout, manifest, CLUSTERS)
|
||||
```
|
||||
|
||||
Surface state is stored at `${RUNTIME_CONFIG_DIR}/.gsd-surface.json`
|
||||
(i.e. `~/.claude/.gsd-surface.json`).
|
||||
|
||||
All paths can be overridden by reading the `CLAUDE_CONFIG_DIR` env var if set.
|
||||
|
||||
---
|
||||
@@ -134,8 +147,9 @@ All paths can be overridden by reading the `CLAUDE_CONFIG_DIR` env var if set.
|
||||
- Missing `surface.cjs` → prompt: "Run `npm i -g get-shit-done` to reinstall GSD."
|
||||
|
||||
<execution_context>
|
||||
Surface state file: `~/.claude/skills/.gsd-surface.json`
|
||||
Install profile marker: `~/.claude/skills/.gsd-profile`
|
||||
Surface state file: `~/.claude/.gsd-surface.json`
|
||||
Install profile marker: `~/.claude/.gsd-profile`
|
||||
Skill dirs: `~/.claude/skills/gsd-*/`
|
||||
Engine module: `~/.claude/get-shit-done/bin/lib/surface.cjs`
|
||||
Cluster definitions: `~/.claude/get-shit-done/bin/lib/clusters.cjs`
|
||||
</execution_context>
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
* Runtime surface module — ADR-0011 Phase 2 (Option B).
|
||||
*
|
||||
* Manages the runtime enable/disable surface state (the `.gsd-surface.json` marker in
|
||||
* each runtime's skills dir) independently of the install-time profile marker
|
||||
* each runtime's config dir root (e.g., ~/.claude)) independently of the install-time profile marker
|
||||
* (`.gsd-profile`). Runtime config locations are resolved by callers.
|
||||
*
|
||||
* Effective skill set = base profile ∪ explicitAdds − disabledClusters − explicitRemoves,
|
||||
@@ -15,6 +15,7 @@
|
||||
* resolveSurface(runtimeConfigDir, manifest, clusterMap)
|
||||
* applySurface(runtimeConfigDir, layout, manifest, clusterMap)
|
||||
* listSurface(runtimeConfigDir, layout, manifest, clusterMap)
|
||||
* pruneSkillDirs(skillsDir, retainedNames, prefix, manifest)
|
||||
*/
|
||||
|
||||
const fs = require('fs');
|
||||
@@ -216,6 +217,89 @@ function applySurface(runtimeConfigDir, layout, manifest, clusterMap) {
|
||||
return resolved;
|
||||
}
|
||||
|
||||
/**
|
||||
* Prune GSD-managed skill directories from a skills directory.
|
||||
*
|
||||
* Removes every directory in `skillsDir` that is GSD-owned but NOT listed
|
||||
* in `retainedNames`. User-owned dirs (not matching the GSD ownership criteria)
|
||||
* are always preserved.
|
||||
*
|
||||
* Ownership criteria:
|
||||
* - Non-empty prefix (e.g. 'gsd-'): dir name starts with that prefix AND
|
||||
* appears in the manifest (manifest membership is required). Dirs that match
|
||||
* the prefix but are NOT in the manifest are treated as user-owned and
|
||||
* preserved — this prevents data loss for user-created gsd-* directories.
|
||||
* A warning is written to stderr when such a dir is encountered.
|
||||
* - Empty prefix (Hermes): dir name appears as a canonical skill stem in the
|
||||
* manifest. User dirs not in the manifest are preserved.
|
||||
* - Empty prefix without manifest, or manifest not a Map: conservative; no
|
||||
* dirs are removed.
|
||||
*
|
||||
* This is the single point of truth for skill-dir pruning. Both _syncGsdDir
|
||||
* (surface apply) and callers that need stand-alone pruning use this function.
|
||||
*
|
||||
* @param {string} skillsDir directory that contains the gsd-STEM sub-dirs
|
||||
* @param {Set<string>} retainedNames set of directory names to keep (e.g. 'gsd-help')
|
||||
* @param {string} prefix GSD dir prefix, e.g. 'gsd-' (or '' for Hermes)
|
||||
* @param {Map<string, string[]>} [manifest] optional; required for Hermes empty-prefix case
|
||||
* and for manifest-membership gate in prefixed case.
|
||||
* Must be a Map; any other type is treated as missing.
|
||||
*/
|
||||
function pruneSkillDirs(skillsDir, retainedNames, prefix, manifest) {
|
||||
if (!fs.existsSync(skillsDir)) return;
|
||||
|
||||
// Finding 2: guard against callers passing a truthy non-Map as manifest.
|
||||
// A non-Map manifest would throw on .keys(); treat it as absent and be conservative.
|
||||
const safeManifest = (manifest instanceof Map) ? manifest : null;
|
||||
|
||||
// Build the canonical stem set from the manifest (used for both prefixed and Hermes paths).
|
||||
// Deletion requires manifest membership — without a valid manifest, be conservative.
|
||||
const canonicalStems = safeManifest
|
||||
? new Set([...safeManifest.keys()].filter(k => !k.startsWith('_calls_agents_')))
|
||||
: null;
|
||||
|
||||
for (const entry of fs.readdirSync(skillsDir)) {
|
||||
const entryPath = path.join(skillsDir, entry);
|
||||
if (!fs.statSync(entryPath).isDirectory()) continue;
|
||||
|
||||
let isGsdOwned;
|
||||
if (prefix !== '') {
|
||||
if (!entry.startsWith(prefix)) {
|
||||
// Does not match prefix at all — user-owned, preserve.
|
||||
continue;
|
||||
}
|
||||
if (!canonicalStems) {
|
||||
// No manifest available: cannot confirm ownership — preserve conservatively.
|
||||
continue;
|
||||
}
|
||||
// Finding 1 fix: prefix match is necessary but NOT sufficient.
|
||||
// The dir must also be in the manifest to be considered GSD-owned.
|
||||
// A user-created gsd-* dir that isn't in the manifest is preserved with a warning.
|
||||
if (!canonicalStems.has(entry.slice(prefix.length))) {
|
||||
process.stderr.write(
|
||||
`[gsd] Warning: ${entry} matches GSD prefix '${prefix}' but is not in the manifest — preserving (user-owned or unknown)\n`
|
||||
);
|
||||
continue;
|
||||
}
|
||||
isGsdOwned = true;
|
||||
} else if (canonicalStems) {
|
||||
// Hermes: GSD-owned iff the directory name appears in the canonical manifest.
|
||||
isGsdOwned = canonicalStems.has(entry);
|
||||
} else {
|
||||
// No manifest available: be conservative, don't remove anything.
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!isGsdOwned) continue; // Hermes path only: preserve user-owned dirs not in manifest
|
||||
if (retainedNames.has(entry)) continue; // GSD-owned and in retain set
|
||||
try {
|
||||
fs.rmSync(entryPath, { recursive: true, force: true });
|
||||
} catch (err) {
|
||||
process.stderr.write(`surface: failed to prune ${entryPath}: ${err.message}\n`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sync destination directory from staged source.
|
||||
*
|
||||
@@ -223,7 +307,7 @@ function applySurface(runtimeConfigDir, layout, manifest, clusterMap) {
|
||||
* For 'agents' kind: same, but only remove files starting with 'gsd-' prefix.
|
||||
* For 'skills' kind: iterate directories in destDir matching kind.prefix; add missing
|
||||
* by copying recursively; remove dirs not in staged set. Preserves dirs not matching
|
||||
* the prefix (user-owned skills).
|
||||
* the prefix (user-owned skills). Pruning is delegated to pruneSkillDirs().
|
||||
*
|
||||
* 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
|
||||
@@ -251,47 +335,15 @@ function _syncGsdDir(stagedDir, destDir, kind, manifest) {
|
||||
})
|
||||
);
|
||||
|
||||
// Copy missing dirs from staged to dest
|
||||
// Copy missing dirs from staged to dest (always overwrite to ensure content is current)
|
||||
for (const dirName of stagedDirs) {
|
||||
const destSubDir = path.join(destDir, dirName);
|
||||
if (!fs.existsSync(destSubDir)) {
|
||||
fs.cpSync(path.join(stagedDir, dirName), destSubDir, { recursive: true });
|
||||
} else {
|
||||
// Overwrite to ensure content is current
|
||||
fs.cpSync(path.join(stagedDir, dirName), destSubDir, { recursive: true });
|
||||
}
|
||||
fs.cpSync(path.join(stagedDir, dirName), destSubDir, { recursive: true });
|
||||
}
|
||||
|
||||
// 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 {}
|
||||
}
|
||||
// Prune GSD-owned dirs that are no longer in the staged set.
|
||||
// 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(
|
||||
@@ -372,6 +424,7 @@ module.exports = {
|
||||
resolveSurface,
|
||||
applySurface,
|
||||
listSurface,
|
||||
// Exported for testing
|
||||
// Exported for testing and for callers that need stand-alone pruning
|
||||
pruneSkillDirs,
|
||||
_syncGsdDir,
|
||||
};
|
||||
|
||||
257
tests/bug-3659-applysurface-prune-skill-dirs.test.cjs
Normal file
257
tests/bug-3659-applysurface-prune-skill-dirs.test.cjs
Normal file
@@ -0,0 +1,257 @@
|
||||
'use strict';
|
||||
/**
|
||||
* Regression test for bug #3659
|
||||
*
|
||||
* applySurface did not prune ~/.claude/skills/gsd-STEM dirs when a cluster
|
||||
* was disabled. install/uninstall both prune correctly via _removeGsdEntries;
|
||||
* applySurface called _syncGsdDir with the right logic but the surface.md spec
|
||||
* directed the AI to use RUNTIME_CONFIG_DIR=~/.claude/skills (the skills dir
|
||||
* itself) instead of the base Claude config dir (~/.claude).
|
||||
*
|
||||
* When runtimeConfigDir = ~/.claude/skills and scope = 'global':
|
||||
* kind.destSubpath = 'skills'
|
||||
* dest = path.join('~/.claude/skills', 'skills') = ~/.claude/skills/skills WRONG
|
||||
*
|
||||
* The pruning ran against the wrong (non-existent) dir so stale gsd-Y dirs
|
||||
* were never removed from ~/.claude/skills/.
|
||||
*
|
||||
* Fix:
|
||||
* 1. surface.md RUNTIME_CONFIG_DIR changed to use the base Claude config dir
|
||||
* (getGlobalDir('claude') = ~/.claude), not ~/.claude/skills.
|
||||
* 2. Surface state file moves to <configDir>/.gsd-surface.json at the config
|
||||
* root, matching install/uninstall conventions.
|
||||
* 3. applySurface is called with scope='global' so the skills kind is active.
|
||||
*
|
||||
* Tests:
|
||||
* a) disabled cluster gsd-STEM dirs are REMOVED from ~/.claude/skills/
|
||||
* b) gsd-STEM dirs in the retain set are preserved
|
||||
* c) non-gsd dirs are UNTOUCHED (user-owned)
|
||||
* d) idempotence: running applySurface twice produces the same on-disk state
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const { writeSurface, applySurface } = require('../get-shit-done/bin/lib/surface.cjs');
|
||||
const { loadSkillsManifest } = require('../get-shit-done/bin/lib/install-profiles.cjs');
|
||||
const { CLUSTERS } = require('../get-shit-done/bin/lib/clusters.cjs');
|
||||
const { resolveRuntimeArtifactLayout } = require('../get-shit-done/bin/lib/runtime-artifact-layout.cjs');
|
||||
const { createTempDir, cleanup } = require('./helpers.cjs');
|
||||
|
||||
const REAL_COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd');
|
||||
|
||||
/**
|
||||
* Build a minimal fixture simulating a Claude global install.
|
||||
*
|
||||
* configDir — analogous to ~/.claude
|
||||
* skillsDir — analogous to ~/.claude/skills (contains gsd-* dirs)
|
||||
*
|
||||
* Pre-populated with:
|
||||
* gsd-explore/SKILL.md — in research_ideate cluster (will be disabled)
|
||||
* gsd-help/SKILL.md — in core_loop cluster (will remain enabled)
|
||||
* my-custom-skill/ — user-owned, not gsd-prefixed (must never be touched)
|
||||
*/
|
||||
function createFixture() {
|
||||
const configDir = createTempDir('gsd-bug3659-');
|
||||
const skillsDir = path.join(configDir, 'skills');
|
||||
fs.mkdirSync(skillsDir, { recursive: true });
|
||||
|
||||
const gsdExplore = path.join(skillsDir, 'gsd-explore');
|
||||
const gsdHelp = path.join(skillsDir, 'gsd-help');
|
||||
const userSkill = path.join(skillsDir, 'my-custom-skill');
|
||||
|
||||
for (const d of [gsdExplore, gsdHelp, userSkill]) {
|
||||
fs.mkdirSync(d, { recursive: true });
|
||||
fs.writeFileSync(path.join(d, 'SKILL.md'), '# skill\n', 'utf8');
|
||||
}
|
||||
|
||||
return { configDir, skillsDir, gsdExplore, gsdHelp, userSkill };
|
||||
}
|
||||
|
||||
/**
|
||||
* Extended fixture that also includes a user-created gsd-* directory.
|
||||
* Used by the all-clusters-disabled counter-test to prove the manifest-membership
|
||||
* gate (Finding 1 fix) protects user-owned gsd-* dirs from data loss.
|
||||
*/
|
||||
function createFixtureWithUserGsdDir() {
|
||||
const base = createFixture();
|
||||
const userGsdDir = path.join(base.skillsDir, 'gsd-mything');
|
||||
fs.mkdirSync(userGsdDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(userGsdDir, 'SKILL.md'), '# user skill\n', 'utf8');
|
||||
return { ...base, userGsdDir };
|
||||
}
|
||||
|
||||
describe('bug-3659: applySurface prunes ~/.claude/skills/gsd-*/ on cluster disable', () => {
|
||||
test('(a) disabled cluster gsd-* dirs are removed from skills dir', (t) => {
|
||||
const { configDir, skillsDir, gsdExplore, gsdHelp } = createFixture();
|
||||
t.after(() => cleanup(configDir));
|
||||
|
||||
// Surface state at configDir (= ~/.claude), NOT at skillsDir (= ~/.claude/skills).
|
||||
// This is the corrected location after the fix.
|
||||
writeSurface(configDir, {
|
||||
baseProfile: 'full',
|
||||
disabledClusters: ['research_ideate'], // contains 'explore'
|
||||
explicitAdds: [],
|
||||
explicitRemoves: [],
|
||||
});
|
||||
|
||||
const manifest = loadSkillsManifest(REAL_COMMANDS_DIR);
|
||||
// scope='global' gives the skills kind for Claude (destSubpath='skills', prefix='gsd-')
|
||||
const layout = resolveRuntimeArtifactLayout('claude', configDir, 'global');
|
||||
applySurface(configDir, layout, manifest, CLUSTERS);
|
||||
|
||||
// gsd-explore is in the research_ideate cluster which was disabled:
|
||||
// it must be pruned from skillsDir.
|
||||
assert.ok(
|
||||
!fs.existsSync(gsdExplore),
|
||||
'gsd-explore/ must be removed from skills dir when research_ideate cluster is disabled'
|
||||
);
|
||||
|
||||
// gsd-help is in core_loop (not disabled) and must survive.
|
||||
assert.ok(
|
||||
fs.existsSync(gsdHelp),
|
||||
'gsd-help/ must be preserved when its cluster is not disabled'
|
||||
);
|
||||
});
|
||||
|
||||
test('(b) gsd-* dirs in retained clusters are preserved', (t) => {
|
||||
const { configDir, skillsDir, gsdHelp } = createFixture();
|
||||
t.after(() => cleanup(configDir));
|
||||
|
||||
// Disable a cluster that does NOT include help (core_loop has help)
|
||||
writeSurface(configDir, {
|
||||
baseProfile: 'full',
|
||||
disabledClusters: ['research_ideate'],
|
||||
explicitAdds: [],
|
||||
explicitRemoves: [],
|
||||
});
|
||||
|
||||
const manifest = loadSkillsManifest(REAL_COMMANDS_DIR);
|
||||
const layout = resolveRuntimeArtifactLayout('claude', configDir, 'global');
|
||||
applySurface(configDir, layout, manifest, CLUSTERS);
|
||||
|
||||
assert.ok(
|
||||
fs.existsSync(gsdHelp),
|
||||
'gsd-help/ must be preserved — core_loop cluster remains enabled'
|
||||
);
|
||||
});
|
||||
|
||||
test('(c) non-gsd user dirs are untouched', (t) => {
|
||||
const { configDir, skillsDir, userSkill } = createFixture();
|
||||
t.after(() => cleanup(configDir));
|
||||
|
||||
writeSurface(configDir, {
|
||||
baseProfile: 'full',
|
||||
disabledClusters: ['research_ideate'],
|
||||
explicitAdds: [],
|
||||
explicitRemoves: [],
|
||||
});
|
||||
|
||||
const manifest = loadSkillsManifest(REAL_COMMANDS_DIR);
|
||||
const layout = resolveRuntimeArtifactLayout('claude', configDir, 'global');
|
||||
applySurface(configDir, layout, manifest, CLUSTERS);
|
||||
|
||||
assert.ok(
|
||||
fs.existsSync(userSkill),
|
||||
'my-custom-skill/ (non-gsd user dir) must be preserved by applySurface'
|
||||
);
|
||||
assert.ok(
|
||||
fs.existsSync(path.join(userSkill, 'SKILL.md')),
|
||||
'user skill SKILL.md must be untouched'
|
||||
);
|
||||
});
|
||||
|
||||
test('(d) idempotence: running applySurface twice produces identical on-disk state', (t) => {
|
||||
const { configDir, skillsDir, gsdExplore, gsdHelp, userSkill } = createFixture();
|
||||
t.after(() => cleanup(configDir));
|
||||
|
||||
writeSurface(configDir, {
|
||||
baseProfile: 'full',
|
||||
disabledClusters: ['research_ideate'],
|
||||
explicitAdds: [],
|
||||
explicitRemoves: [],
|
||||
});
|
||||
|
||||
const manifest = loadSkillsManifest(REAL_COMMANDS_DIR);
|
||||
const layout = resolveRuntimeArtifactLayout('claude', configDir, 'global');
|
||||
|
||||
// First apply
|
||||
applySurface(configDir, layout, manifest, CLUSTERS);
|
||||
const afterFirst = fs.readdirSync(skillsDir).sort();
|
||||
|
||||
// Second apply — must produce exactly the same set
|
||||
applySurface(configDir, layout, manifest, CLUSTERS);
|
||||
const afterSecond = fs.readdirSync(skillsDir).sort();
|
||||
|
||||
assert.deepStrictEqual(
|
||||
afterSecond,
|
||||
afterFirst,
|
||||
'skills dir contents must be identical after two consecutive applySurface calls (idempotent)'
|
||||
);
|
||||
|
||||
// Double-check the pruned dir is gone after both runs
|
||||
assert.ok(
|
||||
!fs.existsSync(gsdExplore),
|
||||
'gsd-explore/ must remain absent after second applySurface call'
|
||||
);
|
||||
|
||||
// User dir must survive both runs
|
||||
assert.ok(
|
||||
fs.existsSync(userSkill),
|
||||
'my-custom-skill/ must survive both applySurface calls'
|
||||
);
|
||||
});
|
||||
|
||||
test('(e) all-clusters-disabled: all gsd-owned dirs removed; user dirs and user gsd-* dirs survive', (t) => {
|
||||
// Counter-test for Finding 1 (data-loss class) and Finding 3 (missing coverage).
|
||||
//
|
||||
// Disables EVERY cluster so the resolved skill set is empty.
|
||||
// Assertions:
|
||||
// 1. gsd-explore/ — GSD-owned, disabled cluster → REMOVED
|
||||
// 2. gsd-help/ — GSD-owned, disabled cluster → REMOVED
|
||||
// 3. my-custom-skill/ — user-owned, no gsd- prefix → PRESERVED
|
||||
// 4. gsd-mything/ — prefix match but NOT in manifest → PRESERVED (Finding 1 fix)
|
||||
const { configDir, skillsDir, gsdExplore, gsdHelp, userSkill, userGsdDir } =
|
||||
createFixtureWithUserGsdDir();
|
||||
t.after(() => cleanup(configDir));
|
||||
|
||||
const allClusters = Object.keys(CLUSTERS);
|
||||
|
||||
writeSurface(configDir, {
|
||||
baseProfile: 'full',
|
||||
disabledClusters: allClusters,
|
||||
explicitAdds: [],
|
||||
explicitRemoves: [],
|
||||
});
|
||||
|
||||
const manifest = loadSkillsManifest(REAL_COMMANDS_DIR);
|
||||
const layout = resolveRuntimeArtifactLayout('claude', configDir, 'global');
|
||||
applySurface(configDir, layout, manifest, CLUSTERS);
|
||||
|
||||
// 1. GSD-owned dirs in now-disabled clusters must be removed.
|
||||
assert.ok(
|
||||
!fs.existsSync(gsdExplore),
|
||||
'gsd-explore/ must be removed when all clusters are disabled'
|
||||
);
|
||||
assert.ok(
|
||||
!fs.existsSync(gsdHelp),
|
||||
'gsd-help/ must be removed when all clusters are disabled'
|
||||
);
|
||||
|
||||
// 2. Non-gsd user dir must be preserved regardless.
|
||||
assert.ok(
|
||||
fs.existsSync(userSkill),
|
||||
'my-custom-skill/ (non-gsd user dir) must survive when all clusters are disabled'
|
||||
);
|
||||
|
||||
// 3. User-created gsd-* dir NOT in the manifest must be preserved.
|
||||
// This is the critical Finding 1 regression guard: without the manifest-membership
|
||||
// gate, gsd-mything/ would have been silently deleted.
|
||||
assert.ok(
|
||||
fs.existsSync(userGsdDir),
|
||||
'gsd-mything/ (user-created gsd-* dir not in manifest) must be preserved — ' +
|
||||
'prefix match alone must not trigger deletion (Finding 1 data-loss fix)'
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -196,10 +196,22 @@ describe('applySurface', () => {
|
||||
fs.writeFileSync(path.join(foreignDir, 'SKILL.md'), '# custom\n', 'utf8');
|
||||
|
||||
const skillsKind = { kind: 'skills', destSubpath: 'skills', prefix: 'gsd-', stage: () => stagedDir };
|
||||
_syncGsdDir(stagedDir, destDir, skillsKind);
|
||||
|
||||
// Build a minimal manifest that includes the GSD-owned stems so that the
|
||||
// manifest-membership gate (Finding 1 fix) correctly identifies gsd-old-skill
|
||||
// as GSD-owned and prunes it. Without a manifest the new code conservatively
|
||||
// preserves all gsd-* dirs it cannot confirm are GSD-owned.
|
||||
const manifest = new Map([
|
||||
['help', []],
|
||||
['update', []],
|
||||
['old-skill', []], // GSD-owned stale stem — must be pruned when not in staged set
|
||||
]);
|
||||
|
||||
_syncGsdDir(stagedDir, destDir, skillsKind, manifest);
|
||||
|
||||
assert.ok(fs.existsSync(path.join(destDir, stem1, 'SKILL.md')), 'gsd-help/SKILL.md should be copied');
|
||||
assert.ok(fs.existsSync(path.join(destDir, stem2, 'SKILL.md')), 'gsd-update/SKILL.md should be copied');
|
||||
// stale gsd- dir removed (it's in the manifest so it is GSD-owned, but not in staged set)
|
||||
assert.ok(!fs.existsSync(staleDir), 'stale gsd-old-skill dir should be removed');
|
||||
assert.ok(fs.existsSync(foreignDir), 'my-custom-skill dir should be preserved');
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user