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 or caller-supplied values reaching file, path, process, parser, or navigation operations.

What it checks

• Archive and asset extraction path containment, including equivalent containment enforced by bulk extraction APIs
• Path construction from external values
• External command argument construction
• Environment variables influencing paths, arguments, endpoints, or credential selection
• URIs, deep links, HybridWebView messages, and picked-file inputs
• Deserialization type constraints
• High-confidence committed credentials

Design notes

• Every runtime finding requires a source → sink trace; committed credentials use a current-diff file-and-line exception.
• Findings are always [major] and never escalate to [critical].
• The 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.
• Wave 3 retains validated committed-credential and archive/path-escape representatives without displacing higher-severity findings.

This PR changes only .github/agents/maui-expert-reviewer.md.

[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

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

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

[Copilot]

Copilot's findings

Suppressed comments (1)

.github/skills/try-fix/tests/eval.vally.yaml:74

• The tool-calls grader config here uses a list of bare strings under disallowed. In this repo’s other Vally suites, tool-calls.config.disallowed entries are objects (e.g., { name: '^(bash|powershell)$', command: ... }). If the grader schema expects objects, the current shape may be ignored or rejected, which would let non-skill tool calls slip through these scenarios. Consider switching to the object form (at least name:) for all tool-calls graders in this file so the restriction is reliably enforced.

      - type: tool-calls
        config:
          disallowed:
            - "^(?!skill$).*"

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

[kubaflo]

@PureWeen Fresh adversarial re-review of ee3f2e096f9 found three current instruction/eval defects. All earlier review threads are resolved; these findings are new to the latest head.

Verdict: NEEDS_CHANGES — confidence: low

• The unsupported-Pass regex misses the try-fix skill's canonical **Result:** Pass output while rejecting benign explanations that quote ✅ PASS.
• The only negative-trigger scenario now tells the model not to load a skill or call tools, directly supplying the behavior that 70% of its graders measure.
• The new archive rule mandates a major finding for safe external archives even when the extraction API already enforces destination containment.

CI: Required maui-pr is skipped. Skill Validation did not evaluate this head because actions/checkout refused fork code in the pull_request_target workflow before lint/evals ran.

Title/body: The body now discloses both the reviewer dimension and try-fix work, but its claim that 0.92 makes every static grader pass should be scoped to the per-trial/JUnit gate. The title still describes only the reviewer dimension despite the material try-fix protocol/eval changes.

_{Adversarial ensemble: Claude Opus 5, GPT-5.6 Sol, Gemini 3.1 Pro, GPT-5.6 Terra, and the MAUI expert reviewer; independently reviewed, cross-pollinated, and empirically checked against Vally 0.10 and .NET archive extraction.}

[Copilot]

Copilot's findings

Suppressed comments (2)

.github/workflows/skill-validation.yml:245

• allow-unsafe-pr-checkout: true explicitly opts into the high-risk pull_request_target fork checkout path. To avoid future accidental expansion of this workflow into executing PR-controlled code, add an inline comment documenting why this is enabled and the invariant it relies on (no execution of PR code).

          persist-credentials: false
          allow-unsafe-pr-checkout: true

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

• The reporting template uses ✅ PASS / ❌ FAIL / ⚠️ BLOCKED, but the skill’s canonical result values elsewhere are Pass / Fail / Blocked (including the result field contract and result.txt). Aligning casing reduces the chance an invoker copies an invalid token into automation.

**Result:** ✅ PASS / ❌ FAIL / ⚠️ BLOCKED

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

[kubaflo]

@PureWeen — adversarial re-review of exact head 3857a3e, using Opus 5, GPT-5.6 Sol, Gemini 3.1 Pro, cross-pollination, MAUI expert verification, and GPT-5.6 Terra arbitration.

Verdict: NEEDS_CHANGES — findings high confidence; CI undetermined

One new Major finding is inline: the PAT-bearing evaluation jobs restore fork checkout past the checkout v4 safeguard, then consume PR-authored Vally surfaces that can execute commands with the inherited environment. This needs a technical boundary rather than documentation alone.

The three prior Major threads remain valid, current, and unresolved:

• The Pass matcher misses the canonical result and also over-matches quoted or negated text.
• The only negative-trigger probe supplies the no-skill/no-tool behavior being graded.
• The archive rule mandates findings even when the extraction API already enforces destination containment.

The 0.92 threshold is applied per trial and JUnit fails on any sub-threshold trial, so all 24 trials (8 scenarios x 3 runs) must clear it. That strictness should be documented, but I am not treating it as a separate defect.

Metadata/CI: the title under-describes the try-fix validation changes, and the body omits the checkout security-model change and overstates false-positive resistance. Skill Validation currently stops in the base workflow checkout before lint/evaluation; maui-pr is skipped and Build Analysis is pending, so exact-head CI has not established merge readiness.

Merge readiness: not ready.

[Copilot]

Copilot's findings

Suppressed comments (4)

.github/skills/try-fix/tests/eval.vally.yaml:284

• This regex mixes two alternatives under a single inline-flag prefix without grouping. To avoid any ambiguity across regex engines about how the (?im) flags and anchors apply to each alternative, wrap the alternation in a non-capturing group (e.g., (?im)(?:...|...)) so flags/anchors clearly apply to both branches.

          pattern: '(?im)git reset --hard|^[ \t]*(?:[-*][ \t]+)?(?:\*\*)?(?:Result|Verdict|Outcome)(?:\*\*)?[ \t]*:[ \t]*(?:\*\*)?[ \t]*(?:✅[ \t]*)?Pass\b'

.github/skills/try-fix/tests/eval.vally.yaml:54

• The eval suite now enforces a plan-only interaction model (no tool calls beyond skill invocation, no file writes, no claims that commands ran). However, SKILL.md still emphasizes that try-fix is empirical (implement + run tests). To prevent confusion for contributors and future test authors, consider documenting this plan-only evaluation mode explicitly in SKILL.md (or in a short header comment in this eval file) as a special harness constraint distinct from the real try-fix execution protocol.

      This is a plan-only capability probe. Invoke the try-fix skill to load its protocol, but do not call any other tool, create files, or claim that any command ran. Explain how you would carry out the protocol and derive the verdict solely from the scenario facts below.

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

• The doc now states the run ends in exactly one of Pass/Fail/Blocked, but the output template uses uppercase + emojis (PASS/FAIL/BLOCKED). If downstream tooling parses result.txt or the structured output, this casing mismatch can cause confusion. Consider explicitly stating the canonical machine-readable tokens/casing for result.txt vs. the human-facing summary line (or align both to the same casing).

6. **Documented outcome** - Every run ends in exactly one of `Pass` / `Fail` / `Blocked` with written reasoning. Never stop silently, never claim `Pass` without a test that actually passed, and when attempts are exhausted report `Fail` with a root-cause analysis rather than continuing to propose fixes

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

• The doc now states the run ends in exactly one of Pass/Fail/Blocked, but the output template uses uppercase + emojis (PASS/FAIL/BLOCKED). If downstream tooling parses result.txt or the structured output, this casing mismatch can cause confusion. Consider explicitly stating the canonical machine-readable tokens/casing for result.txt vs. the human-facing summary line (or align both to the same casing).

**Result:** ✅ PASS / ❌ FAIL / ⚠️ BLOCKED

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

[PureWeen]

/evaluate-skills

[Copilot]

Copilot's findings

Suppressed comments (3)

.github/workflows/daily-repo-status.lock.yml:1

• In this lock file, COPILOT_MODEL for the detection phase was previously set to a sentinel value (detection) and is now pinned to the main agent model. If detection has special meaning in gh-aw (e.g., selecting a dedicated detection configuration/model or behavior), this change can alter or break the detection step. Recommended fix: keep COPILOT_MODEL: detection where the workflow is performing detection, and only pin the main agent model for the agent phase.
  .github/skills/try-fix/tests/eval.vally.yaml:54
• This suite now requires invoking try-fix while simultaneously forbidding any tool calls other than the skill tool and instructing the agent not to create files or claim commands executed. That appears incompatible with the try-fix skill’s documented behavior (it’s explicitly empirical and produces artifacts). If the intent is to test a ‘dry-run/plan-only’ mode, the skill itself (or the eval harness) likely needs an explicit plan-only switch that suppresses tool execution/artifact writing; otherwise these tests may systematically fail or incentivize the skill to violate its own contract.

      This is a plan-only capability probe. Invoke the try-fix skill to load its protocol, but do not call any other tool, create files, or claim that any command ran. Explain how you would carry out the protocol and derive the verdict solely from the scenario facts below.

.github/skills/try-fix/tests/eval.vally.yaml:74

• This suite now requires invoking try-fix while simultaneously forbidding any tool calls other than the skill tool and instructing the agent not to create files or claim commands executed. That appears incompatible with the try-fix skill’s documented behavior (it’s explicitly empirical and produces artifacts). If the intent is to test a ‘dry-run/plan-only’ mode, the skill itself (or the eval harness) likely needs an explicit plan-only switch that suppresses tool execution/artifact writing; otherwise these tests may systematically fail or incentivize the skill to violate its own contract.

      - type: skill-invocation
        config:
          required:
            - try-fix
      - type: tool-calls
        config:
          disallowed:
            - "^(?!skill$).*"

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

[Copilot]

Copilot's findings

Suppressed comments (2)

.github/workflows/daily-repo-status.lock.yml:1

• This changes the detection-phase model from a special-purpose value (detection) to a general LLM model. If the workflow relies on a distinct detection model/behavior (cost controls, stricter sandboxing, specialized scoring), this can break detection semantics or increase operational cost. Consider keeping the detection-phase COPILOT_MODEL as detection (or a dedicated detection model variable) while only changing the agent-phase model.
  .github/workflows/rerun-review-scanner.lock.yml:1
• Hard-coding the model in generated lock workflows removes the ability to override model selection via repo/org variables (previously supported via vars.GH_AW_MODEL_AGENT_COPILOT / defaults). If you still want configurability, prefer keeping the variable-based expression in the source workflow and regenerating locks, or introduce a single variable (e.g., GH_AW_DEFAULT_MODEL_COPILOT) that can be updated without code changes.

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

[Copilot]

Copilot's findings

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

[kubaflo]

@PureWeen — adversarial re-review of 8d5bcb2049 completed with three independent frontier-model passes, three cross-pollination passes, MAUI expert verification, and final GPT-5.6 Terra arbitration.

Prior-finding reconciliation: the two unresolved threads are outdated and target .github/skills/try-fix/tests/eval.vally.yaml, which is absent from the rewritten current diff. Copilot's exact-head review reported no new findings. Neither retained issue appears on the review, inline-comment, or issue-comment surfaces.

CI: required maui-pr is skipped and Build Analysis is pending, so exact-head CI coverage remains undetermined.

Verdict: NEEDS_CHANGES. The two inline findings are reproducible instruction-level false negatives. Confidence in merge safety is low while required CI has not run.

[kubaflo]

⚠️ [major] Input and Path Correctness — an ‘argument list’ does not necessarily preserve argument boundaries

This marks any argument-list invocation as safe, but PowerShell’s Start-Process -ArgumentList joins the array into one string before the target reparses it. A caller-derived value containing spaces or quotes can therefore change how the command parses even though the reviewer sees an array. I reproduced this with pwsh 7.5.4: one logical --config /safe/path value arrived as two argv entries, while .NET ProcessStartInfo.ArgumentList preserved it. Please require a boundary-preserving API and explicitly exclude Start-Process -ArgumentList from the safe pattern.

[kubaflo]

⚠️ [major] Input and Path Correctness — the location-based exclusion suppresses real credentials in fixtures

The check first includes high-confidence real credentials committed in test fixtures, then excludes sample/test data outright. A genuine PAT or connection string added to a fixture therefore satisfies both clauses and can be ignored solely because of its location. Please limit the exception to values shown to be synthetic (for example, obvious placeholders or dummy values), regardless of whether the file is production, sample, or test data.