Fix nyaa grabs being silently dropped as false magnet rejections

add_torrent_response_is_rejected treated success_count == 0 alone as
an outright rejection. qBittorrent returns exactly that (with
pending_count: 1) for every URL-based add while it fetches the
torrent asynchronously — the shape nyaa's RSS feed always uses, since
it hands over a .torrent download URL, never a magnet. Every nyaa
grab was therefore marked MagnetRejected and silently dropped (no
release row, no log line) while the torrent downloaded successfully
in the background. With no release row, best_existing_score always
saw None, so the same episode got re-grabbed from every new feed
entry — the repeated duplicate downloads sitting in
/mnt/media/downloads/ (Mushoku Tensei, Tenki no Ko, Code Geass) traced
back to this. Root-caused by an Opus 5 investigation.

Also stop treating a genuine rejection as silent: process_item now
records a failed release row and logs a warning, matching the
existing HashCaptureFailed pattern, instead of just marking the item
seen and moving on with no trace.

breadarrd/src/importer/mod.rs

@@ -335,6 +335,25 @@ fn move_or_copy_file(src: &Path, dest: &Path) -> Result<()> {
     })
 }
 
+/// Removes whatever's left of a torrent's download folder once every file
+/// breadarr cares about has already been moved out of it — split out from
+/// `run_import_cycle` so it's directly testable without a real qBittorrent
+/// server. `move_or_copy_file` only ever relocates the one video file it
+/// located; a multi-file release's own folder (sample clips, `.nfo`/`.srt`/
+/// `.jpg` sidecars) is otherwise left behind forever, since nothing else
+/// ever points at it once the torrent itself is gone from qBittorrent. A
+/// bare file (a release with no wrapping folder) needs no cleanup here —
+/// `move_or_copy_file` already consumed it. The `!= host_downloads_path`
+/// guard is defense in depth against a malformed `content_path` resolving
+/// to the downloads root itself; every real torrent lands in its own
+/// subdirectory or as a single file, never the root.
+fn cleanup_leftover_download_dir(content_path: &Path, host_downloads_path: &str) -> Result<()> {
+    if content_path.is_dir() && content_path != Path::new(host_downloads_path) {
+        std::fs::remove_dir_all(content_path)?;
+    }
+    Ok(())
+}
+
 /// The copy fallback's actual mechanics, split out so it's directly
 /// testable without needing a real cross-filesystem boundary to force
 /// `rename` to fail.
@@ -1354,10 +1373,18 @@ pub async fn run_import_cycle(
     // the torrent itself is just forgotten rather than left sitting around
     // in a "files missing" error state. Best-effort: a failed delete here
     // never undoes or blocks the import that already succeeded.
-    for hash in &imported_hashes {
+    for (hash, content_path) in &imported_hashes {
         if let Err(e) = qbit.delete_torrent(hash).await {
             tracing::warn!(hash, error = %e, "failed to remove completed torrent from qbittorrent");
         }
+        if let Err(e) = cleanup_leftover_download_dir(content_path, host_downloads_path) {
+            tracing::warn!(
+                hash,
+                path = %content_path.display(),
+                error = %e,
+                "failed to remove leftover download directory after import"
+            );
+        }
     }
 
     // Best-effort, same reasoning as the `delete_torrent` cleanup just
@@ -1394,16 +1421,20 @@ fn process_pending_grabs(
     container_downloads_path: &str,
     host_downloads_path: &str,
     transcode_cfg: Option<&breadarr_shared::config::TranscodeConfig>,
-) -> Result<(ImportStats, Vec<String>)> {
+) -> Result<(ImportStats, Vec<(String, PathBuf)>)> {
     let mut stats = ImportStats::default();
-    // Torrent hashes whose data has been fully dealt with this cycle — moved
-    // into the library, or deleted outright because a better file already
-    // existed (see `ImportOutcome::SkippedAlreadyHaveBetter`, whose only
-    // producer, `import_one`, deletes the losing download itself). Either
-    // way nothing remains at the torrent's original location, so the caller
-    // (`run_import_cycle`, the async I/O boundary this function is
-    // deliberately kept free of) removes each from qBittorrent afterward.
-    let mut imported_hashes = Vec::new();
+    // Torrents whose data has been fully dealt with this cycle — moved into
+    // the library, or deleted outright because a better file already existed
+    // (see `ImportOutcome::SkippedAlreadyHaveBetter`, whose only producer,
+    // `import_one`, deletes the losing download itself). Either way nothing
+    // *breadarr still needs* remains at the torrent's original location, so
+    // the caller (`run_import_cycle`, the async I/O boundary this function is
+    // deliberately kept free of) removes each from qBittorrent afterward and
+    // clears out whatever's left of `content_path` — a multi-file release
+    // only ever has its video moved out by `move_or_copy_file`, so without
+    // this the surrounding folder (sample clips, .nfo/.srt/.jpg sidecars)
+    // sits there forever with no torrent left to account for it.
+    let mut imported_hashes: Vec<(String, PathBuf)> = Vec::new();
 
     for grab in pending {
         let Some(torrent) = torrents.iter().find(|t| t.hash == grab.torrent_hash()) else {
@@ -1467,7 +1498,7 @@ fn process_pending_grabs(
                     stats.imported += outcome.episodes_imported;
                     stats.quality_flagged += outcome.quality_flagged;
                     if outcome.episodes_imported > 0 {
-                        imported_hashes.push(grab.torrent_hash().to_string());
+                        imported_hashes.push((grab.torrent_hash().to_string(), content_path.clone()));
                     }
                 }
                 Err(e) => {
@@ -1505,13 +1536,13 @@ fn process_pending_grabs(
                 if quality_flagged {
                     stats.quality_flagged += 1;
                 }
-                imported_hashes.push(grab.torrent_hash().to_string());
+                imported_hashes.push((grab.torrent_hash().to_string(), content_path.clone()));
             }
             Ok(ImportOutcome::SkippedAlreadyHaveBetter) => {
                 // The download's data is already handled — `import_one`
                 // deleted the losing file itself — so there's nothing left
                 // for qBittorrent to track either.
-                imported_hashes.push(grab.torrent_hash().to_string());
+                imported_hashes.push((grab.torrent_hash().to_string(), content_path.clone()));
             }
             Err(e) => {
                 let error_count = record_import_error(conn, grab.release_id())?;
@@ -3225,6 +3256,57 @@ mod tests {
         std::fs::remove_dir_all(&dir).unwrap();
     }
 
+    #[test]
+    fn cleanup_leftover_download_dir_removes_a_folder_with_only_sidecars_left() {
+        let downloads =
+            std::env::temp_dir().join(format!("breadarr-cleanup-test-{}", std::process::id()));
+        let release_dir = downloads.join("Some Movie 2016 1080p");
+        std::fs::create_dir_all(&release_dir).unwrap();
+        // The video was already moved out by `move_or_copy_file`; only the
+        // sidecar junk qBittorrent downloaded alongside it remains.
+        std::fs::write(release_dir.join("poster.jpg"), b"jpeg bytes").unwrap();
+        std::fs::write(release_dir.join("release.nfo"), b"nfo text").unwrap();
+
+        cleanup_leftover_download_dir(&release_dir, &downloads.to_string_lossy()).unwrap();
+
+        assert!(!release_dir.exists(), "the now-empty-of-video release folder should be gone");
+        assert!(downloads.exists(), "the shared downloads root itself must survive");
+
+        std::fs::remove_dir_all(&downloads).unwrap();
+    }
+
+    #[test]
+    fn cleanup_leftover_download_dir_is_a_noop_for_a_bare_file() {
+        let downloads =
+            std::env::temp_dir().join(format!("breadarr-cleanup-file-test-{}", std::process::id()));
+        std::fs::create_dir_all(&downloads).unwrap();
+        // A release with no wrapping folder — `move_or_copy_file` already
+        // consumed it, so `content_path` no longer exists at all.
+        let bare = downloads.join("Some.Movie.2016.1080p.mp4");
+
+        cleanup_leftover_download_dir(&bare, &downloads.to_string_lossy()).unwrap();
+
+        std::fs::remove_dir_all(&downloads).unwrap();
+    }
+
+    #[test]
+    fn cleanup_leftover_download_dir_refuses_to_remove_the_downloads_root_itself() {
+        let downloads = std::env::temp_dir()
+            .join(format!("breadarr-cleanup-guard-test-{}", std::process::id()));
+        std::fs::create_dir_all(&downloads).unwrap();
+        std::fs::write(downloads.join("unrelated-other-download.mkv"), b"data").unwrap();
+
+        // A malformed `content_path` that happens to equal the downloads
+        // root itself must never be wiped, even though it passes the
+        // `is_dir()` check.
+        cleanup_leftover_download_dir(&downloads, &downloads.to_string_lossy()).unwrap();
+
+        assert!(downloads.exists());
+        assert!(downloads.join("unrelated-other-download.mkv").exists());
+
+        std::fs::remove_dir_all(&downloads).unwrap();
+    }
+
     #[test]
     fn locates_a_single_file_torrent() {
         let dir = std::env::temp_dir().join(format!("breadarr-test-{}", std::process::id()));

breadarrd/src/qbit/mod.rs

@@ -40,6 +40,40 @@ struct AddTorrentResponse {
     success_count: u32,
     #[serde(default)]
     failure_count: u32,
+    /// Present when adding by URL rather than by a magnet/`.torrent` blob
+    /// qBittorrent already has in hand — the torrent itself is fetched
+    /// asynchronously, so the immediate response has neither succeeded nor
+    /// failed yet, just queued. nyaa's RSS feed always hands over a
+    /// `.torrent` download URL (never a magnet), so this is the normal
+    /// shape for every anime grab, not an edge case. Verified live against
+    /// the deployed qBittorrent: a URL add returns HTTP 202 with
+    /// `{"pending_count":1,"success_count":0,"failure_count":0}` — treating
+    /// `success_count == 0` alone as rejection (the previous check) silently
+    /// dropped every one of these while the torrent downloaded successfully
+    /// in the background, with no DB record and no log line.
+    #[serde(default)]
+    pending_count: u32,
+}
+
+/// Interprets a `torrents/add` response body — split out from `add_magnet`
+/// so it's directly testable without a live qBittorrent server.
+/// qBittorrent's add-torrent endpoint returns HTTP 200/202 even when it
+/// rejects the magnet outright (a dead/malformed hash, one it already knows
+/// is unreachable) — the response body is the only signal. Older
+/// qBittorrent versions returned plain text ("Ok." vs "Fails."); newer ones
+/// return a JSON summary with success_count/failure_count/pending_count
+/// instead (verified live against the currently deployed version). Without
+/// checking whichever shape is actually in play, a rejected magnet looks
+/// identical to a real success: the caller records a `release` row as
+/// grabbed and nothing ever downloads, silently and permanently (verified
+/// live — this happened for a real release under the old text-only check,
+/// and separately for every nyaa URL-add under a since-fixed
+/// `success_count == 0` check that didn't account for `pending_count`).
+fn add_torrent_response_is_rejected(body: &str) -> bool {
+    match serde_json::from_str::<AddTorrentResponse>(body) {
+        Ok(r) => r.failure_count > 0 || (r.success_count == 0 && r.pending_count == 0),
+        Err(_) => body.trim() != "Ok.",
+    }
 }
 
 pub struct QbitClient {
@@ -157,23 +191,7 @@ impl QbitClient {
             if !status.is_success() {
                 bail!("qbit add-torrent failed: status={status} body={body:?}");
             }
-            // qBittorrent's add-torrent endpoint returns HTTP 200 even when
-            // it rejects the magnet outright (a dead/malformed hash, one it
-            // already knows is unreachable) — the response body is the only
-            // signal. Older qBittorrent versions returned plain text ("Ok."
-            // vs "Fails."); newer ones return a JSON summary with
-            // success_count/failure_count instead (verified live against
-            // the currently deployed version). Without checking whichever
-            // shape is actually in play, a rejected magnet looks identical
-            // to a real success: the caller records a `release` row as
-            // grabbed and nothing ever downloads, silently and permanently
-            // (verified live — this happened for a real release under the
-            // old text-only check).
-            let rejected = match serde_json::from_str::<AddTorrentResponse>(&body) {
-                Ok(r) => r.failure_count > 0 || r.success_count == 0,
-                Err(_) => body.trim() != "Ok.",
-            };
-            if rejected {
+            if add_torrent_response_is_rejected(&body) {
                 return Err(anyhow::Error::new(MagnetRejected { body }));
             }
             return Ok(());
@@ -292,4 +310,38 @@ mod tests {
             None
         );
     }
+
+    #[test]
+    fn a_pending_url_add_is_not_treated_as_rejected() {
+        // Exact shape qBittorrent 5.x returns for a URL add (every nyaa
+        // grab) — the torrent is queued for an async fetch, not yet
+        // succeeded or failed. This is the shape that used to be
+        // misread as an outright rejection.
+        let body = r#"{"added_torrent_ids":[],"failure_count":0,"pending_count":1,"success_count":0}"#;
+        assert!(!add_torrent_response_is_rejected(body));
+    }
+
+    #[test]
+    fn a_genuine_failure_is_still_rejected() {
+        let body = r#"{"failure_count":1,"pending_count":0,"success_count":0}"#;
+        assert!(add_torrent_response_is_rejected(body));
+    }
+
+    #[test]
+    fn an_in_band_success_is_not_rejected() {
+        // TPB/1337x magnets: qBittorrent already has the info hash, no
+        // async fetch needed, so success_count is set immediately.
+        let body = r#"{"failure_count":0,"pending_count":0,"success_count":1}"#;
+        assert!(!add_torrent_response_is_rejected(body));
+    }
+
+    #[test]
+    fn the_old_plain_text_ok_response_is_not_rejected() {
+        assert!(!add_torrent_response_is_rejected("Ok."));
+    }
+
+    #[test]
+    fn the_old_plain_text_fails_response_is_rejected() {
+        assert!(add_torrent_response_is_rejected("Fails."));
+    }
 }

breadarrd/src/scheduler.rs

@@ -613,6 +613,33 @@ async fn process_item(
     let torrent_hash = match grab_and_capture_hash(qbit, &item.link, qbit_category).await {
         Ok(hash) => hash,
         Err(e) if e.downcast_ref::<crate::qbit::MagnetRejected>().is_some() => {
+            // Same reasoning as `HashCaptureFailed` just below: recording
+            // this as `failed` rather than leaving it with no release row
+            // at all is what makes a genuine rejection visible and frees
+            // the episode/movie to be re-searched later with a different
+            // candidate. A silent `Ok(MagnetRejected)` with nothing written
+            // used to be indistinguishable from a real success once
+            // `mark_seen` ran — a since-fixed bug in the response-rejection
+            // check (see `qbit::add_torrent_response_is_rejected`'s doc
+            // comment) made every nyaa URL-add hit this path even though
+            // the torrent was actually downloading, so this arm went
+            // unnoticed for a long time; logging it now that it only fires
+            // on real rejections.
+            tracing::warn!(title = %item.title, guid = %item.guid, "qbittorrent rejected this release's magnet/torrent");
+            record_grab(
+                conn,
+                media_item.id,
+                episode_id,
+                season_pack_number,
+                source_id,
+                &item.title,
+                &item.guid,
+                release_score,
+                item.size_bytes,
+                qbit_category,
+                None,
+                "failed",
+            )?;
             return Ok(ProcessOutcome::MagnetRejected);
         }
         Err(e) => return Err(e),
@@ -761,6 +788,7 @@ pub struct GrabCycleStats {
     pub grabbed: usize,
     pub errors: usize,
     pub queued_for_review: usize,
+    pub magnet_rejected: usize,
 }
 
 pub async fn run_grab_cycle(
@@ -811,6 +839,7 @@ pub async fn run_grab_cycle(
                 match outcome {
                     ProcessOutcome::Grabbed { .. } => stats.grabbed += 1,
                     ProcessOutcome::QueuedForReview => stats.queued_for_review += 1,
+                    ProcessOutcome::MagnetRejected => stats.magnet_rejected += 1,
                     _ => {}
                 }
             }