Skip to content
Open
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
40 changes: 37 additions & 3 deletions agent/auxiliary_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -3615,6 +3615,13 @@ def _is_unsupported_parameter_error(exc: Exception, param: str) -> bool:
"unrecognized request argument",
"unrecognized parameter",
"invalid parameter",
# AWS Bedrock Converse phrasing for newer Anthropic models
# ("`temperature` is deprecated for this model.") — without this the
# reactive-retry branch never fires and the boto3 ValidationException
# is swallowed by downstream wrappers, surfacing as an empty
# ChatCompletion with all-None fields. Ref: aux vision broken on
# Bedrock Opus 4.7 / Sonnet 4.5 when temperature is passed.
"is deprecated",
))


Expand Down Expand Up @@ -5857,12 +5864,28 @@ def _wrap_if_needed(client_obj, final_model_str: str, base_url_str: str = "",
else (client, final_model))

elif pconfig.auth_type == "aws_sdk":
# AWS SDK providers (Bedrock) — Claude models use the Anthropic Bedrock
# SDK (prompt caching, thinking); non-Claude models use Converse API.
# AWS SDK providers (Bedrock). Two auth paths:
#
# 1. AWS_BEARER_TOKEN_BEDROCK → boto3 Converse API via
# BedrockAuxiliaryClient, for BOTH Claude and non-Claude models.
# The ``anthropic.AnthropicBedrock`` SDK does NOT support bearer
# tokens — it raises ``RuntimeError: could not resolve credentials
# from session`` because its auth helper only consults the boto3
# credential chain for IAM keys, not bearer tokens. boto3's
# Converse call DOES pick up the bearer token natively, and
# Converse supports every Bedrock model including Claude, so we
# route Claude through it too here. This mirrors the dual-path
# routing in ``hermes_cli.runtime_provider`` for the main loop.
#
# 2. IAM credentials (env vars / SSO / instance profile) → Claude
# models use the AnthropicBedrock SDK for full feature parity
# (prompt caching, thinking budgets); non-Claude models use the
# Converse shim.
try:
from agent.bedrock_adapter import (
has_aws_credentials,
is_anthropic_bedrock_model,
resolve_aws_auth_env_var,
resolve_bedrock_region,
)
from agent.anthropic_adapter import build_anthropic_bedrock_client
Expand All @@ -5878,9 +5901,20 @@ def _wrap_if_needed(client_obj, final_model_str: str, base_url_str: str = "",

region = resolve_bedrock_region()
default_model = "anthropic.claude-haiku-4-5-20251001-v1:0"
final_model = _normalize_resolved_model(model or default_model, provider)
final_model = _normalize_resolved_model(model or default_model, provider) or default_model
base_url = f"https://bedrock-runtime.{region}.amazonaws.com"

# Bearer-token path: route everything (Claude included) through the
# boto3 Converse shim, which is the only path that can use the token.
if resolve_aws_auth_env_var() == "AWS_BEARER_TOKEN_BEDROCK":
client = BedrockAuxiliaryClient(region, final_model)
logger.debug(
"resolve_provider_client: bedrock converse (%s, %s, bearer-token)",
final_model, region,
)
return (_to_async_client(client, final_model, is_vision=is_vision) if async_mode

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still routes the new BedrockConverseAuxiliaryClient through _to_async_client(), but _to_async_client has no BedrockConverse branch and will fall through to AsyncOpenAI with the Bedrock runtime URL. Please add a branch returning AsyncBedrockConverseAuxiliaryClient(sync_client) and cover resolve_provider_client(..., async_mode=True) for the bearer-token path.

else (client, final_model))

if is_anthropic_bedrock_model(final_model):
try:
real_client = build_anthropic_bedrock_client(region)
Expand Down
16 changes: 13 additions & 3 deletions agent/bedrock_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -579,11 +579,21 @@ def _convert_content_to_converse(content) -> List[Dict]:
# wire layer, so passing the base64 string directly
# results in double-encoding and Bedrock rejects it with
# "Failed to sanitize image". Ref: #33317.
#
# ``validate=True`` makes b64decode reject payloads with
# characters outside the base64 alphabet instead of
# silently discarding them and returning junk bytes. On a
# malformed data URL we skip the image rather than sending
# the raw base64 string as bytes (which Bedrock rejects
# with ValidationException) or blowing up the whole request.
import base64
import binascii
try:
raw_bytes = base64.b64decode(data)
except Exception:
raw_bytes = data.encode("utf-8")
raw_bytes = base64.b64decode(data, validate=True)
except (binascii.Error, ValueError):
continue
if not raw_bytes:
continue
blocks.append({
"image": {
"format": media_type.split("/")[-1] if "/" in media_type else "jpeg",
Expand Down
73 changes: 69 additions & 4 deletions tests/agent/test_bedrock_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -1104,16 +1104,34 @@ def test_data_url_decoded_to_bytes(self):
assert isinstance(img_block["source"]["bytes"], bytes)
assert img_block["source"]["bytes"] == raw_png

def test_invalid_base64_falls_back_to_encode(self):
def test_invalid_base64_skipped(self):
from agent.bedrock_adapter import _convert_content_to_converse

# Malformed base64 (chars outside the alphabet) must be skipped, not
# forwarded as raw string-bytes — Bedrock rejects junk bytes with
# ValidationException. Skipping degrades gracefully (drop the image)
# instead of failing the whole request.
data_url = "data:image/jpeg;base64,NOT_VALID_BASE64!!!"
content = [{"type": "image_url", "image_url": {"url": data_url}}]
blocks = _convert_content_to_converse(content)

# Should not crash — falls back to encoding the string as bytes
assert len(blocks) == 1
assert isinstance(blocks[0]["image"]["source"]["bytes"], bytes)
# Malformed image is dropped — no image block produced.
assert all("image" not in b for b in blocks)

def test_valid_image_survives_alongside_invalid(self):
from agent.bedrock_adapter import _convert_content_to_converse

good = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8/5+hHgAHggJ/PchI7wAAAABJRU5ErkJggg=="
bad = "data:image/jpeg;base64,NOT_VALID_BASE64!!!"
content = [
{"type": "image_url", "image_url": {"url": good}},
{"type": "image_url", "image_url": {"url": bad}},
]
blocks = _convert_content_to_converse(content)
image_blocks = [b for b in blocks if "image" in b]
# Exactly the one valid image survives.
assert len(image_blocks) == 1
assert isinstance(image_blocks[0]["image"]["source"]["bytes"], bytes)


class TestBearerTokenRoutesToConverse:
Expand Down Expand Up @@ -1157,3 +1175,50 @@ def test_sigv4_claude_still_uses_anthropic_bedrock_sdk(self, monkeypatch):
runtime = self._resolve(monkeypatch, bearer=False)
assert runtime["api_mode"] == "anthropic_messages"
assert runtime.get("bedrock_anthropic") is True


class TestBearerTokenAuxRoutesToConverse:
"""The AUXILIARY client (vision/summarization) must also route bearer-token
Claude models through the boto3 Converse shim (BedrockAuxiliaryClient), not
the AnthropicBedrock SDK — which raises ``RuntimeError: could not resolve
credentials from session`` on bearer tokens. This mirrors the main-loop
routing tested in TestBearerTokenRoutesToConverse. Ref: #28085.
"""

def _resolve_aux(self, monkeypatch, model):
import agent.auxiliary_client as ac

# bedrock is auth_type=aws_sdk in the real PROVIDER_REGISTRY, and with
# the bearer env set has_aws_credentials() is True — no registry mock
# needed. Guard build_anthropic_bedrock_client so a regression that
# routes bearer-token Claude back to the AnthropicBedrock SDK fails loud.
monkeypatch.setenv("AWS_BEARER_TOKEN_BEDROCK", "test-bearer-token-123")
monkeypatch.setenv("AWS_DEFAULT_REGION", "us-east-1")

from agent import anthropic_adapter
def _boom(*a, **k):
raise AssertionError(
"build_anthropic_bedrock_client called on bearer-token path — "
"should route to Converse instead"
)
monkeypatch.setattr(anthropic_adapter, "build_anthropic_bedrock_client", _boom, raising=False)

return ac.resolve_provider_client(provider="bedrock", model=model)

def test_bearer_claude_aux_uses_converse_client(self, monkeypatch):
from agent.auxiliary_client import BedrockAuxiliaryClient, AnthropicAuxiliaryClient

client, final_model = self._resolve_aux(
monkeypatch, "anthropic.claude-opus-4-20250514-v1:0"
)
# Claude + bearer token → Converse shim, NOT AnthropicAuxiliaryClient.
assert isinstance(client, BedrockAuxiliaryClient)
assert not isinstance(client, AnthropicAuxiliaryClient)

def test_bearer_nonclaude_aux_uses_converse_client(self, monkeypatch):
from agent.auxiliary_client import BedrockAuxiliaryClient

client, final_model = self._resolve_aux(
monkeypatch, "amazon.nova-pro-v1:0"
)
assert isinstance(client, BedrockAuxiliaryClient)
6 changes: 5 additions & 1 deletion tests/agent/test_bedrock_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,13 @@ class TestAuxiliaryClientBedrockResolution:

def test_bedrock_returns_client_with_credentials(self, monkeypatch):
"""With valid AWS credentials, Bedrock should return a usable client."""
monkeypatch.setenv("AWS_ACCESS_KEY_ID", "AKIAIOSFODNN7EXAMPLE")
monkeypatch.setenv("AWS_ACCESS_KEY_ID", "AKIAIO...MPLE")
monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")
monkeypatch.setenv("AWS_REGION", "us-west-2")
# Isolate the pure-IAM path: a bearer token in the ambient environment
# (this repo's own dev setup uses one) would otherwise correctly route
# Claude through the Converse shim instead of the AnthropicBedrock SDK.
monkeypatch.delenv("AWS_BEARER_TOKEN_BEDROCK", raising=False)

mock_anthropic_bedrock = MagicMock()
with patch("agent.anthropic_adapter.build_anthropic_bedrock_client",
Expand Down
6 changes: 6 additions & 0 deletions tests/hermes_cli/test_runtime_provider_resolution.py
Original file line number Diff line number Diff line change
Expand Up @@ -1324,6 +1324,12 @@ def _patch_bedrock(monkeypatch, config_default=""):
monkeypatch.setattr(ba, "has_aws_credentials", lambda: True)
monkeypatch.setattr(ba, "resolve_aws_auth_env_var", lambda: "AWS_PROFILE")
monkeypatch.setattr(ba, "resolve_bedrock_region", lambda: "eu-north-1")
# resolve_runtime_provider reads AWS_BEARER_TOKEN_BEDROCK from os.environ
# DIRECTLY (not via the mocked resolve_aws_auth_env_var), so an ambient
# bearer token — present in this repo's own dev environment — would force
# the Converse path and break the IAM/SSO dual-path assertions below.
# Delete it here so the helper models a pure IAM/SSO setup.
monkeypatch.delenv("AWS_BEARER_TOKEN_BEDROCK", raising=False)


def test_resolve_runtime_provider_bedrock_claude_target_model_uses_anthropic_messages(monkeypatch):
Expand Down