diff --git a/packages/react/src/components/add-account-modal.test.ts b/packages/react/src/components/add-account-modal.test.ts index 24b12fc9c..935d38dca 100644 --- a/packages/react/src/components/add-account-modal.test.ts +++ b/packages/react/src/components/add-account-modal.test.ts @@ -232,7 +232,7 @@ describe("runDcrConnect", () => { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", integrationName: "Linear MCP", - existingSlugs: [], + existingClients: [], redirectUri: "https://localhost:5394/api/oauth/callback", integration: TEST_INTEGRATION, }, @@ -259,6 +259,101 @@ describe("runDcrConnect", () => { expect(startArgs!.owner).toBe("user"); }); + it("reuses an existing client minted for this integration instead of minting a duplicate", async () => { + // Regression guard for "Datadog, Datadog 2, … Datadog 10": a prior DCR client + // for THIS integration (same owner, same token host) must be reused, so a + // second connect runs only `start` — never `register`. + const calls: string[] = []; + let startArgs: StartArgs | null = null; + const outcome = await runDcrConnect( + { + probe: (): Promise => { + calls.push("probe"); + return Promise.resolve({ + authorizationUrl: "https://auth.example.com/authorize", + tokenUrl: "https://auth.example.com/token", + registrationEndpoint: "https://auth.example.com/register", + }); + }, + register: (args: RegisterArgs): Promise => { + calls.push("register"); + return Promise.resolve(args.slug); + }, + start: (args: StartArgs): void => { + calls.push("start"); + startArgs = args; + }, + }, + { + discoveryUrl: "https://mcp.example.com/mcp", + owner: "user", + integrationName: "Linear MCP", + existingClients: [ + { + owner: "user", + slug: OAuthClientSlug.make("linear-mcp"), + grant: "authorization_code", + authorizationUrl: "https://auth.example.com/authorize", + tokenUrl: "https://auth.example.com/token", + clientId: "minted-earlier", + origin: { kind: "dynamic_client_registration", integration: TEST_INTEGRATION }, + }, + ], + integration: TEST_INTEGRATION, + }, + ); + + expect(outcome.kind).toBe("started"); + // Probed (to confirm the same server), then reused — NO second registration. + expect(calls).toEqual(["probe", "start"]); + expect(String(startArgs!.client)).toBe("linear-mcp"); + }); + + it("does not reuse a client belonging to a DIFFERENT integration", async () => { + // A Datadog DCR client must never be reused for an Atlassian connect, even + // though both went through DCR — provenance is keyed by integration. + const calls: string[] = []; + await runDcrConnect( + { + probe: (): Promise => + Promise.resolve({ + authorizationUrl: "https://auth.atlassian.com/authorize", + tokenUrl: "https://auth.atlassian.com/token", + registrationEndpoint: "https://auth.atlassian.com/register", + }), + register: (args: RegisterArgs): Promise => { + calls.push("register"); + return Promise.resolve(args.slug); + }, + start: (): void => { + calls.push("start"); + }, + }, + { + discoveryUrl: "https://mcp.atlassian.com/mcp", + owner: "user", + integrationName: "Atlassian", + existingClients: [ + { + owner: "user", + slug: OAuthClientSlug.make("datadog"), + grant: "authorization_code", + authorizationUrl: "https://app.datadoghq.com/oauth2/authorize", + tokenUrl: "https://api.datadoghq.com/oauth2/token", + clientId: "datadog-client", + origin: { + kind: "dynamic_client_registration", + integration: IntegrationSlug.make("datadog"), + }, + }, + ], + integration: TEST_INTEGRATION, + }, + ); + // No Atlassian client exists yet → it mints a fresh one, not reuse Datadog. + expect(calls).toContain("register"); + }); + it("prefers declared scopes over probed scopes when present", async () => { let registerArgs: RegisterArgs | null = null; const outcome = await runDcrConnect( @@ -280,7 +375,7 @@ describe("runDcrConnect", () => { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", integrationName: "App", - existingSlugs: [], + existingClients: [], declaredScopes: ["declared.scope"], integration: TEST_INTEGRATION, }, @@ -313,7 +408,7 @@ describe("runDcrConnect", () => { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", integrationName: "App", - existingSlugs: [], + existingClients: [], integration: TEST_INTEGRATION, }, ); @@ -344,7 +439,7 @@ describe("runDcrConnect", () => { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", integrationName: "App", - existingSlugs: [], + existingClients: [], integration: TEST_INTEGRATION, }, ); @@ -374,7 +469,7 @@ describe("runDcrConnect", () => { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", integrationName: "App", - existingSlugs: [], + existingClients: [], integration: TEST_INTEGRATION, }, ); diff --git a/packages/react/src/components/add-account-modal.tsx b/packages/react/src/components/add-account-modal.tsx index ceb12316d..1e8a72171 100644 --- a/packages/react/src/components/add-account-modal.tsx +++ b/packages/react/src/components/add-account-modal.tsx @@ -42,6 +42,7 @@ import { oauthCallbackUrl, useOAuthPopupFlow } from "../plugins/oauth-sign-in"; import { clientDisplayName, clientHost, + getRootDomain, uniqueClientSlug, useOAuthClientsForIntegration, type OAuthClientOption, @@ -471,8 +472,10 @@ type RunDcrConnectInput = { readonly discoveryUrl: string; readonly owner: Owner; readonly integrationName: string; - /** The owner's existing client slugs, so the minted slug stays unique. */ - readonly existingSlugs: readonly string[]; + /** The owner's existing apps (UNFILTERED). Used both to keep a minted slug + * unique across all apps AND to reuse a client already minted for this + * integration instead of minting a duplicate on every connect. */ + readonly existingClients: readonly OAuthClientOption[]; /** Scopes declared by the integration's method (override the probed ones). */ readonly declaredScopes?: readonly string[]; /** Browser-facing callback URL registered with DCR when available. */ @@ -504,7 +507,28 @@ export async function runDcrConnect( const registrationEndpoint = probe.registrationEndpoint; if (!registrationEndpoint) return { kind: "fallback", reason: "no-registration-endpoint" }; - const slug = uniqueClientSlug(input.integrationName, input.existingSlugs); + // Reuse a client already minted for THIS integration (same owner, same + // discovered token host) instead of minting a duplicate every connect — the + // cause of "Datadog", "Datadog 2", … "Datadog 10" piling up. A fresh probe of + // the same server yields the same endpoints, so an existing DCR client is + // still valid; we re-run only the authorize/token flow with it. + const probedTokenRoot = getRootDomain(probe.tokenUrl); + const reusable = input.existingClients.find( + (app) => + app.owner === input.owner && + app.origin?.integration != null && + String(app.origin.integration) === String(input.integration) && + getRootDomain(app.tokenUrl) === probedTokenRoot, + ); + if (reusable) { + deps.start({ client: reusable.slug, owner: input.owner }); + return { kind: "started" }; + } + + const slug = uniqueClientSlug( + input.integrationName, + input.existingClients.map((app) => String(app.slug)), + ); const scopes = input.declaredScopes && input.declaredScopes.length > 0 ? input.declaredScopes @@ -1145,9 +1169,7 @@ function AddAccountModalView(props: AddAccountModalProps) { discoveryUrl, owner: dcrOwner, integrationName, - existingSlugs: [...oauthApps, ...oauthOtherApps].map((app: OAuthClientOption) => - String(app.slug), - ), + existingClients: oauthAllClients, declaredScopes: method.oauth?.scopes, redirectUri: oauthCallbackUrl(), integration,