fix(ribbon): contain a throwing log sink at both engine-toggle coordinator call sites

[drmoisan]

Suggested title

fix(ribbon): contain a throwing log sink at both engine-toggle coordinator call sites

Summary

• EngineToggleStateCoordinator.CompletePrime now guards its logError call, so a throwing sink no longer skips the prime-marker clear. Before this change, the stale marker blocked every later re-prime for that engine for the rest of the session.
• EngineToggleStateCoordinator.HandleToggleClickAsync now guards its own logError call inside the click-boundary catch, so a throwing sink no longer escapes into the async void Office ribbon handler.
• The report-then-clear ordering in CompletePrime is unchanged. The sink is still invoked before the marker is removed, and the existing PrimeFaultOrdering test file is byte-identical and passes.
• Four new regression tests in TaskMaster.Test/Ribbon/EngineToggleStateCoordinatorTests.ThrowingSink.cs cover both call sites. All four were observed failing on the pre-fix code and pass after the fix.
• Work mode is minor-audit. All seven acceptance criteria in the feature folder's issue.md are checked off with evidence.

Why

• The logError sink is the coordinator's last reporting channel. When it threw inside CompletePrime, the exception escaped before _primeTasks.TryRemove(...) ran. The marker stayed registered, so a later GetPressed returned at the ContainsKey check and never started a new prime. The exception also faulted the discarded continuation without anything observing it.
• At the click boundary, the same sink failure escaped HandleToggleClickAsync. Its caller is an async void Office handler, so the exception would become unobserved. The maintainer added this second call site to the issue's scope on 2026-10-01.

What Changed

Production (TaskMaster/Ribbon/EngineToggleStateCoordinator.cs, 442 to 476 lines)

• CompletePrime: wraps _logError(BuildPrimeFailedMessage(engineName), failure) in try / catch (Exception) with the comment // Intentionally discarded: see the remarks on this method.. _primeTasks.TryRemove stays after the block.
• HandleToggleClickAsync: wraps _logError(BuildToggleFailedMessage(engineName), ex) the same way inside the existing catch (Exception ex). The toggle fault is still reported and is still not rethrown.
• XML documentation is updated in four places: the HandleToggleClickAsync summary and remarks, the GetPrimeTask returns, the StartObservedPrime remarks, and the CompletePrime summary and remarks. The docs now describe three catch clauses (one click boundary, two sink guards) and the "report has been attempted" guarantee.
• Both guards follow the existing in-repo precedent RibbonCommandBoundary.SafeLog.

Tests

• New partial TaskMaster.Test/Ribbon/EngineToggleStateCoordinatorTests.ThrowingSink.cs (215 lines, MSTest + Moq + FluentAssertions, no harness changes):
  • GetPressed_WhenLogSinkThrowsOnFaultedPrime_LaterReadStartsNewPrime
  • GetPressed_WhenLogSinkThrowsOnCanceledPrime_LaterReadStartsNewPrime
  • GetPressed_WhenLogSinkThrows_FirstPrimeCompletesAndMarkerIsCleared
  • HandleToggleClickAsync_WhenLogSinkThrowsOnToggleFault_DoesNotThrowAndAttemptsReport
• TaskMaster.Test/TaskMaster.Test.csproj: one <Compile Include> entry for the new partial.

Docs

• Feature folder docs/features/active/2026-09-30-engine-toggle-throwing-log-sink-leaves-stale-prime-marker-947/ contains:
  • the plan, research, and issue.md acceptance-criteria check-offs;
  • baseline, regression-testing, qa-gates, and other evidence;
  • the small-audit review artifacts (policy-audit, code-review, feature-audit, all dated 2026-10-01T19-30).

Architecture / How It Fits Together

• GetPressed starts a prime through StartPrimeIfNeeded, which registers a marker in _primeTasks. The continuation in StartObservedPrime calls CompletePrime and then completes the marker in a finally.
• CompletePrime reports any non-success outcome through the sink and then clears the marker. A sink failure is now contained between those two steps, so the clear always runs.
• HandleToggleClickAsync is the only boundary that observes an engine fault. Its sink call is now guarded, so the method never throws to its async void caller when the sink throws.

Verification

Completed (from the committed evidence)

• Fail-before: all four new tests failed against the unchanged production file:
  • both prime tests: Moq reported EngineActiveAsync invoked 1 time, expected 2;
  • the marker-cleared assertion failed;
  • the click-boundary test reported InvalidOperationException: sink failed escaping HandleToggleClickAsync.
• Pass-after: coordinator fixture 32/32 passed.
• Final toolchain pass, clean on the first iteration:
  • dotnet tool run csharpier format . and dotnet tool run csharpier check .: no changes.
  • msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true: exit 0.
  • msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true: exit 0.
  • MSTest with coverage: 7,336/7,336 passed.
• Coverage:
  • first-party lines 85.34% and branches 79.73%, unchanged from the baseline;
  • EngineToggleStateCoordinator.cs 167/167 lines;
  • both new catch (Exception) arms covered;
  • changed executable lines 12/12 covered.
• The local coverage run used the plan's DIRECT route. It excluded four UtilitiesCS.Test shell-icon test classes that fail on the local workstation. CI runs them.
• Small-audit review: policy audit, code review, and feature audit all PASS, with 0 Blocking findings.

Recommended

• Confirm the required CI checks are green on the PR head.

Backward Compatibility / Migration Notes

• No public API change. All edited members are internal or private.
• Behaviour changes only when the error sink itself throws: that exception is now discarded instead of propagating.

Risks and Mitigations

• Risk: a sink exception is now discarded and is not observable anywhere. Mitigation: the sink is the last reporting channel, so there is nowhere further to report it. This is the same documented trade as RibbonCommandBoundary.SafeLog, and the remarks on each method state it.
• Rollback: revert the fix commit. The test partial and its csproj entry can be reverted with it.

Review Guide

1. TaskMaster/Ribbon/EngineToggleStateCoordinator.cs: the two guarded sink calls in CompletePrime and HandleToggleClickAsync, then the documentation edits.
2. TaskMaster.Test/Ribbon/EngineToggleStateCoordinatorTests.ThrowingSink.cs.
3. evidence/regression-testing/throwing-sink-fail-before.md and throwing-sink-pass-after.md.
4. The remaining feature-folder evidence is the audit trail and is mostly mechanical.

Follow-ups

Listed for the coordinator to file; none were filed from this branch:

• Guard _notifyUnavailable (and _enginesAccessor) on the HandleToggleClickAsync refusal path. A throwing notify sink can still escape into the async void handler, so the "never throws" remark is currently overstated for that path. The fix could also extract a shared private sink-guard helper for the two duplicated guards.
• Correct the GetPrimeTask <returns> opening wording ("The prime task"). The returned value is the registration marker.
• Split EngineToggleStateCoordinator.cs (476 of 500 lines) before its next change.
• The earlier log-volume follow-up from the prime-marker registration work remains open.
• Reconcile the canonical C# coverage artifact path with the runner's coverage/ output directory.

GitHub Auto-close

• None (no verified closing issues in the PR context; the parallel-run coordinator handles closure).

🤖 Generated with Claude Code