Stop CGEvent tap from gating keystrokes in unrelated apps (#328)#337
Conversation
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.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| 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) |
There was a problem hiding this comment.
The accept tap returns nil (consumes the event) based only on key-code matching, never consulting the coordinator. When acceptCurrentSuggestion() falls through to passTabThrough() — e.g., the guard case .ready = state check fails because state was changed without the overlay hiding yet — passTabThrough calls hideOverlay() → destroyAcceptTap() and returns false. In the old design that false directly prevented consumption. Here, if the accept tap was already committed to the current event delivery chain before CFMachPortInvalidate ran (CGEvent tap chain snapshots are typically taken at dispatch time), it will still fire and silently swallow the Tab key that the coordinator explicitly decided should reach the app. The coordination contract that the Bool return of onEvent expressed is now broken.
| @@ -34,6 +34,11 @@ protocol SuggestionFocusProviding: AnyObject { | |||
| protocol SuggestionInputMonitoring: AnyObject { | |||
| var onEvent: ((CapturedInputEvent) -> Bool)? { get set } | |||
There was a problem hiding this comment.
onEvent return value is now dead weight
handleObserverTap discards the return with _ = onEvent?(capturedEvent), and the accept tap independently decides whether to consume the event. The Bool return type implies callers can still control interception through this callback, but they no longer can. Changing to Void removes a source of confusion for future maintainers and makes the new architectural split explicit in the protocol contract.
| var onEvent: ((CapturedInputEvent) -> Bool)? { get set } | |
| var onEvent: ((CapturedInputEvent) -> Void)? { get set } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| 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") | ||
| } |
There was a problem hiding this comment.
destroyObserverTap is asymmetric with destroyAcceptTap
destroyObserverTap does not log its removal (no CotabbyLogger.app.info(...) call), while destroyAcceptTap does. This inconsistency makes it harder to diagnose tap lifecycle issues from logs when the observer tap is unexpectedly torn down (e.g., during permission revocation).
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Fixes #328 (Cotabby messing with DaVinci Resolve's Spacebar play/pause). The single active
.defaultTapwe installed at the head of the system event chain held every keyDown synchronously until our main-actor callback returned, so any contention on the main actor (AX polling, prediction work, SwiftUI updates) translated into observable keystroke delay in every app — even when Cotabby was disabled globally, because the tap callback still ran before the early-exit check.Replaced with a two-tap design:
.listenOnlyat.headInsertEventTap. Sees every keyDown for classification, but the system does not wait on the callback's return, so a slow main actor cannot stall global keystrokes..defaultTapat.tailAppendEventTap, installed only while a suggestion overlay is visible. Consumes the configured accept key(s) and nothing else. The coordinator turns it on/off via the existingoverlayController.onStateChangehook.In DaVinci Resolve (and any other app where no suggestion ever appears) the accept tap is never installed, so Cotabby is fully out of the synchronous keystroke critical path.
Validation
swiftlint lint --quiet— exit 0xcodebuild -project Cotabby.xcodeproj -scheme Cotabby -destination 'platform=macOS' build— ** BUILD SUCCEEDED **xcodebuild -project Cotabby.xcodeproj -scheme Cotabby -destination 'platform=macOS' build-for-testing— ** TEST BUILD SUCCEEDED **Manual verification recommended: with Cotabby running, play/pause in DaVinci Resolve via Spacebar should now feel native. Tab acceptance in normal text fields should continue to work — verify in TextEdit / Notes / VS Code that the accept tap still swallows Tab when a suggestion is shown.
Linked issues
Fixes #328
Risk / rollout notes
.tailAppendEventTaprather than.headInsertEventTap. The observer is listen-only and never drops events, so the accept tap reliably sees every keystroke regardless of order; this guarantees the observer's classification fires before the accept tap consumes.FocusTrackerstill polls AX every 80 ms regardless ofisGloballyEnabled. That doesn't block keystrokes anymore, but it is still wasted main-actor work when Cotabby is disabled in the focused app.Greptile Summary
Replaces the single always-active
CGEventtap with a two-tap design to fix keystroke latency in unrelated apps (DaVinci Resolve #328): a permanent listen-only observer atheadInsertEventTapfor classification, and a narrow active accept tap attailAppendEventTapthat is installed only while a suggestion overlay is visible.listenOnly, HEAD): always on, never blocks event delivery. Classifies every keyDown and callsonEventfor coordinator notification. The return value ofonEventis now discarded — the old synchronous veto path no longer exists.setAcceptInterceptionActivein response tooverlayController.onStateChange. Independently consumes the accept key based on key-code matching alone, without consulting the coordinator's decision.passTabThrough()(state not ready, permission issue) while the accept tap is installed, it destroys the tap and returnsfalse— but the accept tap may already be committed to the current event delivery chain and could still swallow the Tab key the coordinator intended to pass through.Confidence Score: 3/5
The core latency fix is architecturally sound, but the two-tap design decouples event consumption from the coordinator's decision in a way that could silently swallow Tab keystrokes in edge cases.
The normal acceptance path (overlay visible → Tab pressed → suggestion ready) works correctly. The concern is the decoupled design: the accept tap consumes the key based on key-code alone, while the coordinator's onEvent return value (previously the exclusive veto gate) is now ignored. If acceptCurrentSuggestion() hits a guard and calls passTabThrough() while the accept tap is in the current event delivery chain, the coordinator's intent to let Tab through may not be honoured.
InputMonitor.swift — specifically handleAcceptTap and the interaction with destroyAcceptTap being called mid-chain from the observer callback.
Important Files Changed
Reviews (1): Last reviewed commit: "Split CGEvent tap into listen-only obser..." | Re-trigger Greptile