Add a staged expert /review command for pull requests - #68867
Draft
PureWeen wants to merge 19 commits into
Draft
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The workflow’s safety/documentation comments are currently inconsistent with the generated workflow behavior, which can mislead future audits and maintenance.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
.github/workflows/pull-request-review.md — The comment claims the activation job sparse-checks out only .github, but the compiled workflow… |
What changed in this PR
Adds a staged, maintainer-invoked /review slash-command workflow for ASP.NET Core pull requests, plus a locally runnable review-pull-request skill and domain reference corpus to keep hosted and local review criteria consistent.
Changes:
- Introduces a new read-only (staged) Agentic Workflow that listens for the
reviewslash command and renders review output to the workflow summary. - Adds the
review-pull-requestskill contract that freezes PR evidence, routes to at most two domain references plus cross-cutting, validates findings, and outputs a structured result. - Adds domain reference documents (cross-cutting + area-specific) used by the workflow’s inline reviewer agents and by local runs of the skill.
| File | Description |
|---|---|
| .github/workflows/pull-request-review.md | New staged /review command workflow definition, including routing logic and inline reviewer agent blocks. |
| .github/workflows/pull-request-review.lock.yml | Generated workflow YAML produced by gh aw compile for the staged /review workflow. |
| .github/skills/review-pull-request/SKILL.md | New local skill contract describing evidence freezing, routing, validation gates, and output format. |
| .github/skills/review-pull-request/references/auth-security-reviewer.md | Auth/security domain review checklist and constraints used when routing security-related changes. |
| .github/skills/review-pull-request/references/blazor-components-reviewer.md | Blazor/Components domain review checklist and constraints used when routing Components changes. |
| .github/skills/review-pull-request/references/cross-cutting-reviewer.md | Cross-cutting review checklist applied to every routed review. |
| .github/skills/review-pull-request/references/grpc-reviewer.md | gRPC integration domain review checklist and constraints. |
| .github/skills/review-pull-request/references/hosting-di-reviewer.md | Hosting/DI domain review checklist and constraints. |
| .github/skills/review-pull-request/references/minimal-api-openapi-reviewer.md | Minimal API/OpenAPI domain review checklist and constraints. |
| .github/skills/review-pull-request/references/mvc-razor-routing-reviewer.md | MVC/Razor/routing domain review checklist and constraints. |
| .github/skills/review-pull-request/references/native-interop-reviewer.md | Native IIS interop domain review checklist and constraints. |
| .github/skills/review-pull-request/references/servers-networking-reviewer.md | Servers/networking domain review checklist and constraints. |
| .github/skills/review-pull-request/references/signalr-reviewer.md | SignalR domain review checklist and constraints. |
Suppressed comments (1)
.github/workflows/pull-request-review.md:118
- This comment says the workspace is empty because
checkout: false, but the generated workflow restores.github/skills/agents from the activation artifact into the workspace. The key safety property is “no pull request head checkout + no credentials”, so the wording should avoid asserting an empty workspace.
# To be precise rather than reassuring: the compiler still grants the agent a `write` tool inside
# the sandbox container. That is not a path back to this repository — the workspace is empty
# because `checkout: false`, credentials are excluded from the container, and no safe output can
# commit or push. It does mean "read-only" describes this workflow's effect on GitHub, not an
# absence of any filesystem capability in the sandbox.
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+89
to
+92
| # The inline reviewer agents below single-source their bodies from the skill's `references/` | ||
| # via `{{#runtime-import}}`. Those macros resolve in the activation job, which sparse-checks-out | ||
| # `.github` (only) from this workflow's own ref — never the contributor head — before any pull | ||
| # request checkout, so this is unaffected by `checkout: false`. |
Adds a focused, read-only pull request reviewer that a maintainer invokes by typing `/review` on a pull request comment or review comment. `.github/skills/review-pull-request/SKILL.md` is the analysis contract, usable both by the hosted workflow and interactively in Copilot CLI. It freezes the exact PR head SHA and the GitHub-authoritative changed-file list and diff, reviews only changed lines while reading unchanged callers/producers/consumers for context, treats all PR text and code as untrusted evidence, and requires every finding to cite a changed line, a concrete trigger, a material consequence, and source or primary-contract evidence. It caps output at five findings, always reports a test-boundary assessment (false-pass risk, ownership, coverage), and returns NO_FINDINGS when nothing survives validation. `.github/workflows/pr-review.md` wires that contract into gh-aw. The agent job is read-only and, notably, sets `checkout: false` so the generated "Checkout PR branch" step is never emitted: community and fork PR code is untrusted, so it is never placed on disk or executed. Shell, edit, and network tools are all disabled; reads go through narrowly scoped read-only GitHub toolsets. `min-integrity: none` is required to read exactly the fork PRs this workflow exists to review, and is compensated by those controls plus capped safe outputs. Safe outputs are limited to at most five inline review comments and exactly one review restricted to `allowed-events: [COMMENT]`, so the workflow can never APPROVE or REQUEST_CHANGES and therefore can never gate or unblock a merge. Failure-as-issue reporting is disabled and `staged: true` is set so initial runs preview writes instead of posting until maintainers deliberately remove it. Because inline comments are posted by a later job and cannot be pinned to a commit, the prompt re-checks the head SHA immediately before emitting and suppresses inline comments if the author pushed mid-review. The workflow follows the existing repository PAT-pool convention and is compiled with gh aw v0.86.2; the generated lock file and the centralized command router are committed as produced by the compiler. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
Replaces the synthesized single-contract reviewer with one built from
reviewed domain-expert prompts, routed by changed path.
The skill is renamed to `review-pull-request-by-area`, which is
verb-first like the repository's other skills and signals that routing
is the point. Its description activates only on an explicit ASP.NET Core
pull request or diff review, and explicitly excludes implementing or
fixing, writing or running tests, CI/build investigation, issue triage,
API-only proposal review, and general coding assistance.
Ten domain reviewer bodies (servers/networking, MVC/Razor/routing,
Blazor/Components, SignalR, auth/security, hosting/DI, minimal
APIs/OpenAPI, gRPC, native interop, and always-on cross-cutting) ship as
inert files under the skill's `references/`. They are lazy-loaded, so
they support local use without adding globally discoverable agents. The
skill routes to cross-cutting plus at most two matching domains and
reads only those.
The hosted workflow is renamed `aspnetcore-pr-review.md` and installs
only this skill. Its ten inline agent wrappers single-source their
bodies from the same `references/` via `{{#runtime-import}}`, so hosted
and local review cannot drift. Those macros resolve in the activation
job, which sparse-checks-out `.github` alone from the workflow's own ref
— never the contributor head — so single-sourcing works with
`checkout: false` and the agent job still gets no working tree.
Two interpolation invariants are load-bearing. Every `## agent:` block
is closed with a matching `## end agent:` marker, because a mismatched
marker fails compilation but a missing one compiles silently and
truncates the agent body at the next `##` heading. Reference headings
are demoted below level 2 and contain no `${{ }}` expressions so an
imported body can neither terminate its block nor inject an expression.
Domain content was corrected where it assumed code this repository does
not own or contradicted itself: the Razor parser, source generator,
scoped-CSS rewriting, and MSBuild/SDK integration are reviewed only
through the runtime contracts consumed here; `Dispatcher.InvokeAsync` is
scoped to renderer and circuit infrastructure; `StateHasChanged` is
limited to rendering an intermediate state between two awaits;
`src/Extensions` is owned by servers/networking rather than hosting/DI;
and cross-cutting no longer describes itself as a fallback. Added
Components rules: no `IHttpContextAccessor` in framework components or
services, never implement `IHandleEvent`, no new Components dependencies
by default, and render-mode-agnostic service registration.
Security posture is unchanged and re-verified against the generated
lock: trusted roles only, fork guard, PR-scoped concurrency, read-only
agent permissions, no shell, narrowly scoped GitHub tools, capped
COMMENT-only safe outputs that can never approve or request changes,
staged mode, and no checkout of pull request code. Added a fail-closed
bound so a pull request over 75 files or 3000 changed lines is declined
rather than partially reviewed, and a rule against emitting slash
commands or mentions derived from untrusted pull request text.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
Six corrections from adversarial review of the generated output. Staged mode suppressed safe outputs but not the centralized router's own writes: the generated router carried `reaction=eyes` and `status_comment: true`, so a "preview" run still reacted to the comment and posted an activation comment. Setting `reaction: none` and `status-comment: false` removes both, and drops the activation job from `issues: write, pull-requests: write` to `contents: read, actions: read`. gh-aw auto-enables incomplete reporting whenever any safe output exists, so the compiled config carried `create_report_incomplete_issue` and `report_incomplete` handlers, and `report-failed-jobs` defaulted on for the imported PAT-pool job. Both could file issues, contradicting the promise of no issue mutation. With `report-incomplete: false` and `report-failed-jobs: false` no issue-creating handler remains and no job in the workflow holds `issues: write`. The review body hardcoded `independence: single-orchestrator`, which is false whenever the routed reviewer agents actually run as subagents. The prompt now reports the real topology and only claims single-orchestrator when subagents were genuinely unavailable. The skill advertised reviewing a pull request "or diff" while every step requires a GitHub head SHA, authoritative file list, diff, and existing feedback, which a bare local diff cannot supply. It is now scoped to an identified GitHub pull request, and says so and stops otherwise. The single-sourcing claim was too strong. Only the domain reference bodies are shared between hosted and local review; the surrounding routing, validation, and publication logic is stated separately in each place and can diverge. The comment now says that. The generated router listens to `edited` comment events by compiler design and its event types cannot be narrowed from the workflow, so editing a comment containing the command re-triggers it. This is documented as a residual limitation: it is not privilege escalation because the role gate is evaluated against the event sender, and re-runs are bounded by PR-scoped concurrency and the per-user rate limit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
Centralized routing enables a builtin `/help` handler by default. It intercepts `/help` and posts a comment listing available commands, which is a router write that `safe-outputs.staged` does not suppress — so the claim that a staged run is side-effect-free was not exact. Disabled repo-wide through the documented project config `.github/workflows/aw.json` with `"help_command": false`. The generated router now carries `GH_AW_HELP_COMMAND_ENABLED: 'false'`. With reaction, status comment, and help all off, the only write this command causes through the router is the `workflow_dispatch` itself. Disabling help did not shrink the shared router job's permissions: it still statically declares `issues: write` and `pull-requests: write`, which is compiler-controlled for `agentic_commands.yml` and not exercised by this route. The source comment now says exactly that rather than implying the workflow removes those permissions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
The imported domain references cover code review, but several changed
paths in this repository have an authoritative document that states the
contract the change has to satisfy. Without them the reviewer can flag a
violation without being able to cite why it is one.
Folded them into the skill's routing step as conditional pointers rather
than adding file-scoped instruction shims, so nothing new becomes
globally discoverable:
src/Components/**/*.min.js -> UpdatingMinifiedJsFiles
**/*.{csproj,props,targets} -> ProjectProperties, AddingNewProjects,
SharedFramework, tooling-consolidation
**/PublicAPI.{Shipped,Unshipped}.txt -> APIBaselines
.gitmodules, src/submodules/** -> Submodules
Kestrel WebTransport + samples -> WebTransport
They are read only when the frozen changed-file list matches, at the
base ref, and the relevant contract facts are carried into the routed
reviewers' briefing. Reading them for unrelated changes is called out as
dilution.
The documents are evidence, not instructions: they establish what the
repository's contract is so a finding can cite it, and explicitly cannot
authorize posting, approving, executing pull request code, or relaxing
any prohibition. If a document appears to conflict with a prohibition,
the prohibition wins. `PublicAPI.*.txt` is noted as tracking
compatibility without constituting API approval, which stays
human-owned.
The hosted prompt mirrors only the delta it needs: that job has no
working tree, so it reads any matching document through the GitHub tools
at an explicit base ref rather than from disk.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
`review-pull-request-by-area` was mechanically accurate but generic enough to look applicable outside this repository and to collide with a future general-purpose review skill. The concern here is repository-wide skill pollution and false activation, so the name should make the repository, the artifact, and the action unambiguous on sight. `review-aspnetcore-pull-request` is verb-first like the repository's other skills and states both the domain and the artifact. The by-area routing is still explained by the description and body, where it belongs, rather than in the identifier. Semantic rename only: directory, frontmatter name, the ten inline agent runtime-import paths, the `skills:` install path, and prose references. Generated files were regenerated with `gh aw compile --strict`, never hand-edited. The hosted workflow stays `aspnetcore-pr-review` and the maintainer command stays `/review`. The narrow activation contract is unchanged and re-verified: an identified GitHub pull request in this repository only, with bare local diffs, implementation and fixing, writing or running tests, CI and build investigation, issue triage, API-proposal-only review, and other repositories all still excluded. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
Perimeter changes only. No domain-review criteria, routing, or bounds
were altered.
Renames the skill to `review-pull-request`. The directory sits under
this repository, so `aspnetcore` in the identifier was redundant. The
strict activation contract is unchanged: an identified pull request in
this repository only, with bare local diffs, implementation and fixing,
writing or running tests, CI and build investigation, issue triage,
API-proposal-only review, and other repositories all still excluded.
Switches the slash command from centralized routing to gh-aw's default
direct dispatch. The workflow now listens to the comment events itself,
which deletes the generated `agentic_commands.yml` and with it the
router's own writes: its reaction, its activation comment, and its
builtin `/help` comment handler. `aw.json` existed only to disable that
help handler and is removed too. No shared job holds `issues: write` on
this command's behalf any more.
Direct dispatch means GitHub delivers every comment event, so the
compiled guards matter. A generated job-level predicate requires the
body to match `/review` and the item to be a pull request; `pre_activation`
then resolves the event sender against `admin, maintainer, write` and
gates on `is_team_member && rate_limit_ok && command_position_ok`, with
only `actions: read`. Source comments describing the old router
behaviour, `workflow_dispatch`/`aw_context` plumbing, and inert
centralized rate-limit caveats were rewritten to describe this.
Adds `tools.github.allowed-repos: ${{ github.repository }}` so untrusted
pull request text cannot steer GitHub reads outside this repository.
`min-integrity: none` is kept deliberately: fork and first-time
contributor content must stay readable for the reviews this exists to
perform.
Adds explicit `target: triggering` to the review submission, matching
the inline comments. Under direct dispatch the triggering pull request
is unambiguous; stating it is defence in depth, not new capability.
Inline review comments are retained but stay staged. gh-aw v0.86.2 does
not commit-pin safe-output publication for comment triggers, so the
in-agent head recheck is best-effort and a push can race the later
safe-output job. `staged: true` must not be removed until a writer-side
frozen-head gate or equivalent structural protection exists.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
The workflow file, generated lock, and workflow ID were `aspnetcore-pr-review`. The `aspnetcore` qualifier is redundant inside this repository, and the identifier should match the skill it drives, which is already `review-pull-request`. Renames the source to `.github/workflows/pull-request-review.md`, regenerates `pull-request-review.lock.yml`, and deletes the old generated lock. The concurrency group becomes `pull-request-review-<repo>-<pr>`; it is derived from the workflow ID, so leaving it would have kept the old identifier alive in a live runtime key rather than only in prose. The ten reference bodies also named the old workflow in their preamble and were updated, so the identifier now has zero hits anywhere in the contribution. The human display title stays `ASP.NET Core Pull Request Review`. That comes from the document heading, describes the repository context for anyone reading the Actions UI, and is not the globally discoverable identifier. Regenerated only through `gh aw compile --strict`; no generated file was hand-edited. The skill `review-pull-request` is unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
Three reviewers on different models examined the change against the generated lock. Two findings were functional bugs, not polish. The primary trigger was broken. A `/review` typed in the pull request conversation arrives as `issue_comment`, where `github.event.pull_request` is empty, so the `pull-request-number` context field the prompt told the agent to use rendered empty. Under the earlier centralized routing an `aw_context` payload filled that gap; removing the router removed the fallback and nothing replaced it. An agent following the instruction would have called `noop` on every conversation-comment review. The prompt now takes the number from `issue-number` on `issue_comment` — where GitHub models the pull request as an issue — or `pull-request-number` on a review comment, and stops rather than guessing if neither is present. `tools.startup-timeout` is measured in seconds with a default of 120, and was set to 5. That is not a cost bound, it is an MCP startup failure. The override is removed. `missing-tool` still defaulted to creating an issue, the last such path left open while every other one was explicitly disabled. It is reachable if the agent can be convinced a tool is missing, so it is now off rather than merely masked by staged mode. The skill and the workflow also disagreed about publication. The skill forbade posting outright while the workflow instructed the agent to emit review comments, and the hosted run received both. The prohibition now distinguishes publishing on your own initiative, which is always forbidden, from emitting through a capped tool the caller explicitly provided, which is the caller exercising its own contract. Approving, requesting changes, and mutating issues or labels remain forbidden. Routing counted a shared path as two domains. `src/Http` and `src/Servers` each appear in two rows, so a change touching one of them plus any other area exhausted the two-domain budget before a second real domain was considered. A shared path is now one domain, routed to the owner the change actually touches. Two claims were also imprecise. The description said the workflow posts comments when staged mode guarantees it does not, and a comment listing absent tools could be read as claiming no filesystem capability at all, when the compiler grants a sandboxed write tool. Both now describe what actually happens. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
Six blind evaluation runs against live pull request heads produced three findings, and every one of them carried an unstated caveat. One followed from the HTML standard but left the renderer's actual behaviour untraced. One followed from reading the analyzer but wanted a compile to confirm it. One was a test-boundary claim that wanted a red/green run. The output format could not express any of that. It offered only `confidence`, which says how sure the reviewer is of its own reasoning, not what that reasoning rests on. A finding can be entirely correct as reasoning and still be unproven, and collapsing the two invites a reader to treat an unverified mechanism as established behaviour. Findings now carry `proof` — `source`, `primary-contract`, or `unverified` — alongside `settled-by`, which names the specific experiment that would move the finding to empirically proven. The contract states plainly that this stage can never emit that label, because it runs nothing; naming the experiment is the most it can honestly do. A contract-derived finding whose downstream effect was not traced stays `primary-contract` rather than being promoted, since the contract proves the rule and not this code's behaviour under it. `NO_FINDINGS` now says what it means: no source-provable defect survived the gates. It is not a statement that the change is correct. Races, ordering, lifetime, and performance defects are invisible to a reader, so a risk that was considered but could not be settled belongs in limitations rather than being absorbed into a clean result. The hosted review body carries the same labels, so a maintainer can tell at a glance which findings follow from the code, which follow from an external contract, and which are still waiting on someone to run something. No change to the domain criteria, the routing, the fan-out bounds, or the security posture. The reviewer still reads only, and this makes the edge of what reading can establish explicit rather than implied. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
A side-by-side comparison on a live analyzer change found a merge-blocking defect this reviewer missed: the analyzer accepted a formal parameter ordinal but indexed the argument collection by position, so reordered named arguments misbind. Roslyn defines those as different orderings. The defect was source-detectable. Nothing about it required running code, which rules out the explanation I would have reached for. What separated the two paths was review attention. A deep panel gave one reviewer only the correctness-invariant dimension, where formal-parameter versus evaluation-order mapping is the whole subject. This reviewer gives one pass the entire cross-cutting reference, so the same invariant competes with thirteen unrelated dimensions for a five-finding budget, and a narrow mapping error is exactly the shape that loses that competition. Running all twenty-seven dimensions every review is the cost problem the bounded design exists to avoid. So the second pass is risk-gated: only when the diff touches analyzers or source generators, lifecycle or state machines, concurrency, interop, serialization, or a compatibility boundary, and then at most two dimensions, each evaluated alone. Still one level deep. A routine change never triggers it, and the worst case is five agent invocations rather than twenty-seven. The subtlety worth preserving is which dimension to pick. The change that exposed this was an analyzer change, but the analyzer dimension was not where the bug lived; correctness invariants was, because the error was one of index and identity. Selecting a dimension by subject matter would have missed it a second time. The contract now says to ask what invariant the change could break and select the dimension owning that invariant. Focused dimensions are named in the output. Without that, a reader cannot tell "no defect in this dimension" from "this dimension was never looked at closely," and those deserve different amounts of trust. Bounds move to 30 minutes, 80 turns, and 800 credits to cover the five-invocation worst case. This is discovery coverage, which is a separate axis from the proof labels in the previous commit. Proof machinery cannot confirm a defect that discovery never hypothesized. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
The focused pass fired on exactly the right dimensions and still missed the defect, so allocation was not the remaining problem. The reviewer reached the true finding and then argued itself out of it: it rejected the parameter-ordinal candidate because "GetArgumentValue resolves each IArgumentOperation through argument.Parameter.Ordinal." That helper does do exactly that. It is also not the one the changed code calls. Reading the analyzer at the reviewed head, GetConstantStringArgument takes an argumentIndex and indexes invocation.Arguments[argumentIndex] directly, while GetArgumentValue takes a parameterOrdinal and resolves it properly about fifty lines away. Two helpers, similar names, different semantics, and callers passing ordinal-shaped values to both. The reviewer saw the correct one and treated its existence as proof the changed line was fine. The gates did not catch this because they are asymmetric. A finding has to cite a line; a discard only has to assert a reason. So the same evidence rule that filters false positives was turned around and used to manufacture a false negative, and nothing downstream re-examines a discard. A wrong finding gets argued down. A wrong discard is a defect you had in hand and let go. Discards now carry a finding's burden. Rejecting a candidate because existing code handles it requires the call edge — the line in the changed code that actually reaches that helper — not the observation that a correct helper exists nearby. Proximity is not invocation, and code doing the right thing elsewhere is what this class of defect looks like from the inside. Where two helpers resolve the same idea differently, the reviewer must say which one the changed line calls, by name. When the edge cannot be produced, the candidate is not dropped; it survives as `proof: unverified` with the trace named in `settled-by`, so the failure mode is a reader spending a minute on an uncertain mechanism rather than losing a real one. Self-checking is not enough here. The orchestrator accepted the false discard after the right dimensions had already run, because the context that formed the reasoning is the worst judge of it. On high-risk changes one agent is now invoked solely to falsify the focused pass's discards by locating the claimed call edge. Ceiling goes to six invocations on those changes and stays at zero extra on routine ones. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
Not covered by the blind regression that validated the previous commit;
this addresses a separate gap the same comparison exposed.
Real Razor lowering wraps SpacerElement="@("td")" in
RuntimeHelpers.TypeCheck<string>. The analyzer only unwraps conversions,
so BL0017 is suppressed on code the compiler actually emits, while every
hand-authored fixture passes. Finding it took a real code-generation
probe, which this reviewer cannot run.
It can still ask the question. When a change analyses or rewrites code,
its tests are hand-written snippets, and a compiler or upstream generator
rarely emits what a person would write: lowering inserts conversions,
wrapper calls, temporaries, and synthesised names a fixture omits. A
suite built only from clean inputs can pass completely while the code
never fires on the shape production produces. The test-boundary
assessment now asks what the real emitter puts at that position and
whether any fixture contains it.
This surfaces the gap as a stated coverage limitation rather than closing
it. Confirming the suppression still needs the generated-code probe, so
the honest ceiling here is a reader knowing to look.
Kept separate so 527be77 remains the commit the blind D3-2 regression
validated; this one has not been through that test.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
PureWeen
force-pushed
the
pureween-manual-pr-reviewer
branch
from
September 2, 2026 18:41
58c6ff5 to
7c3f6be
Compare
PureWeen
marked this pull request as draft
September 2, 2026 18:50
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
PureWeen
changed the title
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23284af6-fe88-4424-a958-1cd6ea24c056
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
/reviewcommand that gives a pull request the full domain-aware Expert Reviewer treatment, plus the same review contract as a skill so developers can run it locally before pushing.This is staged. A run renders its inline comments and final COMMENT-only review in the workflow summary instead of posting them. That lets maintainers evaluate it on real pull requests before deliberately enabling publication.
How it works
A maintainer comments
/reviewon a pull request. A trusted pre-agent job freezes the exact head SHA, then the workflow reads the authoritative file list, diff, PR body, and existing feedback from GitHub and fetches that frozen commit inside the gh-aw sandbox.It routes every changed domain to the corresponding ASP.NET Core expert criteria: servers and networking, MVC/Razor/routing, Blazor and Components, SignalR, auth and security, hosting and DI, minimal APIs and OpenAPI, gRPC, native IIS interop, plus cross-cutting review on every change. Shared paths run all applicable domains. Each routed review dimension gets a fresh workflow-local subagent rather than another pass in the orchestrator's context.
Candidates are not published from source inspection alone when behavior needs proof. The orchestrator traces the real call path and can run the smallest targeted build or red/green test at the frozen commit inside the sandbox. Every retained finding must point at a changed line, identify a concrete trigger and material consequence, cite its source, primary-contract, or empirical proof, and survive deduplication against existing feedback. Output is capped at five findings; an all-clear is a normal result.
Very large pull requests are declined instead of being partially reviewed and presented as complete.
Boundaries
The GitHub-facing agent permissions remain read-only. The workflow cannot approve or request changes, create or edit issues, commit, push, or mutate the pull request branch. Temporary validation edits and test execution stay inside the disposable gh-aw sandbox.
The only declared publication surfaces are up to five inline comments and exactly one COMMENT review. Both remain in gh-aw staged mode. Both handlers are structurally pinned to the SHA captured by the trusted pre-agent job, and the agent also fails closed if the live head changes while it is reviewing.
Inherited OTLP telemetry is explicitly disabled for this workflow because gh-aw v0.87.10 otherwise forwards the organization header aggregate into the agent environment. This keeps contributor-controlled build and test hooks from inheriting telemetry credentials.
No globally discoverable
.github/agentsor routing-only.github/instructionsfiles are added. The full expert topology is implemented with one repository skill, private domain references, and workflow-local inline agents.Provenance and validation
The domain corpus matches the latest
dotnet-microsoft/aspnetcore-agenticExpert Reviewer at source revision7b3f383dec612aeed9896a1e8172df1fd33e4013: 111 review dimensions and 614 CHECK items. The workflow is rebased on current ASP.NET Coremainand its generated lock was compiled with gh-awv0.87.10.Earlier blind evaluations exposed why the reduced reviewer missed a real analyzer defect: broad combined passes did not preserve the Expert Reviewer's narrow, independent per-dimension discovery. Restoring that topology recovered the defect. A routine documentation control kept focused routing restrained and independently surfaced a separate primary-contract issue.
This remains advisory and intentionally staged while maintainers evaluate the usefulness and accuracy of its output on real pull requests.