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
2 changes: 1 addition & 1 deletion .github/docs/agent-labels.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ is:pr label:s/agent-reviewed
| `.github/scripts/shared/Update-AgentLabels.ps1` | Label helper module (all label logic) |
| `.github/scripts/Review-PR.ps1` | Orchestrator that calls `Apply-AgentLabels` in Phase 4 |
| `.github/scripts/Resolve-RerunEligibility.ps1` | Deterministic `/review rerun` checker that can apply `s/agent-ready-for-rerun` |
| `.github/scripts/Invoke-RerunReviewTrigger.ps1` | Safe-output handler that applies `s/agent-review-in-progress` before triggering AzDO reruns |
| `.github/scripts/Invoke-RerunReviewTrigger.ps1` | Safe-output handler that validates rerun decisions and emits an actions list; the scanner then dispatches `review-trigger.yml` (which applies `s/agent-review-in-progress` and triggers the AzDO review) |
| `.github/workflows/review-trigger.yml` | Manual `/review` trigger that applies `s/agent-review-in-progress` before triggering AzDO reviews |
| `eng/pipelines/ci-copilot.yml` | AzDO review pipeline that removes `s/agent-review-in-progress` in final cleanup |
| `.github/skills/pr-review/SKILL.md` | Documents label system for the pr-review skill |
Expand Down
195 changes: 127 additions & 68 deletions .github/scripts/Invoke-RerunReviewTrigger.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ BeforeAll {
throw ($parseErrors | ForEach-Object { $_.Message }) -join [Environment]::NewLine
}

$script:ReviewTriggerCooldownMinutes = 60
$script:ReviewTriggerWindowHours = 24
$script:MaxReviewTriggersPerWindow = 3

foreach ($functionName in @('Get-ReviewTriggerRateLimitStatus', 'ConvertTo-SafeLogValue', 'ConvertTo-TrimmedString', 'Test-GhApiPrNotFound', 'Get-MatchingCandidate', 'Normalize-PipelineRef', 'Get-PlatformFromLabels', 'Expand-RerunDecisionItems')) {
foreach ($functionName in @('ConvertTo-SafeLogValue', 'ConvertTo-TrimmedString', 'Get-MatchingCandidate', 'Normalize-PipelineRef', 'Get-PlatformFromLabels', 'Expand-RerunDecisionItems', 'Get-RerunActions')) {
$function = $ast.Find({
$args[0] -is [System.Management.Automation.Language.FunctionDefinitionAst] -and
$args[0].Name -eq $functionName
Expand Down Expand Up @@ -44,6 +40,20 @@ BeforeAll {
rerunCommentId = $RerunCommentId
}
}

function New-TestDecision {
param(
[string]$PRNumber = '123',
[string]$Decision = 'trigger',
[string]$ExpectedHeadSha = 'abc123def'
)

[pscustomobject]@{
pr_number = $PRNumber
decision = $Decision
expected_head_sha = $ExpectedHeadSha
}
}
}

Describe 'ConvertTo-SafeLogValue' {
Expand Down Expand Up @@ -73,28 +83,6 @@ 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
}

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"}]'
Expand Down Expand Up @@ -210,8 +198,6 @@ Describe 'Candidate-sourced values' {
It 'produces the rerun comment id from candidate data, not the agent emission' {
$candidate = New-TestCandidate -RerunCommentId 4242

# The handler reads $candidate.rerunCommentId via:
# $rerunCommentId = if ($candidate.rerunCommentId) { [Int64]$candidate.rerunCommentId } else { [Int64]0 }
$rerunCommentId = if ($candidate.rerunCommentId) { [Int64]$candidate.rerunCommentId } else { [Int64]0 }

$rerunCommentId | Should -Be 4242
Expand Down Expand Up @@ -243,58 +229,131 @@ Describe 'Candidate-sourced values' {
}
}

Describe 'Get-ReviewTriggerRateLimitStatus' {
It 'allows a PR with no recent rerun triggers' {
$now = [datetimeoffset]'2026-06-04T12:00:00Z'
Describe 'Get-RerunActions' {
It 'produces a normalized action for a valid trigger decision, sourcing values from the candidate' {
$items = @(New-TestDecision -PRNumber '123' -Decision 'trigger' -ExpectedHeadSha 'abc123def')
$candidates = @(New-TestCandidate -PRNumber 123 -HeadSha 'abc123def' -Platform 'ios' -PipelineRef 'main' -RerunCommentId 4242)

$result = Get-RerunActions -Items $items -Candidates $candidates -DefaultPipelineRef 'main'

$result.HadFailure | Should -BeFalse
$result.Actions.Count | Should -Be 1
$result.Actions[0].prNumber | Should -Be 123
$result.Actions[0].decision | Should -Be 'trigger'
$result.Actions[0].platform | Should -Be 'ios'
$result.Actions[0].pipelineRef | Should -Be 'main'
$result.Actions[0].rerunCommentId | Should -Be 4242
}

It 'produces an action for a valid skip decision even without a rerun comment id' {
$items = @(New-TestDecision -PRNumber '50' -Decision 'skip' -ExpectedHeadSha 'sha50')
$candidates = @(New-TestCandidate -PRNumber 50 -HeadSha 'sha50' -RerunCommentId 0)

$result = Get-ReviewTriggerRateLimitStatus -TriggeredAt @() -Now $now
$result = Get-RerunActions -Items $items -Candidates $candidates

$result.Allowed | Should -BeTrue
$result.Reason | Should -Be 'allowed'
$result.RecentCount | Should -Be 0
$result.HadFailure | Should -BeFalse
$result.Actions.Count | Should -Be 1
$result.Actions[0].decision | Should -Be 'skip'
$result.Actions[0].rerunCommentId | Should -Be 0
}

It 'blocks rerun triggers inside the cooldown window' {
$now = [datetimeoffset]'2026-06-04T12:00:00Z'
It 'normalizes the pipeline ref and resolves an invalid candidate platform to android' {
$items = @(New-TestDecision -PRNumber '7' -Decision 'trigger' -ExpectedHeadSha 's7')
$candidates = @(New-TestCandidate -PRNumber 7 -HeadSha 's7' -Platform 'macos' -PipelineRef 'refs/heads/feature/x' -RerunCommentId 9)

$result = Get-ReviewTriggerRateLimitStatus `
-TriggeredAt @([datetimeoffset]'2026-06-04T11:30:00Z') `
-Now $now
$result = Get-RerunActions -Items $items -Candidates $candidates

$result.Allowed | Should -BeFalse
$result.Reason | Should -Be 'cooldown-active'
$result.RecentCount | Should -Be 1
$result.HadFailure | Should -BeFalse
$result.Actions[0].platform | Should -Be 'android'
$result.Actions[0].pipelineRef | Should -Be 'feature/x'
}

It 'blocks when the 24 hour quota is exhausted' {
$now = [datetimeoffset]'2026-06-04T12:00:00Z'
It 'aggregates multiple decisions in one pass' {
$items = @(
(New-TestDecision -PRNumber '2' -Decision 'skip' -ExpectedHeadSha 's2'),
(New-TestDecision -PRNumber '1' -Decision 'trigger' -ExpectedHeadSha 's1')
)
$candidates = @(
(New-TestCandidate -PRNumber 1 -HeadSha 's1' -RerunCommentId 11),
(New-TestCandidate -PRNumber 2 -HeadSha 's2' -RerunCommentId 22)
)

$result = Get-ReviewTriggerRateLimitStatus `
-TriggeredAt @(
[datetimeoffset]'2026-06-04T10:00:00Z',
[datetimeoffset]'2026-06-04T08:00:00Z',
[datetimeoffset]'2026-06-03T13:00:00Z'
) `
-Now $now
$result = Get-RerunActions -Items $items -Candidates $candidates

$result.Allowed | Should -BeFalse
$result.Reason | Should -Be 'rerun-quota-exhausted'
$result.RecentCount | Should -Be 3
$result.HadFailure | Should -BeFalse
$result.Actions.Count | Should -Be 2
}

It 'ignores trigger history older than the window' {
$now = [datetimeoffset]'2026-06-04T12:00:00Z'
It 'flags a failure and drops the action when pr_number is not a positive integer' {
$items = @(New-TestDecision -PRNumber '0' -Decision 'trigger' -ExpectedHeadSha 'x')

$result = Get-RerunActions -Items $items -Candidates @()

$result.HadFailure | Should -BeTrue
$result.Actions.Count | Should -Be 0
}

It 'flags a failure for an unknown decision verb' {
$items = @(New-TestDecision -PRNumber '5' -Decision 'launch' -ExpectedHeadSha 'x')
$candidates = @(New-TestCandidate -PRNumber 5 -HeadSha 'x')

$result = Get-RerunActions -Items $items -Candidates $candidates

$result.HadFailure | Should -BeTrue
$result.Actions.Count | Should -Be 0
}

It 'flags a failure when the expected head SHA is missing' {
$items = @(New-TestDecision -PRNumber '5' -Decision 'trigger' -ExpectedHeadSha '')
$candidates = @(New-TestCandidate -PRNumber 5 -HeadSha 'x' -RerunCommentId 1)

$result = Get-RerunActions -Items $items -Candidates $candidates

$result.HadFailure | Should -BeTrue
$result.Actions.Count | Should -Be 0
}

It 'rejects a decision for a PR outside the deterministic candidate set' {
$items = @(New-TestDecision -PRNumber '999' -Decision 'trigger' -ExpectedHeadSha 'x')
$candidates = @(New-TestCandidate -PRNumber 5 -HeadSha 'x')

$result = Get-RerunActions -Items $items -Candidates $candidates

$result.HadFailure | Should -BeTrue
$result.Actions.Count | Should -Be 0
}

It 'rejects a decision whose head SHA does not match the candidate (anti-stale / anti-hallucination)' {
$items = @(New-TestDecision -PRNumber '5' -Decision 'trigger' -ExpectedHeadSha 'STALE')
$candidates = @(New-TestCandidate -PRNumber 5 -HeadSha 'FRESH' -RerunCommentId 1)

$result = Get-RerunActions -Items $items -Candidates $candidates

$result.HadFailure | Should -BeTrue
$result.Actions.Count | Should -Be 0
}

It 'refuses to trigger when the candidate has no rerun comment id' {
$items = @(New-TestDecision -PRNumber '5' -Decision 'trigger' -ExpectedHeadSha 'x')
$candidates = @(New-TestCandidate -PRNumber 5 -HeadSha 'x' -RerunCommentId 0)

$result = Get-RerunActions -Items $items -Candidates $candidates

$result.HadFailure | Should -BeTrue
$result.Actions.Count | Should -Be 0
}

It 'continues processing valid decisions after a failed one' {
$items = @(
(New-TestDecision -PRNumber 'bad' -Decision 'trigger' -ExpectedHeadSha 'x'),
(New-TestDecision -PRNumber '5' -Decision 'skip' -ExpectedHeadSha 's5')
)
$candidates = @(New-TestCandidate -PRNumber 5 -HeadSha 's5' -RerunCommentId 1)

$result = Get-ReviewTriggerRateLimitStatus `
-TriggeredAt @(
[datetimeoffset]'2026-06-04T10:00:00Z',
[datetimeoffset]'2026-06-03T11:00:00Z',
[datetimeoffset]'2026-06-02T12:00:00Z'
) `
-Now $now
$result = Get-RerunActions -Items $items -Candidates $candidates

$result.Allowed | Should -BeTrue
$result.Reason | Should -Be 'allowed'
$result.RecentCount | Should -Be 1
$result.HadFailure | Should -BeTrue
$result.Actions.Count | Should -Be 1
$result.Actions[0].prNumber | Should -Be 5
}
}
Loading
Loading