From a3e1a557c341fca43b38ac737ca4a3d11e00c720 Mon Sep 17 00:00:00 2001 From: Marcus van Elst Date: Tue, 14 Apr 2026 09:11:10 +0200 Subject: [PATCH] fix: use share paths, drop additional from list_dir, remove debug logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: DSM expects share paths (/dev) not volume paths (/volume1/dev). The 408 errors were triggered by any additional field on the wrong path format. - list_shares: use share["path"] directly (e.g. /dev), drop real_path from additional — only volume_status remains - list_dir: remove additional parameter entirely; table now shows name + type (isdir is returned by default); update docstring to show share path examples - client.py: remove diagnostic REQUEST and RAW ERROR stderr logging - tests: update assertions to match share paths and two-column table output Co-Authored-By: Claude Sonnet 4.6 --- src/mcp_synology_filestation/client.py | 7 --- .../tools/filestation.py | 51 ++++++------------- tests/test_tools_filestation.py | 31 +++-------- 3 files changed, 23 insertions(+), 66 deletions(-) diff --git a/src/mcp_synology_filestation/client.py b/src/mcp_synology_filestation/client.py index 54de69a..10ea729 100644 --- a/src/mcp_synology_filestation/client.py +++ b/src/mcp_synology_filestation/client.py @@ -262,11 +262,6 @@ class FileStationClient: info = self._api_cache[api] resolved_version = version if version is not None else info["maxVersion"] url = f"{self._base_url}/webapi/{info['path']}" - sys.stderr.write( - f"[dsm] REQUEST {api}/{method} v{resolved_version} " - f"(NAS max={info['maxVersion']})\n" - ) - sys.stderr.flush() req_params: dict[str, Any] = { "api": api, @@ -300,8 +295,6 @@ class FileStationClient: code = body.get("error", {}).get("code", 0) logger.debug("DSM response: %s/%s — error code %d", api, method, code) - sys.stderr.write(f"[dsm] RAW ERROR {api}/{method}: {body!r}\n") - sys.stderr.flush() # Transparent re-auth on session errors (one retry only) if code in _SESSION_ERROR_CODES and not _is_retry and self._auth_manager: diff --git a/src/mcp_synology_filestation/tools/filestation.py b/src/mcp_synology_filestation/tools/filestation.py index dedbfb5..34afe78 100644 --- a/src/mcp_synology_filestation/tools/filestation.py +++ b/src/mcp_synology_filestation/tools/filestation.py @@ -69,7 +69,7 @@ def register_filestation( data = await client.request( "SYNO.FileStation.List", "list_share", - params={"additional": json.dumps(["real_path", "volume_status"])}, + params={"additional": json.dumps(["volume_status"])}, ) except SynologyError as e: return f"Error: {e}" @@ -82,7 +82,7 @@ def register_filestation( rows = [] for share in shares: name = share.get("name", "") - path = share.get("additional", {}).get("real_path") or share.get("path", "") + path = share.get("path", f"/{name}") vol = share.get("additional", {}).get("volume_status", {}) total = vol.get("totalspace") used = vol.get("usedspace") @@ -119,8 +119,11 @@ def register_filestation( ) -> str: """List the contents of a directory on the NAS. + Use share paths as returned by list_shares (e.g. "/dev", "/data"), + not volume paths (e.g. "/volume1/dev" will not work). + Args: - path: Absolute path on the NAS (e.g. "/volume1/data"). + path: Share-relative path on the NAS (e.g. "/dev" or "/data/photos"). offset: Number of items to skip (for pagination). limit: Maximum items to return (1-500, default 100). sort_by: Sort field — one of: name, size, user, group, mtime, atime, @@ -128,8 +131,8 @@ def register_filestation( sort_direction: "asc" or "desc". Returns: - Formatted table with name, type, size, and modification time, - plus the total item count for pagination context. + Formatted table with name and type, plus the total item count + for pagination context. """ from mcp_synology_filestation.client import SynologyError @@ -151,7 +154,6 @@ def register_filestation( "limit": limit, "sort_by": sort_by, "sort_direction": sort_direction, - "additional": "size,time", # DEBUG: comma-separated (no json.dumps) }, ) except SynologyError as e: @@ -163,45 +165,22 @@ def register_filestation( if not files: return f"Directory '{path}' is empty (or does not exist)." - # Build table + # Build table — name + type only (no additional fields requested) rows = [] for f in files: name = f.get("name", "") - is_dir = f.get("isdir", False) - ftype = "dir" if is_dir else "file" - add = f.get("additional", {}) - size_val = add.get("size") - size_str = "-" if is_dir else _fmt_size(size_val) - mtime = add.get("time", {}).get("mtime") - mtime_str = _fmt_time(mtime) - rows.append((name, ftype, size_str, mtime_str)) + ftype = "dir" if f.get("isdir", False) else "file" + rows.append((name, ftype)) w_name = max(len("Name"), *(len(r[0]) for r in rows)) w_type = max(len("Type"), *(len(r[1]) for r in rows)) - w_size = max(len("Size"), *(len(r[2]) for r in rows)) - w_mtime = max(len("Modified"), *(len(r[3]) for r in rows)) - sep = ( - f"+{'-' * (w_name + 2)}" - f"+{'-' * (w_type + 2)}" - f"+{'-' * (w_size + 2)}" - f"+{'-' * (w_mtime + 2)}+" - ) - header = ( - f"| {'Name':<{w_name}} " - f"| {'Type':<{w_type}} " - f"| {'Size':<{w_size}} " - f"| {'Modified':<{w_mtime}} |" - ) + sep = f"+{'-' * (w_name + 2)}+{'-' * (w_type + 2)}+" + header = f"| {'Name':<{w_name}} | {'Type':<{w_type}} |" lines = [f"Path: {path}", sep, header, sep] - for name, ftype, size_str, mtime_str in rows: - lines.append( - f"| {name:<{w_name}} " - f"| {ftype:<{w_type}} " - f"| {size_str:<{w_size}} " - f"| {mtime_str:<{w_mtime}} |" - ) + for name, ftype in rows: + lines.append(f"| {name:<{w_name}} | {ftype:<{w_type}} |") lines.append(sep) end = offset + len(files) diff --git a/tests/test_tools_filestation.py b/tests/test_tools_filestation.py index f63feef..8be5309 100644 --- a/tests/test_tools_filestation.py +++ b/tests/test_tools_filestation.py @@ -79,7 +79,8 @@ async def test_list_shares_success(config: AppConfig) -> None: result = await tools["list_shares"]() assert "data" in result - assert "/volume1/data" in result + assert "/data" in result # share path, not volume path + assert "/volume1/data" not in result assert "photos" in result assert "2 share(s) found" in result # Check usage percentage appears @@ -120,33 +121,21 @@ async def test_list_shares_dsm_error(config: AppConfig) -> None: @pytest.mark.asyncio async def test_list_dir_success(config: AppConfig) -> None: - """list_dir returns a formatted table with files and directories.""" + """list_dir returns a formatted table with name and type columns.""" client = MagicMock() client.request = AsyncMock( return_value={ "total": 3, "files": [ - { - "name": "documents", - "isdir": True, - "additional": {"size": None, "time": {"mtime": 1700000000}}, - }, - { - "name": "photo.jpg", - "isdir": False, - "additional": {"size": 2_500_000, "time": {"mtime": 1700100000}}, - }, - { - "name": "readme.txt", - "isdir": False, - "additional": {"size": 1024, "time": {"mtime": 1700200000}}, - }, + {"name": "documents", "isdir": True}, + {"name": "photo.jpg", "isdir": False}, + {"name": "readme.txt", "isdir": False}, ], } ) tools = _make_mcp_and_tools(config, client) - result = await tools["list_dir"](path="/volume1/data") + result = await tools["list_dir"](path="/dev") assert "documents" in result assert "photo.jpg" in result @@ -164,11 +153,7 @@ async def test_list_dir_pagination(config: AppConfig) -> None: return_value={ "total": 200, "files": [ - { - "name": f"file{i}.txt", - "isdir": False, - "additional": {"size": 100, "time": {"mtime": 1700000000 + i}}, - } + {"name": f"file{i}.txt", "isdir": False} for i in range(100) ], }