ISSUE-373: Regression test: 'mcs update' must not remove both-scope packs

Sources/mcs/Commands/UpdateCommand.swift

@@ -313,47 +313,68 @@ struct UpdateCommand: LockedCommand {
         output: CLIOutput
     ) throws {
         for run in runs {
-            output.header(run.label)
-
-            let packIDs = run.configuredPackIDs.subtracting(skippedPackIDs).sorted()
-
-            var packs: [any TechPack] = []
-            var unresolved: [String] = []
-            for packID in packIDs {
-                if let pack = registry.pack(for: packID) {
-                    packs.append(pack)
-                } else {
-                    unresolved.append(packID)
-                }
-            }
+            try Self.reapplyScope(
+                run,
+                skippedPackIDs: skippedPackIDs,
+                registry: registry,
+                dryRun: dryRun,
+                env: env,
+                shell: shell,
+                output: output
+            )
+        }
+    }
 
-            for packID in unresolved {
-                output.warn("  \(packID): tracked in state but missing from pack registry — skipping. Run 'mcs pack add' to restore it.")
-            }
+    /// Print one scope's header, resolve its configured packs, and converge the scope onto them.
+    ///
+    /// `static` so tests can drive the real re-apply — `UpdateCommand` builds its own
+    /// `Environment()`, so instance paths are not reachable from a sandboxed test bed.
+    /// The list must stay the scope's own configured set: `Configurator.configure` treats it
+    /// as the complete desired state and unconfigures anything missing, with no prompt here.
+    static func reapplyScope(
+        _ run: UpdateScopeResolver.ScopeRun,
+        skippedPackIDs: Set<String>,
+        registry: TechPackRegistry,
+        dryRun: Bool,
+        env: Environment,
+        shell: any ShellRunning,
+        output: CLIOutput,
+        claudeCLI: (any ClaudeCLI)? = nil
+    ) throws {
+        output.header(run.label)
 
-            guard !packs.isEmpty else {
-                output.info("No packs to refresh in this scope.")
+        var packs: [any TechPack] = []
+        for packID in run.configuredPackIDs.subtracting(skippedPackIDs).sorted() {
+            guard let pack = registry.pack(for: packID) else {
+                output.warn("  \(packID): tracked in state but missing from pack registry — skipping. Run 'mcs pack add' to restore it.")
                 continue
             }
+            packs.append(pack)
+        }
 
-            let configurator = Configurator(
-                environment: env,
-                output: output,
-                shell: shell,
-                registry: registry,
-                strategy: run.strategy
-            )
+        guard !packs.isEmpty else {
+            output.info("No packs to refresh in this scope.")
+            return
+        }
 
-            if dryRun {
-                try configurator.dryRun(packs: packs)
-            } else {
-                try configurator.configure(
-                    packs: packs,
-                    confirmRemovals: false,
-                    excludedComponents: run.excludedComponents,
-                    reusePriorValuesSilently: true
-                )
-            }
+        let configurator = Configurator(
+            environment: env,
+            output: output,
+            shell: shell,
+            registry: registry,
+            strategy: run.strategy,
+            claudeCLI: claudeCLI
+        )
+
+        if dryRun {
+            try configurator.dryRun(packs: packs)
+        } else {
+            try configurator.configure(
+                packs: packs,
+                confirmRemovals: false,
+                excludedComponents: run.excludedComponents,
+                reusePriorValuesSilently: true
+            )
         }
     }
 

Tests/MCSTests/LifecycleIntegrationTests.swift

@@ -204,6 +204,10 @@ private struct LifecycleTestBed {
         try ProjectState(projectRoot: project)
     }
 
+    func globalState() throws -> ProjectState {
+        try ProjectState(stateFile: env.globalStateFile)
+    }
+
     func settingsEnv() throws -> [String: Any] {
         let data = try Data(contentsOf: settingsLocalPath)
         let json = try JSONSerialization.jsonObject(with: data) as? [String: Any] ?? [:]
@@ -2259,6 +2263,73 @@ struct GlobalPackBlockingLifecycleTests {
     }
 }
 
+// MARK: - Update Re-apply
+
+/// End-to-end coverage for the `mcs update` re-apply phase.
+///
+/// Drives the real `UpdateScopeResolver` and `UpdateCommand.reapplyScope` rather than
+/// `UpdateCommand.perform()`, which builds its own `Environment()` and cannot be pointed
+/// at a sandboxed home.
+struct UpdateReapplyLifecycleTests {
+    @Test(
+        "Both-scope pack survives update re-apply",
+        arguments: [
+            UpdateScopeResolver.Filter.projectOnly, // mcs update --project
+            .all, // bare mcs update: global run, then project run
+        ]
+    )
+    func bothScopePackSurvivesUpdate(filter: UpdateScopeResolver.Filter) throws {
+        let bed = try LifecycleTestBed()
+        defer { bed.cleanup() }
+
+        let hookSource = try bed.makeHookSource(name: "check.sh")
+        let shared = MockTechPack(
+            identifier: "shared-pack",
+            displayName: "Shared Pack",
+            components: [bed.hookComponent(pack: "shared-pack", id: "check", source: hookSource, destination: "check.sh")]
+        )
+        let registry = TechPackRegistry(packs: [shared])
+
+        // Installed in both scopes — the case the global-pack block must never reach.
+        try bed.makeConfigurator(registry: registry)
+            .configure(packs: [shared], confirmRemovals: false)
+        try bed.makeGlobalSyncConfigurator(registry: registry)
+            .configure(packs: [shared], confirmRemovals: false)
+        #expect(try bed.globalState().configuredPacks.contains("shared-pack"))
+
+        // Delete the project copy of the hook so a surviving pack is distinguishable from a
+        // re-apply that never ran: `copyPackFile` is convergent, so `configure` restores it.
+        let projectHook = bed.project.appendingPathComponent(".claude/hooks/shared-pack/check.sh")
+        #expect(FileManager.default.fileExists(atPath: projectHook.path))
+        try FileManager.default.removeItem(at: projectHook)
+
+        let runs = try UpdateScopeResolver(environment: bed.env, output: CLIOutput(colorsEnabled: false))
+            .resolve(filter: filter, projectRoot: bed.project)
+        // An empty run list would pass every assertion below without touching anything.
+        #expect(runs.count == (filter == .all ? 2 : 1))
+
+        for run in runs {
+            try UpdateCommand.reapplyScope(
+                run,
+                skippedPackIDs: [],
+                registry: registry,
+                dryRun: false,
+                env: bed.env,
+                shell: ShellRunner(environment: bed.env),
+                output: CLIOutput(colorsEnabled: false),
+                claudeCLI: bed.mockCLI
+            )
+        }
+
+        // The regression guard: an identity-based filter, or a list sourced from anywhere but
+        // the scope's own state, would have handed `configure` a set missing `shared-pack`
+        // and unconfigured it from the project without a prompt.
+        #expect(try bed.projectState().configuredPacks.contains("shared-pack"))
+        #expect(try bed.globalState().configuredPacks.contains("shared-pack"))
+        #expect(FileManager.default.fileExists(atPath: projectHook.path))
+    }
+}
+
 // MARK: - Scenario: Hook Interpreters
 
 struct HookInterpreterLifecycleTests {