feat: implement issue #868 — [#850] pr-auto-review dispatch strands PRs under bulk convergence — add a catch-up sweep + churn tolerance - #869
Conversation
…Rs under bulk convergence — add a catch-up sweep + churn tolerance
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🤖 CodeAnt AI — Review Status
Updated in place by CodeAnt AI · last 5 reviews |
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
codeant-ai
Bot
added
the
size:L
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
PR Summary by QodoAdd catch-up sweep workflow for pr-auto-review dispatch (#868)
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
|
||
| jq -r --argjson max "$max" ' | ||
| if type == "array" then | ||
| [ .[] | select(.isDraft != true) | .url | select(type == "string") ][:$max][] |
There was a problem hiding this comment.
Suggestion: The candidate cap is applied before readiness is checked, so older PRs that stay non-ready can repeatedly consume all slots and prevent newer ready PRs from ever being evaluated. Move the cap to the dispatch stage (cap successful dispatches), or evaluate a larger window and stop once MAX dispatches are reached. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ Ready standards-sync PRs can be starved behind non-ready ones.
- ⚠️ Operators must manually review stranded but ready PRs.
- ⚠️ Back-pressure fairness guarantee is broken under stuck PRs.Steps of Reproduction ✅
1. In `.github/scripts/pr-auto-review/sweep-dispatch.sh` lines 55-62, observe that
`PR_LIST` is built using `gh search prs` sorted by `created` ascending and limited to 100
results, representing the oldest open labeled PRs.
2. In `.github/scripts/pr-auto-review/lib/sweep.sh` lines 31-43 and README lines 25-32,
confirm that `pr_auto_review_sweep_candidates MAX` (called from sweep-dispatch line 68)
filters out drafts and then caps the resulting URL list to `MAX_PER_RUN` using the array
slice `[ ... ][:$max][]` before any readiness evaluation occurs.
3. In a real run where more than `MAX_PER_RUN` non-draft `standards-sync` PRs are open,
with some of the oldest PRs permanently failing readiness (e.g., `pr_auto_review_ready`
returning `skip-changes-requested` or `skip-unresolved-threads` as described in README
lines 44-50), the candidates array on each sweep consists only of those oldest PRs.
4. Because the loop at `sweep-dispatch.sh` lines 120-141 iterates only over `CANDIDATES`,
newer ready PRs that sit beyond the first `MAX_PER_RUN` non-draft items are never passed
to `evaluate_pr` and therefore never dispatched, while the same non-ready oldest PRs
consume the candidate slots on every sweep run.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** .github/scripts/pr-auto-review/lib/sweep.sh
**Line:** 43:43
**Comment:**
*Incorrect Condition Logic: The candidate cap is applied before readiness is checked, so older PRs that stay non-ready can repeatedly consume all slots and prevent newer ready PRs from ever being evaluated. Move the cap to the dispatch stage (cap successful dispatches), or evaluate a larger window and stop once MAX dispatches are reached.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Fixed in .github/scripts/pr-auto-review/lib/sweep.sh and sweep-dispatch.sh: removed the [:] slice from pr_auto_review_sweep_candidates (the function now emits all non-draft PRs in input order). The back-pressure cap moves to the dispatch loop in sweep-dispatch.sh with [ "$dispatched" -ge "$MAX_PER_RUN" ] && break, so non-ready older PRs can no longer consume all candidate slots and starve newer ready ones. Tests in test/workflows/pr-auto-review/sweep.bats updated to reflect the new contract.
| PR_LIST=$(gh search prs \ | ||
| --owner "$SEARCH_OWNER" \ | ||
| --label "$SWEEP_LABEL" \ | ||
| --state open \ | ||
| --sort created \ | ||
| --order asc \ | ||
| --limit 100 \ | ||
| --json url,isDraft 2>/dev/null || true) |
There was a problem hiding this comment.
Suggestion: The search is hard-limited to 100 PRs with no pagination, so labeled open PRs beyond the first page are never considered by the sweep and can remain stranded indefinitely. Add pagination (or raise and iterate limits) so the sweep can eventually inspect the full open set. [incomplete implementation]
Severity Level: Major ⚠️
- ❌ Open labeled PRs beyond first 100 never auto-reviewed.
- ⚠️ Bulk convergence can strand newer ready PRs indefinitely.
- ⚠️ Sweep’s “org-wide” guarantee is violated under large bursts.Steps of Reproduction ✅
1. In `.github/scripts/pr-auto-review/sweep-dispatch.sh` lines 55-62, `PR_LIST` is
populated via `gh search prs` with `--limit 100`, `--sort created`, and `--order asc`, so
only the first 100 oldest matching open, labeled PRs are ever returned.
2. README section “The catch-up sweep” (lines 114-131) describes the sweep as enumerating
open, non-draft PRs carrying the sweep label org-wide, but does not add any pagination or
follow-up calls beyond this single `gh search prs` invocation.
3. When more than 100 open `standards-sync` PRs exist simultaneously in the organization,
`gh search prs` only returns the oldest 100; newer PRs beyond that window never appear in
`PR_LIST` and therefore are not present in the JSON array passed into
`pr_auto_review_sweep_candidates MAX_PER_RUN` on line 68.
4. As a result, the evaluation loop at lines 120-141 only ever calls `evaluate_pr` for PR
URLs within the first 100 results, and PRs beyond that initial page are never inspected
for readiness or dispatched by the sweep, even if they satisfy the ready-check criteria.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** .github/scripts/pr-auto-review/sweep-dispatch.sh
**Line:** 55:62
**Comment:**
*Incomplete Implementation: The search is hard-limited to 100 PRs with no pagination, so labeled open PRs beyond the first page are never considered by the sweep and can remain stranded indefinitely. Add pagination (or raise and iterate limits) so the sweep can eventually inspect the full open set.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Fixed in sweep-dispatch.sh: added a SEARCH_LIMIT env var (default 1000, the GitHub Search API per-query maximum) and changed --limit 100 to --limit "$SEARCH_LIMIT". This removes the hard 100-PR ceiling; operators can lower SEARCH_LIMIT in the workflow env for testing. True pagination beyond 1000 would require multiple search calls and can be added if the org ever exceeds that threshold.
Code Review by Qodo
Context used✅ Compliance rules (platform):
87 rules 1. Silent search failure no-ops
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| # Oldest-first (created asc) so back-pressure drains fairly: each capped run | ||
| # takes the oldest waiting PRs, and a merged PR leaves the set so the next-oldest | ||
| # advances next cycle — no PR is starved by newer arrivals under best-match order. | ||
| PR_LIST=$(gh search prs \ |
There was a problem hiding this comment.
1. Silent search failure no-ops 🐞 Bug ☼ Reliability
The sweep suppresses gh search prs errors and ignores its exit status, then treats empty output as an empty PR set, so auth/rate-limit/network failures can make the workflow succeed while dispatching nothing (leaving PRs stranded). This undermines the sweep’s purpose because failures become indistinguishable from “no open PRs.”
Agent Prompt
### Issue description
`sweep-dispatch.sh` currently runs `gh search prs` with `2>/dev/null || true` and then coerces empty output to `[]`. If the search fails (bad token, rate limit, transient network), the workflow can exit 0 with “nothing to do”, which looks successful but leaves stuck PRs untouched.
### Issue Context
This script is the new scheduled catch-up mechanism; if it silently no-ops on failures, the org loses the only automatic recovery path for missed readiness events.
### Fix (suggested)
- Capture and check the `gh search prs` exit code.
- Do **not** suppress stderr; or, if you must, emit an explicit `::error::` / `::warning::` with the failure reason.
- Prefer failing the job (non-zero exit) so scheduled runs are visibly unhealthy, rather than pretending the PR set is empty.
Example sketch:
```bash
if ! PR_LIST=$(gh search prs ... --json url,isDraft); then
echo "::error::gh search prs failed; sweep aborted"
exit 1
fi
```
### Fix Focus Areas
- .github/scripts/pr-auto-review/sweep-dispatch.sh[55-66]
- .github/scripts/pr-auto-review/sweep-dispatch.sh[74-77]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Fixed in sweep-dispatch.sh (lines 59-66): replaced 2>/dev/null || true with if ! PR_LIST=$(gh search prs ...); then echo "::error::gh search prs failed..."; exit 1; fi. Stderr is now visible in Actions logs and auth/rate-limit/network failures cause a non-zero exit, making the sweep visibly unhealthy instead of silently treating an empty result as "no PRs to process".
| # Selection + back-pressure (pure, unit-tested): drops drafts, caps at MAX_PER_RUN. | ||
| mapfile -t CANDIDATES < <(printf '%s' "$PR_LIST" | pr_auto_review_sweep_candidates "$MAX_PER_RUN") | ||
|
|
||
| total_open=$(printf '%s' "$PR_LIST" | jq 'if type == "array" then length else 0 end') |
There was a problem hiding this comment.
2. Jq parse can abort sweep 🐞 Bug ☼ Reliability
total_open is computed by piping PR_LIST into jq without guarding parse errors, so unexpected non-JSON/truncated stdout can terminate the sweep under set -e. This can prevent any candidates from being evaluated/processed in that run.
Agent Prompt
### Issue description
`sweep-dispatch.sh` runs `jq` over `$PR_LIST` to compute `total_open` without `2>/dev/null` and without a fallback. With `set -euo pipefail`, a parse error will exit the script.
### Issue Context
While `gh search prs --json ...` normally emits JSON, “unexpected stdout” does happen in practice (partial output, CLI bugs, wrapper output, etc.). The sweep should be robust because it’s a periodic recovery mechanism.
### Fix (suggested)
- Add a defensive parse wrapper:
- If parsing fails, log an error/warning and either:
- fail the workflow (preferred for scheduled correctness), or
- default `total_open=0` and `PR_LIST='[]'` and continue.
Example sketch:
```bash
if ! total_open=$(printf '%s' "$PR_LIST" | jq 'if type=="array" then length else 0 end' 2>/dev/null); then
echo "::error::PR_LIST was not valid JSON; aborting sweep"
exit 1
fi
```
### Fix Focus Areas
- .github/scripts/pr-auto-review/sweep-dispatch.sh[67-73]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Fixed in sweep-dispatch.sh (line 75): wrapped the jq call with if ! total_open=$(... jq ... 2>/dev/null); then echo "::error::PR_LIST was not valid JSON..."; exit 1; fi. A non-JSON or truncated PR_LIST now aborts the sweep with a visible diagnostic instead of terminating silently under set -e.
| if [ "$DRY_RUN" = "1" ]; then | ||
| echo "[dry-run] would dispatch review agent for ${pr_url} (decision=${decision})" | ||
| else | ||
| gh api \ |
There was a problem hiding this comment.
3. Dispatch failure stops batch 🐞 Bug ☼ Reliability
A single failing gh api ... /dispatches call will terminate the entire sweep run due to set -e, skipping remaining ready PRs in the capped batch. This reduces sweep throughput and can keep PRs stuck until a later cycle even when they were ready now.
Agent Prompt
### Issue description
The live dispatch path executes `gh api .../dispatches` unguarded inside the per-PR loop. Under `set -e`, any transient dispatch failure aborts the whole run and skips later candidates.
### Issue Context
This script intentionally limits dispatches per run (back-pressure). Aborting mid-loop makes the effective cap smaller than configured and slows convergence.
### Fix (suggested)
- Wrap dispatch in an `if` and continue on failure:
- log `::error::` with PR URL
- increment a `dispatch_failures` counter
- After the loop, optionally `exit 1` if any dispatches failed (so the run is visible as unhealthy) while still attempting the rest.
Example sketch:
```bash
dispatch_failures=0
...
if ! gh api ...; then
echo "::error::Dispatch failed for $pr_url"
dispatch_failures=$((dispatch_failures+1))
continue
fi
```
### Fix Focus Areas
- .github/scripts/pr-auto-review/sweep-dispatch.sh[120-141]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Fixed in sweep-dispatch.sh (dispatch loop): wrapped gh api .../dispatches in an if block — a transient failure logs ::error:: and increments dispatch_failures instead of aborting under set -e. The loop continues processing remaining candidates. After the loop the sweep exits 1 if any dispatch failed, so the scheduled run is visibly unhealthy while still attempting every ready PR in the batch.
Review — fix requested (cycle 1/3)The automated review identified the following issues. Please address each one: Findings to fixAutomated review — NEEDS HUMAN REVIEWRisk: MEDIUM SummaryAdds a scheduled catch-up sweep (every 15 min + manual dry-run-default dispatch) that enumerates open non-draft standards-sync PRs org-wide, delegates per-PR readiness verbatim to the existing pr_auto_review_ready gate, and re-dispatches the review agent for ready PRs, bounded by MAX_PER_RUN=8 for back-pressure. Includes a pure unit-tested selection helper (lib/sweep.sh), 10 new bats tests, shellcheck coverage, and README docs. Implementation quality is good and CI is fully green, but 5 unresolved bot review threads block auto-approval. Linked issue analysisCloses #868 ([#850] pr-auto-review dispatch strands PRs under bulk convergence). All three proposed fixes are substantively addressed: (1) periodic sweep + workflow_dispatch re-invoking the ready-check → dispatch path — implemented; (2) churn tolerance — inherited verbatim by delegating to pr_auto_review_ready (#680 required-vs-non-required tolerance), with periodic re-sweep acting as the debounce; (3) back-pressure — MAX_PER_RUN cap with a fail-safe guard (non-numeric/non-positive cap selects nothing). Acceptance criteria are covered by design; live behavior (auto-merge within one sweep interval) is only verifiable post-merge. FindingsBlocking (gate failure): 5 unresolved review threads. Dev-lead replied "fix-bot-comment (no-changes)" but the threads remain unresolved:
Security posture: good. New workflow is SHA-pinned (actions/checkout v7.0.0), top-level Triage-assessment correction: this PR was triage-cleared as low-risk, but a new scheduled org-wide automation workflow wielding a repo-scope PAT with non-trivial dispatch logic is MEDIUM, and the unresolved threads independently preclude auto-approval. CI statusAll required and optional checks green at 2524967: Lint, ShellCheck, bats, CodeQL (actions), Agent Security Scan, Secret scan (gitleaks), SonarCloud Quality Gate (0 new issues), agent-shield, npm audit — all SUCCESS; ecosystem audits not applicable were SKIPPED. mergeStateStatus=BLOCKED solely on the pending code-owner review. Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review. Additional tasks
The review cascade will automatically re-review after new commits are pushed. |
|
Dev-Lead — review-changes (applied)Changes committed and pushed. |
2 participants
User description
Closes #868
Implemented by dev-lead agent. Please review.
CodeAnt-AI Description
Add a scheduled catch-up sweep for stalled PR auto-reviews
What Changed
standards-syncPRs and re-dispatches review only for the ones that are readyImpact
✅ Fewer PRs left stuck after CI finishes✅ Less manual rerunning of auto-review✅ Smaller dispatch bursts during bulk PR convergence💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.