SaveAsync without InitializeAsync wipes stored commands and the active profile, leaving the preserved profiles' bindings dead

[matt-edmondson]

What's wrong

The #124 fix (PR #139) made KeybindingManager.SaveAsync (Keybinding/KeybindingManager.cs:113-146) keep stored profiles this manager never loaded, and SaveWithoutInitializeTests.SaveAsync_WithoutInitialize_KeepsStoredProfiles pins that. The other two files SaveAsync writes are still overwritten wholesale, though:

• L118-119: Repository.SaveCommandsAsync(Commands.GetAllCommands()) replaces commands.json with only the commands in memory.
• L144-145: Repository.SaveActiveProfileAsync(activeProfile?.Id) replaces active-profile.json, writing null when nothing is active.

JsonKeybindingRepository writes both files outright, with no merge.

Failure scenario (reproduced with a throwaway MSTest)

1. Manager A calls InitializeAsync, registers file.save, calls CreateDefaultProfile(), binds Ctrl+S → file.save, then calls SaveAsync.
2. Manager B uses the same directory and never calls InitializeAsync. It calls Profiles.CreateProfile("other", "Other"), then SaveAsync.
3. Manager C calls InitializeAsync.

Result: the profiles are other, default, there are 0 commands, and active is null. ExecuteChord("default", Ctrl+S) returns null.

The profile and its binding survive, as #124 intended, but the command the binding points to is gone. ExecuteChord skips unregistered commands, so the binding is dead, and the user's active profile is lost too. Keeping the profiles was the point of #124, and this makes that only half-effective.

Suggested fix

• Handle commands the way profiles are handled now. Track the command ids this manager loaded or saved. On save, merge with the stored commands.json, and remove only tracked ids that have since been unregistered.
• Write active-profile.json only if this manager loaded it or changed the active profile. Otherwise leave it alone.

Acceptance criteria

• The scenario above ends with file.save still registered, default still active, and Ctrl+S resolving to file.save.
• A command unregistered by a manager that did load it is still removed on save.
• Tests extend SaveWithoutInitializeTests to cover both.

Related but distinct: #148 (concurrent saves) and #93 (atomic writes).

[matt-edmondson]

Triage

• Category: Bug
• Priority: Medium. This loses persisted user data: registered commands and the active profile. It only happens when a manager saves without calling InitializeAsync, but that is exactly the path SaveAsync on a manager that never called InitializeAsync deletes every stored profile #124/PR Only prune stored profiles this manager loaded or saved #139 set out to make safe, and at the moment it is only half safe.
• Area / suggested assignment: KeybindingManager.SaveAsync and JsonKeybindingRepository persistence
• Related: SaveAsync on a manager that never called InitializeAsync deletes every stored profile #124 (the fix this completes), Concurrent saves silently lose profiles: JsonKeybindingRepository does unsynchronised read-modify-write of profiles.json #148 (concurrent saves), and Delegate profile/command JSON persistence to ktsu.AppDataStorage #93 (delegating persistence to AppDataStorage). If Delegate profile/command JSON persistence to ktsu.AppDataStorage #93 goes ahead, the command merge added here should move with it rather than be rewritten.
• Duplicates: none. Concurrent saves silently lose profiles: JsonKeybindingRepository does unsynchronised read-modify-write of profiles.json #148 is a race between two managers. This issue is one manager that never loaded the data.
• In progress: no open PR in this repository.

---

Generated by Claude Code