refactor(782): consolidate the PR #778 post-merge review residuals into one delivery

QuickFiler.Test/Controllers/QfcItemController.InitializationTests.Part2.cs

@@ -121,10 +121,14 @@ UiThreadDispatcherTransaction transaction
             // QfcTipsDetails.ToggleAsync marshals through the process-wide static
             // UtilitiesCS.UiThread.Dispatcher. In production that is the live UI thread's
             // dispatcher; in this assembly it is either unset or the deliberately parked instance
-            // from QfcItemControllerTestSupport.EnsureUiThreadDispatcher, neither of which can
-            // complete an InvokeAsync. Point it at the pump thread's dispatcher (serviced by the
-            // WinForms loop, proven by WinFormsPumpHostTests.BothMarshalRoutes_*) for the duration
-            // of the test, and restore the previous value in PumpHarness.Restore so no state leaks.
+            // from QfcItemControllerTestSupport.EnsureUiThreadDispatcher. Neither case can carry
+            // the marshalled work to completion, but they fail differently: when unset, the
+            // accessor throws InvalidOperationException before any marshalling is attempted, so
+            // InvokeAsync is never reached; when parked, it is a real dispatcher that is never
+            // pumped, so the InvokeAsync is accepted and then never runs. Point it at the pump
+            // thread's dispatcher (serviced by the WinForms loop, proven by
+            // WinFormsPumpHostTests.BothMarshalRoutes_*) for the duration of the test, and restore
+            // the previous value in PumpHarness.Restore so no state leaks.
             transaction.Install(viewer.UiDispatcher);
 
             return new PumpHarness(controller, viewer, cts, webView, transaction);

QuickFiler.Test/Helper Classes/EmailMoveMonitorTests.cs

@@ -2,10 +2,12 @@
 using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
+using System.Windows.Threading;
 using FluentAssertions;
 using Microsoft.Office.Interop.Outlook;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Moq;
+using QuickFiler.Controllers.Tests;
 using QuickFiler.Helper_Classes;
 using QuickFiler.Interfaces;
 using UtilitiesCS;
@@ -25,22 +27,17 @@ public class EmailMoveMonitorTests
         // UiThread.Dispatcher is process-global, set-once static state. These tests never invoke
         // the default (production) marshal delegate, so they do not depend on UiThread being
         // initialized. To guarantee order-independence even if a future change touches the static
-        // path, the setup/teardown below snapshots the static dispatcher field via reflection
-        // (avoiding a compile-time WindowsBase dependency on System.Windows.Threading.Dispatcher)
-        // and asserts it is unchanged after each test. The class is not parallelized because other
-        // QuickFiler tests intentionally replace UiThread.Dispatcher with dedicated WPF dispatchers.
+        // path, the setup/teardown below snapshots the static dispatcher field through the shared
+        // QuickFiler.Test dispatcher fixture and asserts it is unchanged after each test. The
+        // class is not parallelized because other QuickFiler tests intentionally replace
+        // UiThread.Dispatcher with dedicated WPF dispatchers.
         //
         // The snapshot reads the private _dispatcher backing field rather than the public
         // Dispatcher property (issue #584): the property getter now throws
         // InvalidOperationException when the field is null, and PropertyInfo.GetValue would
         // surface that as a TargetInvocationException from this class's setup and teardown.
         // Reading the field observes the same state without invoking the guard.
-        private object _capturedDispatcher;
-        private static readonly System.Reflection.FieldInfo DispatcherField =
-            typeof(UiThread).GetField(
-                "_dispatcher",
-                System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static
-            );
+        private Dispatcher _capturedDispatcher;
 
         /// <summary>
         /// Counts how many times the injected marshal delegate was invoked.
@@ -50,9 +47,9 @@ public class EmailMoveMonitorTests
         [TestInitialize]
         public void Setup()
         {
-            // Snapshot the static UiThread.Dispatcher (reflectively, to avoid WindowsBase) so
-            // teardown can confirm no test mutated this set-once static state.
-            _capturedDispatcher = DispatcherField?.GetValue(null);
+            // Snapshot the static UiThread.Dispatcher through the fixture accessor so teardown
+            // can confirm no test mutated this set-once static state.
+            _capturedDispatcher = UiThreadDispatcherFixture.Current;
             _marshalInvocationCount = 0;
         }
 
@@ -61,7 +58,7 @@ public void Cleanup()
         {
             // Assert the static dispatcher snapshot is unchanged so any accidental static mutation
             // is caught and tests remain order-independent.
-            object current = DispatcherField?.GetValue(null);
+            Dispatcher current = UiThreadDispatcherFixture.Current;
             current.Should().BeSameAs(_capturedDispatcher);
         }
 

TaskMaster/Ribbon/RibbonViewer.EngineCommands.cs

@@ -69,7 +69,7 @@ internal void InvalidateEngineCommands()
             }
 
             var dispatcher = UiThread.Dispatcher;
-            if (dispatcher != null && !dispatcher.CheckAccess())
+            if (!dispatcher.CheckAccess())
             {
                 dispatcher.Invoke(() =>
                     EngineCommandRefreshPlanner.InvalidateAll(ribbon.InvalidateControl)
@@ -112,7 +112,7 @@ internal void InvalidateEngineToggle(string controlId)
             }
 
             var dispatcher = UiThread.Dispatcher;
-            if (dispatcher != null && !dispatcher.CheckAccess())
+            if (!dispatcher.CheckAccess())
             {
                 dispatcher.Invoke(() => ribbon.InvalidateControl(controlId));
                 return;

UtilitiesCS.Test/OutlookObjects/Folder/WpfDispatcherYieldTests.cs

@@ -10,6 +10,7 @@
 namespace UtilitiesCS.Test.OutlookObjects.Folder
 {
     [TestClass]
+    [DoNotParallelize]
     public sealed class WpfDispatcherYieldTests
     {
         [TestMethod]
@@ -119,7 +120,7 @@ public async Task YieldAsync_WithoutDispatcher_RemainsStrict()
         {
             // Arrange: the dispatcher-free precondition is arranged explicitly. Both lookups return
             // null, so the outcome cannot depend on which pooled thread this test runs on, on test
-            // execution order, or on whether UiThread.Initialize() ran earlier in the process.
+            // execution order, or on whether UiThread.Init() ran earlier in the process.
             var threadProvider = new CountingDispatcherProvider(null);
             var fallbackProvider = new CountingDispatcherProvider(null);
             var dispatcherYield = new WpfDispatcherYield(
@@ -131,7 +132,8 @@ public async Task YieldAsync_WithoutDispatcher_RemainsStrict()
             await dispatcherYield
                 .Invoking(item => item.YieldAsync(CancellationToken.None))
                 .Should()
-                .ThrowAsync<InvalidOperationException>();
+                .ThrowAsync<InvalidOperationException>()
+                .WithMessage(UiThread.DispatcherNotInitializedMessage);
 
             threadProvider
                 .InvocationCount.Should()
@@ -141,6 +143,59 @@ await dispatcherYield
                 .Be(1, "the fallback is tried before the strict contract is enforced");
         }
 
+        /// <summary>
+        /// Pins the production resolution path rather than an injected one. The yielder is built
+        /// through its public parameterless constructor, so its fallback lookup is the real
+        /// process-global accessor; the process-global value is uninstalled for the duration of
+        /// the Act, so that accessor is exercised in its uncaptured state and must surface the
+        /// shared guard message naming the public initialization entry point.
+        ///
+        /// The Act runs on a dedicated fresh thread rather than on the MSTest worker. On a pooled
+        /// worker, <c>Dispatcher.FromThread</c> returns a non-null instance if any earlier test on
+        /// that same thread ever resolved the thread's dispatcher; the thread-affinitized provider
+        /// would then win and the fallback under test would never run. The class-level
+        /// <c>[DoNotParallelize]</c> serializes the write to the process-global static but cannot
+        /// supply thread freshness, so both are required.
+        /// </summary>
+        [TestMethod]
+        public void YieldAsync_ProductionFallbackWithoutDispatcher_ThrowsNamingInit()
+        {
+            // Arrange
+            var dispatcherYield = new WpfDispatcherYield();
+            Exception? observed = null;
+
+            using (UiThreadDispatcherScope.InstallNull())
+            {
+                var worker = new Thread(() =>
+                {
+                    try
+                    {
+                        dispatcherYield.YieldAsync(CancellationToken.None).GetAwaiter().GetResult();
+                    }
+                    catch (Exception ex)
+                    {
+                        observed = ex;
+                    }
+                });
+                worker.IsBackground = true;
+
+                // Act: the worker is joined inside the scope so the uninstalled state is still in
+                // force for the whole of its run.
+                worker.Start();
+                worker.Join();
+            }
+
+            // Assert. A null capture means the Act completed without throwing, which the type
+            // assertion reports directly, so the null-forgiving operator here loses no diagnostic.
+            Exception observedException = observed!;
+            observedException
+                .Should()
+                .BeOfType<InvalidOperationException>(
+                    "the production fallback must surface the uncaptured-dispatcher guard"
+                );
+            observedException.Message.Should().Contain("UiThread.Init()");
+        }
+
         /// <summary>
         /// Records how many times the seam consulted a dispatcher lookup and what that lookup
         /// returned, so tests can pin the resolution order rather than only the outcome.

UtilitiesCS.Test/TestHelpers/UiThreadDispatcherScope.cs

@@ -0,0 +1,126 @@
+using System;
+using System.Reflection;
+using System.Windows.Threading;
+using FluentAssertions;
+using UtilitiesCS;
+
+namespace UtilitiesCS.Test
+{
+    /// <summary>
+    /// Installs a replacement value into the private static <c>UiThread._dispatcher</c> backing
+    /// field for the lifetime of a <c>using</c> statement, and restores the prior value on
+    /// disposal.
+    ///
+    /// Reflection is still required because <c>InternalsVisibleTo</c> exposes internal members
+    /// only; it does not expose private ones, and the backing field is private. Centralising the
+    /// reflection here means the field name appears in exactly one place in this assembly rather
+    /// than at each test that needs to control the dispatcher.
+    /// </summary>
+    /// <remarks>
+    /// This type is deliberately <b>not</b> internally synchronized. It performs an unguarded
+    /// read-then-write against a process-global static, so two tests installing concurrently would
+    /// interleave and one would restore a value the other had already replaced. Serialization of
+    /// writers is provided instead by <c>[DoNotParallelize]</c> on every test class that installs a
+    /// value through this scope. A future caller must not assume this type is thread-safe: adding a
+    /// new installing test class requires adding that attribute to the class as well.
+    ///
+    /// The scope is reachable only from <c>UtilitiesCS.Test</c>. <c>QuickFiler.Test</c> is a
+    /// separate assembly and is not named in the <c>InternalsVisibleTo</c> grants on
+    /// <c>UtilitiesCS</c>, so it uses its own fixture accessor rather than this type.
+    /// </remarks>
+#nullable enable annotations
+    internal sealed class UiThreadDispatcherScope : IDisposable
+    {
+        /// <summary>
+        /// The private static backing field of <c>UiThread.Dispatcher</c>, resolved once.
+        /// </summary>
+        /// <remarks>
+        /// Resolution happens in the static initializer and asserts the field is non-null with a
+        /// stated reason, mirroring the <c>ResolveDispatcherField</c> idiom in
+        /// <c>QuickFiler.Test/Controllers/QfcItemController.UiThreadDispatcherFixture.cs</c>. The
+        /// assertion is what makes a rename of <c>_dispatcher</c> fail loudly: it raises
+        /// <see cref="TypeInitializationException"/> on first use and fails every consuming test,
+        /// rather than degrading to a silent no-op that installs nothing and still passes.
+        /// </remarks>
+        private static readonly FieldInfo DispatcherField = ResolveDispatcherField();
+
+        private Dispatcher? _prior;
+        private bool _disposed;
+
+        private UiThreadDispatcherScope(Dispatcher? prior)
+        {
+            _prior = prior;
+        }
+
+        /// <summary>
+        /// Reads the current value of the backing field directly, without going through the
+        /// <c>UiThread.Dispatcher</c> property.
+        /// </summary>
+        /// <remarks>
+        /// The property throws <see cref="InvalidOperationException"/> when the field is null, so a
+        /// test that needs to observe the uninitialized state — for example to assert that a scope
+        /// restored a null prior — cannot use the property to do it.
+        /// </remarks>
+        internal static Dispatcher? Current => (Dispatcher?)DispatcherField.GetValue(null);
+
+        /// <summary>
+        /// Captures the prior field value, writes <paramref name="replacement"/> in its place, and
+        /// returns a scope that restores the captured value when disposed.
+        /// </summary>
+        /// <param name="replacement">
+        /// The value to install. May be null, which is how a test reproduces the state in which
+        /// <c>UiThread.Init()</c> has never run.
+        /// </param>
+        /// <returns>A scope whose disposal restores the captured prior value.</returns>
+        internal static UiThreadDispatcherScope Install(Dispatcher? replacement)
+        {
+            var prior = (Dispatcher?)DispatcherField.GetValue(null);
+            DispatcherField.SetValue(null, replacement);
+            return new UiThreadDispatcherScope(prior);
+        }
+
+        /// <summary>
+        /// Convenience for <c>Install(null)</c>: installs the uninitialized state in which reading
+        /// <c>UiThread.Dispatcher</c> throws <see cref="InvalidOperationException"/>.
+        /// </summary>
+        /// <returns>A scope whose disposal restores the captured prior value.</returns>
+        internal static UiThreadDispatcherScope InstallNull()
+        {
+            return Install(null);
+        }
+
+        /// <summary>
+        /// Restores the value captured at construction, including when that value was null.
+        /// </summary>
+        /// <remarks>
+        /// The captured prior is written back unconditionally. It is never tested for null first:
+        /// a null prior is a real state that must be restored, and skipping the write for it would
+        /// leak an installed dispatcher into every later test on the same process-global static.
+        /// A second call is a no-op, so the scope is safe inside a <c>using</c> statement that also
+        /// disposes explicitly.
+        /// </remarks>
+        public void Dispose()
+        {
+            if (_disposed)
+            {
+                return;
+            }
+
+            DispatcherField.SetValue(null, _prior);
+            _prior = null;
+            _disposed = true;
+        }
+
+        private static FieldInfo ResolveDispatcherField()
+        {
+            FieldInfo field = typeof(UiThread).GetField(
+                "_dispatcher",
+                BindingFlags.NonPublic | BindingFlags.Static
+            );
+            field.Should().NotBeNull(because: "UiThread._dispatcher backing field must exist");
+            return field;
+        }
+    }
+
+#nullable restore annotations
+}

UtilitiesCS.Test/Threading/IdleActionQueue_Tests.cs

@@ -22,8 +22,45 @@ namespace UtilitiesCS.Test.Threading
     ///     Each test calls ResetStaticState() to ensure isolation.
     /// </summary>
     [TestClass]
+    [DoNotParallelize]
     public class IdleActionQueue_Tests
     {
+        /// <summary>
+        /// Restores the process-global state this class mutates, after every test.
+        ///
+        /// Purpose:
+        ///     Draining the queue, replacing the subscribe guard, and cancelling the pending
+        ///     unsubscribe timer are delegated to the existing private helper rather than
+        ///     duplicated here. Detaching the idle handler is the additional step: the queue
+        ///     subscribes on first use and never unsubscribes until its delayed batch action
+        ///     fires, so without this a spent handler outlives the test that registered it.
+        ///
+        /// Side Effects:
+        ///     Detaching the last handler makes ApplicationIdleTimer stop its timer, which
+        ///     touches process-global System.Windows.Forms.Application.Idle state shared with
+        ///     IdleAsyncQueue_Tests and ApplicationIdleTimer_Tests. That shared reach is why the
+        ///     class carries [DoNotParallelize].
+        /// </summary>
+        [TestCleanup]
+        public void Cleanup()
+        {
+            ResetStaticState();
+
+            // The production handler is private, so the delegate instance that the queue
+            // registered cannot be named here. Rebuilding a delegate over the same static method
+            // yields an equal delegate, and delegate removal matches on equality rather than on
+            // reference identity.
+            var handler = (ApplicationIdleTimer.ApplicationIdleEventHandler)
+                Delegate.CreateDelegate(
+                    typeof(ApplicationIdleTimer.ApplicationIdleEventHandler),
+                    typeof(IdleActionQueue).GetMethod(
+                        "OnApplicationIdle",
+                        BindingFlags.NonPublic | BindingFlags.Static
+                    )
+                );
+            ApplicationIdleTimer.Unsubscribe(handler);
+        }
+
         #region Helpers
 
         /// <summary>