fix(bedrock): support bearer-token auth + raw image bytes for aux vision - #28085
fix(bedrock): support bearer-token auth + raw image bytes for aux vision#28085theodore3131 wants to merge 1 commit into
Conversation
4c3a3bb to
192e79b
Compare
outsourc-e
left a comment
There was a problem hiding this comment.
Ran the touched Bedrock/auxiliary suites locally:
- python3 -m pytest -q -o addopts='' tests/agent/test_auxiliary_client_bedrock.py tests/agent/test_bedrock_adapter.py tests/agent/test_bedrock_integration.py tests/agent/test_resolve_task_provider_priority.py tests/agent/test_unsupported_parameter_retry.py
Result: 209 passed.
I also checked the earlier async concern — the PR now has explicit async wrapper coverage for the converse client, so this looks merge-ready on the auxiliary/vision scope.
|
Hi @teknium1 / @kshitijk4poor — this one's been approved by @outsourc-e but hasn't been merged yet. Would one of you be able to take a look when you have a chance? Quick recap of impact: without this fix, Bedrock users who authenticate via Happy to rebase if needed. Thanks! |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused Bedrock aux/vision fix. I verified the underlying bugs still exist on current main: Bedrock aux routing always uses AnthropicAuxiliaryClient at agent/auxiliary_client.py:3938, data URL images still pass the base64 string into image.source.bytes at agent/bedrock_adapter.py:517, and _resolve_task_provider_model still forces base_url paths to custom before explicit providers at agent/auxiliary_client.py:4729.
Problems
- The new bearer-token async path still looks broken. The PR returns
_to_async_client(client, ...)forBedrockConverseAuxiliaryClientat agent/auxiliary_client.py:3600, but_to_async_clienthas no Bedrock branch on current main and falls through toAsyncOpenAI(api_key='aws-sdk', base_url=bedrock-runtime...)at agent/auxiliary_client.py:3288. That reintroduces the OpenAI-wire Bedrock mismatch for async auxiliary callers. - The malformed data URL handling is weaker than the test suggests. The PR uses
base64.b64decode(data, validate=False)at agent/bedrock_adapter.py:486; Python accepts the test payload====!!notvalidand returns bytes rather than raising, so malformed images are not reliably skipped.
Suggested changes
- Add a
_to_async_clientbranch returningAsyncBedrockConverseAuxiliaryClient(sync_client)and coverresolve_provider_client('bedrock', ..., async_mode=True)withAWS_BEARER_TOKEN_BEDROCKset. - Switch the data URL decode to strict validation and assert the malformed payload produces no image block.
Automated hermes-sweeper review; humans decide final merge/salvage.
| "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 |
There was a problem hiding this comment.
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.
| # double-encodes the payload and the model returns | ||
| # ``ValidationException: Could not process image``. | ||
| try: | ||
| image_bytes = base64.b64decode(data, validate=False) |
There was a problem hiding this comment.
validate=False does not reliably reject malformed data URLs; for example the new test payload ====!!notvalid decodes to bytes under Python's loose decoder. Use validate=True (and probably assert a non-empty decoded payload) if the intended behavior is to skip malformed images.
| # truly cannot be decoded as base64 even loosely). | ||
| # An odd-length truncated string with a pad in the | ||
| # middle reliably raises binascii.Error. | ||
| "url": "data:image/png;base64,====!!notvalid", |
There was a problem hiding this comment.
This payload is not a reliable malformed-base64 fixture with validate=False: Python decodes it to bytes instead of raising, so the test can pass without exercising the skip path. Use a payload that fails strict validation and assert no image block is present.
|
Verified this closes the auxiliary bearer-token gap from #29309, applied on top of #24507. The head ( Result under bearer-only Bedrock: title generation and context compression now succeed, where previously every new session failed with One caveat worth flagging: #24507 + #28085 are the two PRs closest to a complete bearer-token Bedrock fix. |
|
Update — verified
|
|
Friendly ping @teknium1 — this is now in a finished state and ready to land. Your 2026-06-15 review asked for two things: the missing
No conflicts ( |
|
@teknium1 Is there a chance to get this merged for the next update or do you have any other workarounds to get Anthropic models work on AWS Bedrock? |
acd5de2 to
e184763
Compare
Review — rebase required; core aux-vision fix is still live and worth salvagingReviewed the 3-commit bedrock delta ( Already landed on main independently — drop these on rebase
Still broken on main — this is the reason to keep this PR
Recommended actionRebase onto current main, keep commits |
When AWS_BEARER_TOKEN_BEDROCK is the auth source, the auxiliary client's aws_sdk branch routed Claude models to the AnthropicBedrock SDK, which only supports IAM credentials from the boto3 chain and raises "RuntimeError: could not resolve credentials from session" on a bearer token. Route Claude (and everything else) through the existing BedrockAuxiliaryClient Converse shim in that case — boto3's Converse call picks up the bearer token natively and supports all Bedrock models. This mirrors the dual-path routing already present in hermes_cli.runtime_provider for the main model loop. Also: - _is_unsupported_parameter_error now matches Bedrock Converse's "`temperature` is deprecated for this model." phrasing so the reactive retry fires instead of surfacing an empty all-None ChatCompletion on Opus 4.7 / Sonnet 4.5. - _convert_content_to_converse decodes image data URLs with validate=True and skips malformed/empty payloads rather than forwarding the raw base64 string as bytes (which Bedrock rejects with ValidationException).
e184763 to
bd906d3
Compare
Rebased onto current main — now mergeable, scope reduced to the one still-live bugForce-pushed a clean rebase ( Dropped (already on main):
Kept (still broken on main):
Tests: added Diff shrank from +937/-9 across 10 files to +130/-11 across 5 files. |
|
Thanks for the careful review, @teknium1. Both concerns are already addressed on the current HEAD ( 1. async bearer-token path —
2. malformed data URL — the decode now uses strict validation and skips the block on failure instead of forwarding junk bytes:
Both paths have test coverage (async wrapper + malformed-payload skip). @outsourc-e re-ran the touched Bedrock/aux suites — 209 passed. Since the review points are resolved and the branch is mergeable, could this get another look for merge? Happy to rebase if needed. |
fix(bedrock): support bearer-token auth + raw image bytes for aux vision
The auxiliary client could not service vision (or any other) tasks for
users who authenticated to Bedrock via AWS_BEARER_TOKEN_BEDROCK. Four
independent bugs combined to break this path end-to-end; all are fixed
here so a single PR restores the feature.
auxiliary_client.py — bearer-token routing
resolve_provider_client("bedrock", ...) unconditionally wrapped a
freshly-built anthropic.AnthropicBedrock client in
AnthropicAuxiliaryClient. AnthropicBedrock's auth helper only
resolves IAM credentials from the boto3 credential chain and
raises 'RuntimeError: could not resolve credentials from session'
the moment a request is dispatched on bearer-token auth. The main
model loop already handles this in
hermes_cli.runtime_provider._build_runtime_provider by falling
back to the boto3 Converse API for bearer tokens; the auxiliary
client now mirrors that dual-path routing via a new
BedrockConverseAuxiliaryClient (sync) and
AsyncBedrockConverseAuxiliaryClient (async) that delegate to
bedrock_adapter.call_converse.
bedrock_adapter.py — image payload double-encoding
convert_messages_to_converse forwarded the base64 string from a
data: URL into image.source.bytes. That field is a Bedrock blob
shape: boto3 expects raw bytes and base64-encodes the wire
payload itself, so passing the string double-encoded the image
and Bedrock returned 'ValidationException: Could not process
image'. Decode the base64 once before assembling the block;
gracefully skip malformed payloads.
auxiliary_client.py — _resolve_task_provider_model priority
The "base_url + api_key implies provider=custom" heuristic ran
before the explicit-provider check. When the user (or
hermes auxiliary) wrotethe provider directive was silently rewritten to "custom",
handing the Bedrock URL + bearer token to the generic OpenAI
HTTP client. Bedrock 200s on POST /chat/completions with an
empty body, which surfaces downstream as
'ChatCompletion(id=None, choices=None, ...)'. This trap also
bites any other named provider whose adapter is incompatible
with the OpenAI chat-completions wire (anthropic, nous,
openai-codex, ...). Fix: explicit known provider wins over the
heuristic, in both the early-args path and the task-config
path.
auxiliary_client.py — _is_unsupported_parameter_error markers
Bedrock Converse + newer Anthropic models (Opus 4.7, Sonnet
4.5+) reject
temperaturewith the wordingThe "deprecated" marker was missing from the detector, so the
reactive-retry branch in call_llm never fired and the boto3
ValidationException was swallowed by downstream wrappers,
surfacing — once again — as an empty ChatCompletion object.
Add "is deprecated" to the marker set so the existing
strip-and-retry path engages automatically.
Tests
routing split — bearer token picks BedrockConverseAuxiliaryClient,
IAM credentials still pick AnthropicAuxiliaryClient — plus the
adapter's call_converse delegation, max_completion_tokens fallback,
stop-string normalization, and async wrapper identity for cache
eviction.
URL → raw bytes conversion, JPEG format propagation, and graceful
handling of malformed payloads.
TestAuxiliaryClientBedrockResolution suite gains an autouse
_clean_aws_env fixture so user-level AWS_BEARER_TOKEN_BEDROCK no
longer leaks in and routes the IAM-path tests through the wrong
client.
in priority — explicit provider:bedrock with base_url+api_key
must resolve to bedrock, not custom. Covers anthropic,
openai-codex, plain custom-endpoint, auto, and the
task-config path.
parametrize to cover Bedrock's "
temperatureis deprecated forthis model" wording (full ValidationException string and the
bare phrase).
End-to-end smoke: aux vision via Bedrock + bearer token now correctly
identifies a 64x64 red PNG via us.anthropic.claude-opus-4-7 (reply:
'Red', 34 prompt + 6 completion tokens) AND a real screenshot via
us.anthropic.claude-sonnet-4-6 (reply describing portfolio app
contents in full). Full agent test suite passes (3092 tests).