Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/cli/commands/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 10 additions & 3 deletions src/features/commands/commands-processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand All @@ -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 () => {
Expand Down
19 changes: 9 additions & 10 deletions src/features/commands/commands-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down
7 changes: 5 additions & 2 deletions src/features/hooks/hooks-processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

Expand Down
8 changes: 3 additions & 5 deletions src/features/hooks/hooks-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
17 changes: 11 additions & 6 deletions src/features/ignore/ignore-processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
4 changes: 2 additions & 2 deletions src/features/ignore/ignore-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
8 changes: 5 additions & 3 deletions src/features/mcp/mcp-processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof vi.fn>).mockImplementation(
(params: { relativeFilePath: string }) => ({
Expand All @@ -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 () => {
Expand Down
6 changes: 3 additions & 3 deletions src/features/mcp/mcp-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
41 changes: 20 additions & 21 deletions src/features/rules/rules-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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 (
Expand Down
19 changes: 9 additions & 10 deletions src/features/subagents/subagents-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down
26 changes: 26 additions & 0 deletions src/lib/generate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
21 changes: 15 additions & 6 deletions src/lib/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/types/feature-processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ function createMockFile(filePath: string): AiFile {
getFilePath: () => filePath,
getFileContent: () => "content",
getRelativePathFromCwd: () => filePath,
isDeletable: () => true,
} as AiFile;
}

Expand Down
6 changes: 6 additions & 0 deletions src/types/feature-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand All @@ -105,6 +110,7 @@ export abstract class FeatureProcessor {
}
}

// Return count of all orphan files, including non-deletable ones, for check mode
return orphanFiles.length;
}
}
Loading