diff --git a/packages/worker/client/routes/account-integrations.tsx b/packages/worker/client/routes/account-integrations.tsx index 0495e2c3e4..925218e979 100644 --- a/packages/worker/client/routes/account-integrations.tsx +++ b/packages/worker/client/routes/account-integrations.tsx @@ -39,9 +39,14 @@ import { } from '#client/routes/record-table.tsx' import { renderByokExplainer } from '#client/routes/byok-explainer.tsx' import { + addAccountAnchorId, + buildAddAccountHref, buildCustomIntegrationSetupPrompt, buildIntegrationSetupPrompt, integrationProviderSuggestions, + isAddAccountFormOpen, + nextSuggestedConnectionName, + resolveAddAccountConnectionName, } from '#client/routes/integration-provider-catalog.ts' import { integrationDisplayName } from '#client/routes/integration-filter.ts' import { matchesSearchQuery } from '#client/search-filter.ts' @@ -258,8 +263,11 @@ function buildConnectOauthHref(input: { appSlug?: string }) { const params = new URLSearchParams({ provider: input.name }) + const appSlug = input.appSlug?.trim() if (input.platform) { - params.set('platform', input.appSlug?.trim() || '1') + params.set('platform', appSlug || '1') + } else if (appSlug) { + params.set('app', appSlug) } return `/connect/oauth?${params.toString()}` } @@ -268,6 +276,132 @@ function connectActionLabel(status: 'Connected' | 'Needs setup') { return status === 'Connected' ? 'Reconnect' : 'Connect' } +const addAccountLinkCss = { + ...primaryLinkCss, + justifySelf: 'start', + width: 'fit-content', +} + +function AddAccountForm( + handle: Handle<{ + slug: string + platform: boolean + existingNames: ReadonlyArray + open: boolean + openHref: string + }>, +) { + let nameError: string | null = null + let editedName: string | null = null + let boundSlug = handle.props.slug + + function connectHref(connectionName: string) { + return buildConnectOauthHref({ + name: connectionName, + platform: handle.props.platform, + appSlug: handle.props.slug, + }) + } + + return () => { + if (handle.props.slug !== boundSlug) { + boundSlug = handle.props.slug + editedName = null + nameError = null + } + const suggested = nextSuggestedConnectionName( + handle.props.slug, + handle.props.existingNames, + ) + const name = editedName ?? suggested + if (!handle.props.open) { + return ( + + Add another account + + ) + } + return ( +
{ + event.preventDefault() + const resolved = resolveAddAccountConnectionName({ + name, + suggested, + existingNames: handle.props.existingNames, + }) + if (!resolved.ok) { + nameError = resolved.error + handle.update() + return + } + nameError = null + window.location.assign(connectHref(resolved.name)) + }), + css({ + display: 'grid', + gap: spacing.sm, + justifyItems: 'start', + scrollMarginTop: '5.5rem', + }), + ]} + > + + +
+ ) + } +} + function PlugIcon() { return ( ) })} + entry.name), + ...apps.map((app) => app.slug), + ]} + open={isAddAccountFormOpen(getCurrentHref())} + openHref={buildAddAccountHref(getCurrentHref())} + /> )} diff --git a/packages/worker/client/routes/connect-oauth.tsx b/packages/worker/client/routes/connect-oauth.tsx index 2f233d19f3..d5c796c982 100644 --- a/packages/worker/client/routes/connect-oauth.tsx +++ b/packages/worker/client/routes/connect-oauth.tsx @@ -105,6 +105,18 @@ const emptyConnectOauthLoaderData: ConnectOauthLoaderData = { integration: null, } +function buildConnectOauthIntegrationLookupHref( + providerKey: string, + searchParams: URLSearchParams, +) { + const params = new URLSearchParams({ name: providerKey }) + const platform = searchParams.get('platform')?.trim() + if (platform) params.set('platform', platform) + const app = searchParams.get('app')?.trim() + if (app) params.set('app', app) + return `/account/integrations.json?${params.toString()}` +} + /** * SPA-navigation prefetch mirroring the server handler's SSR embed: the * stored or built-in record for `?provider=` visits, resolved before the @@ -125,9 +137,8 @@ export async function connectOauthRouteLoader( if (!providerKey) { return { connectOauth: emptyConnectOauthLoaderData } } - const platformParam = params.get('platform')?.trim() const response = await fetch( - `/account/integrations.json?name=${encodeURIComponent(providerKey)}${platformParam ? `&platform=${encodeURIComponent(platformParam)}` : ''}`, + buildConnectOauthIntegrationLookupHref(providerKey, params), { headers: { Accept: 'application/json' }, credentials: 'include', @@ -521,14 +532,15 @@ export function ConnectOauthRoute(handle: Handle) { const readExistingIntegrationConfig = async ( queryConfig: ConnectOauthQueryConfig, ): Promise => { - const platformParam = + const lookupSearch = typeof window !== 'undefined' - ? (new URLSearchParams(window.location.search) - .get('platform') - ?.trim() ?? '') - : '' + ? new URLSearchParams(window.location.search) + : new URLSearchParams() const response = await fetch( - `/account/integrations.json?name=${encodeURIComponent(queryConfig.providerKey)}${platformParam ? `&platform=${encodeURIComponent(platformParam)}` : ''}`, + buildConnectOauthIntegrationLookupHref( + queryConfig.providerKey, + lookupSearch, + ), { method: 'GET', headers: { Accept: 'application/json' }, diff --git a/packages/worker/client/routes/integration-provider-catalog.node.test.ts b/packages/worker/client/routes/integration-provider-catalog.node.test.ts index 04c7dfbcea..b639e8640a 100644 --- a/packages/worker/client/routes/integration-provider-catalog.node.test.ts +++ b/packages/worker/client/routes/integration-provider-catalog.node.test.ts @@ -1,8 +1,13 @@ import { expect, test } from 'vitest' import { getGuideBySlug } from '#worker/guides/catalog.ts' import { + buildAddAccountHref, buildIntegrationSetupPrompt, integrationProviderSuggestions, + isAddAccountFormOpen, + isTakenConnectionName, + nextSuggestedConnectionName, + resolveAddAccountConnectionName, } from './integration-provider-catalog.ts' test('integration provider suggestions resolve guide-backed prompts and keep a generic fallback', () => { @@ -36,3 +41,66 @@ test('integration provider suggestions resolve guide-backed prompts and keep a g expect(prompt.length).toBeGreaterThan(0) expect(prompt).not.toContain('coding_guide_get') }) + +test('next suggested connection name skips taken {slug}-{n} keys', () => { + expect(nextSuggestedConnectionName('google', ['google'])).toBe('google-2') + expect(nextSuggestedConnectionName('google', ['google', 'google-2'])).toBe( + 'google-3', + ) + expect(nextSuggestedConnectionName('google', ['google', 'google-work'])).toBe( + 'google-2', + ) + expect( + nextSuggestedConnectionName('google', [ + 'google', + 'linear', + 'Google-2', + 'google-personal', + ]), + ).toBe('google-3') +}) + +test('add-account name resolution rejects names already used by any connection or app', () => { + const existingNames = ['google', 'linear', 'google-2', 'Google Work'] + expect(isTakenConnectionName('google-2', existingNames)).toBe(true) + expect(isTakenConnectionName('Google-2', existingNames)).toBe(true) + expect(isTakenConnectionName('google-work', existingNames)).toBe(true) + expect(isTakenConnectionName('google-3', existingNames)).toBe(false) + + const duplicate = resolveAddAccountConnectionName({ + name: 'google-2', + suggested: 'google-3', + existingNames, + }) + expect(duplicate.ok).toBe(false) + if (!duplicate.ok) { + expect(duplicate.error.length).toBeGreaterThan(0) + } + expect( + resolveAddAccountConnectionName({ + name: ' Google-3 ', + suggested: 'google-3', + existingNames, + }), + ).toEqual({ ok: true, name: 'google-3' }) + expect( + resolveAddAccountConnectionName({ + name: ' ', + suggested: 'google-3', + existingNames, + }), + ).toEqual({ ok: true, name: 'google-3' }) +}) + +test('add-account href keeps the current path and search, then opens the form anchor', () => { + expect(isAddAccountFormOpen('/account/integrations/google')).toBe(false) + expect( + isAddAccountFormOpen('/account/integrations/google?add-account=1'), + ).toBe(true) + expect(buildAddAccountHref('/account/integrations/google?q=goo')).toBe( + '/account/integrations/google?q=goo&add-account=1#add-account', + ) + expect(buildAddAccountHref('/account/integrations/apps/google')).toBe( + '/account/integrations/apps/google?add-account=1#add-account', + ) +}) diff --git a/packages/worker/client/routes/integration-provider-catalog.ts b/packages/worker/client/routes/integration-provider-catalog.ts index 9b3345c318..257e14634f 100644 --- a/packages/worker/client/routes/integration-provider-catalog.ts +++ b/packages/worker/client/routes/integration-provider-catalog.ts @@ -1,3 +1,5 @@ +import { normalizeProviderKey } from '@kody-internal/shared/url-hosts.ts' + /** * Suggested providers for the integrations page. Kody intentionally has no * built-in OAuth apps ("bring your own keys"), so each suggestion carries a @@ -96,3 +98,63 @@ export function buildCustomIntegrationSetupPrompt() { 'and completing the OAuth authorization flow.', ].join(' ') } + +function takenConnectionNameSet(existingNames: ReadonlyArray) { + return new Set( + existingNames + .map((name) => normalizeProviderKey(name)) + .filter((name) => name.length > 0), + ) +} + +export function isTakenConnectionName( + name: string, + existingNames: ReadonlyArray, +) { + const key = normalizeProviderKey(name) + return Boolean(key && takenConnectionNameSet(existingNames).has(key)) +} + +export function resolveAddAccountConnectionName(input: { + name: string + suggested: string + existingNames: ReadonlyArray +}): { ok: true; name: string } | { ok: false; error: string } { + const next = normalizeProviderKey(input.name.trim()) || input.suggested + if (isTakenConnectionName(next, input.existingNames)) { + return { + ok: false, + error: 'That name is already used by another connection.', + } + } + return { ok: true, name: next } +} + +export function nextSuggestedConnectionName( + slug: string, + existingNames: ReadonlyArray, +) { + const taken = takenConnectionNameSet(existingNames) + const slugKey = normalizeProviderKey(slug) + if (!slugKey) return slug + if (!taken.has(slugKey)) return slugKey + let n = 2 + while (taken.has(`${slugKey}-${n}`)) n += 1 + return `${slugKey}-${n}` +} + +export const addAccountQueryParam = 'add-account' +export const addAccountAnchorId = 'add-account' + +export function isAddAccountFormOpen(href: string) { + return new URL(href, 'http://localhost').searchParams.has( + addAccountQueryParam, + ) +} + +export function buildAddAccountHref(href: string) { + const url = new URL(href, 'http://localhost') + url.searchParams.set(addAccountQueryParam, '1') + url.hash = addAccountAnchorId + return `${url.pathname}${url.search}${url.hash}` +} diff --git a/packages/worker/src/app/account-integrations-data.node.test.ts b/packages/worker/src/app/account-integrations-data.node.test.ts index e6110ec771..39aba9429b 100644 --- a/packages/worker/src/app/account-integrations-data.node.test.ts +++ b/packages/worker/src/app/account-integrations-data.node.test.ts @@ -318,6 +318,34 @@ test('endpoint-incomplete user records defer to an enabled built-in of the same ) expect(noFallback?.clientId).toBe('user-linear-client') expect(noFallback?.platform ?? false).toBe(false) + + const pinnedByo = await loadAccountIntegrationByName( + env, + fakeUser(userId), + 'work', + { appSlug: 'github' }, + ) + expect(pinnedByo).toMatchObject({ + name: 'work', + appSlug: 'github', + clientId: 'user-github-client', + }) + expect(pinnedByo?.platform ?? false).toBe(false) + + // An incomplete pinned app must not fall back to a built-in that + // happens to share the typed connection name. + const pinnedIncomplete = await loadAccountIntegrationByName( + env, + fakeUser(userId), + 'github-platform', + { appSlug: 'linear' }, + ) + expect(pinnedIncomplete).toMatchObject({ + name: 'github-platform', + appSlug: 'linear', + clientId: 'user-linear-client', + }) + expect(pinnedIncomplete?.platform ?? false).toBe(false) }) test('loadAccountIntegrationsData includes OAuth apps with their connections', async () => { diff --git a/packages/worker/src/app/account-integrations-data.ts b/packages/worker/src/app/account-integrations-data.ts index 17898c28f6..c605697c0f 100644 --- a/packages/worker/src/app/account-integrations-data.ts +++ b/packages/worker/src/app/account-integrations-data.ts @@ -15,6 +15,7 @@ import { getOauthApp, listJoinedIntegrations, listOauthApps, + oauthAppToSetupPrefill, toJoinedIntegrationConfig, type OauthAppSetupPrefill, type PlatformOauthApp, @@ -286,6 +287,21 @@ export async function loadAccountOauthAppBySlug( * (agents typically save tokenUrl and apiBaseUrl but no authorize URL) * cannot, and would dead-end the page on "missing configuration". */ +export function readConnectOauthLookupOptions(searchParams: URLSearchParams) { + const platformParam = searchParams.get('platform')?.trim() + const appParam = searchParams.get('app')?.trim() + return { + preferPlatform: platformParam === '1', + platformSlug: + platformParam && platformParam !== '1' + ? (normalizeProviderKey(platformParam) ?? undefined) + : undefined, + appSlug: appParam + ? (normalizeProviderKey(appParam) ?? undefined) + : undefined, + } +} + function recordCanDriveConnectFlow(record: AccountIntegrationRecord): boolean { return Boolean( record.authorization?.authorizeUrl?.trim() && record.tokenUrl?.trim(), @@ -310,6 +326,12 @@ export async function loadAccountIntegrationByName( * google-2 keeps an existing bring-your-own google connection intact. */ platformSlug?: string + /** + * Saved bring-your-own app to reuse under a new connection name + * (`app=`): connecting `work` on the google app must not depend + * on inferring that app from the typed name. + */ + appSlug?: string }, ): Promise { // A user-lane record still wins when it can actually drive the flow (the @@ -337,6 +359,20 @@ export async function loadAccountIntegrationByName( return (await platformFallback()) ?? record } + if (options?.appSlug) { + const app = await getOauthApp({ + env, + userId: user.mcpUser.userId, + slug: options.appSlug, + }) + if (app) { + // Keep the pinned app even when it cannot drive authorize yet. + // Falling back by the typed connection name can land on a + // different built-in (`app=linear&provider=github-platform`). + return toAppOnlyIntegrationRecord(oauthAppToSetupPrefill(app), name) + } + } + // 2–3. Exact app slug, else field-wise provider-family prefill (shared // client id across github/github-kent, shared google app, etc.). const prefill = await findOauthAppForProviderSetup({ diff --git a/packages/worker/src/app/handlers/account-integrations.ts b/packages/worker/src/app/handlers/account-integrations.ts index 10a79437b4..4db35337f2 100644 --- a/packages/worker/src/app/handlers/account-integrations.ts +++ b/packages/worker/src/app/handlers/account-integrations.ts @@ -1,14 +1,12 @@ import { z } from 'zod' import { jsonResponse } from '#worker/json-response.ts' import { type Action } from 'remix/router' -import { - normalizeProviderKey, - safeParseHost, -} from '@kody-internal/shared/url-hosts.ts' +import { safeParseHost } from '@kody-internal/shared/url-hosts.ts' import { hasAlternativeBuiltInApp, hasStoredConnectClientSecret, loadAccountIntegrationByName, + readConnectOauthLookupOptions, loadAccountIntegrationsData, loadExistingConnectionSummary, loadAccountOauthAppBySlug, @@ -84,18 +82,12 @@ export function createAccountIntegrationsApiHandler(env: Env) { // `platform=1` forces the built-in of the same name; // `platform=` connects that built-in under a // different connection name (rename-instead-of-replace). - const platformParam = searchParams.get('platform')?.trim() + // `app=` reuses a saved bring-your-own app under `name`. const integration = await loadAccountIntegrationByName( env, user, name, - { - preferPlatform: platformParam === '1', - platformSlug: - platformParam && platformParam !== '1' - ? (normalizeProviderKey(platformParam) ?? undefined) - : undefined, - }, + readConnectOauthLookupOptions(searchParams), ) const [builtInAvailable, existingConnection, hasStoredClientSecret] = await Promise.all([ diff --git a/packages/worker/src/app/handlers/connect-oauth.node.test.ts b/packages/worker/src/app/handlers/connect-oauth.node.test.ts index 2213663489..2a2f365c9d 100644 --- a/packages/worker/src/app/handlers/connect-oauth.node.test.ts +++ b/packages/worker/src/app/handlers/connect-oauth.node.test.ts @@ -12,6 +12,16 @@ const mockModule = vi.hoisted(() => ({ hasAlternativeBuiltInApp: vi.fn<() => Promise>(), loadExistingConnectionSummary: vi.fn<() => Promise>(), hasStoredConnectClientSecret: vi.fn<() => Promise>(), + readConnectOauthLookupOptions: (searchParams: URLSearchParams) => { + const platformParam = searchParams.get('platform')?.trim() + const appParam = searchParams.get('app')?.trim() + return { + preferPlatform: platformParam === '1', + platformSlug: + platformParam && platformParam !== '1' ? platformParam : undefined, + appSlug: appParam || undefined, + } + }, renderAppPage: vi.fn<(input: unknown) => Promise>(), })) @@ -34,6 +44,8 @@ vi.mock('#app/account-integrations-data.ts', () => ({ mockModule.loadExistingConnectionSummary(...args), hasStoredConnectClientSecret: (...args: Array) => mockModule.hasStoredConnectClientSecret(...args), + readConnectOauthLookupOptions: (searchParams: URLSearchParams) => + mockModule.readConnectOauthLookupOptions(searchParams), })) vi.mock('#app/ssr-render.tsx', () => ({ @@ -94,7 +106,7 @@ test('provider visits embed SSR loader data and honor platform lookup flags', as env, expect.anything(), 'github', - { preferPlatform: false, platformSlug: undefined }, + { preferPlatform: false, platformSlug: undefined, appSlug: undefined }, ) expect(mockModule.renderAppPage).toHaveBeenCalledWith( expect.objectContaining({ @@ -129,7 +141,7 @@ test('provider visits embed SSR loader data and honor platform lookup flags', as env, expect.anything(), 'google', - { preferPlatform: true, platformSlug: undefined }, + { preferPlatform: true, platformSlug: undefined, appSlug: undefined }, ) await createConnectOauthHandler(env).handler( @@ -143,7 +155,19 @@ test('provider visits embed SSR loader data and honor platform lookup flags', as env, expect.anything(), 'google-2', - { preferPlatform: false, platformSlug: 'google' }, + { preferPlatform: false, platformSlug: 'google', appSlug: undefined }, + ) + + await createConnectOauthHandler(env).handler( + new RequestContext( + new Request('https://example.com/connect/oauth?provider=work&app=google'), + ), + ) + expect(mockModule.loadAccountIntegrationByName).toHaveBeenLastCalledWith( + env, + expect.anything(), + 'work', + { preferPlatform: false, platformSlug: undefined, appSlug: 'google' }, ) }) diff --git a/packages/worker/src/app/handlers/connect-oauth.ts b/packages/worker/src/app/handlers/connect-oauth.ts index 2461ce0670..1ae66eb771 100644 --- a/packages/worker/src/app/handlers/connect-oauth.ts +++ b/packages/worker/src/app/handlers/connect-oauth.ts @@ -5,6 +5,7 @@ import { hasStoredConnectClientSecret, loadAccountIntegrationByName, loadExistingConnectionSummary, + readConnectOauthLookupOptions, } from '#app/account-integrations-data.ts' import { readAuthenticatedAppUser } from '#app/authenticated-user.ts' import { requirePageSession } from '#app/page-auth.ts' @@ -57,18 +58,12 @@ async function loadConnectOauthLoaderData( } // `platform=1` forces the built-in of the same name; `platform=` // connects that built-in under a different connection name. - const platformParam = requestUrl.searchParams.get('platform')?.trim() + // `app=` reuses a saved bring-your-own app under `provider`. const integration = await loadAccountIntegrationByName( env, user, providerKey, - { - preferPlatform: platformParam === '1', - platformSlug: - platformParam && platformParam !== '1' - ? (normalizeProviderKey(platformParam) ?? undefined) - : undefined, - }, + readConnectOauthLookupOptions(requestUrl.searchParams), ) const [builtInAvailable, existingConnection, hasStoredClientSecret] = await Promise.all([ diff --git a/packages/worker/src/app/ssr-render.node.test.ts b/packages/worker/src/app/ssr-render.node.test.ts index d5f9f8ff97..74cc6673b8 100644 --- a/packages/worker/src/app/ssr-render.node.test.ts +++ b/packages/worker/src/app/ssr-render.node.test.ts @@ -952,6 +952,13 @@ test('renderAppPage server-renders simplified integration and secret-approval pa expect(connectionResponse.status).toBe(200) const connectionHtml = await readResponseText(connectionResponse) expect(connectionHtml).toContain('1 account connected.') + expect(connectionHtml).toContain('data-testid="add-account-open"') + expect(connectionHtml).toContain('Add another account') + expect(connectionHtml).toContain( + 'href="/account/integrations/google?add-account=1#add-account"', + ) + expect(connectionHtml).toContain('data-prevent-scroll-reset') + expect(connectionHtml).not.toContain('data-testid="add-account-form"') expect(connectionHtml).toContain('>Reconnect<') expect(connectionHtml).toContain('data-testid="provider-mark"') expect(connectionHtml).toContain('data-testid="integration-advanced"') @@ -1028,6 +1035,33 @@ test('renderAppPage server-renders simplified integration and secret-approval pa expect(builtInHtml).toContain('data-testid="built-in-indicator"') expect(builtInHtml).toContain('Provided by Kody') expect(builtInHtml).toContain('2 accounts connected.') + expect(builtInHtml).toContain('data-testid="add-account-open"') + expect(builtInHtml).toContain('Add another account') + expect(builtInHtml).not.toContain('data-testid="add-account-form"') + + const addAccountResponse = await renderAppPage({ + request: new Request( + 'https://example.com/account/integrations/google?add-account=1#add-account', + { headers: { Cookie: cookie } }, + ), + env, + loaderData: { + accountIntegrations: { + ok: true, + email: 'user@example.com', + username: 'account-user', + integrations: [googleConnection], + apps: [googleApp], + }, + }, + }) + expect(addAccountResponse.status).toBe(200) + const addAccountHtml = await readResponseText(addAccountResponse) + expect(addAccountHtml).toContain('data-testid="add-account-form"') + expect(addAccountHtml).toContain('id="add-account"') + expect(addAccountHtml).toContain('Connection name') + expect(addAccountHtml).toContain('value="google-2"') + expect(addAccountHtml).not.toContain('data-testid="add-account-open"') expect(builtInHtml).toContain('Needs setup') expect(builtInHtml).toContain('>Connect<') expect(builtInHtml).toContain( diff --git a/packages/worker/src/integrations/service.ts b/packages/worker/src/integrations/service.ts index 95d26d511c..778b8a6851 100644 --- a/packages/worker/src/integrations/service.ts +++ b/packages/worker/src/integrations/service.ts @@ -612,7 +612,9 @@ export async function findOauthAppForProviderSetup(input: { return setupPrefillHasAgreedField(merged) ? merged : null } -function oauthAppToSetupPrefill(app: UserOauthApp): OauthAppSetupPrefill { +export function oauthAppToSetupPrefill( + app: UserOauthApp, +): OauthAppSetupPrefill { return { userId: app.userId, slug: app.slug,