From 9acc6c1e55a3274859d2cdcad85694f45974532c Mon Sep 17 00:00:00 2001 From: Ivan Vydrin Date: Wed, 30 Sep 2026 02:17:37 +0300 Subject: [PATCH 1/5] Keep workflow dispatch values out of shell code --- .github/workflows/ci.yml | 4 +++- .github/workflows/publish.yml | 4 +++- .github/workflows/refresh-lock-files.yml | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0673f78..f6d97c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,10 +33,12 @@ jobs: # the lock files were not regenerated, which is a chore rather than a defect -- and the # error NuGet prints for it does not mention the fix. - name: Restore + env: + REF_NAME: ${{ github.head_ref || github.ref_name }} run: | set -euo pipefail if ! dotnet restore --locked-mode; then - echo "::error title=Lock files are out of step::dotnet restore --locked-mode failed. If this is a dependency bump, Dependabot cannot rewrite packages.lock.json under central package management. Run the 'Refresh lock files' workflow against ${{ github.head_ref || github.ref_name }}, then re-run these checks." + echo "::error title=Lock files are out of step::dotnet restore --locked-mode failed. If this is a dependency bump, Dependabot cannot rewrite packages.lock.json under central package management. Run the 'Refresh lock files' workflow against $REF_NAME, then re-run these checks." exit 1 fi diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 4aed047..1b9237f 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -61,11 +61,13 @@ jobs: - name: Resolve version id: version + env: + DISPATCH_VERSION: ${{ inputs.version }} run: | set -euo pipefail if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then - VERSION="${{ inputs.version }}" + VERSION="$DISPATCH_VERSION" else VERSION="${GITHUB_REF#refs/tags/v}" fi diff --git a/.github/workflows/refresh-lock-files.yml b/.github/workflows/refresh-lock-files.yml index 3e00444..a88a366 100644 --- a/.github/workflows/refresh-lock-files.yml +++ b/.github/workflows/refresh-lock-files.yml @@ -62,6 +62,8 @@ jobs: - run: dotnet build --no-restore -c Release - name: Commit the refreshed lock files + env: + BRANCH: ${{ inputs.branch }} run: | set -euo pipefail # Named paths, never `git add -A`: this job can write, so it says exactly what it writes. @@ -77,7 +79,7 @@ jobs: Regenerated with `dotnet restore --force-evaluate`, then verified with `--locked-mode` and a Release build. Dependabot cannot write these under central package management; see .github/workflows/refresh-lock-files.yml.' - git push origin HEAD:'${{ inputs.branch }}' + git push origin "HEAD:$BRANCH" cat <<'NOTE' Pushed. From 766ed47edd49e8b5fdd13e852b139fc748e5f184 Mon Sep 17 00:00:00 2001 From: Ivan Vydrin Date: Wed, 30 Sep 2026 02:19:24 +0300 Subject: [PATCH 2/5] Fix checker races and reduce state-store reads --- src/Healthie.AI/PulseDiagnostician.cs | 10 +- src/Healthie.Abstractions/PulseChecker.cs | 26 +++- .../Scheduling/PulsesScheduler.cs | 21 ++- .../TimerPulseScheduler.cs | 41 +++++- .../Healthie.Tests.Unit/AiDiagnosticsTests.cs | 139 ++++++++++++++++++ .../Healthie.Tests.Unit/BulkStateReadTests.cs | 68 +++++++++ tests/Healthie.Tests.Unit/ConcurrencyTests.cs | 41 ++++++ tests/Healthie.Tests.Unit/DiagnosticsTests.cs | 68 +++++++++ tests/Healthie.Tests.Unit/TestDoubles.cs | 17 ++- .../TimerPulseSchedulerScheduleTests.cs | 102 +++++++++++++ 10 files changed, 512 insertions(+), 21 deletions(-) diff --git a/src/Healthie.AI/PulseDiagnostician.cs b/src/Healthie.AI/PulseDiagnostician.cs index 22efcb3..87b3d91 100644 --- a/src/Healthie.AI/PulseDiagnostician.cs +++ b/src/Healthie.AI/PulseDiagnostician.cs @@ -1,3 +1,4 @@ +using Healthie.Abstractions; using Healthie.Abstractions.Models; using Healthie.Abstractions.Scheduling; using Microsoft.Extensions.AI; @@ -44,14 +45,17 @@ public async Task DiagnoseAsync(string name, CancellationToken c { ArgumentException.ThrowIfNullOrWhiteSpace(name); - var states = await _pulsesScheduler.GetPulsesStatesAsync(cancellationToken).ConfigureAwait(false); + var checkers = await _pulsesScheduler.GetPulseCheckersAsync(cancellationToken).ConfigureAwait(false); - if (!states.TryGetValue(name, out var state)) + if (!checkers.TryGetValue(name, out var checker)) { throw new ArgumentException($"Pulse checker with name '{name}' not found.", nameof(name)); } - var history = await _pulsesScheduler.GetHistoryAsync(name, cancellationToken).ConfigureAwait(false); + var state = await checker.GetStateAsync(cancellationToken).ConfigureAwait(false); + var history = checker is PulseChecker + ? state.History + : await checker.GetHistoryAsync(cancellationToken).ConfigureAwait(false); if (history.Count == 0) { diff --git a/src/Healthie.Abstractions/PulseChecker.cs b/src/Healthie.Abstractions/PulseChecker.cs index 9f0af16..72d94d5 100644 --- a/src/Healthie.Abstractions/PulseChecker.cs +++ b/src/Healthie.Abstractions/PulseChecker.cs @@ -156,7 +156,7 @@ protected PulseChecker( /// /// Builds the state a checker starts from when the store holds nothing for it yet. /// - private PulseCheckerState CreateInitialState() => + internal PulseCheckerState CreateInitialState() => new(_initialInterval, _initialUnhealthyThreshold) { Schedule = _initialSchedule, @@ -505,10 +505,32 @@ public async Task TriggerAsync(CancellationToken cancellationToken = default) var result = await RunCheckAsync(cancellationToken).ConfigureAwait(false); var elapsed = Stopwatch.GetElapsedTime(startedAt); + var staleResult = false; var (oldState, state, changed) = await ApplyAsync( - current => RecordResult(current, result, executedAt), + current => + { + // Another replica can finish a check that started later while this one is + // still running. Its result is newer by definition, so never replace it with + // this older execution or move the last-executed time backwards. + staleResult = current.LastExecutionDateTime is { } lastExecuted && lastExecuted > executedAt; + if (staleResult) + { + return; + } + + RecordResult(current, result, executedAt); + }, cancellationToken).ConfigureAwait(false); + // A later-started execution already recorded its result. This check did run, but it is + // not the state currently being monitored, so it must not emit a state transition or + // count as a persisted health result. The flag is assigned on every CAS attempt, so a + // conflict retry cannot inherit the outcome of an earlier attempt. + if (staleResult) + { + return; + } + // Recorded after the write, so a check whose state could not be stored is not counted // as one that ran -- the same reason a storage failure is not a health result. RecordTelemetry(activity, state, oldState, elapsed); diff --git a/src/Healthie.Abstractions/Scheduling/PulsesScheduler.cs b/src/Healthie.Abstractions/Scheduling/PulsesScheduler.cs index c2b9e9b..406ffd2 100644 --- a/src/Healthie.Abstractions/Scheduling/PulsesScheduler.cs +++ b/src/Healthie.Abstractions/Scheduling/PulsesScheduler.cs @@ -63,9 +63,9 @@ public Task> GetPulseCheckersAsync(Cancellatio /// /// /// Reads the whole set in one call where the store can, which is what every dashboard load and - /// every list request does. A checker with nothing stored yet is still asked individually -- - /// only it can build its own initial state, seeded from the tags and group it declares -- but - /// that is once per checker in its life, not once per page. + /// every list request does. A built-in checker with nothing stored yet creates its initial + /// state locally, preserving its declared tags and group without asking the store again. A + /// custom checker still supplies its own state. /// public async Task> GetPulsesStatesAsync(CancellationToken cancellationToken = default) { @@ -82,9 +82,18 @@ public async Task> GetPulsesStatesAsync(Ca foreach (var checker in checkers) { - states[checker.Name] = stored.TryGetValue(checker.Name, out var state) - ? state - : await checker.GetStateAsync(cancellationToken).ConfigureAwait(false); + if (stored.TryGetValue(checker.Name, out var state)) + { + states[checker.Name] = state; + } + else if (_stateProvider is not null && checker is PulseChecker builtInChecker) + { + states[checker.Name] = builtInChecker.CreateInitialState(); + } + else + { + states[checker.Name] = await checker.GetStateAsync(cancellationToken).ConfigureAwait(false); + } } return states; diff --git a/src/Healthie.DependencyInjection/TimerPulseScheduler.cs b/src/Healthie.DependencyInjection/TimerPulseScheduler.cs index 361863e..700d870 100644 --- a/src/Healthie.DependencyInjection/TimerPulseScheduler.cs +++ b/src/Healthie.DependencyInjection/TimerPulseScheduler.cs @@ -2,6 +2,7 @@ using Healthie.Abstractions; using Healthie.Abstractions.Enums; using Healthie.Abstractions.Scheduling; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using System.Collections.Concurrent; @@ -25,6 +26,8 @@ public sealed class TimerPulseScheduler : IPulseScheduler, IAsyncDisposable, IDi private const double MaxTimerMilliseconds = uint.MaxValue; private readonly ConcurrentDictionary _timers = new(); + private readonly CancellationTokenSource _shutdown = new(); + private readonly CancellationTokenRegistration _applicationStoppingRegistration; /// /// Serialises scheduling, so a checker is never left running under a timer nobody holds. @@ -50,6 +53,21 @@ public TimerPulseScheduler(ILogger? logger = null) _logger = logger; } + /// + /// Initializes a new instance of the class that stops its + /// timers when the host stops. + /// + /// An optional logger for diagnostic output. + /// The host lifetime that stops locally managed timers. + public TimerPulseScheduler( + ILogger? logger, + IHostApplicationLifetime applicationLifetime) + : this(logger) + { + ArgumentNullException.ThrowIfNull(applicationLifetime); + _applicationStoppingRegistration = applicationLifetime.ApplicationStopping.Register(_shutdown.Cancel); + } + /// public Task ScheduleAsync( IPulseChecker checker, @@ -86,25 +104,29 @@ public async Task ScheduleAsync( // Stopping the old schedule and installing the new one is two steps, and two callers // scheduling one checker at once -- an interval changed from the dashboard while the // scheduler starts it, say -- could each install a timer. Only the last would be in the - // dictionary; the other kept running with nothing able to reach it, and its linked - // CancellationTokenSource was never disposed, so its registration on the parent token - // outlived it too. + // dictionary; the other kept running with nothing able to reach it, and its cancellation + // source was never disposed. await _scheduling.WaitAsync(cancellationToken).ConfigureAwait(false); try { + // The caller can cancel while this request waits for another scheduling operation. + // Once a schedule is installed, its lifetime belongs to this scheduler instead: the + // host's ApplicationStopping token, unscheduling, and replacement stop it. + cancellationToken.ThrowIfCancellationRequested(); + // A request can outlive the decision to shut down -- an interval changed from the API - // while the host stops. Installing a timer now would leave one running that Dispose has - // already been past, so there would be nothing left to stop it. Quietly rather than by - // throwing: the host is going away, and that is not the caller's mistake. - if (_disposed) + // while the host stops. Installing a timer now would leave one running that nothing + // would own, so quietly refuse it: the host is going away, and that is not the caller's + // mistake. + if (_disposed || _shutdown.IsCancellationRequested) { return; } StopExisting(checker.Name); - var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + var cts = CancellationTokenSource.CreateLinkedTokenSource(_shutdown.Token); _timers[checker.Name] = cts; _ = Task.Run( @@ -349,6 +371,9 @@ private static CronExpression ParseCron(string expression, string checkerName) /// public void Dispose() { + _applicationStoppingRegistration.Unregister(); + _shutdown.Cancel(); + // Shutdown races the last requests: an interval can be changed from the API while the host // is stopping. Without the lock this is a third writer of _timers, and clearing the // dictionary between an in-flight schedule's stop and its install drops a live timer without diff --git a/tests/Healthie.Tests.Unit/AiDiagnosticsTests.cs b/tests/Healthie.Tests.Unit/AiDiagnosticsTests.cs index b97d0d2..3ba120c 100644 --- a/tests/Healthie.Tests.Unit/AiDiagnosticsTests.cs +++ b/tests/Healthie.Tests.Unit/AiDiagnosticsTests.cs @@ -1,6 +1,8 @@ +using Healthie.Abstractions; using Healthie.Abstractions.Enums; using Healthie.Abstractions.Models; using Healthie.Abstractions.Scheduling; +using Healthie.Abstractions.StateProviding; using Healthie.AI; using Healthie.DependencyInjection; using Microsoft.Extensions.AI; @@ -18,6 +20,24 @@ public class AiDiagnosticsTests private const string UnhealthyChecker = "Healthie.Tests.Unit.AlwaysUnhealthyPulseChecker"; + private sealed class CountingStateProvider : IStateProvider + { + private readonly InMemoryStateProvider _inner = new(); + + public int StateReads { get; private set; } + + public async Task GetStateAsync(string name, CancellationToken cancellationToken = default) + { + StateReads++; + return await _inner.GetStateAsync(name, cancellationToken).ConfigureAwait(false); + } + + public Task SetStateAsync(string name, TState state, CancellationToken cancellationToken = default) + => _inner.SetStateAsync(name, state, cancellationToken); + + public void ResetReads() => StateReads = 0; + } + private static async Task CreateSchedulerAsync(int runs) { var services = new ServiceCollection(); @@ -95,6 +115,61 @@ public async Task DiagnoseAsync_ForAnUnknownChecker_Throws() await Assert.ThrowsAsync(() => diagnostician.DiagnoseAsync("nope", Ct)); } + [Fact] + public async Task DiagnoseAsync_ReadsOnlyTheNamedChecker() + { + await using var checker = new FakePulseChecker("named"); + var state = new PulseCheckerState(PulseInterval.EveryMinute) + { + History = [new PulseCheckerHistoryEntry(PulseCheckerHealth.Unhealthy, "down", DateTime.UtcNow)], + }; + await checker.SetStateAsync(state, Ct); + var scheduler = new LookupOnlyPulsesScheduler(checker, state); + var diagnostician = new PulseDiagnostician(new FakeChatClient("summary"), scheduler); + + await diagnostician.DiagnoseAsync("named", Ct); + + Assert.Equal(1, scheduler.CheckerLookups); + Assert.Equal(0, scheduler.AllStateReads); + Assert.Equal(0, scheduler.HistoryReads); + Assert.Equal(1, checker.StateReadCount); + Assert.Equal(1, checker.HistoryReadCount); + } + + [Fact] + public async Task DiagnoseAsync_UsesTheHistorySuppliedByACustomChecker() + { + await using var checker = new FakePulseChecker("custom"); + var state = new PulseCheckerState(PulseInterval.EveryMinute); + await checker.SetStateAsync(state, Ct); + checker.SetHistory([new PulseCheckerHistoryEntry(PulseCheckerHealth.Unhealthy, "custom history", DateTime.UtcNow)]); + var chatClient = new FakeChatClient("summary"); + var diagnostician = new PulseDiagnostician( + chatClient, + new LookupOnlyPulsesScheduler(checker, state)); + + await diagnostician.DiagnoseAsync("custom", Ct); + + Assert.Equal(1, chatClient.CallCount); + Assert.Contains("custom history", chatClient.LastUserMessage); + } + + [Fact] + public async Task DiagnoseAsync_UsesTheStateHistoryForABuiltInChecker() + { + var provider = new CountingStateProvider(); + await using var checker = new AlwaysUnhealthyPulseChecker(provider); + await checker.TriggerAsync(Ct); + provider.ResetReads(); + var diagnostician = new PulseDiagnostician( + new FakeChatClient("summary"), + new LookupOnlyPulsesScheduler(checker, new PulseCheckerState())); + + await diagnostician.DiagnoseAsync(checker.Name, Ct); + + Assert.Equal(1, provider.StateReads); + } + [Fact] public void AddHealthieAI_RegistersTheDiagnostician() { @@ -109,6 +184,70 @@ public void AddHealthieAI_RegistersTheDiagnostician() } } +internal sealed class LookupOnlyPulsesScheduler(IPulseChecker checker, PulseCheckerState state) : IPulsesScheduler +{ + public int CheckerLookups { get; private set; } + + public int AllStateReads { get; private set; } + + public int HistoryReads { get; private set; } + + public Task> GetPulseCheckersAsync(CancellationToken cancellationToken = default) + { + CheckerLookups++; + return Task.FromResult(new Dictionary { [checker.Name] = checker }); + } + + public Task> GetPulsesStatesAsync(CancellationToken cancellationToken = default) + { + AllStateReads++; + return Task.FromResult(new Dictionary { [checker.Name] = state }); + } + + public Task> GetHistoryAsync(string name, CancellationToken cancellationToken = default) + { + HistoryReads++; + return Task.FromResult(state.History.ToList()); + } + + public Task StartAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + public Task SetIntervalAsync(string name, PulseInterval interval, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task SetScheduleAsync(string name, PulseSchedule? schedule, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task SetUnhealthyThresholdAsync(string name, uint threshold, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task SetTagsAsync(string name, IReadOnlyList tags, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task SetPinnedAsync(string name, bool pinned, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task SetGroupAsync(string name, string? group, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task ResetAsync(string name, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task ActivateAsync(string name, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task DeactivateAsync(string name, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task ClearHistoryAsync(string name, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task SetHistoryEnabledAsync(string name, bool enabled, CancellationToken cancellationToken = default) + => Task.CompletedTask; +} + public class FailureRateAnomalyDetectorTests { private static readonly DateTime Start = new(2026, 7, 15, 9, 0, 0, DateTimeKind.Utc); diff --git a/tests/Healthie.Tests.Unit/BulkStateReadTests.cs b/tests/Healthie.Tests.Unit/BulkStateReadTests.cs index 9b055b6..901071a 100644 --- a/tests/Healthie.Tests.Unit/BulkStateReadTests.cs +++ b/tests/Healthie.Tests.Unit/BulkStateReadTests.cs @@ -1,5 +1,7 @@ +using Healthie.Abstractions; using Healthie.Abstractions.Enums; using Healthie.Abstractions.Models; +using Healthie.Abstractions.Scheduling; using Healthie.Abstractions.StateProviding; using Healthie.DependencyInjection; using Healthie.StateProviding.Relational; @@ -40,6 +42,42 @@ public Task SetStateAsync(string name, TState state, CancellationToken c } } + private sealed class BulkOnlyEmptyProvider : IStateProvider + { + public int BulkReads { get; private set; } + + public int SingleReads { get; private set; } + + public Task GetStateAsync(string name, CancellationToken cancellationToken = default) + { + SingleReads++; + return Task.FromResult(default); + } + + public Task SetStateAsync(string name, TState state, CancellationToken cancellationToken = default) + => Task.CompletedTask; + + public Task> GetStatesAsync( + IEnumerable names, + CancellationToken cancellationToken = default) + { + BulkReads++; + return Task.FromResult>( + new Dictionary(StringComparer.Ordinal)); + } + } + + private sealed class DefaultedPulseChecker(IStateProvider stateProvider) + : PulseChecker(stateProvider, PulseInterval.EveryMinute) + { + public override string? DefaultGroup => " Platform "; + + public override IReadOnlyList DefaultTags => [" cache ", "Critical", "cache"]; + + public override Task CheckAsync(CancellationToken cancellationToken = default) + => Task.FromResult(new PulseCheckerResult(PulseCheckerHealth.Healthy)); + } + [Fact] public async Task AProviderThatDoesNotImplementBulkRead_StillAnswersCorrectly() { @@ -103,6 +141,36 @@ public async Task TheInMemoryProvider_ReadsInBulk() Assert.Equal(PulseInterval.Every3Minutes, states["b"].Interval); } + [Fact] + public async Task ABlankBulkResult_SeedsBuiltInCheckersWithoutIndividualStoreReads() + { + var provider = new BulkOnlyEmptyProvider(); + await using var checker = new DefaultedPulseChecker(provider); + var scheduler = new PulsesScheduler([checker], new CustomPulseScheduler(), new HealthieOptions(), provider); + + var states = await scheduler.GetPulsesStatesAsync(Ct); + + Assert.Equal(1, provider.BulkReads); + Assert.Equal(0, provider.SingleReads); + Assert.Equal("Platform", states[checker.Name].Group); + Assert.Equal(["cache", "Critical"], states[checker.Name].Tags); + } + + [Fact] + public async Task ACustomCheckerWithNoStoredState_StillSuppliesItsOwnState() + { + var provider = new BulkOnlyEmptyProvider(); + await using var checker = new FakePulseChecker("custom"); + var expected = new PulseCheckerState(PulseInterval.Every5Minutes) { Group = "custom" }; + await checker.SetStateAsync(expected, Ct); + var scheduler = new PulsesScheduler([checker], new CustomPulseScheduler(), new HealthieOptions(), provider); + + var states = await scheduler.GetPulsesStatesAsync(Ct); + + Assert.Same(expected, states["custom"]); + Assert.Equal(1, checker.StateReadCount); + } + /// /// Reads must stay independent copies, exactly as a single read does -- StateChanged compares /// the stored state against the one about to be written, and a shared instance makes every diff --git a/tests/Healthie.Tests.Unit/ConcurrencyTests.cs b/tests/Healthie.Tests.Unit/ConcurrencyTests.cs index a20ce57..95b0955 100644 --- a/tests/Healthie.Tests.Unit/ConcurrencyTests.cs +++ b/tests/Healthie.Tests.Unit/ConcurrencyTests.cs @@ -369,6 +369,47 @@ public async Task ACheckWritingItsResult_DoesNotRevertASettingChangedWhileItRan( Assert.Equal("set-while-the-check-ran", state.Group); } + /// + /// Two replicas can legitimately use one storage identity. When the earlier execution finishes + /// after the later one, its stale result must not become the current state or move the recorded + /// execution time backwards. + /// + [Fact] + public async Task AnEarlierExecutionFinishingLate_DoesNotReplaceANewerResult() + { + var store = new InMemoryStateProvider(); + using var earlier = new ControllablePulseChecker(store, name: "shared-checker") + { + NextResult = new PulseCheckerResult(PulseCheckerHealth.Healthy, "earlier"), + }; + using var later = new ControllablePulseChecker(store, name: "shared-checker") + { + NextResult = new PulseCheckerResult(PulseCheckerHealth.Unhealthy, "later"), + }; + var releaseEarlier = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var staleEvents = 0; + earlier.OnCheck = () => releaseEarlier.Task; + earlier.StateChanged += (_, _) => staleEvents++; + + var first = earlier.TriggerAsync(Ct); + await earlier.WaitUntilCheckStartedAsync(); + await Task.Delay(20, Ct); + + await later.TriggerAsync(Ct); + var afterLater = await store.GetStateAsync("shared-checker", Ct); + + releaseEarlier.SetResult(); + await first; + + var final = await store.GetStateAsync("shared-checker", Ct); + + Assert.Equal(PulseCheckerHealth.Unhealthy, final!.LastResult!.Health); + Assert.Equal(afterLater!.LastExecutionDateTime, final.LastExecutionDateTime); + Assert.Equal(1, final.ConsecutiveFailureCount); + Assert.Equal(PulseCheckerHealth.Unhealthy, Assert.Single(final.History).Health); + Assert.Equal(0, staleEvents); + } + /// /// Clearing history changes one field of a shared state document. It must use the same /// conditional-write loop as every setting mutation, or a result or setting written between diff --git a/tests/Healthie.Tests.Unit/DiagnosticsTests.cs b/tests/Healthie.Tests.Unit/DiagnosticsTests.cs index f741959..25058dd 100644 --- a/tests/Healthie.Tests.Unit/DiagnosticsTests.cs +++ b/tests/Healthie.Tests.Unit/DiagnosticsTests.cs @@ -50,6 +50,44 @@ public override async Task CheckAsync(CancellationToken canc } } + /// + /// Makes a completed write appear to have won in another process. The retry then sees the + /// exact result this checker was about to store, which is a non-stale state no-op. + /// + private sealed class FirstWriteWinsElsewhereProvider(IStateProvider inner) : IStateProvider + { + private bool _firstWrite = true; + + public bool SupportsOptimisticConcurrency => inner.SupportsOptimisticConcurrency; + + public Task GetStateAsync(string name, CancellationToken cancellationToken = default) => + inner.GetStateAsync(name, cancellationToken); + + public Task?> GetStateEntryAsync( + string name, + CancellationToken cancellationToken = default) => + inner.GetStateEntryAsync(name, cancellationToken); + + public Task SetStateAsync(string name, TState state, CancellationToken cancellationToken = default) => + inner.SetStateAsync(name, state, cancellationToken); + + public async Task TrySetStateAsync( + string name, + TState state, + string? expectedVersion, + CancellationToken cancellationToken = default) + { + if (!_firstWrite) + { + return await inner.TrySetStateAsync(name, state, expectedVersion, cancellationToken); + } + + _firstWrite = false; + await inner.TrySetStateAsync(name, state, expectedVersion, cancellationToken); + return false; + } + } + private readonly record struct Measurement(string Instrument, double Value, Dictionary Tags); private readonly List _measurements = []; @@ -149,6 +187,36 @@ public async Task ACheck_RecordsItsDurationAndItsResult() Assert.Equal("Healthy", result.Tags[HealthieDiagnostics.ResultTag]); } + /// + /// A CAS retry can find that another replica stored the exact same result. It is not stale, so + /// it still represents a completed check and must be observable through its telemetry, while + /// remaining a no-op for state-change subscribers. + /// + [Fact] + public async Task ACheckWhoseResultAlreadyWonACasRetry_StillRecordsTelemetry() + { + var name = $"same-result-{Guid.NewGuid():N}"; + var inner = new InMemoryStateProvider(); + await inner.SetStateAsync( + name, + new PulseCheckerState { IsHistoryEnabled = false }, + Ct); + var provider = new FirstWriteWinsElsewhereProvider(inner); + using var checker = new NamedChecker(provider) { CheckerName = name }; + var stateChanges = 0; + checker.StateChanged += (_, _) => stateChanges++; + + await checker.TriggerAsync(Ct); + + Assert.Equal(0, stateChanges); + Assert.Equal( + "Healthy", + Assert.Single(For("healthie.check.results", name)).Tags[HealthieDiagnostics.ResultTag]); + Assert.Equal( + "Healthy", + Assert.Single(ActivitiesFor(name)).GetTagItem(HealthieDiagnostics.ResultTag)); + } + [Fact] public async Task AFailedCheck_IsTaggedUnhealthy() { diff --git a/tests/Healthie.Tests.Unit/TestDoubles.cs b/tests/Healthie.Tests.Unit/TestDoubles.cs index 7612deb..ff46e2f 100644 --- a/tests/Healthie.Tests.Unit/TestDoubles.cs +++ b/tests/Healthie.Tests.Unit/TestDoubles.cs @@ -39,6 +39,7 @@ public override Task CheckAsync(CancellationToken cancellati internal sealed class FakePulseChecker(string name) : IPulseChecker { private PulseCheckerState _state = new(PulseInterval.EveryMinute, 0); + private List? _history; public event EventHandler? StateChanged; @@ -50,6 +51,10 @@ internal sealed class FakePulseChecker(string name) : IPulseChecker public int TriggerCount { get; private set; } + public int StateReadCount { get; private set; } + + public int HistoryReadCount { get; private set; } + public string Name => name; public string DisplayName => name; @@ -87,7 +92,10 @@ public Task CheckAsync(CancellationToken cancellationToken = => Task.FromResult(new PulseCheckerResult(PulseCheckerHealth.Healthy, "ok")); public Task GetStateAsync(CancellationToken cancellationToken = default) - => Task.FromResult(_state); + { + StateReadCount++; + return Task.FromResult(_state); + } public Task SetStateAsync(PulseCheckerState state, CancellationToken cancellationToken = default) { @@ -95,6 +103,8 @@ public Task SetStateAsync(PulseCheckerState state, CancellationToken cancellatio return Task.CompletedTask; } + public void SetHistory(IEnumerable history) => _history = [.. history]; + public Task SetIntervalAsync(PulseInterval interval, CancellationToken cancellationToken = default) => Task.CompletedTask; @@ -126,7 +136,10 @@ public Task SetGroupAsync(string? group, CancellationToken cancellationToken = d public Task StartAsync(CancellationToken cancellationToken = default) => Task.FromResult(true); public Task> GetHistoryAsync(CancellationToken cancellationToken = default) - => Task.FromResult(_state.History); + { + HistoryReadCount++; + return Task.FromResult((_history ?? _state.History).ToList()); + } public Task ClearHistoryAsync(CancellationToken cancellationToken = default) => Task.CompletedTask; diff --git a/tests/Healthie.Tests.Unit/TimerPulseSchedulerScheduleTests.cs b/tests/Healthie.Tests.Unit/TimerPulseSchedulerScheduleTests.cs index bcd1ab1..68468b2 100644 --- a/tests/Healthie.Tests.Unit/TimerPulseSchedulerScheduleTests.cs +++ b/tests/Healthie.Tests.Unit/TimerPulseSchedulerScheduleTests.cs @@ -1,7 +1,11 @@ using Healthie.Abstractions; using Healthie.Abstractions.Enums; +using Healthie.Abstractions.Models; using Healthie.Abstractions.Scheduling; using Healthie.DependencyInjection; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; using System.Diagnostics; namespace Healthie.Tests.Unit; @@ -137,6 +141,15 @@ public void DisposingTwice_IsSafe() scheduler.Dispose(); } + [Fact] + public void TheOriginalLoggerConstructor_RemainsAvailableForCompiledConsumers() + { + var constructor = typeof(TimerPulseScheduler).GetConstructor([typeof(ILogger)]); + + Assert.NotNull(constructor); + Assert.True(constructor.GetParameters().Single().HasDefaultValue); + } + [Fact] public async Task ScheduleAsync_WithACronSchedule_TriggersTheChecker() { @@ -164,6 +177,95 @@ await WaitUntilAsync(() => checker.TriggerCount > 0, TimeSpan.FromSeconds(5)), "the period schedule never triggered the checker"); } + /// + /// Scheduling is requested with the host's stopping token, but that token only governs the + /// request itself. Linking it to the long-lived timer made a later request cancellation stop a + /// checker permanently even though the scheduler was still alive. + /// + [Fact] + public async Task ScheduleAsync_CancellingTheRequestAfterSuccess_LeavesTheTimerRunning() + { + await using var scheduler = new TimerPulseScheduler(); + var checker = new FakePulseChecker("request-cancellation"); + using var requestCancellation = CancellationTokenSource.CreateLinkedTokenSource(Ct); + + await scheduler.ScheduleAsync( + checker, + PulseSchedule.Every(TimeSpan.FromMilliseconds(50)), + requestCancellation.Token); + + Assert.True(await WaitUntilAsync(() => checker.TriggerCount > 0, TimeSpan.FromSeconds(5))); + + requestCancellation.Cancel(); + var countAfterCancellation = checker.TriggerCount; + + Assert.True( + await WaitUntilAsync( + () => checker.TriggerCount > countAfterCancellation, + TimeSpan.FromSeconds(5)), + "cancelling the completed scheduling request stopped the timer"); + } + + [Fact] + public async Task ScheduleAsync_CancelledRequest_DoesNotReplaceTheExistingTimer() + { + await using var scheduler = new TimerPulseScheduler(); + var checker = new FakePulseChecker("cancelled-request"); + + await scheduler.ScheduleAsync(checker, PulseSchedule.Every(TimeSpan.FromMilliseconds(50)), Ct); + Assert.True(await WaitUntilAsync(() => checker.TriggerCount > 0, TimeSpan.FromSeconds(5))); + + using var requestCancellation = CancellationTokenSource.CreateLinkedTokenSource(Ct); + requestCancellation.Cancel(); + + await Assert.ThrowsAnyAsync( + () => scheduler.ScheduleAsync( + checker, + PulseSchedule.Every(TimeSpan.FromMilliseconds(100)), + requestCancellation.Token)); + + var countAfterCancelledRequest = checker.TriggerCount; + Assert.True( + await WaitUntilAsync( + () => checker.TriggerCount > countAfterCancelledRequest, + TimeSpan.FromSeconds(5)), + "a cancelled scheduling request replaced the existing timer"); + } + + /// + /// A host stops its hosted services before disposing singleton services. The timer therefore + /// must observe the host lifetime directly rather than relying on disposal to stop its workers. + /// + [Fact] + public async Task StoppingTheHost_StopsTimersBeforeTheContainerIsDisposed() + { + var checker = new FakePulseChecker("host-stopping"); + await checker.SetStateAsync( + new PulseCheckerState(PulseInterval.EveryMinute, 0) + { + Schedule = PulseSchedule.Every(TimeSpan.FromMilliseconds(50)), + }, + Ct); + using var host = new HostBuilder() + .ConfigureServices(services => + { + services.AddSingleton(checker); + services.AddHealthie(); + }) + .Build(); + + await host.StartAsync(Ct); + Assert.True(await WaitUntilAsync(() => checker.TriggerCount > 0, TimeSpan.FromSeconds(5))); + + await host.StopAsync(Ct); + await Task.Delay(200, Ct); + + var countAfterStopping = checker.TriggerCount; + await Task.Delay(400, Ct); + + Assert.Equal(countAfterStopping, checker.TriggerCount); + } + /// /// The whole reason this type exists: six hours is not something /// can say, and before schedules it could not be asked for at all. From c8686bf568fa55713ec5df38213226c75c4b9b66 Mon Sep 17 00:00:00 2001 From: Ivan Vydrin Date: Wed, 30 Sep 2026 02:19:36 +0300 Subject: [PATCH 3/5] Improve dashboard text contrast and checker labels --- .../Components/HealthieDashboard.razor | 12 ++++--- src/Healthie.Dashboard/wwwroot/healthie.css | 25 ++++++++++----- .../DashboardComponentLifecycleTests.cs | 31 +++++++++++++++++-- 3 files changed, 53 insertions(+), 15 deletions(-) diff --git a/src/Healthie.Dashboard/Components/HealthieDashboard.razor b/src/Healthie.Dashboard/Components/HealthieDashboard.razor index 45bb011..a5a7074 100644 --- a/src/Healthie.Dashboard/Components/HealthieDashboard.razor +++ b/src/Healthie.Dashboard/Components/HealthieDashboard.razor @@ -267,14 +267,14 @@ var status = StatusOf(state); var history = HistoryOf(name); } -
+ Rate: @RateLabel(state) @RateUnit(state) @@ -305,20 +306,21 @@ @if (history.Count == 0) { - no runs yet + Pulse: no runs yet } else {
+ Pulse: @history.Count recent @(history.Count == 1 ? "run" : "runs"), oldest first: @string.Join(", ", history.Select(entry => entry.Health)). @foreach (var entry in history) { + title="@entry.Health at @entry.ExecutedAt.ToString("HH:mm:ss") UTC" aria-hidden="true"> }
} - @Relative(state.LastExecutionDateTime) + Last run: @Relative(state.LastExecutionDateTime) @if (Options.AllowMutations) { diff --git a/src/Healthie.Dashboard/wwwroot/healthie.css b/src/Healthie.Dashboard/wwwroot/healthie.css index f751164..7b59ad3 100644 --- a/src/Healthie.Dashboard/wwwroot/healthie.css +++ b/src/Healthie.Dashboard/wwwroot/healthie.css @@ -16,14 +16,14 @@ --hpm-line2: #263340; --hpm-text: #DFE9EE; --hpm-muted: #7C8C97; - --hpm-faint: #4A5862; + --hpm-faint: #71838D; --hpm-ok: #38E08F; --hpm-ok-bg: rgba(56, 224, 143, 0.09); --hpm-warn: #FFC24B; --hpm-warn-bg: rgba(255, 194, 75, 0.10); --hpm-crit: #FF5A64; --hpm-crit-bg: rgba(255, 90, 100, 0.11); - --hpm-paused: #4A5862; + --hpm-paused: #7C8C97; --hpm-paused-bg: rgba(74, 88, 98, 0.18); --hpm-grid: rgba(56, 224, 143, 0.05); @@ -65,6 +65,19 @@ display: block; } +/* Labels that keep the compact dashboard grid readable to assistive technology. */ +.healthie-dashboard .hpm-sr-only { + position: absolute; + width: 1px; + height: 1px; + padding: 0; + margin: -1px; + overflow: hidden; + clip: rect(0, 0, 0, 0); + white-space: nowrap; + border: 0; +} + .healthie-dashboard[data-hpm="light"] { --hpm-bg: #EFF3F2; --hpm-panel: #FFFFFF; @@ -73,14 +86,14 @@ --hpm-line2: #C2CDCA; --hpm-text: #12181B; --hpm-muted: #5A6A70; - --hpm-faint: #93A2A8; + --hpm-faint: #5C6C72; --hpm-ok: #0B9E5E; --hpm-ok-bg: rgba(11, 158, 94, 0.10); --hpm-warn: #B57A0D; --hpm-warn-bg: rgba(181, 122, 13, 0.12); --hpm-crit: #D23440; --hpm-crit-bg: rgba(210, 52, 64, 0.10); - --hpm-paused: #93A2A8; + --hpm-paused: #5A6A70; --hpm-paused-bg: rgba(147, 162, 168, 0.16); --hpm-grid: rgba(11, 158, 94, 0.06); } @@ -380,10 +393,6 @@ border-color: var(--hpm-line2); } -.hpm-row--paused { - opacity: 0.55; -} - .hpm-row-name { min-width: 0; } diff --git a/tests/Healthie.Tests.Unit/DashboardComponentLifecycleTests.cs b/tests/Healthie.Tests.Unit/DashboardComponentLifecycleTests.cs index 7965831..5a3e95b 100644 --- a/tests/Healthie.Tests.Unit/DashboardComponentLifecycleTests.cs +++ b/tests/Healthie.Tests.Unit/DashboardComponentLifecycleTests.cs @@ -1,4 +1,5 @@ using Bunit; +using Healthie.Abstractions.Enums; using Healthie.Abstractions.Models; using Healthie.Dashboard; using Healthie.Dashboard.Components; @@ -29,6 +30,10 @@ private sealed class RecordingDashboardService : StubDashboardService public List> Unsubscribed { get; } = []; + public Dictionary States { get; } = new(StringComparer.Ordinal); + + public Dictionary DisplayNames { get; } = new(StringComparer.Ordinal); + public override Task SubscribeToStateChangesAsync( Func onStateChanged, CancellationToken cancellationToken = default) @@ -47,11 +52,11 @@ public override Task UnsubscribeFromStateChangesAsync( public override Task> GetAllStatesAsync( CancellationToken cancellationToken = default) => - Task.FromResult(new Dictionary(StringComparer.Ordinal)); + Task.FromResult(new Dictionary(States, StringComparer.Ordinal)); public override Task> GetDisplayNamesAsync( CancellationToken cancellationToken = default) => - Task.FromResult(new Dictionary(StringComparer.Ordinal)); + Task.FromResult(new Dictionary(DisplayNames, StringComparer.Ordinal)); } private sealed class FailsFirstLoadDashboardService : StubDashboardService @@ -129,6 +134,28 @@ public void RenderingTheDashboard_Subscribes() Assert.Empty(_service.Unsubscribed); } + [Fact] + public void CheckerRows_NameTheirDetailsRatePulseAndLastRun() + { + _service.States["database"] = new PulseCheckerState + { + LastResult = new PulseCheckerResult(PulseCheckerHealth.Unhealthy, "down"), + LastExecutionDateTime = DateTime.UtcNow, + History = [new(PulseCheckerHealth.Unhealthy, "down", DateTime.UtcNow)], + }; + _service.DisplayNames["database"] = "Orders database"; + + var rendered = _context.Render(); + + rendered.WaitForAssertion(() => + { + Assert.Equal("View Orders database details, status UNHEALTHY", rendered.Find(".hpm-row-select").GetAttribute("aria-label")); + Assert.Equal("Rate:", rendered.Find(".hpm-rate-cell .hpm-sr-only").TextContent.Trim()); + Assert.Equal("Pulse: 1 recent run, oldest first: Unhealthy.", rendered.Find(".hpm-blips .hpm-sr-only").TextContent); + Assert.Equal("Last run:", rendered.Find(".hpm-time .hpm-sr-only").TextContent.Trim()); + }); + } + /// /// The half that was missing. The handler it hands back has to be the one it subscribed with, or /// the service cannot remove it and the leak stands. From c2ed7fc2bc88b243f7a5bff0523a914e7d52e93d Mon Sep 17 00:00:00 2001 From: Ivan Vydrin Date: Wed, 30 Sep 2026 05:19:49 +0300 Subject: [PATCH 4/5] Document hardening fixes for next patch release --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd95a7e..93d4a8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Fixed + +- Timer-scheduled checks now continue after an individual scheduling caller cancels, while host + shutdown still stops them; an older overlapping result can no longer overwrite newer checker state. +- Bulk dashboard state reads and AI diagnoses now avoid redundant state-provider calls. +- Workflow dispatch inputs are passed to shell steps as environment variables, preventing command + injection through a supplied version or branch name. +- Dashboard status text has stronger contrast, and checker details expose their meaning to assistive + technology. + ## [4.1.3] - 2026-09-29 ### Fixed From 58e66081b89cb1618d8f773debdfef5d714262d8 Mon Sep 17 00:00:00 2001 From: Ivan Vydrin Date: Wed, 30 Sep 2026 05:20:42 +0300 Subject: [PATCH 5/5] Prepare 4.1.4 changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93d4a8a..d9721de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +## [4.1.4] - 2026-09-30 + ### Fixed - Timer-scheduled checks now continue after an individual scheduling caller cancels, while host