From 8496a7d29f7ca178ecc05a296f77544b168f8318 Mon Sep 17 00:00:00 2001 From: NeuralFault Date: Tue, 10 Mar 2026 10:38:08 -0400 Subject: [PATCH 1/8] fix(downloads): widen retry exception guard to include SSL/AuthenticationException --- StabilityMatrix.Core/Models/TrackedDownload.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/StabilityMatrix.Core/Models/TrackedDownload.cs b/StabilityMatrix.Core/Models/TrackedDownload.cs index c7a31e3b..1c04f1a5 100644 --- a/StabilityMatrix.Core/Models/TrackedDownload.cs +++ b/StabilityMatrix.Core/Models/TrackedDownload.cs @@ -1,4 +1,5 @@ using System.Diagnostics.CodeAnalysis; +using System.Security.Authentication; using System.Text.Json.Serialization; using AsyncAwaitBestPractices; using NLog; @@ -316,6 +317,15 @@ private void DoCleanup() } } + /// + /// Returns true for transient network/SSL exceptions that are safe to retry. + /// + private static bool IsTransientNetworkException(Exception? ex) => + ex is IOException or AuthenticationException + || ex?.InnerException is IOException or AuthenticationException + || ex is AggregateException ae + && ae.InnerExceptions.Any(e => e is IOException or AuthenticationException); + /// /// Invoked by the task's completion callback /// @@ -349,7 +359,7 @@ private void OnDownloadTaskCompleted(Task task) // Set the exception Exception = task.Exception; - if ((Exception is IOException || Exception?.InnerException is IOException) && attempts < 3) + if (IsTransientNetworkException(Exception) && attempts < 3) { attempts++; Logger.Warn( From 3283355eb1fada0c3e1b54845a5c59d205a9d058 Mon Sep 17 00:00:00 2001 From: NeuralFault Date: Tue, 10 Mar 2026 10:57:16 -0400 Subject: [PATCH 2/8] fix(downloads): add exponential backoff with jitter between retries --- .../Models/TrackedDownload.cs | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/StabilityMatrix.Core/Models/TrackedDownload.cs b/StabilityMatrix.Core/Models/TrackedDownload.cs index 1c04f1a5..bc86d031 100644 --- a/StabilityMatrix.Core/Models/TrackedDownload.cs +++ b/StabilityMatrix.Core/Models/TrackedDownload.cs @@ -319,6 +319,10 @@ private void DoCleanup() /// /// Returns true for transient network/SSL exceptions that are safe to retry. + /// Catches direct IOException/AuthenticationException, the same types wrapped as + /// InnerException (common AggregateException shape from HttpClient), and any leg + /// of a multi-inner AggregateException — covering VPN tunnel resets + /// ("Connection reset by peer") and TLS re-key failures (OpenSSL SSL_ERROR_SSL). /// private static bool IsTransientNetworkException(Exception? ex) => ex is IOException or AuthenticationException @@ -369,9 +373,26 @@ private void OnDownloadTaskCompleted(Task task) attempts ); + // Exponential backoff: 2 s → 4 s → 8 s, capped at 30 s, ±500 ms jitter. + // Gives the VPN tunnel time to re-key/re-route before reconnecting, + // which prevents the retry from hitting the same torn connection. + var delayMs = + (int)Math.Min(2000 * Math.Pow(2, attempts - 1), 30_000) + Random.Shared.Next(-500, 500); + Logger.Debug( + "Download {Download} retrying in {Delay}ms (attempt {Attempt}/3)", + FileName, + delayMs, + attempts + ); + + // Persist Inactive state to disk before the delay so that a restart + // during the backoff window loads the download as a resumable entry. OnProgressStateChanging(ProgressState.Inactive); ProgressState = ProgressState.Inactive; - Resume(); + OnProgressStateChanged(ProgressState.Inactive); + + // Fire-and-forget the delayed resume to avoid blocking the task continuation thread. + Task.Delay(Math.Max(delayMs, 0)).ContinueWith(_ => Resume()).SafeFireAndForget(); return; } From 944d27a88310ba650a41d57107876b751eb6330c Mon Sep 17 00:00:00 2001 From: NeuralFault Date: Tue, 10 Mar 2026 18:19:21 -0400 Subject: [PATCH 3/8] feat: add Retry button for failed model downloads in download manager --- .../Base/PausableProgressItemViewModelBase.cs | 19 ++++++++++++++++++- .../Progress/DownloadProgressItemViewModel.cs | 14 ++++++++++++++ .../Views/ProgressManagerPage.axaml | 9 +++++++++ .../Models/TrackedDownload.cs | 10 ++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/StabilityMatrix.Avalonia/ViewModels/Base/PausableProgressItemViewModelBase.cs b/StabilityMatrix.Avalonia/ViewModels/Base/PausableProgressItemViewModelBase.cs index 0881e65d..6ecea5df 100644 --- a/StabilityMatrix.Avalonia/ViewModels/Base/PausableProgressItemViewModelBase.cs +++ b/StabilityMatrix.Avalonia/ViewModels/Base/PausableProgressItemViewModelBase.cs @@ -14,7 +14,8 @@ public abstract partial class PausableProgressItemViewModelBase : ProgressItemVi nameof(IsPaused), nameof(IsCompleted), nameof(CanPauseResume), - nameof(CanCancel) + nameof(CanCancel), + nameof(CanRetry) )] private ProgressState state = ProgressState.Inactive; @@ -33,9 +34,20 @@ public abstract partial class PausableProgressItemViewModelBase : ProgressItemVi public virtual bool SupportsPauseResume => true; public virtual bool SupportsCancel => true; + /// + /// Override to true in subclasses that support manual retry after failure. + /// Defaults to false so unrelated progress item types are never affected. + /// + public virtual bool SupportsRetry => false; + public bool CanPauseResume => SupportsPauseResume && !IsCompleted && !IsPending; public bool CanCancel => SupportsCancel && !IsCompleted; + /// + /// True only when this item supports retry AND is in the Failed state. + /// + public bool CanRetry => SupportsRetry && State == ProgressState.Failed; + private AsyncRelayCommand? pauseCommand; public IAsyncRelayCommand PauseCommand => pauseCommand ??= new AsyncRelayCommand(Pause); @@ -51,6 +63,11 @@ public abstract partial class PausableProgressItemViewModelBase : ProgressItemVi public virtual Task Cancel() => Task.CompletedTask; + private AsyncRelayCommand? retryCommand; + public IAsyncRelayCommand RetryCommand => retryCommand ??= new AsyncRelayCommand(Retry); + + public virtual Task Retry() => Task.CompletedTask; + [RelayCommand] private Task TogglePauseResume() { diff --git a/StabilityMatrix.Avalonia/ViewModels/Progress/DownloadProgressItemViewModel.cs b/StabilityMatrix.Avalonia/ViewModels/Progress/DownloadProgressItemViewModel.cs index 04809ec2..d8d12bed 100644 --- a/StabilityMatrix.Avalonia/ViewModels/Progress/DownloadProgressItemViewModel.cs +++ b/StabilityMatrix.Avalonia/ViewModels/Progress/DownloadProgressItemViewModel.cs @@ -71,6 +71,11 @@ private void OnProgressStateChanged(ProgressState state) } } + /// + /// Downloads support manual retry when they reach the Failed state. + /// + public override bool SupportsRetry => true; + /// public override Task Cancel() { @@ -91,4 +96,13 @@ public override Task Resume() { return downloadService.TryResumeDownload(download); } + + /// + /// Resets the internal retry counter so the user gets a fresh 3-attempt budget, + /// then re-queues the download exactly as if it were being resumed from pause. + public override Task Retry() + { + download.ResetAttempts(); + return downloadService.TryResumeDownload(download); + } } diff --git a/StabilityMatrix.Avalonia/Views/ProgressManagerPage.axaml b/StabilityMatrix.Avalonia/Views/ProgressManagerPage.axaml index c2286200..89557e6b 100644 --- a/StabilityMatrix.Avalonia/Views/ProgressManagerPage.axaml +++ b/StabilityMatrix.Avalonia/Views/ProgressManagerPage.axaml @@ -113,6 +113,15 @@ IsVisible="{Binding CanCancel}"> + + + + /// Resets the internal retry attempt counter back to zero. + /// Call this before a user-initiated retry so the download gets + /// a fresh budget of automatic retries on the new attempt. + /// + public void ResetAttempts() + { + attempts = 0; + } + public void SetDownloadService(IDownloadService service) { downloadService = service; From 1f06c3e85d803def896a13dad26b688d57c08856 Mon Sep 17 00:00:00 2001 From: NeuralFault Date: Tue, 10 Mar 2026 19:41:59 -0400 Subject: [PATCH 4/8] Fix retry button by re-adding failed downloads to tracking dict before resuming --- .../Progress/DownloadProgressItemViewModel.cs | 5 +++-- .../Views/ProgressManagerPage.axaml | 2 +- .../Models/TrackedDownload.cs | 17 +++++++--------- .../Services/ITrackedDownloadService.cs | 2 ++ .../Services/TrackedDownloadService.cs | 20 +++++++++++++++++++ 5 files changed, 33 insertions(+), 13 deletions(-) diff --git a/StabilityMatrix.Avalonia/ViewModels/Progress/DownloadProgressItemViewModel.cs b/StabilityMatrix.Avalonia/ViewModels/Progress/DownloadProgressItemViewModel.cs index d8d12bed..ff6a2bc7 100644 --- a/StabilityMatrix.Avalonia/ViewModels/Progress/DownloadProgressItemViewModel.cs +++ b/StabilityMatrix.Avalonia/ViewModels/Progress/DownloadProgressItemViewModel.cs @@ -99,10 +99,11 @@ public override Task Resume() /// /// Resets the internal retry counter so the user gets a fresh 3-attempt budget, - /// then re-queues the download exactly as if it were being resumed from pause. + /// then re-registers the download in the service dictionary (it was removed on + /// failure) and resumes it through the normal concurrency queue. public override Task Retry() { download.ResetAttempts(); - return downloadService.TryResumeDownload(download); + return downloadService.TryRestartDownload(download); } } diff --git a/StabilityMatrix.Avalonia/Views/ProgressManagerPage.axaml b/StabilityMatrix.Avalonia/Views/ProgressManagerPage.axaml index 89557e6b..93cc353a 100644 --- a/StabilityMatrix.Avalonia/Views/ProgressManagerPage.axaml +++ b/StabilityMatrix.Avalonia/Views/ProgressManagerPage.axaml @@ -120,7 +120,7 @@ Command="{Binding RetryCommand}" IsVisible="{Binding CanRetry}" ToolTip.Tip="Retry download"> - + diff --git a/StabilityMatrix.Core/Models/TrackedDownload.cs b/StabilityMatrix.Core/Models/TrackedDownload.cs index 3d785d69..9c5b9240 100644 --- a/StabilityMatrix.Core/Models/TrackedDownload.cs +++ b/StabilityMatrix.Core/Models/TrackedDownload.cs @@ -318,11 +318,8 @@ private void DoCleanup() } /// - /// Returns true for transient network/SSL exceptions that are safe to retry. - /// Catches direct IOException/AuthenticationException, the same types wrapped as - /// InnerException (common AggregateException shape from HttpClient), and any leg - /// of a multi-inner AggregateException — covering VPN tunnel resets - /// ("Connection reset by peer") and TLS re-key failures (OpenSSL SSL_ERROR_SSL). + /// Returns true for transient network/SSL exceptions that are safe to retry (ie: VPN tunnel resets or TLS re-key failures) + /// (IOException, AuthenticationException, or either wrapped in an AggregateException). /// private static bool IsTransientNetworkException(Exception? ex) => ex is IOException or AuthenticationException @@ -385,8 +382,7 @@ private void OnDownloadTaskCompleted(Task task) attempts ); - // Persist Inactive state to disk before the delay so that a restart - // during the backoff window loads the download as a resumable entry. + // Persist Inactive to disk before the delay so a restart during backoff loads it as resumable. OnProgressStateChanging(ProgressState.Inactive); ProgressState = ProgressState.Inactive; OnProgressStateChanged(ProgressState.Inactive); @@ -424,13 +420,14 @@ private void OnDownloadTaskCompleted(Task task) } /// - /// Resets the internal retry attempt counter back to zero. - /// Call this before a user-initiated retry so the download gets - /// a fresh budget of automatic retries on the new attempt. + /// Resets the retry counter and silently sets state to Inactive without firing events. + /// Must be called before re-adding to TrackedDownloadService to avoid events + /// firing while the download is absent from the dictionary. /// public void ResetAttempts() { attempts = 0; + ProgressState = ProgressState.Inactive; } public void SetDownloadService(IDownloadService service) diff --git a/StabilityMatrix.Core/Services/ITrackedDownloadService.cs b/StabilityMatrix.Core/Services/ITrackedDownloadService.cs index ee1e2ba8..86c00da3 100644 --- a/StabilityMatrix.Core/Services/ITrackedDownloadService.cs +++ b/StabilityMatrix.Core/Services/ITrackedDownloadService.cs @@ -15,5 +15,7 @@ TrackedDownload NewDownload(string downloadUrl, FilePath downloadPath) => NewDownload(new Uri(downloadUrl), downloadPath); Task TryStartDownload(TrackedDownload download); Task TryResumeDownload(TrackedDownload download); + Task TryRestartDownload(TrackedDownload download); + void UpdateMaxConcurrentDownloads(int newMax); } diff --git a/StabilityMatrix.Core/Services/TrackedDownloadService.cs b/StabilityMatrix.Core/Services/TrackedDownloadService.cs index 12cf3ca7..8738f1fa 100644 --- a/StabilityMatrix.Core/Services/TrackedDownloadService.cs +++ b/StabilityMatrix.Core/Services/TrackedDownloadService.cs @@ -129,6 +129,26 @@ public async Task TryStartDownload(TrackedDownload download) } } + public async Task TryRestartDownload(TrackedDownload download) + { + // Re-create the backing JSON file and re-add to the dictionary. + // Downloads are removed on failure, so this restores the tracking entry + // so that subsequent state-change events can persist normally. + var downloadsDir = new DirectoryPath(settingsManager.DownloadsDirectory); + downloadsDir.Create(); + var jsonFile = downloadsDir.JoinFile($"{download.Id}.json"); + + var jsonFileStream = jsonFile.Info.Open(FileMode.Create, FileAccess.ReadWrite, FileShare.Read); + var json = JsonSerializer.Serialize(download); + jsonFileStream.Write(Encoding.UTF8.GetBytes(json)); + jsonFileStream.Flush(); + + // Handlers are already attached from the original AddDownload call. + downloads.TryAdd(download.Id, (download, jsonFileStream)); + + await TryResumeDownload(download).ConfigureAwait(false); + } + public async Task TryResumeDownload(TrackedDownload download) { if (IsQueueEnabled && ActiveDownloads >= MaxConcurrentDownloads) From b1f92b4323260575bdc9c14bda0354098030b2c9 Mon Sep 17 00:00:00 2001 From: NeuralFault Date: Tue, 10 Mar 2026 21:39:31 -0400 Subject: [PATCH 5/8] Address PR review issues - cancellable retry delay, async file I/O, stream leak, and race guards in Resume/Start --- .../Models/TrackedDownload.cs | 39 ++++++++++++++++++- .../Services/TrackedDownloadService.cs | 32 ++++++++++++--- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/StabilityMatrix.Core/Models/TrackedDownload.cs b/StabilityMatrix.Core/Models/TrackedDownload.cs index 9c5b9240..ed75b7f7 100644 --- a/StabilityMatrix.Core/Models/TrackedDownload.cs +++ b/StabilityMatrix.Core/Models/TrackedDownload.cs @@ -79,6 +79,7 @@ public class TrackedDownload public Exception? Exception { get; private set; } private int attempts; + private CancellationTokenSource? retryDelayCancellationTokenSource; #region Events public event EventHandler? ProgressUpdate; @@ -185,6 +186,11 @@ internal void Start() $"Download state must be inactive or pending to start, not {ProgressState}" ); } + // Cancel any pending auto-retry delay (defensive: Start() accepts Inactive state). + retryDelayCancellationTokenSource?.Cancel(); + retryDelayCancellationTokenSource?.Dispose(); + retryDelayCancellationTokenSource = null; + Logger.Debug("Starting download {Download}", FileName); EnsureDownloadService(); @@ -202,6 +208,11 @@ internal void Start() internal void Resume() { + // Cancel any pending auto-retry delay since we're resuming now. + retryDelayCancellationTokenSource?.Cancel(); + retryDelayCancellationTokenSource?.Dispose(); + retryDelayCancellationTokenSource = null; + if (ProgressState != ProgressState.Inactive && ProgressState != ProgressState.Paused) { Logger.Warn( @@ -209,6 +220,7 @@ internal void Resume() FileName, ProgressState ); + return; } Logger.Debug("Resuming download {Download}", FileName); @@ -236,6 +248,11 @@ internal void Resume() public void Pause() { + // Cancel any pending auto-retry delay. + retryDelayCancellationTokenSource?.Cancel(); + retryDelayCancellationTokenSource?.Dispose(); + retryDelayCancellationTokenSource = null; + if (ProgressState != ProgressState.Working) { Logger.Warn( @@ -265,6 +282,11 @@ public void Cancel() return; } + // Cancel any pending auto-retry delay. + retryDelayCancellationTokenSource?.Cancel(); + retryDelayCancellationTokenSource?.Dispose(); + retryDelayCancellationTokenSource = null; + Logger.Debug("Cancelling download {Download}", FileName); // Cancel token if it exists @@ -387,8 +409,21 @@ private void OnDownloadTaskCompleted(Task task) ProgressState = ProgressState.Inactive; OnProgressStateChanged(ProgressState.Inactive); - // Fire-and-forget the delayed resume to avoid blocking the task continuation thread. - Task.Delay(Math.Max(delayMs, 0)).ContinueWith(_ => Resume()).SafeFireAndForget(); + // Clean up the completed task resources; Resume() will create new ones. + downloadTask = null; + downloadCancellationTokenSource = null; + downloadPauseTokenSource = null; + + // Schedule the retry with a cancellation token so Cancel/Pause can abort the delay. + retryDelayCancellationTokenSource?.Dispose(); + retryDelayCancellationTokenSource = new CancellationTokenSource(); + Task.Delay(Math.Max(delayMs, 0), retryDelayCancellationTokenSource.Token) + .ContinueWith(t => + { + if (t.IsCompletedSuccessfully) + Resume(); + }) + .SafeFireAndForget(); return; } diff --git a/StabilityMatrix.Core/Services/TrackedDownloadService.cs b/StabilityMatrix.Core/Services/TrackedDownloadService.cs index 8738f1fa..69bb2363 100644 --- a/StabilityMatrix.Core/Services/TrackedDownloadService.cs +++ b/StabilityMatrix.Core/Services/TrackedDownloadService.cs @@ -138,13 +138,33 @@ public async Task TryRestartDownload(TrackedDownload download) downloadsDir.Create(); var jsonFile = downloadsDir.JoinFile($"{download.Id}.json"); - var jsonFileStream = jsonFile.Info.Open(FileMode.Create, FileAccess.ReadWrite, FileShare.Read); - var json = JsonSerializer.Serialize(download); - jsonFileStream.Write(Encoding.UTF8.GetBytes(json)); - jsonFileStream.Flush(); + var jsonFileStream = new FileStream( + jsonFile.Info.FullName, + FileMode.Create, + FileAccess.ReadWrite, + FileShare.Read, + bufferSize: 4096, + useAsync: true + ); + var jsonBytes = JsonSerializer.SerializeToUtf8Bytes(download); + + try + { + await jsonFileStream.WriteAsync(jsonBytes).ConfigureAwait(false); + await jsonFileStream.FlushAsync().ConfigureAwait(false); - // Handlers are already attached from the original AddDownload call. - downloads.TryAdd(download.Id, (download, jsonFileStream)); + // Handlers are already attached from the original AddDownload call. + if (!downloads.TryAdd(download.Id, (download, jsonFileStream))) + { + // Already tracked; discard the newly opened stream. + await jsonFileStream.DisposeAsync().ConfigureAwait(false); + } + } + catch + { + await jsonFileStream.DisposeAsync().ConfigureAwait(false); + throw; + } await TryResumeDownload(download).ConfigureAwait(false); } From 9ba4d32f2e455b6a7b938ea7a379ab5585ff0c81 Mon Sep 17 00:00:00 2001 From: NeuralFault Date: Tue, 10 Mar 2026 22:04:48 -0400 Subject: [PATCH 6/8] Extract CancelRetryDelay() helper to eliminate duplication --- .../Models/TrackedDownload.cs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/StabilityMatrix.Core/Models/TrackedDownload.cs b/StabilityMatrix.Core/Models/TrackedDownload.cs index ed75b7f7..9f3874d8 100644 --- a/StabilityMatrix.Core/Models/TrackedDownload.cs +++ b/StabilityMatrix.Core/Models/TrackedDownload.cs @@ -121,6 +121,13 @@ private void EnsureDownloadService() } } + private void CancelRetryDelay() + { + retryDelayCancellationTokenSource?.Cancel(); + retryDelayCancellationTokenSource?.Dispose(); + retryDelayCancellationTokenSource = null; + } + private async Task StartDownloadTask(long resumeFromByte, CancellationToken cancellationToken) { var progress = new Progress(OnProgressUpdate); @@ -187,9 +194,7 @@ internal void Start() ); } // Cancel any pending auto-retry delay (defensive: Start() accepts Inactive state). - retryDelayCancellationTokenSource?.Cancel(); - retryDelayCancellationTokenSource?.Dispose(); - retryDelayCancellationTokenSource = null; + CancelRetryDelay(); Logger.Debug("Starting download {Download}", FileName); @@ -209,9 +214,7 @@ internal void Start() internal void Resume() { // Cancel any pending auto-retry delay since we're resuming now. - retryDelayCancellationTokenSource?.Cancel(); - retryDelayCancellationTokenSource?.Dispose(); - retryDelayCancellationTokenSource = null; + CancelRetryDelay(); if (ProgressState != ProgressState.Inactive && ProgressState != ProgressState.Paused) { @@ -249,9 +252,7 @@ internal void Resume() public void Pause() { // Cancel any pending auto-retry delay. - retryDelayCancellationTokenSource?.Cancel(); - retryDelayCancellationTokenSource?.Dispose(); - retryDelayCancellationTokenSource = null; + CancelRetryDelay(); if (ProgressState != ProgressState.Working) { @@ -283,9 +284,7 @@ public void Cancel() } // Cancel any pending auto-retry delay. - retryDelayCancellationTokenSource?.Cancel(); - retryDelayCancellationTokenSource?.Dispose(); - retryDelayCancellationTokenSource = null; + CancelRetryDelay(); Logger.Debug("Cancelling download {Download}", FileName); From 8d6fbc52e613bea9bb4ebf43aca9aeaf3556e318 Mon Sep 17 00:00:00 2001 From: NeuralFault Date: Tue, 10 Mar 2026 22:09:23 -0400 Subject: [PATCH 7/8] Replace magic number with constant for max retry attempts in TrackedDownload --- StabilityMatrix.Core/Models/TrackedDownload.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/StabilityMatrix.Core/Models/TrackedDownload.cs b/StabilityMatrix.Core/Models/TrackedDownload.cs index 9f3874d8..3a105291 100644 --- a/StabilityMatrix.Core/Models/TrackedDownload.cs +++ b/StabilityMatrix.Core/Models/TrackedDownload.cs @@ -78,6 +78,7 @@ public class TrackedDownload [JsonIgnore] public Exception? Exception { get; private set; } + private const int MaxRetryAttempts = 3; private int attempts; private CancellationTokenSource? retryDelayCancellationTokenSource; @@ -381,7 +382,7 @@ private void OnDownloadTaskCompleted(Task task) // Set the exception Exception = task.Exception; - if (IsTransientNetworkException(Exception) && attempts < 3) + if (IsTransientNetworkException(Exception) && attempts < MaxRetryAttempts) { attempts++; Logger.Warn( @@ -397,10 +398,11 @@ private void OnDownloadTaskCompleted(Task task) var delayMs = (int)Math.Min(2000 * Math.Pow(2, attempts - 1), 30_000) + Random.Shared.Next(-500, 500); Logger.Debug( - "Download {Download} retrying in {Delay}ms (attempt {Attempt}/3)", + "Download {Download} retrying in {Delay}ms (attempt {Attempt}/{MaxAttempts})", FileName, delayMs, - attempts + attempts, + MaxRetryAttempts ); // Persist Inactive to disk before the delay so a restart during backoff loads it as resumable. From 790af453edce26d0d9139cbf648c2dc73d59f546 Mon Sep 17 00:00:00 2001 From: NeuralFault Date: Wed, 8 Apr 2026 20:05:41 -0400 Subject: [PATCH 8/8] fix: address review feedback. - Fix Resume() ignoring Pending state, causing queued downloads to stall after ProcessPendingDownloads() enqueues them (P1) - Preserve sidecar files (.cm-info.json, preview image) on download failure so manual retry can succeed; only delete sidecars on explicit cancel or dismiss (P2) - Add Dismiss action to TrackedDownload, ownloadProgressItemViewModel, and ProgressManagerPage so users can clean up failed downloads without retrying, orphaning sidecars in the process - Fix KeyNotFoundException crash in UpdateJsonForDownload when Dismiss fires a state-change event after the download was already removed from the tracking dictionary on failure - Add unit tests: Resume_WhileInPendingState_SetsStateToWorking, OnFailed_SidecarFilesPreservedForRetry, OnCancelled_SidecarFilesAreDeleted --- .../Base/PausableProgressItemViewModelBase.cs | 19 +- .../Progress/DownloadProgressItemViewModel.cs | 15 ++ .../Views/ProgressManagerPage.axaml | 9 + .../Models/TrackedDownload.cs | 53 +++++- .../Services/TrackedDownloadService.cs | 7 +- .../Models/TrackedDownloadTests.cs | 178 ++++++++++++++++++ 6 files changed, 274 insertions(+), 7 deletions(-) create mode 100644 StabilityMatrix.Tests/Models/TrackedDownloadTests.cs diff --git a/StabilityMatrix.Avalonia/ViewModels/Base/PausableProgressItemViewModelBase.cs b/StabilityMatrix.Avalonia/ViewModels/Base/PausableProgressItemViewModelBase.cs index 6ecea5df..695da5b6 100644 --- a/StabilityMatrix.Avalonia/ViewModels/Base/PausableProgressItemViewModelBase.cs +++ b/StabilityMatrix.Avalonia/ViewModels/Base/PausableProgressItemViewModelBase.cs @@ -15,7 +15,8 @@ public abstract partial class PausableProgressItemViewModelBase : ProgressItemVi nameof(IsCompleted), nameof(CanPauseResume), nameof(CanCancel), - nameof(CanRetry) + nameof(CanRetry), + nameof(CanDismiss) )] private ProgressState state = ProgressState.Inactive; @@ -40,6 +41,12 @@ public abstract partial class PausableProgressItemViewModelBase : ProgressItemVi /// public virtual bool SupportsRetry => false; + /// + /// Override to true in subclasses that support dismissing a failed item, + /// which runs full sidecar cleanup before removing the entry. + /// + public virtual bool SupportsDismiss => false; + public bool CanPauseResume => SupportsPauseResume && !IsCompleted && !IsPending; public bool CanCancel => SupportsCancel && !IsCompleted; @@ -48,6 +55,11 @@ public abstract partial class PausableProgressItemViewModelBase : ProgressItemVi /// public bool CanRetry => SupportsRetry && State == ProgressState.Failed; + /// + /// True only when this item supports dismiss AND is in the Failed state. + /// + public bool CanDismiss => SupportsDismiss && State == ProgressState.Failed; + private AsyncRelayCommand? pauseCommand; public IAsyncRelayCommand PauseCommand => pauseCommand ??= new AsyncRelayCommand(Pause); @@ -68,6 +80,11 @@ public abstract partial class PausableProgressItemViewModelBase : ProgressItemVi public virtual Task Retry() => Task.CompletedTask; + private AsyncRelayCommand? dismissCommand; + public IAsyncRelayCommand DismissCommand => dismissCommand ??= new AsyncRelayCommand(Dismiss); + + public virtual Task Dismiss() => Task.CompletedTask; + [RelayCommand] private Task TogglePauseResume() { diff --git a/StabilityMatrix.Avalonia/ViewModels/Progress/DownloadProgressItemViewModel.cs b/StabilityMatrix.Avalonia/ViewModels/Progress/DownloadProgressItemViewModel.cs index ff6a2bc7..c763d5a4 100644 --- a/StabilityMatrix.Avalonia/ViewModels/Progress/DownloadProgressItemViewModel.cs +++ b/StabilityMatrix.Avalonia/ViewModels/Progress/DownloadProgressItemViewModel.cs @@ -76,6 +76,12 @@ private void OnProgressStateChanged(ProgressState state) /// public override bool SupportsRetry => true; + /// + /// Downloads support dismiss, which cleans up all sidecar files when + /// the user discards a failed download without retrying. + /// + public override bool SupportsDismiss => true; + /// public override Task Cancel() { @@ -106,4 +112,13 @@ public override Task Retry() download.ResetAttempts(); return downloadService.TryRestartDownload(download); } + + /// + /// Runs full cleanup (temp file + sidecar files) for a failed download the user + /// chooses not to retry, then transitions to Cancelled so the service removes it. + public override Task Dismiss() + { + download.Dismiss(); + return Task.CompletedTask; + } } diff --git a/StabilityMatrix.Avalonia/Views/ProgressManagerPage.axaml b/StabilityMatrix.Avalonia/Views/ProgressManagerPage.axaml index 93cc353a..87dfbadc 100644 --- a/StabilityMatrix.Avalonia/Views/ProgressManagerPage.axaml +++ b/StabilityMatrix.Avalonia/Views/ProgressManagerPage.axaml @@ -122,6 +122,15 @@ ToolTip.Tip="Retry download"> + + + + /// Cleans up temp file and all sidecar files (e.g. .cm-info.json, preview image) + /// for a download that has already failed and will not be retried. + /// This transitions the state to so the + /// service removes the tracking entry. + /// + public void Dismiss() + { + if (ProgressState != ProgressState.Failed) + { + Logger.Warn( + "Attempted to dismiss download {Download} but it is not in a failed state ({State})", + FileName, + ProgressState + ); + return; + } + + Logger.Debug("Dismissing failed download {Download}", FileName); + + DoCleanup(); + + OnProgressStateChanging(ProgressState.Cancelled); + ProgressState = ProgressState.Cancelled; + OnProgressStateChanged(ProgressState); + } + public void Cancel() { if (ProgressState is not (ProgressState.Working or ProgressState.Inactive)) @@ -313,9 +344,12 @@ public void SetPending() } /// - /// Deletes the temp file and any extra cleanup files + /// Deletes the temp file and, optionally, any extra cleanup files (e.g. sidecar metadata). + /// Pass as false when the download + /// failed but may be retried — sidecar files (.cm-info.json, preview image) should survive + /// so a manual retry doesn't need to recreate them. /// - private void DoCleanup() + private void DoCleanup(bool includeExtraCleanupFiles = true) { try { @@ -326,6 +360,9 @@ private void DoCleanup() Logger.Warn("Failed to delete temp file {TempFile}", TempFileName); } + if (!includeExtraCleanupFiles) + return; + foreach (var extraFile in ExtraCleanupFileNames) { try @@ -440,11 +477,17 @@ private void OnDownloadTaskCompleted(Task task) ProgressState = ProgressState.Success; } - // For failed or cancelled, delete the temp files - if (ProgressState is ProgressState.Failed or ProgressState.Cancelled) + // For cancelled, delete the temp file and any sidecar metadata. + // For failed, only delete the temp file — sidecar files (.cm-info.json, preview image) + // are preserved so a manual retry doesn't need to recreate them. + if (ProgressState is ProgressState.Cancelled) { DoCleanup(); } + else if (ProgressState is ProgressState.Failed) + { + DoCleanup(includeExtraCleanupFiles: false); + } // For pause, just do nothing OnProgressStateChanged(ProgressState); diff --git a/StabilityMatrix.Core/Services/TrackedDownloadService.cs b/StabilityMatrix.Core/Services/TrackedDownloadService.cs index 69bb2363..3db919db 100644 --- a/StabilityMatrix.Core/Services/TrackedDownloadService.cs +++ b/StabilityMatrix.Core/Services/TrackedDownloadService.cs @@ -245,12 +245,17 @@ private void AdjustSemaphoreCount() /// private void UpdateJsonForDownload(TrackedDownload download) { + // The download may have already been removed from the dictionary (e.g. it failed and was + // then dismissed). In that case there is nothing to persist, so skip silently. + if (!downloads.TryGetValue(download.Id, out var downloadInfo)) + return; + // Serialize to json var json = JsonSerializer.Serialize(download); var jsonBytes = Encoding.UTF8.GetBytes(json); // Write to file - var (_, fs) = downloads[download.Id]; + var (_, fs) = downloadInfo; fs.Seek(0, SeekOrigin.Begin); fs.Write(jsonBytes); fs.Flush(); diff --git a/StabilityMatrix.Tests/Models/TrackedDownloadTests.cs b/StabilityMatrix.Tests/Models/TrackedDownloadTests.cs new file mode 100644 index 00000000..63804534 --- /dev/null +++ b/StabilityMatrix.Tests/Models/TrackedDownloadTests.cs @@ -0,0 +1,178 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using NSubstitute; +using StabilityMatrix.Core.Models; +using StabilityMatrix.Core.Models.FileInterfaces; +using StabilityMatrix.Core.Models.Progress; +using StabilityMatrix.Core.Services; + +namespace StabilityMatrix.Tests.Models; + +[TestClass] +public class TrackedDownloadTests +{ + private string tempDir = null!; + + [TestInitialize] + public void Setup() + { + tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + Directory.CreateDirectory(tempDir); + } + + [TestCleanup] + public void Cleanup() + { + if (Directory.Exists(tempDir)) + Directory.Delete(tempDir, recursive: true); + } + + private TrackedDownload CreateDownload(IDownloadService downloadService) + { + var download = new TrackedDownload + { + Id = Guid.NewGuid(), + SourceUrl = new Uri("https://example.com/model.safetensors"), + DownloadDirectory = new DirectoryPath(tempDir), + FileName = "model.safetensors", + TempFileName = "model.safetensors.partial", + }; + download.SetDownloadService(downloadService); + return download; + } + + // Resume() must proceed when the state is Pending (queued resume fix). + + [TestMethod] + public async Task Resume_WhileInPendingState_SetsStateToWorking() + { + // Arrange – download service that blocks forever until cancelled. + var downloadService = Substitute.For(); + downloadService + .ResumeDownloadToFileAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>(), + Arg.Any(), + Arg.Any() + ) + .Returns(callInfo => + { + var ct = callInfo.Arg(); + return Task.Delay(Timeout.Infinite, ct); + }); + + var download = CreateDownload(downloadService); + download.SetPending(); // Simulate being queued + + // Act + download.Resume(); + + // Assert – Resume() must not have returned early; state is now Working. + Assert.AreEqual(ProgressState.Working, download.ProgressState); + + // Cleanup – cancel and wait for the task to finish. + var cancelledTcs = new TaskCompletionSource(); + download.ProgressStateChanged += (_, state) => + { + if (state == ProgressState.Cancelled) + cancelledTcs.TrySetResult(); + }; + download.Cancel(); + await cancelledTcs.Task.WaitAsync(TimeSpan.FromSeconds(5)); + } + + // Sidecar files (.cm-info.json, preview image) must survive a failed + // download so a manual retry can succeed without recreating them. + + [TestMethod] + public async Task OnFailed_SidecarFilesPreservedForRetry() + { + // Arrange – create sidecar files that ModelImportService would have written. + var sidecarPath = Path.Combine(tempDir, "model.cm-info.json"); + var previewPath = Path.Combine(tempDir, "model.preview.png"); + await File.WriteAllTextAsync(sidecarPath, "{}"); + await File.WriteAllTextAsync(previewPath, "PNG"); + + // Download service that immediately throws a non-transient error + // (InvalidOperationException is not an IOException/AuthenticationException, + // so it goes straight to Failed without any auto-retry attempts). + var downloadService = Substitute.For(); + downloadService + .ResumeDownloadToFileAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>(), + Arg.Any(), + Arg.Any() + ) + .Returns(Task.FromException(new InvalidOperationException("Simulated download error"))); + + var download = CreateDownload(downloadService); + download.ExtraCleanupFileNames.Add(sidecarPath); + download.ExtraCleanupFileNames.Add(previewPath); + + var failedTcs = new TaskCompletionSource(); + download.ProgressStateChanged += (_, state) => + { + if (state == ProgressState.Failed) + failedTcs.TrySetResult(); + }; + + // Act + download.Start(); + await failedTcs.Task.WaitAsync(TimeSpan.FromSeconds(5)); + + // Assert – sidecar files must still exist for a potential manual retry. + Assert.IsTrue(File.Exists(sidecarPath), ".cm-info.json should be preserved after failure"); + Assert.IsTrue(File.Exists(previewPath), "Preview image should be preserved after failure"); + } + + [TestMethod] + public async Task OnCancelled_SidecarFilesAreDeleted() + { + // Sidecar files should still be cleaned up when the user explicitly cancels. + var sidecarPath = Path.Combine(tempDir, "model.cm-info.json"); + var previewPath = Path.Combine(tempDir, "model.preview.png"); + await File.WriteAllTextAsync(sidecarPath, "{}"); + await File.WriteAllTextAsync(previewPath, "PNG"); + + var downloadService = Substitute.For(); + downloadService + .ResumeDownloadToFileAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>(), + Arg.Any(), + Arg.Any() + ) + .Returns(callInfo => + { + var ct = callInfo.Arg(); + return Task.Delay(Timeout.Infinite, ct); + }); + + var download = CreateDownload(downloadService); + download.ExtraCleanupFileNames.Add(sidecarPath); + download.ExtraCleanupFileNames.Add(previewPath); + + var cancelledTcs = new TaskCompletionSource(); + download.ProgressStateChanged += (_, state) => + { + if (state == ProgressState.Cancelled) + cancelledTcs.TrySetResult(); + }; + + download.Start(); + download.Cancel(); + await cancelledTcs.Task.WaitAsync(TimeSpan.FromSeconds(5)); + + Assert.IsFalse(File.Exists(sidecarPath), ".cm-info.json should be deleted on cancel"); + Assert.IsFalse(File.Exists(previewPath), "Preview image should be deleted on cancel"); + } +}