Bug: engine-toggle-coordinator-947-review-residuals

[drmoisan]

• Work Mode: minor-audit

Summary

The #947 review (PR #963) left three residuals in TaskMaster/Ribbon/EngineToggleStateCoordinator.cs:

1. On the refusal path of HandleToggleClickAsync, the "engines unavailable" notification call is still unguarded. A throwing notification sink can escape into the Office ribbon callback, so the method's "never throws" comment overstates that path. This is the same root cause as Bug: engine-toggle-throwing-log-sink-leaves-stale-prime-marker #947.
2. The GetPrimeTask doc comment opens with "The prime task", but the method returns the registration marker.
3. The file is 476 of 500 lines, so it needs splitting before its next change.

Environment

• OS/version: Windows 11 (Outlook VSTO add-in)
• Python version: n/a (C#, .NET Framework 4.8)
• Command/flags used: review of PR fix(ribbon): contain a throwing log sink at both engine-toggle coordinator call sites #963
• Data source or fixture: n/a

Steps to Reproduce

1. Construct the coordinator with engines unavailable and a notification delegate that throws.
2. Invoke the toggle click.

Expected Behavior

• The click handler never throws into the ribbon callback on any path.
• Doc comments match behavior.
• The file has room under the 500-line limit.

Actual Behavior

• The notification exception escapes on the refusal path.
• The GetPrimeTask doc is inaccurate.
• The file is 476 lines.

Logs / Screenshots

• Attached minimal logs or screenshot
• Snippet: PR fix(ribbon): contain a throwing log sink at both engine-toggle coordinator call sites #963 body, Follow-ups 1 to 3; Bug: engine-toggle-throwing-log-sink-leaves-stale-prime-marker #947 review artifacts.

Impact / Severity

• Blocker
• High
• Medium
• Low

Source

From: docs/features/potential/2026-10-01-engine-toggle-coordinator-947-review-residuals.md

[drmoisan]

Restoring sections dropped by the promotion tooling.

Suspected Cause / Notes

#947 (PR #963) guarded only the two _logError call sites. The "engines unavailable" notification on the refusal path of HandleToggleClickAsync has the same exposure. The duplicated guard could become one helper that is applied to both log sinks and notification sinks.

Proposed Fix / Validation Ideas

• Write a regression test first: a throwing notification on the refusal path must not escape. Then add a shared "invoke sink safely" helper and use it at all three sites.
• Correct the GetPrimeTask doc comment: it returns the registration marker, not "the prime task".
• Split EngineToggleStateCoordinator.cs (476 of 500 lines) into partials.
• Sequence this with Bug: engine-toggle-permanent-config-fault-logs-every-poll #948, which edits the same file.

[drmoisan]

Additional residuals from #948 (PR #969, merged at 34c2ed8), consolidated here because they concern the same file and sequence:

• EngineToggleStateCoordinator.cs is now 496 of 500 lines, up from 476. The main test file EngineToggleStateCoordinatorTests.cs is 470. Split both as part of this item, ideally first.
• _notifyUnavailable at line ~186 is unguarded. This is the refusal-path notification already listed here.
• The stale comment in EngineToggleStateCoordinatorTests.Race.cs, lines 196 to 201, still says the re-prime logs a second error. Since Bug: engine-toggle-permanent-config-fault-logs-every-poll #948 it logs once per engine and failure kind.
• The CompletePrime comment should explain why no lock is needed.