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

[cmeans-claude-dev]

Summary

Closes #41.

Same class of spec-vs-tool drift as #33 (which added mtime_from/mtime_to to search_files). The handlers in modules/filestation/{listing,search}.py already accepted additional: list[str] | None, but the FastMCP tool registrations in modules/filestation/__init__.py never surfaced it — callers couldn't request DSM metadata fields like mount_point_type or volume_status even though the spec listed them as configurable.

What changed

• The three tool registrations (tool_list_shares, tool_list_files, tool_search_files) now accept additional: list[str] | None = None. Each has a docstring listing the supported field set for its API.
• Underlying defaults are unchanged (["real_path", "size", "owner", "perm"] for shares, ["size", "time"] for files and search), so existing callers see no behavior diff.
• New validate_additional() helper in helpers.py rejects unknown values up-front against a union whitelist (real_path, size, owner, time, perm, type, mount_point_type, volume_status).

Why a whitelist matters

DSM silently ignores unknown additional values — the field just doesn't appear in the response. That makes typos like "sze" invisible to callers. Validating up-front surfaces them as a clear ToolError naming the bad value and listing the supported set.

Why a union whitelist (not per-API)

The valid set differs per API (mount_point_type/volume_status only meaningful on List share-level calls, etc.), but DSM ignores values that don't apply without erroring. Per-API gating would block legitimate callers from requesting union fields with no upside; the union approach catches typos and injection while staying ergonomic.

Tests

Unit (18 new):

• TestValidateAdditional (12 cases): 7 valid/empty accepted, 4 unknown rejected with clear errors, 1 verifying the tool name appears in the error envelope.
• TestListShares::test_list_shares_additional_round_trips + reject-unknown — confirms a non-default list reaches DSM verbatim in ["a","b"] form.
• TestListFiles::test_list_files_additional_round_trips + reject-unknown — same shape.
• TestSearchFiles::test_search_additional_round_trips + reject-unknown — captures the list (poll) request, not start. For Search, DSM returns full file metadata at poll time, so additional is sent there.

599 unit tests pass at 96.25% coverage. ruff/mypy clean.

Test plan

• uv run pytest tests/modules/filestation/test_helpers.py::TestValidateAdditional -v — 12 pass
• uv run pytest tests/modules/filestation/ -v — full filestation suite green (164 → 182)
• uv run pytest — full unit suite green (599 passed)
• uv run ruff check src/ tests/
• uv run ruff format --check src/ tests/
• uv run mypy src/
• vdsm CI green
• Smoke (post-install): list_files(path="/video", additional=["real_path", "size", "owner", "time", "perm"]) returns extra metadata columns; additional=["sze"] errors with the suggestion line

🤖 Generated with Claude Code

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[cmeans]

LGTM

[cmeans]

QA Round 1 — PASS (no findings)

Reviewed all 8 files on head 64ad257 (rebased on 3e9d96a = #88's merge commit, post-Glama-removal main). 8 files / +223 / -1.

Verified

Code (__init__.py, helpers.py, listing.py, search.py):

• validate_additional() is a clean union-whitelist guard: returns early on None/[], reports any unknown value via error_response(ErrorCode.INVALID_PARAMETER, ...) with the bad value(s) named in the message and the full supported set in the suggestion line. Tool name flows through so the error envelope reads "List shares failed" / "List files failed" / "Search files failed".
• All three FastMCP tool registrations gain additional: list[str] | None = None with per-API docstrings listing the meaningful fields and the default. Pass-through to the underlying handler is consistent across all three.
• All three handlers call validate_additional(...) BEFORE the if additional is None: additional = [defaults] line — so a None/[] input passes through and gets the default; an invalid input raises before the default kicks in. Correct ordering.
• Union vs. per-API trade-off (a caller could pass type to list_shares and DSM would silently ignore it) is explicitly explained in the helpers.py comment AND the PR body — deliberate ergonomic choice, not a gap. Per-API docstrings list only the fields that have effect, which is the right level of guidance to callers.

Tests (test_helpers.py, test_listing.py, test_search.py):

• TestValidateAdditional (12 cases): 7 valid/empty + 4 unknown-rejection + 1 tool-name-in-error. Reject tests assert the bad value AND every supported field land in the error body — strong defensive shape, will catch regressions in either the validation message or the suggestion line.
• Round-trip tests (test_list_shares_additional_round_trips, test_list_files_additional_round_trips, test_search_additional_round_trips) capture the actual DSM request and assert params["additional"] is the exact JSON-encoded string '["a","b"]' — catches serialization regressions, doesn't over-mock. Search variant correctly captures the list (poll) request, not start, since DSM Search returns full metadata at poll time.
• Reject-unknown tests at the handler level (3 tools × 1 each) confirm validation fires before any DSM round-trip.

Issue #41 scope:

• AC 1 (decide expose vs. collapse): chose to expose — the right call given the spec at docs/specs/filestation-module-spec.md:117 already documented additional as configurable.
• AC 2 (whitelist validation if exposing): validate_additional does this; whitelist matches the documented DSM API surface across List + Search exactly.
• AC 4 (CHANGELOG entry): present under ## Unreleased ### Added, correctly slotted above ### Changed (#88 Glama removal).
• AC 3 (collapse path) not applicable.

Local verification on 64ad257

• uv run pytest → 599 passed, 112 deselected, 96.25% coverage. Matches PR body verbatim (post-#85 baseline 581 + 18 new = 599).
• uv run pytest tests/modules/filestation/test_helpers.py::TestValidateAdditional → 12 passed.
• uv run pytest tests/modules/filestation/ → 182 passed (matches "164 → 182" claim in PR body).
• uv run ruff check, ruff format --check, mypy src/ all clean.

CI

12/12 required checks green incl. vdsm integration tests SUCCESS on this exact head.

PR-body checkboxes

Boxes 1–7 flipped (all pre-validated above). Box 8 (post-install smoke against real NAS) deliberately left unchecked — that's a post-merge step on the production NAS; pre-merge equivalent is the green vdsm CI run.

Disposition

Ready for QA Signoff applied as the final act. With this in, #51's MEDIUM block (#41) closes cleanly — only #44 remains in MEDIUM.

[cmeans]

Applying Ready for QA Signoff — clean PR, no findings. Code review, scope vs. issue #41 ACs, validate_additional union whitelist (matches documented DSM API surface), per-API docstrings, validate-before-default ordering, round-trip tests (exact JSON-encoded string capture), reject-unknown tests, and the search-poll-vs-start handling all check out. Local 599/96.25% (matches PR body verbatim), TestValidateAdditional 12/12, filestation suite 164→182, ruff/format/mypy clean, CI 12/12 green incl. vdsm. Boxes 1-7 flipped; #8 (post-install smoke) is post-merge. Closes #41 — last MEDIUM in #51 except #44.