Add AWS Bedrock provider support #1045
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mfenderov
force-pushed
the
feature/bedrock-provider
branch
from
254c74e to
d657e41
Compare
|
Seems related to #542 |
Contributor
|
Finally got around to testing things with bedrock setup properly, sorry for the delay. This provider seems to work, but i noticed that cost/usage information is not being handled properly, the TUI shows both tokens and cost at zero. The models.dev data we use for pricing should include plenty of bedrock models, so we can use that and consider it good enough. With usage tracked, I think it'd be enough to merge this for now |
Implement Bedrock provider using AWS SDK v2 with: - Default credential chain (env vars, shared config, IAM roles) - Support for all Bedrock models via Converse API - Cross-region inference profiles - provider_opts: region, profile, role_arn, external_id, endpoint_url - Streaming responses with tool calling support New files: - pkg/model/provider/bedrock/client.go - Main client - pkg/model/provider/bedrock/convert.go - Message/tool conversion - pkg/model/provider/bedrock/adapter.go - Stream adapter - pkg/model/provider/bedrock/client_test.go - Unit tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add prerequisites section with link to Bedrock Console - Link to AWS SDK credential chain documentation - Clarify authentication flow 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add bearer token auth support (AWS_BEARER_TOKEN_BEDROCK or api_key provider_opt) - Update default model to global.anthropic.claude-sonnet-4-5-20250929-v1:0 - Add inference profile prefix documentation (global., us., eu.) - Add unit test for bearer token transport - Update docs with API key authentication section 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Bedrock's Converse API requires that multiple tool results be grouped into a single user message. The previous implementation created separate messages for each tool result, causing ValidationException: "Expected toolResult blocks at messages.N.content for the following Ids" Changes: - Modified loop to use index-based iteration for index manipulation - Added grouping logic for consecutive MessageRoleTool messages - Added comprehensive test for multi-tool result scenarios 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add 7 new tests to improve coverage: - TestConvertImageURL_NonDataURL - TestConvertImageURL_InvalidDataURLFormat - TestConvertImageURL_InvalidBase64 - TestConvertImageURL_AllFormats (table-driven) - TestConvertImageURL_ValidImage - TestNewClient_NilConfig - TestNewClient_WrongProvider These tests cover critical error paths identified in code review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add pr-reviewer-bedrock.yaml - a multi-agent PR review toolkit that mirrors Claude Code's pr-review-toolkit plugin with 6 specialized agents: - code-reviewer: Guidelines compliance and bug detection - silent-failure-hunter: Error handling audit - code-simplifier: Code simplification - comment-analyzer: Comment quality analysis - test-analyzer: Test coverage analysis - type-analyzer: Type design and invariants Also adds 8 new tests for Bedrock provider: - buildAWSConfig tests (region precedence, env vs provider_opts) - NewClient validation tests (valid config, bearer token paths) - getProviderOpt type mismatch edge cases - Interface compliance assertion Test coverage: Bedrock now at 58% (highest of all providers). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
mfenderov
force-pushed
the
feature/bedrock-provider
branch
from
00b6939 to
f548e0c
Compare
- Update env.Get() calls to handle new (string, bool) return signature - Fix MaxTokens pointer dereference (*int64 instead of int64) - Update mockEnvProvider in tests to match new interface - Remove version field from example (examples shouldn't define version) - Fix typo in example comment 🤖 Co-Authored-By: Claude <[email protected]>
- Add support for cache tokens (CacheReadInputTokens, CacheWriteInputTokens) - Add debug logging to help diagnose usage tracking issues - Add tests for deref helper functions and usage conversion This addresses PR feedback about TUI showing zero tokens/cost. 🤖 Co-Authored-By: Claude <[email protected]>
krissetto
reviewed
Jan 8, 2026
Match models.dev naming convention for automatic pricing lookup. 🤖 Co-Authored-By: Claude <[email protected]>
krissetto
reviewed
Jan 8, 2026
Address review feedback: - Remove amazon-bedrock skip since models.dev has pricing data - Add amazon-bedrock to TestDefaultModels expected providers - Improve dmr skip comment to explain why (local models) 🤖 Co-Authored-By: Claude <[email protected]>
krissetto
reviewed
Jan 8, 2026
Contributor
|
After that last issue regarding the |
Replace api_key provider_opt with token_key field for Bedrock authentication. This aligns with other providers and keeps agent configs shareable by storing env var names instead of secrets directly in YAML. Changes: - Use cfg.TokenKey to specify custom env var name for bearer token - Default to AWS_BEARER_TOKEN_BEDROCK if token_key not specified - Add debug logging when token_key is configured but env var is empty - Update docs to show token_key usage pattern - Fix test to use new TokenKey field Addresses @krissetto's review feedback on PR docker#1045. 🤖 Co-Authored-By: Claude <[email protected]>
Signed-off-by: Christopher Petito <[email protected]>
krissetto
approved these changes
Jan 8, 2026
rumpl
approved these changes
Jan 8, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add AWS Bedrock provider using the Converse API. Supports all Bedrock models (Claude, Titan, Llama, Mistral) with AWS credential chain and Bedrock API key authentication.
This is an initial implementation to keep PR size manageable. Future additions: prompt caching, IAM policy examples, troubleshooting docs.
P.S.
There's an existing PR #542 that also adds Bedrock support, but there's been no movement in more than a month.