KeybindingConfiguration's AutoSave, AutoSaveIntervalMilliseconds and DefaultProfileId/Name have no effect: nothing in the library reads IKeybindingConfiguration

[matt-edmondson]

What's wrong

IKeybindingConfiguration (Keybinding/Contracts/IKeybindingConfiguration.cs) and its implementation KeybindingConfiguration (Keybinding/Models/KeybindingConfiguration.cs) are public API that promise behaviour:

• AutoSave - "whether to auto-save changes"
• AutoSaveIntervalMilliseconds - "the auto-save interval in milliseconds (0 to disable)"
• DefaultProfileId - "the default profile ID to use when no profile is active"
• DefaultProfileName

No code in the library consumes any of them. grep -rn "AutoSave\|IKeybindingConfiguration\|DefaultProfileId" Keybinding/ finds only the interface and the class itself:

• KeybindingManager has no constructor or property that takes an IKeybindingConfiguration; it only ever saves when the caller invokes SaveAsync().
• KeybindingManager.CreateDefaultProfile hard-codes "default" / "Default" as parameter defaults rather than reading the configuration.
• ServiceCollectionExtensions.AddKeybinding* never registers or resolves IKeybindingConfiguration.
• The default constructor and the constructor both default AutoSave to true, which suggests saving happens automatically.

Failure scenario

Following USAGE_EXAMPLES.md ("Using Configuration Objects" and "Configuration with Options Pattern"):

var config = new KeybindingConfiguration("./data", autoSave: true, autoSaveIntervalMilliseconds: 3000);
var manager = new KeybindingManager(new CommandRegistry(), new ProfileManager(), new JsonKeybindingRepository(config.DataDirectory));
await manager.InitializeAsync();
manager.CreateDefaultProfile();
manager.Keybindings.BindChord("file.save", Chord.Parse("Ctrl+S"));
// app exits without an explicit SaveAsync()

The caller reasonably expects the binding to have been auto-saved within 3 seconds. Nothing is written to disk, so every change made since start-up is lost on exit. Likewise, a DefaultProfileId of "my-default" is never used: CreateDefaultProfile() still creates "default", and nothing falls back to the configured profile when none is active.

Suggested fix

Pick one, and make the docs match:

1. Wire it up: give KeybindingManager a constructor (and DI registration) that accepts IKeybindingConfiguration; use DefaultProfileId/DefaultProfileName in CreateDefaultProfile when no arguments are passed; when AutoSave is true and the interval is > 0, save after mutations on a debounced timer (disposed in Dispose), or
2. Retire it: mark AutoSave, AutoSaveIntervalMilliseconds, DefaultProfileId and DefaultProfileName [Obsolete] (or remove them in a major release) and drop the auto-save examples from USAGE_EXAMPLES.md.

Acceptance criteria

• Either a configured AutoSave/interval demonstrably persists changes without an explicit SaveAsync() call (covered by a test), and CreateDefaultProfile() honours DefaultProfileId/DefaultProfileName; or those members are obsolete/removed and no documentation implies they do anything.

[matt-edmondson]

Triage

• Category: Bug (public API and USAGE_EXAMPLES.md promise behaviour that nothing implements)
• Priority: Medium. A caller who trusts AutoSave = true (the default) loses every binding change made since start-up on exit, and gets no error. The workaround is easy (call SaveAsync()), and any app that already saves explicitly is unaffected, so it isn't High.
• Area / suggested assignment: Keybinding/Contracts/IKeybindingConfiguration.cs, Keybinding/Models/KeybindingConfiguration.cs, KeybindingManager (CreateDefaultProfile), ServiceCollectionExtensions, and USAGE_EXAMPLES.md.
• Duplicates: none found. Related to Delegate profile/command JSON persistence to ktsu.AppDataStorage #93 (delegating persistence to ktsu.AppDataStorage). If that lands, it changes where an auto-save timer would live.
• In progress: no. The open Keybinding PR Lock a profile's chords so concurrent binds cannot corrupt them [minor] #142 locks profile chords against concurrent binds. That's a different change, but a debounced auto-save timer would also need that lock.

Suggested next step: choose between wiring it up and retiring it before anyone writes code. Given #93, retiring AutoSave/AutoSaveIntervalMilliseconds with [Obsolete] now and fixing the docs is the cheaper, safer option. DefaultProfileId/DefaultProfileName are simple to honour in CreateDefaultProfile() either way. Wiring up auto-save should wait until #142 merges, so the timer's save runs under the same lock as mutations.

---

Generated by Claude Code