From d28f5d5e1dacfa3e4722e125a48c9aa8fa23bb00 Mon Sep 17 00:00:00 2001
From: Estrella Pan <estrellaxd05@gmail.com>
Date: Fri, 3 Jul 2026 14:39:21 +0200
Subject: [PATCH] fix(qb): rename verify must see new_path before reporting
 success (#754 #749)

The verify loop treated "neither old nor new name in the file list" as a
successful rename. A qBittorrent 200 that never materializes (target name
taken by a duplicate from another source, file list anomaly) was reported
as success, so the renamer re-renamed and re-notified every rename cycle.
Now only an observed new_path returns True; everything else returns False
and backs off via _PENDING_RENAME_COOLDOWN.

Regression tests adapted from PR #1037 by cyber-ayi, which diagnosed the
false-positive contract.

Co-authored-by: cyber-ayi <cyber-ayi@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FAxVyRwrY7z5NotTtgnwVs
---
 .../module/downloader/client/qb_downloader.py | 20 ++++------
 backend/src/test/test_qb_downloader.py        | 40 +++++++++++++++++++
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/backend/src/module/downloader/client/qb_downloader.py b/backend/src/module/downloader/client/qb_downloader.py
index 64a9625d..0fc127b8 100644
--- a/backend/src/module/downloader/client/qb_downloader.py
+++ b/backend/src/module/downloader/client/qb_downloader.py
@@ -377,24 +377,18 @@ class QbDownloader:
                 delay = 0.1 * (2**attempt)
                 await asyncio.sleep(delay)
                 files = await self.torrents_files(torrent_hash)
-                for f in files:
-                    if f.get("name") == new_path:
-                        return True
-                    if f.get("name") == old_path:
-                        # File still has old name - retry the outer backoff loop
-                        break
-                else:
-                    # Inner loop completed without hitting old_path: new_path
-                    # was already matched above (returned) or the file with
-                    # the old name is simply gone -> treat as renamed.
+                if any(f.get("name") == new_path for f in files):
                     return True
+                # new_path 未出现（旧名仍在，或两个名字都不在列表里）一律不算
+                # 成功：盲目返回 True 会让 renamer 每周期重复改名 + 发通知
+                # (#754/#749)。返回 False 由调用方走 _PENDING_RENAME_COOLDOWN 退避。
                 if attempt == 2:
-                    # Final attempt still shows the old name.
                     logger.debug(
-                        "[Downloader] Rename API returned 200 but file unchanged: %s",
+                        "[Downloader] Rename API returned 200 but %s never appeared "
+                        "(rename of %s)",
+                        new_path,
                         old_path,
                     )
-                    return False
             return False
         except (httpx.ConnectError, httpx.RequestError, httpx.TimeoutException) as e:
             logger.warning(f"[Downloader] Failed to rename file {old_path}: {e}")
diff --git a/backend/src/test/test_qb_downloader.py b/backend/src/test/test_qb_downloader.py
index 2749b413..f19e981e 100644
--- a/backend/src/test/test_qb_downloader.py
+++ b/backend/src/test/test_qb_downloader.py
@@ -719,3 +719,43 @@ class TestTorrentsDelete:
         result = await qb.torrents_delete("aaa", delete_files=True)
 
         assert result is False
+
+
+# ---------------------------------------------------------------------------
+# torrents_rename_file verify contract (#754 / #749, from PR #1037)
+# ---------------------------------------------------------------------------
+
+
+class TestRenameVerifyContract:
+    """Pin the verify contract: only trust a rename when new_path appears.
+
+    qBittorrent can answer 200 without actually renaming (e.g. while seeding,
+    or when the target name is taken by a duplicate from another source).
+    Any false True here re-triggers rename + notification every rename cycle.
+    """
+
+    def _make_qb(self, file_list, post_code: int = 200):
+        qb = QbDownloader(host="http://qb", username="u", password="p", ssl=False)
+        qb._post = AsyncMock(return_value=MagicMock(status_code=post_code))
+        qb.torrents_files = AsyncMock(return_value=file_list)
+        return qb
+
+    async def test_verify_false_when_file_keeps_old_name(self):
+        """API 200 but the file never gets renamed -> False."""
+        qb = self._make_qb([{"name": "old.mkv"}])
+        assert await qb.torrents_rename_file("h", "old.mkv", "new.mkv") is False
+
+    async def test_verify_false_when_neither_name_present(self):
+        """Neither old nor new name in the file list -> False, not a blind True."""
+        qb = self._make_qb([{"name": "unrelated.mkv"}])
+        assert await qb.torrents_rename_file("h", "old.mkv", "new.mkv") is False
+
+    async def test_verify_true_when_new_path_appears(self):
+        """A real, verified rename returns True."""
+        qb = self._make_qb([{"name": "new.mkv"}])
+        assert await qb.torrents_rename_file("h", "old.mkv", "new.mkv") is True
+
+    async def test_verify_409_conflict_is_false(self):
+        """Target already exists (duplicate from another source) -> False."""
+        qb = self._make_qb([{"name": "old.mkv"}], post_code=409)
+        assert await qb.torrents_rename_file("h", "old.mkv", "new.mkv") is False