From 27c8aadb93035b70110f6deedf8dfba6d5930818 Mon Sep 17 00:00:00 2001 From: zho Date: Fri, 15 May 2026 07:13:44 +0800 Subject: [PATCH 1/6] added typed error response --- src/server/metadata-filter.ts | 73 +++++++++-- src/server/tool-error.ts | 119 +++++++++++++++++- src/server/tool-response.ts | 10 +- src/server/tools/count-tool.ts | 45 ++++--- src/server/tools/generate-urls-tool.ts | 9 +- src/server/tools/guided-query-tool.test.ts | 20 +-- src/server/tools/guided-query-tool.ts | 50 +++++--- src/server/tools/keyword-search-tool.ts | 47 +++---- src/server/tools/list-namespaces-tool.test.ts | 8 +- src/server/tools/list-namespaces-tool.ts | 10 +- src/server/tools/namespace-router-tool.ts | 13 +- src/server/tools/query-documents-tool.test.ts | 36 +++--- src/server/tools/query-documents-tool.ts | 31 +++-- src/server/tools/query-tool.test.ts | 34 ++--- src/server/tools/query-tool.ts | 37 +++--- .../tools/suggest-query-params-tool.test.ts | 15 +-- src/server/tools/suggest-query-params-tool.ts | 13 +- src/server/tools/test-helpers.ts | 8 ++ 18 files changed, 393 insertions(+), 185 deletions(-) diff --git a/src/server/metadata-filter.ts b/src/server/metadata-filter.ts index cfcf281..00d9ec5 100644 --- a/src/server/metadata-filter.ts +++ b/src/server/metadata-filter.ts @@ -27,6 +27,12 @@ const ALLOWED_FILTER_OPERATORS = new Set([ '$or', ]); +export type MetadataFilterValidationError = { + message: string; + /** Dot-path to the failing segment (metadata key and/or operator chain). */ + field: string; +}; + /** True if value is a string, number, or boolean (allowed for $eq, $gt, etc.). */ function isPrimitiveFilterValue(value: unknown): boolean { return typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean'; @@ -40,10 +46,17 @@ function isPrimitiveArray(value: unknown): boolean { ); } -/** Recursively validate a filter value; returns an error string or null if valid. */ -function validateMetadataFilterValue(value: unknown, path: string[]): string | null { +function err(message: string, path: string[]): MetadataFilterValidationError { + return { message, field: path.join('.') }; +} + +/** Recursively validate a filter value; returns an error or null if valid. */ +function validateMetadataFilterValue( + value: unknown, + path: string[] +): MetadataFilterValidationError | null { if (value === null || value === undefined) { - return `Invalid null/undefined at "${path.join('.')}".`; + return err(`Invalid null/undefined at "${path.join('.')}".`, path); } if (isPrimitiveFilterValue(value) || isPrimitiveArray(value)) { @@ -54,27 +67,39 @@ function validateMetadataFilterValue(value: unknown, path: string[]): string | n for (let i = 0; i < value.length; i++) { const item = value[i]; if (typeof item !== 'object' || item === null || Array.isArray(item)) { - return `Operator "${path[path.length - 1]}" at "${[...path, String(i)].join('.')}" must use an array of filter objects.`; + return err( + `Operator "${path[path.length - 1]}" at "${[...path, String(i)].join('.')}" must use an array of filter objects.`, + [...path, String(i)] + ); } - const nestedError = validateMetadataFilter(item as Record); + const nestedError = validateMetadataFilterRecord(item as Record); if (nestedError) return nestedError; } return null; } if (typeof value !== 'object') { - return `Unsupported filter value at "${path.join('.')}".`; + return err(`Unsupported filter value at "${path.join('.')}".`, path); } for (const [key, nestedValue] of Object.entries(value)) { if (!key.startsWith('$')) { - return `Nested metadata filters must use operator keys starting with "$" at "${path.join('.')}"; got "${key}".`; + return err( + `Nested metadata filters must use operator keys starting with "$" at "${path.join('.')}"; got "${key}".`, + [...path, key] + ); } if (!ALLOWED_FILTER_OPERATORS.has(key)) { - return `Unknown filter operator "${key}" at "${path.join('.')}". Allowed operators: ${[...ALLOWED_FILTER_OPERATORS].join(', ')}.`; + return err( + `Unknown filter operator "${key}" at "${path.join('.')}". Allowed operators: ${[...ALLOWED_FILTER_OPERATORS].join(', ')}.`, + [...path, key] + ); } if ((key === '$in' || key === '$nin') && !isPrimitiveArray(nestedValue)) { - return `Operator "${key}" at "${path.join('.')}" must use an array of primitive values.`; + return err( + `Operator "${key}" at "${path.join('.')}" must use an array of primitive values.`, + [...path, key] + ); } if ( (key === '$eq' || @@ -85,10 +110,16 @@ function validateMetadataFilterValue(value: unknown, path: string[]): string | n key === '$lte') && !isPrimitiveFilterValue(nestedValue) ) { - return `Operator "${key}" at "${path.join('.')}" must use a primitive value.`; + return err(`Operator "${key}" at "${path.join('.')}" must use a primitive value.`, [ + ...path, + key, + ]); } if ((key === '$and' || key === '$or') && !Array.isArray(nestedValue)) { - return `Operator "${key}" at "${path.join('.')}" must use an array of filter objects.`; + return err( + `Operator "${key}" at "${path.join('.')}" must use an array of filter objects.`, + [...path, key] + ); } const nestedError = validateMetadataFilterValue(nestedValue, [...path, key]); @@ -100,11 +131,27 @@ function validateMetadataFilterValue(value: unknown, path: string[]): string | n return null; } -/** Validate a Pinecone metadata filter object; returns an error message or null if valid. */ -export function validateMetadataFilter(filter: Record): string | null { +function validateMetadataFilterRecord( + filter: Record +): MetadataFilterValidationError | null { for (const [field, value] of Object.entries(filter)) { const error = validateMetadataFilterValue(value, [field]); if (error) return error; } return null; } + +/** + * Validate a Pinecone metadata filter object; returns structured error or null if valid. + */ +export function validateMetadataFilterDetailed( + filter: Record +): MetadataFilterValidationError | null { + return validateMetadataFilterRecord(filter); +} + +/** Validate a Pinecone metadata filter object; returns an error message or null if valid. */ +export function validateMetadataFilter(filter: Record): string | null { + const detailed = validateMetadataFilterDetailed(filter); + return detailed?.message ?? null; +} diff --git a/src/server/tool-error.ts b/src/server/tool-error.ts index ec2274a..5fc2e79 100644 --- a/src/server/tool-error.ts +++ b/src/server/tool-error.ts @@ -1,7 +1,9 @@ /** - * Shared error handling for MCP tools: consistent logging and user-facing messages. + * Shared error handling for MCP tools: consistent logging, user-facing messages, + * and typed {@link ToolError} payloads for MCP `isError` responses. */ +import { z } from 'zod'; import { getLogLevel, error as logError } from '../logger.js'; /** User-facing error message: detailed in DEBUG, generic otherwise. */ @@ -14,3 +16,118 @@ export function getToolErrorMessage(error: unknown, fallbackMessage: string): st export function logToolError(toolName: string, error: unknown): void { logError(`Error in ${toolName} tool`, error); } + +export const toolErrorCodeSchema = z.enum([ + 'FLOW_GATE', + 'VALIDATION', + 'PINECONE_ERROR', + 'TIMEOUT', +]); +export type ToolErrorCode = z.infer; + +const flowGateToolErrorSchema = z.object({ + code: z.literal('FLOW_GATE'), + message: z.string(), + recoverable: z.literal(true), + suggestion: z.string(), +}); + +const validationToolErrorSchema = z.object({ + code: z.literal('VALIDATION'), + message: z.string(), + recoverable: z.literal(true), + field: z.string(), + suggestion: z.string().optional(), +}); + +const pineconeToolErrorSchema = z.object({ + code: z.literal('PINECONE_ERROR'), + message: z.string(), + recoverable: z.boolean(), + suggestion: z.string().optional(), +}); + +const timeoutToolErrorSchema = z.object({ + code: z.literal('TIMEOUT'), + message: z.string(), + recoverable: z.literal(true), + suggestion: z.string().optional(), +}); + +export const toolErrorSchema = z.discriminatedUnion('code', [ + flowGateToolErrorSchema, + validationToolErrorSchema, + pineconeToolErrorSchema, + timeoutToolErrorSchema, +]); + +export type ToolError = z.infer; + +const DEFAULT_TIMEOUT_SUGGESTION = 'Retry the request, or increase --request-timeout-ms.'; + +/** Matches {@link withTimeout} rejection message prefix in `retry.ts`. */ +const TIMEOUT_MESSAGE_PATTERN = /^Timeout after \d+ms while waiting for /i; + +export function flowGateToolError(namespace: string, message: string): ToolError { + return { + code: 'FLOW_GATE', + message, + recoverable: true, + suggestion: `Call suggest_query_params for namespace '${namespace}' first`, + }; +} + +export function validationToolError( + message: string, + field: string, + options?: { suggestion?: string } +): ToolError { + return { + code: 'VALIDATION', + message, + recoverable: true, + field, + ...(options?.suggestion ? { suggestion: options.suggestion } : {}), + }; +} + +export function pineconeToolError( + message: string, + options?: { recoverable?: boolean; suggestion?: string } +): ToolError { + return { + code: 'PINECONE_ERROR', + message, + recoverable: options?.recoverable ?? false, + ...(options?.suggestion ? { suggestion: options.suggestion } : {}), + }; +} + +export function timeoutToolError( + message: string, + options?: { suggestion?: string } +): ToolError { + return { + code: 'TIMEOUT', + message, + recoverable: true, + suggestion: options?.suggestion ?? DEFAULT_TIMEOUT_SUGGESTION, + }; +} + +function rawErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +/** + * Map an unexpected thrown error to {@link ToolError} for MCP responses. + * Uses raw `Error#message` for timeout detection (DEBUG mode replaces the user message). + */ +export function classifyToolCatchError(error: unknown, fallbackMessage: string): ToolError { + const raw = rawErrorMessage(error); + const message = getToolErrorMessage(error, fallbackMessage); + if (TIMEOUT_MESSAGE_PATTERN.test(raw)) { + return timeoutToolError(message); + } + return pineconeToolError(message); +} diff --git a/src/server/tool-response.ts b/src/server/tool-response.ts index bf56f29..0c247a6 100644 --- a/src/server/tool-response.ts +++ b/src/server/tool-response.ts @@ -1,3 +1,6 @@ +import type { ToolError } from './tool-error.js'; +import { toolErrorSchema } from './tool-error.js'; + export type TextPayload = { content: Array<{ type: 'text'; text: string }>; isError?: boolean; @@ -15,14 +18,15 @@ export function jsonResponse(payload: unknown): TextPayload { }; } -/** Build an MCP tool error payload with JSON-stringified content and isError: true. */ -export function jsonErrorResponse(payload: unknown): TextPayload { +/** Build an MCP tool error payload with JSON-stringified {@link ToolError} and isError: true. */ +export function jsonErrorResponse(err: ToolError): TextPayload { + const validated = toolErrorSchema.parse(err); return { isError: true, content: [ { type: 'text', - text: JSON.stringify(payload, null, 2), + text: JSON.stringify(validated, null, 2), }, ], }; diff --git a/src/server/tools/count-tool.ts b/src/server/tools/count-tool.ts index 22c7182..7531abf 100644 --- a/src/server/tools/count-tool.ts +++ b/src/server/tools/count-tool.ts @@ -1,21 +1,24 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { z } from 'zod'; import { getPineconeClient } from '../client-context.js'; -import { metadataFilterSchema, validateMetadataFilter } from '../metadata-filter.js'; +import { metadataFilterSchema, validateMetadataFilterDetailed } from '../metadata-filter.js'; import { requireSuggested } from '../suggestion-flow.js'; -import { getToolErrorMessage, logToolError } from '../tool-error.js'; +import { + classifyToolCatchError, + flowGateToolError, + logToolError, + validationToolError, +} from '../tool-error.js'; import { jsonErrorResponse, jsonResponse } from '../tool-response.js'; const COUNT_RESPONSE_STATUS = 'success' as const; -type CountResponse = - | { - status: 'success'; - count: number; - truncated: boolean; - namespace: string; - metadata_filter?: Record; - } - | { status: 'error'; message: string }; +type CountResponse = { + status: 'success'; + count: number; + truncated: boolean; + namespace: string; + metadata_filter?: Record; +}; /** Register the count tool on the MCP server. */ export function registerCountTool(server: McpServer): void { @@ -51,21 +54,19 @@ export function registerCountTool(server: McpServer): void { try { const { namespace, query_text, metadata_filter } = params; if (!query_text.trim()) { - const response: CountResponse = { - status: 'error', - message: 'query_text cannot be empty', - }; - return jsonErrorResponse(response); + return jsonErrorResponse( + validationToolError('query_text cannot be empty', 'query_text') + ); } if (metadata_filter) { - const err = validateMetadataFilter(metadata_filter); + const err = validateMetadataFilterDetailed(metadata_filter); if (err) { - return jsonErrorResponse({ status: 'error', message: err }); + return jsonErrorResponse(validationToolError(err.message, err.field)); } } const flowCheck = requireSuggested(namespace); if (!flowCheck.ok) { - return jsonErrorResponse({ status: 'error', message: flowCheck.message }); + return jsonErrorResponse(flowGateToolError(namespace, flowCheck.message)); } const client = getPineconeClient(); const { count, truncated } = await client.count({ @@ -83,11 +84,7 @@ export function registerCountTool(server: McpServer): void { return jsonResponse(response); } catch (error) { logToolError('count', error); - const response: CountResponse = { - status: 'error', - message: getToolErrorMessage(error, 'Failed to get count'), - }; - return jsonErrorResponse(response); + return jsonErrorResponse(classifyToolCatchError(error, 'Failed to get count')); } } ); diff --git a/src/server/tools/generate-urls-tool.ts b/src/server/tools/generate-urls-tool.ts index ea3adc2..c85bde8 100644 --- a/src/server/tools/generate-urls-tool.ts +++ b/src/server/tools/generate-urls-tool.ts @@ -1,7 +1,7 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { z } from 'zod'; import { generateUrlForNamespace } from '../url-generation.js'; -import { getToolErrorMessage, logToolError } from '../tool-error.js'; +import { classifyToolCatchError, logToolError } from '../tool-error.js'; import { jsonErrorResponse, jsonResponse } from '../tool-response.js'; /** Get metadata from a record (either record.metadata or the record itself). */ @@ -58,10 +58,9 @@ export function registerGenerateUrlsTool(server: McpServer): void { }); } catch (error) { logToolError('generate_urls', error); - return jsonErrorResponse({ - status: 'error', - message: getToolErrorMessage(error, 'Failed to generate URLs'), - }); + return jsonErrorResponse( + classifyToolCatchError(error, 'Failed to generate URLs') + ); } } ); diff --git a/src/server/tools/guided-query-tool.test.ts b/src/server/tools/guided-query-tool.test.ts index 05dcd08..7cd9127 100644 --- a/src/server/tools/guided-query-tool.test.ts +++ b/src/server/tools/guided-query-tool.test.ts @@ -7,6 +7,7 @@ import { makeNamespaceCacheEntry, makeSearchResult, parseToolJson, + assertToolError, } from './test-helpers.js'; vi.mock('../client-context.js', () => ({ @@ -111,7 +112,10 @@ describe('guided_query tool handler', () => { }); expect((raw as { isError?: boolean }).isError).toBe(true); - expect(parseToolJson(raw).message).toBe('user_query cannot be empty'); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('user_query'); + expect(err.message).toBe('user_query cannot be empty'); }); it('returns error when no namespace can be resolved', async () => { @@ -124,13 +128,13 @@ describe('guided_query tool handler', () => { const server = createMockServer(); registerGuidedQueryTool(server as never); - const body = parseToolJson( - await server.getHandler('guided_query')!({ - user_query: 'hello world', - }) - ); + const raw = await server.getHandler('guided_query')!({ + user_query: 'hello world', + }); - expect(body.status).toBe('error'); - expect(String(body.message)).toContain('No namespace available'); + const err = assertToolError(raw); + expect(err.code).toBe('PINECONE_ERROR'); + expect(err.recoverable).toBe(true); + expect(err.message).toContain('No namespace available'); }); }); diff --git a/src/server/tools/guided-query-tool.ts b/src/server/tools/guided-query-tool.ts index fa31a3e..220cde4 100644 --- a/src/server/tools/guided-query-tool.ts +++ b/src/server/tools/guided-query-tool.ts @@ -4,12 +4,17 @@ import { FAST_QUERY_FIELDS, MAX_TOP_K, MIN_TOP_K } from '../../constants.js'; import type { QueryResponse } from '../../types.js'; import { getPineconeClient } from '../client-context.js'; import { formatQueryResultRows } from '../format-query-result.js'; -import { metadataFilterSchema, validateMetadataFilter } from '../metadata-filter.js'; +import { metadataFilterSchema, validateMetadataFilterDetailed } from '../metadata-filter.js'; import { rankNamespacesByQuery } from '../namespace-router.js'; import { getNamespacesWithCache } from '../namespaces-cache.js'; import { suggestQueryParams } from '../query-suggestion.js'; import { markSuggested } from '../suggestion-flow.js'; -import { getToolErrorMessage, logToolError } from '../tool-error.js'; +import { + classifyToolCatchError, + logToolError, + pineconeToolError, + validationToolError, +} from '../tool-error.js'; import { jsonErrorResponse, jsonResponse } from '../tool-response.js'; type GuidedToolName = 'count' | 'fast' | 'detailed' | 'full'; @@ -78,13 +83,15 @@ export function registerGuidedQueryTool(server: McpServer): void { } = params; if (!user_query?.trim()) { - return jsonErrorResponse({ status: 'error', message: 'user_query cannot be empty' }); + return jsonErrorResponse( + validationToolError('user_query cannot be empty', 'user_query') + ); } if (metadata_filter) { - const err = validateMetadataFilter(metadata_filter); + const err = validateMetadataFilterDetailed(metadata_filter); if (err) { - return jsonErrorResponse({ status: 'error', message: err }); + return jsonErrorResponse(validationToolError(err.message, err.field)); } } @@ -93,20 +100,30 @@ export function registerGuidedQueryTool(server: McpServer): void { const ranked = rankNamespacesByQuery(queryText, namespaces, 3); const namespace = inputNamespace ?? ranked[0]?.namespace; + /* + * ToolError mapping: empty index / no routable namespace is backend/data state + * (PINECONE_ERROR, recoverable). Explicit namespace missing from cache is user/input + * (VALIDATION, field namespace). + */ if (!namespace) { - return jsonErrorResponse({ - status: 'error', - message: 'No namespace available. Please run list_namespaces and verify index data.', - }); + return jsonErrorResponse( + pineconeToolError('No namespace available. Please run list_namespaces and verify index data.', { + recoverable: true, + suggestion: 'Call list_namespaces to confirm the index has namespaces, then retry.', + }) + ); } const ns = namespaces.find((n) => n.namespace === namespace); const suggestion = suggestQueryParams(ns?.metadata ?? null, queryText); if (!suggestion.namespace_found) { - return jsonErrorResponse({ - status: 'error', - message: `Namespace "${namespace}" not found in cached namespaces. Call list_namespaces and retry.`, - }); + return jsonErrorResponse( + validationToolError( + `Namespace "${namespace}" not found in cached namespaces. Call list_namespaces and retry.`, + 'namespace', + { suggestion: 'Use a namespace name returned by list_namespaces, then call list_namespaces again if the cache is stale.' } + ) + ); } const selectedTool: GuidedToolName = resolveGuidedToolName(preferred_tool, suggestion); @@ -192,10 +209,9 @@ export function registerGuidedQueryTool(server: McpServer): void { }); } catch (error) { logToolError('guided_query', error); - return jsonErrorResponse({ - status: 'error', - message: getToolErrorMessage(error, 'Failed to execute guided query'), - }); + return jsonErrorResponse( + classifyToolCatchError(error, 'Failed to execute guided query') + ); } } ); diff --git a/src/server/tools/keyword-search-tool.ts b/src/server/tools/keyword-search-tool.ts index 88cfc04..7931e06 100644 --- a/src/server/tools/keyword-search-tool.ts +++ b/src/server/tools/keyword-search-tool.ts @@ -3,13 +3,14 @@ import { z } from 'zod'; import { MAX_TOP_K, MIN_TOP_K } from '../../constants.js'; import { getPineconeClient } from '../client-context.js'; import { formatQueryResultRows } from '../format-query-result.js'; -import { metadataFilterSchema, validateMetadataFilter } from '../metadata-filter.js'; -import { getToolErrorMessage, logToolError } from '../tool-error.js'; +import { metadataFilterSchema, validateMetadataFilterDetailed } from '../metadata-filter.js'; +import type { ToolError } from '../tool-error.js'; +import { classifyToolCatchError, logToolError, validationToolError } from '../tool-error.js'; import { jsonErrorResponse, jsonResponse } from '../tool-response.js'; -/** Response shape for keyword_search: same as query tool for consistency. */ +/** Success response shape for keyword_search (aligned with query tool fields). */ export interface KeywordSearchResponse { - status: 'success' | 'error'; + status: 'success'; query?: string; namespace?: string; index?: string; @@ -29,16 +30,19 @@ export interface KeywordSearchResponse { metadata?: Record; }>; fields?: string[]; - message?: string; } +type KeywordSearchExecResult = + | { ok: true; body: KeywordSearchResponse } + | { ok: false; error: ToolError }; + async function executeKeywordSearch(params: { query_text: string; namespace: string; top_k: number; metadata_filter?: Record; fields?: string[]; -}): Promise { +}): Promise { const { query_text, namespace, top_k, metadata_filter, fields } = params; const normalizedQuery = query_text.trim(); @@ -46,22 +50,25 @@ async function executeKeywordSearch(params: { if (!normalizedQuery) { return { - status: 'error', - message: 'Query text cannot be empty', + ok: false, + error: validationToolError('Query text cannot be empty', 'query_text'), }; } if (!normalizedNamespace) { return { - status: 'error', - message: 'Namespace cannot be empty', + ok: false, + error: validationToolError('Namespace cannot be empty', 'namespace'), }; } if (metadata_filter) { - const filterError = validateMetadataFilter(metadata_filter); + const filterError = validateMetadataFilterDetailed(metadata_filter); if (filterError) { - return { status: 'error', message: filterError }; + return { + ok: false, + error: validationToolError(filterError.message, filterError.field), + }; } } @@ -90,7 +97,7 @@ async function executeKeywordSearch(params: { if (fields?.length) { response.fields = fields; } - return response; + return { ok: true, body: response }; } /** Register the keyword_search tool on the MCP server. */ @@ -127,24 +134,20 @@ export function registerKeywordSearchTool(server: McpServer): void { }, async (params) => { try { - const response = await executeKeywordSearch({ + const result = await executeKeywordSearch({ query_text: params.query_text, namespace: params.namespace, top_k: params.top_k, metadata_filter: params.metadata_filter, fields: params.fields, }); - if (response.status === 'error') { - return jsonErrorResponse(response); + if (!result.ok) { + return jsonErrorResponse(result.error); } - return jsonResponse(response); + return jsonResponse(result.body); } catch (error) { logToolError('keyword_search', error); - const response: KeywordSearchResponse = { - status: 'error', - message: getToolErrorMessage(error, 'Keyword search failed'), - }; - return jsonErrorResponse(response); + return jsonErrorResponse(classifyToolCatchError(error, 'Keyword search failed')); } } ); diff --git a/src/server/tools/list-namespaces-tool.test.ts b/src/server/tools/list-namespaces-tool.test.ts index 639f8eb..6aef8f7 100644 --- a/src/server/tools/list-namespaces-tool.test.ts +++ b/src/server/tools/list-namespaces-tool.test.ts @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { getNamespacesWithCache } from '../namespaces-cache.js'; import { registerListNamespacesTool } from './list-namespaces-tool.js'; -import { createMockServer, parseToolJson } from './test-helpers.js'; +import { createMockServer, parseToolJson, assertToolError } from './test-helpers.js'; vi.mock('../namespaces-cache.js', () => ({ getNamespacesWithCache: vi.fn(), @@ -65,8 +65,8 @@ describe('list_namespaces tool handler', () => { const payload = raw as { isError?: boolean }; expect(payload.isError).toBe(true); - const body = parseToolJson(raw); - expect(body.status).toBe('error'); - expect(String(body.message)).toBe('Failed to list namespaces'); + const err = assertToolError(raw); + expect(err.code).toBe('PINECONE_ERROR'); + expect(err.message).toBe('Failed to list namespaces'); }); }); diff --git a/src/server/tools/list-namespaces-tool.ts b/src/server/tools/list-namespaces-tool.ts index 153ef1c..5817e21 100644 --- a/src/server/tools/list-namespaces-tool.ts +++ b/src/server/tools/list-namespaces-tool.ts @@ -1,6 +1,6 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { getNamespacesWithCache } from '../namespaces-cache.js'; -import { getToolErrorMessage, logToolError } from '../tool-error.js'; +import { classifyToolCatchError, logToolError } from '../tool-error.js'; import { jsonErrorResponse, jsonResponse } from '../tool-response.js'; /** Register the list_namespaces tool on the MCP server. */ @@ -37,11 +37,9 @@ export function registerListNamespacesTool(server: McpServer): void { return jsonResponse(response); } catch (error) { logToolError('list_namespaces', error); - const response = { - status: 'error', - message: getToolErrorMessage(error, 'Failed to list namespaces'), - }; - return jsonErrorResponse(response); + return jsonErrorResponse( + classifyToolCatchError(error, 'Failed to list namespaces') + ); } } ); diff --git a/src/server/tools/namespace-router-tool.ts b/src/server/tools/namespace-router-tool.ts index 02e70d9..4f7be61 100644 --- a/src/server/tools/namespace-router-tool.ts +++ b/src/server/tools/namespace-router-tool.ts @@ -2,7 +2,7 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { z } from 'zod'; import { getNamespacesWithCache } from '../namespaces-cache.js'; import { rankNamespacesByQuery } from '../namespace-router.js'; -import { getToolErrorMessage, logToolError } from '../tool-error.js'; +import { classifyToolCatchError, logToolError, validationToolError } from '../tool-error.js'; import { jsonErrorResponse, jsonResponse } from '../tool-response.js'; /** Register the namespace_router tool on the MCP server. */ @@ -30,7 +30,9 @@ export function registerNamespaceRouterTool(server: McpServer): void { try { const { user_query, top_n } = params; if (!user_query?.trim()) { - return jsonErrorResponse({ status: 'error', message: 'user_query cannot be empty' }); + return jsonErrorResponse( + validationToolError('user_query cannot be empty', 'user_query') + ); } const { data, cache_hit } = await getNamespacesWithCache(); const ranked = rankNamespacesByQuery(user_query.trim(), data, top_n); @@ -45,10 +47,9 @@ export function registerNamespaceRouterTool(server: McpServer): void { return jsonResponse(response); } catch (error) { logToolError('namespace_router', error); - return jsonErrorResponse({ - status: 'error', - message: getToolErrorMessage(error, 'Failed to route namespace'), - }); + return jsonErrorResponse( + classifyToolCatchError(error, 'Failed to route namespace') + ); } } ); diff --git a/src/server/tools/query-documents-tool.test.ts b/src/server/tools/query-documents-tool.test.ts index c68b452..3fad3e2 100644 --- a/src/server/tools/query-documents-tool.test.ts +++ b/src/server/tools/query-documents-tool.test.ts @@ -4,7 +4,7 @@ import { getPineconeClient } from '../client-context.js'; import { reassembleByDocument } from '../reassemble-documents.js'; import * as suggestionFlow from '../suggestion-flow.js'; import { registerQueryDocumentsTool } from './query-documents-tool.js'; -import { createMockServer, makeSearchResult, parseToolJson } from './test-helpers.js'; +import { createMockServer, makeSearchResult, parseToolJson, assertToolError } from './test-helpers.js'; vi.mock('../client-context.js', () => ({ getPineconeClient: vi.fn(), @@ -90,7 +90,10 @@ describe('query_documents tool handler', () => { expect((raw as { isError?: boolean }).isError).toBe(true); expect(query).not.toHaveBeenCalled(); - expect(parseToolJson(raw).message).toBe('query_text cannot be empty'); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('query_text'); + expect(err.message).toBe('query_text cannot be empty'); }); it('returns flow error when suggest_query_params gate fails', async () => { @@ -104,14 +107,14 @@ describe('query_documents tool handler', () => { registerQueryDocumentsTool(server as never); const query = mockedGetClient().query as ReturnType; - const body = parseToolJson( - await server.getHandler('query_documents')!({ - query_text: 'ok', - namespace: 'wg21', - }) - ); + const raw = await server.getHandler('query_documents')!({ + query_text: 'ok', + namespace: 'wg21', + }); - expect(body.status).toBe('error'); + const err = assertToolError(raw); + expect(err.code).toBe('FLOW_GATE'); + expect(err.suggestion).toBe("Call suggest_query_params for namespace 'wg21' first"); expect(query).not.toHaveBeenCalled(); }); @@ -125,13 +128,14 @@ describe('query_documents tool handler', () => { const server = createMockServer(); registerQueryDocumentsTool(server as never); - const body = parseToolJson( - await server.getHandler('query_documents')!({ - query_text: 'ok', - namespace: 'wg21', - }) - ); + const raw = await server.getHandler('query_documents')!({ + query_text: 'ok', + namespace: 'wg21', + }); - expect(body.message).toContain('expired'); + const err = assertToolError(raw); + expect(err.code).toBe('FLOW_GATE'); + expect(err.message).toContain('expired'); + expect(err.suggestion).toBe("Call suggest_query_params for namespace 'wg21' first"); }); }); diff --git a/src/server/tools/query-documents-tool.ts b/src/server/tools/query-documents-tool.ts index 61abe76..fe268f8 100644 --- a/src/server/tools/query-documents-tool.ts +++ b/src/server/tools/query-documents-tool.ts @@ -6,10 +6,15 @@ import { QUERY_DOCUMENTS_MAX_CHUNKS, } from '../../constants.js'; import { getPineconeClient } from '../client-context.js'; -import { metadataFilterSchema, validateMetadataFilter } from '../metadata-filter.js'; +import { metadataFilterSchema, validateMetadataFilterDetailed } from '../metadata-filter.js'; import { reassembleByDocument } from '../reassemble-documents.js'; import { requireSuggested } from '../suggestion-flow.js'; -import { getToolErrorMessage, logToolError } from '../tool-error.js'; +import { + classifyToolCatchError, + flowGateToolError, + logToolError, + validationToolError, +} from '../tool-error.js'; import { jsonErrorResponse, jsonResponse } from '../tool-response.js'; /** @@ -73,20 +78,21 @@ export function registerQueryDocumentsTool(server: McpServer): void { } = params; if (!query_text?.trim()) { - return jsonErrorResponse({ - status: 'error', - message: 'query_text cannot be empty', - }); + return jsonErrorResponse( + validationToolError('query_text cannot be empty', 'query_text') + ); } if (metadata_filter) { - const err = validateMetadataFilter(metadata_filter); - if (err) return jsonErrorResponse({ status: 'error', message: err }); + const err = validateMetadataFilterDetailed(metadata_filter); + if (err) { + return jsonErrorResponse(validationToolError(err.message, err.field)); + } } const flowCheck = requireSuggested(namespace); if (!flowCheck.ok) { - return jsonErrorResponse({ status: 'error', message: flowCheck.message }); + return jsonErrorResponse(flowGateToolError(namespace, flowCheck.message)); } const chunkLimit = Math.min(QUERY_DOCUMENTS_MAX_CHUNKS, top_k * CHUNKS_PER_DOCUMENT); @@ -124,10 +130,9 @@ export function registerQueryDocumentsTool(server: McpServer): void { }); } catch (error) { logToolError('query_documents', error); - return jsonErrorResponse({ - status: 'error', - message: getToolErrorMessage(error, 'Failed to query and reassemble documents'), - }); + return jsonErrorResponse( + classifyToolCatchError(error, 'Failed to query and reassemble documents') + ); } } ); diff --git a/src/server/tools/query-tool.test.ts b/src/server/tools/query-tool.test.ts index fe40079..f2af9d2 100644 --- a/src/server/tools/query-tool.test.ts +++ b/src/server/tools/query-tool.test.ts @@ -3,7 +3,7 @@ import { FAST_QUERY_FIELDS } from '../../constants.js'; import { getPineconeClient } from '../client-context.js'; import * as suggestionFlow from '../suggestion-flow.js'; import { registerQueryTool } from './query-tool.js'; -import { createMockServer, makeSearchResult, parseToolJson } from './test-helpers.js'; +import { createMockServer, makeSearchResult, parseToolJson, assertToolError } from './test-helpers.js'; vi.mock('../client-context.js', () => ({ getPineconeClient: vi.fn(), @@ -102,8 +102,10 @@ describe('query tool handler (preset-driven)', () => { expect((raw as { isError?: boolean }).isError).toBe(true); expect(query).not.toHaveBeenCalled(); - const body = parseToolJson(raw); - expect(body.message).toBe('Query text cannot be empty'); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('query_text'); + expect(err.message).toBe('Query text cannot be empty'); }); it('query: returns flow error when suggest_query_params was not called first', async () => { @@ -126,8 +128,10 @@ describe('query tool handler (preset-driven)', () => { expect((raw as { isError?: boolean }).isError).toBe(true); expect(query).not.toHaveBeenCalled(); - const body = parseToolJson(raw); - expect(body.message).toBe( + const err = assertToolError(raw); + expect(err.code).toBe('FLOW_GATE'); + expect(err.suggestion).toBe("Call suggest_query_params for namespace 'wg21' first"); + expect(err.message).toBe( 'Flow requires suggest_query_params first. Call suggest_query_params with namespace and user_query before query/count tools.' ); }); @@ -143,17 +147,17 @@ describe('query tool handler (preset-driven)', () => { const server = createMockServer(); registerQueryTool(server as never); - const body = parseToolJson( - await server.getHandler('query')!({ - query_text: 'hello', - namespace: 'wg21', - top_k: 10, - preset: 'full', - }) - ); + const raw = await server.getHandler('query')!({ + query_text: 'hello', + namespace: 'wg21', + top_k: 10, + preset: 'full', + }); - expect(body.status).toBe('error'); - expect(body.message).toBe(expiredMsg); + const err = assertToolError(raw); + expect(err.code).toBe('FLOW_GATE'); + expect(err.message).toBe(expiredMsg); + expect(err.suggestion).toBe("Call suggest_query_params for namespace 'wg21' first"); }); it('query: surfaces unreranked hits when client returns reranked:false (rerank fallback shape)', async () => { diff --git a/src/server/tools/query-tool.ts b/src/server/tools/query-tool.ts index d83276c..7a35b72 100644 --- a/src/server/tools/query-tool.ts +++ b/src/server/tools/query-tool.ts @@ -4,9 +4,14 @@ import { FAST_QUERY_FIELDS, MAX_TOP_K, MIN_TOP_K } from '../../constants.js'; import type { QueryResponse } from '../../types.js'; import { getPineconeClient } from '../client-context.js'; import { formatQueryResultRows } from '../format-query-result.js'; -import { metadataFilterSchema, validateMetadataFilter } from '../metadata-filter.js'; +import { metadataFilterSchema, validateMetadataFilterDetailed } from '../metadata-filter.js'; import { requireSuggested } from '../suggestion-flow.js'; -import { getToolErrorMessage, logToolError } from '../tool-error.js'; +import { + classifyToolCatchError, + flowGateToolError, + logToolError, + validationToolError, +} from '../tool-error.js'; import { jsonErrorResponse, jsonResponse } from '../tool-response.js'; type QueryMode = 'query' | 'query_fast' | 'query_detailed'; @@ -26,27 +31,23 @@ async function executeQuery(params: QueryExecParams) { const { query_text, namespace, top_k, use_reranking, metadata_filter, fields, mode } = params; try { if (!query_text.trim()) { - const response: QueryResponse = { - status: 'error', - message: 'Query text cannot be empty', - }; - return jsonErrorResponse(response); + return jsonErrorResponse( + validationToolError('Query text cannot be empty', 'query_text') + ); } if (metadata_filter) { - const filterValidationError = validateMetadataFilter(metadata_filter); + const filterValidationError = validateMetadataFilterDetailed(metadata_filter); if (filterValidationError) { - const response: QueryResponse = { - status: 'error', - message: filterValidationError, - }; - return jsonErrorResponse(response); + return jsonErrorResponse( + validationToolError(filterValidationError.message, filterValidationError.field) + ); } } const flowCheck = requireSuggested(namespace); if (!flowCheck.ok) { - return jsonErrorResponse({ status: 'error', message: flowCheck.message }); + return jsonErrorResponse(flowGateToolError(namespace, flowCheck.message)); } const client = getPineconeClient(); @@ -74,11 +75,9 @@ async function executeQuery(params: QueryExecParams) { return jsonResponse(response); } catch (error) { logToolError(mode, error); - const response: QueryResponse = { - status: 'error', - message: getToolErrorMessage(error, 'An error occurred while processing your query'), - }; - return jsonErrorResponse(response); + return jsonErrorResponse( + classifyToolCatchError(error, 'An error occurred while processing your query') + ); } } diff --git a/src/server/tools/suggest-query-params-tool.test.ts b/src/server/tools/suggest-query-params-tool.test.ts index 3c6e317..4e16179 100644 --- a/src/server/tools/suggest-query-params-tool.test.ts +++ b/src/server/tools/suggest-query-params-tool.test.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { getNamespacesWithCache } from '../namespaces-cache.js'; import { markSuggested } from '../suggestion-flow.js'; import { registerSuggestQueryParamsTool } from './suggest-query-params-tool.js'; -import { createMockServer, makeNamespaceCacheEntry, parseToolJson } from './test-helpers.js'; +import { createMockServer, makeNamespaceCacheEntry, parseToolJson, assertToolError } from './test-helpers.js'; vi.mock('../namespaces-cache.js', () => ({ getNamespacesWithCache: vi.fn(), @@ -83,9 +83,10 @@ describe('suggest_query_params tool handler', () => { }); expect((raw as { isError?: boolean }).isError).toBe(true); - const body = parseToolJson(raw); - expect(body.status).toBe('error'); - expect(body.message).toBe('user_query cannot be empty'); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('user_query'); + expect(err.message).toBe('user_query cannot be empty'); }); it('returns error when namespace cache fails', async () => { @@ -99,8 +100,8 @@ describe('suggest_query_params tool handler', () => { }); expect((raw as { isError?: boolean }).isError).toBe(true); - const body = parseToolJson(raw); - expect(body.status).toBe('error'); - expect(String(body.message)).toBe('Failed to suggest query params'); + const err = assertToolError(raw); + expect(err.code).toBe('PINECONE_ERROR'); + expect(err.message).toBe('Failed to suggest query params'); }); }); diff --git a/src/server/tools/suggest-query-params-tool.ts b/src/server/tools/suggest-query-params-tool.ts index 515dae5..bad2a9e 100644 --- a/src/server/tools/suggest-query-params-tool.ts +++ b/src/server/tools/suggest-query-params-tool.ts @@ -3,7 +3,7 @@ import { z } from 'zod'; import { getNamespacesWithCache } from '../namespaces-cache.js'; import { suggestQueryParams } from '../query-suggestion.js'; import { markSuggested } from '../suggestion-flow.js'; -import { getToolErrorMessage, logToolError } from '../tool-error.js'; +import { classifyToolCatchError, logToolError, validationToolError } from '../tool-error.js'; import { jsonErrorResponse, jsonResponse } from '../tool-response.js'; /** Register the suggest_query_params tool on the MCP server. */ @@ -33,7 +33,9 @@ export function registerSuggestQueryParamsTool(server: McpServer): void { try { const { namespace, user_query } = params; if (!user_query?.trim()) { - return jsonErrorResponse({ status: 'error', message: 'user_query cannot be empty' }); + return jsonErrorResponse( + validationToolError('user_query cannot be empty', 'user_query') + ); } const { data: namespacesInfo, cache_hit } = await getNamespacesWithCache(); const ns = namespacesInfo.find((n) => n.namespace === namespace); @@ -54,10 +56,9 @@ export function registerSuggestQueryParamsTool(server: McpServer): void { return jsonResponse(response); } catch (error) { logToolError('suggest_query_params', error); - return jsonErrorResponse({ - status: 'error', - message: getToolErrorMessage(error, 'Failed to suggest query params'), - }); + return jsonErrorResponse( + classifyToolCatchError(error, 'Failed to suggest query params') + ); } } ); diff --git a/src/server/tools/test-helpers.ts b/src/server/tools/test-helpers.ts index f0db291..b5b6519 100644 --- a/src/server/tools/test-helpers.ts +++ b/src/server/tools/test-helpers.ts @@ -1,4 +1,6 @@ import type { SearchResult } from '../../types.js'; +import type { ToolError } from '../tool-error.js'; +import { toolErrorSchema } from '../tool-error.js'; /** Handler invoked by MCP tool registration (params shape varies by tool). */ export type ToolHandler = (params: Record) => Promise; @@ -38,6 +40,12 @@ export function parseToolJson(payload: unknown): Record { return JSON.parse(text) as Record; } +/** Parse MCP tool error JSON from a handler return value (expects `isError: true`). */ +export function assertToolError(payload: unknown): ToolError { + const raw = parseToolJson(payload); + return toolErrorSchema.parse(raw); +} + export function makeSearchResult(overrides?: Partial): SearchResult { return { id: 'hit-1', From b02da008b9e86de9f0ab96bddef8dd8701632568 Mon Sep 17 00:00:00 2001 From: zho Date: Fri, 15 May 2026 07:14:06 +0800 Subject: [PATCH 2/6] add a test --- CHANGELOG.md | 4 +++ README.md | 14 ++++++++++ src/server.ts | 7 +++++ src/server/tool-error.test.ts | 50 +++++++++++++++++++++++++++++++++++ src/types.ts | 3 +-- 5 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 src/server/tool-error.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index fade38e..98912f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ Tagged releases are published to npm from GitHub Actions when a **GitHub Release ### Added +- Zod `toolErrorSchema` and exported types `ToolError` / `ToolErrorCode` for parsing MCP tool failures; all tools now return this JSON shape in the text content when `isError` is true. +- `validateMetadataFilterDetailed()` returns `{ message, field }` for invalid filters; `validateMetadataFilter()` remains a string-only wrapper for backward compatibility. - `.coderabbit.yaml` sets the pre-merge **docstring coverage** threshold to **79%** (default **80%**) so marginal documentation-only gaps do not block merges; adjust upward as coverage improves. - `registerBuiltinUrlGenerators()` for built-in URL generators; `setupServer()` invokes it so CLI/library parity stays default. - Discriminated result type for `listNamespacesFromKeywordIndex()` (`KeywordIndexNamespacesResult`). @@ -24,6 +26,8 @@ Tagged releases are published to npm from GitHub Actions when a **GitHub Release ### Changed +- **Breaking (MCP):** Tool error bodies no longer use `{ status: 'error', message }`. Failures are typed `ToolError` objects: `code` (`FLOW_GATE` | `VALIDATION` | `PINECONE_ERROR` | `TIMEOUT`), `message`, `recoverable`, optional `suggestion`, and optional `field` (required for `VALIDATION`). The outer MCP result still sets `isError: true`. +- **Breaking (types):** `QueryResponse` and exported `KeywordSearchResponse` no longer include `status: 'error'` / error-only fields; errors use `ToolError` only. - **Breaking (MCP):** `suggest_query_params` and in-process suggestion flow now emit `recommended_tool` as `count` | `fast` | `detailed` | `full` (aligned with the unified `query` tool `preset`), not legacy `query_fast` / `query_detailed` strings. - **Breaking (MCP):** Single hybrid `query` tool with `preset` (`fast` | `detailed` | `full`); removed separate `query_fast` / `query_detailed` tool registrations. - `resolveConfig()` throws if the Pinecone API key is missing (after trim); library callers must supply `apiKey` via overrides or set `PINECONE_API_KEY`. diff --git a/README.md b/README.md index 33a4947..d87ad6e 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,20 @@ A Model Context Protocol (MCP) server that provides semantic search over Pinecon | [CONTRIBUTING.md](CONTRIBUTING.md) | How to contribute | | [SECURITY.md](SECURITY.md) | Vulnerability reporting | +## Error responses + +When a tool fails, the MCP tool result sets **`isError: true`**. The `text` content is JSON matching **`ToolError`** (parse with `toolErrorSchema` from `@will-cppa/pinecone-read-only-mcp`). + +| Field | Description | +| ----- | ----------- | +| `code` | `FLOW_GATE` — `suggest_query_params` was not run for this namespace (or context expired). `VALIDATION` — bad input or metadata filter. `PINECONE_ERROR` — SDK / network / server failure. `TIMEOUT` — outbound Pinecone call exceeded `--request-timeout-ms`. | +| `message` | Human-readable detail (`DEBUG` log level may surface raw SDK messages in the message for `PINECONE_ERROR` / `TIMEOUT`). | +| `recoverable` | Whether the client can plausibly fix the issue and retry (`true` for flow gate, validation, timeouts; typically `false` for generic Pinecone errors). | +| `suggestion` | Optional hint. **`FLOW_GATE`** always includes: `Call suggest_query_params for namespace '' first`. **`TIMEOUT`** suggests retrying or increasing the request timeout. | +| `field` | **Required when `code` is `VALIDATION`:** the input parameter name (e.g. `query_text`, `namespace`) or a dot-path into `metadata_filter` (e.g. `author.$in`). | + +Success payloads are unchanged and do **not** wrap `ToolError`. Clients that still expect `{ "status": "error", "message": "..." }` must migrate to the shape above. + ## Features - **Hybrid Search**: Combines dense and sparse embeddings for superior recall diff --git a/src/server.ts b/src/server.ts index ca24c86..7badaac 100644 --- a/src/server.ts +++ b/src/server.ts @@ -10,6 +10,7 @@ * - {@link resolveConfig} — merge CLI-style overrides with `process.env`. * - {@link setPineconeClient} — inject a client instance before `setupServer()`. * - {@link registerUrlGenerator} / {@link unregisterUrlGenerator} — extend URL synthesis. + * - {@link toolErrorSchema} / {@link ToolError} — parse MCP tool failures (`isError: true` JSON bodies). * - Built-in `mailing` / `slack-Cpplang` URL generators are registered from {@link setupServer} * via {@link registerBuiltinUrlGenerators}; call it yourself if you use the library without `setupServer`. * @@ -35,6 +36,12 @@ import { registerSuggestQueryParamsTool } from './server/tools/suggest-query-par export { setPineconeClient } from './server/client-context.js'; /** Validate user-supplied Pinecone metadata filter objects before querying. */ export { validateMetadataFilter } from './server/metadata-filter.js'; +/** Structured metadata filter validation (`field` dot-path); {@link validateMetadataFilter} remains a string-only wrapper. */ +export { validateMetadataFilterDetailed } from './server/metadata-filter.js'; +export type { MetadataFilterValidationError } from './server/metadata-filter.js'; +/** Zod schema and types for MCP tool error JSON bodies (`isError: true`). */ +export { toolErrorSchema } from './server/tool-error.js'; +export type { ToolError, ToolErrorCode } from './server/tool-error.js'; /** Heuristic field + tool suggestions from a namespace schema + user query. */ export { suggestQueryParams } from './server/query-suggestion.js'; export type { RecommendedTool, SuggestQueryParamsResult } from './server/query-suggestion.js'; diff --git a/src/server/tool-error.test.ts b/src/server/tool-error.test.ts new file mode 100644 index 0000000..87b8000 --- /dev/null +++ b/src/server/tool-error.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from 'vitest'; +import { + classifyToolCatchError, + flowGateToolError, + toolErrorSchema, + validationToolError, +} from './tool-error.js'; + +describe('ToolError schema and builders', () => { + it('FLOW_GATE: includes required suggestion template', () => { + const err = flowGateToolError('wg21', 'Flow requires suggest_query_params first.'); + const parsed = toolErrorSchema.parse(err); + expect(parsed.code).toBe('FLOW_GATE'); + expect(parsed.recoverable).toBe(true); + expect(parsed.suggestion).toBe("Call suggest_query_params for namespace 'wg21' first"); + expect(parsed.message).toContain('Flow requires'); + }); + + it('VALIDATION: requires field and parses', () => { + const err = validationToolError('Unknown filter operator', 'author.$badop'); + const parsed = toolErrorSchema.parse(err); + expect(parsed.code).toBe('VALIDATION'); + expect(parsed.field).toBe('author.$badop'); + expect(parsed.recoverable).toBe(true); + }); + + it('VALIDATION: schema rejects payload missing field', () => { + const bad = { code: 'VALIDATION' as const, message: 'x', recoverable: true as const }; + const result = toolErrorSchema.safeParse(bad); + expect(result.success).toBe(false); + }); + + it('PINECONE_ERROR: classifyToolCatchError maps generic Error', () => { + const err = classifyToolCatchError(new Error('SDK boom'), 'fallback'); + expect(err.code).toBe('PINECONE_ERROR'); + expect(err.recoverable).toBe(false); + expect(toolErrorSchema.parse(err).code).toBe('PINECONE_ERROR'); + }); + + it('TIMEOUT: classifyToolCatchError matches withTimeout message prefix', () => { + const err = classifyToolCatchError( + new Error('Timeout after 100ms while waiting for query'), + 'fallback' + ); + expect(err.code).toBe('TIMEOUT'); + expect(err.recoverable).toBe(true); + expect(err.suggestion).toMatch(/retry|timeout/i); + toolErrorSchema.parse(err); + }); +}); diff --git a/src/types.ts b/src/types.ts index 81ec651..965da6d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -123,7 +123,7 @@ export type KeywordIndexNamespacesResult = | { ok: false; error: string }; export interface QueryResponse { - status: 'success' | 'error'; + status: 'success'; mode?: 'query' | 'query_fast' | 'query_detailed'; query?: string; namespace?: string; @@ -132,7 +132,6 @@ export interface QueryResponse { /** Present when the query requested specific fields. */ fields?: string[]; results?: QueryResultRowShape[]; - message?: string; } /** Internal merged hit shape before rerank (dense + sparse deduped). */ From a66cf105eaa49e13b6e309c67b56c224a08005a0 Mon Sep 17 00:00:00 2001 From: zho Date: Fri, 15 May 2026 07:47:19 +0800 Subject: [PATCH 3/6] fixed type check errors --- src/server/metadata-filter.ts | 8 +++---- src/server/tool-error.ts | 12 ++-------- src/server/tools/count-tool.ts | 4 +--- src/server/tools/generate-urls-tool.ts | 4 +--- src/server/tools/guided-query-tool.ts | 24 ++++++++++--------- src/server/tools/list-namespaces-tool.ts | 4 +--- src/server/tools/namespace-router-tool.ts | 8 ++----- src/server/tools/query-documents-tool.test.ts | 7 +++++- src/server/tools/query-documents-tool.ts | 4 +--- src/server/tools/query-tool.test.ts | 7 +++++- src/server/tools/query-tool.ts | 4 +--- .../tools/suggest-query-params-tool.test.ts | 7 +++++- src/server/tools/suggest-query-params-tool.ts | 8 ++----- 13 files changed, 46 insertions(+), 55 deletions(-) diff --git a/src/server/metadata-filter.ts b/src/server/metadata-filter.ts index 00d9ec5..4bed734 100644 --- a/src/server/metadata-filter.ts +++ b/src/server/metadata-filter.ts @@ -116,10 +116,10 @@ function validateMetadataFilterValue( ]); } if ((key === '$and' || key === '$or') && !Array.isArray(nestedValue)) { - return err( - `Operator "${key}" at "${path.join('.')}" must use an array of filter objects.`, - [...path, key] - ); + return err(`Operator "${key}" at "${path.join('.')}" must use an array of filter objects.`, [ + ...path, + key, + ]); } const nestedError = validateMetadataFilterValue(nestedValue, [...path, key]); diff --git a/src/server/tool-error.ts b/src/server/tool-error.ts index 5fc2e79..d506f96 100644 --- a/src/server/tool-error.ts +++ b/src/server/tool-error.ts @@ -17,12 +17,7 @@ export function logToolError(toolName: string, error: unknown): void { logError(`Error in ${toolName} tool`, error); } -export const toolErrorCodeSchema = z.enum([ - 'FLOW_GATE', - 'VALIDATION', - 'PINECONE_ERROR', - 'TIMEOUT', -]); +export const toolErrorCodeSchema = z.enum(['FLOW_GATE', 'VALIDATION', 'PINECONE_ERROR', 'TIMEOUT']); export type ToolErrorCode = z.infer; const flowGateToolErrorSchema = z.object({ @@ -103,10 +98,7 @@ export function pineconeToolError( }; } -export function timeoutToolError( - message: string, - options?: { suggestion?: string } -): ToolError { +export function timeoutToolError(message: string, options?: { suggestion?: string }): ToolError { return { code: 'TIMEOUT', message, diff --git a/src/server/tools/count-tool.ts b/src/server/tools/count-tool.ts index 7531abf..f9214ff 100644 --- a/src/server/tools/count-tool.ts +++ b/src/server/tools/count-tool.ts @@ -54,9 +54,7 @@ export function registerCountTool(server: McpServer): void { try { const { namespace, query_text, metadata_filter } = params; if (!query_text.trim()) { - return jsonErrorResponse( - validationToolError('query_text cannot be empty', 'query_text') - ); + return jsonErrorResponse(validationToolError('query_text cannot be empty', 'query_text')); } if (metadata_filter) { const err = validateMetadataFilterDetailed(metadata_filter); diff --git a/src/server/tools/generate-urls-tool.ts b/src/server/tools/generate-urls-tool.ts index c85bde8..c8b6941 100644 --- a/src/server/tools/generate-urls-tool.ts +++ b/src/server/tools/generate-urls-tool.ts @@ -58,9 +58,7 @@ export function registerGenerateUrlsTool(server: McpServer): void { }); } catch (error) { logToolError('generate_urls', error); - return jsonErrorResponse( - classifyToolCatchError(error, 'Failed to generate URLs') - ); + return jsonErrorResponse(classifyToolCatchError(error, 'Failed to generate URLs')); } } ); diff --git a/src/server/tools/guided-query-tool.ts b/src/server/tools/guided-query-tool.ts index 220cde4..a6c57da 100644 --- a/src/server/tools/guided-query-tool.ts +++ b/src/server/tools/guided-query-tool.ts @@ -83,9 +83,7 @@ export function registerGuidedQueryTool(server: McpServer): void { } = params; if (!user_query?.trim()) { - return jsonErrorResponse( - validationToolError('user_query cannot be empty', 'user_query') - ); + return jsonErrorResponse(validationToolError('user_query cannot be empty', 'user_query')); } if (metadata_filter) { @@ -107,10 +105,13 @@ export function registerGuidedQueryTool(server: McpServer): void { */ if (!namespace) { return jsonErrorResponse( - pineconeToolError('No namespace available. Please run list_namespaces and verify index data.', { - recoverable: true, - suggestion: 'Call list_namespaces to confirm the index has namespaces, then retry.', - }) + pineconeToolError( + 'No namespace available. Please run list_namespaces and verify index data.', + { + recoverable: true, + suggestion: 'Call list_namespaces to confirm the index has namespaces, then retry.', + } + ) ); } @@ -121,7 +122,10 @@ export function registerGuidedQueryTool(server: McpServer): void { validationToolError( `Namespace "${namespace}" not found in cached namespaces. Call list_namespaces and retry.`, 'namespace', - { suggestion: 'Use a namespace name returned by list_namespaces, then call list_namespaces again if the cache is stale.' } + { + suggestion: + 'Use a namespace name returned by list_namespaces, then call list_namespaces again if the cache is stale.', + } ) ); } @@ -209,9 +213,7 @@ export function registerGuidedQueryTool(server: McpServer): void { }); } catch (error) { logToolError('guided_query', error); - return jsonErrorResponse( - classifyToolCatchError(error, 'Failed to execute guided query') - ); + return jsonErrorResponse(classifyToolCatchError(error, 'Failed to execute guided query')); } } ); diff --git a/src/server/tools/list-namespaces-tool.ts b/src/server/tools/list-namespaces-tool.ts index 5817e21..f7ae9fa 100644 --- a/src/server/tools/list-namespaces-tool.ts +++ b/src/server/tools/list-namespaces-tool.ts @@ -37,9 +37,7 @@ export function registerListNamespacesTool(server: McpServer): void { return jsonResponse(response); } catch (error) { logToolError('list_namespaces', error); - return jsonErrorResponse( - classifyToolCatchError(error, 'Failed to list namespaces') - ); + return jsonErrorResponse(classifyToolCatchError(error, 'Failed to list namespaces')); } } ); diff --git a/src/server/tools/namespace-router-tool.ts b/src/server/tools/namespace-router-tool.ts index 4f7be61..319b0c7 100644 --- a/src/server/tools/namespace-router-tool.ts +++ b/src/server/tools/namespace-router-tool.ts @@ -30,9 +30,7 @@ export function registerNamespaceRouterTool(server: McpServer): void { try { const { user_query, top_n } = params; if (!user_query?.trim()) { - return jsonErrorResponse( - validationToolError('user_query cannot be empty', 'user_query') - ); + return jsonErrorResponse(validationToolError('user_query cannot be empty', 'user_query')); } const { data, cache_hit } = await getNamespacesWithCache(); const ranked = rankNamespacesByQuery(user_query.trim(), data, top_n); @@ -47,9 +45,7 @@ export function registerNamespaceRouterTool(server: McpServer): void { return jsonResponse(response); } catch (error) { logToolError('namespace_router', error); - return jsonErrorResponse( - classifyToolCatchError(error, 'Failed to route namespace') - ); + return jsonErrorResponse(classifyToolCatchError(error, 'Failed to route namespace')); } } ); diff --git a/src/server/tools/query-documents-tool.test.ts b/src/server/tools/query-documents-tool.test.ts index 3fad3e2..6b06f5c 100644 --- a/src/server/tools/query-documents-tool.test.ts +++ b/src/server/tools/query-documents-tool.test.ts @@ -4,7 +4,12 @@ import { getPineconeClient } from '../client-context.js'; import { reassembleByDocument } from '../reassemble-documents.js'; import * as suggestionFlow from '../suggestion-flow.js'; import { registerQueryDocumentsTool } from './query-documents-tool.js'; -import { createMockServer, makeSearchResult, parseToolJson, assertToolError } from './test-helpers.js'; +import { + createMockServer, + makeSearchResult, + parseToolJson, + assertToolError, +} from './test-helpers.js'; vi.mock('../client-context.js', () => ({ getPineconeClient: vi.fn(), diff --git a/src/server/tools/query-documents-tool.ts b/src/server/tools/query-documents-tool.ts index fe268f8..95acaf3 100644 --- a/src/server/tools/query-documents-tool.ts +++ b/src/server/tools/query-documents-tool.ts @@ -78,9 +78,7 @@ export function registerQueryDocumentsTool(server: McpServer): void { } = params; if (!query_text?.trim()) { - return jsonErrorResponse( - validationToolError('query_text cannot be empty', 'query_text') - ); + return jsonErrorResponse(validationToolError('query_text cannot be empty', 'query_text')); } if (metadata_filter) { diff --git a/src/server/tools/query-tool.test.ts b/src/server/tools/query-tool.test.ts index f2af9d2..99d7f0e 100644 --- a/src/server/tools/query-tool.test.ts +++ b/src/server/tools/query-tool.test.ts @@ -3,7 +3,12 @@ import { FAST_QUERY_FIELDS } from '../../constants.js'; import { getPineconeClient } from '../client-context.js'; import * as suggestionFlow from '../suggestion-flow.js'; import { registerQueryTool } from './query-tool.js'; -import { createMockServer, makeSearchResult, parseToolJson, assertToolError } from './test-helpers.js'; +import { + createMockServer, + makeSearchResult, + parseToolJson, + assertToolError, +} from './test-helpers.js'; vi.mock('../client-context.js', () => ({ getPineconeClient: vi.fn(), diff --git a/src/server/tools/query-tool.ts b/src/server/tools/query-tool.ts index 7a35b72..e4269c2 100644 --- a/src/server/tools/query-tool.ts +++ b/src/server/tools/query-tool.ts @@ -31,9 +31,7 @@ async function executeQuery(params: QueryExecParams) { const { query_text, namespace, top_k, use_reranking, metadata_filter, fields, mode } = params; try { if (!query_text.trim()) { - return jsonErrorResponse( - validationToolError('Query text cannot be empty', 'query_text') - ); + return jsonErrorResponse(validationToolError('Query text cannot be empty', 'query_text')); } if (metadata_filter) { diff --git a/src/server/tools/suggest-query-params-tool.test.ts b/src/server/tools/suggest-query-params-tool.test.ts index 4e16179..76d973a 100644 --- a/src/server/tools/suggest-query-params-tool.test.ts +++ b/src/server/tools/suggest-query-params-tool.test.ts @@ -2,7 +2,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { getNamespacesWithCache } from '../namespaces-cache.js'; import { markSuggested } from '../suggestion-flow.js'; import { registerSuggestQueryParamsTool } from './suggest-query-params-tool.js'; -import { createMockServer, makeNamespaceCacheEntry, parseToolJson, assertToolError } from './test-helpers.js'; +import { + createMockServer, + makeNamespaceCacheEntry, + parseToolJson, + assertToolError, +} from './test-helpers.js'; vi.mock('../namespaces-cache.js', () => ({ getNamespacesWithCache: vi.fn(), diff --git a/src/server/tools/suggest-query-params-tool.ts b/src/server/tools/suggest-query-params-tool.ts index bad2a9e..10f929b 100644 --- a/src/server/tools/suggest-query-params-tool.ts +++ b/src/server/tools/suggest-query-params-tool.ts @@ -33,9 +33,7 @@ export function registerSuggestQueryParamsTool(server: McpServer): void { try { const { namespace, user_query } = params; if (!user_query?.trim()) { - return jsonErrorResponse( - validationToolError('user_query cannot be empty', 'user_query') - ); + return jsonErrorResponse(validationToolError('user_query cannot be empty', 'user_query')); } const { data: namespacesInfo, cache_hit } = await getNamespacesWithCache(); const ns = namespacesInfo.find((n) => n.namespace === namespace); @@ -56,9 +54,7 @@ export function registerSuggestQueryParamsTool(server: McpServer): void { return jsonResponse(response); } catch (error) { logToolError('suggest_query_params', error); - return jsonErrorResponse( - classifyToolCatchError(error, 'Failed to suggest query params') - ); + return jsonErrorResponse(classifyToolCatchError(error, 'Failed to suggest query params')); } } ); From b225fe86ee18919ff4a86034153c101c0b2ac9ac Mon Sep 17 00:00:00 2001 From: zho Date: Fri, 15 May 2026 21:10:21 +0800 Subject: [PATCH 4/6] added some tests --- src/server/metadata-filter.test.ts | 53 +++++++++++ src/server/tools/count-tool.test.ts | 91 ++++++++++++++++++ src/server/tools/generate-urls-tool.test.ts | 23 +++++ src/server/tools/guided-query-tool.test.ts | 15 ++- src/server/tools/keyword-search-tool.test.ts | 94 +++++++++++++++++++ .../tools/namespace-router-tool.test.ts | 44 +++++++++ src/server/tools/query-documents-tool.test.ts | 15 +++ src/server/tools/query-tool.test.ts | 18 ++++ 8 files changed, 352 insertions(+), 1 deletion(-) create mode 100644 src/server/metadata-filter.test.ts create mode 100644 src/server/tools/count-tool.test.ts create mode 100644 src/server/tools/generate-urls-tool.test.ts create mode 100644 src/server/tools/keyword-search-tool.test.ts create mode 100644 src/server/tools/namespace-router-tool.test.ts diff --git a/src/server/metadata-filter.test.ts b/src/server/metadata-filter.test.ts new file mode 100644 index 0000000..aaa5ab7 --- /dev/null +++ b/src/server/metadata-filter.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest'; +import { validateMetadataFilter, validateMetadataFilterDetailed } from './metadata-filter.js'; + +describe('validateMetadataFilterDetailed', () => { + it('returns null for a valid filter', () => { + expect( + validateMetadataFilterDetailed({ + year: { $gte: 2020, $lte: 2026 }, + tags: { $in: ['a', 'b'] }, + }) + ).toBeNull(); + }); + + it('returns message and dot-path field for unknown nested operator', () => { + const d = validateMetadataFilterDetailed({ + year: { $regex: '^202' }, + }); + expect(d).not.toBeNull(); + expect(d!.message).toContain('Unknown filter operator'); + expect(d!.field).toBe('year.$regex'); + expect(validateMetadataFilter({ year: { $regex: '^202' } })).toBe(d!.message); + }); + + it('returns field for invalid $in value', () => { + const d = validateMetadataFilterDetailed({ + tags: { $in: 'not-an-array' }, + }); + expect(d!.field).toBe('tags.$in'); + expect(d!.message).toContain('primitive values'); + }); + + it('returns field for null metadata value', () => { + const d = validateMetadataFilterDetailed({ + author: null as unknown as Record, + }); + expect(d!.field).toBe('author'); + expect(d!.message).toContain('null'); + }); + + it('returns field when nested $and value is not an array', () => { + const d = validateMetadataFilterDetailed({ + tags: { $and: { $eq: 'x' } }, + }); + expect(d!.field).toBe('tags.$and'); + }); + + it('returns field when nested $or array element is an array, not a filter object', () => { + const d = validateMetadataFilterDetailed({ + tags: { $or: [[1]] }, + }); + expect(d!.field).toBe('tags.$or.0'); + }); +}); diff --git a/src/server/tools/count-tool.test.ts b/src/server/tools/count-tool.test.ts new file mode 100644 index 0000000..0555802 --- /dev/null +++ b/src/server/tools/count-tool.test.ts @@ -0,0 +1,91 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { getPineconeClient } from '../client-context.js'; +import * as suggestionFlow from '../suggestion-flow.js'; +import { registerCountTool } from './count-tool.js'; +import { assertToolError, createMockServer } from './test-helpers.js'; + +vi.mock('../client-context.js', () => ({ + getPineconeClient: vi.fn(), +})); + +const mockedGetClient = vi.mocked(getPineconeClient); + +describe('count tool handler', () => { + const flowOk = { + ok: true as const, + flow: { + updatedAt: Date.now(), + recommended_tool: 'count' as const, + suggested_fields: [], + user_query: 'q', + }, + }; + + beforeEach(() => { + vi.clearAllMocks(); + vi.spyOn(suggestionFlow, 'requireSuggested').mockReturnValue(flowOk); + mockedGetClient.mockReturnValue({ + count: vi.fn().mockResolvedValue({ count: 3, truncated: false }), + } as never); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('returns VALIDATION when query_text is empty', async () => { + const server = createMockServer(); + registerCountTool(server as never); + const raw = await server.getHandler('count')!({ + namespace: 'wg21', + query_text: ' ', + }); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('query_text'); + expect(mockedGetClient().count).not.toHaveBeenCalled(); + }); + + it('returns VALIDATION when metadata_filter is invalid', async () => { + const server = createMockServer(); + registerCountTool(server as never); + const raw = await server.getHandler('count')!({ + namespace: 'wg21', + query_text: 'doc', + metadata_filter: { year: { $unknown: 1 } }, + }); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('year.$unknown'); + }); + + it('returns FLOW_GATE when suggestion flow is not satisfied', async () => { + vi.spyOn(suggestionFlow, 'requireSuggested').mockReturnValue({ + ok: false, + message: 'Call suggest_query_params first.', + }); + const server = createMockServer(); + registerCountTool(server as never); + const raw = await server.getHandler('count')!({ + namespace: 'wg21', + query_text: 'x', + }); + const err = assertToolError(raw); + expect(err.code).toBe('FLOW_GATE'); + expect(err.suggestion).toBe("Call suggest_query_params for namespace 'wg21' first"); + }); + + it('returns PINECONE_ERROR when client.count throws', async () => { + mockedGetClient.mockReturnValue({ + count: vi.fn().mockRejectedValue(new Error('pinecone down')), + } as never); + const server = createMockServer(); + registerCountTool(server as never); + const raw = await server.getHandler('count')!({ + namespace: 'wg21', + query_text: 'x', + }); + const err = assertToolError(raw); + expect(err.code).toBe('PINECONE_ERROR'); + }); +}); diff --git a/src/server/tools/generate-urls-tool.test.ts b/src/server/tools/generate-urls-tool.test.ts new file mode 100644 index 0000000..252c825 --- /dev/null +++ b/src/server/tools/generate-urls-tool.test.ts @@ -0,0 +1,23 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import * as urlGeneration from '../url-generation.js'; +import { registerGenerateUrlsTool } from './generate-urls-tool.js'; +import { assertToolError, createMockServer } from './test-helpers.js'; + +describe('generate_urls tool handler', () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it('returns PINECONE_ERROR when generateUrlForNamespace throws', async () => { + vi.spyOn(urlGeneration, 'generateUrlForNamespace').mockImplementation(() => { + throw new Error('generator boom'); + }); + const server = createMockServer(); + registerGenerateUrlsTool(server as never); + const raw = await server.getHandler('generate_urls')!({ + namespace: 'mailing', + records: [{ document_number: 'P1234' }], + }); + expect(assertToolError(raw).code).toBe('PINECONE_ERROR'); + }); +}); diff --git a/src/server/tools/guided-query-tool.test.ts b/src/server/tools/guided-query-tool.test.ts index 7cd9127..cd4c31d 100644 --- a/src/server/tools/guided-query-tool.test.ts +++ b/src/server/tools/guided-query-tool.test.ts @@ -3,11 +3,11 @@ import { getPineconeClient } from '../client-context.js'; import { getNamespacesWithCache } from '../namespaces-cache.js'; import { registerGuidedQueryTool } from './guided-query-tool.js'; import { + assertToolError, createMockServer, makeNamespaceCacheEntry, makeSearchResult, parseToolJson, - assertToolError, } from './test-helpers.js'; vi.mock('../client-context.js', () => ({ @@ -137,4 +137,17 @@ describe('guided_query tool handler', () => { expect(err.recoverable).toBe(true); expect(err.message).toContain('No namespace available'); }); + + it('returns VALIDATION when explicit namespace is not in cached namespaces', async () => { + const server = createMockServer(); + registerGuidedQueryTool(server as never); + const raw = await server.getHandler('guided_query')!({ + user_query: 'hello', + namespace: 'not-in-cache', + }); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('namespace'); + expect(err.message).toContain('not-in-cache'); + }); }); diff --git a/src/server/tools/keyword-search-tool.test.ts b/src/server/tools/keyword-search-tool.test.ts new file mode 100644 index 0000000..c3e261f --- /dev/null +++ b/src/server/tools/keyword-search-tool.test.ts @@ -0,0 +1,94 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { getPineconeClient } from '../client-context.js'; +import { registerKeywordSearchTool } from './keyword-search-tool.js'; +import { assertToolError, createMockServer, makeSearchResult } from './test-helpers.js'; + +vi.mock('../client-context.js', () => ({ + getPineconeClient: vi.fn(), +})); + +const mockedGetClient = vi.mocked(getPineconeClient); + +describe('keyword_search tool handler', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockedGetClient.mockReturnValue({ + keywordSearch: vi.fn().mockResolvedValue([makeSearchResult()]), + getSparseIndexName: () => 'rag-hybrid-sparse', + } as never); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('returns VALIDATION when query_text is empty', async () => { + const server = createMockServer(); + registerKeywordSearchTool(server as never); + const raw = await server.getHandler('keyword_search')!({ + query_text: ' ', + namespace: 'ns', + top_k: 5, + }); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('query_text'); + }); + + it('returns VALIDATION when namespace is empty', async () => { + const server = createMockServer(); + registerKeywordSearchTool(server as never); + const raw = await server.getHandler('keyword_search')!({ + query_text: 'hello', + namespace: ' ', + top_k: 5, + }); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('namespace'); + }); + + it('returns VALIDATION when metadata_filter is invalid', async () => { + const server = createMockServer(); + registerKeywordSearchTool(server as never); + const raw = await server.getHandler('keyword_search')!({ + query_text: 'q', + namespace: 'ns', + top_k: 5, + metadata_filter: { bad: { $nope: true } }, + }); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('bad.$nope'); + }); + + it('happy path returns success', async () => { + const server = createMockServer(); + registerKeywordSearchTool(server as never); + const raw = await server.getHandler('keyword_search')!({ + query_text: 'contracts', + namespace: 'wg21', + top_k: 3, + }); + expect((raw as { isError?: boolean }).isError).toBeFalsy(); + const text = (raw as { content: Array<{ text: string }> }).content[0].text; + const body = JSON.parse(text) as { status: string; result_count?: number }; + expect(body.status).toBe('success'); + expect(body.result_count).toBe(1); + }); + + it('returns PINECONE_ERROR when keywordSearch throws', async () => { + mockedGetClient.mockReturnValue({ + keywordSearch: vi.fn().mockRejectedValue(new Error('sparse error')), + getSparseIndexName: () => 'rag-hybrid-sparse', + } as never); + const server = createMockServer(); + registerKeywordSearchTool(server as never); + const raw = await server.getHandler('keyword_search')!({ + query_text: 'q', + namespace: 'ns', + top_k: 5, + }); + expect(assertToolError(raw).code).toBe('PINECONE_ERROR'); + }); +}); diff --git a/src/server/tools/namespace-router-tool.test.ts b/src/server/tools/namespace-router-tool.test.ts new file mode 100644 index 0000000..38ac0fd --- /dev/null +++ b/src/server/tools/namespace-router-tool.test.ts @@ -0,0 +1,44 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { getNamespacesWithCache } from '../namespaces-cache.js'; +import { registerNamespaceRouterTool } from './namespace-router-tool.js'; +import { assertToolError, createMockServer, makeNamespaceCacheEntry } from './test-helpers.js'; + +vi.mock('../namespaces-cache.js', () => ({ + getNamespacesWithCache: vi.fn(), +})); + +const mockedGetNamespaces = vi.mocked(getNamespacesWithCache); + +describe('namespace_router tool handler', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockedGetNamespaces.mockResolvedValue({ + data: [makeNamespaceCacheEntry('papers')], + cache_hit: false, + expires_at: Date.now() + 1_800_000, + }); + }); + + it('returns VALIDATION when user_query is empty', async () => { + const server = createMockServer(); + registerNamespaceRouterTool(server as never); + const raw = await server.getHandler('namespace_router')!({ + user_query: ' ', + top_n: 3, + }); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('user_query'); + }); + + it('returns PINECONE_ERROR when getNamespacesWithCache throws', async () => { + mockedGetNamespaces.mockRejectedValue(new Error('cache failure')); + const server = createMockServer(); + registerNamespaceRouterTool(server as never); + const raw = await server.getHandler('namespace_router')!({ + user_query: 'find cpp papers', + top_n: 2, + }); + expect(assertToolError(raw).code).toBe('PINECONE_ERROR'); + }); +}); diff --git a/src/server/tools/query-documents-tool.test.ts b/src/server/tools/query-documents-tool.test.ts index 6b06f5c..d42f788 100644 --- a/src/server/tools/query-documents-tool.test.ts +++ b/src/server/tools/query-documents-tool.test.ts @@ -123,6 +123,21 @@ describe('query_documents tool handler', () => { expect(query).not.toHaveBeenCalled(); }); + it('returns VALIDATION for invalid metadata_filter', async () => { + const server = createMockServer(); + registerQueryDocumentsTool(server as never); + const query = mockedGetClient().query as ReturnType; + const raw = await server.getHandler('query_documents')!({ + query_text: 'ok', + namespace: 'wg21', + metadata_filter: { x: { $in: {} } }, + }); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('x.$in'); + expect(query).not.toHaveBeenCalled(); + }); + it('returns TTL expiry error from requireSuggested', async () => { vi.spyOn(suggestionFlow, 'requireSuggested').mockReturnValue({ ok: false, diff --git a/src/server/tools/query-tool.test.ts b/src/server/tools/query-tool.test.ts index 99d7f0e..935b730 100644 --- a/src/server/tools/query-tool.test.ts +++ b/src/server/tools/query-tool.test.ts @@ -165,6 +165,24 @@ describe('query tool handler (preset-driven)', () => { expect(err.suggestion).toBe("Call suggest_query_params for namespace 'wg21' first"); }); + it('query: returns VALIDATION for invalid metadata_filter', async () => { + const server = createMockServer(); + registerQueryTool(server as never); + const query = mockedGetClient().query as ReturnType; + const raw = await server.getHandler('query')!({ + query_text: 'hello', + namespace: 'wg21', + top_k: 5, + preset: 'full', + metadata_filter: { year: { $badop: 1 } }, + }); + expect((raw as { isError?: boolean }).isError).toBe(true); + const err = assertToolError(raw); + expect(err.code).toBe('VALIDATION'); + expect(err.field).toBe('year.$badop'); + expect(query).not.toHaveBeenCalled(); + }); + it('query: surfaces unreranked hits when client returns reranked:false (rerank fallback shape)', async () => { mockedGetClient.mockReturnValue({ query: vi From 23682eefcf46b67147852bbecf5af48a965bfa81 Mon Sep 17 00:00:00 2001 From: zho Date: Fri, 15 May 2026 22:56:09 +0800 Subject: [PATCH 5/6] addressed ai reviews --- src/server/tools/guided-query-tool.ts | 6 +++++- src/server/tools/test-helpers.ts | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/server/tools/guided-query-tool.ts b/src/server/tools/guided-query-tool.ts index a6c57da..a745122 100644 --- a/src/server/tools/guided-query-tool.ts +++ b/src/server/tools/guided-query-tool.ts @@ -97,7 +97,11 @@ export function registerGuidedQueryTool(server: McpServer): void { const { data: namespaces, cache_hit } = await getNamespacesWithCache(); const ranked = rankNamespacesByQuery(queryText, namespaces, 3); - const namespace = inputNamespace ?? ranked[0]?.namespace; + const explicitNamespace = inputNamespace?.trim(); + if (inputNamespace !== undefined && !explicitNamespace) { + return jsonErrorResponse(validationToolError('namespace cannot be empty', 'namespace')); + } + const namespace = explicitNamespace ?? ranked[0]?.namespace; /* * ToolError mapping: empty index / no routable namespace is backend/data state * (PINECONE_ERROR, recoverable). Explicit namespace missing from cache is user/input diff --git a/src/server/tools/test-helpers.ts b/src/server/tools/test-helpers.ts index b5b6519..7561338 100644 --- a/src/server/tools/test-helpers.ts +++ b/src/server/tools/test-helpers.ts @@ -32,6 +32,10 @@ export function createMockServer(): { /** Parse JSON body from {@link jsonResponse} / {@link jsonErrorResponse} payload. */ export function parseToolJson(payload: unknown): Record { + const envelope = payload as { isError?: unknown }; + if (envelope?.isError !== true) { + throw new Error('Expected MCP tool response with isError: true'); + } const p = payload as { content: Array<{ type: string; text: string }> }; const text = p.content[0]?.text; if (typeof text !== 'string') { From f580e6b745198f645903b4943e12107f559a7cc5 Mon Sep 17 00:00:00 2001 From: zho Date: Fri, 15 May 2026 23:01:12 +0800 Subject: [PATCH 6/6] fixed tool error --- src/server/tools/count-tool.test.ts | 14 ++++------ src/server/tools/generate-urls-tool.test.ts | 4 +-- src/server/tools/guided-query-tool.test.ts | 11 +++----- src/server/tools/keyword-search-tool.test.ts | 13 ++++----- src/server/tools/list-namespaces-tool.test.ts | 5 ++-- .../tools/namespace-router-tool.test.ts | 7 +++-- src/server/tools/query-documents-tool.test.ts | 14 ++++------ src/server/tools/query-tool.test.ts | 14 ++++------ .../tools/suggest-query-params-tool.test.ts | 8 +++--- src/server/tools/test-helpers.ts | 27 ++++++++++++++----- 10 files changed, 55 insertions(+), 62 deletions(-) diff --git a/src/server/tools/count-tool.test.ts b/src/server/tools/count-tool.test.ts index 0555802..c899d2b 100644 --- a/src/server/tools/count-tool.test.ts +++ b/src/server/tools/count-tool.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { getPineconeClient } from '../client-context.js'; import * as suggestionFlow from '../suggestion-flow.js'; import { registerCountTool } from './count-tool.js'; -import { assertToolError, createMockServer } from './test-helpers.js'; +import { assertToolErrorCode, createMockServer } from './test-helpers.js'; vi.mock('../client-context.js', () => ({ getPineconeClient: vi.fn(), @@ -40,8 +40,7 @@ describe('count tool handler', () => { namespace: 'wg21', query_text: ' ', }); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('query_text'); expect(mockedGetClient().count).not.toHaveBeenCalled(); }); @@ -54,8 +53,7 @@ describe('count tool handler', () => { query_text: 'doc', metadata_filter: { year: { $unknown: 1 } }, }); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('year.$unknown'); }); @@ -70,8 +68,7 @@ describe('count tool handler', () => { namespace: 'wg21', query_text: 'x', }); - const err = assertToolError(raw); - expect(err.code).toBe('FLOW_GATE'); + const err = assertToolErrorCode(raw, 'FLOW_GATE'); expect(err.suggestion).toBe("Call suggest_query_params for namespace 'wg21' first"); }); @@ -85,7 +82,6 @@ describe('count tool handler', () => { namespace: 'wg21', query_text: 'x', }); - const err = assertToolError(raw); - expect(err.code).toBe('PINECONE_ERROR'); + expect(assertToolErrorCode(raw, 'PINECONE_ERROR').code).toBe('PINECONE_ERROR'); }); }); diff --git a/src/server/tools/generate-urls-tool.test.ts b/src/server/tools/generate-urls-tool.test.ts index 252c825..96a57a8 100644 --- a/src/server/tools/generate-urls-tool.test.ts +++ b/src/server/tools/generate-urls-tool.test.ts @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import * as urlGeneration from '../url-generation.js'; import { registerGenerateUrlsTool } from './generate-urls-tool.js'; -import { assertToolError, createMockServer } from './test-helpers.js'; +import { assertToolErrorCode, createMockServer } from './test-helpers.js'; describe('generate_urls tool handler', () => { beforeEach(() => { @@ -18,6 +18,6 @@ describe('generate_urls tool handler', () => { namespace: 'mailing', records: [{ document_number: 'P1234' }], }); - expect(assertToolError(raw).code).toBe('PINECONE_ERROR'); + expect(assertToolErrorCode(raw, 'PINECONE_ERROR').code).toBe('PINECONE_ERROR'); }); }); diff --git a/src/server/tools/guided-query-tool.test.ts b/src/server/tools/guided-query-tool.test.ts index cd4c31d..a748f74 100644 --- a/src/server/tools/guided-query-tool.test.ts +++ b/src/server/tools/guided-query-tool.test.ts @@ -3,7 +3,7 @@ import { getPineconeClient } from '../client-context.js'; import { getNamespacesWithCache } from '../namespaces-cache.js'; import { registerGuidedQueryTool } from './guided-query-tool.js'; import { - assertToolError, + assertToolErrorCode, createMockServer, makeNamespaceCacheEntry, makeSearchResult, @@ -112,8 +112,7 @@ describe('guided_query tool handler', () => { }); expect((raw as { isError?: boolean }).isError).toBe(true); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('user_query'); expect(err.message).toBe('user_query cannot be empty'); }); @@ -132,8 +131,7 @@ describe('guided_query tool handler', () => { user_query: 'hello world', }); - const err = assertToolError(raw); - expect(err.code).toBe('PINECONE_ERROR'); + const err = assertToolErrorCode(raw, 'PINECONE_ERROR'); expect(err.recoverable).toBe(true); expect(err.message).toContain('No namespace available'); }); @@ -145,8 +143,7 @@ describe('guided_query tool handler', () => { user_query: 'hello', namespace: 'not-in-cache', }); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('namespace'); expect(err.message).toContain('not-in-cache'); }); diff --git a/src/server/tools/keyword-search-tool.test.ts b/src/server/tools/keyword-search-tool.test.ts index c3e261f..96e6f21 100644 --- a/src/server/tools/keyword-search-tool.test.ts +++ b/src/server/tools/keyword-search-tool.test.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { getPineconeClient } from '../client-context.js'; import { registerKeywordSearchTool } from './keyword-search-tool.js'; -import { assertToolError, createMockServer, makeSearchResult } from './test-helpers.js'; +import { assertToolErrorCode, createMockServer, makeSearchResult } from './test-helpers.js'; vi.mock('../client-context.js', () => ({ getPineconeClient: vi.fn(), @@ -30,8 +30,7 @@ describe('keyword_search tool handler', () => { namespace: 'ns', top_k: 5, }); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('query_text'); }); @@ -43,8 +42,7 @@ describe('keyword_search tool handler', () => { namespace: ' ', top_k: 5, }); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('namespace'); }); @@ -57,8 +55,7 @@ describe('keyword_search tool handler', () => { top_k: 5, metadata_filter: { bad: { $nope: true } }, }); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('bad.$nope'); }); @@ -89,6 +86,6 @@ describe('keyword_search tool handler', () => { namespace: 'ns', top_k: 5, }); - expect(assertToolError(raw).code).toBe('PINECONE_ERROR'); + expect(assertToolErrorCode(raw, 'PINECONE_ERROR').code).toBe('PINECONE_ERROR'); }); }); diff --git a/src/server/tools/list-namespaces-tool.test.ts b/src/server/tools/list-namespaces-tool.test.ts index 6aef8f7..e77d3a2 100644 --- a/src/server/tools/list-namespaces-tool.test.ts +++ b/src/server/tools/list-namespaces-tool.test.ts @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { getNamespacesWithCache } from '../namespaces-cache.js'; import { registerListNamespacesTool } from './list-namespaces-tool.js'; -import { createMockServer, parseToolJson, assertToolError } from './test-helpers.js'; +import { createMockServer, parseToolJson, assertToolErrorCode } from './test-helpers.js'; vi.mock('../namespaces-cache.js', () => ({ getNamespacesWithCache: vi.fn(), @@ -65,8 +65,7 @@ describe('list_namespaces tool handler', () => { const payload = raw as { isError?: boolean }; expect(payload.isError).toBe(true); - const err = assertToolError(raw); - expect(err.code).toBe('PINECONE_ERROR'); + const err = assertToolErrorCode(raw, 'PINECONE_ERROR'); expect(err.message).toBe('Failed to list namespaces'); }); }); diff --git a/src/server/tools/namespace-router-tool.test.ts b/src/server/tools/namespace-router-tool.test.ts index 38ac0fd..d1d9d27 100644 --- a/src/server/tools/namespace-router-tool.test.ts +++ b/src/server/tools/namespace-router-tool.test.ts @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { getNamespacesWithCache } from '../namespaces-cache.js'; import { registerNamespaceRouterTool } from './namespace-router-tool.js'; -import { assertToolError, createMockServer, makeNamespaceCacheEntry } from './test-helpers.js'; +import { assertToolErrorCode, createMockServer, makeNamespaceCacheEntry } from './test-helpers.js'; vi.mock('../namespaces-cache.js', () => ({ getNamespacesWithCache: vi.fn(), @@ -26,8 +26,7 @@ describe('namespace_router tool handler', () => { user_query: ' ', top_n: 3, }); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('user_query'); }); @@ -39,6 +38,6 @@ describe('namespace_router tool handler', () => { user_query: 'find cpp papers', top_n: 2, }); - expect(assertToolError(raw).code).toBe('PINECONE_ERROR'); + expect(assertToolErrorCode(raw, 'PINECONE_ERROR').code).toBe('PINECONE_ERROR'); }); }); diff --git a/src/server/tools/query-documents-tool.test.ts b/src/server/tools/query-documents-tool.test.ts index d42f788..b41768e 100644 --- a/src/server/tools/query-documents-tool.test.ts +++ b/src/server/tools/query-documents-tool.test.ts @@ -5,10 +5,10 @@ import { reassembleByDocument } from '../reassemble-documents.js'; import * as suggestionFlow from '../suggestion-flow.js'; import { registerQueryDocumentsTool } from './query-documents-tool.js'; import { + assertToolErrorCode, createMockServer, makeSearchResult, parseToolJson, - assertToolError, } from './test-helpers.js'; vi.mock('../client-context.js', () => ({ @@ -95,8 +95,7 @@ describe('query_documents tool handler', () => { expect((raw as { isError?: boolean }).isError).toBe(true); expect(query).not.toHaveBeenCalled(); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('query_text'); expect(err.message).toBe('query_text cannot be empty'); }); @@ -117,8 +116,7 @@ describe('query_documents tool handler', () => { namespace: 'wg21', }); - const err = assertToolError(raw); - expect(err.code).toBe('FLOW_GATE'); + const err = assertToolErrorCode(raw, 'FLOW_GATE'); expect(err.suggestion).toBe("Call suggest_query_params for namespace 'wg21' first"); expect(query).not.toHaveBeenCalled(); }); @@ -132,8 +130,7 @@ describe('query_documents tool handler', () => { namespace: 'wg21', metadata_filter: { x: { $in: {} } }, }); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('x.$in'); expect(query).not.toHaveBeenCalled(); }); @@ -153,8 +150,7 @@ describe('query_documents tool handler', () => { namespace: 'wg21', }); - const err = assertToolError(raw); - expect(err.code).toBe('FLOW_GATE'); + const err = assertToolErrorCode(raw, 'FLOW_GATE'); expect(err.message).toContain('expired'); expect(err.suggestion).toBe("Call suggest_query_params for namespace 'wg21' first"); }); diff --git a/src/server/tools/query-tool.test.ts b/src/server/tools/query-tool.test.ts index 935b730..1b004c0 100644 --- a/src/server/tools/query-tool.test.ts +++ b/src/server/tools/query-tool.test.ts @@ -4,10 +4,10 @@ import { getPineconeClient } from '../client-context.js'; import * as suggestionFlow from '../suggestion-flow.js'; import { registerQueryTool } from './query-tool.js'; import { + assertToolErrorCode, createMockServer, makeSearchResult, parseToolJson, - assertToolError, } from './test-helpers.js'; vi.mock('../client-context.js', () => ({ @@ -107,8 +107,7 @@ describe('query tool handler (preset-driven)', () => { expect((raw as { isError?: boolean }).isError).toBe(true); expect(query).not.toHaveBeenCalled(); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('query_text'); expect(err.message).toBe('Query text cannot be empty'); }); @@ -133,8 +132,7 @@ describe('query tool handler (preset-driven)', () => { expect((raw as { isError?: boolean }).isError).toBe(true); expect(query).not.toHaveBeenCalled(); - const err = assertToolError(raw); - expect(err.code).toBe('FLOW_GATE'); + const err = assertToolErrorCode(raw, 'FLOW_GATE'); expect(err.suggestion).toBe("Call suggest_query_params for namespace 'wg21' first"); expect(err.message).toBe( 'Flow requires suggest_query_params first. Call suggest_query_params with namespace and user_query before query/count tools.' @@ -159,8 +157,7 @@ describe('query tool handler (preset-driven)', () => { preset: 'full', }); - const err = assertToolError(raw); - expect(err.code).toBe('FLOW_GATE'); + const err = assertToolErrorCode(raw, 'FLOW_GATE'); expect(err.message).toBe(expiredMsg); expect(err.suggestion).toBe("Call suggest_query_params for namespace 'wg21' first"); }); @@ -177,8 +174,7 @@ describe('query tool handler (preset-driven)', () => { metadata_filter: { year: { $badop: 1 } }, }); expect((raw as { isError?: boolean }).isError).toBe(true); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('year.$badop'); expect(query).not.toHaveBeenCalled(); }); diff --git a/src/server/tools/suggest-query-params-tool.test.ts b/src/server/tools/suggest-query-params-tool.test.ts index 76d973a..472a573 100644 --- a/src/server/tools/suggest-query-params-tool.test.ts +++ b/src/server/tools/suggest-query-params-tool.test.ts @@ -6,7 +6,7 @@ import { createMockServer, makeNamespaceCacheEntry, parseToolJson, - assertToolError, + assertToolErrorCode, } from './test-helpers.js'; vi.mock('../namespaces-cache.js', () => ({ @@ -88,8 +88,7 @@ describe('suggest_query_params tool handler', () => { }); expect((raw as { isError?: boolean }).isError).toBe(true); - const err = assertToolError(raw); - expect(err.code).toBe('VALIDATION'); + const err = assertToolErrorCode(raw, 'VALIDATION'); expect(err.field).toBe('user_query'); expect(err.message).toBe('user_query cannot be empty'); }); @@ -105,8 +104,7 @@ describe('suggest_query_params tool handler', () => { }); expect((raw as { isError?: boolean }).isError).toBe(true); - const err = assertToolError(raw); - expect(err.code).toBe('PINECONE_ERROR'); + const err = assertToolErrorCode(raw, 'PINECONE_ERROR'); expect(err.message).toBe('Failed to suggest query params'); }); }); diff --git a/src/server/tools/test-helpers.ts b/src/server/tools/test-helpers.ts index 7561338..7dcd118 100644 --- a/src/server/tools/test-helpers.ts +++ b/src/server/tools/test-helpers.ts @@ -1,5 +1,5 @@ import type { SearchResult } from '../../types.js'; -import type { ToolError } from '../tool-error.js'; +import type { ToolError, ToolErrorCode } from '../tool-error.js'; import { toolErrorSchema } from '../tool-error.js'; /** Handler invoked by MCP tool registration (params shape varies by tool). */ @@ -32,10 +32,6 @@ export function createMockServer(): { /** Parse JSON body from {@link jsonResponse} / {@link jsonErrorResponse} payload. */ export function parseToolJson(payload: unknown): Record { - const envelope = payload as { isError?: unknown }; - if (envelope?.isError !== true) { - throw new Error('Expected MCP tool response with isError: true'); - } const p = payload as { content: Array<{ type: string; text: string }> }; const text = p.content[0]?.text; if (typeof text !== 'string') { @@ -44,12 +40,31 @@ export function parseToolJson(payload: unknown): Record { return JSON.parse(text) as Record; } -/** Parse MCP tool error JSON from a handler return value (expects `isError: true`). */ +/** Parse MCP tool error JSON from a handler return value (expects `isError: true` on the envelope). */ export function assertToolError(payload: unknown): ToolError { + const envelope = payload as { isError?: unknown }; + if (envelope?.isError !== true) { + throw new Error('Expected MCP tool response with isError: true'); + } const raw = parseToolJson(payload); return toolErrorSchema.parse(raw); } +/** + * Like {@link assertToolError}, but asserts `code` and narrows the union so + * `field` / `suggestion` are type-safe per variant. + */ +export function assertToolErrorCode( + payload: unknown, + code: C +): Extract { + const err = assertToolError(payload); + if (err.code !== code) { + throw new Error(`Expected tool error code ${code}, got ${err.code}`); + } + return err as Extract; +} + export function makeSearchResult(overrides?: Partial): SearchResult { return { id: 'hit-1',