[codex] Fix migration recovery and switch races - #27
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cacda232d
ℹ️ 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".
| guard !didCompleteLegacyMigration || currentRules.isEmpty else { | ||
| return currentRules |
There was a problem hiding this comment.
Recover missing legacy rules despite old completion flag
When a user already has the old didMigrateLegacyAppRules flag set and their current app-rules store contains any rule, this early return discards every loaded legacy rule. That misses the version-2 recovery path for legacy mappings that the original migration could mark complete without actually writing (for example unavailable apps that were previously dropped), and the caller then marks version 2 complete because mergedRules == state.appRules, making those rules unrecoverable. The merge should still add absent legacy entries while avoiding overwrites of existing rules.
Useful? React with 👍 / 👎.
Summary
This change makes the unreleased legacy app-rule migration conservative, cancels stale programmatic input-method switches, and refreshes the README screenshots for the current grouped menu layout.
Problem and user impact
The v2 migration still read legacy input-method mappings without considering whether the original migration had already completed. On upgrade, that could overwrite an explicit default (
.none) choice, recreate a legacy rule the user had deleted, or replace useful current metadata while recovering a placeholder.Programmatic switching also had no shared cancellation identity. When apps were activated in quick succession, an older effect could continue after the foreground app changed. A related termination window existed when the input-method selection notification cleared pending state before the switch effect sent its completion action, allowing a terminated app to receive an incorrect switch count.
The README composites additionally still showed the previous section separators after the menu groups were consolidated.
Root cause
The migration merge treated every missing key and every
.nonerule as recoverable legacy data, without the olddidMigrateLegacyAppRulescompletion context or a way to distinguish untouched placeholders from user-modified rules.The switching effect was not cancellable and relied only on pending state to associate completion. Pending state can be cleared by system selection notifications before the effect itself finishes, so termination could miss the in-flight task.
Changes
.noneplaceholders (createdAt == updatedAt) and preserve current timestamps and valid metadata, using legacy metadata only when current metadata is missing.programmaticSwitchcancellation ID, cancel previous work on activation, and explicitly cancel for ignored/no-target apps, ignored current apps, and terminated apps.This branch is rebased on the current
mainand preserves the metadata regression coverage added by PR #26 while applying the stricter recovery rules above.Validation
rtk just checkgit diff --check./script/validate_readme_screenshots.sh