fix: add Gemini pre-flight check and engine-availability job summary#150
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ 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 |
|
CI is green and all checks pass. @petry-projects/org-leads — this PR is ready for review and merge when you have a chance. Closes #146 |
There was a problem hiding this comment.
Pull request overview
Adds an engine “pre-flight” validation step to the PR-review cascade so runs surface degraded fallback configuration (especially Gemini) up-front and in the GitHub Actions job summary, before any PRs are processed.
Changes:
- Add
scripts/validate-engines.shto detect Claude/Gemini/Copilot availability, emit::warning::annotations, and append an availability table toGITHUB_STEP_SUMMARY. - Call
validate_enginesat the start ofscripts/review-batch.shand use the computed$GEMINI_AVAILABLEflag for Claude→Gemini fallback decisions. - Add unit tests in
tests/test-validate-engines.shto cover Gemini availability, warning text, and summary output.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
tests/test-validate-engines.sh |
Adds unit tests for validate_engines() behavior via PATH/env mocking. |
scripts/validate-engines.sh |
Introduces pre-flight engine availability checks + job summary output. |
scripts/review-batch.sh |
Runs pre-flight validation before processing PRs; reuses computed Gemini availability in fallback chain. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| local extra_path="${1:-}"; shift | ||
| local combined_path="${extra_path:+$extra_path:}$MOCK_DIR:/usr/local/bin:/usr/bin:/bin" | ||
| ( |
| _emit_engine_summary() { | ||
| local claude_ok="$1" gemini_ok="$2" copilot_ok="$3" | ||
| local dest="${GITHUB_STEP_SUMMARY:-}" | ||
| [ -z "$dest" ] && return 0 | ||
| { | ||
| printf '### Engine availability (pre-flight)\n\n' | ||
| printf '| Engine | Status |\n' | ||
| printf '|---------|--------|\n' | ||
| printf '| Claude | %s |\n' "$(_engine_badge "$claude_ok")" | ||
| printf '| Gemini | %s |\n' "$(_engine_badge "$gemini_ok")" | ||
| printf '| Copilot | %s |\n' "$(_engine_badge "$copilot_ok")" | ||
| printf '\n' | ||
| } >> "$dest" | ||
| } |
| if env GH_TOKEN="${COPILOT_GITHUB_TOKEN:-${GH_TOKEN:-}}" \ | ||
| gh copilot --version >/dev/null 2>&1; then | ||
| copilot_ok=true |
| if env GH_TOKEN="${COPILOT_GITHUB_TOKEN:-${GH_TOKEN:-}}" \ | ||
| gh copilot --version >/dev/null 2>&1; then | ||
| copilot_ok=true |
There was a problem hiding this comment.
Code Review
This pull request introduces a pre-flight engine validation script and integrates it into the batch review process to ensure Claude, Gemini, and Copilot are correctly configured before execution. It also includes a new unit test suite for this validation logic. Feedback from the review suggests using more idiomatic shell variable expansion for string concatenation and adding a warning for Copilot unavailability to align with the script's documented behavior.
| local gemini_reasons="" | ||
| if ! command -v gemini >/dev/null 2>&1; then | ||
| gemini_reasons="Gemini CLI not installed (fix: npm install -g @google/gemini-cli)" | ||
| fi | ||
| if [ -z "${GOOGLE_API_KEY:-}" ]; then | ||
| if [ -n "$gemini_reasons" ]; then | ||
| gemini_reasons="$gemini_reasons; GOOGLE_API_KEY secret not set" | ||
| else | ||
| gemini_reasons="GOOGLE_API_KEY secret not set" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
The logic for building the gemini_reasons string can be simplified using the shell-idiomatic ${var:+$var; } pattern. This is more concise and matches the style used in scripts/review-batch.sh.
| local gemini_reasons="" | |
| if ! command -v gemini >/dev/null 2>&1; then | |
| gemini_reasons="Gemini CLI not installed (fix: npm install -g @google/gemini-cli)" | |
| fi | |
| if [ -z "${GOOGLE_API_KEY:-}" ]; then | |
| if [ -n "$gemini_reasons" ]; then | |
| gemini_reasons="$gemini_reasons; GOOGLE_API_KEY secret not set" | |
| else | |
| gemini_reasons="GOOGLE_API_KEY secret not set" | |
| fi | |
| fi | |
| local gemini_reasons="" | |
| command -v gemini >/dev/null 2>&1 || gemini_reasons="Gemini CLI not installed (fix: npm install -g @google/gemini-cli)" | |
| if [ -z "${GOOGLE_API_KEY:-}" ]; then | |
| gemini_reasons="${gemini_reasons:+$gemini_reasons; }GOOGLE_API_KEY secret not set" | |
| fi |
References
- Use shell-idiomatic variable expansion for concise string concatenation. (link)
| if env GH_TOKEN="${COPILOT_GITHUB_TOKEN:-${GH_TOKEN:-}}" \ | ||
| gh copilot --version >/dev/null 2>&1; then | ||
| copilot_ok=true | ||
| fi |
There was a problem hiding this comment.
The PR description and file header mention that warnings are emitted for any unavailable fallback engine, but currently, only Gemini has a warning. Adding a warning for Copilot would ensure that configuration issues for all fallback engines are visible in the logs. This supports the engine fallback mechanism by ensuring fallbacks are correctly configured before rate limits are encountered.
| if env GH_TOKEN="${COPILOT_GITHUB_TOKEN:-${GH_TOKEN:-}}" \ | |
| gh copilot --version >/dev/null 2>&1; then | |
| copilot_ok=true | |
| fi | |
| if env GH_TOKEN="${COPILOT_GITHUB_TOKEN:-${GH_TOKEN:-}}" \ | |
| gh copilot --version >/dev/null 2>&1; then | |
| copilot_ok=true | |
| else | |
| echo "::warning::Copilot fallback unavailable. When Gemini is rate-limited, runs will abort." | |
| fi |
References
- Ensure engine fallback mechanisms are supported by validating and reporting the status of fallback engines.
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: e8811fbfd49833b4eb8e68f395cf69cbb8c80727
Cascade: triage → deep (triage: haiku 4.5 → deep: sonnet 4.6 + duck: o4-mini → audit: opus 4.7)
Summary
PR satisfies all issue #146 acceptance criteria: pre-flight Gemini check, actionable warnings, job summary table, and unit tests — all CI passes. The headline concern from triage is a real but minor defect: the validate-engines.sh header comment claims '::warning:: annotations emitted for each unavailable engine' but only Gemini gets one; Claude and Copilot availability failures export env vars silently. This is a documentation/completeness gap, not a security or logic regression. No HIGH-risk patterns found.
Findings
- MINOR: Header comment at lines 12-14 of validate-engines.sh states 'For each unavailable fallback engine a ::warning:: annotation is emitted' but only Gemini emits a ::warning::. Claude unavailability and Copilot unavailability are captured in exported vars (CLAUDE_AVAILABLE, COPILOT_AVAILABLE) but produce no annotation. The docstring overpromises, and gemini-code-assist flagged this exact gap without it being addressed before the PR was submitted. (
scripts/validate-engines.shline 13) - MINOR: All 8 unit tests cover only Gemini scenarios. There are no tests for CLAUDE_AVAILABLE or COPILOT_AVAILABLE flag values, nor for warnings (or their absence) on those engines. This leaves the documented-but-unimplemented behaviour path untested and makes it harder to detect future regressions. (
tests/test-validate-engines.sh) - INFO: The else-branch in review-batch.sh (lines ~77-80) re-derives _gemini_miss by re-running 'command -v gemini' and checking GOOGLE_API_KEY at the rate-limit fallback point. Since validate_engines() already ran at startup, the same information is in GEMINI_AVAILABLE and the reasons were already logged. The duplication is harmless but means availability logic exists in two places. (
scripts/review-batch.shline 77) - INFO: _engine_badge() and _emit_engine_summary() are defined at file scope in validate-engines.sh. Sourcing the script exports these helper functions into the caller's namespace. Underscore prefix signals 'private' by convention but bash does not enforce it. Not a security risk; worth noting for future maintainability. (
scripts/validate-engines.shline 63)
Reviewed by the PR-review cascade (triage: haiku 4.5 → deep: sonnet 4.6 + duck: o4-mini → audit: opus 4.7). Reply if you need a human review.
|
Auto-rebase failed — merge conflict — this branch has conflicts with Please resolve the conflicts and push: |
Addresses #146 — silent Gemini fallback failures when GOOGLE_API_KEY is missing or the CLI is not installed. Changes: - scripts/validate-engines.sh (new): provides validate_engines() which checks Claude/Gemini/Copilot availability at startup, exports CLAUDE_AVAILABLE / GEMINI_AVAILABLE / COPILOT_AVAILABLE, emits ::warning:: annotations that include the exact fix command (e.g. 'npm install -g @google/gemini-cli'), and writes an engine-availability table to GITHUB_STEP_SUMMARY. - scripts/review-batch.sh: sources validate-engines.sh and calls validate_engines() before any PR is processed; the Claude→Gemini fallback check now uses the pre-computed $GEMINI_AVAILABLE flag; the "unavailable" warning also includes the specific missing reason and install command. - tests/test-validate-engines.sh (new): 8 unit tests covering the degraded and healthy states for GEMINI_AVAILABLE, ::warning:: annotation content, and job-summary output. GOOGLE_API_KEY is already forwarded in pr-review.yml:62 with a comment; no workflow-file changes are needed. Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
e8811fb to
2defc12
Compare
|
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 2defc129236a3a75479959d55ba87c34e4562cc4
Review mode: triage-approved (single reviewer)
Summary
Confirmation pass on triage's low-risk clearance. The PR adds a pre-flight engine-availability check (scripts/validate-engines.sh), wires it into scripts/review-batch.sh so the Claude→Gemini fallback uses a pre-computed $GEMINI_AVAILABLE flag, and adds 8 unit tests for the new helper. The change is well-scoped, tested, and the headline acceptance-criteria gap from the previous cycle (silent Gemini skip with no operator signal) is closed end-to-end. No HIGH-risk patterns: no auth/secrets/crypto changes, no DB migrations, no shell-injection sinks, no GitHub Actions security smells.
Linked issue analysis
Closes #146. All five acceptance criteria are met:
- Pre-flight check —
validate_engines()is invoked at the top ofreview-batch.shbefore any PR is processed. - Job summary — A Markdown engine-availability table is appended to
GITHUB_STEP_SUMMARY(_emit_engine_summary). - Secret documented —
GOOGLE_API_KEYis already forwarded inpr-review.yml:62per the PR description; no workflow-file change required. - No silent skip — Both the startup warning and the rate-limit fallback warning include the exact install command
npm install -g @google/gemini-cliand the specific missing reason. - Existing healthy-path behaviour unchanged — When Gemini is available, the fallback chain flows identically to before; only the gate now reads
$GEMINI_AVAILABLEinstead of re-deriving it.
Findings
None blocking. The minor documentation/completeness items flagged in the prior cycle (docstring overpromises warnings for all three engines but only Gemini emits one; tests cover only Gemini scenarios) are non-blocking polish items and do not affect correctness. They can be addressed in a follow-up without holding this fix.
CI status
All required checks green:
- agent-shield / AgentShield — SUCCESS
- claude-code / claude — SUCCESS
- CodeQL / Analyze (actions) — SUCCESS
- dependency-audit / Detect ecosystems — SUCCESS
- SonarCloud / SonarCloud Code Analysis — SUCCESS (Quality Gate Passed, 0 new issues)
- Tests / unit-tests — SUCCESS
- CodeRabbit — SUCCESS
- Dependabot config check — SUCCESS
Skipped checks (claude-ci-fix, claude-issue, language-specific audit branches, dependabot-automerge) are expected for a non-dependabot, non-issue, shell-only PR.
Reviewed automatically by the PR-review agent (single-reviewer mode: opus 4.7). Reply if you need a human review.
Addresses #146 — silent Gemini fallback failures when GOOGLE_API_KEY is missing or the CLI is not installed. Changes: - scripts/validate-engines.sh (new): provides validate_engines() which checks Claude/Gemini/Copilot availability at startup, exports CLAUDE_AVAILABLE / GEMINI_AVAILABLE / COPILOT_AVAILABLE, emits ::warning:: annotations that include the exact fix command (e.g. 'npm install -g @google/gemini-cli'), and writes an engine-availability table to GITHUB_STEP_SUMMARY. - scripts/review-batch.sh: sources validate-engines.sh and calls validate_engines() before any PR is processed; the Claude→Gemini fallback check now uses the pre-computed $GEMINI_AVAILABLE flag; the "unavailable" warning also includes the specific missing reason and install command. - tests/test-validate-engines.sh (new): 8 unit tests covering the degraded and healthy states for GEMINI_AVAILABLE, ::warning:: annotation content, and job-summary output. GOOGLE_API_KEY is already forwarded in pr-review.yml:62 with a comment; no workflow-file changes are needed. Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
Addresses #146 — silent Gemini fallback failures when GOOGLE_API_KEY is missing or the CLI is not installed. Changes: - scripts/validate-engines.sh (new): provides validate_engines() which checks Claude/Gemini/Copilot availability at startup, exports CLAUDE_AVAILABLE / GEMINI_AVAILABLE / COPILOT_AVAILABLE, emits ::warning:: annotations that include the exact fix command (e.g. 'npm install -g @google/gemini-cli'), and writes an engine-availability table to GITHUB_STEP_SUMMARY. - scripts/review-batch.sh: sources validate-engines.sh and calls validate_engines() before any PR is processed; the Claude→Gemini fallback check now uses the pre-computed $GEMINI_AVAILABLE flag; the "unavailable" warning also includes the specific missing reason and install command. - tests/test-validate-engines.sh (new): 8 unit tests covering the degraded and healthy states for GEMINI_AVAILABLE, ::warning:: annotation content, and job-summary output. GOOGLE_API_KEY is already forwarded in pr-review.yml:62 with a comment; no workflow-file changes are needed. Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
Addresses #146 — silent Gemini fallback failures when GOOGLE_API_KEY is missing or the CLI is not installed. Changes: - scripts/validate-engines.sh (new): provides validate_engines() which checks Claude/Gemini/Copilot availability at startup, exports CLAUDE_AVAILABLE / GEMINI_AVAILABLE / COPILOT_AVAILABLE, emits ::warning:: annotations that include the exact fix command (e.g. 'npm install -g @google/gemini-cli'), and writes an engine-availability table to GITHUB_STEP_SUMMARY. - scripts/review-batch.sh: sources validate-engines.sh and calls validate_engines() before any PR is processed; the Claude→Gemini fallback check now uses the pre-computed $GEMINI_AVAILABLE flag; the "unavailable" warning also includes the specific missing reason and install command. - tests/test-validate-engines.sh (new): 8 unit tests covering the degraded and healthy states for GEMINI_AVAILABLE, ::warning:: annotation content, and job-summary output. GOOGLE_API_KEY is already forwarded in pr-review.yml:62 with a comment; no workflow-file changes are needed. Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
Addresses #146 — silent Gemini fallback failures when GOOGLE_API_KEY is missing or the CLI is not installed. Changes: - scripts/validate-engines.sh (new): provides validate_engines() which checks Claude/Gemini/Copilot availability at startup, exports CLAUDE_AVAILABLE / GEMINI_AVAILABLE / COPILOT_AVAILABLE, emits ::warning:: annotations that include the exact fix command (e.g. 'npm install -g @google/gemini-cli'), and writes an engine-availability table to GITHUB_STEP_SUMMARY. - scripts/review-batch.sh: sources validate-engines.sh and calls validate_engines() before any PR is processed; the Claude→Gemini fallback check now uses the pre-computed $GEMINI_AVAILABLE flag; the "unavailable" warning also includes the specific missing reason and install command. - tests/test-validate-engines.sh (new): 8 unit tests covering the degraded and healthy states for GEMINI_AVAILABLE, ::warning:: annotation content, and job-summary output. GOOGLE_API_KEY is already forwarded in pr-review.yml:62 with a comment; no workflow-file changes are needed. Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
Addresses #146 — silent Gemini fallback failures when GOOGLE_API_KEY is missing or the CLI is not installed. Changes: - scripts/validate-engines.sh (new): provides validate_engines() which checks Claude/Gemini/Copilot availability at startup, exports CLAUDE_AVAILABLE / GEMINI_AVAILABLE / COPILOT_AVAILABLE, emits ::warning:: annotations that include the exact fix command (e.g. 'npm install -g @google/gemini-cli'), and writes an engine-availability table to GITHUB_STEP_SUMMARY. - scripts/review-batch.sh: sources validate-engines.sh and calls validate_engines() before any PR is processed; the Claude→Gemini fallback check now uses the pre-computed $GEMINI_AVAILABLE flag; the "unavailable" warning also includes the specific missing reason and install command. - tests/test-validate-engines.sh (new): 8 unit tests covering the degraded and healthy states for GEMINI_AVAILABLE, ::warning:: annotation content, and job-summary output. GOOGLE_API_KEY is already forwarded in pr-review.yml:62 with a comment; no workflow-file changes are needed. Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
Addresses #146 — silent Gemini fallback failures when GOOGLE_API_KEY is missing or the CLI is not installed. Changes: - scripts/validate-engines.sh (new): provides validate_engines() which checks Claude/Gemini/Copilot availability at startup, exports CLAUDE_AVAILABLE / GEMINI_AVAILABLE / COPILOT_AVAILABLE, emits ::warning:: annotations that include the exact fix command (e.g. 'npm install -g @google/gemini-cli'), and writes an engine-availability table to GITHUB_STEP_SUMMARY. - scripts/review-batch.sh: sources validate-engines.sh and calls validate_engines() before any PR is processed; the Claude→Gemini fallback check now uses the pre-computed $GEMINI_AVAILABLE flag; the "unavailable" warning also includes the specific missing reason and install command. - tests/test-validate-engines.sh (new): 8 unit tests covering the degraded and healthy states for GEMINI_AVAILABLE, ::warning:: annotation content, and job-summary output. GOOGLE_API_KEY is already forwarded in pr-review.yml:62 with a comment; no workflow-file changes are needed. Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
Addresses #146 — silent Gemini fallback failures when GOOGLE_API_KEY is missing or the CLI is not installed. Changes: - scripts/validate-engines.sh (new): provides validate_engines() which checks Claude/Gemini/Copilot availability at startup, exports CLAUDE_AVAILABLE / GEMINI_AVAILABLE / COPILOT_AVAILABLE, emits ::warning:: annotations that include the exact fix command (e.g. 'npm install -g @google/gemini-cli'), and writes an engine-availability table to GITHUB_STEP_SUMMARY. - scripts/review-batch.sh: sources validate-engines.sh and calls validate_engines() before any PR is processed; the Claude→Gemini fallback check now uses the pre-computed $GEMINI_AVAILABLE flag; the "unavailable" warning also includes the specific missing reason and install command. - tests/test-validate-engines.sh (new): 8 unit tests covering the degraded and healthy states for GEMINI_AVAILABLE, ::warning:: annotation content, and job-summary output. GOOGLE_API_KEY is already forwarded in pr-review.yml:62 with a comment; no workflow-file changes are needed. Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
Addresses #146 — silent Gemini fallback failures when GOOGLE_API_KEY is missing or the CLI is not installed. Changes: - scripts/validate-engines.sh (new): provides validate_engines() which checks Claude/Gemini/Copilot availability at startup, exports CLAUDE_AVAILABLE / GEMINI_AVAILABLE / COPILOT_AVAILABLE, emits ::warning:: annotations that include the exact fix command (e.g. 'npm install -g @google/gemini-cli'), and writes an engine-availability table to GITHUB_STEP_SUMMARY. - scripts/review-batch.sh: sources validate-engines.sh and calls validate_engines() before any PR is processed; the Claude→Gemini fallback check now uses the pre-computed $GEMINI_AVAILABLE flag; the "unavailable" warning also includes the specific missing reason and install command. - tests/test-validate-engines.sh (new): 8 unit tests covering the degraded and healthy states for GEMINI_AVAILABLE, ::warning:: annotation content, and job-summary output. GOOGLE_API_KEY is already forwarded in pr-review.yml:62 with a comment; no workflow-file changes are needed. Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Don Petry <don-petry@users.noreply.github.com>



Summary
scripts/validate-engines.sh: providesvalidate_engines()which runs before any PR is processed. It checks Claude/Gemini/Copilot availability, exportsCLAUDE_AVAILABLE/GEMINI_AVAILABLE/COPILOT_AVAILABLE, emits::warning::annotations that include the exact operator fix command (e.g.npm install -g @google/gemini-cli), and appends an engine-availability table toGITHUB_STEP_SUMMARY.scripts/review-batch.sh: sourcesvalidate-engines.shand callsvalidate_engines()at startup (before any PR loop). The Claude→Gemini fallback check uses the pre-computed$GEMINI_AVAILABLEflag. The "unavailable" warning in the fallback path is also updated to include the specific missing reason and install command.tests/test-validate-engines.sh: 8 unit tests covering degraded and healthy states forGEMINI_AVAILABLE,::warning::annotation content, and job-summary output.Note:
GOOGLE_API_KEYis already forwarded inpr-review.yml:62with a comment explaining its role — no workflow-file changes required.Acceptance criteria coverage
validate_engines()called at top ofreview-batch.sh::warning::emitted with specific reasonGITHUB_STEP_SUMMARYGOOGLE_API_KEYdocumented in workflowpr-review.yml:62npm install -g @google/gemini-clivalidate_engines()tests/test-validate-engines.shTest plan
bash tests/test-validate-engines.shlocally — all 8 tests should passpr-reviewworkflow withGOOGLE_API_KEYunset — confirm::warning::appears in job log with install command and job summary showsGemini | unavailableGOOGLE_API_KEYset — confirm job summary showsGemini | okCloses #146
Generated with Claude Code