review-ef-core-provider

name	review-ef-core-provider
description	Fan-out review of the current branch, an external PR, or a branch in another clone — runs each per-area reviewer over the files it owns and aggregates findings. With --iterate, alternates review/fix passes until two consecutive clean reviews or the iteration cap is reached.
argument-hint	[--all] [--model opus|sonnet|haiku] [--iterate [--max-iterations N]] [<PR#> | <clone-path> [<base-ref> [<head-ref>]] | <base-ref> [<head-ref>]]
allowed-tools	Bash, Read, Glob, Grep, Write, Agent

/review-ef-core-provider

Run the per-area reviewer sub-agents (.claude/agents/*-reviewer.md) over a diff range in parallel and produce one consolidated report. Three modes:

• Local range mode — diff the current repo's branch.
• External PR mode — fetch a GitHub PR and diff against its base.
• External clone mode — diff a branch in a different clone of this repo, while running with the current clone's reviewer briefs and AGENTS.md files. Useful when the branch being reviewed lacks the latest agent/architecture updates that live in the current clone.

User args: $ARGUMENTS

Step 1 — Determine scope

Parse $ARGUMENTS:

• If --all is present, queue every reviewer in the tables below regardless of diff. Skip step 2's filtering.

• If --model <name> is present, capture <name> (must be one of opus, sonnet, haiku) and pass it on every Agent dispatch in step 3. If absent, omit the model parameter so each reviewer falls back to its frontmatter setting (currently inherit for all of them, which means the parent session's model).

• If --iterate is present, enable the iteration loop described in Step 5. It is only valid in local range mode and external clone mode; if combined with external PR mode (a <PR#> token), stop immediately and tell the user that --iterate cannot be used with external PRs (we don't push fixes back to PR branches). If --max-iterations <N> is also present, capture <N> as the cap (must be a positive integer, ≤ 26 because file letters run a–z); default to 10 if absent. --max-iterations without --iterate is an error — tell the user and stop.

• Examine the remaining non-flag tokens in priority order: first try external PR mode, then external clone mode, then local range mode.

  External PR mode — if the first non-flag token looks like a PR number (a bare integer such as 297, or a #-prefixed integer such as #297), treat this as an external PR review:

  1. Run gh pr view <PR#> --json number,title,url,baseRefName,headRefOid and capture the JSON.
  2. Parse <owner>/<repo> from the PR URL (e.g. https://github.com/mongodb/mongo-efcore-provider/pull/297 → mongodb/mongo-efcore-provider).
  3. Run git remote -v and find the remote whose fetch URL contains <owner>/<repo> (case-insensitive; works for both HTTPS and SSH URLs). If multiple remotes match, pick the first. If none match, fall back to trying upstream then origin.
  4. Run git fetch <remote> refs/pull/<PR#>/head to bring the PR's head commit into FETCH_HEAD.
  5. Capture the head SHA: git rev-parse FETCH_HEAD.
  6. Ensure the base branch is locally available: git fetch <remote> <baseRefName> (safe even if already present). The tracking ref will be <remote>/<baseRefName>.
  7. Set base ref = <remote>/<baseRefName> and head ref = the SHA from step 5.
  8. Set diff-repo = the absolute path of the current repo (git rev-parse --show-toplevel).
  9. Record the PR metadata (number, title, URL) and the remote name for display in the step 4 report header.

  External clone mode — otherwise, if the first non-flag token resolves to an existing directory (test -d "<token>"), treat it as the path to another clone of this repo and review the branch checked out there:

  1. Capture an absolute path: <clone> = $(cd "<token>" && pwd) (or realpath). All subsequent git commands and file paths must use this absolute form.
  2. Confirm it's a git repo by running git -C "<clone>" rev-parse --show-toplevel. If that fails, stop and tell the user that <clone> is not a git checkout.
  3. Capture a head label for display and filename use: git -C "<clone>" rev-parse --abbrev-ref HEAD. If it returns the literal HEAD (detached), fall back to git -C "<clone>" rev-parse --short HEAD.
  4. Collect the remaining non-flag tokens (after the path) in order:
     • First → base ref (default: main)
     • Second → head ref (default: HEAD)
  5. Set diff-repo = <clone>. Every git command in the rest of this skill must be run with git -C "<diff-repo>" …, and every file path passed to reviewers must be absolute (<diff-repo>/<repo-relative-path>). The parent agent's own working directory stays in the current clone so that AGENTS.md files and reviewer briefs continue to load from there — that's the point of this mode.

  Local range mode — otherwise, the remaining non-flag tokens are refs in the current repo. Collect them in order:

  • First token → base ref (default: main)
  • Second token → head ref (default: HEAD)
  • Set diff-repo = the absolute path of the current repo (git rev-parse --show-toplevel).

Use <base>...<head> as the diff range throughout (three-dot syntax finds the merge base). Examples:

• /review-ef-core-provider → main...HEAD in the current repo
• /review-ef-core-provider 297 → external PR #297 (fetches and diffs against its base branch)
• /review-ef-core-provider #297 → same as above
• /review-ef-core-provider HEAD~3 → HEAD~3...HEAD in the current repo (last 3 commits only)
• /review-ef-core-provider abc123 def456 → abc123...def456 in the current repo (arbitrary commit range)
• /review-ef-core-provider --all HEAD~5 HEAD~2 → all reviewers, commits HEAD5 through HEAD2 in the current repo
• /review-ef-core-provider ~/code/efcore-pr → review the current branch of the clone at ~/code/efcore-pr against its main
• /review-ef-core-provider ~/code/efcore-pr release/10.0 → review that clone's HEAD against its release/10.0
• /review-ef-core-provider ~/code/efcore-pr origin/main feature-x → review feature-x in that clone against origin/main
• /review-ef-core-provider --iterate ~/code/efcore-pr → review the clone's branch, then loop review → fix → review until two consecutive clean passes (or 10 iterations)
• /review-ef-core-provider --iterate --max-iterations 5 ~/code/efcore-pr → same but cap at 5 iterations
• /review-ef-core-provider --iterate → iterate on the current repo's branch against main

If --all was not passed, run git -C "<diff-repo>" diff --name-only <base>...<head>. If the result is empty, stop and tell the user the range has no changes — do not dispatch reviewers. In iterate mode this end-of-range check is performed at the start of every iteration; a mid-loop empty range means the latest fixer commit reverted all changes (unusual — flag it to the user and stop).

Step 2 — Map changed files → reviewers

Match each changed file against the table. A file may match more than one reviewer; queue all matches. Track files that match nothing — they go in an "Unmapped changes" section of the final report so coverage gaps are visible.

Reviewer	Path patterns
query-reviewer	src/MongoDB.EntityFrameworkCore/Query/**
storage-reviewer	src/MongoDB.EntityFrameworkCore/Storage/**
metadata-reviewer	src/MongoDB.EntityFrameworkCore/Metadata/**
serialization-reviewer	src/MongoDB.EntityFrameworkCore/Serializers/**; src/MongoDB.EntityFrameworkCore/ChangeTracking/**
public-api-reviewer	src/MongoDB.EntityFrameworkCore/Extensions/**; src/MongoDB.EntityFrameworkCore/Infrastructure/**; src/MongoDB.EntityFrameworkCore/Design/**
diagnostics-reviewer	src/MongoDB.EntityFrameworkCore/Diagnostics/**
value-generation-reviewer	src/MongoDB.EntityFrameworkCore/ValueGeneration/**; src/MongoDB.EntityFrameworkCore/Metadata/Conventions/MongoValueGenerationConvention.cs
spec-conformance-reviewer	tests/MongoDB.EntityFrameworkCore.SpecificationTests/**; tests/MongoDB.EntityFrameworkCore.FunctionalTests/Utilities/**; tests/MongoDB.EntityFrameworkCore.FunctionalTests/Usings.cs
vector-search-reviewer	files matching VectorIndex*, VectorSearch*, BinaryVector*, or VectorSimilarity* across src/ and tests/; src/MongoDB.EntityFrameworkCore/Metadata/VectorIndexOptions.cs; src/MongoDB.EntityFrameworkCore/Metadata/VectorIndexBuilder.cs; vector-search helpers in MongoDatabaseFacadeExtensions, MongoQueryableExtensions, MongoIndexBuilderExtensions, MongoPropertyBuilderExtensions
encryption-reviewer	src/MongoDB.EntityFrameworkCore/CryptProvider.cs; src/MongoDB.EntityFrameworkCore/QueryableEncryptionType.cs; files matching QueryableEncryption* across src/ and tests/; encryption properties on MongoOptionsExtension (KeyVaultNamespace, KmsProviders, CryptExtraOptions, CryptProvider*); tests/MongoDB.EntityFrameworkCore.FunctionalTests/Encryption/**

Test-folder mapping: per-area test folders mirror the src/ layout — tests/.../FunctionalTests/Query/, Storage/, Update/, Metadata/, Mapping/, Serialization/, ValueGeneration/, Design/ — and route to the corresponding src/ area reviewer. The Encryption/ test folder routes to encryption-reviewer, and the test utilities at tests/.../FunctionalTests/Utilities/ route to spec-conformance-reviewer. SpecificationTests/<Area>/ test folders route to both the matching area reviewer and spec-conformance-reviewer (the area owns the assertion logic; spec-conformance owns the fixture / inheritance discipline).

Meta-mapping for reviewer definitions: a change to .claude/agents/<name>-reviewer.md is reviewed by that same reviewer (e.g. .claude/agents/query-reviewer.md → query-reviewer). The reviewer is best placed to judge whether its own brief still accurately characterizes the area. Cross-cutting reviewer definitions map to themselves the same way. Exception: pr-summary-reviewer only runs in external PR mode, so a local-range or external-clone-mode change to .claude/agents/pr-summary-reviewer.md won't be self-reviewed — that file's review happens when the PR is run through /review-ef-core-provider <PR#>.

If new area reviewers are added under .claude/agents/, update this table.

Cross-cutting reviewers (always run)

These three reviewers run on every invocation of /review-ef-core-provider, regardless of which files changed. They look across the whole diff for one specific concern. They are additional to — not part of — the path-mapping table above.

Reviewer	Concern
api-stability-reviewer	Public-API / breaking changes (signatures, defaults, visibility, annotation keys, MongoEventId numbering, behavior changes on unchanged signatures)
ef-conformance-reviewer	EF Core integration correctness — multi-version (EF8/EF9/EF10) compat, service registration, annotation hygiene, build-vs-runtime model
security-reviewer	Credential exposure, sensitive-data logging gating, KMS plumbing leaks, TLS surfaces, connection-string redaction

PR-summary reviewer (external PR mode only)

In external PR mode, also dispatch the pr-summary-reviewer agent. It produces a holistic description of the PR (what it does, why) plus an opinion on whether it's a good change. It runs in parallel with everything else, and its output goes at the top of the consolidated report (before ## Summary). Skip it in local range mode and external clone mode — there is no PR body to read.

Step 3 — Dispatch reviewers in parallel

Critical: emit a single assistant message containing one Agent tool-use block per dispatched reviewer — the matched area + feature reviewers from step 2, all three cross-cutting reviewers, and (in external PR mode only) pr-summary-reviewer. Multiple Agent calls in the same message run concurrently; sequential calls do not. Use subagent_type: <reviewer-name> for each. If --model <name> was parsed in step 1, set model: <name> on every block; otherwise omit the field.

In every template below, substitute:

• <base>, <head> — the diff range refs.
• <diff-repo> — the absolute path captured in step 1 (the current repo for local-range / external-PR mode; the other clone for external-clone mode).
• File lists — always passed as absolute paths of the form <diff-repo>/<repo-relative-path> so reviewer Read calls land on the right tree regardless of mode. Don't pass bare repo-relative paths.

Area / feature reviewer prompt template

For each area or feature reviewer, use this template (substitute <base>, <head>, <diff-repo>, and the per-reviewer file list as absolute paths):

You are running as part of a multi-area branch review. The diff range is <base>...<head> in the repo at <diff-repo>.

Files in scope for this iteration that fall in your area (absolute paths):
- <abs-file1>
- <abs-file2>
…

[Iteration N > 1 only — omit this block on iteration 1]
This is iteration <N> of an `--iterate` run. The file list above is narrowed to files the previous fixer commit touched plus files that still carry an unresolved [blocking]/[substantive] finding. The following findings were tagged [fix-in-code][nit] in a previous iteration and have not been fixed; do NOT re-emit them unless the surrounding code has changed materially since the previous iteration:
- <reviewer>: <file>:<line> — <message>
- <reviewer>: <file>:<line> — <message>
…
[end Iteration N > 1 block]

Read those files at their current state. Run git commands with `git -C "<diff-repo>" …` (e.g. `git -C "<diff-repo>" diff <base>...<head> -- <repo-relative-path>`) — the parent agent's working directory may not be <diff-repo>. Pull in adjacent context only as needed to judge the change.

Produce a report in exactly this shape, no preamble:

**Verdict**: one of `approve`, `flag`, `escalate`.
- approve = no concerns
- flag = non-blocking suggestions or nits
- escalate = blocking concern that needs user attention before merge (public-API break, annotation-key rename, behavior change affecting stored documents, multi-EF break, spec-conformance regression, security regression)

**Findings**: bulleted list. Each bullet: `<file>:<line> — [fix-in-code|external-action][blocking|substantive|nit] <one-sentence problem> — <one-sentence fix or action>`.

Every finding carries two tags. The first tag says *who can act on it*:
- `[fix-in-code]` — the finding can be resolved by an in-tree code change (edit a file, add a test, fix a typo, tighten a comment, change a throw type, fix an `#if` branch, etc.) that the fixer agent can make mechanically without external information.
- `[external-action]` — the finding requires something outside this codebase: confirming a JIRA ticket exists, verifying CI matrix configuration, asking the user to confirm intent, auditing call sites in production code outside the diff, double-checking spec wording against an external source, "worth a quick test asserting…" suggestions where the test would require Atlas or encryption infrastructure you don't have, updating `BREAKING-CHANGES.md` wording on behalf of the user, or any other action the fixer agent cannot perform without leaving the repo.

When unsure between fix-in-code and external-action, prefer `[external-action]` — it surfaces the concern without claiming the fixer can address it.

The second tag says *how important it is*:
- `[blocking]` — should land an `escalate` verdict and stop the merge. Wire-format / public-API / multi-EF / spec-conformance / security breaks.
- `[substantive]` — real concern the fixer should address: wrong behavior, missing test for a non-trivial code path, broken `#if` branch, layering violation, lost cancellation propagation, etc.
- `[nit]` — mechanical cosmetic: unused import, comment/message typo, misnamed local, missing trailing newline, wording fix-up in a skip message. Does not affect behavior; safe to defer indefinitely.

When unsure between substantive and nit, prefer `substantive` (conservative — it just means the fixer will act on it). When unsure between blocking and substantive, prefer `blocking` if the change can corrupt stored data, break the public API contract, or pass on one EF version while failing on another.

Use repo-relative paths in findings (not absolute) so output is portable. Emit at most **5** findings per pass. If you have identified more than 5, sort by tag (blocking → substantive → nit) and drop the lowest-priority ones — do not pad the list with extra nits.

**Tests run**: list any `dotnet test --filter` you actually executed and pass/fail. If none, write `none`.

Hard limit: 400 words total. Do not summarize the diff back; the parent agent has it.

Cross-cutter prompt template

For each cross-cutting reviewer, use this template (substitute <base>, <head>, <diff-repo>, and the full changed-file list as absolute paths):

You are running as a cross-cutting reviewer in a multi-area branch review. The diff range is <base>...<head> in the repo at <diff-repo>.

Your concern is not scoped to a directory — it is a single hygiene lens applied across the diff. Files in scope for this iteration (absolute paths):
- <abs-file1>
- <abs-file2>
…

[Iteration N > 1 only — omit this block on iteration 1]
This is iteration <N> of an `--iterate` run. The file list above is narrowed to files the previous fixer commit touched plus files that still carry an unresolved [blocking]/[substantive] finding. The following findings were tagged [fix-in-code][nit] in a previous iteration and have not been fixed; do NOT re-emit them unless the surrounding code has changed materially since the previous iteration:
- <reviewer>: <file>:<line> — <message>
- <reviewer>: <file>:<line> — <message>
…
[end Iteration N > 1 block]

Use `git -C "<diff-repo>" diff <base>...<head>` to see the full picture and `git -C "<diff-repo>" diff <base>...<head> -- <repo-relative-path>` to focus. Read files at their current state where context matters. Skip files that are clearly irrelevant to your concern.

Produce a report in exactly the same shape as the area reviewers (Verdict / Findings / Tests run; same format and 400-word cap; same verdict semantics; repo-relative paths in findings; the same `[fix-in-code]` / `[external-action]` tag *and* the same `[blocking]` / `[substantive]` / `[nit]` severity tag on every finding; the same 5-finding cap; and the same carry-forward-nit rule in iteration N > 1). Findings must be specific to your concern — do not duplicate what an area reviewer would catch.

PR-summary prompt template (external PR mode only)

For pr-summary-reviewer, use this template (substitute the PR fields, <base>, <head>, <diff-repo>, and the full changed-file list as absolute paths):

You are running as the PR summary reviewer in a multi-area branch review of an external pull request.

PR: #<number> — <title>
URL: <url>
Diff range: <base>...<head> in the repo at <diff-repo>

All files changed in this range (absolute paths):
- <abs-file1>
- <abs-file2>
…

Pull the PR body yourself with `gh pr view <number> --json body,labels,additions,deletions,changedFiles,author` to get the author's stated rationale. Use `git -C "<diff-repo>" diff <base>...<head>` for the diff and read files at their current state where context matters.

Produce a report in exactly the shape specified in your agent definition (Description / Assessment / Verdict; 500-word cap). Do not duplicate the per-area or cross-cutting reviewers' work.

Step 4 — Aggregate

After all reviewers return, produce one consolidated report in this shape and show only this to the user (do not paste raw sub-agent transcripts).

For the report heading:

• External PR mode: # PR review: #<number> — <title> followed by the PR URL and diff range: <base>...<head> on separate lines.
• External clone mode: # Clone review: <head-label> in <clone> followed by diff range: <base>...<head> on a separate line. (<head-label> is the branch name or short SHA captured in step 1; <clone> is the absolute path.)
• Local range mode: # Branch review: <base>...<head>

# <heading from above>

## PR summary
(External PR mode only — paste the `pr-summary-reviewer`'s Description, Assessment, and Verdict verbatim. Omit this section in local range mode and external clone mode.)

## Summary
N reviewers ran (M area+feature + 3 cross-cutting [+ 1 PR summary in external PR mode]). X approved, Y flagged, Z escalated. (PR-summary verdict is not counted in those totals — it's a separate lens.)

## Escalations
For each `escalate` verdict from any reviewer — reviewer name, then its findings verbatim. Omit section if none.

## Cross-cutting findings
Group by reviewer (`api-stability-reviewer`, `ef-conformance-reviewer`, `security-reviewer`). For each: verdict + findings as bullets. Always show this section, even if all three approved (in which case list each as `<reviewer> — clean`).

## Area + feature findings
For each area or feature reviewer that ran: verdict + findings as bullets if `flag`, or `<reviewer> — clean` if `approve`. (Escalations are already covered above.)

## Unmapped changes
Files from the diff that didn't match any area or feature reviewer. (Cross-cutters cover the diff regardless, so this is purely a coverage signal for the area mapping.) Omit section if none.

Save to file — after composing the report, determine the output filename. The save location is always the parent agent's current working directory (the current clone), never <diff-repo> when those differ.

• External PR mode: stem is review<number> (e.g. review297). Always saved.
• External clone mode: stem is review-<sanitized-head-label>, where the head label is the branch name or short SHA captured in step 1 with every character that is not [A-Za-z0-9._-] replaced by - (e.g. branch EF-308a → review-EF-308a; branch feature/foo → review-feature-foo). Always saved.
• Local range mode: in single-pass mode, do not save (the report only appears inline). In iterate mode, save using stem review-<sanitized-head-label> derived the same way from git rev-parse --abbrev-ref HEAD of the current repo — each iteration needs a persistent artifact.

Then:

1. List files in the current directory matching <stem>[a-z].md.
2. Find the lowest letter (a–z) not already taken; use a if none exist yet.
3. Write the full report to <stem><letter>.md in the current directory using the Write tool.
4. In iterate mode, also prefix the report's ## Summary section with a line Iteration <N> of <max>. so each file is self-identifying even if letters are non-contiguous.
5. Tell the user the filename at the end of your response (one line, e.g. Saved to review297a.md or Saved to review-EF-308a.md).

When --iterate is set, after step 5 do not end your response — continue to Step 5 below. Otherwise stop after the file-saved line.

Step 5 — Iterate (only when --iterate is set)

The loop alternates review and fix passes in <diff-repo> until two consecutive clean reviews or the iteration cap is hit. Mode prerequisite: only local-range and external-clone modes — external PR mode was already rejected in step 1.

Before the first fixer dispatch, emit a single user-visible sentence announcing the loop, the cap, and the diff-repo (so the user can interrupt if this isn't what they wanted). Example: Iterating in /Users/foo/clone (max 10). Each non-clean iteration will commit fixes to that branch.

Clean criteria

An iteration's aggregated report is clean when all of these hold:

1. No escalate verdicts from any reviewer.
2. No [fix-in-code][blocking] or [fix-in-code][substantive] findings. Severity is the reviewer's call at emit time (see the rubric in Step 3) — the parent does not re-classify. [fix-in-code][nit] findings do not block convergence: they're surfaced in the report and carried forward to the next iteration's reviewers (so they don't get re-emitted on unchanged code) but the fixer is told to skip them in normal operation.
3. [external-action] findings (any severity) are ignored for this convergence check. They're real concerns that should be surfaced in every iteration's report (and in the closing summary so the user can act on them outside the loop), but they cannot be mechanically addressed by the fixer — counting them would prevent any loop from terminating. JIRA-ticket existence, CI-matrix verification, "audit callers outside this diff", "confirm spec source", BREAKING-CHANGES.md wording confirmation, and similar requests all fall here.

State your clean/not-clean call explicitly when you announce the iteration result, with a one-line reason: Iteration <N>: clean — only [nit] findings remain (unused import, message typo); 2 [external-action] notes carried forward. or Iteration <N>: not clean — [fix-in-code][substantive]: missing #if EF10 branch weakens multi-EF compat.

Loop shape

consecutive_clean = 0
narrowed_files = None          # iter 1 uses full <base>...<head>; iters >1 use the narrowed set
carry_forward_nits = []        # list of "<reviewer>: <file>:<line> — <message>" strings tagged [nit] in the prior iter
for iter in 1..max_iter:
    review_report = run_steps_1_through_4(
        iter, max_iter,
        file_filter = narrowed_files,         # None means "all files in <base>...<head>"
        carry_forward_nits = carry_forward_nits)
    save(review_report)                       # the file from Step 4
    if is_clean(review_report):
        consecutive_clean += 1
        if consecutive_clean >= 2:
            announce_success(iter); stop
    else:
        consecutive_clean = 0
    if iter == max_iter:
        announce_max_hit(iter); stop
    fixer_result = dispatch_fixer(review_report, iter, max_iter)
    if fixer_result.failed:
        announce_fixer_failure(iter); stop
    if fixer_result.no_op:
        announce_no_op_stop(iter); stop       # no code changed → re-reviewing the same tree would just re-roll LLM dice
    # Prepare narrowed scope for the next iteration:
    touched = git -C "<diff-repo>" diff --name-only HEAD~1 HEAD     # files the fixer just changed
    open_files = files mentioned in [blocking]/[substantive] findings still in the current report
    narrowed_files = union(touched, open_files)
    carry_forward_nits = every [fix-in-code][nit] finding in the current report
    # next iter will re-resolve HEAD in <diff-repo> via git rev-parse, picking up the new commit

Iteration 1 vs later iterations. Iteration 1 dispatches reviewers against the full diff (git -C "<diff-repo>" diff --name-only <base>...<head>) so the initial pass is exhaustive. From iteration 2 onward, the file set passed into Step 2 is restricted to narrowed_files — the union of files the previous fixer commit touched and files still carrying an unresolved [blocking] or [substantive] finding. This is the single biggest leverage on iteration count: stale [nit] findings on files no one touched stop regenerating, and reviewers focus on what actually changed. Cross-cutters use the same narrowed set — there is no reason to re-scan unchanged code with the same cross-cutting lens iteration after iteration.

Carry-forward of [nit] findings. Every [fix-in-code][nit] finding in iteration N is captured and passed back to the reviewers' dispatch prompts in iteration N+1 as a do-not-re-emit note (see the carry-forward block in the area-reviewer template). The reviewer should only re-emit a previously-nit finding if the surrounding code has changed materially since the previous iteration. This breaks the most common tail-chase pattern: the same nit regenerating every iteration on code no one is going to fix.

If narrowed_files ends up empty in iteration N (the fixer's commit reverted everything and no findings carried forward), treat it as a no-op-equivalent: stop with the no-actionable-findings outcome.

Re-running Steps 1–4 means re-running git -C "<diff-repo>" diff --name-only <base>...<head>, intersecting with narrowed_files if set, re-mapping files (the set may shift after fixes), and re-dispatching reviewers. The diff-repo, base ref, head label, and parsed flags don't need re-derivation — keep them from the initial parse.

fixer_result.no_op detection: the fixer prompt requires its final paragraph to include the literal string no-op (lowercase, exact) when it made no commit. Look for that marker in the fixer's return. Don't infer no-op from the absence of a commit SHA in the response — the marker is the contract.

Fixer agent dispatch

Use the general-purpose agent (it has full tools — Read/Write/Edit/Bash — which reviewers lack). One Agent block, foreground. Subagent prompt template:

You are the fix-applying agent in iteration <N> of <max> of an iterative code review of <diff-repo>.

Your job is to address the findings from the review report below by editing files in <diff-repo> and committing in that repo. Then stop and report what you did.

Rules:
1. Every finding carries two tags: `[fix-in-code|external-action]` (who can act) and `[blocking|substantive|nit]` (how important). Apply every `[fix-in-code][blocking]` and `[fix-in-code][substantive]` finding — those came from a specialised reviewer that knows the area; trust the diagnosis, but read the surrounding code before changing it. Skip every `[external-action]` finding outright regardless of severity — those require something outside this codebase (JIRA, CI config, audits, spec lookups, `BREAKING-CHANGES.md` wording) that you can't perform; they will be surfaced to the user in the closing summary.
2. **Skip `[fix-in-code][nit]` findings by default.** The reviewer classified them as mechanical cosmetic, not behavior-affecting; the loop tolerates them indefinitely. Exception: if you are already editing a given file for a substantive finding, *and* a nit in that same file is trivially mechanical (one-line edit, no judgment required), you may fix it in passing. Never go out of your way to fix a nit — and never fix a nit in a file you would not otherwise be touching this iteration.
3. Do not change behavior beyond what the findings demand. No refactoring, no drive-by cleanups, no scope expansion, no unrelated formatting passes.
4. Respect the codebase guidance. The parent agent's working directory has up-to-date `AGENTS.md` files — read them as needed via relative paths. In particular: preserve file BOMs; `<Nullable>enable</Nullable>` is on in `src/` — annotate new types accordingly; library code uses `ConfigureAwait(false)`; `CancellationToken` flows through unchanged; multi-EF code uses `#if EF8 / EF9 / EF10` define constants — a fix that compiles on one EF version must compile on all three. The diff-repo at <diff-repo> may have *older* `AGENTS.md` files — prefer the up-to-date ones from the parent agent's cwd when they conflict.
5. All edits land in <diff-repo>. Use absolute paths under <diff-repo> when calling Edit/Write, and `git -C "<diff-repo>" …` for every git command.
6. When done editing, run `git -C "<diff-repo>" status --short`. If there are no changes, do not commit — report `no-op` and stop. Otherwise run `git -C "<diff-repo>" add -A` then `git -C "<diff-repo>" commit -m "[review-iter <N>] Address review findings"` (do NOT include any AI-attribution trailers; do NOT push; do NOT amend any previous commit).
7. If a `[fix-in-code]` finding turns out to be unactionable on closer inspection (the reviewer mis-tagged something that really requires user judgment, the area is unfamiliar enough you'd be guessing, etc.), skip it and note that in your final report. If every finding ends up skipped — fine, that's the expected exit signal: end with the `no-op` marker and the parent loop will stop cleanly. Don't invent makework fixes just to produce a commit.

Target diff-repo (for ALL edits and git commands): <diff-repo>

The review report you must address is below verbatim. Read carefully, then apply.

----- BEGIN REVIEW REPORT -----
<paste the full aggregated report from Step 4 here>
----- END REVIEW REPORT -----

Final output (one short paragraph): what you changed, what you skipped and why, the new commit SHA (or `no-op`), and any unexpected obstacles. Cap 200 words.

After the loop

Emit a single closing summary to the user with:

• Outcome: clean (stopped after two consecutive clean reviews), max iterations reached, fixer failed at iteration <N>, or no actionable findings remaining at iteration <N> (the fixer judged that nothing in the latest report could be addressed in code — the only remaining concerns are [external-action]).
• Number of iterations actually run.
• Path to the final report file.
• The final HEAD SHA in <diff-repo> so the user can inspect commits (git -C <diff-repo> log <base>..HEAD --oneline).
• A short bulleted list of every [external-action] finding still present in the final iteration's report, with its reviewer name and the action requested. These need handling outside the loop.

Do not paste the full report transcripts again — they're on disk.

Notes

• Reviewers are non-mutating with respect to the source tree: each one is configured with tools: Read, Grep, Glob, Bash and has no Edit / Write / Patch tool, so they cannot modify files. They can still execute shell commands via Bash (e.g. dotnet test, git diff); scope reviewer Bash usage to inspection commands and treat any suggested fix as something the parent applies, not the reviewer.
• A diff against a feature branch's own base is the typical use; pass an explicit <base-ref> for non-main cases (e.g. /review-ef-core-provider release/10.0). Pass a second positional arg to cap the end of the range (e.g. /review-ef-core-provider HEAD~5 HEAD~1).
• In external PR mode, the remote is inferred from the PR's repo URL, so PRs on forks (upstream, origin, or any named remote) are handled automatically without any extra flags.
• In external clone mode, the parent agent stays in the current clone so that AGENTS.md files, reviewer briefs, and any other repo guidance load from here, while the source code being reviewed and all git history come from <clone>. This is the whole point of the mode: review code on a branch that doesn't yet carry the latest agent/architecture updates, using the up-to-date briefs from the current clone. The pr-summary-reviewer does not run in this mode (no PR body to fetch); if you also want a holistic PR summary, run /review-ef-core-provider <PR#> separately.
• --iterate makes commits in <diff-repo> (one per non-clean iteration). It never pushes. The user can inspect with git -C <diff-repo> log <base>..HEAD afterward and amend / squash / drop commits as they see fit. Severity classification ([blocking] / [substantive] / [nit]) is the reviewer's call at emit time, not the parent's call after the fact — reviewers see the rubric in their dispatch prompt and pick. The loop terminates as soon as no [fix-in-code][blocking] or [fix-in-code][substantive] findings remain ([nit] and [external-action] are carried forward, not counted). If a real substantive concern is leaking through tagged as [nit], the answer is to fix the reviewer brief or surface it to the user — not to tighten the convergence rule.
• The loop has two convergence guards beyond the max-iterations cap. [external-action] findings don't block convergence: reviewers tag any finding that requires looking up a JIRA ticket, verifying CI config, auditing call sites outside the diff, updating BREAKING-CHANGES.md wording, or otherwise leaving the codebase, and those tags exclude the finding from the clean check. The user still sees them — they're listed in every iteration's report and in the closing summary — but the fixer can't address them so they shouldn't pin the loop open. A no-op fixer ends the loop: if the fixer judges that nothing in the current report is mechanically actionable, the next iteration would just be re-rolling LLM verdicts on the same tree, so the loop stops with outcome no actionable findings remaining and surfaces the outstanding [external-action] items.
• Test-running by reviewers is opportunistic — most won't, given that the EF Core provider has three build configurations and a dotnet test over a real database is slow. The /test-all skill is the right tool when a multi-EF build is genuinely needed.