Skip to content

feat(uninstall): opt-in removal of the VirtualHID driver (#679) #705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
malpern wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(uninstall): opt-in removal of the VirtualHID driver (#679) #705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,8 +589,10 @@
await installerEngine.inspectSystem()
}

func uninstall(deleteConfig: Bool) async -> InstallerReport {
await installerEngine.uninstall(deleteConfig: deleteConfig, using: privilegeBroker)
func uninstall(deleteConfig: Bool, removeVirtualHID: Bool = false) async -> InstallerReport {
await installerEngine.uninstall(
deleteConfig: deleteConfig, removeVirtualHID: removeVirtualHID, using: privilegeBroker
)
}

func runFullRepair(reason: String = "RuntimeCoordinator repair request") async -> InstallerReport {
Expand Down Expand Up @@ -1063,7 +1065,7 @@
}

/// Returns a snapshot of current UI state for ViewModel synchronization
/// This method allows KanataViewModel to read UI state without @Published properties

Check warning on line 1068 in Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift

View workflow job for this annotation

GitHub Actions / code-quality 

Use @observable macro instead of @published. Properties on @observable classes are automatically tracked. (no_published)
func getCurrentUIState() -> KanataUIState {
buildUIState()
}
Expand Down Expand Up @@ -1128,7 +1130,7 @@

func backupFailedConfigAndApplySafe(failedConfig: String, mappings: [KeyMapping]) async throws
-> String
{

Check warning on line 1133 in Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift

View workflow job for this annotation

GitHub Actions / code-quality 

Opening braces should be preceded by a single space and on the same line as the declaration (opening_brace)
try await configurationManager.backupFailedConfigAndApplySafe(
failedConfig: failedConfig,
mappings: mappings
Expand Down Expand Up @@ -1271,7 +1273,7 @@
AppLogger.shared.error(
"❌ [RuntimeCoordinator] kanata still not grabbing the keyboard after \(attempts) recovery attempts — giving up (manual intervention / reboot likely needed)"
)
lastError = "Keyboard remapping is not active: kanata could not capture the keyboard after \(attempts) automatic recovery attempts. Try quitting other keyboard tools, then restart KeyPath."

Check warning on line 1276 in Sources/KeyPathAppKit/Managers/RuntimeCoordinator.swift

View workflow job for this annotation

GitHub Actions / code-quality 

Line should be 200 characters or less; currently it has 205 characters (line_length)
grabGiveUpErrorActive = true
notifyStateChanged()
}
Expand Down
17 changes: 16 additions & 1 deletion Sources/KeyPathAppKit/UI/Dialogs/UninstallKeyPathDialog.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ struct UninstallKeyPathDialog: View {
@State private var didSucceed = false
@State private var hasScheduledQuit = false
@State private var autoQuitWorkItem: DispatchWorkItem?
@State private var removeVirtualHID = false

var body: some View {
VStack(spacing: 20) {
Expand All @@ -36,6 +37,20 @@ struct UninstallKeyPathDialog: View {
.foregroundColor(.secondary)
.multilineTextAlignment(.center)

// Optional: also remove the shared Karabiner virtual keyboard driver.
Toggle(isOn: $removeVirtualHID) {
VStack(alignment: .leading, spacing: 2) {
Text("Also remove the virtual keyboard driver")
.font(.caption)
Text("Other tools (e.g. Karabiner-Elements) may rely on it.")
.font(.caption2)
.foregroundColor(.secondary)
}
}
.toggleStyle(.checkbox)
.disabled(isRunning)
.accessibilityIdentifier("uninstall-remove-vhid-toggle")

// Status
if isRunning {
ProgressView()
Expand Down Expand Up @@ -90,7 +105,7 @@ struct UninstallKeyPathDialog: View {
lastError = nil
}

let report = await kanataManager.uninstall(deleteConfig: false)
let report = await kanataManager.uninstall(deleteConfig: false, removeVirtualHID: removeVirtualHID)

await MainActor.run {
isRunning = false
Expand Down
4 changes: 2 additions & 2 deletions Sources/KeyPathAppKit/UI/ViewModels/KanataViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,8 +477,8 @@ class KanataViewModel {
await manager.inspectSystemContext()
}

func uninstall(deleteConfig: Bool) async -> InstallerReport {
await manager.uninstall(deleteConfig: deleteConfig)
func uninstall(deleteConfig: Bool, removeVirtualHID: Bool = false) async -> InstallerReport {
await manager.uninstall(deleteConfig: deleteConfig, removeVirtualHID: removeVirtualHID)
}

func runFullRepair(reason: String) async -> InstallerReport {
Expand Down
21 changes: 17 additions & 4 deletions Sources/KeyPathInstallationWizard/Core/InstallerEngine.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
/// Returns: Blocking requirement if any, nil if all requirements met
private func checkRequirements(for intent: InstallIntent, context: SystemContext) async
-> Requirement?
{

Check warning on line 163 in Sources/KeyPathInstallationWizard/Core/InstallerEngine.swift

View workflow job for this annotation

GitHub Actions / code-quality 

Opening braces should be preceded by a single space and on the same line as the declaration (opening_brace)
// For inspectOnly, no requirements needed
if intent == .inspectOnly {
return nil
Expand Down Expand Up @@ -427,7 +427,7 @@
/// Includes pre-check for VHID Manager activation (per Karabiner documentation)
private func executeRestartService(_ recipe: ServiceRecipe, using broker: PrivilegeBroker)
async throws
{

Check warning on line 430 in Sources/KeyPathInstallationWizard/Core/InstallerEngine.swift

View workflow job for this annotation

GitHub Actions / code-quality 

Opening braces should be preceded by a single space and on the same line as the declaration (opening_brace)
// CRITICAL: Ensure VHID Manager is activated before restarting services
let vhidManager = VHIDDeviceManager()
if !vhidManager.detectActivation() {
Expand All @@ -452,7 +452,7 @@
/// Execute installComponent recipe
private func executeInstallComponent(_ recipe: ServiceRecipe, using broker: PrivilegeBroker)
async throws
{

Check warning on line 455 in Sources/KeyPathInstallationWizard/Core/InstallerEngine.swift

View workflow job for this annotation

GitHub Actions / code-quality 

Opening braces should be preceded by a single space and on the same line as the declaration (opening_brace)
// Map recipe ID to component installation method
switch recipe.id {
case InstallerRecipeID.installCorrectVHIDDriver:
Expand Down Expand Up @@ -509,7 +509,7 @@
/// Execute checkRequirement recipe
private func executeCheckRequirement(_ recipe: ServiceRecipe, using broker: PrivilegeBroker)
async throws
{

Check warning on line 512 in Sources/KeyPathInstallationWizard/Core/InstallerEngine.swift

View workflow job for this annotation

GitHub Actions / code-quality 

Opening braces should be preceded by a single space and on the same line as the declaration (opening_brace)
// Check requirement recipes (e.g., terminate conflicting processes)
switch recipe.id {
case InstallerRecipeID.terminateConflictingProcesses:
Expand All @@ -530,7 +530,7 @@
private func verifyHealthCheck(_ criteria: HealthCheckCriteria) async -> Bool {
if criteria.serviceID == KeyPathConstants.Bundle.daemonID,
criteria.shouldBeRunning
{

Check warning on line 533 in Sources/KeyPathInstallationWizard/Core/InstallerEngine.swift

View workflow job for this annotation

GitHub Actions / code-quality 

Opening braces should be preceded by a single space and on the same line as the declaration (opening_brace)
let managementState = await WizardDependencies.daemonManager?.refreshManagementState()
if managementState == .smappservicePending {
AppLogger.shared.log(
Expand All @@ -542,7 +542,7 @@
let runtimeSnapshot = await ServiceHealthChecker.shared.checkKanataServiceRuntimeSnapshot()
let ready = ServiceHealthChecker.decideKanataHealth(for: runtimeSnapshot).isHealthy
AppLogger.shared.log(
"🔍 [InstallerEngine] Kanata strict health check: state=\(managementState?.description ?? "nil"), running=\(runtimeSnapshot.isRunning), responding=\(runtimeSnapshot.isResponding), inputCaptureReady=\(runtimeSnapshot.inputCaptureReady), ready=\(ready)"

Check warning on line 545 in Sources/KeyPathInstallationWizard/Core/InstallerEngine.swift

View workflow job for this annotation

GitHub Actions / code-quality 

Line should be 200 characters or less; currently it has 266 characters (line_length)
)
return ready
}
Expand Down Expand Up @@ -601,10 +601,23 @@
return report
}

/// Execute uninstall via the existing coordinator (placeholder until uninstall recipes exist)
public func uninstall(deleteConfig: Bool, using broker: PrivilegeBroker) async -> InstallerReport {
AppLogger.shared.log("🗑️ [InstallerEngine] Starting uninstall (deleteConfig: \(deleteConfig))")
_ = broker // Reserved for future privileged uninstall steps
/// Execute uninstall via the existing coordinator.
/// - Parameter removeVirtualHID: when true, also tears down the Karabiner VirtualHID
/// driver. Off by default because the driver is a shared component other tools may use.
public func uninstall(deleteConfig: Bool, removeVirtualHID: Bool = false, using broker: PrivilegeBroker) async -> InstallerReport {
AppLogger.shared.log("🗑️ [InstallerEngine] Starting uninstall (deleteConfig: \(deleteConfig), removeVirtualHID: \(removeVirtualHID))")

// Remove the VirtualHID driver BEFORE the main teardown: the coordinator uninstall
// self-destructs the privileged helper that this step depends on. Best-effort — a
// VHID failure must not block removing KeyPath itself.
if removeVirtualHID {
do {
try await broker.uninstallVirtualHIDDrivers()
AppLogger.shared.log("🗑️ [InstallerEngine] Virtual HID driver removed")
} catch {
AppLogger.shared.log("⚠️ [InstallerEngine] Virtual HID removal failed (continuing uninstall): \(error)")
Comment on lines +615 to +618
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Route opted-in VHID removal through fallback path

When the privileged helper is missing or not responding, broker.uninstallVirtualHIDDrivers() fails in release because the router's privileged-helper branch only calls helperManager.uninstallVirtualHIDDrivers() (no sudo/script fallback), but the catch here swallows that failure and then the normal uninstall can still succeed via UninstallCoordinator's admin-prompt script fallback. In that scenario, a user who checked “Also remove the virtual keyboard driver” gets a successful uninstall while the VirtualHID driver is preserved, so this path needs either a fallback before continuing or a report that the opted-in removal did not happen.

Useful? React with 👍 / 👎.

}
}

let start = Date()
guard let coordinator = WizardDependencies.createUninstallCoordinator?() else {
Expand Down Expand Up @@ -642,7 +655,7 @@
/// Note: Some actions (like installRequiredRuntimeServices) are only in install plans, not repair plans.
public func runSingleAction(_ action: AutoFixAction, using broker: PrivilegeBroker) async
-> InstallerReport
{

Check warning on line 658 in Sources/KeyPathInstallationWizard/Core/InstallerEngine.swift

View workflow job for this annotation

GitHub Actions / code-quality 

Opening braces should be preceded by a single space and on the same line as the declaration (opening_brace)
AppLogger.shared.log("🔧 [InstallerEngine] runSingleAction(\(action), using:) starting")
let context = await inspectSystem()

Expand Down
Loading