diff --git a/builtin_tools.py b/builtin_tools.py index 0df87a3..1d3ba9b 100644 --- a/builtin_tools.py +++ b/builtin_tools.py @@ -54,9 +54,9 @@ async def list_files( """List files and subdirectories at *path* inside the files base directory. Returns a JSON object with an ``entries`` list; each entry has ``name`` - (basename), ``path`` (relative to the listed directory, using ``/`` as the - separator), ``type`` (``"file"`` or ``"directory"``), and ``size`` (bytes, - files only). + (basename), ``path`` (relative to the **base files directory**, using ``/`` + as the separator — this is the value to pass to ``mcpproxy__getfile``), + ``type`` (``"file"`` or ``"directory"``), and ``size`` (bytes, files only). When *recursive* is true (default), descends into subdirectories. Each directory is still emitted as its own entry (with ``type="directory"``) @@ -85,7 +85,7 @@ async def list_files( def _walk(directory: Path, depth: int) -> None: for entry in sorted(directory.iterdir()): is_dir = entry.is_dir() and not entry.is_symlink() - rel = entry.relative_to(target).as_posix() + rel = entry.relative_to(base).as_posix() entries.append( { "name": entry.name, diff --git a/server.py b/server.py index 49440ca..9919ea4 100755 --- a/server.py +++ b/server.py @@ -528,7 +528,11 @@ def register_builtin_tools() -> None: "(default: /app/files, override with MCPPROXY_FILES_DIR). " "Use this to discover screenshots, JSON snapshots, and other files " "produced by package providers such as the Playwright MCP server. " - "Pass a subdirectory path to drill down." + "Pass a subdirectory path to drill down. " + "Each returned entry has a 'path' field (relative to the base " + "files directory) — pass that value directly to mcpproxy__getfile " + "to read the file. Do NOT use just the 'name' (basename) for " + "nested entries, or the file will not be found." ), "input_schema": { "type": "object", diff --git a/tests/test_builtin_tools.py b/tests/test_builtin_tools.py index 667dd6b..fd3358f 100644 --- a/tests/test_builtin_tools.py +++ b/tests/test_builtin_tools.py @@ -199,6 +199,50 @@ async def test_recursive_max_depth(self, tmp_path: Path, monkeypatch): paths = {e["path"] for e in result["entries"]} assert paths == {"sub", "sub/a.txt", "sub/deep"} + @pytest.mark.asyncio + async def test_entry_path_is_relative_to_base_not_listed_dir( + self, tmp_path: Path, monkeypatch + ): + """Entry 'path' must be passable directly to get_file regardless of + which subdirectory was listed. Writes one file at the base and one in + a nested subdir, then round-trips both through list_files -> get_file + using only the returned 'path' field.""" + base = tmp_path / "files" + sub = base / "test-paths" + sub.mkdir(parents=True) + root_file = base / "root.txt" + root_file.write_text("root-content") + nested_file = sub / "nested.txt" + nested_file.write_text("nested-content") + + _set_base(monkeypatch, base) + from builtin_tools import list_files, get_file + + # Listing a subdirectory: 'path' should still be base-relative. + sub_listing = await list_files(_ctx(), path="test-paths") + nested_entry = next( + e for e in sub_listing["entries"] if e["name"] == "nested.txt" + ) + assert nested_entry["path"] == "test-paths/nested.txt" + fetched_nested = await get_file(_ctx(), path=nested_entry["path"]) + assert fetched_nested["ok"] is True + assert fetched_nested["content"] == "nested-content" + + # Recursive listing from the root: every entry's 'path' must round-trip + # through get_file unchanged for files (directories return an error). + root_listing = await list_files(_ctx(), recursive=True) + paths = {e["path"] for e in root_listing["entries"]} + assert paths == {"root.txt", "test-paths", "test-paths/nested.txt"} + for entry in root_listing["entries"]: + if entry["type"] != "file": + continue + fetched = await get_file(_ctx(), path=entry["path"]) + assert fetched["ok"] is True, f"failed to fetch {entry['path']}" + # Cross-check the root file specifically. + root_entry = next(e for e in root_listing["entries"] if e["path"] == "root.txt") + fetched_root = await get_file(_ctx(), path=root_entry["path"]) + assert fetched_root["content"] == "root-content" + @pytest.mark.asyncio async def test_recursive_does_not_follow_dir_symlinks(self, tmp_path: Path, monkeypatch): base = tmp_path / "files" diff --git a/tests/test_mcp_client.sh b/tests/test_mcp_client.sh index a639a37..21231b9 100755 --- a/tests/test_mcp_client.sh +++ b/tests/test_mcp_client.sh @@ -906,12 +906,15 @@ files_fetched = [] for entry in entries: if entry.get('type') != 'file': continue - fname = entry['name'] + # 'path' is relative to base_dir and includes any parent subdirectories, + # so it's the value to pass back to mcpproxy__getfile. Fall back to 'name' + # for older servers that don't populate 'path'. + fpath = entry.get('path') or entry['name'] file_result = _extract( - _call_tool('mcpproxy__getfile', {'path': fname}) + _call_tool('mcpproxy__getfile', {'path': fpath}) ) files_fetched.append({ - 'name': fname, + 'name': fpath, 'size': entry.get('size'), 'ok': file_result.get('ok', True), 'encoding': file_result.get('encoding', 'text'), @@ -948,7 +951,8 @@ try: for e in entries: icon = '📁' if e.get('type') == 'directory' else '📄' size = f\" ({e['size']} bytes)\" if e.get('size') is not None else '' - print(f\" {icon} {e['name']}{size}\") + label = e.get('path') or e.get('name', '') + print(f\" {icon} {label}{size}\") print() for f in files: if f.get('error'):