fix(quickfiler): call CreateCancellationToken first in QfcHomeController.Init (#839)

[drmoisan]

Suggested title

fix(quickfiler): call CreateCancellationToken first in QfcHomeController.Init (#839)

Summary

• QfcHomeController.Init() now calls CreateCancellationToken() as its first statement, so the datamodel loader, the queue loader and the form-controller loader all receive a real, cancellable token instead of default(CancellationToken) and a null source.
• Adds one MSTest regression test, Init_CreatesTokenSourceBeforeAnyLoaderObservesIt, that pins the ordering rule rather than merely the end state.
• Removes one dead commented-out line so the production file stays within the repository's 500-line ceiling; it is 499 lines after the change.
• The defect is latent, not user-visible: the only production caller of this path has no callers of its own. No user-facing behaviour changes in the shipped add-in.
• Full C# toolchain passed on a single clean pass. QuickFiler.Test went from 1393/1393 to 1394/1394 with zero failures.

Why

QfcHomeController has two initialization paths. The asynchronous path (LaunchAsync then InitAsync) creates a CancellationTokenSource and assigns both _token and _tokenSource before any loader runs. The synchronous path (public constructor, then Init(), then Run()) never assigned either field. The factory that would do so, internal void CreateCancellationToken(), existed in the file with zero production callers.

The consequence on the synchronous path was silent: Init() handed default(CancellationToken) to the datamodel and queue loaders and a null CancellationTokenSource to the form-controller loader, so QfcFormController.LoadItems returned early on its null-source guard and nothing loaded, with no log line and no exception.

The insertion point is load-bearing and is not simply "before the form-controller loader". Init() passes this.Token to the datamodel loader and to the queue loader before it reaches the form-controller loader. An unassigned token field is default(CancellationToken), whose CanBeCanceled is false, so inserting the call anywhere after the first statement would leave the datamodel and queue holding a token that can never be cancelled — a quieter version of the same defect. The regression test asserts CanBeCanceled on both of those tokens precisely to reject that weaker fix.

Scope note: the path is currently unreachable in production. RibbonController.LoadQuickFiler() is its only production caller, and a word-bounded search shows that method has no callers repository-wide. This PR repairs the defect where it lives; removing the dead path altogether is tracked separately (see Follow-ups).

What Changed

Core fix (1 file, +1/-2)

• QuickFiler/Controllers/QfcHomeController.cs: added CreateCancellationToken(); as the first statement of Init(); deleted the dead //public QfcFormViewer FormViewer { get => _formViewer; } comment and its adjacent blank line. No executable line was removed and no statement was reordered.

Tests (1 file, +71/-0)

• QuickFiler.Test/Controllers/QfcHomeControllerTests.cs: added Init_CreatesTokenSourceBeforeAnyLoaderObservesIt. It replaces all five loader delegates with Moq fakes, captures the source and the three tokens the loaders observe, and asserts with FluentAssertions that the captured source is non-null, that all three tokens equal that source's Token, that TokenSource is the same instance, and that the datamodel and queue tokens have CanBeCanceled true. It calls Cleanup() afterwards to dispose the source. The existing Init_InitializesCorrectly is byte-identical; the diff is a pure insertion with zero deleted lines.

Docs and evidence

• Feature folder under docs/features/active/2026-09-09-createcancellationtoken-has-no-production-caller-839/: issue record, spec, user story, research note, atomic plan, 30 evidence projections, and the three review artifacts.

Architecture / How It Fits Together

No architectural change. CreateCancellationToken() already existed as the single point that constructs the CancellationTokenSource and caches its Token; the asynchronous path and EfcHomeController both already call it. This change makes the synchronous path follow the same convention, so both initialization paths now establish the cancellation source before any collaborator observes it.

Disposal ownership was already correct and is unchanged: Cleanup() disposes and nulls the source, and Cleanup is passed to the form controller as parentCleanup, which QfcFormController.Cleanup() invokes under a finally. No new disposal code was required. No interface member was added, removed or changed, and none of the 25 construction sites of QfcHomeController changed, because the fix acts in Init() rather than in a constructor.

Verification

Completed

• Fail-before / pass-after pair on the named test: fails against the unfixed production file (exit 1, on the not-null assertion), passes against the fixed file (exit 0).
• Full C# toolchain in CLAUDE.md order, clean on pass number 1:
  • CSharpier check: exit 0, 1624 files, zero unformatted.
  • .NET analyzers via msbuild /t:Rebuild ... /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true: exit 0, 0 Error(s).
  • Nullable via msbuild /t:Rebuild ... /p:TreatWarningsAsErrors=true: exit 0, zero CS86xx diagnostics.
  • MSTest under dotnet-coverage: exit 0, Total 1394, Passed 1394, Failed 0.
• Both msbuild gates used the Rebuild target and recorded 18 Csc task invocations, which demonstrates the compiler actually ran rather than being skipped by MSBuild's up-to-date check, so neither diagnostic gate passed vacuously.
• Coverage for the modified file: 77.91% lines (201/258) before, 77.99% (202/259) after. The inserted statement is executed. Changed-line coverage is 100%.
• All 12 acceptance criteria in spec.md are checked off against evidence.
• Feature review produced policy-audit, code-review and feature-audit artifacts with zero blocking findings.

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
• CI's own MSTest job, which is the authoritative test gate for this repository.

Backward Compatibility / Migration Notes

None. No public API changed, no member was added or removed from any interface, and no file was renamed or moved. The only behavioural change is on a code path that has no reachable production caller, so no shipped behaviour changes.

Risks and Mitigations

• Risk: a second call to Init() on the same controller would allocate a new CancellationTokenSource without disposing the first. Mitigation: this mirrors the pre-existing pattern in InitAsync and EfcHomeController and is confined to a path with no production caller; it is recorded as a Low finding in the code review rather than fixed here, to keep the diff minimal.
• Risk: the regression test calls Cleanup() outside a finally, so an assertion failure leaks the source and viewer for that test run. Mitigation: Low severity, test-only, and it disappears when the dead path is removed.
• Risk: the modified file sits at 77.99% line coverage, below the 80% floor. Mitigation: this is a pre-existing condition that this change improves rather than worsens (77.91% before), and changed-line coverage is 100%. The policy audit records this explicitly as a non-blocking FAIL token rather than hiding it.
• Rollback: revert the single commit 3b6cd70b; the source change is one added line and two removed lines across two files.

Review Guide

1. QuickFiler/Controllers/QfcHomeController.cs — three lines. Confirm the call is the first statement of Init(), before the QfcDataModelLoader call. This is the whole fix.
2. QuickFiler.Test/Controllers/QfcHomeControllerTests.cs — the new test method. The assertions on dataModelToken.CanBeCanceled and queueToken.CanBeCanceled are the ones that make the ordering, not just the existence, of the source observable.
3. Everything else is documentation and evidence projections and can be skimmed. evidence/qa-gates/toolchain-final-pass.md is the one-page index of the toolchain results.

Mechanical noise: none. There are no renames, no moves and no generated files in this diff.

Follow-ups

• Remedy (d), removal of the dead synchronous entry path, is enumerated in the spec's Rollout & Follow-up section naming all five symbols in scope: RibbonController.LoadQuickFiler(), QfcHomeController.Init(), IQfcHomeController.Init(), CreateCancellationToken() and Init_InitializesCorrectly. It is deliberately not filed from this branch. When filing it, note that two files are named IQfcHomeController.cs; the declaring one is QuickFiler/Controllers/IQfcHomeController.cs.
• scripts/vscode/TaskMaster.cli.runsettings enables MSTest class-level parallelism (Workers 0, Scope ClassLevel), which fails three QfcInitEmailQueueZeroBatchTests by poisoning a cached Deedle static initializer. CI passes no settings file and is unaffected, so the defect is invisible to CI while breaking the repository's own documented local coverage command. Both that file and scripts/vscode/Invoke-MSTestWithCoverage.ps1 are outside this change's scope and were not modified; this warrants its own issue.
• Branch coverage was not transcribed in the projections. The inserted statement is a straight-line call with no conditional, so this diff cannot move a branch figure; the omission is a reporting gap only.

GitHub Auto-close

• Closes Bug: createcancellationtoken-has-no-production-caller #839

Issue #839 was verified OPEN on GitHub before this bullet was emitted. Issue #810 appears in the context bundle's author-asserted list only because acceptance criterion AC6 names a test introduced by that issue; #810 is a distinct, already-closed bug and is deliberately not closed here. The bundle's third candidate, #COMMIT-1, is not an issue reference at all — it is a false positive parsed from a COMMIT-1-SHA: field in an evidence artifact — and is deliberately omitted.

🤖 Generated with Claude Code