fix(rules): keep a global default of None across relaunch#48
Conversation
`AppState.start()` seeded the global default lock from the live input source whenever `config.defaultSourceID == nil`. That `nil` is overloaded: it means both "genuine first run, never configured" and "the user deliberately chose None". So every relaunch re-seeded a chosen None back to whatever source was active at launch (e.g. ABC), then persisted it — the user's None silently became ABC. Add `RuleStore.hasPersistedConfiguration`, which reports whether a config has ever been written to `defaults`. That is the only signal that tells a true first run apart from a returning user, since both load as `defaultSourceID == nil`. Gate the seed on it: `start()` captures `isFirstRun` before loading and seeds only then, so a persisted None survives every relaunch. Signed-off-by: Kevin Cui <bh@bugs.cc>
Summary by CodeRabbit
WalkthroughThis PR fixes a bug where setting the global default locked input source to "None" did not persist across app restarts. Sequence Diagram(s)sequenceDiagram
participant AppState
participant RuleStore
participant UserDefaults
AppState->>RuleStore: hasPersistedConfiguration
RuleStore->>UserDefaults: data(forKey: key)
UserDefaults-->>RuleStore: data or nil
RuleStore-->>AppState: isFirstRun (Bool)
AppState->>RuleStore: load()
RuleStore-->>AppState: config
alt isFirstRun && defaultSourceID == nil
AppState->>AppState: seed global default from active source
else returning user with None default
AppState->>AppState: keep defaultSourceID as nil
end
Related Issues: Suggested Labels: bug, fix Suggested Reviewers: (none identified) Poem A rabbit checked its saved-state key, 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/LockIMEKit/Rules/RuleStore.swift (1)
26-36: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider gating on decodability, not just raw data presence.
hasPersistedConfigurationonly checks that someDataexists under the key — it doesn't verify thatload()can actually decode it. If a future schema change (or corrupted defaults) makes decoding fail,load()'stry?falls back to.default(nildefaultSourceID), yethasPersistedConfigurationstill reportstrue. Downstream inAppState.start(), that meansisFirstRunisfalseeven though no usable config was ever recovered, so the app won't reseed the default from the live input source — the very case this PR is fixing to avoid, but now triggered by data corruption rather than user intent, and with no way to self-heal.Since
load()already contains the decode logic, this property could reuse it rather than duplicating the "does data exist" check with different semantics:♻️ Suggested approach: derive from decode success
public var hasPersistedConfiguration: Bool { - defaults.data(forKey: key) != nil + guard let data = defaults.data(forKey: key) else { return false } + return (try? JSONDecoder().decode(LockConfiguration.self, from: data)) != nil }This is a narrow edge case (requires a future breaking schema change or corrupted defaults), but the current implementation's failure mode is a silent, permanent "stuck on None" regression that's hard to distinguish from a deliberate user choice.
🤖 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 `@Sources/LockIMEKit/Rules/RuleStore.swift` around lines 26 - 36, `hasPersistedConfiguration` in `RuleStore` should reflect whether a saved config is actually usable, not just whether raw `Data` exists. Update it to reuse `load()`’s decode path (or equivalent decodability check) so corrupted or schema-mismatched defaults are treated like no persisted configuration. This keeps `AppState.start()`’s first-run detection aligned with `load()` and avoids suppressing reseed when recovery fails.
🤖 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.
Nitpick comments:
In `@Sources/LockIMEKit/Rules/RuleStore.swift`:
- Around line 26-36: `hasPersistedConfiguration` in `RuleStore` should reflect
whether a saved config is actually usable, not just whether raw `Data` exists.
Update it to reuse `load()`’s decode path (or equivalent decodability check) so
corrupted or schema-mismatched defaults are treated like no persisted
configuration. This keeps `AppState.start()`’s first-run detection aligned with
`load()` and avoids suppressing reseed when recovery fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 42c834d7-185c-4408-af5d-54c926a449dc
📒 Files selected for processing (3)
Sources/LockIME/AppState.swiftSources/LockIMEKit/Rules/RuleStore.swiftTests/LockIMEKitTests/RuleStoreTests.swift
1 participant
Setting App Rules → Global default → Locked input source to
Nonedidn't survive a restart: on every launch it reverted to the currently active source (e.g.U.S./ABC), forcing that input method on any app without a per-app rule.The cause is an overloaded
nil.AppState.start()seeded the global default from the live input source wheneverconfig.defaultSourceID == nil, but thatnilmeans two different things — "genuine first run, never configured" and "the user deliberately chose None". Both load identically, so a chosenNonewas re-seeded to whatever source was active at launch and then persisted over the user's choice, every relaunch. A concrete source persisted fine because it isn'tnil, which matches the report.The fix adds
RuleStore.hasPersistedConfiguration— whether a config has ever been written todefaults— as the one signal that tells a true first run apart from a returning user.start()capturesisFirstRunbefore loading and gates the seed on it, so a persistedNoneis left untouched.Added a regression test asserting a saved config with a
nildefault still counts as persisted.Closes #46