fix(aux-client): honor api_mode in explicit_base_url path; read key_env in task config (#16254) - #16256
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes auxiliary client configuration handling so api_mode and key_env are honored when auxiliary tasks use an explicit per-task base_url (the “explicit_base_url/path 3” code path).
Changes:
- Updated
resolve_provider_client()to routeexplicit_base_url + api_mode="anthropic_messages"through the Anthropic Messages client wrapper (instead of always building an OpenAI-wire client). - Updated
_resolve_task_provider_model()to resolvekey_envfrom the environment whenapi_keyis not provided inauxiliary.<task>config. - Added a focused test module covering the path 3 Anthropic dispatch and
key_envbehavior/precedence.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
agent/auxiliary_client.py |
Honors api_mode="anthropic_messages" for explicit_base_url custom endpoints; resolves key_env in task config. |
tests/agent/test_aux_client_path3_anthropic.py |
Regression tests for path 3 Anthropic routing and key_env resolution/precedence. |
Comments suppressed due to low confidence (1)
agent/auxiliary_client.py:1706
- The updated docstring still documents api_mode as only {"chat_completions", "codex_responses", None}, but this function now (and elsewhere in the file) supports "anthropic_messages" as well. Please update the api_mode docstring to include anthropic_messages so callers configuring explicit_base_url aren’t misled about supported values.
explicit_base_url: Optional direct endpoint URL. The SDK client
selected is determined by api_mode (OpenAI, Anthropic, Codex).
explicit_api_key: Optional API key paired with explicit_base_url.
api_mode: API mode override. One of "chat_completions",
"codex_responses", or None (auto-detect). When set to
"codex_responses", the client is wrapped in
CodexAuxiliaryClient to route through the Responses API.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| from agent.anthropic_adapter import build_anthropic_client | ||
| real_client = build_anthropic_client(custom_key, _clean_base) | ||
| except ImportError: |
There was a problem hiding this comment.
In the explicit_base_url + api_mode="anthropic_messages" branch, query parameters are stripped into _dq but then discarded for the Anthropic client: build_anthropic_client() is called with _clean_base and the wrapper stores _clean_base. This diverges from _try_custom_endpoint() (which passes the original custom_base) and can break endpoints that require query params (e.g. Azure api-version) or any custom query-based routing. Consider preserving the original URL semantics for Anthropic mode (either pass the original URL consistently, or plumb the extracted query into the Anthropic client creation).
|
@copilot Finding addressed in commit The inline comment correctly identified that Fixed by passing Added a regression test ( |
…nv in task config (NousResearch#16254) Two sibling bugs in agent/auxiliary_client.py left the explicit_base_url code path (path 3) broken for non-OpenAI API formats. **Bug 1 — path 3 ignores api_mode: anthropic_messages** resolve_provider_client() has three independent branches that all do the same thing: given a (base_url, api_key, api_mode), construct the right SDK client. - Path 1 (_try_custom_endpoint, ~L1177): fixed by NousResearch#7648 - Path 2 (named custom providers, ~L1895): fixed by NousResearch#15059 - Path 3 (explicit_base_url branch, ~L1850): still broken — always built a plain OpenAI client and never checked api_mode Result: when auxiliary.<task> config has base_url + api_mode: anthropic_messages, the code sends an Anthropic-format body to <base_url>/chat/completions (the OpenAI endpoint) instead of /v1/messages, producing 404 invalid_model or 403. Fix: add the same api_mode == "anthropic_messages" dispatch that path 2 has, mirroring lines 1895-1914 exactly. Updated the docstring to remove the hardcoded "OpenAI-compatible" assumption. **Bug 2 — key_env silently dropped from auxiliary task configs** _resolve_task_provider_model() parsed api_key from task_config but had no equivalent for key_env. Contrast with _get_named_custom_provider() (path 2) which correctly resolves key_env via os.getenv(). Result: auxiliary.<task> configs that specify key_env instead of api_key always send api_key="no-key-required", producing 403 access_denied even when the env var is set. Fix: add the same two-line key_env resolution that path 2 uses. Before: test_explicit_base_url_anthropic_messages_returns_anthropic_client asserts isinstance(client, AnthropicAuxiliaryClient) → FAIL (got OpenAI) After: 7 new tests all pass; existing 95 auxiliary client tests unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
build_anthropic_client already handles query params internally (e.g. routing Azure api-version via default_query rather than appending to base_url). Passing _clean_base instead of custom_base dropped those params before the function could process them, breaking Azure-style endpoints that embed api-version in the URL. Path 2 (named custom providers, line 1898) has always passed custom_base correctly; this aligns path 3 (explicit_base_url) to match. Adds a regression test that verifies the original URL (including query params) reaches build_anthropic_client. Addresses Copilot inline finding on PR NousResearch#16256. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
9164446 to
2875e34
Compare
|
Closing to keep the queue clean — happy to reopen if this is still useful. |
Summary
agent/auxiliary_client.pywhereapi_mode: anthropic_messageswas silently ignored whenexplicit_base_urlwas setkey_envbeing silently dropped fromauxiliary.<task>configs, causing 403 errors even when the env var was setThe bugs
resolve_provider_client()has three independent branches that each construct an SDK client from(base_url, api_key, api_mode):api_modedispatch_try_custom_endpoint(~L1177)OPENAI_BASE_URLenvif custom_mode == "anthropic_messages":provider: myrelay+providers.myrelaydictif entry_api_mode == "anthropic_messages":auxiliary.<task>.base_urlflowed viaexplicit_base_urlPath 3 result: Anthropic-format body sent to
<base_url>/chat/completions(OpenAI endpoint) instead of/v1/messages, returning404 invalid_modelor403.Bug 2 —
_resolve_task_provider_model()parsedapi_keyfrom task config but had no equivalent forkey_env. Result: auxiliary task configs that specifykey_envalways sentapi_key="no-key-required", producing 403 even when the env var was set. (Contrast with path 2's_get_named_custom_provider()which correctly readskey_env.)The fix
Path 3: added the same
if api_mode == "anthropic_messages":dispatch block that path 2 has at L1895-1914 (lines 1856-1878 here). Updated theexplicit_base_urldocstring to remove the hardcoded "OpenAI-compatible" assumption.key_env: added two-line
os.getenv(_cfg_key_env)resolution in_resolve_task_provider_model, mirroring_get_named_custom_provider.Test plan
test_explicit_base_url_anthropic_messages_returns_anthropic_clientfails (isinstance(client, AnthropicAuxiliaryClient)→ False, got OpenAI-wire client)test_resolve_task_provider_model_reads_key_envconfirms env var is now resolved;test_resolve_task_provider_model_api_key_wins_over_key_envconfirms precedenceRelated / Positioning
755a2804explicit_base_urlin ACP sessions specifically; this PR addressesauxiliary.<task>configs via_resolve_task_provider_model→resolve_provider_clientwithexplicit_base_url🤖 Generated with Claude Code