feat(mcp): multi-server lifecycle — enable/disable + error visibility (#3196) (#3339)

This commit is contained in:
CodeGhost21
2026-06-04 18:11:39 -04:00
committed by GitHub
parent 7edfb043d3
commit d710252eef
41 changed files with 1386 additions and 41 deletions
@@ -0,0 +1,108 @@
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import InstalledServerDetail from './InstalledServerDetail';
const mockConnect = vi.fn();
const mockDisconnect = vi.fn();
const mockUninstall = vi.fn();
const mockUpdateEnv = vi.fn();
const mockSetEnabled = vi.fn();
vi.mock('../../../services/api/mcpClientsApi', () => ({
mcpClientsApi: {
connect: (...args: unknown[]) => mockConnect(...args),
disconnect: (...args: unknown[]) => mockDisconnect(...args),
uninstall: (...args: unknown[]) => mockUninstall(...args),
updateEnv: (...args: unknown[]) => mockUpdateEnv(...args),
setEnabled: (...args: unknown[]) => mockSetEnabled(...args),
configAssist: vi.fn(),
},
}));
const BASE_SERVER_ENABLED = {
server_id: 'srv-1',
qualified_name: 'acme/test-server',
display_name: 'Test Server',
description: 'A test MCP server',
command_kind: 'node' as const,
command: 'node',
args: [],
env_keys: [] as string[],
installed_at: 1_700_000_000,
enabled: true,
};
const BASE_SERVER_DISABLED = { ...BASE_SERVER_ENABLED, enabled: false };
describe('InstalledServerDetail — enable/disable toggle', () => {
beforeEach(() => {
mockConnect.mockReset();
mockDisconnect.mockReset();
mockUninstall.mockReset();
mockUpdateEnv.mockReset();
mockSetEnabled.mockReset();
});
it('shows Disable button when server is enabled', () => {
render(
<InstalledServerDetail
server={BASE_SERVER_ENABLED}
connStatus={undefined}
onUninstalled={() => {}}
/>
);
expect(screen.getByRole('button', { name: /disable/i })).toBeInTheDocument();
});
it('shows Enable button when server is disabled and hides Connect button', () => {
render(
<InstalledServerDetail
server={BASE_SERVER_DISABLED}
connStatus={undefined}
onUninstalled={() => {}}
/>
);
expect(screen.getByRole('button', { name: /^enable$/i })).toBeInTheDocument();
expect(screen.queryByRole('button', { name: /^connect$/i })).not.toBeInTheDocument();
});
it('calls setEnabled(false) on Disable click and notifies parent via onEnabledChange', async () => {
mockSetEnabled.mockResolvedValue({ server_id: 'srv-1', enabled: false });
const onEnabledChange = vi.fn();
render(
<InstalledServerDetail
server={BASE_SERVER_ENABLED}
connStatus={undefined}
onUninstalled={() => {}}
onEnabledChange={onEnabledChange}
/>
);
await act(async () => {
fireEvent.click(screen.getByRole('button', { name: /disable/i }));
});
await waitFor(() => {
expect(mockSetEnabled).toHaveBeenCalledWith('srv-1', false);
expect(onEnabledChange).toHaveBeenCalledWith('srv-1', false);
});
});
it('surfaces API error inline if setEnabled rejects', async () => {
mockSetEnabled.mockRejectedValue(new Error('Server unavailable'));
render(
<InstalledServerDetail
server={BASE_SERVER_ENABLED}
connStatus={undefined}
onUninstalled={() => {}}
/>
);
await act(async () => {
fireEvent.click(screen.getByRole('button', { name: /disable/i }));
});
await waitFor(() => expect(screen.getByText('Server unavailable')).toBeInTheDocument());
});
});
@@ -28,6 +28,7 @@ const BASE_SERVER = {
args: [],
env_keys: ['API_KEY', 'DB_URL'],
installed_at: 1_700_000_000,
enabled: true,
};
describe('InstalledServerDetail', () => {
@@ -71,6 +72,25 @@ describe('InstalledServerDetail', () => {
expect(screen.getByRole('button', { name: 'Connect' })).toBeInTheDocument();
});
it('shows Connecting… label and disables the Connect button while status=connecting', () => {
render(
<InstalledServerDetail
server={BASE_SERVER}
connStatus={{
server_id: 'srv-1',
qualified_name: 'acme/test-server',
display_name: 'Test Server',
status: 'connecting',
tool_count: 0,
}}
onUninstalled={() => {}}
/>
);
const btn = screen.getByRole('button', { name: /^connecting/i });
expect(btn).toBeInTheDocument();
expect(btn).toBeDisabled();
});
it('shows Disconnect button when connected', () => {
render(
<InstalledServerDetail
@@ -19,12 +19,14 @@ interface InstalledServerDetailProps {
server: InstalledServer;
connStatus: ConnStatus | undefined;
onUninstalled: (serverId: string) => void;
onEnabledChange?: (serverId: string, enabled: boolean) => void;
}
const InstalledServerDetail = ({
server,
connStatus,
onUninstalled,
onEnabledChange,
}: InstalledServerDetailProps) => {
const { t } = useT();
const status: ServerStatus = connStatus?.status ?? 'disconnected';
@@ -115,6 +117,25 @@ const InstalledServerDetail = ({
});
}, [server.server_id, runBusy, onUninstalled]);
const handleSetEnabled = useCallback(
(next: boolean) => {
void runBusy(async () => {
log('set_enabled server_id=%s enabled=%s', server.server_id, next);
await mcpClientsApi.setEnabled(server.server_id, next);
if (!next) {
// Disabling the server: drop stale tool list so no tool rows
// remain in view while the server is disabled, and clear any
// open playground session.
setTools([]);
setPlaygroundTool(null);
}
log('set_enabled done server_id=%s enabled=%s', server.server_id, next);
onEnabledChange?.(server.server_id, next);
});
},
[server.server_id, runBusy, onEnabledChange]
);
const openReconfigure = useCallback(
(prefill?: Record<string, string>) => {
const initial: Record<string, string> = {};
@@ -217,23 +238,35 @@ const InstalledServerDetail = ({
{/* Action buttons */}
<div className="flex flex-wrap gap-2">
{status !== 'connected' ? (
<button
type="button"
disabled={busy || status === 'connecting'}
onClick={handleConnect}
className="rounded-lg bg-primary-500 px-3 py-1.5 text-xs font-medium text-white hover:bg-primary-600 disabled:opacity-50 transition-colors">
{status === 'connecting' ? t('mcp.detail.connecting') : t('mcp.detail.connect')}
</button>
) : (
<button
type="button"
disabled={busy}
onClick={handleDisconnect}
className="rounded-lg border border-stone-200 dark:border-neutral-700 px-3 py-1.5 text-xs font-medium text-stone-600 dark:text-neutral-300 hover:border-stone-300 dark:hover:border-neutral-600 disabled:opacity-50">
{t('mcp.detail.disconnect')}
</button>
)}
{/* Connect / Disconnect — hidden when the server is disabled because the
core refuses connect calls on disabled servers. */}
{server.enabled &&
(status !== 'connected' ? (
<button
type="button"
disabled={busy || status === 'connecting'}
onClick={handleConnect}
className="rounded-lg bg-primary-500 px-3 py-1.5 text-xs font-medium text-white hover:bg-primary-600 disabled:opacity-50 transition-colors">
{status === 'connecting' ? t('mcp.detail.connecting') : t('mcp.detail.connect')}
</button>
) : (
<button
type="button"
disabled={busy}
onClick={handleDisconnect}
className="rounded-lg border border-stone-200 dark:border-neutral-700 px-3 py-1.5 text-xs font-medium text-stone-600 dark:text-neutral-300 hover:border-stone-300 dark:hover:border-neutral-600 disabled:opacity-50">
{t('mcp.detail.disconnect')}
</button>
))}
{/* Enable / Disable toggle */}
<button
type="button"
disabled={busy}
onClick={() => handleSetEnabled(!server.enabled)}
className="rounded-lg border border-stone-200 dark:border-neutral-700 px-3 py-1.5 text-xs font-medium text-stone-600 dark:text-neutral-300 hover:border-stone-300 dark:hover:border-neutral-600 disabled:opacity-50">
{server.enabled ? t('mcp.detail.disable') : t('mcp.detail.enable')}
</button>
<button
type="button"
@@ -18,6 +18,7 @@ const SERVER_1: InstalledServer = {
args: ['-y', 'acme/fs-server'],
env_keys: [],
installed_at: 1_700_000_000,
enabled: true,
};
const SERVER_2: InstalledServer = {
@@ -30,6 +31,7 @@ const SERVER_2: InstalledServer = {
args: ['-y', 'acme/db-server'],
env_keys: ['DB_URL'],
installed_at: 1_700_000_001,
enabled: true,
};
const STATUS_CONNECTED: ConnStatus = {
@@ -29,6 +29,7 @@ const STATUS_DOT: Record<ServerStatus, string> = {
connecting: 'bg-amber-400',
disconnected: 'bg-stone-300 dark:bg-neutral-600',
error: 'bg-coral-500',
disabled: 'bg-stone-200 dark:bg-neutral-700',
};
// i18n keys for the per-status tooltip on the status dot. Reuses the
@@ -40,6 +41,7 @@ const STATUS_I18N_KEYS: Record<ServerStatus, string> = {
connecting: 'channels.status.connecting',
disconnected: 'channels.status.disconnected',
error: 'channels.status.error',
disabled: 'mcp.status.disabled',
};
const InstalledServerList = ({
@@ -34,6 +34,7 @@ const SERVER_FS: InstalledServer = {
config: { region: 'us-east-1' },
installed_at: 1_700_000_000,
last_connected_at: 1_700_001_000,
enabled: true,
};
const SERVER_DB: InstalledServer = {
@@ -46,6 +47,7 @@ const SERVER_DB: InstalledServer = {
args: ['-y', 'acme/db-server'],
env_keys: ['DB_URL'],
installed_at: 1_700_000_500,
enabled: true,
};
describe('McpInventoryManifest: buildManifest', () => {
@@ -22,6 +22,7 @@ const SERVER_FS: InstalledServer = {
args: ['-y', 'acme/fs-server'],
env_keys: ['ROOT_DIR'],
installed_at: 1_700_000_000,
enabled: true,
};
const SERVER_DB: InstalledServer = {
@@ -33,6 +34,7 @@ const SERVER_DB: InstalledServer = {
args: ['-y', 'acme/db-server'],
env_keys: ['DB_URL'],
installed_at: 1_700_000_500,
enabled: true,
};
const renderPanel = (overrides?: {
@@ -14,6 +14,7 @@ const mockInstall = vi.fn();
const mockConnect = vi.fn();
const mockDisconnect = vi.fn();
const mockUninstall = vi.fn();
const mockSetEnabled = vi.fn();
const mockRegistryGet = vi.fn();
const mockRegistrySearch = vi.fn();
const mockConfigAssist = vi.fn();
@@ -26,6 +27,7 @@ vi.mock('../../../services/api/mcpClientsApi', () => ({
connect: (...args: unknown[]) => mockConnect(...args),
disconnect: (...args: unknown[]) => mockDisconnect(...args),
uninstall: (...args: unknown[]) => mockUninstall(...args),
setEnabled: (...args: unknown[]) => mockSetEnabled(...args),
registryGet: (...args: unknown[]) => mockRegistryGet(...args),
registrySearch: (...args: unknown[]) => mockRegistrySearch(...args),
configAssist: (...args: unknown[]) => mockConfigAssist(...args),
@@ -43,6 +45,7 @@ const SERVERS = [
args: ['-y', 'acme/fs-server'],
env_keys: [],
installed_at: 1_700_000_000,
enabled: true,
},
];
@@ -86,6 +89,7 @@ describe('McpServersTab', () => {
mockConnect.mockResolvedValue({ server_id: '', status: 'connected', tools: [] });
mockDisconnect.mockReset();
mockUninstall.mockReset();
mockSetEnabled.mockReset();
mockRegistryGet.mockReset();
mockRegistrySearch.mockReset();
});
@@ -149,6 +153,33 @@ describe('McpServersTab', () => {
expect(screen.getByText('Select a server or browse the catalog.')).toBeInTheDocument();
});
it('refreshes installed list + status after a server is disabled from the detail pane', async () => {
mockInstalledList.mockResolvedValue(SERVERS);
mockStatus.mockResolvedValue(STATUSES_DISCONNECTED);
mockSetEnabled.mockResolvedValue({ server_id: 'srv-1', enabled: false });
render(<McpServersTab />);
vi.useRealTimers();
await waitFor(() => screen.getByText('File Server'));
fireEvent.click(screen.getAllByRole('button', { name: /File Server/i })[0]);
await waitFor(() => screen.getByText('acme/fs-server'));
const installedCallsBefore = mockInstalledList.mock.calls.length;
const statusCallsBefore = mockStatus.mock.calls.length;
const disableBtn = await screen.findByRole('button', { name: /^disable$/i });
fireEvent.click(disableBtn);
// After the toggle, handleEnabledChange must re-call both loadInstalled
// and fetchStatuses so the parent reflects the new enabled state.
await waitFor(() => {
expect(mockSetEnabled).toHaveBeenCalledWith('srv-1', false);
expect(mockInstalledList.mock.calls.length).toBeGreaterThan(installedCallsBefore);
expect(mockStatus.mock.calls.length).toBeGreaterThan(statusCallsBefore);
});
});
it('opens detail pane when a server is clicked', async () => {
mockInstalledList.mockResolvedValue(SERVERS);
mockStatus.mockResolvedValue(STATUSES_DISCONNECTED);
@@ -140,6 +140,15 @@ const McpServersTab = () => {
[loadInstalled, fetchStatuses]
);
const handleEnabledChange = useCallback(
async (_serverId: string, _enabled: boolean) => {
log('enabled_change server_id=%s enabled=%s', _serverId, _enabled);
await loadInstalled();
await fetchStatuses();
},
[loadInstalled, fetchStatuses]
);
// Count rejected settlements and, if any, throw a descriptive error so the
// toolbar surfaces it through its `role="alert"` region — otherwise a bulk
// action that partially (or wholly) fails looks identical to success and
@@ -272,6 +281,7 @@ const McpServersTab = () => {
server={selectedServer}
connStatus={selectedConnStatus}
onUninstalled={serverId => void handleUninstalled(serverId)}
onEnabledChange={(serverId, enabled) => void handleEnabledChange(serverId, enabled)}
/>
)}
</div>
@@ -19,6 +19,13 @@ describe('McpStatusBadge', () => {
expect(screen.getByRole('status')).toHaveTextContent(expectedLabel);
});
it('renders the disabled status badge with label and italic style', () => {
render(<McpStatusBadge status="disabled" />);
const badge = screen.getByRole('status');
expect(badge).toHaveTextContent('Disabled');
expect(badge.className).toContain('italic');
});
it('exposes role="status" and aria-live="polite" for assistive tech', () => {
render(<McpStatusBadge status="connecting" />);
const badge = screen.getByRole('status');
@@ -25,6 +25,11 @@ const STATUS_META: Record<ServerStatus, { i18nKey: string; className: string }>
i18nKey: 'channels.status.error',
className: 'bg-coral-500/10 text-coral-700 border-coral-500/30 dark:text-coral-300',
},
disabled: {
i18nKey: 'mcp.status.disabled',
className:
'bg-stone-100 dark:bg-neutral-800 text-stone-400 dark:text-neutral-500 border-stone-200 dark:border-neutral-700 italic',
},
};
interface McpStatusBadgeProps {
+2 -1
View File
@@ -40,11 +40,12 @@ export type InstalledServer = {
config?: unknown;
installed_at: number;
last_connected_at?: number;
enabled: boolean;
};
export type McpTool = { name: string; description?: string; input_schema: unknown };
export type ServerStatus = 'disconnected' | 'connecting' | 'connected' | 'error';
export type ServerStatus = 'disconnected' | 'connecting' | 'connected' | 'error' | 'disabled';
export type ConnStatus = {
server_id: string;
+3
View File
@@ -1141,6 +1141,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSaving': 'جارٍ الحفظ…',
'mcp.detail.reconfigureSuccess': 'تم تحديث البيئة وإعادة الاتصال.',
'mcp.detail.reconfigureReconnectFailed': 'تم الحفظ، لكن فشلت إعادة الاتصال بالقيم الجديدة.',
'mcp.detail.enable': 'تفعيل',
'mcp.detail.disable': 'تعطيل',
'mcp.status.disabled': 'معطّل',
'mcp.detail.tools': 'الأدوات',
'onboarding.skipForNow': 'التخطي الآن',
'onboarding.localAI.continueWithCloud': 'متابعة مع السحابة',
+3
View File
@@ -1158,6 +1158,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSuccess': 'এনভায়রনমেন্ট আপডেট ও পুনঃসংযোগ করা হয়েছে।',
'mcp.detail.reconfigureReconnectFailed':
'সংরক্ষিত হয়েছে, তবে নতুন মান দিয়ে পুনঃসংযোগ ব্যর্থ হয়েছে।',
'mcp.detail.enable': 'সক্রিয় করুন',
'mcp.detail.disable': 'নিষ্ক্রিয় করুন',
'mcp.status.disabled': 'নিষ্ক্রিয়',
'mcp.detail.tools': 'টুলস',
'onboarding.skipForNow': 'এখনই এড়িয়ে যান',
'onboarding.localAI.continueWithCloud': 'ক্লাউডের সাথে চালিয়ে যান',
+3
View File
@@ -1194,6 +1194,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSuccess': 'Umgebung aktualisiert und neu verbunden.',
'mcp.detail.reconfigureReconnectFailed':
'Gespeichert, aber das Neuverbinden mit den neuen Werten ist fehlgeschlagen.',
'mcp.detail.enable': 'Aktivieren',
'mcp.detail.disable': 'Deaktivieren',
'mcp.status.disabled': 'Deaktiviert',
'mcp.detail.tools': 'Werkzeuge',
'onboarding.skipForNow': 'Vorerst überspringen',
'onboarding.localAI.continueWithCloud': 'Fahren Sie mit der Cloud fort.',
+3
View File
@@ -1385,6 +1385,9 @@ const en: TranslationMap = {
'mcp.detail.reconfigureSaving': 'Saving…',
'mcp.detail.reconfigureSuccess': 'Environment updated and reconnected.',
'mcp.detail.reconfigureReconnectFailed': 'Saved, but reconnecting with the new values failed.',
'mcp.detail.enable': 'Enable',
'mcp.detail.disable': 'Disable',
'mcp.status.disabled': 'Disabled',
'mcp.detail.tools': 'Tools',
'onboarding.skipForNow': 'Skip for Now',
'onboarding.localAI.continueWithCloud': 'Continue with Cloud',
+3
View File
@@ -1191,6 +1191,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSuccess': 'Entorno actualizado y reconectado.',
'mcp.detail.reconfigureReconnectFailed':
'Guardado, pero no se pudo reconectar con los nuevos valores.',
'mcp.detail.enable': 'Habilitar',
'mcp.detail.disable': 'Deshabilitar',
'mcp.status.disabled': 'Deshabilitado',
'mcp.detail.tools': 'Herramientas',
'onboarding.skipForNow': 'Saltar por ahora',
'onboarding.localAI.continueWithCloud': 'Continuar con la nube',
+3
View File
@@ -1192,6 +1192,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSuccess': 'Environnement mis à jour et reconnecté.',
'mcp.detail.reconfigureReconnectFailed':
'Enregistré, mais la reconnexion avec les nouvelles valeurs a échoué.',
'mcp.detail.enable': 'Activer',
'mcp.detail.disable': 'Désactiver',
'mcp.status.disabled': 'Désactivé',
'mcp.detail.tools': 'Outils',
'onboarding.skipForNow': "Passer pour l'instant",
'onboarding.localAI.continueWithCloud': 'Continuer avec Cloud',
+3
View File
@@ -1159,6 +1159,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSuccess': 'एनवायरनमेंट अपडेट हो गया और पुन: कनेक्ट हो गया।',
'mcp.detail.reconfigureReconnectFailed':
'सहेजा गया, लेकिन नए मानों के साथ पुन: कनेक्ट करना विफल रहा।',
'mcp.detail.enable': 'सक्षम करें',
'mcp.detail.disable': 'अक्षम करें',
'mcp.status.disabled': 'अक्षम',
'mcp.detail.tools': 'उपकरण',
'onboarding.skipForNow': 'अभी के लिए छोड़ें',
'onboarding.localAI.continueWithCloud': 'बादल के साथ जारी रखें',
+3
View File
@@ -1166,6 +1166,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSuccess': 'Lingkungan diperbarui dan terhubung kembali.',
'mcp.detail.reconfigureReconnectFailed':
'Tersimpan, tetapi gagal menghubungkan kembali dengan nilai baru.',
'mcp.detail.enable': 'Aktifkan',
'mcp.detail.disable': 'Nonaktifkan',
'mcp.status.disabled': 'Dinonaktifkan',
'mcp.detail.tools': 'Alat',
'onboarding.skipForNow': 'Lewati Sekarang',
'onboarding.localAI.continueWithCloud': 'Lanjutkan dengan Cloud',
+3
View File
@@ -1186,6 +1186,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSuccess': 'Ambiente aggiornato e riconnesso.',
'mcp.detail.reconfigureReconnectFailed':
'Salvato, ma la riconnessione con i nuovi valori non è riuscita.',
'mcp.detail.enable': 'Abilita',
'mcp.detail.disable': 'Disabilita',
'mcp.status.disabled': 'Disabilitato',
'mcp.detail.tools': 'Strumenti',
'onboarding.skipForNow': 'Salta per ora',
'onboarding.localAI.continueWithCloud': 'Continua con Cloud',
+3
View File
@@ -1155,6 +1155,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSaving': '저장 중…',
'mcp.detail.reconfigureSuccess': '환경이 업데이트되고 다시 연결되었습니다.',
'mcp.detail.reconfigureReconnectFailed': '저장했지만 새 값으로 다시 연결하지 못했습니다.',
'mcp.detail.enable': '활성화',
'mcp.detail.disable': '비활성화',
'mcp.status.disabled': '비활성화됨',
'mcp.detail.tools': '도구',
'onboarding.skipForNow': '지금 건너뛰기',
'onboarding.localAI.continueWithCloud': '클라우드 계속하기',
+3
View File
@@ -1179,6 +1179,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSuccess': 'Środowisko zaktualizowane i połączone ponownie.',
'mcp.detail.reconfigureReconnectFailed':
'Zapisano, ale ponowne połączenie z nowymi wartościami nie powiodło się.',
'mcp.detail.enable': 'Włącz',
'mcp.detail.disable': 'Wyłącz',
'mcp.status.disabled': 'Wyłączony',
'mcp.detail.tools': 'Narzędzia',
'onboarding.skipForNow': 'Pomiń na razie',
'onboarding.localAI.continueWithCloud': 'Kontynuuj z chmurą',
+3
View File
@@ -1190,6 +1190,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSaving': 'Salvando…',
'mcp.detail.reconfigureSuccess': 'Ambiente atualizado e reconectado.',
'mcp.detail.reconfigureReconnectFailed': 'Salvo, mas a reconexão com os novos valores falhou.',
'mcp.detail.enable': 'Ativar',
'mcp.detail.disable': 'Desativar',
'mcp.status.disabled': 'Desativado',
'mcp.detail.tools': 'Ferramentas',
'onboarding.skipForNow': 'Ignorar por agora',
'onboarding.localAI.continueWithCloud': 'Continuar com a nuvem',
+3
View File
@@ -1174,6 +1174,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSuccess': 'Окружение обновлено, выполнено переподключение.',
'mcp.detail.reconfigureReconnectFailed':
'Сохранено, но переподключиться с новыми значениями не удалось.',
'mcp.detail.enable': 'Включить',
'mcp.detail.disable': 'Отключить',
'mcp.status.disabled': 'Отключён',
'mcp.detail.tools': 'Инструменты',
'onboarding.skipForNow': 'Пропустить сейчас',
'onboarding.localAI.continueWithCloud': 'Продолжить с Облако',
+3
View File
@@ -1103,6 +1103,9 @@ const messages: TranslationMap = {
'mcp.detail.reconfigureSaving': '正在保存…',
'mcp.detail.reconfigureSuccess': '环境已更新并重新连接。',
'mcp.detail.reconfigureReconnectFailed': '已保存,但使用新值重新连接失败。',
'mcp.detail.enable': '启用',
'mcp.detail.disable': '禁用',
'mcp.status.disabled': '已禁用',
'mcp.detail.tools': '工具',
'onboarding.skipForNow': '暂时跳过',
'onboarding.localAI.continueWithCloud': '继续使用云',
@@ -359,4 +359,34 @@ describe('mcpClientsApi', () => {
expect(result.suggested_env).toEqual({ API_KEY: 'token-value' });
});
});
describe('setEnabled', () => {
it('calls mcp_clients_set_enabled with server_id and enabled=true and returns the result', async () => {
mockCallCoreRpc.mockResolvedValueOnce({ server_id: 'srv-1', enabled: true });
const { mcpClientsApi } = await import('./mcpClientsApi');
const result = await mcpClientsApi.setEnabled('srv-1', true);
expect(mockCallCoreRpc).toHaveBeenCalledWith({
method: 'openhuman.mcp_clients_set_enabled',
params: { server_id: 'srv-1', enabled: true },
});
expect(result.server_id).toBe('srv-1');
expect(result.enabled).toBe(true);
});
it('calls mcp_clients_set_enabled with enabled=false and returns the disabled result', async () => {
mockCallCoreRpc.mockResolvedValueOnce({ server_id: 'srv-2', enabled: false });
const { mcpClientsApi } = await import('./mcpClientsApi');
const result = await mcpClientsApi.setEnabled('srv-2', false);
expect(mockCallCoreRpc).toHaveBeenCalledWith({
method: 'openhuman.mcp_clients_set_enabled',
params: { server_id: 'srv-2', enabled: false },
});
expect(result.server_id).toBe('srv-2');
expect(result.enabled).toBe(false);
});
});
});
+16
View File
@@ -56,6 +56,11 @@ interface DisconnectResult {
status: 'disconnected';
}
interface SetEnabledResult {
server_id: string;
enabled: boolean;
}
interface StatusResult {
servers: ConnStatus[];
}
@@ -234,6 +239,17 @@ export const mcpClientsApi = {
return result;
},
/** Enable or disable a server. Returns the new enabled state. */
setEnabled: async (server_id: string, enabled: boolean): Promise<SetEnabledResult> => {
log('set_enabled server_id=%s enabled=%s', server_id, enabled);
const result = await callCoreRpc<SetEnabledResult>({
method: 'openhuman.mcp_clients_set_enabled',
params: { server_id, enabled },
});
log('set_enabled server_id=%s enabled=%s', result.server_id, result.enabled);
return result;
},
/** Get status for all managed MCP servers. */
status: async (): Promise<ConnStatus[]> => {
log('status');
+8
View File
@@ -36,6 +36,14 @@ pub async fn spawn_installed_servers(config: &Config) {
);
for server in servers {
if !server.enabled {
tracing::info!(
"[mcp-registry] boot: skipping disabled server_id={} qualified={}",
server.server_id,
server.qualified_name
);
continue;
}
let server_id = server.server_id.clone();
let qualified = server.qualified_name.clone();
match connections::connect(config, &server).await {
+72 -11
View File
@@ -85,11 +85,35 @@ fn connections() -> &'static RwLock<HashMap<String, Arc<Connection>>> {
CONNECTIONS.get_or_init(|| RwLock::new(HashMap::new()))
}
// ── Per-server last connect error ────────────────────────────────────────────
static LAST_ERRORS: OnceLock<RwLock<HashMap<String, String>>> = OnceLock::new();
fn last_errors() -> &'static RwLock<HashMap<String, String>> {
LAST_ERRORS.get_or_init(|| RwLock::new(HashMap::new()))
}
/// Read the most recent connect-failure message for `server_id`. `None` when
/// the server has never failed, or when the most recent connect succeeded.
pub async fn last_error_for(server_id: &str) -> Option<String> {
last_errors().read().await.get(server_id).cloned()
}
/// Drop any recorded error for `server_id`. Called on successful connect,
/// explicit disconnect, uninstall, and enable→disable transitions.
pub async fn clear_last_error(server_id: &str) {
last_errors().write().await.remove(server_id);
}
// ── Public API ────────────────────────────────────────────────────────────────
/// Bring up a new MCP client for `server`, run `initialize`, cache the
/// tool list, and store the connection in the global registry.
///
/// On success the prior error (if any) for this `server_id` is cleared.
/// On failure the error message is recorded in [`LAST_ERRORS`] so callers
/// can surface it in status polling without re-attempting the connect.
///
/// Dispatches on `server.transport`:
/// - [`Transport::Stdio`] — spawn `command` + `args` as a subprocess and
/// speak JSON-RPC over stdin/stdout (the original behaviour).
@@ -97,6 +121,30 @@ fn connections() -> &'static RwLock<HashMap<String, Arc<Connection>>> {
/// directly with [`McpHttpClient`]. No subprocess. Needed for the
/// `~99%` of Smithery listings that are HTTP-remote.
pub async fn connect(config: &Config, server: &InstalledServer) -> anyhow::Result<Vec<McpTool>> {
let result = connect_inner(config, server).await;
match &result {
Ok(_) => {
last_errors().write().await.remove(&server.server_id);
tracing::debug!(
"[mcp-registry] last_error cleared server_id={}",
server.server_id
);
}
Err(err) => {
last_errors()
.write()
.await
.insert(server.server_id.clone(), err.to_string());
tracing::debug!(
"[mcp-registry] last_error recorded server_id={} err={err}",
server.server_id
);
}
}
result
}
async fn connect_inner(config: &Config, server: &InstalledServer) -> anyhow::Result<Vec<McpTool>> {
tracing::debug!(
"[mcp-registry] connect server_id={} qualified_name={} transport={}",
server.server_id,
@@ -177,13 +225,15 @@ pub async fn connect(config: &Config, server: &InstalledServer) -> anyhow::Resul
Ok(tools)
}
/// Disconnect and remove from the registry.
/// Disconnect and remove from the registry. Also clears any recorded
/// connect error so the next status poll starts from a clean slate.
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)
};
last_errors().write().await.remove(server_id);
if let Some(c) = conn {
let _ = c.client.close_session().await;
tracing::debug!("[mcp-registry] disconnected server_id={server_id}");
@@ -216,6 +266,12 @@ pub async fn call_tool(
}
/// Return status summaries for all installed servers.
///
/// Priority order: `Disabled` > `Connected` > `Error` > `Disconnected`.
/// - `!s.enabled` → `Disabled` (suppresses tool count and last_error).
/// - connected (id in live registry) → `Connected` + tool count.
/// - recorded connect failure in `LAST_ERRORS` → `Error` + last_error message.
/// - otherwise → `Disconnected`.
pub async fn all_status(config: &Config) -> Vec<ConnStatus> {
let installed = store::list_servers(config).unwrap_or_default();
let connected_ids: Vec<String> = {
@@ -223,29 +279,34 @@ pub async fn all_status(config: &Config) -> Vec<ConnStatus> {
map.keys().cloned().collect()
};
let errors_snapshot = last_errors().read().await.clone();
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 (status, tool_count, last_error) = if !s.enabled {
(ServerStatus::Disabled, 0u32, None)
} else if is_connected {
let map = connections().read().await;
match map.get(&s.server_id) {
let tool_count = match map.get(&s.server_id) {
Some(c) => c.tools_snapshot().await.len() as u32,
None => 0,
}
};
(ServerStatus::Connected, tool_count, None)
} else if let Some(err) = errors_snapshot.get(&s.server_id).cloned() {
(ServerStatus::Error, 0u32, Some(err))
} else {
0
(ServerStatus::Disconnected, 0u32, None)
};
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
},
status,
tool_count,
last_error: None,
last_error,
});
}
out
+1 -1
View File
@@ -53,7 +53,7 @@
pub mod boot;
pub mod bus;
pub mod connections;
mod ops;
pub mod ops;
mod registries;
mod registry;
mod schemas;
+74
View File
@@ -166,6 +166,7 @@ pub async fn mcp_clients_install(
installed_at: now_ms,
last_connected_at: None,
transport,
enabled: true,
};
store::insert_server(config, &server).map_err(|e| e.to_string())?;
@@ -268,6 +269,13 @@ pub async fn mcp_clients_connect(
let server = store::get_server(config, server_id.trim()).map_err(|e| e.to_string())?;
if !server.enabled {
return Err(format!(
"server_id={} is disabled; enable it via mcp_clients_set_enabled before connecting",
server_id.trim()
));
}
let tools = connections::connect(config, &server)
.await
.map_err(|e| e.to_string())?;
@@ -293,6 +301,54 @@ pub async fn mcp_clients_connect(
))
}
// ── set_enabled ────────────────────────────────────────────────────────────────
/// Flip the `enabled` flag on an installed server.
///
/// - `enabled=false`: persist the flip, then disconnect any live session so
/// the server's tools immediately disappear from the agent's surface. The
/// install row and env values are kept intact so re-enabling later does
/// not require re-entering credentials.
/// - `enabled=true`: persist the flip. The server is NOT auto-connected here
/// — the user calls `connect` explicitly. This keeps "enabled" purely a
/// persistent setting and "connected" purely a live-session state.
pub async fn mcp_clients_set_enabled(
config: &Config,
server_id: String,
enabled: bool,
) -> Result<RpcOutcome<Value>, String> {
if server_id.trim().is_empty() {
return Err("server_id must not be empty".to_string());
}
let server_id = server_id.trim().to_string();
tracing::debug!(
"[mcp-client] set_enabled server_id={} enabled={}",
server_id,
enabled
);
// Existence check produces a clear error before we mutate.
let _existing = store::get_server(config, &server_id).map_err(|e| e.to_string())?;
store::update_enabled(config, &server_id, enabled).map_err(|e| e.to_string())?;
if !enabled {
connections::disconnect(&server_id).await;
connections::clear_last_error(&server_id).await;
let _ = publish_global(DomainEvent::McpServerDisconnected {
server_id: server_id.clone(),
reason: Some("disabled".to_string()),
});
}
Ok(RpcOutcome::new(
json!({ "server_id": server_id, "enabled": enabled }),
vec![format!(
"set_enabled server_id={server_id} enabled={enabled}"
)],
))
}
// ── disconnect ────────────────────────────────────────────────────────────────
pub async fn mcp_clients_disconnect(server_id: String) -> Result<RpcOutcome<Value>, String> {
@@ -368,6 +424,24 @@ pub async fn mcp_clients_update_env(
.map_err(|e| e.to_string())?;
}
// A disabled server must not be auto-reconnected even when its env is
// reconfigured — `enabled` is the user-visible "should this be live" gate
// and the same disabled rule that blocks `mcp_clients_connect` applies
// here. The new values are already persisted so a later `set_enabled(true)`
// + `connect` round-trip will pick them up.
if !server.enabled {
return Ok(RpcOutcome::new(
json!({
"server_id": server_id,
"status": "disabled",
"env_keys": server.env_keys,
}),
vec![format!(
"update_env persisted env for server_id={server_id} but did not reconnect: server is disabled"
)],
));
}
match connections::connect(config, &server).await {
Ok(tools) => {
let tool_count = tools.len() as u32;
+53
View File
@@ -29,6 +29,7 @@ pub fn all_controller_schemas() -> Vec<ControllerSchema> {
schemas("config_assist"),
schemas("registry_settings_get"),
schemas("registry_settings_set"),
schemas("set_enabled"),
// Setup-agent surface (mcp_setup namespace, lives in setup_ops.rs).
setup_schemas("search"),
setup_schemas("get"),
@@ -93,6 +94,10 @@ pub fn all_registered_controllers() -> Vec<RegisteredController> {
schema: schemas("registry_settings_set"),
handler: handle_registry_settings_set,
},
RegisteredController {
schema: schemas("set_enabled"),
handler: handle_set_enabled,
},
RegisteredController {
schema: setup_schemas("search"),
handler: handle_setup_search,
@@ -535,6 +540,40 @@ pub fn schemas(function: &str) -> ControllerSchema {
],
},
"set_enabled" => ControllerSchema {
namespace: "mcp_clients",
function: "set_enabled",
description: "Enable or disable an installed MCP server. Disabling auto-disconnects any live session and hides the server's tools from the agent; the install row and env values are kept so re-enabling does not require re-entry.",
inputs: vec![
FieldSchema {
name: "server_id",
ty: TypeSchema::String,
comment: "UUID of the installed server.",
required: true,
},
FieldSchema {
name: "enabled",
ty: TypeSchema::Bool,
comment: "Target state; `false` also disconnects.",
required: true,
},
],
outputs: vec![
FieldSchema {
name: "server_id",
ty: TypeSchema::String,
comment: "Echoed server id.",
required: true,
},
FieldSchema {
name: "enabled",
ty: TypeSchema::Bool,
comment: "Effective enabled state after the call.",
required: true,
},
],
},
// Handled by setup_schemas() — surface a clearer error rather than
// falling through to the generic unknown sink.
"setup_search"
@@ -655,6 +694,20 @@ fn handle_disconnect(params: Map<String, Value>) -> ControllerFuture {
})
}
fn handle_set_enabled(params: Map<String, Value>) -> ControllerFuture {
Box::pin(async move {
let config = config_rpc::load_config_with_timeout().await?;
let server_id = read_required::<String>(&params, "server_id")?;
let enabled = read_required::<bool>(&params, "enabled")?;
to_json(
crate::openhuman::mcp_registry::ops::mcp_clients_set_enabled(
&config, server_id, enabled,
)
.await?,
)
})
}
fn handle_status(params: Map<String, Value>) -> ControllerFuture {
Box::pin(async move {
let _ = params;
+7 -6
View File
@@ -67,9 +67,9 @@ fn schemas_unknown_function_returns_placeholder() {
#[test]
fn all_controller_schemas_covers_expected_methods() {
let schemas = all_controller_schemas();
// 13 mcp_clients (incl. update_env + registry_settings_get/set, #3039)
// + 6 mcp_setup.
assert_eq!(schemas.len(), 19);
// 14 mcp_clients (incl. update_env + registry_settings_get/set from #3039
// and set_enabled from #3196) + 6 mcp_setup.
assert_eq!(schemas.len(), 20);
let mcp_clients_count = schemas
.iter()
.filter(|s| s.namespace == "mcp_clients")
@@ -78,19 +78,20 @@ fn all_controller_schemas_covers_expected_methods() {
.iter()
.filter(|s| s.namespace == "mcp_setup")
.count();
assert_eq!(mcp_clients_count, 13);
assert_eq!(mcp_clients_count, 14);
assert_eq!(mcp_setup_count, 6);
// The #3039 additions are present.
// The #3039 + #3196 additions are present.
let functions: Vec<_> = schemas.iter().map(|s| s.function).collect();
assert!(functions.contains(&"update_env"));
assert!(functions.contains(&"registry_settings_get"));
assert!(functions.contains(&"registry_settings_set"));
assert!(functions.contains(&"set_enabled"));
}
#[test]
fn all_registered_controllers_has_handler_per_schema() {
let controllers = all_registered_controllers();
assert_eq!(controllers.len(), 19);
assert_eq!(controllers.len(), 20);
}
#[test]
+1
View File
@@ -302,6 +302,7 @@ pub async fn mcp_setup_install_and_connect(
installed_at: now_ms,
last_connected_at: None,
transport,
enabled: true,
};
store::insert_server(config, &server).map_err(|e| e.to_string())?;
+100 -4
View File
@@ -101,6 +101,13 @@ fn init_schema(conn: &Connection) -> Result<()> {
conn.execute("ALTER TABLE mcp_servers ADD COLUMN deployment_url TEXT", [])
.context("Failed to add deployment_url column to mcp_servers")?;
}
if !existing_cols.iter().any(|c| c == "enabled") {
conn.execute(
"ALTER TABLE mcp_servers ADD COLUMN enabled INTEGER NOT NULL DEFAULT 1",
[],
)
.context("Failed to add enabled column to mcp_servers")?;
}
Ok(())
}
@@ -140,8 +147,8 @@ pub fn insert_server_conn(conn: &Connection, server: &InstalledServer) -> Result
"INSERT INTO mcp_servers
(server_id, qualified_name, display_name, description, icon_url,
command_kind, command, args_json, env_keys_json, config_json,
installed_at, last_connected_at, transport, deployment_url)
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14)",
installed_at, last_connected_at, transport, deployment_url, enabled)
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15)",
params![
server.server_id,
server.qualified_name,
@@ -157,6 +164,7 @@ pub fn insert_server_conn(conn: &Connection, server: &InstalledServer) -> Result
server.last_connected_at,
server.transport.dispatch_kind(),
server.transport.deployment_url(),
server.enabled as i64,
],
)
.context("Failed to insert mcp_server")?;
@@ -188,7 +196,7 @@ pub fn list_servers_conn(conn: &Connection) -> Result<Vec<InstalledServer>> {
let mut stmt = conn.prepare(
"SELECT server_id, qualified_name, display_name, description, icon_url,
command_kind, command, args_json, env_keys_json, config_json,
installed_at, last_connected_at, transport, deployment_url
installed_at, last_connected_at, transport, deployment_url, enabled
FROM mcp_servers ORDER BY installed_at ASC",
)?;
let rows = stmt.query_map([], map_server_row)?;
@@ -207,7 +215,7 @@ pub fn get_server_conn(conn: &Connection, server_id: &str) -> Result<InstalledSe
let mut stmt = conn.prepare(
"SELECT server_id, qualified_name, display_name, description, icon_url,
command_kind, command, args_json, env_keys_json, config_json,
installed_at, last_connected_at, transport, deployment_url
installed_at, last_connected_at, transport, deployment_url, enabled
FROM mcp_servers WHERE server_id = ?1",
)?;
let mut rows = stmt.query(params![server_id])?;
@@ -269,6 +277,10 @@ fn map_server_row(row: &rusqlite::Row<'_>) -> rusqlite::Result<InstalledServer>
let deployment_url: Option<String> = row.get(13)?;
let transport = Transport::parse(&transport_kind, deployment_url.as_deref());
// `enabled` is a post-migration addition; fall back to `1` (true) for
// any row that predates the column so legacy installs keep auto-connecting.
let enabled: i64 = row.get::<_, Option<i64>>(14)?.unwrap_or(1);
Ok(InstalledServer {
server_id: row.get(0)?,
qualified_name: row.get(1)?,
@@ -283,9 +295,23 @@ fn map_server_row(row: &rusqlite::Row<'_>) -> rusqlite::Result<InstalledServer>
installed_at: row.get(10)?,
last_connected_at: row.get(11)?,
transport,
enabled: enabled != 0,
})
}
pub fn update_enabled(config: &Config, server_id: &str, enabled: bool) -> Result<()> {
with_connection(config, |conn| update_enabled_conn(conn, server_id, enabled))
}
pub fn update_enabled_conn(conn: &Connection, server_id: &str, enabled: bool) -> Result<()> {
conn.execute(
"UPDATE mcp_servers SET enabled = ?2 WHERE server_id = ?1",
params![server_id, enabled as i64],
)
.context("Failed to update mcp_server enabled flag")?;
Ok(())
}
// ── Env values ───────────────────────────────────────────────────────────────
/// Store (insert or replace) env key-value pairs for a server.
@@ -413,6 +439,7 @@ mod tests {
installed_at: 1_700_000_000_000,
last_connected_at: None,
transport: Transport::Stdio,
enabled: true,
}
}
@@ -433,6 +460,7 @@ mod tests {
transport: Transport::HttpRemote {
url: url.to_string(),
},
enabled: true,
}
}
@@ -569,6 +597,74 @@ mod tests {
assert_eq!(servers[1].transport, Transport::Stdio);
}
#[test]
fn enabled_defaults_true_and_roundtrips_false() {
let (_f, conn) = open_test_conn();
let mut server = sample_server("srv-en");
insert_server_conn(&conn, &server).unwrap();
let loaded = get_server_conn(&conn, "srv-en").unwrap();
assert!(loaded.enabled, "new installs default to enabled");
server.server_id = "srv-dis".to_string();
server.enabled = false;
insert_server_conn(&conn, &server).unwrap();
let loaded = get_server_conn(&conn, "srv-dis").unwrap();
assert!(!loaded.enabled);
}
#[test]
fn update_enabled_flips_persisted_value() {
let (_f, conn) = open_test_conn();
let server = sample_server("srv-u");
insert_server_conn(&conn, &server).unwrap();
update_enabled_conn(&conn, "srv-u", false).unwrap();
let loaded = get_server_conn(&conn, "srv-u").unwrap();
assert!(!loaded.enabled);
update_enabled_conn(&conn, "srv-u", true).unwrap();
let loaded = get_server_conn(&conn, "srv-u").unwrap();
assert!(loaded.enabled);
}
#[test]
fn additive_enabled_migration_defaults_legacy_rows_true() {
let tmp = tempfile::NamedTempFile::new().unwrap();
let conn = rusqlite::Connection::open(tmp.path()).unwrap();
// Pre-migration schema (no `enabled` column).
conn.execute_batch(
"CREATE TABLE mcp_servers (
server_id TEXT PRIMARY KEY,
qualified_name TEXT NOT NULL,
display_name TEXT NOT NULL,
description TEXT,
icon_url TEXT,
command_kind TEXT NOT NULL DEFAULT 'node',
command TEXT NOT NULL,
args_json TEXT NOT NULL DEFAULT '[]',
env_keys_json TEXT NOT NULL DEFAULT '[]',
config_json TEXT,
installed_at INTEGER NOT NULL,
last_connected_at INTEGER,
transport TEXT NOT NULL DEFAULT 'stdio',
deployment_url TEXT
);",
)
.unwrap();
conn.execute(
"INSERT INTO mcp_servers
(server_id, qualified_name, display_name, command_kind, command, installed_at)
VALUES ('legacy-en', '@old/server', 'Old', 'node', 'npx', 1700000000000)",
[],
)
.unwrap();
init_schema(&conn).unwrap();
init_schema(&conn).unwrap(); // idempotent
let loaded = get_server_conn(&conn, "legacy-en").unwrap();
assert!(loaded.enabled, "legacy rows default enabled=true");
}
/// Simulates the pre-migration state by dropping the `transport` and
/// `deployment_url` columns *after* schema init, manually inserting a
/// row that lacks them, and then re-running `init_schema` to confirm
+23 -1
View File
@@ -137,6 +137,13 @@ pub struct InstalledServer {
/// Defaults to `Stdio` for rows persisted before the column existed.
#[serde(default = "default_transport")]
pub transport: Transport,
/// Whether this server should be brought up at boot and exposed to the
/// agent. `false` keeps the install row + env values around (so the
/// user can re-enable without re-entering credentials) while preventing
/// auto-connect and hiding the server's tools from the agent. Defaults
/// to `true` for legacy rows persisted before the column existed.
#[serde(default = "default_enabled")]
pub enabled: bool,
}
/// Default for `InstalledServer::transport` when the field is missing from
@@ -146,6 +153,13 @@ fn default_transport() -> Transport {
Transport::Stdio
}
/// Default for `InstalledServer::enabled` when the field is missing from
/// a serialised payload (e.g. legacy persisted rows, callers that haven't
/// migrated their construction site yet).
fn default_enabled() -> bool {
true
}
// ── McpTool ─────────────────────────────────────────────────────────────────
/// A tool exposed by a connected MCP server.
@@ -166,6 +180,7 @@ pub enum ServerStatus {
Connecting,
Connected,
Error,
Disabled,
}
impl ServerStatus {
@@ -175,6 +190,7 @@ impl ServerStatus {
Self::Connecting => "connecting",
Self::Connected => "connected",
Self::Error => "error",
Self::Disabled => "disabled",
}
}
}
@@ -303,6 +319,7 @@ mod tests {
assert_eq!(ServerStatus::Disconnected.as_str(), "disconnected");
assert_eq!(ServerStatus::Connecting.as_str(), "connecting");
assert_eq!(ServerStatus::Error.as_str(), "error");
assert_eq!(ServerStatus::Disabled.as_str(), "disabled");
}
#[test]
@@ -351,6 +368,7 @@ mod tests {
installed_at: 1_700_000_000_000,
last_connected_at: None,
transport: Transport::Stdio,
enabled: true,
};
let v = serde_json::to_value(&server).unwrap();
// env_keys present, but no raw values
@@ -429,10 +447,14 @@ mod tests {
"config": null,
"installed_at": 1_700_000_000_000i64,
"last_connected_at": null
// ← deliberately no `transport` key
// ← deliberately no `transport` or `enabled` key
});
let s: InstalledServer = serde_json::from_value(legacy).unwrap();
assert_eq!(s.transport, Transport::Stdio);
assert!(
s.enabled,
"enabled should default to true when field is absent"
);
}
#[test]
+107
View File
@@ -8644,6 +8644,113 @@ async fn mcp_clients_install_connect_tool_call_happy_path() {
rpc_join.abort();
}
/// `mcp_clients_set_enabled` smoke: installs a server, disables it via RPC,
/// and asserts the response carries `enabled=false` (issue #3196).
#[tokio::test]
async fn mcp_clients_set_enabled_smoke() {
let _env_lock = json_rpc_e2e_env_lock();
let tmp = tempdir().expect("tempdir");
let home = tmp.path();
let openhuman_home = home.join(".openhuman");
let _home_guard = EnvVarGuard::set_to_path("HOME", home);
let _workspace_guard = EnvVarGuard::unset("OPENHUMAN_WORKSPACE");
let _backend_url_guard = EnvVarGuard::unset("BACKEND_URL");
let _vite_backend_guard = EnvVarGuard::unset("VITE_BACKEND_URL");
let (mock_addr, mock_join) = serve_on_ephemeral(mock_upstream_router()).await;
let mock_origin = format!("http://{}", mock_addr);
write_min_config(&openhuman_home, &mock_origin);
let user_scoped_dir = openhuman_home.join("users").join("local");
write_min_config(&user_scoped_dir, &mock_origin);
// Seed the registry detail cache so install resolves offline to the stub.
let stub_path = env!("CARGO_BIN_EXE_test-mcp-stub");
let qualified_name = "@openhuman-test/echo-set-enabled";
let detail = serde_json::json!({
"qualifiedName": qualified_name,
"displayName": "Test Echo SetEnabled",
"description": "Stub for set_enabled smoke.",
"connections": [{
"type": "stdio",
"published": true,
"exampleConfig": { "command": stub_path, "args": [] }
}]
});
let seed_config = openhuman_core::openhuman::config::load_config_with_timeout()
.await
.expect("load config for cache seed");
openhuman_core::openhuman::mcp_registry::store::set_cached(
&seed_config,
&format!("smithery:detail:{qualified_name}"),
&detail.to_string(),
)
.expect("seed smithery detail cache");
let (rpc_addr, rpc_join) = serve_on_ephemeral(build_core_http_router(false)).await;
let rpc_base = format!("http://{}", rpc_addr);
tokio::time::sleep(Duration::from_millis(100)).await;
// ── 1. install ───────────────────────────────────────────────────────────
let install = post_json_rpc(
&rpc_base,
9940,
"openhuman.mcp_clients_install",
json!({ "qualified_name": qualified_name, "env": {} }),
)
.await;
let install_result =
assert_no_jsonrpc_error(&install, "mcp_clients_install (set_enabled smoke)");
let install_body = install_result.get("result").unwrap_or(install_result);
let server_id = install_body
.get("server")
.and_then(|s| s.get("server_id"))
.and_then(Value::as_str)
.expect("install returns server.server_id")
.to_string();
// ── 2. set_enabled=false ─────────────────────────────────────────────────
let set_enabled = post_json_rpc(
&rpc_base,
9941,
"openhuman.mcp_clients_set_enabled",
json!({ "server_id": server_id, "enabled": false }),
)
.await;
let se_result = assert_no_jsonrpc_error(&set_enabled, "mcp_clients_set_enabled (false)");
let se_body = se_result.get("result").unwrap_or(se_result);
assert_eq!(
se_body.get("enabled"),
Some(&json!(false)),
"set_enabled should report enabled=false: {se_body}"
);
assert_eq!(
se_body.get("server_id").and_then(Value::as_str),
Some(server_id.as_str()),
"set_enabled should echo server_id: {se_body}"
);
// ── 3. set_enabled=true restores it ─────────────────────────────────────
let set_enabled_true = post_json_rpc(
&rpc_base,
9942,
"openhuman.mcp_clients_set_enabled",
json!({ "server_id": server_id, "enabled": true }),
)
.await;
let se_true_result =
assert_no_jsonrpc_error(&set_enabled_true, "mcp_clients_set_enabled (true)");
let se_true_body = se_true_result.get("result").unwrap_or(se_true_result);
assert_eq!(
se_true_body.get("enabled"),
Some(&json!(true)),
"set_enabled=true should report enabled=true: {se_true_body}"
);
mock_join.abort();
rpc_join.abort();
}
/// Registry settings RPC: the getter reports `*_set` booleans without ever
/// echoing secret values; the setter persists and clears them (issue #3039
/// gap A6).
+232
View File
@@ -35,6 +35,7 @@ fn make_installed_server() -> InstalledServer {
installed_at: 0,
last_connected_at: None,
transport: Transport::Stdio,
enabled: true,
}
}
@@ -115,3 +116,234 @@ async fn unknown_tool_call_returns_error() {
let _ = connections::disconnect(&server.server_id).await;
}
#[tokio::test]
async fn failed_connect_records_last_error() {
let (_tmp, cfg) = fresh_workspace_config();
let mut server = make_installed_server();
server.command = "/this/path/does/not/exist".to_string();
store::insert_server(&cfg, &server).expect("insert installed server");
let err = connections::connect(&cfg, &server)
.await
.expect_err("connect should fail for bogus command");
assert!(!err.to_string().is_empty());
let recorded = connections::last_error_for(&server.server_id).await;
assert!(
recorded.is_some(),
"LAST_ERRORS must hold the connect failure for server_id={}",
server.server_id
);
}
#[tokio::test]
async fn successful_connect_clears_last_error() {
let (_tmp, cfg) = fresh_workspace_config();
let mut server = make_installed_server();
server.command = "/nonexistent".to_string();
let _ = connections::connect(&cfg, &server).await;
assert!(connections::last_error_for(&server.server_id)
.await
.is_some());
server.command = env!("CARGO_BIN_EXE_test-mcp-stub").to_string();
connections::connect(&cfg, &server)
.await
.expect("real connect succeeds");
assert!(
connections::last_error_for(&server.server_id)
.await
.is_none(),
"successful connect must clear the prior error"
);
let _ = connections::disconnect(&server.server_id).await;
}
#[tokio::test]
async fn status_priority_disabled_outranks_connected() {
let (_tmp, cfg) = fresh_workspace_config();
let mut server = make_installed_server();
server.enabled = false;
store::insert_server(&cfg, &server).expect("insert");
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.status.as_str(),
"disabled",
"disabled server reports `disabled` even before any connect attempt"
);
assert!(mine.last_error.is_none());
}
#[tokio::test]
async fn status_reflects_last_connect_error() {
let (_tmp, cfg) = fresh_workspace_config();
let mut server = make_installed_server();
server.command = "/nonexistent".to_string();
store::insert_server(&cfg, &server).expect("insert");
let _ = connections::connect(&cfg, &server).await;
let statuses = connections::all_status(&cfg).await;
let mine = statuses
.iter()
.find(|s| s.server_id == server.server_id)
.unwrap();
assert_eq!(mine.status.as_str(), "error");
assert!(
mine.last_error
.as_deref()
.map(|s| !s.is_empty())
.unwrap_or(false),
"last_error populated"
);
}
#[tokio::test]
async fn boot_skips_disabled_servers_and_records_errors() {
use openhuman_core::openhuman::mcp_registry::boot;
let (_tmp, cfg) = fresh_workspace_config();
// Server A: enabled, real stub → connects.
let mut a = make_installed_server();
a.server_id = format!("a-{}", uuid::Uuid::new_v4());
store::insert_server(&cfg, &a).expect("insert a");
// Server B: enabled but command does not exist → records error, doesn't crash boot.
let mut b = make_installed_server();
b.server_id = format!("b-{}", uuid::Uuid::new_v4());
b.command = "/nonexistent-mcp".to_string();
store::insert_server(&cfg, &b).expect("insert b");
// Server C: disabled AND command is bogus. If boot ever attempts to
// connect this server, the bogus command will fail and LAST_ERRORS will
// hold an entry. The skip is the only way the post-boot last_error stays
// None — so the assertion below proves the skip actually fired, not just
// that the Disabled-priority logic masked the failure.
let mut c = make_installed_server();
c.server_id = format!("c-{}", uuid::Uuid::new_v4());
c.enabled = false;
c.command = "/nonexistent-disabled-server".to_string();
store::insert_server(&cfg, &c).expect("insert c");
boot::spawn_installed_servers(&cfg).await;
// A is connected; B recorded an error; C never attempted (no error
// recorded despite the bogus command).
let statuses = connections::all_status(&cfg).await;
let by_id = |id: &str| {
statuses
.iter()
.find(|s| s.server_id == id)
.cloned()
.unwrap()
};
assert_eq!(by_id(&a.server_id).status.as_str(), "connected");
assert_eq!(by_id(&b.server_id).status.as_str(), "error");
assert_eq!(by_id(&c.server_id).status.as_str(), "disabled");
assert!(
connections::last_error_for(&c.server_id).await.is_none(),
"disabled server with bogus command must not have been connect-attempted"
);
let _ = connections::disconnect(&a.server_id).await;
}
#[tokio::test]
async fn set_enabled_false_disconnects_running_server() {
use openhuman_core::openhuman::mcp_registry::ops;
let (_tmp, cfg) = fresh_workspace_config();
let server = make_installed_server();
store::insert_server(&cfg, &server).expect("insert");
connections::connect(&cfg, &server).await.expect("connect");
let outcome = ops::mcp_clients_set_enabled(&cfg, server.server_id.clone(), false)
.await
.expect("set_enabled ok");
assert_eq!(outcome.value["enabled"], serde_json::json!(false));
let loaded = store::get_server(&cfg, &server.server_id).unwrap();
assert!(!loaded.enabled);
let statuses = connections::all_status(&cfg).await;
let mine = statuses
.iter()
.find(|s| s.server_id == server.server_id)
.unwrap();
assert_eq!(mine.status.as_str(), "disabled");
}
#[tokio::test]
async fn connect_refuses_disabled_server() {
use openhuman_core::openhuman::mcp_registry::ops;
let (_tmp, cfg) = fresh_workspace_config();
let mut server = make_installed_server();
server.enabled = false;
store::insert_server(&cfg, &server).expect("insert");
let err = ops::mcp_clients_connect(&cfg, server.server_id.clone())
.await
.expect_err("connect must reject disabled server");
assert!(err.to_lowercase().contains("disabled"), "got: {err}");
}
#[tokio::test]
async fn set_enabled_true_clears_disabled_status_but_does_not_auto_connect() {
use openhuman_core::openhuman::mcp_registry::ops;
let (_tmp, cfg) = fresh_workspace_config();
let mut server = make_installed_server();
server.enabled = false;
store::insert_server(&cfg, &server).expect("insert");
ops::mcp_clients_set_enabled(&cfg, server.server_id.clone(), true)
.await
.expect("set_enabled true ok");
let statuses = connections::all_status(&cfg).await;
let mine = statuses
.iter()
.find(|s| s.server_id == server.server_id)
.unwrap();
assert_eq!(
mine.status.as_str(),
"disconnected",
"re-enabling alone must not bring up the subprocess; the user calls connect explicitly"
);
}
#[tokio::test]
async fn update_env_on_disabled_server_persists_but_does_not_reconnect() {
use openhuman_core::openhuman::mcp_registry::ops;
use std::collections::HashMap;
let (_tmp, cfg) = fresh_workspace_config();
let mut server = make_installed_server();
server.enabled = false;
store::insert_server(&cfg, &server).expect("insert");
let mut env = HashMap::new();
env.insert("API_KEY".to_string(), "deadbeef".to_string());
let outcome = ops::mcp_clients_update_env(&cfg, server.server_id.clone(), env)
.await
.expect("update_env on disabled server returns Ok");
assert_eq!(
outcome.value["status"], "disabled",
"disabled server reports status=disabled instead of reconnecting"
);
let statuses = connections::all_status(&cfg).await;
let mine = statuses
.iter()
.find(|s| s.server_id == server.server_id)
.unwrap();
assert_eq!(mine.status.as_str(), "disabled");
}
+378
View File
@@ -0,0 +1,378 @@
//! Regression tests for multi-MCP-server support (#3196).
//!
//! These verify the three risks the issue calls out:
//! 1. Two servers exposing identically-named tools both reach the agent
//! surface without collision (namespaced tool_id keys via server_id).
//! 2. One server failing to connect does not block another from running.
//! 3. Tool listing aggregates across all connected servers and tolerates
//! a missing one.
//!
//! All tests are hermetic — they use the `test-mcp-stub` binary built
//! by Cargo (exposed via `CARGO_BIN_EXE_test-mcp-stub`) and a fresh
//! temp-dir workspace so they do not share SQLite state with each other
//! or with `mcp_registry_e2e.rs`.
use openhuman_core::openhuman::config::Config;
use openhuman_core::openhuman::mcp_registry::types::{CommandKind, InstalledServer, Transport};
use openhuman_core::openhuman::mcp_registry::{connections, ops, store};
// ── helpers ──────────────────────────────────────────────────────────────────
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_stub_server(qualified: &str) -> InstalledServer {
let stub = env!("CARGO_BIN_EXE_test-mcp-stub");
InstalledServer {
server_id: format!("multi-{}", uuid::Uuid::new_v4()),
qualified_name: qualified.to_string(),
display_name: qualified.to_string(),
description: None,
icon_url: None,
command_kind: CommandKind::Binary,
command: stub.to_string(),
args: Vec::new(),
env_keys: Vec::new(),
config: None,
installed_at: 0,
last_connected_at: None,
transport: Transport::Stdio,
enabled: true,
}
}
// ── extract response text (mirrors mcp_registry_e2e.rs convention) ───────────
fn extract_echo_text(result: &serde_json::Value) -> &str {
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("")
}
// ── Test 1: name collision ────────────────────────────────────────────────────
/// Two stub servers both expose the same tool name (`echo`). After connecting
/// both, `all_connected_tools` returns two entries — one per server — and both
/// respond correctly when called by `server_id`, proving the `server_id` is
/// the disambiguator for identically-named tools.
#[tokio::test]
async fn two_servers_same_tool_name_no_collision() {
let (_tmp, cfg) = fresh_workspace_config();
let server_a = make_stub_server("@multi-test/echo-a");
let server_b = make_stub_server("@multi-test/echo-b");
// Both server_ids must be distinct (sanity guard for the disambiguation
// assertions below).
assert_ne!(
server_a.server_id, server_b.server_id,
"two stub servers must have distinct server_ids"
);
store::insert_server(&cfg, &server_a).expect("insert server_a");
store::insert_server(&cfg, &server_b).expect("insert server_b");
// Connect both — each spawns its own subprocess.
let tools_a = connections::connect(&cfg, &server_a)
.await
.expect("connect server_a");
let tools_b = connections::connect(&cfg, &server_b)
.await
.expect("connect server_b");
assert_eq!(tools_a.len(), 1, "server_a: stub advertises one tool");
assert_eq!(tools_b.len(), 1, "server_b: stub advertises one tool");
assert_eq!(tools_a[0].name, "echo", "server_a tool name is `echo`");
assert_eq!(tools_b[0].name, "echo", "server_b tool name is `echo`");
// Both servers are in the connected aggregate.
let all_tools = connections::all_connected_tools().await;
let a_tools: Vec<_> = all_tools
.iter()
.filter(|(sid, _, _)| sid == &server_a.server_id)
.collect();
let b_tools: Vec<_> = all_tools
.iter()
.filter(|(sid, _, _)| sid == &server_b.server_id)
.collect();
assert_eq!(
a_tools.len(),
1,
"server_a contributes exactly one tool to the aggregate"
);
assert_eq!(
b_tools.len(),
1,
"server_b contributes exactly one tool to the aggregate"
);
assert_eq!(
a_tools[0].2.name, "echo",
"server_a's aggregated tool is `echo`"
);
assert_eq!(
b_tools[0].2.name, "echo",
"server_b's aggregated tool is `echo`"
);
// Both are `connected` in all_status.
let statuses = connections::all_status(&cfg).await;
let find = |id: &str| {
statuses
.iter()
.find(|s| s.server_id == id)
.cloned()
.unwrap_or_else(|| panic!("status entry missing for {id}"))
};
assert_eq!(find(&server_a.server_id).status.as_str(), "connected");
assert_eq!(find(&server_a.server_id).tool_count, 1);
assert_eq!(find(&server_b.server_id).status.as_str(), "connected");
assert_eq!(find(&server_b.server_id).tool_count, 1);
let _ = connections::disconnect(&server_a.server_id).await;
let _ = connections::disconnect(&server_b.server_id).await;
}
// ── Test 2: routing ───────────────────────────────────────────────────────────
/// `call_tool` routes by `server_id` even with a shared name — each server
/// subprocess receives and echoes back the exact payload sent to that server_id.
/// Demonstrates the agent → core call path works across multiple servers without
/// cross-wiring.
#[tokio::test]
async fn tool_calls_route_to_the_correct_server() {
let (_tmp, cfg) = fresh_workspace_config();
let server_a = make_stub_server("@route-test/echo-a");
let server_b = make_stub_server("@route-test/echo-b");
store::insert_server(&cfg, &server_a).expect("insert server_a");
store::insert_server(&cfg, &server_b).expect("insert server_b");
connections::connect(&cfg, &server_a)
.await
.expect("connect server_a");
connections::connect(&cfg, &server_b)
.await
.expect("connect server_b");
// Send distinct payloads to each server; verify each echoes its own input.
let result_a = connections::call_tool(
&server_a.server_id,
"echo",
serde_json::json!({ "message": "payload-for-a" }),
)
.await
.expect("call_tool on server_a should succeed");
let result_b = connections::call_tool(
&server_b.server_id,
"echo",
serde_json::json!({ "message": "payload-for-b" }),
)
.await
.expect("call_tool on server_b should succeed");
let text_a = extract_echo_text(&result_a);
let text_b = extract_echo_text(&result_b);
assert_eq!(
text_a, "payload-for-a",
"server_a must echo back exactly what was sent to it"
);
assert_eq!(
text_b, "payload-for-b",
"server_b must echo back exactly what was sent to it"
);
// Cross-verify: calling server_b's id echoes server_b's payload — not server_a's.
assert_ne!(
text_a, text_b,
"two servers must not produce identical outputs for different inputs"
);
let _ = connections::disconnect(&server_a.server_id).await;
let _ = connections::disconnect(&server_b.server_id).await;
}
// ── Test 3: failure isolation ─────────────────────────────────────────────────
/// One server failing to connect must not prevent another from connecting or
/// being usable in the same session. The bad server records an `error` status
/// with a `last_error` message; the good server is `connected` and responds.
#[tokio::test]
async fn failed_connect_does_not_block_healthy_peer() {
let (_tmp, cfg) = fresh_workspace_config();
// "bad" server: non-existent binary.
let mut bad = make_stub_server("@isolation-test/bad");
bad.command = "/this/path/does/not/exist/bad-mcp".to_string();
// "good" server: real stub.
let good = make_stub_server("@isolation-test/good");
store::insert_server(&cfg, &bad).expect("insert bad server");
store::insert_server(&cfg, &good).expect("insert good server");
// Connecting the bad server must fail.
let connect_err = connections::connect(&cfg, &bad)
.await
.expect_err("connect bad server should fail");
assert!(
!connect_err.to_string().is_empty(),
"bad connect error must not be empty"
);
// Connecting the good server must succeed independently.
let good_tools = connections::connect(&cfg, &good)
.await
.expect("connect good server must succeed regardless of the bad peer");
assert_eq!(good_tools.len(), 1, "good server exposes one tool");
// all_status: bad → error with last_error, good → connected.
let statuses = connections::all_status(&cfg).await;
let find = |id: &str| {
statuses
.iter()
.find(|s| s.server_id == id)
.cloned()
.unwrap_or_else(|| panic!("status entry missing for {id}"))
};
let bad_status = find(&bad.server_id);
assert_eq!(
bad_status.status.as_str(),
"error",
"bad server must report `error`"
);
assert!(
bad_status
.last_error
.as_deref()
.map(|s| !s.is_empty())
.unwrap_or(false),
"bad server must have a non-empty last_error"
);
let good_status = find(&good.server_id);
assert_eq!(
good_status.status.as_str(),
"connected",
"good server must be `connected`"
);
assert!(
good_status.last_error.is_none(),
"good server must have no last_error"
);
// The good server is still callable after the peer's failure.
let result = connections::call_tool(
&good.server_id,
"echo",
serde_json::json!({ "message": "isolation-check" }),
)
.await
.expect("call_tool on good server must succeed after peer failure");
assert_eq!(
extract_echo_text(&result),
"isolation-check",
"good server must echo input correctly after peer failure"
);
let _ = connections::disconnect(&good.server_id).await;
}
// ── Test 4: disabled enforcement ──────────────────────────────────────────────
/// A disabled server must not contribute tools to the agent surface and must
/// refuse explicit connect, even when a sibling is connected and active.
#[tokio::test]
async fn disabled_server_contributes_no_tools_to_agent_surface() {
let (_tmp, cfg) = fresh_workspace_config();
// "live" server: enabled, real stub.
let live = make_stub_server("@disabled-test/live");
// "quiet" server: disabled, also uses the real stub binary (so the
// bogus-command failure path cannot mask the disabled enforcement check).
let mut quiet = make_stub_server("@disabled-test/quiet");
quiet.enabled = false;
store::insert_server(&cfg, &live).expect("insert live server");
store::insert_server(&cfg, &quiet).expect("insert quiet server");
// Connect the live server.
connections::connect(&cfg, &live)
.await
.expect("connect live server");
// Attempting to connect the disabled server via ops must fail with
// a clear "disabled" message (matches mcp_registry_e2e::connect_refuses_disabled_server).
let disabled_err = ops::mcp_clients_connect(&cfg, quiet.server_id.clone())
.await
.expect_err("connect must reject a disabled server");
assert!(
disabled_err.to_lowercase().contains("disabled"),
"error must mention 'disabled', got: {disabled_err}"
);
// all_status: live → connected, quiet → disabled.
let statuses = connections::all_status(&cfg).await;
let find = |id: &str| {
statuses
.iter()
.find(|s| s.server_id == id)
.cloned()
.unwrap_or_else(|| panic!("status entry missing for {id}"))
};
assert_eq!(find(&live.server_id).status.as_str(), "connected");
assert_eq!(find(&quiet.server_id).status.as_str(), "disabled");
// The disabled server must not appear in all_connected_tools.
let all_tools = connections::all_connected_tools().await;
let quiet_tools: Vec<_> = all_tools
.iter()
.filter(|(sid, _, _)| sid == &quiet.server_id)
.collect();
assert!(
quiet_tools.is_empty(),
"disabled server must contribute zero tools to the agent surface; found: {quiet_tools:?}"
);
// The live server IS present and callable.
let live_tools: Vec<_> = all_tools
.iter()
.filter(|(sid, _, _)| sid == &live.server_id)
.collect();
assert_eq!(
live_tools.len(),
1,
"live server must contribute its tool even while disabled peer is present"
);
let result = connections::call_tool(
&live.server_id,
"echo",
serde_json::json!({ "message": "live-while-quiet" }),
)
.await
.expect("call_tool on live server must succeed");
assert_eq!(
extract_echo_text(&result),
"live-while-quiet",
"live server echoes correctly while disabled peer exists"
);
let _ = connections::disconnect(&live.server_id).await;
}
@@ -287,6 +287,7 @@ fn test_mcp_server() -> InstalledServer {
installed_at: 0,
last_connected_at: None,
transport: Transport::Stdio,
enabled: true,
}
}