---
name: reviewing-csharp
description: >-
  Reviews C# changes in the ImageBuilder codebase against the repo's modern C#
  conventions and invariants. Use when reviewing a C# diff, pull request, or
  staged/unstaged changes and you want anti-patterns caught before merge.
---

# Reviewing C# changes

Review changed C# code in dotnet/docker-tools for high-confidence violations of repository
conventions. Read `.github/instructions/csharp.instructions.md` first and treat it as the
source of truth for implementation guidance.

## Scope

- Review the C# diff and behavior directly affected by it. Do not report unrelated,
  pre-existing code.
- Report only issues with clear maintenance, correctness, or operational impact. Skip
  subjective style preferences and uncertain findings.
- Do not edit code.
- Use the exemplars in `.github/instructions/csharp.instructions.md` when recommending a
  repository-specific pattern.

## Review checks

### Language and API design

- New files use file-scoped namespaces and do not disable nullable reference types.
- New code is null-annotation clean. Flag `!` when it masks a nullable-correctness problem, but
  not deliberate negative tests such as passing `null!` to verify a guard.
- Use the least accessibility needed; do not introduce public APIs without a demonstrated
  caller.
- Reuse existing methods and services. Flag unused or speculative methods, parameters, and
  abstractions.

### Async

- Async methods end with `Async`; async calls are awaited rather than discarded.
- Thread a `CancellationToken` through async I/O.
- Do not add `async`/`await` when directly returning the task preserves behavior.
- Do not add `ConfigureAwait(...)`; this repository does not use it.

### Control flow and failures

- Use LINQ for transformations and loops for side effects or async work.
- Avoid multiple enumeration when it can change behavior or repeat meaningful work.
- Return empty collections rather than using `null` as control flow.
- Flag broad catches, success-shaped fallbacks, or swallowed failures without an explicit
  recovery path.

### Tests

- Use MSTest attributes, Shouldly for new assertions, and Moq for mocks.
- Tests must run in any order and in parallel; no shared mutable state or ordering assumptions.
- Reuse test helpers under `src/ImageBuilder.Tests/Helpers/`. Avoid disk I/O when an in-memory
  alternative or mock tests the same behavior.
- Do not add comments that only label Arrange, Act, or Assert sections.

### Documentation and comments

- New public or internal APIs have XML documentation where their contract is not obvious.
  Primary XML doc comments go directly on interfaces. Implementations use `<inheritdoc/>`.
- Comments should explain the intention behind code; do not accept comments that merely restate
  the code.
- Comments identify ambiguous input requirements such as fully qualified vs. relative
  paths, string formats, etc. Include examples where appropriate.

## Output format

Group findings by file. For each finding, return:

`path:line - <rule violated> - <concrete impact> - <suggested fix>`

End with `N issues to address` or `No convention violations found`.