From 12c93cc660d4e28efa7216b0e1c669caa67d6249 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Sun, 6 Sep 2026 08:50:35 -0700 Subject: [PATCH] Recover leader preferences with retained rule transactions --- Sources/KeyPathAppKit/CLI/ConfigFacade.swift | 114 ++++-- .../Config/ConfigurationService.swift | 125 +++++- .../Config/RecoverableRuleWrite.swift | 185 ++++++++- .../Managers/SaveCoordinator.swift | 8 +- .../Configuration/PreferencesService.swift | 38 +- .../Services/Packs/PackInstaller.swift | 4 +- .../RuleCollectionsManager+Bootstrap.swift | 22 +- .../RuleCollectionsManager+Mutation.swift | 37 +- .../RuleCollectionsManager+PublicAPI.swift | 41 +- .../RuleCollectionsManager.swift | 18 +- .../KeyPathTests/CLI/ConfigFacadeTests.swift | 88 +++- .../CollectionLeaderRecoveryTests.swift | 7 +- ...DurableConfigPreferenceRecoveryTests.swift | 387 ++++++++++++++++++ .../PackRuleTransactionTests.swift | 37 ++ .../configuration-save-pipeline.md | 50 ++- docs/bugs/leader-preference-crash-recovery.md | 36 ++ .../catalog-led-consolidation-plan.md | 94 ++++- 17 files changed, 1147 insertions(+), 144 deletions(-) create mode 100644 Tests/KeyPathTests/DurableConfigPreferenceRecoveryTests.swift create mode 100644 docs/bugs/leader-preference-crash-recovery.md diff --git a/Sources/KeyPathAppKit/CLI/ConfigFacade.swift b/Sources/KeyPathAppKit/CLI/ConfigFacade.swift index 7243d7a51..711b18c41 100644 --- a/Sources/KeyPathAppKit/CLI/ConfigFacade.swift +++ b/Sources/KeyPathAppKit/CLI/ConfigFacade.swift @@ -74,6 +74,17 @@ public struct ConfigFacade: Sendable { private func applyConfiguration( dryRun: Bool, service: ConfigurationService, permit: ConfigurationOperationGate.Permit ) async throws -> CLIApplyResult { + try await service.recoverPendingRuleWrite( + mutationPermit: permit, + preferenceDefaults: PreferencesService.canonicalDefaults + ) + _ = try await service.applyRecoveredRuntimeIfNeeded( + mutationPermit: permit, + reloadHandler: { await reloadResult() } + ) + await MainActor.run { + PreferencesService.shared.reloadLeaderKeyPreference() + } let collections = await ruleCollectionLoader() let customRules = await customRuleLoader() let enabledCount = collections.filter(\.isEnabled).count @@ -85,7 +96,7 @@ public struct ConfigFacade: Sendable { // neither in sync with the collection, so `keypath collection`/JSON edits to // `selectedOutput` were silently ignored by `keypath apply`. Reconcile both, mirroring // the in-process reconcile the app does on load (RuleCollectionsManager). See #889. - let reconcile = await applyReconciledLeaderKeyPreference(from: collections) + let reconcile = await reconciledLeaderKeyPreference(from: collections) let reconciledCollections = reconcile.map { r in LeaderKeyPreference.reconcileLeaderActivators( in: collections, @@ -93,24 +104,14 @@ public struct ConfigFacade: Sendable { targetLayer: r.reconciled.targetLayer ) } ?? collections - let leaderSnapshot = reconcile?.previous if dryRun { - // A dry run must not persist the reconciled preference. Generation reads the live - // shared preference, so we generate/validate against the reconciled value and then - // always restore the original β€” including when generation/validation throws - // (e.g. a mapping conflict) so a failed preview never leaves the store mutated. - do { - let previewConfig = try await service.generateConfiguration( - ruleCollections: reconciledCollections, - customRules: customRules - ) - try await validateDryRunConfig(previewConfig.content) - } catch { - await restoreLeaderKeyPreference(leaderSnapshot) - throw error - } - await restoreLeaderKeyPreference(leaderSnapshot) + let previewConfig = try await service.generateConfiguration( + ruleCollections: reconciledCollections, + customRules: customRules, + leaderKeyPreference: reconcile?.reconciled + ) + try await validateDryRunConfig(previewConfig.content) return CLIApplyResult( collectionsCount: collections.count, @@ -122,18 +123,32 @@ public struct ConfigFacade: Sendable { ) } - try await service.saveConfiguration( - ruleCollections: reconciledCollections, - customRules: customRules, - mutationPermit: permit - ) - - let reloadSuccess = if let reloadHandler { - await reloadHandler() + if let reconcile { + let preferenceChange = try RecoverableRuleWrite.PreferenceChange.leader( + before: reconcile.previousData, + after: JSONEncoder().encode(reconcile.reconciled) + ) + try await service.saveConfiguration( + ruleCollections: reconciledCollections, + customRules: customRules, + leaderKeyPreference: reconcile.reconciled, + preferenceDefaults: PreferencesService.canonicalDefaults, + preferenceChanges: [preferenceChange], + mutationPermit: permit + ) + await MainActor.run { + PreferencesService.shared.reloadLeaderKeyPreference() + } } else { - await tcpReload() + try await service.saveConfiguration( + ruleCollections: reconciledCollections, + customRules: customRules, + mutationPermit: permit + ) } + let reloadSuccess = await reloadRuntime() + return CLIApplyResult( collectionsCount: collections.count, enabledCount: enabledCount, @@ -143,35 +158,46 @@ public struct ConfigFacade: Sendable { ) } + private func reloadResult() async -> ReloadResult { + let success = await reloadRuntime() + return ReloadResult( + success: success, response: nil, + errorMessage: success ? nil : "CLI reload failed", + protocol: nil, disposition: success ? .applied : .failed + ) + } + + private func reloadRuntime() async -> Bool { + if let reloadHandler { return await reloadHandler() } + return await tcpReload() + } + private struct LeaderReconcile { let reconciled: LeaderKeyPreference - let previous: LeaderKeyPreference + let previousData: Data? } - /// Sync `PreferencesService.leaderKeyPreference` from the enabled Leader Key collection's - /// explicit `selectedOutput` (see `LeaderKeyPreference.reconciled`), returning both the - /// reconciled value (so the caller can rewrite the matching leader activators without - /// re-reading shared state) and the pre-reconcile `previous` (so a dry run can restore it). - /// On a real apply the caller keeps the reconciled value in place so it drives the generated - /// config and future reads, matching what the app does on load. Returns `nil` when nothing - /// changed. + /// Prepare a leader preference candidate without changing shared defaults. A real apply + /// journals this exact preimage with the generated config; a dry run uses only the candidate. @MainActor - private func applyReconciledLeaderKeyPreference(from collections: [RuleCollection]) -> LeaderReconcile? { - let current = PreferencesService.shared.leaderKeyPreference + private func reconciledLeaderKeyPreference(from collections: [RuleCollection]) -> LeaderReconcile? { + let defaults = PreferencesService.canonicalDefaults + let previousData = defaults.data(forKey: PreferencesService.leaderKeyPreferenceKey) + let current: LeaderKeyPreference = if let previousData { + (try? JSONDecoder().decode(LeaderKeyPreference.self, from: previousData)) ?? .default + } else { + .default + } guard let reconciled = LeaderKeyPreference.reconciled(from: collections, current: current) else { return nil } AppLogger.shared.log( "πŸ”‘ [ConfigFacade] Reconciling leader key from collection selectedOutput: '\(reconciled.key)' (was '\(current.key)')" ) - PreferencesService.shared.leaderKeyPreference = reconciled - return LeaderReconcile(reconciled: reconciled, previous: current) - } - - @MainActor - private func restoreLeaderKeyPreference(_ snapshot: LeaderKeyPreference?) { - guard let snapshot else { return } - PreferencesService.shared.leaderKeyPreference = snapshot + return LeaderReconcile( + reconciled: reconciled, + previousData: previousData + ) } // MARK: - Backup / Restore diff --git a/Sources/KeyPathAppKit/Infrastructure/Config/ConfigurationService.swift b/Sources/KeyPathAppKit/Infrastructure/Config/ConfigurationService.swift index f83f6e3a9..745fa4747 100644 --- a/Sources/KeyPathAppKit/Infrastructure/Config/ConfigurationService.swift +++ b/Sources/KeyPathAppKit/Infrastructure/Config/ConfigurationService.swift @@ -32,6 +32,7 @@ public final class ConfigurationService: FileConfigurationProviding { private let ruleCollectionStore: RuleCollectionStore private let customRulesStore: CustomRulesStore + private let synchronizePreferences: @Sendable (RecoverableRuleWrite.PreferenceDefaults) -> Bool @MainActor private var needsRecoveredRuntimeRefresh = false @MainActor private(set) var ruleRecoveryRevision: UInt64 = 0 @@ -58,10 +59,12 @@ public final class ConfigurationService: FileConfigurationProviding { init( configDirectory: String?, ruleCollectionStore: RuleCollectionStore, - customRulesStore: CustomRulesStore + customRulesStore: CustomRulesStore, + synchronizePreferences: @escaping @Sendable (RecoverableRuleWrite.PreferenceDefaults) -> Bool = { $0.value.synchronize() } ) { self.ruleCollectionStore = ruleCollectionStore self.customRulesStore = customRulesStore + self.synchronizePreferences = synchronizePreferences if let customDirectory = configDirectory { self.configDirectory = customDirectory } else { @@ -304,13 +307,57 @@ public final class ConfigurationService: FileConfigurationProviding { customRules: [CustomRule] = [], mutationPermit: ConfigurationOperationGate.Permit? ) async throws { - try await operationGate.withOperation(using: mutationPermit) { @MainActor [self] _ in - let newConfig = try await preparedConfiguration(ruleCollections: ruleCollections, customRules: customRules) + try await operationGate.withOperation(using: mutationPermit) { @MainActor [self] permit in + try await recoverPendingRuleWrite(mutationPermit: permit) + let newConfig = try await preparedConfiguration( + ruleCollections: ruleCollections, customRules: customRules, + leaderKeyPreference: persistedLeaderPreference(in: PreferencesService.canonicalDefaults) + ) try await writeFileAsync(string: newConfig.content, to: configurationPath) await publishSavedConfiguration(newConfig) } } + /// CLI reconciliation retains the generated config and its canonical leader preference + /// in one raw-config journal without rewriting the source collection stores. + func saveConfiguration( + ruleCollections: [RuleCollection], customRules: [CustomRule], + leaderKeyPreference: LeaderKeyPreference, + preferenceDefaults: UserDefaults, + preferenceChanges: [RecoverableRuleWrite.PreferenceChange], + mutationPermit: ConfigurationOperationGate.Permit + ) async throws { + try await operationGate.withOperation(using: mutationPermit) { @MainActor [self] permit in + try await recoverPendingRuleWrite(mutationPermit: permit, preferenceDefaults: preferenceDefaults) + let newConfig = try await preparedConfiguration( + ruleCollections: ruleCollections, + customRules: customRules, + leaderKeyPreference: leaderKeyPreference + ) + let files = ["config": URL(fileURLWithPath: configurationPath)] + let before = try await snapshotRuleFiles(files) + let sendablePreferences = RecoverableRuleWrite.PreferenceDefaults(preferenceDefaults) + let directory = URL(fileURLWithPath: configDirectory) + let pending = try await performRuleFileOperation { + try RecoverableRuleWrite.stage( + files: files, contents: ["config": Data(newConfig.content.utf8)], + directory: directory, scope: .rawConfig, expectedBefore: before, + preferences: sendablePreferences.value, preferenceChanges: preferenceChanges, + synchronizePreferences: self.synchronizePreferences + ) + } + do { + try await performRuleFileOperation { try RecoverableRuleWrite.commit(pending) } + } catch { + let cause = error + do { try await performRuleFileOperation { try RecoverableRuleWrite.rollback(pending) } } + catch { throw RecoverableRuleWrite.WriteFailure(cause: cause, recoveryError: error) } + throw cause + } + await publishSavedConfiguration(newConfig) + } + } + /// Persist the generated file and its two source stores as one recoverable /// write set. The manager supplies its actual stores, which can differ from /// this service's defaults in tests and isolated CLI workflows. @@ -348,10 +395,19 @@ public final class ConfigurationService: FileConfigurationProviding { ruleCollections: [RuleCollection], customRules: [CustomRule], collectionStore: RuleCollectionStore, customStore: CustomRulesStore, mutationPermit: ConfigurationOperationGate.Permit, - packRecord: InstalledPackTracker.RecordChange? = nil + packRecord: InstalledPackTracker.RecordChange? = nil, + preferenceDefaults: UserDefaults? = PreferencesService.canonicalDefaults, + preferenceChanges: [RecoverableRuleWrite.PreferenceChange] = [], + leaderKeyPreference: LeaderKeyPreference? = nil ) async throws -> RuleWrite { try await operationGate.withOperation(using: mutationPermit) { @MainActor [self] permit in - try await recoverPendingRuleWrite(collectionStore: collectionStore, customStore: customStore, mutationPermit: permit, installedPackTracker: packRecord?.tracker) + try await recoverPendingRuleWrite( + collectionStore: collectionStore, + customStore: customStore, + mutationPermit: permit, + installedPackTracker: packRecord?.tracker, + preferenceDefaults: preferenceDefaults + ) try await recoverPendingAppKeymapWrite(mutationPermit: permit) var targets = await ruleWriteFiles(collectionStore: collectionStore, customStore: customStore) if let packRecord { targets["installedPacks"] = await packRecord.tracker.persistenceURL } @@ -362,7 +418,10 @@ public final class ConfigurationService: FileConfigurationProviding { packUpdate = try await packRecord.tracker.prepareUpdate(packRecord) guard packUpdate?.before == before["installedPacks"] else { throw RecoverableRuleWrite.Failure.changedFile("installedPacks") } } else { packUpdate = nil } - let newConfig = try await preparedConfiguration(ruleCollections: ruleCollections, customRules: customRules) + let newConfig = try await preparedConfiguration( + ruleCollections: ruleCollections, customRules: customRules, + leaderKeyPreference: leaderKeyPreference ?? persistedLeaderPreference(in: preferenceDefaults) + ) var payload = try await [ "config": Data(newConfig.content.utf8), "collections": collectionStore.encodedCollections(ruleCollections), @@ -370,11 +429,14 @@ public final class ConfigurationService: FileConfigurationProviding { ] if let packUpdate { payload["installedPacks"] = packUpdate.contents } let contents = payload + let sendablePreferences = preferenceDefaults.map(RecoverableRuleWrite.PreferenceDefaults.init) try Task.checkCancellation() let directory = URL(fileURLWithPath: configDirectory) let pending = try await performRuleFileOperation { try RecoverableRuleWrite.stage(files: files, contents: contents, directory: directory, - scope: packUpdate == nil ? .rules : .packRules, expectedBefore: before) + scope: packUpdate == nil ? .rules : .packRules, expectedBefore: before, + preferences: sendablePreferences?.value, preferenceChanges: preferenceChanges, + synchronizePreferences: self.synchronizePreferences) { data, url in if let packUpdate, url == packUpdate.fileURL { try packUpdate.writeFile(data, url) } else { try RecoverableRuleWrite.durableWrite(data, url) } @@ -418,7 +480,8 @@ public final class ConfigurationService: FileConfigurationProviding { collectionStore: RuleCollectionStore? = nil, customStore: CustomRulesStore? = nil, mutationPermit: ConfigurationOperationGate.Permit? = nil, - installedPackTracker: InstalledPackTracker? = nil + installedPackTracker: InstalledPackTracker? = nil, + preferenceDefaults: UserDefaults? = PreferencesService.canonicalDefaults ) async throws -> Bool { try await operationGate.withOperation(using: mutationPermit) { @MainActor [self] _ in let files = await ruleWriteFiles(collectionStore: collectionStore ?? ruleCollectionStore, customStore: customStore ?? customRulesStore) @@ -431,10 +494,14 @@ public final class ConfigurationService: FileConfigurationProviding { } let packFiles = packTargets let rawFiles = ["config": URL(fileURLWithPath: configurationPath)] + let sendablePreferences = preferenceDefaults.map(RecoverableRuleWrite.PreferenceDefaults.init) let recovered = try await performRuleFileOperation { - let rawRecovered = try RecoverableRuleWrite.recover(files: rawFiles, directory: directory, scope: .rawConfig) - let packRecovered = try RecoverableRuleWrite.recover(files: packFiles, directory: directory, scope: .packRules) - let rulesRecovered = try RecoverableRuleWrite.recover(files: files, directory: directory) + let rawRecovered = try RecoverableRuleWrite.recover( + files: rawFiles, directory: directory, scope: .rawConfig, + preferences: sendablePreferences?.value + ) + let packRecovered = try RecoverableRuleWrite.recover(files: packFiles, directory: directory, scope: .packRules, preferences: sendablePreferences?.value) + let rulesRecovered = try RecoverableRuleWrite.recover(files: files, directory: directory, preferences: sendablePreferences?.value) return (raw: rawRecovered, rules: packRecovered || rulesRecovered) } if recovered.raw || recovered.rules { @@ -515,10 +582,14 @@ public final class ConfigurationService: FileConfigurationProviding { throw AppConfigError.validationFailed(errors: ["Rule collections could not be read completely; repair them before saving app-specific rules"]) } let rules = try await customRulesStore.loadForMutation() + let leaderKeyPreference = persistedLeaderPreference(in: PreferencesService.canonicalDefaults) // Refuse a lossy regeneration, including manual edits to a generated file. // A generated header alone does not prove that the visual editor owns it. let previousKeys = Set(previous.filter(\.mapping.isEnabled).flatMap { $0.overrides.map { $0.inputKey.lowercased() } }) - let expected = try await generateConfiguration(ruleCollections: collections.collections, customRules: rules, appSpecificKeys: previousKeys) + let expected = try await generateConfiguration( + ruleCollections: collections.collections, customRules: rules, + appSpecificKeys: previousKeys, leaderKeyPreference: leaderKeyPreference + ) for (name, content) in [("keypath.kbd", expected.content), ("keypath-apps.kbd", AppConfigGenerator.generate(from: previous))] { let url = URL(fileURLWithPath: configDirectory).appendingPathComponent(name) if FileManager.default.fileExists(atPath: url.path), try !AppConfigGenerator.matchesManagedContent(String(contentsOf: url, encoding: .utf8), expected: content) { @@ -528,7 +599,10 @@ public final class ConfigurationService: FileConfigurationProviding { ) } } - let configuration = try await generateConfiguration(ruleCollections: collections.collections, customRules: rules, appSpecificKeys: appKeys) + let configuration = try await generateConfiguration( + ruleCollections: collections.collections, customRules: rules, + appSpecificKeys: appKeys, leaderKeyPreference: leaderKeyPreference + ) // Validate the exact new include with the new main config before either // replaces its committed file. The engine receives the normal include. guard appKeys.isEmpty || configuration.content.contains("(include keypath-apps.kbd)") else { @@ -627,8 +701,14 @@ public final class ConfigurationService: FileConfigurationProviding { } } - private func preparedConfiguration(ruleCollections: [RuleCollection], customRules: [CustomRule]) async throws -> KanataConfiguration { - let newConfig = try await generateConfiguration(ruleCollections: ruleCollections, customRules: customRules) + private func preparedConfiguration( + ruleCollections: [RuleCollection], customRules: [CustomRule], + leaderKeyPreference: LeaderKeyPreference? = nil + ) async throws -> KanataConfiguration { + let newConfig = try await generateConfiguration( + ruleCollections: ruleCollections, customRules: customRules, + leaderKeyPreference: leaderKeyPreference + ) let validation = await validateConfiguration(newConfig.content) guard validation.isValid else { throw KeyPathError.configuration(.validationFailed(errors: validation.errors)) @@ -636,6 +716,15 @@ public final class ConfigurationService: FileConfigurationProviding { return newConfig } + @MainActor + private func persistedLeaderPreference(in defaults: UserDefaults?) -> LeaderKeyPreference? { + guard let defaults else { return nil } + guard let data = defaults.data(forKey: PreferencesService.leaderKeyPreferenceKey) else { + return .default + } + return (try? JSONDecoder().decode(LeaderKeyPreference.self, from: data)) ?? .default + } + private func publishSavedConfiguration(_ newConfig: KanataConfiguration) async { setCurrentConfiguration(newConfig) let snapshot = observersSnapshot() @@ -652,7 +741,8 @@ public final class ConfigurationService: FileConfigurationProviding { public func generateConfiguration( ruleCollections: [RuleCollection], customRules: [CustomRule] = [], - appSpecificKeys: Set? = nil + appSpecificKeys: Set? = nil, + leaderKeyPreference: LeaderKeyPreference? = nil ) async throws -> KanataConfiguration { // Custom rules come first so they take priority over preset collections let customRuleCollections = customRules.asRuleCollections() @@ -675,11 +765,12 @@ public final class ConfigurationService: FileConfigurationProviding { // Get leader key preference and trigger mode from PreferencesService on MainActor. // Fetched before conflict detection so the leader key participates in it (#463). - let (leaderKeyPref, triggerMode, holdDelayMs) = await MainActor.run { + let (storedLeaderKeyPref, triggerMode, holdDelayMs) = await MainActor.run { (PreferencesService.shared.leaderKeyPreference, PreferencesService.shared.contextHUDTriggerMode, PreferencesService.shared.contextHUDHoldDelayMs) } + let leaderKeyPref = leaderKeyPreference ?? storedLeaderKeyPref // DETECT CONFLICTS BEFORE DEDUPLICATION // This catches cases where multiple collections map the same key, and where diff --git a/Sources/KeyPathAppKit/Infrastructure/Config/RecoverableRuleWrite.swift b/Sources/KeyPathAppKit/Infrastructure/Config/RecoverableRuleWrite.swift index 531a4b9a2..22a56fd0a 100644 --- a/Sources/KeyPathAppKit/Infrastructure/Config/RecoverableRuleWrite.swift +++ b/Sources/KeyPathAppKit/Infrastructure/Config/RecoverableRuleWrite.swift @@ -27,6 +27,7 @@ enum RecoverableRuleWrite { fileprivate let directory: URL fileprivate let scope: Scope fileprivate let journal: Journal + fileprivate let preferences: PreferenceDefaults? } struct Entry: Codable, Equatable, Sendable { @@ -40,6 +41,61 @@ enum RecoverableRuleWrite { let version: Int var committed: Bool let entries: [Entry] + let preferences: [PreferenceEntry] + + init(version: Int, committed: Bool, entries: [Entry], preferences: [PreferenceEntry] = []) { + self.version = version + self.committed = committed + self.entries = entries + self.preferences = preferences + } + + private enum CodingKeys: String, CodingKey { case version, committed, entries, preferences } + + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + version = try container.decode(Int.self, forKey: .version) + committed = try container.decode(Bool.self, forKey: .committed) + entries = try container.decode([Entry].self, forKey: .entries) + preferences = try container.decodeIfPresent([PreferenceEntry].self, forKey: .preferences) ?? [] + } + } + + struct PreferenceEntry: Codable, Equatable, Sendable { + let role: PreferenceRole + let before: Data? + let after: Data + } + + struct PreferenceChange: Sendable { + let role: PreferenceRole + let before: Data? + let after: Data + + static func leader(before: Data?, after: Data) throws -> Self { + try .init( + role: .leader, + before: before.map(requiredPreferenceData), + after: requiredPreferenceData(after) + ) + } + } + + enum PreferenceRole: Codable, Hashable, Sendable { + case leader + + var key: String { + switch self { + case .leader: PreferencesService.leaderKeyPreferenceKey + } + } + } + + struct PreferenceDefaults: @unchecked Sendable { + let value: UserDefaults + init(_ value: UserDefaults) { + self.value = value + } } struct WriteFailure: LocalizedError { @@ -57,12 +113,14 @@ enum RecoverableRuleWrite { enum Failure: LocalizedError { case invalidJournal case changedFile(String) + case changedPreference(String) case systemCall(String, Int32) var errorDescription: String? { switch self { case .invalidJournal: "The pending rule-write journal does not match these rule stores." case let .changedFile(role): "The \(role) file changed outside this operation. Recovery was stopped to preserve it." + case let .changedPreference(key): "The \(key) preference changed outside this operation. Recovery was stopped to preserve it." case let .systemCall(operation, code): "Rule-write \(operation) failed (errno \(code))." } } @@ -80,19 +138,27 @@ enum RecoverableRuleWrite { static func stage( files: [String: URL], contents: [String: Data], directory: URL, scope: Scope, expectedBefore: [String: Data]? = nil, + preferences: UserDefaults? = nil, preferenceChanges: [PreferenceChange] = [], + synchronizePreferences: (PreferenceDefaults) -> Bool = { $0.value.synchronize() }, writeFile: (Data, URL) throws -> Void = durableWrite ) throws -> PendingWrite { try write(files: files, contents: contents, directory: directory, scope: scope, - commitImmediately: false, expectedBefore: expectedBefore, writeFile: writeFile) + commitImmediately: false, expectedBefore: expectedBefore, + preferences: preferences, preferenceChanges: preferenceChanges, + synchronizePreferences: synchronizePreferences, writeFile: writeFile) } private static func write( files: [String: URL], contents: [String: Data], directory: URL, scope: Scope, - commitImmediately: Bool, expectedBefore: [String: Data]? = nil, writeFile: (Data, URL) throws -> Void + commitImmediately: Bool, expectedBefore: [String: Data]? = nil, + preferences: UserDefaults? = nil, preferenceChanges: [PreferenceChange] = [], + synchronizePreferences: (PreferenceDefaults) -> Bool = { $0.value.synchronize() }, + writeFile: (Data, URL) throws -> Void ) throws -> PendingWrite { try validateFiles(files, directory: directory, scope: scope) return try withLock(directory: directory) { - try recoverLocked(files: files, directory: directory, scope: scope, writeFile: writeFile) + try recoverLocked(files: files, directory: directory, scope: scope, + preferences: preferences, writeFile: writeFile) if let expectedBefore { for (role, url) in files { guard try read(url) == expectedBefore[role] else { throw Failure.changedFile(role) } @@ -105,7 +171,20 @@ enum RecoverableRuleWrite { let entries = try files.keys.sorted().map { role in try Entry(role: role, path: files[role]!.standardizedFileURL.path, before: read(files[role]!), after: contents[role]!) } - var journal = Journal(version: 1, committed: false, entries: entries) + guard preferenceChanges.isEmpty || preferences != nil, + Set(preferenceChanges.map(\.role)).count == preferenceChanges.count + else { throw Failure.invalidJournal } + let preferenceEntries = try preferenceChanges.map { change in + guard try readPreference(change.role, from: preferences) == change.before else { + throw Failure.changedPreference(change.role.key) + } + return PreferenceEntry( + role: change.role, + before: change.before, + after: change.after + ) + } + var journal = Journal(version: 2, committed: false, entries: entries, preferences: preferenceEntries) try durableWrite(JSONEncoder().encode(journal), journalURL(directory, scope: scope)) var committing = false do { @@ -114,6 +193,20 @@ enum RecoverableRuleWrite { guard try read(url) == entry.before else { throw Failure.changedFile(entry.role) } try writeFile(entry.after, url) } + if let preferences, !preferenceEntries.isEmpty { + for entry in preferenceEntries { + guard try readPreference(entry.role, from: preferences) == entry.before else { + throw Failure.changedPreference(entry.role.key) + } + } + for change in preferenceChanges { + try writePreference(change.after, role: change.role, to: preferences) + } + guard synchronizePreferences(PreferenceDefaults(preferences)) else { + throw Failure.systemCall("preference synchronize", EIO) + } + try requireAfterPreferences(preferenceEntries, in: preferences) + } // A committed journal may safely be cleaned up at the next startup. if commitImmediately { committing = true @@ -129,7 +222,8 @@ enum RecoverableRuleWrite { journal.committed = false try durableWrite(JSONEncoder().encode(journal), journalURL(directory, scope: scope)) } - try recoverLocked(files: files, directory: directory, scope: scope, writeFile: writeFile) + try recoverLocked(files: files, directory: directory, scope: scope, + preferences: preferences, writeFile: writeFile) } catch { throw WriteFailure(cause: cause, recoveryError: error) } @@ -137,7 +231,10 @@ enum RecoverableRuleWrite { } // Persistence is committed even if journal cleanup must be retried. if commitImmediately { try? removeJournal(directory, scope: scope) } - return PendingWrite(files: files, directory: directory, scope: scope, journal: journal) + return PendingWrite( + files: files, directory: directory, scope: scope, journal: journal, + preferences: preferences.map(PreferenceDefaults.init) + ) } } @@ -149,6 +246,7 @@ enum RecoverableRuleWrite { throw Failure.changedFile(entry.role) } } + try requireAfterPreferences(pending.journal.preferences, in: pending.preferences?.value) var committed = pending.journal committed.committed = true let url = journalURL(pending.directory, scope: pending.scope) @@ -168,7 +266,7 @@ enum RecoverableRuleWrite { try withLock(directory: pending.directory) { try validatePending(pending) try recoverLocked(files: pending.files, directory: pending.directory, - scope: pending.scope, writeFile: durableWrite) + scope: pending.scope, preferences: pending.preferences?.value, writeFile: durableWrite) } } @@ -181,23 +279,26 @@ enum RecoverableRuleWrite { } @discardableResult - static func recover(files: [String: URL], directory: URL, scope: Scope = .rules) throws -> Bool { + static func recover(files: [String: URL], directory: URL, scope: Scope = .rules, + preferences: UserDefaults? = nil) throws -> Bool + { try validateFiles(files, directory: directory, scope: scope) return try withLock(directory: directory) { let hadJournal = try read(journalURL(directory, scope: scope)) != nil - try recoverLocked(files: files, directory: directory, scope: scope, writeFile: durableWrite) + try recoverLocked(files: files, directory: directory, scope: scope, + preferences: preferences, writeFile: durableWrite) return hadJournal } } private static func recoverLocked( - files: [String: URL], directory: URL, scope: Scope, + files: [String: URL], directory: URL, scope: Scope, preferences: UserDefaults? = nil, writeFile: (Data, URL) throws -> Void ) throws { let url = journalURL(directory, scope: scope) guard let data = try read(url) else { return } let journal = try JSONDecoder().decode(Journal.self, from: data) - guard journal.version == 1, + guard journal.version == 1 || journal.version == 2, journal.entries.count == files.count, Set(journal.entries.map(\.role)) == Set(files.keys), journal.entries.allSatisfy({ files[$0.role]?.standardizedFileURL.path == $0.path }), @@ -214,6 +315,7 @@ enum RecoverableRuleWrite { throw Failure.changedFile(entry.role) } } + try validatePreferences(journal.preferences, in: preferences) for entry in journal.entries { let target = files[entry.role]! let actual = try read(target) @@ -228,9 +330,70 @@ enum RecoverableRuleWrite { try syncDirectory(target.deletingLastPathComponent()) } } + if let preferences, !journal.preferences.isEmpty { + for entry in journal.preferences where try readPreference(entry.role, from: preferences) == entry.after { + if let before = entry.before { + try writePreference(before, role: entry.role, to: preferences) + } else { + preferences.removeObject(forKey: entry.role.key) + } + } + guard preferences.synchronize() else { throw Failure.systemCall("preference synchronize", EIO) } + for entry in journal.preferences { + let actual = try readPreference(entry.role, from: preferences) + guard actual == entry.before else { + throw Failure.changedPreference(entry.role.key) + } + } + } try removeJournal(directory, scope: scope) } + private static func validatePreferences(_ entries: [PreferenceEntry], in defaults: UserDefaults?) throws { + guard entries.isEmpty || defaults != nil else { throw Failure.invalidJournal } + guard let defaults else { return } + for entry in entries { + let actual = try readPreference(entry.role, from: defaults) + guard actual == entry.before || actual == entry.after else { + throw Failure.changedPreference(entry.role.key) + } + } + } + + private static func requireAfterPreferences(_ entries: [PreferenceEntry], in defaults: UserDefaults?) throws { + guard entries.isEmpty || defaults != nil else { throw Failure.invalidJournal } + guard let defaults else { return } + for entry in entries { + let actual = try readPreference(entry.role, from: defaults) + guard actual == entry.after else { + throw Failure.changedPreference(entry.role.key) + } + } + } + + private static func preferenceData(_ value: Any?) throws -> Data? { + guard let value else { return nil } + return try PropertyListSerialization.data(fromPropertyList: value, format: .binary, options: 0) + } + + private static func requiredPreferenceData(_ value: Any) throws -> Data { + guard let data = try preferenceData(value) else { throw Failure.invalidJournal } + return data + } + + private static func preferenceValue(_ data: Data) throws -> Any { + try PropertyListSerialization.propertyList(from: data, options: [], format: nil) + } + + private static func readPreference(_ role: PreferenceRole, from defaults: UserDefaults?) throws -> Data? { + guard let defaults else { return nil } + return try preferenceData(defaults.object(forKey: role.key)) + } + + private static func writePreference(_ data: Data, role: PreferenceRole, to defaults: UserDefaults) throws { + try defaults.set(preferenceValue(data), forKey: role.key) + } + static func journalURL(_ directory: URL, scope: Scope = .rules) -> URL { let name = switch scope { case .rules: ".keypath-rule-write.json" diff --git a/Sources/KeyPathAppKit/Managers/SaveCoordinator.swift b/Sources/KeyPathAppKit/Managers/SaveCoordinator.swift index e0999f0b6..fc2687227 100644 --- a/Sources/KeyPathAppKit/Managers/SaveCoordinator.swift +++ b/Sources/KeyPathAppKit/Managers/SaveCoordinator.swift @@ -235,6 +235,8 @@ final class SaveCoordinator { manager: RuleCollectionsManager, mutationPermit: ConfigurationOperationGate.Permit, packRecord: InstalledPackTracker.RecordChange? = nil, + preferenceChanges: [RecoverableRuleWrite.PreferenceChange] = [], + leaderKeyPreference: LeaderKeyPreference? = nil, reloadHandler: (() async -> ReloadResult)? ) async -> SaveResult { do { @@ -251,7 +253,10 @@ final class SaveCoordinator { staged = try await configurationService.stageRuleState( ruleCollections: manager.ruleCollections, customRules: manager.customRules, collectionStore: manager.ruleCollectionStore, customStore: manager.customRulesStore, - mutationPermit: permit, packRecord: packRecord + mutationPermit: permit, packRecord: packRecord, + preferenceDefaults: manager.preferencesService.persistenceDefaults, + preferenceChanges: preferenceChanges, + leaderKeyPreference: leaderKeyPreference ) try Task.checkCancellation() playWriteSound() @@ -312,6 +317,7 @@ final class SaveCoordinator { ) async throws -> Error? { try await configurationService.recoverPendingRuleWrite(mutationPermit: mutationPermit) try await configurationService.recoverPendingAppKeymapWrite(store: appStore, mutationPermit: mutationPermit) + PreferencesService.shared.reloadLeaderKeyPreference() do { let recovery = try await configurationService.applyRecoveredRuntimeIfNeeded(mutationPermit: mutationPermit, reloadHandler: reloadHandler) if recovery?.disposition == .applied { diff --git a/Sources/KeyPathAppKit/Services/Configuration/PreferencesService.swift b/Sources/KeyPathAppKit/Services/Configuration/PreferencesService.swift index caeb50fb1..5026cdb55 100644 --- a/Sources/KeyPathAppKit/Services/Configuration/PreferencesService.swift +++ b/Sources/KeyPathAppKit/Services/Configuration/PreferencesService.swift @@ -163,6 +163,20 @@ final class PreferencesService: @unchecked Sendable { @MainActor static let shared = PreferencesService() + static var canonicalDefaults: UserDefaults { + TestEnvironment.isRunningTests + ? .standard + : UserDefaults(suiteName: "com.keypath.KeyPath")! + } + + static let leaderKeyPreferenceKey = "KeyPath.LeaderKey.Preference" + private let leaderDefaults: UserDefaults + var persistenceDefaults: UserDefaults { + leaderDefaults + } + + private var suppressLeaderPersistence = false + // MARK: - Communication Protocol Configuration /// Communication protocol preference (always TCP) @@ -270,13 +284,28 @@ final class PreferencesService: @unchecked Sendable { /// This is independent of any collection - collections just target this layer var leaderKeyPreference: LeaderKeyPreference { didSet { + guard !suppressLeaderPersistence else { return } if let data = try? JSONEncoder().encode(leaderKeyPreference) { - UserDefaults.standard.set(data, forKey: Keys.leaderKeyPreference) + leaderDefaults.set(data, forKey: Keys.leaderKeyPreference) AppLogger.shared.log("🎯 [Preferences] Leader key: \(leaderKeyPreference.key) β†’ \(leaderKeyPreference.targetLayer.displayName) (enabled: \(leaderKeyPreference.enabled))") } } } + /// Update the generation candidate without creating a crash window before + /// ConfigurationService has durably journaled the preference transaction. + func stageLeaderKeyPreference(_ preference: LeaderKeyPreference) { + suppressLeaderPersistence = true + leaderKeyPreference = preference + suppressLeaderPersistence = false + } + + func reloadLeaderKeyPreference(from defaults: UserDefaults = PreferencesService.canonicalDefaults) { + let value = defaults.data(forKey: Self.leaderKeyPreferenceKey) + .flatMap { try? JSONDecoder().decode(LeaderKeyPreference.self, from: $0) } ?? .default + stageLeaderKeyPreference(value) + } + // MARK: - Education Hints /// How many times the "Overlay Hidden" education message has been shown @@ -402,7 +431,7 @@ final class PreferencesService: @unchecked Sendable { static let isSequenceMode = "KeyPath.Recording.IsSequenceMode" static let verboseKanataLogging = "KeyPath.Diagnostics.VerboseKanataLogging" static let accessibilityTestMode = "KeyPath.Testing.AccessibilityTestMode" - static let leaderKeyPreference = "KeyPath.LeaderKey.Preference" + static let leaderKeyPreference = PreferencesService.leaderKeyPreferenceKey static let contextHUDDisplayMode = "KeyPath.ContextHUD.DisplayMode" static let contextHUDTriggerMode = "KeyPath.ContextHUD.TriggerMode" static let contextHUDTimeout = "KeyPath.ContextHUD.Timeout" @@ -452,7 +481,8 @@ final class PreferencesService: @unchecked Sendable { // MARK: - Initialization - init() { + init(leaderDefaults: UserDefaults = PreferencesService.canonicalDefaults) { + self.leaderDefaults = leaderDefaults // The command-action feature was removed in #879. Clear its retired // preference so older installs do not retain a misleading security flag. UserDefaults.standard.removeObject(forKey: "KeyPath.Security.ConfigCommandActionsEnabled") @@ -520,7 +550,7 @@ final class PreferencesService: @unchecked Sendable { ?? Defaults.accessibilityTestMode // Leader key preference - if let data = UserDefaults.standard.data(forKey: Keys.leaderKeyPreference), + if let data = leaderDefaults.data(forKey: Keys.leaderKeyPreference), let stored = try? JSONDecoder().decode(LeaderKeyPreference.self, from: data) { leaderKeyPreference = stored diff --git a/Sources/KeyPathAppKit/Services/Packs/PackInstaller.swift b/Sources/KeyPathAppKit/Services/Packs/PackInstaller.swift index 7b6f9987e..4d160a594 100644 --- a/Sources/KeyPathAppKit/Services/Packs/PackInstaller.swift +++ b/Sources/KeyPathAppKit/Services/Packs/PackInstaller.swift @@ -67,8 +67,10 @@ public final class PackInstaller { try await manager.configurationService.recoverPendingAppKeymapWrite(mutationPermit: permit) let recovered = try await manager.configurationService.recoverPendingRuleWrite( collectionStore: manager.ruleCollectionStore, customStore: manager.customRulesStore, - mutationPermit: permit, installedPackTracker: tracker + mutationPermit: permit, installedPackTracker: tracker, + preferenceDefaults: manager.preferencesService.persistenceDefaults ) + manager.preferencesService.reloadLeaderKeyPreference(from: manager.preferencesService.persistenceDefaults) do { try await manager.refreshRecoveredRuleStateIfNeeded(recovered, mutationPermit: permit) } catch let error as KeyPathError { diff --git a/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+Bootstrap.swift b/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+Bootstrap.swift index fc86208bf..e66532d07 100644 --- a/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+Bootstrap.swift +++ b/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+Bootstrap.swift @@ -11,9 +11,11 @@ extension RuleCollectionsManager { await withRuleMutation(failure: ()) { [self] permit in do { try await configurationService.recoverPendingAppKeymapWrite(mutationPermit: permit) - try await configurationService.recoverPendingRuleWrite( - collectionStore: ruleCollectionStore, customStore: customRulesStore, mutationPermit: permit + _ = try await configurationService.recoverPendingRuleWrite( + collectionStore: ruleCollectionStore, customStore: customRulesStore, + mutationPermit: permit, preferenceDefaults: preferencesService.persistenceDefaults ) + preferencesService.reloadLeaderKeyPreference(from: preferencesService.persistenceDefaults) } catch { onError?("Could not recover the previous rule edit: \(error.localizedDescription)") return @@ -58,13 +60,19 @@ extension RuleCollectionsManager { dedupeRuleCollectionsInPlace() refreshLayerIndicatorState() - let leaderSnapshot = PreferencesService.shared.leaderKeyPreference - let collectionsSnapshot = ruleCollections + let leaderSnapshot = snapshotLeaderPreference() + let ruleSnapshot = snapshotRuleState() let didReconcileLeader = reconcileLeaderKeyFromCollection() - let applied = await regenerateConfigFromCollections(mutationPermit: permit) - if didReconcileLeader, !applied { - rollbackLeaderReconcile(preference: leaderSnapshot, collections: collectionsSnapshot) + if didReconcileLeader { + await commitRuleMutation( + snapshot: ruleSnapshot, + failureContext: "leader key", + leaderPreferenceBefore: leaderSnapshot, + mutationPermit: permit + ) + } else { + await regenerateConfigFromCollections(mutationPermit: permit) } } } diff --git a/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+Mutation.swift b/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+Mutation.swift index 706fe57ee..8eb931ae3 100644 --- a/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+Mutation.swift +++ b/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+Mutation.swift @@ -39,8 +39,12 @@ extension RuleCollectionsManager { func recoverRuleState(mutationPermit: ConfigurationOperationGate.Permit) async throws { try await configurationService.recoverPendingAppKeymapWrite(mutationPermit: mutationPermit) let recovered = try await configurationService.recoverPendingRuleWrite( - collectionStore: ruleCollectionStore, customStore: customRulesStore, mutationPermit: mutationPermit + collectionStore: ruleCollectionStore, customStore: customRulesStore, + mutationPermit: mutationPermit, preferenceDefaults: preferencesService.persistenceDefaults ) + if pendingLeaderKeyPreference == nil { + preferencesService.reloadLeaderKeyPreference(from: preferencesService.persistenceDefaults) + } try await refreshRecoveredRuleStateIfNeeded(recovered, mutationPermit: mutationPermit) } @@ -90,7 +94,7 @@ extension RuleCollectionsManager { /// Compatibility for editor callers that only need committed/not committed. @discardableResult func commitRuleMutation(snapshot: RuleStateSnapshot, skipReload: Bool = false, failureContext: String? = nil, - leaderPreferenceBefore: LeaderKeyPreference? = nil, + leaderPreferenceBefore: LeaderPreferenceSnapshot? = nil, keymapBefore: (id: String, includePunctuation: Bool)? = nil, mutationPermit: ConfigurationOperationGate.Permit) async -> Bool { @@ -102,24 +106,38 @@ extension RuleCollectionsManager { /// The save owner restores files/runtime; the manager restores its in-memory /// candidate without regenerating over the recovered or externally edited files. @discardableResult - func commitRuleMutationResult(snapshot: RuleStateSnapshot, skipReload: Bool = false, failureContext: String? = nil, leaderPreferenceBefore: LeaderKeyPreference? = nil, + func commitRuleMutationResult(snapshot: RuleStateSnapshot, skipReload: Bool = false, failureContext: String? = nil, leaderPreferenceBefore: LeaderPreferenceSnapshot? = nil, keymapBefore: (id: String, includePunctuation: Bool)? = nil, packRecord: InstalledPackTracker.RecordChange? = nil, mutationPermit: ConfigurationOperationGate.Permit) async -> SaveResult { - let preparedLeaderPreference = PreferencesService.shared.leaderKeyPreference + let preparedLeaderPreference = pendingLeaderKeyPreference ?? preferencesService.leaderKeyPreference let preparedKeymap = (id: activeKeymapId, includePunctuation: keymapIncludesPunctuation) + var preferenceChanges: [RecoverableRuleWrite.PreferenceChange] = [] + do { + if leaderPreferenceBefore != nil { + try preferenceChanges.append(.leader( + before: leaderPreferenceBefore?.data, + after: JSONEncoder().encode(preparedLeaderPreference) + )) + } + } catch { + pendingLeaderKeyPreference = nil + return .failure(error) + } let result = await SaveCoordinator(configurationService: configurationService).saveRuleState( - manager: self, mutationPermit: mutationPermit, packRecord: packRecord, reloadHandler: skipReload ? nil : onRulesChanged + manager: self, mutationPermit: mutationPermit, packRecord: packRecord, + preferenceChanges: preferenceChanges, + leaderKeyPreference: leaderPreferenceBefore == nil ? nil : preparedLeaderPreference, + reloadHandler: skipReload ? nil : onRulesChanged ) + pendingLeaderKeyPreference = nil guard result.success else { ruleCollections = snapshot.collections customRules = snapshot.customRules var preferenceRecoveryDetail = "" if let leaderPreferenceBefore { - if PreferencesService.shared.leaderKeyPreference == preparedLeaderPreference { - PreferencesService.shared.leaderKeyPreference = leaderPreferenceBefore - } else { + if preferencesService.leaderKeyPreference != leaderPreferenceBefore.value { preferenceRecoveryDetail = " A newer leader-key preference was preserved." } } @@ -140,6 +158,9 @@ extension RuleCollectionsManager { } return result } + if leaderPreferenceBefore != nil { + preferencesService.reloadLeaderKeyPreference(from: preferencesService.persistenceDefaults) + } NotificationCenter.default.post(name: .ruleCollectionsChanged, object: nil) return result } diff --git a/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+PublicAPI.swift b/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+PublicAPI.swift index 140ca5348..d336c2260 100644 --- a/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+PublicAPI.swift +++ b/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+PublicAPI.swift @@ -15,7 +15,7 @@ extension RuleCollectionsManager { func replaceCollections(_ collections: [RuleCollection]) async { await withRuleMutation(failure: ()) { [self] permit in guard let snapshot = await recoverAndSnapshotRuleState(mutationPermit: permit) else { return } - let leaderSnapshot = PreferencesService.shared.leaderKeyPreference + let leaderSnapshot = snapshotLeaderPreference() ruleCollections = RuleCollectionDeduplicator.dedupe(collections) dedupeRuleCollectionsInPlace() refreshLayerIndicatorState() @@ -82,7 +82,7 @@ extension RuleCollectionsManager { } } - let leaderPreferenceSnapshot = PreferencesService.shared.leaderKeyPreference + let leaderPreferenceSnapshot = snapshotLeaderPreference() let catalogMatch = RuleCollectionCatalog().defaultCollections().first { $0.id == id } AppLogger.shared.log("πŸ”€ [RuleCollections] catalogMatch=\(catalogMatch?.name ?? "nil")") @@ -174,7 +174,7 @@ extension RuleCollectionsManager { // Special handling: If Leader Key collection is toggled off, reset all momentary activators to default (space) if id == RuleCollectionIdentifier.leaderKey { if isEnabled { - let key = leaderKeyOutput(from: candidate) ?? leaderPreferenceSnapshot.key + let key = leaderKeyOutput(from: candidate) ?? leaderPreferenceSnapshot.value.key syncLeaderKeyPreference(key: key, enabled: true) } else { syncLeaderKeyPreference(enabled: false) @@ -368,7 +368,7 @@ extension RuleCollectionsManager { guard var candidate = ruleCollections.first(where: { $0.id == id }) ?? RuleCollectionCatalog().defaultCollections().first(where: { $0.id == id }) else { return } - let leaderSnapshot = PreferencesService.shared.leaderKeyPreference + let leaderSnapshot = snapshotLeaderPreference() candidate.configuration.updateSelectedOutput(output) candidate.isEnabled = true if let config = candidate.configuration.singleKeyPickerConfig, config.inputKey != "leader" { @@ -630,7 +630,7 @@ extension RuleCollectionsManager { await withRuleMutation(using: mutationPermit, failure: ()) { [self] permit in AppLogger.shared.log("πŸ”‘ [RuleCollections] Updating leader key to '\(newKey)'") guard let snapshot = await recoverAndSnapshotRuleState(mutationPermit: permit) else { return } - let leaderPreferenceSnapshot = PreferencesService.shared.leaderKeyPreference + let leaderPreferenceSnapshot = snapshotLeaderPreference() applyLeaderKeyToMomentaryActivators(newKey) syncLeaderKeyPreference(key: newKey, enabled: true) @@ -690,13 +690,14 @@ extension RuleCollectionsManager { return config.selectedOutput ?? config.presetOptions.first?.output } - private func syncLeaderKeyPreference(key: String? = nil, enabled: Bool) { - var preference = PreferencesService.shared.leaderKeyPreference + private func syncLeaderKeyPreference(key: String? = nil, enabled: Bool, persistImmediately: Bool = false) { + var preference = preferencesService.leaderKeyPreference if let key { preference.key = key } preference.enabled = enabled - PreferencesService.shared.leaderKeyPreference = preference + if persistImmediately { preferencesService.leaderKeyPreference = preference } + else { pendingLeaderKeyPreference = preference } } /// Reconcile the system `leaderKeyPreference` (and the baseβ†’nav leader activator) from @@ -726,12 +727,11 @@ extension RuleCollectionsManager { /// here would clobber a leader configured via the system-preference path while the /// collection is off, so full bidirectional reconciliation is deferred to the /// single-source-of-truth work in #865/#888. - /// - Returns: `true` if it mutated the preference/activators (so the caller must roll - /// back via `rollbackLeaderReconcile` if the subsequent config regen fails), `false` - /// if it was a no-op. + /// - Returns: `true` if it staged a preference/activator change for the caller's rule + /// transaction, `false` if it was a no-op. @discardableResult - func reconcileLeaderKeyFromCollection() -> Bool { - let current = PreferencesService.shared.leaderKeyPreference + func reconcileLeaderKeyFromCollection(persistImmediately: Bool = false) -> Bool { + let current = pendingLeaderKeyPreference ?? preferencesService.leaderKeyPreference // Shared, pure reconcile rule β€” same statement the CLI apply path uses // (ConfigFacade), so all headless paths agree. See LeaderKeyPreference.reconciled. guard let reconciled = LeaderKeyPreference.reconciled(from: ruleCollections, current: current) else { @@ -742,23 +742,10 @@ extension RuleCollectionsManager { "πŸ”‘ [RuleCollections] Reconciling leader key from collection selectedOutput: '\(reconciled.key)' (was '\(current.key)', enabled=\(current.enabled))" ) applyLeaderKeyToLeaderActivators(reconciled.key, targetLayer: current.targetLayer) - syncLeaderKeyPreference(key: reconciled.key, enabled: true) + syncLeaderKeyPreference(key: reconciled.key, enabled: true, persistImmediately: persistImmediately) return true } - /// Silently undo a reconcile whose config regen failed. `leaderKeyPreference` is - /// UserDefaults-persisted via `didSet`, so leaving a reconciled-but-unapplied value in - /// place would permanently mask the drift (the idempotency guard in - /// `reconcileLeaderKeyFromCollection` would see the preference already matches and skip - /// retrying). Restoring both the preference and the in-memory collections lets the next - /// load retry cleanly. Unlike `updateLeaderKey`'s rollback this shows no user message β€” - /// it runs on passive `bootstrap`/`replaceCollections` load paths. See issue #889. - func rollbackLeaderReconcile(preference: LeaderKeyPreference, collections: [RuleCollection]) { - AppLogger.shared.log("↩️ [RuleCollections] Leader reconcile rolled back after failed config regen") - ruleCollections = collections - PreferencesService.shared.leaderKeyPreference = preference - } - /// Save or update a custom rule /// - Parameters: /// - autoResolveConflicts: When true, automatically wins over conflicting rules without prompting. diff --git a/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager.swift b/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager.swift index 0bc3fbec5..cc68b21d0 100644 --- a/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager.swift +++ b/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager.swift @@ -80,6 +80,20 @@ final class RuleCollectionsManager { let configurationService: ConfigurationService let eventListener: KanataEventListener let keymapPreferences: UserDefaults + let preferencesService: PreferencesService + var pendingLeaderKeyPreference: LeaderKeyPreference? + + struct LeaderPreferenceSnapshot: Sendable { + let value: LeaderKeyPreference + let data: Data? + } + + func snapshotLeaderPreference() -> LeaderPreferenceSnapshot { + LeaderPreferenceSnapshot( + value: preferencesService.leaderKeyPreference, + data: preferencesService.persistenceDefaults.data(forKey: PreferencesService.leaderKeyPreferenceKey) + ) + } /// Apply already-persisted rules and retain the runtime disposition. var onRulesChanged: (() async -> ReloadResult)? @@ -127,13 +141,15 @@ final class RuleCollectionsManager { customRulesStore: CustomRulesStore = .shared, configurationService: ConfigurationService, eventListener: KanataEventListener = KanataEventListener(), - keymapPreferences: UserDefaults = .standard + keymapPreferences: UserDefaults = .standard, + preferencesService: PreferencesService? = nil ) { self.ruleCollectionStore = ruleCollectionStore self.customRulesStore = customRulesStore self.configurationService = configurationService self.eventListener = eventListener self.keymapPreferences = keymapPreferences + self.preferencesService = preferencesService ?? PreferencesService.shared } deinit { diff --git a/Tests/KeyPathTests/CLI/ConfigFacadeTests.swift b/Tests/KeyPathTests/CLI/ConfigFacadeTests.swift index 30bf2e79f..cf3337361 100644 --- a/Tests/KeyPathTests/CLI/ConfigFacadeTests.swift +++ b/Tests/KeyPathTests/CLI/ConfigFacadeTests.swift @@ -163,8 +163,9 @@ final class ConfigFacadeTests: XCTestCase { XCTAssertEqual(reloadCount, 0) XCTAssertEqual(try String(contentsOf: configFile, encoding: .utf8), "original config") - let activeItems = try FileManager.default.contentsOfDirectory(atPath: configDirectory.path) - XCTAssertEqual(activeItems, ["keypath.kbd"]) + XCTAssertFalse(FileManager.default.fileExists( + atPath: RecoverableRuleWrite.journalURL(configDirectory, scope: .rawConfig).path + )) } // MARK: - #889: CLI apply must honor the Leader Key collection's selectedOutput @@ -263,6 +264,89 @@ final class ConfigFacadeTests: XCTestCase { ) } + @MainActor + func testFailedApplyDoesNotPersistReconciledLeaderKey() async throws { + let blockedDirectory = tempRoot.appendingPathComponent("blocked") + try Data("not-a-directory".utf8).write(to: blockedDirectory) + PreferencesService.shared.leaderKeyPreference = .default + defer { PreferencesService.shared.leaderKeyPreference = .default } + + let collections = leaderKeyCollections(selectedOutput: "tab") + let facade = ConfigFacade( + configDirectory: blockedDirectory.path, + ruleCollectionLoader: { collections }, + customRuleLoader: { [] }, + reloadHandler: { true } + ) + + do { + _ = try await facade.applyConfiguration(dryRun: false) + XCTFail("Expected CLI apply to fail before committing its retained transaction") + } catch {} + + XCTAssertEqual(PreferencesService.shared.leaderKeyPreference, .default) + XCTAssertFalse( + FileManager.default.fileExists(atPath: RecoverableRuleWrite.journalURL(blockedDirectory, scope: .rawConfig).path) + ) + } + + @MainActor + func testApplyRecoversInterruptedRawLeaderTransactionBeforeReconciling() async throws { + let configDirectory = tempRoot.appendingPathComponent("config", isDirectory: true) + try FileManager.default.createDirectory(at: configDirectory, withIntermediateDirectories: true) + let config = configDirectory.appendingPathComponent("keypath.kbd") + try RecoverableRuleWrite.durableWrite(Data("before-config".utf8), config) + PreferencesService.shared.leaderKeyPreference = .default + defer { PreferencesService.shared.leaderKeyPreference = .default } + let beforeData = try XCTUnwrap( + PreferencesService.canonicalDefaults.data(forKey: PreferencesService.leaderKeyPreferenceKey) + ) + let attempted = LeaderKeyPreference(key: "tab", targetLayer: .navigation, enabled: true) + _ = try RecoverableRuleWrite.stage( + files: ["config": config], contents: ["config": Data("interrupted-config".utf8)], + directory: configDirectory, scope: .rawConfig, + preferences: PreferencesService.canonicalDefaults, + preferenceChanges: [.leader(before: beforeData, after: JSONEncoder().encode(attempted))] + ) + + let collections = leaderKeyCollections(selectedOutput: "tab") + let reloadProbe = ReloadProbe() + let facade = ConfigFacade( + configDirectory: configDirectory.path, + ruleCollectionLoader: { collections }, customRuleLoader: { [] }, + reloadHandler: { await reloadProbe.reload() } + ) + _ = try await facade.applyConfiguration() + + let reloadCount = await reloadProbe.count + XCTAssertEqual(reloadCount, 2) + XCTAssertEqual(PreferencesService.shared.leaderKeyPreference, attempted) + XCTAssertFalse(FileManager.default.fileExists( + atPath: RecoverableRuleWrite.journalURL(configDirectory, scope: .rawConfig).path + )) + XCTAssertTrue(try String(contentsOf: config, encoding: .utf8).contains(";; Input: tab")) + } + + @MainActor + func testApplyReconcilesMalformedStoredLeaderFromDefaultFallback() async throws { + let configDirectory = tempRoot.appendingPathComponent("config", isDirectory: true) + try FileManager.default.createDirectory(at: configDirectory, withIntermediateDirectories: true) + PreferencesService.canonicalDefaults.set( + Data("malformed".utf8), forKey: PreferencesService.leaderKeyPreferenceKey + ) + PreferencesService.shared.reloadLeaderKeyPreference() + defer { PreferencesService.shared.leaderKeyPreference = .default } + + let collections = leaderKeyCollections(selectedOutput: "tab") + let facade = ConfigFacade( + configDirectory: configDirectory.path, + ruleCollectionLoader: { collections }, customRuleLoader: { [] }, reloadHandler: { true } + ) + _ = try await facade.applyConfiguration() + + XCTAssertEqual(PreferencesService.shared.leaderKeyPreference.key, "tab") + } + private func createSymlinkedConfig(link: URL, target: URL) throws { try FileManager.default.createDirectory(at: target, withIntermediateDirectories: true) try FileManager.default.createDirectory( diff --git a/Tests/KeyPathTests/CollectionLeaderRecoveryTests.swift b/Tests/KeyPathTests/CollectionLeaderRecoveryTests.swift index b701d55ca..7a44bfa66 100644 --- a/Tests/KeyPathTests/CollectionLeaderRecoveryTests.swift +++ b/Tests/KeyPathTests/CollectionLeaderRecoveryTests.swift @@ -121,7 +121,7 @@ final class CollectionLeaderRecoveryTests: KeyPathTestCase { XCTAssertEqual(leader.configuration.singleKeyPickerConfig?.selectedOutput, "f18") } - func testFailedLeaderEditPreservesNewerPreference() async throws { + func testFailedLeaderEditWithThirdPreferenceRevisionFailsClosed() async throws { let before = try files() var reloads = 0 var errors: [String] = [] @@ -134,9 +134,10 @@ final class CollectionLeaderRecoveryTests: KeyPathTestCase { return Self.reload(reloads == 1 ? .rejected : .applied) } await manager.updateLeaderKey("f18") - XCTAssertEqual(reloads, 2) - XCTAssertEqual(try files(), before) + XCTAssertEqual(reloads, 1, "A preference conflict must not reload a partially recovered revision") + XCTAssertNotEqual(try files(), before, "Fail-closed recovery must preserve the attempted files for diagnosis") XCTAssertEqual(PreferencesService.shared.leaderKeyPreference.key, "f17") + XCTAssertTrue(FileManager.default.fileExists(atPath: RecoverableRuleWrite.journalURL(directory).path)) XCTAssertTrue(errors.first?.contains("newer leader-key preference was preserved") == true) } } diff --git a/Tests/KeyPathTests/DurableConfigPreferenceRecoveryTests.swift b/Tests/KeyPathTests/DurableConfigPreferenceRecoveryTests.swift new file mode 100644 index 000000000..61581cb47 --- /dev/null +++ b/Tests/KeyPathTests/DurableConfigPreferenceRecoveryTests.swift @@ -0,0 +1,387 @@ +import Foundation +@testable import KeyPathAppKit +import KeyPathCore +import KeyPathRulesCore +@preconcurrency import XCTest + +@MainActor +final class DurableConfigPreferenceRecoveryTests: XCTestCase { + private var directory: URL! + private var defaults: UserDefaults! + private var suiteName: String! + + override func setUp() async throws { + TestEnvironment.forceTestMode = true + directory = FileManager.default.temporaryDirectory + .appendingPathComponent("DurableConfigPreferenceRecovery-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + suiteName = "DurableConfigPreferenceRecovery.\(UUID().uuidString)" + defaults = try XCTUnwrap(UserDefaults(suiteName: suiteName)) + } + + override func tearDown() async throws { + TestEnvironment.forceTestMode = false + defaults.removePersistentDomain(forName: suiteName) + try? FileManager.default.removeItem(at: directory) + } + + func testInterruptedWriteRestoresLeaderFromAnotherDefaultsInstance() throws { + let files = try initialFiles() + let before = LeaderKeyPreference.default + let after = LeaderKeyPreference(key: "f18", targetLayer: .navigation, enabled: true) + try defaults.set(JSONEncoder().encode(before), forKey: PreferencesService.leaderKeyPreferenceKey) + _ = try stage(files: files, leader: after) + + let recoveryDefaults = try XCTUnwrap(UserDefaults(suiteName: suiteName)) + XCTAssertTrue(try RecoverableRuleWrite.recover( + files: files, directory: directory, preferences: recoveryDefaults + )) + XCTAssertEqual(try storedLeader(recoveryDefaults), before) + XCTAssertEqual(try Data(contentsOf: files["config"]!), Data("before-config".utf8)) + XCTAssertFalse(FileManager.default.fileExists(atPath: RecoverableRuleWrite.journalURL(directory).path)) + } + + func testInterruptedCLIRawWriteRestoresCanonicalLeaderAndConfig() throws { + let config = directory.appendingPathComponent("keypath.kbd") + try RecoverableRuleWrite.durableWrite(Data("before-config".utf8), config) + let before = LeaderKeyPreference.default + let after = LeaderKeyPreference(key: "tab", targetLayer: .navigation, enabled: true) + let beforeData = try JSONEncoder().encode(before) + defaults.set(beforeData, forKey: PreferencesService.leaderKeyPreferenceKey) + + _ = try RecoverableRuleWrite.stage( + files: ["config": config], contents: ["config": Data("after-config".utf8)], + directory: directory, scope: .rawConfig, preferences: defaults, + preferenceChanges: [.leader(before: beforeData, after: JSONEncoder().encode(after))] + ) + + let recoveryDefaults = try XCTUnwrap(UserDefaults(suiteName: suiteName)) + XCTAssertTrue(try RecoverableRuleWrite.recover( + files: ["config": config], directory: directory, scope: .rawConfig, + preferences: recoveryDefaults + )) + XCTAssertEqual(try storedLeader(recoveryDefaults), before) + XCTAssertEqual(try Data(contentsOf: config), Data("before-config".utf8)) + XCTAssertFalse(FileManager.default.fileExists( + atPath: RecoverableRuleWrite.journalURL(directory, scope: .rawConfig).path + )) + } + + func testTrackedLeaderConflictLeavesFilesAndJournalForDiagnosis() throws { + let files = try initialFiles() + try defaults.set(JSONEncoder().encode(LeaderKeyPreference.default), + forKey: PreferencesService.leaderKeyPreferenceKey) + _ = try stage( + files: files, + leader: LeaderKeyPreference(key: "f18", targetLayer: .navigation, enabled: true) + ) + try defaults.set( + JSONEncoder().encode(LeaderKeyPreference(key: "f17", targetLayer: .navigation, enabled: true)), + forKey: PreferencesService.leaderKeyPreferenceKey + ) + + XCTAssertThrowsError( + try RecoverableRuleWrite.recover(files: files, directory: directory, preferences: defaults) + ) { error in + XCTAssertTrue(error.localizedDescription.contains(PreferencesService.leaderKeyPreferenceKey)) + } + XCTAssertEqual(try Data(contentsOf: files["config"]!), Data("after-config".utf8)) + XCTAssertTrue(FileManager.default.fileExists(atPath: RecoverableRuleWrite.journalURL(directory).path)) + } + + func testPreferenceChangeDuringFileWritesIsNotOverwritten() throws { + let files = try initialFiles() + let before = LeaderKeyPreference.default + let attempted = LeaderKeyPreference(key: "f18", targetLayer: .navigation, enabled: true) + let newer = LeaderKeyPreference(key: "f17", targetLayer: .navigation, enabled: true) + try defaults.set(JSONEncoder().encode(before), forKey: PreferencesService.leaderKeyPreferenceKey) + var writes = 0 + + XCTAssertThrowsError(try RecoverableRuleWrite.stage( + files: files, + contents: Dictionary(uniqueKeysWithValues: files.map { ($0.key, Data("after-\($0.key)".utf8)) }), + directory: directory, + scope: .rules, + preferences: defaults, + preferenceChanges: [.leader( + before: defaults.data(forKey: PreferencesService.leaderKeyPreferenceKey), + after: JSONEncoder().encode(attempted) + )], + writeFile: { data, url in + writes += 1 + try RecoverableRuleWrite.durableWrite(data, url) + if writes == 1 { + try self.defaults.set(JSONEncoder().encode(newer), + forKey: PreferencesService.leaderKeyPreferenceKey) + } + } + )) + XCTAssertEqual(try storedLeader(defaults), newer) + XCTAssertTrue(FileManager.default.fileExists(atPath: RecoverableRuleWrite.journalURL(directory).path)) + } + + func testCommittedJournalCleanupDoesNotRejectNewerLeader() throws { + let files = try initialFiles() + let committed = LeaderKeyPreference(key: "f18", targetLayer: .navigation, enabled: true) + let newer = LeaderKeyPreference(key: "f17", targetLayer: .navigation, enabled: true) + _ = try stage(files: files, leader: committed) + let pendingData = try Data(contentsOf: RecoverableRuleWrite.journalURL(directory)) + var journal = try JSONDecoder().decode(RecoverableRuleWrite.Journal.self, from: pendingData) + journal.committed = true + try RecoverableRuleWrite.durableWrite( + JSONEncoder().encode(journal), RecoverableRuleWrite.journalURL(directory) + ) + try defaults.set(JSONEncoder().encode(newer), forKey: PreferencesService.leaderKeyPreferenceKey) + + XCTAssertTrue(try RecoverableRuleWrite.recover(files: files, directory: directory, preferences: defaults)) + XCTAssertEqual(try storedLeader(defaults), newer) + XCTAssertFalse(FileManager.default.fileExists(atPath: RecoverableRuleWrite.journalURL(directory).path)) + } + + @MainActor + func testLeaderCandidateIsNotPersistedWhenStagingFailsBeforeJournal() async throws { + let blocked = directory.appendingPathComponent("blocked") + try Data("not-a-directory".utf8).write(to: blocked) + let collections = RuleCollectionStore.testStore(at: directory.appendingPathComponent("RuleCollections.json")) + let rules = CustomRulesStore.testStore(at: directory.appendingPathComponent("CustomRules.json")) + let service = ConfigurationService(configDirectory: blocked.path, ruleCollectionStore: collections, customRulesStore: rules) + let preferences = PreferencesService(leaderDefaults: defaults) + preferences.leaderKeyPreference = .default + let manager = RuleCollectionsManager( + ruleCollectionStore: collections, customRulesStore: rules, + configurationService: service, keymapPreferences: defaults, + preferencesService: preferences + ) + manager.ruleCollections = RuleCollectionCatalog().defaultCollections() + + await manager.updateLeaderKey("f18") + + XCTAssertEqual(try storedLeader(defaults), .default) + XCTAssertFalse(FileManager.default.fileExists(atPath: RecoverableRuleWrite.journalURL(blocked).path)) + } + + @MainActor + func testManagerCommitsAppliedAndPendingLeaderButRestoresRejectedLeader() async throws { + for disposition: ReloadDisposition in [.applied, .pending, .rejected] { + let caseDirectory = directory.appendingPathComponent(String(describing: disposition)) + let (manager, _) = try makeManager(at: caseDirectory) + try preferencesReset(to: .default) + manager.ruleCollections = try leaderCollections() + var reloadCount = 0 + var errors: [String] = [] + manager.onError = { errors.append($0) } + manager.onRulesChanged = { + reloadCount += 1 + return ReloadResult( + success: disposition == .applied, + response: nil, + errorMessage: disposition == .rejected ? "rejected" : nil, + protocol: nil, + disposition: disposition + ) + } + + await manager.updateLeaderKey("tab") + + let expected = disposition == .rejected + ? LeaderKeyPreference.default + : LeaderKeyPreference(key: "tab", targetLayer: .navigation, enabled: true) + XCTAssertEqual(try storedLeader(defaults), expected, "Unexpected durable leader for \(disposition)") + XCTAssertEqual(reloadCount, disposition == .rejected ? 2 : 1, + "Applied/pending should reload once; rejection should reload the restored revision once") + XCTAssertEqual(errors.isEmpty, disposition != .rejected) + XCTAssertFalse(FileManager.default.fileExists(atPath: RecoverableRuleWrite.journalURL(caseDirectory).path)) + } + } + + @MainActor + func testManagerRestoresLeaderWhenDurabilityBarrierFails() async throws { + let barrierCalls = LockedCounter() + let (manager, _) = try makeManager(at: directory, synchronizePreferences: { _ in + barrierCalls.increment() + return false + }) + try preferencesReset(to: .default) + manager.ruleCollections = try leaderCollections() + var reloadCount = 0 + var errors: [String] = [] + manager.onError = { errors.append($0) } + manager.onRulesChanged = { + reloadCount += 1 + return ReloadResult(success: true, response: nil, errorMessage: nil, protocol: nil, disposition: .applied) + } + + await manager.updateLeaderKey("tab") + + XCTAssertEqual(try storedLeader(defaults), .default) + XCTAssertEqual(barrierCalls.value, 1) + XCTAssertEqual(reloadCount, 0) + XCTAssertFalse(errors.isEmpty) + XCTAssertFalse(FileManager.default.fileExists(atPath: RecoverableRuleWrite.journalURL(directory).path)) + } + + @MainActor + func testRootMutationRefreshesLeaderCommittedByAnotherServiceBeforeRollback() async throws { + let (manager, _) = try makeManager(at: directory) + try preferencesReset(to: .default) + manager.ruleCollections = try leaderCollections() + let newer = LeaderKeyPreference(key: "f17", targetLayer: .navigation, enabled: true) + try defaults.set(JSONEncoder().encode(newer), forKey: PreferencesService.leaderKeyPreferenceKey) + manager.onRulesChanged = { + ReloadResult(success: false, response: nil, errorMessage: "rejected", protocol: nil, disposition: .rejected) + } + + await manager.updateLeaderKey("tab") + + XCTAssertEqual(try storedLeader(defaults), newer) + } + + @MainActor + func testManagerRejectsPreferenceChangedAfterSnapshotBeforeJournal() async throws { + let (manager, _) = try makeManager(at: directory) + try preferencesReset(to: .default) + manager.ruleCollections = try leaderCollections() + let beforeFiles = ruleFiles(at: directory) + let intervening = LeaderKeyPreference(key: "f17", targetLayer: .navigation, enabled: true) + let interveningData = try JSONEncoder().encode(intervening) + var reloadCount = 0 + manager.onBeforeSave = { + self.defaults.set(interveningData, forKey: PreferencesService.leaderKeyPreferenceKey) + } + manager.onRulesChanged = { + reloadCount += 1 + return ReloadResult(success: true, response: nil, errorMessage: nil, protocol: nil, disposition: .applied) + } + + await manager.updateLeaderKey("tab") + + XCTAssertEqual(reloadCount, 0) + XCTAssertEqual(try storedLeader(defaults), intervening) + XCTAssertEqual(ruleFiles(at: directory), beforeFiles) + XCTAssertFalse(FileManager.default.fileExists(atPath: RecoverableRuleWrite.journalURL(directory).path)) + } + + @MainActor + func testBootstrapRecoversInterruptedLeaderAndRuleRevision() async throws { + let (stagingManager, stagingService) = try makeManager(at: directory) + try preferencesReset(to: .default) + stagingManager.ruleCollections = try leaderCollections() + let attempted = LeaderKeyPreference(key: "tab", targetLayer: .navigation, enabled: true) + try await stagingService.operationGate.withOperation { @MainActor permit in + _ = try await stagingService.stageRuleState( + ruleCollections: stagingManager.ruleCollections, + customRules: [], + collectionStore: stagingManager.ruleCollectionStore, + customStore: stagingManager.customRulesStore, + mutationPermit: permit, + preferenceDefaults: self.defaults, + preferenceChanges: [.leader( + before: defaults.data(forKey: PreferencesService.leaderKeyPreferenceKey), + after: JSONEncoder().encode(attempted) + )], + leaderKeyPreference: attempted + ) + } + XCTAssertEqual(try storedLeader(defaults), attempted) + + let (recoveringManager, _) = try makeManager(at: directory) + recoveringManager.onRulesChanged = { + ReloadResult(success: true, response: nil, errorMessage: nil, protocol: nil, disposition: .applied) + } + await recoveringManager.bootstrap() + + XCTAssertEqual(try storedLeader(defaults), .default) + XCTAssertEqual(recoveringManager.preferencesService.leaderKeyPreference, .default) + XCTAssertFalse(FileManager.default.fileExists(atPath: RecoverableRuleWrite.journalURL(directory).path)) + } + + private func leaderCollections() throws -> [RuleCollection] { + try [XCTUnwrap(RuleCollectionCatalog().defaultCollections().first { + $0.id == RuleCollectionIdentifier.leaderKey + })] + } + + private func initialFiles() throws -> [String: URL] { + let files = [ + "config": directory.appendingPathComponent("keypath.kbd"), + "collections": directory.appendingPathComponent("RuleCollections.json"), + "customRules": directory.appendingPathComponent("CustomRules.json"), + ] + for (role, url) in files { + try Data("before-\(role)".utf8).write(to: url) + } + return files + } + + private func stage( + files: [String: URL], leader: LeaderKeyPreference + ) throws -> RecoverableRuleWrite.PendingWrite { + try RecoverableRuleWrite.stage( + files: files, + contents: Dictionary(uniqueKeysWithValues: files.map { ($0.key, Data("after-\($0.key)".utf8)) }), + directory: directory, + scope: .rules, + preferences: defaults, + preferenceChanges: [.leader( + before: defaults.data(forKey: PreferencesService.leaderKeyPreferenceKey), + after: JSONEncoder().encode(leader) + )] + ) + } + + private func ruleFiles(at directory: URL) -> [String: Data?] { + Dictionary(uniqueKeysWithValues: ["keypath.kbd", "RuleCollections.json", "CustomRules.json"].map { name in + let url = directory.appendingPathComponent(name) + return (name, try? Data(contentsOf: url)) + }) + } + + private func storedLeader(_ defaults: UserDefaults) throws -> LeaderKeyPreference { + try JSONDecoder().decode( + LeaderKeyPreference.self, + from: XCTUnwrap(defaults.data(forKey: PreferencesService.leaderKeyPreferenceKey)) + ) + } + + private func preferencesReset(to value: LeaderKeyPreference) throws { + try defaults.set(JSONEncoder().encode(value), forKey: PreferencesService.leaderKeyPreferenceKey) + } + + @MainActor + private func makeManager( + at directory: URL, + synchronizePreferences: @escaping @Sendable (RecoverableRuleWrite.PreferenceDefaults) -> Bool = { $0.value.synchronize() } + ) throws -> (RuleCollectionsManager, ConfigurationService) { + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + let collections = RuleCollectionStore.testStore(at: directory.appendingPathComponent("RuleCollections.json")) + let rules = CustomRulesStore.testStore(at: directory.appendingPathComponent("CustomRules.json")) + let service = ConfigurationService( + configDirectory: directory.path, + ruleCollectionStore: collections, + customRulesStore: rules, + synchronizePreferences: synchronizePreferences + ) + let preferences = PreferencesService(leaderDefaults: defaults) + let manager = RuleCollectionsManager( + ruleCollectionStore: collections, + customRulesStore: rules, + configurationService: service, + keymapPreferences: defaults, + preferencesService: preferences + ) + return (manager, service) + } +} + +private final class LockedCounter: @unchecked Sendable { + private let lock = NSLock() + private var count = 0 + + func increment() { + lock.withLock { count += 1 } + } + + var value: Int { + lock.withLock { count } + } +} diff --git a/Tests/KeyPathTests/PackRuleTransactionTests.swift b/Tests/KeyPathTests/PackRuleTransactionTests.swift index 13a26276b..94b40d4b3 100644 --- a/Tests/KeyPathTests/PackRuleTransactionTests.swift +++ b/Tests/KeyPathTests/PackRuleTransactionTests.swift @@ -312,6 +312,43 @@ final class PackRuleTransactionTests: KeyPathTestCase { XCTAssertEqual(try snapshot(), before) } + func testPackRecoveryRefreshesLeaderPreferenceCachedFromStagedRevision() async throws { + let defaults = manager.preferencesService.persistenceDefaults + manager.preferencesService.leaderKeyPreference = .default + let beforeData = try XCTUnwrap(defaults.data(forKey: PreferencesService.leaderKeyPreferenceKey)) + let attempted = LeaderKeyPreference(key: "tab", targetLayer: .navigation, enabled: true) + let service = manager.configurationService + try await service.operationGate.withOperation { @MainActor permit in + _ = try await service.stageRuleState( + ruleCollections: self.manager.ruleCollections, + customRules: self.manager.customRules, + collectionStore: self.manager.ruleCollectionStore, + customStore: self.manager.customRulesStore, + mutationPermit: permit, + preferenceDefaults: defaults, + preferenceChanges: [.leader(before: beforeData, after: JSONEncoder().encode(attempted))], + leaderKeyPreference: attempted + ) + } + let stagedPreferences = PreferencesService(leaderDefaults: defaults) + XCTAssertEqual(stagedPreferences.leaderKeyPreference, attempted) + let freshManager = RuleCollectionsManager( + ruleCollectionStore: manager.ruleCollectionStore, + customRulesStore: manager.customRulesStore, + configurationService: service, + preferencesService: stagedPreferences + ) + let tracker = tracker! + + try await service.operationGate.withOperation { permit in + try await PackInstaller.shared.recoverAndValidateState( + manager: freshManager, tracker: tracker, permit: permit + ) + } + + XCTAssertEqual(stagedPreferences.leaderKeyPreference, .default) + } + func testMetadataOnlyToggleRecoversInterruptedPackBeforeChangingRecords() async throws { let before = try snapshot() let service = manager.configurationService diff --git a/docs/architecture/configuration-save-pipeline.md b/docs/architecture/configuration-save-pipeline.md index 11b4f731e..852cff6ac 100644 --- a/docs/architecture/configuration-save-pipeline.md +++ b/docs/architecture/configuration-save-pipeline.md @@ -11,7 +11,7 @@ completed. CLI apply/restore also participate in directory admission; see below. | --- | --- | --- | | Generate and validate collection-backed configuration | `ConfigurationService` | `saveConfiguration` validates before its atomic write and updates the in-memory configuration and observers. | | Coordinate generated/raw configuration saves | `SaveCoordinator` | Suppresses the watcher, validates, snapshots the last good file, writes, classifies reload, and rolls the file back after rejection or failure. | -| Persist rule and custom-rule source data | `RuleCollectionsManager` | Mutates in-memory state and requests a recoverable config/source-store write from `ConfigurationService`. Notifications follow the file-set commit and precede reload. Runtime rollback and preferences remain separate. | +| Persist rule and custom-rule source data | `RuleCollectionsManager` | Mutates in-memory state and requests a recoverable config/source-store write from `ConfigurationService`. Notifications follow the file-set commit. Leader-key preference changes join this retained transaction; other preferences remain separate. | | Reload the running engine | `ConfigReloadCoordinator` | Produces the four reload dispositions; `pending` means the write succeeded but the runtime is unavailable. | | Handle external file edits | `ConfigHotReloadService` | Validates and reloads changes that were not suppressed as internal writes. | | Create durable pre-edit backups | `ConfigBackupManager` | Used by explicit backup/recovery flows, not as an alternate writer. | @@ -50,8 +50,20 @@ its throwing contract when even the fallback cannot be written. These outcomes assert only file recovery, not a second engine reload or a multi-store rollback. Existing presentation and Boolean success semantics are unchanged. -File rollback/fallback still exists, but source stores, -preferences, and installed-pack records do not yet share one transaction. +File rollback/fallback still exists. Rule transactions now retain the leader-key +preference with their three files until runtime classification. The preference +candidate remains manager-local until the journal is durable; `UserDefaults` +is then synchronized and read back before reload. A process interruption before +commit restores both the files and leader preference. Applied and pending results +commit both. A third leader preference revision stops recovery before any file is +rolled back and retains the journal for diagnosis. + +This does not yet cover logical keymap selection. Its overlay `@AppStorage` +selection is written before the manager receives the mutation, so reconstructing +that preimage would not be a safe transaction. Context HUD trigger/hold settings +and device selection also regenerate configuration through standalone paths. +Those workflows need their own bounded migration. Display-only preferences remain +outside the journal and are preserved. See the [consolidation baseline](../planning/consolidation-baseline.md) for paths and remaining gaps. UI presentation changes require discussion before implementation. @@ -351,10 +363,10 @@ and settle through `commitRuleMutation`. Single-output editing handles the leade preference/activators in the same operation, rather than calling another save after changing the selected output. Replace-all snapshots the original arrays. -The commit helper optionally receives the prior leader preference. On failure it -restores that preference before error feedback only if it still equals the prepared -value; a newer value is preserved and reported. This is in-process preference -rollback only. UserDefaults is not yet part of the crash-recovery journal. +The commit helper optionally receives the prior leader preference. It keeps the +candidate manager-local, then gives the prior and candidate values to the fixed +leader role in the durable rule journal. Failure and startup recovery restore the +preference with the matching files. A third value fails closed before file rollback. Interrupted-save runtime admission is owned by `ConfigurationService`. Recovering rule or app journals marks a runtime refresh requirement; editor callbacks cannot @@ -372,16 +384,30 @@ separate fallback path; ordinary save rollback does not replace an empty origina with generated content. Raw journal recovery participates in editor admission and startup recovery without marking unchanged rule source arrays as stale. +CLI leader reconciliation uses the same raw journal with the fixed canonical +leader preference role. It generates from a local candidate, compares the exact +defaults preimage before staging, and publishes the shared preference only after +the config and preference commit together. CLI admission recovers and reloads a +retained raw revision before deriving another candidate, including for dry runs. +CLI dry runs never mutate defaults. + +Every generation entry reads the leader revision after retained-journal recovery. +Rule managers reload their injected preference store; pack admission refreshes the +manager cache; raw and app-specific configuration generation decode the canonical +durable value directly. Backup/explicit-restore recovery paths do not generate a +leader-dependent candidate before handing control back to these owners. + A restored raw original that fails validation may be replaced by a separately validated raw edit after recovery reload fails. Only an accepted replacement clears that recovery requirement; cancellation, invalid candidates, and journal conflicts never silently clear it. -Logical keymap changes use the same retained rule-state transaction as collection -edits. The manager recovers before candidate preparation and restores its layout -fields plus the matching optimistic display preference on failure. Persistent -keymap preferences update only after accepted settlement; they are not yet part -of the durable file journal. +Logical keymap changes retain rule files through runtime settlement, and the manager +restores its layout fields plus the matching optimistic display preference on +ordinary failure. The overlay can persist that display selection before manager +admission, so the actual prior keymap preference is not yet available to the durable +rule journal. Process-interruption atomicity for keymap preference and files remains +a separate transaction-boundary change. Collection-backed pack install/removal passes its record change through the existing collection toggle into the canonical rule save. The pack record and three rule files diff --git a/docs/bugs/leader-preference-crash-recovery.md b/docs/bugs/leader-preference-crash-recovery.md new file mode 100644 index 000000000..50fd2add4 --- /dev/null +++ b/docs/bugs/leader-preference-crash-recovery.md @@ -0,0 +1,36 @@ +# Leader preference could outlive a rejected rule revision + +The Leader Key collection updates both generated rule files and the +`KeyPath.LeaderKey.Preference` defaults value. Before this fix, the preference +was persisted before `ConfigurationService` created its retained rule journal. +A process exit in that interval could leave the next launch generating from the +new leader preference after the rule files recovered to their prior revision. + +Leader candidates now remain local to `RuleCollectionsManager` while generation +and validation run. `RecoverableRuleWrite` version 2 journals the fixed leader +preference role beside the config and two source stores before writing any of +them. It synchronizes and verifies the canonical `com.keypath.KeyPath` defaults +domain before runtime application. Rejection, failure, cancellation, and startup +recovery restore the preference and files together. Version 1 journals still +decode with no preference entries. + +The installed CLI prepares reconciliation as a local candidate too. A real +`config apply` journals that candidate with the generated config in the raw +config scope; a dry run only injects it into generation. Validation and file +failures therefore cannot advance the GUI defaults domain by themselves. + +The synchronization barrier follows Foundation's documented process-exit contract: +it waits for in-progress defaults writes and the result is read back before reload. +It is not a claim of atomic persistence across sudden power loss. + +Recovery accepts only the recorded before or after leader value. A third value +means another writer changed the same configuration input; recovery stops before +restoring any file and leaves the journal in place. Unrelated display preferences +are not journaled and are never restored by this operation. + +Logical keymap persistence is not covered by this fix. The overlay currently +persists its selection before it calls the rule manager, so its true prior value +is unavailable at journal creation. That workflow requires moving persistence +behind transaction admission rather than inferring a preimage. Context HUD +timing/trigger settings, device selection, and the generated-file handwritten +ownership guard are also separate follow-ups. diff --git a/docs/planning/catalog-led-consolidation-plan.md b/docs/planning/catalog-led-consolidation-plan.md index e8b0f432c..3af3490d5 100644 --- a/docs/planning/catalog-led-consolidation-plan.md +++ b/docs/planning/catalog-led-consolidation-plan.md @@ -1,7 +1,7 @@ # KeyPath: catalog-led consolidation execution plan Date: 2026-09-04 -Status: approved direction; Phase 0 inventory recorded; Phase 1 persistence/recovery foundation implemented, operation migration still open. +Status: approved direction; Phase 0 inventory recorded; Phase 1 operation migration substantially implemented, preferences/revisions and final acceptance still open. Baseline: `master` at `a12bb07d5`; recheck code, branches, and issues before implementation. ## Outcome @@ -478,13 +478,95 @@ verification passed, including process and TCP readiness; installed and distribu executable hashes matched. This is installation/runtime-readiness evidence, not clean-VM first-run or physical-keyboard acceptance. -### System-pack transactions in progress +### System-pack transactions merged in #1289 -Embed managed-collection restore snapshots in installed-pack records so the existing -pack journal covers both with the rule revision. Preserve legacy snapshot reading -and existing restore/default choices. Keep runtime rejection separate from the known -generation-conflict retry. Validate before retiring the legacy multi-write paths. +Managed-collection restore snapshots are embedded in installed-pack records so the +existing pack journal covers both with the rule revision. Legacy snapshot reading +and existing restore/default choices are preserved. Runtime rejection remains +separate from the known generation-conflict retry. Deployment checkpoint: #1288 was installed from merged master `f77e2c2ad63dff743a6aba8fb19c31e3704387fe`. Signing, notarization, stapling, process, and TCP checks passed; installed/distribution executable hashes matched. + +## Coordination checkpoint β€” September 6 + +Live baseline: `origin/master` and integration checkout at +`5e3e9570099628eb77f21f6f011aad8aa9da988b`. GitHub confirms #1289 and #1290 +merged. #1290 explicitly changes the runner reserve to 90 GiB; the older +100 GiB incident notes above remain historical evidence, not current policy. +#1261 external scratch migration is still open and is not implied complete. + +The handoff reports the merged release candidate passed tests, signing, +notarization, stapling and deployment. The coordinator independently reran +`Scripts/verify-installed-app.sh`: signature, Gatekeeper, stapled ticket, +process, launchd and TCP readiness passed. Local evidence: +`/tmp/keypath-catalog-lead-installed-verification.log`. These checks do not +establish physical input or clean-machine first-remap acceptance. + +### Current contracts and sequence + +1. **Durable leader/keymap preferences (implementation active).** Journal the + affected preference fields before their first durable mutation, retain them + through runtime settlement, and recover them with the rule revision. Test + interruption before file staging as well as after persistence. Preserve + unrelated layout/display preferences and unrelated per-keymap punctuation + choices. Keep old journal decoding compatible. No UI change. +2. **Freshness at mutation admission (implementation active).** Refresh normally + committed source revisions before a root editor snapshots its state. A second + instance or CLI commit produces no recovery journal, so recovery-only cache + invalidation is insufficient. Preserve nested candidates and queued intent. +3. **Remaining generation inputs.** Explicitly cover shortcut-list trigger mode, + hold-delay preset/custom value, and device selection. They currently use + notification-driven regeneration; device selection persists separately before + an explicit apply/restart. The first preference slice does not close these. +4. **Global managed-file protection.** App-specific edits have a reproducibility + guard; global collection/mapper saves and standalone regeneration do not. + Extend preservation to those funnels using the prior generation inputs, before + any source or config write. Document CLI apply's explicit overwrite contract + separately; serial admission alone does not give it runtime rollback. +5. **Revision-aware file reconciliation.** Replace `suppressEvents(for:)` in + `ConfigFileWatcher` and save/runtime callers with actual internal revisions. + Buffer through admitted writes and reconcile afterward. Test atomic replacement, + deletion/recreation, same-mtime content changes, overlapping callbacks, + cancellation, and stop/restart. Preserve non-config KindaVim watcher consumers. + External reload validation and publication must refer to the same revision. +6. **Catalog consistency (UI decision pending).** Discuss affected-key/layer + conflict preview, reversible switches, pack ownership, and applied/pending/ + recovery feedback before implementation. Preserve explicit CLI policy choices. +7. **Pure generation boundary.** Supply app-specific keys, device selection, + physical layout, and preference values as immutable inputs; remove generator + reads of disk, caches and UserDefaults. Reuse existing golden outputs. Leave + live adapters and transaction ownership with their canonical owners. +8. **CLI and consumer boundaries.** Inventory actual commands, then remove broad + dependencies where complete command groups can compile independently. There + are 58 direct `KeyPathAppKit` imports in CLI sources at this checkpoint; + removing imports alone does not remove the target dependency. +9. **Advanced support and acceptance.** Preserve hand-written files and explain + conversion requirements. Retain existing features unless Micah approves a + specific retirement/migration proposal. Complete the final journey matrix + above, collecting clean-VM and physical-input evidence separately. + +The existing `codex/module-split-spike` remains untouched. Its inspected diff is +only a Window Snapping target declaration in `Package.swift` (17 additions, +1 deletion); it is not a generation extraction or tested CLI boundary. + +### Progress accounting + +Since `a12bb07d5`, first-parent history contains 28 merged implementation PRs +for this program plus two supporting review/runner PRs (#1276 and #1290). +Count this as delivered implementation evidence, not a percentage: remaining +slices differ substantially in size and require product decisions. + +Formal phase acceptance is currently **0 of 5 phase exit gates (0%) signed off**. +This measures completed acceptance gates, not the fraction of code written. +Phase 0 still has live core-journey evidence gaps; Phase 1 has the concrete +boundaries above; Phases 2–4 have not passed their exit gates. Update each phase +only against its stated exit evidence, and report implementation milestones +alongside this conservative acceptance measure. + +Coordination uses a flat hierarchy: Sol implements substantive changes; Terra +handles bounded refactors/audits; Luna collects mechanical inventories. One local +Swift build slot is assigned explicitly. The lead owns invariant review, final +acceptance, UI escalation, merge and deploy verification. Existing merge +authorization applies; no public release is authorized.