ISSUE-449: Track pack settings ownership per sub-key

Sources/mcs/Core/Settings.swift

@@ -207,11 +207,12 @@ struct Settings: Codable {
     ///   scalars/arrays use "existing wins" semantics.
     ///
     /// - Returns: The hook groups dropped by deduplication whose matcher disagreed with the group
-    ///   that survived. An identical duplicate is the ordinary case and is not reported. Callers
-    ///   with somewhere to report to should surface these; the result is discardable because most
-    ///   callers merge settings that cannot collide.
+    ///   that survived (an identical duplicate is the ordinary case and is not reported), and the
+    ///   extra-JSON key paths `other` actually wrote versus the ones it lost to an existing,
+    ///   different value. Callers with somewhere to report to should surface the losses; the
+    ///   result is discardable because most callers merge settings that cannot collide.
     @discardableResult
-    mutating func merge(with other: Settings) -> [DroppedHookGroup] {
+    mutating func merge(with other: Settings) -> MergeReport {
         var dropped: [DroppedHookGroup] = []
 
         // Hooks: deduplicate by command
@@ -259,27 +260,75 @@ struct Settings: Codable {
 
         // Extra JSON: generic merge for all non-typed keys (env, permissions,
         // alwaysThinkingEnabled, attribution, and any future keys).
-        for (key, valueData) in other.extraJSON {
-            if let existingData = extraJSON[key] {
-                // Both have the key — attempt dict-level merge if both are JSON objects
-                if let selfDict = try? JSONSerialization.jsonObject(with: existingData) as? [String: Any],
-                   let otherDict = try? JSONSerialization.jsonObject(with: valueData) as? [String: Any] {
-                    var merged = selfDict
-                    for (k, v) in otherDict where merged[k] == nil {
-                        merged[k] = v
-                    }
-                    if let data = try? JSONSerialization.data(withJSONObject: merged) {
-                        extraJSON[key] = data
+        var landed: [String] = []
+        var skipped: [String] = []
+        for (key, valueData) in other.extraJSON.sorted(by: { $0.key < $1.key }) {
+            guard let existingData = extraJSON[key] else {
+                extraJSON[key] = valueData
+                landed.append(contentsOf: Self.keyPaths(of: key, in: valueData))
+                continue
+            }
+            // Both have the key — attempt dict-level merge if both are JSON objects
+            if let selfDict = try? JSONSerialization.jsonObject(with: existingData) as? [String: Any],
+               let otherDict = try? JSONSerialization.jsonObject(with: valueData) as? [String: Any] {
+                var merged = selfDict
+                var landedHere: [String] = []
+                for (subKey, value) in otherDict.sorted(by: { $0.key < $1.key }) {
+                    if let current = merged[subKey] {
+                        if !Self.jsonEqual(current, value) { skipped.append("\(key).\(subKey)") }
+                    } else {
+                        merged[subKey] = value
+                        landedHere.append("\(key).\(subKey)")
                     }
-                    // Re-serialization failure: keep existing value (no-op)
                 }
-                // Non-dict: existing wins (no action)
-            } else {
-                extraJSON[key] = valueData
+                if let data = try? JSONSerialization.data(withJSONObject: merged) {
+                    extraJSON[key] = data
+                    landed.append(contentsOf: landedHere)
+                }
+                // Re-serialization failure: keep existing value (no-op)
+            } else if !Self.jsonFragmentsEqual(existingData, valueData) {
+                // Non-dict: existing wins
+                skipped.append(key)
             }
         }
 
-        return dropped
+        return MergeReport(droppedHookGroups: dropped, landedKeyPaths: landed, skippedKeyPaths: skipped)
+    }
+
+    struct MergeReport {
+        var droppedHookGroups: [DroppedHookGroup] = []
+        var landedKeyPaths: [String] = []
+        var skippedKeyPaths: [String] = []
+    }
+
+    // `save(to:dropKeys:)` must not re-inject any of these from the destination file: the
+    // in-memory value is already the destination's minus what was owned, so a key left empty
+    // once its owned sub-keys are removed has to stay gone.
+    static func topLevelKeys(of keyPaths: [String]) -> Set<String> {
+        Set(keyPaths.map { String($0.split(separator: ".", maxSplits: 1)[0]) })
+    }
+
+    /// One level down for objects, so a pack that writes `env.FOO` never claims the user's `env.BAR`.
+    private static func keyPaths(of key: String, in valueData: Data) -> [String] {
+        guard let dict = try? JSONSerialization.jsonObject(with: valueData) as? [String: Any],
+              !dict.isEmpty
+        else {
+            return [key]
+        }
+        return dict.keys.sorted().map { "\(key).\($0)" }
+    }
+
+    private static func jsonEqual(_ lhs: Any, _ rhs: Any) -> Bool {
+        (lhs as AnyObject).isEqual(rhs)
+    }
+
+    private static func jsonFragmentsEqual(_ lhs: Data, _ rhs: Data) -> Bool {
+        guard let left = try? JSONSerialization.jsonObject(with: lhs, options: .fragmentsAllowed),
+              let right = try? JSONSerialization.jsonObject(with: rhs, options: .fragmentsAllowed)
+        else {
+            return lhs == rhs
+        }
+        return jsonEqual(left, right)
     }
 
     // MARK: - Stale key removal

Sources/mcs/Sync/Configurator.swift

@@ -342,13 +342,14 @@ struct Configurator {
             resolvedValues: allValues, output: output
         )
 
-        // 6b. Record contributed settings keys and value hashes in artifact records
-        for (packID, keys) in contributedKeys {
-            if var artifacts = state.artifacts(for: packID) {
-                artifacts.settingsKeys = keys
-                artifacts.settingsHash = settingsHashes[packID]
-                state.setArtifacts(artifacts, for: packID)
-            }
+        // 6b. Record contributed settings keys and value hashes in artifact records. Every pack is
+        // reset, not just the ones that landed a key: a pack whose values were all already set
+        // owns nothing now, and a stale claim would let its removal delete another pack's key.
+        for pack in packs {
+            guard var artifacts = state.artifacts(for: pack.identifier) else { continue }
+            artifacts.settingsKeys = contributedKeys[pack.identifier] ?? []
+            artifacts.settingsHash = settingsHashes[pack.identifier]
+            state.setArtifacts(artifacts, for: pack.identifier)
         }
 
         // 7. Compose CLAUDE markdown file
@@ -478,6 +479,8 @@ struct Configurator {
 
         // Remove auto-derived hook commands and contributed settings keys
         let hasHooksToRemove = !artifacts.hookCommands.isEmpty
+        // Keyed to the record, not its expansion: a legacy key left untouched still has to be
+        // cleared from state, or every later removal warns about it again.
         let hasSettingsToRemove = !artifacts.settingsKeys.isEmpty
         if hasHooksToRemove || hasSettingsToRemove {
             var settings: Settings
@@ -490,6 +493,9 @@ struct Configurator {
                 output.warn("Some artifacts for \(packID) could not be removed. Re-run '\(retryHint)' to retry.")
                 return false
             }
+            let settingsKeys = expandedSettingsKeys(
+                artifacts.settingsKeys, pack: registry.pack(for: packID), packID: packID, onDisk: settings
+            )
             if hasHooksToRemove {
                 let commandsToRemove = Set(artifacts.hookCommands)
                 if var hooks = settings.hooks {
@@ -504,18 +510,18 @@ struct Configurator {
                 }
             }
             if hasSettingsToRemove {
-                settings.removeKeys(artifacts.settingsKeys)
+                settings.removeKeys(settingsKeys)
             }
             do {
-                let dropKeys = Set(artifacts.settingsKeys.filter { !$0.contains(".") })
+                let dropKeys = Settings.topLevelKeys(of: settingsKeys)
                 try settings.save(to: scope.settingsPath, dropKeys: dropKeys)
                 remaining.hookCommands = []
                 remaining.settingsKeys = []
                 remaining.settingsHash = nil
                 for cmd in artifacts.hookCommands {
                     output.dimmed("  Removed hook: \(cmd)")
                 }
-                for key in artifacts.settingsKeys {
+                for key in settingsKeys {
                     output.dimmed("  Removed setting: \(key)")
                 }
             } catch {
@@ -875,11 +881,20 @@ struct Configurator {
         var previousSettingsKeys: [String: [String]] = [:]
         var previousTemplateSections: [String: [String]] = [:]
 
+        // Composition reloads this and reports a parse failure; an unreadable file simply leaves
+        // legacy records unexpanded here.
+        var onDiskSettings = Settings()
+        do {
+            onDiskSettings = try Settings.load(from: scope.settingsPath)
+        } catch {}
+
         for pack in packs {
             let previousArtifacts = state.artifacts(for: pack.identifier)
 
             // Snapshot previous metadata before overwriting (needed by steps 6-7)
-            previousSettingsKeys[pack.identifier] = previousArtifacts?.settingsKeys ?? []
+            previousSettingsKeys[pack.identifier] = expandedSettingsKeys(
+                previousArtifacts?.settingsKeys ?? [], pack: pack, packID: pack.identifier, onDisk: onDiskSettings
+            )
             previousTemplateSections[pack.identifier] = previousArtifacts?.templateSections ?? []
 
             let isNew = additions.contains(pack.identifier)
@@ -1244,6 +1259,15 @@ struct Configurator {
         }
     }
 
+    private func expandedSettingsKeys(
+        _ keyPaths: [String], pack: (any TechPack)?, packID: String, onDisk: Settings
+    ) -> [String] {
+        ConfiguratorSupport.expandLegacySettingsKeys(
+            keyPaths, pack: pack, packID: packID, onDisk: onDisk,
+            settingsFileName: scope.settingsPath.lastPathComponent, output: output
+        )
+    }
+
     // MARK: - Global Dependencies
 
     /// Auto-install brew packages and plugins (project scope only).

Sources/mcs/Sync/ConfiguratorSupport.swift

@@ -404,10 +404,19 @@ enum ConfiguratorSupport {
             guard case let .settingsMerge(source) = component.installAction, let source else { continue }
             do {
                 let packSettings = try Settings.load(from: source, substituting: resolvedValues)
-                if !packSettings.extraJSON.isEmpty {
-                    contributedKeys[pack.identifier, default: []].append(contentsOf: packSettings.extraJSON.keys)
+                let report = settings.merge(with: packSettings)
+                // Only what landed is owned: a key the user (or an earlier pack) already set stays
+                // theirs, so removing this pack can never delete it.
+                if !report.landedKeyPaths.isEmpty {
+                    contributedKeys[pack.identifier, default: []].append(contentsOf: report.landedKeyPaths)
                 }
-                for dropped in settings.merge(with: packSettings) {
+                for keyPath in report.skippedKeyPaths {
+                    output.warn(
+                        "\(pack.displayName): '\(keyPath)' from \(source.lastPathComponent) was not applied"
+                            + " — a different value is already set, and it is kept"
+                    )
+                }
+                for dropped in report.droppedHookGroups {
                     output.warn(
                         "\(pack.displayName): hook group for '\(dropped.command)' under \(dropped.event)"
                             + " was not merged — \(source.lastPathComponent) declares matcher"
@@ -426,6 +435,58 @@ enum ConfiguratorSupport {
         return (hasContent, contributedKeys)
     }
 
+    /// State written before per-sub-key ownership records a whole object such as `env`, and
+    /// stripping it would delete the user's entries inside it. Narrow it to the sub-keys the pack
+    /// declares today; when that can't be known, leave the object alone rather than guess.
+    static func expandLegacySettingsKeys(
+        _ keyPaths: [String],
+        pack: (any TechPack)?,
+        packID: String,
+        onDisk: Settings,
+        settingsFileName: String,
+        output: CLIOutput
+    ) -> [String] {
+        let objectKeys = Set(keyPaths.filter { key in
+            guard !key.contains("."), let data = onDisk.extraJSON[key] else { return false }
+            return (try? JSONSerialization.jsonObject(with: data)) is [String: Any]
+        })
+        guard !objectKeys.isEmpty else { return keyPaths }
+
+        // Merging into an empty value yields every path the pack declares: the most a sync could own.
+        var declared: [String: [String]] = [:]
+        for component in pack?.components ?? [] {
+            guard case let .settingsMerge(source) = component.installAction, let source else { continue }
+            do {
+                var scratch = Settings()
+                for keyPath in try scratch.merge(with: Settings.load(from: source)).landedKeyPaths {
+                    let topLevel = String(keyPath.split(separator: ".", maxSplits: 1)[0])
+                    if objectKeys.contains(topLevel), keyPath != topLevel {
+                        declared[topLevel, default: []].append(keyPath)
+                    }
+                }
+            } catch {
+                output.warn("Could not read \(source.lastPathComponent) from \(packID): \(error.localizedDescription)")
+            }
+        }
+
+        var expanded: [String] = []
+        for keyPath in keyPaths {
+            guard objectKeys.contains(keyPath) else {
+                expanded.append(keyPath)
+                continue
+            }
+            if let subKeys = declared[keyPath] {
+                expanded.append(contentsOf: subKeys)
+            } else {
+                output.warn(
+                    "\(packID): leaving '\(keyPath)' in \(settingsFileName) untouched — the pack's"
+                        + " entries in it can't be told apart from yours. Remove them by hand if they're stale."
+                )
+            }
+        }
+        return expanded
+    }
+
     /// Compute per-pack SHA-256 hashes of contributed settings values from the on-disk file.
     ///
     /// Reads the settings file once and hashes each pack's key-value pairs independently.

Sources/mcs/Sync/GlobalSyncStrategy.swift

@@ -211,7 +211,7 @@ struct GlobalSyncStrategy: SyncStrategy {
         }
 
         // Collect top-level keys to pass as dropKeys, preventing Layer 3 re-injection
-        let dropKeys = Set(allPreviousKeys.filter { !$0.contains(".") })
+        let dropKeys = Settings.topLevelKeys(of: allPreviousKeys)
 
         var (hasContent, contributedKeys) = ConfiguratorSupport.mergePackComponentsIntoSettings(
             packs: packs,

Sources/mcs/Sync/ProjectSyncStrategy.swift

@@ -142,9 +142,23 @@ struct ProjectSyncStrategy: SyncStrategy {
     ) throws -> (contributedKeys: [String: [String]], settingsHashes: [String: String]) {
         var settings = Settings()
 
-        // Collect top-level keys to prevent Layer 3 re-injection of stale extraJSON keys
         let allPreviousKeys = previousSettingsKeys.values.flatMap(\.self)
-        let dropKeys = Set(allPreviousKeys.filter { !$0.contains(".") })
+        let dropKeys = Settings.topLevelKeys(of: allPreviousKeys)
+        // Start from the existing extra keys minus what packs owned, so a pack value merges under
+        // the user's instead of replacing the whole object. Hooks and plugins are still rebuilt
+        // from the packs alone. Ownership stops one level down: an array such as
+        // `permissions.allow` is owned or skipped whole, so Claude Code's approvals in one a pack
+        // owns are still rewritten on sync.
+        do {
+            var existing = try Settings.load(from: scope.settingsPath)
+            existing.removeKeys(allPreviousKeys)
+            settings.extraJSON = existing.extraJSON
+        } catch {
+            output.warn(
+                "Could not parse \(scope.settingsPath.lastPathComponent): \(error.localizedDescription)"
+                    + " — its contents will be replaced"
+            )
+        }
 
         let (hasContent, contributedKeys) = ConfiguratorSupport.mergePackComponentsIntoSettings(
             packs: packs,
@@ -154,10 +168,12 @@ struct ProjectSyncStrategy: SyncStrategy {
             output: output
         )
 
-        if hasContent {
+        if hasContent || !settings.extraJSON.isEmpty {
             do {
                 try settings.save(to: scope.settingsPath, dropKeys: dropKeys)
-                output.success("Composed \(scope.settingsPath.lastPathComponent)")
+                if hasContent {
+                    output.success("Composed \(scope.settingsPath.lastPathComponent)")
+                }
             } catch {
                 output.error("Could not write \(scope.settingsPath.lastPathComponent): \(error.localizedDescription)")
                 output.error("Hooks and plugins will not be active. Re-run '\(scope.syncHint)' after fixing the issue.")