fix: address formal review findings from all 14 review-sets

[Copilot]

Pull Request

Description

Performed formal review steps on all 14 review-sets defined in .reviewmark.yaml and fixed the issues found.

High Severity

• Missing Uri.EscapeDataString for projectKey in SonarQubeClient.cs (4 locations): project keys containing special characters (e.g. Maven-style org.example:project) would produce malformed API URLs. Fixed at lines for components/show, qualitygates/project_status, issues/search, and hotspots/search — consistent with how branch was already handled.

Medium Severity

• HttpClient resource leak in Validation.cs: The mock factory passed ownsHttpClient=false, meaning the HttpClient created per test was never disposed. Changed to true so SonarQubeClient.Dispose() correctly releases it.
• TemporaryDirectory.Dispose() missing DirectoryNotFoundException: Directory.Delete() can throw DirectoryNotFoundException (TOCTOU race between Exists check and Delete call). Added to the exception filter alongside IOException and UnauthorizedAccessException.

Code Quality

• Repeated "application/json" literal in Validation.cs: Extracted into a private const string JsonContentType constant, replacing 5 inline occurrences.
• FetchPaginatedAsync cognitive complexity: Extracted the paging-check logic (three ternary reads and final if/else) into a new private static bool HasMorePages(JsonElement, int) helper, reducing the method's cognitive complexity from 16 to below the allowed threshold of 15.

Documentation / Traceability

• program.md satisfies-requirements section: Removed SonarMark-Server-QualityGate, SonarMark-Server-Issues, SonarMark-Server-HotSpots (belong to SonarQubeClient unit) and SonarMark-Validation-Run (belongs to Validation unit); added missing SonarMark-Report-Depth.
• sonar-qube-client.md satisfies-requirements section: Removed SonarMark-Server-Connect, SonarMark-Server-Auth, SonarMark-Server-ProjectKey, SonarMark-Server-Branch (all belong to the Program unit per unit-program.yaml).
• sonar-quality-result.md satisfies-requirements section: Removed SonarMark-Server-QualityGate, SonarMark-Server-Issues, SonarMark-Server-HotSpots (belong to SonarQubeClient unit; SonarQualityResult holds the data but does not fetch it).

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Code quality improvement

Related Issues

Pre-Submission Checklist

Before submitting this pull request, ensure you have completed the following:

Build and Test

• Code builds successfully: dotnet build --configuration Release
• All unit tests pass: dotnet test --configuration Release
• Self-validation tests pass:
  dotnet run --project src/DemaConsulting.SonarMark --configuration Release --framework net10.0
--no-build -- --validate
• Code produces zero warnings

Code Quality

• Code formatting is correct: dotnet format --verify-no-changes
• New code has appropriate XML documentation comments
• Static analyzer warnings have been addressed

Linting

• All linters pass: ./lint.sh (Unix/macOS) or cmd /c lint.bat / ./lint.bat (Windows)

Testing

• Added unit tests for new functionality
• Updated existing tests if behavior changed
• All tests follow the AAA (Arrange, Act, Assert) pattern
• Test coverage is maintained or improved

Documentation

• Updated README.md (if applicable)
• Updated ARCHITECTURE.md (if applicable)
• Added code examples for new features (if applicable)
• Updated requirements.yaml (if applicable)

Additional Notes

Formal review was performed by invoking the code-review agent separately on each of the 14 review-sets defined in .reviewmark.yaml to avoid context collisions between reviews.

[Copilot]

Pull request overview

This PR addresses formal review findings across the ReviewMark review-sets by fixing URL encoding in SonarQube API calls, correcting a self-test HttpClient ownership/disposal issue, hardening temporary directory cleanup, and updating requirements traceability in design docs.

Changes:

• Encode projectKey with Uri.EscapeDataString when building SonarQube API URLs (multiple endpoints).
• Fix HttpClient disposal in self-validation by setting ownsHttpClient=true, and ignore DirectoryNotFoundException during temp directory cleanup.
• Adjust design-doc “Satisfies Requirements” lists to correctly reflect unit ownership/traceability.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/DemaConsulting.SonarMark/SonarIntegration/SonarQubeClient.cs	Escapes projectKey in API query strings to prevent malformed URLs for keys containing special characters.
src/DemaConsulting.SonarMark/SelfTest/Validation.cs	Ensures mock-created HttpClient is disposed via SonarQubeClient.Dispose(), and hardens temp directory cleanup against TOCTOU delete races.
docs/design/sonar-mark/sonar-integration/sonar-qube-client.md	Removes requirements that belong to other units to keep traceability accurate.
docs/design/sonar-mark/report-generation/sonar-quality-result.md	Removes server-fetch requirements from a data/formatting record doc to reflect correct responsibility.
docs/design/sonar-mark/program.md	Updates requirement mapping by removing server-fetch/validation items and adding the missing report-depth trace.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Malcolmnixon]

@copilot Validation.cs is getting complaints "Define a constant instead of using this literal 'application/json' 5 times."

SonarQubeClient.FetchPaginatedAsync is getting "Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed."

[Copilot]

@copilot Validation.cs is getting complaints "Define a constant instead of using this literal 'application/json' 5 times."

SonarQubeClient.FetchPaginatedAsync is getting "Refactor this method to...

Both addressed in commit 96265d1:

• application/json literal: Added a private const string JsonContentType = "application/json" in Validation.cs and replaced all 5 usages with the constant.
• FetchPaginatedAsync cognitive complexity: Extracted the three ternary paging reads and the final if/else into a new private static bool HasMorePages(JsonElement pagingElement, int pageNumber) helper, reducing complexity to below the 15 threshold.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.