test(quickfiler): give the shared UiThread dispatcher a single owner with restore

[drmoisan]

Suggested title

test(quickfiler): give the shared UiThread dispatcher a single owner with restore

Summary

• QfcItemController.TestSupport.EnsureUiThreadDispatcher() mutated the process-wide static UtilitiesCS.Threading.UiThread._dispatcher by reflection, unsynchronized, and never restored it. Its signature was void, so a caller had nothing to dispose and no way to undo the write. Whichever test ran first permanently redefined a static that every later test in the process observes.
• A second, independent gate for the same static had already grown inside QfcItemController.InitializationTests.Part2.cs — a private SemaphoreSlim UiThreadDispatcherGate plus a private SwapUiThreadDispatcher — so the assembly held two competing mutation protocols for one field, one of which was entirely ungated.
• Both are replaced by a single owner, UiThreadDispatcherFixture, implementing a documented two-lock protocol (TransactionGate then FieldLock, never the reverse) and IDisposable scopes whose restores are conditional (ReferenceEquals compare-then-write) and idempotent. EnsureUiThreadDispatcher() now returns IDisposable.
• Six regression tests R1-R6 pin the contract, each [Timeout(60000)] so a regression fails rather than hangs.
• Zero production files changed. All five build-relevant files are in the QuickFiler.Test project.
• Full toolchain green on the post-merge tree, non-vacuously: 6707 tests, 6707 passed, 0 failed, and zero occurrences of Skipping target "CoreCompile" across both /t:Rebuild gates.

Why

Issue #493 reports that a test helper swaps a process-wide static and never puts it back. That is a test-isolation defect of the kind the repository's unit-test policy names directly under Environment Stability: a test must not rely on, or leave behind, mutable global state.

Root causes, confirmed by reading the pre-change code rather than inferred:

• RC-1 — no restore was possible. EnsureUiThreadDispatcher() was declared internal static void. It resolved UiThread._dispatcher, and if the value was null installed a parked dispatcher. There is no return value, so no caller could ever restore the previous value; the write is permanent for the life of the process. The steady-state value of a shared static therefore depended on test execution order.
• RC-2 — the read-modify-write was unsynchronized. The null check and the SetValue were separate operations with no lock between them, so two concurrent callers could both observe null and both install, with one silently losing.
• RC-3 — a second protocol had grown for the same field. InitializationTests.Part2.cs protected its own long install-test-restore sequences with a private semaphore and its own reflection swap. That gate could not exclude EnsureUiThreadDispatcher, because EnsureUiThreadDispatcher did not know the gate existed. Two protocols for one field is not defense in depth; it is a lost-update hazard, and it is the shape that produced the Build a WinForms message-pump test seam (Application.Run() background thread) to unblock 9 QfcItemController orchestration members #230 deadlock workaround this change subsumes.

What Changed

QuickFiler.Test/Controllers/QfcItemController.UiThreadDispatcherFixture.cs (new, 278 lines)

• UiThreadDispatcherFixture — the sole mutator of UiThread._dispatcher in the assembly's owned files. Current, Exchange, and CompareExchange each hold FieldLock for the whole read-modify-write. EnsureDispatcher() installs only into a null field and returns an EnsureScope.
• BeginTransactionAsync() acquires TransactionGate (a SemaphoreSlim(1,1)) for callers that need to hold a value across a whole test body.
• UiThreadDispatcherTransaction — one-shot Install that fails fast on a second call, and a Dispose that restores conditionally and then releases the gate, so a waiter can never observe the pre-restore value.
• EnsureDispatcher deliberately does not take TransactionGate. That is what keeps callers which are not [Timeout]-bounded from being able to hang.

QuickFiler.Test/Controllers/QfcItemController.UiThreadDispatcherFixtureTests.cs (new, 346 lines)

ID	Test	Contract clause pinned
R1	EnsureDispatcher_WhileATransactionHoldsALiveDispatcher_DoesNotReplaceIt	Ensure installs only into a null field; the #230 clobber precondition is unreachable
R2	EnsureDispatcher_WhenTheFieldIsNull_InstallsAndRestoresOnDispose	Seed, then conditional restore to null
R3	EnsureDispatcher_ScopeDisposedTwice_IsIdempotent	Second dispose neither rewrites nor throws
R4	Transaction_SecondCallerCannotInstallUntilTheFirstRestores	Restore strictly precedes gate release
R5	Transaction_DisposedTwice_DoesNotOverReleaseTheGate	No SemaphoreFullException
R6	Install_CalledTwiceOnTheSameTransaction_ThrowsInvalidOperationException	One-shot install fails fast

QuickFiler.Test/Controllers/QfcItemController.TestSupport.cs (440 lines, +10 / -59)

• EnsureUiThreadDispatcher() changes from void to IDisposable and becomes a one-line delegation to the fixture. This signature change is the fix: it is what makes a restore expressible at all.
• The private parked-dispatcher machinery moves into the fixture rather than being duplicated.

QuickFiler.Test/Controllers/QfcItemController.InitializationTests.Part2.cs (393 lines, +22 / -47)

• The private UiThreadDispatcherGate and SwapUiThreadDispatcher are deleted. BuildPumpHarnessAsync and PumpHarness consume the shared transaction instead.
• The load-bearing properties of the old pump gate are preserved exactly: permit count stays 1, the hold window still opens at BuildPumpHarnessAsync entry and closes in PumpHarness.Restore(), and Restore() stays idempotent via its _restored guard.
• The catch path is strictly improved. If harness construction throws after Install, transaction.Dispose() now also restores the static; the old Release()-only path did not.

QuickFiler.Test/QuickFiler.Test.csproj (+2)

• Two <Compile Include> entries for the new files. See Risks for the disclosed region deviation.

Architecture / How It Fits Together

The fixture establishes one lock order and never varies it:

BeginTransactionAsync  ->  TransactionGate   (held across a test body)
                              |
                              v
Current / Exchange / CompareExchange / EnsureDispatcher  ->  FieldLock  (held only for a read-modify-write)

No path takes FieldLock and then TransactionGate, so no cycle exists. No monitor lock is held across an await. The parked-dispatcher creation, which does wait on a ManualResetEventSlim, is performed before FieldLock is taken, so the only wait inside a lock region is on a thread that itself takes no locks before signalling.

Two ownership levels exist deliberately. A short seed-if-empty (EnsureDispatcher) is gate-free and cheap; a long install-run-restore sequence takes the gate. The asymmetry is the point: it means a caller that forgot a timeout cannot deadlock the suite.

Verification

All four gates were re-run after merging the current integration tip, because this branch was 11 commits behind when the work resumed and prior green was recorded against a base that no longer exists.

Step	Command	Result
1 Format	dotnet tool run csharpier format . then check .	exit 0, 1542 files, no file rewritten
2 Analyzers	msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true	exit 0, 5 warnings / 0 errors
3 Nullable	msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true	exit 0, 5 warnings / 0 errors
4 Test	vstest.console.exe <9 assemblies> /EnableCodeCoverage /InIsolation /TestCaseFilter:TestCategory!=LiveOutlook	exit 0, 6707 passed, 0 failed

The 5 warnings are the pre-existing System.Reactive.PackagesConfigCheck.targets packages.config notices, identical in count to the Phase 0 baseline. Zero error CS and zero warning CS lines appear in either build log.

Non-vacuity. /t:Rebuild was used, never /t:Build: a warm /t:Build returns exit 0 having skipped CoreCompile on every project, so it cannot fail. Measured against the 3.3 MB structured logs of both build steps:

• Skipping target "CoreCompile" occurrences: 0
• csc.exe invocations: 36
• The only skipped targets are 18 GenerateTargetFrameworkMonikerAttribute and 9 CopyMSTestV2Resources, neither of which suppresses compilation or analyzers.
• Both new files appear in the QuickFiler.Test compile inputs, and the output assembly carries a post-build mtime.

Test-count arithmetic. Sibling feature 442 reported 6701 at its merge (PR #649). This branch adds six tests and removes none, so 6707 is the exact expected total. Observed: 6707, with 6707 Passed lines counted independently, hence zero skipped and zero failed. The whole suite was run, not just the changed assembly, so that a regression introduced by the 11 merged base commits could not hide.

Nullable command fidelity. /p:Nullable=enable is deliberately not added. Nullable enforcement here is per-file opt-in via the #nullable enable pragma; forcing the property solution-wide conscripts files that never adopted it and is not what CI runs.

Coverage. This change adds zero production lines; all 624 added lines are in a test assembly, which coverage tooling excludes from the instrumented denominator by policy. The coverage delta attributable to this branch is therefore exactly zero, which the Phase 0 versus final comparison established with a byte-identical whole-repository Cobertura triple. No repository-wide coverage XML is emitted, because doing so would publish a pre-existing sub-floor whole-repo figure that a test-only branch neither caused nor can remediate.

Independent re-verification of the acceptance criteria (not taken on trust from the prior run):

• AC-6 — QfcItemController.FocusAndThemeTests.cs is byte-identical to the current base: git hash-object returns 77c4e709… for both. Still 497 lines. Both named theme tests pass.
• AC-7 — UtilitiesCS/Threading/UiThread.cs is byte-identical to the current base: git hash-object returns 8663db03… for both. No production assembly changed.
• AC-8 — measured line counts: TestSupport.cs 440, InitializationTests.Part2.cs 393, UiThreadDispatcherFixture.cs 278, UiThreadDispatcherFixtureTests.cs 346. All at or under 500.

Backward Compatibility / Migration Notes

EnsureUiThreadDispatcher() changes return type from void to IDisposable. This is a source-breaking change to an internal test helper, contained entirely within QuickFiler.Test. Both existing call sites are in QfcItemController.FocusAndThemeTests.cs, which is unmodified and still compiles: a void-discarding call statement remains valid when the method returns a value.

The pump harness's public surface is byte-compatible. BuildPumpHarnessAsync(host, darkMode) and harness.Restore() are unchanged, so the nine consuming tests across InitializationTests.Part3.cs and SeamFactoryTests.cs needed no edit and were not edited.

Sibling features see an unchanged gate protocol, an unchanged permit count, and an unchanged steady-state field value.

Risks and Mitigations

• Project-file region deviation, disclosed. The epic checkpoint's csproj_region_partition records feature 493 as owning region none, on the evidence that its plan declared no <Compile Include> entry. Two entries were in fact required: QuickFiler.Test.csproj is a legacy non-SDK project, so without them the two new files are not compiled and all six regression tests silently vanish. They were placed at lines 158-159, inside the contiguous QfcItemController.* family block, roughly thirty lines clear of feature 444's QfcCollectionController* entries at lines 122-128. Features 501 and 476 own Viewers\* paths outside this item group's range. No sibling insertion point overlaps. Related correction: the partition describes both item groups as alphabetically ordered, which is not accurate for this file — it is grouped by class family and is not sorted, so placement followed the file's actual convention.
• Exception-safety hardening remains open (non-blocking). PumpHarness.Restore() calls TokenSource.Dispose() before _transaction.Dispose(), and UiThreadDispatcherTransaction.Dispose() calls CompareExchange before ReleaseTransactionGate(), neither inside a try/finally. A throw from the earlier call would leak the gate. Both earlier calls are non-throwing in practice (CancellationTokenSource.Dispose, FieldInfo.SetValue on a resolved static field) and every consumer is [Timeout]-bounded, so the exposure is theoretical. Recommended as follow-up polish.
• R2/R3 assert absolute field values. They are airtight against other transaction holders but not against a concurrent unowned EnsureDispatcher caller, because EnsureDispatcher deliberately stays off TransactionGate. The window is sub-millisecond. Recorded so that a future rare flake in R2 or R3 is diagnosed as this accepted design consequence rather than as a fixture defect.
• An EnsureScope disposed while a transaction's value occupies the field skips its restore permanently. This is the accepted cost of keeping EnsureDispatcher gate-free, and it leaves exposure unchanged from the pre-fix state rather than worsening it.
• Evidence timestamps were synthesized rather than captured (disclosed, non-blocking). The plan defines its TS convention as a timestamp captured per task. From roughly P0-T4 onward, artifact names and Timestamp: fields drift progressively ahead of every machine-generated time source — about 2 to 5 minutes per task, reaching roughly 90 minutes by Phase 5 — with spacing uniform enough to indicate a counter was incremented instead of the clock being read. Concretely, evidence/qa-gates/commit-2.2026-08-27T12-17.md declares Timestamp: 2026-08-27T12-17, but the commit that introduced it was authored 10:46:29 -0400: 90 minutes in the future read as local time, or 90 minutes before the run's first artifact read as UTC. Raw log mtimes, TRX-embedded stamps, and git author dates all agree with each other and contradict the artifact stamps. No acceptance criterion asserts timestamp provenance, and every criterion gates on content, hashes, counts, exit codes, or test results, all of which were re-measured against ground truth during this resume and verified. The artifacts are deliberately not renamed, because doing so would destroy the citation graph already embedded in the plan, the three review artifacts, and the commit messages while substituting one set of unverifiable stamps for another. Recorded for upstream executor tooling rather than remediated here. Artifacts written during this resume were each produced from a date -u read taken immediately before the write.
• One plan gate failed its literal text and the task was still checked off. Plan task P4-T2 required byte-exact set equality of msbuild-log lines containing the token QfcItemController.FocusAndThemeTests.cs. Equality did not hold, and the gate was structurally unsatisfiable as written: at default verbosity every matching line is a csc.exe invocation enumerating the project's whole source set, so any plan that adds a compile item — as this one mandates — breaks equality by construction. Deleting exactly the two added path tokens from the final extracts makes them byte-identical to the baselines, and the diagnostic-bearing subset is empty on both sides. The criterion AC-6 actually asserts is byte-identity of the file plus absence of diagnostics, both independently verified above by other means. Disclosed here rather than buried; the guidance recorded for future plans is not to gate on raw compiler-invocation text.

Review Guide

Read in this order:

1. QfcItemController.UiThreadDispatcherFixture.cs — the lock-ordering comment block first, then EnsureDispatcher and UiThreadDispatcherTransaction.Dispose. The restore-before-release ordering inside Dispose is the load-bearing invariant.
2. The TestSupport.cs diff — the void to IDisposable signature change is the whole fix in one line.
3. The InitializationTests.Part2.cs diff — confirm the old gate is deleted, not merely bypassed, and that the hold window is unchanged.
4. QfcItemController.UiThreadDispatcherFixtureTests.cs — R1 and R4 carry the concurrency claims; R4's doc comment states honestly that it is probabilistic under a broken implementation and why a deterministic version would need a forbidden timed wait.

Follow-ups

• Bug: wpfuidispatchertests-ungated-static-swap #648 (OPEN) — QuickFiler.Test/Controllers/WpfUiDispatcherTests.cs:42-51 still swaps the same static by raw reflection outside both locks. It does restore in a finally, so it is a latent ordering hazard rather than a recurrence of the no-restore defect. Out of scope here by spec; promoted during this work.
• Bug: uithread-dispatcher-null-race-progresstrackerasync #584 (OPEN) — related dispatcher null-race in ProgressTrackerAsync.
• Cross-assembly mutators in UtilitiesCS.Test remain out of reach of any lock inside QuickFiler.Test; recorded as residual risk R-2.
• try/finally hardening of the two restore paths described under Risks.

GitHub Auto-close

None.

This pull request targets epic/quickfiler-bug-family-integration, not the default branch. GitHub registers closing references only for pull requests targeting the default branch, so merging this cannot close an issue and no Closes bullet is emitted. Issues #493, #648 and #584 were each verified OPEN with gh issue view immediately before this pull request was opened, and are referenced by this work without being closed by it. Issue #493 is closed by the epic's integration-to-main pull request, not by this one.