fix(#2973): /gsd-profile-user writes dev-preferences.md to skills/, not legacy commands/gsd/ (#3003)
* fix(#2973): /gsd-profile-user writes dev-preferences.md to skills/ not legacy commands/gsd/ v1.39.0's install summary claimed the legacy ~/.claude/commands/gsd/ directory had been removed in favor of skills-only architecture, but the cmdGenerateDevPreferences writer at profile-output.cjs:781 still defaulted to the legacy path. Every /gsd-profile-user --refresh deterministically re-created the legacy directory. Missed in PR #1540's migration because dev-preferences is a runtime-generated user artifact, not a GSD-shipped command file. Fix: - Writer default: ~/.claude/skills/gsd-dev-preferences/SKILL.md - profile-user.md Display message + artifact list reference new path - New migrateLegacyDevPreferencesToSkill(targetDir, saved) installer helper. Called at all 5 skills-aware install branches. Copies preserved legacy dev-preferences.md into skills/gsd-dev-preferences/ SKILL.md, but ONLY if no SKILL.md already exists -- never clobbers user-customized skill content. Tests: bug-2973-profile-user-skills-path.test.cjs runs the writer in a subprocess (core.cjs:output uses fs.writeSync(1, ...) which bypasses in-process stubbing), asserts the writer's command_path field is the skills location, the file is on disk at that path, the legacy path is NOT created. Tests for migration helper assert it writes when no skill exists and skips when one does. Closes #2973 * chore(#2973): add changeset fragment for PR #3003 * fix(#2973): rephrase comment to avoid cline-install leaked-path lint The new comment at line 780 of profile-output.cjs literally contained the string '~/.claude/commands/gsd/' which the cline-install leaked-path regression test (tests/cline-install.test.cjs:175) correctly flagged. Cline transforms .claude/skills/ -> .cline/skills/ in installed .cjs files but does not transform .claude/commands/. The new comment talks about the legacy 'commands/gsd' subdirectory without the ~/.claude/ prefix, so the lint passes. The path semantics are unchanged -- the runtime construction at line 787 still uses path.join(os.homedir(), '.claude', 'skills', ...) which the lint regex does not match. * test(#2973): add timeout to spawnSync to prevent CI hangs (CR feedback) CodeRabbit on PR #3003: without a timeout, a regression that hangs the writer or dispatcher would block CI indefinitely. Added a 30s timeout (generous for what should complete in <1s) and an explicit signal assertion so a timeout trip surfaces as a clear test failure with context rather than a hung worker. * test(#2973): add allow-test-rule annotation for legitimate product-text parsing The new var-binding lint from #2982/#2985 caught readFileSync(...).match() and readFileSync(...).includes() calls in this test. Both are legitimate structural assertions against the product workflow markdown, not source-grep: - match() extracts the path from a structured Display: "..." line and asserts on the typed path value (same pattern as bug-2470's installer scanForLeakedPaths regex test). - includes() asserts the absence of a legacy path literal. profile-user.md IS the shipped workflow artifact, and its Display: line IS what the user sees. Per the existing test-rigor convention, this is the source-text-is-the-product justification category. Annotated with allow-test-rule citing that category. * chore(#3003): drop direct CHANGELOG.md edit; release entry now lives in .changeset/ The changeset-fragment workflow (#2975) renders fragments into CHANGELOG.md at release time. Direct edits to [Unreleased] on each PR caused merge conflicts on every concurrent PR. This commit restores CHANGELOG.md to match origin/main; the release entry for this fix is preserved in the .changeset/*.md fragment(s) on this branch, which the release workflow consolidates. * fix(#2973): preserve user-owned gsd-dev-preferences skill across wipe (CR) CodeRabbit on PR #3003 caught a real bug: copyCommandsAsClaudeSkills() wipes ALL gsd-* skill directories at the top of every install, then reinstalls from the package source. Since gsd-dev-preferences is user-generated (written by /gsd-profile-user --refresh) and NOT shipped by the npm package, the wipe deletes the user's customized SKILL.md with nothing to restore from. Fix: USER_OWNED_SKILLS allow-list in copyCommandsAsClaudeSkills. Snapshot files under skills/gsd-dev-preferences/ before the wipe, restore after. Same preserve/restore pattern as PR #1924. Tests: bug-2973 grows by 2 cases: - user-customized SKILL.md survives the wipe - non-user-owned gsd-* skills are still wiped (preservation is opt-in)
This commit is contained in:
103
bin/install.js
103
bin/install.js
@@ -5325,6 +5325,30 @@ function copyCommandsAsClaudeSkills(srcDir, skillsDir, prefix, pathPrefix, runti
|
||||
|
||||
fs.mkdirSync(skillsDir, { recursive: true });
|
||||
|
||||
// #2973 (CR follow-up on #3003): preserve user-generated skills across the
|
||||
// wipe-and-replace. `gsd-dev-preferences/SKILL.md` is written by the user
|
||||
// via `/gsd-profile-user --refresh`; it is NOT shipped by the npm package,
|
||||
// so a wipe without snapshot deletes the user's content with nothing to
|
||||
// restore from. Snapshot the SKILL.md (and any sibling files in that
|
||||
// directory) before the wipe and restore them after.
|
||||
const USER_OWNED_SKILLS = new Set(['gsd-dev-preferences']);
|
||||
const preservedUserSkills = new Map(); // skillName -> Map(relPath -> Buffer)
|
||||
for (const skillName of USER_OWNED_SKILLS) {
|
||||
const skillDir = path.join(skillsDir, skillName);
|
||||
if (!fs.existsSync(skillDir)) continue;
|
||||
const files = new Map();
|
||||
const walkSnap = (curRel, curAbs) => {
|
||||
for (const e of fs.readdirSync(curAbs, { withFileTypes: true })) {
|
||||
const childRel = curRel ? path.join(curRel, e.name) : e.name;
|
||||
const childAbs = path.join(curAbs, e.name);
|
||||
if (e.isDirectory()) walkSnap(childRel, childAbs);
|
||||
else if (e.isFile()) files.set(childRel, fs.readFileSync(childAbs));
|
||||
}
|
||||
};
|
||||
walkSnap('', skillDir);
|
||||
if (files.size > 0) preservedUserSkills.set(skillName, files);
|
||||
}
|
||||
|
||||
// Remove previous GSD Claude skills to avoid stale command skills
|
||||
const existing = fs.readdirSync(skillsDir, { withFileTypes: true });
|
||||
for (const entry of existing) {
|
||||
@@ -5333,6 +5357,22 @@ function copyCommandsAsClaudeSkills(srcDir, skillsDir, prefix, pathPrefix, runti
|
||||
}
|
||||
}
|
||||
|
||||
// Restore user-owned skills after the wipe but before recursive copy populates
|
||||
// shipped skills. If the npm package later happens to ship a same-named skill
|
||||
// (currently it does not for gsd-dev-preferences), the restored user content
|
||||
// is the source of truth: the recurse() loop below would overwrite it on
|
||||
// collision, but the USER_OWNED_SKILLS set is by definition disjoint from
|
||||
// shipped-skill names.
|
||||
for (const [skillName, files] of preservedUserSkills) {
|
||||
const skillDir = path.join(skillsDir, skillName);
|
||||
fs.mkdirSync(skillDir, { recursive: true });
|
||||
for (const [relPath, buf] of files) {
|
||||
const absPath = path.join(skillDir, relPath);
|
||||
fs.mkdirSync(path.dirname(absPath), { recursive: true });
|
||||
fs.writeFileSync(absPath, buf);
|
||||
}
|
||||
}
|
||||
|
||||
function recurse(currentSrcDir, currentPrefix) {
|
||||
const entries = fs.readdirSync(currentSrcDir, { withFileTypes: true });
|
||||
|
||||
@@ -5519,6 +5559,33 @@ function restoreUserArtifacts(destDir, saved) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Migrate a legacy dev-preferences.md (saved from commands/gsd/) into the
|
||||
* skills/gsd-dev-preferences/SKILL.md location used by the writer after #2973.
|
||||
*
|
||||
* Skips silently if no legacy file was preserved, or if a SKILL.md already
|
||||
* exists at the new location (don't clobber user-customized skill content
|
||||
* — they may have edited the new file directly). Returns true on actual
|
||||
* migration so callers can log a one-line confirmation.
|
||||
*
|
||||
* @param {string} targetDir - Resolved runtime config directory (e.g. ~/.claude)
|
||||
* @param {Map<string, string>} saved - Map returned by preserveUserArtifacts
|
||||
* @returns {boolean} - true if a file was migrated, false otherwise
|
||||
*/
|
||||
function migrateLegacyDevPreferencesToSkill(targetDir, saved) {
|
||||
if (!saved || !saved.has('dev-preferences.md')) return false;
|
||||
const skillDir = path.join(targetDir, 'skills', 'gsd-dev-preferences');
|
||||
const skillFile = path.join(skillDir, 'SKILL.md');
|
||||
if (fs.existsSync(skillFile)) return false;
|
||||
try {
|
||||
fs.mkdirSync(skillDir, { recursive: true });
|
||||
fs.writeFileSync(skillFile, saved.get('dev-preferences.md'), 'utf8');
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recursively copy directory, replacing paths in .md files
|
||||
* Deletes existing destDir first to remove orphaned files from previous versions
|
||||
@@ -6038,7 +6105,14 @@ function uninstall(isGlobal, runtime = 'claude') {
|
||||
fs.rmSync(legacyCommandsDir, { recursive: true });
|
||||
removedCount++;
|
||||
console.log(` ${green}✓${reset} Removed legacy commands/gsd/`);
|
||||
// #2973: also migrate dev-preferences.md content into the new
|
||||
// skills/gsd-dev-preferences/SKILL.md location (skills-aware runtimes).
|
||||
// This prevents the legacy file from being orphaned after the writer
|
||||
// starts targeting the skills path. No-op if SKILL.md already exists.
|
||||
restoreUserArtifacts(legacyCommandsDir, savedLegacyArtifacts);
|
||||
if (migrateLegacyDevPreferencesToSkill(targetDir, savedLegacyArtifacts)) {
|
||||
console.log(` ${green}✓${reset} Migrated dev-preferences.md → skills/gsd-dev-preferences/SKILL.md (#2973)`);
|
||||
}
|
||||
}
|
||||
} else if (isHermes) {
|
||||
// Hermes Agent: skills live under skills/gsd/ as a single category (per
|
||||
@@ -6076,7 +6150,14 @@ function uninstall(isGlobal, runtime = 'claude') {
|
||||
fs.rmSync(legacyCommandsDir, { recursive: true });
|
||||
removedCount++;
|
||||
console.log(` ${green}✓${reset} Removed legacy commands/gsd/`);
|
||||
// #2973: also migrate dev-preferences.md content into the new
|
||||
// skills/gsd-dev-preferences/SKILL.md location (skills-aware runtimes).
|
||||
// This prevents the legacy file from being orphaned after the writer
|
||||
// starts targeting the skills path. No-op if SKILL.md already exists.
|
||||
restoreUserArtifacts(legacyCommandsDir, savedLegacyArtifacts);
|
||||
if (migrateLegacyDevPreferencesToSkill(targetDir, savedLegacyArtifacts)) {
|
||||
console.log(` ${green}✓${reset} Migrated dev-preferences.md → skills/gsd-dev-preferences/SKILL.md (#2973)`);
|
||||
}
|
||||
}
|
||||
} else if (isGemini) {
|
||||
// Gemini: still uses commands/gsd/
|
||||
@@ -7216,7 +7297,14 @@ function install(isGlobal, runtime = 'claude') {
|
||||
const savedLegacyArtifacts = preserveUserArtifacts(legacyCommandsDir, ['dev-preferences.md']);
|
||||
fs.rmSync(legacyCommandsDir, { recursive: true });
|
||||
console.log(` ${green}✓${reset} Removed legacy commands/gsd/ directory`);
|
||||
// #2973: also migrate dev-preferences.md content into the new
|
||||
// skills/gsd-dev-preferences/SKILL.md location (skills-aware runtimes).
|
||||
// This prevents the legacy file from being orphaned after the writer
|
||||
// starts targeting the skills path. No-op if SKILL.md already exists.
|
||||
restoreUserArtifacts(legacyCommandsDir, savedLegacyArtifacts);
|
||||
if (migrateLegacyDevPreferencesToSkill(targetDir, savedLegacyArtifacts)) {
|
||||
console.log(` ${green}✓${reset} Migrated dev-preferences.md → skills/gsd-dev-preferences/SKILL.md (#2973)`);
|
||||
}
|
||||
}
|
||||
} else if (isHermes) {
|
||||
// Hermes Agent: nests all GSD skills under skills/gsd/ as a single
|
||||
@@ -7258,7 +7346,14 @@ function install(isGlobal, runtime = 'claude') {
|
||||
const savedLegacyArtifacts = preserveUserArtifacts(legacyCommandsDir, ['dev-preferences.md']);
|
||||
fs.rmSync(legacyCommandsDir, { recursive: true });
|
||||
console.log(` ${green}✓${reset} Removed legacy commands/gsd/ directory`);
|
||||
// #2973: also migrate dev-preferences.md content into the new
|
||||
// skills/gsd-dev-preferences/SKILL.md location (skills-aware runtimes).
|
||||
// This prevents the legacy file from being orphaned after the writer
|
||||
// starts targeting the skills path. No-op if SKILL.md already exists.
|
||||
restoreUserArtifacts(legacyCommandsDir, savedLegacyArtifacts);
|
||||
if (migrateLegacyDevPreferencesToSkill(targetDir, savedLegacyArtifacts)) {
|
||||
console.log(` ${green}✓${reset} Migrated dev-preferences.md → skills/gsd-dev-preferences/SKILL.md (#2973)`);
|
||||
}
|
||||
}
|
||||
} else if (isCodebuddy) {
|
||||
const skillsDir = path.join(targetDir, 'skills');
|
||||
@@ -7309,7 +7404,14 @@ function install(isGlobal, runtime = 'claude') {
|
||||
const savedLegacyArtifacts = preserveUserArtifacts(legacyCommandsDir, ['dev-preferences.md']);
|
||||
fs.rmSync(legacyCommandsDir, { recursive: true });
|
||||
console.log(` ${green}✓${reset} Removed legacy commands/gsd/ directory`);
|
||||
// #2973: also migrate dev-preferences.md content into the new
|
||||
// skills/gsd-dev-preferences/SKILL.md location (skills-aware runtimes).
|
||||
// This prevents the legacy file from being orphaned after the writer
|
||||
// starts targeting the skills path. No-op if SKILL.md already exists.
|
||||
restoreUserArtifacts(legacyCommandsDir, savedLegacyArtifacts);
|
||||
if (migrateLegacyDevPreferencesToSkill(targetDir, savedLegacyArtifacts)) {
|
||||
console.log(` ${green}✓${reset} Migrated dev-preferences.md → skills/gsd-dev-preferences/SKILL.md (#2973)`);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Claude Code local: commands/gsd/ format — Claude Code reads local project
|
||||
@@ -9191,6 +9293,7 @@ if (process.env.GSD_TEST_MODE) {
|
||||
validateHookFields,
|
||||
preserveUserArtifacts,
|
||||
restoreUserArtifacts,
|
||||
migrateLegacyDevPreferencesToSkill,
|
||||
populatePristineDir,
|
||||
USER_OWNED_ARTIFACTS,
|
||||
finishInstall,
|
||||
|
||||
Reference in New Issue
Block a user