ci-fix: mark validated draft PRs ready when the fixed test goes green - #36461
Conversation
Adds a target-test verification + mark-ready gate (Step 3.6) to both ci-fix
twins. When a [ci-fix]/[ci-fix-net11] draft PR reaches the green-surface or
unrelated-flake branch, the loop now drills the PR's OWN AzDO test-results for
the SPECIFIC test(s) the fix targeted. If every target test is Passed on >=1
leg and Failed on none (VALIDATED-GREEN), it:
- posts a "target test validated green on <headSha>" comment, and
- transitions the draft PR to ready-for-review.
This is a state transition only — it never approves and never merges; a human
still reviews and merges. Overall red on UNRELATED legs no longer keeps a
validated fix parked as a draft.
Implementation:
- New safe-output mark-pull-request-as-ready-for-review (max:3, target:"*",
required-title-prefix + required-labels; both survive v0.80.9 compile, unlike
update-pull-request). Defense-in-depth scoping to this workflow's own PRs.
- New Step 3.6 with preconditions (isDraft, own-PR self-check, reached from
green/unrelated-flake not caused-by-fix), T1 target-test identification,
T2 AzDO test-results drill-down (not-executed => skip, honest no-overclaim),
T3 idempotent per-head-SHA marker + dry-run gate.
- Header description, outcome table, green/flake hooks, and summary tally
updated; recompiled both locks (0/0, no action/version drift).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36461Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36461" |
There was a problem hiding this comment.
Pull request overview
This PR enhances the ci-status-fix self-watching workflows (main + net11.0) so that when a draft [ci-fix] / [ci-fix-net11] PR is validated (target test passes on the PR’s own CI), the workflow can transition the PR from draft → ready for review (without approving/merging). It does this by introducing a new safe-output (mark-pull-request-as-ready-for-review) and documenting a new “Step 3.6” gate to verify the specific targeted test via AzDO test results.
Changes:
- Add
mark-pull-request-as-ready-for-reviewto safe-outputs (scoped by title prefix + label) for both workflow twins. - Add Step 3.6 documentation describing target-test verification and the mark-ready transition, invoked after green-surface or unrelated-flake paths.
- Regenerate both
.lock.ymlfiles to include the new safe-output tool and handler configuration.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/ci-status-fix.md | Adds safe-output config + Step 3.6 docs for verifying target test and marking draft PRs ready. |
| .github/workflows/ci-status-fix.lock.yml | Compiled lock update to include the new safe-output tool/config for main workflow. |
| .github/workflows/ci-status-fix-net11.md | Same as main workflow, but scoped to [ci-fix-net11] / ci-scan-net11. |
| .github/workflows/ci-status-fix-net11.lock.yml | Compiled lock update to include the new safe-output tool/config for net11 workflow. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 4
| curl -s "https://dev.azure.com/$ORG/$PROJ/_apis/test/runs?buildUri=vstfs:///Build/Build/$BUILD&api-version=7.1" \ | ||
| | tee /tmp/gh-aw/agent/testruns_${P}.json | jq -r '.value[] | "\(.id)\t\(.name)"' | ||
| # per run id, look for each target test's outcome (repeat per target test): | ||
| curl -s "https://dev.azure.com/$ORG/$PROJ/_apis/test/Runs/<runId>/results?api-version=7.1&\$top=1000" \ | ||
| | jq -r '.value[] | select(.testCaseTitle=="<TargetTest>") | "\(.outcome)\t\(.automatedTestName)"' |
There was a problem hiding this comment.
Resolved in 45a45f0. T2 no longer calls _apis/test/... at all (it 302-redirects to a sign-in page anonymously — Hard-Rule 8 — which is exactly why every target looked "not executed"). It now validates at leg granularity via the anonymous _apis/build/builds/{id}/timeline (HTTP 200 JSON), so per-test result paging (x-ms-continuationtoken, $top) and testCaseTitle vs automatedTestName matching no longer apply.
| curl -s "https://dev.azure.com/$ORG/$PROJ/_apis/test/runs?buildUri=vstfs:///Build/Build/$BUILD&api-version=7.1" \ | ||
| | tee /tmp/gh-aw/agent/testruns_${P}.json | jq -r '.value[] | "\(.id)\t\(.name)"' | ||
| # per run id, look for each target test's outcome (repeat per target test): | ||
| curl -s "https://dev.azure.com/$ORG/$PROJ/_apis/test/Runs/<runId>/results?api-version=7.1&\$top=1000" \ | ||
| | jq -r '.value[] | select(.testCaseTitle=="<TargetTest>") | "\(.outcome)\t\(.automatedTestName)"' |
There was a problem hiding this comment.
Resolved in 45a45f0. T2 no longer calls _apis/test/... at all (it 302-redirects to a sign-in page anonymously — Hard-Rule 8 — which is exactly why every target looked "not executed"). It now validates at leg granularity via the anonymous _apis/build/builds/{id}/timeline (HTTP 200 JSON), so per-test result paging (x-ms-continuationtoken, $top) and testCaseTitle vs automatedTestName matching no longer apply.
| - Otherwise emit BOTH safe-outputs for THIS PR number `<P>`: | ||
| 1. `add_comment`: `🎯 Target test validated green on <C.headSha> — <TestList> | ||
| passed (<platforms/legs>, buildId <B>). <if any red: "The remaining red is | ||
| unrelated flake on leg(s) <Y> — not caused by this fix."> Transitioning this PR | ||
| from draft to ready for review; a maintainer still reviews and merges.` | ||
| 2. `mark_pull_request_as_ready_for_review` with `reason:` a one-line justification | ||
| naming the validated test(s) and `<C.headSha>`. |
There was a problem hiding this comment.
Addressed in 45a45f0. T3 now does an all-or-nothing per-run budget pre-check (Hard-Rule 7) and, if any of add_comment/mark_pull_request_as_ready_for_review/add_labels is exhausted, defers the whole mark-ready set atomically to the next continuous-loop tick rather than partially marking a PR. So a full add-comment bucket delays a PR by one sweep, never leaves it half-marked. The ✅ (green nudge) and 🎯 (marked-ready) comments are intentionally distinct, and the deferred PRs self-heal next tick.
| - Otherwise emit BOTH safe-outputs for THIS PR number `<P>`: | ||
| 1. `add_comment`: `🎯 Target test validated green on <C.headSha> — <TestList> | ||
| passed (<platforms/legs>, buildId <B>). <if any red: "The remaining red is | ||
| unrelated flake on leg(s) <Y> — not caused by this fix."> Transitioning this PR | ||
| from draft to ready for review; a maintainer still reviews and merges.` | ||
| 2. `mark_pull_request_as_ready_for_review` with `reason:` a one-line justification | ||
| naming the validated test(s) and `<C.headSha>`. |
There was a problem hiding this comment.
Addressed in 45a45f0. T3 now does an all-or-nothing per-run budget pre-check (Hard-Rule 7) and, if any of add_comment/mark_pull_request_as_ready_for_review/add_labels is exhausted, defers the whole mark-ready set atomically to the next continuous-loop tick rather than partially marking a PR. So a full add-comment bucket delays a PR by one sweep, never leaves it half-marked. The ✅ (green nudge) and 🎯 (marked-ready) comments are intentionally distinct, and the deferred PRs self-heal next tick.
…t as human engagement gh-aw's create_pull_request builds a PR's initial commit through the GitHub API, which stamps author=github-actions[bot] but committer=web-flow. Since 2f6b77b removed web-flow from $BotLogins (to catch a maintainer 'Update branch'), Test-AnyHumanCommitActor's committer check began reading that self-authored commit as human engagement — so every freshly opened [ci-fix] draft PR looked 'human owned' from its first commit and the watch loop skipped it forever (never reaching the surface-green / mark-ready path). Fix: a human COMMITTER only trips the hand-off when the commit AUTHOR is not one of this workflow's own bot identities ($LoopBotCommitAuthors). A human AUTHOR still counts unconditionally, so maintainer direct commits and web-flow-authored 'Update branch' merges continue to hand off correctly. Unit-tested across all six author/committer shapes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| # per run id, look for each target test's outcome (repeat per target test): | ||
| curl -s "https://dev.azure.com/$ORG/$PROJ/_apis/test/Runs/<runId>/results?api-version=7.1&\$top=1000" \ | ||
| | jq -r '.value[] | select(.testCaseTitle=="<TargetTest>") | "\(.outcome)\t\(.automatedTestName)"' |
There was a problem hiding this comment.
Resolved in 45a45f0. T2 no longer calls _apis/test/... at all (it 302-redirects to a sign-in page anonymously — Hard-Rule 8 — which is exactly why every target looked "not executed"). It now validates at leg granularity via the anonymous _apis/build/builds/{id}/timeline (HTTP 200 JSON), so per-test result paging (x-ms-continuationtoken, $top) and testCaseTitle vs automatedTestName matching no longer apply.
| # per run id, look for each target test's outcome (repeat per target test): | ||
| curl -s "https://dev.azure.com/$ORG/$PROJ/_apis/test/Runs/<runId>/results?api-version=7.1&\$top=1000" \ | ||
| | jq -r '.value[] | select(.testCaseTitle=="<TargetTest>") | "\(.outcome)\t\(.automatedTestName)"' |
There was a problem hiding this comment.
Resolved in 45a45f0. T2 no longer calls _apis/test/... at all (it 302-redirects to a sign-in page anonymously — Hard-Rule 8 — which is exactly why every target looked "not executed"). It now validates at leg granularity via the anonymous _apis/build/builds/{id}/timeline (HTTP 200 JSON), so per-test result paging (x-ms-continuationtoken, $top) and testCaseTitle vs automatedTestName matching no longer apply.
…ncy, PS1 web-flow precision Applies four verified findings from a 3-model adversarial self-review of the Step 3.6 draft→ready capability (both twins + shared prefetch script): 1. add_comment cap 3→6. A green DRAFT PR flipped ready in one sweep spends TWO comment slots (Step 3 ✅ surface, which still names the /azp-gated legs a human must kick, AND Step 3.6 T3 🎯 readiness). At max:3 the shared comment bucket drained after ~1 flip and T3's atomicity pre-check then deferred every further mark-ready while the mark-ready/add_labels buckets (also 3) sat idle. Sizing 6 lets ~3 flips (2 comments each) land per sweep. Hard-Rule 7 caps updated to match. 2. T3 comment-idempotency broadened. The build-only path posts "🎯 Build validated green …" but the prior-comment scan matched only "🎯 … target test … validated …", so a landed build-only comment whose mark-ready failed would be re-posted every sweep. Scan now keys on the common "validated green … on <headSha>" substring so both phrasings are recognized. 3. PS1 human-committer carve-out narrowed to the exact self-commit signature. Test-AnyHumanCommitActor previously suppressed committer-based hand-off whenever the author was a loop-bot, so a maintainer amending a bot-authored commit (author stays bot, committer becomes the maintainer) was wrongly read as non-human and the hand-off was missed. Now suppress ONLY when committer=='web-flow' AND author is a loop-bot (the API-created initial PR commit). Empirically verified against pulls/N/commits that the loop's initial commit has committer.login=web-flow and author.login=github-actions[bot]. 4. Step 8 outcome line: when a cycle chains surfaced-green → marked-ready, record ONLY the terminal marked-ready line so one-line-per-issue aggregators don't double-count. Both locks recompiled (v0.80.9): add_comment max 3→6 + body_hash only; no action or permission drift. Twin symmetry preserved (token-normalized diff still 140). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| use. If ANY required bucket is exhausted, emit NONE of them — record `skipped: per-run | ||
| cap reached; deferring mark-ready PR #<P> to next cycle` and stop this gate. Never mark a | ||
| PR ready (or label it) without also posting its 🎯 audit comment in the same sweep — the | ||
| set either all lands or all defers. |
There was a problem hiding this comment.
Fixed in dbb7370 (both twins). Reworded the T3 atomicity invariant so it no longer contradicts the SUPPRESS_COMMENT re-marking path: it now requires the 🎯 audit comment to be guaranteed to exist for this exact head SHA — either posted in this same sweep, or already present from a prior sweep (the SUPPRESS_COMMENT case where mark-ready did not take). The audit trail is never absent, but it is not needlessly re-posted when it already exists.
| use. If ANY required bucket is exhausted, emit NONE of them — record `skipped: per-run | ||
| cap reached; deferring mark-ready PR #<P> to next cycle` and stop this gate. Never mark a | ||
| PR ready (or label it) without also posting its 🎯 audit comment in the same sweep — the | ||
| set either all lands or all defers. |
…-actor fail-closed, idempotency anchor, build-only gated-pipeline guard Applies four findings from a fresh 3-model adversarial pass on the updated Step 3.6 diff (opus-4.8 / gpt-5.5 / opus-4.6; the third returned NO FINDINGS). All mirrored to both twins + shared prefetch script. 1. p/0 no longer fights a maintainer (opus-4.8⚠️ ). The Step 3.6 precondition re-added `p/0` to an already-ready [ci-fix] PR EVERY sweep when it was missing. The human-engagement guard inspects comments/reviews/commits — NOT label changes — so a maintainer who removed `p/0` to de-prioritize a validated PR (a pure triage action) had it re-added indefinitely, violating the loop's "never override a human" contract. `p/0` is now applied exactly ONCE, atomically with the draft→ready flip (T3); the already-ready branch records `already-ready` and reconciles nothing. Trade-off: a rare transient p/0-drop at flip time is not auto-re-added, but the PR is still ready with its 🎯 audit comment — strictly preferable to steam-rolling a deliberate de-prioritization. 2. Human-engagement guard fails CLOSED on an unidentifiable commit (gpt-5.5⚠️ ). Test-IsHumanLogin(null) is false, so a commit whose author AND committer GitHub could not map to accounts (both null — e.g. a maintainer amending with a git email not linked to GitHub) was invisible to Test-AnyHumanCommitActor and the loop could push over that human's work. The loop's OWN commits always resolve (author github-actions[bot]; committer github-actions[bot] or web-flow), so a fully-unresolvable commit is external: now treated as human engagement. Scoped to BOTH actors unresolvable so it never over-trips the loop's own commits. 3. T3 comment-idempotency anchored on the 🎯 emoji (opus-4.8 💡). The dedup scan now requires the leading `🎯` anchor plus the contiguous `validated green on <headSha>`, so the match can never drift into a gapped form that would false-positive on the Step 3 ✅ surface comment and suppress the audit 🎯. 4. Build-only fallback defers gated-pipeline origins (gpt-5.5⚠️ ). The build-only path validates `maui-pr` (302) whole-build green, but a [ci-fix] PR whose ORIGINATING failure was in a /azp-gated pipeline (maui-pr-uitests 313 / maui-pr-devicetests 314) would be undrafted on maui-pr-green alone — those pipelines never auto-run on the PR, so maui-pr green is not evidence the gated break is fixed. Now defers such PRs to a human instead of marking ready. Both locks recompiled (v0.80.9): body_hash only (prompt body is runtime-imported; no safe-output/permission/action drift). Twin symmetry improved (normalized diff 140→124 as the p/0 block is now byte-identical across twins). PS1 parses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| @@ -268,6 +277,44 @@ safe-outputs: | |||
| # min-integrity:approved; the residual is body-marker edits only (no code, no | |||
| @@ -278,6 +287,44 @@ safe-outputs: | |||
| # min-integrity:approved; the residual is body-marker edits only (no code, no | |||
…ed to either-null actor; de-dup post-flip coverage tally Two findings from a fresh 3-model adversarial pass on 88a84a0 (opus-4.8 verified all four round-3 fixes SOUND + found #2 below; gpt-5.5 PS1-guard pass found #1; second gpt-5.5 pass NO FINDINGS). Both are should-fix, both fail in the safe direction. Mirrored to both twins + shared prefetch script. 1. Human-engagement guard now fails CLOSED on EITHER unresolvable commit actor, not only BOTH (Query-CiFixPRs.ps1, Test-AnyHumanCommitActor). Round 3 closed the both-null gap but left the partial-unmapped case open: a maintainer who runs `git commit --amend` on the bot's commit PRESERVES author=github-actions[bot] but stamps the committer as their own git identity — and if that email is not linked to GitHub, committer.login is null. That commit (author resolved to the bot, committer null) fell through as non-human, so the loop could push over a real human hand-off. Widened `($authorKey -eq '') -and (...)` to `-or`. Provably safe against the loop's own commits: BOTH loop signatures resolve BOTH actors (create-PR = github-actions[bot] + web-flow; push = github-actions[bot] twice), so an either-unresolvable commit is never one of ours and the test never over-trips (never stalls a fresh [ci-fix] PR on its own commit). 2. Step 8 coverage tally no longer double-counts a flipped-ready PR in steady state (ci-status-fix.md). The round-3 flow runs Step 3.6 in ALL cases before stopping, so on every post-flip sweep a still-green ready PR records Step 3's `already-surfaced` AND Step 3.6's `already-ready` — two lines for one issue in one cycle. The existing supersede rule only collapsed the flip-cycle pair (`surfaced-green` → `marked-ready`). Generalized it to also collapse the post-flip steady-state pair (`already-surfaced` → `already-ready`): record ONLY the terminal Step 3.6 readiness line. Added `already-surfaced` and `already-ready` to the Step 8 outcome vocabulary (the latter was absent, so an aggregator could not recognize it as terminal). Artifact-only fix — both lines were already idempotent no-ops; no behavioral change. Both locks recompiled (v0.80.9): body_hash only (prompt body runtime-imported; PS1 invoked by path, not embedded — no lock content change). Twin normalized diff holds at 124 (new passages are token-free, byte-identical across twins). PS1 parses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…sede with Step 3.6 terminal outcomes Two outcome-consistency findings from a fresh 3-model pass on a777089 (opus-4.8 + opus-4.6 verified BOTH round-4 fixes sound and the full A–F multi-sweep lifecycle correct; the two findings below are documentation/tally consistency only — no behavioral change). Mirrored to both twins. 1. Hard-Rule 5 "one issue = one outcome" list now enumerates the Step 3.6 terminal outcomes (gpt-5.5⚠️ ). The canonical list previously stopped at "surface a validated-green PR for review" and never mentioned marking the validated draft ready — the highest-value terminal state this PR introduces. Added "mark a target-validated draft PR ready-for-review (Step 3.6 T3 — the terminal outcome that supersedes the same run's surface/annotate precursor line), or record its `already-ready` steady-state no-op". Keeps the rule consistent with the Step 3.6 precondition, the Step 8 vocabulary, and the decision table. 2. Step 8 tally supersede now covers the flake-coincident flip (opus-4.8 💡). The round-4 supersede clause only collapsed a Step 3 GREEN precursor (`surfaced-green`/`already-surfaced`) into the terminal readiness line, but a PR that is unrelated-flake-red AND target-green in the same cycle posts a Step 4 `annotated-flake` line and THEN a Step 3.6 `marked-ready` line — two lines for one issue. Generalized the rule: the readiness line supersedes ANY same-cycle non-readiness precursor (green line OR `annotated-flake`), and the superseded precursor's signal survives in the PR's 🎯/♻️ comment so no information is lost. Artifact/tally-only (pre-existing reachability; both lines were idempotent no-ops). Both locks recompiled (v0.80.9): body_hash only. Twin normalized diff holds at 124 (both passages token-free, byte-identical across twins). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…tency + pin version ref to v0.80.9 Two doc-accuracy fixes flagged by the Copilot reviewer on PR #36461 (both twins; locks recompiled, metadata-only delta — no config change): 1. T3 atomicity vs idempotency contradiction. The atomicity rule said "never mark a PR ready without also posting its 🎯 audit comment in the same sweep", which directly contradicts the SUPPRESS_COMMENT re-marking path (🎯 already present for this head SHA from a prior sweep, mark-ready did not take → re-mark + label WITHOUT re-posting). Reworded the invariant to "the 🎯 audit comment must be GUARANTEED to exist for this exact head SHA — posted this sweep OR already present from a prior sweep", so the audit trail is never absent yet is not needlessly re-posted. 2. Stale gh-aw version reference. Two passages cited v0.79.8 for the "compiler silently drops required-* for update_pull_request" behavior while the workflow is pinned/compiled with v0.80.9. Re-verified against the v0.80.9 lock: update_pull_request still carries NO required_title_prefix/required_labels (mark-ready + add_labels DO), so the claim holds — only the version label was stale. Bumped to v0.80.9; all version refs now consistent. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
kubaflo
left a comment
There was a problem hiding this comment.
🔍 AI-generated review (multi-model: Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro), on behalf of @kubaflo.
✅ LGTM — re-approving after the human-engagement refinement
The delta since my last approval sharpens Test-AnyHumanCommitActor and the comment-bucket sizing — both are genuine correctness improvements:
- Web-flow carve-out narrowed to the exact self-signature. Previously any bot-authored commit suppressed the hand-off; now suppression fires ONLY for
committer=='web-flow' AND author ∈ $LoopBotCommitAuthors— precisely the gh-aw API-created initial PR commit. A named human who amends/rebases a bot commit keepscommitter != 'web-flow', so it now correctly trips human engagement (the old form wrongly swallowed that real maintainer hand-off). - New fail-closed guard on unmapped actors. If EITHER author or committer login is null/empty (e.g. a maintainer's
git commit --amendwith a git email not linked to GitHub →committer.loginnull while author staysgithub-actions[bot]), it's treated as human engagement. Sound: both loop signatures resolve BOTH actors to real accounts, so an either-unresolvable commit is provably never the loop's own — failing safe toward hands-off honors the "never override a human" contract. add_commentmax 3→6 is justified: a draft-flip sweep spends two comment slots (Step 3 ✅ surface + Step 3.6 T3 🎯), so at 3 the shared bucket starved the mark-ready/add_labels caps; 6 lets ~3 flips land per sweep.
Verified in both compiled locks at dbb7370a: mark_pull_request_as_ready_for_review and add_labels retain required_labels:["agentic-workflows"] + correct per-twin required_title_prefix ([ci-fix] / [ci-fix-net11] ), and add_labels stays allowed:["p/0"]. Scope guards intact.
…dotnet#36461) > [!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! ## What this changes Follow-up to dotnet#36317 (the self-watching `ci-status-fix` loop). Today, when a `[ci-fix]` draft PR's CI comes back green — or red only on **unrelated** flakes — the loop posts a comment but **leaves the PR as a draft forever**. A maintainer has to notice it, confirm the specific fixed test actually passed, and flip it to ready. In practice these validated-green draft PRs sit unreviewed for weeks. This PR adds a **target-test verification + mark-ready gate (Step 3.6)** to both twins (`main` + `net11.0`). When a draft `[ci-fix]` / `[ci-fix-net11]` PR reaches the green-surface or unrelated-flake branch, the loop now drills into the PR's **own** AzDO test-results for the **specific test(s)** the fix targeted. If every target test is `Passed` on ≥1 leg and `Failed` on none (VALIDATED-GREEN), it: - posts a `🎯 Target test validated green on <headSha>` comment (naming the test + legs + buildId), and - transitions the draft PR to **ready for review**. This is a **state transition only** — it never approves and never merges; a human still reviews and merges. Overall red on **unrelated** legs no longer keeps a validated fix parked as a draft. ## How it works - **Preconditions** (all required): PR `isDraft == true`; it is unmistakably this workflow's own PR (`[ci-fix]`/`[ci-fix-net11]` title prefix **and** `agentic-workflows` label); and it was reached from the green or **unrelated-flake** path (never from caused-by-fix — that path advances an attempt instead). - **T1 — identify target test(s)** from the `[ci-scan]` issue signature + the PR diff. If no specific test can be identified (e.g. a product build-break), it records a skip — build-only fixes are validated by overall-green, which the existing green branch already handles. - **T2 — drill AzDO test-results** for the build(s) on the PR's current head SHA, filtered to the target test's `testCaseTitle`. A test that never ran (e.g. an `/azp`-gated `maui-pr-uitests`/`maui-pr-devicetests` leg that wasn't kicked) is **not** validated — the loop records an honest "not yet executed" skip and does **not** mark ready. No overclaiming: a green *sibling* leg is not the target test. - **T3 — mark ready + report**, guarded by a per-head-SHA idempotency marker and the existing `dry_run` gate (dry-run emits nothing and tallies `would-mark-ready`). ## Safe-output Adds the `mark-pull-request-as-ready-for-review` safe-output to both twins (`max: 3`, `target: "*"`, `required-title-prefix` + `required-labels`). Unlike `update-pull-request`, this output's `required-*` guards **do** survive the gh-aw v0.80.9 compile (verified against the generated locks), so which-PR scoping is enforced at the handler level in addition to the Step 3.6 preconditions and `min-integrity: approved`. No gh-aw version bump is required — the capability already exists at our pinned v0.80.9. ## Enabling fix — the loop's own create-PR commit no longer counts as "human engaged" While validating this feature against dotnet#36429 I found the mark-ready path was **unreachable for every loop-owned draft PR**, and traced it to a regression from dotnet#36317's own review-hardening. gh-aw's `create_pull_request` builds a PR's initial commit through the GitHub API, which stamps `author=github-actions[bot]` but **`committer=web-flow`**. Commit `2f6b77b330` (in dotnet#36317) removed `web-flow` from the prefetch's bot-login denylist so a maintainer "Update branch" would correctly hand the PR off — but that also made `Test-AnyHumanCommitActor`'s committer check read the loop's **own** first commit as human engagement. Result: every freshly opened `[ci-fix]` draft PR computed `humanEngaged=true` from commit #1, so the watch loop skipped it forever — never surfacing green, never marking ready. This regression is live on `main` today (all four open loop-owned draft PRs have `committer=web-flow`). Fix (`Query-CiFixPRs.ps1`): a human **committer** only trips the hand-off when the commit **author** is not one of this workflow's own bot identities (`$LoopBotCommitAuthors`). A human **author** still counts unconditionally, so maintainer direct commits and web-flow-authored "Update branch" merges continue to hand off correctly. Unit-tested across all six author/committer shapes. ## Validation - Both twins recompiled with `gh aw compile` (0 errors / 0 warnings); locks show no action-SHA or `compiler_version` drift (only frontmatter/body hash + the new handler config). - `mark_pull_request_as_ready_for_review` config confirmed present in **both** locks with the correct `[ci-fix] ` / `[ci-fix-net11] ` prefixes and `agentic-workflows` label; the `safe_outputs` job carries `pull-requests: write`. - Twin symmetry preserved (only `[ci-fix]` ↔ `[ci-fix-net11]` / `ci-scan` ↔ `ci-scan-net11` token differences). - **Live dry-run against dotnet#36429** (scoped `workflow_dispatch`, `dry_run=true`, this branch): the workflow prefetch now computes `humanEngaged=false` for dotnet#36429 (was `true` pre-fix), and the agent's gate walk correctly advances **past** the human-engaged gate to the CI-pending WAIT gate — `checksSettled=false` because dotnet#36429's macOS `SafeAreaEdges` leg is still queued. Once that leg settles green, the same run path reaches Step 3.6 and marks the PR ready. Emitted zero writes (dry-run). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: PureWeen <223556219+Copilot@users.noreply.github.com>
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
What this changes
Follow-up to #36317 (the self-watching
ci-status-fixloop). Today, when a[ci-fix]draft PR's CI comes back green — or red only on unrelated flakes — the loop posts a comment but leaves the PR as a draft forever. A maintainer has to notice it, confirm the specific fixed test actually passed, and flip it to ready. In practice these validated-green draft PRs sit unreviewed for weeks.This PR adds a target-test verification + mark-ready gate (Step 3.6) to both twins (
main+net11.0). When a draft[ci-fix]/[ci-fix-net11]PR reaches the green-surface or unrelated-flake branch, the loop now drills into the PR's own AzDO test-results for the specific test(s) the fix targeted. If every target test isPassedon ≥1 leg andFailedon none (VALIDATED-GREEN), it:🎯 Target test validated green on <headSha>comment (naming the test + legs + buildId), andThis is a state transition only — it never approves and never merges; a human still reviews and merges. Overall red on unrelated legs no longer keeps a validated fix parked as a draft.
How it works
isDraft == true; it is unmistakably this workflow's own PR ([ci-fix]/[ci-fix-net11]title prefix andagentic-workflowslabel); and it was reached from the green or unrelated-flake path (never from caused-by-fix — that path advances an attempt instead).[ci-scan]issue signature + the PR diff. If no specific test can be identified (e.g. a product build-break), it records a skip — build-only fixes are validated by overall-green, which the existing green branch already handles.testCaseTitle. A test that never ran (e.g. an/azp-gatedmaui-pr-uitests/maui-pr-devicetestsleg that wasn't kicked) is not validated — the loop records an honest "not yet executed" skip and does not mark ready. No overclaiming: a green sibling leg is not the target test.dry_rungate (dry-run emits nothing and tallieswould-mark-ready).Safe-output
Adds the
mark-pull-request-as-ready-for-reviewsafe-output to both twins (max: 3,target: "*",required-title-prefix+required-labels). Unlikeupdate-pull-request, this output'srequired-*guards do survive the gh-aw v0.80.9 compile (verified against the generated locks), so which-PR scoping is enforced at the handler level in addition to the Step 3.6 preconditions andmin-integrity: approved. No gh-aw version bump is required — the capability already exists at our pinned v0.80.9.Enabling fix — the loop's own create-PR commit no longer counts as "human engaged"
While validating this feature against #36429 I found the mark-ready path was unreachable for every loop-owned draft PR, and traced it to a regression from #36317's own review-hardening.
gh-aw's
create_pull_requestbuilds a PR's initial commit through the GitHub API, which stampsauthor=github-actions[bot]butcommitter=web-flow. Commit2f6b77b330(in #36317) removedweb-flowfrom the prefetch's bot-login denylist so a maintainer "Update branch" would correctly hand the PR off — but that also madeTest-AnyHumanCommitActor's committer check read the loop's own first commit as human engagement. Result: every freshly opened[ci-fix]draft PR computedhumanEngaged=truefrom commit #1, so the watch loop skipped it forever — never surfacing green, never marking ready. This regression is live onmaintoday (all four open loop-owned draft PRs havecommitter=web-flow).Fix (
Query-CiFixPRs.ps1): a human committer only trips the hand-off when the commit author is not one of this workflow's own bot identities ($LoopBotCommitAuthors). A human author still counts unconditionally, so maintainer direct commits and web-flow-authored "Update branch" merges continue to hand off correctly. Unit-tested across all six author/committer shapes.Validation
gh aw compile(0 errors / 0 warnings); locks show no action-SHA orcompiler_versiondrift (only frontmatter/body hash + the new handler config).mark_pull_request_as_ready_for_reviewconfig confirmed present in both locks with the correct[ci-fix]/[ci-fix-net11]prefixes andagentic-workflowslabel; thesafe_outputsjob carriespull-requests: write.[ci-fix]↔[ci-fix-net11]/ci-scan↔ci-scan-net11token differences).workflow_dispatch,dry_run=true, this branch): the workflow prefetch now computeshumanEngaged=falsefor [ci-fix] De-flake SafeAreaShouldWorkOnAllShellTabs (StaleElementReferenceException on tab switch) #36429 (wastruepre-fix), and the agent's gate walk correctly advances past the human-engaged gate to the CI-pending WAIT gate —checksSettled=falsebecause [ci-fix] De-flake SafeAreaShouldWorkOnAllShellTabs (StaleElementReferenceException on tab switch) #36429's macOSSafeAreaEdgesleg is still queued. Once that leg settles green, the same run path reaches Step 3.6 and marks the PR ready. Emitted zero writes (dry-run).Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com