A torn or corrupt profiles.json loads as zero profiles and the next SaveAsync overwrites it for good (non-atomic saves, corrupt file not kept)

[matt-edmondson]

What's wrong

JsonKeybindingRepository writes every file with File.WriteAllTextAsync directly onto the target path:

• SaveProfileAsync:

  Keybinding/Keybinding/Services/JsonKeybindingRepository.cs

  Lines 43 to 72 in 03d498c

  	public async Task SaveProfileAsync(Profile profile)
  	{
  	Ensure.NotNull(profile);
  	await EnsureInitializedAsync().ConfigureAwait(false);
  	List<Profile> profiles = await LoadAllProfilesInternalAsync().ConfigureAwait(false);
  	List<Profile> profileList = [.. profiles];
  	// Remove existing profile with same ID if it exists
  	profileList.RemoveAll(p => p.Id == profile.Id);
  	profileList.Add(profile);
  	string profilesPath = Path.Combine(_dataDirectory, ProfilesFileName);
  	IEnumerable<ProfileDto> profileDtos = profileList.Select(p => new ProfileDto
  	{
  	Id = p.Id,
  	Name = p.Name,
  	Description = p.Description,
  	Chords = p.GetAllChords().ToDictionary(
  	kvp => kvp.Key,
  	kvp => new ChordDto
  	{
  	Notes = [.. kvp.Value.Notes.Select(n => n.ToString())]
  	})
  	});
  	string json = JsonSerializer.Serialize(profileDtos, _jsonOptions);
  	await File.WriteAllTextAsync(profilesPath, json).ConfigureAwait(false);
  	}

• DeleteProfileAsync:

  Keybinding/Keybinding/Services/JsonKeybindingRepository.cs

  Lines 148 to 149 in 03d498c

  	string json = JsonSerializer.Serialize(profileDtos, _jsonOptions);
  	await File.WriteAllTextAsync(profilesPath, json).ConfigureAwait(false);

• SaveCommandsAsync:

  Keybinding/Keybinding/Services/JsonKeybindingRepository.cs

  Lines 168 to 169 in 03d498c

  	string json = JsonSerializer.Serialize(commandDtos, _jsonOptions);
  	await File.WriteAllTextAsync(commandsPath, json).ConfigureAwait(false);

• SaveActiveProfileAsync:

  Keybinding/Keybinding/Services/JsonKeybindingRepository.cs

  Lines 208 to 209 in 03d498c

  	string json = JsonSerializer.Serialize(activeProfileDto, _jsonOptions);
  	await File.WriteAllTextAsync(activeProfilePath, json).ConfigureAwait(false);

WriteAllTextAsync truncates the file before writing, so a crash or power loss partway through leaves a truncated profiles.json. That one file holds every profile, and KeybindingManager.SaveAsync rewrites it once per profile, so the window is large.

On the next start, LoadAllProfilesInternalAsync catches the JsonException and silently returns an empty list (#L116-L120). LoadCommandsAsync does the same for commands.json (#L193-L197). The app therefore starts with no profiles. The usual startup path, CreateDefaultProfile() followed by SaveAsync(), then overwrites the damaged file, and every user binding it contained is lost for good.

Reproduction (verified against current main)

var dir = /* temp dir */;
using (var m = new KeybindingManager(dir)) {
    await m.InitializeAsync();
    m.Commands.RegisterCommand(new Command("file.save", "Save"));
    for (int i = 0; i < 5; i++) m.Profiles.CreateProfile("p" + i, "P" + i).SetChord("file.save", Chord.Parse("Ctrl+S"));
    await m.SaveAsync();
}
// simulate a torn write: keep only the first half of profiles.json
var path = Path.Combine(dir, "profiles.json");
var full = File.ReadAllText(path); File.WriteAllText(path, full[..(full.Length / 2)]);

using (var m = new KeybindingManager(dir)) {
    await m.InitializeAsync();   // no error; GetAllProfiles().Count == 0
    m.CreateDefaultProfile();
    await m.SaveAsync();         // profiles.json now contains only "default"
}

Result: after the reload the profile count is 0, and the final profiles.json holds only the empty default profile. The five saved profiles and their bindings are gone for good.

Suggested fix

• Write each file to a temp file in the same directory, then swap it into place with File.Move(temp, target, overwrite: true) or File.Replace. A crash then leaves either the old file or the new one, never half of one.
• Keep tolerating a file that fails to parse, but first move it aside (for example to profiles.json.corrupt-<timestamp>) so the next save cannot destroy the only copy.

Acceptance criteria

• A test that truncates profiles.json, reloads, creates a profile and saves finds the original bytes preserved in a side file.
• No save path truncates the live file in place; all of them go through a temp file and a move or replace.
• Loading a corrupt file still returns empty rather than throwing, as it does today.

Related: #148 (concurrent read-modify-write races between saves) is a different root cause in the same code.

[matt-edmondson]

Triage

• Category: Bug
• Priority: High. This is permanent, silent loss of user data. A torn write loads as zero profiles, and the usual startup path then overwrites the only copy. The window is wide because SaveAsync rewrites profiles.json once per profile.
• Area: Keybinding/Services/JsonKeybindingRepository.cs (all save paths and the load error handling)
• Suggested assignment: Persistence/repository maintainer
• Related, not duplicates: Concurrent saves silently lose profiles: JsonKeybindingRepository does unsynchronised read-modify-write of profiles.json #148 (concurrent read-modify-write races) and SaveAsync without InitializeAsync wipes stored commands and the active profile, leaving the preserved profiles' bindings dead #150 (SaveAsync without InitializeAsync wipes data) are in the same repository code. Consider fixing them together, since the temp-file-and-replace write and a save lock touch the same methods.
• Duplicates / in progress: none found; the only open PR (Bump the ktsu group with 1 update #155) is a dependency bump

---

Generated by Claude Code