Stabilize flaky TimeOutTask timing test under parallel/coverage (#191)

.claude/agent-memory/atomic-executor/project_build_test_env.md

@@ -14,6 +14,7 @@ TaskMaster is a .NET Framework 4.8 C#/VSTO solution. Running the repo toolchain
 - vstest.console.exe is at `C:/Program Files/Microsoft Visual Studio/18/Community/Common7/IDE/Extensions/TestPlatform/vstest.console.exe`. Prefix the command with `MSYS_NO_PATHCONV=1` so `/EnableCodeCoverage`, `/Tests:`, `/TestCaseFilter:` are not converted to git paths. Pass test DLL paths as Windows-style backslash paths (mixed forward/back slashes can make vstest report "test source file not found").
 - Coverage: `/EnableCodeCoverage` produces a binary `.coverage`. Convert with `Microsoft.CodeCoverage.Console.exe merge <file> -f xml -o out.xml` (the deprecated `CodeCoverage.exe analyze` subcommand fails). The XML schema is `results/modules/module[@line_coverage,@lines_covered,...]` with per-`function`/`range` data; `source_files` appear AFTER `functions` within each `module`, so resolve `source_id`->path at module-end when stream-parsing.
 - CSharpier is v1.x: use `dotnet tool run csharpier format <path>` (write) and `... check <path>` (verify). The old `csharpier .` syntax prints help. `csharpier check .` repo-wide returns exit 1 only because of pre-existing `TaskMaster.csproj` (a project file, not .cs).
+- CSharpier v1 `format .` ALSO reformats XML project files (`*.csproj`) — it rewrote 8 csproj when run repo-wide. The repo's `.csharpierignore` does NOT list `*.csproj`/`*.props`/`*.targets` (only `**/evidence/**` and coverage artifacts); just `.prettierignore` excludes project files. To avoid out-of-scope csproj churn, run `csharpier check <specific .cs files>` instead of `format .`, or `git checkout -- <csproj paths>` to revert the project-file reformatting after a repo-wide format. The repo intent (per `.prettierignore` and CLAUDE.md C#1) is that `*.csproj`/`*.props`/`*.targets` stay in Visual Studio format.
 - A forced-nullable build (`-p:Nullable=enable -p:TreatWarningsAsErrors=true`) must use `-t:Rebuild` to surface the ~84 pre-existing vendored errors (confined to `SVGControl` and `UtilitiesSwordfish`); an incremental `-t:Build` reports 0 because those assemblies are not recompiled. The forced-nullable Rebuild leaves Debug test DLLs absent (TreatWarningsAsErrors aborts downstream output) — always run a plain `-t:Build -p:Configuration=Debug` afterward to restore `QuickFiler.Test.dll` / `UtilitiesCS.Test.dll` before running vstest.
 - Legacy (non-SDK) csproj: new `.cs` files must be added as explicit `<Compile Include="..."/>` entries; there is no globbing.
 - `IList<T>` of an `internal` type in a `public` interface signature causes CS0051 — the carried type must be `public`. To mock an `internal` interface with Moq, the assembly needs `[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]` in a COMPILED file (QuickFiler's existing one is in `Legacy/IAcceleratorCallbacks.cs`, which is NOT in the csproj, so it is not compiled).

UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs

@@ -134,7 +134,7 @@ public async Task RunWithTimeout_FuncT1TResult_ShouldReturnResult()
             var result = await function.RunWithTimeout(
                 42,
                 CancellationToken.None,
-                milliseconds: 200,
+                milliseconds: 5000,
                 maxAttempts: 0,
                 strict: true
             );

UtilitiesCS.Test/Threading/TimeOutTask_Tests.cs

@@ -7,6 +7,7 @@
 namespace UtilitiesCS.Test
 {
     [TestClass]
+    [DoNotParallelize]
     public partial class TimeOutTask_Tests
     {
         [TestMethod]

docs/features/active/2026-06-12-timeout-task-flaky-timing-191/code-review.2026-06-12T20-49.md

@@ -0,0 +1,108 @@
+# Code Review: TimeOutTask flaky-timing test fix (Issue #191)
+
+**Review Date:** 2026-06-12
+**Reviewer:** feature-review agent
+**Feature Folder:** `docs/features/active/2026-06-12-timeout-task-flaky-timing-191`
+**Base Branch:** `origin/main` (merge-base `aa63315b`)
+**Head Branch:** `bug/timeout-task-flaky-timing` (uncommitted working-tree edits)
+**Review Type:** Initial review (minor-audit)
+
+---
+
+## Executive Summary
+
+This review covers a test-only determinism fix for the flaky test `TimeOutTask_Tests.RunWithTimeout_FuncT1TResult_ShouldReturnResult`. The change is intentionally minimal and confined to test code; the production file `UtilitiesCS/Threading/TimeOutTask.cs` is unchanged. Evidence reviewed: the full `git diff origin/main`, the two changed test files, all five QA-gate artifacts, the determinism repeated-run evidence, and the coverage XML.
+
+**What changed:**
+- `UtilitiesCS.Test/Threading/TimeOutTask_Tests.cs`: added `[DoNotParallelize]` to the `[TestClass]` partial-class declaration (+1 line; 216 → 217).
+- `UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs`: widened the success-path timeout argument `milliseconds: 200` to `milliseconds: 5000` in `RunWithTimeout_FuncT1TResult_ShouldReturnResult`; the assertion `result.Should().Be("result-42")` and the `maxAttempts: 0, strict: true` arguments are preserved (line count unchanged at 484).
+- `.claude/agent-memory/atomic-executor/project_build_test_env.md`: a non-code agent-memory note recording CSharpier v1 csproj-reformatting behavior.
+
+Both mitigations follow established repository precedent: `[DoNotParallelize]` is already used in five other test classes (including `ApplicationIdleTimer_Tests` and `TimerWrapper_Tests` cited in `issue.md`), and a 5000 ms timeout matches the existing precedent comment at `TimeOutTask_Tests.cs` line 76 ("increased from 100ms to 5000ms"). The change does not weaken any assertion and does not alter production timeout semantics.
+
+**Top 3 risks:**
+1. None of material severity. The change is a two-line test-only edit with preserved assertions.
+2. The 5000 ms timeout is an upper bound; on a pathologically starved machine a determinism guarantee is still probabilistic, but the value is 25x the prior window and matches existing repo precedent, so residual risk is low.
+3. The whole-solution nullable build remains red due to pre-existing vendored-project breakage; this is unrelated to the change but means the canonical whole-solution gate cannot be cited as green (the scoped per-file gate is green).
+
+**PR readiness recommendation:** **Go** — The change is minimal, evidence-backed, assertion-preserving, and consistent with repository precedent; no blocking or major findings.
+
+---
+
+## Findings Table
+
+| Severity | File | Location | Finding | Recommendation | Rationale | Evidence |
+|---|---|---|---|---|---|---|
+| Info | `UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs` | line 137 | Timeout widened `200` → `5000` ms; assertion preserved. | None required. | Removes the wall-clock race without weakening intent; matches repo precedent. | `git diff origin/main`; `TimeOutTask_Tests.cs` line 76; `evidence/regression-testing/determinism-repeated-runs.md` |
+| Info | `UtilitiesCS.Test/Threading/TimeOutTask_Tests.cs` | line 10 | `[DoNotParallelize]` added to `[TestClass]`. | None required. | Established repo pattern for timing-sensitive classes. | `git grep -l DoNotParallelize` (5 other classes); `evidence/qa-gates/*` |
+| Info | `UtilitiesCS.Test` (assembly) | `IdleAsyncQueue_Tests.cs` | Pre-existing flaky test failed once in full parallel run; passed 3/3 in isolation. | Track separately; not caused by this change. | File is not in this branch diff; pre-existing. | `evidence/qa-gates/qa-04-test-coverage.md`; `git diff --name-only origin/main` (IdleAsyncQueue not listed) |
+
+No Blockers or Major findings.
+
+---
+
+## Implementation Audit
+
+### C# implementation audit
+
+#### What changed well
+
+- The fix is the smallest viable test-only change. It correctly declines a production `TimeProvider` refactor of the ~775-line `TimeOutTask.cs`, which `issue.md` and `.claude/rules/csharp.md` (TimeProvider guidance is "guidance only") both indicate would be disproportionate for a test-flakiness defect.
+- Both mitigations reuse existing repository patterns rather than inventing a new mechanism, keeping the diff legible and consistent with `ApplicationIdleTimer_Tests`, `TimerWrapper_Tests`, and the existing `TimeOutTask_Tests` generous-timeout precedent.
+- The change is layered defensively: `[DoNotParallelize]` removes the thread-pool contention source, and the widened timeout removes the residual single-test wall-clock sensitivity. Either alone reduces flakiness; together they are deterministic across the captured runs.
+
+#### Type safety and API notes
+
+- No production API, type, or nullable surface changed. The scoped `TreatWarningsAsErrors` recompile reports no diagnostics on either changed file (`evidence/qa-gates/qa-03-nullable.md`).
+- `[DoNotParallelize]` is the standard MSTest assembly-execution attribute; its placement on the `[TestClass]`-bearing partial declaration is correct.
+
+#### Error handling and logging
+
+- Not applicable. No error-handling or logging code is added or modified; the change is an attribute and a numeric literal.
+
+---
+
+## Test Quality Audit
+
+The verification evidence is complete for a test-only change. The five QA-gate artifacts cover formatting, analyzers, nullable, test execution with coverage, and a coverage-delta statement; the regression-testing folder provides the determinism evidence (13/13 passes) and a pre-fix failing-state capture.
+
+### Reviewed test and QA artifacts
+
+- `evidence/qa-gates/qa-01-csharpier.md` — CSharpier `check` on the two changed .cs files, EXIT 0; documents the out-of-scope csproj reformatting and its revert.
+- `evidence/qa-gates/qa-02-analyzers.md` — analyzer build 0 errors; NO_WARNINGS_IN_CHANGED_FILES.
+- `evidence/qa-gates/qa-03-nullable.md` — scoped recompile NONE_IN_CHANGED_FILES; whole-solution exit 1 attributed to pre-existing vendored breakage.
+- `evidence/qa-gates/qa-04-test-coverage.md` — full suite 3814/3815 pass; affected test PASS; documents the pre-existing flaky `IdleAsyncQueue` failure and its 3/3 isolation re-runs.
+- `evidence/qa-gates/qa-05-coverage-delta.md` — no changed-line coverage regression (zero new production lines); module 85.31%.
+- `evidence/regression-testing/determinism-repeated-runs.md` — 12 parallel + 1 coverage run, all PASS, zero `TimeoutException`.
+- `evidence/qa-gates/coverage-post.xml` — Cobertura/MS coverage XML; UtilitiesCS.dll module 85.31% line coverage.
+
+### Quality assessment prompts
+
+- **Determinism:** The change removes a flaky dependency (thread-pool starvation under class-level parallelism plus coverage instrumentation). 13/13 passes with zero timeouts demonstrate the determinism objective is met for the captured environment.
+- **Isolation:** The affected test targets a single behavior (success-path result return); `[DoNotParallelize]` improves isolation by serializing the class.
+- **Speed:** 46–49 ms per parallel run, 98 ms under coverage. The 5000 ms value is an upper-bound timeout, not a wait, so it does not slow the passing path.
+- **Diagnostics:** The FluentAssertions assertion produces a clear failure message; unchanged.
+
+---
+
+## Security / Correctness Checks
+
+| Check | Status | Evidence |
+|---|---|---|
+| No secrets in code | ✅ PASS | Diff contains an attribute and a numeric literal; no secrets. |
+| No unsafe subprocess or command construction | ✅ PASS | No process/command construction in the diff. |
+| Input validation at boundaries | N/A | No boundary code changed; test uses in-memory `Func`. |
+| Error handling remains explicit | ✅ PASS | Production error handling unchanged; assertion preserved. |
+| Configuration / path handling is safe | ✅ PASS | No configuration or path handling in the diff. |
+
+---
+
+## Research Log
+
+No external research was required. All conclusions are grounded in the branch diff, the feature-folder evidence artifacts, and the repository's own policy and precedent files (`.claude/rules/csharp.md`, `issue.md`, existing `[DoNotParallelize]` usages).
+
+---
+
+## Verdict
+
+The change is ready for normal PR flow. It is a minimal, assertion-preserving, test-only determinism fix that reuses two established repository patterns and is backed by complete QA-gate and determinism evidence. The two incidental items — a pre-existing flaky `IdleAsyncQueue` test (not in this diff) and CSharpier v1 reformatting of 8 csproj files (reverted; zero project files remain modified) — are non-blocking and out of scope. This conclusion is consistent with the Findings Table (no Blocker/Major findings) and the Go readiness recommendation.

docs/features/active/2026-06-12-timeout-task-flaky-timing-191/evidence/baseline/baseline-test-run.md

@@ -0,0 +1,16 @@
+# Baseline Test Run (Affected Test, Parallel + Coverage)
+
+Timestamp: 2026-06-13T00-33
+
+Command: vstest.console.exe UtilitiesCS.Test\bin\Debug\UtilitiesCS.Test.dll /Tests:RunWithTimeout_FuncT1TResult_ShouldReturnResult /InIsolation /EnableCodeCoverage
+
+(Environment note: vstest.console.exe resolved to "C:\Program Files\Microsoft Visual Studio\18\Community\Common7\IDE\CommonExtensions\Microsoft\TestWindow\vstest.console.exe"; `/InIsolation` added because the Moq-backed test assembly requires it in this environment per recorded toolchain quirks; run under git-bash with `MSYS_NO_PATHCONV=1`.)
+
+EXIT_CODE: 0
+
+Output Summary:
+- Test Parallelization enabled (Workers: 24, Scope: ClassLevel) — confirms class-level parallelism active at baseline.
+- Passed RunWithTimeout_FuncT1TResult_ShouldReturnResult [102 ms]
+- Total tests: 1; Passed: 1.
+- Coverage attachment produced: TestResults\0ef323d5-3ca0-4c9b-9349-fbfbb276323a\DanMoisan_MEGALODON4_2026-06-12.20_33_13.coverage
+- Coverage headline: a single-test targeted run produces a .coverage binary attachment; numeric module-coverage percent is captured at the full-suite level in Phase 2 (P2-T4). Baseline single-test execution passed under parallel + coverage on this run (the defect is intermittent/load-dependent per the fail-before dossier, not a deterministic baseline failure).

docs/features/active/2026-06-12-timeout-task-flaky-timing-191/evidence/baseline/branch-commit-baseline.md

@@ -0,0 +1,13 @@
+# Branch / Commit Baseline
+
+Timestamp: 2026-06-13T00-31
+
+Command: git rev-parse --abbrev-ref HEAD
+EXIT_CODE: 0
+
+Command: git rev-parse HEAD
+EXIT_CODE: 0
+
+Output Summary:
+- Branch: bug/timeout-task-flaky-timing
+- Commit SHA: aa63315bd432ffbf092cfbb5caa02ee673e7b326

docs/features/active/2026-06-12-timeout-task-flaky-timing-191/evidence/baseline/phase0-instructions-read.md

@@ -0,0 +1,23 @@
+# Phase 0 — Instructions Read
+
+Timestamp: 2026-06-13T00-31
+
+Policy Order:
+1. CLAUDE.md (standing instructions, always loaded)
+2. .claude/rules/general-code-change.md (cross-language code change policy)
+3. .claude/rules/general-unit-test.md (cross-language unit test policy)
+4. .claude/rules/csharp.md (C#-specific rules — language in scope)
+
+Files read:
+- c:\Users\DanMoisan\repos\TaskMaster\CLAUDE.md
+- c:\Users\DanMoisan\repos\TaskMaster\.claude\rules\general-code-change.md
+- c:\Users\DanMoisan\repos\TaskMaster\.claude\rules\general-unit-test.md
+- c:\Users\DanMoisan\repos\TaskMaster\.claude\rules\csharp.md
+- c:\Users\DanMoisan\repos\TaskMaster\.claude\skills\policy-compliance-order\SKILL.md
+- c:\Users\DanMoisan\repos\TaskMaster\.claude\skills\atomic-plan-contract\SKILL.md
+- c:\Users\DanMoisan\repos\TaskMaster\.claude\skills\evidence-and-timestamp-conventions\SKILL.md
+- c:\Users\DanMoisan\repos\TaskMaster\.claude\skills\acceptance-criteria-tracking\SKILL.md
+
+Notes:
+- Work Mode: minor-audit. AC source is issue.md `## Acceptance Criteria` (AC1–AC6) only.
+- Test-only change; no production file modification permitted.

docs/features/active/2026-06-12-timeout-task-flaky-timing-191/evidence/baseline/precedent-capture.md

@@ -0,0 +1,47 @@
+# Precedent Capture
+
+Timestamp: 2026-06-13T00-31
+
+## (a) Affected test — RunWithTimeout_FuncT1TResult_ShouldReturnResult
+Source: UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs lines 127–144
+
+```csharp
+        [TestMethod]
+        public async Task RunWithTimeout_FuncT1TResult_ShouldReturnResult()
+        {
+            // Arrange
+            Func<int, string> function = arg => $"result-{arg}";
+
+            // Act
+            var result = await function.RunWithTimeout(
+                42,
+                CancellationToken.None,
+                milliseconds: 200,
+                maxAttempts: 0,
+                strict: true
+            );
+
+            // Assert
+            result.Should().Be("result-42");
+        }
+```
+
+## (b) [TestClass] / partial class declaration carrying the attribute
+Source: UtilitiesCS.Test/Threading/TimeOutTask_Tests.cs lines 9–10
+
+```csharp
+    [TestClass]
+    public partial class TimeOutTask_Tests
+```
+
+## (c) [DoNotParallelize] precedent
+Source: UtilitiesCS.Test/Threading/ApplicationIdleTimer_Tests.cs lines 16–17
+
+```csharp
+    [TestClass]
+    [DoNotParallelize]
+    public class ApplicationIdleTimer_Tests
+```
+
+Notes:
+- `TimeOutTask_Tests` is a partial class; `[TestClass]` appears on exactly one declaration (TimeOutTask_Tests.cs line 9). A class-level `[DoNotParallelize]` placed there governs the whole partial class.

docs/features/active/2026-06-12-timeout-task-flaky-timing-191/evidence/issue-updates/issue-191.2026-06-12T20-25.md

@@ -0,0 +1,24 @@
+# Issue #191 Update Mirror
+
+Timestamp: 2026-06-13T00-41
+
+PostedAs: body (local feature issue.md only; not posted to GitHub during this execution per "Do NOT commit" directive)
+
+## Exact text applied (Acceptance Criteria section, issue.md)
+
+- [x] AC1: `TimeOutTask_Tests.RunWithTimeout_FuncT1TResult_ShouldReturnResult` (in `UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs`) is made deterministic so it passes consistently under class-level parallelism and under code-coverage instrumentation, not only when run in isolation.
+- [x] AC2: The fix is test-only. No change to `UtilitiesCS/Threading/TimeOutTask.cs` (production timeout semantics unchanged), and the ~775-line production file is not grown. If any other `TimeOutTask` timing test shares the same wall-clock/thread-pool sensitivity, it may be stabilized in the same test-only change.
+- [x] AC3: The fix uses an established repository pattern for timing-sensitive tests — `[DoNotParallelize]` on the affected test class and/or a robust timing approach that does not depend on a tight wall-clock window for trivially-completing work — consistent with `ApplicationIdleTimer_Tests`, `TimerWrapper_Tests`, and the existing generous-timeout precedent in `TimeOutTask_Tests`.
+- [x] AC4: The assertion intent is preserved (the test still verifies that `RunWithTimeout` returns the function's result for the success path). Assertions are not weakened or removed.
+- [x] AC5: Determinism is demonstrated: the affected test(s) pass across repeated runs under class-level parallelism (capture evidence). No other test is regressed.
+- [x] AC6: C# toolchain passes in order — CSharpier -> .NET analyzers -> nullable -> MSTest (vstest) — for the changed test assembly, with no new analyzer/nullable diagnostics and no coverage regression on changed lines.
+
+## Evidence references
+- AC1: evidence/qa-gates/qa-04-test-coverage.md (affected test passed under parallel + coverage); evidence/regression-testing/determinism-repeated-runs.md.
+- AC2: git diff — only two test files changed, 0 production files (TimeOutTask.cs unchanged).
+- AC3: evidence/baseline/precedent-capture.md (ApplicationIdleTimer_Tests [DoNotParallelize] + TimeOutTask_Tests 5000 ms precedents); the change applies both.
+- AC4: TimeOutTask_AdditionalTests.cs — `result.Should().Be("result-42")` preserved.
+- AC5: evidence/regression-testing/determinism-repeated-runs.md (13/13 passes, 0 TimeoutException).
+- AC6: evidence/qa-gates/qa-01-csharpier.md, qa-02-analyzers.md, qa-03-nullable.md, qa-04-test-coverage.md, qa-05-coverage-delta.md.
+
+Note: GitHub issue body was not edited and no commit was made, per the execution directive ("Do NOT commit"). PostedAs reflects the local feature issue.md update only.