diff --git a/mcp-servers/navi-3d/app/mcp_server.py b/mcp-servers/navi-3d/app/mcp_server.py index d5793b1..cbdd9c2 100644 --- a/mcp-servers/navi-3d/app/mcp_server.py +++ b/mcp-servers/navi-3d/app/mcp_server.py @@ -48,8 +48,8 @@ @mcp.tool(name="compile_scad") async def compile_scad_tool( session_id: Annotated[str, Field(description="Navi session ID — files are resolved inside session_files//.")], - source_path: Annotated[str, Field(description="Relative or absolute path to the .scad file.")], - output_path: Annotated[str, Field(description="Relative or absolute path for the output STL.")], + source_path: Annotated[str, Field(description="Just the filename, e.g. 'falcon9_rocket.scad'. The server resolves it inside session_files// automatically.")], + output_path: Annotated[str, Field(description="Just the filename, e.g. 'falcon9_rocket.stl'. The server resolves it inside session_files// automatically.")], ) -> str: """Compile an OpenSCAD script into a binary STL.""" result = await compile_scad(_settings(), session_id, source_path, output_path) @@ -59,7 +59,7 @@ @mcp.tool(name="render_stl") async def render_stl_tool( session_id: Annotated[str, Field(description="Navi session ID — files are resolved inside session_files//.")], - source_path: Annotated[str, Field(description="Path to the STL file to render.")], + source_path: Annotated[str, Field(description="Just the filename, e.g. 'falcon9_rocket.stl'. The server resolves it inside session_files// automatically.")], views: Annotated[list[str] | None, Field(description="Camera views: front, back, top, bottom, left, right, iso. Max 3.")] = None, ) -> str: """Render preview PNG images from an STL file.""" @@ -70,7 +70,7 @@ @mcp.tool(name="lint_scad") async def lint_scad_tool( session_id: Annotated[str, Field(description="Navi session ID — files are resolved inside session_files//.")], - source_path: Annotated[str, Field(description="Path to the .scad file.")], + source_path: Annotated[str, Field(description="Just the filename, e.g. 'falcon9_rocket.scad'. The server resolves it inside session_files// automatically.")], strict: Annotated[bool, Field(description="Treat warnings as errors.")] = False, ) -> str: """Lint an OpenSCAD source file before compiling.""" diff --git a/navi/core/context_builder.py b/navi/core/context_builder.py index 4b5d962..ca56719 100644 --- a/navi/core/context_builder.py +++ b/navi/core/context_builder.py @@ -279,7 +279,10 @@ f"[Session context]\n" f"Session ID: {session_id}\n" f"Session files directory: {_config.settings.session_files_dir}/{session_id}/\n" - f"When writing files the user should see, always use the session directory path above." + f"When writing files the user should see, always use the session directory path above.\n" + f"When calling MCP navi-3d tools (compile_scad, render_stl, lint_scad), pass ONLY the filename " + f"(e.g. 'falcon9_rocket.scad') for source_path and output_path. Do NOT include the session_id or " + f"the session_files directory in those paths — the MCP server resolves them automatically." ) system_msg = Message(role="system", content=system_prompt) conv = [m for m in session_context if m.role != "system"] diff --git a/navi/core/subagent_runner.py b/navi/core/subagent_runner.py index 4302e65..d147e18 100644 --- a/navi/core/subagent_runner.py +++ b/navi/core/subagent_runner.py @@ -139,7 +139,10 @@ f"Parent Session ID: {parent_session_id}\n" f"Session files directory: {settings.session_files_dir}/{parent_session_id}/\n" "For files the user should see, write to this exact session directory. " - "Do not use or invent a subagent_* directory." + "Do not use or invent a subagent_* directory.\n" + "When calling MCP navi-3d tools (compile_scad, render_stl, lint_scad), pass ONLY the filename " + "(e.g. 'falcon9_rocket.scad') for source_path and output_path. Do NOT include the session_id or " + "the session_files directory in those paths — the MCP server resolves them automatically." ) if not sys_parts: sys_parts.append(profile.system_prompt) diff --git a/navi/mcp/tools.py b/navi/mcp/tools.py index fe698a7..e151b61 100644 --- a/navi/mcp/tools.py +++ b/navi/mcp/tools.py @@ -2,7 +2,10 @@ from typing import Any -from navi.tools._internal.base import Tool, ToolResult +from pathlib import Path + +from navi.config import settings +from navi.tools._internal.base import Tool, ToolResult, current_session_id from .manager import McpManager @@ -29,10 +32,41 @@ self._manager = manager self.name = f"mcp:{server_name}:{tool_name}" + @staticmethod + def _normalize_path_param(value: str) -> str: + """Strip session-files prefix so MCP servers receive bare filenames. + + When the LLM passes a full relative path like + ``session_files//falcon9_rocket.scad`` the MCP server would + resolve it a second time, creating double nesting. We keep only + the basename. + """ + p = Path(value) + # If it looks like a nested path, take just the filename + if len(p.parts) > 1: + return p.name + return value + async def execute(self, params: dict[str, Any]) -> ToolResult: + # Defensive copy — never mutate the caller's dict + forwarded = dict(params) + + # 1. Force the real session_id from the agent context so the LLM + # cannot hallucinate a wrong UUID (ghost-session bug). + sid = current_session_id.get() + if sid is not None: + forwarded["session_id"] = sid + + # 2. For navi-3d, normalize source/output paths to bare filenames + # to prevent double path nesting. + if self.server_name == "navi-3d": + for key in ("source_path", "output_path"): + if key in forwarded: + forwarded[key] = self._normalize_path_param(forwarded[key]) + try: output, is_error = await self._manager.call_tool( - self.server_name, self.tool_name, params + self.server_name, self.tool_name, forwarded ) if is_error: return ToolResult( diff --git a/tests/unit/test_mcp.py b/tests/unit/test_mcp.py index f6b7424..f73a3b2 100644 --- a/tests/unit/test_mcp.py +++ b/tests/unit/test_mcp.py @@ -150,3 +150,94 @@ result = await tool.execute({}) assert not result.success assert "server down" in result.error + + async def test_execute_injects_session_id(self): + from navi.tools._internal.base import current_session_id + + mock_manager = AsyncMock(spec=McpManager) + mock_manager.call_tool.return_value = ("ok", False) + tool = McpTool( + server_name="book", + tool_name="search", + description="", + parameters={}, + manager=mock_manager, + ) + token = current_session_id.set("real-session-id") + try: + result = await tool.execute({"query": "foo"}) + assert result.success + mock_manager.call_tool.assert_awaited_once_with( + "book", "search", {"query": "foo", "session_id": "real-session-id"} + ) + finally: + current_session_id.reset(token) + + async def test_execute_overwrites_hallucinated_session_id(self): + from navi.tools._internal.base import current_session_id + + mock_manager = AsyncMock(spec=McpManager) + mock_manager.call_tool.return_value = ("ok", False) + tool = McpTool( + server_name="book", + tool_name="search", + description="", + parameters={}, + manager=mock_manager, + ) + token = current_session_id.set("real-session-id") + try: + result = await tool.execute({"session_id": "fake-id", "query": "foo"}) + assert result.success + args = mock_manager.call_tool.call_args[0][2] + assert args["session_id"] == "real-session-id" + finally: + current_session_id.reset(token) + + async def test_execute_normalizes_navi3d_paths(self): + from navi.tools._internal.base import current_session_id + + mock_manager = AsyncMock(spec=McpManager) + mock_manager.call_tool.return_value = ("ok", False) + tool = McpTool( + server_name="navi-3d", + tool_name="compile_scad", + description="", + parameters={}, + manager=mock_manager, + ) + token = current_session_id.set("real-session-id") + try: + result = await tool.execute({ + "source_path": "session_files/real-session-id/falcon9_rocket.scad", + "output_path": "session_files/real-session-id/falcon9_rocket.stl", + }) + assert result.success + args = mock_manager.call_tool.call_args[0][2] + assert args["source_path"] == "falcon9_rocket.scad" + assert args["output_path"] == "falcon9_rocket.stl" + assert args["session_id"] == "real-session-id" + finally: + current_session_id.reset(token) + + async def test_execute_does_not_modify_paths_for_non_navi3d(self): + from navi.tools._internal.base import current_session_id + + mock_manager = AsyncMock(spec=McpManager) + mock_manager.call_tool.return_value = ("ok", False) + tool = McpTool( + server_name="other-server", + tool_name="do_stuff", + description="", + parameters={}, + manager=mock_manager, + ) + token = current_session_id.set("real-session-id") + try: + result = await tool.execute({"path": "some/nested/path.txt"}) + assert result.success + args = mock_manager.call_tool.call_args[0][2] + assert args["path"] == "some/nested/path.txt" + assert args["session_id"] == "real-session-id" + finally: + current_session_id.reset(token)