feat(engine): configure engine enablement, fallback order and duck via AI_ENGINES; probe every Gemini key

Merged
DO
1mo ago
+1253-149

Summary

Engines can now be switched with GitHub Actions variables (org or repo level), with no code change and no release:

VariableDefaultMeaning
AI_ENGINESclaude,gemini,copilotOrdered 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_ENGINEautomaticRubber-duck engine: claude, gemini, copilot, or none.
AI_DUCK_MODELengine defaultModel for an explicit AI_DUCK_ENGINE.
GEMINI_FLASH_MODEL / GEMINI_PRO_MODELgemini-3.8-flash / gemini-2.5-proGemini model ids, now settable as variables.
REVIEW_ENGINE / DEV_LEAD_ENGINEunsetLegacy 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 by engine.sh, validate-engines.sh and review-batch.sh. An unknown name such as claude,cluade invalidates the whole value: the default chain is used and a warning is logged, so a typo can't silently drop fallbacks. This keeps the existing DEV_LEAD_ENGINES contract (#1546), which is still read when AI_ENGINES is unset.
  • review-batch.sh walks AI_ENGINES instead 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_fallback uses the same parser, and dev-lead-reusable.yml finally maps the variable. (DEV_LEAD_ENGINES existed 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 in AI_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_probe tested only the first key (validate-engines.sh:37). One depleted key marked Gemini unavailable for the whole run, although _gemini_invoke rotates through GOOGLE_API_KEY_2 and _3 at 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.sh accepted any token that let gh copilot --version run. Copilot rejects ghp_ 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.
  • If REVIEW_ENGINE or DEV_LEAD_ENGINE is set as an org variable today, it keeps overriding the primary. Delete it to let AI_ENGINES order decide.

Test plan

  • New tests/dev-lead/unit/test_engine_chain_config.bats (29 cases, run by test-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 and github_pat_ accepted; disabled engines not probed;
    • review-batch.sh with the real library: claude,copilot skips Gemini; gemini,claude makes Gemini primary; a disabled REVIEW_ENGINE is ignored; skip-and-continue; a single-engine chain aborts.
  • tests/test_batch_fallback.bats, test_engine_unavailable_notice.bats and test_batch_skip_reporting.bats now copy the library, and all pass unchanged in their assertions. tests/test_validate_engines.bats and tests/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 -x over all of scripts/: clean. actionlint on 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.sh and validate-workflow-schedules.sh pass. markdownlint is clean on the docs. A gitleaks scan of main..HEAD found 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>), or none with 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/*.sh passes 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.
  • Documentation
    • Added guidance on engine selection, configuration, fallback behavior, and availability checks.