From 17b20ee3859d54cb4d2827c3c0945f321daa9fe1 Mon Sep 17 00:00:00 2001 From: Baptiste LAFOURCADE Date: Thu, 10 Sep 2026 10:10:00 +0200 Subject: [PATCH 1/2] refactor(cli): drop the MCP exclusion no command could reach `framework remove` was the only caller of UninstallUseCase and passed an empty mcpFilter, so UninstallMcpExclusionUseCase never removed or recorded anything, and nothing read the manifest's excludedMcp. Deleted: the use case, the mcpFilter option, McpExclusion and its equality, the manifest's exclusion methods and the excludedMcp member of a tool entry, with the tests and the round-trip fixture that exercised only that path. A manifest already carrying excludedMcp still loads: the tool-entry parser reads named fields and ignores the rest, so the field is dropped on the next write. The new test asserting the rewrite omits it failed before the deletion (the rewrite still held excludedMcp) and passes after. Measured: typecheck, lint, knip and the type-honesty check clean; test:arch 126 passed; unit and integration 4994 passed; e2e 297 passed. Refs #806 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011x4ms5qcGuZgYhCxfdHMUb AIDD-Session-Id: 4acc9a1c-19bc-4468-b8b6-e86644bcba60 --- .../uninstall-mcp-exclusion-use-case.ts | 81 ---------- .../uninstall/uninstall-use-case.ts | 24 +-- cli/src/contexts/framework/domain/manifest.ts | 46 +----- .../domain/manifest/mcp-exclusions.ts | 34 ---- .../framework/domain/manifest/tool-entry.ts | 12 -- .../contexts/tools/domain/mcp-exclusion.ts | 9 -- cli/src/presentation/commands/framework.ts | 1 - .../application/uninstall-plugin.unit.test.ts | 2 - .../uninstall-use-case.unit.test.ts | 69 +------- ...nstall-mcp-exclusion-use-case.unit.test.ts | 108 ------------- .../domain/manifest-round-trip.unit.test.ts | 1 - .../framework/domain/manifest.unit.test.ts | 150 +++--------------- .../manifest/mcp-exclusions.unit.test.ts | 22 --- .../domain/manifest/tool-entry.unit.test.ts | 1 - .../tools/domain/mcp-exclusion.unit.test.ts | 15 +- cli/tests/fixtures/manifests/full.json | 6 - .../fixtures/manifests/mcp-exclusions.json | 21 --- .../framework-wiring.integration.test.ts | 3 +- 18 files changed, 32 insertions(+), 573 deletions(-) delete mode 100644 cli/src/contexts/framework/application/uninstall/uninstall-mcp-exclusion-use-case.ts delete mode 100644 cli/src/contexts/framework/domain/manifest/mcp-exclusions.ts delete mode 100644 cli/tests/contexts/framework/application/uninstall/uninstall-mcp-exclusion-use-case.unit.test.ts delete mode 100644 cli/tests/contexts/framework/domain/manifest/mcp-exclusions.unit.test.ts delete mode 100644 cli/tests/fixtures/manifests/mcp-exclusions.json diff --git a/cli/src/contexts/framework/application/uninstall/uninstall-mcp-exclusion-use-case.ts b/cli/src/contexts/framework/application/uninstall/uninstall-mcp-exclusion-use-case.ts deleted file mode 100644 index 695221f92..000000000 --- a/cli/src/contexts/framework/application/uninstall/uninstall-mcp-exclusion-use-case.ts +++ /dev/null @@ -1,81 +0,0 @@ -import { join } from "node:path"; -import { type MergeFileEntry, removeEntriesFromJson } from "../../../../kernel/merge.js"; -import type { FileReader } from "../../../../kernel/ports/file-reader.js"; -import type { FileWriter } from "../../../../kernel/ports/file-writer.js"; -import type { Logger } from "../../../../kernel/ports/logger.js"; -import type { ToolId } from "../../../../kernel/tool.js"; -import type { McpExclusion } from "../../../tools/domain/mcp-exclusion.js"; -import type { Manifest } from "../../domain/manifest.js"; - -export interface UninstallMcpExclusionOptions { - toolId: ToolId; - manifest: Manifest; - projectRoot: string; - mcpFilter: string[]; -} - -export interface UninstallMcpExclusionResult { - toolId: ToolId; - fileCount: number; - deletedFiles: string[]; -} - -export class UninstallMcpExclusionUseCase { - constructor( - private readonly fs: FileReader & FileWriter, - private readonly logger: Logger - ) {} - - async execute(options: UninstallMcpExclusionOptions): Promise { - const { toolId, manifest, projectRoot, mcpFilter } = options; - this.logger.info(`Removing MCP entries from ${toolId}...`); - const mergeFiles = manifest.getMergeFiles(toolId); - const removedKeys: string[] = []; - const exclusions: McpExclusion[] = []; - for (const mergeFile of mergeFiles) { - const r = await this.processOneMergeFile(mergeFile, projectRoot, mcpFilter); - removedKeys.push(...r.keys); - exclusions.push(...r.exclusions); - } - this.rebuildMergeEntries(toolId, manifest, mcpFilter); - manifest.addExcludedMcp(toolId, exclusions); - return { toolId, fileCount: removedKeys.length, deletedFiles: removedKeys }; - } - - private async processOneMergeFile( - mergeFile: MergeFileEntry, - projectRoot: string, - mcpFilter: string[] - ): Promise<{ keys: string[]; exclusions: McpExclusion[] }> { - if (mergeFile.sectionKey === null) return { keys: [], exclusions: [] }; - const matching = mcpFilter.filter((k) => mergeFile.entries[k] !== undefined); - if (matching.length === 0) return { keys: [], exclusions: [] }; - await this.removeKeysFromJsonFile( - join(projectRoot, mergeFile.relativePath), - mergeFile.sectionKey, - matching - ); - const exclusions = matching.map((k) => ({ configPath: mergeFile.relativePath, entryKey: k })); - return { keys: matching, exclusions }; - } - - private async removeKeysFromJsonFile( - fullPath: string, - sectionKey: string | null, - keysToRemove: string[] - ): Promise { - const content = await this.fs.readFile(fullPath); - await this.fs.writeFile(fullPath, removeEntriesFromJson(content, sectionKey, keysToRemove)); - } - - private rebuildMergeEntries(toolId: ToolId, manifest: Manifest, removedKeys: string[]): void { - const mergeFiles = manifest.getMergeFiles(toolId); - const removedSet = new Set(removedKeys); - const updated = mergeFiles.map((mf) => { - const entries = { ...mf.entries }; - for (const key of removedSet) delete entries[key]; - return { ...mf, entries }; - }); - manifest.updateToolMergeFiles(toolId, updated); - } -} diff --git a/cli/src/contexts/framework/application/uninstall/uninstall-use-case.ts b/cli/src/contexts/framework/application/uninstall/uninstall-use-case.ts index 0354b0b2f..d03b49074 100644 --- a/cli/src/contexts/framework/application/uninstall/uninstall-use-case.ts +++ b/cli/src/contexts/framework/application/uninstall/uninstall-use-case.ts @@ -10,14 +10,12 @@ import type { ToolId } from "../../../../kernel/tool.js"; import { VALID_TOOL_IDS } from "../../../../kernel/tool.js"; import type { Manifest } from "../../domain/manifest.js"; import type { ManifestRepository } from "../../domain/ports/manifest-repository.js"; -import { UninstallMcpExclusionUseCase } from "./uninstall-mcp-exclusion-use-case.js"; import { UninstallPluginUseCase } from "./uninstall-plugin-use-case.js"; import { UninstallToolsUseCase } from "./uninstall-tools-use-case.js"; interface UninstallOptions { toolIds: ToolId[]; projectRoot: string; - mcpFilter: string[]; pluginName?: string; } @@ -30,7 +28,6 @@ interface UninstallToolResult { export class UninstallUseCase { private readonly pluginUninstall: UninstallPluginUseCase; private readonly toolsUninstall: UninstallToolsUseCase; - private readonly mcpExclusion: UninstallMcpExclusionUseCase; constructor( fs: FileReader & FileWriter, @@ -39,11 +36,10 @@ export class UninstallUseCase { ) { this.pluginUninstall = new UninstallPluginUseCase(fs, manifestRepo); this.toolsUninstall = new UninstallToolsUseCase(fs, logger); - this.mcpExclusion = new UninstallMcpExclusionUseCase(fs, logger); } async execute(options: UninstallOptions): Promise { - const { toolIds, projectRoot, mcpFilter, pluginName } = options; + const { toolIds, projectRoot, pluginName } = options; if (pluginName !== undefined) { return this.pluginUninstall.execute({ pluginName, toolIds, projectRoot }); @@ -57,10 +53,7 @@ export class UninstallUseCase { const manifest = await this.loadAndValidate(toolIds); - const results = - mcpFilter.length > 0 - ? await this.runMcpExclusions(toolIds, manifest, projectRoot, mcpFilter) - : await this.toolsUninstall.execute({ toolIds, manifest, projectRoot }); + const results = await this.toolsUninstall.execute({ toolIds, manifest, projectRoot }); await this.manifestRepo.save(manifest); return results; @@ -74,17 +67,4 @@ export class UninstallUseCase { } return manifest; } - - private async runMcpExclusions( - toolIds: ToolId[], - manifest: Manifest, - projectRoot: string, - mcpFilter: string[] - ): Promise { - const results: UninstallToolResult[] = []; - for (const toolId of toolIds) { - results.push(await this.mcpExclusion.execute({ toolId, manifest, projectRoot, mcpFilter })); - } - return results; - } } diff --git a/cli/src/contexts/framework/domain/manifest.ts b/cli/src/contexts/framework/domain/manifest.ts index 280487cd1..dc5cd67c4 100644 --- a/cli/src/contexts/framework/domain/manifest.ts +++ b/cli/src/contexts/framework/domain/manifest.ts @@ -3,8 +3,6 @@ import type { FileHash, InstallationFile } from "../../../kernel/file.js"; import type { MergeFileEntry } from "../../../kernel/merge.js"; import { AIDD_DIR, MANIFEST_FILENAME } from "../../../kernel/paths.js"; import type { ToolId } from "../../../kernel/tool.js"; -import type { McpExclusion } from "../../tools/domain/mcp-exclusion.js"; -import { addExclusions, removeExclusions } from "./manifest/mcp-exclusions.js"; import type { NativeRegistrations } from "./manifest/native-registrations.js"; import { addPluginToEntry, @@ -59,8 +57,7 @@ export class Manifest { toolId: ToolId, version: string, files: InstallationFile[], - mergeFiles: MergeFileEntry[] = [], - excludedMcp: McpExclusion[] = [] + mergeFiles: MergeFileEntry[] = [] ): void { const existing = this._tools.get(toolId); this._tools.set( @@ -70,7 +67,6 @@ export class Manifest { version, files, mergeFiles, - excludedMcp, existingPlugins: existing?.plugins ?? [], }) ); @@ -109,34 +105,6 @@ export class Manifest { return tracked; } - getExcludedMcp(toolId: ToolId): readonly McpExclusion[] { - return this._tools.get(toolId)?.excludedMcp ?? []; - } - - addExcludedMcp(toolId: ToolId, exclusions: McpExclusion[]): void { - const entry = this._tools.get(toolId); - if (!entry) throw new ToolNotInManifestError(toolId); - this._tools.set(toolId, { - ...entry, - excludedMcp: addExclusions(entry.excludedMcp, exclusions), - }); - } - - removeExcludedMcp(toolId: ToolId, exclusions: McpExclusion[]): void { - const entry = this._tools.get(toolId); - if (!entry) throw new ToolNotInManifestError(toolId); - this._tools.set(toolId, { - ...entry, - excludedMcp: removeExclusions(entry.excludedMcp, exclusions), - }); - } - - clearExcludedMcp(toolId: ToolId): void { - const entry = this._tools.get(toolId); - if (!entry) throw new ToolNotInManifestError(toolId); - this._tools.set(toolId, { ...entry, excludedMcp: [] }); - } - updateTrackedFileHash(toolId: ToolId, relativePath: string, hash: FileHash): void { const entry = this._tools.get(toolId); if (!entry) return; @@ -146,18 +114,10 @@ export class Manifest { }); } - updateToolMergeFiles( - toolId: ToolId, - mergeFiles: MergeFileEntry[], - excludedMcp?: McpExclusion[] - ): void { + updateToolMergeFiles(toolId: ToolId, mergeFiles: MergeFileEntry[]): void { const entry = this._tools.get(toolId); if (!entry) throw new ToolNotInManifestError(toolId); - this._tools.set(toolId, { - ...entry, - mergeFiles, - ...(excludedMcp !== undefined && { excludedMcp }), - }); + this._tools.set(toolId, { ...entry, mergeFiles }); } removeTool(toolId: ToolId): void { diff --git a/cli/src/contexts/framework/domain/manifest/mcp-exclusions.ts b/cli/src/contexts/framework/domain/manifest/mcp-exclusions.ts deleted file mode 100644 index 877d2d156..000000000 --- a/cli/src/contexts/framework/domain/manifest/mcp-exclusions.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { type McpExclusion, mcpExclusionEquals } from "../../../tools/domain/mcp-exclusion.js"; - -export interface McpExclusionData { - configPath: string; - entryKey: string; -} - -export function addExclusions( - existing: readonly McpExclusion[], - toAdd: readonly McpExclusion[] -): McpExclusion[] { - const result = [...existing]; - for (const excl of toAdd) { - if (!result.some((e) => mcpExclusionEquals(e, excl))) { - result.push(excl); - } - } - return result; -} - -export function removeExclusions( - existing: readonly McpExclusion[], - toRemove: readonly McpExclusion[] -): McpExclusion[] { - return existing.filter((e) => !toRemove.some((r) => mcpExclusionEquals(e, r))); -} - -export function toMcpExclusionData(exclusions: readonly McpExclusion[]): McpExclusionData[] { - return exclusions.map((e) => ({ configPath: e.configPath, entryKey: e.entryKey })); -} - -export function parseMcpExclusionData(data: readonly McpExclusionData[]): McpExclusion[] { - return data.map((e) => ({ configPath: e.configPath, entryKey: e.entryKey })); -} diff --git a/cli/src/contexts/framework/domain/manifest/tool-entry.ts b/cli/src/contexts/framework/domain/manifest/tool-entry.ts index 5e84b596e..57adee527 100644 --- a/cli/src/contexts/framework/domain/manifest/tool-entry.ts +++ b/cli/src/contexts/framework/domain/manifest/tool-entry.ts @@ -2,13 +2,7 @@ import { DuplicatePluginError, PluginNotFoundError } from "../../../../kernel/er import type { InstallationFile } from "../../../../kernel/file.js"; import type { MergeFileEntry } from "../../../../kernel/merge.js"; import type { ToolId } from "../../../../kernel/tool.js"; -import type { McpExclusion } from "../../../tools/domain/mcp-exclusion.js"; import { InstalledPlugin, type PluginEntryData } from "../plugins/installed-plugin.js"; -import { - type McpExclusionData, - parseMcpExclusionData, - toMcpExclusionData, -} from "./mcp-exclusions.js"; import { type MergeFileEntryData, parseMergeFileEntries, @@ -33,7 +27,6 @@ export interface ToolEntry { readonly version: string; readonly files: readonly TrackedFile[]; readonly mergeFiles: readonly MergeFileEntry[]; - readonly excludedMcp: readonly McpExclusion[]; readonly plugins: readonly InstalledPlugin[]; /** What this tool's own CLI was asked to register, or `undefined` for a tool with * no `nativeActivation` — see {@link NativeRegistrations}. */ @@ -45,7 +38,6 @@ export interface ToolEntryData { version: string; files: TrackedFileData[]; mergeFiles?: MergeFileEntryData[]; - excludedMcp?: McpExclusionData[]; plugins?: PluginEntryData[]; nativeRegistrations?: NativeRegistrationsData; } @@ -55,7 +47,6 @@ export function createToolEntry(params: { version: string; files: InstallationFile[]; mergeFiles: readonly MergeFileEntry[]; - excludedMcp: readonly McpExclusion[]; existingPlugins: readonly InstalledPlugin[]; }): ToolEntry { return { @@ -63,7 +54,6 @@ export function createToolEntry(params: { version: params.version, files: toTrackedFiles(params.files), mergeFiles: params.mergeFiles, - excludedMcp: params.excludedMcp, plugins: params.existingPlugins, }; } @@ -104,7 +94,6 @@ export function serializeToolEntry(entry: ToolEntry): ToolEntryData { version: entry.version, files: toTrackedFileData(entry.files), mergeFiles: toMergeFileEntryData(entry.mergeFiles), - ...(entry.excludedMcp.length > 0 && { excludedMcp: toMcpExclusionData(entry.excludedMcp) }), ...(entry.plugins.length > 0 && { plugins: entry.plugins.map((p) => p.toJSON()) }), ...(entry.nativeRegistrations !== undefined && { nativeRegistrations: toNativeRegistrationsData(entry.nativeRegistrations), @@ -118,7 +107,6 @@ export function parseToolEntry(toolId: ToolId, data: ToolEntryData): ToolEntry { version: data.version, files: parseTrackedFiles(data.files), mergeFiles: parseMergeFileEntries(data.mergeFiles ?? []), - excludedMcp: parseMcpExclusionData(data.excludedMcp ?? []), plugins: (data.plugins ?? []).map((p) => InstalledPlugin.fromJSON(p)), nativeRegistrations: parseNativeRegistrations(data.nativeRegistrations), }; diff --git a/cli/src/contexts/tools/domain/mcp-exclusion.ts b/cli/src/contexts/tools/domain/mcp-exclusion.ts index 8cbe77d68..2933810bc 100644 --- a/cli/src/contexts/tools/domain/mcp-exclusion.ts +++ b/cli/src/contexts/tools/domain/mcp-exclusion.ts @@ -28,12 +28,3 @@ function transformMcpForWin32(content: string): string { export function transformFor(platform: string): ((content: string) => string) | undefined { return platform === "win32" ? transformMcpForWin32 : undefined; } - -export interface McpExclusion { - readonly configPath: string; - readonly entryKey: string; -} - -export function mcpExclusionEquals(a: McpExclusion, b: McpExclusion): boolean { - return a.configPath === b.configPath && a.entryKey === b.entryKey; -} diff --git a/cli/src/presentation/commands/framework.ts b/cli/src/presentation/commands/framework.ts index 2ce15518a..f70be6269 100644 --- a/cli/src/presentation/commands/framework.ts +++ b/cli/src/presentation/commands/framework.ts @@ -102,7 +102,6 @@ async function runFrameworkRemove( const results = await deps.uninstallUseCase.execute({ toolIds: [toolId], projectRoot, - mcpFilter: [], }); const totalFileCount = results.reduce((sum, r) => sum + r.fileCount, 0); printToolRemoved(output, results[0].toolId, totalFileCount); diff --git a/cli/tests/contexts/framework/application/uninstall-plugin.unit.test.ts b/cli/tests/contexts/framework/application/uninstall-plugin.unit.test.ts index 34f77639f..dad3f2454 100644 --- a/cli/tests/contexts/framework/application/uninstall-plugin.unit.test.ts +++ b/cli/tests/contexts/framework/application/uninstall-plugin.unit.test.ts @@ -48,7 +48,6 @@ describe("UninstallUseCase — plugin scope", () => { await new UninstallUseCase(deps.fs, deps.manifestRepo, deps.logger).execute({ toolIds: [], projectRoot: PROJECT_ROOT, - mcpFilter: [], pluginName: "sample-plugin", }); @@ -65,7 +64,6 @@ describe("UninstallUseCase — plugin scope", () => { new UninstallUseCase(deps.fs, deps.manifestRepo, deps.logger).execute({ toolIds: [], projectRoot: PROJECT_ROOT, - mcpFilter: [], pluginName: "nonexistent", }) ).rejects.toThrow(PluginNotFoundError); diff --git a/cli/tests/contexts/framework/application/uninstall-use-case.unit.test.ts b/cli/tests/contexts/framework/application/uninstall-use-case.unit.test.ts index 167230af4..7f85cc001 100644 --- a/cli/tests/contexts/framework/application/uninstall-use-case.unit.test.ts +++ b/cli/tests/contexts/framework/application/uninstall-use-case.unit.test.ts @@ -7,7 +7,6 @@ import "../../../../src/contexts/tools/domain/profiles/cursor/profile.js"; import "../../../../src/contexts/tools/domain/profiles/opencode/profile.js"; import "../../../../src/contexts/tools/domain/profiles/vscode/profile.js"; import { UninstallUseCase } from "../../../../src/contexts/framework/application/uninstall/uninstall-use-case.js"; -import { Manifest } from "../../../../src/contexts/framework/domain/manifest.js"; import { InputRequiredError, NoManifestError, @@ -15,10 +14,6 @@ import { } from "../../../../src/kernel/errors.js"; import type { ToolId } from "../../../../src/kernel/tool.js"; import { buildUnitDeps, initProject, installTool } from "../../../helpers/ports/build-unit-deps.js"; -import { CapturingLogger } from "../../../helpers/ports/capturing-logger.js"; -import { DeterministicHasher } from "../../../helpers/ports/deterministic-hasher.js"; -import { InMemoryFileAdapter } from "../../../helpers/ports/in-memory-file-adapter.js"; -import { InMemoryManifestRepository } from "../../../helpers/ports/in-memory-manifest-repository.js"; const PROJECT_ROOT = "/test-project"; @@ -32,7 +27,6 @@ describe("uninstall", () => { await useCase.execute({ toolIds: ["claude" as ToolId], projectRoot: PROJECT_ROOT, - mcpFilter: [], }); const manifest = await deps.manifestRepo.load(); @@ -51,7 +45,7 @@ describe("uninstall", () => { const useCase = new UninstallUseCase(deps.fs, deps.manifestRepo, deps.logger); await expect( - useCase.execute({ toolIds: ["claude" as ToolId], projectRoot: PROJECT_ROOT, mcpFilter: [] }) + useCase.execute({ toolIds: ["claude" as ToolId], projectRoot: PROJECT_ROOT }) ).resolves.not.toThrow(); }); @@ -68,7 +62,6 @@ describe("uninstall", () => { await useCase.execute({ toolIds: ["claude" as ToolId], projectRoot: PROJECT_ROOT, - mcpFilter: [], }); expect(deps.fs.has(sharedFile)).toBe(true); @@ -87,7 +80,6 @@ describe("uninstall", () => { await useCase.execute({ toolIds: ["vscode" as ToolId], projectRoot: PROJECT_ROOT, - mcpFilter: [], }); expect(deps.fs.has(settingsPath)).toBe(false); @@ -105,30 +97,11 @@ describe("uninstall", () => { await useCase.execute({ toolIds: ["vscode" as ToolId], projectRoot: PROJECT_ROOT, - mcpFilter: [], }); expect(deps.fs.has(keybindingsPath)).toBe(false); }); }); - - describe("MCP removal", () => { - it("full tool removal still works without mcpFilter", async () => { - const deps = await buildUnitDeps(PROJECT_ROOT); - await initProject(deps, PROJECT_ROOT); - await installTool(deps, PROJECT_ROOT, "claude" as ToolId); - - const useCase = new UninstallUseCase(deps.fs, deps.manifestRepo, deps.logger); - await useCase.execute({ - toolIds: ["claude" as ToolId], - projectRoot: PROJECT_ROOT, - mcpFilter: [], - }); - - const manifest = await deps.manifestRepo.load(); - expect(manifest?.getInstalledToolIds()).not.toContain("claude"); - }); - }); }); describe("uninstall — refusals", () => { @@ -140,7 +113,6 @@ describe("uninstall — refusals", () => { new UninstallUseCase(deps.fs, deps.manifestRepo, deps.logger).execute({ toolIds: [], projectRoot: PROJECT_ROOT, - mcpFilter: [], }) ).rejects.toThrow( new InputRequiredError( @@ -156,7 +128,6 @@ describe("uninstall — refusals", () => { new UninstallUseCase(deps.fs, deps.manifestRepo, deps.logger).execute({ toolIds: ["claude"], projectRoot: PROJECT_ROOT, - mcpFilter: [], }) ).rejects.toThrow(NoManifestError); }); @@ -169,45 +140,7 @@ describe("uninstall — refusals", () => { new UninstallUseCase(deps.fs, deps.manifestRepo, deps.logger).execute({ toolIds: ["claude"], projectRoot: PROJECT_ROOT, - mcpFilter: [], }) ).rejects.toThrow(ToolNotInstalledError); }); }); - -describe("uninstall — an MCP filter", () => { - it("strips the named entries and leaves the tool installed", async () => { - const hasher = new DeterministicHasher(); - const servers = { github: { command: "gh" }, playwright: { command: "npx" } }; - const fs = new InMemoryFileAdapter( - { [join(PROJECT_ROOT, ".mcp.json")]: JSON.stringify({ mcpServers: servers }) }, - hasher - ); - const manifest = Manifest.create(); - manifest.addTool( - "claude", - "test", - [], - [ - { - relativePath: ".mcp.json", - sectionKey: "mcpServers", - entries: { - github: hasher.hash(JSON.stringify(servers.github)), - playwright: hasher.hash(JSON.stringify(servers.playwright)), - }, - }, - ] - ); - const repo = new InMemoryManifestRepository(manifest); - - const results = await new UninstallUseCase(fs, repo, new CapturingLogger()).execute({ - toolIds: ["claude"], - projectRoot: PROJECT_ROOT, - mcpFilter: ["github"], - }); - - expect(results).toStrictEqual([{ toolId: "claude", fileCount: 1, deletedFiles: ["github"] }]); - expect(repo.getCurrent()?.hasTool("claude")).toBe(true); - }); -}); diff --git a/cli/tests/contexts/framework/application/uninstall/uninstall-mcp-exclusion-use-case.unit.test.ts b/cli/tests/contexts/framework/application/uninstall/uninstall-mcp-exclusion-use-case.unit.test.ts deleted file mode 100644 index 6a122cbf3..000000000 --- a/cli/tests/contexts/framework/application/uninstall/uninstall-mcp-exclusion-use-case.unit.test.ts +++ /dev/null @@ -1,108 +0,0 @@ -import { join } from "node:path"; -import { describe, expect, it } from "vitest"; -import { UninstallMcpExclusionUseCase } from "../../../../../src/contexts/framework/application/uninstall/uninstall-mcp-exclusion-use-case.js"; -import { Manifest } from "../../../../../src/contexts/framework/domain/manifest.js"; -import type { FileHash } from "../../../../../src/kernel/file.js"; -import { CapturingLogger } from "../../../../helpers/ports/capturing-logger.js"; -import { DeterministicHasher } from "../../../../helpers/ports/deterministic-hasher.js"; -import { InMemoryFileAdapter } from "../../../../helpers/ports/in-memory-file-adapter.js"; - -const PROJECT_ROOT = "/test-project"; -const MCP = ".mcp.json"; -const SECTION = "mcpServers"; -const hasher = new DeterministicHasher(); -const GITHUB = { command: "gh" }; -const PLAYWRIGHT = { command: "npx" }; -const CONTENT = JSON.stringify({ [SECTION]: { github: GITHUB, playwright: PLAYWRIGHT } }); - -function hashOf(value: unknown): FileHash { - return hasher.hash(JSON.stringify(value)); -} - -function claudeMerging( - content: string, - entries: Record, - sectionKey: string | null = SECTION -): { fs: InMemoryFileAdapter; manifest: Manifest } { - const fs = new InMemoryFileAdapter({ [join(PROJECT_ROOT, MCP)]: content }, hasher); - const manifest = Manifest.create(); - manifest.addTool("claude", "test", [], [{ relativePath: MCP, sectionKey, entries }]); - return { fs, manifest }; -} - -function twoServers() { - return claudeMerging(CONTENT, { github: hashOf(GITHUB), playwright: hashOf(PLAYWRIGHT) }); -} - -function exclude( - project: { fs: InMemoryFileAdapter; manifest: Manifest }, - mcpFilter: string[], - logger = new CapturingLogger() -) { - return new UninstallMcpExclusionUseCase(project.fs, logger).execute({ - toolId: "claude", - manifest: project.manifest, - projectRoot: PROJECT_ROOT, - mcpFilter, - }); -} - -describe("UninstallMcpExclusionUseCase", () => { - it("announces the tool it strips entries from", async () => { - const logger = new CapturingLogger(); - - await exclude(twoServers(), ["github"], logger); - - expect(logger.infoMessages).toStrictEqual(["Removing MCP entries from claude..."]); - }); - - it("removes the named entries that exist and reports exactly those", async () => { - const project = twoServers(); - - const result = await exclude(project, ["github", "absent"]); - - expect(result).toStrictEqual({ toolId: "claude", fileCount: 1, deletedFiles: ["github"] }); - expect(JSON.parse(project.fs.getFile(join(PROJECT_ROOT, MCP)) ?? "")).toStrictEqual({ - [SECTION]: { playwright: PLAYWRIGHT }, - }); - }); - - it("stops tracking the entries it removed", async () => { - const project = twoServers(); - - await exclude(project, ["github"]); - - expect(project.manifest.getMergeFiles("claude")).toStrictEqual([ - { relativePath: MCP, sectionKey: SECTION, entries: { playwright: hashOf(PLAYWRIGHT) } }, - ]); - }); - - it("records each removal so a later install leaves the entry out", async () => { - const project = twoServers(); - - await exclude(project, ["github"]); - - expect(project.manifest.getExcludedMcp("claude")).toStrictEqual([ - { configPath: MCP, entryKey: "github" }, - ]); - }); - - it("leaves a file byte-identical when none of the named entries is in it", async () => { - const project = twoServers(); - - const result = await exclude(project, ["absent"]); - - expect(result).toStrictEqual({ toolId: "claude", fileCount: 0, deletedFiles: [] }); - expect(project.fs.getFile(join(PROJECT_ROOT, MCP))).toBe(CONTENT); - }); - - it("leaves a file tracked without a section untouched even when it holds a named key", async () => { - const content = JSON.stringify({ github: GITHUB }); - const project = claudeMerging(content, { github: hashOf(GITHUB) }, null); - - const result = await exclude(project, ["github"]); - - expect(result).toStrictEqual({ toolId: "claude", fileCount: 0, deletedFiles: [] }); - expect(project.fs.getFile(join(PROJECT_ROOT, MCP))).toBe(content); - }); -}); diff --git a/cli/tests/contexts/framework/domain/manifest-round-trip.unit.test.ts b/cli/tests/contexts/framework/domain/manifest-round-trip.unit.test.ts index 3fb07bbd6..5ef74d949 100644 --- a/cli/tests/contexts/framework/domain/manifest-round-trip.unit.test.ts +++ b/cli/tests/contexts/framework/domain/manifest-round-trip.unit.test.ts @@ -28,7 +28,6 @@ describe("Manifest round-trip: every fixture rewrites byte-identical", () => { const names = fixtureNames(); expect(names).toContain("multi-tool.json"); expect(names).toContain("merge-files.json"); - expect(names).toContain("mcp-exclusions.json"); expect(names).toContain("plugins.json"); expect(names).toContain("full.json"); }); diff --git a/cli/tests/contexts/framework/domain/manifest.unit.test.ts b/cli/tests/contexts/framework/domain/manifest.unit.test.ts index 66283a081..862b39193 100644 --- a/cli/tests/contexts/framework/domain/manifest.unit.test.ts +++ b/cli/tests/contexts/framework/domain/manifest.unit.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it } from "vitest"; import { Manifest } from "../../../../src/contexts/framework/domain/manifest.js"; import { InstalledPlugin } from "../../../../src/contexts/framework/domain/plugins/installed-plugin.js"; -import type { McpExclusion } from "../../../../src/contexts/tools/domain/mcp-exclusion.js"; import { InvalidManifestDataError, ToolNotInManifestError } from "../../../../src/kernel/errors.js"; import { FileHash, InstallationFile } from "../../../../src/kernel/file.js"; import type { MergeFileEntry } from "../../../../src/kernel/merge.js"; @@ -199,87 +198,7 @@ describe("Manifest", () => { }); }); - describe("MCP exclusion tracking", () => { - const exclusionA: McpExclusion = { configPath: ".mcp.json", entryKey: "playwright" }; - const exclusionB: McpExclusion = { configPath: ".mcp.json", entryKey: "github" }; - - it("addTool with excludedMcp stores exclusions", () => { - const manifest = Manifest.create(); - manifest.addTool("claude" as ToolId, "3.0.0", claudeFiles, [], [exclusionA]); - expect(manifest.getExcludedMcp("claude" as ToolId)).toEqual([exclusionA]); - }); - - it("getExcludedMcp returns empty array for tool without exclusions", () => { - const manifest = Manifest.create(); - manifest.addTool("claude" as ToolId, "3.0.0", claudeFiles); - expect(manifest.getExcludedMcp("claude" as ToolId)).toEqual([]); - }); - - it("addExcludedMcp appends and deduplicates", () => { - const manifest = Manifest.create(); - manifest.addTool("claude" as ToolId, "3.0.0", claudeFiles); - manifest.addExcludedMcp("claude" as ToolId, [exclusionA]); - manifest.addExcludedMcp("claude" as ToolId, [exclusionA, exclusionB]); - const result = manifest.getExcludedMcp("claude" as ToolId); - expect(result).toHaveLength(2); - expect(result).toEqual([exclusionA, exclusionB]); - }); - - it("addExcludedMcp throws for uninstalled tool", () => { - const manifest = Manifest.create(); - expect(() => manifest.addExcludedMcp("claude" as ToolId, [exclusionA])).toThrow( - /not installed/ - ); - }); - - it("removeExcludedMcp removes matching entries", () => { - const manifest = Manifest.create(); - manifest.addTool("claude" as ToolId, "3.0.0", claudeFiles, [], [exclusionA, exclusionB]); - manifest.removeExcludedMcp("claude" as ToolId, [exclusionA]); - expect(manifest.getExcludedMcp("claude" as ToolId)).toEqual([exclusionB]); - }); - - it("removeExcludedMcp throws for uninstalled tool", () => { - const manifest = Manifest.create(); - expect(() => manifest.removeExcludedMcp("claude" as ToolId, [exclusionA])).toThrow( - /not installed/ - ); - }); - - it("clearExcludedMcp empties the list", () => { - const manifest = Manifest.create(); - manifest.addTool("claude" as ToolId, "3.0.0", claudeFiles, [], [exclusionA, exclusionB]); - manifest.clearExcludedMcp("claude" as ToolId); - expect(manifest.getExcludedMcp("claude" as ToolId)).toEqual([]); - }); - - it("clearExcludedMcp throws for uninstalled tool", () => { - const manifest = Manifest.create(); - expect(() => manifest.clearExcludedMcp("claude" as ToolId)).toThrow(/not installed/); - }); - - it("toJSON/fromJSON round-trip preserves excludedMcp", () => { - const manifest = Manifest.create(); - manifest.addTool("claude" as ToolId, "3.0.0", claudeFiles, [], [exclusionA, exclusionB]); - const restored = Manifest.fromJSON(manifest.toJSON()); - expect(restored.getExcludedMcp("claude" as ToolId)).toEqual([exclusionA, exclusionB]); - }); - - it("fromJSON handles missing excludedMcp (backward compat)", () => { - const manifest = Manifest.create(); - manifest.addTool("claude" as ToolId, "3.0.0", claudeFiles); - const json = manifest.toJSON(); - const restored = Manifest.fromJSON(json); - expect(restored.getExcludedMcp("claude" as ToolId)).toEqual([]); - }); - - it("toJSON omits excludedMcp when empty", () => { - const manifest = Manifest.create(); - manifest.addTool("claude" as ToolId, "3.0.0", claudeFiles); - const json = manifest.toJSON(); - expect(json.tools.claude).not.toHaveProperty("excludedMcp"); - }); - + describe("updateToolMergeFiles()", () => { it("updateToolMergeFiles replaces merge files without touching regular files", () => { const mergeEntry: MergeFileEntry = { relativePath: ".mcp.json", @@ -287,7 +206,7 @@ describe("Manifest", () => { entries: { playwright: makeHash("aabb") }, }; const manifest = Manifest.create(); - manifest.addTool("claude" as ToolId, "3.0.0", claudeFiles, [mergeEntry], [exclusionA]); + manifest.addTool("claude" as ToolId, "3.0.0", claudeFiles, [mergeEntry]); const updatedMerge: MergeFileEntry = { relativePath: ".mcp.json", sectionKey: "mcpServers", @@ -296,7 +215,6 @@ describe("Manifest", () => { manifest.updateToolMergeFiles("claude" as ToolId, [updatedMerge]); expect(manifest.getMergeFiles("claude" as ToolId)).toEqual([updatedMerge]); expect(manifest.getToolFiles("claude" as ToolId)).toHaveLength(2); - expect(manifest.getExcludedMcp("claude" as ToolId)).toEqual([exclusionA]); }); it("updateToolMergeFiles throws for uninstalled tool", () => { @@ -494,10 +412,6 @@ describe("Manifest", () => { expect(Manifest.create().getToolFiles("claude" as ToolId)).toStrictEqual([]); }); - it("has no MCP exclusions", () => { - expect(Manifest.create().getExcludedMcp("claude" as ToolId)).toStrictEqual([]); - }); - it("has no native registrations", () => { expect(Manifest.create().getNativeRegistrations("claude" as ToolId)).toBeUndefined(); }); @@ -585,47 +499,31 @@ describe("Manifest", () => { }); }); - describe("clearExcludedMcp()", () => { - it("keeps the tool's version and files", () => { - const manifest = Manifest.create(); - manifest.addTool( - "claude" as ToolId, - "3.0.0", - claudeFiles, - [], - [{ configPath: ".mcp.json", entryKey: "playwright" }] - ); - - manifest.clearExcludedMcp("claude" as ToolId); - - expect(manifest.getToolVersion("claude" as ToolId)).toBe("3.0.0"); - expect(manifest.getToolFiles("claude" as ToolId).map((f) => f.relativePath)).toStrictEqual([ - ".claude/agents/code-reviewer.md", - ".claude/rules/naming.md", - ]); - }); - }); - - describe("updateToolMergeFiles()", () => { - const exclusion: McpExclusion = { configPath: ".mcp.json", entryKey: "playwright" }; - - it("replaces the exclusions when handed some", () => { - const manifest = Manifest.create(); - manifest.addTool("claude" as ToolId, "3.0.0", claudeFiles, [], [exclusion]); - const replacement: McpExclusion = { configPath: ".mcp.json", entryKey: "github" }; - - manifest.updateToolMergeFiles("claude" as ToolId, [], [replacement]); + describe("a manifest written while the CLI still recorded MCP exclusions", () => { + const written = { + version: 8, + tools: { + claude: { + toolId: "claude", + version: "1.0.0", + files: [], + mergeFiles: [], + excludedMcp: [{ configPath: ".claude/settings.json", entryKey: "old-server" }], + }, + }, + }; - expect(manifest.getExcludedMcp("claude" as ToolId)).toStrictEqual([replacement]); + it("still loads, keeping the tool it records", () => { + expect(Manifest.fromJSON(written).getToolVersion("claude" as ToolId)).toBe("1.0.0"); }); - it("keeps the exclusions when handed none", () => { - const manifest = Manifest.create(); - manifest.addTool("claude" as ToolId, "3.0.0", claudeFiles, [], [exclusion]); - - manifest.updateToolMergeFiles("claude" as ToolId, []); - - expect(manifest.getExcludedMcp("claude" as ToolId)).toStrictEqual([exclusion]); + it("drops the exclusions on its next write", () => { + expect(Manifest.fromJSON(written).toJSON().tools.claude).toStrictEqual({ + toolId: "claude", + version: "1.0.0", + files: [], + mergeFiles: [], + }); }); }); diff --git a/cli/tests/contexts/framework/domain/manifest/mcp-exclusions.unit.test.ts b/cli/tests/contexts/framework/domain/manifest/mcp-exclusions.unit.test.ts deleted file mode 100644 index d4f33fd24..000000000 --- a/cli/tests/contexts/framework/domain/manifest/mcp-exclusions.unit.test.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { - addExclusions, - removeExclusions, -} from "../../../../../src/contexts/framework/domain/manifest/mcp-exclusions.js"; -import type { McpExclusion } from "../../../../../src/contexts/tools/domain/mcp-exclusion.js"; - -const playwright: McpExclusion = { configPath: ".mcp.json", entryKey: "playwright" }; -const github: McpExclusion = { configPath: ".mcp.json", entryKey: "github" }; -const context7: McpExclusion = { configPath: ".mcp.json", entryKey: "context7" }; - -describe("MCP exclusions recorded for a tool", () => { - it("keeps the exclusions already recorded ahead of the ones added", () => { - expect(addExclusions([playwright], [github])).toStrictEqual([playwright, github]); - }); - - it("removes exactly the exclusions named, keeping the rest", () => { - expect(removeExclusions([playwright, github, context7], [playwright, context7])).toStrictEqual([ - github, - ]); - }); -}); diff --git a/cli/tests/contexts/framework/domain/manifest/tool-entry.unit.test.ts b/cli/tests/contexts/framework/domain/manifest/tool-entry.unit.test.ts index c0fe68116..55bcacd7a 100644 --- a/cli/tests/contexts/framework/domain/manifest/tool-entry.unit.test.ts +++ b/cli/tests/contexts/framework/domain/manifest/tool-entry.unit.test.ts @@ -34,7 +34,6 @@ const makeEntry = (plugins: InstalledPlugin[]): ToolEntry => }), ], mergeFiles: [{ relativePath: ".mcp.json", sectionKey: "mcpServers", entries: {} }], - excludedMcp: [], existingPlugins: plugins, }); diff --git a/cli/tests/contexts/tools/domain/mcp-exclusion.unit.test.ts b/cli/tests/contexts/tools/domain/mcp-exclusion.unit.test.ts index 960de5f14..8f398814c 100644 --- a/cli/tests/contexts/tools/domain/mcp-exclusion.unit.test.ts +++ b/cli/tests/contexts/tools/domain/mcp-exclusion.unit.test.ts @@ -1,8 +1,5 @@ import { describe, expect, it } from "vitest"; -import { - mcpExclusionEquals, - transformFor, -} from "../../../../src/contexts/tools/domain/mcp-exclusion.js"; +import { transformFor } from "../../../../src/contexts/tools/domain/mcp-exclusion.js"; function makeConfig(servers: Record): string { return JSON.stringify({ mcpServers: servers }, null, 2); @@ -94,13 +91,3 @@ describe("the win32 transform on a config without servers", () => { ); }); }); - -describe("mcpExclusionEquals", () => { - it("is equal only when both the config path and the entry key match", () => { - const one = { configPath: ".mcp.json", entryKey: "a" }; - - expect(mcpExclusionEquals(one, { configPath: ".mcp.json", entryKey: "a" })).toBe(true); - expect(mcpExclusionEquals(one, { configPath: ".mcp.json", entryKey: "b" })).toBe(false); - expect(mcpExclusionEquals(one, { configPath: "other.json", entryKey: "a" })).toBe(false); - }); -}); diff --git a/cli/tests/fixtures/manifests/full.json b/cli/tests/fixtures/manifests/full.json index cd555c5b8..cb115c2b0 100644 --- a/cli/tests/fixtures/manifests/full.json +++ b/cli/tests/fixtures/manifests/full.json @@ -24,12 +24,6 @@ } } ], - "excludedMcp": [ - { - "configPath": ".claude/settings.json", - "entryKey": "old-server" - } - ], "plugins": [ { "name": "aidd-dev", diff --git a/cli/tests/fixtures/manifests/mcp-exclusions.json b/cli/tests/fixtures/manifests/mcp-exclusions.json deleted file mode 100644 index b7d002942..000000000 --- a/cli/tests/fixtures/manifests/mcp-exclusions.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "version": 8, - "tools": { - "claude": { - "toolId": "claude", - "version": "1.0.0", - "files": [], - "mergeFiles": [], - "excludedMcp": [ - { - "configPath": ".claude/settings.json", - "entryKey": "old-server" - }, - { - "configPath": ".claude/settings.json", - "entryKey": "legacy-server" - } - ] - } - } -} diff --git a/cli/tests/presentation/commands/framework-wiring.integration.test.ts b/cli/tests/presentation/commands/framework-wiring.integration.test.ts index 277bb2f70..2812760bd 100644 --- a/cli/tests/presentation/commands/framework-wiring.integration.test.ts +++ b/cli/tests/presentation/commands/framework-wiring.integration.test.ts @@ -192,7 +192,7 @@ describe("aidd framework install", () => { }); describe("aidd framework remove", () => { - it("removes one AI tool with no MCP narrowing, and counts every file that went", async () => { + it("removes one AI tool, and counts every file that went", async () => { uninstallAiTools.mockResolvedValue([ { toolId: "claude", fileCount: 4 }, { toolId: "claude", fileCount: 3 }, @@ -202,7 +202,6 @@ describe("aidd framework remove", () => { expect(uninstallAiTools).toHaveBeenCalledWith({ toolIds: ["claude"], projectRoot: PROJECT_ROOT, - mcpFilter: [], }); expect(uninstallIdeTool).not.toHaveBeenCalled(); }); From 1b5c17f4cf87859a087bef23be8de30fe69b6000 Mon Sep 17 00:00:00 2001 From: Baptiste LAFOURCADE Date: Thu, 10 Sep 2026 10:10:46 +0200 Subject: [PATCH 2/2] docs(cli): stop listing mcp-exclusions among the manifest members The framework skill's concept table named a module the previous commit deleted. Refs #806 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011x4ms5qcGuZgYhCxfdHMUb AIDD-Session-Id: 4acc9a1c-19bc-4468-b8b6-e86644bcba60 --- cli/.claude/skills/framework/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/.claude/skills/framework/SKILL.md b/cli/.claude/skills/framework/SKILL.md index 45c2ac67d..cad1cfce3 100644 --- a/cli/.claude/skills/framework/SKILL.md +++ b/cli/.claude/skills/framework/SKILL.md @@ -24,7 +24,7 @@ disk is exactly the job that needs all three. | Concept | Location | |---|---| -| The manifest aggregate and its members | `domain/manifest.ts`, `domain/manifest/` (tool-entry, tracked-files, merge-files, mcp-exclusions, native-registrations) | +| The manifest aggregate and its members | `domain/manifest.ts`, `domain/manifest/` (tool-entry, tracked-files, merge-files, native-registrations) | | A plugin's declared state | `domain/plugins/` (installed-plugin, source-resolver, requested-version-policy) | | The diagnosis shape | `domain/doctor.ts` | | Setup orchestration state | `domain/setup-flow.ts` |