Skip to content

Fix SABnzbd downloads completing with no path set (race between active queue and history) - #840

Open
lisim wants to merge 1 commit into
Listenarrs:canaryfrom
lisim:839-sabnzbd-premature-completion
Open

Fix SABnzbd downloads completing with no path set (race between active queue and history)#840
lisim wants to merge 1 commit into
Listenarrs:canaryfrom
lisim:839-sabnzbd-premature-completion

Conversation

@lisim

@lisim lisim commented Aug 20, 2026

Copy link
Copy Markdown

Summary

Fixes #839.

SABnzbd downloads occasionally get marked Completed with an empty DownloadPath, which permanently blocks import: DownloadProcessingJobProcessor.ProcessJobAsync throws Inconsistency: Download {id} has no path set and nothing ever retries with a corrected path afterward — the download just sits ImportBlocked forever. Hit this repeatedly (6 different releases) across a single session of grabbing several audiobooks in quick succession.

Root cause: SABnzbd briefly reports an item as status: "Completed" in the active queue (mode=queue) before archiving it to history (mode=history) with the real storage path. SabnzbdResponseMapper.MapQueueSlotToQueueItem's own existing comment already acknowledges this ("Active SABnzbd queue slots do not reliably expose the completed storage path") but the method still mapped that slot into a QueueItem with Status = "completed" regardless. QueueItemConverter.UpdateFromQueueItem then sees normalizedState == "completed" and calls download.Completed() immediately, before a path was ever set (download.DownloadPath is only assigned if (!string.IsNullOrEmpty(item.LocalPath))). Once Status == Completed, nothing backfills the path — the download is stuck.

Changes

Fixed

  • SabnzbdResponseMapper.MapQueueSlotToQueueItem: when the mapped status is "completed" and there's no explicit storage path on the slot, return null instead of a pathless completed QueueItem.

This isn't a dead end for that download — excluding it from the active-queue result set makes SabnzbdQueueFetchWorkflow.GetMissingTrackedIds treat it as missing from the queue, which sets historyRequired = true and triggers a history lookup in the same poll cycle. History reliably has the real storage path by the time an item shows up there, so the download resolves correctly via MapHistorySlotToQueueItem (the code path that already works), instead of completing early on unreliable telemetry.

Added

  • SabnzbdResponseMapperTests.cs — three focused unit tests directly against the static mapper (no DI/database/filesystem needed):
    • MapQueueSlotToQueueItem_CompletedStatusWithoutStorage_ReturnsNull — the regression case.
    • MapQueueSlotToQueueItem_CompletedStatusWithStorage_ReturnsCompletedItem — a real storage path still resolves normally, unaffected.
    • MapQueueSlotToQueueItem_DownloadingStatusWithoutStorage_StillReturnsItem — the guard only targets "completed"; genuinely in-progress items (which never carry storage) are unaffected.

Testing

  • dotnet test tests/Listenarr.Tests.csproj --filter 'FullyQualifiedName~SabnzbdResponseMapperTests' — 3/3 pass on this branch.
  • Confirmed the regression test actually catches the bug: ran the identical test file against unmodified canary HEAD — MapQueueSlotToQueueItem_CompletedStatusWithoutStorage_ReturnsNull fails there (Assert.Null() Failure: Value is not null), the other two pass on both. So this isn't a test that would have passed regardless of the fix.
  • dotnet build listenarr.slnx — clean, 0 warnings, 0 errors.
  • dotnet format listenarr.slnx --no-restore --verify-no-changes --include <the 2 changed files> — clean, no formatting changes needed.

These tests are plain static-method unit tests (no BaseTests/DI/EF/filesystem semantics involved), so they run cleanly in a sandboxed container without hitting the IFileSystemSemanticsResolver environment limitation I ran into on my previous PR (#837).

Review coverage (per .github/AGENTS.md)

  • Composition/DI: N/A — no service registrations, constructors, or lifetimes touched.
  • Persistence/migrations: N/A — no EF/schema changes.
  • Concurrency/cancellation: this change fixes a concurrency/timing issue (a race between two SABnzbd endpoints reporting inconsistent state for the same item); no new async paths introduced.
  • Filesystem/security boundaries: N/A.
  • Serialization/identity: N/A — pure JSON-slot-to-QueueItem mapping, same shape as existing code.
  • Recovery/restart: improves it — previously-stuck downloads following this exact pattern should now resolve on their own via the next poll cycle rather than needing manual deletion (I was manually deleting these before diagnosing the root cause).
  • Frontend/backend contracts: N/A — backend-only, no API surface change.
  • Platform behavior: N/A — no OS-specific code involved.
  • Tests: added (see above), and verified they actually distinguish fixed vs. unfixed behavior.

Related

I also opened #838 (missing blocklist/blocklist feature) from the same session — unrelated root cause, separate issue, not addressed by this PR.

SABnzbd briefly reports an item as status "Completed" in its active
queue before archiving it to history with the real storage path.
MapQueueSlotToQueueItem mapped that slot into a completed QueueItem
regardless, which let QueueItemConverter mark the download Completed
with an empty DownloadPath - permanently blocking import with
"Inconsistency: Download {id} has no path set", since nothing ever
backfills the path afterward.

When status is "completed" but no storage path is present yet, return
null instead of a pathless completed item. Excluding it from the
active-queue result set makes the poller treat the download as
missing, which triggers a same-cycle history lookup - and history
reliably has the storage path by then. This resolves the download via
the code path that already works correctly, rather than completing
early on unreliable telemetry.

Fixes Listenarrs#839.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
@lisim
lisim requested a review from a team August 20, 2026 09:52
@lisim

lisim commented Aug 20, 2026

Copy link
Copy Markdown
Author

Heads up for review: this touches the same method as #759 (SabnzbdResponseMapper.MapQueueSlotToQueueItem), though in a different part of it — #759 changes the category-filter check near the top, this PR adds a completion guard further down after the storage-path lookup. Different bugs, complementary fixes, but whichever merges second will likely need a small manual resolve on that file (and on SabnzbdAdapterTests.cs/SabnzbdApiMock.cs, where both PRs append new tests in different spots).

No functional overlap that I can see — #759 is about tracked jobs getting hidden by category mismatch, this PR is about the active-queue/history completion race — but wanted to flag the proximity since I noticed it late (after already opening this PR, not before).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: SABnzbd downloads can be marked Completed with no path set, permanently blocking import

1 participant