fix(kiro): capture profileArn from whoami and classify profileArn-required 400s (#993) - #1032
Conversation
…uired 400s (#993) Builder ID imports often lack a profileArn in SQLite. The import now reads the documented whoami --format json surface (narrow shapes, structural validation, imported SQLite ARN stays authoritative, fail closed when absent) and persists it account-scoped. A profileArn- required upstream rejection classifies as the stable, actionable kiro_profile_required instead of the generic validation bucket. No live AWS verification — structural tests only.
#993) A concurrent external Kiro session switch between the SQLite read and whoami could attach account B's profileArn to account A's token. whoami's identity is now accepted only when the session token still matches the import (refresh, or access when refresh is absent); a mid-flight switch regression pins the behavior.
📝 WalkthroughWalkthroughChangesKiro profile identity and error handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SQLiteCredential
participant KiroCLI
participant oauthCredentialFromImported
SQLiteCredential->>oauthCredentialFromImported: provide imported credential and session token
oauthCredentialFromImported->>KiroCLI: request whoami identity
KiroCLI-->>oauthCredentialFromImported: return email and profile ARN
oauthCredentialFromImported->>SQLiteCredential: revalidate active session token
SQLiteCredential-->>oauthCredentialFromImported: confirm matching or changed session
oauthCredentialFromImported-->>SQLiteCredential: return merged or cleared identity metadata
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2dd48990c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // whoami's identity only when the session token STILL matches the import — | ||
| // refresh token, or access token when refresh is absent. | ||
| if (identity.profileArn !== undefined) { | ||
| const current = readKiroCliSqliteCredential(); |
There was a problem hiding this comment.
When a user imports Kiro from KIROCLI_DB_PATH/KIRO_CLI_DB_FILE, the token came from that override database, but kiro-cli whoami still reports the native CLI session. This revalidation calls readKiroCliSqliteCredential() again, which rereads the override and therefore matches the imported token, so OCX can persist account A's access/refresh token with account B's profileArn and send the wrong profile in later Kiro requests. Skip whoami profile capture for import-only DB selectors, or revalidate against the actual native CLI DB that whoami uses.
Useful? React with 👍 / 👎.
| expires: imported.expires, | ||
| source: imported.source === "json" ? "credential-file" : "local-cli", | ||
| ...(imported.profileArn ? { accountId: imported.profileArn } : {}), | ||
| ...(resolvedProfileArn ? { accountId: resolvedProfileArn } : {}), |
There was a problem hiding this comment.
When an existing Kiro account was saved before it had a profile ARN, its stored identity is usually the email. Returning the new credential with accountId set to the newly discovered ARN makes saveCredential() match on accountId ?? email, so a later ordinary login or ocx account login kiro --reauth for the same email appends a second active account instead of updating the old one; the stale profile-less row remains selectable and still fails gated Kiro models. Reconcile by email when adding the ARN to a previously email-only Kiro account, or avoid promoting the ARN to accountId until the store can merge both identities.
Useful? React with 👍 / 👎.
| message: "kiro_profile_required: Kiro requires a CodeWhisperer profileArn for this account and model. Re-login or re-import the matching Kiro account (ocx account login kiro --reauth) so the profile is captured, then retry.", | ||
| status: 400, | ||
| errorType: "invalid_request_error", | ||
| code: "kiro_profile_required", |
There was a problem hiding this comment.
The new kiro_profile_required code is only carried by stream/parser error events; the ordinary non-stream HTTP path calls safeKiroHttpErrorMessage(), then wraps this response as formatErrorResponse(status, "upstream_error", ...), so the /v1/chat/completions reproduction for this 400 still returns top-level code: "upstream_error" instead of the stable code added here. Return a structured classification to the HTTP wrapper or map this message before formatErrorResponse so non-stream clients can handle the same condition.
Useful? React with 👍 / 👎.
#1032 review blocker. The existing session-switch test asserted only that accountId and kiro.profileArn were absent — but pre-fix code ignores whoami's ARN entirely, so those assertions passed against the unfixed implementation too. It was a shape assertion wearing an activation proof's clothes. The email is what pre-fix code WOULD have kept, so asserting its absence is what actually exercises the mismatch path. Adds that, plus the two refresh-absent cases the implementation's access-token fallback needs: access-token-only revalidation succeeding, and an access-token-only session that changes under whoami being rejected. Verified by reverting src/oauth/kiro.ts to the dev version: all three new tests fail against pre-fix code and pass against the fix.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adapters/kiro-errors.ts`:
- Around line 114-122: Update the profile-required detection in the Kiro error
mapping to only apply when status is undefined or 400, preserving event-stream
exceptions while excluding rate-limit responses. Replace the independent
evidence checks with a bounded phrase matching profileArn followed by required,
and add negative tests covering a 429 response and an unrelated required field.
In `@src/oauth/kiro.ts`:
- Around line 285-290: Update the identity revalidation guard in the flow around
readKiroCliSqliteCredential so token comparison runs when either identity.email
or identity.profileArn is present, not only when profileArn exists. Preserve
clearing identity when the imported and current credentials differ, and add a
regression test covering a session switch where whoami returns an email without
a valid profile ARN.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b3e42c8e-1c3b-4957-8a70-fb09405f9303
📒 Files selected for processing (5)
src/adapters/kiro-errors.tssrc/oauth/kiro.tstests/kiro-oauth.test.tstests/kiro-retry.test.tstests/kiro-stream.test.ts
| if (evidence.includes("profilearn") && evidence.includes("required")) { | ||
| return { | ||
| message: "kiro_profile_required: Kiro requires a CodeWhisperer profileArn for this account and model. Re-login or re-import the matching Kiro account (ocx account login kiro --reauth) so the profile is captured, then retry.", | ||
| status: 400, | ||
| errorType: "invalid_request_error", | ||
| code: "kiro_profile_required", | ||
| retryable: false, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Restrict kiro_profile_required to a required-profile 400 response.
Line 114 matches independent occurrences of profilearn and required. A 429 payload such as profileArn is required; quota exceeded becomes a 400 non-retryable error before the rate-limit branch. An unrelated 400 payload such as profileArn is malformed; clientId is required also gives incorrect re-login guidance.
Require status === undefined || status === 400 so event-stream exceptions remain supported. Match a bound profileArn ... required phrase. Add negative tests for a 429 response and an unrelated required field.
Proposed fix
const headerType = headerValue(headers, ":exception-type") || headerValue(headers, ":error-type") || "";
const evidence = [headerType, ...payloadDetails(payloadText), message].join(" ").toLowerCase();
+ const profileArnRequired =
+ /\bprofile\s*arn\b\s+(?:is\s+)?required\b|\brequired\s+(?:for\s+)?profile\s*arn\b/.test(evidence);
if (isContentLengthError(evidence)) {
return {
// ...
};
}
- if (evidence.includes("profilearn") && evidence.includes("required")) {
+ if ((status === undefined || status === 400) && profileArnRequired) {📝 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 (evidence.includes("profilearn") && evidence.includes("required")) { | |
| return { | |
| message: "kiro_profile_required: Kiro requires a CodeWhisperer profileArn for this account and model. Re-login or re-import the matching Kiro account (ocx account login kiro --reauth) so the profile is captured, then retry.", | |
| status: 400, | |
| errorType: "invalid_request_error", | |
| code: "kiro_profile_required", | |
| retryable: false, | |
| }; | |
| } | |
| const profileArnRequired = | |
| /\bprofile\s*arn\b\s+(?:is\s+)?required\b|\brequired\s+(?:for\s+)?profile\s*arn\b/.test(evidence); | |
| if ((status === undefined || status === 400) && profileArnRequired) { | |
| return { | |
| message: "kiro_profile_required: Kiro requires a CodeWhisperer profileArn for this account and model. Re-login or re-import the matching Kiro account (ocx account login kiro --reauth) so the profile is captured, then retry.", | |
| status: 400, | |
| errorType: "invalid_request_error", | |
| code: "kiro_profile_required", | |
| retryable: false, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adapters/kiro-errors.ts` around lines 114 - 122, Update the
profile-required detection in the Kiro error mapping to only apply when status
is undefined or 400, preserving event-stream exceptions while excluding
rate-limit responses. Replace the independent evidence checks with a bounded
phrase matching profileArn followed by required, and add negative tests covering
a 429 response and an unrelated required field.
| if (identity.profileArn !== undefined) { | ||
| const current = readKiroCliSqliteCredential(); | ||
| const importedKey = imported.refresh || imported.access; | ||
| const currentKey = current ? current.refresh || current.access : ""; | ||
| if (!current || currentKey !== importedKey) identity = {}; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Revalidate every whoami identity field.
Line 285 revalidates the SQLite session only when identity.profileArn exists. If session A is imported, the CLI switches to session B, and whoami returns only B's email, Line 289 does not run. Line 306 then returns B's email with A's credentials and any authoritative SQLite profile metadata.
Run the token comparison when either identity.email or identity.profileArn exists. Add a regression test where the session changes and whoami returns an email without a valid profile ARN.
Proposed fix
- if (identity.profileArn !== undefined) {
+ if (identity.email !== undefined || identity.profileArn !== undefined) {
const current = readKiroCliSqliteCredential();
const importedKey = imported.refresh || imported.access;
const currentKey = current ? current.refresh || current.access : "";
if (!current || currentKey !== importedKey) identity = {};📝 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 (identity.profileArn !== undefined) { | |
| const current = readKiroCliSqliteCredential(); | |
| const importedKey = imported.refresh || imported.access; | |
| const currentKey = current ? current.refresh || current.access : ""; | |
| if (!current || currentKey !== importedKey) identity = {}; | |
| } | |
| if (identity.email !== undefined || identity.profileArn !== undefined) { | |
| const current = readKiroCliSqliteCredential(); | |
| const importedKey = imported.refresh || imported.access; | |
| const currentKey = current ? current.refresh || current.access : ""; | |
| if (!current || currentKey !== importedKey) identity = {}; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/oauth/kiro.ts` around lines 285 - 290, Update the identity revalidation
guard in the flow around readKiroCliSqliteCredential so token comparison runs
when either identity.email or identity.profileArn is present, not only when
profileArn exists. Preserve clearing identity when the imported and current
credentials differ, and add a regression test covering a session switch where
whoami returns an email without a valid profile ARN.
1 participant
Bug-stack campaign lane (
devlog/_plan/260805_bug_stack_campaign/110). Fixes #993.Builder ID imports often lack a profileArn in SQLite, so gated models 400 with no actionable path.
whoami --format jsonsurface: narrow shapes only, structural ARN validation, imported SQLite ARN stays authoritative, fail closed when absent. A concurrent session switch between the SQLite read and whoami is revalidated (refresh, or access when refresh is absent) so account A's token never carries account B's ARN.kiro_profile_required(non-retryable, re-login guidance) instead of the generic validation bucket. No hardcoded/derived ARNs; no administrative profile listing.bun run typecheck0 errors;bun run privacy:scanpass; full suite on ssh lidge 8314/0 (campaign tree).Summary by CodeRabbit
Bug Fixes
profileArnerrors with clear re-authentication guidance.Tests