diff --git a/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 b/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 index adfb6677cfc5..abe8610bb359 100644 --- a/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 @@ -194,14 +194,19 @@ function Invoke-Git([string]$Cmd) { return $out } -function Invoke-Gh([string[]]$GhArgs) { +function Invoke-Gh([string[]]$GhArgs, [switch]$Quiet) { + # -Quiet suppresses the non-zero-exit warning for callers that handle a + # $null return themselves and don't want a raw `gh ... exited` line leaking + # into $Script:Warnings (which is rendered into the tracker issue body). $errFile = [System.IO.Path]::GetTempFileName() try { $out = & gh @GhArgs 2>$errFile $exitCode = $LASTEXITCODE if ($exitCode -ne 0) { - $err = Get-Content $errFile -Raw -ErrorAction SilentlyContinue - Write-Warn "gh $($GhArgs -join ' ') exited $exitCode : $err" + if (-not $Quiet) { + $err = Get-Content $errFile -Raw -ErrorAction SilentlyContinue + Write-Warn "gh $($GhArgs -join ' ') exited $exitCode : $err" + } return $null } return $out @@ -1159,12 +1164,13 @@ function Get-CandidatePrChecks { # Scan open PRs targeting main (the Candidate PR is opened on main, not # on the SR branch, since the SR branch may not exist yet in candidate # mode). Cheap: one gh call returning up to 100 open PRs on main. - # Include authorAssociation in the json projection so we can gate on - # OWNER/MEMBER/COLLABORATOR — without this, ANY open PR with - # "Candidate" in its title would spoof the cut PR. + # `gh pr list --json` does NOT expose authorAssociation (it's not a valid + # list projection field). The maintainer spoof-gate below fetches + # author_association per title-matched candidate via the REST API instead. + # Keep this projection limited to valid `gh pr list` fields. $raw = Invoke-Gh @('pr', 'list', '--repo', $Ctx.repo, '--state', 'open', '--base', $Ctx.mainBranch, '--limit', '100', - '--json', 'number,title,author,authorAssociation,updatedAt,url') + '--json', 'number,title,author,updatedAt,url') if ($null -eq $raw) { # gh failed — distinguish from "no Candidate PR found" so the # verdict doesn't silently READY on tool failure. @@ -1180,22 +1186,52 @@ function Get-CandidatePrChecks { $titleMatches = @($mainPrs | Where-Object { $_.title -match '(?i)\bcandidate\b' }) # Author gating: only PRs from a maintainer count. Outside contributors - # never open SR-cut PRs by convention. GraphQL returns authorAssociation - # as the enum 'OWNER' | 'MEMBER' | 'COLLABORATOR' | 'CONTRIBUTOR' | etc. + # never open SR-cut PRs by convention. `gh pr list` can't return the + # association, so fetch it per title-matched candidate from the REST API + # (cheap: titleMatches is almost always 0-1, usually 0 in candidate mode). + # The REST field is 'author_association' (snake_case) — enum + # OWNER|MEMBER|COLLABORATOR|CONTRIBUTOR|... Fail closed: an unreadable + # association excludes the PR, so a missing signal can't let a + # 'Candidate'-titled PR slip through the spoof gate. Distinguish a + # *confirmed* non-maintainer (a real spoofer) from an *unverifiable* one + # (transient gh/REST failure) so a legitimate maintainer Candidate PR isn't + # mislabeled as a spoofer during an actual cut. Use -Quiet so a transient + # lookup miss doesn't embed a raw `gh ... exited` warning in the tracker + # body — the structured WATCH note below carries that signal instead. $maintainerAssociations = @('OWNER', 'MEMBER', 'COLLABORATOR') - $candidates = @($titleMatches | Where-Object { - $assoc = if ($_.PSObject.Properties['authorAssociation']) { $_.authorAssociation } else { $null } - $assoc -and ($maintainerAssociations -contains $assoc) - }) - $rejectedBySpoofGate = $titleMatches.Count - $candidates.Count + $candidates = @() + $spoofers = 0 + $unverifiable = 0 + foreach ($pr in $titleMatches) { + $assocRaw = Invoke-Gh @('api', "repos/$($Ctx.repo)/pulls/$($pr.number)", + '--jq', '.author_association') -Quiet + $assoc = if ($assocRaw) { "$assocRaw".Trim() } else { $null } + if (-not $assoc) { + $unverifiable++ + } elseif ($maintainerAssociations -contains $assoc) { + $candidates += $pr + } else { + $spoofers++ + } + } if ($candidates.Count -eq 0) { - $rejectNote = if ($rejectedBySpoofGate -gt 0) { - " ($rejectedBySpoofGate non-maintainer PR(s) titled 'Candidate' were excluded as not real cut PRs)" - } else { '' } + $excludeNotes = @() + if ($spoofers -gt 0) { + $excludeNotes += "$spoofers non-maintainer PR(s) titled 'Candidate' were excluded as not real cut PRs" + } + if ($unverifiable -gt 0) { + $excludeNotes += "$unverifiable 'Candidate'-titled PR(s) could not have their author association verified (``gh`` REST lookup failed) and were excluded fail-closed — rerun to re-check" + } + $rejectNote = if ($excludeNotes.Count -gt 0) { " ($($excludeNotes -join '; '))" } else { '' } + $nextAction = if ($unverifiable -gt 0) { + "Verify ``gh auth status`` and rerun to re-check author association. When ready to cut, open a Candidate PR against ``$($Ctx.mainBranch)`` selecting the target main commit for the next SR." + } else { + "When ready to cut, open a Candidate PR against ``$($Ctx.mainBranch)`` selecting the target main commit for the next SR." + } return ,@(New-ReadinessCheck -Area $area -Status 'WATCH' ` -Details "No open PR matching ``*Candidate*`` from a maintainer (OWNER/MEMBER/COLLABORATOR) found on ``$($Ctx.mainBranch)``$rejectNote. The Candidate PR is the mechanism that promotes a specific main commit as the SR cut point." ` - -NextAction "When ready to cut, open a Candidate PR against ``$($Ctx.mainBranch)`` selecting the target main commit for the next SR.") + -NextAction $nextAction) } # Build a compact detail string listing all open candidate PRs (almost @@ -1205,9 +1241,24 @@ function Get-CandidatePrChecks { "[#$($_.number)]($repoUrl/pull/$($_.number)) — $titleShort" }) -join '; ' + # Even on the accepted path, surface any title-matches that were excluded + # (a confirmed spoofer or an unverifiable lookup) so a transient REST blip on + # a *second* Candidate-titled PR isn't silently dropped from the captain's view. + $excludedSuffix = '' + if ($spoofers -gt 0 -or $unverifiable -gt 0) { + $parts = @() + if ($spoofers -gt 0) { $parts += "$spoofers non-maintainer" } + if ($unverifiable -gt 0) { $parts += "$unverifiable unverifiable (``gh`` REST lookup failed — rerun to re-check)" } + $excludedSuffix = " Also excluded $($parts -join ' and ') ``*Candidate*``-titled PR(s)." + } + $acceptNextAction = if ($unverifiable -gt 0) { + "Review and merge the Candidate PR when ready; the SR cut follows from its merge commit. Also verify ``gh auth status`` and rerun to re-check the unverifiable Candidate-titled PR(s)." + } else { + "Review and merge the Candidate PR when ready; the SR cut follows from its merge commit." + } return ,@(New-ReadinessCheck -Area $area -Status 'WATCH' ` - -Details "$($candidates.Count) open Candidate PR(s) on ``$($Ctx.mainBranch)``: $links. This PR promotes a specific main commit as the SR cut point — it must be merged (and the SR branch cut from it) before the SR cycle starts." ` - -NextAction "Review and merge the Candidate PR when ready; the SR cut follows from its merge commit.") + -Details "$($candidates.Count) open Candidate PR(s) on ``$($Ctx.mainBranch)``: $links. This PR promotes a specific main commit as the SR cut point — it must be merged (and the SR branch cut from it) before the SR cycle starts.$excludedSuffix" ` + -NextAction $acceptNextAction) } # endregion @@ -1713,6 +1764,18 @@ function Get-IssueTimelinePrs { # `pull_request` member only exists on issues that are actually PRs if (-not $iss.PSObject.Properties['pull_request']) { continue } if (-not $iss.pull_request) { continue } + # Cross-referenced PRs can live in OTHER repositories (forks, or wholly + # unrelated projects whose own PRs happened to reference this issue). + # Only same-repo PRs are real fix candidates. A foreign PR number looked + # up against $Repo either 404s (low numbers below the repo's PR range — + # surfacing a `gh pr view` warning in the tracker) or, worse, silently + # matches an unrelated $Repo PR that happens to share the number. Filter + # to $Repo. The timeline API populates `repository.full_name` for both + # same-repo and cross-repo references, so this is reliable. + if (-not $iss.PSObject.Properties['repository']) { continue } + $issRepo = $iss.repository + if (-not $issRepo -or -not $issRepo.PSObject.Properties['full_name']) { continue } + if ($issRepo.full_name -ne $Repo) { continue } if (-not $iss.PSObject.Properties['number']) { continue } $prs += [int]$iss.number } diff --git a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 index 66b29db34927..75774a18d0a9 100644 --- a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 @@ -864,6 +864,206 @@ try { Remove-Item -Path Env:GET_RELEASE_READINESS_TEST_MODE -ErrorAction SilentlyContinue } +# ───── gh-stubbed regression tests (cross-repo filter + author gate) ───── +# These exercise functions that call `Invoke-Gh`. We shadow Invoke-Gh with a +# per-test dispatcher ($script:GhStub) so the assertions are deterministic and +# offline, then restore the real function so the E2E section is unaffected. +$script:GhStub = $null +$script:OrigInvokeGh = ${function:Invoke-Gh} +function Invoke-Gh { param([string[]]$GhArgs, [switch]$Quiet) & $script:GhStub $GhArgs } +try { + # ── Get-IssueTimelinePrs: only same-repo cross-references are fix candidates ── + # Regression: timeline `cross-referenced` events can point at PRs in OTHER + # repos (forks like praveenkumarkarunanithi/maui#24, unrelated projects like + # zhollis21/AniSprinkles#102). Those numbers, looked up against dotnet/maui, + # either 404 (low numbers → warning embedded in the tracker issue) or silently + # match an unrelated same-numbered PR. The repo filter must drop them. + Write-Host "`n[Unit] Get-IssueTimelinePrs (cross-repo cross-reference filter)" -ForegroundColor Cyan + $script:GhStub = { + param([string[]]$GhArgs) + @' +[ + { "event": "cross-referenced", "source": { "type": "issue", "issue": { + "number": 35625, "pull_request": {"url":"x"}, "repository": { "full_name": "dotnet/maui" } } } }, + { "event": "cross-referenced", "source": { "type": "issue", "issue": { + "number": 102, "pull_request": {"url":"x"}, "repository": { "full_name": "zhollis21/AniSprinkles" } } } }, + { "event": "cross-referenced", "source": { "type": "issue", "issue": { + "number": 24, "pull_request": {"url":"x"}, "repository": { "full_name": "praveenkumarkarunanithi/maui" } } } }, + { "event": "cross-referenced", "source": { "type": "issue", "issue": { + "number": 35962, "pull_request": {"url":"x"}, "repository": { "full_name": "dotnet/maui" } } } }, + { "event": "cross-referenced", "source": { "type": "issue", "issue": { + "number": 999, "repository": { "full_name": "dotnet/maui" } } } }, + { "event": "labeled" } +] +'@ + } + $timelinePrs = Get-IssueTimelinePrs -Repo 'dotnet/maui' -IssueNumber 12345 + Assert-Eq -Label "timeline keeps only same-repo PRs; drops foreign #24/#102 and non-PR #999" ` + -Expected '35625,35962' -Actual (($timelinePrs | Sort-Object) -join ',') + + # A timeline with ONLY foreign cross-refs must yield zero candidates (no + # `gh pr view ` against dotnet/maui → no 404 warning in the tracker). + $script:GhStub = { + param([string[]]$GhArgs) + @' +[ + { "event": "cross-referenced", "source": { "type": "issue", "issue": { + "number": 24, "pull_request": {"url":"x"}, "repository": { "full_name": "praveenkumarkarunanithi/maui" } } } }, + { "event": "cross-referenced", "source": { "type": "issue", "issue": { + "number": 877, "pull_request": {"url":"x"}, "repository": { "full_name": "DIPSAS/DIPS.Mobile.UI" } } } } +] +'@ + } + $foreignOnly = @(Get-IssueTimelinePrs -Repo 'dotnet/maui' -IssueNumber 12345) + Assert-Eq -Label "timeline with only foreign cross-refs yields 0 candidates" ` + -Expected 0 -Actual $foreignOnly.Count + + # ── Get-CandidatePrChecks: maintainer author-gate via REST author_association ── + # Regression: `gh pr list --json` does not support authorAssociation, so the + # spoof-gate now fetches author_association per title-matched candidate from + # the REST API. Verify (a) a MEMBER-authored "Candidate" PR is accepted and a + # CONTRIBUTOR-authored one is excluded, and (b) when ALL title matches are + # non-maintainers the gate reports them as excluded spoofers. + Write-Host "`n[Unit] Get-CandidatePrChecks (REST author-association spoof gate)" -ForegroundColor Cyan + $candCtx = @{ mode = 'candidate'; repo = 'dotnet/maui'; mainBranch = 'main'; priorSrBranch = 'release/10.0.1xx-sr8' } + + # (a) member candidate present alongside a contributor spoof + a non-match. + $script:GhStub = { + param([string[]]$GhArgs) + if ($GhArgs[0] -eq 'pr' -and $GhArgs[1] -eq 'list') { + return @' +[ + {"number":777,"title":"June 8th, Candidate","author":{"login":"rmarinho"},"updatedAt":"2026-06-18T00:00:00Z","url":"u"}, + {"number":888,"title":"Candidate build for testing","author":{"login":"rando"},"updatedAt":"2026-06-18T00:00:00Z","url":"u"}, + {"number":999,"title":"Fix button layout","author":{"login":"x"},"updatedAt":"2026-06-18T00:00:00Z","url":"u"} +] +'@ + } + if ($GhArgs[0] -eq 'api' -and ($GhArgs -contains '.author_association')) { + if ($GhArgs[1] -match '/pulls/777$') { return 'MEMBER' } + if ($GhArgs[1] -match '/pulls/888$') { return 'CONTRIBUTOR' } + return 'NONE' + } + return $null + } + $candChecks = @(Get-CandidatePrChecks -Ctx $candCtx) + Assert-Eq -Label "candidate gate returns exactly one check" -Expected 1 -Actual $candChecks.Count + Assert-Eq -Label "member-authored Candidate PR accepted (WATCH)" -Expected 'WATCH' -Actual $candChecks[0].Status + Assert-Eq -Label "accepted check names the member PR #777" -Expected $true ` + -Actual ([bool]($candChecks[0].Details -match '#777')) + Assert-Eq -Label "contributor spoof #888 excluded from accepted check" -Expected $true ` + -Actual ([bool]($candChecks[0].Details -notmatch '#888')) + + # (b) only a contributor-authored "Candidate" PR exists → gate rejects it and + # reports the exclusion count (no candidate accepted). + $script:GhStub = { + param([string[]]$GhArgs) + if ($GhArgs[0] -eq 'pr' -and $GhArgs[1] -eq 'list') { + return @' +[ {"number":888,"title":"Candidate build for testing","author":{"login":"rando"},"updatedAt":"2026-06-18T00:00:00Z","url":"u"} ] +'@ + } + if ($GhArgs[0] -eq 'api' -and ($GhArgs -contains '.author_association')) { + return 'CONTRIBUTOR' + } + return $null + } + $spoofChecks = @(Get-CandidatePrChecks -Ctx $candCtx) + Assert-Eq -Label "spoof-only gate still returns one (WATCH) check" -Expected 'WATCH' -Actual $spoofChecks[0].Status + Assert-Eq -Label "spoof-only gate reports the excluded non-maintainer PR" -Expected $true ` + -Actual ([bool]($spoofChecks[0].Details -match 'non-maintainer')) + Assert-Eq -Label "confirmed spoofer is NOT reported as could-not-verify" -Expected $true ` + -Actual ([bool]($spoofChecks[0].Details -notmatch 'could not have their author association verified')) + + # (c) a maintainer-titled Candidate PR whose author-association REST lookup + # fails transiently (Invoke-Gh returns $null on non-zero gh exit). It must be + # excluded fail-closed, but reported as UNVERIFIABLE — NOT mislabeled as a + # confirmed non-maintainer spoofer. A transient blip during a real cut must + # not tell the release captain their own legitimate PR isn't from a + # maintainer. (The dedicated Invoke-Gh -Quiet test further below proves the + # transient lookup failure stays out of the tracker body; this case shadows + # Invoke-Gh and so asserts the *classification*, not the suppression.) + $script:GhStub = { + param([string[]]$GhArgs) + if ($GhArgs[0] -eq 'pr' -and $GhArgs[1] -eq 'list') { + return @' +[ {"number":777,"title":"June 8th, Candidate","author":{"login":"rmarinho"},"updatedAt":"2026-06-18T00:00:00Z","url":"u"} ] +'@ + } + # author_association lookup fails → mirror Invoke-Gh's $null-on-failure. + return $null + } + $unverChecks = @(Get-CandidatePrChecks -Ctx $candCtx) + Assert-Eq -Label "unverifiable author-assoc gate still returns one (WATCH) check" -Expected 'WATCH' -Actual $unverChecks[0].Status + Assert-Eq -Label "unverifiable Candidate PR reported as could-not-verify" -Expected $true ` + -Actual ([bool]($unverChecks[0].Details -match 'could not have their author association verified')) + Assert-Eq -Label "unverifiable Candidate PR NOT mislabeled as non-maintainer spoofer" -Expected $true ` + -Actual ([bool]($unverChecks[0].Details -notmatch 'non-maintainer')) + Assert-Eq -Label "unverifiable gate NextAction tells captain to rerun" -Expected $true ` + -Actual ([bool]($unverChecks[0].NextAction -match 'rerun')) + + # (d) a maintainer Candidate PR (#777, MEMBER) co-exists with a SECOND + # title-matched PR (#888) whose author-association lookup fails transiently. + # The valid candidate is accepted, but the accepted check must still SURFACE + # the co-existing unverifiable sibling instead of silently dropping it. + $script:GhStub = { + param([string[]]$GhArgs) + if ($GhArgs[0] -eq 'pr' -and $GhArgs[1] -eq 'list') { + return @' +[ + {"number":777,"title":"June 8th, Candidate","author":{"login":"rmarinho"},"updatedAt":"2026-06-18T00:00:00Z","url":"u"}, + {"number":888,"title":"Candidate build for testing","author":{"login":"rando"},"updatedAt":"2026-06-18T00:00:00Z","url":"u"} +] +'@ + } + if ($GhArgs[0] -eq 'api' -and ($GhArgs -contains '.author_association')) { + if ($GhArgs[1] -match '/pulls/777$') { return 'MEMBER' } + return $null # #888 lookup fails → unverifiable + } + return $null + } + $mixedChecks = @(Get-CandidatePrChecks -Ctx $candCtx) + Assert-Eq -Label "mixed accept+unverifiable still returns one (WATCH) check" -Expected 'WATCH' -Actual $mixedChecks[0].Status + Assert-Eq -Label "mixed: accepted check names the member PR #777" -Expected $true ` + -Actual ([bool]($mixedChecks[0].Details -match '#777')) + Assert-Eq -Label "mixed: accepted check surfaces the co-existing unverifiable sibling" -Expected $true ` + -Actual ([bool]($mixedChecks[0].Details -match 'unverifiable')) + Assert-Eq -Label "mixed: NextAction tells captain to rerun for the unverifiable sibling" -Expected $true ` + -Actual ([bool]($mixedChecks[0].NextAction -match 'rerun')) +} finally { + ${function:Invoke-Gh} = $script:OrigInvokeGh + $script:GhStub = $null +} + +# ───── Invoke-Gh -Quiet (warning-suppression contract) ───── +# Get-CandidatePrChecks fetches author_association with `Invoke-Gh ... -Quiet` +# specifically so a transient REST failure does NOT leak a raw `gh ... exited` +# line into $Script:Warnings (which is rendered into the tracker issue body). +# The classification tests above shadow Invoke-Gh, so they cannot observe this. +# Here we exercise the REAL Invoke-Gh against a simulated failing `gh` (a stub +# function that just sets a non-zero $LASTEXITCODE — no Write-Error, which would +# throw under $ErrorActionPreference='Stop') to prove the contract directly. +Write-Host "`n[Unit] Invoke-Gh -Quiet (warning suppression)" -ForegroundColor Cyan +$loudResult = $null; $loudWarnings = -1 +$quietResult = 'sentinel'; $quietWarnings = -1 +function gh { $global:LASTEXITCODE = 7 } +try { + $Script:Warnings.Clear() + $loudResult = Invoke-Gh @('api', 'repos/dotnet/maui/pulls/1') + $loudWarnings = $Script:Warnings.Count + + $Script:Warnings.Clear() + $quietResult = Invoke-Gh @('api', 'repos/dotnet/maui/pulls/1') -Quiet + $quietWarnings = $Script:Warnings.Count +} finally { + Remove-Item Function:gh -ErrorAction SilentlyContinue + $Script:Warnings.Clear() +} +Assert-Eq -Label 'Invoke-Gh returns $null on non-zero gh exit (no -Quiet)' -Expected $true -Actual ($null -eq $loudResult) +Assert-Eq -Label 'Invoke-Gh without -Quiet records a warning on failure' -Expected $true -Actual ($loudWarnings -ge 1) +Assert-Eq -Label 'Invoke-Gh -Quiet returns $null on non-zero gh exit' -Expected $true -Actual ($null -eq $quietResult) +Assert-Eq -Label 'Invoke-Gh -Quiet records NO warning on failure' -Expected 0 -Actual $quietWarnings + # ───── Get-RevertedPrFromSubject (revert false-green guard) ───── Write-Host "`n[Unit] Get-RevertedPrFromSubject (revert classification)" -ForegroundColor Cyan