Enhance MauiBot AI summary review output + support net11-targeting PRs

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

Summary

• Posts MauiBot AI Summary output as a pull request review with parsed APPROVE, REQUEST_CHANGES, or safe COMMENT fallback.
• Uses the new AI Review Summary layout with segmented status chips, collapsed review sessions, and merged Future Action content.
• Keeps PR finalization out of the automated review process; AI Summary updates no longer preserve or merge SECTION:PR-FINALIZE blocks.
• Adds visible AI Summary guidance telling users to comment /review rerun after new comments or commits when they want a fresh review. The command implementation is intentionally split into a follow-up PR.
• Hides stale MauiBot AI Summary / try-fix artifacts with GitHub minimization instead of deleting them, while preserving same-run try-fix and AI Summary reviews.
• Updates the Copilot pipeline to pass review IDs and patch review bodies after deep UI tests.
• Hardens gate setup/retry handling by committing squashed PR changes before verification, resetting the review branch before gate retries, and detecting BlazorWebView unit-test project paths.

Validation

• Parsed changed PowerShell scripts with System.Management.Automation.Language.Parser.
• Parsed .github/workflows/review-trigger.yml as YAML.
• Invoke-Pester .github/scripts/Post-AISummaryComment.Tests.ps1,.github/scripts/Remove-StaleMauiBotComments.Tests.ps1 -CI
• Dry-run AI Summary generation verified the rerun note, segmented chips, and collapsed review session layout.
• Verified Detect-TestsInDiff.ps1 maps src/BlazorWebView/tests/MauiBlazorWebView.UnitTests/UriExtensions_Tests.cs to src/BlazorWebView/tests/MauiBlazorWebView.UnitTests/MauiBlazorWebView.UnitTests.csproj.

net11.0-targeting PR support

This branch also makes the Copilot review pipeline handle net11.0-targeting PRs as well as main/net10, from a single pipeline branch (consolidates and supersedes #35994):

• Runtime base-branch auto-detection (eng/pipelines/ci-copilot.yml): the CopilotReview and DeepUITests stages read the PR's baseRefName via gh, allowlist-validate it (^(main|net[0-9]+\.0)$), and check out that base before workload install and the squash-merge — so workloads (net11 11.0.100 vs net10 10.0.100) and the merge base follow the PR. Trusted scripts are captured from the pipeline ref first (security rule 3); Task 1 Setup runs from $TRUSTED.
• Branch-aware test TargetFramework: BuildAndRunHostApp.ps1 and run-device-tests/Run-DeviceTests.ps1 now derive the TFM from Directory.Build.props (Get-MauiTfmVersion in shared-utils.ps1) instead of hardcoding net10.0, so deep-UI/device tests build net11.0-android on net11. The DeepUITests stage restores the reviewed pipeline-branch scripts over the worktree before the per-category loop.

main/net10 behavior is unchanged by design (base-detection checks out main = the original flow; global.json stays 10.0.108). review-trigger.yml and Review-PR.ps1 are intentionally untouched.

Validation: 8 live net11 runs with 100% correct base detection (e.g. #35891 gate ran net11 Core tests 44/45 → 45/45, conflict-free squash-merge onto net11.0); reproduced and root-caused a separate net11.0-baseline MAUIX2017 Xaml.UnitTests build break (unrelated to this change). Confirmation runs from this branch cover one net11 PR and one main PR.

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

Or

• Run remotely in PowerShell:

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

[github-actions]

Skill Validation Results

@kubaflo — new skill validation results are available based on this last commit: 34738c8.
To request a fresh validation after new comments or commits, comment /evaluate-skills.

❌ Skill Validation Results — 34738c8 · Enhance MauiBot AI summary review output + support net11-targeting PRs · 2026-06-29T17:03:43Z

✅ Static Checks Passed

Skills: 20 | Eval specs linted: 7

Full lint output

── .github/skills/agentic-labeler/tests/eval.vally.yaml
npm warn deprecated prebuild-install@7.1.3: No longer maintained. Please contact the author of the relevant native addon; alternatives are available.
✔ .github/skills/agentic-labeler/tests/eval.vally.yaml is valid
── .github/skills/code-review/tests/eval.capability.vally.yaml
✔ .github/skills/code-review/tests/eval.capability.vally.yaml is valid
── .github/skills/code-review/tests/eval.vally.yaml
✔ .github/skills/code-review/tests/eval.vally.yaml is valid
── .github/skills/code-review/tests/hermeticity.vally.yaml
✔ .github/skills/code-review/tests/hermeticity.vally.yaml is valid
── .github/skills/evaluate-pr-tests/tests/eval.vally.yaml
✔ .github/skills/evaluate-pr-tests/tests/eval.vally.yaml is valid
── .github/skills/try-fix/tests/eval.vally.yaml
✔ .github/skills/try-fix/tests/eval.vally.yaml is valid
── .github/skills/verify-tests-fail-without-fix/tests/eval.vally.yaml
✔ .github/skills/verify-tests-fail-without-fix/tests/eval.vally.yaml is valid

❌ LLM Evaluation Failed

🔍 Full results and investigation steps

[kubaflo]

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

[MauiBot]

Expert Review — 1 findings

See inline comments for details.

[MauiBot]

⚠️ Merge Conflict Detected — This PR has merge conflicts with its target branch. Please rebase onto the target branch and resolve the conflicts.

[Copilot]

Pull request overview

This PR enhances the MauiBot/Copilot PR review pipeline output and robustness, and adds support for PRs targeting net11.0 (branch-aware base checkout + TFM selection) while improving deep UI test resilience and diagnostics.

Changes:

• Add base-branch auto-detection/validation in ci-copilot.yml so workloads + merge base follow the PR’s actual target branch (e.g., net11.0).
• Improve deep UI test reliability and reporting (timeouts around adb, crash/fixture-setup classification, richer artifacts, time-budget guardrails).
• Add telemetry aggregation/publishing for Copilot token usage plus supporting tests and label/outcome parsing hardening.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/TestUtils/src/UITest.Appium/Actions/AppiumLifecycleActions.cs	Adds an Appium command to attempt crash recovery (Android-focused) via adb + relaunch.
src/Controls/tests/TestCases.Shared.Tests/UtilExtensions.cs	Uses crash-recovery-aware wait for “Go To Test” to reduce cascading OneTimeSetUp failures.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/_IssuesUITest.cs	Switches issue navigation to the new recovery-aware “Go To Test” wait.
eng/scripts/detect-ui-test-categories.ps1	Skips redundant fork-head checkout when already on prepared review worktree; adds dependency-bump smoke category fallback.
eng/pipelines/ci-copilot.yml	Adds base-branch validation/checkout, more robust adb handling, trusted gate verdict propagation, deep UI test budget/diagnostics, and token-usage stage.
.github/workflows/copilot-review-tests.md	Tweaks review-trigger text formatting for rerun instructions.
.github/workflows/copilot-review-tests.lock.yml	Regenerates gh-aw lock metadata for the workflow doc change.
.github/skills/verify-tests-fail-without-fix/scripts/Verify-TestsFail.Tests.ps1	New Pester tests for build-error classification (ensures “build broke” => INCONCLUSIVE).
.github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1	Improves env/build error detection, retry recovery, and supports PR-deleted fix files.
.github/skills/run-device-tests/scripts/Run-DeviceTests.ps1	Makes device-test TFMs branch-aware (net10 vs net11).
.github/skills/review-test-failures/SKILL.md	Updates the skill template wording/formatting.
.github/scripts/shared/Update-AgentLabels.Tests.ps1	New tests for label derivation from authoritative artifacts (winner.json, gate-result.txt).
.github/scripts/shared/Update-AgentLabels.ps1	Uses gate-result.txt + winner.json as primary sources instead of prose parsing.
.github/scripts/shared/shared-utils.ps1	Adds Get-MauiTfmVersion to derive MAUI TFM version from repo metadata.
.github/scripts/shared/Get-AggregatedTrxFromDirectory.ps1	Adds app-crash cascade detection/flagging for deep UI TRX aggregation.
.github/scripts/shared/Aggregate-UITestArtifacts.Tests.ps1	Adds coverage for new “app crash cascade” classification behavior.
.github/scripts/shared/Aggregate-UITestArtifacts.ps1	Mirrors app-crash cascade detection/flagging in the shared aggregator.
.github/scripts/shared/Aggregate-CopilotTokenUsage.ps1	New script to aggregate Copilot telemetry records into publishable artifacts.
.github/scripts/Review-Tests.ps1	Adjusts badge coloring and consolidates rerun guidance into one line.
.github/scripts/Review-PR.Tests.ps1	Adds tests for Copilot token usage helpers and AzDO log-command defanging.
.github/scripts/Post-AISummaryComment.Tests.ps1	Expands tests for gate-status mapping, approve veto behavior, and UI-results annotations.
.github/scripts/post-ai-summary-comment.ps1	Updates AI summary rendering, gate/status chip behavior, approve veto logic, and UI-results annotation.
.github/scripts/DetectUITestCategories.Tests.ps1	New tests for prepared-review-worktree detection helper.
.github/scripts/BuildAndRunHostApp.ps1	Makes HostApp TFMs branch-aware; adds Android warm-up and Windows app-path resolution.
.github/scripts/Aggregate-CopilotTokenUsage.Tests.ps1	New Pester tests validating token-usage aggregation outputs.

[kubaflo]

🔍 AI-generated reply on @kubaflo's behalf.

Valid refinement — flagging as a follow-up rather than changing it blind, since it touches the deep-UI security orchestration on a branch with concurrent work. Context: the copy at :1360-1362 is deliberately security-positive today (it replaces the PR's .github/scripts with the trusted main versions before the per-category loop, per the comment citing rule 3), so PR scripts never execute. Your point stands that the trusted copy then lives in a PR-writable worktree (chmod u+w) and could in principle be mutated by a later untrusted dotnet test within the job. The clean fix is to invoke the deep-UI scripts directly from $TRUSTED (read-only) and drop the cp-back entirely — but that requires rewiring every .github/scripts/… call in the stage (retry wrapper, aggregators, label/token scripts) to $TRUSTED/… while keeping CWD on the worktree src/. I'd rather land that as its own validated change. Leaving open.

[kubaflo]

🔍 AI-generated reply on @kubaflo's behalf.

Same root cause as the trusted-copy thread above — the per-category retry wrapper resolves from .github/scripts/shared/… (worktree) rather than $TRUSTED. Mitigated today by the trusted-over-worktree restore at :1360, but the proper fix is to resolve it (and the other stage scripts) from $TRUSTED and fail closed if absent. Tracking with the companion thread; leaving open for a dedicated, pipeline-validated change rather than a partial edit on a contested branch.

[PureWeen]

Multi-model adversarial review ⚠️

Methodology: 3 independent reviewers (different frontier models) analyzed this round in parallel, with adversarial cross-validation — single-reviewer findings were sent to the other two models for adjudication before inclusion. This is review round 12 (HEAD 8eca146f).

The security architecture held up under scrutiny again. The findings below are all bounded by the same fact: MauiBot's posted review event (APPROVE/COMMENT/REQUEST_CHANGES) is advisory — human approval is still required and the real merge gate is the maui-pr build. So none of these are merge-blocking risks; they're edge cases where the advisory APPROVE could be slightly too generous.

---

✅ Verified safe (trust chain held)

Spot-checked and confirmed correct by multiple reviewers:

• Gate trust chain — Get-AIReviewEventForRun fails closed on an empty TrustedGateResult (throws), keys the APPROVE veto off the trusted pipeline verdict (not the agent-writable gate-result.txt), and is covered by a test asserting a forged gate-result.txt cannot flip a trusted PASSED. Solid.
• Trusted-script overlay / DeepUITests / fork-head handling — the fork head is never checked into the trusted worktree; detect-ui-test-categories.ps1 skips fork checkout (trusted-gated).
• Aggregate-CopilotTokenUsage.ps1 — null-safe sums, and the path filter requires /copilot-token-usage/raw/ while excluding the PR/agent-writable CustomAgentLogsTmp + agent-pr-session dirs.
• AppiumLifecycleActions.RunAdb / WaitForGoToTestButtonWithRecovery — RunAdb uses ArgumentList (no shell injection) with a 15s timeout+Kill; the recovery helper rethrows on genuine failure (does not swallow).
• The round-11 💡 (the tools.dotnet-is-a-proxy note) was addressed — thanks.

---

⚠️ A non-compiling fix is reported as INCONCLUSIVE instead of FAILED — 2/3 reviewers

verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 (status map L1238, exit routing L1850 / L1893).

$gateInfraError (L1850) lumps both the without-fix (baseline) build error and the with-fix build error into one non-blocking bucket → exit 3 (INCONCLUSIVE). But the script already distinguishes them in prose just above: L1278 $wBuildError renders "🩺 Fix does not compile" vs L1282 $woBuildError renders "Base branch does not compile".

A baseline build failure is legitimately inconclusive (you can't establish the bug exists). But a with-fix-only build failure means the PR's own fix is broken — that's a definitive failure, not infra noise, yet it currently exits 3 (non-blocking) on every branch.

Scenario: PR adds a fix that doesn't compile; baseline builds fine. The verify gate exits 3 (INCONCLUSIVE) rather than 1 (FAILED), so the advisory signal under-reports a genuinely broken fix.

Suggested fix: at L1850, only treat baseline build errors as infra-INCONCLUSIVE. If $wBuildError.Count -gt 0 -and $woBuildError.Count -eq 0 (only the fix fails to build), keep FAILED/exit 1. The two sets are already computed at L1257-1258.

Mitigation: the maui-pr build blocks a non-compiling fix regardless, so this only weakens the advisory verify signal — it doesn't let broken code merge.

---

⚠️ An all-crash deep-UI run leaves APPROVE intact — 2/3 reviewers (design question)

post-ai-summary-comment.ps1 Test-RunValidationFailed (veto regex L477) vs the deep-UI render in ci-copilot.yml (L1887-1906).

When every deep-UI category crashes the HostApp (regularFailed==0, appCrashCategories>0), the render uses ⚠️ and "the HostApp crashed mid-run, so N tests could not complete" (L1890/L1906). The veto regex only matches ❌ **…tests** or a non-zero N failed count — so the all-crash header matches neither, the veto doesn't fire, and an APPROVE recommendation survives a run where zero deep-UI tests actually completed.

This is partly deliberate: an app crash is genuinely ambiguous (emulator/infra flake vs. PR regression), and you intentionally render it ⚠️ rather than ❌ so a good PR isn't shown as "all failed". The narrow question is whether a positive APPROVE is right when no deep-UI test produced a passing signal — arguably it should land on COMMENT (neutral) rather than APPROVE. Mixed runs (regularFailed>0) already render ❌ and do veto, and the body prominently warns the human either way.

Option: when appCrashCategories>0 -and regularFailed==0, downgrade APPROVE→COMMENT (not REQUEST_CHANGES — the crash may be a flake).

---

💡 A strictly-better alternative fix doesn't downgrade an explicit APPROVE — 2/3 flagged, but tested as intentional

post-ai-summary-comment.ps1 Get-AIReviewEventForRun L510: the non-PR-winner check (Test-HasNonPRWinner) only upgrades COMMENT→REQUEST_CHANGES, leaving an explicit APPROVE intact.

Two reviewers flagged this as a veto gap. On adjudication it turns out to be deliberate and explicitly unit-tested — Post-AISummaryComment.Tests.ps1 has a test named "does not override an exact approve recommendation" (isPRFix=$false + Final Recommendation: APPROVE → APPROVE). The design treats a non-PR winner as an advisory "we found a stronger alternative" preference, not a validation failure. Flagging only to confirm the intent is what you want; no change needed unless you'd prefer a strictly-better alternative to soften APPROVE→COMMENT.

---

Investigated and discarded (single-reviewer, refuted on follow-up)

• Labels from agent-writable artifacts (raised as Security): discarded. The s/agent-* labels are triage/metrics signal only — the real merge-readiness check uses GitHub-native reviewDecision, which derives from the gate-protected posted review event. Not a trust boundary; at most a cosmetic dashboard inconsistency.
• NaN/Infinity in token aggregation: discarded as unreachable. [double]::TryParse("NaN"/"Infinity", Float) does succeed and would throw on the later [long] round — but the aggregator only reads the pipeline-written raw/ token logs (typed integer counts), which are not PR-influenceable and never emit non-finite values. Purely defensive; left as a 💡 for optional hardening (reject non-finite in Get-NumericOrNull).

---

Non-blocking observation

This branch is currently CONFLICTING / DIRTY against main — there's a real merge conflict to resolve before it can merge (a fresh conflict from the new commits, not a code-quality issue).

---

Test coverage: strong. The veto/gate logic is well covered by Post-AISummaryComment.Tests.ps1 (forged-gate, INCONCLUSIVE-doesn't-block, deep-UI real-render veto, TRX-wording false-trigger). Gaps map exactly to the findings above: no test for a with-fix-only build error (Finding A) or an all-crash deep-UI run (Finding B).

Prior reviews: round-11 verdict was clean; the round-11 suggestion was addressed this round. No unresolved blocking threads from prior rounds.

3 independent reviewers · adversarial consensus · code-only review (CI status intentionally out of scope).

[kubaflo]

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

@PureWeen — addressed the round-12 findings in 2317c9555e (forward commit on the current head 8eca146f):

Finding	Status
⚠️ A — with-fix-only build error reported INCONCLUSIVE	Fixed — only a baseline build error or an env error stays infra-INCONCLUSIVE; a with-fix-only build error (baseline compiles, the fix doesn't) is now FAILED/exit 1.
⚠️ B — all-crash deep-UI leaves APPROVE intact	Fixed — new Test-DeepUITestsHadNoSignal softens APPROVE→COMMENT (neutral, not REQUEST_CHANGES) when every category setup-failed and nothing passed.
💡 C — non-PR-winner doesn't downgrade explicit APPROVE	Confirmed intentional — kept as-is (the "does not override an exact approve" test encodes the design: a non-PR winner is an advisory preference, not a validation failure). Thanks for verifying.
Copilot: RunAdb pipe-drain deadlock	Fixed — async drain before WaitForExit.
Copilot: non-portable timeout adb wait-for-device	Fixed — .NET process timeout.
Copilot: deep-UI scripts run from worktree (×2, security)	Left open — valid refinements; the clean fix rewires every deep-UI .github/scripts/… call to $TRUSTED. Replied inline; landing it as a dedicated, pipeline-validated change rather than a partial edit.

Tests: +6 Post-AISummaryComment cases (A/B), 36/36 + verify-tests-fail 5/5; UITest.Appium builds.

On the merge conflict: still outstanding — I deliberately did not fold a main merge into this push because the branch has concurrent commits landing (the round-12 RunAdb/deep-UI/token-usage work arrived while I was preparing this), and a divergent merge could collide. The conflict (a Review-Tests.ps1 badge block + the gh-aw lock) is small and should be resolved in a focused merge once the in-flight commits settle.

[PureWeen]

Adversarial re-review — round 13 (commit 2317c95)

Thanks for turning the round-12 notes around so quickly, @kubaflo. This commit ("Address round-12 review: build-error classification, all-crash veto, adb robustness") is a direct response to the previous review, so I focused on verifying each of the three fixes is correct and complete — i.e. that it closes the exact scenario originally flagged, not just an adjacent one.

I read the full source (not just the diff), traced the trusted-gate trust chain end-to-end, and empirically tested both regex/branch changes against the real pipeline render strings. Three reviewers reached the same conclusion independently: the adb fix is fully correct; the other two fixes handle a sibling case but each misses the originally-flagged one.

---

✅ adb robustness — correct and complete (verified)

This one is done right:

• RunAdb (AppiumLifecycleActions.cs:374-408) — both RedirectStandardOutput and RedirectStandardError are set (L381-382) with UseShellExecute=false, so BeginOutputReadLine()/BeginErrorReadLine() (L395-396) won't throw, and the no-op handlers drain both pipes before WaitForExit(15000) — the deadlock is eliminated. Kill() is try/catch-guarded. ✅
• BuildAndRunHostApp.ps1 wait-for-device (~L283-287) — portable Start-Process -PassThru -NoNewWindow + WaitForExit(90000) + guarded Kill() correctly replaces the non-portable timeout 90 adb. ✅

No regression here.

---

⚠️ Finding B (all-crash deep-UI APPROVE veto) — still incomplete: it catches the setup-failure sibling but misses the originally-flagged app-crash render

(2 of 3 reviewers rated this must-fix; all 3 agree the originally-flagged escape is still open.)

The new Test-DeepUITestsHadNoSignal (post-ai-summary-comment.ps1:485-503) keys its positive match on (L501):

'(?im)could not run:\s*OneTimeSetUp/fixture setup failure'

But ci-copilot.yml emits two mutually-exclusive regularFailed==0 "no-signal" headers, and appCrashCategories>0 takes priority (L1900):

• App-crash render — ci-copilot.yml:1906 (the originally-flagged case): … the HostApp crashed mid-run, so N tests could not complete …
• Pure setup-failure render — ci-copilot.yml:1911 (the sibling): … could not run: OneTimeSetUp/fixture setup failure …

The regex wording appears only in the L1911 sibling. The L1906 app-crash header (and the per-category app-crash body at L1800, app crashed mid-run; … could not complete) never contain could not run: OneTimeSetUp/fixture setup failure.

Concrete trigger: every deep-UI category crashes the HostApp (appCrashCategories>0, regularFailed==0, totalPassed==0) and the run recommends APPROVE → Test-DeepUITestsHadNoSignal returns $false → the APPROVE→COMMENT soften at L534 does not fire → a positive APPROVE survives a run where zero deep-UI tests completed — the exact escape Finding B set out to close. The doc-comment (L487) claims HostApp-crash coverage, and the new tests in Post-AISummaryComment.Tests.ps1 only exercise the could not run: OneTimeSetUp/fixture setup failure wording, so the gap isn't caught.

Suggested fix — broaden the positive match to the app-crash render, keeping the completed-test guards:

$noSignalHeader = ($uiContent -match '(?im)could not run:\s*OneTimeSetUp/fixture setup failure') -or
                  ($uiContent -match '(?im)the HostApp crashed mid-run, so .*could not complete')
return ($noSignalHeader -and
        $uiContent -notmatch '(?im)\b\d+\s+passed\b' -and
        $uiContent -notmatch '(?im)\b[1-9]\d*\s+failed\b')

The \b\d+\s+passed\b guard still correctly excludes the app-crash-with-passes header (L1908, which emits passNote). Please add a test using the literal L1906 wording so the app-crash path is covered.

(Scope note: this is bounded by APPROVE being advisory — a human still merges — so it's not a hard-gate bypass. But it leaves the originally-identified misleading-APPROVE escape open, so it's worth closing in this same PR.)

---

⚠️ Finding A (build-error classification) — verdict/veto fix is correct ✅, but the report headline + Gate chip still render INCONCLUSIVE for a non-compiling fix

The important half is done right and I verified it: the exit-code split (verify-tests-fail.ps1:1851-1854) means a with-fix-only build error (baseline compiles) now hits the else branch → exit 1 → Review-PR.ps1 maps to FAILED → ci-copilot.yml:827 makes the process exit code authoritative → GATE_VERDICT=FAILED → the APPROVE veto fires → REQUEST_CHANGES. Definitive build failures now block and veto APPROVE correctly. ✅

What's still inconsistent is the display — and it's the exact contradiction round-12 pointed at. Write-MarkdownReport (L1236-1238) still computes $hasBuildError from both without-fix and with-fix results:

$hasBuildError = ($WithoutFixResultsList | ? { $_.BuildError }) -or ($WithFixResultsList | ? { $_.BuildError })
$status = if ($VerificationPassed) { "PASSED" } elseif ($hasEnvError -or $hasBuildError) { "INCONCLUSIVE" } else { "FAILED" }

So a non-compiling fix renders ⚠️ INCONCLUSIVE in the report headline while the gate exits FAILED and the body says "Fix does NOT compile … definitive failure". That report is copied verbatim into gate/content.md, and Get-GateStatus (post-ai-summary-comment.ps1:235) parses the INCONCLUSIVE header → the human-facing Gate status chip (L744) shows Inconclusive. Net: the posted comment frames a definitive, blocking build failure as a non-blocking infra flake — the inconsistency round-12 asked to remove.

Suggested fix — mirror the L1851-1854 split inside Write-MarkdownReport so only a baseline build/env error maps to INCONCLUSIVE:

$baselineBuildError = ($WithoutFixResultsList | Where-Object { $_.BuildError }).Count -gt 0
$status = if ($VerificationPassed) { "PASSED" }
          elseif ($hasEnvError -or $baselineBuildError) { "INCONCLUSIVE" }
          else { "FAILED" }

---

Summary

Round-12 fix	Correct?	Complete?
adb robustness	✅	✅ Complete
A — build-error classification	✅ verdict/veto/blocking now correct (exit-code authoritative)	⚠️ headline (verify-tests-fail.ps1:1238) + Gate chip still render INCONCLUSIVE for a with-fix-only build error
B — all-crash deep-UI veto	⚠️ works for the setup-failure sibling	❌ regex misses the app-crash render (ci-copilot.yml:1906) — the originally-flagged all-crash APPROVE escape is still open

New regressions introduced by this commit: none — the trusted-gate trust chain (FAILED veto + exit-code authority) is intact. Both open items are incomplete fixes of the originally-flagged issues, not new breakage.

One non-blocking housekeeping note: the PR is currently CONFLICTING / DIRTY against main — there's a merge conflict to resolve before this can merge.

---

_{🔍 Methodology: three independent reviewers cross-checked each round-12 fix against full source + the real pipeline render strings; empirical regex/branch testing confirmed the app-crash gap. Posted as a non-blocking comment (not an approval/changes-requested) — sign-off is a human call.}

[kubaflo]

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

@PureWeen — thanks for the precise round-13 verification; you were right that A and B each closed a sibling case but not the originally-flagged one. Both completed in 5369e1dd49:

• Finding A (display): mirrored the exit-code split inside Write-MarkdownReport — only a baseline build/env error now maps to INCONCLUSIVE; a with-fix-only build error renders FAILED in the headline + Gate chip, consistent with the gate exit and the "Fix does NOT compile" body. No more "definitive failure framed as infra flake".
• Finding B (app-crash): broadened Test-DeepUITestsHadNoSignal to match both no-signal headers — the OneTimeSetUp/fixture setup failure render and the app-crash render (the HostApp crashed mid-run, so N … could not complete, ci-copilot.yml ~1906) which takes priority when appCrashCategories>0. The N passed/N failed guards still exclude the with-passes crash header. Added a test using the literal app-crash wording.

Tests: 43/43 (Post-AISummaryComment + Verify-TestsFail), including the new app-crash case. adb robustness already confirmed ✅.

Merge conflict still pending — noted; will land as a focused merge once the in-flight branch commits settle.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.

[PureWeen]

Adversarial re-review — round 14 (commit 5369e1d)

Both round-13 items are now fully closed @kubaflo — thanks for the quick turnaround. This commit ("round-13: complete Findings A & B (display headline + app-crash veto)") directly completes the two incomplete-fix findings from the last review, so I focused on verifying each now closes the exact originally-flagged scenario (not just an adjacent one) and introduces no new regression. Three reviewers plus an independent empirical pass all reached the same conclusion: complete, correct, no regressions.

---

✅ Finding A (build-error headline / Gate chip) — RESOLVED

The headline now keys off a baseline-only build error, mirroring the exit-code split exactly:

$baselineBuildError = @($WithoutFixResultsList | Where-Object { $_.BuildError }).Count -gt 0
$status = if ($VerificationPassed) { "PASSED" } elseif ($hasEnvError -or $baselineBuildError) { "INCONCLUSIVE" } else { "FAILED" }

Verified consistent across every layer for a with-fix-only build error (baseline compiles, the PR's fix doesn't):

• headline $status → ❌ FAILED (was ⚠️ INCONCLUSIVE) — verify-tests-fail.ps1:1241-1243
• matches the exit-code predicate $gateInfraError = $anyEnvError -or $baselineBuildError → exit 1 (same .Count -gt 0 shape, L1856/L1859)
• matches the body classification 🩺 Fix does not compile (L1283) and the console box (L1916)
• the human-facing Gate chip now renders Failed: Get-GateStatus's structured marker Gate Result:\s*(?:\S+\s*)?(FAILED|…) (L235) consumes the emoji and returns Failed before the \binconclusive\b word-fallback (L244) is ever reached.

The old $hasBuildError variable is fully removed (zero remaining references). The contradiction round-13 flagged is gone.

---

✅ Finding B (all-crash deep-UI APPROVE veto) — RESOLVED

Test-DeepUITestsHadNoSignal now matches both no-signal renders:

$noSignalHeader = ($uiContent -match '(?im)could not run:\s*OneTimeSetUp/fixture setup failure') -or
                  ($uiContent -match '(?im)the HostApp crashed mid-run, so .*could not complete')

I empirically tested the function against the exact ci-copilot.yml render strings for every variant:

Deep-UI render	Softened?	Correct?
L1906 app-crash, no passes (the originally-flagged escape)	yes → COMMENT	✅
L1906 app-crash with passes	no (kept APPROVE)	✅
L1908 app-crash + regular failures	no	✅
L1911 setup-failure, no passes (sibling, already handled)	yes	✅
L1913 setup-failure with passes/fails	no	✅
normal pass/fail run	no	✅
L1800/L1803 per-category <details> body	no false-positive	✅

The originally-flagged all-crash escape is closed: because appCrashCategories>0 forces the crash header (L1900→L1906), matching that header explicitly is exactly right. No false-positive on the <details> body — it says "app crashed mid-run;" / "crashed during this category", neither of which contains the required literal "the HostApp crashed mid-run, so". The (?im) (no s flag) is intentionally safe — . can't cross newlines, so .* stays on the single header line. And the two new tests (Post-AISummaryComment.Tests.ps1:326-336) exercise the app-crash path directly — both the no-passes (TRUE) and with-passes (FALSE) cases.

---

✅ No new regressions

• @(...).Count -gt 0 is a strict hardening over the old truthy-object -or (null-safe; now matches the exit-code block's idiom). Logically equivalent for the true/false cases.
• Broadening $noSignalHeader does not risk over-softening — it's only one of three conjuncts; any N passed or non-zero N failed anywhere in the report still forces $false and preserves APPROVE.
• No third no-signal render path remains; the all-empty "0 passed, 0 failed" no-tests case is a distinct scenario, correctly out of scope.

---

Summary

Round-13 finding	Status
A — headline/Gate chip for a non-compiling fix	✅ Complete & correct
B — app-crash APPROVE veto	✅ Complete & correct
New regressions	None

From a code standpoint this is clean — I have no remaining findings on the review-infrastructure changes. The one non-code item left before merge: the PR is still CONFLICTING / DIRTY against main, so there's a merge conflict to resolve.

---

_{🔍 Methodology: three independent reviewers cross-checked each round-13 fix against full source + the real pipeline render strings, with empirical regex/branch testing of all deep-UI render variants. Posted as a non-blocking comment (not an approval/changes-requested) — sign-off is a human call.}