History

Render the two-tier picker and an auto-registered clients section

Wire the modal to the new grouping: tier-1 exact/intent matches stay the default-selectable list, root-domain near-misses render in a visually separated, subdued "Other apps on this provider" group (selectable but never auto-selected), and the auto-registered DCR clients for this integration live in a collapsed "Auto-registered clients" section at the bottom, deletable but not pickable. Resolving the chosen client now spans every pickable tier so the near-miss and escape-hatch apps actually work. Also drop the DCR connect path's dependency on the picker's slug list: the slug is deterministic per authorization server since Part A, so runDcrConnect no longer needs existingSlugs threaded in.

Rhys Sullivan committed 4 months ago

Commit 9afe2ec1 parent 149f9092 files · +172 −12Browse files

Unified · Split

2 files
  1. packages/react/src/components/add-account-modal.test.ts +33 −0
  2. packages/react/src/components/add-account-modal.tsx +139 −12
packages/react/src/components/add-account-modal.test.ts+33−0
@@ -575,4 +575,37 @@ describe("runDcrConnect", () => { · 574 unmodified lines
575575 });
576576 expect(started).toBe(false);
577577 });
578+
579+ it("mints a deterministic server-keyed slug WITHOUT the picker's slug list", async () => {
580+ // Part B removed the DCR connect path's dependency on the picker's app list:
581+ // the slug is derived from the authorization server (Part A), so callers no
582+ // longer thread `existingSlugs`. This omits it entirely and still succeeds.
583+ let registerArgs: RegisterArgs | null = null;
584+ const outcome = await runDcrConnect(
585+ {
586+ probe: (): Promise<ProbeResult | null> =>
587+ Promise.resolve({
588+ issuer: "https://auth.example.com",
589+ authorizationUrl: "https://auth.example.com/authorize",
590+ tokenUrl: "https://auth.example.com/token",
591+ registrationEndpoint: "https://auth.example.com/register",
592+ }),
593+ register: (args: RegisterArgs): Promise<OAuthClientSlug | null> => {
594+ registerArgs = args;
595+ return Promise.resolve(OAuthClientSlug.make("app"));
596+ },
597+ start: (): void => {},
598+ },
599+ {
600+ discoveryUrl: "https://mcp.example.com/mcp",
601+ owner: "user",
602+ integrationName: "App",
603+ integration: TEST_INTEGRATION,
604+ },
605+ );
606+ expect(outcome.kind).toBe("started");
607+ expect(registerArgs).not.toBeNull();
608+ // Slug comes from the issuer host, independent of any picker state.
609+ expect(String(registerArgs!.slug)).toBe("dcr-auth-example-com");
610+ });
578611 });
packages/react/src/components/add-account-modal.tsx+139−12
@@ -48,6 +48,7 @@ import { · 47 unmodified lines
4848 clientDisplayName,
4949 clientHost,
5050 dcrClientSlug,
51+ selectDcrClientsForIntegration,
5152 uniqueClientSlug,
5253 useOAuthClientsForIntegration,
5354 type OAuthClientOption,
@@ -687,8 +688,10 @@ type RunDcrConnectInput = { · 633 unmodified lines
687688 readonly resourceFallback?: string;
688689 readonly owner: Owner;
689690 readonly integrationName: string;
690− /** Kept for callers that still pass picker state. DCR slugs are server keyed. */
691− readonly existingSlugs: readonly string[];
691+ /** @deprecated DCR slugs are deterministic per authorization server (Part A),
692+ * so the connect path no longer needs the picker's slug list. Accepted but
693+ * ignored; retained only so existing test callers keep compiling. */
694+ readonly existingSlugs?: readonly string[];
692695 /** Scopes declared by the integration's method (override the probed ones). */
693696 readonly declaredScopes?: readonly string[];
694697 /** Browser-facing callback URL registered with DCR when available. */
@@ -902,6 +905,10 @@ function AddAccountModalView(props: AddAccountModalProps) { · 207 unmodified lines
902905 // FIX 3 escape hatch: when no registered app matched the integration's
903906 // endpoints, the unmatched apps are collapsed behind an opt-in expander.
904907 const [showOtherApps, setShowOtherApps] = useState(false);
908+ // Auto-registered (DCR) clients are plumbing, not pickable apps: they live in a
909+ // collapsed management section at the bottom of the OAuth tab, revealed on
910+ // demand so they can be inspected and deleted without cluttering the picker.
911+ const [showAutoRegistered, setShowAutoRegistered] = useState(false);
905912 // Inline OAuth app management — edit re-opens the registration form for an
906913 // existing app (upsert by owner+slug); remove confirms before deleting. Both
907914 // hold the FULL app summary (with endpoints + resource) so the edit prefill
@@ -1080,6 +1087,7 @@ function AddAccountModalView(props: AddAccountModalProps) { · 172 unmodified lines
10801087 // unconditionally; in DCR mode the result is ignored until/unless we fall back.
10811088 const {
10821089 clients: oauthApps,
1090+ nearMatches: oauthNearApps,
10831091 otherClients: oauthOtherApps,
10841092 loading: oauthLoading,
10851093 endpointMatched: oauthEndpointMatched,
@@ -1092,8 +1100,29 @@ function AddAccountModalView(props: AddAccountModalProps) { · 6 unmodified lines
10921100 // unrelated provider's app.
10931101 tokenUrl: method?.oauth?.tokenUrl ?? oauthFallbackProbe?.tokenUrl,
10941102 authorizationUrl: method?.oauth?.authorizationUrl ?? oauthFallbackProbe?.authorizationUrl,
1103+ // Recorded intent: a manual app registered from THIS integration's dialog is
1104+ // a tier-1 match regardless of host.
1105+ integration,
10951106 requireEndpointMatch: isDcr,
10961107 });
1108+ // Auto-registered (DCR) clients for THIS integration. Excluded from the picker
1109+ // above, but surfaced in a collapsed management section so they stay
1110+ // deletable. Reads the same optimistic list the picker does.
1111+ const dcrClients = useMemo(
1112+ () =>
1113+ selectDcrClientsForIntegration(clientSummaries as readonly OAuthClientOption[], {
1114+ integration,
1115+ tokenUrl: method?.oauth?.tokenUrl ?? oauthFallbackProbe?.tokenUrl,
1116+ authorizationUrl: method?.oauth?.authorizationUrl ?? oauthFallbackProbe?.authorizationUrl,
1117+ }),
1118+ [
1119+ clientSummaries,
1120+ integration,
1121+ method?.oauth?.tokenUrl,
1122+ method?.oauth?.authorizationUrl,
1123+ oauthFallbackProbe,
1124+ ],
1125+ );
10971126 const oauthPopup = useOAuthPopupFlow({
10981127 popupName: "add-account-oauth",
10991128 detectPopupClosed: false,
@@ -1110,8 +1139,15 @@ function AddAccountModalView(props: AddAccountModalProps) { · 10 unmodified lines
11101139 // Editing reuses the registration form (createClient upserts by owner+slug),
11111140 // so it occupies the same full-bleed sub-view as registering.
11121141 const oauthEditing = isOAuth && editingClient !== null;
1142+ // Resolve the picked slug across every PICKABLE tier: exact matches, the
1143+ // subdued near-match tier, and the unrelated escape-hatch apps. All three are
1144+ // selectable (only the default auto-selection is restricted to tier 1), so the
1145+ // connect button must resolve a client from any of them. DCR clients are not
1146+ // in these lists, so they can never be chosen as the connect app.
11131147 const chosenClient: OAuthClientOption | null =
1114− oauthApps.find((c: OAuthClientOption) => String(c.slug) === selectedApp) ?? null;
1148+ [...oauthApps, ...oauthNearApps, ...oauthOtherApps].find(
1149+ (c: OAuthClientOption) => String(c.slug) === selectedApp,
1150+ ) ?? null;
11151151 const oauthBusy = ccBusy || oauthPopup.busy;
11161152 const cimdConnecting = cimdBusy || oauthPopup.busy;
11171153 const dcrConnecting = dcrBusy || oauthPopup.busy;
@@ -1496,9 +1532,8 @@ function AddAccountModalView(props: AddAccountModalProps) { · 378 unmodified lines
14961532 resourceFallback: method.oauth?.discoveryUrl,
14971533 owner: dcrOwner,
14981534 integrationName,
1499− existingSlugs: [...oauthApps, ...oauthOtherApps].map((app: OAuthClientOption) =>
1500− String(app.slug),
1501− ),
1535+ // DCR slugs are server-keyed (Part A): the connect path no longer depends
1536+ // on the picker's app list, so it need not be threaded here.
15021537 declaredScopes: method.oauth?.scopes,
15031538 redirectUri: oauthCallbackUrl(),
15041539 integration,
@@ -1558,9 +1593,7 @@ function AddAccountModalView(props: AddAccountModalProps) { · 53 unmodified lines
15581593 <div className="px-5 py-5">
15591594 <OAuthClientForm
15601595 integrationName={integrationName}
1561− existingSlugs={[...oauthApps, ...oauthOtherApps].map((app: OAuthClientOption) =>
1562− String(app.slug),
1563− )}
1596+ existingSlugs={clientSummaries.map((c: OAuthClientSummary) => String(c.slug))}
15641597 fixedSlug={editingClient.slug}
15651598 fixedOwner={editingClient.owner}
15661599 prefill={{
@@ -1587,9 +1620,8 @@ function AddAccountModalView(props: AddAccountModalProps) { · 20 unmodified lines
15871620 <div className="px-5 py-5">
15881621 <OAuthClientForm
15891622 integrationName={integrationName}
1590− existingSlugs={[...oauthApps, ...oauthOtherApps].map((app: OAuthClientOption) =>
1591− String(app.slug),
1592− )}
1623+ integrationSlug={integration}
1624+ existingSlugs={clientSummaries.map((c: OAuthClientSummary) => String(c.slug))}
15931625 autoRegisterRejectedReason={dcrFallbackMessage}
15941626 prefill={{
15951627 authorizationUrl:
@@ -1881,6 +1913,101 @@ function AddAccountModalView(props: AddAccountModalProps) { · 285 unmodified lines
18811913 </Button>
18821914 </RadioGroup>
18831915 )}
1916+
1917+ {/* Tier 2 — apps that only match by root domain (a
1918+ near-miss, e.g. the same provider's MCP app for a REST
1919+ integration). Visually separated and subdued so they are
1920+ never mistaken for the integration's own app, but still
1921+ selectable as an escape hatch. Shown alongside tier 1, not
1922+ mixed into it. */}
1923+ {oauthNearApps.length > 0 && (
1924+ <div className="space-y-2 border-t border-border/60 pt-3">
1925+ <p className="text-xs font-medium text-muted-foreground">
1926+ Other apps on this provider
1927+ </p>
1928+ <p className="text-xs text-muted-foreground">
1929+ These share {integrationName}&apos;s domain but weren&apos;t
1930+ registered for it. Pick one only if you know it applies.
1931+ </p>
1932+ <RadioGroup
1933+ value={selectedApp}
1934+ onValueChange={setPickedApp}
1935+ className="gap-2 pt-1"
1936+ >
1937+ {oauthNearApps.map((app: OAuthClientOption) => (
1938+ <OAuthAppRadioRow
1939+ key={String(app.slug)}
1940+ app={app}
1941+ idPrefix="near-app"
1942+ variant="other"
1943+ showOwnerLabel={ownerDisplay.showOwnerLabels}
1944+ onManage={manageHandlersFor(app)}
1945+ />
1946+ ))}
1947+ </RadioGroup>
1948+ </div>
1949+ )}
1950+
1951+ {/* Auto-registered (DCR) clients: plumbing, never
1952+ pickable. Kept here, collapsed, so they can be reviewed
1953+ and deleted without cluttering the picker. */}
1954+ {dcrClients.length > 0 && (
1955+ <div className="border-t border-border/60 pt-3">
1956+ <button
1957+ type="button"
1958+ className="flex w-full items-center justify-between text-xs font-medium text-muted-foreground hover:text-foreground"
1959+ onClick={() => setShowAutoRegistered((prev) => !prev)}
1960+ aria-expanded={showAutoRegistered}
1961+ >
1962+ <span>Auto-registered clients ({dcrClients.length})</span>
1963+ <span aria-hidden>{showAutoRegistered ? "−" : "+"}</span>
1964+ </button>
1965+ {showAutoRegistered ? (
1966+ <div className="space-y-2 pt-2">
1967+ <p className="text-xs text-muted-foreground">
1968+ Created automatically when connecting. Reused across
1969+ connections, not selectable as your app.
1970+ </p>
1971+ {dcrClients.map((app: OAuthClientOption) => (
1972+ <div
1973+ key={String(app.slug)}
1974+ className="flex items-center gap-3 rounded-lg border border-border/60 bg-background/40 px-3 py-2.5"
1975+ >
1976+ <span className="min-w-0 flex-1">
1977+ <span className="block text-sm font-medium">
1978+ {clientDisplayName(String(app.slug))}
1979+ </span>
1980+ <span className="block truncate text-xs text-muted-foreground">
1981+ {clientHost(app.tokenUrl)}
1982+ </span>
1983+ </span>
1984+ {ownerDisplay.showOwnerLabels ? (
1985+ <Badge variant="outline">
1986+ {ownerLabel(app.owner)}
1987+ </Badge>
1988+ ) : null}
1989+ <Button
1990+ type="button"
1991+ variant="ghost"
1992+ size="sm"
1993+ className="shrink-0 text-destructive hover:text-destructive"
1994+ onClick={() => {
1995+ const summary = clientSummaries.find(
1996+ (c: OAuthClientSummary) =>
1997+ c.owner === app.owner &&
1998+ String(c.slug) === String(app.slug),
1999+ );
2000+ if (summary) setRemovingClient(summary);
2001+ }}
2002+ >
2003+ Remove
2004+ </Button>
2005+ </div>
2006+ ))}
2007+ </div>
2008+ ) : null}
2009+ </div>
2010+ )}
18842011 </div>
18852012 )
18862013 ) : (

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t