Skip to content

feat: implement issue #399 — PR Review Agent — failures detected 2026-05-27 - #487

Merged
don-petry merged 11 commits into
mainfrom
dev-lead/issue-399-20260608-0048
Jun 8, 2026
Merged

don-petry merged 11 commits into
mainfrom
dev-lead/issue-399-20260608-0048

Conversation

@don-petry

Copy link
Copy Markdown
Collaborator

Closes #399

Implemented by dev-lead agent. Please review.

@don-petry
don-petry requested a review from a team as a code owner June 8, 2026 01:02
Copilot AI review requested due to automatic review settings June 8, 2026 01:02
@coderabbitai

coderabbitai Bot commented Jun 8, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@don-petry, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 39 minutes and 13 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c87e9828-da51-48c8-8daf-4430aea68e14

📥 Commits

Reviewing files that changed from the base of the PR and between a6c33c3 and c42552c.

📒 Files selected for processing (8)
  • scripts/engine.sh
  • scripts/review-batch.sh
  • scripts/review-one-pr.sh
  • scripts/validate-engines.sh
  • tests/test-validate-engines.sh
  • tests/test_batch_fallback.bats
  • tests/test_rate_limit.bats
  • tests/test_validate_engines.bats
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev-lead/issue-399-20260608-0048

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a Gemini billing probe to detect depleted prepayment credits early, preventing unnecessary retries and delays. It also updates rate-limiting patterns to recognize Gemini billing exhaustion and adds comprehensive unit tests. Feedback on the changes highlights a portability issue in scripts/validate-engines.sh where the GNU-specific head -n -1 is used instead of a POSIX-compliant alternative like sed '$d'. Additionally, redundant regex patterns matching depleted credits in both scripts/validate-engines.sh and scripts/engine.sh should be simplified.

Comment thread scripts/validate-engines.sh Outdated
Comment thread scripts/validate-engines.sh Outdated
Comment thread scripts/engine.sh Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 817caf58be

ℹ️ 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".

Comment thread scripts/validate-engines.sh
Comment thread scripts/validate-engines.sh Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — rate-limited (intent: review-changes)

PR: #487
The retry cron will re-attempt automatically.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Note

@don-petry I received your request but all AI engines are currently rate-limited. I'll retry automatically once the rate limit clears.
Rate limit resets at: unknown

@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:35
@don-petry
don-petry disabled auto-merge June 8, 2026 01:41
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — rate-limited (intent: fix-bot-comment)

PR: #487
Please re-trigger manually (re-mention @dev-lead) when the rate limit clears — the original request cannot be reconstructed automatically.

@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:41
@don-petry
don-petry disabled auto-merge June 8, 2026 01:42
@don-petry

Copy link
Copy Markdown
Collaborator Author

Note

@don-petry I received your request but all AI engines are currently rate-limited. Please re-mention @dev-lead when the rate limit clears (estimated: unknown) — I cannot reconstruct the original instruction automatically.

@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:42
@don-petry
don-petry disabled auto-merge June 8, 2026 01:42
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:42
@don-petry
don-petry disabled auto-merge June 8, 2026 01:43
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:43
@don-petry
don-petry disabled auto-merge June 8, 2026 01:44
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:44
@don-petry
don-petry disabled auto-merge June 8, 2026 01:44
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:44
@don-petry
don-petry disabled auto-merge June 8, 2026 01:45
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:45
@don-petry
don-petry disabled auto-merge June 8, 2026 01:46
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:46
@don-petry
don-petry disabled auto-merge June 8, 2026 01:46
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #487
No changes were committed, but the PR still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews). The retry cron will re-attempt automatically. Next attempt after: 2026-06-08T06:04:54Z

@don-petry

Copy link
Copy Markdown
Collaborator Author

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.
Next attempt after: 2026-06-08T06:04:54Z

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2b43a404d8

ℹ️ 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".

Comment thread scripts/review-batch.sh
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 8, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8229988abe

ℹ️ 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".

Comment thread scripts/review-batch.sh Outdated
donpetry-bot
donpetry-bot previously approved these changes Jun 8, 2026

@donpetry-bot donpetry-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: b98d3cd1884ad9cbfd402ce745f216207ec91d1c
Review mode: triage-approved (single reviewer)

Summary

Bug fix for issue #399. Adds a Gemini REST billing probe in validate-engines.sh so depleted prepayment credits are detected at pre-flight, avoiding the Gemini CLI's 10× ~4-minute retry loop on every PR invocation. Companion changes: add RESOURCE_EXHAUSTED / credits.*depleted patterns to engine.sh rate-limit detection, short-circuit review-batch.sh when PRS_FILE is empty, and skip Gemini at runtime when the probe already flagged it unavailable. Well-scoped CI orchestration fix.

Linked issue analysis

Closes #399 (PR Review Agent — failures detected 2026-05-27). The change directly addresses the failure mode: Gemini billing depletion was causing each scheduled review to burn ~4 minutes on retry backoff before falling through to Copilot. The probe + early-switch logic eliminate that waste at the source.

Findings

  • Probe is fail-open — only marks Gemini unavailable on an explicit credits.*depleted match in the response body; transient quota errors (RESOURCE_EXHAUSTED without billing-specific text) still let Gemini proceed. A dedicated bats test (test_validate_engines.bats) guards this distinction.
  • POSIX portability — earlier gemini-code-assist feedback flagged GNU-specific head -n -1; the committed code uses sed '$d' instead, which is portable.
  • No new secrets or auth surface — GOOGLE_API_KEY is read from the existing env and interpolated into a double-quoted curl URL; this matches Google's standard auth pattern and how the rest of the codebase already uses the key. Request body is hardcoded; no user input.
  • Short-circuit ordering in review-batch.sh — the empty-PRS_FILE check now precedes the Copilot smoke test, with a regression bats test asserting that no Copilot smoke test runs when there are zero candidate PRs (avoiding spurious failures when COPILOT_GITHUB_TOKEN is absent on idle scheduled runs).
  • Test coverage — 4 new test files updated covering: billing-depleted detection, transient quota tolerance, empty-batch short-circuit, and the runtime fallback path. Test mocks for curl are isolated per-scenario.

Nothing requiring changes.

CI status

All checks green: shellcheck, bats, unit-tests, CodeQL (actions + python), SonarCloud (Quality Gate passed, 0 new issues), AgentShield, Secret scan (gitleaks), Agent Security Scan, Compile agentic workflows, validate-agent-profiles, gh-aw-compile, dependency audit (skipped — no matching ecosystems). CodeRabbit has already approved at this SHA.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 4.7). Reply if you need a human review.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 71d7691177

ℹ️ 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".

Comment thread scripts/review-batch.sh Outdated
Comment thread scripts/validate-engines.sh Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6477c96478

ℹ️ 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".

Comment thread scripts/review-batch.sh Outdated
donpetry-bot
donpetry-bot previously approved these changes Jun 8, 2026

@donpetry-bot donpetry-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: 6477c96478d25f139389a837a202d94586d171be
Review mode: triage-approved (single reviewer)

Summary

Bug fix for issue #399. Adds a pre-flight Gemini REST billing probe in validate-engines.sh so depleted prepayment credits are detected before invocation, avoiding the Gemini CLI's 10× ~4-minute retry loop on every PR review. Companion changes:

  • Extend engine.sh rate-limit patterns to recognise RESOURCE_EXHAUSTED / credits.*depleted.
  • Short-circuit review-batch.sh when PRS_FILE is empty (moved above the Copilot smoke test so empty scheduled runs no longer fail on absent COPILOT_GITHUB_TOKEN).
  • Skip Gemini at batch start when the probe already flagged it unavailable, so no PR pays the per-call retry delay.

Well-scoped CI orchestration fix with thorough bats + shell-script tests covering the new branches (billing-depleted, transient RESOURCE_EXHAUSTED without billing text, empty-PRs short-circuit, Gemini→Copilot skip).

Linked issue analysis

Closes #399 (Gemini prepayment credits depletion causing per-PR ~4-minute retry stalls). The probe + early skip directly remove the wasted retries, and the rate-limit pattern additions ensure the runtime detector still classifies billing errors correctly if the probe ever fails open.

Findings

No blocking issues. The duplication of the credits.*depleted pattern across validate-engines.sh (probe-time body check) and engine.sh (runtime CLI-output classification) is intentional — they're matched against different things at different times. The probe fails open on non-billing errors, which is the right default for a pre-flight check.

CI status

All required checks green: Lint, ShellCheck, CodeQL, gitleaks, SonarCloud (quality gate passed), bats, unit-tests, AgentShield, Agent Security Scan, validate-agent-profiles, Compile agentic workflows. Prior donpetry-bot review approved at b98d3cd1; new commits since then are only main-merges (#438, #400) and do not modify this PR's own files.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 4.7). Reply if you need a human review.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

@sonarqubecloud

sonarqubecloud Bot commented Jun 8, 2026

Copy link
Copy Markdown

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@donpetry-bot donpetry-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: c42552c6487667f3a8a9cc2507a1fb3a067cdf12
Review mode: triage-approved (single reviewer)

Summary

Operational hardening of the PR-review engine pipeline: detect Gemini prepayment-credit depletion up front (via a minimal REST probe in validate-engines.sh) instead of letting the Gemini CLI burn ~4 minutes retrying, switch the batch to Copilot immediately when that probe trips, and short-circuit the whole batch when there are no candidate PRs so an absent/invalid COPILOT_GITHUB_TOKEN can't fail a no-op scheduled run. Also extends the rate-limit regex with RESOURCE_EXHAUSTED and credits.*depleted, and guards DUCK_PID usage in review-one-pr.sh against the now-possible unset case.

Linked issue analysis

Closes #399 (Gemini billing-depleted handling). The diff substantively addresses the issue: (a) detects the condition before any PR work, (b) updates rate-limit classification so a per-call surface of the same error is recognized as a rate-limit (not a CLI bug), (c) cleanly routes around the dead engine to Copilot, and (d) aborts loudly when no fallback is available rather than retrying into a guaranteed failure.

Findings

No blocking findings.

Minor observations (non-blocking):

  • _gemini_billing_probe is fail-open on any non-billing error (network, invalid key, etc.), which matches the comment and is the right call — Gemini still gets a chance and surfaces the real error at call time.
  • The probe sends a 1-token generateContent request on every batch start; cost is negligible but the timeout 15 cap means a hung endpoint adds up to ~15s to every run. Acceptable for an operations-time pre-flight.
  • Trailing-whitespace cleanup in validate-engines.sh, test_batch_fallback.bats, and test_validate_engines.bats is bundled in — fine, no behavior change.

CI status

All 29 checks green: ShellCheck, bats, unit-tests, validate-agent-profiles, CodeQL (actions + python), Secret scan (gitleaks), Agent Security Scan, SonarCloud, dependency-audit ecosystems, gh-aw-compile, CodeRabbit. One earlier review run shows CANCELLED but a subsequent review run is SUCCESS, consistent with normal restart behavior.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 4.7). Reply if you need a human review.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c42552c648

ℹ️ 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".

timeout 15 curl -sS --max-time 10 \
-X POST \
-H "Content-Type: application/json" \
-H "X-Goog-Api-Key: ${GOOGLE_API_KEY}" \

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep the Gemini API key out of curl argv

When validate_engines runs on a shared or self-hosted runner with GOOGLE_API_KEY set, this still exposes the credential through the curl process arguments because the header value is interpolated directly into -H "X-Goog-Api-Key: ..."; process listings can read that argument while the probe is in flight. Fresh evidence despite the prior thread: the current code moved the key from the URL into an argv header rather than into a non-argv channel, so use a curl config/header file or another mechanism that does not place the secret in the command line.

Useful? React with 👍 / 👎.

Comment thread scripts/review-one-pr.sh
Comment on lines +583 to +584
if [ "${DUCK_ENGINE:-}" = "gemini" ] && [ "${GEMINI_AVAILABLE:-false}" != "true" ]; then
echo "::notice::Skipping Gemini duck reviewer — GEMINI_AVAILABLE=false per pre-flight probe"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Clear skipped duck output before reusing it

When a batch reviews more than one PR and this branch skips the Gemini duck (DUCK_ENGINE=gemini with GEMINI_AVAILABLE not true), /tmp/cascade/rubber-duck.json is never removed or overwritten. A valid duck JSON left by an earlier PR is then accepted by the later if [ -s "$DUCK_OUTPUT" ] && jq empty ... check, causing the current PR to synthesize its deep review with a stale duck verdict from a different PR; remove or truncate DUCK_OUTPUT/duck.log before the optional launch or in the skip branch.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PR Review Agent — failures detected 2026-05-27

3 participants