feat: implement issue #948 — P2 (#860 follow-up): runaway-PR detection in daily health check (commits/comments/cycles/age thresholds) - #1032
Conversation
…n in daily health check (commits/comments/cycles/age thresholds)
|
Warning Review limit reached
Next review available in: 2 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 (4)
📝 WalkthroughWalkthroughAdds runaway-PR detection: a new bash library computing threshold-based reasons (commits/comments/cycles/age-with-churn), a scan script enumerating open PRs and generating a markdown report, workflow steps to run the scan and file a GitHub issue when candidates are found, lint.yml registration of a new Bats test file, and comprehensive Bats test coverage. ChangesRunaway PR Detection
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow as daily-pr-review-health.yml
participant Scan as pr_runaway_scan.sh
participant GH as GitHub API
participant Lib as pr-runaway-detect.sh
Workflow->>Scan: run scan step
Scan->>GH: fetch open PR numbers
loop each open PR
Scan->>GH: fetch PR detail (commits, comments, created_at)
Scan->>Lib: pr_runaway_reasons(metrics)
Lib-->>Scan: reason lines
end
Scan->>Lib: generate_runaway_report(candidates)
Lib-->>Scan: markdown report
Scan->>Workflow: export HAS_RUNAWAY, RUNAWAY_COUNT
Workflow->>Workflow: create issue if HAS_RUNAWAY==true
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #1032 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a runaway PR detection system consisting of a detection library, a daily scan script, and a comprehensive BATS test suite. The reviewer feedback focuses on enhancing script robustness, portability, and performance. Key recommendations include ensuring the date command is portable across Linux and macOS, handling GitHub API and mktemp failures gracefully, using an EXIT trap for temporary file cleanup, optimizing jq usage to avoid multiple subshells, and utilizing setup-managed directories for temporary files in BATS tests.
There was a problem hiding this comment.
Pull request overview
Adds a “runaway PR” detection net to the daily PR review health check, flagging open PRs that exceed soft activity thresholds so novel runaways are surfaced quickly via the existing health-check issue channel.
Changes:
- Introduces a pure bash library for runaway-candidate detection and report rendering (
pr-runaway-detect.sh). - Adds a daily GH API scan script that evaluates all open PRs and emits a markdown report + env flags (
pr_runaway_scan.sh), wired into thedaily-pr-review-healthworkflow. - Adds Bats unit coverage for threshold boundaries, env overrides, and report rendering; hooks the test into the lint workflow.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_pr_runaway_detect.bats | New Bats unit tests for runaway detection logic and report rendering. |
| scripts/pr_runaway_scan.sh | New daily scanner script that gathers PR metrics, computes automation cycles, and emits a markdown report + workflow env flags. |
| scripts/lib/pr-runaway-detect.sh | New pure-function library implementing threshold logic, age calculation, and markdown report generation. |
| .github/workflows/lint.yml | Runs the new Bats test in CI. |
| .github/workflows/daily-pr-review-health.yml | Runs the scan daily and files a health-check issue when runaway candidates are detected. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/daily-pr-review-health.yml:
- Around line 116-137: The runaway issue creation step reads
`pr_runaway_report.md` directly into `github.rest.issues.create`, but it is not
size-limited like `pr_review_health_report.md`, so oversized reports can break
issue creation. Update the `Create runaway-candidate issue` script to truncate
or slice the report body to a safe limit before calling `issues.create`, using
the same pattern already applied in the health report path, and keep the
empty-report guard in place.
- Around line 104-137: The `Create runaway-candidate issue` step in the daily PR
review health workflow passes `pr_runaway_report.md` directly into
`github.rest.issues.create`, so it needs the same size cap used for
`pr_review_health_report.md`. Add a truncation guard in the
`actions/github-script` block before creating the issue, using the existing
report read logic (`fs.readFileSync`) and `issue` creation path, so oversized
runaway scan output is shortened to stay within GitHub’s issue-body limit.
In `@scripts/lib/pr-runaway-detect.sh`:
- Around line 1-2: The Bash script entrypoint in pr-runaway-detect.sh is missing
the required strict mode, so add the same set of shell safety flags used by
other scripts in this directory. Update the script header immediately after the
shebang to enable set -euo pipefail so the script is safe when run directly or
sourced without a parent already setting those options.
- Around line 108-115: `generate_runaway_report` is printing the raw threshold
env vars, which can differ from the values actually used by
`pr_runaway_reasons`. Update the report text to use the same normalized integer
thresholds produced by `_runaway_int` (or a shared helper) for
`RUNAWAY_MAX_COMMITS`, `RUNAWAY_MAX_COMMENTS`, `RUNAWAY_MAX_CYCLES`, and
`RUNAWAY_MIN_AGE_HOURS`, so the summary matches enforcement even when overrides
are invalid. Preserve the existing messaging in `generate_runaway_report` while
ensuring the displayed thresholds reflect the values `pr_runaway_reasons`
applies.
In `@scripts/pr_runaway_scan.sh`:
- Around line 59-64: The open PR listing in the PR scan script is swallowing
real `gh api` failures and treating them as “no open PRs.” Update the `open_prs`
collection in `scripts/pr_runaway_scan.sh` so failures from `gh api --paginate`
are not masked by `|| true`, and make `open_prs` handling distinguish an empty
result from a failed request; use the existing `open_prs` check and the `gh api`
invocation to surface an error and exit non-zero when listing PRs fails.
In `@tests/test_pr_runaway_detect.bats`:
- Around line 158-161: The RUNAWAY_MIN_AGE_HOURS override test still relies on
the default churn threshold, so make the setup explicit in the
pr_runaway_reasons test case. Update the test named "RUNAWAY_MIN_AGE_HOURS
override is respected" to set RUNAWAY_CHURN_MIN_CYCLES alongside
RUNAWAY_MIN_AGE_HOURS before calling pr_runaway_reasons, so the age-override
check is isolated from the default churn gate behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e997cd90-ac38-4764-a40b-3c58b94a90f7
📒 Files selected for processing (5)
.github/workflows/daily-pr-review-health.yml.github/workflows/lint.ymlscripts/lib/pr-runaway-detect.shscripts/pr_runaway_scan.shtests/test_pr_runaway_detect.bats
|
@coderabbitai resolve |
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
✅ Action performedComments resolved and changes approved. |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #1032 |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: b9a2a35ea728809eae63d13417e308b45fbb89b4
Review mode: triage-approved (single reviewer)
Summary
Adds a detection-only runaway-PR net (issue #948, #860 follow-up): a pure-function bash library (scripts/lib/pr-runaway-detect.sh) with env-overridable soft thresholds, a daily scan script (scripts/pr_runaway_scan.sh) wired into daily-pr-review-health.yml, and 22 bats unit tests. Triage assessment confirmed correct.
Linked issue analysis
Closes #948. All four acceptance criteria are substantively met: (1) the daily health check flags any open PR over thresholds with a direct link and the triggering metric(s) via a markdown table in the health-report issue; (2) all thresholds (RUNAWAY_MAX_COMMITS/COMMENTS/CYCLES, RUNAWAY_MIN_AGE_HOURS, RUNAWAY_CHURN_MIN_CYCLES) are env-overridable with the defaults specified in the issue (>50/>200/>10/>48h); (3) the age criterion requires active agent churn, with an explicit false-positive-guard test for a quiet aged PR and a normal-converging-PR test; (4) bats coverage pins boundary behaviour (strict >), overrides, multi-metric output, and defensive input degradation. Cycle counting reuses compute_pr_automation_cycles from the #926 budget lib so both guards agree on semantics. Detection only — no PR mutation, matching the issue requirement.
Findings
No blocking findings.
- Workflow change reuses established patterns from the same file: DON_PETRY_BOT_GH_PAT was already the GH_TOKEN for the existing health-check step, issue creation uses github.token under the existing issues: write permission, and the actions/github-script pin (3a2844b7e9c422d3c10d287c895573f7108da1b3) was verified via the GitHub API to match the v9.0.0 tag and is identical to the existing pin in this workflow.
- PR titles are sanitized (newlines/tabs/pipes stripped) before markdown-table interpolation — no report-injection vector.
- Threshold env parsing falls back to defaults (not 0) on bad overrides, preventing accidental flag-everything behaviour.
- All 18 review threads resolved; CodeRabbit's earlier changes-requested review was dismissed and superseded by its approval at the head SHA.
- Secret-scanning MCP tool unavailable in this run; gitleaks CI check passed. No secrets in the diff (only workflow secret references).
- Minor (non-blocking): the per-PR gather_pr_automation_events call makes the scan O(open PRs) in API requests; acceptable under the workflow's 20-minute timeout for this repo's scale.
CI status
All checks green at head SHA b9a2a35: shellcheck, bats, unit-tests, CodeQL (actions+python), SonarCloud, gitleaks secret scan, agent-shield, gh-aw-compile, permissions/stub-structure guards all SUCCESS; remaining checks SKIPPED (ecosystem-specific audits not applicable).
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
|
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #1032 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
Closes #948
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit
New Features
Tests