feat(pr-limits): wire the admission gate into dev-lead-fix-issue.sh (#1003) - #1004
Conversation
…1003) Phase 3 of the PR-limits initiative (epic petry-projects/.github#505): enforce the org-wide open-automation-PR cap at the source. Before opening a new PR, dev-lead now asks the shared admission gate whether the standing queue of open, non-draft automation PRs is at the signed-off cap and defers if so. - New helpers in scripts/dev-lead-fix-issue.sh: - _plg_fetch: pull the guard (scripts/lib/pr-limit-gate.sh) + config (standards/pr-limits.json) from the PUBLIC petry-projects/.github@main at runtime — single source of truth, not vendored. PLG_STANDARDS_DIR routes to a local checkout for tests/offline. - admission_gate_allows: source the guard, call plg_admission_gate "dev-lead". FAIL-OPEN on any fetch/source/unexpected-rc problem so a guard defect never wedges PR creation; rc==1 is the only real defer. - deferral_comment_exists / post_deferral_comment: one idempotent note (marker <!-- dev-lead-issue-deferred -->) so drain-retries never spam. - Call site sits AFTER the DEV_LEAD_DRY_RUN early-exit (a dry-run never defers) and BEFORE branch creation; on defer it opens nothing, leaves the issue labeled dev-lead for retry, posts the note, and exits 0. - Config authoritative in petry-projects/.github (cap signed off at 50, a single org-wide soft ceiling; ~44 baseline < 50 so nothing is refused today — bounds runaway growth only). Tests: new tests/dev-lead/unit/test_fix_issue_pr_limit_gate.bats (6 cases: under-cap opens PR, at/over-cap defers, single deferral comment, idempotent, dry-run-not-consulted, fetch-failure fail-open) — 6/6 pass; test_fix_issue.bats 16/16 (no regression); shellcheck clean; dev-lead-lint passes. Hand-authored after the dev-lead engine's own run implemented + validated the same design but hit the 1800s CI timeout before it could push (#1003). Closes #1003 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 49 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 (2)
📝 WalkthroughWalkthroughAdds an org-wide PR-admission gate to ChangesAdmission Gate Enforcement
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
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 |
|
@donpetry-bot review |
|
@don-petry I'm on it — starting a fresh review now. Results will appear in a few minutes. |
![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 implements a PR-limit admission gate in dev-lead-fix-issue.sh to defer creating new automation PRs when the org-wide cap is reached, accompanied by a comprehensive suite of BATS unit tests. The code review feedback focuses on enhancing script robustness and error handling. Key recommendations include explicitly handling mktemp -d failures where set -e is suspended, validating positional parameters to avoid unbound variable errors under set -u, adding a fallback for null .body values in jq queries, and verifying the presence of the gh CLI. Additionally, minor improvements were suggested for the tests, such as using here-strings and guarding grep -c commands against non-zero exit codes.
There was a problem hiding this comment.
Pull request overview
This PR wires the org-wide PR-limit admission gate into dev-lead-fix-issue.sh so dev-lead defers opening new automation PRs when the shared open-PR cap is reached, while remaining fail-open on gate/fetch issues.
Changes:
- Added runtime fetch + sourcing of the shared PR-limit guard and config, and a pre-branch admission check that can defer PR creation.
- Implemented an idempotent “deferred” issue comment path keyed by a marker to avoid retry spam.
- Added unit tests covering allow/defer, idempotency, dry-run behavior, and fail-open on fetch failures.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/dev-lead-fix-issue.sh | Fetches and consults the PR-limit admission gate before branch/PR creation; posts an idempotent deferral comment on defer. |
| tests/dev-lead/unit/test_fix_issue_pr_limit_gate.bats | Adds Bats unit coverage for the new gate + deferral behavior with local fixtures/stubs. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@scripts/dev-lead-fix-issue.sh`:
- Around line 247-248: The deferral-comment count in the issue-comments lookup
only checks the first page, which can miss an existing marker and cause
duplicate defer comments. Update the comment-counting logic in the relevant gh
api call to paginate through all issue comments when searching for
PLG_DEFER_MARKER, using the existing count/lookup flow in the script so it
considers every page before deciding whether to post another deferral comment.
In `@tests/dev-lead/unit/test_fix_issue_pr_limit_gate.bats`:
- Line 29: The test setup is inheriting PRIOR_COMMENTS_JSON from the caller’s
environment, which makes the bats tests non-hermetic. Update the setup in
test_fix_issue_pr_limit_gate.bats so PRIOR_COMMENTS_JSON is explicitly reset for
each test run instead of using any existing value, keeping the defer/comment
checks isolated and deterministic.
- Around line 114-118: The fake gate in plg_admission_gate should assert the
production contract instead of always failing open. Update the test fixture so
it verifies the caller passes source "dev-lead" and a readable PR_LIMITS_CONFIG
path, and make it fail the test when those inputs are missing or unreadable.
Keep the behavior tied to plg_admission_gate in the generated pr-limit-gate.sh
fixture so the tests catch regressions if production stops providing the
expected arguments.
🪄 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: fb2e8b2a-3b34-4697-b111-40024b7ac644
📒 Files selected for processing (2)
scripts/dev-lead-fix-issue.shtests/dev-lead/unit/test_fix_issue_pr_limit_gate.bats
|
CI checks on this PR are still running. Once they complete, re-mention Posted by the donpetry-bot PR-review cascade. |
|
@donpetry-bot I'm on it — starting a fresh review now. Results will appear in a few minutes. |
|
@coderabbitai resolve |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #1004 |
|
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. |
✅ Action performedComments resolved and changes approved. |
- admission_gate_allows: fail open if `mktemp -d` fails (set -e is suspended in the `if !` caller context, so an unchecked failure would continue with an empty tmp dir). [Gemini HIGH] - _plg_fetch: guard `[ "$#" -lt 2 ] && return 1` so set -u can't crash on a missing arg. [Gemini] - deferral_comment_exists: paginate the issue-comments lookup (`gh api --paginate ... | jq -s 'add // 0'`) so a marker on a later page is not missed (which would post a duplicate defer comment), and guard null bodies with `(.body // "")`. [CodeRabbit MAJOR + Gemini] - tests: reset PRIOR_COMMENTS_JSON unconditionally (hermetic); the fake guard now asserts the production contract (source == "dev-lead", readable PR_LIMITS_CONFIG) so a regression can't pass via fail-open; paginate-tolerant gh stub; `grep -c ... || true`. [CodeRabbit] Skipped Gemini's "add command -v gh to main()" as out of scope — the script and engine already assume gh throughout. shellcheck clean; dev-lead-lint passes; new bats 6/6; test_fix_issue.bats 16/16. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUhVsBbihbTjVvbcVf1WuU
|
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
|
@donpetry-bot review |
|
@don-petry I'm on it — starting a fresh review now. Results will appear in a few minutes. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: a42e5f12797f79c4155cc719eb89b40b834de988
Cascade: triage → deep (triage: haiku 4.5 → deep: opus 4.8 + duck: o4-mini → audit: fable 5)
Summary
Phase 3 of the PR-limits epic wires an org-wide admission gate into dev-lead-fix-issue.sh (118 lines) with a 226-line BATS suite (6 cases). The triage escalation is stale: it reviewed commit a42e5f1, but the author then pushed 8fbe0c2 ('harden gate per advisory review') which resolves every Gemini finding — the HIGH mktemp -d failure now fails open explicitly, _plg_fetch validates arity under set -u, and the comments jq query is null-safe via (.body // ""); the low test nits (grep -c || true, here-string) are also fixed. Remaining items are non-blocking: the suggested command -v gh check was not added (gh is guaranteed in the Actions runner and already used throughout), and the gate sources a guard fetched at runtime from the PUBLIC petry-projects/.github@main — a deliberate single-source-of-truth design, first-party, defensively guarded (declare -F) and fail-open, so it does not expand the org trust boundary. No secrets/auth/crypto/DB changes; no injection (PLG_DEFER_MARKER is a hardcoded constant). Downstream impact: (none). Completed CI checks are green (gitleaks, Lint, guards); several checks were still in progress at review time. run_secret_scanning MCP was not available; no secrets present in the diff regardless.
Findings
- INFO: Triage escalation signals are stale: they describe commit a42e5f1, but head advanced to 8fbe0c2 ('fix(pr-limits): harden gate per advisory review') which addresses the HIGH mktemp finding and the MEDIUM _plg_fetch/jq-null findings. Verified directly in the current diff.
- MINOR: admission_gate_allows fetches scripts/lib/pr-limit-gate.sh from the PUBLIC petry-projects/.github and
sources it at runtime, pinned to the mutable ref 'main'. This is a deliberate, documented single-source-of-truth design (epic #505) from a first-party org repo, guarded by declare -F and fully fail-open, so it does not expand the trust boundary beyond the org. Noting the runtime code-execution path for awareness; not blocking. - MINOR: Gemini's suggested
command -v ghpresence check was not added. Non-blocking: gh is guaranteed present in the GitHub Actions runner and is already relied upon throughout the existing script (check_existing_pr, etc.). - INFO: At review time several CI checks were still in progress/queued (unit-tests, SonarCloud, CodeQL, dev-lead test matrix). Completed checks are green (Secret scan/gitleaks, Lint, Holdout Guard, Test-Deletion Guard). Approval assumes the in-progress checks complete green as they did on the prior head.
Reviewed by the PR-review cascade (triage: haiku 4.5 → deep: opus 4.8 + duck: o4-mini → audit: fable 5). Reply if you need a human review.
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 8fbe0c258d0009ceee40aae79df317044989c9d1
Review mode: triage-approved (single reviewer)
Summary
Phase 3 of the PR-limits epic: wires an org-wide open-automation-PR admission gate into scripts/dev-lead-fix-issue.sh. Before opening a PR it fetches the shared guard (scripts/lib/pr-limit-gate.sh) + config (standards/pr-limits.json) from the public petry-projects/.github@main, calls plg_admission_gate "dev-lead", and defers (idempotent single comment, issue left labeled dev-lead) when the queue is at the signed-off cap. FAIL-OPEN on any fetch/source/unexpected-rc problem. Gate sits after the dry-run early-exit and before branch creation. +344/-0 across 2 files (script + new 6-case bats suite).
Linked issue analysis
Closes #1003. The dev-lead engine's own run implemented and validated this identical design but hit the 1800s CI timeout before it could push; this PR hand-authors the same gate. Issue substantively addressed — the admission gate is implemented, tested (6/6), and shellcheck-clean, with no regression in test_fix_issue.bats (16/16).
Findings
No blocking findings.
- Fail-open logic is sound: mktemp/guard-fetch/config-fetch/source failures and any unexpected guard rc all allow PR creation; only rc==1 defers. set -e is respected via
if ! sourceand... || rc=$?capture, anddeclare -Fconfirms the entry point before calling it. - Idempotent deferral: deferral_comment_exists paginates the comments API and is null-safe/non-numeric-safe, so a marker on a later page cannot cause a duplicate defer comment.
- Advisory (gemini, line 186 — validate positional params under set -u): ALREADY RESOLVED — _plg_fetch opens with
[ "$#" -lt 2 ] && return 1. - Advisory (gemini, line 279 —
command -v ghpreflight): non-blocking. gh is guaranteed present in the Actions runner and the entire script already depends on it, so a missing-gh check here adds no real safety. - Observation (not blocking): the guard is fetched at runtime from the public petry-projects/.github@main and sourced. This is a deliberate, signed-off single-source-of-truth pattern shared by all automation callers (not vendored), over the authenticated gh API against an org-controlled repo — an accepted trust relationship, not new untrusted-input execution. All security scans (CodeQL, Agent Security Scan, agent-shield, gitleaks, SonarCloud) passed.
- Secret scan: the run_secret_scanning MCP tool is not exposed in this environment; the gitleaks CI check covers secret scanning and passed. No secrets in the diff.
CI status
All substantive checks green: shellcheck, ShellCheck, bats, unit / unit-tests, CodeQL, Analyze (actions/python), Secret scan (gitleaks), SonarCloud, Agent Security Scan, agent-shield, holdout-guard, validate-fixtures, template-drift, prompt-coverage, caller/toplevel-permissions. The several CANCELLED dev-lead/review entries are superseded concurrency runs; one review / review is IN_PROGRESS (this review job). SonarCloud Quality Gate passed with 0 new issues.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Phase 3 of the PR-limits initiative (epic petry-projects/.github#505): enforce the org-wide open-automation-PR cap at the source.
What
Before opening a new automation PR,
dev-lead-fix-issue.shnow asks the shared admission gate whether the standing queue of open non-draft automation PRs is at the signed-off cap, and defers if so._plg_fetch— pulls the guard (scripts/lib/pr-limit-gate.sh) + config (standards/pr-limits.json) from the publicpetry-projects/.github@mainat runtime → single source of truth, not vendored.PLG_STANDARDS_DIRroutes to a local checkout for tests/offline.admission_gate_allows— sources the guard, callsplg_admission_gate "dev-lead". FAIL-OPEN on any fetch/source/unexpected-rc problem so a guard defect never wedges PR creation;rc==1is the only real defer.<!-- dev-lead-issue-deferred -->); drain-retries never spam.DEV_LEAD_DRY_RUNearly-exit (a dry-run never defers) and BEFORE branch creation; on defer it opens nothing, leaves the issue labeleddev-lead, posts the note, exits 0.Behavior today
Config is authoritative in
petry-projects/.github— cap signed off at 50, a single org-wide soft ceiling. Baseline is ~44 open automation PRs, so nothing is refused today; this only bounds runaway growth.Tests
New
tests/dev-lead/unit/test_fix_issue_pr_limit_gate.bats(6 cases: under-cap opens PR, at/over-cap defers, single deferral comment, idempotent, dry-run-not-consulted, fetch-failure fail-open) — 6/6 pass;test_fix_issue.bats16/16 (no regression); shellcheck clean;dev-lead-lintpasses.Note
Hand-authored after the dev-lead engine's own run implemented + validated the identical design but hit the 1800s CI timeout before it could push (see #1003). Closes #1003.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes