You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
RenameProfile clears the description when none is passed, and replaces the Profile object so held references silently stop persisting #113
ProfileManager.RenameProfile (Keybinding/Services/ProfileManager.cs:~153-185) has two defects:
newDescription is documented as optional, but leaving it out erases the description. The new profile is built with newDescription as-is, so passing null sets Description = null instead of keeping the current value.
It builds a brand-new Profile and swaps it in with TryRemove followed by TryAdd. Any reference a caller already holds is now detached from the manager. That includes the objects returned by CreateProfile, CreateDefaultProfile and GetActiveProfile. Writes through that reference never reach GetProfile or SaveAsync.
Failure scenario (reproduced with a temporary MSTest)
varp=pm.CreateProfile("p","Old","my desc");pm.RenameProfile("p","New Name");pm.GetProfile("p").Description// null (expected "my desc")
p.SetChord("x.y",chordA);pm.GetProfile("p") has x.y?// False: the binding is silently lost
By code trace (not hit in a 20k-iteration probe): between the remove and the add, a concurrent GetProfile returns null. A concurrent CreateProfile(id) in that window makes the TryAdd fail, which drops the renamed profile and all its bindings.
Suggested fix
Keep the existing description when newDescription is null: newDescription ?? profile.Description.
Rename in place. Make Name/Description settable, internally or under the manager's lock, so the stored instance and any held references stay the same object. If replacement has to stay, use _profiles[id] = updated or TryUpdate rather than a remove/add pair.
Acceptance:
after RenameProfile(id, newName), the description is unchanged;
ReferenceEquals(before, GetProfile(id)) holds;
chords set through the earlier reference are visible and persisted.
Priority: Medium. Two defects: a rename without a description erases it, and bindings written through a reference held from before the rename are silently lost. Both are real data loss, but only on the rename path. The concurrent CreateProfile window is by code trace only.
Notes:newDescription ?? profile.Description is a one-line fix and can ship on its own. Renaming in place is the more invasive half, because Name/Description are currently immutable. Decide whether that is a public API change (settable properties) or stays internal.
Activity
matt-edmondson commentedon Sep 26, 2026
Triage
CreateProfilewindow is by code trace only.ProfileManager.RenameProfile,Profile(Name/Descriptionsetters)RenameProfileremove/add swap as a smaller second hole. Also related to Concurrent CreateProfile calls for the same id can return a Profile that is never stored, so its bindings are silently lost #112 (map-level race inCreateProfile). Fixing all three together, with one lock orConcurrentDictionarydiscipline inProfileManager, avoids three overlapping edits.Notes:
newDescription ?? profile.Descriptionis a one-line fix and can ship on its own. Renaming in place is the more invasive half, becauseName/Descriptionare currently immutable. Decide whether that is a public API change (settable properties) or stays internal.Generated by Claude Code