From 7b04aaf41f2bb643bf2e0e8799eb6b3363e445ab Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Wed, 2 Sep 2026 15:25:50 +0200 Subject: [PATCH 1/3] ISSUE-371: Flag packs configured in both global and project scope - Add a doctor check naming duplicated skills, hooks and CLAUDE.md sections, with --fix removing the project copy only when provably lossless - Share the file-drift and template-dependency rules so doctor and sync cannot disagree about what is installed - Add per-project pack removal to ProjectIndex, so removing one pack no longer stamps a sync that never happened Claude-Session: https://claude.ai/code/session_01MkBJdGBUoZ2aRtchjaKXMN --- Sources/mcs/Core/FileHasher.swift | 32 ++ Sources/mcs/Core/ProjectIndex.swift | 12 + Sources/mcs/Doctor/CoreDoctorChecks.swift | 34 +- Sources/mcs/Doctor/DoctorRunner.swift | 17 +- .../mcs/Doctor/ScopeDuplicationCheck.swift | 404 +++++++++++++++++ Sources/mcs/Sync/Configurator.swift | 6 +- Sources/mcs/TechPack/TechPack.swift | 13 + .../MCSTests/LifecycleIntegrationTests.swift | 413 ++++++++++++++++++ Tests/MCSTests/ProjectIndexTests.swift | 53 +++ 9 files changed, 962 insertions(+), 22 deletions(-) create mode 100644 Sources/mcs/Doctor/ScopeDuplicationCheck.swift diff --git a/Sources/mcs/Core/FileHasher.swift b/Sources/mcs/Core/FileHasher.swift index 2b468ed4..f6671f0e 100644 --- a/Sources/mcs/Core/FileHasher.swift +++ b/Sources/mcs/Core/FileHasher.swift @@ -16,6 +16,38 @@ enum FileHasher { SHA256.hash(data: data).map { String(format: "%02x", $0) }.joined() } + /// How a file on disk compares to a hash recorded when it was installed. + /// + /// Callers map these onto their own vocabulary — `FileContentCheck` reports them, and + /// `ScopeDuplicationCheck` refuses to delete anything that is not `.matches` — but the + /// policy for *what counts as drift* lives here so the two cannot disagree. A disagreement + /// would mean deleting a file doctor elsewhere reports as user-modified. + enum DriftState { + case matches + case missing + case directory + case changed + case unreadable(any Error) + } + + /// Compare a file against its recorded hash. + /// + /// A missing file has nothing to compare and a directory is covered by the entries for the + /// files inside it, so neither is drift. An unreadable file is reported rather than assumed + /// intact — the caller decides how cautious to be. + static func drift(of url: URL, expecting expectedHash: String) -> DriftState { + var isDirectory: ObjCBool = false + guard FileManager.default.fileExists(atPath: url.path, isDirectory: &isDirectory) else { + return .missing + } + if isDirectory.boolValue { return .directory } + do { + return try sha256(of: url) == expectedHash ? .matches : .changed + } catch { + return .unreadable(error) + } + } + /// Result of hashing all files in a directory, with per-file error resilience. struct DirectoryHashResult { let hashes: [(relativePath: String, hash: String)] diff --git a/Sources/mcs/Core/ProjectIndex.swift b/Sources/mcs/Core/ProjectIndex.swift index 27f65399..f0e9981a 100644 --- a/Sources/mcs/Core/ProjectIndex.swift +++ b/Sources/mcs/Core/ProjectIndex.swift @@ -68,6 +68,18 @@ struct ProjectIndex { data.projects.removeAll { $0.path == projectPath } } + /// Remove a specific pack from one project entry, pruning the entry if nothing remains. + /// + /// Deliberately not expressed as `upsert` with the surviving packs: `upsert` stamps + /// `lastSynced` with the current time, which would claim a sync that never happened. + func removePack(_ packID: String, fromProject path: String, in data: inout IndexData) { + guard let index = data.projects.firstIndex(where: { $0.path == path }) else { return } + data.projects[index].packs.removeAll { $0 == packID } + if data.projects[index].packs.isEmpty { + data.projects.remove(at: index) + } + } + /// Remove a specific pack from all project entries. Prunes entries with no remaining packs. func removePack(_ packID: String, from data: inout IndexData) { for i in data.projects.indices { diff --git a/Sources/mcs/Doctor/CoreDoctorChecks.swift b/Sources/mcs/Doctor/CoreDoctorChecks.swift index 5b7078a0..b35c6496 100644 --- a/Sources/mcs/Doctor/CoreDoctorChecks.swift +++ b/Sources/mcs/Doctor/CoreDoctorChecks.swift @@ -9,12 +9,18 @@ import Foundation // - **Cleanup**: Removing deprecated components (MCP servers, plugins) // - **Migration**: One-time data moves (state files) // - **Trivial repairs**: Permission fixes (chmod), gitignore additions (idempotent) +// - **Scope reconciliation**: Removing a pack from one scope when a provably equivalent copy +// exists in another, by calling `Configurator.unconfigurePack` rather than re-implementing +// removal. This is the one category that drives the sync engine, so it carries a higher bar: +// the check must refuse the fix unless it can prove nothing is lost — see +// `ScopeDuplicationCheck`, which gates on component subset, prompt-answer parity, and the +// recorded hash of every file it would delete. Do not copy the pattern without the gates. // // `doctor --fix` does NOT handle: // - **Additive operations**: Installing packages, registering servers, copying hooks/skills/commands. // These are `mcs sync`'s responsibility. // -// This separation keeps `doctor --fix` predictable and non-destructive. +// This separation keeps `doctor --fix` predictable, and destructive only where it can show its work. struct CommandCheck: DoctorCheck { let name: String @@ -196,21 +202,17 @@ struct FileContentCheck: DoctorCheck { let expectedHash: String func check() -> CheckResult { - var isDir: ObjCBool = false - guard FileManager.default.fileExists(atPath: path.path, isDirectory: &isDir) else { - return .skip("missing (checked separately)") - } - if isDir.boolValue { - return .skip("directory (contents checked individually)") - } - do { - let currentHash = try FileHasher.sha256(of: path) - if currentHash == expectedHash { - return .pass("content matches") - } - return .warn("modified since last sync") - } catch { - return .fail("could not read file: \(error.localizedDescription)") + switch FileHasher.drift(of: path, expecting: expectedHash) { + case .matches: + .pass("content matches") + case .missing: + .skip("missing (checked separately)") + case .directory: + .skip("directory (contents checked individually)") + case .changed: + .warn("modified since last sync") + case let .unreadable(error): + .fail("could not read file: \(error.localizedDescription)") } } diff --git a/Sources/mcs/Doctor/DoctorRunner.swift b/Sources/mcs/Doctor/DoctorRunner.swift index fabfca84..b8a2135b 100644 --- a/Sources/mcs/Doctor/DoctorRunner.swift +++ b/Sources/mcs/Doctor/DoctorRunner.swift @@ -18,6 +18,12 @@ struct DoctorRunner { let skipConfirmation: Bool /// Explicit pack filter. If nil, uses packs from project state or pack registry. let packFilter: String? + + /// `packFilter` split into identifiers, so the comma convention is defined in one place. + private var packFilterIDs: Set? { + packFilter.map { Set($0.components(separatedBy: ",")) } + } + /// When true, check only globally-configured packs (ignores project scope). let globalOnly: Bool let registry: TechPackRegistry @@ -222,6 +228,14 @@ struct DoctorRunner { let context = ProjectDoctorContext(projectRoot: root, registry: registry) nonComponentChecks += ProjectDoctorChecks.checks(context: context) } + // Packs configured in this project *and* globally. Self-skips when no global + // scope exists, so it costs nothing on machines that never ran `--global`. + nonComponentChecks += ScopeDuplicationCheck.checks( + projectRoot: root, + registry: registry, + environment: env, + packFilter: packFilterIDs + ) } // Global-scoped template freshness check (always runs, self-skips if no global CLAUDE.md) @@ -297,8 +311,7 @@ struct DoctorRunner { globalExcludedComponentIDs: Set ) -> [CheckScope] { // --pack flag: single scope, use globalOnly to determine effective root - if let filter = packFilter { - let packIDs = Set(filter.components(separatedBy: ",")) + if let packIDs = packFilterIDs { let effectiveRoot = globalOnly ? nil : projectRoot // Load artifacts and excluded components from the appropriate state var artifacts: [String: PackArtifactRecord] = [:] diff --git a/Sources/mcs/Doctor/ScopeDuplicationCheck.swift b/Sources/mcs/Doctor/ScopeDuplicationCheck.swift new file mode 100644 index 00000000..31f9eae5 --- /dev/null +++ b/Sources/mcs/Doctor/ScopeDuplicationCheck.swift @@ -0,0 +1,404 @@ +import Foundation + +/// Reports a pack configured in *both* the global scope and the current project, where its +/// artifacts are installed twice and take effect twice. +/// +/// `mcs sync` blocks a globally-installed pack from being *added* to a project +/// (`ConfiguratorSupport.globallyBlockedIDs`), but deliberately leaves a pack already present in +/// both scopes selectable — blocking it would drop it from the desired pack set, which +/// `mcs sync --all` converges on with no prompt, silently unconfiguring it. That transition-only +/// rule means pre-existing duplicates persist, and this check is what finds them. +/// +/// `fix()` removes the project-scoped copy and keeps the global one, but only once it can prove +/// the removal is lossless — see the obstacle gates below. +struct ScopeDuplicationCheck: DoctorCheck { + let packID: String + let projectRoot: URL + let registry: TechPackRegistry + let environment: Environment + + /// Computed once by `checks(...)` and reused by `check()` and `fixCommandPreview`, which the + /// runner reads four times in total within one pass. Diagnosing is not free — two state + /// decodes, a disk read per template, and a hash of every installed file — and nothing mutates + /// project state between those reads. `fix()` re-derives instead, because the user is prompted + /// in between and the filesystem may have moved on. + private let diagnosis: Diagnosis + + var name: String { + "Scope duplication: \(packID)" + } + + var section: String { + "Project" + } + + /// Non-nil only when the removal is provably lossless. A nil preview makes `DoctorRunner` + /// treat the check as unfixable and surface `fix()`'s `.notFixable` reason as a hint instead. + var fixCommandPreview: String? { + guard case .duplicated(_, nil) = diagnosis else { return nil } + return "remove the project-scoped copy of '\(packID)' (keeps global)" + } + + // MARK: - Check + + func check() -> CheckResult { + switch diagnosis { + case let .resolved(result): + result + case let .duplicated(summary, blocked): + .fail( + "also installed globally — \(summary) duplicated; " + + (blocked ?? "run 'mcs doctor --fix' to remove the project copy") + ) + } + } + + // MARK: - Fix + + func fix() -> FixResult { + // Re-derived rather than reusing the stored diagnosis: the user was prompted in between, + // and an edit made since then must still block the removal. + let inputs: Inputs? + do { + inputs = try Self.makeInputs( + packID: packID, projectRoot: projectRoot, + registry: registry, environment: environment + ) + } catch { + return .failed("could not read state: \(error.localizedDescription)") + } + guard let inputs, case let .duplicated(_, blocked) = Self.diagnose(inputs) else { + return .notFixable("no longer duplicated — re-run 'mcs doctor'") + } + if let blocked { + return .notFixable(blocked) + } + + let output = CLIOutput() + let shell = ShellRunner(environment: environment) + var state = inputs.projectState + let sharedGitignoreEntries = inputs.globalState.artifacts(for: packID)?.gitignoreEntries ?? [] + + let configurator = Configurator( + environment: environment, + output: output, + shell: shell, + registry: registry, + strategy: ProjectSyncStrategy(projectPath: projectRoot, environment: environment) + ) + // Default `refCountScope` (nil → this project's path): the global scope still counts, so + // brew packages and plugins report `.stillNeeded` and stay installed. Passing + // `packRemoveSentinel` here would exclude every scope and uninstall them. + configurator.unconfigurePack(packID, state: &state) + + do { + try state.save() + } catch { + return .failed("could not write .mcs-project: \(error.localizedDescription)") + } + + // `unconfigurePack` keeps the pack in state whenever cleanup was partial (the shrinking-set + // pattern) or the settings file could not be parsed. Leave the project index alone in that + // case so reference counting stays conservative. + guard !state.configuredPacks.contains(packID) else { + return .failed("some artifacts could not be removed — re-run 'mcs sync' to retry") + } + + restoreSharedGitignoreEntries(sharedGitignoreEntries, shell: shell, output: output) + pruneProjectIndex(output: output) + + return .fixed("removed project-scoped '\(packID)' (global copy kept)") + } + + /// Re-add gitignore lines the global copy still claims. + /// + /// `GitignoreManager` writes one file for the whole machine, so a gitignore entry is a shared + /// resource exactly like a brew package or a plugin — but it is the one such resource + /// `ResourceRefCounter.Resource` omits, so `unconfigurePack` deletes it on behalf of any single + /// scope. `addEntry` is idempotent, so re-adding is safe. + /// + /// This only repairs the doctor path. The same unguarded delete is a live bug in `mcs sync` + /// deselection, `mcs pack remove` (where the first scope strips the lines out from under the + /// scopes that follow), and stale-artifact reconciliation. Fixing it properly means adding a + /// third `ResourceRefCounter.Resource` case so `Configurator.removeGitignoreArtifact` becomes + /// ref-counted like its brew and plugin siblings — tracked as a follow-up. + private func restoreSharedGitignoreEntries( + _ entries: [String], + shell: any ShellRunning, + output: CLIOutput + ) { + guard !entries.isEmpty else { return } + let manager = GitignoreManager(shell: shell) + for entry in entries { + do { + try manager.addEntry(entry) + } catch { + output.warn("Could not restore gitignore entry '\(entry)': \(error.localizedDescription)") + } + } + } + + /// Drop this pack from this project's entry in `~/.mcs/projects.yaml`. + /// Failure warns rather than fails, mirroring `Configurator.saveStateAndUpdateIndex`. + private func pruneProjectIndex(output: CLIOutput) { + let indexFile = ProjectIndex(path: environment.projectsIndexFile) + do { + var data = try indexFile.load() + indexFile.removePack(packID, fromProject: projectRoot.path, in: &data) + try indexFile.save(data) + } catch { + output.warn("Could not update project index: \(error.localizedDescription)") + output.warn("Cross-project resource tracking may be inaccurate. Re-run 'mcs sync' to retry.") + } + } + + // MARK: - Diagnosis + + private enum Diagnosis { + /// Nothing to report; the reason is already a final result. + case resolved(CheckResult) + /// `blocked` is nil when the project copy can be removed without losing anything. + case duplicated(summary: String, blocked: String?) + } + + /// Everything a diagnosis reads, gathered once. + private struct Inputs { + let pack: any TechPack + let projectRoot: URL + let environment: Environment + let projectState: ProjectState + let globalState: ProjectState + + var packID: String { + pack.identifier + } + } + + /// Load both scopes' state. Returns nil when the pack is unknown to the registry or no longer + /// configured in both scopes — either way there is nothing to diagnose. + private static func makeInputs( + packID: String, + projectRoot: URL, + registry: TechPackRegistry, + environment: Environment + ) throws -> Inputs? { + guard let pack = registry.pack(for: packID) else { return nil } + let projectState = try ProjectState(projectRoot: projectRoot) + let globalState = try ProjectState(stateFile: environment.globalStateFile) + guard projectState.configuredPacks.contains(packID), + globalState.configuredPacks.contains(packID) + else { + return nil + } + return Inputs( + pack: pack, projectRoot: projectRoot, environment: environment, + projectState: projectState, globalState: globalState + ) + } + + private static func diagnose(_ inputs: Inputs) -> Diagnosis { + let pack = inputs.pack + let projectExcluded = inputs.projectState.excludedComponents(for: inputs.packID) + let globalExcluded = inputs.globalState.excludedComponents(for: inputs.packID) + + // One partition, read from both sides: components shared with the global scope are + // candidates for duplication, and the rest are what removing the project copy would lose. + let globalIDs = Set(pack.components.filter { !globalExcluded.contains($0.id) }.map(\.id)) + let projectComponents = pack.components.filter { !projectExcluded.contains($0.id) } + let duplicatedComponents = projectComponents.filter { + globalIDs.contains($0.id) && duplicatesAcrossScopes($0.installAction) + } + let projectOnly = projectComponents.filter { !globalIDs.contains($0.id) } + + let duplicatedSectionCount: Int + do { + let templates = try pack.templates + duplicatedSectionCount = sectionIdentifiers(templates, excluding: projectExcluded) + .intersection(sectionIdentifiers(templates, excluding: globalExcluded)) + .count + } catch { + return .resolved(.warn( + "could not load templates for '\(inputs.packID)': \(error.localizedDescription)" + )) + } + + guard !duplicatedComponents.isEmpty || duplicatedSectionCount > 0 else { + return .resolved(.pass("configured in both scopes, but no artifacts overlap")) + } + + // First obstacle wins — each gate names something the user must resolve before the + // project copy can be removed without losing anything. + let blocked = divergentComponentsObstacle(projectOnly) + ?? divergentPromptObstacle(inputs) + ?? editedFileObstacle(inputs) + + return .duplicated( + summary: summarize(components: duplicatedComponents, sectionCount: duplicatedSectionCount), + blocked: blocked + ) + } + + /// Whether installing this action in both scopes leaves two copies that both take effect. + /// + /// Only `copyPackFile` does. Its files land in `/.claude/` and `~/.claude/` + /// independently, and a hook component's settings entry is written with a different command + /// prefix per scope (`Constants.HookCommand`), so the two entries are distinct strings that + /// both fire. Everything else either shadows (project `settings.local.json` over global + /// `settings.json`, MCP `local` over `user`), is installed once (the project scope skips brew + /// packages and plugins entirely), writes one idempotent line (gitignore), or is a one-shot + /// side effect rather than a standing artifact (`shellCommand`). + /// + /// Deliberately exhaustive: a new install action should not silently default to "harmless". + private static func duplicatesAcrossScopes(_ action: ComponentInstallAction) -> Bool { + switch action { + case .copyPackFile: + true + case .mcpServer, .plugin, .brewInstall, .shellCommand, .settingsMerge, .gitignoreEntries: + false + } + } + + private static func sectionIdentifiers( + _ templates: [TemplateContribution], + excluding excluded: Set + ) -> Set { + Set(templates.excludingDependencies(on: excluded).map(\.sectionIdentifier)) + } + + private static func summarize(components: [ComponentDefinition], sectionCount: Int) -> String { + let counts = components.reduce(into: [ComponentType: Int]()) { $0[$1.type, default: 0] += 1 } + var parts = ComponentType.allCases.compactMap { type in + counts[type].map { counted($0, type.rawValue.lowercased()) } + } + if sectionCount > 0 { + parts.append(counted(sectionCount, "CLAUDE.md sections")) + } + return parts.joined(separator: ", ") + } + + /// Only file-copy component types reach this (skills, hooks, commands, agents, + /// configurations), so trimming a trailing "s" is a safe singular. + private static func counted(_ count: Int, _ plural: String) -> String { + count == 1 && plural.hasSuffix("s") ? "\(count) \(plural.dropLast())" : "\(count) \(plural)" + } + + // MARK: - Obstacles to a lossless removal + + /// The global scope must install everything the project scope does. Divergent `--customize` + /// choices mean removal would delete a component nothing else provides. + private static func divergentComponentsObstacle(_ projectOnly: [ComponentDefinition]) -> String? { + guard !projectOnly.isEmpty else { return nil } + let names = projectOnly.map(\.displayName).sorted().joined(separator: ", ") + return "the project scope installs \(names), which the global scope excludes — " + + "remove it with 'mcs sync' instead" + } + + /// Both scopes must have answered the pack's prompts identically, or removal would silently + /// switch this project onto the global answer. The project's key set is used deliberately — a + /// `fileDetect` prompt is dropped in global scope, so the global value is absent and correctly + /// reads as divergent. + private static func divergentPromptObstacle(_ inputs: Inputs) -> String? { + let projectValues = inputs.projectState.resolvedValues ?? [:] + let globalValues = inputs.globalState.resolvedValues ?? [:] + let context = ProjectSyncStrategy( + projectPath: inputs.projectRoot, environment: inputs.environment + ).makeConfigContext(output: CLIOutput(), resolvedValues: projectValues, priorValues: [:]) + + let divergent = inputs.pack.declaredPrompts(context: context) + .map(\.key) + .filter { projectValues[$0] != globalValues[$0] } + .sorted() + guard !divergent.isEmpty else { return nil } + return "the two scopes answered \(divergent.joined(separator: ", ")) differently — " + + "removing the project copy would switch it to the global answer" + } + + /// Refuse to delete files the user has edited. `unconfigurePack` removes tracked files without + /// consulting the recorded hash, so drift here is data loss. An unreadable file cannot be + /// proven untouched, so it counts as at risk rather than being deleted. + private static func editedFileObstacle(_ inputs: Inputs) -> String? { + guard let record = inputs.projectState.artifacts(for: inputs.packID) else { return nil } + + let atRisk = record.fileHashes.compactMap { relativePath, expectedHash -> String? in + let fileURL = inputs.projectRoot.appendingPathComponent(relativePath) + switch FileHasher.drift(of: fileURL, expecting: expectedHash) { + case .matches, .missing, .directory: + return nil + case .changed: + return relativePath + case let .unreadable(error): + return "\(relativePath) (unreadable: \(error.localizedDescription))" + } + } + + guard !atRisk.isEmpty else { return nil } + return "\(atRisk.sorted().joined(separator: ", ")) changed since install — " + + "removing the project copy would discard those edits" + } + + // MARK: - Factory + + /// One check per pack configured in *both* the project and the global scope. + /// + /// Reads global state directly rather than reusing `DoctorRunner`'s `globallyConfiguredPackIDs`, + /// which falls back to the pack *registry* when the global state file is absent — reusing it + /// would report every project pack as a duplicate on a machine that never ran + /// `mcs sync --global`. + static func checks( + projectRoot: URL, + registry: TechPackRegistry, + environment: Environment, + packFilter: Set? + ) -> [any DoctorCheck] { + let globalPacks: Set + let projectPacks: Set + do { + let globalState = try ProjectState(stateFile: environment.globalStateFile) + // No global scope has ever been synced, so nothing can be duplicated. + guard globalState.exists else { return [] } + globalPacks = globalState.configuredPacks + projectPacks = try ProjectState(projectRoot: projectRoot).configuredPacks + } catch { + // Corrupt state is already reported by `ProjectStateFileCheck` with a specific fix. + // A second, vaguer complaint about the same file would only add noise. + return [] + } + + var duplicated = globalPacks.intersection(projectPacks) + if let packFilter { + duplicated.formIntersection(packFilter) + } + + return duplicated.sorted().map { packID in + ScopeDuplicationCheck( + packID: packID, projectRoot: projectRoot, + registry: registry, environment: environment, + diagnosis: initialDiagnosis( + packID: packID, projectRoot: projectRoot, + registry: registry, environment: environment + ) + ) + } + } + + private static func initialDiagnosis( + packID: String, + projectRoot: URL, + registry: TechPackRegistry, + environment: Environment + ) -> Diagnosis { + do { + // Both scopes were just confirmed to list this pack, so a nil here means the registry + // does not know it — a pack synced from a source that has since been removed. + guard let inputs = try makeInputs( + packID: packID, projectRoot: projectRoot, + registry: registry, environment: environment + ) else { + return .resolved(.skip("pack '\(packID)' is not registered")) + } + return diagnose(inputs) + } catch { + return .resolved(.warn("could not read pack state: \(error.localizedDescription)")) + } + } +} diff --git a/Sources/mcs/Sync/Configurator.swift b/Sources/mcs/Sync/Configurator.swift index a2494139..e6a9038b 100644 --- a/Sources/mcs/Sync/Configurator.swift +++ b/Sources/mcs/Sync/Configurator.swift @@ -804,10 +804,8 @@ struct Configurator { for pack in packs { do { let excluded = excludedComponents[pack.identifier] ?? [] - let allTemplates = try pack.templates - preloadedTemplates[pack.identifier] = allTemplates.filter { template in - !template.dependencies.contains(where: excluded.contains) - } + preloadedTemplates[pack.identifier] = try pack.templates + .excludingDependencies(on: excluded) } catch { output.warn("Could not load templates for \(pack.displayName): \(error.localizedDescription)") } diff --git a/Sources/mcs/TechPack/TechPack.swift b/Sources/mcs/TechPack/TechPack.swift index 6472ca50..1418cf97 100644 --- a/Sources/mcs/TechPack/TechPack.swift +++ b/Sources/mcs/TechPack/TechPack.swift @@ -50,6 +50,19 @@ struct TemplateContribution { } } +extension Collection { + /// The templates a scope actually composes, given the components it excluded. + /// + /// A template whose `dependencies` name an excluded component is dropped, so excluding a + /// component also removes any section that only makes sense alongside it. Sync applies this + /// when preloading templates and doctor applies it when deciding whether a section exists in + /// both scopes; if the two ever disagreed, doctor would report a duplicated CLAUDE.md section + /// that sync never composed. + func excludingDependencies(on excluded: Set) -> [TemplateContribution] { + filter { !$0.dependencies.contains(where: excluded.contains) } + } +} + /// Protocol that all tech packs must conform to. /// Packs are applied to projects via `mcs sync`. /// Doctor and configure only run pack-specific logic for installed packs. diff --git a/Tests/MCSTests/LifecycleIntegrationTests.swift b/Tests/MCSTests/LifecycleIntegrationTests.swift index c3474ad5..35692813 100644 --- a/Tests/MCSTests/LifecycleIntegrationTests.swift +++ b/Tests/MCSTests/LifecycleIntegrationTests.swift @@ -2112,3 +2112,416 @@ struct GlobalPackBlockingLifecycleTests { } } } + +// MARK: - Scope Duplication (Issue #371) + +/// A pack configured in both the global scope and a project installs its artifacts twice. +/// `mcs sync` blocks the *transition* that creates a duplicate but deliberately leaves an existing +/// one in place, so these tests cover the only thing that finds it afterwards. +/// +/// The check reads two real `ProjectState` files with populated artifact records and file hashes, +/// so every fixture is built with the real configurators rather than hand-planted JSON. +@Suite("Scope duplication check") +struct ScopeDuplicationCheckTests { + /// Configure `pack` in the project first, then globally — `filterGloballyBlocked` rejects the + /// reverse order, so this is the only sequence that produces a both-scope pack. + private func configureBothScopes( + bed: LifecycleTestBed, + pack: any TechPack, + registry: TechPackRegistry, + projectExclusions: [String: Set] = [:], + globalExclusions: [String: Set] = [:] + ) throws { + try bed.makeConfigurator(registry: registry).configure( + packs: [pack], confirmRemovals: false, excludedComponents: projectExclusions + ) + try bed.makeGlobalSyncConfigurator(registry: registry).configure( + packs: [pack], confirmRemovals: false, excludedComponents: globalExclusions + ) + } + + private func checks( + bed: LifecycleTestBed, + registry: TechPackRegistry, + packFilter: String? = nil + ) -> [any DoctorCheck] { + ScopeDuplicationCheck.checks( + projectRoot: bed.project, + registry: registry, + environment: bed.env, + packFilter: packFilter.map { Set($0.components(separatedBy: ",")) } + ) + } + + /// A pack with one skill, one hook and one template — the three surfaces that duplicate. + private func duplicatingPack(bed: LifecycleTestBed) throws -> any TechPack { + try MockTechPack( + identifier: "dup-pack", + displayName: "Dup Pack", + components: [ + bed.skillComponent( + pack: "dup-pack", id: "skillA", + source: bed.makeSkillSource(name: "dup-skill.md"), + destination: "dup-skill.md" + ), + bed.hookComponent( + pack: "dup-pack", id: "hookA", + source: bed.makeHookSource(name: "dup-hook.sh"), + destination: "dup-hook.sh", + hookRegistration: HookRegistration(event: .preToolUse) + ), + ], + templates: [ + TemplateContribution( + sectionIdentifier: "dup-pack", + templateContent: "Dup pack guidance.", + placeholders: [] + ), + ] + ) + } + + // MARK: Detection + + @Test("Names every duplicated surface when a pack is configured in both scopes") + func reportsDuplicatedSurfaces() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + let pack = try duplicatingPack(bed: bed) + let registry = TechPackRegistry(packs: [pack]) + try configureBothScopes(bed: bed, pack: pack, registry: registry) + + let emitted = checks(bed: bed, registry: registry) + let check = try #require(emitted.first) + #expect(emitted.count == 1) + let result = check.check() + guard case let .fail(message) = result else { + Issue.record("Expected .fail, got \(result)") + return + } + #expect(message.contains("also installed globally")) + #expect(message.contains("1 skill")) + #expect(message.contains("1 hook")) + #expect(message.contains("1 CLAUDE.md section")) + #expect(check.name == "Scope duplication: dup-pack") + #expect(check.section == "Project") + } + + @Test("Passes when the pack is in both scopes but nothing actually duplicates") + func passesWhenNoArtifactsOverlap() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + // An MCP server registers `local` in the project and `user` globally — the local one + // shadows rather than duplicating — and the pack ships no templates or files. + let pack = MockTechPack( + identifier: "mcp-only", + displayName: "MCP Only", + components: [bed.mcpComponent(pack: "mcp-only", id: "srv", name: "srv")] + ) + let registry = TechPackRegistry(packs: [pack]) + try configureBothScopes(bed: bed, pack: pack, registry: registry) + + let emitted = checks(bed: bed, registry: registry) + let check = try #require(emitted.first) + #expect(emitted.count == 1) + let result = check.check() + guard case let .pass(message) = result else { + Issue.record("Expected .pass, got \(result)") + return + } + #expect(message.contains("no artifacts overlap")) + } + + @Test("A template excluded in one scope is not counted as duplicated") + func templateFilteredByDependencyIsNotDuplicated() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + // The template depends on the hook, so excluding the hook globally drops the section + // there too — `Configurator.preloadTemplates` filters it before composition. + let pack = try MockTechPack( + identifier: "tmpl-pack", + displayName: "Template Pack", + components: [ + bed.hookComponent( + pack: "tmpl-pack", id: "hookA", + source: bed.makeHookSource(name: "tmpl-hook.sh"), + destination: "tmpl-hook.sh", + isRequired: false + ), + ], + templates: [ + TemplateContribution( + sectionIdentifier: "tmpl-pack", + templateContent: "Guidance.", + placeholders: [], + dependencies: ["tmpl-pack.hookA"] + ), + ] + ) + let registry = TechPackRegistry(packs: [pack]) + try configureBothScopes( + bed: bed, pack: pack, registry: registry, + globalExclusions: ["tmpl-pack": Set(["tmpl-pack.hookA"])] + ) + + let emitted = checks(bed: bed, registry: registry) + let check = try #require(emitted.first) + #expect(emitted.count == 1) + let result = check.check() + guard case .pass = result else { + Issue.record("Expected .pass — nothing survives in both scopes, got \(result)") + return + } + } + + // MARK: Factory scoping + + @Test("Emits nothing when no global scope has ever been synced") + func emitsNothingWithoutGlobalState() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + let pack = try duplicatingPack(bed: bed) + let registry = TechPackRegistry(packs: [pack]) + try bed.makeConfigurator(registry: registry) + .configure(packs: [pack], confirmRemovals: false) + + // The regression guard: falling back to the pack registry here — as DoctorRunner does + // for check scoping — would report every project pack as a duplicate. + #expect(checks(bed: bed, registry: registry).isEmpty) + } + + @Test("Emits nothing for a pack configured in only one scope") + func emitsNothingForSingleScopePack() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + let pack = try duplicatingPack(bed: bed) + let registry = TechPackRegistry(packs: [pack]) + try bed.makeGlobalSyncConfigurator(registry: registry) + .configure(packs: [pack], confirmRemovals: false) + + #expect(checks(bed: bed, registry: registry).isEmpty) + } + + @Test("Honours the --pack filter") + func respectsPackFilter() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + let pack = try duplicatingPack(bed: bed) + let registry = TechPackRegistry(packs: [pack]) + try configureBothScopes(bed: bed, pack: pack, registry: registry) + + #expect(checks(bed: bed, registry: registry, packFilter: "other-pack").isEmpty) + #expect(checks(bed: bed, registry: registry, packFilter: "dup-pack").count == 1) + } + + // MARK: Fixability gates + + @Test("Refuses to fix when the project installs a component the global scope excludes") + func blocksFixOnDivergentExclusions() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + let pack = try duplicatingPack(bed: bed) + let registry = TechPackRegistry(packs: [pack]) + try configureBothScopes( + bed: bed, pack: pack, registry: registry, + globalExclusions: ["dup-pack": Set(["dup-pack.hookA"])] + ) + + let check = try #require(checks(bed: bed, registry: registry).first) + #expect(check.fixCommandPreview == nil) + let result = check.fix() + guard case let .notFixable(reason) = result else { + Issue.record("Expected .notFixable, got \(result)") + return + } + #expect(reason.contains("hookA")) + #expect(try bed.projectState().configuredPacks.contains("dup-pack")) + } + + @Test("Refuses to fix when the two scopes answered a prompt differently") + func blocksFixOnDivergentPromptAnswers() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + let pack = try MockPromptTechPack( + identifier: "prompt-pack", + displayName: "Prompt Pack", + prompts: [PromptDefinition( + key: "__TOKEN__", type: .input, + label: nil, defaultValue: "shared", options: nil, + detectPatterns: nil, scriptCommand: nil + )], + components: [ + bed.skillComponent( + pack: "prompt-pack", id: "skillA", + source: bed.makeSkillSource(name: "prompt-skill.md"), + destination: "prompt-skill.md" + ), + ] + ) + let registry = TechPackRegistry(packs: [pack]) + try configureBothScopes(bed: bed, pack: pack, registry: registry) + + // Simulate the global scope having been answered differently. + var globalState = try ProjectState(stateFile: bed.env.globalStateFile) + globalState.setResolvedValues(["__TOKEN__": "a-different-answer"]) + try globalState.save() + + let check = try #require(checks(bed: bed, registry: registry).first) + let result = check.fix() + guard case let .notFixable(reason) = result else { + Issue.record("Expected .notFixable, got \(result)") + return + } + #expect(reason.contains("__TOKEN__")) + } + + /// The guard for issue #365: `unconfigurePack` deletes tracked files without consulting the + /// recorded hash, so an edited file would be silently destroyed. Drift must block the fix. + @Test("Refuses to fix when an installed file was edited, and leaves it untouched") + func blocksFixOnEditedFile() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + let pack = try duplicatingPack(bed: bed) + let registry = TechPackRegistry(packs: [pack]) + try configureBothScopes(bed: bed, pack: pack, registry: registry) + + let installedSkill = bed.project.appendingPathComponent(".claude/skills/dup-skill.md") + try "# Skill\nMy own edits.".write(to: installedSkill, atomically: true, encoding: .utf8) + + let check = try #require(checks(bed: bed, registry: registry).first) + #expect(check.fixCommandPreview == nil) + let result = check.fix() + guard case let .notFixable(reason) = result else { + Issue.record("Expected .notFixable, got \(result)") + return + } + #expect(reason.contains("dup-skill.md")) + #expect(reason.contains("changed since install")) + + let survived = try String(contentsOf: installedSkill, encoding: .utf8) + #expect(survived.contains("My own edits.")) + #expect(try bed.projectState().configuredPacks.contains("dup-pack")) + } + + // MARK: Fix + + @Test("Fix removes the project copy, keeps the global one, and prunes the project index") + func fixRemovesProjectCopyOnly() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + let pack = try duplicatingPack(bed: bed) + let registry = TechPackRegistry(packs: [pack]) + try configureBothScopes(bed: bed, pack: pack, registry: registry) + + let projectSkill = bed.project.appendingPathComponent(".claude/skills/dup-skill.md") + let globalSkill = bed.home.appendingPathComponent(".claude/skills/dup-skill.md") + #expect(FileManager.default.fileExists(atPath: projectSkill.path)) + #expect(FileManager.default.fileExists(atPath: globalSkill.path)) + + let check = try #require(checks(bed: bed, registry: registry).first) + #expect(check.fixCommandPreview != nil) + let result = check.fix() + guard case let .fixed(message) = result else { + Issue.record("Expected .fixed, got \(result)") + return + } + #expect(message.contains("global copy kept")) + + // Project copy gone, global copy intact. + #expect(!FileManager.default.fileExists(atPath: projectSkill.path)) + #expect(FileManager.default.fileExists(atPath: globalSkill.path)) + #expect(try !(bed.projectState().configuredPacks.contains("dup-pack"))) + #expect(try ProjectState(stateFile: bed.env.globalStateFile) + .configuredPacks.contains("dup-pack")) + + // The project's CLAUDE.local.md section is gone; the global CLAUDE.md keeps its own. + let projectClaude = (try? String(contentsOf: bed.claudeLocalPath, encoding: .utf8)) ?? "" + #expect(!projectClaude.contains("Dup pack guidance.")) + + // The project index no longer credits this project with the pack. + let indexData = try ProjectIndex(path: bed.env.projectsIndexFile).load() + let projectEntry = indexData.projects.first { $0.path == bed.project.path } + #expect(projectEntry?.packs.contains("dup-pack") != true) + + // Re-running finds nothing left to report. + #expect(checks(bed: bed, registry: registry).isEmpty) + } + + /// `unconfigurePack` removes gitignore entries without reference counting, so the project + /// removal would otherwise strip a line the global copy still claims. + @Test("Fix preserves gitignore entries the global copy still claims") + func fixPreservesSharedGitignoreEntries() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + let pack = try MockTechPack( + identifier: "ignore-pack", + displayName: "Ignore Pack", + components: [ + bed.skillComponent( + pack: "ignore-pack", id: "skillA", + source: bed.makeSkillSource(name: "ignore-skill.md"), + destination: "ignore-skill.md" + ), + ComponentDefinition( + id: "ignore-pack.ignores", + displayName: "ignores", + description: "Gitignore entries", + type: .configuration, + packIdentifier: "ignore-pack", + dependencies: [], + isRequired: true, + installAction: .gitignoreEntries(entries: [".mcs-scratch"]) + ), + ] + ) + let registry = TechPackRegistry(packs: [pack]) + try configureBothScopes(bed: bed, pack: pack, registry: registry) + + let gitignore = bed.home.appendingPathComponent(".config/git/ignore") + #expect(try String(contentsOf: gitignore, encoding: .utf8).contains(".mcs-scratch")) + + let fixResult = try #require(checks(bed: bed, registry: registry).first).fix() + guard case .fixed = fixResult else { + Issue.record("Expected .fixed, got \(fixResult)") + return + } + + let after = try String(contentsOf: gitignore, encoding: .utf8) + #expect(after.contains(".mcs-scratch")) + } + + // MARK: Runner integration + + @Test("Doctor surfaces the duplication as an issue") + func doctorReportsDuplicationAsIssue() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + let pack = try duplicatingPack(bed: bed) + let registry = TechPackRegistry(packs: [pack]) + + try bed.makeConfigurator(registry: registry) + .configure(packs: [pack], confirmRemovals: false) + var baselineRunner = bed.makeDoctorRunner(registry: registry) + let baseline = try baselineRunner.run() + + try bed.makeGlobalSyncConfigurator(registry: registry) + .configure(packs: [pack], confirmRemovals: false) + var runner = bed.makeDoctorRunner(registry: registry) + let duplicated = try runner.run() + + // Deltas, not absolutes: ambient checks contribute their own results. + #expect(duplicated.issues > baseline.issues) + } +} diff --git a/Tests/MCSTests/ProjectIndexTests.swift b/Tests/MCSTests/ProjectIndexTests.swift index 85e69119..f1cf5508 100644 --- a/Tests/MCSTests/ProjectIndexTests.swift +++ b/Tests/MCSTests/ProjectIndexTests.swift @@ -106,6 +106,59 @@ struct ProjectIndexTests { #expect(data.projects[0].packs == ["swift"]) } + @Test("RemovePack from one project leaves other projects untouched") + func removePackFromSingleProject() { + let index = ProjectIndex(path: URL(fileURLWithPath: "/tmp/test.yaml")) + var data = ProjectIndex.IndexData() + index.upsert(projectPath: "/path/a", packIDs: ["ios", "swift"], in: &data) + index.upsert(projectPath: "/path/b", packIDs: ["ios"], in: &data) + + index.removePack("ios", fromProject: "/path/a", in: &data) + + #expect(data.projects.count == 2) + #expect(data.projects.first { $0.path == "/path/a" }?.packs == ["swift"]) + #expect(data.projects.first { $0.path == "/path/b" }?.packs == ["ios"]) + } + + @Test("RemovePack from one project prunes the entry when nothing remains") + func removePackPrunesEmptiedProject() { + let index = ProjectIndex(path: URL(fileURLWithPath: "/tmp/test.yaml")) + var data = ProjectIndex.IndexData() + index.upsert(projectPath: "/path/a", packIDs: ["ios"], in: &data) + + index.removePack("ios", fromProject: "/path/a", in: &data) + + #expect(data.projects.isEmpty) + } + + /// The reason this is not expressed as `upsert` with the surviving packs: `upsert` stamps + /// `lastSynced` with the current time, which would claim a sync that never happened. + @Test("RemovePack from one project preserves lastSynced") + func removePackPreservesLastSynced() { + let index = ProjectIndex(path: URL(fileURLWithPath: "/tmp/test.yaml")) + var data = ProjectIndex.IndexData() + data.projects = [ProjectIndex.ProjectEntry( + path: "/path/a", packs: ["ios", "swift"], lastSynced: "2020-01-01T00:00:00Z" + )] + + index.removePack("ios", fromProject: "/path/a", in: &data) + + #expect(data.projects[0].lastSynced == "2020-01-01T00:00:00Z") + #expect(data.projects[0].packs == ["swift"]) + } + + @Test("RemovePack from one project is a no-op for an unknown path") + func removePackUnknownProject() { + let index = ProjectIndex(path: URL(fileURLWithPath: "/tmp/test.yaml")) + var data = ProjectIndex.IndexData() + index.upsert(projectPath: "/path/a", packIDs: ["ios"], in: &data) + + index.removePack("ios", fromProject: "/path/missing", in: &data) + + #expect(data.projects.count == 1) + #expect(data.projects[0].packs == ["ios"]) + } + // MARK: - Queries @Test("Projects with pack returns matching entries") From ff063db9c51a800494a21384b671a6d693480e5d Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Wed, 2 Sep 2026 15:26:36 +0200 Subject: [PATCH 2/3] Document the scope-duplication doctor check in the architecture map - List ScopeDuplicationCheck.swift alongside the other Doctor modules, noting the three gates that must pass before --fix removes anything Claude-Session: https://claude.ai/code/session_01MkBJdGBUoZ2aRtchjaKXMN --- CLAUDE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CLAUDE.md b/CLAUDE.md index ba9c7d59..4cb1c33a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -115,6 +115,7 @@ mcs config set # Set a configuration value (true/false) - `CoreDoctorChecks.swift` — check structs (CommandCheck, MCPServerCheck, PluginCheck, HookCheck, GitignoreCheck, CommandFileCheck, FileExistsCheck, FileContentCheck, HookSettingsCheck, SettingsKeysCheck, SettingsDriftCheck, PackGitignoreCheck, ProjectIndexCheck) - `DerivedDoctorChecks.swift` — `deriveDoctorCheck()` extension on ComponentDefinition - `ProjectDoctorChecks.swift` — project-scoped checks (CLAUDE.local.md freshness, state file) +- `ScopeDuplicationCheck.swift` — flags a pack configured in both the project and global scope, reporting only artifacts that genuinely exist twice; `--fix` removes the project copy via `Configurator.unconfigurePack`, gated on component subset, prompt-answer parity, and the recorded hash of every file it would delete - `SectionValidator.swift` — validation of CLAUDE.local.md section markers ### Commands (`Sources/mcs/Commands/`) From f49d28bb6051ddc353ee69cadfdb00e070a20268 Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Wed, 2 Sep 2026 16:04:07 +0200 Subject: [PATCH 3/3] Trim whitespace around comma-separated --pack ids - '--pack "ios, swift"' previously produced a " swift" id that matched no pack and was reported as unregistered instead of being checked - Add a doctor integration test asserting the two spellings of one filter produce identical runs Claude-Session: https://claude.ai/code/session_01MkBJdGBUoZ2aRtchjaKXMN --- Sources/mcs/Doctor/DoctorRunner.swift | 11 +++++- .../mcs/Doctor/ScopeDuplicationCheck.swift | 8 +++-- .../DoctorRunnerIntegrationTests.swift | 34 +++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/Sources/mcs/Doctor/DoctorRunner.swift b/Sources/mcs/Doctor/DoctorRunner.swift index 88e23ebf..dfdf6921 100644 --- a/Sources/mcs/Doctor/DoctorRunner.swift +++ b/Sources/mcs/Doctor/DoctorRunner.swift @@ -20,8 +20,17 @@ struct DoctorRunner { let packFilter: String? /// `packFilter` split into identifiers, so the comma convention is defined in one place. + /// + /// Entries are trimmed and empties dropped: `--pack "ios, swift"` is a natural thing to type, + /// and an untrimmed `" swift"` matches no pack — the run would just report it as unregistered. private var packFilterIDs: Set? { - packFilter.map { Set($0.components(separatedBy: ",")) } + packFilter.map { + Set( + $0.components(separatedBy: ",") + .map { $0.trimmingCharacters(in: .whitespaces) } + .filter { !$0.isEmpty } + ) + } } /// When true, check only globally-configured packs (ignores project scope). diff --git a/Sources/mcs/Doctor/ScopeDuplicationCheck.swift b/Sources/mcs/Doctor/ScopeDuplicationCheck.swift index 31f9eae5..39a8c1bb 100644 --- a/Sources/mcs/Doctor/ScopeDuplicationCheck.swift +++ b/Sources/mcs/Doctor/ScopeDuplicationCheck.swift @@ -241,9 +241,11 @@ struct ScopeDuplicationCheck: DoctorCheck { /// Whether installing this action in both scopes leaves two copies that both take effect. /// /// Only `copyPackFile` does. Its files land in `/.claude/` and `~/.claude/` - /// independently, and a hook component's settings entry is written with a different command - /// prefix per scope (`Constants.HookCommand`), so the two entries are distinct strings that - /// both fire. Everything else either shadows (project `settings.local.json` over global + /// independently, and a hook component's settings entry embeds the scope's hook directory + /// (`Constants.HookCommand.projectDirectory` / `.globalDirectory`) — the interpreter is + /// resolved per component and is the same in both scopes, but the directory differs, so the + /// two entries are distinct strings that both fire. Everything else either shadows (project + /// `settings.local.json` over global /// `settings.json`, MCP `local` over `user`), is installed once (the project scope skips brew /// packages and plugins entirely), writes one idempotent line (gitignore), or is a one-shot /// side effect rather than a standing artifact (`shellCommand`). diff --git a/Tests/MCSTests/DoctorRunnerIntegrationTests.swift b/Tests/MCSTests/DoctorRunnerIntegrationTests.swift index 0ae38269..47b6d9bf 100644 --- a/Tests/MCSTests/DoctorRunnerIntegrationTests.swift +++ b/Tests/MCSTests/DoctorRunnerIntegrationTests.swift @@ -552,4 +552,38 @@ struct DoctorSummaryWarningCountTests { #expect(withGhost.warnings == baseline.warnings + 1) } + + /// `--pack "ios, swift"` is a natural thing to type. Without trimming, the second id carries a + /// leading space, matches no pack, and the run reports it as unregistered instead of checking it. + @Test("--pack filter tolerates whitespace around comma-separated ids") + func packFilterTrimsWhitespace() throws { + let (home, project) = try makeSandboxProject(label: "packfilter-whitespace") + defer { try? FileManager.default.removeItem(at: home) } + + let registry = TechPackRegistry(packs: [ + MockTechPack(identifier: "pack-a", displayName: "Pack A"), + MockTechPack(identifier: "pack-b", displayName: "Pack B"), + ]) + + var state = try ProjectState(projectRoot: project) + state.recordPack("pack-a") + state.recordPack("pack-b") + try state.save() + + var tightRunner = makeRunner( + home: home, projectRoot: project, registry: registry, packFilter: "pack-a,pack-b" + ) + let tight = try tightRunner.run() + + var spacedRunner = makeRunner( + home: home, projectRoot: project, registry: registry, packFilter: " pack-a , pack-b " + ) + let spaced = try spacedRunner.run() + + // Identical filters spelled differently must produce identical runs — in particular no + // extra "not registered" advisory for a space-prefixed id. + #expect(spaced.warnings == tight.warnings) + #expect(spaced.passed == tight.passed) + #expect(spaced.issues == tight.issues) + } }