Conversation
There was a problem hiding this comment.
3 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="cli/nao_core/commands/debug.py">
<violation number="1" location="cli/nao_core/commands/debug.py:61">
P2: Bearer token path returns success without actually testing connectivity. Every other provider in this function makes an API call to verify the connection works. This path just checks the token string is non-empty, so `nao debug` will report "Connected" even with an invalid/expired token or unreachable region.</violation>
<violation number="2" location="cli/nao_core/commands/debug.py:63">
P2: Missing `ImportError` handling for `boto3`. Other optional provider SDKs (e.g., `ollama`) wrap their import in `try/except ImportError` and return a clear message. This bare import will produce an unhelpful raw error if `boto3` is not installed.</violation>
</file>
<file name="apps/backend/src/agents/providers.ts">
<violation number="1" location="apps/backend/src/agents/providers.ts:302">
P2: Explicitly passing `accessKeyId`/`secretAccessKey` from env vars is redundant — the `@ai-sdk/amazon-bedrock` SDK reads `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` from the environment by default. Worse, this prevents the full AWS credential chain (instance roles, SSO, ECS task roles) from working. Passing just `{ region }` achieves the same behavior and keeps the credential provider chain intact.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| region = os.environ.get("AWS_REGION", "us-east-1") | ||
| bearer_token = api_key or os.environ.get("AWS_BEARER_TOKEN_BEDROCK") | ||
| if bearer_token: | ||
| return True, f"Bearer token configured (region: {region})" |
There was a problem hiding this comment.
P2: Bearer token path returns success without actually testing connectivity. Every other provider in this function makes an API call to verify the connection works. This path just checks the token string is non-empty, so nao debug will report "Connected" even with an invalid/expired token or unreachable region.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/nao_core/commands/debug.py, line 61:
<comment>Bearer token path returns success without actually testing connectivity. Every other provider in this function makes an API call to verify the connection works. This path just checks the token string is non-empty, so `nao debug` will report "Connected" even with an invalid/expired token or unreachable region.</comment>
<file context>
@@ -53,6 +54,17 @@ def _check_available_models(provider: str, api_key: str) -> Tuple[bool, str]:
+ region = os.environ.get("AWS_REGION", "us-east-1")
+ bearer_token = api_key or os.environ.get("AWS_BEARER_TOKEN_BEDROCK")
+ if bearer_token:
+ return True, f"Bearer token configured (region: {region})"
+
+ import boto3
</file context>
| if bearer_token: | ||
| return True, f"Bearer token configured (region: {region})" | ||
|
|
||
| import boto3 |
There was a problem hiding this comment.
P2: Missing ImportError handling for boto3. Other optional provider SDKs (e.g., ollama) wrap their import in try/except ImportError and return a clear message. This bare import will produce an unhelpful raw error if boto3 is not installed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/nao_core/commands/debug.py, line 63:
<comment>Missing `ImportError` handling for `boto3`. Other optional provider SDKs (e.g., `ollama`) wrap their import in `try/except ImportError` and return a clear message. This bare import will produce an unhelpful raw error if `boto3` is not installed.</comment>
<file context>
@@ -53,6 +54,17 @@ def _check_available_models(provider: str, api_key: str) -> Tuple[bool, str]:
+ if bearer_token:
+ return True, f"Bearer token configured (region: {region})"
+
+ import boto3
+
+ client = boto3.client("bedrock", region_name=region)
</file context>
| import boto3 | |
| try: | |
| import boto3 | |
| except ImportError: | |
| return ( | |
| False, | |
| "Provider 'bedrock' requires the optional dependency 'boto3'. Install it to use this provider.", | |
| ) |
apps/backend/src/agents/providers.ts
Outdated
| const region = process.env.AWS_REGION ?? 'us-east-1'; | ||
| const config = settings.apiKey | ||
| ? { apiKey: settings.apiKey, region } | ||
| : { |
There was a problem hiding this comment.
P2: Explicitly passing accessKeyId/secretAccessKey from env vars is redundant — the @ai-sdk/amazon-bedrock SDK reads AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY from the environment by default. Worse, this prevents the full AWS credential chain (instance roles, SSO, ECS task roles) from working. Passing just { region } achieves the same behavior and keeps the credential provider chain intact.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/backend/src/agents/providers.ts, line 302:
<comment>Explicitly passing `accessKeyId`/`secretAccessKey` from env vars is redundant — the `@ai-sdk/amazon-bedrock` SDK reads `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` from the environment by default. Worse, this prevents the full AWS credential chain (instance roles, SSO, ECS task roles) from working. Passing just `{ region }` achieves the same behavior and keeps the credential provider chain intact.</comment>
<file context>
@@ -285,6 +295,17 @@ const MODEL_CREATORS: Record<LlmProvider, ModelCreator> = {
+ const region = process.env.AWS_REGION ?? 'us-east-1';
+ const config = settings.apiKey
+ ? { apiKey: settings.apiKey, region }
+ : {
+ region,
+ accessKeyId: process.env.AWS_ACCESS_KEY_ID,
</file context>
|
Thank you so much for your first contribution I'll test it very soon with API KEY and AWS access/secrets keys and give you feedback. In the meantime could you rebase with main and fix the failing tests pls? |
b55f7ed to
5ef2f68
Compare
Adds support for AWS Bedrock as an LLM provider, with Claude Sonnet 4.6 and Claude Opus 4.6 available by default.
Files touched
providers.tsadds Bedrock provider config and model creator with support for both bearer token and IAM credential authllm.tsaddsbedrockto theLlmProvidertypeusage-filters.tsxadds 'AWS Bedrock' display label.env.examplerequired env vars for both auth methodsdebug.py/__init__.pyCLI support for BedrockAuthentication
Bedrock supports two authentication methods as discussed in #108
AWS_BEARER_TOKEN_BEDROCKAWS_ACCESS_KEY_ID+AWS_SECRET_ACCESS_KEYRegion defaults to
us-east-1ifAWS_REGIONis not set.Note on missing
contextWindowandcostPerMI tested this at work where we have custom pricing. Unlike other providers, Bedrock models do not have
contextWindoworcostPerMfields populated. AWS Bedrock pricing also varies by region, availability zone, and enterprise agreement, making it impractical to hardcode. Since I see examples of providers with no value in these fields, I decided to omit. But happy to consider any modifications here.Note
I've tested this only with bearer token. I don't have access to access key/secret mode of authentication, but boto should handle it well.