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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
Expand All @@ -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`.
Expand Down
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
*
Expand All @@ -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';
Expand Down
53 changes: 53 additions & 0 deletions src/server/metadata-filter.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
73 changes: 60 additions & 13 deletions src/server/metadata-filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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)) {
Expand All @@ -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' ||
Expand All @@ -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]);
Expand All @@ -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;
}
50 changes: 50 additions & 0 deletions src/server/tool-error.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
Loading
Loading