One invalid chord entry in profiles.json makes KeybindingManager.InitializeAsync throw, although a syntactically corrupt file is tolerated

[matt-edmondson]

What's wrong

JsonKeybindingRepository.LoadAllProfilesInternalAsync (Keybinding/Services/JsonKeybindingRepository.cs:~99-120) catches only JsonException. For a corrupted file it returns an empty list.

The DTO-to-model conversion inside the same try builds Profile, Note and Chord objects. Those constructors throw ArgumentException when the JSON is well-formed but the content is invalid:

• a chord with an empty notes array
• a note string that is empty or whitespace
• a profile with an empty id or name

That ArgumentException escapes LoadAllProfilesAsync, so KeybindingManager.InitializeAsync fails and the app can't start. SaveProfileAsync and DeleteProfileAsync reload the file first, so they throw the same exception and SaveAsync breaks too.

A hand-edited profile file is enough to hit this, and so is one written by an older version or partly migrated.

Reproduction (verified against the built library)

Write profiles.json in the data directory:

[{"id":"default","name":"Default","chords":{"file.save":{"notes":[]}}}]

Then run await new KeybindingManager(dir).InitializeAsync();.

• Actual: ArgumentException: Chord must contain at least one note (Parameter 'notes') escapes InitializeAsync. If the file contains {not json instead, InitializeAsync succeeds, because the JsonException is swallowed.
• Expected: the same tolerance as the corrupted-file path. The invalid binding or profile is skipped, the rest of the file loads, and nothing crashes.

Suggested fix / acceptance criteria

• In the conversion, catch ArgumentException for each entry:
  • skip a chord entry whose notes are empty or invalid
  • skip a profile whose id or name is invalid
  • at minimum, catch ArgumentException alongside JsonException
• Make sure a later SaveProfileAsync doesn't silently drop the skipped entries without telling the caller. For example, log or surface them.
• Acceptance: the JSON above loads the default profile without a file.save binding, and InitializeAsync doesn't throw. Add a test for it next to the existing corrupted-file test.

[matt-edmondson]

Triage

• Category: Bug
• Priority: High. One well-formed but invalid entry in profiles.json stops InitializeAsync, so the host app can't start. SaveProfileAsync and DeleteProfileAsync reload the file first, so they fail too. The file only has to be hand-edited or written by an older version. A syntactically corrupt file is handled gracefully, which makes the difference in tolerance surprising.
• Area / suggested owner: JsonKeybindingRepository.LoadAllProfilesInternalAsync, the DTO-to-model conversion (maintainer: matt-edmondson)
• Duplicates / in progress: No duplicate found, and no open PR covers it. Related persistence issues: SaveAsync on a manager that never called InitializeAsync deletes every stored profile #124 (SaveAsync before InitializeAsync deletes profiles) and Delegate profile/command JSON persistence to ktsu.AppDataStorage #93 (delegate persistence to AppDataStorage). A fix for Delegate profile/command JSON persistence to ktsu.AppDataStorage #93 should keep the per-entry tolerance.
• Notes: Skip entries one at a time, catching ArgumentException per chord and per profile, rather than using a blanket catch. That way one bad binding doesn't discard the whole profile. Add a test next to the existing corrupted-file test.

---

Generated by Claude Code