Add typed error response (#88)

* added typed error response

* add a test

* fixed type check errors

* added some tests

* addressed ai reviews

* fixed tool error

---------

Co-authored-by: zho <jornathanm910923@gmail.com>

Author: jonathanMLDev

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`.

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 '<ns>' 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

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';

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<string, unknown>,
+    });
+    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');
+  });
+});

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<string, unknown>);
+      const nestedError = validateMetadataFilterRecord(item as Record<string, unknown>);
       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, unknown>): string | null {
+function validateMetadataFilterRecord(
+  filter: Record<string, unknown>
+): 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<string, unknown>
+): 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, unknown>): string | null {
+  const detailed = validateMetadataFilterDetailed(filter);
+  return detailed?.message ?? null;
+}

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);
+  });
+});