[CI] Refactor ci-copilot pipeline: scope env vars per task

[T-Gro]

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

Refactors the ci-copilot pipeline (eng/pipelines/ci-copilot.yml) by splitting the monolithic Run PR Reviewer Agent bash task into 4 separate tasks with scoped env: blocks. Each task receives only the environment variables it needs, improving security and clarity.

Pipeline Architecture (Before → After)

Before: Single bash task with all tokens (GH_TOKEN, COPILOT_GITHUB_TOKEN) available to every step.

After: 4 isolated tasks with scoped credentials:

Task	Phase	Tokens	Purpose
1	Setup	GH_TOKEN	Branch checkout, PR merge, copy trusted scripts
2	Gate	GH_TOKEN (read-only)	UI test detection, dotnet build/test, regression analysis
3	CopilotReview	COPILOT_GITHUB_TOKEN	Expert review + try-fix (no GH_TOKEN — agent cannot post)
4	Post	GH_TOKEN	Post comments, labels, AI summary

Key Changes

eng/pipelines/ci-copilot.yml

• Split single bash task into 4 tasks with isolated env: blocks
• PRNumber parameter type changed from string to number
• persistCredentials set to false for security
• Removed gh auth login step (tokens passed via env vars instead)
• Added coalesce() for detectedCategories output variable routing (supports both Gate and AI-refreshed values)

.github/scripts/Review-PR.ps1

• Added -Phase parameter (Setup, Gate, CopilotReview, Post) for per-task invocation
• Added -TrustedScriptsDir parameter for trusted script snapshot support
• Added cross-phase data persistence: Gate writes regression data, categories, and gate result to files; CopilotReview and Post restore from files
• Added setup-complete sentinel: Setup writes marker, other phases verify before proceeding
• Added TrustedScriptsDir null guards with local fallback (prevents Split-Path $null crash)
• Updated phase routing comments to document token scoping

Security Model

The primary security improvement is isolating the Copilot agent (Task 3) from GH_TOKEN. This prevents the AI agent from directly posting comments or pushing code. Only COPILOT_GITHUB_TOKEN is available in Task 3, which is scoped to Copilot operations only.

Task 2 (Gate) has GH_TOKEN for read-only GitHub API access needed by Detect-TestsInDiff.ps1 to fetch PR metadata and labels.

Cross-Phase Data Flow

Since each task runs in a separate process, variables must be persisted to files:

Gate → files:
  gate-result.txt, regression-risks.json, regression-tests.json,
  regression-platform.txt, detect-script-path.txt, uitest-categories.txt

CopilotReview ← files:
  Restores all Gate outputs for expert review context

Post ← files:
  Restores gate-result.txt for comment posting

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

Or

• Run remotely in PowerShell:

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

[kubaflo]

Note: posting as a formal review since the previous two issue comments with the same content were deleted shortly after posting. HEAD is unchanged at 82679867 since 10:09:19Z (no new commits, identical blob SHAs 9f8a1fb… / 698b0d3…), so the findings below are unchanged from my prior comments — included again here for completeness so the PR has a durable record of the analysis.

The token-scoping refactor is the right shape: per-task env: blocks, two distinct trusted phases (Setup/Post), an untrusted Gate with no env, and a CopilotReview phase that gets only COPILOT_GITHUB_TOKEN. persistCredentials: false, removal of the gh auth login step, and --secret-env-vars=GH_TOKEN,COPILOT_GITHUB_TOKEN,GITHUB_TOKEN are all good belt-and-braces moves. The four issues below are the ones I'd want fixed before this lands.

---

🔴 1. trusted-scripts/ is writable by PR code

eng/pipelines/ci-copilot.yml 567-572 (Setup, Task 1):

TRUSTED="$(Agent.TempDirectory)/trusted-scripts"
mkdir -p "$TRUSTED"
cp -r .github/scripts "$TRUSTED/scripts"
cp -r .github/skills  "$TRUSTED/skills"
cp -r eng/scripts     "$TRUSTED/eng-scripts"

Agent.TempDirectory is owned by the agent user, and nothing makes the copies read-only. Task 2 (Gate) runs dotnet build / dotnet test on PR-controlled code and can chmod +w and overwrite anything under $TRUSTED/ — for example scripts/post-ai-summary-comment.ps1 or eng-scripts/detect-ui-test-categories.ps1. Task 4 (Post) then re-invokes Review-PR.ps1 -Phase Post from that directory with GH_TOKEN (line 704) and runs the modified script.

So an attacker PR with a dotnet test payload that rewrites $AGENT_TEMPDIRECTORY/trusted-scripts/... re-acquires GH_COMMENT_TOKEN in Task 4 — defeating the whole partition.

Fix: lock the directory after copy.

cp -r .github/scripts "$TRUSTED/scripts"
cp -r .github/skills  "$TRUSTED/skills"
cp -r eng/scripts     "$TRUSTED/eng-scripts"
chmod -R a-w "$TRUSTED"     # at minimum
# or better: own as a different user (e.g. root via sudo) and chmod 0555

Even better, copy into a path outside the agent user's writable space (e.g. /opt/maui-ci-trusted/$(Build.BuildId), root-owned via the image bootstrap).

---

🔴 2. $gateOutputDir referenced in Setup-only path → Join-Path $null crash

.github/scripts/Review-PR.ps1 306-314:

if ($Phase -eq 'Setup') {
    $sentinelDir = if ($TrustedScriptsDir) { Split-Path $TrustedScriptsDir -Parent } else { $gateOutputDir }
    "OK" | Set-Content (Join-Path $sentinelDir "setup-complete") -Encoding UTF8
    ...
    exit 0
}

$gateOutputDir is only defined at line 575, inside if ($runGate) { ... }. In CI this happens to work because -TrustedScriptsDir is always passed, so the first branch is taken. But the comment block at lines 91-93 explicitly advertises a "no -Phase for backward-compat for local development" mode, and pwsh Review-PR.ps1 -Phase Setup -PRNumber 1 (no trusted dir) hits the else branch and resolves $gateOutputDir to $null, then Join-Path $null "setup-complete" throws.

Fix: make the fallback well-defined, e.g.

$sentinelDir = if ($TrustedScriptsDir) {
    Split-Path $TrustedScriptsDir -Parent
} else {
    Join-Path $RepoRoot "CustomAgentLogsTmp/PRState/$PRNumber/PRAgent"
}
New-Item -ItemType Directory -Force -Path $sentinelDir | Out-Null

Same pattern would help line 666 / 786 where $gateVerdictDir has the same fallback.

---

🟠 3. Tier 3 category refresh deleted, never reinstated

Review-PR.ps1 829-831:

# NOTE: Tier 3 category refresh (re-running detect-ui-test-categories.ps1 with AI
# categories from pre-flight) has been moved to the Post phase so that PR-controlled
# scripts don't run in a task that has COPILOT_GITHUB_TOKEN in scope.

The note is correct about why it was removed from CopilotReview, but the refresh code was never actually added to the if ($runPost) { ... } block (lines 840-897) — only the AI-Summary post and label application live there. Net effect: when the agent writes ai-categories.md, nothing reads it back, and **Detected UI test categories** in the AI Summary comment will reflect only Tier-1/2 (filename heuristics), missing the AI tier on every PR.

Either (a) re-add the refresh block at the top of if ($runPost), reading $aiCategoriesFile and re-running detect-ui-test-categories.ps1 -AiCategories ..., or (b) drop the misleading NOTE and document that Tier 3 is intentionally gone.

---

🟠 4. ##vso[] filter is bypassable

ci-copilot.yml 615 / 645:

... | sed 's/^##vso\[/##_vso[/'

The Azure DevOps agent matches logging commands with surprisingly forgiving rules — leading whitespace, leading \r (common in Windows-encoded output and ANSI-colored streams), and embedded ANSI escapes can all let a ##vso[...] slip past ^##vso\[. Examples that bypass this regex but are still parsed by the agent on at least some configurations:

\r##vso[task.setvariable variable=GH_TOKEN;isSecret=false]leak
   ##vso[task.prependpath]/tmp/evil
\e[0m##vso[task.logissue type=error]anything

Tighter version:

... | tr -d '\r' | sed -E 's/^[[:space:]]*##vso\[/##_vso[/'

(Strip \r first, then anchor after optional whitespace.) Worth adding to Setup and Post too — both echo PR-derived content (Setup runs the merge, Post echoes gate/content.md and friends, both written by Gate/CopilotReview which were untrusted).

---

🟡 Smaller things

• Setup failure → no PR comment, just a red X. If Task 1 throws (merge conflict, gh transient failure), the sentinel isn't written, Tasks 3 and 4 exit 0 silently (lines 628-637, 675-678), and the PR author sees "maui-pr failed" with no AI Summary comment to explain. The job result is at least red because of Task 1, so this is UX, not security — but consider posting a "Setup failed: $reason" comment from a small failure-only step that still has GH_COMMENT_TOKEN.

• Cross-phase state via working tree. gate-result.txt is persisted to Split-Path $TrustedScriptsDir -Parent (good — outside the working tree), but ai-categories.md and gate/content.md live under CustomAgentLogsTmp/PRState/... in the working tree, and Gate runs PR code that can rewrite either. Post should treat anything it reads from there as untrusted (it's posted verbatim into a PR comment, so XSS-in-Markdown / spoofed gate verdict are both reachable). Worth a sanitization pass when reading.

• cp -r CustomAgentLogsTmp $(Build.ArtifactStagingDirectory)/copilot-logs/ in Task 4. The comment "exclude .copilot session state — may contain auth metadata" is spot on for ~/.copilot, but the same caveat applies to CustomAgentLogsTmp/ — Copilot's intermediate output files there can include tool transcripts. If they're meant to be public artifacts, fine; if not, strip them before publish.

• Setup also has GH_TOKEN and runs git merge on PR code. Out of scope for this PR, but worth noting: a malicious .gitattributes with a merge= driver pointing at a script in the PR can run that script during merge with GH_TOKEN in env. Mitigations: git -c core.attributesFile=/dev/null merge ... or git config merge.<driver>.driver false before the merge.

---

🟢 What I really like

• Per-task env: partition with # env: deliberately empty on Gate. Crystal clear, easy to audit, easy to break the wrong way and notice in review.
• persistCredentials: false + removal of gh auth login (no ~/.config/gh/hosts.yml left on disk for dotnet subprocesses to read).
• --secret-env-vars=... on the copilot invocation as defense-in-depth even after the env: scoping.
• Sentinel-based gating between tasks is the right pattern for "did Setup succeed?" (just needs the not-silent-on-failure tweak above).
• Phase routing in PowerShell with backward-compat for local runs — the $run<Phase> flags read very nicely.

---

Summary

#	Severity	Where	Issue
1	🔴 High	ci-copilot.yml 567-572	Trusted scripts dir writable by Gate (PR code) → Post task re-acquires GH_TOKEN
2	🔴 High	Review-PR.ps1 309	$gateOutputDir undefined in -Phase Setup local mode → Join-Path $null
3	🟠 Med	Review-PR.ps1 829 / runPost block	Tier 3 AI category refresh deleted with "moved to Post" comment, never re-added
4	🟠 Med	ci-copilot.yml 615/645	##vso[] filter regex bypassable by \r, leading whitespace, ANSI escapes
5	🟡 Low	ci-copilot.yml 628-637, 675-678	Setup failure → silent skip of Post → no PR comment explaining
6	🟡 Low	Post phase	Treat working-tree content written by Gate/CopilotReview as untrusted
7	🟡 Low	Setup phase	git merge of PR code with GH_TOKEN in env — mitigate .gitattributes merge drivers

Happy to elaborate on any of these. The overall direction is great — the env-scoping refactor was overdue.

[PureWeen]

🔍 Adversarial PR Review

Methodology: 3 independent reviewers with adversarial consensus
CI: ✅ 30/30 checks passing

Summary

#	Sev	Consensus	Category	Finding
1	❌	2/3	Security	trusted-scripts/ writable → Post runs tampered scripts with GH_TOKEN
2	❌	3/3	Security	gate-result.txt spoofable by PR test code
3	❌	3/3	Logic	$gateOutputDir undefined in Setup without -TrustedScriptsDir
4	❌	3/3	Regression	Tier 3 category refresh deleted, never reinstated
5	❌	Verified	Logic	eng-scripts path off by one level → category detection silently skipped
6	⚠️	3/3	Security	##vso[] filter bypassable
7	⚠️	2/3	Config Impact	PRNumber type change is breaking

Verdict: 5 blocking issues, 2 warnings. The security architecture (per-task env scoping) is the right direction, but the implementation has gaps that undermine the security boundary — particularly #1 and #2 which allow Gate-phase PR code to tamper with files that Post executes with GH_TOKEN.

⚠️ Author's Claimed Fixes Not Present

The author comment states all 4 issues from kubaflo's review are "addressed in current HEAD (8267986)." All 3 reviewers independently confirmed none of the 4 fixes are in the code on disk. The commit hash is unchanged.

Discarded Findings (1/3, no consensus)

• setup-complete sentinel writable by Gate (single reviewer)
• $testDetectScript uses $PSScriptRoot (single reviewer)
• Post applies gate-failed label when gate never ran (single reviewer)
• $reviewBranch checkout warns but continues (single reviewer)
• post-ai-summary-comment.ps1 called twice (single reviewer — intentional idempotent design)

[T-Gro]

Addressing all findings from both reviews. Current HEAD: d4d7bdb.

#	Finding	Status	Where
1	trusted-scripts/ writable by PR code	Fixed: chmod -R a-w after copy	ci-copilot.yml L572
2	gate-result.txt spoofable by test code	Fixed: writes to Agent.TempDirectory, not working tree	Review-PR.ps1 L663-667
3	sentinel dir null in local-dev mode	Fixed: fallback to CustomAgentLogsTmp path	Review-PR.ps1 L309-315
4	Tier 3 category refresh deleted not moved	Fixed: comment now says "removed in phased design"	Review-PR.ps1 L836
5	eng-scripts path off by one level	Fixed in d4d7bdb: uses TrustedScriptsDir directly	Review-PR.ps1 L511
6	vso filter bypassable via CR/whitespace	Fixed: tr -d '\r' pipe sed -E with space class	ci-copilot.yml L608, L647
7	PRNumber type:number may break callers	Intentional. AzDO coerces string '33687' to number at queue time. Existing callers passing numeric strings will work.

Waived items (not in scope for this PR):

• Setup failure silent skip: UX concern, not a token leak. Build fails red from Task 1.
• content.md writable by PR code: posted to attacker's own PR as display text. No label or token impact (labels from gate-result.txt which is now in Agent.TempDirectory).
• .gitattributes merge driver: git merge --squash does not invoke custom merge drivers.
• CustomAgentLogsTmp in artifacts: intentional diagnostic output, no auth metadata.

[Copilot]

Pull request overview

This PR refactors the ci-copilot Azure DevOps pipeline to reduce secret exposure by splitting the previous single “Run PR Reviewer Agent” step into distinct phases with tightly scoped env: blocks, and updates Review-PR.ps1 to support per-phase invocation via a new -Phase parameter (plus -TrustedScriptsDir) so trusted scripts can be copied before merging PR code.

Changes:

• Split the pipeline into 4 tasks (Setup / Gate / CopilotReview / Post) with scoped environment variables and a “trusted-scripts” copy staged in $(Agent.TempDirectory).
• Add phase routing to .github/scripts/Review-PR.ps1, including log file suffixing per phase and cross-task state handoff via sentinel/result files.
• Harden Copilot invocation by passing --secret-env-vars and move gate verdict persistence to $(Agent.TempDirectory) to avoid PR-writeable state.

Reviewed changes

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

File	Description
eng/pipelines/ci-copilot.yml	Splits the monolithic reviewer step into 4 tasks with scoped secrets and trusted-script staging.
.github/scripts/Review-PR.ps1	Adds -Phase/-TrustedScriptsDir to enable per-task execution and cross-task state sharing.

Comments suppressed due to low confidence (1)

eng/pipelines/ci-copilot.yml:88

• Since PRNumber is now a numeric parameter, the current validation only checks for an empty string. It will accept 0 (or other invalid values) and proceed to run against a non-existent PR. Consider validating PRNumber is > 0 (and possibly an integer) before continuing.

          - bash: |
              echo "Validating PR Number parameter..."
              if [ -z "${{ parameters.PRNumber }}" ]; then
                echo "##vso[task.logissue type=error]PRNumber parameter is required"
                exit 1
              fi

[JanKrivanek]

LGTM!

[kubaflo]

Code Review — PR #35324

Independent Assessment

What this changes: Splits the monolithic CI pipeline task into 4 sequential tasks with scoped environment variables. Each task calls Review-PR.ps1 -Phase <phase> with only the secrets it needs. Review-PR.ps1 gains -Phase and -TrustedScriptsDir parameters with phase routing guards.

Inferred motivation: Security hardening — the Gate phase (dotnet build/test) and CopilotReview phase previously had access to GH_TOKEN which could push to the repo. Now only Setup and Post phases get GH_TOKEN, and only CopilotReview gets COPILOT_GITHUB_TOKEN.

Findings

⚠️ Warning — Cross-phase variable loss: regression test instructions

Severity: Medium
Files: .github/scripts/Review-PR.ps1:1676-1700

Variables $risksData, $regressionTests, and $regrPlatform are defined in the Gate phase (Steps 4) but consumed in CopilotReview phase (Step 6) for building regression test instructions in try-fix prompts. In phased mode (separate pwsh processes), these variables are undefined in CopilotReview, so $regressionTestInstruction will always be empty.

Impact: Try-fix candidates won't receive mandatory regression test instructions and could produce fixes that re-introduce previously-fixed bugs.

Fix: Persist regression data to gate-result-dir/regression-risks.json in the Gate phase, restore it in CopilotReview (similar to how $gateResult is already persisted).

⚠️ Warning — Tier 3 AI category refresh silently skipped in phased mode

Severity: Low
Files: .github/scripts/Review-PR.ps1:1853

$detectScript is defined in Gate (line 682) but referenced in CopilotReview (line 1853). The null guard if ($detectScript -and ...) prevents errors but silently skips the AI-tier category refresh. The $uitestCategories comparison at line 1872 would also be against an undefined value.

Impact: Low — detectedCategories output variable is already emitted during Gate, and the deep UI test stage reads it from there. The refresh would only change the content.md file for the AI summary comment.

⚠️ Warning — RunPost step name comment is misleading

Severity: Low
Files: eng/pipelines/ci-copilot.yml:733

The comment on name: RunPost says "RunDeepUITests / UpdateAISummaryComment stages can read this step's output variables (detectedCategories, detectedPlatform)" — but detectedCategories actually comes from RunGate, not RunPost. The downstream stages were correctly updated to reference RunGate.detectedCategories and RunPost.aiSummaryCommentId.

Fix: Update the comment to say RunPost.<var> is only for aiSummaryCommentId.

💡 Suggestion — Persist regression risks to file for CopilotReview phase

The gate result persistence pattern (write to gate-result.txt, restore in later phases) works well. Apply the same pattern to regression risks:

# End of Gate: persist
if ($risksData) { $risksData | ConvertTo-Json -Depth 10 | Set-Content (Join-Path $gateVerdictDir "regression-risks.json") }

# Start of CopilotReview: restore
$risksFile = Join-Path (Split-Path $TrustedScriptsDir -Parent) "regression-risks.json"
if (Test-Path $risksFile) { $risksData = Get-Content $risksFile -Raw | ConvertFrom-Json }

Devil's Advocate

1. Are the cross-phase variable losses actually a problem? In practice, the Gate phase already runs regression tests. The CopilotReview phase using regression instructions only helps try-fix candidates avoid regressions — so it's a quality degradation, not a correctness bug. The phase design still works end-to-end.
2. Could persistCredentials:false break anything? dotnet/maui is public, so unauthenticated git fetch works. The Setup phase uses GH_TOKEN via env var for gh commands, not git credentials. Safe.
3. Does PRNumber type:number break anything? AzDO converts to string in env var contexts. The existing validation step still works. The removal of default: '' makes it required, which is correct behavior.

Verdict: NEEDS_CHANGES

Confidence: high
Summary: The architecture is sound and the security improvements are valuable. The cross-phase regression data loss (⚠️ Warning #1) is the main concern — try-fix candidates in phased mode lose mandatory regression test instructions, which could lead to fixes that re-introduce previously-fixed bugs. The misleading comment and Tier 3 refresh skip are minor. Recommend persisting regression risks to file before merging.

[kubaflo]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

Adversarial multi-model review — Round 2

Methodology: 3 independent reviewers with adversarial consensus. Every finding below was verified against the source at head bbd60558. Consensus counts shown per finding. This is a COMMENT review, not an approval or change-request.

The author addressed all findings from the prior round — these are new findings against the per-task refactor and the new Remove-StaleMauiBotComments.ps1 cleanup script.

❌/⚠️ Findings

1. ⚠️ Security — Cross-phase state files live in a writable directory & fail open (3/3)
gate-result.txt, detect-script-path.txt, regression-*.json, and uitest-categories.txt are written to Split-Path $TrustedScriptsDir -Parent, i.e. $(Build.ArtifactStagingDirectory) — the writable parent of the chmod -R a-w trusted-github/ directory (Review-PR.ps1:1258-1288). Three consequences:

• detect-script-path.txt is read back and executed in CopilotReview (& pwsh -NoProfile -File $detectScript ..., line 1542). A tampered path runs with COPILOT_GITHUB_TOKEN. The Test-Path $detectScript guard (line 1535) checks existence, not trust.
• gate-result.txt could be forged to PASSED, yielding the s/agent-gate-passed label.
• A missing gate-result.txt fails open to SKIPPED (lines 1309, 1594) rather than failing closed.

Fix: persist cross-phase state into a dedicated subdirectory that is also chmod a-w after writing; re-derive the detect script from $TrustedScriptsDir/eng-scripts/... instead of persisting and executing a stored path (or validate the restored path starts with $TrustedScriptsDir); treat a missing gate result as a failure, not SKIPPED.

2. ⚠️ Security — ##vso[...] is not stripped from PR-controlled stdout (2/3)
The new security doc (rule 6, line 28) and its checklist (line 41) require piping PR-controlled output through tr -d '\r' | sed -E 's/##vso\[[^]]*\]//g', but no such stripping exists anywhere in the pipeline. Gate and CopilotReview run dotnet build/dotnet test, whose stdout flows unfiltered to Azure Pipelines, which will honor an injected ##vso[task.setvariable variable=GateFailed]false (or similar) from PR test/build output.

Fix: pipe the inner PR-controlled subprocess output through the documented stripper, keeping the script's own ##vso emissions outside the pipe.

3. ⚠️ Data Loss — Marker-based comment deletion has no author check (2/3)
Remove-StaleMauiBotComments.ps1:92-94: $matchesGeneratedMarker deletes any comment whose body contains <!-- AI Summary --> or <!-- AI Gate -->, regardless of author — unlike $matchesBotOnlyContent (96-100), which is gated by Test-IsMauiBotCommentAuthor. A maintainer who quotes the marker in their own comment would have it deleted on the next run.

Fix: AND the marker check with Test-IsMauiBotCommentAuthor, mirroring the bot-only-content path.

4. ⚠️ Logic — gh api --paginate | ConvertFrom-Json breaks past 100 items (1/3, reproduced)
Remove-StaleMauiBotComments.ps1:52,58,128,134 and post-ai-summary-comment.ps1. gh api --paginate concatenates one JSON array per page ([...][...]), which ConvertFrom-Json rejects (Additional text encountered after finished reading JSON content). The catch returns @(), so stale-comment/review cleanup silently no-ops on PRs with >100 comments or reviews. It fails safe (nothing wrongly deleted) but does not perform its function at scale.

Fix: use gh api --paginate --slurp (merges pages into one array) or --jq '.[]' and re-wrap.

5. 💡 Testing — Safety-critical new scripts are under-tested (2/3)
Remove-StaleMauiBotComments.ps1 deletes comments and dismisses reviews but has no Pester coverage; the new test files don't cover paginated parsing or missing/corrupt cross-phase state files. Suggest tests for: non-bot comments with the marker are NOT deleted (after fix #3), missing gate-result.txt behavior, and multi-page API responses.

Minor (single-reviewer, verified, non-blocking): Setup invokes Review-PR.ps1 from the worktree path rather than $TRUSTED (ci-copilot.yml:628; pre-merge, so low risk but inconsistent with the trusted-copy model); Setup uses set +e with no exit, so a Setup failure doesn't short-circuit later tasks (it surfaces later via CopilotFailed, but downstream phases still run on a possibly-bad state); Detect-TestsInDiff.ps1 declares [string]$PRNumber while callers now pass [int] (auto-converts; cosmetic).

CI status

maui-pr (build / pack / integration tests) is green. However maui-pr-uitests (Android Controls API 30 CollectionView) is failing — this looks like an unrelated flake, as the PR does not touch CollectionView — and Skill Validation is failing (LLM evaluation failed), which may be related since this PR adds a new instructions doc. Not a clean/mergeable verdict until those are understood and green.

[kubaflo]

Test Failure Review

@T-Gro - new test-failure review results are available based on this last commit: bbd6055.
To request a fresh review after new comments, commits, or CI runs, comment /review tests.

⚠️ Test Failure Review - bbd6055 - [CI] Refactor ci-copilot pipeline: scope env vars per task - 2026-06-02T18:18:45Z

Overall verdict: Needs human investigation

The Android CollectionView UI-test failure looks unrelated to this PR's .github/CI automation scope, but the Skill Validation check overlaps files changed by the PR and only reports LLM evaluation failed without detailed evaluation output in the gathered context. The available data is partial because Build Analysis has no build-specific details and optional authenticated AzDO test-result records include unrelated Roslyn/CoreCLR failures.

Failure	Verdict	Evidence
Skill Validation check	Needs human investigation	The Check Run at 78328354172 failed with output title LLM evaluation failed and links to Actions run 26584935471 for head bbd6055. That run evaluated verify-tests-fail-without-fix, and this PR changes .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1; however the gathered context does not include the detailed LLM evaluation text needed to identify the exact assertion.
maui-pr-uitests / Android Controls (API 30) CollectionView	Likely unrelated	AzDO build 1442480 failed on refs/pull/35324/merge in maui-pr-uitests, with failed job/logs for Controls (API 30) CollectionView (logId 4315/4325). The logs contain 23 deduplicated CollectionView UI failures, mostly System.TimeoutException : Timed out waiting for element..., plus one VerifyGroupItemScrollToByIndexWithEndPositionAndHorizontalGrid_Potato expected-value failure. The PR scope is 10 .github/eng/pipelines/ci-copilot.yml automation files, with no inferred platform, Android, Controls, or CollectionView changes; recent base build 1444191 for the same definition also failed on main.
Build Analysis check	Insufficient data	The check failed, but the gathered context only has the generic Build Analysis documentation URL and no build-specific known-issue match, failure title, or log excerpt. It cannot be responsibly classified beyond noting that the PR also has a failed maui-pr-uitests build.
Optional authenticated AzDO _apis/test records	Likely unrelated	Authenticated test-result enrichment attached 23 deduplicated non-MAUI failures from run names such as coreclr Linux x64 Checked jitstress1, coreclr windows ..., and Microsoft.CodeAnalysis.... These do not match the maui-pr-uitests Android CollectionView job or the PR's changed .github/CI files, so they should not be counted as PR-caused MAUI failures.

Recommended action

Inspect the Skill Validation LLM-evaluation result for the changed verify-tests-fail-without-fix skill/script; treat the Android CollectionView UI-test failures and unrelated Roslyn/CoreCLR enrichment as non-blocking for this PR unless a rerun reproduces a failure tied to the PR scope.

Evidence details

PR scope from the gathered context:

• PR [CI] Refactor ci-copilot pipeline: scope env vars per task #35324: [CI] Refactor ci-copilot pipeline: scope env vars per task
• Head: feature/refactor-copilot-yml at bbd6055
• Labels: s/agent-reviewed, area-ai-agents
• Changed files: .github/instructions/ci-copilot-pipeline-security.instructions.md, .github/scripts/Post-AISummaryComment.Tests.ps1, .github/scripts/Review-PR.ps1, .github/scripts/post-ai-summary-comment.ps1, .github/scripts/shared/Aggregate-UITestArtifacts.Tests.ps1, .github/scripts/shared/Aggregate-UITestArtifacts.ps1, .github/scripts/shared/Get-AggregatedTrxFromDirectory.ps1, .github/scripts/shared/Remove-StaleMauiBotComments.ps1, .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1, eng/pipelines/ci-copilot.yml
• Inferred platforms from files: none

Failing checks in context:

• Skill Validation: completed/failure, details https://github.com/dotnet/maui/runs/78328354172
• maui-pr-uitests: completed/failure, build 1442480
• maui-pr-uitests (Android UITests Controls (API 30) CollectionView): completed/failure, job URL Android CollectionView logs
• Build Analysis: completed/failure, details URL points to generic documentation

AzDO build evidence:

• Build 1442480: maui-pr-uitests (definition 313), failed / completed, branch refs/pull/35324/merge, source version 6f90925459a6651dca5258480cbae7ebb499b6b6
• Failed timeline records include Android UITests, phase Controls (API 30), job Controls (API 30) CollectionView, task Controls (API 30) CollectionView, and Publish the android_ui_tests_controls_30 test results
• The CollectionView log failures reference Microsoft.Maui.TestCases.Tests.CollectionView_GroupingFeatureTests and UITest.Appium waits/teardown, not changed PR files
• Recent base-branch builds for the same definition include 1444191 failed on refs/heads/main at 2026-06-02T02:29:23Z, plus older failed main builds 1431731, 1419912, and 1419503

Limitations:

• The local gatherer used authenticated Azure CLI AzDO access. The workflow normally relies on public build/timeline/log APIs unless AZDO_TOKEN is present.
• The gathered context has no detailed Build Analysis result payload and no detailed Skill Validation LLM-evaluation assertion text.
• Helix aggregate data was not checked (helix.checked=false), but the observed actionable failure is from maui-pr-uitests, not maui-pr-devicetests.
• Authenticated _apis/test results are optional enrichment; in this context they include unrelated Roslyn/CoreCLR test runs and were not used to infer MAUI PR causality.