Repository navigation
fix(issue-pr-loop): finish open PRs before starting new issues - #431
Conversation
Workers skipped every PR whose checks were still running and claimed new issues instead, so open PRs piled up and conflicts grew more likely. - Add -MaxOpenPrs (default 8, 0 = no limit): while that many of the loop's open PRs lack needs-human/on-hold, no new issue is queued. Stranded branches, stale claims and finished parents still are. The held count shows in the survey stats. - A PR with an unresolved review thread, or a changes-requested review newer than the last push and the loop's last reply, is worked even while its checks run, ranks with failed-CI PRs, and is not Light. - Get-PrActivity reads head commit time and thread resolution with one paginated GraphQL query (gh pr list --json commits exceeds the node limit). Ported from thomhurst/TUnit#7423. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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. |
|
Caution Review failedThe pull request is closed. Walkthrough
Merge Risk:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6eb4ada871
ℹ️ 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".
| # 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. |
There was a problem hiding this comment.
Include actionable conversation reviews in feedback detection
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_REQUESTED review. Get-Candidates consequently 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 👍 / 👎.
| 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.
Paginate review threads before counting unresolved feedback
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 CHANGES_REQUESTED review, UnresolvedThreads becomes zero and the pending-check filter skips actionable work. Request thread pageInfo and fetch the remaining pages before deciding that the PR has no unresolved feedback.
Useful? React with 👍 / 👎.
[Medium impact] The PR changes which work the issue-PR loop sends to agents. The PR appears safe to merge, with non-blocking gaps that can delay work on review feedback. FindingsSummaryThe PR adds
Reviews (1) · Last reviewed commit: "docs(issue-pr-loop): rewrap the survey s..." · Reviewed by Greptile |
| 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.
Reopening a resolved thread can leave a waiting PR skipped. Test-HasNewFeedback detects the thread, but Get-PrFingerprint excludes thread state. Select-Item therefore keeps the existing wait until checks finish or it expires.
Include thread state in both survey and post-unit fingerprints so reopened feedback wakes the PR without repeatedly waking unchanged 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.
Get-PrActivity reads only the first 100 review threads, including resolved threads. If those are resolved and a later thread remains open, UnresolvedThreads becomes zero. A pending PR without a newer changes-requested review is then skipped until checks finish.
Fetch all thread pages before treating the PR as having no unresolved feedback.
ReviewThe change looks sound. I read the diff and checked that the helpers it relies on exist in the PR head ( What I checked
Minor points, not blocking
LGTM overall. |
Workers skipped every PR whose checks were still running and claimed new issues instead, so open PRs piled up and the chance of conflicts grew.
Changes to
scripts/Start-IssuePrLoop.ps1:-MaxOpenPrs(default 8, 0 = no limit). While that many of the loop's open PRs (drafts included) lackneeds-human/on-hold, no new issue is queued; stranded branches, stale in-progress claims and finished parents still are. The held count appears in the survey stats ("held by the open-PR limit").Get-PrActivityfetches head commit time and thread resolution in one paginated GraphQL query (gh pr list --json commitsexceeds GitHub's node limit).The matching SKILL.md priority-list change went to
maindirectly (4d52aec).Ported from thomhurst/TUnit#7423. This repo has no
Test-IssuePrLoop.ps1; the script parses cleanly, and a throwaway harness that loads the survey functions passed the same scenarios as TUnit's tests (feedback wakes a pending PR, a loop reply or an older review does not, parked PRs don't count toward the limit, drafts do, a zero limit holds nothing).Summary by CodeRabbit