MOT-4176 feat(llm-router,providers): provider-side env-var credential fallback - #566
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
skill-check — worker0 verified, 47 skipped (no docs/).
Four for four. Nicely done. |
📝 WalkthroughWalkthroughProvider resolution now accepts an optional credential environment variable. Shared fallback logic injects trimmed API keys when router responses lack credentials, and supported providers pass centralized environment-variable constants through declarations, router clients, and stream or embedding calls. ChangesCredential environment resolution
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Summary
A provider's API key set in the provider's own environment was never loaded: every provider declares
credential_env_var(e.g.ZAI_API_KEY) in itsProviderDeclaration, but the onlystd::env::varread happens inside llm-router's process duringrouter::provider::resolve(registry/resolve.rs). If the router runs under a different supervisor/environment than the provider, the key is present-but-unreachable and the provider failsNotConfigured.Fix: when the router resolves no credential, the provider now falls back to reading its own declared env var from its own process. Strictly subordinate — any router-resolved credential (config slice or router-process env) always wins, so nothing that works today changes.
Per commit:
with_api_key_fallback+ env-readingapply_credential_env_fallbackinprovider_scaffold/router_client.rs. InjectsCredential::ApiKeywithsource=Env,configured=trueonly when the router returnedcredential: Noneand the var is non-empty after trim (whitespace-only keys are rejected — stricter than the router's ownis_empty()check).pub const CREDENTIAL_ENV_VAR, reused bydeclaration()and the resolve call sites (single source of truth).scaffold::resolve+ScaffoldCache::resolvegain a trailingcredential_env_var: Option<&str>and apply the fallback before returning/caching. All call sites wired atomically: anthropic/zai/openai/xai/llamacpp passSome(CREDENTIAL_ENV_VAR)on both hot (stream_fn,embed) and discovery paths (via each crate'srouter_clientwrapper, whose external 2-arg signature is unchanged); openai-codex passesNone(OAuth-only); kimi's inlined resolve wraps its response with the shared helper. llamacpp's no-key local flow is untouched (unset var = no-op).Env vars:
ANTHROPIC_API_KEY,ZAI_API_KEY,MOONSHOT_API_KEY,OPENAI_API_KEY,XAI_API_KEY,LLAMACPP_API_KEY.Out of scope (unchanged): router-side resolve precedence,
CredentialSourceprotocol enum (reusesEnv), OAuth handling, version bumps.Test Coverage
Per-crate (each crate is its own standalone Cargo workspace; built+tested from its own dir):
llm-router 94 lib (+13/+4 int); anthropic 83; zai 62; kimi 61; openai 78; xai 75; llamacpp 63; openai-codex 56 — all green, plus each crate's doc/integration suites.
Known pre-existing failure (not this PR):
provider-llamacppintegrationprovider_registers_with_persisted_token_and_discovers_catalog_without_a_credential(catalog count 0 vs 1) reproduces identically at the pre-branch commit in an isolated worktree; the fallback wiring lands only in the final commit, after the failure already exists.Pre-Landing Review
Per-task spec+quality reviews (all approved) plus a final whole-branch review: verdict ready to merge, no Critical/Important findings. Deferred minors: (1) the no-credential integration tests are now coupled to ambient env — a CI shell exporting e.g.
LLAMACPP_API_KEYcould flip them (suggest hermeticremove_varin a follow-up); (2) env-injected keys ride the existing 30s resolve-cache TTL (matches current semantics; informational).Summary by CodeRabbit
New Features
Bug Fixes