Skip to content

Run formal code reviews for all .reviewmark.yaml review-sets and apply findings#121

Merged
Malcolmnixon merged 3 commits intomainfrom
copilot/run-code-review-sub-agent
Mar 31, 2026
Merged

Run formal code reviews for all .reviewmark.yaml review-sets and apply findings#121
Malcolmnixon merged 3 commits intomainfrom
copilot/run-code-review-sub-agent

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 31, 2026

Pull Request

Description

Ran the code-review agent against all 13 review-sets defined in .reviewmark.yaml and applied every actionable finding.

Code fixes:

  • Context.WriteError – restore Console.ForegroundColor in a try-finally block so color is recovered even if Error.WriteLine throws
  • ContextTests.csContext_WriteError_Silent_DoesNotWriteToConsole was redirecting Console.Out instead of Console.Error; WriteError writes to stderr
  • GitHubRepoConnectorbranchReleases.ToDictionary(r => r.TagName!, ...) throws ArgumentException when two releases share a tag; switched to GroupBy(...).ToDictionary(...) to keep the first
  • ProcessRunner.TryRunAsync – redirected stderr but never read it; if the stderr pipe buffer fills the child process deadlocks; now reads both streams concurrently with Task.WhenAll

Documentation fixes:

  • validation.mdRunGitIntegration description said it runs against the local Git repo; it actually uses MockRepoConnector with hardcoded data like every other validation method
  • repo-connectors.md + introduction.mdGitHubRepoConnector.cs was listed under the GitHub/ subdirectory; the file lives in RepoConnectors/ directly

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 tests pass: dotnet test --configuration Release
  • Self-validation tests pass:
    dotnet run --project src/DemaConsulting.BuildMark --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

Quality Checks

Please run the following checks before submitting:

  • 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

Reviews with no actionable findings: BuildMark-Program, BuildMark-Validation, BuildMark-Cli, BuildMark-Utilities, BuildMark-System, BuildMark-AllRequirements, OTS-Dependencies.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/BuildMark/BuildMark/test/DemaConsulting.BuildMark.Tests/bin/Debug/net10.0/DemaConsulting.BuildMark.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/BuildMark/BuildMark/test/DemaConsulting.BuildMark.Tests/bin/Debug/net10.0/DemaConsulting.BuildMark.Tests.deps.json /home/REDACTED/work/BuildMark/BuildMark/test/DemaConsulting.BuildMark.Tests/bin/Debug/net10.0/testhost.dll --port 39035 --endpoint 127.0.0.1:039035 --role client --parentprocessid 4371 --telemetryoptedin false (http block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/BuildMark/BuildMark/test/DemaConsulting.BuildMark.Tests/bin/Debug/net8.0/DemaConsulting.BuildMark.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/BuildMark/BuildMark/test/DemaConsulting.BuildMark.Tests/bin/Debug/net8.0/DemaConsulting.BuildMark.Tests.deps.json /home/REDACTED/work/BuildMark/BuildMark/test/DemaConsulting.BuildMark.Tests/bin/Debug/net8.0/testhost.dll --port 43993 --endpoint 127.0.0.1:043993 --role client --parentprocessid 4363 --telemetryoptedin false (http block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/BuildMark/BuildMark/test/DemaConsulting.BuildMark.Tests/bin/Debug/net9.0/DemaConsulting.BuildMark.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/BuildMark/BuildMark/test/DemaConsulting.BuildMark.Tests/bin/Debug/net9.0/DemaConsulting.BuildMark.Tests.deps.json /home/REDACTED/work/BuildMark/BuildMark/test/DemaConsulting.BuildMark.Tests/bin/Debug/net9.0/testhost.dll --port 32911 --endpoint 127.0.0.1:032911 --role client --parentprocessid 4369 --telemetryoptedin false (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI and others added 2 commits March 31, 2026 21:25
…ag handling, stream deadlock, and doc corrections

Agent-Logs-Url: https://github.com/demaconsulting/BuildMark/sessions/87b2160b-c7f8-4317-a0d5-9007212c5f96

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Agent-Logs-Url: https://github.com/demaconsulting/BuildMark/sessions/87b2160b-c7f8-4317-a0d5-9007212c5f96

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
Copilot AI changed the title [WIP] Run code review sub-agent for formal reviews Run formal code reviews for all .reviewmark.yaml review-sets and apply findings Mar 31, 2026
Copilot AI requested a review from Malcolmnixon March 31, 2026 21:26
@Malcolmnixon Malcolmnixon marked this pull request as ready for review March 31, 2026 22:08
Copilot AI review requested due to automatic review settings March 31, 2026 22:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR applies fixes identified by running automated code reviews across the .reviewmark.yaml review-sets, focusing on correctness, deadlock avoidance, and documentation accuracy.

Changes:

  • Fixed console error handling and updated tests to correctly capture Console.Error.
  • Prevented duplicate-tag failures in GitHub release lookup and avoided potential process deadlocks by consuming both stdout/stderr.
  • Corrected design documentation to reflect actual validation behavior and the current file layout.

Reviewed changes

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

Show a summary per file
File Description
test/DemaConsulting.BuildMark.Tests/Cli/ContextTests.cs Updates the silent-mode WriteError test to capture stderr instead of stdout.
src/DemaConsulting.BuildMark/RepoConnectors/ProcessRunner.cs Ensures stderr is read alongside stdout in TryRunAsync to prevent pipe-buffer deadlocks.
src/DemaConsulting.BuildMark/RepoConnectors/GitHubRepoConnector.cs Handles duplicate release tag names safely by grouping before building the tag→release map.
src/DemaConsulting.BuildMark/Cli/Context.cs Restores console foreground color via try/finally to avoid leaving the console in a bad state on exceptions.
docs/design/self-test/validation.md Corrects RunGitIntegration documentation to match its use of MockRepoConnector.
docs/design/repo-connectors/repo-connectors.md Fixes the documented path for GitHubRepoConnector.cs.
docs/design/introduction.md Updates the repository structure diagram to reflect the actual location of GitHubRepoConnector.cs.

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

@Malcolmnixon Malcolmnixon merged commit 035c3b4 into main Mar 31, 2026
19 checks passed
@Malcolmnixon Malcolmnixon deleted the copilot/run-code-review-sub-agent branch March 31, 2026 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants