diff --git a/archreview/03-server-runtime.md b/archreview/03-server-runtime.md index b6c37ceb..66b212fe 100644 --- a/archreview/03-server-runtime.md +++ b/archreview/03-server-runtime.md @@ -175,9 +175,8 @@ Closed since 9cad9ed0: **hard-kill failover** (`HardKillFailoverTests`, in-proce **U9 — LOW: `EnsureVariable` silently ignores changed historize-intent on an existing node** (`OtOpcUaNodeManager.cs:1345-1349`) — invariant still lives in two files' comments rather than an assert. -**U10 — LOW (NEW): The `DriverResilienceStatusTracker` has no reader — pipeline observability is log-only.** -The tracker is registered, fed by every retry/breaker event, and consumed by nothing (Admin `/hosts` Stream E.2/E.3 never shipped). The interim mitigation (retry/breaker-open/breaker-close logging in `DriverResiliencePipelineBuilder.cs:124-170`, added with #10) is real and correctly gated, but breaker state — now a production behavior that can reject calls (`BrokenCircuitException`) — is invisible to operators except in logs. Known residual from FOLLOWUP-10; recorded here so the review tracks it. -*Recommendation:* ship the `/hosts` resilience column, or at minimum a metric per breaker-open. +**U10 — LOW (NEW): The `DriverResilienceStatusTracker` has no reader — pipeline observability is log-only.** — ✅ **REMEDIATED** (R2-10, branch `r2/10-resilience-observability`). The tracker now has a production reader: a periodic (5 s) full-snapshot publisher (`DriverResilienceStatusPublisherService`, driver nodes, registered next to the tracker with a wiring guard `ResilienceStatusReaderRegistrationTests`) → `driver-resilience-status` DPS topic → per-admin-node `DriverResilienceStatusBridge` → `IDriverResilienceStatusStore` → a resilience section on `DriverStatusPanel.razor` (breaker-OPEN / consecutive-failures / in-flight / staleness chips, read in-process per the self-HubConnection ban). Two tracker truthfulness gaps fixed along the way: `RecordBreakerClose` + `IsBreakerOpen` (breaker-open state is now derivable) and `RecordSuccess` wired onto the invoker success paths (`ConsecutiveFailures` is now a true consecutive counter). Mirrors the driver-health flow one-for-one. *(Original finding:)* The tracker was registered, fed by every retry/breaker event, and consumed by nothing (Admin `/hosts` Stream E.2/E.3 never shipped); the interim mitigation was retry/breaker logging in `DriverResiliencePipelineBuilder.cs`. +*Live gate (T10) pending:* docker-dev breaker-open observation on both centrals + staleness dim. --- diff --git a/archreview/04-adminui.md b/archreview/04-adminui.md index 9b790c74..cbc275b3 100644 --- a/archreview/04-adminui.md +++ b/archreview/04-adminui.md @@ -240,7 +240,9 @@ There is **no bUnit** — 77 razor components have no render/binding coverage, w ~31 `aria-*`/`role=` attributes across 77 components, concentrated in the modals (`role="dialog"`, `btn-close` aria-labels). Gaps: text-glyph expander buttons (`▼`/`▶` in `UnsTree.razor:49`) with no `aria-expanded`/label; modals have no focus trap or Escape handling; live tails (`Alerts`) have no `aria-live` region; tables lack captions/scope. Form labeling is decent (most inputs have `for`/`id` pairs). Acceptable for an internal ops tool; worth a targeted pass on the tree and the modals if operator diversity matters. -### U-5 (NEW) — Resilience runtime status has no AdminUI surface; overrides are authored blind +### U-5 (NEW) — Resilience runtime status has no AdminUI surface; overrides are authored blind — ✅ REMEDIATED (R2-10) + +**Remediated** on branch `r2/10-resilience-observability`: `DriverResilienceStatusTracker` now flows to the AdminUI through exactly the recommended idiom — a per-node DPS bridge (`DriverResilienceStatusBridge`) → in-process store (`IDriverResilienceStatusStore`, registered beside `IDriverStatusSnapshotStore` in `AddOtOpcUaDriverStatusServices`) → a **resilience section on `DriverStatusPanel.razor`** (embedded on all 8 driver pages, right where the `ResilienceConfig` overrides are authored — closing the feedback loop). It renders breaker-OPEN / consecutive-failures / in-flight / staleness chips, read in-process (self-HubConnection ban honored). The driver-node feed is a periodic 5 s full-snapshot publisher (`DriverResilienceStatusPublisherService`) with a wiring guard. Live gate (T10) — docker-dev breaker-open on both centrals + staleness — pending. *(Original finding below.)* The remediation merges made per-driver resilience live in production: #10 (`bacea1a4`) wired `CapabilityInvoker` into every `DriverInstanceActor` dispatch site, and #13 (`75403caa`) applies the AdminUI-authored `ResilienceConfig` overrides from the deploy artifact. But `DriverResilienceStatusTracker` (`src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceStatusTracker.cs`) still has **no reader** — no AdminUI component, hub, or endpoint consumes it (`grep -rn Resilience src/Server/ZB.MOM.WW.OtOpcUa.AdminUI` matches only the authoring form + driver pages). The planned `/hosts` Stream E.2/E.3 status panels never shipped; the interim observability surface is retry/breaker **log lines** added during #10. `archreview/plans/STATUS.md` explicitly records this as "still open as an Admin-UI follow-on." diff --git a/archreview/plans/FOLLOWUP-10-resilience-dispatch-gap.md b/archreview/plans/FOLLOWUP-10-resilience-dispatch-gap.md index febcac70..998438c9 100644 --- a/archreview/plans/FOLLOWUP-10-resilience-dispatch-gap.md +++ b/archreview/plans/FOLLOWUP-10-resilience-dispatch-gap.md @@ -39,11 +39,19 @@ the concrete invoker in would reverse that architectural decision. ResilienceConfig change. Per-instance overrides now reach the invoker. See [`FOLLOWUP-13`](FOLLOWUP-13-resilience-config-artifact.md). -**Observability finding + fix.** The resilience-status tracker has **no reader** (Admin `/hosts`, Phase 6.1 Stream +**Observability finding + fix.** The resilience-status tracker had **no reader** (Admin `/hosts`, Phase 6.1 Stream E.2/E.3, was never built) and the pipeline builder logged nothing — so the pipeline ran **invisibly**. Added retry / breaker-open / breaker-close **structured logging** to `DriverResiliencePipelineBuilder` (optional ILogger, wired in the Host DI). This is the operator-facing observability surface AND the prerequisite for any live-verify. +> **UPDATE (R2-10, branch `r2/10-resilience-observability`):** the tracker now HAS a proper reader. A periodic +> 5 s DPS publisher (`DriverResilienceStatusPublisherService`, driver nodes) mirrors each snapshot → +> `driver-resilience-status` topic → per-admin `DriverResilienceStatusBridge` → `IDriverResilienceStatusStore` → +> breaker/retry/in-flight/staleness chips on `DriverStatusPanel.razor` (mirrors the driver-health flow). Tracker +> truthfulness fixed too: `IsBreakerOpen`/`RecordBreakerClose` + `RecordSuccess` on the invoker success paths. The +> log-line remains the drill-down; the panel is the primary surface. **The live behavioral gate below (observing the +> `circuit-breaker OPENED` line on a dead endpoint) is now folded into R2-10's T10 live-`/run` — still pending.** + **Verification done (deterministic + local):** - Negative control: unwrapping any site fails the Runtime build with OTOPCUA0001. - Recording-invoker Runtime test: write routes via `ExecuteWriteAsync` with the `IPerCallHostResolver` host; diff --git a/archreview/plans/R2-10-resilience-observability-plan.md b/archreview/plans/R2-10-resilience-observability-plan.md index 9d552d58..a24c8daa 100644 --- a/archreview/plans/R2-10-resilience-observability-plan.md +++ b/archreview/plans/R2-10-resilience-observability-plan.md @@ -191,3 +191,14 @@ Subscription lifecycle copies the existing pattern exactly: inject the store, su 1. Mark U10/U-5 remediated with the branch/commit; update STATUS.md; annotate FOLLOWUP-10 that the reader shipped and (if observed in T10) the live behavioral log-line gate closed. Commit: `docs(archreview): R2-10 resilience observability shipped`. **Full-suite gate before merge:** `dotnet test ZB.MOM.WW.OtOpcUa.slnx` green (integration suites need their fixtures per CLAUDE.md; the new tests themselves are fixture-free). + +--- + +## Execution deviations (R2-10) + +Executed on branch `r2/10-resilience-observability` (off master `46fedda3`). T1–T9 + T11 completed; T10 deferred-live. + +- **T2 breaker-close test uses a controllable `TimeProvider` (`ManualTimeProvider`), not wall-clock waits.** Driving the real Polly breaker to `OnClosed` requires elapsing the 15 s `BreakDuration` then a successful half-open probe. Rather than a 15 s `Task.Delay`, the test injects a `TimeProvider` that advances both `GetUtcNow` and `GetTimestamp` off one tick counter (`Advance(16 s)`), making the close deterministic and instant. Same self-contained-fake style the repo already uses (`TriePermissionEvaluatorTests`); no new package. This is the one deviation from the plan's "extend the existing breaker test" wording — the existing test only reaches breaker-open, and open→close needs time control. +- **T9 panel renders healthy hosts quietly** rather than one always-visible row per host. Per the plan's "healthy is quiet" intent, the resilience section shows: `No resilience activity recorded yet.` when the store has no entries for the instance; `All targets healthy.` when entries exist but none has an active signal; and a chip row **only** for hosts with breaker-OPEN / consecutive-failures>0 / in-flight>0 / stale. Keeps the panel silent under normal operation (the tracker populates an entry for every host that has ever had a successful call, so an always-on per-host row would be noisy). +- **T10 (live-`/run` gate) deferred-live.** Code-complete + offline-verified (unit + full-solution build). The docker-dev breaker-open observation on both centrals + recovery-clear + staleness-dim is a heavy serial pass left for the consolidated live session (also the first live sighting of the #10 `circuit-breaker OPENED` log line, closing FOLLOWUP-10's behavioral gate). Marked `deferred-live` in `.tasks.json`. +- **T6/T8 registration:** the publisher's `Func` is registered via `TryAddSingleton` (idempotent with the one `Program.cs` registers for the alarm-command router, per plan). The resilience bridge intentionally has **no SignalR hub** (the panel reads the store in-process; resilience has no browser-JS consumer) — a deliberate simplification the plan calls for. diff --git a/archreview/plans/R2-10-resilience-observability-plan.md.tasks.json b/archreview/plans/R2-10-resilience-observability-plan.md.tasks.json index 4f00eb90..a8a1e80f 100644 --- a/archreview/plans/R2-10-resilience-observability-plan.md.tasks.json +++ b/archreview/plans/R2-10-resilience-observability-plan.md.tasks.json @@ -11,7 +11,7 @@ { "id": "T7", "subject": "AdminUI: IDriverResilienceStatusStore + InMemory impl + AddOtOpcUaDriverStatusServices registration (TDD mirroring DriverStatusSnapshotStoreTests)", "status": "completed", "blockedBy": ["T4"] }, { "id": "T8", "subject": "AdminUI: DriverResilienceStatusBridge DPS actor + spawn in WithOtOpcUaSignalRBridges", "status": "completed", "blockedBy": ["T7"] }, { "id": "T9", "subject": "AdminUI: resilience chip section in DriverStatusPanel.razor (in-process store read; no bUnit — verified by T10)", "status": "completed", "blockedBy": ["T7"] }, - { "id": "T10", "subject": "Live-/run gate on docker-dev: rebuild BOTH centrals, S7 dead-endpoint breaker-open, both-replica chips, recovery clear, staleness dim", "status": "pending", "blockedBy": ["T6", "T8", "T9"] }, - { "id": "T11", "subject": "Docs + review ledger: mark 03/U10 + 04/U-5 remediated; update STATUS.md + FOLLOWUP-10", "status": "pending", "blockedBy": ["T10"] } + { "id": "T10", "subject": "Live-/run gate on docker-dev: rebuild BOTH centrals, S7 dead-endpoint breaker-open, both-replica chips, recovery clear, staleness dim", "status": "deferred-live", "blockedBy": ["T6", "T8", "T9"] }, + { "id": "T11", "subject": "Docs + review ledger: mark 03/U10 + 04/U-5 remediated; update STATUS.md + FOLLOWUP-10", "status": "completed", "blockedBy": ["T10"] } ] } diff --git a/archreview/plans/STATUS.md b/archreview/plans/STATUS.md index 59eaad4e..b97ac4d5 100644 --- a/archreview/plans/STATUS.md +++ b/archreview/plans/STATUS.md @@ -174,6 +174,7 @@ what the unit leg cannot; (c) migrate the 3 xunit v2 holdouts once Akka ships an | **R2-01** | 05/STAB-14 (High regression, rank 1), 05/STAB-15 (rank 3), 05/STAB-8 seam | `r2/01-s7-fault-hardening` (off `1676c8f4`) | `fcdcf0e2`…`bdb7ed41` (13 task commits) | **Code-complete, offline-verified.** All prod changes in `S7Driver.cs`. STAB-14: connect-timeout surfaces as `TimeoutException` (not an escaping OCE) at the `EnsureConnectedAsync`/`InitializeAsync` source + `when(token.IsCancellationRequested)` filters on the read/write ensure-wrappers and all three poll-loop OCE catches — subscription poll loops survive an unreachable-host outage and recover (T2 regression RED→GREEN). STAB-15: `IsS7ConnectionFatal` broadened to the ISO-on-TCP framing surface (TPKT/TPDU/WrongNumberOfBytes/`PlcException{WrongNumberReceivedBytes}`, NOT `InvalidDataException`); mid-PDU cancel marks handle dead; probe classifies+marks-dead under the gate. STAB-8 (S7 part): `EnsureConnectedAsync` documented as the R2-09 connect-throttle seam (note-only). Driver.S7.Tests 246/246. **Live outage gate deferred (T11)** — `S7_1500ConnectTimeoutOutageTests` (env `S7_TIMEOUT_OUTAGE_START_CMD`/`STOP_CMD`, SYN-blackhole) skips clean offline; the connect-*timeout* class the `docker restart` bounce can't produce. (Pre-existing unrelated CLI docs-scan test fails at base — out of scope.) | | **R2-03** | 02/S13 (rank 5), 02/S12, 02/P7 | `r2/03-vt-failure-quality` (off `1676c8f4`) | `b9775485`…`6bb91e8d` (12 task commits) | **Code-complete, offline-verified.** S13: VT script failure/timeout degrades the node to `OpcUaQuality.Bad` (carrying last-known value) instead of freezing on last-Good forever — additive `EvaluationResult.Quality`, bridge historizes `0x80020000u` BadInternalError, published once per Good→Bad transition gated on all deps arrived (inputs-ready, no cold-start flash), recovery force-publishes next Good. S12: apply-boundary `ClearCompiledScripts` racing an in-flight eval into ObjectDisposedException retried once in `RoslynVirtualTagEvaluator` (re-fetch→recompile into current generation); stress guard reproduces pre-fix, deterministically green post-fix. P7: apply-boundary clear gated on a changed deployed expression set (`HashSet.SetEquals`, ordinal) — identical redeploy / rename-with-same-expressions no longer forces full VT recompile. Runtime.Tests 372/372, RoslynVirtualTagEvaluatorTests 20/20. **Deferred (T11/T12):** full cross-suite sweep + docker-dev live-`/run` (fail→Bad→recover + P7 no-recompile spot-check). | | **R2-02** | 01/S-6 (High, rank 2), 01/U-6, 01/S-8=03/S12 (rank 4), 04/C-7, 01/S-7=03/S13 | `r2/02-resilience-config-hardening` (off `1676c8f4`) | `4be13429`…`dd82e740` (17 task commits) | **Code-complete, offline-verified.** **S-6:** parser clamps timeout/retry/breaker overrides (timeout≤0→tier default, breaker==1→2, caps) with diagnostics + belt-and-braces `Build` clamp so pipeline construction can never throw Polly `ValidationException` (new `ResiliencePolicyRanges` in Core.Abstractions); brick-repro + `Build_NeverThrows_ForHostileDirectOptions` + factory-seam wiring proof; AdminUI `Validate()` range warnings. **S-8:** parser forces Write/AlarmAck retry→0 (spec invariant) **and** dispatch flips to `isIdempotent:false` (RED-first on the Runtime wiring test); non-idempotent arm caches the no-retry snapshot once/invoker (01/P-4) + tracker in-flight accounting; `WriteIdempotentAttribute` false claim corrected; retry cells disabled. **U-6:** bulkhead knob deleted end-to-end (migration-free; stored keys ignored) + `Options_properties_are_exactly_the_pipeline_wired_set` inertness guard. **C-7:** `ResilienceFormModel` JsonObject-bag round-trip preserves unknown keys; malformed JSON → `ParseFailed` lock + verbatim `ToJson` + discard button. **S-7:** per-`Create` `Interlocked` options-generation in the pipeline-cache key kills the respawn stale-pipeline race. Core resilience 121/121, AdminUI form 10/10, Analyzers 32/32, Runtime 364/364; 0 production OTOPCUA0001. **Deferred (T15/T18):** docker-dev `:9200` `/run` (warning banner, input lockout, unknown-key survival) + whole-solution sweep. | +| **R2-10** | 03/U10=04/U-5 (Medium/Low, action #10) | `r2/10-resilience-observability` (off `46fedda3`) | `205d3980`…`` (11 task commits) | **Code-complete, offline-verified.** The resilience status tracker now has a production reader, mirroring the driver-health flow. Core truthfulness: `RecordBreakerClose` + derived `IsBreakerOpen` (breaker-open state derivable), `OnClosed` records the close, and `RecordSuccess` wired onto the invoker success paths (`ConsecutiveFailures` is a true consecutive counter, not stuck-forever). Transport: `DriverResilienceStatusChanged` Commons contract → periodic 5 s full-snapshot publisher `DriverResilienceStatusPublisherService` (driver nodes, registered in `AddOtOpcUaDriverFactories` with wiring guard `ResilienceStatusReaderRegistrationTests` — closes the built-but-never-wired class) → `driver-resilience-status` DPS topic → per-admin-node `DriverResilienceStatusBridge` → `IDriverResilienceStatusStore` → resilience chips (breaker-OPEN / consecutive-failures / in-flight / staleness) on `DriverStatusPanel.razor`, read in-process (self-HubConnection ban honored). Core.Tests resilience 31/31 (incl. deterministic breaker-close via a controllable TimeProvider), Host.IntegrationTests publisher+guard 6/6, AdminUI.Tests store 5/5; full-solution build clean. **Deferred (T10):** docker-dev live-`/run` — S7 dead-endpoint breaker-open observed on BOTH centrals + recovery-clear + staleness-dim (also the first live sighting of the #10 `circuit-breaker OPENED` log line, closing FOLLOWUP-10's behavioral gate). | **Report anchor drifts to fold into the next report refresh** (from the R2-02 verification pass): (1) `01-core-composition.md` U-6 anchor `DriverResilienceOptions.cs:23-25` → the two bulkhead properties sat at