Uh oh!
There was an error while loading. Please reload this page.
fix(providers): forward YAML api_key to Claude provider (custom endpoints) - #393
Conversation
4a6a125 to
97db2a5Compare
Jason Robert (jrob5756)
left a comment
There was a problem hiding this comment.
I took a look at the branch. The four-line factory change is right, and it closes a real silent drop: the schema accepted api_key for name: claude and the factory threw it away. I also checked that api_key was already threaded through _initialize_client, _resolve_anthropic_model, and build_agent, so nothing downstream needed touching.
Most of my comments are on the docs and the example, and they share one root cause. The PR description says the Anthropic SDK prefers auth_token when both credentials are set. It doesn't. auth_headers returns {**self._api_key_auth, **self._bearer_auth}, so both X-Api-Key and Authorization: Bearer go out on every request. The SDK also stops reading credential env vars entirely once either credential is passed explicitly, which is what breaks the new example's documented happy path.
Two things that had nowhere to sit inline:
- No CHANGELOG entry. Five of the last six non-release commits on main touch it, and this changes YAML behavior a user can observe.
- Nothing in the suite asserts that a credential reaches the Anthropic client. I mutated
_pydantic_ai/agent_builder.py:112to dropapi_keyand resend it as a bearer token, and all 4831 unit tests still passed.test_pydantic_ai_agent_builder.pyalready hastest_auth_token_reaches_client; anapi_keytwin assertingclient.auth_headers == {"X-Api-Key": ...}would close the header-swap gap.
One more for context, since this PR is what documents the gateway pattern. _resolve_anthropic_model (agent_builder.py:97-98) does its own env lookup and then passes both credentials unconditionally, while _initialize_client passes them conditionally. So with this example plus an ambient ANTHROPIC_API_KEY, validate_connection() sends only the bearer token, but every real model call also sends the user's Anthropic key to the gateway. That predates this PR and is probably out of scope, though the new docs shouldn't describe the path as safe.
| provider: | ||
| name: claude | ||
| base_url: ${ANTHROPIC_BASE_URL:-https://api.anthropic.com} | ||
| auth_token: ${ANTHROPIC_AUTH_TOKEN:-placeholder-token} |
There was a problem hiding this comment.
This default can't work. ${ANTHROPIC_AUTH_TOKEN:-placeholder-token} always resolves to something, and the Anthropic SDK stops reading credential env vars as soon as either credential is passed explicitly. I loaded this file with ANTHROPIC_API_KEY exported and got api_key=None with headers {'Authorization': 'Bearer placeholder-token'}. That is a 401 against any real endpoint while the user's valid key sits unused, and the error tells them to check the credentials that were fine.
copilot-local-llm.yaml gets away with ${OPENAI_API_KEY:-ollama} because Ollama ignores the key. A placeholder bearer token fails everywhere.
Since base_url already defaults to stock Anthropic, api_key is the credential that matches the default endpoint, and it demonstrates the field this PR actually fixes. Right now that field only appears in a comment block at the bottom of the file.
| auth_token: ${ANTHROPIC_AUTH_TOKEN:-placeholder-token} | |
| api_key: ${ANTHROPIC_API_KEY:-placeholder-key} |
There was a problem hiding this comment.
Fixed in 5ea76e5. The example now declares api_key: ${ANTHROPIC_API_KEY:-placeholder-key} against the default Anthropic endpoint, and the bearer-token variant moved to the commented gateway block at the bottom of the file, where it pairs with a gateway base_url.
| name: claude | ||
| base_url: ${ANTHROPIC_BASE_URL:-https://api.anthropic.com} | ||
| auth_token: ${ANTHROPIC_AUTH_TOKEN:-placeholder-token} | ||
| default_model: claude-sonnet-4 |
There was a problem hiding this comment.
claude-sonnet-4 is the Copilot catalog name (schema.py:856). The Anthropic REST IDs are claude-sonnet-4-5 and claude-sonnet-4-20250514. This example talks to the Anthropic API directly, so it 404s on first run, and conductor validate won't catch it because there is no model check. The claude.md snippets in this same PR use 4.5.
| default_model: claude-sonnet-4 | |
| default_model: claude-sonnet-4-5 |
There was a problem hiding this comment.
Fixed in 5ea76e5 — default_model: claude-sonnet-4-5.
| # Claude provider with a custom / bring-your-own endpoint | ||
| # | ||
| # Demonstrates structured ``runtime.provider`` configuration for the Claude | ||
| # provider (issue #353). Use this when you need to route Anthropic Claude API |
There was a problem hiding this comment.
Issue #353 is "MCP cleanup exits AnyIO cancel scopes from a different asyncio task", which is unrelated to provider config. copilot-local-llm.yaml:3 uses this convention correctly, so someone will eventually follow the number and land on a closed AnyIO bug. Point it at the issue this PR closes, or drop the parenthetical.
| # provider (issue #353). Use this when you need to route Anthropic Claude API | |
| # provider. Use this when you need to route Anthropic Claude API |
There was a problem hiding this comment.
Fixed in 5ea76e5 — the parenthetical is dropped.
| # | ||
| # Usage: | ||
| # | ||
| # export ANTHROPIC_AUTH_TOKEN=sk-ant-... |
There was a problem hiding this comment.
Pairs with the swap suggested below. sk-ant-... is the shape of an Anthropic console key, which belongs in x-api-key. As written, this tells people to send their API key as a bearer token to api.anthropic.com, which rejects it.
| # export ANTHROPIC_AUTH_TOKEN=sk-ant-... | |
| # export ANTHROPIC_API_KEY=sk-ant-... |
There was a problem hiding this comment.
Fixed in 5ea76e5 — the usage line now exports ANTHROPIC_API_KEY=sk-ant-....
| # Alternative BYOK variant using ``api_key`` instead of ``auth_token`` (commented out): | ||
| # | ||
| # runtime: | ||
| # provider: | ||
| # name: claude | ||
| # base_url: ${ANTHROPIC_BASE_URL:-https://api.anthropic.com} | ||
| # api_key: ${ANTHROPIC_API_KEY:-placeholder-key} | ||
| # default_model: claude-sonnet-4 |
There was a problem hiding this comment.
Keeping a commented alternative in an example file is fine, and there is precedent for it. Two mechanical problems though: this block sits after the top-level output: and is indented as if it were top-level, so uncommenting it verbatim produces an invalid top-level runtime: key. It is a replacement for lines 26-31, and saying so would save someone a puzzled minute.
If you take the swap above, this is also where the bearer token belongs, next to a base_url that is actually a gateway:
| # Alternative BYOK variant using ``api_key`` instead of ``auth_token`` (commented out): | |
| # | |
| # runtime: | |
| # provider: | |
| # name: claude | |
| # base_url: ${ANTHROPIC_BASE_URL:-https://api.anthropic.com} | |
| # api_key: ${ANTHROPIC_API_KEY:-placeholder-key} | |
| # default_model: claude-sonnet-4 | |
| # Gateway variant: to authenticate against a bearer-token proxy (LiteLLM, | |
| # Databricks AI Gateway), replace the ``runtime:`` block above (lines 26-31) | |
| # with the following. Note that supplying auth_token means ANTHROPIC_API_KEY | |
| # is ignored. | |
| # | |
| # runtime: | |
| # provider: | |
| # name: claude | |
| # base_url: ${ANTHROPIC_BASE_URL:-https://gateway.example.com/v1} | |
| # auth_token: ${ANTHROPIC_AUTH_TOKEN:-placeholder-token} | |
| # default_model: claude-sonnet-4-5 |
There was a problem hiding this comment.
Fixed in 5ea76e5 — your suggested gateway-variant block is in, including the note that it replaces lines 26-31 and that supplying auth_token means ANTHROPIC_API_KEY is ignored.
| ### Configuration Rules and Precedence | ||
| - **YAML precedence over environment variables**: Setting a field in YAML overrides its corresponding environment variable (`api_key` in YAML overrides `ANTHROPIC_API_KEY`, `auth_token` overrides `ANTHROPIC_AUTH_TOKEN`, and `base_url` overrides `ANTHROPIC_BASE_URL`). |
There was a problem hiding this comment.
These three overrides are not independent. The SDK puts all credential env resolution behind a single flag: if either api_key or auth_token is passed, it reads neither ANTHROPIC_API_KEY nor ANTHROPIC_AUTH_TOKEN. I checked with both env vars exported, and a YAML auth_token alone leaves api_key as None. Only base_url behaves per-field.
This is the rule behind the broken example, so it is worth stating outright.
| -**YAML precedence over environment variables**: Setting a field in YAML overrides its corresponding environment variable (`api_key` in YAML overrides `ANTHROPIC_API_KEY`, `auth_token` overrides `ANTHROPIC_AUTH_TOKEN`, and `base_url` overrides `ANTHROPIC_BASE_URL`). | |
| -**`base_url` precedence**: YAML `base_url`overrides `ANTHROPIC_BASE_URL`; when omitted, the env var is used. | |
| -**Credential precedence**: `api_key` and `auth_token` are resolved together, not independently. Setting **either** in YAML makes the Anthropic SDK skip environment-variable credential resolution entirely, so a YAML `auth_token` also suppresses `ANTHROPIC_API_KEY`, and vice versa. If you set one credential in YAML and expect the other from the environment, it resolves to `None` with no warning. |
There was a problem hiding this comment.
Fixed in 5ea76e5 — your suggested wording is in: base_url falls back per-field, while api_key/auth_token resolve as a unit (setting either in YAML suppresses both env vars). The code now matches the doc too: _resolve_anthropic_model previously did its own independent env lookups, so a YAML auth_token plus an ambient ANTHROPIC_API_KEY would send both headers on real model calls. It now mirrors the SDK's unit semantics, so the documented rule holds for actual requests, not just validate_connection().
| ### Configuration Rules and Precedence | ||
| - **YAML precedence over environment variables**: Setting a field in YAML overrides its corresponding environment variable (`api_key` in YAML overrides `ANTHROPIC_API_KEY`, `auth_token` overrides `ANTHROPIC_AUTH_TOKEN`, and `base_url` overrides `ANTHROPIC_BASE_URL`). | ||
| - **Authentication header selection**: Use `api_key` for standard Anthropic keys (`x-api-key` header). Use `auth_token` for gateways expecting bearer authentication (`Authorization: Bearer` header). If both `api_key` and `auth_token` are configured, Conductor passes both parameters directly to the Anthropic SDK, which handles client authorization. Pick the single option your endpoint requires; `auth_token` is designed for bearer proxies. |
There was a problem hiding this comment.
The SDK does not arbitrate. auth_headers is {**self._api_key_auth, **self._bearer_auth}, and I confirmed a client built with both set emits {'X-Api-Key': ..., 'Authorization': 'Bearer ...'}. Combined with a gateway base_url, that is the credential leak configuration.md:164 refuses to allow implicitly for Copilot.
configuration.md:205 is the house style for this: it names the mechanism and says a warning is logged. Worth matching.
| -**Authentication header selection**: Use `api_key` for standard Anthropic keys (`x-api-key` header). Use `auth_token` for gateways expecting bearer authentication (`Authorization: Bearer` header). If both `api_key` and `auth_token` are configured, Conductor passes both parameters directly to the Anthropic SDK, which handles client authorization. Pick the single option your endpoint requires; `auth_token` is designed for bearer proxies. | |
| -**Authentication header selection**: Use `api_key` for standard Anthropic keys (`x-api-key` header). Use `auth_token` for gateways expecting bearer authentication (`Authorization: Bearer` header). **Set exactly one.** If both are configured, the Anthropic SDK does not choose between them: it sends `X-Api-Key` and `Authorization: Bearer` on every request, so your Anthropic key reaches whatever `base_url` points at. Conductor forwards both without arbitrating. |
There was a problem hiding this comment.
Fixed in 5ea76e5 — the bullet now says to set exactly one, names the mechanism (SDK sends both X-Api-Key and Authorization: Bearer), and says a warning is logged, matching the configuration.md house style. The warning itself is implemented at both Anthropic client construction points (ClaudeProvider._initialize_client and _resolve_anthropic_model), including the env-only dual-credential case.
| |---|---| | ||
| | `base_url` | `ANTHROPIC_BASE_URL` | | ||
| | `api_key` | `ANTHROPIC_API_KEY` | | ||
| | `auth_token` | `ANTHROPIC_AUTH_TOKEN` | |
There was a problem hiding this comment.
Sitting directly under the Copilot table, whose rows genuinely are independent, this reads as the same kind of per-field chain. It isn't, and the visual parallel teaches the wrong model.
| |`auth_token`|`ANTHROPIC_AUTH_TOKEN`| | |
| |`auth_token`|`ANTHROPIC_AUTH_TOKEN`| | |
| Unlike the Copilot chains above, the two credential rows are not independent. | |
| The Anthropic SDK resolves them as a unit: set either one in YAML and it reads | |
| neither env var. Only `base_url` falls back on its own. |
There was a problem hiding this comment.
Fixed in 5ea76e5 — your suggested note is under the table.
| async def test_create_claude_provider_yaml_api_key_takes_precedence_over_env( | ||
| self, | ||
| mock_anthropic_module: Any, | ||
| mock_anthropic_class: Any, | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
| """When both YAML api_key and ANTHROPIC_API_KEY are set, YAML wins.""" | ||
| from unittest.mock import AsyncMock | ||
| monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-env") | ||
| mock_anthropic_module.__version__ = "0.77.0" | ||
| mock_client = MagicMock() | ||
| mock_client.models.list = AsyncMock(return_value=MagicMock(data=[])) | ||
| mock_client.close = AsyncMock(return_value=None) | ||
| mock_anthropic_class.return_value = mock_client | ||
| settings = ProviderSettings(name="claude", api_key=SecretStr("sk-yaml")) | ||
| provider = await create_provider( | ||
| "claude", | ||
| validate=False, | ||
| provider_settings=settings, | ||
| ) | ||
| assert isinstance(provider, ClaudeProvider) | ||
| assert provider._api_key == "sk-yaml" |
There was a problem hiding this comment.
This one can't test precedence. create_provider never reads ANTHROPIC_API_KEY, and AsyncAnthropic is patched out, so the setenv on line 202 has nothing to influence. I replaced it with delenv, the exact opposite of the test's premise, and it still passed. That makes it a behavioral duplicate of test_create_claude_provider_extracts_api_key_from_settings above.
The name and docstring are the problem more than the duplication: the PR description cites this test as proof of precedence, and the next person to touch this code will too. Asserting on the SDK call gives it something real to guard, including the wrong-header case:
| asyncdeftest_create_claude_provider_yaml_api_key_takes_precedence_over_env( | |
| self, | |
| mock_anthropic_module: Any, | |
| mock_anthropic_class: Any, | |
| monkeypatch: pytest.MonkeyPatch, | |
| ) ->None: | |
| """When both YAML api_key and ANTHROPIC_API_KEY are set, YAML wins.""" | |
| fromunittest.mockimportAsyncMock | |
| monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-env") | |
| mock_anthropic_module.__version__="0.77.0" | |
| mock_client=MagicMock() | |
| mock_client.models.list=AsyncMock(return_value=MagicMock(data=[])) | |
| mock_client.close=AsyncMock(return_value=None) | |
| mock_anthropic_class.return_value=mock_client | |
| settings=ProviderSettings(name="claude", api_key=SecretStr("sk-yaml")) | |
| provider=awaitcreate_provider( | |
| "claude", | |
| validate=False, | |
| provider_settings=settings, | |
| ) | |
| assertisinstance(provider, ClaudeProvider) | |
| assertprovider._api_key=="sk-yaml" | |
| asyncdeftest_create_claude_provider_forwards_api_key_to_sdk_client( | |
| self, | |
| mock_anthropic_module: Any, | |
| mock_anthropic_class: Any, | |
| ) ->None: | |
| """The YAML api_key reaches the Anthropic client as api_key, not as a bearer token.""" | |
| fromunittest.mockimportAsyncMock | |
| mock_anthropic_module.__version__="0.77.0" | |
| mock_client=MagicMock() | |
| mock_client.models.list=AsyncMock(return_value=MagicMock(data=[])) | |
| mock_client.close=AsyncMock(return_value=None) | |
| mock_anthropic_class.return_value=mock_client | |
| settings=ProviderSettings(name="claude", api_key=SecretStr("sk-yaml")) | |
| provider=awaitcreate_provider( | |
| "claude", | |
| validate=False, | |
| provider_settings=settings, | |
| ) | |
| assertisinstance(provider, ClaudeProvider) | |
| assertmock_anthropic_class.call_args.kwargs["api_key"] =="sk-yaml" | |
| assert"auth_token"notinmock_anthropic_class.call_args.kwargs |
There was a problem hiding this comment.
Fixed in 5ea76e5 — replaced with your suggested test asserting the SDK call kwargs (call_count == 1, kwargs['api_key'] == 'sk-yaml', 'auth_token' not in kwargs). Also added an api_key twin in test_pydantic_ai_agent_builder.py asserting the exact auth_headers surface ({'X-Api-Key': ...}), plus a test that an explicit auth_token suppresses an ambient ANTHROPIC_API_KEY — I asserted auth_headers rather than only client.auth_token because the bug is observable at the HTTP header layer, and this catches the ambient-env leak that a property-only assertion would miss. The existing test_auth_token_reaches_client also now cleans up the autouse fixture's ANTHROPIC_API_KEY and asserts exact auth_headers.
| if provider_settings.api_key is not None: | ||
| claude_api_key = provider_settings.api_key.get_secret_value() |
There was a problem hiding this comment.
The unwrap itself matches the auth_token lines above it and the hermes branch below, so no complaint there.
Worth considering the parity angle though. copilot.py:513-519 warns when api_key and bearer_token both resolve, so the precedence is visible instead of silent. This PR makes the same collision reachable on Claude for the first time, since api_key was previously dropped and the two could never meet at the SDK. The Claude consequence is the worse of the two: Copilot's SDK ignores the loser, Anthropic's sends both headers, so the credential the user didn't intend actually reaches base_url.
factory.py has no module logger, so ClaudeProvider.__init__ in claude.py is probably the better home for the check, guarding on api_key is not None and auth_token is not None.
There was a problem hiding this comment.
Done, with one adjustment to the suggested home. There are two real Anthropic client construction seams — ClaudeProvider._initialize_client (validation/model-metadata client) and _resolve_anthropic_model (per-agent model calls) — so the warning lives at both, in 5ea76e5 and 55d3bdf, rather than generically in __init__. That also catches the case where __init__ sees only one credential but the effective runtime pair collides. Kept it a warning rather than a ValidationError to match the Copilot parity you referenced and to avoid a breaking behavior change beyond the review scope. The ProviderSettings.auth_token docstring is corrected as well (no more "instead of x-api-key").
hertznsk
commented
Aug 10, 2026
Thanks for the thorough review — the SDK behavior corrections (both-headers, unit credential resolution) were exactly right, and verifying them against the installed SDK changed the shape of the fix. Addressed in 5ea76e5, 55d3bdf, and 705438e: 1. CHANGELOG — entry added under Unreleased → Fixed covering the 2. Header-level test — added 3. The Full suite green (4980 passed on the rebased base), |
hertznsk
commented
Aug 10, 2026
One small follow-up in 32b5cd7, spotted while re-checking the gateway snippets: the Anthropic SDK appends |
…, and tests - Example: use api_key with placeholder against the default Anthropic endpoint, claude-sonnet-4-5 model id, drop the wrong issue reference, move the bearer-token gateway variant to the commented block - Docs: credentials resolve as a unit (either YAML credential suppresses both env vars); the SDK sends both headers when both are set — set exactly one; lead snippet shows a single credential - agent_builder: mirror SDK unit semantics so an explicit credential never mixes with an ambient env credential (credential-leak fix), and warn when both credentials are effectively set - claude provider: warn when both credentials reach the SDK client (parity with the Copilot provider's api_key/bearer_token warning) - Tests: assert the YAML api_key reaches AsyncAnthropic as api_key; add an api_key header twin and an ambient-env-suppression test; fix the auth_token test's env leakage; warning coverage on both client paths - CHANGELOG entry
The SDK resolves ANTHROPIC_API_KEY/ANTHROPIC_AUTH_TOKEN internally when neither credential is passed explicitly, so an env-only dual setup would otherwise send both headers during validate_connection() before any agent-execution warning could fire.
…teways The Anthropic SDK appends /v1/messages itself, so a base_url ending in /v1 produces /v1/v1/messages on LiteLLM-style gateways. The gateway snippets now show base_url without the prefix, and both the example's commented block and claude.md explain when the prefix is required (e.g. Databricks AI Gateway).
32b5cd7 to
8a4e387CompareCodecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## main #393 +/- ##
=======================================
Coverage ? 91.32% =======================================
Files ? 108 Lines ? 17479 Branches ? 0 =======================================
Hits ? 15963 Misses ? 1516 Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Jason Robert (jrob5756)
left a comment
There was a problem hiding this comment.
LGTM. Approved!
Uh oh!
There was an error while loading. Please reload this page.
Summary
Enables the Claude provider to accept a YAML-declared
api_keyin structuredruntime.providerconfig, completing the custom-endpoint story forname: claude. Previouslybase_urlandauth_tokenwere accepted by the schema butapi_keywas silently dropped at the factory boundary, forcing users to rely solely on theANTHROPIC_API_KEYenv var.fix(providers):ProviderRegistrynow forwardssettings.api_keytoClaudeProvider, so a YAMLapi_keyreaches the Anthropic client instead of being dropped.fix(providers):_resolve_anthropic_modelnow resolves credentials as a unit, mirroring the Anthropic SDK — setting either credential in YAML suppresses bothANTHROPIC_API_KEYandANTHROPIC_AUTH_TOKEN. Previously a YAMLauth_tokenstill let an ambientANTHROPIC_API_KEYride along, and the SDK sent bothX-Api-KeyandAuthorization: Bearerheaders to whateverbase_urlpointed at (a credential leak against gateway endpoints).fix(providers): when both credentials are effectively set, Conductor now logs a warning naming the both-headers-sent behavior (parity with the Copilot provider'sapi_key/bearer_tokenwarning), at both Anthropic client construction points.test(config): schema-level regression tests lock the accepted structured fields forclaude,SecretStrredaction inmodel_dump(mode="json"),api_key+auth_tokencoexistence, and rejection of empty-string secrets. Factory tests assert the YAMLapi_keyreachesAsyncAnthropicasapi_key; agent-builder tests assert the exactauth_headerssurface for each credential, including ambient-env suppression.docs(providers)/docs(configuration): custom-endpoint and gateway-auth documentation for Claude, including the credential-unit resolution rule and the "set exactly one" header-selection rule; the stale "not yet implemented" claim indocs/configuration.mdis corrected with a Copilot-vs-Claude field compatibility table.docs(examples): newexamples/claude-custom-endpoint.yaml(env-interpolated with:-placeholderdefaults somake validate-examplespasses with no env set) + README row.When both
api_keyandauth_tokenare set, the Anthropic SDK does not arbitrate: it sends bothX-Api-KeyandAuthorization: Beareron every request. Conductor forwards both without arbitrating and logs a warning; the docs direct users to set exactly one.Test plan
make check(ruff + ty) — cleanmake test— 5278 passed, 47 skippeduv run pytest tests/test_config/test_provider_settings.py tests/test_providers/test_factory.py tests/test_providers/test_claude_parameter_passing.py— passesconductor validate examples/claude-custom-endpoint.yamlin a clean env — passesapi_keyreachesAsyncAnthropicasapi_key(and not asauth_token)auth_headers-level tests proving an explicit credential never mixes with an ambient env credentialANTHROPIC_BASE_URL+ANTHROPIC_API_KEYagainst a real gateway) — deferred to PR review; no out-of-band credentials available in the authoring environmentNotes
_resolve_anthropic_modeland the dual-credential warnings.bearer_token(Copilot) vsauth_token(Claude) naming divergence — a non-breakingbearer_tokenalias for Claude was identified as a possible future improvement.