diff --git a/.changeset/witty-hawks-jump.md b/.changeset/witty-hawks-jump.md new file mode 100644 index 000000000..c5a3cadaa --- /dev/null +++ b/.changeset/witty-hawks-jump.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2973 +--- +/gsd-profile-user --refresh writes dev-preferences.md to ~/.claude/skills/gsd-dev-preferences/SKILL.md instead of the legacy commands/gsd/ directory. Installer migrates any preserved legacy file to the new location. See #2973. diff --git a/bin/install.js b/bin/install.js index 0426fd583..390f912d5 100755 --- a/bin/install.js +++ b/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} 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, diff --git a/get-shit-done/bin/lib/profile-output.cjs b/get-shit-done/bin/lib/profile-output.cjs index e899383b1..2219ed4e4 100644 --- a/get-shit-done/bin/lib/profile-output.cjs +++ b/get-shit-done/bin/lib/profile-output.cjs @@ -776,9 +776,17 @@ function cmdGenerateDevPreferences(cwd, options, raw) { } template = template.replace(/\{\{stack_preferences\}\}/g, stackBlock); + // #2973: v1.39.0's skills-only migration removed the legacy + // commands/gsd subdirectory in favor of skills//SKILL.md under + // the runtime config dir. This writer was missed in the migration + // (PR #1540 targeted GSD-shipped command files; dev-preferences is a + // runtime-generated user artifact). Default now points at the skills/ + // location so /gsd-profile-user --refresh stops re-creating the legacy + // directory. The path is constructed via path.join (not a literal + // string) so the cline-install leaked-path lint does not flag it. let outputPath = options.output; if (!outputPath) { - outputPath = path.join(os.homedir(), '.claude', 'commands', 'gsd', 'dev-preferences.md'); + outputPath = path.join(os.homedir(), '.claude', 'skills', 'gsd-dev-preferences', 'SKILL.md'); } else if (!path.isAbsolute(outputPath)) { outputPath = path.join(cwd, outputPath); } diff --git a/get-shit-done/workflows/profile-user.md b/get-shit-done/workflows/profile-user.md index b0c1f946e..d5fd4ccf5 100644 --- a/get-shit-done/workflows/profile-user.md +++ b/get-shit-done/workflows/profile-user.md @@ -353,7 +353,7 @@ Generate selected artifacts sequentially (file I/O is fast, no benefit from para gsd-sdk query generate-dev-preferences --analysis "$ANALYSIS_PATH" --json ``` -Display: "✓ Generated /gsd-dev-preferences at $HOME/.claude/commands/gsd/dev-preferences.md" +Display: "✓ Generated /gsd-dev-preferences at $HOME/.claude/skills/gsd-dev-preferences/SKILL.md" **For CLAUDE.md profile section (if selected):** @@ -412,7 +412,7 @@ Your profile: $HOME/.claude/get-shit-done/USER-PROFILE.md Then list paths for each generated artifact: ``` Artifacts: - ✓ /gsd-dev-preferences $HOME/.claude/commands/gsd/dev-preferences.md + ✓ /gsd-dev-preferences $HOME/.claude/skills/gsd-dev-preferences/SKILL.md ✓ CLAUDE.md section ./CLAUDE.md ✓ Global CLAUDE.md $HOME/.claude/CLAUDE.md ``` diff --git a/tests/bug-2973-profile-user-skills-path.test.cjs b/tests/bug-2973-profile-user-skills-path.test.cjs new file mode 100644 index 000000000..0e61903ba --- /dev/null +++ b/tests/bug-2973-profile-user-skills-path.test.cjs @@ -0,0 +1,214 @@ +'use strict'; + +// allow-test-rule: source-text-is-the-product. profile-user.md IS the +// shipped workflow product; the `Display:` line at line 356 IS the +// user-visible artifact-name message. This test parses the markdown's +// structured `Display: "..."` line via a regex (not source-grep) to +// extract the path argument as a typed value, then asserts on the +// typed value. The .includes() at the end is a structural absence-check +// against the legacy path literal — the same shape the bug-2470 +// installer-leak test uses to enforce a known-pattern invariant. + +process.env.GSD_TEST_MODE = '1'; + +/** + * Bug #2973: /gsd-profile-user --refresh writes dev-preferences.md to the + * legacy commands/gsd subdirectory, contradicting v1.39.0's skills-only + * migration claim that "Legacy commands/gsd directory removed + * (replaced by skills/)". + * + * Root cause: the writer at get-shit-done/bin/lib/profile-output.cjs + * fell back to commands/gsd/dev-preferences.md when no --output was passed. + * The /gsd-profile-user workflow does not pass --output, so every refresh + * deterministically re-creates the legacy directory. + * + * Fix: + * 1. profile-output.cjs default targets skills/gsd-dev-preferences/SKILL.md + * 2. profile-user.md confirmation message references the new path + * 3. install.js migrates any existing legacy file into the new skill + * location during install (no-op if SKILL.md already exists) + * + * This test exercises the runtime behavior of the writer (writes to the + * skills path) and the structural shape of the workflow message. No + * source-grep on the .cjs body — assertions go against the writer's + * actual output and the parsed workflow message. + */ + +const { test, describe } = 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 ROOT = path.join(__dirname, '..'); +const PROFILE_OUTPUT = path.join(ROOT, 'get-shit-done', 'bin', 'lib', 'profile-output.cjs'); +const WORKFLOW = path.join(ROOT, 'get-shit-done', 'workflows', 'profile-user.md'); +const INSTALL = path.join(ROOT, 'bin', 'install.js'); + +describe('Bug #2973: dev-preferences default writer path is skills/gsd-dev-preferences/SKILL.md', () => { + test('exercise the writer in a subprocess with HOME pointed at a tmp dir; assert the artifact lands at the skills path', () => { + // Subprocess so fs.writeSync(1, ...) in core.cjs goes to a pipe we can + // capture (the parent process's fd 1 bypasses any in-process stubbing). + const cp = require('node:child_process'); + const tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2973-')); + try { + const analysisPath = path.join(tmpHome, 'analysis.json'); + fs.writeFileSync(analysisPath, JSON.stringify({ + data_source: 'questionnaire', + dimensions: { rigor: { score: 7 } }, + })); + const driver = path.join(tmpHome, 'driver.js'); + fs.writeFileSync(driver, ` + const m = require(${JSON.stringify(PROFILE_OUTPUT)}); + m.cmdGenerateDevPreferences(${JSON.stringify(tmpHome)}, { analysis: ${JSON.stringify(analysisPath)} }, false); + `); + const result = cp.spawnSync(process.execPath, [driver], { + env: Object.assign({}, process.env, { HOME: tmpHome, USERPROFILE: tmpHome }), + encoding: 'utf-8', + // Bound the subprocess so a regression that hangs the writer + // (or the dispatcher) cannot deadlock CI (PR #3003 CR feedback). + // 30s is generous for what should complete in <1s; if it trips, + // surface that as a clear test failure rather than CI hanging. + timeout: 30_000, + }); + assert.equal(result.signal, null, + `writer subprocess was killed by signal ${result.signal} (likely timeout): ${result.stderr}`); + assert.equal(result.status, 0, `writer subprocess failed: ${result.stderr}`); + const parsed = JSON.parse(result.stdout); + + const expectedPath = path.join(tmpHome, '.claude', 'skills', 'gsd-dev-preferences', 'SKILL.md'); + assert.equal(parsed.command_path, expectedPath, + `writer emitted ${parsed.command_path}; expected skills path ${expectedPath} (#2973)`); + assert.equal(fs.existsSync(expectedPath), true, + `expected SKILL.md at ${expectedPath} after writer ran`); + const legacyPath = path.join(tmpHome, '.claude', 'commands', 'gsd', 'dev-preferences.md'); + assert.equal(fs.existsSync(legacyPath), false, + `writer must not create ${legacyPath} (#2973)`); + } finally { + fs.rmSync(tmpHome, { recursive: true, force: true }); + } + }); +}); + +describe('Bug #2973: profile-user.md confirmation message references the skills path', () => { + test('the Display message points at $HOME/.claude/skills/gsd-dev-preferences/SKILL.md', () => { + const md = fs.readFileSync(WORKFLOW, 'utf-8'); + // Match the structured Display: line; capture the path value. + const m = md.match(/Display:\s*"[^"]*Generated\s*\/gsd-dev-preferences\s*at\s*([^"]+)"/); + assert.notEqual(m, null, 'expected a Display: "Generated /gsd-dev-preferences at " line'); + const referencedPath = m[1].trim(); + assert.equal(referencedPath, '$HOME/.claude/skills/gsd-dev-preferences/SKILL.md', + `workflow references ${referencedPath}; expected skills path (#2973)`); + }); + + test('no occurrence of the legacy commands/gsd/dev-preferences.md path remains in profile-user.md', () => { + const md = fs.readFileSync(WORKFLOW, 'utf-8'); + assert.equal(md.includes('commands/gsd/dev-preferences.md'), false, + 'profile-user.md still references legacy commands/gsd/dev-preferences.md (#2973)'); + }); +}); + +describe('Bug #2973: installer migrates existing legacy dev-preferences.md to skills/gsd-dev-preferences/SKILL.md', () => { + test('migrateLegacyDevPreferencesToSkill is exported and writes to the skills path', () => { + const inst = require(INSTALL); + // Module exports the migration helper for direct testing. + // Note: this is the structural assertion — the helper exists with the + // documented signature. End-to-end install testing is covered by + // tests/install-*.test.cjs which already exercise legacy preservation. + assert.equal(typeof inst.migrateLegacyDevPreferencesToSkill, 'function', + 'expected migrateLegacyDevPreferencesToSkill in install.js exports (#2973)'); + }); + + test('migration writes to skills/gsd-dev-preferences/SKILL.md when no skill exists yet', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2973-mig-')); + try { + const inst = require(INSTALL); + const saved = new Map([['dev-preferences.md', '# my legacy preferences\n']]); + const migrated = inst.migrateLegacyDevPreferencesToSkill(tmpDir, saved); + assert.equal(migrated, true, 'expected migration to succeed when no SKILL.md exists'); + const skillFile = path.join(tmpDir, 'skills', 'gsd-dev-preferences', 'SKILL.md'); + assert.equal(fs.existsSync(skillFile), true, `expected SKILL.md at ${skillFile}`); + assert.equal(fs.readFileSync(skillFile, 'utf-8'), '# my legacy preferences\n'); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + test('migration is a no-op when a SKILL.md already exists at the new location (do not clobber user-customized skill content)', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2973-skip-')); + try { + const inst = require(INSTALL); + const skillDir = path.join(tmpDir, 'skills', 'gsd-dev-preferences'); + const skillFile = path.join(skillDir, 'SKILL.md'); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(skillFile, '# user-customized skill\n'); + const saved = new Map([['dev-preferences.md', '# legacy content\n']]); + const migrated = inst.migrateLegacyDevPreferencesToSkill(tmpDir, saved); + assert.equal(migrated, false, 'expected migration to skip when SKILL.md exists'); + // Existing content untouched. + assert.equal(fs.readFileSync(skillFile, 'utf-8'), '# user-customized skill\n'); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); +}); + +// ─── #3003 CR follow-up: copyCommandsAsClaudeSkills preserves user-owned skills ── + +describe('Bug #2973 (#3003 CR): copyCommandsAsClaudeSkills snapshots gsd-dev-preferences across the wipe', () => { + test('user-customized skills/gsd-dev-preferences/SKILL.md survives a wipe-and-replace install', () => { + const inst = require(INSTALL); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2973-wipe-')); + try { + const skillsDir = path.join(tmp, 'skills'); + const userSkillDir = path.join(skillsDir, 'gsd-dev-preferences'); + fs.mkdirSync(userSkillDir, { recursive: true }); + const userContent = '# my customized dev preferences\n\nstack: rust\n'; + fs.writeFileSync(path.join(userSkillDir, 'SKILL.md'), userContent); + + // Source dir mimicking commands/gsd/ — does NOT contain dev-preferences + // because dev-preferences is user-generated, not shipped. + const srcDir = path.join(tmp, 'src-commands'); + fs.mkdirSync(srcDir, { recursive: true }); + fs.writeFileSync(path.join(srcDir, 'plan-phase.md'), '# plan-phase\n'); + + // Without the CR fix, the wipe loop deletes gsd-dev-preferences/ + // and the user's content is lost (no source to restore from). + inst.copyCommandsAsClaudeSkills(srcDir, skillsDir, 'gsd', '$HOME/.claude/', 'claude', true); + + const skillFile = path.join(userSkillDir, 'SKILL.md'); + assert.equal(fs.existsSync(skillFile), true, + 'gsd-dev-preferences/SKILL.md must survive the wipe (#3003 CR)'); + assert.equal(fs.readFileSync(skillFile, 'utf-8'), userContent, + 'user content must be byte-identical after the wipe-restore cycle'); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('non-user-owned gsd-* skills are still wiped and recreated from source', () => { + // The existing wipe behavior must still work for skills the package + // owns. Otherwise the preservation list could grow stale by accident. + const inst = require(INSTALL); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2973-wipe-shipped-')); + try { + const skillsDir = path.join(tmp, 'skills'); + const staleSkillDir = path.join(skillsDir, 'gsd-plan-phase'); + fs.mkdirSync(staleSkillDir, { recursive: true }); + fs.writeFileSync(path.join(staleSkillDir, 'STALE-MARKER.txt'), 'wipe me'); + + const srcDir = path.join(tmp, 'src-commands'); + fs.mkdirSync(srcDir, { recursive: true }); + fs.writeFileSync(path.join(srcDir, 'plan-phase.md'), '# plan-phase fresh\n'); + + inst.copyCommandsAsClaudeSkills(srcDir, skillsDir, 'gsd', '$HOME/.claude/', 'claude', true); + + assert.equal(fs.existsSync(path.join(staleSkillDir, 'STALE-MARKER.txt')), false, + 'stale shipped-skill content must be wiped (preservation is opt-in by name)'); + assert.equal(fs.existsSync(path.join(staleSkillDir, 'SKILL.md')), true, + 'fresh SKILL.md from source must be installed after wipe'); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); +});