diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b4e8aee84..1a58abdd46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ - **fix(kiro): route enterprise IAM Identity Center accounts to their regional endpoint** ([#3631] — thanks @artickc): Kiro/CodeWhisperer access tokens and Q Developer profile ARNs are region-bound, so enterprise IAM Identity Center accounts outside `us-east-1` (e.g. `eu-central-1`) were rejected by the default host. Adds `resolveKiroRegion` (stored region → profileArn region → `us-east-1`) and `kiroRuntimeHost` (regional `q.{region}.amazonaws.com`, legacy `codewhisperer.us-east-1` for the default), routes chat + usage to the regional endpoint, and discovers the region-matched `profileArn` via `ListAvailableProfiles` in a best-effort `postExchange` hook. 9 tests. - **fix(combo): skip same-provider/connection targets on connection-level errors** ([#3637] — thanks @herjarsa): on connection-level upstream errors (408/500/502/503/504/524), remaining same-`provider:connection` targets in a combo request are now skipped to avoid hammering a known-bad connection, in both the priority and round-robin paths. Adjusted in review to **exclude OmniRoute circuit-breaker-open responses** (503 + `X-OmniRoute-Provider-Breaker` / `provider_circuit_open`) from this skip, preserving the invariant that a breaker-open is an ordinary target failure (the next same-provider target is still tried). Co-authored with @herjarsa. - **/v1/responses**: detect stream readiness for tool-call-only and `object`-less chunks so Codex-shaped (reasoning + tools) requests no longer fail with "Stream ended before producing useful content" (#3612) +- **Kiro/AWS auto-import**: set a descriptive account name and dedupe by `profileArn` so imports no longer create nameless duplicate "OAuth Account" rows (#3615) --- diff --git a/src/app/api/oauth/kiro/auto-import/route.ts b/src/app/api/oauth/kiro/auto-import/route.ts index 6bacc16d62..eb8c104520 100755 --- a/src/app/api/oauth/kiro/auto-import/route.ts +++ b/src/app/api/oauth/kiro/auto-import/route.ts @@ -2,7 +2,13 @@ import { NextResponse } from "next/server"; import { homedir } from "os"; import { join } from "path"; import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; -import { createProviderConnection, isCloudEnabled, resolveProxyForProvider } from "@/models"; +import { + createProviderConnection, + getProviderConnections, + updateProviderConnection, + isCloudEnabled, + resolveProxyForProvider, +} from "@/models"; import { syncToCloud } from "@/lib/cloudSync"; import { getConsistentMachineId } from "@/shared/utils/machineId"; import { KiroService } from "@/lib/oauth/services/kiro"; @@ -235,6 +241,59 @@ async function tryAwsSsoCache(targetProvider: string): Promise<{ return { found: false, triedPath: cachePath }; } +// ── Helpers (exported for unit-testing) ────────────────────────────────────── + +/** + * Derives a human-readable display name for a Kiro/AWS connection when the + * OAuth token carries no email claim (social-auth / AWS SSO tokens). Falls + * back through: email → profileArn-based label → provider+region label. + * + * Exported for unit tests (#3615). + */ +export function deriveKiroConnectionName(opts: { + email: string | null | undefined; + profileArn: string | undefined; + region: string | undefined; + targetProvider: string; +}): string { + const { email, profileArn, region, targetProvider } = opts; + if (email) return email; + const r = region || "us-east-1"; + if (profileArn) return `AWS CodeWhisperer (${r})`; + if (targetProvider === "amazon-q") return `Amazon Q (${r})`; + return `Kiro (${r})`; +} + +type ProviderConnectionLike = { + id?: unknown; + providerSpecificData?: unknown; + [key: string]: unknown; +}; + +/** + * Scans a list of existing provider connections and returns the first one + * whose stored `providerSpecificData.profileArn` matches the given ARN. + * Returns null when profileArn is undefined/null or no match is found. + * + * Exported for unit tests (#3615). + */ +export function findKiroConnectionByProfileArn( + connections: ProviderConnectionLike[], + profileArn: string | undefined +): ProviderConnectionLike | null { + if (!profileArn) return null; + for (const conn of connections) { + const psd = conn.providerSpecificData; + if (psd && typeof psd === "object" && !Array.isArray(psd)) { + const stored = (psd as Record).profileArn; + if (typeof stored === "string" && stored === profileArn) { + return conn; + } + } + } + return null; +} + // ── Save to OmniRoute DB ────────────────────────────────────────────────────── async function saveAndRespond( @@ -299,16 +358,44 @@ async function saveAndRespond( const email = kiroService.extractEmailFromJWT(accessToken); - await createProviderConnection({ - provider: targetProvider, - authType: "oauth", - accessToken, - refreshToken, - expiresAt, - email: email || null, - providerSpecificData, - testStatus: "active", - } as any); + // Derive a descriptive name so the UI never shows a blank "OAuth Account" + // when the token carries no email claim (Kiro social-auth / AWS SSO). + const connectionName = deriveKiroConnectionName({ + email, + profileArn, + region: result.region, + targetProvider, + }); + + // Dedup by profileArn: if an existing connection already has the same ARN + // just refresh its tokens instead of inserting a new row. This prevents the + // duplicate-row accumulation reported in #3615 (4 rows after 6 days). + const existingConnections = await getProviderConnections({ provider: targetProvider }); + const existingByArn = findKiroConnectionByProfileArn(existingConnections, profileArn); + + if (existingByArn && typeof existingByArn.id === "string") { + await updateProviderConnection(existingByArn.id, { + accessToken, + refreshToken, + expiresAt, + email: email || null, + name: connectionName, + providerSpecificData, + testStatus: "active", + }); + } else { + await createProviderConnection({ + provider: targetProvider, + authType: "oauth", + accessToken, + refreshToken, + expiresAt, + email: email || null, + name: connectionName, + providerSpecificData, + testStatus: "active", + } as any); + } if (isCloudEnabled()) { const machineId = await getConsistentMachineId(); diff --git a/tests/unit/kiro-auto-import-name-dedup-3615.test.ts b/tests/unit/kiro-auto-import-name-dedup-3615.test.ts new file mode 100644 index 0000000000..e491b6d309 --- /dev/null +++ b/tests/unit/kiro-auto-import-name-dedup-3615.test.ts @@ -0,0 +1,153 @@ +/** + * Regression tests for #3615 — Kiro/AWS auto-import creates a nameless + * "OAuth Account" when email is null, and accumulates duplicate rows for + * subsequent imports with the same profileArn. + * + * Two bugs: + * (a) No display name derived when email=null → UI shows blank "OAuth Account". + * (b) No profileArn dedup guard → every import creates a new DB row. + * + * Both tests use the helpers extracted from saveAndRespond() so we can test + * them without spinning up a full Next.js route. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +// Hermetic temp DATA_DIR so importing the route's dependency graph is safe. +import fs from "node:fs"; +import os from "node:os"; + +const tmpDir = fs.mkdtempSync(os.tmpdir() + "/omniroute-kiro-3615-"); +process.env.DATA_DIR = tmpDir; +process.env.JWT_SECRET = process.env.JWT_SECRET || "test-jwt-secret-3615"; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "test-api-key-secret-3615"; + +test.after(() => { + try { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } catch { + // best effort + } +}); + +// ── helpers under test ──────────────────────────────────────────────────────── + +import { + deriveKiroConnectionName, + findKiroConnectionByProfileArn, +} from "../../src/app/api/oauth/kiro/auto-import/route.ts"; + +// ── (a) Display name derivation ─────────────────────────────────────────────── + +test("derives email as name when email is present", () => { + const name = deriveKiroConnectionName({ + email: "user@example.com", + profileArn: "arn:aws:iam::123456789012:user/test", + region: "us-east-1", + targetProvider: "kiro", + }); + assert.equal(name, "user@example.com"); +}); + +test("derives AWS CodeWhisperer label from profileArn when email is null (enterprise SSO)", () => { + const name = deriveKiroConnectionName({ + email: null, + profileArn: "arn:aws:iam::123456789012:user/test", + region: "eu-west-1", + targetProvider: "kiro", + }); + assert.equal(name, "AWS CodeWhisperer (eu-west-1)"); +}); + +test("derives Kiro label from region when email is null and no profileArn", () => { + const name = deriveKiroConnectionName({ + email: null, + profileArn: undefined, + region: "ap-northeast-1", + targetProvider: "kiro", + }); + assert.equal(name, "Kiro (ap-northeast-1)"); +}); + +test("uses fallback region when region is also absent", () => { + const name = deriveKiroConnectionName({ + email: null, + profileArn: undefined, + region: undefined, + targetProvider: "kiro", + }); + assert.equal(name, "Kiro (us-east-1)"); +}); + +test("uses amazon-q label for amazon-q targetProvider with no email", () => { + const name = deriveKiroConnectionName({ + email: null, + profileArn: undefined, + region: "us-east-1", + targetProvider: "amazon-q", + }); + assert.equal(name, "Amazon Q (us-east-1)"); +}); + +test("derived name is never empty or null", () => { + const name = deriveKiroConnectionName({ + email: null, + profileArn: undefined, + region: undefined, + targetProvider: "kiro", + }); + assert.ok(name && name.length > 0, `expected a non-empty name, got: ${JSON.stringify(name)}`); +}); + +// ── (b) ProfileArn dedup ────────────────────────────────────────────────────── + +// We mock the DB layer to assert that findKiroConnectionByProfileArn calls +// getProviderConnections with the right filter and returns the matching row. + +const FAKE_PROFILE_ARN = "arn:aws:iam::123456789012:user/sso-user"; + +const fakeConnectionWithArn = { + id: "conn-abc", + provider: "kiro", + authType: "oauth", + email: null, + providerSpecificData: { profileArn: FAKE_PROFILE_ARN, region: "us-east-1" }, +}; + +const fakeConnectionNoArn = { + id: "conn-xyz", + provider: "kiro", + authType: "oauth", + email: "other@example.com", + providerSpecificData: { region: "us-east-1" }, +}; + +test("findKiroConnectionByProfileArn returns the matching connection", async () => { + // The function should scan existing kiro connections and match by profileArn. + const result = await findKiroConnectionByProfileArn( + [fakeConnectionWithArn, fakeConnectionNoArn], + FAKE_PROFILE_ARN + ); + assert.deepEqual(result, fakeConnectionWithArn); +}); + +test("findKiroConnectionByProfileArn returns null when no match exists", async () => { + const result = await findKiroConnectionByProfileArn( + [fakeConnectionNoArn], + FAKE_PROFILE_ARN + ); + assert.equal(result, null); +}); + +test("findKiroConnectionByProfileArn returns null for empty connection list", async () => { + const result = await findKiroConnectionByProfileArn([], FAKE_PROFILE_ARN); + assert.equal(result, null); +}); + +test("findKiroConnectionByProfileArn returns null when profileArn arg is undefined", async () => { + const result = await findKiroConnectionByProfileArn( + [fakeConnectionWithArn], + undefined + ); + assert.equal(result, null); +});