fix(input-monitor): harden event tap to avoid IME conflict (#103)#106
Open
debugtheworldbot wants to merge 4 commits intomainfrom
Open
fix(input-monitor): harden event tap to avoid IME conflict (#103)#106debugtheworldbot wants to merge 4 commits intomainfrom
debugtheworldbot wants to merge 4 commits intomainfrom
Conversation
Re-enable the CGEventTap on tapDisabledByTimeout / tapDisabledByUserInput instead of leaving it dormant, and move the synchronous NSRunningApplication lookup off the event tap callback thread via a new AppIdentityCache that resolves unknown PIDs asynchronously and coalesces concurrent lookups. Pre-cache the frontmost app's PID so the common path avoids the async hop entirely. These two changes keep the callback fast and the tap healthy, reducing interference with input methods such as WeChat IME on macOS 15. Refs #103
Lock in the core invariants behind the IME-conflict fix: - resolver is never called synchronously from the event-tap callback path - concurrent lookups for the same PID coalesce into a single resolution - isTapDisabledSignal flags only tapDisabledByTimeout / tapDisabledByUserInput Refs #103
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces an AppIdentityCache to handle application identity resolution asynchronously, avoiding event tap delays, and adds logic to recover from disabled event taps. Technical feedback points out a potential crash in the tap callback when handling system-disabled signals, suggests optimizing lock contention within the cache, and recommends implementing negative caching for failed resolutions to prevent task queue bloat.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens KeyStats’ macOS input event tap pipeline to avoid IME conflicts by promptly re-enabling disabled event taps and moving app-identity resolution off the event-tap callback path via an asynchronous PID→bundleId cache.
Changes:
- Add
isTapDisabledSignal(_:)and use it in the event tap callback to immediately re-enable the tap when the system disables it. - Introduce
AppIdentity+AppIdentityCacheto resolve PID→(bundleId,name) asynchronously with coalescing of concurrent resolutions. - Add a new focused test suite for
AppIdentityCachebehavior and wire it intoswift test.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Package.swift | Includes the new AppIdentityCacheTests.swift in the explicit SwiftPM test target sources list. |
| KeyStatsTests/AppIdentityCacheTests.swift | Adds 13 tests covering tap-disable signal detection and AppIdentityCache async/coalescing semantics. |
| KeyStats/StatsModels.swift | Adds isTapDisabledSignal(_:) helper for reuse + test coverage. |
| KeyStats/InputMonitor.swift | Re-enables the event tap upon receiving .tapDisabledByTimeout / .tapDisabledByUserInput signals. |
| KeyStats/AppStats.swift | Introduces AppIdentity, dispatcher abstraction, and the thread-safe async AppIdentityCache. |
| KeyStats/AppActivityTracker.swift | Refactors to use AppIdentityCache and resolves NSRunningApplication(processIdentifier:) on a utility queue. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Merge the cache-hit / pending-check / frontmost-read into a single lock block via lookupLocked(pid:), removing two lock roundtrips from the uncached fast path on the event tap callback thread. - Cache resolver failures in unresolvablePIDs so events from unresolvable system processes no longer spawn a new background resolution on every hit. - Clear the negative-cache entry for a PID when it shows up as frontmost, so PID reuse after process termination is handled. Addresses: #106 (comment) Addresses: #106 (comment)
Apple's docs note that the event parameter is NULL when the tap callback is invoked with a tap-disabled notification. Swift maps CGEventTapCallBack to a non-optional CGEvent, so calling Unmanaged.passUnretained on a NULL event is a latent crash. The disable path is purely a state signal, so returning nil matches the notification semantics and is safe. Addresses: #106 (comment)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tapDisabledByTimeout/tapDisabledByUserInput）后一直处于半死状态，可能干扰输入法事件链路的问题。NSRunningApplication(processIdentifier:)的同步查询移到后台队列，避免回调超时反过来触发 tap 被禁用。Changes
Fix
InputMonitor.swift：事件回调收到 tap-disable 信号时立即重新启用 tap。AppActivityTracker.swift：改写为 Cocoa 薄封装，内部使用新的AppIdentityCache。AppStats.swift：新增AppIdentity/AppIdentityCache（纯 Swift，PID 缓存 + 异步解析 + 同 PID 并发合并）。StatsModels.swift：抽出isTapDisabledSignal(_:)供回调使用与测试覆盖。Test
KeyStatsTests/AppIdentityCacheTests.swift（13 个用例）：isTapDisabledSignal仅对 timeout / userInput 返回 trueupdateFrontmost预填 PID 让后续查询零解析开销Package.swift加入新测试文件。Test plan
swift test全部通过（39 个测试，+13 新增）Closes #103