fix(TaskMaster.Test): add synchronization barrier to terminal-notification-hook test (#751)

[drmoisan]

Summary

Removes the race that made TerminalNotificationHookFailure_DoesNotReplaceDispatchFault a flaky required check. The test asserted on a counter that a terminal hook increments on a thread-pool continuation, with no barrier guaranteeing the hook had run by the assertion point. It failed once on CI with Expected sut.InvokedTerminalHookCount to be 1, but found 0, then passed on an identical-code re-run.

This is a test-only fix. No production file is modified.

Closes #751

Root cause

ReleaseAsync() unblocks the production worker before the fixture's terminal hook has necessarily executed. The hook runs on a continuation, so the assertion and the increment were unordered with respect to each other. Research confirmed the race is confined to the test double: no production code path shares it, and there are no production overrides or subclasses of the affected member.

The change

Three lines across two files, both under TaskMaster.Test:

TaskMaster.Test/AppGlobals/AppOlObjectsFolderTreeServiceTests.cs — in TerminalNotificationHookFailure_DoesNotReplaceDispatchFault:

• Inserted a barrier between await run.Operation.ReleaseAsync(); and sut.LoadCount.Should().Be(0);:
  (await GetExceptionAsync(await run.Terminal)).Should().BeSameAs(fault);
• Hardened the counter read:
  sut.InvokedTerminalHookCount.Should().Be(1); becomes Volatile.Read(ref sut.InvokedTerminalHookCount).Should().Be(1);

TaskMaster.Test/AppGlobals/AppOlObjectsFolderTreeServiceLifecycleTests.cs — in OnFolderTreeServiceInitializationTerminal:

• InvokedTerminalHookCount++; becomes Interlocked.Increment(ref InvokedTerminalHookCount);

The barrier awaits the captured tuple member run.Terminal, not a fresh sut.NextTerminal read. This is load-bearing: the fixture swaps in a new, never-completed signal via Interlocked.Exchange before completing the captured one, so a fresh read would never complete and the test would hang.

The expected value stays 1. It is not relaxed, widened to a range, or deleted. No sleep, no polling, no retry, no [Ignore], no [DoNotParallelize], and no narrowed test filter was introduced.

Note that the "Changed files overview" section of the generated PR context reports "Core logic changes: 0 files" and its churn-ranked list omits both files above, because the evidence artifacts in this branch outrank them by line count. The two files named here are the complete set of source changes.

Why the barrier closes the race rather than narrowing it

The hook increments, then Interlocked.Exchangees in a fresh signal, then TrySetResults the previous one — the exact instance the captured tuple holds. The increment therefore precedes the completion in program order with two full fences between them, so the awaiting continuation cannot observe a pre-increment state. Deadlock-freedom holds because the fixture signals before it throws.

Verification

• Full solution suite, pre-change baseline: 9 assemblies, 6984 tests, 6984 passed, 0 failed. The recorded baseline failure set is empty, so every downstream gate reduces to requiring zero failures.
• Full solution suite, post-change: 6984 passed, 0 failed.
• Pre-change repeat-run series (3 runs) and post-change repeat-run series (5 runs), both under the CI-shaped invocation with no intervening rebuild or edit: 408/408 in TaskMaster.Test on every run, target test Passed on every post-change run.
• Toolchain pass completed clean on the first attempt, in order: csharpier format, csharpier check . (exit 0 across 1574 files), analyzer build, nullable build, full MSTest run. No step rewrote a tracked file and no restart was required.
• Both test files remain within the 500-line limit (493 and 490).

Fail-before evidence uses the dossier-plus-stress-record route rather than temporary instrumentation, because instrumenting the fixture to force a red would restate the race rather than reproduce it, and the instrumentation would not survive into the branch. The pre-change three-run series came out green, which is recorded as observed rather than asserted; the one genuine natural red remains the original CI failure.

Coverage

First-party production line coverage: 85.081% before, 85.059% after (56000/65820 and 55986/65820). Both clear the 85% floor. The 0.022pp movement is confined to a package with zero changed lines and is run-to-run drift.

The branch changes zero production lines, so the no-regression-on-changed-lines requirement has an empty subject. The changed test lines are themselves covered.

One procedural gap is recorded rather than papered over: the plan's own numeric coverage capture returned no figures, because it required the .coverage search under each results directory to match exactly one file and vstest emits two under /InIsolation (the published attachment plus an in-run copy). That precondition was unsatisfiable as written. The figures above were recovered independently from the retained attachments during review. The canonical artifacts/csharp/coverage.xml is absent, which the policy audit records as a FAIL row on artifact presence; it was deliberately not generated here because doing so would have added a path outside the branch's asserted footprint.

Review outcome

Feature review returned 0 blocking findings and 7 non-blocking observations. Audit artifacts are committed under the feature folder: policy-audit.2026-09-03T17-30.md, code-review.2026-09-03T17-30.md, feature-audit.2026-09-03T17-30.md.

All 10 acceptance criteria in spec.md are satisfied and checked, each verified independently during review rather than accepted from the executor's report.

Follow-ups, not addressed here

Deliberately out of scope to keep the fix minimal, and recorded for separate triage:

• The barrier converts a hypothetical future "terminal hook never invoked" regression from a named assertion failure into an unbounded wait. The method carries no [Timeout], and CI invokes vstest without /Settings, so the repository runsettings do not apply. Adding a method-level timeout is the suggested remedy.
• The plan template's .coverage locate step needs a deterministic disambiguation rule instead of an exactly-one cardinality assertion.

Footprint

Two source files, three added lines, net +1 line, zero production files. Everything else on the branch is documentation and evidence under the feature folder. The production file TaskMaster/AppGlobals/AppOlObjects.FolderTreeService.cs is byte-identical to its state at the branch point, confirmed by blob hash on both sides.

Related context, referenced without closing: the test-determinism effort tracked in issue 729, and the CI failure observed on PR 746.