Fix false-success CSV reporting on cancel, stale plan, and mid-decide cancellation

docs/CONVERSION-WORKFLOW.md

@@ -21,8 +21,9 @@ For a file that needs conversion, EC applies this policy:
 | ASCII, Unicode with a BOM, or text whose encoding EC can prove from its bytes | Convert automatically |
 | Legacy text or BOM-less Unicode whose encoding cannot be proven safely | Do not convert; ask you to choose the original encoding |
 
-A file that already matches the target encoding and BOM is reported as **Unchanged** and
-is not decoded or rewritten. No source choice is needed because no conversion occurs.
+A file that already matches the target encoding and BOM is still strictly validated in
+full. A valid file is reported as **Unchanged** and is not rewritten; an invalid file is
+reported as an error instead. No source choice is needed because no conversion occurs.
 
 If you choose a source encoding, EC uses it only to read the original bytes. It does not
 disable strict decoding, output verification, backup verification, or safe installation.

docs/SAFETY.md

@@ -34,7 +34,9 @@ both the output and backup have verified.
 | ASCII, Unicode with a BOM, or text whose encoding EC can prove from its bytes | Convert automatically. |
 | Legacy text or BOM-less Unicode whose encoding cannot be proven safely | Do not convert; ask you to choose the original encoding. |
 
-An unchanged file is reported as `Unchanged` and is not decoded or rewritten.
+A file already matching the target is strictly validated in full. A valid file
+is reported as `Unchanged` and is not rewritten; an invalid file is reported as
+an error instead.
 
 A source encoding chosen by the user controls only how EC reads the original
 bytes. It does not bypass any safety check.

sources/EncodingChecker.Tests/ConversionOrchestrationTests.cs

@@ -415,6 +415,33 @@ public void ChoosingAnEncodingForNoFilesChangesNothing()
         Assert.Equal(before, File.ReadAllBytes(path));
     }
 
+    [Fact]
+    public void ChoosingAnEncodingForNoFilesChangesNothing_EvenForFilesAlreadyMarkedConverted()
+    {
+        // The scope check above uses a batch where the only file is ambiguous, so its
+        // Result was never "would convert" to begin with. A mixed batch is needed to
+        // prove this same no-op scope doesn't leak a stale Converted for an eligible
+        // file the decide pass already marked before this response was scripted.
+        string ambiguous = Write("ambiguous.txt", "Le café était déjà prêt", "windows-1252");
+        string eligible = Write("eligible.txt", "plain ascii here", "ascii");
+        byte[] ambiguousBefore = File.ReadAllBytes(ambiguous);
+        byte[] eligibleBefore = File.ReadAllBytes(eligible);
+
+        List<ConversionReportEntry> entries = View();
+        OrchestrationResult result = Convert(entries, _ => new ConfirmationResponse(
+            ConfirmationChoice.ChooseSourceEncoding, "windows-1252", []));
+
+        Assert.Equal(OrchestrationOutcome.Cancelled, result.Outcome);
+        Assert.Equal(ambiguousBefore, File.ReadAllBytes(ambiguous));
+        Assert.Equal(eligibleBefore, File.ReadAllBytes(eligible));
+
+        Assert.All(entries, entry => Assert.True(entry.NotAttempted));
+
+        using var csv = new StringWriter();
+        ConversionReport.WriteCsv(entries, csv);
+        Assert.DoesNotContain("Converted", csv.ToString());
+    }
+
     // ------------------------------------------------------- nothing gets modified
 
     [Fact]
@@ -426,15 +453,51 @@ public void CancellingTheConfirmation_ModifiesNothing()
         byte[] jpBefore = File.ReadAllBytes(jp);
         byte[] plainBefore = File.ReadAllBytes(plain);
 
+        List<ConversionReportEntry> entries = View();
         OrchestrationResult result = Convert(
-            View(), _ => ConfirmationResponse.Cancel, backup: true);
+            entries, _ => ConfirmationResponse.Cancel, backup: true);
 
         Assert.Equal(OrchestrationOutcome.Cancelled, result.Outcome);
         Assert.Equal(jpBefore, File.ReadAllBytes(jp));
         Assert.Equal(plainBefore, File.ReadAllBytes(plain));
 
         // Not even a backup, which would be a modification of the directory.
         Assert.Empty(Directory.GetFiles(_root, "*.bak"));
+
+        // The decide pass marks plain.txt "would convert" before the user is even asked.
+        // Cancelling must not let that leak into the exported report as completed work.
+        Assert.All(entries, entry => Assert.True(entry.NotAttempted));
+
+        using var csv = new StringWriter();
+        ConversionReport.WriteCsv(entries, csv);
+        Assert.DoesNotContain("Converted", csv.ToString());
+    }
+
+    [Fact]
+    public void CancellingDuringTheDecidePass_MarksEntriesUnattemptedBeforePropagating()
+    {
+        // The decide pass runs before the user is ever asked and can itself be
+        // cancelled. Whatever it already marked "would convert" must not be left
+        // looking like completed work once the exception propagates.
+        Write("plain.txt", "plain ascii here", "ascii");
+
+        List<ConversionReportEntry> entries = View();
+        using var cts = new CancellationTokenSource();
+        cts.Cancel();
+
+        Assert.ThrowsAny<OperationCanceledException>(() =>
+            new ConversionOrchestrator(Proceed).Run(
+                entries, _root, "utf-8", targetWriteBom: false,
+                backup: false, preview: false,
+                ScanEngine.DefaultMaxParallelism,
+                _ => { },
+                cts.Token));
+
+        Assert.All(entries, entry => Assert.True(entry.NotAttempted));
+
+        using var csv = new StringWriter();
+        ConversionReport.WriteCsv(entries, csv);
+        Assert.DoesNotContain("Converted", csv.ToString());
     }
 
     [Fact]
@@ -447,8 +510,9 @@ public void AFileChangedAfterTheConfirmation_StopsTheWholeRun()
 
         byte[] stableBefore = File.ReadAllBytes(stable);
 
+        List<ConversionReportEntry> entries = View();
         OrchestrationResult result = Convert(
-            View(),
+            entries,
             Proceed,
             betweenPlanAndWrite: plan =>
             {
@@ -466,6 +530,14 @@ public void AFileChangedAfterTheConfirmation_StopsTheWholeRun()
         // Neither file, not just the one that moved.
         Assert.Equal(stableBefore, File.ReadAllBytes(stable));
         Assert.Equal("changed underneath", File.ReadAllText(moving));
+
+        // Both entries were marked "would convert" by the decide pass before the plan was
+        // shown; going stale must not let that leak into the exported report as done work.
+        Assert.All(entries, entry => Assert.True(entry.NotAttempted));
+
+        using var csv = new StringWriter();
+        ConversionReport.WriteCsv(entries, csv);
+        Assert.DoesNotContain("Converted", csv.ToString());
     }
 
     [Fact]

sources/EncodingChecker.Tests/ConversionSafetyInvariantTests.cs

@@ -248,6 +248,11 @@ public void WhatIf_NeverModifiesAnything()
             SourceHasBom = false,
             TargetEncoding = "windows-1252",
             TargetHasBom = false,
+
+            // Without this, policy refuses the file before the dry-run branch is even
+            // reached (windows-1252 needs an explicit source), and the assertions below
+            // would hold trivially without exercising -WhatIf at all.
+            SourceEncodingWasSpecified = true,
         };
 
         var completed = new EntrySink();
@@ -256,6 +261,9 @@ public void WhatIf_NeverModifiesAnything()
             ScanEngine.DefaultMaxParallelism,
             whatIf: true, backup: false, completed.Add, CancellationToken.None);
 
+        // Confirms the file was actually eligible to convert, so preservation below is
+        // proof of dry-run behavior rather than an artifact of an earlier refusal.
+        Assert.Equal(ConversionRowResult.Converted, Assert.Single(completed).Result);
         Assert.Equal(original, File.ReadAllBytes(path));
         Assert.False(File.Exists(path + ".bak"));
     }

sources/EncodingChecker/ConversionOrchestrator.cs

@@ -123,6 +123,39 @@ internal OrchestrationResult Run(
         foreach (ConversionReportEntry entry in entries)
             entry.ResetAttemptEvidence();
 
+        try
+        {
+            return DecideConfirmAndRun(
+                entries, baseDirectory, targetCharset, targetWriteBom, backup, preview,
+                maxParallelism, onEntry, cancellationToken, startedUtc);
+        }
+        catch (OperationCanceledException)
+        {
+            // Cancellation anywhere before a write pass starts can leave entries the
+            // decide pass already marked "would convert". The write pass below has its
+            // own cancellation handling that returns normally instead of throwing, so it
+            // never reaches this catch; only a decide-phase cancellation does.
+            ConversionReportEntry.MarkUnattempted(entries, []);
+            throw;
+        }
+    }
+
+    /// <summary>
+    /// The decide/confirm/write sequence, split out so <see cref="Run"/> can wrap all of
+    /// it in one cancellation handler.
+    /// </summary>
+    private OrchestrationResult DecideConfirmAndRun(
+        IReadOnlyList<ConversionReportEntry> entries,
+        string baseDirectory,
+        string targetCharset,
+        bool targetWriteBom,
+        bool backup,
+        bool preview,
+        int maxParallelism,
+        Action<ConversionReportEntry> onEntry,
+        CancellationToken cancellationToken,
+        DateTime startedUtc)
+    {
         // Detection and hashing share one read, so the plan cannot mix different bytes.
         // Detection still runs for explicit choices to preserve provenance and safety vetoes.
         ScanEngine.RefreshSourceSnapshots(
@@ -158,6 +191,10 @@ internal OrchestrationResult Run(
             catch (InvalidOperationException ex)
             {
                 // An undecided entry means the caller failed to provide a complete plan.
+                // The decide pass above already marked some entries "would convert"; no
+                // write pass will ever reach them now.
+                ConversionReportEntry.MarkUnattempted(entries, []);
+
                 return new OrchestrationResult
                 {
                     Outcome = OrchestrationOutcome.CouldNotPlan,
@@ -169,6 +206,10 @@ internal OrchestrationResult Run(
 
             if (response.Choice == ConfirmationChoice.Cancel)
             {
+                // The decide pass already marked some entries "would convert"; the write
+                // pass that would have made that true is never going to run.
+                ConversionReportEntry.MarkUnattempted(entries, []);
+
                 return new OrchestrationResult
                 {
                     Outcome = OrchestrationOutcome.Cancelled,
@@ -182,6 +223,8 @@ internal OrchestrationResult Run(
                 if (!ApplyChosenSource(
                         response.SourceEncoding, response.Files, entries))
                 {
+                    ConversionReportEntry.MarkUnattempted(entries, []);
+
                     return new OrchestrationResult
                     {
                         Outcome = OrchestrationOutcome.Cancelled,
@@ -201,6 +244,10 @@ internal OrchestrationResult Run(
 
             if (stale.Count > 0)
             {
+                // The decide pass already marked some entries "would convert"; the write
+                // pass that would have made that true is never going to run.
+                ConversionReportEntry.MarkUnattempted(entries, []);
+
                 return new OrchestrationResult
                 {
                     Outcome = OrchestrationOutcome.PlanWentStale,