Skip to content

Surface p/0 PRs as Preview blockers; drop stale below-watermark SR branches from the tracker matrix - #35971

Merged
PureWeen merged 8 commits into
mainfrom
pureween/preview-p0-pr-blockers
Jun 18, 2026
Merged

Surface p/0 PRs as Preview blockers; drop stale below-watermark SR branches from the tracker matrix#35971
PureWeen merged 8 commits into
mainfrom
pureween/preview-p0-pr-blockers

Conversation

@PureWeen

@PureWeen PureWeen commented Jun 17, 2026

Copy link
Copy Markdown
Member

Note

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

Follow-up to #35807. Two independent release-readiness fixes.


Fix 1 — Surface p/0-labelled PRs as Preview release blockers

Summary

The Preview readiness engine (Get-PreviewReadiness.ps1) only treated p/0 issues as release blockers. p/0-labelled PRs targeting a preview/candidate branch were silently bucketed into the generic "Release branch PRs" WATCH count and rendered as "Needs review or triage" rows — never hoisted, never blocking.

The root cause is structural: the p/0 blocker path used gh issue list --label p/0, which by design never returns PRs. So p/0 PRs were invisible to the blocker logic.

This was observed live on the net11.0 preview6 tracker (#35866), where #34758, #35626, and #34600 (all p/0, base net11.0) appeared only as generic WATCH rows instead of blockers.

What changed

Carves p/0-labelled PRs out of the generic human-PR bucket — the label data is already fetched by Get-OpenPullRequests, so no extra API call — and:

  • adds a BLOCKED P/0 release-branch PRs check (parallel to the p/0-issues check) so the overall verdict turns red when one is open;
  • itemizes each p/0 PR as a 🔥 P/0 PR row in the hoisted 🔴 High-priority items section (with base ref + age + per-PR next action);
  • excludes the new check from the 🔴 Blocking summary (its PRs are already enumerated in the hoist) — exactly matching the p/0-issues treatment;
  • updates the WATCH note + hoist header/intro text from 3 → 4 high-priority categories.

A PR whose base is the survey ref is release-relevant by definition, so — unlike issues — no title/milestone relevance filter is applied.

Testability

Adds a small StrictMode-safe Test-IsP0Pr helper plus a dot-source guard on the engine (mirroring Find-ReleaseReadinessTrackers.ps1) so the predicate can be unit-tested without invoking the full git/gh-backed report flow.


Fix 2 — Drop stale below-watermark SR branches from the tracker matrix

Summary

The Lane 1 in-flight detector (Find-ReleaseReadinessTrackers.ps1) treated tag-absence as the sole in-flight signal. Abandoned hotfix leftovers like SR2 (patch 21) and SR3 (patch 33) — which never published their stable tags and sit far below the shipped watermark (SR7 patch 71) — were still emitted as trackers. The workflow then spun up a no-op matrix job per branch: the per-job activity gate skipped issue creation, but the job still ran.

What changed

Adds a secondary Test-IsStaleSrBranch disambiguator applied only after Test-IsBranchInFlight returns true. A branch is stale when both:

  • its patch is strictly below the highest shipped patch, and
  • it has had no commits within the activity window (idle).

Tag-existence stays the primary signal; the idle requirement preserves the out-of-order / security-hotfix case — a real reset branch below the watermark has recent commits and is therefore not dropped. Freshly-cut live SRs sit at/above the watermark and are never affected.

Dropping these at the detector removes them from the workflow matrix entirely. Verified safe: SR2/SR3 have no open tracker issues, so nothing is stranded (only SR8/SR9/preview6 have open trackers).


Tests

Test-ReleaseReadiness.ps1:

  • 12 new unit assertions for Test-IsP0Pr (predicate: p/0 present/absent, missing/null/empty labels, hashtable-shaped labels, null PR; carve-out semantics: p/0 subset selected, generic bucket excludes them).
  • 7 new unit assertions for Test-IsStaleSrBranch (below-watermark idle → stale; above/equal watermark → not stale; below-watermark but active → not stale; no shipped tags → never fires).
  • Live-repo E2E expectations updated: net10 now surfaces 2 SR trackers (SR8 + SR9) instead of 4.
Passed: 517   Failed: 0

The Preview readiness engine only treated p/0 *issues* as release
blockers. p/0-labelled PRs targeting a preview/candidate branch (e.g.
#34758, #35626 against net11.0) were silently bucketed into the generic
"Release branch PRs" WATCH count and rendered as "Needs review or triage"
rows — never hoisted, never blocking. The issues-only path used
`gh issue list --label p/0`, which by design never returns PRs, so p/0
PRs were structurally invisible as blockers.

This carves p/0-labelled PRs out of the generic human-PR bucket (the
label data is already fetched by Get-OpenPullRequests, so no extra API
call) and:

- adds a BLOCKED "P/0 release-branch PRs" check (parallel to the p/0
  issues check) so the overall verdict turns red when one is open;
- itemizes each p/0 PR as a "🔥 P/0 PR" row in the hoisted
  "🔴 High-priority items" section;
- excludes the new check from the "🔴 Blocking" summary (its PRs are
  already enumerated in the hoist), matching the p/0-issues treatment.

A PR whose base IS the survey ref is release-relevant by definition, so
— unlike issues — no title/milestone relevance filter is applied.

Adds a StrictMode-safe Test-IsP0Pr helper plus a dot-source guard on the
engine (mirroring Find-ReleaseReadinessTrackers.ps1) so the predicate can
be unit-tested without invoking the full git/gh-backed report flow.

Follow-up to #35807.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35971

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35971"

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Skill Validation Results

@PureWeen — new skill validation results are available based on this last commit: b4b0bbd.
To request a fresh validation after new comments or commits, comment /evaluate-skills.

Overall Passed Static Passed LLM Skipped Skills 20 Agents 5

Skill Validation Resultsb4b0bbd · Surface p/0 PRs as Preview blockers; drop stale below-watermark SR branches from the tracker matrix · 2026-06-17T17:45:19Z

✅ Static Checks Passed

Skills checked: 20 | Agents checked: 5

Full validator output
Found 1 skill(s)
[release-readiness] 📊 release-readiness: 5,530 BPE tokens [chars/4: 5,406] (comprehensive ✗), 40 sections, 5 code blocks
[release-readiness]    ⚠  Skill is 5,530 BPE tokens (chars/4 estimate: 5,406) — "comprehensive" skills hurt performance by 2.9pp on average. Consider splitting into 2–3 focused skills.
✅ All checks passed (1 skill(s))
Found 5 agent(s)
Validated 5 agent(s)
✅ All checks passed (5 agent(s))

⏭️ LLM Evaluation: Skipped

No changed skills with eval tests found.

🔍 Full results and investigation steps

@github-actions github-actions Bot added the area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions label Jun 17, 2026
The Lane 1 in-flight detector treated tag-absence as the sole in-flight
signal, so abandoned hotfix leftovers like SR2 (patch 21) and SR3
(patch 33) — which never published their stable tags and sit far below
the shipped watermark (SR7 patch 71) — were still emitted as trackers.
The workflow then spun up a no-op matrix job per branch (the per-job
activity gate skipped issue creation, but the job still ran).

Add a secondary `Test-IsStaleSrBranch` disambiguator applied only after
`Test-IsBranchInFlight` returns true. A branch is stale when BOTH its
patch is strictly below the highest shipped patch AND it has had no
commits within the activity window. Tag-existence stays the primary
signal; the idle requirement preserves the out-of-order/security-hotfix
case (a real reset branch below the watermark has recent commits and is
therefore not dropped). Freshly-cut live SRs sit at/above the watermark
and are never affected.

Dropping these at the detector removes them from the workflow matrix
entirely. Verified safe: SR2/SR3 have no open tracker issues, so nothing
is stranded.

Adds 7 unit assertions for the predicate and updates the live-repo E2E
expectations (net10 now surfaces 2 SR trackers — SR8 + SR9 — instead of
4). Full suite: 517 passed, 0 failed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen PureWeen changed the title Surface p/0-labelled PRs as Preview release blockers Surface p/0 PRs as Preview blockers; drop stale below-watermark SR branches from the tracker matrix Jun 17, 2026
PureWeen and others added 2 commits June 17, 2026 12:35
…tions

Two fixes from a multi-model adversarial review of this PR:

- Get-PreviewReadiness.ps1: add `P0PullRequests` to the JSON `$report`.
  The p/0-PR carve-out mutates `$targetHumanPRs` (serialized as
  `ReleasePullRequests`), so p/0 PRs were dropped from the structured
  output entirely — a JSON consumer saw only the BLOCKED-check count, not
  the PR objects. p/0 *issues* stay in `PriorityIssues` (that path filters,
  not removes), so this restores PR/issue parity. Purely additive field.

- Test-ReleaseReadiness.ps1: replace the SR2/SR3/SR7 "tracker absent"
  checks (if/else guard + tautological `Assert-Eq -Expected $true
  -Actual $true`) with direct `Assert-Eq -Expected $false -Actual
  ($bySr.ContainsKey(N))`. The assertion now carries the real signal and
  produces a meaningful expected/actual diff on failure.

Full suite: Passed 517, Failed 0 (live E2E included).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-IsP0Pr

Three consistency fixes surfaced by a second adversarial review pass:

- Get-PreviewReadiness.ps1: add 'Maestro PRs' to $highPriorityCheckAreas.
  Maestro PRs are hoisted into the 🔴 High-priority items table as
  '📦 Maestro PR' rows, but the Area was missing from the exclusion list,
  so a BLOCKED Maestro check rendered a second time in the Blocking
  summary. This matches the documented intent of that array (exclude every
  hoisted category to avoid duplicate rows under two headings).

- Get-PreviewReadiness.ps1: add `MergeUpPullRequests = $mergeUpPRs` to the
  JSON $report. Like the p/0 PRs, merge-up PRs are carved out of
  $targetHumanPRs (serialized as ReleasePullRequests) and were otherwise
  absent from the structured output. Completes the JSON-parity fix.

- Get-PreviewReadiness.ps1: Test-IsP0Pr now handles a whole-PR-as-hashtable
  (IDictionary) shape via Contains/indexer, not just PSCustomObject. A
  hashtable PR's PSObject.Properties has no 'labels' member, so the prior
  code silently returned $false for that (test-mock) shape. Mirrors the
  dual-shape handling already in Get-ReleaseReadiness.ps1. Production data
  (gh --json) is unaffected (still the PSCustomObject path).

Adds 3 unit assertions for the hashtable-PR shape. Full suite: 520/0
(live E2E included).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Multi-model code review — PR #35971

3-model consensus: LGTM with 2 non-blocking suggestions

What this changes

Two independent release-readiness infrastructure fixes:

  1. Surface p/0 PRs as Preview blockers — Previously only p/0 issues were hoisted as blockers (via gh issue list --label p/0, which never returns PRs). Now carves p/0-labelled PRs out of the generic bucket and surfaces them in the 🔴 High-priority blocker section, with a dedicated "P/0 release-branch PRs" BLOCKED/READY check.

  2. Drop stale below-watermark SR branches — Abandoned hotfix leftovers (SR2/SR3) below the shipped watermark (SR7 patch 71) were still emitted as trackers because tag-absence kept them "in-flight". The new Test-IsStaleSrBranch disambiguator drops branches that are BOTH below the highest shipped patch AND idle (no commits in the activity window), removing them from the tracker matrix entirely.

Review approach

Independent phase: 3 models (Claude Opus 4.8, GPT-5.5, Gemini 3.1 Pro) each read full source files, diff, git history, and CI status without seeing each other's outputs.

Cross-pollination phase: All 3 models reconciled their findings, resolved severity disputes, and confirmed validity.

Findings

💡 Suggestion 1 — Staleness guard edge-case

File: .github/skills/release-readiness/scripts/Find-ReleaseReadinessTrackers.ps1:717
Category: edge-case
All 3 models flagged this

The staleness guard re-introduces a below-watermark heuristic that could drop a legitimate tag-absent SR if it goes idle >7 days AND a higher SR ships out-of-order (advancing the watermark above it).

Why non-blocking:

  • Impact is soft and self-healing: any commit (including inevitable release-time version bump) or tag publish re-surfaces the branch
  • Probability is low under MAUI's in-order SR cadence (requires out-of-order ship + below-watermark + tag-absent + 7d idle + existing tracker)
  • Consistent with workflow's existing design (already treats "zero recent commits" as a suppression signal)
  • Code documents the trade-off (docstring lines 303-308)

Optional follow-up: Add a stronger stale signal (explicit deny-list, branch age threshold, or honor existing tracker issues) to fully close the gap.

💡 Suggestion 2 — Maestro p/0 PR edge-case

File: .github/skills/release-readiness/scripts/Get-PreviewReadiness.ps1:1040
Category: edge-case
Confirmed by all 3 models after cross-pollination

The p/0 carve-out derives from $targetHumanPRs (after Maestro PRs removed), so a p/0-labelled Maestro PR won't trigger the "P/0 release-branch PRs" BLOCKED check.

Why low impact:

  • All Maestro PRs are independently hoisted to 🔴 High-priority items regardless of label, so the item isn't invisible
  • Only the dedicated BLOCKED escalation is missed
  • Rare combo: mergeable, non-conflicted, p/0-labelled automated PR

Optional follow-up: If intent is that ANY p/0 PR blocks shipment, derive $p0Prs from the full target set before the Maestro split (while still rendering under a single category to avoid dupes).

Verification performed by reviewers

  • ✅ Read full source files (not just diff hunks)
  • ✅ Ran unit suite: all 7 Test-IsStaleSrBranch + 15 Test-IsP0Pr new assertions pass (517 total passed, 0 failed)
  • ✅ Confirmed lone test failure (T8 in Get-ExpectedShipDate) is pre-existing, timezone-flaky, and outside this diff
  • ✅ Verified StrictMode-safety of Test-IsP0Pr (null PR, missing/null/empty labels, PSCustomObject vs IDictionary shapes — all covered)
  • ✅ Confirmed no shell-injection surface (gh array args, markdown properly escaped)
  • ✅ Verified dot-source guard doesn't break production (pwsh -File path runs the main driver)
  • ✅ Confirmed staleness continue is safe for Lane 2 logic
  • ✅ CI checks: passing

Verdict

LGTM (2 of 3 models; Gemini raised to NEEDS_DISCUSSION on staleness severity, then reconciled to non-blocking after cross-pollination)

Confidence: High

Reasoning: Both fixes correctly target real root causes with minimal, safe, well-tested logic. The 22 new unit assertions all pass. The 2 suggestions are documented, low-probability edge-cases with soft impact — worth noting for completeness, but they don't gate merging a clear net improvement that fixes the reported SR2/SR3 no-op tracker issue and the p/0 PR visibility gap observed on net11.0 preview6.


Review conducted by: @kubaflo's autonomous multi-model review loop
Models: Claude Opus 4.8, GPT-5.5, Gemini 3.1 Pro
Timestamp: 2026-06-17T18:44Z

# workflow matrix entirely (no no-op per-tracker job). Tag-existence
# stays the primary signal; the idle requirement preserves the
# out-of-order/hotfix case (a real reset branch has recent commits).
if (Test-IsStaleSrBranch -BranchPatch $branchPatch -HighestShippedPatch $highestShippedPatch -RecentActivityCount $recent) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 [3-model consensus] The staleness guard (Test-IsStaleSrBranch) re-introduces a below-watermark heuristic that could drop a legitimate tag-absent SR if it goes idle >7 days AND a higher SR ships out-of-order (advancing the watermark). Impact is soft (self-healing: any commit or tag publish re-surfaces it) and probability is low under MAUI's in-order SR cadence, but consider adding a stronger stale signal (explicit deny-list, branch age threshold, or honoring existing tracker issues) to fully close the gap. Non-blocking — the code documents the trade-off and correctly filters the reported abandoned SR2/SR3 branches.

# IS the survey ref is release-relevant by definition, so — unlike issues — no
# title/milestone relevance filter is needed. Labels are already fetched by
# Get-OpenPullRequests, so this needs no extra API call.
$p0Prs = @($targetHumanPRs | Where-Object { Test-IsP0Pr $_ })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 [3-model consensus after cross-pollination] The p/0 carve-out derives from $targetHumanPRs (after Maestro PRs removed), so a p/0-labelled Maestro PR won't trigger the "P/0 release-branch PRs" BLOCKED check. Low impact: all Maestro PRs are independently hoisted to 🔴 High-priority regardless of label, so the item isn't invisible — only the dedicated BLOCKED escalation is missed for the rare combo of a mergeable, non-conflicted, p/0-labelled automated PR. If intent is that ANY p/0 PR blocks shipment, derive $p0Prs from the full target set before the Maestro split. Non-blocking.

PureWeen and others added 3 commits June 17, 2026 15:25
…w engine

A p/0-labelled Maestro or merge-up PR previously did NOT trip the dedicated
"P/0 release-branch PRs" BLOCKED check: $p0Prs was derived from $targetHumanPRs
(after Maestro AND merge-up PRs had already been removed), so an automated PR
carrying a p/0 label was silently downgraded to a Maestro/merge-up row instead
of being escalated.

Reorder the carve-out so p/0 PRs are extracted FIRST from the full survey-ref
target set, then the Maestro / human / merge-up splits exclude those numbers.
This makes p/0 the strongest categorization signal: any p/0 PR (regardless of
author or merge-up status) is surfaced once as a 🔥 P/0 PR row and trips the
BLOCKED check, with no double-counting in the Maestro/generic buckets. Inflight
(net<major>.0) p/0 PRs remain non-escalated, consistent with prior behavior.

Add 9 regression assertions modeling a mixed set (human/Maestro/merge-up × p/0)
that verify the precedence ordering and bucket exclusions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…real logic

The round-5 "precedence" unit tests re-implemented the engine's PR carve-out
filter expressions inline and asserted on that parallel copy, so a regression in
the engine's own expressions (e.g. a typo in the live `-notcontains` clause)
would have passed the tests — they verified a transcription, not the engine.

Extract the carve-out precedence logic into a pure, side-effect-free function
`Get-CategorizedPullRequests` (defined before the dot-source guard so tests can
call it). The driver now calls it and destructures the five returned buckets
(P0Prs, MaestroPRs, MergeUpPRs, TargetHumanPRs, InflightHumanPRs); the unit
tests call the same function, so they exercise the exact code the engine runs.

Behavior is unchanged (verified: 539 tests pass; live in-flight run against
release/11.0.1xx-preview6 produces an identical report and buckets inflight
Maestro PRs correctly). Expand coverage with inflight-scope cases (inflight
Maestro PR still buckets as Maestro; inflight p/0 PR does NOT escalate — only
survey-ref PRs block) and an empty-input safety case.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Get-OpenPullRequests returns AutomationNull (not a literal @()) for a branch
with no open PRs, because an empty `gh pr list` result collapses through
`return @()`. When that AutomationNull is bound to the [array] parameters of
Get-CategorizedPullRequests it becomes $null, and @($null) seeds a single-element
array whose lone element is $null. Iterating it and dereferencing $_.author under
`Set-StrictMode -Version Latest` throws "The property 'author' cannot be found on
this object".

Reachable trigger: an in-flight release-readiness run against a release branch
that EXISTS but has zero open PRs (e.g. a freshly-cut preview branch before any
PRs target it) while the inflight net<major>.0 branch carries Maestro PRs —
@($null) + @(maestroPRs) puts the null element first and crashes the report.
The earlier preview6 live runs dodged it only because preview6 isn't cut yet
(404 → surveyExists=False → targetPRs stayed a clean literal @()).

Fix: normalize both inputs at the top of the function by stripping $null
elements, making it robust to null / AutomationNull / @($null) inputs. Add
regression tests that reproduce the exact AutomationNull value via
ConvertFrom-JsonOrEmptyArray '[]' (a literal @() does NOT reproduce the bug).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Multi-Model Review — Round 2

Verdict: ✅ LGTM (unanimous)

All three models independently reviewed round-2 changes and reached LGTM. Round-1 findings have been addressed:

  • ✅ P/0 PR precedence fixed via Get-CategorizedPullRequests extraction
  • ✅ StrictMode/null hardening added (AutomationNull, function guards)
  • ✅ Staleness guard remains theoretical edge-case (documented, tested, self-healing)

Non-Blocking Suggestions

Two minor suggestions surfaced during cross-pollination (both non-blocking):

  1. Draft P/0 PR edge-caseTest-IsP0Pr keys off the p/0 label only (no draft filter), so a draft p/0-labelled PR flips the aggregate check to BLOCKED while its row shows "Draft PR; wait until ready." This is likely by-design (a p/0 label on a release-targeting PR is an explicit blocking signal regardless of draft state), but worth a one-line intent confirmation.

  2. Staleness guard theoretical gap — Carried over from round-1: the heuristic could drop a legitimate SR if it goes idle AND a higher SR ships out-of-order. Low risk (self-healing, idle requirement preserves hotfix case, pre-GA path never fires), well-documented, and tested. Only relevant if the team wants to fully close the theoretical gap.

Bottom line: PR is merge-ready. The implementation resolves the P/0 PR categorization cleanly without regressions.


# workflow matrix entirely (no no-op per-tracker job). Tag-existence
# stays the primary signal; the idle requirement preserves the
# out-of-order/hotfix case (a real reset branch has recent commits).
if (Test-IsStaleSrBranch -BranchPatch $branchPatch -HighestShippedPatch $highestShippedPatch -RecentActivityCount $recent) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 [Carried over from Round 1 — non-blocking] Test-IsStaleSrBranch (defined L286-316: RecentActivityCount -le 0 -and BranchPatch -lt HighestShippedPatch) is unchanged since round-1; only tests were added. The staleness heuristic (idle beyond the activity window AND patch below the highest-shipped watermark) could still drop a legitimate tag-absent SR if it goes idle AND a higher SR ships out-of-order (advancing the watermark). Verified low-risk: self-healing (any commit or tag publish re-surfaces it), the idle requirement preserves the security-hotfix / out-of-order case (a real hotfix branch has recent commits), and the pre-GA path (highest=0) never fires. Unit tests cover the core, hotfix, equal-watermark, and pre-GA cases (all pass) and the docstring documents the trade-off. Remains a non-blocking suggestion to add a stronger stale signal (explicit deny-list, branch-age threshold, or honoring existing tracker issues) only if the team wants to fully close the theoretical gap.

# P/0-labelled PRs targeting the release branch are blockers (parallel to P/0
# issues). They are itemized in the hoisted "🔴 High-priority items" section.
if ($p0Prs.Count -gt 0) {
$checks += New-Check -Area "P/0 release-branch PRs" -Status "BLOCKED" -Details "$($p0Prs.Count) open P/0-labelled PR(s) target ``$SurveyRef``. See 🔴 High-priority items at top." -NextAction "Land or de-prioritize each P/0 PR before shipping."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 [Non-blocking — intent confirmation] The "P/0 release-branch PRs" check goes BLOCKED whenever $p0Prs.Count > 0 (line 1143), and $p0Prs (line 781) / Test-IsP0Pr key off the p/0 label only — no isDraft filter. So a draft p/0-labelled PR also flips this check to BLOCKED, while its hoisted 🔥 P/0 PR row renders Get-PRAction's draft text "Draft PR; wait until ready for review." (line 675-676 → 1377). The aggregate next-action ("Land or de-prioritize each P/0 PR before shipping") and the per-row "wait until ready" therefore read in slightly different registers. On balance this is defensible by-design: a p/0 label deliberately placed on a PR targeting the survey branch is an explicit release-blocking signal regardless of draft state (the comment at line 1141-1142 frames it as parallel to p/0 issues, which always block), and surfacing "a release-critical change is still a draft" is arguably the useful behavior. Keeping as a one-line intent confirmation only: if a not-yet-ready draft should NOT gate shipment, filter drafts out of $p0Prs (Where-Object { -not $_.isDraft }) or downgrade the check to WATCH when every p/0 PR is a draft.

Comment-only change in Get-PreviewReadiness.ps1 documenting why the P/0
release-branch PR check is label-only and does not filter drafts: a `p/0`
label deliberately placed on a release-targeting PR is an explicit
"must ship" signal regardless of draft state. The per-row entry still
surfaces the draft state via Get-PRAction ("Draft PR; wait until ready
for review."). Records the intent confirmation requested in PR review.

No functional change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Multi-Model Review — Round 3

Verdict: ✅ LGTM (unanimous)

All three models independently reviewed round-3 changes and reached LGTM. This is a 6-line, comment-only follow-up to round-2.

What Changed in Round 3

The incremental commit cf90d44 adds a documentation comment resolving round-2's non-blocking suggestion about draft p/0 PR intent. The comment clarifies that the p/0 PR carve-out is deliberately label-only (no isDraft filter) — surfacing "a release-critical change is still a draft" is the intended behavior, parallel to how p/0 issues always block.

Verification

  • ✅ Comment-only scope confirmed via git diff (6 insertions, 1 file)
  • ✅ Comment claim is accurate: Get-PRAction does return "Draft PR; wait until ready", and the 🔥 P/0 row preserves it
  • ✅ Unit test suite passes (483 passed, 1 pre-existing unrelated failure in timezone-sensitive test)
  • ✅ No new issues introduced

Bottom Line

Clean documentation follow-up to an already twice-approved PR. Ready to merge.


@kubaflo

kubaflo commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Note

🤖 This review was automatically generated by a multi-model AI review system (Claude Opus 4.8, GPT-5.5, Gemini 3.1 Pro). Three models independently reviewed the code, then cross-pollinated their findings to produce this consolidated review.

@PureWeen
PureWeen merged commit dc08af3 into main Jun 18, 2026
15 of 18 checks passed
@PureWeen
PureWeen deleted the pureween/preview-p0-pr-blockers branch June 18, 2026 17:45
@github-actions github-actions Bot added this to the .NET 10 SR9 milestone Jun 18, 2026
PureWeen added a commit that referenced this pull request Jun 19, 2026
Addresses the optional test-coverage suggestion from the PR review: the
single-PR BLOCKED fixture never exercised the multi-PR comma-joined naming
("#a, #b") the release captain sees. Add a two-p/0-PR fixture asserting the
count and the joined #35970, #35971 format.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PureWeen added a commit that referenced this pull request Jun 19, 2026
…ive assertions, add deterministic fixture coverage (#36004)

<!-- 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!

### Summary

Follow-up to #35971. De-flakes a pre-existing time-bomb in the
release-readiness skill's **own** test suite
(`.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1`).
Skills-only change — no framework code touched.

### The time-bomb

The suite had four **live, wall-clock-dependent** assertions that
hardcoded `hasRecentActivity = $true` against real release branches
(SR8, SR9, the active-SR `foreach` loop, and preview6).

The detector (`Find-ReleaseReadinessTrackers.ps1`) computes
`hasRecentActivity` from `Get-RecentCommitCount`, which runs `git log
<branch> --since=7.days` and returns `(count > 0)`. So those assertions
are only true *while the real branch has had a commit in the last 7
days*. The moment a servicing branch goes quiet for 7 days — a
**normal** end-of-cycle state — the assertion flips red.

On **2026-06-18** this actually happened: SR8's last commit (the SR7→SR8
merge #35810) landed 2026-06-11, so `--since=7.days` returned 0 and the
two SR8 assertions went red. They self-heal on the next commit, but
nondeterministic red in a suite whose entire selling point is
determinism is a credibility bug.

### What changed

- **Removed the wall-clock-dependent live assertions.** The end-to-end
detector run now asserts only that `hasRecentActivity` is a real
`[bool]` the detector emitted — never a date-dependent value.
- **Added genuinely deterministic coverage of the recency-window math**
via a synthetic fixture: a throwaway temp git repo with commits at
controlled dates (`GIT_AUTHOR_DATE`/`GIT_COMMITTER_DATE` at now−6d /
now−8d / now−30d), then calls the **real** `Get-RecentCommitCount`
(dot-sourced) against it and asserts exact counts for the 7/10/60/1-day
windows plus the `origin/`-prefixed ref form. Zero network, zero
dependence on "today"; the temp repo is cleaned up in a `finally`.
- **Corrected the misleading comments** that equated "active SR" with
`hasRecentActivity = true`. An active SR can legitimately idle for >7
days; `hasRecentActivity` is a 7-day-window signal, not a synonym for
"active".

### Verification

Full suite is green: `pwsh -NoProfile -File
.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1` →
**Passed: 552, Failed: 0** (was 545/2 before, with the two SR8 reds).
The new fixture assertions also pass under `-SkipE2E` (offline),
confirming they're network-independent.

> Note: the `maui-pr` framework pipeline intentionally skips for
skills-only PRs — that's expected, not a failure.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PureWeen added a commit that referenced this pull request Jun 19, 2026
Addresses the optional test-coverage suggestion from the PR review: the
single-PR BLOCKED fixture never exercised the multi-PR comma-joined naming
("#a, #b") the release captain sees. Add a two-p/0-PR fixture asserting the
count and the joined #35970, #35971 format.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PureWeen added a commit that referenced this pull request Jun 19, 2026
Addresses the optional test-coverage suggestion from the PR review: the
single-PR BLOCKED fixture never exercised the multi-PR comma-joined naming
("#a, #b") the release captain sees. Add a two-p/0-PR fixture asserting the
count and the joined #35970, #35971 format.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen PureWeen changed the title Surface p/0 PRs as Preview blockers; drop stale below-watermark SR branches from the tracker matrix Release-readiness: cross-major preview leak fix, milestone blocker, E2E refresh, and hoisted candidate-PR section Jul 5, 2026
@PureWeen PureWeen changed the title Release-readiness: cross-major preview leak fix, milestone blocker, E2E refresh, and hoisted candidate-PR section Surface p/0 PRs as Preview blockers; drop stale below-watermark SR branches from the tracker matrix Jul 5, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants