Re-arm removals for downloads that never left the queue

src/job_manager.py

@@ -136,12 +136,48 @@ async def _queue_has_items(self):
                 "job_runner/full_queue at start: %s",
                 queue_manager.format_queue(full_queue),
             )
+            self._forget_deleted_downloads_still_in_queue(full_queue)
             return True
 
         self.arr.tracker.reset()
         logger.verbose("Removal Jobs: None triggered (Queue is empty)")
         return False
 
+    def _forget_deleted_downloads_still_in_queue(self, full_queue):
+        """
+        Drop downloads from the deleted tracker that are still in the queue.
+
+        The deleted tracker stops the same download from being removed twice
+        within one run, but is otherwise only cleared once the queue is empty.
+        A download that is still in the queue was evidently not removed, so
+        keeping it in the tracker would silently suppress every future removal
+        attempt for as long as decluttarr runs.
+
+        Downloads handled via obsolete_tag are exempt: they are meant to stay in
+        the queue, so their presence is not evidence of a failed removal.
+        """
+        queue_download_ids = {item.get("downloadId") for item in full_queue} - {None}
+        still_in_queue = {
+            download_id
+            for download_id in self.arr.tracker.deleted
+            if download_id in queue_download_ids
+            and download_id not in self.arr.tracker.obsolete_tagged
+        }
+        if not still_in_queue:
+            return
+
+        logger.debug(
+            "job_manager.py/_forget_deleted_downloads_still_in_queue: "
+            "Marked as deleted but still in queue (removal did not take effect), "
+            "thus eligible for removal again: %s",
+            ", ".join(sorted(still_in_queue)),
+        )
+        self.arr.tracker.deleted = [
+            download_id
+            for download_id in self.arr.tracker.deleted
+            if download_id not in still_in_queue
+        ]
+
     async def _download_clients_connected(self):
         for clients in [
             self.settings.download_clients.qbittorrent,

src/jobs/removal_handler.py

@@ -23,6 +23,7 @@ async def remove_downloads(self, affected_downloads, blocklist):
                 await self._remove_download(affected_download, download_id, blocklist)
             elif handling_method == "obsolete_tag":
                 await self._tag_as_obsolete(affected_download, download_id)
+                self.arr.tracker.obsolete_tagged.append(download_id)
 
             # Print out detailed removal messages (if any)
             if "removal_messages" in affected_download:

src/settings/_instances.py

@@ -29,6 +29,7 @@ def __init__(self):
         self.defective = {}
         self.download_progress = {}
         self.deleted = []
+        self.obsolete_tagged = []
         self.extension_checked = []
 
     def reset(self) -> None:
@@ -38,6 +39,7 @@ def reset(self) -> None:
             self.defective,
             self.download_progress,
             self.deleted,
+            self.obsolete_tagged,
             self.extension_checked,
         ):
             attr.clear()

tests/jobs/test_job_manager.py

@@ -4,6 +4,9 @@
 import requests
 
 from src.job_manager import JobManager
+from src.jobs.removal_handler import RemovalHandler
+from src.settings._instances import Tracker
+from src.utils.queue_manager import QueueManager
 
 
 @pytest.mark.asyncio
@@ -75,3 +78,136 @@ async def test_run_download_client_jobs_handles_connection_check_request_errors(
         result = await manager.run_download_client_jobs()
 
     assert result is None
+
+
+def _job_manager_with_deleted(deleted):
+    manager = JobManager(MagicMock())
+    manager.arr = MagicMock()
+    manager.arr.tracker = Tracker()
+    manager.arr.tracker.deleted.extend(deleted)
+    return manager
+
+
+def _queue_item(download_id, queue_id=1):
+    return {
+        "id": queue_id,
+        "downloadId": download_id,
+        "title": "Some.Release",
+        "protocol": "torrent",
+        "downloadClient": "qBittorrent",
+    }
+
+
+@pytest.mark.asyncio
+async def test_queue_has_items_forgets_deleted_downloads_that_are_back_in_queue():
+    """A removal that did not stick must not block all future removals.
+
+    The deleted tracker is otherwise only cleared once the queue is empty, so a
+    download that keeps coming back would be silently skipped by RemovalHandler
+    forever (strikes keep counting up, but no removal is ever triggered).
+    """
+    # Arrange
+    manager = _job_manager_with_deleted(["still-in-queue", "actually-removed"])
+
+    get_queue_items = AsyncMock(return_value=[_queue_item("still-in-queue")])
+
+    # Act
+    with patch.object(QueueManager, "get_queue_items", get_queue_items):
+        has_items = await manager._queue_has_items()  # pylint: disable=W0212
+
+    # Assert
+    assert has_items is True
+    assert manager.arr.tracker.deleted == ["actually-removed"]
+    # The orphans job that triggered this bug only shows up in the full queue
+    get_queue_items.assert_awaited_once_with("full")
+
+
+@pytest.mark.asyncio
+async def test_forgotten_download_is_removed_again_on_the_next_run():
+    """After being forgotten, the download is removed instead of silently skipped."""
+    # Arrange
+    manager = _job_manager_with_deleted(["comes-back"])
+    manager.settings.general.public_tracker_handling = "remove"
+    manager.settings.download_clients.qbittorrent = []
+    manager.settings.download_clients.get_download_client_by_name.return_value = (
+        None,
+        None,
+    )
+    manager.arr.remove_queue_item = AsyncMock()
+    queue_item = _queue_item("comes-back", queue_id=42)
+
+    # Act
+    with patch.object(
+        QueueManager, "get_queue_items", AsyncMock(return_value=[queue_item])
+    ):
+        await manager._queue_has_items()  # pylint: disable=W0212
+    await RemovalHandler(
+        arr=manager.arr,
+        settings=manager.settings,
+        job_name="remove_metadata_missing",
+    ).remove_downloads(
+        {"comes-back": {**queue_item, "queue_ids": [queue_item["id"]]}},
+        blocklist=True,
+    )
+
+    # Assert
+    manager.arr.remove_queue_item.assert_awaited_once_with(queue_id=42, blocklist=True)
+
+
+@pytest.mark.asyncio
+async def test_queue_has_items_keeps_deleted_downloads_that_are_gone():
+    """Downloads that really were removed stay in the tracker (no double removal)."""
+    # Arrange
+    manager = _job_manager_with_deleted(["actually-removed"])
+
+    # Act
+    with patch.object(
+        QueueManager,
+        "get_queue_items",
+        AsyncMock(return_value=[_queue_item("something-else")]),
+    ):
+        has_items = await manager._queue_has_items()  # pylint: disable=W0212
+
+    # Assert
+    assert has_items is True
+    assert manager.arr.tracker.deleted == ["actually-removed"]
+
+
+@pytest.mark.asyncio
+async def test_obsolete_tagged_download_is_not_tagged_again_on_the_next_run():
+    """Obsolete-tagged downloads stay in the queue by design and must not be re-tagged.
+
+    With private_tracker_handling/public_tracker_handling set to "obsolete_tag",
+    the download is tagged instead of removed and deliberately remains in the
+    queue, so its presence is not evidence of a failed removal.
+    """
+    # Arrange
+    manager = _job_manager_with_deleted([])
+    manager.settings.general.public_tracker_handling = "obsolete_tag"
+    manager.settings.general.obsolete_tag = "Obsolete"
+    qbit = MagicMock(ready=True, set_tag=AsyncMock())
+    manager.settings.download_clients.qbittorrent = [qbit]
+    manager.settings.download_clients.get_download_client_by_name.return_value = (
+        qbit,
+        "qbittorrent",
+    )
+    queue_item = _queue_item("tagged", queue_id=7)
+
+    # Act
+    for _ in range(2):
+        with patch.object(
+            QueueManager, "get_queue_items", AsyncMock(return_value=[queue_item])
+        ):
+            await manager._queue_has_items()  # pylint: disable=W0212
+        await RemovalHandler(
+            arr=manager.arr,
+            settings=manager.settings,
+            job_name="remove_stalled",
+        ).remove_downloads(
+            {"tagged": {**queue_item, "queue_ids": [queue_item["id"]]}},
+            blocklist=False,
+        )
+
+    # Assert
+    qbit.set_tag.assert_awaited_once_with(tags=["Obsolete"], hashes=["tagged"])
+    manager.arr.remove_queue_item.assert_not_called()