Fix skill evaluation fixtures and failure reporting

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

Description of Change

This follow-up repairs the skill-evaluation failures exposed by #36799 and hardens the credentialed Vally evaluator so skill regressions, infrastructure failures, incomplete trials, parser differences, and reporting-side failures are classified accurately.

Root Causes

• try-fix-capabilities could exceed its numeric threshold while remaining red because Vally independently hard-fails execution errors and incomplete trials. Aggregate score never overrides those vetoes.
• pr-review-gh-auth routed correctly, but its isolated fixture omitted the phase-specific unauthenticated public-GitHub fallback required by the grader.
• Later failures exposed evaluator defects rather than skill regressions: PAT authentication did not prove model entitlement, model discovery did not initially match Vally 0.12's effective execution plan, matrix-only token offsets removed run-level rotation entropy, and parallel legs amplified shared PAT contention.
• Run 31518503277 restored the synthetic target correctly, but one agent deleted Vally's pre-existing untracked try-fix/ input to force an empty git status; Vally correctly treated that deletion as workspace drift.
• Run 31524806025 completed every evaluation successfully, but a transient GitHub API fetch failure occurred before the comment step exported its already-computed verdict, making reporting red despite green suites.
• Review found parser-semantic splits between Psych and Vally's pinned yaml@2.9.0: duplicate keys could be first-wins in AST policy checks but last-wins at runtime, while YAML merge keys/tags were interpreted differently and could hide executable evaluator fields from trusted validation.

Changes

• Replaced unreliable operational probes with bounded fixtures while preserving repeated trials, behavioral rubrics, strict thresholds, and hard-veto semantics.
• Added the unauthenticated GitHub fallback fixture and deterministic skill-invocation/tool-call graders.
• Upgraded to Vally 0.12 and made aggregation fail closed on execution errors, incomplete/skipped stimuli, malformed output, invocation failures, and missing mandatory coverage.
• Added trusted fixture preparation plus path, symlink, Git, environment, executable, attachment, hook, settings, MCP, executor, and repository-control isolation.
• Serialized matrix legs while retaining three trials per stimulus.
• Made model discovery use only executed eval*.vally.yaml specs and Vally 0.12's effective executor and prompt-grader fields; implicit prompt-grader fallbacks are rejected.
• Added model-aware PAT selection with run/rerun/matrix-distributed rotation and exact sentinel probes through the pinned evaluator runtime; reused it in the advisory soak.
• Clarified that try-fix must preserve every pre-existing untracked path, including evaluator-loaded skill inputs. Trusted validation enforces both the skill contract and restoration fixture instruction while retaining diff-empty as the method-independent veto.
• Exported the authoritative evaluation verdict before PR-comment API calls, added bounded API retries, and made comment publication advisory so transient GitHub failures cannot erase or invert the commit status.
• Reduced soak workflow permissions to contents: read and made /user transport failures explicit without exposing PATs.
• Made required-stimulus policy fail closed when a configured stimulus is renamed or removed, and bound each fixture marker/ref to the YAML stimulus that owns it so a correctly named decoy cannot satisfy the policy.
• Parse each eval spec once and reject duplicate semantic mapping keys, textual YAML merge keys, and normalized explicit merge tags recursively before loading. This prevents Psych/Vally parser disagreement from bypassing prompt, ref, repository-control, or executable-environment validation.
• Limited frozen-worktree test detection to full immutable commit IDs; ordinary explicit branch bases continue using authoritative PR file metadata.
• Added deterministic regressions for model planning, PAT distribution and transport errors, deleted evaluator controls, host-only XAML invocation, evaluator-owned input preservation, required-stimulus rename/decoy/parser bypasses, branch-versus-immutable test detection, and reporting-side verdict preservation.

Validation

Current head: fd8f3d543e1a1eb90e8432a0cbae07a18b52dbdd.

• Exact-head Skill Validation run 31807806898, maui-pr build 1553690, Build Analysis, CLA, static checks, Pester, and the hermeticity gate are green.
• All evaluated suites passed. Key suites: try-fix-restore-protocol 1.00 / 1.00, try-fix-capabilities 0.93 / 0.80, pr-review-gh-auth 1.00 / 0.80, and verify-tests-fail-without-fix-capabilities 0.94 / 0.87.
• Local trusted evaluator/reporting harness: 70 tests, 306 assertions, 0 failures/errors/skips.
• All 16 Vally specs pass strict lint with pinned @microsoft/vally-cli@0.12.0.
• All 8 skill test directories pass trusted validation and resolve the effective plan to gpt-5.6-sol plus claude-opus-5; non-eval soak/hermeticity specs do not contaminate matrix selection.
• Ruby syntax, Bash syntax, ShellCheck, workflow YAML parsing, Pester (8 passed), and git diff --check pass.
• Final whole-diff review found no high-confidence correctness or security blockers; all review threads are resolved.
• pull_request_target executes trusted workflow/helper code from default main; candidate selector/reporting helper behavior is therefore covered deterministically here and becomes the credentialed trusted path after merge.

Issues Fixed

Follow-up to #36799.

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

Or

• Run remotely in PowerShell:

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

[azure-pipelines]

Azure Pipelines:
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR updates the MAUI skill-evaluation (Vally) fixtures and the CI reporting logic so eval results are both deterministic (fixtures materialized correctly) and accurately reported (execution errors vs threshold failures).

Changes:

• Improve the skill-validation workflow’s JUnit parsing/reporting to explicitly surface execution errors and failed trials instead of implying “threshold-only” outcomes.
• Stabilize try-fix-capabilities by isolating the try-fix skill, pinning a worktree fixture, restoring the 15-minute timeout, and fixing stale target paths.
• Stabilize pr-review-gh-auth by materializing pr-preflight.md, requiring evidence the guidance was actually read, and adjusting the suite threshold to keep the judge signal load-bearing.

Show a summary per file

File	Description
.github/workflows/skill-validation.yml	Enhances eval summary/table output to distinguish suite pass/fail from execution errors and failed trials.
.github/skills/try-fix/tests/eval.vally.yaml	Pins a MAUI worktree fixture, restores time budget, corrects paths, and adds deterministic tool-call grading for restore behavior.
.github/skills/pr-review/tests/eval.gh-auth.vally.yaml	Materializes pr-preflight.md into the workspace and adds a deterministic “guidance read” grader while preserving judge weight via threshold tuning.

Copilot's findings

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

[Copilot]

Review details

Suppressed comments (1)

.github/workflows/skill-evaluation-soak.yml:26

• Workflow permissions appear broader than necessary for this scheduled/dispatch soak. The job doesn’t call the GitHub API (it runs Vally locally, uploads artifacts, and intentionally avoids passing a GitHub token to the Copilot process), so pull-requests, issues, checks, and statuses permissions can be dropped to reduce blast radius if a step is compromised.

• Files reviewed: 33/34 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (1)

.github/scripts/SelectVallyToken.sh:103

• curl ... || true can yield an empty $status when curl fails (timeout/DNS/etc), producing an unclear warning like HTTP and making it harder to distinguish network failures from non-200 responses. Emit a deterministic sentinel on curl failure so logs clearly show why a PAT slot was skipped.

				https://api.github.com/user || true
	)
	if [ "$status" != "200" ]; then
		echo "::warning::Skipping unavailable Copilot PAT slot $slot (GitHub /user returned HTTP $status)"
		continue

• Files reviewed: 33/34 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 33/34 changed files
• Comments generated: 0 new
• Review effort level: Lite

[kubaflo]

@PureWeen — adversarial re-review of the current head.

Verdict: NEEDS_DISCUSSION (COMMENT review only)
Confidence: low — this changes privileged shared CI infrastructure.

Three independent frontier reviews were cross-pollinated, then reconciled by the MAUI expert reviewer and a final Terra arbitration. One reproducible moderate concern survived; see the inline comment.

• The prior effective-model probe mismatch is fixed: model discovery now matches the pinned Vally execution plan and the executed eval*.vally.yaml set.
• The prior PAT-rotation issue is fixed: selection now combines run, rerun, and matrix entropy. The remaining leg-0/hermeticity overlap was reproduced but is not material enough to retain.
• Current required CI is green, including skill-validation and all reported maui-pr legs. This pull_request_target design intentionally executes trusted helper code from the base github.workflow_sha, so the candidate validator change is not exercised until merge.
• The title and implementation align. The validation paragraph describing the older Windows .buildtasks lock failure is now stale relative to the green current checks.

[Copilot]

Review details

Suppressed comments (1)

.github/scripts/SetupVallyRuntime.sh:22

• This installs from the public npm registry at runtime on every run; transient registry/network issues will directly translate into evaluator failures. Consider adding caching (e.g., actions/setup-node cache + a stable cache key per VALLY_VERSION) or pinning to a pre-built artifact/source mirror used by CI so evaluation reliability isn’t tied to npm availability.

	npm install \
		--no-audit \
		--no-fund \
		--no-save \
		--loglevel=error \
		"@microsoft/vally-cli@${vally_version}"

• Files reviewed: 33/34 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (2)

.github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1:878

• The fallback that re-adds PRNumber when no ChangedFiles were found is currently disabled whenever -BaseBranch is set (via $ExplicitBaseBranch). If -BaseBranch is a branch name (normal PR use), this can prevent the script from using GitHub PR metadata even when git diff can’t determine changed files (e.g., missing refs / shallow state).

Gate the “no PR metadata” behavior on an immutable explicit base SHA (fixture mode) rather than any base-branch value.

    # Fall back to PR number if no changed files from git diff
    if (-not $params.ContainsKey("ChangedFiles") -and $PRNumber -and -not $ExplicitBaseBranch) {
        $params.PRNumber = $PRNumber
    }

.github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1:856

• Get-AutoDetectedTests treats any non-empty -BaseBranch as a signal to ignore PR metadata and instead compute ChangedFiles via git diff. But -BaseBranch is also a normal input for real PR runs (it can be a branch name per the parameter docs), so this changes behavior unexpectedly and makes detection depend on local fetch/merge-base state rather than the GitHub PR file list.

Consider limiting the “local/frozen worktree” path to immutable base commits (the fixture case) instead of any base-branch name.

This issue also appears on line 875 of the same file.

    # An explicit base identifies a local/frozen worktree. Prefer its immutable
    # diff over PR metadata so an unrelated real PR number cannot change the run.
    if ($MergeBase -and $ExplicitBaseBranch) {
        $changedFiles = git diff $MergeBase HEAD --name-only 2>$null
        if (-not $changedFiles -or $changedFiles.Count -eq 0) {

• Files reviewed: 33/34 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 33/34 changed files
• Comments generated: 0 new
• Review effort level: Lite

[kubaflo]

@PureWeen Adversarial re-review: NEEDS_CHANGES at abd7643f61a4c49fe4ca23b9a7a94c0cf12b8a1e.

The direct stimulus-rename bypass from the prior review is fixed, and both maui-pr 1550324 and the exact-head Skill Validation run are green. However, independent reproduction and cross-model arbitration retained one moderate trusted-spec binding gap: a correctly named decoy can satisfy this validation while the fixture-marker-owning stimulus is renamed and loses its mandatory preservation policy.

The PR description also cites prior-head validation runs (1549071 / 31532398686); those should be refreshed to the exact-head runs above.

[Copilot]

Review details

• Files reviewed: 33/34 changed files
• Comments generated: 0 new
• Review effort level: Lite

[kubaflo]

@PureWeen Adversarial re-review: NEEDS_CHANGES at e3958156f9856ee3c3fb53e2594cb9b9ac61d6ed.

The new AST binding fixes the ordinary decoy-plus-renamed-owner payload, but exact-head reproduction, independent Claude/Gemini review, cross-checking, MAUI expert verification, and final GPT-5.6 Terra arbitration retained a distinct first-key/last-key parser differential. A duplicate YAML mapping key lets the trusted AST validation approve one node while YAML.safe_load_file and Vally execute another; the real preparer accepts the reproduced payload with exit 0.

Exact-head Skill Validation is green. Required maui-pr 1550895 is red only on an unmatched MSBuild ProjectRootElement null-reference crash in macOS Debug; all unit, pack, and integration legs passed, so that failure does not explain or refute this finding. The title remains accurate, but the body overstates fixture binding and its CI section is stale.

Verdict: NEEDS_CHANGES — high confidence in one major trusted-boundary finding; low merge-readiness confidence while required CI remains red.

[Copilot]

Review details

Suppressed comments (1)

.github/scripts/SelectVallyToken.sh:153

• SelectVallyToken.sh writes the selected Copilot PAT into $GITHUB_OUTPUT (step output). Even with ::add-mask::, exposing a secret via step outputs increases the chance of accidental disclosure (e.g., future debug echoing of outputs or reuse as job outputs). Prefer keeping the PAT in-process (select + run in the same step), or writing it to a restricted file under $RUNNER_TEMP and only outputting the file path (or another non-secret handle).

if [ -z "$selected_token" ]; then
	echo "::error::Every configured COPILOT_PAT_* secret is unavailable, rate-limited, or missing a required model" >&2
	exit 1
fi

echo "token=$selected_token" >> "$github_output"

• Files reviewed: 33/34 changed files
• Comments generated: 0 new
• Review effort level: Lite

[kubaflo]

@PureWeen — NEEDS_CHANGES at 51bc0282bc6b03fdf68ba209a3c14c2449234386. Independent Opus review, MAUI expert verification, and Terra arbitration converged on one new parser-differential bypass; the prior literal duplicate-key issue is fixed. The exact-head validator suite passed and required maui-pr is green. The PR description’s validation section still names the previous e3958156 head and should be refreshed after the fix.

[Copilot]

Review details

• Files reviewed: 33/34 changed files
• Comments generated: 0 new
• Review effort level: Lite

[kubaflo]

@PureWeen — adversarial re-review of the current 4bce54e3 head.

Verdict: NEEDS_CHANGES. Confidence: low. The finding itself is reproduced with high confidence; merge-safety confidence is capped because this is shared credentialed CI infrastructure.

Claude Opus reproduced the tagged-merge bypass; Gemini initially disagreed based on an incorrect js-yaml premise, then independently confirmed it after cross-pollination. The MAUI expert and GPT-5.6 Terra also reproduced and upheld the P1 finding. GPT-5.6 Sol returned no usable report.

Required maui-pr and CLA checks pass, but the added regression covers only plain <<, not explicit merge tags. The earlier plain-<< parser-differential finding is fixed; this explicit-tag path is new and non-duplicate. The title matches the implementation, but the PR body still cites the older e3958156 head and stale validation counts/links.

[Copilot]

Review details

• Files reviewed: 33/34 changed files
• Comments generated: 0 new
• Review effort level: Lite

[kubaflo]

@PureWeen — adversarial re-review of the current fd8f3d54 head.

Verdict: LGTM. Confidence: low. This is a COMMENT review, not an approval. The code-level conclusion is high-confidence; overall merge-safety confidence is capped because the change is shared credentialed CI infrastructure and pull_request_target intentionally executes trusted helpers from the base revision rather than this PR head.

Claude Opus, Gemini 3.1 Pro, and GPT-5.6 Sol independently found the explicit merge-tag fix complete after cross-pollination. The MAUI expert fuzzed 8,274 tag/parser forms with 892 runtime danger-sink cases and found zero bypasses; GPT-5.6 Terra upheld the clean result. No material current inline findings remain.

The earlier P1 is resolved: the recursive line-214 guard now rejects both textual << and normalized tag:yaml.org,2002:merge keys, while the new regression fails against the prior head and passes here. Required maui-pr and CLA checks are green; targeted exact-head merge tests pass. The title and description now match the implementation, head, validation counts, and CI links.