fix(download): remove a cancelled torrent that never started

shelfmark/download/clients/base_handler.py

@@ -451,6 +451,8 @@ def _handle_cancelled_download(
         download_id: str,
         protocol: str,
         status_callback: Callable[[str, str | None], None],
+        *,
+        remove_if_unstarted: bool = False,
     ) -> None:
         if protocol == "usenet":
             logger.info("Download cancelled, removing from %s: %s", client.name, download_id)
@@ -464,6 +466,12 @@ def _handle_cancelled_download(
                     client.name,
                     e,
                 )
+        elif remove_if_unstarted and self._remove_unstarted_torrent(client, download_id):
+            logger.info(
+                "Download cancelled before it started, removed from %s: %s",
+                client.name,
+                download_id,
+            )
         else:
             logger.info(
                 "Download cancelled for protocol=%s; leaving in %s: %s",
@@ -473,6 +481,24 @@ def _handle_cancelled_download(
             )
         status_callback("cancelled", "Cancelled")
 
+    def _remove_unstarted_torrent(self, client: DownloadClient, download_id: str) -> bool:
+        """Remove a torrent that downloaded nothing; True if it was removed.
+
+        Torrents are normally left in the client after a cancel so an already-downloaded one
+        can keep seeding. One at 0% has nothing to seed and nothing to resume, and leaving it
+        queued only holds a download slot and piles up dead entries.
+        """
+        try:
+            status = client.get_status(download_id)
+            if status.progress > 0:  # it has data, so it may be seeding or resumable
+                return False
+            return bool(client.remove(download_id, delete_files=True))
+        except _CLIENT_CLEANUP_ERRORS as e:
+            logger.warning(
+                "Could not remove unstarted torrent %s from %s: %s", download_id, client.name, e
+            )
+            return False
+
     def _resolve_download_path_once(
         self,
         client: DownloadClient,
@@ -908,6 +934,7 @@ def download(
                 cancel_flag=cancel_flag,
                 progress_callback=progress_callback,
                 status_callback=status_callback,
+                added_by_shelfmark=existing is None,
             )
 
         except Exception as e:
@@ -924,8 +951,14 @@ def _poll_and_complete(
         cancel_flag: Event,
         progress_callback: Callable[[float], None],
         status_callback: Callable[[str, str | None], None],
+        *,
+        added_by_shelfmark: bool = False,
     ) -> str | None:
-        """Poll the download client for progress and handle completion."""
+        """Poll the download client for progress and handle completion.
+
+        ``added_by_shelfmark`` is False when the task joined a torrent the client already had;
+        such a torrent is never removed on cancel.
+        """
         poll_interval = self._poll_interval()
         # Track consecutive "not found" errors - torrents may take time to appear in client
         not_found_count = 0
@@ -1026,7 +1059,13 @@ def _poll_and_complete(
 
             # Handle cancellation
             if cancel_flag.is_set():
-                self._handle_cancelled_download(client, download_id, protocol, status_callback)
+                self._handle_cancelled_download(
+                    client,
+                    download_id,
+                    protocol,
+                    status_callback,
+                    remove_if_unstarted=added_by_shelfmark,
+                )
                 return None
 
             # Handle completed file (wait briefly for files to appear)
@@ -1038,7 +1077,13 @@ def _poll_and_complete(
             )
             if not source_path_obj:
                 if cancel_flag.is_set():
-                    self._handle_cancelled_download(client, download_id, protocol, status_callback)
+                    self._handle_cancelled_download(
+                        client,
+                        download_id,
+                        protocol,
+                        status_callback,
+                        remove_if_unstarted=added_by_shelfmark,
+                    )
                     return None
                 status_callback(
                     "error",

tests/download/test_torrent_removal.py

@@ -0,0 +1,100 @@
+"""A cancelled torrent that never started is removed from the client."""
+
+from __future__ import annotations
+
+from threading import Event
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from shelfmark.core.models import DownloadTask
+from shelfmark.download.clients import DownloadState, DownloadStatus
+from shelfmark.release_sources.prowlarr.handler import ProwlarrHandler
+
+
+def _status(state: DownloadState, progress: float = 0.0, **kw) -> DownloadStatus:
+    return DownloadStatus(
+        progress=progress,
+        state=state,
+        message="Queued" if state == DownloadState.QUEUED else None,
+        complete=kw.get("complete", False),
+        file_path=None,
+    )
+
+
+class _Recorder:
+    def __init__(self) -> None:
+        self.events: list[tuple[str, str | None]] = []
+
+    def status(self, status: str, message: str | None) -> None:
+        self.events.append((status, message))
+
+
+Q, D = DownloadState.QUEUED, DownloadState.DOWNLOADING
+
+
+# --- removing a torrent that never started -----------------------------------------------
+
+
+def _cancel(status: DownloadStatus, *, existing: bool, protocol: str = "torrent") -> MagicMock:
+    client = MagicMock()
+    client.name = "qbittorrent"
+    client.find_existing.return_value = ("dl", status) if existing else None
+    client.add_download.return_value = "dl"
+    client.remove.return_value = True
+    cancel = Event()
+
+    def get_status(_id: str) -> DownloadStatus:
+        cancel.set()  # the user cancels while the download is being polled
+        return status
+
+    client.get_status.side_effect = get_status
+    release = {"protocol": protocol, "magnetUrl": "magnet:?xt=urn:btih:abc123"}
+    with (
+        patch("shelfmark.release_sources.prowlarr.handler.get_release", return_value=release),
+        patch("shelfmark.release_sources.prowlarr.handler.get_client", return_value=client),
+        patch("shelfmark.release_sources.prowlarr.handler.POLL_INTERVAL", 0.01),
+    ):
+        ProwlarrHandler().download(
+            task=DownloadTask(task_id="c", source="prowlarr", title="Book"),
+            cancel_flag=cancel,
+            progress_callback=lambda _p: None,
+            status_callback=_Recorder().status,
+        )
+    return client
+
+
+@pytest.mark.parametrize("state", [DownloadState.QUEUED, DownloadState.DOWNLOADING])
+def test_a_cancelled_torrent_that_downloaded_nothing_is_removed(state: DownloadState) -> None:
+    client = _cancel(_status(state, 0.0), existing=False)
+
+    client.remove.assert_called_once_with("dl", delete_files=True)
+
+
+def test_a_cancelled_torrent_with_data_is_left_for_seeding() -> None:
+    client = _cancel(_status(D, 12.0), existing=False)
+
+    client.remove.assert_not_called()
+
+
+def test_a_cancelled_torrent_the_user_already_had_is_never_removed() -> None:
+    client = _cancel(_status(Q, 0.0), existing=True)
+
+    client.remove.assert_not_called()
+
+
+def test_a_complete_or_seeding_torrent_is_not_removed() -> None:
+    client = _cancel(_status(DownloadState.SEEDING, 100.0, complete=True), existing=False)
+
+    client.remove.assert_not_called()
+
+
+def test_a_failed_removal_does_not_break_the_cancel() -> None:
+    status = _status(Q, 0.0)
+    client = MagicMock()
+    client.remove.side_effect = OSError("boom")
+    client.get_status.return_value = status
+
+    ProwlarrHandler()._handle_cancelled_download(
+        client, "dl", "torrent", lambda *_a: None, remove_if_unstarted=True
+    )  # does not raise