[#850] harden pr-auto-review catch-up sweep (starvation ordering, pagination, error handling)

Follow-up robustness findings on the #868/#869 catch-up sweep (.github/scripts/pr-auto-review/), non-blocking for small convergences but worth fixing:

Cap-before-readiness starvation (codeant): MAX_PER_RUN is applied to candidates before readiness is checked, so a run of older non-ready PRs can consume all slots and starve newer ready ones. Fix: cap on the number dispatched (ready), not considered.
No pagination (codeant): the labeled-PR search is hard-limited to 100 with no pagination; PRs beyond page 1 are never swept.
Silent search failure no-ops (qodo): a failed gh search/API call silently yields an empty set — the sweep should surface the error, not treat it as 'nothing to do'.
jq parse can abort the sweep (qodo): guard the jq/parse step so one malformed payload doesn't abort the whole run.
Refs: [#850] pr-auto-review dispatch strands PRs under bulk convergence — add a catch-up sweep + churn tolerance #868, feat: implement issue #868 — [#850] pr-auto-review dispatch strands PRs under bulk convergence — add a catch-up sweep + churn tolerance #869, Epic Epic: Fully-automated fleet-wide version+channel shim compliance (no manual deploy / admin-merge) #850.

Dev-Lead Implementation Plan

Issue: #872 — [#850] harden pr-auto-review catch-up sweep (starvation ordering, pagination, error handling)

Discrepancy note (read first)

The #868/#869 catch-up sweep this issue asks to harden is not present in the repo: .github/scripts/pr-auto-review/ contains only README.md and lib/ready-check.sh, and there is no MAX_PER_RUN, labeled-PR search, or gh search sweep anywhere in the tree. The four findings fully specify the required behavior, so I'm implementing the catch-up sweep to spec, with all four robustness properties designed in from the start, following the repo's existing ready-check.sh pattern (pure, unit-tested lib/ core + thin I/O orchestrator + bats + README contract).

Scope

Add a catch-up sweep for pr-auto-review that re-evaluates open labeled PRs and dispatches the review agent for ready ones — capping on the number dispatched (not considered), paginating the search fully, surfacing search/API failures instead of no-oping, and guarding the JSON parse so one malformed payload cannot abort the run.

Implementation Checklist

.github/scripts/pr-auto-review/lib/sweep.sh — pure, side-effect-free decision core:
- pr_auto_review_sweep_order — oldest-first (FIFO) candidate ordering so ready PRs are never perpetually starved
- pr_auto_review_sweep_merge_pages / pr_auto_review_sweep_page_full — full pagination (no 100-item cliff)
- pr_auto_review_sweep_valid_search — distinguishes a valid empty result from a failed search (surface, don't no-op)
- pr_auto_review_sweep_extract — guarded parse: malformed payload returns non-zero instead of aborting under set -e
- pr_auto_review_sweep_plan — cap on the count dispatched (ready), skipping non-ready candidates without consuming a slot
.github/scripts/pr-auto-review/sweep.sh — thin gh I/O orchestrator wiring the pure core + ready-check.sh
.github/scripts/pr-auto-review/README.md — document the sweep contract
.github/workflows/pr-auto-review-tests.yml — add the new scripts to the shellcheck step

Tests to Write

test/workflows/pr-auto-review/sweep.bats — cap-on-dispatched anti-starvation, ordering, page merge/dedupe, page-full predicate, valid-vs-failed search, guarded parse of malformed payload

Test Command

bats --print-output-on-failure test/workflows/pr-auto-review/ (plus bash .dev-lead/scripts/dev-lead-lint.sh)

Dev-Lead: Implementation Complete

Important context — the sweep did not exist yet

Issue #872 asked to harden the #868/#869 catch-up sweep, but that sweep was absent from the repo (only lib/ready-check.sh existed under .github/scripts/pr-auto-review/; no MAX_PER_RUN, labeled-PR search, or gh search anywhere; git history has a single commit). The four findings fully specify the required behaviour, so I implemented the catch-up sweep to spec, with all four robustness properties designed in from the start, following the repo's existing ready-check.sh pattern (pure unit-tested lib/ core + thin gh I/O orchestrator + bats + README). This was flagged in the Phase 1 plan comment above.

Plan Execution

Pure decision core lib/sweep.sh (order, page_full, merge_pages, valid_search, extract, plan)
Thin orchestrator sweep.sh wiring the pure core + ready-check.sh to gh
sweep.bats — 22 unit tests (TDD: red → green)
README contract + shellcheck wiring in pr-auto-review-tests.yml
Full suite (77 bats) + dev-lead-lint.sh + shellcheck all green
End-to-end smoke tests with a stub gh (happy path, search-failure surfacing, anti-starvation cap)

How each #872 finding is addressed

Cap-before-readiness starvation — readiness is evaluated for every candidate first; pr_auto_review_sweep_plan caps on the count dispatched (ready), so non-ready PRs consume no slot. Verified end-to-end: with cap=1 and two older draft PRs ahead of two ready ones, the oldest ready PR is dispatched (the drafts do not eat the slot).
No pagination — /search/issues is paged while pr_auto_review_sweep_page_full reports a full page; pages merged/deduped via pr_auto_review_sweep_merge_pages.
Silent search failure — a non-zero gh exit or a payload pr_auto_review_sweep_valid_search rejects aborts the run with an error instead of yielding an empty set. Verified: a rate-limit failure exits non-zero.
jq parse abort — pr_auto_review_sweep_extract catches a jq failure and returns non-zero rather than aborting mid-pipe under set -e.
Plus deterministic oldest-first ordering (pr_auto_review_sweep_order) so the longest-waiting ready PR is never perpetually starved.

Test Results

1..22
ok 1 plan: older non-ready candidates do not consume dispatch slots (anti-starvation)
ok 2 plan: dispatches at most MAX ready PRs
ok 3 plan: fewer ready than MAX dispatches all ready
ok 4 plan: no ready candidates dispatches nothing
ok 5 plan: MAX of 0 dispatches nothing
ok 6 plan: empty candidate array dispatches nothing
ok 7 order: sorts candidates oldest updatedAt first
ok 8 order: ties on updatedAt break by number ascending
ok 9 page_full: a full page signals more pages may exist (exit 0)
ok 10 page_full: a short page signals the last page (exit 1)
ok 11 page_full: an empty page signals the last page (exit 1)
ok 12 merge_pages: concatenated pages merge and dedupe by number
ok 13 merge_pages: a single page passes through unchanged in count
ok 14 valid_search: a valid (even empty) JSON array is accepted
ok 15 valid_search: an API error object is rejected (surface, do not no-op)
ok 16 valid_search: an empty string is rejected
ok 17 valid_search: malformed JSON is rejected
ok 18 extract: a well-formed search payload yields number+updatedAt candidates
ok 19 extract: accepts the REST search field name updated_at
ok 20 extract: a malformed payload returns non-zero instead of aborting
ok 21 extract: a non-array payload returns non-zero
ok 22 extract: guarded call under set -e lets the caller continue past a bad payload

(Full suite: bats test/workflows/pr-auto-review/ → 77 passing; dev-lead-lint.sh → all checks passed; shellcheck -x on both new scripts → clean.)

Files Changed

.github/scripts/pr-auto-review/lib/sweep.sh (new): pure, side-effect-free sweep decision core
.github/scripts/pr-auto-review/sweep.sh (new): thin gh I/O orchestrator
test/workflows/pr-auto-review/sweep.bats (new): 22 unit tests for the four findings + ordering
.github/scripts/pr-auto-review/README.md: documents the sweep contract and robustness properties
.github/workflows/pr-auto-review-tests.yml: shellchecks the two new scripts

Notes

The sweep has no scheduled workflow entry point yet — it is a runnable script (REPO=… bash .github/scripts/pr-auto-review/sweep.sh, honours DRY_RUN, SWEEP_LABEL, MAX_PER_RUN). Wiring a schedule: workflow that calls it is a natural follow-up but was out of scope for these robustness findings.
Per-PR fact-gathering is fail-closed: a gh error during readiness evaluation aborts the run (never scores a PR ready on incomplete data), matching the reusable workflow's semantics.

docs(ci-standards): feature-ideation backlog enhancement (backfill) +…

Navigation Menu

Repository navigation

[#850] harden pr-auto-review catch-up sweep (starvation ordering, pagination, error handling) #872

Dev-Lead Implementation Plan

Discrepancy note (read first)

Scope

Implementation Checklist

Tests to Write

Test Command

Dev-Lead: Implementation Complete

Important context — the sweep did not exist yet

Plan Execution

How each #872 finding is addressed

Test Results

Files Changed

Notes

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[#850] harden pr-auto-review catch-up sweep (starvation ordering, pagination, error handling) #872

Description

Activity

don-petry commented on Jul 22, 2026

Dev-Lead Implementation Plan

Discrepancy note (read first)

Scope

Implementation Checklist

Tests to Write

Test Command

don-petry commented on Jul 22, 2026

Dev-Lead: Implementation Complete

Important context — the sweep did not exist yet

Plan Execution

How each #872 finding is addressed

Test Results

Files Changed

Notes

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions