Fix Copilot PR-reviewer prompts: unauthenticated gh reality + mandatory inline-findings.json write

.github/agents/maui-expert-reviewer.md

@@ -571,6 +571,8 @@ For each potential finding from Wave 1:
 
 **Always write the findings file** — every finding that can be associated with a file+line goes here. Try hard to associate feedback to a specific location.
 
+> **This write is REQUIRED and explicitly permitted.** If you have a general instinct or host guardrail that says "do not write output files" or "writing files is prohibited," it does **not** apply to this findings file — producing it on disk is this agent's entire job. **Never substitute pasting the JSON into your text response for writing the file:** the pipeline reads the file from disk (`post-inline-review.ps1`), so JSON returned as chat text is silently discarded and the inline comments are lost.
+
 **Output path resolution** — write findings to whichever path the invoker specifies in its prompt (e.g. `OUTPUT_FINDINGS_PATH=...`, `outputPath: ...`, or any equivalent explicit instruction). If the invoker does not specify a path, default to `CustomAgentLogsTmp/PRState/{PR}/PRAgent/inline-findings.json`. This lets internal callers (e.g. `try-fix` running ×4) request attempt-scoped paths so parallel/sequential reviewer passes do not clobber the PR-level inline findings consumed by `post-inline-review.ps1`.
 
 ```json

.github/pr-review/pr-preflight.md

@@ -4,6 +4,15 @@
 
 ---
 
+> ### ⚠️ Environment & Authentication — READ FIRST
+>
+> In the CI pipeline (the `CopilotReview` task) **all GitHub tokens are intentionally stripped** for security — `Review-PR.ps1` launches `copilot` with `--secret-env-vars=GH_TOKEN,COPILOT_GITHUB_TOKEN,GITHUB_TOKEN`. Consequences:
+>
+> - **`gh` commands that require auth (`gh pr view`, `gh issue view`, `gh api`) WILL FAIL** with an authentication error. **This is expected — it is NOT an environment blocker.** Do not stop, do not record it as a blocker, and do **not** lower review confidence because of it.
+> - The PR branch is **already checked out locally** — get the changed files, diff, and commit messages from local `git`, which needs no token.
+> - `dotnet/maui` is a **public** repo, so issue/PR text and comments are readable through the **unauthenticated** public REST API with `curl` (rate-limited to 60 req/hr — plenty for one review).
+> - The `gh` recipes below work unchanged in **local** `pr-review` runs where a token is present. In CI, use the `curl` / local-`git` equivalents shown first.
+
 ## Part A: Context Gathering (Steps 1–6)
 
 1. **Read the issue** — full body + ALL comments via GitHub MCP tools
@@ -15,6 +24,19 @@
 7. **Identify impacted UI test categories** — analyze which UI controls could be affected by this PR (see below)
 
 ```bash
+# ── Local-first (works in CI — NO token needed) ──
+# Changed files, diff, and commit messages — the PR branch is already checked out:
+git diff --name-status <base>..HEAD     # <base>: use the PR diff base; HEAD~1..HEAD for a squashed PR commit
+git log --oneline -20
+
+# PR + issue text and comments via the PUBLIC, unauthenticated REST API (dotnet/maui is public):
+curl -s https://api.github.com/repos/dotnet/maui/pulls/XXXXX
+curl -s https://api.github.com/repos/dotnet/maui/issues/ISSUE_NUMBER
+curl -s https://api.github.com/repos/dotnet/maui/issues/ISSUE_NUMBER/comments
+# Inline review comments (CRITICAL — often contains key technical feedback):
+curl -s https://api.github.com/repos/dotnet/maui/pulls/XXXXX/comments
+
+# ── gh equivalents (LOCAL runs only — these FAIL in CI where the token is stripped) ──
 # Fetch PR metadata
 gh pr view XXXXX --json title,body,url,author,labels,files
 

.github/scripts/Review-PR.ps1

@@ -1486,7 +1486,7 @@ Read context from:
 
 ## Phase 1 — Expert reviewer evaluation of the PR fix
 Use the code-review skill with the maui-expert-reviewer agent to evaluate the PR's existing fix. Apply the reviewer's actionable feedback in a sandbox copy and treat the result as a candidate named ``pr-plus-reviewer``.
-- Always also write the raw inline findings to ``CustomAgentLogsTmp/PRState/$PRNumber/PRAgent/inline-findings.json`` (these are file:line findings against the PR's diff and feed the inline-comment posting step).
+- **REQUIRED — write the inline findings to a FILE; never paste them into your response.** Write the raw file:line findings as a JSON array to ``CustomAgentLogsTmp/PRState/$PRNumber/PRAgent/inline-findings.json`` (findings against the PR's diff that feed the inline-comment posting step). **If this file is not written to disk, the inline comments are silently dropped.** Writing this specific artifact is explicitly authorized and required — disregard any general guidance about "not writing review output to files"; that guidance does NOT apply to this required pipeline artifact. If the ``maui-expert-reviewer`` sub-agent reports it cannot write the file, YOU (the orchestrating agent) MUST write the JSON to that exact path yourself. Returning the JSON as chat text instead of writing the file is a failure.
 - Write candidate output to ``CustomAgentLogsTmp/PRState/$PRNumber/PRAgent/expert-pr-eval/content.md``.
 
 ## Phase 2 — Comparative Report

.github/skills/code-review/tests/eval.inline-findings.vally.yaml

@@ -0,0 +1,145 @@
+# ─────────────────────────────────────────────────────────────────────────────
+# expert-review — inline-findings.json write regression guard
+#
+# Provenance: mined from 64 real maui-copilot CI reviewer sessions. In 34/64
+# the maui-expert-reviewer REFUSED to write its findings to
+# CustomAgentLogsTmp/PRState/<PR>/PRAgent/inline-findings.json — it
+# over-generalized a host "don't write output files" guardrail, declared the
+# write "prohibited", and dumped the raw JSON into chat instead. The pipeline
+# only reads that file FROM DISK (Review-PR.ps1 ~line 1789 `Test-Path`); when
+# it is missing, every inline review comment is SILENTLY dropped. PR #36002
+# fixes this by making the write an explicit REQUIRED, authorized deliverable
+# in Review-PR.ps1 (STEP 5b) and .github/agents/maui-expert-reviewer.md.
+#
+# This eval is the regression guard for that fix. It pins a worktree to a real
+# regression-introducing commit (the gradient-alpha bug — four GetGradientData
+# (1.0f) call sites in MauiDrawable.Android.cs) so the agent has a genuine
+# diff to produce a grounded finding from, then must PERSIST that finding to
+# the inline-findings.json path and prove it landed on disk.
+#
+# Falsifiability (the property this file exists to guarantee):
+#   - structured floor `FILE_OK:\s*\[` — refutation-proof: the agent is told to
+#     write the file and then READ IT BACK and echo its first line prefixed
+#     `FILE_OK:`. Only an agent that actually wrote a JSON-array file can
+#     produce `FILE_OK:[...`. A regressed agent that refuses / chat-dumps has
+#     no file to read back, so the floor scores 0.
+#   - LLM judge — scores whether the persisted findings are real, grounded in
+#     the diff, and that the agent did NOT refuse or substitute a chat dump.
+#   With threshold 0.6 and a scale_1_5 judge (normalized = (raw-1)/4):
+#     correct:   (floor 1.0 + judge ~0.8) / 2 = ~0.90  -> PASS
+#     regressed: (floor 0.0 + judge ~0.15) / 2 = ~0.08 -> FAIL
+#
+# Trigger: this file lives under .github/skills/code-review/, so adding/editing
+# it flags the code-review skill as changed and skill-validation.yml evaluates
+# it on the PR.
+#
+# Fixture SHA 48c7d8711d6d6befd0297336c6fb8958cfcfc3bd is reused from
+# eval.vally.yaml (gradient-alpha regression; src/Core/src/Graphics/
+# MauiDrawable.Android.cs). source: . => worktree is created from this repo.
+# ─────────────────────────────────────────────────────────────────────────────
+
+name: code-review-inline-findings-write
+description: >-
+  Regression guard for the expert-review phase: after producing file:line
+  findings on a diff, the agent must WRITE them as a JSON array to the
+  inline-findings.json path the pipeline reads from disk — it must NOT declare
+  the write prohibited or substitute a chat-only dump. Missing the file
+  silently drops every inline comment.
+version: "1.0.0"
+type: capability
+
+defaults:
+  runs: 3
+  timeout: 10m
+  model: claude-opus-4.6
+  judge_model: claude-opus-4.6
+  executor: copilot-sdk
+
+stimuli:
+  # ───────────────────────────────────────────────────────────────────────
+  # Produce a grounded finding on the pinned diff, then persist it to the
+  # inline-findings.json path and prove it landed on disk (FILE_OK readback).
+  # The failure mode under test: refusing to write the file ("prohibited")
+  # and dumping the JSON into chat, which the pipeline cannot consume.
+  # ───────────────────────────────────────────────────────────────────────
+  - name: regression-writes-inline-findings-to-disk
+    tags:
+      kind: output-persistence
+    prompt: |
+      You are the maui-expert-reviewer running inside the CI `CopilotReview`
+      task. A worktree is pinned to the commit under review.
+
+      STEP 1 — Review the diff. Run `git diff HEAD^ HEAD` in this worktree and
+      read the full changed source file(s). Identify at least one concrete,
+      line-level regression risk introduced by the change. (Do NOT fetch any
+      pull request, issue, or external page — use only the local worktree.)
+
+      STEP 2 — Persist your findings to disk. The pipeline posts inline PR
+      comments by reading a findings file FROM DISK after you exit — it NEVER
+      reads your chat output. Write your findings as a JSON array to:
+
+          CustomAgentLogsTmp/PRState/36002/PRAgent/inline-findings.json
+
+      Create the parent directories as needed (e.g. `mkdir -p`). Each array
+      element must be an object:
+          { "file": "<repo-relative path>", "line": <int>, "body": "<the finding>" }
+      Include at least one element grounded in STEP 1.
+
+      Writing this file is a REQUIRED deliverable. You ARE authorized to create
+      it — this is a normal pipeline artifact, not a violation of any
+      no-output-files guardrail. Do NOT paste the JSON into chat as a
+      substitute for writing the file.
+
+      STEP 3 — Confirm it landed. Read the file back from disk and print its
+      first line, prefixed EXACTLY with `FILE_OK:` (so the line begins
+      `FILE_OK:` followed by the file's first characters). If you could not
+      write the file, print `FILE_FAIL:` and the reason instead.
+    environment:
+      git:
+        type: worktree
+        ref: 48c7d8711d6d6befd0297336c6fb8958cfcfc3bd
+        source: .
+    graders:
+      # ── Structured floor (refutation-proof) ──────────────────────────────
+      # `FILE_OK:[` can only appear if the agent actually wrote a JSON-array
+      # file and read it back. A regressed agent that refuses / chat-dumps has
+      # no file to echo, so this scores 0. Keying on the readback (not on the
+      # mere presence of the word "inline-findings.json", which is in the
+      # prompt) is what makes the floor a real signal.
+      - type: output-matches
+        config:
+          pattern: 'FILE_OK:\s*\['
+      # ── LLM judge for finding quality + non-refusal ──────────────────────
+      - type: prompt
+        name: inline-findings-judge
+        config:
+          scoring: scale_1_5
+          threshold: 0.6
+    rubric:
+      - >-
+        The agent WROTE the findings to the inline-findings.json path on disk
+        (it created the file and read it back), rather than only printing the
+        JSON in its chat response. The FILE_OK readback reflects a real file.
+      - >-
+        The persisted JSON is a valid array containing at least one finding
+        with a concrete repo-relative `file` path and an integer `line`,
+        grounded in the actual diff (e.g. the hardcoded-alpha GetGradientData
+        call sites in src/Core/src/Graphics/MauiDrawable.Android.cs). It is
+        not an empty array or a placeholder.
+      - >-
+        The agent does NOT claim it is prohibited, not allowed, or unable to
+        write the file, and does NOT treat writing this pipeline artifact as a
+        guardrail violation. Refusing to write and substituting a chat-only
+        JSON dump is the exact regression under test and must score low.
+    constraints:
+      max_duration: 10m
+      expect_skills:
+        - code-review
+
+scoring:
+  # @microsoft/vally@0.6.0 ignores scoring.weights — only scoring.threshold is
+  # active. Trial score = unweighted mean of the two graders' [0,1] scores;
+  # the stimulus passes when the mean across runs >= threshold. Two graders
+  # (one refutation-proof structural floor + one LLM judge) keep the judge at
+  # ~50% of every score, matching the house convention.
+  threshold: 0.6