Fix bugs found during formal code review of all 43 review-sets

docs/design/spdx-tool-targets/spdx-tool-targets.md

@@ -17,27 +17,28 @@ The subsystem consists of two MSBuild `.targets` files:
 - `buildMultiTargeting/DemaConsulting.SpdxTool.Targets.targets` — injected for
   multi-TFM projects
 
-Both files define the `DecorateNuGetSbom` target, which runs after the `Pack` target
+Both files define the `DecorateSbomTarget` target, which runs after the `Pack` target
 in the MSBuild pipeline.
 
 ### Workflow Invocation
 
-The `DecorateNuGetSbom` target conditionally invokes `spdx-tool run-workflow` with
+The `DecorateSbomTarget` target conditionally invokes `spdx-tool run-workflow` with
 a user-supplied workflow file. The workflow file path is specified via the
-`SpdxWorkflowFile` MSBuild property. The `spdx-tool` global tool must be installed
-and available on the system `PATH`.
+`SpdxWorkflowFile` MSBuild property. The `spdx-tool` command is configurable via the
+`SpdxToolCommand` property (defaults to `dotnet spdx-tool`).
 
 ### Configuration Properties
 
-| MSBuild Property     | Default | Description                                          |
-|----------------------|---------|------------------------------------------------------|
-| `DecorateSBOM`       | `false` | Set to `true` to enable SBOM decoration during pack  |
-| `GenerateSBOM`       | `true`  | When `false`, skips decoration (no SBOM to decorate) |
-| `SpdxWorkflowFile`   | —       | Path to the workflow YAML file for decoration        |
+| MSBuild Property     | Default              | Description                                          |
+|----------------------|----------------------|------------------------------------------------------|
+| `DecorateSBOM`       | `false`              | Set to `true` to enable SBOM decoration during pack  |
+| `GenerateSBOM`       | `true`               | When `false`, skips decoration (no SBOM to decorate) |
+| `SpdxWorkflowFile`   | `spdx-workflow.yaml` | Path to the workflow YAML file for decoration        |
+| `SpdxToolCommand`    | `dotnet spdx-tool`   | Command used to invoke the spdx-tool                 |
 
 ## Conditional Execution
 
-The `DecorateNuGetSbom` target is skipped when:
+The `DecorateSbomTarget` target is skipped when:
 
 - `DecorateSBOM` is not set to `true` (opt-in required)
 - `GenerateSBOM` is `false` (no SBOM generated to decorate)
@@ -52,7 +53,7 @@ dotnet pack
 Pack target completes (NuGet .nupkg + embedded SBOM generated)
       │
       ▼
-DecorateNuGetSbom target
+DecorateSbomTarget target
       │
       ├─► Check DecorateSBOM == true  (skip if false)
       │

docs/design/spdx-tool/self-test/validate-run-nuget-workflow.md

@@ -11,10 +11,9 @@ resolved from a NuGet package in the local NuGet cache and executed successfully
 ### Setup
 
 1. Creates a `validate.tmp` working directory.
-2. Writes an SPDX JSON document as a target for the NuGet workflow.
-3. Writes a workflow YAML that uses the `nuget:` input to reference a known
-
-   NuGet package and a workflow file within it.
+2. Writes a workflow YAML that uses the `nuget:` input to reference a known
+   NuGet package (`DemaConsulting.SpdxWorkflows`) and a workflow file within it
+   (`GetDotNetVersion.yaml`), capturing the output into a variable.
 
 ### Execution
 
@@ -23,7 +22,6 @@ Calls `Validate.RunSpdxTool("validate.tmp", ["--silent", "run-workflow", "workfl
 ### Verification
 
 - The workflow must complete with exit code 0.
-- The SPDX document must be updated as expected by the NuGet workflow.
 
 ### Teardown
 
@@ -32,13 +30,11 @@ Deletes the `validate.tmp` directory.
 ## Error Handling
 
 - Returns `false` if `RunSpdxTool` returns a non-zero exit code.
-- Returns `false` if the SPDX document is not updated as expected.
 - The result is recorded in the `TestResults` collection as `Passed` or `Failed`.
 
 ## Constraints
 
 - Requires internet or NuGet cache access to download/restore the NuGet package.
 - The temporary directory is always deleted in a `finally` block.
 - `PathHelpers.SafePathCombine` is used to prevent path traversal when resolving
-
   the workflow file within the NuGet package.

docs/reqstream/spdx-tool/self-test/validate-basic.yaml

@@ -7,12 +7,12 @@ sections:
   - title: SelfTest ValidateBasic
     requirements:
       - id: SpdxTool-ST-Basic
-        title: The SelfTest shall exercise the basic commands (version, help, silent, log).
+        title: The SelfTest shall exercise the validate command with basic CLI flags (silent, log).
         tags:
           - self-test
         justification: |
-          Self-validation of basic tool functionality ensures the core CLI features
-          (version reporting, help display, silent mode, logging) work correctly
+          Self-validation of basic tool functionality ensures the core CLI flags
+          (silent mode, logging) work correctly with the validate command
           after installation or deployment.
         tests:
           - SpdxTool_Basic

docs/user_guide/introduction.md

@@ -93,7 +93,7 @@ spdx-tool [options] <command> [arguments]
 * `add-package` - Add package to SPDX document (workflow only)
 * `add-relationship <spdx.json> <args>` - Add relationship between elements
 * `copy-package <spdx.json> <args>` - Copy package between SPDX documents (workflow only)
-* `diagram <spdx.json> <mermaid.txt>` - Generate mermaid diagram
+* `diagram <spdx.json> <mermaid.txt> [tools]` - Generate mermaid diagram
 * `find-package <spdx.json> <criteria>` - Find package ID in SPDX document
 * `get-version <spdx.json> <criteria>` - Get the version of an SPDX package
 * `hash <operation> <algorithm> <file>` - Generate or verify hashes of files

src/DemaConsulting.SpdxTool/Commands/CopyPackage.cs

@@ -47,7 +47,7 @@ public sealed class CopyPackage : Command
     public static readonly CommandEntry Entry = new(
         Command,
         "copy-package <spdx.json> <args>",
-        "Copy package between SPDX documents (workflow only).",
+        "Copy package between SPDX documents.",
         [
             "This command copies a package from one SPDX document to another.",
             "",

src/DemaConsulting.SpdxTool/Commands/Query.cs

@@ -167,7 +167,7 @@ public static string QueryProgramOutput(string pattern, string program, string[]
         var stdoutTask = process.StandardOutput.ReadToEndAsync();
         var stderrTask = process.StandardError.ReadToEndAsync();
         Task.WaitAll(stdoutTask, stderrTask);
-        var output = stdoutTask.Result.Trim() + stderrTask.Result.Trim();
+        var output = (stdoutTask.Result + "\n" + stderrTask.Result).Trim();
 
         // Wait for the process to exit
         process.WaitForExit();

src/DemaConsulting.SpdxTool/Commands/SetVariable.cs

@@ -78,8 +78,8 @@
         var value = GetMapString(inputs, "value", variables) ??
                     throw new YamlException(step.Start, step.End, "'set-variable' command missing 'value' input");
 
-        // Get the 'output' input
-        var output = GetMapString(inputs, "output", variables) ??
+        // Get the 'output' input (not expanded - used literally as the variable key)
+        var output = GetMapString(inputs, "output") ??
                      throw new YamlException(step.Start, step.End, "'set-variable' command missing 'output' input");
 
         // Save the value to the variables

src/DemaConsulting.SpdxTool/Program.cs

@@ -149,12 +149,13 @@ public static void PrintUsage(Context context)
             Usage: spdx-tool [options] <command> [arguments]
 
             Options:
-              -h, --help                               Show this help message and exit
+              -h, -?, --help                           Show this help message and exit
               -v, --version                            Show version information and exit
               -l, --log <log-file>                     Log output to file
               -s, --silent                             Silence console output
                   --validate                           Perform self-validation
               -r, --result <file>                      Self-validation result file (.trx or .xml for JUnit)
+                  --depth <level>                      Self-validation report depth level
 
             Commands:
             """);

src/DemaConsulting.SpdxTool/SelfTest/ValidateBasic.cs

@@ -40,18 +40,18 @@
         // Report validation result to console
         if (passed)
         {
-            context.WriteLine($"✓ SpdxTool_Validate - Passed");
+            context.WriteLine($"✓ SpdxTool_Basic - Passed");
         }
         else
         {
-            context.WriteError($"✗ SpdxTool_Validate - Failed");
+            context.WriteError($"✗ SpdxTool_Basic - Failed");
         }
 
         // Add validation result to test results collection
         results.Results.Add(
             new TestResult
             {
-                Name = "SpdxTool_Validate",
+                Name = "SpdxTool_Basic",
                 ClassName = "DemaConsulting.SpdxTool.SelfTest.ValidateBasic",
                 ComputerName = Environment.MachineName,
                 StartTime = DateTime.Now,
@@ -68,7 +68,7 @@
         try
         {
             // Create the temporary validation folder
             Directory.CreateDirectory("validate.tmp");
 
             // Run validation tests for both valid and invalid documents
             return DoValidateValid() && DoValidateInvalid();

src/DemaConsulting.SpdxTool/SelfTest/ValidateRenameId.cs

@@ -136,6 +136,10 @@
                 Packages:
                 [
                     { Id: "SPDXRef-Package-2" }
+                ],
+                Relationships:
+                [
+                    { RelatedElement: "SPDXRef-Package-2" }
                 ]
             };
         }

test/DemaConsulting.SpdxTool.Tests/Commands/ToMarkdownTests.cs

@@ -154,15 +154,15 @@ public void ToMarkdown_ValidSpdxFile_GeneratesMarkdown()
 
             // Assert: Verify the root packages section
             var rootPackagesIndex = markdown.IndexOf("# Root Packages", StringComparison.Ordinal);
-            Assert.IsGreaterThanOrEqualTo(0, rootPackagesIndex);
+            Assert.IsGreaterThanOrEqualTo(rootPackagesIndex, 0);
 
             // Assert: Verify the packages section
             var packagesIndex = markdown.IndexOf("# Packages", StringComparison.Ordinal);
-            Assert.IsGreaterThanOrEqualTo(0, packagesIndex);
+            Assert.IsGreaterThanOrEqualTo(packagesIndex, 0);
 
             // Assert: Verify the tools section
             var toolsIndex = markdown.IndexOf("# Tools", StringComparison.Ordinal);
-            Assert.IsGreaterThanOrEqualTo(0, toolsIndex);
+            Assert.IsGreaterThanOrEqualTo(toolsIndex, 0);
 
             // Assert: Verify "Test Application" is a root package
             var testPackageIndex = markdown.IndexOf("| Test Application | 1.2.3 | MIT |", StringComparison.Ordinal);
@@ -174,7 +174,7 @@ public void ToMarkdown_ValidSpdxFile_GeneratesMarkdown()
 
             // Assert: Verify "Test Tool" is a tool
             var testToolPosition = markdown.IndexOf("| Test Tool | 3.4.5 | MIT |", StringComparison.Ordinal);
-            Assert.IsGreaterThan(toolsIndex, testToolPosition);
+            Assert.IsGreaterThan(testToolPosition, toolsIndex);
         }
         finally
         {