Merge pull request #343 from Timmoth/bug/337-non-atomic-config-saves

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

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

@@ -0,0 +1,128 @@
+using Tests.EndToEnd.Infra;
+using Xunit.Abstractions;
+
+namespace Tests.EndToEnd;
+
+// Load-side half of https://github.com/Timmoth/RackPeek/issues/337.
+//
+// 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> {
+    // Exactly what the serializer writes for three bare servers.
+    private const string _fullConfig = """
+                                       version: 4
+                                       resources:
+                                       - kind: Server
+                                         name: srv-a
+                                       - kind: Server
+                                         name: srv-b
+                                       - kind: Server
+                                         name: srv-c
+                                       connections: []
+
+                                       """;
+
+    private async Task<string> ExecuteAsync(params string[] args) {
+        outputHelper.WriteLine($"rpk {string.Join(" ", args)}");
+
+        var output = await YamlCliTestHost.RunAsync(
+            args,
+            fs.Root,
+            outputHelper,
+            "config.yaml");
+
+        outputHelper.WriteLine(output);
+        return output;
+    }
+
+    private string ConfigPath => Path.Combine(fs.Root, "config.yaml");
+
+    [Fact]
+    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");
+
+        // 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_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");
+
+        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));
+    }
+}