fix: address open review findings from PR #102

[don-petry]

Rebased version of #105. Addresses review findings from the Gemini Pro PR (#102):

• scripts/review-batch.sh: update engine comment to include gemini
• scripts/engine.sh: update run_duck() docstring and duck-engine comments to reflect 3-engine support
• .github/workflows/pr-review.yml: warn when Gemini CLI is unavailable after best-effort installs
• docs/pr-review-agent/machine-user-setup.md: add explicit secret naming reference table

Replaces #105 (had merge conflict with main).

Summary by CodeRabbit

• Documentation
  • Enhanced setup documentation for token and secret configuration
  • Updated review engine configuration documentation with fallback chain details
  • Improved workflow validation and error messaging for AI engine availability

[coderabbitai]

Warning

Rate limit exceeded

@don-petry has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 52 minutes and 24 seconds before requesting another review.

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 @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 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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a027cead-9796-4eb2-bb40-912a8cd0e7ce

📥 Commits

Reviewing files that changed from the base of the PR and between b941538 and 6492326.

📒 Files selected for processing (4)

• .github/workflows/pr-review.yml
• docs/pr-review-agent/machine-user-setup.md
• scripts/engine.sh
• scripts/review-batch.sh

📝 Walkthrough

Walkthrough

The PR adds Gemini CLI availability validation to two workflow branches and updates documentation across scripts and setup guides to clarify the primary-engine-fallback configuration and secret naming conventions.

Changes

Gemini CLI Validation and Engine Documentation

Layer / File(s)	Summary
Gemini CLI Validation in Workflow .github/workflows/pr-review.yml	After installing @google/gemini-cli, the workflow now checks for the gemini command availability in both claude and copilot review-engine branches, emitting a ::warning:: if unavailable.
Engine Selection Documentation scripts/engine.sh, scripts/review-batch.sh	Comments are refined to explicitly document that the rubber-duck engine uses Copilot when Claude is primary (and vice versa), and REVIEW_ENGINE now describes a fallback chain of claude -> gemini -> copilot.
Setup and Secret Naming Reference docs/pr-review-agent/machine-user-setup.md	A new "Secret naming reference" section clarifies that DON_PETRY_BOT_GH_PAT is the bot reviewer PAT and GH_PAT is a separate Copilot-user PAT, with a table indicating which workflows use each.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• petry-projects/.github-private#102: Modifies the pr-review workflow's review-engine installation path with Gemini CLI install/check logic.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main purpose of the pull request—addressing review findings from PR #102—and is clear and specific.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/pr102-review-findings-v2

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[gemini-code-assist]

Code Review

This pull request updates the documentation and script comments to support the Gemini engine and clarify the 'rubber duck' review logic. Key changes include a new secret naming reference table and updated fallback chain descriptions. Review feedback recommends including the GOOGLE_API_KEY in the documentation for completeness and points out that the Gemini case in the run_duck function is currently unreachable, suggesting it be utilized to improve review diversity.

[Copilot]

Pull request overview

This PR addresses outstanding review findings from earlier Gemini-engine work by aligning comments/docs with the current 3-engine (Claude/Gemini/Copilot) support and improving CI observability when optional Gemini tooling isn’t available.

Changes:

• Update scripts/review-batch.sh env docs to reflect the Claude → Gemini → Copilot fallback chain.
• Clarify scripts/engine.sh duck-engine comments/docstring for the 3-engine setup.
• Add workflow warnings when best-effort Gemini CLI installation doesn’t result in an available gemini command.
• Add a secret naming reference table to the machine-user setup docs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
scripts/review-batch.sh	Updates header docs to include Gemini in REVIEW_ENGINE and the fallback chain.
scripts/engine.sh	Updates duck-engine comments/docstring to reflect actual cross-engine selection.
.github/workflows/pr-review.yml	Warns when gemini CLI is still unavailable after best-effort install paths.
docs/pr-review-agent/machine-user-setup.md	Adds a concise table clarifying PAT/secret naming and usage.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[donpetry-bot]

Automated review — APPROVED ✓

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

Summary

Documentation and comment updates across 4 files to align with 3-engine (Claude/Gemini/Copilot) support. No runtime logic changes. Adds a GitHub Actions ::warning:: annotation when the Gemini CLI is unavailable after best-effort install, and a secret naming reference table to the machine-user setup docs.

Linked issue analysis

No closing issues linked. PR body references PR #102 (Gemini Pro integration) and #105 (prior version with merge conflict) for context — this is a clean rebase of #105 onto current main.

Findings

No issues found. All changes are strictly documentation, comments, or non-destructive CI observability improvements:

• scripts/review-batch.sh — Header comment updated to list gemini as a valid REVIEW_ENGINE value and describe the fallback chain. Comment-only.
• scripts/engine.sh — Four comment/docstring lines updated to clarify duck-engine selection logic per primary engine. Comment-only.
• .github/workflows/pr-review.yml — Adds command -v gemini check after best-effort install in two engine cases (claude and copilot). Emits a ::warning:: annotation if unavailable. Safe, non-blocking, follows the existing pattern used for the Copilot CLI check immediately below.
• docs/pr-review-agent/machine-user-setup.md — Adds a secret naming reference table documenting DON_PETRY_BOT_GH_PAT and GH_PAT purposes. No secrets exposed — names and usage descriptions only.

CI status

Check	Status
CodeQL (actions)	✅ Passed
SonarCloud	✅ Passed (0 issues, 0 hotspots)
AgentShield	✅ Passed
Dependency audit	✅ Passed (ecosystems skipped — none detected)
CodeRabbit	✅ Passed

---

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

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

scripts/engine.sh (1)

1-1: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add strict shell mode to this script.

set -euo pipefail is missing in scripts/engine.sh, which violates the repository shell-script standard and weakens failure handling.

Suggested patch

 #!/usr/bin/env bash
+set -euo pipefail
 # Engine abstraction layer for LLM invocations.

As per coding guidelines, scripts/**/*.{sh,bash}: Scripts must be POSIX-compatible shell with #!/usr/bin/env bash shebang and set -euo pipefail.

🤖 Prompt for 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.

In `@scripts/engine.sh` at line 1, Add strict shell mode to the script by
inserting set -euo pipefail immediately after the existing shebang
(#!/usr/bin/env bash) in scripts/engine.sh so the script exits on errors, treats
unset variables as errors, and fails on pipeline errors; ensure this placement
is the first executable line and remains POSIX-/bash-compatible.

🤖 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/pr-review.yml:
- Around line 132-134: The warning about the missing gemini CLI is misleading
for REVIEW_ENGINE=copilot; update the gemini check so the echo only warns when
REVIEW_ENGINE is not "copilot" (or change the message to accurately state
limited impact for copilot). Locate the shell block that tests command -v gemini
and the echo line, reference the REVIEW_ENGINE variable there, and either skip
emitting the warning when REVIEW_ENGINE=copilot or adjust the text to explicitly
say copilot is unaffected while Gemini-dependent paths will be skipped for other
engines.

---

Outside diff comments:
In `@scripts/engine.sh`:
- Line 1: Add strict shell mode to the script by inserting set -euo pipefail
immediately after the existing shebang (#!/usr/bin/env bash) in
scripts/engine.sh so the script exits on errors, treats unset variables as
errors, and fails on pipeline errors; ensure this placement is the first
executable line and remains POSIX-/bash-compatible.

🪄 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: 8030808b-1161-4788-a00f-4fe04d89bfda

📥 Commits

Reviewing files that changed from the base of the PR and between 28a5cb1 and b941538.

📒 Files selected for processing (4)

• .github/workflows/pr-review.yml
• docs/pr-review-agent/machine-user-setup.md
• scripts/engine.sh
• scripts/review-batch.sh

[donpetry-bot]

Superseded by automated re-review at e728a47106b00397ffca2f6d69b61261e46f94bc — click to expand prior review.

Review — fix requested (cycle 2/3)

The automated review identified the following issues. Please address each one:

Findings to fix

[Findings would be inserted here]

Additional tasks

1. Resolve all unresolved review thread comments from other reviewers
2. Ensure all CI checks pass after your changes
3. Rebase on the target branch if behind
4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

[github-actions]

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved manually.

Please resolve the conflicts and push:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

[donpetry-bot]

Review — fix requested (cycle 3/3)

The automated review identified the following issues. Please address each one:

Findings to fix

[Findings would be inserted here]

Additional tasks

1. Resolve all unresolved review thread comments from other reviewers
2. Ensure all CI checks pass after your changes
3. Rebase on the target branch if behind
4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

[donpetry-bot]

Automated review — human attention needed

This PR has been through 3 automated review cycles (cap: 3) without converging on an approval-and-merge state. Further automated review has been paused to avoid infinite loops.

Please take a look manually, or close this PR if it's no longer needed. Once a human review resolves the situation, remove the needs-human-review label and the cascade can be re-engaged on the next push.

Posted by the donpetry-bot PR-review cascade.

[don-petry]

Dev-lead review — all findings addressed

I've reviewed the open CodeRabbit CHANGES_REQUESTED findings against the current branch HEAD (239c5c8e). Both items are resolved in commits a1200d3 and 1b85647:

Finding	Status
scripts/engine.sh — missing set -euo pipefail	✅ Fixed — line 2 of engine.sh
.github/workflows/pr-review.yml — misleading gemini CLI warning in copilot branch	✅ Fixed — claude branch now says "Gemini-based fallback paths will be skipped"; copilot branch says "Gemini duck review will be skipped" (accurate: DUCK_ENGINE=gemini when REVIEW_ENGINE=copilot)

The CodeRabbit CHANGES_REQUESTED review was based on commits 28a5cb1d…b9415382 — the fixes landed after that review ran. All other findings (GOOGLE_API_KEY in docs, gemini duck-engine routing, run_duck docstring) were also addressed in those commits.

SonarQube quality gate passed. No remaining valid blockers identified.

Removing needs-human-review label and requesting re-review to re-engage the cascade.

[don-petry]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[don-petry]

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention.

To resolve manually instead:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

[don-petry]

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention.

To resolve manually instead:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

[don-petry]

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

Claude will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention.

To resolve manually instead:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push