v3.11.2: apply plans rooted at a drive, refuse source choices that cannot be applied

README.md

@@ -1,6 +1,6 @@
 [![CI](https://github.com/amrali-eg/EncodingChecker/actions/workflows/ci.yml/badge.svg)](https://github.com/amrali-eg/EncodingChecker/actions/workflows/ci.yml)
 
-# EncodingChecker v3.11.1
+# EncodingChecker v3.11.2
 
 EncodingChecker is a Windows tool for finding, checking, and safely converting text-file encodings. Use the GUI for everyday work or the command line for repeatable jobs.
 

docs/RELEASE-NOTES-v3.11.2.md

@@ -0,0 +1,103 @@
+# EncodingChecker v3.11.2
+
+Two silent failures made loud, a measured speedup, and the first backlog this
+project has kept.
+
+## A plan rooted at a drive can be applied
+
+`-BasePath D:\` produced a normal-looking plan that `-Apply` then refused
+entirely, reporting every file as resolving outside the plan's own directory —
+blaming the paths rather than the root. The containment check appended a
+separator to a root that already ended in one, so the prefix became `D:\\` and
+nothing could ever match it.
+
+This shipped broken in **v3.11.0 and v3.11.1**. It was found before v3.11.0,
+recorded, reported as closed, and rediscovered from scratch during an unrelated
+review. That is why this release also adds a backlog.
+
+## A source choice that cannot be applied is refused, not dropped
+
+In the review dialog, each refused row carries the path resolved for it. Rows
+that resolved to nothing were filtered out of the ticked set in silence, so the
+confirm button did nothing and the run reported **"Conversion cancelled. No
+files were modified."** — a cancellation nobody asked for, after the user had
+ticked files and chosen an encoding.
+
+The drive-root defect was one way in. It is not the only one: the results list
+is cleared when a scan *starts* and never when the directory box changes, and
+that box accepts typing, a recent entry, or a dragged folder. Scan one folder,
+point the box at another, and every row in the next review resolves outside the
+plan's root.
+
+The review now says which ticked files it cannot act on and what to do about it,
+and stays open. The *Proceed* path already handled this correctly; only the
+source-choice path failed silently.
+
+## Conversion is faster
+
+The parallelism cap was `min(ProcessorCount, 4)`. Conversion is bound by
+per-file I/O latency rather than CPU, so the cap bit well before core count did.
+It is now `min(ProcessorCount, 8)`.
+
+Measured over 2,000 files:
+
+| | Before (4) | After (8) |
+|---|---|---|
+| Without backups | 4,207 ms | 2,511 ms |
+| With backups | 10,516 ms | 7,305 ms |
+
+Past 8 the curve flattens and backup runs stop improving, which is why 8 rather
+than something larger. `ProcessorCount` still binds first on small machines, and
+`-MaxParallelism` still overrides.
+
+## Recovery files are read with the options they were written with
+
+The plan and sidecar stores passed a configured options object when writing and
+none when reading. Nothing observable changes today — both settings affect
+writing only — but a setting added later for the writer would have altered every
+file EC produces without altering what EC accepts, and each one would have
+stopped loading silently.
+
+## A defect backlog
+
+[`docs/DEFECT-BACKLOG.md`](DEFECT-BACKLOG.md) records the status of all
+thirty-five findings from the two independent reviews that preceded v3.11.0,
+re-derived from the source rather than carried over from a summary, plus
+everything found since. Open items are scored on two axes — what a user loses,
+and how easily it happens — because one number hides the difference between a
+critical impact nobody can trigger and a low impact everyone trips over.
+
+It also records three hashing optimisations that were built, measured and
+rejected, so the next person does not have to re-derive them.
+
+## Release engineering
+
+- The GUI smoke suite now **gates the release**. All nine phases run against the
+  signed, published executable, after signing and before packaging, and a
+  failure stops publication.
+- The release workflow can be **rehearsed without publishing**. A manual run does
+  everything a release does except create the release. Publishing requires a tag
+  push, not merely a tag reference, so a run started by hand cannot publish
+  whatever reference it was given.
+
+## Compatibility
+
+No conversion or classification behaviour changes. Conversion semantics stay at
+**6**, the plan schema at **5**, the journal schema at **4**, and exit codes are
+unchanged.
+
+The drive-root fix does change what happens when such a plan is applied — it
+converts where it previously refused — but the decisions a plan records mean
+exactly what they meant before. Only the resolution of the paths was wrong, so
+the semantics version is deliberately left alone.
+
+## Verification
+
+- 646 tests pass, none skipped; release build with no warnings
+- The nine-phase GUI smoke suite passes
+- Each fix was mutation-checked — the change reverted, the intended test required
+  to fail, the file restored byte-identical — **except** the options-object
+  change, which no test can demonstrate without adding a setting to production
+  code purely to make one fail. That is stated rather than papered over.
+- No four-corpus audit was run. The checklist requires one for a release that
+  changes detection or conversion policy; this changes neither.

sources/EncodingChecker.Tests/ConversionConfirmationFormTests.cs

@@ -1,4 +1,4 @@
-using System.Text;
+using System.Text;
 using System.Windows.Forms;
 
 namespace EncodingChecker.Tests;
@@ -110,6 +110,88 @@ public void ItBuildsWhenNothingIsRefused()
         });
     }
 
+    /// <summary>
+    /// A plan whose root does not contain its own files. The GUI reaches this whenever the
+    /// directory box changes after a scan - by typing, by picking a recent entry, or by
+    /// dropping a folder on it - because the results list is only cleared when a scan
+    /// starts.
+    /// </summary>
+    private ConversionPlan PlanRootedElsewhere(string target = "utf-8")
+    {
+        var entries = new EntrySink();
+
+        ScanEngine.ScanDirectory(
+            new ScanDirectoryOptions
+            {
+                BaseDirectory = _root,
+                IncludeSubdirectories = true,
+                IncludePatterns = ["*"],
+                Action = ScanAction.Convert,
+                TargetCharset = target,
+                TargetWriteBom = false,
+                WhatIf = true,
+            },
+            entries.Add,
+            CancellationToken.None);
+
+        string elsewhere = Directory.CreateTempSubdirectory("ec_elsewhere_").FullName;
+
+        return ConversionPlan.FromEntries(
+            entries, elsewhere, target, targetHasBom: false,
+            backupEnabled: true, explicitSource: null);
+    }
+
+    private static T Find<T>(Control root, string name) where T : Control =>
+        Descendants(root).OfType<T>().Single(c => c.Name == name);
+
+    [Fact]
+    public void ASourceChoiceThatCannotBeAppliedIsRefusedRatherThanDropped()
+    {
+        // Each refused row carries the path this review resolved for it. When the plan's
+        // root does not contain the file that path is null, and the ticked set used to be
+        // built by filtering those rows out - so the button did nothing, and the run
+        // reported "Conversion cancelled. No files were modified." The user had ticked a
+        // file and chosen an encoding; the cancellation was neither theirs nor explained.
+        Write("legacy.txt", "Le café était déjà prêt", "windows-1252");
+        ConversionPlan plan = PlanRootedElsewhere();
+
+        UiTest.OnStaThread(() =>
+        {
+            using var form = new ConversionConfirmationForm(plan);
+            form.CreateControl();
+
+            var refused = Find<ListView>(form, "lstRefusedFiles");
+            var chooser = Find<ComboBox>(form, "lstSourceEncoding");
+            var confirm = Find<Button>(form, "btnConfirmSourceEncoding");
+
+            Assert.Single(refused.Items);
+
+            // The row is listed, and the path behind it is exactly what is missing.
+            Assert.Null(refused.Items[0].Tag);
+
+            refused.Items[0].Checked = true;
+            chooser.SelectedItem = "windows-1252";
+
+            Assert.True(
+                confirm.Enabled,
+                "The confirm button was disabled, so the refusal below could not be reached.");
+
+            string? problem = form.DescribeUnusableScope();
+
+            Assert.False(
+                problem is null,
+                "The review would have applied a choice it cannot scope to any file, "
+                + "which resolves to a cancellation the user never asked for.");
+
+            Assert.Contains("no longer inside this review", problem!, StringComparison.Ordinal);
+            Assert.Contains("Run View again", problem, StringComparison.Ordinal);
+
+            // Refusing means staying put: nothing is chosen and no result is reported.
+            Assert.Equal(DialogResult.None, form.DialogResult);
+            Assert.Empty(form.ChosenFiles);
+        });
+    }
+
     [Fact]
     public void InteractiveControlsExposeStableAutomationIds()
     {

sources/EncodingChecker.Tests/ConversionPlanTests.cs

@@ -139,6 +139,65 @@ public void ThePlanKeepsNonAsciiNamesAndApostrophesReadable()
         Assert.DoesNotContain("\\u", json, StringComparison.Ordinal);
     }
 
+    private static ConversionPlan PlanRootedAt(string baseDirectory, string relativePath) =>
+        new()
+        {
+            CreatedUtc = "2026-01-01T00:00:00.0000000Z",
+            EcVersion = "test",
+            BaseDirectory = baseDirectory,
+            TargetEncoding = "utf-8",
+            TargetHasBom = false,
+            BackupEnabled = false,
+            Files =
+            [
+                new PlannedFile
+                {
+                    RelativePath = relativePath,
+                    Size = 1,
+                    Sha256 = new string('0', 64),
+                    Action = PlannedAction.Convert,
+                    SourceEncoding = "windows-1252",
+                    SourceCodePage = 1252,
+                    SourceHasBom = false,
+                    SourceWasSpecified = true,
+                    SourceInterpretation = SourceInterpretation.ExplicitSource,
+                },
+            ],
+        };
+
+    [Theory]
+    [InlineData(@"C:\", @"file.txt", @"C:\file.txt")]
+    [InlineData(@"C:\", @"sub\file.txt", @"C:\sub\file.txt")]
+    [InlineData(@"C:\data", @"file.txt", @"C:\data\file.txt")]
+    [InlineData(@"C:\data\", @"file.txt", @"C:\data\file.txt")]
+    public void APlanResolvesItsFilesWhateverItsRootLooksLike(
+        string baseDirectory, string relativePath, string expected)
+    {
+        // A drive root is left alone by TrimEndingDirectorySeparator, so the old
+        // containment prefix became "C:\\" and matched nothing. Every file in a plan
+        // rooted at a drive was then reported as resolving outside the plan's own
+        // directory, and the whole run was refused - with a message blaming the file
+        // paths rather than the root.
+        ConversionPlan plan = PlanRootedAt(baseDirectory, relativePath);
+
+        Assert.Equal(expected, plan.ResolvePath(plan.Files[0]));
+    }
+
+    [Theory]
+    [InlineData(@"C:\data", @"..\outside.txt")]
+    [InlineData(@"C:\data", @"..\data-sibling\file.txt")]
+    [InlineData(@"C:\data", @"")]
+    [InlineData(@"C:\", @"")]
+    public void APathThatIsNotAFileBeneathTheRootIsStillRejected(
+        string baseDirectory, string relativePath)
+    {
+        // Widening the prefix must not widen what the plan will touch. The empty cases
+        // resolve to the root directory itself, which is not a file in the plan.
+        ConversionPlan plan = PlanRootedAt(baseDirectory, relativePath);
+
+        Assert.Null(plan.ResolvePath(plan.Files[0]));
+    }
+
     [Fact]
     public void PlanningWritesNothing()
     {

sources/EncodingChecker/ConversionConfirmationForm.cs

@@ -23,6 +23,20 @@ internal sealed class ConversionConfirmationForm : Form
     private readonly ConversionPlan _plan;
     private readonly ComboBox _sourceChoice = new() { Name = "lstSourceEncoding" };
     private readonly Button _resolve = new() { Name = "btnConfirmSourceEncoding" };
+
+    /// <summary>
+    /// Shown in place when the ticked rows cannot be acted on. A silent no-op here
+    /// resolves to "Conversion cancelled. No files were modified." - a cancellation
+    /// the user never asked for, blamed on them.
+    /// </summary>
+    private readonly Label _scopeProblem = new()
+    {
+        Name = "lblSourceChoiceProblem",
+        AutoSize = true,
+        MaximumSize = new Size(660, 0),
+        Visible = false,
+        ForeColor = Color.FromArgb(0xB0, 0x28, 0x28),
+    };
     private ListView? _refusedList;
 
     /// <summary>
@@ -259,6 +273,13 @@ private Panel BuildRefusalPanel(List<PlannedFile> refused)
         _resolve.MinimumSize = new Size(230, 0);
         _resolve.Click += (_, _) =>
         {
+            if (DescribeUnusableScope() is { } problem)
+            {
+                ShowScopeProblem(problem);
+                return;
+            }
+
+            _scopeProblem.Visible = false;
             ChosenSourceEncoding = (string)_sourceChoice.SelectedItem!;
             ChosenFiles = TickedFiles();
             DialogResult = DialogResult.Retry;
@@ -269,6 +290,7 @@ private Panel BuildRefusalPanel(List<PlannedFile> refused)
 
         chooser.Controls.Add(_sourceChoice);
         chooser.Controls.Add(_resolve);
+        chooser.Controls.Add(_scopeProblem);
 
         var note = new Label
         {
@@ -305,6 +327,42 @@ private List<string> TickedFiles() =>
             .Select(p => p!)
     ];
 
+    /// <summary>
+    /// Why the ticked rows cannot be acted on, or <see langword="null"/> when they can.
+    /// </summary>
+    /// <remarks>
+    /// Separate from the click handler so it can be tested without showing a window:
+    /// <c>PerformClick</c> does nothing on a control that is not effectively visible, and
+    /// a unit test that had to show one would need an interactive desktop.
+    /// </remarks>
+    internal string? DescribeUnusableScope()
+    {
+        ListViewItem[] ticked =
+            [.. _refusedList?.CheckedItems.Cast<ListViewItem>() ?? []];
+
+        if (ticked.Length == 0)
+            return "Tick at least one file to use this encoding for.";
+
+        // A row carries the path this review resolved for it. A null one cannot be matched
+        // to an entry later, so acting on it would drop the file from the scope in silence.
+        int unresolvable = ticked.Count(item => item.Tag is not string);
+
+        if (unresolvable == 0)
+            return null;
+
+        return $"{unresolvable} of the {ticked.Length} ticked file(s) are no longer "
+               + "inside this review's directory, so the chosen encoding cannot be "
+               + "applied to them. Run View again for the directory these files are "
+               + "actually in, then choose the encoding.";
+    }
+
+    /// <summary>Says why the button did nothing, instead of doing nothing.</summary>
+    private void ShowScopeProblem(string message)
+    {
+        _scopeProblem.Text = message;
+        _scopeProblem.Visible = true;
+    }
+
     /// <summary>
     /// Keeps the button scope explicit.
     /// </summary>

sources/EncodingChecker/ConversionMetadata.cs

@@ -147,6 +147,10 @@ internal static class ConversionMetadataStore
     // default encoder escapes every non-ASCII character, which turns the text EC
     // exists to handle into \uXXXX. Relaxed escaping is safe here because none of
     // this JSON is ever embedded in HTML.
+    // One options object for both directions. Writing already used it; reading did not,
+    // so a setting added here for the writer - a naming policy, most obviously - would
+    // have changed what EC produces without changing what EC accepts, and every file it
+    // wrote would have stopped loading. Nothing observable changes today.
     private static readonly JsonSerializerOptions Options = new()
     {
         WriteIndented = true,
@@ -231,7 +235,8 @@ internal static string ComputeSha256(string path)
                 new UTF8Encoding(false));
 
             ConversionMetadata? readBack =
-                JsonSerializer.Deserialize<ConversionMetadata>(File.ReadAllText(tempPath));
+                JsonSerializer.Deserialize<ConversionMetadata>(
+                    File.ReadAllText(tempPath), Options);
 
             if (readBack is null)
                 return $"The temporary recovery record for '{path}' could not be read back.";