Repository navigation
fix(issue-pr-loop): finish open PRs before starting new issues #431
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,14 +10,19 @@ | |
| # 2. Runs the merged-worktree sweep and surveys open PRs and issues with `gh`. When | ||
| # the latest push CI run of any workflow on main failed, fixing main comes first. | ||
| # 3. Skips drafts, `needs-human` and `on-hold` items, PRs whose checks are still | ||
| # running, and items whose GitHub state is unchanged since an agent last deferred | ||
| # them. Also skips issues that cannot be worked yet: issues with open native | ||
| # sub-issues or open `blocked_by` dependencies, issues that already have an open | ||
| # closing PR, and bot-owned tracking issues. Stranded branches follow the same | ||
| # rules. Ranks local `issue-*` branches with unpublished commits and no open PR | ||
| # above new issues, then parents whose sub-issues are all closed, then issues that | ||
| # block the most other issues. An item that a deferral named as its blocker goes | ||
| # first, so the next unit works the blocker instead of the blocked item. | ||
| # running (unless they conflict, have unresolved review threads, or have a | ||
| # changes-requested review newer than the last push), and items whose GitHub state | ||
| # is unchanged since an agent last deferred them. Also skips issues that cannot be | ||
| # worked yet: issues with open native sub-issues or open `blocked_by` dependencies, | ||
| # issues that already have an open closing PR, and bot-owned tracking issues. | ||
| # Stranded branches follow the same rules. Ranks local `issue-*` branches with | ||
| # unpublished commits and no open PR above new issues. Starts no new issue while | ||
| # -MaxOpenPrs or more of the loop's PRs (drafts included) are open and not parked, | ||
| # because every PR in flight is another chance of a conflict; existing PRs, | ||
| # stranded branches, and stale claims still run. Then come parents whose | ||
| # sub-issues are all closed, then issues that block the most other issues. An item | ||
| # that a deferral named as its blocker goes first, so the next unit works the | ||
| # blocker instead of the blocked item. | ||
| # 4. Acquires the item's Redis lock, moving on to the next candidate when another | ||
| # worker holds it. | ||
| # 5. Starts the agent on that one item, with lower reasoning effort for PRs that | ||
|
|
@@ -59,7 +64,7 @@ | |
| # | ||
| # Stop: create the stop file printed at startup (checked between units), or Ctrl+C. | ||
| # Usage: pwsh scripts/Start-IssuePrLoop.ps1 [-Agent auto|claude|codex] [-IdleSeconds n] [-MaxUnits n] | ||
| # [-DeferSeconds n] [-MaxDeferSeconds n] [-LightEffort level] [-HeavyEffort level] | ||
| # [-DeferSeconds n] [-MaxDeferSeconds n] [-MaxOpenPrs n] [-LightEffort level] [-HeavyEffort level] | ||
| # [-TriageEvery n] [-TriageBatch n] [-AuditDays n] [-ParkedAuditHours n] [-DiscoveryHours n] [-DryRun] | ||
| # pwsh scripts/Start-IssuePrLoop.ps1 -Report [-ReportHours n] | ||
|
|
||
|
|
@@ -78,6 +83,9 @@ param( | |
| # Upper bound for a repeatedly deferred item's backoff. | ||
| [ValidateRange(60, 604800)] | ||
| [int]$MaxDeferSeconds = 259200, | ||
| # Open, unparked loop PRs at which no new issue is started. 0 means no limit. | ||
| [ValidateRange(0, 1000)] | ||
| [int]$MaxOpenPrs = 8, | ||
| # Reasoning effort for PRs that look ready to merge, and for all other units. | ||
| # An empty value keeps the agent's configured default. | ||
| [string]$LightEffort = 'medium', | ||
|
|
@@ -252,6 +260,62 @@ function Get-PrFingerprint($Pr) { | |
| Get-Hash ((@($Pr.headRefOid, $Pr.mergeable, ($labels -join ','), $review, $comment) + $checks) -join '|') | ||
| } | ||
|
|
||
| function ConvertTo-Instant($Value) { | ||
| if ($Value -is [datetime]) { return [DateTimeOffset]$Value } | ||
| if ($Value) { return [DateTimeOffset]::Parse([string]$Value, [Globalization.CultureInfo]::InvariantCulture) } | ||
| return $null | ||
| } | ||
|
|
||
| # True when a PR has review feedback that no unit has dispositioned: an unresolved review | ||
| # thread (units resolve each thread they disposition), or a changes-requested review newer | ||
| # than both the last push and the loop's last reply. Such feedback can be handled while | ||
| # checks still run. Conversation comments do not count: a finished review with no | ||
| # findings is also a comment, and would send units to PRs with nothing to do. | ||
| function Test-HasNewFeedback($Pr, $Activity) { | ||
| if (-not $Activity) { return $false } | ||
| if ($Activity.UnresolvedThreads -gt 0) { return $true } | ||
| $since = ConvertTo-Instant $Activity.HeadCommittedAt | ||
| if (-not $since) { return $false } | ||
| $requested = [Collections.Generic.List[object]]::new() | ||
| foreach ($review in @($Pr.reviews)) { | ||
| if (-not $review -or -not $review.submittedAt) { continue } | ||
| $at = ConvertTo-Instant $review.submittedAt | ||
| # The loop replies to review threads as the PR author, which GitHub records as reviews. | ||
| if ($review.author.login -eq $Pr.author.login) { if ($at -gt $since) { $since = $at } } | ||
| elseif ($review.state -eq 'CHANGES_REQUESTED') { $requested.Add($at) } | ||
| } | ||
| foreach ($comment in @($Pr.comments)) { | ||
| if ($comment -and $comment.createdAt -and ([string]$comment.body).Contains($loopMarker)) { | ||
| $at = ConvertTo-Instant $comment.createdAt | ||
| if ($at -gt $since) { $since = $at } | ||
| } | ||
| } | ||
| @($requested | Where-Object { $_ -gt $since }).Count -gt 0 | ||
| } | ||
|
|
||
| # Returns PR number -> head commit time and unresolved review thread count for open PRs. | ||
| # `gh pr list --json commits` asks for every commit's authors and exceeds GitHub's | ||
| # GraphQL node limit, so this asks only for the head commit. | ||
| function Get-PrActivity { | ||
| $query = 'query($owner: String!, $name: String!, $cursor: String) { repository(owner: $owner, name: $name) { ' + | ||
| 'pullRequests(states: OPEN, first: 100, after: $cursor) { pageInfo { hasNextPage endCursor } ' + | ||
| 'nodes { number commits(last: 1) { nodes { commit { committedDate } } } reviewThreads(first: 100) { nodes { isResolved } } } } } }' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For a PR with more than 100 review threads, this query reads only the first 100 threads; the surrounding pagination advances the PR connection, not each PR's thread connection. If those initial threads are resolved and a later thread contains new feedback without a Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fetch all thread pages before treating the PR as having no unresolved feedback. |
||
| $activity = @{} | ||
| $cursor = $null | ||
| do { | ||
| $variables = if ($cursor) { @('-f', "cursor=$cursor") } else { @() } | ||
| $page = (Invoke-GraphQl $query $variables).pullRequests | ||
| foreach ($node in @($page.nodes)) { | ||
| $activity[[int]$node.number] = [pscustomobject]@{ | ||
| HeadCommittedAt = @($node.commits.nodes)[0].commit.committedDate | ||
| UnresolvedThreads = @($node.reviewThreads.nodes | Where-Object { -not $_.isResolved }).Count | ||
| } | ||
| } | ||
| $cursor = $page.pageInfo.endCursor | ||
| } while ($page.pageInfo.hasNextPage) | ||
| return $activity | ||
| } | ||
|
|
||
| function Get-IssueFingerprint($Issue) { | ||
| $labels = @($Issue.labels.nodes | ForEach-Object { $_.name } | Sort-Object) | ||
| $assignees = @($Issue.assignees.nodes | ForEach-Object { $_.login } | Sort-Object) | ||
|
|
@@ -317,18 +381,18 @@ $deferCategories = @('human', 'upstream', 'credentials', 'locked') | |
| $surveyStats = [ordered]@{} | ||
|
|
||
| function Reset-SurveyStats { | ||
| foreach ($key in 'issues', 'issue-deferred', 'issue-locked', 'issue-excluded', 'issue-parent', 'issue-blocked', 'issue-has-pr', 'issue-claimed', | ||
| foreach ($key in 'issues', 'issue-wip', 'issue-deferred', 'issue-locked', 'issue-excluded', 'issue-parent', 'issue-blocked', 'issue-has-pr', 'issue-claimed', | ||
| 'prs', 'pr-deferred', 'pr-locked', 'pr-pending', 'pr-excluded', 'pr-draft') { | ||
| $surveyStats[$key] = 0 | ||
| } | ||
| } | ||
|
|
||
| function Format-SurveyStats { | ||
| $s = $surveyStats | ||
| ('issues: {0} open, {1} deferred, {2} locked, {3} excluded by label/assignee/bot, {4} with open sub-issues, {5} blocked, {6} with open PR, {7} claimed; ' + | ||
| 'PRs: {8} open, {9} deferred, {10} locked, {11} pending checks, {12} needs-human/on-hold, {13} draft') -f ` | ||
| ('issues: {0} open, {1} deferred, {2} locked, {3} excluded by label/assignee/bot, {4} with open sub-issues, {5} blocked, {6} with open PR, {7} claimed, ' + | ||
| '{14} held by the open-PR limit; PRs: {8} open, {9} deferred, {10} locked, {11} pending checks, {12} needs-human/on-hold, {13} draft') -f ` | ||
| $s['issues'], $s['issue-deferred'], $s['issue-locked'], $s['issue-excluded'], $s['issue-parent'], $s['issue-blocked'], $s['issue-has-pr'], $s['issue-claimed'], | ||
| $s['prs'], $s['pr-deferred'], $s['pr-locked'], $s['pr-pending'], $s['pr-excluded'], $s['pr-draft'] | ||
| $s['prs'], $s['pr-deferred'], $s['pr-locked'], $s['pr-pending'], $s['pr-excluded'], $s['pr-draft'], $s['issue-wip'] | ||
| } | ||
|
|
||
| function Invoke-GraphQl([string]$Query, [string[]]$Variables) { | ||
|
|
@@ -456,21 +520,32 @@ function Get-Candidates { | |
| $prs = gh pr list --repo $repoSlug --author '@me' --state open --limit 500 --json $prFields | ConvertFrom-Json | ||
| if ($LASTEXITCODE -ne 0) { throw 'gh pr list failed.' } | ||
| $surveyStats['prs'] = @($prs).Count | ||
| # Open, unparked PRs, drafts included: each is another chance of a conflict. | ||
| $inFlight = 0 | ||
| $prActivity = if (@($prs).Count -gt 0) { Get-PrActivity } else { @{} } | ||
| $ranked = foreach ($pr in $prs) { | ||
| $parked = @($pr.labels | Where-Object { $_.name -in $excludedPrLabels }).Count -gt 0 | ||
| if (-not $parked) { $inFlight++ } | ||
| if ($pr.isDraft) { $surveyStats['pr-draft']++; continue } | ||
| if (@($pr.labels | Where-Object { $_.name -in $excludedPrLabels }).Count -gt 0) { $surveyStats['pr-excluded']++; continue } | ||
| if ($parked) { $surveyStats['pr-excluded']++; continue } | ||
| $checks = @($pr.statusCheckRollup) | ||
| $pending = @($checks | Where-Object { (Get-CheckState $_) -in 'PENDING', 'EXPECTED', 'QUEUED', 'IN_PROGRESS', 'WAITING', 'REQUESTED', '' }) | ||
| $failed = @($checks | Where-Object { (Get-CheckState $_) -in 'FAILURE', 'ERROR', 'TIMED_OUT', 'CANCELLED', 'ACTION_REQUIRED', 'STARTUP_FAILURE' }) | ||
| $conflicting = $pr.mergeable -eq 'CONFLICTING' | ||
| # Running checks leave nothing to do yet; the next survey picks the PR up. | ||
| if ($pending.Count -gt 0 -and -not $conflicting) { $surveyStats['pr-pending']++; continue } | ||
| $rank = if ($pr.mergeStateStatus -eq 'CLEAN' -and $failed.Count -eq 0) { 0 } | ||
| $feedback = Test-HasNewFeedback $pr $prActivity[[int]$pr.number] | ||
| # Running checks leave nothing to do yet, unless the PR conflicts or has new review | ||
| # feedback: finishing open PRs beats starting new ones. The next survey picks it up. | ||
| if ($pending.Count -gt 0 -and -not $conflicting -and -not $feedback) { $surveyStats['pr-pending']++; continue } | ||
| $rank = if ($pr.mergeStateStatus -eq 'CLEAN' -and $failed.Count -eq 0 -and -not $feedback) { 0 } | ||
| elseif ($conflicting) { 1 } | ||
| elseif ($failed.Count -gt 0) { 2 } | ||
| elseif ($failed.Count -gt 0 -or $feedback) { 2 } | ||
| else { 3 } | ||
| $hint = if ($feedback -and $pending.Count -gt 0) { | ||
| "PR #$($pr.number) has review feedback without a disposition while checks are still running. Handle the feedback now instead of waiting for the checks." | ||
| } | ||
| else { '' } | ||
| # A green, clean PR usually needs only the merge gate or a disposition reply. | ||
| New-Candidate 'pr' ([int]$pr.number) $rank (Get-PrFingerprint $pr) ($rank -eq 0) | ||
| New-Candidate 'pr' ([int]$pr.number) $rank (Get-PrFingerprint $pr) ($rank -eq 0) $hint | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reopening a resolved thread can leave a waiting PR skipped. Include thread state in both survey and post-unit fingerprints so reopened feedback wakes the PR without repeatedly waking unchanged feedback. |
||
| } | ||
| foreach ($c in @($ranked | Sort-Object Rank, Number)) { $candidates.Add($c) } | ||
|
|
||
|
|
@@ -479,6 +554,9 @@ function Get-Candidates { | |
| $openIssues = @{} | ||
| foreach ($issue in $issues) { $openIssues[[int]$issue.number] = $issue } | ||
| $script:lastIssues = $issues | ||
| # Every open PR is another chance of a conflict, so finish existing work before | ||
| # starting more once enough PRs are in flight. | ||
| $atPrLimit = $MaxOpenPrs -gt 0 -and $inFlight -ge $MaxOpenPrs | ||
| $stranded = Get-StrandedBranches $openIssues | ||
| foreach ($number in @($stranded.Keys | Sort-Object)) { | ||
| $s = $stranded[$number] | ||
|
|
@@ -530,6 +608,7 @@ function Get-Candidates { | |
| New-Candidate 'issue' $number 5 $fingerprint $true $hint | ||
| continue | ||
| } | ||
| if ($atPrLimit) { $surveyStats['issue-wip']++; continue } | ||
| $candidate = New-Candidate 'issue' $number 6 $fingerprint $false | ||
| $candidate.Blocking = [int]$issue.issueDependenciesSummary.blocking | ||
| $candidate.Comments = [int]$issue.comments.totalCount | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a reviewer posts findings as a PR conversation comment while checks are pending, this function returns false unless there is also an unresolved thread or a qualifying
CHANGES_REQUESTEDreview.Get-Candidatesconsequently skips the PR and can start another issue despite the outstanding findings. AGENTS.md explicitly identifies conversation comments as a review surface used by Claude and requires checking both surfaces every cycle. Distinguish actionable conversation reviews from empty acknowledgments rather than excluding that surface entirely.AGENTS.md reference: AGENTS.md:L63-L63
Useful? React with 👍 / 👎.