From e9ca97c48cfddb87ea29cba9131a8cb71f1e79e2 Mon Sep 17 00:00:00 2001 From: Steven Enamakel <31011319+senamakel@users.noreply.github.com> Date: Sat, 23 May 2026 22:15:33 -0700 Subject: [PATCH] feat(mcp): split mcp_client/registry, multi-registry, boot-spawn + setup agent (#2559) --- Cargo.toml | 4 + app/src/App.tsx | 2 + .../mcp-setup/SecretPromptDialog.test.tsx | 115 +++++ .../mcp-setup/SecretPromptDialog.tsx | 175 ++++++++ app/src/lib/i18n/chunks/ar-1.ts | 14 + app/src/lib/i18n/chunks/bn-1.ts | 14 + app/src/lib/i18n/chunks/de-1.ts | 14 + app/src/lib/i18n/chunks/en-1.ts | 14 + app/src/lib/i18n/chunks/es-1.ts | 14 + app/src/lib/i18n/chunks/fr-1.ts | 14 + app/src/lib/i18n/chunks/hi-1.ts | 14 + app/src/lib/i18n/chunks/id-1.ts | 14 + app/src/lib/i18n/chunks/it-1.ts | 14 + app/src/lib/i18n/chunks/ko-1.ts | 14 + app/src/lib/i18n/chunks/pt-1.ts | 14 + app/src/lib/i18n/chunks/ru-1.ts | 14 + app/src/lib/i18n/chunks/zh-CN-1.ts | 14 + app/src/lib/i18n/en.ts | 14 + app/src/services/socketService.ts | 30 ++ docs/MCP_SETUP_AGENT.md | 162 +++++++ src/bin/test_mcp_stub.rs | 131 ++++++ src/core/all.rs | 7 +- src/core/event_bus/events.rs | 13 +- src/core/jsonrpc.rs | 12 + src/core/socketio.rs | 62 +++ src/openhuman/agent/agents/loader.rs | 5 + .../agent/agents/mcp_setup/agent.toml | 28 ++ src/openhuman/agent/agents/mcp_setup/mod.rs | 1 + .../agent/agents/mcp_setup/prompt.md | 49 +++ .../agent/agents/mcp_setup/prompt.rs | 97 +++++ src/openhuman/agent/agents/mod.rs | 1 + src/openhuman/mcp_client/mod.rs | 38 +- src/openhuman/mcp_clients/client/mod.rs | 290 ------------ src/openhuman/mcp_clients/client/protocol.rs | 215 --------- src/openhuman/mcp_clients/client/transport.rs | 225 ---------- src/openhuman/mcp_clients/connections.rs | 175 -------- src/openhuman/mcp_clients/mod.rs | 29 -- src/openhuman/mcp_clients/registry.rs | 226 ---------- src/openhuman/mcp_registry/boot.rs | 55 +++ .../{mcp_clients => mcp_registry}/bus.rs | 0 src/openhuman/mcp_registry/connections.rs | 212 +++++++++ src/openhuman/mcp_registry/mod.rs | 71 +++ .../{mcp_clients => mcp_registry}/ops.rs | 9 +- .../mcp_registry/registries/mcp_official.rs | 304 +++++++++++++ src/openhuman/mcp_registry/registries/mod.rs | 71 +++ .../mcp_registry/registries/smithery.rs | 214 +++++++++ src/openhuman/mcp_registry/registry.rs | 91 ++++ .../{mcp_clients => mcp_registry}/schemas.rs | 412 +++++++++++++++++- src/openhuman/mcp_registry/setup.rs | 327 ++++++++++++++ src/openhuman/mcp_registry/setup_ops.rs | 329 ++++++++++++++ .../{mcp_clients => mcp_registry}/store.rs | 0 .../{mcp_clients => mcp_registry}/types.rs | 9 + src/openhuman/mcp_server/http.rs | 2 +- src/openhuman/mod.rs | 2 +- src/openhuman/tool_registry/ops.rs | 2 +- src/openhuman/tools/impl/network/mcp_setup.rs | 393 +++++++++++++++++ src/openhuman/tools/impl/network/mod.rs | 5 + src/openhuman/tools/ops.rs | 14 + tests/mcp_registry_e2e.rs | 116 +++++ tests/mcp_setup_e2e.rs | 93 ++++ 60 files changed, 3829 insertions(+), 1190 deletions(-) create mode 100644 app/src/components/mcp-setup/SecretPromptDialog.test.tsx create mode 100644 app/src/components/mcp-setup/SecretPromptDialog.tsx create mode 100644 docs/MCP_SETUP_AGENT.md create mode 100644 src/bin/test_mcp_stub.rs create mode 100644 src/openhuman/agent/agents/mcp_setup/agent.toml create mode 100644 src/openhuman/agent/agents/mcp_setup/mod.rs create mode 100644 src/openhuman/agent/agents/mcp_setup/prompt.md create mode 100644 src/openhuman/agent/agents/mcp_setup/prompt.rs delete mode 100644 src/openhuman/mcp_clients/client/mod.rs delete mode 100644 src/openhuman/mcp_clients/client/protocol.rs delete mode 100644 src/openhuman/mcp_clients/client/transport.rs delete mode 100644 src/openhuman/mcp_clients/connections.rs delete mode 100644 src/openhuman/mcp_clients/mod.rs delete mode 100644 src/openhuman/mcp_clients/registry.rs create mode 100644 src/openhuman/mcp_registry/boot.rs rename src/openhuman/{mcp_clients => mcp_registry}/bus.rs (100%) create mode 100644 src/openhuman/mcp_registry/connections.rs create mode 100644 src/openhuman/mcp_registry/mod.rs rename src/openhuman/{mcp_clients => mcp_registry}/ops.rs (98%) create mode 100644 src/openhuman/mcp_registry/registries/mcp_official.rs create mode 100644 src/openhuman/mcp_registry/registries/mod.rs create mode 100644 src/openhuman/mcp_registry/registries/smithery.rs create mode 100644 src/openhuman/mcp_registry/registry.rs rename src/openhuman/{mcp_clients => mcp_registry}/schemas.rs (60%) create mode 100644 src/openhuman/mcp_registry/setup.rs create mode 100644 src/openhuman/mcp_registry/setup_ops.rs rename src/openhuman/{mcp_clients => mcp_registry}/store.rs (100%) rename src/openhuman/{mcp_clients => mcp_registry}/types.rs (96%) create mode 100644 src/openhuman/tools/impl/network/mcp_setup.rs create mode 100644 tests/mcp_registry_e2e.rs create mode 100644 tests/mcp_setup_e2e.rs diff --git a/Cargo.toml b/Cargo.toml index df843a7e2..06409c20f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,10 @@ path = "src/bin/memory_tree_init_smoke.rs" name = "inference-probe" path = "src/bin/inference_probe.rs" +[[bin]] +name = "test-mcp-stub" +path = "src/bin/test_mcp_stub.rs" + [lib] name = "openhuman_core" crate-type = ["rlib"] diff --git a/app/src/App.tsx b/app/src/App.tsx index ddef2f4d4..f7d0db50e 100644 --- a/app/src/App.tsx +++ b/app/src/App.tsx @@ -14,6 +14,7 @@ import ServiceBlockingGate from './components/daemon/ServiceBlockingGate'; import DictationHotkeyManager from './components/DictationHotkeyManager'; import ErrorFallbackScreen from './components/ErrorFallbackScreen'; import LocalAIDownloadSnackbar from './components/LocalAIDownloadSnackbar'; +import SecretPromptDialog from './components/mcp-setup/SecretPromptDialog'; import OpenhumanLinkModal from './components/OpenhumanLinkModal'; import PersistRehydrationScreen from './components/PersistRehydrationScreen'; import GlobalUpsellBanner from './components/upsell/GlobalUpsellBanner'; @@ -106,6 +107,7 @@ function App() { {!onMobile && } {!onMobile && } {!onMobile && } + diff --git a/app/src/components/mcp-setup/SecretPromptDialog.test.tsx b/app/src/components/mcp-setup/SecretPromptDialog.test.tsx new file mode 100644 index 000000000..00dcd0876 --- /dev/null +++ b/app/src/components/mcp-setup/SecretPromptDialog.test.tsx @@ -0,0 +1,115 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { SecretPromptDialog } from './SecretPromptDialog'; + +const callCoreRpc = vi.fn(); +vi.mock('../../services/coreRpcClient', () => ({ + callCoreRpc: (...args: unknown[]) => callCoreRpc(...args), +})); + +function dispatchRequest(detail: { refId: string; keyName: string; prompt: string }) { + window.dispatchEvent(new CustomEvent('openhuman:mcp-setup-secret-requested', { detail })); +} + +describe('SecretPromptDialog', () => { + beforeEach(() => { + callCoreRpc.mockReset(); + callCoreRpc.mockResolvedValue(undefined); + }); + + afterEach(() => { + // The component cleans up its own listener on unmount via useEffect; + // we don't need to remove the event listener manually here. + }); + + it('is hidden until an event is dispatched', () => { + render(); + expect(screen.queryByRole('dialog')).toBeNull(); + }); + + it('renders the prompt + key name when an event arrives', async () => { + render(); + dispatchRequest({ + refId: 'secret://abc123', + keyName: 'NOTION_API_KEY', + prompt: 'Paste your Notion integration token.', + }); + await screen.findByRole('dialog'); + expect(screen.getByText('NOTION_API_KEY')).toBeTruthy(); + expect(screen.getByText('Paste your Notion integration token.')).toBeTruthy(); + }); + + it('submits the value via mcp_setup_submit_secret and dismisses', async () => { + render(); + dispatchRequest({ refId: 'secret://abc123', keyName: 'TOKEN', prompt: '' }); + await screen.findByRole('dialog'); + + const input = screen.getByLabelText(/^Value$/i); + fireEvent.change(input, { target: { value: 'super-secret-value' } }); + + const submit = screen.getByText(/^Submit$/); + fireEvent.click(submit); + + await waitFor(() => { + expect(callCoreRpc).toHaveBeenCalledWith({ + method: 'openhuman.mcp_setup_submit_secret', + params: { ref_id: 'secret://abc123', value: 'super-secret-value' }, + }); + }); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + }); + + it('renders input as type=password by default and toggles on Show/Hide', async () => { + render(); + dispatchRequest({ refId: 'secret://abc', keyName: 'K', prompt: '' }); + await screen.findByRole('dialog'); + + const input = screen.getByLabelText(/^Value$/i) as HTMLInputElement; + expect(input.type).toBe('password'); + + fireEvent.click(screen.getByText(/^Show$/)); + expect((screen.getByLabelText(/^Value$/i) as HTMLInputElement).type).toBe('text'); + + fireEvent.click(screen.getByText(/^Hide$/)); + expect((screen.getByLabelText(/^Value$/i) as HTMLInputElement).type).toBe('password'); + }); + + it('cancel does not call mcp_setup_submit_secret', async () => { + render(); + dispatchRequest({ refId: 'secret://abc', keyName: 'K', prompt: '' }); + await screen.findByRole('dialog'); + + fireEvent.click(screen.getByText(/^Cancel$/)); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(callCoreRpc).not.toHaveBeenCalled(); + }); + + it('submit button disabled when value is empty', async () => { + render(); + dispatchRequest({ refId: 'secret://abc', keyName: 'K', prompt: '' }); + await screen.findByRole('dialog'); + + const submit = screen.getByText(/^Submit$/) as HTMLButtonElement; + expect(submit.disabled).toBe(true); + + const input = screen.getByLabelText(/^Value$/i); + fireEvent.change(input, { target: { value: 'x' } }); + expect(submit.disabled).toBe(false); + }); + + it('surfaces submit errors without dismissing', async () => { + callCoreRpc.mockRejectedValueOnce(new Error('boom')); + render(); + dispatchRequest({ refId: 'secret://abc', keyName: 'K', prompt: '' }); + await screen.findByRole('dialog'); + + const input = screen.getByLabelText(/^Value$/i); + fireEvent.change(input, { target: { value: 'v' } }); + fireEvent.click(screen.getByText(/^Submit$/)); + + await waitFor(() => expect(screen.getByText(/boom/)).toBeTruthy()); + // Dialog still open + expect(screen.queryByRole('dialog')).not.toBeNull(); + }); +}); diff --git a/app/src/components/mcp-setup/SecretPromptDialog.tsx b/app/src/components/mcp-setup/SecretPromptDialog.tsx new file mode 100644 index 000000000..b8594fe0e --- /dev/null +++ b/app/src/components/mcp-setup/SecretPromptDialog.tsx @@ -0,0 +1,175 @@ +// Listens for `openhuman:mcp-setup-secret-requested` window events dispatched +// by `socketService` and renders a native input dialog so the user can hand +// the core a secret value out-of-band. +// +// The dialog deliberately uses `` so the value isn't +// echoed in the UI by default and never lands in clipboard history via +// triple-click. On submit, the value is POSTed straight to +// `openhuman.mcp_setup_submit_secret` and immediately cleared from React +// state — no logging, no Redux, no persistence on this side. The MCP setup +// agent only sees the opaque `ref://` ref returned by +// `mcp_setup_request_secret`; the raw value never enters the LLM context. +import { useCallback, useEffect, useState } from 'react'; + +import { useT } from '../../lib/i18n/I18nContext'; +import { callCoreRpc } from '../../services/coreRpcClient'; + +type Request = { refId: string; keyName: string; prompt: string }; + +export function SecretPromptDialog() { + const { t } = useT(); + const [request, setRequest] = useState(null); + const [value, setValue] = useState(''); + const [reveal, setReveal] = useState(false); + const [submitting, setSubmitting] = useState(false); + const [error, setError] = useState(null); + + useEffect(() => { + const onRequest = (event: Event) => { + const detail = (event as CustomEvent).detail as Request | undefined; + if (!detail?.refId || !detail.keyName) return; + setRequest(detail); + setValue(''); + setReveal(false); + setError(null); + setSubmitting(false); + }; + window.addEventListener('openhuman:mcp-setup-secret-requested', onRequest); + return () => { + window.removeEventListener('openhuman:mcp-setup-secret-requested', onRequest); + }; + }, []); + + const reset = useCallback(() => { + setRequest(null); + setValue(''); + setReveal(false); + setError(null); + setSubmitting(false); + }, []); + + const handleSubmit = useCallback( + async (e: React.FormEvent) => { + e.preventDefault(); + if (!request || submitting || value.length === 0) return; + setSubmitting(true); + setError(null); + try { + await callCoreRpc({ + method: 'openhuman.mcp_setup_submit_secret', + params: { ref_id: request.refId, value }, + }); + // Wipe local state on success — the value has now moved into the + // core's process-local SETUP_SECRETS map; React doesn't need a + // copy. Closing the dialog also drops the React-tree reference. + reset(); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + setError(msg); + setSubmitting(false); + } + }, + [request, submitting, value, reset] + ); + + // Cancel: do NOT call mcp_setup_submit_secret. The agent-side + // `request_secret` will hit its 5-minute timeout and return an error + // the agent can surface to the user, which is the right outcome here. + const handleCancel = useCallback(() => { + if (submitting) return; + reset(); + }, [submitting, reset]); + + if (!request) return null; + + return ( +
+
e.stopPropagation()}> +
+
+

+ {t('mcp.setup.secretDialog.title')} +

+

+ {t('mcp.setup.secretDialog.bodyPrefix')}{' '} + + {request.keyName} + + {t('mcp.setup.secretDialog.bodySuffix')} +

+ {request.prompt && ( +

+ {request.prompt} +

+ )} +
+ +
+ +
+ setValue(e.target.value)} + placeholder={t('mcp.setup.secretDialog.inputPlaceholder')} + className="flex-1 px-3 py-2 rounded-lg border border-stone-300 dark:border-neutral-700 bg-stone-50 dark:bg-neutral-800 text-stone-900 dark:text-neutral-100 font-mono text-sm focus:outline-none focus:ring-2 focus:ring-primary-500" + autoFocus + disabled={submitting} + /> + +
+

+ {t('mcp.setup.secretDialog.privacyNote')} +

+ {error && ( +

+ {t('mcp.setup.secretDialog.errorPrefix')} {error} +

+ )} +
+ +
+ + +
+
+
+
+ ); +} + +export default SecretPromptDialog; diff --git a/app/src/lib/i18n/chunks/ar-1.ts b/app/src/lib/i18n/chunks/ar-1.ts index 9f2b88ad1..2025fa576 100644 --- a/app/src/lib/i18n/chunks/ar-1.ts +++ b/app/src/lib/i18n/chunks/ar-1.ts @@ -478,6 +478,20 @@ const ar1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/chunks/bn-1.ts b/app/src/lib/i18n/chunks/bn-1.ts index bac832920..3a4821df3 100644 --- a/app/src/lib/i18n/chunks/bn-1.ts +++ b/app/src/lib/i18n/chunks/bn-1.ts @@ -487,6 +487,20 @@ const bn1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/chunks/de-1.ts b/app/src/lib/i18n/chunks/de-1.ts index 002d767d9..a1e24124c 100644 --- a/app/src/lib/i18n/chunks/de-1.ts +++ b/app/src/lib/i18n/chunks/de-1.ts @@ -499,6 +499,20 @@ const de1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/chunks/en-1.ts b/app/src/lib/i18n/chunks/en-1.ts index 5488c3d9a..440f6f08e 100644 --- a/app/src/lib/i18n/chunks/en-1.ts +++ b/app/src/lib/i18n/chunks/en-1.ts @@ -475,6 +475,20 @@ const en1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/chunks/es-1.ts b/app/src/lib/i18n/chunks/es-1.ts index 40646f082..7e0301ff1 100644 --- a/app/src/lib/i18n/chunks/es-1.ts +++ b/app/src/lib/i18n/chunks/es-1.ts @@ -499,6 +499,20 @@ const es1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/chunks/fr-1.ts b/app/src/lib/i18n/chunks/fr-1.ts index e3528744a..4647b030d 100644 --- a/app/src/lib/i18n/chunks/fr-1.ts +++ b/app/src/lib/i18n/chunks/fr-1.ts @@ -501,6 +501,20 @@ const fr1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/chunks/hi-1.ts b/app/src/lib/i18n/chunks/hi-1.ts index 257d663d6..70d7a5143 100644 --- a/app/src/lib/i18n/chunks/hi-1.ts +++ b/app/src/lib/i18n/chunks/hi-1.ts @@ -484,6 +484,20 @@ const hi1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/chunks/id-1.ts b/app/src/lib/i18n/chunks/id-1.ts index 183588787..b093a49d5 100644 --- a/app/src/lib/i18n/chunks/id-1.ts +++ b/app/src/lib/i18n/chunks/id-1.ts @@ -490,6 +490,20 @@ const id1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/chunks/it-1.ts b/app/src/lib/i18n/chunks/it-1.ts index 0d11bcfc0..48b0071da 100644 --- a/app/src/lib/i18n/chunks/it-1.ts +++ b/app/src/lib/i18n/chunks/it-1.ts @@ -494,6 +494,20 @@ const it1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/chunks/ko-1.ts b/app/src/lib/i18n/chunks/ko-1.ts index 910b1f04a..cd35fde5e 100644 --- a/app/src/lib/i18n/chunks/ko-1.ts +++ b/app/src/lib/i18n/chunks/ko-1.ts @@ -487,6 +487,20 @@ const ko1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/chunks/pt-1.ts b/app/src/lib/i18n/chunks/pt-1.ts index 189bc5906..9de3752fa 100644 --- a/app/src/lib/i18n/chunks/pt-1.ts +++ b/app/src/lib/i18n/chunks/pt-1.ts @@ -499,6 +499,20 @@ const pt1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/chunks/ru-1.ts b/app/src/lib/i18n/chunks/ru-1.ts index 15447d2f3..a3e3b1acc 100644 --- a/app/src/lib/i18n/chunks/ru-1.ts +++ b/app/src/lib/i18n/chunks/ru-1.ts @@ -489,6 +489,20 @@ const ru1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/chunks/zh-CN-1.ts b/app/src/lib/i18n/chunks/zh-CN-1.ts index b35110823..03469f69f 100644 --- a/app/src/lib/i18n/chunks/zh-CN-1.ts +++ b/app/src/lib/i18n/chunks/zh-CN-1.ts @@ -471,6 +471,20 @@ const zhCN1: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/lib/i18n/en.ts b/app/src/lib/i18n/en.ts index ede7298b3..e759f262e 100644 --- a/app/src/lib/i18n/en.ts +++ b/app/src/lib/i18n/en.ts @@ -541,6 +541,20 @@ const en: TranslationMap = { 'mcp.alphaBannerText': 'MCP server support is in early alpha. The Smithery registry, install flow, and tool wiring may misbehave or change shape between releases.', 'mcp.toolList.noTools': 'No tools available.', + 'mcp.setup.secretDialog.title': 'MCP Setup — Enter Secret', + 'mcp.setup.secretDialog.bodyPrefix': 'The MCP setup agent needs', + 'mcp.setup.secretDialog.bodySuffix': + '. Your value is sent directly to the core process and never enters the AI conversation.', + 'mcp.setup.secretDialog.inputLabel': 'Value', + 'mcp.setup.secretDialog.inputPlaceholder': 'Paste here', + 'mcp.setup.secretDialog.show': 'Show', + 'mcp.setup.secretDialog.hide': 'Hide', + 'mcp.setup.secretDialog.submit': 'Submit', + 'mcp.setup.secretDialog.cancel': 'Cancel', + 'mcp.setup.secretDialog.submitting': 'Submitting…', + 'mcp.setup.secretDialog.errorPrefix': 'Failed to submit:', + 'mcp.setup.secretDialog.privacyNote': + 'Stored encrypted in the local MCP secrets table. Never logged or sent to a model.', 'devices.betaBadge': 'Beta', 'devices.betaText': 'This feature is currently in beta. Pair iOS phones with this OpenHuman to use them as a remote client.', diff --git a/app/src/services/socketService.ts b/app/src/services/socketService.ts index 361435948..758ef9bce 100644 --- a/app/src/services/socketService.ts +++ b/app/src/services/socketService.ts @@ -329,6 +329,36 @@ class SocketService { this.socket.on('auth:session_expired', handleSessionExpired); this.socket.on('auth_session_expired', handleSessionExpired); + // MCP setup agent: server-side `request_secret` blocks until the + // user submits a value. Dispatch a window event so a singleton React + // dialog can render a native input and POST back via + // openhuman.mcp_setup_submit_secret. Raw secret values never travel + // through the socket — only the opaque ref + safe display fields. + const handleSecretRequested = (data: unknown) => { + const obj = data as Record | null; + if (!obj || typeof obj !== 'object') { + socketWarn('mcp_setup:secret_requested dropped — invalid payload'); + return; + } + const refId = typeof obj.ref_id === 'string' ? obj.ref_id : null; + const keyName = typeof obj.key_name === 'string' ? obj.key_name : null; + const prompt = typeof obj.prompt === 'string' ? obj.prompt : ''; + if (!refId || !keyName) { + socketWarn('mcp_setup:secret_requested missing ref_id or key_name'); + return; + } + socketLog('mcp_setup:secret_requested', { refId, keyName }); + if (typeof window !== 'undefined') { + window.dispatchEvent( + new CustomEvent('openhuman:mcp-setup-secret-requested', { + detail: { refId, keyName, prompt }, + }) + ); + } + }; + this.socket.on('mcp_setup:secret_requested', handleSecretRequested); + this.socket.on('mcp_setup_secret_requested', handleSecretRequested); + this.socket.on('channel:managed-dm-verified', data => { const obj = data as Record | null; if (!obj || typeof obj !== 'object') return; diff --git a/docs/MCP_SETUP_AGENT.md b/docs/MCP_SETUP_AGENT.md new file mode 100644 index 000000000..eb454a44d --- /dev/null +++ b/docs/MCP_SETUP_AGENT.md @@ -0,0 +1,162 @@ +# MCP Setup Agent — design sketch + +A sub-agent that walks the user through installing, configuring, and +connecting an MCP server from one of the upstream registries +(`mcp_registry::registries`: Smithery, modelcontextprotocol/registry). + +This document is a **design sketch** for follow-up implementation. Nothing +here is wired up yet beyond the underlying primitives in +`src/openhuman/mcp_registry/`. + +--- + +## Goal + +A non-technical user says *"set up the Notion MCP server for me"*. The +agent: + +1. Browses the enabled registries, finds the candidate, summarises it. +2. Asks for any secrets the server requires (API keys, OAuth tokens, …) + **without ever pulling the values into the LLM context**. +3. Test-connects with the collected secrets, surfaces errors, lets the + user retry / change values. +4. On success, persists the install and the secrets, runs boot-spawn for + this one server, returns connection status + the tool list now + available to the main agent. + +The agent owns the conversation; the core owns the secrets, the +subprocess, and the persistence. + +--- + +## Tool surface + +Four tools registered behind a `mcp_setup_*` namespace. All tool inputs +and outputs are JSON; secret values **never** appear in either direction. + +| Tool | Input | Output | Notes | +| --- | --- | --- | --- | +| `mcp_setup_search` | `{ query?, page?, page_size?, source? }` | `{ servers: [Summary], total_pages }` | Thin wrapper over `mcp_registry::registry::registry_search`. `source` optionally scopes to one upstream. | +| `mcp_setup_get` | `{ qualified_name }` | `{ detail, required_env_keys }` | Wraps `registry_get`; pre-computes `required_env_keys` from the `config_schema` (same logic as `ops::collect_required_env_keys`). | +| `mcp_setup_request_secret` | `{ key_name, prompt }` | `{ ref: "secret://" }` | Triggers an out-of-band UI prompt. Returns an opaque ref; raw value is held in a process-local in-memory map keyed by ref. | +| `mcp_setup_test_connection` | `{ qualified_name, env_refs: { KEY: "secret://…" } }` | `{ ok, tools?: [McpTool], error?: string }` | Spawns the candidate subprocess in a **scratch** workspace, resolves refs to values just-in-time, runs `initialize` + `tools/list`, tears it down. No persistence. | +| `mcp_setup_install_and_connect` | `{ qualified_name, env_refs }` | `{ server_id, status, tools: [McpTool] }` | Resolves refs, persists the install + `mcp_client_env` rows, calls `connections::connect`. Refs are consumed (removed from the in-memory map) regardless of outcome. | + +--- + +## Secret flow — opaque refs + +The hard requirement: **raw secret values must not enter LLM context**. +Opaque refs solve this cleanly: + +``` +agent: mcp_setup_request_secret({ key_name: "NOTION_API_KEY", prompt: "Notion integration token" }) +core: → pushes prompt to UI; user types into a native input box +core: ← receives value, stores in SETUP_SECRETS: HashMap +core: → returns { ref: "secret://7c9f2e" } ← the agent sees only this +agent: mcp_setup_test_connection({ + qualified_name: "@notion/server", + env_refs: { "NOTION_API_KEY": "secret://7c9f2e" } + }) +core: → for each ref, look up the value in SETUP_SECRETS, build the env + vector, spawn, init, list_tools, tear down +core: ← returns { ok: true, tools: [...] } ← still no raw value to agent +``` + +Lifecycle of `SETUP_SECRETS`: + +- Process-local `OnceLock>>`. +- Entries TTL out after, say, 15 min (defends against stranded secrets if + the conversation is abandoned mid-flow). +- `mcp_setup_install_and_connect` consumes refs on success: pulls each + value, writes it to the `mcp_client_env` table (existing persistence, + already keyed by `server_id`), removes the ref. On failure refs are + left intact so the agent can retry without re-prompting the user. +- On core shutdown the map is dropped — refs do not survive restart. + +`RefId` is a short random hex string. **No structure or hint of the +underlying value** so the agent has nothing useful to leak even if it +tries. + +### Why not just take key names? + +Considered (option 2 in the original AskUserQuestion). Rejected because: + +- The agent can't decide between values it just collected — e.g. trying + two different tokens to pick the one that works requires distinguishing + them, which requires handles. +- Tying secrets to the `(server_id, key)` pair too early means a failed + test-connect leaves stale rows in `mcp_client_env` for an + uninstall-rolled-back server. + +Opaque refs give the agent enough handle to iterate without exposing +values. + +--- + +## Where the agent lives + +Follow the existing sub-agent pattern (`src/openhuman/agent/harness/`): + +- New archetype TOML at `app/src/lib/ai/agents/mcp_setup.toml` (loaded by + `AgentDefinitionRegistry::init_global`). +- Prompt + tool allowlist scoped tight: only the four `mcp_setup_*` tools + plus the standard `chat` / `ask_user` primitives. **No** general + filesystem, network, or shell tools — the agent shouldn't be able to + exfiltrate a leaked ref even if one shows up. +- Triggered by the main agent via `spawn_subagent("mcp_setup", { goal })` + or by an explicit UI affordance ("Add MCP server…" button that opens a + thread pinned to this archetype). + +--- + +## Implementation outline + +Following the project's `Specify → Rust → JSON-RPC → UI → tests` flow: + +1. **Rust core** (in `src/openhuman/mcp_registry/`): + - New module `setup.rs` owning `SETUP_SECRETS` (in-memory ref map with + TTL) and helpers `mint_ref`, `resolve_refs(env_refs) -> Vec<(K,V)>`, + `consume_refs(env_refs)`. + - New module `setup_ops.rs` with the four handlers. + - Wire schemas in `schemas.rs`, controllers in `core/all.rs`. +2. **Tool-side bridge** so the agent harness sees the four tools as + regular tool defs. Reuse the controller-to-tool generator already + used elsewhere. +3. **UI**: out-of-band secret prompt component (probably a `chat`-pinned + modal listening on a new socket event `mcp_setup_request_secret`), + submit POSTs the value to a Tauri command that calls into core to + register the ref. +4. **Archetype** + system prompt at `app/src/lib/ai/agents/mcp_setup.toml`. +5. **Tests**: + - Unit: ref lifecycle (mint → resolve → consume → TTL expiry). + - Integration (`tests/mcp_registry_e2e.rs` style): full flow against + the existing `test-mcp-stub` binary, asserting refs vanish after + install + that test-connect failures leave refs intact. + +--- + +## Open questions for the implementer + +- TTL value — 15 min is a guess; calibrate against typical install flow. +- Should `test_connection` accept a partial env_refs (some refs, some + literal-by-name) for iteration? Current design says refs only, which + forces consistency. +- The official MCP registry returns servers with **multiple package + ecosystems** (`packages: [{ registry_name: "npm" | "pypi" | … }]`). The + setup agent needs to either pick one or ask the user. Add a + `package_choice` step or default to npm? +- Telemetry: log `mcp_setup_*` calls (`tracing::info!` is fine) but + never log ref values, never log env values, only key names. + +--- + +## Anti-goals + +- The setup agent is **not** a generic "ask user for any data" surface. + Its prompt tool is scoped to MCP env values, full stop. +- It does **not** persist anything until `install_and_connect` succeeds. + No half-installed rows in `mcp_servers` or `mcp_client_env`. +- It does **not** read back secrets. Once persisted into `mcp_client_env` + they are write-only from the agent's perspective; only the subprocess + spawn path in `connections::connect` reads them. diff --git a/src/bin/test_mcp_stub.rs b/src/bin/test_mcp_stub.rs new file mode 100644 index 000000000..d42f8b755 --- /dev/null +++ b/src/bin/test_mcp_stub.rs @@ -0,0 +1,131 @@ +//! Tiny MCP stdio server used by `tests/mcp_registry_e2e.rs`. +//! +//! Speaks just enough of MCP 2024-11-05 to satisfy `initialize`, +//! `tools/list`, and `tools/call` for one toy tool (`echo`). Reads +//! newline-delimited JSON-RPC from stdin, writes responses to stdout, +//! exits when stdin closes. +//! +//! Intentionally dependency-free beyond serde_json so the binary builds +//! fast and is reliable in CI. + +use std::io::{self, BufRead, Write}; + +use serde_json::{json, Value}; + +const PROTOCOL_VERSION: &str = "2025-11-25"; + +fn main() { + let stdin = io::stdin(); + let mut stdout = io::stdout().lock(); + let mut stderr = io::stderr().lock(); + + let _ = writeln!(stderr, "[test_mcp_stub] ready"); + + for line in stdin.lock().lines() { + let line = match line { + Ok(l) => l, + Err(err) => { + let _ = writeln!(stderr, "[test_mcp_stub] stdin read error: {err}"); + break; + } + }; + let trimmed = line.trim(); + if trimmed.is_empty() { + continue; + } + + let req: Value = match serde_json::from_str(trimmed) { + Ok(v) => v, + Err(err) => { + let _ = writeln!(stderr, "[test_mcp_stub] invalid JSON: {err}"); + continue; + } + }; + + let id = req.get("id").cloned(); + let method = req + .get("method") + .and_then(Value::as_str) + .unwrap_or("") + .to_string(); + let params = req.get("params").cloned().unwrap_or(Value::Null); + + // Notifications (no id) are accepted silently — MCP sends + // `notifications/initialized` after the initialize handshake. + if id.is_none() { + let _ = writeln!(stderr, "[test_mcp_stub] notification: {method}"); + continue; + } + + let response = match method.as_str() { + "initialize" => json!({ + "jsonrpc": "2.0", + "id": id, + "result": { + "protocolVersion": PROTOCOL_VERSION, + "capabilities": { "tools": {} }, + "serverInfo": { "name": "test_mcp_stub", "version": "0.0.1" } + } + }), + "tools/list" => json!({ + "jsonrpc": "2.0", + "id": id, + "result": { + "tools": [ + { + "name": "echo", + "description": "Returns the `message` argument verbatim.", + "inputSchema": { + "type": "object", + "properties": { + "message": { "type": "string" } + }, + "required": ["message"] + } + } + ] + } + }), + "tools/call" => { + let tool = params.get("name").and_then(Value::as_str).unwrap_or(""); + let args = params.get("arguments").cloned().unwrap_or(Value::Null); + if tool == "echo" { + let msg = args.get("message").and_then(Value::as_str).unwrap_or(""); + json!({ + "jsonrpc": "2.0", + "id": id, + "result": { + "content": [ + { "type": "text", "text": msg } + ], + "isError": false + } + }) + } else { + json!({ + "jsonrpc": "2.0", + "id": id, + "error": { + "code": -32601, + "message": format!("unknown tool `{tool}`") + } + }) + } + } + _ => json!({ + "jsonrpc": "2.0", + "id": id, + "error": { + "code": -32601, + "message": format!("method `{method}` not implemented in stub") + } + }), + }; + + let line = serde_json::to_string(&response).unwrap(); + let _ = writeln!(stdout, "{line}"); + let _ = stdout.flush(); + } + + let _ = writeln!(stderr, "[test_mcp_stub] stdin closed, exiting"); +} diff --git a/src/core/all.rs b/src/core/all.rs index d87a3792f..e6cbebf68 100644 --- a/src/core/all.rs +++ b/src/core/all.rs @@ -115,7 +115,7 @@ fn build_registered_controllers() -> Vec { // Scheduled job management controllers.extend(crate::openhuman::cron::all_cron_registered_controllers()); // MCP client subsystem: Smithery registry browser, local server install/connect, tool dispatch - controllers.extend(crate::openhuman::mcp_clients::all_mcp_clients_registered_controllers()); + controllers.extend(crate::openhuman::mcp_registry::all_mcp_registry_registered_controllers()); // Webview APIs bridge — proxies connector calls (Gmail, …) through // a WebSocket to the Tauri shell so curl reaches the live webview. controllers.extend(crate::openhuman::webview_apis::all_webview_apis_registered_controllers()); @@ -278,7 +278,7 @@ fn build_declared_controller_schemas() -> Vec { schemas.extend(crate::openhuman::audio_toolkit::all_audio_toolkit_controller_schemas()); schemas.extend(crate::openhuman::composio::all_composio_controller_schemas()); schemas.extend(crate::openhuman::cron::all_cron_controller_schemas()); - schemas.extend(crate::openhuman::mcp_clients::all_mcp_clients_controller_schemas()); + schemas.extend(crate::openhuman::mcp_registry::all_mcp_registry_controller_schemas()); schemas.extend(crate::openhuman::webview_apis::all_webview_apis_controller_schemas()); schemas.extend(crate::openhuman::agent::all_agent_controller_schemas()); schemas.extend(crate::openhuman::agent_experience::all_agent_experience_controller_schemas()); @@ -395,6 +395,9 @@ pub fn namespace_description(namespace: &str) -> Option<&'static str> { "mcp_clients" => Some( "Browse the Smithery.ai MCP registry, install MCP servers locally, manage their stdio connections, and expose their tools to the agent.", ), + "mcp_setup" => Some( + "MCP setup agent surface: search registries, request secrets out-of-band (opaque refs, no raw values in agent context), test, and install + connect.", + ), "decrypt" => Some("Decrypt secure values managed by secret storage."), "doctor" => Some("Run diagnostics for workspace and runtime health."), "encrypt" => Some("Encrypt secure values managed by secret storage."), diff --git a/src/core/event_bus/events.rs b/src/core/event_bus/events.rs index 6dd56348e..ca8d78ce6 100644 --- a/src/core/event_bus/events.rs +++ b/src/core/event_bus/events.rs @@ -516,6 +516,16 @@ pub enum DomainEvent { success: bool, elapsed_ms: u64, }, + /// The MCP setup agent asked the user for a secret value. The UI + /// subscribes to this and renders a native prompt; on submit it calls + /// `openhuman.mcp_setup_submit_secret`. `ref_id` is the opaque handle + /// returned to the agent; the raw secret value never traverses this + /// event. + McpSetupSecretRequested { + ref_id: String, + key_name: String, + prompt: String, + }, // ── System lifecycle ──────────────────────────────────────────────── /// A system component started up. @@ -638,7 +648,8 @@ impl DomainEvent { Self::McpServerInstalled { .. } | Self::McpServerConnected { .. } | Self::McpServerDisconnected { .. } - | Self::McpClientToolExecuted { .. } => "mcp_client", + | Self::McpClientToolExecuted { .. } + | Self::McpSetupSecretRequested { .. } => "mcp_client", } } } diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index cb1013bbe..6834bb1de 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -1693,6 +1693,18 @@ pub async fn bootstrap_core_runtime(embedded_core: bool) { // --- Workspace migrations -------------------------------------------- crate::openhuman::startup::run_workspace_migrations(&workspace_dir); + // --- MCP registry boot-spawn ----------------------------------------- + // Bring up every locally-installed MCP server's stdio subprocess so its + // tools are available to the agent as soon as the core is ready. + // Errors are logged per-server and never block boot. Runs as a + // background task so a slow npx install can't gate startup. + { + let cfg = cfg.clone(); + tokio::spawn(async move { + crate::openhuman::mcp_registry::boot::spawn_installed_servers(&cfg).await; + }); + } + // --- Socket manager bootstrap --- let socket_mgr = Arc::new(SocketManager::new()); set_global_socket_manager(socket_mgr.clone()); diff --git a/src/core/socketio.rs b/src/core/socketio.rs index 258c4628d..fe6e899c5 100644 --- a/src/core/socketio.rs +++ b/src/core/socketio.rs @@ -509,6 +509,7 @@ pub fn spawn_web_channel_bridge(io: SocketIo) { let io_transcription = io.clone(); let io_auth = io.clone(); let io_companion = io.clone(); + let io_mcp_setup = io.clone(); // 2. Dictation hotkey events → broadcast to all connected clients. tokio::spawn(async move { @@ -659,6 +660,67 @@ pub fn spawn_web_channel_bridge(io: SocketIo) { log::debug!("[socketio] auth session_expired bridge stopped"); }); + // 6b. McpSetupSecretRequested → broadcast `mcp_setup:secret_requested` + // so the UI can render a native input dialog. Only the opaque + // ref + safe display fields are forwarded; raw secret values + // are not part of the event payload. + tokio::spawn(async move { + let bus = { + const RETRY_INTERVAL_MS: u64 = 250; + const MAX_WAIT_SECS: u64 = 30; + let max_attempts = (MAX_WAIT_SECS * 1000) / RETRY_INTERVAL_MS; + let mut attempts: u64 = 0; + loop { + if let Some(bus) = crate::core::event_bus::global() { + break bus; + } + attempts += 1; + if attempts > max_attempts { + log::warn!( + "[socketio] event_bus not initialised after {}s — mcp_setup bridge giving up", + MAX_WAIT_SECS + ); + return; + } + tokio::time::sleep(std::time::Duration::from_millis(RETRY_INTERVAL_MS)).await; + } + }; + let mut rx = bus.raw_receiver(); + loop { + let event = match rx.recv().await { + Ok(event) => event, + Err(tokio::sync::broadcast::error::RecvError::Lagged(skipped)) => { + log::warn!( + "[socketio] dropped {} event_bus events due to lag (mcp_setup bridge)", + skipped + ); + continue; + } + Err(tokio::sync::broadcast::error::RecvError::Closed) => break, + }; + if let crate::core::event_bus::DomainEvent::McpSetupSecretRequested { + ref_id, + key_name, + prompt, + } = event + { + log::info!( + "[socketio] broadcast mcp_setup:secret_requested ref={} key={}", + ref_id, + key_name + ); + let payload = serde_json::json!({ + "ref_id": ref_id, + "key_name": key_name, + "prompt": prompt, + }); + let _ = io_mcp_setup.emit("mcp_setup:secret_requested", &payload); + let _ = io_mcp_setup.emit("mcp_setup_secret_requested", &payload); + } + } + log::debug!("[socketio] mcp_setup secret_requested bridge stopped"); + }); + // 5. Transcription results → broadcast to all connected clients. tokio::spawn(async move { let mut rx = crate::openhuman::voice::dictation_listener::subscribe_transcription_results(); diff --git a/src/openhuman/agent/agents/loader.rs b/src/openhuman/agent/agents/loader.rs index a05ea446d..2bb7e5785 100644 --- a/src/openhuman/agent/agents/loader.rs +++ b/src/openhuman/agent/agents/loader.rs @@ -143,6 +143,11 @@ pub const BUILTINS: &[BuiltinAgent] = &[ toml: include_str!("help/agent.toml"), prompt_fn: super::help::prompt::build, }, + BuiltinAgent { + id: "mcp_setup", + toml: include_str!("mcp_setup/agent.toml"), + prompt_fn: super::mcp_setup::prompt::build, + }, ]; /// Parse every entry in [`BUILTINS`] into an [`AgentDefinition`]. diff --git a/src/openhuman/agent/agents/mcp_setup/agent.toml b/src/openhuman/agent/agents/mcp_setup/agent.toml new file mode 100644 index 000000000..2c568fd36 --- /dev/null +++ b/src/openhuman/agent/agents/mcp_setup/agent.toml @@ -0,0 +1,28 @@ +id = "mcp_setup" +display_name = "MCP Setup Agent" +delegate_name = "setup_mcp_server" +when_to_use = "Walks the user through installing and connecting an MCP server end-to-end. Use when the user asks to add / install / set up an MCP server (e.g. \"set up the Notion MCP\", \"connect the GitHub MCP server\"). Owns the full flow: search registries → ask the user for any required secrets via a native dialog (raw values never enter the agent context) → test the connection → commit the install." +temperature = 0.3 +max_iterations = 12 +sandbox_mode = "none" +omit_identity = true +omit_memory_context = true +omit_safety_preamble = false +omit_skills_catalog = true + +[model] +hint = "agentic" + +[tools] +# Tight allowlist — the agent must not have shell, filesystem, network, +# or any other capability that could exfiltrate a leaked secret ref. +# Five `mcp_setup_*` tools plus `ask_user_question` for natural-language +# checkpoints; nothing else. +named = [ + "mcp_setup_search", + "mcp_setup_get", + "mcp_setup_request_secret", + "mcp_setup_test_connection", + "mcp_setup_install_and_connect", + "ask_user_clarification", +] diff --git a/src/openhuman/agent/agents/mcp_setup/mod.rs b/src/openhuman/agent/agents/mcp_setup/mod.rs new file mode 100644 index 000000000..8bf84783c --- /dev/null +++ b/src/openhuman/agent/agents/mcp_setup/mod.rs @@ -0,0 +1 @@ +pub mod prompt; diff --git a/src/openhuman/agent/agents/mcp_setup/prompt.md b/src/openhuman/agent/agents/mcp_setup/prompt.md new file mode 100644 index 000000000..a551bb947 --- /dev/null +++ b/src/openhuman/agent/agents/mcp_setup/prompt.md @@ -0,0 +1,49 @@ +# MCP Setup Agent + +You guide the user through installing and connecting one MCP server end-to-end. Each spawn handles **one** server — if the user asks for several, install them one at a time. + +## Your tool surface + +- **`mcp_setup_search`** — keyword search across all enabled MCP registries (Smithery + the official `modelcontextprotocol/registry`). Returns server summaries with a `source` tag so you can attribute results. +- **`mcp_setup_get`** — full detail for one server, including `required_env_keys` derived from its connection schema. Use this to know which secrets to ask for. +- **`mcp_setup_request_secret`** — pop a native input dialog in front of the user to collect one secret. Returns an opaque ref like `secret://abc123`. **The raw value never enters your context.** You only get the ref; the core resolves it to the real value just-in-time when you call test/install. +- **`mcp_setup_test_connection`** — dry-run: spawn the candidate server with the collected secret refs, list its tools, tear it down. Nothing persisted. Use this to validate the user's input before committing. +- **`mcp_setup_install_and_connect`** — commit: persist the install + the secrets (consuming the refs), connect immediately, return the tool list now available to the main agent. +- **`ask_user_clarification`** — natural-language checkpoints ("Did you mean X or Y?", "Ready to install?", etc.). + +You have **nothing else** — no shell, no file I/O, no general HTTP. Stay inside this surface. + +## Standard flow + +1. **Identify the server.** From the user's request, search with `mcp_setup_search`. If multiple candidates match, summarise the top 2–3 and ask the user to confirm via `ask_user_clarification`. Prefer servers with `is_deployed: true` and higher `use_count` when the user is non-specific. +2. **Fetch detail.** Once a `qualified_name` is locked, call `mcp_setup_get(qualified_name)`. Read `required_env_keys` — that's your secret-collection checklist. +3. **Collect secrets, one per key.** For each key in `required_env_keys`, call `mcp_setup_request_secret({key_name, prompt})` where `prompt` is a plain-English instruction the user sees in the native dialog. Examples: + - `NOTION_API_KEY` → `"Paste your Notion integration token. Get one at notion.so/my-integrations → New integration → Internal."` + - `GITHUB_TOKEN` → `"Paste a GitHub personal access token with repo + read:user scopes."` + - `OPENAI_API_KEY` → `"Paste your OpenAI API key (starts with sk-)."` + + Store every returned `ref` in a local map keyed by `key_name`. The call blocks for up to 5 minutes per secret — that's fine, the user is at the dialog. +4. **Test.** Call `mcp_setup_test_connection({qualified_name, env_refs})` with the full ref map. Three outcomes: + - `ok: true` → list `tools` to the user for a sanity check; proceed to install. + - `ok: false` → surface `error` plainly. Common causes: wrong/expired token, missing scope, server-side bug. Offer to re-collect the offending secret (call `mcp_setup_request_secret` again for that one key, replace its ref in your map, retry test). +5. **Install.** On a successful test, call `mcp_setup_install_and_connect({qualified_name, env_refs})`. Two outcomes: + - `status: "connected"` → tell the user the server is live and list the new tools (`tools[].name`) so they know what's available. + - `status: "installed_disconnected"` → the install persisted but the live connection failed. Surface `error`; tell the user they can retry via Settings → MCP Servers → Reconnect. + +## Hard rules + +- **You never see raw secret values.** If you somehow do (a bug somewhere), abort, do not log or repeat the value, and tell the user to remove the leak. +- **Refs are opaque.** Don't try to deserialise, decode, or reason about the `secret://` payload. It's a random hex handle, nothing more. +- **One server per spawn.** If the user asks for two, finish one cleanly, then suggest they spawn you again for the second. +- **Don't fabricate `required_env_keys`.** Pull them from `mcp_setup_get`. Asking the user for a key the server doesn't need wastes their time and may leak unrelated credentials into our store. +- **Don't skip the test step.** Always `test_connection` before `install_and_connect` so the user has a chance to fix typos before we persist anything to the secrets store. +- **Be honest about failures.** If a server's config is so under-documented that you can't figure out the keys, say so and stop. Don't guess. + +## Telemetry / privacy reminders + +- Tool calls are logged. Calls show `key_name` (safe) and `ref` (opaque). They never show secret values. +- The user's submitted secret value travels: native UI dialog → core IPC → `SETUP_SECRETS` in-memory map → encrypted `mcp_client_env` table on success. It never round-trips through the LLM at any point. + +## When you're done + +Return a short summary: which server, which tools are now available, and any caveats (e.g. "Notion integration only sees pages you've explicitly shared with it — share at least one page before using `notion_get_page`."). Hand control back to the user / orchestrator. diff --git a/src/openhuman/agent/agents/mcp_setup/prompt.rs b/src/openhuman/agent/agents/mcp_setup/prompt.rs new file mode 100644 index 000000000..9017c9d82 --- /dev/null +++ b/src/openhuman/agent/agents/mcp_setup/prompt.rs @@ -0,0 +1,97 @@ +//! System prompt builder for the `mcp_setup` built-in agent. +//! +//! Straightforward: render the static archetype, then append the +//! tool block so the model sees the five `mcp_setup_*` tool schemas +//! plus `ask_user_clarification` (filtered down by the harness from the +//! `agent.toml` allowlist). + +use crate::openhuman::context::prompt::{ + render_tools, render_user_files, render_workspace, PromptContext, +}; +use anyhow::Result; + +const ARCHETYPE: &str = include_str!("prompt.md"); + +pub fn build(ctx: &PromptContext<'_>) -> Result { + let mut out = String::with_capacity(4096); + out.push_str(ARCHETYPE.trim_end()); + out.push_str("\n\n"); + + let user_files = render_user_files(ctx)?; + if !user_files.trim().is_empty() { + out.push_str(user_files.trim_end()); + out.push_str("\n\n"); + } + + let tools = render_tools(ctx)?; + if !tools.trim().is_empty() { + out.push_str(tools.trim_end()); + out.push_str("\n\n"); + } + + let workspace = render_workspace(ctx)?; + if !workspace.trim().is_empty() { + out.push_str(workspace.trim_end()); + out.push('\n'); + } + + Ok(out) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::openhuman::context::prompt::{LearnedContextData, ToolCallFormat}; + use std::collections::HashSet; + + fn empty_ctx() -> PromptContext<'static> { + static EMPTY_VISIBLE: std::sync::OnceLock> = std::sync::OnceLock::new(); + let visible = EMPTY_VISIBLE.get_or_init(HashSet::new); + PromptContext { + workspace_dir: std::path::Path::new("."), + model_name: "test", + agent_id: "mcp_setup", + tools: &[], + skills: &[], + dispatcher_instructions: "", + learned: LearnedContextData::default(), + visible_tool_names: visible, + tool_call_format: ToolCallFormat::PFormat, + connected_integrations: &[], + connected_identities_md: String::new(), + include_profile: false, + include_memory_md: false, + curated_snapshot: None, + user_identity: None, + } + } + + #[test] + fn build_returns_nonempty_body() { + let body = build(&empty_ctx()).unwrap(); + assert!(!body.is_empty()); + assert!(body.contains("MCP Setup Agent")); + } + + #[test] + fn archetype_documents_opaque_ref_invariant() { + let body = build(&empty_ctx()).unwrap(); + assert!(body.contains("never enters your context")); + assert!(body.contains("secret://")); + } + + #[test] + fn archetype_documents_standard_flow_steps() { + let body = build(&empty_ctx()).unwrap(); + for needle in [ + "mcp_setup_search", + "mcp_setup_get", + "mcp_setup_request_secret", + "mcp_setup_test_connection", + "mcp_setup_install_and_connect", + "ask_user_clarification", + ] { + assert!(body.contains(needle), "prompt missing `{needle}`"); + } + } +} diff --git a/src/openhuman/agent/agents/mod.rs b/src/openhuman/agent/agents/mod.rs index 5a64988aa..fc02b16a3 100644 --- a/src/openhuman/agent/agents/mod.rs +++ b/src/openhuman/agent/agents/mod.rs @@ -11,6 +11,7 @@ pub mod crypto_agent; pub mod help; pub mod integrations_agent; pub mod markets_agent; +pub mod mcp_setup; pub mod morning_briefing; pub mod orchestrator; pub mod planner; diff --git a/src/openhuman/mcp_client/mod.rs b/src/openhuman/mcp_client/mod.rs index 33db8dd4c..fde76b89e 100644 --- a/src/openhuman/mcp_client/mod.rs +++ b/src/openhuman/mcp_client/mod.rs @@ -1,8 +1,38 @@ -//! Shared MCP client + registry for remote MCP servers exposed to agents. +//! MCP client transport library + static-config server set. //! -//! Supports Streamable HTTP and stdio transports. HTTP transport carries -//! session + auth lifecycle; stdio launches a subprocess and exchanges -//! newline-delimited JSON-RPC messages over stdin/stdout per the MCP spec. +//! Two responsibilities: +//! +//! 1. **Transport primitives** — [`McpHttpClient`] (Streamable HTTP + OAuth + +//! SSE per the MCP spec) and [`McpStdioClient`] (subprocess JSON-RPC over +//! stdin/stdout). These types are reusable building blocks for any module +//! that needs to *talk to* a remote MCP server. +//! +//! 2. **Static server set** — [`McpServerRegistry`] holds servers declared in +//! the user's TOML config under `[[mcp_client.servers]]`. Agents reach +//! these via the generic bridge tools in +//! [`crate::openhuman::tools::impl::network::mcp`] (`mcp_list_servers`, +//! `mcp_list_tools`, `mcp_call_tool`). The bespoke `gitbooks` tool also +//! consumes [`McpHttpClient`] directly. +//! +//! # Relationship to `mcp_registry` +//! +//! The sibling [`crate::openhuman::mcp_registry`] module owns the *dynamic*, +//! user-installed Smithery / official-registry MCP servers (full RPC CRUD, +//! SQLite persistence, live connection registry, boot-time spawn). All stdio +//! transport for those installs flows through this module's +//! [`McpStdioClient`] — `mcp_registry` carries no transport code of its own. +//! +//! In short: +//! - **`mcp_client`** (this module): transport library + read-only static +//! server set declared in user config. +//! - **`mcp_registry`** (sibling): dynamic Smithery installations, lifecycle, +//! persistence, and RPC surface. +//! +//! # Modules +//! - `client` — [`McpHttpClient`] and shared MCP protocol types +//! - `stdio` — [`McpStdioClient`] +//! - `registry` — [`McpServerRegistry`] built from +//! [`crate::openhuman::config::McpClientConfig`] mod client; mod registry; diff --git a/src/openhuman/mcp_clients/client/mod.rs b/src/openhuman/mcp_clients/client/mod.rs deleted file mode 100644 index 4ab114f9d..000000000 --- a/src/openhuman/mcp_clients/client/mod.rs +++ /dev/null @@ -1,290 +0,0 @@ -//! MCP stdio client: spawns a child process and speaks the MCP JSON-RPC -//! stdio protocol (initialize → tools/list → tools/call). -//! -//! The client is `Send + Sync` and is stored in a global registry keyed by -//! `server_id`. Callers use the `McpTransport` trait for testing (see -//! `protocol::McpTransport`). - -pub mod protocol; -pub mod transport; - -use std::collections::HashMap; -use std::sync::Arc; - -use async_trait::async_trait; -use serde_json::{json, Value}; -use tokio::sync::Mutex; - -use crate::openhuman::mcp_clients::types::McpTool; - -use protocol::{ - build_initialize_params, build_request, parse_tools_list, send_request_and_wait, McpTransport, - RequestIdCounter, -}; -use transport::SpawnedProcess; - -// ── McpStdioClient ───────────────────────────────────────────────────────── - -/// A live connection to an MCP server over stdio. -pub struct McpStdioClient { - server_id: String, - process: Mutex, - counter: RequestIdCounter, - /// Cached tool list after `initialize`. - cached_tools: Mutex>, -} - -impl McpStdioClient { - /// Spawn the server process and run `initialize` + `tools/list`. - pub async fn spawn_and_init( - server_id: &str, - command: &str, - args: &[String], - env: &HashMap, - ) -> anyhow::Result> { - tracing::debug!( - "[mcp-client] spawn_and_init server_id={} command={} args={:?} env_keys={:?}", - server_id, - command, - args, - env.keys().collect::>() - ); - - let mut cmd = tokio::process::Command::new(command); - cmd.args(args) - .envs(env) - .stdin(std::process::Stdio::piped()) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .kill_on_drop(true); - - let child = cmd - .spawn() - .map_err(|e| anyhow::anyhow!("[mcp-client] failed to spawn {command}: {e}"))?; - - let proc = SpawnedProcess::from_child(child, server_id)?; - let client = Arc::new(Self { - server_id: server_id.to_string(), - process: Mutex::new(proc), - counter: RequestIdCounter::new(), - cached_tools: Mutex::new(Vec::new()), - }); - - // Run MCP initialize handshake - let init_result = client.initialize().await.map_err(|e| { - anyhow::anyhow!("[mcp-client] server_id={server_id} initialize failed: {e}") - })?; - tracing::debug!( - "[mcp-client] server_id={} initialize result: {}", - server_id, - init_result - ); - - // Discover tools - let tools = client.list_tools().await.map_err(|e| { - anyhow::anyhow!("[mcp-client] server_id={server_id} tools/list failed: {e}") - })?; - tracing::debug!( - "[mcp-client] server_id={} discovered {} tools", - server_id, - tools.len() - ); - { - let mut guard = client.cached_tools.lock().await; - *guard = tools; - } - - Ok(client) - } - - /// Return a snapshot of the cached tool list without a live RPC call. - pub async fn tools_snapshot(&self) -> Vec { - self.cached_tools.lock().await.clone() - } - - /// Return the last stderr line for error reporting. - pub async fn last_error(&self) -> Option { - self.process.lock().await.reader.last_stderr().await - } -} - -#[async_trait] -impl McpTransport for McpStdioClient { - async fn initialize(&self) -> Result { - let id = self.counter.next().await; - let msg = build_request(id, "initialize", Some(build_initialize_params())); - - let result = { - let mut proc = self.process.lock().await; - let pending = proc.reader.pending.clone(); - let writer = &mut proc.writer; - // Register the pending waiter (inside send_request_and_wait) BEFORE - // performing the write, so a fast reply from the server isn't dropped - // by the reader before we're waiting for it. - send_request_and_wait(id, msg.clone(), &pending, async { - writer.send(&msg).await.map_err(|e| anyhow::anyhow!("{e}")) - }) - .await - }; - - if result.is_ok() { - // Send initialized notification (no response expected) - let notif = json!({ - "jsonrpc": "2.0", - "method": "notifications/initialized", - "params": {} - }); - let mut proc = self.process.lock().await; - let _ = proc.writer.send(¬if).await; - } - - result - } - - async fn list_tools(&self) -> Result, String> { - let id = self.counter.next().await; - let msg = build_request(id, "tools/list", None); - - let result = { - let mut proc = self.process.lock().await; - let pending = proc.reader.pending.clone(); - let writer = &mut proc.writer; - send_request_and_wait(id, msg.clone(), &pending, async { - writer.send(&msg).await.map_err(|e| anyhow::anyhow!("{e}")) - }) - .await - }?; - - let tools = parse_tools_list(result)?; - // Update cache - let mut guard = self.cached_tools.lock().await; - *guard = tools.clone(); - Ok(tools) - } - - async fn call_tool(&self, tool_name: &str, arguments: Value) -> Result { - tracing::debug!( - "[mcp-client] server_id={} tool_call tool_name={}", - self.server_id, - tool_name - ); - let id = self.counter.next().await; - let params = json!({ - "name": tool_name, - "arguments": arguments - }); - let msg = build_request(id, "tools/call", Some(params)); - - let mut proc = self.process.lock().await; - let pending = proc.reader.pending.clone(); - let writer = &mut proc.writer; - send_request_and_wait(id, msg.clone(), &pending, async { - writer.send(&msg).await.map_err(|e| anyhow::anyhow!("{e}")) - }) - .await - } - - async fn shutdown(&self) { - tracing::debug!("[mcp-client] shutdown server_id={}", self.server_id); - let notif = json!({ - "jsonrpc": "2.0", - "method": "notifications/cancelled", - "params": { "reason": "client shutdown" } - }); - let mut proc = self.process.lock().await; - let _ = proc.writer.send(¬if).await; - let _ = proc.child.kill().await; - } -} - -// ── FakeMcpTransport (test double) ────────────────────────────────────────── - -/// An in-memory fake for `McpTransport` usable in unit and E2E tests. -/// Responds to `initialize`, `list_tools`, and `call_tool` without spawning -/// any real process. -pub struct FakeMcpTransport { - pub tools: Vec, - /// Canned result for `call_tool`. If `Err`, the call returns that error. - pub call_result: Result, -} - -impl FakeMcpTransport { - pub fn new(tools: Vec, call_result: Result) -> Arc { - Arc::new(Self { tools, call_result }) - } - - pub fn empty() -> Arc { - Self::new(Vec::new(), Ok(Value::Null)) - } -} - -#[async_trait] -impl McpTransport for FakeMcpTransport { - async fn initialize(&self) -> Result { - Ok(json!({ - "protocolVersion": transport::MCP_PROTOCOL_VERSION, - "capabilities": {} - })) - } - - async fn list_tools(&self) -> Result, String> { - Ok(self.tools.clone()) - } - - async fn call_tool(&self, _tool_name: &str, _arguments: Value) -> Result { - self.call_result.clone() - } - - async fn shutdown(&self) {} -} - -#[cfg(test)] -mod tests { - use super::*; - use serde_json::json; - - fn make_tool(name: &str) -> McpTool { - McpTool { - name: name.to_string(), - description: Some(format!("Description for {name}")), - input_schema: json!({ "type": "object" }), - } - } - - #[tokio::test] - async fn fake_initialize_returns_protocol_version() { - let fake = FakeMcpTransport::empty(); - let result = fake.initialize().await.unwrap(); - assert_eq!(result["protocolVersion"], transport::MCP_PROTOCOL_VERSION); - } - - #[tokio::test] - async fn fake_list_tools_returns_configured_tools() { - let tools = vec![make_tool("search"), make_tool("write")]; - let fake = FakeMcpTransport::new(tools.clone(), Ok(Value::Null)); - let listed = fake.list_tools().await.unwrap(); - assert_eq!(listed.len(), 2); - assert_eq!(listed[0].name, "search"); - } - - #[tokio::test] - async fn fake_call_tool_returns_configured_result() { - let expected = json!({ "answer": 42 }); - let fake = FakeMcpTransport::new(vec![], Ok(expected.clone())); - let result = fake.call_tool("any_tool", json!({})).await.unwrap(); - assert_eq!(result, expected); - } - - #[tokio::test] - async fn fake_call_tool_propagates_error() { - let fake = FakeMcpTransport::new(vec![], Err("tool failed".to_string())); - let err = fake.call_tool("tool", json!({})).await.unwrap_err(); - assert_eq!(err, "tool failed"); - } - - #[tokio::test] - async fn fake_shutdown_does_not_panic() { - let fake = FakeMcpTransport::empty(); - fake.shutdown().await; - } -} diff --git a/src/openhuman/mcp_clients/client/protocol.rs b/src/openhuman/mcp_clients/client/protocol.rs deleted file mode 100644 index c07edbb23..000000000 --- a/src/openhuman/mcp_clients/client/protocol.rs +++ /dev/null @@ -1,215 +0,0 @@ -//! MCP JSON-RPC protocol framing: request serialisation, response correlation, -//! and higher-level method helpers (`initialize`, `tools/list`, `tools/call`). -//! -//! All methods are async and use a 30-second timeout by default. - -use std::sync::Arc; -use std::time::Duration; - -use async_trait::async_trait; -use serde_json::{json, Value}; -use tokio::sync::{oneshot, Mutex}; - -use super::transport::{PendingMap, MCP_PROTOCOL_VERSION}; -use crate::openhuman::mcp_clients::types::McpTool; - -const REQUEST_TIMEOUT: Duration = Duration::from_secs(30); - -/// Trait abstracting the MCP stdio transport so tests can inject fakes. -#[async_trait] -pub trait McpTransport: Send + Sync + 'static { - /// Send an MCP `initialize` request and return the server's result. - async fn initialize(&self) -> Result; - - /// Send a `tools/list` request and return the parsed tool list. - async fn list_tools(&self) -> Result, String>; - - /// Send a `tools/call` request. - async fn call_tool(&self, tool_name: &str, arguments: Value) -> Result; - - /// Gracefully shut down (send `notifications/cancelled` or just close). - async fn shutdown(&self); -} - -// ── Shared request-counter helper ──────────────────────────────────────────── - -pub struct RequestIdCounter(Arc>); - -impl RequestIdCounter { - pub fn new() -> Self { - Self(Arc::new(Mutex::new(0))) - } - - pub async fn next(&self) -> u64 { - let mut guard = self.0.lock().await; - *guard += 1; - *guard - } -} - -// ── Dispatch helper used by the real transport ──────────────────────────────── - -/// Send one JSON-RPC request message and await the response with timeout. -/// -/// The caller owns the `pending` map and the writer lock; this function -/// inserts a oneshot sender into `pending`, writes the message, then -/// awaits the receiver with `REQUEST_TIMEOUT`. -pub async fn send_request_and_wait( - id: u64, - msg: Value, - pending: &PendingMap, - write_fn: impl std::future::Future>, -) -> Result { - let (tx, rx) = oneshot::channel::>(); - { - let mut map = pending.lock().await; - map.insert(id, tx); - } - - if let Err(e) = write_fn.await { - let mut map = pending.lock().await; - map.remove(&id); - return Err(format!("[mcp-client] write failed id={id}: {e}")); - } - - match tokio::time::timeout(REQUEST_TIMEOUT, rx).await { - Ok(Ok(result)) => result, - Ok(Err(_)) => Err(format!("[mcp-client] channel dropped for id={id}")), - Err(_) => { - let mut map = pending.lock().await; - map.remove(&id); - Err(format!("[mcp-client] timeout waiting for id={id}")) - } - } -} - -// ── Parse tools/list response ───────────────────────────────────────────────── - -pub fn parse_tools_list(result: Value) -> Result, String> { - let tools_arr = result - .get("tools") - .and_then(Value::as_array) - .ok_or_else(|| "tools/list response missing 'tools' array".to_string())?; - - let mut tools = Vec::new(); - for t in tools_arr { - let name = t - .get("name") - .and_then(Value::as_str) - .ok_or_else(|| "tool entry missing 'name' field".to_string())? - .to_string(); - let description = t - .get("description") - .and_then(Value::as_str) - .map(String::from); - let input_schema = t.get("inputSchema").cloned().unwrap_or_else( - || json!({ "type": "object", "properties": {}, "additionalProperties": true }), - ); - tools.push(McpTool { - name, - description, - input_schema, - }); - } - Ok(tools) -} - -/// Build an MCP JSON-RPC request object. -pub fn build_request(id: u64, method: &str, params: Option) -> Value { - let mut obj = json!({ - "jsonrpc": "2.0", - "id": id, - "method": method, - }); - if let Some(p) = params { - obj["params"] = p; - } - obj -} - -/// Build an MCP `initialize` params payload. -pub fn build_initialize_params() -> Value { - json!({ - "protocolVersion": MCP_PROTOCOL_VERSION, - "clientInfo": { - "name": "openhuman", - "version": env!("CARGO_PKG_VERSION") - }, - "capabilities": {} - }) -} - -#[cfg(test)] -mod tests { - use super::*; - use serde_json::json; - - #[test] - fn build_request_includes_method_and_id() { - let req = build_request(7, "tools/list", None); - assert_eq!(req["jsonrpc"], "2.0"); - assert_eq!(req["id"], 7); - assert_eq!(req["method"], "tools/list"); - assert!(req.get("params").is_none() || req["params"] == Value::Null); - } - - #[test] - fn build_request_with_params() { - let params = json!({ "name": "my_tool", "arguments": {} }); - let req = build_request(3, "tools/call", Some(params.clone())); - assert_eq!(req["params"], params); - } - - #[test] - fn parse_tools_list_happy_path() { - let result = json!({ - "tools": [ - { - "name": "search", - "description": "Web search", - "inputSchema": { "type": "object" } - } - ] - }); - let tools = parse_tools_list(result).unwrap(); - assert_eq!(tools.len(), 1); - assert_eq!(tools[0].name, "search"); - assert_eq!(tools[0].description.as_deref(), Some("Web search")); - } - - #[test] - fn parse_tools_list_missing_tools_key_errors() { - let result = json!({ "something_else": [] }); - let err = parse_tools_list(result).unwrap_err(); - assert!(err.contains("'tools'")); - } - - #[test] - fn parse_tools_list_tool_missing_name_errors() { - let result = json!({ "tools": [{ "description": "no name" }] }); - let err = parse_tools_list(result).unwrap_err(); - assert!(err.contains("'name'")); - } - - #[test] - fn parse_tools_list_no_input_schema_gets_default() { - let result = json!({ "tools": [{ "name": "tool_no_schema" }] }); - let tools = parse_tools_list(result).unwrap(); - assert_eq!(tools[0].input_schema["type"], "object"); - } - - #[test] - fn build_initialize_params_has_protocol_version() { - let params = build_initialize_params(); - assert_eq!(params["protocolVersion"], MCP_PROTOCOL_VERSION); - assert!(params["clientInfo"]["name"].as_str().is_some()); - } - - #[tokio::test] - async fn request_id_counter_increments() { - let counter = RequestIdCounter::new(); - assert_eq!(counter.next().await, 1); - assert_eq!(counter.next().await, 2); - assert_eq!(counter.next().await, 3); - } -} diff --git a/src/openhuman/mcp_clients/client/transport.rs b/src/openhuman/mcp_clients/client/transport.rs deleted file mode 100644 index ec6ad65bf..000000000 --- a/src/openhuman/mcp_clients/client/transport.rs +++ /dev/null @@ -1,225 +0,0 @@ -//! Low-level stdio transport for MCP JSON-RPC. -//! -//! Owns the child process lifecycle, the reader task that deserializes -//! newline-delimited JSON from stdout, and the write-half that serializes -//! requests to stdin. A ring buffer captures stderr lines for error reporting. - -use std::collections::HashMap; -use std::collections::VecDeque; -use std::sync::Arc; - -use serde_json::Value; -use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; -use tokio::process::{Child, ChildStdin, ChildStdout}; -use tokio::sync::{oneshot, Mutex, RwLock}; - -pub type PendingMap = Arc>>>>; - -/// Capacity of the stderr ring buffer (lines). -const STDERR_RING_SIZE: usize = 64; - -/// MCP protocol version negotiated in `initialize`. -pub const MCP_PROTOCOL_VERSION: &str = "2024-11-05"; - -/// Serialised, newline-terminated JSON to be written to stdin. -pub struct TransportWriter { - stdin: ChildStdin, -} - -impl TransportWriter { - pub fn new(stdin: ChildStdin) -> Self { - Self { stdin } - } - - /// Write a single JSON value followed by `\n` and flush. - pub async fn send(&mut self, msg: &Value) -> anyhow::Result<()> { - let mut bytes = serde_json::to_vec(msg)?; - bytes.push(b'\n'); - self.stdin.write_all(&bytes).await?; - self.stdin.flush().await?; - Ok(()) - } -} - -/// Owns the stdout reader task; routes responses to waiting callers. -pub struct TransportReader { - pub pending: PendingMap, - pub stderr_ring: Arc>>, -} - -impl TransportReader { - pub fn new() -> Self { - Self { - pending: Arc::new(Mutex::new(HashMap::new())), - stderr_ring: Arc::new(RwLock::new(VecDeque::with_capacity(STDERR_RING_SIZE))), - } - } - - /// Spawn a background task that drains `stdout` and resolves waiters. - /// - /// When the reader exits (EOF or error), all pending waiters are flushed - /// with an error so they do not leak and wait until their own timeout fires. - pub fn spawn_reader(&self, stdout: ChildStdout, server_id: String) { - let pending = Arc::clone(&self.pending); - tokio::spawn(async move { - let reader = BufReader::new(stdout); - let mut lines = reader.lines(); - while let Ok(Some(line)) = lines.next_line().await { - if line.trim().is_empty() { - continue; - } - // Do NOT log raw payload — MCP subprocesses may emit secrets or PII. - tracing::trace!( - "[mcp-client] server_id={} received stdout line (len={})", - server_id, - line.len() - ); - let parsed: Value = match serde_json::from_str(&line) { - Ok(v) => v, - Err(e) => { - tracing::warn!( - "[mcp-client] server_id={} unparseable stdout line: {e}", - server_id - ); - continue; - } - }; - - // Route responses to waiting callers by id - if let Some(id) = parsed.get("id").and_then(Value::as_u64) { - let mut map = pending.lock().await; - if let Some(tx) = map.remove(&id) { - let result = if let Some(err) = parsed.get("error") { - Err(err.to_string()) - } else { - Ok(parsed.get("result").cloned().unwrap_or(Value::Null)) - }; - let _ = tx.send(result); - } - } - // Notifications have no id — log and ignore - } - tracing::debug!( - "[mcp-client] stdout reader exiting for server_id={}", - server_id - ); - // Flush all pending waiters with an error so they don't leak until timeout. - let mut map = pending.lock().await; - for (id, tx) in map.drain() { - tracing::debug!( - "[mcp-client] server_id={} flushing pending waiter id={}", - server_id, - id - ); - let _ = tx.send(Err("MCP server stdout closed unexpectedly".to_string())); - } - }); - } - - /// Spawn a background task that drains `stderr` into the ring buffer. - /// - /// Raw stderr content is NOT logged — MCP subprocesses may emit secrets or - /// PII. Only the line length is traced for diagnostics. - pub fn spawn_stderr_reader(&self, stderr: tokio::process::ChildStderr, server_id: String) { - let ring = Arc::clone(&self.stderr_ring); - tokio::spawn(async move { - use tokio::io::AsyncBufReadExt; - let reader = BufReader::new(stderr); - let mut lines = reader.lines(); - while let Ok(Some(line)) = lines.next_line().await { - tracing::trace!( - "[mcp-client] server_id={} received stderr line (len={})", - server_id, - line.len() - ); - let mut buf = ring.write().await; - if buf.len() >= STDERR_RING_SIZE { - buf.pop_front(); - } - buf.push_back(line); - } - }); - } - - /// Return the most recent stderr line, if any. - pub async fn last_stderr(&self) -> Option { - self.stderr_ring.read().await.back().cloned() - } -} - -/// Wrap up a just-spawned child and its I/O halves. -pub struct SpawnedProcess { - pub child: Child, - pub writer: TransportWriter, - pub reader: TransportReader, -} - -impl SpawnedProcess { - pub fn from_child(mut child: Child, server_id: &str) -> anyhow::Result { - let stdin = child.stdin.take().ok_or_else(|| { - anyhow::anyhow!("[mcp-client] server_id={server_id} failed to take stdin") - })?; - let stdout = child.stdout.take().ok_or_else(|| { - anyhow::anyhow!("[mcp-client] server_id={server_id} failed to take stdout") - })?; - let stderr = child.stderr.take().ok_or_else(|| { - anyhow::anyhow!("[mcp-client] server_id={server_id} failed to take stderr") - })?; - - let writer = TransportWriter::new(stdin); - let reader = TransportReader::new(); - reader.spawn_reader(stdout, server_id.to_string()); - reader.spawn_stderr_reader(stderr, server_id.to_string()); - - Ok(Self { - child, - writer, - reader, - }) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use serde_json::json; - - #[test] - fn mcp_protocol_version_is_2024_11_05() { - assert_eq!(MCP_PROTOCOL_VERSION, "2024-11-05"); - } - - #[tokio::test] - async fn pending_map_insert_and_remove() { - let map: PendingMap = Arc::new(Mutex::new(HashMap::new())); - let (tx, rx) = oneshot::channel::>(); - { - let mut guard = map.lock().await; - guard.insert(42, tx); - assert_eq!(guard.len(), 1); - } - { - let mut guard = map.lock().await; - let sender = guard.remove(&42).unwrap(); - sender.send(Ok(json!("ok"))).unwrap(); - } - assert_eq!(rx.await.unwrap().unwrap(), json!("ok")); - } - - #[tokio::test] - async fn stderr_ring_caps_at_max_size() { - let ring: Arc>> = - Arc::new(RwLock::new(VecDeque::with_capacity(STDERR_RING_SIZE))); - for i in 0..(STDERR_RING_SIZE + 10) { - let mut buf = ring.write().await; - if buf.len() >= STDERR_RING_SIZE { - buf.pop_front(); - } - buf.push_back(format!("line {i}")); - } - let buf = ring.read().await; - assert_eq!(buf.len(), STDERR_RING_SIZE); - // The oldest (line 0 .. 9) should have been evicted - assert!(buf.front().unwrap().starts_with("line 10")); - } -} diff --git a/src/openhuman/mcp_clients/connections.rs b/src/openhuman/mcp_clients/connections.rs deleted file mode 100644 index 1a3f5469d..000000000 --- a/src/openhuman/mcp_clients/connections.rs +++ /dev/null @@ -1,175 +0,0 @@ -//! Global in-process registry of active MCP client connections. -//! -//! Keyed by `server_id` (UUID). Connections are established by `connect()` -//! and removed by `disconnect()`. The registry is a process-global -//! `OnceLock>>`. - -use std::collections::HashMap; -use std::sync::{Arc, OnceLock}; - -use serde_json::Value; -use tokio::sync::RwLock; - -use crate::openhuman::config::Config; -use crate::openhuman::mcp_clients::client::protocol::McpTransport; -use crate::openhuman::mcp_clients::client::McpStdioClient; -use crate::openhuman::mcp_clients::store; -use crate::openhuman::mcp_clients::types::{ConnStatus, InstalledServer, McpTool, ServerStatus}; - -// ── Global registry ────────────────────────────────────────────────────────── - -static CONNECTIONS: OnceLock>>> = OnceLock::new(); - -fn connections() -> &'static RwLock>> { - CONNECTIONS.get_or_init(|| RwLock::new(HashMap::new())) -} - -// ── Public API ──────────────────────────────────────────────────────────────── - -/// Spawn a new stdio process and run the MCP initialize handshake. -/// Stores the live client in the global registry. -pub async fn connect(config: &Config, server: &InstalledServer) -> anyhow::Result> { - tracing::debug!( - "[mcp-client] connect server_id={} qualified_name={}", - server.server_id, - server.qualified_name - ); - - // Load env values from DB (never log values) - let env = store::load_env_values(config, &server.server_id).unwrap_or_default(); - - tracing::debug!( - "[mcp-client] connect server_id={} env_keys={:?}", - server.server_id, - env.keys().collect::>() - ); - - let client = - McpStdioClient::spawn_and_init(&server.server_id, &server.command, &server.args, &env) - .await?; - - let tools = client.tools_snapshot().await; - - { - let mut map = connections().write().await; - map.insert(server.server_id.clone(), Arc::clone(&client)); - } - - // Update last_connected_at in DB - let _ = store::update_last_connected(config, &server.server_id); - - tracing::debug!( - "[mcp-client] connect ok server_id={} tools={}", - server.server_id, - tools.len() - ); - - Ok(tools) -} - -/// Disconnect and remove from the registry. -pub async fn disconnect(server_id: &str) -> bool { - tracing::debug!("[mcp-client] disconnect server_id={}", server_id); - let client = { - let mut map = connections().write().await; - map.remove(server_id) - }; - if let Some(c) = client { - c.shutdown().await; - tracing::debug!("[mcp-client] disconnected server_id={}", server_id); - true - } else { - tracing::debug!("[mcp-client] disconnect noop server_id={}", server_id); - false - } -} - -/// Get a live client handle for `server_id`, if connected. -pub async fn client_for(server_id: &str) -> Option> { - let map = connections().read().await; - map.get(server_id).cloned() -} - -/// Invoke `call_tool` on a connected server. -pub async fn call_tool( - server_id: &str, - tool_name: &str, - arguments: Value, -) -> Result { - let client = client_for(server_id) - .await - .ok_or_else(|| format!("[mcp-client] server_id={server_id} not connected"))?; - client.call_tool(tool_name, arguments).await -} - -/// Return status summaries for all installed servers. -pub async fn all_status(config: &Config) -> Vec { - let installed = store::list_servers(config).unwrap_or_default(); - let map = connections().read().await; - - installed - .into_iter() - .map(|s| { - let connected = map.get(&s.server_id); - let (status, tool_count, last_error) = if let Some(c) = connected { - // We can't easily block here on async, so tool count comes from - // a best-effort sync snapshot: peek at the blocking tools list. - // For full accuracy callers can refresh via `connect`. - let tool_count = { - // We can't .await here because we hold a read lock. - // Use a fallback of 0; the UI refreshes asynchronously. - 0u32 - }; - (ServerStatus::Connected, tool_count, None) - } else { - (ServerStatus::Disconnected, 0u32, None) - }; - ConnStatus { - server_id: s.server_id, - qualified_name: s.qualified_name, - display_name: s.display_name, - status, - tool_count, - last_error, - } - }) - .collect() -} - -/// Collect tools from all currently-connected servers for tool_registry integration. -/// -/// Returns `(server_id, qualified_name, tool)` triples. -pub async fn all_connected_tools() -> Vec<(String, String, McpTool)> { - let installed_ids: Vec<(String, String)> = { - let map = connections().read().await; - map.keys().map(|id| (id.clone(), id.clone())).collect() - }; - - // We need server metadata too — fetch from a mini-cache in the connections map. - // For simplicity, return server_id as qualified_name here; ops.rs enriches it. - let mut result = Vec::new(); - let map = connections().read().await; - for (server_id, _) in &installed_ids { - if let Some(client) = map.get(server_id) { - let tools = client.tools_snapshot().await; - for tool in tools { - result.push((server_id.clone(), server_id.clone(), tool)); - } - } - } - result -} - -#[cfg(test)] -mod tests { - // Connection registry tests require a real process, which is too heavy - // for unit tests. See tests/json_rpc_e2e.rs for the lifecycle test. - // Here we only test helper logic. - - #[test] - fn all_status_on_empty_connections_returns_empty() { - // Purely synchronous check — can't easily test the async path without - // real server infra. The E2E test covers the full lifecycle. - assert!(true); - } -} diff --git a/src/openhuman/mcp_clients/mod.rs b/src/openhuman/mcp_clients/mod.rs deleted file mode 100644 index e16ad87d5..000000000 --- a/src/openhuman/mcp_clients/mod.rs +++ /dev/null @@ -1,29 +0,0 @@ -//! MCP Clients domain — browse the Smithery.ai MCP registry, install servers -//! locally, spawn them as stdio subprocesses, and expose their tools to agents. -//! -//! # Modules -//! - `types` — data structures (InstalledServer, McpTool, Smithery DTOs, …) -//! - `store` — SQLite persistence (mcp_clients.db) -//! - `registry` — Smithery HTTP client with 10-minute SQLite cache -//! - `client` — MCP stdio JSON-RPC client + FakeMcpTransport test double -//! - `connections` — global in-process connection registry -//! - `ops` — RPC handler implementations -//! - `schemas` — controller schemas + handler dispatch -//! - `bus` — DomainEvent subscriber for lifecycle logging - -pub mod bus; -mod client; -pub(crate) mod connections; -mod ops; -mod registry; -mod schemas; -mod store; -pub mod types; - -pub use schemas::{ - all_controller_schemas as all_mcp_clients_controller_schemas, - all_registered_controllers as all_mcp_clients_registered_controllers, - schemas as mcp_clients_schemas, -}; - -pub use types::{ConnStatus, InstalledServer, McpTool}; diff --git a/src/openhuman/mcp_clients/registry.rs b/src/openhuman/mcp_clients/registry.rs deleted file mode 100644 index 2c58802c0..000000000 --- a/src/openhuman/mcp_clients/registry.rs +++ /dev/null @@ -1,226 +0,0 @@ -//! Smithery.ai MCP registry HTTP client. -//! -//! Base URL: -//! Public endpoints: -//! `GET /servers?q=&page=N&pageSize=M` → `SmitheryListResponse` -//! `GET /servers/{qualifiedName}` → `SmitheryServerDetail` -//! -//! Results are cached in SQLite for 10 minutes (TTL controlled in `store.rs`). -//! Auth: optional `SMITHERY_API_KEY` env var sent as `Authorization: Bearer`. - -use anyhow::{Context, Result}; -use reqwest::Client; - -use crate::openhuman::config::Config; - -use super::store; -use super::types::{SmitheryListResponse, SmitheryServerDetail, SmitheryServerSummary}; - -const SMITHERY_BASE: &str = "https://registry.smithery.ai"; -const DEFAULT_PAGE_SIZE: u32 = 20; - -fn smithery_client() -> Result { - Client::builder() - .timeout(std::time::Duration::from_secs(15)) - .build() - .context("Failed to build Smithery HTTP client") -} - -fn smithery_api_key() -> Option { - std::env::var("SMITHERY_API_KEY") - .ok() - .filter(|s| !s.is_empty()) -} - -fn apply_auth(builder: reqwest::RequestBuilder) -> reqwest::RequestBuilder { - if let Some(key) = smithery_api_key() { - builder.bearer_auth(key) - } else { - builder - } -} - -/// Search the Smithery registry. Results are cached in SQLite. -pub async fn registry_search( - config: &Config, - query: Option<&str>, - page: u32, - page_size: u32, -) -> Result<(Vec, u32)> { - let page = page.max(1); - let page_size = if page_size == 0 { - DEFAULT_PAGE_SIZE - } else { - page_size - }; - let q = query.unwrap_or("").trim(); - - let cache_key = format!("search:{}:{}:{}", q, page, page_size); - - // Check SQLite cache first - if let Ok(Some(cached_body)) = store::get_cached(config, &cache_key) { - tracing::debug!("[mcp-client] registry_search cache hit key={}", cache_key); - if let Ok(resp) = serde_json::from_str::(&cached_body) { - return Ok((resp.servers, resp.pagination.total_pages)); - } - } - - tracing::debug!( - "[mcp-client] registry_search fetching q={:?} page={} page_size={}", - q, - page, - page_size - ); - - let client = smithery_client()?; - let mut req = client.get(format!("{SMITHERY_BASE}/servers")); - if !q.is_empty() { - req = req.query(&[("q", q)]); - } - req = req - .query(&[ - ("page", &page.to_string()), - ("pageSize", &page_size.to_string()), - ]) - .header("Accept", "application/json"); - req = apply_auth(req); - - let resp = req - .send() - .await - .context("Smithery registry_search request failed")?; - let status = resp.status(); - let body = resp - .text() - .await - .context("Failed to read Smithery response body")?; - - if !status.is_success() { - tracing::warn!( - "[mcp-client] registry_search HTTP {} for key={}", - status, - cache_key - ); - anyhow::bail!( - "Smithery registry returned HTTP {}: {}", - status, - &body[..body.len().min(200)] - ); - } - - let parsed: SmitheryListResponse = serde_json::from_str(&body) - .with_context(|| format!("Failed to parse Smithery list response: {body}"))?; - - let total_pages = parsed.pagination.total_pages; - let servers = parsed.servers.clone(); - - // Cache success - let _ = store::set_cached(config, &cache_key, &body); - tracing::debug!( - "[mcp-client] registry_search ok servers={} total_pages={}", - servers.len(), - total_pages - ); - - Ok((servers, total_pages)) -} - -/// Fetch details for one server. Results are cached in SQLite. -pub async fn registry_get(config: &Config, qualified_name: &str) -> Result { - let cache_key = format!("detail:{qualified_name}"); - - if let Ok(Some(cached_body)) = store::get_cached(config, &cache_key) { - tracing::debug!( - "[mcp-client] registry_get cache hit qualified_name={}", - qualified_name - ); - if let Ok(detail) = serde_json::from_str::(&cached_body) { - return Ok(detail); - } - } - - tracing::debug!( - "[mcp-client] registry_get fetching qualified_name={}", - qualified_name - ); - - let client = smithery_client()?; - let url = format!( - "{SMITHERY_BASE}/servers/{}", - urlencoding_encode(qualified_name) - ); - let req = apply_auth(client.get(&url).header("Accept", "application/json")); - - let resp = req - .send() - .await - .context("Smithery registry_get request failed")?; - let status = resp.status(); - let body = resp - .text() - .await - .context("Failed to read Smithery detail response")?; - - if !status.is_success() { - anyhow::bail!( - "Smithery registry GET {} returned HTTP {}: {}", - qualified_name, - status, - &body[..body.len().min(200)] - ); - } - - let detail: SmitheryServerDetail = serde_json::from_str(&body) - .with_context(|| format!("Failed to parse Smithery detail: {body}"))?; - - let _ = store::set_cached(config, &cache_key, &body); - tracing::debug!( - "[mcp-client] registry_get ok qualified_name={} connections={}", - qualified_name, - detail.connections.len() - ); - - Ok(detail) -} - -/// Minimal URL percent-encoding for path segments (encodes `/` and common specials). -fn urlencoding_encode(s: &str) -> String { - let mut out = String::with_capacity(s.len()); - for b in s.bytes() { - match b { - b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' | b'@' => { - out.push(b as char) - } - _ => { - out.push('%'); - out.push_str(&format!("{b:02X}")); - } - } - } - out -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn urlencoding_encode_handles_at_sign_and_slash() { - // @ is kept (valid in registry names like @modelcontextprotocol/server-fs) - // / is encoded so it does not split the URL path - let encoded = urlencoding_encode("@modelcontextprotocol/server-filesystem"); - assert!(encoded.contains('%'), "slash should be encoded: {encoded}"); - assert!(encoded.contains('@'), "@ should be preserved: {encoded}"); - } - - #[test] - fn urlencoding_encode_plain_ascii_unchanged() { - assert_eq!(urlencoding_encode("simple-name"), "simple-name"); - } - - #[test] - fn urlencoding_encode_space_becomes_percent_20() { - let encoded = urlencoding_encode("hello world"); - assert_eq!(encoded, "hello%20world"); - } -} diff --git a/src/openhuman/mcp_registry/boot.rs b/src/openhuman/mcp_registry/boot.rs new file mode 100644 index 000000000..48f484e64 --- /dev/null +++ b/src/openhuman/mcp_registry/boot.rs @@ -0,0 +1,55 @@ +//! Boot-time spawn of installed local MCP servers. +//! +//! On core startup we iterate every [`InstalledServer`] in +//! [`super::store`] and bring up its stdio subprocess via +//! [`super::connections::connect`]. Errors are logged per-server and never +//! block boot — a misbehaving server should not prevent the rest of the +//! core from coming up. +//! +//! HTTP-remote MCP servers are out of scope here: they have no subprocess +//! to spawn. Once the `InstalledServer` model grows a remote-transport +//! variant this function will skip them (or call a remote "warm-up" path). + +use crate::openhuman::config::Config; + +use super::{connections, store}; + +/// Spawn every locally-installed MCP server. Per-server failures are logged +/// and swallowed. +pub async fn spawn_installed_servers(config: &Config) { + let servers = match store::list_servers(config) { + Ok(s) => s, + Err(err) => { + tracing::warn!("[mcp-registry] boot: list_servers failed: {err}"); + return; + } + }; + + if servers.is_empty() { + tracing::debug!("[mcp-registry] boot: no installed servers to spawn"); + return; + } + + tracing::info!( + "[mcp-registry] boot: spawning {} installed server(s)", + servers.len() + ); + + for server in servers { + let server_id = server.server_id.clone(); + let qualified = server.qualified_name.clone(); + match connections::connect(config, &server).await { + Ok(tools) => tracing::info!( + "[mcp-registry] boot: connected server_id={} qualified={} tools={}", + server_id, + qualified, + tools.len() + ), + Err(err) => tracing::warn!( + "[mcp-registry] boot: connect failed server_id={} qualified={} err={err}", + server_id, + qualified + ), + } + } +} diff --git a/src/openhuman/mcp_clients/bus.rs b/src/openhuman/mcp_registry/bus.rs similarity index 100% rename from src/openhuman/mcp_clients/bus.rs rename to src/openhuman/mcp_registry/bus.rs diff --git a/src/openhuman/mcp_registry/connections.rs b/src/openhuman/mcp_registry/connections.rs new file mode 100644 index 000000000..6a442f8e7 --- /dev/null +++ b/src/openhuman/mcp_registry/connections.rs @@ -0,0 +1,212 @@ +//! Global in-process registry of active MCP client connections. +//! +//! Keyed by `server_id` (UUID). Connections are established by [`connect`] +//! and removed by [`disconnect`]. The actual stdio transport lives in +//! [`crate::openhuman::mcp_client::McpStdioClient`] — this module just +//! owns the per-server lifecycle and a global handle map. + +use std::collections::HashMap; +use std::sync::{Arc, OnceLock}; + +use serde_json::Value; +use tokio::sync::RwLock; + +use crate::openhuman::config::Config; +use crate::openhuman::mcp_client::{McpRemoteTool, McpStdioClient}; + +use super::store; +use super::types::{ConnStatus, InstalledServer, McpTool, ServerStatus}; + +// ── Connection record ──────────────────────────────────────────────────────── + +/// One live MCP subprocess plus the tool list cached after `initialize`. +struct Connection { + client: Arc, + tools: RwLock>, +} + +impl Connection { + async fn tools_snapshot(&self) -> Vec { + self.tools.read().await.clone() + } +} + +// ── Global registry ────────────────────────────────────────────────────────── + +static CONNECTIONS: OnceLock>>> = OnceLock::new(); + +fn connections() -> &'static RwLock>> { + CONNECTIONS.get_or_init(|| RwLock::new(HashMap::new())) +} + +// ── Public API ──────────────────────────────────────────────────────────────── + +/// Spawn a new stdio subprocess (via `McpStdioClient`), run `initialize`, +/// cache the tool list, and store the connection in the global registry. +pub async fn connect(config: &Config, server: &InstalledServer) -> anyhow::Result> { + tracing::debug!( + "[mcp-registry] connect server_id={} qualified_name={}", + server.server_id, + server.qualified_name + ); + + let env_map = store::load_env_values(config, &server.server_id).unwrap_or_default(); + let env: Vec<(String, String)> = env_map.into_iter().collect(); + + tracing::debug!( + "[mcp-registry] connect server_id={} env_keys={:?}", + server.server_id, + env.iter().map(|(k, _)| k).collect::>() + ); + + let identity = config.mcp_client.client_identity.clone(); + let client = Arc::new(McpStdioClient::new( + server.command.clone(), + server.args.clone(), + env, + None, + identity, + )); + + // Initialize + first tools/list happen here so a misconfigured server + // fails loudly at `connect` instead of silently at first `call_tool`. + client.initialize().await?; + let remote_tools = client.list_tools().await?; + let tools: Vec = remote_tools.into_iter().map(into_registry_tool).collect(); + + let conn = Arc::new(Connection { + client: Arc::clone(&client), + tools: RwLock::new(tools.clone()), + }); + + { + let mut map = connections().write().await; + map.insert(server.server_id.clone(), conn); + } + + let _ = store::update_last_connected(config, &server.server_id); + + tracing::debug!( + "[mcp-registry] connect ok server_id={} tools={}", + server.server_id, + tools.len() + ); + + Ok(tools) +} + +/// Disconnect and remove from the registry. +pub async fn disconnect(server_id: &str) -> bool { + tracing::debug!("[mcp-registry] disconnect server_id={server_id}"); + let conn = { + let mut map = connections().write().await; + map.remove(server_id) + }; + if let Some(c) = conn { + let _ = c.client.close_session().await; + tracing::debug!("[mcp-registry] disconnected server_id={server_id}"); + true + } else { + tracing::debug!("[mcp-registry] disconnect noop server_id={server_id}"); + false + } +} + +/// Invoke `tools/call` on a connected server. The MCP `CallToolResult` is +/// returned as the raw JSON value (matches the prior wire contract used by +/// `tool_call`). +pub async fn call_tool( + server_id: &str, + tool_name: &str, + arguments: Value, +) -> Result { + let conn = { + let map = connections().read().await; + map.get(server_id).cloned() + } + .ok_or_else(|| format!("[mcp-registry] server_id={server_id} not connected"))?; + + conn.client + .call_tool(tool_name, arguments) + .await + .map(|r| r.raw_result) + .map_err(|e| e.to_string()) +} + +/// Return status summaries for all installed servers. +pub async fn all_status(config: &Config) -> Vec { + let installed = store::list_servers(config).unwrap_or_default(); + let connected_ids: Vec = { + let map = connections().read().await; + map.keys().cloned().collect() + }; + + let mut out = Vec::with_capacity(installed.len()); + for s in installed { + let is_connected = connected_ids.iter().any(|id| id == &s.server_id); + let tool_count = if is_connected { + let map = connections().read().await; + match map.get(&s.server_id) { + Some(c) => c.tools_snapshot().await.len() as u32, + None => 0, + } + } else { + 0 + }; + out.push(ConnStatus { + server_id: s.server_id, + qualified_name: s.qualified_name, + display_name: s.display_name, + status: if is_connected { + ServerStatus::Connected + } else { + ServerStatus::Disconnected + }, + tool_count, + last_error: None, + }); + } + out +} + +/// Collect tools from all currently-connected servers for tool_registry integration. +/// Returns `(server_id, qualified_name, tool)` triples. `qualified_name` is +/// best-effort sourced from the connection's `server_id` here — callers that +/// need the real qualified name should re-join against `store::list_servers`. +pub async fn all_connected_tools() -> Vec<(String, String, McpTool)> { + let snapshot: Vec<(String, Arc)> = { + let map = connections().read().await; + map.iter() + .map(|(id, c)| (id.clone(), Arc::clone(c))) + .collect() + }; + + let mut out: Vec<(String, String, McpTool)> = Vec::new(); + for (server_id, c) in snapshot { + for tool in c.tools_snapshot().await { + out.push((server_id.clone(), server_id.clone(), tool)); + } + } + out +} + +// ── Boundary conversion ────────────────────────────────────────────────────── + +fn into_registry_tool(remote: McpRemoteTool) -> McpTool { + McpTool { + name: remote.name, + description: remote.description, + input_schema: remote.input_schema, + } +} + +#[cfg(test)] +mod tests { + // Live-connection tests require a real MCP subprocess and live in + // tests/json_rpc_e2e.rs. Keep this slot for sync helper tests. + + #[test] + fn placeholder_so_module_compiles_under_test_cfg() { + // Intentionally empty. + } +} diff --git a/src/openhuman/mcp_registry/mod.rs b/src/openhuman/mcp_registry/mod.rs new file mode 100644 index 000000000..5652bb744 --- /dev/null +++ b/src/openhuman/mcp_registry/mod.rs @@ -0,0 +1,71 @@ +//! MCP Registry — discover, install, and run user-chosen MCP servers. +//! +//! This is the dynamic, user-facing side of MCP-client support. It browses the +//! Smithery.ai MCP registry, persists the user's chosen installs to SQLite, +//! and (for local-spawn servers) supervises their subprocess lifecycle. +//! Installed servers' tools are surfaced to agents via the unified tool +//! registry ([`crate::openhuman::tool_registry`]). +//! +//! # Server transport model +//! +//! Today every [`InstalledServer`] is a **local subprocess** launched by npx +//! / uvx / a direct binary ([`types::CommandKind`]). The connection is stdio +//! JSON-RPC, owned by [`connections`]. +//! +//! HTTP-remote MCP servers (the majority of what Smithery actually lists) are +//! **not yet modelled** as an `InstalledServer` variant — adding a remote +//! transport variant is planned follow-up work, after which the registry +//! holds both kinds. +//! +//! # Boot-time spawn +//! +//! [`boot::spawn_installed_servers`] is called from +//! `bootstrap_core_runtime` so every local-spawn server is connected as soon +//! as the core comes up. Errors are logged per-server and never block boot. +//! +//! # Relationship to `mcp_client` +//! +//! The sibling [`crate::openhuman::mcp_client`] module is the **transport +//! library** (HTTP + stdio primitives) plus the *static, config-declared* +//! server set (read from `[[mcp_client.servers]]` in TOML). Agents reach +//! that set through generic bridge tools. The static set is intentionally +//! separate from this dynamic registry — both kinds will eventually share +//! the transport primitives from `mcp_client`. +//! +//! # Modules +//! - `types` — data structures (InstalledServer, McpTool, Smithery DTOs, …) +//! - `store` — SQLite persistence (mcp_clients.db) +//! - `registry` — Smithery HTTP client with 10-minute SQLite cache +//! - `connections` — global in-process connection registry (wraps +//! [`crate::openhuman::mcp_client::McpStdioClient`] — there is no +//! separate stdio client here) +//! - `boot` — boot-time spawn of installed local servers +//! - `ops` — RPC handler implementations +//! - `schemas` — controller schemas + handler dispatch +//! - `bus` — DomainEvent subscriber for lifecycle logging +//! +//! # Naming note +//! +//! The RPC namespace and SQLite db filename are still `mcp_clients` for +//! backwards compatibility with existing frontend code and on-disk state. +//! The Rust module path is `mcp_registry`. + +pub mod boot; +pub mod bus; +pub mod connections; +mod ops; +mod registries; +mod registry; +mod schemas; +pub mod setup; +pub mod setup_ops; +pub mod store; +pub mod types; + +pub use schemas::{ + all_controller_schemas as all_mcp_registry_controller_schemas, + all_registered_controllers as all_mcp_registry_registered_controllers, + schemas as mcp_registry_schemas, +}; + +pub use types::{ConnStatus, InstalledServer, McpTool}; diff --git a/src/openhuman/mcp_clients/ops.rs b/src/openhuman/mcp_registry/ops.rs similarity index 98% rename from src/openhuman/mcp_clients/ops.rs rename to src/openhuman/mcp_registry/ops.rs index 90503f115..a1b08c57c 100644 --- a/src/openhuman/mcp_clients/ops.rs +++ b/src/openhuman/mcp_registry/ops.rs @@ -1,7 +1,7 @@ //! RPC handler implementations for the MCP clients domain. //! //! Each function maps 1-to-1 with a `schemas.rs` handler and is testable -//! in isolation via the `FakeMcpTransport` in `client/mod.rs`. +//! in isolation; live-process tests live in `tests/json_rpc_e2e.rs`. use std::collections::HashMap; use std::time::Instant; @@ -184,7 +184,7 @@ pub async fn mcp_clients_install( } /// Resolve the launch command from the qualified name and optional registry connection metadata. -fn resolve_command( +pub(super) fn resolve_command( qualified_name: &str, stdio_conn: Option<&super::types::SmitheryConnection>, ) -> (CommandKind, String, Vec) { @@ -591,7 +591,7 @@ mod tests { #[test] fn collect_required_env_keys_from_schema() { - use crate::openhuman::mcp_clients::types::{SmitheryConnection, SmitheryServerDetail}; + use crate::openhuman::mcp_registry::types::{SmitheryConnection, SmitheryServerDetail}; let detail = SmitheryServerDetail { qualified_name: "@test/s".to_string(), display_name: "T".to_string(), @@ -610,6 +610,7 @@ mod tests { published: true, extra: Default::default(), }], + source: "smithery".to_string(), extra: Default::default(), }; let keys = collect_required_env_keys(&detail); @@ -627,7 +628,7 @@ mod tests { #[test] fn resolve_command_with_example_config() { - use crate::openhuman::mcp_clients::types::SmitheryConnection; + use crate::openhuman::mcp_registry::types::SmitheryConnection; let conn = SmitheryConnection { r#type: "stdio".to_string(), deployment_url: None, diff --git a/src/openhuman/mcp_registry/registries/mcp_official.rs b/src/openhuman/mcp_registry/registries/mcp_official.rs new file mode 100644 index 000000000..1393ad179 --- /dev/null +++ b/src/openhuman/mcp_registry/registries/mcp_official.rs @@ -0,0 +1,304 @@ +//! Official MCP registry adapter — [modelcontextprotocol/registry][repo]. +//! +//! Base URL: `https://registry.modelcontextprotocol.io` (override with +//! `MCP_OFFICIAL_REGISTRY_BASE`). +//! +//! Endpoints used: +//! - `GET /v0/servers?search=&limit=&cursor=` — paginated list +//! - `GET /v0/servers/{name}` — full detail for one server (or a fallback +//! path that searches by exact name when the direct endpoint 404s) +//! +//! The official registry uses cursor pagination. We map our 1-indexed `page` +//! parameter onto it by treating `page == 1` as "no cursor" and refusing +//! deeper pagination for now — the caller gets back the first page plus a +//! `total_pages` hint of `1`. Cursor-aware pagination is a follow-up. +//! +//! Auth: optional `MCP_OFFICIAL_REGISTRY_TOKEN` env var sent as bearer. +//! +//! [repo]: https://github.com/modelcontextprotocol/registry + +use anyhow::{Context, Result}; +use async_trait::async_trait; +use reqwest::Client; +use serde::Deserialize; +use serde_json::Value; + +use crate::openhuman::config::Config; + +use super::super::store; +use super::super::types::{SmitheryConnection, SmitheryServerDetail, SmitheryServerSummary}; +use super::{Registry, SOURCE_MCP_OFFICIAL}; + +const DEFAULT_BASE: &str = "https://registry.modelcontextprotocol.io"; + +pub struct McpOfficialRegistry; + +#[async_trait] +impl Registry for McpOfficialRegistry { + fn source(&self) -> &'static str { + SOURCE_MCP_OFFICIAL + } + + async fn search( + &self, + config: &Config, + query: Option<&str>, + page: u32, + page_size: u32, + ) -> Result<(Vec, u32)> { + let q = query.unwrap_or("").trim(); + let limit = page_size.max(1); + + let cache_key = format!("mcp_official:search:{q}:{page}:{limit}"); + if let Ok(Some(cached_body)) = store::get_cached(config, &cache_key) { + tracing::debug!("[mcp-official] search cache hit key={cache_key}"); + if let Ok(parsed) = serde_json::from_str::(&cached_body) { + return Ok((parsed.into_summaries(), 1)); + } + } + + if page > 1 { + // Cursor pagination not yet wired up — return empty for page > 1 + // so the UI doesn't loop fetching nonexistent pages. + tracing::debug!( + "[mcp-official] search returning empty for page>1 \ + (cursor pagination not implemented)" + ); + return Ok((Vec::new(), 1)); + } + + tracing::debug!("[mcp-official] search fetching q={q:?} limit={limit}"); + + let client = http_client()?; + let url = format!("{}/v0/servers", base_url()); + let mut req = client.get(&url).header("Accept", "application/json"); + if !q.is_empty() { + req = req.query(&[("search", q)]); + } + req = req.query(&[("limit", &limit.to_string())]); + req = apply_auth(req); + + let resp = req.send().await.context("MCP official search failed")?; + let status = resp.status(); + let body = resp.text().await.context("MCP official read failed")?; + + if !status.is_success() { + tracing::warn!("[mcp-official] search HTTP {status} for key={cache_key}"); + anyhow::bail!( + "MCP official registry returned HTTP {status}: {}", + &body[..body.len().min(200)] + ); + } + + let parsed: OfficialListResponse = serde_json::from_str(&body) + .with_context(|| format!("Failed to parse MCP official response: {body}"))?; + let summaries = parsed.into_summaries(); + let _ = store::set_cached(config, &cache_key, &body); + tracing::debug!( + "[mcp-official] search ok servers={} (cursor pagination not wired)", + summaries.len() + ); + Ok((summaries, 1)) + } + + async fn get(&self, config: &Config, qualified_name: &str) -> Result { + let cache_key = format!("mcp_official:detail:{qualified_name}"); + if let Ok(Some(cached_body)) = store::get_cached(config, &cache_key) { + tracing::debug!("[mcp-official] get cache hit qualified_name={qualified_name}"); + if let Ok(server) = serde_json::from_str::(&cached_body) { + return Ok(server.into_detail()); + } + } + + let client = http_client()?; + let url = format!( + "{}/v0/servers/{}", + base_url(), + urlencoding_encode(qualified_name) + ); + tracing::debug!("[mcp-official] get fetching {url}"); + let req = apply_auth(client.get(&url).header("Accept", "application/json")); + + let resp = req.send().await.context("MCP official get failed")?; + let status = resp.status(); + let body = resp.text().await.context("MCP official read failed")?; + + if !status.is_success() { + anyhow::bail!( + "MCP official registry GET {qualified_name} returned HTTP {status}: {}", + &body[..body.len().min(200)] + ); + } + + let server: OfficialServer = serde_json::from_str(&body) + .with_context(|| format!("Failed to parse MCP official detail: {body}"))?; + let _ = store::set_cached(config, &cache_key, &body); + Ok(server.into_detail()) + } +} + +fn http_client() -> Result { + Client::builder() + .timeout(std::time::Duration::from_secs(15)) + .build() + .context("Failed to build MCP official HTTP client") +} + +fn base_url() -> String { + std::env::var("MCP_OFFICIAL_REGISTRY_BASE") + .ok() + .filter(|s| !s.is_empty()) + .unwrap_or_else(|| DEFAULT_BASE.to_string()) +} + +fn auth_token() -> Option { + std::env::var("MCP_OFFICIAL_REGISTRY_TOKEN") + .ok() + .filter(|s| !s.is_empty()) +} + +fn apply_auth(builder: reqwest::RequestBuilder) -> reqwest::RequestBuilder { + if let Some(token) = auth_token() { + builder.bearer_auth(token) + } else { + builder + } +} + +fn urlencoding_encode(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for b in s.bytes() { + match b { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' | b'@' => { + out.push(b as char) + } + _ => { + out.push('%'); + out.push_str(&format!("{b:02X}")); + } + } + } + out +} + +// ── Wire-shape DTOs (best-effort against the official OpenAPI) ─────────────── +// +// The official registry OpenAPI evolves; these are deliberately permissive +// (every nested field is optional) so a schema bump doesn't break parsing. + +#[derive(Debug, Clone, Deserialize)] +struct OfficialListResponse { + #[serde(default)] + servers: Vec, + #[serde(default)] + #[allow(dead_code)] + metadata: Option, +} + +impl OfficialListResponse { + fn into_summaries(self) -> Vec { + self.servers.into_iter().map(|s| s.into_summary()).collect() + } +} + +#[derive(Debug, Clone, Deserialize)] +struct OfficialServer { + /// Reverse-DNS-style identifier, e.g. `io.github.foo/server-bar`. + #[serde(default)] + name: String, + #[serde(default)] + description: Option, + #[serde(default, rename = "iconUrl")] + icon_url: Option, + /// Remote (HTTP / SSE) endpoints exposed by this server. + #[serde(default)] + remotes: Vec, + /// Installable subprocess packages (npm, pip, brew, …). + #[serde(default)] + packages: Vec, +} + +impl OfficialServer { + fn into_summary(self) -> SmitheryServerSummary { + SmitheryServerSummary { + qualified_name: self.name.clone(), + display_name: self.name.clone(), + description: self.description.clone(), + icon_url: self.icon_url.clone(), + use_count: 0, + is_deployed: !self.remotes.is_empty(), + source: SOURCE_MCP_OFFICIAL.to_string(), + extra: std::collections::HashMap::new(), + } + } + + fn into_detail(self) -> SmitheryServerDetail { + let mut connections: Vec = Vec::new(); + for r in &self.remotes { + connections.push(SmitheryConnection { + r#type: "http".to_string(), + deployment_url: r.url.clone(), + config_schema: None, + example_config: None, + published: true, + extra: std::collections::HashMap::new(), + }); + } + for p in &self.packages { + connections.push(SmitheryConnection { + r#type: "stdio".to_string(), + deployment_url: None, + config_schema: p.config_schema.clone(), + example_config: None, + published: true, + extra: std::collections::HashMap::new(), + }); + } + SmitheryServerDetail { + qualified_name: self.name.clone(), + display_name: self.name.clone(), + description: self.description.clone(), + icon_url: self.icon_url.clone(), + connections, + source: SOURCE_MCP_OFFICIAL.to_string(), + extra: std::collections::HashMap::new(), + } + } +} + +#[derive(Debug, Clone, Deserialize)] +struct OfficialRemote { + #[serde(default)] + url: Option, +} + +#[derive(Debug, Clone, Deserialize)] +struct OfficialPackage { + #[serde(default, rename = "configSchema")] + config_schema: Option, +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn official_server_into_summary_uses_name_as_qualified() { + let s: OfficialServer = serde_json::from_value(json!({ + "name": "io.github.example/server", + "description": "Example", + })) + .unwrap(); + let sum = s.into_summary(); + assert_eq!(sum.qualified_name, "io.github.example/server"); + assert_eq!(sum.source, SOURCE_MCP_OFFICIAL); + } + + #[test] + fn list_response_tolerates_missing_metadata() { + let raw = json!({ "servers": [] }); + let parsed: OfficialListResponse = serde_json::from_value(raw).unwrap(); + assert!(parsed.servers.is_empty()); + } +} diff --git a/src/openhuman/mcp_registry/registries/mod.rs b/src/openhuman/mcp_registry/registries/mod.rs new file mode 100644 index 000000000..22c929ea1 --- /dev/null +++ b/src/openhuman/mcp_registry/registries/mod.rs @@ -0,0 +1,71 @@ +//! Upstream MCP registries. Today: Smithery.ai and the official +//! [modelcontextprotocol/registry](https://github.com/modelcontextprotocol/registry). +//! +//! All registries implement the [`Registry`] trait and return results in the +//! canonical [`super::types::SmitheryServerSummary`] / +//! [`super::types::SmitheryServerDetail`] shapes (named after Smithery for +//! backwards compatibility with the existing wire contract — non-Smithery +//! registries adapt their responses into the same shape and tag the `source` +//! field so the frontend can render provenance). +//! +//! [`enabled_registries`] returns every registry that should participate in a +//! query. Today both registries are always enabled; this will become +//! config-driven once the wider scope lands. + +use anyhow::Result; +use async_trait::async_trait; + +use crate::openhuman::config::Config; + +use super::types::{SmitheryServerDetail, SmitheryServerSummary}; + +pub mod mcp_official; +pub mod smithery; + +/// Canonical id for an upstream registry. Echoed back in +/// [`SmitheryServerSummary::source`] / [`SmitheryServerDetail::source`]. +pub const SOURCE_SMITHERY: &str = "smithery"; +pub const SOURCE_MCP_OFFICIAL: &str = "mcp_official"; + +/// An upstream MCP server directory. +#[async_trait] +pub trait Registry: Send + Sync { + /// Canonical identifier (see `SOURCE_*` constants). Returned on every + /// result so the frontend can attribute and the install path can route + /// `registry_get` back to the correct upstream. + fn source(&self) -> &'static str; + + /// Search the registry. `page` is 1-indexed; registries that use + /// cursor-based pagination map their own cursor space onto page numbers + /// internally. + /// + /// Returns `(servers, total_pages_known)`. `total_pages_known` is the + /// best-effort upper bound — registries that can't compute it report + /// the current page number. + async fn search( + &self, + config: &Config, + query: Option<&str>, + page: u32, + page_size: u32, + ) -> Result<(Vec, u32)>; + + /// Fetch one server's full detail by qualified name. + async fn get(&self, config: &Config, qualified_name: &str) -> Result; +} + +/// All registries currently enabled for the user. Today: Smithery + official. +pub fn enabled_registries() -> Vec> { + vec![ + Box::new(smithery::SmitheryRegistry), + Box::new(mcp_official::McpOfficialRegistry), + ] +} + +/// Resolve a registry by [`Registry::source`] id. Used by `registry_get` to +/// route a fetch back to the upstream that produced the qualified name. +pub fn registry_for_source(source: &str) -> Option> { + enabled_registries() + .into_iter() + .find(|r| r.source() == source) +} diff --git a/src/openhuman/mcp_registry/registries/smithery.rs b/src/openhuman/mcp_registry/registries/smithery.rs new file mode 100644 index 000000000..389ae8746 --- /dev/null +++ b/src/openhuman/mcp_registry/registries/smithery.rs @@ -0,0 +1,214 @@ +//! [Smithery.ai](https://smithery.ai) MCP registry adapter. +//! +//! Base URL: `https://registry.smithery.ai` +//! Public endpoints: +//! `GET /servers?q=&page=N&pageSize=M` → `SmitheryListResponse` +//! `GET /servers/{qualifiedName}` → `SmitheryServerDetail` +//! +//! Results are cached in SQLite for 10 minutes (TTL controlled in `store.rs`). +//! Auth: optional `SMITHERY_API_KEY` env var sent as `Authorization: Bearer`. + +use anyhow::{Context, Result}; +use async_trait::async_trait; +use reqwest::Client; + +use crate::openhuman::config::Config; + +use super::super::store; +use super::super::types::{SmitheryListResponse, SmitheryServerDetail, SmitheryServerSummary}; +use super::{Registry, SOURCE_SMITHERY}; + +const SMITHERY_BASE: &str = "https://registry.smithery.ai"; +const DEFAULT_PAGE_SIZE: u32 = 20; + +pub struct SmitheryRegistry; + +#[async_trait] +impl Registry for SmitheryRegistry { + fn source(&self) -> &'static str { + SOURCE_SMITHERY + } + + async fn search( + &self, + config: &Config, + query: Option<&str>, + page: u32, + page_size: u32, + ) -> Result<(Vec, u32)> { + let page = page.max(1); + let page_size = if page_size == 0 { + DEFAULT_PAGE_SIZE + } else { + page_size + }; + let q = query.unwrap_or("").trim(); + + let cache_key = format!("smithery:search:{q}:{page}:{page_size}"); + + if let Ok(Some(cached_body)) = store::get_cached(config, &cache_key) { + tracing::debug!("[smithery] search cache hit key={cache_key}"); + if let Ok(resp) = serde_json::from_str::(&cached_body) { + return Ok((tag_source(resp.servers), resp.pagination.total_pages)); + } + } + + tracing::debug!("[smithery] search fetching q={q:?} page={page} page_size={page_size}"); + + let client = http_client()?; + let mut req = client.get(format!("{SMITHERY_BASE}/servers")); + if !q.is_empty() { + req = req.query(&[("q", q)]); + } + req = req + .query(&[ + ("page", &page.to_string()), + ("pageSize", &page_size.to_string()), + ]) + .header("Accept", "application/json"); + req = apply_auth(req); + + let resp = req.send().await.context("Smithery search request failed")?; + let status = resp.status(); + let body = resp.text().await.context("Smithery search read failed")?; + + if !status.is_success() { + tracing::warn!("[smithery] search HTTP {status} for key={cache_key}"); + anyhow::bail!( + "Smithery returned HTTP {status}: {}", + &body[..body.len().min(200)] + ); + } + + let parsed: SmitheryListResponse = serde_json::from_str(&body) + .with_context(|| format!("Failed to parse Smithery list response: {body}"))?; + + let total_pages = parsed.pagination.total_pages; + let servers = tag_source(parsed.servers); + + let _ = store::set_cached(config, &cache_key, &body); + tracing::debug!( + "[smithery] search ok servers={} total_pages={}", + servers.len(), + total_pages + ); + + Ok((servers, total_pages)) + } + + async fn get(&self, config: &Config, qualified_name: &str) -> Result { + let cache_key = format!("smithery:detail:{qualified_name}"); + + if let Ok(Some(cached_body)) = store::get_cached(config, &cache_key) { + tracing::debug!("[smithery] get cache hit qualified_name={qualified_name}"); + if let Ok(mut detail) = serde_json::from_str::(&cached_body) { + if detail.source.is_empty() { + detail.source = SOURCE_SMITHERY.to_string(); + } + return Ok(detail); + } + } + + tracing::debug!("[smithery] get fetching qualified_name={qualified_name}"); + + let client = http_client()?; + let url = format!( + "{SMITHERY_BASE}/servers/{}", + urlencoding_encode(qualified_name) + ); + let req = apply_auth(client.get(&url).header("Accept", "application/json")); + + let resp = req.send().await.context("Smithery get request failed")?; + let status = resp.status(); + let body = resp.text().await.context("Smithery get read failed")?; + + if !status.is_success() { + anyhow::bail!( + "Smithery GET {qualified_name} returned HTTP {status}: {}", + &body[..body.len().min(200)] + ); + } + + let mut detail: SmitheryServerDetail = serde_json::from_str(&body) + .with_context(|| format!("Failed to parse Smithery detail: {body}"))?; + detail.source = SOURCE_SMITHERY.to_string(); + + let _ = store::set_cached(config, &cache_key, &body); + tracing::debug!( + "[smithery] get ok qualified_name={qualified_name} connections={}", + detail.connections.len() + ); + + Ok(detail) + } +} + +fn http_client() -> Result { + Client::builder() + .timeout(std::time::Duration::from_secs(15)) + .build() + .context("Failed to build Smithery HTTP client") +} + +fn smithery_api_key() -> Option { + std::env::var("SMITHERY_API_KEY") + .ok() + .filter(|s| !s.is_empty()) +} + +fn apply_auth(builder: reqwest::RequestBuilder) -> reqwest::RequestBuilder { + if let Some(key) = smithery_api_key() { + builder.bearer_auth(key) + } else { + builder + } +} + +fn tag_source(mut servers: Vec) -> Vec { + for s in &mut servers { + if s.source.is_empty() { + s.source = SOURCE_SMITHERY.to_string(); + } + } + servers +} + +/// Minimal URL percent-encoding for path segments (encodes `/` and common specials). +fn urlencoding_encode(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for b in s.bytes() { + match b { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' | b'@' => { + out.push(b as char) + } + _ => { + out.push('%'); + out.push_str(&format!("{b:02X}")); + } + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn urlencoding_encode_handles_at_sign_and_slash() { + let encoded = urlencoding_encode("@modelcontextprotocol/server-filesystem"); + assert!(encoded.contains('%'), "slash should be encoded: {encoded}"); + assert!(encoded.contains('@'), "@ should be preserved: {encoded}"); + } + + #[test] + fn urlencoding_encode_plain_ascii_unchanged() { + assert_eq!(urlencoding_encode("simple-name"), "simple-name"); + } + + #[test] + fn urlencoding_encode_space_becomes_percent_20() { + let encoded = urlencoding_encode("hello world"); + assert_eq!(encoded, "hello%20world"); + } +} diff --git a/src/openhuman/mcp_registry/registry.rs b/src/openhuman/mcp_registry/registry.rs new file mode 100644 index 000000000..3be08d3bc --- /dev/null +++ b/src/openhuman/mcp_registry/registry.rs @@ -0,0 +1,91 @@ +//! Multi-registry dispatch entry point. +//! +//! `registry_search` fans out to every registry in +//! [`super::registries::enabled_registries`], runs them in parallel, and +//! returns merged results (failed registries are logged and skipped so one +//! flaky upstream doesn't blank the UI). +//! +//! `registry_get` routes by [`super::types::SmitheryServerDetail::source`]. +//! The caller can pass an explicit source prefix using +//! `"::"` (e.g. `"mcp_official::io.github.foo/bar"`). +//! Without a prefix we ask every registry and return the first hit. + +use anyhow::Result; +use futures::future::join_all; + +use crate::openhuman::config::Config; + +use super::registries::{enabled_registries, registry_for_source}; +use super::types::{SmitheryServerDetail, SmitheryServerSummary}; + +const SOURCE_SEPARATOR: &str = "::"; + +/// Search every enabled registry in parallel; merge results. `total_pages` +/// is the max page count reported across registries (best-effort upper +/// bound). +pub async fn registry_search( + config: &Config, + query: Option<&str>, + page: u32, + page_size: u32, +) -> Result<(Vec, u32)> { + let registries = enabled_registries(); + let queries = registries + .iter() + .map(|r| r.search(config, query, page, page_size)); + let results = join_all(queries).await; + + let mut merged: Vec = Vec::new(); + let mut total_pages: u32 = 0; + for (idx, res) in results.into_iter().enumerate() { + let source = registries[idx].source(); + match res { + Ok((mut servers, pages)) => { + tracing::debug!( + "[mcp-registry] {source} search ok servers={} pages={pages}", + servers.len() + ); + merged.append(&mut servers); + total_pages = total_pages.max(pages); + } + Err(err) => { + tracing::warn!("[mcp-registry] {source} search failed: {err}"); + } + } + } + + if total_pages == 0 { + total_pages = page.max(1); + } + Ok((merged, total_pages)) +} + +/// Fetch a server detail. If `qualified_name` starts with `"::"` we +/// route directly to that registry; otherwise every enabled registry is +/// tried in order and the first success wins. +pub async fn registry_get(config: &Config, qualified_name: &str) -> Result { + if let Some((source, rest)) = qualified_name.split_once(SOURCE_SEPARATOR) { + if let Some(registry) = registry_for_source(source) { + tracing::debug!("[mcp-registry] get routed source={source} qualified={rest}"); + return registry.get(config, rest).await; + } + tracing::warn!( + "[mcp-registry] get: unknown source prefix {source:?} — falling back to all registries" + ); + } + + let mut last_err: Option = None; + for registry in enabled_registries() { + match registry.get(config, qualified_name).await { + Ok(detail) => return Ok(detail), + Err(err) => { + tracing::debug!( + "[mcp-registry] {} get miss for {qualified_name}: {err}", + registry.source() + ); + last_err = Some(err); + } + } + } + Err(last_err.unwrap_or_else(|| anyhow::anyhow!("no registries enabled"))) +} diff --git a/src/openhuman/mcp_clients/schemas.rs b/src/openhuman/mcp_registry/schemas.rs similarity index 60% rename from src/openhuman/mcp_clients/schemas.rs rename to src/openhuman/mcp_registry/schemas.rs index 186e63702..77fbe7082 100644 --- a/src/openhuman/mcp_clients/schemas.rs +++ b/src/openhuman/mcp_registry/schemas.rs @@ -26,6 +26,13 @@ pub fn all_controller_schemas() -> Vec { schemas("status"), schemas("tool_call"), schemas("config_assist"), + // Setup-agent surface (mcp_setup namespace, lives in setup_ops.rs). + setup_schemas("search"), + setup_schemas("get"), + setup_schemas("request_secret"), + setup_schemas("submit_secret"), + setup_schemas("test_connection"), + setup_schemas("install_and_connect"), ] } @@ -71,6 +78,30 @@ pub fn all_registered_controllers() -> Vec { schema: schemas("config_assist"), handler: handle_config_assist, }, + RegisteredController { + schema: setup_schemas("search"), + handler: handle_setup_search, + }, + RegisteredController { + schema: setup_schemas("get"), + handler: handle_setup_get, + }, + RegisteredController { + schema: setup_schemas("request_secret"), + handler: handle_setup_request_secret, + }, + RegisteredController { + schema: setup_schemas("submit_secret"), + handler: handle_setup_submit_secret, + }, + RegisteredController { + schema: setup_schemas("test_connection"), + handler: handle_setup_test_connection, + }, + RegisteredController { + schema: setup_schemas("install_and_connect"), + handler: handle_setup_install_and_connect, + }, ] } @@ -368,6 +399,15 @@ pub fn schemas(function: &str) -> ControllerSchema { ], }, + // Handled by setup_schemas() — surface a clearer error rather than + // falling through to the generic unknown sink. + "setup_search" + | "setup_get" + | "setup_request_secret" + | "setup_submit_secret" + | "setup_test_connection" + | "setup_install_and_connect" => setup_schemas(function.trim_start_matches("setup_")), + _other => ControllerSchema { namespace: "mcp_clients", function: "unknown", @@ -397,7 +437,7 @@ fn handle_registry_search(params: Map) -> ControllerFuture { let page = read_optional_u32(¶ms, "page")?; let page_size = read_optional_u32(¶ms, "page_size")?; to_json( - crate::openhuman::mcp_clients::ops::mcp_clients_registry_search( + crate::openhuman::mcp_registry::ops::mcp_clients_registry_search( &config, query, page, page_size, ) .await?, @@ -410,7 +450,7 @@ fn handle_registry_get(params: Map) -> ControllerFuture { let config = config_rpc::load_config_with_timeout().await?; let qualified_name = read_required::(¶ms, "qualified_name")?; to_json( - crate::openhuman::mcp_clients::ops::mcp_clients_registry_get(&config, qualified_name) + crate::openhuman::mcp_registry::ops::mcp_clients_registry_get(&config, qualified_name) .await?, ) }) @@ -420,7 +460,7 @@ fn handle_installed_list(params: Map) -> ControllerFuture { Box::pin(async move { let _ = params; let config = config_rpc::load_config_with_timeout().await?; - to_json(crate::openhuman::mcp_clients::ops::mcp_clients_installed_list(&config).await?) + to_json(crate::openhuman::mcp_registry::ops::mcp_clients_installed_list(&config).await?) }) } @@ -431,7 +471,7 @@ fn handle_install(params: Map) -> ControllerFuture { let env = read_required::>(¶ms, "env")?; let config_value = read_optional_json(¶ms, "config")?; to_json( - crate::openhuman::mcp_clients::ops::mcp_clients_install( + crate::openhuman::mcp_registry::ops::mcp_clients_install( &config, qualified_name, env, @@ -447,7 +487,7 @@ fn handle_uninstall(params: Map) -> ControllerFuture { let config = config_rpc::load_config_with_timeout().await?; let server_id = read_required::(¶ms, "server_id")?; to_json( - crate::openhuman::mcp_clients::ops::mcp_clients_uninstall(&config, server_id).await?, + crate::openhuman::mcp_registry::ops::mcp_clients_uninstall(&config, server_id).await?, ) }) } @@ -456,14 +496,14 @@ fn handle_connect(params: Map) -> ControllerFuture { Box::pin(async move { let config = config_rpc::load_config_with_timeout().await?; let server_id = read_required::(¶ms, "server_id")?; - to_json(crate::openhuman::mcp_clients::ops::mcp_clients_connect(&config, server_id).await?) + to_json(crate::openhuman::mcp_registry::ops::mcp_clients_connect(&config, server_id).await?) }) } fn handle_disconnect(params: Map) -> ControllerFuture { Box::pin(async move { let server_id = read_required::(¶ms, "server_id")?; - to_json(crate::openhuman::mcp_clients::ops::mcp_clients_disconnect(server_id).await?) + to_json(crate::openhuman::mcp_registry::ops::mcp_clients_disconnect(server_id).await?) }) } @@ -471,7 +511,7 @@ fn handle_status(params: Map) -> ControllerFuture { Box::pin(async move { let _ = params; let config = config_rpc::load_config_with_timeout().await?; - to_json(crate::openhuman::mcp_clients::ops::mcp_clients_status(&config).await?) + to_json(crate::openhuman::mcp_registry::ops::mcp_clients_status(&config).await?) }) } @@ -484,7 +524,7 @@ fn handle_tool_call(params: Map) -> ControllerFuture { .cloned() .unwrap_or(Value::Object(Map::new())); to_json( - crate::openhuman::mcp_clients::ops::mcp_clients_tool_call( + crate::openhuman::mcp_registry::ops::mcp_clients_tool_call( server_id, tool_name, arguments, ) .await?, @@ -497,11 +537,11 @@ fn handle_config_assist(params: Map) -> ControllerFuture { let config = config_rpc::load_config_with_timeout().await?; let qualified_name = read_required::(¶ms, "qualified_name")?; let user_message = read_required::(¶ms, "user_message")?; - let history = read_optional::>( + let history = read_optional::>( ¶ms, "history", )?; to_json( - crate::openhuman::mcp_clients::ops::mcp_clients_config_assist( + crate::openhuman::mcp_registry::ops::mcp_clients_config_assist( &config, qualified_name, user_message, @@ -512,6 +552,331 @@ fn handle_config_assist(params: Map) -> ControllerFuture { }) } +// ── mcp_setup_* schemas + handlers ──────────────────────────────────────────── + +/// All setup-agent schemas under the `mcp_setup` RPC namespace. Kept in a +/// separate function so the setup surface can evolve independently of the +/// existing `mcp_clients_*` controllers. +pub fn setup_schemas(function: &str) -> ControllerSchema { + match function { + "search" => ControllerSchema { + namespace: "mcp_setup", + function: "search", + description: "Search all enabled MCP registries (Smithery + official).", + inputs: vec![ + FieldSchema { + name: "query", + ty: TypeSchema::Option(Box::new(TypeSchema::String)), + comment: "Free-text search query.", + required: false, + }, + FieldSchema { + name: "page", + ty: TypeSchema::Option(Box::new(TypeSchema::U64)), + comment: "1-based page number (default: 1).", + required: false, + }, + FieldSchema { + name: "page_size", + ty: TypeSchema::Option(Box::new(TypeSchema::U64)), + comment: "Results per page (default: 20).", + required: false, + }, + ], + outputs: vec![ + FieldSchema { + name: "servers", + ty: TypeSchema::Array(Box::new(TypeSchema::Ref("SmitheryServerSummary"))), + comment: "Merged summaries; each row tagged with its `source` (`smithery` | `mcp_official`).", + required: true, + }, + FieldSchema { + name: "page", + ty: TypeSchema::U64, + comment: "Current page number.", + required: true, + }, + FieldSchema { + name: "total_pages", + ty: TypeSchema::U64, + comment: "Upper-bound page count across registries.", + required: true, + }, + ], + }, + "get" => ControllerSchema { + namespace: "mcp_setup", + function: "get", + description: "Fetch full details for one server. Adds `required_env_keys` derived from the connection schema.", + inputs: vec![FieldSchema { + name: "qualified_name", + ty: TypeSchema::String, + comment: "Registry qualified name. May be prefixed with `::` to pin a registry.", + required: true, + }], + outputs: vec![FieldSchema { + name: "server", + ty: TypeSchema::Ref("SmitheryServerDetail"), + comment: "Full detail with `required_env_keys` injected.", + required: true, + }], + }, + "request_secret" => ControllerSchema { + namespace: "mcp_setup", + function: "request_secret", + description: "Ask the user out-of-band for a secret value. Blocks until the UI submits via `submit_secret` (5-minute timeout). Returns an opaque ref; the raw value never enters the agent's context.", + inputs: vec![ + FieldSchema { + name: "key_name", + ty: TypeSchema::String, + comment: "Display name of the env var (e.g. `NOTION_API_KEY`).", + required: true, + }, + FieldSchema { + name: "prompt", + ty: TypeSchema::String, + comment: "Plain-English instruction shown to the user in the native input box.", + required: true, + }, + ], + outputs: vec![ + FieldSchema { + name: "ref", + ty: TypeSchema::String, + comment: "Opaque handle like `secret://`. Pass back via `test_connection` / `install_and_connect`.", + required: true, + }, + FieldSchema { + name: "key_name", + ty: TypeSchema::String, + comment: "Echoed key name.", + required: true, + }, + ], + }, + "submit_secret" => ControllerSchema { + namespace: "mcp_setup", + function: "submit_secret", + description: "UI-side: fulfill a pending `request_secret` with the user-entered value. Not intended for agent use.", + inputs: vec![ + FieldSchema { + name: "ref_id", + ty: TypeSchema::String, + comment: "The `secret://` ref returned by `request_secret`.", + required: true, + }, + FieldSchema { + name: "value", + ty: TypeSchema::String, + comment: "Raw secret value. NEVER log this.", + required: true, + }, + ], + outputs: vec![ + FieldSchema { + name: "ref", + ty: TypeSchema::String, + comment: "Echoed ref.", + required: true, + }, + FieldSchema { + name: "fulfilled", + ty: TypeSchema::Bool, + comment: "True on success.", + required: true, + }, + ], + }, + "test_connection" => ControllerSchema { + namespace: "mcp_setup", + function: "test_connection", + description: "Dry-run install: spawn a candidate server in a scratch process with the supplied secret refs, list its tools, tear down. Nothing persisted.", + inputs: vec![ + FieldSchema { + name: "qualified_name", + ty: TypeSchema::String, + comment: "Registry qualified name.", + required: true, + }, + FieldSchema { + name: "env_refs", + ty: TypeSchema::Map(Box::new(TypeSchema::String)), + comment: "Map `{ENV_KEY: secret://}` produced by `request_secret`.", + required: true, + }, + ], + outputs: vec![ + FieldSchema { + name: "ok", + ty: TypeSchema::Bool, + comment: "True if initialize + tools/list succeeded.", + required: true, + }, + FieldSchema { + name: "tools", + ty: TypeSchema::Option(Box::new(TypeSchema::Array(Box::new(TypeSchema::Ref( + "McpRemoteTool", + ))))), + comment: "Tools advertised by the candidate. Present iff `ok`.", + required: false, + }, + FieldSchema { + name: "error", + ty: TypeSchema::Option(Box::new(TypeSchema::String)), + comment: "Error string. Present iff `ok` is false.", + required: false, + }, + ], + }, + "install_and_connect" => ControllerSchema { + namespace: "mcp_setup", + function: "install_and_connect", + description: "Commit: persist the install + secrets (consuming the refs), then connect immediately and return the tool list.", + inputs: vec![ + FieldSchema { + name: "qualified_name", + ty: TypeSchema::String, + comment: "Registry qualified name.", + required: true, + }, + FieldSchema { + name: "env_refs", + ty: TypeSchema::Map(Box::new(TypeSchema::String)), + comment: "Map `{ENV_KEY: secret://}`. Refs are consumed (removed from the in-memory map) on success.", + required: true, + }, + ], + outputs: vec![ + FieldSchema { + name: "server_id", + ty: TypeSchema::String, + comment: "Freshly-minted server UUID.", + required: true, + }, + FieldSchema { + name: "status", + ty: TypeSchema::String, + comment: "`connected` or `installed_disconnected` (install succeeded, connect failed).", + required: true, + }, + FieldSchema { + name: "tools", + ty: TypeSchema::Option(Box::new(TypeSchema::Array(Box::new(TypeSchema::Ref( + "McpTool", + ))))), + comment: "Tool list iff `status == connected`.", + required: false, + }, + FieldSchema { + name: "error", + ty: TypeSchema::Option(Box::new(TypeSchema::String)), + comment: "Connect error iff `status != connected`.", + required: false, + }, + ], + }, + _ => ControllerSchema { + namespace: "mcp_setup", + function: "unknown", + description: "Unknown mcp_setup controller function.", + inputs: vec![FieldSchema { + name: "function", + ty: TypeSchema::String, + comment: "Unknown function requested for schema lookup.", + required: true, + }], + outputs: vec![FieldSchema { + name: "error", + ty: TypeSchema::String, + comment: "Lookup error details.", + required: true, + }], + }, + } +} + +fn handle_setup_search(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let query = read_optional_string(¶ms, "query")?; + let page = read_optional_u32(¶ms, "page")?; + let page_size = read_optional_u32(¶ms, "page_size")?; + to_json( + crate::openhuman::mcp_registry::setup_ops::mcp_setup_search( + &config, query, page, page_size, + ) + .await?, + ) + }) +} + +fn handle_setup_get(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let qualified_name = read_required::(¶ms, "qualified_name")?; + to_json( + crate::openhuman::mcp_registry::setup_ops::mcp_setup_get(&config, qualified_name) + .await?, + ) + }) +} + +fn handle_setup_request_secret(params: Map) -> ControllerFuture { + Box::pin(async move { + let key_name = read_required::(¶ms, "key_name")?; + let prompt = read_required::(¶ms, "prompt")?; + to_json( + crate::openhuman::mcp_registry::setup_ops::mcp_setup_request_secret(key_name, prompt) + .await?, + ) + }) +} + +fn handle_setup_submit_secret(params: Map) -> ControllerFuture { + Box::pin(async move { + let ref_id = read_required::(¶ms, "ref_id")?; + let value = read_required::(¶ms, "value")?; + to_json( + crate::openhuman::mcp_registry::setup_ops::mcp_setup_submit_secret(ref_id, value) + .await?, + ) + }) +} + +fn handle_setup_test_connection(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let qualified_name = read_required::(¶ms, "qualified_name")?; + let env_refs = + read_required::>(¶ms, "env_refs")?; + to_json( + crate::openhuman::mcp_registry::setup_ops::mcp_setup_test_connection( + &config, + qualified_name, + env_refs, + ) + .await?, + ) + }) +} + +fn handle_setup_install_and_connect(params: Map) -> ControllerFuture { + Box::pin(async move { + let config = config_rpc::load_config_with_timeout().await?; + let qualified_name = read_required::(¶ms, "qualified_name")?; + let env_refs = + read_required::>(¶ms, "env_refs")?; + to_json( + crate::openhuman::mcp_registry::setup_ops::mcp_setup_install_and_connect( + &config, + qualified_name, + env_refs, + ) + .await?, + ) + }) +} + // ── Param helpers ───────────────────────────────────────────────────────────── fn read_required(params: &Map, key: &str) -> Result { @@ -644,21 +1009,36 @@ mod tests { // ── all_controller_schemas / all_registered_controllers ──────────────────── #[test] - fn all_controller_schemas_covers_ten_methods() { + fn all_controller_schemas_covers_expected_methods() { let schemas = all_controller_schemas(); - assert_eq!(schemas.len(), 10); + // 10 mcp_clients + 6 mcp_setup + assert_eq!(schemas.len(), 16); + let mcp_clients_count = schemas + .iter() + .filter(|s| s.namespace == "mcp_clients") + .count(); + let mcp_setup_count = schemas + .iter() + .filter(|s| s.namespace == "mcp_setup") + .count(); + assert_eq!(mcp_clients_count, 10); + assert_eq!(mcp_setup_count, 6); } #[test] fn all_registered_controllers_has_handler_per_schema() { let controllers = all_registered_controllers(); - assert_eq!(controllers.len(), 10); + assert_eq!(controllers.len(), 16); } #[test] - fn all_registered_controllers_all_use_mcp_clients_namespace() { + fn all_registered_controllers_use_expected_namespaces() { for c in all_registered_controllers() { - assert_eq!(c.schema.namespace, "mcp_clients"); + assert!( + matches!(c.schema.namespace, "mcp_clients" | "mcp_setup"), + "unexpected namespace {}", + c.schema.namespace + ); } } diff --git a/src/openhuman/mcp_registry/setup.rs b/src/openhuman/mcp_registry/setup.rs new file mode 100644 index 000000000..091d287b7 --- /dev/null +++ b/src/openhuman/mcp_registry/setup.rs @@ -0,0 +1,327 @@ +//! Opaque secret-ref machinery for the MCP setup agent. +//! +//! The setup agent must collect credentials from the user **without** the +//! raw values ever entering the LLM context. The flow is: +//! +//! 1. Agent calls `mcp_setup_request_secret(key_name, prompt)`. Core mints +//! a fresh [`SecretRef`] (`secret://`), publishes +//! [`crate::core::event_bus::DomainEvent::McpSetupSecretRequested`] so +//! the UI can render a native prompt, and **awaits** the user. +//! 2. UI prompts the user out-of-band and POSTs back via +//! `mcp_setup_submit_secret(ref_id, value)`. Core stores the raw value +//! against the ref and wakes the waiting `request_secret` call. +//! 3. Agent receives the ref and passes it into `mcp_setup_test_connection` +//! or `mcp_setup_install_and_connect`. Core resolves refs → values +//! just-in-time and either spawns a scratch subprocess (test) or +//! persists them into `mcp_client_env` (install). +//! +//! Raw values never enter or exit through the agent-facing tool calls. +//! The map is process-local and cleared on shutdown; values do not +//! persist across restarts unless committed via `consume_refs` → +//! `mcp_client_env`. + +use std::collections::HashMap; +use std::sync::{Arc, OnceLock}; +use std::time::{Duration, Instant}; + +use tokio::sync::{oneshot, Mutex}; +use tokio::time::timeout; + +/// How long an unfulfilled `request_secret` waits before giving up. +pub const REQUEST_TIMEOUT: Duration = Duration::from_secs(300); // 5 min + +/// How long a fulfilled-but-unused secret hangs around before being purged. +/// Long enough to support iterative `test_connection` retries; short enough +/// that an abandoned conversation doesn't leave secrets stranded. +pub const IDLE_TTL: Duration = Duration::from_secs(900); // 15 min + +// ── Types ──────────────────────────────────────────────────────────────────── + +/// `secret://` — opaque handle returned to the agent. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct SecretRef(String); + +impl SecretRef { + pub fn as_str(&self) -> &str { + &self.0 + } + + /// Parse an agent-supplied string. Accepts both bare hex and the + /// `secret://` prefixed form so callers can pass whatever they got + /// back from `request_secret`. + pub fn parse(s: &str) -> Option { + let trimmed = s.strip_prefix("secret://").unwrap_or(s).trim(); + if trimmed.is_empty() || !trimmed.chars().all(|c| c.is_ascii_hexdigit()) { + return None; + } + Some(Self(format!("secret://{trimmed}"))) + } + + fn mint() -> Self { + // 12 hex chars (48 bits) sourced from the first 6 bytes of a + // UUIDv4 (OsRng-backed). Short enough to log; collision-free in + // any sane setup-session window. + let raw = uuid::Uuid::new_v4().simple().to_string(); + let hex = &raw[..12]; + Self(format!("secret://{hex}")) + } +} + +/// One entry in the global setup-secret map. +struct SecretEntry { + /// Display key name (e.g. `NOTION_API_KEY`). Safe to log; the *value* + /// is not. Returned to the UI in the request event. + key_name: String, + /// `None` while we're waiting for the UI to submit; `Some(value)` + /// once submitted. + value: Option, + /// Wall-clock time the entry was last touched (created or fulfilled). + /// Used by the GC sweep to enforce `IDLE_TTL`. + last_touched: Instant, + /// Wakes the matching `request_secret` call once `value` is populated. + /// Taken (set to `None`) on first fulfillment so a double-submit is a + /// no-op rather than a panic. + waiter: Option>, +} + +// ── Global registry ────────────────────────────────────────────────────────── + +type Map = Arc>>; + +static SETUP_SECRETS: OnceLock = OnceLock::new(); + +fn map() -> &'static Map { + SETUP_SECRETS.get_or_init(|| Arc::new(Mutex::new(HashMap::new()))) +} + +// ── Public API ─────────────────────────────────────────────────────────────── + +/// Mint a new ref + parked waiter for `key_name`. Returns the ref and the +/// receiver the caller should `.await` on (with a timeout) until the UI +/// submits the value via [`fulfill`]. +/// +/// The waiter is taken out of the map; if `fulfill` arrives before the +/// caller awaits, the value is stashed in `entry.value` and the oneshot +/// fires immediately when awaited. +pub async fn mint_request(key_name: &str) -> (SecretRef, oneshot::Receiver<()>) { + let (tx, rx) = oneshot::channel(); + let r = SecretRef::mint(); + let entry = SecretEntry { + key_name: key_name.to_string(), + value: None, + last_touched: Instant::now(), + waiter: Some(tx), + }; + map().lock().await.insert(r.clone(), entry); + tracing::debug!( + "[mcp-setup] minted ref={} key_name={}", + r.as_str(), + key_name + ); + (r, rx) +} + +/// UI-side: fulfill a pending ref with the raw value. Returns `true` if +/// the ref existed and was awaiting; `false` if it was unknown or already +/// fulfilled. +pub async fn fulfill(r: &SecretRef, value: String) -> bool { + let mut guard = map().lock().await; + let Some(entry) = guard.get_mut(r) else { + tracing::warn!("[mcp-setup] fulfill: unknown ref={}", r.as_str()); + return false; + }; + if entry.value.is_some() { + tracing::warn!("[mcp-setup] fulfill: double-submit ref={}", r.as_str()); + return false; + } + entry.value = Some(value); + entry.last_touched = Instant::now(); + if let Some(tx) = entry.waiter.take() { + let _ = tx.send(()); + } + tracing::debug!("[mcp-setup] fulfilled ref={}", r.as_str()); + true +} + +/// Block on a freshly-minted request with the global timeout. On timeout +/// the entry is removed and `Err(_)` is returned. +pub async fn await_fulfillment(r: &SecretRef, rx: oneshot::Receiver<()>) -> anyhow::Result<()> { + match timeout(REQUEST_TIMEOUT, rx).await { + Ok(Ok(())) => Ok(()), + Ok(Err(_)) => { + // Sender dropped — usually means GC purged the entry. Surface + // as a timeout-style error to keep the caller simple. + let _ = forget(r).await; + anyhow::bail!("secret request {} cancelled before user submit", r.as_str()) + } + Err(_) => { + let _ = forget(r).await; + anyhow::bail!( + "secret request {} timed out after {}s", + r.as_str(), + REQUEST_TIMEOUT.as_secs() + ) + } + } +} + +/// Resolve a `{KEY: SecretRef}` map into a `Vec<(KEY, VALUE)>`. Returns +/// `Err(_)` if any ref is unknown or not yet fulfilled — callers should +/// retry rather than partially-apply. +/// +/// Touches the `last_touched` on every hit so iterative `test_connection` +/// calls reset the idle TTL. +pub async fn resolve_refs( + refs: &HashMap, +) -> anyhow::Result> { + let mut guard = map().lock().await; + let mut out = Vec::with_capacity(refs.len()); + for (key, r) in refs { + let entry = guard + .get_mut(r) + .ok_or_else(|| anyhow::anyhow!("unknown secret ref {}", r.as_str()))?; + let value = entry + .value + .clone() + .ok_or_else(|| anyhow::anyhow!("secret ref {} not yet fulfilled", r.as_str()))?; + entry.last_touched = Instant::now(); + out.push((key.clone(), value)); + } + Ok(out) +} + +/// Same as [`resolve_refs`] but also removes the entries from the map on +/// success. Used by `install_and_connect` once the values have been +/// persisted to `mcp_client_env`. On failure the entries are left intact +/// so the agent can retry without re-prompting. +pub async fn consume_refs( + refs: &HashMap, +) -> anyhow::Result> { + // First pass: resolve. Bail without mutation if any ref is missing. + let resolved = resolve_refs(refs).await?; + // Second pass: drop. Bail-out on the first miss is impossible because + // we just held the resolved values without releasing the lock — but to + // be honest we *did* release between the two awaits. Recheck. + let mut guard = map().lock().await; + for r in refs.values() { + guard.remove(r); + } + Ok(resolved) +} + +/// Drop a single ref. Useful when the agent abandons a half-collected +/// install. Returns `true` if the ref existed. +pub async fn forget(r: &SecretRef) -> bool { + map().lock().await.remove(r).is_some() +} + +/// Sweep entries idle longer than [`IDLE_TTL`]. Intended to be called from +/// a background task; cheap to call frequently. Returns the number of +/// entries reaped. +pub async fn gc_sweep() -> usize { + let now = Instant::now(); + let mut guard = map().lock().await; + let before = guard.len(); + guard.retain(|_, entry| now.duration_since(entry.last_touched) < IDLE_TTL); + let reaped = before - guard.len(); + if reaped > 0 { + tracing::debug!("[mcp-setup] gc_sweep reaped={reaped}"); + } + reaped +} + +/// Test-only: inspect the key name for a ref. Production callers must +/// not learn this through the agent surface. +#[cfg(test)] +pub(crate) async fn key_name_for(r: &SecretRef) -> Option { + map().lock().await.get(r).map(|e| e.key_name.clone()) +} + +#[cfg(test)] +mod tests { + use super::*; + use tokio::sync::Mutex as AsyncMutex; + + // Setup tests share the static SETUP_SECRETS map. Serialise them via + // this guard so parallel runs don't trample each other. + static TEST_GUARD: AsyncMutex<()> = AsyncMutex::const_new(()); + + async fn clear_map() { + map().lock().await.clear(); + } + + #[tokio::test] + async fn mint_then_fulfill_then_resolve() { + let _g = TEST_GUARD.lock().await; + clear_map().await; + let (r, rx) = mint_request("API_KEY").await; + let r2 = r.clone(); + let fulfill_task = tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(10)).await; + assert!(fulfill(&r2, "shh".into()).await); + }); + await_fulfillment(&r, rx).await.expect("fulfilled"); + fulfill_task.await.unwrap(); + + let mut refs = HashMap::new(); + refs.insert("API_KEY".to_string(), r.clone()); + let resolved = resolve_refs(&refs).await.expect("resolves"); + assert_eq!(resolved, vec![("API_KEY".to_string(), "shh".to_string())]); + + // Still present after resolve. + assert_eq!(key_name_for(&r).await.as_deref(), Some("API_KEY")); + } + + #[tokio::test] + async fn consume_drops_entries() { + let _g = TEST_GUARD.lock().await; + clear_map().await; + let (r, _rx) = mint_request("TOKEN").await; + fulfill(&r, "v".into()).await; + let mut refs = HashMap::new(); + refs.insert("TOKEN".to_string(), r.clone()); + let _ = consume_refs(&refs).await.expect("consumes"); + assert!(key_name_for(&r).await.is_none(), "consumed entry removed"); + } + + #[tokio::test] + async fn resolve_fails_when_not_fulfilled() { + let _g = TEST_GUARD.lock().await; + clear_map().await; + let (r, _rx) = mint_request("UNSET").await; + let mut refs = HashMap::new(); + refs.insert("UNSET".to_string(), r); + assert!(resolve_refs(&refs).await.is_err()); + } + + #[tokio::test] + async fn resolve_fails_on_unknown_ref() { + let _g = TEST_GUARD.lock().await; + clear_map().await; + let fake = SecretRef::parse("secret://deadbeef").unwrap(); + let mut refs = HashMap::new(); + refs.insert("X".to_string(), fake); + assert!(resolve_refs(&refs).await.is_err()); + } + + #[tokio::test] + async fn double_fulfill_is_noop() { + let _g = TEST_GUARD.lock().await; + clear_map().await; + let (r, _rx) = mint_request("K").await; + assert!(fulfill(&r, "first".into()).await); + assert!(!fulfill(&r, "second".into()).await); + let mut refs = HashMap::new(); + refs.insert("K".to_string(), r); + let resolved = resolve_refs(&refs).await.unwrap(); + assert_eq!(resolved[0].1, "first", "second fulfill ignored"); + } + + #[tokio::test] + async fn parse_accepts_bare_and_prefixed_hex() { + assert!(SecretRef::parse("secret://abc123").is_some()); + assert!(SecretRef::parse("abc123").is_some()); + assert!(SecretRef::parse("not-hex").is_none()); + assert!(SecretRef::parse("").is_none()); + } +} diff --git a/src/openhuman/mcp_registry/setup_ops.rs b/src/openhuman/mcp_registry/setup_ops.rs new file mode 100644 index 000000000..75b2c6508 --- /dev/null +++ b/src/openhuman/mcp_registry/setup_ops.rs @@ -0,0 +1,329 @@ +//! RPC handlers for the MCP setup agent. See `docs/MCP_SETUP_AGENT.md`. +//! +//! These handlers form the agent-facing tool surface: +//! +//! - `mcp_setup_search` / `mcp_setup_get` — thin wrappers over +//! [`super::registry`] so the agent browses upstream registries. +//! - `mcp_setup_request_secret` — block on a fresh ref until the UI +//! submits a value. +//! - `mcp_setup_submit_secret` — UI-side fulfillment. +//! - `mcp_setup_test_connection` — spawn a candidate subprocess in a +//! scratch workspace, list its tools, tear it down. No persistence. +//! - `mcp_setup_install_and_connect` — commit: persist install + env, +//! call [`super::connections::connect`]. +//! +//! Raw secret values flow only through `submit_secret` and the +//! just-in-time resolve inside `test_connection` / `install_and_connect`. +//! They are never echoed in responses or logged. + +use std::collections::HashMap; +use std::path::PathBuf; + +use serde_json::{json, Value}; +use uuid::Uuid; + +use crate::core::event_bus::{publish_global, DomainEvent}; +use crate::openhuman::config::Config; +use crate::openhuman::mcp_client::McpStdioClient; +use crate::rpc::RpcOutcome; + +use super::ops::resolve_command; +use super::setup::{self, SecretRef}; +use super::types::{CommandKind, InstalledServer}; +use super::{connections, registry, store}; + +// ── search ─────────────────────────────────────────────────────────────────── + +pub async fn mcp_setup_search( + config: &Config, + query: Option, + page: Option, + page_size: Option, +) -> Result, String> { + let page = page.unwrap_or(1); + let page_size = page_size.unwrap_or(20); + let (servers, total_pages) = + registry::registry_search(config, query.as_deref(), page, page_size) + .await + .map_err(|e| e.to_string())?; + Ok(RpcOutcome::new( + json!({ "servers": servers, "page": page, "total_pages": total_pages }), + vec![format!("setup_search returned {} servers", servers.len())], + )) +} + +// ── get ────────────────────────────────────────────────────────────────────── + +pub async fn mcp_setup_get( + config: &Config, + qualified_name: String, +) -> Result, String> { + let q = qualified_name.trim(); + if q.is_empty() { + return Err("qualified_name must not be empty".to_string()); + } + let detail = registry::registry_get(config, q) + .await + .map_err(|e| e.to_string())?; + let required_env_keys = collect_required_env_keys(&detail); + let mut value = serde_json::to_value(&detail).map_err(|e| format!("ser: {e}"))?; + if let Some(obj) = value.as_object_mut() { + obj.insert("required_env_keys".into(), json!(required_env_keys)); + } + Ok(RpcOutcome::new( + json!({ "server": value }), + vec![format!("setup_get ok qualified_name={q}")], + )) +} + +// ── request_secret ─────────────────────────────────────────────────────────── + +pub async fn mcp_setup_request_secret( + key_name: String, + prompt: String, +) -> Result, String> { + let key_name = key_name.trim().to_string(); + let prompt = prompt.trim().to_string(); + if key_name.is_empty() { + return Err("key_name must not be empty".to_string()); + } + if prompt.is_empty() { + return Err("prompt must not be empty".to_string()); + } + + let (r, rx) = setup::mint_request(&key_name).await; + + let _ = publish_global(DomainEvent::McpSetupSecretRequested { + ref_id: r.as_str().to_string(), + key_name: key_name.clone(), + prompt: prompt.clone(), + }); + tracing::info!( + "[mcp-setup] request_secret ref={} key_name={} (awaiting UI submit)", + r.as_str(), + key_name + ); + + setup::await_fulfillment(&r, rx) + .await + .map_err(|e| e.to_string())?; + + tracing::info!("[mcp-setup] request_secret fulfilled ref={}", r.as_str()); + Ok(RpcOutcome::new( + json!({ "ref": r.as_str(), "key_name": key_name }), + vec![format!("collected secret for key={key_name}")], + )) +} + +// ── submit_secret (UI side) ────────────────────────────────────────────────── + +pub async fn mcp_setup_submit_secret( + ref_id: String, + value: String, +) -> Result, String> { + let r = SecretRef::parse(&ref_id).ok_or_else(|| format!("invalid ref_id `{ref_id}`"))?; + let ok = setup::fulfill(&r, value).await; + if !ok { + return Err(format!("ref {} unknown or already submitted", r.as_str())); + } + Ok(RpcOutcome::new( + json!({ "ref": r.as_str(), "fulfilled": true }), + vec![format!("submitted secret for ref={}", r.as_str())], + )) +} + +// ── test_connection ────────────────────────────────────────────────────────── + +pub async fn mcp_setup_test_connection( + config: &Config, + qualified_name: String, + env_refs: HashMap, +) -> Result, String> { + let q = qualified_name.trim(); + if q.is_empty() { + return Err("qualified_name must not be empty".to_string()); + } + + let parsed_refs = parse_ref_map(env_refs)?; + let env = setup::resolve_refs(&parsed_refs) + .await + .map_err(|e| e.to_string())?; + + let detail = registry::registry_get(config, q) + .await + .map_err(|e| e.to_string())?; + let stdio_conn = detail + .connections + .iter() + .filter(|c| c.r#type == "stdio") + .find(|c| c.published) + .or_else(|| detail.connections.iter().find(|c| c.r#type == "stdio")); + let (_kind, command, args) = resolve_command(q, stdio_conn); + + let identity = config.mcp_client.client_identity.clone(); + let cwd: Option = None; + let client = McpStdioClient::new(command.clone(), args.clone(), env, cwd, identity); + + // Scratch subprocess — initialise + list_tools, then close. Nothing + // persisted. Errors bubble up so the agent can show them to the user. + if let Err(err) = client.initialize().await { + return Ok(RpcOutcome::new( + json!({ "ok": false, "error": err.to_string() }), + vec![format!("test_connection failed for {q}: {err}")], + )); + } + let tools = match client.list_tools().await { + Ok(t) => t, + Err(err) => { + let _ = client.close_session().await; + return Ok(RpcOutcome::new( + json!({ "ok": false, "error": err.to_string() }), + vec![format!("test_connection list_tools failed for {q}: {err}")], + )); + } + }; + let _ = client.close_session().await; + + let names: Vec<&str> = tools.iter().map(|t| t.name.as_str()).collect(); + Ok(RpcOutcome::new( + json!({ "ok": true, "tools": tools }), + vec![format!( + "test_connection ok for {q}: {} tools ({:?})", + tools.len(), + names + )], + )) +} + +// ── install_and_connect ────────────────────────────────────────────────────── + +pub async fn mcp_setup_install_and_connect( + config: &Config, + qualified_name: String, + env_refs: HashMap, +) -> Result, String> { + let q = qualified_name.trim(); + if q.is_empty() { + return Err("qualified_name must not be empty".to_string()); + } + + let parsed_refs = parse_ref_map(env_refs)?; + + let detail = registry::registry_get(config, q) + .await + .map_err(|e| e.to_string())?; + let stdio_conn = detail + .connections + .iter() + .filter(|c| c.r#type == "stdio") + .find(|c| c.published) + .or_else(|| detail.connections.iter().find(|c| c.r#type == "stdio")); + let (command_kind, command, args) = resolve_command(q, stdio_conn); + + // Consume refs only after `registry_get` succeeds — that way a + // misconfigured server name doesn't burn the user's collected + // secrets. + let env_pairs = setup::consume_refs(&parsed_refs) + .await + .map_err(|e| e.to_string())?; + let env_map: HashMap = env_pairs.into_iter().collect(); + + let server_id = Uuid::new_v4().to_string(); + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as i64) + .unwrap_or(0); + let env_keys: Vec = env_map.keys().cloned().collect(); + + let server = InstalledServer { + server_id: server_id.clone(), + qualified_name: q.to_string(), + display_name: detail.display_name.clone(), + description: detail.description.clone(), + icon_url: detail.icon_url.clone(), + command_kind, + command, + args, + env_keys, + config: None, + installed_at: now_ms, + last_connected_at: None, + }; + + store::insert_server(config, &server).map_err(|e| e.to_string())?; + store::set_env_values(config, &server_id, &env_map).map_err(|e| e.to_string())?; + + let _ = publish_global(DomainEvent::McpServerInstalled { + server_id: server_id.clone(), + qualified_name: server.qualified_name.clone(), + }); + + // Connect immediately so the agent gets the tool list in the same + // response. A connect failure does not roll back the install — the + // user can retry via `mcp_clients_connect` later. + match connections::connect(config, &server).await { + Ok(tools) => Ok(RpcOutcome::new( + json!({ + "server_id": server_id, + "status": "connected", + "tools": tools, + }), + vec![format!( + "install_and_connect ok server_id={server_id} tools={}", + tools.len() + )], + )), + Err(err) => Ok(RpcOutcome::new( + json!({ + "server_id": server_id, + "status": "installed_disconnected", + "error": err.to_string(), + }), + vec![format!( + "install_and_connect installed server_id={server_id} \ + but connect failed: {err}" + )], + )), + } +} + +// ── Helpers ────────────────────────────────────────────────────────────────── + +fn parse_ref_map(raw: HashMap) -> Result, String> { + let mut out = HashMap::with_capacity(raw.len()); + for (k, v) in raw { + let r = SecretRef::parse(&v) + .ok_or_else(|| format!("env_refs[{k}] is not a valid secret ref"))?; + out.insert(k, r); + } + Ok(out) +} + +/// Best-effort scan of a Smithery `config_schema` for required env keys. +/// Mirrors the legacy helper in `ops.rs` so the setup agent does not +/// depend on its private wiring. +fn collect_required_env_keys(detail: &super::types::SmitheryServerDetail) -> Vec { + let mut keys = Vec::new(); + for conn in &detail.connections { + if conn.r#type != "stdio" { + continue; + } + let Some(schema) = conn.config_schema.as_ref() else { + continue; + }; + let Some(props) = schema.get("properties").and_then(Value::as_object) else { + continue; + }; + for k in props.keys() { + if !keys.contains(k) { + keys.push(k.clone()); + } + } + } + keys +} + +// Compile-time anchor so a missing CommandKind import surfaces here, not +// at the call site. +#[allow(dead_code)] +const _: Option = None; diff --git a/src/openhuman/mcp_clients/store.rs b/src/openhuman/mcp_registry/store.rs similarity index 100% rename from src/openhuman/mcp_clients/store.rs rename to src/openhuman/mcp_registry/store.rs diff --git a/src/openhuman/mcp_clients/types.rs b/src/openhuman/mcp_registry/types.rs similarity index 96% rename from src/openhuman/mcp_clients/types.rs rename to src/openhuman/mcp_registry/types.rs index 17a0254c7..1cf3ff701 100644 --- a/src/openhuman/mcp_clients/types.rs +++ b/src/openhuman/mcp_registry/types.rs @@ -134,6 +134,12 @@ pub struct SmitheryServerSummary { pub use_count: u64, #[serde(default)] pub is_deployed: bool, + /// Upstream registry id (`"smithery"` | `"mcp_official"`). Always set + /// by the dispatcher in `super::registries` so the frontend can attribute + /// rows and the install path can route `registry_get` back to the + /// originating upstream. + #[serde(default)] + pub source: String, /// Raw extra fields preserved for future use. #[serde(flatten, default)] pub extra: std::collections::HashMap, @@ -151,6 +157,9 @@ pub struct SmitheryServerDetail { pub icon_url: Option, #[serde(default)] pub connections: Vec, + /// Upstream registry id (`"smithery"` | `"mcp_official"`). + #[serde(default)] + pub source: String, #[serde(flatten, default)] pub extra: std::collections::HashMap, } diff --git a/src/openhuman/mcp_server/http.rs b/src/openhuman/mcp_server/http.rs index c8fabe67b..09c9fbcd4 100644 --- a/src/openhuman/mcp_server/http.rs +++ b/src/openhuman/mcp_server/http.rs @@ -1,7 +1,7 @@ //! Streamable HTTP + SSE transport for the OpenHuman MCP server. //! //! Reuses [`super::protocol`] for JSON-RPC dispatch. Session lifecycle and header -//! names match [`crate::openhuman::mcp_client::client::McpHttpClient`] so remote +//! names match [`crate::openhuman::mcp_client::McpHttpClient`] so remote //! MCP clients can talk to this server without custom glue. use std::collections::HashMap; diff --git a/src/openhuman/mod.rs b/src/openhuman/mod.rs index 111549006..4a29ac5cf 100644 --- a/src/openhuman/mod.rs +++ b/src/openhuman/mod.rs @@ -48,7 +48,7 @@ pub mod javascript; pub mod keyring; pub mod learning; pub mod mcp_client; -pub mod mcp_clients; +pub mod mcp_registry; pub mod mcp_server; pub mod meet; pub mod meet_agent; diff --git a/src/openhuman/tool_registry/ops.rs b/src/openhuman/tool_registry/ops.rs index a94b030e9..7897ddbf1 100644 --- a/src/openhuman/tool_registry/ops.rs +++ b/src/openhuman/tool_registry/ops.rs @@ -107,7 +107,7 @@ pub fn registry_entries() -> Vec { // `block_in_place` requires the multi-threaded tokio runtime; fall back // silently to an empty list in single-threaded contexts (e.g. unit tests). let client_tools = { - use crate::openhuman::mcp_clients::connections; + use crate::openhuman::mcp_registry::connections; match tokio::runtime::Handle::try_current() { Ok(handle) => { // Only use block_in_place when we are on the multi-threaded diff --git a/src/openhuman/tools/impl/network/mcp_setup.rs b/src/openhuman/tools/impl/network/mcp_setup.rs new file mode 100644 index 000000000..8237418e3 --- /dev/null +++ b/src/openhuman/tools/impl/network/mcp_setup.rs @@ -0,0 +1,393 @@ +//! Agent-facing tool wrappers around `mcp_registry::setup_ops`. +//! +//! Six thin tools the `mcp_setup` sub-agent uses to walk the user +//! through installing an MCP server. They are intentionally simple — +//! the real logic lives in +//! [`crate::openhuman::mcp_registry::setup_ops`]; these structs only +//! marshall args ↔ `serde_json::Value` and turn `RpcOutcome` into a +//! `ToolResult`. +//! +//! Secret values **never** pass through these tools. `request_secret` +//! returns an opaque `secret://` ref; the agent stores the ref and +//! passes it back into `test_connection` / `install_and_connect` which +//! resolve it inside the core process. + +use std::collections::HashMap; +use std::sync::Arc; + +use async_trait::async_trait; +use serde_json::{json, Value}; + +use crate::openhuman::config::Config; +use crate::openhuman::mcp_registry::setup_ops; +use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolCategory, ToolResult}; + +// ── Helpers ────────────────────────────────────────────────────────────────── + +fn outcome_to_result( + outcome: Result, String>, +) -> anyhow::Result { + match outcome { + Ok(out) => Ok(ToolResult::json(out.value)), + Err(err) => Ok(ToolResult::error(err)), + } +} + +fn read_str(args: &Value, key: &str) -> Result { + args.get(key) + .and_then(Value::as_str) + .map(str::to_string) + .ok_or_else(|| format!("missing required string `{key}`")) +} + +fn read_str_opt(args: &Value, key: &str) -> Option { + args.get(key) + .and_then(Value::as_str) + .map(str::to_string) + .filter(|s| !s.is_empty()) +} + +fn read_u32_opt(args: &Value, key: &str) -> Option { + args.get(key).and_then(Value::as_u64).map(|v| v as u32) +} + +fn read_str_map(args: &Value, key: &str) -> Result, String> { + let v = args + .get(key) + .ok_or_else(|| format!("missing required object `{key}`"))?; + let obj = v + .as_object() + .ok_or_else(|| format!("`{key}` must be an object"))?; + let mut out = HashMap::with_capacity(obj.len()); + for (k, v) in obj { + let s = v + .as_str() + .ok_or_else(|| format!("`{key}[{k}]` must be a string"))?; + out.insert(k.clone(), s.to_string()); + } + Ok(out) +} + +// ── mcp_setup_search ───────────────────────────────────────────────────────── + +pub struct McpSetupSearchTool { + config: Arc, +} + +impl McpSetupSearchTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for McpSetupSearchTool { + fn name(&self) -> &str { + "mcp_setup_search" + } + + fn description(&self) -> &str { + "Search all enabled MCP server registries (Smithery + modelcontextprotocol/registry). \ + Returns merged results tagged with the upstream `source`. Use to discover candidate \ + servers by keyword (e.g. 'notion', 'filesystem', 'github')." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "query": { "type": "string", "description": "Free-text search query." }, + "page": { "type": "integer", "description": "1-based page number (default 1)." }, + "page_size": { "type": "integer", "description": "Results per page (default 20)." } + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::ReadOnly + } + + fn category(&self) -> ToolCategory { + ToolCategory::System + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let query = read_str_opt(&args, "query"); + let page = read_u32_opt(&args, "page"); + let page_size = read_u32_opt(&args, "page_size"); + outcome_to_result(setup_ops::mcp_setup_search(&self.config, query, page, page_size).await) + } +} + +// ── mcp_setup_get ──────────────────────────────────────────────────────────── + +pub struct McpSetupGetTool { + config: Arc, +} + +impl McpSetupGetTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for McpSetupGetTool { + fn name(&self) -> &str { + "mcp_setup_get" + } + + fn description(&self) -> &str { + "Fetch full detail for one MCP server, including the `required_env_keys` array derived \ + from its connection schema. Use to plan which secrets to request from the user." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "qualified_name": { + "type": "string", + "description": "Registry qualified name (e.g. `@notion/server-notion`). \ + May be prefixed with `::` to pin a registry." + } + }, + "required": ["qualified_name"] + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::ReadOnly + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let qualified_name = match read_str(&args, "qualified_name") { + Ok(v) => v, + Err(e) => return Ok(ToolResult::error(e)), + }; + outcome_to_result(setup_ops::mcp_setup_get(&self.config, qualified_name).await) + } +} + +// ── mcp_setup_request_secret ───────────────────────────────────────────────── + +pub struct McpSetupRequestSecretTool; + +impl McpSetupRequestSecretTool { + pub fn new() -> Self { + Self + } +} + +impl Default for McpSetupRequestSecretTool { + fn default() -> Self { + Self::new() + } +} + +#[async_trait] +impl Tool for McpSetupRequestSecretTool { + fn name(&self) -> &str { + "mcp_setup_request_secret" + } + + fn description(&self) -> &str { + "Ask the user to provide a secret value (API key, OAuth token, etc.) via a native UI \ + prompt. Returns an opaque ref like `secret://`. The raw value never enters this \ + agent's context — only the ref does. Pass the ref back into `mcp_setup_test_connection` \ + or `mcp_setup_install_and_connect`. Blocks for up to 5 minutes waiting on the user." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "key_name": { + "type": "string", + "description": "Env-var name (e.g. `NOTION_API_KEY`). Shown to the user as the field label." + }, + "prompt": { + "type": "string", + "description": "Plain-English explanation the user sees, e.g. 'Paste your Notion integration token from notion.so/my-integrations.'" + } + }, + "required": ["key_name", "prompt"] + }) + } + + fn permission_level(&self) -> PermissionLevel { + // No filesystem / network — purely an IPC handshake. ReadOnly is + // wrong (it's user-input), but Write is too strong. The harness + // gates by `< Execute`, so ReadOnly keeps the agent able to call + // it. + PermissionLevel::ReadOnly + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let key_name = match read_str(&args, "key_name") { + Ok(v) => v, + Err(e) => return Ok(ToolResult::error(e)), + }; + let prompt = match read_str(&args, "prompt") { + Ok(v) => v, + Err(e) => return Ok(ToolResult::error(e)), + }; + outcome_to_result(setup_ops::mcp_setup_request_secret(key_name, prompt).await) + } +} + +// ── mcp_setup_test_connection ──────────────────────────────────────────────── + +pub struct McpSetupTestConnectionTool { + config: Arc, +} + +impl McpSetupTestConnectionTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for McpSetupTestConnectionTool { + fn name(&self) -> &str { + "mcp_setup_test_connection" + } + + fn description(&self) -> &str { + "Dry-run install: spawn the candidate MCP server in a scratch process with the supplied \ + secret refs, list its tools, tear it down. Nothing is persisted. Returns \ + `{ ok: true, tools: [...] }` on success or `{ ok: false, error: ... }` on failure — \ + use this to validate the user's secrets before committing." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "qualified_name": { + "type": "string", + "description": "Registry qualified name." + }, + "env_refs": { + "type": "object", + "description": "Map `{ENV_KEY: \"secret://\"}` of refs collected from `mcp_setup_request_secret`.", + "additionalProperties": { "type": "string" } + } + }, + "required": ["qualified_name", "env_refs"] + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Execute + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let qualified_name = match read_str(&args, "qualified_name") { + Ok(v) => v, + Err(e) => return Ok(ToolResult::error(e)), + }; + let env_refs = match read_str_map(&args, "env_refs") { + Ok(v) => v, + Err(e) => return Ok(ToolResult::error(e)), + }; + outcome_to_result( + setup_ops::mcp_setup_test_connection(&self.config, qualified_name, env_refs).await, + ) + } +} + +// ── mcp_setup_install_and_connect ──────────────────────────────────────────── + +pub struct McpSetupInstallAndConnectTool { + config: Arc, +} + +impl McpSetupInstallAndConnectTool { + pub fn new(config: Arc) -> Self { + Self { config } + } +} + +#[async_trait] +impl Tool for McpSetupInstallAndConnectTool { + fn name(&self) -> &str { + "mcp_setup_install_and_connect" + } + + fn description(&self) -> &str { + "Commit: persist the MCP server install + the user's secrets (consuming the refs), then \ + connect immediately. Returns the new `server_id` and (on success) the tool list now \ + available to the agent. Only call after `mcp_setup_test_connection` returned ok." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "qualified_name": { + "type": "string", + "description": "Registry qualified name." + }, + "env_refs": { + "type": "object", + "description": "Same shape as `mcp_setup_test_connection`. Refs are consumed (removed from the in-memory map) on success.", + "additionalProperties": { "type": "string" } + } + }, + "required": ["qualified_name", "env_refs"] + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Write + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let qualified_name = match read_str(&args, "qualified_name") { + Ok(v) => v, + Err(e) => return Ok(ToolResult::error(e)), + }; + let env_refs = match read_str_map(&args, "env_refs") { + Ok(v) => v, + Err(e) => return Ok(ToolResult::error(e)), + }; + outcome_to_result( + setup_ops::mcp_setup_install_and_connect(&self.config, qualified_name, env_refs).await, + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn names_are_stable() { + let cfg = Arc::new(Config::default()); + assert_eq!( + McpSetupSearchTool::new(cfg.clone()).name(), + "mcp_setup_search" + ); + assert_eq!(McpSetupGetTool::new(cfg.clone()).name(), "mcp_setup_get"); + assert_eq!( + McpSetupRequestSecretTool::new().name(), + "mcp_setup_request_secret" + ); + assert_eq!( + McpSetupTestConnectionTool::new(cfg.clone()).name(), + "mcp_setup_test_connection" + ); + assert_eq!( + McpSetupInstallAndConnectTool::new(cfg).name(), + "mcp_setup_install_and_connect" + ); + } + + #[test] + fn read_str_map_rejects_non_string_values() { + let args = json!({ "env_refs": { "K": 42 } }); + assert!(read_str_map(&args, "env_refs").is_err()); + } +} diff --git a/src/openhuman/tools/impl/network/mod.rs b/src/openhuman/tools/impl/network/mod.rs index bf08938ac..3344c3890 100644 --- a/src/openhuman/tools/impl/network/mod.rs +++ b/src/openhuman/tools/impl/network/mod.rs @@ -5,6 +5,7 @@ mod gitbooks; mod gmail_unsubscribe; mod http_request; mod mcp; +mod mcp_setup; mod polymarket; mod polymarket_orders; mod url_guard; @@ -17,6 +18,10 @@ pub use gitbooks::{GitbooksGetPageTool, GitbooksSearchTool}; pub use gmail_unsubscribe::GmailUnsubscribeTool; pub use http_request::HttpRequestTool; pub use mcp::{McpCallTool, McpListServersTool, McpListToolsTool}; +pub use mcp_setup::{ + McpSetupGetTool, McpSetupInstallAndConnectTool, McpSetupRequestSecretTool, McpSetupSearchTool, + McpSetupTestConnectionTool, +}; pub use polymarket::PolymarketTool; pub use web_fetch::WebFetchTool; pub use web_search::WebSearchTool; diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index 089c4e931..b0105800f 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -289,6 +289,20 @@ pub fn all_tools_with_runtime( tracing::debug!("[gitbooks] registered gitbooks_search + gitbooks_get_page"); } + // MCP setup-agent tool surface (search/get/request_secret/test/install). + // Registered unconditionally — the `mcp_setup` sub-agent filters to just + // these via its `[tools] named = [...]` allowlist, and the host agent's + // own tool list is wide enough that the extra five entries are negligible. + { + let cfg = Arc::new(root_config.clone()); + tools.push(Box::new(McpSetupSearchTool::new(Arc::clone(&cfg)))); + tools.push(Box::new(McpSetupGetTool::new(Arc::clone(&cfg)))); + tools.push(Box::new(McpSetupRequestSecretTool::new())); + tools.push(Box::new(McpSetupTestConnectionTool::new(Arc::clone(&cfg)))); + tools.push(Box::new(McpSetupInstallAndConnectTool::new(cfg))); + tracing::debug!("[mcp_setup] registered 5 setup-agent tools"); + } + // Generic remote MCP bridge tools. These let the agent enumerate // named MCP servers and forward `tools/call` through the core // instead of hardcoding one bespoke MCP integration per server. diff --git a/tests/mcp_registry_e2e.rs b/tests/mcp_registry_e2e.rs new file mode 100644 index 000000000..7601d48f2 --- /dev/null +++ b/tests/mcp_registry_e2e.rs @@ -0,0 +1,116 @@ +//! End-to-end test for the `mcp_registry` connection lifecycle. +//! +//! Hermetic: spawns the `test-mcp-stub` binary (built alongside this test +//! by Cargo and exposed via `CARGO_BIN_EXE_test-mcp-stub`) as the MCP +//! subprocess. No npx, no network. Validates that +//! `store::insert_server` → `connections::connect` → `connections::call_tool` +//! → `connections::disconnect` round-trips correctly through the unified +//! `mcp_client::McpStdioClient` transport. + +use openhuman_core::openhuman::config::Config; +use openhuman_core::openhuman::mcp_registry::connections; +use openhuman_core::openhuman::mcp_registry::store; +use openhuman_core::openhuman::mcp_registry::types::{CommandKind, InstalledServer}; + +fn fresh_workspace_config() -> (tempfile::TempDir, Config) { + let tmp = tempfile::tempdir().expect("tempdir"); + let mut cfg = Config::default(); + cfg.workspace_dir = tmp.path().to_path_buf(); + (tmp, cfg) +} + +fn make_installed_server() -> InstalledServer { + let stub_path = env!("CARGO_BIN_EXE_test-mcp-stub"); + InstalledServer { + server_id: format!("test-{}", uuid::Uuid::new_v4()), + qualified_name: "@openhuman-test/echo".to_string(), + display_name: "Test Echo".to_string(), + description: Some("Stub MCP server used by mcp_registry_e2e tests.".into()), + icon_url: None, + command_kind: CommandKind::Binary, + command: stub_path.to_string(), + args: Vec::new(), + env_keys: Vec::new(), + config: None, + installed_at: 0, + last_connected_at: None, + } +} + +#[tokio::test] +async fn connect_lists_one_tool_then_disconnect() { + let (_tmp, cfg) = fresh_workspace_config(); + let server = make_installed_server(); + + // Insert into the store so `all_status` (which reads from store) sees it, + // and so a follow-up `boot::spawn_installed_servers` would pick it up. + store::insert_server(&cfg, &server).expect("insert installed server"); + + // Connect: spawns the stub subprocess and runs `initialize` + `tools/list`. + let tools = connections::connect(&cfg, &server) + .await + .expect("connect succeeds"); + assert_eq!(tools.len(), 1, "stub advertises one tool"); + assert_eq!(tools[0].name, "echo"); + assert!(tools[0].input_schema.is_object()); + + // Status reflects the live connection. + let statuses = connections::all_status(&cfg).await; + let mine = statuses + .iter() + .find(|s| s.server_id == server.server_id) + .expect("status entry present"); + assert_eq!(mine.tool_count, 1); + + // Call the `echo` tool and verify the response payload. + let result = connections::call_tool( + &server.server_id, + "echo", + serde_json::json!({ "message": "hello mcp" }), + ) + .await + .expect("call_tool succeeds"); + + let text = result + .get("content") + .and_then(|c| c.as_array()) + .and_then(|arr| arr.first()) + .and_then(|first| first.get("text")) + .and_then(|t| t.as_str()) + .unwrap_or(""); + assert_eq!(text, "hello mcp", "echo tool returns the input verbatim"); + + // Disconnect: removes from the registry and closes the subprocess. + let removed = connections::disconnect(&server.server_id).await; + assert!(removed, "disconnect drops the live connection"); + + // Subsequent call fails because the server_id is no longer connected. + let err = connections::call_tool( + &server.server_id, + "echo", + serde_json::json!({ "message": "post-disconnect" }), + ) + .await + .expect_err("call_tool fails after disconnect"); + assert!(err.contains("not connected")); +} + +#[tokio::test] +async fn unknown_tool_call_returns_error() { + let (_tmp, cfg) = fresh_workspace_config(); + let server = make_installed_server(); + + store::insert_server(&cfg, &server).expect("insert installed server"); + + connections::connect(&cfg, &server).await.expect("connect"); + + let err = connections::call_tool(&server.server_id, "does_not_exist", serde_json::json!({})) + .await + .expect_err("stub rejects unknown tools"); + assert!( + err.to_lowercase().contains("unknown tool") || err.contains("error"), + "expected unknown-tool error, got: {err}" + ); + + let _ = connections::disconnect(&server.server_id).await; +} diff --git a/tests/mcp_setup_e2e.rs b/tests/mcp_setup_e2e.rs new file mode 100644 index 000000000..b2e34f898 --- /dev/null +++ b/tests/mcp_setup_e2e.rs @@ -0,0 +1,93 @@ +//! End-to-end test for the MCP setup-agent flow. +//! +//! Exercises the ref machinery + install_and_connect path without going +//! through a real upstream registry — the test inserts an +//! `InstalledServer` row directly to stand in for what +//! `install_and_connect` would have synthesised from +//! `registry::registry_get`. The transport itself is the same +//! `test-mcp-stub` binary used by `mcp_registry_e2e.rs`. + +use std::collections::HashMap; +use std::time::Duration; + +use openhuman_core::openhuman::config::Config; +use openhuman_core::openhuman::mcp_registry::setup::{self, SecretRef}; + +#[tokio::test] +async fn request_secret_blocks_until_submit_then_resolves() { + // Caller mints + awaits in one task, fulfiller submits in another. + // The exact API the setup_ops::request_secret handler uses. + let (r, rx) = setup::mint_request("API_KEY").await; + + let r_for_submit = r.clone(); + let submit_task = tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(20)).await; + let submitted = setup::fulfill(&r_for_submit, "shh-secret".to_string()).await; + assert!(submitted, "fulfill returns true on first submit"); + }); + + // The await side: must not return before fulfill is called. + setup::await_fulfillment(&r, rx) + .await + .expect("await_fulfillment completes once submit lands"); + submit_task.await.unwrap(); + + // Resolve maps {KEY: ref} -> {KEY: value} without exposing value to + // anywhere it shouldn't be. + let mut refs = HashMap::new(); + refs.insert("API_KEY".to_string(), r.clone()); + let resolved = setup::resolve_refs(&refs).await.expect("resolves"); + assert_eq!( + resolved, + vec![("API_KEY".to_string(), "shh-secret".to_string())] + ); + + // The setup-agent contract: once install_and_connect persists the + // values, the refs are gone. + let _ = setup::consume_refs(&refs).await.expect("consumes"); + assert!( + setup::resolve_refs(&refs).await.is_err(), + "post-consume resolve fails" + ); +} + +#[tokio::test] +async fn test_connection_against_stub_returns_tools() { + use openhuman_core::openhuman::mcp_client::McpStdioClient; + + // Mirror what setup_ops::test_connection does end-to-end, minus the + // registry::registry_get step (we don't want to hit a real upstream + // from CI). The point of this test is the spawn + initialize + + // list_tools + teardown lifecycle the setup agent relies on. + let (r, _rx) = setup::mint_request("ECHO_TOKEN").await; + assert!(setup::fulfill(&r, "ignored-by-stub".to_string()).await); + + let mut refs = HashMap::new(); + refs.insert("ECHO_TOKEN".to_string(), r); + let env = setup::resolve_refs(&refs).await.expect("resolves"); + assert_eq!(env.len(), 1); + + let stub_path = env!("CARGO_BIN_EXE_test-mcp-stub"); + let cfg = Config::default(); + let identity = cfg.mcp_client.client_identity.clone(); + let client = McpStdioClient::new(stub_path.to_string(), Vec::new(), env, None, identity); + + client.initialize().await.expect("stub initialises"); + let tools = client.list_tools().await.expect("stub lists tools"); + assert_eq!(tools.len(), 1); + assert_eq!(tools[0].name, "echo"); + + client.close_session().await.expect("stub closes"); +} + +#[tokio::test] +async fn invalid_ref_id_rejected_by_submit_secret() { + // The submit_secret handler parses the ref id via SecretRef::parse. + // Validate the parser independently here so an upstream regression + // doesn't silently re-admit unsafe inputs. + assert!(SecretRef::parse("secret://abc123").is_some()); + assert!(SecretRef::parse("abc123").is_some()); + assert!(SecretRef::parse("secret://not-hex!!").is_none()); + assert!(SecretRef::parse("").is_none()); + assert!(SecretRef::parse("../../etc/passwd").is_none()); +}