Converge /review tests with azdo-build-investigator via shared CI facts doc

[PureWeen]

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

This is a tooling/agent-infrastructure change only (everything is under .github/ plus the local /review tests runner) — no product source or public API changes.

It converges the automated /review tests workflow (the review-test-failures skill) with the interactive azdo-build-investigator skill so that both deliver the same end result — a CI merge-readiness verdict plus a base-branch baseline failure review — while removing the duplicated MAUI CI knowledge that was copy-pasted across several files and at real risk of drifting.

The two skills can't share tooling (the ci-analysis plugin that powers azdo-build-investigator is a stdio MCP server, which the gh-aw runtime can't run), so "minimal duplication" is achieved by sharing docs, not tools.

Architecture: "shared knowledge, two roles"

One canonical facts doc; every consumer references it instead of re-stating the facts.

• New .github/docs/maui-ci-facts.md — single source of truth for pipeline names/IDs (maui-pr 302, maui-pr-devicetests 314, maui-pr-uitests 313), AzDO data sources, the XHarness exit-0 blind spot, test-count deduplication, the baseline-comparison rule, visual-baseline/platform-mismatch guidance, Gradle/CFSClean signatures, the common-failure-pattern table, and the merge-readiness criteria. Its header lists every consumer, kept bidirectionally accurate.

What changed

Area	Change
review-test-failures/SKILL.md	References the facts doc; adds an overall merge-readiness verdict and baseline reasoning + an On base? column.
azdo-build-investigator/SKILL.md	Slimmed (~106→68 lines) to reference the facts doc; keeps its unique value (ci-analysis-first, the outdated-maui-public correction, escalation to helix-investigation).
Gather-TestFailureContext.ps1	Extends the deterministic gatherer with per-test base-branch baseline extraction + comparison (failures.baseline, baselineMatchCount, baselineSummary, alsoFailsOnBaseline).
copilot-review-tests.md (+ lock)	Output template gains a Baseline badge, an On base? column, and merge-readiness verdict/colors.
Review-Tests.ps1	Local runner: merge-readiness verdict→color map + a Baseline badge from failures.baselineMatchCount.
ci-status-main.md / ci-status-net11.md (+ locks)	Removed duplicated pipeline-ID table, "key points" bullets, failure-pattern table, and dedup prose — now reference the facts doc; added a facts-doc existence check to the connectivity probe.

Validation

• Both PowerShell scripts AST-parse cleanly.
• gh aw compile of all three workflows: 0 errors, and a second recompile is idempotent (no further lock diff) — the compiled .lock.yml files are in sync with their .md bodies. (gh-aw {{#runtime-import}}s the body and pins a body_hash, so body edits surface as a hash-only lock diff.)
• Grep sweep confirms no inline copies of the canonical pipeline/failure-pattern tables remain outside the facts doc, and the facts-doc consumer list exactly matches the set of files that reference it (bidirectional).

Intentionally out of scope (follow-up)

A parallel "repo-health" analysis flagged a couple of broader items left untouched here: the pipeline name→ID table is still duplicated in copilot-instructions.md and trigger-azdo-pipeline-setup, and there are some dead pr agent references in copilot-instructions.md. Those are the next concentric ring of cleanup and aren't required for this convergence.

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

Or

• Run remotely in PowerShell:

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

[github-actions]

Skill Validation Results

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

✅ Skill Validation Results — 7f2b109 · Converge /review tests with azdo-build-investigator via shared CI facts doc · 2026-06-25T15:31:48Z

✅ 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: Skipped

No changed skills with eval specs found.

🔍 Full results and investigation steps

[kubaflo]

---

🤖 AI-GENERATED MULTI-MODEL CODE REVIEW 🤖

This review was independently performed by 3 AI models (Claude Opus 4.8, GPT-5.5, Gemini 3.1 Pro), then cross-pollinated to reconcile differences.

Round: 2
HEAD: 8015c6cb
Timestamp: 2026-06-23T13:43 UTC+02

---

🎯 Verdict: NEEDS CHANGES ❌

Confidence: High (unanimous after cross-pollination)

Independent verdicts:

• Claude Opus 4.8: LGTM/High → NEEDS_CHANGES/Medium (flipped after investigating GPT's finding)
• GPT-5.5: NEEDS_CHANGES/Low → NEEDS_CHANGES/High (maintained, confidence raised)
• Gemini 3.1 Pro: LGTM/High → NEEDS_CHANGES/High (flipped after investigating GPT's finding)

📊 Summary

This PR converges the /review tests workflow with azdo-build-investigator skill by creating a canonical maui-ci-facts.md and adding base-branch baseline comparison logic to Gather-TestFailureContext.ps1.

What's good:
✅ Eliminates duplicated CI knowledge across multiple files
✅ Creates single source of truth for pipeline IDs, failure patterns, merge-readiness criteria
✅ PowerShell scripts AST-parse cleanly
✅ Lock files properly regenerated
✅ Round 2 commits address device-test XHarness over-confidence from adversarial review

What needs fixing:
❌ Baseline log read failures are silently reported as zero baseline failures — see critical finding below

🚨 Critical Finding — UNANIMOUS

All 3 models independently investigated and confirmed this issue after cross-pollination:

❌ Baseline log read failures are silently reported as zero baseline failures

Location: .github/skills/review-test-failures/scripts/Gather-TestFailureContext.ps1, line 663

The problem:
Get-BuildLogTestFailures catches exceptions while reading baseline build logs but does not record them:

1. Timeline fetch failures (lines 643-645): Return without setting $result.error
2. Per-log read exceptions (catch block at line 663-665): Silently swallowed

Consequence:
The caller (lines 1055-1061) reports baselineFailureCount = 0 with note = $null for a failed base build where logs are expired/inaccessible. This means:

• ❌ Missing/expired baseline data looks like a cleanly-inspected zero-failure baseline
• ❌ Pre-existing failures can be misclassified as PR-caused (no alsoFailsOnBaseline stamp)
• ❌ Contradicts the PR's own facts doc (maui-ci-facts.md:119: "if baseline data is missing or the base build is inaccessible, say so")

Why this matters:
AzDO log retention/expiry makes the "build accessible, logs expired" case realistic in production.

The fix:
Bubble timeline errors into $result.error and accumulate per-log read failures into a non-null baselineSummary.note. Treat such rows as "Insufficient data" downstream rather than a clean zero-failure baseline.

Cross-pollination notes:

• Opus's independent review flagged general under-match as a suggestion but missed the indistinguishability from clean baseline. After investigating GPT's finding directly against HEAD 8015c6cb, Opus confirmed it contradicts the PR's own shipped facts doc and flipped to NEEDS_CHANGES.
• Gemini's independent review gave LGTM/High with no findings. After investigating GPT's claim, Gemini confirmed the finding is correct and flipped to NEEDS_CHANGES/High.
• GPT's independent review caught this issue initially. After cross-pollination, GPT maintained NEEDS_CHANGES and raised confidence from Low to High.

💡 Additional Suggestions (Non-blocking)

3 minor suggestions for future improvements (see inline comments):

1. Canceled builds: Treat 'canceled' as weak/inconclusive baseline evidence (compounds the silent-zero gap)
2. Log-only extraction: Surface the MaxLogs=8 cap in summary (under-match shouldn't look like confident clean baseline)
3. Most-recent-only: Single most-recent build misses intermittent base flakes (intentional, noted for awareness)

These do not affect merge readiness beyond the blocking error above.

🔍 Key Review Points

CI Status — Resolved

Initial disagreement: GPT flagged maui-pr: skipping as undetermined CI coverage

Resolution: All 3 models agree after investigation:

• maui-pr skip is by-design for .github/** changes (path exclusion in eng/pipelines/ci.yml)
• Actual validation: Skill Validation, Static validation, and skill validation all pass
• CI coverage is appropriate for .github/skills/** tooling changes

Round 2 Commits

The 2 new commits in Round 2 correctly address adversarial review findings:

• ✅ "Don't overstate device-test baseline confidence on XHarness exit-0"
• ✅ "Align device-test baseline guidance and de-dup succeeded baseline rows"

These fix the XHarness exit-0 blind spot where succeeded baselines were reported with overconfidence.

Code Quality

✅ PowerShell scripts AST-parse cleanly
✅ Lock files properly regenerated (gh aw compile idempotent)
✅ Coverage verified: duplicated tables removed from all consumers
✅ Facts doc correctly captures pipeline IDs and failure patterns

🏁 Recommendation

REQUEST CHANGES — The baseline log read failure issue must be fixed before merge:

1. It contradicts the PR's own documentation (maui-ci-facts.md)
2. It can misclassify pre-existing failures as PR-caused
3. The fix is straightforward and in-scope for this PR

After the fix:
This is excellent infrastructure work that eliminates dangerous duplication and adds valuable baseline comparison. The fix for the error finding is trivial — bubble up the log read failures so they're visible in the baseline summary.

---

This review used the code-review skill with independence-first assessment, full-context analysis, and empirical verification by all 3 models.

[kubaflo]

Note

🤖 This review was generated by AI — an automated multi-model code review (Claude Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro), independent reviews reconciled via cross-pollination.

---

🔍 AI Multi-Model Code Review — Round 3

HEAD: c8fb2880 · Re-review after Round 2 REQUEST_CHANGES (8015c6cb)

---

✅ Verdict: LGTM — Round 2 blocker resolved

Confidence: High — unanimous (all 3 models, independently and after cross-pollination)

Model	Verdict	R2 error resolved?
Claude Opus 4.8	LGTM / High	✅ yes
GPT-5.5	LGTM / High	✅ yes
Gemini 3.1 Pro	LGTM / High	✅ yes

Notably, GPT-5.5 — the model that originally caught the Round 2 error — independently confirmed its own finding is genuinely fixed, not superficially patched.

---

🎯 Round 2 Blocker — Fixed ✓

Round 2 finding (unanimous ❌ Error): Baseline log read failures were silently reported as zero baseline failures. Timeline-fetch failures and per-log read exceptions in Get-BuildLogTestFailures were swallowed; the caller emitted baselineFailureCount=0 with note=$null for a failed/expired base build — so an inaccessible baseline looked clean, and pre-existing failures could be misattributed to the PR. This contradicted the PR's own maui-ci-facts.md guidance.

The Round 3 fix (Gather-TestFailureContext.ps1) — verified by all 3 models against the worktree:

1. ✅ Build-metadata inaccessible → now sets result.error = "Build <id> metadata was not accessible." (was a possibly-null passthrough)
2. ✅ Timeline fetch failure → now sets result.error = "Timeline for build <id> was not accessible (logs may be expired)." (previously returned with error unset)
3. ✅ Per-log read exception → the silent # Ignore is replaced with a $logReadFailures counter; after the loop, result.error reports "<n> of <N> baseline build log(s) could not be read … baseline failure list is incomplete."
4. ✅ Caller builds an honest note ($noteParts): surfaces error, flags a not-succeeded base build that yielded 0 extractable failures as "inconclusive, not clean", and reports truncation when totalFailedRecords > inspectedLogCount. note stays $null only for a genuinely clean baseline.
5. ✅ SKILL.md now instructs the agent that a row whose note flags inconclusive/incomplete is not a clean zero-failure baseline even when baselineFailureCount=0 — don't conclude PR-caused; fold into Insufficient data.

Correctness probes (all passed)

• No green-baseline false positive: the elseif ($baseFailureCount -eq 0) inconclusive note lives strictly inside the foreach ($base in $notSucceeded) loop (failed/partiallySucceeded/canceled). A succeeded most-recent base build takes the earlier continue and legitimately reports baselineFailureCount=0 / note=$null. ✔
• No schema regression: totalFailedRecords / inspectedLogCount are additive, consumed only by the new note logic. ✔
• Prior non-blocking suggestions also addressed: the MaxLogs=8 under-match is now surfaced via the truncation note; canceled/partial base builds are covered by the zero-extract inconclusive note.

---

🟢 CI Status

• Static validation ✅ pass · Skill Validation ✅ pass · skill-validation ✅ pass · license/cla ✅ pass
• maui-pr ⏭️ skipping — by-design (.github/** path exclusion in eng/pipelines/ci.yml); Build Analysis pending (informational)
• Required checks are not red.

---

💡 One Non-Blocking Suggestion (see inline)

The new error-surfacing + $noteParts logic is now load-bearing correctness code but ships without a unit-test harness (review-test-failures has no tests/ dir — pre-existing for this script). Non-blocking, and mitigated because every new branch errs toward inconclusive/incomplete (the safe direction). A small Pester harness locking the five semantics (listed inline) would protect against future drift. All three models flagged this as the sole follow-up.

---

🏁 Recommendation

LGTM — the Round 2 blocker is fully and correctly resolved, the fix introduces no regression and no green-baseline false positive, and the prior non-blocking suggestions are addressed too. Ready for human approval.

---

Reviewed with the code-review skill: independence-first (code before narrative), full-file reading in a worktree at PR HEAD, empirical control-flow verification, and CI cross-checking by all 3 models.

[kubaflo]

💡 💡 Suggestion (non-blocking): Consider a small Pester harness for the new baseline-note branching

The metadata / timeline / per-log error surfacing in Get-BuildLogTestFailures and the three-way $noteParts logic here are now load-bearing correctness code — they're what stops an expired/inaccessible baseline from being read as a clean zero-failure baseline (the Round 2 finding). They currently ship without unit tests (review-test-failures has no tests/ dir).

This is non-blocking and consistent with how skill scripts are treated in this repo, and it's mitigated by the fact that every new branch errs toward inconclusive/incomplete (the safe direction). But a tiny harness asserting:

• timeline-fetch failure → result.error set,
• per-log read exception → result.error mentions the unreadable count,
• notSucceeded base + 0 extractable failures → note flags "inconclusive, not clean",
• totalFailedRecords > inspectedLogCount → truncation note,
• genuinely-clean baseline → note = $null,

would lock in these now-critical semantics against future drift. All three reviewers independently flagged this as the only (non-blocking) follow-up.

— 3-model consensus (Opus 4.8 / GPT-5.5 / Gemini 3.1 Pro), non-blocking.

[kubaflo]

Note

🤖 This review was generated by AI — an automated multi-model code review (Claude Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro), independent reviews reconciled via cross-pollination.

---

🔍 AI Multi-Model Code Review — Round 4

HEAD: 69825c17 · New commit since R3 LGTM: "Add deterministic merge-readiness gate to /review tests" (+315/−13)

---

🔴 Verdict: NEEDS CHANGES

Confidence: High — unanimous (all 3 models, independently and after cross-pollination)

Model	Independent	After cross-pollination
Claude Opus 4.8	NEEDS_CHANGES / High	NEEDS_CHANGES / High
GPT-5.5	NEEDS_CHANGES / High	NEEDS_CHANGES / High
Gemini 3.1 Pro	NEEDS_CHANGES / High	NEEDS_CHANGES / High

The new deterministic merge-readiness gate is a genuinely good idea and mostly well-built — but cross-pollination surfaced a reachable false-green hole in its core logic, and the PR currently can't merge. Two ❌ blockers below.

---

❌ Blocker 1 — Gate emits Ready to merge for an accessible failing check with zero extracted failures (false green)

The whole point of this gate is to make a green verdict trustworthy. It currently isn't, for the most common real case: a compile/build break.

Reproduction (Gather-TestFailureContext.ps1, verified by all 3 models + manual trace):

• A required check X is failed and its AzDO build is accessible → bucketing loop (L1255–1266) hits continue (L1258); X is added to neither inaccessibleFailingChecks nor unmappedFailingChecks.
• X yields zero extracted test failures (a compile/build/infra failure has no parseable test rows), so dedupedFailures is empty.
• Ceiling chain: L1276 false → L1280 false → L1289 false (failingChecks.Count is not 0) → L1292 else → Ready to merge.

The L1289 guard only covers failingChecks.Count -eq 0; there's no branch for "failing checks exist but contributed no extracted/explained failure."

Suggested fix: track accessible failing checks that contributed ≥1 extracted (or otherwise explained — baseline/known-issue) failure; if an accessible failing check contributed none, cap at Needs human investigation (e.g. a failingChecksWithoutExtractedFailures bucket folded into the L1280 branch), or require in the else that every failing check is accounted for before allowing Ready to merge. Details inline at L1289.

(Severity reconciliation: Opus suggestion→error, GPT warning→error, Gemini held error — converged ❌ after each model re-traced the control flow.)

---

❌ Blocker 2 — PR has merge conflicts with main (required checks blocked)

• gh pr view → mergeable: CONFLICTING, mergeStateStatus: DIRTY.
• The 4 required checks (maui-pr, maui-pr-devicetests, maui-pr-reviewer, maui-pr-uitests) are action_required → "Skipped due to merge conflicts" — blocked by the conflict, not failing on code.
• Conflicting files: .github/workflows/ci-status-main.lock.yml, ci-status-main.md, ci-status-net11.lock.yml (gh-aw frontmatter_hash/body_hash collisions — both this PR and main recompiled them).
• Action: rebase/merge main, then re-run gh aw compile and commit the regenerated .lock.yml files so the hashes match. Required checks can't go green until this is resolved (code-review Rule #6 → cannot LGTM regardless of code quality).

---

🟢 What's good

• The gate concept (a deterministic verdictCeiling the LLM can't override upward) is the right design; the Insufficient data / Needs human investigation precedence for inaccessible/pending/unmapped checks is sound.
• matchesKnownIssue and retriedStillFailing are well-reasoned evidence signals; retriedStillFailing correctly resists "looks flaky" hand-waving.
• Get-KnownBuildIssues fails open (empty patterns + knownIssues.queried=false) when gh is unavailable — good, and the docs tell the model not to treat a no-match as proof.
• ErrorMessage matching has no escaping bug (literal IndexOf); ErrorPattern regex is validated at load with a substring fallback.

💡 Non-blocking

• Known-issue generic over-match (inline @ L690): a short/generic ErrorMessage registry entry can semantically over-match unrelated failures and mask a real regression as "unrelated." Consider a min-length / word-boundary or check-area guard.
• Still no Pester harness for this now load-bearing logic — this re-raises the open R3 suggestion (thread on Gather-TestFailureContext.ps1). Blocker 1 is Exhibit A: a single ceiling truth-table test (failingChecks ≥ 1 with dedupedFailures == 0 ⇒ expect non-green) would have caught it pre-merge. (Not reposted inline — the prior thread is still open.)

---

🏁 Recommendation

REQUEST_CHANGES. Fix the false-green ceiling hole (Blocker 1) and resolve the merge conflict + recompile the gh-aw locks (Blocker 2). The gate is close — it mainly needs to refuse a green verdict when an accessible failing check produced no explained failure, and a small unit test to lock that in.

---

Reviewed with the code-review skill: independence-first (code before narrative), full-file reading in a worktree at PR HEAD, manual control-flow verification of the disputed finding, and CI/mergeability cross-checking by all 3 models.

[kubaflo]

💡 💡 Suggestion (non-blocking) — Generic/short known-issue ErrorMessage can over-match unrelated failures

Good news first: this is not an escaping bug. ErrorMessage matches via an unanchored literal String.IndexOf(..., OrdinalIgnoreCase) (regex metacharacters are treated literally), and ErrorPattern is intentional regex validated at load (Regex.IsMatch('', pattern)) with a substring fallback on invalid syntax — both are sound.

The residual risk is semantic over-match: a short or generic Known Build Error ErrorMessage (e.g. "timeout", "connection reset", "##[error]") is a substring of many unrelated failures, so a real PR-caused failure could be silently attributed as Likely unrelated and cited by issue number. Consider a minimum-length / word-boundary guard, or only treating ErrorMessage as a hit when combined with the same check/area, so a generic registry entry can't mask a genuine regression.

— Opus 4.8 / GPT-5.5 (suggestion) · Gemini 3.1 Pro (warning); reconciled non-blocking.

[kubaflo]

Note

🤖 This review was generated by AI — an automated multi-model code review (Claude Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro), independent reviews reconciled via cross-pollination.

---

🔍 AI Multi-Model Code Review — Round 5

HEAD: 248f16e3 · New commits since R4: "Catch build-job breaks (crossgen/R2R, NativeAOT)" + "Make the job-level baseline diff deterministic" (+398/−25)

---

🔴 Verdict: NEEDS CHANGES

Confidence: High — unanimous (all 3 models, independently and after cross-pollination)

Model	Independent	After cross-pollination
Claude Opus 4.8	NEEDS_CHANGES (residual = warning)	NEEDS_CHANGES (conceded residual = error)
GPT-5.5	NEEDS_CHANGES (residual = error)	NEEDS_CHANGES (error)
Gemini 3.1 Pro	NEEDS_CHANGES (residual = error)	NEEDS_CHANGES (error)

Real progress this round — but the R4 false-green is narrowed, not closed, and the PR still can't merge. Two ❌ blockers remain.

---

✅ What R5 fixed (good progress)

• New build-break extractor catches [FAIL] lines and crossgen2 / ReadyToRun / NativeAOT-ILC / linker / MSBuild error shapes — the common compile-break case from R4 is now caught.
• New $unexplainedLegs backstop: a failed Task leg with no test-shaped failure caps the ceiling at Needs human investigation instead of falling through to green.
• New legsRegressedVsBase gate: a leg red-on-PR / green-on-base forces Not ready (forbids Ready to merge).

These are the right ideas and close most of the R4 hole.

---

❌ Blocker 1 (still open) — Residual false-green: an accessible red check with zero accounted-for failures still reaches the green else

The backstop enumerates break shapes instead of structurally guaranteeing "no green while an accessible failing check is unaccounted-for." $unexplainedLegs is gated type -eq 'Task' (L1149/L1154), its .Add sits inside the per-log read try after the fetch (L1155), and $logsToRead is capped at -First 12 (L1130). So three real shapes each yield dedupedFailures == 0 and unexplainedLegs == 0 on an accessible failing check (accessible=true L1070; the check continues at L1504-1505), falling through to else → "Ready to merge" (L1550):

1. Job-level infra break (job timeout / agent-provisioning loss) where no child Task is failed (the running task is marked canceled) → skipped by the type -eq 'Task' filter. (Routine on maui-pr's long device/UI legs.)
2. Log-read throw on a failed Task → the exception escapes before .Add to $unexplainedLegs (L1155).
3. Failed Task beyond the -First 12 cap → never inspected by the extractor or the backstop.

Because failingChecks.Count > 0, the L1546 guard (failingChecks.Count -eq 0 -and …) is false, so execution reaches the green else. A PR whose maui-pr legs died on a job timeout could still be reported Ready to merge. (Detail + the fix posted as a reply on the open R4 false-green thread.)

Robust fix (structural, not shape-enumeration): make the ceiling enforce that any accessible failing check with zero accounted-for failures caps at ≥ Needs human investigation — e.g. count accessibleFailingChecks that contributed 0 dedupedFailures/unexplainedLegs and fold that count into the L1539-style branch, rather than relying on the extractor to recognize every break shape. (Severity reconciled to ❌ error unanimously — Opus re-traced and conceded warning→error.)

---

❌ Blocker 2 (still open) — Merge conflict blocks required checks

• mergeable: CONFLICTING, mergeStateStatus: DIRTY (unchanged from R4).
• maui-pr, maui-pr-devicetests, maui-pr-reviewer, maui-pr-uitests → action_required "Skipped due to merge conflicts" — they did not run (gated on the unbuildable merge commit), not code failures.
• Action: merge/rebase main, then re-run gh aw compile and commit the regenerated ci-status-*.lock.yml so the hashes match and the maui-pr* checks execute. Per code-review Rule #6, required checks blocked by an unresolved conflict cap the verdict below LGTM regardless of code quality.

---

💡 Non-blocking

• legsRegressedVsBase retry-masking (inline @ L1447): a base leg that failed-then-passed-on-retry is read as failed-on-base, suppressing a real regression signal. Conservative (can't cause a false-green), but weakens the new gate. Consider treating base failed-then-recovered as green for the comparison.
• Still no Pester harness for this now +500-line load-bearing gate. Blocker 1 is the second missing-unit-test escape in two rounds; a tiny truth-table test (failing check + 0 accounted-for ⇒ expect non-green; job-level / thrown-log / >12-cap cases) would lock it down. (Reinforces the open R3 thread — not reposted inline.)

---

🏁 Recommendation

REQUEST_CHANGES. Close the residual false-green structurally (don't enumerate break shapes — guarantee no green while an accessible failing check is unaccounted-for), and resolve the merge conflict + recompile the gh-aw locks so the required checks actually run. The gate is genuinely close; it mainly needs the structural guarantee and a unit test to prove it.

---

Reviewed with the code-review skill: independence-first, full-file reading in a worktree at PR HEAD, manual control-flow re-tracing of the disputed finding (Opus conceded warning→error after re-trace), and CI/mergeability cross-checking by all 3 models.

[kubaflo]

💡 💡 Suggestion (non-blocking) — legsRegressedVsBase: a base leg that failed-then-passed-on-retry is read as failed-on-base, masking a real regression

Here hasFailed is checked before hasSucceeded (L1447 vs L1452), so a base-branch leg that failed on attempt 1 but passed on retry is stamped failed-on-base (pre-existing). That suppresses the regressed-vs-base signal for the same leg when it's red on the PR.

This is a conservative false-negative — it can only ever fail to lower the ceiling, never raise it, so it can't cause a false-green. But it weakens the new deterministic-regression gate exactly when it'd be most useful. Consider treating a base leg that failed-then-recovered as green for the regression comparison (or surface it as Needs human investigation rather than silently pre-existing).

— Claude Opus 4.8 (cross-pollination); GPT-5.5 / Gemini 3.1 Pro concur non-blocking.

[kubaflo]

Note

🤖 AI-generated follow-up (multi-model review automation) — timing correction.

Heads-up on a race: my Round 5 REQUEST_CHANGES above was conducted against 248f16e3; while it was posting, you pushed a1efd049 "Close false-green ceiling hole for unaccounted failing checks." So the review landed on a1efd049, but its Blocker 1 section describes the pre-a1efd049 state. Correcting the record:

✅ Blocker 1 (residual false-green) — RESOLVED by a1efd049

That commit implements exactly the structural guard the review recommended, at build granularity:

• new $unaccountedFailingChecks — an accessible build that backs a currently-failing check but contributed zero extracted failures and zero unexplained-leg records;
• folded into the Needs human investigation branch (-or $unaccountedFailingChecks.Count -gt 0).

I traced it against the three escape paths from the review and it closes all of them — Job-level infra break (timeout/agent loss, no failed child Task), log-read throw, and failed record past the per-build cap — because each leaves an accessible failing check whose build contributed nothing, which the new guard now catches regardless of break shape. This is the "don't enumerate break shapes; guarantee no-green-while-unaccounted-red" property. 👍 Resolving the false-green thread.

❌ Blocker 2 (merge conflict) — STILL the remaining gate

At a1efd049 the PR is still mergeable: CONFLICTING / mergeStateStatus: DIRTY, and maui-pr / maui-pr-devicetests / maui-pr-reviewer / maui-pr-uitests are still action_required "Skipped due to merge conflicts" — they haven't run. Rebase/merge main, then re-run gh aw compile and commit the regenerated ci-status-*.lock.yml so the required checks execute.

Net: the gate logic is now in good shape; the only thing keeping this at REQUEST_CHANGES is the unresolved merge conflict (required checks can't go green until it's fixed). The non-blocking items still stand: the legsRegressedVsBase retry-masking nit (inline) and the missing Pester harness (a truth-table test for the new $unaccountedFailingChecks guard would lock in exactly this fix).

Full multi-model re-verification of a1efd049 will run on the next change.

[PureWeen]

Adversarial review round — 5 false-green / precision holes closed (fd25781)

Ran a 3-model adversarial pass (three independent reviewers, adversarial consensus) over the gatherer with one focus: can a real PR-caused failure ever reach a green verdict? Five vectors were confirmed against the code and fixed. All land in Gather-TestFailureContext.ps1.

#	Hole	Consensus	Fix
A	A failed Task leg with no log id, past the 12-read cap, or whose log read threw produced no failure and no unexplained-leg record — yet the build stayed "accounted" via another explained leg → false green	3/3	Post-loop sweep records every uninspected failed Task leg as an unexplained leg, reusing the existing ceiling cap
B	A failure the deterministic prior could attribute neither way (indeterminate) fell through to the green else	2/3	New gate.unattributedFailures caps the ceiling at Needs human investigation
C	The gate read only status/conclusion, so a red classic StatusContext (state=FAILURE, status/conclusion null — e.g. an AzDO-posted commit status) was invisible	1/3, verified on this PR's own skill-validation status	Projection + interesting-check filter + pending/failing split now route by state too
D	A generic leg name (Build) that both failed and succeeded on base was force-marked pre-existing (masking a real regression)	1/3	Resolves to inconclusive-on-base → indeterminate (capped by B); prevents both a false pre-existing subtraction and a false regression
Ge1	A deterministic device-test BUILD break (source=azdo-build-error) was suppressed from the regression cap alongside the XHarness exit-0 test blind spot	1/3	Only device-test TEST results are suppressed now; build breaks regress

Already-addressed prior findings (referenced, not re-posted):

• The line-1654 ❌ "ceiling can emit Ready to merge for an accessible build that yielded nothing" is closed by the earned-green guard (gate.unaccountedFailingChecks) in a1efd04952.
• The line-1447 💡 "a base leg that failed-and-succeeded" suggestion is Fix D above — thanks, that was a real hole.

Intentionally declined (documented as conservative-by-intent):

• Making the regression cap depend on matchesKnownIssue (so a known-flaky regression could be downgraded) is not done. The north star is over-blocking beats false-green, and the still-open line-819 known-issue over-match (generic/short ErrorMessage via unanchored IndexOf) makes that downgrade risky. The cap stays conservative.

Validation: both PS1 AST-clean; ceiling + check-split + leg-diff truth tables green (15 + 10 assertions); lock recompiled (body_hash-only). No APPROVE/REQUEST_CHANGES — comment only.

[PureWeen]

Adversarial review round — 7 residual gate holes closed (92c1840)

Ran another 3-reviewer adversarial pass over the deterministic gate. Every finding was verified against the code before fixing (deterministic facts over votes). All seven land in Gather-TestFailureContext.ps1; four were genuine false-green vectors:

#	Hole	Class	Fix
1	Device build-break regression read source from the deduped representative (only has sources plural) → always empty → a device-pipeline crossgen/R2R break never regressed	false green (silent no-op)	read source from the raw occurrence
2	A regression signal conflicting with a base-failure signal, and a leg-only base match (no exact test+platform match), were dismissed as pre-existing-on-base	false green	only an exact test+platform base match dismisses; conflicts + leg-only → indeterminate (→ NHI)
3	A Known Build Error text match dismissed a red check to green with zero baseline corroboration — a broad matcher could shadow a real PR break	false green	known-issue is dismissable only when the leg was actually compared on base; else indeterminate
4	An unreadable timeline (independent of accessible, which is build-metadata only) masked by a sibling test-API "contribution" skipped the earned-green guard	false green	added a timelineReadable flag; the guard only counts a contributing build as accounted when its timeline was readable
5	A proven regression coexisting with NHI reasons reported as Needs human investigation instead of Not ready	softened signal	regression cap now promotes NHI → Not ready (still non-green)
6	A failing StatusContext carries context/targetUrl, not name/detailsUrl → landed in unmappedFailingChecks with a blank name	false red + unreadable	projection falls back to context/targetUrl so it is named and build-resolvable
7	A failed non-Task leaf leg (agent lost / timeout / cancel before any Task fails) was skipped by the Task-only sweep	narrow false green	also surfaced as an unexplained leg

Plus a hardening: the pending/failing split is now a strict catch-all — any interesting, non-pending check is treated as failing, so an unrecognized status/state shape can never escape both buckets.

Fix #3 also addresses the still-open ⚠️ on the known-issue matcher (line ~819): an over-broad matcher can no longer dismiss a red check on text alone — a base comparison is now required before a known-issue match counts as "pre-existing".

Validation: both PS1 files AST-clean; a 31-assertion truth-table harness covers the attribution precedence, the device build-break source, the pending/failing catch-all, and the ceiling + regression cap. Docs synced (SKILL.md, maui-ci-facts.md, workflow body + recompiled lock — body_hash re-pinned only). Continuing the adversarial loop until a round yields nothing.

[PureWeen]

Adversarial review — round 6

Ran another loop of three independent reviewers against the latest HEAD. They surfaced four attribution holes in the gate. All four are now fixed (commit 512a2acbf8); each is safe-by-construction — it can only raise the ceiling toward Needs human investigation, never relax it toward a false green.

#	Consensus	Hole	Fix
A	3/3	A new native crash / segfault / OOM / test-host crash / unhandled exception sharing a leg with a dismissable test failure had no coded signature → it was dropped as a fallback rollup and the dismissed test accounted for the whole build (false green).	Get-BuildErrorSignature now recognizes fatal non-coded breaks (exit-code signals 132–143/159, SIGSEGV/SIGABRT/core dumped, OOM, test-host crash, anchored Unhandled exception.) and routes them through the always-extract + baseline-matched path. The ordinary exit code 1 test-runner rollup is deliberately excluded (so normal test noise still suppresses). A pre-existing crash still baseline-matches and is dismissed (no false red); a new one surfaces/caps.
B	1/3	Wrapper-exception collision: Get-FailureReasonSignature took the first *Exception, so a PR NullReferenceException and a base TimeoutException both wrapped in AggregateException collapsed to one token → slipped past the reason-conflict veto.	Unwrap Aggregate/TargetInvocation/TypeInitializationException to the inner cause before signing.
C	1/3	Unknown-reason collision: the reason veto only fired when both reasons were positively known; a same-test red-on-both pair with unrecognized messages was dismissed as pre-existing.	Added a normalized message-fingerprint fallback (test failures only) that flags a conflict when the PR's message text is structurally absent from base for that test.
D	1/3	Cross-pipeline laundering: baseline keys from all pipeline definitions were merged into one definition-blind set, so a PR failure in pipeline A could be dismissed by a same-key base failure that only ever occurred in pipeline B.	Scope baseline matching to a shared pipeline definition when both sides are tagged (falls back to definition-blind when either is untagged, so no legitimate dismissal is lost).

The three 1/3 findings (B/C/D) were verified directly against the live code and each only adds vetoes/caps, so they were applied without a dispute round.

Assessed and intentionally not changed:

• Canceled leaf task with no error issue (1/3, medium): by AzDO semantics that's a cascade victim, not a root cause — the triggering timeout/failure carries its own error issue and is already swept, and the root cause is classified. A broad cap here would fire on nearly every red build (every parallel cascade-cancel), which would make the tool over-block to uselessness. The realistic PR-induced-hang variants (task timeout → failed+issue; job timeout → job infra-error) are already caught.
• Two low-severity contrived shapes (a single check name backed by two builds where one is inaccessible; a Helix fail count hidden under a non-fail field) — both require combinations the existing inspection paths don't admit.

Docs synced (skill contract, shared CI facts, workflow body); lock recompiled (body-hash-only). Gatherer AST-clean; 122/122 deterministic harness assertions pass.

[kubaflo]

Note

🤖 This review was generated by AI — an automated multi-model code review (Claude Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro), independent reviews reconciled via cross-pollination.

---

🔍 AI Multi-Model Code Review — Round 7

Panel HEAD: 2b9594e6 · Since the last panel (fd25781a): 5 commits, +720/−108 — a large false-green hardening pass + removal of the stray test.ps1.

⏱️ Note: while this panel ran, the head advanced to 512a2acb ("close four round-6 false-green paths"). I re-verified both findings against 512a2acb: the fingerprint digit-collapse is unchanged (still L490 — and the new 512a2acb message-fingerprint fallback reuses the same normalization, so it doesn't close the hole), and the device-test confirmedZero logic is unchanged (now ~L1573). The merge conflict is also still present. So this review applies to the current head.

---

🔴 Verdict: NEEDS CHANGES

Confidence: High — unanimous (all 3 models, independently and after cross-pollination)

This round closed ~16 false-green/precision holes and even cites prior panel findings by name in the comments — genuinely strong work. The panel still found one new reachable false-green introduced by the new fingerprint logic, plus a device-test edge, and the PR still can't merge.

---

❌ Blocker (still open, 4th round) — Merge conflict with main

The sole thing blocking merge, unchanged since R4:

• mergeable: CONFLICTING, mergeStateStatus: DIRTY; maui-pr / -devicetests / -reviewer / -uitests all action_required "Skipped due to merge conflicts" — they have not run.
• Action: merge/rebase main, then gh aw compile and commit the regenerated ci-status-*.lock.yml. Per Rule #6, required checks blocked by the conflict cap the verdict below LGTM regardless of code quality.

---

❌ New reachable false-green — Get-ErrorFingerprint digit-collapse (inline @ L490)

[regex]::Replace($t, '\d+', '#') collapses every digit run — including digits inside symbol identifiers — so CS0246: type 'Issue1234' and CS0246: type 'Issue5678' produce the same fingerprint. A PR-new build break then fingerprint-matches a different base break, gets alsoFailsOnBaseline = true, and is laundered as pre-existing. The panel traced the full green path: no other ceiling term catches the single colliding leg, so the verdict reaches Ready to merge with maui-pr RED for a real new compile break. This defeats the function's own docstring ("distinguishes two DIFFERENT breaks that share an error code").

• Severity: ❌ error (Opus + Gemini error; GPT warning — majority error). Narrow (collision on both sides, same record+code, identical modulo digits) and strictly better than R6, but a true "dismisses the only evidence" false-green.
• Fix: collapse only standalone integers — (?<![A-Za-z])\d+(?![A-Za-z]) — so counts/durations normalize but identifier digits stay distinct. (Details inline.)

---

⚠️ Device-test confirmedZero on partial Helix data (~L1573 at head 512a2acb)

If one maui-pr-devicetests Helix job's …/aggregated fetch throws (contributing nothing to $anyHelixFail/$anyHelixCount) while a sibling reported Failed == 0, deviceTestFailedConfirmedZero is set true and the green check is trusted though the thrown job is unknown. The token-less GitHub Actions path skips the test-API confirmation, so nothing compensates → a failure on the unreadable job can be missed. Unanimous ⚠️ warning. Fix: gate confirmedZero on -not $anyHelixReadError, or treat a thrown aggregated fetch as an unexplained leg. (Details inline.)

---

💡 Non-blocking

• Get-FailCountsFromObject 5000-node guard — the recursion cap could skip a deeply-nested Failed > 0, yielding a wrong confirmedZero. Narrow; worth a depth/teardown note. (Opus suggestion.)
• Pester harness still missing — test.ps1 was a scratch file (correctly removed), not a real harness. The gate is now ~+1300 lines of edge-case logic over 7 rounds; the fingerprint digit-collapse is exactly the kind of bug a truth-table test (Issue1234 vs Issue5678 must not collide) would catch. Reinforces the open R3 thread — not reposted inline.

---

🏁 Recommendation

REQUEST_CHANGES, but the gate is very close:

1. Resolve the merge conflict + recompile the gh-aw locks (the only actual merge blocker — 4 rounds running).
2. Tighten the fingerprint regex to standalone integers so it stops laundering numerically-suffixed symbols.
3. (Recommended) gate device-test confirmedZero on no-Helix-read-error, and finally add the Pester truth-table harness.

The hardening this round is impressive and has absorbed every prior finding — this is "fix one over-broad regex + rebase," not a redesign.

---

Reviewed with the code-review skill: independence-first, full-file reading in a worktree at PR HEAD, full green-path control-flow trace of the disputed fingerprint finding (Opus conceded suggestion→error with a unit-test repro), and CI/mergeability cross-checking by all 3 models.

[PureWeen]

Round 7 — adversarial review (three independent reviewers)

Three independent reviewers (different model families) re-reviewed the gate. Six findings were real and have been pushed as e5a7c61120; one was assessed and intentionally left as-is. Every fix is safe-by-construction — each can only add over-blocking (Needs human investigation), never remove a real block, so none can introduce a false green.

#	Reviewers	Finding	Fix
A	2/3	Get-ErrorFingerprint over-collapsed build errors: identifier-internal digits collapsed (Handler1 == Handler2) and a hard 120-char truncation dropped differentiating suffixes, so a PR-new break could fingerprint-match a base break of the same code.	Lookaround digit regex (only standalone counts collapse) + append a hash of the full normalized text past 120 chars.
D	2/3	Device-test green was confirmed from an unpaginated _apis/test/runs read summing only the first ~100-run page — a failing run in the tail read clean. (Retried device builds publish a new run per attempt, so >1 page is realistic.)	New Get-AzDoTestRuns pages the continuation token to completion; refuses positive confirmation when the run set is truncated.
B	1/3	helix-aggregated and azdo-test-results occurrences were untagged, so round-6 definition-scoping fell back to definition-blind → cross-pipeline laundering.	Both occurrences now carry buildDefinition.
C	1/3	A multi-inner AggregateException picked only the first non-wrapper exception, so a PR-introduced inner could hide behind a base-matching first inner.	Get-FailureReasonSignature now emits a sorted compound token of all inner exceptions.
E	1/3	A swallowed Helix aggregate read (one job threw, another read clean) could still set deviceTestFailedConfirmedZero.	Positive confirmation now requires every discovered job to be read without error.
F	1/3	A dismissible test failure with an empty errorMessage produced no reason and no fingerprint, so the name-only key dismissed it as pre-existing-on-base.	A zero-corroboration match (no reason and no message) is now forced to indeterminate.

Assessed, not fixed (G, 1/3): the assert:expected-actual reason token is coarse and numeric actuals collapse in the fingerprint fallback. Left as-is deliberately — the test is already red-on-base (dismissible by core philosophy), and flagging every numeric-actual delta would over-fire on flaky numeric/timing asserts (useless-tool risk). Non-numeric actuals already conflict correctly via the fingerprint.

The deterministic truth-table harness was extended to 153 assertions (all green), and the three docs (SKILL.md, maui-ci-facts.md, copilot-review-tests.md) were synced.

After seven rounds across three model families, the gate now closes every concrete false-green path surfaced. I'm stopping the adversarial loop here — remaining findings are in clear diminishing-returns territory.

[PureWeen]

Two production bugs found by running the gatherer against a live PR

Stress-testing the deterministic gatherer against a real PR (#34758, anonymous/production path) surfaced two genuine reliability bugs that the adversarial loop could not — both are now fixed in 9c4679d0b1.

#	Bug	Impact	Fix
PROD-1	@() array-subexpression on a List[object] throws ArgumentException ("Argument types do not match") from PowerShell's PSToObjectArrayBinder/MaybeDebase for certain element shapes	Crash under $ErrorActionPreference=Stop → the gatherer aborts and produces no gate at all (the workflow loses its deterministic floor)	The three gate List materializations (pendingChecks, failingChecks, unexplainedLegs) now use .ToArray() — a direct CLR call that bypasses the dynamic binder — instead of @()
PROD-2	_apis/test/runs silently ignores the buildIds filter and returns project-wide runs from the beginning of time (a maui build's query returned 2022-era Roslyn/runtime crossgen runs, build.id 602)	Latent false-green vector: phantom runs report zero failures, so their failedTests sum to 0 and could falsely set deviceTestFailedConfirmedZero over the real failing device-test build. Also pollutes every authenticated report with cross-repo noise	Scope by buildUri=vstfs:///Build/Build/<id> (honored server-side) + drop any run carrying an explicit mismatched build.id (defense in depth)

Verified end-to-end against #34758: the gatherer now runs to completion and the test-run set contains only that build's runs — 0 cross-repo contaminants (was 20). PROD-2 is dormant in today's anonymous production path (the authenticated test-API block is skipped without a token), but it is a real false-green landmine the moment an AZDO_TOKEN is ever supplied to the runner, so it is fixed now.

Deterministic harness extended to 167 assertions (PROD-1/PROD-2 coverage), all green.

[kubaflo]

Note

🤖 AI-generated status note (multi-model review automation).

Round 8 (ed1499fa) — both Round-7 findings are now fixed, so I'm posting a short status note rather than re-running a full panel:

• ✅ Fingerprint digit-collapse → now collapses only standalone integers ((?<![A-Za-z])\d+(?![A-Za-z])). Numerically-suffixed symbols no longer collide; the resolved inline thread has the details.
• ✅ Device-test confirmedZero → now guarded by -not $anyHelixReadError, so a thrown Helix aggregated-fetch no longer lets a partial green be trusted.
• ➕ Also in this push: "close six round-7 false-green paths", two production-reliability bug fixes, infra/flaky-leg regressed-vs-base suppression, and a ci-scan registry to demote single-base flakes. The gate continues to tighten nicely.

❌ Still the one blocker — merge conflict with main (now 5 rounds)

mergeable: CONFLICTING / mergeStateStatus: DIRTY; maui-pr / -devicetests / -reviewer / -uitests remain action_required "Skipped due to merge conflicts" — they still haven't run. This is the only thing keeping the PR from a green review. Please merge/rebase main, then gh aw compile and commit the regenerated ci-status-*.lock.yml so the required checks execute. Once they run green, the gate code itself is in good shape and I'll do a full green-path re-verification.

(The +1000-line gate is still without a Pester harness — the round-7 fingerprint bug is exactly what a truth-table test would have caught; strongly worth adding alongside the rebase.)

[kubaflo]

Note

🤖 This review was generated by AI — an automated multi-model code review (Claude Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro), tracked across 9 rounds.

---

🔍 AI Multi-Model Code Review — PR #36067 · Round 9

HEAD: 793ac43c (merged origin/main)

✅ Verdict: LGTM — the long-standing blocker is resolved

After 8 rounds, the one thing that kept this at NEEDS_CHANGES is now fixed.

🎯 The merge conflict is resolved

• Round 4–8 blocker: the PR was CONFLICTING/DIRTY with main (gh-aw ci-status-* locks), so the maui-pr* required checks were stuck at action_required "Skipped due to merge conflicts".
• Round 9 merges origin/main cleanly: now mergeable: MERGEABLE. No leftover conflict markers; both ci-status-*.lock.yml files are valid. maui-pr now correctly shows skipping (by-design .github/** path exclusion), and Skill Validation + Static validation pass.

🧠 The gate code was already cleared

The gatherer (Gather-TestFailureContext.ps1) is byte-identical to the Round 8 head — this round is purely the merge. Across rounds the panel verified, and the author fixed, every finding:

• R2 baseline-log-read → zero ✅ fixed (R3)
• R4/R5 false-green ceiling (compile breaks, job-level/log-throw/>12-cap) ✅ fixed via the structural $unaccountedFailingChecks guard
• R7 Get-ErrorFingerprint digit-collapse ✅ fixed ((?<![A-Za-z])\d+(?![A-Za-z]))
• R7 device-test confirmedZero on partial Helix data ✅ fixed (-not $anyHelixReadError)
• R6 source/sources slip ✅ fixed ($occ.source)
• Plus ~20 additional residual false-green/precision holes the author proactively closed.

The deterministic merge-readiness gate is now a well-hardened, 8-term ceiling that the panel could not break with a remaining false-green.

💡 One standing non-blocking suggestion

Still no Pester harness for this ~+1800-line load-bearing gate (open thread). Several rounds' bugs (the R7 fingerprint digit-collapse, the R6 field-name slip) were exactly the class a truth-table test would catch. Strongly recommended as a fast-follow — assert verdictCeiling for each shape (pending / inaccessible / unmapped / unexplained / unaccounted / unattributed / aborted / canceled / device-test-unverified / regressed / clean) to lock the no-false-green invariant. Two minor open nits (known-issue generic over-match; legsRegressedVsBase base retry-masking) remain as awareness notes.

🏁 Recommendation

LGTM. The blocker is cleared, the gate code is cleared by 9 rounds of multi-model review, and the relevant .github/skills/** validation passes. Ready for human approval. Adding the Pester harness alongside merge would be the ideal close.

---

Reviewed with the code-review skill: independence-first across 9 rounds, full-file reading in a worktree, manual control-flow tracing of every disputed false-green, and CI/mergeability verification.

[kubaflo]

Note

🤖 AI-generated approval (multi-model review automation, approving on @kubaflo's behalf).

✅ Approving — 3-model LGTM (Round 9).

After 9 rounds, the sole blocker (the 5-round merge conflict with main) is resolved by merging origin/main: now mergeable, maui-pr correctly skips (by-design .github/**), and Skill+Static validation pass. The deterministic merge-readiness gatherer is byte-identical to the multi-model-cleared Round 8 head — every prior finding (baseline-log-read→zero, the structural false-green ceiling, the fingerprint digit-collapse, device-test confirmedZero, the source/sources slip, plus ~20 proactive hardenings) was fixed and verified.

Fast-follow (non-blocking): add the Pester truth-table harness for the gate to lock the no-false-green invariant.

[PureWeen]

Adversarial review — Round 10 (3 independent reviewers, adversarial consensus)

Methodology: 3 independent reviewers traced the false-green gate from scratch against head 793ac43c; findings reconciled by adversarial consensus (a finding only survives if it's verified against source and points in the false-green direction — saying "Ready to merge" while a real PR-caused failure exists). All five surviving fixes are strictly conservative: each can only turn a green verdict into Needs human investigation, never the reverse.

All findings below are already fixed in 7f2b109994 (harness 198/198, AST-clean).

#	Finding	Consensus	Fix
A	Get-ErrorFingerprint collapsed digits adjacent to _ and backtick. Round-7 only excluded letter-adjacent digits, so two distinct same-code build breaks (Handler_1 vs Handler_2, generic arity Foo`1 vs Foo`2) fingerprinted identically → same dedup key → a PR-new break laundered as pre-existing.	2/3 (Reviewers 2 + 3, ❌)	Widened the standalone-count lookaround to the full identifier set [A-Za-z0-9_+""+]`.
B	Device-test confirmedZero (authenticated test-API) trusted failedTests==0 even when a run was incomplete/aborted (incompleteTests>0, unanalyzedTests>0, or state != Completed) — an aborted run that never emits a Failed outcome confirmed green.	3/3	Veto positive confirmation on any incomplete run. [Ignore]d / notApplicable tests are deliberately excluded, so the common case still confirms (no over-block).
C	Device-test confirmedZero (Helix aggregated path) let one job's clean count confirm green while a sibling job returned no count (countless/empty aggregate) or its shape-scan truncated at the 5000-node guard.	2/3 (class) (Reviewers 1 + 3)	Get-FailCountsFromObject now reports a truncated scan; the loop tracks anyHelixUnverified and refuses confirmation.
D	Baseline dismissal keys were pulled from older not-succeeded base builds in the lookback window when BaselineBuildsPerDefinition>1 — an older failure that was since fixed and is absent on the tip could dismiss a PR-reintroduced break. (No-op at the shipped default of 1.)	1/3 (Reviewer 1, ⚠️)	Only the most-recent completed base build contributes dismissal keys, matching the documented contract and the leg-map. Older builds contribute advisory notes only.
E	maui-ci-facts.md overstated the scope guard ("…unless this PR changes the test, its snapshot/baseline, or the platform code it exercises") — the implemented guard name-matches PR-edited test files only.	1/3 (Reviewer 1, ⚠️ doc)	Corrected the bullet so the doc no longer promises protection the code doesn't deliver.

Refuted (adversarial value): Reviewers 2 + 3 flagged finding A as a false green; Reviewer 1 initially argued the full-text SHA tail at L568-571 would disambiguate. That tail is only appended when the normalized text exceeds 120 chars — a typical short CS#### one-liner normalizes well under that, so it provides no protection and the collision is real. Net: finding A stands.

Verified sound, no change (no finding): verdict ceiling captures every non-green attribution (the green branch is unreachable while any cap term is non-zero); the ci-scan integration can only demote regressed-vs-base → indeterminate (still NHI, never green); cross-pipeline laundering scope guards; buildUri (not buildIds) test-run scoping; truncated-paging and per-run results-exception backstops; R2/R4/R5/R6/R7 prior-round fixes all present and sound.

Posted as a comment only — approval is a human decision.