diff --git a/src/cli/commands/generate.ts b/src/cli/commands/generate.ts index 799460e98..1914c00be 100644 --- a/src/cli/commands/generate.ts +++ b/src/cli/commands/generate.ts @@ -120,6 +120,17 @@ export async function generateCommand(logger: Logger, options: GenerateOptions): } if (totalGenerated === 0) { + if (check) { + if (result.hasDiff) { + throw new CLIError( + "Files are not up to date. Run 'rulesync generate' to update.", + ErrorCodes.GENERATION_FAILED, + ); + } + logger.success("✓ All files are up to date."); + return; + } + const enabledFeatures = features.join(", "); logger.info(`✓ All files are up to date (${enabledFeatures})`); return; diff --git a/src/features/commands/commands-processor.test.ts b/src/features/commands/commands-processor.test.ts index 152531d75..3f4e2dd1d 100644 --- a/src/features/commands/commands-processor.test.ts +++ b/src/features/commands/commands-processor.test.ts @@ -1221,7 +1221,7 @@ describe("CommandsProcessor", () => { } }); - it("should filter out non-deletable files when forDeletion is true", async () => { + it("should include non-deletable files for orphan detection when forDeletion is true", async () => { processor = new CommandsProcessor({ logger, baseDir: testDir, toolTarget: "claudecode" }); mockFindFilesByGlobs.mockResolvedValue([ @@ -1239,8 +1239,15 @@ describe("CommandsProcessor", () => { ); const filesToDelete = await processor.loadToolFiles({ forDeletion: true }); - expect(filesToDelete).toHaveLength(1); - expect(filesToDelete[0]?.getRelativeFilePath()).toBe("deletable.md"); + // Should return all files for orphan detection, regardless of isDeletable() + // isDeletable() will be checked during actual deletion in removeOrphanAiFiles + expect(filesToDelete).toHaveLength(2); + const deletable = filesToDelete.find((f) => f.getRelativeFilePath() === "deletable.md"); + const nonDeletable = filesToDelete.find( + (f) => f.getRelativeFilePath() === "non-deletable.md", + ); + expect(deletable?.isDeletable()).toBe(true); + expect(nonDeletable?.isDeletable()).toBe(false); }); it("should reject path traversal in loadToolFiles", async () => { diff --git a/src/features/commands/commands-processor.ts b/src/features/commands/commands-processor.ts index 535167849..525a3ac36 100644 --- a/src/features/commands/commands-processor.ts +++ b/src/features/commands/commands-processor.ts @@ -453,16 +453,15 @@ export class CommandsProcessor extends FeatureProcessor { const commandFilePaths = await findFilesByGlobs(globPattern); if (forDeletion) { - const toolCommands = commandFilePaths - .map((path) => - factory.class.forDeletion({ - baseDir: this.baseDir, - relativeDirPath: paths.relativeDirPath, - relativeFilePath: this.safeRelativePath(baseDirFull, path), - global: this.global, - }), - ) - .filter((cmd) => cmd.isDeletable()); + const toolCommands = commandFilePaths.map((path) => + factory.class.forDeletion({ + baseDir: this.baseDir, + relativeDirPath: paths.relativeDirPath, + relativeFilePath: this.safeRelativePath(baseDirFull, path), + global: this.global, + }), + ); + // Don't filter by isDeletable() here; it will be checked during actual deletion this.logger.debug( `Successfully loaded ${toolCommands.length} ${paths.relativeDirPath} commands`, diff --git a/src/features/hooks/hooks-processor.test.ts b/src/features/hooks/hooks-processor.test.ts index b3d5003b0..d6b80d794 100644 --- a/src/features/hooks/hooks-processor.test.ts +++ b/src/features/hooks/hooks-processor.test.ts @@ -173,10 +173,13 @@ describe("HooksProcessor", () => { expect(files[0]?.getRelativeFilePath()).toBe("hooks.json"); }); - it("should return empty array for claudecode when forDeletion (not deletable)", async () => { + it("should return file for claudecode when forDeletion (not deletable) for orphan detection", async () => { const processor = new HooksProcessor({ logger, baseDir: testDir, toolTarget: "claudecode" }); const files = await processor.loadToolFiles({ forDeletion: true }); - expect(files).toHaveLength(0); + // Should return file even if not deletable, for orphan detection + // isDeletable() will be checked during actual deletion in removeOrphanAiFiles + expect(files).toHaveLength(1); + expect(files[0].isDeletable()).toBe(false); }); }); diff --git a/src/features/hooks/hooks-processor.ts b/src/features/hooks/hooks-processor.ts index d7ffc06c6..475b61a5f 100644 --- a/src/features/hooks/hooks-processor.ts +++ b/src/features/hooks/hooks-processor.ts @@ -272,11 +272,9 @@ export class HooksProcessor extends FeatureProcessor { relativeFilePath: paths.relativeFilePath, global: this.global, }); - const list = toolHooks.isDeletable?.() !== false ? [toolHooks] : []; - this.logger.debug( - `Successfully loaded ${list.length} ${this.toolTarget} hooks files for deletion`, - ); - return list; + // Return the file for orphan detection; isDeletable() will be checked during actual deletion + this.logger.debug(`Successfully loaded 1 ${this.toolTarget} hooks file for deletion check`); + return [toolHooks]; } const toolHooks = await factory.class.fromFile({ diff --git a/src/features/ignore/ignore-processor.test.ts b/src/features/ignore/ignore-processor.test.ts index cf85ffee8..898b0c078 100644 --- a/src/features/ignore/ignore-processor.test.ts +++ b/src/features/ignore/ignore-processor.test.ts @@ -405,7 +405,7 @@ describe("IgnoreProcessor", () => { }); describe("loadToolFiles with forDeletion: true", () => { - it("should filter out non-deletable files when loading for deletion", async () => { + it("should include non-deletable files for orphan detection when loading for deletion", async () => { await ensureDir(join(testDir, ".claude")); await writeFileContent( join(testDir, ".claude", "settings.json"), @@ -423,9 +423,11 @@ describe("IgnoreProcessor", () => { expect(allFiles).toHaveLength(1); expect(allFiles[0]?.isDeletable()).toBe(false); - // Load tool files for deletion (should exclude non-deletable files) - const filesToDelete = await processor.loadToolFiles({ forDeletion: true }); - expect(filesToDelete).toHaveLength(0); + // Load tool files for deletion checking - should include all files for orphan detection + // isDeletable() will be checked during actual deletion in removeOrphanAiFiles + const filesForDeletionCheck = await processor.loadToolFiles({ forDeletion: true }); + expect(filesForDeletionCheck).toHaveLength(1); + expect(filesForDeletionCheck[0]?.isDeletable()).toBe(false); }); it("should treat claudecode-legacy ignore files the same as claudecode", async () => { @@ -450,8 +452,11 @@ describe("IgnoreProcessor", () => { expect(allFiles[0]).toBeInstanceOf(ClaudecodeIgnore); expect(allFiles[0]?.isDeletable()).toBe(false); - const filesToDelete = await processor.loadToolFiles({ forDeletion: true }); - expect(filesToDelete).toHaveLength(0); + // loadToolFiles({ forDeletion: true }) should still include non-deletable files + // for orphan detection; isDeletable() is checked during actual deletion + const filesForDeletionCheck = await processor.loadToolFiles({ forDeletion: true }); + expect(filesForDeletionCheck).toHaveLength(1); + expect(filesForDeletionCheck[0]?.isDeletable()).toBe(false); }); it("should return deletable files with correct paths", async () => { diff --git a/src/features/ignore/ignore-processor.ts b/src/features/ignore/ignore-processor.ts index 47692ed45..ba9719b37 100644 --- a/src/features/ignore/ignore-processor.ts +++ b/src/features/ignore/ignore-processor.ts @@ -156,8 +156,8 @@ export class IgnoreProcessor extends FeatureProcessor { relativeFilePath: paths.relativeFilePath, }); - const toolIgnores = toolIgnore.isDeletable() ? [toolIgnore] : []; - return toolIgnores; + // Return the file for orphan detection; isDeletable() will be checked during actual deletion + return [toolIgnore]; } const toolIgnores = await this.loadToolIgnores(); diff --git a/src/features/mcp/mcp-processor.test.ts b/src/features/mcp/mcp-processor.test.ts index 825f611e1..ede4db7b2 100644 --- a/src/features/mcp/mcp-processor.test.ts +++ b/src/features/mcp/mcp-processor.test.ts @@ -1177,7 +1177,7 @@ describe("McpProcessor", () => { expect(filesToDelete).toEqual([]); }); - it("should filter out non-deletable files in global mode", async () => { + it("should include non-deletable files for orphan detection in global mode", async () => { // Mock forDeletion to return non-deletable instance (vi.mocked(ClaudecodeMcp).forDeletion as ReturnType).mockImplementation( (params: { relativeFilePath: string }) => ({ @@ -1196,8 +1196,10 @@ describe("McpProcessor", () => { const filesToDelete = await processor.loadToolFiles({ forDeletion: true }); - // loadToolFiles with forDeletion: true should filter out non-deletable files - expect(filesToDelete).toHaveLength(0); + // loadToolFiles with forDeletion: true should include all files for orphan detection + // isDeletable() will be checked during actual deletion in removeOrphanAiFiles + expect(filesToDelete).toHaveLength(1); + expect(filesToDelete[0].isDeletable()).toBe(false); }); it("should not filter out deletable files in local mode", async () => { diff --git a/src/features/mcp/mcp-processor.ts b/src/features/mcp/mcp-processor.ts index dc29e6cc8..c46426336 100644 --- a/src/features/mcp/mcp-processor.ts +++ b/src/features/mcp/mcp-processor.ts @@ -378,9 +378,9 @@ export class McpProcessor extends FeatureProcessor { global: this.global, }); - const toolMcps = toolMcp.isDeletable() ? [toolMcp] : []; - this.logger.debug(`Successfully loaded ${toolMcps.length} ${this.toolTarget} MCP files`); - return toolMcps; + // Return the file for orphan detection; isDeletable() will be checked during actual deletion + this.logger.debug(`Successfully loaded 1 ${this.toolTarget} MCP file for deletion check`); + return [toolMcp]; } const toolMcps = [ diff --git a/src/features/rules/rules-processor.ts b/src/features/rules/rules-processor.ts index 2886c85c1..479dcde42 100644 --- a/src/features/rules/rules-processor.ts +++ b/src/features/rules/rules-processor.ts @@ -940,7 +940,8 @@ export class RulesProcessor extends FeatureProcessor { }; /** - * Build deletion rules from discovered file paths: resolve dir, check traversal, create forDeletion, filter isDeletable. + * Build deletion rules from discovered file paths: resolve dir, check traversal, create forDeletion. + * Don't filter by isDeletable() here; it will be checked during actual deletion. * * Two modes: * - Root mode (no opts): `relativeFilePath` = `basename(filePath)`, traversal checks `relativeDirPath` against `this.baseDir`. @@ -953,26 +954,24 @@ export class RulesProcessor extends FeatureProcessor { ): ToolRule[] => { const isNonRoot = opts !== undefined; const effectiveBaseDir = isNonRoot ? opts.baseDirOverride : this.baseDir; - return filePaths - .map((filePath) => { - const relativeDirPath = isNonRoot - ? opts.relativeDirPathOverride - : resolveRelativeDirPath(filePath); - const relativeFilePath = isNonRoot - ? relative(effectiveBaseDir, filePath) - : basename(filePath); - checkPathTraversal({ - relativePath: isNonRoot ? relativeFilePath : relativeDirPath, - intendedRootDir: effectiveBaseDir, - }); - return factory.class.forDeletion({ - baseDir: this.baseDir, - relativeDirPath, - relativeFilePath, - global: this.global, - }); - }) - .filter((rule) => rule.isDeletable()); + return filePaths.map((filePath) => { + const relativeDirPath = isNonRoot + ? opts.relativeDirPathOverride + : resolveRelativeDirPath(filePath); + const relativeFilePath = isNonRoot + ? relative(effectiveBaseDir, filePath) + : basename(filePath); + checkPathTraversal({ + relativePath: isNonRoot ? relativeFilePath : relativeDirPath, + intendedRootDir: effectiveBaseDir, + }); + return factory.class.forDeletion({ + baseDir: this.baseDir, + relativeDirPath, + relativeFilePath, + global: this.global, + }); + }); }; const findFilesWithFallback = async ( diff --git a/src/features/subagents/subagents-processor.ts b/src/features/subagents/subagents-processor.ts index 9675b99d2..460999523 100644 --- a/src/features/subagents/subagents-processor.ts +++ b/src/features/subagents/subagents-processor.ts @@ -376,16 +376,15 @@ export class SubagentsProcessor extends FeatureProcessor { ); if (forDeletion) { - const toolSubagents = subagentFilePaths - .map((path) => - factory.class.forDeletion({ - baseDir: this.baseDir, - relativeDirPath: paths.relativeDirPath, - relativeFilePath: basename(path), - global: this.global, - }), - ) - .filter((subagent) => subagent.isDeletable()); + const toolSubagents = subagentFilePaths.map((path) => + factory.class.forDeletion({ + baseDir: this.baseDir, + relativeDirPath: paths.relativeDirPath, + relativeFilePath: basename(path), + global: this.global, + }), + ); + // Don't filter by isDeletable() here; it will be checked during actual deletion this.logger.debug( `Successfully loaded ${toolSubagents.length} ${paths.relativeDirPath} subagents`, diff --git a/src/lib/generate.test.ts b/src/lib/generate.test.ts index 1a6a5c457..79be45168 100644 --- a/src/lib/generate.test.ts +++ b/src/lib/generate.test.ts @@ -365,6 +365,32 @@ describe("generate", () => { expect(result.mcpCount).toBe(0); expect(McpProcessor).not.toHaveBeenCalled(); }); + + it("should remove orphan MCP files when delete option is enabled and source is empty", async () => { + mockConfig.getFeatures.mockReturnValue(["mcp"]); + mockConfig.getDelete.mockReturnValue(true); + + const existingFiles = [{ file: "existing", getFilePath: () => "/path/to/existing" }]; + const mockProcessor = { + loadToolFiles: vi.fn().mockResolvedValue(existingFiles), + removeOrphanAiFiles: vi.fn().mockResolvedValue(1), + loadRulesyncFiles: vi.fn().mockResolvedValue([]), + convertRulesyncFilesToToolFiles: vi.fn(), + writeAiFiles: vi.fn(), + }; + vi.mocked(McpProcessor).mockImplementation(function () { + return mockProcessor as unknown as McpProcessor; + }); + + const result = await generate({ logger, config: mockConfig as never }); + + expect(result.mcpCount).toBe(0); + expect(result.hasDiff).toBe(true); + expect(mockProcessor.convertRulesyncFilesToToolFiles).not.toHaveBeenCalled(); + expect(mockProcessor.writeAiFiles).not.toHaveBeenCalled(); + expect(mockProcessor.loadToolFiles).toHaveBeenCalledWith({ forDeletion: true }); + expect(mockProcessor.removeOrphanAiFiles).toHaveBeenCalledWith(existingFiles, []); + }); }); describe("commands feature", () => { diff --git a/src/lib/generate.ts b/src/lib/generate.ts index 820171ef8..fa81c457a 100644 --- a/src/lib/generate.ts +++ b/src/lib/generate.ts @@ -349,13 +349,22 @@ async function generateMcpCore(params: { }); const rulesyncFiles = await processor.loadRulesyncFiles(); - const toolFiles = await processor.convertRulesyncFilesToToolFiles(rulesyncFiles); + let result; - const result = await processFeatureGeneration({ - config, - processor, - toolFiles, - }); + if (rulesyncFiles.length === 0) { + result = await processEmptyFeatureGeneration({ + config, + processor, + }); + } else { + const toolFiles = await processor.convertRulesyncFilesToToolFiles(rulesyncFiles); + + result = await processFeatureGeneration({ + config, + processor, + toolFiles, + }); + } totalCount += result.count; allPaths.push(...result.paths); diff --git a/src/types/feature-processor.test.ts b/src/types/feature-processor.test.ts index 39d12f152..08ac7abc5 100644 --- a/src/types/feature-processor.test.ts +++ b/src/types/feature-processor.test.ts @@ -23,6 +23,7 @@ function createMockFile(filePath: string): AiFile { getFilePath: () => filePath, getFileContent: () => "content", getRelativePathFromCwd: () => filePath, + isDeletable: () => true, } as AiFile; } diff --git a/src/types/feature-processor.ts b/src/types/feature-processor.ts index 01004f1a6..588d7436f 100644 --- a/src/types/feature-processor.ts +++ b/src/types/feature-processor.ts @@ -97,6 +97,11 @@ export abstract class FeatureProcessor { const orphanFiles = existingFiles.filter((f) => !generatedPaths.has(f.getFilePath())); for (const aiFile of orphanFiles) { + // Skip files that are not deletable (e.g., shared config files) + if (!aiFile.isDeletable()) { + continue; + } + const filePath = aiFile.getFilePath(); if (this.dryRun) { this.logger.info(`[DRY RUN] Would delete: ${filePath}`); @@ -105,6 +110,7 @@ export abstract class FeatureProcessor { } } + // Return count of all orphan files, including non-deletable ones, for check mode return orphanFiles.length; } }