Make config saves atomic and refuse to read a damaged config (#337)

[Timmoth]

Fixes #337.

Saving config.yaml was neither atomic nor durable, and a damaged file was then accepted without complaint on the next load. Together those could lose an inventory.

What was wrong

Writes. PhysicalTextFileStore.WriteAllTextAsync was a bare File.WriteAllTextAsync, which opens with FileMode.Create — the file is truncated to zero before a byte goes back — and never flushes. So an interrupted write could leave a truncated config, and a save that had returned successfully could still be lost to power failure. The migration backup path (BackupOriginalAsync) went through the same unsafe write, so the one existing safety net was written unsafely too.

Reads. A damaged file loaded without an error, and the next save made it permanent. Worse than the issue reported: on current staging an unparseable config loaded as an empty inventory with exit 0 — the boot-time catch added for unreadable stores was swallowing the parse failure too. rpk summary cheerfully printed Hardware (0), and the next write would have persisted that emptiness over a recoverable file.

What this changes

Atomic, durable saves. The store now writes to a temp file in the same directory, flushes through the OS cache to disk (Flush(true)), then renames over the destination. A rename is atomic, so readers only ever see the old or the new file. Failed writes clean up their temp file and leave the destination untouched. This also makes migration backups safe, since they share the path.

Damaged configs fail loudly instead of reading as empty. A new ConfigLoadException is latched on the collection when the config exists but cannot be understood, and every read path (ThrowIfLoadFailed()) and write path (the existing EnsureLoadedAsync()) refuses while it is set. The CLI reports it as Config error: with exit code 5 rather than a stack trace; MCP surfaces the message instead of a generic failure.

Two cases are detected: the file fails to parse, and the file parses but carries no schema version (so it is truncated before the version line, or is not a RackPeek config at all). An empty file — the one the CLI creates on first run — is still a legitimately empty inventory, and legacy configs with no version: key still migrate normally.

The Web UI stays usable for repair. Routes.razor gated the whole app on a successful load, so a damaged config left the UI stuck on "Loading…" — including the /yaml editor that is how you fix it. It now renders anyway; pages that read the inventory surface the failure themselves, and the editor reports a still-broken save inline rather than tearing down the circuit.

What is deliberately not fixed

A save interrupted at a clean resource boundary leaves valid YAML that is indistinguishable from a smaller inventory. Nothing at load time can detect that, which is exactly why the atomic-write half is the primary remedy rather than the detection half. I considered treating "no connections: key" as a damage signal and rejected it: 11 of the 12 v4 test fixtures omit it, so it would break hand-written configs, which are explicitly supported.

Tests

Tests/Yaml/PhysicalTextFileStoreTests.cs (5) — round-trip, full replacement, no temp debris, failed writes leave the original intact, and a concurrent reader sampling the file during 40 alternating writes never observes a partial one. That last test is the bug demonstration: against the old implementation it fails with Observed a partially written config (12288 bytes; expected 200011 or 50011).

Tests/EndToEnd/CorruptConfigTests.cs (6) — an unparseable config and a non-RackPeek file both fail with a message naming the file and never print Hardware (0); a damaged config is byte-for-byte unchanged after an attempted write; an empty config and a healthy config still work; a legacy versionless config still migrates.

Tests.E2e/DamagedConfigTests.cs (2) — with a damaged config staged in the container, the app still renders, the /yaml editor loads it, and repairing it through the editor restores the inventory; and an inventory page never renders an empty list over the damaged file.

The three load-side tests and the concurrency test all fail on the parent commit and pass here. PlaywrightFixture gained WriteConfigAsync/ReadConfigAsync so a test can stage a damaged config, which cannot be produced through the UI (the editor validates before saving).

Full suite green: 375 CLI + 260 discovery + 70 MCP + E2E, dotnet format clean.

🤖 Generated with Claude Code