* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/sturdy-wasps-sprint.md
Normal file
5
.changeset/sturdy-wasps-sprint.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 1453
|
||||
---
|
||||
clean up stale get-shit-done paths in Codex and Kimi skill mirrors on upgrade (#1453)
|
||||
@@ -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)
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in New Issue
Block a user