fix(bedrock): honor api_mode=bedrock_converse so bearer-token auth works for Claude - #64857
fix(bedrock): honor api_mode=bedrock_converse so bearer-token auth works for Claude#64857taoxee wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused Bedrock fix. Current main already addresses the bearer-token-only failure by routing AWS_BEARER_TOKEN_BEDROCK users to Converse in hermes_cli/runtime_provider.py:1979-2008 (commit 5e6a0d9ee). The remaining explicit override and Converse 1M-context behavior are still worth evaluating.
Problems
- The routing hunk conflicts with current behavior: the current Claude condition is
is_anthropic_bedrock_model(...) and not _has_bearer_tokenathermes_cli/runtime_provider.py:1985-1986. Integrate the explicitbedrock_converseoverride with that condition so bearer-token routing remains intact. - The PR adds no tests. Main's bearer routing is covered in
tests/agent/test_bedrock_adapter.py:1756-1796; add coverage for the explicit override and for theadditionalModelRequestFieldsgenerated bybuild_converse_kwargs().
Suggested changes
- Preserve main's automatic bearer-token route and add the explicit-config route as an additional Converse selector.
- Add focused resolver and request-shaping tests for the two new paths.
Automated hermes-sweeper review.
| # with SigV4 and ignores AWS_BEARER_TOKEN_BEDROCK — so bearer-token-only | ||
| # setups (no IAM key, no ~/.aws) must route Claude through the Converse | ||
| # API, which DOES honor the bearer token. The Converse path injects the | ||
| # 1M-context beta via additionalModelRequestFields (see |
There was a problem hiding this comment.
Current main now selects Converse for AWS_BEARER_TOKEN_BEDROCK at runtime_provider.py:1979-2008. Please integrate this override with that bearer-token condition rather than replacing the current routing logic, so both selectors retain the Converse path.
…e for Claude main already routes bearer-token Claude users through the Converse API (5e6a0d9), but that path does NOT set the 1M-context beta — so those users silently lose the 1M window that the AnthropicBedrock SDK path gets via its client-level anthropic-beta header. This adds the beta to the Converse path and layers an explicit config override on top. - resolve_runtime_provider(): honor an explicit model.api_mode: bedrock_converse as an additional Converse selector, integrated with main's bearer-token condition (either signal routes Claude to Converse). Default behavior is unchanged: no override + no bearer token → Claude keeps the AnthropicBedrock SDK path. - build_converse_kwargs(): inject the 1M-context beta via additionalModelRequestFields, gated on is_anthropic_bedrock_model(). anthropic_beta is an Anthropic-only field, so non-Claude Converse models (Nova, Llama, DeepSeek) must not receive it or Bedrock 400s. Tests: resolver coverage for the explicit override (SigV4 + bearer, and the unchanged default), and request-shaping coverage asserting the 1M beta is present for Claude and absent for non-Claude models.
34875a6 to
5895796
Compare
|
Rebased onto current
|
SummaryNine PRs address or reference the Bedrock complex: #24507, #26531, #34742, and merged #64628 change bearer-token routing; #28085, #33536, #34742, and #64628 decode image payloads; #64628 also fixes region-scoped discovery, while #54431/#63650 update the Anthropic SDK and #64857 adds an explicit Converse override plus 1M-context request shaping. The merged #64628 is the recorded best fix for both #28156 and #33317, but the visible diffs leave separable auxiliary, strict-validation, SDK-version, context-metadata, and explicit-override work in some open PRs. Related pull requests
Duplicates#24507, #26531, and the bearer-routing portion of #34742 overlap the routing now merged in #64628; #33536 and the image-decoding portions of #28085/#34742 duplicate #64628's raw-bytes fix. #54431 and #63650 overlap on the Anthropic SDK bump, while #64857's original bearer fix overlaps #64628 but its explicit override and Converse 1M request shaping do not. Suggested consolidationKeep #64857 open with a salvage path limited to the explicit Converse override and tested Claude-only 1M request shaping, and keep #28085 open only for auxiliary bearer routing plus strict malformed-image/deprecated-parameter hardening. Close #24507 and #33536 as duplicates of merged #64628; leave closed #26531/#34742 closed as superseded or salvaged references. For #54431 and #63650, require author action to rebase and either select one clean SDK-pin update or split #63650's tested context metadata from the competing version bump; do not disturb merged #64628, the recorded best fix for both issues. Cross-PR triage: Reviewed 9 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 86 kB of PR diffs, 36 kB of issue/PR text, 41 kB of discussion (50 comments), 11 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Problem
Claude models on Bedrock are hardcoded to
api_mode=anthropic_messagesinresolve_runtime_provider()(hermes_cli/runtime_provider.py), which routes them through the AnthropicBedrock SDK. That SDK signs requests with SigV4 and resolves credentials via boto3'ssession.get_credentials()— it never readsAWS_BEARER_TOKEN_BEDROCK.For users authenticated with a Bedrock bearer token only (no IAM access key, no
~/.aws), every Claude call fails:The retries can't recover a missing credential, so it always exhausts them. Non-Claude Bedrock models already use the Converse API, which does honor the bearer token.
Fix
resolve_runtime_provider(): honor an explicitmodel.api_mode: bedrock_conversein config so Claude-on-Bedrock can route through the Converse API too. Default behavior is unchanged — without that config, Claude still uses the AnthropicBedrock path.build_converse_kwargs(): inject the 1M-context beta viaadditionalModelRequestFieldsso the Converse path keeps the 1M context window (the AnthropicBedrock client attaches it as a client-levelanthropic-betaheader). Bedrock ignores the beta for models that don't support it, so it's safe to send unconditionally — matchingbuild_anthropic_bedrock_client().Testing
Verified on
us-east-1with a bearer-token-only setup (no IAM):resolve_runtime_provider()returnsbedrock_conversewhen configured.claude-sonnet-5andclaude-opus-4-8succeed (end_turn, real content) using onlyAWS_BEARER_TOKEN_BEDROCK.