feat(filestation): expose additional param on three list/search tools

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Added
+
+- **`additional` parameter exposed on `list_shares`, `list_files`, and `search_files` MCP tools** (#87) — closes #41. The underlying handlers in `modules/filestation/listing.py` and `modules/filestation/search.py` already accepted an `additional: list[str] | None` parameter, but the FastMCP tool registrations in `modules/filestation/__init__.py` never surfaced it — callers couldn't request DSM metadata fields beyond the per-tool defaults. Same class of spec-vs-tool drift as #33 (which added `mtime_from`/`mtime_to` to `search_files`). Now the three tool registrations accept `additional: list[str] | None = None`, with a docstring per tool listing the supported field set; the underlying defaults (`["real_path", "size", "owner", "perm"]` for shares, `["size", "time"]` for files and search) are unchanged so existing callers see no behavior difference. New `validate_additional()` helper in `modules/filestation/helpers.py` rejects unknown values up-front against a union whitelist (`real_path`, `size`, `owner`, `time`, `perm`, `type`, `mount_point_type`, `volume_status`) — DSM silently ignores unknown values (the field just doesn't appear in the response), which would have made typos like `"sze"` invisible; validating up-front surfaces the typo as a clear ToolError naming the bad value and listing the supported set. The whitelist is a union across List + Search APIs; tighter per-API gating isn't worth the maintenance cost since DSM ignores values that don't apply (e.g. `mount_point_type` on a non-share path) without erroring. New tests: 12 cases in `TestValidateAdditional` (7 valid/empty accepted, 4 unknown rejected with clear errors, 1 verifying the tool name appears in the error envelope) plus six round-trip + reject-unknown tests across `TestListShares`, `TestListFiles`, and `TestSearchFiles`. The Search round-trip captures the `list` (poll) request, not `start` — DSM Search returns full file metadata at poll time, so `additional` is sent there. 599 unit tests pass at 96.25% coverage.
+
 ### Changed
 
 - **Remove Glama integration** (#88) — drops the Glama score badge from `README.md` and deletes the `glama.json` config file at the repo root. Glama's "Maintenance" sub-score is driven primarily by `issues responded to in the last N months`, which conflicts with this project's use of GitHub issues as a planned roadmap (the baggage tracker #51 + per-item issues are TODOs, not unanswered support requests). The badge therefore mis-represents project health to anyone landing on the README, and the heuristic isn't tunable from our side. Removing both rather than gaming the metric or adding a workflow note. PyPI / GitHub release / MCP registry remain the canonical distribution and discovery surfaces; the README still carries the standard PyPI / Python-versions / License / Tests / Coverage / Downloads badges plus per-installer and per-OS download breakdowns from `cmeans/pypi-winnow-downloads`.

src/mcp_synology/modules/filestation/__init__.py

@@ -268,11 +268,18 @@ def _desc(name: str) -> str:
         async def tool_list_shares(
             sort_by: str = "name",
             sort_direction: str = "asc",
+            additional: list[str] | None = None,
         ) -> str:
+            """`additional`: optional list of DSM metadata fields. Valid values:
+            real_path, size, owner, time, perm, mount_point_type, volume_status.
+            Defaults to ["real_path", "size", "owner", "perm"]. Unknown values
+            are rejected before reaching DSM.
+            """
             client = await manager.get_client()
             return manager.with_update_notice(
                 await list_shares(
                     client,
+                    additional=additional,
                     sort_by=sort_by,
                     sort_direction=sort_direction,
                     recycle_bin_status=recycle_status,
@@ -296,7 +303,12 @@ async def tool_list_files(
             sort_direction: str = "asc",
             offset: int = 0,
             limit: int = 200,
+            additional: list[str] | None = None,
         ) -> str:
+            """`additional`: optional list of DSM metadata fields. Valid values:
+            real_path, size, owner, time, perm, type, mount_point_type. Defaults
+            to ["size", "time"]. Unknown values are rejected before reaching DSM.
+            """
             client = await manager.get_client()
             return manager.with_update_notice(
                 await list_files(
@@ -308,6 +320,7 @@ async def tool_list_files(
                     sort_direction=sort_direction,
                     offset=offset,
                     limit=limit,
+                    additional=additional,
                     hide_recycle=hide_recycle,
                     file_type_indicator=indicator,
                 )
@@ -360,7 +373,12 @@ async def tool_search_files(
             exclude_pattern: str | None = None,
             recursive: bool = True,
             limit: int = 500,
+            additional: list[str] | None = None,
         ) -> str:
+            """`additional`: optional list of DSM metadata fields. Valid values:
+            real_path, size, owner, time, perm, type. Defaults to ["size", "time"].
+            Unknown values are rejected before reaching DSM.
+            """
             client = await manager.get_client()
             return manager.with_update_notice(
                 await search_files(
@@ -376,6 +394,7 @@ async def tool_search_files(
                     exclude_pattern=exclude_pattern,
                     recursive=recursive,
                     limit=limit,
+                    additional=additional,
                     file_type_indicator=indicator,
                     timeout=search_timeout,
                     poll_interval=search_poll_interval,

src/mcp_synology/modules/filestation/helpers.py

@@ -8,10 +8,53 @@
 from datetime import UTC, datetime
 
 from mcp_synology.core.client import DsmClient
-from mcp_synology.core.errors import SynologyError
+from mcp_synology.core.errors import ErrorCode, SynologyError
+from mcp_synology.core.formatting import error_response
 
 logger = logging.getLogger(__name__)
 
+# Union of `additional` field values DSM 7.x accepts across SYNO.FileStation.List
+# and SYNO.FileStation.Search. Tighter per-API whitelisting isn't worth the
+# maintenance cost — DSM silently ignores values that don't apply to a given
+# endpoint (e.g. `mount_point_type` on a non-share path simply doesn't appear
+# in the response), so the union acts as a typo-and-injection guard without
+# blocking valid use cases. Documented in `docs/specs/filestation-module-spec.md`.
+_VALID_ADDITIONAL_FIELDS: frozenset[str] = frozenset(
+    {
+        "real_path",
+        "size",
+        "owner",
+        "time",
+        "perm",
+        "type",
+        "mount_point_type",
+        "volume_status",
+    }
+)
+
+
+def validate_additional(values: list[str] | None, *, tool_name: str) -> None:
+    """Reject unknown `additional` field names before they hit DSM.
+
+    DSM accepts unknown values silently (the field just doesn't appear in the
+    response), which makes typos invisible to callers. Validating up-front
+    surfaces the typo as a clear ToolError naming the bad value and listing
+    the supported set.
+    """
+    if not values:
+        return
+    unknown = [v for v in values if v not in _VALID_ADDITIONAL_FIELDS]
+    if unknown:
+        error_response(
+            ErrorCode.INVALID_PARAMETER,
+            f"{tool_name} failed: unknown 'additional' field(s): {sorted(set(unknown))!r}.",
+            retryable=False,
+            param="additional",
+            value=values,
+            suggestion=("Supported fields: " + ", ".join(sorted(_VALID_ADDITIONAL_FIELDS)) + "."),
+        )
+
+
 # Size unit multipliers (binary: 1 KB = 1024 bytes)
 _SIZE_UNITS: dict[str, int] = {
     "B": 1,

src/mcp_synology/modules/filestation/listing.py

@@ -20,6 +20,7 @@
     ensure_recycle_status,
     file_type_icon,
     normalize_path,
+    validate_additional,
 )
 
 logger = logging.getLogger(__name__)
@@ -53,6 +54,7 @@ async def list_shares(
     file_type_indicator: str = "emoji",
 ) -> str:
     """List all shared folders on the NAS."""
+    validate_additional(additional, tool_name="List shares")
     if additional is None:
         additional = ["real_path", "size", "owner", "perm"]
 
@@ -129,6 +131,7 @@ async def list_files(
     file_type_indicator: str = "emoji",
 ) -> str:
     """List files and folders within a directory path."""
+    validate_additional(additional, tool_name="List files")
     if additional is None:
         additional = ["size", "time"]
 

src/mcp_synology/modules/filestation/search.py

@@ -20,6 +20,7 @@
     normalize_path,
     parse_human_size,
     parse_mtime,
+    validate_additional,
 )
 
 if TYPE_CHECKING:
@@ -64,6 +65,7 @@ async def search_files(
     poll_interval: float = 1.0,
 ) -> str:
     """Search for files by name, type, size, or modification date."""
+    validate_additional(additional, tool_name="Search files")
     if additional is None:
         additional = ["size", "time"]
 

tests/modules/filestation/test_helpers.py

@@ -6,6 +6,7 @@
 from unittest.mock import AsyncMock
 
 import pytest
+from mcp.server.fastmcp.exceptions import ToolError
 
 from mcp_synology.core.errors import SynologyError
 from mcp_synology.modules.filestation.helpers import (
@@ -17,6 +18,7 @@
     normalize_path,
     parse_human_size,
     parse_mtime,
+    validate_additional,
     validate_share_path,
 )
 
@@ -170,6 +172,55 @@ def test_comma_escape(self) -> None:
         assert result == "/video/a\\,b"
 
 
+class TestValidateAdditional:
+    """Closes #41: `additional` field whitelist enforcement.
+
+    DSM silently accepts unknown values (the field just doesn't appear in the
+    response), so a typo like `"sze"` would never produce a visible error.
+    `validate_additional` rejects unknown values up-front with a clear
+    ToolError naming the bad value and listing the supported set.
+    """
+
+    @pytest.mark.parametrize(
+        "value",
+        [
+            None,
+            [],
+            ["size"],
+            ["size", "time"],
+            ["real_path", "size", "owner", "perm"],
+            ["mount_point_type", "volume_status"],
+            ["type"],
+        ],
+    )
+    def test_accepts_valid_or_empty(self, value: list[str] | None) -> None:
+        validate_additional(value, tool_name="List shares")  # no exception
+
+    @pytest.mark.parametrize(
+        "value,bad",
+        [
+            (["sze"], "sze"),
+            (["size", "tme"], "tme"),
+            (["mount_point_type", "junk"], "junk"),
+            (["", "size"], ""),
+        ],
+    )
+    def test_rejects_unknown(self, value: list[str], bad: str) -> None:
+        with pytest.raises(ToolError) as exc:
+            validate_additional(value, tool_name="List shares")
+        body = str(exc.value)
+        assert "List shares failed" in body
+        assert bad in body
+        # Suggestion lists every supported field.
+        for field in ("real_path", "size", "owner", "time", "perm", "type"):
+            assert field in body
+
+    def test_tool_name_appears_in_error(self) -> None:
+        with pytest.raises(ToolError) as exc:
+            validate_additional(["bogus"], tool_name="Search files")
+        assert "Search files failed" in str(exc.value)
+
+
 class TestMatchesPattern:
     def test_glob_match(self) -> None:
         assert matches_pattern("Severance.S02E10.mkv", "*.mkv")

tests/modules/filestation/test_listing.py

@@ -155,6 +155,38 @@ async def test_list_shares_error(self, mock_client: DsmClient) -> None:
         assert body["status"] == "error"
         assert body["error"]["code"] == "permission_denied"
 
+    @respx.mock
+    async def test_list_shares_additional_round_trips(self, mock_client: DsmClient) -> None:
+        """Closes #41: caller-supplied `additional` reaches DSM verbatim.
+
+        The pre-fix tool registration didn't surface `additional`, so callers
+        could not request fields like `mount_point_type` / `volume_status`.
+        Round-trip test: pass a non-default list and confirm the DSM request
+        carries the values in the documented `["a","b"]` form.
+        """
+        captured: list[dict[str, str]] = []
+
+        def side_effect(request: httpx.Request) -> httpx.Response:
+            captured.append(dict(request.url.params))
+            return httpx.Response(200, json={"success": True, "data": {"shares": [], "total": 0}})
+
+        respx.get(f"{BASE_URL}/webapi/entry.cgi").mock(side_effect=side_effect)
+        await list_shares(
+            mock_client,
+            additional=["mount_point_type", "volume_status"],
+        )
+        assert len(captured) == 1
+        assert captured[0]["additional"] == '["mount_point_type","volume_status"]'
+
+    async def test_list_shares_rejects_unknown_additional(self, mock_client: DsmClient) -> None:
+        """Unknown `additional` field is rejected before any DSM round-trip."""
+        with pytest.raises(ToolError) as exc_info:
+            await list_shares(mock_client, additional=["sze"])
+        body = json.loads(str(exc_info.value))
+        assert body["status"] == "error"
+        assert body["error"]["code"] == "invalid_parameter"
+        assert "sze" in body["error"]["message"]
+
 
 class TestListFiles:
     @respx.mock
@@ -234,6 +266,32 @@ async def test_list_files_error(self, mock_client: DsmClient) -> None:
         assert body["status"] == "error"
         assert body["error"]["code"] == "not_found"
 
+    @respx.mock
+    async def test_list_files_additional_round_trips(self, mock_client: DsmClient) -> None:
+        """Closes #41: caller-supplied `additional` reaches DSM verbatim."""
+        captured: list[dict[str, str]] = []
+
+        def side_effect(request: httpx.Request) -> httpx.Response:
+            captured.append(dict(request.url.params))
+            return httpx.Response(200, json={"success": True, "data": {"files": [], "total": 0}})
+
+        respx.get(f"{BASE_URL}/webapi/entry.cgi").mock(side_effect=side_effect)
+        await list_files(
+            mock_client,
+            path="/video",
+            additional=["real_path", "size", "owner", "time", "perm"],
+        )
+        assert len(captured) == 1
+        assert captured[0]["additional"] == '["real_path","size","owner","time","perm"]'
+
+    async def test_list_files_rejects_unknown_additional(self, mock_client: DsmClient) -> None:
+        with pytest.raises(ToolError) as exc_info:
+            await list_files(mock_client, path="/video", additional=["junk"])
+        body = json.loads(str(exc_info.value))
+        assert body["status"] == "error"
+        assert body["error"]["code"] == "invalid_parameter"
+        assert "junk" in body["error"]["message"]
+
 
 class TestListRecycleBin:
     @respx.mock

tests/modules/filestation/test_search.py

@@ -233,3 +233,45 @@ def side_effect(request: httpx.Request) -> httpx.Response:
         # Both lands as Unix-epoch-seconds strings in DSM start params.
         assert captured.get("mtime_from") == "1775001600"  # 2026-04-01 00:00 UTC
         assert captured.get("mtime_to") == "1775044800"  # 2026-04-01 12:00 UTC
+
+    @respx.mock
+    async def test_search_additional_round_trips(self, mock_client: DsmClient) -> None:
+        """Closes #41: caller-supplied `additional` reaches DSM verbatim.
+
+        For Search, `additional` is sent on the `list` (poll) request, not on
+        `start` — DSM returns the full file metadata at poll time.
+        """
+        captured_list: dict[str, str] = {}
+
+        def side_effect(request: httpx.Request) -> httpx.Response:
+            params = dict(request.url.params)
+            if params.get("method") == "start":
+                return httpx.Response(200, json={"success": True, "data": {"taskid": "s-add"}})
+            if params.get("method") == "list":
+                captured_list.update(params)
+                return httpx.Response(
+                    200,
+                    json={"success": True, "data": {"files": [], "finished": True, "total": 0}},
+                )
+            return httpx.Response(200, json={"success": True, "data": {}})
+
+        respx.get(f"{BASE_URL}/webapi/entry.cgi").mock(side_effect=side_effect)
+
+        await search_files(
+            mock_client,
+            folder_path="/video",
+            additional=["size", "time", "perm"],
+        )
+        assert captured_list.get("additional") == '["size","time","perm"]'
+
+    async def test_search_rejects_unknown_additional(self, mock_client: DsmClient) -> None:
+        with pytest.raises(ToolError) as exc_info:
+            await search_files(
+                mock_client,
+                folder_path="/video",
+                additional=["nope"],
+            )
+        body = json.loads(str(exc_info.value))
+        assert body["status"] == "error"
+        assert body["error"]["code"] == "invalid_parameter"
+        assert "nope" in body["error"]["message"]