Add "Input and Path Correctness" review dimension to maui-expert-reviewer

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

Adds a new review dimension (#31, Input and Path Correctness, severity [major]) to .github/agents/maui-expert-reviewer.md. MAUI's reviewer had no dimension covering external/caller-supplied values reaching file, path, process, parser, or navigation operations; this graduates that coverage in.

What it checks (each finding must carry a source → sink trace — origin, sink, missing guard):

• Archive/asset extraction path containment (entry-by-entry, stays under destination root)
• Path construction from external values normalized + prefix-checked
• External commands invoked via argument list, not concatenated strings
• Environment variables influencing path/arg/endpoint/credential selection are validated (excludes standard build vars)
• URIs, deep links, HybridWebView messages, picked-file names validated before use
• Deserialization constrains expected types / disables runtime type-name resolution (HybridWebView JSON bridge)
• High-confidence real credentials not committed (excludes placeholders/test data)

Design notes:

• Flat [major] — deliberately never escalates to [critical]. Further classification is left to the human reviewer.
• Every runtime finding requires a source → sink trace; committed credentials use a file-and-line exception.
• A global framing rule requires concrete mechanical failures without inferring intent.
• Conditional routing covers Resizetizer, Build.Tasks, engineering scripts, HybridWebView/WebView, FilePicker/MediaPicker/FileSystem, app-link surfaces, and Shell URI handling. Credential-like additions activate the dimension regardless of path.
• Wave 3 preserves validated committed-credential and archive/path-escape findings without displacing higher-severity findings.

Try-fix validation stabilization:

• Clarifies Pass / Fail / Blocked outcomes and iteration exhaustion while preserving explicit Blocked conditions.
• Updates the vally 0.10.0 capability scenarios to reason from supplied facts in a bare harness without searching for a live checkout.
• Keeps stated device unavailability authoritative so the no-device scenario still reports Blocked.
• Rubrics, graders, scoring thresholds, and trial counts are unchanged.

Reviewed with independent adversarial passes plus the MAUI domain reviewer; source→sink tracing and scoped CHECK triggers keep false-positive noise low.

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

Or

• Run remotely in PowerShell:

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

[Copilot]

Pull request overview

Adds a new “Input and Path Correctness” review dimension to the maui-expert-reviewer agent guidance, expanding the reviewer’s coverage to include source→sink tracing for external inputs that flow into file/path/process/parser/navigation operations.

Changes:

• Increments the reviewer dimension count from 30 to 31 and adds a new dimension section: 31. Input and Path Correctness.
• Adds a global “framing rule” to keep findings mechanical (no intent inference / no categorization by the agent).
• Introduces an Input & Path Boundaries routing table plus a conditional trigger to activate the new dimension based on diff characteristics and specific path patterns.

Show a summary per file

File	Description
.github/agents/maui-expert-reviewer.md	Adds the new review dimension, updates dimension count, and routes relevant file areas to the new dimension.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 1

[PureWeen]

Fixed in 272ab3c. Corrected the Android platform note — it now states the FilePicker/MediaPicker path may resolve to an absolute path outside the app sandbox (shared/external storage) before any copy into app cache, and instructs the reviewer to treat the returned path as external and apply containment/canonicalization before further path operations. Verified against FileSystemUtils.android.cs:59-69.

[kubaflo]

Note

🤖 This review was automatically generated by a multi-model AI review system (Claude Opus 4.8, GPT-5.5, Gemini 3.1 Pro). Three models reviewed independently, then cross-pollinated. Mechanical facts were verified against the repo.

Multi-Model Review — PR #36211

Verdict: ⚠️ NEEDS_CHANGES (2 of 3 models; the 3rd was LGTM-with-suggestions)
Confidence: Medium-high (the split is severity-calibration — all three agree the findings are real and the design is strong)

The design is genuinely good 👍

All three models praised this: the source → sink trace requirement ("no trace → no finding") is an excellent false-positive control, the flat-[major] / never-[critical] cap is a defensible deliberate choice to avoid LLM security-alarmism and defer severity to humans, and the mechanics are clean — I verified the numbering is continuous (28→29→30→31), the count is updated everywhere (the only remaining "30" is the Accessibility dimension number), and all 9 routing-table paths exist.

The items below are targeted, cheap fixes — not a rejection of the approach.

⚠️ 1. The committed-secrets CHECK contradicts this dimension's own gating rule (primary)

The dimension mandates that every finding "MUST show a source → sink trace … the operation it reaches (file write, process argument, deserialization, navigation) … No trace → no finding" (L442). But CHECK #7 (L452) covers committed secrets — a literal value at rest with an origin but none of the enumerated sinks. So under a strict reading of the gate, a committed-secret finding can never be emitted.

This isn't theoretical: our own panel split on it — GPT-5.5 read the trace gate as suppressing the secret CHECK, while Opus 4.8 assumed it fires at [major]. That disagreement is exactly the problem: an ambiguous gating rule will make the real sub-agents behave inconsistently on the one CHECK most likely to matter.
Fix: either explicitly exempt committed-secret findings from the source→sink-trace requirement, or move secrets to a dedicated check/dimension (they're a different shape from "input reaches a sink").

⚠️ 2. Routing omits the input surfaces the dimension explicitly names (unanimous — all 3 models)

The CHECKs and platform notes call out FilePicker/MediaPicker, picked-file names, and deep/app links — but the Input & Path Boundaries table (L528) doesn't route src/Essentials/src/FilePicker/**, MediaPicker/**, AppActions/**, Launcher/**, WebAuthenticator/**, or the AppLinks entry points (all of which exist). A PR editing FilePicker.android.cs to canonicalize a provider display name therefore wouldn't be path-routed to this dimension — it can only activate via the softer behavior-based Conditional trigger.
Fix: add those Essentials/AppLinks paths so the runtime input surfaces get the same path-based activation reliability as the build-time ones (Resizetizer, Build.Tasks).

💡 3. "categorize / category" is lexically overloaded (minor)

The global "Framing rule for all findings" (L475) says "Do not categorize a finding … never editorialize about a finding's category." The intent (don't assign a security/sensitivity/intent classification) is clear from context — but the term collides with (a) the What NOT to Flag table's own "Category" column header and (b) Wave 3's hard requirement that every finding embed **[severity] Dimension** (itself a categorization). A literal-minded sub-agent could soften its own dimension/severity labels.
Fix: tighten to e.g. "Do not assign a security or sensitivity classification, and do not speculate about intent."

💬 Discussion (non-blocking)

Opus noted the flat-[major] cap is in mild tension with Wave 2's security → critical rule, and combined with Wave 3's "≤15 findings, prioritize by severity", a genuinely severe mechanical failure (a committed live credential, a runtime zip-slip) reported at [major] could be crowded out below other dimensions' [critical] findings and dropped from the posted set. A one-line cross-reference (or exempting committed-secret findings from truncation) would harden against silent drop-through.

Model	Verdict	Confidence
Claude Opus 4.8	LGTM (w/ suggestions)	high
GPT-5.5	NEEDS_CHANGES	high
Gemini 3.1 Pro	NEEDS_CHANGES	high

---

Reviewed at head c084614f. CI: license/cla ✅ pass; maui-pr skipping (by-design .github/** path-exclusion). Agent-definition change — no build/test gate; reviewed as prompt design.

[PureWeen]

Multi-model adversarial review — PR #36211

Methodology: 3 independent reviewers analyzed the diff in isolation, then the one disputed finding was cross-validated by the other two (adversarial consensus). Claims were verified against the repo. Event is COMMENT — this review never approves or requests changes.

Verdict: ⚠️ 3 should-fix items, none merge-blocking. The design is sound and the prior-round fixes hold up: every routing path exists, the Android FilePicker/MediaPicker platform note is factually correct against FileSystemUtils.android.cs (EnsurePhysicalPath can return an external-storage absolute path before any cache copy), the trace-gate Exception cleanly unblocks the committed-credentials CHECK, and no consumer (code-review, try-fix, Review-PR.ps1) keys off the dimension count, heading numbering, or table shape — so the 30→31 bump and the new tables break nothing structurally.

Findings (all ⚠️ should-fix — see inline comments)

1. Flat-[major] cap collides with Wave-3 severity truncation — 2/3 (inline at L444–445)
Two facets: the Wave-3 retention carve-out (L445) sits inside the dimension body, but the ≤15 findings — prioritize by severity trimmer lives in Wave 3 (≈L630) with no back-reference, so the executor never sees it; and the cap has no stated precedence over the Wave-2 security/data-loss → critical rubric (≈L602), while the carve-out rescues only committed-credential findings — a Zip-Slip/traversal finding, also forced to [major], can still be trimmed below unrelated [critical]s. (Same tension a prior reviewer raised as non-blocking; FIX 5 partially addressed it, but consensus finds the compensating carve-out mislocated and under-scoped.)

2. Routing omits the WebView input layer — 3/3 (after dispute) (inline at L537)
src/Controls/src/Core/WebView/** (WebViewSourceTypeConverter, navigation event args) isn't path-routed; only the Core handler layer is. Breaks the Controls+Core pairing used one row above for HybridWebView.

Prior review status

copilot-pull-request-reviewer (COMMENTED) and kubaflo (CHANGES_REQUESTED) raised 4 inline points + 1 non-blocking discussion note; all were addressed in commit 272ab3c. That CHANGES_REQUESTED review predates the fix commit and is stale against the current head. The findings above are additional and were not among the resolved four — finding 1 extends kubaflo's non-blocking note about [major] crowd-out.

Test coverage

N/A — this PR changes a single agent-spec markdown file with no executable code paths; there are no unit/UI tests for agent prose. Consistency was validated by inspection (counts, numbering, routing-path existence, factual claims).

Non-blocking follow-up (outside this PR's changed file)

.github/skills/try-fix/SKILL.md:281 still says "not all 30" in prose — now stale at 31. Worth a follow-up touch since this PR is what bumped the count. Not filed as a finding: it's outside the single file this PR changes.

3 independent reviewers with adversarial consensus.

[PureWeen]

⚠️ Logic / Severity precedence — Two gaps around the flat-[major] cap:

1. No precedence vs the Wave-2 rubric. Wave 2 (≈L602) defines critical = "data loss, crash, infinite loop, security issue". A Zip-Slip / path-traversal finding (arbitrary-file write = data loss) textbook-matches that rubric, yet this line forces [major]. A validator told both things at once will tag it inconsistently — state which wins.
2. The L445 carve-out is under-scoped. It shields only committed-credential findings from the ≤15 trim; a genuine Zip-Slip finding — also forced to [major] — gets no protection and can be dropped below unrelated [critical]s.

Fix: either let the concrete data-loss/traversal CHECKs emit [critical] (matching the rubric), or extend the retention rule to all high-impact #31 findings — and add one line making a dimension's explicit cap take precedence over the general Wave-2 rubric.

Flagged by: 2/3 reviewers

[PureWeen]

Addressed in 27bfe1a. Added a precedence line to the Wave 2 severity block: a dimension's explicit severity cap (#31 is flat [major], never [critical]) now takes precedence over the general rubric, so a validator is no longer told two contradictory things. Kept the flat-[major] design intact (deliberately does not emit [critical]) and instead broadened the retention carve-out — see the Wave 3 reply.

[PureWeen]

Addressed. The retention/precedence rule now lives in Wave 3 (line 634) where the ≤15 trimmer actually reads it: the two Input-and-Path representatives are posted in addition to the top-15 (set may reach 17) and never displace a higher-severity finding from another dimension. Line 445 cross-references that Wave 3 rule, so [major] findings no longer rely on precedence language that sat outside the trimmer.

[PureWeen]

⚠️ Logic / Mechanism gap — Right idea, but this carve-out lives where the trimmer never reads it. The ≤15 findings — prioritize by severity truncation runs in Wave 3 (≈L630), which has no back-reference to this per-dimension retention rule. In this file's model, a dimension's body prose is consumed by that dimension's Wave-1 sub-agent, while truncation happens later in the top-level Wave-3 pass. Scenario: 15+ [critical] findings from other dimensions fill the cap; the Wave-3 executor sorts by severity and drops this [major] committed-credential finding — exactly what this line was written to prevent. Fix: move/duplicate the retention rule into the Wave 3 Rules block (near L630) so it sits where trimming actually occurs.

Flagged by: 2/3 reviewers

[PureWeen]

Addressed in 27bfe1a. Moved the enforcement into the Wave 3 trim rules (right next to the ≤15 cap, where the trimmer actually reads), and reworded the dimension-body note to point at that enforcement. Also broadened it beyond credentials to cover archive/path-escape findings so those aren't crowded out either.

[PureWeen]

⚠️ Completeness / Routing gap — This row routes only the handler layer and misses src/Controls/src/Core/WebView/**, which is where the input actually enters: WebViewSourceTypeConverter.cs parses a string → WebViewSource (URL/HTML) and WebNavigating/NavigatedEventArgs carry URLs. A PR editing those would not path-activate this dimension (the behavioral Conditional trigger is a heuristic a reviewer may not fire for a TypeConverter), and it breaks the Controls+Core pairing the HybridWebView row just above (L536) applies. Fix:

| `src/Core/src/Handlers/WebView/**`, `src/Controls/src/Core/WebView/**` | Input and Path Correctness, Logic and Correctness | all |

Flagged by: 3/3 reviewers (1/3 initial → confirmed 3/3 after dispute round)

[PureWeen]

Addressed in 27bfe1a. Paired src/Controls/src/Core/WebView/** with the Core handler row so the Controls-layer WebView surface is now routed to Input and Path Correctness + Logic and Correctness.

[kubaflo]

🤖 AI-generated review — multi-model panel (Claude Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro), independent reviews + cross-pollination. Round 2.

✅ LGTM (high confidence)

Thanks for the thorough follow-through — all three R1 findings are cleanly resolved, verified independently:

R1 finding	R2 fix	Verified
Secret/trace contradiction	Added an explicit Exception exempting the committed-credentials CHECK from the source→sink-trace requirement ("cite the file:line of the committed value instead")	✅ Scoped uniquely to CHECK #7; nothing else exempted
Routing gap	Added two rows — FilePicker/MediaPicker/FileSystem and AppActions/Launcher/WebAuthenticator/AppLinkEntry	✅ All 16 routed paths exist; tables well-formed
Framing-rule "categorize/category" overload	Reworded: "Beyond the required **[severity] Dimension** label, do not assign any sensitivity or risk classification…"	✅ Overload gone, meaning preserved

The reworded Android platform note is now factually accurate — Opus verified against FileSystemUtils.android.cs that file:// returns uri.Path directly and an external-storage authority returns an absolute ExternalStorageDirectory path before any cache copy, so R1's "resolve only into app-controlled locations" was a false safety claim; R2 correctly says "treat the returned path as external and apply containment/canonicalization." Numbering (#31), the "31 dimensions" counts, and cross-refs are all consistent.

Model	Verdict	Confidence
Claude Opus 4.8	LGTM	high
Gemini 3.1 Pro	LGTM	high
GPT-5.5	NEEDS_CHANGES	high (see note)

On the split: GPT-5.5 flagged the Android display-name clause as "overstating safety" — but Opus empirically verified (against the actual source) that the R2 note treats the path as external and is actionable, so that concern doesn't hold. GPT's other point (Browser not routed) is a fair, minor completeness gap — captured below as a suggestion.

💡 Non-blocking suggestions

• Route src/Essentials/src/Browser/** — Browser.OpenAsync takes a caller-supplied string/Uri and hands it to a platform open-URL API; it's the one remaining URI surface not path-routed (the behavior-based Conditional trigger still catches it). (GPT-5.5)
• Tighten the display-name clause to "treat provider-supplied display names as external and basename/containment-check them" rather than "are canonicalized," since canonicalization resolves traversal rather than rejecting it. (GPT-5.5 — optional polish; the surrounding note already treats paths as external)
• L444 still reads "…what category an issue is…" — coherent in context (seriousness), not a regression, but consider "how serious an issue is" for full consistency with the reworded framing rule. (Opus 4.8)

---

Reviewed at head 272ab3c0. CI: license/cla ✅ pass; maui-pr skipping (by-design .github/** path-exclusion).

[Copilot]

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 0 new

[kubaflo]

Note

🔍 AI-generated review (automated multi-model reviewer, on @kubaflo's behalf). A human maintainer makes the final call.

✅ R3 — LGTM (re-affirmed)

Incremental commit 27bfe1ae ("Address adversarial review: WebView routing + Wave 2/3 severity-cap") is doc-only (+4/−2 to .github/agents/maui-expert-reviewer.md) and a consistent refinement of the dimension #31 approved at R2:

• Routing: adds src/Controls/src/Core/WebView/** alongside src/Core/src/Handlers/WebView/** → Controls-side WebView changes now also activate Input and Path Correctness + Logic and Correctness. Sensible.
• Wave 2 severity: clarifies that a per-dimension severity cap (e.g. #31's [major]-only, no [critical]) takes precedence over the general rubric. Matches the dimension's own text.
• Wave 3 trim: the "keep at least one representative" rule now covers archive/path-escape findings in addition to committed-credential findings, and the ≤15-cap exception is stated explicitly.

Zero runtime/code impact (agent-instruction markdown). No new gate applies (maui-pr is by-design skipping on .github/**). Consistent with the base I already approved — no regression.

Verdict: LGTM (high). ● Approving.

[PureWeen]

Multi-model adversarial review — PR #36211 (round 2, head 27bfe1ae)

Methodology: 3 independent reviewers with adversarial consensus (parallel review + a dispute round on 1/3 findings). This round re-reviews the incremental commit 27bfe1ae from the prior adversarial round; findings targeting self-introduced changes from that round were held to a lowered discard threshold per multi-round self-correction.

Findings (2 — both wording refinements on lines introduced by the prior round; non-blocking)

⚠️ Logic — Wave 3 retention exception is ambiguous (exceed vs displace the ≤15 cap) · L632 · 3/3 reviewers (one rated must-fix)
The intended behavior — retain #31 reps in addition to the top-15 — is correct, but the wording also admits a "displace a [critical] within the cap" reading that would silently drop genuine crash/data-loss findings. Trivial one-clause fix. See inline.

⚠️ Consistency — or vs and mismatch in the retention sentence · L445 · 1/3, retained (self-introduced line, concrete reasoning)
L445 says retain "committed-credential or archive/path-escape" while the same sentence says "of each" and L632 says "and." An agent could drop one protected category. See inline.

Verified clean (no defect)

• Routing: every new path in the Input & Path Boundaries table exists in the repo — the WebView-pairing fix (src/Core/src/Handlers/WebView/** + src/Controls/src/Core/WebView/**), the Essentials FilePicker/MediaPicker/FileSystem/AppActions/Launcher/WebAuthenticator dirs, AppLinkEntry.cs, and the Shell URI handlers. No dead routes.
• Count bump 30→31 is consistent everywhere inside the agent file; no stray "30 dimensions" remains.
• Wave 2 precedence rule (per-dimension [major] cap takes precedence over the general rubric) is correctly placed and unambiguous — no path leaves a #31 finding stamped [critical].
• Cross-references ("the Wave 3 rules enforce…", "Input & Path Boundaries paths above", "committed-credentials CHECK below") all resolve to real anchors.
• Committed-credentials trace exemption is internally consistent. A third reviewer's concern that the general "Framing rule for all findings" conflicts with it was discarded after a dispute round (2 reviewers disagreed): the framing rule is anti-editorializing tone guidance, not a structural gate, and #31 already gives explicit local formatting for credential findings ("cite the file and line").

Test coverage

N/A — doc-only change to an agent instruction spec (.github/agents/maui-expert-reviewer.md); no runtime code, no automated tests apply.

Prior review status

The automated multi-model reviewer (@kubaflo) and the Copilot bot both re-reviewed 27bfe1ae — green / no new comments — and all earlier inline threads are resolved. The two findings above are minor wording tightenings.

Non-blocking follow-up (outside this PR's single-file scope — not a finding against this PR)

• .github/skills/try-fix/SKILL.md:281 still reads "not all 30"; the conceptual 30→31 bump makes it stale. Separate file, out of this PR's declared scope.

[PureWeen]

⚠️ Logic — The ≤15-findings trim rule and this retention exception can be read two ways, and one reading silently drops genuine [critical] findings.

• "Exception to the ≤15 cap" reads as post more than 15 (retained [Spec] Linux parity does not appear in wiki #31 reps are added beyond the cap).
• "even when higher-severity findings from other dimensions would otherwise fill the limit" reads as evict a [critical] to make room within 15.

Concrete case: a PR yields 15 [critical] findings from other dimensions plus a committed-credential and an archive/path-escape finding from #31. The "exceed" reading posts 17; the "displace" reading posts 15 and silently drops 2 real [critical] (crash/data-loss) findings — two runs can legitimately pick either branch.

Suggested fix: state the mechanic explicitly, e.g. "these retained #31 findings are posted in addition to the top-15 (the set may reach 17); they never displace a higher-severity finding."

Flagged by: 3/3 reviewers (one rated must-fix). Targets a line introduced by the prior review round.

[PureWeen]

Addressed in 8d910b0. Made the mechanic explicit: retained #31 representatives are posted in addition to the top-15 (the set may reach 17) and are never swapped in for, and never displace, a higher-severity finding from another dimension. Removes the "exceed vs displace" ambiguity.

[PureWeen]

⚠️ Consistency — Disjunctive vs conjunctive mismatch inside this retention rule.

This sentence says retain "a committed-credential finding or an archive/path-escape finding" (keep at least one of the two), but the same sentence then says "keeping at least one representative of each," and the Wave 3 rule at line 632 says "committed-credential finding and one archive/path-escape finding."

Concrete case: a PR has exactly one committed-credential finding and one archive/path-escape finding among 16+ total. An agent anchoring on the "or" here could retain just one category and drop the other — satisfying this clause while violating line 632's "and".

Suggested fix: change "or" → "and" here to match "of each" and line 632.

Flagged by: 1/3 reviewers; retained under multi-round self-correction (self-introduced line from the prior round, with concrete reasoning).

[PureWeen]

Addressed in 8d910b0. Changed "committed-credential finding or an archive/path-escape finding" to "and", so this sentence now agrees with its own "of each" and with the "and" in the Wave 3 rule (L632).

[PureWeen]

/evaluate-skills

[Copilot]

Copilot's findings

• Files reviewed: 3/3 changed files
• Comments generated: 4

[PureWeen]

Intentional and in scope. The try-fix Pass/Fail/Blocked clarification (Lever A) plus the vally stimulus preamble (Lever B) were added to make the try-fix Skill Validation eval non-flaky — the same reviewer tooling this PR extends. The eval now passes at 0.86 with no timeout/error trials. Rubrics, graders, and thresholds are byte-for-byte unchanged; only the skill prose and scenario framing changed.

[PureWeen]

/evaluate-skills

[Copilot]

Copilot's findings

• Files reviewed: 3/3 changed files
• Comments generated: 2

[kubaflo]

🔍 AI-generated review (multi-model pipeline: Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro), on behalf of @kubaflo — round 6 at 1ddd1a78.

Multi-model review — PR #36211 (review dimension #31 "Input and Path Correctness")

Since my round-5 LGTM (b48a748c) the branch rebased onto newer main and added the substantive work across 3 files: dimension #31 in the reviewer, a try-fix SKILL bump, and try-fix eval stabilization.

New content — verified

maui-expert-reviewer.md — dimension #31 "Input and Path Correctness" [major]. This is well-designed review guidance:

• Source → sink trace is mandatory ("where the value originates → the operation it reaches → the specific missing guard; no trace → no finding"), with a single narrow exception for committed-credentials (value-at-rest). This is exactly the discipline that keeps a security-flavored dimension from emitting vague/speculative findings.
• CHECK rules cover the real mechanical bug classes: Zip-Slip/path-escape on archive extraction, path-prefix checks that account for directory-boundary false matches (/root vs /rootother), argument-list vs concatenated command strings, env-var validation (sensibly excluding DOTNET_*/ANDROID_HOME), external-input validation (URIs, deep/app links, HybridWebView bridge messages, picked-file names), deserialization type constraints, committed secrets.
• Framing rule forbids intent/sensitivity speculation ("a change that appears to harden against misuse may simply be a correctness fix — never assume") — the classification is left to the human. Good guardrail against false "security fix" labeling.
• Severity discipline: every finding capped at [major] (never escalates to [critical]), and a Wave-3 retention rule keeps ≥1 committed-credential and ≥1 path-escape finding even past the top-15 cap (set may reach 17). Coherent with the try-fix dimension-count bump 30→31.
• Path→dimension routing targets the right MAUI surfaces (HybridWebView, WebView, ShellUriHandler/ShellNavigationManager, FilePicker/MediaPicker/FileSystem, AppActions/Launcher/WebAuthenticator/AppLinkEntry, Resizetizer, Build.Tasks, eng/cake, eng/scripts).

Head commit 1ddd1a78 extends the eval preamble to the two heavy fix stimuli ("No live repository… is attached… do not report Blocked merely because the environment is bare; reason from the scenario and give a documented verdict") — a targeted fix for the bare-sandbox "Blocked" flakiness in happy-path-distinct-alternative-fix.

CI — the red is the env-gate, not this PR

The PR checks show evaluate (try-fix) + Harness hermeticity gate red, but both failed in 1–2 s with no failed step, annotated "Branch 'main' is not allowed to deploy to copilot-pat-pool due to environment protection rules." That's the pull_request_target environment gate (not PR-caused). The authoritative run is green: results comment Overall Passed · LLM Passed · Hermetic ✅, try-fix-capabilities 0.86 / 0.60 ✅ (1/1 suite met threshold), and the required "Skill Validation" aggregate check = success.

Findings

• ⚠️ Residual scenario flakiness (non-blocking). happy-path-distinct-alternative-fix is still 🟡 flaky at head — pass@k 100% but pass^k 29.6% (❌ prompt 2/3). The suite clears threshold, and the head commit's preamble is aimed squarely at this, but the fix's stabilizing effect isn't yet demonstrated across runs. Worth one or two confirmation runs before treating this scenario as settled.

Verdict: 🟢 LGTM (substance) · confidence high

Dimension #31 is high-quality, well-scoped review guidance (trace-required, mechanical-only framing, sensible severity cap + retention), and the authoritative eval passes green with a genuinely hermetic harness. Posted as a comment rather than an approval only because the happy-path-distinct-alternative-fix scenario remains visibly flaky (pass^k ~30%) and the head commit is an in-flight stabilization for exactly that — a stable-green confirmation run would make this a clean approve.

_{ℹ Automated multi-model review (Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro), cross-pollinated and verified against the files + live eval results at 1ddd1a78. Advisory — a human maintainer makes the final merge decision.}

[Copilot]

Copilot's findings

Comments suppressed due to low confidence (3)

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

• This paragraph implies committed-credential and archive/path-escape findings will always exist (“do not drop … a committed-credential finding and an archive/path-escape finding”). In many PRs there may be none, and the current wording can encourage inventing findings to satisfy the rule. Make the “retain representatives” guidance conditional on such findings being present.

Report all findings at `[major]`. This dimension does not escalate to `[critical]`; leave any judgment about how serious or what category an issue is to the human reviewer.
When Wave 3 trims to the top findings by severity, do not drop this dimension's highest-impact findings — a committed-credential finding and an archive/path-escape finding — solely because other dimensions contributed `[critical]` items; the Wave 3 rules enforce keeping at least one representative of each in the posted set.

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

• Wave 3’s exception rule uses “always retain at least one … finding” language, which is impossible to satisfy when a PR has zero committed-credential or archive/path-escape findings. This can push the agent toward fabricating findings. Make the exception conditional (“if any exist”).

Rules:
- Group related findings on adjacent lines into a single entry
- Limit to ≤15 findings — prioritize by severity
- Exception to the ≤15 cap: from Input and Path Correctness (#31), always retain at least one representative committed-credential finding and one archive/path-escape finding — these are posted **in addition to** the top-15 (the set may reach 17); they are never swapped in for, and never displace, a higher-severity finding from another dimension
- Exclude findings already present in existing PR comments (checked in Wave 0 step 5)

.github/skills/try-fix/SKILL.md:158

• The new guidance makes Blocked both mandatory (when no device/emulator) and “a last resort”, which is internally inconsistent and can lead to unpredictable verdicts. Clarify that Blocked is only for cases where device execution is the sole missing validation step, and allow a documented Fail when the approach can be conclusively ruled out without running the test.

**If device/emulator is unavailable:** Report `result.txt` = `Blocked` with explanation. Do NOT manufacture a Pass.

**`Blocked` is a last resort, not a default.** Reserve it for when you genuinely cannot determine *any* outcome — e.g. the code compiled but there is no device/emulator to run the test, so no Pass/Fail is knowable. It is NOT the right result when you were handed the bug, the prior failed attempts, and root-cause evidence: that is enough to reason to a conclusion. An unfamiliar or bare working directory is not by itself `Blocked` — work from the problem context you were given, and report a documented `Fail` (with analysis) rather than stopping at `Blocked` when the evidence already points to one.

• Files reviewed: 3/3 changed files
• Comments generated: 0 new

[PureWeen]

Adversarial review verdict: no remaining findings.

The review initially identified four concrete issues; all are addressed on head 834beec247:

• The no-device vally scenario now preserves the required Blocked verdict instead of conflicting with its rubric.
• Credential-like additions activate Input and Path Correctness outside the path-routing table.
• Wave 3 retention applies only when matching findings survived validation, avoiding any implication that findings should be manufactured.
• Bare-environment guidance no longer overrides explicit Blocked conditions such as unavailable devices or missing baseline files.

Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist. The final diff remains limited to the reviewer agent, try-fix skill, and its vally capability suite. The capability YAML parses with all 8 stimuli, and the changed scenario directly covers the corrected verdict contract. Existing review threads on earlier wording are superseded or answered; a fresh human approval is still required after the latest push.

[Copilot]

Copilot's findings

Comments suppressed due to low confidence (1)

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

• The routing table lists some path patterns in multiple places (e.g., eng/** and src/Controls/src/Build.Tasks/** are already covered under Build & Engineering, but are repeated again under Input & Path Boundaries). This overlap makes it easy for a reviewer/agent to pick only one row and accidentally miss dimensions. Consider consolidating so each path pattern appears once, with the union of required dimensions.

| Path Pattern | Dimensions | Platform |
|---|---|---|
| `eng/**`, `src/Controls/src/Build.Tasks/**` | Build & MSBuild, Regression Prevention | all |

### Input & Path Boundaries

• Files reviewed: 3/3 changed files
• Comments generated: 0 new

[kubaflo]

Adversarial re-review — PR #36211 (834beec)

Three non-duplicate major findings identify contradictory or tautological Vally stimuli. This corrects my earlier assessment of the eval stabilization after tracing the prompts against the unchanged execution rubrics.

CI: Skill Validation failed before Vally at the base workflow's fork-checkout guard, so evaluation and hermeticity were skipped; maui-pr was also skipped. The checkout failure is not caused by this diff, but final-head Vally behavior remains unverified.

Verdict: NEEDS_CHANGES. Confidence is low overall because validation did not execute, but high for the inline findings.

[kubaflo]

[major] Regression Prevention — This new preamble asks the agent to reason hypothetically while the unchanged rubrics still grade live baseline, artifact, test, and restore behavior. Scenario 1 requires saved protocol artifacts and a clean restore; scenario 5 exists specifically to verify EstablishBrokenBaseline.ps1 -Restore rather than raw git. Those checks can now pass from narrated intent without exercising either behavior, contradicting the suite header's claim that these are live behavioral-protocol tests. Give these stimuli a controlled fixture/worktree (as sibling suites do), or explicitly make the suite plan-only instead of overriding discovery and execution.

[kubaflo]

[major] Regression Prevention — report Blocked supplies the exact graded answer in the suite's only positive Blocked verdict-integrity scenario. An agent that would otherwise regress to claiming Pass without running the device test is now instructed to emit the expected verdict, so the scenario measures prompt following rather than whether the skill derives the result correctly. Keep the stated device unavailability, but remove the required verdict and let the skill infer it.

[kubaflo]

[major] Logic and Correctness — This preamble says no device is attached, but this scenario's rubric requires Fail after exhausted attempts. The skill's canonical table and lines 156/158 preserve unavailable-device outcomes as Blocked, so a protocol-compliant response fails the rubric while a response that ignores the device rule passes it. This scenario is about iteration exhaustion, not device availability; remove or device or provide completed-attempt evidence instead.