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 all 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
26 changes: 25 additions & 1 deletion .github/scripts/Invoke-RerunReviewTrigger.Tests.ps1
View file
Open in desktop
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', 'Get-MatchingCandidate', 'Normalize-PipelineRef', 'Get-PlatformFromLabels')) {
foreach ($functionName in @('Get-ReviewTriggerRateLimitStatus', 'ConvertTo-SafeLogValue', 'ConvertTo-TrimmedString', 'Test-GhApiPrNotFound', 'Get-MatchingCandidate', 'Normalize-PipelineRef', 'Get-PlatformFromLabels')) {
$function = $ast.Find({
$args[0] -is [System.Management.Automation.Language.FunctionDefinitionAst] -and
$args[0].Name -eq $functionName
Expand Down Expand Up @@ -73,6 +73,30 @@ Describe 'ConvertTo-SafeLogValue' {
}
}

Describe 'Test-GhApiPrNotFound' {
It 'recognizes stale PR responses' {
Test-GhApiPrNotFound 'gh: Not Found (HTTP 404)' | Should -BeTrue
Test-GhApiPrNotFound 'gh: Gone (HTTP 410)' | Should -BeTrue
}

It 'does not hide credential, rate-limit, or transient failures' {
Test-GhApiPrNotFound 'gh: Bad credentials (HTTP 401)' | Should -BeFalse
Test-GhApiPrNotFound 'gh: API rate limit exceeded (HTTP 403)' | Should -BeFalse
Test-GhApiPrNotFound 'gh: Internal Server Error (HTTP 500)' | Should -BeFalse
Test-GhApiPrNotFound '' | Should -BeFalse
}
}

Describe 'ConvertTo-TrimmedString' {
It 'returns empty string for null values' {
ConvertTo-TrimmedString $null | Should -Be ''
}

It 'trims non-null values' {
ConvertTo-TrimmedString " ok`n" | Should -Be 'ok'
}
}

Describe 'Get-MatchingCandidate' {
It 'matches only PRs in the deterministic candidate set' {
$candidates = @(
Expand Down
56 changes: 55 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 @@ -68,6 +68,26 @@ function ConvertTo-SafeLogValue {
return $safe
}

function Test-GhApiPrNotFound {
param([string]$Output)

if ([string]::IsNullOrWhiteSpace($Output)) {
return $false
}

return $Output -match '(?i)\bHTTP\s+(404|410)\b' -or $Output -match '(?i)\b(Not Found|Gone)\b'
}

function ConvertTo-TrimmedString {
param([AllowNull()][object]$Value)

if ($null -eq $Value) {
return ''
}

return ([string]$Value).Trim()
}

function Add-CommentReaction {
param(
[Parameter(Mandatory = $true)][Int64]$CommentId,
Expand Down Expand Up @@ -270,6 +290,7 @@ if ($items.Count -eq 0) {
exit 0
}
$candidates = @(Get-CandidateItems -Path $env:RERUN_CANDIDATES_PATH)
$hadProcessingFailure = $false

foreach ($item in $items) {
$prNumber = 0
Expand Down Expand Up @@ -311,7 +332,33 @@ 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
$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 }
if (Test-GhApiPrNotFound -Output $prError) {
$global:LASTEXITCODE = 0
Write-Host " ⏭️ PR #$prNumber no longer exists; skipping stale decision"
continue
}

throw "Failed to load PR #$prNumber via gh api: $(ConvertTo-SafeLogValue $prError)"
}
if ([string]::IsNullOrWhiteSpace($prJson)) {
throw "Failed to load PR #$prNumber via gh api: empty response."
}
try {
$pr = $prJson | ConvertFrom-Json
} catch {
throw "Failed to parse PR #$prNumber response from gh api: $(ConvertTo-SafeLogValue ([string]$_))"
}
if ($pr.state -ne 'open') {
Write-Host " ⏭️ PR #$prNumber is not open ($($pr.state)); skipping"
continue
Expand Down Expand Up @@ -399,6 +446,13 @@ foreach ($item in $items) {
} catch {
$target = if ($prNumber -gt 0) { "PR #$prNumber" } else { "agent decision" }
Write-Host "::error::Failed to process $target`: $(ConvertTo-SafeLogValue ([string]$_))"
$hadProcessingFailure = $true
continue
}
}

if ($hadProcessingFailure) {
exit 1
}

exit 0
13 changes: 10 additions & 3 deletions .github/scripts/Query-RerunReadyPRs.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,16 @@ function Get-PlatformFromLabels {
return 'android'
}

$searchResult = gh pr list `
$searchJson = gh pr list `
--repo "$Owner/$Repo" `
--state open `
--label $ReadyForRerunLabel `
--limit $MaxPRs `
--json number,title,url,headRefOid,isDraft,labels | ConvertFrom-Json
--json number,title,url,headRefOid,isDraft,labels,author
if ($LASTEXITCODE -ne 0) {
throw "Failed to list open PRs labeled '$ReadyForRerunLabel' (gh pr list exited with code $LASTEXITCODE)."
}
$searchResult = $searchJson | ConvertFrom-Json

$candidates = @()
foreach ($pr in @($searchResult)) {
Expand All @@ -101,14 +105,17 @@ 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
$rawAuthorLogin = if ($pr.author -and $pr.author.login) { [string]$pr.author.login } else { '' }
$authorLogin = Normalize-GitHubActorLogin $rawAuthorLogin
$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
70 changes: 54 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 @@ -69,6 +69,12 @@ BeforeAll {
}

Describe 'Resolve-RerunEligibility' {
It 'normalizes app-style GitHub bot author logins' {
Normalize-GitHubActorLogin 'app/dependabot' | Should -Be 'dependabot[bot]'
Normalize-GitHubActorLogin ' dependabot[bot] ' | Should -Be 'dependabot[bot]'
Normalize-GitHubActorLogin '' | Should -Be ''
}

It 'parses review command branch and platform options for reruns' {
$parsed = ConvertFrom-ReviewCommand '/review -b feature/regression-check -p ios'

Expand All @@ -90,6 +96,11 @@ Describe 'Resolve-RerunEligibility' {
Should -Be 'feature/regression-check'
}

It 'normalizes app-style GitHub actor logins to bot logins' {
Normalize-GitHubActorLogin 'app/dependabot' | Should -Be 'dependabot[bot]'
Normalize-GitHubActorLogin 'dev-user' | Should -Be 'dev-user'
}

It 'finds latest normal review command while ignoring rerun and tests commands' {
$comments = @(
New-TestComment -Id 1 -Body '/review -b old/ref -p android' -CreatedAt '2026-05-31T09:00:00Z'
Expand Down Expand Up @@ -193,17 +204,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 +237,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 +251,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 +301,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 +332,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 +367,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,21 +442,35 @@ 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'
}

It 'renders normalized app-style bot authors in rerun context' {
$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 3 -Body '/review rerun' -CreatedAt '2026-05-31T09:50:00Z'
)

$context = New-RerunContextMarkdown -Comments $comments -Commits @() -CurrentHeadSha 'abcdef123' -PRAuthorLogin 'app/dependabot'

$context | Should -Match 'PR author: dependabot\[bot\]'
$context | Should -Match 'New non-command author comments: 0'
}

@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.

💡 Test Quality — this test name overclaims; the "no bot evidence" assertions are satisfied by the pre-existing user.type -eq 'Bot' filter, not by the new normalization
Flagged by: 2/3 reviewers explicitly + 1 supporting non-finding (provenance: round-2 introduced)

The round-2 fix added Normalize-GitHubActorLogin and applies it inside Test-CommentIsEvidence (check #4 — author equality, now normalized). But check #6 in the same function still unconditionally rejects Comment.user.type -eq 'Bot' — that line survived round-2 unchanged. So for a bot-authored PR, the bot's own comments are rejected at #6 regardless of whether #4 matches.

Empirically verified by dot-sourcing the function:

  • Test-CommentIsEvidence(botComment, PRAuthorLogin='app/dependabot') → False (rejected at [Spec] Transitions #6, even though Aloha System.Maui! #4 now passes)
  • Removing Normalize-GitHubActorLogin from inside Test-CommentIsEvidence and running this test: still passes — because New non-command author comments: 0 is satisfied by [Spec] Transitions #6, and Should -Not -Match 'Dependabot follow-up' is satisfied by the rendering loop which filters via the same Test-CommentIsEvidence.
  • The only assertion in this test that actually exercises the new code is line 470: Should -Match 'PR author: dependabot[bot]' (rendering the normalized author).

Implication: the fix is decision-neutral for bot evidence — its only functional effect is the displayed PR author: line in the context markdown (an advisory input to the AI scanner; the deterministic evidence counts are unaffected). Constructing a non-bot comment whose user.login needs app/→[bot] normalization to match the author is not a realistic scenario — REST issue/PR comments always emit bot logins in the X[bot] form, never app/X.

Fix: either (a) rename this test to focus solely on display normalization (renders normalized app-style bot author in context markdown) and remove the misleading "without counting bot comments as evidence" claim, or (b) split into two tests: one for the display (passes with the fix), one for bot-comment-rejection (passes regardless, documenting the #6 filter as the actual gate). Also worth removing the Normalize-GitHubActorLogin call inside Test-CommentIsEvidence itself — it's dead code given check #6 — though leaving it as defensive symmetry is also defensible.

}
Loading
Loading