From 8e339d647a9c5304332d811bafef401cf09f16cd Mon Sep 17 00:00:00 2001 From: sirmacik <127441966+sirmacik@users.noreply.github.com> Date: Tue, 12 May 2026 17:55:36 +0200 Subject: [PATCH 1/3] fix(claudecode-mcp): write global MCP to documented ~/.claude.json path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/`. Earlier rulesync versions (≤ v8.17.0) wrote to the wrong path `~/.claude/.claude.json`. Issue #1387 described this exact bug; it was closed by the reporter as "filed prematurely" but the root cause is still present in main. ## Changes ### src/features/mcp/claudecode-mcp.ts - `getSettablePaths({ global: true })` now returns the documented path `{ relativeDirPath: ".", relativeFilePath: ".claude.json" }`. - `fromFile` falls back to the legacy path `~/.claude/.claude.json` with a deprecation warning via the optional `logger` parameter when the recommended path is absent. Mirrors the backward-compatibility pattern established by PR #333 (`RulesyncMcp.fromFile` for legacy `.rulesync/.mcp.json`). - `fromRulesyncMcp` always writes to the recommended path; the legacy file is never modified, deleted, or renamed by rulesync. ### Migration philosophy Rulesync intentionally does NOT actively migrate user files at legacy paths. The pattern is consistent with every prior legacy-path change in the project (#333, commit 57871956): read-fallback + deprecation warning, never touch user data. Users who want to fully eliminate any ghost-MCP risk from Claude Code's runtime behavior should manually delete `~/.claude/.claude.json` after upgrading. ### Tests - `fromFile`: 5 new tests - prefers recommended path when both exist - falls back to legacy path with deprecation warning when only legacy exists - does NOT fall back to legacy in local mode - initializes empty mcpServers when neither file exists - works without a logger when reading the legacy path - `fromRulesyncMcp`: 4 tests - fresh install: initializes recommended path - does NOT modify legacy file (byte-identical assertion) - does not touch legacy file in local mode - preserves unrelated keys on `~/.claude.json` via RMW - Existing global-mode tests updated to assert the new path. ### E2E - Updated the global-MCP matrix entry: claudecode → `.claude.json`. - New test: `should preserve legacy ~/.claude/.claude.json when writing to recommended path (global)` — byte-identical legacy assertion at CLI level. ## Verification - `pnpm test src/features/mcp/claudecode-mcp.test.ts`: 56 tests pass. - `pnpm test:e2e src/e2e/e2e-mcp.spec.ts`: 47 tests pass. - `pnpm cicheck`: clean. ## Notes for upgrading users After upgrading, rulesync writes global MCP to `~/.claude.json` (the documented path). If you had a previous rulesync version that wrote to `~/.claude/.claude.json`, that file is left in place untouched. To fully eliminate any ghost-MCP risk from Claude Code's runtime behavior, manually delete the legacy file: rm ~/.claude/.claude.json ## Related - Closes #1387 (was closed prematurely by reporter without fix). - Related-but-out-of-scope: #1275 (`global` flag not passed to `ClaudecodeMcp` constructor) — separate concern, separate PR. --- src/e2e/e2e-mcp.spec.ts | 78 +++++- src/features/mcp/claudecode-mcp.test.ts | 330 ++++++++++++++++++++++-- src/features/mcp/claudecode-mcp.ts | 79 +++++- 3 files changed, 449 insertions(+), 38 deletions(-) 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..2737773ec 100644 --- a/src/features/mcp/claudecode-mcp.test.ts +++ b/src/features/mcp/claudecode-mcp.test.ts @@ -6,8 +6,9 @@ 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 { RulesyncMcp } from "./rulesync-mcp.js"; @@ -34,9 +35,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 +63,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 +74,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 +313,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 +322,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 +354,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 +372,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 +395,154 @@ 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 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 +686,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 +731,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 +779,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 +822,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 +1250,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 +1273,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 +1289,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..cc255eeb9 100644 --- a/src/features/mcp/claudecode-mcp.ts +++ b/src/features/mcp/claudecode-mcp.ts @@ -1,7 +1,8 @@ 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 type { Logger } from "../../utils/logger.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { ToolMcp, @@ -25,18 +26,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 +61,61 @@ export class ClaudecodeMcp extends ToolMcp { outputRoot = process.cwd(), validate = true, global = false, - }: ToolMcpFromFileParams): Promise { + logger, + }: ToolMcpFromFileParams & { logger?: Logger }): 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( + `⚠️ 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 +137,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, From d33b4523cde615c881174ec304fab478ae9ca912 Mon Sep 17 00:00:00 2001 From: sirmacik <127441966+sirmacik@users.noreply.github.com> Date: Wed, 13 May 2026 11:09:45 +0200 Subject: [PATCH 2/3] fix(claudecode-mcp): surface legacy warnings --- src/features/mcp/claudecode-mcp.test.ts | 30 +++++++++++++++++++++++++ src/features/mcp/claudecode-mcp.ts | 3 +-- src/features/mcp/mcp-processor.test.ts | 12 ++++++++++ src/features/mcp/mcp-processor.ts | 1 + src/features/mcp/tool-mcp.ts | 5 ++++- 5 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/features/mcp/claudecode-mcp.test.ts b/src/features/mcp/claudecode-mcp.test.ts index 2737773ec..ef4c0f5f0 100644 --- a/src/features/mcp/claudecode-mcp.test.ts +++ b/src/features/mcp/claudecode-mcp.test.ts @@ -10,6 +10,7 @@ import { createMockLogger } from "../../test-utils/mock-logger.js"; import { setupTestDirectory } from "../../test-utils/test-directories.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", () => { @@ -459,6 +460,35 @@ describe("ClaudecodeMcp", () => { 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`. diff --git a/src/features/mcp/claudecode-mcp.ts b/src/features/mcp/claudecode-mcp.ts index cc255eeb9..61db3652c 100644 --- a/src/features/mcp/claudecode-mcp.ts +++ b/src/features/mcp/claudecode-mcp.ts @@ -2,7 +2,6 @@ import { join } from "node:path"; import { ValidationResult } from "../../types/ai-file.js"; import { fileExists, readFileContent, readOrInitializeFileContent } from "../../utils/file.js"; -import type { Logger } from "../../utils/logger.js"; import { RulesyncMcp } from "./rulesync-mcp.js"; import { ToolMcp, @@ -62,7 +61,7 @@ export class ClaudecodeMcp extends ToolMcp { validate = true, global = false, logger, - }: ToolMcpFromFileParams & { logger?: Logger }): Promise { + }: ToolMcpFromFileParams): Promise { const paths = this.getSettablePaths({ global }); const recommendedPath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath); 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..6ad354ca1 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; From c41bf4f0b1aecdf4ad41447d614e299f98259de6 Mon Sep 17 00:00:00 2001 From: sirmacik <127441966+sirmacik@users.noreply.github.com> Date: Wed, 13 May 2026 11:17:57 +0200 Subject: [PATCH 3/3] fix(claudecode-mcp): harden migration edge cases --- src/features/mcp/claudecode-mcp.ts | 4 ++-- src/features/mcp/tool-mcp.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/features/mcp/claudecode-mcp.ts b/src/features/mcp/claudecode-mcp.ts index 61db3652c..a166ff4ee 100644 --- a/src/features/mcp/claudecode-mcp.ts +++ b/src/features/mcp/claudecode-mcp.ts @@ -90,7 +90,7 @@ export class ClaudecodeMcp extends ToolMcp { ); if (await fileExists(legacyPath)) { logger?.warn( - `⚠️ Using deprecated path "${legacyPath}". Please migrate to "${recommendedPath}"`, + `Warning: using deprecated path "${legacyPath}". Please migrate to "${recommendedPath}"`, ); const fileContent = await readFileContent(legacyPath); const json = JSON.parse(fileContent); @@ -151,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/tool-mcp.ts b/src/features/mcp/tool-mcp.ts index 6ad354ca1..5467b5411 100644 --- a/src/features/mcp/tool-mcp.ts +++ b/src/features/mcp/tool-mcp.ts @@ -40,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