diff --git a/__tests__/domains/config/merger/conflict-resolver.test.ts b/__tests__/domains/config/merger/conflict-resolver.test.ts index 6d272c6..acb51a3 100644 --- a/__tests__/domains/config/merger/conflict-resolver.test.ts +++ b/__tests__/domains/config/merger/conflict-resolver.test.ts @@ -11,8 +11,10 @@ function createMergeResult(): MergeResult { hooksAdded: 0, hooksPreserved: 0, hooksSkipped: 0, + hooksRemoved: 0, mcpServersPreserved: 0, mcpServersSkipped: 0, + mcpServersRemoved: 0, conflictsDetected: [], newlyInstalledHooks: [], newlyInstalledServers: [], diff --git a/__tests__/domains/config/merger/merge-engine.test.ts b/__tests__/domains/config/merger/merge-engine.test.ts new file mode 100644 index 0000000..ba45b4a --- /dev/null +++ b/__tests__/domains/config/merger/merge-engine.test.ts @@ -0,0 +1,150 @@ +import { describe, expect, it } from "bun:test"; +import { mergeHooks, mergeMcp, mergeSettings } from "@/domains/config/merger/merge-engine.js"; +import type { HookEntry, MergeResult, SettingsJson } from "@/domains/config/merger/types.js"; + +function createMergeResult(): MergeResult { + return { + merged: {}, + hooksAdded: 0, + hooksPreserved: 0, + hooksSkipped: 0, + hooksRemoved: 0, + mcpServersPreserved: 0, + mcpServersSkipped: 0, + mcpServersRemoved: 0, + conflictsDetected: [], + newlyInstalledHooks: [], + newlyInstalledServers: [], + hooksByOrigin: new Map(), + }; +} + +describe("merge-engine deprecation removal", () => { + describe("mergeHooks removes deprecated hooks", () => { + it("should remove hook in installed but not in source", () => { + const sourceHooks: Record = { + SessionStart: [{ type: "command", command: "node new-hook.js" }], + }; + const destHooks: Record = { + SessionStart: [ + { type: "command", command: "node new-hook.js" }, + { type: "command", command: "node deprecated-hook.js" }, + ], + }; + const result = createMergeResult(); + + const merged = mergeHooks(sourceHooks, destHooks, result, { + installedSettings: { + hooks: ["node deprecated-hook.js"], + }, + }); + + // deprecated-hook.js should be removed + expect(merged.SessionStart).toHaveLength(1); + expect(result.hooksRemoved).toBe(1); + expect(result.removedHooks).toContain("node deprecated-hook.js"); + }); + + it("should preserve user-added hook not in installed", () => { + const sourceHooks: Record = { + SessionStart: [{ type: "command", command: "node ck-hook.js" }], + }; + const destHooks: Record = { + SessionStart: [ + { type: "command", command: "node ck-hook.js" }, + { type: "command", command: "node user-hook.js" }, + ], + }; + const result = createMergeResult(); + + const merged = mergeHooks(sourceHooks, destHooks, result, { + installedSettings: { + hooks: ["node ck-hook.js"], // user-hook.js not in installed = user added it + }, + }); + + // user-hook.js should be preserved + expect(merged.SessionStart).toHaveLength(2); + expect(result.hooksRemoved).toBe(0); + }); + + it("should not remove anything with empty installedSettings", () => { + const sourceHooks: Record = { + SessionStart: [{ type: "command", command: "node new-hook.js" }], + }; + const destHooks: Record = { + SessionStart: [{ type: "command", command: "node old-hook.js" }], + }; + const result = createMergeResult(); + + const merged = mergeHooks(sourceHooks, destHooks, result, { + installedSettings: { hooks: [] }, + }); + + // old-hook.js preserved (fresh install scenario) + expect(merged.SessionStart).toHaveLength(2); + expect(result.hooksRemoved).toBe(0); + }); + }); + + describe("mergeMcp removes deprecated servers", () => { + it("should remove server in installed but not in source", () => { + const sourceMcp: SettingsJson["mcp"] = { + servers: { "new-server": { command: "npx new" } }, + }; + const destMcp: SettingsJson["mcp"] = { + servers: { + "new-server": { command: "npx new" }, + "deprecated-server": { command: "npx old" }, + }, + }; + const result = createMergeResult(); + + const merged = mergeMcp(sourceMcp, destMcp, result, { + installedSettings: { + mcpServers: ["deprecated-server"], + }, + }); + + expect(merged?.servers).not.toHaveProperty("deprecated-server"); + expect(merged?.servers).toHaveProperty("new-server"); + expect(result.mcpServersRemoved).toBe(1); + expect(result.removedMcpServers).toContain("deprecated-server"); + }); + + it("should preserve user-added server not in installed", () => { + const sourceMcp: SettingsJson["mcp"] = { + servers: { "ck-server": { command: "npx ck" } }, + }; + const destMcp: SettingsJson["mcp"] = { + servers: { + "ck-server": { command: "npx ck" }, + "user-server": { command: "npx user" }, + }, + }; + const result = createMergeResult(); + + const merged = mergeMcp(sourceMcp, destMcp, result, { + installedSettings: { + mcpServers: ["ck-server"], // user-server not in installed + }, + }); + + expect(merged?.servers).toHaveProperty("ck-server"); + expect(merged?.servers).toHaveProperty("user-server"); + expect(result.mcpServersRemoved).toBe(0); + }); + }); + + describe("mergeSettings initializes removal counters", () => { + it("should initialize hooksRemoved and mcpServersRemoved to 0", () => { + const source: SettingsJson = {}; + const dest: SettingsJson = {}; + + const result = mergeSettings(source, dest); + + expect(result.hooksRemoved).toBe(0); + expect(result.mcpServersRemoved).toBe(0); + }); + }); +}); diff --git a/src/domains/config/merger/merge-engine.ts b/src/domains/config/merger/merge-engine.ts index a5f61ec..7b3021d 100644 --- a/src/domains/config/merger/merge-engine.ts +++ b/src/domains/config/merger/merge-engine.ts @@ -1,14 +1,16 @@ /** * Core merge logic for settings */ -import { logger } from "@/shared/logger.js"; +import { logger, normalizeCommand } from "@/shared"; import { mergeHookEntries } from "./conflict-resolver.js"; +import { extractCommands } from "./diff-calculator.js"; import type { HookConfig, HookEntry, MergeOptions, MergeResult, SettingsJson } from "./types.js"; /** * Merge hooks configurations * User hooks are preserved, CK hooks are added (deduplicated by command) * Respects user deletions when installedSettings is provided + * Removes deprecated hooks (in installed but not in source) */ export function mergeHooks( sourceHooks: Record, @@ -20,6 +22,12 @@ export function mergeHooks( const installedHooks = options?.installedSettings?.hooks ?? []; const sourceKit = options?.sourceKit; + // Extract all commands from source for deprecation check + const sourceCommands = new Set(); + for (const entries of Object.values(sourceHooks)) { + extractCommands(entries, sourceCommands); + } + for (const [eventName, sourceEntries] of Object.entries(sourceHooks)) { const destEntries = destHooks[eventName] || []; merged[eventName] = mergeHookEntries( @@ -32,13 +40,85 @@ export function mergeHooks( ); } + // Remove deprecated hooks: in installedHooks but NOT in source + // These are hooks that CK previously installed but no longer ships + if (installedHooks.length > 0) { + const deprecatedHooks = installedHooks.filter( + (hook) => !sourceCommands.has(normalizeCommand(hook)), + ); + + if (deprecatedHooks.length > 0) { + result.removedHooks = result.removedHooks ?? []; + + for (const [eventName, entries] of Object.entries(merged)) { + const filtered = removeDeprecatedFromEntries( + entries as (HookConfig | HookEntry)[], + deprecatedHooks, + result, + ); + if (filtered.length > 0) { + merged[eventName] = filtered; + } else { + // Remove empty event arrays + delete merged[eventName]; + } + } + } + } + return merged; } +/** + * Remove deprecated hooks from entries array + * Returns filtered entries with deprecated hooks removed + */ +function removeDeprecatedFromEntries( + entries: (HookConfig | HookEntry)[], + deprecatedHooks: string[], + result: MergeResult, +): (HookConfig | HookEntry)[] { + const deprecatedSet = new Set(deprecatedHooks.map((h) => normalizeCommand(h))); + const filtered: (HookConfig | HookEntry)[] = []; + + for (const entry of entries) { + if ("hooks" in entry && entry.hooks) { + // HookConfig with hooks array - filter individual hooks + const remainingHooks = entry.hooks.filter((h) => { + if (h.command && deprecatedSet.has(normalizeCommand(h.command))) { + result.hooksRemoved++; + result.removedHooks?.push(h.command); + logger.info(`Removed deprecated hook: ${h.command.slice(0, 60)}...`); + return false; + } + return true; + }); + if (remainingHooks.length > 0) { + filtered.push({ ...entry, hooks: remainingHooks }); + } + } else if ("command" in entry) { + // Single HookEntry + if (deprecatedSet.has(normalizeCommand(entry.command))) { + result.hooksRemoved++; + result.removedHooks?.push(entry.command); + logger.info(`Removed deprecated hook: ${entry.command.slice(0, 60)}...`); + } else { + filtered.push(entry); + } + } else { + // Unknown structure, keep it + filtered.push(entry); + } + } + + return filtered; +} + /** * Merge MCP configurations * User servers are preserved, new CK servers are added * Respects user deletions when installedSettings is provided + * Removes deprecated servers (in installed but not in source) */ export function mergeMcp( sourceMcp: SettingsJson["mcp"], @@ -135,6 +215,31 @@ export function mergeMcp( } } + // Remove deprecated servers: in installedServers but NOT in source + // These are servers that CK previously installed but no longer ships + if (installedServers.length > 0 && merged.servers) { + const sourceServerNames = new Set(Object.keys(sourceMcp.servers || {})); + const deprecatedServers = installedServers.filter((server) => !sourceServerNames.has(server)); + + if (deprecatedServers.length > 0) { + result.removedMcpServers = result.removedMcpServers ?? []; + + for (const serverName of deprecatedServers) { + if (serverName in merged.servers) { + delete merged.servers[serverName]; + result.mcpServersRemoved++; + result.removedMcpServers.push(serverName); + logger.info(`Removed deprecated MCP server: ${serverName}`); + } + } + + // Clean up empty servers object + if (merged.servers && Object.keys(merged.servers).length === 0) { + merged.servers = undefined; + } + } + } + // Copy other MCP keys that don't exist for (const key of Object.keys(sourceMcp)) { if (key !== "servers" && !(key in merged)) { @@ -163,8 +268,10 @@ export function mergeSettings( hooksAdded: 0, hooksPreserved: 0, hooksSkipped: 0, + hooksRemoved: 0, mcpServersPreserved: 0, mcpServersSkipped: 0, + mcpServersRemoved: 0, conflictsDetected: [], newlyInstalledHooks: [], newlyInstalledServers: [], diff --git a/src/domains/config/merger/types.ts b/src/domains/config/merger/types.ts index ee00003..f8e48cd 100644 --- a/src/domains/config/merger/types.ts +++ b/src/domains/config/merger/types.ts @@ -58,8 +58,10 @@ export interface MergeResult { hooksAdded: number; hooksPreserved: number; hooksSkipped: number; // Hooks skipped because user removed them + hooksRemoved: number; // Hooks removed because kit no longer ships them mcpServersPreserved: number; mcpServersSkipped: number; // Servers skipped because user removed them + mcpServersRemoved: number; // Servers removed because kit no longer ships them mcpServersOverwritten?: number; // Servers overwritten due to timestamp comparison conflictsDetected: string[]; // Track what was actually installed (for persistence) @@ -70,6 +72,9 @@ export interface MergeResult { /** Conflict resolution tracking for summary display */ hookConflicts?: HookConflictInfo[]; mcpConflicts?: McpConflictInfo[]; + /** Deprecated entries removed during this merge */ + removedHooks?: string[]; + removedMcpServers?: string[]; } // Options for merge operations