From e7f2958c167018155d0d700c98de47da185f51c4 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 25 Jul 2026 02:02:55 +0000 Subject: [PATCH 1/2] fix(mcp): treat unknown MCP server names as caller errors resolveMcpServerSetting threw plain Error for missing/blank server identifiers, so routine agent typos (e.g. reconnecting a removed server) opened Sentry issues. Throw McpCallerError so observability keeps them on mcp-event logs only. --- .../mcp-servers/shared.node.test.ts | 93 +++++++++++++++++++ .../mcp/capabilities/mcp-servers/shared.ts | 6 +- 2 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 packages/worker/src/mcp/capabilities/mcp-servers/shared.node.test.ts diff --git a/packages/worker/src/mcp/capabilities/mcp-servers/shared.node.test.ts b/packages/worker/src/mcp/capabilities/mcp-servers/shared.node.test.ts new file mode 100644 index 0000000000..f0cb5646be --- /dev/null +++ b/packages/worker/src/mcp/capabilities/mcp-servers/shared.node.test.ts @@ -0,0 +1,93 @@ +import { expect, test, vi } from 'vitest' +import { McpCallerError } from '#mcp/caller-error.ts' +import { type McpServerSettingMetadata } from '#worker/mcp-client/settings-types.ts' + +const mockModule = vi.hoisted(() => ({ + getMcpServerSettingById: vi.fn(), + listMcpServerSettings: vi.fn(), +})) + +vi.mock('#worker/mcp-client/settings-service.ts', () => ({ + getMcpServerSettingById: (...args: Array) => + mockModule.getMcpServerSettingById(...args), + listMcpServerSettings: (...args: Array) => + mockModule.listMcpServerSettings(...args), +})) + +const { resolveMcpServerSetting } = await import('./shared.ts') + +function setting( + overrides: Partial = {}, +): McpServerSettingMetadata { + return { + id: 'server-1', + name: 'ha', + url: 'https://example.com/mcp', + enabled: true, + createdAt: '2026-01-01T00:00:00.000Z', + updatedAt: '2026-01-01T00:00:00.000Z', + ...overrides, + } +} + +test('resolveMcpServerSetting throws McpCallerError for unknown server names', async () => { + mockModule.getMcpServerSettingById.mockReset() + mockModule.listMcpServerSettings.mockReset() + mockModule.getMcpServerSettingById.mockResolvedValue(null) + mockModule.listMcpServerSettings.mockResolvedValue([setting()]) + + const missing = resolveMcpServerSetting({ + env: { APP_DB: {} as D1Database }, + userId: 'user-1', + server: 'recipe-keeper', + }) + + await expect(missing).rejects.toThrow(McpCallerError) + await expect(missing).rejects.toThrow( + 'No MCP server matches "recipe-keeper". Saved servers: ha.', + ) +}) + +test('resolveMcpServerSetting throws McpCallerError for blank server identifiers', async () => { + mockModule.getMcpServerSettingById.mockReset() + mockModule.listMcpServerSettings.mockReset() + + const blank = resolveMcpServerSetting({ + env: { APP_DB: {} as D1Database }, + userId: 'user-1', + server: ' ', + }) + + await expect(blank).rejects.toThrow(McpCallerError) + await expect(blank).rejects.toThrow( + 'Provide the MCP server id or name in "server".', + ) + expect(mockModule.getMcpServerSettingById).not.toHaveBeenCalled() +}) + +test('resolveMcpServerSetting resolves by id or normalized name', async () => { + mockModule.getMcpServerSettingById.mockReset() + mockModule.listMcpServerSettings.mockReset() + + const byId = setting({ id: 'server-by-id', name: 'ha' }) + mockModule.getMcpServerSettingById.mockResolvedValueOnce(byId) + await expect( + resolveMcpServerSetting({ + env: { APP_DB: {} as D1Database }, + userId: 'user-1', + server: 'server-by-id', + }), + ).resolves.toEqual(byId) + + mockModule.getMcpServerSettingById.mockResolvedValueOnce(null) + mockModule.listMcpServerSettings.mockResolvedValueOnce([ + setting({ id: 'server-by-name', name: 'ha' }), + ]) + await expect( + resolveMcpServerSetting({ + env: { APP_DB: {} as D1Database }, + userId: 'user-1', + server: 'HA', + }), + ).resolves.toMatchObject({ id: 'server-by-name', name: 'ha' }) +}) diff --git a/packages/worker/src/mcp/capabilities/mcp-servers/shared.ts b/packages/worker/src/mcp/capabilities/mcp-servers/shared.ts index 9eeeb171d3..ae52ff81ea 100644 --- a/packages/worker/src/mcp/capabilities/mcp-servers/shared.ts +++ b/packages/worker/src/mcp/capabilities/mcp-servers/shared.ts @@ -1,5 +1,6 @@ import { z } from 'zod' import { normalizeMcpServerName } from '@kody-internal/shared/mcp-servers.ts' +import { McpCallerError } from '#mcp/caller-error.ts' import { getCachedMcpClientHubSnapshot } from '#worker/mcp-client/hub-client.ts' import { getMcpServerSettingById, @@ -66,7 +67,7 @@ export async function resolveMcpServerSetting(input: { }): Promise { const identifier = input.server.trim() if (!identifier) { - throw new Error('Provide the MCP server id or name in "server".') + throw new McpCallerError('Provide the MCP server id or name in "server".') } const byId = await getMcpServerSettingById({ env: input.env, @@ -82,7 +83,8 @@ export async function resolveMcpServerSetting(input: { const byName = settings.find((setting) => setting.name === normalized) if (byName) return byName const available = settings.map((setting) => setting.name).join(', ') || 'none' - throw new Error( + // Unknown id/name is a caller mistake (stale name, typo). Keep it off Sentry. + throw new McpCallerError( `No MCP server matches "${identifier}". Saved servers: ${available}.`, ) } From 72c30323c9eca3a93e7359e97f628c09f2e63f8e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 26 Jul 2026 10:57:17 +0000 Subject: [PATCH 2/2] test(mcp): assert blank server skips list lookup Address CodeRabbit feedback on the blank-identifier path. --- .../worker/src/mcp/capabilities/mcp-servers/shared.node.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/worker/src/mcp/capabilities/mcp-servers/shared.node.test.ts b/packages/worker/src/mcp/capabilities/mcp-servers/shared.node.test.ts index f0cb5646be..2e5010b34b 100644 --- a/packages/worker/src/mcp/capabilities/mcp-servers/shared.node.test.ts +++ b/packages/worker/src/mcp/capabilities/mcp-servers/shared.node.test.ts @@ -63,6 +63,7 @@ test('resolveMcpServerSetting throws McpCallerError for blank server identifiers 'Provide the MCP server id or name in "server".', ) expect(mockModule.getMcpServerSettingById).not.toHaveBeenCalled() + expect(mockModule.listMcpServerSettings).not.toHaveBeenCalled() }) test('resolveMcpServerSetting resolves by id or normalized name', async () => {