Backup pipeline robustness follow-up: event-loop blocking, task refs, argv

[ClaydeCode]

Follow-up to a4a80bc, closes #127. Four robustness fixes in shard_core/service/backup.py, each with tests in tests/test_backup.py. This file has bitten repeatedly (#52, #58, #74, #106 → #117/#119), so it's treated with tests-first care.

Changes

1. Unblock the event loop on the Azure marker upload. _write_marker_blob called the sync BlobClient.upload_blob directly on the loop; a slow Azure call stalled everything, including the auth path. Now await asyncio.to_thread(...); the function is async and its caller awaits it.
2. Unblock the event loop on rclone obscure. Replaced blocking subprocess.run with asyncio.create_subprocess_exec. Also added an exit-code check that raises BackupFailedError — previously a failed obscure silently returned an empty crypt-password (broken encryption). (Small deliberate hardening beyond the literal ask, consistent with the "robustness follow-up" theme; called out for review.)
3. Hold strong references to fire-and-forget tasks. asyncio holds tasks weakly, so a long backup task (and the unreferenced on_task_done wrapper) could be GC'd mid-run. Added a module-level background_tasks set with a discard callback, mirroring app_lifecycle.py.
4. Build the rclone argv as a list instead of formatting a string and calling command.split(), which broke on any path or SAS URL containing whitespace. Replaces the COMMAND_TEMPLATE / CLEARTEXT_COMMAND_TEMPLATE strings with _build_backup_command / _build_cleartext_backup_command; the v18: all backups fail — unknown rclone flag --azureblob-no-check-container (release blocker) #117 unsupported-flag guard now runs against both argv builders.

Verification

• tests/test_backup.py: 15 passed (8 new/strengthened). New tests teeth-checked by breaking the code under them and confirming they fail.
• Full suite: 233 passed, 1 failed. The single failure (test_app_lifecycle.py::test_app_starts_and_stops) is environmental — the dev VM's own clayde-traefik-1 permanently binds host port 80, which the quick_stop mock app (ports: 80:80) collides with (Bind for :::80 failed: port is already allocated). Unrelated to this diff; it passes on a clean CI runner.
• ruff check .: clean.

Review panel

Four adversarial reviewers ran (diff + issue only): adversarial correctness, test adversary, DevEx/readability, security (SAS URL + passphrase are passed as subprocess argv).

Blocking findings (both from the test adversary — fixed in 63d7200):

• Strong-ref test only covered the main task, not the spawned on_task_done wrapper → now spies on _register_background_task and asserts both tasks register.
• The backup-outcome signal (success + failure-with-exception) was entirely unasserted → added tests for both; teeth-verified.

Advisory, addressed: assert unsplit argv reaches create_subprocess_exec; exercise Path→str and the marker URL surgery / payload forwarding + an in-thread upload error; rename the done-callback lambda param to stop shadowing task.

Advisory, dismissed with reason:

• obscure stderr reaches the UI websocket (security). No reachable leak: rclone obscure doesn't echo its argv, and the passphrase is machine-generated diceware. Matches the existing _backup_directory stderr handling; changing both sites is out of scope.
• _build_cleartext_backup_command is dead in production (correctness/DevEx). Pre-existing (the old CLEARTEXT_COMMAND_TEMPLATE was likewise test-only); kept so the v18: all backups fail — unknown rclone flag --azureblob-no-check-container (release blocker) #117 flag guard covers both argv paths. Not deleting pre-existing dead code in this PR.

🤖 Generated with Claude Code