Fix accept tap race

[FuJacob]

Summary

Fixes the accept-key race by making the active consuming tap own acceptance delivery instead of relying on the listen-only observer to run first. The observer now ignores accept keys, and consumed accept events carry an explicit replay flag so rejected or failed accepts can safely pass Tab back to the host app without double-replaying observed keys.

Validation

• xcodebuild -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 **
• xcodebuild test -project Cotabby.xcodeproj -scheme Cotabby -destination 'platform=macOS' -only-testing:CotabbyTests/SuggestionStateHelperTests -only-testing:CotabbyTests/SuggestionSessionReconcilerTests
  • Did not execute tests locally: the app-hosted test bundle failed to load because the mapped CotabbyTests.xctest bundle and host process had different Team IDs.

Linked issues

None.

Risk / rollout notes

This changes accept-key ownership in the event-tap pipeline. Manual smoke testing should cover partial accept, final-chunk accept, stale-session pass-through, and browser/editor Tab behavior.

Greptile Summary

This PR restructures the accept-key event pipeline so the active consuming tap (not the listen-only observer) is the single source of truth for acceptance delivery. The observer now silently skips accept keys, the accept tap creates the CapturedInputEvent with replaysWhenAcceptanceRejected: true and dispatches onEvent asynchronously, and all rejection paths in acceptSuggestion have been updated to replay the consumed key back to the host app.

• InputMonitor.swift: Removes the pendingObserverAcceptedKeyEvent / isHandlingAcceptKeyObserverEvent / deferred-teardown machinery; observer tap early-returns on accept keys; accept tap now constructs the event and calls onEvent via DispatchQueue.main.async before returning nil.
• InputModels.swift: Adds replaysWhenAcceptanceRejected field (default false) and isAcceptanceKeyEvent helper to CapturedInputEvent.
• SuggestionCoordinator+Acceptance.swift: Guards replayConsumedAcceptKey calls behind replay.replaysWhenAcceptanceRejected; adds replay in the insert-failed path; removes state == .ready gate from shouldConsumeAcceptKeyProvider.

Confidence Score: 3/5

Safe on the happy path, but the early-exit in handleInputEvent for disabled reasons does not replay the consumed Tab, which breaks the replay contract introduced by this PR.

All rejection branches inside acceptSuggestion correctly call passTabThrough(replay:) or replayConsumedAcceptKey directly. However, handleInputEvent has a prior early-exit — the disabledReason check — that bypasses acceptSuggestion entirely and returns false without checking event.replaysWhenAcceptanceRejected. If that branch runs for an event the accept tap already consumed (because the coordinator's disabled state changed between the tap callback and the async dispatch), the user's Tab is silently dropped. The timing window is narrow, but the code path is real and the omission is inconsistent with the rest of the replay design.

SuggestionCoordinator+Input.swift — the disabledReason early-exit at the top of handleInputEvent needs a replay check to match the pattern used everywhere else in acceptSuggestion.

Important Files Changed

Filename	Overview
Cotabby/Services/Input/InputMonitor.swift	Major rework: accept-key delivery moved from observer tap to consuming accept tap. Observer now skips accept keys entirely; accept tap creates CapturedInputEvent and posts onEvent via DispatchQueue.main.async before returning nil. Removed pendingObserverAcceptedKeyEvent, isHandlingAcceptKeyObserverEvent, shouldRemoveAcceptTapAfterPendingEvent, and deferDuringCurrentAcceptKeyEvent logic.
Cotabby/App/Coordinators/SuggestionCoordinator+Input.swift	handleInputEvent's disabled-reason early-exit path does not check event.replaysWhenAcceptanceRejected, leaving a gap where an accept-tap-consumed Tab can be silently dropped if the coordinator's disabled state changes in the async window.
Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift	Added replay call in the insert-failed path and tightened passTabThrough to guard on replaysWhenAcceptanceRejected before calling replayConsumedAcceptKey. All rejection branches in acceptSuggestion correctly handle replay.
Cotabby/Models/InputModels.swift	Added replaysWhenAcceptanceRejected field (default false) and isAcceptanceKeyEvent computed property. Explicit memberwise initializer preserves backward-compatible default. Model change is clean.

Sequence Diagram

sequenceDiagram
    participant HID as HID Event System
    participant ObsTap as Observer Tap (listenOnly, head)
    participant AccTap as Accept Tap (defaultTap, tail)
    participant Main as Main Queue (async)
    participant Coord as SuggestionCoordinator
    participant HostApp as Host App

    HID->>ObsTap: keyDown (Tab)
    Note over ObsTap: isAcceptanceKeyEvent → skip onEvent
    ObsTap-->>HID: passUnretained (not consumed)

    HID->>AccTap: keyDown (Tab)
    AccTap->>AccTap: shouldConsumeAcceptKeyProvider()?
    alt session active and overlay visible
        AccTap->>Main: "async { onEvent(capturedEvent) }"
        AccTap-->>HID: nil (consumed)
        Main->>Coord: "onEvent [replaysWhenAcceptanceRejected=true]"
        alt acceptance succeeds
            Coord->>Coord: insertSuggestion / advanceSession
        else acceptance rejected
            Coord->>HostApp: synthesized Tab replayed
        end
    else no valid session
        AccTap-->>HID: passUnretained
        HID->>HostApp: Tab delivered normally
    end

Loading

_{Reviews (1): Last reviewed commit: "Fix accept tap race" | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[greptile-apps]

DispatchQueue.main.async is used from an @MainActor-isolated method. While this works because DispatchQueue.main and @MainActor share the same underlying thread, it bypasses Swift's concurrency type-system guarantees and can produce warnings under strict concurrency checks. Task { @MainActor in … } is the idiomatic form here and makes the isolation explicit.

Suggested change

	DispatchQueue.main.async {
	_ = onEvent(capturedEvent)
	}
	Task { @MainActor in
	_ = onEvent(capturedEvent)
	}

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!