From c93394bad852b707cec098735b611dd7e97fffc8 Mon Sep 17 00:00:00 2001 From: chodeus Date: Fri, 21 Aug 2026 05:53:02 +0800 Subject: [PATCH 1/2] fix: never cancel or remove transfers this item did not enqueue - The watchdog skips file states the item doesn't own; a shared peer directory can put another item's transfers in FileStates, and a stuck foreign transfer could trip a bailout that cancels a live download - RemoveItemFilesAsync filters to owned transfers before cancelling, removing, or deleting local basenames --- .../Clients/Soulseek/SlskdDownloadManager.cs | 7 +- .../Clients/Soulseek/SlskdWatchdog.cs | 5 + tests/Sleezer.Tests/Sleezer.Tests.csproj | 8 ++ tests/Sleezer.Tests/SlskdWatchdogTests.cs | 104 ++++++++++++++++++ 4 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 tests/Sleezer.Tests/SlskdWatchdogTests.cs diff --git a/src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs b/src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs index bfb79cb..6b3c50d 100644 --- a/src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs +++ b/src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs @@ -1426,7 +1426,12 @@ private async Task CleanupSupersededAttemptsAsync(List stale, private async Task RemoveItemFilesAsync(SlskdDownloadItem item, SlskdProviderSettings settings, HashSet? protectBasenames = null) { - List files = item.SlskdDownloadDirectory?.Files ?? []; + // The merged directory view can carry another item's transfers from a shared + // peer directory; cancelling/removing those would kill a live download. + List files = (item.SlskdDownloadDirectory?.Files ?? []) + .Where(f => item.OwnsFile(f.Filename)) + .ToList(); + if (files.Count == 0 || item.Username == null) { _logger.Debug("No slskd transfers to cancel for {ItemId} (directory not populated); relying on local folder deletion", item.ID); diff --git a/src/Sleezer/Download/Clients/Soulseek/SlskdWatchdog.cs b/src/Sleezer/Download/Clients/Soulseek/SlskdWatchdog.cs index e193588..0284748 100644 --- a/src/Sleezer/Download/Clients/Soulseek/SlskdWatchdog.cs +++ b/src/Sleezer/Download/Clients/Soulseek/SlskdWatchdog.cs @@ -41,6 +41,11 @@ public async Task InspectAsync(SlskdDownloadItem item, SlskdProviderSettings set foreach (SlskdFileState fileState in item.FileStates.Values) { + // A shared peer directory can put another item's transfers in FileStates — + // never cancel a file this item didn't enqueue. + if (!item.OwnsFile(fileState.File.Filename)) + continue; + if (fileState.WatchdogCancelled) continue; diff --git a/tests/Sleezer.Tests/Sleezer.Tests.csproj b/tests/Sleezer.Tests/Sleezer.Tests.csproj index da0f387..a227e5c 100644 --- a/tests/Sleezer.Tests/Sleezer.Tests.csproj +++ b/tests/Sleezer.Tests/Sleezer.Tests.csproj @@ -107,6 +107,14 @@ LinkBase="SourceUnderTest" /> + + + + diff --git a/tests/Sleezer.Tests/SlskdWatchdogTests.cs b/tests/Sleezer.Tests/SlskdWatchdogTests.cs new file mode 100644 index 0000000..d82d067 --- /dev/null +++ b/tests/Sleezer.Tests/SlskdWatchdogTests.cs @@ -0,0 +1,104 @@ +using FluentValidation.Results; +using NLog; +using NzbDrone.Core.Parser.Model; +using NzbDrone.Plugin.Sleezer.Download.Clients.Soulseek; +using NzbDrone.Plugin.Sleezer.Download.Clients.Soulseek.Models; +using NzbDrone.Plugin.Sleezer.Indexers.Soulseek; +using Xunit; + +namespace Sleezer.Tests; + +// ProcessUserTransfers assigns a whole per-peer-directory transfer group to the +// item owning the group's FIRST file, so two items sharing a peer directory can +// see each other's transfers in FileStates. The watchdog cancels at slskd, so an +// unfiltered sweep here kills the other item's live download. +public class SlskdWatchdogTests +{ + private sealed class RecordingApiClient : ISlskdApiClient + { + public List Deleted { get; } = []; + + public Task DeleteTransferAsync(SlskdProviderSettings settings, string username, string fileId, bool remove = false) + { + Deleted.Add(fileId); + return Task.CompletedTask; + } + + public Task EnqueueDownloadAsync(SlskdProviderSettings settings, string username, IEnumerable<(string Filename, long Size)> files, string? externalId = null, string? destination = null) => + Task.FromResult(new SlskdEnqueueResult(null, [], [])); + public Task> GetAllTransfersAsync(SlskdProviderSettings settings, bool includeRemoved = false) => + Task.FromResult(new List()); + public Task GetUserTransfersAsync(SlskdProviderSettings settings, string username) => + Task.FromResult(null); + public Task GetTransferAsync(SlskdProviderSettings settings, string username, string fileId) => + Task.FromResult(null); + public Task GetQueuePositionAsync(SlskdProviderSettings settings, string username, string fileId) => + Task.FromResult(null); + public Task DeleteAllCompletedAsync(SlskdProviderSettings settings) => Task.CompletedTask; + public Task GetDownloadPathAsync(SlskdProviderSettings settings) => + Task.FromResult(null); + public Task GetDestinationConfigAsync(SlskdProviderSettings settings) => + Task.FromResult(null); + public Task TestConnectionAsync(SlskdProviderSettings settings) => + Task.FromResult(null); + public Task<(List Events, int TotalCount)> GetEventsAsync(SlskdProviderSettings settings, int offset, int limit) => + Task.FromResult((new List(), 0)); + } + + private const string OwnedFile = @"@@peer\Artist\Album\01.flac"; + private const string ForeignFile = @"@@peer\Artist\Album\02.flac"; + + // Queued past the position threshold is the one bailout that needs no aged + // timestamps — FirstQueuedAt is only set on a second UpdateFile. + private static SlskdDownloadFile StuckFile(string id, string filename) => new( + Id: id, + Username: "peer", + Direction: "Download", + Filename: filename, + Size: 1000, + StartOffset: 0, + State: "Queued, Remotely", + RequestedAt: DateTime.UtcNow, + EnqueuedAt: DateTime.UtcNow, + StartedAt: DateTime.MinValue, + BytesTransferred: 0, + AverageSpeed: 0, + BytesRemaining: 1000, + ElapsedTime: TimeSpan.Zero, + PercentComplete: 0, + RemainingTime: TimeSpan.Zero, + EndedAt: null, + PlaceInQueue: 9999); + + private static SlskdDownloadItem NewItem(params string[] filenames) + { + string source = "[" + string.Join(",", filenames.Select(f => + $"{{\"Filename\":{System.Text.Json.JsonSerializer.Serialize(f)},\"Size\":1000}}")) + "]"; + return new SlskdDownloadItem(new ReleaseInfo { Source = source, Title = "t", DownloadUrl = "u" }) + { + Username = "peer" + }; + } + + [Fact] + public async Task Watchdog_cancels_only_the_files_this_item_enqueued() + { + SlskdDownloadItem item = NewItem(OwnedFile); + + // Shared peer directory: slskd reports both items' transfers in one group. + item.SlskdDownloadDirectory = new SlskdDownloadDirectory( + @"@@peer\Artist\Album", + 2, + [StuckFile("owned-id", OwnedFile), StuckFile("foreign-id", ForeignFile)]); + + Assert.Equal(2, item.FileStates.Count); + + RecordingApiClient api = new(); + SlskdProviderSettings settings = new() { MaxQueuePositionBeforeCancel = 500 }; + + await new SlskdWatchdog(api, LogManager.GetLogger("tests")) + .InspectAsync(item, settings, CancellationToken.None); + + Assert.Equal(["owned-id"], api.Deleted); + } +} From 78bba777d345f0631cd4becb1d835a0786447375 Mon Sep 17 00:00:00 2001 From: chodeus <190988615+chodeus@users.noreply.github.com> Date: Fri, 21 Aug 2026 12:48:57 +0800 Subject: [PATCH 2/2] fix: use the accepted-ownership predicate on the destructive paths OwnsAcceptedFile landed on main with the extras work; the watchdog and removal guards were written before it existed and still counted files slskd rejected at enqueue as ours. Those produce no transfer of ours, so cancelling or removing one under that name hits another item's download. --- .../Clients/Soulseek/SlskdDownloadManager.cs | 2 +- .../Clients/Soulseek/SlskdWatchdog.cs | 4 ++-- tests/Sleezer.Tests/SlskdWatchdogTests.cs | 22 +++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs b/src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs index beb292d..4c4e6e1 100644 --- a/src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs +++ b/src/Sleezer/Download/Clients/Soulseek/SlskdDownloadManager.cs @@ -1514,7 +1514,7 @@ private async Task RemoveItemFilesAsync(SlskdDownloadItem item, SlskdProviderSet // The merged directory view can carry another item's transfers from a shared // peer directory; cancelling/removing those would kill a live download. List files = (item.SlskdDownloadDirectory?.Files ?? []) - .Where(f => item.OwnsFile(f.Filename)) + .Where(f => item.OwnsAcceptedFile(f.Filename)) .ToList(); if (files.Count == 0 || item.Username == null) diff --git a/src/Sleezer/Download/Clients/Soulseek/SlskdWatchdog.cs b/src/Sleezer/Download/Clients/Soulseek/SlskdWatchdog.cs index 0284748..7f6270e 100644 --- a/src/Sleezer/Download/Clients/Soulseek/SlskdWatchdog.cs +++ b/src/Sleezer/Download/Clients/Soulseek/SlskdWatchdog.cs @@ -42,8 +42,8 @@ public async Task InspectAsync(SlskdDownloadItem item, SlskdProviderSettings set foreach (SlskdFileState fileState in item.FileStates.Values) { // A shared peer directory can put another item's transfers in FileStates — - // never cancel a file this item didn't enqueue. - if (!item.OwnsFile(fileState.File.Filename)) + // never cancel a transfer slskd didn't accept for this item. + if (!item.OwnsAcceptedFile(fileState.File.Filename)) continue; if (fileState.WatchdogCancelled) diff --git a/tests/Sleezer.Tests/SlskdWatchdogTests.cs b/tests/Sleezer.Tests/SlskdWatchdogTests.cs index d82d067..c6363c3 100644 --- a/tests/Sleezer.Tests/SlskdWatchdogTests.cs +++ b/tests/Sleezer.Tests/SlskdWatchdogTests.cs @@ -101,4 +101,26 @@ public async Task Watchdog_cancels_only_the_files_this_item_enqueued() Assert.Equal(["owned-id"], api.Deleted); } + + // slskd creates no transfer for a file it rejected, so one under that name + // is another item's — cancelling it would kill a live download. + [Fact] + public async Task Watchdog_leaves_a_transfer_slskd_rejected_for_this_item_alone() + { + SlskdDownloadItem item = NewItem(OwnedFile, ForeignFile); + item.MarkEnqueueFailed([ForeignFile]); + + item.SlskdDownloadDirectory = new SlskdDownloadDirectory( + @"@@peer\Artist\Album", + 2, + [StuckFile("owned-id", OwnedFile), StuckFile("rejected-id", ForeignFile)]); + + RecordingApiClient api = new(); + SlskdProviderSettings settings = new() { MaxQueuePositionBeforeCancel = 500 }; + + await new SlskdWatchdog(api, LogManager.GetLogger("tests")) + .InspectAsync(item, settings, CancellationToken.None); + + Assert.Equal(["owned-id"], api.Deleted); + } }