feat(engine): configure engine enablement, fallback order and duck via AI_ENGINES; probe every Gemini key
Merged
DO
1mo ago+1253+1253+0123456789012345678901234567890123456789-149-149-012345678901234567890123456789
Summary
Engines can now be switched with GitHub Actions variables (org or repo level), with no code change and no release:
| Variable | Default | Meaning |
|---|---|---|
AI_ENGINES | claude,gemini,copilot | Ordered list of enabled engines. An engine not listed is never used; the order is the fallback order; the first entry is the primary. |
AI_DUCK_ENGINE | automatic | Rubber-duck engine: claude, gemini, copilot, or none. |
AI_DUCK_MODEL | engine default | Model for an explicit AI_DUCK_ENGINE. |
GEMINI_FLASH_MODEL / GEMINI_PRO_MODEL | gemini-3.8-flash / gemini-2.5-pro | Gemini model ids, now settable as variables. |
REVIEW_ENGINE / DEV_LEAD_ENGINE | unset | Legacy primary overrides. Ignored, with a warning, when AI_ENGINES disables the engine they name. |
Examples: AI_ENGINES=claude,gemini turns Copilot off. AI_ENGINES=gemini,claude makes Gemini primary. AI_ENGINES=claude means Claude only.
scripts/lib/engine-chain.sh(new) parses the chain, shared byengine.sh,validate-engines.shandreview-batch.sh. An unknown name such asclaude,cluadeinvalidates the whole value: the default chain is used and a warning is logged, so a typo can't silently drop fallbacks. This keeps the existingDEV_LEAD_ENGINEScontract (#1546), which is still read whenAI_ENGINESis unset.review-batch.shwalksAI_ENGINESinstead of the hard-coded claude → gemini → copilot:- it starts on the first available engine when the primary is unusable;
- if later engines exist but none is usable, it skips that PR and continues (previously the session could abort);
- it aborts only when the chain has no later engine at all, which matches the old end-of-chain behaviour.
- dev-lead's
run_writer_with_fallbackuses the same parser, anddev-lead-reusable.ymlfinally maps the variable. (DEV_LEAD_ENGINESexisted in code but no workflow ever passed it.) - Rubber duck: chosen by
AI_DUCK_ENGINE, otherwise the built-in cross-engine default, otherwise the next usable engine inAI_ENGINES. When no engine is usable, it's skipped with a notice instead of failing on every review. - Workflows can't split a string, so they derive the primary with
startsWith(vars.AI_ENGINES, …). The scripts enforce the full list.
Problem
- Gemini's three keys (#1777 / #1780) never helped the pre-flight check.
_gemini_billing_probetested only the first key (validate-engines.sh:37). One depleted key marked Gemini unavailable for the whole run, although_gemini_invokerotates throughGOOGLE_API_KEY_2and_3at call time. The probe now checks every configured key. Gemini is unavailable only when all of them are depleted, and depleted keys are named (never their values). - Copilot was reported available on a classic PAT (#1960).
validate-engines.shaccepted any token that letgh copilot --versionrun. Copilot rejectsghp_at call time, so every Copilot fallback and every Claude-primary duck failed. The probe and the headroom check (engine.sh:299) now apply the rule dev-lead already had (#1495). - No way to switch providers without a code change. The fallback order was hard-coded, and dev-lead's kill switch wasn't reachable from any variable.
Part of #1960 (AC-1, AC-2, AC-4). For AC-3, the Gemini pro model is now a variable (GEMINI_PRO_MODEL), but I haven't changed the default, since I can't verify which model this key can use. Epic #1895.
Risk
Medium. This changes engine selection and fallback in pr-review and dev-lead.
- With every variable unset, behaviour matches today's default chain.
- There are two deliberate changes:
- the pre-flight probe now marks Copilot unavailable on a
ghp_token and Gemini available when any key has credits; - a Gemini runtime failure followed by an unavailable Copilot skips the PR instead of aborting the session.
- the pre-flight probe now marks Copilot unavailable on a
- If
REVIEW_ENGINEorDEV_LEAD_ENGINEis set as an org variable today, it keeps overriding the primary. Delete it to letAI_ENGINESorder decide.
Test plan
- New
tests/dev-lead/unit/test_engine_chain_config.bats(29 cases, run bytest-dev-lead.yml), covering:- parsing: order, case, duplicates, typo and glob fall back to the default, legacy
DEV_LEAD_ENGINES; - primary and override rules, availability, and the forward-only fallback walk;
- duck selection, including
none, an explicit engine and model, and a model that must not follow the duck onto another engine; - validate: key 1 depleted but keys 2 and 3 fine means available; all depleted means unavailable;
ghp_rejected andgithub_pat_accepted; disabled engines not probed; review-batch.shwith the real library:claude,copilotskips Gemini;gemini,claudemakes Gemini primary; a disabledREVIEW_ENGINEis ignored; skip-and-continue; a single-engine chain aborts.
- parsing: order, case, duplicates, typo and glob fall back to the default, legacy
tests/test_batch_fallback.bats,test_engine_unavailable_notice.batsandtest_batch_skip_reporting.batsnow copy the library, and all pass unchanged in their assertions.tests/test_validate_engines.batsandtests/test-validate-engines.sh(12/12) also pass.- I ran all 56 bats files touching the engine, pricing, batch, validate or review scripts. No new failures compared with the post-#1958 baseline; the remaining 49 are the same environmental failures seen on main.
shellcheck --severity=warning -xover all ofscripts/: clean.actionlinton the two workflows: only the pre-existing SC2129 on main.check-duplicate-decls.sh,validate-caller-inputs.sh,caller_stub_freeze.sh,validate-ingress-if.shandvalidate-workflow-schedules.shpass. markdownlint is clean on the docs. A gitleaks scan ofmain..HEADfound no leaks.
Rollback
Revert this PR. There are no side effects: without the new variables, behaviour matches the pre-PR default chain.
Monitoring
- Each pr-review and dev-lead run logs
::notice::Engine chain (AI_ENGINES): …plus a notice for any disabled engine. - A depleted Gemini key shows as
::warning::Gemini key(s) with depleted prepayment credits: GOOGLE_API_KEY…, and Gemini stays available while any key has credits. - The tier-2 log line now reads
rubber duck (<model> via <engine>), ornonewith a skip notice. - This takes effect only after Release Manager autocut cuts it to
next.
Interaction contract
- N/A — this PR does not add or change an agentic role.
Checklist
-
shellcheck scripts/*.shpasses for any changed shell scripts. - Docs updated where behavior or standards changed (
docs/engine-configuration.md,docs/pr-review-agent/setup.md).
🤖 Generated with Claude Code
https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R
Generated by Claude Code
Summary by CodeRabbit
- New Features
- Configure an ordered chain of AI engines, with automatic selection and fallback when an engine is unavailable or rate-limited.
- Choose or disable the engine used for optional “rubber-duck” reviews.
- Gemini availability checks now account for multiple configured API keys.
- Bug Fixes
- Copilot credentials using classic
ghp_tokens are no longer treated as valid.
- Copilot credentials using classic
- Documentation
- Added guidance on engine selection, configuration, fallback behavior, and availability checks.