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
This commit is contained in:
Estrella Pan
2026-07-03 14:39:21 +02:00
parent d7e3f38732
commit d28f5d5e1d
2 changed files with 47 additions and 13 deletions

View File

@@ -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}")

View File

@@ -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