Fix last tab issue#385
Conversation
| CotabbyLogger.app.debug( | ||
| "Accept tap consumed keyCode=\(keyCode) modifiers=\(eventModifiers.rawValue)" | ||
| ) |
There was a problem hiding this comment.
The new deferred-pending-event path (around line 349) emits the exact same .debug string as the original Layer-2 consumption path (~line 384). When grepping logs to diagnose a timing issue it is impossible to tell which path fired. Adding a short qualifier like "pending" to the new path's message would make the two branches immediately distinguishable without any other changes.
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!
| if isAcceptKeyEvent, shouldConsume, acceptTap != nil { | ||
| pendingObserverAcceptedKeyEvent = PendingAcceptedKeyEvent( | ||
| keyCode: capturedEvent.keyCode, | ||
| modifiers: ShortcutModifierMask(eventFlags: capturedEvent.flags) | ||
| ) | ||
| } |
There was a problem hiding this comment.
pendingObserverAcceptedKeyEvent has no cleanup path when the accept tap is system-disabled mid-flight
If the system disables the accept tap (.tapDisabledByTimeout) between the observer callback setting pendingObserverAcceptedKeyEvent and the tap's keyDown handler running, handleAcceptTap receives the disable notification, re-enables the tap, and returns — without clearing pendingObserverAcceptedKeyEvent. macOS should re-deliver the original keyDown after re-enable so the pending event is consumed in the next callback, but if the event is somehow dropped (e.g. very short tap disable window), the accept tap stays alive indefinitely with a stale pending entry and shouldRemoveAcceptTapAfterPendingEvent = true. The tap will then consume and destroy itself on the next Tab press even after a new suggestion session starts, leaving the new session's accept tap unexpectedly gone. Clearing pendingObserverAcceptedKeyEvent (and resetting shouldRemoveAcceptTapAfterPendingEvent) in the .tapDisabledByTimeout / .tapDisabledByUserInput branch of handleAcceptTap would close this window.
Summary
Validation
Linked issues
Risk / rollout notes
Greptile Summary
This PR fixes a race condition where Chrome (and similar apps) would receive a spurious Tab keystroke after Cotabby inserted text on the final suggestion chunk. The root cause is the ordering of the two event taps: the listen-only observer (head-insert) fires first and can trigger overlay dismissal — which previously destroyed the accept tap — before the active accept tap (tail-append) had a chance to swallow the same physical Tab.
pendingObserverAcceptedKeyEvent,isHandlingAcceptKeyObserverEvent,shouldRemoveAcceptTapAfterPendingEvent) coordinate deferred teardown:destroyAcceptTap()now defaults to deferring removal when called from inside an observer accept-key callback, and the accept tap itself handles the deferred cleanup once it has consumed the pending key.stop(),refresh(), and permission-revoked call-sites passdeferDuringCurrentAcceptKeyEvent: falseto preserve the previous immediate-teardown semantics for those paths.Confidence Score: 4/5
The change is confined to a single file and the happy path is well-reasoned; the main residual risk is a stale pending-event record if the accept tap is system-disabled at a narrow moment between the observer and accept-tap callbacks.
The fix correctly threads three new state flags through the observer and accept tap callbacks on the MainActor, preserving all existing safeguards for every path except the new deferred-consume path where those guards are intentionally bypassed because the decision was already made by the observer. Two minor robustness gaps exist: the system-tap-disabled re-enable path does not clear the pending event record, and the two consumption log messages are identical, making post-hoc log analysis harder.
Cotabby/Services/Input/InputMonitor.swift — specifically the .tapDisabledByTimeout branch in handleAcceptTap and the duplicate debug log string near the new pending-event consumption site.
Important Files Changed
Sequence Diagram
sequenceDiagram participant User as User presses Tab participant OT as Observer Tap head listen-only participant Coord as Coordinator onEvent participant AT as Accept Tap tail active participant App as Host App User->>OT: keyDown Tab OT->>OT: classify as .acceptance OT->>OT: "isHandlingAcceptKeyObserverEvent = true" OT->>Coord: onEvent returns true Coord->>Coord: hide overlay Coord->>OT: setAcceptInterceptionActive false deferred OT->>OT: set pendingObserverAcceptedKeyEvent OT->>OT: defer clears flag skip immediate destroy OT-->>AT: event propagates in chain AT->>AT: key matches accept binding AT->>AT: pendingObserverAcceptedKeyEvent matches cleared AT->>AT: shouldRemoveAcceptTapAfterPendingEvent destroyAcceptTapImmediately AT-->>App: return nil event consumedReviews (1): Last reviewed commit: "Fix last tab issue" | Re-trigger Greptile