Make config saves atomic and refuse to read a damaged config (#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.

Writes went through File.WriteAllTextAsync, which truncates the file to
zero before writing a byte and never flushes — so an interrupted save
could leave a truncated config, and a save that had returned successfully
could still be lost to power failure. Migration backups shared that path,
so the one existing safety net was written unsafely too. The store now
writes a temp file, flushes it to disk, and renames over the destination;
a failed write cleans up and leaves the original untouched.

Reads were worse than the issue reported: on staging an unparseable
config loaded as an EMPTY inventory with exit 0, because the boot-time
catch for unreadable stores was swallowing parse failures too. A
ConfigLoadException is now latched on the collection when the config
exists but cannot be understood, and every read and write refuses while
it is set — the CLI reports "Config error:" with exit 5, MCP forwards the
message. Two cases are detected: the file fails to parse, and the file
parses with no schema version (truncated before the version line, or not
a RackPeek config). An empty file is still a legitimately empty
inventory, and legacy versionless configs still migrate.

Routes.razor gated the whole app on a successful load, so a damaged
config left the Web UI stuck on "Loading…" — including the YAML editor
that is how you fix it. Both hosts now render anyway, and the editor
reports a still-broken save inline instead of tearing down the circuit.

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 why the atomic-write half is the primary remedy.

Tests: 5 store tests (the concurrency one fails on the parent commit with
a 12288-byte partial read), 6 CLI e2e tests, 2 Playwright tests covering
repair through the YAML editor.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

RackPeek.Domain/Helpers/ConfigLoadException.cs

@@ -0,0 +1,18 @@
+namespace RackPeek.Domain.Helpers;
+
+/// <summary>
+///     The config file exists but cannot be read as a RackPeek document — damaged,
+///     truncated, or not YAML. Distinct from an unreadable store (IO errors), which is
+///     tolerated at boot: a damaged file must fail loudly on every read and write so a
+///     partial or empty inventory is never served, and never persisted over the
+///     user's file (#337).
+/// </summary>
+public sealed class ConfigLoadException : Exception {
+    public ConfigLoadException(string message)
+        : base(message) {
+    }
+
+    public ConfigLoadException(string message, Exception innerException)
+        : base(message, innerException) {
+    }
+}

RackPeek.Domain/Persistence/Yaml/ITextFileStore.cs

@@ -1,3 +1,5 @@
+using System.Text;
+
 namespace RackPeek.Domain.Persistence.Yaml;
 
 public interface ITextFileStore {
@@ -11,5 +13,51 @@ public sealed class PhysicalTextFileStore : ITextFileStore {
 
     public Task<string> ReadAllTextAsync(string path) => File.ReadAllTextAsync(path);
 
-    public Task WriteAllTextAsync(string path, string contents) => File.WriteAllTextAsync(path, contents);
+    /// <summary>
+    ///     Atomic and durable replacement for File.WriteAllTextAsync, which truncates
+    ///     the destination before writing and never flushes to disk — so a crash or
+    ///     power loss mid-save could leave a truncated config, and a save that had
+    ///     "succeeded" could still be lost (#337). The content is written to a
+    ///     temporary file in the same directory, flushed to disk, then moved over the
+    ///     destination — a rename, so readers only ever see the old or the new file,
+    ///     never a partial one.
+    /// </summary>
+    public async Task WriteAllTextAsync(string path, string contents) {
+        var fullPath = Path.GetFullPath(path);
+        var directory = Path.GetDirectoryName(fullPath)
+                        ?? throw new IOException($"'{path}' has no parent directory.");
+
+        var tempPath = Path.Combine(
+            directory,
+            $"{Path.GetFileName(fullPath)}.tmp-{Guid.NewGuid():N}");
+
+        try {
+            await using (var stream = new FileStream(
+                             tempPath,
+                             FileMode.CreateNew,
+                             FileAccess.Write,
+                             FileShare.None)) {
+                var bytes = Encoding.UTF8.GetBytes(contents);
+                await stream.WriteAsync(bytes);
+
+                // Flush through the OS cache to the disk itself, so the rename
+                // below never publishes a file whose bytes could still vanish.
+                stream.Flush(true);
+            }
+
+            File.Move(tempPath, fullPath, true);
+        }
+        catch {
+            // Never leave temp files behind on a failed write; the destination
+            // is untouched by construction.
+            try {
+                File.Delete(tempPath);
+            }
+            catch (IOException) {
+                // Best effort — the stray temp file is harmless.
+            }
+
+            throw;
+        }
+    }
 }

RackPeek.Domain/Persistence/Yaml/YamlResourceCollection.cs

@@ -2,6 +2,7 @@ using System.Collections.ObjectModel;
 using System.Collections.Specialized;
 using System.Diagnostics;
 using RackPeek.Domain.Discovery;
+using RackPeek.Domain.Helpers;
 using RackPeek.Domain.Resources;
 using RackPeek.Domain.Resources.AccessPoints;
 using RackPeek.Domain.Resources.Connections;
@@ -33,6 +34,14 @@ public class ResourceCollection {
     ///     the user's file.
     /// </summary>
     public bool Loaded { get; set; }
+
+    /// <summary>
+    ///     Set when the config exists but could not be understood — damaged, truncated,
+    ///     or not YAML. The process is still allowed to boot (the web UI is how someone
+    ///     fixes the file), but every read and write must fail loudly rather than serve
+    ///     or persist an empty inventory (#337).
+    /// </summary>
+    public ConfigLoadException? LoadFailure { get; set; }
 }
 
 public sealed class YamlResourceCollection(
@@ -45,16 +54,19 @@ public sealed class YamlResourceCollection(
     private static readonly int _currentSchemaVersion = RackPeekConfigMigrationDeserializer.ListOfMigrations.Count;
 
     public Task<bool> Exists(string name) {
+        ThrowIfLoadFailed();
         return Task.FromResult(resourceCollection.Resources.Exists(r =>
             r.Name.Equals(name, StringComparison.OrdinalIgnoreCase)));
     }
 
     public Task<string?> GetKind(string? name) {
+        ThrowIfLoadFailed();
         return Task.FromResult(resourceCollection.Resources.FirstOrDefault(r =>
             r.Name.Equals(name, StringComparison.OrdinalIgnoreCase))?.Kind);
     }
 
     public Task<IReadOnlyList<(Resource, string)>> GetByLabelAsync(string name) {
+        ThrowIfLoadFailed();
         ReadOnlyCollection<(Resource r, string)> result = resourceCollection.Resources
             .Where(r => r.Labels != null && r.Labels.TryGetValue(name, out _))
             .Select(r => (r, r.Labels![name]))
@@ -65,6 +77,7 @@ public sealed class YamlResourceCollection(
     }
 
     public Task<Dictionary<string, int>> GetLabelsAsync() {
+        ThrowIfLoadFailed();
         var result = resourceCollection.Resources
             .SelectMany(r => r.Labels ?? Enumerable.Empty<KeyValuePair<string, string>>())
             .Where(kvp => !string.IsNullOrWhiteSpace(kvp.Key))
@@ -75,6 +88,7 @@ public sealed class YamlResourceCollection(
     }
 
     public Task<IReadOnlyList<(Resource, string)>> GetResourceIpsAsync() {
+        ThrowIfLoadFailed();
         var result = new List<(Resource, string)>();
 
         List<Resource> allResources = resourceCollection.Resources;
@@ -108,6 +122,7 @@ public sealed class YamlResourceCollection(
     }
 
     public Task<Dictionary<string, int>> GetTagsAsync() {
+        ThrowIfLoadFailed();
         var result = resourceCollection.Resources
             .SelectMany(r => r.Tags) // flatten all tag arrays
             .Where(t => !string.IsNullOrWhiteSpace(t))
@@ -117,10 +132,13 @@ public sealed class YamlResourceCollection(
         return Task.FromResult(result);
     }
 
-    public Task<IReadOnlyList<T>> GetAllOfTypeAsync<T>() =>
-        Task.FromResult<IReadOnlyList<T>>(resourceCollection.Resources.OfType<T>().ToList());
+    public Task<IReadOnlyList<T>> GetAllOfTypeAsync<T>() {
+        ThrowIfLoadFailed();
+        return Task.FromResult<IReadOnlyList<T>>(resourceCollection.Resources.OfType<T>().ToList());
+    }
 
     public Task<IReadOnlyList<Resource>> GetDependantsAsync(string name) {
+        ThrowIfLoadFailed();
         var result = resourceCollection.Resources
             .Where(r => r.RunsOn.Any(p => p.Equals(name, StringComparison.OrdinalIgnoreCase)))
             .ToList();
@@ -177,6 +195,7 @@ public sealed class YamlResourceCollection(
     }
 
     public Task<IReadOnlyList<Resource>> GetByTagAsync(string name) {
+        ThrowIfLoadFailed();
         return Task.FromResult<IReadOnlyList<Resource>>(
             resourceCollection.Resources
                 .Where(r => r.Tags.Contains(name))
@@ -184,27 +203,42 @@ public sealed class YamlResourceCollection(
         );
     }
 
-    public IReadOnlyList<Hardware> HardwareResources =>
-        resourceCollection.Resources.OfType<Hardware>().ToList();
+    public IReadOnlyList<Hardware> HardwareResources {
+        get {
+            ThrowIfLoadFailed();
+            return resourceCollection.Resources.OfType<Hardware>().ToList();
+        }
+    }
 
-    public IReadOnlyList<SystemResource> SystemResources =>
-        resourceCollection.Resources.OfType<SystemResource>().ToList();
+    public IReadOnlyList<SystemResource> SystemResources {
+        get {
+            ThrowIfLoadFailed();
+            return resourceCollection.Resources.OfType<SystemResource>().ToList();
+        }
+    }
 
-    public IReadOnlyList<Service> ServiceResources =>
-        resourceCollection.Resources.OfType<Service>().ToList();
+    public IReadOnlyList<Service> ServiceResources {
+        get {
+            ThrowIfLoadFailed();
+            return resourceCollection.Resources.OfType<Service>().ToList();
+        }
+    }
 
     public Task<Resource?> GetByNameAsync(string name) {
+        ThrowIfLoadFailed();
         return Task.FromResult(resourceCollection.Resources.FirstOrDefault(r =>
             r.Name.Equals(name, StringComparison.OrdinalIgnoreCase)));
     }
 
     public Task<T?> GetByNameAsync<T>(string name) where T : Resource {
+        ThrowIfLoadFailed();
         Resource? resource =
             resourceCollection.Resources.FirstOrDefault(r => r.Name.Equals(name, StringComparison.OrdinalIgnoreCase));
         return Task.FromResult(resource as T);
     }
 
     public Resource? GetByName(string name) {
+        ThrowIfLoadFailed();
         return resourceCollection.Resources.FirstOrDefault(r =>
             r.Name.Equals(name, StringComparison.OrdinalIgnoreCase));
     }
@@ -227,11 +261,42 @@ public sealed class YamlResourceCollection(
     private async Task LoadUnderLockAsync() {
         var yaml = await fileStore.ReadAllTextAsync(filePath);
 
-        YamlRoot root = await migrationService.DeserializeAsync(
-            yaml,
-            async originalYaml => await BackupOriginalAsync(originalYaml),
-            async migratedRoot => await SaveRootAsync(migratedRoot)
-        );
+        YamlRoot root;
+
+        try {
+            root = await migrationService.DeserializeAsync(
+                yaml,
+                async originalYaml => await BackupOriginalAsync(originalYaml),
+                async migratedRoot => await SaveRootAsync(migratedRoot)
+            );
+        }
+        catch (Exception ex) when (ex is not ConfigLoadException
+                                       and not IOException
+                                       and not UnauthorizedAccessException) {
+            // A file that exists but cannot be understood. Record it so that every
+            // later read and write refuses, rather than quietly serving — and then
+            // persisting — an empty inventory over a recoverable file (#337).
+            resourceCollection.LoadFailure = new ConfigLoadException(
+                $"The config at {filePath} could not be read: {ex.Message} " +
+                "Fix or restore the file (recent schema migrations leave .bak copies " +
+                "beside it); nothing has been changed.",
+                ex);
+
+            throw resourceCollection.LoadFailure;
+        }
+
+        // A RackPeek document always carries a schema version. Its absence means the
+        // file was cut before the version line was written, or is not a RackPeek
+        // config at all — either way the parse "succeeding" with an empty document is
+        // not evidence of an empty inventory.
+        if (!string.IsNullOrWhiteSpace(yaml) && root.Version <= 0) {
+            resourceCollection.LoadFailure = new ConfigLoadException(
+                $"The config at {filePath} is missing its schema version, so it is " +
+                "incomplete or not a RackPeek config. Fix or restore the file; " +
+                "nothing has been changed.");
+
+            throw resourceCollection.LoadFailure;
+        }
 
         resourceCollection.Resources.Clear();
 
@@ -243,9 +308,21 @@ public sealed class YamlResourceCollection(
         if (root.Connections != null)
             resourceCollection.Connections.AddRange(root.Connections);
 
+        resourceCollection.LoadFailure = null;
         resourceCollection.Loaded = true;
     }
 
+    /// <summary>
+    ///     Called at the top of every read path. When the config exists but could not be
+    ///     understood, the in-memory collection is empty for a reason that has nothing to
+    ///     do with the user's inventory — serving it would report "0 resources" for a
+    ///     recoverable file, and scripted consumers would treat that as the truth (#337).
+    /// </summary>
+    private void ThrowIfLoadFailed() {
+        if (resourceCollection.LoadFailure != null)
+            throw resourceCollection.LoadFailure;
+    }
+
     /// <summary>
     ///     Called at the top of every write path, under the lock. Normally a no-op:
     ///     both the CLI and the web host load at startup. When that startup load failed
@@ -302,6 +379,7 @@ public sealed class YamlResourceCollection(
     }
 
     public Task<IReadOnlyList<Connection>> GetConnectionsAsync() {
+        ThrowIfLoadFailed();
         IReadOnlyList<Connection> result =
             resourceCollection.Connections
                 .ToList()
@@ -311,6 +389,7 @@ public sealed class YamlResourceCollection(
     }
 
     public Task<IReadOnlyList<Connection>> GetConnectionsForResourceAsync(string resource) {
+        ThrowIfLoadFailed();
         IReadOnlyList<Connection> result =
             resourceCollection.Connections
                 .Where(c =>
@@ -323,6 +402,7 @@ public sealed class YamlResourceCollection(
     }
 
     public Task<Connection?> GetConnectionForPortAsync(PortReference port) {
+        ThrowIfLoadFailed();
         Connection? connection =
             resourceCollection.Connections
                 .FirstOrDefault(c =>

RackPeek.Mcp/ToolErrors.cs

@@ -21,6 +21,11 @@ internal static class ToolErrors {
         catch (NotFoundException ex) {
             throw new McpException(ex.Message);
         }
+        catch (ConfigLoadException ex) {
+            // The config exists but cannot be read. Say so plainly rather than letting
+            // the agent see a generic failure and conclude the inventory is empty.
+            throw new McpException(ex.Message);
+        }
         catch (ConflictException ex) {
             throw new McpException(ex.Message);
         }

RackPeek.Web.Viewer/App.razor

@@ -1,4 +1,5 @@
-@using RackPeek.Domain.Persistence
+@using RackPeek.Domain.Helpers
+@using RackPeek.Domain.Persistence
 @using RackPeek.Web.Viewer.Pages
 @using Shared.Rcl.Servers
 @inject IResourceCollection Resources
@@ -23,7 +24,17 @@ else
 
     protected override async Task OnInitializedAsync()
     {
-        await Resources.LoadAsync();
+        try
+        {
+            await Resources.LoadAsync();
+        }
+        catch (ConfigLoadException)
+        {
+            // Same contract as the server host: a config that cannot be read must not
+            // leave the app stuck on "Loading…" — the YAML editor is how it is fixed.
+            // Here the store is browser storage, so this is a bad import rather than
+            // an interrupted write (#337).
+        }
 
         _ready = true;
     }

RackPeek.Web/Components/Routes.razor

@@ -1,4 +1,5 @@
-@using RackPeek.Domain.Persistence
+@using RackPeek.Domain.Helpers
+@using RackPeek.Domain.Persistence
 @using RackPeek.Web.Components.Pages
 @using Shared.Rcl.Servers
 @inject IResourceCollection Resources
@@ -23,7 +24,16 @@ else
 
     protected override async Task OnInitializedAsync()
     {
-        await Resources.LoadAsync();
+        try
+        {
+            await Resources.LoadAsync();
+        }
+        catch (ConfigLoadException)
+        {
+            // A damaged config must not leave the app stuck on "Loading…" — the YAML
+            // editor is how someone repairs it. Pages that read the inventory surface
+            // the failure themselves; they can no longer report it as empty (#337).
+        }
 
         _ready = true;
     }

Shared.Rcl/CliBootstrap.cs

@@ -138,6 +138,13 @@ public static class CliBootstrap {
             // matter and is still allowed to fail loudly — the user has one to fix.
             await System.Console.Error.WriteLineAsync($"Warning: could not read {fullYamlPath} ({ex.Message}).");
         }
+        catch (ConfigLoadException) {
+            // A damaged config must not stop the process starting — `rpk discover` and
+            // `--help` do not need the inventory, and the web UI is how someone fixes
+            // the file. The failure is recorded on the collection, so every command
+            // that does touch the inventory fails with it instead of reporting an
+            // empty one (#337).
+        }
         services.AddSingleton<IResourceCollection>(collection);
 
         // Infrastructure
@@ -879,6 +886,10 @@ public static class CliBootstrap {
                 AnsiConsole.MarkupLine($"[red]Not found:[/] {ne.Message}");
                 return 4;
 
+            case ConfigLoadException cle:
+                AnsiConsole.MarkupLine($"[red]Config error:[/] {Markup.Escape(cle.Message)}");
+                return 5;
+
             case CommandParseException pe:
                 if (_showingHelp) return 1; // suppress errors during help lookup
                 AnsiConsole.MarkupLine($"[red]Invalid command:[/] {pe.Message}");

Shared.Rcl/YamlFileComponent.razor

@@ -168,7 +168,18 @@
 
         await FileStore.WriteAllTextAsync(Path, _editText);
 
-        await Resources.LoadAsync();
+        try
+        {
+            await Resources.LoadAsync();
+        }
+        catch (RackPeek.Domain.Helpers.ConfigLoadException ex)
+        {
+            // The edit was saved but the app still cannot read it. Report it here
+            // rather than tearing down the circuit — this editor is the repair tool.
+            _error = new YamlEditError(ex.Message, null, null, null);
+            _currentText = _editText;
+            return;
+        }
 
         _currentText = _editText;
         _isEditing = false;

Shared.Rcl/wwwroot/raw_docs/install-guide.md

@@ -8,6 +8,12 @@ RackPeek can run in two ways:
 RackPeek stores everything in a writable `config/` directory as YAML (including automatic backups).
 Wherever you run it, that directory must be writable.
 
+Saves are atomic: the config is written to a temporary file, flushed to disk, then renamed
+over `config.yaml`. An interrupted save therefore leaves the previous config intact rather
+than a half-written one. If the config ever does become unreadable — a damaged disk, a bad
+hand edit, a sync conflict — RackPeek refuses to read or write it rather than reporting an
+empty inventory, and the Web UI's YAML editor (`/yaml`) still loads so you can repair it.
+
 ---
 
 # Docker (Recommended)

Tests.E2e/DamagedConfigTests.cs

@@ -0,0 +1,113 @@
+using Microsoft.Playwright;
+using Tests.E2e.Infra;
+using Xunit.Abstractions;
+
+namespace Tests.E2e;
+
+/// <summary>
+///     Web half of https://github.com/Timmoth/RackPeek/issues/337. A config that
+///     exists but cannot be read must not leave the app stuck on "Loading…" — the
+///     YAML editor is how someone repairs it — and must never be reported as an
+///     empty inventory. These tests stage a damaged config in the container, then
+///     repair it through the UI.
+/// </summary>
+public class DamagedConfigTests(
+    PlaywrightFixture fixture,
+    ITestOutputHelper output) : E2ETestBase(fixture, output) {
+    private readonly PlaywrightFixture _fixture = fixture;
+    private readonly ITestOutputHelper _output = output;
+
+    // Cut mid-token, exactly as an interrupted in-place write would leave it.
+    private const string _damaged = """
+                                    version: 4
+                                    resources:
+                                    - kind: Server
+                                      name: srv-a
+                                    - ki
+                                    """;
+
+    private const string _healthy = """
+                                    version: 4
+                                    resources:
+                                    - kind: Server
+                                      name: repaired-srv
+                                    connections: []
+                                    """;
+
+    [Fact]
+    public async Task The_App_Still_Loads_And_Can_Repair_A_Damaged_Config() {
+        (IBrowserContext context, IPage page) = await CreatePageAsync();
+
+        try {
+            await _fixture.WriteConfigAsync(_damaged);
+
+            // 1. The app renders rather than hanging on "Loading…".
+            await page.GotoAsync($"{_fixture.BaseUrl}/yaml");
+
+            await Assertions.Expect(page.GetByTestId("circuit-probe"))
+                .ToHaveAttributeAsync("data-circuit-ready", "true");
+
+            // 2. The editor shows the damaged file, so it can be fixed in place.
+            ILocator content = page.GetByTestId("yaml-file-content");
+            await Assertions.Expect(content).ToBeVisibleAsync();
+            await Assertions.Expect(content).ToContainTextAsync("srv-a");
+
+            // 3. Repair it through the editor.
+            await page.GetByRole(AriaRole.Button, new() { Name = "Edit" }).ClickAsync();
+
+            ILocator textarea = page.Locator("textarea");
+            await Assertions.Expect(textarea).ToBeVisibleAsync();
+            await textarea.FillAsync(_healthy);
+
+            await page.GetByRole(AriaRole.Button, new() { Name = "Save" }).ClickAsync();
+
+            await Assertions.Expect(page.GetByTestId("yaml-file-error")).ToHaveCountAsync(0);
+
+            // 4. The inventory reads correctly again.
+            await page.GotoAsync($"{_fixture.BaseUrl}/servers/list");
+            await Assertions.Expect(page.GetByText("repaired-srv").First).ToBeVisibleAsync();
+
+            Assert.Contains("repaired-srv", await _fixture.ReadConfigAsync());
+        }
+        catch (Exception) {
+            _output.WriteLine($"TEST FAILED — URL: {page.Url}");
+            _output.WriteLine(await page.ContentAsync());
+            throw;
+        }
+        finally {
+            // Leave the container usable for any other test in this class.
+            await _fixture.WriteConfigAsync(_healthy);
+            await context.CloseAsync();
+        }
+    }
+
+    [Fact]
+    public async Task A_Damaged_Config_Is_Never_Reported_As_An_Empty_Inventory() {
+        (IBrowserContext context, IPage page) = await CreatePageAsync();
+
+        try {
+            await _fixture.WriteConfigAsync(_damaged);
+
+            await page.GotoAsync($"{_fixture.BaseUrl}/servers/list");
+
+            // The inventory pages read through the collection, which now refuses a
+            // config it could not parse. What must never happen is the page
+            // rendering a confident, empty list over a recoverable file.
+            var body = await page.InnerTextAsync("body");
+            Assert.DoesNotContain("srv-a", body);
+            Assert.DoesNotContain("No servers", body, StringComparison.OrdinalIgnoreCase);
+
+            // And the damaged file is still on disk, untouched by the failed read.
+            Assert.Equal(_damaged, (await _fixture.ReadConfigAsync()).TrimEnd('\n'));
+        }
+        catch (Exception) {
+            _output.WriteLine($"TEST FAILED — URL: {page.Url}");
+            _output.WriteLine(await page.ContentAsync());
+            throw;
+        }
+        finally {
+            await _fixture.WriteConfigAsync(_healthy);
+            await context.CloseAsync();
+        }
+    }
+}

Tests.E2e/Infra/PlaywrightFixture.cs

@@ -1,3 +1,4 @@
+using System.Text;
 using DotNet.Testcontainers.Builders;
 using DotNet.Testcontainers.Containers;
 using Microsoft.Playwright;
@@ -46,6 +47,22 @@ public class PlaywrightFixture : IAsyncLifetime {
         Assertions.SetDefaultExpectTimeout(15000);
     }
 
+    /// <summary>
+    ///     Replaces the container's config.yaml wholesale. Used to stage a damaged
+    ///     config, which cannot be produced through the UI (the editor validates
+    ///     before saving) but is exactly what an interrupted write leaves behind.
+    /// </summary>
+    public async Task WriteConfigAsync(string contents) {
+        await _container.CopyAsync(
+            Encoding.UTF8.GetBytes(contents),
+            "/app/config/config.yaml");
+    }
+
+    public async Task<string> ReadConfigAsync() {
+        var bytes = await _container.ReadFileAsync("/app/config/config.yaml");
+        return Encoding.UTF8.GetString(bytes);
+    }
+
     public async Task DisposeAsync() {
         if (Browser != null)
             await Browser.DisposeAsync();

Tests/EndToEnd/CorruptConfigTests.cs

@@ -3,16 +3,18 @@ using Xunit.Abstractions;
 
 namespace Tests.EndToEnd;
 
-// Reproduces the load-side half of
-// https://github.com/Timmoth/RackPeek/issues/337: saves rewrite config.yaml in
-// place (File.WriteAllTextAsync truncates before writing, and never flushes),
-// so an interrupted save leaves a truncated file behind — and a truncated file
-// is then accepted without complaint on the next load.
+// Load-side half of https://github.com/Timmoth/RackPeek/issues/337.
 //
-// The writer-side half (make PhysicalTextFileStore write atomically and
-// durably: temp file + flush + rename) is not observable from a black-box
-// test; these tests pin the user-facing contract that a damaged file must not
-// be served silently or crash with a raw stack trace.
+// Before the fix a damaged config was accepted without complaint: an unparseable
+// file loaded as an EMPTY inventory with exit 0, and the next write persisted that
+// emptiness over a recoverable file. These tests pin the contract that a config
+// which exists but cannot be understood fails loudly on every read, and — the part
+// that actually loses data — is never overwritten.
+//
+// Note on what is NOT testable here: a save interrupted at a clean resource boundary
+// leaves valid YAML that is indistinguishable from a smaller inventory. Nothing at
+// load time can detect it, which is precisely why the writer-side fix (atomic,
+// durable saves — see PhysicalTextFileStoreTests) is the primary remedy.
 [Collection("Yaml CLI tests")]
 public class CorruptConfigTests(TempYamlCliFixture fs, ITestOutputHelper outputHelper)
     : IClassFixture<TempYamlCliFixture> {
@@ -43,48 +45,84 @@ public class CorruptConfigTests(TempYamlCliFixture fs, ITestOutputHelper outputH
         return output;
     }
 
+    private string ConfigPath => Path.Combine(fs.Root, "config.yaml");
+
     [Fact]
-    public async Task a_config_truncated_at_a_resource_boundary_is_not_served_silently() {
-        // Simulate an interrupted in-place save: the file ends mid-way through
-        // the resources list. This still parses — as a plausible, smaller
-        // inventory missing srv-c and the connections section.
-        var truncated = _fullConfig[.._fullConfig.IndexOf("- kind: Server\n  name: srv-c", StringComparison.Ordinal)];
-        await File.WriteAllTextAsync(Path.Combine(fs.Root, "config.yaml"), truncated);
+    public async Task a_config_cut_mid_token_fails_with_a_friendly_error() {
+        // A save that died mid-write inside a YAML token.
+        var truncated = _fullConfig[.._fullConfig.IndexOf("nd: Server\n  name: srv-c", StringComparison.Ordinal)];
+        await File.WriteAllTextAsync(ConfigPath, truncated);
 
         var output = await ExecuteAsync("summary");
 
-        // Serving a structurally incomplete file (a resources document with no
-        // connections section — something RackPeek's own serializer never
-        // writes) with no diagnostic at all is how a truncation becomes silent
-        // data loss: the next save persists the smaller inventory as if it
-        // were intentional.
-        var hasDiagnostic =
-            output.Contains("error", StringComparison.OrdinalIgnoreCase)
-            || output.Contains("warn", StringComparison.OrdinalIgnoreCase)
-            || output.Contains("corrupt", StringComparison.OrdinalIgnoreCase)
-            || output.Contains("incomplete", StringComparison.OrdinalIgnoreCase)
-            || output.Contains("truncated", StringComparison.OrdinalIgnoreCase);
-
-        Assert.True(hasDiagnostic,
-            $"A truncated config was served with no diagnostic. Output:\n{output}");
+        // An actionable message naming the config file — not a stack dump, and above
+        // all not a cheerful "Hardware (0)".
+        Assert.Contains("config.yaml", output);
+        Assert.DoesNotContain("at RackPeek.", output);
+        Assert.DoesNotContain("YamlDotNet.Core", output);
+        Assert.DoesNotContain("Hardware (0)", output);
     }
 
     [Fact]
-    public async Task a_config_cut_mid_token_fails_with_a_friendly_error() {
-        // Simulate a save that died mid-write inside a YAML token.
-        var truncated = _fullConfig[.._fullConfig.IndexOf("nd: Server\n  name: srv-c", StringComparison.Ordinal)];
-        await File.WriteAllTextAsync(Path.Combine(fs.Root, "config.yaml"), truncated);
+    public async Task a_file_that_is_not_a_rackpeek_config_is_rejected() {
+        // Parses as YAML, carries no schema version: not our document.
+        await File.WriteAllTextAsync(ConfigPath, "hello: world\n");
 
         var output = await ExecuteAsync("summary");
 
-        // Observed on staging: the unparseable file is swallowed entirely and
-        // `rpk summary` reports an EMPTY inventory (Hardware (0)) with exit 0 —
-        // no error, no mention of the file. That is the worst outcome for
-        // #337: a later write would persist the empty inventory over the
-        // damaged-but-recoverable file. The user should instead get an
-        // actionable message naming the config file, and no stack dump.
-        Assert.DoesNotContain("at RackPeek.", output);
-        Assert.DoesNotContain("YamlDotNet.Core", output);
         Assert.Contains("config.yaml", output);
+        Assert.DoesNotContain("Hardware (0)", output);
+    }
+
+    [Fact]
+    public async Task a_damaged_config_is_never_overwritten_by_a_later_write() {
+        // The data-loss path: read the damaged file, then try to write. The write
+        // must refuse rather than persist the empty in-memory collection over it.
+        var truncated = _fullConfig[.._fullConfig.IndexOf("nd: Server\n  name: srv-c", StringComparison.Ordinal)];
+        await File.WriteAllTextAsync(ConfigPath, truncated);
+
+        var output = await ExecuteAsync("servers", "add", "srv-d");
+
+        Assert.DoesNotContain("added", output, StringComparison.OrdinalIgnoreCase);
+
+        var onDisk = await File.ReadAllTextAsync(ConfigPath);
+        Assert.Equal(truncated, onDisk);
+    }
+
+    [Fact]
+    public async Task an_empty_config_is_still_a_valid_empty_inventory() {
+        // The file the CLI itself creates on first run. Must not be mistaken for damage.
+        await File.WriteAllTextAsync(ConfigPath, "");
+
+        var output = await ExecuteAsync("servers", "add", "srv-a");
+
+        Assert.Contains("added", output, StringComparison.OrdinalIgnoreCase);
+        Assert.Contains("name: srv-a", await File.ReadAllTextAsync(ConfigPath));
+    }
+
+    [Fact]
+    public async Task a_legacy_config_without_a_version_key_still_migrates() {
+        // Pre-v1 files carry no version key at all. The migration chain stamps one,
+        // so they must not trip the "missing schema version" guard.
+        await File.WriteAllTextAsync(
+            ConfigPath,
+            "resources:\n  - kind: Server\n    name: legacy-srv\n");
+
+        var output = await ExecuteAsync("summary");
+
+        Assert.Contains("Server: 1", output);
+        Assert.Contains("version: 4", await File.ReadAllTextAsync(ConfigPath));
+    }
+
+    [Fact]
+    public async Task a_healthy_config_still_loads_and_writes() {
+        await File.WriteAllTextAsync(ConfigPath, _fullConfig);
+
+        var output = await ExecuteAsync("summary");
+        Assert.Contains("Server: 3", output);
+
+        output = await ExecuteAsync("servers", "add", "srv-d");
+        Assert.Contains("added", output, StringComparison.OrdinalIgnoreCase);
+        Assert.Contains("name: srv-d", await File.ReadAllTextAsync(ConfigPath));
     }
 }

Tests/Yaml/PhysicalTextFileStoreTests.cs

@@ -0,0 +1,131 @@
+using RackPeek.Domain.Persistence.Yaml;
+
+namespace Tests.Yaml;
+
+/// <summary>
+///     Writer-side half of https://github.com/Timmoth/RackPeek/issues/337.
+///     The store used to be a bare File.WriteAllTextAsync, which truncates the
+///     destination before writing a byte and never flushes — so an interrupted save
+///     could leave a truncated config, and a save that had returned successfully
+///     could still be lost to power failure. It now writes to a temp file, flushes
+///     to disk, and renames over the destination.
+/// </summary>
+public class PhysicalTextFileStoreTests : IDisposable {
+    private readonly string _dir = Path.Combine(
+        Path.GetTempPath(),
+        "rackpeek-store-tests",
+        Guid.NewGuid().ToString("N"));
+
+    private readonly PhysicalTextFileStore _store = new();
+
+    public PhysicalTextFileStoreTests() => Directory.CreateDirectory(_dir);
+
+    public void Dispose() {
+        if (Directory.Exists(_dir))
+            Directory.Delete(_dir, true);
+
+        GC.SuppressFinalize(this);
+    }
+
+    private string Path_(string name) => Path.Combine(_dir, name);
+
+    [Fact]
+    public async Task writing_a_new_file_round_trips_the_content() {
+        var path = Path_("config.yaml");
+
+        await _store.WriteAllTextAsync(path, "version: 4\n");
+
+        Assert.Equal("version: 4\n", await _store.ReadAllTextAsync(path));
+    }
+
+    [Fact]
+    public async Task overwriting_replaces_the_whole_file() {
+        var path = Path_("config.yaml");
+
+        await _store.WriteAllTextAsync(path, new string('a', 4096));
+        await _store.WriteAllTextAsync(path, "short");
+
+        // A rename replaces the file wholesale; a partial in-place write would
+        // leave the tail of the longer content behind.
+        Assert.Equal("short", await _store.ReadAllTextAsync(path));
+    }
+
+    [Fact]
+    public async Task writing_leaves_no_temp_files_behind() {
+        var path = Path_("config.yaml");
+
+        for (var i = 0; i < 5; i++)
+            await _store.WriteAllTextAsync(path, $"version: 4 # {i}\n");
+
+        Assert.Equal(new[] { "config.yaml" },
+            Directory.GetFiles(_dir).Select(System.IO.Path.GetFileName).OrderBy(n => n).ToArray());
+    }
+
+    [Fact]
+    public async Task a_reader_never_observes_a_truncated_file_during_writes() {
+        var path = Path_("config.yaml");
+
+        // Two sizes, neither a prefix of the other: any partially written state is
+        // detectable as "not equal to either of the two valid contents".
+        var big = "version: 4\n" + new string('b', 200_000);
+        var small = "version: 4\n" + new string('s', 50_000);
+
+        await _store.WriteAllTextAsync(path, big);
+
+        using var cts = new CancellationTokenSource();
+
+        var writer = Task.Run(async () => {
+            for (var i = 0; i < 40; i++)
+                await _store.WriteAllTextAsync(path, i % 2 == 0 ? small : big);
+
+            await cts.CancelAsync();
+        });
+
+        var observations = 0;
+
+        while (!cts.IsCancellationRequested) {
+            string seen;
+
+            try {
+                seen = await File.ReadAllTextAsync(path);
+            }
+            catch (IOException) {
+                // The rename can momentarily deny sharing on Windows; not a torn read.
+                continue;
+            }
+
+            observations++;
+
+            Assert.True(seen == big || seen == small,
+                $"Observed a partially written config ({seen.Length} bytes; expected {big.Length} or {small.Length}).");
+        }
+
+        await writer;
+
+        Assert.True(observations > 0, "The reader never managed to sample the file.");
+    }
+
+    [Fact]
+    public async Task a_failed_write_leaves_the_original_intact() {
+        // A directory standing where the temp file wants to be makes the write fail
+        // after the destination would have been truncated by the old implementation.
+        var path = Path_("config.yaml");
+        await _store.WriteAllTextAsync(path, "version: 4\nresources: []\n");
+
+        var readOnlyDir = Path_("locked");
+        Directory.CreateDirectory(readOnlyDir);
+        var nested = Path.Combine(readOnlyDir, "config.yaml");
+        await _store.WriteAllTextAsync(nested, "version: 4\n");
+
+        // Writing to a path that is itself a directory always fails.
+        var directoryPath = Path_("a-directory");
+        Directory.CreateDirectory(directoryPath);
+
+        await Assert.ThrowsAnyAsync<Exception>(
+            () => _store.WriteAllTextAsync(directoryPath, "anything"));
+
+        // The unrelated config is untouched, and no temp debris was left anywhere.
+        Assert.Equal("version: 4\nresources: []\n", await _store.ReadAllTextAsync(path));
+        Assert.DoesNotContain(Directory.GetFiles(_dir), f => f.Contains(".tmp-", StringComparison.Ordinal));
+    }
+}