ci-copilot: support net11-targeting PRs (auto-detect base branch + #35677 on net11.0) - #35994
ci-copilot: support net11-targeting PRs (auto-detect base branch + #35677 on net11.0)#35994kubaflo wants to merge 2 commits into
Conversation
Applies the 12-file content of #35677 (origin/main...pr-35677) on top of net11.0 so the Copilot review pipeline carries the enhanced AI Summary review output, token-usage aggregation, and deep-UI-test review-id patching on the net11 line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The copilot review pipeline previously installed workloads and squash-merged the PR using whatever branch the pipeline was launched from (self). A PR targeting net11.0 reviewed from a main-based pipeline branch (or vice-versa) therefore got the wrong .NET SDK band and hit cross-branch squash-merge conflicts. Add a 'Resolve PR base branch' step (CopilotReview + DeepUITests stages) that reads the PR's baseRefName via gh, allowlist-validates it (^(main|net[0-9]+\.0)$), and switches the worktree (detached) to origin/<baseRef> before workload install and the squash-merge. build.ps1 --target=dotnet then provisions the correct band (net11 11.0.100 / main 10.0.100) and Review-PR.ps1 -Phase Setup squash-merges onto the PR's real base — so one pipeline branch handles both main- and net11-targeting PRs. Trusted scripts are captured from the pipeline ref before the base checkout so later tasks keep running the reviewed pipeline-branch scripts (security rule 3); Task 1 Setup now sources Review-PR.ps1 from $TRUSTED like the later tasks. review-trigger.yml and Review-PR.ps1 are intentionally unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35994Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35994" |
kubaflo
left a comment
There was a problem hiding this comment.
Synthesized Review — PR #35994
Title: ci-copilot: support net11-targeting PRs (auto-detect base branch + #35677 on net11.0)
Author: kubaflo (Jakub Florkowski) · Base: net11.0 · Head: feature/net-11-ai-review
HEAD SHA: 9ae99c1e3fa875feae81dc2813f6acc9e2b169c3
Labels: area-infrastructure · Changed files: 12 (+1800 / −161)
Verdict: NEEDS_DISCUSSION (confidence: medium)
Verdicts disagreed across the four model reviews, so per the conservative-resolution rule
(NEEDS_CHANGES > NEEDS_DISCUSSION > LGTM) the single NEEDS_DISCUSSION governs. The
discussion point is not a code defect — it is solely about CI status at review time (see
"CI status" below). Both inline findings are non-blocking suggestions.
Per-model verdicts
| Model | Verdict | Confidence | Findings |
|---|---|---|---|
| gemini-3.1-pro-preview | LGTM | high | 0 |
| gpt-5.5 | NEEDS_DISCUSSION | medium | 0 (CI-not-green concern) |
| claude-opus-4.6 | LGTM | high | 1 (Review-PR.ps1:748) |
| claude-opus-4.8 | LGTM | high | 1 (ci-copilot.yml:949) |
What the PR does
Infrastructure-only change to the Copilot PR-review pipeline so it can review
net11.0-targeting PRs (not just main/net10). Two pieces:
- Ports #35677 (enhanced MauiBot AI Summary review, token-usage aggregation, deep-UI-test
review-id patching) ontonet11.0. ci-copilot.ymlauto-detects the PR base branch — a new "Resolve PR base branch" step
(in bothCopilotReviewandDeepUITestsstages) readsbaseRefNameviagh,
allowlist-validates it (^(main|net[0-9]+\.0)$, fail-closed), and switches the detached
worktree toorigin/<baseRef>before workload install and the squash-merge. Trusted
scripts are captured from the pipeline ref before the base checkout (security rule 3).
Reviewers concurred the security model is sound: trusted-script snapshot ordering, anchored
allowlist regex, AzDO log-command-injection defense (ConvertTo-AzdoSafeConsole), and
token-usage paths scoped to exclude PR-controllable directories.
Synthesized findings (2 — both non-blocking suggestions)
Both were validated against HEAD code (git show 9ae99c1e…:<path>); both anchor on added (+)
lines that are inside the PR diff, so they are valid for inline posting. They are distinct
file/concern pairs — no clustering required.
1. .github/scripts/Review-PR.ps1:748 — token telemetry (from claude-opus-4.6)
The $outputFields filter (HEAD lines 745–749) keeps any path containing output/completion
(minus cache/total). Since Select-CanonicalTokenFields keeps all shallowest matches,
a future Copilot payload exposing both outputTokens and reasoningOutputTokens at the same
depth would sum both and double-count reasoning tokens. The inputFields filter already
excludes cache variants; adding -and $_.Path -notmatch '(?i)reasoning' restores symmetry.
Forward-looking only — current payloads are unaffected.
2. eng/pipelines/ci-copilot.yml:949 — CI security control duplication (from claude-opus-4.8)
The PR-base allowlist ^(main|net[0-9]+\.0)$ + PRNumber re-guard is duplicated verbatim
across the two "Resolve PR base branch" steps (CopilotReview line 161, DeepUITests line 949
— both confirmed present at HEAD). This allowlist is the control deciding which branch the
worktree switches to before workload install + squash-merge, so silent divergence is the risk.
Extracting a shared steps template (the CopilotReview copy additionally captures $TRUSTED)
prevents drift.
Dropped / invalid findings
None. Every finding raised by any model was verified to reference code present at HEAD. gemini
and gpt-5.5 raised no inline findings.
CI status (basis for the NEEDS_DISCUSSION)
| Check | Result |
|---|---|
| Post results comment (GitHub Actions) | fail (5s) |
| maui-pr · Run Helix Unit Tests Windows Helix Unit Tests (Release) | fail (57m) |
| maui-pr · Windows Helix Unit Tests (Debug) | pass |
| maui-pr · Build macOS/Windows (Debug & Release), Pack macOS | pass |
| maui-pr (overall), Build Windows (Release), Pack Windows | pending |
| Copilot review agent / detection / gate / static validation | pass |
gpt-5.5 declined LGTM until those failures are confirmed unrelated. opus-4.6 and opus-4.8
assessed them as unrelated: no product or test code changed; only the Release Helix leg
failed while Debug passed; maui-pr is a different pipeline from the edited ci-copilot.yml.
To resolve to LGTM: confirm/re-run the Windows Helix (Release) leg and the "Post results
comment" job clean, and let the pending maui-pr legs finish.
| $outputFields = @($tokenFields | Where-Object { | ||
| $_.Path -match '(?i)(output|completion)' -and | ||
| $_.Path -notmatch '(?i)(cache|cached)' -and | ||
| $_.Path -notmatch '(?i)total' |
There was a problem hiding this comment.
🤖 AI-Generated Review (multi-model: GPT-5.5, Claude Opus 4.8, Claude Opus 4.6, Gemini 3.1 Pro)
Non-blocking suggestion (token telemetry): This $outputFields filter keeps any field whose path contains output or completion (excluding cache/cached and total). Because Select-CanonicalTokenFields retains all shallowest matches at the same depth, a future Copilot CLI usage payload that exposes both outputTokens and reasoningOutputTokens side-by-side would sum both — double-counting reasoning tokens inside outputTokens. The inputFields filter already guards its cache variants; consider excluding reasoning here for symmetry:
$outputFields = @($tokenFields | Where-Object {
$_.Path -match '(?i)(output|completion)' -and
$_.Path -notmatch '(?i)(cache|cached)' -and
$_.Path -notmatch '(?i)total' -and
$_.Path -notmatch '(?i)reasoning'
})Forward-looking only — current payloads are unaffected. (Flagged by claude-opus-4.6.)
| exit 1 | ||
| fi | ||
| BASE_REF=$(gh pr view "${PARAM_PR_NUMBER}" --json baseRefName -q .baseRefName) | ||
| if ! [[ "${BASE_REF}" =~ ^(main|net[0-9]+\.0)$ ]]; then |
There was a problem hiding this comment.
🤖 AI-Generated Review (multi-model: GPT-5.5, Claude Opus 4.8, Claude Opus 4.6, Gemini 3.1 Pro)
Non-blocking suggestion (CI security control): The PR-base allowlist ^(main|net[0-9]+\.0)$ plus the PRNumber re-guard is duplicated verbatim between the two "Resolve PR base branch" steps — the CopilotReview stage (line 161) and this DeepUITests copy (line 949). This allowlist is the control that decides which branch the worktree is switched to before workload install and the squash-merge, so silent divergence is the real risk: fixing or tightening one copy could leave the other permissive. Consider extracting a shared steps template (the only real difference is that the CopilotReview copy also captures $TRUSTED) so the validation cannot drift. (Flagged by claude-opus-4.8.)
|
Consolidated into #35677. The net11 enablement (runtime PR-base auto-detection for workloads + merge base, and branch-aware deep-UI/device-test TFM) was cherry-picked onto `feature/enhanced-reviewer` (#35677), which now handles both `main`/net10 and `net11.0` PRs from a single pipeline branch — so this separate net11 branch is no longer needed. Closing in favor of #35677. |
#35677) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Summary - Posts MauiBot AI Summary output as a pull request review with parsed `APPROVE`, `REQUEST_CHANGES`, or safe `COMMENT` fallback. - Uses the new AI Review Summary layout with segmented status chips, collapsed review sessions, and merged Future Action content. - Keeps PR finalization out of the automated review process; AI Summary updates no longer preserve or merge `SECTION:PR-FINALIZE` blocks. - Adds visible AI Summary guidance telling users to comment `/review rerun` after new comments or commits when they want a fresh review. The command implementation is intentionally split into a follow-up PR. - Hides stale MauiBot AI Summary / try-fix artifacts with GitHub minimization instead of deleting them, while preserving same-run try-fix and AI Summary reviews. - Updates the Copilot pipeline to pass review IDs and patch review bodies after deep UI tests. - Hardens gate setup/retry handling by committing squashed PR changes before verification, resetting the review branch before gate retries, and detecting BlazorWebView unit-test project paths. ## Validation - Parsed changed PowerShell scripts with `System.Management.Automation.Language.Parser`. - Parsed `.github/workflows/review-trigger.yml` as YAML. - `Invoke-Pester .github/scripts/Post-AISummaryComment.Tests.ps1,.github/scripts/Remove-StaleMauiBotComments.Tests.ps1 -CI` - Dry-run AI Summary generation verified the rerun note, segmented chips, and collapsed review session layout. - Verified `Detect-TestsInDiff.ps1` maps `src/BlazorWebView/tests/MauiBlazorWebView.UnitTests/UriExtensions_Tests.cs` to `src/BlazorWebView/tests/MauiBlazorWebView.UnitTests/MauiBlazorWebView.UnitTests.csproj`. ## net11.0-targeting PR support This branch also makes the Copilot review pipeline handle **`net11.0`-targeting PRs** as well as `main`/net10, from a single pipeline branch (consolidates and supersedes #35994): - **Runtime base-branch auto-detection** (`eng/pipelines/ci-copilot.yml`): the `CopilotReview` and `DeepUITests` stages read the PR's `baseRefName` via `gh`, allowlist-validate it (`^(main|net[0-9]+\.0)$`), and check out that base **before** workload install and the squash-merge — so workloads (net11 `11.0.100` vs net10 `10.0.100`) and the merge base follow the PR. Trusted scripts are captured from the pipeline ref first (security rule 3); Task 1 Setup runs from `$TRUSTED`. - **Branch-aware test TargetFramework**: `BuildAndRunHostApp.ps1` and `run-device-tests/Run-DeviceTests.ps1` now derive the TFM from `Directory.Build.props` (`Get-MauiTfmVersion` in `shared-utils.ps1`) instead of hardcoding `net10.0`, so deep-UI/device tests build `net11.0-android` on net11. The `DeepUITests` stage restores the reviewed pipeline-branch scripts over the worktree before the per-category loop. `main`/net10 behavior is unchanged by design (base-detection checks out `main` = the original flow; `global.json` stays `10.0.108`). `review-trigger.yml` and `Review-PR.ps1` are intentionally untouched. **Validation:** 8 live net11 runs with 100% correct base detection (e.g. #35891 gate ran net11 Core tests 44/45 → 45/45, conflict-free squash-merge onto net11.0); reproduced and root-caused a separate net11.0-baseline `MAUIX2017` Xaml.UnitTests build break (unrelated to this change). Confirmation runs from this branch cover one net11 PR and one main PR. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com> Co-authored-by: Copilot <Copilot@users.noreply.github.com> Co-authored-by: Copilot CI <copilot-ci@microsoft.com> Co-authored-by: kubaflo <kubaflo@users.noreply.github.com>
dotnet#35677) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Summary - Posts MauiBot AI Summary output as a pull request review with parsed `APPROVE`, `REQUEST_CHANGES`, or safe `COMMENT` fallback. - Uses the new AI Review Summary layout with segmented status chips, collapsed review sessions, and merged Future Action content. - Keeps PR finalization out of the automated review process; AI Summary updates no longer preserve or merge `SECTION:PR-FINALIZE` blocks. - Adds visible AI Summary guidance telling users to comment `/review rerun` after new comments or commits when they want a fresh review. The command implementation is intentionally split into a follow-up PR. - Hides stale MauiBot AI Summary / try-fix artifacts with GitHub minimization instead of deleting them, while preserving same-run try-fix and AI Summary reviews. - Updates the Copilot pipeline to pass review IDs and patch review bodies after deep UI tests. - Hardens gate setup/retry handling by committing squashed PR changes before verification, resetting the review branch before gate retries, and detecting BlazorWebView unit-test project paths. ## Validation - Parsed changed PowerShell scripts with `System.Management.Automation.Language.Parser`. - Parsed `.github/workflows/review-trigger.yml` as YAML. - `Invoke-Pester .github/scripts/Post-AISummaryComment.Tests.ps1,.github/scripts/Remove-StaleMauiBotComments.Tests.ps1 -CI` - Dry-run AI Summary generation verified the rerun note, segmented chips, and collapsed review session layout. - Verified `Detect-TestsInDiff.ps1` maps `src/BlazorWebView/tests/MauiBlazorWebView.UnitTests/UriExtensions_Tests.cs` to `src/BlazorWebView/tests/MauiBlazorWebView.UnitTests/MauiBlazorWebView.UnitTests.csproj`. ## net11.0-targeting PR support This branch also makes the Copilot review pipeline handle **`net11.0`-targeting PRs** as well as `main`/net10, from a single pipeline branch (consolidates and supersedes dotnet#35994): - **Runtime base-branch auto-detection** (`eng/pipelines/ci-copilot.yml`): the `CopilotReview` and `DeepUITests` stages read the PR's `baseRefName` via `gh`, allowlist-validate it (`^(main|net[0-9]+\.0)$`), and check out that base **before** workload install and the squash-merge — so workloads (net11 `11.0.100` vs net10 `10.0.100`) and the merge base follow the PR. Trusted scripts are captured from the pipeline ref first (security rule 3); Task 1 Setup runs from `$TRUSTED`. - **Branch-aware test TargetFramework**: `BuildAndRunHostApp.ps1` and `run-device-tests/Run-DeviceTests.ps1` now derive the TFM from `Directory.Build.props` (`Get-MauiTfmVersion` in `shared-utils.ps1`) instead of hardcoding `net10.0`, so deep-UI/device tests build `net11.0-android` on net11. The `DeepUITests` stage restores the reviewed pipeline-branch scripts over the worktree before the per-category loop. `main`/net10 behavior is unchanged by design (base-detection checks out `main` = the original flow; `global.json` stays `10.0.108`). `review-trigger.yml` and `Review-PR.ps1` are intentionally untouched. **Validation:** 8 live net11 runs with 100% correct base detection (e.g. dotnet#35891 gate ran net11 Core tests 44/45 → 45/45, conflict-free squash-merge onto net11.0); reproduced and root-caused a separate net11.0-baseline `MAUIX2017` Xaml.UnitTests build break (unrelated to this change). Confirmation runs from this branch cover one net11 PR and one main PR. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com> Co-authored-by: Copilot <Copilot@users.noreply.github.com> Co-authored-by: Copilot CI <copilot-ci@microsoft.com> Co-authored-by: kubaflo <kubaflo@users.noreply.github.com>
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Summary
Enables the Copilot PR-review pipeline (
maui-copilot) to review net11.0-targeting PRs, not justmain/net10 ones — without merging anything tomain. This is the net11 sibling of #35677.Two pieces, both on this branch (based on
origin/net11.0):Ports Enhance MauiBot AI summary review output + support net11-targeting PRs #35677 (enhanced MauiBot AI review) onto net11.0 — the 12-file content of Enhance MauiBot AI summary review output + support net11-targeting PRs #35677 applied cleanly on top of
net11.0(zero conflicts), so the net11 line carries the enhanced AI Summary review output, token-usage aggregation, and deep-UI-test review-id patching.ci-copilot.ymlnow auto-detects the PR's base branch. Previously the pipeline installed workloads and squash-merged the PR using whatever branch it was launched from (self). A net11-targeting PR reviewed from amain-based pipeline branch (or vice-versa) therefore got the wrong .NET SDK band and hit cross-branch squash-merge conflicts.A new "Resolve PR base branch" step (in both the
CopilotReviewandDeepUITestsstages):baseRefNameviagh,^(main|net[0-9]+\.0)$, fail-closed),origin/<baseRef>before workload install and the squash-merge.build.ps1 --target=dotnetthen provisions the correct band (net1111.0.100/ main10.0.100), andReview-PR.ps1 -Phase Setupsquash-merges onto the PR's real base — so one pipeline branch handles both main- and net11-targeting PRs.Trusted scripts are captured from the pipeline ref before the base checkout, so later tasks keep running the reviewed pipeline-branch scripts (security rule 3); Task 1 Setup now sources
Review-PR.ps1from$TRUSTEDlike the later tasks.Intentionally NOT changed
review-trigger.yml— unchanged (so this is testable without amainmerge; point the pipeline at this branch via/review --branch feature/net-11-ai-revieworworkflow_dispatch).Review-PR.ps1— unchanged (its CI path already "stay on current branch and squash-merge"; that branch is now the PR's base).Validation
ci-copilot.yml.bash -non both new "Resolve PR base branch" steps.System.Management.Automation.Language.Parseron all 8 changed PowerShell files.Invoke-PesteronAggregate-CopilotTokenUsage.Tests.ps1,Post-AISummaryComment.Tests.ps1,Review-PR.Tests.ps1→ 40/40 passed.main/net11.0/net10.0; rejects injection,refs/heads/...,net11.1, empty).ci-copilot-pipeline-security.instructions.md— no new violations.Relationship to #35677
#35677 targets
main. This PR targetsnet11.0and applies the same #35677 content plus the base-branch auto-detection. The reviewer infrastructure (#35677) is carried via the trusted-script copy; the reviewed tree is always the PR's real base (mainornet11.0) + the PR — so the separation between reviewer and reviewed code is preserved.