Recover an interrupted write instead of the stale backup

[matt-edmondson]

Fixes #310

The defect

WriteText deletes the main file before moving the temp file into its place:

FileSystem.File.Delete(appData.FilePath);   // original gone
...
FileSystem.File.Move(tempFilePath, appData.FilePath);   // replacement arrives

A process killed in that window leaves: main missing, .tmp holding the save that was in flight, .bk holding the content before it.

ReadText's FileNotFoundException handler only ever looked for .bk. So the next load restored the previous content, discarded the newest save with no error surfaced to the caller, and left the .tmp orphaned on disk — nothing reads it again, and nothing cleans it up.

The change

The recovery handler tries the temp file first, then the backup:

if (TryRestoreFrom(MakeTempFilePath(appData.FilePath), appData.FilePath)
	|| TryRestoreFrom(MakeBackupFilePath(appData.FilePath), appData.FilePath))
{
	return ReadText(appData);
}

TryRestoreFrom is the existing backup-recovery body — copy the candidate over the main file, then move it aside under a unique timestamped name — lifted into a helper so both candidates get the same treatment rather than the temp path growing a second copy of it.

Why the ordering is safe. WriteText writes the temp file in full (WriteAllText) before entering the block that deletes anything, so whenever the main file is missing because of this window, the temp file holds a complete write. Preferring it is not a guess about which file is newer — it is the only ordering in which the window can occur.

Why archive rather than delete. Moving the consumed candidate aside is what keeps LoadOrCreate's corrupt-file recovery terminating. That path deletes a file it cannot deserialize and reads again; a candidate left in place would be promoted again on every pass, and a deleted one would lose content that is worth keeping for inspection. Archiving also means a temp file that was itself truncated — a crash during WriteAllText, where the main file still exists and is authoritative — can never be promoted twice.

ReadText's fall-through is unchanged when no temp file exists, which is the normal case: WriteText's Move consumes the temp file on every successful save, so it is only ever present after an interruption.

Tests

test	covers
TestReadTextRecoversTheInterruptedWriteRatherThanTheStaleBackup	the defect — the exact on-disk state from the issue, asserting the in-flight content is what comes back
TestReadTextArchivesTheRecoveredTempFileRatherThanLeavingItOrphaned	the second half of the report — the temp file is gone from .tmp and present under an archive name

The second is not redundant. A fix that read the temp file and left it in place would pass the first while leaving the orphan the issue also reports; one that deleted it would pass both halves of "not orphaned" while losing the content and breaking the corrupt-file loop. Asserting on the archive pins the behaviour the fix actually relies on.

SetUpInterruptedWrite builds the state the issue describes rather than trying to kill a process mid-write, which is what makes this testable at all against MockFileSystem.

TestReadTextRestoresFromBackupIfMainFileMissing is untouched and still passes — with no temp file present, the backup path runs exactly as before.

Proved failing without the fix. Reverting only AppDataStorage/AppData.cs and keeping both tests:

failed TestReadTextRecoversTheInterruptedWriteRatherThanTheStaleBackup (55ms)
  Assertion failed. Expected strings to be equal.
  The newest save should be recovered, not the stale backup.
failed TestReadTextArchivesTheRecoveredTempFileRatherThanLeavingItOrphaned (54ms)
  Assertion failed. Expected condition to be false.
  The consumed temp file should not be left orphaned on disk.

  total: 105   failed: 2   succeeded: 103

Both fail on the reported symptoms — the stale backup winning, and the orphan left behind — not on a message that merely changed shape. The 103 pre-existing tests are unaffected in both directions.

Verification

• dotnet build AppDataStorage.sln -c Release — succeeded, 0 errors
• dotnet test AppDataStorage.sln -c Release — 105 total, 105 passed, 0 failed, 0 skipped
• Same suite against reverted AppData.cs — 2 of 105 failed, as above

Run on .NET SDK 10.0.401, Linux. The 3 build warnings (System.Text.Encodings.Web / System.IO.Pipelines / System.Text.Json 10.0.12 reporting no net7.0 support) are pre-existing and unrelated — they reproduce identically with this change reverted.

What this does not do

The issue offers a second, complementary fix: restructure WriteText so the original is never deleted before the replacement is in place. That is deliberately left alone here.

It is the better long-term shape, but it is a change to the write protocol rather than to recovery — it needs an overwriting Move, which netstandard2.0 (one of this library's six target frameworks) does not offer, so it would mean a per-TFM implementation of the durability path. That belongs in its own change, weighed on its own. This fix is also still needed afterwards: any repository written by a version that has the current WriteText can already have an orphaned .tmp on disk, and only ReadText can recover it.

Worth stating plainly: this narrows the consequence of the window rather than closing the window. A crash still loses nothing, but the save is recovered on the next load rather than never — it does not make WriteText atomic.

🤖 Generated with Claude Code

https://claude.ai/code/session_012Pup1E96GmvMiRTNMdABat

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud