diff --git a/.changeset/legacy-cleanup-keeps-user-files.md b/.changeset/legacy-cleanup-keeps-user-files.md new file mode 100644 index 0000000000..3f6ccdd7e9 --- /dev/null +++ b/.changeset/legacy-cleanup-keeps-user-files.md @@ -0,0 +1,5 @@ +--- +'@fission-ai/openspec': patch +--- + +Stop legacy cleanup deleting the user's own files. The six pre-skills tools that kept their commands in a `/commands/openspec/` folder (Claude Code, CodeBuddy, Qoder, Lingma, Crush and Gemini CLI) had that whole folder removed recursively whenever it existed, so a command the user kept there, such as a team review checklist, was deleted along with OpenSpec's files, and the summary named only the folder. Because `openspec init` cleans up automatically when there is no TTY, an agent or CI running plain `openspec init` did this without `--force` and without a prompt, and `openspec update --force` did the same. Cleanup now deletes only the files OpenSpec wrote there: `proposal`, `apply` and `archive` files that still carry the OpenSpec markers every legacy command was generated with, so a same-named file the user wrote is kept. It never follows a symlinked command folder, removes the folder only once nothing else is left in it, and lists each thing it kept. A folder holding nothing OpenSpec wrote is no longer reported as legacy at all. A folder holding only OpenSpec's files, or nothing, is still removed exactly as before, with the same summary line. diff --git a/src/core/legacy-cleanup.ts b/src/core/legacy-cleanup.ts index 978ec4963f..66835c6a2d 100644 --- a/src/core/legacy-cleanup.ts +++ b/src/core/legacy-cleanup.ts @@ -26,19 +26,27 @@ export const LEGACY_CONFIG_FILES = [ 'QWEN.md', ] as const; +/** The three commands the old SlashCommandRegistry wrote into each directory. */ +const LEGACY_DIRECTORY_COMMAND_FILES = ['proposal.md', 'apply.md', 'archive.md'] as const; + /** * Legacy slash command patterns from the old SlashCommandRegistry. * These map toolId to the path pattern where legacy commands were created. * Some tools used a directory structure, others used individual files. */ export const LEGACY_SLASH_COMMAND_PATHS: Record = { - // Directory-based: .tooldir/commands/openspec/ or .tooldir/commands/openspec/*.md - 'claude': { type: 'directory', path: '.claude/commands/openspec' }, - 'codebuddy': { type: 'directory', path: '.codebuddy/commands/openspec' }, - 'qoder': { type: 'directory', path: '.qoder/commands/openspec' }, - 'lingma': { type: 'directory', path: '.lingma/commands/openspec' }, - 'crush': { type: 'directory', path: '.crush/commands/openspec' }, - 'gemini': { type: 'directory', path: '.gemini/commands/openspec' }, + // Directory-based: .tooldir/commands/openspec/. Each entry names the files + // OpenSpec wrote there, because users keep their own commands in the same + // folder: only those files are deleted, and the folder only once it is empty. + 'claude': { type: 'directory', path: '.claude/commands/openspec', managedFileNames: LEGACY_DIRECTORY_COMMAND_FILES }, + 'codebuddy': { type: 'directory', path: '.codebuddy/commands/openspec', managedFileNames: LEGACY_DIRECTORY_COMMAND_FILES }, + 'qoder': { type: 'directory', path: '.qoder/commands/openspec', managedFileNames: LEGACY_DIRECTORY_COMMAND_FILES }, + // Lingma support arrived after the opsx rename and has always written to + // `.lingma/commands/opsx/`, so OpenSpec never put a file here: only an empty + // leftover folder is removed. + 'lingma': { type: 'directory', path: '.lingma/commands/openspec', managedFileNames: [] }, + 'crush': { type: 'directory', path: '.crush/commands/openspec', managedFileNames: LEGACY_DIRECTORY_COMMAND_FILES }, + 'gemini': { type: 'directory', path: '.gemini/commands/openspec', managedFileNames: ['proposal.toml', 'apply.toml', 'archive.toml'] }, // File-based: individual openspec-*.md files in a commands/workflows/prompts folder 'cursor': { type: 'files', pattern: '.cursor/commands/openspec-*.md' }, @@ -110,6 +118,8 @@ export const LEGACY_GLOBAL_SLASH_COMMAND_PATHS: Record `${pattern.path}/${name}`)); } } else if (pattern.type === 'files' && pattern.pattern) { const patterns = Array.isArray(pattern.pattern) ? pattern.pattern : [pattern.pattern]; @@ -335,6 +357,102 @@ export async function detectLegacySlashCommands( return { directories, files }; } +/** + * Splits a legacy command directory's entries into the files OpenSpec wrote + * there and everything else, sorted. A file counts as OpenSpec's only when it + * is a regular file with a managed name whose content still carries the + * OpenSpec markers every legacy command was written with; a folder, a link, or + * a same-named file the user wrote is the user's. Subdirectories are listed + * with a trailing '/'. Returns undefined when the directory cannot be read or + * is itself a symlink, which is never followed. + */ +async function readLegacyCommandDir( + dirPath: string, + managedFileNames: readonly string[] +): Promise<{ managed: string[]; others: string[] } | undefined> { + let entries; + try { + if ((await fs.lstat(dirPath)).isSymbolicLink()) { + return undefined; + } + entries = await fs.readdir(dirPath, { withFileTypes: true }); + } catch { + return undefined; + } + + const managed: string[] = []; + const others: string[] = []; + for (const entry of entries) { + if ( + entry.isFile() && + managedFileNames.includes(entry.name) && + (await isGeneratedLegacyCommand(path.join(dirPath, entry.name))) + ) { + managed.push(entry.name); + } else { + others.push(entry.isDirectory() ? `${entry.name}/` : entry.name); + } + } + return { managed: managed.sort(), others: others.sort() }; +} + +/** + * The legacy command directory, and its tool, that a repo-local path is one of + * OpenSpec's own files in. + */ +function legacyCommandDirForFile(file: string): { toolId: string; dir: string } | undefined { + const normalizedFile = normalizePathForMatch(file); + for (const [toolId, pattern] of Object.entries(LEGACY_SLASH_COMMAND_PATHS)) { + if (pattern.type !== 'directory' || !pattern.path) continue; + const dir = pattern.path; + if (pattern.managedFileNames?.some((name) => normalizedFile === `${dir}/${name}`)) { + return { toolId, dir }; + } + } + return undefined; +} + +/** + * Removes a legacy command directory once OpenSpec's files are gone from it, + * or records what is left in it as kept. Never recursive: whatever remains was + * not written by OpenSpec. Returns true when the directory was removed. + */ +async function settleLegacyCommandDir( + projectPath: string, + dirPath: string, + result: CleanupResult +): Promise { + const fullPath = FileSystemUtils.joinPath(projectPath, dirPath); + const remaining = await readLegacyCommandDir(fullPath, []); + if (!remaining) { + return false; + } + if (remaining.others.length === 0) { + await fs.rmdir(fullPath); + result.deletedDirs.push(dirPath); + return true; + } + result.keptFiles!.push(...remaining.others.map((name) => `${dirPath}/${name}`)); + return false; +} + +/** + * Whether a file is a legacy command OpenSpec generated: a regular file (not a + * link) whose content carries the OpenSpec markers. Every legacy slash command + * was written with them, and OpenSpec refused to update one that lost them, so + * a same-named file without them is the user's. + */ +async function isGeneratedLegacyCommand(filePath: string): Promise { + try { + if (!(await fs.lstat(filePath)).isFile()) { + return false; + } + return hasOpenSpecMarkers(await fs.readFile(filePath, 'utf-8')); + } catch { + return false; + } +} + /** * Detects legacy global slash command files. * @@ -506,6 +624,8 @@ export interface CleanupResult { modifiedFiles: string[]; /** Directories that were deleted */ deletedDirs: string[]; + /** Entries left in a legacy command directory because OpenSpec did not write them */ + keptFiles?: string[]; /** Whether project.md exists and needs manual migration */ projectMdNeedsMigration: boolean; /** Error messages if any operations failed */ @@ -529,6 +649,7 @@ export async function cleanupLegacyArtifacts( deletedFileReplacementLabels: {}, modifiedFiles: [], deletedDirs: [], + keptFiles: [], projectMdNeedsMigration: detection.hasProjectMd, errors: [], }; @@ -548,21 +669,50 @@ export async function cleanupLegacyArtifacts( } } - // Delete legacy slash command directories (these are 100% OpenSpec-managed) + // Delete legacy slash command directories: only the files OpenSpec wrote, + // then the directory once it is empty. Detection reports a directory only + // when it holds nothing else, but a file the user added since is still kept. for (const dirPath of detection.slashCommandDirs) { const fullPath = FileSystemUtils.joinPath(projectPath, dirPath); try { - await fs.rm(fullPath, { recursive: true, force: true }); - result.deletedDirs.push(dirPath); + const managedFileNames = legacyManagedFileNamesForDir(dirPath); + const entries = await readLegacyCommandDir(fullPath, managedFileNames); + if (!entries) { + continue; + } + const deleted: string[] = []; + for (const name of entries.managed) { + const filePath = path.join(fullPath, name); + // Check again just before deleting: the file may have been replaced + // with the user's own since the scan. A kept file is reported below. + if (!(await isGeneratedLegacyCommand(filePath))) { + continue; + } + await fs.unlink(filePath); + deleted.push(name); + } + if (!(await settleLegacyCommandDir(projectPath, dirPath, result))) { + result.deletedFiles.push(...deleted.map((name) => `${dirPath}/${name}`)); + } } catch (error: any) { result.errors.push(`Failed to delete directory ${dirPath}: ${error.message}`); } } // Delete legacy slash command files (these are 100% OpenSpec-managed) + const partlyCleanedDirs = new Set(); for (const filePath of detection.slashCommandFiles) { const fullPath = FileSystemUtils.joinPath(projectPath, filePath); try { + const commandDir = legacyCommandDirForFile(filePath); + if (commandDir) { + partlyCleanedDirs.add(commandDir.dir); + // Check again just before deleting: the file may have been replaced + // with the user's own since detection. A kept file is reported below. + if (!(await isGeneratedLegacyCommand(fullPath))) { + continue; + } + } await fs.unlink(fullPath); result.deletedFiles.push(filePath); } catch (error: any) { @@ -570,6 +720,16 @@ export async function cleanupLegacyArtifacts( } } + // A legacy command directory that also held the user's files was cleaned + // file by file above; record what was left in it. + for (const dirPath of partlyCleanedDirs) { + try { + await settleLegacyCommandDir(projectPath, dirPath, result); + } catch (error: any) { + result.errors.push(`Failed to delete directory ${dirPath}: ${error.message}`); + } + } + // Delete managed global slash command files (these are 100% OpenSpec-managed) const globalPromptMatchesByPath = new Map( getLegacyGlobalPromptMatches(detection).map((prompt) => [prompt.path, prompt] as const) @@ -621,7 +781,14 @@ export async function cleanupLegacyArtifacts( export function formatCleanupSummary(result: CleanupResult): string { const lines: string[] = []; - if (result.deletedFiles.length > 0 || result.deletedDirs.length > 0 || result.modifiedFiles.length > 0) { + const keptFiles = result.keptFiles ?? []; + + if ( + result.deletedFiles.length > 0 || + result.deletedDirs.length > 0 || + result.modifiedFiles.length > 0 || + keptFiles.length > 0 + ) { lines.push('Cleaned up legacy files:'); for (const file of result.deletedFiles) { @@ -637,6 +804,10 @@ export function formatCleanupSummary(result: CleanupResult): string { lines.push(` ✓ Removed ${dir}/ (replaced by OpenSpec skills and commands)`); } + for (const entry of keptFiles) { + lines.push(` • Kept ${entry} (not created by OpenSpec)`); + } + for (const file of result.modifiedFiles) { lines.push(` ✓ Removed OpenSpec markers from ${file}`); } @@ -832,8 +1003,24 @@ function legacyToolIdForDir(dir: string): string | undefined { return undefined; } +/** The files OpenSpec wrote into a repo-local legacy slash-command directory. */ +function legacyManagedFileNamesForDir(dir: string): readonly string[] { + const normalizedDir = normalizePathForMatch(dir); + for (const pattern of Object.values(LEGACY_SLASH_COMMAND_PATHS)) { + if (pattern.type === 'directory' && pattern.path === normalizedDir) { + return pattern.managedFileNames ?? []; + } + } + return []; +} + /** The tool that owns a repo-local legacy slash-command file, if any. */ function legacyToolIdForFile(file: string): string | undefined { + // A file from a directory-based tool, reported because the directory also + // holds the user's own files. + const commandDir = legacyCommandDirForFile(file); + if (commandDir) return commandDir.toolId; + // Normalize to forward slashes so the glob patterns match on Windows too. const normalizedFile = normalizePathForMatch(file); for (const [toolId, pattern] of Object.entries(LEGACY_SLASH_COMMAND_PATHS)) { diff --git a/test/core/legacy-cleanup.test.ts b/test/core/legacy-cleanup.test.ts index f357052649..16f18f3e8b 100644 --- a/test/core/legacy-cleanup.test.ts +++ b/test/core/legacy-cleanup.test.ts @@ -270,7 +270,7 @@ ${OPENSPEC_MARKERS.end}`); it('should detect legacy Claude slash command directory', async () => { const dirPath = path.join(testDir, '.claude', 'commands', 'openspec'); await fs.mkdir(dirPath, { recursive: true }); - await fs.writeFile(path.join(dirPath, 'proposal.md'), 'content'); + await fs.writeFile(path.join(dirPath, 'proposal.md'), '\ncontent\n\n'); const result = await detectLegacySlashCommands(testDir); expect(result.directories).toContain('.claude/commands/openspec'); @@ -612,7 +612,7 @@ ${OPENSPEC_MARKERS.end}`); it('should delete legacy slash command directories', async () => { const dirPath = path.join(testDir, '.claude', 'commands', 'openspec'); await fs.mkdir(dirPath, { recursive: true }); - await fs.writeFile(path.join(dirPath, 'proposal.md'), 'content'); + await fs.writeFile(path.join(dirPath, 'proposal.md'), '\ncontent\n\n'); const detection = await detectLegacyArtifacts(testDir); const result = await cleanupLegacyArtifacts(testDir, detection); @@ -1162,6 +1162,7 @@ ${OPENSPEC_MARKERS.end}`); expect(LEGACY_SLASH_COMMAND_PATHS['claude']).toEqual({ type: 'directory', path: '.claude/commands/openspec', + managedFileNames: ['proposal.md', 'apply.md', 'archive.md'], }); expect(LEGACY_SLASH_COMMAND_PATHS['cursor']).toEqual({ diff --git a/test/core/legacy-cleanup.user-files.test.ts b/test/core/legacy-cleanup.user-files.test.ts new file mode 100644 index 0000000000..5f3520688d --- /dev/null +++ b/test/core/legacy-cleanup.user-files.test.ts @@ -0,0 +1,374 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import os from 'os'; +import { + cleanupLegacyArtifacts, + detectLegacyArtifacts, + formatCleanupSummary, + formatDetectionSummary, + getToolsFromLegacyArtifacts, + omitToolLegacyArtifacts, + LEGACY_SLASH_COMMAND_PATHS, +} from '../../src/core/legacy-cleanup.js'; +import { OPENSPEC_MARKERS } from '../../src/core/config.js'; +import { runCLI } from '../helpers/run-cli.js'; + +/** + * Pre-opsx tools kept their commands in a `/commands/openspec/` folder, + * and users keep their own commands in that same folder. Cleanup may delete + * only the files OpenSpec wrote there, and the folder only once nothing else + * is left in it. + */ + +// The files the old SlashCommandRegistry wrote, per directory-based tool. +const LEGACY_COMMAND_DIRS = [ + { toolId: 'claude', dir: '.claude/commands/openspec', files: ['apply.md', 'archive.md', 'proposal.md'], userFile: 'team-review.md' }, + { toolId: 'codebuddy', dir: '.codebuddy/commands/openspec', files: ['apply.md', 'archive.md', 'proposal.md'], userFile: 'team-review.md' }, + { toolId: 'qoder', dir: '.qoder/commands/openspec', files: ['apply.md', 'archive.md', 'proposal.md'], userFile: 'team-review.md' }, + { toolId: 'crush', dir: '.crush/commands/openspec', files: ['apply.md', 'archive.md', 'proposal.md'], userFile: 'team-review.md' }, + { toolId: 'gemini', dir: '.gemini/commands/openspec', files: ['apply.toml', 'archive.toml', 'proposal.toml'], userFile: 'team-review.toml' }, +]; + +const CLAUDE_DIR = '.claude/commands/openspec'; +const CLAUDE_FILES = ['apply.md', 'archive.md', 'proposal.md']; + +describe('legacy command directories and the files users keep in them', () => { + let testDir: string; + let originalEnv: NodeJS.ProcessEnv; + + beforeEach(async () => { + originalEnv = { ...process.env }; + testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-legacy-user-files-')); + process.env.CODEX_HOME = path.join(testDir, 'codex-home'); + await fs.mkdir(path.join(testDir, 'openspec'), { recursive: true }); + }); + + afterEach(async () => { + process.env = originalEnv; + await fs.rm(testDir, { recursive: true, force: true }); + }); + + const inProject = (dir: string, ...names: string[]) => path.join(testDir, dir, ...names); + const exists = (filePath: string) => fs.access(filePath).then(() => true, () => false); + + // A file named like a legacy command gets the markers every legacy command + // was written with; any other file is plain user content. + const generatedContent = (name: string) => `${OPENSPEC_MARKERS.start}\ncontent of ${name}\n${OPENSPEC_MARKERS.end}\n`; + const isCommandName = (name: string) => /^(proposal|apply|archive)\.(md|toml)$/.test(name); + + async function writeFiles(dir: string, names: readonly string[]): Promise { + for (const name of names) { + const filePath = inProject(dir, name); + await fs.mkdir(path.dirname(filePath), { recursive: true }); + await fs.writeFile(filePath, isCommandName(name) ? generatedContent(name) : `content of ${name}`); + } + } + + it('covers every directory-based legacy entry', () => { + const directoryTools = Object.entries(LEGACY_SLASH_COMMAND_PATHS) + .filter(([, pattern]) => pattern.type === 'directory') + .map(([toolId]) => toolId) + .sort(); + expect(directoryTools).toEqual([...LEGACY_COMMAND_DIRS.map((entry) => entry.toolId), 'lingma'].sort()); + }); + + describe.each(LEGACY_COMMAND_DIRS)('$toolId', ({ toolId, dir, files, userFile }) => { + it('removes the folder when it holds only OpenSpec files', async () => { + await writeFiles(dir, files); + + const detection = await detectLegacyArtifacts(testDir); + expect(detection.slashCommandDirs).toContain(dir); + const result = await cleanupLegacyArtifacts(testDir, detection); + + expect(result.deletedDirs).toContain(dir); + expect(await exists(inProject(dir))).toBe(false); + }); + + it('keeps a user file and removes only the OpenSpec files', async () => { + await writeFiles(dir, [...files, userFile]); + + const detection = await detectLegacyArtifacts(testDir); + const result = await cleanupLegacyArtifacts(testDir, detection); + + expect(await fs.readFile(inProject(dir, userFile), 'utf-8')).toBe(`content of ${userFile}`); + for (const name of files) { + expect(await exists(inProject(dir, name))).toBe(false); + } + expect(result.deletedDirs).not.toContain(dir); + expect(result.deletedFiles).toEqual(expect.arrayContaining(files.map((name) => `${dir}/${name}`))); + expect(result.keptFiles).toEqual([`${dir}/${userFile}`]); + expect(getToolsFromLegacyArtifacts(detection)).toContain(toolId); + }); + }); + + it('keeps a nested folder of user commands', async () => { + await writeFiles(CLAUDE_DIR, [...CLAUDE_FILES, path.join('team', 'review.md')]); + + const result = await cleanupLegacyArtifacts(testDir, await detectLegacyArtifacts(testDir)); + + expect(await fs.readFile(inProject(CLAUDE_DIR, 'team', 'review.md'), 'utf-8')).toBe( + `content of ${path.join('team', 'review.md')}` + ); + expect(result.keptFiles).toEqual([`${CLAUDE_DIR}/team/`]); + expect(result.deletedDirs).not.toContain(CLAUDE_DIR); + }); + + it('treats a folder that carries a legacy file name as the user\'s', async () => { + await writeFiles(CLAUDE_DIR, ['proposal.md', path.join('apply.md', 'notes.md')]); + + const result = await cleanupLegacyArtifacts(testDir, await detectLegacyArtifacts(testDir)); + + expect(await exists(inProject(CLAUDE_DIR, 'apply.md', 'notes.md'))).toBe(true); + expect(await exists(inProject(CLAUDE_DIR, 'proposal.md'))).toBe(false); + expect(result.keptFiles).toEqual([`${CLAUDE_DIR}/apply.md/`]); + }); + + it('keeps a Gemini markdown file that shares a legacy command name', async () => { + const dir = '.gemini/commands/openspec'; + await writeFiles(dir, ['proposal.toml', 'proposal.md']); + + const result = await cleanupLegacyArtifacts(testDir, await detectLegacyArtifacts(testDir)); + + expect(await exists(inProject(dir, 'proposal.md'))).toBe(true); + expect(await exists(inProject(dir, 'proposal.toml'))).toBe(false); + expect(result.keptFiles).toEqual([`${dir}/proposal.md`]); + }); + + it('neither reports nor touches a folder holding no OpenSpec files', async () => { + await writeFiles(CLAUDE_DIR, ['team-review.md']); + + const detection = await detectLegacyArtifacts(testDir); + expect(detection.slashCommandDirs).not.toContain(CLAUDE_DIR); + expect(detection.slashCommandFiles.filter((file) => file.startsWith(CLAUDE_DIR))).toEqual([]); + expect(detection.hasLegacyArtifacts).toBe(false); + + await cleanupLegacyArtifacts(testDir, detection); + expect(await exists(inProject(CLAUDE_DIR, 'team-review.md'))).toBe(true); + }); + + it('leaves files in the Lingma folder alone, because OpenSpec never wrote there', async () => { + const dir = '.lingma/commands/openspec'; + await writeFiles(dir, ['proposal.md']); + + const detection = await detectLegacyArtifacts(testDir); + expect(detection.slashCommandDirs).not.toContain(dir); + await cleanupLegacyArtifacts(testDir, detection); + + expect(await exists(inProject(dir, 'proposal.md'))).toBe(true); + }); + + it('still removes an empty leftover legacy folder', async () => { + const dir = '.lingma/commands/openspec'; + await fs.mkdir(inProject(dir), { recursive: true }); + + const detection = await detectLegacyArtifacts(testDir); + expect(detection.slashCommandDirs).toContain(dir); + const result = await cleanupLegacyArtifacts(testDir, detection); + + expect(result.deletedDirs).toContain(dir); + expect(await exists(inProject(dir))).toBe(false); + }); + + it('keeps a file added between detection and cleanup', async () => { + await writeFiles(CLAUDE_DIR, CLAUDE_FILES); + const detection = await detectLegacyArtifacts(testDir); + expect(detection.slashCommandDirs).toContain(CLAUDE_DIR); + + // e.g. while the interactive upgrade prompt was waiting + await writeFiles(CLAUDE_DIR, ['team-review.md']); + const result = await cleanupLegacyArtifacts(testDir, detection); + + expect(await exists(inProject(CLAUDE_DIR, 'team-review.md'))).toBe(true); + expect(result.deletedDirs).not.toContain(CLAUDE_DIR); + expect(result.keptFiles).toEqual([`${CLAUDE_DIR}/team-review.md`]); + }); + + it('lists OpenSpec\'s files, not the folder, in the upgrade prompt when the folder holds user files', async () => { + await writeFiles(CLAUDE_DIR, [...CLAUDE_FILES, 'team-review.md']); + + const summary = formatDetectionSummary(await detectLegacyArtifacts(testDir)); + const lines = summary.split('\n').map((line) => line.trim()); + + expect(lines).toContain(`• ${CLAUDE_DIR}/proposal.md`); + expect(lines).not.toContain(`• ${CLAUDE_DIR}/`); + expect(summary).not.toContain('team-review.md'); + }); + + it('names what was kept in the cleanup summary and never claims the folder was removed', async () => { + await writeFiles(CLAUDE_DIR, [...CLAUDE_FILES, 'team-review.md']); + + const result = await cleanupLegacyArtifacts(testDir, await detectLegacyArtifacts(testDir)); + const summary = formatCleanupSummary(result); + + expect(summary).toContain(`Kept ${CLAUDE_DIR}/team-review.md (not created by OpenSpec)`); + expect(summary).toContain(`Removed ${CLAUDE_DIR}/proposal.md`); + expect(summary).not.toContain(`Removed ${CLAUDE_DIR}/ `); + }); + + it('leaves a mixed folder untouched when its tool is omitted from cleanup', async () => { + await writeFiles(CLAUDE_DIR, [...CLAUDE_FILES, 'team-review.md']); + + const detection = omitToolLegacyArtifacts(await detectLegacyArtifacts(testDir), ['claude']); + expect(detection.slashCommandFiles.filter((file) => file.startsWith(CLAUDE_DIR))).toEqual([]); + await cleanupLegacyArtifacts(testDir, detection); + + expect(await exists(inProject(CLAUDE_DIR, 'proposal.md'))).toBe(true); + }); + + it('keeps a user-authored file that only shares a legacy command name', async () => { + await fs.mkdir(inProject(CLAUDE_DIR), { recursive: true }); + await fs.writeFile(inProject(CLAUDE_DIR, 'proposal.md'), 'my own proposal command\n'); + + const detection = await detectLegacyArtifacts(testDir); + expect(detection.slashCommandDirs).not.toContain(CLAUDE_DIR); + expect(detection.hasLegacyArtifacts).toBe(false); + await cleanupLegacyArtifacts(testDir, detection); + + expect(await fs.readFile(inProject(CLAUDE_DIR, 'proposal.md'), 'utf-8')).toBe('my own proposal command\n'); + }); + + it('keeps a same-named user file beside OpenSpec files and deletes only the generated ones', async () => { + await writeFiles(CLAUDE_DIR, ['apply.md', 'archive.md']); + await fs.writeFile(inProject(CLAUDE_DIR, 'proposal.md'), 'my own proposal command\n'); + + const detection = await detectLegacyArtifacts(testDir); + expect(detection.slashCommandDirs).not.toContain(CLAUDE_DIR); + expect(formatDetectionSummary(detection)).not.toContain(`${CLAUDE_DIR}/proposal.md`); + const result = await cleanupLegacyArtifacts(testDir, detection); + + expect(await fs.readFile(inProject(CLAUDE_DIR, 'proposal.md'), 'utf-8')).toBe('my own proposal command\n'); + expect(await exists(inProject(CLAUDE_DIR, 'apply.md'))).toBe(false); + expect(result.keptFiles).toEqual([`${CLAUDE_DIR}/proposal.md`]); + }); + + it.each([ + ['a folder of only OpenSpec files', [] as string[]], + ['a folder that also holds user files', ['team-review.md']], + ])('keeps proposal.md when the user replaces it between detection and cleanup (%s)', async (_label, extra) => { + await writeFiles(CLAUDE_DIR, [...CLAUDE_FILES, ...extra]); + const detection = await detectLegacyArtifacts(testDir); + + // e.g. while the interactive upgrade prompt was waiting + await fs.writeFile(inProject(CLAUDE_DIR, 'proposal.md'), 'my own proposal command\n'); + const result = await cleanupLegacyArtifacts(testDir, detection); + + expect(await fs.readFile(inProject(CLAUDE_DIR, 'proposal.md'), 'utf-8')).toBe('my own proposal command\n'); + expect(await exists(inProject(CLAUDE_DIR, 'apply.md'))).toBe(false); + expect(result.deletedFiles).not.toContain(`${CLAUDE_DIR}/proposal.md`); + expect(result.deletedDirs).not.toContain(CLAUDE_DIR); + expect(result.keptFiles).toContain(`${CLAUDE_DIR}/proposal.md`); + }); + + it('keeps proposal.md when the user replaces it after cleanup has scanned the folder', async () => { + await writeFiles(CLAUDE_DIR, CLAUDE_FILES); + const detection = await detectLegacyArtifacts(testDir); + expect(detection.slashCommandDirs).toContain(CLAUDE_DIR); + const proposalPath = inProject(CLAUDE_DIR, 'proposal.md'); + + // Swap in the user's file right after cleanup's own directory scan has + // read the generated one, so only a check just before the unlink catches it. + const realReadFile = fs.readFile.bind(fs); + let replaced = false; + const spy = vi.spyOn(fs, 'readFile').mockImplementation((async (file: any, options?: any) => { + const content = await realReadFile(file, options); + if (!replaced && file === proposalPath) { + replaced = true; + await fs.writeFile(proposalPath, 'my own proposal command\n'); + } + return content; + }) as typeof fs.readFile); + let result; + try { + result = await cleanupLegacyArtifacts(testDir, detection); + } finally { + spy.mockRestore(); + } + + expect(replaced).toBe(true); + expect(await fs.readFile(proposalPath, 'utf-8')).toBe('my own proposal command\n'); + expect(await exists(inProject(CLAUDE_DIR, 'apply.md'))).toBe(false); + expect(result.deletedFiles).not.toContain(`${CLAUDE_DIR}/proposal.md`); + expect(result.deletedDirs).not.toContain(CLAUDE_DIR); + expect(result.keptFiles).toContain(`${CLAUDE_DIR}/proposal.md`); + }); + + // Creating symlinks on Windows needs elevated rights. + it.skipIf(process.platform === 'win32')('never follows a symlinked legacy command folder', async () => { + const shared = path.join(testDir, 'shared-commands'); + await fs.mkdir(shared, { recursive: true }); + for (const name of CLAUDE_FILES) { + await fs.writeFile(path.join(shared, name), generatedContent(name)); + } + await fs.mkdir(inProject('.claude/commands'), { recursive: true }); + await fs.symlink(shared, inProject(CLAUDE_DIR), 'dir'); + + const detection = await detectLegacyArtifacts(testDir); + expect(detection.slashCommandDirs).not.toContain(CLAUDE_DIR); + expect(detection.slashCommandFiles.filter((file) => file.startsWith(CLAUDE_DIR))).toEqual([]); + await cleanupLegacyArtifacts(testDir, detection); + + expect((await fs.readdir(shared)).sort()).toEqual(CLAUDE_FILES); + expect((await fs.lstat(inProject(CLAUDE_DIR))).isSymbolicLink()).toBe(true); + }); +}); + +describe('openspec init with a legacy command folder', () => { + const T = 120_000; + let base: string; + + beforeEach(async () => { + base = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-legacy-init-')); + }); + + afterEach(async () => { + await fs.rm(base, { recursive: true, force: true }); + }); + + async function legacyProject(withUserFile: boolean) { + const home = path.join(base, 'home'); + const project = path.join(base, 'project'); + const dir = path.join(project, '.claude', 'commands', 'openspec'); + await fs.mkdir(home, { recursive: true }); + await fs.mkdir(dir, { recursive: true }); + for (const name of CLAUDE_FILES) { + await fs.writeFile( + path.join(dir, name), + `---\nname: OpenSpec: ${name}\n---\n${OPENSPEC_MARKERS.start}\nold\n${OPENSPEC_MARKERS.end}\n` + ); + } + if (withUserFile) { + await fs.writeFile(path.join(dir, 'team-review.md'), '---\ndescription: my team review checklist\n---\nReview carefully.\n'); + } + const env = { + HOME: home, + USERPROFILE: home, + XDG_CONFIG_HOME: path.join(home, '.config'), + XDG_DATA_HOME: path.join(home, '.local', 'share'), + CODEX_HOME: path.join(home, '.codex'), + OPENSPEC_NO_ANIMATION: '1', + }; + const init = (extraArgs: string[]) => + runCLI(['init', '--tools', 'claude', ...extraArgs], { cwd: project, env, timeoutMs: 60_000 }); + return { dir, init }; + } + + it('removes a folder holding only OpenSpec files', async () => { + const { dir, init } = await legacyProject(false); + expect((await init([])).exitCode).toBe(0); + await expect(fs.access(dir)).rejects.toThrow(); + }, T); + + // Without a TTY, init cleans up automatically even without --force, which + // is how agents and CI run it. + it.each([[[] as string[]], [['--force']]])('keeps a user file in the folder (init %j)', async (extraArgs) => { + const { dir, init } = await legacyProject(true); + + const result = await init(extraArgs); + + expect(result.exitCode).toBe(0); + expect(await fs.readFile(path.join(dir, 'team-review.md'), 'utf-8')).toContain('Review carefully.'); + await expect(fs.access(path.join(dir, 'proposal.md'))).rejects.toThrow(); + expect(result.stdout).toContain(`Kept ${CLAUDE_DIR}/team-review.md (not created by OpenSpec)`); + }, T); +}); diff --git a/test/core/update.test.ts b/test/core/update.test.ts index 86fa6bba05..04c2d4107a 100644 --- a/test/core/update.test.ts +++ b/test/core/update.test.ts @@ -2782,12 +2782,13 @@ ${OPENSPEC_MARKERS.end} 'old' ); - // Create legacy slash command directory + // Create legacy slash command directory holding a command OpenSpec wrote. + // Only those files are removed; any other file here is the user's. const legacyCommandDir = path.join(testDir, '.claude', 'commands', 'openspec'); await fs.mkdir(legacyCommandDir, { recursive: true }); await fs.writeFile( - path.join(legacyCommandDir, 'old-command.md'), - 'old command' + path.join(legacyCommandDir, 'proposal.md'), + '\nold command\n\n' ); const consoleSpy = vi.spyOn(console, 'log'); @@ -2937,7 +2938,7 @@ More user content after markers. await fs.mkdir(legacyCommandDir, { recursive: true }); await fs.writeFile( path.join(legacyCommandDir, 'proposal.md'), - 'old command content' + '\nold command content\n\n' ); const consoleSpy = vi.spyOn(console, 'log'); @@ -2988,7 +2989,7 @@ More user content after markers. await fs.mkdir(path.join(testDir, '.claude', 'commands', 'openspec'), { recursive: true }); await fs.writeFile( path.join(testDir, '.claude', 'commands', 'openspec', 'proposal.md'), - 'content' + '\ncontent\n\n' ); await fs.mkdir(path.join(testDir, '.cursor', 'commands'), { recursive: true }); @@ -3060,7 +3061,7 @@ More user content after markers. await fs.mkdir(legacyCommandDir, { recursive: true }); await fs.writeFile( path.join(legacyCommandDir, 'proposal.md'), - 'old command' + '\nold command\n\n' ); const consoleSpy = vi.spyOn(console, 'log'); @@ -3104,7 +3105,7 @@ More user content after markers. await fs.mkdir(path.join(testDir, '.claude', 'commands', 'openspec'), { recursive: true }); await fs.writeFile( path.join(testDir, '.claude', 'commands', 'openspec', 'proposal.md'), - 'content' + '\ncontent\n\n' ); await fs.mkdir(path.join(testDir, '.cursor', 'commands'), { recursive: true }); @@ -3148,7 +3149,7 @@ More user content after markers. await fs.mkdir(legacyCommandDir, { recursive: true }); await fs.writeFile( path.join(legacyCommandDir, 'proposal.md'), - 'old command content' + '\nold command content\n\n' ); const consoleSpy = vi.spyOn(console, 'log'); @@ -3195,7 +3196,7 @@ More user content after markers. await fs.mkdir(path.join(testDir, '.claude', 'commands', 'openspec'), { recursive: true }); await fs.writeFile( path.join(testDir, '.claude', 'commands', 'openspec', 'proposal.md'), - 'content' + '\ncontent\n\n' ); // Create update command with force option @@ -3227,7 +3228,7 @@ More user content after markers. await fs.mkdir(path.join(testDir, '.claude', 'commands', 'openspec'), { recursive: true }); await fs.writeFile( path.join(testDir, '.claude', 'commands', 'openspec', 'proposal.md'), - 'content' + '\ncontent\n\n' ); // Create update command with force option @@ -3252,7 +3253,7 @@ More user content after markers. await fs.mkdir(path.join(testDir, '.claude', 'commands', 'openspec'), { recursive: true }); await fs.writeFile( path.join(testDir, '.claude', 'commands', 'openspec', 'proposal.md'), - 'content' + '\ncontent\n\n' ); const forceUpdateCommand = new UpdateCommand({ force: true });