Skip to content

fix(llm): honor base_url on the Anthropic provider path (#780) - #888

Merged
frankbria merged 3 commits into
mainfrom
fix/780-anthropic-base-url
Jul 24, 2026
Merged

fix(llm): honor base_url on the Anthropic provider path (#780)#888
frankbria merged 3 commits into
mainfrom
fix/780-anthropic-base-url

Conversation

@frankbria

Copy link
Copy Markdown
Owner

Closes #780 (remaining scope after #860 fixed the model override).

Problem

get_provider("anthropic", base_url=...) silently dropped base_url, so a configured llm.base_url (proxy/gateway deployments) never reached the Anthropic SDK.

Change

  • AnthropicProvider accepts base_url and passes it to all three SDK client construction sites (sync Anthropic, both AsyncAnthropic paths).
  • get_provider("anthropic", ...) forwards base_url instead of dropping it.
  • Env-leak guard: resolve_llm_settings previously fell back to the ambient OPENAI_BASE_URL env var for every provider — harmless only because the anthropic branch dropped it. Now that base_url is honored, that fallback is gated to OpenAI-compatible providers (openai, ollama, vllm, compatible) so an ambient OPENAI_BASE_URL cannot silently redirect Anthropic traffic. An explicit llm.base_url in .codeframe/config.yaml still applies to any provider.
  • Renamed _OPENAI_COMPATIBLEOPENAI_COMPATIBLE_PROVIDERS (now shared with core.llm_resolution); updated the one demo-doc reference.

Tests

tests/core/test_llm_resolution.py:

  • test_anthropic_base_url_override_is_honored — asserts the value reaches the SDK client itself, guarding against a silent re-drop
  • test_openai_base_url_env_does_not_leak_to_anthropic
  • test_config_base_url_applies_to_anthropic
  • test_openai_base_url_env_applies_to_openai_compatible (regression)

Third-party review (pre-PR)

opencode (GLM) reviewed the diff: no Critical/Major findings. Both Minor findings (dangling _OPENAI_COMPATIBLE reference in demo-pr552.md, stale docstring in llm_resolution.py) are fixed in this PR.

Known limitations

Pre-existing call sites that bypass resolve_llm_settings still won't pick up a configured base_url (deliberate defaults, out of scope here): core/tasks.py get_provider() fallback, ui/routers/session_chat_ws.py, and the legacy api_key branches in prd_discovery.py / streaming_chat.py. Candidate follow-up under the #861 migration pattern.

- AnthropicProvider accepts base_url and passes it to all three SDK
  client construction sites (sync + both async)
- get_provider('anthropic', base_url=...) forwards it instead of
  silently dropping it
- resolve_llm_settings: the OPENAI_BASE_URL env fallback now applies
  to OpenAI-compatible providers only, so an ambient value cannot
  redirect Anthropic traffic; explicit config llm.base_url still
  applies to any provider
- rename _OPENAI_COMPATIBLE -> OPENAI_COMPATIBLE_PROVIDERS (shared
  with core.llm_resolution)
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Precision bug-hunting review in progress

Scope: concrete defects only (logic errors, security, data loss, breaking contracts, leaks) — style/architecture/coverage handled by the general review.

  • Gather PR diff + existing comments
  • Trace base_url flow through AnthropicProvider, get_provider, resolve_llm_settings
  • Verify env-leak guard (OPENAI_BASE_URL gating)
  • Confirm or discard candidate findings against real callers/callees
  • Post inline findings + summary

View job run

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Precision bug-hunting review in progress

Scope: concrete defects only (logic errors, security, data loss, breaking contracts, leaks) — style/architecture/coverage handled by the general review.

  • Gather PR diff + existing comments
  • Trace base_url flow through AnthropicProvider, get_provider, resolve_llm_settings
  • Verify env-leak guard (OPENAI_BASE_URL gating)
  • Confirm or discard candidate findings against real callers/callees
  • Post inline findings + summary

View job run

@frankbria

Copy link
Copy Markdown
Owner Author

Third-party review (post-PR, opencode/GLM) — APPROVE

Second-pass review of the final diff:

  • Both minors from the pre-PR pass verified fixed (demo-doc rename reference, llm_resolution.py docstring).
  • Rename is clean repo-wide; base_url threading verified through provider_kwargs → create_provider → get_provider → AnthropicProvider and all three SDK client construction sites; None falls back to the SDK default.
  • Security-positive: gating OPENAI_BASE_URL to OpenAI-compatible providers closes a credential-routing hole (an ambient value could previously have redirected Anthropic calls + ANTHROPIC_API_KEY to an arbitrary endpoint once base_url was honored).
  • Informational, no action: the legacy direct AnthropicProvider(api_key=...) paths in streaming_chat.py / prd_discovery.py deliberately keep their pre-[P2.27] Route remaining direct provider constructions through the shared llm_resolution chain #861 contract and fall back to the honoring resolution chain when no api_key is passed — tracked as a follow-up in the PR body.

Demo evidence: demo-pr888.md (override reaches SDK client; env-leak guard; config passthrough; suite green).

… OPENAI_BASE_URL env is OpenAI-compatible only)
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @frankbria's task in 2m 56s —— View job


Precision bug-hunting review — complete

Scope: concrete defects only (logic errors, security, data loss, breaking contracts, leaks) — style/architecture/coverage handled by the general review.

  • Gather PR diff + existing comments
  • Trace base_url flow through AnthropicProvider, get_provider, resolve_llm_settings
  • Verify env-leak guard (OPENAI_BASE_URL gating)
  • Confirm or discard candidate findings against real callers/callees
  • Post inline findings + summary

✅ GLM review: no defects found.

I traced the full chain and confirmed the implementation is correct:

base_url threading — correct. resolve_llm_settingsLLMSettings.provider_kwargs() (llm_resolution.py:45-46) → create_providerget_provider (__init__.py:105) → AnthropicProvider.__init__ (anthropic.py:71). All three SDK client construction sites receive it:

  • sync Anthropicanthropic.py:81
  • async_complete AsyncAnthropicanthropic.py:158
  • async_stream AsyncAnthropicanthropic.py:214

(complete/stream reuse the lazy sync client, so they're covered by line 81.) base_url=None falls through to the SDK default, as intended — no None-deref.

Env-leak guard — correct and behavior-preserving. llm_resolution.py:91-93 now applies the OPENAI_BASE_URL env tier only when provider_type in OPENAI_COMPATIBLE_PROVIDERS, while explicit config llm.base_url still applies to any provider. The gate set (openai, ollama, vllm, compatible) is identical to the routing set in get_provider (__init__.py:78), so a provider is env-eligible iff it is routed to OpenAIProvider — no provider slips through either side. For OpenAI-compatible providers the change is equivalent to the old config OR env; for anthropic/mock it now correctly excludes the ambient value.

No residual leak — security claim verified. Grep confirms OPENAI_BASE_URL is read in exactly two places, both gated. The legacy bypass sites named in the PR body construct AnthropicProvider(api_key=...) with no base_url (prd_discovery.py:258, streaming_chat.py:195) and read no env var, so an ambient OPENAI_BASE_URL cannot reach the Anthropic SDK through them either → ANTHROPIC_API_KEY can't be silently redirected. No other code path feeds OPENAI_BASE_URL to the Anthropic provider.

Rename — clean. Zero remaining references to _OPENAI_COMPATIBLE anywhere in the repo. No circular-import risk (both cross-package imports in resolve_llm_settings are function-local).

No inline findings to post.

@frankbria
frankbria merged commit bb65eb7 into main Jul 24, 2026
10 of 11 checks passed
@frankbria
frankbria deleted the fix/780-anthropic-base-url branch July 24, 2026 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P3.9] get_provider() silently drops model/base_url overrides for the Anthropic provider

1 participant