Stabilize HealthChecksPlus, fix critical bugs, and release v4.0.0 - #47
Merged
Conversation
- Consolidate policy resolution (FindPolicy/GetHealthyPolicy/ ResolveForegroundPolicy/ResolveBackgroundPolicy/ScheduleIfDue) shared by both the HTTP and background execution paths, fixing the Degraded policy being silently ignored on the HTTP path - Fail fast at construction with a clear message when a health check has no matching Healthy policy, instead of throwing NullReferenceException later at runtime - Stop disposing the adopted external check on every CheckHealthAsync call in WrapperBaseHealthCheckPlus, which broke IDisposable checks after their first execution - Switch scheduling comparisons from DateTime.Now to DateTime.UtcNow - Fix malformed exception message in HealthChecksPlusAppExtension - Update SECURITY.md supported version table and bump package version to 4.0.0 in HealthCheckPlus and HealthCheckPlus.Abstractions - Add regression/characterization tests for all of the above - Add audit report and action plan under doc/ - Bump Samples and test project dependencies (Swashbuckle.AspNetCore, Microsoft.OpenApi, coverlet.collector, Microsoft.NET.Test.Sdk, xunit.v3) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e 1) - Replace the process-wide static fields in HealthChecksPlusExtension (_addedHealthChecksPlus, _externalCheck) with HealthChecksPlusRegistrationState, an instance scoped to each IServiceCollection and retrieved via the same ServiceDescriptor scan pattern AddCheckLinkTo already used for HealthCheckServiceOptions - This removes the state leak between multiple hosts built in the same process (WebApplicationFactory, .NET Aspire, parallel tests), where two containers adopting an external check under the same name would previously share the same wrapped instance - Add a regression test proving two independent hosts no longer share the registration state or adopted external check instances - Update the action plan progress log Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ipeline (Fase 2) - Adopt the original registration by hooking into IServiceCollection.Configure<HealthCheckServiceOptions> — the same public, documented Options pipeline the original registration itself went through — instead of scanning ServiceDescriptor.ImplementationInstance and reflecting into a captured ConfigureNamedOptions<HealthCheckServiceOptions> delegate to reconstruct a throwaway copy of the options - Replace the "DefaultHealthCheckService" internal type-name string match in AddHealthChecksPlus with removal by the public HealthCheckService service type - AddCheckLinkTo now throws a clear InvalidOperationException when the named check was never registered, instead of silently doing nothing - No public API changes: AddCheckLinkTo's signature and Samples/README usage are unchanged - Add integration tests covering the adoption/replacement behavior and the new not-found error - Update the action plan progress log Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Add Microsoft.AspNetCore.TestHost to the test project and a shared TestHost.CreateAsync helper (TestServer over a HostBuilder), rather than WebApplicationFactory to avoid depending on the Samples projects' real Redis dependency - Cover the Degraded/Unhealthy policy scenarios through a real HTTP request against the full pipeline, using the public IStateHealthChecksPlus SwitchTo* API to force state and real delays to prove the policy period is honored - Cover the background service end-to-end: periodic rerun and the "publish only when the report changes" filter, previously untested - Cover the middleware end-to-end: ResultStatusCodes mapping and the WriteDetailsWithException response writer - Measure real code coverage with coverlet; the measurement surfaced a real gap (AddUnhealthyPolicy itself was never exercised by any test) which is fixed in this same change - Update the action plan progress log Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ads, disposal (R1-R3) - R1: harden the native HealthCheckPublisherHostedService removal in AddBackgroundPolicy to match on the fully qualified internal type name instead of the short name (no public marker type exists for this one, unlike HealthCheckService in Fase 2 — documented in code); add a direct DI-inspection regression test instead of relying on timing alone - R2: add coverage for the two previously-untested UseHealthChecksPlus overloads that take a port, exercising the port-matching predicate via HttpContext.Connection.LocalPort against a real TestServer request - R3: dispose adopted external check instances when the container shuts down. DefaultHealthCheckServicePlus now takes HealthChecksPlusRegistrationState as a constructor dependency and implements IDisposable, disposing ExternalCheck.Values — piggybacking on DefaultHealthCheckServicePlus's existing factory-based (and therefore container-disposed) singleton registration, since HealthChecksPlusRegistrationState itself is registered as a ready-made instance and is never disposed by the container - Update the action plan progress log Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… (Fase 4) Implements the Fase 4 metrics scope (System.Diagnostics.Metrics, zero new dependencies): check execution/duration/status-transition instruments and publisher invocation/duration instruments, tested via MeterListener. While closing metric test-coverage gaps, surfaced and fixed a pre-existing bug where a throwing publisher permanently killed the background service's loop with no operational trace. A follow-up advisor-driven re-validation pass then found and fixed several related silent-failure paths across the session's earlier changes: metrics recording that could propagate an exception into the unprotected HTTP health-check path, a disposal loop with no fault isolation between adopted external checks, and a GetOrAdd race that could double-construct an adopted check under concurrent scheduling. Added a healthcheckplus.anomalies counter so these defensive paths are also visible as metrics, not just logs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ScheduleIfDue read whether a check was due and marked it Running as two separate, unsynchronized steps, so two concurrent callers deciding the same check was due at once (e.g. an HTTP request racing a background cycle) could both schedule and run it, with the loser silently dropping its result in Update(). Adds CacheHealthCheckPlus.TryBeginRun, guarded by the existing lock, making the due-check and the Running flag one atomic operation. Regression tests reproduce the race deterministically via Barrier-synchronized concurrent callers before confirming the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…es (Fase 4, P4.8-P4.10) Adds docs/ARCHITECTURE.md (internal design for maintainers) and docs/RUNBOOK.md (operational reference for on-call), and rewrites CONTRIBUTING.md to describe the actual project instead of a leftover generic template. Establishes a documentation policy: permanent docs describe current behavior in terms of benefits and points of attention, never by referencing the internal action-plan/phase tracking used to get there. README/CHANGELOG cleanup: extracted version history into a new CHANGELOG.md so README stays current-state only; fixed a stale version label and a sample that referenced an API removed two versions ago. Relocates the generated API reference from src/docs to docs/api, and moves XmlDocMarkdownGenerator's output path to match. Validates all sample projects end-to-end (not just compiled): fixes a publisher example that didn't satisfy its own interface, a counter bug that kept a new sample's flaky check from ever changing status, and adds a new HealthCheckPlusDemoMetrics sample demonstrating the metrics instrumentation via a MeterListener, since no sample covered it before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This folder holds the pt-BR audit/plan/progress-tracking files used to run this project's stabilization/hardening effort - not permanent product documentation. Renaming it to TODO/ makes that unmistakable; the folder is expected to be deleted entirely once no longer needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The plan/progress files link to each other by path; update those internal links to match the new folder name, and log the rename itself in the progress tracker. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
All links between README, docs/ARCHITECTURE.md, docs/RUNBOOK.md, CONTRIBUTING.md, CHANGELOG.md, and the generated API reference resolve correctly; permanent docs describe only the already-fixed behavior, never the pre-fix state. Full solution build and test suite green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI previously only built/tested on ubuntu-latest and only compiled (never executed tests against) the net8.0/net9.0 builds of the library. Adds a Windows/Linux/macOS matrix to build.yml, and multi-targets HealthCheckPlusTests to net8.0/net9.0/net10.0 so the suite actually runs against all three - Microsoft.AspNetCore.TestHost is versioned per target framework to match each runtime's shared framework. publish.yml packed and pushed NuGet packages without building or testing first, so a tag pointing at a broken commit could publish a package that can't be unpublished. Adds a build+test gate before packing, and --skip-duplicate on the push for idempotency. Also adds NuGet package caching to all three workflows, and removes a dead swift/macOS conditional left over from CodeQL's generic template (this repo only analyzes csharp). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Comments and XML doc comments must describe the code itself - what it does and why - not the internal work-tracking process used to build it. Rewrites every comment that cited phase/step numbers, the plan/progress files, or "advisor"/re-validation narration, keeping the technical rationale (why a decision was made, what a test guards against) and dropping the process framing. Samples/ was already clean; XML docs had no such references either. Also fixes two things found along the way: a duplicated comment block in HealthChecksPlusRegistrationState.cs, and a comment in WrapperBaseHealthCheckPlus.cs that still described disposal as a future step even though it's already implemented. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- N1: ambient cancellation (e.g. httpContext.RequestAborted) mid-flight left every scheduled check permanently marked Running, since Task.WhenAll faulting skipped the loop that releases it. Update() now runs in a finally block on both the HTTP and background paths. - N2: AddCheckLinkTo cached an adopted check built from whichever scope first constructed it, disposed right after that call returned - breaking any adopted check with a scoped dependency (e.g. AddDbContextCheck<T>) from its second execution on. The adoption factory now owns a dedicated scope for the check's entire lifetime, disposed together with it at shutdown. - N3: publisher dispatch had no timeout of its own, so a publisher that never completes froze the whole background loop - checks included - forever. It's now bounded by the same per-cycle Timeout as check execution. - N4: a check registered with a Healthy policy but left off the `names` list passed to AddHealthChecksPlus slipped past the existing fail-fast validation and crashed every request with an unhandled KeyNotFoundException instead. The constructor now validates both directions of that misconfiguration. Each fix has a red/green regression test; full suite (72 tests) green and stable across 3 runs on net8.0/net9.0/net10.0, 0 warnings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- N5: 5 metrics call sites in the background service's publisher path had no guard of their own, so a throwing MeterListener could be misattributed as the publisher itself failing (wrong log, wrong "error" metric) instead of surfacing the real problem. Consolidated behind a shared SafeRecordMetric helper, mirroring CacheHealthCheckPlus's existing pattern. - N6: CacheHealthCheckPlus.CreateReport() (consumed by publishers and by Status()/AddStatusName) hardcoded null description, zero duration, no exception, no data and no tags for every entry, even though the cache already tracked all of that per check - so a named-status callback computed a different answer than the functionally identical callback wired to the HTTP endpoint. Tags needed new plumbing (ItemCacheHealth.Tags + CacheHealthCheckPlus.SetTags, populated from the real registrations in DefaultHealthCheckServicePlus's constructor) since the cache never tracked them before. - N7: HealthCheckPlusBackGroundOptions.Predicate decides which checks the background service runs, but the report it hashes (WhenReportChange) and publishes had no notion of it, so a predicate-excluded check never left its InitCache seed status (Healthy) and was published as such forever. Filtered at the point of use (HealthCheckPlusBackGroundService), keeping CacheHealthCheckPlus itself predicate-agnostic. - N8: StopAsync swallowed an exception from cancelling the stopping token with a fully empty catch block - no log, no metric, nothing - violating this project's own no-silent-catch rule. Now logs a warning before continuing shutdown regardless. Each fix has a red/green regression test; also hardened a pre-existing flaky test's timing margin (unrelated bug, just needed more headroom under the added test load). Full suite (77 tests) green and stable across 3 runs on net8.0/net9.0/net10.0, 0 warnings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- CacheHealthCheckPlus: SwithState/StatusResult/FullStatus/ConvertToPlus threw a raw KeyNotFoundException for an unknown check name; a shared GetItemOrThrow helper now throws a clear ArgumentException naming the check instead. - DefaultHealthCheckServicePlus: calling AddUnhealthyPolicy/AddDegradedPolicy twice for the same check and status silently kept only the first registration (FindPolicy's FirstOrDefault). Added ValidatePolicyUniqueness, failing fast at construction alongside the existing policy validations. - HealthCheckPlusBackGroundService: publisher metrics were keyed by GetType().Name, so two publishers with the same class name in different namespaces collapsed into one metric series. Now keyed by GetType().FullName. - Added an HTTP-pipeline-level regression test (via TestHost) for a health check registered without AddCheckPlus/AddCheckLinkTo - previously only covered at the constructor/unit level. - CI: build.yml/publish.yml only installed the .NET 10 SDK despite the test project being multi-targeted net8.0/net9.0/net10.0; dotnet-version now lists all three explicitly. Each production fix has a red/green regression test. Full suite (84 tests) green and stable across 3 runs on net8.0/net9.0/net10.0, 0 warnings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BackgroundService_ShouldNotMisattributeFailure_WhenAMetricsListenerThrows RecordingAPublishedInvocation failed on windows-latest CI (Linux/macOS passed): it needed more than one ~1.1s background cycle within a 3s wait, too tight a margin for that runner. Found 4 more tests with the same risk profile (short wait relative to the minimum cycle time, never actually exercised on a real Windows CI run) and widened all of them at once instead of discovering each one across separate CI runs: - BackgroundService_ShouldRecordSkippedCondition_WhenPublisherConditionReturnsFalse, BackgroundService_ShouldNotCollidePublisherMetrics_ForSameShortTypeNameInDifferentNamespaces: 1.5s -> 3s (each only needs 1 cycle) - BackgroundService_ShouldRecordCheckExecutionMetrics_WithBackgroundOrigin, BackgroundService_ShouldExcludePredicateFilteredChecks_FromThePublishedReport: 2s -> 3s (each only needs 1 cycle) - The one that failed: 3s -> 5s (needs >1 cycle - same margin already proven reliable in the same CI run by its sibling test, BackgroundService_ShouldRecordError_WhenPublisherThrows, which didn't flake) Full suite (84 tests) green and stable across 3 runs on net8.0/net9.0/net10.0, 0 warnings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rsarial pass A batch-wide cancellationToken.IsCancellationRequested flag was being used as proof that a specific check's own OperationCanceledException was caused by that token - so a genuinely broken check (its Factory, or its own CheckHealthAsync, throwing an unrelated OCE) could be misclassified as "ambient cancellation" whenever some OTHER check in the same batch was the one that actually triggered it, silently keeping the broken check's stale Healthy status forever instead of reporting it Unhealthy. Fixed at both points: Factory-thrown OCEs are now laundered into a Faulted outcome inside RunCheckAsync (logged and preserved as an inner exception), and the CheckHealthAsync guard now compares the exception's own CancellationToken against the token actually handed to the check instead of relying on the ambient flag alone. Also fixes a related gap where a factory construction failure produced no log line at all. Also documents (rather than re-engineers, given this area's history of regressions across recent review rounds) that DateRef can advance on an aborted attempt while status/duration/origin still reflect the last completed run, and corrects a couple of doc/comment inaccuracies found in the same pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ad/duplicated code The three narrow adversarial-review rounds in this session each found a real bug inside the fix from the round before, always in the same area: cancellation classification independently reimplemented at CheckHealthPlusAsync's finally, BackGroudCheckHealthPlusAsync's finally, and RunCheckAsync's own guard. The first two are now a single ApplyBatchResults/ClassifyBatchTask pair (the RunCheckAsync guard stays separate - it answers a different question at a different point in the control flow, so folding it in would be a forced abstraction, not a fix). Following up with a broader sweep for the same failure pattern elsewhere in the codebase, run as two independent parallel audits (duplication-by-call-site and accidental-complexity), every finding verified by hand before acting: - The "period >= 1 second" rule, reimplemented at ~10 call sites across HealthCheckPlusBackGroundOptions and HealthChecksPlusExtension, is now one PeriodValidation helper. - HealthCheckPlusOptions' six JSON response writers shared one WriteReport helper instead of six copies of the same tail - which had already drifted: WriteShortDetails was missing the "; charset=utf-8" every other overload set. - CacheHealthCheckPlus's four TryGetXxx methods now share TryGetByStatus, mirroring the pattern HealthReportExtensions already used for the same shape. - CheckHealthPlusAsync/BackGroudCheckHealthPlusAsync's fan-out loop (build the due-registration list, start the tasks) is now BuildDueRegistrations/ StartBatch, parameterized by the one thing that actually differs between the two paths: policy resolution. - CacheHealthCheckPlus's status-aggregation formula was triplicated; one copy (a string.Empty-keyed cache entry, recomputed every cycle) was dead code nothing ever read. Removed, and the remaining two calls now share AggregateStatus(). - IHealthCheckPlusPolicyStatus had exactly one implementation anywhere in the codebase and served no mocking/extension need - removed in favor of the concrete HealthCheckPlusPolicyStatus record. - WrapperBaseHealthCheckPlus carried the full Dispose(bool)/finalizer boilerplate for a class with no finalizer and no subclasses - collapsed to a single guarded Dispose(). 32 new tests cover the extracted validation/writer/dispose behavior, including a regression test for the ContentType drift. All other changes are behavior-preserving refactors verified against the existing suite. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…r tracked checks, fix a silent background-loop death and a stale manual-override status, and stop embedding Abstractions.dll AddHealthChecksPlus(names) required a separately maintained list of check names, seeded into the cache at registration time. That list only existed to be cross-checked against the real registrations later - two of the constructor's four validations existed purely to catch it drifting from them. The cache's IStateHealthChecksPlus factory only actually runs on first DI resolution, by which point every AddCheckPlus/AddCheckLinkTo/native AddCheck call has already happened, so the real registrations were always available as the seed. AddHealthChecksPlus() now takes no parameter and seeds directly from them; the two now-tautological validations, and the CacheHealthCheckPlus members that existed only to support them, are gone. Also fixes two real bugs surfaced by a from-scratch baseline audit (three independent reviewers, one per angle, everything cross-checked by hand before acting): - HealthCheckPlusBackGroundService's report-building step (CreateReport + FilterReportByPredicate, which runs the consumer's own Predicate) sat completely outside any try/catch, unlike every other step in the loop. A throwing Predicate killed the background loop permanently and silently - checks stopped running, publishers stopped firing, with no log or metric. Now caught, logged, and counted as its own anomaly; the loop continues. - CacheHealthCheckPlus.SwithState (SwitchToUnhealthy/SwitchToDegraded) never refreshed a named status aggregate (Status(name), via AddStatusName) - a manual override could go unreflected there until the next request or background cycle happened to run, breaking the documented catch-exception-then-gate-on-status pattern for an unbounded time. Also stops HealthCheckPlus.csproj from embedding a private copy of HealthCheckPlus.Abstractions.dll while declaring zero dependency on that package - confirmed via an actual pack + nuspec inspection that installing both packages (as the README's own instructions suggest for projects that only need the abstractions) could load two independent copies of the same types. Removing the custom packaging target lets a normal package dependency get declared instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Picks up the AddHealthChecksPlus() signature change and several XML doc comment updates from earlier rounds (e.g. HealthCheckPlusBackGroundOptions. Timeout's InfiniteTimeSpan note) that hadn't been regenerated into the published API reference yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…alls LastResult/DateRef/Duration/Origin were four independent mutable properties on ItemCacheHealth, written one assignment at a time inside Update()/ ReleaseRunning() with no synchronization. A concurrent reader landing between two of those writes could observe an inconsistent mix - e.g. a new Status paired with the old Description, or an Exception that doesn't match either. Confirmed empirically: a stress test alternating Update() between two distinct results while continuously reading CreateReport() showed 86,495 torn reads out of 500,000 (17.3%) before this fix, 0 after. Bundled the four fields into one immutable CheckResultSnapshot, swapped with a single reference assignment (never torn on .NET). Bundling alone isn't enough, though: any call site reading more than one of the four as separate property accesses (item.LastResult, then later item.Duration) stays exposed, since each access re-reads the snapshot independently. Added ItemCacheHealth.Snapshot for a single read, and updated every such call site (CreateReport, TryGetByStatus, ReleaseRunning, and DefaultHealthCheckServicePlus's own report-building loop) to read it once. Also stops ConvertToPlus from handing out the live, mutable ItemCacheHealth objects themselves - the widest read window of all, since a slow JSON serialization pass (every "Plus" response writer) could span many concurrent Update() calls. It now returns a frozen DataHealthPlusSnapshot per entry instead, which as a side effect also stops IDataHealthPlus.Name's setter (required by the interface) from being able to mutate the shared cache entry in place. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…licy lookup Closes out the from-scratch baseline audit's backlog left after the H1/H2/M1/M2 round (commit 17ac7bf): the Medium findings (M3-M7) plus all seven Low-severity ones (L1-L7), plus a follow-up performance fix on the same code path. Medium findings: - M3: RunPublisherAsync evaluated a publisher's PublisherCondition outside its own try/catch, so a throwing condition was only ever caught one level up, as the generic cycle-dispatch failure with no indication of which publisher or why. Moved inside the try/catch - it's now attributed to that exact publisher (HealthCheckPublisherError, and the "error" result on healthcheckplus.publisher.invocations). - M4: nothing validated that AddUnhealthyPolicy/AddDegradedPolicy's target name actually corresponded to a registered check - a typo silently registered a policy FindPolicy could never match. New ValidatePolicyTargets fails fast at startup, alongside the existing ValidateHealthyPolicies/ValidatePolicyUniqueness. - M5: PublishingOptions.Enabled's XML doc claimed a setter that doesn't exist and the wrong default for how AddBackgroundPolicy actually configures it. Doc rewritten to describe the real behavior. - M6: a check that had never run even once (InitCache's Healthy/Origin=None seed) could be published by the background service as a genuine Healthy result if its own Delay outlived the cycle's Delay+Idle - the README's own example values trigger this on the first cycle. FilterReportByPredicate (renamed FilterReportForPublishing) now also excludes never-run checks, the same way it already excluded Predicate-filtered ones (new CacheHealthCheckPlus.HasEverRun). - M7: AddCheckPlus/AddCheckLinkTo's delay/period XML docs described IHealthCheckPublisher timing instead of the check's own Healthy-policy scheduling. Docs rewritten with the real per-path fallback behavior. Low-severity findings: - L1: SwithState (SwitchToUnhealthy/SwitchToDegraded) dropped a manual override with zero signal when the check was already Running - exactly the "no silent catch" pattern this project's own doctrine forbids elsewhere. Now logged (HealthCheckPlusSwitchToDropped) and counted as a switchto_dropped_while_running anomaly. - L2: policy lookup (FindPolicy/ValidateHealthyPolicies/ValidatePolicyUniqueness/ ValidatePolicyTargets) and the named-status dictionaries (_statusName/ _statusFunction) compared names case-sensitively while the main cache (_statusDeps) is already OrdinalIgnoreCase - a policy could silently never match its own check over a casing difference alone. Unified to OrdinalIgnoreCase throughout. - L3: IStateHealthChecksPlus now documents the ArgumentExceptions its members can throw for an unregistered name (confirmed the docs generator doesn't render <exception> tags anywhere in this project, so docs/api is unaffected). - L4: ConvertToPlus returned a lazy Select that every "Plus" response writer enumerates while already streaming a JSON response - a failure could surface mid-write, after a truncated document had already gone out. Now materialized eagerly so it fails before returning, not during enumeration. - L5: five raw internal downcasts (HealthCheckService -> DefaultHealthCheckServicePlus, IStateHealthChecksPlus -> CacheHealthCheckPlus) threw a generic InvalidCastException if a consumer decorated/replaced one of those registrations. New Internal.InternalCast.To<T> helper names the registration and the actual type found instead. - L6: FindPolicy was a linear scan through every registered policy, called once per registration on every background cycle/HTTP request - effectively quadratic since policy count scales with registration count. Indexed into a Dictionary<(NormalizedName, HealthStatus), HealthCheckPlusPolicyStatus> built once in the constructor (after ValidatePolicyUniqueness, which already rules out key collisions) - FindPolicy is now O(1). The remaining O(registrations) baseline (touching every registration every cycle regardless of what's due) stays a documented, deliberate tradeoff - fixing it would need a different scheduler design, not justified at this library's documented scale. - L7: a RUNBOOK.md sentence claimed a check with no explicit period has no backoff "on either path" - false for the background path, which always falls back to HealthCheckPlusBackGroundOptions' own per-status defaults. Only the HTTP-only path (no AddBackgroundPolicy) truly has none. docs/ARCHITECTURE.md, docs/RUNBOOK.md, CHANGELOG.md, and the affected docs/api pages are updated to match. 145/145 tests green on net8/9/10, Debug and Release builds clean with 0 warnings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ndent audit's findings
A third, from-scratch independent audit (told not to trust this session's own prior
claims) confirmed the H1/H2/M1-M7/L1-L7 backlog stays closed, but found new issues two
prior audit rounds missed:
- CacheHealthCheckPlus.ReleaseRunning cleared Running before reading/rewriting the
cached snapshot, with neither step under the lock TryBeginRun uses - a genuine data
race on a non-volatile bool under the CLR memory model. Fixed by moving the whole
read-rewrite-clear sequence inside the same lock, matching Update()'s existing
publish-before-clear ordering. The specific interleaving could not be reproduced via a
stress test even under heavy thread oversubscription (the window is two adjacent
statements wide); the fix stands on the ordering/data-race argument, not an empirical
repro, and the tracker records that explicitly.
- HealthCheckPlusBackGroundOptions.Delay only rejected Timeout.InfiniteTimeSpan, so a
negative Delay was accepted silently and only failed later inside Task.Delay, killing
the background loop with no log or metric - the same failure class as the earlier H1
fix, via a different validation gap. Fixed with a new PeriodValidation.EnsureNonNegative
(not EnsureAtLeastOneSecond, which would have broken every sub-second Delay used across
the integration suite).
- Two EventId collisions hidden behind #pragma warning disable SYSLIB1006 (publisher
error/timeout sharing 104; the background cycle's own error log also sharing 104 with
the publisher error log) - both renumbered, both pragmas removed, plus 4 dead unused
EventId constants deleted.
- AggregateStatus/LastReport threw InvalidOperationException for zero registered checks
instead of a sensible vacuous result - now Healthy/null, matching the native
HealthReport.Status's own empty-entries default.
- AddUnhealthyPolicy/AddDegradedPolicy/AddCheckPlus/AddCheckLinkTo accepted a null/empty
check name with no validation, surfacing later as a confusing NullReferenceException
instead of a clear ArgumentException at the actual call site.
- AddBackgroundPolicy could be called more than once, silently registering a second,
independent background service instance (breaking change: now fails fast).
- The background service could publish a phantom Healthy result for a check whose first
real run completed in the gap between building the report and deciding which checks
were eligible to publish - FilterReportForPublishing (renamed BuildReportForPublishing)
now decides both from one CacheHealthCheckPlus.CreateReport(includeName) snapshot read
instead of two separate reads at two different times; the now-dead HasEverRun helper
was removed from both classes.
- StopAsync's ContinueWith never observed a faulted background-loop task, so a fault
escaping every other guard would vanish with zero signal - now logged (Critical) via a
testable ObserveLoopCompletion, mirroring the existing ClassifyBatchTask pattern.
- InternalCast.To<T>(null, ...) threw NullReferenceException instead of its own clear
message.
- Doc-only: every documented metric tag name across ARCHITECTURE.md/RUNBOOK.md was
missing the healthcheckplus. prefix the code actually emits; AddDegradedPolicy's period
doc said "Unhealthy"; the earlier delay/period doc rewrite still said "while it stays
Healthy" when the HTTP path's Healthy-policy fallback applies to any status without its
own policy; RUNBOOK claimed an empty response body by default (native default is
WriteMinimalPlaintext); Status("")'s doc didn't match its actual no-throw behavior;
~9 real V4.0.0 changes were missing from CHANGELOG.md.
Two findings from the fresh audit were verified and rejected rather than fixed: a claimed
timeout/ambient-cancellation misclassification (the only real race window requires both
causes simultaneously, in which case the ambient cancellation would have ended the check
anyway) and RUNBOOK's exception-message sentence (already correctly scoped). A claimed
README overclaim about native-only registration being valid could not be located in any
doc file and is recorded as unconfirmed rather than invented.
145->158/158 tests across net8/9/10, Debug+Release, 0 warnings.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…h audit, centralize EventId governance, and close the remaining low-severity findings A fourth independent audit against b7b2e52 confirmed everything in that commit holds (including empirically re-verifying the ReleaseRunning fix this time, correcting an earlier, wrongly pessimistic "could not reproduce" claim in the tracker) and found two new Medium-severity, empirically-confirmed issues plus a mistake introduced while fixing the prior round's EventId collision: - CheckHealthPlusAsync/BackGroudCheckHealthPlusAsync/BuildDueRegistrations could leave a check marked Running forever - a log call and StartBatch ran outside the try/finally that releases it via ApplyBatchResults, so a throwing ILogger (a broken third-party sink) left the check stuck Running with no recovery short of a process restart. Fixed by wrapping that gap in its own try/catch (new ReleaseRunningForBatch helper) ahead of the existing try/finally, and by wrapping BuildDueRegistrations's own marking loop the same way. New test with a throwing ILogger, confirmed red then green. - UpdateStatusName/Status(name) could let a slow, stale call (e.g. a routine cycle working from an old report) silently overwrite a fresher one (e.g. a manual override that just landed) if the stale call happened to finish last. Fixed with optimistic versioning: a new _stateVersion counter (bumped once per real state change, inside Update()) tags each computed value, and a write is only applied if its version isn't older than what's already stored - no lock held across the consumer-supplied delegate. New test reproduces the exact interleaving, confirmed red then green. - The prior round's EventId fix (104 -> 105) traded one collision for another (105 was already HealthCheckPlusBackGroundWarningId) and missed two more pre-existing ones (106, 107) - all hidden behind #pragma warning disable SYSLIB1006. Fixed by renumbering EventIdsPublisher into a range that doesn't overlap EventIds' full set. Also, per explicit request, centralized EventId governance project-wide: a single new HealthCheckPlusEventIds catalog replaces the three independent per-class EventIds/ EventIdsPublisher definitions (CacheHealthCheckPlus, DefaultHealthCheckServicePlus, HealthCheckPlusBackGroundService), removing the "which other class also feeds this logger" step that caused the mistake above. EventName strings are unchanged; only the numeric ids and where they're declared changed. Closed the remaining low-severity findings from the fourth audit: - StopAsync's shutdown continuation used TaskScheduler.Current (whatever scheduler happens to be ambient at the call site - a well-known real .NET pitfall) instead of TaskScheduler.Default, which could tie shutdown to an unrelated custom scheduler's state. Fixed; new test with a scheduler that records every task routed through it, confirmed red then green. - SwithState read item.LastResult twice (once under lock, once outside) - re-analyzed and could not construct a live race given Running's exclusive-ownership guarantee, so fixed as defense-in-depth (read once, reuse) rather than a demonstrated exploit. - The named-status function table (_statusFunction) was a plain Dictionary relying on an unenforced "AddStatusName always runs before real traffic" ordering assumption - changed to ConcurrentDictionary, matching the class's other shared state, with AddStatusName now using atomic TryAdd instead of ContainsKey+Add. Also fixed several doc nits the fourth audit found: a stale FilterReportForPublishing reference and an unprefixed metric tag in ARCHITECTURE.md, a missing row in RUNBOOK's anomaly table, an ambiguous period doc on AddCheckPlus/AddCheckLinkTo, and missing <exception> docs on the new validation guards. 161/161 tests on net8.0 Release (net9.0/net10.0 not re-run this round at the user's request - this round's changes are behavior-preserving except where called out above, each with its own red/green verification). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…und by a fifth audit, and correct five documentation/tracker inaccuracies Fifth independent audit round, framed to measure convergence across rounds rather than just find more issues - found two remaining code bugs and five doc/tracker accuracy nits, all fixed here. - ApplyBatchResults now isolates each batch item in its own try/catch: a throwing ILogger on the AmbientCancellation path used to abort the whole loop, leaving every later item in the same batch stuck Running forever instead of just the one that failed. Failures are collected and rethrown together as one AggregateException once the batch is fully processed, so they still surface to the caller. - UpdateStatusName()/Status(name)'s lazy path now capture the state version and the report atomically under the same lock, instead of as two unsynchronized steps - closing a narrow window where a concurrent Update() landing in between could tie the version while the report content had already diverged. Update() correspondingly moved its SetResult/Running=false/_stateVersion++ under that same lock. - Corrected CHANGELOG.md's EventId-collision bullet (was missing the self-inflicted 105 collision), ARCHITECTURE.md's overstated Running-release invariant and its background-path wording, SwithState's imprecise ownership justification (in both the tracker and the matching source comment), the tracker's unverifiable EventId-renumbering claim, and four missing <exception> XML doc tags on the policy-registration extension methods (docs/api regenerated). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…vice resource leak found by a sixth audit, and correct further documentation drift Sixth independent audit round, against commit e41de63. No Critical or High correctness findings - both e41de63 fixes hold up (one confirmed with a deterministic negative control and a 24-thread real-service stress harness). Fixes for everything the round did find: - UpdateStatusName_ShouldNotLetAStaleConcurrentCall_OverwriteAFresherOverride failed intermittently under full-suite thread-pool contention (it blocked on Task.Run needing a pool thread within 10s). Switched to a dedicated Thread, matching this file's other concurrency tests. - ApplyBatchResults's AggregateException could silently replace Task.WhenAll's original exception (e.g. ambient cancellation) when both failed at once, via ordinary CLR exception-in-finally semantics. Both CheckHealthPlusAsync/BackGroudCheckHealthPlusAsync now capture Task.WhenAll's failure via ExceptionDispatchInfo (preserving its stack trace in the normal single-failure case) and combine it with any ApplyBatchResults failure into one AggregateException instead of one erasing the other. - HealthCheckPlusBackGroundService's _stopping CancellationTokenSource was never disposed - relevant since multi-host-per-process is an explicitly documented use case. The class now implements IDisposable (safe: it's registered via AddHostedService, and the DI container already disposes every disposable singleton on shutdown). - CHANGELOG.md under-flagged three real breaking changes now that v3.0.1 (already live on NuGet) makes this a real upgrade path, not just a hypothetical one. - docs/ARCHITECTURE.md still described the versioning scheme and lock rationale e41de63 superseded in one paragraph but not a second one covering the same mechanism, and a malformed XML doc comment on two UseHealthChecksPlus overloads dropped part of their <remarks>. - Missing/imprecise exception docs across HealthChecksPlusExtension.cs and HealthChecksPlusAppExtension.cs, and a documented, intentionally unfixed scope note on AddStatusName's default delegate and StartBatch's OOM-only orphaned-task window. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lakiness root causes found by a seventh audit, and correct a further round of documentation drift Seventh independent audit round, against commit 7493f64. No Critical or High code-correctness findings - the ExceptionDispatchInfo rewrite holds under a randomized stress test (240 observations, 0 leaked Running). The one Medium finding, corroborated independently by 2 of 3 agents, is fixed here along with everything else the round found: - HealthCheckPlusBackGroundService's IDisposable fix assumed the container always calls StopAsync before disposing it - empirically false: if a DIFFERENT IHostedService registered after this one throws from its own StartAsync, the generic host disposes the container without ever calling StopAsync first, racing _stopping's disposal against the still-running loop. Now also implements IAsyncDisposable.DisposeAsync(), which cancels and actually awaits the loop before disposing the token source - preferred automatically by any container that disposes itself asynchronously. New regression test observes the private loop Task via reflection and was confirmed red against the old immediate-dispose behavior. - Two pre-existing test-flakiness root causes, visible only under a real full-solution 3-TFM-parallel run: thread-pool starvation (fixed at the root with a new ModuleInitializer raising the process's minimum thread-pool worker count, instead of each test individually working around it) and integration tests with too-tight fixed wall-clock budgets (fixed with a new polling helper replacing fixed delays in 4 tests). Confirmed stable across repeated concurrent 3-TFM runs. - The prior round's flaky-test rewrite (Task.Run -> Thread) had two latent hazards - no IsBackground=true, and an unbounded inner Wait() - both fixed, plus the thread body now captures and rethrows any exception on the test's own thread instead of crashing the process. - The AggregateException/ExceptionDispatchInfo fix from the prior round had no dedicated assertion on its actual shape - added. - CHANGELOG.md wasn't flagging the Abstractions package-dependency fix as Breaking even though it now makes upgrading only one of the two packages from the already-published 3.0.1 fail with NU1605; also documented a previously-unlisted log this cycle already added. - A grab-bag of doc/comment inaccuracies: a second, unfixed copy of a prior round's Idle-default doc gap; ArgumentNullException/ ArgumentException tags mismapped for the null case across 4 methods; a stale comment calling an already-fixed race "not fixed here"; AddCheckLinkTo's second InvalidOperationException left undocumented; a stale Content-Type claim and a non-existent check name in README.md/README.txt; several ARCHITECTURE.md cosmetics; a pre-v3.0.0-era error message string; wrong return-type doc summaries. - Corrected 3 more inaccuracies in this file's own prior round-6 entry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e mediums, a publisher-registration-order medium, and a batch of low-severity/cosmetic findings from an eighth audit Guards every ILogger call in CacheHealthCheckPlus/DefaultHealthCheckServicePlus/HealthCheckPlusBackGroundService (SafeLog, new logging_sink_failed anomaly), fixes Status(name) diverging from a Predicate-scoped endpoint, fixes UpdateStatusName() running after a batch's own rethrow, fixes AddCheckLinkTo's locale-dependent name comparison, and fixes AddBackgroundPolicy silently letting the native HealthCheckPublisherHostedService come back if AddHealthChecks() is called again afterward. Also folds in the round's low-severity/cosmetic backlog (options->Options namespace casing, missing null guards and doc fixes in HealthReportExtensions, csproj packaging metadata, and the root cause of a previously-flaky background-service end-to-end test). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…emaining backlog one item at a time The ninth round's own UpdateStatusName() reorder had reintroduced an exception-masking bug fixed two rounds earlier, and the new AddBackgroundPolicy fail-fast guard threw even when reviving the native publisher was harmless (no publisher registered) - both fixed and covered by new tests, corroborated independently by two of three audit agents. Also resolves the remaining backlog raised by that same round: HealthCheckPlus.Abstractions no longer forces the ASP.NET Core shared runtime onto abstraction-only consumers and ships its own dedicated README instead of the main package's; Dispose() now eventually observes a background-loop fault that happens after it already returned; a StatusHealthReport delegate that calls SwitchToUnhealthy/SwitchToDegraded now fails fast instead of recursing into a StackOverflowException; the triplicated SafeLog helper is now a single shared implementation. A tempting UpdateStatusName() perf optimization (sharing one HealthReport across unfiltered named aggregates) was tried, found to reopen the cross-aggregate isolation gap the previous round's own fix closed, and reverted - documented in ARCHITECTURE.md so it isn't retried blind. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…f-attention page Concurrency pillar: fixed a real hash-collision risk in WhenReportChange (compares report content directly now instead of a 32-bit hash of it) and closed a reasoned-only TOCTOU gap in Update()'s Running check as defense in depth; three other reasoned-only Lows were analyzed and confirmed not bugs. Operational pillar re-verified clean via a final dotnet pack pass. Documentation pillar: added docs/POINTS_OF_ATTENTION.md, a plain-language list of what to know before building on this library, derived from ARCHITECTURE.md's existing internal notes rather than kept as an independent copy. Rewrote README.md/README.txt's subtitle, opening, and Features section for clarity, fixed several typos, and corrected CONTRIBUTING.md's stale "branch off main" (a ninth-round finding that had gone unfixed until now). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ep the docs set for navigability Scheduling, State, Publishing, and Logging and anomalies now live under docs/architecture/, linked from a new Table of Contents; the main file keeps only a short teaser plus a link to each, staying a map instead of a wall of text. Every internal doc cross-reference (CONTRIBUTING.md, POINTS_OF_ATTENTION.md, RUNBOOK.md, two source comments) that still pointed at the pre-split location now points directly at the subpage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cumentation links Removed a dead local|Any CPU solution configuration (no differentiated behavior anywhere in the repo) before migrating, and registered the docs that were missing from the solution's docs folder. Fixed 5 links pointing at a branch this repository has never had (master, not main), and converted the root README's self-referential links to relative paths so they resolve correctly regardless of which branch they're viewed on. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… v4.0.0 hardening cycle The repository never had any architecture decision record control. Reviewed the full main...develop diff with the adrplus decision-check agent and recorded the 9 decisions that warranted one: native metrics instrumentation, deriving the tracked check set from DI registrations, per-host state isolation, the fail-fast-at-startup doctrine, the logging/metrics/background delegate guard pattern, the centralized EventId catalog, the immutable snapshot pattern for concurrent state, the Abstractions packaging changes, and depending only on supported public contracts for framework integration. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e temporary tracker All 9 architecture decision records are now Accepted. Added docs/RELEASE_METHODOLOGY.md, describing how this release's quality was verified and its outcome, linked from README.md and CHANGELOG.md. Removed TODO/ (the pt-BR working tracker this hardening cycle was logged in) now that the migration work is finalized - its durable record lives in docs/RELEASE_METHODOLOGY.md and docs/adr/ instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| reportsByName.Add((item.Key, item.Value, BuildReport(item.Value.IncludeName, excludeNeverRun: false))); | ||
| } | ||
| } | ||
| _isUpdatingStatusNames = true; |
| } | ||
| finally | ||
| { | ||
| _isUpdatingStatusNames = false; |
Comment on lines
+541
to
+545
| catch (Exception ex) | ||
| { | ||
| SafeLog(() => _logger.LogWarning(MetricsRecordingErrorEventId, ex, | ||
| "Recording metrics for health check '{HealthCheckName}' failed; the check result itself was not affected.", key)); | ||
| } |
| { | ||
| _countIdletopublish = 0; | ||
|
|
||
| CancellationTokenSource? publishCancellation = null; |
Comment on lines
+229
to
+246
| catch (Exception ex) | ||
| { | ||
| // Each failing publisher already logged its own error/timeout and | ||
| // recorded the "error" metric inside RunPublisherAsync (with which | ||
| // publisher, duration, and exception) - this also covers a publisher | ||
| // that didn't finish within Timeout, since RunPublisherAsync's own | ||
| // timeout catch (observing this same linked token) logs/metrics it | ||
| // and rethrows. This log adds the signal that was otherwise missing: | ||
| // that the background loop is continuing despite the failure above, | ||
| // instead of leaving no operational trace of whether it's still | ||
| // alive or has silently died - without this try/catch (unlike the | ||
| // check-execution block above it, which already has one), | ||
| // Task.WhenAll's rethrown exception would fault the loop's | ||
| // fire-and-forget Task silently. | ||
| SafeLog(() => Log.HealthCheckPublisherCycleError(_logger, ex)); | ||
|
|
||
| SafeRecordMetric(() => HealthCheckPlusMetrics.RecordAnomaly(AnomalyReason.PublisherCycleFailedButContinued)); | ||
| } |
Comment on lines
+528
to
531
| { | ||
| SafeLog(() => Log.HealthCheckPublisherMetricsRecordingError(_logger, ex)); | ||
| } | ||
| } |
Comment on lines
+171
to
+174
| catch (Exception) | ||
| { | ||
| RecordAnomaly(AnomalyReason.LoggingSinkFailed); | ||
| } |
Comment on lines
+102
to
+113
| catch (Exception ex) | ||
| { | ||
| // A consumer-supplied IDisposable.Dispose() throwing must not abort the loop: | ||
| // without this try/catch, every adopted check after the first failing one would | ||
| // be silently left undisposed for the rest of process shutdown, with no signal | ||
| // anywhere that it happened. SafeLog guards the log call itself for the same | ||
| // reason - a throwing ILogger sink here must not reintroduce the exact bug this | ||
| // try/catch exists to prevent. | ||
| SafeLog(() => Log.HealthCheckDisposeError(_logger, name, ex)); | ||
|
|
||
| SafeRecordMetric(() => HealthCheckPlusMetrics.RecordAnomaly(AnomalyReason.AdoptedCheckDisposeFailed)); | ||
| } |
Comment on lines
+127
to
+130
| catch (Exception metricsEx) | ||
| { | ||
| SafeLog(() => Log.HealthCheckMetricsRecordingError(_logger, metricsEx)); | ||
| } |
Comment on lines
+597
to
+600
| catch (Exception ex) | ||
| { | ||
| if (sta.DateRef.Add(itemToRum.Period.Value) < DateTime.Now) | ||
| { | ||
| _cacheStatus.Running(item.Name, true); | ||
| registrationstorun.Add(itemToRum); | ||
| } | ||
| whenAllFailure = ExceptionDispatchInfo.Capture(ex); | ||
| } |
… the metrics test BackgroundService_ShouldRecordPublished_ThenSkippedNoChange_AsReportStaysTheSame assumed exactly one "published" duration measurement (.Single()), but the check can legitimately miss the very first idle cycle if it hasn't run yet by then - that cycle publishes an empty report, and the next cycle genuinely republishes once the check's real result changes the report. Both are real publishes, not a bug; asserting "at least one" (matching the pattern already used by BackgroundService_ShouldExcludeNotYetRunChecks_FromThePublishedReport) is what the test actually needs. Reproduced by temporarily widening the check's own delay to force the race deterministically, confirmed the fix tolerates it, then reverted the delay and ran the full suite (188/188 across net8.0/net9.0/net10.0). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HealthCheckPlus v4.0.0 — hardening cycle
This PR merges the full v4.0.0 hardening cycle from
developintomain. It closes 134 findings raised across several independent, blind review rounds (concurrency/shared-state correctness, documentation-vs-code accuracy, and operational viability under real failure conditions), adds native metrics, reorganizes documentation, and introduces this project's first Architecture Decision Records.See
CHANGELOG.mdfor the full itemized list — this description covers the highlights.Breaking changes
AddHealthChecksPlus()no longer takes anamesparameter — the tracked set is now derived directly from what's actually registered (AddCheckPlus/AddCheckLinkTo/any nativeIHealthChecksBuilderextension), so it can no longer drift from reality.AddUnhealthyPolicy/AddDegradedPolicy/AddCheckPlus/AddCheckLinkTonow fail fast at startup if a policy names a check that was never registered (typo protection).AddBackgroundPolicynow fails fast if called twice for the sameIServiceCollection, and also fails fast if the nativeHealthCheckPublisherHostedServicereappears (e.g. a laterAddHealthChecks()call) while a publisher is registered.HealthCheckPluspackage now declares a real dependency onHealthCheckPlus.Abstractionsinstead of embedding a private copy of its DLL — projects referencing both packages must upgrade them together (NU1605otherwise).EventIds are now assigned from one centralized catalog (fixes several collisions) —EventNamestrings are unchanged, but numeric IDs shift; update any alerting keyed on the numeric ID.Optionsnamespace casing changed fromHealthCheckPlus.optionstoHealthCheckPlus.Options.HealthReportExtensions/IStateHealthChecksPlusmembers now throwArgumentException/ArgumentNullExceptioninstead ofNullReferenceException/KeyNotFoundException— update any narrowcatchblocks written against the old exception types.IHealthCheckPlusPolicyStatusinterface (3.0.1) was removed;HealthCheckPlusPolicyStatusis the concrete type consumers already use.Correctness fixes
Several data races and TOCTOU-shaped bugs around concurrent check scheduling/execution were closed: torn reads of a cached result, a check left permanently
Runningafter a failure mid-release, double-scheduling the same check from two concurrent callers, a named status aggregate (Status(name)) losing a manual override under concurrent writes, and a background-service shutdown path that could leak or silently fail to observe a faulted loop.Nearly every place a throwing
ILoggersink could previously break something other than logging (killing the background loop, turning a successful check into a 500, dropping a publisher dispatch, masking a real exception) is now defended, surfacing as alogging_sink_failedanomaly instead.Observability
System.Diagnostics.Metricsinstrumentation for check executions, status transitions, and publisher invocations — no new package dependency.Documentation
ARCHITECTURE.mdinto a lean overview + linked subpages underdocs/architecture/for the densest topics.docs/POINTS_OF_ATTENTION.md— a plain-language list of what to know before integrating the library.docs/RELEASE_METHODOLOGY.md— how this release's quality was verified.docs/adr/— the first 9 ADRs for this project, covering native metrics, per-host state isolation, the fail-fast doctrine, the logging/metrics/delegate guard pattern, the centralizedEventIdcatalog, the immutable-snapshot state model,Abstractionspackaging, and depending only on supported public contracts..slnxand fixed broken/branch-dependent doc links.Quality