fix: unbreak main and follow up on the Blackhole handoff review (#1346)

DownloadHistoryService.record_download and updated the single production
caller, but not the eleven in the test suite, leaving main red with 32
failures. Pass None, which is what the pre-#1336 behaviour recorded.

For the Blackhole handoff (#1345): add_download publishes the torrent
before the cancel check runs, and BlackholeClient.remove() is a no-op,
so
the watcher picks the file up regardless. Reporting a bare "Cancelled"
hid
that from the user. Name the completed handoff in the cancellation
message
instead, drop the _handle_cancelled_download call whose usenet branch
cannot apply to a handoff-only client, and record why the orchestrator
no
longer verifies HandoffResult.path.

Finally, make tests/direct_download a package: test_libgen_extract.py
imports tests.libgen.sample_html across test directories, so without an
__init__.py pytest named its modules by bare basename and a same-named
module elsewhere would collide.

shelfmark/download/clients/base_handler.py

@@ -873,11 +873,25 @@ class ExternalClientHandler(DownloadHandler, ABC):
                 )
                 if getattr(client, "handoff_only", False) is True:
                     if cancel_flag.is_set():
-                        self._handle_cancelled_download(
-                            client, download_id, request.protocol, status_callback
+                        # The file is published and unpublishing it would race a watcher
+                        # that may already have consumed it, so the handoff stands even
+                        # though the task is cancelled. Say so rather than leaving a bare
+                        # "Cancelled" the user cannot act on.
+                        logger.info(
+                            "Cancelled after handoff to %s; leaving publication in place: %s",
+                            client.name,
+                            download_id,
+                        )
+                        status_callback(
+                            "cancelled",
+                            f"Cancelled, but the torrent was already handed off to {client.name}",
                         )
                         return None
-                    # A watcher can consume the publication as soon as add_download returns.
+                    # A watcher can consume the publication as soon as add_download returns,
+                    # so the handoff completes here rather than in the poll loop. The
+                    # orchestrator deliberately does not check that this path still exists:
+                    # a consumed publication is indistinguishable from a bogus one, and
+                    # treating it as an error is the failure this avoids (#1345).
                     progress_callback(100)
                     self._on_download_complete(task)
                     return HandoffResult(

shelfmark/download/orchestrator.py

@@ -320,7 +320,14 @@ def queue_release(
             logger.info("Release already in queue: %s", task.title)
             return False, "Release is already in the download queue"
 
-        logger.info("Release queued with priority %s: %s (downloads=%s, release_data.downloads=%s, extra=%s)", priority, task.title, task.downloads, release_data.get("downloads"), extra)
+        logger.info(
+            "Release queued with priority %s: %s (downloads=%s, release_data.downloads=%s, extra=%s)",
+            priority,
+            task.title,
+            task.downloads,
+            release_data.get("downloads"),
+            extra,
+        )
 
         # Broadcast status update via WebSocket
         if ws_manager:

shelfmark/main.py

@@ -1073,7 +1073,12 @@ def api_download_release() -> Response | tuple[Response, int]:
             release_payload = dict(data)
             release_payload["content_type"] = resolved_content_type
 
-        logger.info("Download request received. keys=%s downloads=%s extra.downloads=%s", list(data.keys()), data.get("downloads"), data.get("extra", {}).get("downloads") if isinstance(data.get("extra"), dict) else None)
+        logger.info(
+            "Download request received. keys=%s downloads=%s extra.downloads=%s",
+            list(data.keys()),
+            data.get("downloads"),
+            data.get("extra", {}).get("downloads") if isinstance(data.get("extra"), dict) else None,
+        )
 
         priority = data.get("priority", 0)
         # Per-user download overrides

shelfmark/release_sources/direct_download/annas_archive.py

@@ -821,9 +821,7 @@ def _enrich_search_results_with_downloads(books: list[BrowseRecord]) -> None:
     # Fetch counts in parallel using the inline_info API (cheaper than summary)
     counts: dict[str, int] = {}
     with concurrent.futures.ThreadPoolExecutor(max_workers=5) as executor:
-        futures = {
-            executor.submit(_fetch_download_count_inline, bid): bid for bid in book_ids
-        }
+        futures = {executor.submit(_fetch_download_count_inline, bid): bid for bid in book_ids}
         for future in concurrent.futures.as_completed(futures):
             bid = futures[future]
             try:

shelfmark/release_sources/direct_download/source.py

@@ -1,5 +1,6 @@
 """Direct Download search and release-source integration."""
 
+import contextlib
 from pathlib import Path
 from typing import TYPE_CHECKING, ClassVar
 
@@ -43,10 +44,8 @@ def _extract_downloads(record: BrowseRecord) -> int | None:
     if record.info and "Downloads" in record.info:
         downloads_value = record.info["Downloads"]
         if isinstance(downloads_value, list) and len(downloads_value) > 0:
-            try:
+            with contextlib.suppress(ValueError, TypeError):
                 downloads = int(downloads_value[0])
-            except (ValueError, TypeError):
-                pass
         elif isinstance(downloads_value, (int, float)):
             downloads = int(downloads_value)
     return downloads

src/frontend/src/components/activity/activityMappers.ts

@@ -89,7 +89,8 @@ export const downloadToActivityItem = (book: Book, statusKey: DownloadStatusKey)
       ? Math.trunc(book.request_id)
       : undefined;
   const downloadsCount = getDownloadsCount(book);
-  const downloadsText = downloadsCount != null ? `${downloadsCount.toLocaleString()} downloads` : undefined;
+  const downloadsText =
+    downloadsCount != null ? `${downloadsCount.toLocaleString()} downloads` : undefined;
   const metaLine = joinMetaParts([
     toOptionalText(book.format)?.toUpperCase(),
     toOptionalText(book.size),

src/frontend/src/components/resultsViews/CardView.tsx

@@ -211,7 +211,16 @@ export const CardView = ({
                   <span>{book.size}</span>
                 </>
               )}
-              {searchMode !== 'universal' && (() => { const d = getDownloadsCount(book); return d != null && d > 0 ? <> <span>•</span> <span>{d.toLocaleString()}</span> </> : null; })()}
+              {searchMode !== 'universal' &&
+                (() => {
+                  const d = getDownloadsCount(book);
+                  return d != null && d > 0 ? (
+                    <>
+                      {' '}
+                      <span>•</span> <span>{d.toLocaleString()}</span>{' '}
+                    </>
+                  ) : null;
+                })()}
             </div>
           )}
         </div>

src/frontend/src/components/resultsViews/CompactView.tsx

@@ -222,7 +222,15 @@ export const CompactView = ({
                   <span>{book.size}</span>
                 </>
               )}
-              {(() => { const d = getDownloadsCount(book); return d != null && d > 0 ? <> <span>•</span> <span>{d.toLocaleString()}</span> </> : null; })()}
+              {(() => {
+                const d = getDownloadsCount(book);
+                return d != null && d > 0 ? (
+                  <>
+                    {' '}
+                    <span>•</span> <span>{d.toLocaleString()}</span>{' '}
+                  </>
+                ) : null;
+              })()}
             </div>
           )}
 

src/frontend/src/components/resultsViews/ListView.tsx

@@ -291,7 +291,10 @@ export const ListView = ({
                 {/* Direct mode: Downloads - Desktop only */}
                 {searchMode !== 'universal' && (
                   <div className="hidden justify-center text-xs text-gray-700 sm:flex dark:text-gray-200">
-                    {(() => { const d = getDownloadsCount(book); return d != null && d > 0 ? d.toLocaleString() : '-'; })()}
+                    {(() => {
+                      const d = getDownloadsCount(book);
+                      return d != null && d > 0 ? d.toLocaleString() : '-';
+                    })()}
                   </div>
                 )}
 

src/frontend/src/utils/releasePayload.ts

@@ -8,6 +8,23 @@ export interface ReleaseDownloadOptions {
   bookPlan?: PackBook[];
 }
 
+/** Download count for a release, preferring what the release itself reports. */
+const releaseDownloads = (book: Book, release: Release): number => {
+  if (typeof release.extra?.downloads === 'number') {
+    return release.extra.downloads;
+  }
+  if (typeof book.downloads === 'number' && book.downloads > 0) {
+    return book.downloads;
+  }
+  if (typeof book.extra?.downloads === 'number') {
+    return book.extra.downloads;
+  }
+  if (Array.isArray(book.info?.Downloads) && book.info.Downloads.length > 0) {
+    return Number(book.info.Downloads[0]) || 0;
+  }
+  return 0;
+};
+
 /** Build the body for /api/releases/download (and /api/releases/inspect). */
 export function buildReleaseDownloadPayload(
   book: Book,
@@ -30,16 +47,7 @@ export function buildReleaseDownloadPayload(
     format: release.format,
     size: release.size,
     size_bytes: release.size_bytes,
-    downloads:
-      typeof release.extra?.downloads === 'number'
-        ? release.extra.downloads
-        : typeof book.downloads === 'number' && book.downloads > 0
-          ? book.downloads
-          : typeof book.extra?.downloads === 'number'
-            ? book.extra.downloads
-            : Array.isArray(book.info?.Downloads) && book.info.Downloads.length > 0
-              ? Number(book.info.Downloads[0]) || 0
-              : 0,
+    downloads: releaseDownloads(book, release),
     download_url: release.download_url,
     protocol: release.protocol,
     indexer: release.indexer,

src/frontend/src/utils/requestPayload.ts

@@ -95,30 +95,36 @@ export const buildReleaseDataFromMetadataRelease = (
   };
 };
 
- export const buildReleaseDataFromDirectBook = (book: Book) => {
-   const source = getBrowseSource(book);
-   const downloads =
-     typeof book.downloads === 'number' && book.downloads > 0
-       ? book.downloads
-       : Array.isArray(book.info?.Downloads) && book.info.Downloads.length > 0
-         ? Number(book.info.Downloads[0]) || 0
-         : 0;
-   return {
-     source,
-     source_id: book.id,
-     title: book.title || 'Unknown title',
-     author: book.author,
-     year: book.year,
-     format: book.format,
-     size: book.size,
-     downloads,
-     preview: book.preview,
-     content_type: 'ebook' as const,
-     // Browsing a source directly means the book record IS the release record.
-     language: book.language,
-     search_mode: 'direct' as const,
-   };
- };
+/** Download count for a book queued straight from a browse result. */
+const directBookDownloads = (book: Book): number => {
+  if (typeof book.downloads === 'number' && book.downloads > 0) {
+    return book.downloads;
+  }
+  if (Array.isArray(book.info?.Downloads) && book.info.Downloads.length > 0) {
+    return Number(book.info.Downloads[0]) || 0;
+  }
+  return 0;
+};
+
+export const buildReleaseDataFromDirectBook = (book: Book) => {
+  const source = getBrowseSource(book);
+  const downloads = directBookDownloads(book);
+  return {
+    source,
+    source_id: book.id,
+    title: book.title || 'Unknown title',
+    author: book.author,
+    year: book.year,
+    format: book.format,
+    size: book.size,
+    downloads,
+    preview: book.preview,
+    content_type: 'ebook' as const,
+    // Browsing a source directly means the book record IS the release record.
+    language: book.language,
+    search_mode: 'direct' as const,
+  };
+};
 
 export const buildDirectRequestPayload = (book: Book): CreateRequestPayload => {
   const bookData = buildDirectBookRequestData(book);

tests/core/test_activity_routes_api.py

@@ -71,6 +71,7 @@ def _record_terminal_download(
         size="1 MB",
         preview=None,
         content_type="ebook",
+        downloads=None,
         origin=origin,
     )
     svc.finalize_download(
@@ -414,6 +415,7 @@ class TestActivityRoutes:
             size="1 MB",
             preview=None,
             content_type="ebook",
+            downloads=None,
             origin="direct",
         )
         active_status = _sample_status_payload()
@@ -597,6 +599,7 @@ class TestActivityRoutes:
             size="1 MB",
             preview=None,
             content_type="ebook",
+            downloads=None,
             origin="direct",
         )
 
@@ -652,6 +655,7 @@ class TestActivityRoutes:
             size="1 MB",
             preview=None,
             content_type="ebook",
+            downloads=None,
             origin="requested",
             retry_payload=retry_payload,
         )
@@ -1005,6 +1009,7 @@ class TestActivityRoutes:
             size="1 MB",
             preview=None,
             content_type="ebook",
+            downloads=None,
             origin="direct",
         )
 
@@ -1053,6 +1058,7 @@ class TestActivityRoutes:
             size="1 MB",
             preview=None,
             content_type="ebook",
+            downloads=None,
             origin="requested",
             retry_payload=retry_payload,
         )
@@ -1152,6 +1158,7 @@ class TestActivityRoutes:
             size="1 MB",
             preview=None,
             content_type="ebook",
+            downloads=None,
             origin="requested",
             retry_payload=retry_payload,
         )
@@ -1197,6 +1204,7 @@ class TestActivityRoutes:
             size="2 MB",
             preview=None,
             content_type="ebook",
+            downloads=None,
             origin="direct",
         )
 

tests/core/test_download_api_guardrails.py

@@ -538,6 +538,7 @@ class TestRetryDownloadEndpointGuardrails:
             size="1 MB",
             preview=None,
             content_type="ebook",
+            downloads=None,
             origin="direct",
             retry_payload=retry_payload,
         )
@@ -733,6 +734,7 @@ class TestRetryDownloadEndpointGuardrails:
             size="1 MB",
             preview=None,
             content_type="ebook",
+            downloads=None,
             origin="requested",
             retry_payload=retry_payload,
         )

tests/core/test_download_history_service.py

@@ -34,6 +34,7 @@ def test_record_download_stores_utc_iso_timestamps():
             size=None,
             preview=None,
             content_type="ebook",
+            downloads=None,
             origin="direct",
         )
 

tests/direct_download/__init__.py

@@ -0,0 +1 @@
+"""Tests for the Direct Download source."""

tests/download/test_orchestrator_lifecycle.py

@@ -2,7 +2,7 @@ from __future__ import annotations
 
 from pathlib import Path
 from threading import Event
-from unittest.mock import ANY, MagicMock
+from unittest.mock import ANY, MagicMock, call
 
 import pytest
 
@@ -298,3 +298,10 @@ def test_download_task_completes_blackhole_handoff_without_post_processing(
         queue.update_progress.assert_called_once_with(task.task_id, 100)
     else:
         queue.update_progress.assert_not_called()
+    if handoff == "cancelled":
+        # The publication is already out of our hands, so a bare "Cancelled" would
+        # misdescribe what the watcher is about to do with it.
+        assert (
+            call(task.task_id, "Cancelled, but the torrent was already handed off to blackhole")
+            in queue.update_status_message.call_args_list
+        )