Repository navigation
feat(issue-pr-loop): finish open PRs before starting new issues - #4084
Conversation
Add -MaxOpenPrs (default 8): while that many unparked loop PRs are open, no fresh issue is queued. PRs with unresolved review threads or a changes-requested review newer than the last push are no longer skipped for pending checks.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 9 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. CodeRabbit Verify
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Review: no blocking issues. I read the diff against the surrounding script. This is a script-only change, so none of the AGENTS.md hot-path or benchmark rules apply.
One minor thing, not blocking. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53970c51f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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.
For a PR with more than 100 review threads, this query ignores every thread after the first page; the surrounding cursor loop paginates only pullRequests. If the first 100 threads are resolved and a later thread is unresolved, Test-HasNewFeedback returns false unless a separate changes-requested review qualifies. Consequently, a pending PR is still skipped despite actionable feedback, and a clean PR can incorrectly receive light effort. Fetch the remaining thread pages before computing UnresolvedThreads, as the existing review gate in scripts/Assert-PrGreen.ps1 does.
AGENTS.md reference: AGENTS.md:L54-L55
Useful? React with 👍 / 👎.
 [Medium impact] The changes affect work selection in the issue/PR loop. Fix the deferral check before merging so reopened feedback can actually reach a worker. FindingsSummaryThis PR makes review feedback actionable while checks run and adds
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| 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.
When a reviewer reopens a resolved thread on a deferred PR, Test-HasNewFeedback makes it actionable. But Get-PrFingerprint leaves out thread state. If the head, checks, and other comments are unchanged, Test-Deferred still skips the PR until CI changes or the deferral expires. Include thread state in both survey and post-unit fingerprints so this feedback can reach a worker immediately.
| 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.
reviewThreads(first: 100) reads only one page. If a PR has more than 100 threads, the first page can be fully resolved while a later thread remains open. UnresolvedThreads then becomes zero, so a comment-only review on that later thread does not wake the PR while checks run. Read the remaining thread pages before deciding that no unresolved feedback exists.
Problem
Loop workers skipped every PR whose checks were still running and started new issues instead, so open PRs piled up and the chance of conflicts grew.
Change
Ported from thomhurst/TUnit#7423.
scripts/Start-IssuePrLoop.ps1: new-MaxOpenPrs(default 8, 0 = no limit). While that many of the loop's open PRs lackneeds-human/on-hold, no fresh issue is queued; stranded branches, stale in-progress claims and closed-children parents still are. The held count appears in the idle survey stats.CHANGES_REQUESTEDreview newer than both the head commit and the loop's last reply, is no longer skipped for pending checks: it ranks with failed-CI PRs, is not a light unit, and gets a hint to handle the feedback now. Conversation comments do not count (a review with no findings is also a comment).Get-PrActivityreads head commit time and review-thread resolution for all open PRs in one paginated GraphQL query (addingcommitstogh pr list --jsonexceeds GitHub's node limit).scripts/Test-IssuePrLoop.ps1: tests for the feedback detection and the open-PR limit.The matching SKILL.md priority-list text was pushed to
mainin 09bf00c.Validation:
pwsh -NoProfile -File scripts/Test-IssuePrLoop.ps1passes;Get-PrActivitychecked against this repository's live open PRs. Script-only change, no hot-path cost.