From d9eb79fd568c6a1346678ccbf8be01a42d3146c7 Mon Sep 17 00:00:00 2001 From: Steven Enamakel <31011319+senamakel@users.noreply.github.com> Date: Mon, 8 Jun 2026 10:19:11 -0400 Subject: [PATCH] feat(mcp-registry): rich install screen and table UI redesign (#3492) --- .../channels/mcp/InstallDialog.test.tsx | 131 ++++++-- .../components/channels/mcp/InstallDialog.tsx | 315 ++++++++++++++---- .../channels/mcp/McpCatalogBrowser.test.tsx | 2 +- .../channels/mcp/McpCatalogBrowser.tsx | 2 +- .../components/channels/mcp/McpServerCard.tsx | 83 ++--- .../channels/mcp/McpServersTab.test.tsx | 28 +- .../components/channels/mcp/McpServersTab.tsx | 74 ++-- app/src/lib/i18n/ar.ts | 13 + app/src/lib/i18n/bn.ts | 13 + app/src/lib/i18n/de.ts | 13 + app/src/lib/i18n/en.ts | 13 + app/src/lib/i18n/es.ts | 13 + app/src/lib/i18n/fr.ts | 13 + app/src/lib/i18n/hi.ts | 13 + app/src/lib/i18n/id.ts | 13 + app/src/lib/i18n/it.ts | 13 + app/src/lib/i18n/ko.ts | 13 + app/src/lib/i18n/pl.ts | 13 + app/src/lib/i18n/pt.ts | 13 + app/src/lib/i18n/ru.ts | 13 + app/src/lib/i18n/zh-CN.ts | 13 + .../__tests__/Skills.mcp-coming-soon.test.tsx | 2 +- .../playwright/specs/mcp-tab-flow.spec.ts | 73 ++-- .../mcp_registry/registries/mcp_official.rs | 167 ++++++++++ src/openhuman/mcp_registry/types.rs | 170 +++++++++- 25 files changed, 1000 insertions(+), 229 deletions(-) diff --git a/app/src/components/channels/mcp/InstallDialog.test.tsx b/app/src/components/channels/mcp/InstallDialog.test.tsx index 27c429063..768f5ad13 100644 --- a/app/src/components/channels/mcp/InstallDialog.test.tsx +++ b/app/src/components/channels/mcp/InstallDialog.test.tsx @@ -19,10 +19,23 @@ const DETAIL = { qualified_name: 'acme/test-server', display_name: 'Test Server', description: 'A test server', - connections: [], + connections: [{ type: 'stdio', published: true }], required_env_keys: ['API_KEY', 'SECRET_TOKEN'], }; +const DETAIL_NO_ENV = { + qualified_name: 'acme/simple-server', + display_name: 'Simple Server', + description: 'No env needed', + connections: [{ type: 'stdio', published: true }], + required_env_keys: [], +}; + +async function goToConfigureStep() { + await waitFor(() => screen.getByRole('button', { name: 'Configure & install' })); + fireEvent.click(screen.getByRole('button', { name: 'Configure & install' })); +} + describe('InstallDialog', () => { beforeEach(() => { mockRegistryGet.mockReset(); @@ -32,7 +45,6 @@ describe('InstallDialog', () => { }); it('shows loading state while fetching detail', () => { - // Never resolves within the test mockRegistryGet.mockReturnValue(new Promise(() => {})); render( {}} onCancel={() => {}} /> @@ -40,15 +52,38 @@ describe('InstallDialog', () => { expect(screen.getByText('Loading server details...')).toBeInTheDocument(); }); - it('renders env key inputs from registry_get', async () => { + it('renders detail overview with server info', async () => { mockRegistryGet.mockResolvedValue(DETAIL); render( {}} onCancel={() => {}} /> ); await waitFor(() => { - expect(screen.getByLabelText('API_KEY')).toBeInTheDocument(); + expect(screen.getByText('Test Server')).toBeInTheDocument(); }); + expect(screen.getByText('A test server')).toBeInTheDocument(); + expect(screen.getByText('Requires configuration')).toBeInTheDocument(); + expect(screen.getByText('Runs locally')).toBeInTheDocument(); + }); + + it('shows env key preview badges on detail step', async () => { + mockRegistryGet.mockResolvedValue(DETAIL); + render( + {}} onCancel={() => {}} /> + ); + + await waitFor(() => screen.getByText('API_KEY')); + expect(screen.getByText('SECRET_TOKEN')).toBeInTheDocument(); + }); + + it('renders env key inputs after clicking configure', async () => { + mockRegistryGet.mockResolvedValue(DETAIL); + render( + {}} onCancel={() => {}} /> + ); + + await goToConfigureStep(); + expect(screen.getByLabelText('API_KEY')).toBeInTheDocument(); expect(screen.getByLabelText('SECRET_TOKEN')).toBeInTheDocument(); }); @@ -58,8 +93,7 @@ describe('InstallDialog', () => { {}} onCancel={() => {}} /> ); - await waitFor(() => screen.getByLabelText('API_KEY')); - + await goToConfigureStep(); const input = screen.getByLabelText('API_KEY') as HTMLInputElement; expect(input.type).toBe('password'); }); @@ -70,11 +104,9 @@ describe('InstallDialog', () => { {}} onCancel={() => {}} /> ); - await waitFor(() => screen.getByLabelText('API_KEY')); - + await goToConfigureStep(); const showButtons = screen.getAllByRole('button', { name: 'Show' }); fireEvent.click(showButtons[0]); - const input = screen.getByLabelText('API_KEY') as HTMLInputElement; expect(input.type).toBe('text'); }); @@ -97,8 +129,7 @@ describe('InstallDialog', () => { {}} /> ); - await waitFor(() => screen.getByLabelText('API_KEY')); - + await goToConfigureStep(); fireEvent.change(screen.getByLabelText('API_KEY'), { target: { value: 'my-api-key' } }); fireEvent.change(screen.getByLabelText('SECRET_TOKEN'), { target: { value: 'my-secret' } }); @@ -111,7 +142,6 @@ describe('InstallDialog', () => { env: { API_KEY: 'my-api-key', SECRET_TOKEN: 'my-secret' }, config: undefined, }); - // Auto-connect on success (issue #3039 gap B3). expect(mockConnect).toHaveBeenCalledWith('srv-1'); expect(onSuccess).toHaveBeenCalledWith(installedServer); }); @@ -135,7 +165,7 @@ describe('InstallDialog', () => { {}} /> ); - await waitFor(() => screen.getByLabelText('API_KEY')); + await goToConfigureStep(); fireEvent.change(screen.getByLabelText('API_KEY'), { target: { value: 'k' } }); fireEvent.change(screen.getByLabelText('SECRET_TOKEN'), { target: { value: 's' } }); @@ -144,7 +174,6 @@ describe('InstallDialog', () => { }); expect(mockConnect).toHaveBeenCalledWith('srv-1'); - // A connect failure must NOT block the install success callback. expect(onSuccess).toHaveBeenCalledWith(installedServer); }); @@ -154,9 +183,7 @@ describe('InstallDialog', () => { {}} onCancel={() => {}} /> ); - await waitFor(() => screen.getByLabelText('API_KEY')); - - // Leave API_KEY empty, fill only SECRET_TOKEN + await goToConfigureStep(); fireEvent.change(screen.getByLabelText('SECRET_TOKEN'), { target: { value: 'secret' } }); await act(async () => { @@ -175,8 +202,7 @@ describe('InstallDialog', () => { {}} onCancel={() => {}} /> ); - await waitFor(() => screen.getByLabelText('API_KEY')); - + await goToConfigureStep(); fireEvent.change(screen.getByLabelText('API_KEY'), { target: { value: 'key' } }); fireEvent.change(screen.getByLabelText('SECRET_TOKEN'), { target: { value: 'secret' } }); @@ -187,7 +213,7 @@ describe('InstallDialog', () => { await waitFor(() => screen.getByText('Server error')); }); - it('calls onCancel when Cancel is clicked', async () => { + it('calls onCancel when Cancel is clicked on detail step', async () => { mockRegistryGet.mockResolvedValue(DETAIL); const onCancel = vi.fn(); render( @@ -210,8 +236,71 @@ describe('InstallDialog', () => { /> ); - await waitFor(() => screen.getByLabelText('API_KEY')); + await goToConfigureStep(); const input = screen.getByLabelText('API_KEY') as HTMLInputElement; expect(input.value).toBe('prefilled-key'); }); + + it('installs directly from detail step when no env keys required', async () => { + const installedServer = { + server_id: 'srv-2', + ...DETAIL_NO_ENV, + command_kind: 'node' as const, + command: 'node', + args: [], + env_keys: [], + installed_at: 2000, + }; + mockRegistryGet.mockResolvedValue(DETAIL_NO_ENV); + mockInstall.mockResolvedValue(installedServer); + + const onSuccess = vi.fn(); + render( + {}} /> + ); + + await waitFor(() => screen.getByRole('button', { name: 'Install' })); + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: 'Install' })); + }); + + expect(mockInstall).toHaveBeenCalledWith({ + qualified_name: 'acme/simple-server', + env: {}, + config: undefined, + }); + expect(onSuccess).toHaveBeenCalledWith(installedServer); + }); + + it('shows connection info on detail step', async () => { + mockRegistryGet.mockResolvedValue({ + ...DETAIL, + connections: [ + { type: 'stdio', published: true }, + { type: 'http', published: false, deployment_url: 'https://example.com/mcp' }, + ], + }); + render( + {}} onCancel={() => {}} /> + ); + + await waitFor(() => screen.getByText('Available connections')); + expect(screen.getByText('stdio')).toBeInTheDocument(); + expect(screen.getByText('http')).toBeInTheDocument(); + }); + + it('navigates back from configure to detail step', async () => { + mockRegistryGet.mockResolvedValue(DETAIL); + render( + {}} onCancel={() => {}} /> + ); + + await goToConfigureStep(); + expect(screen.getByLabelText('API_KEY')).toBeInTheDocument(); + + fireEvent.click(screen.getByText(`← Test Server`)); + await waitFor(() => { + expect(screen.getByText('A test server')).toBeInTheDocument(); + }); + }); }); diff --git a/app/src/components/channels/mcp/InstallDialog.tsx b/app/src/components/channels/mcp/InstallDialog.tsx index ae2162c32..962b1c8fe 100644 --- a/app/src/components/channels/mcp/InstallDialog.tsx +++ b/app/src/components/channels/mcp/InstallDialog.tsx @@ -1,18 +1,27 @@ /** - * Install dialog for an MCP server. - * Fetches the server detail, renders env-key inputs (password type with - * show/hide toggle), an optional raw-JSON config textarea, and calls - * `install` on submit. + * Rich install screen for an MCP server. + * + * Two-step flow: + * 1. **Detail** — server overview (icon, title, author, description, stats, + * transport type). If no env vars are required, a single "Install" button + * kicks off the install directly. + * 2. **Configure** — env var inputs + optional raw JSON config, then install. + * + * Uses `mcpClientsApi.registryGet` for detail, `mcpClientsApi.install` for + * the install itself, and best-effort `mcpClientsApi.connect` post-install. */ import debug from 'debug'; import { useCallback, useEffect, useRef, useState } from 'react'; import { useT } from '../../../lib/i18n/I18nContext'; import { mcpClientsApi } from '../../../services/api/mcpClientsApi'; -import type { InstalledServer, SmitheryServerDetail } from './types'; +import { deriveAuthor } from './McpServerCard'; +import type { InstalledServer, SmitheryConnection, SmitheryServerDetail } from './types'; const log = debug('mcp-clients:install'); +type Step = 'detail' | 'configure'; + interface InstallDialogProps { qualifiedName: string; prefillEnv?: Record; @@ -20,24 +29,37 @@ interface InstallDialogProps { onCancel: () => void; } +function pickTransportLabel(connections: SmitheryConnection[]): string | null { + const types = new Set(connections.map(c => c.type)); + if (types.has('stdio')) return 'stdio'; + if (types.has('http')) return 'http'; + return connections[0]?.type ?? null; +} + +function formatUseCount(count: number): string { + if (count >= 1_000_000) return `${(count / 1_000_000).toFixed(1)}M`; + if (count >= 1_000) return `${(count / 1_000).toFixed(1)}k`; + return String(count); +} + const InstallDialog = ({ qualifiedName, prefillEnv, onSuccess, onCancel }: InstallDialogProps) => { const { t } = useT(); + const [detail, setDetail] = useState(null); const [loadingDetail, setLoadingDetail] = useState(true); const [detailError, setDetailError] = useState(null); + const latestQualifiedNameRef = useRef(qualifiedName); + + const [step, setStep] = useState('detail'); const [envValues, setEnvValues] = useState>({}); const [showEnv, setShowEnv] = useState>({}); const [configJson, setConfigJson] = useState(''); + const [showAdvanced, setShowAdvanced] = useState(false); const [installing, setInstalling] = useState(false); const [installError, setInstallError] = useState(null); - // Track the latest qualifiedName seen by the effect to guard against stale - // async responses when qualifiedName changes or the component unmounts. - const latestQualifiedNameRef = useRef(qualifiedName); - - // Fetch server detail on mount or when qualifiedName changes. useEffect(() => { latestQualifiedNameRef.current = qualifiedName; setLoadingDetail(true); @@ -47,13 +69,8 @@ const InstallDialog = ({ qualifiedName, prefillEnv, onSuccess, onCancel }: Insta mcpClientsApi .registryGet(qualifiedName) .then(d => { - // Discard response if a newer request has already been issued. - if (latestQualifiedNameRef.current !== requestedName) { - log('discarding stale detail response for %s', requestedName); - return; - } + if (latestQualifiedNameRef.current !== requestedName) return; setDetail(d); - // Pre-fill env values from prop (suggested by config assistant) or empty. const initial: Record = {}; for (const key of d.required_env_keys ?? []) { initial[key] = prefillEnv?.[key] ?? ''; @@ -74,6 +91,8 @@ const InstallDialog = ({ qualifiedName, prefillEnv, onSuccess, onCancel }: Insta }); }, [qualifiedName, prefillEnv, t]); + const hasEnvKeys = (detail?.required_env_keys ?? []).length > 0; + const toggleShowEnv = useCallback((key: string) => { setShowEnv(prev => ({ ...prev, [key]: !prev[key] })); }, []); @@ -85,7 +104,6 @@ const InstallDialog = ({ qualifiedName, prefillEnv, onSuccess, onCancel }: Insta const handleInstall = useCallback(async () => { if (!detail) return; - // Validate required keys are filled. for (const key of detail.required_env_keys ?? []) { if (!envValues[key]?.trim()) { setInstallError(t('mcp.install.missingRequired').replace('{key}', key)); @@ -93,7 +111,6 @@ const InstallDialog = ({ qualifiedName, prefillEnv, onSuccess, onCancel }: Insta } } - // Parse optional JSON config. let parsedConfig: unknown = undefined; if (configJson.trim()) { try { @@ -115,10 +132,6 @@ const InstallDialog = ({ qualifiedName, prefillEnv, onSuccess, onCancel }: Insta config: parsedConfig, }); log('install success server_id=%s', server.server_id); - // Best-effort auto-connect — fire-and-forget so the dialog closes - // immediately on install success. A slow or failing handshake must - // never block onSuccess (the detail view surfaces errors and offers - // a Connect retry). void mcpClientsApi .connect(server.server_id) .then(() => log('auto-connect success server_id=%s', server.server_id)) @@ -139,6 +152,17 @@ const InstallDialog = ({ qualifiedName, prefillEnv, onSuccess, onCancel }: Insta } }, [detail, envValues, configJson, qualifiedName, onSuccess, t]); + const handleDirectInstall = useCallback(async () => { + if (hasEnvKeys) { + setStep('configure'); + setInstallError(null); + return; + } + await handleInstall(); + }, [hasEnvKeys, handleInstall]); + + // ── Loading / error states ─────────────────────────────────────────────── + if (loadingDetail) { return (
@@ -165,36 +189,195 @@ const InstallDialog = ({ qualifiedName, prefillEnv, onSuccess, onCancel }: Insta if (!detail) return null; + const author = deriveAuthor(qualifiedName); + const transport = pickTransportLabel(detail.connections); + + // ── Step 1: Detail overview ────────────────────────────────────────────── + + if (step === 'detail') { + return ( +
+ + + {/* Header */} +
+ {detail.icon_url ? ( + + ) : ( +
+ 🔌 +
+ )} +
+

+ {detail.display_name} +

+ {author && ( +

+ {t('mcp.install.by')} {author} +

+ )} +
+
+ + {/* Stats badges */} +
+ {transport && ( + + {transport === 'stdio' + ? t('mcp.install.transportLocal') + : t('mcp.install.transportRemote')} + + )} + {detail.use_count != null && detail.use_count > 0 && ( + + {t('mcp.install.useCount').replace('{count}', formatUseCount(detail.use_count))} + + )} + {detail.is_deployed && ( + + {t('mcp.install.deployed')} + + )} + {hasEnvKeys && ( + + {t('mcp.install.requiresConfig')} + + )} +
+ + {/* Description */} + {detail.description && ( +
+ {detail.description} +
+ )} + + {/* Connections info */} + {detail.connections.length > 0 && ( +
+

+ {t('mcp.install.connections')} +

+
+ {detail.connections.map((conn, i) => ( +
+ + {conn.type} + {conn.published && ( + + ({t('mcp.install.published')}) + + )} + {conn.deployment_url && ( + + {conn.deployment_url} + + )} +
+ ))} +
+
+ )} + + {/* Required env keys preview */} + {hasEnvKeys && ( +
+

+ {t('mcp.install.requiredEnv')} +

+
+ {detail.required_env_keys!.map(key => ( + + {key} + + ))} +
+
+ )} + + {/* Install error (shown when no-config install fails) */} + {installError && ( +
+ {installError} +
+ )} + + {/* Actions */} +
+ + +
+
+ ); + } + + // ── Step 2: Configure & install ────────────────────────────────────────── + return (
- {/* Header */} -
+ + + {/* Compact header */} +
{detail.icon_url ? ( ) : ( -
+
🔌
)} -
-

- {t('mcp.install.title').replace('{name}', detail.display_name)} -

- {detail.description && ( -

- {detail.description} -

- )} -
+

+ {t('mcp.install.configureTitle').replace('{name}', detail.display_name)} +

{/* Env var inputs */} - {(detail.required_env_keys ?? []).length > 0 && ( -
+ {hasEnvKeys && ( +

{t('mcp.install.requiredEnv')}

@@ -202,7 +385,7 @@ const InstallDialog = ({ qualifiedName, prefillEnv, onSuccess, onCancel }: Insta
@@ -213,13 +396,13 @@ const InstallDialog = ({ qualifiedName, prefillEnv, onSuccess, onCancel }: Insta onChange={e => handleEnvChange(key, e.target.value)} placeholder={t('mcp.install.enterValue').replace('{key}', key)} disabled={installing} - className="flex-1 rounded-lg border border-stone-200 dark:border-neutral-700 bg-white dark:bg-neutral-900 px-3 py-1.5 text-sm text-stone-800 dark:text-neutral-100 placeholder:text-stone-400 dark:placeholder:text-neutral-500 focus:outline-none focus:ring-2 focus:ring-primary-500/40 disabled:opacity-50" + className="flex-1 rounded-lg border border-stone-200 dark:border-neutral-700 bg-white dark:bg-neutral-900 px-3 py-2 text-sm text-stone-800 dark:text-neutral-100 placeholder:text-stone-400 dark:placeholder:text-neutral-500 focus:outline-none focus:ring-2 focus:ring-primary-500/40 disabled:opacity-50" />
@@ -228,25 +411,35 @@ const InstallDialog = ({ qualifiedName, prefillEnv, onSuccess, onCancel }: Insta
)} - {/* Optional JSON config */} -
- -