Fix two service-lifecycle defects found during the physical-HID proof

Scripts/lib/xcode.sh

@@ -3,8 +3,8 @@
 # Canonical local Xcode selection for KeyPath build, test, deploy, and release scripts.
 # Set KEYPATH_DEV_XCODE_DEVELOPER_DIR only when intentionally validating another toolchain.
 
-KEYPATH_STABLE_XCODE_VERSION="${KEYPATH_STABLE_XCODE_VERSION:-26.6}"
-KEYPATH_STABLE_XCODE_DEVELOPER_DIR="${KEYPATH_STABLE_XCODE_DEVELOPER_DIR:-/Applications/Xcode-26.6.app/Contents/Developer}"
+KEYPATH_STABLE_XCODE_VERSION="${KEYPATH_STABLE_XCODE_VERSION:-27.0}"
+KEYPATH_STABLE_XCODE_DEVELOPER_DIR="${KEYPATH_STABLE_XCODE_DEVELOPER_DIR:-/Applications/Xcode-27.app/Contents/Developer}"
 
 keypath_xcode_version() {
     local developer_dir="$1"

Sources/KeyPathAppKit/CLI/RulesFacade.swift

@@ -5,21 +5,64 @@ import KeyPathRulesCore
 public struct RulesFacade: Sendable {
     private let store: CustomRulesStore
     private let operation: CLIConfigurationOperation?
+    /// Loads the rule collections the generated config will be built from.
+    ///
+    /// Only the CLI-operation path supplies this. A custom rule can collide with a
+    /// key an *enabled collection* already claims (its mappings, or its momentary
+    /// layer activator), and `ConfigurationService.generateConfiguration` refuses to
+    /// generate at all in that case. Without this loader `addRule` only sees other
+    /// custom rules, so it happily persists a rule that every later `apply` will
+    /// reject — the rule is reported as created, never takes effect, and poisons
+    /// the store until it is removed by hand.
+    private let collectionLoader: (@Sendable () async -> [RuleCollection])?
 
     public init() {
         store = .shared
         operation = nil
+        collectionLoader = nil
     }
 
     init(operation: CLIConfigurationOperation) {
-        store = CustomRulesStore(fileURL: URL(fileURLWithPath: operation.directory)
-            .appendingPathComponent("CustomRules.json"))
+        let directory = URL(fileURLWithPath: operation.directory)
+        store = CustomRulesStore(fileURL: directory.appendingPathComponent("CustomRules.json"))
         self.operation = operation
+        let collectionStore = RuleCollectionStore(
+            fileURL: directory.appendingPathComponent("RuleCollections.json")
+        )
+        collectionLoader = { await collectionStore.loadCollections() }
     }
 
     init(store: CustomRulesStore) {
         self.store = store
         operation = nil
+        collectionLoader = nil
+    }
+
+    init(store: CustomRulesStore, collectionLoader: @escaping @Sendable () async -> [RuleCollection]) {
+        self.store = store
+        operation = nil
+        self.collectionLoader = collectionLoader
+    }
+
+    /// Conflicts that `rules` introduce against the enabled collections, relative to
+    /// `baselineRules`. Pre-existing conflicts between collections are subtracted so a
+    /// store that is already in a bad state does not block an unrelated rule.
+    private func collectionConflicts(
+        adding rules: [CustomRule], baseline baselineRules: [CustomRule]
+    ) async -> [KeyPathError.MappingConflictInfo] {
+        guard let collectionLoader else { return [] }
+        let collections = await collectionLoader()
+        // Custom rules first, exactly as generateConfiguration orders them.
+        let after = RuleCollectionDeduplicator.detectConflicts(
+            in: rules.asRuleCollections() + collections
+        )
+        guard !after.isEmpty else { return [] }
+        let before = Set(
+            RuleCollectionDeduplicator
+                .detectConflicts(in: baselineRules.asRuleCollections() + collections)
+                .map(\.description)
+        )
+        return after.filter { !before.contains($0.description) }
     }
 
     private func withMutation<Result: Sendable>(
@@ -102,6 +145,7 @@ public struct RulesFacade: Sendable {
     ) async throws -> RuleAddResult {
         try await withMutation {
             var rules = await store.loadRules()
+            let baseline = rules
             let existingIndex = rules.firstIndex(where: { $0.input == input })
 
             if let existingIndex {
@@ -116,6 +160,12 @@ public struct RulesFacade: Sendable {
                     let existing = rules[existingIndex]
                     let merged = try Self.mergeRules(existing: existing, newAction: action, newBehavior: behavior)
                     rules[existingIndex] = merged
+                    let conflicts = await collectionConflicts(adding: rules, baseline: baseline)
+                    if !conflicts.isEmpty {
+                        throw CLICollectionConflictError(
+                            input: input, conflicts: conflicts, ruleName: merged.displayTitle
+                        )
+                    }
                     try await store.saveRules(rules)
                     return .merged(CLIRuleDetail(from: merged))
                 }
@@ -138,6 +188,21 @@ public struct RulesFacade: Sendable {
                 deviceOverrides: deviceOverrides
             )
             rules.append(rule)
+
+            // A key an enabled collection already claims cannot be taken by a custom
+            // rule: config generation refuses the whole file rather than silently
+            // picking a winner. Refuse here, before anything is written, so the CLI
+            // never reports a rule as created that can never be applied.
+            let conflicts = await collectionConflicts(adding: rules, baseline: baseline)
+            if !conflicts.isEmpty {
+                if onConflict == .skip {
+                    return .skipped
+                }
+                throw CLICollectionConflictError(
+                    input: input, conflicts: conflicts, ruleName: rule.displayTitle
+                )
+            }
+
             try await store.saveRules(rules)
 
             let detail = CLIRuleDetail(from: rule)
@@ -162,7 +227,9 @@ public struct RulesFacade: Sendable {
             var rules = await store.loadRules()
             let before = rules.count
             rules.removeAll { $0.input == input }
-            if rules.count == before { return false }
+            if rules.count == before {
+                return false
+            }
             try await store.saveRules(rules)
             return true
         }
@@ -413,3 +480,38 @@ public struct CLIConflictError: Error, CustomStringConvertible {
         "Rule already exists for '\(input)'"
     }
 }
+
+/// The requested key is already claimed by an enabled rule collection (either one of
+/// its mappings or its momentary layer activator). Config generation refuses to build
+/// a file with two owners for one key, so the rule is rejected before it is persisted
+/// rather than written and then rejected by every subsequent `apply`.
+public struct CLICollectionConflictError: Error, CustomStringConvertible {
+    public let input: String
+    public let conflicts: [KeyPathError.MappingConflictInfo]
+    /// Display name the rejected rule would have had as a collection. Custom rules
+    /// appear in the conflict as a collection named after their display title, so it
+    /// is filtered out of `collectionNames` — the user does not need to be told their
+    /// own pending rule is one of the claimants.
+    public let ruleName: String
+
+    public init(input: String, conflicts: [KeyPathError.MappingConflictInfo], ruleName: String = "") {
+        self.input = input
+        self.conflicts = conflicts
+        self.ruleName = ruleName
+    }
+
+    /// Names of the collections claiming the key, excluding the pending custom rule.
+    public var collectionNames: [String] {
+        var seen = Set<String>()
+        return conflicts.flatMap(\.conflictingCollections)
+            .filter { $0 != ruleName && seen.insert($0).inserted }
+    }
+
+    public var explanation: String {
+        conflicts.map(\.userExplanation).joined(separator: "\n\n")
+    }
+
+    public var description: String {
+        "Key '\(input)' is already used by \(collectionNames.joined(separator: ", "))"
+    }
+}

Sources/KeyPathAppKit/Managers/HelperMaintenance.swift

@@ -31,7 +31,9 @@ public final class HelperMaintenance {
     /// by `runCleanupAndRepair` so UI callers don't surface them as error text.
     public var lastErrorLine: String? {
         for line in logLines.reversed() {
-            if line.hasPrefix("🧹 Cleanup & Repair") { continue }
+            if line.hasPrefix("🧹 Cleanup & Repair") {
+                continue
+            }
             if line.hasPrefix("❌") || line.hasPrefix("⚠️") {
                 return line
             }
@@ -248,8 +250,12 @@ public final class HelperMaintenance {
 
         paths = Array(Set(paths)) // unique
         paths.sort { lhs, rhs in
-            if lhs == "/Applications/KeyPath.app" { return true }
-            if rhs == "/Applications/KeyPath.app" { return false }
+            if lhs == "/Applications/KeyPath.app" {
+                return true
+            }
+            if rhs == "/Applications/KeyPath.app" {
+                return false
+            }
             return lhs < rhs
         }
         return paths
@@ -470,3 +476,11 @@ extension HelperMaintenance {
         testHooks = hooks
     }
 }
+
+/// Sendable is already required of this type: the wizard protocol it conforms to
+/// in WizardProtocolConformances.swift inherits Sendable, so the conformance has
+/// been in force all along. Swift 6 requires it to be declared alongside the
+/// class rather than in the conformance file, so state it here. This records the
+/// existing guarantee where the compiler wants it; it is not a new claim about
+/// this type's thread safety.
+extension HelperMaintenance: @unchecked Sendable {}

Sources/KeyPathAppKit/Managers/KanataDaemonManager.swift

@@ -208,7 +208,9 @@ public class KanataDaemonManager {
     /// - Returns: true if SMAppService reports `.enabled` OR launchctl has the job
     nonisolated func isInstalled() async -> Bool {
         let smStatus = await systemStateProvider.cachedSMAppServiceStatus(for: Self.kanataPlistName)
-        if smStatus == .enabled { return true }
+        if smStatus == .enabled {
+            return true
+        }
 
         let evidence = await systemStateProvider.launchctlPrint(target: "system/\(Self.kanataServiceID)")
         if evidence.exitCode == 0 {
@@ -795,3 +797,11 @@ private extension KanataDaemonManager {
         NotificationCenter.default.post(name: .smAppServiceApprovalRequired, object: nil)
     }
 }
+
+/// Sendable is already required of this type: the wizard protocol it conforms to
+/// in WizardProtocolConformances.swift inherits Sendable, so the conformance has
+/// been in force all along. Swift 6 requires it to be declared alongside the
+/// class rather than in the conformance file, so state it here. This records the
+/// existing guarantee where the compiler wants it; it is not a new claim about
+/// this type's thread safety.
+extension KanataDaemonManager: @unchecked Sendable {}

Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift

@@ -1229,9 +1229,15 @@ public class RuntimeCoordinator: SaveCoordinatorDelegate {
         isIntentionalTransition: Bool,
         isRecovering: Bool
     ) -> GrabRecoveryGate {
-        if active { return .recordSuccess }
-        if isIntentionalTransition { return .suppressedDuringTransition }
-        if isRecovering { return .suppressedRecoveryInFlight }
+        if active {
+            return .recordSuccess
+        }
+        if isIntentionalTransition {
+            return .suppressedDuringTransition
+        }
+        if isRecovering {
+            return .suppressedRecoveryInFlight
+        }
         return .evaluate
     }
 
@@ -1450,7 +1456,9 @@ public class RuntimeCoordinator: SaveCoordinatorDelegate {
         try await mutateAppKeymaps(store: store) { keymaps in
             guard let index = keymaps.firstIndex(where: { $0.mapping.bundleIdentifier == bundleIdentifier }) else { return }
             keymaps[index].overrides.removeAll { $0.id == overrideID }
-            if keymaps[index].overrides.isEmpty { keymaps.remove(at: index) }
+            if keymaps[index].overrides.isEmpty {
+                keymaps.remove(at: index)
+            }
         }
     }
 
@@ -1760,3 +1768,11 @@ struct ReloadResult {
         self.disposition = disposition ?? (success ? .applied : .failed)
     }
 }
+
+/// Sendable is already required of this type: the wizard protocol it conforms to
+/// in WizardProtocolConformances.swift inherits Sendable, so the conformance has
+/// been in force all along. Swift 6 requires it to be declared alongside the
+/// class rather than in the conformance file, so state it here. This records the
+/// existing guarantee where the compiler wants it; it is not a new claim about
+/// this type's thread safety.
+extension RuntimeCoordinator: @unchecked Sendable {}

Sources/KeyPathAppKit/Managers/UninstallCoordinator.swift

@@ -618,3 +618,11 @@ struct AppleScriptResult {
 
 // NOTE: AppleScriptRunner was removed - now using PrivilegedCommandRunner which respects
 // TestEnvironment.useSudoForPrivilegedOps for sudo-based execution in test environments.
+
+/// Sendable is already required of this type: the wizard protocol it conforms to
+/// in WizardProtocolConformances.swift inherits Sendable, so the conformance has
+/// been in force all along. Swift 6 requires it to be declared alongside the
+/// class rather than in the conformance file, so state it here. This records the
+/// existing guarantee where the compiler wants it; it is not a new claim about
+/// this type's thread safety.
+extension UninstallCoordinator: @unchecked Sendable {}

Sources/KeyPathAppKit/Services/Kanata/KanataDaemonService.swift

@@ -172,6 +172,28 @@ final class KanataDaemonService {
         return false
     }
 
+    /// SMAppService settles a fresh registration asynchronously, so a status
+    /// read taken immediately after `register()` can still report
+    /// `.notRegistered` and fail a start that would have succeeded a moment
+    /// later. Poll briefly before concluding the registration did not persist.
+    /// Status is slow synchronous IPC, so keep the attempt count small.
+    private func waitForRegistrationToSettle(
+        maxAttempts: Int = 6,
+        delayMilliseconds: Int = 250
+    ) async -> SMAppService.Status {
+        var status = await currentRegistrationStatus()
+        for attempt in 1 ... maxAttempts {
+            guard status == .notRegistered || status == .notFound else {
+                return status
+            }
+            if attempt < maxAttempts {
+                try? await Task.sleep(for: .milliseconds(delayMilliseconds))
+                status = await currentRegistrationStatus()
+            }
+        }
+        return status
+    }
+
     private enum StoppedPostcondition: Equatable {
         case satisfied
         case registrationPresent
@@ -258,7 +280,7 @@ final class KanataDaemonService {
             // Registration can race with an IPC error. The fresh status below
             // is authoritative, so do not fail before observing the result.
             try? await registerDaemon()
-            finalRegistrationStatus = await currentRegistrationStatus()
+            finalRegistrationStatus = await waitForRegistrationToSettle()
         @unknown default:
             throw KanataDaemonServiceError.startFailed(reason: "Unknown SMAppService registration state")
         }
@@ -343,11 +365,47 @@ final class KanataDaemonService {
     /// KeepAlive respawn.
     func restart() async throws {
         AppLogger.shared.log("🔄 [KanataDaemonService] Restart requested")
-        try await stop()
-        try await start()
+        do {
+            try await stop()
+            try await start()
+        } catch {
+            // `stop()` removes the SMAppService registration before it verifies
+            // its postcondition, and `start()` deliberately swallows a failed
+            // re-register. Either way a thrown restart can leave the daemon with
+            // no registration at all, which launchd reports as a missing service
+            // and which no command-line path recovers. Put the registration back
+            // before surfacing the failure so the caller is never left worse off
+            // than before the restart.
+            await restoreRegistrationAfterFailedRestart()
+            throw error
+        }
         AppLogger.shared.info("✅ [KanataDaemonService] Restart requested successfully")
     }
 
+    /// Re-register the daemon after a failed restart, but only when the
+    /// registration is actually gone. A registration that merely awaits approval
+    /// is left alone: re-registering cannot clear that state and would discard
+    /// the pending approval.
+    private func restoreRegistrationAfterFailedRestart() async {
+        let status = await currentRegistrationStatus()
+        guard status == .notRegistered || status == .notFound else { return }
+
+        AppLogger.shared.warn(
+            "⚠️ [KanataDaemonService] Restart failed with the daemon unregistered; restoring registration"
+        )
+        do {
+            try await registerDaemon()
+            AppLogger.shared.info(
+                "✅ [KanataDaemonService] Registration restored after failed restart"
+            )
+        } catch {
+            AppLogger.shared.error(
+                "❌ [KanataDaemonService] Could not restore registration after failed restart: "
+                    + error.localizedDescription
+            )
+        }
+    }
+
     /// Returns whether the internal recovery daemon is currently active.
     func isDaemonRunning() async -> Bool {
         let status = await refreshStatus()

Sources/KeyPathAppKit/Services/Permissions/PermissionRequestService.swift

@@ -175,3 +175,11 @@ public final class PermissionRequestService {
         return (im, ax)
     }
 }
+
+/// Sendable is already required of this type: the wizard protocol it conforms to
+/// in WizardProtocolConformances.swift inherits Sendable, so the conformance has
+/// been in force all along. Swift 6 requires it to be declared alongside the
+/// class rather than in the conformance file, so state it here. This records the
+/// existing guarantee where the compiler wants it; it is not a new claim about
+/// this type's thread safety.
+extension PermissionRequestService: @unchecked Sendable {}