Route task-failure error codes through error_from_code (F18)

[cmeans-claude-dev]

Summary

• Closes Route task-failure error codes through error_from_code() for specific envelopes #30. F18 from the PR Add structured error responses with JSON envelopes #9 self-audit.
• operations.py:256 (copy/move) and operations.py:365 (delete) previously emitted a bare DSM_ERROR envelope when the background task completed with an error dict, diverging from synchronous paths in the same module that already route through error_from_code() via synology_error_response().
• Both task-completion paths now pass the DSM code through error_from_code(err_code, "SYNO.FileStation.CopyMove" | ".Delete") and use the mapped exception's error_code, retryable, and suggestion for the envelope (with the previous generic suggestion as fallback).

Behavior change

Callers catching dsm_error on these two paths will now receive the more specific code. This is the exact change QA flagged as wanting its own review.

DSM code	Before	After
408	dsm_error	not_found
414	dsm_error	already_exists
416	dsm_error	disk_full (retryable=true)
418 / 419	dsm_error	invalid_parameter
1100	dsm_error	filestation_error
105	dsm_error	permission_denied
unmapped (e.g. 9999)	dsm_error	dsm_error (unchanged)

Note on issue AC wording: the issue body uses "e.g. PERMISSION_DENIED for 1100" as an example, but error_from_code(1100, "SYNO.FileStation.*") returns FileStationError (envelope filestation_error) — 1100 is FileStation's "insufficient filesystem permissions," not common code 105. The mapping remains more specific than dsm_error; broadening 1100 → permission_denied is out of scope (it would affect synchronous paths across every FileStation module). Raised for visibility.

Changes

File	Change
src/mcp_synology/modules/filestation/operations.py	Import error_from_code; route err_code through it at both task-completion sites; use mapped.error_code, mapped.retryable, mapped.suggestion or <generic fallback>. Message text unchanged (preserves the DSM error code {err_code} on path: {err_path} format).
tests/modules/filestation/test_operations.py	Updated test_copy_task_completes_with_error + test_delete_task_completes_with_error to assert filestation_error on code 1100 and the per-code suggestion text. Added four new cases: 408→not_found (copy), 416→disk_full+retryable=true (copy), 105→permission_denied (delete), and an unknown-code (9999) fallback case confirming dsm_error + generic suggestion.
CHANGELOG.md	New ### Changed entry under ## Unreleased (#30).

Test assertions that changed

Every existing assertion is enumerated here per issue AC:

1. test_copy_task_completes_with_error
   • body["error"]["code"] == "dsm_error" → "filestation_error"
   • Added: "shared folder" in body["error"]["suggestion"].lower() (per-code mapping now wins).
2. test_delete_task_completes_with_error
   • body["error"]["code"] == "dsm_error" → "filestation_error"

No other existing assertions changed. The timeout / poll-error tests are untouched (those paths do not go through error_from_code).

Test plan

• uv run ruff check src/ tests/ — clean
• uv run ruff format --check src/ tests/ — clean
• uv run mypy src/ — clean (strict)
• uv run pytest — 499 passed, 96.04% coverage (95% gate)
• New failing tests observed (red), then pass after impl (green)

Out of scope

• F17 (param/value) — shipped in Add param/value to five error_response() call sites (F17) #31
• Synchronous error paths — already use synology_error_response / error_from_code
• Rethinking 1100's mapping (FileStation vs common-105 semantics) — separate concern

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

Starting QA review — adding QA Active. CI fully green (lint, mypy, pytest 3.11/3.12/3.13, vdsm integration, version-sync, validate-server-json, on-push). Will run the test plan locally against this branch and re-verify the six new/updated test cases per the PR body.

[cmeans]

QA Review — PR #32

Verdict: pass — zero findings. Ready to apply Ready for QA Signoff.

Verification (re-run locally on feat/task-error-from-code @ HEAD)

Check	Result
uv run ruff check src/ tests/	clean
uv run ruff format --check src/ tests/	66 files already formatted
uv run mypy src/ (strict)	clean, 27 files
uv run pytest	499 passed, 94 deselected, 96.04% coverage (95% gate)
Targeted run of the 2 updated + 4 new task-error cases	6/6 pass
CI (all jobs)	green (lint, mypy, pytest 3.11/3.12/3.13, vdsm integration, version-sync, validate-server-json, on-push)

Deselected (94): -m 'not integration and not vdsm' from pyproject.toml addopts — integration/vdsm markers are intentionally excluded from the default suite and run in CI's separate vdsm integration tests job (passing here). Not a silent skip.

Code review

• operations.py:256 / operations.py:365 — both task-completion sites correctly call error_from_code(err_code, "SYNO.FileStation.CopyMove" | "SYNO.FileStation.Delete"). Signature verified against core/errors.py:264 (error_from_code(code: int, api_name: str = "")). Note: issue #30's code snippet had the args reversed (error_from_code("SYNO.FileStation.CopyMove", err_code)) — the PR uses the real signature. No bug.
• Envelope fields — mapped.error_code, mapped.retryable, mapped.suggestion or <generic fallback> all resolve correctly against the SynologyError hierarchy (core/errors.py:72–148). Message text is preserved verbatim (DSM error code {err_code} on path: {err_path}) — no caller-visible string break.
• Issue-AC pushback on 1100 — PR correctly documents that error_from_code(1100, "SYNO.FileStation.*") returns FileStationError (envelope filestation_error), not common-105's permission_denied. 1100 is FileStation's filesystem-permissions error and should stay filestation_error; broadening to permission_denied would be a different and larger change. The issue's "e.g. PERMISSION_DENIED for 1100" wording was illustrative, not load-bearing. Agreed — out of scope.
• CHANGELOG — one ### Changed entry under ## Unreleased, standard Keep-a-Changelog category, explicitly calls out the semantic change ("callers previously catching dsm_error on these two paths will now receive the more specific code").

Issue #30 AC coverage

• ✅ Both sites route through error_from_code()
• ✅ Suggestion from per-code mapping with generic fallback
• ✅ Unit tests assert specific envelope (1100 → filestation_error, 408 → not_found, 416 → disk_full+retryable, 105 → permission_denied, 9999 → dsm_error fallback)
• ✅ CHANGELOG ## Unreleased → ### Changed with behavior-change call-out
• ✅ Existing assertion changes enumerated (§"Test assertions that changed" in PR body — 2 changes, both in test_copy/delete_task_completes_with_error; timeout/poll-error tests correctly left alone since those paths don't go through error_from_code)

Scope of behavior change

Limited to the two background-task completion paths in operations.py. Synchronous paths already routed through synology_error_response() → error_from_code() — no second-order impact. Unknown codes still produce dsm_error (explicit test: test_copy_task_error_unknown_code_falls_back).

Clean, surgical F18 fix. Applying Ready for QA Signoff as my final act.

[cmeans]

LGTM