From c12d4c9f4bf311d5552fdcb1a96003fefb6e4db6 Mon Sep 17 00:00:00 2001 From: tech nasty <262050521+t3chnaztea@users.noreply.github.com> Date: Sun, 12 Jul 2026 21:48:53 -0500 Subject: [PATCH] fix(downloads): orphan cleanup skipped live empty-queue snapshots, stranding deleted downloads and blocking re-grabs with 409 Deleting a torrent directly in the client when it was the only active download left its Download record stranded forever, and every re-grab returned HTTP 409 "already active". RemoveOrphansAsync bailed whenever a snapshot returned 0 items while downloads were tracked, on the theory the client might be unreachable, but that condition is already fully excluded upstream by the UsedCachedSnapshot and IsUnavailable guards: the poller surfaces every timeout/cancel/error as a cached or unavailable snapshot, never as a live empty one, so a live empty queue genuinely means the items are gone. Removing the redundant guard lets cleanup proceed on live empty snapshots while preserving the #640 protections unchanged: unreachable clients are still skipped, ImportPending/Ready/ImportBlocked are never terminalized (only Queued/Downloading/Paused are considered), and the per-item 5-minute grace period still shields fresh adds. Co-Authored-By: Claude Fable 5 --- .../Queue/DownloadOrphanCleanupService.cs | 16 ++-- .../DownloadOrphanCleanupServiceTests.cs | 92 ++++++++++++++++++- 2 files changed, 98 insertions(+), 10 deletions(-) diff --git a/listenarr.application/Downloads/Queue/DownloadOrphanCleanupService.cs b/listenarr.application/Downloads/Queue/DownloadOrphanCleanupService.cs index b08662039..a95c088fc 100644 --- a/listenarr.application/Downloads/Queue/DownloadOrphanCleanupService.cs +++ b/listenarr.application/Downloads/Queue/DownloadOrphanCleanupService.cs @@ -48,16 +48,14 @@ public async Task RemoveOrphansAsync( return; } + // A snapshot that reaches this point is guaranteed live: the UsedCachedSnapshot + // and IsUnavailable guards above already excluded every unreachable path (the + // poller surfaces timeout/cancel/error as cached or unavailable, never as a live + // empty snapshot). An empty live queue therefore means the items really are gone, + // so an empty snapshot must be allowed to terminalize orphans - otherwise deleting + // your only active torrent strands its Download record forever and blocks re-grabs + // with 409. The per-item grace period below still protects fresh adds. var clientQueue = clientQueueResult.QueueItems; - if (clientQueue.Count == 0 && clientDownloads.Any()) - { - logger.LogWarning( - "Skipping orphan cleanup for client {ClientName}: client returned 0 queue items but {Count} downloads are tracked. Client may be temporarily unreachable.", - client.Name, - clientDownloads.Count); - return; - } - var liveClientItemIds = BuildLiveClientItemIds(clientQueue, mappedQueueItems); var now = DateTime.UtcNow; var cleanupCandidates = clientDownloads diff --git a/tests/Features/Application/Downloads/Queue/DownloadOrphanCleanupServiceTests.cs b/tests/Features/Application/Downloads/Queue/DownloadOrphanCleanupServiceTests.cs index e3ba31431..cd294d0ed 100644 --- a/tests/Features/Application/Downloads/Queue/DownloadOrphanCleanupServiceTests.cs +++ b/tests/Features/Application/Downloads/Queue/DownloadOrphanCleanupServiceTests.cs @@ -86,7 +86,7 @@ await service.RemoveOrphansAsync( [Fact] [Trait("Method", "RemoveOrphansAsync")] - public async Task RemoveOrphansAsync_DoesNotRemoveIdlessDownloadWhenLiveSnapshotIsEmpty() + public async Task RemoveOrphansAsync_RemovesIdlessDownloadWhenLiveSnapshotIsEmpty() { var client = CreateClient(); var download = await AddDownloadAsync( @@ -102,6 +102,96 @@ await service.RemoveOrphansAsync( [], [download]); + Assert.Null(await _downloadRepository.GetByIdAsync(download.Id)); + _metricsMock.Verify(m => m.Increment("download.orphan.unlinked_removed", 1), Times.Once); + _metricsMock.Verify(m => m.Increment("download.orphan.removed", It.IsAny()), Times.Never); + } + + [Fact] + [Trait("Method", "RemoveOrphansAsync")] + public async Task RemoveOrphansAsync_RemovesOnlyTrackedDownloadWhenLiveSnapshotIsEmpty() + { + var client = CreateClient(); + var download = await AddDownloadAsync( + id: "sole-active-orphan", + clientId: client.Id, + status: DownloadStatus.Downloading, + startedAt: DateTime.UtcNow.AddMinutes(-10), + metadata: new Dictionary + { + ["ClientDownloadId"] = "deleted-in-client" + }); + var service = _provider.GetRequiredService(); + + await service.RemoveOrphansAsync( + client, + CreateLiveSnapshot(client, []), + [], + [download]); + + Assert.Null(await _downloadRepository.GetByIdAsync(download.Id)); + _metricsMock.Verify(m => m.Increment("download.orphan.removed", 1), Times.Once); + _metricsMock.Verify(m => m.Increment("download.orphan.unlinked_removed", It.IsAny()), Times.Never); + } + + [Fact] + [Trait("Method", "RemoveOrphansAsync")] + public async Task RemoveOrphansAsync_DoesNotRemoveRecentDownloadWhenLiveSnapshotIsEmpty() + { + var client = CreateClient(); + var download = await AddDownloadAsync( + id: "recent-empty-snapshot-download", + clientId: client.Id, + status: DownloadStatus.Downloading, + startedAt: DateTime.UtcNow.AddMinutes(-1), + metadata: new Dictionary + { + ["ClientDownloadId"] = "deleted-in-client" + }); + var service = _provider.GetRequiredService(); + + await service.RemoveOrphansAsync( + client, + CreateLiveSnapshot(client, []), + [], + [download]); + + Assert.NotNull(await _downloadRepository.GetByIdAsync(download.Id)); + _metricsMock.Verify(m => m.Increment(It.IsAny(), It.IsAny()), Times.Never); + } + + [Theory] + [Trait("Method", "RemoveOrphansAsync")] + [InlineData(true, false)] + [InlineData(false, true)] + public async Task RemoveOrphansAsync_DoesNotRemoveWhenEmptySnapshotIsNotTrusted(bool usedCachedSnapshot, bool isUnavailable) + { + var client = CreateClient(); + var download = await AddDownloadAsync( + id: "untrusted-empty-snapshot-download", + clientId: client.Id, + status: DownloadStatus.Downloading, + startedAt: DateTime.UtcNow.AddMinutes(-10), + metadata: new Dictionary + { + ["ClientDownloadId"] = "deleted-in-client" + }); + var service = _provider.GetRequiredService(); + + await service.RemoveOrphansAsync( + client, + new ClientQueueFetchResult( + client, + [], + usedCachedSnapshot, + isUnavailable, + snapshotAge: null, + failureReason: isUnavailable ? "unavailable" : null, + snapshotState: isUnavailable ? "unavailable" : "cached", + snapshotRefreshedAtUtc: DateTimeOffset.UtcNow), + [], + [download]); + Assert.NotNull(await _downloadRepository.GetByIdAsync(download.Id)); _metricsMock.Verify(m => m.Increment(It.IsAny(), It.IsAny()), Times.Never); }