fix(review): tighten qB login check, propagate delete failures end-to-end, close cache race

Post-review fixes for the batch-3.2.8 branch:

- auth() and the setup wizard again require a positive success marker on
  200 responses ('Ok.' body) so a proxy or non-qB service answering
  200 + HTML is not mistaken for a login; 204 stays the qB >= 5.2 success.
- delete_rule no longer wraps a failed torrent deletion into a 200 reply,
  and the WebUI torrents-delete endpoint reports the failure instead of
  answering 'Torrents deleted' unconditionally.
- The close-on-failed-auth teardown now lives once in
  DownloadClient.__aenter__ (client.logout() before raising) instead of
  being copy-pasted into each concrete client.
- search_all() records a cache generation before querying and skips the
  cache write if an invalidation landed in between, so a stale snapshot
  can no longer overwrite a newer invalidation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014w1Z6Nxy6XTRgkFXqPr9Zh

backend/src/module/api/downloader.py

@@ -53,8 +53,14 @@ async def resume_torrents(req: TorrentHashesRequest):
 async def delete_torrents(req: TorrentDeleteRequest):
     hashes = "|".join(req.hashes)
     async with DownloadClient() as client:
-        await client.delete_torrent(hashes, delete_files=req.delete_files)
-    return {"msg_en": "Torrents deleted", "msg_zh": "种子已删除"}
+        ok = await client.delete_torrent(hashes, delete_files=req.delete_files)
+    if not ok:
+        return {
+            "status": False,
+            "msg_en": "Failed to delete torrents",
+            "msg_zh": "删除种子失败",
+        }
+    return {"status": True, "msg_en": "Torrents deleted", "msg_zh": "种子已删除"}
 
 
 @router.post("/torrents/tag", dependencies=[Depends(get_current_user)])

backend/src/module/api/setup.py

@@ -162,10 +162,11 @@ async def test_downloader(req: TestDownloaderRequest):
                 data={"username": req.username, "password": req.password},
             )
             # qBittorrent < 5.2 answers 200 + "Ok."; >= 5.2 answers 204 with
-            # an empty body on success (#1044).
-            if (
-                login_resp.status_code in (200, 204)
-                and "fails" not in login_resp.text.lower()
+            # an empty body on success (#1044). Keep the positive body check
+            # for 200 so a proxy answering 200 + HTML is not reported as a
+            # working login.
+            if login_resp.status_code == 204 or (
+                login_resp.status_code == 200 and "ok" in login_resp.text.lower()
             ):
                 return TestResultResponse(
                     success=True,

backend/src/module/database/bangumi.py

@@ -72,13 +72,17 @@ _bangumi_cache: list[Bangumi] | None = None
 _bangumi_cache_time: float = 0
 _BANGUMI_CACHE_TTL: float = 300.0  # 5 minutes - extended from 60s to reduce DB queries
 _bangumi_cache_lock = threading.Lock()
+# Bumped on every invalidation so a search_all() that was already past the
+# cache check cannot overwrite a newer invalidation with its stale snapshot.
+_bangumi_cache_gen = 0
 
 
 def _invalidate_bangumi_cache():
-    global _bangumi_cache, _bangumi_cache_time
+    global _bangumi_cache, _bangumi_cache_time, _bangumi_cache_gen
     with _bangumi_cache_lock:
         _bangumi_cache = None
         _bangumi_cache_time = 0
+        _bangumi_cache_gen += 1
 
 
 class BangumiDatabase:
@@ -376,6 +380,7 @@ class BangumiDatabase:
                 and (now - _bangumi_cache_time) < _BANGUMI_CACHE_TTL
             ):
                 return _bangumi_cache
+            gen_at_query = _bangumi_cache_gen
         statement = select(Bangumi)
         result = self.session.execute(statement)
         bangumis = list(result.scalars().all())
@@ -384,8 +389,9 @@ class BangumiDatabase:
         for b in bangumis:
             self.session.expunge(b)
         with _bangumi_cache_lock:
-            _bangumi_cache = bangumis
-            _bangumi_cache_time = now
+            if _bangumi_cache_gen == gen_at_query:
+                _bangumi_cache = bangumis
+                _bangumi_cache_time = now
         return bangumis
 
     def search_id(self, _id: int) -> Optional[Bangumi]:

backend/src/module/downloader/client/aria2_downloader.py

@@ -49,10 +49,6 @@ class Aria2Downloader:
                 )
                 await asyncio.sleep(5)
                 times += 1
-        # Failed auth: close the client here or its connection pool leaks,
-        # because DownloadClient.__aenter__ raises before __aexit__ can run.
-        await self._client.aclose()
-        self._client = None
         return False
 
     async def logout(self):

backend/src/module/downloader/client/qb_downloader.py

@@ -48,8 +48,13 @@ class QbDownloader:
                     data={"username": self.username, "password": self.password},
                 )
                 # qBittorrent < 5.2 answers 200 + "Ok." / "Fails.";
-                # qBittorrent >= 5.2 answers 204 with an empty body on success (#1044).
-                if resp.status_code in (200, 204) and "fails" not in resp.text.lower():
+                # qBittorrent >= 5.2 answers 204 with an empty body on success
+                # (#1044). Keep the positive body check for 200 so a proxy or
+                # non-qB service answering 200 + HTML is not mistaken for a
+                # successful login.
+                if (
+                    resp.status_code == 200 and resp.text.startswith("Ok")
+                ) or resp.status_code == 204:
                     return True
                 elif resp.status_code == 403:
                     logger.error("Login refused by qBittorrent Server")
@@ -82,10 +87,6 @@ class QbDownloader:
                 else:
                     logger.error(f"Unknown error: {e}")
                 break
-        # Failed auth: DownloadClient.__aenter__ raises before __aexit__ can run,
-        # so the client must be closed here or its connection pool leaks.
-        await self._client.aclose()
-        self._client = None
         return False
 
     async def logout(self):

backend/src/module/downloader/download_client.py

@@ -52,6 +52,10 @@ class DownloadClient(TorrentPath):
         if not self.authed:
             await self.auth()
             if not self.authed:
+                # __aexit__ never runs when we raise here, so close the
+                # concrete client's connection pool now or it leaks on every
+                # failed connect (#1043).
+                await self.client.logout()
                 raise ConnectionError("Download client authentication failed")
         else:
             logger.error("[Downloader] Already authed.")

backend/src/module/manager/torrent.py

@@ -58,6 +58,14 @@ class TorrentManager(Database):
                 torrent_message = None
                 if file:
                     torrent_message = await self.delete_torrents(data, client)
+                    if torrent_message.status_code == 500:
+                        return ResponseModel(
+                            status_code=500,
+                            status=False,
+                            msg_en=f"Deleted rule for {data.official_title}, "
+                            "but deleting its torrents failed.",
+                            msg_zh=f"已删除 {data.official_title} 规则，但删除种子失败。",
+                        )
                 logger.info(f"[Manager] Delete rule for {data.official_title}")
                 return ResponseModel(
                     status_code=200,

backend/src/test/conftest.py

@@ -222,5 +222,5 @@ def mock_download_client():
     ]
     client.pause_torrent.return_value = None
     client.resume_torrent.return_value = None
-    client.delete_torrent.return_value = None
+    client.delete_torrent.return_value = True
     return client

backend/src/test/test_api_downloader.py

@@ -62,7 +62,7 @@ def mock_download_client():
     ]
     client.pause_torrent.return_value = None
     client.resume_torrent.return_value = None
-    client.delete_torrent.return_value = None
+    client.delete_torrent.return_value = True
     return client
 
 
@@ -433,3 +433,24 @@ class TestAutoTagTorrents:
         assert data["unmatched_count"] == 1
         assert len(data["unmatched"]) == 1
         mock_download_client.add_tag.assert_not_called()
+
+
+class TestDeleteTorrentsFailure:
+    def test_delete_failure_is_reported(self, authed_client, mock_download_client):
+        """A rejected qB delete must not be reported as success (#1046)."""
+        mock_download_client.delete_torrent.return_value = False
+        with patch("module.api.downloader.DownloadClient") as MockClient:
+            MockClient.return_value.__aenter__ = AsyncMock(
+                return_value=mock_download_client
+            )
+            MockClient.return_value.__aexit__ = AsyncMock(return_value=False)
+
+            response = authed_client.post(
+                "/api/v1/downloader/torrents/delete",
+                json={"hashes": ["abc123"], "delete_files": True},
+            )
+
+        assert response.status_code == 200
+        data = response.json()
+        assert data["status"] is False
+        assert data["msg_en"] == "Failed to delete torrents"

backend/src/test/test_download_client.py

@@ -48,6 +48,19 @@ class TestAuth:
         await download_client.auth()
         assert download_client.authed is False
 
+    async def test_aenter_closes_client_on_failed_auth(
+        self, download_client, mock_qb_client
+    ):
+        """__aenter__ must close the concrete client's pool before raising,
+        because __aexit__ never runs on a failed connect (leak fix, #1043)."""
+        mock_qb_client.auth.return_value = False
+
+        with pytest.raises(ConnectionError):
+            async with download_client:
+                pass
+
+        mock_qb_client.logout.assert_awaited_once()
+
 
 # ---------------------------------------------------------------------------
 # init_downloader

backend/src/test/test_qb_downloader.py

@@ -584,27 +584,31 @@ class TestAuthQb52Compat:
 
         assert result is False
 
-    async def test_auth_closes_client_on_failure(self):
-        """A failed auth must aclose() the httpx client (leak fix)."""
+    async def test_auth_returns_false_on_200_html_body(self):
+        """A 200 + HTML page (proxy, wrong service) is not a successful login."""
         qb = QbDownloader(
             host="localhost:8080", username="admin", password="pass", ssl=False
         )
 
         mock_client = AsyncMock()
         mock_resp = MagicMock()
-        mock_resp.status_code = 403
-        mock_resp.text = "Forbidden"
+        mock_resp.status_code = 200
+        mock_resp.text = "<html><body>Sign in</body></html>"
         mock_client.post = AsyncMock(return_value=mock_resp)
 
-        with patch(
-            "module.downloader.client.qb_downloader.httpx.AsyncClient",
-            return_value=mock_client,
+        with (
+            patch(
+                "module.downloader.client.qb_downloader.httpx.AsyncClient",
+                return_value=mock_client,
+            ),
+            patch(
+                "module.downloader.client.qb_downloader.asyncio.sleep",
+                new_callable=AsyncMock,
+            ),
         ):
-            result = await qb.auth()
+            result = await qb.auth(retry=1)
 
         assert result is False
-        mock_client.aclose.assert_awaited_once()
-        assert qb._client is None
 
 
 # ---------------------------------------------------------------------------

backend/src/test/test_setup.py

@@ -373,3 +373,19 @@ class TestTestDownloaderHardening:
         assert data["success"] is False
         assert "secret-detail-xyz" not in data["message_en"]
         assert "secret-detail-xyz" not in data["message_zh"]
+
+    def test_login_rejects_200_html_body(self, client, mock_first_run):
+        """A proxy answering 200 + HTML to the login POST is not a success."""
+        from unittest.mock import MagicMock
+
+        get_resp = MagicMock(text="qBittorrent WebUI")
+        login_resp = MagicMock(
+            status_code=200, text="<html><body>portal</body></html>"
+        )
+        cls_patch = self._mock_client(get_resp=get_resp, login_resp=login_resp)
+        try:
+            response = self._post(client)
+        finally:
+            cls_patch.stop()
+        assert response.status_code == 200
+        assert response.json()["success"] is False