From 7ee229ba23476ce1ac6dfe148c76dbe54af863b8 Mon Sep 17 00:00:00 2001 From: DK09876 Date: Thu, 12 Feb 2026 02:09:28 -0700 Subject: [PATCH] Fix MCP extra args rejection and bank ID resolution priority (#351) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix MCP extra args rejection and bank ID resolution priority Two fixes to the MCP middleware: 1. Strip unknown tool arguments: LLMs frequently add extra fields like "explanation" to tool calls. FastMCP's Pydantic TypeAdapter rejects these with "Unexpected keyword argument". The middleware now intercepts tools/call requests and removes unknown fields before they reach validation. 2. Bank ID resolution priority: Path now takes priority over header. Previously X-Bank-Id header was checked first, meaning /mcp/my-bank/ with X-Bank-Id: other-bank would silently use other-bank in multi-bank mode. Now the URL path is authoritative — single-bank mode connections cannot be overridden by headers. Co-Authored-By: Claude Opus 4.6 * docs: update MCP server docs with mental model tools and fixes - Add all mental model tools (create, list, get, update, delete, refresh) - Add list_banks and create_bank tool docs - Document single-bank vs multi-bank modes - Fix bank selection priority: path > header > default - Add Accept header to curl example - Add timestamp param to retain, max_tokens to recall Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- hindsight-api/hindsight_api/api/mcp.py | 52 +++++++- hindsight-docs/docs/developer/mcp-server.md | 138 +++++++++++++++++--- 2 files changed, 164 insertions(+), 26 deletions(-) diff --git a/hindsight-api/hindsight_api/api/mcp.py b/hindsight-api/hindsight_api/api/mcp.py index 6e0eb68b..ace7784b 100644 --- a/hindsight-api/hindsight_api/api/mcp.py +++ b/hindsight-api/hindsight_api/api/mcp.py @@ -114,9 +114,39 @@ def create_mcp_server(memory: MemoryEngine, multi_bank: bool = True) -> FastMCP: logger.info(f"Loading MCP extension: {mcp_extension.__class__.__name__}") mcp_extension.register_tools(mcp, memory) + # Make all tools tolerant of extra arguments from LLMs (e.g., "explanation") + _make_tools_tolerant(mcp) + return mcp +def _make_tools_tolerant(mcp: FastMCP) -> None: + """Wrap all tool run methods to strip unknown arguments before validation. + + LLMs frequently add extra fields like "explanation" or "reasoning" to tool calls. + FastMCP's Pydantic TypeAdapter rejects these with "Unexpected keyword argument". + This wraps each tool's run() to filter arguments to only known parameters. + """ + try: + for name, tool in mcp._tool_manager._tools.items(): + if hasattr(tool, "parameters") and tool.parameters: + allowed = set(tool.parameters.get("properties", {}).keys()) + original_run = tool.run + + async def _tolerant_run(arguments, _allowed=allowed, _orig=original_run): + extra_keys = set(arguments.keys()) - _allowed + if extra_keys: + logger.debug(f"Stripping unknown arguments from tool call: {extra_keys}") + arguments = {k: v for k, v in arguments.items() if k in _allowed} + return await _orig(arguments) + + # FunctionTool is a Pydantic model with extra='forbid', so use + # object.__setattr__ to bypass Pydantic's setter validation. + object.__setattr__(tool, "run", _tolerant_run) + except (AttributeError, KeyError) as e: + logger.warning(f"Could not make tools tolerant of extra arguments: {e}") + + class MCPMiddleware: """ASGI middleware that intercepts MCP requests and routes to appropriate MCP server. @@ -142,6 +172,11 @@ class MCPMiddleware: - No bank management tools (list_banks, create_bank) - Recommended for agent isolation + Bank ID resolution priority: + 1. URL path (e.g., /mcp/{bank_id}/) → single-bank mode + 2. X-Bank-Id header → multi-bank mode + 3. HINDSIGHT_MCP_BANK_ID env var → multi-bank mode (default: "default") + Examples: # Single-bank mode (recommended for agent isolation) claude mcp add --transport http my-agent http://localhost:8888/mcp/my-agent-bank/ \\ @@ -242,20 +277,25 @@ class MCPMiddleware: _current_schema.set(tenant_context.schema_name) if tenant_context and tenant_context.schema_name else None ) - # Try to get bank_id from header first (for Claude Code compatibility) - bank_id = self._get_header(scope, "X-Bank-Id") + # Resolve bank_id: path takes priority over header. + # Path = user's explicit connection endpoint (e.g., /mcp/my-bank/). + # X-Bank-Id header = per-request override for multi-bank mode only. + bank_id = None bank_id_from_path = False - - # If no header, try to extract from path: /{bank_id}/... new_path = path - if not bank_id and path.startswith("/") and len(path) > 1: + + # First, try to extract from path: /{bank_id}/... + if path.startswith("/") and len(path) > 1: parts = path[1:].split("/", 1) if parts[0]: - # First segment looks like a bank_id bank_id = parts[0] bank_id_from_path = True new_path = "/" + parts[1] if len(parts) > 1 else "/" + # If no path-based bank_id, try X-Bank-Id header (multi-bank mode) + if not bank_id: + bank_id = self._get_header(scope, "X-Bank-Id") + # Fall back to default bank_id if not bank_id: bank_id = DEFAULT_BANK_ID diff --git a/hindsight-docs/docs/developer/mcp-server.md b/hindsight-docs/docs/developer/mcp-server.md index 6e920e6b..2b98e931 100644 --- a/hindsight-docs/docs/developer/mcp-server.md +++ b/hindsight-docs/docs/developer/mcp-server.md @@ -71,6 +71,7 @@ curl -X POST http://localhost:8888/mcp \ -H "Authorization: Bearer your-secret-key" \ -H "X-Bank-Id: my-bank" \ -H "Content-Type: application/json" \ + -H "Accept: application/json, text/event-stream" \ -d '{"jsonrpc": "2.0", "method": "tools/list", "id": 1}' ``` @@ -78,10 +79,10 @@ If the key is missing or invalid, requests will receive a `401 Unauthorized` res ## Bank Selection -Specify the memory bank via: +The memory bank is resolved in this priority order: -1. **X-Bank-Id header** (recommended): `--header "X-Bank-Id: my-bank"` -2. **URL path**: `http://localhost:8888/mcp/my-bank/` +1. **URL path** (highest priority): `http://localhost:8888/mcp/my-bank/` +2. **X-Bank-Id header**: `--header "X-Bank-Id: my-bank"` 3. **Default**: Uses `HINDSIGHT_MCP_BANK_ID` env var (default: "default") ## Per-Bank Endpoints @@ -93,6 +94,19 @@ This design: - **Enforces isolation** — each MCP connection is scoped to a single bank - **Enables multi-tenant setups** — connect different users to different endpoints +## Two Modes + +The MCP server operates in two modes depending on the URL: + +| Mode | URL | Tools | bank_id | +|------|-----|-------|---------| +| **Single-bank** | `/mcp/{bank_id}/` | Memory + mental model tools | Implicit from URL | +| **Multi-bank** | `/mcp/` | All tools including bank management | Explicit `bank_id` parameter on each tool | + +**Single-bank mode** (recommended) scopes all operations to the bank in the URL. Tools don't expose a `bank_id` parameter. + +**Multi-bank mode** exposes all tools with an optional `bank_id` parameter, plus bank management tools (`list_banks`, `create_bank`). + --- ## Available Tools @@ -105,6 +119,7 @@ Store information to long-term memory. |-----------|------|----------|-------------| | `content` | string | Yes | The fact or memory to store | | `context` | string | No | Category for the memory (default: `general`) | +| `timestamp` | string | No | ISO 8601 timestamp for when the event occurred | **Example:** ```json @@ -133,6 +148,7 @@ Search memories to provide personalized responses. |-----------|------|----------|-------------| | `query` | string | Yes | Natural language search query | | `max_results` | integer | No | Maximum results to return (default: 10) | +| `max_tokens` | integer | No | Maximum tokens to return (default: 4096) | **Example:** ```json @@ -144,21 +160,6 @@ Search memories to provide personalized responses. } ``` -**Response:** -```json -{ - "results": [ - { - "id": "fact_abc123", - "text": "User prefers Python over JavaScript for backend development", - "type": "world", - "context": "programming_preferences", - "event_date": null - } - ] -} -``` - **When to use:** - Start of conversation to recall relevant context - Before making recommendations @@ -195,10 +196,107 @@ Generate thoughtful analysis by synthesizing stored memories with the bank's per --- +### create_mental_model + +Create a mental model — a living document that stays current with your memories. Mental models are pre-computed reflections that get automatically refreshed as new memories are stored. + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `name` | string | Yes | Human-readable name for the mental model | +| `source_query` | string | Yes | The query used to generate and refresh the model | +| `tags` | list[string] | No | Tags for organizing and filtering models | +| `max_tokens` | integer | No | Maximum tokens for model content (default: 2048) | + +**Example:** +```json +{ + "name": "create_mental_model", + "arguments": { + "name": "Team Directory", + "source_query": "Who works here and what do they do?", + "tags": ["team", "people"] + } +} +``` + +Content generation runs asynchronously. The response includes an `operation_id` to track progress. + +--- + +### list_mental_models + +List all mental models in a bank, optionally filtered by tags. + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `tags` | list[string] | No | Filter models by tags | + +--- + +### get_mental_model + +Retrieve a specific mental model by ID, including its full content. + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `mental_model_id` | string | Yes | The ID of the mental model to retrieve | + +--- + +### update_mental_model + +Update a mental model's metadata or settings. + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `mental_model_id` | string | Yes | The ID of the mental model to update | +| `name` | string | No | New name | +| `source_query` | string | No | New source query | +| `tags` | list[string] | No | New tags | +| `max_tokens` | integer | No | New max tokens | + +--- + +### delete_mental_model + +Permanently delete a mental model. + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `mental_model_id` | string | Yes | The ID of the mental model to delete | + +--- + +### refresh_mental_model + +Re-generate a mental model's content from the latest memories. Runs asynchronously. + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `mental_model_id` | string | Yes | The ID of the mental model to refresh | + +--- + +### list_banks (multi-bank mode only) + +List all available memory banks. + +--- + +### create_bank (multi-bank mode only) + +Create a new memory bank or retrieve an existing one. + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `bank_id` | string | Yes | The ID for the new bank | + +--- + ## Integration with AI Assistants The MCP server can be used with any MCP-compatible AI assistant. See the [Authentication](#authentication) section above for Claude Code and Claude Desktop configuration examples. Each user can have their own configuration pointing to their personal memory bank using either: -- The `X-Bank-Id` header (recommended) -- A bank-specific URL path like `/mcp/alice/` +- A bank-specific URL path like `/mcp/alice/` (recommended) +- The `X-Bank-Id` header