Skip to content

fix(quickfiler-test): route WpfUiDispatcherTests static swap through the shared fixture - #719

Merged
drmoisan merged 18 commits into
mainfrom
bug/wpfuidispatchertests-ungated-static-swap-648
Sep 1, 2026
Merged

fix(quickfiler-test): route WpfUiDispatcherTests static swap through the shared fixture#719
drmoisan merged 18 commits into
mainfrom
bug/wpfuidispatchertests-ungated-static-swap-648

Conversation

@drmoisan

@drmoisan drmoisan commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • Routes the process-wide UtilitiesCS.Threading.UiThread._dispatcher swap in QuickFiler.Test/Controllers/WpfUiDispatcherTests.cs through the shared UiThreadDispatcherFixture transaction introduced by Bug: uithread-dispatcher-static-swap-no-restore #493, removing the last ungated mutator of that static inside QuickFiler.Test.
  • Replaces raw reflection (GetField / SetValue) and an unconditional finally restore with UiThreadDispatcherFixture.BeginTransactionAsync(), transaction.Install(dispatcher), and transaction.Dispose(), which restores by ReferenceEquals compare-then-write and only then releases the gate.
  • Behavior is unchanged: Invoke, InvokeAsync, and BeginInvoke are still each asserted to execute their delegate on the dispatcher's own thread, and Construction_YieldsAnIUiDispatcher is untouched.
  • Exactly one .cs file changes. No production source, no UtilitiesCS/ path, and no UtilitiesCS.Test/ path is modified.
  • Feature review returned PASS with zero blocking findings; all seven acceptance criteria were independently re-verified.

Why

WpfUiDispatcherTests.cs predates the shared fixture, so it had no gated path available when it was written. After #493 landed, every other mutation of that static from this assembly's owned files holds FieldLock for the whole read-modify-write, leaving this one call site as the remaining ungated writer.

Unlike the originating #493 defect, this site did restore the previous value, so it was a lesser and distinct concern rather than a recurrence of the no-restore bug. What it lacked was participation in the lock protocol: it never acquired UiThreadDispatcherFixture.FieldLock, so its read-modify-write could interleave with a fixture transaction, and its restore was an unconditional write rather than the fixture's conditional compare-then-write.

This was recorded as accepted residual risk R-1 of #493, whose Rollout & Follow-up item 3 asked that it be promoted as its own small issue once the shared fixture existed.

What Changed

Test source (1 file)

  • QuickFiler.Test/Controllers/WpfUiDispatcherTests.cs — reflection block and unconditional restore removed; using System.Reflection; and using UtilitiesCS; dropped; the method is now async Task with [Timeout(GateTimeoutMs)] and a private const int GateTimeoutMs = 60000; class field matching the sibling Bug: uithread-dispatcher-static-swap-no-restore #493 regression tests; nested try/finally so transaction.Dispose() runs before QfcItemControllerTestSupport.ShutdownDispatcher(dispatcher).

Evidence and audits (50 files)

  • Phase 0 baseline, Phase 1 regression-testing, and Phase 2 QA-gate artifacts for every plan task, plus the fail-before exception dossier and the three feature-review audit artifacts.

Agent memory (17 files)

  • Lessons recorded during preparation, execution, and review.

Architecture / How It Fits Together

UiThreadDispatcherFixture (QuickFiler.Test/Controllers/QfcItemController.UiThreadDispatcherFixture.cs) is the single owner of every mutation of UiThread._dispatcher made from this assembly's owned files, and #493's AC-4 gates that uniqueness. It guards two concerns with two locks: TransactionGate serialises whole transactions, and FieldLock makes each read-modify-write of the static atomic. Lock order is TransactionGate then FieldLock, never the reverse, so no cycle exists.

The test now acquires the gate, installs the running dispatcher through the returned UiThreadDispatcherTransaction, and disposes it to restore. No second reflection lookup is introduced.

Verification

Completed

  • CSharpier check across the tree: exit 0, 1566 files, no drift.
  • Analyzer rebuild (/t:Rebuild, EnableNETAnalyzers, EnforceCodeStyleInBuild): exit 0, 0 Error(s), warning count equal to the Phase 0 baseline.
  • Nullable rebuild (/t:Rebuild, TreatWarningsAsErrors): exit 0, 0 Error(s), warning count equal to baseline.
  • Scoped run of WpfUiDispatcherTests: 2/2 passing.
  • Full QuickFiler.Test.dll under the repo runsettings (Scope=ClassLevel, Workers=0): 1285/1285 passing, no regression against the 1285 baseline.
  • Coverage suite: 6925/6925 passing. Repo-wide first-party coverage 85.373% line (54964/64381) and 79.694% branch (23215/29130), both above the 85%/75% floors.
  • Feature review re-ran the five dispatcher-touching classes together at ClassLevel with 24 workers: 47/47 passing.

Recommended

  • dotnet tool run csharpier check .
  • msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true
  • msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true
  • vstest.console.exe QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /Settings:scripts\vscode\TaskMaster.cli.runsettings /InIsolation /TestCaseFilter:"TestCategory!=LiveOutlook"

A green run under class-level parallelization does not prove the race is eliminated; it shows only that the gated path is stable under that scope. The defect was a latent ordering hazard, dormant under CI settings, so no deterministic red run was available. A fail-before exception dossier records why and names the substitute evidence.

Backward Compatibility / Migration Notes

None. The change is confined to one test method body. No public API, no production code, and no project file is altered.

Risks and Mitigations

Review Guide

  1. QuickFiler.Test/Controllers/WpfUiDispatcherTests.cs — the entire functional change.
  2. docs/features/active/2026-08-27-wpfuidispatchertests-ungated-static-swap-648/code-review.2026-09-01T14-06.md — including the assessment of the one deviation from the plan.
  3. The remaining evidence files are mechanical gate records.

One deviation from the approved plan, accepted on review. Plan task P1-T2 directed the gate acquisition as a single expression, await UiThreadDispatcherFixture.BeginTransactionAsync().ConfigureAwait(false). At the 16-column indent that line exceeds CSharpier's 100-column default, and CSharpier reprints it as a three-line member chain, which would have made P1-T6's single-line token assertion unsatisfiable for any executor who followed P1-T2 literally. The reviewer reproduced this against the manifest-pinned CSharpier 1.2.6 rather than accepting the claim. The delivered form binds the task to a local first; it is the same invocation, the same task, and the same ConfigureAwait(false).

Follow-ups

Recorded here rather than opened as issues, because acceptance criterion AC-6 restricts this branch's diff to a named path set and a promotion would breach it. They are reported separately for consolidated filing.

  • Two plan-authoring defects. P1-T2 and P1-T6 were mutually unsatisfiable as authored (above). Separately, P2-T3's condition "no diagnostic names Controllers\WpfUiDispatcherTests.cs" matches the csc.exe command line and its response-file echo on a clean build, so an unrestricted log search returns false hits and the condition is unsatisfiable unless first filtered to diagnostic lines. Reachability of both: certain.
  • Raw Cobertura remains reachable in branch history. Two ~10.6 MB reports were committed before being converted to package-level JaCoCo projections, so the working tree no longer carries them but the blobs stay reachable. Squash-merging this PR drops both with no history rewrite. The projections reconcile exactly to the source root counters. Note this is a pre-existing repository-wide pattern: origin/main currently carries 281 .cobertura.xml files.
  • Evidence Timestamp: fields are synthetic. Several artifacts carry stamps inconsistent with their own quoted build banners and commit dates. Review confirmed the gates did in fact run, via recorded elapsed times and artifact mtimes; the stamps simply cannot establish inter-gate ordering.
  • .globalconfig is referenced twice in CLAUDE.md but does not exist. .editorconfig is the actual analyzer severity source. Documentation-only.

PR Context Bundle Errors

The generated context bundle at artifacts/pr_context.summary.txt is unreliable for this PR and its auto-close list was not used:

Each number was verified directly against GitHub. Only #648 is closed by this PR.

GitHub Auto-close

drmoisan and others added 18 commits August 31, 2026 20:14
Merged origin/main (c7b4f08) into the branch before executing any plan
task. Two QuickFiler.Test.csproj citations moved by one line: the
EnsureNuGetPackageBuildImports target cited by P0-T6 (:480 -> :481) and
the Controllers\WpfUiDispatcherTests.cs compile item spec cited by P2-T3
and P2-T4 (:190 -> :191). Recorded the reconciliation as item 4 of the
plan's Corrections section, listing the citations re-derived and
confirmed unchanged. Asserted strings are unchanged; the plan validator
passes on the corrected file.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
…the shared fixture (#648)

`QuickFiler.Test/Controllers/WpfUiDispatcherTests.cs` mutated the process-wide static
`UtilitiesCS.Threading.UiThread._dispatcher` by raw reflection and restored it in an
unconditional `finally`, participating in neither of the two locks introduced by #493.
It was the last ungated writer of that static inside `QuickFiler.Test`.

The swap now goes through `UiThreadDispatcherFixture.BeginTransactionAsync()` and the
returned `UiThreadDispatcherTransaction`: `Install` performs the read-modify-write under
`FieldLock`, and `Dispose` restores by `ReferenceEquals` compare-then-write before
releasing `TransactionGate`. The test is declared `async Task` because the gate is
awaited and carries `[Timeout(GateTimeoutMs)]` so a deadlock fails rather than hangs.
`using System.Reflection;` and `using UtilitiesCS;` are removed.

Behavior is unchanged: `Invoke`, `InvokeAsync`, and `BeginInvoke` are still asserted to
execute their delegate on the dispatcher's own thread, and the body of
`Construction_YieldsAnIUiDispatcher` is untouched.

Also adds the Phase 0 baseline, Phase 1 regression, and Phase 2 QA-gate evidence for
issue #648 and checks off AC-1 through AC-7 in the feature issue record.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
…n checklist (#648)

Housekeeping commit for issue #648. Adds
docs/features/active/2026-08-27-wpfuidispatchertests-ungated-static-swap-648/evidence/qa-gates/p2-t18-commit.md,
which records the previous commit's exit code and the scoped porcelain status captured immediately
after it, and flips the final [P2-T18] checkbox in the plan file. Those two writes necessarily
postdate the commit they describe, so they could not be included in it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
The two committed coverage artifacts were raw dotnet-coverage Cobertura
reports totalling 21,578,965 bytes across 387,854 lines. Commit d0955dc
established that raw Cobertura must not enter history as feature
evidence; issue #646 applied the same substitution earlier in this run.

Each report is replaced by a package-level JaCoCo projection. Both
reconcile exactly to their source root counters before deletion:
baseline lines-covered=54966 lines-valid=64381, final lines-covered=54964
lines-valid=64381, 9 first-party packages each. Committed size falls to
2,942 bytes with no loss of a figure any plan gate read.

Both reports are the post-processed first-party-filtered documents, so
the derived 85.373% line and 79.694% branch figures are the policy
coverage figures and clear the 85/75 floors. P0-T15, P2-T7, P2-T8 and
P2-T16 had all been satisfied against the raw files before this pass;
the sequence is recorded in the substitution note.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
Adds the policy-audit, code-review and feature-audit for the branch.
Verdict PASS with zero blocking findings; all seven acceptance criteria
independently re-verified and none contradicted.

Six non-blocking findings recorded. The one that bears on the merge is
F-1: the raw Cobertura reports were committed before being converted, so
both blobs stay reachable from branch history even though the working
tree no longer carries them. A squash merge drops them.

Also removes the single absolute host path from the Phase 0 policy-read
artifact, which finding F-3 identified. The worktree is now named by its
identifier alone, since the absolute form is account-derived.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
Five memories from this run: the CSharpier member-chain wrap that makes a
single-line token gate unsatisfiable, the repo SDK and nullable rebuild
correction, the new-file format-loop restart, the Cobertura-substitution
history residue, and the synthetic evidence timestamps.

Also records that the tracked orchestrator-state blob belongs to whichever
item last force-added it, so an unprotected overwrite destroys another
item's committed record as well as polluting the footprint, and that the
pre-implementation gate needs lifecycle_ready even on a rich checkpoint.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
@drmoisan
drmoisan merged commit 5670b3c into main Sep 1, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: wpfuidispatchertests-ungated-static-swap

1 participant