One policy engine, and a GUI that asks before it writes

README.md

@@ -14,7 +14,8 @@ Each [release](https://github.com/amrali-eg/EncodingChecker/releases) publishes
 
 - Layered detection: byte-order-mark and heuristic checks for Unicode encodings, [UtfUnknown](https://github.com/CharsetDetector/UTF-unknown) for legacy code pages, each candidate independently verified by strict decoding before being trusted.
 - Lossless, safe conversion: every write is verified afterward by comparing a SHA-256 hash of the decoded content, so a silent encoder substitution (e.g. an unrepresentable character) is caught and reported as an error instead of corrupting the file.
-- Refuses to convert files whose encoding the bytes do not determine, naming the encodings actually in conflict, with `-From` to supply the answer yourself.
+- Refuses to convert files whose encoding the bytes do not determine, naming the encodings actually in conflict, with `-From` (or the GUI's source-encoding selection) to supply the answer yourself. One policy engine decides for every surface.
+- The GUI confirms before writing, showing exactly what will happen to each file and carrying out that same plan rather than re-deciding.
 - `-Plan`/`-Apply` preflight: review what a conversion would do, then carry out exactly that — the plan is bound to the files' hashes and is refused whole if they change.
 - Optional `.bak` backup before overwriting, and a `-WhatIf` dry-run mode that reports what would happen without touching any file.
 - Covered by an xUnit test suite exercising the detection/conversion engine, CLI argument parsing, and CSV report formatting across multilingual content and edge cases.
@@ -28,6 +29,65 @@ Two options apply to **Convert**:
 - **Back up original files before converting (.bak)** — keeps each original as `<file>.bak` before it is replaced. The equivalent of the CLI's `-Backup`.
 - **Preview changes without modifying files** — reports which files *would* be converted without writing anything and without creating any `.bak`. Previewed rows keep their current encoding and stay selected, so you can review the result and then convert for real. The equivalent of the CLI's `-WhatIf`.
 
+### Confirming a conversion
+
+**Convert** does not write anything immediately. It first works out what would happen to
+every selected file, then shows that for approval:
+
+```
+Convert 417 of 480 selected file(s) to utf-8 without BOM
+
+  386  Encoding determined by the file's own bytes            Will convert.
+   31  Encoding undetermined, every reading agrees on the text Will convert; the label
+                                                               is a choice, the content
+                                                               is not.
+   22  Encoding undetermined, readings disagree on the text    WILL NOT be converted.
+   39  Already in the target encoding                          Nothing to do.
+    2  Encoding could not be identified                        Left alone.
+
+Directory        C:\Source
+Source encoding  detected per file
+Backups          enabled — each original kept as <file>.bak
+Guarantees       strict codecs, verified output, atomic install, ambiguity refusal
+```
+
+The conversion that runs is the one shown. Nothing is detected a second time between the
+confirmation and the writing, so the dialog cannot describe one set of conclusions while
+a different set is carried out — the same property `-Apply` has.
+
+If the files change between the confirmation and the writing, **nothing is converted** —
+the same all-or-nothing check `-Apply` makes, for the same reason: a person reading a
+dialog takes time, and what they approved was the files as they were.
+
+When files are refused, the dialog lists them with the encodings actually in conflict and
+offers the one thing that resolves it: saying which encoding they are. That selection is
+the GUI's `-From`. It replaces detection for those files and nothing else — the bytes must
+still decode strictly as the chosen encoding, the output is still verified to hold exactly
+the same text, and a failed backup still stops the conversion.
+
+The choice applies only to the files you tick, and the button says how many. A batch can
+easily hold refused files in different encodings — Cyrillic in koi8-r beside French in
+windows-1252 — and one answer settles only the files it was given about. Imposing it on
+the rest would repeat, one level up, the mistake the refusal exists to prevent.
+
+### One policy engine
+
+The GUI and the CLI ask the same question of the same code:
+
+```
+detection / explicit source → classification → PlannedAction → CLI, GUI, plan
+```
+
+[`ConversionPolicy`](sources/EncodingChecker/ConversionPolicy.cs) decides; every surface
+acts on that decision rather than reaching its own. A missing classification is an
+internal error, never a safe state: an entry that reaches a conversion or a plan without
+one is refused or raises, rather than being treated as unambiguous.
+
+EC also counts how often it works out an encoding, and asserts in its test suite that a
+file is never examined twice — once when scanned, and never again between a decision being
+approved and carried out. Applying a plan, and confirming a GUI conversion, do no
+detection at all.
+
 ## Command-line usage
 
 Launch `EncodingChecker.exe` with arguments to run in console mode instead. Run `EncodingChecker.exe -?` (or `-h`, `/?`, `--help`) at any time to print this from the tool itself.
@@ -219,8 +279,13 @@ These are the guarantees the implementation actually provides.
   opt-in, since a script can keep the report.
 - A conversion whose source encoding cannot be determined from the file's own
   bytes is refused rather than guessed at, when the competing encodings would
-  produce different text. `-From` overrides the detection, not the conversion
-  safeguards. See [Ambiguous encodings](#ambiguous-encodings-and--from).
+  produce different text. `-From`, and the GUI's source-encoding selection,
+  override the detection, not the conversion safeguards.
+  See [Ambiguous encodings](#ambiguous-encodings-and--from).
+  <br>That decision is made in one place for every surface, so the GUI and the
+  CLI cannot diverge on what is safe. They previously could, and did: the
+  classification ran only during a Convert-mode scan, the GUI scans in Detect
+  mode, and so the GUI converted the files the CLI refused.
 - A plan written by `-Plan` is bound to the SHA-256 of every file it schedules,
   to the directory those files are under, and to the conversion behaviour it was
   approved under. `-Apply` verifies all of them before writing anything and

sources/EncodingChecker.Tests/AssemblyInfo.cs

@@ -0,0 +1,9 @@
+using Xunit;
+
+// DetectionCountTests measures process-global counters to assert that EC never works out
+// a file's encoding twice. Those counts are only meaningful if nothing else is detecting
+// at the same time, and xUnit runs test classes in parallel by default.
+//
+// The whole suite runs in well under a second, so serialising it costs nothing worth
+// weighing against being able to state that invariant as a test.
+[assembly: CollectionBehavior(DisableTestParallelization = true)]

sources/EncodingChecker.Tests/ConversionConfirmationFormTests.cs

@@ -0,0 +1,286 @@
+using System.Text;
+using System.Windows.Forms;
+
+namespace EncodingChecker.Tests;
+
+/// <summary>
+/// The confirmation dialog is the only part of the safety model a GUI user ever reads,
+/// and until now it was also the only part no test had ever executed. Layout code that
+/// has never run is layout code that throws the first time somebody converts a directory
+/// with an unusual mix of outcomes — and it would throw at exactly the moment the user is
+/// being asked to approve something.
+///
+/// These build it against real plans rather than asserting on pixels: every outcome mix,
+/// on an STA thread, checking that it constructs and that what it says matches the plan
+/// it was given.
+/// </summary>
+public sealed class ConversionConfirmationFormTests : IDisposable
+{
+    private readonly string _root =
+        Directory.CreateTempSubdirectory("ec_dialog_").FullName;
+
+    public void Dispose()
+    {
+        try
+        {
+            Directory.Delete(_root, recursive: true);
+        }
+        catch (IOException)
+        {
+            // Best-effort cleanup.
+        }
+    }
+
+    /// <summary>Runs <paramref name="body"/> on an STA thread, as WinForms requires.</summary>
+    private static void OnUiThread(Action body)
+    {
+        Exception? failure = null;
+
+        var thread = new Thread(() =>
+        {
+            try
+            {
+                body();
+            }
+            catch (Exception ex)
+            {
+                failure = ex;
+            }
+        });
+
+        thread.SetApartmentState(ApartmentState.STA);
+        thread.Start();
+
+        Assert.True(thread.Join(TimeSpan.FromSeconds(30)), "the dialog did not finish");
+
+        if (failure is not null)
+            throw new Xunit.Sdk.XunitException($"the dialog threw: {failure}");
+    }
+
+    private void Write(string name, string text, string charset) =>
+        File.WriteAllBytes(
+            Path.Combine(_root, name), Encoding.GetEncoding(charset).GetBytes(text));
+
+    /// <summary>A plan over whatever is currently in the directory.</summary>
+    private ConversionPlan Plan(bool backup = true, string target = "utf-8")
+    {
+        var entries = new List<ConversionReportEntry>();
+
+        ScanEngine.ScanDirectory(
+            new ScanDirectoryOptions
+            {
+                BaseDirectory = _root,
+                IncludeSubdirectories = true,
+                IncludePatterns = ["*"],
+                Action = ScanAction.Convert,
+                TargetCharset = target,
+                TargetWriteBom = false,
+                WhatIf = true,
+            },
+            entries.Add,
+            CancellationToken.None);
+
+        return ConversionPlan.FromEntries(
+            entries, _root, target, targetHasBom: false,
+            backupEnabled: backup, explicitSource: null);
+    }
+
+    private static IEnumerable<Control> Descendants(Control root)
+    {
+        foreach (Control child in root.Controls)
+        {
+            yield return child;
+
+            foreach (Control nested in Descendants(child))
+                yield return nested;
+        }
+    }
+
+    private static string AllText(Control root) =>
+        string.Join("\n", Descendants(root).Select(c => c.Text));
+
+    [Fact]
+    public void ItBuildsForAMixOfEveryOutcome()
+    {
+        Write("jp.txt", "こんにちは世界。日本語のテキストです。", "shift_jis");
+        Write("ambiguous.txt", "Le café était déjà prêt", "windows-1252");
+        Write("plain.txt", "just ascii here", "ascii");
+        Write("already.txt", "already utf-8 世界", "utf-8");
+
+        ConversionPlan plan = Plan();
+
+        OnUiThread(() =>
+        {
+            using var form = new ConversionConfirmationForm(plan);
+
+            Assert.NotEmpty(Descendants(form).ToList());
+        });
+    }
+
+    [Fact]
+    public void ItBuildsWhenNothingIsRefused()
+    {
+        // The common case, and the one where a refusal panel must not appear at all.
+        Write("jp.txt", "こんにちは世界。日本語のテキストです。", "shift_jis");
+
+        ConversionPlan plan = Plan();
+
+        OnUiThread(() =>
+        {
+            using var form = new ConversionConfirmationForm(plan);
+
+            Assert.DoesNotContain("need an explicit source encoding", AllText(form));
+        });
+    }
+
+    [Fact]
+    public void ItBuildsWhenEverythingIsRefused()
+    {
+        // Nothing to convert. The button has to say so rather than offering an action
+        // that would do nothing.
+        Write("a.txt", "Le café était déjà prêt", "windows-1252");
+        Write("b.txt", "Привет мир, это русский", "koi8-r");
+
+        ConversionPlan plan = Plan();
+
+        Assert.All(plan.Files, f => Assert.Equal(PlannedAction.Refuse, f.Action));
+
+        OnUiThread(() =>
+        {
+            using var form = new ConversionConfirmationForm(plan);
+
+            string text = AllText(form);
+
+            Assert.Contains("need an explicit source encoding", text);
+            Assert.Contains("Nothing to convert", text);
+            Assert.DoesNotContain("Convert 1 file", text);
+        });
+    }
+
+    [Fact]
+    public void ItNamesTheCompetingEncodingsRatherThanJustReportingLowConfidence()
+    {
+        // "Could not be determined" on its own gives a user nothing to act on. The
+        // alternatives and the way out are what make the refusal actionable.
+        Write("ambiguous.txt", "Le café était déjà prêt", "windows-1252");
+
+        ConversionPlan plan = Plan();
+        PlannedFile refused = Assert.Single(plan.Files);
+
+        Assert.NotEmpty(refused.CompetingEncodings);
+
+        OnUiThread(() =>
+        {
+            using var form = new ConversionConfirmationForm(plan);
+
+            List<string> cells =
+            [
+                .. Descendants(form)
+                    .OfType<ListView>()
+                    .SelectMany(v => v.Items.Cast<ListViewItem>())
+                    .SelectMany(i => i.SubItems.Cast<ListViewItem.ListViewSubItem>())
+                    .Select(sub => sub.Text)
+            ];
+
+            Assert.Contains("ambiguous.txt", cells);
+            Assert.Contains(cells, c => c.Contains(refused.CompetingEncodings[0]));
+
+            // And the way out is offered, populated from the charsets EC supports.
+            ComboBox chooser = Assert.Single(Descendants(form).OfType<ComboBox>());
+
+            Assert.True(chooser.Items.Count > 1);
+            Assert.Contains("windows-1252", chooser.Items.Cast<string>());
+        });
+    }
+
+    [Fact]
+    public void ItMakesTheScopeOfAnEncodingChoiceUnmistakable()
+    {
+        // The button says how many files the choice would apply to, and the count moves
+        // with the ticks. A user must never have to infer how far their answer reaches.
+        Write("french.txt", "Le café était déjà prêt", "windows-1252");
+        Write("russian.txt", "Привет мир, это русский текст", "koi8-r");
+
+        ConversionPlan plan = Plan();
+
+        Assert.Equal(2, plan.Files.Count(f => f.Action == PlannedAction.Refuse));
+
+        OnUiThread(() =>
+        {
+            using var form = new ConversionConfirmationForm(plan);
+
+            // ListView caches check state until it has a window handle, and only raises
+            // ItemChecked once it does. Nothing here pumps messages; the handle is enough.
+            form.CreateControl();
+            _ = form.Handle;
+
+            ListView list = Assert.Single(Descendants(form).OfType<ListView>());
+
+            list.CreateControl();
+            _ = list.Handle;
+
+            // Everything the dialog asked about starts ticked, and the button says so.
+            Assert.Equal(2, list.CheckedItems.Count);
+            Assert.Contains("for 2 file(s)", AllText(form));
+
+            list.Items[0].Checked = false;
+
+            Assert.Contains("for 1 file(s)", AllText(form));
+
+            // And with none ticked there is nothing to apply, so it cannot be pressed.
+            list.Items[1].Checked = false;
+
+            Button apply = Assert.Single(
+                Descendants(form).OfType<Button>(),
+                b => b.Text.StartsWith("Use this encoding"));
+
+            Assert.False(apply.Enabled);
+        });
+    }
+
+    [Fact]
+    public void ItSaysWhetherOriginalsWillBeKept()
+    {
+        // Whether a conversion is undoable is part of what is being approved.
+        Write("jp.txt", "こんにちは世界。テキスト", "shift_jis");
+
+        ConversionPlan withBackups = Plan(backup: true);
+        ConversionPlan without = Plan(backup: false);
+
+        OnUiThread(() =>
+        {
+            using var kept = new ConversionConfirmationForm(withBackups);
+            using var lost = new ConversionConfirmationForm(without);
+
+            Assert.Contains(".bak", AllText(kept));
+            Assert.Contains("DISABLED", AllText(lost));
+        });
+    }
+
+    [Fact]
+    public void ItReportsThePlanItWasGivenRatherThanRecountingTheDirectory()
+    {
+        // The dialog must describe the decisions that will execute. Recomputing here is
+        // how a confirmation ends up describing something other than what happens.
+        Write("jp.txt", "こんにちは世界。日本語のテキストです。", "shift_jis");
+        Write("ambiguous.txt", "Le café était déjà prêt", "windows-1252");
+
+        ConversionPlan plan = Plan();
+
+        int convert = plan.Files.Count(f => f.Action == PlannedAction.Convert);
+
+        // Change the directory after planning. The dialog must not notice.
+        File.Delete(Path.Combine(_root, "jp.txt"));
+        Write("late-arrival.txt", "added after the plan", "ascii");
+
+        OnUiThread(() =>
+        {
+            using var form = new ConversionConfirmationForm(plan);
+
+            string text = AllText(form);
+
+            Assert.Contains($"Convert {convert} of {plan.Files.Count} selected", text);
+            Assert.DoesNotContain("late-arrival", text);
+        });
+    }
+}