fix(#1629): cleanup legacy .devin/skills/gsd- dirs on Windsurf reinstall
Pre-#1615 Windsurf installs wrote skills under .devin/skills/gsd-*/ (Devin Desktop preferred dir, #1085). PR #1615 moved Windsurf to .windsurf/workflows/ but never cleaned up the old layout. Users upgrading from a pre-#1615 install were left with dead .devin/skills/gsd-* directories that nothing reads anymore. Fix: added cleanupWindsurfLegacyDevinSkills() which mirrors the Codex cleanupCodexSkillMetadataSidecars() pattern. Runs on Windsurf local install, removes GSD-managed .devin/skills/gsd-* dirs, preserves user content (non-gsd- dirs, gsd-dev-preferences per #2973, symlinks). Empty .devin/ and .devin/skills/ containers are pruned; non-empty ones are left intact. 5 regression tests: removes gsd-* dirs; preserves user content; skips symlinks (escape guard); no-op when absent; end-to-end install removes pre-staged legacy artifacts. Refs #1629 (Finding B; Finding A addressed in #1630).
This commit is contained in:
Tom Boucher
5
.changeset/daring-foxes-greet.md
Normal file
5
.changeset/daring-foxes-greet.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 0
|
||||
---
|
||||
**Windsurf reinstall removes legacy .devin/skills/ artifacts** — pre-#1615 installs wrote skills under .devin/skills/gsd-*/ (Devin Desktop layout, #1085). #1615 moved Windsurf to .windsurf/workflows/ but never cleaned up the old layout. Reinstalls now remove GSD-managed .devin/skills/gsd-* dirs; user-owned content is preserved.
|
||||
@@ -3254,6 +3254,66 @@ function cleanupCodexSkillMetadataSidecars(skillsDir) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove legacy Windsurf skill artifacts from .devin/skills/gsd- directories.
|
||||
*
|
||||
* Pre-#1615 Windsurf installs wrote skills under .devin/ (Devin Desktop
|
||||
* preferred dir, #1085). #1615 moved Windsurf to .windsurf/workflows/.
|
||||
* Old .devin/skills/gsd- dirs linger on disk indefinitely and confuse
|
||||
* users who see two GSD trees.
|
||||
*
|
||||
* Preserves user-owned content:
|
||||
* - non-gsd-* dirs under .devin/skills/ (user-authored skills)
|
||||
* - gsd-dev-preferences/ (user-owned per #2973)
|
||||
* - any files (not dirs) under .devin/skills/
|
||||
*
|
||||
* @param {string} workspaceDir - workspace root (process.cwd() for local installs)
|
||||
* @returns {number} count of removed legacy gsd-* skill directories
|
||||
*/
|
||||
function cleanupWindsurfLegacyDevinSkills(workspaceDir) {
|
||||
const legacySkillsDir = path.join(workspaceDir, '.devin', 'skills');
|
||||
if (!fs.existsSync(legacySkillsDir)) return 0;
|
||||
|
||||
// Mirror the user-owned list from cleanupCodexSkillMetadataSidecars (#2973).
|
||||
const _userOwnedSkillDirs = new Set(['gsd-dev-preferences']);
|
||||
let removed = 0;
|
||||
|
||||
for (const entry of fs.readdirSync(legacySkillsDir, { withFileTypes: true })) {
|
||||
if (!entry.isDirectory() || !entry.name.startsWith('gsd-')) continue;
|
||||
if (_userOwnedSkillDirs.has(entry.name)) continue;
|
||||
|
||||
const dirToRemove = path.join(legacySkillsDir, entry.name);
|
||||
try {
|
||||
// Symlink guard: if the gsd-* dir is itself a symlink pointing outside
|
||||
// the .devin tree, deleting through it could escape the tree. Skip.
|
||||
const stat = fs.lstatSync(dirToRemove);
|
||||
if (stat.isSymbolicLink()) continue;
|
||||
|
||||
fs.rmSync(dirToRemove, { recursive: true, force: true });
|
||||
removed++;
|
||||
} catch (_err) {
|
||||
// Fail open — a single bad dir must not block the install.
|
||||
}
|
||||
}
|
||||
|
||||
// If .devin/skills/ is now empty, prune it. If .devin/ itself is then empty,
|
||||
// prune that too — leaves the workspace clean for the new .windsurf/ layout.
|
||||
// Never remove non-empty containers (user may have other Devin content).
|
||||
try {
|
||||
if (fs.existsSync(legacySkillsDir) && fs.readdirSync(legacySkillsDir).length === 0) {
|
||||
fs.rmdirSync(legacySkillsDir);
|
||||
const devinDir = path.join(workspaceDir, '.devin');
|
||||
if (fs.existsSync(devinDir) && fs.readdirSync(devinDir).length === 0) {
|
||||
fs.rmdirSync(devinDir);
|
||||
}
|
||||
}
|
||||
} catch (_err) {
|
||||
// best-effort container cleanup
|
||||
}
|
||||
|
||||
return removed;
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate the GSD config block for Codex config.toml.
|
||||
* @param {Array<{name: string, description: string}>} agents
|
||||
@@ -9623,6 +9683,17 @@ function install(isGlobal, runtime = 'claude', options = {}) {
|
||||
cleanupCodexSkillMetadataSidecars(path.join(targetDir, 'skills'));
|
||||
}
|
||||
|
||||
// #1629 Finding B: Windsurf local only — remove legacy .devin/skills/gsd-*
|
||||
// dirs from pre-#1615 installs. #1615 moved Windsurf to .windsurf/workflows/
|
||||
// but never cleaned up the old .devin/skills/ layout (#1085). User-owned
|
||||
// content is preserved (non-gsd- dirs, gsd-dev-preferences, symlinks).
|
||||
if (isWindsurf && !isGlobal) {
|
||||
const removedCount = cleanupWindsurfLegacyDevinSkills(process.cwd());
|
||||
if (removedCount > 0) {
|
||||
console.log(` ${green}✓${reset} Removed ${removedCount} legacy .devin/skills/gsd-* dir(s) (pre-#1615 Windsurf layout)`);
|
||||
}
|
||||
}
|
||||
|
||||
// Hermes only: write DESCRIPTION.md for the gsd/ category after layout install
|
||||
if (isHermes) {
|
||||
writeHermesCategoryDescription(path.join(targetDir, 'skills', 'gsd'));
|
||||
@@ -11978,6 +12049,7 @@ module.exports = {
|
||||
convertClaudeAgentToCodexAgent,
|
||||
generateCodexAgentToml,
|
||||
cleanupCodexSkillMetadataSidecars,
|
||||
cleanupWindsurfLegacyDevinSkills,
|
||||
generateCodexConfigBlock,
|
||||
stripGsdFromCodexConfig,
|
||||
migrateCodexHooksMapFormat,
|
||||
|
||||
@@ -1416,6 +1416,130 @@ describe('windsurf local install writes workflow slash commands (#1615)', () =>
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #1629 Finding B: legacy .devin/skills/gsd-* cleanup on Windsurf reinstall ─
|
||||
describe('cleanupWindsurfLegacyDevinSkills — removes pre-#1615 skill artifacts (#1629)', () => {
|
||||
const { cleanupWindsurfLegacyDevinSkills } = require('../bin/install.js');
|
||||
|
||||
test('removes GSD-managed gsd-* dirs under .devin/skills/', (t) => {
|
||||
const tmpDir = createTempDir('gsd-1629b-cleanup-');
|
||||
t.after(() => cleanup(tmpDir));
|
||||
|
||||
// Stage legacy .devin/skills/gsd-*/ artifacts (pre-#1615 layout)
|
||||
const legacySkillsDir = path.join(tmpDir, '.devin', 'skills');
|
||||
for (const skill of ['gsd-help', 'gsd-plan-phase', 'gsd-ship']) {
|
||||
const skillDir = path.join(legacySkillsDir, skill);
|
||||
fs.mkdirSync(skillDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(skillDir, 'SKILL.md'), '# Legacy skill\n');
|
||||
}
|
||||
|
||||
const removed = cleanupWindsurfLegacyDevinSkills(tmpDir);
|
||||
|
||||
assert.strictEqual(removed, 3, 'should remove exactly 3 gsd-* dirs');
|
||||
for (const skill of ['gsd-help', 'gsd-plan-phase', 'gsd-ship']) {
|
||||
assert.ok(
|
||||
!fs.existsSync(path.join(legacySkillsDir, skill)),
|
||||
`${skill} should be removed from .devin/skills/`,
|
||||
);
|
||||
}
|
||||
// Empty container dirs should also be pruned
|
||||
assert.ok(!fs.existsSync(legacySkillsDir), '.devin/skills/ should be pruned when empty');
|
||||
assert.ok(!fs.existsSync(path.join(tmpDir, '.devin')), '.devin/ should be pruned when empty');
|
||||
});
|
||||
|
||||
test('preserves user-owned non-gsd- content under .devin/skills/', (t) => {
|
||||
const tmpDir = createTempDir('gsd-1629b-preserve-');
|
||||
t.after(() => cleanup(tmpDir));
|
||||
|
||||
const legacySkillsDir = path.join(tmpDir, '.devin', 'skills');
|
||||
// Stage mixed content: legacy GSD + user-authored + user-owned gsd-dev-preferences
|
||||
fs.mkdirSync(path.join(legacySkillsDir, 'gsd-help'), { recursive: true });
|
||||
fs.writeFileSync(path.join(legacySkillsDir, 'gsd-help', 'SKILL.md'), '# legacy\n');
|
||||
fs.mkdirSync(path.join(legacySkillsDir, 'my-custom-skill'), { recursive: true });
|
||||
fs.writeFileSync(path.join(legacySkillsDir, 'my-custom-skill', 'SKILL.md'), '# user\n');
|
||||
fs.mkdirSync(path.join(legacySkillsDir, 'gsd-dev-preferences'), { recursive: true });
|
||||
fs.writeFileSync(path.join(legacySkillsDir, 'gsd-dev-preferences', 'SKILL.md'), '# prefs\n');
|
||||
|
||||
const removed = cleanupWindsurfLegacyDevinSkills(tmpDir);
|
||||
|
||||
assert.strictEqual(removed, 1, 'only gsd-help should be removed (gsd-dev-preferences is user-owned)');
|
||||
assert.ok(!fs.existsSync(path.join(legacySkillsDir, 'gsd-help')), 'legacy gsd-help removed');
|
||||
assert.ok(
|
||||
fs.existsSync(path.join(legacySkillsDir, 'my-custom-skill')),
|
||||
'user-authored my-custom-skill must be preserved',
|
||||
);
|
||||
assert.ok(
|
||||
fs.existsSync(path.join(legacySkillsDir, 'gsd-dev-preferences')),
|
||||
'user-owned gsd-dev-preferences must be preserved (#2973)',
|
||||
);
|
||||
// Container NOT pruned because it still has user content
|
||||
assert.ok(fs.existsSync(legacySkillsDir), '.devin/skills/ preserved when user content remains');
|
||||
assert.ok(fs.existsSync(path.join(tmpDir, '.devin')), '.devin/ preserved when user content remains');
|
||||
});
|
||||
|
||||
test('skips symlinks pointing outside the .devin tree (escape guard)', (t) => {
|
||||
const tmpDir = createTempDir('gsd-1629b-symlink-');
|
||||
t.after(() => cleanup(tmpDir));
|
||||
|
||||
const legacySkillsDir = path.join(tmpDir, '.devin', 'skills');
|
||||
fs.mkdirSync(legacySkillsDir, { recursive: true });
|
||||
// Create a symlink that points outside the tree
|
||||
const outsideTarget = path.join(tmpDir, 'secret');
|
||||
fs.mkdirSync(outsideTarget);
|
||||
fs.writeFileSync(path.join(outsideTarget, 'secret.txt'), 'secret\n');
|
||||
fs.symlinkSync(outsideTarget, path.join(legacySkillsDir, 'gsd-symlinked'));
|
||||
|
||||
const removed = cleanupWindsurfLegacyDevinSkills(tmpDir);
|
||||
|
||||
assert.strictEqual(removed, 0, 'symlinked gsd-* dir must not be removed');
|
||||
assert.ok(
|
||||
fs.existsSync(path.join(legacySkillsDir, 'gsd-symlinked')),
|
||||
'symlink must be preserved (escape guard)',
|
||||
);
|
||||
assert.ok(
|
||||
fs.existsSync(path.join(outsideTarget, 'secret.txt')),
|
||||
'out-of-tree target must not be touched',
|
||||
);
|
||||
});
|
||||
|
||||
test('no-op when .devin/skills/ does not exist', () => {
|
||||
const tmpDir = createTempDir('gsd-1629b-noop-');
|
||||
const removed = cleanupWindsurfLegacyDevinSkills(tmpDir);
|
||||
assert.strictEqual(removed, 0, 'should return 0 when .devin/skills/ is absent');
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('install(false, "windsurf") removes pre-existing .devin/skills/gsd-* on reinstall', (t) => {
|
||||
// End-to-end: stage legacy artifacts, run a fresh Windsurf install,
|
||||
// verify the old layout is cleaned up while the new .windsurf/ layout is written.
|
||||
const tmpDir = createTempDir('gsd-1629b-e2e-');
|
||||
t.after(() => cleanup(tmpDir));
|
||||
const previousCwd = process.cwd();
|
||||
process.chdir(tmpDir);
|
||||
try {
|
||||
// Stage pre-#1615 artifacts
|
||||
const legacyDir = path.join(tmpDir, '.devin', 'skills', 'gsd-help');
|
||||
fs.mkdirSync(legacyDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(legacyDir, 'SKILL.md'), '# legacy\n');
|
||||
|
||||
// Fresh Windsurf install
|
||||
install(false, 'windsurf');
|
||||
|
||||
// Legacy layout should be cleaned up
|
||||
assert.ok(
|
||||
!fs.existsSync(path.join(tmpDir, '.devin', 'skills', 'gsd-help')),
|
||||
'pre-existing .devin/skills/gsd-help should be removed by fresh windsurf install',
|
||||
);
|
||||
// New layout should be present
|
||||
assert.ok(
|
||||
fs.existsSync(path.join(tmpDir, '.windsurf', 'workflows')),
|
||||
'.windsurf/workflows/ should exist after fresh install',
|
||||
);
|
||||
} finally {
|
||||
process.chdir(previousCwd);
|
||||
}
|
||||
});
|
||||
});
|
||||
// ─── Section N+1: #767 — disallowedTools injection for read-only agents ──────
|
||||
//
|
||||
// Verifies (installer-behavioral test — drives install() to a temp dir):
|
||||
|
||||
Reference in New Issue
Block a user