diff --git a/.changeset/daring-foxes-greet.md b/.changeset/daring-foxes-greet.md new file mode 100644 index 000000000..b235dc1f9 --- /dev/null +++ b/.changeset/daring-foxes-greet.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1631 +--- +**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. diff --git a/bin/install.js b/bin/install.js index 6c0a018fe..f3da4b1da 100755 --- a/bin/install.js +++ b/bin/install.js @@ -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, diff --git a/tests/install.test.cjs b/tests/install.test.cjs index 2c24b8cdd..66adf95bc 100644 --- a/tests/install.test.cjs +++ b/tests/install.test.cjs @@ -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):