From 5e183210523ef1854496e387949a128a95d3f9a4 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Tue, 15 Sep 2026 08:30:42 -0700 Subject: [PATCH 1/5] Keep the daemon registered when a restart fails A failed `keypath service restart` could leave com.keypath.kanata absent from the launchd system domain entirely, with no command-line path back: `stop()` removes the SMAppService registration before it verifies its postcondition, and `start()` swallows a failed re-register. Proven on a disposable macOS 26 guest, where launchctl then reported no such service and `system repair` needed user action to recover. Restart now restores the registration before surfacing the failure, so a restart that cannot succeed leaves the caller no worse off than before it ran. A registration merely awaiting approval is left alone, since re-registering would discard the pending approval. Starting also polls briefly while a fresh registration settles. SMAppService registers asynchronously, so the status read taken immediately after register() can still report notRegistered and fail a start that would have worked moments later. That is the likely cause of recovery needing two attempts after an emergency stop. Also corrects two user-facing messages that sent this investigation down the wrong path: the restart failure hint blamed administrator authorization without mentioning the service state, and the runtime wizard said "Click Fix" while its buttons read Start and Restart. Co-Authored-By: Claude Opus 5 --- .../Services/Kanata/KanataDaemonService.swift | 64 ++++++++++++- .../Service/ServiceRestartCommand.swift | 2 +- .../UI/Pages/WizardKanataServicePage.swift | 6 +- .../KanataDaemonServiceIntegrationTests.swift | 95 ++++++++++++++++++- 4 files changed, 158 insertions(+), 9 deletions(-) diff --git a/Sources/KeyPathAppKit/Services/Kanata/KanataDaemonService.swift b/Sources/KeyPathAppKit/Services/Kanata/KanataDaemonService.swift index ed39d0e75..7f3f4484f 100644 --- a/Sources/KeyPathAppKit/Services/Kanata/KanataDaemonService.swift +++ b/Sources/KeyPathAppKit/Services/Kanata/KanataDaemonService.swift @@ -172,6 +172,28 @@ final class KanataDaemonService { return false } + /// SMAppService settles a fresh registration asynchronously, so a status + /// read taken immediately after `register()` can still report + /// `.notRegistered` and fail a start that would have succeeded a moment + /// later. Poll briefly before concluding the registration did not persist. + /// Status is slow synchronous IPC, so keep the attempt count small. + private func waitForRegistrationToSettle( + maxAttempts: Int = 6, + delayMilliseconds: Int = 250 + ) async -> SMAppService.Status { + var status = await currentRegistrationStatus() + for attempt in 1 ... maxAttempts { + guard status == .notRegistered || status == .notFound else { + return status + } + if attempt < maxAttempts { + try? await Task.sleep(for: .milliseconds(delayMilliseconds)) + status = await currentRegistrationStatus() + } + } + return status + } + private enum StoppedPostcondition: Equatable { case satisfied case registrationPresent @@ -258,7 +280,7 @@ final class KanataDaemonService { // Registration can race with an IPC error. The fresh status below // is authoritative, so do not fail before observing the result. try? await registerDaemon() - finalRegistrationStatus = await currentRegistrationStatus() + finalRegistrationStatus = await waitForRegistrationToSettle() @unknown default: throw KanataDaemonServiceError.startFailed(reason: "Unknown SMAppService registration state") } @@ -343,11 +365,47 @@ final class KanataDaemonService { /// KeepAlive respawn. func restart() async throws { AppLogger.shared.log("🔄 [KanataDaemonService] Restart requested") - try await stop() - try await start() + do { + try await stop() + try await start() + } catch { + // `stop()` removes the SMAppService registration before it verifies + // its postcondition, and `start()` deliberately swallows a failed + // re-register. Either way a thrown restart can leave the daemon with + // no registration at all, which launchd reports as a missing service + // and which no command-line path recovers. Put the registration back + // before surfacing the failure so the caller is never left worse off + // than before the restart. + await restoreRegistrationAfterFailedRestart() + throw error + } AppLogger.shared.info("✅ [KanataDaemonService] Restart requested successfully") } + /// Re-register the daemon after a failed restart, but only when the + /// registration is actually gone. A registration that merely awaits approval + /// is left alone: re-registering cannot clear that state and would discard + /// the pending approval. + private func restoreRegistrationAfterFailedRestart() async { + let status = await currentRegistrationStatus() + guard status == .notRegistered || status == .notFound else { return } + + AppLogger.shared.warn( + "⚠️ [KanataDaemonService] Restart failed with the daemon unregistered; restoring registration" + ) + do { + try await registerDaemon() + AppLogger.shared.info( + "✅ [KanataDaemonService] Registration restored after failed restart" + ) + } catch { + AppLogger.shared.error( + "❌ [KanataDaemonService] Could not restore registration after failed restart: " + + error.localizedDescription + ) + } + } + /// Returns whether the internal recovery daemon is currently active. func isDaemonRunning() async -> Bool { let status = await refreshStatus() diff --git a/Sources/KeyPathCLI/Commands/Service/ServiceRestartCommand.swift b/Sources/KeyPathCLI/Commands/Service/ServiceRestartCommand.swift index 691b834d0..18780fc2b 100644 --- a/Sources/KeyPathCLI/Commands/Service/ServiceRestartCommand.swift +++ b/Sources/KeyPathCLI/Commands/Service/ServiceRestartCommand.swift @@ -23,7 +23,7 @@ struct ServiceRestart: AsyncParsableCommand { } else { let error = CLIError.serviceControlFailed( action: "restart", - hint: "macOS may require administrator authorization for system services. Check 'keypath service status --json' or use KeyPath's repair UI." + hint: "The service did not reach a running, responding state. Run 'keypath service status --json' to see where it stopped, then 'keypath system repair' or KeyPath's repair UI. Registering a system service can also need approval in System Settings." ) CLIOutput.writeError(error, context: ctx) throw error.code.exitCode diff --git a/Sources/KeyPathInstallationWizard/UI/Pages/WizardKanataServicePage.swift b/Sources/KeyPathInstallationWizard/UI/Pages/WizardKanataServicePage.swift index 26007ff6c..5105d33ef 100644 --- a/Sources/KeyPathInstallationWizard/UI/Pages/WizardKanataServicePage.swift +++ b/Sources/KeyPathInstallationWizard/UI/Pages/WizardKanataServicePage.swift @@ -201,15 +201,15 @@ public struct WizardKanataServicePage: View { case .running: "KeyPath Runtime, powered by Kanata Engine, is running and ready to process keyboard events." case .stopped: - "KeyPath runtime is not running. Click Fix to start it." + "KeyPath runtime is not running. Click Start to start it." case .failed: - "KeyPath Runtime, powered by Kanata Engine, failed to start. Click Fix to retry." + "KeyPath Runtime, powered by Kanata Engine, failed to start. Click Restart to retry." case .starting: "Starting KeyPath runtime…" case .stopping: "KeyPath runtime is shutting down." case .unknown: - "Checking KeyPath runtime status… If this takes too long, click Fix." + "Checking KeyPath runtime status… If this takes too long, click Start." } } diff --git a/Tests/KeyPathTests/Services/KanataDaemonServiceIntegrationTests.swift b/Tests/KeyPathTests/Services/KanataDaemonServiceIntegrationTests.swift index d090b0a3c..913c32bd7 100644 --- a/Tests/KeyPathTests/Services/KanataDaemonServiceIntegrationTests.swift +++ b/Tests/KeyPathTests/Services/KanataDaemonServiceIntegrationTests.swift @@ -10,21 +10,41 @@ private class MockSMAppService: SMAppServiceProtocol, @unchecked Sendable { case unregisterFailed } - var status: SMAppService.Status + private var storedStatus: SMAppService.Status var registerCalled = false var unregisterCalled = false var calls: [String] = [] var statusesAfterUnregister: [SMAppService.Status] = [] var failingUnregisterCalls: Set = [] + var failingRegisterCalls: Set = [] var statusBeforeRegisterError: SMAppService.Status? + /// Statuses handed out by the next reads, ahead of the stored value. Models + /// SMAppService settling a registration asynchronously. + var pendingStatusReads: [SMAppService.Status] = [] + + var status: SMAppService.Status { + get { + guard !pendingStatusReads.isEmpty else { return storedStatus } + return pendingStatusReads.removeFirst() + } + set { storedStatus = newValue } + } + + /// The stored value, read without consuming a pending read. + var settledStatus: SMAppService.Status { + storedStatus + } init(status: SMAppService.Status = .notRegistered) { - self.status = status + storedStatus = status } func register() throws { registerCalled = true calls.append("register") + if failingRegisterCalls.contains(calls.count(where: { $0 == "register" })) { + throw MockError.registerFailed + } if let statusBeforeRegisterError { status = statusBeforeRegisterError throw MockError.registerFailed @@ -277,6 +297,77 @@ final class KanataDaemonServiceIntegrationTests: KeyPathAsyncTestCase { XCTAssertEqual(mock.status, .notRegistered) } + /// A restart that fails must never leave the daemon unregistered. `stop()` + /// removes the SMAppService registration before verifying its postcondition, + /// so without a rollback a failed restart leaves launchd with no such + /// service and no command-line path back. + func testRestartRestoresRegistrationWhenStartCannotRegister() async { + let mock = MockSMAppService(status: .enabled) + // Fail only the start path's register, so the rollback's register works. + mock.failingRegisterCalls = [1] + useService(mock) + service = KanataDaemonService() + + do { + try await service.restart() + XCTFail("Expected restart to fail when the service cannot re-register") + } catch { + // Expected: the restart still reports failure. + } + + XCTAssertEqual( + mock.settledStatus, .enabled, + "A failed restart must leave the daemon registered, not unregistered" + ) + XCTAssertEqual( + mock.calls.count(where: { $0 == "register" }), 2, + "Expected the failed start's register plus the rollback's register" + ) + } + + /// The same guarantee when the failure happens in `stop()` rather than + /// `start()`: the registration is already gone by the time stop throws. + func testRestartRestoresRegistrationWhenStopFails() async { + let mock = MockSMAppService(status: .enabled) + useService(mock) + // The process never goes away, so the stopped postcondition is never met. + KanataDaemonService.processRunningOverride = { true } + KanataDaemonService.privilegedStopOverride = {} + service = KanataDaemonService() + + do { + try await service.restart() + XCTFail("Expected restart to fail when the daemon never stops") + } catch { + // Expected: the restart still reports failure. + } + + XCTAssertEqual( + mock.settledStatus, .enabled, + "A restart that fails during stop must still leave the daemon registered" + ) + } + + /// SMAppService settles a registration asynchronously, so the status read + /// taken right after `register()` can still say `.notRegistered`. Starting + /// must poll rather than fail a start that would have succeeded, which is + /// what made recovery take two attempts. + func testStartPollsWhileRegistrationSettles() async throws { + let mock = MockSMAppService(status: .notRegistered) + // Entry read, the read inside register(), and one stale post-register read. + mock.pendingStatusReads = [.notRegistered, .notRegistered, .notRegistered] + useService(mock) + service = KanataDaemonService() + + try await service.start() + + XCTAssertEqual(mock.settledStatus, .enabled) + XCTAssertEqual( + mock.calls.count(where: { $0 == "register" }), 1, + "A settling registration must not trigger a second register" + ) + } + func testStatusRefresh_ShouldDetectChanges() async { // Given: Initial unknown state From a018dc5e159edcf25cdb9bb562c30b5eaf7997da Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Tue, 15 Sep 2026 08:44:18 -0700 Subject: [PATCH 2/5] Refuse custom rules for keys an enabled collection owns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User-visible bug: with the built-in "Home Row Arrows" collection enabled (it claims `f` on the base layer as a momentary layer activator), running keypath-cli rule add f --tap f --hold lsft --on-conflict replace --apply reported the rule as created. The rule could never take effect: config generation keeps the collection's `layer_home-arrows_f` binding and drops the custom `beh_base_f` one, and `ConfigurationService.generateConfiguration` refuses to generate at all once `RuleCollectionDeduplicator.detectConflicts` sees the collision (the activator-vs-mapping branch, #667). Reproduced both halves in tests. The detector does fire, so the conflict is not invisible — but it fires far too late. `RulesFacade.addRule` compared the new rule only against other custom rules in CustomRulesStore, never against collections, so it persisted the rule and reported success; the apply that followed then failed, and the rule stayed in the store poisoning every later `keypath config apply` until it was removed by hand. `--on-conflict replace` could not help, because it cannot see a collection-owned key at all. Fix, matching this codebase's detect-and-explain lineage (#458, #462, #463, #464, #466, #667) and the GUI's interactive resolution (#460) rather than silently letting one side win: the CLI-operation `RulesFacade` now loads RuleCollections.json and pre-flights the prospective rule through the same `detectConflicts` the generator uses, before anything is written. Conflicts that already existed between collections are subtracted, so a store that is already in a bad state does not block an unrelated key. `--on-conflict=skip` no-ops; every other strategy raises `CLICollectionConflictError`, which `rule add` renders as an exit-4 conflict naming the owning collection and telling the user to disable it or pick another key. Nothing is persisted. The app-facing `RulesFacade()` and `RulesFacade(store:)` initialisers supply no collection loader, so their behaviour is unchanged. Co-Authored-By: Claude Opus 5 --- Sources/KeyPathAppKit/CLI/RulesFacade.swift | 102 ++++++- .../Commands/Rule/RuleAddCommand.swift | 8 + Sources/KeyPathCLICommon/CLIError.swift | 4 +- .../CLI/CLIRuleCollectionConflictTests.swift | 249 ++++++++++++++++++ 4 files changed, 359 insertions(+), 4 deletions(-) create mode 100644 Tests/KeyPathTests/CLI/CLIRuleCollectionConflictTests.swift diff --git a/Sources/KeyPathAppKit/CLI/RulesFacade.swift b/Sources/KeyPathAppKit/CLI/RulesFacade.swift index f9b0bf6b1..b16d6b068 100644 --- a/Sources/KeyPathAppKit/CLI/RulesFacade.swift +++ b/Sources/KeyPathAppKit/CLI/RulesFacade.swift @@ -5,21 +5,64 @@ import KeyPathRulesCore public struct RulesFacade: Sendable { private let store: CustomRulesStore private let operation: CLIConfigurationOperation? + /// Loads the rule collections the generated config will be built from. + /// + /// Only the CLI-operation path supplies this. A custom rule can collide with a + /// key an *enabled collection* already claims (its mappings, or its momentary + /// layer activator), and `ConfigurationService.generateConfiguration` refuses to + /// generate at all in that case. Without this loader `addRule` only sees other + /// custom rules, so it happily persists a rule that every later `apply` will + /// reject — the rule is reported as created, never takes effect, and poisons + /// the store until it is removed by hand. + private let collectionLoader: (@Sendable () async -> [RuleCollection])? public init() { store = .shared operation = nil + collectionLoader = nil } init(operation: CLIConfigurationOperation) { - store = CustomRulesStore(fileURL: URL(fileURLWithPath: operation.directory) - .appendingPathComponent("CustomRules.json")) + let directory = URL(fileURLWithPath: operation.directory) + store = CustomRulesStore(fileURL: directory.appendingPathComponent("CustomRules.json")) self.operation = operation + let collectionStore = RuleCollectionStore( + fileURL: directory.appendingPathComponent("RuleCollections.json") + ) + collectionLoader = { await collectionStore.loadCollections() } } init(store: CustomRulesStore) { self.store = store operation = nil + collectionLoader = nil + } + + init(store: CustomRulesStore, collectionLoader: @escaping @Sendable () async -> [RuleCollection]) { + self.store = store + operation = nil + self.collectionLoader = collectionLoader + } + + /// Conflicts that `rules` introduce against the enabled collections, relative to + /// `baselineRules`. Pre-existing conflicts between collections are subtracted so a + /// store that is already in a bad state does not block an unrelated rule. + private func collectionConflicts( + adding rules: [CustomRule], baseline baselineRules: [CustomRule] + ) async -> [KeyPathError.MappingConflictInfo] { + guard let collectionLoader else { return [] } + let collections = await collectionLoader() + // Custom rules first, exactly as generateConfiguration orders them. + let after = RuleCollectionDeduplicator.detectConflicts( + in: rules.asRuleCollections() + collections + ) + guard !after.isEmpty else { return [] } + let before = Set( + RuleCollectionDeduplicator + .detectConflicts(in: baselineRules.asRuleCollections() + collections) + .map(\.description) + ) + return after.filter { !before.contains($0.description) } } private func withMutation( @@ -102,6 +145,7 @@ public struct RulesFacade: Sendable { ) async throws -> RuleAddResult { try await withMutation { var rules = await store.loadRules() + let baseline = rules let existingIndex = rules.firstIndex(where: { $0.input == input }) if let existingIndex { @@ -116,6 +160,12 @@ public struct RulesFacade: Sendable { let existing = rules[existingIndex] let merged = try Self.mergeRules(existing: existing, newAction: action, newBehavior: behavior) rules[existingIndex] = merged + let conflicts = await collectionConflicts(adding: rules, baseline: baseline) + if !conflicts.isEmpty { + throw CLICollectionConflictError( + input: input, conflicts: conflicts, ruleName: merged.displayTitle + ) + } try await store.saveRules(rules) return .merged(CLIRuleDetail(from: merged)) } @@ -138,6 +188,19 @@ public struct RulesFacade: Sendable { deviceOverrides: deviceOverrides ) rules.append(rule) + + // A key an enabled collection already claims cannot be taken by a custom + // rule: config generation refuses the whole file rather than silently + // picking a winner. Refuse here, before anything is written, so the CLI + // never reports a rule as created that can never be applied. + let conflicts = await collectionConflicts(adding: rules, baseline: baseline) + if !conflicts.isEmpty { + if onConflict == .skip { return .skipped } + throw CLICollectionConflictError( + input: input, conflicts: conflicts, ruleName: rule.displayTitle + ) + } + try await store.saveRules(rules) let detail = CLIRuleDetail(from: rule) @@ -413,3 +476,38 @@ public struct CLIConflictError: Error, CustomStringConvertible { "Rule already exists for '\(input)'" } } + +/// The requested key is already claimed by an enabled rule collection (either one of +/// its mappings or its momentary layer activator). Config generation refuses to build +/// a file with two owners for one key, so the rule is rejected before it is persisted +/// rather than written and then rejected by every subsequent `apply`. +public struct CLICollectionConflictError: Error, CustomStringConvertible { + public let input: String + public let conflicts: [KeyPathError.MappingConflictInfo] + /// Display name the rejected rule would have had as a collection. Custom rules + /// appear in the conflict as a collection named after their display title, so it + /// is filtered out of `collectionNames` — the user does not need to be told their + /// own pending rule is one of the claimants. + public let ruleName: String + + public init(input: String, conflicts: [KeyPathError.MappingConflictInfo], ruleName: String = "") { + self.input = input + self.conflicts = conflicts + self.ruleName = ruleName + } + + /// Names of the collections claiming the key, excluding the pending custom rule. + public var collectionNames: [String] { + var seen = Set() + return conflicts.flatMap(\.conflictingCollections) + .filter { $0 != ruleName && seen.insert($0).inserted } + } + + public var explanation: String { + conflicts.map(\.userExplanation).joined(separator: "\n\n") + } + + public var description: String { + "Key '\(input)' is already used by \(collectionNames.joined(separator: ", "))" + } +} diff --git a/Sources/KeyPathCLI/Commands/Rule/RuleAddCommand.swift b/Sources/KeyPathCLI/Commands/Rule/RuleAddCommand.swift index f425d8c0c..f05587a12 100644 --- a/Sources/KeyPathCLI/Commands/Rule/RuleAddCommand.swift +++ b/Sources/KeyPathCLI/Commands/Rule/RuleAddCommand.swift @@ -169,6 +169,14 @@ struct RuleAdd: AsyncParsableCommand, Sendable { ) CLIOutput.writeError(error, context: ctx) throw CLIExitCode.conflict.exitCode + } catch let collisionErr as CLICollectionConflictError { + let error = CLIError.conflict( + collisionErr.description, + hint: "Disable the collection that owns this key (keypath collection disable ), or pick a different key. Nothing was written.", + details: [collisionErr.explanation] + ) + CLIOutput.writeError(error, context: ctx) + throw CLIExitCode.conflict.exitCode } catch is CLIConflictError { let error = CLIError.conflict( "Rule already exists for '\(input)'", diff --git a/Sources/KeyPathCLICommon/CLIError.swift b/Sources/KeyPathCLICommon/CLIError.swift index 984d27ec2..148692422 100644 --- a/Sources/KeyPathCLICommon/CLIError.swift +++ b/Sources/KeyPathCLICommon/CLIError.swift @@ -53,8 +53,8 @@ public extension CLIError { CLIError(code: .validation, message: message, hint: hint, details: details, docsUrl: nil) } - static func conflict(_ message: String, hint: String? = nil) -> CLIError { - CLIError(code: .conflict, message: message, hint: hint, details: nil, docsUrl: nil) + static func conflict(_ message: String, hint: String? = nil, details: [String]? = nil) -> CLIError { + CLIError(code: .conflict, message: message, hint: hint, details: details, docsUrl: nil) } static func invalidKey(_ key: String, label: String) -> CLIError { diff --git a/Tests/KeyPathTests/CLI/CLIRuleCollectionConflictTests.swift b/Tests/KeyPathTests/CLI/CLIRuleCollectionConflictTests.swift new file mode 100644 index 000000000..55d3b2850 --- /dev/null +++ b/Tests/KeyPathTests/CLI/CLIRuleCollectionConflictTests.swift @@ -0,0 +1,249 @@ +@testable import KeyPathAppKit +import KeyPathCore +import KeyPathRulesCore +import XCTest + +/// Regression coverage for the real-world report: with the built-in "Home Row +/// Arrows" collection enabled (it claims `f` on the base layer as a momentary +/// layer activator), `keypath rule add f --tap f --hold lsft --on-conflict +/// replace --apply` reported the rule as created, yet the generated kanata +/// config kept the collection's `layer_home-arrows_f` binding and the custom +/// `beh_base_f` binding never took effect. +/// +/// Two separate facts, both verified here: +/// 1. `RuleCollectionDeduplicator.detectConflicts` *does* see this collision +/// (activator-vs-mapping, #667), so config generation refuses to build. +/// 2. `RulesFacade.addRule` used to persist the rule anyway, because it only +/// ever compared against other custom rules — so the rule was reported as +/// created, could never be applied, and broke every later `apply` until it +/// was removed by hand. +@MainActor +final class CLIRuleCollectionConflictTests: XCTestCase { + private func homeRowArrows() throws -> RuleCollection { + let collection = try XCTUnwrap( + RuleCollectionCatalog().defaultCollections() + .first { $0.id == RuleCollectionIdentifier.homeRowArrows }, + "Catalog must contain the Home Row Arrows collection" + ) + XCTAssertEqual(collection.momentaryActivator?.input, "f") + XCTAssertEqual(collection.momentaryActivator?.sourceLayer, .base) + return collection + } + + private func tapHoldF() -> (action: KeyAction, behavior: MappingBehavior) { + ( + .keystroke(key: "f"), + .dualRole(DualRoleBehavior( + tapAction: .keystroke(key: "f"), + holdAction: .keystroke(key: "lsft"), + tapTimeout: 200 + )) + ) + } + + private func makeFacade( + collections: [RuleCollection] + ) throws -> (facade: RulesFacade, store: CustomRulesStore, directory: URL) { + let directory = FileManager.default.temporaryDirectory + .appendingPathComponent("kp-rule-conflict-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + let store = CustomRulesStore(fileURL: directory.appendingPathComponent("CustomRules.json")) + let facade = RulesFacade(store: store, collectionLoader: { collections }) + return (facade, store, directory) + } + + // MARK: - The collision is real + + func testActivatorVersusCustomRuleIsDetectedAsAConflict() throws { + var arrows = try homeRowArrows() + arrows.isEnabled = true + let rule = CustomRule( + input: "f", action: tapHoldF().action, behavior: tapHoldF().behavior + ) + + let conflicts = RuleCollectionDeduplicator.detectConflicts( + in: [rule].asRuleCollections() + [arrows] + ) + + XCTAssertEqual(conflicts.count, 1, "Home Row Arrows' `f` activator collides with a base-layer `f` rule") + XCTAssertEqual(conflicts.first?.inputKey, "f") + XCTAssertTrue( + conflicts.first?.conflictingCollections.contains("Home Row Arrows") ?? false, + "The conflict must name the collection that owns the key" + ) + } + + /// The silent drop itself: generation keeps the collection's activator and + /// discards the custom rule's binding. This is why persisting the rule and + /// reporting success is wrong — it can never take effect. + func testGenerationKeepsTheActivatorAndDropsTheCustomBinding() throws { + var arrows = try homeRowArrows() + arrows.isEnabled = true + let rule = CustomRule( + input: "f", action: tapHoldF().action, behavior: tapHoldF().behavior + ) + + let deduped = RuleCollectionDeduplicator.dedupe([rule].asRuleCollections() + [arrows]) + let config = KanataConfiguration.generateFromCollections(deduped) + + XCTAssertTrue(config.contains("layer_home-arrows_f"), "Collection activator survives") + XCTAssertFalse( + config.contains("@beh_base_f"), + "The custom rule's binding is never referenced by a layer — it has no effect" + ) + } + + // MARK: - addRule refuses instead of persisting a rule that can never apply + + func testAddRuleRefusesAKeyOwnedByAnEnabledCollection() async throws { + var arrows = try homeRowArrows() + arrows.isEnabled = true + let (facade, store, directory) = try makeFacade(collections: [arrows]) + defer { try? FileManager.default.removeItem(at: directory) } + + do { + _ = try await facade.addRule( + input: "f", action: tapHoldF().action, behavior: tapHoldF().behavior, + onConflict: .replace + ) + XCTFail("addRule must refuse a key an enabled collection already claims") + } catch let error as CLICollectionConflictError { + XCTAssertEqual(error.input, "f") + XCTAssertEqual( + error.collectionNames, ["Home Row Arrows"], + "The error names the owning collection only, not the rejected rule itself" + ) + XCTAssertFalse(error.explanation.isEmpty) + XCTAssertTrue( + error.description.contains("Home Row Arrows"), + "The one-line message must name the collection: \(error.description)" + ) + } + + let stored = await store.loadRules() + XCTAssertTrue(stored.isEmpty, "Nothing may be persisted when the rule can never be applied") + } + + func testAddRuleRefusesAKeyMappedByAnEnabledCollection() async throws { + let capsCollection = try XCTUnwrap( + RuleCollectionCatalog().defaultCollections() + .first { $0.id == RuleCollectionIdentifier.capsLockRemap } + ) + var enabled = capsCollection + enabled.isEnabled = true + let (facade, store, directory) = try makeFacade(collections: [enabled]) + defer { try? FileManager.default.removeItem(at: directory) } + + do { + _ = try await facade.addRule(input: "caps", action: .keystroke(key: "esc")) + XCTFail("addRule must refuse a key an enabled collection already maps") + } catch is CLICollectionConflictError { + // expected + } + let stored = await store.loadRules() + XCTAssertTrue(stored.isEmpty) + } + + func testAddRuleSkipStrategyNoOpsInsteadOfThrowing() async throws { + var arrows = try homeRowArrows() + arrows.isEnabled = true + let (facade, store, directory) = try makeFacade(collections: [arrows]) + defer { try? FileManager.default.removeItem(at: directory) } + + let result = try await facade.addRule( + input: "f", action: tapHoldF().action, behavior: tapHoldF().behavior, + onConflict: .skip + ) + + guard case .skipped = result else { + return XCTFail("--on-conflict=skip must no-op on a collection-owned key, got \(result)") + } + let stored = await store.loadRules() + XCTAssertTrue(stored.isEmpty) + } + + // MARK: - The check must not over-fire + + func testAddRuleAllowsAKeyNoEnabledCollectionClaims() async throws { + var arrows = try homeRowArrows() + arrows.isEnabled = true + let (facade, store, directory) = try makeFacade(collections: [arrows]) + defer { try? FileManager.default.removeItem(at: directory) } + + let result = try await facade.addRule(input: "f13", action: .keystroke(key: "f14")) + + guard case .created = result else { + return XCTFail("An unclaimed key must still be accepted, got \(result)") + } + let stored = await store.loadRules() + XCTAssertEqual(stored.map(\.input), ["f13"]) + } + + func testAddRuleAllowsAKeyOwnedOnlyByADisabledCollection() async throws { + var arrows = try homeRowArrows() + arrows.isEnabled = false + let (facade, store, directory) = try makeFacade(collections: [arrows]) + defer { try? FileManager.default.removeItem(at: directory) } + + let result = try await facade.addRule( + input: "f", action: tapHoldF().action, behavior: tapHoldF().behavior + ) + + guard case .created = result else { + return XCTFail("A disabled collection claims nothing, got \(result)") + } + let stored = await store.loadRules() + XCTAssertEqual(stored.map(\.input), ["f"]) + } + + /// A store that is already conflicted must not block an unrelated rule: only + /// conflicts the new rule introduces are held against it. + func testPreExistingCollectionConflictDoesNotBlockAnUnrelatedRule() async throws { + var arrows = try homeRowArrows() + arrows.isEnabled = true + let conflicting = RuleCollection( + id: UUID(), + name: "Rival F Mapper", + summary: "Also claims f on the base layer", + category: .custom, + mappings: [KeyMapping(input: "f", action: .keystroke(key: "x"))], + isEnabled: true, + isSystemDefault: false, + icon: "square.and.pencil", + targetLayer: .base + ) + + let (facade, store, directory) = try makeFacade(collections: [arrows, conflicting]) + defer { try? FileManager.default.removeItem(at: directory) } + + XCTAssertFalse( + RuleCollectionDeduplicator.detectConflicts(in: [arrows, conflicting]).isEmpty, + "Precondition: the collections conflict with each other" + ) + + let result = try await facade.addRule(input: "f13", action: .keystroke(key: "f14")) + guard case .created = result else { + return XCTFail("A pre-existing conflict must not block an unrelated key, got \(result)") + } + let stored = await store.loadRules() + XCTAssertEqual(stored.map(\.input), ["f13"]) + } + + /// The facade the app and the plain CLI initialiser build has no collection + /// loader, so its behaviour is unchanged. + func testFacadeWithoutCollectionLoaderIsUnchanged() async throws { + let directory = FileManager.default.temporaryDirectory + .appendingPathComponent("kp-rule-noloader-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directory) } + let store = CustomRulesStore(fileURL: directory.appendingPathComponent("CustomRules.json")) + + let result = try await RulesFacade(store: store).addRule( + input: "f", action: tapHoldF().action, behavior: tapHoldF().behavior + ) + + guard case .created = result else { + return XCTFail("Expected created, got \(result)") + } + } +} From c862e4a0ff1cff804eaa70ecd202e636d17c08c1 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Tue, 15 Sep 2026 10:59:26 -0700 Subject: [PATCH 3/5] Advance the stable Xcode pin to 27.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Xcode 26.6 was upgraded in place on the build machine, leaving a bundle still named Xcode-26.6.app that actually contains Xcode 27.0 (27A266a). The pin guard compares the reported version, so it correctly refused the mismatch — and every canonical build, test, deploy, and release script stopped working, CI included. Nothing had built since, because the only runs in between were docs-only or a ping job. Advance the pin rather than reinstall the old toolchain: 26.6 no longer exists on the machine, and the bundle now holds the release build of 27. The bundle is renamed to Xcode-27.app so its name matches its contents; the misleading name is what made this look like a missing install. Verified per the documented procedure: ensure-metal-toolchain.sh passes with no override, DeploymentScriptContractTests passes through run-tests-safe.sh, and both suites from this branch run clean through the canonical runner rather than a bare swift test. Note for anyone bisecting: Xcode 27's Metal toolchain reports unavailable on the same invocation that downloads it, and resolves once the asset cryptex mounts. Re-run the script rather than concluding it failed. Co-Authored-By: Claude Opus 5 --- Scripts/lib/xcode.sh | 4 ++-- Tests/KeyPathTests/Lint/DeploymentScriptContractTests.swift | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Scripts/lib/xcode.sh b/Scripts/lib/xcode.sh index 8ce7f4301..62387b802 100644 --- a/Scripts/lib/xcode.sh +++ b/Scripts/lib/xcode.sh @@ -3,8 +3,8 @@ # Canonical local Xcode selection for KeyPath build, test, deploy, and release scripts. # Set KEYPATH_DEV_XCODE_DEVELOPER_DIR only when intentionally validating another toolchain. -KEYPATH_STABLE_XCODE_VERSION="${KEYPATH_STABLE_XCODE_VERSION:-26.6}" -KEYPATH_STABLE_XCODE_DEVELOPER_DIR="${KEYPATH_STABLE_XCODE_DEVELOPER_DIR:-/Applications/Xcode-26.6.app/Contents/Developer}" +KEYPATH_STABLE_XCODE_VERSION="${KEYPATH_STABLE_XCODE_VERSION:-27.0}" +KEYPATH_STABLE_XCODE_DEVELOPER_DIR="${KEYPATH_STABLE_XCODE_DEVELOPER_DIR:-/Applications/Xcode-27.app/Contents/Developer}" keypath_xcode_version() { local developer_dir="$1" diff --git a/Tests/KeyPathTests/Lint/DeploymentScriptContractTests.swift b/Tests/KeyPathTests/Lint/DeploymentScriptContractTests.swift index c1d1bfba2..02b18be7c 100644 --- a/Tests/KeyPathTests/Lint/DeploymentScriptContractTests.swift +++ b/Tests/KeyPathTests/Lint/DeploymentScriptContractTests.swift @@ -36,8 +36,8 @@ final class DeploymentScriptContractTests: XCTestCase { "Scripts/release-doctor.sh", ] - XCTAssertTrue(xcodeContract.contains("Xcode-26.6.app/Contents/Developer")) - XCTAssertTrue(xcodeContract.contains(#"KEYPATH_STABLE_XCODE_VERSION="${KEYPATH_STABLE_XCODE_VERSION:-26.6}""#)) + XCTAssertTrue(xcodeContract.contains("Xcode-27.app/Contents/Developer")) + XCTAssertTrue(xcodeContract.contains(#"KEYPATH_STABLE_XCODE_VERSION="${KEYPATH_STABLE_XCODE_VERSION:-27.0}""#)) XCTAssertTrue(xcodeContract.contains("/Applications/Xcode.app/Contents/Developer")) XCTAssertTrue(xcodeContract.contains("keypath_xcode_version")) XCTAssertTrue(xcodeContract.contains("KEYPATH_DEV_XCODE_DEVELOPER_DIR")) From b47b91a8d911da5a76028d819ae640d618f3fb90 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Tue, 15 Sep 2026 11:29:21 -0700 Subject: [PATCH 4/5] Clear the warnings Xcode 27 surfaced, so the zero-warning gate holds Advancing the pin exposed 40 warnings that Xcode 26.6 accepted silently. The build and test guardrails allow none, so nothing could merge until these were addressed. Neither group was introduced by this branch. Sendable conformance placement (24 warnings, all in one file). Six classes conform to wizard protocols that inherit Sendable, declared in WizardProtocolConformances.swift rather than beside each class. Swift 6 requires the conformance to live with its type, so each class now states it in its own file. The conformance was already in force through the protocol, so this records an existing guarantee where the compiler wants it rather than making a new claim about thread safety. A test subclass has to restate the inherited conformance, so it does. Environment access outside a View (16 warnings, two call sites). The wizard dismissal path read @Environment(\.dismiss), and the overlay keycap read @Environment(\.services), from tests that construct those views without installing them in a hierarchy. SwiftUI warns on every such read. Both views now accept an injected value that tests supply, and fall back to the environment in the app, where it is real. The overlay test even documented its reliance on the environment default; that reliance is now explicit rather than incidental. Verified with the canonical runner across all affected suites: 106 tests pass with zero build-log and test-log warnings. Co-Authored-By: Claude Opus 5 --- .../Managers/HelperMaintenance.swift | 8 ++++++ .../Managers/KanataDaemonManager.swift | 8 ++++++ .../Managers/RuntimeCoordinator.swift | 8 ++++++ .../Managers/UninstallCoordinator.swift | 8 ++++++ .../PermissionRequestService.swift | 8 ++++++ .../Services/System/SystemValidator.swift | 8 ++++++ .../UI/Overlay/OverlayKeycapView.swift | 11 +++++++- .../UI/InstallationWizardView+Dismissal.swift | 15 +++++++++-- .../UI/InstallationWizardView.swift | 12 ++++++++- .../FirstSuccessOnboardingGateTests.swift | 12 +++++++-- .../Services/UnmappedLayerKeyStyleTests.swift | 10 ++++++-- .../KeyPathTests/UI/LayerSelectorTests.swift | 2 +- .../VallackOverlayZoneTests.swift | 25 +++++++++++++++---- 13 files changed, 121 insertions(+), 14 deletions(-) diff --git a/Sources/KeyPathAppKit/Managers/HelperMaintenance.swift b/Sources/KeyPathAppKit/Managers/HelperMaintenance.swift index 900155b5b..d7ee45a86 100644 --- a/Sources/KeyPathAppKit/Managers/HelperMaintenance.swift +++ b/Sources/KeyPathAppKit/Managers/HelperMaintenance.swift @@ -470,3 +470,11 @@ extension HelperMaintenance { testHooks = hooks } } + +// Sendable is already required of this type: the wizard protocol it conforms to +// in WizardProtocolConformances.swift inherits Sendable, so the conformance has +// been in force all along. Swift 6 requires it to be declared alongside the +// class rather than in the conformance file, so state it here. This records the +// existing guarantee where the compiler wants it; it is not a new claim about +// this type's thread safety. +extension HelperMaintenance: @unchecked Sendable {} diff --git a/Sources/KeyPathAppKit/Managers/KanataDaemonManager.swift b/Sources/KeyPathAppKit/Managers/KanataDaemonManager.swift index 09e6666a9..7f3963ca8 100644 --- a/Sources/KeyPathAppKit/Managers/KanataDaemonManager.swift +++ b/Sources/KeyPathAppKit/Managers/KanataDaemonManager.swift @@ -795,3 +795,11 @@ private extension KanataDaemonManager { NotificationCenter.default.post(name: .smAppServiceApprovalRequired, object: nil) } } + +// Sendable is already required of this type: the wizard protocol it conforms to +// in WizardProtocolConformances.swift inherits Sendable, so the conformance has +// been in force all along. Swift 6 requires it to be declared alongside the +// class rather than in the conformance file, so state it here. This records the +// existing guarantee where the compiler wants it; it is not a new claim about +// this type's thread safety. +extension KanataDaemonManager: @unchecked Sendable {} diff --git a/Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift b/Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift index 26b5da851..537d0614b 100644 --- a/Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift +++ b/Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift @@ -1760,3 +1760,11 @@ struct ReloadResult { self.disposition = disposition ?? (success ? .applied : .failed) } } + +// Sendable is already required of this type: the wizard protocol it conforms to +// in WizardProtocolConformances.swift inherits Sendable, so the conformance has +// been in force all along. Swift 6 requires it to be declared alongside the +// class rather than in the conformance file, so state it here. This records the +// existing guarantee where the compiler wants it; it is not a new claim about +// this type's thread safety. +extension RuntimeCoordinator: @unchecked Sendable {} diff --git a/Sources/KeyPathAppKit/Managers/UninstallCoordinator.swift b/Sources/KeyPathAppKit/Managers/UninstallCoordinator.swift index 5b00ee896..51e13d9a5 100644 --- a/Sources/KeyPathAppKit/Managers/UninstallCoordinator.swift +++ b/Sources/KeyPathAppKit/Managers/UninstallCoordinator.swift @@ -618,3 +618,11 @@ struct AppleScriptResult { // NOTE: AppleScriptRunner was removed - now using PrivilegedCommandRunner which respects // TestEnvironment.useSudoForPrivilegedOps for sudo-based execution in test environments. + +// Sendable is already required of this type: the wizard protocol it conforms to +// in WizardProtocolConformances.swift inherits Sendable, so the conformance has +// been in force all along. Swift 6 requires it to be declared alongside the +// class rather than in the conformance file, so state it here. This records the +// existing guarantee where the compiler wants it; it is not a new claim about +// this type's thread safety. +extension UninstallCoordinator: @unchecked Sendable {} diff --git a/Sources/KeyPathAppKit/Services/Permissions/PermissionRequestService.swift b/Sources/KeyPathAppKit/Services/Permissions/PermissionRequestService.swift index d384cf119..3cc9cbf8b 100644 --- a/Sources/KeyPathAppKit/Services/Permissions/PermissionRequestService.swift +++ b/Sources/KeyPathAppKit/Services/Permissions/PermissionRequestService.swift @@ -175,3 +175,11 @@ public final class PermissionRequestService { return (im, ax) } } + +// Sendable is already required of this type: the wizard protocol it conforms to +// in WizardProtocolConformances.swift inherits Sendable, so the conformance has +// been in force all along. Swift 6 requires it to be declared alongside the +// class rather than in the conformance file, so state it here. This records the +// existing guarantee where the compiler wants it; it is not a new claim about +// this type's thread safety. +extension PermissionRequestService: @unchecked Sendable {} diff --git a/Sources/KeyPathAppKit/Services/System/SystemValidator.swift b/Sources/KeyPathAppKit/Services/System/SystemValidator.swift index 13157c971..a98fdfe85 100644 --- a/Sources/KeyPathAppKit/Services/System/SystemValidator.swift +++ b/Sources/KeyPathAppKit/Services/System/SystemValidator.swift @@ -1025,3 +1025,11 @@ private final class SystemCaptureCompletionState: @unchecked Sendable { return true } } + +// Sendable is already required of this type: the wizard protocol it conforms to +// in WizardProtocolConformances.swift inherits Sendable, so the conformance has +// been in force all along. Swift 6 requires it to be declared alongside the +// class rather than in the conformance file, so state it here. This records the +// existing guarantee where the compiler wants it; it is not a new claim about +// this type's thread safety. +extension SystemValidator: @unchecked Sendable {} diff --git a/Sources/KeyPathAppKit/UI/Overlay/OverlayKeycapView.swift b/Sources/KeyPathAppKit/UI/Overlay/OverlayKeycapView.swift index 03ca54ab0..9a3b5121a 100644 --- a/Sources/KeyPathAppKit/UI/Overlay/OverlayKeycapView.swift +++ b/Sources/KeyPathAppKit/UI/Overlay/OverlayKeycapView.swift @@ -332,7 +332,16 @@ struct OverlayKeycapView: View { return meta } - @Environment(\.services) var services + @Environment(\.services) private var environmentServices + + /// Stands in for the environment's service container when this view is + /// evaluated outside a rendered hierarchy. Reading `@Environment` on an + /// uninstalled view produces a SwiftUI runtime warning, so tests that drive + /// the keycap's styling directly inject the container instead. Nil in the + /// app, where the environment supplies it. + var servicesOverride: ServiceContainer? + + var services: ServiceContainer { servicesOverride ?? environmentServices } /// Whether mouse is hovering over this key @State var isHovering = false diff --git a/Sources/KeyPathInstallationWizard/UI/InstallationWizardView+Dismissal.swift b/Sources/KeyPathInstallationWizard/UI/InstallationWizardView+Dismissal.swift index 7a77016f2..29d97b55a 100644 --- a/Sources/KeyPathInstallationWizard/UI/InstallationWizardView+Dismissal.swift +++ b/Sources/KeyPathInstallationWizard/UI/InstallationWizardView+Dismissal.swift @@ -32,7 +32,7 @@ public extension InstallationWizardView { } NotificationCenter.default.post(name: .wizardStartupRevalidate, object: nil) - dismiss() + performDismiss() if shouldShowFirstSuccess { onFirstSuccess?() } @@ -50,6 +50,17 @@ public extension InstallationWizardView { refreshTask?.cancel() stopLoginItemsApprovalPolling() - dismiss() + performDismiss() + } + + /// Dismiss through the injected handler when one is present, so a view that + /// was never installed never reads the dismiss action out of the environment. + @MainActor + internal func performDismiss() { + if let dismissHandler { + dismissHandler() + } else { + dismiss() + } } } diff --git a/Sources/KeyPathInstallationWizard/UI/InstallationWizardView.swift b/Sources/KeyPathInstallationWizard/UI/InstallationWizardView.swift index 0de0fc8be..adf3ddd8b 100644 --- a/Sources/KeyPathInstallationWizard/UI/InstallationWizardView.swift +++ b/Sources/KeyPathInstallationWizard/UI/InstallationWizardView.swift @@ -80,22 +80,32 @@ public struct InstallationWizardView: View { /// Used to suppress the global operation overlay to avoid duplicate progress treatments. @State private var hasInlineProgressIndicator: Bool = false + /// Stands in for the SwiftUI dismiss action when this view is exercised + /// outside a rendered hierarchy. Reading `@Environment(\.dismiss)` on a view + /// that was never installed produces a SwiftUI runtime warning, so callers + /// that drive the dismissal path directly supply this instead of letting the + /// environment be touched. Nil in the app, where the environment is real. + let dismissHandler: (@MainActor () -> Void)? + public init(initialPage: WizardPage? = nil, onFirstSuccess: (() -> Void)? = nil) { self.initialPage = initialPage self.onFirstSuccess = onFirstSuccess postStartStateDetector = nil + dismissHandler = nil } init( initialPage: WizardPage? = nil, onFirstSuccess: (() -> Void)? = nil, didShowWelcomePage: Bool = false, - postStartStateDetector: @escaping @MainActor () async -> SystemStateResult + postStartStateDetector: @escaping @MainActor () async -> SystemStateResult, + dismissHandler: (@MainActor () -> Void)? = nil ) { self.initialPage = initialPage self.onFirstSuccess = onFirstSuccess _didShowWelcomePage = State(initialValue: didShowWelcomePage) self.postStartStateDetector = postStartStateDetector + self.dismissHandler = dismissHandler } public var currentFixDescriptionForUI: String? { diff --git a/Tests/KeyPathTests/InstallationWizard/FirstSuccessOnboardingGateTests.swift b/Tests/KeyPathTests/InstallationWizard/FirstSuccessOnboardingGateTests.swift index 1fae965df..c6c76480b 100644 --- a/Tests/KeyPathTests/InstallationWizard/FirstSuccessOnboardingGateTests.swift +++ b/Tests/KeyPathTests/InstallationWizard/FirstSuccessOnboardingGateTests.swift @@ -93,7 +93,11 @@ final class FirstSuccessOnboardingGateTests: XCTestCase { autoFixActions: [], detectionTimestamp: Date() ) - } + }, + // Drive dismissal without touching @Environment: this view is never + // installed in a hierarchy, and reading the environment's dismiss + // action on an uninstalled view emits a SwiftUI runtime warning. + dismissHandler: {} ) view.stateMachine.updateWizardState(.serviceNotRunning, issues: []) @@ -129,7 +133,11 @@ final class FirstSuccessOnboardingGateTests: XCTestCase { autoFixActions: [], detectionTimestamp: Date() ) - } + }, + // Drive dismissal without touching @Environment: this view is never + // installed in a hierarchy, and reading the environment's dismiss + // action on an uninstalled view emits a SwiftUI runtime warning. + dismissHandler: {} ) await view.refreshPostStartStateAndDismiss() diff --git a/Tests/KeyPathTests/Services/UnmappedLayerKeyStyleTests.swift b/Tests/KeyPathTests/Services/UnmappedLayerKeyStyleTests.swift index 7aa7305bb..f04a4e871 100644 --- a/Tests/KeyPathTests/Services/UnmappedLayerKeyStyleTests.swift +++ b/Tests/KeyPathTests/Services/UnmappedLayerKeyStyleTests.swift @@ -37,7 +37,10 @@ final class UnmappedLayerKeyStyleTests: KeyPathTestCase { scale: 1.0, currentLayerName: layer, layerKeyInfo: info, - zoneSubtitle: zoneSubtitle + zoneSubtitle: zoneSubtitle, + // Inject the container rather than reading @Environment: this view is + // never installed in a hierarchy, and an uninstalled read warns. + servicesOverride: ServiceContainer() ) } @@ -55,7 +58,10 @@ final class UnmappedLayerKeyStyleTests: KeyPathTestCase { scale: 1.0, currentLayerName: layer, layerKeyInfo: info, - zoneSubtitle: zoneSubtitle + zoneSubtitle: zoneSubtitle, + // Inject the container rather than reading @Environment: this view is + // never installed in a hierarchy, and an uninstalled read warns. + servicesOverride: ServiceContainer() ) } diff --git a/Tests/KeyPathTests/UI/LayerSelectorTests.swift b/Tests/KeyPathTests/UI/LayerSelectorTests.swift index f5666a225..4066f19f6 100644 --- a/Tests/KeyPathTests/UI/LayerSelectorTests.swift +++ b/Tests/KeyPathTests/UI/LayerSelectorTests.swift @@ -243,7 +243,7 @@ extension LayerSelectorTests { } } -private final class StubRuntimeCoordinator: RuntimeCoordinator { +private final class StubRuntimeCoordinator: RuntimeCoordinator, @unchecked Sendable { var stubLayerNames: [String] = [] var stubChangeLayerResult: Bool = false diff --git a/Tests/KeyPathTests/VallackOverlayZoneTests.swift b/Tests/KeyPathTests/VallackOverlayZoneTests.swift index c749d8429..88b412d4f 100644 --- a/Tests/KeyPathTests/VallackOverlayZoneTests.swift +++ b/Tests/KeyPathTests/VallackOverlayZoneTests.swift @@ -161,7 +161,10 @@ final class VallackOverlayZoneTests: XCTestCase { baseLabel: "Q", isPressed: true, scale: 1.0, - zoneColor: Color.blue.opacity(0.45) + zoneColor: Color.blue.opacity(0.45), + // Inject the container rather than reading @Environment: this view is + // never installed in a hierarchy, and an uninstalled read warns. + servicesOverride: ServiceContainer() ) let bg = view.backgroundColor XCTAssertEqual( @@ -178,7 +181,10 @@ final class VallackOverlayZoneTests: XCTestCase { isPressed: false, scale: 1.0, isOneShot: true, - zoneColor: Color.blue.opacity(0.45) + zoneColor: Color.blue.opacity(0.45), + // Inject the container rather than reading @Environment: this view is + // never installed in a hierarchy, and an uninstalled read warns. + servicesOverride: ServiceContainer() ) let bg = view.backgroundColor let oneShotColor = Color(red: 0.2, green: 0.7, blue: 0.8) @@ -196,7 +202,10 @@ final class VallackOverlayZoneTests: XCTestCase { baseLabel: "Q", isPressed: false, scale: 1.0, - zoneColor: zoneColor + zoneColor: zoneColor, + // Inject the container rather than reading @Environment: this view is + // never installed in a hierarchy, and an uninstalled read warns. + servicesOverride: ServiceContainer() ) let bg = view.backgroundColor XCTAssertEqual( @@ -215,7 +224,10 @@ final class VallackOverlayZoneTests: XCTestCase { scale: 1.0, currentLayerName: "vallack-nav", layerKeyInfo: .mapped(displayLabel: "←", outputKey: "left", outputKeyCode: 123, collectionId: RuleCollectionIdentifier.vallackNavigation), - zoneColor: zoneColor + zoneColor: zoneColor, + // Inject the container rather than reading @Environment: this view is + // never installed in a hierarchy, and an uninstalled read warns. + servicesOverride: ServiceContainer() ) let bg = view.backgroundColor XCTAssertEqual( @@ -230,7 +242,10 @@ final class VallackOverlayZoneTests: XCTestCase { key: PhysicalKey(keyCode: 4, label: "H", x: 5, y: 2, width: 1, height: 1), baseLabel: "H", isPressed: false, - scale: 1.0 + scale: 1.0, + // Inject the container rather than reading @Environment: this view is + // never installed in a hierarchy, and an uninstalled read warns. + servicesOverride: ServiceContainer() ) let bg = view.backgroundColor let defaultAlpha = GMKColorway.default.alphaBaseColor From f2fac85caac4c81db020b9efbce949b7cd1e39df Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Tue, 15 Sep 2026 11:39:12 -0700 Subject: [PATCH 5/5] Silence the last State read and satisfy the formatter Two loose ends from the Xcode 27 warning cleanup. The wizard status overview read @State from a test that never installs it in a view hierarchy, which SwiftUI warns about and which hands back a fresh instance on every read. It takes the same injected-value treatment as the other two views: tests supply the value, the app still uses real state. The formatter also has to be satisfied on every file this branch touches, not only on the lines it changed. An earlier attempt reverted that whole-file formatting to keep the diff tight, which was the wrong call: the code-quality gate lints entire changed files, so the reformatting is required rather than incidental churn. The Sendable notes are doc comments now, which is what the docComments rule wants ahead of a declaration. 129 tests pass across every affected suite with zero warnings, and swiftformat reports nothing left to format. Co-Authored-By: Claude Opus 5 --- Sources/KeyPathAppKit/CLI/RulesFacade.swift | 8 ++- .../Managers/HelperMaintenance.swift | 24 +++++--- .../Managers/KanataDaemonManager.swift | 16 +++--- .../Managers/RuntimeCoordinator.swift | 28 ++++++---- .../Managers/UninstallCoordinator.swift | 12 ++-- .../PermissionRequestService.swift | 12 ++-- .../Services/System/SystemValidator.swift | 44 ++++++++++----- .../UI/Overlay/OverlayKeycapView.swift | 56 ++++++++++++++----- Sources/KeyPathCLICommon/CLIError.swift | 4 +- .../WizardSystemStatusOverview.swift | 26 +++++++-- .../UI/InstallationWizardView.swift | 16 ++++-- .../UI/Pages/WizardKanataServicePage.swift | 24 ++++++-- ...tusOverviewPermissionVisibilityTests.swift | 6 +- 13 files changed, 192 insertions(+), 84 deletions(-) diff --git a/Sources/KeyPathAppKit/CLI/RulesFacade.swift b/Sources/KeyPathAppKit/CLI/RulesFacade.swift index b16d6b068..e6d4faea5 100644 --- a/Sources/KeyPathAppKit/CLI/RulesFacade.swift +++ b/Sources/KeyPathAppKit/CLI/RulesFacade.swift @@ -195,7 +195,9 @@ public struct RulesFacade: Sendable { // never reports a rule as created that can never be applied. let conflicts = await collectionConflicts(adding: rules, baseline: baseline) if !conflicts.isEmpty { - if onConflict == .skip { return .skipped } + if onConflict == .skip { + return .skipped + } throw CLICollectionConflictError( input: input, conflicts: conflicts, ruleName: rule.displayTitle ) @@ -225,7 +227,9 @@ public struct RulesFacade: Sendable { var rules = await store.loadRules() let before = rules.count rules.removeAll { $0.input == input } - if rules.count == before { return false } + if rules.count == before { + return false + } try await store.saveRules(rules) return true } diff --git a/Sources/KeyPathAppKit/Managers/HelperMaintenance.swift b/Sources/KeyPathAppKit/Managers/HelperMaintenance.swift index d7ee45a86..d1dd82f3a 100644 --- a/Sources/KeyPathAppKit/Managers/HelperMaintenance.swift +++ b/Sources/KeyPathAppKit/Managers/HelperMaintenance.swift @@ -31,7 +31,9 @@ public final class HelperMaintenance { /// by `runCleanupAndRepair` so UI callers don't surface them as error text. public var lastErrorLine: String? { for line in logLines.reversed() { - if line.hasPrefix("🧹 Cleanup & Repair") { continue } + if line.hasPrefix("🧹 Cleanup & Repair") { + continue + } if line.hasPrefix("❌") || line.hasPrefix("⚠️") { return line } @@ -248,8 +250,12 @@ public final class HelperMaintenance { paths = Array(Set(paths)) // unique paths.sort { lhs, rhs in - if lhs == "/Applications/KeyPath.app" { return true } - if rhs == "/Applications/KeyPath.app" { return false } + if lhs == "/Applications/KeyPath.app" { + return true + } + if rhs == "/Applications/KeyPath.app" { + return false + } return lhs < rhs } return paths @@ -471,10 +477,10 @@ extension HelperMaintenance { } } -// Sendable is already required of this type: the wizard protocol it conforms to -// in WizardProtocolConformances.swift inherits Sendable, so the conformance has -// been in force all along. Swift 6 requires it to be declared alongside the -// class rather than in the conformance file, so state it here. This records the -// existing guarantee where the compiler wants it; it is not a new claim about -// this type's thread safety. +/// Sendable is already required of this type: the wizard protocol it conforms to +/// in WizardProtocolConformances.swift inherits Sendable, so the conformance has +/// been in force all along. Swift 6 requires it to be declared alongside the +/// class rather than in the conformance file, so state it here. This records the +/// existing guarantee where the compiler wants it; it is not a new claim about +/// this type's thread safety. extension HelperMaintenance: @unchecked Sendable {} diff --git a/Sources/KeyPathAppKit/Managers/KanataDaemonManager.swift b/Sources/KeyPathAppKit/Managers/KanataDaemonManager.swift index 7f3963ca8..256ee81ad 100644 --- a/Sources/KeyPathAppKit/Managers/KanataDaemonManager.swift +++ b/Sources/KeyPathAppKit/Managers/KanataDaemonManager.swift @@ -208,7 +208,9 @@ public class KanataDaemonManager { /// - Returns: true if SMAppService reports `.enabled` OR launchctl has the job nonisolated func isInstalled() async -> Bool { let smStatus = await systemStateProvider.cachedSMAppServiceStatus(for: Self.kanataPlistName) - if smStatus == .enabled { return true } + if smStatus == .enabled { + return true + } let evidence = await systemStateProvider.launchctlPrint(target: "system/\(Self.kanataServiceID)") if evidence.exitCode == 0 { @@ -796,10 +798,10 @@ private extension KanataDaemonManager { } } -// Sendable is already required of this type: the wizard protocol it conforms to -// in WizardProtocolConformances.swift inherits Sendable, so the conformance has -// been in force all along. Swift 6 requires it to be declared alongside the -// class rather than in the conformance file, so state it here. This records the -// existing guarantee where the compiler wants it; it is not a new claim about -// this type's thread safety. +/// Sendable is already required of this type: the wizard protocol it conforms to +/// in WizardProtocolConformances.swift inherits Sendable, so the conformance has +/// been in force all along. Swift 6 requires it to be declared alongside the +/// class rather than in the conformance file, so state it here. This records the +/// existing guarantee where the compiler wants it; it is not a new claim about +/// this type's thread safety. extension KanataDaemonManager: @unchecked Sendable {} diff --git a/Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift b/Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift index 537d0614b..eaa390935 100644 --- a/Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift +++ b/Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift @@ -1229,9 +1229,15 @@ public class RuntimeCoordinator: SaveCoordinatorDelegate { isIntentionalTransition: Bool, isRecovering: Bool ) -> GrabRecoveryGate { - if active { return .recordSuccess } - if isIntentionalTransition { return .suppressedDuringTransition } - if isRecovering { return .suppressedRecoveryInFlight } + if active { + return .recordSuccess + } + if isIntentionalTransition { + return .suppressedDuringTransition + } + if isRecovering { + return .suppressedRecoveryInFlight + } return .evaluate } @@ -1450,7 +1456,9 @@ public class RuntimeCoordinator: SaveCoordinatorDelegate { try await mutateAppKeymaps(store: store) { keymaps in guard let index = keymaps.firstIndex(where: { $0.mapping.bundleIdentifier == bundleIdentifier }) else { return } keymaps[index].overrides.removeAll { $0.id == overrideID } - if keymaps[index].overrides.isEmpty { keymaps.remove(at: index) } + if keymaps[index].overrides.isEmpty { + keymaps.remove(at: index) + } } } @@ -1761,10 +1769,10 @@ struct ReloadResult { } } -// Sendable is already required of this type: the wizard protocol it conforms to -// in WizardProtocolConformances.swift inherits Sendable, so the conformance has -// been in force all along. Swift 6 requires it to be declared alongside the -// class rather than in the conformance file, so state it here. This records the -// existing guarantee where the compiler wants it; it is not a new claim about -// this type's thread safety. +/// Sendable is already required of this type: the wizard protocol it conforms to +/// in WizardProtocolConformances.swift inherits Sendable, so the conformance has +/// been in force all along. Swift 6 requires it to be declared alongside the +/// class rather than in the conformance file, so state it here. This records the +/// existing guarantee where the compiler wants it; it is not a new claim about +/// this type's thread safety. extension RuntimeCoordinator: @unchecked Sendable {} diff --git a/Sources/KeyPathAppKit/Managers/UninstallCoordinator.swift b/Sources/KeyPathAppKit/Managers/UninstallCoordinator.swift index 51e13d9a5..d6e480db8 100644 --- a/Sources/KeyPathAppKit/Managers/UninstallCoordinator.swift +++ b/Sources/KeyPathAppKit/Managers/UninstallCoordinator.swift @@ -619,10 +619,10 @@ struct AppleScriptResult { // NOTE: AppleScriptRunner was removed - now using PrivilegedCommandRunner which respects // TestEnvironment.useSudoForPrivilegedOps for sudo-based execution in test environments. -// Sendable is already required of this type: the wizard protocol it conforms to -// in WizardProtocolConformances.swift inherits Sendable, so the conformance has -// been in force all along. Swift 6 requires it to be declared alongside the -// class rather than in the conformance file, so state it here. This records the -// existing guarantee where the compiler wants it; it is not a new claim about -// this type's thread safety. +/// Sendable is already required of this type: the wizard protocol it conforms to +/// in WizardProtocolConformances.swift inherits Sendable, so the conformance has +/// been in force all along. Swift 6 requires it to be declared alongside the +/// class rather than in the conformance file, so state it here. This records the +/// existing guarantee where the compiler wants it; it is not a new claim about +/// this type's thread safety. extension UninstallCoordinator: @unchecked Sendable {} diff --git a/Sources/KeyPathAppKit/Services/Permissions/PermissionRequestService.swift b/Sources/KeyPathAppKit/Services/Permissions/PermissionRequestService.swift index 3cc9cbf8b..7adf702d1 100644 --- a/Sources/KeyPathAppKit/Services/Permissions/PermissionRequestService.swift +++ b/Sources/KeyPathAppKit/Services/Permissions/PermissionRequestService.swift @@ -176,10 +176,10 @@ public final class PermissionRequestService { } } -// Sendable is already required of this type: the wizard protocol it conforms to -// in WizardProtocolConformances.swift inherits Sendable, so the conformance has -// been in force all along. Swift 6 requires it to be declared alongside the -// class rather than in the conformance file, so state it here. This records the -// existing guarantee where the compiler wants it; it is not a new claim about -// this type's thread safety. +/// Sendable is already required of this type: the wizard protocol it conforms to +/// in WizardProtocolConformances.swift inherits Sendable, so the conformance has +/// been in force all along. Swift 6 requires it to be declared alongside the +/// class rather than in the conformance file, so state it here. This records the +/// existing guarantee where the compiler wants it; it is not a new claim about +/// this type's thread safety. extension PermissionRequestService: @unchecked Sendable {} diff --git a/Sources/KeyPathAppKit/Services/System/SystemValidator.swift b/Sources/KeyPathAppKit/Services/System/SystemValidator.swift index a98fdfe85..c10d1e427 100644 --- a/Sources/KeyPathAppKit/Services/System/SystemValidator.swift +++ b/Sources/KeyPathAppKit/Services/System/SystemValidator.swift @@ -246,7 +246,9 @@ public class SystemValidator { defer { Self.activeValidations -= 1 } // Respect cancellation before counting - if Task.isCancelled { return Self.makeCancelledSnapshot() } + if Task.isCancelled { + return Self.makeCancelledSnapshot() + } // Only count owner in tests to avoid cross-test interference if !TestEnvironment.isRunningTests || Self.countingOwner == ObjectIdentifier(self) { Self.validationCount += 1 @@ -470,9 +472,15 @@ public class SystemValidator { static func combinedCaptureStatus( _ statuses: [SystemSnapshotCaptureStatus] ) -> SystemSnapshotCaptureStatus { - if statuses.contains(.failed) { return .failed } - if statuses.contains(.cancelled) { return .cancelled } - if statuses.contains(.timedOut) { return .timedOut } + if statuses.contains(.failed) { + return .failed + } + if statuses.contains(.cancelled) { + return .cancelled + } + if statuses.contains(.timedOut) { + return .timedOut + } return .complete } @@ -859,7 +867,9 @@ public class SystemValidator { } private func checkTCPConfiguration() -> Bool { - if TestEnvironment.isRunningTests { return true } + if TestEnvironment.isRunningTests { + return true + } let plistPath = KanataDaemonManager.getActivePlistPath() guard let plistData = FileManager.default.contents(atPath: plistPath) else { @@ -981,7 +991,9 @@ private final class SystemCaptureCompletionState: @unchecked Sendable { func setContinuation(_ continuation: CheckedContinuation) { let completedResult = state.withLock { state -> SystemSnapshot? in - if let result = state.result { return result } + if let result = state.result { + return result + } state.continuation = continuation return nil } @@ -995,7 +1007,9 @@ private final class SystemCaptureCompletionState: @unchecked Sendable { state.operationTask = task return state.result != nil } - if alreadyCompleted { task.cancel() } + if alreadyCompleted { + task.cancel() + } } func setTimeoutTask(_ task: Task) { @@ -1003,7 +1017,9 @@ private final class SystemCaptureCompletionState: @unchecked Sendable { state.timeoutTask = task return state.result != nil } - if alreadyCompleted { task.cancel() } + if alreadyCompleted { + task.cancel() + } } func complete(with result: SystemSnapshot) -> Bool { @@ -1026,10 +1042,10 @@ private final class SystemCaptureCompletionState: @unchecked Sendable { } } -// Sendable is already required of this type: the wizard protocol it conforms to -// in WizardProtocolConformances.swift inherits Sendable, so the conformance has -// been in force all along. Swift 6 requires it to be declared alongside the -// class rather than in the conformance file, so state it here. This records the -// existing guarantee where the compiler wants it; it is not a new claim about -// this type's thread safety. +/// Sendable is already required of this type: the wizard protocol it conforms to +/// in WizardProtocolConformances.swift inherits Sendable, so the conformance has +/// been in force all along. Swift 6 requires it to be declared alongside the +/// class rather than in the conformance file, so state it here. This records the +/// existing guarantee where the compiler wants it; it is not a new claim about +/// this type's thread safety. extension SystemValidator: @unchecked Sendable {} diff --git a/Sources/KeyPathAppKit/UI/Overlay/OverlayKeycapView.swift b/Sources/KeyPathAppKit/UI/Overlay/OverlayKeycapView.swift index 9a3b5121a..20efcbad3 100644 --- a/Sources/KeyPathAppKit/UI/Overlay/OverlayKeycapView.swift +++ b/Sources/KeyPathAppKit/UI/Overlay/OverlayKeycapView.swift @@ -118,12 +118,18 @@ struct OverlayKeycapView: View { /// info, or an explicit `XX` blocker with no label/output. var isVisuallyUnmappedLayerKey: Bool { guard let info = layerKeyInfo else { return true } - if info.isTransparent { return true } - if info.isLayerSwitch { return false } + if info.isTransparent { + return true + } + if info.isLayerSwitch { + return false + } if info.appLaunchIdentifier != nil || info.systemActionIdentifier != nil || info.urlIdentifier != nil { return false } - if isNavIdentityMapping { return false } + if isNavIdentityMapping { + return false + } if key.layoutRole == .narrowModifier { return true } @@ -152,11 +158,21 @@ struct OverlayKeycapView: View { return false } // Has a mapping if it's not transparent and has actual content - if info.isTransparent { return false } - if info.isLayerSwitch { return true } - if info.appLaunchIdentifier != nil { return true } - if info.systemActionIdentifier != nil { return true } - if info.urlIdentifier != nil { return true } + if info.isTransparent { + return false + } + if info.isLayerSwitch { + return true + } + if info.appLaunchIdentifier != nil { + return true + } + if info.systemActionIdentifier != nil { + return true + } + if info.urlIdentifier != nil { + return true + } // Check if output differs from input (not identity mapping) if let outputKey = info.outputKey { return outputKey.lowercased() != inputKeyName @@ -218,13 +234,19 @@ struct OverlayKeycapView: View { } // Special keys always render their own content - if hasSpecialLabel { return true } + if hasSpecialLabel { + return true + } // If there's a nav overlay symbol, render it (arrow only, letter handled by floating label) - if navOverlaySymbol != nil { return true } + if navOverlaySymbol != nil { + return true + } // Nav identity mappings render their own centered label - if isNavIdentityMapping { return true } + if isNavIdentityMapping { + return true + } // If key is remapped to a different output, render the label directly // (floating labels only exist for base layout characters like A-Z, not for mapped outputs) @@ -299,8 +321,12 @@ struct OverlayKeycapView: View { /// Whether the overlay should fall back to the base label (keymap or physical) var shouldUseBaseLabel: Bool { guard let info = layerKeyInfo else { return true } - if info.isTransparent { return true } - if info.isLayerSwitch { return false } + if info.isTransparent { + return true + } + if info.isLayerSwitch { + return false + } if info.appLaunchIdentifier != nil || info.systemActionIdentifier != nil || info.urlIdentifier != nil { return false } @@ -341,7 +367,9 @@ struct OverlayKeycapView: View { /// app, where the environment supplies it. var servicesOverride: ServiceContainer? - var services: ServiceContainer { servicesOverride ?? environmentServices } + var services: ServiceContainer { + servicesOverride ?? environmentServices + } /// Whether mouse is hovering over this key @State var isHovering = false diff --git a/Sources/KeyPathCLICommon/CLIError.swift b/Sources/KeyPathCLICommon/CLIError.swift index 148692422..20634f160 100644 --- a/Sources/KeyPathCLICommon/CLIError.swift +++ b/Sources/KeyPathCLICommon/CLIError.swift @@ -37,7 +37,9 @@ public enum CLIDocsURL { public extension CLIError { static func notFound(_ entity: String, query: String, listCommand: String, suggestions: [String] = []) -> CLIError { var hint = "Run '\(listCommand)' to see available \(entity.lowercased())s" - if !suggestions.isEmpty { hint = "Did you mean: \(suggestions.joined(separator: ", "))?\n" + hint } + if !suggestions.isEmpty { + hint = "Did you mean: \(suggestions.joined(separator: ", "))?\n" + hint + } return CLIError(code: .notFound, message: "\(entity) not found: '\(query)'", hint: hint, details: ["query: '\(query)'"], docsUrl: nil) } diff --git a/Sources/KeyPathInstallationWizard/UI/Components/WizardSystemStatusOverview.swift b/Sources/KeyPathInstallationWizard/UI/Components/WizardSystemStatusOverview.swift index 676e58a9d..d151e9d0a 100644 --- a/Sources/KeyPathInstallationWizard/UI/Components/WizardSystemStatusOverview.swift +++ b/Sources/KeyPathInstallationWizard/UI/Components/WizardSystemStatusOverview.swift @@ -19,6 +19,18 @@ public struct WizardSystemStatusOverview: View { @Binding public var visibleIssueCount: Int @State private var duplicateCopies: [String] = [] + + /// Stands in for the detected copies when this view is evaluated outside a + /// rendered hierarchy. Reading `@State` on an uninstalled view produces a + /// SwiftUI runtime warning and yields a fresh instance each time, so tests + /// that read the status items directly supply the value instead. Nil in the + /// app, where the state is real. + private let duplicateCopiesOverride: [String]? + + private var resolvedDuplicateCopies: [String] { + duplicateCopiesOverride ?? duplicateCopies + } + /// Cache heavy probes so SwiftUI re-renders don’t hammer the filesystem/network private static var cache = ProbeCache() @@ -29,7 +41,8 @@ public struct WizardSystemStatusOverview: View { kanataIsRunning: Bool, showAllItems: Bool, navSequence: Binding<[WizardPage]>, - visibleIssueCount: Binding + visibleIssueCount: Binding, + duplicateCopiesOverride: [String]? = nil ) { self.systemState = systemState self.issues = issues @@ -38,6 +51,7 @@ public struct WizardSystemStatusOverview: View { self.showAllItems = showAllItems _navSequence = navSequence _visibleIssueCount = visibleIssueCount + self.duplicateCopiesOverride = duplicateCopiesOverride } public var body: some View { @@ -165,7 +179,7 @@ public struct WizardSystemStatusOverview: View { } return issueStatus(for: helperIssues) }() - let helperSubtitle: String? = duplicateCopies.count > 1 ? "Multiple app copies detected" : nil + let helperSubtitle: String? = resolvedDuplicateCopies.count > 1 ? "Multiple app copies detected" : nil items.append( StatusItemModel( id: "privileged-helper", @@ -432,7 +446,9 @@ public struct WizardSystemStatusOverview: View { public static func filteredDisplayItems(_ items: [StatusItemModel], showAllItems: Bool) -> [StatusItemModel] { - if showAllItems { return items } + if showAllItems { + return items + } // Show incomplete items. Treat .unverified as "complete enough" - we can't verify it, // so don't alarm the user with it in the issues list. return items.filter { $0.status != .completed && $0.status != .unverified } @@ -593,7 +609,9 @@ public struct WizardSystemStatusOverview: View { private func getCommunicationServerStatus() -> InstallationStatus { // Keep this lightweight on the UI thread: if Kanata is running, assume comm server is available. // Detailed TCP health is validated elsewhere by InstallerEngine. - if systemState == .initializing { return .notStarted } + if systemState == .initializing { + return .notStarted + } return kanataIsRunning ? .completed : .notStarted } diff --git a/Sources/KeyPathInstallationWizard/UI/InstallationWizardView.swift b/Sources/KeyPathInstallationWizard/UI/InstallationWizardView.swift index adf3ddd8b..edecf3579 100644 --- a/Sources/KeyPathInstallationWizard/UI/InstallationWizardView.swift +++ b/Sources/KeyPathInstallationWizard/UI/InstallationWizardView.swift @@ -135,7 +135,9 @@ public struct InstallationWizardView: View { } } .onChange(of: isOperationRunning) { _, newValue in - if !newValue { hasKeyboardFocus = true } + if !newValue { + hasKeyboardFocus = true + } } .onChange(of: showAllSummaryItems) { _, showAll in stateMachine.customSequence = showAll ? nil : navSequence @@ -144,10 +146,14 @@ public struct InstallationWizardView: View { handlePageChange(from: oldPage, to: newPage) } .onChange(of: navSequence) { _, newSeq in - if !showAllSummaryItems { stateMachine.customSequence = newSeq } + if !showAllSummaryItems { + stateMachine.customSequence = newSeq + } } .onChange(of: showingCloseConfirmation) { _, newValue in - if !newValue { hasKeyboardFocus = true } + if !newValue { + hasKeyboardFocus = true + } } .modifier( KeyboardNavigationModifier( @@ -172,7 +178,9 @@ public struct InstallationWizardView: View { Text("Login Items will open. Find KeyPath under Background Items and flip the switch to enable it.") } .onChange(of: showingBackgroundApprovalPrompt) { _, isShowing in - if isShowing { startLoginItemsApprovalPolling() } + if isShowing { + startLoginItemsApprovalPolling() + } } } diff --git a/Sources/KeyPathInstallationWizard/UI/Pages/WizardKanataServicePage.swift b/Sources/KeyPathInstallationWizard/UI/Pages/WizardKanataServicePage.swift index 5105d33ef..2e2087c02 100644 --- a/Sources/KeyPathInstallationWizard/UI/Pages/WizardKanataServicePage.swift +++ b/Sources/KeyPathInstallationWizard/UI/Pages/WizardKanataServicePage.swift @@ -455,12 +455,24 @@ public struct WizardKanataServicePage: View { guard hasErrorSignal else { return false } // Ignore known non-fatal runtime noise that should not drive wizard crash UI. - if lower.contains("error writing reloadresult: broken pipe") { return false } - if lower.contains("broken pipe (os error 32)") { return false } - if lower.contains("connection reset by peer") { return false } - if lower.contains("client sent an invalid message") { return false } - if lower.contains("iohiddeviceopen error: (iokit/common) exclusive access and device already open") { return false } - if lower.contains("iohiddeviceopen error: (iokit/common) not permitted apple internal keyboard / trackpad") { return false } + if lower.contains("error writing reloadresult: broken pipe") { + return false + } + if lower.contains("broken pipe (os error 32)") { + return false + } + if lower.contains("connection reset by peer") { + return false + } + if lower.contains("client sent an invalid message") { + return false + } + if lower.contains("iohiddeviceopen error: (iokit/common) exclusive access and device already open") { + return false + } + if lower.contains("iohiddeviceopen error: (iokit/common) not permitted apple internal keyboard / trackpad") { + return false + } return true } diff --git a/Tests/KeyPathTests/InstallationWizard/WizardSystemStatusOverviewPermissionVisibilityTests.swift b/Tests/KeyPathTests/InstallationWizard/WizardSystemStatusOverviewPermissionVisibilityTests.swift index b6be2f5fd..26e512e81 100644 --- a/Tests/KeyPathTests/InstallationWizard/WizardSystemStatusOverviewPermissionVisibilityTests.swift +++ b/Tests/KeyPathTests/InstallationWizard/WizardSystemStatusOverviewPermissionVisibilityTests.swift @@ -25,7 +25,11 @@ final class WizardSystemStatusOverviewPermissionVisibilityTests: XCTestCase { kanataIsRunning: false, showAllItems: true, navSequence: .constant(nav), - visibleIssueCount: .constant(visible) + visibleIssueCount: .constant(visible), + // Supply the value rather than reading @State: this view is never + // installed in a hierarchy, and an uninstalled read warns and hands + // back a fresh instance each time. + duplicateCopiesOverride: [] ) let items = overview.statusItems