* fix: move mental model usage metering into engine for MCP support Mental model validation hooks (validate_mental_model_get, validate_mental_model_refresh) were only called in REST HTTP handlers, not in the engine. MCP tools call engine methods directly, so usage metering was skipped entirely for MCP mental model operations. Moved pre-validation and post-completion hooks into memory_engine.py (matching the retain/recall/reflect pattern) and removed the duplicate code from http.py. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: remove double validation from create_mental_model and add internal checks - Remove pre-validation from create_mental_model since callers always call submit_async_refresh_mental_model next (which validates), preventing double credit checks - Add is_internal checks to mental model metering validators (matching the existing pattern for recall/reflect) so background worker tasks skip billing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: prevent 307 redirect on /mcp that breaks MCP tool discovery Starlette's Mount class redirects /mcp to /mcp/ with a 307 Temporary Redirect. Many MCP clients don't follow POST redirects, which causes tool discovery to fail (0 tools discovered despite successful auth). Add _MCPPathRewriteMiddleware that rewrites /mcp to /mcp/ at the ASGI level before routing, preventing the redirect entirely. Both /mcp and /mcp/ now work identically. Add regression test test_mcp_no_trailing_slash_works to verify URLs with and without trailing slashes discover tools correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * harden MCP server for real-world usage - Remove MCP_ENDPOINTS blocklist so banks named "sse"/"messages" route correctly - Scope SSE body rewriting to text/event-stream responses only to prevent data corruption - Add _validate_mental_model_inputs for name, source_query, max_tokens validation in MCP tools - Improve "not found" error messages to include bank_id context - Fix fragile tool count assertions (exact → minimum bounds) - Add integration tests: tool execution, input validation, edge-case bank names - Add unit tests for validation helper and tool-level validation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: replace Mount + rewrite middleware with wrapping middleware Starlette's Mount class redirects /mcp -> /mcp/ with 307, which MCP clients don't follow. Previously we patched this with _MCPPathRewriteMiddleware. Now MCPMiddleware wraps the FastAPI app directly via add_middleware, intercepting /mcp* requests before they reach Starlette's router. No Mount means no redirect. - Remove _MCPPathRewriteMiddleware (no longer needed) - Remove app.mount() call - Add prefix parameter to MCPMiddleware - Use app.add_middleware() for proper Starlette integration - Simplify path stripping (just remove prefix, no mount/root_path handling) - Update routing test to match current behavior (no MCP_ENDPOINTS blocklist) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: update stale docstring referencing removed _MCPPathRewriteMiddleware Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
169 lines
6.1 KiB
Python
169 lines
6.1 KiB
Python
"""Tests for MCPExtension loading and tool registration."""
|
|
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
import pytest
|
|
from fastmcp import FastMCP
|
|
|
|
from hindsight_api import MemoryEngine
|
|
from hindsight_api.extensions.mcp import MCPExtension
|
|
|
|
|
|
class MockMCPExtension(MCPExtension):
|
|
"""Test extension that registers a custom tool."""
|
|
|
|
def __init__(self, config=None):
|
|
super().__init__(config)
|
|
self.register_tools_called = False
|
|
self.registered_mcp = None
|
|
self.registered_memory = None
|
|
|
|
def register_tools(self, mcp: FastMCP, memory: MemoryEngine) -> None:
|
|
"""Register a test tool to verify extension was called."""
|
|
self.register_tools_called = True
|
|
self.registered_mcp = mcp
|
|
self.registered_memory = memory
|
|
|
|
@mcp.tool()
|
|
async def test_extension_tool(query: str) -> str:
|
|
"""A test tool registered by the extension."""
|
|
return f"Extension tool received: {query}"
|
|
|
|
|
|
class TestMCPExtensionBase:
|
|
"""Tests for MCPExtension base class."""
|
|
|
|
def test_mcp_extension_is_abstract(self):
|
|
"""MCPExtension.register_tools is abstract and must be implemented."""
|
|
with pytest.raises(TypeError, match="abstract method"):
|
|
MCPExtension()
|
|
|
|
def test_subclass_can_be_instantiated(self):
|
|
"""Subclass implementing register_tools can be instantiated."""
|
|
ext = MockMCPExtension()
|
|
assert ext is not None
|
|
assert ext.register_tools_called is False
|
|
|
|
def test_register_tools_receives_mcp_and_memory(self):
|
|
"""register_tools receives FastMCP and MemoryEngine instances."""
|
|
ext = MockMCPExtension()
|
|
mcp = FastMCP("test")
|
|
memory = MagicMock(spec=MemoryEngine)
|
|
|
|
ext.register_tools(mcp, memory)
|
|
|
|
assert ext.register_tools_called is True
|
|
assert ext.registered_mcp is mcp
|
|
assert ext.registered_memory is memory
|
|
|
|
|
|
class TestMCPExtensionLoading:
|
|
"""Tests for MCPExtension loading in create_mcp_server."""
|
|
|
|
@pytest.fixture
|
|
def mock_memory(self):
|
|
"""Create a mock MemoryEngine."""
|
|
memory = MagicMock()
|
|
memory._tenant_extension = MagicMock()
|
|
memory._tenant_extension.authenticate_mcp = MagicMock()
|
|
return memory
|
|
|
|
def test_create_mcp_server_without_extension(self, mock_memory):
|
|
"""create_mcp_server works without MCPExtension configured."""
|
|
from hindsight_api.api.mcp import create_mcp_server
|
|
|
|
with patch("hindsight_api.api.mcp.load_extension", return_value=None):
|
|
mcp = create_mcp_server(mock_memory)
|
|
|
|
# Core tools should be registered
|
|
tools = mcp._tool_manager._tools
|
|
assert "retain" in tools
|
|
assert "recall" in tools
|
|
assert "reflect" in tools
|
|
# Extension tool should NOT be present
|
|
assert "test_extension_tool" not in tools
|
|
|
|
def test_create_mcp_server_with_extension(self, mock_memory):
|
|
"""create_mcp_server loads and calls MCPExtension when configured."""
|
|
from hindsight_api.api.mcp import create_mcp_server
|
|
|
|
mock_ext = MockMCPExtension()
|
|
|
|
with patch("hindsight_api.api.mcp.load_extension", return_value=mock_ext):
|
|
mcp = create_mcp_server(mock_memory)
|
|
|
|
# Extension should have been called
|
|
assert mock_ext.register_tools_called is True
|
|
|
|
# Core tools should still be registered
|
|
tools = mcp._tool_manager._tools
|
|
assert "retain" in tools
|
|
assert "recall" in tools
|
|
|
|
# Extension tool should also be registered
|
|
assert "test_extension_tool" in tools
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_extension_tool_is_callable(self, mock_memory):
|
|
"""Tool registered by extension can be called."""
|
|
from hindsight_api.api.mcp import create_mcp_server
|
|
|
|
mock_ext = MockMCPExtension()
|
|
|
|
with patch("hindsight_api.api.mcp.load_extension", return_value=mock_ext):
|
|
mcp = create_mcp_server(mock_memory)
|
|
|
|
# Get and call the extension tool
|
|
tools = mcp._tool_manager._tools
|
|
test_tool = tools["test_extension_tool"]
|
|
result = await test_tool.fn(query="hello world")
|
|
|
|
assert result == "Extension tool received: hello world"
|
|
|
|
def test_load_extension_called_with_correct_args(self, mock_memory):
|
|
"""load_extension is called with 'MCP' prefix and MCPExtension class."""
|
|
from hindsight_api.api.mcp import create_mcp_server
|
|
|
|
with patch("hindsight_api.api.mcp.load_extension") as mock_load:
|
|
mock_load.return_value = None
|
|
create_mcp_server(mock_memory)
|
|
|
|
mock_load.assert_called_once_with("MCP", MCPExtension)
|
|
|
|
|
|
class TestMCPExtensionIntegration:
|
|
"""Integration tests verifying extension tools work end-to-end."""
|
|
|
|
@pytest.fixture
|
|
def mock_memory(self):
|
|
"""Create a mock MemoryEngine with required methods."""
|
|
memory = MagicMock()
|
|
memory.retain_batch_async = MagicMock()
|
|
memory.submit_async_retain = MagicMock(return_value={"operation_id": "test-op"})
|
|
memory.recall_async = MagicMock(return_value=MagicMock(results=[]))
|
|
memory.reflect_async = MagicMock(return_value=MagicMock(text="reflection"))
|
|
memory.list_banks = MagicMock(return_value=[])
|
|
memory.get_bank_profile = MagicMock(return_value={"id": "test"})
|
|
memory._tenant_extension = MagicMock()
|
|
return memory
|
|
|
|
def test_extension_tools_coexist_with_core_tools(self, mock_memory):
|
|
"""Extension tools are added alongside core tools, not replacing them."""
|
|
from hindsight_api.api.mcp import create_mcp_server
|
|
|
|
mock_ext = MockMCPExtension()
|
|
|
|
with patch("hindsight_api.api.mcp.load_extension", return_value=mock_ext):
|
|
mcp = create_mcp_server(mock_memory)
|
|
|
|
tools = mcp._tool_manager._tools
|
|
# All core tools present
|
|
assert "retain" in tools
|
|
assert "recall" in tools
|
|
assert "reflect" in tools
|
|
assert "list_banks" in tools
|
|
assert "create_bank" in tools
|
|
# Extension tool also present
|
|
assert "test_extension_tool" in tools
|
|
# At least 11 core + 1 extension = 12 tools (may grow as new tools are added)
|
|
assert len(tools) >= 12
|