From 4fa66278bb80360eb9d1578a6d8f02334368f4cb Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Thu, 26 Mar 2026 11:47:55 +0100 Subject: [PATCH 1/2] Extract shared git-root resolver for ~/.claude.json project key lookups - Add ProjectDetector.resolveProjectKey(from:in:) to walk up from project root to git root when resolving ~/.claude.json project keys - Fix ConfigurationDiscovery.discoverMCPServers() using the shared utility (mcs export was silently missing MCP servers in subdirectory projects) - Refactor MCPServerCheck to use the shared utility instead of inline walk-up loop --- Sources/mcs/Core/ProjectDetector.swift | 22 ++++ Sources/mcs/Doctor/CoreDoctorChecks.swift | 21 +--- .../mcs/Export/ConfigurationDiscovery.swift | 4 +- .../CoreDoctorCheckSandboxTests.swift | 112 ++++++++++++++++++ .../DoctorRunnerIntegrationTests.swift | 65 ++++++++++ Tests/MCSTests/ProjectDetectorTests.swift | 89 ++++++++++++++ 6 files changed, 296 insertions(+), 17 deletions(-) diff --git a/Sources/mcs/Core/ProjectDetector.swift b/Sources/mcs/Core/ProjectDetector.swift index 552fc95..f482f38 100644 --- a/Sources/mcs/Core/ProjectDetector.swift +++ b/Sources/mcs/Core/ProjectDetector.swift @@ -21,6 +21,28 @@ enum ProjectDetector { return nil } + /// Walk up from `startingPath` looking for a path that exists in `projectKeys`. + /// Stops at (and includes) the first directory containing `.git/`. + /// Returns the matching key string, or nil if no match is found. + /// + /// This resolves the mismatch between `findProjectRoot()` (which may return a + /// subdirectory containing CLAUDE.local.md) and the Claude CLI's convention of + /// keying project-scoped entries by the git root in `~/.claude.json`. + static func resolveProjectKey(from startingPath: URL, in projectKeys: Set) -> String? { + let fm = FileManager.default + var current = startingPath.standardizedFileURL + while current.path != "/" { + if projectKeys.contains(current.path) { + return current.path + } + if fm.fileExists(atPath: current.appendingPathComponent(".git").path) { + break + } + current = current.deletingLastPathComponent() + } + return nil + } + /// Convenience: find project root from the current working directory. static func findProjectRoot() -> URL? { let cwd = URL(fileURLWithPath: FileManager.default.currentDirectoryPath) diff --git a/Sources/mcs/Doctor/CoreDoctorChecks.swift b/Sources/mcs/Doctor/CoreDoctorChecks.swift index ce8a252..5bd2b79 100644 --- a/Sources/mcs/Doctor/CoreDoctorChecks.swift +++ b/Sources/mcs/Doctor/CoreDoctorChecks.swift @@ -68,22 +68,13 @@ struct MCPServerCheck: DoctorCheck { guard let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else { return .fail("~/.claude.json contains invalid JSON") } - // Walk up to the git root: Claude CLI keys local-scope servers by the git root, - // which may differ from the mcs project root in subdirectory projects. if let root = projectRoot, - let projects = json[Constants.JSONKeys.projects] as? [String: Any] { - var candidate: URL? = root - while let path = candidate, path.path != "/" { - if let projectEntry = projects[path.path] as? [String: Any], - let projectMCP = projectEntry[Constants.JSONKeys.mcpServers] as? [String: Any], - projectMCP[serverName] != nil { - return .pass("registered") - } - if FileManager.default.fileExists(atPath: path.appendingPathComponent(".git").path) { - break - } - candidate = path.deletingLastPathComponent() - } + let projects = json[Constants.JSONKeys.projects] as? [String: Any], + let matchedKey = ProjectDetector.resolveProjectKey(from: root, in: Set(projects.keys)), + let projectEntry = projects[matchedKey] as? [String: Any], + let projectMCP = projectEntry[Constants.JSONKeys.mcpServers] as? [String: Any], + projectMCP[serverName] != nil { + return .pass("registered") } // Fall back to global/user-scoped servers if let mcpServers = json[Constants.JSONKeys.mcpServers] as? [String: Any], diff --git a/Sources/mcs/Export/ConfigurationDiscovery.swift b/Sources/mcs/Export/ConfigurationDiscovery.swift index 4329957..5d2b03c 100644 --- a/Sources/mcs/Export/ConfigurationDiscovery.swift +++ b/Sources/mcs/Export/ConfigurationDiscovery.swift @@ -161,9 +161,9 @@ struct ConfigurationDiscovery { } } case let .project(projectRoot): - // Read project-scoped servers from projects[path].mcpServers if let projects = json[Constants.JSONKeys.projects] as? [String: Any], - let projectEntry = projects[projectRoot.path] as? [String: Any], + let matchedKey = ProjectDetector.resolveProjectKey(from: projectRoot, in: Set(projects.keys)), + let projectEntry = projects[matchedKey] as? [String: Any], let servers = projectEntry[Constants.JSONKeys.mcpServers] as? [String: Any] { for (name, value) in servers { if let serverDict = value as? [String: Any] { diff --git a/Tests/MCSTests/CoreDoctorCheckSandboxTests.swift b/Tests/MCSTests/CoreDoctorCheckSandboxTests.swift index f7ee907..aeb6632 100644 --- a/Tests/MCSTests/CoreDoctorCheckSandboxTests.swift +++ b/Tests/MCSTests/CoreDoctorCheckSandboxTests.swift @@ -108,6 +108,118 @@ struct MCPServerCheckSandboxTests { return } } + + @Test("pass when subdirectory project root walks up to find server at git root") + func passWalkUpToGitRoot() throws { + let home = try makeGlobalTmpDir(label: "mcp-walkup") + defer { try? FileManager.default.removeItem(at: home) } + let env = Environment(home: home) + + let gitRoot = home.appendingPathComponent("my-project") + let subProject = gitRoot.appendingPathComponent("packages/lib") + try FileManager.default.createDirectory( + at: gitRoot.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory(at: subProject, withIntermediateDirectories: true) + + let claudeJSON: [String: Any] = [ + "projects": [ + gitRoot.path: [ + "mcpServers": [ + "serena": ["command": "npx", "args": ["-y", "serena"]], + ], + ], + ], + ] + let data = try JSONSerialization.data(withJSONObject: claudeJSON) + try data.write(to: env.claudeJSON) + + let check = MCPServerCheck( + name: "Serena", serverName: "serena", + projectRoot: subProject, environment: env + ) + let result = check.check() + guard case .pass = result else { + Issue.record("Expected .pass (walk-up), got \(result)") + return + } + } + + @Test("walk-up stops at .git boundary and does not escape repo") + func walkUpStopsAtGitBoundary() throws { + let home = try makeGlobalTmpDir(label: "mcp-boundary") + defer { try? FileManager.default.removeItem(at: home) } + let env = Environment(home: home) + + let outerRepo = home.appendingPathComponent("outer") + let innerRepo = outerRepo.appendingPathComponent("inner") + try FileManager.default.createDirectory( + at: outerRepo.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: innerRepo.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + + let claudeJSON: [String: Any] = [ + "projects": [ + outerRepo.path: [ + "mcpServers": [ + "serena": ["command": "npx", "args": ["-y", "serena"]], + ], + ], + ], + ] + let data = try JSONSerialization.data(withJSONObject: claudeJSON) + try data.write(to: env.claudeJSON) + + let check = MCPServerCheck( + name: "Serena", serverName: "serena", + projectRoot: innerRepo, environment: env + ) + let result = check.check() + guard case .fail = result else { + Issue.record("Expected .fail (should not escape git boundary), got \(result)") + return + } + } + + @Test("pass when projectRoot equals gitRoot (regression)") + func passExactMatchRegression() throws { + let home = try makeGlobalTmpDir(label: "mcp-exact") + defer { try? FileManager.default.removeItem(at: home) } + let env = Environment(home: home) + + let projectRoot = home.appendingPathComponent("my-project") + try FileManager.default.createDirectory( + at: projectRoot.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + + let claudeJSON: [String: Any] = [ + "projects": [ + projectRoot.path: [ + "mcpServers": [ + "serena": ["command": "npx", "args": ["-y", "serena"]], + ], + ], + ], + ] + let data = try JSONSerialization.data(withJSONObject: claudeJSON) + try data.write(to: env.claudeJSON) + + let check = MCPServerCheck( + name: "Serena", serverName: "serena", + projectRoot: projectRoot, environment: env + ) + let result = check.check() + guard case .pass = result else { + Issue.record("Expected .pass (exact match regression), got \(result)") + return + } + } } // MARK: - PluginCheck Sandbox Tests diff --git a/Tests/MCSTests/DoctorRunnerIntegrationTests.swift b/Tests/MCSTests/DoctorRunnerIntegrationTests.swift index 8a13802..75d1ffe 100644 --- a/Tests/MCSTests/DoctorRunnerIntegrationTests.swift +++ b/Tests/MCSTests/DoctorRunnerIntegrationTests.swift @@ -162,4 +162,69 @@ struct DoctorRunnerIntegrationTests { // Should detect the pack from project state try runner.run() } + + @Test("MCPServerCheck passes via walk-up when project root is a subdirectory of git root") + func mcpCheckWalksUpToGitRoot() throws { + let home = try makeGlobalTmpDir(label: "runner-walkup") + defer { try? FileManager.default.removeItem(at: home) } + let env = Environment(home: home) + + // Git root at home/my-repo, project root at home/my-repo/packages/lib + let gitRoot = home.appendingPathComponent("my-repo") + let subProject = gitRoot.appendingPathComponent("packages/lib") + try FileManager.default.createDirectory( + at: gitRoot.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: subProject.appendingPathComponent(Constants.FileNames.claudeDirectory), + withIntermediateDirectories: true + ) + + // Pack with an MCP component + let mcpComponent = ComponentDefinition( + id: "test-pack.my-mcp", + displayName: "My MCP", + description: "Test MCP server", + type: .mcpServer, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .mcpServer(MCPServerConfig( + name: "my-mcp", command: "npx", args: ["-y", "my-mcp"], env: [:] + )) + ) + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [mcpComponent] + ) + let registry = TechPackRegistry(packs: [pack]) + + // Record pack in project state at the subdirectory root + var state = try ProjectState(projectRoot: subProject) + state.recordPack("test-pack") + state.setArtifacts( + PackArtifactRecord(mcpServers: [MCPServerRef(name: "my-mcp", scope: "local")]), + for: "test-pack" + ) + try state.save() + + // Write ~/.claude.json with the server keyed at the git root (as Claude CLI does) + let claudeJSON: [String: Any] = [ + "projects": [ + gitRoot.path: [ + "mcpServers": [ + "my-mcp": ["command": "npx", "args": ["-y", "my-mcp"]], + ], + ], + ], + ] + let data = try JSONSerialization.data(withJSONObject: claudeJSON) + try data.write(to: env.claudeJSON) + + // DoctorRunner with projectRoot at the subdirectory + var runner = makeRunner(home: home, projectRoot: subProject, registry: registry) + try runner.run() + } } diff --git a/Tests/MCSTests/ProjectDetectorTests.swift b/Tests/MCSTests/ProjectDetectorTests.swift index cf12183..79072e5 100644 --- a/Tests/MCSTests/ProjectDetectorTests.swift +++ b/Tests/MCSTests/ProjectDetectorTests.swift @@ -80,6 +80,95 @@ struct ProjectDetectorTests { } } +// MARK: - resolveProjectKey Tests + +struct ProjectDetectorResolveKeyTests { + private func makeTmpDir() throws -> URL { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("mcs-resolvekey-test-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + return dir + } + + @Test("Returns matching key when projectRoot equals git root") + func exactMatch() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + try FileManager.default.createDirectory( + at: tmpDir.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + + let keys: Set = [tmpDir.standardizedFileURL.path] + let result = ProjectDetector.resolveProjectKey(from: tmpDir, in: keys) + #expect(result == tmpDir.standardizedFileURL.path) + } + + @Test("Walks up from subdirectory to find key at git root") + func walkUpToGitRoot() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + try FileManager.default.createDirectory( + at: tmpDir.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + let subDir = tmpDir.appendingPathComponent("packages/my-lib") + try FileManager.default.createDirectory(at: subDir, withIntermediateDirectories: true) + + let keys: Set = [tmpDir.standardizedFileURL.path] + let result = ProjectDetector.resolveProjectKey(from: subDir, in: keys) + #expect(result == tmpDir.standardizedFileURL.path) + } + + @Test("Stops at git boundary and does not escape the repo") + func stopsAtGitBoundary() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let outer = tmpDir.appendingPathComponent("outer") + let inner = outer.appendingPathComponent("inner") + try FileManager.default.createDirectory( + at: outer.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: inner.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + + // Key is at outer, but search starts at inner — should NOT find outer + let keys: Set = [outer.standardizedFileURL.path] + let result = ProjectDetector.resolveProjectKey(from: inner, in: keys) + #expect(result == nil) + } + + @Test("Returns nil when no key matches") + func noMatch() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + try FileManager.default.createDirectory( + at: tmpDir.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + + let keys: Set = ["/some/other/path"] + let result = ProjectDetector.resolveProjectKey(from: tmpDir, in: keys) + #expect(result == nil) + } + + @Test("Returns nil when project keys set is empty") + func emptyKeys() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let result = ProjectDetector.resolveProjectKey(from: tmpDir, in: []) + #expect(result == nil) + } +} + struct ProjectStateTests { private func makeTmpDir() throws -> URL { let dir = FileManager.default.temporaryDirectory From 633d15477f8a22ac21b7cf222aa5f631fbc6ab20 Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Thu, 26 Mar 2026 12:00:25 +0100 Subject: [PATCH 2/2] Add ConfigurationDiscovery tests for subdirectory project MCP server discovery - Walk-up finds servers keyed at git root from subdirectory project - Exact match when projectRoot equals gitRoot (regression) - Boundary: nested .git prevents escaping to outer repo --- .../ConfigurationDiscoveryTests.swift | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 Tests/MCSTests/ConfigurationDiscoveryTests.swift diff --git a/Tests/MCSTests/ConfigurationDiscoveryTests.swift b/Tests/MCSTests/ConfigurationDiscoveryTests.swift new file mode 100644 index 0000000..1b5b1bc --- /dev/null +++ b/Tests/MCSTests/ConfigurationDiscoveryTests.swift @@ -0,0 +1,126 @@ +import Foundation +@testable import mcs +import Testing + +struct ConfigurationDiscoveryTests { + @Test("discovers MCP servers when project root is a subdirectory of git root") + func discoversMCPServersViaWalkUp() throws { + let home = try makeGlobalTmpDir(label: "discovery-walkup") + defer { try? FileManager.default.removeItem(at: home) } + let env = Environment(home: home) + + let gitRoot = home.appendingPathComponent("my-repo") + let subProject = gitRoot.appendingPathComponent("packages/lib") + try FileManager.default.createDirectory( + at: gitRoot.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: subProject.appendingPathComponent(Constants.FileNames.claudeDirectory), + withIntermediateDirectories: true + ) + + let claudeJSON: [String: Any] = [ + "projects": [ + gitRoot.path: [ + "mcpServers": [ + "docs-server": [ + "command": "npx", + "args": ["-y", "docs-server"], + ], + ], + ], + ], + ] + let data = try JSONSerialization.data(withJSONObject: claudeJSON) + try data.write(to: env.claudeJSON) + + let discovery = ConfigurationDiscovery(environment: env, output: CLIOutput()) + let config = discovery.discover(scope: ConfigurationDiscovery.Scope.project(subProject)) + + #expect(config.mcpServers.count == 1) + #expect(config.mcpServers.first?.name == "docs-server") + #expect(config.mcpServers.first?.scope == "local") + } + + @Test("discovers MCP servers when project root equals git root") + func discoversMCPServersExactMatch() throws { + let home = try makeGlobalTmpDir(label: "discovery-exact") + defer { try? FileManager.default.removeItem(at: home) } + let env = Environment(home: home) + + let projectRoot = home.appendingPathComponent("my-project") + try FileManager.default.createDirectory( + at: projectRoot.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: projectRoot.appendingPathComponent(Constants.FileNames.claudeDirectory), + withIntermediateDirectories: true + ) + + let claudeJSON: [String: Any] = [ + "projects": [ + projectRoot.path: [ + "mcpServers": [ + "docs-server": [ + "command": "npx", + "args": ["-y", "docs-server"], + ], + ], + ], + ], + ] + let data = try JSONSerialization.data(withJSONObject: claudeJSON) + try data.write(to: env.claudeJSON) + + let discovery = ConfigurationDiscovery(environment: env, output: CLIOutput()) + let config = discovery.discover(scope: ConfigurationDiscovery.Scope.project(projectRoot)) + + #expect(config.mcpServers.count == 1) + #expect(config.mcpServers.first?.name == "docs-server") + } + + @Test("returns empty when no MCP servers match subdirectory project") + func noMCPServersWhenBoundaryBlocks() throws { + let home = try makeGlobalTmpDir(label: "discovery-boundary") + defer { try? FileManager.default.removeItem(at: home) } + let env = Environment(home: home) + + let outerRepo = home.appendingPathComponent("outer") + let innerRepo = outerRepo.appendingPathComponent("inner") + try FileManager.default.createDirectory( + at: outerRepo.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: innerRepo.appendingPathComponent(".git"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: innerRepo.appendingPathComponent(Constants.FileNames.claudeDirectory), + withIntermediateDirectories: true + ) + + // Server is keyed at outer repo, but inner repo has its own .git boundary + let claudeJSON: [String: Any] = [ + "projects": [ + outerRepo.path: [ + "mcpServers": [ + "docs-server": [ + "command": "npx", + "args": ["-y", "docs-server"], + ], + ], + ], + ], + ] + let data = try JSONSerialization.data(withJSONObject: claudeJSON) + try data.write(to: env.claudeJSON) + + let discovery = ConfigurationDiscovery(environment: env, output: CLIOutput()) + let config = discovery.discover(scope: ConfigurationDiscovery.Scope.project(innerRepo)) + + #expect(config.mcpServers.isEmpty) + } +}