fix: recover CGEventTap after system events — fixes #167 system-wide input degradation#169
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review Walkthrough
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex pls review this PR, take into account we're trying to fix a performance issue that scales up with usage only when the mouse buttons options are enabled |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@FIXES.md`:
- Around line 43-44: The documentation in FIXES.md currently implies that
`activeShortcuts` is cleared as part of
`MouseChordActionManager.forceRebuild()`, but it is actually cleared in
`ShortcutsManager.rebuildAllInputHooks()`. Revise the wording to explicitly
attribute the clearing of `activeShortcuts` and the stopping of in-progress
tracking to `ShortcutsManager.rebuildAllInputHooks()` to remove the ambiguity
about which component owns this responsibility.
In `@Swift` Shift/packages/CGEventSupervisor/README.md:
- Around line 3-9: The CGEventSupervisor vendored package declares MIT License
in its README but lacks a corresponding LICENSE file in the Swift
Shift/packages/CGEventSupervisor/ directory, and the upstream source also lacks
explicit license documentation. Resolve this compliance issue by either creating
a LICENSE file in the CGEventSupervisor package directory containing the full
MIT license text, or update the README to accurately reflect the actual
licensing status after confirming with the upstream author. Do not claim a
license without proper documentation to back it up.
In `@Swift`
Shift/packages/CGEventSupervisor/Sources/CGEventSupervisor/CGEventSupervisor+Setup.swift:
- Around line 58-61: The eventMask computed property's reduce closure performs
an unchecked shift operation (1 << $1.rawValue) that can overflow if
CGEventType.rawValue exceeds safe bit widths. Add bounds checking in the reduce
closure to filter out invalid event types before applying the bitwise operation.
Guard against rawValues that would cause shift overflow by validating they fall
within a safe range (typically 0 to 63 for 64-bit operations) before performing
the shift and OR operation.
In `@Swift`
Shift/packages/CGEventSupervisor/Sources/CGEventSupervisor/CGEventSupervisor+Subscribers.swift:
- Around line 28-35: The needsSetup check in the subscriber update logic only
detects when events are added because it compares against the old
subscribedEvents before updating the __nsEventSubscribers record. When an
existing subscriber narrows its event set (subscribes to fewer events), the tap
is not rebuilt and continues listening to stale events. Fix this by computing
needsSetup after updating the __nsEventSubscribers with the new events, so it
can detect both additive changes and when the overall set of subscribed events
decreases, triggering a tap rebuild when necessary. Apply the same fix to the
duplicate logic at lines 51-58.
In `@Swift` Shift/src/Manager/ShortcutsManager.swift:
- Around line 820-825: The startHealthCheckTimer method's timer closure
currently forces a rebuild every 60 seconds whenever isSubscribed is true, which
can interrupt an in-progress chord gesture. Add an additional guard condition in
the timer closure (after the existing guard that checks isSubscribed) to check
whether a chord gesture is currently active before calling forceRebuild(),
ensuring that rebuilds are skipped if a chord gesture is in progress.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b5a9836-b071-4dc3-8a0d-d38165ba77a3
📒 Files selected for processing (13)
.gitignoreFIXES.mdSwift Shift.xcodeproj/project.pbxprojSwift Shift/packages/CGEventSupervisor/Package.swiftSwift Shift/packages/CGEventSupervisor/README.mdSwift Shift/packages/CGEventSupervisor/Sources/CGEventSupervisor/CGEventSupervisor+Callback.swiftSwift Shift/packages/CGEventSupervisor/Sources/CGEventSupervisor/CGEventSupervisor+Setup.swiftSwift Shift/packages/CGEventSupervisor/Sources/CGEventSupervisor/CGEventSupervisor+Subscribers.swiftSwift Shift/packages/CGEventSupervisor/Sources/CGEventSupervisor/CGEventSupervisor+Types.swiftSwift Shift/packages/CGEventSupervisor/Sources/CGEventSupervisor/CGEventSupervisor.swiftSwift Shift/packages/CGEventSupervisor/Sources/CGEventSupervisor/Extensions/CGEvent.swiftSwift Shift/packages/CGEventSupervisor/Sources/CGEventSupervisor/Extensions/NSEvent.swiftSwift Shift/src/Manager/ShortcutsManager.swift
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bad94028f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
pablopunk
force-pushed
the
fractal-Swift-Shift-https-github-com-pablopunk-swiftshift-issues-167-foll-62dcab
branch
2 times, most recently
from
fc1e7e3 to
7918f58
Compare
|
CodeRabbit review follow-up: ✅ FIXES.md ambiguity — fixed: clarified that ✅ LICENSE file — added MIT LICENSE file to the vendored package. ✅ Health check timer mid-gesture — added guard for ❌ eventMask shift overflow — skipped. This is pre-existing upstream code in CGEventSupervisor, not introduced by this PR. CGEventType raw values are well under 64 (largest is ~30), so overflow is not practically reachable. ❌ Subscriber needsSetup ordering — skipped. Also pre-existing upstream code. The scenario (narrowing an existing subscriber's events) never occurs in SwiftShift's usage — we either subscribe to a fixed set or cancel entirely. |
Fixes #167 — system-wide input degradation after 2-3 days caused by CGEventTap silently dying on sleep/wake, display changes, and session lock. - Vendor CGEventSupervisor as local package with: - Proper Mach port teardown (run loop source + CFMachPortInvalidate) - Auto-reenable tap on kCGEventTapDisabledByTimeout/UserInput - Add system event recovery observers (didWake, sessionDidBecomeActive, didChangeScreenParameters) - Add forceRebuild() to fix stale isSubscribed flag after cancelAll() - Clear active shortcut state during recovery - Add 60s health check timer for Secure Input recovery
pablopunk
force-pushed
the
fractal-Swift-Shift-https-github-com-pablopunk-swiftshift-issues-167-foll-62dcab
branch
from
7918f58 to
32f9518
Compare
pablopunk
deleted the
fractal-Swift-Shift-https-github-com-pablopunk-swiftshift-issues-167-foll-62dcab
branch
1 participant
Problem
SwiftShift's mouse-click shortcuts (Command+click for Move, Command+right-click for Resize) use a CGEventTap to intercept mouse events. macOS silently disables event taps on sleep/wake, display changes, session lock, and slow callbacks — but SwiftShift had no recovery mechanism. A dead tap causes system-wide input lag because CGEventTaps are synchronous (WindowServer pauses all input until the callback returns).
Closes #167.
Root Cause
The reporter uses Command+click for Move and Command+right-click for Resize. Each shortcut press creates CGEventSupervisor subscriptions which create a CGEventTap. The tap dies silently on system events, and without recovery the tap stays dead — causing the system-wide input delay. Restarting the app works because it creates a fresh tap.
Fixes
Summary by CodeRabbit