diff --git a/fastmcp_slim/fastmcp/server/providers/filesystem_discovery.py b/fastmcp_slim/fastmcp/server/providers/filesystem_discovery.py index a79050264..8e4929ba9 100644 --- a/fastmcp_slim/fastmcp/server/providers/filesystem_discovery.py +++ b/fastmcp_slim/fastmcp/server/providers/filesystem_discovery.py @@ -13,6 +13,7 @@ import hashlib import importlib.util import sys from dataclasses import dataclass, field +from importlib.machinery import ModuleSpec from pathlib import Path from types import ModuleType @@ -116,6 +117,26 @@ def _compute_module_name(file_path: Path, package_root: Path) -> str: return ".".join(parts) +def _package_path_matches(module: ModuleType, package_root: Path) -> bool: + """Check whether a package module's __path__ points at package_root. + + Used to tell whether a top-level package name already present in + sys.modules belongs to this provider (same directory) or to a different + provider that happens to share the package name. + """ + module_paths = getattr(module, "__path__", None) + if not module_paths: + return False + package_root = package_root.resolve() + return any(Path(p).resolve() == package_root for p in module_paths) + + +def _private_package_prefix(directory: Path) -> str: + """Compute a collision-safe synthetic package name anchored at a directory.""" + digest = hashlib.sha1(str(directory.resolve()).encode()).hexdigest()[:12] + return f"_fastmcp_pkg_{digest}" + + def import_module_from_file( file_path: Path, provider_root: Path | None = None ) -> ModuleType: @@ -150,6 +171,35 @@ def import_module_from_file( # Import as part of a package module_name = _compute_module_name(file_path, package_root) + # If another provider has already registered this top-level package name + # from a different directory, importing normally would resolve against + # that provider's directory (wrong file, or ModuleNotFoundError for a + # sibling that only exists here). Isolate this provider's tree under a + # private anchor package so both providers coexist. The anchor is a + # namespace package whose __path__ points at this provider's tree, so + # importlib resolves every intermediate package and relative import + # normally beneath it. + top_name = module_name.split(".")[0] + existing_top = sys.modules.get(top_name) + if existing_top is not None and not _package_path_matches( + existing_top, package_root + ): + anchor = _private_package_prefix(package_root.parent) + if anchor not in sys.modules: + spec = ModuleSpec(anchor, loader=None, is_package=True) + anchor_module = importlib.util.module_from_spec(spec) + anchor_module.__path__ = [str(package_root.parent)] + sys.modules[anchor] = anchor_module + private_name = f"{anchor}.{module_name}" + try: + if private_name in sys.modules: + return importlib.reload(sys.modules[private_name]) + return importlib.import_module(private_name) + except ImportError as e: + raise ImportError( + f"Failed to import {module_name} from {file_path}: {e}" + ) from e + # Temporarily add package root's parent to sys.path for the import package_parent = str(package_root.parent) path_added = package_parent not in sys.path diff --git a/tests/fs/test_provider.py b/tests/fs/test_provider.py index cce887a28..b115c5ee7 100644 --- a/tests/fs/test_provider.py +++ b/tests/fs/test_provider.py @@ -459,6 +459,100 @@ def charge(amount: float) -> str: names = {t.name for t in tools_list} assert names == {"greet", "charge"} + async def test_mounted_servers_with_same_package_and_module_name( + self, tmp_path: Path + ): + """Two providers sharing a package/module path should not collide. + + Reproduces the exact scenario from the bug report: both providers have a + `components/tools.py` package. Without isolation the second provider + reuses the first's cached module and `list_tools` returns the first + server's tool twice. + """ + for server_name, tool_name in [ + ("server1", "server1_tool"), + ("server2", "server2_tool"), + ]: + components = tmp_path / server_name / "components" + components.mkdir(parents=True) + (components / "__init__.py").write_text("") + (components / "tools.py").write_text( + f"""\ +from fastmcp.tools import tool + +@tool +def {tool_name}() -> str: + return "{tool_name}" +""" + ) + + server1 = FastMCP( + "server1", + providers=[FileSystemProvider(tmp_path / "server1" / "components")], + ) + server2 = FastMCP( + "server2", + providers=[FileSystemProvider(tmp_path / "server2" / "components")], + ) + parent = FastMCP("parent") + parent.mount(server1) + parent.mount(server2) + + tools = await parent.list_tools() + assert {t.name for t in tools} == {"server1_tool", "server2_tool"} + + result = await parent.call_tool("server2_tool", {}) + assert "server2_tool" in str(result) + + async def test_mounted_servers_with_same_package_different_module( + self, tmp_path: Path + ): + """Same package name but different module filenames must coexist. + + The package directory name (`components`) collides while the leaf module + differs (`a.py` vs `b.py`). The shared package must not resolve to the + first provider's directory, which would drop the second provider's tool. + A relative import inside each module exercises that the isolated tree is + a real package. + """ + for server_name, module_name, tool_name in [ + ("server1", "a", "server1_tool"), + ("server2", "b", "server2_tool"), + ]: + components = tmp_path / server_name / "components" + components.mkdir(parents=True) + (components / "__init__.py").write_text("") + (components / "_shared.py").write_text(f'LABEL = "{tool_name}"\n') + (components / f"{module_name}.py").write_text( + f"""\ +from fastmcp.tools import tool + +from ._shared import LABEL + +@tool +def {tool_name}() -> str: + return LABEL +""" + ) + + server1 = FastMCP( + "server1", + providers=[FileSystemProvider(tmp_path / "server1" / "components")], + ) + server2 = FastMCP( + "server2", + providers=[FileSystemProvider(tmp_path / "server2" / "components")], + ) + parent = FastMCP("parent") + parent.mount(server1) + parent.mount(server2) + + tools = await parent.list_tools() + assert {t.name for t in tools} == {"server1_tool", "server2_tool"} + + result = await parent.call_tool("server2_tool", {}) + assert "server2_tool" in str(result) + class TestFileSystemProviderVersioning: """Tests for version propagation through FileSystemProvider."""