ci-fix: mark validated draft PRs ready when the fixed test goes green#36461
Open
PureWeen wants to merge 12 commits into
Open
ci-fix: mark validated draft PRs ready when the fixed test goes green#36461PureWeen wants to merge 12 commits into
PureWeen wants to merge 12 commits into
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>
Contributor
|
🚀 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" |
PureWeen
had a problem deploying
to
copilot-pat-pool
GitHub Actions
Failure
github-actions
Bot
added
the
area-infrastructure
Contributor
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
Comment on lines
+818
to
+822
| 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)"' |
Member
Author
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.
Comment on lines
+828
to
+832
| 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)"' |
Member
Author
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.
Comment on lines
+840
to
+846
| - 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>`. |
Member
Author
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.
Comment on lines
+850
to
+856
| - 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>`. |
Member
Author
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>
Comment on lines
+820
to
+822
| # 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)"' |
Member
Author
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.
Comment on lines
+830
to
+832
| # 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)"' |
Member
Author
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.
Comment on lines
+887
to
+890
| Validate instead at **leg granularity** using ONLY the anonymous `_apis/build/...` timeline | ||
| (Hard-Rule 8) — which is a STRONGER bar anyway: a `succeeded` category leg means every test in | ||
| that category, including the target, passed on that platform (a de-flaked / target test is | ||
| never skipped, so a green category leg cannot hide a target-test failure). |
Member
Author
There was a problem hiding this comment.
Fixed in 32a06ea. T2 no longer treats a succeeded leg as unconditional proof the specific target ran. Added a diff-based guard: before trusting leg-green, confirm the PR's own diff does not add [Ignore]/[Explicit], Assert.Ignore/Assert.Inconclusive, Skip=, a category exclusion, an #if-out, or an early return around the target. If the target could be runtime-skipped, leg-green is insufficient and the PR defers to a human instead of being marked ready.
Comment on lines
+897
to
+900
| Validate instead at **leg granularity** using ONLY the anonymous `_apis/build/...` timeline | ||
| (Hard-Rule 8) — which is a STRONGER bar anyway: a `succeeded` category leg means every test in | ||
| that category, including the target, passed on that platform (a de-flaked / target test is | ||
| never skipped, so a green category leg cannot hide a target-test failure). |
Member
Author
There was a problem hiding this comment.
Fixed in 32a06ea. T2 no longer treats a succeeded leg as unconditional proof the specific target ran. Added a diff-based guard: before trusting leg-green, confirm the PR's own diff does not add [Ignore]/[Explicit], Assert.Ignore/Assert.Inconclusive, Skip=, a category exclusion, an #if-out, or an early return around the target. If the target could be runtime-skipped, leg-green is insufficient and the PR defers to a human instead of being marked ready.
Comment on lines
+788
to
+791
| - else `add_comment` on PR #<P>: `✅ Attempt <attempt>/10 validated — the fix's CI is | ||
| green on <C.headSha>. Ready for human review.` naming any `/azp`-gated legs (uitests | ||
| def 313 / devicetests def 314) that have not run and still need a maintainer `/azp | ||
| run`; record `surfaced-green PR #<P> (attempt <attempt>/10)`. |
Member
Author
There was a problem hiding this comment.
Fixed in 32a06ea. The ✅ surface comment now emits "Ready for human review" only when the PR is not a draft. On a still-draft PR it instead says the cross-platform readiness gate decides the flip, and Step 3.6's 🎯 comment is the sole ready-for-review announcement — so the message can't claim readiness while the PR is still a draft.
Comment on lines
+798
to
+801
| - else `add_comment` on PR #<P>: `✅ Attempt <attempt>/10 validated — the fix's CI is | ||
| green on <C.headSha>. Ready for human review.` naming any `/azp`-gated legs (uitests | ||
| def 313 / devicetests def 314) that have not run and still need a maintainer `/azp | ||
| run`; record `surfaced-green PR #<P> (attempt <attempt>/10)`. |
Member
Author
There was a problem hiding this comment.
Fixed in 32a06ea. The ✅ surface comment now emits "Ready for human review" only when the PR is not a draft. On a still-draft PR it instead says the cross-platform readiness gate decides the flip, and Step 3.6's 🎯 comment is the sole ready-for-review announcement — so the message can't claim readiness while the PR is still a draft.
kubaflo
previously approved these changes
Jul 9, 2026
kubaflo
left a comment
kubaflo
left a comment
Contributor
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-affirming after the mark-ready hardening (e18cec28 → 45a45f00)
Re-reviewed the 4 new commits. All improvements, verified against the compiled locks:
- Stronger VALIDATED-GREEN (T2) — now requires the target's category leg
succeededon every platform that runs it (failed on none, none unverified), not just ≥1 leg. A platform with no leg for the category is correctly treated as "not a gap." This reduces premature mark-ready. Good. - New
add-labelssafe-output — both locks compile it with a HARD allowlistallowed:["p/0"]+required_title_prefix([ci-fix]/[ci-fix-net11]) +required_labels:["agentic-workflows"], so the agent can only ever addp/0, only on its own PRs, only on the draft→ready transition (all-or-nothing with the comment + mark-ready). Verified server-side, not just prompt-level. mark_pull_request_as_ready_for_reviewscope guards still intact in both twins.Query-CiFixPRs.ps1change is a sync-maintenance comment keeping$BotLogins/$LoopBotCommitAuthorsfrom drifting — no behavior change.
CI: .github-only, maui-pr skipping, no red. Nice hardening pass.
Resolve the three still-live Copilot-reviewer comments on #36461 (each mirrored to both twins). The eight earlier comments about `_apis/test` paging / testCaseTitle-vs-automatedTestName are already obsolete — the prior commit replaced that query with the anonymous `_apis/build/.../timeline` leg-level method — so GitHub marks those threads outdated. A. T1 "no target test" no longer strands build-only fixes as permanent drafts. The old text stopped and claimed "Step 3 handles overall-green", but Step 3 only comments — Step 3.6 is the sole draft->ready flip. Now a build-only fix validates at WHOLE-BUILD granularity: require every maui-pr (def 302) platform build leg succeeded/completed on the head SHA (cross-platform guard applied to the build), set a build-only TARGET, and proceed to T3 to mark ready. T3's comment gains a build-only phrasing variant. B. T2 no longer treats a succeeded category leg as unconditional proof the specific target ran. A job can go green while the target is runtime-skipped (`[Ignore]`, `Assert.Ignore/Inconclusive`, `Skip=`, `#if`-out, early return). Added a diff-based guard: confirm the fix does NOT add any skip/ignore/ short-circuit around the target; if it might, leg-green is insufficient and the PR defers to a human instead of being marked ready. C. Step 3's green-surface ✅ comment no longer overclaims. It said "Ready for human review" even when the PR stays a draft (3.6 may not flip it). Now the "Ready for human review" clause is emitted only when the PR is NOT a draft; a still-draft PR gets a truthful note that the readiness gate decides the flip, and 3.6's 🎯 comment remains the sole ready-for-review announcement. Both locks recompiled 0/0 (body_hash-only delta). Twin symmetry preserved (token-normalized diff unchanged). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment on lines
+975
to
+989
| - **Comment idempotency (dup-suppress only — never gates the mark-ready).** We only reach | ||
| T3 while `C.isDraft == true` (the precondition stops an already-ready PR before here). So | ||
| if a prior bot `🎯 … target test … validated … on <C.headSha>` comment for THIS head | ||
| already exists, the comment landed on an earlier sweep but the **mark-ready did NOT take | ||
| effect** (the PR is still a draft) — set `SUPPRESS_COMMENT = true` (do not re-post the | ||
| duplicate 🎯 comment) but STILL complete the mark-ready + label below. Never treat the | ||
| comment's existence as "already marked ready" while the PR is still a draft. Record this | ||
| case as `re-marking PR #<P> (🎯 present; mark-ready did not take on a prior sweep)`. | ||
| - **Atomicity budget pre-check (Hard-Rule 7).** Determine the outputs this gate will emit: | ||
| `mark_pull_request_as_ready_for_review` + `add_labels` always, plus `add_comment` unless | ||
| `SUPPRESS_COMMENT`. Verify a free per-run slot remains in EACH bucket you are about to | ||
| 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. |
Comment on lines
+402
to
+404
| ever land on THIS workflow's own PRs. `update_pull_request` CANNOT be | ||
| config-locked to a title/label in gh-aw v0.79.8 (the compiler silently drops | ||
| `required-*` for that output — verified against the lock), so it keeps |
Comment on lines
+985
to
+999
| - **Comment idempotency (dup-suppress only — never gates the mark-ready).** We only reach | ||
| T3 while `C.isDraft == true` (the precondition stops an already-ready PR before here). So | ||
| if a prior bot `🎯 … target test … validated … on <C.headSha>` comment for THIS head | ||
| already exists, the comment landed on an earlier sweep but the **mark-ready did NOT take | ||
| effect** (the PR is still a draft) — set `SUPPRESS_COMMENT = true` (do not re-post the | ||
| duplicate 🎯 comment) but STILL complete the mark-ready + label below. Never treat the | ||
| comment's existence as "already marked ready" while the PR is still a draft. Record this | ||
| case as `re-marking PR #<P> (🎯 present; mark-ready did not take on a prior sweep)`. | ||
| - **Atomicity budget pre-check (Hard-Rule 7).** Determine the outputs this gate will emit: | ||
| `mark_pull_request_as_ready_for_review` + `add_labels` always, plus `add_comment` unless | ||
| `SUPPRESS_COMMENT`. Verify a free per-run slot remains in EACH bucket you are about to | ||
| 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. |
Comment on lines
+412
to
+414
| ever land on THIS workflow's own PRs. `update_pull_request` CANNOT be | ||
| config-locked to a title/label in gh-aw v0.79.8 (the compiler silently drops | ||
| `required-*` for that output — verified against the lock), so it keeps |
…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>
Comment on lines
+999
to
+1002
| 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. |
Member
Author
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.
Comment on lines
+1009
to
+1012
| 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>
Comment on lines
273
to
277
| @@ -268,6 +277,44 @@ safe-outputs: | |||
| # min-integrity:approved; the residual is body-marker edits only (no code, no | |||
Comment on lines
283
to
287
| @@ -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
approved these changes
Jul 10, 2026
kubaflo
left a comment
kubaflo
left a comment
Contributor
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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