Replace PR reviewer orchestration with a frozen review bundle

[PureWeen]

Summary

Replaces the orchestration-heavy review-pull-request skill (supersedes #69467) with a simpler design:

• scripts/prepare-review.cs is a trusted producer. It is a .NET file-based app that uses only the BCL and runs with dotnet run prepare-review.cs -- --pr N. It:

  • freezes the PR's head, merge base, and base tip;
  • fetches the authoritative diff and existing feedback;
  • routes guides with the existing routing rules, including renames via previous_filename;
  • writes a frozen review bundle with a v2 manifest, including per-phase timings.

  The manifest's producer is the SHA-256 of the running prepare-review.cs source. A bundle from any other producer is rejected on reuse.

  Script-local Directory.Build.props, Directory.Build.targets, and Directory.Packages.props isolate it from the repository's build: no repo-root imports, and native AOT, trimming, and apphost are disabled. The isolated xUnit v3 test project, scripts/PrepareReview.Tests.csproj, is not referenced by any solution or product build. Run it with dotnet run --project .github/skills/review-pull-request/scripts/PrepareReview.Tests.csproj after activating the repo SDK.

  Reviewer guidance comes from a selected trusted source: the local checkout when run interactively, or the workflow's own commit when hosted. It never comes from the target release branch or from PR-controlled content. Every link found in a guide is classified and recorded:

  • policy sections are included as review criteria;
  • architecture documents are included as orientation-only context;
  • all other links are recorded as skipped.

  The repository's "Security Concerns Are Out of Scope" exclusion is carried over verbatim.

• SKILL.md is now a review-only contract (173 lines):

  • a one-line local bootstrap;
  • one worker per routed guide, launched as a full-capability general-purpose agent explicitly on gpt-5.6-sol. A confirmed worker type or model mismatch makes that guide incomplete;
  • tool read denials are reported with the exact error and never blamed on content exclusion;
  • a required STATUS: line: BLOCKED / INCOMPLETE / FINDINGS / NO_FINDINGS;
  • every guide marked complete, incomplete, or excluded;
  • new findings, still-applicable existing feedback, and UNRESOLVED candidates reported separately;
  • acceptance rules and P1–P3 severity;
  • a requirement to read the full callee body before accepting or discarding a claim;
  • "unchanged behavior" discards must compare old and new effects along the exact input sequence.

  External-contract and metadata gaps are UNRESOLVED, not INCOMPLETE.

• pull-request-review.md (gh-aw) runs the same producer when /review is requested and before the agent starts. It first installs .NET SDK 11.0.100-rc.1.26420.103, the same version as global.json, via actions/setup-dotnet (pinned by SHA). The workflow does not check out the repo, so this version is pinned by hand. It then downloads the script and its three props files at GITHUB_WORKFLOW_SHA. The hosted agent keeps its result in the conversation and writes no files.

  • The agent has no GitHub tools and reads only from the bundle.
  • A trusted verify_live_head job must pass before safe_outputs publishes a COMMENT-only review pinned to the frozen head.
  • The workflow was recompiled with gh-aw v0.89.21 --strict. The compile is reproducible, and actions-lock.json is unchanged.

This fixes the original failure where release branches that predate the shared guides, such as release/11.0, could not be reviewed. There is no automatic fallback to main, local files, or the PR head.

Guide routing is still hard-coded to the two existing guides: CrossCutting always, plus BlazorComponents for src/Components. New architecture documents are supporting context and do not require new routing; a new review guide would.

Validation

Producer tests: 40/40 on macOS and on Windows 11 (core.autocrlf=true). They cover routing, link classification, renames, mixed-line policies, exclusions, producer provenance, JSON byte compatibility, and malformed or empty inputs.

C# producer vs the previous JavaScript producer (differential): outputs were compared file by file, except Git pack storage, which is compressed nondeterministically. The only allowed differences were timings, the producer hash, and temp paths.

• macOS: byte-identical on 17 open PRs, including a release/11.0 PR, a rename, and a JSInterop-only PR. The same exit and readiness behavior was seen for merged, invalid, and existing-output inputs, bad --repo, an invalid token, and a dirty guidance tree. Mean time: 52.6 s (C#) vs 59.6 s (JS).
• Windows: byte-identical on [release/11.0] Use SignalR authentication refresh for Blazor Identity #68895 and Add JSInterop analyzer ensuring 'IsInteractive' is checked before using it. #67969, at 114–122 s (C#) vs 226–248 s (JS).
• Hosted: setup-dotnet took about 9 s, and the producer about 20 s cold, vs about 30 s for JS.

Real-PR regression evaluation (local, unpublished, Windows, independently audited): this PR's contract vs its previous revision, on #68895, #67969, #68365, #67616, #69484, and #65504 (negative control). There were 23 runs, all with gpt-5.6-sol (model use verified from logs).

Measure	Previous revision	This revision
Recall on 8 known true positives, first run	3/8	4/8
Recall on 8 known true positives, including extra runs	4/8	6/8
Severity: correct / inflated	1 / 4	5 / 2
False positives	2	2
BLOCKED / INCOMPLETE	one INCOMPLETE, which was not an input failure	none

• In this revision, no true positive was discarded or demoted with wrong reasoning.
• Duplicate recognition was correct in every run.
• The negative control returned NO_FINDINGS in every run.
• All versions still miss the [release/11.0] Use SignalR authentication refresh for Blazor Identity #68895 sliding-expiration issue.

Model probe: the same revision on gpt-6.1-sol cost about half as much (0.48×). It demoted real regressions to UNRESOLVED five times, citing a missing binding contract or external evidence, and hedged a sixth. The workflow therefore stays on gpt-5.6-sol; see the follow-ups.

Hosted, on a fork only (this revision): fork run 36767006150 at b93874e6, the head before worker pinning. Every job succeeded (agent, detection, verify_live_head, safe_outputs).

• Timings: setup-dotnet took 9 s, preparing the bundle 22 s, and the agent 280 s.
• Result: the planted InputDate culture bug was published as a P2 COMMENT review on the correct line, pinned to the frozen head. Both routed guides completed.
• Earlier failures: two earlier fork runs of this revision failed in the agent step with a Copilot CLI 1.0.80 error, 400 Invalid 'input[N].id': 'ctc_call_…'. Expected an ID that begins with 'fc'. Each failure came immediately after the agent used the file-create tool to save its result. The same fixture on the previous JS revision (run 36765116023) wrote no file and passed.
• Fix: the workflow prompt now tells the agent to keep its structured result in the conversation and never write files. tools: edit: false did not remove that tool.

Worker pinning (native Windows validation): in 2 of 4 developer-style native runs, the coordinator launched guide workers as explore agents on gpt-5.6-luna, yet still reported both guides complete and NO_FINDINGS. SKILL.md now pins the workers (see above). Fork run 36774667141 at this head passed every job and published the planted P2 finding. Its log shows both workers as General-purpose(gpt-5.6-sol). Six more native Windows runs at this head all used general-purpose / gpt-5.6-sol workers (12 of 12, confirmed from runtime telemetry), and no read denials were blamed on content exclusion.

Earlier hosted and local validation (JS producer; the design is otherwise unchanged):

• Planted-bug positive/control pairs on main and on a release/11.0-era base. Each finding was published on the correct line at the frozen head, and each control had no findings.
• Moved head, end-of-run gate (run 36085449165): the head moved while the agent was running, verify_live_head failed, and nothing was published.
• Moved head during preparation (run 36084564466): the producer blocked before the agent started.
• Order root component operations without serializing circuit batches #69484, where main's reviewer returned BLOCKED twice (runs 36171766315 and 36173504000), completed locally with NO_FINDINGS.
• Hosted smoke run 36542918680 published a COMMENT review pinned to the frozen head.
• Earlier 14-PR real-PR evaluation: 6 true positives confirmed by independent validation, none of which the previous reviewer found. No accepted finding was shown to be false.

Known limitation: recall varies between runs

This is an advisory reviewer, not a merge gate; NO_FINDINGS does not mean the PR is correct. Across five gpt-5.6-sol runs over two contract revisions, the known #68365 InputBase P2 issue was found twice. Both runs at this head missed it, even though their workers matched the run that found it. That was an exploration miss, not a wrong discard, so the smoke-test criterion for that PR is not met. All versions also miss the #68895 sliding-expiration issue. Improving recall (an evaluation harness and a reasoning-level sweep) is a follow-up.

Known limitation: external contracts

Reviewers read only the frozen bundle, so they cannot cite primary external contracts such as the WHATWG HTML or Streams specs, or dotnet/runtime source. Findings that depend on one are reported as UNRESOLVED rather than guessed.

• Locally: main's reviewer can fetch specs today, so this PR occasionally misses what main catches. For example, main accepted the case-sensitive VALUE issue on [Blazor] Preserve null values for <option value="@null"> in single select binding #67616 using the WHATWG attribute-name contract.
• Hosted: a review that needs a spec publishes nothing.

Allowing primary-spec evidence (and, for hosted, a network allowlist) is a permission decision deferred to a follow-up PR.

Known limitation: long bundle paths on Windows

On Windows, a Copilot CLI path-verification bug treats files whose absolute path exceeds 256 characters as outside the allowed directories. This happens even inside %TEMP%, the working directory, or an --add-dir root, and even with LongPathsEnabled=1. The CLI keeps the long file in its \\?\ extended-length form but stores allowed directories in normal form, so they never match. Non-interactive runs with --allow-all-tools therefore deny those bundle reads (thousands of files per bundle, up to 382 characters). Approve the prompt interactively, or pass --allow-all-paths, which is verified but grants access to every path; --add-dir does not help. Otherwise the reviewer returns INCOMPLETE and quotes the tool's error. The CLI's error text doesn't mention path length, so the reviewer can't yet suggest this fix itself; improving that message is a follow-up. The previous JS producer used the same layout, and hosted runs (Linux) are unaffected.

Known limitation: publication freshness and moving bases

verify_live_head runs in a separate job before safe_outputs. A push between the two, or a Re-run failed jobs of only safe_outputs after the head moved, can still publish the review. It remains a COMMENT-only review pinned to the commit that was actually reviewed.

The producer also returns BLOCKED if the target or base branch moves during preparation. This is rare hosted, where preparation takes about 20 s, but it happened on a busy main in local runs; one retry recovered it every time.

Follow-ups (separate PRs)

• Contract wording: a before/after regression never needs an external contract; a feature's own frozen documentation counts as its contract; UNRESOLVED must name the exact missing artifact. Evaluate this, then re-compare models: gpt-6.1-sol, and a Sol vs Astra reasoning-level sweep.
• Existing feedback that was claimed fixed but still applies should surface as a finding, not only as coverage.
• Base moves during preparation: one bounded automatic retry.
• Publication job: re-check the live head inside it.
• Primary-spec evidence access (a permission decision).
• Offline evaluation: a harness to replace the JS-only planted-fixture builder removed here, and tracking of recall variance.
• Hosted coverage that is only possible after merge: the upstream repository environment, community fork PRs, and docs-only and large PRs.
• Not yet tested: suppression of a valid finding when one guide is incomplete.
• Shorter bundle paths for Windows, and a reviewer hint for the non-interactive permission error.
• Bundle size: 0.6–0.7 GB; about 20 s to prepare hosted, about 2–3 min on Windows.
• Linked-issue bodies in the bundle.
• Architecture documents reach workers only when a routed guide links to them.

No product code changes.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved moderate issues and a critical publication-gating issue remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Reject noop alongside review outputs · New

What changed in this PR

Replaces live review orchestration with a trusted, frozen v2 review bundle and updates hosted publication gating.

Changes:

• Freezes PR state, diffs, feedback, guide routing, and linked guidance.
• Simplifies the review skill to consume bundle evidence.
• Updates the hosted workflow and lockfile with validation and live-head checks.
• Adds producer and offline evaluation tests.

File	Summary
.github/​workflows/​pull-request-review.md	Hosted workflow and publication gates; unresolved GHES hostname handling and conflicting noop output validation issues.
.github/​workflows/​pull-request-review.lock.yml	Generated workflow lockfile update.
.github/​skills/​review-pull-request/​tests/​prepare-review.test.mjs	Producer behavior and validation tests.
.github/​skills/​review-pull-request/​tests/​prepare-eval-fixture.mjs	Evaluation fixture generation; routing and rename applicability are not fully recomputed.
.github/​skills/​review-pull-request/​SKILL.md	Frozen-bundle review contract.
.github/​skills/​review-pull-request/​scripts/​prepare-review.mjs	Bundle producer; unresolved fragment parsing, external-link recording, and architecture-link classification issues.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Bundle reuse can accept a concurrently stale target, and process-output limits are enforced only after unbounded buffering.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Bound process output while reading streams · New
• Recheck target identity after bundle validation · New

Resolved since last review (1)

• Reject noop alongside review outputs