fix(settings): never set aside a file another program is writing

Reading while denying writers makes a file that is being rewritten in
place read as busy instead of as invalid content, and the read handle is
held until the swap, so the content judged is the content replaced. The
swap uses a replace operation: an invalid original moves to its kept-aside
name in the same step, a failed save leaves it in place, and a reader that
allows replacement no longer blocks a save.

Access denied is permanent and reported at once; only a sharing violation
is waited out, so a read-only profile no longer stalls a window. One
invalid value costs only that value, a trailing comma, a comment and a
quoted number are accepted, and a saved theme the JSON reader cannot
decode is discarded instead of stopping startup. Kept-aside files use the
first free of nine names. Only this store's own temporary files are
removed. ADR-0006 records the on-disk protocol.

Tests cover each of these with real handles held the way other programs
hold them, a deterministic interleaving of two instances, the on-disk
format against the previous store, and bounded retries in the concurrent
test.

Refs #46

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

docs/adr/0006-settings-file-protocol.md

@@ -0,0 +1,95 @@
+# ADR-0006: Settings file protocol
+
+- **Status:** Accepted
+- **Date:** 2026-11-10
+
+## Context
+
+One `settings.json` per edition is shared by every reader window and by every
+process that uses the same data folder: an installed copy and a portable copy, or
+two builds started from different folders, are separate processes over one file.
+
+AppKit's `SettingsStore` returns defaults whenever the file cannot be read and writes
+a whole snapshot through one fixed temporary name with no lock. With real files and
+real processes this lost data in two ways: a load during a sharing violation returned
+defaults that the next save wrote back, and two processes saving in turn erased each
+other's changes.
+
+The files beside `settings.json` are now a contract between application versions that
+may run at the same time, so the protocol is recorded here.
+
+## Decision
+
+`AppSettingsStore` in the core library owns the file. AppKit still defines the shell
+preferences (`ShellSettings`); only persistence moved.
+
+**Change, not snapshot.** `Update` takes a function from the stored settings to the
+new settings. It runs under the lock against what is on disk at that moment, so a
+caller cannot write back a stale copy of a value it did not change.
+
+**Lock.** `settings.json.lock` is opened with no sharing for the duration of an
+update. It is never deleted: deleting a lock file leaves a window in which a second
+opener is refused with access denied. The operating system releases the handle if
+the process dies. A lock that stays held for every attempt refuses the update.
+
+**Read.** The file is opened for reading while denying writers. A program that is
+rewriting the file in place therefore reads as busy, not as invalid content, and
+during an update no such program can start until the swap has happened. Readers
+allow the file to be replaced, so a read never blocks a save.
+
+**What a read can find.**
+
+| Found | Used for display | On update |
+| --- | --- | --- |
+| No file | Defaults | The file is created |
+| Valid settings | The stored values | Replaced |
+| Some invalid values | Valid values kept, invalid ones as defaults | Replaced; the original is kept aside |
+| Not settings at all | Defaults | Replaced; the original is kept aside |
+| Unreadable or being written | Defaults | Refused; nothing is written |
+
+A trailing comma, a comment and a quoted number are valid: they are what a hand edit
+commonly leaves behind. A file over 16 MiB is not loaded.
+
+**Write.** The new content is written to `settings.json.<32 hex digits>.tmp`,
+flushed to disk, and swapped in with a replace operation, which also moves an invalid
+original to its kept-aside name in the same step. If the file did not exist when it
+was read, the temporary file is renamed into place without overwriting. A failed
+update leaves the stored file as it was and removes its temporary file.
+
+**Kept-aside files.** `settings.json.corrupt`, then `settings.json.corrupt-2` to
+`settings.json.corrupt-9`; the first free name is used, and the last is reused once
+all exist. Nothing reads them back; they exist so a hand edit gone wrong can be
+recovered.
+
+**Abandoned temporary files.** A process killed between its write and its swap
+leaves a temporary file. The next update, holding the lock, removes files that match
+this store's own name pattern and no others.
+
+**Waiting.** Only a sharing violation is waited out: at most 20 attempts, 50 ms
+apart, for the lock, the read and the swap. Access denied is treated as permanent and
+reported at once, so a read-only profile does not stall a window.
+
+**Reporting.** Faults are reported by kind with the exception type and HRESULT. No
+path and no operating-system message text reaches the log.
+
+## Consequences
+
+- An update is a synchronous call on the window thread. It normally takes a few
+  milliseconds and at most about one second per contended step.
+- The JSON written is byte-compatible with earlier releases, so a downgrade reads it.
+- An earlier release running against the same folder neither takes the lock nor
+  denies writers. While it is mid-save, this store reads the file as busy and refuses
+  the update; while this store holds the file, the earlier release's save fails and
+  it reports that the setting could not be saved. Neither loses stored data, but the
+  two can refuse each other until both are the same version.
+- The store keeps no in-memory copy. Windows hold their own view of the settings and
+  merge only the fields they changed (ADR-0004).
+
+## Options considered
+
+- **A named mutex instead of a lock file.** Rejected: a session-local name does not
+  cover two sessions of one user on the same profile, and a global name can be
+  created first by another account on the machine.
+- **Fixing AppKit's store instead.** Still worthwhile for other applications, but the
+  read-modify-write API is a different shape, and this application needed the fix
+  without waiting for a package release.

docs/security/threat-model.md

@@ -34,7 +34,7 @@ physical directory. It does not authorize native commands or remote resource acc
 | Remote resources | Denied by CSP and resource interception |
 | External links | HTTP(S) only, actual browser user gesture, external Windows handler; no document navigation outside current page |
 | Heading navigation | Labels extracted after sanitation; authored identity attributes stripped; generated numeric targets only. Fixed host operations reveal/jump/collapse on the current intercepted page; authored scripts, messages and host objects remain disabled |
-| Settings / logs | AppKit sanitation; per-user data root; log operational events without document content or paths. Settings changes are read-modify-write under a lock file beside `settings.json` and replace it by rename; an unreadable file is never overwritten, invalid content is kept as `settings.json.corrupt`, a file over 16 MiB is not loaded, and faults are logged by kind, exception type and HRESULT only |
+| Settings / logs | AppKit sanitation; per-user data root; log operational events without document content or paths. Settings changes are read-modify-write under a lock file beside `settings.json` and are swapped in atomically; a file that is unreadable or being written by another program is never overwritten, invalid content is kept aside under a bounded set of names, a file over 16 MiB is not loaded, and faults are logged by kind, exception type and HRESULT only (ADR-0006) |
 | Updates | Repository HTTPS feed, Velopack integrity checks and production signing preflight |
 | Repository / CI | Protected main and release tags; producer-bound checks; no untrusted PR checkout with privileged review token |
 

docs/user-guide.md

@@ -73,7 +73,8 @@ Documents are rendered locally. Authored scripts and remote images are blocked;
 - **Open in editor is disabled:** hover for the reason, then configure an installed editor in Settings.
 - **The viewer is not a default option:** use Setup rather than a portable copy, then reopen Windows Default Apps.
 - **The app does not start:** confirm Microsoft Edge WebView2 Runtime is installed and inspect the activity logs if available.
-- **A setting could not be saved:** another program is holding `settings.json`, or the folder is read-only. Nothing is overwritten while the file cannot be read; try the change again. If the file was edited by hand and is no longer valid, the app starts with defaults and keeps the invalid file as `settings.json.corrupt` on the next save.
+- **A setting could not be saved:** another program is using `settings.json`, or the file or folder is read-only. Nothing is overwritten while the file cannot be read; try the change again.
+- **Settings were edited by hand:** a trailing comma, a comment or a quoted number is accepted. A value that is not valid is ignored and the rest are kept. The next save keeps the file as it was beside the new one, as `settings.json.corrupt` (then `.corrupt-2` up to `.corrupt-9`).
 
 For help, use [Discussions](https://github.com/KofTwentyTwo/MarkdownViewr.Windows/discussions). Report reproducible problems through [Issues](https://github.com/KofTwentyTwo/MarkdownViewr.Windows/issues/new/choose), including **Help → About** version, Windows version, and relevant sanitized logs. Report vulnerabilities privately using [SECURITY.md](../SECURITY.md).
 

src/MarkdownViewr.Windows.Core/AppSettings.cs

@@ -202,8 +202,9 @@ public sealed class AppSettings : ShellSettings
          _ = Themes.ReaderTheme.ParseCompatible(json);
          return true;
       }
-      catch(JsonException)
+      catch(Exception exception) when(exception is JsonException or InvalidOperationException)
       {
+         // A string the JSON reader cannot decode, such as an unpaired surrogate escape, is invalid content too.
          return false;
       }
    }

src/MarkdownViewr.Windows.Core/AppSettingsContext.cs

@@ -3,13 +3,17 @@
  * SPDX-License-Identifier: MIT
  */
 
+using System.Text.Json;
 using System.Text.Json.Serialization;
 
 
 namespace MarkdownViewr.Windows.Core;
 
-/// <summary>Source-generated JSON serializer for <see cref="AppSettings"/>, so trimmed builds need no reflection.</summary>
-[JsonSourceGenerationOptions(WriteIndented = true)]
+/// <summary>
+/// Source-generated JSON serializer for <see cref="AppSettings"/>, so trimmed builds need no reflection.
+/// Reading tolerates what a hand edit commonly leaves behind: a trailing comma, a comment, a quoted number.
+/// </summary>
+[JsonSourceGenerationOptions(WriteIndented = true, AllowTrailingCommas = true, ReadCommentHandling = JsonCommentHandling.Skip, NumberHandling = JsonNumberHandling.AllowReadingFromString)]
 [JsonSerializable(typeof(AppSettings))]
 public sealed partial class AppSettingsContext : JsonSerializerContext
 {

src/MarkdownViewr.Windows.Core/AppSettingsStore.cs

@@ -5,25 +5,36 @@
 
 using System.Text;
 using System.Text.Json;
+using System.Text.Json.Nodes;
 
 
 namespace MarkdownViewr.Windows.Core;
 
 /// <summary>
 /// The settings file shared by every window and every process of one edition. A change is
-/// applied to the latest stored settings under a cross-process lock and replaces the file
-/// atomically, so two instances cannot erase each other's change. A file that cannot be
-/// read is never treated as empty: only invalid content is replaced, and it is kept aside.
+/// applied to the latest stored settings under a cross-process lock and swapped in atomically,
+/// so two instances cannot erase each other's change. A file that cannot be read, or that
+/// another program is writing, is never treated as empty; content that is invalid while
+/// nobody is writing it is repaired where possible and the original is kept aside.
+/// See ADR-0006 for the on-disk protocol.
 /// </summary>
 public sealed class AppSettingsStore
 {
-   /// <summary>Attempts for the lock, a read, and the final replace before the operation gives up.</summary>
+   /// <summary>Attempts for the lock, a read, and the final swap before the operation gives up.</summary>
    public const int MaximumAttempts = 20;
 
    /// <summary>A larger file is not this application's settings and is never loaded into memory.</summary>
    public const int MaximumBytes = 16 * 1024 * 1024;
 
-   private static readonly TimeSpan s_retryDelay = TimeSpan.FromMilliseconds(50);
+   /// <summary>Invalid files kept aside before the newest slot is reused.</summary>
+   public const int MaximumKeptAside = 9;
+
+   /// <summary>The pause between attempts while another handle is in the way.</summary>
+   public static readonly TimeSpan RetryDelay = TimeSpan.FromMilliseconds(50);
+
+   private const int SharingViolation = unchecked((int)0x80070020);
+   private const int LockViolation = unchecked((int)0x80070021);
+   private static readonly JsonDocumentOptions s_lenient = new() { AllowTrailingCommas = true, CommentHandling = JsonCommentHandling.Skip };
    private readonly Action<SettingsFault, Exception?> _report;
    private readonly Action<TimeSpan> _wait;
 
@@ -40,7 +51,7 @@ public sealed class AppSettingsStore
 
 
 
-   /// <summary>The settings file.</summary>
+   /// <summary>The settings file, as an absolute path.</summary>
    public string Path { get; }
 
 
@@ -48,7 +59,8 @@ public sealed class AppSettingsStore
    /// <summary>Reads the stored settings for display. A missing, invalid, or unreadable file yields defaults and is left untouched.</summary>
    public AppSettings Load()
    {
-      _ = TryRead(out AppSettings settings);
+      _ = TryRead(out AppSettings settings, out FileStream? stored);
+      stored?.Dispose();
       return settings;
    }
 
@@ -57,7 +69,8 @@ public sealed class AppSettingsStore
    /// <summary>
    /// Applies one change to the latest stored settings and saves the result. Returns the saved
    /// settings, or null when nothing was written: the lock stayed busy, the file could not be
-   /// read, or the write failed. The previous file is intact in every null case.
+   /// read or was being written by another program, or the write failed. The stored file is
+   /// unchanged in every null case.
    /// </summary>
    public AppSettings? Update(Func<AppSettings, AppSettings> change)
    {
@@ -70,20 +83,20 @@ public sealed class AppSettingsStore
          {
             return null;
          }
-         SettingsRead read = TryRead(out AppSettings current);
-         if(read == SettingsRead.Unreadable)
+         SettingsRead read = TryRead(out AppSettings current, out FileStream? stored);
+         // The read handle denies writers until the swap, so the content judged here is the content replaced.
+         using(stored)
          {
-            return null;
+            if(read == SettingsRead.Unreadable)
+            {
+               return null;
+            }
+            AppSettings edited = change(current);
+            edited.Sanitize();
+            RemoveAbandonedTemporaryFiles();
+            Replace(JsonSerializer.SerializeToUtf8Bytes(edited, AppSettingsContext.Default.AppSettings), stored is not null, read is SettingsRead.Corrupt or SettingsRead.Repaired);
+            return edited;
          }
-         if(read == SettingsRead.Corrupt)
-         {
-            Retry(() => File.Move(Path, Path + ".corrupt", overwrite: true));
-         }
-         AppSettings edited = change(current);
-         edited.Sanitize();
-         RemoveAbandonedTemporaryFiles();
-         Replace(JsonSerializer.SerializeToUtf8Bytes(edited, AppSettingsContext.Default.AppSettings));
-         return edited;
       }
       catch(Exception exception) when(exception is IOException or UnauthorizedAccessException)
       {
@@ -94,7 +107,10 @@ public sealed class AppSettingsStore
 
 
 
-   /// <summary>Holds an exclusive handle beside the settings file; the operating system releases it if the process dies.</summary>
+   /// <summary>
+   /// Holds an exclusive handle beside the settings file; the operating system releases it if the
+   /// process dies. A lock that stays held, or cannot be opened for another I/O reason, is busy.
+   /// </summary>
    private FileStream? Acquire()
    {
       try
@@ -103,7 +119,7 @@ public sealed class AppSettingsStore
          Retry(() => gate = new FileStream(Path + ".lock", FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None));
          return gate;
       }
-      catch(IOException exception) when(IsTransient(exception))
+      catch(IOException exception)
       {
          _report(SettingsFault.Busy, exception);
          return null;
@@ -112,36 +128,27 @@ public sealed class AppSettingsStore
 
 
 
-   /// <summary>Distinguishes a missing file, invalid content, and a file that could not be read; only the last keeps the caller from writing.</summary>
-   private SettingsRead TryRead(out AppSettings settings)
+   /// <summary>
+   /// Opens the file denying writers, so a program caught rewriting it in place reads as busy
+   /// rather than as invalid content, then distinguishes missing, valid, repaired and invalid.
+   /// The open handle is returned so the caller can keep writers out until it has finished.
+   /// </summary>
+   private SettingsRead TryRead(out AppSettings settings, out FileStream? stored)
    {
       settings = new AppSettings();
+      stored = null;
       SettingsRead result;
       try
       {
-         string? json = null;
-         Retry(() => json = ReadText());
-         AppSettings? loaded = json is null ? null : JsonSerializer.Deserialize(json, AppSettingsContext.Default.AppSettings);
-         if(loaded is null)
-         {
-            _report(SettingsFault.Corrupt, null);
-            result = SettingsRead.Corrupt;
-         }
-         else
-         {
-            settings = loaded;
-            result = SettingsRead.Loaded;
-         }
+         FileStream? stream = null;
+         Retry(() => stream = new FileStream(Path, FileMode.Open, FileAccess.Read, FileShare.Read | FileShare.Delete));
+         stored = stream;
+         result = Parse(stream!, ref settings);
       }
       catch(Exception exception) when(exception is FileNotFoundException or DirectoryNotFoundException)
       {
          result = SettingsRead.Missing;
       }
-      catch(JsonException exception)
-      {
-         _report(SettingsFault.Corrupt, exception);
-         result = SettingsRead.Corrupt;
-      }
       catch(Exception exception) when(exception is IOException or UnauthorizedAccessException)
       {
          _report(SettingsFault.Unreadable, exception);
@@ -153,22 +160,75 @@ public sealed class AppSettingsStore
 
 
 
-   /// <summary>Reads without blocking a concurrent replace; an oversized file reads as invalid content instead of exhausting memory.</summary>
-   private string? ReadText()
+   /// <summary>Reads at most <see cref="MaximumBytes"/>; one invalid value costs only that value, never the whole profile.</summary>
+   private SettingsRead Parse(FileStream stream, ref AppSettings settings)
    {
-      using var stream = new FileStream(Path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete);
       if(stream.Length > MaximumBytes)
       {
-         return null;
+         _report(SettingsFault.Corrupt, null);
+         return SettingsRead.Corrupt;
       }
-      using var reader = new StreamReader(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks: true);
-      return reader.ReadToEnd();
+      using var reader = new StreamReader(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks: true, bufferSize: 4096, leaveOpen: true);
+      string json = reader.ReadToEnd();
+      try
+      {
+         if(JsonSerializer.Deserialize(json, AppSettingsContext.Default.AppSettings) is { } loaded)
+         {
+            settings = loaded;
+            return SettingsRead.Loaded;
+         }
+      }
+      catch(JsonException)
+      {
+         if(Salvage(json) is { } repaired)
+         {
+            settings = repaired;
+            _report(SettingsFault.Repaired, null);
+            return SettingsRead.Repaired;
+         }
+      }
+      _report(SettingsFault.Corrupt, null);
+      return SettingsRead.Corrupt;
    }
 
 
 
-   /// <summary>Writes beside the file, flushes to disk, then renames over it; a failure leaves the previous file and no temporary file.</summary>
-   private void Replace(byte[] content)
+   /// <summary>Keeps every stored value that is valid on its own and drops the ones that are not; null when the file is not a JSON object at all.</summary>
+   private static AppSettings? Salvage(string json)
+   {
+      try
+      {
+         if(JsonNode.Parse(json, documentOptions: s_lenient) is not JsonObject stored)
+         {
+            return null;
+         }
+         foreach(string name in stored.Select(property => property.Key).ToArray())
+         {
+            try
+            {
+               _ = new JsonObject { [name] = stored[name]?.DeepClone() }.Deserialize(AppSettingsContext.Default.AppSettings);
+            }
+            catch(JsonException)
+            {
+               stored.Remove(name);
+            }
+         }
+         return stored.Deserialize(AppSettingsContext.Default.AppSettings);
+      }
+      catch(Exception exception) when(exception is JsonException or ArgumentException)
+      {
+         // Not JSON, or an object that repeats a property name.
+         return null;
+      }
+   }
+
+
+
+   /// <summary>
+   /// Writes beside the file and flushes to disk, then swaps it in. A failure leaves the stored
+   /// file and no temporary file. Invalid content being replaced is kept under a free name.
+   /// </summary>
+   private void Replace(byte[] content, bool exists, bool keepAside)
    {
       string temporary = Path + "." + Guid.NewGuid().ToString("N") + ".tmp";
       bool replaced = false;
@@ -179,7 +239,16 @@ public sealed class AppSettingsStore
             stream.Write(content);
             stream.Flush(flushToDisk: true);
          }
-         Retry(() => File.Move(temporary, Path, overwrite: true));
+         if(exists)
+         {
+            string? aside = keepAside ? KeptAsideName() : null;
+            Retry(() => File.Replace(temporary, Path, aside, ignoreMetadataErrors: true));
+         }
+         else
+         {
+            // No overwrite: a file that appeared since the read is not this instance's to replace.
+            File.Move(temporary, Path);
+         }
          replaced = true;
       }
       finally
@@ -193,18 +262,36 @@ public sealed class AppSettingsStore
 
 
 
-   /// <summary>A writer that died before its rename left a temporary file; the lock holder is the only writer, so every one found is abandoned.</summary>
+   /// <summary>The first free name for an invalid file; once every slot is used the last slot is reused.</summary>
+   private string KeptAsideName()
+   {
+      string candidate = Path + ".corrupt";
+      for(int slot = 2; slot <= MaximumKeptAside && File.Exists(candidate); slot++)
+      {
+         candidate = Path + ".corrupt-" + slot.ToString(System.Globalization.CultureInfo.InvariantCulture);
+      }
+      return candidate;
+   }
+
+
+
+   /// <summary>A writer that died before its swap left a temporary file with this store's own name pattern; under the lock every such file is abandoned.</summary>
    private void RemoveAbandonedTemporaryFiles()
    {
-      foreach(string file in Directory.EnumerateFiles(System.IO.Path.GetDirectoryName(Path)!, System.IO.Path.GetFileName(Path) + "*.tmp"))
+      string prefix = System.IO.Path.GetFileName(Path) + ".";
+      foreach(string file in Directory.EnumerateFiles(System.IO.Path.GetDirectoryName(Path)!, prefix + "*.tmp"))
       {
-         TryDelete(file);
+         string name = System.IO.Path.GetFileName(file);
+         if(name.Length == prefix.Length + 36 && Guid.TryParseExact(name.AsSpan(prefix.Length, 32), "N", out _))
+         {
+            TryDelete(file);
+         }
       }
    }
 
 
 
-   /// <summary>Repeats an operation only for a sharing violation or a pending delete, a bounded number of times; the last failure is thrown.</summary>
+   /// <summary>Repeats an operation only while another handle is in the way, a bounded number of times; the last failure is thrown.</summary>
    private void Retry(Action operation)
    {
       for(int attempt = 1; ; attempt++)
@@ -214,24 +301,17 @@ public sealed class AppSettingsStore
             operation();
             return;
          }
-         catch(Exception exception) when(attempt < MaximumAttempts && IsTransient(exception))
+         catch(IOException exception) when(attempt < MaximumAttempts && IsTransient(exception))
          {
-            _wait(s_retryDelay);
+            _wait(RetryDelay);
          }
       }
    }
 
 
 
-   /// <summary>Another handle on the file (an editor, a scanner, a second instance) or a delete that has not completed; a missing file is not transient.</summary>
-   private static bool IsTransient(Exception exception)
-   {
-      if(exception is UnauthorizedAccessException)
-      {
-         return true;
-      }
-      return exception is IOException and not (FileNotFoundException or DirectoryNotFoundException) && (exception.HResult & 0xFFFF) is 32 or 33;
-   }
+   /// <summary>A sharing or byte-range lock violation: another handle on the file that is expected to go away. Access denied is not transient.</summary>
+   private static bool IsTransient(IOException exception) => exception.HResult is SharingViolation or LockViolation;
 
 
 

src/MarkdownViewr.Windows.Core/SettingsFault.cs

@@ -9,15 +9,14 @@ namespace MarkdownViewr.Windows.Core;
 /// <summary>Why the settings file could not be used as stored; reported without paths or operating-system message text.</summary>
 public enum SettingsFault
 {
-   /// <summary>The file exists but does not hold valid settings; defaults are used and the next save keeps the invalid file aside.</summary>
+   /// <summary>The file does not hold settings at all; defaults are used and the next save keeps the invalid file aside.</summary>
    Corrupt,
-
-   /// <summary>The file could not be read after bounded retries; it is left untouched and nothing is written over it.</summary>
+   /// <summary>Some stored values were invalid and read as their defaults; the next save keeps the original file aside.</summary>
+   Repaired,
+   /// <summary>The file could not be read, or another program was writing it; it is left untouched and nothing is written over it.</summary>
    Unreadable,
-
-   /// <summary>Another process kept the settings lock for every attempt; the change was not saved.</summary>
+   /// <summary>Another instance kept the settings lock for every attempt; the change was not saved.</summary>
    Busy,
-
-   /// <summary>The change could not be written; the previous file remains in place.</summary>
+   /// <summary>The change could not be written; the stored file is unchanged.</summary>
    WriteFailed,
 }

src/MarkdownViewr.Windows.Core/SettingsRead.cs

@@ -11,13 +11,12 @@ internal enum SettingsRead
 {
    /// <summary>Valid stored settings.</summary>
    Loaded,
-
    /// <summary>No file yet; defaults apply and the first save creates it.</summary>
    Missing,
-
-   /// <summary>Invalid content; defaults apply and a save keeps the invalid file aside.</summary>
+   /// <summary>Valid values were kept and invalid ones dropped; a save keeps the original file aside.</summary>
+   Repaired,
+   /// <summary>Not settings at all; defaults apply and a save keeps the invalid file aside.</summary>
    Corrupt,
-
    /// <summary>The content is unknown, so it must not be replaced.</summary>
    Unreadable,
 }

tests/MarkdownViewr.Windows.Core.Tests/AppSettingsStoreTests.cs

@@ -4,13 +4,15 @@
  */
 
 using System.Text;
+using System.Text.Json.Nodes;
 
 
 namespace MarkdownViewr.Windows.Core.Tests;
 
 /// <summary>
 /// The settings file is shared by windows and processes: these tests use real files, real
-/// exclusive handles and concurrent store instances, and count attempts instead of time.
+/// handles held the way other programs hold them, and concurrent store instances. They count
+/// attempts instead of measuring time.
 /// </summary>
 public class AppSettingsStoreTests
 {
@@ -54,6 +56,18 @@ public class AppSettingsStoreTests
 
 
 
+   /// <summary>The store addresses one file whatever the working folder becomes later.</summary>
+   [Fact]
+   public void Constructor_RelativePath_IsMadeAbsolute()
+   {
+      var store = new AppSettingsStore(Path.Combine("relative-profile", "settings.json"));
+      Assert.True(Path.IsPathFullyQualified(store.Path));
+      Assert.Equal(Path.GetFullPath(Path.Combine("relative-profile", "settings.json")), store.Path);
+      Assert.Throws<ArgumentException>(() => new AppSettingsStore(" "));
+   }
+
+
+
    /// <summary>A change is applied to what is stored now, not to what this instance read earlier.</summary>
    [Fact]
    public void Update_AnotherInstanceSavedFirst_ChangesTheLatestSettings()
@@ -79,6 +93,39 @@ public class AppSettingsStoreTests
 
 
 
+   /// <summary>Callers that merge return a new object: that object is what is saved and what comes back.</summary>
+   [Fact]
+   public void Update_ChangeReturnsANewInstance_SavesAndReturnsThatInstance()
+   {
+      using var temp = new TempDirectory();
+      Seed(temp);
+      var store = new AppSettingsStore(temp.File("settings.json"));
+      var replacement = new AppSettings { DocumentTheme = "Dracula", OutlineWidth = 9000 };
+      AppSettings? saved = store.Update(_ => replacement);
+      Assert.Same(replacement, saved);
+      Assert.Equal(500, replacement.OutlineWidth);
+      AppSettings loaded = store.Load();
+      Assert.Equal("Dracula", loaded.DocumentTheme);
+      Assert.Equal("System", loaded.Theme);
+      Assert.Empty(loaded.RecentFiles);
+   }
+
+
+
+   /// <summary>Stored values outside their range are repaired on every read, not only on save.</summary>
+   [Fact]
+   public void Load_StoredValuesOutOfRange_AreSanitized()
+   {
+      using var temp = new TempDirectory();
+      File.WriteAllText(temp.File("settings.json"), """{"OutlineDepth":99,"Theme":"Purple","OutlineWidth":-4}""");
+      AppSettings loaded = new AppSettingsStore(temp.File("settings.json")).Load();
+      Assert.Equal(6, loaded.OutlineDepth);
+      Assert.Equal("System", loaded.Theme);
+      Assert.Equal(140, loaded.OutlineWidth);
+   }
+
+
+
    /// <summary>A file saved by hand with a byte order mark, in UTF-8 or UTF-16, is still the user's settings.</summary>
    [Theory]
    [InlineData(false)]
@@ -97,24 +144,78 @@ public class AppSettingsStoreTests
 
 
 
+   /// <summary>Without a byte order mark the file is UTF-8, so text outside ASCII reads as written.</summary>
+   [Fact]
+   public void Load_Utf8WithoutByteOrderMark_ReadsTextOutsideAscii()
+   {
+      using var temp = new TempDirectory();
+      File.WriteAllBytes(temp.File("settings.json"), new UTF8Encoding(encoderShouldEmitUTF8Identifier: false).GetBytes("""{"GlobalCss":"/* café ☕ */"}"""));
+      Assert.Equal("/* café ☕ */", new AppSettingsStore(temp.File("settings.json")).Load().GlobalCss);
+   }
+
+
+
+   /// <summary>What a hand edit commonly leaves behind is still valid settings: a trailing comma, a comment, a quoted number.</summary>
+   [Theory]
+   [InlineData("""{"Theme":"Dark","OutlineDepth":4,}""")]
+   [InlineData("""{ /* my settings */ "Theme":"Dark", "OutlineDepth":4 }""")]
+   [InlineData("""{"Theme":"Dark","OutlineDepth":"4"}""")]
+   public void Load_HandEditedSyntax_ReadsWithoutAFault(string content)
+   {
+      using var temp = new TempDirectory();
+      File.WriteAllText(temp.File("settings.json"), content);
+      var faults = new List<SettingsFault>();
+      AppSettings loaded = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load();
+      Assert.Equal("Dark", loaded.Theme);
+      Assert.Equal(4, loaded.OutlineDepth);
+      Assert.Empty(faults);
+   }
+
+
+
+   /// <summary>The file on disk stays what earlier versions wrote and can read: indented UTF-8 JSON with no byte order mark.</summary>
+   [Fact]
+   public void Update_WrittenFile_IsIndentedUtf8WithoutByteOrderMarkAndReadableByThePreviousStore()
+   {
+      using var temp = new TempDirectory();
+      Seed(temp);
+      byte[] bytes = File.ReadAllBytes(temp.File("settings.json"));
+      Assert.Equal((byte)'{', bytes[0]);
+      Assert.Contains(Environment.NewLine + "  \"Theme\": \"Dark\"", Encoding.UTF8.GetString(bytes), StringComparison.Ordinal);
+      var previous = new KofTwentyTwo.AppKit.Settings.SettingsStore<AppSettings>(temp.File("settings.json"), AppSettingsContext.Default.AppSettings);
+      AppSettings read = previous.Load();
+      Assert.Equal("Nord", read.DocumentTheme);
+      Assert.Equal("body { color: red }", read.GlobalCss);
+      Assert.Equal([KeptPath], read.RecentFiles);
+      read.Theme = "Light";
+      Assert.True(previous.Save(read));
+      AppSettings loaded = new AppSettingsStore(temp.File("settings.json")).Load();
+      Assert.Equal("Light", loaded.Theme);
+      Assert.Equal("Nord", loaded.DocumentTheme);
+   }
+
+
+
    /// <summary>Another handle holding the file for a moment (a scanner, a backup) is waited out, not read as an empty profile.</summary>
    [Fact]
    public void Load_FileLockedBriefly_RetriesAndReadsTheStoredValues()
    {
       using var temp = new TempDirectory();
       Seed(temp);
-      FileStream? exclusive = new(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.None);
-      int waits = 0;
+      using var exclusive = new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.None);
+      var delays = new List<TimeSpan>();
       var faults = new List<SettingsFault>();
-      var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ =>
+      var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), delay =>
       {
-         if(++waits == 3)
+         delays.Add(delay);
+         if(delays.Count == 3)
          {
             exclusive.Dispose();
          }
       });
       AppSettings loaded = store.Load();
-      Assert.Equal(3, waits);
+      Assert.Equal([AppSettingsStore.RetryDelay, AppSettingsStore.RetryDelay, AppSettingsStore.RetryDelay], delays);
+      Assert.Equal(TimeSpan.FromMilliseconds(50), AppSettingsStore.RetryDelay);
       Assert.Equal("Nord", loaded.DocumentTheme);
       Assert.Equal([KeptPath], loaded.RecentFiles);
       Assert.Empty(faults);
@@ -144,8 +245,8 @@ public class AppSettingsStoreTests
 
 
    /// <summary>
-   /// The data-loss regression: a read failure is not an empty profile. Nothing is written over
-   /// a file whose content is unknown, so the stored theme, CSS and history survive.
+   /// A read failure is not an empty profile: nothing is written over a file whose content is
+   /// unknown, so the stored theme, CSS and history survive.
    /// </summary>
    [Fact]
    public void Update_FileLockedThroughout_WritesNothingOverIt()
@@ -176,13 +277,102 @@ public class AppSettingsStoreTests
 
 
 
-   /// <summary>Only invalid content is corruption: reading it yields defaults, reports it, and leaves the file where it is.</summary>
+   /// <summary>
+   /// Access denied is permanent, not a moment to wait out: it is reported at once, defaults are
+   /// shown, and nothing is written. A folder where the file should be answers exactly that way.
+   /// </summary>
+   [Fact]
+   public void LoadAndUpdate_AccessDenied_ReportUnreadableWithoutWaitingOrWriting()
+   {
+      using var temp = new TempDirectory();
+      Directory.CreateDirectory(temp.File("settings.json"));
+      int waits = 0;
+      var faults = new List<(SettingsFault Fault, Exception? Exception)>();
+      var store = new AppSettingsStore(temp.File("settings.json"), (fault, exception) => faults.Add((fault, exception)), _ => waits++);
+      Assert.Equal("Catppuccin Mocha", store.Load().DocumentTheme);
+      bool changed = false;
+      Assert.Null(store.Update(settings =>
+      {
+         changed = true;
+         return settings;
+      }));
+      Assert.False(changed);
+      Assert.Equal(0, waits);
+      Assert.Equal([SettingsFault.Unreadable, SettingsFault.Unreadable], faults.Select(entry => entry.Fault));
+      Assert.All(faults, entry => Assert.IsType<UnauthorizedAccessException>(entry.Exception));
+      Assert.True(Directory.Exists(temp.File("settings.json")));
+      Assert.Equal(["settings.json.lock"], FileNames(temp.Path));
+   }
+
+
+
+   /// <summary>
+   /// Another program rewriting the file in place has truncated it and not finished. That is a
+   /// busy file, not invalid content: it is not set aside, and once the writer finishes the
+   /// change is applied to what the writer wrote.
+   /// </summary>
+   [Fact]
+   public void Update_FileBeingRewrittenInPlace_WaitsForTheWriterAndKeepsItsContent()
+   {
+      using var temp = new TempDirectory();
+      Seed(temp);
+      using var writer = new FileStream(temp.File("settings.json"), FileMode.Create, FileAccess.Write, FileShare.ReadWrite | FileShare.Delete);
+      int waits = 0;
+      var faults = new List<SettingsFault>();
+      var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ =>
+      {
+         if(++waits == 2)
+         {
+            writer.Write("""{"Theme":"Light","DocumentTheme":"Dracula","GlobalCss":"p { margin: 0 }"}"""u8);
+            writer.Dispose();
+         }
+      });
+      AppSettings? saved = store.Update(settings =>
+      {
+         settings.OutlineDepth = 5;
+         return settings;
+      });
+      Assert.NotNull(saved);
+      Assert.Equal(2, waits);
+      Assert.Empty(faults);
+      AppSettings loaded = store.Load();
+      Assert.Equal("Light", loaded.Theme);
+      Assert.Equal("Dracula", loaded.DocumentTheme);
+      Assert.Equal("p { margin: 0 }", loaded.GlobalCss);
+      Assert.Equal(5, loaded.OutlineDepth);
+      Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path));
+   }
+
+
+
+   /// <summary>A writer that never finishes leaves the save refused and the file exactly as the writer has it.</summary>
+   [Fact]
+   public void Update_FileStillBeingRewritten_IsRefusedAndNothingIsSetAside()
+   {
+      using var temp = new TempDirectory();
+      Seed(temp);
+      var faults = new List<SettingsFault>();
+      var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => { });
+      using(new FileStream(temp.File("settings.json"), FileMode.Create, FileAccess.Write, FileShare.ReadWrite | FileShare.Delete))
+      {
+         Assert.Null(store.Update(settings => settings));
+         Assert.Equal("Catppuccin Mocha", store.Load().DocumentTheme);
+      }
+      Assert.Equal([SettingsFault.Unreadable, SettingsFault.Unreadable], faults);
+      Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path));
+      Assert.Equal(0, new FileInfo(temp.File("settings.json")).Length);
+   }
+
+
+
+   /// <summary>Content that is not settings at all yields defaults, is reported, and stays where it is until something is saved.</summary>
    [Theory]
    [InlineData("")]
    [InlineData("{")]
    [InlineData("null")]
    [InlineData("[1]")]
-   [InlineData("{\"OutlineDepth\":\"deep\"}")]
+   [InlineData("\"text\"")]
+   [InlineData("not json")]
    public void Load_InvalidContent_ReturnsDefaultsAndLeavesTheFile(string content)
    {
       using var temp = new TempDirectory();
@@ -197,39 +387,256 @@ public class AppSettingsStoreTests
 
 
 
-   /// <summary>Saving over invalid content keeps that content aside, so a hand edit gone wrong can still be recovered.</summary>
+   /// <summary>An invalid value in a file that also has hand-edited syntax still costs only that value.</summary>
    [Fact]
-   public void Update_InvalidContent_KeepsItAsideAndSavesTheChange()
+   public void Load_InvalidValueBesideHandEditedSyntax_KeepsEveryOtherValue()
+   {
+      using var temp = new TempDirectory();
+      File.WriteAllText(temp.File("settings.json"), """{ /* mine */ "Theme":"Dark", "OutlineDepth":"deep", "DocumentTheme":"Nord", }""");
+      var faults = new List<SettingsFault>();
+      AppSettings loaded = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load();
+      Assert.Equal([SettingsFault.Repaired], faults);
+      Assert.Equal("Dark", loaded.Theme);
+      Assert.Equal("Nord", loaded.DocumentTheme);
+      Assert.Equal(3, loaded.OutlineDepth);
+   }
+
+
+
+   /// <summary>One invalid value costs only that value: every other stored preference is kept.</summary>
+   [Fact]
+   public void Load_OneInvalidValue_KeepsEveryOtherValue()
+   {
+      using var temp = new TempDirectory();
+      File.WriteAllText(temp.File("settings.json"), """{"Theme":"Dark","OutlineDepth":5.5,"DocumentTheme":"Nord","RecentFiles":["C:\\docs\\a.md",5],"GlobalCss":"body { color: red }","FutureSetting":{"a":1}}""");
+      var faults = new List<SettingsFault>();
+      AppSettings loaded = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load();
+      Assert.Equal([SettingsFault.Repaired], faults);
+      Assert.Equal("Dark", loaded.Theme);
+      Assert.Equal("Nord", loaded.DocumentTheme);
+      Assert.Equal("body { color: red }", loaded.GlobalCss);
+      Assert.Equal(3, loaded.OutlineDepth);
+      Assert.Empty(loaded.RecentFiles);
+   }
+
+
+
+   /// <summary>A repeated property beside an invalid value is still read without throwing into startup.</summary>
+   [Fact]
+   public void Load_RepeatedPropertyAndInvalidValue_DoesNotThrow()
+   {
+      using var temp = new TempDirectory();
+      File.WriteAllText(temp.File("settings.json"), """{"Theme":"Dark","Theme":"Light","OutlineDepth":"deep"}""");
+      var faults = new List<SettingsFault>();
+      AppSettings loaded = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load();
+      Assert.Contains(Assert.Single(faults), new[] { SettingsFault.Corrupt, SettingsFault.Repaired });
+      Assert.Equal(3, loaded.OutlineDepth);
+   }
+
+
+
+   /// <summary>A saved theme whose text the JSON reader cannot decode is discarded like any other invalid theme; it cannot stop the application starting.</summary>
+   [Fact]
+   public void Load_SavedThemeWithUnpairedSurrogate_DiscardsThatThemeOnly()
+   {
+      using var temp = new TempDirectory();
+      string valid = Themes.ThemeCatalog.Find("Nord").Json;
+      string undecodable = valid.Replace("\"Nord\"", "\"\\uD800\"", StringComparison.Ordinal);
+      Assert.NotEqual(valid, undecodable, StringComparer.Ordinal);
+      var stored = new JsonObject { ["Theme"] = "Dark", ["UserThemes"] = new JsonArray(undecodable, valid) };
+      File.WriteAllText(temp.File("settings.json"), stored.ToJsonString());
+      AppSettings loaded = new AppSettingsStore(temp.File("settings.json")).Load();
+      Assert.Equal("Dark", loaded.Theme);
+      Assert.Equal([valid], loaded.UserThemes);
+   }
+
+
+
+   /// <summary>Saving over invalid content keeps that content under a free name; an earlier kept file is not overwritten.</summary>
+   [Fact]
+   public void Update_InvalidContent_KeepsItAsideUnderAFreeNameAndSavesTheChange()
    {
       using var temp = new TempDirectory();
       File.WriteAllText(temp.File("settings.json"), "{ \"Theme\": \"Dark\", ");
-      File.WriteAllText(temp.File("settings.json.corrupt"), "an older invalid file");
+      File.WriteAllText(temp.File("settings.json.corrupt"), "an earlier invalid file");
       var store = new AppSettingsStore(temp.File("settings.json"));
       Assert.NotNull(store.Update(settings =>
       {
          settings.DocumentTheme = "Nord";
          return settings;
       }));
-      Assert.Equal("{ \"Theme\": \"Dark\", ", File.ReadAllText(temp.File("settings.json.corrupt")));
+      Assert.Equal("an earlier invalid file", File.ReadAllText(temp.File("settings.json.corrupt")));
+      Assert.Equal("{ \"Theme\": \"Dark\", ", File.ReadAllText(temp.File("settings.json.corrupt-2")));
       Assert.Equal("Nord", store.Load().DocumentTheme);
       Assert.Equal("System", store.Load().Theme);
+      Assert.Equal(["settings.json", "settings.json.corrupt", "settings.json.corrupt-2", "settings.json.lock"], FileNames(temp.Path));
    }
 
 
 
-   /// <summary>A file far larger than any settings is not loaded into memory, even when it would parse; it reads as invalid content.</summary>
+   /// <summary>A repaired file is saved with its valid values and the change, and the original is kept for the value that was dropped.</summary>
    [Fact]
-   public void Load_OversizedFile_ReadsAsInvalidContent()
+   public void Update_RepairedContent_SavesTheValidValuesAndKeepsTheOriginalAside()
    {
       using var temp = new TempDirectory();
-      byte[] valid = Encoding.UTF8.GetBytes("""{"DocumentTheme":"Nord"}""");
-      byte[] padded = new byte[AppSettingsStore.MaximumBytes + 1];
+      const string Original = """{"Theme":"Dark","OutlineDepth":"deep","DocumentTheme":"Nord"}""";
+      File.WriteAllText(temp.File("settings.json"), Original);
+      var store = new AppSettingsStore(temp.File("settings.json"));
+      Assert.NotNull(store.Update(settings =>
+      {
+         settings.GlobalCss = "p { margin: 0 }";
+         return settings;
+      }));
+      Assert.Equal(Original, File.ReadAllText(temp.File("settings.json.corrupt")));
+      var faults = new List<SettingsFault>();
+      AppSettings loaded = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load();
+      Assert.Empty(faults);
+      Assert.Equal("Dark", loaded.Theme);
+      Assert.Equal("Nord", loaded.DocumentTheme);
+      Assert.Equal("p { margin: 0 }", loaded.GlobalCss);
+   }
+
+
+
+   /// <summary>Kept files are bounded: once every slot is used the newest slot is reused and the older ones stay.</summary>
+   [Fact]
+   public void Update_EveryKeptSlotUsed_ReusesTheNewestSlot()
+   {
+      using var temp = new TempDirectory();
+      File.WriteAllText(temp.File("settings.json.corrupt"), "kept 1");
+      for(int slot = 2; slot <= AppSettingsStore.MaximumKeptAside; slot++)
+      {
+         string number = slot.ToString(System.Globalization.CultureInfo.InvariantCulture);
+         File.WriteAllText(temp.File("settings.json.corrupt-" + number), "kept " + number);
+      }
+      File.WriteAllText(temp.File("settings.json"), "newest invalid");
+      Assert.NotNull(new AppSettingsStore(temp.File("settings.json")).Update(settings => settings));
+      Assert.Equal("kept 1", File.ReadAllText(temp.File("settings.json.corrupt")));
+      Assert.Equal("kept 8", File.ReadAllText(temp.File("settings.json.corrupt-8")));
+      Assert.Equal("newest invalid", File.ReadAllText(temp.File("settings.json.corrupt-9")));
+      Assert.False(File.Exists(temp.File("settings.json.corrupt-10")));
+      Assert.Equal(9, AppSettingsStore.MaximumKeptAside);
+   }
+
+
+
+   /// <summary>When the save over invalid content fails, the invalid file is still in place: it is never moved away first.</summary>
+   [Fact]
+   public void Update_InvalidContentAndFailedSave_LeavesTheInvalidFileInPlace()
+   {
+      using var temp = new TempDirectory();
+      File.WriteAllText(temp.File("settings.json"), "not json");
+      var faults = new List<SettingsFault>();
+      var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => { });
+      using(new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.Read))
+      {
+         Assert.Null(store.Update(settings => settings));
+      }
+      Assert.Equal([SettingsFault.Corrupt, SettingsFault.WriteFailed], faults);
+      Assert.Equal("not json", File.ReadAllText(temp.File("settings.json")));
+      Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path));
+   }
+
+
+
+   /// <summary>A change that throws leaves the file, the lock and the folder as they were; the next save works.</summary>
+   [Fact]
+   public void Update_ChangeThrows_LeavesTheFileAndReleasesTheLock()
+   {
+      using var temp = new TempDirectory();
+      File.WriteAllText(temp.File("settings.json"), "not json");
+      var store = new AppSettingsStore(temp.File("settings.json"));
+      Assert.Throws<InvalidOperationException>(() => store.Update(_ => throw new InvalidOperationException("caller bug")));
+      Assert.Equal("not json", File.ReadAllText(temp.File("settings.json")));
+      Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path));
+      Assert.Throws<ArgumentNullException>(() => store.Update(null!));
+      Assert.NotNull(store.Update(settings => settings));
+   }
+
+
+
+   /// <summary>Reading for display holds nothing afterwards: another program can replace or rewrite the file at once.</summary>
+   [Fact]
+   public void Load_AfterReading_ReleasesTheFile()
+   {
+      using var temp = new TempDirectory();
+      Seed(temp);
+      var store = new AppSettingsStore(temp.File("settings.json"));
+      Assert.Equal("Nord", store.Load().DocumentTheme);
+      using(new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.ReadWrite, FileShare.None))
+      {
+         Assert.Equal("Catppuccin Mocha", new AppSettingsStore(temp.File("settings.json"), wait: _ => { }).Load().DocumentTheme);
+      }
+      Assert.Equal("Nord", store.Load().DocumentTheme);
+   }
+
+
+
+   /// <summary>
+   /// Between judging the stored content and swapping in the new file, no other program can
+   /// rewrite it in place: the content that was read is the content that is replaced.
+   /// </summary>
+   [Fact]
+   public void Update_WhileTheChangeRuns_TheStoredFileCannotBeRewrittenInPlace()
+   {
+      using var temp = new TempDirectory();
+      Seed(temp);
+      var store = new AppSettingsStore(temp.File("settings.json"));
+      bool refused = false;
+      Assert.NotNull(store.Update(settings =>
+      {
+         refused = Record.Exception(() => new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Write, FileShare.ReadWrite | FileShare.Delete).Dispose()) is IOException;
+         settings.Theme = "Light";
+         return settings;
+      }));
+      Assert.True(refused);
+      Assert.Equal("Light", store.Load().Theme);
+      Assert.Equal("Nord", store.Load().DocumentTheme);
+   }
+
+
+
+   /// <summary>A file that appears after this instance found none was written by someone else and is not overwritten.</summary>
+   [Fact]
+   public void Update_FileAppearsAfterTheRead_IsNotOverwritten()
+   {
+      using var temp = new TempDirectory();
+      var faults = new List<SettingsFault>();
+      var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault));
+      Assert.Null(store.Update(settings =>
+      {
+         File.WriteAllText(temp.File("settings.json"), """{"DocumentTheme":"Dracula"}""");
+         settings.Theme = "Light";
+         return settings;
+      }));
+      Assert.Equal([SettingsFault.WriteFailed], faults);
+      Assert.Equal("""{"DocumentTheme":"Dracula"}""", File.ReadAllText(temp.File("settings.json")));
+      Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path));
+      AppSettings? saved = store.Update(settings =>
+      {
+         settings.Theme = "Light";
+         return settings;
+      });
+      Assert.NotNull(saved);
+      Assert.Equal("Dracula", saved.DocumentTheme);
+   }
+
+
+
+   /// <summary>A file is read up to the size bound and not loaded past it, even when it would parse.</summary>
+   [Theory]
+   [InlineData(0, true)]
+   [InlineData(1, false)]
+   public void Load_FileAtAndPastTheSizeBound_IsBoundedExactly(int extraBytes, bool loaded)
+   {
+      using var temp = new TempDirectory();
+      byte[] padded = new byte[AppSettingsStore.MaximumBytes + extraBytes];
       Array.Fill(padded, (byte)' ');
-      valid.CopyTo(padded, 0);
+      """{"DocumentTheme":"Nord"}"""u8.CopyTo(padded);
       File.WriteAllBytes(temp.File("settings.json"), padded);
       var faults = new List<SettingsFault>();
-      Assert.Equal("Catppuccin Mocha", new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load().DocumentTheme);
-      Assert.Equal([SettingsFault.Corrupt], faults);
+      Assert.Equal(loaded ? "Nord" : "Catppuccin Mocha", new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault)).Load().DocumentTheme);
+      Assert.Equal(loaded ? [] : [SettingsFault.Corrupt], faults);
    }
 
 
@@ -255,13 +662,51 @@ public class AppSettingsStoreTests
 
 
 
+   /// <summary>
+   /// Mutual exclusion, shown without threads: while one instance is inside its change, a second
+   /// instance's save is refused for every attempt, and the first instance's change is what ends
+   /// up stored.
+   /// </summary>
+   [Fact]
+   public void Update_WhileAnotherInstanceIsInsideItsChange_IsRefusedAndTheFirstChangeIsSaved()
+   {
+      using var temp = new TempDirectory();
+      Seed(temp);
+      int waits = 0;
+      var faults = new List<SettingsFault>();
+      var first = new AppSettingsStore(temp.File("settings.json"));
+      var second = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => waits++);
+      bool secondChanged = false;
+      AppSettings? saved = first.Update(settings =>
+      {
+         Assert.Null(second.Update(other =>
+         {
+            secondChanged = true;
+            other.Theme = "Light";
+            return other;
+         }));
+         settings.DocumentTheme = "Dracula";
+         return settings;
+      });
+      Assert.NotNull(saved);
+      Assert.False(secondChanged);
+      Assert.Equal(AppSettingsStore.MaximumAttempts - 1, waits);
+      Assert.Equal([SettingsFault.Busy], faults);
+      AppSettings loaded = second.Load();
+      Assert.Equal("Dracula", loaded.DocumentTheme);
+      Assert.Equal("Dark", loaded.Theme);
+      Assert.NotNull(second.Update(other => other));
+   }
+
+
+
    /// <summary>A lock released while this instance waits lets the change through.</summary>
    [Fact]
    public void Update_LockReleasedDuringWait_SavesTheChange()
    {
       using var temp = new TempDirectory();
       Seed(temp);
-      FileStream other = new(temp.File("settings.json.lock"), FileMode.Open, FileAccess.ReadWrite, FileShare.None);
+      using var other = new FileStream(temp.File("settings.json.lock"), FileMode.Open, FileAccess.ReadWrite, FileShare.None);
       var store = new AppSettingsStore(temp.File("settings.json"), wait: _ => other.Dispose());
       AppSettings? saved = store.Update(settings =>
       {
@@ -275,9 +720,32 @@ public class AppSettingsStoreTests
 
 
 
-   /// <summary>A handle that forbids replacing the file fails the save cleanly: the previous file stays and no temporary file is left.</summary>
+   /// <summary>A reader that allows the file to be replaced, as this store's own reads do, does not stop a save.</summary>
    [Fact]
-   public void Update_ReplaceBlocked_LeavesPreviousFileAndNoTemporaryFile()
+   public void Update_FileOpenByAReaderThatAllowsReplacement_StillSaves()
+   {
+      using var temp = new TempDirectory();
+      Seed(temp);
+      int waits = 0;
+      var store = new AppSettingsStore(temp.File("settings.json"), wait: _ => waits++);
+      using(new FileStream(temp.File("settings.json"), FileMode.Open, FileAccess.Read, FileShare.Read | FileShare.Delete))
+      {
+         Assert.NotNull(store.Update(settings =>
+         {
+            settings.Theme = "Light";
+            return settings;
+         }));
+      }
+      Assert.Equal(0, waits);
+      Assert.Equal("Light", store.Load().Theme);
+      Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path));
+   }
+
+
+
+   /// <summary>A handle that forbids replacing the file fails the save cleanly: the stored file stays and no temporary file is left.</summary>
+   [Fact]
+   public void Update_ReplaceBlocked_LeavesTheStoredFileAndNoTemporaryFile()
    {
       using var temp = new TempDirectory();
       Seed(temp);
@@ -301,45 +769,106 @@ public class AppSettingsStoreTests
 
 
 
+   /// <summary>A read-only settings file fails the save at once instead of stalling the window for every attempt.</summary>
+   [Fact]
+   public void Update_ReadOnlyFile_FailsWithoutWaiting()
+   {
+      using var temp = new TempDirectory();
+      Seed(temp);
+      byte[] before = File.ReadAllBytes(temp.File("settings.json"));
+      int waits = 0;
+      var faults = new List<(SettingsFault Fault, Exception? Exception)>();
+      var store = new AppSettingsStore(temp.File("settings.json"), (fault, exception) => faults.Add((fault, exception)), _ => waits++);
+      File.SetAttributes(temp.File("settings.json"), FileAttributes.ReadOnly);
+      try
+      {
+         Assert.Null(store.Update(settings =>
+         {
+            settings.Theme = "Light";
+            return settings;
+         }));
+         Assert.Equal("Nord", store.Load().DocumentTheme);
+      }
+      finally
+      {
+         File.SetAttributes(temp.File("settings.json"), FileAttributes.Normal);
+      }
+      Assert.Equal(0, waits);
+      (SettingsFault fault, Exception? exception) = Assert.Single(faults);
+      Assert.Equal(SettingsFault.WriteFailed, fault);
+      Assert.IsType<UnauthorizedAccessException>(exception);
+      Assert.Equal(before, File.ReadAllBytes(temp.File("settings.json")));
+      Assert.Equal(["settings.json", "settings.json.lock"], FileNames(temp.Path));
+   }
+
+
+
+   /// <summary>A lock that can never be taken, here a folder with the lock's name, is a failed write reported at once, not a busy lock.</summary>
+   [Fact]
+   public void Update_LockCannotBeOpened_FailsWithoutWaiting()
+   {
+      using var temp = new TempDirectory();
+      Directory.CreateDirectory(temp.File("settings.json.lock"));
+      int waits = 0;
+      var faults = new List<SettingsFault>();
+      var store = new AppSettingsStore(temp.File("settings.json"), (fault, _) => faults.Add(fault), _ => waits++);
+      Assert.Null(store.Update(settings => settings));
+      Assert.Equal(0, waits);
+      Assert.Equal([SettingsFault.WriteFailed], faults);
+      Assert.Empty(FileNames(temp.Path));
+   }
+
+
+
    /// <summary>A location that cannot hold a settings folder reports a failed write instead of throwing into the caller.</summary>
    [Fact]
    public void Update_FolderCannotBeCreated_ReportsWriteFailed()
    {
       using var temp = new TempDirectory();
       File.WriteAllText(temp.File("blocker"), "a file where the folder should be");
+      int waits = 0;
       var faults = new List<SettingsFault>();
-      var store = new AppSettingsStore(Path.Combine(temp.File("blocker"), "settings.json"), (fault, _) => faults.Add(fault), _ => { });
+      var store = new AppSettingsStore(Path.Combine(temp.File("blocker"), "settings.json"), (fault, _) => faults.Add(fault), _ => waits++);
       Assert.Null(store.Update(settings => settings));
       Assert.Equal([SettingsFault.WriteFailed], faults);
+      Assert.Equal(0, waits);
       Assert.Equal("Catppuccin Mocha", store.Load().DocumentTheme);
    }
 
 
 
-   /// <summary>Temporary files left by a writer that died before its rename are removed by the next save; one still in use is left alone.</summary>
+   /// <summary>
+   /// Only this store's own abandoned temporary files are removed by the next save. One still in
+   /// use, the fixed-name temporary file an older release writes, and anything else ending in
+   /// .tmp are left alone.
+   /// </summary>
    [Fact]
-   public void Update_AbandonedTemporaryFiles_AreRemovedUnlessInUse()
+   public void Update_AbandonedTemporaryFiles_RemovesOnlyItsOwn()
    {
       using var temp = new TempDirectory();
       Seed(temp);
-      File.WriteAllText(temp.File("settings.json.tmp"), "left by the previous store");
-      File.WriteAllText(temp.File("settings.json.0123.tmp"), "left by a writer that died");
-      File.WriteAllText(temp.File("settings.json.busy.tmp"), "still open elsewhere");
-      File.WriteAllText(temp.File("notes.tmp"), "not ours");
+      string own = "settings.json." + Guid.NewGuid().ToString("N") + ".tmp";
+      string inUse = "settings.json." + Guid.NewGuid().ToString("N") + ".tmp";
+      string notHex = "settings.json." + new string('z', 32) + ".tmp";
+      foreach(string name in new[] { own, inUse, notHex, "settings.json.tmp", "settings.json.bak.tmp", "settings.json.0123.tmp", "notes.tmp" })
+      {
+         File.WriteAllText(temp.File(name), "left behind");
+      }
       var store = new AppSettingsStore(temp.File("settings.json"));
-      using(new FileStream(temp.File("settings.json.busy.tmp"), FileMode.Open, FileAccess.Read, FileShare.None))
+      using(new FileStream(temp.File(inUse), FileMode.Open, FileAccess.Read, FileShare.None))
       {
          Assert.NotNull(store.Update(settings => settings));
       }
-      Assert.Equal(["notes.tmp", "settings.json", "settings.json.busy.tmp", "settings.json.lock"], FileNames(temp.Path));
+      string[] expected = [inUse, notHex, "notes.tmp", "settings.json", "settings.json.0123.tmp", "settings.json.bak.tmp", "settings.json.lock", "settings.json.tmp"];
+      Assert.Equal(expected.Order(StringComparer.Ordinal), FileNames(temp.Path), StringComparer.Ordinal);
    }
 
 
 
    /// <summary>
-   /// The lost-update regression: instances that each record their own documents at the same
-   /// time keep every one. A refused save is retried, as a user would; a save that reported
-   /// success must never be erased by another instance.
+   /// Instances that each record their own documents at the same time keep every one. A refused
+   /// save is retried a bounded number of times, as a user would; a save that reported success
+   /// must never be erased by another instance.
    /// </summary>
    [Fact]
    public void Update_ConcurrentInstances_KeepEveryChange()
@@ -348,32 +877,18 @@ public class AppSettingsStoreTests
       const int Instances = 4;
       const int Each = 5;
       using var start = new ManualResetEventSlim();
+      var failures = new System.Collections.Concurrent.ConcurrentQueue<string>();
       var threads = new List<Thread>();
       for(int instance = 0; instance < Instances; instance++)
       {
          int id = instance;
-         var thread = new Thread(() =>
-         {
-            var store = new AppSettingsStore(temp.File("settings.json"));
-            start.Wait();
-            for(int index = 0; index < Each; index++)
-            {
-               string path = $@"C:\docs\instance{id}-{index}.md";
-               while(store.Update(settings =>
-               {
-                  settings.RecentFiles = Documents.RecentDocuments.Record(settings.RecentFiles, path);
-                  return settings;
-               }) is null)
-               {
-                  Thread.Yield();
-               }
-            }
-         });
+         var thread = new Thread(() => RecordDocuments(temp.File("settings.json"), id, Each, start, failures));
          thread.Start();
          threads.Add(thread);
       }
       start.Set();
       threads.ForEach(thread => thread.Join());
+      Assert.Empty(failures);
       string[] recorded = new AppSettingsStore(temp.File("settings.json")).Load().RecentFiles;
       Assert.Equal(Instances * Each, recorded.Length);
       Assert.Equal(Instances * Each, recorded.Distinct(StringComparer.Ordinal).Count());
@@ -382,6 +897,39 @@ public class AppSettingsStoreTests
 
 
 
+   /// <summary>One instance's work: a failure is recorded for the test to assert, never thrown on a bare thread.</summary>
+   private static void RecordDocuments(string file, int id, int count, ManualResetEventSlim start, System.Collections.Concurrent.ConcurrentQueue<string> failures)
+   {
+      try
+      {
+         var store = new AppSettingsStore(file);
+         start.Wait();
+         for(int index = 0; index < count; index++)
+         {
+            string path = $@"C:\docs\instance{id}-{index}.md";
+            int refusals = 0;
+            while(store.Update(settings =>
+            {
+               settings.RecentFiles = Documents.RecentDocuments.Record(settings.RecentFiles, path);
+               return settings;
+            }) is null)
+            {
+               if(++refusals == 50)
+               {
+                  failures.Enqueue(path + " was refused 50 times");
+                  return;
+               }
+            }
+         }
+      }
+      catch(Exception exception) when(exception is IOException or UnauthorizedAccessException or InvalidOperationException)
+      {
+         failures.Enqueue(exception.ToString());
+      }
+   }
+
+
+
    /// <summary>A profile with non-default values in several unrelated fields, so a wipe is visible.</summary>
    private static void Seed(TempDirectory temp)
    {