From 91383e07cb4105b71a82ee2c482afb4b42222464 Mon Sep 17 00:00:00 2001 From: Jacob Fu <141651335+FuJacob@users.noreply.github.com> Date: Thu, 28 May 2026 01:35:35 -0700 Subject: [PATCH] Split CGEvent tap into listen-only observer + on-demand accept tap The single active .defaultTap at .headInsertEventTap held every system keyDown synchronously until Cotabby's main-actor callback returned. That made Cotabby's mere presence interfere with keystroke timing in other apps. DaVinci Resolve's Spacebar play/pause was the canonical victim (#328): when the main actor was contended by AX polling or prediction work, the tap delayed event delivery enough that Resolve mis-paired keyDown/keyUp and required multiple presses to pause. Steady state is now a listen-only tap at the head of the chain, which the system does not gate on the callback. A second narrow .defaultTap is installed at the tail only while a suggestion overlay is visible, consumes the accept key, and is torn down immediately when the overlay hides. In apps where no suggestion ever appears (DaVinci Resolve, the Finder, etc.), Cotabby never sits on the synchronous keystroke path. --- .../Coordinators/SuggestionCoordinator.swift | 12 +- .../Models/SuggestionSubsystemContracts.swift | 5 + Cotabby/Services/Input/InputMonitor.swift | 203 ++++++++++++++---- 3 files changed, 178 insertions(+), 42 deletions(-) diff --git a/Cotabby/App/Coordinators/SuggestionCoordinator.swift b/Cotabby/App/Coordinators/SuggestionCoordinator.swift index 455bb3f..4bde6b8 100644 --- a/Cotabby/App/Coordinators/SuggestionCoordinator.swift +++ b/Cotabby/App/Coordinators/SuggestionCoordinator.swift @@ -136,7 +136,17 @@ final class SuggestionCoordinator: ObservableObject { } overlayController.onStateChange = { [weak self] state in - self?.overlayState = state + guard let self else { return } + self.overlayState = state + // Only sit in the synchronous keystroke critical path while a suggestion is actually + // visible. With the overlay hidden, Cotabby observes via a listen-only tap that does + // not gate event delivery to other apps (issue #328). + switch state { + case .visible: + self.inputMonitor.setAcceptInterceptionActive(true) + case .hidden: + self.inputMonitor.setAcceptInterceptionActive(false) + } } visualContextCoordinator.onStateChange = { [weak self] status, excerpt in diff --git a/Cotabby/Models/SuggestionSubsystemContracts.swift b/Cotabby/Models/SuggestionSubsystemContracts.swift index 763be75..6b2c74e 100644 --- a/Cotabby/Models/SuggestionSubsystemContracts.swift +++ b/Cotabby/Models/SuggestionSubsystemContracts.swift @@ -34,6 +34,11 @@ protocol SuggestionFocusProviding: AnyObject { protocol SuggestionInputMonitoring: AnyObject { var onEvent: ((CapturedInputEvent) -> Bool)? { get set } var onSuppressedSyntheticInput: (() -> Void)? { get set } + + /// Drives the lifecycle of the active accept-key tap. The coordinator turns this on while + /// a suggestion overlay is visible and off otherwise, so Cotabby only sits in the synchronous + /// keystroke path during the brief windows it actually needs to consume the accept key. + func setAcceptInterceptionActive(_ active: Bool) } @MainActor diff --git a/Cotabby/Services/Input/InputMonitor.swift b/Cotabby/Services/Input/InputMonitor.swift index 2f211a8..1baf077 100644 --- a/Cotabby/Services/Input/InputMonitor.swift +++ b/Cotabby/Services/Input/InputMonitor.swift @@ -3,15 +3,22 @@ import Foundation import Logging /// File overview: -/// Owns the global keyboard event tap used to detect typing, navigation, dismissal keys, +/// Owns the global keyboard event taps used to detect typing, navigation, dismissal keys, /// and `Tab` acceptance. This is the boundary between raw CGEvents and Cotabby's smaller /// input-event vocabulary. /// /// `CapturedInputEvent` now lives in `Models/InputModels.swift` so the rest of the app can depend /// on the semantic event type without importing this event-tap implementation. -/// Installs a session event tap. -/// We still observe normal typing, but we can now consume `Tab` when Cotabby has a valid suggestion. +/// Installs two taps: +/// - A steady-state `.listenOnly` observer at the head of the chain. Listen-only taps do not gate +/// event delivery on the callback's return, so a slow main actor cannot stall global keystrokes +/// in unrelated apps (DaVinci Resolve's Spacebar play/pause is the canonical victim of an active +/// tap here — see issue #328). +/// - A narrow `.defaultTap` accept tap at the tail, installed only while a suggestion is visible. +/// This is the only path that consumes events, and it only exists for the brief window a +/// suggestion is on screen. When no overlay is showing, Cotabby is fully out of the keystroke +/// critical path. @MainActor final class InputMonitor { var onEvent: ((CapturedInputEvent) -> Bool)? @@ -24,16 +31,19 @@ final class InputMonitor { /// Reads the current full-accept key code from the model at event time. var fullAcceptanceKeyCodeProvider: @MainActor () -> CGKeyCode = { CGKeyCode(UInt16.max) } - /// When false, the tap passes keystrokes through without classifying or notifying the - /// coordinator. This eliminates per-keystroke overhead in apps where Tabby will never act + /// When false, the observer passes keystrokes through without classifying or notifying the + /// coordinator. This eliminates per-keystroke overhead in apps where Cotabby will never act /// (terminals, globally disabled, per-app disabled). var shouldProcessEventsProvider: @MainActor () -> Bool = { true } private let permissionProvider: @MainActor () -> Bool private let suppressionController: InputSuppressionController - private var eventTap: CFMachPort? - private var runLoopSource: CFRunLoopSource? + private var observerTap: CFMachPort? + private var observerRunLoopSource: CFRunLoopSource? + + private var acceptTap: CFMachPort? + private var acceptRunLoopSource: CFRunLoopSource? init( permissionProvider: @escaping @MainActor () -> Bool, @@ -43,30 +53,49 @@ final class InputMonitor { self.suppressionController = suppressionController } - /// Installs the event tap and begins listening for global keyboard activity. + /// Installs the observer tap and begins listening for global keyboard activity. func start() { CotabbyLogger.app.info("Input monitor starting") refresh() } - /// Removes the event tap and stops observing keyboard events. + /// Removes both taps and stops observing keyboard events. func stop() { CotabbyLogger.app.info("Input monitor stopping") - destroyTap() + destroyAcceptTap() + destroyObserverTap() } - /// Re-evaluates whether the tap should exist after a permission change. + /// Re-evaluates whether the observer tap should exist after a permission change. + /// The accept tap is also torn down if permission was revoked; it gets re-installed lazily + /// the next time the coordinator presents a suggestion. func refresh() { if permissionProvider() { - installTapIfNeeded() + installObserverTapIfNeeded() } else { - destroyTap() + destroyAcceptTap() + destroyObserverTap() } } - /// Creates and enables the CGEvent tap only when permissions allow observation. - private func installTapIfNeeded() { - guard eventTap == nil else { + /// Installs (when `active == true`) or removes (when `false`) the narrow active tap that + /// consumes the accept key so the focused application never sees it. The coordinator calls + /// this when a suggestion becomes visible or hidden, so Cotabby only blocks event delivery + /// during the brief window when there is actually something to accept. + func setAcceptInterceptionActive(_ active: Bool) { + guard permissionProvider() else { + destroyAcceptTap() + return + } + if active { + installAcceptTapIfNeeded() + } else { + destroyAcceptTap() + } + } + + private func installObserverTapIfNeeded() { + guard observerTap == nil else { return } @@ -77,29 +106,72 @@ final class InputMonitor { } let monitor = Unmanaged.fromOpaque(userInfo).takeUnretainedValue() - // The CGEvent tap callback is a C function pointer. `assumeIsolated` tells Swift that - // we are deliberately hopping back onto this `@MainActor` object before touching state. return MainActor.assumeIsolated { - monitor.handleTap(type: type, event: event) + monitor.handleObserverTap(type: type, event: event) } } guard let tap = CGEvent.tapCreate( tap: .cgSessionEventTap, place: .headInsertEventTap, + options: .listenOnly, + eventsOfInterest: CGEventMask(mask), + callback: callback, + userInfo: Unmanaged.passUnretained(self).toOpaque() + ) else { + CotabbyLogger.app.warning("Failed to create CGEvent observer tap — Input Monitoring permission may be missing") + return + } + CotabbyLogger.app.info("CGEvent observer tap installed (listen-only)") + + observerTap = tap + let source = CFMachPortCreateRunLoopSource(kCFAllocatorDefault, tap, 0) + observerRunLoopSource = source + + if let source { + CFRunLoopAddSource(CFRunLoopGetMain(), source, .commonModes) + } + + CGEvent.tapEnable(tap: tap, enable: true) + } + + private func installAcceptTapIfNeeded() { + guard acceptTap == nil else { + return + } + + let mask = (1 << CGEventType.keyDown.rawValue) + let callback: CGEventTapCallBack = { _, type, event, userInfo in + guard let userInfo else { + return Unmanaged.passUnretained(event) + } + + let monitor = Unmanaged.fromOpaque(userInfo).takeUnretainedValue() + return MainActor.assumeIsolated { + monitor.handleAcceptTap(type: type, event: event) + } + } + + // Tail-append so this tap runs *after* the head-inserted observer. The observer is + // listen-only and never drops events, so the accept tap reliably sees the accept key + // even though it runs second; the order guarantees the observer's classification fires + // before this tap consumes the event. + guard let tap = CGEvent.tapCreate( + tap: .cgSessionEventTap, + place: .tailAppendEventTap, options: .defaultTap, eventsOfInterest: CGEventMask(mask), callback: callback, userInfo: Unmanaged.passUnretained(self).toOpaque() ) else { - CotabbyLogger.app.warning("Failed to create CGEvent tap — Input Monitoring permission may be missing") + CotabbyLogger.app.warning("Failed to create CGEvent accept tap") return } - CotabbyLogger.app.info("CGEvent tap installed") + CotabbyLogger.app.info("CGEvent accept tap installed (active, accept-key only)") - eventTap = tap + acceptTap = tap let source = CFMachPortCreateRunLoopSource(kCFAllocatorDefault, tap, 0) - runLoopSource = source + acceptRunLoopSource = source if let source { CFRunLoopAddSource(CFRunLoopGetMain(), source, .commonModes) @@ -108,28 +180,43 @@ final class InputMonitor { CGEvent.tapEnable(tap: tap, enable: true) } - /// Tears down the event tap and run-loop source to avoid leaking global event observers. - private func destroyTap() { - if let source = runLoopSource { + private func destroyObserverTap() { + if let source = observerRunLoopSource { CFRunLoopRemoveSource(CFRunLoopGetMain(), source, .commonModes) } + observerRunLoopSource = nil - runLoopSource = nil - - if let tap = eventTap { + if let tap = observerTap { CFMachPortInvalidate(tap) } + observerTap = nil + } + + private func destroyAcceptTap() { + guard acceptTap != nil || acceptRunLoopSource != nil else { + return + } + if let source = acceptRunLoopSource { + CFRunLoopRemoveSource(CFRunLoopGetMain(), source, .commonModes) + } + acceptRunLoopSource = nil - eventTap = nil + if let tap = acceptTap { + CFMachPortInvalidate(tap) + } + acceptTap = nil + CotabbyLogger.app.info("CGEvent accept tap removed") } - /// Routes each raw keyboard event through suppression, classification, and optional interception. - private func handleTap(type: CGEventType, event: CGEvent) -> Unmanaged? { + /// Listen-only observer: classifies the event and notifies the coordinator. The return value + /// of `onEvent` is ignored here because a listen-only tap cannot drop or modify events. + /// Consumption of the accept key is handled by the separate active accept tap. + private func handleObserverTap(type: CGEventType, event: CGEvent) -> Unmanaged? { switch type { case .tapDisabledByTimeout, .tapDisabledByUserInput: - CotabbyLogger.app.warning("CGEvent tap was disabled by system, re-enabling") - if let eventTap { - CGEvent.tapEnable(tap: eventTap, enable: true) + CotabbyLogger.app.warning("Observer tap was disabled by system, re-enabling") + if let observerTap { + CGEvent.tapEnable(tap: observerTap, enable: true) } return Unmanaged.passUnretained(event) @@ -139,17 +226,51 @@ final class InputMonitor { return Unmanaged.passUnretained(event) } - // Short-circuit before classification when Tabby won't act on events for the - // current app (terminals, globally disabled, per-app disabled). The tap callback - // runs synchronously before macOS delivers the keystroke, so any work here adds - // latency the user feels in the target app. + // Short-circuit before classification when Cotabby won't act on events for the + // current app. Even though this tap is listen-only, classification still does work + // on the main actor that we should skip when nothing will use the result. guard shouldProcessEventsProvider() else { return Unmanaged.passUnretained(event) } let capturedEvent = classify(event: event) - let shouldIntercept = onEvent?(capturedEvent) ?? false - return shouldIntercept ? nil : Unmanaged.passUnretained(event) + _ = onEvent?(capturedEvent) + return Unmanaged.passUnretained(event) + + default: + return Unmanaged.passUnretained(event) + } + } + + /// Active accept tap: only consumes the configured accept keys, so the focused application + /// never sees them when a suggestion is on screen. All other keys pass through unchanged. + /// This tap intentionally does not invoke `onEvent` — the observer tap is the single source + /// of classification, and it has already fired for this keystroke by the time we run. + private func handleAcceptTap(type: CGEventType, event: CGEvent) -> Unmanaged? { + switch type { + case .tapDisabledByTimeout, .tapDisabledByUserInput: + CotabbyLogger.app.warning("Accept tap was disabled by system, re-enabling") + if let acceptTap { + CGEvent.tapEnable(tap: acceptTap, enable: true) + } + return Unmanaged.passUnretained(event) + + case .keyDown: + guard shouldProcessEventsProvider() else { + return Unmanaged.passUnretained(event) + } + + let keyCode = CGKeyCode(event.getIntegerValueField(.keyboardEventKeycode)) + let flags = event.flags + let noModifiers = flags.isDisjoint(with: [.maskCommand, .maskControl, .maskAlternate, .maskShift]) + guard noModifiers else { + return Unmanaged.passUnretained(event) + } + + if keyCode == fullAcceptanceKeyCodeProvider() || keyCode == acceptanceKeyCodeProvider() { + return nil + } + return Unmanaged.passUnretained(event) default: return Unmanaged.passUnretained(event)