Trigger Expert Code Review when draft PR becomes ready for review#8955
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the repo’s automation and CI configuration so the “Expert Code Review (on open)” agent workflow also runs when a draft PR is transitioned to “ready for review”, and it additionally enables Azure DevOps progress/summary reporting in test runs.
Changes:
- Add
pull_request.ready_for_reviewto the agentic workflow trigger so draft→ready transitions invoke the expert review agent. - Gate
--report-azdo-progress/--report-azdo-summarybehindTF_BUILDfor MSBuild-injected test arguments to keep local runs quiet. - Enable
--report-azdo-progress/--report-azdo-summaryexplicitly in the Release--test-modulesCI paths (Windows + non-Windows), and recompile the workflow lock file.
Show a summary per file
| File | Description |
|---|---|
| test/Directory.Build.targets | Adds TF_BUILD-gated AzDO progress/summary reporting flags to injected test host args. |
| eng/pipelines/steps/test-non-windows.yml | Adds AzDO progress/summary flags to Release --test-modules run on non-Windows. |
| azure-pipelines.yml | Adds AzDO progress/summary flags to Release --test-modules run on Windows. |
| .github/workflows/review-on-open.agent.md | Adds ready_for_review to PR trigger types for the expert review workflow. |
| .github/workflows/review-on-open.agent.lock.yml | Regenerated compiled workflow reflecting the new trigger and updated gh-aw tooling versions. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 2
This comment has been minimized.
This comment has been minimized.
Evangelink
left a comment
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
Expert Code Review — PR #8955
Verdict: COMMENT — No blocking issues. Ready to merge.
22-Dimension Verdict Table
| # | Dimension | Severity | Result | Notes |
|---|---|---|---|---|
| 1 | Algorithmic Correctness | MAJOR | ✅ N/A | Pure CI/workflow config; no runtime logic |
| 2 | Threading & Concurrency | BLOCKING | ✅ N/A | No concurrent code |
| 3 | Security | BLOCKING | ✅ Clean | roles: [admin, maintainer, write] and repository_id fork-guard in lock file both intact |
| 4 | Public API | BLOCKING | ✅ N/A | No public API surface changes |
| 5 | Performance | MAJOR | ✅ N/A | CI config only |
| 6 | Cross-TFM Compatibility | MAJOR | ✅ N/A | CI config only |
| 7 | Localization | MAJOR | ✅ N/A | No user-facing strings |
| 8 | Test Coverage | MAJOR | ✅ Clean | --report-azdo-progress / --report-azdo-summary already exercised by HelpInfoAllExtensionsTests.cs (lines 131, 139, 389, 405) |
| 9 | Error Handling | MAJOR | ✅ N/A | CI config only |
| 10 | Code Style | MINOR | ✅ Clean | Consistent with existing conventions |
| 11 | Documentation | MINOR | ✅ Clean | Comments in azure-pipelines.yml, test-non-windows.yml, and Directory.Build.targets are thorough; review-on-open.agent.md comment update is accurate |
| 12 | Backward Compatibility | BLOCKING | ✅ Clean | ready_for_review trigger is purely additive; opened behavior unchanged |
| 13 | IPC Contract Stability | BLOCKING | ✅ N/A | No IPC protocol changes |
| 14 | Resource Cleanup | MAJOR | ✅ N/A | No resource lifecycle changes |
| 15 | Analyzer Correctness | MAJOR | ✅ N/A | No Roslyn analyzer changes |
| 16 | SDK/MSBuild Integration | MAJOR | ✅ Clean | '$(TF_BUILD)' == 'true' condition is correct — MSBuild string comparison is case-insensitive, so it matches True as set by Azure Pipelines |
| 17 | CLI Options Consistency | MAJOR | ✅ Clean | New flags already present in help/info acceptance tests |
| 18 | Agentic Workflow Correctness | MAJOR | ✅ Clean | See detailed notes below |
| 19 | Scope Discipline | MINOR | AzDO CI improvement (new flags) is bundled with the ready_for_review trigger fix; both are small, coherent CI changes — acceptable, no action required |
|
| 20 | Data Validation | MAJOR | ✅ N/A | CI config only |
| 21 | Dependency Management | MINOR | ✅ Clean | gh-aw v0.75.4 → v0.76.1 bump is a natural side-effect of recompiling; no surprises |
| 22 | Observability | MINOR | ✅ Clean | --report-azdo-progress / --report-azdo-summary improve live build visibility and failure summaries in AzDO |
Dimension 18 — Agentic Workflow Correctness (detailed)
ready_for_reviewtrigger: Correct. GitHub fires this event exactly when a PR transitions from draft → ready. The existingif: github.event.pull_request.draft == falseguard is redundant for this event type (it's guaranteed non-draft) but harmless and serves as self-documentation.- Lock file regeneration:
gh-aw-metadatashowsstrict: true,compiler_version: v0.76.1, and thefrontmatter_hashchanged from6b9954e9...→ad51ec84..., confirming the lock was properly regenerated with--strictfrom the updated source. .antigravityadditions: Consistent with the existing pattern of AI-assistant config folders (.claude,.codex,.crush,.gemini). The model allowlist in the firewall config already listedantigravityroutes; the sparse-checkout andGH_AW_AGENT_FOLDERS/GH_AW_AGENT_FILESentries now align.restore_inline_skills.shstep: New in v0.76.1; properly adds a restore step for.github/skillsand includes the artifact path in the upload list.
Informational Notes (non-blocking)
-
review.agent.lock.yml(slash-command variant) not updated: This lock file remains at v0.75.4 and does not include.antigravityinGH_AW_AGENT_FOLDERSor therestore_inline_skills.shstep. Since the slash command is currently working, this is not a problem for this PR, but consider recompiling it as a follow-up to bring it to parity. -
Pre-existing flag duplication in AzDO CI: In Azure Pipelines,
TF_BUILD=Trueis set at job start, so test executables are built with--report-azdo-progress --report-azdo-summaryembedded inTestingPlatformCommandLineArguments. The CI scripts then also pass these flags explicitly. This double-passing was already present for--hangdump,--report-azdo, and--diagnostic, so MTP already handles duplicate CLI arguments gracefully. The PR extends the same pre-existing pattern — no action required.
Generated by Expert Code Review (on open) for issue #8955 · sonnet46 3.2M
3314309 to
40be7b2
Compare
The `Expert Code Review (on open)` workflow only listened to `pull_request: opened`, so it never ran when a PR was transitioned from draft to ready for review (GitHub fires a separate `ready_for_review` event in that case). Users had to fall back to the `/review` slash command. Add `ready_for_review` to the trigger types. The existing `if:` condition (`github.event.pull_request.draft == false`) keeps the `opened` event a no-op for drafts, so behavior for normal opens is unchanged. Recompiled the lock file with `gh aw compile review-on-open.agent --strict`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
40be7b2 to
2445953
Compare
🧪 Test quality grade — PR #8955No new or modified test methods were identified in the changed regions Re-run with
|
The workflow now triggers on both `opened` and `ready_for_review`, so the`name` (`Expert Code Review (on open)`) and `description` (`...when a non-draft PR is opened.`) were misleading — they still read as if it only ran on PR open. Rename to `Expert Code Review (on PR ready)` and rewrite the description to`Automatically runs the expert-reviewer agent when a PR becomes ready for review — either opened as non-draft, or transitioned from draft to ready.`, then recompile so the lock file (including run-name, WORKFLOW_NAME envs, etc.) stays in sync. Addresses Copilot review comments on PR #8955. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The
Expert Code Review (on open)workflow only listened topull_request: opened, so it never ran when a PR was transitioned from draft to ready for review (GitHub fires a separateready_for_reviewevent in that case). Users had to fall back to the/reviewslash command.This adds
ready_for_reviewto the triggertypes. The existingif:condition (github.event.pull_request.draft == false) already filters out drafts, so theopenedevent remains a no-op for drafts and there is no behavior change for normal non-draft opens.Recompiled the lock file with
gh aw compile review-on-open.agent --strict.Context
This was noticed alongside a transient failure on https://github.com/microsoft/testfx/actions/runs/27191975267, where the agent step failed because the gh-aw firewall
awf-squidcontainer reported unhealthy (dependency failed to start: container awf-squid is unhealthy). That failure is an infrastructure issue inside the gh-aw runtime stack — not something this workflow can fix — and the right mitigation there is a re-run. This PR addresses the second, actionable issue: the missingready_for_reviewtrigger.