feat(rules): add one-shot switch action for app/URL rules#22
Conversation
Per-app and per-URL rules could only lock a source — continuously re-applied while the rule is in force. Some workflows just want to land on a source when an app or page becomes active, then stay free to change it. This adds a second action, switch, as a fourth app mode (.switched) and a URL RuleAction (.switchOnce); the global default stays lock-only. The engine fires the one-shot exactly once per genuine entry via an in-memory transition key, with a separate slot for a launcher overlay's own switch so an excursion never re-yanks the underlying app. A switch installs no standing target, so the user is never reverted after moving away. Import/export carries the new action: app rules through their mode, URL rules through an added action field with lenient decoding so existing v1.x UserDefaults and pre-release .lockime backups still load (a missing action defaults to lock). Signed-off-by: Kevin Cui <bh@bugs.cc>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughThis PR adds a "switch once" action as an alternative to continuous "locking" for per-app and per-URL input source rules, addressing the user request to switch to other input methods rather than remaining permanently locked. A new Possibly Related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
98-100:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the XcodeGen/xcbeautify sentence inline.
The leading
+at the start of line 100 will render as a list item in Markdown, breaking the paragraph. Please foldxcbeautifyback into the same sentence.🛠 Proposed fix
-Requires Xcode 26+ (the app itself targets macOS 14+), and -[XcodeGen](https://github.com/yonaskolb/XcodeGen) -+ [xcbeautify](https://github.com/cpisciotta/xcbeautify) (`brew install xcodegen xcbeautify`). +Requires Xcode 26+ (the app itself targets macOS 14+), and [XcodeGen](https://github.com/yonaskolb/XcodeGen) + [xcbeautify](https://github.com/cpisciotta/xcbeautify) (`brew install xcodegen xcbeautify`).🤖 Prompt for 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. In `@README.md` around lines 98 - 100, The `+` character at the beginning of the line containing `[xcbeautify]` is being interpreted as a Markdown list marker, which breaks the paragraph flow. Remove the line break before the `+` and fold the `xcbeautify` installation instruction back into the same sentence as the XcodeGen reference, keeping the entire installation instruction as a continuous paragraph without list markers.
🤖 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 `@Sources/LockIMEKit/LockEngine/LockEngine.swift`:
- Around line 241-252: The `fireSwitchOnceIfNeeded` function unconditionally
assigns `slot = key` after calling `controller.switchOnce(...)`, but since
`switchOnce` can early-return without actually performing the switch when the
current source is unknown, this marks the one-shot as already fired even though
no switch occurred. Only assign `slot = key` after `controller.switchOnce(...)`
completes successfully by checking the return value of `switchOnce` and only
updating `slot` when the switch actually executed, rather than updating it
unconditionally.
In `@Tests/LockIMEKitTests/LockEngineTests.swift`:
- Around line 801-834: The test switchActivationReasons has a mismatch between
its intent and assertions in the URL switch section. The test name indicates URL
switches should be logged as .urlMatched, but the URL rule branch only asserts
.startupApplied after calling apply on engine2. To fix this, add a re-activation
step (such as calling monitor2.activate() with the Safari bundle ID) after
applying the configuration to trigger the URL matching behavior, then update the
assertion from events2.last?.reason == .startupApplied to events2.last?.reason
== .urlMatched to properly validate that URL switches are logged with the
correct activation reason.
---
Outside diff comments:
In `@README.md`:
- Around line 98-100: The `+` character at the beginning of the line containing
`[xcbeautify]` is being interpreted as a Markdown list marker, which breaks the
paragraph flow. Remove the line break before the `+` and fold the `xcbeautify`
installation instruction back into the same sentence as the XcodeGen reference,
keeping the entire installation instruction as a continuous paragraph without
list markers.
🪄 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: 0c9f1310-8e20-464c-8e1f-6622628c6fed
📒 Files selected for processing (28)
README.mdSources/LockIME/AppState.swiftSources/LockIME/Localizable.xcstringsSources/LockIME/UI/Settings/AppRulesSettingsPane.swiftSources/LockIME/UI/Settings/ImportReviewSheet.swiftSources/LockIME/UI/Settings/URLRulesSettingsPane.swiftSources/LockIMEKit/Backup/ConfigBackup.swiftSources/LockIMEKit/Backup/ImportPlan.swiftSources/LockIMEKit/LockEngine/LockController.swiftSources/LockIMEKit/LockEngine/LockEngine.swiftSources/LockIMEKit/Rules/LockConfiguration.swiftSources/LockIMEKit/Rules/RuleResolver.swiftTests/LockIMEKitTests/ConfigBackupTests.swiftTests/LockIMEKitTests/ImportPlanTests.swiftTests/LockIMEKitTests/LockConfigurationTests.swiftTests/LockIMEKitTests/LockControllerTests.swiftTests/LockIMEKitTests/LockEngineTests.swiftTests/LockIMEKitTests/RuleResolverTests.swiftTests/LockIMEKitTests/RuleStoreTests.swiftdocs/DESIGN.mddocs/README/README.de.mddocs/README/README.es.mddocs/README/README.fr.mddocs/README/README.ja.mddocs/README/README.pt.mddocs/README/README.ru.mddocs/README/README.zh-CN.mddocs/README/README.zh-TW.md
fireSwitchOnceIfNeeded marked the one-shot as fired even when LockController.switchOnce early-returned because the current input source could not be read (a transient TIS failure), permanently suppressing the intended switch for that entry. Guard on a readable source before consuming the key, so the one-shot stays eligible for the next reevaluation. When the source is known but already equals the target, the switch still no-ops and the key is consumed, so a user who later moves away from an app entered already on target is not re-yanked. Also fix a test whose name promised a URL .urlMatched assertion but only checked the apply-driven reason: add a navigation step that re-resolves to a different URL switch rule and asserts .urlMatched. Signed-off-by: Kevin Cui <bh@bugs.cc>
Per-app and per-URL rules could only lock a source — continuously re-applied while the rule is in force. Some workflows just want to land on a source when an app or page becomes active, then stay free to change it. This adds a second action, switch, as a fourth app mode (.switched) and a URL RuleAction (.switchOnce); the global default stays lock-only.
The engine fires the one-shot exactly once per genuine entry via an in-memory transition key, with a separate slot for a launcher overlay's own switch so an excursion never re-yanks the underlying app. A switch installs no standing target, so the user is never reverted after moving away.
Import/export carries the new action: app rules through their mode, URL rules through an added action field with lenient decoding so existing v1.x UserDefaults and pre-release .lockime backups still load (a missing action defaults to lock).
Closed: #20