Skip to content

Restrict /review rerun eligibility to author activity #35874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PureWeen merged 8 commits into from
Jun 15, 2026
Merged

Restrict /review rerun eligibility to author activity #35874

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
129b09d
Restrict review rerun eligibility to author activity
Copilot Jun 11, 2026
7aa823e
Fix rerun scanner candidate prompt handoff
Copilot Jun 11, 2026
ace4373
Checkout scripts before rerun scanner pre-activation
Copilot Jun 11, 2026
6579016
Handle stale rerun scanner PR decisions cleanly
Copilot Jun 11, 2026
3939e24
Address rerun scanner review feedback
Copilot Jun 11, 2026
f264273
Address rerun scanner safe-output review feedback
Copilot Jun 11, 2026
0e519c1
Fix rerun scanner PR fetch stderr handling
Copilot Jun 11, 2026
df5f314
Scope rerun candidate-query failure handling to gh pr list
Copilot Jun 11, 2026
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
10 changes: 9 additions & 1 deletion .github/scripts/Invoke-RerunReviewTrigger.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,13 @@ foreach ($item in $items) {
}

Write-Host "Processing PR #$prNumber decision=$decision reason=$(ConvertTo-SafeLogValue $reason)"
$pr = gh api "repos/$Owner/$Repo/pulls/$prNumber" | ConvertFrom-Json
$prJson = & gh api "repos/$Owner/$Repo/pulls/$prNumber" 2>$null
if ($LASTEXITCODE -ne 0 -or [string]::IsNullOrWhiteSpace($prJson)) {

@PureWeen PureWeen Jun 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

⚠️ Error handling — failure modes silently swallowed (2/3 reviewers)

The new defensive PR fetch combined with the exit 0 added at line 412 is too broad. gh api ... 2>$null followed by $global:LASTEXITCODE = 0 and continue treats every gh failure (auth expired, secondary rate limit, transient 5xx, malformed JSON) identically to "PR was deleted / 404". The step then keeps iterating, swallows the error, and exit 0 reports the whole safe-output job green.

Under a real systemic failure (PAT/App token rotated mid-run, GitHub outage, secondary rate limit while a large queue is draining), every queued rerun decision is silently skipped without surfacing the underlying cause — operators see a clean green run while no actual reruns fire. That's exactly the failure shape this script is supposed to convert into a visible error.

Suggested fix: distinguish stale-PR from transient/credential failures. Capture stderr (don't drop it), and only short-circuit when the response indicates a known stale status (e.g., 404/410, gone, not found). For 401/403/5xx/secondary-rate-limit, surface the error and let the step fail visibly. Alternatively, accumulate a per-iteration error count and exit 1 at the bottom if any candidate failed for non-stale reasons.

$global:LASTEXITCODE = 0
Write-Host " ⏭️ PR #$prNumber could not be loaded; skipping stale decision"
continue
}
$pr = $prJson | ConvertFrom-Json
if ($pr.state -ne 'open') {
Write-Host " ⏭️ PR #$prNumber is not open ($($pr.state)); skipping"
continue
Expand Down Expand Up @@ -402,3 +408,5 @@ foreach ($item in $items) {
continue
}
}

exit 0
6 changes: 4 additions & 2 deletions .github/scripts/Query-RerunReadyPRs.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ $searchResult = gh pr list `
--state open `
--label $ReadyForRerunLabel `
--limit $MaxPRs `
--json number,title,url,headRefOid,isDraft,labels | ConvertFrom-Json
--json number,title,url,headRefOid,isDraft,labels,author | ConvertFrom-Json

$candidates = @()
foreach ($pr in @($searchResult)) {
Expand All @@ -101,14 +101,16 @@ foreach ($pr in @($searchResult)) {
$latestRerun = Get-LatestRerunComment -Comments $activity
$reviewOptionAuthors = @(Get-ReviewOptionAuthorLogins -Comments $activity)
$reviewOptions = Get-LatestReviewCommandOptions -Comments $activity -AllowedAuthorLogins $reviewOptionAuthors
$contextMarkdown = New-RerunContextMarkdown -Comments $activity -Commits $commits -CurrentHeadSha $pr.headRefOid -CurrentLabels $labels
$authorLogin = if ($pr.author -and $pr.author.login) { [string]$pr.author.login } else { '' }

@PureWeen PureWeen Jun 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

⚠️ Logic — bot-authored PR login format mismatch (2/3 reviewers)

Here $pr.author.login from gh pr list --json author resolves to an app-style login for bot authors (verified live against this repo): app/dependabot, app/dotnet-maestro, app/github-actions, app/copilot-swe-agent. But REST gh api .../pulls/N returns user.login = "dependabot[bot]", and comment user.login also uses the [bot] suffix form. Test-CommentIsEvidence does OrdinalIgnoreCase equality on these strings, so a bot author's own comments never match this $authorLogin.

Practical impact is bounded (one reviewer overstated this as ❌; verified): the deterministic eligibility gate invoked from /review rerun lives in Resolve-RerunEligibility.ps1 and gets its author login from REST ($pr.user.login at lines 620/645), which IS the canonical [bot] form — so this code path matches comment user.login correctly. Additionally, Test-CommentIsEvidence already filters Comment.user.type -eq 'Bot' regardless of login, so bot-on-own-PR comments never count anyway. What breaks is the scanner's advisory context markdown: for bot-authored PRs (Dependabot, Maestro, copilot-swe-agent) it will display PR author: app/... (confusing) and New non-command author comments: 0 even if the bot has actually commented post-summary — biasing the agent's trigger/skip reasoning.

Suggested fix: either resolve the author via REST inside the loop (one extra API call per candidate; cheap given MaxPRs is small), or normalise app/X → X[bot] when is_bot is true. Adding ,isBot to the --json selector makes the normalisation deterministic without a second round-trip.

$contextMarkdown = New-RerunContextMarkdown -Comments $activity -Commits $commits -CurrentHeadSha $pr.headRefOid -PRAuthorLogin $authorLogin -CurrentLabels $labels
$platform = if ($reviewOptions.Platform) { $reviewOptions.Platform } else { Get-PlatformFromLabels -Labels $labels }
$pipelineRef = if ($reviewOptions.PipelineRef) { $reviewOptions.PipelineRef } else { 'main' }

$candidates += [pscustomobject]@{
prNumber = $number
title = [string]$pr.title
url = [string]$pr.url
authorLogin = $authorLogin
isDraft = [bool]$pr.isDraft
headSha = [string]$pr.headRefOid
platform = $platform
Expand Down
47 changes: 31 additions & 16 deletions .github/scripts/Resolve-RerunEligibility.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,17 +193,30 @@ Describe 'Resolve-RerunEligibility' {
$result.Reason | Should -Be 'no-new-comments-or-commits'
}

It 'accepts a non-command comment after the latest AI Summary' {
It 'accepts a non-command PR author comment after the latest AI Summary' {
$comments = @(
New-TestComment -Id 1 -Body (New-AISummaryBody) -CreatedAt '2026-05-31T09:00:00Z' -UpdatedAt '2026-05-31T09:30:00Z' -Login 'MauiBot' -Type 'User'
New-TestComment -Id 2 -Body 'I pushed the requested update.' -CreatedAt '2026-05-31T09:45:00Z'
New-TestComment -Id 10 -Body '/review rerun' -CreatedAt '2026-05-31T10:00:00Z'
)

$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 10 -CurrentHeadSha 'abcdef123'
$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 10 -CurrentHeadSha 'abcdef123' -PRAuthorLogin 'dev-user'

$result.Eligible | Should -BeTrue
$result.Reason | Should -Be 'new-comment-after-ai-summary'
$result.Reason | Should -Be 'new-author-comment-after-ai-summary'
}

It 'rejects a non-author maintainer comment after the latest AI Summary' {
$comments = @(
New-TestComment -Id 1 -Body (New-AISummaryBody) -CreatedAt '2026-05-31T09:00:00Z' -UpdatedAt '2026-05-31T09:30:00Z' -Login 'MauiBot' -Type 'User'
New-TestComment -Id 2 -Body 'Could you please check the AI suggestions?' -CreatedAt '2026-05-31T09:45:00Z' -Login 'kubaflo' -Kind 'review'
New-TestComment -Id 10 -Body '/review rerun' -CreatedAt '2026-05-31T10:00:00Z' -Login 'kubaflo'
)

$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 10 -CurrentHeadSha 'abcdef123' -PRAuthorLogin 'dev-user'

$result.Eligible | Should -BeFalse
$result.Reason | Should -Be 'no-new-comments-or-commits'
}

It 'uses AI Summary creation time as the activity checkpoint when the summary was edited later' {
Expand All @@ -213,10 +226,10 @@ Describe 'Resolve-RerunEligibility' {
New-TestComment -Id 10 -Body '/review rerun' -CreatedAt '2026-05-31T10:00:00Z'
)

$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 10 -CurrentHeadSha 'abcdef123'
$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 10 -CurrentHeadSha 'abcdef123' -PRAuthorLogin 'dev-user'

$result.Eligible | Should -BeTrue
$result.Reason | Should -Be 'new-comment-after-ai-summary'
$result.Reason | Should -Be 'new-author-comment-after-ai-summary'
}

It 'selects the newest AI Summary by creation time instead of edit time' {
Expand All @@ -227,10 +240,10 @@ Describe 'Resolve-RerunEligibility' {
New-TestComment -Id 10 -Body '/review rerun' -CreatedAt '2026-05-31T10:30:00Z'
)

$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 10 -CurrentHeadSha '2222222abcdef'
$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 10 -CurrentHeadSha '2222222abcdef' -PRAuthorLogin 'dev-user'

$result.Eligible | Should -BeTrue
$result.Reason | Should -Be 'new-comment-after-ai-summary'
$result.Reason | Should -Be 'new-author-comment-after-ai-summary'
}

It 'ignores forged AI Summary comments from non-bots' {
Expand Down Expand Up @@ -277,10 +290,10 @@ Describe 'Resolve-RerunEligibility' {
New-TestComment -Id 4659999999 -Body '/review rerun' -CreatedAt '2026-06-09T09:00:00Z'
)

$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 4659999999 -CurrentHeadSha '6e9af5bc8b5d0023400d653500951fb46df44170'
$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 4659999999 -CurrentHeadSha '6e9af5bc8b5d0023400d653500951fb46df44170' -PRAuthorLogin 'dev-user'

$result.Eligible | Should -BeTrue
$result.Reason | Should -Be 'new-comment-after-ai-summary'
$result.Reason | Should -Be 'new-author-comment-after-ai-summary'
}

It 'uses the first session marker from an AI Summary' {
Expand Down Expand Up @@ -308,18 +321,18 @@ new
$result.Reason | Should -Be 'no-new-comments-or-commits'
}

It 'accepts a non-command comment after the previous rerun command' {
It 'accepts a non-command PR author comment after the previous rerun command' {
$comments = @(
New-TestComment -Id 1 -Body (New-AISummaryBody) -CreatedAt '2026-05-31T09:00:00Z' -UpdatedAt '2026-05-31T09:30:00Z' -Login 'MauiBot' -Type 'User'
New-TestComment -Id 8 -Body '/review rerun' -CreatedAt '2026-05-31T09:45:00Z'
New-TestComment -Id 9 -Body 'Follow-up detail after rerun request.' -CreatedAt '2026-05-31T09:50:00Z'
New-TestComment -Id 10 -Body '/review rerun' -CreatedAt '2026-05-31T10:00:00Z'
)

$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 10 -CurrentHeadSha 'abcdef123'
$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 10 -CurrentHeadSha 'abcdef123' -PRAuthorLogin 'dev-user'

$result.Eligible | Should -BeTrue
$result.Reason | Should -Be 'new-comment-after-previous-rerun'
$result.Reason | Should -Be 'new-author-comment-after-previous-rerun'
}

It 'does not reuse old activity from before a previous rerun command' {
Expand All @@ -343,10 +356,10 @@ new
New-TestComment -Id 10 -Body '/review rerun' -CreatedAt '2026-05-31T10:00:00Z'
)

$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 10 -CurrentHeadSha 'abcdef123'
$result = Resolve-RerunEligibility -Comments $comments -Commits @() -CurrentCommentId 10 -CurrentHeadSha 'abcdef123' -PRAuthorLogin 'dev-user'

$result.Eligible | Should -BeTrue
$result.Reason | Should -Be 'new-comment-after-ai-summary'
$result.Reason | Should -Be 'new-author-comment-after-ai-summary'
}

It 'accepts a current head SHA that differs from the latest reviewed session' {
Expand Down Expand Up @@ -418,20 +431,22 @@ new
$comments = @(
New-TestComment -Id 1 -Body (New-AISummaryBody) -CreatedAt '2026-05-31T09:00:00Z' -UpdatedAt '2026-05-31T09:30:00Z' -Login 'MauiBot' -Type 'User'
New-TestComment -Id 2 -Body 'New author context.' -CreatedAt '2026-05-31T09:45:00Z'
New-TestComment -Id 11 -Body 'Reviewer reminder.' -CreatedAt '2026-05-31T09:46:00Z' -Login 'reviewer'
New-TestComment -Id 3 -Body '/review rerun' -CreatedAt '2026-05-31T09:50:00Z'
)
$commits = @(
New-TestCommit -Sha 'fedcba9876543210' -Date '2026-05-31T09:48:00Z'
)

$context = New-RerunContextMarkdown -Comments $comments -Commits $commits -CurrentHeadSha 'fedcba9876543210' -CurrentLabels @('s/agent-review-in-progress')
$context = New-RerunContextMarkdown -Comments $comments -Commits $commits -CurrentHeadSha 'fedcba9876543210' -PRAuthorLogin 'dev-user' -CurrentLabels @('s/agent-review-in-progress')

$context | Should -Match '# Rerun Context'
$context | Should -Match 'New non-command comments: 1'
$context | Should -Match 'New non-command author comments: 1'
$context | Should -Match 'New commits: 1'
$context | Should -Match '`s/agent-ready-for-rerun` present: false'
$context | Should -Match '`s/agent-review-in-progress` present: true'
$context | Should -Match 'New author context'
$context | Should -Not -Match 'Reviewer reminder'
$context | Should -Match 'fedcba9'
$context | Should -Not -Match '\| .*\/review rerun'
}
Expand Down
36 changes: 26 additions & 10 deletions .github/scripts/Resolve-RerunEligibility.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
.DESCRIPTION
This script is intentionally deterministic: it never uses AI and never
inspects untrusted text semantically. A rerun is eligible only when there is
new PR activity after the previous AI Summary or previous /review rerun:
a new non-command comment, or a new commit.
new PR-author activity after the previous AI Summary or previous /review rerun:
a new non-command PR-author comment, or a new commit.
#>

param(
Expand All @@ -27,7 +27,7 @@ $ErrorActionPreference = 'Stop'
$AISummaryMarker = '<!-- AI Summary -->'
$ReadyForRerunLabel = 's/agent-ready-for-rerun'
$ReviewInProgressLabel = 's/agent-review-in-progress'
$ReadyForRerunLabelDescription = 'AI review has new PR activity and is ready for rerun'
$ReadyForRerunLabelDescription = 'AI review has a new PR-author comment or commit and is ready for rerun'
$ReadyForRerunLabelColor = '5319E7'
$AISummaryAuthorLogins = @(
'MauiBot'
Expand Down Expand Up @@ -279,12 +279,22 @@ function Get-LatestReviewedSha {
function Test-CommentIsEvidence {
param(
[Parameter(Mandatory = $true)]$Comment,
[Parameter(Mandatory = $true)][Int64]$CurrentCommentId
[Parameter(Mandatory = $true)][Int64]$CurrentCommentId,
[string]$PRAuthorLogin
)

if ([Int64]$Comment.id -eq $CurrentCommentId) {
return $false
}
if ([string]::IsNullOrWhiteSpace($PRAuthorLogin)) {
return $false
}
if (-not $Comment.user -or [string]::IsNullOrWhiteSpace([string]$Comment.user.login)) {
return $false
}
if (-not ([string]$Comment.user.login).Equals($PRAuthorLogin, [StringComparison]::OrdinalIgnoreCase)) {
return $false
}
if (Test-RerunCommand $Comment.body) {
return $false
}
Expand All @@ -302,11 +312,12 @@ function Test-HasEvidenceCommentAfter {
param(
[object[]]$Comments,
[Parameter(Mandatory = $true)][datetimeoffset]$Checkpoint,
[Parameter(Mandatory = $true)][Int64]$CurrentCommentId
[Parameter(Mandatory = $true)][Int64]$CurrentCommentId,
[string]$PRAuthorLogin
)

return [bool]@($Comments | Where-Object {
(Test-CommentIsEvidence -Comment $_ -CurrentCommentId $CurrentCommentId) -and
(Test-CommentIsEvidence -Comment $_ -CurrentCommentId $CurrentCommentId -PRAuthorLogin $PRAuthorLogin) -and
(Get-ObjectDate $_ 'created_at') -gt $Checkpoint
} | Select-Object -First 1)
}
Expand Down Expand Up @@ -401,6 +412,7 @@ function New-RerunContextMarkdown {
[object[]]$Comments,
[object[]]$Commits,
[string]$CurrentHeadSha,
[string]$PRAuthorLogin,
[object[]]$CurrentLabels = @()
)

Expand All @@ -426,7 +438,7 @@ function New-RerunContextMarkdown {
$evidenceComments = @()
if ($checkpoint) {
$evidenceComments = @($Comments | Where-Object {
(Test-CommentIsEvidence -Comment $_ -CurrentCommentId 0) -and
(Test-CommentIsEvidence -Comment $_ -CurrentCommentId 0 -PRAuthorLogin $PRAuthorLogin) -and
(Get-ObjectDate $_ 'created_at') -gt $checkpoint
} | Sort-Object @{ Expression = { Get-ObjectDate $_ 'created_at' }; Descending = $false }, @{ Expression = { [Int64]$_.id }; Descending = $false })
}
Expand Down Expand Up @@ -465,6 +477,7 @@ function New-RerunContextMarkdown {
} else {
$lines.Add('- Activity checkpoint: none')
}
$lines.Add("- PR author: $(if ([string]::IsNullOrWhiteSpace($PRAuthorLogin)) { 'unknown' } else { $PRAuthorLogin })")
$lines.Add("- Latest reviewed SHA: $(if ($latestReviewedSha) { $latestReviewedSha } else { 'unknown' })")
$lines.Add("- Current head SHA: $(if ($CurrentHeadSha) { $CurrentHeadSha } else { 'unknown' })")
$lines.Add("- Current head differs from latest reviewed SHA: $($headDiffers.ToString().ToLowerInvariant())")
Expand All @@ -473,7 +486,7 @@ function New-RerunContextMarkdown {
$lines.Add('')
$lines.Add('## New activity since checkpoint')
$lines.Add('')
$lines.Add("- New non-command comments: $($evidenceComments.Count)")
$lines.Add("- New non-command author comments: $($evidenceComments.Count)")
$lines.Add("- New commits: $($newCommits.Count)")
$lines.Add('')

Expand Down Expand Up @@ -519,6 +532,7 @@ function Resolve-RerunEligibility {
[object[]]$Commits,
[Parameter(Mandatory = $true)][Int64]$CurrentCommentId,
[string]$CurrentHeadSha,
[string]$PRAuthorLogin,
[object[]]$CurrentLabels = @()
)

Expand Down Expand Up @@ -565,8 +579,8 @@ function Resolve-RerunEligibility {
return [pscustomobject]@{ Eligible = $true; Reason = 'new-head-commit'; Label = $ReadyForRerunLabel }
}

if (Test-HasEvidenceCommentAfter -Comments $Comments -Checkpoint $checkpoint -CurrentCommentId $CurrentCommentId) {
$reason = if ($checkpointReason -eq 'previous-rerun') { 'new-comment-after-previous-rerun' } else { 'new-comment-after-ai-summary' }
if (Test-HasEvidenceCommentAfter -Comments $Comments -Checkpoint $checkpoint -CurrentCommentId $CurrentCommentId -PRAuthorLogin $PRAuthorLogin) {
$reason = if ($checkpointReason -eq 'previous-rerun') { 'new-author-comment-after-previous-rerun' } else { 'new-author-comment-after-ai-summary' }
return [pscustomobject]@{ Eligible = $true; Reason = $reason; Label = $ReadyForRerunLabel }
}

Expand Down Expand Up @@ -603,6 +617,7 @@ if ($ContextOutputPath) {
-Comments $comments `
-Commits $commits `
-CurrentHeadSha $pr.head.sha `
-PRAuthorLogin $pr.user.login `
-CurrentLabels $labels
$contextDir = Split-Path -Parent $ContextOutputPath
if ($contextDir) {
Expand All @@ -627,6 +642,7 @@ $result = Resolve-RerunEligibility `
-Commits $commits `
-CurrentCommentId $CurrentCommentId `
-CurrentHeadSha $pr.head.sha `
-PRAuthorLogin $pr.user.login `
-CurrentLabels $labels

Write-Host "Rerun eligibility: $($result.Eligible) ($($result.Reason))"
Expand Down
Loading
Loading