ISSUE-372: Warn when global sync duplicates project-installed packs

Sources/mcs/Sync/Configurator.swift

@@ -193,14 +193,24 @@ struct Configurator {
             packs: packs, output: output, filesystemContext: fsContext
         )
         let headerLabel = scope.isGlobalScope ? "Plan (Global)" : "Plan"
-        ConfiguratorSupport.dryRunSummary(
+        let additions = ConfiguratorSupport.dryRunSummary(
             packs: packs,
             state: state,
             header: headerLabel,
             output: output,
             artifactSummary: { strategy.printArtifactSummary($0, output: output) },
             removalSummary: { strategy.printRemovalSummary($0, output: output) }
         )
+
+        // Emitted after the plan so it reads as a consequence of it, reusing the plan's own
+        // additions set so the warning can never disagree with what was just printed.
+        ConfiguratorSupport.warnProjectDuplication(
+            isGlobalScope: scope.isGlobalScope,
+            additions: additions,
+            packs: packs,
+            environment: environment,
+            output: output
+        )
     }
 
     // MARK: - Configure (Multi-Pack)
@@ -251,6 +261,21 @@ struct Configurator {
             }
         }
 
+        // Advisory only: a pack entering the global scope duplicates its artifacts in every
+        // project that already holds it. Read-only — it never touches `packs` or the index.
+        //
+        // Placed after the removal gate, not before: cancelling there returns without installing
+        // anything, and a warning about duplicates that were never created would be a lie — one
+        // that also costs a needless index read. Still ahead of all install work, so it precedes
+        // the artifacts it warns about.
+        ConfiguratorSupport.warnProjectDuplication(
+            isGlobalScope: scope.isGlobalScope,
+            additions: additions,
+            packs: packs,
+            environment: environment,
+            output: output
+        )
+
         for packID in removals.sorted() {
             unconfigurePack(packID, state: &state)
         }

Sources/mcs/Sync/ConfiguratorSupport.swift

@@ -24,6 +24,98 @@ enum ConfiguratorSupport {
         })
     }
 
+    /// Warning lines naming the tracked projects that already configure packs newly entering
+    /// the *global* scope. Empty when nothing applies.
+    ///
+    /// The mirror of `globallyBlockedIDs`: that rule stops a globally-installed pack being added
+    /// to a project; this one reports the reverse move. Installing globally is legitimate, but it
+    /// duplicates every hook and skill in the projects that already hold the pack — the global and
+    /// project copies register as distinct settings entries and both fire.
+    ///
+    /// Keyed on `additions` rather than the whole selection, so re-syncing a pack the global scope
+    /// already has stays silent, and `mcs update` — which re-applies each scope's existing set —
+    /// never warns at all.
+    ///
+    /// Pure so the wording itself is testable: `CLIOutput` writes straight to stdout and the suite
+    /// has no way to capture it. Element 0 is the `warn` header, the rest are plain detail lines;
+    /// there is exactly one header however many packs are involved.
+    ///
+    /// - Parameter pathExists: Injected so tests never reach the real filesystem.
+    static func projectDuplicationWarning(
+        additions: Set<String>,
+        displayNames: [String: String],
+        index: ProjectIndex.IndexData,
+        pathExists: (String) -> Bool = { FileManager.default.fileExists(atPath: $0) }
+    ) -> [String] {
+        var details: [String] = []
+        for packID in additions.sorted() {
+            // The global scope carries its own index entry. It is the scope being installed
+            // into, not a project that ends up with a duplicate.
+            let paths = index.projects
+                .filter { !$0.isGlobal && $0.packs.contains(packID) }
+                .map(\.path)
+                .filter(pathExists)
+                .sorted()
+            guard !paths.isEmpty else { continue }
+            details.append("    \(displayNames[packID] ?? packID) → \(paths.joined(separator: ", "))")
+        }
+        guard !details.isEmpty else { return [] }
+
+        return ["\(details.count) pack(s) being installed globally are already configured in other projects:"]
+            + details
+            + [
+                "  Their hooks and skills will run twice there until the project copy is removed.",
+                "  Run 'mcs doctor --fix' in those projects to drop the project copy.",
+            ]
+    }
+
+    /// Emit `projectDuplicationWarning`, doing nothing outside the global scope.
+    ///
+    /// The scope gate lives here rather than at each call site: only a global install can create
+    /// this duplication, so every caller needs the same check, and a future one that forgot it
+    /// would print a nonsensical "installed globally" notice during a project sync.
+    ///
+    /// Advisory only: it never filters the pack set — `Configurator.configure` treats that set as a
+    /// complete desired state and unconfigures anything missing from it — and never writes to the
+    /// index, stale entries included. An unreadable index degrades to a notice rather than failing
+    /// the sync; `ProjectIndexCheck` is what reports a broken index.
+    static func warnProjectDuplication(
+        isGlobalScope: Bool,
+        additions: Set<String>,
+        packs: [any TechPack],
+        environment: Environment,
+        output: CLIOutput
+    ) {
+        // Ordered so a project-scope sync and a no-op global sync both return before any disk read.
+        guard isGlobalScope, !additions.isEmpty else { return }
+
+        let index: ProjectIndex.IndexData
+        do {
+            index = try ProjectIndex(path: environment.projectsIndexFile).load()
+        } catch {
+            output.warn("Could not read project index: \(error.localizedDescription)")
+            output.plain("  Skipping the check for packs already configured in other projects.")
+            return
+        }
+
+        let displayNames = Dictionary(
+            packs.map { ($0.identifier, $0.displayName) },
+            uniquingKeysWith: { first, _ in first }
+        )
+        let lines = projectDuplicationWarning(
+            additions: additions,
+            displayNames: displayNames,
+            index: index
+        )
+        guard let header = lines.first else { return }
+
+        output.plain("")
+        output.warn(header)
+        for line in lines.dropFirst() {
+            output.plain(line)
+        }
+    }
+
     /// Build a `ComponentExecutor` from the common dependencies.
     static func makeExecutor(
         environment: Environment,
@@ -49,14 +141,18 @@ enum ConfiguratorSupport {
     ///
     /// Shared orchestration for both project and global dry-run flows.
     /// Callers provide scope-specific closures for artifact and removal display.
+    ///
+    /// Returns the pack IDs newly entering this scope, so a caller needing the same diff reuses
+    /// this one rather than re-deriving it and risking the two disagreeing.
+    @discardableResult
     static func dryRunSummary(
         packs: [any TechPack],
         state: ProjectState,
         header: String,
         output: CLIOutput,
         artifactSummary: (_ pack: any TechPack) -> Void,
         removalSummary: (_ artifacts: PackArtifactRecord) -> Void
-    ) {
+    ) -> Set<String> {
         let selectedIDs = Set(packs.map(\.identifier))
         let previousIDs = state.configuredPacks
 
@@ -71,7 +167,7 @@ enum ConfiguratorSupport {
             output.info("No packs selected. Nothing would change.")
             output.plain("")
             output.dimmed("No changes made (dry run).")
-            return
+            return additions
         }
 
         // Show additions
@@ -112,6 +208,7 @@ enum ConfiguratorSupport {
         }
         output.plain("")
         output.dimmed("No changes made (dry run).")
+        return additions
     }
 
     /// Present per-pack component multi-select and return excluded component IDs.

Tests/MCSTests/LifecycleIntegrationTests.swift

@@ -43,10 +43,13 @@ private struct LifecycleTestBed {
         )
     }
 
-    func makeGlobalSyncConfigurator(registry: TechPackRegistry = TechPackRegistry()) -> Configurator {
+    func makeGlobalSyncConfigurator(
+        registry: TechPackRegistry = TechPackRegistry(),
+        warningCounter: WarningCounter? = nil
+    ) -> Configurator {
         Configurator(
             environment: env,
-            output: CLIOutput(colorsEnabled: false),
+            output: CLIOutput(colorsEnabled: false, warningCounter: warningCounter),
             shell: ShellRunner(environment: env),
             registry: registry,
             strategy: GlobalSyncStrategy(environment: env),
@@ -1067,6 +1070,149 @@ struct GlobalScopeLifecycleTests {
     }
 }
 
+// MARK: - Scenario 5c: Global install warns about projects that already hold the pack
+
+struct GlobalDuplicationWarningTests {
+    /// Seed `~/.mcs/projects.yaml` as a prior `mcs sync` in each project would have.
+    private func seedIndex(env: Environment, entries: [(String, [String])]) throws {
+        let indexFile = ProjectIndex(path: env.projectsIndexFile)
+        var data = ProjectIndex.IndexData()
+        for entry in entries {
+            indexFile.upsert(projectPath: entry.0, packIDs: entry.1, in: &data)
+        }
+        try indexFile.save(data)
+    }
+
+    /// Project entries only. The `__global__` entry legitimately changes during a global
+    /// sync (step 11 upserts it), so it is excluded from the read-only assertion.
+    private func projectEntries(env: Environment) throws -> [ProjectIndex.ProjectEntry] {
+        try ProjectIndex(path: env.projectsIndexFile).load()
+            .projects
+            .filter { !$0.isGlobal }
+            .sorted { $0.path < $1.path }
+    }
+
+    private func makeProjects(in home: URL, _ names: [String]) throws -> [URL] {
+        try names.map { name in
+            let url = home.appendingPathComponent(name)
+            try FileManager.default.createDirectory(at: url, withIntermediateDirectories: true)
+            return url
+        }
+    }
+
+    private func duplicatedPack(bed: LifecycleTestBed) throws -> MockTechPack {
+        let hookSource = try bed.makeHookSource(name: "dup-hook.sh")
+        return MockTechPack(
+            identifier: "dup-pack",
+            displayName: "Dup Pack",
+            components: [
+                bed.hookComponent(pack: "dup-pack", id: "hook", source: hookSource, destination: "dup-hook.sh"),
+            ]
+        )
+    }
+
+    @Test("Warns when tracked projects already configure a pack entering the global scope")
+    func warnsAndLeavesProjectEntriesUntouched() throws {
+        let bed = try LifecycleTestBed()
+        defer { bed.cleanup() }
+
+        let projects = try makeProjects(in: bed.home, ["project-a", "project-b"])
+        try seedIndex(env: bed.env, entries: projects.map { ($0.path, ["dup-pack"]) })
+        let before = try projectEntries(env: bed.env)
+
+        let pack = try duplicatedPack(bed: bed)
+        let counter = WarningCounter()
+        try bed.makeGlobalSyncConfigurator(registry: TechPackRegistry(packs: [pack]), warningCounter: counter)
+            .configure(packs: [pack], confirmRemovals: false)
+
+        // One `warn` header covers every pack and project it names, so the count is
+        // stable no matter how many duplicates there are.
+        #expect(counter.count == 1)
+
+        // Advisory only: the warning must not touch the projects it names, nor prune
+        // their index entries. Only the global scope changes.
+        let after = try projectEntries(env: bed.env)
+        #expect(after == before)
+        #expect(try ProjectState(stateFile: bed.env.globalStateFile).configuredPacks.contains("dup-pack"))
+    }
+
+    @Test("Silent when no tracked project holds the pack being installed globally")
+    func silentWithoutOverlap() throws {
+        let bed = try LifecycleTestBed()
+        defer { bed.cleanup() }
+
+        let projects = try makeProjects(in: bed.home, ["project-a"])
+        try seedIndex(env: bed.env, entries: projects.map { ($0.path, ["other-pack"]) })
+
+        let pack = try duplicatedPack(bed: bed)
+        let counter = WarningCounter()
+        try bed.makeGlobalSyncConfigurator(registry: TechPackRegistry(packs: [pack]), warningCounter: counter)
+            .configure(packs: [pack], confirmRemovals: false)
+
+        #expect(counter.count == 0)
+    }
+
+    @Test("Re-syncing a pack the global scope already has does not warn again")
+    func silentOnSecondGlobalSync() throws {
+        let bed = try LifecycleTestBed()
+        defer { bed.cleanup() }
+
+        let projects = try makeProjects(in: bed.home, ["project-a"])
+        try seedIndex(env: bed.env, entries: projects.map { ($0.path, ["dup-pack"]) })
+
+        let pack = try duplicatedPack(bed: bed)
+        let registry = TechPackRegistry(packs: [pack])
+        let first = WarningCounter()
+        try bed.makeGlobalSyncConfigurator(registry: registry, warningCounter: first)
+            .configure(packs: [pack], confirmRemovals: false)
+        #expect(first.count == 1)
+
+        // The trigger is a transition, not an identity: the pack is no longer an addition,
+        // so the second sync is quiet. This is also what keeps `mcs update` silent.
+        let second = WarningCounter()
+        try bed.makeGlobalSyncConfigurator(registry: registry, warningCounter: second)
+            .configure(packs: [pack], confirmRemovals: false)
+        #expect(second.count == 0)
+    }
+
+    @Test("--dry-run surfaces the same warning and installs nothing")
+    func warnsOnDryRunWithoutInstalling() throws {
+        let bed = try LifecycleTestBed()
+        defer { bed.cleanup() }
+
+        let projects = try makeProjects(in: bed.home, ["project-a"])
+        try seedIndex(env: bed.env, entries: projects.map { ($0.path, ["dup-pack"]) })
+
+        let pack = try duplicatedPack(bed: bed)
+        let counter = WarningCounter()
+        try bed.makeGlobalSyncConfigurator(registry: TechPackRegistry(packs: [pack]), warningCounter: counter)
+            .dryRun(packs: [pack])
+
+        // `dryRun` keeps no diff of its own, so this covers the second call site's
+        // locally-derived additions set.
+        #expect(counter.count == 1)
+        #expect(try ProjectState(stateFile: bed.env.globalStateFile).configuredPacks.isEmpty)
+    }
+
+    @Test("An unreadable project index degrades to a notice — the sync still completes")
+    func unreadableIndexDoesNotFailSync() throws {
+        let bed = try LifecycleTestBed()
+        defer { bed.cleanup() }
+
+        try "{{ not yaml".write(to: bed.env.projectsIndexFile, atomically: true, encoding: .utf8)
+
+        let pack = try duplicatedPack(bed: bed)
+        let counter = WarningCounter()
+        try bed.makeGlobalSyncConfigurator(registry: TechPackRegistry(packs: [pack]), warningCounter: counter)
+            .configure(packs: [pack], confirmRemovals: false)
+
+        // One warning for the unreadable index. The later index *write* also fails, but
+        // reports through `output.error`, which does not touch the counter.
+        #expect(counter.count == 1)
+        #expect(try ProjectState(stateFile: bed.env.globalStateFile).configuredPacks.contains("dup-pack"))
+    }
+}
+
 // MARK: - Scenario 5b: Shell Command Component Lifecycle
 
 struct ShellCommandLifecycleTests {