Add param/value to five error_response() call sites (F17)

[cmeans-claude-dev]

Summary

Closes #29. Pure envelope metadata addition — no behavior change, no message text change.

Five error_response() call sites in modules/filestation/*.py previously emitted envelopes without the param/value fields that smart clients use to dispatch on which argument triggered the failure. Adding them makes the envelopes self-describing (client can re-prompt specifically for the offending path, or log the timeout value, without parsing the human-readable message).

Changes

File:line	Envelope code	Now includes
src/mcp_synology/modules/filestation/metadata.py:76	NOT_FOUND (multi-path get_info empty result)	param="paths", value=paths
src/mcp_synology/modules/filestation/metadata.py:222	TIMEOUT (get_dir_size timeout)	param="timeout", value=timeout
src/mcp_synology/modules/filestation/operations.py:245	TIMEOUT (copy/move timeout)	param="timeout", value=timeout
src/mcp_synology/modules/filestation/operations.py:353	TIMEOUT (delete timeout)	param="timeout", value=timeout
src/mcp_synology/modules/filestation/transfer.py:209	FILESYSTEM_ERROR (download local-write OSError fallthrough)	param="dest_folder", value=dest_folder

Five matching regression assertions added to existing tests:

• test_empty_files_list_returns_not_found (test_metadata.py)
• test_dir_size_timeout (test_metadata.py)
• test_copy_timeout (test_operations.py)
• test_delete_timeout (test_operations.py)
• test_download_write_permission_error (test_transfer.py)

Scope notes

Why these specific sites

Audited every error_response( call in src/mcp_synology/modules/filestation/*.py. Sites already carrying param/value were untouched (operations.py:94 rename-rejects-path, transfer.py:44,71,82,120,134,162,201 upload/download pre-flight checks). Sites where no concrete argument is meaningfully at fault (transfer.py:162 disk-full preflight — a calculation, not an arg; transfer.py:201 ENOSPC — same) were left alone.

Explicitly out of scope

• operations.py:258 and :367 — bare DSM_ERROR on background-task failures. These are F18 / issue Route task-failure error codes through error_from_code() for specific envelopes #30, because the desired change there is to route err_code through error_from_code() for a more specific envelope code. That's a behavior change, not a metadata addition, and QA explicitly wanted it reviewed separately.
• Call sites outside modules/filestation/*.py (e.g., modules/system/*.py). Issue Add param/value to error_response() call sites for structured client dispatch #29 scoped the audit to filestation.

Test plan

Automated (CI)

• uv run ruff check src/ tests/ — clean
• uv run mypy src/ — clean (no type changes)
• uv run pytest — 495 passed, 96.03% coverage (no new tests, five regressions tightened)
• All 5 updated assertion pairs exercise the fields newly added at the paired source site
• validate-server-json on CI should remain green (no workflow changes)

Manual

None required — this is a structured-response metadata change verified by the automated assertions.

🤖 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]

QA Review — PR #31

Verification (this session, on pr-31 HEAD e333c0f)

Check	Result
uv run pytest (full suite)	495 passed, 94 deselected (integration + vdsm markers, excluded by addopts; vdsm runs in .github/workflows/vdsm.yml — green on this PR), coverage 96.03%
Targeted: test_empty_files_list_returns_not_found, test_dir_size_timeout, test_copy_timeout, test_delete_timeout, test_download_write_permission_error	5/5 PASSED — each exercises the param/value fields added at its paired source site
uv run ruff check .	All checks passed
uv run mypy src	Success: no issues found in 27 source files
CI on PR	lint, typecheck, test (3.11/3.12/3.13), version-sync, validate-server-json, vdsm integration tests — all green

Scope vs issue #29

AC	Status
Audit all error_response() calls in modules/filestation/*.py and add param/value where applicable	Complete — full census below
Existing tests continue to pass (envelope changes are additive)	495/495 green
Regression assertions for at least one site per file	Exceeded — 5 regressions across 3 files (2 in metadata, 2 in operations, 1 in transfer)
No changes to human-readable message text	Verified — diff adds only param=/value= lines; no message string touched
CHANGELOG ## Unreleased → ### Changed entry	Present, accurate, and correctly under ### Changed (not ### Added)

Audit census

Grepped every error_response( call in src/mcp_synology/modules/filestation/*.py and classified:

Modified (5):

• metadata.py:75 NOT_FOUND empty-result → param="paths", value=paths
• metadata.py:222 TIMEOUT get_dir_size → param="timeout", value=timeout
• operations.py:243 TIMEOUT copy/move → param="timeout", value=timeout
• operations.py:351 TIMEOUT delete → param="timeout", value=timeout
• transfer.py:207 FILESYSTEM_ERROR download write OSError → param="dest_folder", value=dest_folder

Already had param/value (confirmed by reading the file) — correctly left untouched:

• operations.py:94 rename new_name is a path → param="new_name"
• transfer.py:44 upload local not-found → param="local_path"
• transfer.py:71 upload already-exists → param="filename"
• transfer.py:82 upload read OSError → param="local_path"
• transfer.py:120 download dest not-found → param="dest_folder"
• transfer.py:134 download dest exists → param="dest_folder"

Deliberately out of scope:

• transfer.py:162 / transfer.py:201 disk-full preflight + ENOSPC: PR body correctly reasons that the disk_full code itself is the dispatch signal; the state being measured (free space vs file size) isn't a direct arg fault. Defensible judgment call.
• operations.py:257 / operations.py:365 bare DSM_ERROR in background-task failure paths: explicitly deferred to F18 / issue #30, where the desired change is routing err_code through error_from_code() (a behavior change, not a metadata addition). Correct scope boundary.

Code notes

• value=paths at metadata.py:80 passes the list through. error_response() types value: Any | None, so list values serialize cleanly — verified by the regression assertion body["error"]["value"] == ["/video/missing1", "/video/missing2"].
• value=timeout passes a float; regression asserts ... == 1.0 — clean JSON roundtrip.
• All new fields use the raw input arg (paths, timeout, dest_folder) rather than a normalized/derived form, which is the right choice for client dispatch (clients want to key off what they sent).

Findings

None. Applying Ready for QA Signoff.

[cmeans]

QA audit — applying Ready for QA Signoff

Verification in this session on pr-31 HEAD e333c0f:

• uv run pytest: 495 passed, 94 deselected (integration + vdsm markers excluded by addopts; vdsm runs in its own workflow — green here), coverage 96.03%
• All 5 newly-tightened regressions (test_empty_files_list_returns_not_found, test_dir_size_timeout, test_copy_timeout, test_delete_timeout, test_download_write_permission_error) pass and each exercises the param/value fields at its paired source site
• ruff check, mypy src: clean
• All CI checks green

Independent audit of every error_response( call site in src/mcp_synology/modules/filestation/*.py confirms the PR's classification: 5 sites modified, 6 sites already had param/value, 4 sites deliberately out of scope (2 disk-full preflight — defensible, 2 DSM_ERROR — correctly deferred to F18 / issue #30). All 5 issue #29 ACs met. message text unchanged; CHANGELOG entry in ### Changed as required. Zero findings.

[cmeans]

LGTM