refactor(input): extract FocusStealCorrector with regression suite#4
Merged
Merged
Conversation
RecordedKey was nested inside KeyEventTap, which forced callers to refer to it as KeyEventTap.RecordedKey and coupled the type's visibility to that class. Moving it to its own top-level type prepares for FocusStealCorrector extraction (which will own RecordedKey-shaped data without depending on KeyEventTap). No behavior change. FocusStealTests updated to use the new bare name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 86-line correctFocusSteal() method, six instance properties, and the focus-steal-related guards in handleKeyDown/activateServer/deactivateServer are all moved out of OngeulInputController into a dedicated FocusStealCorrector class. The corrector owns its state machine and depends on three injectable protocols: - KeyEvidenceSource: the external key buffer (CGEventTap in production) - Scheduler: time-based scheduling (DispatchQueue.main in production) - FocusStealModeController: the hangul engine surface it needs OngeulInputController now adopts FocusStealDelegate to receive the side-effect callbacks (apply result, post synthetic backspaces, sync icon, expose currentBundleId/client-attached). No behavior change. All 78 existing tests pass. The 215-line diff is ~160 lines removed from the controller plus ~55 added to wire up the corrector instance and the delegate extension. The corrector itself lives in three new files (~400 lines total). This enables deterministic regression tests for race scenarios that the existing 78 tests don't reach (which will follow in the next commit). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
17 deterministic tests cover the focus-steal state machine and the race scenarios the existing 78 tests don't reach: - Happy path: 2 keys buffered → 10ms wait → 2 synthetic backspaces → replay processes both keys - Empty buffer / stale (>500ms) keys: corrector no-ops - Late keys arriving during the 10ms wait via the evidence source: get added to the backspace count and replayed - Keys arriving via handle() during buffering / backspace countdown / replay pending: all consumed and replayed in order - nil keyLabel during buffering: consumed but not buffered (preserves the existing keyLabelFromEvent filtering behavior) - cancel() at each lifecycle phase: no backspace post, no replay - bundleId change between replay enqueue and dispatch: replay skipped (regression test for PR #2's H3 fix) - Detached client at replay time: replay skipped - english→korean force path: forceKoreanForReplay called once, mode transitions - Already-korean path: forceKoreanForReplay not called - Double startCorrection: previous task cancelled, only new fires - Synthetic backspace handling takes precedence over buffering guard Test infrastructure: - ManualScheduler: advance(by:) controls time, runImmediates() triggers enqueued immediate work - FakeKeyEvidence: queues batches of keys for sequential consumeKeys() calls (initial read + late check) - FakeModeController: spies on processKey + forceKoreanForReplay - SpyFocusStealDelegate: records all side-effect callbacks, with configurable bundleId and client-attached state Total: 95 tests pass (78 existing + 17 new). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
hiking90
added a commit
that referenced
this pull request
May 21, 2026
User report: after running ./scripts/install.sh and selecting CapsLock as toggle key, the permission dialog appeared and the System Settings pane opened — but Ongeul never appeared in the Input Monitoring list. Root cause: macOS will only register an app in the TCC Input Monitoring list when the app calls IOHIDManagerOpen AND its Info.plist contains `NSInputMonitoringUsageDescription`. Without the key, the API call is either silently rejected at registration time or the system refuses to list the app for user grant — the on-demand registration flow is dead. This was doc 32 § 미해결 #4 ("NSInputMonitoringUsageDescription IMK 적용성 — 미검증"). Now verified as required. Changes: - Info.plist: add `NSInputMonitoringUsageDescription` (Korean — matches CFBundleDevelopmentRegion = ko). Explains the CapsLock short/long press use and notes it's only used when toggleKey == .capsLock. - Info.plist: add `NSAccessibilityUsageDescription` too. Ongeul has been working with Accessibility without it (older API path was permissive), but recent macOS versions are stricter; this is best practice and makes the System Settings prompt informative. - design/32 § 미해결 #4 updated: mark verified + applied. Plist validated with plutil -lint. User recovery procedure after pulling this fix: 1. ./scripts/install.sh (rebuilds + reinstalls + tccutil reset) 2. **Logout / login** (Info.plist changes require IMK cache flush) 3. Open prefs → change toggleKey to CapsLock → OK 4. App now appears in System Settings → Input Monitoring 5. Toggle on → reselect input source → CapsLock long-press works Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hiking90
added a commit
that referenced
this pull request
May 21, 2026
…an mode) User report: in 영문→long→short→short sequence, the first short press correctly exits real Caps Lock (LED off), but the second short press — intended to toggle to Korean — instead turns the LED back on, looking to the user like Caps Lock re-activated. Root cause: design conflict between doc 30 SET semantics (LED ON = Korean) and doc 32 HID realLockOn (LED ON = real Caps Lock active). In HID mode, the LED was carrying two meanings: - LED on for Korean mode (doc 30 SET via setMode/toggleEngineMode) - LED on for realLockOn (doc 32 explicit setState in enterRealCapsLock) Visually identical, semantically distinct. After exiting realLockOn and then toggling to Korean, the LED turned on for the second reason and the user (correctly) interpreted it as Caps Lock — conflicting with the macOS convention "Caps Lock LED on = uppercase locked". Fix: in HID mode, LED only reflects realLockOn. Mode indicator moves to the menu bar icon entirely. - InputStateCoordinator.setMode and toggleEngineMode: LED sync gate changed from `mode != .hidRealLockOn` to `mode == .cgEventTapAuthority`. doc 30 SET semantics still apply when HID isn't active (fallback); in HID mode, mode changes don't touch LED. - OngeulInputController.performExitRealCapsLock: was `setState(korean)` (LED on if restored to Korean per doc 30 SET); now unconditionally `setState(false)` (LED off — restored mode shows via menu bar icon). - doc 32 updated: § Doc 30과의 관계 clarifies the LED scope split, § 단일 상태 enum module table updated, ASCII state diagram note changed, transitions table includes LED column showing OFF/ON explicitly, 동작 시나리오 #1 and #4 reworded. End-user behavior after this fix: - 영문 → long → short → short → Korean - LED: off → on(realLockOn) → off(exit) → off(toggle) - Mode: English → English+caps → English → Korean - LED never turns on for "just being in Korean mode" — only when real Caps Lock is active. Matches macOS convention; resolves user confusion. Phase 1 PR (#13) unaffected — doc 30 SET continues to apply in the CGEventTap-only baseline (no HID). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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
Extracts the focus-steal state machine from `OngeulInputController` into a dedicated `FocusStealCorrector` class, and adds 17 deterministic regression tests covering race scenarios the existing 78 tests don't reach.
Base branch: `fix/post-review-hardening` (PR #2) — this depends on the focus-steal hardening that lives there (H3 replay WorkItem, N5 forceKoreanForReplay signature). Will be rebased onto main once #2 lands.
What changes
Three commits, each a clean unit:
`refactor: extract RecordedKey to top-level type` — Moves the `RecordedKey` struct out of `KeyEventTap` so it can be owned by the corrector without coupling to that class. Mechanical rename of 13 test references.
`refactor: extract FocusStealCorrector` — The 86-line `correctFocusSteal()` method, six instance properties, the lifecycle cleanup in `activateServer`/`deactivateServer`, and the focus-steal guards in `handleKeyDown` all move into `FocusStealCorrector`. The controller adopts `FocusStealDelegate` to receive side-effect callbacks. No behavior change; 78 existing tests pass.
`test: add FocusStealCorrector regression suite` — 17 new tests + test doubles (`ManualScheduler`, `FakeKeyEvidence`, `FakeModeController`, `SpyFocusStealDelegate`).
Design
The corrector depends on three injectable protocols:
The controller becomes the `FocusStealDelegate`, exposing `applyResult`, `postSyntheticBackspaces`, `syncIconKorean`, `currentBundleId`, and `hasAttachedClient`. This keeps IMK/CGEvent specifics out of the corrector while preserving the existing call sites.
Regression scenarios covered
Stats
Test plan
🤖 Generated with Claude Code