feat: configure exposed mcp tools per bank (#439)
* feat: configure exposed mcp tools per bank * fix: update configurable fields count to 11 after adding mcp_enabled_tools
This commit is contained in:
parent
f5b94d4b28
commit
4b328a9cb3
9 changed files with 514 additions and 12 deletions
|
|
@ -8,12 +8,48 @@ from contextvars import ContextVar
|
||||||
from fastmcp import FastMCP
|
from fastmcp import FastMCP
|
||||||
|
|
||||||
from hindsight_api import MemoryEngine
|
from hindsight_api import MemoryEngine
|
||||||
|
from hindsight_api.config import _get_raw_config
|
||||||
from hindsight_api.engine.memory_engine import _current_schema
|
from hindsight_api.engine.memory_engine import _current_schema
|
||||||
from hindsight_api.extensions import MCPExtension, load_extension
|
from hindsight_api.extensions import MCPExtension, load_extension
|
||||||
from hindsight_api.extensions.tenant import AuthenticationError
|
from hindsight_api.extensions.tenant import AuthenticationError
|
||||||
from hindsight_api.mcp_tools import MCPToolsConfig, register_mcp_tools
|
from hindsight_api.mcp_tools import MCPToolsConfig, register_mcp_tools
|
||||||
from hindsight_api.models import RequestContext
|
from hindsight_api.models import RequestContext
|
||||||
|
|
||||||
|
# All tools available in the system (explicit list — no wildcards)
|
||||||
|
_ALL_TOOLS: frozenset[str] = frozenset(
|
||||||
|
{
|
||||||
|
"retain",
|
||||||
|
"recall",
|
||||||
|
"reflect",
|
||||||
|
"list_banks",
|
||||||
|
"create_bank",
|
||||||
|
"list_mental_models",
|
||||||
|
"get_mental_model",
|
||||||
|
"create_mental_model",
|
||||||
|
"update_mental_model",
|
||||||
|
"delete_mental_model",
|
||||||
|
"refresh_mental_model",
|
||||||
|
"list_directives",
|
||||||
|
"create_directive",
|
||||||
|
"delete_directive",
|
||||||
|
"list_memories",
|
||||||
|
"get_memory",
|
||||||
|
"delete_memory",
|
||||||
|
"list_documents",
|
||||||
|
"get_document",
|
||||||
|
"delete_document",
|
||||||
|
"list_operations",
|
||||||
|
"get_operation",
|
||||||
|
"cancel_operation",
|
||||||
|
"list_tags",
|
||||||
|
"get_bank",
|
||||||
|
"get_bank_stats",
|
||||||
|
"update_bank",
|
||||||
|
"delete_bank",
|
||||||
|
"clear_memories",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
# Configure logging from HINDSIGHT_API_LOG_LEVEL environment variable
|
# Configure logging from HINDSIGHT_API_LOG_LEVEL environment variable
|
||||||
_log_level_str = os.environ.get("HINDSIGHT_API_LOG_LEVEL", "info").lower()
|
_log_level_str = os.environ.get("HINDSIGHT_API_LOG_LEVEL", "info").lower()
|
||||||
_log_level_map = {
|
_log_level_map = {
|
||||||
|
|
@ -82,16 +118,11 @@ def create_mcp_server(memory: MemoryEngine, multi_bank: bool = True) -> FastMCP:
|
||||||
"""
|
"""
|
||||||
mcp = FastMCP("hindsight-mcp-server")
|
mcp = FastMCP("hindsight-mcp-server")
|
||||||
|
|
||||||
# Configure and register tools using shared module
|
global_config = _get_raw_config()
|
||||||
config = MCPToolsConfig(
|
|
||||||
bank_id_resolver=get_current_bank_id,
|
# Tools available for this mode (multi-bank exposes all tools; single-bank excludes bank-management tools)
|
||||||
api_key_resolver=get_current_api_key, # Propagate API key for tenant auth
|
_SINGLE_BANK_TOOLS: frozenset[str] = frozenset(
|
||||||
tenant_id_resolver=get_current_tenant_id, # Propagate tenant_id for usage metering
|
{
|
||||||
api_key_id_resolver=get_current_api_key_id, # Propagate api_key_id for usage metering
|
|
||||||
include_bank_id_param=multi_bank,
|
|
||||||
tools=None
|
|
||||||
if multi_bank
|
|
||||||
else {
|
|
||||||
"retain",
|
"retain",
|
||||||
"recall",
|
"recall",
|
||||||
"reflect",
|
"reflect",
|
||||||
|
|
@ -118,7 +149,23 @@ def create_mcp_server(memory: MemoryEngine, multi_bank: bool = True) -> FastMCP:
|
||||||
"update_bank",
|
"update_bank",
|
||||||
"delete_bank",
|
"delete_bank",
|
||||||
"clear_memories",
|
"clear_memories",
|
||||||
}, # Scoped tools for single-bank mode (excludes multi-bank management: list_banks, create_bank, get_bank_stats)
|
}
|
||||||
|
)
|
||||||
|
base_tools: frozenset[str] | None = None if multi_bank else _SINGLE_BANK_TOOLS
|
||||||
|
|
||||||
|
# Apply global mcp_enabled_tools filter (env-level allowlist)
|
||||||
|
if global_config.mcp_enabled_tools is not None:
|
||||||
|
allowed = frozenset(global_config.mcp_enabled_tools)
|
||||||
|
base_tools = (base_tools if base_tools is not None else _ALL_TOOLS) & allowed
|
||||||
|
|
||||||
|
# Configure and register tools using shared module
|
||||||
|
config = MCPToolsConfig(
|
||||||
|
bank_id_resolver=get_current_bank_id,
|
||||||
|
api_key_resolver=get_current_api_key, # Propagate API key for tenant auth
|
||||||
|
tenant_id_resolver=get_current_tenant_id, # Propagate tenant_id for usage metering
|
||||||
|
api_key_id_resolver=get_current_api_key_id, # Propagate api_key_id for usage metering
|
||||||
|
include_bank_id_param=multi_bank,
|
||||||
|
tools=base_tools,
|
||||||
retain_fire_and_forget=False, # HTTP MCP supports sync/async modes
|
retain_fire_and_forget=False, # HTTP MCP supports sync/async modes
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -232,6 +232,7 @@ ENV_LOG_LEVEL = "HINDSIGHT_API_LOG_LEVEL"
|
||||||
ENV_LOG_FORMAT = "HINDSIGHT_API_LOG_FORMAT"
|
ENV_LOG_FORMAT = "HINDSIGHT_API_LOG_FORMAT"
|
||||||
ENV_WORKERS = "HINDSIGHT_API_WORKERS"
|
ENV_WORKERS = "HINDSIGHT_API_WORKERS"
|
||||||
ENV_MCP_ENABLED = "HINDSIGHT_API_MCP_ENABLED"
|
ENV_MCP_ENABLED = "HINDSIGHT_API_MCP_ENABLED"
|
||||||
|
ENV_MCP_ENABLED_TOOLS = "HINDSIGHT_API_MCP_ENABLED_TOOLS"
|
||||||
ENV_ENABLE_BANK_CONFIG_API = "HINDSIGHT_API_ENABLE_BANK_CONFIG_API"
|
ENV_ENABLE_BANK_CONFIG_API = "HINDSIGHT_API_ENABLE_BANK_CONFIG_API"
|
||||||
ENV_GRAPH_RETRIEVER = "HINDSIGHT_API_GRAPH_RETRIEVER"
|
ENV_GRAPH_RETRIEVER = "HINDSIGHT_API_GRAPH_RETRIEVER"
|
||||||
ENV_MPFP_TOP_K_NEIGHBORS = "HINDSIGHT_API_MPFP_TOP_K_NEIGHBORS"
|
ENV_MPFP_TOP_K_NEIGHBORS = "HINDSIGHT_API_MPFP_TOP_K_NEIGHBORS"
|
||||||
|
|
@ -397,6 +398,7 @@ DEFAULT_LOG_LEVEL = "info"
|
||||||
DEFAULT_LOG_FORMAT = "text" # Options: "text", "json"
|
DEFAULT_LOG_FORMAT = "text" # Options: "text", "json"
|
||||||
DEFAULT_WORKERS = 1
|
DEFAULT_WORKERS = 1
|
||||||
DEFAULT_MCP_ENABLED = True
|
DEFAULT_MCP_ENABLED = True
|
||||||
|
DEFAULT_MCP_ENABLED_TOOLS: list[str] | None = None # None = all tools enabled
|
||||||
DEFAULT_ENABLE_BANK_CONFIG_API = True
|
DEFAULT_ENABLE_BANK_CONFIG_API = True
|
||||||
DEFAULT_GRAPH_RETRIEVER = "link_expansion" # Options: "link_expansion", "mpfp", "bfs"
|
DEFAULT_GRAPH_RETRIEVER = "link_expansion" # Options: "link_expansion", "mpfp", "bfs"
|
||||||
DEFAULT_MPFP_TOP_K_NEIGHBORS = 20 # Fan-out limit per node in MPFP graph traversal
|
DEFAULT_MPFP_TOP_K_NEIGHBORS = 20 # Fan-out limit per node in MPFP graph traversal
|
||||||
|
|
@ -638,6 +640,7 @@ class HindsightConfig:
|
||||||
log_level: str
|
log_level: str
|
||||||
log_format: str
|
log_format: str
|
||||||
mcp_enabled: bool
|
mcp_enabled: bool
|
||||||
|
mcp_enabled_tools: list[str] | None # None = all tools; explicit list = allowlist
|
||||||
enable_bank_config_api: bool
|
enable_bank_config_api: bool
|
||||||
|
|
||||||
# Recall
|
# Recall
|
||||||
|
|
@ -757,6 +760,8 @@ class HindsightConfig:
|
||||||
# These fields are manually tagged as safe to expose and modify.
|
# These fields are manually tagged as safe to expose and modify.
|
||||||
# Excludes credentials, infrastructure config, provider/model selection, and performance tuning.
|
# Excludes credentials, infrastructure config, provider/model selection, and performance tuning.
|
||||||
_CONFIGURABLE_FIELDS = {
|
_CONFIGURABLE_FIELDS = {
|
||||||
|
# MCP tool access control
|
||||||
|
"mcp_enabled_tools",
|
||||||
# Retention settings (behavioral)
|
# Retention settings (behavioral)
|
||||||
"retain_chunk_size",
|
"retain_chunk_size",
|
||||||
"retain_extraction_mode",
|
"retain_extraction_mode",
|
||||||
|
|
@ -1033,6 +1038,9 @@ class HindsightConfig:
|
||||||
log_level=os.getenv(ENV_LOG_LEVEL, DEFAULT_LOG_LEVEL),
|
log_level=os.getenv(ENV_LOG_LEVEL, DEFAULT_LOG_LEVEL),
|
||||||
log_format=os.getenv(ENV_LOG_FORMAT, DEFAULT_LOG_FORMAT).lower(),
|
log_format=os.getenv(ENV_LOG_FORMAT, DEFAULT_LOG_FORMAT).lower(),
|
||||||
mcp_enabled=os.getenv(ENV_MCP_ENABLED, str(DEFAULT_MCP_ENABLED)).lower() == "true",
|
mcp_enabled=os.getenv(ENV_MCP_ENABLED, str(DEFAULT_MCP_ENABLED)).lower() == "true",
|
||||||
|
mcp_enabled_tools=[t.strip() for t in os.getenv(ENV_MCP_ENABLED_TOOLS).split(",") if t.strip()]
|
||||||
|
if os.getenv(ENV_MCP_ENABLED_TOOLS)
|
||||||
|
else DEFAULT_MCP_ENABLED_TOOLS,
|
||||||
enable_bank_config_api=os.getenv(ENV_ENABLE_BANK_CONFIG_API, str(DEFAULT_ENABLE_BANK_CONFIG_API)).lower()
|
enable_bank_config_api=os.getenv(ENV_ENABLE_BANK_CONFIG_API, str(DEFAULT_ENABLE_BANK_CONFIG_API)).lower()
|
||||||
== "true",
|
== "true",
|
||||||
# Recall
|
# Recall
|
||||||
|
|
|
||||||
|
|
@ -239,6 +239,7 @@ def main():
|
||||||
log_level=args.log_level,
|
log_level=args.log_level,
|
||||||
log_format=config.log_format,
|
log_format=config.log_format,
|
||||||
mcp_enabled=config.mcp_enabled,
|
mcp_enabled=config.mcp_enabled,
|
||||||
|
mcp_enabled_tools=config.mcp_enabled_tools,
|
||||||
enable_bank_config_api=config.enable_bank_config_api,
|
enable_bank_config_api=config.enable_bank_config_api,
|
||||||
graph_retriever=config.graph_retriever,
|
graph_retriever=config.graph_retriever,
|
||||||
mpfp_top_k_neighbors=config.mpfp_top_k_neighbors,
|
mpfp_top_k_neighbors=config.mpfp_top_k_neighbors,
|
||||||
|
|
|
||||||
|
|
@ -265,6 +265,55 @@ def register_mcp_tools(
|
||||||
if "clear_memories" in tools_to_register:
|
if "clear_memories" in tools_to_register:
|
||||||
_register_clear_memories(mcp, memory, config)
|
_register_clear_memories(mcp, memory, config)
|
||||||
|
|
||||||
|
_apply_bank_tool_filtering(mcp, memory, config)
|
||||||
|
|
||||||
|
|
||||||
|
def _apply_bank_tool_filtering(mcp: FastMCP, memory: MemoryEngine, config: MCPToolsConfig) -> None:
|
||||||
|
"""Filter bank-level mcp_enabled_tools from both tools/list and tool invocation.
|
||||||
|
|
||||||
|
Wraps _tool_manager.get_tools() so that:
|
||||||
|
- tools/list only returns permitted tools (they are hidden, not just blocked)
|
||||||
|
- tools/call for a disabled tool raises NotFoundError (via the manager) before run()
|
||||||
|
|
||||||
|
tool.run wrappers are kept as defense-in-depth for any caller that bypasses the manager.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
tool_manager = mcp._tool_manager
|
||||||
|
original_get_tools = tool_manager.get_tools
|
||||||
|
|
||||||
|
async def _filtered_get_tools():
|
||||||
|
all_tools = await original_get_tools()
|
||||||
|
bank_id = config.bank_id_resolver()
|
||||||
|
if not bank_id:
|
||||||
|
return all_tools
|
||||||
|
request_context = _get_request_context(config)
|
||||||
|
bank_cfg = await memory._config_resolver.get_bank_config(bank_id, request_context)
|
||||||
|
enabled: list[str] | None = bank_cfg.get("mcp_enabled_tools")
|
||||||
|
if enabled is None:
|
||||||
|
return all_tools
|
||||||
|
enabled_set = set(enabled)
|
||||||
|
return {k: v for k, v in all_tools.items() if k in enabled_set}
|
||||||
|
|
||||||
|
setattr(tool_manager, "get_tools", _filtered_get_tools)
|
||||||
|
|
||||||
|
# Defense-in-depth: also wrap tool.run for any direct caller that bypasses the manager
|
||||||
|
for name, tool in tool_manager._tools.items():
|
||||||
|
original_run = tool.run
|
||||||
|
|
||||||
|
async def _filtered_run(arguments, _name=name, _orig=original_run):
|
||||||
|
bank_id = config.bank_id_resolver()
|
||||||
|
if bank_id:
|
||||||
|
request_context = _get_request_context(config)
|
||||||
|
bank_cfg = await memory._config_resolver.get_bank_config(bank_id, request_context)
|
||||||
|
enabled: list[str] | None = bank_cfg.get("mcp_enabled_tools")
|
||||||
|
if enabled is not None and _name not in enabled:
|
||||||
|
raise ValueError(f"Tool '{_name}' is not enabled for bank '{bank_id}'")
|
||||||
|
return await _orig(arguments)
|
||||||
|
|
||||||
|
object.__setattr__(tool, "run", _filtered_run)
|
||||||
|
except (AttributeError, KeyError) as e:
|
||||||
|
logger.warning(f"Could not apply bank tool filtering: {e}")
|
||||||
|
|
||||||
|
|
||||||
def _register_retain(mcp: FastMCP, memory: MemoryEngine, config: MCPToolsConfig) -> None:
|
def _register_retain(mcp: FastMCP, memory: MemoryEngine, config: MCPToolsConfig) -> None:
|
||||||
"""Register the retain tool."""
|
"""Register the retain tool."""
|
||||||
|
|
|
||||||
|
|
@ -82,7 +82,7 @@ async def test_hierarchical_fields_categorization():
|
||||||
assert "disposition_empathy" in configurable
|
assert "disposition_empathy" in configurable
|
||||||
|
|
||||||
# Verify count is correct
|
# Verify count is correct
|
||||||
assert len(configurable) == 10
|
assert len(configurable) == 11
|
||||||
|
|
||||||
# Verify credential fields (NEVER exposed)
|
# Verify credential fields (NEVER exposed)
|
||||||
assert "llm_api_key" in credentials
|
assert "llm_api_key" in credentials
|
||||||
|
|
|
||||||
|
|
@ -352,6 +352,69 @@ async def test_middleware_handles_both_endpoints(mock_memory):
|
||||||
assert "create_bank" not in single_bank_tools
|
assert "create_bank" not in single_bank_tools
|
||||||
|
|
||||||
|
|
||||||
|
def test_global_mcp_enabled_tools_filter_restricts_registered_tools(mock_memory):
|
||||||
|
"""Test that global mcp_enabled_tools env setting restricts which tools are registered."""
|
||||||
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
from hindsight_api.api.mcp import create_mcp_server
|
||||||
|
|
||||||
|
mock_cfg = MagicMock()
|
||||||
|
mock_cfg.mcp_enabled_tools = ["retain", "recall"]
|
||||||
|
|
||||||
|
with patch("hindsight_api.api.mcp._get_raw_config", return_value=mock_cfg):
|
||||||
|
mcp_server = create_mcp_server(mock_memory, multi_bank=True)
|
||||||
|
|
||||||
|
tools = mcp_server._tool_manager._tools
|
||||||
|
assert "retain" in tools
|
||||||
|
assert "recall" in tools
|
||||||
|
assert "reflect" not in tools
|
||||||
|
assert "list_banks" not in tools
|
||||||
|
assert "create_bank" not in tools
|
||||||
|
assert "list_mental_models" not in tools
|
||||||
|
|
||||||
|
|
||||||
|
def test_global_mcp_enabled_tools_none_exposes_all_tools(mock_memory):
|
||||||
|
"""Test that mcp_enabled_tools=None (default) exposes all tools."""
|
||||||
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
from hindsight_api.api.mcp import create_mcp_server
|
||||||
|
|
||||||
|
mock_cfg = MagicMock()
|
||||||
|
mock_cfg.mcp_enabled_tools = None
|
||||||
|
|
||||||
|
with patch("hindsight_api.api.mcp._get_raw_config", return_value=mock_cfg):
|
||||||
|
mcp_server = create_mcp_server(mock_memory, multi_bank=True)
|
||||||
|
|
||||||
|
tools = mcp_server._tool_manager._tools
|
||||||
|
assert "retain" in tools
|
||||||
|
assert "recall" in tools
|
||||||
|
assert "reflect" in tools
|
||||||
|
assert "list_banks" in tools
|
||||||
|
assert "create_bank" in tools
|
||||||
|
|
||||||
|
|
||||||
|
def test_global_mcp_enabled_tools_intersects_with_single_bank_mode(mock_memory):
|
||||||
|
"""Test that global filter intersects with single-bank mode tool set.
|
||||||
|
|
||||||
|
list_banks is in the global allowlist but NOT in single-bank mode, so it
|
||||||
|
should be absent from the final registered set.
|
||||||
|
"""
|
||||||
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
from hindsight_api.api.mcp import create_mcp_server
|
||||||
|
|
||||||
|
mock_cfg = MagicMock()
|
||||||
|
mock_cfg.mcp_enabled_tools = ["retain", "recall", "list_banks"]
|
||||||
|
|
||||||
|
with patch("hindsight_api.api.mcp._get_raw_config", return_value=mock_cfg):
|
||||||
|
mcp_server = create_mcp_server(mock_memory, multi_bank=False)
|
||||||
|
|
||||||
|
tools = mcp_server._tool_manager._tools
|
||||||
|
assert "retain" in tools
|
||||||
|
assert "recall" in tools
|
||||||
|
assert "list_banks" not in tools # single-bank mode excludes it regardless
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_routing_logic_from_url_path():
|
async def test_routing_logic_from_url_path():
|
||||||
"""Test that routing correctly selects server based on URL structure.
|
"""Test that routing correctly selects server based on URL structure.
|
||||||
|
|
|
||||||
|
|
@ -1246,3 +1246,114 @@ class TestEmptyListReturns:
|
||||||
mcp = _make_mcp_server(mock_memory, {"list_tags"}, include_bank_id=True)
|
mcp = _make_mcp_server(mock_memory, {"list_tags"}, include_bank_id=True)
|
||||||
result = await _tools(mcp)["list_tags"].fn()
|
result = await _tools(mcp)["list_tags"].fn()
|
||||||
assert '"items": []' in result or "[]" in result
|
assert '"items": []' in result or "[]" in result
|
||||||
|
|
||||||
|
# =========================================================================
|
||||||
|
# Bank-Level Tool Filtering Tests
|
||||||
|
# =========================================================================
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def mock_memory_with_resolver():
|
||||||
|
"""Create a mock MemoryEngine with config resolver for bank filtering tests."""
|
||||||
|
memory = MagicMock()
|
||||||
|
memory.retain_batch_async = AsyncMock()
|
||||||
|
memory.recall_async = AsyncMock(
|
||||||
|
return_value=MagicMock(
|
||||||
|
model_dump_json=lambda indent=None: '{"results": []}',
|
||||||
|
model_dump=lambda: {"results": []},
|
||||||
|
)
|
||||||
|
)
|
||||||
|
memory._config_resolver = MagicMock()
|
||||||
|
memory._config_resolver.get_bank_config = AsyncMock(return_value={})
|
||||||
|
return memory
|
||||||
|
|
||||||
|
|
||||||
|
class TestBankToolFiltering:
|
||||||
|
"""Tests for bank-level mcp_enabled_tools filtering via _apply_bank_tool_filtering."""
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_disallowed_tool_raises_error(self, mock_memory_with_resolver):
|
||||||
|
"""Tool not in bank's mcp_enabled_tools list is hidden from get_tools()."""
|
||||||
|
from fastmcp import FastMCP
|
||||||
|
|
||||||
|
mock_memory_with_resolver._config_resolver.get_bank_config = AsyncMock(
|
||||||
|
return_value={"mcp_enabled_tools": ["retain"]}
|
||||||
|
)
|
||||||
|
|
||||||
|
mcp = FastMCP("test")
|
||||||
|
config = MCPToolsConfig(
|
||||||
|
bank_id_resolver=lambda: "test-bank",
|
||||||
|
include_bank_id_param=False,
|
||||||
|
tools={"retain", "recall"},
|
||||||
|
)
|
||||||
|
register_mcp_tools(mcp, mock_memory_with_resolver, config)
|
||||||
|
|
||||||
|
# Both tools are registered in the manager's internal dict
|
||||||
|
assert "recall" in mcp._tool_manager._tools
|
||||||
|
|
||||||
|
# But get_tools() (used by tools/list and tools/call) filters it out
|
||||||
|
visible = await mcp._tool_manager.get_tools()
|
||||||
|
assert "retain" in visible
|
||||||
|
assert "recall" not in visible
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_allowed_tool_remains_visible(self, mock_memory_with_resolver):
|
||||||
|
"""Tool in bank's mcp_enabled_tools list stays visible in get_tools()."""
|
||||||
|
from fastmcp import FastMCP
|
||||||
|
|
||||||
|
mock_memory_with_resolver._config_resolver.get_bank_config = AsyncMock(
|
||||||
|
return_value={"mcp_enabled_tools": ["retain", "recall"]}
|
||||||
|
)
|
||||||
|
|
||||||
|
mcp = FastMCP("test")
|
||||||
|
config = MCPToolsConfig(
|
||||||
|
bank_id_resolver=lambda: "test-bank",
|
||||||
|
include_bank_id_param=False,
|
||||||
|
tools={"retain", "recall"},
|
||||||
|
)
|
||||||
|
register_mcp_tools(mcp, mock_memory_with_resolver, config)
|
||||||
|
|
||||||
|
visible = await mcp._tool_manager.get_tools()
|
||||||
|
assert "retain" in visible
|
||||||
|
assert "recall" in visible
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_no_filter_when_mcp_enabled_tools_absent(self, mock_memory_with_resolver):
|
||||||
|
"""When bank config has no mcp_enabled_tools key, all tools remain visible."""
|
||||||
|
from fastmcp import FastMCP
|
||||||
|
|
||||||
|
mock_memory_with_resolver._config_resolver.get_bank_config = AsyncMock(return_value={})
|
||||||
|
|
||||||
|
mcp = FastMCP("test")
|
||||||
|
config = MCPToolsConfig(
|
||||||
|
bank_id_resolver=lambda: "test-bank",
|
||||||
|
include_bank_id_param=False,
|
||||||
|
tools={"retain", "recall"},
|
||||||
|
)
|
||||||
|
register_mcp_tools(mcp, mock_memory_with_resolver, config)
|
||||||
|
|
||||||
|
visible = await mcp._tool_manager.get_tools()
|
||||||
|
assert "retain" in visible
|
||||||
|
assert "recall" in visible
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_filter_skipped_when_no_bank_id(self, mock_memory_with_resolver):
|
||||||
|
"""When bank_id resolver returns None, config is not fetched and all tools are visible."""
|
||||||
|
from fastmcp import FastMCP
|
||||||
|
|
||||||
|
mock_memory_with_resolver._config_resolver.get_bank_config = AsyncMock(
|
||||||
|
return_value={"mcp_enabled_tools": ["retain"]} # Would block recall
|
||||||
|
)
|
||||||
|
|
||||||
|
mcp = FastMCP("test")
|
||||||
|
config = MCPToolsConfig(
|
||||||
|
bank_id_resolver=lambda: None, # No bank_id context
|
||||||
|
include_bank_id_param=False,
|
||||||
|
tools={"retain", "recall"},
|
||||||
|
)
|
||||||
|
register_mcp_tools(mcp, mock_memory_with_resolver, config)
|
||||||
|
|
||||||
|
visible = await mcp._tool_manager.get_tools()
|
||||||
|
# Filter bypassed — config resolver was never consulted, all tools visible
|
||||||
|
assert "recall" in visible
|
||||||
|
mock_memory_with_resolver._config_resolver.get_bank_config.assert_not_called()
|
||||||
|
|
|
||||||
|
|
@ -38,6 +38,46 @@ type ObservationsEdits = {
|
||||||
observations_mission: string | null;
|
observations_mission: string | null;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
type MCPEdits = {
|
||||||
|
mcp_enabled_tools: string[] | null;
|
||||||
|
};
|
||||||
|
|
||||||
|
// ─── MCP tool catalogue ───────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
const MCP_TOOL_GROUPS: { label: string; tools: string[] }[] = [
|
||||||
|
{ label: "Core", tools: ["retain", "recall", "reflect"] },
|
||||||
|
{
|
||||||
|
label: "Bank management",
|
||||||
|
tools: [
|
||||||
|
"list_banks",
|
||||||
|
"create_bank",
|
||||||
|
"get_bank",
|
||||||
|
"get_bank_stats",
|
||||||
|
"update_bank",
|
||||||
|
"delete_bank",
|
||||||
|
"clear_memories",
|
||||||
|
],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
label: "Mental models",
|
||||||
|
tools: [
|
||||||
|
"list_mental_models",
|
||||||
|
"get_mental_model",
|
||||||
|
"create_mental_model",
|
||||||
|
"update_mental_model",
|
||||||
|
"delete_mental_model",
|
||||||
|
"refresh_mental_model",
|
||||||
|
],
|
||||||
|
},
|
||||||
|
{ label: "Directives", tools: ["list_directives", "create_directive", "delete_directive"] },
|
||||||
|
{ label: "Memories", tools: ["list_memories", "get_memory", "delete_memory"] },
|
||||||
|
{ label: "Documents", tools: ["list_documents", "get_document", "delete_document"] },
|
||||||
|
{ label: "Operations", tools: ["list_operations", "get_operation", "cancel_operation"] },
|
||||||
|
{ label: "Tags", tools: ["list_tags"] },
|
||||||
|
];
|
||||||
|
|
||||||
|
const ALL_TOOLS: string[] = MCP_TOOL_GROUPS.flatMap((g) => g.tools);
|
||||||
|
|
||||||
// ─── Slice helpers ────────────────────────────────────────────────────────────
|
// ─── Slice helpers ────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
function retainSlice(config: Record<string, any>): RetainEdits {
|
function retainSlice(config: Record<string, any>): RetainEdits {
|
||||||
|
|
@ -56,6 +96,12 @@ function observationsSlice(config: Record<string, any>): ObservationsEdits {
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function mcpSlice(config: Record<string, any>): MCPEdits {
|
||||||
|
return {
|
||||||
|
mcp_enabled_tools: config.mcp_enabled_tools ?? null,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
const DEFAULT_PROFILE: ProfileData = {
|
const DEFAULT_PROFILE: ProfileData = {
|
||||||
reflect_mission: "",
|
reflect_mission: "",
|
||||||
disposition_skepticism: 3,
|
disposition_skepticism: 3,
|
||||||
|
|
@ -79,14 +125,17 @@ export function BankConfigView() {
|
||||||
observationsSlice({})
|
observationsSlice({})
|
||||||
);
|
);
|
||||||
const [reflectEdits, setReflectEdits] = useState<ProfileData>(DEFAULT_PROFILE);
|
const [reflectEdits, setReflectEdits] = useState<ProfileData>(DEFAULT_PROFILE);
|
||||||
|
const [mcpEdits, setMcpEdits] = useState<MCPEdits>(mcpSlice({}));
|
||||||
|
|
||||||
// Per-section saving/error state
|
// Per-section saving/error state
|
||||||
const [retainSaving, setRetainSaving] = useState(false);
|
const [retainSaving, setRetainSaving] = useState(false);
|
||||||
const [observationsSaving, setObservationsSaving] = useState(false);
|
const [observationsSaving, setObservationsSaving] = useState(false);
|
||||||
const [reflectSaving, setReflectSaving] = useState(false);
|
const [reflectSaving, setReflectSaving] = useState(false);
|
||||||
|
const [mcpSaving, setMcpSaving] = useState(false);
|
||||||
const [retainError, setRetainError] = useState<string | null>(null);
|
const [retainError, setRetainError] = useState<string | null>(null);
|
||||||
const [observationsError, setObservationsError] = useState<string | null>(null);
|
const [observationsError, setObservationsError] = useState<string | null>(null);
|
||||||
const [reflectError, setReflectError] = useState<string | null>(null);
|
const [reflectError, setReflectError] = useState<string | null>(null);
|
||||||
|
const [mcpError, setMcpError] = useState<string | null>(null);
|
||||||
|
|
||||||
// Reset dialog
|
// Reset dialog
|
||||||
|
|
||||||
|
|
@ -103,6 +152,10 @@ export function BankConfigView() {
|
||||||
() => JSON.stringify(reflectEdits) !== JSON.stringify(baseProfile),
|
() => JSON.stringify(reflectEdits) !== JSON.stringify(baseProfile),
|
||||||
[reflectEdits, baseProfile]
|
[reflectEdits, baseProfile]
|
||||||
);
|
);
|
||||||
|
const mcpDirty = useMemo(
|
||||||
|
() => JSON.stringify(mcpEdits) !== JSON.stringify(mcpSlice(baseConfig)),
|
||||||
|
[mcpEdits, baseConfig]
|
||||||
|
);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (bankId) loadAll();
|
if (bankId) loadAll();
|
||||||
|
|
@ -130,6 +183,7 @@ export function BankConfigView() {
|
||||||
setRetainEdits(retainSlice(cfg));
|
setRetainEdits(retainSlice(cfg));
|
||||||
setObservationsEdits(observationsSlice(cfg));
|
setObservationsEdits(observationsSlice(cfg));
|
||||||
setReflectEdits(prof);
|
setReflectEdits(prof);
|
||||||
|
setMcpEdits(mcpSlice(cfg));
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
console.error("Failed to load bank data:", err);
|
console.error("Failed to load bank data:", err);
|
||||||
} finally {
|
} finally {
|
||||||
|
|
@ -184,6 +238,20 @@ export function BankConfigView() {
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const saveMCP = async () => {
|
||||||
|
if (!bankId) return;
|
||||||
|
setMcpSaving(true);
|
||||||
|
setMcpError(null);
|
||||||
|
try {
|
||||||
|
await client.updateBankConfig(bankId, mcpEdits);
|
||||||
|
setBaseConfig((prev) => ({ ...prev, ...mcpEdits }));
|
||||||
|
} catch (err: any) {
|
||||||
|
setMcpError(err.message || "Failed to save MCP settings");
|
||||||
|
} finally {
|
||||||
|
setMcpSaving(false);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
if (!bankId) {
|
if (!bankId) {
|
||||||
return (
|
return (
|
||||||
<div className="flex items-center justify-center py-12">
|
<div className="flex items-center justify-center py-12">
|
||||||
|
|
@ -348,11 +416,139 @@ export function BankConfigView() {
|
||||||
onChange={(v) => setReflectEdits((prev) => ({ ...prev, disposition_empathy: v }))}
|
onChange={(v) => setReflectEdits((prev) => ({ ...prev, disposition_empathy: v }))}
|
||||||
/>
|
/>
|
||||||
</ConfigSection>
|
</ConfigSection>
|
||||||
|
|
||||||
|
{/* MCP Tools Section */}
|
||||||
|
<ConfigSection
|
||||||
|
title="MCP Tools"
|
||||||
|
description="Restrict which MCP tools this bank exposes to agents"
|
||||||
|
error={mcpError}
|
||||||
|
dirty={mcpDirty}
|
||||||
|
saving={mcpSaving}
|
||||||
|
onSave={saveMCP}
|
||||||
|
>
|
||||||
|
<FieldRow
|
||||||
|
label="Restrict tools"
|
||||||
|
description="When off, all tools are available. When on, only the selected tools can be invoked for this bank."
|
||||||
|
>
|
||||||
|
<div className="flex justify-end">
|
||||||
|
<Toggle
|
||||||
|
value={mcpEdits.mcp_enabled_tools !== null}
|
||||||
|
onChange={(restricted) =>
|
||||||
|
setMcpEdits({
|
||||||
|
mcp_enabled_tools: restricted ? [...ALL_TOOLS] : null,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
/>
|
||||||
|
</div>
|
||||||
|
</FieldRow>
|
||||||
|
{mcpEdits.mcp_enabled_tools !== null && (
|
||||||
|
<ToolSelector
|
||||||
|
selected={mcpEdits.mcp_enabled_tools}
|
||||||
|
onChange={(tools) => setMcpEdits({ mcp_enabled_tools: tools })}
|
||||||
|
/>
|
||||||
|
)}
|
||||||
|
</ConfigSection>
|
||||||
</div>
|
</div>
|
||||||
</>
|
</>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ─── ToolSelector ─────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
function ToolSelector({
|
||||||
|
selected,
|
||||||
|
onChange,
|
||||||
|
}: {
|
||||||
|
selected: string[];
|
||||||
|
onChange: (tools: string[]) => void;
|
||||||
|
}) {
|
||||||
|
const selectedSet = new Set(selected);
|
||||||
|
|
||||||
|
const toggleTool = (tool: string) => {
|
||||||
|
const next = new Set(selectedSet);
|
||||||
|
if (next.has(tool)) {
|
||||||
|
next.delete(tool);
|
||||||
|
} else {
|
||||||
|
next.add(tool);
|
||||||
|
}
|
||||||
|
onChange(ALL_TOOLS.filter((t) => next.has(t)));
|
||||||
|
};
|
||||||
|
|
||||||
|
const allSelected = ALL_TOOLS.every((t) => selectedSet.has(t));
|
||||||
|
const noneSelected = selected.length === 0;
|
||||||
|
|
||||||
|
const toggleAll = () => {
|
||||||
|
onChange(allSelected ? [] : [...ALL_TOOLS]);
|
||||||
|
};
|
||||||
|
|
||||||
|
return (
|
||||||
|
<div className="px-6 py-4 space-y-4">
|
||||||
|
<div className="flex items-center justify-between">
|
||||||
|
<p className="text-xs text-muted-foreground">
|
||||||
|
{selected.length} of {ALL_TOOLS.length} tools enabled
|
||||||
|
</p>
|
||||||
|
<button type="button" onClick={toggleAll} className="text-xs text-primary hover:underline">
|
||||||
|
{allSelected ? "Deselect all" : "Select all"}
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
<div className="space-y-4">
|
||||||
|
{MCP_TOOL_GROUPS.map((group) => {
|
||||||
|
const groupSelected = group.tools.filter((t) => selectedSet.has(t)).length;
|
||||||
|
const groupAll = groupSelected === group.tools.length;
|
||||||
|
return (
|
||||||
|
<div key={group.label}>
|
||||||
|
<div className="flex items-center justify-between mb-1.5">
|
||||||
|
<p className="text-xs font-medium text-muted-foreground uppercase tracking-wide">
|
||||||
|
{group.label}
|
||||||
|
</p>
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
onClick={() => {
|
||||||
|
const next = new Set(selectedSet);
|
||||||
|
if (groupAll) {
|
||||||
|
group.tools.forEach((t) => next.delete(t));
|
||||||
|
} else {
|
||||||
|
group.tools.forEach((t) => next.add(t));
|
||||||
|
}
|
||||||
|
onChange(ALL_TOOLS.filter((t) => next.has(t)));
|
||||||
|
}}
|
||||||
|
className="text-xs text-primary hover:underline"
|
||||||
|
>
|
||||||
|
{groupAll ? "Deselect" : "Select all"}
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
<div className="flex flex-wrap gap-2">
|
||||||
|
{group.tools.map((tool) => {
|
||||||
|
const active = selectedSet.has(tool);
|
||||||
|
return (
|
||||||
|
<button
|
||||||
|
key={tool}
|
||||||
|
type="button"
|
||||||
|
onClick={() => toggleTool(tool)}
|
||||||
|
className={`px-2.5 py-1 rounded text-xs font-mono transition-colors border ${
|
||||||
|
active
|
||||||
|
? "bg-primary text-primary-foreground border-primary"
|
||||||
|
: "bg-muted/30 text-muted-foreground border-border/40 hover:border-primary/40"
|
||||||
|
}`}
|
||||||
|
>
|
||||||
|
{tool}
|
||||||
|
</button>
|
||||||
|
);
|
||||||
|
})}
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
})}
|
||||||
|
</div>
|
||||||
|
{noneSelected && (
|
||||||
|
<p className="text-xs text-destructive">
|
||||||
|
Warning: no tools selected — agents will be blocked from all MCP calls for this bank.
|
||||||
|
</p>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
// ─── ConfigSection ────────────────────────────────────────────────────────────
|
// ─── ConfigSection ────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
function ConfigSection({
|
function ConfigSection({
|
||||||
|
|
|
||||||
|
|
@ -799,10 +799,36 @@ Configuration for MCP server endpoints.
|
||||||
| Variable | Description | Default |
|
| Variable | Description | Default |
|
||||||
|----------|-------------|---------|
|
|----------|-------------|---------|
|
||||||
| `HINDSIGHT_API_MCP_ENABLED` | Enable MCP server at `/mcp/{bank_id}/` | `true` |
|
| `HINDSIGHT_API_MCP_ENABLED` | Enable MCP server at `/mcp/{bank_id}/` | `true` |
|
||||||
|
| `HINDSIGHT_API_MCP_ENABLED_TOOLS` | Comma-separated allowlist of MCP tools to expose globally (empty = all tools) | - |
|
||||||
| `HINDSIGHT_API_MCP_AUTH_TOKEN` | Bearer token for MCP authentication (optional) | - |
|
| `HINDSIGHT_API_MCP_AUTH_TOKEN` | Bearer token for MCP authentication (optional) | - |
|
||||||
| `HINDSIGHT_API_MCP_LOCAL_BANK_ID` | Memory bank ID for local MCP | `mcp` |
|
| `HINDSIGHT_API_MCP_LOCAL_BANK_ID` | Memory bank ID for local MCP | `mcp` |
|
||||||
| `HINDSIGHT_API_MCP_INSTRUCTIONS` | Additional instructions appended to retain/recall tool descriptions | - |
|
| `HINDSIGHT_API_MCP_INSTRUCTIONS` | Additional instructions appended to retain/recall tool descriptions | - |
|
||||||
|
|
||||||
|
**Tool Access Control:**
|
||||||
|
|
||||||
|
`HINDSIGHT_API_MCP_ENABLED_TOOLS` restricts which MCP tools are registered at the server level. This is useful for read-only deployments or limiting surface area:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
# Expose only recall (read-only deployment)
|
||||||
|
export HINDSIGHT_API_MCP_ENABLED_TOOLS=recall
|
||||||
|
|
||||||
|
# Expose recall and reflect only
|
||||||
|
export HINDSIGHT_API_MCP_ENABLED_TOOLS=recall,reflect
|
||||||
|
```
|
||||||
|
|
||||||
|
Available tool names: `retain`, `recall`, `reflect`, `list_banks`, `create_bank`, `list_mental_models`, `get_mental_model`, `create_mental_model`, `update_mental_model`, `delete_mental_model`, `refresh_mental_model`, `list_directives`, `create_directive`, `delete_directive`, `list_memories`, `get_memory`, `delete_memory`, `list_documents`, `get_document`, `delete_document`, `list_operations`, `get_operation`, `cancel_operation`, `list_tags`, `get_bank`, `get_bank_stats`, `update_bank`, `delete_bank`, `clear_memories`.
|
||||||
|
|
||||||
|
This can also be overridden per bank via the [config API](#hierarchical-configuration):
|
||||||
|
|
||||||
|
```bash
|
||||||
|
# Restrict a specific bank to read-only MCP access
|
||||||
|
curl -X PATCH http://localhost:8888/v1/default/banks/my-bank/config \
|
||||||
|
-H "Content-Type: application/json" \
|
||||||
|
-d '{"updates": {"mcp_enabled_tools": ["recall"]}}'
|
||||||
|
```
|
||||||
|
|
||||||
|
When a bank-level `mcp_enabled_tools` is set, tools not in the list return a clear error when invoked (they still appear in the tools list for MCP protocol compatibility).
|
||||||
|
|
||||||
**MCP Authentication:**
|
**MCP Authentication:**
|
||||||
|
|
||||||
By default, the MCP endpoint is open. For production deployments, set `HINDSIGHT_API_MCP_AUTH_TOKEN` to require Bearer token authentication:
|
By default, the MCP endpoint is open. For production deployments, set `HINDSIGHT_API_MCP_AUTH_TOKEN` to require Bearer token authentication:
|
||||||
|
|
@ -983,6 +1009,7 @@ Configuration fields are categorized for security:
|
||||||
1. **Configurable Fields** - Safe behavioral settings that can be customized per-bank:
|
1. **Configurable Fields** - Safe behavioral settings that can be customized per-bank:
|
||||||
- Retention: `retain_chunk_size`, `retain_extraction_mode`, `retain_mission`, `retain_custom_instructions`
|
- Retention: `retain_chunk_size`, `retain_extraction_mode`, `retain_mission`, `retain_custom_instructions`
|
||||||
- Observations: `enable_observations`, `observations_mission`
|
- Observations: `enable_observations`, `observations_mission`
|
||||||
|
- MCP access control: `mcp_enabled_tools`
|
||||||
|
|
||||||
2. **Credential Fields** - NEVER exposed or configurable via API:
|
2. **Credential Fields** - NEVER exposed or configurable via API:
|
||||||
- API keys: `*_api_key` (all LLM API keys)
|
- API keys: `*_api_key` (all LLM API keys)
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue