Replay swallowed Tab when coordinator bails on acceptance#363
Conversation
passTabThrough was misnamed: once the active accept tap consumed Tab, the function only cleared overlay state — the focused app never saw the keystroke. In Gmail (and most browser-rendered fields) the prepareAcceptance reconciliation check returns .invalid often enough that users perceive Cotabby as randomly eating their Tab keypress without inserting or moving focus. Thread the original CapturedInputEvent from handleInputEvent down through acceptCurrentSuggestion / acceptEntireSuggestion into passTabThrough, and have it call the new InputMonitor.replayConsumedAcceptKey after hideOverlay tears down the accept tap. Suppression is armed before the synthetic post so the observer tap recognizes the replay as Cotabby's own work.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
|
|
||
| if event.kind == .acceptance { | ||
| return acceptCurrentSuggestion() | ||
| return acceptCurrentSuggestion(originalEvent: event) | ||
| } | ||
|
|
||
| if event.kind == .fullAcceptance { | ||
| return acceptEntireSuggestion() | ||
| return acceptEntireSuggestion(originalEvent: event) | ||
| } |
There was a problem hiding this comment.
originalEvent is forwarded unconditionally to acceptCurrentSuggestion/acceptEntireSuggestion. When the overlay is hidden the accept tap is not installed, so the original Tab reaches the focused app normally. But if interactionState.activeSession == nil (the typical state when nothing is suggested), acceptSuggestion falls through to passTabThrough(replay: originalEvent), which calls replayConsumedAcceptKey and posts a second Tab to the HID stream. The focused app then receives the original Tab keyDown plus the replayed one — a silent double-Tab on every Tab press without an active suggestion.
The replay should only be dispatched when the accept tap was actually armed, i.e., when the overlay was visible at the moment the keystroke arrived. Gating on overlayState.isVisible before passing originalEvent closes the gap: if the overlay is already hidden the accept tap never consumed the event, so there is nothing to replay.
| } | |
| if event.kind == .acceptance { | |
| return acceptCurrentSuggestion() | |
| return acceptCurrentSuggestion(originalEvent: event) | |
| } | |
| if event.kind == .fullAcceptance { | |
| return acceptEntireSuggestion() | |
| return acceptEntireSuggestion(originalEvent: event) | |
| } | |
| if event.kind == .acceptance { | |
| return acceptCurrentSuggestion(originalEvent: overlayState.isVisible ? event : nil) | |
| } | |
| if event.kind == .fullAcceptance { | |
| return acceptEntireSuggestion(originalEvent: overlayState.isVisible ? event : nil) | |
| } |
1 participant
Summary
Investigation in this conversation traced the "sometimes pressing Tab in Gmail doesn't accept and misses the capture" symptom to a contract violation in our acceptance path.
After #337 the active accept tap installs whenever the suggestion overlay is visible and unconditionally consumes Tab as long as it's there. But the coordinator's decision to actually accept runs later in
acceptSuggestion(SuggestionCoordinator+Acceptance.swift:22) and can still bail along several paths:interactionState.activeSession == nil(background refresh nuked the session while overlay was still up).prepareAcceptancereturns.invalidbecause the live preceding text no longer reconciles with the session's snapshot.(3) is the Gmail one. Gmail compose is
contentEditablerendered in Chrome/Safari/Arc; browser AX preceding-text reads occasionally lag selection state by a frame, so the reconciliation check returns.invalidmore often there. When that happens the coordinator callspassTabThrough(reason:)— which, despite the name, only hides the overlay and clears state. Tab was already gone, swallowed by the tap. From Gmail's perspective the keystroke never arrived; from the user's perspective Cotabby ate their Tab.This PR makes
passTabThroughhonest: thread the originalCapturedInputEventfromhandleInputEventdown throughacceptCurrentSuggestion/acceptEntireSuggestionintopassTabThrough, and afterhideOverlaytears down the accept tap, re-post the captured key to the focused app via the newInputMonitor.replayConsumedAcceptKey(keyCode:flags:). Suppression is armed beforehand so our observer tap recognizes the replay as Cotabby's own work instead of treating it as fresh input.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 needed:
Linked issues
Refs #337 (which introduced the consume-before-confirm tap split).
Risk / rollout notes
.cghidEventTap, identical to howSuggestionInserterinjects accepted suggestion text. Same suppression mechanism, so no new loop risk.originalEventparameter has a default value ofnileverywhere it was added, so any existing internal caller that doesn't have an event (none today, but defensively) still compiles.replayConsumedAcceptKeyfails to construct a synthetic event (extremely unlikely) we log a warning and silently drop — same user-visible behavior as today, no regression.Greptile Summary
This PR fixes a real usability bug where pressing Tab in Gmail (and similar browser-hosted
contentEditablefields) would silently swallow the keystroke when Cotabby's AX reconciliation check rejected the accept. The fix threads the originalCapturedInputEventthrough the bail paths inacceptSuggestionand re-posts it via a newreplayConsumedAcceptKeymethod after the accept tap is torn down.InputMonitor.replayConsumedAcceptKeysynthesizes a new keyDown+keyUp pair at.cghidEventTap(matching the existingSuggestionInserterpattern) and armsInputSuppressionControllerbeforehand so the observer tap treats the replay as Cotabby's own synthetic work rather than fresh user input.passTabThroughnow accepts an optionalreplay: CapturedInputEvent?parameter, and each bail branch inacceptSuggestionforwardsoriginalEventinto it; the default value ofnilkeeps all existing callers source-compatible.SuggestionCoordinator+Input.swiftunconditionally passesoriginalEvent: eventregardless of whether the overlay was visible — and therefore whether the accept tap was installed. When no suggestion is showing the accept tap is not active, the original Tab already reaches the focused app, andreplayConsumedAcceptKeyposts a second Tab, producing a double-keystroke on every Tab press in a text field with no suggestion active.Confidence Score: 3/5
Not safe to merge as-is: the fix correctly handles the Gmail bail case but introduces a double-Tab regression on every Tab press in any text field where no suggestion is currently showing.
The replay mechanism in
InputMonitorand thepassTabThroughchanges are well-structured, butSuggestionCoordinator+Input.swiftpassesoriginalEvent: eventunconditionally — including when the overlay is hidden and the accept tap was never installed. In that case the original Tab keyDown travels through to the focused app normally, andreplayConsumedAcceptKeythen posts a second Tab. The suppression token is consumed by the replayed keyDown at the observer tap (not the original), so the focused app sees both events. This would manifest as every Tab press in an enabled text field producing two Tab events whenever Cotabby has no active suggestion, which covers the vast majority of Tab presses throughout the day.Cotabby/App/Coordinators/SuggestionCoordinator+Input.swift — the two lines that pass
originalEvent: eventneed to be gated onoverlayState.isVisible.Important Files Changed
originalEventunconditionally to the acceptance path regardless of overlay visibility, causingreplayConsumedAcceptKeyto fire — and double-Tab — on every Tab press when no suggestion is active.originalEventthrough all bail paths inacceptSuggestionintopassTabThrough; the replay call is correctly placed afterhideOverlay(which synchronously tears down the accept tap via theonStateChangechain), so the replayed Tab won't be re-consumed.replayConsumedAcceptKeyarms suppression for exactly one keyDown before posting the synthesized keyDown+keyUp pair at.cghidEventTap, consistent with howSuggestionInserterhandles accepted text.replayConsumedAcceptKeyto theSuggestionInputMonitoringprotocol with a clear doc comment; no issues.Sequence Diagram
sequenceDiagram participant User participant HID as HID Event Stream participant OT as Observer Tap (head, listen-only) participant AT as Accept Tap (tail, active) participant Coord as SuggestionCoordinator participant IM as InputMonitor participant SC as SuppressionController participant App as Focused App (e.g. Gmail) Note over User,App: Happy path — overlay visible, reconciliation succeeds User->>HID: Tab keyDown HID->>OT: keyDown OT->>Coord: handleInputEvent(.acceptance, event) Coord->>Coord: acceptSuggestion → prepareAcceptance → .ready Coord->>App: insert suggestion text (via SuggestionInserter) HID->>AT: keyDown AT-->>HID: nil (consumed) Note over User,App: Bail path (this PR) — overlay visible, reconciliation returns .invalid User->>HID: Tab keyDown HID->>OT: keyDown OT->>Coord: handleInputEvent(.acceptance, event) Coord->>Coord: acceptSuggestion → passTabThrough(replay: event) Coord->>Coord: hideOverlay → destroyAcceptTap (synchronous) Coord->>IM: replayConsumedAcceptKey(keyCode, flags) IM->>SC: registerSyntheticInsertion(expectedKeyDownCount: 1) IM->>HID: post keyDown (replay) IM->>HID: post keyUp (replay) HID->>AT: keyDown (original — accept tap fires despite invalidation) AT-->>HID: nil (consumed) HID->>OT: keyDown (replay) OT->>SC: consumeIfNeeded() → true OT-->>HID: passthrough (listen-only) HID->>App: replay keyDown HID->>App: replay keyUp Note over User,App: Bug (no overlay) — accept tap not installed, originalEvent passed unconditionally User->>HID: Tab keyDown (no suggestion visible) HID->>OT: keyDown OT->>Coord: handleInputEvent(.acceptance, event) Coord->>Coord: "activeSession==nil → passTabThrough(replay: event)" Coord->>IM: replayConsumedAcceptKey (accept tap was never installed!) IM->>HID: post keyDown (replay) HID->>App: original keyDown HID->>App: original keyUp HID->>App: replay keyDown (double-Tab) HID->>App: replay keyUpReviews (1): Last reviewed commit: "Replay swallowed Tab when coordinator ba..." | Re-trigger Greptile