Never classify a CLOSED issue as open-on-main in release-readiness - #36410
Conversation
The regression classifier's per-PR aggregation assigned `open-on-main` (Tier 2, ACTIVE regression) to any OPEN candidate fix PR on main without checking the issue's own state. A CLOSED issue with a still-OPEN candidate PR — e.g. a giant 'Candidate' changelog PR that `Fixes`-lists dozens of issues while unmerged — was therefore reported as an active blocking regression. That is contradictory: an unmerged OPEN PR cannot have closed a completed issue (real-world: #35615 shown under 'open-on-main' in the SR9 tracker while candidate #35716 was still open). Add a CLOSED-issue guard: when the aggregated best verdict is exactly `open-on-main` and the issue is CLOSED, first attempt the same comment-prose recovery path 1 uses — a merged fix verifiably present on the SR wins and returns `closed-fix-unlinked` (Tier 3). Otherwise fall to the honest `no-fix-yet` (Tier 3 for a CLOSED issue, non-blocking), since the automation can't pin a verified fix on this SR and the open candidate hasn't merged. The shared recovery logic is extracted into Resolve-ClosedFixUnlinked and called from both the empty-strongPrs CLOSED path and the new guard, preserving the revert guards, fix-phrase gate, and tooling-only skips exactly. Scope is strict: only `open-on-main` + CLOSED is contradictory; every other verdict (merged-*, backport-in-progress, rejected-from-sr, in-sr-*, needs-human-review) is unchanged even for CLOSED issues, and genuinely-OPEN issues still get `open-on-main`. Adds three unit tests covering the reroute, the recovery win, and the OPEN-issue regression guard. 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 -- 36410Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36410" |
Skill Validation Results
✅ Skill Validation Results —
|
There was a problem hiding this comment.
Pull request overview
This PR fixes an inconsistency in the release-readiness regression classifier so that a CLOSED issue is never reported as an active open-on-main regression (Tier 2). It adds a guard to reroute the contradictory open-on-main + CLOSED combination to either closed-fix-unlinked (when a verified fix is present on the SR via comment-prose recovery) or no-fix-yet otherwise, and it factors the comment-prose recovery logic into a reusable helper.
Changes:
- Add
Resolve-ClosedFixUnlinkedhelper to consolidate “comment-prose fix recovery” logic for CLOSED issues. - Add a
Classify-RegressionCandidateguard: if the best verdict isopen-on-mainbut the issue is CLOSED, never emitopen-on-main; instead recover or downgrade. - Add unit tests covering CLOSED+OPEN-candidate scenarios and a regression guard for OPEN issues.
Show a summary per file
| File | Description |
|---|---|
| .github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 | Extracts comment-prose recovery into Resolve-ClosedFixUnlinked and adds the CLOSED+open-on-main guard in Classify-RegressionCandidate. |
| .github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 | Adds unit tests validating the new guard behavior and the recovery path for CLOSED issues. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
| baseRefName = 'inflight/candidate' | ||
| mergedAt = '2026-06-01T00:00:00Z' | ||
| closedAt = '2026-06-01T00:00:00Z' | ||
| body = 'Fixes #35104' |
…ding Adversarial multi-model review of PR #36410 surfaced two non-blocking nits; both are safe, non-behavioral polish (suite stays 829/0): - Test-ReleaseReadiness.ps1: the recovery test's Get-PrInfo fallback returns a fix PR whose body is 'Fixes #35104' — a DIFFERENT issue than the one under test (#35615). That is intentional: it proves Resolve-ClosedFixUnlinked's recovery fires on the COMMENT citation, not the PR body. Restore the explanatory inline comment (mirroring the pre-existing block) so the mock no longer reads as a stray copy-paste stub. Flagged independently by a review model and the repo's own Copilot reviewer. - Get-ReleaseReadiness.ps1: the CLOSED-guard no-fix-yet evidence string said 'the only candidate fix PR', which is inaccurate when multiple OPEN candidate PRs cross-reference the same CLOSED issue (candidateFixPrs already lists all). Drop 'only' for honest phrasing. Advisory text only — not consumed by any tier/verdict logic and asserted by no test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
kubaflo
left a comment
There was a problem hiding this comment.
Note
🔍 AI-generated review (automated multi-model orchestrator, on behalf of @kubaflo) — Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro.
✅ APPROVED — CLOSED-issue guard against contradictory open-on-main
Two changes, both sound:
1. Refactor — extract Resolve-ClosedFixUnlinked. The closed-fix-unlinked fallback I approved in #36111 is lifted verbatim into a named function (same two gates: fix-phrase-only + merged-AND-on-SR, with revert/rollback/tooling drops). No behavior change on the original path — the fallback now just calls the extracted function. Good cleanup that also makes the logic reusable by the new guard below.
2. New guard — open-on-main + CLOSED is contradictory. An unmerged PR cannot have closed an issue, so attributing a giant still-open [Candidate] changelog PR's OPEN state to an already-closed issue (real-world #35615 under SR9 via #35716) is a false Tier-2 blocker. The guard reroutes only that exact combination: first retry the comment-prose recovery (a verified merged-on-SR fix wins → closed-fix-unlinked), else the honest no-fix-yet.
The scoping is exactly right — strictly open-on-main + CLOSED; every other verdict (merged-, backport-in-progress, rejected-from-sr, in-sr-, needs-human-review) is left untouched for CLOSED issues since those remain actionable.
Tests — thorough
- Test 1: CLOSED + open candidate + no comment fix →
no-fix-yet, and verifiesGet-OverallVerdictdowngrades it to non-blocking 🟢 (the whole point). - Test 2: CLOSED + comment-cited merged fix on SR → recovery wins →
closed-fix-unlinked(Tier 3). Nicely, the fixture sets the cited PR's body to a different issue (#35104) to prove recovery fires on the comment citation, not the PR body. - Test 3 (regression guard): OPEN issue + open candidate → stays
open-on-main, proving the guard is CLOSED-only.
CI green (Static validation, Validate (PR), Skill Validation all pass). Clean, well-tested, resolves a real false-positive. Verdict: Approve.
…otnet#36410) <!-- 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! ## Why The release-readiness regression classifier could report a **CLOSED** issue as an active `open-on-main` regression (Tier 2 — "wait for main merge, then backport"). That's contradictory: an unmerged OPEN PR cannot have closed a completed issue. It happens when a giant still-open "Candidate" changelog PR `Fixes`-lists dozens of issues — its OPEN state gets attributed to an already-completed issue. Real-world: issue dotnet#35615 (CLOSED/COMPLETED) surfaced under `open-on-main` in the SR9 tracker while candidate PR dotnet#35716 was still open. ## What `Classify-RegressionCandidate` now enforces: **never emit `open-on-main` for a CLOSED issue.** When the aggregated best verdict is exactly `open-on-main` and the issue is CLOSED: 1. First try the same comment-prose recovery the empty-candidate path uses — a merged fix verifiably present on the SR wins → `closed-fix-unlinked` (Tier 3, "no ship risk; add a closing reference for traceability"). 2. Otherwise fall to the honest `no-fix-yet` (Tier 3 for a CLOSED issue via the existing `Get-OverallVerdict` downgrade) — the automation can't pin a verified fix on this SR and the open candidate hasn't merged. The shared recovery logic is extracted into a new `Resolve-ClosedFixUnlinked` helper, called from both the empty-`strongPrs` CLOSED path and the new guard, preserving the fix-phrase gate, merged-on-SR gate, revert guards, and tooling-only skips exactly. **Strictly scoped:** only `open-on-main` + CLOSED is contradictory. Every other verdict (`merged-on-main-no-backport`, `backport-in-progress`, `rejected-from-sr`, `in-sr-*`, `needs-human-review`) is unchanged even for CLOSED issues — those remain legitimately actionable (the SR may still need the backport). Genuinely-OPEN issues still get `open-on-main`. ## Tests Three new unit tests in `Test-ReleaseReadiness.ps1`: 1. CLOSED issue + OPEN candidate on main + no comment fix → `no-fix-yet`, proven non-blocking (🟢) via `Get-OverallVerdict`. 2. CLOSED issue + OPEN candidate + comment-cited merged fix on SR → `closed-fix-unlinked` (recovery wins). 3. Regression guard: OPEN issue + OPEN candidate → stays `open-on-main`. Full suite green (829/0). 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!
Why
The release-readiness regression classifier could report a CLOSED issue as an active
open-on-mainregression (Tier 2 — "wait for main merge, then backport"). That's contradictory: an unmerged OPEN PR cannot have closed a completed issue. It happens when a giant still-open "Candidate" changelog PRFixes-lists dozens of issues — its OPEN state gets attributed to an already-completed issue. Real-world: issue #35615 (CLOSED/COMPLETED) surfaced underopen-on-mainin the SR9 tracker while candidate PR #35716 was still open.What
Classify-RegressionCandidatenow enforces: never emitopen-on-mainfor a CLOSED issue. When the aggregated best verdict is exactlyopen-on-mainand the issue is CLOSED:closed-fix-unlinked(Tier 3, "no ship risk; add a closing reference for traceability").no-fix-yet(Tier 3 for a CLOSED issue via the existingGet-OverallVerdictdowngrade) — the automation can't pin a verified fix on this SR and the open candidate hasn't merged.The shared recovery logic is extracted into a new
Resolve-ClosedFixUnlinkedhelper, called from both the empty-strongPrsCLOSED path and the new guard, preserving the fix-phrase gate, merged-on-SR gate, revert guards, and tooling-only skips exactly.Strictly scoped: only
open-on-main+ CLOSED is contradictory. Every other verdict (merged-on-main-no-backport,backport-in-progress,rejected-from-sr,in-sr-*,needs-human-review) is unchanged even for CLOSED issues — those remain legitimately actionable (the SR may still need the backport). Genuinely-OPEN issues still getopen-on-main.Tests
Three new unit tests in
Test-ReleaseReadiness.ps1:no-fix-yet, proven non-blocking (🟢) viaGet-OverallVerdict.closed-fix-unlinked(recovery wins).open-on-main.Full suite green (829/0).
Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com