Replace PR agent with pr-review skill and decouple comment posting

[kubaflo]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

🏗️ Overview

This PR replaces the monolithic PR agent with a modular, skill-based PR review orchestrator. The legacy 2-file agent is decomposed into 4 dedicated phase skills, each with clear boundaries, inputs, and outputs.

TL;DR — One big agent file → four focused skills. Cleaner, composable, easier to maintain.

---

🔀 Before → After

┌─────────────────────────────────┐       ┌─────────────────────────────────────────────┐
│          BEFORE (Agent)         │       │              AFTER (Skills)                  │
├─────────────────────────────────┤       ├─────────────────────────────────────────────┤
│                                 │       │                                             │
│  agents/pr.md (310 lines)      │       │  skills/pr-review/SKILL.md  (orchestrator)  │
│         +                      │  ──►  │     ├─ skills/pr-preflight/SKILL.md         │
│  agents/pr/post-gate.md (331)  │       │     ├─ skills/pr-gate/SKILL.md              │
│  agents/pr/SHARED-RULES.md     │       │     ├─ try-fix  (existing skill)            │
│  agents/pr/PLAN-TEMPLATE.md    │       │     └─ skills/pr-report/SKILL.md            │
│                                 │       │                                             │
│  5 comment scripts              │       │  2-section unified comment                  │
│                                 │       │                                             │
└─────────────────────────────────┘       └─────────────────────────────────────────────┘

---

✨ Key Changes

1️⃣ Agent → Skill Architecture

Phase	Skill	Responsibility
0	pr-review	🎯 Master orchestrator — sets up branch, invokes phases
1	pr-preflight	📋 Context gathering — reads issue, PR, comments, classifies files
2	pr-gate	🚦 Test verification — tests must FAIL without fix, PASS with fix
3	try-fix (existing)	🔧 Multi-model fix exploration (Sonnet → Opus)
4	pr-report	📊 Final recommendation — APPROVE or REQUEST CHANGES

2️⃣ Decoupled Comment Posting

pr-review produces output files only (CustomAgentLogsTmp/PRState/). Comments are posted separately via ai-summary-comment — no tight coupling.

3️⃣ Unified Comment Model

Consolidated from 5 separate comment scripts → 2 sections in a single unified comment:

❌ Removed Script	✅ Now Part Of
post-try-fix-comment.ps1	PR-REVIEW section
post-verify-tests-comment.ps1	PR-REVIEW section
post-write-tests-comment.ps1	PR-REVIEW section

post-pr-finalize-comment.ps1 now injects into the unified comment by default (use -Standalone for legacy behavior).

4️⃣ Simplified CI & Orchestration

Review-PR.ps1 went from complex 3-phase script → clean 4-step orchestrator:

Step 1 → pr-review skill     (all 4 phases)
Step 2 → pr-finalize skill   (verify title/description)
Step 3 → ai-summary-comment  (post unified comment)
Step 4 → Update-AgentLabels  (apply labels)

ci-copilot.yml — removed explicit -RunFinalize -PostSummaryComment flags (all phases run automatically).

---

📁 Files Changed

✅ Added (4 files)

File	Purpose
.github/skills/pr-review/SKILL.md	Master orchestrator (Phase 0–4)
.github/skills/pr-preflight/SKILL.md	Phase 1: Context gathering
.github/skills/pr-gate/SKILL.md	Phase 2: Test verification gate
.github/skills/pr-report/SKILL.md	Phase 4: Final recommendation

❌ Deleted (7 files)

File	Reason
.github/agents/pr.md	Replaced by pr-review skill
.github/agents/pr/PLAN-TEMPLATE.md	Skills use direct output files
.github/agents/pr/SHARED-RULES.md	Rules distributed to phase skills
.github/agents/pr/post-gate.md	Replaced by pr-gate + pr-report
post-try-fix-comment.ps1	Merged into unified PR-REVIEW comment
post-verify-tests-comment.ps1	Merged into unified PR-REVIEW comment
post-write-tests-comment.ps1	Not part of main review workflow

🔧 Modified (14 files)

File	Change
.github/scripts/Review-PR.ps1	Major refactor → 4-step skill orchestration
.github/skills/ai-summary-comment/SKILL.md	Simplified to 2-section model
.github/skills/ai-summary-comment/.../post-ai-summary-comment.ps1	Updated for unified comment
.github/skills/ai-summary-comment/.../post-pr-finalize-comment.ps1	Injects into unified comment
.github/copilot-instructions.md	Replaced agent entry with skill entries
.github/README-AI.md	Updated architecture docs
.github/docs/agent-labels.md	Terminology: "Gate" → "Validate"
.github/scripts/shared/Update-AgentLabels.ps1	Updated for phase structure
.github/scripts/shared/Start-Emulator.ps1	Minor utility updates
.github/skills/pr-finalize/SKILL.md	Minor context update
.github/skills/verify-tests-fail-without-fix/SKILL.md	Minor context update
.github/skills/ai-summary-comment/NO-EXTERNAL-REFERENCES-RULE.md	Updated for 2-section model
.github/instructions/sandbox.instructions.md	Updated references
eng/pipelines/ci-copilot.yml	Simplified invocation

---

📊 Impact

 25 files changed
 843 insertions(+)
 3,271 deletions(-)

Net reduction of ~2,400 lines — less code, better structure. 🎉

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34460

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34460"

[Copilot]

Pull request overview

This PR migrates the Copilot-driven PR review automation from the legacy “pr agent” docs/scripts to a skill-based workflow centered on a new pr-review orchestrator (plus phase skills), and updates CI + commenting/labeling plumbing accordingly.

Changes:

• Introduces new PR review skills (pr-review, pr-preflight, pr-gate, pr-report) and removes legacy PR agent instruction files.
• Refactors Review-PR.ps1 to run a 4-step skill sequence (review → finalize → post summary → apply labels) and updates CI invocation.
• Simplifies AI summary comment tooling by removing try-fix / write-tests / verify-tests comment scripts, and updates related docs.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
eng/pipelines/ci-copilot.yml	Updates CI to invoke Review-PR.ps1 without legacy flags; publishes artifacts.
.github/skills/verify-tests-fail-without-fix/SKILL.md	Documentation wording adjustment for where verification output is consumed.
.github/skills/pr-review/SKILL.md	New orchestrator skill describing the 4-phase PR review workflow.
.github/skills/pr-report/SKILL.md	New “report/final recommendation” phase skill documentation.
.github/skills/pr-preflight/SKILL.md	New “context gathering” phase skill documentation.
.github/skills/pr-gate/SKILL.md	New “test verification” phase skill documentation and output format.
.github/skills/pr-finalize/SKILL.md	Updates wording to reference pr-review skill rather than the old agent.
.github/skills/ai-summary-comment/SKILL.md	Updates AI summary comment skill docs to match the new workflow structure.
.github/skills/ai-summary-comment/scripts/post-write-tests-comment.ps1	Deletes legacy unified-comment section updater for write-tests attempts.
.github/skills/ai-summary-comment/scripts/post-verify-tests-comment.ps1	Deletes legacy unified-comment section updater for verification results.
.github/skills/ai-summary-comment/scripts/post-try-fix-comment.ps1	Deletes legacy unified-comment section updater for try-fix attempts.
.github/skills/ai-summary-comment/scripts/post-pr-finalize-comment.ps1	Changes default behavior to inject into unified AI Summary comment; renames flag to -Standalone.
.github/skills/ai-summary-comment/scripts/post-ai-summary-comment.ps1	Adjusts how review sessions are wrapped and displayed (commit info moved to top-level summary).
.github/skills/ai-summary-comment/NO-EXTERNAL-REFERENCES-RULE.md	Terminology updates (“PR agent” → “agent”).
.github/scripts/shared/Update-AgentLabels.ps1	Terminology update in header; label parsing still reads gate/content.md.
.github/scripts/shared/Start-Emulator.ps1	Adds -Headless flag and uses headless mode by default in CI contexts.
.github/scripts/Review-PR.ps1	Major refactor to a 4-step skill-based orchestration flow.
.github/README-AI.md	Updates repository AI docs to point to the new skill-based workflow.
.github/instructions/sandbox.instructions.md	Updates guidance to reference pr-review skill instead of the old agent.
.github/docs/agent-labels.md	Updates documentation terminology and architecture description for the new workflow.
.github/copilot-instructions.md	Updates “available agents/skills” documentation to include pr-review and phase skills.
.github/agents/pr/SHARED-RULES.md	Removes legacy agent shared rules doc.
.github/agents/pr/post-gate.md	Removes legacy agent Phase 3/4 doc.
.github/agents/pr/PLAN-TEMPLATE.md	Removes legacy plan template for the old agent.
.github/agents/pr.md	Removes legacy PR agent definition.

Comments suppressed due to low confidence (1)

.github/skills/ai-summary-comment/SKILL.md:282

• The expected directory structure uses PRAgent/validate/, but the actual posting/labeling scripts auto-load from PRAgent/gate/ (e.g., post-ai-summary-comment.ps1 and Update-AgentLabels.ps1). Please align this doc with the implemented folder names, otherwise agents will write files to the wrong location and the summary/labels won’t pick them up.

The agent writes phase output files that comment scripts auto-load:

CustomAgentLogsTmp/PRState/{PRNumber}/
├── PRAgent/
│ ├── pre-flight/
│ │ └── content.md # Phase 1 output (auto-loaded by post-ai-summary-comment.ps1)
│ ├── validate/
│ │ ├── content.md # Phase 2 output (auto-loaded by post-ai-summary-comment.ps1)
│ │ └── verify-tests-fail/ # Script output from verify-tests-fail.ps1
│ │ ├── verification-report.md
│ │ ├── verification-log.txt
│ │ ├── test-without-fix.log
│ │ └── test-with-fix.log
│ ├── try-fix/

</details>

[PureWeen]

Code Review: Skills Architecture Refactoring

Overall: Good refactoring. The shift from agent-based to skill-based orchestration cuts ~3,800 lines, cleanly separates "what to do" (skill/phase files) from "how to post it" (scripts), and moves deterministic work out of LLM control. The 4-phase workflow is preserved intact.

---

🔴 Should fix before merge

1. git checkout - on error paths (Review-PR.ps1 lines 201/208/235)
In non-CI local mode, git checkout - toggles to main instead of the user's original branch after failures. CI mode is unaffected (auto-sets -UseCurrentBranch). Fix: capture original branch name before any checkout and use it explicitly on error paths.

�� Nice to fix

2. Fork fetch git errors swallowed (Review-PR.ps1 line 205)
git fetch ... 2>&1 | Out-Null suppresses the actual git error message. There IS a Write-Error with the fork URL, so it's not silent — but the reason for failure (auth, network, bad ref) is lost. Suggest capturing stderr.

3. git pull --ff-only non-fatal (Review-PR.ps1 lines 171-174)
If pull fails, script continues with only a yellow warning and could review stale code. Consider making it fatal or at least logging what commit is being used as the base.

4. Orphaned legacy parameters (post-pr-finalize-comment.ps1 lines 49-59)
10 legacy parameters are accepted but explicitly ignored. Should be removed to avoid confusion.

📋 Separate PR

5. provision.yml Xcode fallback (eng/pipelines/common/provision.yml)
Good change, but purely infrastructure — unrelated to the skills refactoring. Would be cleaner as its own PR.

✅ Already fixed

• Model count ×2/×4 inconsistency in Quick Reference table (pushed fix in 5ba6389)

✅ Things done well

• Autonomous execution rules prevent "waiting for human" hangs
• EstablishBrokenBaseline.ps1 auto-restore fixes the 3.7hr CI waste
• Phase files in .github/pr-review/ are easy to read and update independently
• post-ai-summary-comment.ps1 has robust downstream validation of phase outputs
• No stale references to deleted agents/pr/ or old scripts

[kubaflo]

Thanks for the review @PureWeen ! All actionable items addressed in 7755046:

1. ✅ git checkout - on error paths — Captured original branch before any checkout, use it explicitly on all 3 error paths instead of git checkout -.

2. ✅ Fork fetch errors swallowed — git fetch output is now captured and included in the Write-Error message so the failure reason (auth, network, bad ref) is visible.

3. ✅ git pull --ff-only non-fatal — Now logs the pull failure output and prints the base commit SHA (Review base: main @ <sha>) so it's clear what code is being reviewed.

4. ✅ Orphaned legacy parameters — Removed all 11 unused legacy parameters from post-pr-finalize-comment.ps1.

[PureWeen]

Re-Review (post-rebase)

All previously flagged issues have been fixed ✅ — git checkout - replaced with $originalBranch, fork fetch errors captured, legacy params removed, model count consistent at ×4.

New changes since last review

pr-build-status skill → azdo-build-investigator: Self-contained PowerShell scripts replaced by a thin MAUI-context layer that delegates to the ci-analysis skill from dotnet/arcade-skills plugin. Reasonable architecture — CI investigation logic is shared across dotnet repos rather than duplicated locally.

.github/copilot/settings.json: Registers the dotnet/arcade-skills plugin. This is the dependency powering the new skill. Looks fine.

🟡 One remaining issue

Stale references to deleted pr-build-status in 3 places:

• .github/README-AI.md (~line 253 and ~368)
• .github/copilot-instructions.md (~line 320)

These still list pr-build-status as a skill. Should be updated to reference azdo-build-investigator and document the new skill + its ci-analysis dependency.

Otherwise looks good to merge.

[kubaflo]

Stale pr-build-status references updated to azdo-build-investigator in 4aaa18a — all 3 spots in README-AI.md and copilot-instructions.md now reflect the new skill name and its ci-analysis dependency.

[PureWeen]

Multi-Model Code Review (5 LLMs × consensus filter)

Consensus Findings (2+ models agree)

🟡 MODERATE -- Review-PR.ps1 ~line 227-229 -- Incomplete cleanup on git commit failure (5/5 models)

When git merge --squash succeeds but git commit fails, the error path only deletes $tempBranch then exits. Missing:

• git reset --hard HEAD -- staged index left dirty
• git checkout $originalBranch -- script exits with HEAD on pr-review-$PRNumber
• git branch -D $reviewBranch -- orphan branch left behind

The conflict path (lines 237-243) handles all three correctly. Suggested fix -- mirror it:

if ($LASTEXITCODE -ne 0) {
    git branch -D $tempBranch 2>$null
    git reset --hard HEAD 2>$null
    git checkout $originalBranch 2>$null
    git branch -D $reviewBranch 2>$null
    Write-Error "Failed to create squashed commit"; exit 1
}

---

🟡 MODERATE -- provision.yml ~line 146-158 -- Silent Xcode fallback affects all CI pipelines (4/5 models)

Old behavior: exit 1 with clear message when requested Xcode missing.
New behavior: Warning + silently selects latest installed Xcode.

This affects all pipelines consuming provision.yml (ci.yml, ci-official.yml, device-tests.yml, handlers.yml -- 11 consumers), not just ci-copilot.yml. Consider scoping via opt-in env var or adding ##vso[task.logissue type=warning] for AzDO visibility.

---

Review generated by 5-model consensus (claude-opus-4.6 ×2, claude-sonnet-4.6, gemini-3-pro-preview, gpt-5.3-codex). Only issues flagged by 2+ models are included.

[PureWeen]

Multi-Model Re-Review (Gemini 3 Pro, GPT 5.1, Sonnet 4.5)

Previous fixes all confirmed intact ✅ (originalBranch, fork errors, legacy params, model count)

Stale pr-build-status references also fixed in latest commit ✅

New findings from multi-model review

🟡 Invoke-CopilotStep exit codes ignored (Review-PR.ps1 lines 447, 461)
If Step 1 (PR Review) fails, the script proceeds to Step 2 (PR Finalize) without checking. Consider checking $LASTEXITCODE or the return value and at least logging a clear warning.
(Found by: Gemini, GPT)

🟡 Process cleanup too aggressive (Review-PR.ps1 lines 539-543)
Get-Process -Name "copilot" | Stop-Process kills ALL copilot instances on the machine, not just the one started by this script. On a dev box, this could terminate other Copilot sessions. Consider scoping by PID or skipping cleanup when not in CI.
(Found by: Gemini, GPT)

🟡 Try-Fix mandatory vs skip contradiction (SKILL.md)
Lines 96-99 say Phase 3 is "MANDATORY… NEVER SKIP… NO EXCEPTIONS", but the Gate failure path (lines 86-89) says "Tests FAIL with fix → Skip Try-Fix, proceed to Report." These should be reconciled.
(Found by: GPT)

🟢 Minor: README-AI.md skill listing
Lists pr-preflight, pr-gate, pr-report as standalone skills (~lines 62-65, 78-82). These are phase docs in .github/pr-review/, not skills. Could be misleading.
(Found by: GPT)

Overall

All three models agreed this is a good refactoring with clean architecture. The items above are the only net-new findings beyond what was already addressed.

[kubaflo]

Re: Multi-Model Re-Review Findings

Thanks for running the multi-model review — appreciate the thoroughness. However, I disagree with most of the findings here. Let me explain:

---

🟡 Invoke-CopilotStep exit codes — Disagree, by design

Steps 1 and 2 are intentionally independent. Step 1 (PR Review) does code review + test verification. Step 2 (PR Finalize) checks title/description accuracy. These serve completely different purposes.

If Step 1 crashes or times out, Step 2's analysis is still valuable — knowing the PR title doesn't match the implementation is useful regardless of whether code review completed. Adding exit code gating would create unnecessary coupling between independent phases and reduce the overall value of a partial run.

The downstream script (post-ai-summary-comment.ps1) already handles missing phase output gracefully — if a phase produced no content files, that section is simply omitted from the comment. So a Step 1 failure doesn't corrupt Step 2 output.

---

🟡 Process cleanup — Disagree, correctly scoped for its context

This cleanup runs at the very end of Review-PR.ps1, which is the CI orchestrator. By this point, ALL Invoke-CopilotStep calls have completed — any remaining copilot or copilot-related node processes are orphans from this script's own execution.

---

🟡 Try-Fix "contradiction" — Disagree, not a contradiction

These target different scenarios:

• Line 88 ("Skip Try-Fix"): Gate found the fix is broken — tests FAIL with the fix applied. Running 4 Try-Fix models against a broken baseline would be meaningless since there's nothing working to compare alternatives against. Going straight to Report is the only sensible action.

• Line 96 ("MANDATORY, NEVER SKIP"): This targets the LLM's tendency to shortcut. When Gate passes (fix works, tests pass), the LLM naturally wants to say "fix looks correct, no need to explore alternatives." The emphatic language prevents that — you MUST explore alternatives even when the fix looks perfect.

These aren't contradictory — line 88 is a degenerate failure case, line 96 is about the happy path. The "NO EXCEPTIONS" applies to the scenario where Try-Fix would actually be meaningful.

---

🟢 README phase listing — Agree, minor doc fix

Fair point — I'll clarify that pr-preflight, pr-gate, and pr-report are phase docs consumed by the pr-review orchestrator, not independently invokable skills.