TODO follow-ups: corrupt-file backup, list JSON, reset confirmation, command-flow tests

CHANGELOG.md

@@ -25,15 +25,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Tolerant JSON persistence** — atomic writes (temp-file + rename), missing
   properties default, unknown properties ignored, case-insensitive matching,
   string-valued enums.
+- **Corrupt-file resilience** — a malformed settings file is copied to a
+  `{file}.bak` sidecar and the class falls back to defaults, rather than
+  crashing startup or letting the next write destroy the unreadable content.
 - **`settings` command branch** — `list` and `reset` (`<SettingsClassName>` and
   `--all`), drop-in via `CommandConfiguratorExtensions.AddSettingsBranch()`. All
-  commands honour `-v` / `--verbose`.
+  commands honour `-v` / `--verbose`. `reset` confirms before overwriting
+  (defaults to "no"; skip with `-f` / `--force`); `list` renders complex and
+  collection values as compact JSON.
 - **`ISettingsStore`** — enumerate registrations, resolve instances, and reset one
   or all classes at runtime.
 - Full XML documentation on the public surface.
-- Test suite (xUnit) with 22 tests covering load-on-missing-file, automatic
+- Test suite (xUnit) with 32 tests covering load-on-missing-file, automatic
   persistence + round-trip, explicit persistence, debounce coalescing, reset /
-  reset-all, tolerant deserialisation, atomic writes, and error surfacing.
+  reset-all, tolerant deserialisation, corrupt-file backup, atomic writes, error
+  surfacing, `settings list` value formatting, and end-to-end command flows
+  (including the `reset` confirmation prompt).
 - SourceLink, deterministic builds, published symbol packages (`snupkg`).
 - `TreatWarningsAsErrors=true`, `AnalysisLevel=latest` — zero-warning public API.
 - Package icon, with the editable source vector kept under `design/icons/`.

README.md

@@ -134,9 +134,12 @@ AppSettings (Automatic)
 └──────────┴──────┘
 
 $ my-cli settings reset AppSettings
+Reset 'AppSettings' to defaults? This overwrites the saved file and cannot be undone. [y/N]: y
 Reset 'AppSettings' to defaults.
 ```
 
+`reset` prompts for confirmation (defaulting to "no") before overwriting. Pass `-f` / `--force` to skip the prompt in scripts or CI.
+
 ---
 
 ## Persistence model
@@ -174,6 +177,28 @@ Supply your own `SettingsOptions.SerializerOptions` if you need different behavi
 
 ---
 
+## Nested values & keeping it simple
+
+Persistence handles whatever `System.Text.Json` can serialise — nested objects, lists, and dictionaries all round-trip to and from the JSON file without extra work. Two things to know once you go beyond scalars:
+
+- **Automatic save only fires on the top-level setter.** `OnPropertyChanged()` runs when you assign a property *on the settings object* — not when you mutate *inside* a nested object or collection:
+
+  ```csharp
+  settings.Profile = new Profile { Name = "Ada" };   // ✅ persisted (the setter ran)
+  settings.Profile.Name = "Ada";                     // ❌ not detected in Automatic mode
+  settings.RecentFiles.Add("notes.txt");             // ❌ not detected
+  ```
+
+  To persist a nested change: reassign the whole property, model nested values as immutable `record`s and swap them (`settings.Profile = settings.Profile with { Name = "Ada" };`), or call `settings.Save()` / `await settings.SaveAsync()` after mutating.
+
+- **`settings list` renders complex values as compact JSON**, so the table stays readable instead of printing a type name.
+
+### Recommendation: prefer scalar properties
+
+Keep your life simple — make settings properties **scalars** (`string`, `bool`, numbers, `enum`, `DateTime`, `Guid`, `Uri`, …) wherever you can. A flat scalar settings class never hits the in-place-mutation gotcha above, reads cleanly in `settings list`, and is trivial to reason about. Reach for a nested object or collection only when you're happy to assign it wholesale.
+
+---
+
 ## Resetting at runtime
 
 `ISettingsStore` is registered as a singleton and aggregates every class you registered. It powers `settings reset`, and you can use it directly:
@@ -221,7 +246,7 @@ Everything else is transitive.
 
 ## Contributing
 
-Issues and PRs welcome. The [TODO](TODO.md) tracks outstanding ideas.
+Issues and PRs welcome.
 
 When contributing code, please keep the zero-warning, fully-documented public surface. `TreatWarningsAsErrors` is on for a reason.
 

src/NextIteration.SpectreConsole.Settings/Commands/ListSettingsCommand.cs

@@ -1,5 +1,7 @@
 using System.Globalization;
 using System.Reflection;
+using System.Text.Json;
+using System.Text.Json.Serialization;
 
 using Spectre.Console;
 using Spectre.Console.Cli;
@@ -16,6 +18,14 @@ public sealed class ListSettingsCommand(ISettingsStore store) : AsyncCommand<Lis
     {
         private readonly ISettingsStore _store = store;
 
+        // Compact (single-line) JSON for rendering complex values in the table;
+        // enums as strings to match how they're written to disk.
+        private static readonly JsonSerializerOptions _displayJsonOptions = new(JsonSerializerDefaults.General)
+        {
+            WriteIndented = false,
+            Converters = { new JsonStringEnumConverter() },
+        };
+
         /// <summary>CLI settings for <c>settings list</c>.</summary>
         public sealed class Settings : SettingsCommandSettings
         {
@@ -81,7 +91,7 @@ private void RenderRegistration(SettingsRegistration registration)
             AnsiConsole.Write(table);
         }
 
-        private static string FormatValue(PropertyInfo property, object instance)
+        internal static string FormatValue(PropertyInfo property, object instance)
         {
             object? value;
             try
@@ -94,13 +104,46 @@ private static string FormatValue(PropertyInfo property, object instance)
                 return $"<error: {ex.InnerException?.Message ?? ex.Message}>";
             }
 
-            return value switch
+            if (value is null)
+            {
+                return string.Empty;
+            }
+
+            var type = value.GetType();
+            if (IsScalar(type))
+            {
+                return value switch
+                {
+                    string s => s,
+                    IFormattable formattable => formattable.ToString(null, CultureInfo.InvariantCulture),
+                    _ => value.ToString() ?? string.Empty,
+                };
+            }
+
+            // Complex or collection value: render as compact JSON so the table
+            // stays informative instead of printing a bare type name.
+            try
+            {
+                return JsonSerializer.Serialize(value, type, _displayJsonOptions);
+            }
+            catch (NotSupportedException ex)
             {
-                null => string.Empty,
-                string s => s,
-                IFormattable formattable => formattable.ToString(null, CultureInfo.InvariantCulture),
-                _ => value.ToString() ?? string.Empty,
-            };
+                return $"<unserialisable: {ex.Message}>";
+            }
+        }
+
+        private static bool IsScalar(Type type)
+        {
+            var t = Nullable.GetUnderlyingType(type) ?? type;
+            return t.IsPrimitive
+                || t.IsEnum
+                || t == typeof(string)
+                || t == typeof(decimal)
+                || t == typeof(DateTime)
+                || t == typeof(DateTimeOffset)
+                || t == typeof(TimeSpan)
+                || t == typeof(Guid)
+                || t == typeof(Uri);
         }
     }
 }

src/NextIteration.SpectreConsole.Settings/Commands/ResetSettingsCommand.cs

@@ -30,6 +30,11 @@ public sealed class Settings : SettingsCommandSettings
             [CommandOption("--all")]
             [Description("Reset all registered settings classes")]
             public bool All { get; set; }
+
+            /// <summary>Skip the confirmation prompt. Useful in scripts.</summary>
+            [CommandOption("-f|--force")]
+            [Description("Reset without confirmation")]
+            public bool Force { get; set; }
         }
 
         /// <inheritdoc />
@@ -51,6 +56,15 @@ protected override async Task<int> ExecuteAsync(CommandContext context, Settings
                         return 0;
                     }
 
+                    if (!await ConfirmAsync(
+                        settings,
+                        $"Reset all {_store.Registrations.Count} settings class(es) to defaults? This overwrites their saved files and cannot be undone.",
+                        cancellationToken).ConfigureAwait(false))
+                    {
+                        AnsiConsole.MarkupLine("[yellow]Reset cancelled.[/]");
+                        return 0;
+                    }
+
                     await _store.ResetAllAsync(cancellationToken).ConfigureAwait(false);
                     AnsiConsole.MarkupLine($"[green]Reset all {_store.Registrations.Count} settings class(es) to defaults.[/]");
                     return 0;
@@ -73,6 +87,15 @@ protected override async Task<int> ExecuteAsync(CommandContext context, Settings
                     return 1;
                 }
 
+                if (!await ConfirmAsync(
+                    settings,
+                    $"Reset '{Markup.Escape(registration.Name)}' to defaults? This overwrites the saved file and cannot be undone.",
+                    cancellationToken).ConfigureAwait(false))
+                {
+                    AnsiConsole.MarkupLine("[yellow]Reset cancelled.[/]");
+                    return 0;
+                }
+
                 await _store.ResetAsync(registration.SettingsType, cancellationToken).ConfigureAwait(false);
                 AnsiConsole.MarkupLine($"[green]Reset '{Markup.Escape(registration.Name)}' to defaults.[/]");
                 return 0;
@@ -84,6 +107,19 @@ protected override async Task<int> ExecuteAsync(CommandContext context, Settings
             }
         }
 
+        // Returns true when the reset should proceed: either --force was passed
+        // or the user confirmed. The prompt defaults to "no" since a reset is
+        // destructive.
+        private static async Task<bool> ConfirmAsync(Settings settings, string message, CancellationToken cancellationToken)
+        {
+            if (settings.Force)
+            {
+                return true;
+            }
+
+            return await AnsiConsole.ConfirmAsync(message, defaultValue: false, cancellationToken).ConfigureAwait(false);
+        }
+
         private void RenderAvailableClasses()
         {
             if (_store.Registrations.Count == 0)

src/NextIteration.SpectreConsole.Settings/Persistence/SettingsStore.cs

@@ -122,15 +122,32 @@ private static SettingsBase Load(SettingsTypeDescriptor descriptor)
             }
             catch (JsonException ex)
             {
-                // A malformed file shouldn't crash CLI startup. Surface it via
-                // the configured handler, then fall back to defaults. (Missing
-                // files are the common case and don't reach here.)
+                // A malformed file shouldn't crash CLI startup. Preserve the
+                // unreadable content as a sidecar before defaults take over —
+                // otherwise the next write silently overwrites it — then
+                // surface the error and fall back to defaults. (Missing files
+                // are the common case and don't reach here.)
+                TryBackupCorruptFile(descriptor.FilePath);
                 descriptor.ErrorHandler(ex);
             }
 
             return descriptor.Factory();
         }
 
+        private static void TryBackupCorruptFile(string filePath)
+        {
+            try
+            {
+                File.Copy(filePath, filePath + ".bak", overwrite: true);
+            }
+            catch
+            {
+                // Best-effort: a failed backup must not prevent falling back to
+                // defaults. The original file is left untouched (the copy, not a
+                // move), so nothing is lost by a backup failure here.
+            }
+        }
+
         private static void ResetInstanceToDefaults(SettingsBase instance, SettingsTypeDescriptor descriptor)
         {
             var defaults = descriptor.Factory();