Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 38 additions & 6 deletions agent/auxiliary_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
``auxiliary.<task>.provider`` only. HTTP 402 in call_llm() falls through the chain.
"""

import asyncio
import contextlib
import contextvars
import functools
Expand Down Expand Up @@ -4283,7 +4284,16 @@ def _to_async_client(sync_client, model: str, is_vision: bool = False):
if _client_declares(sync_client, "HERMES_SKIP_ASYNC_WRAP"):
return sync_client, model
sync_base_url = str(sync_client.base_url)
async_kwargs = {"api_key": sync_client.api_key, "base_url": sync_base_url}
# A callable api_key lives in the SDK's _api_key_provider (client.api_key is "" then);
# rebuilding with the empty string silently drops Authorization → every async aux call
# 401s. The async SDK awaits the provider, so bridge the (blocking) sync mint via a thread.
_key_provider = getattr(sync_client, "_api_key_provider", None)
if callable(_key_provider):
async def _async_key_provider(_provider=_key_provider):
return await asyncio.to_thread(_provider)
async_kwargs = {"api_key": _async_key_provider, "base_url": sync_base_url}
else:
async_kwargs = {"api_key": sync_client.api_key, "base_url": sync_base_url}
if base_url_host_matches(sync_base_url, "openrouter.ai"):
headers = _apply_user_default_headers(build_or_headers())
elif _is_official_codex_base_url(sync_base_url):
Expand Down Expand Up @@ -4577,8 +4587,14 @@ def _resolve_custom_branch(req: _ResolveRequest) -> _ResolveResult:
if req.api_mode == "anthropic_messages":
wrap_base = (req.explicit_base_url or "").strip().rstrip("/")
custom_key = (
(req.explicit_api_key or "").strip()
or _scoped_key_env("OPENAI_API_KEY")
# A callable (key_cmd token source) must reach the client uncalled:
# .strip() raises AttributeError (#88667) and str() would send the
# object repr as the bearer (#104460's class).
req.explicit_api_key
if callable(req.explicit_api_key)
else (req.explicit_api_key or "").strip()
) or (
_scoped_key_env("OPENAI_API_KEY")
or _read_main_api_key_if_same_host(custom_base)
or "no-key-required" # local servers don't need auth
)
Expand All @@ -4591,7 +4607,11 @@ def _resolve_custom_branch(req: _ResolveRequest) -> _ResolveResult:
# Re-resolution loses the provider name and falls back to OpenRouter or a wrong API-key provider —
# the main agent already solved this, we just need to reuse its answer. (#45472)
_main_base = str(main_runtime.get("base_url") or "").strip().rstrip("/")
_main_key = str(main_runtime.get("api_key") or "").strip()
_main_key = (
main_runtime.get("api_key")
if callable(main_runtime.get("api_key"))
else str(main_runtime.get("api_key") or "").strip()
)
if _main_base and _main_key:
custom_base, custom_key = _main_base, _main_key
if custom_base and custom_key:
Expand Down Expand Up @@ -4652,7 +4672,13 @@ def _resolve_named_custom_branch(req: _ResolveRequest) -> Optional[_ResolveResul
# whatever the caller left blank, never replaces what the caller set (compression prompts carry
# conversation history, so a silently swapped destination is a data-routing bug, not a nuisance).
custom_base = (req.explicit_base_url or custom_entry.get("base_url") or "").strip()
custom_key = (req.explicit_api_key or "").strip() or _named_custom_api_key(custom_entry, provider, custom_base)
# Callable contract (key_cmd): an explicit token source is passed through
# uncalled; only strings are normalised (#88667).
custom_key = (
req.explicit_api_key
if callable(req.explicit_api_key)
else (req.explicit_api_key or "").strip()
) or _named_custom_api_key(custom_entry, provider, custom_base)
if custom_key == "no-key-required":
logger.warning("resolve_provider_client: named custom provider %r has no resolvable "
"api_key — request will be sent with placeholder no-key-required "
Expand Down Expand Up @@ -4716,7 +4742,13 @@ def _resolve_api_key_branch(req: _ResolveRequest, pconfig: Any, resolve_creds: C
# Explicit api_key override (fallback_model / custom_providers entry) lets callers
# authenticate where no built-in credential is registered for this alias.
if req.explicit_api_key:
api_key = req.explicit_api_key.strip() or api_key
# Callable contract (key_cmd): pass the token source through uncalled;
# .strip() on it raised AttributeError (#88667, follow-up PR #105595).
api_key = (
req.explicit_api_key
if callable(req.explicit_api_key)
else req.explicit_api_key.strip() or api_key
)
raw_base_url = str(creds.get("base_url", "")).strip().rstrip("/") or pconfig.inference_base_url
if req.explicit_base_url:
raw_base_url = req.explicit_base_url.strip().rstrip("/")
Expand Down
121 changes: 118 additions & 3 deletions tests/agent/test_command_token_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,12 @@ class TestAuxiliaryResolverHonoursKeyCmd:
"""

@staticmethod
def _resolve(monkeypatch, entry):
"""Resolve *entry* as a named custom provider; return the api_key seen."""
def _resolve(monkeypatch, entry, explicit_api_key: object = None):
"""Resolve *entry* as a named custom provider; return the api_key seen.

``explicit_api_key`` is typed loosely on purpose: the ``key_cmd``
contract hands ``resolve_provider_client`` a callable, not a str.
"""
import agent.auxiliary_client as ac
from hermes_cli import runtime_provider as rp

Expand All @@ -321,7 +325,7 @@ def _spy(*, api_key, base_url, **kw):
return SimpleNamespace(api_key=api_key, base_url=base_url)

monkeypatch.setattr(ac, "_create_openai_client", _spy)
ac.resolve_provider_client("dbx")
ac.resolve_provider_client("dbx", explicit_api_key=explicit_api_key)
return seen.get("api_key")

BASE = {"base_url": "https://example.invalid/v1", "model": "m1"}
Expand All @@ -347,3 +351,114 @@ def test_blank_key_cmd_keeps_the_placeholder(self, monkeypatch):
assert self._resolve(
monkeypatch, {**self.BASE, "key_cmd": " "}
) == "no-key-required"


class TestExplicitCallableSurvivesCustomResolution:
"""Callers hand ``resolve_provider_client`` a callable (key_cmd token,
vision auto-routing through the main runtime's callable credential) and
every custom branch must pass it through *uncalled*: ``.strip()`` on it
raised AttributeError (issue #88667), ``str()`` would send the object repr
as the bearer (#104460's bug class)."""

TOK = CommandTokenSource("printf minted-token", "dbx")

@staticmethod
def _spy_client(monkeypatch):
import agent.auxiliary_client as ac

seen = {}

def _spy(*, api_key, base_url, **kw):
seen["api_key"] = api_key
return SimpleNamespace(api_key=api_key, base_url=base_url)

monkeypatch.setattr(ac, "_create_openai_client", _spy)
return seen

def test_named_custom_branch_honours_explicit_callable(self, monkeypatch):
from hermes_cli import runtime_provider as rp

monkeypatch.setattr(
rp, "_get_named_custom_provider",
lambda name: {"base_url": "https://example.invalid/v1", "model": "m1",
"name": "dbx"} if name == "dbx" else None,
)
seen = self._spy_client(monkeypatch)
import agent.auxiliary_client as ac

ac.resolve_provider_client("dbx", explicit_api_key=self.TOK)
assert seen.get("api_key") is self.TOK, (
"an explicit callable must reach the client unchanged, not be "
"stripped (crash) or stringified (stale-garbage bearer)"
)

def test_bare_custom_branch_honours_explicit_callable(self, monkeypatch):
seen = self._spy_client(monkeypatch)
import agent.auxiliary_client as ac

ac.resolve_provider_client(
"custom",
explicit_base_url="https://example.invalid/v1",
explicit_api_key=self.TOK,
)
assert seen.get("api_key") is self.TOK

def test_main_runtime_callable_is_not_stringified(self, monkeypatch):
"""The main-runtime reuse branch must keep the callable; ``str()`` on
it would send the object repr as the bearer (the #104460 class)."""
seen = self._spy_client(monkeypatch)
import agent.auxiliary_client as ac

ac.resolve_provider_client(
"custom",
main_runtime={"base_url": "https://example.invalid/v1",
"api_key": self.TOK, "model": "m1"},
)
assert seen.get("api_key") is self.TOK


class TestAsyncWrapPreservesCallableKey:
"""``_to_async_client`` must carry the callable key across the sync→async hop.

The OpenAI SDK stores a callable api_key as ``_api_key_provider`` and blanks
``client.api_key`` until the first request refreshes it. Rebuilding the async
client from the (still empty) attribute silently sends no Authorization
header — every async auxiliary call (vision) 401s against a key_cmd provider.
The async SDK awaits the provider, so the bridge must be awaitable.
"""

def test_async_client_keeps_a_working_key_provider(self):
import asyncio

import agent.auxiliary_client as ac

calls = []

def _provider():
calls.append(1)
return "tok-from-provider"

class _StubSync:
api_key = "" # what the SDK leaves behind after callable init
base_url = "https://example.invalid/v1"
_api_key_provider = staticmethod(_provider)

async_client, _ = ac._to_async_client(_StubSync(), "m1")
bridge = getattr(async_client, "_api_key_provider", None)
assert bridge is not None, (
"a sync client with a callable key must rebuild the async client "
"with an api-key provider, not the empty api_key attribute"
)

async def _mint_via_bridge():
# Mirror one request: the async SDK calls _refresh_api_key() in
# _prepare_options before building auth headers.
await getattr(async_client, "_refresh_api_key")()
return getattr(async_client, "auth_headers")

assert asyncio.run(_mint_via_bridge()) == {
"Authorization": "Bearer " + "tok-from-provider"
}
assert calls == [1]