diff --git a/Sources/KeyPathAppKit/Core/PrivilegedOperationsRouter.swift b/Sources/KeyPathAppKit/Core/PrivilegedOperationsRouter.swift index a53b3a0cc..e5a36dc0e 100644 --- a/Sources/KeyPathAppKit/Core/PrivilegedOperationsRouter.swift +++ b/Sources/KeyPathAppKit/Core/PrivilegedOperationsRouter.swift @@ -95,6 +95,22 @@ public final class PrivilegedOperationsRouter { case .privilegedHelper: do { try await helperManager.installRequiredRuntimeServices() + // Stale-helper guard (MAL-57): a resident helper from before + // the ProcessType=Interactive template fix reports success but + // rewrites the old plist — the helper binary only refreshes on + // process restart, so it can lag the app across an update. + // Verify the result and redo the rewrite via the in-app sudo + // path (ServiceBootstrapper + current PlistGenerator, with its + // own configured-postflight). Must NOT route back through the + // helper — sudoInstallRequiredRuntimeServices would, via + // InstallerEngine → PrivilegeBroker → this router's + // helper-first repairVHIDDaemonServices. + if serviceHealthChecker.isVHIDDaemonPlistPresentButMisconfigured() { + AppLogger.shared.warn( + "⚠️ [PrivilegedOperationsRouter] VHID daemon plist still misconfigured after helper install — stale helper template, rewriting via in-app sudo path" + ) + try await sudoRepairVHIDServices() + } } catch { try await sudoInstallRequiredRuntimeServices() } diff --git a/Sources/KeyPathAppKit/Services/System/SystemValidator.swift b/Sources/KeyPathAppKit/Services/System/SystemValidator.swift index 5662108b4..4b95c2003 100644 --- a/Sources/KeyPathAppKit/Services/System/SystemValidator.swift +++ b/Sources/KeyPathAppKit/Services/System/SystemValidator.swift @@ -47,6 +47,7 @@ public class SystemValidator { let vhidInstalled: Bool let vhidVersionMismatch: Bool let karabinerDriverExtensionEnabled: Bool + let vhidDaemonPlistMisconfigured: Bool let timestamp: Date func isFresh(ttl: TimeInterval) -> Bool { @@ -432,7 +433,7 @@ public class SystemValidator { AppLogger.shared .log( - "🔍 [SystemValidator] Components: kanata=\(facts.kanataBinaryInstalled), driver=\(karabinerDriverInstalled), daemon=\(karabinerDaemonRunning), vhid=\(vhidHealthy), vhidServices=\(vhidServicesHealthy), vhidVersionMismatch=\(facts.vhidVersionMismatch)" + "🔍 [SystemValidator] Components: kanata=\(facts.kanataBinaryInstalled), driver=\(karabinerDriverInstalled), daemon=\(karabinerDaemonRunning), vhid=\(vhidHealthy), vhidServices=\(vhidServicesHealthy), vhidPlistMisconfigured=\(facts.vhidDaemonPlistMisconfigured), vhidVersionMismatch=\(facts.vhidVersionMismatch)" ) return ComponentStatus( @@ -442,6 +443,7 @@ public class SystemValidator { vhidDeviceInstalled: facts.vhidInstalled, vhidDeviceHealthy: vhidHealthy, vhidServicesHealthy: vhidServicesHealthy, + vhidDaemonPlistMisconfigured: facts.vhidDaemonPlistMisconfigured, vhidVersionMismatch: facts.vhidVersionMismatch ) } @@ -474,11 +476,22 @@ public class SystemValidator { "⏱️ [TIMING] SystemValidator.checkComponents Karabiner extension check completed in \(String(format: "%.3f", Date().timeIntervalSince(karabinerStart)))s" ) + // A VHID daemon plist from before the MAL-57 fix (missing + // ProcessType=Interactive) keeps the daemon running day-to-day but + // leaves it vulnerable to starvation under CPU load (stuck-key + // autorepeat). Surface it so the wizard offers a one-click repair + // that rewrites the plist — old installs never migrate otherwise. + // Lives in the cached facts so the plist read doesn't run on every + // validation cycle; invalidateCaches() refreshes it after repairs. + let vhidDaemonPlistMisconfigured = + ServiceHealthChecker.shared.isVHIDDaemonPlistPresentButMisconfigured() + let facts = ComponentInstallationFacts( kanataBinaryInstalled: kanataBinaryInstalled, vhidInstalled: vhidInstalled, vhidVersionMismatch: vhidVersionMismatch, karabinerDriverExtensionEnabled: karabinerDriverExtensionEnabled, + vhidDaemonPlistMisconfigured: vhidDaemonPlistMisconfigured, timestamp: Date() ) cachedComponentFacts = facts diff --git a/Sources/KeyPathInstallationWizard/Core/ActionDeterminer.swift b/Sources/KeyPathInstallationWizard/Core/ActionDeterminer.swift index d1585c6a1..d0bc45db6 100644 --- a/Sources/KeyPathInstallationWizard/Core/ActionDeterminer.swift +++ b/Sources/KeyPathInstallationWizard/Core/ActionDeterminer.swift @@ -53,7 +53,9 @@ public enum ActionDeterminer { } } - if !context.components.vhidServicesHealthy { + // Covers a pre-MAL-57 plist (missing ProcessType=Interactive) too: + // it needs the rewrite even when the daemon is otherwise healthy. + if context.components.vhidRuntimeServicesNeedRepair { actions.append(.installRequiredRuntimeServices) } @@ -115,7 +117,7 @@ public enum ActionDeterminer { actions.append(.installPrivilegedHelper) } - if !context.components.vhidServicesHealthy { + if context.components.vhidRuntimeServicesNeedRepair { actions.append(.installRequiredRuntimeServices) } diff --git a/Sources/KeyPathInstallationWizard/Core/ServiceHealthChecker.swift b/Sources/KeyPathInstallationWizard/Core/ServiceHealthChecker.swift index 984c843a5..780062e18 100644 --- a/Sources/KeyPathInstallationWizard/Core/ServiceHealthChecker.swift +++ b/Sources/KeyPathInstallationWizard/Core/ServiceHealthChecker.swift @@ -889,22 +889,46 @@ public final class ServiceHealthChecker: @unchecked Sendable { ) return false } + return Self.vhidDaemonPlistContentIsValid(dict) + } - let expectedPath = - "/Library/Application Support/org.pqrs/Karabiner-DriverKit-VirtualHIDDevice/Applications/Karabiner-VirtualHIDDevice-Daemon.app/Contents/MacOS/Karabiner-VirtualHIDDevice-Daemon" + /// Whether an installed VHID daemon plist predates the MAL-57 fix and needs rewriting. + /// + /// Presence-gated: a missing plist returns `false` — that situation is + /// "services not installed" and is already surfaced via service health. + /// A plist that exists but has stale content (wrong program path or + /// missing ProcessType=Interactive) counts as misconfigured, so existing + /// installs migrate via a wizard repair. A present-but-unparseable plist + /// also counts: repair rewrites it either way. + /// + /// - Returns: `true` if the plist exists but is misconfigured + public func isVHIDDaemonPlistPresentButMisconfigured() -> Bool { + let plistPath = getPlistPath(for: Self.vhidDaemonServiceID) + guard let dict = NSDictionary(contentsOfFile: plistPath) as? [String: Any] else { + return Foundation.FileManager().fileExists(atPath: plistPath) + } + return !Self.vhidDaemonPlistContentIsValid(dict) + } - if let args = dict["ProgramArguments"] as? [String], let first = args.first { - let pathOK = first == expectedPath - let processTypeOK = (dict["ProcessType"] as? String) == "Interactive" + /// Shared content check for the VHID daemon plist: DriverKit daemon path + /// in ProgramArguments plus the MAL-57 ProcessType=Interactive key. + /// Logs only when the content is invalid — this runs on every validation + /// cycle via SystemValidator, not just during install/repair. + private static func vhidDaemonPlistContentIsValid(_ dict: [String: Any]) -> Bool { + guard let args = dict["ProgramArguments"] as? [String], let first = args.first else { + AppLogger.shared.log( + "🔍 [ServiceHealthChecker] VHID plist ProgramArguments missing or malformed" + ) + return false + } + let pathOK = first == PlistGenerator.vhidDaemonPath + let processTypeOK = (dict["ProcessType"] as? String) == "Interactive" + if !pathOK || !processTypeOK { AppLogger.shared.log( "🔍 [ServiceHealthChecker] VHID plist ProgramArguments[0]=\(first) | pathOK=\(pathOK) | processTypeOK=\(processTypeOK)" ) - return pathOK && processTypeOK } - AppLogger.shared.log( - "🔍 [ServiceHealthChecker] VHID plist ProgramArguments missing or malformed" - ) - return false + return pathOK && processTypeOK } // MARK: - Private Helpers diff --git a/Sources/KeyPathInstallationWizard/Core/SystemInspector.swift b/Sources/KeyPathInstallationWizard/Core/SystemInspector.swift index f5b6dd413..01b050ea3 100644 --- a/Sources/KeyPathInstallationWizard/Core/SystemInspector.swift +++ b/Sources/KeyPathInstallationWizard/Core/SystemInspector.swift @@ -256,6 +256,24 @@ public enum SystemInspector { userAction: nil )) } + // Gated on healthy services: when they're down, the issue above + // already carries the same fix and the rewrite happens anyway. + // Severity .warning, not .error: the daemon still works day-to-day, + // so this must nudge toward a one-click migration without flipping + // the whole app to a failed state (MainAppStateController counts + // .error issues as blocking). Wizard routing and the fix button are + // identifier-based and severity-agnostic, so the repair path is intact. + if context.components.vhidDaemonPlistMisconfigured, context.components.vhidServicesHealthy { + issues.append(WizardIssue( + identifier: .component(.vhidDaemonMisconfigured), + severity: .warning, + category: .installation, + title: "VHID Daemon Configuration Outdated", + description: "The Karabiner VirtualHID daemon service uses an outdated configuration that can cause stuck or repeating keys under heavy CPU load. Repair reinstalls the service with the corrected settings.", + autoFixAction: .installRequiredRuntimeServices, + userAction: nil + )) + } } // MARK: - Conflict Issues diff --git a/Sources/KeyPathWizardCore/SystemSnapshot.swift b/Sources/KeyPathWizardCore/SystemSnapshot.swift index 5e362a4b3..1f5722961 100644 --- a/Sources/KeyPathWizardCore/SystemSnapshot.swift +++ b/Sources/KeyPathWizardCore/SystemSnapshot.swift @@ -183,6 +183,11 @@ public struct ComponentStatus: Sendable { /// VHID services health (daemon + manager) independent of Kanata service /// Use for Karabiner Components page which should only care about VHID, not Kanata public let vhidServicesHealthy: Bool + /// True when the installed VHID daemon plist exists but predates the + /// MAL-57 fix (wrong program path or missing ProcessType=Interactive). + /// The daemon may be running fine, but it is vulnerable to starvation + /// under CPU load and the plist should be rewritten via repair. + public let vhidDaemonPlistMisconfigured: Bool public let vhidVersionMismatch: Bool public init( @@ -192,6 +197,7 @@ public struct ComponentStatus: Sendable { vhidDeviceInstalled: Bool, vhidDeviceHealthy: Bool, vhidServicesHealthy: Bool, + vhidDaemonPlistMisconfigured: Bool = false, vhidVersionMismatch: Bool ) { self.kanataBinaryInstalled = kanataBinaryInstalled @@ -200,15 +206,27 @@ public struct ComponentStatus: Sendable { self.vhidDeviceInstalled = vhidDeviceInstalled self.vhidDeviceHealthy = vhidDeviceHealthy self.vhidServicesHealthy = vhidServicesHealthy + self.vhidDaemonPlistMisconfigured = vhidDaemonPlistMisconfigured self.vhidVersionMismatch = vhidVersionMismatch } /// Required components for the normal split-runtime architecture. + /// Deliberately excludes `vhidDaemonPlistMisconfigured`: a stale plist + /// still runs day-to-day, so it surfaces as a repairable wizard issue + /// rather than a missing component that would fail readiness app-wide. public var hasAllRequired: Bool { kanataBinaryInstalled && karabinerDriverInstalled && karabinerDaemonRunning && vhidDeviceHealthy && vhidServicesHealthy && !vhidVersionMismatch } + /// The VHID runtime services need (re)installation: either they are + /// unhealthy, or the installed daemon plist predates the MAL-57 fix and + /// must be rewritten. Single source for the installRequiredRuntimeServices + /// trigger so repair and install plans cannot drift apart. + public var vhidRuntimeServicesNeedRepair: Bool { + !vhidServicesHealthy || vhidDaemonPlistMisconfigured + } + /// Convenience factory for empty/fallback state public static var empty: ComponentStatus { ComponentStatus( diff --git a/Tests/KeyPathTests/InstallationWizard/WizardPureLogicTests.swift b/Tests/KeyPathTests/InstallationWizard/WizardPureLogicTests.swift index b7abb63a6..7e7f4a66b 100644 --- a/Tests/KeyPathTests/InstallationWizard/WizardPureLogicTests.swift +++ b/Tests/KeyPathTests/InstallationWizard/WizardPureLogicTests.swift @@ -190,6 +190,64 @@ final class WizardPureLogicTests: XCTestCase { XCTAssertEqual(serviceIssue?.autoFixAction, .installRequiredRuntimeServices) } + /// A pre-MAL-57 plist (missing ProcessType=Interactive) on an otherwise + /// healthy system must surface proactively with a one-click repair — + /// old installs never migrate otherwise. + func test_inspect_vhidDaemonPlistMisconfigured_producesMisconfiguredIssueWithAutoFix() { + let components = ComponentStatus( + kanataBinaryInstalled: true, + karabinerDriverInstalled: true, + karabinerDaemonRunning: true, + vhidDeviceInstalled: true, + vhidDeviceHealthy: true, + vhidServicesHealthy: true, + vhidDaemonPlistMisconfigured: true, + vhidVersionMismatch: false + ) + let context = makeContext(components: components) + let (state, issues) = SystemInspector.inspect(context: context) + + let issue = issues.first { $0.identifier == .component(.vhidDaemonMisconfigured) } + XCTAssertNotNil(issue, "Stale VHID daemon plist should generate a misconfigured issue") + XCTAssertEqual(issue?.category, .installation, "Must be .installation so the router sends it to the Karabiner components page") + XCTAssertEqual( + issue?.severity, .warning, + "Daemon still works — warning nudges migration without flipping the app to a failed state" + ) + XCTAssertEqual(issue?.autoFixAction, .installRequiredRuntimeServices) + + // The daemon is still running — state stays .active; the issue alone + // drives routing and the components-page repair button. + XCTAssertEqual(state, .active) + } + + /// When the VHID services are already down, the unhealthy-services issue + /// carries the same fix — don't double-alarm for one root cause. + func test_inspect_vhidDaemonPlistMisconfiguredAndServicesUnhealthy_emitsSingleIssue() { + let components = ComponentStatus( + kanataBinaryInstalled: true, + karabinerDriverInstalled: true, + karabinerDaemonRunning: true, + vhidDeviceInstalled: true, + vhidDeviceHealthy: true, + vhidServicesHealthy: false, + vhidDaemonPlistMisconfigured: true, + vhidVersionMismatch: false + ) + let context = makeContext(components: components) + let (_, issues) = SystemInspector.inspect(context: context) + + XCTAssertNil( + issues.first { $0.identifier == .component(.vhidDaemonMisconfigured) }, + "Misconfigured-plist issue is suppressed while services are unhealthy" + ) + let serviceIssue = issues.first { $0.identifier == .component(.vhidDeviceManager) } + XCTAssertEqual( + serviceIssue?.autoFixAction, .installRequiredRuntimeServices, + "The unhealthy-services issue carries the same repair" + ) + } + func test_inspect_multipleIssues_allReported() { let components = ComponentStatus( kanataBinaryInstalled: true, @@ -1025,8 +1083,42 @@ final class WizardPureLogicTests: XCTestCase { XCTAssertTrue(actions.contains(.installRequiredRuntimeServices)) } + func test_determineRepairActions_vhidDaemonPlistMisconfigured_includesInstallRuntimeServices() { + let components = ComponentStatus( + kanataBinaryInstalled: true, + karabinerDriverInstalled: true, + karabinerDaemonRunning: true, + vhidDeviceInstalled: true, + vhidDeviceHealthy: true, + vhidServicesHealthy: true, + vhidDaemonPlistMisconfigured: true, + vhidVersionMismatch: false + ) + let context = makeContext(components: components) + let actions = ActionDeterminer.determineRepairActions(context: context) + XCTAssertTrue(actions.contains(.installRequiredRuntimeServices), + "Stale VHID daemon plist needs the runtime-services rewrite even when the daemon is healthy") + } + // MARK: - ActionDeterminer: Install Actions + func test_determineInstallActions_vhidDaemonPlistMisconfigured_includesInstallRuntimeServices() { + let components = ComponentStatus( + kanataBinaryInstalled: true, + karabinerDriverInstalled: true, + karabinerDaemonRunning: true, + vhidDeviceInstalled: true, + vhidDeviceHealthy: true, + vhidServicesHealthy: true, + vhidDaemonPlistMisconfigured: true, + vhidVersionMismatch: false + ) + let context = makeContext(components: components) + let actions = ActionDeterminer.determineInstallActions(context: context) + XCTAssertTrue(actions.contains(.installRequiredRuntimeServices), + "Install plans share the vhidRuntimeServicesNeedRepair trigger with repair plans") + } + func test_determineInstallActions_allHealthy_returnsEmpty() { let context = makeContext() let actions = ActionDeterminer.determineInstallActions(context: context) diff --git a/Tests/KeyPathTests/Services/ServiceHealthCheckerTests.swift b/Tests/KeyPathTests/Services/ServiceHealthCheckerTests.swift index abe4b6b46..31e43fdd5 100644 --- a/Tests/KeyPathTests/Services/ServiceHealthCheckerTests.swift +++ b/Tests/KeyPathTests/Services/ServiceHealthCheckerTests.swift @@ -16,6 +16,11 @@ final class ServiceHealthCheckerTests: XCTestCase { override func setUp() async throws { try await super.setUp() checker = await MainActor.run { ServiceHealthChecker.shared } + // The shared singleton's 2s-TTL health cache can carry entries from + // other suites in the same process (e.g. InstallerEngine tests that + // probed services against a different launch-daemons dir). Clear it + // so plist-existence checks here see this test's temp dir. + checker.invalidateHealthCache() originalSMFactory = KanataDaemonManager.smServiceFactory originalSudoEnv = ProcessInfo.processInfo.environment["KEYPATH_USE_SUDO"] originalAllowAdminOperationsInTests = TestEnvironment.allowAdminOperationsInTests @@ -77,7 +82,10 @@ final class ServiceHealthCheckerTests: XCTestCase { FileManager.default.createFile(atPath: url.path, contents: Data(), attributes: nil) } - private func writeVHIDPlist(programPath: String, processType: String? = "Interactive") throws { + private func writeVHIDPlist( + programPath: String = PlistGenerator.vhidDaemonPath, + processType: String? = "Interactive" + ) throws { var dict: [String: Any] = [ "ProgramArguments": [programPath] ] @@ -262,9 +270,7 @@ final class ServiceHealthCheckerTests: XCTestCase { } func testIsVHIDDaemonConfiguredCorrectlyReadsProgramArguments() throws { - let expectedPath = - "/Library/Application Support/org.pqrs/Karabiner-DriverKit-VirtualHIDDevice/Applications/Karabiner-VirtualHIDDevice-Daemon.app/Contents/MacOS/Karabiner-VirtualHIDDevice-Daemon" - try writeVHIDPlist(programPath: expectedPath) + try writeVHIDPlist() XCTAssertTrue(checker.isVHIDDaemonConfiguredCorrectly()) } @@ -276,9 +282,42 @@ final class ServiceHealthCheckerTests: XCTestCase { /// Plists from before the MAL-57 starvation fix lack ProcessType=Interactive /// and must report misconfigured so repair rewrites them. func testIsVHIDDaemonConfiguredCorrectlyReturnsFalseWithoutProcessType() throws { - let expectedPath = - "/Library/Application Support/org.pqrs/Karabiner-DriverKit-VirtualHIDDevice/Applications/Karabiner-VirtualHIDDevice-Daemon.app/Contents/MacOS/Karabiner-VirtualHIDDevice-Daemon" - try writeVHIDPlist(programPath: expectedPath, processType: nil) + try writeVHIDPlist(processType: nil) XCTAssertFalse(checker.isVHIDDaemonConfiguredCorrectly()) } + + // MARK: - isVHIDDaemonPlistPresentButMisconfigured (proactive MAL-57 migration) + + /// No plist at all is "services not installed", not "misconfigured" — + /// that case is owned by service health, not the plist-content check. + func testVHIDPlistMisconfiguredReturnsFalseWhenPlistMissing() { + XCTAssertFalse(checker.isVHIDDaemonPlistPresentButMisconfigured()) + } + + func testVHIDPlistMisconfiguredReturnsFalseForCorrectPlist() throws { + try writeVHIDPlist() + XCTAssertFalse(checker.isVHIDDaemonPlistPresentButMisconfigured()) + } + + /// A pre-MAL-57 plist (exists, correct path, no ProcessType=Interactive) + /// must report misconfigured so existing installs migrate via repair. + func testVHIDPlistMisconfiguredReturnsTrueWithoutProcessType() throws { + try writeVHIDPlist(processType: nil) + XCTAssertTrue(checker.isVHIDDaemonPlistPresentButMisconfigured()) + } + + func testVHIDPlistMisconfiguredReturnsTrueForWrongProgramPath() throws { + try writeVHIDPlist(programPath: "/wrong/path") + XCTAssertTrue(checker.isVHIDDaemonPlistPresentButMisconfigured()) + } + + /// A present-but-unparseable plist is misconfigured (repair rewrites it), + /// not "missing" — only true absence is owned by service health. + func testVHIDPlistMisconfiguredReturnsTrueForCorruptPlist() throws { + let url = tempLaunchDaemonsDir.appendingPathComponent( + "\(ServiceHealthChecker.vhidDaemonServiceID).plist" + ) + try Data("not a plist".utf8).write(to: url) + XCTAssertTrue(checker.isVHIDDaemonPlistPresentButMisconfigured()) + } }