From 23853680a6b7c0e2c2c3ac793d3c60a3d5e0ad31 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 25 May 2026 18:07:08 +0000 Subject: [PATCH 1/3] Make listfiles entry path relative to base directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'path' field on each entry was previously relative to the listed subdirectory, which caused callers (and LLMs) to fail when passing it back to mcpproxy__getfile — the parent prefix was missing for nested entries. Anchor 'path' to the base files directory so it is always directly usable with getfile, and call that out in the tool description. --- builtin_tools.py | 8 ++++---- server.py | 6 +++++- tests/test_builtin_tools.py | 19 +++++++++++++++++++ 3 files changed, 28 insertions(+), 5 deletions(-) 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..6733de7 100644 --- a/tests/test_builtin_tools.py +++ b/tests/test_builtin_tools.py @@ -199,6 +199,25 @@ 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.""" + base = tmp_path / "files" + sub = base / "playwright" + sub.mkdir(parents=True) + (sub / "snap.yml").write_text("y") + _set_base(monkeypatch, base) + from builtin_tools import list_files, get_file + result = await list_files(_ctx(), path="playwright") + entry = next(e for e in result["entries"] if e["name"] == "snap.yml") + assert entry["path"] == "playwright/snap.yml" + fetched = await get_file(_ctx(), path=entry["path"]) + assert fetched["ok"] is True + assert fetched["content"] == "y" + @pytest.mark.asyncio async def test_recursive_does_not_follow_dir_symlinks(self, tmp_path: Path, monkeypatch): base = tmp_path / "files" From 9b720679c57bae5650b278e47229cf50f7e2bb89 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 25 May 2026 18:09:35 +0000 Subject: [PATCH 2/3] test_mcp_client.sh: use entry path for display and getfile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The listing renderer and the follow-up getfile call both used 'name' (basename), so files inside subdirectories were shown without their parent prefix and the getfile lookup failed. Use 'path' (relative to the base files directory) — which the listfiles tool now returns — falling back to 'name' for compatibility. --- tests/test_mcp_client.sh | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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'): From 95680293dd71f4471bdc848f6c5c5b4f16b07457 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 25 May 2026 18:12:44 +0000 Subject: [PATCH 3/3] Broaden listfiles path round-trip test Use a generic 'test-paths' subdirectory and verify both a base-level file and a nested file round-trip through list_files -> get_file using only the returned 'path' field, both when listing the subdir directly and when listing the root recursively. --- tests/test_builtin_tools.py | 45 ++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/tests/test_builtin_tools.py b/tests/test_builtin_tools.py index 6733de7..fd3358f 100644 --- a/tests/test_builtin_tools.py +++ b/tests/test_builtin_tools.py @@ -203,20 +203,45 @@ async def test_recursive_max_depth(self, tmp_path: Path, monkeypatch): 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.""" + """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 / "playwright" + sub = base / "test-paths" sub.mkdir(parents=True) - (sub / "snap.yml").write_text("y") + 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 - result = await list_files(_ctx(), path="playwright") - entry = next(e for e in result["entries"] if e["name"] == "snap.yml") - assert entry["path"] == "playwright/snap.yml" - fetched = await get_file(_ctx(), path=entry["path"]) - assert fetched["ok"] is True - assert fetched["content"] == "y" + + # 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):