From 6717d1cb84a039ca6c3563f28b2df2879ceb0b7e Mon Sep 17 00:00:00 2001 From: Shane Neuville <5375137+PureWeen@users.noreply.github.com> Date: Wed, 25 Mar 2026 09:44:35 -0500 Subject: [PATCH 01/12] Add GitHub Actions workflow to run evaluate-pr-tests via Copilot CLI (#34548) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > [!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! ## Description Adds a [gh-aw (GitHub Agentic Workflows)](https://github.github.com/gh-aw/introduction/overview/) workflow that automatically evaluates test quality on PRs using the `evaluate-pr-tests` skill. ### What it does When a PR adds or modifies test files, this workflow: 1. **Checks out the PR branch** (including fork PRs) in a pre-agent step 2. **Runs the `evaluate-pr-tests` skill** via Copilot CLI in a sandboxed container 3. **Posts the evaluation report** as a PR comment using gh-aw safe-outputs ### Triggers | Trigger | When | Fork PR support | |---------|------|-----------------| | `pull_request` | Automatic on test file changes (`src/**/tests/**`) | ❌ Blocked by `pre_activation` gate | | `workflow_dispatch` | Manual — enter PR number | ✅ Works for all PRs | | `issue_comment` (`/evaluate-tests`) | Comment on PR | ⚠️ Same-repo only (see Known Limitations) | ### Security model | Layer | Implementation | |-------|---------------| | **gh-aw sandbox** | Agent runs in container with scrubbed credentials, network firewall | | **Safe outputs** | Max 1 PR comment per run, content-limited | | **Checkout without execution** | `steps:` checks out PR code but never executes workspace scripts | | **Base branch restoration** | `.github/skills/`, `.github/instructions/`, `.github/copilot-instructions.md` restored from base branch after checkout | | **Fork PR activation gate** | `pull_request` events blocked for forks via `head.repo.id == repository_id` | | **Pinned actions** | SHA-pinned `actions/checkout`, `actions/github-script`, etc. | | **Minimal permissions** | Each job declares only what it needs | | **Concurrency** | One evaluation per PR, cancels in-progress | | **Threat detection** | gh-aw built-in threat detection analyzes agent output | ### Files added/modified - `.github/workflows/copilot-evaluate-tests.md` — gh-aw workflow source - `.github/workflows/copilot-evaluate-tests.lock.yml` — Compiled workflow (auto-generated by `gh aw compile`) - `.github/skills/evaluate-pr-tests/scripts/Gather-TestContext.ps1` — Test context gathering script (binary-safe file download, path traversal protection) - `.github/instructions/gh-aw-workflows.instructions.md` — Copilot instructions for gh-aw development ### Known Limitations **Fork PR evaluation via `/evaluate-tests` comment is not supported in v1.** The gh-aw platform inserts a `checkout_pr_branch.cjs` step after all user steps, which may overwrite base-branch skill files restored for fork PRs. This is a known gh-aw platform limitation — user steps always run before platform-generated steps, with no way to insert steps after. **Workaround:** Use `workflow_dispatch` (Actions UI → "Run workflow" → enter PR number) to evaluate fork PRs. This trigger bypasses the platform checkout step entirely and works correctly. **Related upstream issues:** - [github/gh-aw#18481](https://github.com/github/gh-aw/issues/18481) — "Using gh-aw in forks of repositories" - [github/gh-aw#18518](https://github.com/github/gh-aw/issues/18518) — Fork detection and warning in `gh aw init` - [github/gh-aw#18520](https://github.com/github/gh-aw/issues/18520) — Fork context hint in failure messages - [github/gh-aw#18521](https://github.com/github/gh-aw/issues/18521) — Fork support documentation ### Fixes - Fixes #34602 --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jakub Florkowski --- .github/aw/actions-lock.json | 10 + .../gh-aw-workflows.instructions.md | 223 ++++ .github/scripts/Checkout-GhAwPr.ps1 | 100 ++ .../scripts/Gather-TestContext.ps1 | 130 +- .../workflows/copilot-evaluate-tests.lock.yml | 1089 +++++++++++++++++ .github/workflows/copilot-evaluate-tests.md | 139 +++ 6 files changed, 1677 insertions(+), 14 deletions(-) create mode 100644 .github/instructions/gh-aw-workflows.instructions.md create mode 100644 .github/scripts/Checkout-GhAwPr.ps1 create mode 100644 .github/workflows/copilot-evaluate-tests.lock.yml create mode 100644 .github/workflows/copilot-evaluate-tests.md diff --git a/.github/aw/actions-lock.json b/.github/aw/actions-lock.json index 774742dfa79e..4a1a1a9ed381 100644 --- a/.github/aw/actions-lock.json +++ b/.github/aw/actions-lock.json @@ -5,6 +5,16 @@ "version": "v8", "sha": "ed597411d8f924073f98dfc5c65a23a2325f34cd" }, + "github/gh-aw-actions/setup@v0.62.1": { + "repo": "github/gh-aw-actions/setup", + "version": "v0.62.1", + "sha": "95c4e2aa6adbdf63ff0b0fbf09945ad4f4716fea" + }, + "github/gh-aw-actions/setup@v0.62.2": { + "repo": "github/gh-aw-actions/setup", + "version": "v0.62.2", + "sha": "20045bbd5ad2632b9809856c389708eab1bd16ef" + }, "github/gh-aw/actions/setup@v0.43.19": { "repo": "github/gh-aw/actions/setup", "version": "v0.43.19", diff --git a/.github/instructions/gh-aw-workflows.instructions.md b/.github/instructions/gh-aw-workflows.instructions.md new file mode 100644 index 000000000000..30d02675e639 --- /dev/null +++ b/.github/instructions/gh-aw-workflows.instructions.md @@ -0,0 +1,223 @@ +--- +applyTo: + - ".github/workflows/*.md" + - ".github/workflows/*.lock.yml" +--- + +# gh-aw (GitHub Agentic Workflows) Guidelines + +## Architecture + +gh-aw workflows are authored as `.md` files with YAML frontmatter, compiled to `.lock.yml` via `gh aw compile`. The lock file is auto-generated — **never edit it manually**. + +### Execution Model + +``` +activation job (renders prompt from base branch .md via runtime-import) + ↓ +agent job: + user steps: (pre-agent, OUTSIDE firewall, has GITHUB_TOKEN) + ↓ + platform steps: (configure git → checkout_pr_branch.cjs → install CLI) + ↓ + agent: (INSIDE sandboxed container, NO credentials) +``` + +| Context | Has GITHUB_TOKEN | Has gh CLI | Has git creds | Can execute scripts | +|---------|-----------------|-----------|---------------|-------------------| +| `steps:` (user) | ✅ Yes | ✅ Yes | ✅ Yes | ✅ Yes — **be careful** | +| Platform steps | ✅ Yes | ✅ Yes | ✅ Yes | Platform-controlled | +| Agent container | ❌ Scrubbed | ❌ Scrubbed | ❌ Scrubbed | ✅ But sandboxed | + +### Step Ordering (Critical) + +User `steps:` **always run before** platform-generated steps. You cannot insert user steps after platform steps. + +The platform's `checkout_pr_branch.cjs` runs with `if: (github.event.pull_request) || (github.event.issue.pull_request)` — it is **skipped** for `workflow_dispatch` triggers. + +### Prompt Rendering + +The prompt is built in the **activation job** via `{{#runtime-import .github/workflows/.md}}`. This reads the `.md` file from the **base branch** workspace (before any PR checkout). The rendered prompt is uploaded as an artifact and downloaded by the agent job. + +- The agent prompt is always the base branch version — fork PRs cannot alter it +- The prompt references files on disk (e.g., `SKILL.md`) — those files must exist in the agent's workspace + +### Fork PR Activation Gate + +`gh aw compile` automatically injects a fork guard into the activation job's `if:` condition: `head.repo.id == repository_id`. This blocks fork PRs on `pull_request` events. This is **platform behavior** — do not add it manually. + +## Fork PR Handling + +### The "pwn-request" Threat Model + +The classic attack requires **checkout + execution** of fork code with elevated credentials. Checkout alone is not dangerous — the vulnerability is executing workspace scripts with `GITHUB_TOKEN`. + +Reference: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/ + +### Fork PR Behavior by Trigger + +| Trigger | `checkout_pr_branch.cjs` runs? | Fork handling | +|---------|-------------------------------|---------------| +| `pull_request` | ✅ Yes | Blocked by auto-generated activation gate | +| `workflow_dispatch` | ❌ Skipped | ✅ Works — user steps handle checkout and restore is final | +| `issue_comment` (same-repo) | ✅ Yes | ✅ Works — files already on PR branch | +| `issue_comment` (fork) | N/A | ❌ Blocked by fail-closed fork guard in `Checkout-GhAwPr.ps1` | + +### The `issue_comment` + Fork Problem + +For `/slash-command` triggers on fork PRs, `checkout_pr_branch.cjs` runs AFTER all user steps and re-checks out the fork branch. This overwrites any files restored by user steps (e.g., `.github/skills/`). There is no way to run user steps after platform steps. A fork could include a crafted `SKILL.md` that alters the agent's evaluation behavior. + +**Current approach (fail-closed fork guard):** `Checkout-GhAwPr.ps1` checks `isCrossRepository` via `gh pr view` for `issue_comment` triggers. If the PR is from a fork or the API call fails, the script exits with code 1. Fork PRs should use `workflow_dispatch` instead, where `checkout_pr_branch.cjs` is skipped and the user step restore is the final workspace state. + +**Upstream issue:** [github/gh-aw#18481](https://github.com/github/gh-aw/issues/18481) — "Using gh-aw in forks of repositories" + +### Safe Pattern: Checkout + Restore + +Use the shared `.github/scripts/Checkout-GhAwPr.ps1` script, which implements checkout + restore in a single reusable step: + +```yaml +steps: + - name: Checkout PR and restore agent infrastructure + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number || inputs.pr_number }} + run: pwsh .github/scripts/Checkout-GhAwPr.ps1 +``` + +The script: +1. Captures the base branch SHA before checkout +2. **Fork guard** (`issue_comment` only): checks `isCrossRepository` — exits 1 if fork or API failure +3. Checks out the PR branch via `gh pr checkout` +4. Deletes `.github/skills/` and `.github/instructions/` (prevents fork-added files) +5. Restores them from the base branch SHA (best-effort, non-fatal) + +**Behavior by trigger:** +- **`workflow_dispatch`**: Fork guard skipped. Platform checkout is skipped, so the restore IS the final workspace state (trusted files from base branch) +- **`issue_comment`** (same-repo): Fork guard passes. Platform re-checks out PR branch — files already match, effectively a no-op +- **`issue_comment`** (fork): Fork guard rejects — exits 1 with actionable notice to use `workflow_dispatch` +- **`pull_request`** (same-repo): Fork guard skipped. Files already exist, restore is a no-op + +### Anti-Patterns + +**Do NOT skip checkout for fork PRs:** + +```bash +# ❌ ANTI-PATTERN: Makes fork PRs unevaluable +if [ "$HEAD_OWNER" != "$BASE_OWNER" ]; then + echo "Skipping checkout for fork PR" + exit 0 # Agent evaluates workflow branch instead of PR +fi +``` + +Skipping checkout means the agent evaluates the wrong files. The correct approach is: always check out the PR, then restore agent infrastructure from the base branch. + +**Do NOT execute workspace code after fork checkout:** + +```yaml +# ❌ DANGEROUS: runs fork code with GITHUB_TOKEN +- name: Checkout PR + run: gh pr checkout "$PR_NUMBER" ... +- name: Run analysis + run: pwsh .github/skills/some-script.ps1 +``` + +If you need to run scripts, either: +1. Run them **before** the checkout (from the base branch) +2. Run them **inside the agent container** (sandboxed, no tokens) + +## Compilation + +```bash +# Compile after every change to the .md source +gh aw compile .github/workflows/.md + +# This updates: +# - .github/workflows/.lock.yml (auto-generated) +# - .github/aw/actions-lock.json +``` + +**Always commit the compiled lock file alongside the source `.md`.** + +## Common Patterns + +### Pre-Agent Data Prep (the `steps:` pattern) + +Use `steps:` for any operation requiring GitHub API access that the agent needs: + +```yaml +steps: + - name: Fetch PR data + env: + GH_TOKEN: ${{ github.token }} + run: | + gh pr view "$PR_NUMBER" --json title,body > pr-metadata.json + gh pr diff "$PR_NUMBER" --name-only > changed-files.txt +``` + +### Safe Outputs (Posting Comments) + +```yaml +safe-outputs: + add-comment: + max: 1 + target: "*" # Required for workflow_dispatch (no triggering PR context) +``` + +### Concurrency + +Include all trigger-specific PR number sources: + +```yaml +concurrency: + group: "my-workflow-${{ github.event.issue.number || github.event.pull_request.number || inputs.pr_number || github.run_id }}" + cancel-in-progress: true +``` + +### Noise Reduction + +Filter `pull_request` triggers to relevant paths and add a gate step: + +```yaml +on: + pull_request: + paths: + - 'src/**/tests/**' + +steps: + - name: Gate — skip if no relevant files + if: github.event_name == 'pull_request' + run: | + FILES=$(gh pr diff "$PR_NUMBER" --name-only | grep -E '\.cs$' || true) + if [ -z "$FILES" ]; then exit 1; fi +``` + +Manual triggers (`workflow_dispatch`, `issue_comment`) should bypass the gate. Note: `exit 1` causes a red ❌ on non-matching PRs — this is intentional (no built-in "skip" mechanism in gh-aw steps). + +## Limitations + +| What | Behavior | Workaround | +|------|----------|------------| +| User steps always before platform steps | Cannot run user code after `checkout_pr_branch.cjs` | Use `workflow_dispatch` for fork PRs; see [gh-aw#18481](https://github.com/github/gh-aw/issues/18481) | +| `--allow-all-tools` in lock.yml | Emitted by `gh aw compile` | Cannot override from `.md` source | +| MCP integrity filtering | Fork PRs blocked as "unapproved" | Use `steps:` checkout instead of MCP | +| `gh` CLI inside agent | Credentials scrubbed | Use `steps:` for API calls, or MCP tools | +| `issue_comment` trigger | Requires workflow on default branch | Must merge to `main` before `/slash-commands` work | +| Duplicate runs | gh-aw sometimes creates 2 runs per dispatch | Harmless, use concurrency groups | + +### Upstream References + +- [github/gh-aw#18481](https://github.com/github/gh-aw/issues/18481) — Fork support tracking issue +- [github/gh-aw#18518](https://github.com/github/gh-aw/issues/18518) — Fork detection in `gh aw init` +- [github/gh-aw#18521](https://github.com/github/gh-aw/issues/18521) — Fork support documentation + +## Troubleshooting + +| Symptom | Cause | Fix | +|---------|-------|-----| +| Agent evaluates wrong PR | `workflow_dispatch` checks out workflow branch | Add `gh pr checkout` in `steps:` | +| Agent can't find SKILL.md | Fork PR branch doesn't have `.github/skills/` | Agent posts "rebase or use `workflow_dispatch`" message; or rebase fork on `main` | +| Fork PR rejected on `/evaluate-tests` | Fail-closed fork guard in `Checkout-GhAwPr.ps1` | Use `workflow_dispatch` with `pr_number` input instead | +| `gh` commands fail in agent | Credentials scrubbed inside container | Move to `steps:` section | +| Lock file out of date | Forgot to recompile | Run `gh aw compile` | +| Integrity filtering warning | MCP reading fork PR data | Expected, non-blocking | +| `/slash-command` doesn't trigger | Workflow not on default branch | Merge to `main` first | diff --git a/.github/scripts/Checkout-GhAwPr.ps1 b/.github/scripts/Checkout-GhAwPr.ps1 new file mode 100644 index 000000000000..4a6380a50421 --- /dev/null +++ b/.github/scripts/Checkout-GhAwPr.ps1 @@ -0,0 +1,100 @@ +<# +.SYNOPSIS + Shared PR checkout for gh-aw (GitHub Agentic Workflows). + +.DESCRIPTION + Checks out a PR branch and restores trusted agent infrastructure (skills, + instructions) from the base branch. For issue_comment triggers, fork PRs + are rejected (fail-closed) because the platform's checkout_pr_branch.cjs + overwrites restored files after user steps. Fork PRs should use + workflow_dispatch instead. + + SECURITY NOTE: This script checks out PR code onto disk. This is safe + because NO subsequent user steps execute workspace code — the gh-aw + platform copies the workspace into a sandboxed container with scrubbed + credentials before starting the agent. The classic "pwn-request" attack + requires checkout + execution; we only do checkout. + + DO NOT add steps after this that run scripts from the workspace + (e.g., ./build.sh, pwsh ./script.ps1). That would create an actual + fork code execution vulnerability. See: + https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/ + +.NOTES + Required environment variables (set by the calling workflow step): + GH_TOKEN - GitHub token for API access + PR_NUMBER - PR number to check out + GITHUB_REPOSITORY - owner/repo (set by GitHub Actions) + GITHUB_ENV - path to env file (set by GitHub Actions) + GITHUB_EVENT_NAME - trigger type (set by GitHub Actions) +#> + +$ErrorActionPreference = 'Stop' + +# ── Validate inputs ────────────────────────────────────────────────────────── + +if (-not $env:PR_NUMBER -or $env:PR_NUMBER -eq '0') { + Write-Host "No PR number available, using default checkout" + exit 0 +} + +$PrNumber = $env:PR_NUMBER + +# ── Fork guard (issue_comment only) ───────────────────────────────────────── +# For issue_comment triggers, platform's checkout_pr_branch.cjs runs AFTER user +# steps and re-checks out the fork branch, overwriting any restored skill/instruction +# files. A fork could include a crafted SKILL.md that alters agent behavior. +# Fail closed: if we can't verify origin, exit 1 (not 0). +# Fork PRs can still be evaluated via workflow_dispatch (where platform checkout is skipped). + +if ($env:GITHUB_EVENT_NAME -eq 'issue_comment') { + $isFork = gh pr view $PrNumber --repo $env:GITHUB_REPOSITORY --json isCrossRepository --jq '.isCrossRepository' 2>&1 + if ($LASTEXITCODE -ne 0) { + Write-Host "❌ Could not verify PR origin — failing closed" + exit 1 + } + if ($isFork -eq 'true') { + Write-Host "::notice::Fork PR detected — /evaluate-tests via issue_comment is not supported for fork PRs. Use workflow_dispatch with pr_number=$PrNumber instead." + exit 1 + } +} + +# ── Save base branch SHA ───────────────────────────────────────────────────── +# Must be captured BEFORE checkout replaces HEAD. +# Exported for potential use by downstream platform steps (e.g., checkout_pr_branch.cjs) + +$BaseSha = git rev-parse HEAD +if ($LASTEXITCODE -ne 0) { + Write-Host "❌ Failed to get current HEAD SHA" + exit 1 +} +Add-Content -Path $env:GITHUB_ENV -Value "BASE_SHA=$BaseSha" + +# ── Checkout PR branch ────────────────────────────────────────────────────── + +Write-Host "Checking out PR #$PrNumber..." +gh pr checkout $PrNumber --repo $env:GITHUB_REPOSITORY +if ($LASTEXITCODE -ne 0) { + Write-Host "❌ Failed to checkout PR #$PrNumber" + exit 1 +} +Write-Host "✅ Checked out PR #$PrNumber" +git log --oneline -1 + +# ── Restore agent infrastructure from base branch ──────────────────────────── +# Best-effort restore of skill/instruction files from the base branch. +# - workflow_dispatch: platform checkout is skipped, so this IS the final state +# - issue_comment (same-repo): platform's checkout_pr_branch.cjs runs after and +# overwrites, but files already match (same repo). Fork PRs are blocked above. +# - pull_request (same-repo): files already exist, this is a no-op +# rm -rf first to prevent fork-added files from surviving the restore. + +if (Test-Path '.github/skills/') { Remove-Item -Recurse -Force '.github/skills/' } +if (Test-Path '.github/instructions/') { Remove-Item -Recurse -Force '.github/instructions/' } + +git checkout $BaseSha -- .github/skills/ .github/instructions/ .github/copilot-instructions.md 2>&1 +if ($LASTEXITCODE -eq 0) { + Write-Host "✅ Restored agent infrastructure from base branch ($BaseSha)" +} else { + Write-Host "⚠️ Could not restore agent infrastructure from base branch — files may come from the PR branch" +} diff --git a/.github/skills/evaluate-pr-tests/scripts/Gather-TestContext.ps1 b/.github/skills/evaluate-pr-tests/scripts/Gather-TestContext.ps1 index 8b8805b5596b..7fe0cdedba50 100644 --- a/.github/skills/evaluate-pr-tests/scripts/Gather-TestContext.ps1 +++ b/.github/skills/evaluate-pr-tests/scripts/Gather-TestContext.ps1 @@ -12,6 +12,12 @@ - Find existing similar tests - Assess platform scope +.PARAMETER PrNumber + Explicit PR number to evaluate. When provided, the script uses + `gh pr view ` to detect the base branch and `gh pr diff ` + to get the changed files. This avoids relying on the currently checked-out + branch, which is critical for workflow_dispatch triggers. + .PARAMETER BaseBranch Base branch to diff against. Auto-detected from PR if not specified. @@ -19,13 +25,16 @@ Directory to write the context report to. .EXAMPLE - ./Gather-TestContext.ps1 + ./Gather-TestContext.ps1 -PrNumber 31244 .EXAMPLE ./Gather-TestContext.ps1 -BaseBranch "origin/main" #> param( + [Parameter(Mandatory = $false)] + [int]$PrNumber, + [Parameter(Mandatory = $false)] [string]$BaseBranch, @@ -42,7 +51,22 @@ New-Item -ItemType Directory -Force -Path $OutputDir | Out-Null $reportPath = Join-Path $OutputDir "context.md" # --- 1. Detect base branch --- -if (-not $BaseBranch) { +$usePrDiff = $false +if ($PrNumber -gt 0) { + # Explicit PR number — use gh pr view/diff so we don't depend on local branch + Write-Host "📋 Evaluating PR #$PrNumber (explicit)" + if (-not $BaseBranch) { + try { + $prJson = gh pr view $PrNumber --json baseRefName 2>$null + if ($prJson) { + $prInfo = $prJson | ConvertFrom-Json + $BaseBranch = "origin/$($prInfo.baseRefName)" + } + } catch { } + if (-not $BaseBranch) { $BaseBranch = "origin/main" } + } + $usePrDiff = $true +} elseif (-not $BaseBranch) { try { $prJson = gh pr view --json baseRefName 2>$null if ($prJson) { @@ -61,15 +85,69 @@ git fetch origin --quiet 2>$null # --- 2. Get changed files --- $changedFiles = @() -$diffOutput = git diff --name-only "$BaseBranch...HEAD" 2>$null -if ($diffOutput) { - $changedFiles = $diffOutput -split "`r?`n" | ForEach-Object { $_.Trim() } | Where-Object { $_ -ne "" } -} else { - $diffOutput = git diff --name-only "$BaseBranch" 2>$null + +if ($changedFiles.Count -eq 0 -and $usePrDiff) { + # Use gh pr diff to get file list directly from GitHub API — works regardless of local checkout + $diffOutput = gh pr diff $PrNumber --name-only 2>$null if ($diffOutput) { $changedFiles = $diffOutput -split "`r?`n" | ForEach-Object { $_.Trim() } | Where-Object { $_ -ne "" } } } +if ($changedFiles.Count -eq 0) { + $diffOutput = git diff --name-only "$BaseBranch...HEAD" 2>$null + if ($diffOutput) { + $changedFiles = $diffOutput -split "`r?`n" | ForEach-Object { $_.Trim() } | Where-Object { $_ -ne "" } + } else { + $diffOutput = git diff --name-only "$BaseBranch" 2>$null + if ($diffOutput) { + $changedFiles = $diffOutput -split "`r?`n" | ForEach-Object { $_.Trim() } | Where-Object { $_ -ne "" } + } + } +} + +# --- 2b. Download missing files via GitHub API (needed when PR isn't checked out locally) --- +if ($usePrDiff -and $changedFiles.Count -gt 0) { + $headSha = $null + try { + $headSha = gh pr view $PrNumber --json headRefOid --jq '.headRefOid' 2>$null + } catch { } + + if ($headSha) { + $downloadCount = 0 + $repoRootFull = [System.IO.Path]::GetFullPath($RepoRoot) + $owner, $repo = ($env:GITHUB_REPOSITORY ?? "dotnet/maui") -split '/', 2 + foreach ($file in $changedFiles) { + # Path traversal guard: ensure resolved path stays within repo root + $targetPath = [System.IO.Path]::GetFullPath((Join-Path $RepoRoot $file)) + if (-not $targetPath.StartsWith($repoRootFull + [System.IO.Path]::DirectorySeparatorChar)) { + Write-Warning "Skipping out-of-root path: $file" + continue + } + + if (-not (Test-Path $targetPath)) { + try { + $dir = [System.IO.Path]::GetDirectoryName($targetPath) + if ($dir -and -not (Test-Path $dir)) { + New-Item -ItemType Directory -Force -Path $dir | Out-Null + } + $encodedFile = [Uri]::EscapeDataString($file) -replace '%2F', '/' + $apiPath = "repos/$owner/$repo/contents/$($encodedFile)?ref=$headSha" + $b64 = gh api $apiPath --jq '.content' 2>$null + if ($b64) { + $bytes = [System.Convert]::FromBase64String(($b64 -replace '\s', '')) + [System.IO.File]::WriteAllBytes($targetPath, $bytes) + $downloadCount++ + } + } catch { + Write-Host "⚠️ Could not download $file via API: $_" + } + } + } + if ($downloadCount -gt 0) { + Write-Host "📥 Downloaded $downloadCount file(s) from PR #$PrNumber head ($($headSha.Substring(0,7)))" + } + } +} if ($changedFiles.Count -eq 0) { Write-Host "⚠️ No changed files detected. Check your branch and base branch." @@ -128,7 +206,8 @@ function Test-UITestConventions { # --- Naming (only flag files in Issues/ directory that look like issue tests) --- $fileName = [System.IO.Path]::GetFileNameWithoutExtension($TestFile) if ($TestFile -match "Issues/" -and $fileName -match "^Issue" -and $fileName -notmatch "^Issue\d+$") { - $issues += "Issue test file name ``$fileName`` should follow ``IssueXXXXX`` pattern" + $safeName = Escape-ForCodeSpan $fileName + $issues += "Issue test file name ``$safeName`` should follow ``IssueXXXXX`` pattern" } # --- Inheritance --- @@ -225,7 +304,8 @@ function Test-UITestConventions { # For .xaml files, skip C# attribute checks (they live in code-behind) if ($hostFile -notmatch "\.xaml$") { if ($hostContent -notmatch "\[Issue\(") { - $issues += "HostApp page ``$([System.IO.Path]::GetFileName($hostFile))`` missing ``[Issue()]`` attribute" + $safeHost = Escape-ForCodeSpan ([System.IO.Path]::GetFileName($hostFile)) + $issues += "HostApp page ``$safeHost`` missing ``[Issue()]`` attribute" } } if ($hostContent -match "new\s+Frame\b") { @@ -334,7 +414,8 @@ function Test-XamlTestConventions { # File naming for issues $fileName = [System.IO.Path]::GetFileNameWithoutExtension($TestFile) if ($TestFile -match "Issues/" -and $fileName -notmatch "^Maui\d+$") { - $issues += "Issue test file name ``$fileName`` doesn't follow ``MauiXXXXX`` pattern" + $safeName = Escape-ForCodeSpan $fileName + $issues += "Issue test file name ``$safeName`` doesn't follow ``MauiXXXXX`` pattern" } return @{ Issues = $issues; Info = $info } @@ -353,7 +434,25 @@ $report += "" $report += "| Category | Count | Files |" $report += "|----------|-------|-------|" -function Format-FileList { param([string[]]$files) if ($files.Count -eq 0) { return "_none_" } return ($files | ForEach-Object { "``$_``" }) -join ", " } +function Escape-ForCodeSpan { + param([string]$Text) + # Neutralise characters that break markdown code spans or line structure. + # Backticks are replaced with a visually similar RIGHT SINGLE QUOTATION MARK (U+2019) + # so the surrounding `` delimiters stay balanced. Newlines / carriage-returns are + # stripped because they would break table rows or heading lines. + return ($Text -replace '`', [char]0x2019 -replace '[\r\n]', '') +} + +function Format-FileList { + param([string[]]$files) + if ($files.Count -eq 0) { return "_none_" } + return ($files | ForEach-Object { + # Escape markdown metacharacters to prevent injection via crafted filenames. + # Use double-backtick code spans (`` ... ``) so literal backticks render correctly. + $escaped = (Escape-ForCodeSpan $_) -replace '\|', '\|' -replace '<', '<' -replace '>', '>' + "````$escaped````" + }) -join ", " +} $report += "| **Fix files** | $($fixFiles.Count) | $(Format-FileList $fixFiles) |" $report += "| **UI Tests (NUnit)** | $($uiTestFiles.Count) | $(Format-FileList $uiTestFiles) |" @@ -396,7 +495,8 @@ if ($uiTestFiles.Count -gt 0) { $hostName -eq $baseName } $result = Test-UITestConventions -TestFile $testFile -HostAppFiles $matchingHostFiles - $report += "### ``$baseName``" + $safeBase = Escape-ForCodeSpan $baseName + $report += "### ``$safeBase``" if ($result.Info.Count -gt 0) { foreach ($i in $result.Info) { $report += "- ℹ️ $i" } } @@ -419,7 +519,8 @@ if ($unitTestFiles.Count -gt 0) { foreach ($testFile in $unitTestFiles) { $baseName = [System.IO.Path]::GetFileNameWithoutExtension($testFile) $result = Test-UnitTestConventions -TestFile $testFile - $report += "### ``$baseName``" + $safeBase = Escape-ForCodeSpan $baseName + $report += "### ``$safeBase``" if ($result.Info.Count -gt 0) { foreach ($i in $result.Info) { $report += "- ℹ️ $i" } } @@ -442,7 +543,8 @@ if ($xamlTestFiles.Count -gt 0) { foreach ($testFile in ($xamlTestFiles | Where-Object { $_ -match "\.cs$" })) { $baseName = [System.IO.Path]::GetFileNameWithoutExtension($testFile) $result = Test-XamlTestConventions -TestFile $testFile - $report += "### ``$baseName``" + $safeBase = Escape-ForCodeSpan $baseName + $report += "### ``$safeBase``" if ($result.Info.Count -gt 0) { foreach ($i in $result.Info) { $report += "- ℹ️ $i" } } diff --git a/.github/workflows/copilot-evaluate-tests.lock.yml b/.github/workflows/copilot-evaluate-tests.lock.yml new file mode 100644 index 000000000000..c1e75132b153 --- /dev/null +++ b/.github/workflows/copilot-evaluate-tests.lock.yml @@ -0,0 +1,1089 @@ +# ___ _ _ +# / _ \ | | (_) +# | |_| | __ _ ___ _ __ | |_ _ ___ +# | _ |/ _` |/ _ \ '_ \| __| |/ __| +# | | | | (_| | __/ | | | |_| | (__ +# \_| |_/\__, |\___|_| |_|\__|_|\___| +# __/ | +# _ _ |___/ +# | | | | / _| | +# | | | | ___ _ __ _ __| |_| | _____ ____ +# | |/\| |/ _ \ '__| |/ /| _| |/ _ \ \ /\ / / ___| +# \ /\ / (_) | | | | ( | | | | (_) \ V V /\__ \ +# \/ \/ \___/|_| |_|\_\|_| |_|\___/ \_/\_/ |___/ +# +# This file was automatically generated by gh-aw (v0.62.2). DO NOT EDIT. +# +# To update this file, edit the corresponding .md file and run: +# gh aw compile +# Not all edits will cause changes to this file. +# +# For more information: https://github.github.com/gh-aw/introduction/overview/ +# +# Evaluates test quality, coverage, and appropriateness on PRs that add or modify tests +# +# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"9a0e480927d61956be62f85669d2e599525442694d280126849fe87e727c31df","compiler_version":"v0.62.2","strict":true} + +name: "Evaluate PR Tests" +"on": + issue_comment: + types: + - created + pull_request: + paths: + - src/**/tests/** + - src/**/test/** + types: + - opened + - synchronize + - reopened + - ready_for_review + workflow_dispatch: + inputs: + pr_number: + description: PR number to evaluate + required: true + type: number + +permissions: {} + +concurrency: + cancel-in-progress: true + group: evaluate-pr-tests-${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number || github.run_id }} + +run-name: "Evaluate PR Tests" + +jobs: + activation: + needs: pre_activation + if: > + (needs.pre_activation.outputs.activated == 'true') && (((github.event_name == 'pull_request' && github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || (github.event_name == 'issue_comment' && + github.event.issue.pull_request && + startsWith(github.event.comment.body, '/evaluate-tests'))) && ((github.event_name != 'pull_request') || (github.event.pull_request.head.repo.id == github.repository_id))) + runs-on: ubuntu-slim + permissions: + contents: read + outputs: + body: ${{ steps.sanitized.outputs.body }} + comment_id: "" + comment_repo: "" + lockdown_check_failed: ${{ steps.generate_aw_info.outputs.lockdown_check_failed == 'true' }} + model: ${{ steps.generate_aw_info.outputs.model }} + secret_verification_result: ${{ steps.validate-secret.outputs.verification_result }} + text: ${{ steps.sanitized.outputs.text }} + title: ${{ steps.sanitized.outputs.title }} + steps: + - name: Setup Scripts + uses: github/gh-aw-actions/setup@20045bbd5ad2632b9809856c389708eab1bd16ef # v0.62.2 + with: + destination: ${{ runner.temp }}/gh-aw/actions + - name: Generate agentic run info + id: generate_aw_info + env: + GH_AW_INFO_ENGINE_ID: "copilot" + GH_AW_INFO_ENGINE_NAME: "GitHub Copilot CLI" + GH_AW_INFO_MODEL: "claude-sonnet-4.6" + GH_AW_INFO_VERSION: "" + GH_AW_INFO_AGENT_VERSION: "latest" + GH_AW_INFO_CLI_VERSION: "v0.62.2" + GH_AW_INFO_WORKFLOW_NAME: "Evaluate PR Tests" + GH_AW_INFO_EXPERIMENTAL: "false" + GH_AW_INFO_SUPPORTS_TOOLS_ALLOWLIST: "true" + GH_AW_INFO_STAGED: "false" + GH_AW_INFO_ALLOWED_DOMAINS: '["defaults"]' + GH_AW_INFO_FIREWALL_ENABLED: "true" + GH_AW_INFO_AWF_VERSION: "v0.24.3" + GH_AW_INFO_AWMG_VERSION: "" + GH_AW_INFO_FIREWALL_TYPE: "squid" + GH_AW_COMPILED_STRICT: "true" + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/generate_aw_info.cjs'); + await main(core, context); + - name: Validate COPILOT_GITHUB_TOKEN secret + id: validate-secret + run: ${RUNNER_TEMP}/gh-aw/actions/validate_multi_secret.sh COPILOT_GITHUB_TOKEN 'GitHub Copilot CLI' https://github.github.com/gh-aw/reference/engines/#github-copilot-default + env: + COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }} + - name: Checkout .github and .agents folders + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + sparse-checkout: | + .github + .agents + sparse-checkout-cone-mode: true + fetch-depth: 1 + - name: Check workflow file timestamps + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_WORKFLOW_FILE: "copilot-evaluate-tests.lock.yml" + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/check_workflow_timestamp_api.cjs'); + await main(); + - name: Compute current body text + id: sanitized + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/compute_text.cjs'); + await main(); + - name: Create prompt with built-in context + env: + GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt + GH_AW_SAFE_OUTPUTS: ${{ env.GH_AW_SAFE_OUTPUTS }} + GH_AW_EXPR_93C755A4: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} + GH_AW_GITHUB_ACTOR: ${{ github.actor }} + GH_AW_GITHUB_EVENT_COMMENT_ID: ${{ github.event.comment.id }} + GH_AW_GITHUB_EVENT_DISCUSSION_NUMBER: ${{ github.event.discussion.number }} + GH_AW_GITHUB_EVENT_ISSUE_NUMBER: ${{ github.event.issue.number }} + GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }} + GH_AW_GITHUB_REPOSITORY: ${{ github.repository }} + GH_AW_GITHUB_RUN_ID: ${{ github.run_id }} + GH_AW_GITHUB_WORKSPACE: ${{ github.workspace }} + GH_AW_IS_PR_COMMENT: ${{ github.event.issue.pull_request && 'true' || '' }} + run: | + bash ${RUNNER_TEMP}/gh-aw/actions/create_prompt_first.sh + { + cat << 'GH_AW_PROMPT_EOF' + + GH_AW_PROMPT_EOF + cat "${RUNNER_TEMP}/gh-aw/prompts/xpia.md" + cat "${RUNNER_TEMP}/gh-aw/prompts/temp_folder_prompt.md" + cat "${RUNNER_TEMP}/gh-aw/prompts/markdown.md" + cat "${RUNNER_TEMP}/gh-aw/prompts/safe_outputs_prompt.md" + cat << 'GH_AW_PROMPT_EOF' + + Tools: add_comment, missing_tool, missing_data, noop + + + The following GitHub context information is available for this workflow: + {{#if __GH_AW_GITHUB_ACTOR__ }} + - **actor**: __GH_AW_GITHUB_ACTOR__ + {{/if}} + {{#if __GH_AW_GITHUB_REPOSITORY__ }} + - **repository**: __GH_AW_GITHUB_REPOSITORY__ + {{/if}} + {{#if __GH_AW_GITHUB_WORKSPACE__ }} + - **workspace**: __GH_AW_GITHUB_WORKSPACE__ + {{/if}} + {{#if __GH_AW_GITHUB_EVENT_ISSUE_NUMBER__ }} + - **issue-number**: #__GH_AW_GITHUB_EVENT_ISSUE_NUMBER__ + {{/if}} + {{#if __GH_AW_GITHUB_EVENT_DISCUSSION_NUMBER__ }} + - **discussion-number**: #__GH_AW_GITHUB_EVENT_DISCUSSION_NUMBER__ + {{/if}} + {{#if __GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER__ }} + - **pull-request-number**: #__GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER__ + {{/if}} + {{#if __GH_AW_GITHUB_EVENT_COMMENT_ID__ }} + - **comment-id**: __GH_AW_GITHUB_EVENT_COMMENT_ID__ + {{/if}} + {{#if __GH_AW_GITHUB_RUN_ID__ }} + - **workflow-run-id**: __GH_AW_GITHUB_RUN_ID__ + {{/if}} + + + GH_AW_PROMPT_EOF + cat "${RUNNER_TEMP}/gh-aw/prompts/github_mcp_tools_with_safeoutputs_prompt.md" + if [ "$GITHUB_EVENT_NAME" = "issue_comment" ] && [ -n "$GH_AW_IS_PR_COMMENT" ] || [ "$GITHUB_EVENT_NAME" = "pull_request_review_comment" ] || [ "$GITHUB_EVENT_NAME" = "pull_request_review" ]; then + cat "${RUNNER_TEMP}/gh-aw/prompts/pr_context_prompt.md" + fi + cat << 'GH_AW_PROMPT_EOF' + + GH_AW_PROMPT_EOF + cat << 'GH_AW_PROMPT_EOF' + {{#runtime-import .github/workflows/copilot-evaluate-tests.md}} + GH_AW_PROMPT_EOF + } > "$GH_AW_PROMPT" + - name: Interpolate variables and render templates + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt + GH_AW_EXPR_93C755A4: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} + GH_AW_GITHUB_REPOSITORY: ${{ github.repository }} + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/interpolate_prompt.cjs'); + await main(); + - name: Substitute placeholders + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt + GH_AW_EXPR_93C755A4: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} + GH_AW_GITHUB_ACTOR: ${{ github.actor }} + GH_AW_GITHUB_EVENT_COMMENT_ID: ${{ github.event.comment.id }} + GH_AW_GITHUB_EVENT_DISCUSSION_NUMBER: ${{ github.event.discussion.number }} + GH_AW_GITHUB_EVENT_ISSUE_NUMBER: ${{ github.event.issue.number }} + GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }} + GH_AW_GITHUB_REPOSITORY: ${{ github.repository }} + GH_AW_GITHUB_RUN_ID: ${{ github.run_id }} + GH_AW_GITHUB_WORKSPACE: ${{ github.workspace }} + GH_AW_IS_PR_COMMENT: ${{ github.event.issue.pull_request && 'true' || '' }} + GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED: ${{ needs.pre_activation.outputs.activated }} + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + + const substitutePlaceholders = require('${{ runner.temp }}/gh-aw/actions/substitute_placeholders.cjs'); + + // Call the substitution function + return await substitutePlaceholders({ + file: process.env.GH_AW_PROMPT, + substitutions: { + GH_AW_EXPR_93C755A4: process.env.GH_AW_EXPR_93C755A4, + GH_AW_GITHUB_ACTOR: process.env.GH_AW_GITHUB_ACTOR, + GH_AW_GITHUB_EVENT_COMMENT_ID: process.env.GH_AW_GITHUB_EVENT_COMMENT_ID, + GH_AW_GITHUB_EVENT_DISCUSSION_NUMBER: process.env.GH_AW_GITHUB_EVENT_DISCUSSION_NUMBER, + GH_AW_GITHUB_EVENT_ISSUE_NUMBER: process.env.GH_AW_GITHUB_EVENT_ISSUE_NUMBER, + GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER: process.env.GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER, + GH_AW_GITHUB_REPOSITORY: process.env.GH_AW_GITHUB_REPOSITORY, + GH_AW_GITHUB_RUN_ID: process.env.GH_AW_GITHUB_RUN_ID, + GH_AW_GITHUB_WORKSPACE: process.env.GH_AW_GITHUB_WORKSPACE, + GH_AW_IS_PR_COMMENT: process.env.GH_AW_IS_PR_COMMENT, + GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED: process.env.GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED + } + }); + - name: Validate prompt placeholders + env: + GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt + run: bash ${RUNNER_TEMP}/gh-aw/actions/validate_prompt_placeholders.sh + - name: Print prompt + env: + GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt + run: bash ${RUNNER_TEMP}/gh-aw/actions/print_prompt_summary.sh + - name: Upload activation artifact + if: success() + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + with: + name: activation + path: | + /tmp/gh-aw/aw_info.json + /tmp/gh-aw/aw-prompts/prompt.txt + retention-days: 1 + + agent: + needs: activation + runs-on: ubuntu-latest + permissions: + contents: read + issues: read + pull-requests: read + env: + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + GH_AW_ASSETS_ALLOWED_EXTS: "" + GH_AW_ASSETS_BRANCH: "" + GH_AW_ASSETS_MAX_SIZE_KB: 0 + GH_AW_MCP_LOG_DIR: /tmp/gh-aw/mcp-logs/safeoutputs + GH_AW_WORKFLOW_ID_SANITIZED: copilotevaluatetests + outputs: + checkout_pr_success: ${{ steps.checkout-pr.outputs.checkout_pr_success || 'true' }} + detection_conclusion: ${{ steps.detection_conclusion.outputs.conclusion }} + detection_success: ${{ steps.detection_conclusion.outputs.success }} + has_patch: ${{ steps.collect_output.outputs.has_patch }} + inference_access_error: ${{ steps.detect-inference-error.outputs.inference_access_error || 'false' }} + model: ${{ needs.activation.outputs.model }} + output: ${{ steps.collect_output.outputs.output }} + output_types: ${{ steps.collect_output.outputs.output_types }} + steps: + - name: Setup Scripts + uses: github/gh-aw-actions/setup@20045bbd5ad2632b9809856c389708eab1bd16ef # v0.62.2 + with: + destination: ${{ runner.temp }}/gh-aw/actions + - name: Set runtime paths + run: | + echo "GH_AW_SAFE_OUTPUTS=${RUNNER_TEMP}/gh-aw/safeoutputs/outputs.jsonl" >> "$GITHUB_ENV" + echo "GH_AW_SAFE_OUTPUTS_CONFIG_PATH=${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" >> "$GITHUB_ENV" + echo "GH_AW_SAFE_OUTPUTS_TOOLS_PATH=${RUNNER_TEMP}/gh-aw/safeoutputs/tools.json" >> "$GITHUB_ENV" + - name: Checkout repository + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + - name: Create gh-aw temp directory + run: bash ${RUNNER_TEMP}/gh-aw/actions/create_gh_aw_tmp_dir.sh + - name: Configure gh CLI for GitHub Enterprise + run: bash ${RUNNER_TEMP}/gh-aw/actions/configure_gh_for_ghe.sh + env: + GH_TOKEN: ${{ github.token }} + - env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + if: github.event_name == 'pull_request' + name: Gate — skip if no test source files in diff + run: "TEST_FILES=$(gh pr diff \"$PR_NUMBER\" --repo \"$GITHUB_REPOSITORY\" --name-only \\\n | grep -E '\\.(cs|xaml)$' \\\n | grep -iE '(tests?/|TestCases|UnitTests|DeviceTests)' \\\n || true)\nif [ -z \"$TEST_FILES\" ]; then\n echo \"⏭️ No test source files (.cs/.xaml) found in PR diff. Skipping evaluation.\"\n exit 1\nfi\necho \"✅ Found test files to evaluate:\"\necho \"$TEST_FILES\" | head -20\n" + - env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} + name: Checkout PR and restore agent infrastructure + run: pwsh .github/scripts/Checkout-GhAwPr.ps1 + + - name: Configure Git credentials + env: + REPO_NAME: ${{ github.repository }} + SERVER_URL: ${{ github.server_url }} + run: | + git config --global user.email "github-actions[bot]@users.noreply.github.com" + git config --global user.name "github-actions[bot]" + git config --global am.keepcr true + # Re-authenticate git with GitHub token + SERVER_URL_STRIPPED="${SERVER_URL#https://}" + git remote set-url origin "https://x-access-token:${{ github.token }}@${SERVER_URL_STRIPPED}/${REPO_NAME}.git" + echo "Git configured with standard GitHub Actions identity" + - name: Checkout PR branch + id: checkout-pr + if: | + (github.event.pull_request) || (github.event.issue.pull_request) + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} + with: + github-token: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/checkout_pr_branch.cjs'); + await main(); + - name: Install GitHub Copilot CLI + run: ${RUNNER_TEMP}/gh-aw/actions/install_copilot_cli.sh latest + env: + GH_HOST: github.com + - name: Install AWF binary + run: bash ${RUNNER_TEMP}/gh-aw/actions/install_awf_binary.sh v0.24.3 + - name: Determine automatic lockdown mode for GitHub MCP Server + id: determine-automatic-lockdown + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }} + GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }} + with: + script: | + const determineAutomaticLockdown = require('${{ runner.temp }}/gh-aw/actions/determine_automatic_lockdown.cjs'); + await determineAutomaticLockdown(github, context, core); + - name: Download container images + run: bash ${RUNNER_TEMP}/gh-aw/actions/download_docker_images.sh ghcr.io/github/gh-aw-firewall/agent:0.24.3 ghcr.io/github/gh-aw-firewall/api-proxy:0.24.3 ghcr.io/github/gh-aw-firewall/squid:0.24.3 ghcr.io/github/gh-aw-mcpg:v0.1.19 ghcr.io/github/github-mcp-server:v0.32.0 node:lts-alpine + - name: Write Safe Outputs Config + run: | + mkdir -p ${RUNNER_TEMP}/gh-aw/safeoutputs + mkdir -p /tmp/gh-aw/safeoutputs + mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs + cat > ${RUNNER_TEMP}/gh-aw/safeoutputs/config.json << 'GH_AW_SAFE_OUTPUTS_CONFIG_EOF' + {"add_comment":{"max":1,"target":"*"},"missing_data":{},"missing_tool":{},"noop":{"max":1}} + GH_AW_SAFE_OUTPUTS_CONFIG_EOF + - name: Write Safe Outputs Tools + run: | + cat > ${RUNNER_TEMP}/gh-aw/safeoutputs/tools_meta.json << 'GH_AW_SAFE_OUTPUTS_TOOLS_META_EOF' + { + "description_suffixes": { + "add_comment": " CONSTRAINTS: Maximum 1 comment(s) can be added. Target: *." + }, + "repo_params": {}, + "dynamic_tools": [] + } + GH_AW_SAFE_OUTPUTS_TOOLS_META_EOF + cat > ${RUNNER_TEMP}/gh-aw/safeoutputs/validation.json << 'GH_AW_SAFE_OUTPUTS_VALIDATION_EOF' + { + "add_comment": { + "defaultMax": 1, + "fields": { + "body": { + "required": true, + "type": "string", + "sanitize": true, + "maxLength": 65000 + }, + "item_number": { + "issueOrPRNumber": true + }, + "repo": { + "type": "string", + "maxLength": 256 + } + } + }, + "missing_data": { + "defaultMax": 20, + "fields": { + "alternatives": { + "type": "string", + "sanitize": true, + "maxLength": 256 + }, + "context": { + "type": "string", + "sanitize": true, + "maxLength": 256 + }, + "data_type": { + "type": "string", + "sanitize": true, + "maxLength": 128 + }, + "reason": { + "type": "string", + "sanitize": true, + "maxLength": 256 + } + } + }, + "missing_tool": { + "defaultMax": 20, + "fields": { + "alternatives": { + "type": "string", + "sanitize": true, + "maxLength": 512 + }, + "reason": { + "required": true, + "type": "string", + "sanitize": true, + "maxLength": 256 + }, + "tool": { + "type": "string", + "sanitize": true, + "maxLength": 128 + } + } + }, + "noop": { + "defaultMax": 1, + "fields": { + "message": { + "required": true, + "type": "string", + "sanitize": true, + "maxLength": 65000 + } + } + } + } + GH_AW_SAFE_OUTPUTS_VALIDATION_EOF + node ${RUNNER_TEMP}/gh-aw/actions/generate_safe_outputs_tools.cjs + - name: Generate Safe Outputs MCP Server Config + id: safe-outputs-config + run: | + # Generate a secure random API key (360 bits of entropy, 40+ chars) + # Mask immediately to prevent timing vulnerabilities + API_KEY=$(openssl rand -base64 45 | tr -d '/+=') + echo "::add-mask::${API_KEY}" + + PORT=3001 + + # Set outputs for next steps + { + echo "safe_outputs_api_key=${API_KEY}" + echo "safe_outputs_port=${PORT}" + } >> "$GITHUB_OUTPUT" + + echo "Safe Outputs MCP server will run on port ${PORT}" + + - name: Start Safe Outputs MCP HTTP Server + id: safe-outputs-start + env: + DEBUG: '*' + GH_AW_SAFE_OUTPUTS_PORT: ${{ steps.safe-outputs-config.outputs.safe_outputs_port }} + GH_AW_SAFE_OUTPUTS_API_KEY: ${{ steps.safe-outputs-config.outputs.safe_outputs_api_key }} + GH_AW_SAFE_OUTPUTS_TOOLS_PATH: ${{ runner.temp }}/gh-aw/safeoutputs/tools.json + GH_AW_SAFE_OUTPUTS_CONFIG_PATH: ${{ runner.temp }}/gh-aw/safeoutputs/config.json + GH_AW_MCP_LOG_DIR: /tmp/gh-aw/mcp-logs/safeoutputs + run: | + # Environment variables are set above to prevent template injection + export DEBUG + export GH_AW_SAFE_OUTPUTS_PORT + export GH_AW_SAFE_OUTPUTS_API_KEY + export GH_AW_SAFE_OUTPUTS_TOOLS_PATH + export GH_AW_SAFE_OUTPUTS_CONFIG_PATH + export GH_AW_MCP_LOG_DIR + + bash ${RUNNER_TEMP}/gh-aw/actions/start_safe_outputs_server.sh + + - name: Start MCP Gateway + id: start-mcp-gateway + env: + GH_AW_SAFE_OUTPUTS: ${{ env.GH_AW_SAFE_OUTPUTS }} + GH_AW_SAFE_OUTPUTS_API_KEY: ${{ steps.safe-outputs-start.outputs.api_key }} + GH_AW_SAFE_OUTPUTS_PORT: ${{ steps.safe-outputs-start.outputs.port }} + GITHUB_MCP_GUARD_MIN_INTEGRITY: ${{ steps.determine-automatic-lockdown.outputs.min_integrity }} + GITHUB_MCP_GUARD_REPOS: ${{ steps.determine-automatic-lockdown.outputs.repos }} + GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} + run: | + set -eo pipefail + mkdir -p /tmp/gh-aw/mcp-config + + # Export gateway environment variables for MCP config and gateway script + export MCP_GATEWAY_PORT="80" + export MCP_GATEWAY_DOMAIN="host.docker.internal" + MCP_GATEWAY_API_KEY=$(openssl rand -base64 45 | tr -d '/+=') + echo "::add-mask::${MCP_GATEWAY_API_KEY}" + export MCP_GATEWAY_API_KEY + export MCP_GATEWAY_PAYLOAD_DIR="/tmp/gh-aw/mcp-payloads" + mkdir -p "${MCP_GATEWAY_PAYLOAD_DIR}" + export MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD="524288" + export DEBUG="*" + + export GH_AW_ENGINE="copilot" + export MCP_GATEWAY_DOCKER_COMMAND='docker run -i --rm --network host -v /var/run/docker.sock:/var/run/docker.sock -e MCP_GATEWAY_PORT -e MCP_GATEWAY_DOMAIN -e MCP_GATEWAY_API_KEY -e MCP_GATEWAY_PAYLOAD_DIR -e MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD -e DEBUG -e MCP_GATEWAY_LOG_DIR -e GH_AW_MCP_LOG_DIR -e GH_AW_SAFE_OUTPUTS -e GH_AW_SAFE_OUTPUTS_CONFIG_PATH -e GH_AW_SAFE_OUTPUTS_TOOLS_PATH -e GH_AW_ASSETS_BRANCH -e GH_AW_ASSETS_MAX_SIZE_KB -e GH_AW_ASSETS_ALLOWED_EXTS -e DEFAULT_BRANCH -e GITHUB_MCP_SERVER_TOKEN -e GITHUB_MCP_GUARD_MIN_INTEGRITY -e GITHUB_MCP_GUARD_REPOS -e GITHUB_REPOSITORY -e GITHUB_SERVER_URL -e GITHUB_SHA -e GITHUB_WORKSPACE -e GITHUB_TOKEN -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RUN_ATTEMPT -e GITHUB_JOB -e GITHUB_ACTION -e GITHUB_EVENT_NAME -e GITHUB_EVENT_PATH -e GITHUB_ACTOR -e GITHUB_ACTOR_ID -e GITHUB_TRIGGERING_ACTOR -e GITHUB_WORKFLOW -e GITHUB_WORKFLOW_REF -e GITHUB_WORKFLOW_SHA -e GITHUB_REF -e GITHUB_REF_NAME -e GITHUB_REF_TYPE -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -e GH_AW_SAFE_OUTPUTS_PORT -e GH_AW_SAFE_OUTPUTS_API_KEY -v /tmp/gh-aw/mcp-payloads:/tmp/gh-aw/mcp-payloads:rw -v /opt:/opt:ro -v /tmp:/tmp:rw -v '"${GITHUB_WORKSPACE}"':'"${GITHUB_WORKSPACE}"':rw ghcr.io/github/gh-aw-mcpg:v0.1.19' + + mkdir -p /home/runner/.copilot + cat << GH_AW_MCP_CONFIG_EOF | bash ${RUNNER_TEMP}/gh-aw/actions/start_mcp_gateway.sh + { + "mcpServers": { + "github": { + "type": "stdio", + "container": "ghcr.io/github/github-mcp-server:v0.32.0", + "env": { + "GITHUB_HOST": "\${GITHUB_SERVER_URL}", + "GITHUB_PERSONAL_ACCESS_TOKEN": "\${GITHUB_MCP_SERVER_TOKEN}", + "GITHUB_READ_ONLY": "1", + "GITHUB_TOOLSETS": "context,repos,issues,pull_requests" + }, + "guard-policies": { + "allow-only": { + "min-integrity": "$GITHUB_MCP_GUARD_MIN_INTEGRITY", + "repos": "$GITHUB_MCP_GUARD_REPOS" + } + } + }, + "safeoutputs": { + "type": "http", + "url": "http://host.docker.internal:$GH_AW_SAFE_OUTPUTS_PORT", + "headers": { + "Authorization": "\${GH_AW_SAFE_OUTPUTS_API_KEY}" + }, + "guard-policies": { + "write-sink": { + "accept": [ + "*" + ] + } + } + } + }, + "gateway": { + "port": $MCP_GATEWAY_PORT, + "domain": "${MCP_GATEWAY_DOMAIN}", + "apiKey": "${MCP_GATEWAY_API_KEY}", + "payloadDir": "${MCP_GATEWAY_PAYLOAD_DIR}" + } + } + GH_AW_MCP_CONFIG_EOF + - name: Download activation artifact + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: activation + path: /tmp/gh-aw + - name: Clean git credentials + continue-on-error: true + run: bash ${RUNNER_TEMP}/gh-aw/actions/clean_git_credentials.sh + - name: Execute GitHub Copilot CLI + id: agentic_execution + # Copilot CLI tool arguments (sorted): + timeout-minutes: 15 + run: | + set -o pipefail + touch /tmp/gh-aw/agent-step-summary.md + # shellcheck disable=SC1003 + sudo -E awf --env-all --container-workdir "${GITHUB_WORKSPACE}" --mount "${RUNNER_TEMP}/gh-aw:${RUNNER_TEMP}/gh-aw:ro" --mount "${RUNNER_TEMP}/gh-aw:/host${RUNNER_TEMP}/gh-aw:ro" --allow-domains "api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.github.com,api.githubcopilot.com,api.individual.githubcopilot.com,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,github.com,host.docker.internal,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,ppa.launchpad.net,raw.githubusercontent.com,registry.npmjs.org,s.symcb.com,s.symcd.com,security.ubuntu.com,telemetry.enterprise.githubcopilot.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com,www.googleapis.com" --log-level info --proxy-logs-dir /tmp/gh-aw/sandbox/firewall/logs --enable-host-access --image-tag 0.24.3 --skip-pull --enable-api-proxy \ + -- /bin/bash -c '/usr/local/bin/copilot --add-dir /tmp/gh-aw/ --log-level all --log-dir /tmp/gh-aw/sandbox/agent/logs/ --add-dir "${GITHUB_WORKSPACE}" --disable-builtin-mcps --allow-all-tools --allow-all-paths --prompt "$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"' 2>&1 | tee -a /tmp/gh-aw/agent-stdio.log + env: + COPILOT_AGENT_RUNNER_TYPE: STANDALONE + COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }} + COPILOT_MODEL: claude-sonnet-4.6 + GH_AW_MCP_CONFIG: /home/runner/.copilot/mcp-config.json + GH_AW_PHASE: agent + GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt + GH_AW_SAFE_OUTPUTS: ${{ env.GH_AW_SAFE_OUTPUTS }} + GH_AW_VERSION: v0.62.2 + GITHUB_API_URL: ${{ github.api_url }} + GITHUB_AW: true + GITHUB_HEAD_REF: ${{ github.head_ref }} + GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} + GITHUB_REF_NAME: ${{ github.ref_name }} + GITHUB_SERVER_URL: ${{ github.server_url }} + GITHUB_STEP_SUMMARY: /tmp/gh-aw/agent-step-summary.md + GITHUB_WORKSPACE: ${{ github.workspace }} + GIT_AUTHOR_EMAIL: github-actions[bot]@users.noreply.github.com + GIT_AUTHOR_NAME: github-actions[bot] + GIT_COMMITTER_EMAIL: github-actions[bot]@users.noreply.github.com + GIT_COMMITTER_NAME: github-actions[bot] + XDG_CONFIG_HOME: /home/runner + - name: Detect inference access error + id: detect-inference-error + if: always() + continue-on-error: true + run: bash ${RUNNER_TEMP}/gh-aw/actions/detect_inference_access_error.sh + - name: Configure Git credentials + env: + REPO_NAME: ${{ github.repository }} + SERVER_URL: ${{ github.server_url }} + run: | + git config --global user.email "github-actions[bot]@users.noreply.github.com" + git config --global user.name "github-actions[bot]" + git config --global am.keepcr true + # Re-authenticate git with GitHub token + SERVER_URL_STRIPPED="${SERVER_URL#https://}" + git remote set-url origin "https://x-access-token:${{ github.token }}@${SERVER_URL_STRIPPED}/${REPO_NAME}.git" + echo "Git configured with standard GitHub Actions identity" + - name: Copy Copilot session state files to logs + if: always() + continue-on-error: true + run: | + # Copy Copilot session state files to logs folder for artifact collection + # This ensures they are in /tmp/gh-aw/ where secret redaction can scan them + SESSION_STATE_DIR="$HOME/.copilot/session-state" + LOGS_DIR="/tmp/gh-aw/sandbox/agent/logs" + + if [ -d "$SESSION_STATE_DIR" ]; then + echo "Copying Copilot session state files from $SESSION_STATE_DIR to $LOGS_DIR" + mkdir -p "$LOGS_DIR" + cp -v "$SESSION_STATE_DIR"/*.jsonl "$LOGS_DIR/" 2>/dev/null || true + echo "Session state files copied successfully" + else + echo "No session-state directory found at $SESSION_STATE_DIR" + fi + - name: Stop MCP Gateway + if: always() + continue-on-error: true + env: + MCP_GATEWAY_PORT: ${{ steps.start-mcp-gateway.outputs.gateway-port }} + MCP_GATEWAY_API_KEY: ${{ steps.start-mcp-gateway.outputs.gateway-api-key }} + GATEWAY_PID: ${{ steps.start-mcp-gateway.outputs.gateway-pid }} + run: | + bash ${RUNNER_TEMP}/gh-aw/actions/stop_mcp_gateway.sh "$GATEWAY_PID" + - name: Redact secrets in logs + if: always() + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/redact_secrets.cjs'); + await main(); + env: + GH_AW_SECRET_NAMES: 'COPILOT_GITHUB_TOKEN,GH_AW_GITHUB_MCP_SERVER_TOKEN,GH_AW_GITHUB_TOKEN,GITHUB_TOKEN' + SECRET_COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }} + SECRET_GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }} + SECRET_GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }} + SECRET_GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Append agent step summary + if: always() + run: bash ${RUNNER_TEMP}/gh-aw/actions/append_agent_step_summary.sh + - name: Copy Safe Outputs + if: always() + run: | + mkdir -p /tmp/gh-aw + cp "$GH_AW_SAFE_OUTPUTS" /tmp/gh-aw/safeoutputs.jsonl 2>/dev/null || true + - name: Ingest agent output + id: collect_output + if: always() + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_SAFE_OUTPUTS: ${{ env.GH_AW_SAFE_OUTPUTS }} + GH_AW_ALLOWED_DOMAINS: "api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.github.com,api.githubcopilot.com,api.individual.githubcopilot.com,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,github.com,host.docker.internal,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,ppa.launchpad.net,raw.githubusercontent.com,registry.npmjs.org,s.symcb.com,s.symcd.com,security.ubuntu.com,telemetry.enterprise.githubcopilot.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com,www.googleapis.com" + GITHUB_SERVER_URL: ${{ github.server_url }} + GITHUB_API_URL: ${{ github.api_url }} + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/collect_ndjson_output.cjs'); + await main(); + - name: Parse agent logs for step summary + if: always() + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_AGENT_OUTPUT: /tmp/gh-aw/sandbox/agent/logs/ + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/parse_copilot_log.cjs'); + await main(); + - name: Parse MCP Gateway logs for step summary + if: always() + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/parse_mcp_gateway_log.cjs'); + await main(); + - name: Print firewall logs + if: always() + continue-on-error: true + env: + AWF_LOGS_DIR: /tmp/gh-aw/sandbox/firewall/logs + run: | + # Fix permissions on firewall logs so they can be uploaded as artifacts + # AWF runs with sudo, creating files owned by root + sudo chmod -R a+r /tmp/gh-aw/sandbox/firewall/logs 2>/dev/null || true + # Only run awf logs summary if awf command exists (it may not be installed if workflow failed before install step) + if command -v awf &> /dev/null; then + awf logs summary | tee -a "$GITHUB_STEP_SUMMARY" + else + echo 'AWF binary not installed, skipping firewall log summary' + fi + - name: Upload agent artifacts + if: always() + continue-on-error: true + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + with: + name: agent + path: | + /tmp/gh-aw/aw-prompts/prompt.txt + /tmp/gh-aw/sandbox/agent/logs/ + /tmp/gh-aw/redacted-urls.log + /tmp/gh-aw/mcp-logs/ + /tmp/gh-aw/sandbox/firewall/logs/ + /tmp/gh-aw/agent-stdio.log + /tmp/gh-aw/agent/ + /tmp/gh-aw/safeoutputs.jsonl + /tmp/gh-aw/agent_output.json + if-no-files-found: ignore + # --- Threat Detection (inline) --- + - name: Check if detection needed + id: detection_guard + if: always() + env: + OUTPUT_TYPES: ${{ steps.collect_output.outputs.output_types }} + HAS_PATCH: ${{ steps.collect_output.outputs.has_patch }} + run: | + if [[ -n "$OUTPUT_TYPES" || "$HAS_PATCH" == "true" ]]; then + echo "run_detection=true" >> "$GITHUB_OUTPUT" + echo "Detection will run: output_types=$OUTPUT_TYPES, has_patch=$HAS_PATCH" + else + echo "run_detection=false" >> "$GITHUB_OUTPUT" + echo "Detection skipped: no agent outputs or patches to analyze" + fi + - name: Clear MCP configuration for detection + if: always() && steps.detection_guard.outputs.run_detection == 'true' + run: | + rm -f /tmp/gh-aw/mcp-config/mcp-servers.json + rm -f /home/runner/.copilot/mcp-config.json + rm -f "$GITHUB_WORKSPACE/.gemini/settings.json" + - name: Prepare threat detection files + if: always() && steps.detection_guard.outputs.run_detection == 'true' + run: | + mkdir -p /tmp/gh-aw/threat-detection/aw-prompts + cp /tmp/gh-aw/aw-prompts/prompt.txt /tmp/gh-aw/threat-detection/aw-prompts/prompt.txt 2>/dev/null || true + cp /tmp/gh-aw/agent_output.json /tmp/gh-aw/threat-detection/agent_output.json 2>/dev/null || true + for f in /tmp/gh-aw/aw-*.patch; do + [ -f "$f" ] && cp "$f" /tmp/gh-aw/threat-detection/ 2>/dev/null || true + done + echo "Prepared threat detection files:" + ls -la /tmp/gh-aw/threat-detection/ 2>/dev/null || true + - name: Setup threat detection + if: always() && steps.detection_guard.outputs.run_detection == 'true' + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + WORKFLOW_NAME: "Evaluate PR Tests" + WORKFLOW_DESCRIPTION: "Evaluates test quality, coverage, and appropriateness on PRs that add or modify tests" + HAS_PATCH: ${{ steps.collect_output.outputs.has_patch }} + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/setup_threat_detection.cjs'); + await main(); + - name: Ensure threat-detection directory and log + if: always() && steps.detection_guard.outputs.run_detection == 'true' + run: | + mkdir -p /tmp/gh-aw/threat-detection + touch /tmp/gh-aw/threat-detection/detection.log + - name: Execute GitHub Copilot CLI + if: always() && steps.detection_guard.outputs.run_detection == 'true' + id: detection_agentic_execution + # Copilot CLI tool arguments (sorted): + # --allow-tool shell(cat) + # --allow-tool shell(grep) + # --allow-tool shell(head) + # --allow-tool shell(jq) + # --allow-tool shell(ls) + # --allow-tool shell(tail) + # --allow-tool shell(wc) + timeout-minutes: 20 + run: | + set -o pipefail + touch /tmp/gh-aw/agent-step-summary.md + # shellcheck disable=SC1003 + sudo -E awf --env-all --container-workdir "${GITHUB_WORKSPACE}" --mount "${RUNNER_TEMP}/gh-aw:${RUNNER_TEMP}/gh-aw:ro" --mount "${RUNNER_TEMP}/gh-aw:/host${RUNNER_TEMP}/gh-aw:ro" --allow-domains "api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.github.com,api.githubcopilot.com,api.individual.githubcopilot.com,github.com,host.docker.internal,raw.githubusercontent.com,registry.npmjs.org,telemetry.enterprise.githubcopilot.com" --log-level info --proxy-logs-dir /tmp/gh-aw/sandbox/firewall/logs --enable-host-access --image-tag 0.24.3 --skip-pull --enable-api-proxy \ + -- /bin/bash -c '/usr/local/bin/copilot --add-dir /tmp/gh-aw/ --log-level all --log-dir /tmp/gh-aw/sandbox/agent/logs/ --add-dir "${GITHUB_WORKSPACE}" --disable-builtin-mcps --allow-tool '\''shell(cat)'\'' --allow-tool '\''shell(grep)'\'' --allow-tool '\''shell(head)'\'' --allow-tool '\''shell(jq)'\'' --allow-tool '\''shell(ls)'\'' --allow-tool '\''shell(tail)'\'' --allow-tool '\''shell(wc)'\'' --prompt "$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"' 2>&1 | tee -a /tmp/gh-aw/threat-detection/detection.log + env: + COPILOT_AGENT_RUNNER_TYPE: STANDALONE + COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }} + COPILOT_MODEL: claude-sonnet-4.6 + GH_AW_PHASE: detection + GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt + GH_AW_VERSION: v0.62.2 + GITHUB_API_URL: ${{ github.api_url }} + GITHUB_AW: true + GITHUB_HEAD_REF: ${{ github.head_ref }} + GITHUB_REF_NAME: ${{ github.ref_name }} + GITHUB_SERVER_URL: ${{ github.server_url }} + GITHUB_STEP_SUMMARY: /tmp/gh-aw/agent-step-summary.md + GITHUB_WORKSPACE: ${{ github.workspace }} + GIT_AUTHOR_EMAIL: github-actions[bot]@users.noreply.github.com + GIT_AUTHOR_NAME: github-actions[bot] + GIT_COMMITTER_EMAIL: github-actions[bot]@users.noreply.github.com + GIT_COMMITTER_NAME: github-actions[bot] + XDG_CONFIG_HOME: /home/runner + - name: Parse threat detection results + id: parse_detection_results + if: always() && steps.detection_guard.outputs.run_detection == 'true' + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + with: + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/parse_threat_detection_results.cjs'); + await main(); + - name: Upload threat detection log + if: always() && steps.detection_guard.outputs.run_detection == 'true' + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + with: + name: detection + path: /tmp/gh-aw/threat-detection/detection.log + if-no-files-found: ignore + - name: Set detection conclusion + id: detection_conclusion + if: always() + env: + RUN_DETECTION: ${{ steps.detection_guard.outputs.run_detection }} + DETECTION_SUCCESS: ${{ steps.parse_detection_results.outputs.success }} + run: | + if [[ "$RUN_DETECTION" != "true" ]]; then + echo "conclusion=skipped" >> "$GITHUB_OUTPUT" + echo "success=true" >> "$GITHUB_OUTPUT" + echo "Detection was not needed, marking as skipped" + elif [[ "$DETECTION_SUCCESS" == "true" ]]; then + echo "conclusion=success" >> "$GITHUB_OUTPUT" + echo "success=true" >> "$GITHUB_OUTPUT" + echo "Detection passed successfully" + else + echo "conclusion=failure" >> "$GITHUB_OUTPUT" + echo "success=false" >> "$GITHUB_OUTPUT" + echo "Detection found issues" + fi + + conclusion: + needs: + - activation + - agent + - safe_outputs + if: (always()) && ((needs.agent.result != 'skipped') || (needs.activation.outputs.lockdown_check_failed == 'true')) + runs-on: ubuntu-slim + permissions: + contents: read + discussions: write + issues: write + pull-requests: write + concurrency: + group: "gh-aw-conclusion-copilot-evaluate-tests" + cancel-in-progress: false + outputs: + noop_message: ${{ steps.noop.outputs.noop_message }} + tools_reported: ${{ steps.missing_tool.outputs.tools_reported }} + total_count: ${{ steps.missing_tool.outputs.total_count }} + steps: + - name: Setup Scripts + uses: github/gh-aw-actions/setup@20045bbd5ad2632b9809856c389708eab1bd16ef # v0.62.2 + with: + destination: ${{ runner.temp }}/gh-aw/actions + - name: Download agent output artifact + id: download-agent-output + continue-on-error: true + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: agent + path: /tmp/gh-aw/ + - name: Setup agent output environment variable + if: steps.download-agent-output.outcome == 'success' + run: | + mkdir -p /tmp/gh-aw/ + find "/tmp/gh-aw/" -type f -print + echo "GH_AW_AGENT_OUTPUT=/tmp/gh-aw/agent_output.json" >> "$GITHUB_ENV" + - name: Process No-Op Messages + id: noop + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_AGENT_OUTPUT: ${{ env.GH_AW_AGENT_OUTPUT }} + GH_AW_NOOP_MAX: "1" + GH_AW_WORKFLOW_NAME: "Evaluate PR Tests" + with: + github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/noop.cjs'); + await main(); + - name: Record Missing Tool + id: missing_tool + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_AGENT_OUTPUT: ${{ env.GH_AW_AGENT_OUTPUT }} + GH_AW_WORKFLOW_NAME: "Evaluate PR Tests" + with: + github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/missing_tool.cjs'); + await main(); + - name: Handle Agent Failure + id: handle_agent_failure + if: always() + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_AGENT_OUTPUT: ${{ env.GH_AW_AGENT_OUTPUT }} + GH_AW_WORKFLOW_NAME: "Evaluate PR Tests" + GH_AW_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + GH_AW_AGENT_CONCLUSION: ${{ needs.agent.result }} + GH_AW_WORKFLOW_ID: "copilot-evaluate-tests" + GH_AW_SECRET_VERIFICATION_RESULT: ${{ needs.activation.outputs.secret_verification_result }} + GH_AW_CHECKOUT_PR_SUCCESS: ${{ needs.agent.outputs.checkout_pr_success }} + GH_AW_INFERENCE_ACCESS_ERROR: ${{ needs.agent.outputs.inference_access_error }} + GH_AW_LOCKDOWN_CHECK_FAILED: ${{ needs.activation.outputs.lockdown_check_failed }} + GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Test evaluation by [{workflow_name}]({run_url})*\",\"runStarted\":\"🔬 Evaluating tests on this PR… [{workflow_name}]({run_url})\",\"runSuccess\":\"✅ Test evaluation complete! [{workflow_name}]({run_url})\",\"runFailure\":\"❌ Test evaluation failed. [{workflow_name}]({run_url}) {status}\"}" + GH_AW_GROUP_REPORTS: "false" + GH_AW_FAILURE_REPORT_AS_ISSUE: "true" + GH_AW_TIMEOUT_MINUTES: "15" + with: + github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/handle_agent_failure.cjs'); + await main(); + - name: Handle No-Op Message + id: handle_noop_message + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_AGENT_OUTPUT: ${{ env.GH_AW_AGENT_OUTPUT }} + GH_AW_WORKFLOW_NAME: "Evaluate PR Tests" + GH_AW_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + GH_AW_AGENT_CONCLUSION: ${{ needs.agent.result }} + GH_AW_NOOP_MESSAGE: ${{ steps.noop.outputs.noop_message }} + GH_AW_NOOP_REPORT_AS_ISSUE: "true" + with: + github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/handle_noop_message.cjs'); + await main(); + + pre_activation: + if: > + ((github.event_name == 'pull_request' && github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || (github.event_name == 'issue_comment' && + github.event.issue.pull_request && + startsWith(github.event.comment.body, '/evaluate-tests'))) && ((github.event_name != 'pull_request') || (github.event.pull_request.head.repo.id == github.repository_id)) + runs-on: ubuntu-slim + outputs: + activated: ${{ steps.check_membership.outputs.is_team_member == 'true' }} + matched_command: '' + steps: + - name: Setup Scripts + uses: github/gh-aw-actions/setup@20045bbd5ad2632b9809856c389708eab1bd16ef # v0.62.2 + with: + destination: ${{ runner.temp }}/gh-aw/actions + - name: Check team membership for workflow + id: check_membership + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_REQUIRED_ROLES: admin,maintainer,write + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/check_membership.cjs'); + await main(); + + safe_outputs: + needs: agent + if: ((!cancelled()) && (needs.agent.result != 'skipped')) && (needs.agent.outputs.detection_success == 'true') + runs-on: ubuntu-slim + permissions: + contents: read + discussions: write + issues: write + pull-requests: write + timeout-minutes: 15 + env: + GH_AW_CALLER_WORKFLOW_ID: "${{ github.repository }}/copilot-evaluate-tests" + GH_AW_ENGINE_ID: "copilot" + GH_AW_ENGINE_MODEL: "claude-sonnet-4.6" + GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Test evaluation by [{workflow_name}]({run_url})*\",\"runStarted\":\"🔬 Evaluating tests on this PR… [{workflow_name}]({run_url})\",\"runSuccess\":\"✅ Test evaluation complete! [{workflow_name}]({run_url})\",\"runFailure\":\"❌ Test evaluation failed. [{workflow_name}]({run_url}) {status}\"}" + GH_AW_WORKFLOW_ID: "copilot-evaluate-tests" + GH_AW_WORKFLOW_NAME: "Evaluate PR Tests" + outputs: + code_push_failure_count: ${{ steps.process_safe_outputs.outputs.code_push_failure_count }} + code_push_failure_errors: ${{ steps.process_safe_outputs.outputs.code_push_failure_errors }} + comment_id: ${{ steps.process_safe_outputs.outputs.comment_id }} + comment_url: ${{ steps.process_safe_outputs.outputs.comment_url }} + create_discussion_error_count: ${{ steps.process_safe_outputs.outputs.create_discussion_error_count }} + create_discussion_errors: ${{ steps.process_safe_outputs.outputs.create_discussion_errors }} + process_safe_outputs_processed_count: ${{ steps.process_safe_outputs.outputs.processed_count }} + process_safe_outputs_temporary_id_map: ${{ steps.process_safe_outputs.outputs.temporary_id_map }} + steps: + - name: Setup Scripts + uses: github/gh-aw-actions/setup@20045bbd5ad2632b9809856c389708eab1bd16ef # v0.62.2 + with: + destination: ${{ runner.temp }}/gh-aw/actions + - name: Download agent output artifact + id: download-agent-output + continue-on-error: true + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: agent + path: /tmp/gh-aw/ + - name: Setup agent output environment variable + if: steps.download-agent-output.outcome == 'success' + run: | + mkdir -p /tmp/gh-aw/ + find "/tmp/gh-aw/" -type f -print + echo "GH_AW_AGENT_OUTPUT=/tmp/gh-aw/agent_output.json" >> "$GITHUB_ENV" + - name: Configure GH_HOST for enterprise compatibility + shell: bash + run: | + # Derive GH_HOST from GITHUB_SERVER_URL so the gh CLI targets the correct + # GitHub instance (GHES/GHEC). On github.com this is a harmless no-op. + GH_HOST="${GITHUB_SERVER_URL#https://}" + GH_HOST="${GH_HOST#http://}" + echo "GH_HOST=${GH_HOST}" >> "$GITHUB_ENV" + - name: Process Safe Outputs + id: process_safe_outputs + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 + env: + GH_AW_AGENT_OUTPUT: ${{ env.GH_AW_AGENT_OUTPUT }} + GH_AW_ALLOWED_DOMAINS: "api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.github.com,api.githubcopilot.com,api.individual.githubcopilot.com,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,github.com,host.docker.internal,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,ppa.launchpad.net,raw.githubusercontent.com,registry.npmjs.org,s.symcb.com,s.symcd.com,security.ubuntu.com,telemetry.enterprise.githubcopilot.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com,www.googleapis.com" + GITHUB_SERVER_URL: ${{ github.server_url }} + GITHUB_API_URL: ${{ github.api_url }} + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"add_comment\":{\"max\":1,\"target\":\"*\"},\"missing_data\":{},\"missing_tool\":{},\"noop\":{\"max\":1,\"report-as-issue\":\"true\"}}" + with: + github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} + script: | + const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs'); + setupGlobals(core, github, context, exec, io); + const { main } = require('${{ runner.temp }}/gh-aw/actions/safe_output_handler_manager.cjs'); + await main(); + - name: Upload safe output items + if: always() + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + with: + name: safe-output-items + path: /tmp/gh-aw/safe-output-items.jsonl + if-no-files-found: ignore + diff --git a/.github/workflows/copilot-evaluate-tests.md b/.github/workflows/copilot-evaluate-tests.md new file mode 100644 index 000000000000..c84f83f8855e --- /dev/null +++ b/.github/workflows/copilot-evaluate-tests.md @@ -0,0 +1,139 @@ +--- +description: Evaluates test quality, coverage, and appropriateness on PRs that add or modify tests +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + paths: + - 'src/**/tests/**' + - 'src/**/test/**' + issue_comment: + types: [created] + workflow_dispatch: + inputs: + pr_number: + description: 'PR number to evaluate' + required: true + type: number + +if: >- + (github.event_name == 'pull_request' && github.event.pull_request.draft == false) || + github.event_name == 'workflow_dispatch' || + (github.event_name == 'issue_comment' && + github.event.issue.pull_request && + startsWith(github.event.comment.body, '/evaluate-tests')) + +permissions: + contents: read + issues: read + pull-requests: read + +engine: + id: copilot + model: claude-sonnet-4.6 + +safe-outputs: + add-comment: + max: 1 + target: "*" + noop: + messages: + footer: "> 🧪 *Test evaluation by [{workflow_name}]({run_url})*" + run-started: "🔬 Evaluating tests on this PR… [{workflow_name}]({run_url})" + run-success: "✅ Test evaluation complete! [{workflow_name}]({run_url})" + run-failure: "❌ Test evaluation failed. [{workflow_name}]({run_url}) {status}" + +tools: + github: + toolsets: [default] + +network: defaults + +concurrency: + group: "evaluate-pr-tests-${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number || github.run_id }}" + cancel-in-progress: true + +timeout-minutes: 15 + +steps: + - name: Gate — skip if no test source files in diff + if: github.event_name == 'pull_request' + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + TEST_FILES=$(gh pr diff "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --name-only \ + | grep -E '\.(cs|xaml)$' \ + | grep -iE '(tests?/|TestCases|UnitTests|DeviceTests)' \ + || true) + if [ -z "$TEST_FILES" ]; then + echo "⏭️ No test source files (.cs/.xaml) found in PR diff. Skipping evaluation." + exit 1 + fi + echo "✅ Found test files to evaluate:" + echo "$TEST_FILES" | head -20 + + - name: Checkout PR and restore agent infrastructure + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} + run: pwsh .github/scripts/Checkout-GhAwPr.ps1 +--- + +# Evaluate PR Tests + +Invoke the **evaluate-pr-tests** skill: read and follow `.github/skills/evaluate-pr-tests/SKILL.md`. + +## Context + +- **Repository**: ${{ github.repository }} +- **PR Number**: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} + +The PR branch has been checked out for you. All files from the PR are available locally. + +## Pre-flight check + +Before starting, verify the skill file exists: + +```bash +test -f .github/skills/evaluate-pr-tests/SKILL.md +``` + +If the file is **missing**, the fork PR branch is likely not rebased on the latest `main`. Post a comment using `add_comment`: + +```markdown +## 🧪 PR Test Evaluation + +❌ **Cannot evaluate**: this PR's branch does not include the evaluate-pr-tests skill (`.github/skills/evaluate-pr-tests/SKILL.md` is missing). + +**Fix**: rebase your fork on the latest `main` branch, or use the **workflow_dispatch** trigger (Actions tab → "Evaluate PR Tests" → "Run workflow" → enter PR number) which handles this automatically. +``` + +Then stop — do not proceed with the evaluation. + +## Running the skill + +1. Use `gh pr view ` to fetch PR metadata (title, body, labels, base branch). If `gh` CLI is unavailable, use the GitHub MCP tools instead. +2. Run `pwsh .github/skills/evaluate-pr-tests/scripts/Gather-TestContext.ps1` to gather automated context +3. Read the context report and the actual changed files, then evaluate per SKILL.md criteria +4. Post results using `add_comment` with `item_number` set to the PR number + +## Posting Results + +Call `add_comment` with `item_number` set to the PR number. Wrap the report in a collapsible `
` block: + +```markdown +## 🧪 PR Test Evaluation + +**Overall Verdict:** [✅ Tests are adequate | ⚠️ Tests need improvement | ❌ Tests are insufficient] + +[1-2 sentence summary] + +> 👍 / 👎 — Was this evaluation helpful? React to let us know! + +
+📊 Expand Full Evaluation + +[Full report from SKILL.md] + +
+``` From 720a9d4a935413c89e0748198549cf92694ee051 Mon Sep 17 00:00:00 2001 From: Shane Neuville <5375137+PureWeen@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:05:33 -0500 Subject: [PATCH 02/12] Allow fork PRs to auto-trigger evaluate-pr-tests workflow (#34655) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Enables the copilot-evaluate-tests gh-aw workflow to run on fork PRs by adding `forks: ["*"]` to the `pull_request` trigger and removing the fork guard from `Checkout-GhAwPr.ps1`. ## Changes 1. **copilot-evaluate-tests.md**: Added `forks: ["*"]` to opt out of gh-aw auto-injected fork activation guard. Scoped `Checkout-GhAwPr.ps1` step to `workflow_dispatch` only (redundant for other triggers since platform handles checkout). 2. **copilot-evaluate-tests.lock.yml**: Recompiled via `gh aw compile` — fork guard removed from activation `if:` conditions. 3. **Checkout-GhAwPr.ps1**: Removed the `isCrossRepository` fork guard. Updated header docs and restore comments to accurately describe behavior for all trigger×fork combinations (including corrected step ordering). 4. **gh-aw-workflows.instructions.md**: Updated all stale references to the removed fork guard. Documented `forks: ["*"]` opt-in, clarified residual risk model for fork PRs, and updated troubleshooting table. ## Security Model Fork PRs are safe because: - Agent runs in **sandboxed container** with all credentials scrubbed - Output limited to **1 comment** via `safe-outputs: add-comment: max: 1` - Agent **prompt comes from base branch** (`runtime-import`) — forks cannot alter instructions - Pre-flight check catches missing `SKILL.md` if fork isn't rebased on `main` - No workspace code is executed with `GITHUB_TOKEN` (checkout without execution) ## Testing - ✅ `workflow_dispatch` tested against fork PR #34621 - ✅ Lock.yml statically verified — fork guard removed from `if:` conditions - ⏳ `pull_request` trigger on fork PRs can only be verified post-merge (GitHub Actions reads lock.yml from default branch) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../gh-aw-workflows.instructions.md | 35 +++++++++-------- .github/scripts/Checkout-GhAwPr.ps1 | 39 +++++-------------- .../workflows/copilot-evaluate-tests.lock.yml | 15 ++++--- .github/workflows/copilot-evaluate-tests.md | 7 +++- 4 files changed, 44 insertions(+), 52 deletions(-) diff --git a/.github/instructions/gh-aw-workflows.instructions.md b/.github/instructions/gh-aw-workflows.instructions.md index 30d02675e639..c8cd70ecc3be 100644 --- a/.github/instructions/gh-aw-workflows.instructions.md +++ b/.github/instructions/gh-aw-workflows.instructions.md @@ -44,7 +44,9 @@ The prompt is built in the **activation job** via `{{#runtime-import .github/wor ### Fork PR Activation Gate -`gh aw compile` automatically injects a fork guard into the activation job's `if:` condition: `head.repo.id == repository_id`. This blocks fork PRs on `pull_request` events. This is **platform behavior** — do not add it manually. +By default, `gh aw compile` automatically injects a fork guard into the activation job's `if:` condition: `head.repo.id == repository_id`. This blocks fork PRs on `pull_request` events. + +To **allow fork PRs**, add `forks: ["*"]` to the `pull_request` trigger in the `.md` frontmatter. The compiler removes the auto-injected guard from the compiled `if:` conditions. This is safe when the workflow uses the `Checkout-GhAwPr.ps1` pattern (checkout + trusted-infra restore) and the agent is sandboxed. ## Fork PR Handling @@ -58,16 +60,17 @@ Reference: https://securitylab.github.com/resources/github-actions-preventing-pw | Trigger | `checkout_pr_branch.cjs` runs? | Fork handling | |---------|-------------------------------|---------------| -| `pull_request` | ✅ Yes | Blocked by auto-generated activation gate | +| `pull_request` (default) | ✅ Yes | Blocked by auto-generated activation gate unless `forks: ["*"]` is set | +| `pull_request` + `forks: ["*"]` | ✅ Yes | ✅ Works — user steps restore trusted infra before agent runs | | `workflow_dispatch` | ❌ Skipped | ✅ Works — user steps handle checkout and restore is final | | `issue_comment` (same-repo) | ✅ Yes | ✅ Works — files already on PR branch | -| `issue_comment` (fork) | N/A | ❌ Blocked by fail-closed fork guard in `Checkout-GhAwPr.ps1` | +| `issue_comment` (fork) | ✅ Yes | ⚠️ Works — `checkout_pr_branch.cjs` re-checks out fork branch after user steps, potentially overwriting restored infra. Acceptable because agent is sandboxed (no credentials, max 1 comment via safe-outputs). Pre-flight check catches missing `SKILL.md` if fork isn't rebased. | ### The `issue_comment` + Fork Problem -For `/slash-command` triggers on fork PRs, `checkout_pr_branch.cjs` runs AFTER all user steps and re-checks out the fork branch. This overwrites any files restored by user steps (e.g., `.github/skills/`). There is no way to run user steps after platform steps. A fork could include a crafted `SKILL.md` that alters the agent's evaluation behavior. +For `/slash-command` triggers on fork PRs, `checkout_pr_branch.cjs` runs AFTER all user steps and re-checks out the fork branch. This overwrites any files restored by user steps (e.g., `.github/skills/`). A fork could include a crafted `SKILL.md` that alters the agent's evaluation behavior. -**Current approach (fail-closed fork guard):** `Checkout-GhAwPr.ps1` checks `isCrossRepository` via `gh pr view` for `issue_comment` triggers. If the PR is from a fork or the API call fails, the script exits with code 1. Fork PRs should use `workflow_dispatch` instead, where `checkout_pr_branch.cjs` is skipped and the user step restore is the final workspace state. +**Accepted residual risk:** The agent runs in a sandboxed container with all credentials scrubbed. The worst outcome is a manipulated evaluation comment (`safe-outputs: add-comment: max: 1`). The agent has no ability to push code, access secrets, or exfiltrate data. The pre-flight check in the agent prompt catches the case where `SKILL.md` is missing entirely (fork not rebased on `main`). **Upstream issue:** [github/gh-aw#18481](https://github.com/github/gh-aw/issues/18481) — "Using gh-aw in forks of repositories" @@ -86,16 +89,16 @@ steps: The script: 1. Captures the base branch SHA before checkout -2. **Fork guard** (`issue_comment` only): checks `isCrossRepository` — exits 1 if fork or API failure -3. Checks out the PR branch via `gh pr checkout` -4. Deletes `.github/skills/` and `.github/instructions/` (prevents fork-added files) -5. Restores them from the base branch SHA (best-effort, non-fatal) +2. Checks out the PR branch via `gh pr checkout` +3. Deletes `.github/skills/` and `.github/instructions/` (prevents fork-added files) +4. Restores them from the base branch SHA (best-effort, non-fatal) **Behavior by trigger:** -- **`workflow_dispatch`**: Fork guard skipped. Platform checkout is skipped, so the restore IS the final workspace state (trusted files from base branch) -- **`issue_comment`** (same-repo): Fork guard passes. Platform re-checks out PR branch — files already match, effectively a no-op -- **`issue_comment`** (fork): Fork guard rejects — exits 1 with actionable notice to use `workflow_dispatch` -- **`pull_request`** (same-repo): Fork guard skipped. Files already exist, restore is a no-op +- **`workflow_dispatch`**: Platform checkout is skipped, so the restore IS the final workspace state (trusted files from base branch) +- **`pull_request`** (same-repo): User step restores trusted infra. `checkout_pr_branch.cjs` runs after and re-checks out PR branch — for same-repo PRs, skill files typically match main unless the PR modified them. +- **`pull_request`** (fork with `forks: ["*"]`): Same as above, but fork's skill files may differ. Same residual risk as `issue_comment` fork case — agent is sandboxed, pre-flight catches missing `SKILL.md`. +- **`issue_comment`** (same-repo): Platform re-checks out PR branch — files already match, effectively a no-op +- **`issue_comment`** (fork): Platform re-checks out fork branch after us, overwriting restored files. Agent is sandboxed; pre-flight in the prompt catches missing `SKILL.md` ### Anti-Patterns @@ -197,7 +200,7 @@ Manual triggers (`workflow_dispatch`, `issue_comment`) should bypass the gate. N | What | Behavior | Workaround | |------|----------|------------| -| User steps always before platform steps | Cannot run user code after `checkout_pr_branch.cjs` | Use `workflow_dispatch` for fork PRs; see [gh-aw#18481](https://github.com/github/gh-aw/issues/18481) | +| User steps always before platform steps | Cannot run user code after `checkout_pr_branch.cjs` | For `issue_comment` fork PRs, accept sandboxed residual risk; see [gh-aw#18481](https://github.com/github/gh-aw/issues/18481) | | `--allow-all-tools` in lock.yml | Emitted by `gh aw compile` | Cannot override from `.md` source | | MCP integrity filtering | Fork PRs blocked as "unapproved" | Use `steps:` checkout instead of MCP | | `gh` CLI inside agent | Credentials scrubbed | Use `steps:` for API calls, or MCP tools | @@ -215,8 +218,8 @@ Manual triggers (`workflow_dispatch`, `issue_comment`) should bypass the gate. N | Symptom | Cause | Fix | |---------|-------|-----| | Agent evaluates wrong PR | `workflow_dispatch` checks out workflow branch | Add `gh pr checkout` in `steps:` | -| Agent can't find SKILL.md | Fork PR branch doesn't have `.github/skills/` | Agent posts "rebase or use `workflow_dispatch`" message; or rebase fork on `main` | -| Fork PR rejected on `/evaluate-tests` | Fail-closed fork guard in `Checkout-GhAwPr.ps1` | Use `workflow_dispatch` with `pr_number` input instead | +| Agent can't find SKILL.md | Fork PR branch doesn't include `.github/skills/` | Rebase fork on `main`, or use `workflow_dispatch` with `pr_number` input | +| Fork PR skipped on `pull_request` | `forks: ["*"]` not in workflow frontmatter | Add `forks: ["*"]` under `pull_request:` in the `.md` source and recompile | | `gh` commands fail in agent | Credentials scrubbed inside container | Move to `steps:` section | | Lock file out of date | Forgot to recompile | Run `gh aw compile` | | Integrity filtering warning | MCP reading fork PR data | Expected, non-blocking | diff --git a/.github/scripts/Checkout-GhAwPr.ps1 b/.github/scripts/Checkout-GhAwPr.ps1 index 4a6380a50421..a2f9533bb7d2 100644 --- a/.github/scripts/Checkout-GhAwPr.ps1 +++ b/.github/scripts/Checkout-GhAwPr.ps1 @@ -4,10 +4,13 @@ .DESCRIPTION Checks out a PR branch and restores trusted agent infrastructure (skills, - instructions) from the base branch. For issue_comment triggers, fork PRs - are rejected (fail-closed) because the platform's checkout_pr_branch.cjs - overwrites restored files after user steps. Fork PRs should use - workflow_dispatch instead. + instructions) from the base branch. Works for both same-repo and fork PRs. + + This script is only invoked for workflow_dispatch triggers. For pull_request + and issue_comment, the gh-aw platform's checkout_pr_branch.cjs handles PR + checkout automatically (it runs as a platform step after all user steps). + workflow_dispatch skips the platform checkout entirely, so this script is + the only thing that gets the PR code onto disk. SECURITY NOTE: This script checks out PR code onto disk. This is safe because NO subsequent user steps execute workspace code — the gh-aw @@ -26,7 +29,6 @@ PR_NUMBER - PR number to check out GITHUB_REPOSITORY - owner/repo (set by GitHub Actions) GITHUB_ENV - path to env file (set by GitHub Actions) - GITHUB_EVENT_NAME - trigger type (set by GitHub Actions) #> $ErrorActionPreference = 'Stop' @@ -40,25 +42,6 @@ if (-not $env:PR_NUMBER -or $env:PR_NUMBER -eq '0') { $PrNumber = $env:PR_NUMBER -# ── Fork guard (issue_comment only) ───────────────────────────────────────── -# For issue_comment triggers, platform's checkout_pr_branch.cjs runs AFTER user -# steps and re-checks out the fork branch, overwriting any restored skill/instruction -# files. A fork could include a crafted SKILL.md that alters agent behavior. -# Fail closed: if we can't verify origin, exit 1 (not 0). -# Fork PRs can still be evaluated via workflow_dispatch (where platform checkout is skipped). - -if ($env:GITHUB_EVENT_NAME -eq 'issue_comment') { - $isFork = gh pr view $PrNumber --repo $env:GITHUB_REPOSITORY --json isCrossRepository --jq '.isCrossRepository' 2>&1 - if ($LASTEXITCODE -ne 0) { - Write-Host "❌ Could not verify PR origin — failing closed" - exit 1 - } - if ($isFork -eq 'true') { - Write-Host "::notice::Fork PR detected — /evaluate-tests via issue_comment is not supported for fork PRs. Use workflow_dispatch with pr_number=$PrNumber instead." - exit 1 - } -} - # ── Save base branch SHA ───────────────────────────────────────────────────── # Must be captured BEFORE checkout replaces HEAD. # Exported for potential use by downstream platform steps (e.g., checkout_pr_branch.cjs) @@ -82,11 +65,9 @@ Write-Host "✅ Checked out PR #$PrNumber" git log --oneline -1 # ── Restore agent infrastructure from base branch ──────────────────────────── -# Best-effort restore of skill/instruction files from the base branch. -# - workflow_dispatch: platform checkout is skipped, so this IS the final state -# - issue_comment (same-repo): platform's checkout_pr_branch.cjs runs after and -# overwrites, but files already match (same repo). Fork PRs are blocked above. -# - pull_request (same-repo): files already exist, this is a no-op +# This script only runs for workflow_dispatch (other triggers use the platform's +# checkout_pr_branch.cjs instead). For workflow_dispatch the platform checkout is +# skipped, so this restore IS the final workspace state. # rm -rf first to prevent fork-added files from surviving the restore. if (Test-Path '.github/skills/') { Remove-Item -Recurse -Force '.github/skills/' } diff --git a/.github/workflows/copilot-evaluate-tests.lock.yml b/.github/workflows/copilot-evaluate-tests.lock.yml index c1e75132b153..5b5dc7f9b37d 100644 --- a/.github/workflows/copilot-evaluate-tests.lock.yml +++ b/.github/workflows/copilot-evaluate-tests.lock.yml @@ -22,7 +22,7 @@ # # Evaluates test quality, coverage, and appropriateness on PRs that add or modify tests # -# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"9a0e480927d61956be62f85669d2e599525442694d280126849fe87e727c31df","compiler_version":"v0.62.2","strict":true} +# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"d671028235c1b911c7a816a257b07b02793a6b57747b4358f792af183e26ca07","compiler_version":"v0.62.2","strict":true} name: "Evaluate PR Tests" "on": @@ -30,6 +30,8 @@ name: "Evaluate PR Tests" types: - created pull_request: + # forks: # Fork filtering applied via job conditions + # - "*" # Fork filtering applied via job conditions paths: - src/**/tests/** - src/**/test/** @@ -57,9 +59,9 @@ jobs: activation: needs: pre_activation if: > - (needs.pre_activation.outputs.activated == 'true') && (((github.event_name == 'pull_request' && github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || (github.event_name == 'issue_comment' && + (needs.pre_activation.outputs.activated == 'true') && ((github.event_name == 'pull_request' && github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || (github.event_name == 'issue_comment' && github.event.issue.pull_request && - startsWith(github.event.comment.body, '/evaluate-tests'))) && ((github.event_name != 'pull_request') || (github.event.pull_request.head.repo.id == github.repository_id))) + startsWith(github.event.comment.body, '/evaluate-tests'))) runs-on: ubuntu-slim permissions: contents: read @@ -324,7 +326,8 @@ jobs: run: "TEST_FILES=$(gh pr diff \"$PR_NUMBER\" --repo \"$GITHUB_REPOSITORY\" --name-only \\\n | grep -E '\\.(cs|xaml)$' \\\n | grep -iE '(tests?/|TestCases|UnitTests|DeviceTests)' \\\n || true)\nif [ -z \"$TEST_FILES\" ]; then\n echo \"⏭️ No test source files (.cs/.xaml) found in PR diff. Skipping evaluation.\"\n exit 1\nfi\necho \"✅ Found test files to evaluate:\"\necho \"$TEST_FILES\" | head -20\n" - env: GH_TOKEN: ${{ github.token }} - PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} + PR_NUMBER: ${{ inputs.pr_number }} + if: github.event_name == 'workflow_dispatch' name: Checkout PR and restore agent infrastructure run: pwsh .github/scripts/Checkout-GhAwPr.ps1 @@ -986,9 +989,9 @@ jobs: pre_activation: if: > - ((github.event_name == 'pull_request' && github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || (github.event_name == 'issue_comment' && + (github.event_name == 'pull_request' && github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || (github.event_name == 'issue_comment' && github.event.issue.pull_request && - startsWith(github.event.comment.body, '/evaluate-tests'))) && ((github.event_name != 'pull_request') || (github.event.pull_request.head.repo.id == github.repository_id)) + startsWith(github.event.comment.body, '/evaluate-tests')) runs-on: ubuntu-slim outputs: activated: ${{ steps.check_membership.outputs.is_team_member == 'true' }} diff --git a/.github/workflows/copilot-evaluate-tests.md b/.github/workflows/copilot-evaluate-tests.md index c84f83f8855e..854c2ec407d8 100644 --- a/.github/workflows/copilot-evaluate-tests.md +++ b/.github/workflows/copilot-evaluate-tests.md @@ -3,6 +3,7 @@ description: Evaluates test quality, coverage, and appropriateness on PRs that a on: pull_request: types: [opened, synchronize, reopened, ready_for_review] + forks: ["*"] paths: - 'src/**/tests/**' - 'src/**/test/**' @@ -72,10 +73,14 @@ steps: echo "✅ Found test files to evaluate:" echo "$TEST_FILES" | head -20 + # Only needed for workflow_dispatch — for pull_request and issue_comment, + # the gh-aw platform's checkout_pr_branch.cjs handles PR checkout automatically. + # workflow_dispatch skips the platform checkout entirely, so we must do it here. - name: Checkout PR and restore agent infrastructure + if: github.event_name == 'workflow_dispatch' env: GH_TOKEN: ${{ github.token }} - PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} + PR_NUMBER: ${{ inputs.pr_number }} run: pwsh .github/scripts/Checkout-GhAwPr.ps1 --- From 53e657527f001ed18635eea3ab9a3a7de42c1acf Mon Sep 17 00:00:00 2001 From: Stephane Delcroix Date: Sat, 28 Mar 2026 20:23:57 +0100 Subject: [PATCH 03/12] Add regression test for #34713: Binding with Converter and x:DataType is compiled (#34717) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Adds regression tests for #34713 verifying the XAML source generator correctly handles bindings with `Converter={StaticResource ...}` inside `x:DataType` scopes. Closes #34713 ## Investigation After thorough investigation of the source generator pipeline (`KnownMarkups.cs`, `CompiledBindingMarkup.cs`, `NodeSGExtensions.cs`): ### When converter IS in page resources (compile-time resolution ✅) `GetResourceNode()` walks the XAML tree, finds the converter resource, and `ProvideValueForStaticResourceExtension` returns the variable directly — **no runtime `ProvideValue` call**. The converter is referenced at compile time. ### When converter is NOT in page resources (runtime resolution ✅) `GetResourceNode()` returns null → falls through to `IsValueProvider` → generates `StaticResourceExtension.ProvideValue(serviceProvider)`. The `SimpleValueTargetProvider` provides the full parent chain, and `TryGetApplicationLevelResource` checks `Application.Current.Resources`. The binding IS still compiled into a `TypedBinding` — only the converter resolution is deferred. ### Verified on both `main` and `net11.0` All tests pass on both branches. ## Tests added | Test | What it verifies | |------|-----------------| | `SourceGenResolvesConverterAtCompileTime_ImplicitResources` | Converter in implicit `` → compile-time resolution, no `ProvideValue` | | `SourceGenResolvesConverterAtCompileTime_ExplicitResourceDictionary` | Converter in explicit `` → compile-time resolution, no `ProvideValue` | | `SourceGenCompilesBindingWithConverterToTypedBinding` | Converter NOT in page resources → still compiled to `TypedBinding`, no raw `Binding` fallback | | `BindingWithConverterFromAppResourcesWorksCorrectly` × 3 | Runtime behavior correct for all inflators (Runtime, XamlC, SourceGen) | Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Xaml.UnitTests/Issues/Maui34713.xaml | 14 ++ .../Xaml.UnitTests/Issues/Maui34713.xaml.cs | 203 ++++++++++++++++++ 2 files changed, 217 insertions(+) create mode 100644 src/Controls/tests/Xaml.UnitTests/Issues/Maui34713.xaml create mode 100644 src/Controls/tests/Xaml.UnitTests/Issues/Maui34713.xaml.cs diff --git a/src/Controls/tests/Xaml.UnitTests/Issues/Maui34713.xaml b/src/Controls/tests/Xaml.UnitTests/Issues/Maui34713.xaml new file mode 100644 index 000000000000..b621d073f473 --- /dev/null +++ b/src/Controls/tests/Xaml.UnitTests/Issues/Maui34713.xaml @@ -0,0 +1,14 @@ + + + + + + + diff --git a/src/Controls/tests/Xaml.UnitTests/Issues/Maui34713.xaml.cs b/src/Controls/tests/Xaml.UnitTests/Issues/Maui34713.xaml.cs new file mode 100644 index 000000000000..098fb9051b21 --- /dev/null +++ b/src/Controls/tests/Xaml.UnitTests/Issues/Maui34713.xaml.cs @@ -0,0 +1,203 @@ +using System; +using System.Globalization; +using Microsoft.Maui.ApplicationModel; +using Microsoft.Maui.Controls.Core.UnitTests; +using Microsoft.Maui.Dispatching; +using Microsoft.Maui.UnitTests; +using Xunit; + +using static Microsoft.Maui.Controls.Xaml.UnitTests.MockSourceGenerator; + +namespace Microsoft.Maui.Controls.Xaml.UnitTests; + +public partial class Maui34713 : ContentPage +{ + public Maui34713() + { + InitializeComponent(); + } + + [Collection("Issue")] + public class Tests : IDisposable + { + public Tests() + { + DispatcherProvider.SetCurrent(new DispatcherProviderStub()); + } + + public void Dispose() + { + AppInfo.SetCurrent(null); + Application.SetCurrentApplication(null); + DispatcherProvider.SetCurrent(null); + } + + const string SharedCs = @" +using System; +using System.Globalization; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; +namespace Microsoft.Maui.Controls.Xaml.UnitTests +{ +public class Maui34713ViewModel { public bool IsActive { get; set; } public string Name { get; set; } = """"; } +public class Maui34713BoolToTextConverter : IValueConverter { + public object Convert(object v, Type t, object p, CultureInfo c) => v is true ? ""Active"" : ""Inactive""; + public object ConvertBack(object v, Type t, object p, CultureInfo c) => throw new NotImplementedException(); + } +}"; + +[Fact] +internal void SourceGenResolvesConverterAtCompileTime_ImplicitResources() +{ + // When converter IS in page resources (implicit), source gen should + // resolve it at compile time - no runtime ProvideValue needed. + var xaml = @" + + + + + + + "; + + var cs = @" +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; +namespace Microsoft.Maui.Controls.Xaml.UnitTests +{ +[XamlProcessing(XamlInflator.Runtime, true)] +public partial class Maui34713Test1 : ContentPage { public Maui34713Test1() { InitializeComponent(); } } + }" + SharedCs; + + var result = CreateMauiCompilation() + .WithAdditionalSource(cs, hintName: "Maui34713Test1.xaml.cs") + .RunMauiSourceGenerator(new AdditionalXamlFile("Issues/Maui34713Test1.xaml", xaml, TargetFramework: "net10.0")); + + var generated = result.GeneratedInitializeComponent(); + + Assert.Contains("TypedBinding", generated, StringComparison.Ordinal); + // Converter should be resolved at compile time - no ProvideValue call + Assert.DoesNotContain(".ProvideValue(", generated, StringComparison.Ordinal); +} + +[Fact] +internal void SourceGenResolvesConverterAtCompileTime_ExplicitResourceDictionary() +{ + // When converter IS in page resources (explicit RD), source gen should + // also resolve it at compile time. + var xaml = @" + + + + + + + + + "; + + var cs = @" +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; +namespace Microsoft.Maui.Controls.Xaml.UnitTests +{ +[XamlProcessing(XamlInflator.Runtime, true)] +public partial class Maui34713Test2 : ContentPage { public Maui34713Test2() { InitializeComponent(); } } + }" + SharedCs; + + var result = CreateMauiCompilation() + .WithAdditionalSource(cs, hintName: "Maui34713Test2.xaml.cs") + .RunMauiSourceGenerator(new AdditionalXamlFile("Issues/Maui34713Test2.xaml", xaml, TargetFramework: "net10.0")); + + var generated = result.GeneratedInitializeComponent(); + + Assert.Contains("TypedBinding", generated, StringComparison.Ordinal); + // Converter should be resolved at compile time - no ProvideValue call + Assert.DoesNotContain(".ProvideValue(", generated, StringComparison.Ordinal); +} + +[Fact] +internal void SourceGenCompilesBindingWithConverterToTypedBinding() +{ + // When the converter is NOT in page resources, the binding should + // still be compiled into a TypedBinding. + var result = CreateMauiCompilation() + .WithAdditionalSource( + @"using System; + using System.Globalization; + using Microsoft.Maui.Controls; + using Microsoft.Maui.Controls.Xaml; + + namespace Microsoft.Maui.Controls.Xaml.UnitTests; + + [XamlProcessing(XamlInflator.Runtime, true)] + public partial class Maui34713 : ContentPage + { + public Maui34713() => InitializeComponent(); + } + + public class Maui34713ViewModel + { + public bool IsActive { get; set; } + public string Name { get; set; } = string.Empty; + } + + public class Maui34713BoolToTextConverter : IValueConverter + { + public object? Convert(object? value, Type targetType, object? parameter, CultureInfo culture) + => value is true ? ""Active"" : ""Inactive""; + + public object? ConvertBack(object? value, Type targetType, object? parameter, CultureInfo culture) + => throw new NotImplementedException(); + } + ") + .RunMauiSourceGenerator(typeof(Maui34713)); + + var generated = result.GeneratedInitializeComponent(); + + Assert.Contains("TypedBinding", generated, StringComparison.Ordinal); + Assert.Contains("Converter = extension.Converter", generated, StringComparison.Ordinal); + Assert.DoesNotContain("new global::Microsoft.Maui.Controls.Binding(", generated, StringComparison.Ordinal); +} + +[Theory] +[XamlInflatorData] +internal void BindingWithConverterFromAppResourcesWorksCorrectly(XamlInflator inflator) +{ + var mockApp = new MockApplication(); + mockApp.Resources.Add("BoolToTextConverter", new Maui34713BoolToTextConverter()); + Application.SetCurrentApplication(mockApp); + + var page = new Maui34713(inflator); + page.BindingContext = new Maui34713ViewModel { IsActive = true, Name = "Test" }; + + Assert.Equal("Active", page.label0.Text); + Assert.Equal("Test", page.label1.Text); +} +} +} + +#nullable enable + +public class Maui34713ViewModel +{ + public bool IsActive { get; set; } + public string Name { get; set; } = string.Empty; +} + +public class Maui34713BoolToTextConverter : IValueConverter +{ + public object? Convert(object? value, Type targetType, object? parameter, CultureInfo culture) + => value is true ? "Active" : "Inactive"; + + public object? ConvertBack(object? value, Type targetType, object? parameter, CultureInfo culture) + => throw new NotImplementedException(); +} From 23e0a2acdfda547c4984520021d59c117a2ae8e7 Mon Sep 17 00:00:00 2001 From: Shane Neuville <5375137+PureWeen@users.noreply.github.com> Date: Mon, 30 Mar 2026 09:35:05 -0500 Subject: [PATCH 04/12] =?UTF-8?q?Add=20merge=20flow:=20net11.0=20=E2=86=92?= =?UTF-8?q?=20next=20release=20branch=20(#34626)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > [!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! ## Description Adds arcade inter-branch merge workflow and configuration to automate merging `net11.0` into the `release/11.0.1xx-preview3` branch. ### Files added | File | Purpose | |------|---------| | `github-merge-flow-release-11.jsonc` | Merge flow config — source `net11.0`, target `release/11.0.1xx-preview3` | | `.github/workflows/merge-net11-to-release.yml` | GitHub Actions workflow — triggers on push to net11.0, daily cron, manual dispatch | ### How it works Uses the shared [dotnet/arcade inter-branch merge infrastructure](https://github.com/dotnet/arcade/blob/main/.github/workflows/inter-branch-merge-base.yml): - **Event-driven**: triggers on push to `net11.0`, with daily cron safety net - **ResetToTargetPaths**: auto-resets `global.json`, `NuGet.config`, `eng/Version.Details.xml`, `eng/Versions.props`, `eng/common/*` to target branch versions - **QuietComments**: reduces GitHub notification noise - Skips PRs when only Maestro bot commits exist ### Incrementing for future releases When cutting a new release (e.g., preview4), update: 1. `github-merge-flow-release-11.jsonc` → change `MergeToBranch` value 2. `.github/workflows/merge-net11-to-release.yml` → update workflow `name` field Follows the same pattern as `merge-main-to-net11.yml` / `github-merge-flow-net11.jsonc`. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/merge-net11-to-release.yml | 22 ++++++++++++++++++++ github-merge-flow-release-11.jsonc | 10 +++++++++ 2 files changed, 32 insertions(+) create mode 100644 .github/workflows/merge-net11-to-release.yml create mode 100644 github-merge-flow-release-11.jsonc diff --git a/.github/workflows/merge-net11-to-release.yml b/.github/workflows/merge-net11-to-release.yml new file mode 100644 index 000000000000..ac13e47f9372 --- /dev/null +++ b/.github/workflows/merge-net11-to-release.yml @@ -0,0 +1,22 @@ +# Merge net11.0 → next release branch +# Target branch is configured in /github-merge-flow-release-11.jsonc (MergeToBranch) + +name: Merge net11.0 to next release + +on: + workflow_dispatch: + push: + branches: + - net11.0 + schedule: + - cron: '0 4 * * *' + +permissions: + contents: write + pull-requests: write + +jobs: + Merge: + uses: dotnet/arcade/.github/workflows/inter-branch-merge-base.yml@main + with: + configuration_file_path: 'github-merge-flow-release-11.jsonc' diff --git a/github-merge-flow-release-11.jsonc b/github-merge-flow-release-11.jsonc new file mode 100644 index 000000000000..e691b33db918 --- /dev/null +++ b/github-merge-flow-release-11.jsonc @@ -0,0 +1,10 @@ +// Update MergeToBranch when cutting a new release. +{ + "merge-flow-configurations": { + "net11.0": { + "MergeToBranch": "release/11.0.1xx-preview3", + "ExtraSwitches": "-QuietComments", + "ResetToTargetPaths": "global.json;NuGet.config;eng/Version.Details.xml;eng/Versions.props;eng/common/*" + } + } +} From cdead2797610b0fc01566996f2bb531a0cf373e1 Mon Sep 17 00:00:00 2001 From: Pedro Jesus Date: Wed, 1 Apr 2026 09:12:42 -0300 Subject: [PATCH 05/12] Fix Flyout memory leak (#34485) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Issues Fixed Fixes #33355 ### Description of Change This report has the goal to provide a detailed progress on the solution of the memory-leak The test consists doing 100 navigations between 2 pages, as the image below suggest image Running the gc-dump on the desired objects this is what I found. > BaseLine is the dump when the app starts. | | | | | | | ------------------------------------ | ------- | ------------ | -------------- | ------------------------ | | Object Type | Count | Size (Bytes) | Expected Count | Status | | **Page2** | **100** | 84,000 | 1 | ❌ **LEAKED** (99 extra) | | **PageHandler** | **103** | 9,888 | 4 | ❌ **LEAKED** (99 extra) | | **StackNavigationManager** | **102** | 16,320 | 1 | ❌ **LEAKED** (101 extra) | | **StackNavigationManager.Callbacks** | **102** | 5,712 | 1 | ❌ **LEAKED** (101 extra) | | **NavigationViewFragment** | **102** | 5,712 | ~2 | ❌ **LEAKED** (100 extra) | So the first fix was to call `Disconnect` handler, on the `previousDetail` during the `FlyoutPage.Detail_set`. The PageChanges and Navigated events will not see this, since this `set` is not considered a navigation in .Net Maui. After that we see the following data | | | | | | | ------------------------------------ | ------- | ------------ | ---------------------- | ------------ | | Object Type | Count | Size (Bytes) | vs Baseline | Status | | **Page2** | **100** | 84,000 | Same | ❌ **LEAKED** | | **PageHandler** | **103** | 9,888 | Same | ❌ **LEAKED** | | **StackNavigationManager** | **102** | 16,320 | Same | ❌ **LEAKED** | | **StackNavigationManager.Callbacks** | **1** | 56 | ✅ **FIXED!** (was 102) | ✅ **Good!** | | **NavigationViewFragment** | **102** | 5,712 | Same | ❌ **LEAKED** | So, calling the Disconnect handler will fix the leak at `StackNavigationManager.Callbacks`. Next step was to investigate the `StackNavigationManager` and see what's holding it. On `StackNavigationManager` I see lot of object that should be cleaned up in order to release other references from it. After cleaning it up the result is | | | | | | |---|---|---|---|---| |Object Type|Count|Size (Bytes)|vs Baseline|Status| |**Page2**|**1**|840|✅ **FIXED!** (was 100)|✅ **Perfect!**| |**PageHandler**|**4**|384|✅ **FIXED!** (was 103)|✅ **Perfect!**| |**StackNavigationManager**|**102**|16,320|❌ Still leaking (was 102)|❌ **Unchanged**| |**StackNavigationManager.Callbacks**|**1**|56|✅ Fixed (was 102)|✅ **Good!**| |**NavigationViewFragment**|**102**|5,712|❌ Still leaking (was 102)|❌ **Unchanged**| So something is still holding the `StackNavigationManager` and `NavigationViewFragment` so I changed the approach and found that `NavigationViewFragment` is holding everything and after fixing that, cleaning it up on `Destroy` method. here's the result | | | | | | |---|---|---|---|---| |Object Type|Count|Size (Bytes)|vs Previous|Status| |**Page2**|**1**|840|✅ Same|✅ **Perfect!**| |**PageHandler**|**4**|384|✅ Same|✅ **Perfect!**| |**StackNavigationManager**|**1**|160|🎉 **FIXED!** (was 102)|🎉 **FIXED!**| |**StackNavigationManager.Callbacks**|**1**|56|✅ Same|✅ **Perfect!**| |**NavigationViewFragment**|**102**|5,712|⚠️ Still present|⚠️ **Remaining**| With that there's still the leak of `NavigationViewFragment`, looking at the graph the something on Android side is holding it, there's no root into managed objects, as far the gcdump can tell. I tried to cleanup the `FragmentManager`, `NavController` and so on but without success (maybe I did it wrong). There's still one instance of page 2, somehow it lives longer, I don't think it's a leak object because since its value is 1. For reference the Page2 graph is ``` Page2 (1 instance) └── PageHandler └── EventHandler └── IOnFocusChangeListenerImplementor (Native Android) └── UNDEFINED ``` Looking into www I found that android caches those Fragments, sadly in our case we don't reuse them. The good part is each object has only 56 bytes, so it shouldn't be a big deal, I believe we can take the improvements made by this PR and keep an eye on that, maybe that's fixed when moved to Navigation3 implementation. --- .../src/Core/FlyoutPage/FlyoutPage.cs | 9 ++++ .../tests/DeviceTests/Memory/MemoryTests.cs | 48 +++++++++++++++++++ .../FlyoutView/FlyoutViewHandler.Android.cs | 7 ++- .../Navigation/NavigationViewFragment.cs | 1 + 4 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/Controls/src/Core/FlyoutPage/FlyoutPage.cs b/src/Controls/src/Core/FlyoutPage/FlyoutPage.cs index 580652749e05..c2f8f9de38bb 100644 --- a/src/Controls/src/Core/FlyoutPage/FlyoutPage.cs +++ b/src/Controls/src/Core/FlyoutPage/FlyoutPage.cs @@ -76,6 +76,15 @@ public Page Detail { previousDetail.SendNavigatedFrom( new NavigatedFromEventArgs(destinationPage: value, NavigationType.Replace)); + + if (previousDetail.IsLoaded) + { + previousDetail.OnUnloaded(previousDetail.DisconnectHandlers); + } + else + { + previousDetail.DisconnectHandlers(); + } } _detail.SendNavigatedTo(new NavigatedToEventArgs(previousDetail, NavigationType.Replace)); diff --git a/src/Controls/tests/DeviceTests/Memory/MemoryTests.cs b/src/Controls/tests/DeviceTests/Memory/MemoryTests.cs index 5a804343d967..93e9a4fe3a34 100644 --- a/src/Controls/tests/DeviceTests/Memory/MemoryTests.cs +++ b/src/Controls/tests/DeviceTests/Memory/MemoryTests.cs @@ -89,9 +89,11 @@ void SetupBuilder() #if IOS || MACCATALYST handlers.AddHandler(); handlers.AddHandler(); + handlers.AddHandler(); #else handlers.AddHandler(); handlers.AddHandler(); + handlers.AddHandler(); #endif }); }); @@ -148,6 +150,52 @@ await CreateHandlerAndAddToWindow(new Window(navPage), async () => await AssertionExtensions.WaitForGC(references.ToArray()); } + #if ANDROID + [Fact("FlyoutPage Detail Navigation Does Not Leak")] + public async Task FlyoutPageDetailNavigationDoesNotLeak() + { + SetupBuilder(); + + var references = new List(); + + var initialDetail = new NavigationPage(new ContentPage { Title = "Initial Detail" }); + + var flyoutPage = new FlyoutPage + { + Flyout = new ContentPage { Title = "Flyout" }, + Detail = initialDetail + }; + + await CreateHandlerAndAddToWindow(new Window(flyoutPage), async () => + { + for (int i = 0; i < 4; i++) + { + var detailPage = new ContentPage + { + Title = $"Detail {i}", + Content = new Label { Text = $"Content {i}" } + }; + var navPage = new NavigationPage(detailPage); + + flyoutPage.Detail = navPage; + flyoutPage.IsPresented = false; + + await OnLoadedAsync(detailPage); + + references.Add(new(detailPage)); + references.Add(new(navPage)); + } + }); + + + // The last page will be alive and attached to the FlyoutPage + references.RemoveAt(references.Count - 1); + references.RemoveAt(references.Count - 1); + + await AssertionExtensions.WaitForGC(references.ToArray()); + } +#endif + [Theory("Handler Does Not Leak")] [InlineData(typeof(ActivityIndicator))] [InlineData(typeof(Border))] diff --git a/src/Core/src/Handlers/FlyoutView/FlyoutViewHandler.Android.cs b/src/Core/src/Handlers/FlyoutView/FlyoutViewHandler.Android.cs index fac7d6e86772..97f4e0eb75f5 100644 --- a/src/Core/src/Handlers/FlyoutView/FlyoutViewHandler.Android.cs +++ b/src/Core/src/Handlers/FlyoutView/FlyoutViewHandler.Android.cs @@ -70,8 +70,11 @@ void UpdateDetailsFragmentView() if (context is null) return; - if (VirtualView.Detail?.Handler is IPlatformViewHandler pvh) - pvh.DisconnectHandler(); + if (_detailViewFragment?.DetailView is IView previousDetail && + previousDetail != VirtualView.Detail) + { + previousDetail.Handler?.DisconnectHandler(); + } var fragmentManager = MauiContext.GetFragmentManager(); diff --git a/src/Core/src/Platform/Android/Navigation/NavigationViewFragment.cs b/src/Core/src/Platform/Android/Navigation/NavigationViewFragment.cs index 496990dee1a7..a5fd1a999391 100644 --- a/src/Core/src/Platform/Android/Navigation/NavigationViewFragment.cs +++ b/src/Core/src/Platform/Android/Navigation/NavigationViewFragment.cs @@ -89,6 +89,7 @@ public override void OnDestroy() { _currentView = null; _fragmentContainerView = null; + _navigationManager = null; base.OnDestroy(); } From 59b36c27ecc67458b89323fe098f779c2c3fc18b Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Thu, 2 Apr 2026 12:24:24 +0100 Subject: [PATCH 06/12] [spec] Add `maui device list` command spec to CLI design doc (#34277) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > [!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 Adds the `maui device list` command specification to the existing CLI design document (`docs/design/cli.md`). This command provides unified, cross-platform device enumeration without requiring a project context. See [PR #33865](https://github.com/dotnet/maui/pull/33865) for the full DevTools spec. ## What is added ### Command: `maui device list [--platform

] [--json]` Lists connected devices, running emulators, and available simulators across all platforms in a single call. ### Two approaches analysis The spec analyzes two discovery approaches and recommends the project-free one: | | MSBuild (`dotnet run --list-devices`) | Native CLI (`maui device list`) | |---|---|---| | Project required | Yes | **No** | | Cross-platform | One TFM at a time | All platforms at once | | Speed | Slower (MSBuild eval) | Fast (<2s) | | ID compatibility | Source of truth | Same native IDs | ### Scenarios requiring project-free discovery 1. AI agent bootstrapping (no `.csproj` yet) 2. IDE startup (device picker before project loads) 3. Environment validation ("can I see my phone?") 4. CI pipeline setup (check emulator before build) 5. Multi-project solutions (unified view) 6. Cross-platform overview (all devices at once) ### Recommended approach `maui device list` uses direct native tool invocation (`adb devices`, `simctl list`, `devicectl list`). Device IDs are compatible with `dotnet run --device`, making them complementary: ``` maui device list → "What devices exist on this machine?" dotnet run --list-devices → "What devices can run this project?" ``` ### Other changes - Added references to `ComputeAvailableDevices` MSBuild targets in [dotnet/android](https://github.com/dotnet/android) and [dotnet/macios](https://github.com/dotnet/macios) - Updated AI agent workflow example to include device discovery step - Fixed AppleDev.Tools reference (xcdevice → devicectl) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/design/cli.md | 179 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 174 insertions(+), 5 deletions(-) diff --git a/docs/design/cli.md b/docs/design/cli.md index c61fbb07a61f..ec396c2dba94 100644 --- a/docs/design/cli.md +++ b/docs/design/cli.md @@ -1,7 +1,7 @@ --- description: "Design document for the maui CLI tool" date: 2026-01-07 -updated: 2026-02-26 +updated: 2026-02-27 --- # `maui` CLI Design Document @@ -197,6 +197,170 @@ All commands use consistent exit codes: | 4 | Network error (download failed) | | 5 | Resource not found | +## Device Discovery + +### `maui device list` + +Lists connected devices, running emulators, and available simulators +across all platforms from a single command. + +**Usage:** + +```bash +maui device list [--platform

] [--json] +``` + +**Options:** + +- `--platform `: Filter by platform (`android`, `ios`, + `maccatalyst`). If omitted, lists all platforms. +- `--json`: Structured JSON output for machine consumption. + +**Human-readable output:** + +``` +ID Description Type Platform Status +emulator-5554 Pixel 7 - API 35 Emulator android Online +0A041FDD400327 Pixel 7 Pro Device android Online +94E71AE5-8040-4DB2-8A9C-6CD24EF4E7DE iPhone 16 - iOS 26.0 Simulator ios Shutdown +FBF5DCE8-EE2B-4215-8118-3A2190DE1AD7 iPhone 14 - iOS 26.0 Simulator ios Booted +AF40CC64-2CDB-5F16-9651-86BCDF380881 My iPhone 15 Device ios Paired +``` + +**JSON output (`--json`):** + +```json +{ + "devices": [ + { + "id": "emulator-5554", + "description": "Pixel 7 - API 35", + "type": "Emulator", + "platform": "android", + "status": "Online" + }, + { + "id": "FBF5DCE8-EE2B-4215-8118-3A2190DE1AD7", + "description": "iPhone 14 - iOS 26.0", + "type": "Simulator", + "platform": "ios", + "status": "Booted" + } + ] +} +``` + +The `id` field is the same identifier accepted by `dotnet run --device +`, so output from `maui device list` can be piped directly into a +run command. + +### Two Approaches to Device Enumeration + +There are two ways to enumerate devices, each suited to different +scenarios. + +#### Approach A: Via `dotnet run --list-devices` (project-based) + +The .NET SDK (≥ .NET 11) provides `dotnet run --list-devices`, which +calls the [`ComputeAvailableDevices`][compute-android] MSBuild target +defined by each platform workload ([spec][dotnet-run-spec]): + +- **Android** ([dotnet/android]): calls `adb devices`, returns + serial, description, type (Device/Emulator), status, model +- **Apple** ([dotnet/macios]): calls `simctl list` and `devicectl + list`, returns UDID, description, type (Device/Simulator), + OS version, RuntimeIdentifier + +This approach **requires a project file** — MSBuild evaluates the +`.csproj` to locate the correct workload targets. It also operates +**per-framework**: you select a target framework first, then get +devices for that platform only. + +[dotnet/android]: https://github.com/dotnet/android +[dotnet/macios]: https://github.com/dotnet/macios + +#### Approach B: Direct native tool invocation (project-free) + +The `maui` CLI calls the same native tools directly — `adb devices`, +`xcrun simctl list devices`, `xcrun devicectl list devices` — without +evaluating any MSBuild project. This returns a unified, cross-platform +device list in a single call. + +#### Comparison + +| | Approach A (MSBuild) | Approach B (Native CLI) | +|---|---|---| +| **Project required** | Yes — needs `.csproj` | No | +| **Cross-platform** | One platform per call (per TFM) | All platforms in one call | +| **Metadata** | Rich (RuntimeIdentifier, workload-specific fields) | Standard (id, description, type, status) | +| **Speed** | Slower (MSBuild evaluation + restore) | Fast (<2s, direct process calls) | +| **ID compatibility** | Source of truth for `dotnet run --device` | Same native IDs — compatible | +| **Requires workloads** | Yes (platform workload must be installed) | Only native tools (`adb`, `simctl`) | +| **Extensible** | Workloads add new device types automatically | Must add support per platform | + +#### Scenarios Without a Project + +Several real workflows need device enumeration **before** a project +exists or **outside** any project context: + +1. **AI agent bootstrapping** — An agent starting a "vibe coding" + session needs to discover available targets before scaffolding a + project. It cannot call `dotnet run --list-devices` because there + is no `.csproj` yet. + +2. **IDE startup** — VS Code opens a workspace with no MAUI project + loaded. The extension needs to populate its device picker to show + the user what's available. A project-free query is the only option. + +3. **Environment validation** — A developer runs `maui device list` + to answer "can I see my phone?" without needing to be inside any + project directory. This is a diagnostic step, not a build step. + +4. **CI pipeline setup** — A CI script checks that the expected + emulator or simulator is running before invoking `dotnet run`. + The check should not depend on a specific project file. + +5. **Multi-project solutions** — A solution contains both Android and + iOS projects. The developer wants a single unified device list + rather than running `--list-devices` per project. + +6. **Cross-platform overview** — `dotnet run --list-devices` shows + devices for one TFM at a time. A developer switching between + Android and iOS wants to see everything at once. + +#### Recommended Approach + +`maui device list` uses **Approach B** (direct native tool invocation) +as its primary implementation: + +- It works anywhere — no project, no workload targets, no MSBuild + evaluation overhead. +- Device identifiers are the same native IDs used by + `ComputeAvailableDevices`, so they are fully compatible with + `dotnet run --device`. +- The `maui` CLI already wraps these native tools for other commands + (environment setup, emulator management), so device listing is a + natural extension. + +When a project **is** available and the user wants framework-specific +device filtering, `dotnet run --list-devices` remains the right tool — +it provides richer metadata (RuntimeIdentifier) and benefits from +workload-specific logic. The two approaches are complementary: + +``` +maui device list → "What devices exist on this machine?" +dotnet run --list-devices → "What devices can run this project?" +``` + +**Platform Implementation:** + +| Platform | Native tool | What is enumerated | +|----------|------------|-------------------| +| Android | `adb devices -l` | Physical devices and running emulators | +| iOS (simulators) | `xcrun simctl list devices --json` | All simulators (booted + shutdown) | +| iOS (physical) | `xcrun devicectl list devices` | Connected physical devices | +| Mac Catalyst | (host machine) | The Mac itself | + ## App Inspection Commands (Future) > **Note**: App inspection commands are planned for a future release. The initial release focuses on environment setup and device management. @@ -268,7 +432,6 @@ Initial implementation targets Android and iOS/Mac Catalyst, with Windows and ma ### Future Commands -- `maui device list` for unified device/emulator/simulator listing across platforms - `maui screenshot` for capturing screenshots of running apps - `maui logs` for streaming device logs - `maui tree` for inspecting the visual tree @@ -292,6 +455,9 @@ maui tree --json # future ### AI Agent Workflow ```bash +# 0. Discover available devices (no project needed) +maui device list --json + # 1. Make code changes # ... agent modifies MainPage.xaml ... @@ -392,7 +558,7 @@ Visual Studio consumes the `Xamarin.Android.Tools.AndroidSdk` NuGet package from |----------|------------|--------------| | Workspace open | `maui apple check --json`, `maui android jdk check --json` | Show environment status in status bar / problems panel | | Environment fix | `maui android install --json` | Display progress bar, stream `type: "progress"` messages | -| Device picker | `maui device list --json` (future) | Populate device dropdown / selection UI | +| Device picker | `maui device list --json` | Populate device dropdown / selection UI | | Emulator launch | `maui android emulator start --json` | Show notification, update device list on completion | ### Benefits @@ -440,12 +606,11 @@ simulators) are now included above. These were inspired by: Future commands: -- `maui device list` for unified device listing - `maui logs` for viewing console output - `maui tree` for displaying the visual tree - `maui screenshot` for capturing screenshots -**Decision**: Environment setup ships first. Device listing and app inspection commands +**Decision**: Environment setup and device listing ship first. App inspection commands follow in a future release. ## References @@ -454,9 +619,13 @@ follow in a future release. - [dotnet run for .NET MAUI specification][dotnet-run-spec] - [Workload manifest specification][workload-spec] - [AppleDev.Tools][appledev-tools] - Wraps simctl and devicectl commands +- [ComputeAvailableDevices (Android)][compute-android] - Android workload MSBuild target +- [ComputeAvailableDevices (Apple)][compute-apple] - Apple workload MSBuild target - [System.CommandLine documentation](https://learn.microsoft.com/dotnet/standard/commandline/) - [Android Debug Bridge (ADB)](https://developer.android.com/studio/command-line/adb) - [simctl command-line tool](https://nshipster.com/simctl/) [vibe-wpf]: https://github.com/jonathanpeppers/vibe-wpf [appledev-tools]: https://github.com/Redth/AppleDev.Tools +[compute-android]: https://github.com/dotnet/android/blob/main/Documentation/docs-mobile/building-apps/build-targets.md#computeavailabledevices +[compute-apple]: https://github.com/dotnet/macios/blob/main/docs/building-apps/build-targets.md#computeavailabledevices From 148b9aa549e646b5b59fd619a0357a6419cce7a0 Mon Sep 17 00:00:00 2001 From: Stephane Delcroix Date: Thu, 2 Apr 2026 14:41:39 +0200 Subject: [PATCH 07/12] Fix x:Key values not escaped in source-generated C# string literals (#34727) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Fixes #34726 The XAML source generator interpolates `x:Key` values directly into generated C# string literals without escaping special characters. If an `x:Key` contains a double quote (`"`), backslash (`\`), or control character, the generated C# is syntactically invalid. ## Changes - **`SetPropertyHelpers.cs`** — Escape the `x:Key` value via `CSharpExpressionHelpers.EscapeForString()` before interpolating into the generated `resources["..."] = ...` assignment. - **`KnownMarkups.cs`** — Same fix for `DynamicResource` key emission (`new DynamicResource("...")`). - **`CSharpExpressionHelpers.cs`** — Changed `EscapeForString` from `private static` to `internal static` so it can be reused from `SetPropertyHelpers` and `KnownMarkups`. ## Test Added `Maui34726.xaml` / `.xaml.cs` XAML unit test with `x:Key` values containing double quotes and backslashes: - **SourceGen path**: Verifies the generated code compiles without diagnostics - **Runtime/XamlC paths**: Verifies the resources are accessible by their original (unescaped) key names Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/SourceGen/CSharpExpressionHelpers.cs | 86 +++++++++++-------- src/Controls/src/SourceGen/KnownMarkups.cs | 2 +- .../src/SourceGen/SetPropertyHelpers.cs | 67 ++++++++------- .../Xaml.UnitTests/Issues/Maui34726.xaml | 12 +++ .../Xaml.UnitTests/Issues/Maui34726.xaml.cs | 44 ++++++++++ 5 files changed, 140 insertions(+), 71 deletions(-) create mode 100644 src/Controls/tests/Xaml.UnitTests/Issues/Maui34726.xaml create mode 100644 src/Controls/tests/Xaml.UnitTests/Issues/Maui34726.xaml.cs diff --git a/src/Controls/src/SourceGen/CSharpExpressionHelpers.cs b/src/Controls/src/SourceGen/CSharpExpressionHelpers.cs index f6a1cfc22bbf..9d973b0058aa 100644 --- a/src/Controls/src/SourceGen/CSharpExpressionHelpers.cs +++ b/src/Controls/src/SourceGen/CSharpExpressionHelpers.cs @@ -94,11 +94,11 @@ public static bool IsExplicitExpression(string? value) { if (string.IsNullOrEmpty(value)) return false; - + var trimmed = value!.Trim(); - return trimmed.Length > 3 - && trimmed[0] == '{' - && trimmed[1] == '=' + return trimmed.Length > 3 + && trimmed[0] == '{' + && trimmed[1] == '=' && trimmed[trimmed.Length - 1] == '}'; } @@ -147,8 +147,8 @@ public static bool IsImplicitExpression(string? value, Func? canRe foreach (var op in CSharpOperators) { // Use case-insensitive for word-based aliases (AND, OR, LT, GT, LTE, GTE) - var comparison = (op == " AND " || op == " OR " || op == " LT " || op == " GT " || op == " LTE " || op == " GTE ") - ? StringComparison.OrdinalIgnoreCase + var comparison = (op == " AND " || op == " OR " || op == " LT " || op == " GT " || op == " LTE " || op == " GTE ") + ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal; if (trimmed.IndexOf(op, comparison) >= 0) return true; @@ -176,7 +176,7 @@ static bool StartsWithMarkupExtension(string trimmed, Func? canRes return false; var identifier = trimmed.Substring(start, end - start); - + // Handle prefixed identifiers like x:Type or local:MyExtension var colonIndex = identifier.IndexOf(':'); if (colonIndex >= 0) @@ -210,10 +210,10 @@ internal readonly struct BareIdentifierResult { ///

True if should be treated as C# expression, false for markup extension. public bool IsExpression { get; init; } - + /// True if both markup extension and property exist (ambiguous). public bool IsAmbiguous { get; init; } - + /// The bare identifier name (for diagnostic reporting). public string? Name { get; init; } } @@ -274,11 +274,11 @@ public static BareIdentifierResult ClassifyExpression(string? markupString, Func if (isMarkup) { // Markup extension wins (backward compatible), but check for ambiguity - return new BareIdentifierResult - { - IsExpression = false, - IsAmbiguous = isProperty, - Name = name + return new BareIdentifierResult + { + IsExpression = false, + IsAmbiguous = isProperty, + Name = name }; } @@ -315,7 +315,7 @@ public static (string? prefix, string name) ParseBareIdentifier(string value) var match = BareIdentifierPattern.Match(value.Trim()); if (!match.Success) return (null, value); - + var prefix = match.Groups[1].Success ? match.Groups[1].Value : null; var name = match.Groups[2].Value; return (prefix, name); @@ -350,7 +350,7 @@ public static (string? prefix, string name) ParseBareIdentifier(string value) static bool IsKnownMarkupExtension(string name) { - return KnownMarkupExtensions.Contains(name) + return KnownMarkupExtensions.Contains(name) || KnownMarkupExtensions.Contains(name + "Extension"); } @@ -363,7 +363,7 @@ static bool IsKnownMarkupExtension(string name) public static string GetExpressionCode(string value) { var trimmed = value.Trim(); - + // Remove outer braces string code; if (IsExplicitExpression(value)) @@ -394,17 +394,17 @@ static string TransformOperatorAliases(string code) { // Replace word-based aliases with C# operators (case-insensitive, with spaces) var result = code; - + // Logical operators result = ReplaceWordOperator(result, " AND ", " && "); result = ReplaceWordOperator(result, " OR ", " || "); - + // Comparison operators (must do multi-char first to avoid partial replacements) result = ReplaceWordOperator(result, " LTE ", " <= "); result = ReplaceWordOperator(result, " GTE ", " >= "); result = ReplaceWordOperator(result, " LT ", " < "); result = ReplaceWordOperator(result, " GT ", " > "); - + return result; } @@ -415,7 +415,7 @@ static string ReplaceWordOperator(string code, string word, string replacement) { var result = new StringBuilder(); int i = 0; - + while (i < code.Length) { // Skip string literals (single or double quoted) @@ -445,7 +445,7 @@ static string ReplaceWordOperator(string code, string word, string replacement) } continue; } - + // Check for word match (case-insensitive) if (i + word.Length <= code.Length) { @@ -457,11 +457,11 @@ static string ReplaceWordOperator(string code, string word, string replacement) continue; } } - + result.Append(code[i]); i++; } - + return result.ToString(); } @@ -523,7 +523,7 @@ static string TransformQuotes(string code) if (i < code.Length && code[i] == '\'') { i++; // Skip closing quote - + var contentStr = content.ToString(); // Always convert to string literal (double quotes) @@ -542,7 +542,7 @@ static string TransformQuotes(string code) { backslashCount++; } - + if (backslashCount % 2 == 1) { // Odd backslashes: quote is already escaped @@ -609,10 +609,10 @@ public static string TransformQuotesWithSemantics(string code, Compilation compi foreach (var literal in stringLiterals) { var expectedType = DetermineExpectedType(literal, compilation, contextTypes); - + // If expected type is char, convert back to char literal bool shouldBeChar = expectedType?.SpecialType == SpecialType.System_Char; - + if (shouldBeChar) { // Get the string content and create a char literal @@ -648,7 +648,7 @@ static string EscapeForChar(string value) { if (value.Length != 1) return value; - + return value[0] switch { '\'' => "\\'", @@ -668,7 +668,7 @@ static string EscapeForChar(string value) { // Walk up to find the context var parent = literal.Parent; - + while (parent != null) { switch (parent) @@ -766,19 +766,31 @@ static string EscapeForChar(string value) /// /// Escapes a string for use in a C# string literal. /// - static string EscapeForString(string value) + internal static string EscapeForString(string value) { var sb = new StringBuilder(); foreach (var c in value) { switch (c) { - case '"': sb.Append("\\\""); break; - case '\\': sb.Append("\\\\"); break; - case '\n': sb.Append("\\n"); break; - case '\r': sb.Append("\\r"); break; - case '\t': sb.Append("\\t"); break; - default: sb.Append(c); break; + case '"': + sb.Append("\\\""); + break; + case '\\': + sb.Append("\\\\"); + break; + case '\n': + sb.Append("\\n"); + break; + case '\r': + sb.Append("\\r"); + break; + case '\t': + sb.Append("\\t"); + break; + default: + sb.Append(c); + break; } } return sb.ToString(); diff --git a/src/Controls/src/SourceGen/KnownMarkups.cs b/src/Controls/src/SourceGen/KnownMarkups.cs index c9ade31c9a2a..5efec5a76147 100644 --- a/src/Controls/src/SourceGen/KnownMarkups.cs +++ b/src/Controls/src/SourceGen/KnownMarkups.cs @@ -271,7 +271,7 @@ public static bool ProvideValueForDynamicResourceExtension(ElementNode markupNod if (key is null) throw new Exception(); - value = $"new global::Microsoft.Maui.Controls.Internals.DynamicResource(\"{key}\")"; + value = $"new global::Microsoft.Maui.Controls.Internals.DynamicResource(\"{CSharpExpressionHelpers.EscapeForString(key)}\")"; return true; } diff --git a/src/Controls/src/SourceGen/SetPropertyHelpers.cs b/src/Controls/src/SourceGen/SetPropertyHelpers.cs index a1c2897ca310..9d098dbace03 100644 --- a/src/Controls/src/SourceGen/SetPropertyHelpers.cs +++ b/src/Controls/src/SourceGen/SetPropertyHelpers.cs @@ -19,7 +19,7 @@ public static void SetPropertyValue(IndentedTextWriter writer, ILocalValue paren if (propertyName.Equals(XmlName._CreateContent)) return; //already handled - + //TODO I believe ContentProperty should be resolved here var localName = propertyName.LocalName; bool attached = false; @@ -144,7 +144,8 @@ public static void AddToResourceDictionary(IndentedTextWriter writer, ILocalValu context.KeysInRD[parentVar] = []; context.KeysInRD[parentVar].Add((((ValueNode)keyNode).Value as string)!); var key = ((ValueNode)keyNode).Value as string; - writer.WriteLine($"{parentVar.ValueAccessor}[\"{key}\"] = {(getNodeValue(node, context.Compilation.ObjectType)).ValueAccessor};"); + var escapedKey = CSharpExpressionHelpers.EscapeForString(key!); + writer.WriteLine($"{parentVar.ValueAccessor}[\"{escapedKey}\"] = {(getNodeValue(node, context.Compilation.ObjectType)).ValueAccessor};"); return; } writer.WriteLine($"{parentVar.ValueAccessor}.Add({getNodeValue(node, context.Compilation.ObjectType).ValueAccessor});"); @@ -283,7 +284,7 @@ static void ConnectEvent(IndentedTextWriter writer, ILocalValue parentVar, strin static bool CanSetValue(IFieldSymbol? bpFieldSymbol, INode node, ITypeSymbol parentType, string localName, SourceGenContext context, NodeSGExtensions.GetNodeValueDelegate getNodeValue, out string? explicitPropertyName) { explicitPropertyName = null; - + if (bpFieldSymbol != null) { // Normal BP case - apply existing logic @@ -311,13 +312,13 @@ static bool CanSetValue(IFieldSymbol? bpFieldSymbol, INode node, ITypeSymbol par if (localVar.Type.InheritsFrom(bpTypeAndConverter?.type!, context)) return true; - + if (bpFieldSymbol.Type.IsInterface() && localVar.Type.Implements(bpTypeAndConverter?.type!)) return true; return false; } - + // Heuristic: If BP is null but the type has a property/field with a BindablePropertyAttribute, // assume the BP will be generated by another source generator // Only apply this for non-BindingBase nodes (CanSetBinding handles BindingBase) @@ -325,25 +326,25 @@ static bool CanSetValue(IFieldSymbol? bpFieldSymbol, INode node, ITypeSymbol par { return parentType.HasBindablePropertyHeuristic(localName, context, out explicitPropertyName); } - + return false; } static void SetValue(IndentedTextWriter writer, ILocalValue parentVar, IFieldSymbol? bpFieldSymbol, string localName, string? explicitPropertyName, INode node, SourceGenContext context, NodeSGExtensions.GetNodeValueDelegate getNodeValue) { // Determine bindable property name: use BP field symbol if available, otherwise use heuristic - var bpName = bpFieldSymbol != null - ? bpFieldSymbol.ToFQDisplayString() + var bpName = bpFieldSymbol != null + ? bpFieldSymbol.ToFQDisplayString() : $"{parentVar.Type.ToFQDisplayString()}.{explicitPropertyName ?? $"{localName}Property"}"; - + var pType = bpFieldSymbol?.GetBPTypeAndConverter(context)?.type; var property = bpFieldSymbol == null ? parentVar.Type.GetAllProperties(localName, context).FirstOrDefault() : null; - + if (node is ValueNode valueNode) { using (context.ProjectItem.EnableLineInfo ? PrePost.NewLineInfo(writer, (IXmlLineInfo)node, context.ProjectItem) : PrePost.NoBlock()) { - var valueString = bpFieldSymbol != null + var valueString = bpFieldSymbol != null ? valueNode.ConvertTo(bpFieldSymbol, writer, context, parentVar) : (property != null ? valueNode.ConvertTo(property, writer, context, parentVar) : getNodeValue(node, context.Compilation.ObjectType).ValueAccessor); writer.WriteLine($"{parentVar.ValueAccessor}.SetValue({bpName}, {valueString});"); @@ -355,7 +356,7 @@ static void SetValue(IndentedTextWriter writer, ILocalValue parentVar, IFieldSym { var localVar = getNodeValue(elementNode, context.Compilation.ObjectType); var cast = string.Empty; - + if (bpFieldSymbol != null) { // BP case: check for double implicit conversion first @@ -376,7 +377,7 @@ static void SetValue(IndentedTextWriter writer, ILocalValue parentVar, IFieldSym { cast = $"({property.Type.ToFQDisplayString()})"; } - + writer.WriteLine($"{parentVar.ValueAccessor}.SetValue({bpName}, {cast}{localVar.ValueAccessor});"); } } @@ -456,7 +457,7 @@ .. toType.GetMembers().OfType().Where(m => m.MethodKind == Method { // Check if this conversion operator can convert fromType to toType if (SymbolEqualityComparer.Default.Equals(conversionOp.Parameters[0].Type, fromType) && - SymbolEqualityComparer.Default.Equals(conversionOp.ReturnType, toType)) + SymbolEqualityComparer.Default.Equals(conversionOp.ReturnType, toType)) { return true; } @@ -468,15 +469,15 @@ .. toType.GetMembers().OfType().Where(m => m.MethodKind == Method { var fromIsCollection = fromType.AllInterfaces.Any(i => i.ToString() == "System.Collections.IEnumerable") && fromType.SpecialType != SpecialType.System_String; var toIsCollection = toType.AllInterfaces.Any(i => i.ToString() == "System.Collections.IEnumerable") && toType.SpecialType != SpecialType.System_String; - + // Both must be collections, or both must be non-collections if (fromIsCollection == toIsCollection) { // Same inheritance chain or one is an interface if (fromType.InheritsFrom(toType, context) || - toType.InheritsFrom(fromType, context) || - toType.TypeKind == TypeKind.Interface || - fromType.TypeKind == TypeKind.Interface) + toType.InheritsFrom(fromType, context) || + toType.TypeKind == TypeKind.Interface || + fromType.TypeKind == TypeKind.Interface) { return true; } @@ -510,27 +511,27 @@ static void Set(IndentedTextWriter writer, ILocalValue parentVar, string localNa static bool CanSetBinding(IFieldSymbol? bpFieldSymbol, INode node, ITypeSymbol parentType, string localName, SourceGenContext context, out string? explicitPropertyName) { explicitPropertyName = null; - + // Check if it's a BindingBase node if (!IsBindingBaseNode(node, context)) return false; - + // If we have a BP field symbol, we can set binding if (bpFieldSymbol != null) return true; - + // Heuristic: If BP is null but the type has a property/field with a BindablePropertyAttribute, // assume the BP will be generated by another source generator if (!string.IsNullOrEmpty(localName)) return parentType.HasBindablePropertyHeuristic(localName, context, out explicitPropertyName); - + return false; } static void SetBinding(IndentedTextWriter writer, ILocalValue parentVar, IFieldSymbol? bpFieldSymbol, string localName, string? explicitPropertyName, INode node, SourceGenContext context, NodeSGExtensions.GetNodeValueDelegate getNodeValue) { var localVariable = getNodeValue((ElementNode)node, context.Compilation.ObjectType); - + if (bpFieldSymbol != null) { // Normal case: we have the BP field symbol @@ -582,9 +583,9 @@ static void Add(IndentedTextWriter writer, ILocalValue parentVar, XmlName proper if (localName != null) //one of those will return true, but we need the propertyType - _ = CanGetValue(parentVar, bpFieldSymbol, attached, context, out propertyType) || CanGet(parentVar, localName, context, out propertyType, out propertySymbol); - - else + _ = CanGetValue(parentVar, bpFieldSymbol, attached, context, out propertyType) || CanGet(parentVar, localName, context, out propertyType, out propertySymbol); + + else propertyType = parentVar.Type; if (CanAddToResourceDictionary(parentVar, propertyType!, (ElementNode)valueNode, context, getNodeValue)) @@ -594,7 +595,7 @@ static void Add(IndentedTextWriter writer, ILocalValue parentVar, XmlName proper rdAccessor = new DirectValue(propertyType!, GetOrGetValue(parentVar, bpFieldSymbol, propertySymbol, valueNode, context)); else rdAccessor = parentVar; - + AddToResourceDictionary(writer, rdAccessor, (ElementNode)valueNode, context, getNodeValue); return; } @@ -684,7 +685,7 @@ static bool TryHandleExpressionBinding(IndentedTextWriter writer, ILocalValue pa context.ReportDiagnostic(Diagnostic.Create(Descriptors.AmbiguousMemberExpression, bothLocation, resolution.RootIdentifier, context.RootType?.Name ?? "this", dataTypeSymbol.Name)); return true; // Handled (with error) } - + // Warn if member name conflicts with a well-known static type if (resolution.ConflictsWithStaticType) { @@ -695,8 +696,8 @@ static bool TryHandleExpressionBinding(IndentedTextWriter writer, ILocalValue pa } // Handle not-found case for simple identifiers - if (resolution.Location == MemberLocation.Neither && - !string.IsNullOrEmpty(resolution.RootIdentifier) && + if (resolution.Location == MemberLocation.Neither && + !string.IsNullOrEmpty(resolution.RootIdentifier) && MemberResolver.IsSimpleIdentifier(expression.Code)) { var neitherLocation = LocationCreate(context.ProjectItem.RelativePath!, (IXmlLineInfo)valueNode, expression.Code); @@ -741,7 +742,7 @@ static bool TryHandleExpressionBinding(IndentedTextWriter writer, ILocalValue pa static void SetExpressionBinding(IndentedTextWriter writer, ILocalValue parentVar, IFieldSymbol bpFieldSymbol, string expression, ITypeSymbol dataTypeSymbol, SourceGenContext context, ValueNode valueNode) { var bpName = bpFieldSymbol.ToFQDisplayString(); - + var sourceTypeName = dataTypeSymbol.ToFQDisplayString(); // Transform quotes with semantic context - char literals stay as char only if target expects char @@ -786,7 +787,7 @@ static void SetExpressionBinding(IndentedTextWriter writer, ILocalValue parentVa if (getterExpression.Contains("?.")) getterExpression += "!"; writer.WriteLine($"__source => ({getterExpression}, true),"); - + // Generate setter if expression is a simple property chain AND the terminal property is writable if (analysis.IsSettable && IsExpressionWritable(expression, dataTypeSymbol, context)) { @@ -806,7 +807,7 @@ static void SetExpressionBinding(IndentedTextWriter writer, ILocalValue parentVa bpFieldSymbol.Name)); } } - + // Generate handlers array if (handlers.Count == 0) { diff --git a/src/Controls/tests/Xaml.UnitTests/Issues/Maui34726.xaml b/src/Controls/tests/Xaml.UnitTests/Issues/Maui34726.xaml new file mode 100644 index 000000000000..d601f6b33dd0 --- /dev/null +++ b/src/Controls/tests/Xaml.UnitTests/Issues/Maui34726.xaml @@ -0,0 +1,12 @@ + + + + + Red + Blue + Green + + + diff --git a/src/Controls/tests/Xaml.UnitTests/Issues/Maui34726.xaml.cs b/src/Controls/tests/Xaml.UnitTests/Issues/Maui34726.xaml.cs new file mode 100644 index 000000000000..82d581cb5864 --- /dev/null +++ b/src/Controls/tests/Xaml.UnitTests/Issues/Maui34726.xaml.cs @@ -0,0 +1,44 @@ +using Xunit; +using static Microsoft.Maui.Controls.Xaml.UnitTests.MockSourceGenerator; + +namespace Microsoft.Maui.Controls.Xaml.UnitTests; + +public partial class Maui34726 : ContentPage +{ + public Maui34726() => InitializeComponent(); + + [Collection("Issue")] + public class Tests + { + [Theory] + [XamlInflatorData] + internal void XKeyWithSpecialCharsProducesValidCode(XamlInflator inflator) + { + if (inflator == XamlInflator.SourceGen) + { + var result = CreateMauiCompilation() + .WithAdditionalSource( +""" +namespace Microsoft.Maui.Controls.Xaml.UnitTests; + +[XamlProcessing(XamlInflator.Runtime, true)] +public partial class Maui34726 : ContentPage +{ + public Maui34726() => InitializeComponent(); +} +""") + .RunMauiSourceGenerator(typeof(Maui34726)); + Assert.Empty(result.Diagnostics); + } + else + { + var page = new Maui34726(inflator); + Assert.NotNull(page); + Assert.Equal(3, page.Resources.Count); + Assert.True(page.Resources.ContainsKey("Key\"Quote")); + Assert.True(page.Resources.ContainsKey("Key\\Backslash")); + Assert.True(page.Resources.ContainsKey("SimpleKey")); + } + } + } +} From 7344067498c771c8625c796d478ce4aeb8c53818 Mon Sep 17 00:00:00 2001 From: Jakub Florkowski Date: Thu, 25 Dec 2025 00:45:38 +0100 Subject: [PATCH 08/12] Fix crash when displaying alerts on unloaded pages Added null checks for the Window property in DisplayAlertAsync, DisplayActionSheetAsync, and DisplayPromptAsync methods in Page.cs to prevent NullReferenceException when the page is no longer attached to a window. New UI tests verify that async alert requests do not crash the app after navigating away from the page. --- src/Controls/src/Core/Page/Page.cs | 52 ++++++- .../Issues/Issue33287.xaml.cs | 139 ++++++++++++++++++ .../Tests/Issues/Issue33287.cs | 42 ++++++ 3 files changed, 230 insertions(+), 3 deletions(-) create mode 100644 src/Controls/tests/TestCases.HostApp/Issues/Issue33287.xaml.cs create mode 100644 src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs diff --git a/src/Controls/src/Core/Page/Page.cs b/src/Controls/src/Core/Page/Page.cs index d4ceedf58f99..49ac4af53e35 100644 --- a/src/Controls/src/Core/Page/Page.cs +++ b/src/Controls/src/Core/Page/Page.cs @@ -323,10 +323,26 @@ public Task DisplayActionSheetAsync(string title, string cancel, string var args = new ActionSheetArguments(title, cancel, destruction, buttons); args.FlowDirection = flowDirection; + + // If page is no longer attached to a window (e.g., navigated away), ignore the action sheet request + if (Window is null) + { + // Complete the task with cancel result + args.SetResult(cancel); + return args.Result.Task; + } + if (IsPlatformEnabled) Window.AlertManager.RequestActionSheet(this, args); else - _pendingActions.Add(() => Window.AlertManager.RequestActionSheet(this, args)); + _pendingActions.Add(() => + { + // Check again in case window was detached while waiting + if (Window is not null) + Window.AlertManager.RequestActionSheet(this, args); + else + args.SetResult(cancel); + }); return args.Result.Task; } @@ -384,10 +400,25 @@ public Task DisplayAlertAsync(string title, string message, string accept, var args = new AlertArguments(title, message, accept, cancel); args.FlowDirection = flowDirection; + // If page is no longer attached to a window (e.g., navigated away), ignore the alert request + if (Window is null) + { + // Complete the task with default result (cancel) + args.SetResult(false); + return args.Result.Task; + } + if (IsPlatformEnabled) Window.AlertManager.RequestAlert(this, args); else - _pendingActions.Add(() => Window.AlertManager.RequestAlert(this, args)); + _pendingActions.Add(() => + { + // Check again in case window was detached while waiting + if (Window is not null) + Window.AlertManager.RequestAlert(this, args); + else + args.SetResult(false); + }); return args.Result.Task; } @@ -408,10 +439,25 @@ public Task DisplayAlertAsync(string title, string message, string accept, { var args = new PromptArguments(title, message, accept, cancel, placeholder, maxLength, keyboard, initialValue); + // If page is no longer attached to a window (e.g., navigated away), ignore the prompt request + if (Window is null) + { + // Complete the task with null result + args.SetResult(null); + return args.Result.Task; + } + if (IsPlatformEnabled) Window.AlertManager.RequestPrompt(this, args); else - _pendingActions.Add(() => Window.AlertManager.RequestPrompt(this, args)); + _pendingActions.Add(() => + { + // Check again in case window was detached while waiting + if (Window is not null) + Window.AlertManager.RequestPrompt(this, args); + else + args.SetResult(null); + }); return args.Result.Task; } diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.xaml.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.xaml.cs new file mode 100644 index 000000000000..c76767bc5064 --- /dev/null +++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.xaml.cs @@ -0,0 +1,139 @@ +using System; +using System.Threading.Tasks; + +namespace Maui.Controls.Sample.Issues; + +[Issue(IssueTracker.Github, 33287, "DisplayAlertAsync throws NullReferenceException when page is no longer displayed", PlatformAffected.All)] +public partial class Issue33287 : NavigationPage +{ + public Issue33287() : base(new Issue33287MainPage()) + { + } +} + +public partial class Issue33287MainPage : ContentPage +{ + public Issue33287MainPage() + { + Title = "Issue 33287"; + + var layout = new VerticalStackLayout + { + Padding = 20, + Spacing = 10 + }; + + layout.Children.Add(new Label + { + Text = "DisplayAlertAsync NullReferenceException Test", + FontSize = 18, + FontAttributes = FontAttributes.Bold + }); + + layout.Children.Add(new Label + { + Text = "1. Tap 'Navigate to Second Page'\n2. Immediately tap 'Go Back'\n3. Wait 5 seconds - should NOT crash", + TextColor = Colors.Gray + }); + + layout.Children.Add(new Button + { + Text = "Navigate to Second Page", + AutomationId = "NavigateButton", + Command = new Command(async () => + { + Console.WriteLine("[Issue33287] Navigating to SecondPage"); + StatusLabel.Text = "Status: Navigating..."; + await Navigation.PushAsync(new Issue33287SecondPage(this)); + }) + }); + + StatusLabel = new Label + { + Text = "Status: Ready", + AutomationId = "StatusLabel", + FontAttributes = FontAttributes.Bold + }; + layout.Children.Add(StatusLabel); + + Content = layout; + } + + public Label StatusLabel { get; private set; } + + public void UpdateStatus(string status) + { + StatusLabel.Text = $"Status: {status}"; + Console.WriteLine($"[Issue33287] {status}"); + } +} + +public class Issue33287SecondPage : ContentPage +{ + private readonly Issue33287MainPage _mainPage; + + public Issue33287SecondPage(Issue33287MainPage mainPage) + { + _mainPage = mainPage; + Title = "Second Page"; + + var layout = new VerticalStackLayout + { + Padding = 20, + Spacing = 10 + }; + + layout.Children.Add(new Label + { + Text = "Second Page - Tap 'Go Back' quickly!", + FontSize = 18, + FontAttributes = FontAttributes.Bold + }); + + layout.Children.Add(new Button + { + Text = "Go Back", + AutomationId = "GoBackButton", + Command = new Command(async () => + { + Console.WriteLine("[Issue33287] Going back..."); + await Navigation.PopAsync(); + }) + }); + + Content = layout; + } + + protected override async void OnAppearing() + { + base.OnAppearing(); + + Console.WriteLine("[Issue33287] SecondPage.OnAppearing - Starting 5 second delay"); + _mainPage?.UpdateStatus("On second page, waiting 5 seconds..."); + + // Delay before showing alert + await Task.Delay(5000); + + Console.WriteLine("[Issue33287] Attempting to show DisplayAlertAsync"); + _mainPage?.UpdateStatus("Showing alert from unloaded page..."); + + try + { + // This should cause NullReferenceException if page is no longer displayed + await DisplayAlertAsync("Test Alert", "This alert was delayed", "OK"); + + Console.WriteLine("[Issue33287] ✅ DisplayAlertAsync succeeded (page still loaded or fix applied)"); + _mainPage?.UpdateStatus("✅ Alert shown successfully"); + } + catch (NullReferenceException ex) + { + Console.WriteLine($"[Issue33287] ❌ NullReferenceException caught: {ex.Message}"); + _mainPage?.UpdateStatus("❌ NullReferenceException occurred!"); + } + catch (Exception ex) + { + Console.WriteLine($"[Issue33287] ❌ Exception: {ex.GetType().Name}: {ex.Message}"); + _mainPage?.UpdateStatus($"❌ Exception: {ex.GetType().Name}"); + } + } +} diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs new file mode 100644 index 000000000000..c3700872c1cb --- /dev/null +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs @@ -0,0 +1,42 @@ +using NUnit.Framework; +using UITest.Appium; +using UITest.Core; + +namespace Microsoft.Maui.TestCases.Tests.Issues; + +public class Issue33287 : _IssuesUITest +{ + public override string Issue => "DisplayAlertAsync throws NullReferenceException when page is no longer displayed"; + + public Issue33287(TestDevice device) : base(device) { } + + [Test] + [Category(UITestCategories.Page)] + public void DisplayAlertAsyncShouldNotCrashWhenPageUnloaded() + { + App.WaitForElement("NavigateButton"); + + // Navigate to second page + App.Tap("NavigateButton"); + + // Wait for second page to appear + App.WaitForElement("GoBackButton"); + + // Immediately go back before the 5 second delay completes + App.Tap("GoBackButton"); + + // Wait for navigation to complete + App.WaitForElement("StatusLabel"); + + // Wait for the delayed DisplayAlertAsync to be triggered (5 seconds + buffer) + System.Threading.Thread.Sleep(6000); + + // Get the status - should not show NullReferenceException + var status = App.FindElement("StatusLabel").GetText(); + Console.WriteLine($"[TEST] Final status: {status}"); + + // Assert that no NullReferenceException occurred + Assert.That(status, Does.Not.Contain("NullReferenceException"), + "DisplayAlertAsync should not throw NullReferenceException when page is unloaded"); + } +} From a7a7dfdc8d66612415b973311aa711c64b32f51b Mon Sep 17 00:00:00 2001 From: Jakub Florkowski Date: Fri, 26 Dec 2025 15:03:41 +0100 Subject: [PATCH 09/12] Add trace log for null Window in action sheet Added a Trace.WriteLine statement to log when DisplayActionSheetAsync is called but the page is not attached to a window. This helps with debugging scenarios where the action sheet is not shown due to the page being detached. --- src/Controls/src/Core/Page/Page.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Controls/src/Core/Page/Page.cs b/src/Controls/src/Core/Page/Page.cs index 49ac4af53e35..6b29785e9257 100644 --- a/src/Controls/src/Core/Page/Page.cs +++ b/src/Controls/src/Core/Page/Page.cs @@ -327,7 +327,7 @@ public Task DisplayActionSheetAsync(string title, string cancel, string // If page is no longer attached to a window (e.g., navigated away), ignore the action sheet request if (Window is null) { - // Complete the task with cancel result + Trace.WriteLine("DisplayActionSheetAsync: Window is null, action sheet will not be shown. This can happen if the page is not attached to a window."); args.SetResult(cancel); return args.Result.Task; } From 30990375dd512a7fb1eacb9f2c7d237f57650b59 Mon Sep 17 00:00:00 2001 From: Jakub Florkowski Date: Thu, 2 Apr 2026 15:59:24 +0200 Subject: [PATCH 10/12] Address review feedback: fix race condition, preserve queuing, improve tests - Capture Window in local variable to fix TOCTOU race (Copilot review) - Only early-return when IsPlatformEnabled && Window is null to preserve pending action queuing for not-yet-attached pages (Copilot review) - Add consistent Trace.WriteLine logging across all three methods (pictos) - Assert positive success (contains) instead of absence of error (PureWeen) - Replace Thread.Sleep with polling loop for deterministic wait (Copilot review) - Rename .xaml.cs to .cs and remove partial keywords (no XAML file) (Copilot review) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Controls/src/Core/Page/Page.cs | 84 ++++++----- .../TestCases.HostApp/Issues/Issue33287.cs | 107 ++++++++++++++ .../Issues/Issue33287.xaml.cs | 139 ------------------ .../Tests/Issues/Issue33287.cs | 36 ++--- 4 files changed, 177 insertions(+), 189 deletions(-) create mode 100644 src/Controls/tests/TestCases.HostApp/Issues/Issue33287.cs delete mode 100644 src/Controls/tests/TestCases.HostApp/Issues/Issue33287.xaml.cs diff --git a/src/Controls/src/Core/Page/Page.cs b/src/Controls/src/Core/Page/Page.cs index 6b29785e9257..2c760ff1996d 100644 --- a/src/Controls/src/Core/Page/Page.cs +++ b/src/Controls/src/Core/Page/Page.cs @@ -324,25 +324,31 @@ public Task DisplayActionSheetAsync(string title, string cancel, string args.FlowDirection = flowDirection; - // If page is no longer attached to a window (e.g., navigated away), ignore the action sheet request - if (Window is null) + var window = Window; + if (IsPlatformEnabled) { - Trace.WriteLine("DisplayActionSheetAsync: Window is null, action sheet will not be shown. This can happen if the page is not attached to a window."); - args.SetResult(cancel); - return args.Result.Task; + if (window is null) + { + Trace.WriteLine("DisplayActionSheetAsync: Window is null, action sheet will not be shown. This can happen if the page is not attached to a window."); + args.SetResult(cancel); + return args.Result.Task; + } + window.AlertManager.RequestActionSheet(this, args); } - - if (IsPlatformEnabled) - Window.AlertManager.RequestActionSheet(this, args); else + { _pendingActions.Add(() => { - // Check again in case window was detached while waiting - if (Window is not null) - Window.AlertManager.RequestActionSheet(this, args); + var w = Window; + if (w is not null) + w.AlertManager.RequestActionSheet(this, args); else + { + Trace.WriteLine("DisplayActionSheetAsync: Window is null, action sheet will not be shown. This can happen if the page is not attached to a window."); args.SetResult(cancel); + } }); + } return args.Result.Task; } @@ -400,25 +406,31 @@ public Task DisplayAlertAsync(string title, string message, string accept, var args = new AlertArguments(title, message, accept, cancel); args.FlowDirection = flowDirection; - // If page is no longer attached to a window (e.g., navigated away), ignore the alert request - if (Window is null) + var window = Window; + if (IsPlatformEnabled) { - // Complete the task with default result (cancel) - args.SetResult(false); - return args.Result.Task; + if (window is null) + { + Trace.WriteLine("DisplayAlertAsync: Window is null, alert will not be shown. This can happen if the page is not attached to a window."); + args.SetResult(false); + return args.Result.Task; + } + window.AlertManager.RequestAlert(this, args); } - - if (IsPlatformEnabled) - Window.AlertManager.RequestAlert(this, args); else + { _pendingActions.Add(() => { - // Check again in case window was detached while waiting - if (Window is not null) - Window.AlertManager.RequestAlert(this, args); + var w = Window; + if (w is not null) + w.AlertManager.RequestAlert(this, args); else + { + Trace.WriteLine("DisplayAlertAsync: Window is null, alert will not be shown. This can happen if the page is not attached to a window."); args.SetResult(false); + } }); + } return args.Result.Task; } @@ -439,25 +451,31 @@ public Task DisplayAlertAsync(string title, string message, string accept, { var args = new PromptArguments(title, message, accept, cancel, placeholder, maxLength, keyboard, initialValue); - // If page is no longer attached to a window (e.g., navigated away), ignore the prompt request - if (Window is null) + var window = Window; + if (IsPlatformEnabled) { - // Complete the task with null result - args.SetResult(null); - return args.Result.Task; + if (window is null) + { + Trace.WriteLine("DisplayPromptAsync: Window is null, prompt will not be shown. This can happen if the page is not attached to a window."); + args.SetResult(null); + return args.Result.Task; + } + window.AlertManager.RequestPrompt(this, args); } - - if (IsPlatformEnabled) - Window.AlertManager.RequestPrompt(this, args); else + { _pendingActions.Add(() => { - // Check again in case window was detached while waiting - if (Window is not null) - Window.AlertManager.RequestPrompt(this, args); + var w = Window; + if (w is not null) + w.AlertManager.RequestPrompt(this, args); else + { + Trace.WriteLine("DisplayPromptAsync: Window is null, prompt will not be shown. This can happen if the page is not attached to a window."); args.SetResult(null); + } }); + } return args.Result.Task; } diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.cs new file mode 100644 index 000000000000..cb90b47b5116 --- /dev/null +++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.cs @@ -0,0 +1,107 @@ +using System; +using System.Threading.Tasks; + +namespace Maui.Controls.Sample.Issues; + +[Issue(IssueTracker.Github, 33287, "DisplayAlertAsync throws NullReferenceException when page is no longer displayed", PlatformAffected.All)] +public class Issue33287 : NavigationPage +{ + public Issue33287() : base(new Issue33287MainPage()) + { + } +} + +public class Issue33287MainPage : ContentPage +{ + public Issue33287MainPage() + { + Title = "Issue 33287"; + + var statusLabel = new Label + { + Text = "Status: Ready", + AutomationId = "StatusLabel", + FontAttributes = FontAttributes.Bold + }; + + Content = new VerticalStackLayout + { + Padding = 20, + Spacing = 10, + Children = + { + new Label + { + Text = "DisplayAlertAsync NullReferenceException Test", + FontSize = 18, + FontAttributes = FontAttributes.Bold + }, + new Label + { + Text = "1. Tap 'Navigate to Second Page'\n2. Immediately tap 'Go Back'\n3. Wait 5 seconds - should NOT crash", + TextColor = Colors.Gray + }, + new Button + { + Text = "Navigate to Second Page", + AutomationId = "NavigateButton", + Command = new Command(async () => + { + statusLabel.Text = "Status: Navigating..."; + await Navigation.PushAsync(new Issue33287SecondPage(statusLabel)); + }) + }, + statusLabel + } + }; + } +} + +public class Issue33287SecondPage : ContentPage +{ + readonly Label _statusLabel; + + public Issue33287SecondPage(Label statusLabel) + { + _statusLabel = statusLabel; + Title = "Second Page"; + + Content = new VerticalStackLayout + { + Padding = 20, + Spacing = 10, + Children = + { + new Label + { + Text = "Second Page - Tap 'Go Back' quickly!", + FontSize = 18, + FontAttributes = FontAttributes.Bold + }, + new Button + { + Text = "Go Back", + AutomationId = "GoBackButton", + Command = new Command(async () => await Navigation.PopAsync()) + } + } + }; + } + + protected override async void OnAppearing() + { + base.OnAppearing(); + + await Task.Delay(5000); + + try + { + await DisplayAlertAsync("Test Alert", "This alert was delayed", "OK"); + _statusLabel.Text = "Status: ✅ Alert completed"; + } + catch (Exception ex) + { + _statusLabel.Text = $"Status: ❌ {ex.GetType().Name}"; + } + } +} diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.xaml.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.xaml.cs deleted file mode 100644 index c76767bc5064..000000000000 --- a/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.xaml.cs +++ /dev/null @@ -1,139 +0,0 @@ -using System; -using System.Threading.Tasks; - -namespace Maui.Controls.Sample.Issues; - -[Issue(IssueTracker.Github, 33287, "DisplayAlertAsync throws NullReferenceException when page is no longer displayed", PlatformAffected.All)] -public partial class Issue33287 : NavigationPage -{ - public Issue33287() : base(new Issue33287MainPage()) - { - } -} - -public partial class Issue33287MainPage : ContentPage -{ - public Issue33287MainPage() - { - Title = "Issue 33287"; - - var layout = new VerticalStackLayout - { - Padding = 20, - Spacing = 10 - }; - - layout.Children.Add(new Label - { - Text = "DisplayAlertAsync NullReferenceException Test", - FontSize = 18, - FontAttributes = FontAttributes.Bold - }); - - layout.Children.Add(new Label - { - Text = "1. Tap 'Navigate to Second Page'\n2. Immediately tap 'Go Back'\n3. Wait 5 seconds - should NOT crash", - TextColor = Colors.Gray - }); - - layout.Children.Add(new Button - { - Text = "Navigate to Second Page", - AutomationId = "NavigateButton", - Command = new Command(async () => - { - Console.WriteLine("[Issue33287] Navigating to SecondPage"); - StatusLabel.Text = "Status: Navigating..."; - await Navigation.PushAsync(new Issue33287SecondPage(this)); - }) - }); - - StatusLabel = new Label - { - Text = "Status: Ready", - AutomationId = "StatusLabel", - FontAttributes = FontAttributes.Bold - }; - layout.Children.Add(StatusLabel); - - Content = layout; - } - - public Label StatusLabel { get; private set; } - - public void UpdateStatus(string status) - { - StatusLabel.Text = $"Status: {status}"; - Console.WriteLine($"[Issue33287] {status}"); - } -} - -public class Issue33287SecondPage : ContentPage -{ - private readonly Issue33287MainPage _mainPage; - - public Issue33287SecondPage(Issue33287MainPage mainPage) - { - _mainPage = mainPage; - Title = "Second Page"; - - var layout = new VerticalStackLayout - { - Padding = 20, - Spacing = 10 - }; - - layout.Children.Add(new Label - { - Text = "Second Page - Tap 'Go Back' quickly!", - FontSize = 18, - FontAttributes = FontAttributes.Bold - }); - - layout.Children.Add(new Button - { - Text = "Go Back", - AutomationId = "GoBackButton", - Command = new Command(async () => - { - Console.WriteLine("[Issue33287] Going back..."); - await Navigation.PopAsync(); - }) - }); - - Content = layout; - } - - protected override async void OnAppearing() - { - base.OnAppearing(); - - Console.WriteLine("[Issue33287] SecondPage.OnAppearing - Starting 5 second delay"); - _mainPage?.UpdateStatus("On second page, waiting 5 seconds..."); - - // Delay before showing alert - await Task.Delay(5000); - - Console.WriteLine("[Issue33287] Attempting to show DisplayAlertAsync"); - _mainPage?.UpdateStatus("Showing alert from unloaded page..."); - - try - { - // This should cause NullReferenceException if page is no longer displayed - await DisplayAlertAsync("Test Alert", "This alert was delayed", "OK"); - - Console.WriteLine("[Issue33287] ✅ DisplayAlertAsync succeeded (page still loaded or fix applied)"); - _mainPage?.UpdateStatus("✅ Alert shown successfully"); - } - catch (NullReferenceException ex) - { - Console.WriteLine($"[Issue33287] ❌ NullReferenceException caught: {ex.Message}"); - _mainPage?.UpdateStatus("❌ NullReferenceException occurred!"); - } - catch (Exception ex) - { - Console.WriteLine($"[Issue33287] ❌ Exception: {ex.GetType().Name}: {ex.Message}"); - _mainPage?.UpdateStatus($"❌ Exception: {ex.GetType().Name}"); - } - } -} diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs index c3700872c1cb..a16ee61318ca 100644 --- a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs @@ -15,28 +15,30 @@ public Issue33287(TestDevice device) : base(device) { } public void DisplayAlertAsyncShouldNotCrashWhenPageUnloaded() { App.WaitForElement("NavigateButton"); - + // Navigate to second page App.Tap("NavigateButton"); - + // Wait for second page to appear App.WaitForElement("GoBackButton"); - - // Immediately go back before the 5 second delay completes + + // Go back before the 5-second delay completes App.Tap("GoBackButton"); - - // Wait for navigation to complete + + // Wait for the main page and the delayed DisplayAlertAsync to resolve (~5s + buffer) App.WaitForElement("StatusLabel"); - - // Wait for the delayed DisplayAlertAsync to be triggered (5 seconds + buffer) - System.Threading.Thread.Sleep(6000); - - // Get the status - should not show NullReferenceException - var status = App.FindElement("StatusLabel").GetText(); - Console.WriteLine($"[TEST] Final status: {status}"); - - // Assert that no NullReferenceException occurred - Assert.That(status, Does.Not.Contain("NullReferenceException"), - "DisplayAlertAsync should not throw NullReferenceException when page is unloaded"); + + // Assert positive success: the status must contain the success marker. + // Without the fix the app crashes (NRE), so StatusLabel is never updated. + var status = App.WaitForElement("StatusLabel", timeout: TimeSpan.FromSeconds(10)).GetText(); + int retries = 12; + while (retries-- > 0 && (status is null || !status.Contains("✅"))) + { + System.Threading.Thread.Sleep(1000); + status = App.FindElement("StatusLabel").GetText(); + } + + Assert.That(status, Does.Contain("✅"), + "App should show success status after DisplayAlertAsync completes on an unloaded page"); } } From 51f7bcc4c3cd07aa867310cc900c782f68e5c8b1 Mon Sep 17 00:00:00 2001 From: Jakub Florkowski Date: Tue, 7 Apr 2026 12:04:34 +0200 Subject: [PATCH 11/12] Improve UI test: use WaitForTextToBePresentInElement, reduce delay - Replace hand-rolled Thread.Sleep polling loop with the built-in WaitForTextToBePresentInElement helper (review feedback) - Reduce Task.Delay from 5s to 3s for faster CI runs while still allowing enough time to navigate back before the alert fires Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../TestCases.HostApp/Issues/Issue33287.cs | 2 +- .../Tests/Issues/Issue33287.cs | 19 ++++--------------- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.cs index cb90b47b5116..1c513721e3f7 100644 --- a/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.cs +++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.cs @@ -92,7 +92,7 @@ protected override async void OnAppearing() { base.OnAppearing(); - await Task.Delay(5000); + await Task.Delay(3000); try { diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs index a16ee61318ca..70e674cce702 100644 --- a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs @@ -22,23 +22,12 @@ public void DisplayAlertAsyncShouldNotCrashWhenPageUnloaded() // Wait for second page to appear App.WaitForElement("GoBackButton"); - // Go back before the 5-second delay completes + // Go back before the delayed DisplayAlertAsync completes App.Tap("GoBackButton"); - // Wait for the main page and the delayed DisplayAlertAsync to resolve (~5s + buffer) - App.WaitForElement("StatusLabel"); - - // Assert positive success: the status must contain the success marker. - // Without the fix the app crashes (NRE), so StatusLabel is never updated. - var status = App.WaitForElement("StatusLabel", timeout: TimeSpan.FromSeconds(10)).GetText(); - int retries = 12; - while (retries-- > 0 && (status is null || !status.Contains("✅"))) - { - System.Threading.Thread.Sleep(1000); - status = App.FindElement("StatusLabel").GetText(); - } - - Assert.That(status, Does.Contain("✅"), + // Wait for the delayed DisplayAlertAsync to resolve and update the status label. + // Without the fix the app crashes (NRE), so the status label is never updated. + Assert.That(App.WaitForTextToBePresentInElement("StatusLabel", "✅", timeout: TimeSpan.FromSeconds(10)), Is.True, "App should show success status after DisplayAlertAsync completes on an unloaded page"); } } From ebe052a509bb695e92fdf6cc2c8bb47e61848f5f Mon Sep 17 00:00:00 2001 From: Jakub Florkowski Date: Tue, 7 Apr 2026 13:36:42 +0200 Subject: [PATCH 12/12] Fix UI test: verify app stays alive instead of cross-page label update The previous test updated a MainPage label from a detached SecondPage's async on iOS the UI update never reaches the visiblecontinuation label. Simplified to verify the app remains responsive after the delayed DisplayAlertAsync fires on the unloaded page. Without the fix the NRE crashes the app and the assertion fails (positive test). Also reduced delay from 5s to 2s for faster test execution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../TestCases.HostApp/Issues/Issue33287.cs | 55 ++++--------------- .../Tests/Issues/Issue33287.cs | 19 ++++--- 2 files changed, 22 insertions(+), 52 deletions(-) diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.cs index 1c513721e3f7..8b674f593ea0 100644 --- a/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.cs +++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue33287.cs @@ -17,41 +17,24 @@ public Issue33287MainPage() { Title = "Issue 33287"; - var statusLabel = new Label - { - Text = "Status: Ready", - AutomationId = "StatusLabel", - FontAttributes = FontAttributes.Bold - }; - Content = new VerticalStackLayout { Padding = 20, Spacing = 10, Children = { - new Label - { - Text = "DisplayAlertAsync NullReferenceException Test", - FontSize = 18, - FontAttributes = FontAttributes.Bold - }, - new Label - { - Text = "1. Tap 'Navigate to Second Page'\n2. Immediately tap 'Go Back'\n3. Wait 5 seconds - should NOT crash", - TextColor = Colors.Gray - }, new Button { Text = "Navigate to Second Page", AutomationId = "NavigateButton", Command = new Command(async () => - { - statusLabel.Text = "Status: Navigating..."; - await Navigation.PushAsync(new Issue33287SecondPage(statusLabel)); - }) + await Navigation.PushAsync(new Issue33287SecondPage())) }, - statusLabel + new Label + { + Text = "MainPage", + AutomationId = "MainPageLabel" + } } }; } @@ -59,25 +42,15 @@ public Issue33287MainPage() public class Issue33287SecondPage : ContentPage { - readonly Label _statusLabel; - - public Issue33287SecondPage(Label statusLabel) + public Issue33287SecondPage() { - _statusLabel = statusLabel; Title = "Second Page"; Content = new VerticalStackLayout { Padding = 20, - Spacing = 10, Children = { - new Label - { - Text = "Second Page - Tap 'Go Back' quickly!", - FontSize = 18, - FontAttributes = FontAttributes.Bold - }, new Button { Text = "Go Back", @@ -92,16 +65,10 @@ protected override async void OnAppearing() { base.OnAppearing(); - await Task.Delay(3000); + // Wait long enough for the user/test to navigate back + await Task.Delay(2000); - try - { - await DisplayAlertAsync("Test Alert", "This alert was delayed", "OK"); - _statusLabel.Text = "Status: ✅ Alert completed"; - } - catch (Exception ex) - { - _statusLabel.Text = $"Status: ❌ {ex.GetType().Name}"; - } + // Without the fix this throws NullReferenceException and crashes the app + await DisplayAlertAsync("Test Alert", "This alert was delayed", "OK"); } } diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs index 70e674cce702..5a5e166ad556 100644 --- a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33287.cs @@ -16,18 +16,21 @@ public void DisplayAlertAsyncShouldNotCrashWhenPageUnloaded() { App.WaitForElement("NavigateButton"); - // Navigate to second page + // Navigate to second page (starts a 2-second delayed DisplayAlertAsync) App.Tap("NavigateButton"); - // Wait for second page to appear + // Wait for second page to appear, then go back immediately App.WaitForElement("GoBackButton"); - - // Go back before the delayed DisplayAlertAsync completes App.Tap("GoBackButton"); - // Wait for the delayed DisplayAlertAsync to resolve and update the status label. - // Without the fix the app crashes (NRE), so the status label is never updated. - Assert.That(App.WaitForTextToBePresentInElement("StatusLabel", "✅", timeout: TimeSpan.FromSeconds(10)), Is.True, - "App should show success status after DisplayAlertAsync completes on an unloaded page"); + // Back on main page — wait for the delayed DisplayAlertAsync to fire. + // Without the fix the NRE crashes the app and this element becomes unreachable. + App.WaitForElement("MainPageLabel"); + System.Threading.Thread.Sleep(3000); + + // Verify the app is still alive and responsive after the alert fired on the detached page. + // Without the fix the app process is dead and this call will throw/timeout. + Assert.That(App.FindElement("MainPageLabel").GetText(), Is.EqualTo("MainPage"), + "App should remain responsive after DisplayAlertAsync on an unloaded page"); } }