Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 56 additions & 1 deletion .github/scripts/Invoke-RerunReviewTrigger.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ BeforeAll {
$script:ReviewTriggerWindowHours = 24
$script:MaxReviewTriggersPerWindow = 3

foreach ($functionName in @('Get-ReviewTriggerRateLimitStatus', 'ConvertTo-SafeLogValue', 'ConvertTo-TrimmedString', 'Test-GhApiPrNotFound', 'Get-MatchingCandidate', 'Normalize-PipelineRef', 'Get-PlatformFromLabels')) {
foreach ($functionName in @('Get-ReviewTriggerRateLimitStatus', 'ConvertTo-SafeLogValue', 'ConvertTo-TrimmedString', 'Test-GhApiPrNotFound', 'Get-MatchingCandidate', 'Normalize-PipelineRef', 'Get-PlatformFromLabels', 'Expand-RerunDecisionItems')) {
$function = $ast.Find({
$args[0] -is [System.Management.Automation.Language.FunctionDefinitionAst] -and
$args[0].Name -eq $functionName
Expand Down Expand Up @@ -85,6 +85,61 @@ Describe 'Test-GhApiPrNotFound' {
Test-GhApiPrNotFound 'gh: Internal Server Error (HTTP 500)' | Should -BeFalse
Test-GhApiPrNotFound '' | Should -BeFalse
}

It 'does not misclassify bare "Not Found"/"Gone" text without an HTTP 404/410 status' {
# Proxy/firewall/auth error bodies can contain these words without the
# resource actually being deleted. Treating them as 404 previously caused
# open PRs to be falsely skipped, silently cancelling every rerun.
Test-GhApiPrNotFound 'proxy error: Not Found' | Should -BeFalse
Test-GhApiPrNotFound 'The page you requested is Gone' | Should -BeFalse
}
}

Describe 'Expand-RerunDecisionItems' {
It 'expands a single item carrying a JSON-string decisions array' {
$json = '[{"pr_number":"1","decision":"trigger"},{"pr_number":"2","decision":"skip"}]'
$item = [pscustomobject]@{ type = 'trigger_rerun_review'; decisions = $json }
$result = Expand-RerunDecisionItems -Items @($item)
$result.Count | Should -Be 2
$result[0].pr_number | Should -Be '1'
$result[0].decision | Should -Be 'trigger'
$result[1].pr_number | Should -Be '2'
$result[1].decision | Should -Be 'skip'
}

It 'expands a decisions array that is already an object array' {
$item = [pscustomobject]@{
type = 'trigger_rerun_review'
decisions = @(
[pscustomobject]@{ pr_number = '7'; decision = 'trigger' }
)
}
$result = Expand-RerunDecisionItems -Items @($item)
$result.Count | Should -Be 1
$result[0].pr_number | Should -Be '7'
}

It 'aggregates decisions across multiple items' {
$a = [pscustomobject]@{ type = 'trigger_rerun_review'; decisions = '[{"pr_number":"1","decision":"trigger"}]' }
$b = [pscustomobject]@{ type = 'trigger_rerun_review'; decisions = '[{"pr_number":"2","decision":"skip"}]' }
$result = Expand-RerunDecisionItems -Items @($a, $b)
$result.Count | Should -Be 2
($result | ForEach-Object { $_.pr_number }) | Should -Be @('1', '2')
}

It 'passes through a legacy scalar item without a decisions field' {
$item = [pscustomobject]@{ type = 'trigger_rerun_review'; pr_number = '9'; decision = 'trigger' }
$result = Expand-RerunDecisionItems -Items @($item)
$result.Count | Should -Be 1
$result[0].pr_number | Should -Be '9'
}

It 'ignores empty or null decisions payloads' {
$empty = [pscustomobject]@{ type = 'trigger_rerun_review'; decisions = '' }
$nullItem = [pscustomobject]@{ type = 'trigger_rerun_review'; decisions = $null }
$result = Expand-RerunDecisionItems -Items @($empty, $nullItem)
$result.Count | Should -Be 0
}
}

Describe 'ConvertTo-TrimmedString' {
Expand Down
120 changes: 102 additions & 18 deletions .github/scripts/Invoke-RerunReviewTrigger.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,56 @@ $MaxReviewTriggersPerWindow = 3

. "$PSScriptRoot/shared/Update-AgentLabels.ps1"

function Expand-RerunDecisionItems {
param([object[]]$Items)

# A custom safe-output job is capped at one invocation per run, so the agent
# now batches every candidate's decision into a single item's `decisions`
# field (a JSON array, or array of objects). Expand that into one object per
# decision. Items that already carry scalar decision fields (legacy shape or
# a single decision) are passed through unchanged for back-compatibility.
$expanded = [System.Collections.Generic.List[object]]::new()
foreach ($item in $Items) {
$rawDecisions = $item.PSObject.Properties['decisions']
if (-not $rawDecisions -or $null -eq $rawDecisions.Value) {
if ($item.PSObject.Properties['pr_number']) {
$expanded.Add($item)
}
continue
}

$value = $rawDecisions.Value
$parsed = $null
if ($value -is [string]) {
if ([string]::IsNullOrWhiteSpace($value)) { continue }
try {
$parsed = $value | ConvertFrom-Json
} catch {
Write-Host "::warning::Skipping unparseable decisions payload: $($_.Exception.Message)"
continue
}
} else {
$parsed = $value
}

foreach ($decision in @($parsed)) {
if ($null -ne $decision) {
$expanded.Add($decision)
}
}
}

return $expanded.ToArray()
}

function Get-AgentItems {
if (-not $env:GH_AW_AGENT_OUTPUT -or -not (Test-Path $env:GH_AW_AGENT_OUTPUT)) {
throw "GH_AW_AGENT_OUTPUT is missing or does not exist."
}

$payload = Get-Content -Raw -LiteralPath $env:GH_AW_AGENT_OUTPUT | ConvertFrom-Json
return @($payload.items | Where-Object { $_.type -eq 'trigger_rerun_review' })
$triggerItems = @($payload.items | Where-Object { $_.type -eq 'trigger_rerun_review' })
return Expand-RerunDecisionItems -Items $triggerItems
}

function Get-CandidateItems {
Expand Down Expand Up @@ -75,7 +118,13 @@ function Test-GhApiPrNotFound {
return $false
}

return $Output -match '(?i)\bHTTP\s+(404|410)\b' -or $Output -match '(?i)\b(Not Found|Gone)\b'
# Only treat an explicit HTTP 404/410 status as "deleted". gh emits a
# structured error such as "gh: Not Found (HTTP 404)" for a genuinely
# missing resource. The bare words "Not Found"/"Gone" are intentionally NOT
# matched on their own: auth failures, proxy/firewall errors, and rate-limit
# bodies can contain that text and previously caused open PRs to be falsely
# classified as deleted, silently skipping every rerun dispatch.
return $Output -match '(?i)\bHTTP\s+(404|410)\b'
}

function ConvertTo-TrimmedString {
Expand All @@ -88,6 +137,30 @@ function ConvertTo-TrimmedString {
return ([string]$Value).Trim()
}

function Get-PullRequestApiResult {
param(
[Parameter(Mandatory = $true)][string]$Owner,
[Parameter(Mandatory = $true)][string]$Repo,
[Parameter(Mandatory = $true)][int]$PRNumber
)

$stdErrFile = New-TemporaryFile
try {
$output = @(& gh api "repos/$Owner/$Repo/pulls/$PRNumber" 2> $stdErrFile)
$exitCode = $LASTEXITCODE
$json = ConvertTo-TrimmedString ($output | Out-String)
$stdErr = ConvertTo-TrimmedString (Get-Content -Raw -LiteralPath $stdErrFile -ErrorAction SilentlyContinue)
} finally {
Remove-Item -LiteralPath $stdErrFile -Force -ErrorAction SilentlyContinue
}

return [pscustomobject]@{
ExitCode = $exitCode
Json = $json
StdErr = $stdErr
}
}

function Add-CommentReaction {
param(
[Parameter(Mandatory = $true)][Int64]$CommentId,
Expand Down Expand Up @@ -332,25 +405,36 @@ foreach ($item in $items) {
}

Write-Host "Processing PR #$prNumber decision=$decision reason=$(ConvertTo-SafeLogValue $reason)"
$prStdErrFile = New-TemporaryFile
try {
$prOutput = @(& gh api "repos/$Owner/$Repo/pulls/$prNumber" 2> $prStdErrFile)
$prExitCode = $LASTEXITCODE
$prJson = ConvertTo-TrimmedString ($prOutput | Out-String)
$prStdErr = ConvertTo-TrimmedString (Get-Content -Raw -LiteralPath $prStdErrFile -ErrorAction SilentlyContinue)
} finally {
Remove-Item -LiteralPath $prStdErrFile -Force -ErrorAction SilentlyContinue
}
if ($prExitCode -ne 0) {
$prError = if ([string]::IsNullOrWhiteSpace($prStdErr)) { $prJson } else { $prStdErr }
$prFetch = Get-PullRequestApiResult -Owner $Owner -Repo $Repo -PRNumber $prNumber
if ($prFetch.ExitCode -ne 0) {
$prError = if ([string]::IsNullOrWhiteSpace($prFetch.StdErr)) { $prFetch.Json } else { $prFetch.StdErr }
# Always surface the raw gh error so misclassified failures are
# debuggable instead of being swallowed by the not-found guard.
Write-Host " ⚠️ gh api for PR #$prNumber failed (exit $($prFetch.ExitCode)): $(ConvertTo-SafeLogValue $prError)"
if (Test-GhApiPrNotFound -Output $prError) {
$global:LASTEXITCODE = 0
Write-Host " ⏭️ PR #$prNumber no longer exists; skipping stale decision"
continue
# Confirm with a second authenticated probe before treating the
# PR as deleted. A single transient/auth/proxy 404 must not
# silently cancel a real rerun dispatch.
$confirm = Get-PullRequestApiResult -Owner $Owner -Repo $Repo -PRNumber $prNumber
$confirmError = if ([string]::IsNullOrWhiteSpace($confirm.StdErr)) { $confirm.Json } else { $confirm.StdErr }
if ($confirm.ExitCode -ne 0 -and (Test-GhApiPrNotFound -Output $confirmError)) {
$global:LASTEXITCODE = 0
Write-Host " ⏭️ PR #$prNumber no longer exists (confirmed HTTP 404/410); skipping stale decision"
continue
}
if ($confirm.ExitCode -eq 0) {
# First 404 was transient; recheck recovered. Reuse the successful result.
$global:LASTEXITCODE = 0
$prFetch = $confirm
Write-Host " ✓ PR #$prNumber recheck succeeded (transient 404 recovered)"
} else {
throw "PR #$prNumber returned a not-found error that did not reproduce on re-check; refusing to silently skip. First: $(ConvertTo-SafeLogValue $prError); Recheck: $(ConvertTo-SafeLogValue $confirmError)"
}
} else {
throw "Failed to load PR #$prNumber via gh api: $(ConvertTo-SafeLogValue $prError)"
}

throw "Failed to load PR #$prNumber via gh api: $(ConvertTo-SafeLogValue $prError)"
}
$prJson = $prFetch.Json
if ([string]::IsNullOrWhiteSpace($prJson)) {
throw "Failed to load PR #$prNumber via gh api: empty response."
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,19 @@ function Format-MarkdownCell {
# `List<T>` that GitHub markdown would otherwise swallow as an HTML tag. The
# engine's own markers are emitted via AppendLine, not through this formatter,
# so escaping cells never disturbs them.
return (($Value -replace "\|", "\|") -replace "<", "&lt;" -replace ">", "&gt;").Trim()
# Collapse embedded newlines first: a malformed upstream title can contain a
# literal CR/LF (observed: ci-scan issue #35957), which would otherwise split
# the markdown table row across physical lines and break the rendered table.
# Escape each pipe AND double only the backslash run immediately preceding it:
# a title may legally contain a literal `\|`, and escaping only the pipe would
# yield `\\|` — which GFM renders as a literal `\` plus an ACTIVE column delimiter
# (table breakout). Doubling the pipe-adjacent run makes `\|` -> `\\\|`, a literal
# `\|`. Scoping the doubling to `(\\*)\|` (rather than every backslash) preserves a
# title's other backslash escapes (e.g. `\[link\](url)` is not de-escaped into an
# active link). No-pipe-adjacent-backslash titles are unaffected (`a | b` -> `a \| b`).
$v = $Value -replace "[\r\n]+", " "
$v = [regex]::Replace($v, '(\\*)\|', { param($m) ($m.Groups[1].Value * 2) + '\|' })
return ($v -replace "<", "&lt;" -replace ">", "&gt;").Trim()
}

function Format-GitHubHandle {
Expand Down
Loading
Loading