Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions apps/desktop/src/api/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,23 @@ export function saveMcpServers(
})
}

/** Flip one server's `enabled` bit (the Connectors toggle). Scoped like
* `saveMcpServers`, but on the per-server endpoint: the whole-map replace
* 400s when ANY entry in `mcp_servers` fails validation, which turned every
* toggle into a silent no-op for configs with one bad entry. */
export function setMcpServerEnabledFor(
name: string,
enabled: boolean,
profile?: ProfileScope
): Promise<{ ok: boolean }> {
return window.hermesDesktop.api<{ ok: boolean }>({
...capabilityScoped(profile),
path: `/api/mcp/servers/${encodeURIComponent(name)}/enabled`,
method: 'PUT',
body: { enabled }
})
}

/** Capture the source before the first await. Every OAuth RPC, including
* cleanup after a foreground switch, belongs to this (connection, profile).
* Import the store lazily: it consumes the API barrel during initialization. */
Expand Down
187 changes: 187 additions & 0 deletions apps/desktop/src/app/capabilities/mcp/use-mcp-servers.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
// @vitest-environment jsdom
import { QueryClientProvider } from '@tanstack/react-query'
import { act, cleanup, renderHook, waitFor } from '@testing-library/react'
import { atom } from 'nanostores'
import { createElement } from 'react'
import { afterEach, describe, expect, it, vi } from 'vitest'

import type * as HermesApi from '@/hermes'
import { getMcpCatalog, saveMcpServers, setMcpServerEnabledFor } from '@/hermes'
import { notifyError } from '@/store/notifications'
import { queryClient } from '@/lib/query-client'

import { useMcpServers } from './use-mcp-servers'

const mocks = vi.hoisted(() => ({
getConfig: vi.fn(),
runProbe: vi.fn()
}))

vi.mock('@/hermes', async importOriginal => ({
...(await importOriginal<typeof HermesApi>()),
getHermesConfigRecord: mocks.getConfig,
getMcpCatalog: vi.fn().mockResolvedValue({ entries: [] }),
peekConfigReadOrigin: vi.fn(() => null),
profileScopeKey: vi.fn(() => 'test-scope'),
retainConfigReadOrigin: vi.fn((record: unknown) => record),
saveMcpServers: vi.fn().mockResolvedValue({ ok: true }),
setMcpServerEnabledFor: vi.fn().mockResolvedValue({ ok: true })
}))

vi.mock('@/i18n', () => ({
useI18n: () => ({
t: {
settings: {
mcp: {
authenticatedMessage: () => '',
authenticatedTitle: '',
invalidJson: '',
reloadFailed: 'Reload failed',
removeFailed: '',
savedMessage: () => '',
savedTitle: '',
saveFailed: 'Could not save MCP servers'
}
}
}
})
}))

vi.mock('@/lib/mcp-dashboard-oauth', () => ({
completeMcpDesktopOAuth: vi.fn()
}))

vi.mock('@/lib/mcp-cost', () => ({
estimateServerTokens: vi.fn(() => null),
serverUsageCount: vi.fn(() => null)
}))

vi.mock('@/lib/mcp-probe-cache', () => ({
NEEDS_AUTH_RE: /needs auth/i,
probeCache: { get: vi.fn(), set: vi.fn() },
probeKey: vi.fn(() => 'probe')
}))

vi.mock('@/store/connections', () => ({
$activeConnectionId: atom(null)
}))

vi.mock('@/store/free-tier', () => ({
$freeTierStatus: atom(null)
}))

vi.mock('@/store/notifications', () => ({
notify: vi.fn(),
notifyError: vi.fn()
}))

vi.mock('@/store/profile', () => ({
$activeGatewayProfile: atom(null),
normalizeProfileKey: (name: null | string | undefined) => (name ?? '').trim() || 'default'
}))

vi.mock('@/store/session', () => ({
$activeSessionId: atom(null)
}))

vi.mock('./use-mcp-draft', () => ({
useMcpDraft: () => ({
cursor: 0,
dirty: false,
draft: '',
patchDraft: vi.fn(),
reset: vi.fn(),
resetDraft: vi.fn(),
setCursor: vi.fn()
})
}))

vi.mock('./use-mcp-probes', () => ({
useMcpProbes: () => ({
costFor: () => ({ tokens: null, uses: null }),
probes: {},
resetForProfileSwitch: vi.fn(),
retainProbes: vi.fn(),
runProbe: mocks.runProbe,
setProbe: vi.fn(),
toolCounts: {},
usageByServer: {}
})
}))

afterEach(() => {
cleanup()
queryClient.clear()
window.localStorage.clear()
vi.clearAllMocks()
})

const wrapper = ({ children }: { children: React.ReactNode }) =>
createElement(QueryClientProvider, { client: queryClient }, children)

const CONFIG = { mcp_servers: { alpha: { command: 'npx', args: ['-y', 'alpha'] } } }

async function renderController() {
mocks.getConfig.mockResolvedValue(CONFIG)

const { result } = renderHook(() => useMcpServers({ gateway: null }), { wrapper })

await waitFor(() => expect(result.current.configLoading).toBe(false))
await waitFor(() => expect(result.current.profilePending).toBe(false))

return result
}

describe('setServerEnabled', () => {
it('disables through the per-server endpoint, not the whole-map save', async () => {
const { result } = await renderController()

await act(async () => {
await result.current.setServerEnabled('alpha', false)
})

expect(setMcpServerEnabledFor).toHaveBeenCalledWith('alpha', false, undefined)
expect(saveMcpServers).not.toHaveBeenCalled()
expect(result.current.servers.alpha.enabled).toBe(false)
expect(mocks.runProbe).not.toHaveBeenCalled()
})

it('re-enables by dropping the enabled key and probes the server', async () => {
const { result } = await renderController()

await act(async () => {
await result.current.setServerEnabled('alpha', false)
})
await act(async () => {
await result.current.setServerEnabled('alpha', true)
})

expect(setMcpServerEnabledFor).toHaveBeenLastCalledWith('alpha', true, undefined)
expect('enabled' in result.current.servers.alpha).toBe(false)
expect(mocks.runProbe).toHaveBeenCalledWith('alpha')
})

it('does not paint the cache or stay silent when the backend rejects the toggle', async () => {
const { result } = await renderController()

vi.mocked(setMcpServerEnabledFor).mockResolvedValueOnce({ ok: false })

await act(async () => {
await result.current.setServerEnabled('alpha', false)
})

expect(notifyError).toHaveBeenCalledTimes(1)
expect('enabled' in result.current.servers.alpha).toBe(false)
expect(saveMcpServers).not.toHaveBeenCalled()
})

it('skips servers the config does not know', async () => {
const { result } = await renderController()

await act(async () => {
await result.current.setServerEnabled('ghost', true)
})

expect(setMcpServerEnabledFor).not.toHaveBeenCalled()
})
})
44 changes: 42 additions & 2 deletions apps/desktop/src/app/capabilities/mcp/use-mcp-servers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import {
type McpTestResult,
type ProfileScope,
profileScopeKey,
saveMcpServers
saveMcpServers,
setMcpServerEnabledFor
} from '@/hermes'
import { useI18n } from '@/i18n'
import { completeMcpDesktopOAuth } from '@/lib/mcp-dashboard-oauth'
Expand Down Expand Up @@ -313,7 +314,46 @@ export function useMcpServers({ gateway, profile }: UseMcpServersOptions): McpSe
}

const setServerEnabled = async (serverName: string, enabled: boolean) => {
if ((await writeEntry(serverName, entry => withEnabled(entry, enabled))) && enabled) {
const base = servers[serverName]

if (!base || profilePending) {
return
}

const epoch = profileEpoch.current

try {
const outcome = await setMcpServerEnabledFor(serverName, enabled, profile ?? undefined)

if (profileEpoch.current !== epoch) {
return
}

// The per-server endpoint, not a whole-map `saveMcpServers`: one invalid
// entry anywhere in `mcp_servers` 400s the replace, which used to leave
// every toggle a silent no-op. Both failure shapes land in the notify.
if (outcome.ok !== true) {
throw new Error(`server '${serverName}' rejected the ${enabled ? 'enable' : 'disable'}`)
}
} catch (err) {
notifyError(err, m.saveFailed)

return
}

// Paint only after the backend accepted the write. The optimistic paint
// flipped the switch and reverted on the next config refetch, reading as
// "the toggle does nothing".
const nextServers = { ...servers, [serverName]: withEnabled(base, enabled) }

setConfig(current => (current ? { ...current, mcp_servers: nextServers } : current))
mirror(nextServers, doc =>
doc[serverName] ? { ...doc, [serverName]: withEnabled(doc[serverName], enabled) } : doc
)

void silentReload()

if (enabled) {
void fleet.runProbe(serverName)
}
}
Expand Down