Operational test coverage gaps: re-auth retry on upload/download, get_dir_size edge cases, restore_from_recycle_bin integration

[cmeans-claude-dev]

Motivation

Companion to #96. Where #96 systematizes coverage for the multipath/array-flattening regression class, this issue tracks operational error-path gaps in the File Station and core client that are unrelated to multipath — specifically session re-auth retries, async-task edge cases on get_dir_size, and the absence of any integration/vdsm exercise of restore_from_recycle_bin.

Verified against current code at 8b0d200 (post-#97 merge). One scope item from the original brain-dump (download partial-file cleanup on OSError) was dropped after verification — already thoroughly covered by test_download_partial_cleanup, test_download_enospc_reports_disk_full, and test_download_non_enospc_oserror_reports_filesystem_error in tests/modules/filestation/test_transfer.py.

Scope

1. Re-auth retry on upload_file and download_file

Production: src/mcp_synology/core/client.py:409-427 (upload), :512-526 (download). Both branches handle DSM session-expired codes (106/107/119) by re-authenticating and replaying the request. Upload re-opens the file handle on retry; download re-issues the streamed request.

Existing coverage: tests/core/test_client.py:120-171 exercises the generic request() re-auth path with code 106, and the negative case for code 105 (permission denied — must NOT retry, per CLAUDE.md). Neither test exercises the upload or download paths specifically.

Ask: Add unit tests that cover upload_file() and download_file() re-auth specifically:

• Upload: respx-script first POST returning 106, second POST succeeding. Assert auth.login() called between attempts and the file pointer was reset/reopened so the second POST sends the full payload.
• Download: respx-script first GET returning 106, second GET streaming successfully. Assert the partial file (if any) was cleaned up before retry, and the final file matches the streamed bytes.
• Negative case mirroring 105 for both upload and download: no retry, error surfaces with the standard 105 mapping.

2. get_dir_size async-task edge cases

Production: src/mcp_synology/modules/filestation/metadata.py:172-259.

Existing coverage: tests/modules/filestation/test_metadata.py:195-316 covers happy path (test_dir_size_success), start error (test_dir_size_start_error), poll error (test_dir_size_poll_error), 599 instant-completion vDSM quirk (test_dir_size_error_599_instant_completion), and polling timeout (test_dir_size_timeout). Integration: tests/test_integration.py:511-518 covers the happy path on a real NAS.

Gaps:

• Status dict missing expected keys (e.g., finished absent, total_size absent) — currently the polling loop assumes shape; no test asserts graceful failure.
• Partial progress observed mid-poll (status dict reports processing_path but no totals yet) — verify the formatter handles incomplete intermediate states without crashing.
• Background task cleanup on timeout: per CLAUDE.md "All async background tasks ... must use try/finally to ensure stop/clean is called. Orphaned tasks consume CPU indefinitely on the NAS." The test_dir_size_timeout test should also assert that the cleanup call (stop/clean) was issued in the finally branch.

Ask: Add three unit tests covering the above. The cleanup-on-timeout assertion is the highest-value one — it's a CLAUDE.md-documented invariant with no test backstop today.

3. restore_from_recycle_bin integration/vDSM coverage

Production: src/mcp_synology/modules/filestation/operations.py:530-593.

Existing coverage: tests/modules/filestation/test_operations.py:799-844 — four unit tests using respx mocks (test_restore_success, test_restore_to_custom_dest, test_restore_full_path, test_restore_error). Zero integration or vDSM tests — tests/test_integration.py and tests/vdsm/test_vdsm_integration.py do not import or exercise the restore handler at all.

Overlap with #96: #96 item 2 already asks for a "dedicated unit + vdsm test for restore_from_recycle_bin multipath" framed under the multipath agenda. This issue's ask is the broader operational case — single-path restore-after-delete, restore to a custom destination, restore when source no longer exists in #recycle. A single PR can satisfy both issues if the test class covers single-path and multi-path scenarios end-to-end with a real #recycle populated by a prior delete.

Ask: Add a TestRestoreRecycleBin integration class that:

1. Deletes a fixture file/folder so it lands in #recycle.
2. Invokes restore_from_recycle_bin with default destination — asserts the original path is restored and the recycle entry is gone.
3. Invokes with an explicit dest_folder_path — asserts file lands at the override location.
4. Multipath restore (satisfies Systematic single-path + multi-path test coverage for File Station write tools, with vdsm integration #96 item 2) — restores N entries in one call, asserts all N appear at their original paths AND each is a distinct entry (no comma-mangled path).

Out of scope

• Download partial-file cleanup on OSError — already covered (verified during this issue's drafting).
• Concurrency / lock-coordination tests for re-auth — separate concern; the asyncio.Lock correctness is exercised implicitly by the existing race-condition unit tests.
• Performance benchmarks for re-auth latency.
• General integration coverage for read tools (list_files, search_files, etc.) — these have happy-path integration coverage and no operational-edge-case backstop is requested here.

Why now

The patterns flagged here are the kinds of bugs that show up in production but rarely under unit-test fixtures: a session expires mid-upload of a 5 GB file; a get_dir_size task gets orphaned because the cleanup never ran; a user restores from recycle bin and it silently no-ops because the multipath payload was wrong. Each is a CLAUDE.md-documented invariant or a documented production failure mode. Closing them now is one bounded PR; chasing each in production costs a fix + release + coordination cycle.

References

• Sister issue (multipath regression class): Systematic single-path + multi-path test coverage for File Station write tools, with vdsm integration #96
• CLAUDE.md sections referenced: "Background task cleanup" (Error Handling), "Session errors trigger transparent re-auth" (Auth)
• Verification commit: 8b0d200