From bfa793a93735757a3773c79fc04d0910870665b7 Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Fri, 6 Mar 2026 09:02:24 +0100 Subject: [PATCH 1/4] Add GlobalSyncStrategy test coverage through Configurator (#100) - Extract MockClaudeCLI and MockTechPack into shared TestHelpers.swift - Add GlobalConfiguratorTests.swift with 30 tests across 10 suites covering MCP scope override, settings preservation, file installation, convergence flows, unconfigure, dry-run, excluded components, and state tracking - Add TrackingMockTechPack for verifying configureProject is not called --- Tests/MCSTests/GlobalConfiguratorTests.swift | 1395 +++++++++++++++++ Tests/MCSTests/ProjectConfiguratorTests.swift | 90 -- Tests/MCSTests/TestHelpers.swift | 123 ++ 3 files changed, 1518 insertions(+), 90 deletions(-) create mode 100644 Tests/MCSTests/GlobalConfiguratorTests.swift create mode 100644 Tests/MCSTests/TestHelpers.swift diff --git a/Tests/MCSTests/GlobalConfiguratorTests.swift b/Tests/MCSTests/GlobalConfiguratorTests.swift new file mode 100644 index 0000000..1f95aa8 --- /dev/null +++ b/Tests/MCSTests/GlobalConfiguratorTests.swift @@ -0,0 +1,1395 @@ +import Foundation +@testable import mcs +import Testing + +// MARK: - Global MCP Scope Tests + +struct GlobalMCPScopeTests { + private func makeTmpDir() throws -> URL { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("mcs-global-mcp-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + // Create required subdirectories for global scope + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".claude"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".mcs"), + withIntermediateDirectories: true + ) + return dir + } + + private func makeConfigurator( + home: URL, + mockCLI: MockClaudeCLI = MockClaudeCLI() + ) -> Configurator { + let env = Environment(home: home) + return Configurator( + environment: env, + output: CLIOutput(colorsEnabled: false), + shell: ShellRunner(environment: env), + strategy: GlobalSyncStrategy(environment: env), + claudeCLI: mockCLI + ) + } + + @Test("MCP server is registered with user scope regardless of pack declaration") + func mcpServerUsesUserScope() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let mockCLI = MockClaudeCLI() + let configurator = makeConfigurator(home: tmpDir, mockCLI: mockCLI) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.mcp-server", + displayName: "Test MCP", + description: "Test MCP server", + type: .mcpServer, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .mcpServer(MCPServerConfig( + name: "test-mcp", command: "/usr/bin/true", args: [], env: [:] + )) + )] + ) + + try configurator.configure(packs: [pack], confirmRemovals: false) + + #expect(mockCLI.mcpAddCalls.count == 1) + #expect(mockCLI.mcpAddCalls.first?.scope == "user") + } + + @Test("MCP scope override ignores explicit local scope in pack config") + func mcpScopeOverrideIgnoresLocalDeclaration() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let mockCLI = MockClaudeCLI() + let configurator = makeConfigurator(home: tmpDir, mockCLI: mockCLI) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.mcp-local", + displayName: "Local MCP", + description: "Pack declares local scope", + type: .mcpServer, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .mcpServer(MCPServerConfig( + name: "mcp-local", command: "/usr/bin/true", args: [], env: [:], + scope: "local" + )) + )] + ) + + try configurator.configure(packs: [pack], confirmRemovals: false) + + #expect(mockCLI.mcpAddCalls.first?.scope == "user") + } + + @Test("Artifact record stores user scope for MCP server") + func mcpArtifactRecordsUserScope() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let configurator = makeConfigurator(home: tmpDir) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.mcp", + displayName: "MCP", + description: "MCP server", + type: .mcpServer, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .mcpServer(MCPServerConfig( + name: "my-mcp", command: "/usr/bin/true", args: [], env: [:] + )) + )] + ) + + try configurator.configure(packs: [pack], confirmRemovals: false) + + let env = Environment(home: tmpDir) + let state = try ProjectState(stateFile: env.globalStateFile) + let artifacts = state.artifacts(for: "test-pack") + #expect(artifacts?.mcpServers.first?.scope == "user") + #expect(artifacts?.mcpServers.first?.name == "my-mcp") + } + + @Test("Stale MCP server is removed with user scope") + func staleMCPRemovedWithUserScope() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let mockCLI = MockClaudeCLI() + let configurator = makeConfigurator(home: tmpDir, mockCLI: mockCLI) + + // Pack v1: two MCP servers + let packV1 = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ + ComponentDefinition( + id: "test-pack.mcp-keep", + displayName: "MCP Keep", + description: "Kept", + type: .mcpServer, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .mcpServer(MCPServerConfig( + name: "mcp-keep", command: "/usr/bin/true", args: [], env: [:] + )) + ), + ComponentDefinition( + id: "test-pack.mcp-drop", + displayName: "MCP Drop", + description: "Dropped", + type: .mcpServer, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .mcpServer(MCPServerConfig( + name: "mcp-drop", command: "/usr/bin/true", args: [], env: [:] + )) + ), + ] + ) + + try configurator.configure(packs: [packV1], confirmRemovals: false) + mockCLI.mcpRemoveCalls = [] + + // Pack v2: mcp-drop removed + let packV2 = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ + ComponentDefinition( + id: "test-pack.mcp-keep", + displayName: "MCP Keep", + description: "Kept", + type: .mcpServer, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .mcpServer(MCPServerConfig( + name: "mcp-keep", command: "/usr/bin/true", args: [], env: [:] + )) + ), + ] + ) + + try configurator.configure(packs: [packV2], confirmRemovals: false) + + #expect(mockCLI.mcpRemoveCalls.contains( + MockClaudeCLI.MCPRemoveCall(name: "mcp-drop", scope: "user") + )) + } +} + +// MARK: - Global Settings Composition Tests + +struct GlobalSettingsCompositionTests { + private func makeTmpDir() throws -> URL { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("mcs-global-settings-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".claude"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".mcs"), + withIntermediateDirectories: true + ) + return dir + } + + private func makeConfigurator( + home: URL, + mockCLI: MockClaudeCLI = MockClaudeCLI() + ) -> Configurator { + let env = Environment(home: home) + return Configurator( + environment: env, + output: CLIOutput(colorsEnabled: false), + shell: ShellRunner(environment: env), + strategy: GlobalSyncStrategy(environment: env), + claudeCLI: mockCLI + ) + } + + @Test("Existing user settings are preserved after global sync") + func preservesExistingUserSettings() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + // Pre-write settings.json with user content + let settingsPath = tmpDir.appendingPathComponent(".claude/settings.json") + let userSettings = """ + {"permissions":{"allow":["Bash(npm test)"]}} + """ + try userSettings.write(to: settingsPath, atomically: true, encoding: .utf8) + + let configurator = makeConfigurator(home: tmpDir) + + // Pack with a hook file that will add hooks to settings + let packDir = tmpDir.appendingPathComponent("pack/hooks") + try FileManager.default.createDirectory(at: packDir, withIntermediateDirectories: true) + let hookSource = packDir.appendingPathComponent("start.sh") + try "#!/bin/bash\necho start".write(to: hookSource, atomically: true, encoding: .utf8) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.hook", + displayName: "Start hook", + description: "Session start hook", + type: .hookFile, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + hookEvent: "SessionStart", + installAction: .copyPackFile( + source: hookSource, + destination: "start.sh", + fileType: .hook + ) + )] + ) + + try configurator.configure(packs: [pack], confirmRemovals: false) + + // Verify the file still has user content by reading raw JSON + let rawData = try Data(contentsOf: settingsPath) + let rawJSON = try JSONSerialization.jsonObject(with: rawData) as? [String: Any] + let permissions = rawJSON?["permissions"] as? [String: Any] + let allow = permissions?["allow"] as? [String] + #expect(allow?.contains("Bash(npm test)") == true) + // Hook should also be present + let result = try Settings.load(from: settingsPath) + let sessionGroups = result.hooks?["SessionStart"] ?? [] + #expect(!sessionGroups.isEmpty) + } + + @Test("Managed hooks are stripped before recompose to prevent duplication") + func stripsMangedHooksBeforeRecompose() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let configurator = makeConfigurator(home: tmpDir) + + let packDir = tmpDir.appendingPathComponent("pack/hooks") + try FileManager.default.createDirectory(at: packDir, withIntermediateDirectories: true) + let hookSource = packDir.appendingPathComponent("start.sh") + try "#!/bin/bash\necho start".write(to: hookSource, atomically: true, encoding: .utf8) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.hook", + displayName: "Start hook", + description: "Session start hook", + type: .hookFile, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + hookEvent: "SessionStart", + installAction: .copyPackFile( + source: hookSource, + destination: "start.sh", + fileType: .hook + ) + )] + ) + + // First sync + try configurator.configure(packs: [pack], confirmRemovals: false) + // Second sync + try configurator.configure(packs: [pack], confirmRemovals: false) + + let settingsPath = tmpDir.appendingPathComponent(".claude/settings.json") + let result = try Settings.load(from: settingsPath) + let sessionGroups = result.hooks?["SessionStart"] ?? [] + // Should have exactly 1 hook group, not duplicated + #expect(sessionGroups.count == 1) + } + + @Test("Corrupt settings.json throws fileOperationFailed") + func corruptJSONThrows() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let settingsPath = tmpDir.appendingPathComponent(".claude/settings.json") + try "{ invalid json".write(to: settingsPath, atomically: true, encoding: .utf8) + + let configurator = makeConfigurator(home: tmpDir) + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.hook", + displayName: "Hook", + description: "Hook", + type: .hookFile, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + hookEvent: "SessionStart", + installAction: .copyPackFile( + source: settingsPath, // dummy, won't be reached + destination: "hook.sh", + fileType: .hook + ) + )] + ) + + #expect(throws: MCSError.self) { + try configurator.configure(packs: [pack], confirmRemovals: false) + } + } + + @Test("Empty packs do not delete settings.json") + func emptyPacksDoNotDeleteSettingsJSON() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let settingsPath = tmpDir.appendingPathComponent(".claude/settings.json") + try "{}".write(to: settingsPath, atomically: true, encoding: .utf8) + + let configurator = makeConfigurator(home: tmpDir) + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack" + ) + + try configurator.configure(packs: [pack], confirmRemovals: false) + + #expect(FileManager.default.fileExists(atPath: settingsPath.path)) + } + + @Test("Hook commands use global path prefix") + func hookCommandPrefixUsesGlobalPath() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let configurator = makeConfigurator(home: tmpDir) + + let packDir = tmpDir.appendingPathComponent("pack/hooks") + try FileManager.default.createDirectory(at: packDir, withIntermediateDirectories: true) + let hookSource = packDir.appendingPathComponent("start.sh") + try "#!/bin/bash\necho start".write(to: hookSource, atomically: true, encoding: .utf8) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.hook", + displayName: "Start hook", + description: "Session start hook", + type: .hookFile, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + hookEvent: "SessionStart", + installAction: .copyPackFile( + source: hookSource, + destination: "start.sh", + fileType: .hook + ) + )] + ) + + try configurator.configure(packs: [pack], confirmRemovals: false) + + let settingsPath = tmpDir.appendingPathComponent(".claude/settings.json") + let result = try Settings.load(from: settingsPath) + let command = result.hooks?["SessionStart"]?.first?.hooks?.first?.command + #expect(command == "bash ~/.claude/hooks/start.sh") + // Must NOT use project-relative path + #expect(command?.hasPrefix("bash .claude/") != true) + } +} + +// MARK: - Global Claude Composition Tests + +struct GlobalClaudeCompositionTests { + private func makeTmpDir() throws -> URL { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("mcs-global-claude-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".claude"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".mcs"), + withIntermediateDirectories: true + ) + return dir + } + + private func makeConfigurator(home: URL) -> Configurator { + let env = Environment(home: home) + return Configurator( + environment: env, + output: CLIOutput(colorsEnabled: false), + shell: ShellRunner(environment: env), + strategy: GlobalSyncStrategy(environment: env), + claudeCLI: MockClaudeCLI() + ) + } + + @Test("Template sections are written to global CLAUDE.md") + func composesToGlobalClaudeMD() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + templates: [TemplateContribution( + sectionIdentifier: "test-pack.section", + templateContent: "## Test Section\nSome content here.", + placeholders: [] + )] + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [pack], confirmRemovals: false) + + let claudePath = tmpDir.appendingPathComponent(".claude/CLAUDE.md") + #expect(FileManager.default.fileExists(atPath: claudePath.path)) + let content = try String(contentsOf: claudePath, encoding: .utf8) + #expect(content.contains("## Test Section")) + #expect(content.contains("mcs:begin test-pack.section")) + } + + @Test("Empty contributions do not create CLAUDE.md") + func emptyContributionsSilent() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack" + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [pack], confirmRemovals: false) + + let claudePath = tmpDir.appendingPathComponent(".claude/CLAUDE.md") + #expect(!FileManager.default.fileExists(atPath: claudePath.path)) + } + + @Test("Existing user content in CLAUDE.md is preserved on re-sync") + func existingUserContentPreserved() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + // First sync: install a section so the file has markers + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + templates: [TemplateContribution( + sectionIdentifier: "test-pack.section", + templateContent: "## Pack Content", + placeholders: [] + )] + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [pack], confirmRemovals: false) + + // Simulate user appending custom content outside section markers + let claudePath = tmpDir.appendingPathComponent(".claude/CLAUDE.md") + var content = try String(contentsOf: claudePath, encoding: .utf8) + content += "\n# My Custom Content\nUser notes here.\n" + try content.write(to: claudePath, atomically: true, encoding: .utf8) + + // Re-sync the same pack — user content should survive + try configurator.configure(packs: [pack], confirmRemovals: false) + + let updated = try String(contentsOf: claudePath, encoding: .utf8) + #expect(updated.contains("# My Custom Content")) + #expect(updated.contains("User notes here.")) + #expect(updated.contains("## Pack Content")) + } + + @Test("Template sections are tracked in artifact record") + func templateSectionsTracked() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + templates: [TemplateContribution( + sectionIdentifier: "test-pack.section", + templateContent: "## Section", + placeholders: [] + )] + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [pack], confirmRemovals: false) + + let env = Environment(home: tmpDir) + let state = try ProjectState(stateFile: env.globalStateFile) + let sections = state.artifacts(for: "test-pack")?.templateSections ?? [] + #expect(sections.contains("test-pack.section")) + } +} + +// MARK: - Global File Installation Tests + +struct GlobalFileInstallationTests { + private func makeTmpDir() throws -> URL { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("mcs-global-files-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".claude"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".mcs"), + withIntermediateDirectories: true + ) + return dir + } + + private func makeConfigurator( + home: URL, + mockCLI: MockClaudeCLI = MockClaudeCLI() + ) -> Configurator { + let env = Environment(home: home) + return Configurator( + environment: env, + output: CLIOutput(colorsEnabled: false), + shell: ShellRunner(environment: env), + strategy: GlobalSyncStrategy(environment: env), + claudeCLI: mockCLI + ) + } + + @Test("Skill file installed to global skills directory") + func skillInstalledToGlobalDir() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let sourceDir = tmpDir.appendingPathComponent("pack/skills") + try FileManager.default.createDirectory(at: sourceDir, withIntermediateDirectories: true) + let source = sourceDir.appendingPathComponent("my-skill.md") + try "# My Skill".write(to: source, atomically: true, encoding: .utf8) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.skill", + displayName: "My Skill", + description: "A skill", + type: .skill, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .copyPackFile(source: source, destination: "my-skill.md", fileType: .skill) + )] + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [pack], confirmRemovals: false) + + let dest = tmpDir.appendingPathComponent(".claude/skills/my-skill.md") + #expect(FileManager.default.fileExists(atPath: dest.path)) + } + + @Test("Hook file installed to global hooks directory") + func hookInstalledToGlobalDir() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let sourceDir = tmpDir.appendingPathComponent("pack/hooks") + try FileManager.default.createDirectory(at: sourceDir, withIntermediateDirectories: true) + let source = sourceDir.appendingPathComponent("start.sh") + try "#!/bin/bash".write(to: source, atomically: true, encoding: .utf8) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.hook", + displayName: "Start Hook", + description: "A hook", + type: .hookFile, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .copyPackFile(source: source, destination: "start.sh", fileType: .hook) + )] + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [pack], confirmRemovals: false) + + let dest = tmpDir.appendingPathComponent(".claude/hooks/start.sh") + #expect(FileManager.default.fileExists(atPath: dest.path)) + } + + @Test("File path recorded relative to claude directory") + func filePathRecordedRelativeToClaudeDir() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let sourceDir = tmpDir.appendingPathComponent("pack/skills") + try FileManager.default.createDirectory(at: sourceDir, withIntermediateDirectories: true) + let source = sourceDir.appendingPathComponent("my-skill.md") + try "# Skill".write(to: source, atomically: true, encoding: .utf8) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.skill", + displayName: "My Skill", + description: "A skill", + type: .skill, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .copyPackFile(source: source, destination: "my-skill.md", fileType: .skill) + )] + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [pack], confirmRemovals: false) + + let env = Environment(home: tmpDir) + let state = try ProjectState(stateFile: env.globalStateFile) + let files = state.artifacts(for: "test-pack")?.files ?? [] + #expect(files.contains("skills/my-skill.md")) + } + + @Test("Stale file removed from global directory") + func staleFileRemovedFromGlobalDir() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let sourceDir = tmpDir.appendingPathComponent("pack/skills") + try FileManager.default.createDirectory(at: sourceDir, withIntermediateDirectories: true) + let sourceA = sourceDir.appendingPathComponent("skill-a.md") + try "skill a".write(to: sourceA, atomically: true, encoding: .utf8) + let sourceB = sourceDir.appendingPathComponent("skill-b.md") + try "skill b".write(to: sourceB, atomically: true, encoding: .utf8) + + let packV1 = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ + ComponentDefinition( + id: "test-pack.skill-a", + displayName: "Skill A", + description: "First", + type: .skill, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .copyPackFile(source: sourceA, destination: "skill-a.md", fileType: .skill) + ), + ComponentDefinition( + id: "test-pack.skill-b", + displayName: "Skill B", + description: "Second", + type: .skill, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .copyPackFile(source: sourceB, destination: "skill-b.md", fileType: .skill) + ), + ] + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [packV1], confirmRemovals: false) + + let destB = tmpDir.appendingPathComponent(".claude/skills/skill-b.md") + #expect(FileManager.default.fileExists(atPath: destB.path)) + + // Pack v2: skill-b removed + let packV2 = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ + ComponentDefinition( + id: "test-pack.skill-a", + displayName: "Skill A", + description: "First", + type: .skill, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .copyPackFile(source: sourceA, destination: "skill-a.md", fileType: .skill) + ), + ] + ) + + try configurator.configure(packs: [packV2], confirmRemovals: false) + + let destA = tmpDir.appendingPathComponent(".claude/skills/skill-a.md") + #expect(FileManager.default.fileExists(atPath: destA.path)) + #expect(!FileManager.default.fileExists(atPath: destB.path)) + } +} + +// MARK: - Global Resolve Built-In Values Tests + +struct GlobalResolveBuiltInValuesTests { + @Test("Global scope returns empty built-in values") + func resolveBuiltInValuesReturnsEmptyDict() { + let env = Environment(home: FileManager.default.temporaryDirectory) + let strategy = GlobalSyncStrategy(environment: env) + let shell = ShellRunner(environment: env) + let output = CLIOutput(colorsEnabled: false) + + let values = strategy.resolveBuiltInValues(shell: shell, output: output) + #expect(values.isEmpty) + } +} + +// MARK: - Global Convergence Flow Tests + +struct GlobalConvergenceFlowTests { + private func makeTmpDir() throws -> URL { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("mcs-global-flow-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".claude"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".mcs"), + withIntermediateDirectories: true + ) + return dir + } + + private func makeConfigurator(home: URL) -> Configurator { + let env = Environment(home: home) + return Configurator( + environment: env, + output: CLIOutput(colorsEnabled: false), + shell: ShellRunner(environment: env), + strategy: GlobalSyncStrategy(environment: env), + claudeCLI: MockClaudeCLI() + ) + } + + @Test("configureProject hooks are NOT called in global scope") + func configureProjectHooksNotCalled() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let pack = TrackingMockTechPack( + identifier: "test-pack", + displayName: "Test Pack" + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [pack], confirmRemovals: false) + + #expect(pack.configureProjectCallCount == 0) + } +} + +// MARK: - Global Unconfigure Pack Tests + +struct GlobalUnconfigurePackTests { + private func makeTmpDir() throws -> URL { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("mcs-global-unconfig-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".claude"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".mcs"), + withIntermediateDirectories: true + ) + return dir + } + + private func makeConfigurator( + home: URL, + mockCLI: MockClaudeCLI = MockClaudeCLI() + ) -> Configurator { + let env = Environment(home: home) + return Configurator( + environment: env, + output: CLIOutput(colorsEnabled: false), + shell: ShellRunner(environment: env), + strategy: GlobalSyncStrategy(environment: env), + claudeCLI: mockCLI + ) + } + + @Test("unconfigurePack removes MCP server with user scope") + func unconfigureRemovesMCPWithUserScope() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let mockCLI = MockClaudeCLI() + let configurator = makeConfigurator(home: tmpDir, mockCLI: mockCLI) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.mcp", + displayName: "MCP", + description: "MCP server", + type: .mcpServer, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .mcpServer(MCPServerConfig( + name: "my-mcp", command: "/usr/bin/true", args: [], env: [:] + )) + )] + ) + + // First sync to install + try configurator.configure(packs: [pack], confirmRemovals: false) + mockCLI.mcpRemoveCalls = [] + + // Second sync with empty packs to trigger unconfigure + try configurator.configure(packs: [], confirmRemovals: false) + + #expect(mockCLI.mcpRemoveCalls.contains( + MockClaudeCLI.MCPRemoveCall(name: "my-mcp", scope: "user") + )) + } + + @Test("unconfigurePack removes files from global directory") + func unconfigureRemovesFilesFromGlobalDir() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let sourceDir = tmpDir.appendingPathComponent("pack/skills") + try FileManager.default.createDirectory(at: sourceDir, withIntermediateDirectories: true) + let source = sourceDir.appendingPathComponent("my-skill.md") + try "# Skill".write(to: source, atomically: true, encoding: .utf8) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.skill", + displayName: "Skill", + description: "A skill", + type: .skill, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .copyPackFile(source: source, destination: "my-skill.md", fileType: .skill) + )] + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [pack], confirmRemovals: false) + + let dest = tmpDir.appendingPathComponent(".claude/skills/my-skill.md") + #expect(FileManager.default.fileExists(atPath: dest.path)) + + // Unconfigure by deselecting the pack + try configurator.configure(packs: [], confirmRemovals: false) + + #expect(!FileManager.default.fileExists(atPath: dest.path)) + } + + @Test("unconfigurePack strips template sections from CLAUDE.md") + func unconfigureStripsTemplateSections() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + templates: [TemplateContribution( + sectionIdentifier: "test-pack.section", + templateContent: "## Managed Section", + placeholders: [] + )] + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [pack], confirmRemovals: false) + + let claudePath = tmpDir.appendingPathComponent(".claude/CLAUDE.md") + let before = try String(contentsOf: claudePath, encoding: .utf8) + #expect(before.contains("## Managed Section")) + + // Unconfigure + try configurator.configure(packs: [], confirmRemovals: false) + + let after = try String(contentsOf: claudePath, encoding: .utf8) + #expect(!after.contains("## Managed Section")) + #expect(!after.contains("mcs:begin test-pack.section")) + } + + @Test("unconfigurePack strips settings keys from settings.json") + func unconfigureStripsSettingsKeys() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let configurator = makeConfigurator(home: tmpDir) + + let packDir = tmpDir.appendingPathComponent("pack/hooks") + try FileManager.default.createDirectory(at: packDir, withIntermediateDirectories: true) + let hookSource = packDir.appendingPathComponent("hook.sh") + try "#!/bin/bash".write(to: hookSource, atomically: true, encoding: .utf8) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.hook", + displayName: "Hook", + description: "A hook", + type: .hookFile, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + hookEvent: "SessionStart", + installAction: .copyPackFile( + source: hookSource, + destination: "hook.sh", + fileType: .hook + ) + )] + ) + + // Install + try configurator.configure(packs: [pack], confirmRemovals: false) + + let settingsPath = tmpDir.appendingPathComponent(".claude/settings.json") + let before = try Settings.load(from: settingsPath) + #expect(before.hooks?["SessionStart"] != nil) + + // Unconfigure + try configurator.configure(packs: [], confirmRemovals: false) + + let after = try Settings.load(from: settingsPath) + let sessionGroups = after.hooks?["SessionStart"] ?? [] + #expect(sessionGroups.isEmpty) + } + + @Test("Deselection of one pack fully unconfigures it while keeping the other") + func deselectionTriggersUnconfigure() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let mockCLI = MockClaudeCLI() + let configurator = makeConfigurator(home: tmpDir, mockCLI: mockCLI) + + let packA = MockTechPack( + identifier: "pack-a", + displayName: "Pack A", + components: [ComponentDefinition( + id: "pack-a.mcp", + displayName: "MCP A", + description: "MCP A", + type: .mcpServer, + packIdentifier: "pack-a", + dependencies: [], + isRequired: true, + installAction: .mcpServer(MCPServerConfig( + name: "mcp-a", command: "/usr/bin/true", args: [], env: [:] + )) + )] + ) + + let packB = MockTechPack( + identifier: "pack-b", + displayName: "Pack B", + components: [ComponentDefinition( + id: "pack-b.mcp", + displayName: "MCP B", + description: "MCP B", + type: .mcpServer, + packIdentifier: "pack-b", + dependencies: [], + isRequired: true, + installAction: .mcpServer(MCPServerConfig( + name: "mcp-b", command: "/usr/bin/true", args: [], env: [:] + )) + )] + ) + + // First sync: both packs + try configurator.configure(packs: [packA, packB], confirmRemovals: false) + mockCLI.mcpRemoveCalls = [] + + // Second sync: only pack-b (deselect pack-a) + try configurator.configure(packs: [packB], confirmRemovals: false) + + // pack-a's MCP should be removed + #expect(mockCLI.mcpRemoveCalls.contains( + MockClaudeCLI.MCPRemoveCall(name: "mcp-a", scope: "user") + )) + // pack-b's MCP should NOT be removed + #expect(!mockCLI.mcpRemoveCalls.contains( + MockClaudeCLI.MCPRemoveCall(name: "mcp-b", scope: "user") + )) + + let env = Environment(home: tmpDir) + let state = try ProjectState(stateFile: env.globalStateFile) + #expect(state.configuredPacks.contains("pack-b")) + #expect(!state.configuredPacks.contains("pack-a")) + } +} + +// MARK: - Global Dry Run Tests + +struct GlobalDryRunTests { + private func makeTmpDir() throws -> URL { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("mcs-global-dryrun-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".claude"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".mcs"), + withIntermediateDirectories: true + ) + return dir + } + + private func makeConfigurator(home: URL) -> Configurator { + let env = Environment(home: home) + return Configurator( + environment: env, + output: CLIOutput(colorsEnabled: false), + shell: ShellRunner(environment: env), + strategy: GlobalSyncStrategy(environment: env), + claudeCLI: MockClaudeCLI() + ) + } + + @Test("Dry run does not create any files in global scope") + func dryRunNoFilesCreated() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ComponentDefinition( + id: "test-pack.mcp", + displayName: "MCP", + description: "MCP", + type: .mcpServer, + packIdentifier: "test-pack", + dependencies: [], + isRequired: true, + installAction: .mcpServer(MCPServerConfig( + name: "my-mcp", command: "/usr/bin/true", args: [], env: [:] + )) + )], + templates: [TemplateContribution( + sectionIdentifier: "test-pack.section", + templateContent: "## Section", + placeholders: [] + )] + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.dryRun(packs: [pack]) + + // No state file should be created + let env = Environment(home: tmpDir) + #expect(!FileManager.default.fileExists(atPath: env.globalStateFile.path)) + // No CLAUDE.md + #expect(!FileManager.default.fileExists(atPath: env.globalClaudeMD.path)) + // No settings.json + #expect(!FileManager.default.fileExists(atPath: env.claudeSettings.path)) + } + + @Test("Dry run preserves existing global state") + func dryRunPreservesState() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let configurator = makeConfigurator(home: tmpDir) + + // Install a pack first + let packA = MockTechPack( + identifier: "pack-a", + displayName: "Pack A", + components: [ComponentDefinition( + id: "pack-a.mcp", + displayName: "MCP A", + description: "MCP A", + type: .mcpServer, + packIdentifier: "pack-a", + dependencies: [], + isRequired: true, + installAction: .mcpServer(MCPServerConfig( + name: "mcp-a", command: "/usr/bin/true", args: [], env: [:] + )) + )] + ) + try configurator.configure(packs: [packA], confirmRemovals: false) + + let env = Environment(home: tmpDir) + let stateBefore = try ProjectState(stateFile: env.globalStateFile) + #expect(stateBefore.configuredPacks.contains("pack-a")) + + // Dry run with different pack + let packB = MockTechPack( + identifier: "pack-b", + displayName: "Pack B" + ) + try configurator.dryRun(packs: [packB]) + + // State should be unchanged + let stateAfter = try ProjectState(stateFile: env.globalStateFile) + #expect(stateAfter.configuredPacks.contains("pack-a")) + #expect(!stateAfter.configuredPacks.contains("pack-b")) + } +} + +// MARK: - Global Excluded Components Tests + +struct GlobalExcludedComponentsTests { + private func makeTmpDir() throws -> URL { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("mcs-global-exclude-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".claude"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".mcs"), + withIntermediateDirectories: true + ) + return dir + } + + private func makeConfigurator( + home: URL, + mockCLI: MockClaudeCLI = MockClaudeCLI() + ) -> Configurator { + let env = Environment(home: home) + return Configurator( + environment: env, + output: CLIOutput(colorsEnabled: false), + shell: ShellRunner(environment: env), + strategy: GlobalSyncStrategy(environment: env), + claudeCLI: mockCLI + ) + } + + @Test("Excluded MCP server is removed with user scope") + func excludedMCPRemovedWithUserScope() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let mockCLI = MockClaudeCLI() + let configurator = makeConfigurator(home: tmpDir, mockCLI: mockCLI) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ + ComponentDefinition( + id: "test-pack.mcp-a", + displayName: "MCP A", + description: "Kept", + type: .mcpServer, + packIdentifier: "test-pack", + dependencies: [], + isRequired: false, + installAction: .mcpServer(MCPServerConfig( + name: "mcp-a", command: "/usr/bin/true", args: [], env: [:] + )) + ), + ComponentDefinition( + id: "test-pack.mcp-b", + displayName: "MCP B", + description: "To exclude", + type: .mcpServer, + packIdentifier: "test-pack", + dependencies: [], + isRequired: false, + installAction: .mcpServer(MCPServerConfig( + name: "mcp-b", command: "/usr/bin/true", args: [], env: [:] + )) + ), + ] + ) + + // First sync: both installed + try configurator.configure(packs: [pack], confirmRemovals: false) + mockCLI.mcpRemoveCalls = [] + + // Second sync: exclude mcp-b + try configurator.configure( + packs: [pack], + confirmRemovals: false, + excludedComponents: ["test-pack": ["test-pack.mcp-b"]] + ) + + #expect(mockCLI.mcpRemoveCalls.contains( + MockClaudeCLI.MCPRemoveCall(name: "mcp-b", scope: "user") + )) + } + + @Test("Excluded file is removed from global directory") + func excludedFileRemovedFromGlobalDir() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let sourceDir = tmpDir.appendingPathComponent("pack/skills") + try FileManager.default.createDirectory(at: sourceDir, withIntermediateDirectories: true) + let sourceA = sourceDir.appendingPathComponent("skill-a.md") + try "skill a".write(to: sourceA, atomically: true, encoding: .utf8) + let sourceB = sourceDir.appendingPathComponent("skill-b.md") + try "skill b".write(to: sourceB, atomically: true, encoding: .utf8) + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack", + components: [ + ComponentDefinition( + id: "test-pack.skill-a", + displayName: "Skill A", + description: "First", + type: .skill, + packIdentifier: "test-pack", + dependencies: [], + isRequired: false, + installAction: .copyPackFile(source: sourceA, destination: "skill-a.md", fileType: .skill) + ), + ComponentDefinition( + id: "test-pack.skill-b", + displayName: "Skill B", + description: "Second", + type: .skill, + packIdentifier: "test-pack", + dependencies: [], + isRequired: false, + installAction: .copyPackFile(source: sourceB, destination: "skill-b.md", fileType: .skill) + ), + ] + ) + + let configurator = makeConfigurator(home: tmpDir) + + // First sync: both installed + try configurator.configure(packs: [pack], confirmRemovals: false) + + let destB = tmpDir.appendingPathComponent(".claude/skills/skill-b.md") + #expect(FileManager.default.fileExists(atPath: destB.path)) + + // Second sync: exclude skill-b + try configurator.configure( + packs: [pack], + confirmRemovals: false, + excludedComponents: ["test-pack": ["test-pack.skill-b"]] + ) + + #expect(!FileManager.default.fileExists(atPath: destB.path)) + let destA = tmpDir.appendingPathComponent(".claude/skills/skill-a.md") + #expect(FileManager.default.fileExists(atPath: destA.path)) + } +} + +// MARK: - Global State and Index Tests + +struct GlobalStateAndIndexTests { + private func makeTmpDir() throws -> URL { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("mcs-global-state-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".claude"), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".mcs"), + withIntermediateDirectories: true + ) + return dir + } + + private func makeConfigurator(home: URL) -> Configurator { + let env = Environment(home: home) + return Configurator( + environment: env, + output: CLIOutput(colorsEnabled: false), + shell: ShellRunner(environment: env), + strategy: GlobalSyncStrategy(environment: env), + claudeCLI: MockClaudeCLI() + ) + } + + @Test("State file saved to global location") + func stateFileSavedToGlobalLocation() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack" + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [pack], confirmRemovals: false) + + let env = Environment(home: tmpDir) + #expect(FileManager.default.fileExists(atPath: env.globalStateFile.path)) + } + + @Test("Project index uses global sentinel identifier") + func projectIndexUsesGlobalSentinel() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let pack = MockTechPack( + identifier: "test-pack", + displayName: "Test Pack" + ) + + let configurator = makeConfigurator(home: tmpDir) + try configurator.configure(packs: [pack], confirmRemovals: false) + + let env = Environment(home: tmpDir) + let index = ProjectIndex(path: env.projectsIndexFile) + let data = try index.load() + let globalProjects = index.projects(withPack: "test-pack", in: data) + let globalEntry = globalProjects.first { $0.path == ProjectIndex.globalSentinel } + #expect(globalEntry != nil) + } +} diff --git a/Tests/MCSTests/ProjectConfiguratorTests.swift b/Tests/MCSTests/ProjectConfiguratorTests.swift index 87185e5..02b412f 100644 --- a/Tests/MCSTests/ProjectConfiguratorTests.swift +++ b/Tests/MCSTests/ProjectConfiguratorTests.swift @@ -1658,96 +1658,6 @@ struct CorruptStateAbortTests { } } -/// Mock `ClaudeCLI` that records calls without executing real shell commands. -private final class MockClaudeCLI: ClaudeCLI, @unchecked Sendable { - struct MCPAddCall: Equatable { - let name: String - let scope: String - let arguments: [String] - } - - struct MCPRemoveCall: Equatable { - let name: String - let scope: String - } - - struct PluginCall: Equatable { - let name: String - } - - var isAvailable: Bool { - true - } - - var mcpAddCalls: [MCPAddCall] = [] - var mcpRemoveCalls: [MCPRemoveCall] = [] - var pluginMarketplaceAddCalls: [String] = [] - var pluginInstallCalls: [PluginCall] = [] - var pluginRemoveCalls: [PluginCall] = [] - - /// Result to return from all operations. Defaults to success. - var result = ShellResult(exitCode: 0, stdout: "", stderr: "") - - @discardableResult - func mcpAdd(name: String, scope: String, arguments: [String]) -> ShellResult { - mcpAddCalls.append(MCPAddCall(name: name, scope: scope, arguments: arguments)) - return result - } - - @discardableResult - func mcpRemove(name: String, scope: String) -> ShellResult { - mcpRemoveCalls.append(MCPRemoveCall(name: name, scope: scope)) - return result - } - - @discardableResult - func pluginMarketplaceAdd(repo: String) -> ShellResult { - pluginMarketplaceAddCalls.append(repo) - return result - } - - @discardableResult - func pluginInstall(ref: PluginRef) -> ShellResult { - pluginInstallCalls.append(PluginCall(name: ref.bareName)) - return result - } - - @discardableResult - func pluginRemove(ref: PluginRef) -> ShellResult { - pluginRemoveCalls.append(PluginCall(name: ref.bareName)) - return result - } -} - -/// Minimal TechPack implementation for tests. -private struct MockTechPack: TechPack { - let identifier: String - let displayName: String - let description: String = "Mock pack for testing" - let components: [ComponentDefinition] - let templates: [TemplateContribution] - private let storedChecks: [any DoctorCheck] - - init( - identifier: String, - displayName: String, - components: [ComponentDefinition] = [], - templates: [TemplateContribution] = [], - supplementaryDoctorChecks: [any DoctorCheck] = [] - ) { - self.identifier = identifier - self.displayName = displayName - self.components = components - self.templates = templates - storedChecks = supplementaryDoctorChecks - } - - func supplementaryDoctorChecks(projectRoot _: URL?) -> [any DoctorCheck] { - storedChecks - } - - func configureProject(at _: URL, context _: ProjectConfigContext) throws {} -} // MARK: - parseRepoName Tests diff --git a/Tests/MCSTests/TestHelpers.swift b/Tests/MCSTests/TestHelpers.swift new file mode 100644 index 0000000..0e1d570 --- /dev/null +++ b/Tests/MCSTests/TestHelpers.swift @@ -0,0 +1,123 @@ +import Foundation +@testable import mcs + +/// Mock `ClaudeCLI` that records calls without executing real shell commands. +final class MockClaudeCLI: ClaudeCLI, @unchecked Sendable { + struct MCPAddCall: Equatable { + let name: String + let scope: String + let arguments: [String] + } + + struct MCPRemoveCall: Equatable { + let name: String + let scope: String + } + + struct PluginCall: Equatable { + let name: String + } + + var isAvailable: Bool { + true + } + + var mcpAddCalls: [MCPAddCall] = [] + var mcpRemoveCalls: [MCPRemoveCall] = [] + var pluginMarketplaceAddCalls: [String] = [] + var pluginInstallCalls: [PluginCall] = [] + var pluginRemoveCalls: [PluginCall] = [] + + /// Result to return from all operations. Defaults to success. + var result = ShellResult(exitCode: 0, stdout: "", stderr: "") + + @discardableResult + func mcpAdd(name: String, scope: String, arguments: [String]) -> ShellResult { + mcpAddCalls.append(MCPAddCall(name: name, scope: scope, arguments: arguments)) + return result + } + + @discardableResult + func mcpRemove(name: String, scope: String) -> ShellResult { + mcpRemoveCalls.append(MCPRemoveCall(name: name, scope: scope)) + return result + } + + @discardableResult + func pluginMarketplaceAdd(repo: String) -> ShellResult { + pluginMarketplaceAddCalls.append(repo) + return result + } + + @discardableResult + func pluginInstall(ref: PluginRef) -> ShellResult { + pluginInstallCalls.append(PluginCall(name: ref.bareName)) + return result + } + + @discardableResult + func pluginRemove(ref: PluginRef) -> ShellResult { + pluginRemoveCalls.append(PluginCall(name: ref.bareName)) + return result + } +} + +/// Minimal TechPack implementation for tests. +struct MockTechPack: TechPack { + let identifier: String + let displayName: String + let description: String = "Mock pack for testing" + let components: [ComponentDefinition] + let templates: [TemplateContribution] + private let storedChecks: [any DoctorCheck] + + init( + identifier: String, + displayName: String, + components: [ComponentDefinition] = [], + templates: [TemplateContribution] = [], + supplementaryDoctorChecks: [any DoctorCheck] = [] + ) { + self.identifier = identifier + self.displayName = displayName + self.components = components + self.templates = templates + storedChecks = supplementaryDoctorChecks + } + + func supplementaryDoctorChecks(projectRoot _: URL?) -> [any DoctorCheck] { + storedChecks + } + + func configureProject(at _: URL, context _: ProjectConfigContext) throws {} +} + +/// Mock TechPack that tracks `configureProject` invocations. +final class TrackingMockTechPack: TechPack, @unchecked Sendable { + let identifier: String + let displayName: String + let description: String = "Tracking mock pack" + let components: [ComponentDefinition] + let templates: [TemplateContribution] + var configureProjectCallCount = 0 + + init( + identifier: String, + displayName: String, + components: [ComponentDefinition] = [], + templates: [TemplateContribution] = [] + ) { + self.identifier = identifier + self.displayName = displayName + self.components = components + self.templates = templates + } + + func supplementaryDoctorChecks(projectRoot _: URL?) -> [any DoctorCheck] { + [] + } + + func configureProject(at _: URL, context _: ProjectConfigContext) throws { + configureProjectCallCount += 1 + } +} From 9a5983afb459a32cd721a4f46e2106bd1626960a Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Fri, 6 Mar 2026 09:10:33 +0100 Subject: [PATCH 2/4] Simplify global test helpers and strengthen assertions - Extract shared makeGlobalTmpDir() and makeGlobalConfigurator() into TestHelpers.swift, removing 9 duplicated copies - Replace stringly-typed "user" scope with Constants.MCPScope.user - Strengthen corrupt JSON test to assert MCSError.fileOperationFailed specifically --- Tests/MCSTests/GlobalConfiguratorTests.swift | 387 ++++--------------- Tests/MCSTests/TestHelpers.swift | 33 ++ 2 files changed, 103 insertions(+), 317 deletions(-) diff --git a/Tests/MCSTests/GlobalConfiguratorTests.swift b/Tests/MCSTests/GlobalConfiguratorTests.swift index 1f95aa8..bfe9fb1 100644 --- a/Tests/MCSTests/GlobalConfiguratorTests.swift +++ b/Tests/MCSTests/GlobalConfiguratorTests.swift @@ -5,43 +5,13 @@ import Testing // MARK: - Global MCP Scope Tests struct GlobalMCPScopeTests { - private func makeTmpDir() throws -> URL { - let dir = FileManager.default.temporaryDirectory - .appendingPathComponent("mcs-global-mcp-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) - // Create required subdirectories for global scope - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".claude"), - withIntermediateDirectories: true - ) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".mcs"), - withIntermediateDirectories: true - ) - return dir - } - - private func makeConfigurator( - home: URL, - mockCLI: MockClaudeCLI = MockClaudeCLI() - ) -> Configurator { - let env = Environment(home: home) - return Configurator( - environment: env, - output: CLIOutput(colorsEnabled: false), - shell: ShellRunner(environment: env), - strategy: GlobalSyncStrategy(environment: env), - claudeCLI: mockCLI - ) - } - @Test("MCP server is registered with user scope regardless of pack declaration") func mcpServerUsesUserScope() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir(label: "mcp") defer { try? FileManager.default.removeItem(at: tmpDir) } let mockCLI = MockClaudeCLI() - let configurator = makeConfigurator(home: tmpDir, mockCLI: mockCLI) + let configurator = makeGlobalConfigurator(home: tmpDir, mockCLI: mockCLI) let pack = MockTechPack( identifier: "test-pack", @@ -63,16 +33,16 @@ struct GlobalMCPScopeTests { try configurator.configure(packs: [pack], confirmRemovals: false) #expect(mockCLI.mcpAddCalls.count == 1) - #expect(mockCLI.mcpAddCalls.first?.scope == "user") + #expect(mockCLI.mcpAddCalls.first?.scope == Constants.MCPScope.user) } @Test("MCP scope override ignores explicit local scope in pack config") func mcpScopeOverrideIgnoresLocalDeclaration() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let mockCLI = MockClaudeCLI() - let configurator = makeConfigurator(home: tmpDir, mockCLI: mockCLI) + let configurator = makeGlobalConfigurator(home: tmpDir, mockCLI: mockCLI) let pack = MockTechPack( identifier: "test-pack", @@ -94,15 +64,15 @@ struct GlobalMCPScopeTests { try configurator.configure(packs: [pack], confirmRemovals: false) - #expect(mockCLI.mcpAddCalls.first?.scope == "user") + #expect(mockCLI.mcpAddCalls.first?.scope == Constants.MCPScope.user) } @Test("Artifact record stores user scope for MCP server") func mcpArtifactRecordsUserScope() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) let pack = MockTechPack( identifier: "test-pack", @@ -126,17 +96,17 @@ struct GlobalMCPScopeTests { let env = Environment(home: tmpDir) let state = try ProjectState(stateFile: env.globalStateFile) let artifacts = state.artifacts(for: "test-pack") - #expect(artifacts?.mcpServers.first?.scope == "user") + #expect(artifacts?.mcpServers.first?.scope == Constants.MCPScope.user) #expect(artifacts?.mcpServers.first?.name == "my-mcp") } @Test("Stale MCP server is removed with user scope") func staleMCPRemovedWithUserScope() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let mockCLI = MockClaudeCLI() - let configurator = makeConfigurator(home: tmpDir, mockCLI: mockCLI) + let configurator = makeGlobalConfigurator(home: tmpDir, mockCLI: mockCLI) // Pack v1: two MCP servers let packV1 = MockTechPack( @@ -196,7 +166,7 @@ struct GlobalMCPScopeTests { try configurator.configure(packs: [packV2], confirmRemovals: false) #expect(mockCLI.mcpRemoveCalls.contains( - MockClaudeCLI.MCPRemoveCall(name: "mcp-drop", scope: "user") + MockClaudeCLI.MCPRemoveCall(name: "mcp-drop", scope: Constants.MCPScope.user) )) } } @@ -204,38 +174,9 @@ struct GlobalMCPScopeTests { // MARK: - Global Settings Composition Tests struct GlobalSettingsCompositionTests { - private func makeTmpDir() throws -> URL { - let dir = FileManager.default.temporaryDirectory - .appendingPathComponent("mcs-global-settings-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".claude"), - withIntermediateDirectories: true - ) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".mcs"), - withIntermediateDirectories: true - ) - return dir - } - - private func makeConfigurator( - home: URL, - mockCLI: MockClaudeCLI = MockClaudeCLI() - ) -> Configurator { - let env = Environment(home: home) - return Configurator( - environment: env, - output: CLIOutput(colorsEnabled: false), - shell: ShellRunner(environment: env), - strategy: GlobalSyncStrategy(environment: env), - claudeCLI: mockCLI - ) - } - @Test("Existing user settings are preserved after global sync") func preservesExistingUserSettings() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } // Pre-write settings.json with user content @@ -245,7 +186,7 @@ struct GlobalSettingsCompositionTests { """ try userSettings.write(to: settingsPath, atomically: true, encoding: .utf8) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) // Pack with a hook file that will add hooks to settings let packDir = tmpDir.appendingPathComponent("pack/hooks") @@ -289,10 +230,10 @@ struct GlobalSettingsCompositionTests { @Test("Managed hooks are stripped before recompose to prevent duplication") func stripsMangedHooksBeforeRecompose() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) let packDir = tmpDir.appendingPathComponent("pack/hooks") try FileManager.default.createDirectory(at: packDir, withIntermediateDirectories: true) @@ -333,13 +274,13 @@ struct GlobalSettingsCompositionTests { @Test("Corrupt settings.json throws fileOperationFailed") func corruptJSONThrows() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let settingsPath = tmpDir.appendingPathComponent(".claude/settings.json") try "{ invalid json".write(to: settingsPath, atomically: true, encoding: .utf8) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) let pack = MockTechPack( identifier: "test-pack", displayName: "Test Pack", @@ -360,20 +301,23 @@ struct GlobalSettingsCompositionTests { )] ) - #expect(throws: MCSError.self) { + #expect { try configurator.configure(packs: [pack], confirmRemovals: false) + } throws: { error in + guard case MCSError.fileOperationFailed = error else { return false } + return true } } @Test("Empty packs do not delete settings.json") func emptyPacksDoNotDeleteSettingsJSON() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let settingsPath = tmpDir.appendingPathComponent(".claude/settings.json") try "{}".write(to: settingsPath, atomically: true, encoding: .utf8) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) let pack = MockTechPack( identifier: "test-pack", displayName: "Test Pack" @@ -386,10 +330,10 @@ struct GlobalSettingsCompositionTests { @Test("Hook commands use global path prefix") func hookCommandPrefixUsesGlobalPath() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) let packDir = tmpDir.appendingPathComponent("pack/hooks") try FileManager.default.createDirectory(at: packDir, withIntermediateDirectories: true) @@ -430,35 +374,9 @@ struct GlobalSettingsCompositionTests { // MARK: - Global Claude Composition Tests struct GlobalClaudeCompositionTests { - private func makeTmpDir() throws -> URL { - let dir = FileManager.default.temporaryDirectory - .appendingPathComponent("mcs-global-claude-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".claude"), - withIntermediateDirectories: true - ) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".mcs"), - withIntermediateDirectories: true - ) - return dir - } - - private func makeConfigurator(home: URL) -> Configurator { - let env = Environment(home: home) - return Configurator( - environment: env, - output: CLIOutput(colorsEnabled: false), - shell: ShellRunner(environment: env), - strategy: GlobalSyncStrategy(environment: env), - claudeCLI: MockClaudeCLI() - ) - } - @Test("Template sections are written to global CLAUDE.md") func composesToGlobalClaudeMD() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let pack = MockTechPack( @@ -471,7 +389,7 @@ struct GlobalClaudeCompositionTests { )] ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [pack], confirmRemovals: false) let claudePath = tmpDir.appendingPathComponent(".claude/CLAUDE.md") @@ -483,7 +401,7 @@ struct GlobalClaudeCompositionTests { @Test("Empty contributions do not create CLAUDE.md") func emptyContributionsSilent() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let pack = MockTechPack( @@ -491,7 +409,7 @@ struct GlobalClaudeCompositionTests { displayName: "Test Pack" ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [pack], confirmRemovals: false) let claudePath = tmpDir.appendingPathComponent(".claude/CLAUDE.md") @@ -500,7 +418,7 @@ struct GlobalClaudeCompositionTests { @Test("Existing user content in CLAUDE.md is preserved on re-sync") func existingUserContentPreserved() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } // First sync: install a section so the file has markers @@ -514,7 +432,7 @@ struct GlobalClaudeCompositionTests { )] ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [pack], confirmRemovals: false) // Simulate user appending custom content outside section markers @@ -534,7 +452,7 @@ struct GlobalClaudeCompositionTests { @Test("Template sections are tracked in artifact record") func templateSectionsTracked() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let pack = MockTechPack( @@ -547,7 +465,7 @@ struct GlobalClaudeCompositionTests { )] ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [pack], confirmRemovals: false) let env = Environment(home: tmpDir) @@ -560,38 +478,9 @@ struct GlobalClaudeCompositionTests { // MARK: - Global File Installation Tests struct GlobalFileInstallationTests { - private func makeTmpDir() throws -> URL { - let dir = FileManager.default.temporaryDirectory - .appendingPathComponent("mcs-global-files-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".claude"), - withIntermediateDirectories: true - ) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".mcs"), - withIntermediateDirectories: true - ) - return dir - } - - private func makeConfigurator( - home: URL, - mockCLI: MockClaudeCLI = MockClaudeCLI() - ) -> Configurator { - let env = Environment(home: home) - return Configurator( - environment: env, - output: CLIOutput(colorsEnabled: false), - shell: ShellRunner(environment: env), - strategy: GlobalSyncStrategy(environment: env), - claudeCLI: mockCLI - ) - } - @Test("Skill file installed to global skills directory") func skillInstalledToGlobalDir() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let sourceDir = tmpDir.appendingPathComponent("pack/skills") @@ -614,7 +503,7 @@ struct GlobalFileInstallationTests { )] ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [pack], confirmRemovals: false) let dest = tmpDir.appendingPathComponent(".claude/skills/my-skill.md") @@ -623,7 +512,7 @@ struct GlobalFileInstallationTests { @Test("Hook file installed to global hooks directory") func hookInstalledToGlobalDir() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let sourceDir = tmpDir.appendingPathComponent("pack/hooks") @@ -646,7 +535,7 @@ struct GlobalFileInstallationTests { )] ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [pack], confirmRemovals: false) let dest = tmpDir.appendingPathComponent(".claude/hooks/start.sh") @@ -655,7 +544,7 @@ struct GlobalFileInstallationTests { @Test("File path recorded relative to claude directory") func filePathRecordedRelativeToClaudeDir() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let sourceDir = tmpDir.appendingPathComponent("pack/skills") @@ -678,7 +567,7 @@ struct GlobalFileInstallationTests { )] ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [pack], confirmRemovals: false) let env = Environment(home: tmpDir) @@ -689,7 +578,7 @@ struct GlobalFileInstallationTests { @Test("Stale file removed from global directory") func staleFileRemovedFromGlobalDir() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let sourceDir = tmpDir.appendingPathComponent("pack/skills") @@ -726,7 +615,7 @@ struct GlobalFileInstallationTests { ] ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [packV1], confirmRemovals: false) let destB = tmpDir.appendingPathComponent(".claude/skills/skill-b.md") @@ -776,35 +665,9 @@ struct GlobalResolveBuiltInValuesTests { // MARK: - Global Convergence Flow Tests struct GlobalConvergenceFlowTests { - private func makeTmpDir() throws -> URL { - let dir = FileManager.default.temporaryDirectory - .appendingPathComponent("mcs-global-flow-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".claude"), - withIntermediateDirectories: true - ) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".mcs"), - withIntermediateDirectories: true - ) - return dir - } - - private func makeConfigurator(home: URL) -> Configurator { - let env = Environment(home: home) - return Configurator( - environment: env, - output: CLIOutput(colorsEnabled: false), - shell: ShellRunner(environment: env), - strategy: GlobalSyncStrategy(environment: env), - claudeCLI: MockClaudeCLI() - ) - } - @Test("configureProject hooks are NOT called in global scope") func configureProjectHooksNotCalled() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let pack = TrackingMockTechPack( @@ -812,7 +675,7 @@ struct GlobalConvergenceFlowTests { displayName: "Test Pack" ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [pack], confirmRemovals: false) #expect(pack.configureProjectCallCount == 0) @@ -822,42 +685,13 @@ struct GlobalConvergenceFlowTests { // MARK: - Global Unconfigure Pack Tests struct GlobalUnconfigurePackTests { - private func makeTmpDir() throws -> URL { - let dir = FileManager.default.temporaryDirectory - .appendingPathComponent("mcs-global-unconfig-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".claude"), - withIntermediateDirectories: true - ) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".mcs"), - withIntermediateDirectories: true - ) - return dir - } - - private func makeConfigurator( - home: URL, - mockCLI: MockClaudeCLI = MockClaudeCLI() - ) -> Configurator { - let env = Environment(home: home) - return Configurator( - environment: env, - output: CLIOutput(colorsEnabled: false), - shell: ShellRunner(environment: env), - strategy: GlobalSyncStrategy(environment: env), - claudeCLI: mockCLI - ) - } - @Test("unconfigurePack removes MCP server with user scope") func unconfigureRemovesMCPWithUserScope() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let mockCLI = MockClaudeCLI() - let configurator = makeConfigurator(home: tmpDir, mockCLI: mockCLI) + let configurator = makeGlobalConfigurator(home: tmpDir, mockCLI: mockCLI) let pack = MockTechPack( identifier: "test-pack", @@ -884,13 +718,13 @@ struct GlobalUnconfigurePackTests { try configurator.configure(packs: [], confirmRemovals: false) #expect(mockCLI.mcpRemoveCalls.contains( - MockClaudeCLI.MCPRemoveCall(name: "my-mcp", scope: "user") + MockClaudeCLI.MCPRemoveCall(name: "my-mcp", scope: Constants.MCPScope.user) )) } @Test("unconfigurePack removes files from global directory") func unconfigureRemovesFilesFromGlobalDir() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let sourceDir = tmpDir.appendingPathComponent("pack/skills") @@ -913,7 +747,7 @@ struct GlobalUnconfigurePackTests { )] ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [pack], confirmRemovals: false) let dest = tmpDir.appendingPathComponent(".claude/skills/my-skill.md") @@ -927,7 +761,7 @@ struct GlobalUnconfigurePackTests { @Test("unconfigurePack strips template sections from CLAUDE.md") func unconfigureStripsTemplateSections() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let pack = MockTechPack( @@ -940,7 +774,7 @@ struct GlobalUnconfigurePackTests { )] ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [pack], confirmRemovals: false) let claudePath = tmpDir.appendingPathComponent(".claude/CLAUDE.md") @@ -957,10 +791,10 @@ struct GlobalUnconfigurePackTests { @Test("unconfigurePack strips settings keys from settings.json") func unconfigureStripsSettingsKeys() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) let packDir = tmpDir.appendingPathComponent("pack/hooks") try FileManager.default.createDirectory(at: packDir, withIntermediateDirectories: true) @@ -1004,11 +838,11 @@ struct GlobalUnconfigurePackTests { @Test("Deselection of one pack fully unconfigures it while keeping the other") func deselectionTriggersUnconfigure() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let mockCLI = MockClaudeCLI() - let configurator = makeConfigurator(home: tmpDir, mockCLI: mockCLI) + let configurator = makeGlobalConfigurator(home: tmpDir, mockCLI: mockCLI) let packA = MockTechPack( identifier: "pack-a", @@ -1053,11 +887,11 @@ struct GlobalUnconfigurePackTests { // pack-a's MCP should be removed #expect(mockCLI.mcpRemoveCalls.contains( - MockClaudeCLI.MCPRemoveCall(name: "mcp-a", scope: "user") + MockClaudeCLI.MCPRemoveCall(name: "mcp-a", scope: Constants.MCPScope.user) )) // pack-b's MCP should NOT be removed #expect(!mockCLI.mcpRemoveCalls.contains( - MockClaudeCLI.MCPRemoveCall(name: "mcp-b", scope: "user") + MockClaudeCLI.MCPRemoveCall(name: "mcp-b", scope: Constants.MCPScope.user) )) let env = Environment(home: tmpDir) @@ -1070,35 +904,9 @@ struct GlobalUnconfigurePackTests { // MARK: - Global Dry Run Tests struct GlobalDryRunTests { - private func makeTmpDir() throws -> URL { - let dir = FileManager.default.temporaryDirectory - .appendingPathComponent("mcs-global-dryrun-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".claude"), - withIntermediateDirectories: true - ) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".mcs"), - withIntermediateDirectories: true - ) - return dir - } - - private func makeConfigurator(home: URL) -> Configurator { - let env = Environment(home: home) - return Configurator( - environment: env, - output: CLIOutput(colorsEnabled: false), - shell: ShellRunner(environment: env), - strategy: GlobalSyncStrategy(environment: env), - claudeCLI: MockClaudeCLI() - ) - } - @Test("Dry run does not create any files in global scope") func dryRunNoFilesCreated() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let pack = MockTechPack( @@ -1123,7 +931,7 @@ struct GlobalDryRunTests { )] ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.dryRun(packs: [pack]) // No state file should be created @@ -1137,10 +945,10 @@ struct GlobalDryRunTests { @Test("Dry run preserves existing global state") func dryRunPreservesState() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) // Install a pack first let packA = MockTechPack( @@ -1182,42 +990,13 @@ struct GlobalDryRunTests { // MARK: - Global Excluded Components Tests struct GlobalExcludedComponentsTests { - private func makeTmpDir() throws -> URL { - let dir = FileManager.default.temporaryDirectory - .appendingPathComponent("mcs-global-exclude-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".claude"), - withIntermediateDirectories: true - ) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".mcs"), - withIntermediateDirectories: true - ) - return dir - } - - private func makeConfigurator( - home: URL, - mockCLI: MockClaudeCLI = MockClaudeCLI() - ) -> Configurator { - let env = Environment(home: home) - return Configurator( - environment: env, - output: CLIOutput(colorsEnabled: false), - shell: ShellRunner(environment: env), - strategy: GlobalSyncStrategy(environment: env), - claudeCLI: mockCLI - ) - } - @Test("Excluded MCP server is removed with user scope") func excludedMCPRemovedWithUserScope() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let mockCLI = MockClaudeCLI() - let configurator = makeConfigurator(home: tmpDir, mockCLI: mockCLI) + let configurator = makeGlobalConfigurator(home: tmpDir, mockCLI: mockCLI) let pack = MockTechPack( identifier: "test-pack", @@ -1262,13 +1041,13 @@ struct GlobalExcludedComponentsTests { ) #expect(mockCLI.mcpRemoveCalls.contains( - MockClaudeCLI.MCPRemoveCall(name: "mcp-b", scope: "user") + MockClaudeCLI.MCPRemoveCall(name: "mcp-b", scope: Constants.MCPScope.user) )) } @Test("Excluded file is removed from global directory") func excludedFileRemovedFromGlobalDir() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let sourceDir = tmpDir.appendingPathComponent("pack/skills") @@ -1305,7 +1084,7 @@ struct GlobalExcludedComponentsTests { ] ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) // First sync: both installed try configurator.configure(packs: [pack], confirmRemovals: false) @@ -1329,35 +1108,9 @@ struct GlobalExcludedComponentsTests { // MARK: - Global State and Index Tests struct GlobalStateAndIndexTests { - private func makeTmpDir() throws -> URL { - let dir = FileManager.default.temporaryDirectory - .appendingPathComponent("mcs-global-state-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".claude"), - withIntermediateDirectories: true - ) - try FileManager.default.createDirectory( - at: dir.appendingPathComponent(".mcs"), - withIntermediateDirectories: true - ) - return dir - } - - private func makeConfigurator(home: URL) -> Configurator { - let env = Environment(home: home) - return Configurator( - environment: env, - output: CLIOutput(colorsEnabled: false), - shell: ShellRunner(environment: env), - strategy: GlobalSyncStrategy(environment: env), - claudeCLI: MockClaudeCLI() - ) - } - @Test("State file saved to global location") func stateFileSavedToGlobalLocation() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let pack = MockTechPack( @@ -1365,7 +1118,7 @@ struct GlobalStateAndIndexTests { displayName: "Test Pack" ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [pack], confirmRemovals: false) let env = Environment(home: tmpDir) @@ -1374,7 +1127,7 @@ struct GlobalStateAndIndexTests { @Test("Project index uses global sentinel identifier") func projectIndexUsesGlobalSentinel() throws { - let tmpDir = try makeTmpDir() + let tmpDir = try makeGlobalTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let pack = MockTechPack( @@ -1382,7 +1135,7 @@ struct GlobalStateAndIndexTests { displayName: "Test Pack" ) - let configurator = makeConfigurator(home: tmpDir) + let configurator = makeGlobalConfigurator(home: tmpDir) try configurator.configure(packs: [pack], confirmRemovals: false) let env = Environment(home: tmpDir) diff --git a/Tests/MCSTests/TestHelpers.swift b/Tests/MCSTests/TestHelpers.swift index 0e1d570..46b2dfa 100644 --- a/Tests/MCSTests/TestHelpers.swift +++ b/Tests/MCSTests/TestHelpers.swift @@ -121,3 +121,36 @@ final class TrackingMockTechPack: TechPack, @unchecked Sendable { configureProjectCallCount += 1 } } + +// MARK: - Global Test Helpers + +/// Create a temp directory pre-configured for global-scope tests (`.claude/` + `.mcs/` subdirectories). +func makeGlobalTmpDir(label: String = "global") throws -> URL { + let dir = FileManager.default.temporaryDirectory + .appendingPathComponent("mcs-\(label)-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(Constants.FileNames.claudeDirectory), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: dir.appendingPathComponent(".mcs"), + withIntermediateDirectories: true + ) + return dir +} + +/// Create a `Configurator` configured for global-scope sync. +func makeGlobalConfigurator( + home: URL, + mockCLI: MockClaudeCLI = MockClaudeCLI() +) -> Configurator { + let env = Environment(home: home) + return Configurator( + environment: env, + output: CLIOutput(colorsEnabled: false), + shell: ShellRunner(environment: env), + strategy: GlobalSyncStrategy(environment: env), + claudeCLI: mockCLI + ) +} From 536a43db1912b16908ec6bee8e81b0bcf7e63acf Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Fri, 6 Mar 2026 09:12:33 +0100 Subject: [PATCH 3/4] Fix consecutive blank lines lint error in ProjectConfiguratorTests --- Tests/MCSTests/ProjectConfiguratorTests.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/MCSTests/ProjectConfiguratorTests.swift b/Tests/MCSTests/ProjectConfiguratorTests.swift index 02b412f..ebb0c7d 100644 --- a/Tests/MCSTests/ProjectConfiguratorTests.swift +++ b/Tests/MCSTests/ProjectConfiguratorTests.swift @@ -1658,7 +1658,6 @@ struct CorruptStateAbortTests { } } - // MARK: - parseRepoName Tests struct ParseRepoNameTests { From 91180065addaf9650449e94145c39ac6b6ef40b9 Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Fri, 6 Mar 2026 09:14:04 +0100 Subject: [PATCH 4/4] Add lint-before-commit rule to CLAUDE.md - Require running swiftformat and swiftlint on changed files before committing --- CLAUDE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CLAUDE.md b/CLAUDE.md index 5a6a6fc..72a7456 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -158,6 +158,7 @@ swiftlint --fix - **Never amend commits** — always create new commits so the change history stays trackable - **Never force-push** — use regular `git push` only +- **Always run `swiftformat` and `swiftlint` on changed files before committing** — CI will reject PRs that fail lint. Run `swiftformat ` then `swiftlint` to catch issues locally ## Key Design Decisions