feat(downloadstation): Phase 2 — 7 WRITE tools + DS error codes

CHANGELOG.md

@@ -4,9 +4,11 @@
 
 ### Added
 
+- **Download Station module — Phase 2 (WRITE tools)** (#105). Second of three planned PRs from the DS module spec (`docs/superpowers/specs/2026-05-13-downloadstation-module-design.md`). Adds seven WRITE-tier tools: `create_download` (URI list or local `.torrent`/`.nzb` file via multipart POST), `delete_download` (explicit `delete_data: bool` required — `True` proceeds with the destructive default-DSM-behavior of removing both task and files, `False` is REFUSED with a clear error because DSM v1 `Task.delete` has no documented "keep files" mode), `pause_download` / `resume_download` (siblings sharing a private `_task_state_change` helper that walks the per-task error list), `edit_download` (currently `destination` only — DSM's `Task.edit` supported-field set varies by DSM version and is verified per-NAS), `set_download_config` (partial updates only — only supplied fields are written), `set_schedule` (168-character weekly plan, validated client-side via the same `SCHEDULE_PLAN_LENGTH` invariant `format_schedule_grid` uses on the read side). New `core/downloadstation_errors.py` carries DS-specific 400-series error codes (different semantics from FileStation's reuse of the same numeric range — DS 400 is "file upload failed", FS 400 is "invalid parameter"); `error_from_code()` dispatches to the DS map via a deferred local import when the api begins with `SYNO.DownloadStation`, with explicit regression tests guarding that codes 105 (permission denied) and 106 (session expired) still fall through to the common 100-series handlers for DS APIs — the CLAUDE.md "never re-auth on 105" invariant must hold for DS calls too. New `DsmClient.create_download_task_with_file()` mirrors the existing `upload_file()` multipart helper with the same re-auth-on-session-error retry semantics and `_sid`-masked debug logging. Bonus fix to `DsmClient.request()` debug logging: was calling `.keys()` unconditionally on the response `data` field, which crashed for endpoints (like `Task.delete`) that return a list instead of a dict — now guarded with `isinstance(data, dict)`. Re-auth retry tests added for `create_download_task_with_file` (multipart path) and the URI-path `create_download` documents the standard-GET re-auth surface — closes the operational coverage gap flagged at issue #99 for the DS module specifically. 62 new tests; every new module file at 100% line coverage; 727 repo tests pass at 96.26% overall.
+
 - **Download Station module — Phase 1 (READ tools)** (#104). New module `mcp_synology.modules.downloadstation` adds five read-only tools — `list_downloads`, `get_download_info`, `get_download_stats`, `get_download_config`, `get_schedule` — that expose Synology's Download Station package via DSM API v1 (`SYNO.DownloadStation.Task`, `Info`, `Statistic`, `Schedule`). Module is opt-in via `instances.[name].modules.downloadstation.enabled: true`; the API preflight skips registration entirely when the DS package is not installed on the NAS, so users without DS pay zero LLM-context cost (no tools registered, no handler imports, no MODULE_INFO consumed beyond the preflight). Module shape mirrors File Station: domain-split files (`tasks.py`, `stats.py`, `config.py`, `helpers.py`), lazy handler imports inside `register()`, `respx`-mocked unit tests. Reuses the existing shared DSM session — DSM does not gate API access by session name, so no auth-manager changes were needed; ADR-0001's deferral of per-client sessions applies to client-isolation, not per-package isolation. `list_downloads` does client-side status filtering (DSM v1 `Task.list` lacks server-side filter support); `get_download_info` requests all five `additional` groups (`detail,transfer,file,tracker,peer`) and renders them as distinct sections; `get_schedule` parses DSM's 168-character `schedule_plan` into a 7×24 ASCII grid. Tool input validation uses the project's standard `error_response(ErrorCode.INVALID_PARAMETER, ...)` structured envelope. Phase 2 (task CRUD writes + global config writes) and Phase 3 (BT search + RSS) to follow as separate PRs. 60 new unit tests; every new module file (`__init__.py`, `tasks.py`, `stats.py`, `config.py`, `helpers.py`) at 100% line coverage. Full design at `docs/superpowers/specs/2026-05-13-downloadstation-module-design.md`.
 
-- **ADR 0001 — Per-Client DSM Sessions and the Streamable HTTP Roadmap** (#PR_PLACEHOLDER) — closes #47. The 2026-04-16 project-wide review flagged the largest spec-vs-code gap: `docs/specs/architecture.md` documented a `session_key` parameter on `AuthManager.get_session()` for per-MCP-client DSM sessions under Streamable HTTP, but the live signature was `get_session() -> str` with no key and no plumbing. Three options were considered — ship the multi-session path now, retract the spec, or formally defer with the helper restructured. This PR records the deferral (Option 3) and closes the spec drift. New `docs/adr/0001-per-client-dsm-sessions.md` captures the question, options-with-tradeoffs, decision, consequences, and five concrete revisit triggers (Streamable HTTP gaining a concrete plan, multi-tenant deployment use case, subagent isolation request, tool-surface roughly doubling, single-shared-session pain showing up under real load). The ADR also inventories what the next implementation step looks like when a trigger fires, so the deferred work has a clear bounded shape rather than an open-ended re-design. Spec changes: `docs/specs/architecture.md` "Auth Manager" example reverted to the live `get_session()` signature with a forward-pointer to the planned section + ADR; the dedicated section renamed "Future" → "Planned: Per-Client Sessions (Streamable HTTP)" with an explicit deferral-not-oversight statement and a back-reference to the ADR.
+- **ADR 0001 — Per-Client DSM Sessions and the Streamable HTTP Roadmap** (#100) — closes #47. The 2026-04-16 project-wide review flagged the largest spec-vs-code gap: `docs/specs/architecture.md` documented a `session_key` parameter on `AuthManager.get_session()` for per-MCP-client DSM sessions under Streamable HTTP, but the live signature was `get_session() -> str` with no key and no plumbing. Three options were considered — ship the multi-session path now, retract the spec, or formally defer with the helper restructured. This PR records the deferral (Option 3) and closes the spec drift. New `docs/adr/0001-per-client-dsm-sessions.md` captures the question, options-with-tradeoffs, decision, consequences, and five concrete revisit triggers (Streamable HTTP gaining a concrete plan, multi-tenant deployment use case, subagent isolation request, tool-surface roughly doubling, single-shared-session pain showing up under real load). The ADR also inventories what the next implementation step looks like when a trigger fires, so the deferred work has a clear bounded shape rather than an open-ended re-design. Spec changes: `docs/specs/architecture.md` "Auth Manager" example reverted to the live `get_session()` signature with a forward-pointer to the planned section + ADR; the dedicated section renamed "Future" → "Planned: Per-Client Sessions (Streamable HTTP)" with an explicit deferral-not-oversight statement and a back-reference to the ADR.
 
 - **publish.yml: gate PyPI publish on `server.json` registry-schema validation** (#89) — closes #44. Adds a `validate-server-json` job that runs before `publish-pypi` on every release tag, mirroring the same check that runs on every PR via `ci.yml`. Without this gate, a malformed `server.json` (new required field in a future registry schema, type change, etc.) would PyPI-publish cleanly and then fail at the registry leg — leaving a discoverable PyPI release that isn't in the MCP registry, with no way to roll back PyPI short of yanking. v0.5.1 hit the registry-leg-fails-after-PyPI scenario for a *different* reason (mcp-publisher OIDC audience mismatch, fixed in #79), so the failure mode isn't theoretical. The new job uses the same `./.github/actions/install-mcp-publisher` composite that `publish-registry` uses, so the validating publisher version always matches the publishing one. Job runs in parallel with `build` (no dep on it), and `publish-pypi` now lists both as `needs:` so a validation failure stops the entire pipeline before any external side-effect. The optional `publish-manifest` artifact named in #44 is deferred — it's a tooling-version reconciliation aid that's only useful if a CI-PR / release-tag mismatch *does* fire, and is straightforward to add later when there's a concrete failure to debug.
 

src/mcp_synology/core/client.py

@@ -298,7 +298,14 @@ async def request(
 
         if body.get("success"):
             data: dict[str, Any] = body.get("data", {})
-            logger.debug("DSM response: %s/%s — success (keys: %s)", api, method, list(data.keys()))
+            if isinstance(data, dict):
+                logger.debug(
+                    "DSM response: %s/%s — success (keys: %s)", api, method, list(data.keys())
+                )
+            else:
+                logger.debug(
+                    "DSM response: %s/%s — success (list, %d items)", api, method, len(data)
+                )
             return data
 
         code = body.get("error", {}).get("code", 0)
@@ -426,6 +433,121 @@ def _open_file() -> BinaryIO:
 
         raise error_from_code(code, api)
 
+    async def create_download_task_with_file(
+        self,
+        file_path: Path,
+        filename: str,
+        *,
+        uri: str | None = None,
+        destination: str | None = None,
+        username: str | None = None,
+        password: str | None = None,
+        timeout: float = 300.0,
+        _is_retry: bool = False,
+    ) -> dict[str, Any]:
+        """Create a Download Station task by uploading a .torrent or .nzb file.
+
+        Multipart POST against SYNO.DownloadStation.Task.create v1. Modeled on
+        ``upload()`` — same re-auth-on-session-error retry semantics, same
+        debug logging with ``_sid`` masked. Returns the ``data`` dict from the
+        DSM response (typically ``{"task_id": "..."}`` or ``{}``).
+        """
+        api = "SYNO.DownloadStation.Task"
+        http = self._get_http()
+
+        if api not in self._api_cache:
+            from mcp_synology.core.errors import ApiNotFoundError
+
+            raise ApiNotFoundError(
+                f"API '{api}' not found. Call query_api_info() first.",
+                code=102,
+            )
+
+        info = self._api_cache[api]
+        # DS Task.create pins to v1 — no v2 multipart path documented.
+        resolved_version = 1
+        url = f"{self._base_url}/webapi/{info.path}"
+
+        form_data: dict[str, str] = {
+            "api": api,
+            "version": str(resolved_version),
+            "method": "create",
+        }
+        if uri is not None:
+            form_data["uri"] = uri
+        if destination is not None:
+            form_data["destination"] = destination
+        if username is not None:
+            form_data["username"] = username
+        if password is not None:
+            form_data["password"] = password
+
+        # SID must be a query parameter, not a form field — same pattern as
+        # the FileStation Upload API.
+        query_params: dict[str, str] = {}
+        if self._sid:
+            query_params["_sid"] = self._sid
+
+        _sensitive = frozenset({"_sid", "password"})
+        log_data = {
+            k: ("***" if k in _sensitive else v) for k, v in {**form_data, **query_params}.items()
+        }
+        retry_tag = " (retry)" if _is_retry else ""
+        logger.debug(
+            "DSM POST%s: %s/create v%d — %s, file=%s",
+            retry_tag,
+            api,
+            resolved_version,
+            log_data,
+            filename,
+        )
+
+        def _open_file() -> BinaryIO:
+            return open(file_path, "rb")  # noqa: SIM115
+
+        fh = _open_file()
+        try:
+            resp = await http.post(
+                url,
+                params=query_params,
+                data=form_data,
+                files={"file": (filename, fh, "application/octet-stream")},
+                timeout=httpx.Timeout(timeout),
+            )
+        finally:
+            fh.close()
+
+        resp.raise_for_status()
+        body = resp.json()
+
+        if body.get("success"):
+            logger.debug("DSM response: %s/create — success", api)
+            data: dict[str, Any] = body.get("data", {})
+            return data
+
+        code = body.get("error", {}).get("code", 0)
+        logger.debug("DSM response: %s/create — error code %d", api, code)
+
+        # Re-auth on session errors (file must be re-opened on retry)
+        if code in _SESSION_ERROR_CODES and not _is_retry and self._re_auth_callback:
+            logger.info("Session error %d on %s/create, attempting re-auth.", code, api)
+            try:
+                await self._re_auth_callback()
+            except SynologyError:
+                raise error_from_code(code, api) from None
+            return await self.create_download_task_with_file(
+                file_path,
+                filename,
+                uri=uri,
+                destination=destination,
+                username=username,
+                password=password,
+                timeout=timeout,
+                _is_retry=True,
+            )
+
+        raise error_from_code(code, api)
+
     async def download(
         self,
         path: str,

src/mcp_synology/core/downloadstation_errors.py

@@ -0,0 +1,66 @@
+"""Download Station API error codes (400-series, DS-specific semantics).
+
+DS overloads the same numeric range that FILESTATION_ERROR_CODES uses, with
+different meanings (e.g., DS 400 = file upload failed; FS 400 = invalid
+parameter). Keeping the maps in separate files makes the overlap visible.
+
+To add a new code: add an entry below AND ensure docs/error-codes.md has a
+matching section (the tests/core/test_help_urls.py test enforces parity).
+"""
+
+from __future__ import annotations
+
+from mcp_synology.core.errors import ErrorCode, SynologyError
+
+
+class DownloadStationError(SynologyError):
+    """Base exception for Download Station API errors."""
+
+    error_code = ErrorCode.DSM_ERROR
+
+
+DOWNLOADSTATION_ERROR_CODES: dict[int, tuple[str, str]] = {
+    400: (
+        "File upload failed",
+        "Failed to upload .torrent or .nzb file. Check the file is readable "
+        "and within DSM's upload size limit.",
+    ),
+    401: (
+        "Max number of tasks reached",
+        "DSM has hit its max-concurrent-tasks limit. Pause or delete finished "
+        "tasks first, then retry.",
+    ),
+    402: (
+        "Destination denied",
+        "Destination share rejected the task. Check the user has write "
+        "permission on the destination share.",
+    ),
+    403: (
+        "Destination doesn't exist",
+        "Destination path doesn't exist on the NAS. Verify the share and "
+        "folder with list_files before retrying.",
+    ),
+    404: (
+        "Invalid task id",
+        "Task id not found. Run list_downloads to get the current set of ids.",
+    ),
+    405: (
+        "Invalid task action",
+        "Action is not valid for this task's current state (e.g., resuming a "
+        "task that is already running, or pausing a finished task).",
+    ),
+    406: (
+        "No default destination",
+        "DS has no default destination configured. Set one via set_download_config "
+        "or in DSM > Download Station > Settings > General.",
+    ),
+    407: (
+        "Set destination failed",
+        "Setting the task destination failed. The path may be invalid or the "
+        "user may lack write permission.",
+    ),
+    408: (
+        "File doesn't exist",
+        "A referenced file (typically a .torrent or .nzb path on the NAS) does not exist.",
+    ),
+}

src/mcp_synology/core/errors.py

@@ -268,6 +268,19 @@ def error_from_code(code: int, api_name: str = "") -> SynologyError:
     for Auth vs FileStation APIs. This function checks API-specific codes
     first, then falls back to common codes.
     """
+    # Download Station codes (400-series for SYNO.DownloadStation.*)
+    # Deferred import to avoid a circular dependency: downloadstation_errors
+    # imports from this module (errors.py), so we cannot import it at module top.
+    if "DownloadStation" in api_name:
+        from mcp_synology.core.downloadstation_errors import (  # noqa: PLC0415
+            DOWNLOADSTATION_ERROR_CODES,
+            DownloadStationError,
+        )
+
+        if code in DOWNLOADSTATION_ERROR_CODES:
+            message, suggestion = DOWNLOADSTATION_ERROR_CODES[code]
+            return DownloadStationError(message, code=code, suggestion=suggestion)
+
     # Auth API codes (400-series for SYNO.API.Auth)
     if "Auth" in api_name and code in AUTH_ERROR_CODES:
         message, suggestion = AUTH_ERROR_CODES[code]