fix(oauth): route DCR reconnect through registration flow

.changeset/dcr-reconnect-through-registration.md

@@ -0,0 +1,9 @@
+---
+"@executor-js/react": patch
+---
+
+**Reconnecting a DCR connection now re-registers instead of reusing a stranded client**
+
+A dynamically registered OAuth client is bound to the redirect URI it registered with. Once the app's callback origin changed (127.0.0.1 to localhost), Reconnect still started the flow against the stored client, and the authorization server rejected it — leaving no way to repair the connection.
+
+Reconnect now takes the same probe → CIMD-or-register → start route as the initial connect, so the registration gateway replaces the stranded client against the current redirect URI. Methods with a fixed, hand-registered app are unaffected and keep using their stored client.

packages/core/sdk/src/client.ts

@@ -168,7 +168,18 @@ export interface IntegrationAccountHandoff {
     readonly clientId?: string;
     readonly authorizationUrl?: string;
     readonly tokenUrl?: string;
-    readonly resource?: string;
+    /** RFC 8707 resource indicator. On a reconnect handoff this is the STORED
+     *  client's value, and an EXPLICIT null means the stored client was
+     *  registered WITHOUT a resource indicator — that absence must survive a
+     *  re-registration (some servers reject any `resource` parameter).
+     *  Undefined means no stored value was carried. */
+    readonly resource?: string | null;
+    /** Reconnect only: the stored client binding is an auto-minted DCR client
+     *  (or its row is gone), so the modal may re-run the automatic
+     *  probe/registration flow. Absent or false pins the reconnect to the
+     *  stored client — a static/BYO or first-party binding must never be
+     *  silently rebound to an automatic client. */
+    readonly dynamicRegistration?: boolean;
   };
 }
 

packages/core/sdk/src/oauth-register-dynamic.test.ts

@@ -656,6 +656,56 @@ describe("oauth.registerDynamicClient", () => {
     ),
   );
 
+  // After the A→B drift recovery above, the owner holds TWO matching-resource
+  // clients: the stale one (bound to redirect A, oldest) and the recovery one
+  // (bound to redirect B). The reuse decision must prefer a candidate matching
+  // resource AND the current redirect across ALL candidates — taking only the
+  // OLDEST matching-resource candidate and then checking its redirect would
+  // mint yet another client on EVERY reconnect after the first drift.
+  it.effect(
+    "reuses the drift-recovery client on later reconnects instead of registering again",
+    () =>
+      Effect.scoped(
+        Effect.gen(function* () {
+          const server = yield* serveOAuthTestServer({ scopes: ["read"] });
+          const { executor } = yield* makeTestWorkspaceHarness({ plugins });
+          yield* executor.acme.seed();
+          const probe = yield* executor.oauth.probe({ url: server.mcpResourceUrl });
+
+          const registerAt = (slug: string, redirectUri: string) =>
+            executor.oauth.registerDynamicClient({
+              owner: "org",
+              slug: OAuthClientSlug.make(slug),
+              issuer: probe.issuer,
+              registrationEndpoint: probe.registrationEndpoint!,
+              authorizationUrl: probe.authorizationUrl,
+              tokenUrl: probe.tokenUrl,
+              resource: probe.resource,
+              scopes: ["read"],
+              tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported,
+              clientName: "Acme DCR",
+              redirectUri,
+              originIntegration: INTEG,
+            });
+
+          // Original sandbox at redirect A, then the drift recovery at redirect B.
+          yield* registerAt("original-sandbox", FLOW_REDIRECT_URI);
+          const driftedRedirectUri = "https://localhost:6410/api/oauth/callback";
+          const recovered = yield* registerAt("recreated-sandbox", driftedRedirectUri);
+          yield* server.clearRequests;
+
+          // A later reconnect at the SAME (current) redirect B: the recovery
+          // client already matches resource + redirect, so it is reused — no
+          // third registration, no third row.
+          const reused = yield* registerAt("later-reconnect", driftedRedirectUri);
+          expect(registerRequestCount(yield* server.requests)).toBe(0);
+          expect(String(reused)).toBe(String(recovered));
+          const clients = yield* executor.oauth.listClients();
+          expect(clients).toHaveLength(2);
+        }),
+      ),
+  );
+
   // Regression: Mercury's authorization server vets `client_name` and rejects
   // any value containing its own brand with `invalid_client_metadata`, which
   // the old auto-generated "Executor for Mercury MCP" always tripped. The UI

packages/core/sdk/src/oauth-service.ts

@@ -1144,9 +1144,22 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
       // token grant doesn't involve the redirect URI).
       const takenSlugs = new Set(candidates.map((client) => String(client.slug)));
       if (resource !== null) {
-        const matchingResource = candidates.find((client) => client.resource === resource);
-        if (matchingResource && redirectMatches(matchingResource)) {
-          return { existingSlug: matchingResource.slug, registrationSlug: matchingResource.slug };
+        // Prefer a candidate matching resource AND the current redirect across
+        // ALL candidates (mirroring the resource-less branch below). Candidates
+        // are oldest-first, so after an origin drift the oldest matching-
+        // resource row is the STRANDED one — but the first drift recovery
+        // already minted a client bound to the CURRENT callback, and later
+        // reconnects must reuse that instead of registering another duplicate
+        // each time. Known limitation: the legacy null-redirect rule in
+        // `redirectMatches` (a legacy row with no stored redirect matches any
+        // flow redirect) still lets such a row win over a later, exactly-
+        // matching one; kept deliberately so upgrades don't re-register every
+        // client whose callback never changed.
+        const reusable = candidates.find(
+          (client) => client.resource === resource && redirectMatches(client),
+        );
+        if (reusable) {
+          return { existingSlug: reusable.slug, registrationSlug: reusable.slug };
         }
         const slug = uniqueDcrSlug(
           dcrClientSlug(issuer, candidates.length > 0 ? resource : null, input.slug),

packages/react/src/components/accounts-section.tsx

@@ -1,14 +1,20 @@
 import { useEffect, useMemo, useState } from "react";
-import { useAtomValue, useAtomSet } from "@effect/atom-react";
+import { useAtomValue, useAtomRefresh, useAtomSet } from "@effect/atom-react";
 import * as AsyncResult from "effect/unstable/reactivity/AsyncResult";
 import * as Exit from "effect/Exit";
-import { IntegrationSlug, type Connection, type Owner } from "@executor-js/sdk/shared";
+import {
+  IntegrationSlug,
+  type Connection,
+  type OAuthClientSummary,
+  type Owner,
+} from "@executor-js/sdk/shared";
 import type { IntegrationAccountHandoff } from "@executor-js/sdk/client";
 import { toast } from "sonner";
 
 import {
   addConnectionOptimistic,
   connectionsForIntegrationAtom,
+  oauthClientsOptimisticAtom,
   refreshConnection,
   removeConnectionOptimistic,
   startOAuth,
@@ -23,11 +29,14 @@ import type { AuthMethod } from "../lib/auth-placements";
 import {
   connectionNeedsReconsent,
   oauthReconnectPayload,
+  reconnectClientsView,
   reconnectMode,
+  reconnectRoute,
   reconsentRequiredScopes,
+  retryReconnectClientsOnMenuOpen,
 } from "../plugins/oauth-reconnect";
 import { useOAuthPopupFlow } from "../plugins/oauth-sign-in";
-import { AddAccountModal } from "./add-account-modal";
+import { AddAccountModal, hasDcr } from "./add-account-modal";
 import { ConnectionEditSheet } from "./metadata-edit-sheet";
 import type { CreateCustomMethod } from "./add-custom-method-modal";
 import {
@@ -117,6 +126,19 @@ function AccountRow(props: {
   readonly showOwnerLabel: boolean;
   readonly onEdit: () => void;
   readonly onReconnect: () => void;
+  /** Reconnect routing needs the stored client binding; while the client
+   *  summaries carry no data the route is unknown, so the action is
+   *  disabled rather than guessed (same idiom as "Check now" above). */
+  readonly reconnectDisabled: boolean;
+  /** The summaries query failed: the Reconnect item stays disabled but says
+   *  so (never a silently dead action). Retained stale data never routes — a
+   *  binding changed since the snapshot could misroute. Opening the menu
+   *  retries the query via `onMenuOpenChange`, so the hint reflects a retry
+   *  that just failed, not a permanently stuck state. */
+  readonly reconnectFailed: boolean;
+  /** Forwarded to the row menu; the owner uses the OPEN transition to retry
+   *  a failed client-summaries query. */
+  readonly onMenuOpenChange: (open: boolean) => void;
   readonly onRemove: () => void;
 }) {
   const { connection, needsReconsent } = props;
@@ -245,7 +267,7 @@ function AccountRow(props: {
         {props.showOwnerLabel ? (
           <Badge variant="outline">{ownerLabel(connection.owner)}</Badge>
         ) : null}
-        <DropdownMenu>
+        <DropdownMenu onOpenChange={props.onMenuOpenChange}>
           <DropdownMenuTrigger asChild>
             <Button
               variant="ghost"
@@ -270,8 +292,17 @@ function AccountRow(props: {
             <DropdownMenuItem className="text-sm" onClick={props.onEdit}>
               Edit
             </DropdownMenuItem>
-            <DropdownMenuItem className="text-sm" onClick={props.onReconnect}>
+            <DropdownMenuItem
+              className="text-sm"
+              disabled={props.reconnectDisabled}
+              onClick={props.onReconnect}
+            >
               Reconnect
+              {props.reconnectFailed ? (
+                // Same failed-query voice as the modal's picker errors; the
+                // trailing placement mirrors DropdownMenuShortcut.
+                <span className="ml-auto text-xs text-destructive">Failed to load</span>
+              ) : null}
             </DropdownMenuItem>
             <DropdownMenuItem variant="destructive" className="text-sm" onClick={props.onRemove}>
               Remove
@@ -289,13 +320,29 @@ function OwnerAccounts(props: {
   readonly showOwnerLabels: boolean;
   readonly methods: readonly AuthMethod[];
   readonly onEdit: (connection: Connection) => void;
-  readonly onDcrReconnect: (connection: Connection) => void;
+  /** Hand the connection to the modal's automatic reconnect flow. Only called
+   *  once the stored binding was vetted as auto-minted DCR (or gone);
+   *  `storedClient` is that binding's summary — undefined when the row is
+   *  gone — so the handoff can carry its resource. */
+  readonly onDcrReconnect: (
+    connection: Connection,
+    storedClient: OAuthClientSummary | undefined,
+  ) => void;
   /** The integration's declared oauth scopes — compared against each connection's
    *  granted `oauthScope` to flag connections that must reconnect for new access. */
   readonly declaredScopes: readonly string[] | undefined;
 }) {
   const { integration, owner } = props;
   const connections = useAtomValue(connectionsForIntegrationAtom({ integration, owner }));
+  // Registered-app summaries: the Reconnect routing below inspects the STORED
+  // client binding (its origin kind and resource), not just the method's
+  // capability, before sending a connection into the automatic flow. Routing
+  // only ever reads a current successful load — never stale data — and a
+  // failed query marks the action failed (recoverable — opening the row menu
+  // retries the query).
+  const allClients = useAtomValue(oauthClientsOptimisticAtom);
+  const refreshClients = useAtomRefresh(oauthClientsOptimisticAtom);
+  const clientsView = reconnectClientsView(allClients);
   // Removal confirms in a dialog. State lives here (not in the row) because the
   // Remove menu item closes its dropdown on click, which would unmount a dialog
   // nested inside it — so the row only nominates the connection to remove.
@@ -327,11 +374,26 @@ function OwnerAccounts(props: {
         (candidate: AuthMethod) =>
           candidate.kind === "oauth" && String(candidate.template) === String(connection.template),
       );
-      if (
-        method?.oauth?.supportsDynamicRegistration === true ||
-        method?.oauth?.discoveryUrl != null
-      ) {
-        props.onDcrReconnect(connection);
+      // Route by the STORED binding's origin (`reconnectRoute`): an
+      // auto-minted DCR binding re-runs the automatic probe/registration flow
+      // (direct reuse dead-ends once the callback origin drifts, #1542); a
+      // static/BYO or first-party binding takes the direct path below even on
+      // a discovery-capable integration — re-registering would silently
+      // rebind the connection to an automatic client. Unless the client list
+      // is a CURRENT success the binding is UNKNOWN and no route may be
+      // chosen: the menu item is disabled until then, and this guard
+      // backstops a race — a permanent wrong choice on a guess is never
+      // acceptable. A failed refresh never routes, even when stale data is
+      // retained: a binding changed since the snapshot could repeat the
+      // origin-drift dead end or silently rebind to an automatic client.
+      const route = reconnectRoute(
+        clientsView.kind === "ready" ? clientsView.clients : undefined,
+        connection,
+        hasDcr(method),
+      );
+      if (route.kind === "unknown") return;
+      if (route.kind === "automatic") {
+        props.onDcrReconnect(connection, route.stored);
         return;
       }
       const payload = oauthReconnectPayload(connection);
@@ -451,6 +513,19 @@ function OwnerAccounts(props: {
             showOwnerLabel={props.showOwnerLabels}
             onEdit={() => props.onEdit(connection)}
             onReconnect={() => void handleReconnect(connection)}
+            // An OAuth Reconnect routes by the stored client binding; without
+            // a current successful load (loading or failed) the route is
+            // unknown, so the action waits. Static-credential rows refresh
+            // without the binding.
+            reconnectDisabled={
+              reconnectMode(connection) === "oauth" && clientsView.kind !== "ready"
+            }
+            // A failed load is surfaced on the item (not silently disabled)
+            // and recovers on menu open, which retries the query.
+            reconnectFailed={reconnectMode(connection) === "oauth" && clientsView.kind === "failed"}
+            onMenuOpenChange={(open: boolean) =>
+              retryReconnectClientsOnMenuOpen(open, clientsView, refreshClients)
+            }
             onRemove={() => setRemovingConnection(connection)}
           />
         ))}
@@ -632,7 +707,10 @@ export function AccountsSection(props: {
               showOwnerLabels={ownerDisplay.showOwnerLabels}
               methods={methods}
               onEdit={setEditingConnection}
-              onDcrReconnect={(connection: Connection) => {
+              onDcrReconnect={(
+                connection: Connection,
+                storedClient: OAuthClientSummary | undefined,
+              ) => {
                 if (connection.oauthClient == null) return;
                 setReconnectHandoff({
                   key: `reconnect:${connection.owner}:${String(connection.integration)}:${String(
@@ -648,6 +726,19 @@ export function AccountsSection(props: {
                     action: "reconnect",
                     slug: String(connection.oauthClient),
                     owner: connection.oauthClientOwner ?? connection.owner,
+                    // Vetted by the routing in `handleReconnect`: the stored
+                    // binding is auto-minted DCR (or its row is gone), so the
+                    // modal may re-run the automatic flow.
+                    dynamicRegistration: true,
+                    // The stored client's RFC 8707 resource — an EXPLICIT null
+                    // for a client registered WITHOUT a resource indicator
+                    // (that absence always survives re-registration); a stored
+                    // value is reconciled against the probe downstream, so a
+                    // migrated resource follows the server. Omitted when the
+                    // row is gone.
+                    ...(storedClient !== undefined
+                      ? { resource: storedClient.resource ?? null }
+                      : {}),
                   },
                 });
               }}