fix: resolve VersionMark-CommandLine-Review, VersionMark-Utilities-Review, VersionMark-Configuration-Review, VersionMark-VersionInfo-Review, and VersionMark-Validation-Review failures (#37)

Copilot · Malcolmnixon · Copilot · web-flow · commit 7c3c0dd4da8e · 2026-03-14T14:12:48.000-04:00
* Initial plan

* fix: address VersionMark-CommandLine-Review failures

- Add missing justification and tags to Cmd-ErrorOutput, Cmd-InvalidArgs, Cmd-ExitCode requirements
- Fix stale test reference VersionMark_GeneratesMarkdownReport -&gt; VersionMark_PublishCommand_GeneratesMarkdownReport in publish.yaml
- Remove non-existent VersionMark_CapturesVersions test reference from capture.yaml
- Align priority table in command-line.md with Program.cs priority comments

Co-authored-by: Malcolmnixon &lt;1863707+Malcolmnixon@users.noreply.github.com&gt;

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI &lt;175728472+Copilot@users.noreply.github.com&gt;

* fix: correct VersionMark-Fmt-MarkdownList requirement wording (subscript → bold)

Co-authored-by: Malcolmnixon &lt;1863707+Malcolmnixon@users.noreply.github.com&gt;

* fix: correct stale remarks on RunCommand in VersionMarkConfig.cs

Co-authored-by: Malcolmnixon &lt;1863707+Malcolmnixon@users.noreply.github.com&gt;

* fix: resolve VersionMark-VersionInfo-Review failures

Co-authored-by: Malcolmnixon &lt;1863707+Malcolmnixon@users.noreply.github.com&gt;

* fix: resolve VersionMark-Validation-Review failures

Co-authored-by: Malcolmnixon &lt;1863707+Malcolmnixon@users.noreply.github.com&gt;

* fix: strengthen two weak test assertions in CommandLine test files

TEST-01 fix 1 - Program_Run_WithVersionFlag_DisplaysVersionOnly (ProgramTests.cs):
Added positive assertion that a version string IS printed, in addition to the
existing negative assertions that the banner is NOT shown. Without this,
the test would pass even if nothing was printed at all.

TEST-01 fix 2 - IntegrationTest_SilentFlag_SuppressesOutput (IntegrationTests.cs):
Captured the output instead of discarding it and added DoesNotContain assertions
for the tool's normal banner content. The prior test only checked exit code and
had a comment 'Output check removed', making the test name misleading.

All 57 ContextTests/ProgramTests/IntegrationTests pass on net8.0/net9.0/net10.0.

---------

Co-authored-by: copilot-swe-agent[bot] &lt;198982749+Copilot@users.noreply.github.com&gt;
Co-authored-by: Malcolmnixon &lt;1863707+Malcolmnixon@users.noreply.github.com&gt;
Co-authored-by: Malcolm Nixon &lt;Malcolm.nixon@gmail.com&gt;
Co-authored-by: Copilot Autofix powered by AI &lt;175728472+Copilot@users.noreply.github.com&gt;
diff --git a/docs/design/command-line.md b/docs/design/command-line.md
@@ -34,12 +34,12 @@ generate event-log entries. This satisfies requirements `VersionMark-Cmd-ExitCod
 | Priority | Condition              | Action                          |
 |----------|------------------------|---------------------------------|
 | 1        | `context.Version`      | Print version string and return |
-| 2        | Print banner           | Always executed after priority 1|
-| 3        | `context.Help`         | Print usage and return          |
-| 4        | `context.Validate`     | Run self-validation and return  |
-| 5        | `context.Capture`      | Run capture mode and return     |
-| 5.5      | `context.Publish`      | Run publish mode and return     |
-| 6        | Default                | Run placeholder tool logic      |
+| —        | Print banner           | Always executed after priority 1|
+| 2        | `context.Help`         | Print usage and return          |
+| 3        | `context.Validate`     | Run self-validation and return  |
+| 4        | `context.Capture`      | Run capture mode and return     |
+| 4.5      | `context.Publish`      | Run publish mode and return     |
+| 5        | Default                | Run placeholder tool logic      |
 
 This dispatch order satisfies requirements `VersionMark-Cmd-Version`, `VersionMark-Cmd-Help`,
 `VersionMark-Cmd-Validate`, `VersionMark-Cap-Capture`, and `VersionMark-Pub-Publish`.
diff --git a/docs/design/validation.md b/docs/design/validation.md
@@ -30,7 +30,8 @@ organizes all test execution internally.
 
 1. Creates a `TemporaryDirectory` (see below).
 2. Writes a minimal `.versionmark.yaml` containing only the `dotnet` tool.
-3. Constructs a `Context` with `--capture`, `--job-id test-job`, and `--output <file>`.
+3. Constructs a `Context` with `--silent`, `--log <file>`, `--capture`, `--job-id test-job`,
+   and `--output <file>`.
 4. Changes the current directory to the temp directory and calls `Program.Run`.
 5. Verifies exit code is 0, output file exists, `JobId` equals `"test-job"`, and `dotnet`
    version was captured and is non-empty.
@@ -43,7 +44,8 @@ The test name is `VersionMark_CapturesVersions`, satisfying `VersionMark-Cap-Cap
 
 1. Creates a `TemporaryDirectory`.
 2. Writes two `VersionInfo` JSON files with known content.
-3. Constructs a `Context` with `--publish`, `--report <file>`, and `-- versionmark-*.json`.
+3. Constructs a `Context` with `--silent`, `--log <file>`, `--publish`, `--report <file>`,
+   `--report-depth 2`, and `-- versionmark-*.json`.
 4. Changes the current directory to the temp directory and calls `Program.Run`.
 5. Verifies exit code is 0, report file exists, and contains `## Tool Versions`,
    `**dotnet**`, `**node**`, `8.0.0`, and `20.0.0`.
diff --git a/docs/design/version-info.md b/docs/design/version-info.md
@@ -20,7 +20,8 @@ The `VersionInfo` record (`VersionInfo.cs`) is a positional record with two prop
 `SaveToFile` serializes the record to indented JSON using `JsonSerializer.Serialize` with
 `WriteIndented = true` and writes it to the specified path using UTF-8 encoding. Non-`InvalidOperationException`
 errors are wrapped and re-thrown as `InvalidOperationException` with context. This satisfies
-requirements `VersionMark-Cap-JsonOutput` and `VersionMark-Cap-DefaultOutput`.
+requirement `VersionMark-Cap-JsonOutput`. The default output filename (`versionmark-<job-id>.json`)
+is determined by the CLI layer and contributes to satisfying `VersionMark-Cap-DefaultOutput`.
 
 ### LoadFromFile Method
 
diff --git a/docs/reqstream/capture.yaml b/docs/reqstream/capture.yaml
@@ -12,7 +12,6 @@ sections:
             tags:
               - capture
             tests:
-              - VersionMark_CapturesVersions
               - Program_Run_WithCaptureCommand_CapturesToolVersions
               - IntegrationTest_CaptureCommand_CapturesToolVersions
 
diff --git a/docs/reqstream/command-line.yaml b/docs/reqstream/command-line.yaml
@@ -94,12 +94,22 @@ sections:
 
           - id: VersionMark-Cmd-ErrorOutput
             title: The tool shall write error messages to stderr.
+            justification: |
+              Separating error output from standard output allows callers and CI/CD
+              pipelines to distinguish normal output from failure diagnostics.
+            tags:
+              - cli
             tests:
               - Context_WriteError_NotSilent_WritesToConsole
               - IntegrationTest_UnknownArgument_ReturnsError
 
           - id: VersionMark-Cmd-InvalidArgs
             title: The tool shall reject unknown or malformed command-line arguments with a descriptive error.
+            justification: |
+              Clear rejection of invalid arguments prevents silent misconfiguration and
+              guides users toward correct usage without requiring external documentation.
+            tags:
+              - cli
             tests:
               - Context_Create_UnknownArgument_ThrowsArgumentException
               - Context_Create_LogFlag_WithoutValue_ThrowsArgumentException
@@ -108,6 +118,11 @@ sections:
 
           - id: VersionMark-Cmd-ExitCode
             title: The tool shall return a non-zero exit code on failure.
+            justification: |
+              A non-zero exit code on failure enables CI/CD pipelines and scripts to
+              detect and respond to tool failures automatically.
+            tags:
+              - cli
             tests:
               - Context_WriteError_SetsErrorExitCode
               - IntegrationTest_UnknownArgument_ReturnsError
diff --git a/docs/reqstream/formatter.yaml b/docs/reqstream/formatter.yaml
@@ -41,7 +41,7 @@ sections:
 
           - id: VersionMark-Fmt-MarkdownList
             title: >-
-              The tool shall use subscript formatting and parenthesized job IDs to indicate which
+              The tool shall use bold formatting and parenthesized job IDs to indicate which
               jobs had which versions.
             justification: |
               Provides clear visual distinction between different job executions and their
diff --git a/docs/reqstream/publish.yaml b/docs/reqstream/publish.yaml
@@ -12,7 +12,7 @@ sections:
             tags:
               - publish
             tests:
-              - VersionMark_GeneratesMarkdownReport
+              - VersionMark_PublishCommand_GeneratesMarkdownReport
 
           - id: VersionMark-Pub-Report
             title: The tool shall support --report parameter to specify the output markdown file path in publish mode.
@@ -22,7 +22,7 @@ sections:
             tags:
               - publish
             tests:
-              - VersionMark_GeneratesMarkdownReport
+              - VersionMark_PublishCommand_GeneratesMarkdownReport
 
           - id: VersionMark-Pub-ReportDepth
             title: The tool shall support --report-depth parameter to control heading depth in generated markdown.
@@ -52,7 +52,8 @@ sections:
             tags:
               - publish
             tests:
-              - VersionMark_GeneratesMarkdownReport
+              - VersionMark_PublishCommandWithCustomGlobPatterns_FiltersFiles
+              - Context_Create_GlobPatternsAfterSeparator_CapturesPatterns
 
           - id: VersionMark-Pub-Consolidate
             title: The tool shall read and parse JSON files matching the specified glob patterns in publish mode.
@@ -62,7 +63,7 @@ sections:
             tags:
               - publish
             tests:
-              - VersionMark_GeneratesMarkdownReport
+              - VersionMark_PublishCommand_GeneratesMarkdownReport
 
           - id: VersionMark-Pub-ConflictReport
             title: The tool shall report errors when no JSON files match the specified glob patterns.
diff --git a/src/DemaConsulting.VersionMark/VersionMarkConfig.cs b/src/DemaConsulting.VersionMark/VersionMarkConfig.cs
@@ -331,8 +331,9 @@ public VersionInfo FindVersions(IEnumerable<string> toolNames, string jobId)
     /// <returns>The combined stdout and stderr output.</returns>
     /// <exception cref="InvalidOperationException">Thrown when the command fails to execute.</exception>
     /// <remarks>
-    ///     Commands are split on the first space character to separate executable from arguments.
-    ///     This does not handle quoted arguments containing spaces.
+    ///     Commands are delegated to the OS shell (<c>cmd.exe /c</c> on Windows, <c>/bin/sh -c</c>
+    ///     elsewhere) via <c>ArgumentList</c> to avoid escaping issues. This supports <c>.cmd</c>/<c>.bat</c>
+    ///     files on Windows and shell features (pipes, redirects, built-ins) on all platforms.
     /// </remarks>
     private static string RunCommand(string command)
     {
diff --git a/test/DemaConsulting.VersionMark.Tests/IntegrationTests.cs b/test/DemaConsulting.VersionMark.Tests/IntegrationTests.cs
@@ -196,15 +196,17 @@ public void IntegrationTest_SilentFlag_SuppressesOutput()
     {
         // Arrange & Act - Run the application with --silent flag
         var exitCode = Runner.Run(
-            out var _,
+            out var output,
             "dotnet",
             _dllPath,
             "--silent");
 
         // Assert - Verify success
         Assert.AreEqual(0, exitCode);
 
-        // Output check removed since silent mode may still produce some output
+        // Verify the tool's normal output is suppressed
+        Assert.DoesNotContain("VersionMark version", output);
+        Assert.DoesNotContain("Copyright (c) DEMA Consulting", output);
     }
 
     /// <summary>
diff --git a/test/DemaConsulting.VersionMark.Tests/ProgramTests.cs b/test/DemaConsulting.VersionMark.Tests/ProgramTests.cs
@@ -45,6 +45,7 @@ public void Program_Run_WithVersionFlag_DisplaysVersionOnly()
 
             // Assert - Verify version-only output
             var output = outWriter.ToString();
+            Assert.IsFalse(string.IsNullOrWhiteSpace(output), "Version string should be printed");
             Assert.DoesNotContain("Copyright", output);
             Assert.DoesNotContain("VersionMark version", output);
         }
diff --git a/test/DemaConsulting.VersionMark.Tests/VersionInfoTests.cs b/test/DemaConsulting.VersionMark.Tests/VersionInfoTests.cs
@@ -278,6 +278,31 @@ public void VersionInfo_EmptyVersions_SavesAndLoadsCorrectly()
         }
     }
 
+    /// <summary>
+    ///     Test LoadFromFile throws ArgumentException when JSON deserializes to null (e.g., literal "null").
+    /// </summary>
+    [TestMethod]
+    public void VersionInfo_LoadFromFile_NullJson_ThrowsArgumentException()
+    {
+        // Arrange
+        var tempFile = Path.GetTempFileName();
+        try
+        {
+            File.WriteAllText(tempFile, "null");
+
+            // Act & Assert
+            var exception = Assert.Throws<ArgumentException>(() => VersionInfo.LoadFromFile(tempFile));
+            Assert.Contains("deserialize", exception.Message, StringComparison.OrdinalIgnoreCase);
+        }
+        finally
+        {
+            if (File.Exists(tempFile))
+            {
+                File.Delete(tempFile);
+            }
+        }
+    }
+
     /// <summary>
     ///     Test VersionInfo with special characters in values.
     /// </summary>

Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,7 @@ public void Program_Run_WithVersionFlag_DisplaysVersionOnly()`
`45`	`45`
`46`	`46`	`// Assert - Verify version-only output`
`47`	`47`	`var output = outWriter.ToString();`
	`48`	`+ Assert.IsFalse(string.IsNullOrWhiteSpace(output), "Version string should be printed");`
`48`	`49`	`Assert.DoesNotContain("Copyright", output);`
`49`	`50`	`Assert.DoesNotContain("VersionMark version", output);`
`50`	`51`	`}`