From 16aa09aca5fa10a2fda864317ff8e4e1163a16bc Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 3 Jul 2026 05:08:28 -0500 Subject: [PATCH] =?UTF-8?q?feat(mcp):=20first-class=20MCP=20tab=20?= =?UTF-8?q?=E2=80=94=20catalog,=20GUI=20auth/probe/logs,=20per-tool=20gati?= =?UTF-8?q?ng?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Cursor-style MCP manager inside Capabilities, plus the backend it needs. - Server list with brand/favicon avatars + live status dot and a capability summary (N tools, M prompts, K resources); Servers | Catalog views. - Catalog: one-click install of Nous-approved servers with required-env prompts. - GUI OAuth: Authenticate opens the system browser from the TTY-less backend and verifies a token actually lands; header/API-key servers are never pushed down OAuth; a dirty mcp.json can't drop a freshly-persisted auth field. - Full-width mcp.json editor (ecosystem document format) + pinned stdio/agent LogTail; probes cached 5m and keyed by (profile, config) so revisiting never respawns the fleet or shows a stale probe. - Whole-map persistence (PUT /api/mcp/servers) so deletes/toggles actually stick (the generic /api/config deep-merge could not remove keys). - perf: MCP probe/auth no longer hold the global skills lock, so a slow stdio spawn can't stall every other request into a 15s timeout. - per-tool include/exclude gating (lib/mcp-tool-filter) mirroring the CLI loader. --- apps/desktop/src/app/skills/mcp-tab.tsx | 1481 ++++++++++++++++++ apps/desktop/src/hermes.test.ts | 4 +- apps/desktop/src/hermes.ts | 59 +- apps/desktop/src/lib/mcp-tool-filter.test.ts | 74 + apps/desktop/src/lib/mcp-tool-filter.ts | 61 + apps/desktop/src/types/hermes.ts | 12 + hermes_cli/logs.py | 3 + hermes_cli/mcp_config.py | 58 +- hermes_cli/web_server.py | 167 +- tests/hermes_cli/test_mcp_config.py | 21 + tools/mcp_oauth.py | 27 + tools/mcp_tool.py | 28 +- 12 files changed, 1959 insertions(+), 36 deletions(-) create mode 100644 apps/desktop/src/app/skills/mcp-tab.tsx create mode 100644 apps/desktop/src/lib/mcp-tool-filter.test.ts create mode 100644 apps/desktop/src/lib/mcp-tool-filter.ts diff --git a/apps/desktop/src/app/skills/mcp-tab.tsx b/apps/desktop/src/app/skills/mcp-tab.tsx new file mode 100644 index 000000000..230f3f82d --- /dev/null +++ b/apps/desktop/src/app/skills/mcp-tab.tsx @@ -0,0 +1,1481 @@ +import { + SiFigma, + SiGithub, + SiGitlab, + SiLinear, + SiNotion, + SiPostgresql, + SiSentry, + SiStripe, + SiSupabase, + SiVercel +} from '@icons-pack/react-simple-icons' +import { useStore } from '@nanostores/react' +import { useQuery } from '@tanstack/react-query' +import { type ComponentType, type SVGProps, useEffect, useMemo, useRef, useState } from 'react' + +import { type CodeEditorApi } from '@/components/chat/code-editor' +import { JsonDocumentEditor } from '@/components/chat/json-document-editor' +import { LogTail } from '@/components/chat/log-tail' +import { PageLoader } from '@/components/page-loader' +import { Button } from '@/components/ui/button' +import { Codicon } from '@/components/ui/codicon' +import { ErrorBanner } from '@/components/ui/error-state' +import { Input } from '@/components/ui/input' +import { Switch } from '@/components/ui/switch' +import { TextTab } from '@/components/ui/text-tab' +import { + authMcpServer, + getLogs, + getMcpCatalog, + type HermesGateway, + installMcpCatalogEntry, + type McpCatalogEntry, + type McpTestResult, + saveMcpServers, + testMcpServer +} from '@/hermes' +import { useI18n } from '@/i18n' +import { countEnabledTools, isToolEnabled, toggleToolInServer } from '@/lib/mcp-tool-filter' +import { cn } from '@/lib/utils' +import { notify, notifyError } from '@/store/notifications' +import { $activeGatewayProfile, normalizeProfileKey } from '@/store/profile' +import { $activeSessionId } from '@/store/session' +import type { HermesConfigRecord } from '@/types/hermes' + +import { invalidateHermesConfig, setHermesConfigCache, useHermesConfigRecord } from '../hooks/use-config-record' +import { useOnProfileSwitch } from '../hooks/use-on-profile-switch' +import { DetailPane, ICON_BUTTON, MASTER_DETAIL_WIDE_COLS } from '../master-detail' +import { PanelAddButton, PanelEmpty } from '../overlays/panel' +import { prettyName } from '../settings/helpers' +import { useDeepLinkHighlight } from '../settings/use-deep-link-highlight' + +type McpServers = Record> + +// The editor always speaks the ecosystem's mcp.json document format — names +// are the JSON keys, transport is inferred from `command` vs `url` — so any +// README's "add this to your mcp.json" snippet pastes verbatim. Storage stays +// the config.yaml `mcp_servers` map (CLI/TUI untouched). +const STARTER_ENTRY = { command: 'npx', args: ['-y', '@modelcontextprotocol/server-filesystem', '/path/to/dir'] } + +const pretty = (value: unknown) => JSON.stringify(value, null, 2) +const wrapDoc = (entries: McpServers) => pretty({ mcpServers: entries }) + +const isServerShape = (value: Record) => + typeof value.command === 'string' || typeof value.url === 'string' + +// Cursor/Claude write `type`; Hermes reads `transport`. Normalize on the way +// in so pasted configs behave identically under the CLI/TUI loader. +function normalizeEntry(entry: Record): Record { + if (typeof entry.type === 'string' && entry.transport === undefined) { + const { type, ...rest } = entry + + return { ...rest, transport: type } + } + + return entry +} + +/** Accepts `{"mcpServers": {...}}` (ecosystem), a bare name→config map, or throws. */ +function parseServersDoc(raw: string): McpServers { + const parsed = JSON.parse(raw) as unknown + + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + throw new Error('Expected a JSON object') + } + + const doc = parsed as Record + + if (isServerShape(doc)) { + throw new Error('Wrap the server in {"mcpServers": {"name": …}} so it has a name') + } + + const wrapper = doc.mcpServers ?? doc.mcp_servers + + const map = + wrapper && typeof wrapper === 'object' && !Array.isArray(wrapper) ? (wrapper as McpServers) : (doc as McpServers) + + return Object.fromEntries(Object.entries(map).map(([name, entry]) => [name, normalizeEntry(entry)])) +} + +function getServers(config: HermesConfigRecord | null): McpServers { + const raw = config?.mcp_servers + + return raw && typeof raw === 'object' && !Array.isArray(raw) ? (raw as McpServers) : {} +} + +// The runtime gate is `enabled: false` — the same flag `hermes mcp` and the +// agent's MCP loader read. +const serverEnabled = (server: Record) => server.enabled !== false + +const NEEDS_AUTH_RE = /\b(401|unauthorized|forbidden|invalid[_ ]?token|authentication|oauth)\b/i + +// Shared cache for the Nous-approved catalog — feeds both description enrichment +// and the Catalog install view; invalidated after an install. +const MCP_CATALOG_KEY = ['mcp-catalog'] as const + +// Probe results outlive the component: each probe is a REAL connect/disconnect +// (stdio servers get spawned!), so re-entering the page must not re-probe the +// fleet. Manual refresh / auth / toggle-on bypass the cache. +const PROBE_TTL_MS = 5 * 60_000 +const probeCache = new Map() + +// A probe is only valid for one (profile, exact-config) pair. Keying the cache +// by a fingerprint of the connection-relevant fields — plus the active profile +// — means a same-name edit (url/command/env change) or a same-named server in +// another profile MISSES the cache instead of showing a stale probe. +const serverFingerprint = (server: Record): string => + JSON.stringify([server.url, server.command, server.args, server.env, server.headers, server.transport, server.auth]) + +const probeKey = (name: string, server: Record | undefined): string => + `${normalizeProfileKey($activeGatewayProfile.get())}::${name}::${serverFingerprint(server ?? {})}` + +type Probe = McpTestResult | 'probing' + +type ServerStatus = 'off' | 'probing' | 'ok' | 'needs-auth' | 'error' | 'unknown' + +function statusOf(server: Record, probe: Probe | undefined): ServerStatus { + if (!serverEnabled(server)) { + return 'off' + } + + if (probe === 'probing') { + return 'probing' + } + + if (!probe) { + return 'unknown' + } + + if (probe.ok) { + return 'ok' + } + + return NEEDS_AUTH_RE.test(probe.error ?? '') ? 'needs-auth' : 'error' +} + +const STATUS_DOT: Record = { + ok: 'bg-emerald-500', + error: 'bg-red-500', + 'needs-auth': 'bg-amber-500', + probing: 'animate-pulse bg-foreground/40', + off: 'bg-foreground/20', + unknown: 'bg-foreground/20' +} + +// "12 tools enabled" / "25 tools, 1 prompts, 103 resources enabled" — only +// the capabilities the server actually has. When a `server` config is passed, +// the tool count reflects the per-tool include/exclude filter (what's actually +// registered), not the raw discovered count. +// TODO(i18n): literals until the UX settles. +function capabilitySummary(probe: McpTestResult, server?: Record): string { + const toolCount = server ? countEnabledTools(server, probe.tools.map(tool => tool.name)) : probe.tools.length + + const parts = [ + `${toolCount} tools`, + ...(probe.prompts ? [`${probe.prompts} prompts`] : []), + ...(probe.resources ? [`${probe.resources} resources`] : []) + ] + + return `${parts.join(', ')} enabled` +} + +// TODO(i18n): literals until the UX settles. +function statusLine(status: ServerStatus, probe: Probe | undefined, server?: Record): string { + switch (status) { + case 'ok': + return capabilitySummary(probe as McpTestResult, server) + + case 'probing': + return 'Connecting…' + + case 'needs-auth': + return 'Needs authentication' + + case 'error': + return 'Error' + + case 'off': + return 'Off' + + default: + return '' + } +} + +// --------------------------------------------------------------------------- +// Cursor → server-block mapping. A tolerant character walker (not JSON.parse — +// it must work mid-edit) that finds each server's key+object range inside the +// mcpServers container, so the editor cursor selects a server and the block +// can be highlighted. +// --------------------------------------------------------------------------- + +interface ServerBlock { + from: number + name: string + to: number +} + +function scanServerBlocks(text: string): ServerBlock[] { + const skipString = (index: number): number => { + let i = index + 1 + + while (i < text.length) { + if (text[i] === '\\') { + i += 2 + } else if (text[i] === '"') { + return i + 1 + } else { + i++ + } + } + + return i + } + + // Container: the object after "mcpServers"/"mcp_servers", else the doc root. + let start = -1 + const wrapper = /"mcpServers"|"mcp_servers"/.exec(text) + + if (wrapper) { + let i = wrapper.index + wrapper[0].length + + while (i < text.length && text[i] !== '{') { + i++ + } + + start = i + } else { + start = text.indexOf('{') + } + + if (start < 0 || text[start] !== '{') { + return [] + } + + const blocks: ServerBlock[] = [] + let i = start + 1 + + while (i < text.length) { + const ch = text[i] + + if (ch === '}') { + break + } + + if (ch !== '"') { + i++ + + continue + } + + const keyStart = i + const keyEnd = skipString(i) + const name = text.slice(keyStart + 1, keyEnd - 1) + i = keyEnd + + while (i < text.length && text[i] !== ':') { + i++ + } + + i++ + + while (i < text.length && /\s/.test(text[i])) { + i++ + } + + if (text[i] === '{') { + let depth = 0 + let j = i + + while (j < text.length) { + const c = text[j] + + if (c === '"') { + j = skipString(j) + + continue + } + + if (c === '{') { + depth++ + } else if (c === '}') { + depth-- + + if (depth === 0) { + j++ + + break + } + } + + j++ + } + + blocks.push({ from: keyStart, name, to: j }) + i = j + } else { + // Non-object value — skip to the next sibling. + while (i < text.length && text[i] !== ',' && text[i] !== '}') { + if (text[i] === '"') { + i = skipString(i) + + continue + } + + i++ + } + } + } + + return blocks +} + +export function McpTab({ gateway }: { gateway: HermesGateway | null }) { + const { t } = useI18n() + const m = t.settings.mcp + const activeSessionId = useStore($activeSessionId) + + // Shared config cache (see use-config-record): revisiting the tab paints the + // cached record instantly; mutations write through `setConfig` and stay + // visible to the other settings surfaces. + const { + data: config, + isLoading: configLoading, + isError: configFailed, + error: configError, + refetch: refetchConfig + } = useHermesConfigRecord() + + const setConfig = setHermesConfigCache + + const [saving, setSaving] = useState(false) + const [probes, setProbes] = useState>({}) + const probesRef = useRef(probes) + probesRef.current = probes + + // Master document draft. `docVersion` remounts the editor when the draft is + // regenerated programmatically (list-side mutations); `dirty` guards user + // edits from being clobbered by those regenerations. + const [draft, setDraft] = useState('') + const [dirty, setDirty] = useState(false) + const [docVersion, setDocVersion] = useState(0) + const [logSource, setLogSource] = useState<'stdio' | 'agent'>('stdio') + + // Selection IS the editor cursor: whichever server block contains it is the + // configured server on the left. Cursor outside every block → the list. + const editorApi = useRef(null) + const [cursor, setCursor] = useState(0) + const blocks = useMemo(() => scanServerBlocks(draft), [draft]) + + const activeBlock = useMemo( + () => blocks.find(block => cursor >= block.from && cursor <= block.to) ?? null, + [blocks, cursor] + ) + + const selected = activeBlock?.name ?? null + + const focusServer = (name: string) => { + const block = blocks.find(b => b.name === name) + + if (block) { + // Land just inside the key so the block claims the cursor. + editorApi.current?.setCursor(block.from + 1) + setCursor(block.from + 1) + } + } + + const servers = useMemo(() => getServers(config ?? null), [config]) + + // Config/document order, not alphabetical — the list mirrors mcp.json. + const names = useMemo(() => Object.keys(servers), [servers]) + + // Left column view: the configured fleet, or the Nous-approved catalog to + // install from. Both share one cached catalog fetch (also feeds description + // enrichment below), so switching between them never re-requests. + const [leftView, setLeftView] = useState<'catalog' | 'servers'>('servers') + const catalogQuery = useQuery({ queryKey: MCP_CATALOG_KEY, queryFn: getMcpCatalog, staleTime: 5 * 60_000 }) + const catalog = catalogQuery.data?.entries ?? [] + + const descriptionFor = (serverName: string, server: Record): null | string => { + const lower = serverName.toLowerCase() + + const match = catalog.find( + entry => + entry.name.toLowerCase() === lower || + (entry.url && entry.url === server.url) || + (entry.command && entry.command === server.command) + ) + + return match?.description ?? null + } + + const resetDraft = (entries: McpServers) => { + setDraft(wrapDoc(entries)) + setDirty(false) + setDocVersion(version => version + 1) + } + + // Mirror a list-side mutation into a dirty draft without losing the user's + // other edits. Unparseable drafts are left alone — save resolves the race. + const patchDraft = (mutate: (doc: McpServers) => McpServers) => { + try { + setDraft(wrapDoc(mutate(parseServersDoc(draft)))) + setDocVersion(version => version + 1) + } catch { + // Draft is mid-edit / invalid JSON; the user's text wins until save. + } + } + + // Seed the editor draft from config exactly once, the first time it lands. + // Background refetches thereafter update the list but must not clobber an + // in-progress edit — the draft is the user's until they save or reset. + const draftSeeded = useRef(false) + + useEffect(() => { + if (config && !draftSeeded.current) { + draftSeeded.current = true + resetDraft(getServers(config)) + } + }, [config]) + + // A profile switch invalidates the config query (see store/profile.ts), which + // refetches the new backend's mcp.json. Reset per-profile view state so the + // draft reseeds for the new profile and the old profile's probes don't linger + // (the probe cache is already profile-keyed, so this just forces a re-probe). + useOnProfileSwitch(() => { + draftSeeded.current = false + setProbes({}) + setCursor(0) + }) + + useDeepLinkHighlight({ + block: 'nearest', + elementId: serverName => `mcp-server-${serverName}`, + onResolve: focusServer, + param: 'server', + ready: serverName => blocks.some(block => block.name === serverName) + }) + + const runProbe = async (serverName: string) => { + const key = probeKey(serverName, servers[serverName]) + setProbes(current => ({ ...current, [serverName]: 'probing' })) + + try { + const result = await testMcpServer(serverName) + probeCache.set(key, { at: Date.now(), result }) + setProbes(current => ({ ...current, [serverName]: result })) + } catch (err) { + const result = { ok: false, error: err instanceof Error ? err.message : String(err), tools: [] } + probeCache.set(key, { at: Date.now(), result }) + setProbes(current => ({ ...current, [serverName]: result })) + } + } + + // First-class OAuth: opens the system browser, blocks until the flow lands a + // token (verified on disk — a friendly tools/list is not proof), then the + // auth result doubles as the probe (it carries the tool list). + const [authing, setAuthing] = useState(null) + + const authenticate = async (serverName: string) => { + setAuthing(serverName) + setProbes(current => ({ ...current, [serverName]: 'probing' })) + + try { + const result = await authMcpServer(serverName) + setProbes(current => ({ ...current, [serverName]: result })) + // Cache under the POST-auth fingerprint (auth: oauth) on success — that's + // the config the mount effect will read back, so it hits this entry. + const probedConfig = result.ok ? { ...servers[serverName], auth: 'oauth' } : servers[serverName] + probeCache.set(probeKey(serverName, probedConfig), { at: Date.now(), result }) + + if (result.ok) { + // The endpoint persisted `auth: oauth` — mirror it locally. + const nextServers = { ...servers, [serverName]: { ...servers[serverName], auth: 'oauth' } } + setConfig(current => (current ? { ...current, mcp_servers: nextServers } : current)) + + // Mirror `auth: oauth` into the editor too. If we only reset a clean + // draft, a dirty draft keeps the pre-auth text and the next Save would + // drop the freshly-persisted auth field — so patch the dirty draft in + // place instead of clobbering the user's other edits. + if (dirty) { + patchDraft(doc => (doc[serverName] ? { ...doc, [serverName]: { ...doc[serverName], auth: 'oauth' } } : doc)) + } else { + resetDraft(nextServers) + } + + // TODO(i18n): literal until the UX settles. + notify({ kind: 'success', title: 'Authenticated', message: `${serverName}: ${result.tools.length} tools` }) + void silentReload() + } else if (result.error) { + notifyError(new Error(result.error), serverName) + } + } catch (err) { + setProbes(current => ({ + ...current, + [serverName]: { ok: false, error: err instanceof Error ? err.message : String(err), tools: [] } + })) + notifyError(err, serverName) + } finally { + setAuthing(null) + } + } + + // It should just know: probe enabled servers as config arrives — but through + // the cache, so revisiting the page doesn't respawn/reconnect the fleet. + useEffect(() => { + for (const [serverName, server] of Object.entries(servers)) { + if (!serverEnabled(server) || probesRef.current[serverName] !== undefined) { + continue + } + + const cached = probeCache.get(probeKey(serverName, server)) + + if (cached && Date.now() - cached.at < PROBE_TTL_MS) { + setProbes(current => ({ ...current, [serverName]: cached.result })) + } else { + void runProbe(serverName) + } + } + // Re-run only when the server set changes; runProbe is recreated every + // render and adding it would re-probe the fleet on every keystroke. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [servers]) + + // Config writes reach live sessions immediately — no manual "Reload MCP". + const silentReload = async () => { + if (!gateway) { + return + } + + try { + await gateway.request('reload.mcp', { confirm: true, session_id: activeSessionId ?? undefined }) + } catch (err) { + notifyError(err, m.reloadFailed) + } + } + + // Whole-map replace (NOT saveHermesConfig, which deep-merges and so can never + // delete a server, drop `enabled: false`, or remove a nested field). Only + // after the replace lands do we write the cache through + reload live sessions. + const persist = async (nextServers: McpServers) => { + await saveMcpServers(nextServers) + setConfig(current => ({ ...current, mcp_servers: nextServers })) + void silentReload() + } + + // A catalog install wrote a new server into config.yaml on the backend — + // refresh both the config (so the fleet + editor pick it up) and the catalog + // (installed state), then reload live sessions. + const onCatalogInstalled = () => { + void invalidateHermesConfig() + void catalogQuery.refetch() + void silentReload() + } + + const withEnabled = (server: Record, enabled: boolean) => { + const next = { ...server } + + if (enabled) { + delete next.enabled + } else { + next.enabled = false + } + + return next + } + + const toggleServer = async (serverName: string, enabled: boolean) => { + try { + await persist({ ...servers, [serverName]: withEnabled(servers[serverName], enabled) }) + + if (dirty) { + patchDraft(doc => (doc[serverName] ? { ...doc, [serverName]: withEnabled(doc[serverName], enabled) } : doc)) + } else { + resetDraft({ ...servers, [serverName]: withEnabled(servers[serverName], enabled) }) + } + + if (enabled) { + void runProbe(serverName) + } + } catch (err) { + notifyError(err, m.saveFailed) + } + } + + // Per-tool gating writes the server's `tools.include`/`tools.exclude` and + // persists like any other config change (immediate reload of live sessions). + // The probe still lists every discovered tool; the filter decides which ones + // the agent actually registers. + const toggleTool = async (serverName: string, toolName: string) => { + const base = servers[serverName] + + if (!base) { + return + } + + const next = toggleToolInServer(base, toolName) + + try { + await persist({ ...servers, [serverName]: next }) + + if (dirty) { + patchDraft(doc => + doc[serverName] ? { ...doc, [serverName]: toggleToolInServer(doc[serverName], toolName) } : doc + ) + } else { + resetDraft({ ...servers, [serverName]: next }) + } + } catch (err) { + notifyError(err, m.saveFailed) + } + } + + const removeServer = async (serverName: string) => { + setSaving(true) + + try { + const next = { ...servers } + delete next[serverName] + + await persist(next) + + if (dirty) { + patchDraft(doc => { + const patched = { ...doc } + delete patched[serverName] + + return patched + }) + } else { + resetDraft(next) + } + + setCursor(0) + } catch (err) { + notifyError(err, m.removeFailed) + } finally { + setSaving(false) + } + } + + // "+" seeds a starter entry into the document (unique key) and marks it + // dirty — naming happens in the editor, like every other mcp.json. + const addServer = () => { + let base: McpServers + + try { + base = parseServersDoc(draft) + } catch { + base = { ...servers } + } + + let key = 'my-server' + + for (let i = 2; key in base; i++) { + key = `my-server-${i}` + } + + const nextDraft = wrapDoc({ ...base, [key]: STARTER_ENTRY }) + setDraft(nextDraft) + setDirty(true) + setDocVersion(version => version + 1) + + // Focus the fresh block once the editor remounts with the new doc. + const from = nextDraft.indexOf(`"${key}"`) + + if (from >= 0) { + requestAnimationFrame(() => { + editorApi.current?.setCursor(from + 1) + setCursor(from + 1) + }) + } + } + + const saveDoc = async () => { + let entries: McpServers + + try { + entries = parseServersDoc(draft) + } catch (err) { + notifyError(err, m.invalidJson) + + return + } + + setSaving(true) + + const prevServers = servers + + try { + await persist(entries) + resetDraft(entries) + // Keep only probes for servers that survived AND kept the same config; + // removed OR edited entries drop their probe so the mount effect re-probes + // the new shape (the cache also misses on the changed fingerprint). + setProbes(current => + Object.fromEntries( + Object.entries(current).filter( + ([name]) => + name in entries && serverFingerprint(entries[name]) === serverFingerprint(prevServers[name] ?? {}) + ) + ) + ) + notify({ kind: 'success', title: m.savedTitle, message: m.savedMessage('mcp.json') }) + } catch (err) { + notifyError(err, m.saveFailed) + } finally { + setSaving(false) + } + } + + // Cached data paints instantly; a spinner only ever shows on the first-ever + // load, and a failed load gets a real retry — never a silent blank pane. + if (configFailed && !config) { + return ( +
+ + + {configError instanceof Error ? configError.message : m.failedLoad} + + + +
+ ) + } + + if (!config) { + return + } + + // Zero servers and a pristine doc: one centered invitation — with a path into + // the catalog (kept out when the user is already browsing it). + if (Object.keys(servers).length === 0 && !dirty && leftView === 'servers') { + return ( +
+ + + + + } + description={m.emptyDesc} + icon="plug" + title={m.emptyTitle} + /> +
+ ) + } + + // Selection may reference an unsaved block (freshly pasted) — fall back to + // the draft's parsed entry so the config pane can still describe it. + const savedEntry = selected ? servers[selected] : undefined + + const draftEntry = (() => { + if (!selected || savedEntry) { + return undefined + } + + try { + return parseServersDoc(draft)[selected] + } catch { + return undefined + } + })() + + const activeEntry = savedEntry ?? draftEntry + + return ( +
+ {/* LEFT: the focused block's server config, or the fleet list / catalog. */} + + + {/* RIGHT: the mcp.json editor, logs hard-pinned below. */} +
+ + mcp.json + {dirty && } + + } + highlight={activeBlock ? { from: activeBlock.from, to: activeBlock.to } : null} + initialValue={draft} + onChange={next => { + setDraft(next) + setDirty(true) + }} + onCursorChange={setCursor} + onFormatJsonError={error => notifyError(new Error(error), m.invalidJson)} + onSave={() => void saveDoc()} + remountKey={docVersion} + trailing={ + + } + /> + + {(['stdio', 'agent'] as const).map(kind => ( + setLogSource(kind)} + > + {kind} + + ))} + + } + defaultHeight={176} + id="mcp-logs" + title={ + // TODO(i18n): literal until the UX settles. + + {selected && savedEntry ? selected : 'All servers'} + + } + > + + +
+
+ ) +} + +// --------------------------------------------------------------------------- +// Left column: one server's config (mirrors the block under the cursor). +// --------------------------------------------------------------------------- + +function ServerConfig({ + authing, + description, + entry, + name, + onAuthenticate, + onBack, + onProbe, + onRemove, + onToggle, + onToggleTool, + probe, + saved, + saving +}: { + authing: boolean + description: null | string + entry: Record + name: string + onAuthenticate: () => void + onBack: () => void + onProbe: () => void + onRemove: () => void + onToggle: (checked: boolean) => void + onToggleTool: (toolName: string) => void + probe: Probe | undefined + saved: boolean + saving: boolean +}) { + const { t } = useI18n() + const m = t.settings.mcp + const status = statusOf(entry, probe) + + // OAuth is only offered to servers that are actually OAuth-shaped. A server + // with `headers` uses API-key/bearer auth — a 401 there means a bad key, NOT + // "log in with OAuth"; routing it through the browser flow would wrongly + // rewrite its config to `auth: oauth`. So: explicit `auth: oauth` can re-auth + // on failure; an auth-less HTTP server may try OAuth on a 401; header servers + // never do. + const hasHeaderAuth = !!entry.headers && typeof entry.headers === 'object' + + const canAuth = + typeof entry.url === 'string' && + !hasHeaderAuth && + (entry.auth === 'oauth' ? status === 'needs-auth' || status === 'error' : !entry.auth && status === 'needs-auth') + + const summary = probe && probe !== 'probing' && probe.ok ? capabilitySummary(probe, entry) : null + + return ( + // p-2 matches the list view's container so flipping list ⇄ config keeps + // content anchored at the same origin. +
+ {/* Geometry cloned from McpRow so nothing jumps when flipping list ⇄ + config: items-start with per-element top margins that reproduce the + row's h-11 centering exactly (h-5 controls → mt-3, size-6 avatar → + mt-2.5, h-4 switch → mt-3.5) no matter how tall the text column gets. */} +
+ + +
+

{prettyName(name)}

+

+ {typeof entry.url === 'string' ? entry.url : [entry.command, ...((entry.args as string[]) ?? [])].join(' ')} +

+ {summary &&

{summary}

} +
+ {saved && ( + // Direct row children (no wrapper): the icons↔switch gap must be the + // row's own gap-2, byte-identical to McpRow. + <> + + + + )} +
+ + {description && ( +

+ {description} +

+ )} + + {canAuth && saved && ( +
+ +
+ )} + {!saved && ( + // TODO(i18n): literal until the UX settles. +

Unsaved — save mcp.json to connect.

+ )} + + {status === 'probing' && } + + {/* No inline error dump — the status dot/line says "Error"/"Needs + authentication", and the actual failure lands in the logs pane below + (and the console). A big red block here just shouts the same thing. */} + + {probe && probe !== 'probing' && probe.ok && probe.tools.length > 0 && ( +
+ {/* Chip = a discovered tool; click to include/exclude it (struck + through when excluded, so it won't register). The probe always + lists every tool regardless of the filter. + TODO(i18n): titles are literal until the UX settles. */} + {probe.tools.map(tool => { + const on = isToolEnabled(entry, tool.name) + + return ( + + ) + })} +
+ )} +
+ ) +} + +// The enable toggle, shared by the row and the config header. It reflects the +// configured `enabled` flag ONLY — full-strength when on, dimmed when off — so +// "is this on?" reads instantly from config, never gated on a probe that can +// take seconds (stdio servers spawn `npx`). Whether it's actually *connected* +// is the status dot's job, not the switch's. +function ServerSwitch({ + className, + disabled, + enabled, + name, + onToggle +}: { + className?: string + disabled: boolean + enabled: boolean + name: string + onToggle: (checked: boolean) => void +}) { + return ( + + ) +} + +// Refresh + delete, identical beside every toggle (rows and config header). +function ServerIconActions({ + className, + onProbe, + onRemove, + probing, + saving +}: { + className?: string + onProbe: () => void + onRemove: () => void + probing: boolean + saving: boolean +}) { + const { t } = useI18n() + const m = t.settings.mcp + + return ( + + + + + ) +} + +// Small gray attribute chip (transport / auth / needs-build), matching the +// catalog's flat row treatment. +function CatalogTag({ children }: { children: string }) { + return ( + + {children} + + ) +} + +// The Nous-approved MCP catalog: one-click installs of curated servers, with an +// inline prompt for any required credentials (never shows stored values). On +// install the parent refetches config + catalog and reloads live sessions. +function McpCatalog({ + entries, + loading, + onInstalled +}: { + entries: McpCatalogEntry[] + loading: boolean + onInstalled: () => void +}) { + const { t } = useI18n() + const m = t.settings.mcp + const [installing, setInstalling] = useState(null) + const [envDrafts, setEnvDrafts] = useState>>({}) + const [envOpenFor, setEnvOpenFor] = useState(null) + + const install = async (entry: McpCatalogEntry) => { + const required = entry.required_env.filter(env => env.required) + const draft = envDrafts[entry.name] ?? {} + + // Reveal the credential prompt first; only error once it's shown and unfilled. + if (required.some(env => !draft[env.name]?.trim())) { + if (envOpenFor !== entry.name) { + setEnvOpenFor(entry.name) + + return + } + + notify({ kind: 'error', title: m.catalogEnvPrompt(entry.name), message: m.catalogEnvRequired }) + + return + } + + setInstalling(entry.name) + + try { + await installMcpCatalogEntry(entry.name, draft) + notify({ kind: 'success', title: m.catalogInstallStarted(entry.name), message: '' }) + setEnvOpenFor(null) + onInstalled() + } catch (err) { + notifyError(err, m.catalogInstallFailed(entry.name)) + } finally { + setInstalling(null) + } + } + + if (loading) { + return + } + + if (entries.length === 0) { + return + } + + return ( +
+ {entries.map(entry => { + const draft = envDrafts[entry.name] ?? {} + + return ( +
+
+ {/* 2px nudge so the start-aligned avatar sits where McpRow's + center-aligned one does — no jump when flipping Servers⇄Catalog. */} + +
+
+ {prettyName(entry.name)} + {entry.transport} + {entry.auth_type === 'oauth' && OAuth} + {entry.auth_type === 'api_key' && API key} + {entry.needs_install && !entry.installed && {m.catalogNeedsInstall}} + {entry.installed && ( + + {entry.enabled ? m.catalogEnabled : m.catalogInstalled} + + )} +
+

{entry.description}

+ {envOpenFor === entry.name && entry.required_env.length > 0 && ( +
+ {entry.required_env.map(env => ( + + ))} +
+ )} +
+ +
+
+ ) + })} +
+ ) +} + +const LOG_POLL_MS = 2000 + +const STDIO_MARKER_RE = /^===== \[.*\] starting MCP server '(.+)' =====$/ + +// Keep only the stdio-log sections belonging to one server. The shared file +// has no per-line tags — sections start at that server's session marker and +// run until the next marker (any server's). +function filterStdioSections(lines: string[], server: string): string[] { + const out: string[] = [] + let inSection = false + + for (const line of lines) { + const marker = STDIO_MARKER_RE.exec(line.trim()) + + if (marker) { + inSection = marker[1] === server + } + + if (inSection) { + out.push(line) + } + } + + return out +} + +// The MCP output channel — Cursor's "MCP Logs" equivalent, pinned under the +// editor. Scope follows the cursor-selected server (all servers otherwise); +// source controls live in the pane header. Body is the app's tool-output +// surface: CodeCardBody typography + the floating hover-reveal copy button. +function McpLogs({ server, source }: { server: null | string; source: 'stdio' | 'agent' }) { + const [lines, setLines] = useState(null) + + useEffect(() => { + let cancelled = false + + const poll = async () => { + try { + const response = + source === 'stdio' + ? await getLogs({ file: 'mcp', lines: 500 }) + : await getLogs({ file: 'agent', lines: 300, search: server ?? 'mcp' }) + + if (!cancelled) { + setLines(source === 'stdio' && server ? filterStdioSections(response.lines, server) : response.lines) + } + } catch { + // Backend momentarily unavailable — keep the last tail. + } + } + + setLines(null) + void poll() + const timer = window.setInterval(() => void poll(), LOG_POLL_MS) + + return () => { + cancelled = true + window.clearInterval(timer) + } + }, [server, source]) + + // TODO(i18n): literal until the UX settles. + return +} + +// --------------------------------------------------------------------------- +// Avatars + list rows +// --------------------------------------------------------------------------- + +// Brand glyphs for well-known MCP providers, exactly the Messaging avatar +// treatment (simpleicons on a 16% brand tint). Unknown servers fall back to +// the same letter monogram Messaging uses. +const MCP_BRAND_ICONS: Record>; color: string }> = { + figma: { Icon: SiFigma, color: '#F24E1E' }, + github: { Icon: SiGithub, color: '#181717' }, + gitlab: { Icon: SiGitlab, color: '#FC6D26' }, + linear: { Icon: SiLinear, color: '#5E6AD2' }, + notion: { Icon: SiNotion, color: '#000000' }, + postgres: { Icon: SiPostgresql, color: '#4169E1' }, + postgresql: { Icon: SiPostgresql, color: '#4169E1' }, + sentry: { Icon: SiSentry, color: '#362D59' }, + stripe: { Icon: SiStripe, color: '#635BFF' }, + supabase: { Icon: SiSupabase, color: '#3FCF8E' }, + vercel: { Icon: SiVercel, color: '#000000' } +} + +const brandFor = (name: string) => { + const lower = name.toLowerCase() + + return MCP_BRAND_ICONS[lower] ?? Object.entries(MCP_BRAND_ICONS).find(([key]) => lower.includes(key))?.[1] ?? null +} + +// PlatformAvatar (messaging), copied 1:1 — same size, radius, type scale, and +// brand-tint treatment — plus a status dot overlay. Identity ladder: curated +// brand glyph → letter monogram. We deliberately do NOT fetch remote favicons: +// a configured MCP URL can be a private/internal host, and hitting Google's +// favicon service for it would leak that hostname off-box. +function McpAvatar({ className, name, status }: { className?: string; name: string; status: ServerStatus }) { + const brand = brandFor(name) + + return ( + + {brand ? ( + + ) : ( + name.charAt(0).toUpperCase() + )} + + + ) +} + +function McpRow({ + active, + busy, + enabled, + name, + onProbe, + onRemove, + onSelect, + onToggle, + status, + statusText +}: { + active: boolean + busy: boolean + enabled: boolean + name: string + onProbe: () => void + onRemove: () => void + onSelect: () => void + onToggle: (checked: boolean) => void + status: ServerStatus + statusText: string +}) { + return ( +
+ + + +
+ ) +} diff --git a/apps/desktop/src/hermes.test.ts b/apps/desktop/src/hermes.test.ts index 96c326484..e9ad04a55 100644 --- a/apps/desktop/src/hermes.test.ts +++ b/apps/desktop/src/hermes.test.ts @@ -73,9 +73,7 @@ describe('Hermes REST session helpers', () => { }) it('uses a longer timeout for active profile refresh during desktop startup', async () => { - api - .mockResolvedValueOnce({ current: 'default' }) - .mockResolvedValueOnce({ profiles: [] }) + api.mockResolvedValueOnce({ current: 'default' }).mockResolvedValueOnce({ profiles: [] }) await refreshActiveProfile() diff --git a/apps/desktop/src/hermes.ts b/apps/desktop/src/hermes.ts index c5a73391f..734f2a308 100644 --- a/apps/desktop/src/hermes.ts +++ b/apps/desktop/src/hermes.ts @@ -22,7 +22,6 @@ import type { LogsResponse, McpCatalogResponse, McpServerSummary, - McpServerTestResponse, MemoryProviderConfig, MemoryProviderOAuthStatus, MemoryStatusResponse, @@ -343,6 +342,7 @@ export function getLogs(params: { file?: string level?: string lines?: number + search?: string }): Promise { const query = new URLSearchParams() @@ -362,6 +362,10 @@ export function getLogs(params: { query.set('component', params.component) } + if (params.search) { + query.set('search', params.search) + } + const suffix = query.toString() return window.hermesDesktop.api({ @@ -592,6 +596,49 @@ export function toggleSkill(name: string, enabled: boolean): Promise<{ ok: boole }) } +export interface McpTestResult { + ok: boolean + error?: string + tools: { name: string; description: string }[] + /** Capability counts (absent on older backends / failed probes). */ + prompts?: number + resources?: number +} + +/** Connect to the server, list its tools, disconnect. Slow (spawns/handshakes + * for real) — well past the 15s default fetch timeout. */ +export function testMcpServer(name: string): Promise { + return window.hermesDesktop.api({ + ...profileScoped(), + path: `/api/mcp/servers/${encodeURIComponent(name)}/test`, + method: 'POST', + timeoutMs: 60_000 + }) +} + +/** Replace the whole `mcp_servers` map (the mcp.json editor's save). Unlike + * `saveHermesConfig`, this REPLACES rather than deep-merges, so deletes, + * re-enables (dropping `enabled: false`), and removed nested fields persist. */ +export function saveMcpServers(servers: Record>): Promise<{ ok: boolean }> { + return window.hermesDesktop.api<{ ok: boolean }>({ + ...profileScoped(), + path: '/api/mcp/servers', + method: 'PUT', + body: { servers } + }) +} + +/** Run the OAuth flow for an HTTP server — opens the system browser and blocks + * until the user finishes (or gives up), hence the very generous timeout. */ +export function authMcpServer(name: string): Promise { + return window.hermesDesktop.api({ + ...profileScoped(), + path: `/api/mcp/servers/${encodeURIComponent(name)}/auth`, + method: 'POST', + timeoutMs: 300_000 + }) +} + export function getToolsets(): Promise { return window.hermesDesktop.api({ ...profileScoped(), @@ -1033,16 +1080,6 @@ export function listMcpServers(): Promise<{ servers: McpServerSummary[] }> { }) } -export function testMcpServer(name: string): Promise { - return window.hermesDesktop.api({ - ...profileScoped(), - path: `/api/mcp/servers/${encodeURIComponent(name)}/test`, - method: 'POST', - // Connect + list tools can be slow for stdio servers that boot a process. - timeoutMs: 60_000 - }) -} - export function setMcpServerEnabled(name: string, enabled: boolean): Promise<{ ok: boolean }> { return window.hermesDesktop.api<{ ok: boolean }>({ ...profileScoped(), diff --git a/apps/desktop/src/lib/mcp-tool-filter.test.ts b/apps/desktop/src/lib/mcp-tool-filter.test.ts new file mode 100644 index 000000000..ade59df5e --- /dev/null +++ b/apps/desktop/src/lib/mcp-tool-filter.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from 'vitest' + +import { countEnabledTools, isToolEnabled, readToolsFilter, toggleToolInServer } from './mcp-tool-filter' + +describe('readToolsFilter', () => { + it('returns empty when no tools object', () => { + expect(readToolsFilter({ command: 'x' })).toEqual({ exclude: undefined, include: undefined }) + }) + + it('reads include/exclude and ignores non-string entries', () => { + expect(readToolsFilter({ tools: { exclude: ['c', 2], include: ['a', 'b', null] } })).toEqual({ + exclude: ['c'], + include: ['a', 'b'] + }) + }) +}) + +describe('isToolEnabled', () => { + it('enables everything with no filter', () => { + expect(isToolEnabled({ command: 'x' }, 'anything')).toBe(true) + }) + + it('include wins over exclude', () => { + const server = { tools: { exclude: ['a'], include: ['a'] } } + expect(isToolEnabled(server, 'a')).toBe(true) + expect(isToolEnabled(server, 'b')).toBe(false) + }) + + it('exclude disables listed tools', () => { + const server = { tools: { exclude: ['b'] } } + expect(isToolEnabled(server, 'a')).toBe(true) + expect(isToolEnabled(server, 'b')).toBe(false) + }) +}) + +describe('toggleToolInServer', () => { + it('adds a fresh tool to a new exclude denylist when disabled', () => { + const next = toggleToolInServer({ command: 'x' }, 'a') + expect(next.tools).toEqual({ exclude: ['a'] }) + }) + + it('re-enabling removes the tool and drops the empty exclude/tools', () => { + const next = toggleToolInServer({ command: 'x', tools: { exclude: ['a'] } }, 'a') + expect(next.tools).toBeUndefined() + }) + + it('respects include mode: toggling removes from include', () => { + const next = toggleToolInServer({ tools: { include: ['a', 'b'] } }, 'a') + expect(next.tools).toEqual({ include: ['b'] }) + }) + + it('respects include mode: re-enabling adds back to include', () => { + const next = toggleToolInServer({ tools: { include: ['b'] } }, 'a') + expect(next.tools).toEqual({ include: ['b', 'a'] }) + }) + + it('preserves sibling tools keys like resources/prompts', () => { + const next = toggleToolInServer({ tools: { resources: false } }, 'a') + expect(next.tools).toEqual({ exclude: ['a'], resources: false }) + }) + + it('does not mutate the input server', () => { + const server = { tools: { exclude: ['a'] } } + toggleToolInServer(server, 'b') + expect(server.tools.exclude).toEqual(['a']) + }) +}) + +describe('countEnabledTools', () => { + it('counts enabled tools out of a discovered list', () => { + const server = { tools: { exclude: ['b'] } } + expect(countEnabledTools(server, ['a', 'b', 'c'])).toBe(2) + }) +}) diff --git a/apps/desktop/src/lib/mcp-tool-filter.ts b/apps/desktop/src/lib/mcp-tool-filter.ts new file mode 100644 index 000000000..8653c32e0 --- /dev/null +++ b/apps/desktop/src/lib/mcp-tool-filter.ts @@ -0,0 +1,61 @@ +// Per-tool MCP gating. A server's optional `tools.include` (whitelist) / +// `tools.exclude` (denylist) decide which discovered tools the agent registers +// — `include` wins, no filter means all. Mirrors `_register_server_tools` in +// `tools/mcp_tool.py`. + +export interface McpToolsFilter { + exclude?: string[] + include?: string[] +} + +type ServerConfig = Record + +const asNames = (value: unknown): string[] | undefined => + Array.isArray(value) ? value.filter((v): v is string => typeof v === 'string') : undefined + +const toolsObject = (server: ServerConfig | null | undefined): Record => { + const tools = server?.tools + + return tools && typeof tools === 'object' && !Array.isArray(tools) ? (tools as Record) : {} +} + +export function readToolsFilter(server: ServerConfig | null | undefined): McpToolsFilter { + const tools = toolsObject(server) + + return { exclude: asNames(tools.exclude), include: asNames(tools.include) } +} + +export function isToolEnabled(server: ServerConfig | null | undefined, name: string): boolean { + const { exclude, include } = readToolsFilter(server) + + return include?.length ? include.includes(name) : !exclude?.includes(name) +} + +// Toggle one tool, preserving the config's mode (include if present, else an +// exclude denylist). Empty lists — and an emptied `tools` — are dropped. +export function toggleToolInServer(server: ServerConfig, name: string): ServerConfig { + const { exclude, include } = readToolsFilter(server) + const key = include?.length ? 'include' : 'exclude' + const current = (key === 'include' ? include : exclude) ?? [] + const names = current.includes(name) ? current.filter(n => n !== name) : [...current, name] + const tools = { ...toolsObject(server) } + + if (names.length) { + tools[key] = names + } else { + delete tools[key] + } + + const next = { ...server } + + if (Object.keys(tools).length) { + next.tools = tools + } else { + delete next.tools + } + + return next +} + +export const countEnabledTools = (server: ServerConfig | null | undefined, names: string[]): number => + names.filter(name => isToolEnabled(server, name)).length diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index e360668c1..946c2d327 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -509,9 +509,17 @@ export interface AnalyticsResponse { summary: AnalyticsSkillsSummary top_skills: AnalyticsSkillEntry[] } + /** Per-tool-name call counts. Absent on older backends. */ + tools?: AnalyticsToolEntry[] totals: AnalyticsTotals } +export interface AnalyticsToolEntry { + count: number + percentage: number + tool: string +} + export interface AnalyticsSkillEntry { last_used_at: null | number manage_count: number @@ -640,6 +648,10 @@ export interface SkillInfo { description: string enabled: boolean name: string + /** Total observed activity (use + view + patch). Absent on older backends. */ + usage?: number + /** 'agent' = learned/local (editable), 'bundled' = ships with Hermes, 'hub' = installed. */ + provenance?: 'agent' | 'bundled' | 'hub' } export interface ToolsetInfo { diff --git a/hermes_cli/logs.py b/hermes_cli/logs.py index 220051f73..b27c5fa4c 100644 --- a/hermes_cli/logs.py +++ b/hermes_cli/logs.py @@ -35,6 +35,9 @@ LOG_FILES = { "gateway": "gateway.log", "gui": "gui.log", "desktop": "desktop.log", + # Every stdio MCP subprocess's stderr (tools/mcp_tool.py redirects it + # here, with per-server session markers) — the "MCP output channel". + "mcp": "mcp-stderr.log", } # Log line timestamp regex — matches "2026-04-05 22:35:00,123" or diff --git a/hermes_cli/mcp_config.py b/hermes_cli/mcp_config.py index 56e98b722..29f6499c7 100644 --- a/hermes_cli/mcp_config.py +++ b/hermes_cli/mcp_config.py @@ -26,7 +26,7 @@ from hermes_cli.config import ( from hermes_cli.colors import Colors, color from hermes_constants import display_hermes_home from hermes_cli.mcp_security import validate_mcp_server_entry -from tools.mcp_tool import _ENV_VAR_PATTERN +from tools.mcp_tool import _ENV_VAR_PATTERN, _env_ref_name logger = logging.getLogger(__name__) @@ -117,6 +117,39 @@ def _remove_mcp_server(name: str) -> bool: return True +def _replace_mcp_servers(servers: Dict[str, dict]) -> Tuple[bool, List[str]]: + """Replace the WHOLE ``mcp_servers`` map in config.yaml. + + Unlike ``_save_mcp_server`` (per-key upsert), this sets the entire map so + the GUI's mcp.json editor can delete servers, drop an ``enabled: false`` + flag (re-enable), or remove nested fields and have those *removals* land on + disk. A plain ``/api/config`` deep-merge can only add/override keys, never + delete them — which is why edits appeared to succeed but the old entry + survived (see MCP tab persistence bug). + + Every entry is validated up front; on any suspicious command/args the whole + save is rejected (returns ``(False, issues)``) so a bad paste can't be + partially applied. An empty map removes the key entirely. + """ + issues: List[str] = [] + for name, cfg in servers.items(): + if not isinstance(cfg, dict): + issues.append(f"Server '{name}': expected an object") + continue + issues.extend(validate_mcp_server_entry(name, cfg)) + + if issues: + return False, issues + + config = load_config() + if servers: + config["mcp_servers"] = dict(servers) + else: + config.pop("mcp_servers", None) + save_config(config) + return True, [] + + def _env_key_for_server(name: str) -> str: """Convert server name to an env-var key like ``MCP_MYSERVER_API_KEY``.""" return f"MCP_{name.upper().replace('-', '_')}_API_KEY" @@ -214,12 +247,16 @@ def _resolve_mcp_server_config(config: dict) -> dict: def _probe_single_server( - name: str, config: dict, connect_timeout: float = 30 + name: str, config: dict, connect_timeout: float = 30, *, details: Optional[dict] = None ) -> List[Tuple[str, str]]: """Temporarily connect to one MCP server, list its tools, disconnect. Returns list of ``(tool_name, description)`` tuples. Raises on connection failure. + + ``details``: optional dict the probe fills with extra capability counts + (``prompts``, ``resources``) — an out-param so the return shape stays + stable for existing CLI callers. """ issues = validate_mcp_server_entry(name, config) if issues: @@ -249,6 +286,19 @@ def _probe_single_server( if len(desc) > 80: desc = desc[:77] + "..." tools_found.append((t.name, desc)) + if details is not None: + # Capability probes are best-effort: servers without the + # capability raise, which just means "0". + try: + result = await server.session.list_prompts() + details["prompts"] = len(result.prompts) + except Exception: + pass + try: + result = await server.session.list_resources() + details["resources"] = len(result.resources) + except Exception: + pass finally: await server.shutdown() @@ -632,8 +682,8 @@ def cmd_mcp_test(args): elif headers: for k, v in headers.items(): if isinstance(v, str) and ("key" in k.lower() or "auth" in k.lower()): - # Mask the value - resolved = _ENV_VAR_PATTERN.sub(lambda m: os.getenv(m.group(1), ""), v) + # Mask the value (accepts ${VAR} and Cursor-style ${env:VAR}) + resolved = _ENV_VAR_PATTERN.sub(lambda m: os.getenv(_env_ref_name(m.group(1)), ""), v) if len(resolved) > 8: masked = resolved[:4] + "***" + resolved[-4:] else: diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index ca4e5b02e..ea4e71ffc 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -8805,6 +8805,12 @@ class MCPServerCreate(BaseModel): profile: Optional[str] = None +class MCPServersReplace(BaseModel): + # Whole-map replace (name → raw server config) for the GUI mcp.json editor. + servers: Dict[str, Dict[str, Any]] = {} + profile: Optional[str] = None + + def _redact_mcp_env(env: Dict[str, Any]) -> Dict[str, str]: """Mask secret-shaped MCP env values for read responses.""" out: Dict[str, str] = {} @@ -8890,6 +8896,25 @@ async def add_mcp_server(body: MCPServerCreate, profile: Optional[str] = None): return _mcp_server_summary(name, server_config) +@app.put("/api/mcp/servers") +async def replace_mcp_servers(body: MCPServersReplace, profile: Optional[str] = None): + """Replace the entire ``mcp_servers`` map (the GUI mcp.json editor's save). + + The generic ``/api/config`` endpoint deep-merges maps, so it can never + delete a server key, drop an ``enabled: false`` flag, or remove a nested + field — edits looked saved but the stale entry survived on disk. This + endpoint sets the whole map so removals actually persist. Storage stays + the config.yaml ``mcp_servers`` key the CLI/TUI already read. + """ + from hermes_cli.mcp_config import _replace_mcp_servers + + with _profile_scope(body.profile or profile): + ok, issues = _replace_mcp_servers(body.servers) + if not ok: + raise HTTPException(status_code=400, detail="; ".join(issues)) + return {"ok": True} + + @app.delete("/api/mcp/servers/{name}") async def remove_mcp_server(name: str, profile: Optional[str] = None): from hermes_cli.mcp_config import _remove_mcp_server @@ -8911,19 +8936,21 @@ async def test_mcp_server(name: str, profile: Optional[str] = None): if name not in servers: raise HTTPException(status_code=404, detail=f"Server '{name}' not found") + details: Dict[str, Any] = {} + def _probe_scoped(): - # Re-enter the scope INSIDE the worker thread so call-time - # resolution during the probe — env-placeholder expansion in - # _resolve_mcp_server_config reading the profile's .env — sees the - # selected profile, matching the config the server was saved into. - # (asyncio.to_thread copies contextvars, but entering explicitly - # keeps the lock-protected SKILLS_DIR swap balanced per-thread.) - # The probe's dedicated MCP event-loop thread is covered too: - # _run_on_mcp_loop wraps scheduled coroutines with the caller's - # HERMES_HOME override (see mcp_tool._wrap_with_home_override), so - # OAuth token stores resolve against the selected profile as well. - with _profile_scope(profile): - return _probe_single_server(name, servers[name]) + # Home-only scope (contextvar), NOT _profile_scope. A probe blocks for + # as long as the server takes to spawn/connect — a stdio `npx` cold + # start is many seconds — and _profile_scope holds a process-global + # skills lock for its ENTIRE body. Holding that across the probe + # serialized every other endpoint (config/skills/toolsets all take the + # same lock), so a slow server made unrelated requests time out at 15s. + # The probe touches no skills globals; it only needs the HERMES_HOME + # override for .env interpolation + OAuth token resolution, which the + # contextvar provides (copied into this to_thread worker; and + # _run_on_mcp_loop re-wraps it onto the MCP event-loop thread). + with _config_profile_scope(profile): + return _probe_single_server(name, servers[name], details=details) try: # Probe blocks on a dedicated MCP event loop — run in a thread so the @@ -8938,9 +8965,103 @@ async def test_mcp_server(name: str, profile: Optional[str] = None): return { "ok": True, "tools": [{"name": t, "description": d} for t, d in tools], + "prompts": details.get("prompts", 0), + "resources": details.get("resources", 0), } +@app.post("/api/mcp/servers/{name}/auth") +async def auth_mcp_server(name: str, profile: Optional[str] = None): + """Run the OAuth flow for an HTTP MCP server (opens the system browser). + + Mirrors ``hermes mcp login``: wipe cached OAuth state so the probe forces + a fresh browser flow, connect, then verify a token actually landed on disk + (some providers serve tools/list unauthenticated — see + ``_reauth_oauth_server``). Blocks until the browser flow completes, so it + runs in a worker thread. ``auth: oauth`` is persisted only on success. + """ + from hermes_cli.mcp_config import ( + _get_mcp_servers, + _oauth_tokens_present, + _probe_single_server, + _save_mcp_server, + ) + + with _profile_scope(profile): + servers = _get_mcp_servers() + if name not in servers: + raise HTTPException(status_code=404, detail=f"Server '{name}' not found") + + cfg = dict(servers[name]) + if not cfg.get("url"): + raise HTTPException( + status_code=400, + detail="stdio servers authenticate via env keys, not OAuth", + ) + # A server carrying `headers` uses API-key/bearer auth; a 401 there is a bad + # key, not an OAuth prompt. Refuse rather than rewrite it to `auth: oauth` + # and corrupt a working header-auth config. (Explicit `auth: oauth` is fine.) + if cfg.get("headers") and cfg.get("auth") != "oauth": + raise HTTPException( + status_code=400, + detail="This server uses header/API-key auth, not OAuth — check its key.", + ) + cfg["auth"] = "oauth" + + def _run(): + from tools.mcp_oauth import force_interactive_oauth + + # Home-only scope, not _profile_scope: this blocks on the browser flow + # for up to minutes; holding the shared skills lock that whole time + # would freeze every other endpoint. Config writes here (_save_mcp_server) + # resolve HERMES_HOME via the contextvar override, which is all they need. + with _config_profile_scope(profile), force_interactive_oauth(): + try: + from tools.mcp_oauth_manager import get_manager + + get_manager().remove(name) + except Exception: + pass # No cached state to clear — fine. + # The default 30s connect timeout would kill the flow while the + # user is still looking at the browser consent screen — give the + # whole browser round-trip room (the callback itself caps at 300s). + tools = _probe_single_server(name, cfg, connect_timeout=240) + if not _oauth_tokens_present(name): + return { + "ok": False, + "error": ( + "The server responded, but no OAuth token was obtained — " + "this provider may require a manually-registered OAuth " + "client (see `hermes mcp login`)." + ), + "tools": [], + } + _save_mcp_server(name, cfg) + return { + "ok": True, + "tools": [{"name": t, "description": d} for t, d in tools], + } + + try: + return await asyncio.to_thread(_run) + except Exception as exc: + msg = str(exc) + # Providers that gate RFC 7591 registration to pre-approved clients + # (Figma's MCP catalog, etc.) 403 the register call before any + # authorization URL exists — surface what's actually happening + # instead of a bare "403 Forbidden". + lowered = msg.lower() + if "403" in msg and ("regist" in lowered or "forbidden" in lowered): + msg = ( + f"'{name}' only allows pre-approved OAuth clients — it rejected " + "client registration (403), so no browser flow can start. " + "Options: add a pre-registered client to this server's entry " + "(oauth: {client_id: ..., client_secret: ...}), or use the " + "provider's stdio / API-key server instead." + ) + return {"ok": False, "error": msg, "tools": []} + + class MCPEnabledToggle(BaseModel): enabled: bool profile: Optional[str] = None @@ -11185,12 +11306,31 @@ class SkillToggle(BaseModel): async def get_skills(profile: Optional[str] = None): from tools.skills_tool import _find_all_skills from hermes_cli.skills_config import get_disabled_skills + from tools.skill_usage import ( + _read_bundled_manifest_names, + _read_hub_installed_names, + activity_count, + load_usage, + ) with _profile_scope(profile): config = load_config() disabled = get_disabled_skills(config) skills = _find_all_skills(skip_disabled=True) + usage = load_usage() + # Set-based provenance (same classification as skill_usage.provenance, + # without a per-skill manifest read): hub > bundled > agent, where + # "agent" covers agent-authored AND local hand-made skills — the ones + # the user may edit/delete from the UI. + bundled_names = _read_bundled_manifest_names() + hub_names = _read_hub_installed_names() for s in skills: s["enabled"] = s["name"] not in disabled + s["usage"] = activity_count(usage.get(s["name"], {})) + s["provenance"] = ( + "hub" if s["name"] in hub_names + else "bundled" if s["name"] in bundled_names + else "agent" + ) return skills @@ -11908,6 +12048,9 @@ async def get_usage_analytics(days: int = 30, profile: Optional[str] = None): "totals": totals, "period_days": days, "skills": skills, + # Per-tool-name call counts (already computed by InsightsEngine); + # the desktop Capabilities page aggregates these per toolset. + "tools": insights_report.get("tools", []), } finally: db.close() diff --git a/tests/hermes_cli/test_mcp_config.py b/tests/hermes_cli/test_mcp_config.py index d052499bf..3986fe712 100644 --- a/tests/hermes_cli/test_mcp_config.py +++ b/tests/hermes_cli/test_mcp_config.py @@ -490,6 +490,27 @@ class TestEnvVarInterpolation: assert _interpolate_env_vars(True) is True assert _interpolate_env_vars(None) is None + def test_interpolate_cursor_env_prefix(self, monkeypatch): + """Cursor-style ${env:VAR} resolves the same secret as ${VAR}.""" + monkeypatch.setenv("MY_KEY", "secret123") + from tools.mcp_tool import _interpolate_env_vars + + assert _interpolate_env_vars("Bearer ${env:MY_KEY}") == "Bearer secret123" + + def test_interpolate_cursor_env_prefix_missing(self, monkeypatch): + """An unset ${env:VAR} keeps its literal placeholder, like ${VAR}.""" + monkeypatch.delenv("MISSING_VAR", raising=False) + from tools.mcp_tool import _interpolate_env_vars + + assert _interpolate_env_vars("Bearer ${env:MISSING_VAR}") == "Bearer ${env:MISSING_VAR}" + + def test_env_ref_name_strips_prefix(self): + from tools.mcp_tool import _env_ref_name + + assert _env_ref_name("env:API_KEY") == "API_KEY" + assert _env_ref_name("API_KEY") == "API_KEY" + assert _env_ref_name(" env:API_KEY ") == "API_KEY" + # --------------------------------------------------------------------------- # Tests: probe-path env resolution (#37792) diff --git a/tools/mcp_oauth.py b/tools/mcp_oauth.py index db70cd4b9..89d0051bc 100644 --- a/tools/mcp_oauth.py +++ b/tools/mcp_oauth.py @@ -104,6 +104,15 @@ _oauth_interactive_enabled: "contextvars.ContextVar[bool]" = contextvars.Context "_oauth_interactive_enabled", default=True ) +# Forces _is_interactive() past the stdin-TTY check for flows driven from a +# GUI (dashboard/desktop REST): the browser + localhost callback server do all +# the work there, and the stdin paste fallback degrades harmlessly (EOF is +# swallowed by _paste_callback_reader). Suppression still wins — background +# discovery must never start a browser flow. +_oauth_interactive_forced: "contextvars.ContextVar[bool]" = contextvars.ContextVar( + "_oauth_interactive_forced", default=False +) + # Skip tokens accepted at the paste prompt — exit OAuth without auth. _SKIP_TOKENS = frozenset({"skip", "cancel", "s", "n", "no", "q", "quit"}) @@ -150,12 +159,30 @@ def _is_interactive() -> bool: """Return True if we can reasonably expect to interact with a user.""" if not _oauth_interactive_enabled.get(): return False + if _oauth_interactive_forced.get(): + return True try: return sys.stdin.isatty() except (AttributeError, ValueError): return False +@contextmanager +def force_interactive_oauth(): + """Treat the current execution context as interactive despite no TTY. + + For GUI-driven auth (dashboard/desktop REST endpoint): the user IS present + — just not on stdin. Opens the browser + localhost callback flow that the + TTY heuristic would otherwise refuse. Same ContextVar propagation story as + suppress_interactive_oauth() (#35927). + """ + token = _oauth_interactive_forced.set(True) + try: + yield + finally: + _oauth_interactive_forced.reset(token) + + @contextmanager def suppress_interactive_oauth(): """Disable stdin-based OAuth prompts for the current execution context. diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 5622d68be..56dd228ca 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -359,6 +359,19 @@ _CREDENTIAL_PATTERN = re.compile( _ENV_VAR_PATTERN = re.compile(r"\$\{([^}]+)\}") +def _env_ref_name(ref: str) -> str: + """Normalize a ``${...}`` reference body into an env-var name. + + Accepts Cursor-style ``${env:VAR}`` in addition to plain ``${VAR}`` by + stripping a leading ``env:`` prefix. The result is the bare variable name + to look up in the secret scope / ``os.environ``. + """ + ref = ref.strip() + if ref.startswith("env:"): + ref = ref[len("env:"):].strip() + return ref + + # --------------------------------------------------------------------------- # Security helpers # --------------------------------------------------------------------------- @@ -3207,17 +3220,20 @@ def _interrupted_call_result() -> str: def _interpolate_env_vars(value): """Recursively resolve ``${VAR}`` placeholders. - Resolves from the active profile's secret scope when multiplexing is on - (so an MCP server config's ``${API_KEY}`` picks up the routed profile's - value, not the process-global ``os.environ`` which may hold another - profile's), falling back to ``os.environ`` otherwise. Unset vars keep the - literal ``${VAR}`` placeholder, as before. + Both ``${VAR}`` and Cursor-style ``${env:VAR}`` are accepted — the + ``env:`` prefix is stripped so a doc copied from a Cursor / Claude MCP + config resolves the same secret. Resolves from the active profile's secret + scope when multiplexing is on (so an MCP server config's ``${API_KEY}`` + picks up the routed profile's value, not the process-global ``os.environ`` + which may hold another profile's), falling back to ``os.environ`` + otherwise. Unset vars keep the literal placeholder, as before. """ from agent.secret_scope import get_secret as _get_secret if isinstance(value, str): def _replace(m): - return _get_secret(m.group(1), m.group(0)) or m.group(0) + name = _env_ref_name(m.group(1)) + return _get_secret(name, m.group(0)) or m.group(0) return _ENV_VAR_PATTERN.sub(_replace, value) if isinstance(value, dict): return {k: _interpolate_env_vars(v) for k, v in value.items()}