From 8d9be64f862f7e8242557f07bcac569a38e5fdbf Mon Sep 17 00:00:00 2001 From: Gerald Versluis Date: Thu, 26 Feb 2026 15:06:17 +0100 Subject: [PATCH] Fix dogfood scripts: workflow trigger, build detection, and in-progress handling Three fixes for the dogfooding infrastructure: 1. **Workflow trigger**: Reverted dogfood-comment.yml from check_run to pull_request_target trigger. Azure DevOps check runs never populate the pull_requests[] array, so the check_run-based condition was always false and the workflow never ran. 2. **AzDO direct fallback**: Added Azure DevOps API as a fallback when GitHub Checks API doesn't return build info. Queries refs/pull/{PR}/merge branch directly to find completed builds. 3. **In-progress build detection**: Added detection for builds that are currently running. Shows a clear message telling users to wait instead of a generic 'no build found' error. Also warns when a newer build is in progress but older artifacts are available. Additional improvements: - Build ID validation (numeric check) to prevent URL injection - PR number validation in bash script - Protection against set -e crashes on invalid jq input - Better error messages for all failure scenarios Tested locally with multiple real PRs (#34216, #34250, #99999999) across both scripts, verifying: successful builds, in-progress builds, error handling, and input validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/dogfood-comment.yml | 33 +++---- eng/scripts/get-maui-pr.ps1 | 116 ++++++++++++++++++----- eng/scripts/get-maui-pr.sh | 129 +++++++++++++++++++++----- 3 files changed, 215 insertions(+), 63 deletions(-) diff --git a/.github/workflows/dogfood-comment.yml b/.github/workflows/dogfood-comment.yml index b2a823d75d64..c79b361dacb5 100644 --- a/.github/workflows/dogfood-comment.yml +++ b/.github/workflows/dogfood-comment.yml @@ -1,9 +1,16 @@ name: Add Dogfooding Comment on: - # Trigger when the maui-pr build check completes - check_run: - types: [completed] + # Use pull_request_target to run in the context of the base branch + # This allows commenting on PRs from forks + # Note: check_run trigger doesn't work because Azure DevOps check runs + # don't populate the pull_requests[] field, so we can't get the PR number. + pull_request_target: + types: [opened, reopened, synchronize] + branches: + - 'main' + - 'net*' + - 'release/**' # Allow manual triggering workflow_dispatch: @@ -15,23 +22,13 @@ on: # Ensure only one instance runs at a time per PR to prevent duplicate comments concurrency: - group: dogfood-comment-${{ github.event.check_run.pull_requests[0].number || github.event.inputs.pr_number || 'unknown' }} + group: dogfood-comment-${{ github.event.pull_request.number || github.event.inputs.pr_number }} cancel-in-progress: true jobs: add-dogfood-comment: - # Only run on the dotnet org, for the maui-pr check, when it completes successfully - if: | - github.repository_owner == 'dotnet' && - ( - github.event_name == 'workflow_dispatch' || - ( - github.event_name == 'check_run' && - github.event.check_run.name == 'maui-pr (Pack .NET MAUI Pack Windows)' && - github.event.check_run.conclusion == 'success' && - github.event.check_run.pull_requests[0] != null - ) - ) + # Only run on the dotnet org to avoid running on forks + if: ${{ github.repository_owner == 'dotnet' }} runs-on: ubuntu-latest permissions: pull-requests: write @@ -41,8 +38,8 @@ jobs: uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 with: script: | - // Get PR number from either the check_run event or manual input - const prNumber = context.payload.check_run?.pull_requests?.[0]?.number || context.payload.inputs?.pr_number; + // Get PR number from either the PR event or manual input + const prNumber = context.payload.pull_request?.number || context.payload.inputs?.pr_number; const bashScript = 'https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh'; const psScript = 'https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1'; diff --git a/eng/scripts/get-maui-pr.ps1 b/eng/scripts/get-maui-pr.ps1 index e20f81a86828..eeb19414d1aa 100644 --- a/eng/scripts/get-maui-pr.ps1 +++ b/eng/scripts/get-maui-pr.ps1 @@ -146,12 +146,33 @@ function Get-PullRequestInfo { } } -# Get build information from GitHub Checks API +# Check if a build is currently in progress for this PR via Azure DevOps API +function Test-BuildInProgress { + param([int]$PrNumber) + + try { + $buildsUrl = "https://dev.azure.com/$AzureDevOpsOrg/$AzureDevOpsProject/_apis/build/builds?api-version=7.1&branchName=refs/pull/$PrNumber/merge&`$top=10" + $response = Invoke-RestMethod -Uri $buildsUrl -Headers @{ "User-Agent" = "MAUI-PR-Script" } -TimeoutSec 30 + + foreach ($build in $response.value) { + if ($build.definition.name -eq "maui-pr" -and $build.status -in @("inProgress", "notStarted", "postponed")) { + return $true + } + } + return $false + } + catch { + return $false + } +} + +# Get build information from GitHub Checks API, with AzDO fallback function Get-BuildInfo { - param([string]$SHA) + param([string]$SHA, [int]$PrNumber) Write-Info "Looking for build artifacts for commit $($SHA.Substring(0, 7))..." + # Strategy 1: Try GitHub Checks API try { $checksUrl = "https://api.github.com/repos/$GitHubRepo/commits/$SHA/check-runs" $response = Invoke-RestMethod -Uri $checksUrl -Headers ($GitHubHeaders + @{ @@ -163,34 +184,87 @@ function Get-BuildInfo { $_.name -like "maui-pr*" -and $_.name -notlike "*uitests*" -and $_.status -eq "completed" -and $_.details_url -match 'buildId=' } | Select-Object -First 1 - if (-not $buildCheck) { - throw "No completed build found for this PR. The build may still be in progress or may have failed." - } - - if ($buildCheck.conclusion -ne "success") { - Write-Warn "Build completed with status: $($buildCheck.conclusion)" - if (-not $Yes) { - $continue = Read-Host "Do you want to continue anyway? (y/N)" - if ($continue -ne "y" -and $continue -ne "Y") { - throw "Build was not successful. Aborting." + if ($buildCheck) { + if ($buildCheck.conclusion -ne "success") { + Write-Warn "Build completed with status: $($buildCheck.conclusion)" + if (-not $Yes) { + $continue = Read-Host "Do you want to continue anyway? (y/N)" + if ($continue -ne "y" -and $continue -ne "Y") { + throw "Build was not successful. Aborting." + } + } + } + + # Extract build ID from details URL + if ($buildCheck.details_url -match 'buildId=(\d+)') { + Write-Success "Found build ID: $($Matches[1]) (via GitHub Checks)" + return @{ + BuildId = $Matches[1] + Status = $buildCheck.conclusion + Url = $buildCheck.details_url } } } + } + catch { + Write-Info "GitHub Checks API lookup failed, trying Azure DevOps directly..." + } + + # Strategy 2: Query Azure DevOps directly (handles merge commits not reported to GitHub) + Write-Info "Searching Azure DevOps directly for PR #$PrNumber builds..." + try { + $buildsUrl = "https://dev.azure.com/$AzureDevOpsOrg/$AzureDevOpsProject/_apis/build/builds?api-version=7.1&branchName=refs/pull/$PrNumber/merge&`$top=10" + $response = Invoke-RestMethod -Uri $buildsUrl -Headers @{ "User-Agent" = "MAUI-PR-Script" } -TimeoutSec 30 + + $completedBuild = $response.value | Where-Object { + $_.definition.name -eq "maui-pr" -and $_.status -eq "completed" + } | Select-Object -First 1 - # Extract build ID from details URL - if ($buildCheck.details_url -match 'buildId=(\d+)') { + if ($completedBuild) { + # Validate build ID is numeric + if ("$($completedBuild.id)" -notmatch '^\d+$') { + throw "Invalid build ID received from Azure DevOps API" + } + + # Check if a newer build is in progress (user may have pushed a new commit) + $inProgressBuild = $response.value | Where-Object { + $_.definition.name -eq "maui-pr" -and $_.status -in @("inProgress", "notStarted", "postponed") + } | Select-Object -First 1 + if ($inProgressBuild) { + Write-Warn "A newer build is currently in progress. The available artifacts may be from a previous commit." + Write-Warn "If you just pushed changes, wait for the new build to complete." + } + + if ($completedBuild.result -ne "succeeded") { + Write-Warn "Build completed with result: $($completedBuild.result)" + if (-not $Yes) { + $continue = Read-Host "Do you want to continue anyway? (y/N)" + if ($continue -ne "y" -and $continue -ne "Y") { + throw "Build was not successful. Aborting." + } + } + } + + $buildUrl = "https://dev.azure.com/$AzureDevOpsOrg/$AzureDevOpsProject/_build/results?buildId=$($completedBuild.id)" + Write-Success "Found build ID: $($completedBuild.id) (via Azure DevOps)" return @{ - BuildId = $Matches[1] - Status = $buildCheck.conclusion - Url = $buildCheck.details_url + BuildId = "$($completedBuild.id)" + Status = $completedBuild.result + Url = $buildUrl } } - - throw "Could not extract build ID from check run details." } catch { - throw "Failed to get build information: $_" + Write-Info "Azure DevOps direct lookup also failed." } + + # No build found - check if one is in progress + $buildInProgress = Test-BuildInProgress -PrNumber $PrNumber + if ($buildInProgress) { + throw "No completed build found, but a build is currently in progress for PR #$PrNumber. Please wait for it to complete and try again. Check status: https://github.com/dotnet/maui/pull/$PrNumber" + } + + throw "No completed build found for PR #$PrNumber. The PR may not have triggered CI builds yet (draft PRs don't auto-trigger builds), or the build may have failed. Check: https://github.com/dotnet/maui/pull/$PrNumber" } # Get artifacts from Azure DevOps @@ -468,7 +542,7 @@ try { Write-Info "Current target framework: .NET $targetNetVersion.0" Write-Step "Finding build artifacts" - $buildInfo = Get-BuildInfo -SHA $prInfo.SHA + $buildInfo = Get-BuildInfo -SHA $prInfo.SHA -PrNumber $PrNumber Write-Step "Downloading artifacts" $downloadUrl = Get-BuildArtifacts -BuildId $buildInfo.BuildId diff --git a/eng/scripts/get-maui-pr.sh b/eng/scripts/get-maui-pr.sh index 9541a0556644..8a37b98c6006 100644 --- a/eng/scripts/get-maui-pr.sh +++ b/eng/scripts/get-maui-pr.sh @@ -178,12 +178,32 @@ get_pr_info() { echo "$pr_json" } -# Get build information from GitHub Checks API +# Check if a build is currently in progress for this PR via Azure DevOps API +check_build_in_progress() { + local pr_num="$1" + + local builds_url="https://dev.azure.com/$AZURE_DEVOPS_ORG/$AZURE_DEVOPS_PROJECT/_apis/build/builds?api-version=7.1&branchName=refs/pull/$pr_num/merge&\$top=10" + local builds_json + builds_json=$(curl -s -H "User-Agent: MAUI-PR-Script" "$builds_url" 2>/dev/null) || return 1 + + # Check if any maui-pr build is in progress + local in_progress + in_progress=$(echo "$builds_json" | jq -r '[.value[] | select(.definition.name == "maui-pr" and (.status == "inProgress" or .status == "notStarted" or .status == "postponed"))] | length' 2>/dev/null || echo "0") + + if [ "$in_progress" != "0" ] && [ -n "$in_progress" ]; then + return 0 # true - build is in progress + fi + return 1 # false - no build in progress +} + +# Get build information from GitHub Checks API, with AzDO fallback get_build_info() { local sha="$1" + local pr_num="$2" info "Looking for build artifacts for commit ${sha:0:7}..." + # Strategy 1: Try GitHub Checks API local checks_url="https://api.github.com/repos/$GITHUB_REPO/commits/$sha/check-runs" local checks_json checks_json=$(curl -s -H "User-Agent: MAUI-PR-Script" -H "Accept: application/vnd.github.v3+json" ${GITHUB_AUTH_HEADER:+-H "$GITHUB_AUTH_HEADER"} "$checks_url") @@ -191,37 +211,91 @@ get_build_info() { # Find the main MAUI build check (not uitests) local build_check=$(echo "$checks_json" | jq -r '.check_runs[] | select((.name | startswith("maui-pr")) and (.name | contains("uitests") | not) and .status == "completed" and (.details_url | contains("buildId="))) | @json' | head -n 1) - if [ -z "$build_check" ] || [ "$build_check" == "null" ]; then - error "No completed build found for this PR" - info "The build may still be in progress or may have failed." - exit 1 + if [ -n "$build_check" ] && [ "$build_check" != "null" ]; then + local conclusion=$(echo "$build_check" | jq -r '.conclusion') + if [ "$conclusion" != "success" ]; then + warning "Build completed with status: $conclusion" + if [ "$YES_FLAG" = true ]; then + info "Auto-accepting non-successful build (-y flag)" + else + read -p "Do you want to continue anyway? (y/N) " -n 1 -r + echo >&2 + if [[ ! $REPLY =~ ^[Yy]$ ]]; then + error "Build was not successful. Aborting." + exit 1 + fi + fi + fi + + # Extract build ID from details URL + local details_url=$(echo "$build_check" | jq -r '.details_url') + if [[ "$details_url" =~ buildId=([0-9]+) ]]; then + local build_id="${BASH_REMATCH[1]}" + success "Found build ID: $build_id (via GitHub Checks)" + echo "$build_id" + return 0 + fi fi - local conclusion=$(echo "$build_check" | jq -r '.conclusion') - if [ "$conclusion" != "success" ]; then - warning "Build completed with status: $conclusion" - if [ "$YES_FLAG" = true ]; then - info "Auto-accepting non-successful build (-y flag)" - else - read -p "Do you want to continue anyway? (y/N) " -n 1 -r - echo >&2 - if [[ ! $REPLY =~ ^[Yy]$ ]]; then - error "Build was not successful. Aborting." + # Strategy 2: Query Azure DevOps directly (handles merge commits not reported to GitHub) + info "Searching Azure DevOps directly for PR #$pr_num builds..." + local builds_url="https://dev.azure.com/$AZURE_DEVOPS_ORG/$AZURE_DEVOPS_PROJECT/_apis/build/builds?api-version=7.1&branchName=refs/pull/$pr_num/merge&\$top=10" + local builds_json + builds_json=$(curl -s -H "User-Agent: MAUI-PR-Script" "$builds_url" 2>/dev/null) + + if [ -n "$builds_json" ]; then + local completed_build + completed_build=$(echo "$builds_json" | jq -r '[.value[] | select(.definition.name == "maui-pr" and .status == "completed")] | first | @json' 2>/dev/null || echo "") + + if [ -n "$completed_build" ] && [ "$completed_build" != "null" ]; then + local azdo_build_id=$(echo "$completed_build" | jq -r '.id') + local azdo_result=$(echo "$completed_build" | jq -r '.result') + + # Validate build ID is numeric + if ! [[ "$azdo_build_id" =~ ^[0-9]+$ ]]; then + error "Invalid build ID received from Azure DevOps API" exit 1 fi + + # Check if a newer build is in progress (user may have pushed a new commit) + local in_progress_count + in_progress_count=$(echo "$builds_json" | jq -r '[.value[] | select(.definition.name == "maui-pr" and (.status == "inProgress" or .status == "notStarted" or .status == "postponed"))] | length' 2>/dev/null || echo "0") + if [ "$in_progress_count" != "0" ] && [ -n "$in_progress_count" ]; then + warning "A newer build is currently in progress. The available artifacts may be from a previous commit." + warning "If you just pushed changes, wait for the new build to complete." + fi + + if [ "$azdo_result" != "succeeded" ]; then + warning "Build completed with result: $azdo_result" + if [ "$YES_FLAG" = true ]; then + info "Auto-accepting non-successful build (-y flag)" + else + read -p "Do you want to continue anyway? (y/N) " -n 1 -r + echo >&2 + if [[ ! $REPLY =~ ^[Yy]$ ]]; then + error "Build was not successful. Aborting." + exit 1 + fi + fi + fi + + success "Found build ID: $azdo_build_id (via Azure DevOps)" + echo "$azdo_build_id" + return 0 fi fi - # Extract build ID from details URL - local details_url=$(echo "$build_check" | jq -r '.details_url') - if [[ "$details_url" =~ buildId=([0-9]+) ]]; then - local build_id="${BASH_REMATCH[1]}" - success "Found build ID: $build_id" - echo "$build_id" - return 0 + # No build found - check if one is in progress + if check_build_in_progress "$pr_num"; then + error "No completed build found, but a build is currently in progress for PR #$pr_num" + info "Please wait for it to complete and try again." + info "Check status: https://github.com/dotnet/maui/pull/$pr_num" + exit 1 fi - error "Could not extract build ID from check run details" + error "No completed build found for PR #$pr_num" + info "The PR may not have triggered CI builds yet (draft PRs don't auto-trigger builds), or the build may have failed." + info "Check: https://github.com/dotnet/maui/pull/$pr_num" exit 1 } @@ -450,6 +524,13 @@ main() { fi pr_number="${positional_args[0]}" # Global for error handler + + # Validate PR number is numeric + if ! [[ "$pr_number" =~ ^[0-9]+$ ]]; then + error "PR number must be a valid number, got: $pr_number" + exit 1 + fi + local project_path_arg="${positional_args[1]:-}" # Check dependencies @@ -497,7 +578,7 @@ EOF step "Finding build artifacts" local build_id - build_id=$(get_build_info "$pr_sha") + build_id=$(get_build_info "$pr_sha" "$pr_number") step "Downloading artifacts" local download_url