diff --git a/docs/tools.md b/docs/tools.md index 95492a5..8738658 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -50,9 +50,9 @@ | `ListProfilesTool` | `list_profiles` | List all available profiles | | `ShareFileTool` | `share_file` | Copy an existing local file into session files and return a download link | | `ContentPublishTool` | `content_publish` | Register an existing session file for inline viewing in chat | -| `McpStatusTool` | `mcp_status` | Check connectivity and list tools for configured MCP servers | +| `McpStatusTool` | `mcp_status` | Check connectivity and list tools for configured MCP servers (marks servers accepting per-user keys) | | `CreateMcpServerTool` | `create_mcp_server` | Scaffold a new MCP server directory with boilerplate | -| `TestMcpToolTool` | `test_mcp_tool` | Execute a single MCP tool call in isolation for diagnostics | +| `TestMcpToolTool` | `test_mcp_tool` | Execute a single MCP tool call in isolation for diagnostics; for BYOK servers runs on the current user's saved key | | `ReflectTool` | `reflect` | Self-reflection and analysis | | `PeerTool` | `peer` | Swarm communication: `list` machines, `status` of one peer, `ask` a peer a question (answered by a real agent run on the other side) | | `PlanTool` | `plan` | Agent-invoked planning: fresh plan or re-plan with a `reason` (and optional `updated_goal`). The tool result instructs the agent to wait for user confirmation when the task is complex, and to proceed immediately otherwise | diff --git a/navi/tools/mcp_status.py b/navi/tools/mcp_status.py index cb04542..6317c3d 100644 --- a/navi/tools/mcp_status.py +++ b/navi/tools/mcp_status.py @@ -36,10 +36,17 @@ lines: list[str] = [] for name, client in manager.clients.items(): + cfg = manager._get_configs().get(name) if not client.connected: lines.append(f"Server: {name} (disconnected)") continue lines.append(f"Server: {name} (connected)") + if cfg is not None and cfg.user_key is not None: + dest = cfg.user_key.header or cfg.user_key.env + lines.append( + f" (accepts a per-user key via {dest} — without one this " + "server runs on the shared default)" + ) try: tools = await client.list_tools() for t in tools: diff --git a/navi/tools/test_mcp_tool.py b/navi/tools/test_mcp_tool.py index 80eb4d0..00ff419 100644 --- a/navi/tools/test_mcp_tool.py +++ b/navi/tools/test_mcp_tool.py @@ -2,7 +2,7 @@ import asyncio -from ._internal.base import Tool, ToolContext, ToolResult +from ._internal.base import Tool, ToolContext, ToolResult, current_user_id class TestMcpToolTool(Tool): @@ -13,7 +13,9 @@ "Returns the raw output and whether the tool reported an error. " "Always use this after writing or editing an MCP server to verify " "each tool works before reporting success to the user. " - "Do NOT use mcp_status for testing individual tools — mcp_status is for discovery only." + "Do NOT use mcp_status for testing individual tools — mcp_status is for discovery only. " + "For servers with a per-user key slot (BYOK), the call runs on the " + "current user's saved key — test results reflect what this user sees." ) parameters = { "type": "object", @@ -81,8 +83,11 @@ ) try: + uid = ctx.user_id if ctx else None + if uid is None: + uid = current_user_id.get() output, is_error = await asyncio.wait_for( - manager.call_tool(server_name, tool_name, arguments), + manager.call_tool(server_name, tool_name, arguments, user_id=uid), timeout=30.0, ) except asyncio.TimeoutError: diff --git a/tests/unit/tools/test_mcp_meta_tools.py b/tests/unit/tools/test_mcp_meta_tools.py new file mode 100644 index 0000000..48c5b1f --- /dev/null +++ b/tests/unit/tools/test_mcp_meta_tools.py @@ -0,0 +1,79 @@ +""""test_mcp_tool / mcp_status after the BYOK change — user key semantics.""" + +from unittest.mock import MagicMock + +from navi.mcp.config import McpServerConfig, McpUserKey +from navi.tools._internal.base import current_user_id, ToolContext +from navi.tools.mcp_status import McpStatusTool +from navi.tools.test_mcp_tool import TestMcpToolTool + + +def _configs(): + return { + "http-server": McpServerConfig( + transport="streamable_http", + url="https://x", + user_key=McpUserKey(header="Authorization", prefix="Bearer "), + ), + "plain": McpServerConfig(transport="sse", url="https://p"), + } + + +class RecordingManager: + def __init__(self): + self.calls: list = [] + self._configs_dict = _configs() + + @property + def clients(self): + client = MagicMock() + client.connected = True + return {"http-server": client, "plain": client} + + def _get_configs(self): + return self._configs_dict + + async def call_tool(self, server_name, tool_name, arguments=None, *, user_id=None): + self.calls.append((server_name, tool_name, arguments, user_id)) + return ("ok", False) + + +class TestTestMcpToolForwarding: + async def test_forwards_ctx_user_id(self): + manager = RecordingManager() + tool = TestMcpToolTool(mcp_manager=manager) + result = await tool.execute( + {"server_name": "http-server", "tool_name": "t"}, + ToolContext(session_id="s1", user_id="u9"), + ) + assert result.success + assert manager.calls == [("http-server", "t", {}, "u9")] + + async def test_forwards_contextvar_user_id_without_ctx(self): + manager = RecordingManager() + tool = TestMcpToolTool(mcp_manager=manager) + token = current_user_id.set("u7") + try: + await tool.execute({"server_name": "http-server", "tool_name": "t"}) + finally: + current_user_id.reset(token) + assert manager.calls == [("http-server", "t", {}, "u7")] + + async def test_no_user_resolves_none(self): + manager = RecordingManager() + tool = TestMcpToolTool(mcp_manager=manager) + await tool.execute({"server_name": "http-server", "tool_name": "t"}) + assert manager.calls == [("http-server", "t", {}, None)] + + +class TestMcpStatusByok: + async def test_marks_byok_servers(self): + manager = RecordingManager() + tool = McpStatusTool(mcp_manager=manager) + result = await tool.execute({}) + lines = result.output.splitlines() + idx_http = next(i for i, l in enumerate(lines) if "http-server" in l) + idx_plain = next(i for i, l in enumerate(lines) if "plain " in l) + assert "accepts a per-user key via Authorization" in lines[idx_http + 1] + assert "accepts a per-user key" not in lines[idx_plain] + assert "accepts a per-user key" not in lines[idx_plain + 1] \ No newline at end of file