Document automated PR review workflow

[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!

Description of Change

Adds a guide for Microsoft maintainers and community contributors explaining the automated PR review workflow:

• /review
• /review <platform>
• /review rerun
• /review tests

The guide also explains the maui-copilot pipeline flow, AI Summary and Test Failure Review comments, local /review tests usage, troubleshooting, and related implementation files.

Companion review-output changes

Beyond the documentation, this PR also refines the review-comment output to keep it consistent with the guide and with the project's review-symbol conventions:

• Find-RegressionRisks.ps1 — distinct colorless severity glyphs in the regression markdown (✗ REVERT / ⚠ OVERLAP / ● CLEAN).
• post-ai-summary-comment.ps1 — Test-PhaseContentIsNoOp recognizes the ● (and legacy 🟢) regression no-op markers so empty sections stay suppressed; Next Steps renders below Review Sessions.
• post-inline-review.ps1 — inline-comment marker uses a generic (multi-model) label instead of a hardcoded model roster.
• pr-preflight.md — review-symbol set aligned (✗/⚠/ℹ).

Producer/consumer pairs (e.g. the regression no-op markers) are kept in lock-step and covered by Post-AISummaryComment.Tests.ps1.

Issues Fixed

No issue filed.

[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 -- 35704

Or

• Run remotely in PowerShell:

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

[kubaflo]

/review -b feature/enhanced-reviewer -p android

[MauiBot]

Expert Review — 4 findings

See inline comments for details.

[kubaflo]

Updated this branch to current main (it was 103 commits behind). All four documentation-correctness findings from the automated review were stale artifacts of the outdated base — every referenced path now exists on main:

• .github/workflows/copilot-review-tests.md / .lock.yml — gh-aw source/compiled workflow for /review tests
• .github/scripts/Review-Tests.ps1 — local runner for /review tests
• .github/skills/review-test-failures/SKILL.md — test-failure classification rubric

The /review tests exclusion claim is also accurate now: review-trigger.yml (lines 66–68) explicitly sets matched=false for /review tests before the general /review match, so the two workflows don't both run. No doc edits were needed. Ready for re-review.

[kubaflo]

🤖 AI Agent — Updated documentation to reflect recent workflow changes:

Changes documented:

1. Command minimization (Hide /review, /review rerun, and /review tests command comments as resolved #35895, Always minimize /review, /review rerun, and /review tests command comments once authorized #36021): All /review, /review rerun, and /review tests commands are now minimized (collapsed as "Resolved") after authorization
2. Rerun eligibility (Restrict /review rerun eligibility to author activity #35874): Only PR author activity (commits or non-command comments) triggers reruns; reviewer comments no longer satisfy eligibility
3. Automated scanner (Add gh-aw rerun review scanner #35685): Hourly gh-aw workflow processes queued reruns automatically
4. Troubleshooting: Added entries for rerun eligibility and command visibility issues

The documentation now accurately describes the current state of the review automation system. Ready for review! 📚

[PureWeen]

Adversarial review — PR #35704

Methodology: 3 independent reviewers (different model families) with adversarial consensus. Because this branch is behind main and its local infra files are stale, every finding was cross-checked against origin/main source (the actual merge target), not the local checkout.

This is a documentation-only PR, so the entire concern is accuracy. The prior automated review's 4 findings were stale-base artifacts and are genuinely resolved — on current main every referenced path exists, the command-comment minimization feature is implemented (PRs #35895 / #36021), and review-trigger.yml really does exclude /review tests. Also verified accurate against main: all 12 referenced file paths, platform-label inference, the hourly scanner cron, Review-Tests.ps1 params/output paths, and the /review tests comment-only behavior. Nicely written guide overall.

The findings below are remaining accuracy gaps versus current main:

Must fix

• ❌ line 113 — /review rerun does not run the pipeline directly; it applies the s/agent-ready-for-rerun label and the hourly scanner triggers AzDO (may be delayed ~1h or skipped). 3/3, verified.
• ❌ lines 17 & 20 — /review rerun permission understated: a non-first-time PR author can rerun their own PR with no write access. 3/3, verified.

Should fix

• ⚠️ lines 107 & 246 — Older AI Summary sessions are not retained; the script posts a PR review and hides older ones, keeping only the latest session. Confirmed via source.
• ⚠️ lines 252–256 — The comment's top-level heading is ## Tests Failure Analysis, not ## Test Failure Review (that's only the <summary> label). 2/3, verified.
• ⚠️ lines 96–105 — Review-PR.ps1 phase list has an extra phase and the regression/gate order swapped vs the 7-step script header. 3/3, verified.

Consider

• 💡 line 90 — The pipeline actually has a 4th CleanupReviewLock stage. The "at a high level … three stages" framing is defensible (one reviewer considered it fine), but you could mention the cleanup stage for completeness. 2/3.

Test coverage: N/A — documentation only, no executable code paths.

Prior reviews: MauiBot's earlier expert review (4 findings) was resolved by the branch update to current main; those are not re-raised here.

3 independent reviewers with adversarial consensus. Posted as a comment (no approve / request-changes).

[kubaflo]

🤖 AI Agent — Addressed all review findings in ead1342b6d:

Must-fix:

• ✅ line 113 — Documented that /review rerun applies s/agent-ready-for-rerun label and is processed by hourly scanner (may delay ~1h), not direct pipeline trigger
• ✅ lines 17 & 20 — Fixed permission documentation: non-first-time PR authors can rerun their own PR without write access

Should-fix:

• ✅ lines 107 & 246 — Corrected: older AI Summary sessions are hidden as outdated, not retained side-by-side
• ✅ lines 252–256 — Fixed: test comment heading is ## Tests Failure Analysis, not ## Test Failure Review
• ✅ lines 96–105 — Removed phantom phase from Review-PR.ps1 phase list and fixed regression/gate order (now matches script header)

Consider:

• line 90 (4th cleanup stage) — Kept as-is. The "three stages" framing is accurate for user-facing workflow (ReviewPR → RunDeepUITests → UpdateAISummaryComment). The CleanupReviewLock stage is infrastructure cleanup not directly relevant to understanding the review flow.

All findings have been addressed. Ready for re-review! 🎯

[PureWeen]

Round 2 — multi-model adversarial re-review. This follow-up uses 3 independent reviewers with adversarial consensus. It builds on my prior review (pullrequestreview-4547825053).

Round-1 findings: resolved. ✅ All five documentation findings from the prior review were correctly addressed (rerun mechanics + s/agent-ready-for-rerun/rerun-review-scanner, rerun permissions for non-first-time authors, session replace/hide wording, the ## Tests Failure Analysis heading, and the Review-PR.ps1 phase list). Verified against origin/main. Thank you.

What changed since round 1: the PR grew from a docs-only change to also modify four production files (Find-RegressionRisks.ps1, post-ai-summary-comment.ps1, post-inline-review.ps1, pr-preflight.md) via the "Improve emoji choices and reorder review structure" commit. Those changes introduce one real regression and a few smaller issues.

Findings (ranked)

❌ Regression — 🟢→● swap breaks regression-section no-op suppression (3/3 reviewers)
Find-RegressionRisks.ps1 now emits ● No regression risks detected… / ● No implementation files modified… (lines 797, 346), but the consumer post-ai-summary-comment.ps1 → Test-PhaseContentIsNoOp still matches those strings with regexes anchored on the literal 🟢 (lines ~100–101). For the common case (no regression risk), the no-op check now returns false, so an empty "Regression Cross-Reference → ● No regression risks detected" block leaks into every AI Summary comment instead of being suppressed. The unit tests don't catch this because Post-AISummaryComment.Tests.ps1 fixtures (lines 63, 68) still feed the old 🟢 strings. Fix: update the two regexes (e.g. ^[●🟢]\s+…) and the test fixtures, keeping producer and consumer in lock-step. (Inline on line 797.)

⚠️ Logic — section reorder is the inverse of the commit's stated intent (3/3 reviewers; functionally harmless)
The commit message says "Move Next Steps section below Review Sessions (user feedback)", but before this change $futureActionSection (Next Steps / Future Action) was already below $newSessionBlock (Review Sessions). The swap moves Next Steps above Review Sessions — the opposite of the message. The reorder itself is safe (the <!-- SESSION:… --> markers stay intact, so older-session hiding still works); the concern is that code and stated intent disagree. Fix: confirm which order the feedback asked for; if Next Steps should be below, swap the two blocks back. (Inline on line 623.)

💡 Information design — ● collapses three severities into one glyph in colorless output (2/3 reviewers)
In console output the change is cosmetic (Write-Host keeps -ForegroundColor), but the markdown content.md / PR-comment paths (REVERT, OVERLAP, CLEAN at lines 735/770/797) have no color, so all three render as an identical ● — the at-a-glance severity cue is lost. Mitigated by distinct text and mutually-exclusive branches. Fix (optional): use distinct colorless glyphs (e.g. the ✗/⚠/ℹ set already adopted in pr-preflight.md).

💡 Documentation — troubleshooting row contradicts the corrected session wording (1/3 reviewers, orchestrator-verified)
The Troubleshooting table still says "The comment preserves prior review sessions for traceability" (line 298), which conflicts with the now-corrected "New runs replace the review and hide older sessions" / "minimized and hidden as outdated" elsewhere in the same doc. Looks like a spot missed when fixing the round-1 session-retention finding. Fix: reword to match (older sessions are minimized/hidden as outdated). (Inline on line 298.)

💡 Maintainability — hardcoded model roster in inline-comment marker (3/3 reviewers)
post-inline-review.ps1 line 168 prepends > 🔍 **AI-Generated Review** (multi-model: GPT-5.5, Claude Opus 4.8, Claude Opus 4.6, Gemini 3.1 Pro) to every comment. This list isn't sourced from runtime/config and will silently go stale (and arguably already differs from the documented try-fix model set). Fix: use a generic "multi-model AI review" label or inject the active model list from run metadata. (Inline on line 168.)

Non-blocking note

The PR title and description still present this as documentation-only ("Adds a guide…") with no mention of the four production-script/template changes. Consider updating the description so the script changes are reviewable in context.

Test coverage

The behavioral change in Find-RegressionRisks.ps1 has no test exercising the new ● output; the existing Post-AISummaryComment.Tests.ps1 fixtures still assert against 🟢, which is why the regex regression isn't caught. Updating those fixtures alongside the fix would close the gap.

Methodology: 3 independent reviewers with adversarial consensus; consensus counts shown per finding. Factual claims verified against origin/main. Posted as a comment — not an approval or change request.

[kubaflo]

🔍 This is an AI-generated comment posted on @kubaflo's behalf.

@PureWeen — Round 2 findings addressed in ac2dc2e77c. Ready for re-review. 🎯

#	Finding	Status
❌ 3/3	🟢→● broke regression no-op suppression	Fixed — both Test-PhaseContentIsNoOp regexes now ^(?:●|🟢)\s+…; fixtures refreshed to ● + 🟢 back-compat
⚠️ 3/3	Section order inverse of commit intent	Fixed — Next Steps now below Review Sessions
💡 3/3	Hardcoded model roster in inline marker	Fixed — generic (multi-model) label
💡 2/3	● collapses 3 severities in colorless md	Fixed — ✗ REVERT / ⚠ OVERLAP / ● CLEAN
💡 1/3	Troubleshooting row contradicts session wording	Fixed — reworded to minimized/hidden

Note on the surrogate-pair gotcha: I initially used [●🟢], but the back-compat test caught that 🟢 (U+1F7E2) is an astral surrogate pair that splits inside a character class — switched to the (?:●|🟢) alternation, which matches the full pair.

Validation: full .github/scripts Pester suite 241/241 pass (the regression now has explicit coverage: the ● no-op fixtures + the kept-content ✗/⚠ fixtures).

I also updated the PR description to cover the production-script changes (per your non-blocking note). All 4 inline threads replied + resolved. Thanks for the thorough re-review!

[kubaflo]

🔍 This is an AI-generated comment posted on @kubaflo's behalf.

Refreshed this branch against the latest main (it was 23 commits behind) via a clean fast-forward merge — no conflicts. The PR's net diff is unchanged: still exactly the same 6 files (the pr-review-workflow.md guide plus the companion review-output scripts and tests).

@PureWeen — the branch is now current and ready for another look whenever you have a chance. Thanks!

[kubaflo]

🔍 This is an AI-generated comment posted on @kubaflo's behalf.

@PureWeen — friendly nudge for re-review. All Round-2 findings are addressed in ac2dc2e (regression no-op (?:●|🟢) regex + fixtures, Next Steps below Review Sessions, generic (multi-model) marker, ✗/⚠/● severity glyphs, troubleshooting wording). The subsequent MauiBot review found no new code findings — its CHANGES_REQUESTED is only the INCONCLUSIVE gate (a docs+scripts PR has no device/UI tests to run). Ready for re-review when you have a moment — thanks!

[PureWeen]

Adversarial multi-model re-review — Round 3

Methodology: 3 independent reviewers (different model families) with adversarial consensus, scoped strictly to this PR's changed files and verified against origin/main (the merge target).

Round-2 findings — all verified resolved ✅

All five prior-round findings are genuinely fixed in ac2dc2e77c (not merely reworded). One reviewer empirically executed Test-PhaseContentIsNoOp against every updated fixture to confirm:

1. ❌ Regression (regex no-op suppression) — Resolved. The consumer regex now uses ^(?:●|🟢)\s+… alternation, matching both the new ● producers (Find-RegressionRisks.ps1:346,797) and legacy 🟢; the empty "no regression risks" section is suppressed again. The author's choice of alternation over a [●🟢] character class is the correct fix — 🟢 (U+1F7E2) is a surrogate pair that splits inside [], so a char class would mis-match (verified empirically, 3/3).
2. ⚠️ Logic (section order) — Resolved. $newSessionBlock now precedes $futureActionSection; SESSION markers still wrap the block as a contiguous unit, so rerun/replace parsing is unaffected.
3. 💡 Maintainability (hardcoded model roster) — Resolved. Marker is now (multi-model).
4. 💡 Info design (severity glyphs) — Resolved. ✗/⚠/● distinguish REVERT/OVERLAP/CLEAN in colorless markdown; downstream consumers key off risks.json/result.txt, never the glyph, so nothing breaks.
5. 💡 Documentation (troubleshooting row) — Resolved; wording now matches the minimize-and-hide behavior.

New findings this round

⚠️ Documentation — "expanded by default" contradicts the collapsed <details> (2/3, verified) — see inline on line 247. Doc lines 247 and 298 claim the latest session is expanded, but the code emits a plain <details> (no open) with a "click to expand" summary, so it renders collapsed. The <details> block is pre-existing on main, so the in-scope fix is the doc (or emit <details open> if expanded-by-default is intended).

💡 Testing — fixtures don't exercise the real producer (1/3 formal + 1/3 corroborated). Post-AISummaryComment.Tests.ps1 asserts against static string literals that happen to match current producer output, rather than feeding the actual content.md from Find-RegressionRisks.ps1 into Test-PhaseContentIsNoOp. A future producer-only glyph change (e.g. ●→○) would silently reintroduce the round-2 regression while the suite stays green. A small integration test that runs the producer and asserts suppression would make the coupling self-policing. Non-blocking — a pre-existing test-architecture gap, not a regression in this commit.

Test coverage

The new Post-AISummaryComment.Tests.ps1 adds direct coverage of the consumer regex (including a 🟢 back-compat case) — a real improvement over round 2, where no test caught the regression. The remaining gap is the producer/consumer integration noted above.

Prior reviews

No prior human review flagged the "expanded by default" inconsistency. Existing MauiBot reviews are the pipeline's own auto-generated AI Summary, not human findings.

Verdict: Round-2 findings verified resolved; no new functional bugs. One ⚠️ doc inconsistency and one 💡 testing suggestion remain — both non-blocking.

---

3 independent reviewers · adversarial consensus · CI status out of scope.

[kubaflo]

🔍 This is an AI-generated comment posted on @kubaflo's behalf.

@PureWeen — thanks for the Round-3 pass and for confirming all five Round-2 findings resolved. Addressed the new ⚠️ in bad5f62c76:

• Documentation (247/298) — Verified the session block is a plain <details> (renders collapsed), so the "expanded by default" wording was wrong. Reworded the doc to say the Review Sessions section is collapsed/click-to-expand. Inline thread resolved.
• 💡 Testing (producer/consumer integration) — Acknowledged. Agree it's a worthwhile self-policing improvement; since you flagged it as a non-blocking, pre-existing test-architecture gap (the static fixtures happen to match current producer output), I've left it out of this docs-focused PR rather than expand scope. Happy to follow up with a small integration test that runs Find-RegressionRisks.ps1 and asserts suppression if you'd like it here.

Ready for re-review.

[PureWeen]

Adversarial multi-model re-review — Round 4

Methodology: 3 independent reviewers (different model families) with adversarial consensus, scoped to this round's change and verified against the actual source.

Round-3 finding — verified resolved ✅

The new commit bad5f62c76 correctly fixes the round-3 ⚠️ Documentation finding: "The latest session is expanded by default" → "collapsed by default — expand the Review Sessions section." This now matches the code, which emits a plain <details> (no open) with a "click to expand" summary. The troubleshooting row (line 298) was updated consistently. 3/3 reviewers confirmed this part is accurate.

New finding this round

⚠️ Documentation — the replacement clause introduces a fresh inaccuracy (3/3 reviewers; one rated ❌) — see inline on line 247.

While fixing the collapsed/expanded wording, the commit also added: "Within it, the latest session appears first; older sessions are minimized and hidden as outdated." All three reviewers independently traced the code and found this contradicts reality: each posted AI Summary contains exactly one session (the current HEAD commit), so there is no latest-first ordering "within it," and older sessions are separate prior comments/reviews minimized via Hide-StaleMauiBot* — not nested inside the Review Sessions block. A reader who expands the section expecting a history of minimized older runs will find only one session and may think history was lost. Suggested rewording in the inline comment.

This is a documentation precision issue on a docs-only change — non-blocking, but worth a one-line tweak before merge so the doc matches the script's own synopsis ("the replacement review contains only the latest review session, keyed by the current HEAD commit SHA").

Multi-round provenance

This finding targets wording introduced by bad5f62c76 itself (the round-3 fix). Per self-correction review, it was independently confirmed by all three reviewers with concrete code citations and re-verified against source by the orchestrator.

Prior reviews / test coverage

No prior human review flagged this clause. The 4 supporting PowerShell/template files are unchanged since round 3 (all round-2 findings remain resolved) and were not re-reviewed this round. Existing MauiBot reviews are the pipeline's own auto-generated AI Summary, not human findings.

Verdict: The round-3 fix is correct; one new ⚠️ documentation inaccuracy was introduced by the same commit. Non-blocking but recommended to correct.

---

3 independent reviewers · adversarial consensus · CI status out of scope.

[kubaflo]

🔍 This is an AI-generated comment posted on @kubaflo's behalf.

@PureWeen — thanks for the Round-4 catch. Fixed in 11c74c41b1: the round-3 wording I added ("latest session appears first; older sessions minimized within it") implied a nested history that doesn't exist. Reworded so the doc matches the code — each AI Summary comment holds only the latest session (keyed to its HEAD commit), and previous review comments are the ones minimized/hidden as outdated. Aligned the troubleshooting row too. Inline thread resolved. Ready for re-review.

[PureWeen]

Adversarial multi-model re-review — Round 5

Methodology: 3 independent reviewers (different model families) with adversarial consensus, verified against the actual source.

Round-4 finding — verified resolved ✅ (3/3, unanimous)

Commit 11c74c41b1 correctly fixes the round-4 ⚠️ Documentation finding. The previous wording ("Within it, the latest session appears first; older sessions are minimized and hidden as outdated") falsely implied a nested, ordered history inside the Review Sessions block. The new wording accurately describes the one-session-per-comment model, and all three reviewers independently confirmed it against post-ai-summary-comment.ps1:

• Line 247 — "collapsed by default" ✓ (the <details> block has no open attribute); "expand … to read the latest session, which is keyed to the current HEAD commit" ✓ (the comment body contains exactly one $newSessionBlock, wrapped in <!-- SESSION:$commitSha7 --> for the current HEAD); "Previous review comments are minimized and hidden as outdated" ✓ (prior AI Summary comments/reviews are hidden as separate comments via Hide-StaleMauiBotIssueComments / Hide-StaleMauiBotPullRequestReviews, not nested in the block).
• Line 298 (troubleshooting row) — "Each comment holds only the latest session (keyed to its HEAD commit)" and "Expand the Review Sessions section in the newest comment" ✓. Accurate and consistent with line 247.

No new issues

All three reviewers returned "Fix correct and complete; no new issues." The 4 supporting PowerShell/template files are unchanged since round 3 (all round-2 findings remain resolved) and were not re-reviewed this round. The doc now matches the script's own synopsis ("the replacement review contains only the latest review session, keyed by the current HEAD commit SHA").

Verdict: All findings from rounds 2–4 are verified resolved; no outstanding issues from this reviewer. 👍

---

3 independent reviewers · adversarial consensus · CI status out of scope.