From 3ef43ca926e9f12b4a724a45aad92a2e8d649fba Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 9 Nov 2026 14:54:12 -0500 Subject: [PATCH] fix(overview): P3 review fixes 003-005 - no redirects on probes, per-target publish so a hung instance cannot stale the rest, probe docs (scadaproj#48) Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV --- ZB.MOM.WW.Overview/CLAUDE.md | 7 ++ .../Polling/OverviewPollerService.cs | 110 ++++++++++++++---- .../PollingServiceCollectionExtensions.cs | 6 +- .../Registry/OverviewOptions.cs | 13 ++- .../OverviewPollerServiceTests.cs | 49 ++++++++ .../ProbeRedirectTests.cs | 61 ++++++++++ code-reviews/Overview/findings.md | 14 +-- code-reviews/README.md | 13 +-- .../2026-07-22-overview-dashboard-design.md | 4 +- 9 files changed, 233 insertions(+), 44 deletions(-) create mode 100644 ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/ProbeRedirectTests.cs diff --git a/ZB.MOM.WW.Overview/CLAUDE.md b/ZB.MOM.WW.Overview/CLAUDE.md index 1531c5d..6ec41df 100644 --- a/ZB.MOM.WW.Overview/CLAUDE.md +++ b/ZB.MOM.WW.Overview/CLAUDE.md @@ -4,6 +4,13 @@ The hosted family dashboard (Blazor Server). Build/test from this directory: `do **Backlog / open work:** lives in Gitea issues on `dohertj2/scadaproj` (`tea issues list --repo dohertj2/scadaproj`) — currently #14 (re-run the two OtOpcUa-blocked Phase 4 acceptance checks). The history below is a record, not a status tracker; file new follow-ups as issues. +## Probe behaviour (P3 review fixes, scadaproj#48) + +- The probe client does not follow redirects (Overview-003): an auth-wall 302 on `/health/active` maps to Unknown, not Active. +- Probes are not gated by the sweep (Overview-004): each target publishes its own result when it lands, and a target still + being probed is skipped by later ticks, so a hung instance with a long `TimeoutSeconds` cannot stale the others. +- `/health/active` is probed whenever ready answered; `HasActiveRole` only gates the Active/Standby interpretation (Overview-005). + ## Index history (migrated from the scadaproj root CLAUDE.md) > Moved verbatim out of the scadaproj root `CLAUDE.md` on 2026-09-03 so it loads only when needed. **This file is now the authoritative place for this history — append new entries here and keep the root index row short.** diff --git a/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/OverviewPollerService.cs b/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/OverviewPollerService.cs index 8ca4893..3a10591 100644 --- a/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/OverviewPollerService.cs +++ b/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/OverviewPollerService.cs @@ -33,6 +33,7 @@ public sealed class OverviewPollerService : BackgroundService private readonly OverviewMetrics _metrics; private readonly PollCycleHeartbeat _heartbeat; private readonly List _targets; + private readonly object _publishLock = new(); /// Initializes a new . /// The validated registry. @@ -81,21 +82,18 @@ public sealed class OverviewPollerService : BackgroundService using var timer = new PeriodicTimer(tick, _timeProvider); + // Sweeps are NOT awaited by the tick loop (Overview-004): a hung instance with a long + // timeout override must not delay the next tick for every other instance. A target that is + // still being probed is skipped by later sweeps (see PollTarget.TryClaim), so slow probes + // never pile up, and each probe publishes its own result as soon as it lands. + var pending = new List(); + try { do { - try - { - await SweepAsync(stoppingToken).ConfigureAwait(false); - } - catch (Exception ex) when (!stoppingToken.IsCancellationRequested) - { - // One bad sweep must not end the loop: a faulted BackgroundService stops the - // whole host under the default StopHost policy (Overview-001). The heartbeat is - // not stamped, so the dashboard's own health check still notices the gap. - _logger.LogError(ex, "Overview poll sweep failed; retrying on the next tick."); - } + pending.RemoveAll(t => t.IsCompleted); + pending.Add(SweepSafeAsync(stoppingToken)); } while (await timer.WaitForNextTickAsync(stoppingToken).ConfigureAwait(false)); } @@ -103,6 +101,37 @@ public sealed class OverviewPollerService : BackgroundService { // Ordinary shutdown. } + finally + { + // Probes observe the cancelled token; let them unwind before the host tears down the client. + try + { + await Task.WhenAll(pending).ConfigureAwait(false); + } + catch + { + // SweepSafeAsync already logged; nothing to add at shutdown. + } + } + } + + private async Task SweepSafeAsync(CancellationToken stoppingToken) + { + try + { + await SweepAsync(stoppingToken).ConfigureAwait(false); + } + catch (Exception ex) when (!stoppingToken.IsCancellationRequested) + { + // One bad sweep must not end the loop: a faulted BackgroundService stops the + // whole host under the default StopHost policy (Overview-001). The heartbeat is + // not stamped, so the dashboard's own health check still notices the gap. + _logger.LogError(ex, "Overview poll sweep failed; retrying on the next tick."); + } + catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested) + { + // Ordinary shutdown. + } } /// Runs one sweep: probes every due instance and publishes the resulting snapshot. @@ -118,7 +147,8 @@ public sealed class OverviewPollerService : BackgroundService var startedAt = _timeProvider.GetTimestamp(); var now = _timeProvider.GetUtcNow(); - var due = _targets.Where(t => IsDue(t, now)).ToList(); + // TryClaim last: only a target that is actually going to be probed may be marked in flight. + var due = _targets.Where(t => IsDue(t, now) && t.TryClaim()).ToList(); if (due.Count > 0) { @@ -127,12 +157,9 @@ public sealed class OverviewPollerService : BackgroundService var client = _httpClientFactory.CreateClient(HttpClientName); var probe = new ZbHealthReportClient(client); - var results = await Task - .WhenAll(due.Select(target => ProbeAsync(probe, target, now, cancellationToken))) + await Task + .WhenAll(due.Select(target => ProbeAndPublishAsync(probe, target, now, cancellationToken))) .ConfigureAwait(false); - - for (var i = 0; i < due.Count; i++) - due[i].Last = results[i]; } // Re-checked after the fan-out: a shutdown that lands mid-sweep leaves the probed targets @@ -140,8 +167,12 @@ public sealed class OverviewPollerService : BackgroundService // misleading picture into the store for as long as the process takes to exit. cancellationToken.ThrowIfCancellationRequested(); - var snapshot = BuildSnapshot(now); - _store.Publish(snapshot); + OverviewSnapshot snapshot; + lock (_publishLock) + { + snapshot = BuildSnapshot(now); + _store.Publish(snapshot); + } _metrics.RecordSweep(_timeProvider.GetElapsedTime(startedAt)); @@ -152,6 +183,35 @@ public sealed class OverviewPollerService : BackgroundService return snapshot; } + /// + /// Probes one target, then records and publishes its result immediately — the sweep's remaining + /// laggards must not hold back a result that is already in hand (Overview-004). + /// + private async Task ProbeAndPublishAsync( + ZbHealthReportClient probe, + PollTarget target, + DateTimeOffset now, + CancellationToken cancellationToken) + { + try + { + var result = await ProbeAsync(probe, target, now, cancellationToken).ConfigureAwait(false); + + lock (_publishLock) + { + target.Last = result; + + // A shutdown mid-probe must not publish a half-probed mix (see SweepAsync). + if (!cancellationToken.IsCancellationRequested) + _store.Publish(BuildSnapshot(now)); + } + } + finally + { + target.Release(); + } + } + private bool IsDue(PollTarget target, DateTimeOffset now) => target.Last?.LastPolledUtc is not { } polled || now - polled + DueTolerance >= target.Timings.PollInterval; @@ -283,9 +343,17 @@ public sealed class OverviewPollerService : BackgroundService public required InstanceSnapshot Template { get; init; } /// - /// The last published result, or null before the first probe. Written only by the sweep - /// loop after its Task.WhenAll has completed, so it is never touched concurrently. + /// The last published result, or null before the first probe. Written only under the + /// poller's publish lock, by the probe task that holds this target's claim. /// public InstanceSnapshot? Last { get; set; } + + private int _inFlight; + + /// Claims the target for one probe; false when a previous probe is still running. + public bool TryClaim() => Interlocked.CompareExchange(ref _inFlight, 1, 0) == 0; + + /// Releases the claim taken by . + public void Release() => Volatile.Write(ref _inFlight, 0); } } diff --git a/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/PollingServiceCollectionExtensions.cs b/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/PollingServiceCollectionExtensions.cs index 470b5cf..dedd1c7 100644 --- a/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/PollingServiceCollectionExtensions.cs +++ b/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/PollingServiceCollectionExtensions.cs @@ -31,7 +31,11 @@ public static class PollingServiceCollectionExtensions // Health bodies are small and must not be served from anywhere but the instance itself. client.DefaultRequestHeaders.CacheControl = new() { NoCache = true, NoStore = true }; - }); + }) + // A followed redirect would turn an auth wall's 200 login page into "Active" + // (Overview-003). With redirects off, a 3xx surfaces as its own status code and the + // active tier maps it to Unknown; the ready tier already requires a parseable body. + .ConfigurePrimaryHttpMessageHandler(() => new SocketsHttpHandler { AllowAutoRedirect = false }); services.AddHostedService(); diff --git a/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Registry/OverviewOptions.cs b/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Registry/OverviewOptions.cs index 127aa27..745d3bb 100644 --- a/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Registry/OverviewOptions.cs +++ b/ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Registry/OverviewOptions.cs @@ -5,9 +5,11 @@ namespace ZB.MOM.WW.Overview.Registry; /// Overview configuration section and validated by . /// /// -/// Every instance is probed the same way — GET {BaseUrl}/health/ready, plus -/// {BaseUrl}/health/active when . There is -/// deliberately no per-instance probe-mode switch: one probe model, one status derivation. +/// Every instance is probed the same way — GET {BaseUrl}/health/ready, then +/// {BaseUrl}/health/active whenever ready answered, regardless of +/// (single-instance gateways publish their session counts +/// on the active tier). only controls whether an +/// Active/Standby badge is derived from the active answer. There is deliberately no per-instance probe-mode switch: one probe model, one status derivation. /// public sealed class OverviewOptions { @@ -75,8 +77,9 @@ public sealed class InstanceEntry /// /// Whether this instance participates in an active/standby pair. False for the single-instance - /// gateways, which have no such concept — their cards show status without an Active badge and - /// are never probed for /health/active. + /// gateways, which have no such concept — their cards show status without an Active badge. The + /// active tier is still probed for them (it carries their session counts); this flag only gates + /// the Active/Standby interpretation. /// public bool HasActiveRole { get; set; } diff --git a/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/OverviewPollerServiceTests.cs b/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/OverviewPollerServiceTests.cs index bdb836e..99c8896 100644 --- a/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/OverviewPollerServiceTests.cs +++ b/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/OverviewPollerServiceTests.cs @@ -401,6 +401,55 @@ public class OverviewPollerServiceTests Assert.All(snapshot.AllInstances, i => Assert.Equal(InstanceStatus.Up, i.Status)); } + [Fact] + public async Task Sweep_HungInstanceDoesNotHoldUpResultsOrLaterSweepsForTheOthers() + { + // Overview-004: a hung instance (long timeout override) used to gate the whole sweep, so + // every other instance went Stale between polls. The fast instance must publish as soon as + // it answers, and a later sweep must not wait for the hung one. + var handler = new RoutingHandler() + .Json("http://fast:8080/health/ready", HttpStatusCode.OK, HealthBody.Ready("Healthy")) + .Json("http://slow:8080/health/ready", HttpStatusCode.OK, HealthBody.Ready("Healthy")); + var options = Registry(App("App", Node("fast", "http://fast:8080"), Node("slow", "http://slow:8080"))); + var time = new FakeTimeProvider(Start); + var (poller, store) = Build(options, new HangingHandler(handler, "slow"), time); + + var first = poller.SweepAsync(); + + // The fast result is published while the slow probe is still hanging. + await WaitUntilAsync(() => store.Current.AllInstances.Any(i => i.Instance == "fast" && i.Status == InstanceStatus.Up)); + Assert.False(first.IsCompleted); + + time.Advance(TimeSpan.FromSeconds(11)); + var second = await poller.SweepAsync().WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.False(first.IsCompleted); + Assert.Equal(2, ReadyProbes(handler, "fast")); + Assert.Equal(InstanceStatus.Up, second.AllInstances.Single(i => i.Instance == "fast").Status); + } + + private static async Task WaitUntilAsync(Func condition) + { + var deadline = DateTime.UtcNow.AddSeconds(10); + while (!condition()) + { + Assert.True(DateTime.UtcNow < deadline, "condition not met in time"); + await Task.Delay(10); + } + } + + private sealed class HangingHandler(RoutingHandler inner, string hangingHost) : HttpMessageHandler + { + protected override async Task SendAsync( + HttpRequestMessage request, CancellationToken cancellationToken) + { + if (request.RequestUri!.Host == hangingHost) + await Task.Delay(Timeout.Infinite, cancellationToken).ConfigureAwait(false); + + return await inner.HandleAsync(request).ConfigureAwait(false); + } + } + [Fact] public async Task Sweep_UnregisteredInstances_RenderAsPendingNotAsHealthy() { diff --git a/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/ProbeRedirectTests.cs b/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/ProbeRedirectTests.cs new file mode 100644 index 0000000..59f0c0a --- /dev/null +++ b/ZB.MOM.WW.Overview/tests/ZB.MOM.WW.Overview.Tests/ProbeRedirectTests.cs @@ -0,0 +1,61 @@ +using System.Net; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.Overview.Polling; + +namespace ZB.MOM.WW.Overview.Tests; + +/// Overview-003: the pooled probe client must not follow redirects. +public sealed class ProbeRedirectTests +{ + [Fact] + public async Task ActiveProbe_AuthRedirectToLoginPage_IsNotReportedAsActive() + { + var port = FreePort(); + var prefix = $"http://127.0.0.1:{port}/"; + using var listener = new HttpListener(); + listener.Prefixes.Add(prefix); + listener.Start(); + var server = Task.Run(async () => + { + while (listener.IsListening) + { + HttpListenerContext ctx; + try { ctx = await listener.GetContextAsync(); } catch { return; } + if (ctx.Request.Url!.AbsolutePath == "/login") + { + ctx.Response.StatusCode = 200; + ctx.Response.ContentType = "text/html"; + await ctx.Response.OutputStream.WriteAsync("login"u8.ToArray()); + } + else + { + ctx.Response.StatusCode = 302; + ctx.Response.RedirectLocation = "/login"; + } + ctx.Response.Close(); + } + }); + + var services = new ServiceCollection(); + services.AddLogging(); + services.AddOverviewPolling(); + await using var provider = services.BuildServiceProvider(); + var client = provider.GetRequiredService().CreateClient(OverviewPollerService.HttpClientName); + + var probe = await new ZbHealthReportClient(client) + .ProbeStatusAsync($"{prefix}health/active", TimeSpan.FromSeconds(5)); + + listener.Stop(); + Assert.Equal(302, probe.StatusCode); + Assert.Equal(ActiveState.Unknown, StatusDerivation.ObserveActive(true, probe)); + } + + private static int FreePort() + { + var l = new System.Net.Sockets.TcpListener(IPAddress.Loopback, 0); + l.Start(); + var port = ((IPEndPoint)l.LocalEndpoint).Port; + l.Stop(); + return port; + } +} diff --git a/code-reviews/Overview/findings.md b/code-reviews/Overview/findings.md index bf6a67c..b44ebeb 100644 --- a/code-reviews/Overview/findings.md +++ b/code-reviews/Overview/findings.md @@ -10,7 +10,7 @@ | Last reviewed | 2026-11-09 | | Reviewer | Claude | | Commit reviewed | `91fb2b4` | -| Open findings | 3 | +| Open findings | 0 | ## Summary @@ -183,7 +183,7 @@ Resolved 2026-11-09 in the commit tagged `(scadaproj#43)` — `LeaderResolver.Re | Severity | Low | | Priority | P3 | | Category | Correctness & logic bugs | -| Status | Open | +| Status | Resolved | | Location | `ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/ZbHealthReportClient.cs:112` (client config `ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/PollingServiceCollectionExtensions.cs:25`) | **Description** @@ -216,7 +216,7 @@ so a 3xx maps to `Unknown`. Optionally, for a non-empty body that is not health **Resolution** -_Unresolved._ +Resolved 2026-11-09 in the commit tagged `(scadaproj#48)` — the named probe `HttpClient` now uses `SocketsHttpHandler { AllowAutoRedirect = false }`, so a 3xx on `/health/active` surfaces as its own status code and `ObserveActive` maps it to Unknown (the ready tier already requires a parseable body). Regression test `ProbeRedirectTests.ActiveProbe_AuthRedirectToLoginPage_IsNotReportedAsActive` (real loopback listener, 302 to a 200 login page; failed without the fix). Not done: the optional 'non-empty non-JSON 200 body is Unknown' rule — a plain-text 200 from a non-ZbHealth app on the active tier is a legitimate Active signal, so that needs an owner decision if wanted. ### Overview-004 — One slow instance gates the single sweep, so every other instance goes Stale @@ -225,7 +225,7 @@ _Unresolved._ | Severity | Low | | Priority | P3 | | Category | Error handling & resilience | -| Status | Open | +| Status | Resolved | | Location | `ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Polling/OverviewPollerService.cs:120` (sequential ready + active at `:155-171`; validator `ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Registry/OverviewOptionsValidator.cs:69-76`) | **Description** @@ -257,7 +257,7 @@ registry, naming the offending override. **Resolution** -_Unresolved._ +Resolved 2026-11-09 in the commit tagged `(scadaproj#48)` — chose decoupling over a validator rule. The tick loop no longer awaits the sweep; each target is claimed (`TryClaim`) while its probe is in flight, so later ticks skip a hung target, and each probe records and publishes its own result the moment it completes (publish under a lock). `SweepAsync` still awaits its own batch and returns the final snapshot, so existing semantics for callers and tests hold. Regression test `Sweep_HungInstanceDoesNotHoldUpResultsOrLaterSweepsForTheOthers` (failed without the fix). Side effect: the heartbeat/sweep metric now describe the batch started on each tick, and a tick whose targets are all still in flight stamps the heartbeat immediately (the loop is alive; the probes are bounded by their timeouts). ### Overview-005 — Docs say `/health/active` is probed only when `HasActiveRole`; the code probes it for every reachable instance @@ -266,7 +266,7 @@ _Unresolved._ | Severity | Low | | Priority | P3 | | Category | Documentation & XML docs | -| Status | Open | +| Status | Resolved | | Location | `ZB.MOM.WW.Overview/src/ZB.MOM.WW.Overview/Registry/OverviewOptions.cs:9` (also `:76-80`) | **Description** @@ -298,4 +298,4 @@ shown. **Resolution** -_Unresolved._ +Resolved 2026-11-09 in the commit tagged `(scadaproj#48)` — corrected the `OverviewOptions` remarks, the `InstanceEntry.HasActiveRole` XML doc, design doc §3/§5 and the Overview CLAUDE.md: the active tier is probed whenever ready answered and `HasActiveRole` only gates the Active/Standby badge. diff --git a/code-reviews/README.md b/code-reviews/README.md index 15a22ad..6ccb5ff 100644 --- a/code-reviews/README.md +++ b/code-reviews/README.md @@ -40,7 +40,7 @@ code-reviews/ ## Summary -10 of 10 libraries reviewed. 38 pending findings across all libraries. +10 of 10 libraries reviewed. 35 pending findings across all libraries. Coverage: Secrets, LocalDb, GalaxyRepository and Overview first reviewed, and Auth, Health, Theme, Audit, Configuration and Telemetry re-reviewed, on 2026-11-09 (#15). Each finding also @@ -51,8 +51,8 @@ carries a P1/P2/P3 priority (P1 = Critical/High, P2 = Medium, P3 = Low). | Critical | 0 | | High | 0 | | Medium | 11 | -| Low | 27 | -| **Total** | **38** | +| Low | 24 | +| **Total** | **35** | ## Library Status @@ -64,7 +64,7 @@ carries a P1/P2/P3 priority (P1 = Critical/High, P2 = Medium, P3 = Low). | [GalaxyRepository](GalaxyRepository/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/0 | 0 | 8 | | [Health](Health/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/1 | 1 | 7 | | [LocalDb](LocalDb/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/4/3 | 7 | 7 | -| [Overview](Overview/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/3 | 3 | 5 | +| [Overview](Overview/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/0 | 0 | 5 | | [Secrets](Secrets/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/6/13 | 19 | 19 | | [Telemetry](Telemetry/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/3 | 3 | 11 | | [Theme](Theme/findings.md) | 2026-11-09 | `91fb2b4` | 0/0/0/1 | 1 | 8 | @@ -100,7 +100,7 @@ _None open._ | Secrets-013 | [Secrets](Secrets/findings.md) | UI add/rotate/delete: if the audit write throws after the store write succeeded, the change is reported and audited as a failure and the resolver cache is not evicted | | Secrets-017 | [Secrets](Secrets/findings.md) | `secret set`/`rotate` take the plaintext on argv; the host builder re-parses argv as configuration and can crash with the value in the error | -### Low (27) +### Low (24) | ID | Library | Title | |----|---------|-------| @@ -111,9 +111,6 @@ _None open._ | LocalDb-005 | [LocalDb](LocalDb/findings.md) | Auto-assigned rowid / AUTOINCREMENT keys collide across nodes and LWW silently drops one row | | LocalDb-006 | [LocalDb](LocalDb/findings.md) | Column names containing `'` are captured but break the apply and snapshot SQL | | LocalDb-007 | [LocalDb](LocalDb/findings.md) | README is stale on version, adoption, test count, `Synchronous` values and baseline scope | -| Overview-003 | [Overview](Overview/findings.md) | Status-only active probe treats any 200, including a followed redirect to a login page, as Active | -| Overview-004 | [Overview](Overview/findings.md) | One slow instance gates the single sweep, so every other instance goes Stale | -| Overview-005 | [Overview](Overview/findings.md) | Docs say `/health/active` is probed only when `HasActiveRole`; the code probes it for every reachable instance | | Secrets-004 | [Secrets](Secrets/findings.md) | Corrupt or wrong-length crypto fields escape `Decrypt` as `ArgumentException`: not audited, and breaks the cipher contract | | Secrets-005 | [Secrets](Secrets/findings.md) | Store-path rules are lexical: drive-relative paths pass "rooted", and `\\?\` / UNC / 8.3 / symlink spellings bypass "under content root" | | Secrets-006 | [Secrets](Secrets/findings.md) | Resolver cache can re-cache a stale value after `Invalidate`, and expired plaintext is never evicted | diff --git a/docs/plans/2026-07-22-overview-dashboard-design.md b/docs/plans/2026-07-22-overview-dashboard-design.md index 401c88e..79ac0bf 100644 --- a/docs/plans/2026-07-22-overview-dashboard-design.md +++ b/docs/plans/2026-07-22-overview-dashboard-design.md @@ -78,7 +78,7 @@ ZB.MOM.WW.Overview/ # plain directory in scadaproj — NOT All four family apps are first-class registry citizens; `HasActiveRole` is simply omitted for the two single-instance gateways (mxgw, HistorianGateway), which have no active/standby concept — their cards show status without the Active badge. -- Every instance is probed the same way: GET `{BaseUrl}/health/ready`, plus `{BaseUrl}/health/active` when `HasActiveRole`. There is deliberately no per-instance probe-mode switch — one probe model, one status derivation. +- Every instance is probed the same way: GET `{BaseUrl}/health/ready`, then `{BaseUrl}/health/active` whenever ready answered (single-instance gateways publish session counts there; `HasActiveRole` only gates the Active/Standby badge — Overview-005). There is deliberately no per-instance probe-mode switch — one probe model, one status derivation. - `PollIntervalSeconds`/`TimeoutSeconds`/`StaleAfterSeconds` are overridable per application and per instance (instance wins) for slow links or chatty pairs; `StaleAfterSeconds` must exceed `PollIntervalSeconds` (validated). - Bound + validated with `AddValidatedOptions` (`ZB.MOM.WW.Configuration`): non-empty registry, absolute http(s) URLs, unique app/instance names, positive intervals, stale > poll. Malformed registry fails boot via `ConfigPreflight`, not the first poll. @@ -112,7 +112,7 @@ One `OverviewPollerService : BackgroundService`: - Parses the canonical shape `{ status, totalDurationMs, entries: { name: { status, description, durationMs } } }`. Parsing lives in a small `ZbHealthReportClient` inside this app for v1; promoting it into a `ZB.MOM.WW.Health.Client` package is a deliberate later step, not a v1 dependency (recommendation: promote only when a second consumer appears). - Derived per-instance state: - **Up** — ready 200 + `"Healthy"`; **Degraded** — 200 + `"Degraded"`; **Down** — 503 / `"Unhealthy"`; **Unreachable** — timeout, refused, DNS, non-JSON (kept distinct from Down: it usually means network/VPN/registry-typo, not the app). - - **Active/Standby** — `/health/active` 200 → Active, 503 → Standby, error → Unknown (only when `HasActiveRole`). + - **Active/Standby** — `/health/active` 200 → Active, 503 → Standby, error → Unknown (badge shown only when `HasActiveRole`; redirects are not followed, so a 3xx → Unknown — Overview-003). - Plus: last poll UTC, latency ms, per-check entries, consecutive-failure count (state flips to Down/Unreachable only after **2** consecutive failures to avoid single-blip flapping; recovery is immediate). - **Cache-first rendering:** results land in an in-memory `OverviewSnapshotStore` (single immutable snapshot swapped atomically); an event notifies circuits to re-render. Page loads NEVER trigger a poll — they read the cached snapshot and render instantly; the poller timer is the only thing that does HTTP. No persistence — a restart repolls the world within one cadence. - **Staleness:** every instance snapshot carries `LastUpdatedUtc`. When the UI renders (or a periodic sweep runs), any instance whose snapshot age exceeds its effective `StaleAfterSeconds` is shown as **Stale (out of date/offline)** regardless of its last known status — this catches a stuck poller, a paused refresh, or a machine-slept dashboard, not just failed probes.