From 2e13930653015aefaebd5361c0b3571edb8fc506 Mon Sep 17 00:00:00 2001 From: Dan Moisan Date: Fri, 14 Aug 2026 08:55:50 -0400 Subject: [PATCH 1/3] (chore): update claude --- .claude/rules/csharp.md | 8 ++++---- .claude/skills/csharp-qa-gate/SKILL.md | 8 +++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/.claude/rules/csharp.md b/.claude/rules/csharp.md index f0a5f9070..143866c58 100644 --- a/.claude/rules/csharp.md +++ b/.claude/rules/csharp.md @@ -11,9 +11,9 @@ This rule file summarizes the C#-specific policies for this repository. ## Toolchain -1. **Formatting — CSharpier**: All C# source files must be formatted with CSharpier. Do not use `dotnet format`. Command: `dotnet tool run csharpier .` or `csharpier .` -2. **Linting — .NET Analyzers**: C# code must pass Roslyn/.NET analyzer diagnostics. Command: `msbuild .sln /t:Build /p:Configuration=Debug /p:Platform="Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true` -3. **Type Checking — Nullable Analysis**: Enable nullable reference types and fail on warnings. Command: `msbuild .sln /t:Build /p:Configuration=Debug /p:Platform="Any CPU" /p:Nullable=enable /p:TreatWarningsAsErrors=true` +1. **Formatting — CSharpier**: All C# source files must be formatted with CSharpier. Do not use `dotnet format`. Run `dotnet tool restore` first when the manifest tool has not been restored. Apply formatting with `dotnet tool run csharpier format .` and verify read-only with `dotnet tool run csharpier check .`. Always invoke through `dotnet tool run` so the manifest-pinned CSharpier version is used. +2. **Linting — .NET Analyzers**: C# code must pass Roslyn/.NET analyzer diagnostics. Command: `msbuild .sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true`. `/t:Rebuild` is intentional for a warm local worktree: `/t:Build` can skip `CoreCompile` through MSBuild incrementality and exit 0 without running analyzers. CI may retain `/t:Build` on a cold checkout. +3. **Type Checking — Nullable Analysis**: Compiler and nullable-flow diagnostics must pass with warnings as errors. Command: `msbuild .sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true`. `/t:Rebuild` is required locally so compiler and nullable-flow diagnostics actually run. Projects opt into nullable per file with `#nullable enable`; do not pass `/p:Nullable=enable`, which opts every unannotated file in at once. 4. **Testing — MSTest + Moq + FluentAssertions**: Run tests with: `vstest.console.exe /EnableCodeCoverage` Run the toolchain in order: format → lint → type-check → test. Restart from step 1 if any step fails or changes files. @@ -80,7 +80,7 @@ This repository adopts a fixed set of FIVE static-analysis packages, wired into ### Severity-first ordering invariant -All new analyzer rule severities are configured in `.editorconfig` at `severity = suggestion` (never `warning`/`error`) BEFORE any `` item is wired into a project. This is required because the type-check toolchain step runs `msbuild ... /p:Nullable=enable /p:TreatWarningsAsErrors=true`, which promotes any `warning`-severity analyzer diagnostic to a build error. Keeping new analyzer diagnostics at `suggestion` (message level) prevents the analyzer adoption from breaking the protected nullable gate. +All new analyzer rule severities are configured in `.editorconfig` at `severity = suggestion` (never `warning`/`error`) BEFORE any `` item is wired into a project. This is required because the type-check toolchain step runs `msbuild ... /p:TreatWarningsAsErrors=true`, which promotes any `warning`-severity analyzer diagnostic to a build error. Keeping new analyzer diagnostics at `suggestion` (message level) prevents the analyzer adoption from breaking the protected nullable gate. ### Deferred analyzer — SecurityCodeScan.VS2019 diff --git a/.claude/skills/csharp-qa-gate/SKILL.md b/.claude/skills/csharp-qa-gate/SKILL.md index 43ded1e93..0d3b005a0 100644 --- a/.claude/skills/csharp-qa-gate/SKILL.md +++ b/.claude/skills/csharp-qa-gate/SKILL.md @@ -27,11 +27,13 @@ Before invoking this gate, the agent must have: Run the full toolchain in this exact order. If any step fails or modifies files, fix the issue and restart from step 1. Do not stop the loop until all four steps complete without errors in a single pass. -1. `dotnet tool run csharpier .` -2. `msbuild .sln /t:Build /p:Configuration=Debug /p:Platform="Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true` -3. `msbuild .sln /t:Build /p:Configuration=Debug /p:Platform="Any CPU" /p:Nullable=enable /p:TreatWarningsAsErrors=true` +1. `dotnet tool restore` (when the manifest tool has not been restored), then `dotnet tool run csharpier format .` to apply formatting and `dotnet tool run csharpier check .` to verify read-only. +2. `msbuild .sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true` +3. `msbuild .sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true` 4. `vstest.console.exe /EnableCodeCoverage` +`/t:Rebuild /m` is intentional for the local gate: on a warm worktree `/t:Build` can skip `CoreCompile` through MSBuild incrementality and exit 0 without running analyzers or the compiler. CI may retain `/t:Build` on a cold checkout. Projects opt into nullable per file with `#nullable enable`; do not pass `/p:Nullable=enable`. + If the environment prevents running any tool, stop and report the change as **unverified**. Do not declare completion. ## Delta Requirements (Zero-Regression Hard Gate) From 2b8ff3ef52f5f976e977578408ece0144dc0d0a2 Mon Sep 17 00:00:00 2001 From: Dan Moisan Date: Fri, 14 Aug 2026 09:17:30 -0400 Subject: [PATCH 2/3] fix(tests): remove wall-clock race in TimeOutTask proxy-propagation tests PR #552 CI failed on TimeoutAfter_GenericTask_ShouldPropagateFaultedSourceException_WhenSourceFaultsLater with "Expected a to be thrown, but found ". The PR diff is documentation-only; the defect is a pre-existing race in the test, not a regression introduced by the change. The four source-completes-later tests build the TimeoutAfter proxy with a 100 ms timeout and then complete the source on the very next statement. That arms a 100 ms timer whose callback calls TrySetException(TimeoutException) on the proxy. Whichever of the timer callback and the source completion arrives first wins the proxy. Under the CI runner's parallel execution plus coverage instrumentation the runner thread was preempted for longer than 100 ms between the two statements, so the timer won and the proxy surfaced TimeoutException. Fix: replace the 100 ms literal with NonElapsingTimeoutMs (int.MaxValue). The value stays finite, so TimeoutAfter still builds the proxy and exercises MarshalTaskResults rather than short-circuiting on Timeout.Infinite, but the timer cannot elapse during a test run. No assertion is weakened, no sleep or retry is introduced, and production TimeOutTask.cs is unchanged - its timeout-versus-completion race is correct behavior. Verification: the failure was reproduced deterministically by temporarily inserting a 300 ms stall between proxy creation and source completion, which produced the exact CI error; with the fix in place and the same stall, the test passed. The stall was removed before commit. Full toolchain clean in a single pass: csharpier format/check, msbuild /t:Rebuild with analyzers and code style, msbuild /t:Rebuild with TreatWarningsAsErrors, and the full MSTest suite with coverage (6435/6435 passed). Co-Authored-By: Claude Opus 5 (1M context) --- .../Threading/TimeOutTask_AdditionalTests.cs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs b/UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs index 71d2c69d6..4a868718f 100644 --- a/UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs +++ b/UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs @@ -8,12 +8,22 @@ namespace UtilitiesCS.Test { public partial class TimeOutTask_Tests { + /// + /// A finite timeout that cannot elapse during a test run. The source-completes-later + /// tests need a finite value so TimeoutAfter builds the proxy and exercises + /// MarshalTaskResults rather than short-circuiting on . + /// A short window (previously 100 ms) races the source completion: when the runner thread + /// is preempted for longer than the window, the timeout timer faults the proxy with + /// before the source result is marshalled. + /// + private const int NonElapsingTimeoutMs = int.MaxValue; + [TestMethod] public async Task TimeoutAfter_GenericTask_ShouldPropagateFaultedSourceException_WhenSourceFaultsLater() { // Arrange var source = new TaskCompletionSource(); - var proxy = source.Task.TimeoutAfter(100); + var proxy = source.Task.TimeoutAfter(NonElapsingTimeoutMs); // Act source.SetException(new InvalidOperationException("boom")); @@ -28,7 +38,7 @@ public async Task TimeoutAfter_GenericTask_ShouldPropagateCancellation_WhenSourc { // Arrange var source = new TaskCompletionSource(); - var proxy = source.Task.TimeoutAfter(100); + var proxy = source.Task.TimeoutAfter(NonElapsingTimeoutMs); // Act source.SetCanceled(); @@ -43,7 +53,7 @@ public async Task TimeoutAfter_NonGenericTask_ShouldPropagateFaultedSourceExcept { // Arrange var source = new TaskCompletionSource(); - var proxy = ((Task)source.Task).TimeoutAfter(100); + var proxy = ((Task)source.Task).TimeoutAfter(NonElapsingTimeoutMs); // Act source.SetException(new InvalidOperationException("boom")); @@ -58,7 +68,7 @@ public async Task TimeoutAfter_NonGenericTask_ShouldPropagateCancellation_WhenSo { // Arrange var source = new TaskCompletionSource(); - var proxy = ((Task)source.Task).TimeoutAfter(100); + var proxy = ((Task)source.Task).TimeoutAfter(NonElapsingTimeoutMs); // Act source.SetCanceled(); From d208fa6804feeb3c7b356501b9782aefc699f38e Mon Sep 17 00:00:00 2001 From: Dan Moisan Date: Fri, 14 Aug 2026 09:27:03 -0400 Subject: [PATCH 3/3] fix(tests): drive TimeoutAfter timeout from an injectable TimeProvider seam Replaces the previous commit's int.MaxValue timeout constant, which removed the observed flake by making the timeout window enormous rather than by controlling time. That is a magic-number workaround, not determinism infrastructure, and it does not satisfy the general unit test policy requirement that async tests use a fake-timer facility to advance simulated time deterministically. TimeOutTask.TimeoutAfter arms the timeout with new Timer(...) directly, so there was no seam a test could control. This adds one, matching the pattern already used by UtilitiesCS/Threading/ThreadMonitor.cs in the same folder: both TimeoutAfter(Task, int) and TimeoutAfter(Task, int) take an optional TimeProvider that defaults to TimeProvider.System, and the timer is created via TimeProvider.CreateTimer instead of the Timer constructor. Microsoft.Bcl. TimeProvider is already referenced by UtilitiesCS and Microsoft.Extensions. TimeProvider.Testing by UtilitiesCS.Test, so no new dependency is introduced. The parameter is optional, so all existing call sites are unchanged and production behavior is identical. The four source-completes-later tests now pass a FakeTimeProvider that is never advanced. The timeout timer cannot fire, so the source result is provably the one marshalled to the proxy rather than merely probably so. Two positive-control tests are added that advance the fake clock past the deadline and assert the proxy faults with TimeoutException; without them a mis-wired seam that never armed the timer would pass unnoticed. Full toolchain clean in a single pass: csharpier check, msbuild /t:Rebuild with analyzers and code style, msbuild /t:Rebuild with TreatWarningsAsErrors, and the full MSTest suite with coverage (6437/6437 passed, up two for the new controls). Co-Authored-By: Claude Opus 5 (1M context) --- .../Threading/TimeOutTask_AdditionalTests.cs | 55 +++++++++++++++---- UtilitiesCS/Threading/TimeOutTask.cs | 37 +++++++++---- 2 files changed, 71 insertions(+), 21 deletions(-) diff --git a/UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs b/UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs index 4a868718f..281f91f94 100644 --- a/UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs +++ b/UtilitiesCS.Test/Threading/TimeOutTask_AdditionalTests.cs @@ -2,6 +2,7 @@ using System.Threading; using System.Threading.Tasks; using FluentAssertions; +using Microsoft.Extensions.Time.Testing; using Microsoft.VisualStudio.TestTools.UnitTesting; namespace UtilitiesCS.Test @@ -9,21 +10,21 @@ namespace UtilitiesCS.Test public partial class TimeOutTask_Tests { /// - /// A finite timeout that cannot elapse during a test run. The source-completes-later - /// tests need a finite value so TimeoutAfter builds the proxy and exercises - /// MarshalTaskResults rather than short-circuiting on . - /// A short window (previously 100 ms) races the source completion: when the runner thread - /// is preempted for longer than the window, the timeout timer faults the proxy with - /// before the source result is marshalled. + /// The source-completes-later tests race the timeout timer against the source completion. + /// Driving TimeoutAfter from a that is never advanced + /// makes the outcome deterministic: the timer cannot fire, so the source result is always + /// the one marshalled to the proxy. A real-clock timeout leaves the outcome dependent on + /// how long the runner thread is preempted between arming the timer and completing the + /// source, which is what failed under CI parallelism plus coverage instrumentation. /// - private const int NonElapsingTimeoutMs = int.MaxValue; + private static FakeTimeProvider FrozenClock() => new FakeTimeProvider(); [TestMethod] public async Task TimeoutAfter_GenericTask_ShouldPropagateFaultedSourceException_WhenSourceFaultsLater() { // Arrange var source = new TaskCompletionSource(); - var proxy = source.Task.TimeoutAfter(NonElapsingTimeoutMs); + var proxy = source.Task.TimeoutAfter(100, FrozenClock()); // Act source.SetException(new InvalidOperationException("boom")); @@ -38,7 +39,7 @@ public async Task TimeoutAfter_GenericTask_ShouldPropagateCancellation_WhenSourc { // Arrange var source = new TaskCompletionSource(); - var proxy = source.Task.TimeoutAfter(NonElapsingTimeoutMs); + var proxy = source.Task.TimeoutAfter(100, FrozenClock()); // Act source.SetCanceled(); @@ -53,7 +54,7 @@ public async Task TimeoutAfter_NonGenericTask_ShouldPropagateFaultedSourceExcept { // Arrange var source = new TaskCompletionSource(); - var proxy = ((Task)source.Task).TimeoutAfter(NonElapsingTimeoutMs); + var proxy = ((Task)source.Task).TimeoutAfter(100, FrozenClock()); // Act source.SetException(new InvalidOperationException("boom")); @@ -68,7 +69,7 @@ public async Task TimeoutAfter_NonGenericTask_ShouldPropagateCancellation_WhenSo { // Arrange var source = new TaskCompletionSource(); - var proxy = ((Task)source.Task).TimeoutAfter(NonElapsingTimeoutMs); + var proxy = ((Task)source.Task).TimeoutAfter(100, FrozenClock()); // Act source.SetCanceled(); @@ -78,6 +79,38 @@ public async Task TimeoutAfter_NonGenericTask_ShouldPropagateCancellation_WhenSo await act.Should().ThrowAsync(); } + [TestMethod] + public async Task TimeoutAfter_GenericTask_ShouldFaultWithTimeout_WhenInjectedClockPassesTheDeadline() + { + // Arrange + var clock = FrozenClock(); + var source = new TaskCompletionSource(); + var proxy = source.Task.TimeoutAfter(100, clock); + + // Act + clock.Advance(TimeSpan.FromMilliseconds(100)); + + // Assert + Func act = async () => await proxy; + await act.Should().ThrowAsync(); + } + + [TestMethod] + public async Task TimeoutAfter_NonGenericTask_ShouldFaultWithTimeout_WhenInjectedClockPassesTheDeadline() + { + // Arrange + var clock = FrozenClock(); + var source = new TaskCompletionSource(); + var proxy = ((Task)source.Task).TimeoutAfter(100, clock); + + // Act + clock.Advance(TimeSpan.FromMilliseconds(100)); + + // Assert + Func act = async () => await proxy; + await act.Should().ThrowAsync(); + } + [TestMethod] public async Task RunWithTimeout_Func_ShouldReturnDefault_WhenTaskIsCanceledWithoutRetries() { diff --git a/UtilitiesCS/Threading/TimeOutTask.cs b/UtilitiesCS/Threading/TimeOutTask.cs index f9e97dcf6..22cefecc6 100644 --- a/UtilitiesCS/Threading/TimeOutTask.cs +++ b/UtilitiesCS/Threading/TimeOutTask.cs @@ -835,10 +835,16 @@ int repeatAttempts /// /// /// + /// + /// Clock used to arm the timeout timer. When null, is used + /// (production); tests pass a FakeTimeProvider so the timeout fires only when the fake + /// clock is advanced, making the timeout-versus-completion race deterministic. + /// /// public static Task TimeoutAfter( this Task task, - int millisecondsTimeout + int millisecondsTimeout, + TimeProvider? timeProvider = null ) { // Short-circuit #1: infinite timeout or task already completed @@ -861,7 +867,7 @@ int millisecondsTimeout } // Set up a timer to complete after the specified timeout period - Timer timer = new Timer( + ITimer timer = (timeProvider ?? TimeProvider.System).CreateTimer( state => { // Recover your state information @@ -871,8 +877,8 @@ int millisecondsTimeout myTcs.TrySetException(new TimeoutException()); }, tcs, - millisecondsTimeout, - Timeout.Infinite + TimeSpan.FromMilliseconds(millisecondsTimeout), + Timeout.InfiniteTimeSpan ); // Wire up the logic for what happens when source task completes @@ -880,7 +886,7 @@ int millisecondsTimeout (antecedent, state) => { // Recover our state data - var tuple = (Tuple>)state; + var tuple = (Tuple>)state; // Cancel the Timer tuple.Item1.Dispose(); @@ -915,7 +921,18 @@ public static Task TimeoutAfter(this Task task, int millisecondsTimeout, int rep return result!; } - public static Task TimeoutAfter(this Task task, int millisecondsTimeout) + /// + /// + /// + /// Clock used to arm the timeout timer. When null, is used + /// (production); tests pass a FakeTimeProvider so the timeout fires only when the fake + /// clock is advanced, making the timeout-versus-completion race deterministic. + /// + public static Task TimeoutAfter( + this Task task, + int millisecondsTimeout, + TimeProvider? timeProvider = null + ) { // Short-circuit #1: infinite timeout or task already completed if (task.IsCompleted || (millisecondsTimeout == Timeout.Infinite)) @@ -937,7 +954,7 @@ public static Task TimeoutAfter(this Task task, int millisecondsTimeout) } // Set up a timer to complete after the specified timeout period - Timer timer = new Timer( + ITimer timer = (timeProvider ?? TimeProvider.System).CreateTimer( state => { // Recover your state information @@ -947,8 +964,8 @@ public static Task TimeoutAfter(this Task task, int millisecondsTimeout) myTcs.TrySetException(new TimeoutException()); }, tcs, - millisecondsTimeout, - Timeout.Infinite + TimeSpan.FromMilliseconds(millisecondsTimeout), + Timeout.InfiniteTimeSpan ); // Wire up the logic for what happens when source task completes @@ -956,7 +973,7 @@ public static Task TimeoutAfter(this Task task, int millisecondsTimeout) (antecedent, state) => { // Recover our state data - var tuple = (Tuple>)state; + var tuple = (Tuple>)state; // Cancel the Timer tuple.Item1.Dispose();