-
Notifications
You must be signed in to change notification settings - Fork 67
Add support for AWS Bedrock in MI Copilot #996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds AWS Bedrock authentication: types, state-machine flows, credential validation service, Bedrock connection and cache handling, UI for entering AWS credentials, and a Bedrock SDK dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Login UI
participant SM as AI State Machine
participant Auth as Auth Service
participant Bedrock as AWS Bedrock
User->>UI: Click "Enter AWS Bedrock credentials"
UI->>SM: AUTH_WITH_AWS_BEDROCK
SM->>SM: enter awsBedrockFlow
User->>UI: Submit credentials
UI->>SM: SUBMIT_AWS_CREDENTIALS(payload)
SM->>SM: enter validatingAwsCredentials
SM->>Auth: validateAwsCredentials(payload)
Auth->>Bedrock: test generateText call
alt valid
Bedrock-->>Auth: success
Auth-->>SM: token / store secrets
SM->>UI: authenticated state
else invalid
Bedrock-->>Auth: error
Auth-->>SM: throw error
SM->>UI: return to awsBedrockFlow with error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
workspaces/mi/mi-extension/src/ai-panel/copilot/connection.ts (1)
19-20: Bedrock integration looks coherent; verify region prefixes, cache shape, and consider minor refactors
getBedrockRegionalPrefixcoversus,us-gov,eu,ap, and mapsca/satous, butvalidRegionsinauth.tsalso include prefixes likeme-andaf-. It would be safer to explicitly handle those or at least confirm that defaulting them to'us'matches the Bedrock inference profile naming rules.- For the fallback branch you use
console.warn; elsewhere this module relies onlogInfo/logError. Consider reusing the shared logger for consistency.- In the AWS_BEDROCK branch of
getAnthropicClientyou recreate the Bedrock client on each call and return early instead of caching like the Anthropic client. That’s functionally fine but may be slightly inefficient; caching the factory (or at least updatingcachedAuthMethod) would bring symmetry and avoid repeated setup.getProviderCacheControlnow returns{ bedrock: { cachePoint: { type: 'default' } } }for AWS_BEDROCK and{ anthropic: { cacheControl: { type: 'ephemeral' } } }otherwise; please double‑check these shapes and keys against the latest@ai-sdk/*provider cache control docs to ensure they’re interpreted correctly.Also applies to: 32-54, 177-205, 223-232
workspaces/mi/mi-extension/src/ai-panel/aiMachine.ts (1)
25-26: AWS Bedrock auth flow mirrors API key flow; consider handlingCANCELfor parityThe new Bedrock path (
AUTH_WITH_AWS_BEDROCK→awsBedrockFlow→validatingAwsCredentialswithvalidateAwsCredentialsService) is wired consistently with the existing API key flow and uses the same error‑clearing patterns.One small consistency gap:
awsBedrockFlowhandlesCANCEL_LOGINbut notCANCEL, whereas bothssoFlowandapiKeyFlowhandle both events. IfAI_EVENT_TYPE.CANCELcan be emitted while in the Bedrock flow, adding a matching transition to#mi-ai.Unauthenticated(clearingloginMethodanderrorMessage) would keep behavior uniform.Also applies to: 140-146, 162-165, 253-288, 452-464, 480-481
workspaces/mi/mi-core/src/state-machine-types.ts (1)
193-198: AwsBedrockSecrets and AuthCredentials union are well-shaped; consider a small doc/type polish.The
AwsBedrockSecretsstructure and its inclusion inAuthCredentialsare consistent with how Bedrock credentials are validated and stored inauth.ts. Two small follow‑ups you might consider:
- Export
AwsBedrockSecrets(if you expect other modules to type against it) instead of re-declaring the shape elsewhere.- Update the
AIUserTokencomment to mention whattokenrepresents forAWS_BEDROCK(currently it only mentions MI Intel and Anthropic).Also applies to: 204-216, 218-220
workspaces/mi/mi-extension/src/ai-panel/auth.ts (2)
110-144: AWS_BEDROCK branch in getAccessToken is functionally fine but slightly confusing semantically.Returning
accessKeyIdforLoginMethod.AWS_BEDROCKkeepsgetAccessTokenusable as a generic “do we have credentials?” probe, but the namegetAccessTokenis now a bit misleading (it’s not a bearer token for Bedrock). This is acceptable, just be aware at call sites not to treat this as an HTTP Authorization token for AWS requests.
415-485: AWS credential validation flow is solid; consider relaxing heuristics and improving logging/UX.The overall approach—lightweight Bedrock call via
generateText, then persisting secrets in VS Code’s secret storage—is reasonable. A few refinements would make it more robust:
- The access key check
startsWith('AKIA') || startsWith('ASIA')is brittle; AWS uses additional valid prefixes now. A non‑empty check (and letting the Bedrock call be the source of truth) is safer.- Maintaining a hard‑coded
validRegionslist risks divergence from actual Bedrock support and from any other region list you use elsewhere. Consider centralizing this list or deferring to the provider/AWS errors for unsupported regions.- Use the existing
logErrorhelper instead ofconsole.errorso Bedrock failures end up in the same log channel and you can better control what (if anything) is emitted about the error object.- Optionally, mirror
validateApiKeyby settingmaxRetries: 0and mapping common Bedrock error codes to friendlier user messages (invalid credentials vs. region not enabled vs. network issues), to avoid generic “Validation failed” UX.None of these are blockers, but addressing them would reduce future breakage and improve the user experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
workspaces/mi/mi-core/src/state-machine-types.ts(5 hunks)workspaces/mi/mi-extension/package.json(1 hunks)workspaces/mi/mi-extension/src/ai-panel/aiMachine.ts(6 hunks)workspaces/mi/mi-extension/src/ai-panel/auth.ts(3 hunks)workspaces/mi/mi-extension/src/ai-panel/copilot/connection.ts(4 hunks)workspaces/mi/mi-extension/src/ai-panel/copilot/suggestions/suggestions.ts(1 hunks)workspaces/mi/mi-visualizer/src/views/AIPanel/component/WaitingForLoginSection.tsx(3 hunks)workspaces/mi/mi-visualizer/src/views/AIPanel/index.tsx(1 hunks)workspaces/mi/mi-visualizer/src/views/LoggedOutWindow/index.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
workspaces/mi/mi-visualizer/src/views/AIPanel/component/WaitingForLoginSection.tsx (1)
workspaces/common-libs/ui-toolkit/src/components/Codicon/Codicon.tsx (1)
Codicon(44-56)
workspaces/mi/mi-extension/src/ai-panel/aiMachine.ts (2)
workspaces/mi/mi-core/src/state-machine-types.ts (1)
AIMachineContext(222-227)workspaces/mi/mi-extension/src/ai-panel/auth.ts (1)
validateAwsCredentials(415-485)
workspaces/mi/mi-extension/src/ai-panel/copilot/connection.ts (1)
workspaces/mi/mi-extension/src/ai-panel/auth.ts (2)
getAwsBedrockCredentials(149-160)getLoginMethod(101-104)
workspaces/mi/mi-extension/src/ai-panel/auth.ts (2)
workspaces/mi/mi-core/src/state-machine-types.ts (2)
AIUserToken(218-220)AuthCredentials(204-216)workspaces/mi/mi-extension/src/ai-panel/copilot/connection.ts (1)
getBedrockRegionalPrefix(37-54)
🔇 Additional comments (8)
workspaces/mi/mi-visualizer/src/views/LoggedOutWindow/index.tsx (1)
143-145: AWS Bedrock login entrypoint wiring looks correctThe new handler and button cleanly mirror the existing Anthropic API key flow and correctly dispatch
AUTH_WITH_AWS_BEDROCKviarpcClient.Also applies to: 175-175
workspaces/mi/mi-visualizer/src/views/AIPanel/index.tsx (1)
59-64: Authenticating substate routing updated correctly for AWS BedrockIncluding
validatingAwsCredentialsinisValidatingand routing bothawsBedrockFlowandvalidatingAwsCredentialstoWaitingForLoginSectionkeeps the Bedrock flow consistent with the API key flow.workspaces/mi/mi-visualizer/src/views/AIPanel/component/WaitingForLoginSection.tsx (1)
197-205: AWS Bedrock credential form wiring and validation gating look solidState management for
awsCredentials, the visibility toggles, andconnectWithAwsCredentials(including trimming and disabling the primary button when the form is incomplete or validating) are consistent and align with the state machine’sSUBMIT_AWS_CREDENTIALSflow.Also applies to: 211-223, 264-269, 275-285, 348-460
workspaces/mi/mi-core/src/state-machine-types.ts (3)
112-119: AWS Bedrock substates are wired consistently into the AI machine state.Adding
'awsBedrockFlow'and'validatingAwsCredentials'underAuthenticatingfits the existing hierarchical pattern and keeps the state space explicit and type-safe. No issues here.
125-146: New AWS Bedrock AI events and payload typing look correct.
AUTH_WITH_AWS_BEDROCKandSUBMIT_AWS_CREDENTIALSare added to bothAI_EVENT_TYPEandAIMachineEventMapwith a concrete payload shape, which will give good type safety for the new flow. Just ensure all senders use thepayloadobject with these exact field names.Also applies to: 148-174
182-186: LoginMethod enum extension aligns with the auth flows.Introducing
ANTHROPIC_KEYandAWS_BEDROCKhere matches the usages inauth.ts(validation and token retrieval) and keeps the login methods centralized. No further changes needed in this enum.workspaces/mi/mi-extension/src/ai-panel/auth.ts (2)
30-41: New Bedrock imports and regional helper usage are consistent.
createAmazonBedrockandgetBedrockRegionalPrefixare used only in the Bedrock validation path and keep the provider‑specific logic localized. No structural issues with these additions.
146-161: getAwsBedrockCredentials helper matches stored secret shape and keeps consumers clean.The helper correctly narrows on
LoginMethod.AWS_BEDROCKand returns the exact credential fields needed by callers, which keeps Bedrock-specific logic out of generic auth code. Looks good.
| "xstate": "^4.38.3", | ||
| "zod": "^3.24.1" | ||
| "zod": "^3.24.1", | ||
| "@ai-sdk/amazon-bedrock": "~3.0.56" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify version pinning consistency and package availability.
The new @ai-sdk/amazon-bedrock dependency uses a tilde (~3.0.56) for version pinning, while the existing @ai-sdk/anthropic dependency uses a caret (^2.0.35). This inconsistency may be intentional, but should be clarified. Additionally, verify that the specified version exists on npm and contains no known vulnerabilities.
🏁 Script executed:
#!/bin/bash
# Description: Verify the AWS Bedrock SDK version exists and check for security advisories
# Check if the package version exists on npm
npm view @ai-sdk/[email protected] version 2>/dev/null || echo "Version 3.0.56 not found"
# Get latest version for comparison
echo "Latest version available:"
npm view @ai-sdk/amazon-bedrock@latest version 2>/dev/null
# Check for known security advisories (if npm audit is available)
echo "Checking for security issues..."
npm audit --package-lock-only @ai-sdk/[email protected] 2>/dev/null || echo "Audit not available in this context"Length of output: 95
🌐 Web query:
@ai-sdk/amazon-bedrock compatibility peerDependencies ai-sdk version requirements
💡 Result:
Short answer: @ai-sdk/amazon-bedrock does not list the core "ai" package as a peerDependency — instead it depends on the provider framework packages (e.g. @ai-sdk/provider, @ai-sdk/provider-utils) and typically requires the matching family of AI SDK packages for the same major release. Check the exact package.json for the version you plan to install, but common facts:
- The Amazon Bedrock provider package is published separately (e.g. versions 2.x / 3.x / 3.0.16 / 3.0.33 shown on npm) and is installed alongside the core "ai" package. [1][2]
- The provider packages declare dependencies on @ai-sdk/provider and @ai-sdk/provider-utils (not a direct peerDependency on "ai"); some builds only declare an optional peerDependency on zod. See the provider docs and package metadata. [1][3]
- Recommendation: match provider + core major versions (use the same AI SDK major series — e.g. v4 vs v5) and inspect the specific @ai-sdk/amazon-bedrock package.json for the peerDependencies for the exact published version you’ll install. You can view metadata on the package page or the docs. [1][2][3]
Sources:
[1] AI SDK — Amazon Bedrock provider docs.
[2] @ai-sdk/amazon-bedrock npm listing (package + version info).
[3] package metadata / registry snapshots showing dependencies (e.g. provider / provider-utils and zod peerDependency).
🏁 Script executed:
#!/bin/bash
# Fetch detailed package metadata for @ai-sdk/[email protected]
npm view @ai-sdk/[email protected] --json | jq '.peerDependencies, .dependencies' 2>/dev/null || echo "Failed to fetch metadata"Length of output: 238
🌐 Web query:
ai-sdk major version 5 amazon bedrock provider 3 compatibility
💡 Result:
Short answer: Use the provider version set required by AI SDK v5 — @ai-sdk/amazon-bedrock v2.0.x. AI SDK v5 expects @ai-sdk/* packages at 2.x (and [email protected]), so an @ai-sdk/amazon-bedrock v3 release is not the documented target for AI SDK v5 and may be incompatible. [1][2]
References
- AI SDK 4→5 migration (lists required package versions for v5, including @ai-sdk/* = 2.0.0). [1]
- Amazon Bedrock provider docs for the v5-era SDK (shows @ai-sdk/amazon-bedrock usage/migration guidance). [2]
Update @ai-sdk/amazon-bedrock to version 2.x for AI SDK v5 compatibility.
The specified version @ai-sdk/[email protected] is incompatible with the core ai@^5.0.76 dependency. AI SDK v5 expects @ai-sdk/* packages at 2.x. Change the dependency to @ai-sdk/amazon-bedrock@^2.0.x to match the project's AI SDK version series.
🤖 Prompt for AI Agents
In workspaces/mi/mi-extension/package.json around line 1064, the dependency
"@ai-sdk/amazon-bedrock": "~3.0.56" is incompatible with the project's AI SDK v5
which requires @ai-sdk packages in the 2.x series; update this dependency to use
the 2.x range (for example "@ai-sdk/amazon-bedrock": "^2.0.x") so it aligns with
ai@^5.0.76 and reinstall/update lockfile to ensure consistent dependency
resolution.
| role: "system" as const, | ||
| content: systemPrompt, | ||
| providerOptions: cacheOptions, // Cache system prompt only | ||
| providerOptions: cacheOptions as any, // Cache system prompt only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Avoid as any — properly type the provider options.
Using as any bypasses TypeScript's type safety and can hide type errors. With the AWS Bedrock integration, the proper approach is to extend the provider options type to accommodate both Anthropic and AWS Bedrock options, rather than casting to any.
Please verify the return type of getProviderCacheControl() and ensure the providerOptions field is properly typed to accept the returned value:
🏁 Script executed:
#!/bin/bash
# Description: Check the return type of getProviderCacheControl and how providerOptions is typed in the AI SDK
# Find the definition of getProviderCacheControl
ast-grep --pattern 'getProviderCacheControl($_) {
$$$
}'
# Also search for its type signature
rg -n -A 5 'getProviderCacheControl'Length of output: 16904
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full getProviderCacheControl function with all return cases
rg -n -A 20 'export const getProviderCacheControl = async' workspaces/mi/mi-extension/src/ai-panel/copilot/connection.ts
# Also check the full context around line 76 in suggestions.ts
rg -n -B 5 -A 5 'providerOptions: cacheOptions as any' workspaces/mi/mi-extension/src/ai-panel/copilot/suggestions/suggestions.ts
Length of output: 216
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full getProviderCacheControl function with all return cases
rg -n -A 20 'export const getProviderCacheControl = async' workspaces/mi/mi-extension/src/ai-panel/copilot/connection.ts
# Also check the full context around line 76 in suggestions.ts (the as any cast)
rg -n -B 10 -A 5 'providerOptions: cacheOptions as any' workspaces/mi/mi-extension/src/ai-panel/copilot/suggestions/suggestions.ts
# Also search for the type definition of ProviderCacheOptions if it exists
rg -n 'type ProviderCacheOptions|interface ProviderCacheOptions' workspaces/mi/mi-extension/src/ai-panel/copilot/
Length of output: 939
---
🏁 Script executed:
```shell
#!/bin/bash
# Check how other files use cacheOptions - see if they also use as any
rg -n 'providerOptions.*cacheOptions' workspaces/mi/mi-extension/src/ai-panel/copilot/
# Check imports to understand what types are being used
rg -n 'import.*streamText|import.*ModelMessage' workspaces/mi/mi-extension/src/ai-panel/copilot/suggestions/suggestions.ts
# Check the broader pattern - are there type definitions for provider options?
rg -n 'providerOptions' workspaces/mi/mi-extension/src/ai-panel/copilot/suggestions/suggestions.ts
Length of output: 921
Remove the unnecessary as any cast — it's inconsistent with the rest of the codebase.
The same cacheOptions from getProviderCacheControl() is used without as any in five other files: unit_test_case_generate.ts, generation/generations.ts, diagnostics/diagnostics.ts, and connectors/connectors.ts. Remove the cast on line 76 to maintain consistency and preserve type safety:
providerOptions: cacheOptions, // Cache system prompt only
🤖 Prompt for AI Agents
In workspaces/mi/mi-extension/src/ai-panel/copilot/suggestions/suggestions.ts
around line 76, remove the unnecessary `as any` type cast on `providerOptions:
cacheOptions as any` and change it to `providerOptions: cacheOptions` to match
other usages and preserve type safety; update only that line so it uses the
existing `cacheOptions` value without casting.
workspaces/mi/mi-visualizer/src/views/AIPanel/component/WaitingForLoginSection.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
workspaces/mi/mi-visualizer/src/views/AIPanel/component/WaitingForLoginSection.tsx (1)
211-223: Consider adding format validation for AWS credentials.The validation only checks for non-empty fields, unlike the Anthropic flow which validates key format and length. Consider adding client-side checks for AWS credential formats (e.g., access keys typically start with
AKIAorASIA, regions follow patterns likeus-east-1) to provide immediate feedback before backend validation.Example validation you could add:
const connectWithAwsCredentials = () => { + const trimmedAccessKey = awsCredentials.accessKeyId.trim(); + const trimmedSecretKey = awsCredentials.secretAccessKey.trim(); + const trimmedRegion = awsCredentials.region.trim(); + + if (!trimmedAccessKey || !trimmedSecretKey || !trimmedRegion) { + return; + } + + // Basic AWS access key format validation + if (!/^(AKIA|ASIA)[A-Z0-9]{16}$/.test(trimmedAccessKey)) { + // Set client error state similar to Anthropic flow + return; + } + + // Basic AWS region format validation + if (!/^[a-z]{2}-[a-z]+-\d+$/.test(trimmedRegion)) { + // Set client error state for invalid region format + return; + } + - if (awsCredentials.accessKeyId.trim() && awsCredentials.secretAccessKey.trim() && awsCredentials.region.trim()) { rpcClient.sendAIStateEvent({ type: AI_EVENT_TYPE.SUBMIT_AWS_CREDENTIALS, payload: { - accessKeyId: awsCredentials.accessKeyId.trim(), - secretAccessKey: awsCredentials.secretAccessKey.trim(), - region: awsCredentials.region.trim(), + accessKeyId: trimmedAccessKey, + secretAccessKey: trimmedSecretKey, + region: trimmedRegion, sessionToken: awsCredentials.sessionToken.trim() || undefined }, }); - } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/mi/mi-visualizer/src/views/AIPanel/component/WaitingForLoginSection.tsx(3 hunks)
🔇 Additional comments (2)
workspaces/mi/mi-visualizer/src/views/AIPanel/component/WaitingForLoginSection.tsx (2)
197-205: LGTM!The AWS credentials state is well-structured, follows the existing pattern for sensitive field visibility, and correctly separates concerns.
264-285: LGTM!The credential change handler correctly uses
keyoffor type safety and functional state updates. The visibility toggle handlers follow the established pattern and work correctly.
| if (loginMethod === LoginMethod.AWS_BEDROCK) { | ||
| const isFormValid = awsCredentials.accessKeyId.trim() && | ||
| awsCredentials.secretAccessKey.trim() && | ||
| awsCredentials.region.trim(); | ||
|
|
||
| return ( | ||
| <Container> | ||
| <AlertContainer variant="primary"> | ||
| <Title>Connect with AWS Bedrock</Title> | ||
| <SubTitle> | ||
| Enter your AWS credentials to connect to MI Copilot via AWS Bedrock. Your credentials will be securely stored | ||
| and used for authentication. | ||
| </SubTitle> | ||
|
|
||
| <InputContainer> | ||
| <InputRow> | ||
| <StyledTextField | ||
| type={showAccessKey ? "text" : "password"} | ||
| placeholder="AWS Access Key ID" | ||
| value={awsCredentials.accessKeyId} | ||
| onInput={handleAwsCredentialChange('accessKeyId')} | ||
| {...(isValidating ? { disabled: true } : {})} | ||
| /> | ||
| <EyeButton | ||
| type="button" | ||
| onClick={toggleAccessKeyVisibility} | ||
| title={showAccessKey ? "Hide access key" : "Show access key"} | ||
| {...(isValidating ? { disabled: true } : {})} | ||
| > | ||
| <Codicon name={showAccessKey ? "eye-closed" : "eye"} /> | ||
| </EyeButton> | ||
| </InputRow> | ||
| </InputContainer> | ||
|
|
||
| <InputContainer> | ||
| <InputRow> | ||
| <StyledTextField | ||
| type={showSecretKey ? "text" : "password"} | ||
| placeholder="AWS Secret Access Key" | ||
| value={awsCredentials.secretAccessKey} | ||
| onInput={handleAwsCredentialChange('secretAccessKey')} | ||
| {...(isValidating ? { disabled: true } : {})} | ||
| /> | ||
| <EyeButton | ||
| type="button" | ||
| onClick={toggleSecretKeyVisibility} | ||
| title={showSecretKey ? "Hide secret key" : "Show secret key"} | ||
| {...(isValidating ? { disabled: true } : {})} | ||
| > | ||
| <Codicon name={showSecretKey ? "eye-closed" : "eye"} /> | ||
| </EyeButton> | ||
| </InputRow> | ||
| </InputContainer> | ||
|
|
||
| <InputContainer> | ||
| <InputRow> | ||
| <StyledTextField | ||
| type="text" | ||
| placeholder="AWS Region (e.g., us-east-1)" | ||
| value={awsCredentials.region} | ||
| onInput={handleAwsCredentialChange('region')} | ||
| {...(isValidating ? { disabled: true } : {})} | ||
| /> | ||
| </InputRow> | ||
| </InputContainer> | ||
|
|
||
| <InputContainer> | ||
| <InputRow> | ||
| <StyledTextField | ||
| type={showSessionToken ? "text" : "password"} | ||
| placeholder="Session Token (optional)" | ||
| value={awsCredentials.sessionToken} | ||
| onInput={handleAwsCredentialChange('sessionToken')} | ||
| {...(isValidating ? { disabled: true } : {})} | ||
| /> | ||
| <EyeButton | ||
| type="button" | ||
| onClick={toggleSessionTokenVisibility} | ||
| title={showSessionToken ? "Hide session token" : "Show session token"} | ||
| {...(isValidating ? { disabled: true } : {})} | ||
| > | ||
| <Codicon name={showSessionToken ? "eye-closed" : "eye"} /> | ||
| </EyeButton> | ||
| </InputRow> | ||
| </InputContainer> | ||
|
|
||
| {errorMessage && ( | ||
| <ErrorMessage> | ||
| <Codicon name="error" /> | ||
| <span>{errorMessage}</span> | ||
| </ErrorMessage> | ||
| )} | ||
|
|
||
| <ButtonContainer> | ||
| <VSCodeButton | ||
| appearance="primary" | ||
| onClick={connectWithAwsCredentials} | ||
| {...(isValidating || !isFormValid ? { disabled: true } : {})} | ||
| > | ||
| {isValidating ? "Validating..." : "Connect with AWS Bedrock"} | ||
| </VSCodeButton> | ||
| <VSCodeButton | ||
| appearance="secondary" | ||
| onClick={cancelLogin} | ||
| {...(isValidating ? { disabled: true } : {})} | ||
| > | ||
| Cancel | ||
| </VSCodeButton> | ||
| </ButtonContainer> | ||
| </AlertContainer> | ||
| </Container> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor UX gap: No feedback when form is incomplete.
The Connect button is disabled when required fields are empty (line 445), but unlike the Anthropic flow which shows validation errors (lines 320-325), users receive no explanation for why the button is disabled. Consider adding helper text or adopting the clientError pattern from the Anthropic flow to improve discoverability.
You could add a helper message below the subtitle or show per-field validation:
<SubTitle>
Enter your AWS credentials to connect to MI Copilot via AWS Bedrock. Your credentials will be securely stored
and used for authentication.
</SubTitle>
+
+ {!isFormValid && !isValidating && (
+ <SubTitle style={{ color: 'var(--vscode-descriptionForeground)', fontStyle: 'italic' }}>
+ Please fill in all required fields (Access Key ID, Secret Access Key, and Region).
+ </SubTitle>
+ )}Or adopt the Anthropic pattern by adding client-side validation that sets error messages when the user tries to submit an incomplete form.
📝 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.
| if (loginMethod === LoginMethod.AWS_BEDROCK) { | |
| const isFormValid = awsCredentials.accessKeyId.trim() && | |
| awsCredentials.secretAccessKey.trim() && | |
| awsCredentials.region.trim(); | |
| return ( | |
| <Container> | |
| <AlertContainer variant="primary"> | |
| <Title>Connect with AWS Bedrock</Title> | |
| <SubTitle> | |
| Enter your AWS credentials to connect to MI Copilot via AWS Bedrock. Your credentials will be securely stored | |
| and used for authentication. | |
| </SubTitle> | |
| <InputContainer> | |
| <InputRow> | |
| <StyledTextField | |
| type={showAccessKey ? "text" : "password"} | |
| placeholder="AWS Access Key ID" | |
| value={awsCredentials.accessKeyId} | |
| onInput={handleAwsCredentialChange('accessKeyId')} | |
| {...(isValidating ? { disabled: true } : {})} | |
| /> | |
| <EyeButton | |
| type="button" | |
| onClick={toggleAccessKeyVisibility} | |
| title={showAccessKey ? "Hide access key" : "Show access key"} | |
| {...(isValidating ? { disabled: true } : {})} | |
| > | |
| <Codicon name={showAccessKey ? "eye-closed" : "eye"} /> | |
| </EyeButton> | |
| </InputRow> | |
| </InputContainer> | |
| <InputContainer> | |
| <InputRow> | |
| <StyledTextField | |
| type={showSecretKey ? "text" : "password"} | |
| placeholder="AWS Secret Access Key" | |
| value={awsCredentials.secretAccessKey} | |
| onInput={handleAwsCredentialChange('secretAccessKey')} | |
| {...(isValidating ? { disabled: true } : {})} | |
| /> | |
| <EyeButton | |
| type="button" | |
| onClick={toggleSecretKeyVisibility} | |
| title={showSecretKey ? "Hide secret key" : "Show secret key"} | |
| {...(isValidating ? { disabled: true } : {})} | |
| > | |
| <Codicon name={showSecretKey ? "eye-closed" : "eye"} /> | |
| </EyeButton> | |
| </InputRow> | |
| </InputContainer> | |
| <InputContainer> | |
| <InputRow> | |
| <StyledTextField | |
| type="text" | |
| placeholder="AWS Region (e.g., us-east-1)" | |
| value={awsCredentials.region} | |
| onInput={handleAwsCredentialChange('region')} | |
| {...(isValidating ? { disabled: true } : {})} | |
| /> | |
| </InputRow> | |
| </InputContainer> | |
| <InputContainer> | |
| <InputRow> | |
| <StyledTextField | |
| type={showSessionToken ? "text" : "password"} | |
| placeholder="Session Token (optional)" | |
| value={awsCredentials.sessionToken} | |
| onInput={handleAwsCredentialChange('sessionToken')} | |
| {...(isValidating ? { disabled: true } : {})} | |
| /> | |
| <EyeButton | |
| type="button" | |
| onClick={toggleSessionTokenVisibility} | |
| title={showSessionToken ? "Hide session token" : "Show session token"} | |
| {...(isValidating ? { disabled: true } : {})} | |
| > | |
| <Codicon name={showSessionToken ? "eye-closed" : "eye"} /> | |
| </EyeButton> | |
| </InputRow> | |
| </InputContainer> | |
| {errorMessage && ( | |
| <ErrorMessage> | |
| <Codicon name="error" /> | |
| <span>{errorMessage}</span> | |
| </ErrorMessage> | |
| )} | |
| <ButtonContainer> | |
| <VSCodeButton | |
| appearance="primary" | |
| onClick={connectWithAwsCredentials} | |
| {...(isValidating || !isFormValid ? { disabled: true } : {})} | |
| > | |
| {isValidating ? "Validating..." : "Connect with AWS Bedrock"} | |
| </VSCodeButton> | |
| <VSCodeButton | |
| appearance="secondary" | |
| onClick={cancelLogin} | |
| {...(isValidating ? { disabled: true } : {})} | |
| > | |
| Cancel | |
| </VSCodeButton> | |
| </ButtonContainer> | |
| </AlertContainer> | |
| </Container> | |
| ); | |
| } | |
| if (loginMethod === LoginMethod.AWS_BEDROCK) { | |
| const isFormValid = awsCredentials.accessKeyId.trim() && | |
| awsCredentials.secretAccessKey.trim() && | |
| awsCredentials.region.trim(); | |
| return ( | |
| <Container> | |
| <AlertContainer variant="primary"> | |
| <Title>Connect with AWS Bedrock</Title> | |
| <SubTitle> | |
| Enter your AWS credentials to connect to MI Copilot via AWS Bedrock. Your credentials will be securely stored | |
| and used for authentication. | |
| </SubTitle> | |
| {!isFormValid && !isValidating && ( | |
| <SubTitle style={{ color: 'var(--vscode-descriptionForeground)', fontStyle: 'italic' }}> | |
| Please fill in all required fields (Access Key ID, Secret Access Key, and Region). | |
| </SubTitle> | |
| )} | |
| <InputContainer> | |
| <InputRow> | |
| <StyledTextField | |
| type={showAccessKey ? "text" : "password"} | |
| placeholder="AWS Access Key ID" | |
| value={awsCredentials.accessKeyId} | |
| onInput={handleAwsCredentialChange('accessKeyId')} | |
| {...(isValidating ? { disabled: true } : {})} | |
| /> | |
| <EyeButton | |
| type="button" | |
| onClick={toggleAccessKeyVisibility} | |
| title={showAccessKey ? "Hide access key" : "Show access key"} | |
| {...(isValidating ? { disabled: true } : {})} | |
| > | |
| <Codicon name={showAccessKey ? "eye-closed" : "eye"} /> | |
| </EyeButton> | |
| </InputRow> | |
| </InputContainer> | |
| <InputContainer> | |
| <InputRow> | |
| <StyledTextField | |
| type={showSecretKey ? "text" : "password"} | |
| placeholder="AWS Secret Access Key" | |
| value={awsCredentials.secretAccessKey} | |
| onInput={handleAwsCredentialChange('secretAccessKey')} | |
| {...(isValidating ? { disabled: true } : {})} | |
| /> | |
| <EyeButton | |
| type="button" | |
| onClick={toggleSecretKeyVisibility} | |
| title={showSecretKey ? "Hide secret key" : "Show secret key"} | |
| {...(isValidating ? { disabled: true } : {})} | |
| > | |
| <Codicon name={showSecretKey ? "eye-closed" : "eye"} /> | |
| </EyeButton> | |
| </InputRow> | |
| </InputContainer> | |
| <InputContainer> | |
| <InputRow> | |
| <StyledTextField | |
| type="text" | |
| placeholder="AWS Region (e.g., us-east-1)" | |
| value={awsCredentials.region} | |
| onInput={handleAwsCredentialChange('region')} | |
| {...(isValidating ? { disabled: true } : {})} | |
| /> | |
| </InputRow> | |
| </InputContainer> | |
| <InputContainer> | |
| <InputRow> | |
| <StyledTextField | |
| type={showSessionToken ? "text" : "password"} | |
| placeholder="Session Token (optional)" | |
| value={awsCredentials.sessionToken} | |
| onInput={handleAwsCredentialChange('sessionToken')} | |
| {...(isValidating ? { disabled: true } : {})} | |
| /> | |
| <EyeButton | |
| type="button" | |
| onClick={toggleSessionTokenVisibility} | |
| title={showSessionToken ? "Hide session token" : "Show session token"} | |
| {...(isValidating ? { disabled: true } : {})} | |
| > | |
| <Codicon name={showSessionToken ? "eye-closed" : "eye"} /> | |
| </EyeButton> | |
| </InputRow> | |
| </InputContainer> | |
| {errorMessage && ( | |
| <ErrorMessage> | |
| <Codicon name="error" /> | |
| <span>{errorMessage}</span> | |
| </ErrorMessage> | |
| )} | |
| <ButtonContainer> | |
| <VSCodeButton | |
| appearance="primary" | |
| onClick={connectWithAwsCredentials} | |
| {...(isValidating || !isFormValid ? { disabled: true } : {})} | |
| > | |
| {isValidating ? "Validating..." : "Connect with AWS Bedrock"} | |
| </VSCodeButton> | |
| <VSCodeButton | |
| appearance="secondary" | |
| onClick={cancelLogin} | |
| {...(isValidating ? { disabled: true } : {})} | |
| > | |
| Cancel | |
| </VSCodeButton> | |
| </ButtonContainer> | |
| </AlertContainer> | |
| </Container> | |
| ); | |
| } |
Implements: wso2/mi-vscode#1350
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.