Skip to content

fix(ribbon): report a repeated engine prime failure once per engine and failure kind - #969

Merged
drmoisan merged 19 commits into
mainfrom
bug/engine-toggle-permanent-config-fault-logs-every-poll-948
Oct 2, 2026
Merged

drmoisan merged 19 commits into
mainfrom
bug/engine-toggle-permanent-config-fault-logs-every-poll-948

Conversation

@drmoisan

@drmoisan drmoisan commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Suggested title

fix(ribbon): report a repeated engine prime failure once per engine and failure kind

Summary

  • EngineToggleStateCoordinator.CompletePrime now reports a prime failure once per pair of engine key and base-exception type. A permanent configuration fault no longer writes one error log entry on every ribbon getPressed poll.
  • Every poll still re-primes. Only the report is suppressed, and a different failure kind for the same engine is reported once more.
  • The record of a reported pair is written by the statement directly after the sink call, inside the existing sink guard, so a sink that throws leaves the report owed.
  • The first prime-failure message now states that further failures of that kind for that engine are not logged again.
  • Toggle-click faults are unchanged: they are still reported on every click.
  • Seven new MSTest regression tests cover repeated faults, repeated cancellations, recovery after a later successful prime, a change of failure kind, per-key suppression, the toggle path, and the message text.

Why

A prime of the engine toggle state that faults for a persistent reason (for example a missing configuration) re-ran on every getPressed poll, and each run reported the same failure through logError. The log filled with identical entries for the life of the session. The fix keeps the re-prime behaviour, so a later successful prime still caches the state and invalidates the control once, and suppresses only the repeated report.

What Changed

Production (TaskMaster/Ribbon/EngineToggleStateCoordinator.cs, 496 lines):

  • New field _reportedPrimeFaults, a ConcurrentDictionary<(string EngineName, Type FaultType), byte>.
  • CompletePrime wraps the existing sink guard in if (!_reportedPrimeFaults.ContainsKey(reportKey)) and records the pair after the sink call returns. _primeTasks.TryRemove(engineName, out _) stays the last statement, so the report-then-clear ordering is preserved.
  • BuildPrimeFailedMessage appends one sentence. The leading text is unchanged.
  • Documentation: the CompletePrime summary, one new remarks paragraph, the report-then-clear comment, and the GetPrimeTask returns element.

Tests:

  • New partial TaskMaster.Test/Ribbon/EngineToggleStateCoordinatorTests.RepeatFaultSuppression.cs (290 lines, seven [TestMethod] methods), registered in TaskMaster.Test/TaskMaster.Test.csproj.
  • No existing test partial was modified.

Documentation and evidence (feature folder docs/features/active/2026-09-30-engine-toggle-permanent-config-fault-logs-every-poll-948/):

  • spec v1.3 with all sixteen acceptance criteria checked off, the research record, the atomic plan (v0.7), baseline, regression, QA-gate and review evidence, and the three audit artifacts.

Architecture / How It Fits Together

RibbonController calls GetPressed for each engine toggle. On an uncached key, StartPrimeIfNeeded registers an in-flight marker and starts one prime; CompletePrime observes its outcome. At most one prime per key is in flight, because a new prime starts only after the marker is removed, and the marker is removed only by the last statement of CompletePrime. The unlocked check-then-record on _reportedPrimeFaults therefore cannot race for a given key. HandleToggleClickAsync does not read the new dictionary.

Verification

Completed (recorded in the feature folder evidence):

  • Fail-before: the seven new tests failed against the unchanged production file; the repeated-fault test observed 5 reports where 1 is expected.
  • Pass-after: the coordinator fixture passed 39 of 39.
  • C# toolchain, one clean pass in order: 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 (0 errors, 0 warnings); msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true (0 errors, 0 warnings); repository test-and-coverage run, 7361 of 7361 passed.
  • Local coverage route: the stall probe reproduced the known shell-icon failure on this workstation, so the local run used the runner's inner collector invocation with the four UtilitiesCS.Test shell-icon classes excluded through a TestCaseFilter and a blame hang timeout, as spec v1.3 permits. CI runs those classes unfiltered.
  • Coverage: first-party lines 85.35% at baseline and 85.35% final; branches 79.75% at baseline and 79.74% final. The branch change comes from the UtilitiesCS package, which this change does not edit; the TaskMaster package covered counts rose (lines 2467 to 2477, branches 517 to 519) with missed counts unchanged. The coordinator file is at 177 of 177 lines and 39 of 40 branches, and both outcomes of the new guard are covered (2 of 2).
  • Feature review: policy audit PASS, code review PASS (0 blocking, 2 non-blocking), feature audit PASS (16 of 16 acceptance criteria).

Recommended:

  • Confirm the CI checks on this pull request, including the unfiltered UtilitiesCS.Test run.

Backward Compatibility / Migration Notes

  • No public API change. The new field is private, and the message change only appends text.
  • Behaviour change: a repeated prime failure of a kind already reported for an engine is no longer logged again during the session. The suppression record is never cleared, so a new kind or a new session reports again.

Risks and Mitigations

  • Risk: a fault that recurs after an intermediate success is not logged a second time. Mitigation: the first occurrence is logged with a message that states the suppression rule, and a different failure kind is still reported.
  • Risk: the production file is at 496 of the 500-line limit. Mitigation: listed as a follow-up below; the split is already proposed separately.
  • Rollback: revert the production file and remove the new test partial and its csproj entry.

Review Guide

  1. TaskMaster/Ribbon/EngineToggleStateCoordinator.cs: the CompletePrime body and the new field.
  2. TaskMaster.Test/Ribbon/EngineToggleStateCoordinatorTests.RepeatFaultSuppression.cs.
  3. TaskMaster.Test/TaskMaster.Test.csproj (one line).
  4. docs/features/active/2026-09-30-engine-toggle-permanent-config-fault-logs-every-poll-948/spec.md and the three review artifacts in that folder. The remaining feature-folder files are execution evidence.

The branch also carries three agent-memory files committed by the preparation passes and the promotion record under docs/features/potential/promoted/.

Follow-ups

These are listed here for the coordinator to file; none were filed from this branch.

  • EngineToggleStateCoordinatorTests.Race.cs lines 196 to 201: the remark says the re-prime logs a second error; it is now a suppressed repeat.
  • Split EngineToggleStateCoordinator.cs (496 lines) and the primary fixture (470 lines) before the next change to either; this overlaps the pending review-residuals item for the 947 change.
  • Add the per-key serialisation rationale (no lock is needed) to the CompletePrime comment.
  • _notifyUnavailable at line 186 remains unguarded (pre-existing, from the 947 review).
  • Plan authoring: dotnet-coverage writes Cobertura line hits as 0 or 1, so hit-count comparisons between lines cannot be used as acceptance conditions (the plan was corrected at execution time, versions 0.6 and 0.7).
  • Evidence Timestamp: labels in the plan payload convention are not clock-derived; derive them from the clock.
  • Canonical local C# coverage artifact path convention for feature review.
  • Run-to-run coverage variance in the UtilitiesCS package (8 lines, 2 branches between two runs with no UtilitiesCS change).

GitHub Auto-close

🤖 Generated with Claude Code

drmoisan and others added 19 commits October 1, 2026 07:21
…plan not yet cleared)

Work in progress committed on coordinator instruction. Research and spec are complete; the atomic plan was still being authored and preflight has not run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Claude-Session: https://claude.ai/code/session_01NcieP6KJzhzgHkRh4F21Po
… stop)

The plan is marked INCOMPLETE: stopped for quota. Phase task lists, command blocks and the internal review record are not yet written; preflight has not run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Claude-Session: https://claude.ai/code/session_01NcieP6KJzhzgHkRh4F21Po
…e 947 landing order

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d spec to 1.3

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ALL CLEAR)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflict in .claude/agent-memory/task-researcher/MEMORY.md resolved by taking the main index and re-adding the 948 research entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ciliation merge

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…-offs on maintainer request

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…he repeat fault suppression fix

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…sue 948)

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…comparison, stopped at P3-T10 for a plan correction

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…he branch row because the collector writes binary line hits

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…3-T10 resume and the not-lowered statement

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…-results version label

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…idence

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…blocking)

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
@drmoisan
drmoisan merged commit 34c2ed8 into main Oct 2, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: engine-toggle-permanent-config-fault-logs-every-poll

1 participant