fix(964): guard every EngineToggleStateCoordinator sink call and split the coordinator into partials

[drmoisan]

Suggested title

fix(964): guard every EngineToggleStateCoordinator sink call and split the coordinator into partials

Summary

• The refusal path of EngineToggleStateCoordinator.HandleToggleClickAsync, taken when the engines accessor returns null, no longer lets a throwing notifyUnavailable sink escape into the async void Office handler. The notification failure is reported once through logError, and a logError failure on that path is contained.
• All three sink call sites (refusal-path notification, click-boundary logError, prime-fault logError in CompletePrime) now go through one private guard, TryInvokeSink. Only two catch clauses remain in the type: the click boundary and the guard.
• The issue Bug: engine-toggle-permanent-config-fault-logs-every-poll #948 record placement is preserved: a prime fault kind is recorded only after its report reaches the sink. A new test pins the case where the sink throws and the report stays owed.
• The GetPrimeTask documentation now describes the registration marker rather than "the prime task".
• TaskMaster/Ribbon/EngineToggleStateCoordinator.cs is split, as a pure move, into three cohesive partials (302, 197 and 86 lines), all registered in TaskMaster.csproj.

Why

Issue #964 records the residual findings of the #947 review. The refusal-path notification was the one sink call left unguarded, so a throwing sink could crash the click handler. The three sites guarded the sink in three different ways, the GetPrimeTask doc comment had drifted from the marker semantics, and the coordinator file had grown past the size the follow-up changes needed.

What Changed

Production (TaskMaster/Ribbon/)

• EngineToggleStateCoordinator.cs: the type contract, fields, constructor, GetPressed, HandleToggleClickAsync, ExecuteToggleAsync, and the new TryInvokeSink guard. The constructor and the GetPressed / HandleToggleClickAsync remarks state the non-throwing precondition on the engines accessor.
• EngineToggleStateCoordinator.Prime.cs (new): the prime lifecycle, moved unchanged apart from routing the CompletePrime sink call through the guard.
• EngineToggleStateCoordinator.Messages.cs (new): the null-name token, RenderEngineName and the message builders, plus the new BuildNotifyFailedMessage.
• TaskMaster.csproj: two Compile items.

Tests (TaskMaster.Test/Ribbon/)

• EngineToggleStateCoordinatorTests.SinkGuard.cs (new):
  • refusal-path tests for a throwing notification sink, with and without a throwing log sink;
  • a data-driven test for a null or empty engine key;
  • a guard test for the Bug: engine-toggle-permanent-config-fault-logs-every-poll #948 record placement under the shared guard.
• EngineToggleStateCoordinatorTests.cs: a harness OnNotify hook, so a test can model a throwing notification sink.
• EngineToggleStateCoordinatorTests.Race.cs: documentation-only update.
• TaskMaster.Test.csproj: one Compile item.

Docs and evidence

• docs/features/active/2026-10-01-engine-toggle-coordinator-947-review-residuals-964/: the issue record, the atomic plan, the remediation inputs and plan for one review cycle, the review artifacts, and evidence under evidence/. The evidence is projections and summaries only; no raw coverage or trx documents are committed.

Architecture / How It Fits Together

TryInvokeSink(Action sinkCall, out Exception sinkFailure) invokes a sink. It returns true on a normal return, or false with the contained exception, and never rethrows. Each caller decides what a failure means:

• the refusal path forwards a notification failure to logError through the guard once more;
• the click boundary discards a logError failure;
• CompletePrime records the fault kind only in the true branch, then clears the marker as its last statement.

The ordering invariants from #942, #944, #947 and #948 are unchanged.

Verification

Completed (recorded under the feature folder's evidence/)

• Fail-before: the three new refusal-path tests failed against the split but unfixed coordinator (43 run, 3 failed, as expected). After the fix the fixture passed 43 of 43.
• Full C# toolchain in one clean pass (CLAUDE.md commands): CSharpier check of 1640 files; analyzer /t:Rebuild with 0 errors and 0 warnings; TreatWarningsAsErrors /t:Rebuild with 0 errors and 0 warnings; Invoke-MSTestWithCoverage.ps1 passed 7390 of 7390.
• Coverage:
  • repository lines 85.97% and branches 80.13%, up from 85.96% and 80.10% at baseline;
  • coordinator files 203 of 203 lines and 44 of 44 branches;
  • new methods TryInvokeSink and BuildNotifyFailedMessage at 100% of lines.
• Edit-scope census: the three production files show exactly the planned token and phrase changes, 14 protected method and field spans are hash-identical to base, and the existing test partials are byte-identical apart from the documented changes.
• Reduced review: PASS with 0 blocking findings. One remediation cycle closed the related findings CR-1 (untested null-or-empty engine-name arm) and CR-4 (assertion symmetry), and the re-audit was PASS with 0 blocking findings.
• Final commit: CR-5 and O-6 were closed as comment and documentation edits only. CSharpier check on the touched file exits 0; the full toolchain was not re-run for that commit.

Recommended

• Branch CI on the PR head.

Backward Compatibility / Migration Notes

None. The type is internal, the split is a pure move within one partial class, and no public signature changed.

Risks and Mitigations

• Broad catch in TryInvokeSink: it contains every exception a sink throws. This is accepted and documented in place; it mirrors RibbonCommandBoundary.SafeLog, and the caller is an async void Office handler, where an escaping exception is the worse outcome.
• Throwing engines accessor: an accessor that throws is not guarded, by design. The constructor documentation states it as a precondition of the composition root.
• Rollback: revert the PR. The change is confined to the coordinator, its tests and the two project files.

Review Guide

1. TaskMaster/Ribbon/EngineToggleStateCoordinator.cs: the TryInvokeSink guard and the refusal-path and click-boundary call sites.
2. TaskMaster/Ribbon/EngineToggleStateCoordinator.Prime.cs: CompletePrime, where the record happens only after the sink returns and the marker is cleared last.
3. TaskMaster.Test/Ribbon/EngineToggleStateCoordinatorTests.SinkGuard.cs.
4. EngineToggleStateCoordinator.Messages.cs is a mechanical move. Most of the evidence Markdown under the feature folder is audit trail.

Follow-ups

• The review recorded these observations, none blocking:
  • the primary test fixture is at 481 of 500 lines;
  • the coordinator files do not opt in to #nullable enable;
  • the canonical artifacts/csharp/coverage.xml path and quality-tiers.yml are absent repository-wide (already tracked).
• Some cycle-1 evidence timestamps were re-stamped to a host-clock reading after first being written as estimates. This is disclosed in each affected file and rated Minor in the re-audit, with no task owed.

GitHub Auto-close

• Closes Bug: engine-toggle-coordinator-947-review-residuals #964