Fix: subscribe to ItemsChanged before fetching items to avoid missing early events

Agent-Logs-Url: https://github.com/microsoft/PowerToys/sessions/3f380bd3-a56a-4708-80ef-3e7d534c4f6f

Co-authored-by: MuyuanMS <116717757+MuyuanMS@users.noreply.github.com>

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs

@@ -160,17 +160,6 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa
             return;
         }
 
-        // Guard against a race with UnsafeCleanup on the UI thread.
-        // _initializeItemsTask runs on a background thread and may call
-        // InitializeProperties() after SafeCleanup() has already removed the
-        // event subscriptions.  If we allowed the subscription to happen here
-        // there would be no cleanup path for it, leaking the view model via
-        // the cross-process COM event registration.
-        if (IsCleanedUp)
-        {
-            return;
-        }
-
         if (!IsFastInitialized)
         {
             FastInitializeProperties();
@@ -195,15 +184,6 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa
         model.PropChanged += Model_PropChanged;
         Command.PropertyChanged += Command_PropertyChanged;
 
-        // Second guard: handle the narrow race window where SafeCleanup ran
-        // between the check above and the subscriptions just made.
-        if (IsCleanedUp)
-        {
-            model.PropChanged -= Model_PropChanged;
-            Command.PropertyChanged -= Command_PropertyChanged;
-            return;
-        }
-
         UpdateProperty(nameof(Name));
         UpdateProperty(nameof(Title));
         UpdateProperty(nameof(Subtitle));

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ContentPageViewModel.cs

@@ -129,8 +129,8 @@ public partial class ContentPageViewModel : PageViewModel, ICommandBarContext
 
         UpdateDetails();
 
-        FetchContent();
         model.ItemsChanged += Model_ItemsChanged;
+        FetchContent();
 
         DoOnUiThread(
         () =>

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ContentTreeViewModel.cs

@@ -44,9 +44,9 @@ public partial class ContentTreeViewModel(ITreeContent _tree, WeakReference<IPag
             UpdateProperty(nameof(Root));
         }
 
-        FetchContent();
         model.PropChanged += Model_PropChanged;
         model.ItemsChanged += Model_ItemsChanged;
+        FetchContent();
     }
 
     // Theoretically, we should unify this with the one in CommandPalettePageViewModelFactory

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Dock/DockBandViewModel.cs

@@ -209,8 +209,8 @@ public sealed partial class DockBandViewModel : ExtensionObjectViewModel
         var list = command.Model.Unsafe as IListPage;
         if (list is not null)
         {
-            InitializeFromList(list);
             list.ItemsChanged += HandleItemsChanged;
+            InitializeFromList(list);
         }
         else
         {

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionObjectViewModel.cs

@@ -271,22 +271,8 @@ public abstract partial class ExtensionObjectViewModel : ObservableObject, IBatc
         // base doesn't do anything, but sub-classes should override this.
     }
 
-    private volatile bool _isCleanedUp;
-
-    /// <summary>
-    /// Gets a value indicating whether <see cref="SafeCleanup"/> has been called on this object.
-    /// Safe to read from any thread.  Once set, it is never cleared.
-    /// </summary>
-    public bool IsCleanedUp => _isCleanedUp;
-
     public virtual void SafeCleanup()
     {
-        // Set the flag *before* UnsafeCleanup so that any concurrent
-        // InitializeProperties calls on background threads can see it and
-        // bail out before subscribing to COM events that would never be
-        // unsubscribed.
-        _isCleanedUp = true;
-
         try
         {
             UnsafeCleanup();

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListViewModel.cs

@@ -957,8 +957,8 @@ public partial class ListViewModel : PageViewModel, IDisposable
         Filters?.InitializeProperties();
         UpdateProperty(nameof(Filters));
 
-        FetchItems(true);
         model.ItemsChanged += Model_ItemsChanged;
+        FetchItems(true);
     }
 
     private static IGridPropertiesViewModel? LoadGridPropertiesViewModel(IGridProperties? gridProperties)
@@ -1113,15 +1113,6 @@ public partial class ListViewModel : PageViewModel, IDisposable
         CancelAndDisposeTokenSource(ref _fetchItemsCancellationTokenSource);
         CancelAndDisposeTokenSource(ref _selectedItemCts);
 
-        // Unsubscribe the selected-item PropertyChanged handler to prevent
-        // _lastSelectedItem from keeping this ListViewModel alive via the
-        // SelectedItemPropertyChanged delegate.
-        if (_lastSelectedItem is not null)
-        {
-            _lastSelectedItem.PropertyChanged -= SelectedItemPropertyChanged;
-            _lastSelectedItem = null;
-        }
-
         lock (_listLock)
         {
             foreach (var item in Items)

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/PageViewModel.cs

@@ -158,14 +158,6 @@ public partial class PageViewModel : ExtensionObjectViewModel, IPageContext
 
     public override void InitializeProperties()
     {
-        // Guard against a race with UnsafeCleanup on the UI thread.
-        // If this ViewModel has already been cleaned up, do not subscribe to
-        // any COM events as there would be no path to unsubscribe them.
-        if (IsCleanedUp)
-        {
-            return;
-        }
-
         var page = _pageModel.Unsafe;
         if (page is null)
         {
@@ -190,13 +182,6 @@ public partial class PageViewModel : ExtensionObjectViewModel, IPageContext
         UpdateProperty(nameof(HasSearchBox));
 
         page.PropChanged += Model_PropChanged;
-
-        // Second guard: handle the narrow race window where SafeCleanup ran
-        // between the check above and the subscription just made.
-        if (IsCleanedUp)
-        {
-            page.PropChanged -= Model_PropChanged;
-        }
     }
 
     private void Model_PropChanged(object sender, IPropChangedEventArgs args)

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ShellViewModel.cs

@@ -177,13 +177,6 @@ public partial class ShellViewModel : ObservableObject,
                             {
                                 if (cancellationToken.IsCancellationRequested)
                                 {
-                                    // SafeCleanup removes COM event subscriptions that were added during
-                                    // initialization (items PropChanged, page PropChanged, etc.).
-                                    // Without this, items remain alive via cross-process COM event
-                                    // registrations that are never unregistered when navigation is cancelled
-                                    // by a rapid second hotkey press.
-                                    viewModel.SafeCleanup();
-
                                     if (viewModel is IDisposable disposable)
                                     {
                                         try
@@ -201,12 +194,7 @@ public partial class ShellViewModel : ObservableObject,
 
                                 CurrentPage = viewModel;
                             },
-                            // Use CancellationToken.None (not cancellationToken) so that this
-                            // UI-thread task is always scheduled and always runs its body.
-                            // If we passed the already-cancelled token the task would be placed
-                            // in the Cancelled state immediately and the cleanup lambda above
-                            // would never execute, leaking the initialized ViewModel.
-                            CancellationToken.None,
+                            cancellationToken,
                             TaskCreationOptions.None,
                             _scheduler);
                         await t;
@@ -219,12 +207,6 @@ public partial class ShellViewModel : ObservableObject,
         {
             if (cancellationToken.IsCancellationRequested)
             {
-                // SafeCleanup removes COM event subscriptions that were added during
-                // initialization (items PropChanged, page PropChanged, etc.).
-                // Without this, items remain alive via cross-process COM event
-                // registrations that are never unregistered when navigation is cancelled.
-                viewModel.SafeCleanup();
-
                 if (viewModel is IDisposable disposable)
                 {
                     try