From e627584f873c734a12ef238e68d2d362c74fa9e3 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 20 Jun 2026 13:37:38 -0400 Subject: [PATCH] fix(#1453): rewrite stale get-shit-done paths in Codex skill mirror on upgrade (#1491) * fix(#1453): clean up stale get-shit-done paths in Codex skill mirror on upgrade Extends planLegacyCleanup in gsd-core/bin/lib/legacy-cleanup.cjs to scan skills/gsd-* subdirectories for .md files that still embed the pre-rename get-shit-done/ path (e.g. ~/.agents/skills/gsd-docs-update/SKILL.md). These stale copies are removed by cleanupLegacyGsdCc during the next install/upgrade so Codex can no longer discover and select them. Adds 5 regression tests to tests/issue-607-legacy-cleanup.test.cjs covering the stale path detection, non-flagging of fresh skills and user-owned dirs, and the end-to-end ~/.agents/skills scenario from the issue. Co-Authored-By: Claude Sonnet 4.6 * fix(#1453): add gsd-allow-legacy-name markers to intentional legacy name references Comments and test descriptions in legacy-cleanup.cjs and its test file legitimately cite the old 'get-shit-done' directory name to explain what the cleanup logic removes. Add the lint-exemption marker to each line so lint-legacy-dir-name passes. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/sturdy-wasps-sprint.md | 5 ++ gsd-core/bin/lib/legacy-cleanup.cjs | 96 ++++++++++++++++++++++ tests/issue-607-legacy-cleanup.test.cjs | 103 +++++++++++++++++++++++- 3 files changed, 201 insertions(+), 3 deletions(-) create mode 100644 .changeset/sturdy-wasps-sprint.md diff --git a/.changeset/sturdy-wasps-sprint.md b/.changeset/sturdy-wasps-sprint.md new file mode 100644 index 000000000..db0c22e18 --- /dev/null +++ b/.changeset/sturdy-wasps-sprint.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1453 +--- +clean up stale get-shit-done paths in Codex and Kimi skill mirrors on upgrade (#1453) diff --git a/gsd-core/bin/lib/legacy-cleanup.cjs b/gsd-core/bin/lib/legacy-cleanup.cjs index 61400a69c..7d51ebf9b 100644 --- a/gsd-core/bin/lib/legacy-cleanup.cjs +++ b/gsd-core/bin/lib/legacy-cleanup.cjs @@ -37,6 +37,33 @@ const OLD_PACKAGE_SIGNAL = 'gsd-core' + '-cc'; */ const GSD_MANAGED_SUBTREES = ['hooks', 'commands']; +/** + * Substring that identifies a skill file as referencing the pre-rename GSD + * runtime config subdirectory. Assembled from parts to avoid self-flagging. + * + * Old installs wrote skill bodies that embed the path to the GSD runtime + * directory — e.g. `@$HOME/.codex/get-shit-done/workflows/plan.md`. After // gsd-allow-legacy-name + * the rename to `gsd-core/` (#604), those embedded paths are stale and the + * skill file must be removed so the runtime does not pick up the wrong copy. + * + * Issue: #1453 + */ +const LEGACY_SKILL_PATH_SIGNAL = 'get-shit-done'; // gsd-allow-legacy-name + +/** + * Prefix that identifies a skill directory as GSD-managed. + * Only `gsd-*` subdirectories under the `skills/` subtree are scanned; user + * skill directories with other prefixes are never touched. + */ +const GSD_SKILL_DIR_PREFIX = 'gsd-'; + +/** + * File extensions eligible for the stale-skill-path scan. + * SKILL.md is the only file in a codex/cursor/kilo/etc skill directory that + * embeds an @-import path to the GSD runtime config tree. + */ +const SKILL_MD_EXTENSIONS = new Set(['.md']); + /** * Extensions eligible for the content-reference scan. * @@ -113,6 +140,24 @@ function fileContainsOldPackageSignal(absPath, fsMod) { } } +/** + * Return true if the file at `absPath` contains the legacy skill path signal + * (`get-shit-done` as a path component inside an @-import or similar reference). // gsd-allow-legacy-name + * Skips unreadable files (returns false on any error). + * + * @param {string} absPath + * @param {object} fsMod + * @returns {boolean} + */ +function fileContainsLegacySkillPathSignal(absPath, fsMod) { + try { + const content = fsMod.readFileSync(absPath, 'utf8'); + return content.includes('/' + LEGACY_SKILL_PATH_SIGNAL + '/'); // gsd-allow-legacy-name + } catch { + return false; + } +} + // ─── Public API ────────────────────────────────────────────────────────────── /** @@ -122,6 +167,9 @@ function fileContainsOldPackageSignal(absPath, fsMod) { * Possible reasons in returned entries: * - 'content-references-old-package': a code file whose content contains * the old package name signal (hooks/ and commands/ subtrees only). + * - 'stale-get-shit-done-path': a skill markdown file whose content contains + * a path reference to the pre-rename `get-shit-done/` runtime directory // gsd-allow-legacy-name + * (skills/ subtree, gsd-* directories only). Issue #1453. * - 'legacy-shared-cache': the old package's shared update-check cache file. * * @param {string[]} configDirs - absolute paths to runtime config dirs to scan @@ -164,6 +212,54 @@ function planLegacyCleanup(configDirs, opts = {}) { } } } + + // #1453: Scan skills/gsd-* directories for stale get-shit-done path references. // gsd-allow-legacy-name + // + // Background: older GSD installs wrote SKILL.md files that embedded a path to + // the GSD runtime config directory, e.g.: + // @$HOME/.codex/get-shit-done/workflows/docs-update.md // gsd-allow-legacy-name + // + // After the rename to gsd-core/ (#604), the correct path is: + // @$HOME/.codex/gsd-core/workflows/docs-update.md + // + // When Codex upgrades to gsd-core 1.5.0 it writes fresh skill files to + // ~/.codex/skills/ but does NOT remove stale copies that an older install + // may have placed under OTHER discoverable skill roots (e.g. ~/.agents/skills/, + // ~/.config/agents/skills/). Codex can pick up either copy and the stale one + // breaks the session (#1453). + // + // This scan removes GSD-managed skill files (under gsd-* subdirs) that still + // reference the old path. Only .md files are scanned (SKILL.md is the sole + // embedded-path carrier in a skill dir). The skills/ dir itself is not deleted; + // user-owned non-gsd-* skill dirs are never touched. + const skillsDir = path.join(configDir, 'skills'); + let skillDirEntries; + try { + skillDirEntries = fsMod.readdirSync(skillsDir, { withFileTypes: true }); + } catch { + skillDirEntries = null; + } + if (skillDirEntries) { + for (const entry of skillDirEntries) { + // Only process gsd-* subdirectories (GSD-managed skill dirs). + if (!entry.isDirectory()) continue; + if (!entry.name.startsWith(GSD_SKILL_DIR_PREFIX)) continue; + + const skillDir = path.join(skillsDir, entry.name); + const files = collectFilesUnder(skillDir, fsMod); + + for (const absPath of files) { + // Never flag user-authored dev-preferences artifacts + if (isDevPreferencesPath(absPath)) continue; + + // Only scan .md files for the stale path signal. + const ext = path.extname(absPath).toLowerCase(); + if (SKILL_MD_EXTENSIONS.has(ext) && fileContainsLegacySkillPathSignal(absPath, fsMod)) { + addCandidate(absPath, 'stale-get-shit-done-path'); // gsd-allow-legacy-name + } + } + } + } } // Legacy shared cache (fixed name from the old package) diff --git a/tests/issue-607-legacy-cleanup.test.cjs b/tests/issue-607-legacy-cleanup.test.cjs index 96154cce4..7a7680ef7 100644 --- a/tests/issue-607-legacy-cleanup.test.cjs +++ b/tests/issue-607-legacy-cleanup.test.cjs @@ -204,21 +204,118 @@ describe('issue-607 legacy-cleanup: planLegacyCleanup', () => { assert.deepEqual(paths, sorted, 'plan must be sorted by path'); }); - // ── only content-references-old-package and legacy-shared-cache reasons ──── + // ── only known reasons ───────────────────────────────────────────────────── - test('plan entries only ever have reason content-references-old-package or legacy-shared-cache', () => { + test('plan entries only ever have known reasons', () => { writeFile(path.join(configDir, 'hooks', 'gsd-worker.js'), '// ' + OLD_PACKAGE_SIGNAL); const cachePath = path.join(homeDir, '.cache', 'gsd', 'gsd-update-check.json'); writeFile(cachePath, '{}'); + // Stale skill file (#1453) + const staleSkillPath = path.join(configDir, 'skills', 'gsd-docs-update', 'SKILL.md'); + writeFile(staleSkillPath, '@$HOME/.codex/' + 'get-shit-done' + '/workflows/docs-update.md\n'); // gsd-allow-legacy-name // User custom hook — should NOT appear writeFile(path.join(configDir, 'hooks', 'gsd-my-custom.js'), '// user hook, clean'); const plan = planLegacyCleanup([configDir], { homeDir }); - const validReasons = new Set(['content-references-old-package', 'legacy-shared-cache']); + const validReasons = new Set(['content-references-old-package', 'legacy-shared-cache', 'stale-get-shit-done-path']); // gsd-allow-legacy-name for (const entry of plan) { assert.ok(validReasons.has(entry.reason), `unexpected reason: ${entry.reason}`); } }); + + // ── #1453 stale skill path in ~/.agents/skills/gsd-* ────────────────────── + + test('#1453: flags a SKILL.md whose content contains a get-shit-done path reference with reason stale-get-shit-done-path', () => { // gsd-allow-legacy-name + // Simulate a stale ~/.agents/skills/gsd-docs-update/SKILL.md left by an + // older GSD install that embedded the pre-rename runtime path. + const staleSkillFile = path.join(configDir, 'skills', 'gsd-docs-update', 'SKILL.md'); + writeFile( + staleSkillFile, + '---\nname: gsd-docs-update\n---\n' + + '@$HOME/.codex/' + 'get-shit-done' + '/workflows/docs-update.md\n' // gsd-allow-legacy-name + ); + + const plan = planLegacyCleanup([configDir], { homeDir }); + + const entry = plan.find((p) => p.path === staleSkillFile); + assert.ok(entry, 'expected stale SKILL.md to appear in plan'); + assert.equal(entry.reason, 'stale-get-shit-done-path'); // gsd-allow-legacy-name + }); + + test('#1453: does NOT flag a SKILL.md whose content contains the new gsd-core path', () => { + // A freshly installed SKILL.md references the new runtime directory name. + const freshSkillFile = path.join(configDir, 'skills', 'gsd-docs-update', 'SKILL.md'); + writeFile( + freshSkillFile, + '---\nname: gsd-docs-update\n---\n' + + '@$HOME/.codex/gsd-core/workflows/docs-update.md\n' + ); + + const plan = planLegacyCleanup([configDir], { homeDir }); + + const entry = plan.find((p) => p.path === freshSkillFile); + assert.equal(entry, undefined, 'fresh SKILL.md (gsd-core path) must NOT appear in plan'); + }); + + test('#1453: does NOT flag SKILL.md files under user-owned (non-gsd-*) skill directories', () => { + // A user-authored skill dir with a custom name must never be touched. + const userSkillFile = path.join(configDir, 'skills', 'my-custom-skill', 'SKILL.md'); + writeFile( + userSkillFile, + '---\nname: my-custom-skill\n---\n' + + 'This skill uses ' + 'get-shit-done' + ' concepts.\n' // gsd-allow-legacy-name + ); + + const plan = planLegacyCleanup([configDir], { homeDir }); + + const entry = plan.find((p) => p.path === userSkillFile); + assert.equal(entry, undefined, 'user-owned (non-gsd-*) SKILL.md must NOT appear in plan'); + }); + + test('#1453: flags SKILL.md with stale path but preserves SKILL.md in the same dir without stale path', () => { + // Two skill dirs: one stale (get-shit-done ref), one fresh (gsd-core ref). // gsd-allow-legacy-name + const staleSkill = path.join(configDir, 'skills', 'gsd-docs-update', 'SKILL.md'); + const freshSkill = path.join(configDir, 'skills', 'gsd-help', 'SKILL.md'); + writeFile(staleSkill, '@$HOME/.codex/' + 'get-shit-done' + '/workflows/docs-update.md\n'); // gsd-allow-legacy-name + writeFile(freshSkill, '@$HOME/.codex/gsd-core/workflows/help.md\n'); + + const plan = planLegacyCleanup([configDir], { homeDir }); + + const staleEntry = plan.find((p) => p.path === staleSkill); + const freshEntry = plan.find((p) => p.path === freshSkill); + + assert.ok(staleEntry, 'stale SKILL.md must appear in plan'); + assert.equal(staleEntry.reason, 'stale-get-shit-done-path'); // gsd-allow-legacy-name + assert.equal(freshEntry, undefined, 'fresh SKILL.md must NOT appear in plan'); + }); + + test('#1453: regression — after upgrade to gsd-core 1.5.0, stale ~/.agents/skills/gsd-docs-update/SKILL.md is removed', () => { + // Simulate the exact scenario from issue #1453: + // ~/.agents/skills/gsd-docs-update/SKILL.md references the old get-shit-done runtime. // gsd-allow-legacy-name + // The upgrade installs correctly to ~/.codex/skills/ but leaves the stale + // ~/.agents/skills/ copy which Codex can still discover. + const agentsDir = path.join(homeDir, '.agents'); + const staleSkill = path.join(agentsDir, 'skills', 'gsd-docs-update', 'SKILL.md'); + writeFile( + staleSkill, + '---\nname: gsd-docs-update\n---\n' + + '@$HOME/.Codex/' + 'get-shit-done' + '/workflows/docs-update.md\n' // gsd-allow-legacy-name + ); + + // planLegacyCleanup receives ~/.agents as one of the configDirs (as _LEGACY_SCAN_SUBDIR_NAMES + // includes '.agents' — the antigravity local form). The plan should flag the stale skill. + const plan = planLegacyCleanup([agentsDir], { homeDir }); + + const entry = plan.find((p) => p.path === staleSkill); + assert.ok(entry, 'stale ~/.agents/skills/gsd-docs-update/SKILL.md must appear in plan'); + assert.equal(entry.reason, 'stale-get-shit-done-path'); // gsd-allow-legacy-name + + // Applying the plan removes the stale file + const result = applyLegacyCleanup(plan); + assert.ok(result.removed.includes(staleSkill), 'stale SKILL.md must appear in removed[]'); + assert.equal(result.errors.length, 0, 'no errors expected'); + assert.equal(require('node:fs').existsSync(staleSkill), false, 'stale SKILL.md must be deleted on disk'); + }); }); describe('issue-607 legacy-cleanup: applyLegacyCleanup', () => {