From a8b987aaa199f045ad0c61f7b716ca2d86ef1639 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Mon, 12 Oct 2026 20:34:43 +0000 Subject: [PATCH] fix(bedrock): apply configured request timeouts to Bedrock clients providers.bedrock.request_timeout_seconds and providers.bedrock.models..timeout_seconds resolved correctly through hermes_cli.timeouts.get_provider_request_timeout, but no Bedrock client was built with the result. The cached boto3 bedrock-runtime client (ambient and profile-scoped) was created with no botocore Config, so it kept botocore's 60s read / 60s connect, and the AnthropicBedrock SDK client was built with _client_timeout(None), i.e. a fixed 900s read. The resolver (3143d32330) landed after the cached Converse client (0cb8c51fa5) and was wired only into the OpenAI-wire and native Anthropic builders; 611657487f documented Bedrock as not covered instead of wiring it. - _get_bedrock_runtime_client(region, model) resolves the model's timeout (model over provider) and passes Config(read_timeout=..., connect_timeout=60) only when one is configured; unset or invalid builds the exact no-Config client used before. - Both client caches (ambient and per-profile-home) key on the effective read timeout, so models with different overrides in one region never share a client, and the same timeout reuses one client. Stale-connection eviction drops every timeout slot of the region. - converse/converse_stream dispatch picks the client from the request's modelId, so a /model switch or a fallback to another Bedrock model uses that model's timeout. call_converse and the context probe pass their model. - AnthropicBedrock: build_anthropic_bedrock_client takes the timeout; the bound client and the per-request client key carry the active model's value (connect stays 10s, unset stays 900s). - Docs: replace "not currently wired for AWS Bedrock" with what is wired. This closes a configuration gap only. A socket read timeout bounds each read, not the whole call; retry policy and the stale detector are unchanged. --- agent/anthropic_adapter.py | 7 +- agent/bedrock_adapter.py | 80 ++++-- agent/chat_completion_helpers.py | 2 +- agent/client_lifecycle.py | 5 +- cli-config.yaml.example | 6 +- .../test_auxiliary_explicit_cancellation.py | 2 +- tests/agent/test_bedrock_adapter.py | 22 +- tests/agent/test_bedrock_request_timeout.py | 262 ++++++++++++++++++ tests/agent/test_plugin_stream_hooks.py | 2 +- website/docs/user-guide/configuration.md | 2 +- 10 files changed, 345 insertions(+), 45 deletions(-) create mode 100644 tests/agent/test_bedrock_request_timeout.py diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index 73e1d19106..2b368f333c 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -520,13 +520,14 @@ def build_anthropic_client(api_key, base_url: str | None = None, timeout: float return _new_sdk_client(sdk, kwargs, headers, route=base_url) -def build_anthropic_bedrock_client(region: str): +def build_anthropic_bedrock_client(region: str, timeout: float | None = None): """AnthropicBedrock client for Bedrock Claude models (boto3 default credential chain). The SDK's native Bedrock adapter gives full Claude feature parity (prompt caching, thinking budgets, adaptive thinking, fast mode) that Converse lacks. The common betas plus ``context-1m-2025-08-07`` are attached: without the latter Bedrock caps Opus 4.6/4.7 at 200K. A configured ``bedrock.guardrail`` rides as InvokeModel headers so every client built here - (primary, auxiliary, per-request rebuild) enforces it.""" + (primary, auxiliary, per-request rebuild) enforces it. ``timeout`` is the resolved + ``providers.bedrock`` request timeout; None keeps the 900s read / 10s connect default.""" from agent.bedrock_adapter import bedrock_guardrail_headers, scoped_aws_session_kwargs sdk = _require_sdk("the Bedrock provider") if not hasattr(sdk, "AnthropicBedrock"): @@ -537,7 +538,7 @@ def build_anthropic_bedrock_client(region: str): aws_kwargs = {"aws_access_key": scoped.get("aws_access_key_id"), "aws_secret_key": scoped.get("aws_secret_access_key"), "aws_session_token": scoped.get("aws_session_token"), "aws_profile": scoped.get("profile_name")} return sdk.AnthropicBedrock( - aws_region=region, timeout=_client_timeout(None), **{k: v for k, v in aws_kwargs.items() if v}, + aws_region=region, timeout=_client_timeout(timeout), **{k: v for k, v in aws_kwargs.items() if v}, max_retries=0, # retry belongs to hermes's outer loop (honors Retry-After) default_headers={**_beta_header([*_COMMON_BETAS, _CONTEXT_1M_BETA]), **bedrock_guardrail_headers()}, ) diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index 3f6e724195..0339d66d01 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -25,11 +25,16 @@ from agent.errors import EmptyStreamError logger = logging.getLogger(__name__) -_bedrock_runtime_client_cache: dict[str, Any] = {} -_bedrock_control_client_cache: dict[str, Any] = {} -# Routed multiplex profiles: one client per (profile home, region). boto3 freezes the credential -# chain into the client at construction, so a region-only slot would sign profile B's calls with A's keys. -_bedrock_clients_by_home: dict[tuple[str, str, str], Any] = {} +# Keyed by (region, read_timeout): boto3 freezes its Config into the client, so two models with +# different ``timeout_seconds`` overrides in one region need two clients (None = botocore default). +_bedrock_runtime_client_cache: dict[tuple[str, Optional[float]], Any] = {} +_bedrock_control_client_cache: dict[tuple[str, Optional[float]], Any] = {} +# Routed multiplex profiles: one client per (profile home, service, region, read_timeout). boto3 freezes +# the credential chain into the client too, so a region-only slot would sign profile B's calls with A's keys. +_bedrock_clients_by_home: dict[tuple[str, str, str, Optional[float]], Any] = {} +# botocore's own connect default, set explicitly so a long read budget never stretches the TCP/TLS +# connect wait: a 600s ``request_timeout_seconds`` is for slow generations, not unreachable endpoints. +_BOTO_CONNECT_TIMEOUT_SECONDS = 60 # botocore session kwarg <- profile .env variable (the explicit sources of the default chain). _AWS_SCOPED_CREDENTIAL_VARS: tuple[tuple[str, str], ...] = ( @@ -123,28 +128,46 @@ def _require_boto3(): return boto3 -def _cached_client(cache: dict[str, Any], service: str, region: str): - """Get or create a per-region boto3 client. Unscoped: the default credential chain, one client per - region. Routed profile: one client per (home, service, region), built from that profile's scoped - ``AWS_*`` (falling back to the default chain only for what the profile does not set).""" +def _new_client(factory: Any, service: str, region: str, read_timeout: Optional[float]): + """``factory.client(...)``; no ``Config`` at all when no timeout is configured, so the unset + case builds exactly the client botocore always built (60s read / 60s connect).""" + if read_timeout is None: + return factory.client(service, region_name=region) + from botocore.config import Config + config = Config(read_timeout=read_timeout, connect_timeout=_BOTO_CONNECT_TIMEOUT_SECONDS) + return factory.client(service, region_name=region, config=config) + + +def _cached_client(cache: dict[tuple[str, Optional[float]], Any], service: str, region: str, *, + read_timeout: Optional[float] = None): + """Get or create a boto3 client per (region, read timeout). Unscoped: the default credential + chain. Routed profile: one client per (home, service, region, read timeout), built from that + profile's scoped ``AWS_*`` (falling back to the default chain only for what the profile does not set).""" from hermes_constants import get_hermes_home_override, hermes_home_key if get_hermes_home_override() is None: - if region not in cache: - cache[region] = _require_boto3().client(service, region_name=region) - return cache[region] - key = (hermes_home_key(), service, region) - client = _bedrock_clients_by_home.get(key) + key = (region, read_timeout) + if key not in cache: + cache[key] = _new_client(_require_boto3(), service, region, read_timeout) + return cache[key] + home_key = (hermes_home_key(), service, region, read_timeout) + client = _bedrock_clients_by_home.get(home_key) if client is None: # Scope check first: a cred-less multiplex profile must hit the ambient-chain # refusal, not a boto3 ImportError on hosts that lack the package. kwargs = scoped_aws_session_kwargs() - client = _require_boto3().Session(**kwargs).client(service, region_name=region) - _bedrock_clients_by_home[key] = client + client = _new_client(_require_boto3().Session(**kwargs), service, region, read_timeout) + _bedrock_clients_by_home[home_key] = client return client -def _get_bedrock_runtime_client(region: str): - return _cached_client(_bedrock_runtime_client_cache, "bedrock-runtime", region) +def _get_bedrock_runtime_client(region: str, model: Optional[str] = None): + """The ``bedrock-runtime`` client for *region* whose read timeout is *model*'s configured + request timeout (``providers.bedrock.models..timeout_seconds`` over + ``providers.bedrock.request_timeout_seconds``). Resolved per call, so a model switch or a + fallback to another Bedrock model gets that model's budget, not the one the session started with.""" + from hermes_cli.timeouts import get_provider_request_timeout + return _cached_client(_bedrock_runtime_client_cache, "bedrock-runtime", region, + read_timeout=get_provider_request_timeout("bedrock", model)) def _get_bedrock_control_client(region: str): @@ -161,11 +184,18 @@ def reset_client_cache(): def invalidate_runtime_client(region: str) -> bool: - """Evict one region's cached ``bedrock-runtime`` client (stale HTTP pool); True if evicted.""" + """Evict one region's cached ``bedrock-runtime`` clients, every read timeout (stale HTTP pool); + True if any was evicted.""" from hermes_constants import get_hermes_home_override, hermes_home_key if get_hermes_home_override() is not None: - return _bedrock_clients_by_home.pop((hermes_home_key(), "bedrock-runtime", region), None) is not None - return _bedrock_runtime_client_cache.pop(region, None) is not None + cache: dict = _bedrock_clients_by_home + prefix = (hermes_home_key(), "bedrock-runtime", region) + else: + cache, prefix = _bedrock_runtime_client_cache, (region,) + stale = [key for key in list(cache) if key[:len(prefix)] == prefix] + for key in stale: + cache.pop(key, None) + return bool(stale) @@ -470,7 +500,9 @@ def bind_bedrock_runtime(agent, base_url: str, api_mode: str) -> None: agent._is_anthropic_oauth = False if api_mode == "anthropic_messages": from agent.anthropic_adapter import build_anthropic_bedrock_client - agent._anthropic_client = build_anthropic_bedrock_client(agent._bedrock_region) + from hermes_cli.timeouts import get_provider_request_timeout + agent._anthropic_client = build_anthropic_bedrock_client( + agent._bedrock_region, timeout=get_provider_request_timeout("bedrock", getattr(agent, "model", None))) else: agent._anthropic_client = None @@ -1326,7 +1358,7 @@ def call_converse( """Non-streaming Converse call → OpenAI-compatible response. Retries once without a rejected cachePoint placement; evicts the cached client on stale-connection errors.""" from agent.prompt_caching import is_one_shot_request - client = _get_bedrock_runtime_client(region) + client = _get_bedrock_runtime_client(region, model) # Auxiliary-only entry point: a one-shot's cachePoint is a write no later call reads. kwargs = build_converse_kwargs(model, messages, tools, max_tokens, temperature, top_p, stop_sequences, guardrail_config, prompt_cache=not is_one_shot_request(messages, tools)) @@ -1494,7 +1526,7 @@ def probe_bedrock_context_length(model_id: str, region: str) -> Optional[int]: probe costs nothing. An accepted tier is a safe lower bound; None (no creds/network/unparseable) → static table.""" from agent.model_metadata import parse_context_limit_from_error try: - client = _get_bedrock_runtime_client(region) + client = _get_bedrock_runtime_client(region, model_id) except Exception as exc: # boto3 missing / credential resolution failure logger.debug("Bedrock context probe skipped for %s: %s", model_id, exc) return None diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index d8a4b815ae..c682fbd4d7 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -739,7 +739,7 @@ def _bedrock_converse_call(api_kwargs: dict, *, stream: bool, on_stream_denied=N recover_from_cache_point_rejection, recover_from_redacted_reasoning_rejection) region = api_kwargs.pop("__bedrock_region__", "us-east-1") api_kwargs.pop("__bedrock_converse__", None) - client = _get_bedrock_runtime_client(region) + client = _get_bedrock_runtime_client(region, api_kwargs.get("modelId")) # the active model's timeout method = client.converse_stream if stream else client.converse finish = (lambda raw: raw.get("stream", [])) if stream else normalize_converse_response try: diff --git a/agent/client_lifecycle.py b/agent/client_lifecycle.py index e8834058a5..dae0aec375 100644 --- a/agent/client_lifecycle.py +++ b/agent/client_lifecycle.py @@ -489,7 +489,8 @@ class ClientLifecycleMixin: def _request_anthropic_client_key(self) -> tuple: """Cache key over everything forcing a fresh client: credential, base URL/region, timeout, 1M-beta flag.""" if getattr(self, "provider", None) == "bedrock": - return ("bedrock", getattr(self, "_bedrock_region", "us-east-1") or "us-east-1") + return ("bedrock", getattr(self, "_bedrock_region", "us-east-1") or "us-east-1", + get_provider_request_timeout(self.provider, self.model)) return ( "direct", self._anthropic_api_key, getattr(self, "_anthropic_base_url", None), get_provider_request_timeout(self.provider, self.model), bool(getattr(self, "_oauth_1m_beta_disabled", False)), @@ -508,7 +509,7 @@ class ClientLifecycleMixin: def _build_anthropic_client_for_key(self, key: tuple) -> Any: from agent.anthropic_adapter import build_anthropic_bedrock_client, build_anthropic_client if key[0] == "bedrock": - return build_anthropic_bedrock_client(key[1]) + return build_anthropic_bedrock_client(key[1], timeout=key[2]) return build_anthropic_client(key[1], key[2], timeout=key[3], drop_context_1m_beta=key[4]) def _create_request_anthropic_client(self, *, reason: str) -> Any: diff --git a/cli-config.yaml.example b/cli-config.yaml.example index e1aebd715c..9386b5a043 100644 --- a/cli-config.yaml.example +++ b/cli-config.yaml.example @@ -288,8 +288,10 @@ model: # implicit non-stream stale detector is auto-disabled for local endpoints # and can scale upward for very large contexts. # -# Not currently wired for AWS Bedrock (bedrock_converse + AnthropicBedrock -# SDK paths) — those use boto3 with its own timeout configuration. +# AWS Bedrock (providers.bedrock): the resolved value is the socket read +# timeout of the boto3 Converse client and of the AnthropicBedrock SDK +# client, chosen per request for the active model. Connect stays fixed +# (botocore 60s / SDK 10s); unset keeps botocore's 60s read on Converse. # # providers: # ollama-local: diff --git a/tests/agent/test_auxiliary_explicit_cancellation.py b/tests/agent/test_auxiliary_explicit_cancellation.py index 79d3aea6e7..db5bb6a622 100644 --- a/tests/agent/test_auxiliary_explicit_cancellation.py +++ b/tests/agent/test_auxiliary_explicit_cancellation.py @@ -495,7 +495,7 @@ def test_bedrock_silent_nonstream_request_is_isolated_without_close_wakeup() -> release = threading.Event() runtime_client = _BedrockRuntimeClient(started, release) reset_client_cache() - _bedrock_runtime_client_cache["us-test-1"] = runtime_client + _bedrock_runtime_client_cache[("us-test-1", None)] = runtime_client client = aux.BedrockAuxiliaryClient("us-test-1", "bedrock-test") try: exc, elapsed = _cancel_silent_request(client, started, _invoke_generic) diff --git a/tests/agent/test_bedrock_adapter.py b/tests/agent/test_bedrock_adapter.py index 9897b21b7e..c5a7dca3d0 100644 --- a/tests/agent/test_bedrock_adapter.py +++ b/tests/agent/test_bedrock_adapter.py @@ -1391,14 +1391,16 @@ class TestInvalidateRuntimeClient: reset_client_cache, ) reset_client_cache() - _bedrock_runtime_client_cache["us-east-1"] = "dead-client" - _bedrock_runtime_client_cache["us-west-2"] = "live-client" + # Keyed by (region, read_timeout): every timeout variant of the region shares the dead pool. + _bedrock_runtime_client_cache[("us-east-1", None)] = "dead-client" + _bedrock_runtime_client_cache[("us-east-1", 300.0)] = "dead-client-300" + _bedrock_runtime_client_cache[("us-west-2", None)] = "live-client" evicted = invalidate_runtime_client("us-east-1") assert evicted is True - assert "us-east-1" not in _bedrock_runtime_client_cache - assert _bedrock_runtime_client_cache["us-west-2"] == "live-client" + assert not [key for key in _bedrock_runtime_client_cache if key[0] == "us-east-1"] + assert _bedrock_runtime_client_cache[("us-west-2", None)] == "live-client" def test_returns_false_when_region_not_cached(self): from agent.bedrock_adapter import invalidate_runtime_client, reset_client_cache @@ -1463,7 +1465,7 @@ class TestCallConverseInvalidatesOnStaleError: error_response={"Error": {"Code": "ValidationException", "Message": "bad"}}, operation_name="Converse", ) - _bedrock_runtime_client_cache["us-east-1"] = live_client + _bedrock_runtime_client_cache[("us-east-1", None)] = live_client with pytest.raises(ClientError): call_converse( @@ -1472,7 +1474,7 @@ class TestCallConverseInvalidatesOnStaleError: messages=[{"role": "user", "content": "hi"}], ) - assert _bedrock_runtime_client_cache.get("us-east-1") is live_client, ( + assert _bedrock_runtime_client_cache.get(("us-east-1", None)) is live_client, ( "validation errors do not indicate a dead connection — keep the client" ) @@ -1542,7 +1544,7 @@ class TestAgentBedrockStreamRecovery: "stopReason": "end_turn", "usage": {"inputTokens": 1, "outputTokens": 1, "totalTokens": 2}, } - _bedrock_runtime_client_cache["us-east-1"] = client + _bedrock_runtime_client_cache[("us-east-1", None)] = client agent = SimpleNamespace(_disable_streaming=False, _safe_print=MagicMock(), model="m", provider="bedrock") stream = _BedrockStream(agent, dict(self._KW), on_first_delta=None) @@ -1554,7 +1556,7 @@ class TestAgentBedrockStreamRecovery: assert agent._disable_streaming is True assert "InvokeModelWithResponseStream" in agent._safe_print.call_args.args[0] # Not a stale connection — client stays cached. - assert _bedrock_runtime_client_cache.get("us-east-1") is client + assert _bedrock_runtime_client_cache.get(("us-east-1", None)) is client @pytest.mark.parametrize("stream", [True, False]) @pytest.mark.parametrize("retry_fails", [False, True]) @@ -1625,14 +1627,14 @@ class TestAgentBedrockStreamRecovery: reset_client_cache() dead_client = MagicMock() dead_client.converse_stream.side_effect = ConnectionClosedError(endpoint_url="https://bedrock.example") - _bedrock_runtime_client_cache["us-east-1"] = dead_client + _bedrock_runtime_client_cache[("us-east-1", None)] = dead_client denied = MagicMock() with pytest.raises(ConnectionClosedError): _bedrock_converse_call(dict(self._KW), stream=True, on_stream_denied=denied) denied.assert_not_called() - assert "us-east-1" not in _bedrock_runtime_client_cache + assert ("us-east-1", None) not in _bedrock_runtime_client_cache # --------------------------------------------------------------------------- diff --git a/tests/agent/test_bedrock_request_timeout.py b/tests/agent/test_bedrock_request_timeout.py new file mode 100644 index 0000000000..20134602c1 --- /dev/null +++ b/tests/agent/test_bedrock_request_timeout.py @@ -0,0 +1,262 @@ +"""Bedrock request timeouts reach the clients that serve the call. + +``providers.bedrock.request_timeout_seconds`` and ``providers.bedrock.models..timeout_seconds`` +resolved correctly but never reached a client: the boto3 Converse client kept botocore's 60s read +timeout and the AnthropicBedrock SDK client its 900s default. These tests read the real botocore / +httpx client config (dummy credentials, IMDS off) and intercept the botocore API call itself, so no +request leaves the process. +""" + +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +pytest.importorskip("boto3") +pytest.importorskip("botocore.client") + +import boto3 +import botocore.client + +from agent import bedrock_adapter +from agent.bedrock_adapter import _get_bedrock_runtime_client, invalidate_runtime_client + +REGION = "us-east-1" +RUNTIME_URL = f"https://bedrock-runtime.{REGION}.amazonaws.com" +CLAUDE = "us.anthropic.claude-sonnet-4-5-20250929-v1:0" # model override 300 +NOVA = "us.amazon.nova-pro-v1:0" # no model override -> provider 600 +LLAMA = "us.meta.llama4-maverick-17b-instruct-v1:0" # model override 120 +OVERRIDES = {"providers": {"bedrock": { + "request_timeout_seconds": 600, + "models": {CLAUDE: {"timeout_seconds": 300}, LLAMA: {"timeout_seconds": 120}}, +}}} +BOTOCORE_DEFAULT_TIMEOUT = 60 + + +@pytest.fixture(autouse=True) +def aws_env(monkeypatch): + monkeypatch.setenv("AWS_ACCESS_KEY_ID", "AKIA-AMBIENT") + monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "ambient-secret") + monkeypatch.setenv("AWS_EC2_METADATA_DISABLED", "true") + for var in ("AWS_PROFILE", "AWS_SESSION_TOKEN", "AWS_BEARER_TOKEN_BEDROCK"): + monkeypatch.delenv(var, raising=False) + bedrock_adapter.reset_client_cache() + yield + bedrock_adapter.reset_client_cache() + + +@pytest.fixture +def config(monkeypatch): + """Install ``cfg`` as the loaded config (the operator report's offline technique).""" + def install(cfg): + monkeypatch.setattr("hermes_cli.config.load_config_readonly", lambda: cfg) + install({}) + return install + + +@pytest.fixture +def botocore_calls(monkeypatch): + """Intercept every botocore API call: record (operation, serving client, its read timeout).""" + calls = [] + + def fake_make_api_call(client, operation_name, params): + calls.append((operation_name, client, client.meta.config.read_timeout, params.get("modelId"))) + if operation_name == "ConverseStream": + return {"stream": []} + return {"output": {"message": {"role": "assistant", "content": [{"text": "ok"}]}}, + "stopReason": "end_turn", "usage": {"inputTokens": 1, "outputTokens": 1, "totalTokens": 2}} + monkeypatch.setattr(botocore.client.BaseClient, "_make_api_call", fake_make_api_call) + return calls + + +def _converse_kwargs(model): + return {"__bedrock_converse__": True, "__bedrock_region__": REGION, "modelId": model, + "messages": [{"role": "user", "content": [{"text": "hi"}]}]} + + +# --- client construction ---------------------------------------------------------------------- + +def test_unset_config_builds_exactly_the_botocore_default_client(config): + baseline = boto3.client("bedrock-runtime", region_name=REGION).meta.config + + built = _get_bedrock_runtime_client(REGION, CLAUDE).meta.config + + assert (built.read_timeout, built.connect_timeout) == (BOTOCORE_DEFAULT_TIMEOUT, BOTOCORE_DEFAULT_TIMEOUT) + assert (built.read_timeout, built.connect_timeout, built.retries, built.max_pool_connections) == ( + baseline.read_timeout, baseline.connect_timeout, baseline.retries, baseline.max_pool_connections) + + +@pytest.mark.parametrize("model, expected", [(NOVA, 600), (CLAUDE, 300)], ids=["provider", "model-wins"]) +def test_configured_timeout_is_the_client_read_timeout_and_connect_stays_bounded(config, model, expected): + config(OVERRIDES) + + client_config = _get_bedrock_runtime_client(REGION, model).meta.config + + assert client_config.read_timeout == expected + assert client_config.connect_timeout == BOTOCORE_DEFAULT_TIMEOUT, "a long read budget must not stretch connect" + assert client_config.retries == boto3.client("bedrock-runtime", region_name=REGION).meta.config.retries + + +@pytest.mark.parametrize("provider_value, model_value", [("soon", -5), (0, "later"), ("", None)]) +def test_invalid_values_behave_like_unset(config, provider_value, model_value): + config({"providers": {"bedrock": {"request_timeout_seconds": provider_value, + "models": {CLAUDE: {"timeout_seconds": model_value}}}}}) + + client = _get_bedrock_runtime_client(REGION, CLAUDE) + + assert client.meta.config.read_timeout == BOTOCORE_DEFAULT_TIMEOUT + assert client is _get_bedrock_runtime_client(REGION), "invalid must share the unset client, not a new one" + + +def test_models_with_different_overrides_never_share_a_client_in_one_region(config): + config(OVERRIDES) + + claude, nova, llama = (_get_bedrock_runtime_client(REGION, m) for m in (CLAUDE, NOVA, LLAMA)) + + assert [c.meta.config.read_timeout for c in (claude, nova, llama)] == [300, 600, 120] + assert len({id(claude), id(nova), id(llama)}) == 3 + # Bounded: same region + same effective timeout reuses one client. + assert _get_bedrock_runtime_client(REGION, CLAUDE) is claude + assert _get_bedrock_runtime_client(REGION, "us.amazon.nova-lite-v1:0") is nova # provider-level 600 too + + config({}) # unset must not collide with any configured slot + unset = _get_bedrock_runtime_client(REGION, CLAUDE) + assert unset not in (claude, nova, llama) + assert unset.meta.config.read_timeout == BOTOCORE_DEFAULT_TIMEOUT + + +def test_profile_scoped_clients_carry_their_own_profile_timeouts(tmp_path, monkeypatch): + """A routed profile's clients come from its own config.yaml and own cache slot, per timeout.""" + from agent import secret_scope + from hermes_constants import reset_hermes_home_override, set_hermes_home_override + + home = tmp_path / "profile-home" + home.mkdir() + (home / ".env").write_text("AWS_ACCESS_KEY_ID=AKIA-PROFILE\nAWS_SECRET_ACCESS_KEY=profile-secret\n") + (home / "config.yaml").write_text( + "providers:\n bedrock:\n request_timeout_seconds: 600\n" + f" models:\n {CLAUDE}:\n timeout_seconds: 300\n") + monkeypatch.setattr(secret_scope, "_MULTIPLEX_ACTIVE", True) + + def in_profile(fn): + h_tok = set_hermes_home_override(str(home)) + s_tok = secret_scope.set_secret_scope(secret_scope.build_profile_secret_scope(home)) + try: + return fn() + finally: + secret_scope.reset_secret_scope(s_tok) + reset_hermes_home_override(h_tok) + + claude, nova, claude_again = in_profile(lambda: ( + _get_bedrock_runtime_client(REGION, CLAUDE), _get_bedrock_runtime_client(REGION, NOVA), + _get_bedrock_runtime_client(REGION, CLAUDE))) + + assert (claude.meta.config.read_timeout, nova.meta.config.read_timeout) == (300, 600) + assert claude.meta.config.connect_timeout == nova.meta.config.connect_timeout == BOTOCORE_DEFAULT_TIMEOUT + assert claude is claude_again and claude is not nova + assert claude._request_signer._credentials.access_key == "AKIA-PROFILE" + # The launch context (no override, its own unset config) keeps botocore's default client. + ambient = _get_bedrock_runtime_client(REGION, CLAUDE) + assert ambient is not claude and ambient.meta.config.read_timeout == BOTOCORE_DEFAULT_TIMEOUT + # Stale-pool eviction drops every timeout slot of the profile's region, never the launch context's. + assert in_profile(lambda: invalidate_runtime_client(REGION)) is True + assert in_profile(lambda: _get_bedrock_runtime_client(REGION, CLAUDE)) is not claude + assert _get_bedrock_runtime_client(REGION, CLAUDE) is ambient + + +# --- dispatch --------------------------------------------------------------------------------- + +def test_converse_and_converse_stream_are_served_with_the_requested_models_timeout(config, botocore_calls): + from agent.chat_completion_helpers import _BedrockStream, _dispatch_nonstreaming_api_request + config(OVERRIDES) + + agent = SimpleNamespace(api_mode="bedrock_converse", provider="bedrock", model=CLAUDE, + _disable_streaming=False, _safe_print=MagicMock()) + _dispatch_nonstreaming_api_request(agent, _converse_kwargs(CLAUDE), make_client=None) + _BedrockStream(agent, _converse_kwargs(NOVA), on_first_delta=None)._open_stream(_converse_kwargs(NOVA)) + + assert [(op, timeout, model) for op, _client, timeout, model in botocore_calls] == [ + ("Converse", 300, CLAUDE), ("ConverseStream", 600, NOVA)] + + +def test_auxiliary_call_converse_uses_the_models_timeout(config, botocore_calls): + from agent.bedrock_adapter import call_converse + config(OVERRIDES) + + call_converse(region=REGION, model=LLAMA, messages=[{"role": "user", "content": "hi"}]) + + assert [(op, timeout) for op, _c, timeout, _m in botocore_calls] == [("Converse", 120)] + + +def _agent(): + from agent.context_compressor import ContextCompressor + from run_agent import AIAgent + agent = AIAgent.__new__(AIAgent) + agent.model, agent.provider = "claude-opus-4.8", "anthropic" + agent.base_url, agent.api_key, agent.api_mode = "https://api.anthropic.com", "sk-ant", "anthropic_messages" + agent.client, agent._anthropic_client, agent._client_kwargs = None, MagicMock(), {} + agent.quiet_mode, agent._config_context_length, agent._primary_runtime = True, None, {} + agent.max_tokens = None + agent.context_compressor = ContextCompressor( + model=agent.model, threshold_percent=0.5, base_url=agent.base_url, api_key="sk-ant", + provider="anthropic", quiet_mode=True, config_context_length=None, + ) + return agent + + +def _dispatch_turn(agent): + from agent.chat_completion_helpers import _build_bedrock_kwargs, _dispatch_nonstreaming_api_request + kwargs = _build_bedrock_kwargs(agent, [{"role": "user", "content": "hi"}], None, None) + _dispatch_nonstreaming_api_request(agent, kwargs, make_client=None) + + +def test_model_switch_and_fallback_dispatch_with_the_new_models_timeout(config, botocore_calls, monkeypatch): + from agent.client_lifecycle import _swap_fallback_clients + monkeypatch.setattr("agent.model_metadata.get_model_context_length", lambda *a, **k: 200_000) + config(OVERRIDES) + agent = _agent() + + agent.switch_model(LLAMA, "bedrock", api_key="aws-sdk", base_url=RUNTIME_URL, api_mode="bedrock_converse") + _dispatch_turn(agent) + # Fallback onto another Bedrock model, bound the way route_binding.bind_route_entry binds it. + agent.model, agent.provider, agent.base_url, agent.api_mode = NOVA, "bedrock", RUNTIME_URL, "bedrock_converse" + _swap_fallback_clients(agent, MagicMock(base_url=RUNTIME_URL, api_key="aws-sdk"), "bedrock", NOVA, + RUNTIME_URL, "bedrock_converse") + _dispatch_turn(agent) + agent.switch_model(LLAMA, "bedrock", api_key="aws-sdk", base_url=RUNTIME_URL, api_mode="bedrock_converse") + _dispatch_turn(agent) + + assert [(timeout, model) for _op, _c, timeout, model in botocore_calls] == [ + (120, LLAMA), (600, NOVA), (120, LLAMA)] + first, second, third = (client for _op, client, _t, _m in botocore_calls) + assert first is third and first is not second + + +# --- AnthropicBedrock (Claude via the anthropic SDK) ------------------------------------------ + +def test_anthropic_bedrock_client_takes_the_timeout_and_keeps_default_when_unset(): + pytest.importorskip("anthropic") + from agent.anthropic_adapter import build_anthropic_bedrock_client + + configured = build_anthropic_bedrock_client(REGION, timeout=300.0).timeout + unset = build_anthropic_bedrock_client(REGION).timeout + + assert (configured.read, configured.connect) == (300.0, 10.0) + assert (unset.read, unset.connect) == (900.0, 10.0) + + +def test_anthropic_bedrock_request_client_follows_the_active_model(config, monkeypatch): + pytest.importorskip("anthropic") + monkeypatch.setattr("agent.model_metadata.get_model_context_length", lambda *a, **k: 200_000) + other_claude = "us.anthropic.claude-haiku-4-5-20251001-v1:0" # provider-level 600 + config(OVERRIDES) + agent = _agent() + + agent.switch_model(CLAUDE, "bedrock", api_key="aws-sdk", base_url=RUNTIME_URL, api_mode="anthropic_messages") + shared = agent._anthropic_client.timeout + first = agent._create_request_anthropic_client(reason="test").timeout + agent.model = other_claude # e.g. a fallback onto another Bedrock Claude in the same region + second = agent._create_request_anthropic_client(reason="test").timeout + + assert (shared.read, first.read, second.read) == (300.0, 300.0, 600.0) + assert first.connect == second.connect == 10.0 diff --git a/tests/agent/test_plugin_stream_hooks.py b/tests/agent/test_plugin_stream_hooks.py index 414106c2b1..ccc4504cdf 100644 --- a/tests/agent/test_plugin_stream_hooks.py +++ b/tests/agent/test_plugin_stream_hooks.py @@ -323,7 +323,7 @@ def test_bedrock_reasoning_delta_reaches_plugin_only_observer(monkeypatch): monkeypatch.setattr("hermes_cli.config.cfg_get", lambda *_args, **_kwargs: True) monkeypatch.setattr( "agent.bedrock_adapter._get_bedrock_runtime_client", - lambda _region: SimpleNamespace(converse_stream=lambda **_kwargs: {"stream": []}), + lambda *_args: SimpleNamespace(converse_stream=lambda **_kwargs: {"stream": []}), ) monkeypatch.setattr("agent.bedrock_adapter.is_stale_connection_error", lambda _exc: False) monkeypatch.setattr("agent.bedrock_adapter.is_streaming_access_denied_error", lambda _exc: False) diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index 8220b94d23..889057dc25 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -167,7 +167,7 @@ You can set `providers..request_timeout_seconds` for a provider-wide request You can also set `providers..stale_timeout_seconds` for the non-streaming stale-call detector, plus `providers..models..stale_timeout_seconds` for a model-specific override. This wins over the legacy `HERMES_API_CALL_STALE_TIMEOUT` env var. The same key is the streaming stale-stream deadline: an explicit value is used as-is — the implicit context-size tiers (240s above 50k tokens, 300s above 100k) and the reasoning-model floors apply only to the 180s default, so an explicit value can shorten how long a hung stream is tolerated. -Leaving these unset keeps the legacy defaults (`HERMES_API_TIMEOUT=1800`s, `HERMES_API_CALL_STALE_TIMEOUT=90`s, native Anthropic 900s). The non-streaming stale detector is auto-disabled for local endpoints when left implicit and can scale upward for very large contexts. Not currently wired for AWS Bedrock (both `bedrock_converse` and AnthropicBedrock SDK paths use boto3 with its own timeout configuration). See the commented example in [`cli-config.yaml.example`](https://github.com/NousResearch/hermes-agent/blob/main/cli-config.yaml.example). +Leaving these unset keeps the legacy defaults (`HERMES_API_TIMEOUT=1800`s, `HERMES_API_CALL_STALE_TIMEOUT=90`s, native Anthropic 900s). The non-streaming stale detector is auto-disabled for local endpoints when left implicit and can scale upward for very large contexts. On AWS Bedrock (`providers.bedrock`), the resolved request timeout is the socket read timeout of the boto3 Converse client (`bedrock_converse`) and of the AnthropicBedrock SDK client, picked per request for the active model so a `/model` switch or fallback uses that model's value. The connect timeout stays fixed (botocore's 60s on Converse, 10s on AnthropicBedrock), and a read timeout bounds each socket read, not the whole call. Unset keeps botocore's 60s read on Converse and 900s on AnthropicBedrock. See the commented example in [`cli-config.yaml.example`](https://github.com/NousResearch/hermes-agent/blob/main/cli-config.yaml.example). ## Update Behavior