fix(quickfiler): resolve issue #726 code-correctness sweep (6 of 8 findings)

[drmoisan]

Suggested title

fix(quickfiler): resolve issue #726 code-correctness sweep (6 of 8 findings)

Summary

• Hardens FilerQueue's consumer-running flag with a try/finally barrier so an exception in the worker loop can no longer leave the queue permanently stuck; applies the same hardening to the sibling FlagChangeTrainingQueue.
• Routes EfcItemController's two fire-and-forget InitializeWebViewAsync calls through a new guarded wrapper, mirroring the existing QfcItemController fault boundary, so a fault is logged instead of silently discarded.
• Guards three boundary error-sink delegates (EfcItemController, QfcItemController, EfcFormController) against a null or throwing sink, closing a latent path back to the exact unobserved-fault behavior those boundaries exist to prevent.
• Fixes a null-guard/ordering defect in QfcHomeController's metrics write path and removes an orphaned calendar appointment when diagnostics are empty.
• Fixes TaskViewer.ProcessCmdKey to fall through to base.ProcessCmdKey when the controller does not consume an Alt-chord keystroke, instead of unconditionally swallowing every Alt-modified key.
• Removes dead code: two unused locals in QfcFormViewer.ProcessCmdKey, and a write-only field in QfcFormController that started a discarded, unawaited MailItemHelper.FromMailItemAsync call per item.
• Adds a missing positive regression test for the Keys.Menu | Keys.Alt Alt-chord shape and two FilerQueue regression tests (null-item guard, empty-Helpers exceptional-exit recovery).
• Explicitly defers three sub-findings, each with evidence, rather than forcing a fix outside this change's scope (see "Follow-ups").

Why

This consolidates eight code-correctness findings surfaced during the bugs-638-644-647 parallel-orchestration review sweep. Each finding was identified by a prior item's feature-review agent but deliberately left unfixed at the time because fixing it would have exceeded that item's declared file-footprint scope. Rather than run eight separate bug-fix cycles for findings that are each small in isolation, they are consolidated into this one issue (see issue #726 body, and the repo's established practice for consolidated multi-defect issues per #451 and #619).

Two findings are rated High severity individually:

• The FilerQueue flag leak (finding 1) can hang the background mail-filing mover once the narrow trigger condition — an exception inside the diagnostic branch of the catch handler, or a null item — is hit.
• The EfcItemController unguarded fire-and-forget (finding 4) is classified live by the same reasoning that made the sibling fix in issue Bug: qfc-initializewebviewasync-fault-is-unobserved #670 necessary: a discarded faulted Task under .NET Framework 4.5+ is silently finalized with no diagnostic.

What Changed

Core logic:

• QuickFiler/Controllers/FilerQueue.cs — null guard on Enqueue; ConsumeAsync narrowed to internal (no external callers); worker loop body wrapped in try/finally clearing _consumerRunning under the lock on every exit path; empty/null-safe diagnostic logging extracted to LogItemFailure.
• TaskVisualization/FlagChangeTrainingQueue.cs — the identical try/finally guard-reset hardening applied to this sibling queue's consumer loop.
• QuickFiler/Controllers/EfcItemController.cs + new QuickFiler/Controllers/EfcItemController.WebViewFaultBoundary.cs — both InitializeWebViewAsync call sites now go through InitializeWebViewGuardedAsync, which catches and routes faults to an injectable WebViewInitializationErrorSink.
• QuickFiler/Controllers/QfcItemController.WebViewFaultBoundary.cs and QuickFiler/Controllers/EfcFormController.cs — sink invocations wrapped so a null or throwing sink delegate falls back to the logger instead of faulting the guard task.
• QuickFiler/Controllers/QfcHomeController.Metrics.cs — null-safe diagnostics line array; deletes the orphaned calendar appointment when diagnostics are empty (kept the existing call order because GetMoveDiagnostics takes the appointment by ref).
• TaskVisualization/TaskViewer.cs — ProcessCmdKey now returns true only when the controller actually consumed the Alt-chord keystroke, otherwise falls through to base.ProcessCmdKey.
• QuickFiler/Viewers/QfcFormViewer.cs — removed two dead locals with no effect on the real event pipeline.
• QuickFiler/Controllers/QfcFormController.cs + QfcFormController.Actions.cs — removed the write-only _helperTasks field, which started a MailItemHelper.FromMailItemAsync call per item and discarded the result unawaited; the sync pipeline it fed builds its own MailItemHelper via a separate synchronous constructor.

Tests:

• QuickFiler.Test/Controllers/FilerQueueTests.cs — Enqueue_NullItem_ThrowsArgumentNullException; ConsumeAsync_ItemWithEmptyHelpersThrows_ConsumerRecoversForLaterItems (proves the consumer recovers and processes a later item after an exceptional exit, not just the normal empty-queue exit).
• QuickFiler.Test/Controllers/EfcViewerTests.cs — ClaimsAltChord_WithMenuKeyAndAlt_ReturnsTrue (the Keys.Menu disjunct of ClaimsAltChord's guard was previously deletable without failing any existing test).

Build:

• QuickFiler/QuickFiler.csproj — registers the new EfcItemController.WebViewFaultBoundary.cs compile item.

Architecture / How It Fits Together

No new components or wiring. Each fix stays within the file(s) that own the defective behavior:

• FilerQueue / FlagChangeTrainingQueue: internal loop hardening, no external contract change (ConsumeAsync's narrowing to internal was verified to have zero external callers).
• EfcItemController's new fault boundary is a partial class file mirroring the existing QfcItemController.WebViewFaultBoundary.cs pattern already in the codebase, so both viewers now expose the same shape of injectable error sink.
• The QfcFormController dead-field removal has no downstream effect: the field was never read, so the pipeline's actual behavior (a fresh MailItemHelper built synchronously per item inside QfcItemController.Initialize) is unchanged.

Verification

Completed:

• dotnet tool run csharpier format . — clean, 14 files in the intended change set, no unrelated drift.
• msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true — Build succeeded, 0 Error(s).
• msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true — Build succeeded, 0 Error(s).
• vstest.console.exe QuickFiler.Test.dll TaskVisualization.Test.dll /Platform:x64 /InIsolation — 1475/1475 tests passed, including the three new regression tests.

Recommended:

• CI pipeline run against this branch (not verified in this PR beyond the local toolchain above).

Backward Compatibility / Migration Notes

• FilerQueue.ConsumeAsync visibility narrowed from public to internal. Verified via repository-wide grep that it has no external callers; the Consumer property that tests read remains public and unchanged.
• No other public API surface changed.

Risks and Mitigations

• Risk: the try/finally change to FilerQueue's and FlagChangeTrainingQueue's consumer loops changes control flow in a background worker.
  Mitigation: the normal empty-queue exit path still clears the flag inline as before; the finally only adds coverage for the previously-unhandled exceptional exit. New regression test ConsumeAsync_ItemWithEmptyHelpersThrows_ConsumerRecoversForLaterItems proves the consumer keeps draining after an exceptional item.
• Risk: removing _helperTasks eliminates a live Outlook COM call (FromMailItemAsync) per item in the sync LoadItems path.
  Mitigation: confirmed by repository-wide grep that the field was write-only (no reader anywhere, including tests); the call's result was never consumed, so no behavior depended on it running.

Review Guide

Suggested order:

1. QuickFiler/Controllers/FilerQueue.cs + FilerQueueTests.cs — the highest-severity fix and its regression tests.
2. TaskVisualization/FlagChangeTrainingQueue.cs — the same pattern applied to the sibling queue.
3. QuickFiler/Controllers/EfcItemController.cs + EfcItemController.WebViewFaultBoundary.cs (new file) + QfcItemController.WebViewFaultBoundary.cs + EfcFormController.cs — the fault-boundary guarding cluster (findings 4 and 5).
4. QuickFiler/Controllers/QfcHomeController.Metrics.cs — the null-guard/appointment-deletion fix.
5. TaskVisualization/TaskViewer.cs and QuickFiler/Viewers/QfcFormViewer.cs + EfcViewerTests.cs — the Alt-chord/ProcessCmdKey cluster (findings 7 and 8).
6. QuickFiler/Controllers/QfcFormController.cs + QfcFormController.Actions.cs — the dead-field removal (finding 6, live sub-part).

No mechanical moves or renames in this change.

Follow-ups

Three sub-findings from the issue are explicitly deferred, not fixed here:

• Finding 3 (StoreWrapperController.EvaluateLaunchReadiness conflates two distinct causes): already tracked as an open item in docs/features/active/2026-07-09-storewrapper-dialog-imprecise-for-genuine-failure-287/spec.md; resolving it needs a new readiness state and crosses the UtilitiesCS/TaskMaster assembly boundary.
• Finding 6, LoadFolderHandler/InitAsync gap: FolderPredictor has only an async InitAsync, no synchronous equivalent. The sync LoadFolderHandler's three production call sites run through PopulateFolderComboBox, which checks InvokeRequired — forcing a synchronous block on the async init risks a UI-thread deadlock. A minimal fix here would trade a latent hang for a more certain one; needs a dedicated design decision, not a one-line patch.
• Finding 6, dead QfcHighConfidencePreFilter.FilterAsync: confirmed unreachable in production by grep (HighConfidencePreFilterLoader is declared with a default delegate but never invoked anywhere). Removal cascades into three test files (QfcHighConfidencePreFilterTests.cs, QfcHomeControllerIssue218Tests.cs, QfcHomeControllerRunAsyncHighConfidenceTests.cs), exceeding this change's minimal-fix scope.
• Finding 6, two stale comments and one stale test docstring from item Bug: quickfiler-carry-folder-predictor-to-item-controller #678's remediation: the issue gives no file/line citation, and Bug: quickfiler-carry-folder-predictor-to-item-controller #678 touches 26 files in this repo — too broad to identify reliably without expanding scope for a documentation-only latent nit.

GitHub Auto-close

• Closes Bug: quickfiler-review-sweep-code-defects #726