[Feature] Added AWS Bedrock Integration for Claude #705
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds AWS Bedrock support for Claude: server-side Bedrock credential handling and model mapping, async model-list APIs, a Bedrock status endpoint, UI controls and hooks for Bedrock credentials/status, Bedrock model options and display adjustments, provider factory/type updates, and related plumbing across server, UI, and libs. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client/UI
participant Server as Server (ClaudeProvider)
participant Settings as SettingsService
participant BedrockSDK as AWS Bedrock SDK
participant AnthropicSDK as Anthropic SDK
UI->>Server: executeQuery(model, messages, options)
Server->>Settings: getCredentials()
Settings-->>Server: credentials (includes awsBedrock)
Server->>Server: mapToBedrockModel(model)
alt Bedrock configured & enabled
Server->>BedrockSDK: send messages(mappedModel, sdkOptions)
BedrockSDK-->>Server: response
else Use Anthropic directly
Server->>AnthropicSDK: send messages(model, sdkOptions)
AnthropicSDK-->>Server: response
end
Server-->>UI: result
sequenceDiagram
participant App as App Startup
participant Root as RootLayout
participant Hook as useBedrockStatus
participant API as /api/settings/bedrock-status
participant Store as App Store
App->>Root: mount
Root->>Hook: invoke
Hook->>API: GET /api/settings/bedrock-status
API-->>Hook: {bedrockConfigured, bedrockEnabled, region}
Hook->>Store: setBedrockConfigured(...)
Hook->>Store: setBedrockEnabled(...)
Store-->>Root: state updated
Root-->>App: render with Bedrock-aware model lists
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @frederikschr, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant feature by integrating AWS Bedrock as an alternative backend for Claude models. It enables users, particularly those in enterprise environments with AWS restrictions, to leverage their existing AWS infrastructure for accessing Claude. The changes span across backend services for credential management and model resolution, and frontend components for user configuration and model selection, ensuring a seamless experience for Bedrock users. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
frederikschr
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces AWS Bedrock integration for Claude models, which is a significant enhancement. The implementation is well-structured, correctly isolating the Bedrock logic within the ClaudeProvider and updating the provider factory and settings services accordingly. The UI changes for configuration are also comprehensive. I've identified a few areas for improvement, primarily concerning type safety, code structure, and a minor bug in the UI validation logic for Bedrock credentials. Addressing these points will enhance the maintainability and robustness of this new feature.
| if (enabled) { | ||
| if (!accessKeyId || !secretAccessKey) { | ||
| toast.error('Access Key ID and Secret Access Key are required when enabled'); | ||
| return; | ||
| } | ||
| if (!region) { | ||
| toast.error('AWS Region is required when enabled'); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
The current validation logic requires users to re-enter their AWS credentials every time they save changes in this section if Bedrock is enabled. This is because the key-related state variables are always empty for security. This creates a poor user experience, especially when only changing the region or disabling the feature.
The validation should only enforce key presence when Bedrock is being enabled for the first time (i.e., when enabled is true and isConfigured is false).
| if (enabled) { | |
| if (!accessKeyId || !secretAccessKey) { | |
| toast.error('Access Key ID and Secret Access Key are required when enabled'); | |
| return; | |
| } | |
| if (!region) { | |
| toast.error('AWS Region is required when enabled'); | |
| return; | |
| } | |
| } | |
| if (enabled) { | |
| // Only require keys if enabling for the first time and they haven't been provided. | |
| if (!isConfigured && (!accessKeyId || !secretAccessKey)) { | |
| toast.error('Access Key ID and Secret Access Key are required when enabling Bedrock for the first time.'); | |
| return; | |
| } | |
| if (!region) { | |
| toast.error('AWS Region is required when enabled'); | |
| return; | |
| } | |
| } |
| async function mapToBedrockModel(model: string, provider: ClaudeProvider): Promise<string> { | ||
| // If already a Bedrock model ID, pass through | ||
| if (model.includes('.anthropic.claude-') || model.startsWith('anthropic.claude-')) { | ||
| logger.debug(`Model already in Bedrock format: ${model}`); | ||
| return model; | ||
| } | ||
|
|
||
| // If Bedrock not configured, pass through unchanged | ||
| if (!(await provider['isBedrockConfigured']())) { | ||
| return model; | ||
| } | ||
| return env; | ||
|
|
||
| // Get AWS region for region-specific model IDs (check ENV first, then credentials) | ||
| let region = process.env.AWS_REGION || process.env.AWS_DEFAULT_REGION; | ||
| if (!region) { | ||
| const config = await provider['getBedrockConfig'](); | ||
| region = config?.region || 'us-east-1'; | ||
| } | ||
| const regionPrefix = region.startsWith('eu-') ? 'eu' : 'us'; | ||
|
|
||
| // Map standard Claude model IDs to Bedrock format | ||
| // Use Cross-Region Inference profiles (with region prefix) for models that support it | ||
| const bedrockModelMap: Record<string, string> = { | ||
| 'claude-opus-4-5-20251101': `${regionPrefix}.anthropic.claude-opus-4-5-20251101-v1:0`, | ||
| 'claude-sonnet-4-5-20250929': `${regionPrefix}.anthropic.claude-sonnet-4-5-20250929-v1:0`, | ||
| 'claude-sonnet-4-20250514': `${regionPrefix}.anthropic.claude-sonnet-4-5-20250929-v1:0`, // Legacy | ||
| 'claude-3-5-sonnet-20241022': 'anthropic.claude-3-5-sonnet-20241022-v2:0', | ||
| 'claude-haiku-4-5-20251001': `${regionPrefix}.anthropic.claude-haiku-4-5-20251001-v1:0`, | ||
| }; | ||
|
|
||
| const bedrockModel = bedrockModelMap[model]; | ||
| if (bedrockModel) { | ||
| logger.info(`Mapped Claude model to Bedrock: ${model} → ${bedrockModel}`); | ||
| return bedrockModel; | ||
| } | ||
|
|
||
| logger.warn(`No Bedrock mapping for model: ${model}, using as-is`); | ||
| return model; | ||
| } |
There was a problem hiding this comment.
For better encapsulation and type safety, consider moving mapToBedrockModel to be a private method within the ClaudeProvider class. Currently, it's a free function that accesses private methods of ClaudeProvider using string indexing (e.g., provider['isBedrockConfigured']()), which bypasses TypeScript's static analysis. Making it a private method would allow direct and type-safe access to this.isBedrockConfigured() and this.getBedrockConfig(), improving code maintainability and adhering to object-oriented principles.
| private static settingsService: any | null = null; | ||
|
|
||
| /** | ||
| * Initialize provider factory with settings service | ||
| * Should be called once during server startup | ||
| */ | ||
| static initialize(settingsService: any): void { | ||
| this.settingsService = settingsService; | ||
| } | ||
|
|
||
| /** | ||
| * Get the settings service instance | ||
| */ | ||
| static getSettingsService(): any | null { | ||
| return this.settingsService; | ||
| } |
There was a problem hiding this comment.
The settingsService is currently typed as any, which compromises type safety. To improve maintainability and leverage TypeScript's benefits, it should be strongly typed. Please import SettingsService from ../services/settings-service.js and use it as the type for the settingsService property and in the method signatures.
| private static settingsService: any | null = null; | |
| /** | |
| * Initialize provider factory with settings service | |
| * Should be called once during server startup | |
| */ | |
| static initialize(settingsService: any): void { | |
| this.settingsService = settingsService; | |
| } | |
| /** | |
| * Get the settings service instance | |
| */ | |
| static getSettingsService(): any | null { | |
| return this.settingsService; | |
| } | |
| private static settingsService: SettingsService | null = null; | |
| /** | |
| * Initialize provider factory with settings service | |
| * Should be called once during server startup | |
| */ | |
| static initialize(settingsService: SettingsService): void { | |
| this.settingsService = settingsService; | |
| } | |
| /** | |
| * Get the settings service instance | |
| */ | |
| static getSettingsService(): SettingsService | null { | |
| return this.settingsService; | |
| } |
| }; | ||
| }> | ||
| ): Promise<string> { | ||
| async function extractTextFromStream(stream: AsyncIterable<any>): Promise<string> { |
There was a problem hiding this comment.
The stream parameter is typed as AsyncIterable<any>, which reduces type safety. The previous implementation had a more specific type. To restore type safety and improve code clarity, please define a more specific type for the stream messages that reflects the expected structure, including the possibility of message.content being a string or an array of blocks.
async function extractTextFromStream(stream: AsyncIterable<{
type: string;
subtype?: string;
result?: string;
message?: {
content?: string | Array<{ type: string; text?: string }>;
};
}>): Promise<string> {![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/ui/src/components/views/board-view/shared/model-selector.tsx (1)
95-107: Default Claude selection should honor Bedrock-only mode.When Bedrock is enabled, switching providers forces
'sonnet', which isn’t in the Bedrock-only list—so nothing appears selected (and it may not be a valid Bedrock model). Default to a Bedrock model whenbedrockEnabledis true.🐛 Suggested fix
- } else if (provider === 'claude' && selectedProvider !== 'claude') { - // Switch to Claude's default model - onModelSelect('sonnet'); - } + } else if (provider === 'claude' && selectedProvider !== 'claude') { + const defaultClaudeModel = bedrockEnabled + ? BEDROCK_MODELS[0]?.id ?? 'sonnet' + : 'sonnet'; + onModelSelect(defaultClaudeModel); + }apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx (1)
305-365: AddclaudeModelsto the dependency arrays to prevent stale UI when toggling Bedrock.Both
currentModel(lines 305–365) and the grouped models memo (lines 422–473) readclaudeModels, but don't include it in their dependency arrays. When Bedrock is toggled,claudeModelsis recalculated, but these memos won't re-run, causing the UI to display stale selections.Suggested fix
}, [ selectedModel, selectedThinkingLevel, + claudeModels, availableCursorModels, transformedCodexModels, dynamicOpencodeModels, ]);- }, [favoriteModels, availableCursorModels, transformedCodexModels, allOpencodeModels]); + }, [favoriteModels, claudeModels, availableCursorModels, transformedCodexModels, allOpencodeModels]);
🤖 Fix all issues with AI agents
In `@apps/server/src/routes/settings/routes/get-bedrock-status.ts`:
- Around line 17-27: The current logic sets bedrockConfigured based on
config?.enabled which hides "configured but disabled" state; change the check so
bedrockConfigured is true when AWS Bedrock credentials exist regardless of
enabled (e.g., derive isConfigured from presence of config.accessKeyId and
config.region or necessary secrets obtained via settingsService.getCredentials),
keep bedrockEnabled using config?.enabled, and ensure region returns the region
only if credentials exist (use the same credentials-presence check instead of
config?.enabled).
In `@apps/server/src/services/settings-service.ts`:
- Around line 479-485: The awsBedrock configured flag in
apps/server/src/services/settings-service.ts only checks accessKeyId and region,
so it can be true when secretAccessKey is missing; update the configured
expression for the awsBedrock object to also require
credentials.awsBedrock?.secretAccessKey (e.g., include it in the boolean check
alongside accessKeyId and region) so the UI reflects true authentication
readiness; ensure you still use maskKey for accessKeyMasked/secretKeyMasked and
maintain existing enabled/region fallbacks.
In `@apps/ui/src/components/views/board-view/shared/model-constants.ts`:
- Around line 65-72: The entry with id
'anthropic.claude-3-5-sonnet-20241022-v2:0' incorrectly sets hasThinking to
true; update that model object so hasThinking: false (remove any UI logic that
relies on this flag for claude-3-5-sonnet-20241022 variants) so the UI won't
show thinking controls for Claude 3.5 Sonnet.
In
`@apps/ui/src/components/views/settings-view/providers/claude-settings-tab/aws-bedrock-section.tsx`:
- Around line 118-147: The handleSave flow currently always sends empty strings
for accessKeyId/secretAccessKey/sessionToken which wipes stored creds; change
the payload construction before calling api.settings.updateCredentials so
awsBedrock always includes enabled and region but only includes accessKeyId,
secretAccessKey and sessionToken properties when the corresponding local values
are non-empty (i.e., conditionally add them to the object), and do not send
empty strings or explicit clears unless the user explicitly chooses a "clear"
action; update the call site in handleSave where api.settings.updateCredentials
is invoked to use this conditional payload.
🧹 Nitpick comments (7)
libs/model-resolver/src/resolver.ts (1)
83-93: Redundant Bedrock checks can be simplified.
modelKey.includes('claude-')already covers the other two conditions, so the extra checks don’t change behavior and can confuse future readers. Consider collapsing to a single check (or switch to a stricter regex if you intended to narrow allowed formats).♻️ Proposed simplification
- if ( - modelKey.includes('claude-') || - modelKey.includes('.anthropic.claude-') || - modelKey.startsWith('anthropic.claude-') - ) { + if (modelKey.includes('claude-')) {apps/server/src/providers/claude-provider.ts (3)
48-86: Avoid accessing private methods via string indexing.The
mapToBedrockModelfunction accesses private methods onClaudeProviderusing string indexing (provider['isBedrockConfigured']()), which bypasses TypeScript's access control and creates fragile coupling. Consider making this a method on the class instead.♻️ Suggested refactor
-async function mapToBedrockModel(model: string, provider: ClaudeProvider): Promise<string> { - // If already a Bedrock model ID, pass through - if (model.includes('.anthropic.claude-') || model.startsWith('anthropic.claude-')) { - logger.debug(`Model already in Bedrock format: ${model}`); - return model; - } - - // If Bedrock not configured, pass through unchanged - if (!(await provider['isBedrockConfigured']())) { - return model; - } - - // Get AWS region for region-specific model IDs (check ENV first, then credentials) - let region = process.env.AWS_REGION || process.env.AWS_DEFAULT_REGION; - if (!region) { - const config = await provider['getBedrockConfig'](); - region = config?.region || 'us-east-1'; - }Move this logic into
ClaudeProvideras a private methodmapToBedrockModel(model: string): Promise<string>that can directly callthis.isBedrockConfigured()andthis.getBedrockConfig().
111-114: Dynamic import may cause circular dependency issues.The dynamic
import('./provider-factory.js')insidegetBedrockConfigis used to access the settings service. While this defers the import, it creates an implicit dependency cycle (provider-factory imports claude-provider, claude-provider dynamically imports provider-factory).Consider injecting the settings service during provider construction or using a separate settings accessor module to avoid this pattern.
369-427: Bedrock model IDs are hardcoded without region awareness.The Bedrock models in
getAvailableModels()use hardcoded region prefixes (eu.andus.), but the actual region used at runtime comes from configuration. This may confuse users if their configured region doesn't match the displayed models.Consider either:
- Generating model IDs dynamically based on the configured region
- Showing only region-agnostic model IDs (without
eu./us.prefix)- Filtering to show only models matching the configured region
apps/server/src/providers/provider-factory.ts (1)
62-77: Consider typing the settings service properly.Using
anyforsettingsServiceloses type safety. If theSettingsServicetype is available, use it here.♻️ Suggested improvement
+import type { SettingsService } from '../services/settings-service.js'; + export class ProviderFactory { - private static settingsService: any | null = null; + private static settingsService: SettingsService | null = null; - static initialize(settingsService: any): void { + static initialize(settingsService: SettingsService): void { this.settingsService = settingsService; } - static getSettingsService(): any | null { + static getSettingsService(): SettingsService | null { return this.settingsService; }apps/ui/src/components/views/settings-view/providers/claude-settings-tab.tsx (1)
10-41: Add AwsBedrockSection to the barrel export for consistency.
Other components in this directory (SkillsSection, SubagentsSection, SubagentCard) are exported from the./claude-settings-tab/index.tsbarrel. Update the barrel to include AwsBedrockSection, then import from it to maintain consistency and refactor-friendliness.export { AwsBedrockSection } from './aws-bedrock-section';Then update the import in
claude-settings-tab.tsxto use the barrel.apps/server/src/index.ts (1)
170-173: Move import to top of file with other imports.The inline
importstatement at line 172 works but violates ES module conventions where all imports should be declared at the top of the file. This improves readability and makes dependencies immediately visible.♻️ Suggested refactor
Move the import to the top of the file with other imports (around line 48):
import { ProviderFactory } from './providers/provider-factory.js';Then update lines 170-173 to just:
-// Initialize ProviderFactory with settingsService for Bedrock credential access -import { ProviderFactory } from './providers/provider-factory.js'; -ProviderFactory.initialize(settingsService); +// Initialize ProviderFactory with settingsService for Bedrock credential access +ProviderFactory.initialize(settingsService);
| const credentials = await settingsService.getCredentials(); | ||
| const config = credentials.awsBedrock; | ||
|
|
||
| const isConfigured = !!(config?.enabled && config.accessKeyId && config.region); | ||
|
|
||
| res.json({ | ||
| success: true, | ||
| bedrockConfigured: isConfigured, | ||
| bedrockEnabled: config?.enabled || false, | ||
| region: isConfigured ? config?.region : null, | ||
| }); |
There was a problem hiding this comment.
bedrockConfigured shouldn’t depend on enabled.
Requiring config.enabled means “configured but disabled” can never be reported, which breaks the UI flow that shows both Claude + Bedrock lists when credentials exist. Consider checking credentials independently (and include the secret key if required).
🐛 Suggested fix
- const isConfigured = !!(config?.enabled && config.accessKeyId && config.region);
+ const isConfigured = !!(
+ config?.accessKeyId &&
+ config?.secretAccessKey &&
+ config?.region
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const credentials = await settingsService.getCredentials(); | |
| const config = credentials.awsBedrock; | |
| const isConfigured = !!(config?.enabled && config.accessKeyId && config.region); | |
| res.json({ | |
| success: true, | |
| bedrockConfigured: isConfigured, | |
| bedrockEnabled: config?.enabled || false, | |
| region: isConfigured ? config?.region : null, | |
| }); | |
| const credentials = await settingsService.getCredentials(); | |
| const config = credentials.awsBedrock; | |
| const isConfigured = !!( | |
| config?.accessKeyId && | |
| config?.secretAccessKey && | |
| config?.region | |
| ); | |
| res.json({ | |
| success: true, | |
| bedrockConfigured: isConfigured, | |
| bedrockEnabled: config?.enabled || false, | |
| region: isConfigured ? config?.region : null, | |
| }); |
🤖 Prompt for AI Agents
In `@apps/server/src/routes/settings/routes/get-bedrock-status.ts` around lines 17
- 27, The current logic sets bedrockConfigured based on config?.enabled which
hides "configured but disabled" state; change the check so bedrockConfigured is
true when AWS Bedrock credentials exist regardless of enabled (e.g., derive
isConfigured from presence of config.accessKeyId and config.region or necessary
secrets obtained via settingsService.getCredentials), keep bedrockEnabled using
config?.enabled, and ensure region returns the region only if credentials exist
(use the same credentials-presence check instead of config?.enabled).
| awsBedrock: { | ||
| configured: !!(credentials.awsBedrock?.accessKeyId && credentials.awsBedrock?.region), | ||
| enabled: credentials.awsBedrock?.enabled || false, | ||
| region: credentials.awsBedrock?.region || '', | ||
| accessKeyMasked: maskKey(credentials.awsBedrock?.accessKeyId || ''), | ||
| secretKeyMasked: maskKey(credentials.awsBedrock?.secretAccessKey || ''), | ||
| }, |
There was a problem hiding this comment.
Configured status should include the secret key.
configured ignores secretAccessKey, so the UI can show Bedrock as configured even when auth will fail. Consider requiring the secret (unless role-based auth is intended).
🐛 Suggested fix
- configured: !!(credentials.awsBedrock?.accessKeyId && credentials.awsBedrock?.region),
+ configured: !!(
+ credentials.awsBedrock?.accessKeyId &&
+ credentials.awsBedrock?.secretAccessKey &&
+ credentials.awsBedrock?.region
+ ),🤖 Prompt for AI Agents
In `@apps/server/src/services/settings-service.ts` around lines 479 - 485, The
awsBedrock configured flag in apps/server/src/services/settings-service.ts only
checks accessKeyId and region, so it can be true when secretAccessKey is
missing; update the configured expression for the awsBedrock object to also
require credentials.awsBedrock?.secretAccessKey (e.g., include it in the boolean
check alongside accessKeyId and region) so the UI reflects true authentication
readiness; ensure you still use maskKey for accessKeyMasked/secretKeyMasked and
maintain existing enabled/region fallbacks.
| { | ||
| id: 'anthropic.claude-3-5-sonnet-20241022-v2:0', | ||
| label: '3.5 Sonnet (Bedrock)', | ||
| description: 'Via AWS Bedrock', | ||
| badge: 'AWS', | ||
| provider: 'claude', | ||
| hasThinking: true, | ||
| }, |
There was a problem hiding this comment.
Incorrect hasThinking value for Claude 3.5 Sonnet.
Claude 3.5 Sonnet (claude-3-5-sonnet-20241022) does not support extended thinking—only Claude 4+ models have this capability. Setting hasThinking: true will mislead the UI into offering thinking controls that won't work.
🐛 Proposed fix
{
id: 'anthropic.claude-3-5-sonnet-20241022-v2:0',
label: '3.5 Sonnet (Bedrock)',
description: 'Via AWS Bedrock',
badge: 'AWS',
provider: 'claude',
- hasThinking: true,
+ hasThinking: false,
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| id: 'anthropic.claude-3-5-sonnet-20241022-v2:0', | |
| label: '3.5 Sonnet (Bedrock)', | |
| description: 'Via AWS Bedrock', | |
| badge: 'AWS', | |
| provider: 'claude', | |
| hasThinking: true, | |
| }, | |
| { | |
| id: 'anthropic.claude-3-5-sonnet-20241022-v2:0', | |
| label: '3.5 Sonnet (Bedrock)', | |
| description: 'Via AWS Bedrock', | |
| badge: 'AWS', | |
| provider: 'claude', | |
| hasThinking: false, | |
| }, |
🤖 Prompt for AI Agents
In `@apps/ui/src/components/views/board-view/shared/model-constants.ts` around
lines 65 - 72, The entry with id 'anthropic.claude-3-5-sonnet-20241022-v2:0'
incorrectly sets hasThinking to true; update that model object so hasThinking:
false (remove any UI logic that relies on this flag for
claude-3-5-sonnet-20241022 variants) so the UI won't show thinking controls for
Claude 3.5 Sonnet.
| const handleSave = async () => { | ||
| // Validation | ||
| if (enabled) { | ||
| if (!accessKeyId || !secretAccessKey) { | ||
| toast.error('Access Key ID and Secret Access Key are required when enabled'); | ||
| return; | ||
| } | ||
| if (!region) { | ||
| toast.error('AWS Region is required when enabled'); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| setIsSaving(true); | ||
| try { | ||
| const api = getElectronAPI(); | ||
| if (!api?.settings?.updateCredentials) { | ||
| toast.error('Settings API not available'); | ||
| return; | ||
| } | ||
|
|
||
| const result = await api.settings.updateCredentials({ | ||
| awsBedrock: { | ||
| enabled, | ||
| accessKeyId: accessKeyId || undefined, | ||
| secretAccessKey: secretAccessKey || undefined, | ||
| sessionToken: sessionToken || undefined, | ||
| region, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Avoid wiping stored credentials when saving disabled/blank fields.
The payload always includes empty key fields, so toggling off or saving without retyping clears stored credentials—preventing a “configured but disabled” state. Only send key fields when the user provides new values (or add an explicit clear action).
🐛 Suggested fix
- const result = await api.settings.updateCredentials({
- awsBedrock: {
- enabled,
- accessKeyId: accessKeyId || undefined,
- secretAccessKey: secretAccessKey || undefined,
- sessionToken: sessionToken || undefined,
- region,
- },
- });
+ const awsBedrockUpdate: Record<string, unknown> = { enabled, region };
+ if (accessKeyId) awsBedrockUpdate.accessKeyId = accessKeyId;
+ if (secretAccessKey) awsBedrockUpdate.secretAccessKey = secretAccessKey;
+ if (sessionToken) awsBedrockUpdate.sessionToken = sessionToken;
+
+ const result = await api.settings.updateCredentials({
+ awsBedrock: awsBedrockUpdate,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleSave = async () => { | |
| // Validation | |
| if (enabled) { | |
| if (!accessKeyId || !secretAccessKey) { | |
| toast.error('Access Key ID and Secret Access Key are required when enabled'); | |
| return; | |
| } | |
| if (!region) { | |
| toast.error('AWS Region is required when enabled'); | |
| return; | |
| } | |
| } | |
| setIsSaving(true); | |
| try { | |
| const api = getElectronAPI(); | |
| if (!api?.settings?.updateCredentials) { | |
| toast.error('Settings API not available'); | |
| return; | |
| } | |
| const result = await api.settings.updateCredentials({ | |
| awsBedrock: { | |
| enabled, | |
| accessKeyId: accessKeyId || undefined, | |
| secretAccessKey: secretAccessKey || undefined, | |
| sessionToken: sessionToken || undefined, | |
| region, | |
| }, | |
| }); | |
| const handleSave = async () => { | |
| // Validation | |
| if (enabled) { | |
| if (!accessKeyId || !secretAccessKey) { | |
| toast.error('Access Key ID and Secret Access Key are required when enabled'); | |
| return; | |
| } | |
| if (!region) { | |
| toast.error('AWS Region is required when enabled'); | |
| return; | |
| } | |
| } | |
| setIsSaving(true); | |
| try { | |
| const api = getElectronAPI(); | |
| if (!api?.settings?.updateCredentials) { | |
| toast.error('Settings API not available'); | |
| return; | |
| } | |
| const awsBedrockUpdate: Record<string, unknown> = { enabled, region }; | |
| if (accessKeyId) awsBedrockUpdate.accessKeyId = accessKeyId; | |
| if (secretAccessKey) awsBedrockUpdate.secretAccessKey = secretAccessKey; | |
| if (sessionToken) awsBedrockUpdate.sessionToken = sessionToken; | |
| const result = await api.settings.updateCredentials({ | |
| awsBedrock: awsBedrockUpdate, | |
| }); |
🤖 Prompt for AI Agents
In
`@apps/ui/src/components/views/settings-view/providers/claude-settings-tab/aws-bedrock-section.tsx`
around lines 118 - 147, The handleSave flow currently always sends empty strings
for accessKeyId/secretAccessKey/sessionToken which wipes stored creds; change
the payload construction before calling api.settings.updateCredentials so
awsBedrock always includes enabled and region but only includes accessKeyId,
secretAccessKey and sessionToken properties when the corresponding local values
are non-empty (i.e., conditionally add them to the object), and do not send
empty strings or explicit clears unless the user explicitly chooses a "clear"
action; update the call site in handleSave where api.settings.updateCredentials
is invoked to use this conditional payload.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/src/providers/simple-query-service.ts (1)
43-44: Update JSDoc to reflect actual behavior.The comment states "(default: [] for simple queries)" but with the new conditional spread at lines 136-137,
allowedToolsis now omitted entirely when not provided rather than defaulting to an empty array. This distinction may matter to downstream providers.📝 Suggested fix
- /** Tools to allow (default: [] for simple queries) */ + /** Tools to allow (omitted if not provided, allowing provider defaults) */ allowedTools?: string[];
🤖 Fix all issues with AI agents
In `@apps/server/src/providers/simple-query-service.ts`:
- Around line 222-223: The streamingQuery invocation in validate-issue.ts is
missing the allowedTools option, causing provider defaults to be used; update
the call to streamingQuery (in validateIssue / the function around line ~194) to
include allowedTools: ['Read','Glob','Grep'] in the options object so it matches
other callers (generate-suggestions.ts, sync-spec.ts, generate-spec.ts,
generate-features-from-spec.ts) and ensures the provider receives the expected
tools list.
♻️ Duplicate comments (1)
apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
70-75: SethasThinkingto false for Claude 3.5 Sonnet (Bedrock).Leaving this as
truewill expose thinking controls that aren’t supported for this model.🐛 Proposed fix
{ id: 'anthropic.claude-3-5-sonnet-20241022-v2:0', label: '3.5 Sonnet (Bedrock)', description: 'Via AWS Bedrock', badge: 'AWS', provider: 'claude', - hasThinking: true, + hasThinking: false, },Does AWS Bedrock Claude 3.5 Sonnet support extended thinking / reasoning controls?
| // Only include allowedTools if explicitly provided | ||
| ...(options.allowedTools !== undefined && { allowedTools: options.allowedTools }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the current implementation of streamingQuery around lines 222-223
cat -n apps/server/src/providers/simple-query-service.ts | sed -n '210,235p'Repository: AutoMaker-Org/automaker
Length of output: 1387
🏁 Script executed:
# Find all callers of streamingQuery in the codebase
rg -n 'streamingQuery\s*\(' --type ts -A 3 | head -100Repository: AutoMaker-Org/automaker
Length of output: 2908
🏁 Script executed:
# Check if there's a simpleQuery function in the same file to compare patterns
rg -n 'simpleQuery\s*\(' apps/server/src/providers/simple-query-service.ts -A 5Repository: AutoMaker-Org/automaker
Length of output: 608
🏁 Script executed:
# Find the provider interface/defaults to understand what happens when allowedTools is omitted
rg -n 'allowedTools' apps/server/src/providers/simple-query-service.ts -B 3 -A 3Repository: AutoMaker-Org/automaker
Length of output: 1754
🏁 Script executed:
# Get full context for each caller of streamingQuery to see if allowedTools is passed
rg -n 'streamingQuery\s*\(' apps/server/src/routes/suggestions/generate-suggestions.ts -A 15 | head -30Repository: AutoMaker-Org/automaker
Length of output: 768
🏁 Script executed:
# Check all callers more comprehensively with explicit allowedTools presence
for file in $(rg -l 'streamingQuery\s*\(' --type ts); do
echo "=== File: $file ==="
rg -A 20 'streamingQuery\s*\(' "$file" | grep -A 20 'streamingQuery'
doneRepository: AutoMaker-Org/automaker
Length of output: 6381
🏁 Script executed:
# Check the provider interface to see what defaults allowedTools to when undefined
rg -n 'executeQuery' apps/server/src/providers/ --type ts -B 5 -A 10 | grep -A 15 'executeQuery'Repository: AutoMaker-Org/automaker
Length of output: 17149
🏁 Script executed:
# Look for the actual provider implementation to understand defaults
fd -e ts apps/server/src/providers/ | xargs grep -l 'allowedTools' | head -5Repository: AutoMaker-Org/automaker
Length of output: 476
🏁 Script executed:
# Get the full streamingQuery call in validate-issue.ts with more context
rg -A 25 'const result = await streamingQuery' apps/server/src/routes/github/routes/validate-issue.tsRepository: AutoMaker-Org/automaker
Length of output: 1157
🏁 Script executed:
# Check if there are any other callers of streamingQuery we might have missed
rg -l 'streamingQuery\(' --type tsRepository: AutoMaker-Org/automaker
Length of output: 387
Fix breaking change in validate-issue.ts where streamingQuery call doesn't pass allowedTools.
validate-issue.ts doesn't provide allowedTools when calling streamingQuery. With the current implementation, this omits allowedTools entirely from the provider options, causing the call to use provider defaults instead of the expected ['Read', 'Glob', 'Grep']. Add allowedTools: ['Read', 'Glob', 'Grep'] to the streamingQuery call in validate-issue.ts (line ~194), consistent with all other callers (generate-suggestions.ts, sync-spec.ts, generate-spec.ts, generate-features-from-spec.ts).
🤖 Prompt for AI Agents
In `@apps/server/src/providers/simple-query-service.ts` around lines 222 - 223,
The streamingQuery invocation in validate-issue.ts is missing the allowedTools
option, causing provider defaults to be used; update the call to streamingQuery
(in validateIssue / the function around line ~194) to include allowedTools:
['Read','Glob','Grep'] in the options object so it matches other callers
(generate-suggestions.ts, sync-spec.ts, generate-spec.ts,
generate-features-from-spec.ts) and ensures the provider receives the expected
tools list.
1 participant
I showed Automaker to my Dad and he really liked it. However, he can only use the AWS Bedrock Claude Models because of his job, so I added the ability to enable Bedrock in the Anthropic Provider settings and pass in credentials there.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.