GetAllChords() returns a live view, so it changes under the caller and throws "Collection was modified" when binding while iterating

[matt-edmondson]

What's wrong

Keybinding/Models/Profile.cs:113 returns a wrapper over the profile's internal dictionary instead of a copy:

public IReadOnlyDictionary<string, Chord> GetAllChords() => Chords.AsReadOnly();

Profile.cs:145 has the same problem:

public IReadOnlyCollection<string> BoundCommands => Chords.Keys;

KeybindingService.GetAllChords() and GetAllChords(profileId) pass this wrapper straight through. The other collection getters (GetAllProfiles, GetAllCommands, …) return ToList() snapshots, so this one behaves differently from the rest of the API.

Failure scenarios (single-threaded, so separate from #109)

1. The returned value changes after it is returned:

   var all = svc.GetAllChords();   // 1 binding
   svc.BindChord("b", chord);
   all.Count;                      // 2, expected 1

2. Binding while enumerating throws:

   foreach (var kv in svc.GetAllChords())
       if (!svc.HasChordBinding("b")) svc.BindChord("b", kv.Value);
   // InvalidOperationException: Collection was modified; enumeration operation may not execute.

Both were reproduced with a temporary MSTest test against the current main.

Code that snapshots bindings for a settings UI, or that copies bindings between profiles, hits this directly. Because the return type is read-only, callers reasonably assume they got a snapshot.

Suggested fix

public IReadOnlyDictionary<string, Chord> GetAllChords() => new Dictionary<string, Chord>(Chords).AsReadOnly();
public IReadOnlyCollection<string> BoundCommands => [.. Chords.Keys];

Acceptance: a dictionary returned by GetAllChords() doesn't change after a later BindChord or UnbindChord, and binding while enumerating it doesn't throw. Add tests for both.

[matt-edmondson]

Triage

• Category: Bug
• Priority: Medium. Single-threaded code that binds while iterating, such as copying bindings between profiles or building a settings UI, hits InvalidOperationException. The read-only return type makes callers assume they have a snapshot, and the other collection getters in this API already return snapshots.
• Area / suggested assignee: Profile.GetAllChords() and Profile.BoundCommands in Models/Profile.cs (:113, :145), which KeybindingService.GetAllChords passes through. Maintainer: @matt-edmondson.
• Duplicates: None. Related: Concurrent BindChord calls corrupt a profile's bindings, despite the "thread-safe services" guarantee #109 (concurrent BindChord corrupts bindings). A snapshot return helps there too, but Concurrent BindChord calls corrupt a profile's bindings, despite the "thread-safe services" guarantee #109 still needs locking.
• In progress: No. Open PR Normalize the key in the Note(NoteName) constructor as the string one does #126 targets Note(NoteName) doesn't uppercase before canonicalizing, so a chord built from NoteName "control" shows "Ctrl+S" but isn't equal to Chord.Parse("Ctrl+S") #121.

Next steps: Return a copied dictionary and key list. Add the two tests from the issue: the snapshot is unchanged after a later bind, and binding during enumeration doesn't throw.

---

Generated by Claude Code