Skip to content

Fix dogfood scripts: workflow trigger, build detection, and in-progress handling#34259

Merged
jfversluis merged 1 commit intomainfrom
fix/dogfood-scripts
Feb 26, 2026
Merged

Fix dogfood scripts: workflow trigger, build detection, and in-progress handling#34259
jfversluis merged 1 commit intomainfrom
fix/dogfood-scripts

Conversation

@jfversluis
Copy link
Copy Markdown
Member

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description

Three fixes for the dogfooding infrastructure that was merged in #33198 but never worked correctly:

1. Workflow trigger fix (dogfood-comment.yml)

The workflow was changed from pull_request_target to check_run trigger in #33198, but Azure DevOps check runs never populate the pull_requests[] array. This means github.event.check_run.pull_requests[0] != null was always false, and the workflow never ran.

Fix: Reverted to pull_request_target trigger with a comment explaining why check_run does not work.

2. AzDO direct fallback (both scripts)

When the GitHub Checks API does not return build info (which can happen for merge commits or in certain timing windows), the scripts now fall back to querying the Azure DevOps API directly using refs/pull/{PR}/merge branch name.

This approach is modeled after the working implementation in maui-version.

3. In-progress build detection (both scripts)

When no completed build is found, the scripts now query AzDO for builds with status in ("inProgress", "notStarted", "postponed"). Instead of the generic "no build found" error, users now see:

  • Build in progress: "A build is currently in progress. Please wait for it to complete."
  • Stale artifacts: When an older build has artifacts but a newer build is running, warns users

Additional improvements

  • Build ID numeric validation 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

Testing

Tested locally with multiple real PRs across both scripts:

…ss 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>
Copilot AI review requested due to automatic review settings February 26, 2026 14:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes three critical issues in the dogfooding infrastructure that prevented it from working correctly after its initial merge in #33198: workflow trigger incompatibility with Azure DevOps check runs, missing fallback for querying build information directly from Azure DevOps, and lack of in-progress build detection.

Changes:

  • Reverted workflow trigger from check_run to pull_request_target with explanation of Azure DevOps limitation
  • Added Azure DevOps API fallback for finding builds when GitHub Checks API doesn't return results
  • Implemented in-progress build detection to provide better user feedback instead of generic errors

Reviewed changes

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

File Description
.github/workflows/dogfood-comment.yml Fixed workflow trigger to use pull_request_target instead of check_run due to Azure DevOps limitation
eng/scripts/get-maui-pr.sh Added AzDO fallback, in-progress detection, input validation, and improved error messages
eng/scripts/get-maui-pr.ps1 Added AzDO fallback, in-progress detection, and improved error messages
Comments suppressed due to low confidence (3)

eng/scripts/get-maui-pr.sh:191

  • The jq filter logic for checking in-progress builds is duplicated in lines 191 and 262. Consider extracting this filter pattern into a variable or using the check_build_in_progress function instead of repeating the jq query.
    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")

eng/scripts/get-maui-pr.sh:248

  • When jq fails or returns invalid JSON, the fallback || echo \"\" produces an empty string, but this isn't distinguishing between 'no completed builds' and 'jq failed to parse the response'. Consider adding explicit error handling to differentiate between API errors and legitimately empty results.
        completed_build=$(echo "$builds_json" | jq -r '[.value[] | select(.definition.name == "maui-pr" and .status == "completed")] | first | @json' 2>/dev/null || echo "")

eng/scripts/get-maui-pr.ps1:225

  • The build ID validation uses string interpolation \"$($completedBuild.id)\" which could mask issues if the id property is null or missing. Consider using -not ($completedBuild.id -is [int]) or checking for null explicitly before the regex match.
            if ("$($completedBuild.id)" -notmatch '^\d+$') {

@jfversluis jfversluis merged commit c7a9ed5 into main Feb 26, 2026
5 of 15 checks passed
@jfversluis jfversluis deleted the fix/dogfood-scripts branch February 26, 2026 14:13
Tamilarasan-Paranthaman pushed a commit that referenced this pull request Mar 2, 2026
…ss handling (#34259)

<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting
artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from
this PR and let us know in a comment if this change resolves your issue.
Thank you!

## Description

Three fixes for the dogfooding infrastructure that was merged in #33198
but never worked correctly:

### 1. Workflow trigger fix (`dogfood-comment.yml`)
The workflow was changed from `pull_request_target` to `check_run`
trigger in #33198, but Azure DevOps check runs **never populate the
`pull_requests[]` array**. This means
`github.event.check_run.pull_requests[0] != null` was **always false**,
and the workflow never ran.

**Fix**: Reverted to `pull_request_target` trigger with a comment
explaining why `check_run` does not work.

### 2. AzDO direct fallback (both scripts)
When the GitHub Checks API does not return build info (which can happen
for merge commits or in certain timing windows), the scripts now fall
back to querying the Azure DevOps API directly using
`refs/pull/{PR}/merge` branch name.

This approach is modeled after the working implementation in
[maui-version](https://github.com/jfversluis/maui-version).

### 3. In-progress build detection (both scripts)
When no completed build is found, the scripts now query AzDO for builds
with `status in ("inProgress", "notStarted", "postponed")`. Instead of
the generic "no build found" error, users now see:
- **Build in progress**: "A build is currently in progress. Please wait
for it to complete."
- **Stale artifacts**: When an older build has artifacts but a newer
build is running, warns users

### Additional improvements
- Build ID numeric validation 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

## Testing

Tested locally with multiple real PRs across both scripts:
- ✅ PR #34216 (completed, successful) - both scripts find build and
apply artifacts
- ✅ PR #34250 (in-progress build) - correctly finds existing artifacts
with warning
- ✅ PR #99999999 (non-existent) - proper error handling
- ✅ Invalid PR number ("abc") - input validation works
- ✅ Multi-model code review (Claude Sonnet 4.5, GPT-5.1, Gemini 3 Pro) -
all findings addressed

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants