From 01e0c1d794748942b3ec6739cdcb87ae009d38ba Mon Sep 17 00:00:00 2001 From: mika kuns Date: Tue, 9 Jun 2026 09:53:58 +0200 Subject: [PATCH] fix(ui): dispose VM subscriptions/timers, guard offline Stop, align review delta-path - DetailsIslandViewModel/TasksIslandViewModel/ListsIslandViewModel: implement IDisposable, unsubscribe Loc.LanguageChanged and worker events (memory leaks). - IslandsShellViewModel: dispose the three System.Timers.Timer instances. - StopAsync: guard on Task/IsRunning/IsConnected and wrap CancelTask in try/catch. - TaskMatchesList virtual:review now matches WaitingForReview (aligns with ReviewFilter). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Islands/DetailsIslandViewModel.cs | 49 +++++++++++++++---- .../Islands/ListsIslandViewModel.cs | 12 ++++- .../Islands/TasksIslandViewModel.cs | 14 ++++-- .../ViewModels/IslandsShellViewModel.cs | 12 ++++- 4 files changed, 72 insertions(+), 15 deletions(-) diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs index d674fbb..53e0758 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs @@ -46,13 +46,21 @@ public sealed class LogLineViewModel }; } -public sealed partial class DetailsIslandViewModel : ViewModelBase +public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable { private readonly IDbContextFactory _dbFactory; private readonly IWorkerClient _worker; private readonly IServiceProvider _services; private readonly INotesApi _notesApi; + // Captured handler delegates for disposal + private readonly EventHandler _langChangedHandler; + private readonly System.ComponentModel.PropertyChangedEventHandler _workerPropertyChangedHandler; + private readonly Action _workerTaskStartedHandler; + private readonly Action _workerTaskFinishedHandler; + private readonly Action _workerWorktreeUpdatedHandler; + private readonly Action _workerTaskUpdatedHandler; + [ObservableProperty] private bool _isNotesMode; [ObservableProperty] private bool _isPrepMode; [ObservableProperty] private bool _isPrepRunning; @@ -501,13 +509,14 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase _notesApi = notesApi; Notes = new NotesEditorViewModel(_notesApi); Subtasks.CollectionChanged += (_, _) => NotifyStepsChanged(); - Loc.LanguageChanged += (_, _) => + _langChangedHandler = (_, _) => { OnPropertyChanged(nameof(AgentStatusLabel)); RecomputeModelBadge(); RecomputeTurnsBadge(); RecomputeAgentBadge(); }; + Loc.LanguageChanged += _langChangedHandler; // Subscribe once; filter by current task id inside the handler _worker.TaskMessageEvent += OnTaskMessage; @@ -516,7 +525,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase _worker.PrepFinishedEvent += OnPrepFinished; // Re-evaluate CanExecute when worker connection flips. - _worker.PropertyChanged += (_, e) => + _workerPropertyChangedHandler = (_, e) => { if (e.PropertyName == nameof(WorkerClient.IsConnected)) { @@ -526,14 +535,17 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase ContinueCommand.NotifyCanExecuteChanged(); } }; + _worker.PropertyChanged += _workerPropertyChangedHandler; // If the task row's live status changes (e.g. TaskStarted/Finished), mirror it. - _worker.TaskStartedEvent += (slot, taskId, startedAt) => + _workerTaskStartedHandler = (slot, taskId, startedAt) => { if (Task?.Id == taskId) AgentState = "running"; _ = RefreshChildOutcomeAsync(taskId); }; - _worker.TaskFinishedEvent += (slot, taskId, status, finishedAt) => + _worker.TaskStartedEvent += _workerTaskStartedHandler; + + _workerTaskFinishedHandler = (slot, taskId, status, finishedAt) => { if (Task?.Id != taskId) return; FlushClaudeBuffer(); @@ -548,20 +560,23 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase _ = RefreshChildOutcomeAsync(taskId); _ = RefreshOutcomeAsync(taskId); }; + _worker.TaskFinishedEvent += _workerTaskFinishedHandler; - _worker.WorktreeUpdatedEvent += taskId => + _workerWorktreeUpdatedHandler = taskId => { if (Task?.Id == taskId) _ = RefreshWorktreeAsync(taskId); if (Task?.IsPlanningParent == true) _ = RefreshPlanningChildAsync(taskId); _ = RefreshChildOutcomeAsync(taskId); }; + _worker.WorktreeUpdatedEvent += _workerWorktreeUpdatedHandler; - _worker.TaskUpdatedEvent += taskId => + _workerTaskUpdatedHandler = taskId => { if (Task?.Id == taskId) _ = RefreshStatusAsync(taskId); if (Task?.IsPlanningParent == true) _ = RefreshPlanningChildAsync(taskId); _ = RefreshChildOutcomeAsync(taskId); }; + _worker.TaskUpdatedEvent += _workerTaskUpdatedHandler; Subtasks.CollectionChanged += (_, _) => { @@ -579,6 +594,20 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase PrepLog.CollectionChanged += (_, _) => OnPropertyChanged(nameof(ShowPrepEmptyState)); } + public void Dispose() + { + Loc.LanguageChanged -= _langChangedHandler; + _worker.PropertyChanged -= _workerPropertyChangedHandler; + _worker.TaskStartedEvent -= _workerTaskStartedHandler; + _worker.TaskFinishedEvent -= _workerTaskFinishedHandler; + _worker.WorktreeUpdatedEvent -= _workerWorktreeUpdatedHandler; + _worker.TaskUpdatedEvent -= _workerTaskUpdatedHandler; + _worker.TaskMessageEvent -= OnTaskMessage; + _worker.PrepStartedEvent -= OnPrepStarted; + _worker.PrepLineEvent -= OnPrepLine; + _worker.PrepFinishedEvent -= OnPrepFinished; + } + private void OnTaskMessage(string taskId, string line) { if (taskId != _subscribedTaskId) return; @@ -1412,8 +1441,10 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase [RelayCommand] private async System.Threading.Tasks.Task StopAsync() { - if (Task == null) return; - await _worker.CancelTaskAsync(Task.Id); + if (Task == null || !IsRunning) return; + if (!_worker.IsConnected) return; + try { await _worker.CancelTaskAsync(Task.Id); } + catch { /* offline */ } } [RelayCommand(CanExecute = nameof(CanEnqueue))] diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/ListsIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/ListsIslandViewModel.cs index cd14462..9336028 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/ListsIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/ListsIslandViewModel.cs @@ -16,7 +16,7 @@ namespace ClaudeDo.Ui.ViewModels.Islands; public enum ListKind { Smart, Virtual, User } -public sealed partial class ListsIslandViewModel : ViewModelBase +public sealed partial class ListsIslandViewModel : ViewModelBase, IDisposable { private readonly IDbContextFactory _dbFactory; private readonly IServiceProvider? _services; @@ -141,6 +141,8 @@ public sealed partial class ListsIslandViewModel : ViewModelBase public string MachineNameLocal => Loc.T("vm.lists.localSuffix", MachineName); public string UserInitials { get; } + private readonly EventHandler _langChangedHandler; + public ListsIslandViewModel(IDbContextFactory dbFactory, IServiceProvider? services = null, WorkerClient? worker = null) { _dbFactory = dbFactory; @@ -163,7 +165,13 @@ public sealed partial class ListsIslandViewModel : ViewModelBase _worker.ConnectionRestoredEvent += () => _ = RefreshCountsAsync(); } - Loc.LanguageChanged += (_, _) => RefreshLocalizedLabels(); + _langChangedHandler = (_, _) => RefreshLocalizedLabels(); + Loc.LanguageChanged += _langChangedHandler; + } + + public void Dispose() + { + Loc.LanguageChanged -= _langChangedHandler; } private static string? SmartListNameKey(string id) => id switch diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs index 015b641..67890f7 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs @@ -14,7 +14,7 @@ using TaskStatus = ClaudeDo.Data.Models.TaskStatus; namespace ClaudeDo.Ui.ViewModels.Islands; -public sealed partial class TasksIslandViewModel : ViewModelBase +public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable { private readonly IDbContextFactory _dbFactory; private readonly IWorkerClient? _worker; @@ -71,6 +71,8 @@ public sealed partial class TasksIslandViewModel : ViewModelBase public Func? ShowUnfinishedPlanningModal { get; set; } + private readonly EventHandler _langChangedHandler; + public TasksIslandViewModel(IDbContextFactory dbFactory, IWorkerClient? worker = null) { _dbFactory = dbFactory; @@ -85,7 +87,13 @@ public sealed partial class TasksIslandViewModel : ViewModelBase _worker.RefineStartedEvent += OnRefineStarted; _worker.RefineFinishedEvent += OnRefineFinished; } - Loc.LanguageChanged += (_, _) => RefreshLocalizedText(); + _langChangedHandler = (_, _) => RefreshLocalizedText(); + Loc.LanguageChanged += _langChangedHandler; + } + + public void Dispose() + { + Loc.LanguageChanged -= _langChangedHandler; } private void RefreshLocalizedText() @@ -178,7 +186,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase ListKind.Smart when list.Id == "smart:my-day" => t.IsMyDay, ListKind.Smart when list.Id == "smart:important" => t.IsStarred, ListKind.Smart when list.Id == "smart:planned" => t.ScheduledFor != null, - ListKind.Virtual when list.Id == "virtual:review" => t.Status == TaskStatus.Done && t.Worktree?.State == WorktreeState.Active && t.ParentTaskId == null, + ListKind.Virtual when list.Id == "virtual:review" => t.Status == TaskStatus.WaitingForReview, ListKind.User => $"user:{t.ListId}" == list.Id, _ => false, }; diff --git a/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs b/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs index 976ee2f..e1fa8ac 100644 --- a/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs @@ -15,7 +15,7 @@ using Microsoft.EntityFrameworkCore; namespace ClaudeDo.Ui.ViewModels; -public sealed partial class IslandsShellViewModel : ViewModelBase +public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable { public ListsIslandViewModel? Lists { get; } public TasksIslandViewModel? Tasks { get; } @@ -268,6 +268,16 @@ public sealed partial class IslandsShellViewModel : ViewModelBase }); } + public void Dispose() + { + _clearTimer.Stop(); + _clearTimer.Dispose(); + _connectTimer.Stop(); + _connectTimer.Dispose(); + _primeStatusTimer.Stop(); + _primeStatusTimer.Dispose(); + } + private void RefreshBannerFromStatus() { switch (_updateCheck.LastCheckStatus)