diff --git a/packages/worker/src/mcp/caller-error.ts b/packages/worker/src/mcp/caller-error.ts new file mode 100644 index 0000000000..4b42bc3439 --- /dev/null +++ b/packages/worker/src/mcp/caller-error.ts @@ -0,0 +1,22 @@ +import { getErrorCauseChain } from '@kody-internal/shared/error-message.ts' + +/** + * A failure the caller can clear from the message alone: a missing or + * conflicting argument, an id that does not resolve, a precondition the caller + * has to undo first. Kody's MCP surface is agent-facing, so these are routine + * conversation turns rather than defects — they stay on the `mcp-event` log + * line and out of Sentry, where they open issues that look like platform bugs + * and trip triage automation. + */ +export class McpCallerError extends Error { + constructor(message: string, options?: ErrorOptions) { + super(message, options) + this.name = 'McpCallerError' + } +} + +export function isMcpCallerError(error: unknown) { + return getErrorCauseChain(error).some( + (entry) => entry instanceof McpCallerError, + ) +} diff --git a/packages/worker/src/mcp/capabilities/meta/search.ts b/packages/worker/src/mcp/capabilities/meta/search.ts index b4c2a6454f..f97d1f210a 100644 --- a/packages/worker/src/mcp/capabilities/meta/search.ts +++ b/packages/worker/src/mcp/capabilities/meta/search.ts @@ -1,4 +1,5 @@ import { z } from 'zod' +import { McpCallerError } from '#mcp/caller-error.ts' import { defineDomainCapability } from '#mcp/capabilities/define-domain-capability.ts' import { capabilityDomainNames } from '#mcp/capabilities/domain-metadata.ts' import { type CapabilityContext } from '#mcp/capabilities/types.ts' @@ -115,7 +116,7 @@ export const searchCapability = defineDomainCapability( const query = args.query?.trim() ?? '' const domainFilter = args.domain?.trim() || undefined if (!query && !domainFilter) { - throw new Error('Provide "query" or "domain".') + throw new McpCallerError('Provide "query" or "domain".') } const conversationId = resolveConversationId(args.conversationId) const userId = ctx.callerContext.user?.userId ?? null diff --git a/packages/worker/src/mcp/capabilities/openapi-provider/index.ts b/packages/worker/src/mcp/capabilities/openapi-provider/index.ts index ef0a67bf47..f26cc873fa 100644 --- a/packages/worker/src/mcp/capabilities/openapi-provider/index.ts +++ b/packages/worker/src/mcp/capabilities/openapi-provider/index.ts @@ -2,6 +2,7 @@ import { defineCapability } from '#mcp/capabilities/define-capability.ts' import { defineDomain } from '#mcp/capabilities/define-domain.ts' import { type CapabilityDomain } from '#mcp/capabilities/domain-metadata.ts' import { type Capability, type DomainSpec } from '#mcp/capabilities/types.ts' +import { McpCallerError } from '#mcp/caller-error.ts' import { type OpenApiBinding, type OpenApiBindingOperation, @@ -190,7 +191,7 @@ function createCapabilityFromOperation(input: { async handler(args, ctx) { const userId = ctx.callerContext.user?.userId if (!userId) { - throw new Error( + throw new McpCallerError( `OpenAPI capability "${binding.name}:${operation.slug}" requires an authenticated user.`, ) } diff --git a/packages/worker/src/mcp/capabilities/openapi-provider/operation-request.node.test.ts b/packages/worker/src/mcp/capabilities/openapi-provider/operation-request.node.test.ts index 3630ee8fd3..60801c5ce5 100644 --- a/packages/worker/src/mcp/capabilities/openapi-provider/operation-request.node.test.ts +++ b/packages/worker/src/mcp/capabilities/openapi-provider/operation-request.node.test.ts @@ -1,4 +1,5 @@ import { expect, test, vi } from 'vitest' +import { McpCallerError } from '#mcp/caller-error.ts' import * as secretService from '#mcp/secrets/service.ts' import * as communityRepo from '#worker/community/repo.ts' import * as packageRepo from '#worker/package-registry/repo.ts' @@ -222,6 +223,49 @@ test('rejects absolute and protocol-relative operation paths', async () => { } }) +test('missing path parameters throw McpCallerError so Sentry skips them', async () => { + const error = await executeOpenApiOperationRequest({ + env: createEnv(), + userId: 'user-1', + baseUrl: 'https://app.example.com', + storageContext: null, + binding: { + ...bindingBase, + apiBaseUrl: 'https://api.widgets.example', + }, + operation: getWidget, + args: {}, + globalFetch: vi.fn() as unknown as typeof fetch, + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(McpCallerError) + expect(error).toMatchObject({ + message: + 'Missing required path parameter "widgetId" for OpenAPI operation path /widgets/{widgetId}.', + }) +}) + +test('non-object params throw McpCallerError', async () => { + const error = await executeOpenApiOperationRequest({ + env: createEnv(), + userId: 'user-1', + baseUrl: 'https://app.example.com', + storageContext: null, + binding: { + ...bindingBase, + apiBaseUrl: 'https://api.widgets.example', + }, + operation: getWidget, + args: { params: ['not-an-object'] as unknown as Record }, + globalFetch: vi.fn() as unknown as typeof fetch, + }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(McpCallerError) + expect(error).toMatchObject({ + message: 'OpenAPI operation args.params must be an object.', + }) +}) + test('injects auth headers and overrides caller attempts', async () => { const usageSpy = vi .spyOn(usageModule, 'recordUsage') diff --git a/packages/worker/src/mcp/capabilities/openapi-provider/operation-request.ts b/packages/worker/src/mcp/capabilities/openapi-provider/operation-request.ts index 788c8894ab..2adfbf6173 100644 --- a/packages/worker/src/mcp/capabilities/openapi-provider/operation-request.ts +++ b/packages/worker/src/mcp/capabilities/openapi-provider/operation-request.ts @@ -4,6 +4,7 @@ import { parseIntegrationJson, type IntegrationConfig, } from '#mcp/capabilities/integrations/integration-shared.ts' +import { McpCallerError } from '#mcp/caller-error.ts' import { assertIntegrationHostAllowed } from '#mcp/execute-modules/integration-host-allowlist.ts' import { executeGatewayFetch } from '#mcp/fetch-gateway.ts' import { @@ -161,7 +162,8 @@ function buildOperationUrl(input: { let path = input.path.trim() path = path.replace(/\{([^}/]+)\}/g, (_match, name: string) => { if (!(name in input.params)) { - throw new Error( + // Caller omitted a path placeholder; clear from the message alone. + throw new McpCallerError( `Missing required path parameter "${name}" for OpenAPI operation path ${input.path}.`, ) } @@ -665,13 +667,15 @@ function asStringRecord( ): Record { if (value == null) return {} if (typeof value !== 'object' || Array.isArray(value)) { - throw new Error(`OpenAPI operation args.${fieldName} must be an object.`) + throw new McpCallerError( + `OpenAPI operation args.${fieldName} must be an object.`, + ) } const result: Record = {} for (const [key, entry] of Object.entries(value)) { if (entry == null) continue if (typeof entry !== 'string' && typeof entry !== 'number') { - throw new Error( + throw new McpCallerError( `OpenAPI operation args.${fieldName}.${key} must be a string or number.`, ) } diff --git a/packages/worker/src/mcp/capabilities/packages/delete-package.ts b/packages/worker/src/mcp/capabilities/packages/delete-package.ts index 528b1d48b1..61907d3b26 100644 --- a/packages/worker/src/mcp/capabilities/packages/delete-package.ts +++ b/packages/worker/src/mcp/capabilities/packages/delete-package.ts @@ -1,4 +1,5 @@ import { z } from 'zod' +import { McpCallerError } from '#mcp/caller-error.ts' import { defineDomainCapability } from '#mcp/capabilities/define-domain-capability.ts' import { capabilityDomainNames } from '#mcp/capabilities/domain-metadata.ts' import { requireMcpUser } from '#mcp/capabilities/meta/require-user.ts' @@ -43,7 +44,7 @@ export const deletePackageCapability = defineDomainCapability( packageId: args.package_id, }) if (!existing) { - throw new Error('Saved package not found for this user.') + throw new McpCallerError('Saved package not found for this user.') } await deleteSavedPackageProjection({ env: ctx.env, diff --git a/packages/worker/src/mcp/capabilities/packages/get-package.ts b/packages/worker/src/mcp/capabilities/packages/get-package.ts index 578141624f..e3e59d9e7b 100644 --- a/packages/worker/src/mcp/capabilities/packages/get-package.ts +++ b/packages/worker/src/mcp/capabilities/packages/get-package.ts @@ -1,4 +1,5 @@ import { z } from 'zod' +import { McpCallerError } from '#mcp/caller-error.ts' import { defineDomainCapability } from '#mcp/capabilities/define-domain-capability.ts' import { capabilityDomainNames } from '#mcp/capabilities/domain-metadata.ts' import { requireMcpUser } from '#mcp/capabilities/meta/require-user.ts' @@ -54,7 +55,7 @@ export const getPackageCapability = defineDomainCapability( packageId: args.package_id, }) if (!saved) { - throw new Error('Saved package not found for this user.') + throw new McpCallerError('Saved package not found for this user.') } const username = await resolvePublicUsername({ db: ctx.env.APP_DB, diff --git a/packages/worker/src/mcp/capabilities/packages/package-update.ts b/packages/worker/src/mcp/capabilities/packages/package-update.ts index 3bbe72f7bb..d3df8e56b0 100644 --- a/packages/worker/src/mcp/capabilities/packages/package-update.ts +++ b/packages/worker/src/mcp/capabilities/packages/package-update.ts @@ -1,4 +1,5 @@ import { z } from 'zod' +import { McpCallerError } from '#mcp/caller-error.ts' import { defineDomainCapability } from '#mcp/capabilities/define-domain-capability.ts' import { capabilityDomainNames } from '#mcp/capabilities/domain-metadata.ts' import { requireMcpUser } from '#mcp/capabilities/meta/require-user.ts' @@ -61,14 +62,14 @@ export const packageUpdateCapability = defineDomainCapability( hidden: args.changes.hidden, }) if (!changed) { - throw new Error('Saved package not found for this user.') + throw new McpCallerError('Saved package not found for this user.') } const savedPackage = await getSavedPackageById(ctx.env.APP_DB, { userId: owner.ownerUserId, packageId: args.package_id, }) if (!savedPackage) { - throw new Error('Saved package not found for this user.') + throw new McpCallerError('Saved package not found for this user.') } return { ok: true as const, diff --git a/packages/worker/src/mcp/capabilities/packages/resolve-package-source.ts b/packages/worker/src/mcp/capabilities/packages/resolve-package-source.ts index 9497c49bc6..9a334ce708 100644 --- a/packages/worker/src/mcp/capabilities/packages/resolve-package-source.ts +++ b/packages/worker/src/mcp/capabilities/packages/resolve-package-source.ts @@ -1,3 +1,4 @@ +import { McpCallerError } from '#mcp/caller-error.ts' import { getSavedPackageById, getSavedPackageByKodyId, @@ -15,7 +16,9 @@ function requireExactlyOnePackageSourceIdentity(input: PackageSourceIdentity) { (input.package_id !== undefined ? 1 : 0) + (input.kody_id !== undefined ? 1 : 0) if (count !== 1) { - throw new Error('Provide exactly one of `package_id` or `kody_id`.') + throw new McpCallerError( + 'Provide exactly one of `package_id` or `kody_id`.', + ) } } @@ -42,11 +45,11 @@ export async function resolveOwnedPackageSource(input: { }) if (!savedPackage) { const missingId = input.args.package_id ?? input.args.kody_id - throw new Error(`Saved package "${missingId}" was not found.`) + throw new McpCallerError(`Saved package "${missingId}" was not found.`) } const source = await getEntitySourceById(input.db, savedPackage.sourceId) if (!source || source.user_id !== input.userId) { - throw new Error('Repo source was not found for this user.') + throw new McpCallerError('Repo source was not found for this user.') } return { packageId: savedPackage.id, diff --git a/packages/worker/src/mcp/capabilities/repo/repo-open-session.ts b/packages/worker/src/mcp/capabilities/repo/repo-open-session.ts index ac8a0f1a4a..37ab2ece41 100644 --- a/packages/worker/src/mcp/capabilities/repo/repo-open-session.ts +++ b/packages/worker/src/mcp/capabilities/repo/repo-open-session.ts @@ -1,3 +1,4 @@ +import { McpCallerError } from '#mcp/caller-error.ts' import { defineDomainCapability } from '#mcp/capabilities/define-domain-capability.ts' import { capabilityDomainNames } from '#mcp/capabilities/domain-metadata.ts' import { type CapabilityContext } from '#mcp/capabilities/types.ts' @@ -25,7 +26,9 @@ export const repoOpenSessionCapability = defineDomainCapability( async handler(args, ctx: CapabilityContext) { const user = ctx.callerContext.user if (!user) { - throw new Error('repo_open_session requires an authenticated user.') + throw new McpCallerError( + 'repo_open_session requires an authenticated user.', + ) } const requested = await resolveRepoSourceReference({ @@ -42,7 +45,7 @@ export const repoOpenSessionCapability = defineDomainCapability( }) if (existingSession) { if (existingSession.source_id !== requested.source.id) { - throw new Error( + throw new McpCallerError( 'Active repo session does not match the requested source. Discard the current session before opening a new source.', ) } diff --git a/packages/worker/src/mcp/observability.node.test.ts b/packages/worker/src/mcp/observability.node.test.ts index a66236151b..79d59ca751 100644 --- a/packages/worker/src/mcp/observability.node.test.ts +++ b/packages/worker/src/mcp/observability.node.test.ts @@ -29,8 +29,9 @@ vi.mock('@sentry/cloudflare', () => ({ })) const { logMcpEvent } = await import('./observability.ts') +const { McpCallerError } = await import('./caller-error.ts') -test('logMcpEvent keeps sandbox failures on mcp-event logs and skips Sentry', () => { +function captureMcpEvents(run: () => void) { sentryMock.captureException.mockClear() sentryMock.captureMessage.mockClear() sentryMock.withScope.mockClear() @@ -44,6 +45,25 @@ test('logMcpEvent keeps sandbox failures on mcp-event logs and skips Sentry', () } }) as typeof console.info try { + run() + } finally { + console.info = originalInfo + } + return payloads +} + +const callerFailureBase = { + category: 'mcp', + tool: 'capability', + outcome: 'failure', + durationMs: 3, + baseUrl: 'https://example.com', + hasUser: true, + userId: 'user-1', +} as const + +test('logMcpEvent keeps sandbox failures on mcp-event logs and skips Sentry', () => { + const payloads = captureMcpEvents(() => { logMcpEvent({ category: 'mcp', tool: 'execute', @@ -61,24 +81,16 @@ test('logMcpEvent keeps sandbox failures on mcp-event logs and skips Sentry', () }) logMcpEvent({ - category: 'mcp', - tool: 'capability', + ...callerFailureBase, capabilityName: 'value_get', capabilitySource: 'builtin', - outcome: 'failure', - durationMs: 3, - baseUrl: 'https://example.com', - hasUser: true, - userId: 'user-1', sandboxError: false, failurePhase: 'handler', errorName: 'Error', errorMessage: 'platform handler blew up', cause: new Error('platform handler blew up'), }) - } finally { - console.info = originalInfo - } + }) expect(payloads).toHaveLength(2) expect(JSON.parse(payloads[0]!)).toMatchObject({ @@ -100,3 +112,69 @@ test('logMcpEvent keeps sandbox failures on mcp-event logs and skips Sentry', () expect(sentryMock.scope.setLevel).toHaveBeenCalledWith('error') expect(sentryMock.scope.setUser).toHaveBeenCalledWith({ id: 'user-1' }) }) + +test('logMcpEvent keeps caller mistakes out of Sentry', () => { + const payloads = captureMcpEvents(() => { + logMcpEvent({ + ...callerFailureBase, + capabilityName: 'search', + failurePhase: 'handler', + errorName: 'McpCallerError', + errorMessage: 'Provide "query" or "domain".', + cause: new McpCallerError('Provide "query" or "domain".'), + }) + + logMcpEvent({ + ...callerFailureBase, + capabilityName: 'repo_open_session', + failurePhase: 'handler', + errorName: 'Error', + errorMessage: 'Opening the session failed.', + cause: new Error('Opening the session failed.', { + cause: new McpCallerError('Discard the current session first.'), + }), + }) + + logMcpEvent({ + ...callerFailureBase, + capabilityName: 'value_get', + failurePhase: 'parse_input', + errorName: 'ZodError', + errorMessage: 'name: Required', + cause: new Error('name: Required'), + }) + + logMcpEvent({ + ...callerFailureBase, + tool: 'search', + toolName: 'search', + callerError: true, + errorName: 'EntityBatchError', + errorMessage: 'All entity lookups failed.', + }) + }) + + expect(payloads).toHaveLength(4) + expect(sentryMock.captureException).not.toHaveBeenCalled() + expect(sentryMock.captureMessage).not.toHaveBeenCalled() +}) + +test('logMcpEvent still reports platform failures that wrap no caller error', () => { + captureMcpEvents(() => { + logMcpEvent({ + ...callerFailureBase, + capabilityName: 'package_get', + failurePhase: 'handler', + errorName: 'Error', + errorMessage: 'D1 write failed.', + cause: new Error('D1 write failed.', { + cause: new Error('storage unavailable'), + }), + }) + }) + + expect(sentryMock.captureException).toHaveBeenCalledTimes(1) + expect(sentryMock.captureException).toHaveBeenCalledWith( + expect.objectContaining({ message: 'D1 write failed.' }), + ) +}) diff --git a/packages/worker/src/mcp/observability.ts b/packages/worker/src/mcp/observability.ts index 2d2e207e54..c33dee1afa 100644 --- a/packages/worker/src/mcp/observability.ts +++ b/packages/worker/src/mcp/observability.ts @@ -1,5 +1,6 @@ import * as Sentry from '@sentry/cloudflare' import { type McpCallerContext } from '@kody-internal/shared/chat.ts' +import { isMcpCallerError } from './caller-error.ts' export type McpToolKind = 'search' | 'execute' | 'capability' | 'app' @@ -20,6 +21,11 @@ export type McpObservabilityPayload = { userId?: string failurePhase?: McpFailurePhase sandboxError?: boolean + /** + * Set at failure sites that report a caller mistake without throwing an + * `McpCallerError` (batch lookups, early argument validation). + */ + callerError?: boolean registeredCapabilityCount?: number errorName?: string errorMessage?: string @@ -53,16 +59,27 @@ export function errorFields(error: unknown): { return { errorName: 'Unknown', errorMessage: String(error) } } +/** + * Failures the caller caused and can fix. They stay on the structured + * `mcp-event` log line; sending them to Sentry creates issues that look like + * platform bugs and trip triage automation. + */ +function isCallerFailure(payload: McpObservabilityPayload, cause?: unknown) { + // Sandbox failures come from caller-supplied module code (bad Notion + // filters, syntax errors, thrown strings). + if (payload.sandboxError) return true + // Arguments that never matched the declared schema never reached a handler. + if (payload.failurePhase === 'parse_input') return true + if (payload.callerError) return true + return isMcpCallerError(cause) +} + function reportMcpFailureToSentry( payload: McpObservabilityPayload, cause?: unknown, ) { try { - // Sandbox failures are caller/user-module errors (bad Notion filters, - // syntax errors, thrown strings). They stay on the structured - // `mcp-event` log line; sending them to Sentry creates warning issues - // that look like platform bugs and trip triage automation. - if (payload.sandboxError) return + if (isCallerFailure(payload, cause)) return if (!Sentry.isInitialized()) return const client = Sentry.getClient() if (!client?.getOptions().dsn) return diff --git a/packages/worker/src/mcp/tools/search-detail.ts b/packages/worker/src/mcp/tools/search-detail.ts index de0fbb330a..233858242d 100644 --- a/packages/worker/src/mcp/tools/search-detail.ts +++ b/packages/worker/src/mcp/tools/search-detail.ts @@ -1,4 +1,5 @@ import { buildPackageAppUrl } from '@kody-internal/shared/public-urls.ts' +import { McpCallerError } from '#mcp/caller-error.ts' import { parseIntegrationConfig, parseIntegrationJson, @@ -51,7 +52,7 @@ export async function resolveEntityDetail(input: { if (ref.type === 'capability') { const spec = input.searchRows.registry.capabilitySpecs[ref.id] if (!spec) { - throw new Error('Capability not found.') + throw new McpCallerError('Capability not found.') } const relatedOperations = collectRelatedCapabilityOperations({ spec, @@ -68,7 +69,9 @@ export async function resolveEntityDetail(input: { } if (!input.userId) { - throw new Error('Authentication required to access saved user entities.') + throw new McpCallerError( + 'Authentication required to access saved user entities.', + ) } if (ref.type === 'package') { @@ -82,7 +85,7 @@ export async function resolveEntityDetail(input: { kodyId: ref.id, })) if (!record) { - throw new Error('Saved package not found for this user.') + throw new McpCallerError('Saved package not found for this user.') } const loaded = await loadPackageSourceBySourceId({ env: input.agent.getEnv(), @@ -129,7 +132,7 @@ export async function resolveEntityDetail(input: { }, })) if (!row) { - throw new Error('Persisted value not found for this user.') + throw new McpCallerError('Persisted value not found for this user.') } return { type: 'value' as const, @@ -146,7 +149,7 @@ export async function resolveEntityDetail(input: { ref.id, ) if (!integration) { - throw new Error('Saved integration not found for this user.') + throw new McpCallerError('Saved integration not found for this user.') } const relatedPackageSuggestions = await collectIntegrationPackageSuggestions({ @@ -174,7 +177,7 @@ export async function resolveEntityDetail(input: { (secret) => secret.name === ref.id, ) if (!row) { - throw new Error('Secret not found for this user.') + throw new McpCallerError('Secret not found for this user.') } return { type: 'secret' as const, diff --git a/packages/worker/src/mcp/tools/search-tool-runner.ts b/packages/worker/src/mcp/tools/search-tool-runner.ts index e89e71c70f..accfc3dfec 100644 --- a/packages/worker/src/mcp/tools/search-tool-runner.ts +++ b/packages/worker/src/mcp/tools/search-tool-runner.ts @@ -1,5 +1,6 @@ import * as Sentry from '@sentry/cloudflare' import { resolvePublicUsername } from '#app/user-lookup.ts' +import { isMcpCallerError } from '#mcp/caller-error.ts' import { type McpRegistrationAgent } from '#mcp/mcp-registration-agent.ts' import { callerContextFields, @@ -78,6 +79,7 @@ export async function runSearchTool(input: { hasUser, userId: userId ?? undefined, sandboxError: false, + callerError: true, errorName: 'ValidationError', errorMessage: 'Provide "query", "entity", or "domain".', message: 'Search request missing query, entity, and domain.', @@ -189,6 +191,7 @@ export async function runSearchTool(input: { ok: false as const, entityRef, error: error.message, + callerError: isMcpCallerError(cause), } } }), @@ -233,6 +236,7 @@ export async function runSearchTool(input: { ok: false entityRef: string error: string + callerError: boolean } > } = await Sentry.startSpan( @@ -320,6 +324,18 @@ export async function runSearchTool(input: { markdownParts.push(entityResult.markdown) } const allFailed = successCount === 0 + const failedEntries = outcome.results.filter( + ( + entry, + ): entry is Extract<(typeof outcome.results)[number], { ok: false }> => + !entry.ok, + ) + // Only treat a total batch failure as caller-caused when every + // failed lookup was a caller mistake. Shared platform failures + // (DB/load) still reach Sentry. + const allCallerFailures = + failedEntries.length > 0 && + failedEntries.every((entry) => entry.callerError) logMcpEvent({ category: 'mcp', tool: 'search', @@ -332,6 +348,7 @@ export async function runSearchTool(input: { ...(allFailed ? { sandboxError: false, + ...(allCallerFailures ? { callerError: true } : {}), errorName: 'EntityBatchError', errorMessage: 'All entity lookups failed.', }