diff --git a/src/e2e/e2e-mcp.spec.ts b/src/e2e/e2e-mcp.spec.ts index 8864ae00f..42123e250 100644 --- a/src/e2e/e2e-mcp.spec.ts +++ b/src/e2e/e2e-mcp.spec.ts @@ -5,7 +5,7 @@ import { setTimeout } from "node:timers/promises"; import { describe, expect, it } from "vitest"; import { RULESYNC_MCP_RELATIVE_FILE_PATH } from "../constants/rulesync-paths.js"; -import { readFileContent, writeFileContent } from "../utils/file.js"; +import { fileExists, readFileContent, writeFileContent } from "../utils/file.js"; import { runGenerate, runImport, @@ -221,7 +221,7 @@ describe("E2E: mcp (global mode)", () => { const { getProjectDir, getHomeDir } = useGlobalTestDirectories(); it.each([ - { target: "claudecode", outputPath: join(".claude", ".claude.json") }, + { target: "claudecode", outputPath: ".claude.json" }, { target: "cursor", outputPath: join(".cursor", "mcp.json") }, { target: "geminicli", outputPath: join(".gemini", "settings.json") }, { target: "opencode", outputPath: join(".config", "opencode", "opencode.jsonc") }, @@ -266,6 +266,78 @@ describe("E2E: mcp (global mode)", () => { expect(generatedContent).toContain("test-server"); }); + it("should preserve legacy ~/.claude/.claude.json when writing to recommended path (global)", async () => { + // Pins both behaviors end-to-end: (a) canonical ~/.claude.json receives + // fresh mcpServers AND preserves Claude Code's own user-config keys via + // RMW; (b) legacy ~/.claude/.claude.json is byte-identical after + // generate (matches the no-destructive-action invariant from PR #333). + const projectDir = getProjectDir(); + const homeDir = getHomeDir(); + + // Pre-seed the canonical ~/.claude.json with Claude Code's own keys. + await writeFileContent( + join(homeDir, ".claude.json"), + JSON.stringify( + { + mcpServers: { "previously-managed": { command: "node" } }, + projects: { "/home/user/proj-a": { allowedTools: ["*"] } }, + feedbackSurveyState: { lastShownAt: 1234567890 }, + }, + null, + 2, + ), + ); + + // Pre-seed a legacy orphan with specific content for byte-identical check. + const legacyPath = join(homeDir, ".claude", ".claude.json"); + const legacyContent = JSON.stringify( + { mcpServers: { "stale-server": { command: "node", args: ["stale.js"] } } }, + null, + 2, + ); + await writeFileContent(legacyPath, legacyContent); + + // Source: a fresh server in .rulesync/mcp.json. + const mcpContent = JSON.stringify( + { + root: true, + mcpServers: { + "test-server": { + description: "Test MCP server", + type: "stdio", + command: "echo", + args: ["hello"], + env: {}, + }, + }, + }, + null, + 2, + ); + await writeFileContent(join(projectDir, RULESYNC_MCP_RELATIVE_FILE_PATH), mcpContent); + + await runGenerate({ + target: "claudecode", + features: "mcp", + global: true, + env: { HOME_DIR: homeDir }, + }); + + // Canonical ~/.claude.json has fresh mcpServers from rulesync and + // retains Claude Code's own user-config keys via RMW spread. + const newContent = await readFileContent(join(homeDir, ".claude.json")); + expect(newContent).toContain("test-server"); + expect(newContent).not.toContain("previously-managed"); + expect(newContent).toContain("projects"); + expect(newContent).toContain("/home/user/proj-a"); + expect(newContent).toContain("feedbackSurveyState"); + expect(newContent).toContain("1234567890"); + + // Legacy file is preserved byte-for-byte. rulesync never modifies it. + expect(await fileExists(legacyPath)).toBe(true); + expect(await readFileContent(legacyPath)).toBe(legacyContent); + }); + it("should ignore non-root mcp in global mode", async () => { const projectDir = getProjectDir(); const homeDir = getHomeDir(); @@ -314,7 +386,7 @@ describe("E2E: mcp (global mode)", () => { }); // Verify: root mcp content is present, non-root mcp content is absent - const generatedContent = await readFileContent(join(homeDir, ".claude", ".claude.json")); + const generatedContent = await readFileContent(join(homeDir, ".claude.json")); expect(generatedContent).toContain("root-server"); expect(generatedContent).not.toContain("non-root-server"); }); diff --git a/src/features/mcp/claudecode-mcp.test.ts b/src/features/mcp/claudecode-mcp.test.ts index 9b6f285cd..ef4c0f5f0 100644 --- a/src/features/mcp/claudecode-mcp.test.ts +++ b/src/features/mcp/claudecode-mcp.test.ts @@ -6,9 +6,11 @@ import { RULESYNC_MCP_SCHEMA_URL, RULESYNC_RELATIVE_DIR_PATH, } from "../../constants/rulesync-paths.js"; +import { createMockLogger } from "../../test-utils/mock-logger.js"; import { setupTestDirectory } from "../../test-utils/test-directories.js"; -import { ensureDir, writeFileContent } from "../../utils/file.js"; +import { ensureDir, fileExists, readFileContent, writeFileContent } from "../../utils/file.js"; import { ClaudecodeMcp } from "./claudecode-mcp.js"; +import { McpProcessor } from "./mcp-processor.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; describe("ClaudecodeMcp", () => { @@ -34,9 +36,16 @@ describe("ClaudecodeMcp", () => { }); it("should return correct paths for global mode", () => { + // Per Claude Code docs (https://docs.claude.com/en/docs/claude-code/mcp), + // user-scope MCP servers are stored in ~/.claude.json at HOME root — + // NOT inside ~/.claude/. The earlier (≤ v8.17.0) path of + // `.claude/.claude.json` is now treated as a legacy read-only + // fallback in `fromFile` (with a deprecation warning) and is + // never modified by rulesync. Mirrors the backward-compatibility + // pattern from PR #333 (`RulesyncMcp.fromFile`). const paths = ClaudecodeMcp.getSettablePaths({ global: true }); - expect(paths.relativeDirPath).toBe(".claude"); + expect(paths.relativeDirPath).toBe("."); expect(paths.relativeFilePath).toBe(".claude.json"); }); }); @@ -55,7 +64,7 @@ describe("ClaudecodeMcp", () => { it("should return false in global mode", () => { const claudecodeMcp = new ClaudecodeMcp({ - relativeDirPath: ".claude", + relativeDirPath: ".", relativeFilePath: ".claude.json", fileContent: JSON.stringify({ mcpServers: {} }), global: true, @@ -66,7 +75,7 @@ describe("ClaudecodeMcp", () => { it("should return false when created via forDeletion with global: true", () => { const claudecodeMcp = ClaudecodeMcp.forDeletion({ - relativeDirPath: ".claude", + relativeDirPath: ".", relativeFilePath: ".claude.json", global: true, }); @@ -305,11 +314,7 @@ describe("ClaudecodeMcp", () => { }, }, }; - await ensureDir(join(testDir, ".claude")); - await writeFileContent( - join(testDir, ".claude/.claude.json"), - JSON.stringify(jsonData, null, 2), - ); + await writeFileContent(join(testDir, ".claude.json"), JSON.stringify(jsonData, null, 2)); const claudecodeMcp = await ClaudecodeMcp.fromFile({ outputRoot: testDir, @@ -318,7 +323,7 @@ describe("ClaudecodeMcp", () => { expect(claudecodeMcp).toBeInstanceOf(ClaudecodeMcp); expect(claudecodeMcp.getJson()).toEqual(jsonData); - expect(claudecodeMcp.getFilePath()).toBe(join(testDir, ".claude/.claude.json")); + expect(claudecodeMcp.getFilePath()).toBe(join(testDir, ".claude.json")); expect(claudecodeMcp.isDeletable()).toBe(false); }); @@ -350,7 +355,7 @@ describe("ClaudecodeMcp", () => { expect(claudecodeMcp).toBeInstanceOf(ClaudecodeMcp); expect(claudecodeMcp.getJson()).toEqual({ mcpServers: {} }); - expect(claudecodeMcp.getFilePath()).toBe(join(testDir, ".claude/.claude.json")); + expect(claudecodeMcp.getFilePath()).toBe(join(testDir, ".claude.json")); expect(claudecodeMcp.isDeletable()).toBe(false); }); @@ -368,9 +373,8 @@ describe("ClaudecodeMcp", () => { }, version: "1.0.0", }; - await ensureDir(join(testDir, ".claude")); await writeFileContent( - join(testDir, ".claude/.claude.json"), + join(testDir, ".claude.json"), JSON.stringify(existingGlobalConfig, null, 2), ); @@ -392,6 +396,183 @@ describe("ClaudecodeMcp", () => { }); expect((json as any).version).toBe("1.0.0"); }); + + it("should prefer recommended path when both recommended and legacy exist (global)", async () => { + // Mirrors the RulesyncMcp.fromFile precedent (PR #333). + const recommendedContent = { + mcpServers: { "recommended-server": { command: "node" } }, + }; + const legacyContent = { + mcpServers: { "legacy-server": { command: "node" } }, + }; + await writeFileContent(join(testDir, ".claude.json"), JSON.stringify(recommendedContent)); + await ensureDir(join(testDir, ".claude")); + await writeFileContent( + join(testDir, ".claude", ".claude.json"), + JSON.stringify(legacyContent), + ); + + const logger = createMockLogger(); + const claudecodeMcp = await ClaudecodeMcp.fromFile({ + outputRoot: testDir, + global: true, + logger, + }); + + expect(claudecodeMcp.getJson()).toEqual(recommendedContent); + expect(claudecodeMcp.getFilePath()).toBe(join(testDir, ".claude.json")); + // No deprecation warning when recommended is used. + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it("should fall back to legacy path with deprecation warning when only legacy exists (global)", async () => { + // Also asserts byte-identity: fromFile is read-only on legacy by + // contract. Mirrors PR #333. + const legacyContent = { + mcpServers: { "legacy-server": { command: "node", args: ["legacy.js"] } }, + }; + const legacyPath = join(testDir, ".claude", ".claude.json"); + const legacyOnDisk = JSON.stringify(legacyContent); + await ensureDir(join(testDir, ".claude")); + await writeFileContent(legacyPath, legacyOnDisk); + + const logger = createMockLogger(); + const claudecodeMcp = await ClaudecodeMcp.fromFile({ + outputRoot: testDir, + global: true, + logger, + }); + + // Data loaded from the legacy path. + expect(claudecodeMcp.getJson()).toEqual(legacyContent); + // Instance reflects the legacy path so callers see where data came from. + expect(claudecodeMcp.getFilePath()).toBe(join(testDir, ".claude", ".claude.json")); + + // Deprecation warning fired with both paths in the message. + expect(logger.warn).toHaveBeenCalledTimes(1); + const warnMessage = logger.warn.mock.calls[0]?.[0] as string; + expect(warnMessage).toContain(join(testDir, ".claude", ".claude.json")); + expect(warnMessage).toContain(join(testDir, ".claude.json")); + expect(warnMessage).toContain("deprecated"); + + // Read-only invariant: legacy file is byte-identical after read. + // Confirms fromFile never writes to the legacy path. + expect(await readFileContent(legacyPath)).toBe(legacyOnDisk); + }); + + it("should pass legacy fallback warnings through the processor path", async () => { + // Regression guard: `fromFile` warnings must surface through the + // real processor/CLI path, not just direct class calls. + const legacyContent = { + mcpServers: { "legacy-server": { command: "node", args: ["legacy.js"] } }, + }; + const legacyPath = join(testDir, ".claude", ".claude.json"); + await ensureDir(join(testDir, ".claude")); + await writeFileContent(legacyPath, JSON.stringify(legacyContent)); + + const logger = createMockLogger(); + const processor = new McpProcessor({ + outputRoot: testDir, + toolTarget: "claudecode", + global: true, + logger, + }); + + const toolFiles = await processor.loadToolFiles(); + + expect(toolFiles).toHaveLength(1); + expect(toolFiles[0]?.getFilePath()).toBe(legacyPath); + expect(logger.warn).toHaveBeenCalledTimes(1); + const warnMessage = logger.warn.mock.calls[0]?.[0] as string; + expect(warnMessage).toContain(legacyPath); + expect(warnMessage).toContain(join(testDir, ".claude.json")); + expect(warnMessage).toContain("deprecated"); + }); + + it("should NOT fall back to legacy path in local mode", async () => { + // Legacy fallback is a global-mode-only concession. Local mode + // reads `.mcp.json`, never `~/.claude/.claude.json`. + await ensureDir(join(testDir, ".claude")); + await writeFileContent( + join(testDir, ".claude", ".claude.json"), + JSON.stringify({ mcpServers: { "legacy-server": { command: "node" } } }), + ); + + const logger = createMockLogger(); + const claudecodeMcp = await ClaudecodeMcp.fromFile({ + outputRoot: testDir, + global: false, + logger, + }); + + expect(claudecodeMcp.getJson()).toEqual({ mcpServers: {} }); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it("should initialize empty mcpServers when neither file exists (global)", async () => { + // Fresh install: neither recommended nor legacy exists. fromFile + // returns an empty-mcpServers instance at the recommended path + // with no deprecation warning. + const logger = createMockLogger(); + const claudecodeMcp = await ClaudecodeMcp.fromFile({ + outputRoot: testDir, + global: true, + logger, + }); + + expect(claudecodeMcp.getJson()).toEqual({ mcpServers: {} }); + expect(claudecodeMcp.getFilePath()).toBe(join(testDir, ".claude.json")); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it("should work without a logger when reading the legacy path (global)", async () => { + // Pins the optional-logger contract. + const legacyContent = { mcpServers: { x: { command: "node" } } }; + await ensureDir(join(testDir, ".claude")); + await writeFileContent( + join(testDir, ".claude", ".claude.json"), + JSON.stringify(legacyContent), + ); + + const claudecodeMcp = await ClaudecodeMcp.fromFile({ + outputRoot: testDir, + global: true, + }); + + expect(claudecodeMcp.getJson()).toEqual(legacyContent); + }); + + it("should throw when only the legacy file exists and contains malformed JSON (global)", async () => { + // Pins the contract: corrupt legacy JSON throws on parse rather + // than silently falling through to an empty mcpServers. + await ensureDir(join(testDir, ".claude")); + await writeFileContent(join(testDir, ".claude", ".claude.json"), "{ broken json"); + + await expect( + ClaudecodeMcp.fromFile({ + outputRoot: testDir, + global: true, + }), + ).rejects.toThrow(); + }); + + it("should throw when recommended file is malformed even if legacy is valid (global)", async () => { + // Pins the contract: legacy fallback only triggers when the + // recommended file is *absent*, not when present-but-corrupt. + await writeFileContent(join(testDir, ".claude.json"), "{ broken json"); + await ensureDir(join(testDir, ".claude")); + await writeFileContent( + join(testDir, ".claude", ".claude.json"), + JSON.stringify({ mcpServers: { x: { command: "node" } } }), + ); + + await expect( + ClaudecodeMcp.fromFile({ + outputRoot: testDir, + global: true, + }), + ).rejects.toThrow(); + }); }); describe("fromRulesyncMcp", () => { @@ -535,7 +716,7 @@ describe("ClaudecodeMcp", () => { expect(claudecodeMcp).toBeInstanceOf(ClaudecodeMcp); expect(claudecodeMcp.getJson()).toEqual(jsonData); - expect(claudecodeMcp.getRelativeDirPath()).toBe(".claude"); + expect(claudecodeMcp.getRelativeDirPath()).toBe("."); expect(claudecodeMcp.getRelativeFilePath()).toBe(".claude.json"); expect(claudecodeMcp.isDeletable()).toBe(false); }); @@ -580,9 +761,8 @@ describe("ClaudecodeMcp", () => { }, version: "1.0.0", }; - await ensureDir(join(testDir, ".claude")); await writeFileContent( - join(testDir, ".claude/.claude.json"), + join(testDir, ".claude.json"), JSON.stringify(existingGlobalConfig, null, 2), ); @@ -629,9 +809,8 @@ describe("ClaudecodeMcp", () => { }, customProperty: "value", }; - await ensureDir(join(testDir, ".claude")); await writeFileContent( - join(testDir, ".claude/.claude.json"), + join(testDir, ".claude.json"), JSON.stringify(existingGlobalConfig, null, 2), ); @@ -673,6 +852,143 @@ describe("ClaudecodeMcp", () => { }); expect((json as any).customProperty).toBe("value"); }); + + it("should initialize ~/.claude.json on fresh install (no legacy, no existing config)", async () => { + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: ".mcp.json", + fileContent: JSON.stringify({ + mcpServers: { + "fresh-server": { command: "node", args: ["fresh.js"] }, + }, + }), + }); + + const claudecodeMcp = await ClaudecodeMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp, + global: true, + }); + + // Canonical file is the new path, populated with rulesync source. + expect(claudecodeMcp.getFilePath()).toBe(join(testDir, ".claude.json")); + expect(claudecodeMcp.getJson()).toEqual({ + mcpServers: { + "fresh-server": { command: "node", args: ["fresh.js"] }, + }, + }); + // Legacy path stays absent. + expect(await fileExists(join(testDir, ".claude", ".claude.json"))).toBe(false); + }); + + it("should NOT modify the legacy file when writing to recommended path (global)", async () => { + // Pins the no-destructive-action invariant (precedent PR #333). + await ensureDir(join(testDir, ".claude")); + const legacyContent = JSON.stringify( + { + mcpServers: { "stale-server": { command: "node", args: ["stale.js"] } }, + userAddedAtLegacyPath: { important: true }, + }, + null, + 2, + ); + const legacyPath = join(testDir, ".claude", ".claude.json"); + await writeFileContent(legacyPath, legacyContent); + + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: ".mcp.json", + fileContent: JSON.stringify({ + mcpServers: { "new-server": { command: "node", args: ["new.js"] } }, + }), + }); + + const claudecodeMcp = await ClaudecodeMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp, + global: true, + }); + + // Recommended path has fresh mcpServers from rulesync source. + expect(claudecodeMcp.getFilePath()).toBe(join(testDir, ".claude.json")); + expect(claudecodeMcp.getJson()).toEqual({ + mcpServers: { "new-server": { command: "node", args: ["new.js"] } }, + }); + // Legacy file unchanged — byte-for-byte. + const after = await readFileContent(legacyPath); + expect(after).toBe(legacyContent); + }); + + it("should not touch ~/.claude/.claude.json in local mode (content preserved verbatim)", async () => { + // Legacy file unchanged byte-for-byte in local mode. + await ensureDir(join(testDir, ".claude")); + const legacyContent = JSON.stringify({ + mcpServers: { other: { command: "node" } }, + }); + const legacyPath = join(testDir, ".claude", ".claude.json"); + await writeFileContent(legacyPath, legacyContent); + + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: ".mcp.json", + fileContent: JSON.stringify({ + mcpServers: { local: { command: "node" } }, + }), + }); + + await ClaudecodeMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp, + global: false, + }); + + // Legacy file content unchanged by local-mode generate. + const after = await readFileContent(legacyPath); + expect(after).toBe(legacyContent); + }); + + it("should preserve unrelated keys on ~/.claude.json (global mode)", async () => { + // RMW must preserve Claude Code's own keys (`projects`, `hooks`, + // `feedbackSurveyState`, etc.) while replacing only `mcpServers`. + const existingClaudeJson = { + mcpServers: { + "old-managed": { command: "node", args: ["old.js"] }, + }, + projects: { "/home/user/proj-a": { allowedTools: ["*"] } }, + hooks: { PreToolUse: [] }, + feedbackSurveyState: { lastShownAt: 1234567890 }, + }; + await writeFileContent( + join(testDir, ".claude.json"), + JSON.stringify(existingClaudeJson, null, 2), + ); + + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: ".mcp.json", + fileContent: JSON.stringify({ + mcpServers: { "fresh-server": { command: "python" } }, + }), + }); + + const claudecodeMcp = await ClaudecodeMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp, + global: true, + }); + + const json = claudecodeMcp.getJson(); + // mcpServers replaced with rulesync source. + expect(json.mcpServers).toEqual({ + "fresh-server": { command: "python" }, + }); + // Non-mcp keys preserved verbatim. + expect((json as any).projects).toEqual({ + "/home/user/proj-a": { allowedTools: ["*"] }, + }); + expect((json as any).hooks).toEqual({ PreToolUse: [] }); + expect((json as any).feedbackSurveyState).toEqual({ lastShownAt: 1234567890 }); + }); }); describe("toRulesyncMcp", () => { @@ -964,9 +1280,8 @@ describe("ClaudecodeMcp", () => { }, }, }; - await ensureDir(join(testDir, ".claude")); await writeFileContent( - join(testDir, ".claude/.claude.json"), + join(testDir, ".claude.json"), JSON.stringify(originalJsonData, null, 2), ); @@ -988,7 +1303,7 @@ describe("ClaudecodeMcp", () => { // Verify data integrity expect(newClaudecodeMcp.getJson()).toEqual(originalJsonData); - expect(newClaudecodeMcp.getFilePath()).toBe(join(testDir, ".claude/.claude.json")); + expect(newClaudecodeMcp.getFilePath()).toBe(join(testDir, ".claude.json")); }); }); @@ -1004,8 +1319,7 @@ describe("ClaudecodeMcp", () => { }); it("should handle malformed JSON in global config gracefully", async () => { - await ensureDir(join(testDir, ".claude")); - await writeFileContent(join(testDir, ".claude/.claude.json"), "{ invalid: json }"); + await writeFileContent(join(testDir, ".claude.json"), "{ invalid: json }"); await expect( ClaudecodeMcp.fromFile({ diff --git a/src/features/mcp/claudecode-mcp.ts b/src/features/mcp/claudecode-mcp.ts index c9cc8209b..a166ff4ee 100644 --- a/src/features/mcp/claudecode-mcp.ts +++ b/src/features/mcp/claudecode-mcp.ts @@ -1,7 +1,7 @@ import { join } from "node:path"; import { ValidationResult } from "../../types/ai-file.js"; -import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; +import { fileExists, readFileContent, readOrInitializeFileContent } from "../../utils/file.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { ToolMcp, @@ -25,18 +25,28 @@ export class ClaudecodeMcp extends ToolMcp { } /** - * In global mode, ~/.claude/.claude.json should not be deleted - * as it may contain other user settings. + * In global mode, ~/.claude.json should not be deleted as it is the + * user's primary Claude Code config and contains many other settings + * managed by Claude Code itself (feature flags, project trust list, + * hooks, user settings, model selection, etc.). * In local mode, .mcp.json can be safely deleted. */ override isDeletable(): boolean { return !this.global; } + /** + * Legacy global path used by rulesync ≤ v8.17.0. The documented store + * is `~/.claude.json`; `fromFile` falls back here with a deprecation + * warning (mirrors PR #333). Never modified or removed by rulesync. + */ + private static readonly LEGACY_GLOBAL_DIR = ".claude"; + private static readonly LEGACY_GLOBAL_FILE = ".claude.json"; + static getSettablePaths({ global }: { global?: boolean } = {}): ToolMcpSettablePaths { if (global) { return { - relativeDirPath: ".claude", + relativeDirPath: ".", relativeFilePath: ".claude.json", }; } @@ -50,20 +60,61 @@ export class ClaudecodeMcp extends ToolMcp { outputRoot = process.cwd(), validate = true, global = false, + logger, }: ToolMcpFromFileParams): Promise { const paths = this.getSettablePaths({ global }); - const fileContent = - (await readFileContentOrNull( - join(outputRoot, paths.relativeDirPath, paths.relativeFilePath), - )) ?? '{"mcpServers":{}}'; - const json = JSON.parse(fileContent); - const newJson = { ...json, mcpServers: json.mcpServers ?? {} }; + const recommendedPath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); + + // Try the recommended path first. + if (await fileExists(recommendedPath)) { + const fileContent = await readFileContent(recommendedPath); + const json = JSON.parse(fileContent); + const newJson = { ...json, mcpServers: json.mcpServers ?? {} }; + return new ClaudecodeMcp({ + outputRoot, + relativeDirPath: paths.relativeDirPath, + relativeFilePath: paths.relativeFilePath, + fileContent: JSON.stringify(newJson, null, 2), + validate, + global, + }); + } + // Backward compatibility: fall back to the legacy path with a + // deprecation warning. Mirrors `RulesyncMcp.fromFile` (PR #333). + if (global) { + const legacyPath = join( + outputRoot, + ClaudecodeMcp.LEGACY_GLOBAL_DIR, + ClaudecodeMcp.LEGACY_GLOBAL_FILE, + ); + if (await fileExists(legacyPath)) { + logger?.warn( + `Warning: using deprecated path "${legacyPath}". Please migrate to "${recommendedPath}"`, + ); + const fileContent = await readFileContent(legacyPath); + const json = JSON.parse(fileContent); + const newJson = { ...json, mcpServers: json.mcpServers ?? {} }; + // Reflect the legacy path so callers see where data came from; + // `fromRulesyncMcp` always writes to the recommended path. + return new ClaudecodeMcp({ + outputRoot, + relativeDirPath: ClaudecodeMcp.LEGACY_GLOBAL_DIR, + relativeFilePath: ClaudecodeMcp.LEGACY_GLOBAL_FILE, + fileContent: JSON.stringify(newJson, null, 2), + validate, + global, + }); + } + } + + // Neither recommended nor legacy exists: initialize empty mcpServers + // at the recommended path (existing fromFile contract). return new ClaudecodeMcp({ outputRoot, relativeDirPath: paths.relativeDirPath, relativeFilePath: paths.relativeFilePath, - fileContent: JSON.stringify(newJson, null, 2), + fileContent: JSON.stringify({ mcpServers: {} }, null, 2), validate, global, }); @@ -85,6 +136,9 @@ export class ClaudecodeMcp extends ToolMcp { const mcpJson = { ...json, mcpServers: rulesyncMcp.getMcpServers() }; + // The legacy path `~/.claude/.claude.json` is intentionally not touched + // here — see the LEGACY_GLOBAL_* docstring above for the rationale. + return new ClaudecodeMcp({ outputRoot, relativeDirPath: paths.relativeDirPath, @@ -97,7 +151,7 @@ export class ClaudecodeMcp extends ToolMcp { toRulesyncMcp(): RulesyncMcp { return this.toRulesyncMcpDefault({ - fileContent: JSON.stringify({ mcpServers: this.json.mcpServers }, null, 2), + fileContent: JSON.stringify({ mcpServers: this.json.mcpServers ?? {} }, null, 2), }); } diff --git a/src/features/mcp/mcp-processor.test.ts b/src/features/mcp/mcp-processor.test.ts index e5c3fd621..4cafe033c 100644 --- a/src/features/mcp/mcp-processor.test.ts +++ b/src/features/mcp/mcp-processor.test.ts @@ -221,6 +221,7 @@ describe("McpProcessor", () => { outputRoot: testDir, validate: true, global: false, + logger: expect.any(Object), }); }); @@ -248,6 +249,7 @@ describe("McpProcessor", () => { outputRoot: testDir, validate: true, global: false, + logger: expect.any(Object), }); }); @@ -276,6 +278,7 @@ describe("McpProcessor", () => { outputRoot: testDir, validate: true, global: true, + logger: expect.any(Object), }); }); }); @@ -305,6 +308,7 @@ describe("McpProcessor", () => { outputRoot: testDir, validate: true, global: false, + logger: expect.any(Object), }); }); }); @@ -334,6 +338,7 @@ describe("McpProcessor", () => { outputRoot: testDir, validate: true, global: false, + logger: expect.any(Object), }); }); }); @@ -363,6 +368,7 @@ describe("McpProcessor", () => { outputRoot: testDir, validate: true, global: false, + logger: expect.any(Object), }); }); @@ -391,6 +397,7 @@ describe("McpProcessor", () => { outputRoot: testDir, validate: true, global: true, + logger: expect.any(Object), }); }); }); @@ -420,6 +427,7 @@ describe("McpProcessor", () => { outputRoot: testDir, validate: true, global: false, + logger: expect.any(Object), }); }); }); @@ -449,6 +457,7 @@ describe("McpProcessor", () => { outputRoot: testDir, validate: true, global: false, + logger: expect.any(Object), }); }); @@ -477,6 +486,7 @@ describe("McpProcessor", () => { outputRoot: testDir, validate: true, global: true, + logger: expect.any(Object), }); }); }); @@ -505,6 +515,7 @@ describe("McpProcessor", () => { outputRoot: testDir, validate: true, global: true, + logger: expect.any(Object), }); }); @@ -551,6 +562,7 @@ describe("McpProcessor", () => { outputRoot: testDir, validate: true, global: false, + logger: expect.any(Object), }); }); }); diff --git a/src/features/mcp/mcp-processor.ts b/src/features/mcp/mcp-processor.ts index aec200c78..1ed920ae6 100644 --- a/src/features/mcp/mcp-processor.ts +++ b/src/features/mcp/mcp-processor.ts @@ -390,6 +390,7 @@ export class McpProcessor extends FeatureProcessor { outputRoot: this.outputRoot, validate: true, global: this.global, + logger: this.logger, }), ]; this.logger.debug(`Successfully loaded ${toolMcps.length} ${this.toolTarget} MCP files`); diff --git a/src/features/mcp/tool-mcp.ts b/src/features/mcp/tool-mcp.ts index 76a6a9117..5467b5411 100644 --- a/src/features/mcp/tool-mcp.ts +++ b/src/features/mcp/tool-mcp.ts @@ -5,6 +5,7 @@ import { } from "../../constants/rulesync-paths.js"; import { AiFileFromFileParams, AiFileParams } from "../../types/ai-file.js"; import { ToolFile } from "../../types/tool-file.js"; +import type { Logger } from "../../utils/logger.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; export type ToolMcpParams = AiFileParams; @@ -19,7 +20,9 @@ export type ToolMcpFromRulesyncMcpParams = Omit< export type ToolMcpFromFileParams = Pick< AiFileFromFileParams, "outputRoot" | "validate" | "global" ->; +> & { + logger?: Logger; +}; export type ToolMcpForDeletionParams = { outputRoot?: string; @@ -37,7 +40,7 @@ export abstract class ToolMcp extends ToolFile { constructor({ ...rest }: ToolMcpParams) { super({ ...rest, - validate: true, // Skip validation during construction + validate: true, // ToolMcp runs subclass validation below when requested }); // Validate after setting patterns, if validation was requested