Route task-failure error codes through error_from_code (F18)

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Changed
 
+- **Task-completion error envelopes now route through `error_from_code()` for specific codes** (#30) — addresses F18 from the PR #9 self-audit. When a background CopyMove or Delete task completes with an `error` dict, `operations.py:256` (copy/move) and `operations.py:365` (delete) now pass the DSM error code through `error_from_code(err_code, "SYNO.FileStation.CopyMove" | "SYNO.FileStation.Delete")` to build the envelope, matching the synchronous error paths elsewhere in the module. Envelope `code` becomes the mapped value (e.g. `408` → `not_found`, `414` → `already_exists`, `416` → `disk_full`, `1100` → `filestation_error`, `105` → `permission_denied`); envelope `retryable` inherits from the mapped exception (e.g. `disk_full` is `retryable=true`); envelope `suggestion` comes from the per-code mapping when one exists and falls back to the previous generic suggestion otherwise. Unknown/unmapped codes still produce the `dsm_error` envelope. **Behavior change** — callers previously catching `dsm_error` on these two paths will now receive the more specific code. Tests `test_copy_task_completes_with_error` and `test_delete_task_completes_with_error` updated to assert `filestation_error` on code 1100 (plus the per-code suggestion text); four new cases cover 408→not_found, 416→disk_full+retryable, 105→permission_denied, and the unknown-code fallback.
 - **Structured error envelopes now include `param`/`value` on five more call sites** (#29) — addresses F17 from the PR #9 self-audit. Five `error_response()` call sites in `modules/filestation/*.py` previously emitted envelopes without the `param`/`value` fields that smart clients could dispatch on: `metadata.py:76` (`get_file_info` multi-path empty-result → `param="paths"`, `value=<paths>`), `metadata.py:222` (`get_dir_size` timeout → `param="timeout"`, `value=<timeout>`), `operations.py:245` (copy/move timeout), `operations.py:353` (delete timeout), and `transfer.py:209` (download local-write `OSError` → `param="dest_folder"`, `value=<dest_folder>`). Existing `message` text is unchanged. Regression assertions added to `test_empty_files_list_returns_not_found`, `test_dir_size_timeout`, `test_copy_timeout`, `test_delete_timeout`, and `test_download_write_permission_error`.
 
 ### Added

src/mcp_synology/modules/filestation/operations.py

@@ -6,7 +6,7 @@
 import logging
 from typing import TYPE_CHECKING, Any
 
-from mcp_synology.core.errors import ErrorCode, SynologyError
+from mcp_synology.core.errors import ErrorCode, SynologyError, error_from_code
 from mcp_synology.core.formatting import (
     error_response,
     format_size,
@@ -254,11 +254,18 @@ async def _copy_move(
         err = status["error"]
         err_code = err.get("code", 0) if isinstance(err, dict) else err
         err_path = status.get("path", "")
+        # Route err_code through error_from_code so callers see a specific
+        # envelope (e.g. not_found, disk_full) matching the synchronous error
+        # paths in this module. Unknown codes fall back to DSM_ERROR.
+        mapped = error_from_code(err_code, "SYNO.FileStation.CopyMove")
         error_response(
-            ErrorCode.DSM_ERROR,
+            mapped.error_code,
             f"{operation} files failed: DSM error code {err_code} on path: {err_path}",
-            retryable=False,
-            suggestion="Check that source paths exist and you have permission to access them.",
+            retryable=mapped.retryable,
+            suggestion=(
+                mapped.suggestion
+                or "Check that source paths exist and you have permission to access them."
+            ),
         )
 
     # Build response
@@ -362,11 +369,18 @@ async def delete_files(
         err = status["error"]
         err_code = err.get("code", 0) if isinstance(err, dict) else err
         err_path = status.get("path", "")
+        # Route err_code through error_from_code so callers see a specific
+        # envelope (e.g. permission_denied, not_found) matching the synchronous
+        # error paths in this module. Unknown codes fall back to DSM_ERROR.
+        mapped = error_from_code(err_code, "SYNO.FileStation.Delete")
         error_response(
-            ErrorCode.DSM_ERROR,
+            mapped.error_code,
             f"Delete files failed: DSM error code {err_code} on path: {err_path}",
-            retryable=False,
-            suggestion="Check that paths exist and you have permission to delete them.",
+            retryable=mapped.retryable,
+            suggestion=(
+                mapped.suggestion
+                or "Check that paths exist and you have permission to delete them."
+            ),
         )
 
     # Determine recycle bin status per share

tests/modules/filestation/test_operations.py

@@ -258,7 +258,13 @@ def side_effect(request: httpx.Request) -> httpx.Response:
 
     @respx.mock
     async def test_copy_task_completes_with_error(self, mock_client: DsmClient) -> None:
-        """Copy task finishes but status has an ``error`` key → dsm_error."""
+        """Copy task finishes with FileStation code 1100 → filestation_error.
+
+        Asserts the envelope is routed through ``error_from_code`` so callers
+        see the specific envelope code (``filestation_error``) and the per-code
+        suggestion from FILESTATION_ERROR_CODES instead of the old generic
+        ``dsm_error`` fallback.
+        """
 
         def side_effect(request: httpx.Request) -> httpx.Response:
             params = dict(request.url.params)
@@ -288,9 +294,118 @@ def side_effect(request: httpx.Request) -> httpx.Response:
                 dest_folder="/video/Archive",
             )
         body = json.loads(str(exc_info.value))
-        assert body["error"]["code"] == "dsm_error"
+        assert body["error"]["code"] == "filestation_error"
         assert "1100" in body["error"]["message"]
         assert "/video/restricted/file.mkv" in body["error"]["message"]
+        # Suggestion now comes from the per-code mapping, not the generic fallback.
+        assert "shared folder" in body["error"]["suggestion"].lower()
+
+    @respx.mock
+    async def test_copy_task_error_maps_408_to_not_found(self, mock_client: DsmClient) -> None:
+        """Copy task error 408 → ``not_found`` envelope (not ``dsm_error``)."""
+
+        def side_effect(request: httpx.Request) -> httpx.Response:
+            params = dict(request.url.params)
+            method = params.get("method", "")
+            if method == "start":
+                return httpx.Response(200, json={"success": True, "data": {"taskid": "cm-408"}})
+            if method == "status":
+                return httpx.Response(
+                    200,
+                    json={
+                        "success": True,
+                        "data": {
+                            "finished": True,
+                            "error": {"code": 408},
+                            "path": "/video/missing/file.mkv",
+                        },
+                    },
+                )
+            return httpx.Response(200, json={"success": True, "data": {}})
+
+        respx.get(f"{BASE_URL}/webapi/entry.cgi").mock(side_effect=side_effect)
+
+        with pytest.raises(ToolError) as exc_info:
+            await copy_files(
+                mock_client,
+                paths=["/video/missing/file.mkv"],
+                dest_folder="/video/Archive",
+            )
+        body = json.loads(str(exc_info.value))
+        assert body["error"]["code"] == "not_found"
+
+    @respx.mock
+    async def test_copy_task_error_maps_416_to_disk_full_retryable(
+        self, mock_client: DsmClient
+    ) -> None:
+        """Copy task error 416 → ``disk_full`` envelope, retryable=True."""
+
+        def side_effect(request: httpx.Request) -> httpx.Response:
+            params = dict(request.url.params)
+            method = params.get("method", "")
+            if method == "start":
+                return httpx.Response(200, json={"success": True, "data": {"taskid": "cm-416"}})
+            if method == "status":
+                return httpx.Response(
+                    200,
+                    json={
+                        "success": True,
+                        "data": {
+                            "finished": True,
+                            "error": {"code": 416},
+                            "path": "/video/big/file.mkv",
+                        },
+                    },
+                )
+            return httpx.Response(200, json={"success": True, "data": {}})
+
+        respx.get(f"{BASE_URL}/webapi/entry.cgi").mock(side_effect=side_effect)
+
+        with pytest.raises(ToolError) as exc_info:
+            await copy_files(
+                mock_client,
+                paths=["/video/big/file.mkv"],
+                dest_folder="/video/Archive",
+            )
+        body = json.loads(str(exc_info.value))
+        assert body["error"]["code"] == "disk_full"
+        assert body["error"]["retryable"] is True
+
+    @respx.mock
+    async def test_copy_task_error_unknown_code_falls_back(self, mock_client: DsmClient) -> None:
+        """Unknown/unmapped error code still yields ``dsm_error`` + generic suggestion."""
+
+        def side_effect(request: httpx.Request) -> httpx.Response:
+            params = dict(request.url.params)
+            method = params.get("method", "")
+            if method == "start":
+                return httpx.Response(200, json={"success": True, "data": {"taskid": "cm-999"}})
+            if method == "status":
+                return httpx.Response(
+                    200,
+                    json={
+                        "success": True,
+                        "data": {
+                            "finished": True,
+                            "error": {"code": 9999},
+                            "path": "/video/file.mkv",
+                        },
+                    },
+                )
+            return httpx.Response(200, json={"success": True, "data": {}})
+
+        respx.get(f"{BASE_URL}/webapi/entry.cgi").mock(side_effect=side_effect)
+
+        with pytest.raises(ToolError) as exc_info:
+            await copy_files(
+                mock_client,
+                paths=["/video/file.mkv"],
+                dest_folder="/video/Archive",
+            )
+        body = json.loads(str(exc_info.value))
+        assert body["error"]["code"] == "dsm_error"
+        assert "9999" in body["error"]["message"]
+        assert "source paths exist" in body["error"]["suggestion"]
 
     @respx.mock
     async def test_copy_poll_error_mid_operation(self, mock_client: DsmClient) -> None:
@@ -375,7 +490,7 @@ def side_effect(request: httpx.Request) -> httpx.Response:
 
     @respx.mock
     async def test_delete_task_completes_with_error(self, mock_client: DsmClient) -> None:
-        """Delete task finishes with an error dict → dsm_error."""
+        """Delete task error 1100 → ``filestation_error`` via error_from_code."""
 
         def side_effect(request: httpx.Request) -> httpx.Response:
             params = dict(request.url.params)
@@ -401,9 +516,42 @@ def side_effect(request: httpx.Request) -> httpx.Response:
         with pytest.raises(ToolError) as exc_info:
             await delete_files(mock_client, paths=["/video/locked/file.mkv"])
         body = json.loads(str(exc_info.value))
-        assert body["error"]["code"] == "dsm_error"
+        assert body["error"]["code"] == "filestation_error"
         assert "1100" in body["error"]["message"]
 
+    @respx.mock
+    async def test_delete_task_error_maps_105_to_permission_denied(
+        self, mock_client: DsmClient
+    ) -> None:
+        """Delete task error 105 (common) → ``permission_denied`` envelope."""
+
+        def side_effect(request: httpx.Request) -> httpx.Response:
+            params = dict(request.url.params)
+            method = params.get("method", "")
+            if method == "start":
+                return httpx.Response(200, json={"success": True, "data": {"taskid": "del-105"}})
+            if method == "status":
+                return httpx.Response(
+                    200,
+                    json={
+                        "success": True,
+                        "data": {
+                            "finished": True,
+                            "error": {"code": 105},
+                            "path": "/video/locked/file.mkv",
+                        },
+                    },
+                )
+            return httpx.Response(200, json={"success": True, "data": {}})
+
+        respx.get(f"{BASE_URL}/webapi/entry.cgi").mock(side_effect=side_effect)
+
+        with pytest.raises(ToolError) as exc_info:
+            await delete_files(mock_client, paths=["/video/locked/file.mkv"])
+        body = json.loads(str(exc_info.value))
+        assert body["error"]["code"] == "permission_denied"
+        assert "105" in body["error"]["message"]
+
 
 class TestRestoreFromRecycleBin:
     @respx.mock