From 8b8537eea77f4fe470c60f89983bd6d4fa0ee45d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:31:18 -0700 Subject: [PATCH] refactor(agent/azure_identity_adapter,acp_openai_bridge): drop unreachable httpx shim, share tool-call name parsing, compact docs --- agent/acp_openai_bridge.py | 122 +++++++++++++------------------- agent/azure_identity_adapter.py | 102 +++++++++----------------- 2 files changed, 86 insertions(+), 138 deletions(-) diff --git a/agent/acp_openai_bridge.py b/agent/acp_openai_bridge.py index afc39b83c5..157655c00a 100644 --- a/agent/acp_openai_bridge.py +++ b/agent/acp_openai_bridge.py @@ -1,12 +1,11 @@ """OpenAI-shape bridge shared by Hermes' ACP clients. -ACP has no OpenAI-style ``tools``/``tool_calls`` channel, so Hermes' tool -schemas travel INTO the prompt as text (:func:`render_tool_bridge_sections`) and -calls are parsed back OUT of the response text (:func:`extract_tool_calls_from_text`). -Clients differ only in WHICH tools they forward (``allowlist``): a CLI with no -tools of its own forwards everything; an autonomous agent with its own -read/edit/execute tools forwards only Hermes' agent-level tools, since -re-offering overlapping ones makes Hermes redo finished work. +ACP has no OpenAI-style ``tools``/``tool_calls`` channel, so Hermes' tool schemas travel INTO the +prompt as text (:func:`render_tool_bridge_sections`) and calls are parsed back OUT of the response +text (:func:`extract_tool_calls_from_text`). Clients differ only in WHICH tools they forward +(``allowlist``): a CLI with no tools of its own forwards everything; an autonomous agent with its own +read/edit/execute tools forwards only Hermes' agent-level tools, since re-offering overlapping ones +makes Hermes redo finished work. """ from __future__ import annotations @@ -16,10 +15,7 @@ import re from types import SimpleNamespace from typing import Any, Iterable -from openai.types.chat.chat_completion_message_tool_call import ( - ChatCompletionMessageToolCall, - Function, -) +from openai.types.chat.chat_completion_message_tool_call import ChatCompletionMessageToolCall, Function TOOL_CALL_BLOCK_RE = re.compile(r"\s*(\{.*?\})\s*", re.DOTALL) TOOL_CALL_JSON_RE = re.compile( @@ -34,14 +30,8 @@ TOOL_CALL_CONTRACT = ( ) __all__ = [ - "TOOL_CALL_BLOCK_RE", - "TOOL_CALL_JSON_RE", - "TOOL_CALL_CONTRACT", - "StreamChunks", - "build_openai_tool_call", - "tool_specs_from_openai_tools", - "render_tool_bridge_sections", - "extract_tool_calls_from_text", + "TOOL_CALL_BLOCK_RE", "TOOL_CALL_JSON_RE", "TOOL_CALL_CONTRACT", "StreamChunks", "build_openai_tool_call", + "tool_specs_from_openai_tools", "render_tool_bridge_sections", "extract_tool_calls_from_text", "completion_to_stream_chunks", ] @@ -81,8 +71,7 @@ def completion_to_stream_chunks(completion: SimpleNamespace) -> StreamChunks: ) data_chunk = SimpleNamespace( choices=[SimpleNamespace(index=0, delta=delta, finish_reason=choice.finish_reason)], - model=completion.model, - usage=None, + model=completion.model, usage=None, ) usage_chunk = SimpleNamespace(choices=[], model=completion.model, usage=completion.usage) chunks = StreamChunks([data_chunk, usage_chunk]) @@ -95,41 +84,35 @@ def completion_to_stream_chunks(completion: SimpleNamespace) -> StreamChunks: def build_openai_tool_call(*, call_id: str, name: str, arguments: str) -> ChatCompletionMessageToolCall: """Build an OpenAI-compatible tool-call object for downstream handling.""" return ChatCompletionMessageToolCall( - id=call_id, - call_id=call_id, - response_item_id=None, - type="function", + id=call_id, call_id=call_id, response_item_id=None, type="function", function=Function(name=name, arguments=arguments), ) +def _named_function(container: Any) -> tuple[dict[str, Any], str] | None: + """``(fn, stripped name)`` from ``container["function"]`` when it is a dict with a non-blank name, else None.""" + fn = container.get("function") if isinstance(container, dict) else None + name = fn.get("name") if isinstance(fn, dict) else None + return (fn, name.strip()) if isinstance(name, str) and name.strip() else None + + def tool_specs_from_openai_tools( - tools: list[dict[str, Any]] | None, - *, - allowlist: Iterable[str] | None = None, + tools: list[dict[str, Any]] | None, *, allowlist: Iterable[str] | None = None, ) -> list[dict[str, Any]]: """Flatten OpenAI ``tools`` into ``{name, description, parameters}`` specs; malformed entries are skipped.""" allowed = {str(n).strip() for n in allowlist} if allowlist is not None else None specs: list[dict[str, Any]] = [] for t in tools or []: - fn = t.get("function") or {} if isinstance(t, dict) else None - if not isinstance(fn, dict): - continue - name = fn.get("name") - if not isinstance(name, str) or not name.strip(): - continue - name = name.strip() - if allowed is not None and name not in allowed: + named = _named_function(t) + if named is None or (allowed is not None and named[1] not in allowed): continue + fn, name = named specs.append({"name": name, "description": fn.get("description", ""), "parameters": fn.get("parameters", {})}) return specs def render_tool_bridge_sections( - tools: list[dict[str, Any]] | None, - tool_choice: Any = None, - *, - allowlist: Iterable[str] | None = None, + tools: list[dict[str, Any]] | None, tool_choice: Any = None, *, allowlist: Iterable[str] | None = None, ) -> list[str]: """Prompt sections carrying the forwarded tool schemas + choice hint (empty list when neither applies).""" specs = tool_specs_from_openai_tools(tools, allowlist=allowlist) @@ -141,45 +124,43 @@ def render_tool_bridge_sections( return sections +def _parse_tool_call(raw_json: str, ordinal: int) -> ChatCompletionMessageToolCall | None: + """One ```` JSON body → tool call, or None when malformed. Missing id → ``acp_call_``.""" + try: + obj = json.loads(raw_json) + except Exception: + return None + named = _named_function(obj) + if named is None: + return None + fn, fn_name = named + fn_args = fn.get("arguments", "{}") + if not isinstance(fn_args, str): + fn_args = json.dumps(fn_args, ensure_ascii=False) + call_id = obj.get("id") + if not isinstance(call_id, str) or not call_id.strip(): + call_id = f"acp_call_{ordinal}" + return build_openai_tool_call(call_id=call_id, name=fn_name, arguments=fn_args) + + def extract_tool_calls_from_text(text: str) -> tuple[list[ChatCompletionMessageToolCall], str]: """Pull ```` blocks out of an ACP response. - Returns ``(tool_calls, cleaned_text)`` with the consumed blocks removed so the - assistant message doesn't show raw JSON. Bare-JSON fallback runs only when no - XML block parsed. + Returns ``(tool_calls, cleaned_text)`` with the consumed blocks removed so the assistant message + doesn't show raw JSON. Bare-JSON fallback runs only when no XML block parsed. """ if not isinstance(text, str) or not text.strip(): return [], "" - extracted: list[ChatCompletionMessageToolCall] = [] consumed_spans: list[tuple[int, int]] = [] - - def _try_add_tool_call(raw_json: str) -> None: - try: - obj = json.loads(raw_json) - except Exception: - return - fn = obj.get("function") if isinstance(obj, dict) else None - if not isinstance(fn, dict): - return - fn_name = fn.get("name") - if not isinstance(fn_name, str) or not fn_name.strip(): - return - fn_args = fn.get("arguments", "{}") - if not isinstance(fn_args, str): - fn_args = json.dumps(fn_args, ensure_ascii=False) - call_id = obj.get("id") - if not isinstance(call_id, str) or not call_id.strip(): - call_id = f"acp_call_{len(extracted)+1}" - extracted.append(build_openai_tool_call(call_id=call_id, name=fn_name.strip(), arguments=fn_args)) - - for m in TOOL_CALL_BLOCK_RE.finditer(text): - _try_add_tool_call(m.group(1)) - consumed_spans.append((m.start(), m.end())) - if not extracted: - for m in TOOL_CALL_JSON_RE.finditer(text): - _try_add_tool_call(m.group(0)) + for pattern, group in ((TOOL_CALL_BLOCK_RE, 1), (TOOL_CALL_JSON_RE, 0)): + for m in pattern.finditer(text): + call = _parse_tool_call(m.group(group), len(extracted) + 1) + if call is not None: + extracted.append(call) consumed_spans.append((m.start(), m.end())) + if extracted: + break if not consumed_spans: return extracted, text.strip() @@ -190,7 +171,6 @@ def extract_tool_calls_from_text(text: str) -> tuple[list[ChatCompletionMessageT merged.append((start, end)) else: merged[-1] = (merged[-1][0], max(merged[-1][1], end)) - parts: list[str] = [] cursor = 0 for start, end in merged: diff --git a/agent/azure_identity_adapter.py b/agent/azure_identity_adapter.py index a6cf20ed58..11f7172bd3 100644 --- a/agent/azure_identity_adapter.py +++ b/agent/azure_identity_adapter.py @@ -1,18 +1,11 @@ """Microsoft Entra ID adapter for Microsoft Foundry. -Keyless auth via the `azure-identity` ``DefaultAzureCredential`` chain (env -service principal → workload identity → managed identity → VS Code → Azure -CLI → azd → PowerShell → broker). Mirrors ``agent/bedrock_adapter.py``: - -* Lazy import: `azure-identity` loads only when ``model.auth_mode = entra_id``. -* ``build_token_provider`` returns the zero-arg callable Microsoft's sample - plugs into ``OpenAI(api_key=token_provider, ...)``; the SDK calls it before - every request, so refresh is transparent. -* Consumer helpers are split by purpose (display / cache / http-bearer) so - logging paths never mint tokens and tokens never leak into cache keys. -* No persisted JWT: azure-identity caches in-process / OS keychain; Hermes - does not duplicate that in ``auth.json``. - +Keyless auth via the `azure-identity` ``DefaultAzureCredential`` chain (env service principal → +workload identity → managed identity → VS Code → Azure CLI → azd → PowerShell → broker). Mirrors +``agent/bedrock_adapter.py``: `azure-identity` is imported lazily (only for ``auth_mode = entra_id``); +``build_token_provider`` returns the zero-arg callable the OpenAI SDK calls before every request +(transparent refresh); consumer helpers are split by purpose so logging paths never mint tokens and +tokens never leak into cache keys; no JWT is persisted (azure-identity caches in-process / OS keychain). Reference: https://learn.microsoft.com/azure/ai-foundry/foundry-models/how-to/configure-entra-id """ @@ -27,11 +20,9 @@ from typing import Any, Callable, Dict, Optional logger = logging.getLogger(__name__) -# Microsoft-documented Foundry inference scope for ALL endpoint shapes -# (*.openai.azure.com, *.services.ai.azure.com, *.ai.azure.com). The older -# ``https://cognitiveservices.azure.com/.default`` is an ARM control-plane -# scope rejected for inference by newer resources; override via -# ``model.entra.scope`` if required. +# Microsoft-documented Foundry inference scope for ALL endpoint shapes. The older +# ``https://cognitiveservices.azure.com/.default`` is an ARM control-plane scope rejected for +# inference by newer resources; override via ``model.entra.scope`` if required. SCOPE_AI_AZURE_DEFAULT = "https://ai.azure.com/.default" _AZURE_IDENTITY_FEATURE = "provider.azure_identity" @@ -61,19 +52,16 @@ def _require_azure_identity(): from tools.lazy_deps import ensure, FeatureUnavailable except ImportError as exc: raise ImportError(_INSTALL_MSG + "Install it with: pip install azure-identity") from exc - try: ensure(_AZURE_IDENTITY_FEATURE, prompt=False) except FeatureUnavailable as exc: raise ImportError(_INSTALL_MSG + str(exc)) from exc - import azure.identity as _ai # noqa: WPS440 — retry after lazy install return _ai def reset_credential_cache() -> None: - """Clear the cached ``DefaultAzureCredential`` (tests, profile switches). Tolerates - tests that monkeypatch ``build_credential`` with a plain function lacking ``cache_clear``.""" + """Clear the cached ``DefaultAzureCredential`` (tests, profile switches); tolerates a monkeypatched plain function.""" cache_clear = getattr(build_credential, "cache_clear", None) if callable(cache_clear): cache_clear() @@ -81,22 +69,18 @@ def reset_credential_cache() -> None: @dataclass(frozen=True) class EntraIdentityConfig: - """Hermes-managed Entra knobs. Everything else (tenant, SP secret, - federated token file, sovereign authority, ``AZURE_CLIENT_ID``...) flows - through azure-identity's standard ``AZURE_*`` env vars. + """Hermes-managed Entra knobs; everything else (tenant, SP secret, federated token file, authority, + ``AZURE_CLIENT_ID``...) flows through azure-identity's standard ``AZURE_*`` env vars. - ``exclude_interactive_browser`` is an internal knob keeping probes - non-interactive; the setup wizard never writes it. Frozen so it is - hashable for ``lru_cache`` and serializable across multiprocessing - (workers rebuild the credential in their own process). + ``exclude_interactive_browser`` keeps probes non-interactive; the setup wizard never writes it. + Frozen so it is hashable for ``lru_cache`` and picklable across multiprocessing workers. """ scope: str = SCOPE_AI_AZURE_DEFAULT exclude_interactive_browser: bool = True def __post_init__(self) -> None: - scope = str(self.scope or "").strip() or SCOPE_AI_AZURE_DEFAULT - object.__setattr__(self, "scope", scope) + object.__setattr__(self, "scope", str(self.scope or "").strip() or SCOPE_AI_AZURE_DEFAULT) def to_dict(self) -> Dict[str, Any]: return {"scope": self.scope, "exclude_interactive_browser": self.exclude_interactive_browser} @@ -112,19 +96,16 @@ class EntraIdentityConfig: @functools.lru_cache(maxsize=1) def build_credential(config: EntraIdentityConfig) -> Any: - """Cached ``DefaultAzureCredential``. ``maxsize=1`` is intentional: a process uses one - ``model.entra.*`` block at a time (a second config just evicts the first). Only Hermes - knobs are passed as kwargs; the rest comes from ``AZURE_*`` env vars.""" + """Cached ``DefaultAzureCredential``. ``maxsize=1`` is intentional: a process uses one ``model.entra.*`` + block at a time. Only Hermes knobs are passed as kwargs; the rest comes from ``AZURE_*`` env vars.""" ai = _require_azure_identity() kwargs: Dict[str, Any] = {} - # SDK default already excludes the browser; only pass when opting in. - if not config.exclude_interactive_browser: + if not config.exclude_interactive_browser: # SDK default already excludes the browser kwargs["exclude_interactive_browser_credential"] = False return ai.DefaultAzureCredential(**kwargs) -def _resolve_config(config: Optional[EntraIdentityConfig], scope: Optional[str], - **overrides: Any) -> EntraIdentityConfig: +def _resolve_config(config: Optional[EntraIdentityConfig], scope: Optional[str], **overrides: Any) -> EntraIdentityConfig: if config is not None: return config return EntraIdentityConfig(scope=(scope or "").strip() or SCOPE_AI_AZURE_DEFAULT, **overrides) @@ -149,13 +130,12 @@ def build_token_provider(scope: Optional[str] = None, *, config: Optional[EntraI ) -> Callable[[], str]: """Zero-arg callable minting a fresh Entra bearer JWT — pass as ``OpenAI(api_key=...)``. - Scope precedence: ``config.scope`` > ``scope`` kwarg > default. ``base_url`` is unused - (back-compat). Not picklable: ship the ``EntraIdentityConfig`` and rebuild in the worker. + Scope precedence: ``config.scope`` > ``scope`` kwarg > default. ``base_url`` is unused (back-compat). + Not picklable: ship the ``EntraIdentityConfig`` and rebuild in the worker. """ ai = _require_azure_identity() config = _resolve_config(config, scope, exclude_interactive_browser=exclude_interactive_browser) - credential = build_credential(config) - return ai.get_bearer_token_provider(credential, config.scope) + return ai.get_bearer_token_provider(build_credential(config), config.scope) def _probe_token(config: EntraIdentityConfig, timeout_seconds: float) -> Optional[Dict[str, Any]]: @@ -179,16 +159,15 @@ def has_azure_identity_credentials(scope: Optional[str] = None, *, config: Optio **overrides: Any) -> bool: """Timeout-bounded probe: can the chain mint a token now? Never raises. - ``allow_install=False`` makes it a strict "is installed?" check for hot paths (CLI startup) - where pip must never run. NOT used by ``is_provider_configured()`` (structural, no mint). + ``allow_install=False`` makes it a strict "is installed?" check for hot paths (CLI startup) where pip + must never run. NOT used by ``is_provider_configured()`` (structural, no mint). """ failure = _install_failure(allow_install) if failure is not None: if "exc" in failure: logger.debug("azure-identity lazy install unavailable: %s", failure["exc"]) return False - config = _resolve_config(config, scope, **overrides) - result = _probe_token(config, timeout_seconds) + result = _probe_token(_resolve_config(config, scope, **overrides), timeout_seconds) if result is None: logger.debug("Entra token service probe timed out after %ss", timeout_seconds) return False @@ -203,8 +182,8 @@ def _env(name: str) -> str: def _scoped_env(name: str) -> str: - """Credential-bearing env read via the profile secret scope so a multiplexed profile never - reports another profile's env-bridged credentials; unscoped CLI probes fall back to plain env.""" + """Credential-bearing env read via the profile secret scope so a multiplexed profile never reports + another profile's env-bridged credentials; unscoped CLI probes fall back to plain env.""" try: from agent.secret_scope import UnscopedSecretError, get_secret @@ -231,22 +210,19 @@ def describe_active_credential(config: Optional[EntraIdentityConfig] = None, *, **overrides: Any) -> Dict[str, Any]: """Doctor / preflight diagnostics. Never raises; ``{"ok": False, "error": ...}`` on failure. - azure-identity hides the winning inner credential, so this reports a coarse picture (env - sources, token expiry) rather than a class name; ``AZURE_LOG_LEVEL=DEBUG`` shows the chain. + azure-identity hides the winning inner credential, so this reports a coarse picture (env sources, + token expiry) rather than a class name; ``AZURE_LOG_LEVEL=DEBUG`` shows the chain. """ info: Dict[str, Any] = {"ok": False} failure = _install_failure(allow_install) if failure is not None: info["error"], info["hint"] = failure["error"], failure["hint"] return info - config = _resolve_config(config, scope, **overrides) info["scope"] = config.scope if _env("AZURE_TENANT_ID"): info["tenant_id_env"] = _env("AZURE_TENANT_ID") - info["env_sources"] = [label for label, present in _ENV_SOURCE_CHECKS if present()] - result = _probe_token(config, timeout_seconds) if result is None: info["error"] = f"Token probe timed out after {timeout_seconds:.0f}s" @@ -277,9 +253,9 @@ def is_token_provider(value: Any) -> bool: def materialize_bearer_for_http(value: Any) -> str: """Mint a fresh Bearer JWT for a manual HTTP request (calls the provider once). - Only for sites building ``Authorization`` outside the OpenAI SDK; the Anthropic SDK can't take - a callable, so :func:`build_bearer_http_client` calls this from an httpx hook. ``ValueError`` - on an unusable value or empty token. + Only for sites building ``Authorization`` outside the OpenAI SDK; the Anthropic SDK can't take a + callable, so :func:`build_bearer_http_client` calls this from an httpx hook. ``ValueError`` on an + unusable value or empty token. """ if is_token_provider(value): token = value() @@ -299,21 +275,13 @@ def _strip_auth_headers(request: Any) -> None: def build_bearer_http_client(token_provider: Callable[[], str], **httpx_kwargs: Any) -> Any: """``httpx.Client`` minting a fresh Entra bearer JWT per outbound request. - The Anthropic SDK computes ``Authorization`` once at construction, so per-request refresh needs - a ``request`` hook: mint (cheap — azure-identity caches), strip pre-set auth headers, set + The Anthropic SDK computes ``Authorization`` once at construction, so per-request refresh needs a + ``request`` hook: mint (cheap — azure-identity caches), strip pre-set auth headers, set ``Authorization: Bearer``. ``httpx_kwargs`` are forwarded verbatim (``timeout``, ``transport``...). """ if not is_token_provider(token_provider): raise ValueError("build_bearer_http_client requires a zero-arg callable token provider") - - try: - import httpx - except ImportError as exc: # pragma: no cover — httpx ships with openai/anthropic - raise ImportError( - "httpx is required for Entra ID bearer auth on Microsoft Foundry " - "Anthropic-style endpoints. It is normally a transitive " - "dependency of the openai/anthropic SDKs." - ) from exc + import httpx def _inject_bearer(request: "httpx.Request") -> None: try: