diff --git a/archreview/plans/00-INDEX.md b/archreview/plans/00-INDEX.md index adbe2cb3..5fa01302 100644 --- a/archreview/plans/00-INDEX.md +++ b/archreview/plans/00-INDEX.md @@ -21,7 +21,7 @@ co-located `.tasks.json`. | # | Plan | Findings | Tasks | Plan author | Executor | Reviewer | |---|---|---|---|---|---|---| | 1 | [R3-01 Mesh gRPC `.AllowAnonymous()`](R3-01-mesh-grpc-allowanonymous-plan.md) | 03/S14 **Critical** | 10 | opus | sonnet (+ fable for the `DisableLogin=false` rig gate) | opus | -| 2 | [R3-02 Stale-Good family](R3-02-stale-good-family-plan.md) | 03/S15, 05/STAB-19, 05/STAB-18, 02/S14 | TBD | **fable** | opus | opus | +| 2 | [R3-02 Stale-Good family](R3-02-stale-good-family-plan.md) | 03/S15, 05/STAB-19, 05/STAB-18, 02/S14 | 19 | **fable** | opus | opus | | 3 | [R3-03 Restart re-subscribe + watchdog `max()`](R3-03-restart-resubscribe-watchdog-plan.md) | 03/S16, S17, U6 | 9 | opus | opus (upgraded from sonnet: the plan redesigns the spawn path + adds a supervision-restart hook) | opus | | 4 | [R3-04 One role authority](R3-04-one-role-authority-plan.md) | 01/S-12 = 03/C5, 01/C-9 | 13 | opus | sonnet | opus | | 5 | [R3-05 ACL hot path + key expiry](R3-05-acl-hot-path-expiry-plan.md) | 03/S18, S19 | 12 | opus | opus | opus | diff --git a/archreview/plans/R3-02-stale-good-family-plan.md b/archreview/plans/R3-02-stale-good-family-plan.md new file mode 100644 index 00000000..686d5020 --- /dev/null +++ b/archreview/plans/R3-02-stale-good-family-plan.md @@ -0,0 +1,582 @@ +# Design + Implementation Plan — R3-02 The Stale-Good Family + +- **Source reports:** `archreview/03-server-runtime.md` §S15 · `archreview/05-protocol-drivers.md` §STAB-19 + §STAB-18 (and §Recommendations item 1) · `archreview/02-scripting-alarms.md` §S14 (and §Recommendations item 2) · `archreview/00-OVERALL.md` theme 1 ("The stale-Good family") — 2026-08-19 round-3 re-review at `12b0035a` +- **Prior plan consumed:** `plans/R2-03-vt-failure-quality-plan.md` (02/S13 — the first member of the family, closed in round 2; its transition-once / force-republish-on-recovery / inputs-ready-gate decisions are **inherited, not re-litigated** here) +- **Plan verified against tree at:** master `7c77a8c7` (code byte-identical to `12b0035a`; the merge added only the archreview docs) — every anchor re-opened and re-checked 2026-08-20 +- **Scope:** OVERALL prioritized action item **#2** — "Stale-Good family: comms-loss fan-out writes `BadNoCommunication` to raw + UNS value variables with force-republish on recovery; Calculation threads the real `statusCode`; MQTT re-subscribe initial push (or suppression reset); VT path honours input quality. One contract test per seam" (03 S15 High, 05 STAB-19 High, 05 STAB-18 High, 02 S14 Medium). +- **Cross-cutting requirement (OVERALL theme 1):** state the contract **once** and pin it with one test per seam that *holds a Bad input and asserts on the derived node*. The contract, as this plan states it, is in §"The quality contract" below and is quoted verbatim into the one doc section every seam links to. +- **Sibling-plan boundary (R3-03, 03/S16 + 03/S17):** R3-03 edits `DriverHostActor.HandleRestartDriver` (`:2916-2932`) and the watchdog's `??` at `DriverInstanceActor.cs:574`. **This plan does not touch either method.** Every S15 host edit lives in new, separately-named methods (`FanCommsLossToValueNodes`, `TrackCommsLatch*`) plus one new `Receive` registration, and the only `DriverInstanceActor` edit is an additive `PostStop` Tell. **Land R3-03 first** (it is smaller and its S16 re-subscribe is what makes this plan's recovery contract hold for an operator-restarted driver); rebase this branch over it — the textual overlap is zero by construction, so the rebase is a no-op. + +## Verification summary + +All four findings **CONFIRMED at `7c77a8c7`** — none stale, none partially fixed. One finding is *under*-stated (S15: Modbus goes Bad per tag on its own, so the stale-Good window on Modbus is shorter than "the whole outage" — but the S15 mechanism is still absent and every non-per-tag failure shape still holds Good indefinitely; see Finding 1). Anchor drift status: + +| Finding | Report anchor | Status at `7c77a8c7` | +|---|---|---| +| 03/S15 | `DriverHostActor.cs:1248-1283` (`OnDriverConnectivityChanged`), `:1334-1359` (`FanCommsLossToDependencyMux`), `:1240-1245` (rationale), `:1310-1318` (forceRaise recovery remark) | **Exact.** The handler writes `AlarmQualityUpdate` per native condition (`:1267-1277`) and one `AttributeValueBatchPublished` of `BadNoCommunication`/null to the mux (`:1346-1353`). **Nothing targets `_opcUaPublishActor` with a value/status write.** The per-driver index it needs already exists: `_valueRefsByDriver` (`:305`, `DriverInstanceId → [(RawPath, HashSet)]`, node set held by reference so it covers the raw node AND every UNS mirror). The host has **no value cache** (single-source fan-out by design, `:253-258`), so "keep the last value" can only be done at the node manager. | +| 03/S15 publish path | `ForwardBatchToMux :1166-1200` → `OpcUaPublishActor.AttributeValueBatchUpdate` (`OpcUaPublishActor.cs:57`, handled `:348-368`) → `IOpcUaAddressSpaceSink.WriteValues` (`IOpcUaAddressSpaceSink.cs:35-44`; `SdkAddressSpaceSink.cs:31-32`; `DeferredAddressSpaceSink.cs:33`) → `OtOpcUaNodeManager.WriteValues :577-640` → `WriteValueLocked :663-680` | **Traced; the node manager does NOT dedup** — `WriteValueLocked` sets `Value`/`StatusCode`/`Timestamp` unconditionally and calls `ClearChangeMasks` (`:672-677`). The only dedup on the value path is **driver-side**: `PollGroupEngine.PollOnceAsync` raises only on `forceRaise \|\| value differs \|\| status differs` (`PollGroupEngine.cs:404`), with `forceRaise: true` on the initial poll of every new subscription (`:259`) and `LastValues` per `SubscriptionState` (`:467`). `ModbusDriver` adds a deadband cache `_lastPublishedByRef` (`:55`, `ShouldPublish :244-259`) that **is** cleared in `TeardownAsync` (`:1810`). SDK-level: the UA stack's `DataChangeMonitoredItem` (default trigger `StatusValue`) does not queue a notification when value AND status are unchanged — a timestamp-only rewrite is invisible to subscribers, which is why a Bad write must actually change the node's status for the later Good to be visible (it does). | +| 03/S15 watchdog | `DriverInstanceActor.cs:567-584` (`EvaluateCommsLossWatchdog`), `:463-470` (Connected `DisconnectObserved` handler), `:633-658` (Reconnecting) | **Exact.** Trip ⇒ `DetachSubscription()` (`:467`, nulls the handle + unhooks handlers, **does not unsubscribe the driver**) → `Become(Reconnecting)` → `Reconnecting()` Tells `ConnectivityChanged(false)` (`:637`) → `RetryConnect` runs `InitializeAsync(teardownFirst: true)` (`:640`) which calls `driver.ShutdownAsync` first (`:701-725`) → on `InitializeSucceeded`: `Become(Connected)` (Tells `ConnectivityChanged(true)`, `:456`) then `ResubscribeDesired()` (`:657` → `:1171-1177` self-Tell `Subscribe`). For every `PollGroupEngine` driver, `ShutdownAsync` disposes the engine's subscriptions (`PollGroupEngine.DisposeAsync :428-457`, clears `_subscriptions`; the engine is reusable afterwards) so the re-subscribe creates a fresh `SubscriptionState` with empty `LastValues` ⇒ the initial poll runs `forceRaise: true` ⇒ **every tag re-publishes with its real quality even if unchanged.** This is the recovery mechanism S15 relies on, and it is sound for the seven poll drivers (`Modbus, FOCAS, TwinCAT, AbLegacy, S7, Sql, AbCip` construct a `PollGroupEngine`). It is exactly what MQTT lacks (STAB-18). | +| 03/S15 sink surface | — | **New fact:** `IOpcUaAddressSpaceSink` has **16 implementers** (3 in `src`: the interface's defaults, `DeferredAddressSpaceSink`, `SdkAddressSpaceSink`; 13 test fakes). A new abstract member costs 13 fake edits; a default-implemented member costs none, and the `DeferredSinkForwardingReflectionTests` guard (`tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/`) forces the deferred forwarder to be written. Design below uses a default-implemented member. | +| 05/STAB-19 | `CalculationDriver.cs:330-352` (`OnDependencyValue`), `DependencyConsumerMuxAdapter.cs:71-72`, `CalculationEvaluator.cs:181-187` (`BuildReadCache`), `DependencyMuxActor.cs:114, :150` | **Exact** for the first, second and fourth (`OnDependencyValue` is `:329-352`, `_arrivedDeps.Add` at `:336`, `statusCode` parameter never read; adapter comment `:71` "Quality is stripped at the mux boundary" + `statusCode: 0u` at `:72`; mux forwards `msg.StatusCode` at `:114`/`:150`). **Minor drift** on the third: `BuildReadCache` is `:176-188`, the `StatusCode: 0u` stamp at `:185`. Also verified: `_depValues` is `Dictionary` (`:85`) — no per-dep status storage exists; the passthrough fast path (`:88-93`) bypasses the read cache entirely; `MarkEvaluationSucceeded :549-559` already bypasses value-dedup while `LastPublishedBad` (the R2-03 recovery idiom), which this plan generalises. **Precedent in the same file:** `CalculationDriver.SubscribeAsync :356-371` performs an *initial-data callback* for each subscribed ref — the push-model `forceRaise` STAB-18 asks MQTT to adopt already exists in-tree. | +| 05/STAB-18 | `MqttSubscriptionManager.cs:325-365` (`SubscribeAsync`), `:762-767` (suppression), `SparkplugIngestor.cs:470-506` (`SubscribeAsync`) | **Exact** (suppression compare at `:762-766`, `Values.Update` then `if (!unchanged) Raise`). Verified the cache lifetime claim: `LastValueCache` is a **driver-owned readonly field** (`MqttDriver.cs:127`), handed to every manager/ingestor the driver builds (`CreateSubscriptionManager :1147-1161`, `cache: _values` at `:1154`; Sparkplug `:1193`), and `MqttDriver.TeardownAsync :1387-1411` never touches it — so it survives the watchdog's `ShutdownAsync` + `InitializeAsync` **and** every redeploy. Verified there is **no connection-lost hook**: `MqttConnection` exposes only `Reconnected` (`:350`) and `MessageReceived` (`:363`); `OnDisconnectedAsync :932-948` flips state to `Reconnecting` (`:941`) and wakes the supervisor — nothing stales values. `SparkplugIngestor.ResetSessionState` documents the opposite policy explicitly ("Values are deliberately **not** staled here … Only a death … stales a tag", `:583-588`) — this plan overrides that remark, with the reason recorded below. | +| 05/STAB-18 rebuild shape (b) | — | **Confirmed at the host:** every deploy re-sends `SetDesiredSubscriptions`, and the Connected handler (`DriverInstanceActor.cs:483-486`) always self-Tells `Subscribe` (unsubscribe-then-subscribe in `HandleSubscribeAsync`). A poll driver answers with a fresh initial push; `MqttSubscriptionManager.SubscribeAsync`/`SparkplugIngestor.SubscribeAsync` record handles and push nothing — a rebuilt node reseeded `BadWaitingForInitialData` (`OtOpcUaNodeManager.cs:2125/:2220`) stays there until the plant publishes. | +| 02/S14 | `VirtualTagActor.cs:33-38` (`DependencyValueChanged` doc: "the virtual-tag engine ignores it"), `:187-191` (`OnDependencyChanged`), `RoslynVirtualTagEvaluator.cs:165-177` (`BuildReadCache`) | **Exact** (`BuildReadCache` is `:165-178`, `StatusCode: 0u` at `:174`). `_dependencies` is `Dictionary` (`:75`); `OnDependencyBatch :200-217` has the same shape; `Evaluate :223-266` dedups on value only (`:254`); `OnEvaluationFailed :277-289` is the R2-03 transition-once tail gated on `AllDependenciesArrived` (`:295`); `OnReassertValue :179-185` re-emits `_lastPublishedBad ? BadInternalError : Good`. `VirtualTagHostActor.OnResult :214-242` bridges `result.StatusCode` **verbatim** to `AttributeValueUpdate` (`:227-228`) and historizes non-reassert results with that status (`:238-241`) — so a propagated sub-code needs no bridge change. `IVirtualTagEvaluator.Evaluate` takes `IReadOnlyDictionary` (`IVirtualTagEvaluator.cs:20`) and has **6 implementers** (Roslyn + 5 test fakes across 4 Runtime.Tests files). | +| Parity reference | `ScriptedAlarmEngine.cs:746-765` (worst-of-input computed BEFORE the readiness short-circuit; a Bad input holds the prior condition state and annotates quality out of band), `AreInputsReady :829-840` (null value OR Bad ⇒ not ready; Uncertain IS ready), `WorstInputStatus :923-941` (skips `StatusNeverPushed` = `BadWaitingForInitialData`) | **Exact.** This is the in-tree policy the VT and Calculation seams are aligned to. | + +Supporting facts verified: `OpcUaStatus` constants (`Commons/OpcUa/OpcUaStatus.cs`: `Good`, `Uncertain 0x40000000`, `Bad`, `BadWaitingForInitialData 0x80320000 :39`, `BadNoCommunication 0x80310000 :43`, `BadInternalError :47`, `Severity/IsGood/IsUncertain/IsBad :52-66`); `Commons` references `Core.Abstractions` (so `DataValueSnapshot` is usable from Commons interfaces); `Driver.Calculation` references `Core.Abstractions` + `Commons`; `Core.ScriptedAlarms` references `Core.Abstractions` only. Existing test scaffolding: `DriverHostActorCommsLossTests` (real `DependencyMuxActor`, `SeedV3Deployment` with raw tags + UNS refs, `PresenceBudget`), `DriverHostActorBatchFanOutTests` (publish-actor probe asserting `AttributeValueBatchUpdate` raw+UNS fan-out), `DriverInstanceActorCommsLossWatchdogTests` (injectable `commsLossTimeout`, stable-health stub), `CalculationDependencyFlowTests`, `VirtualTagActorTests`/`VirtualTagHostActorTests` (R2-03 transition tests), `MqttSubscriptionManagerTests`/`SparkplugIngestorTests`/`MqttConnectionTests` (+ `MiniBroker`), `LastValueCacheTests`. Modbus fixture (`tests/Drivers/…Modbus.IntegrationTests/Docker/profiles/standard.json`): **HR[0..31] = address-as-value (constant, config-seeded — survives a container restart), HR[100] = auto-increment** — the constant/changing pair the live gate needs. + +### Design choices resolved in this plan + +1. **One contract, stated once** (§below). Every per-seam section derives its behaviour from it rather than re-deciding. +2. **S15 keeps the last value** via a new status-only sink write (`WriteStatuses`) rather than `WriteValues` with `null` — the host holds no value, the node does; and an OPC UA client reading a Bad node with its last-known value is the Part 8 "last known value" convention SCADA clients expect. The null-through-`WriteValues` route is kept as the documented fallback if the sink change is judged too wide. +3. **S15 publishes nothing Good on reconnect.** Recovery rides the driver's own re-publish (the poll engine's `forceRaise`, or the push-model initial push STAB-18 adds). The host instead **audits** recovery: refs still latched Bad 30 s after `Connected` are logged + metered, which is the operator-visible signature of any future driver that forgets its initial push. +4. **STAB-18 lands BEFORE S15** (or in the same PR with MQTT first in the commit order). Today an MQTT ref latched Bad by the watchdog is stuck only at the mux; once S15 latches the *value nodes* too, an MQTT driver without STAB-18 would show every external client Bad until the plant value changes. Ordering is the mitigation. +5. **STAB-18 fixes both halves:** (A) an initial push of cached observed snapshots on every `SubscribeAsync` (rebuild shape), and (B) a `ConnectionLost` stale of every observed ref to `BadNoCommunication` (value kept) so the reconnect's identical retained/birth replay *differs in status* and is no longer suppressed — and so MQTT marks Bad on its own disconnect edge like every poll driver does per tag, not only after the 90 s watchdog. "Clear the suppression memory" alone was rejected: it recovers retained topics but leaves non-retained ones serving stale-Good from the cache — the exact contract violation. +6. **Calculation + VT quality policy: hold on Bad, propagate the worst input's status; evaluate through Uncertain and stamp the output with it.** Alternatives weighed in Finding 2; this is the alarm engine's existing policy (`AreInputsReady` + out-of-band worst-of-input quality) and the only one that is honest for a comms-lost input (an Uncertain output would claim the value is merely imprecise; it may be arbitrarily old). +7. **No new message-contract field on the mux path.** `DependencyValueChanged.StatusCode` already carries the sub-code since #478/#519; only the *consumers* change (adapter forwards it; VT/calc store it). `IVirtualTagEvaluator` gains a snapshot overload with a default implementation so the 5 fakes compile unchanged. +8. **Worst-of-input helper lives once**, in `Core.Abstractions` next to `DataValueSnapshot`, and the alarm engine's private copy is re-pointed at it (theme 4: "one rule stated twice diverges"). + +--- + +## The quality contract + +> **A value consumer never reports a quality better than its source's.** A node derived from a driver value (the raw node, every UNS mirror, a Calculation output, a virtual tag, a scripted condition's quality) carries, at worst, the worst status among its inputs: a Bad input makes the derived node Bad with that input's sub-code and the last-known value; an Uncertain input makes it Uncertain; a source that has stopped communicating is a Bad input (`BadNoCommunication`) for everything downstream of it, **including the external OPC UA data plane**. Recovery is a *publish*, never an assumption: the first post-recovery value must land even when it equals the value held before the outage, so every dedup on the path — driver caches, the node's own status, a consumer's last-published memory — must be keyed on (value, status), not value alone. + +Pinned per seam by a test that holds a Bad input and asserts on the **derived** node (never on the signal): + +| Seam | Derived node asserted | Test (new) | +|---|---|---| +| S15 host fan-out | raw value node + its UNS mirror (publish-actor probe) | `DriverHostActorCommsLossValueNodesTests` + recovery `DriverHostCommsLossRecoveryTests` | +| STAB-18 MQTT | the ref's raised snapshot after reconnect / resubscribe | `MqttSubscriptionManagerRecoveryTests`, `SparkplugIngestorRecoveryTests` | +| STAB-19 Calculation | the calc tag's `OnDataChange` snapshot | `CalculationDriverInputQualityTests` | +| 02/S14 VT | the VT's `EvaluationResult` (status + value) at the host bridge | `VirtualTagActorInputQualityTests` | + +Plus one **falsification control for the dedup trap** per recovering seam (see Finding 1 §Tests "vacuity"): the recovery assertion must be run with a source that re-publishes the **identical** value, and the plan names the variant that must FAIL. + +--- + +## Finding 1 — 03/S15: Driver comms loss never marks raw/UNS value variables Bad (High) + +### Restatement + +When a driver is declared comms-lost (watchdog trip, failed connect, operator Force Reconnect — anything that enters `Reconnecting`), `DriverHostActor.OnDriverConnectivityChanged` annotates its Part 9 conditions `BadNoCommunication` and fans a synthetic Bad batch to the dependency mux, so scripted alarms and virtual tags see the outage. The driver's raw value nodes and their UNS mirrors — the product's external surface — get nothing. A client subscribed to a value node sees the last value with a stale timestamp and **Good** quality for as long as the driver publishes nothing, i.e. the whole outage for any driver whose failure shape is "reader throws / no data" rather than "per-tag Bad snapshot". + +### Verification + +Confirmed (summary table). Two refinements the fix must respect: + +- **Modbus already degrades per tag, quickly.** `ModbusDriver.ReadAsync` turns a per-tag read exception into `DataValueSnapshot(null, BadCommunicationError 0x80050000, …)` (`:511`, `:517`; coalesced path `:995`), the poll engine raises it (status changed, `PollGroupEngine.cs:404`), and the host fans it to raw + UNS nodes within one poll interval. So on the docker-dev Modbus rig the value nodes go **Bad (0x80050000, value null) within seconds** of stopping the fixture — *before* the 90 s watchdog. What S15 adds for Modbus is the `BadNoCommunication` annotation on the watchdog edge (idempotent over an already-Bad node) and, critically, coverage for the shapes where the driver raises nothing: a reader that throws as a whole (`RequireTransport()` with a null transport, a coalesced-read throw — `PollGroupEngine` catches and calls `onError` without raising), S7 (whose reader "never throws for device faults" per 05/STAB-21 but can also stall), TwinCAT/OpcUaClient/Galaxy/MTConnect session drops that stop the push without a Bad, and any driver whose last act before going quiet was a Good publish. **No double-handling harm:** the S15 write is status-only over whatever value the node holds (null after Modbus's own Bad), and the Bad→Bad re-stamp is exactly the "unchanged code suppresses" shape `WriteAlarmQuality` already documents for conditions — here it is a second `ClearChangeMasks`, which the SDK's `StatusValue` trigger dedups away at the monitored item (status unchanged). +- **Ordering on the way down is safe.** The Connected-state `DisconnectObserved` handler calls `DetachSubscription()` *before* `Become(Reconnecting)` (`:467-469`), and `Reconnecting` has no `DataChangeForward` receive — so any driver value already in the mailbox is dropped as unhandled, and the `ConnectivityChanged(false)` the new behaviour Tells (`:637`) is the last value-affecting message the host sees for that driver until recovery. The S15 Bad write cannot be overtaken by a late Good. +- **Ordering on the way up relies on the driver.** `Connected()` Tells `ConnectivityChanged(true)` (`:456`) *before* `ResubscribeDesired()` (`:657`), so the host learns "connected" before any value arrives. Publishing Good there would assert a quality the host has not observed (the existing remark at `:1310-1318` says so for the mux; it holds for nodes). The first real value arrives from the fresh subscription's initial push. + +### Root cause + +#481 built the per-driver ref index and the comms-loss fan-out for *internal* consumers (mux) and *conditions* (Part 9), and stopped one seam short of the external data plane. The host has no value cache (by design — single source, no drift), so "mark Bad keeping the value" needs a node-manager primitive that did not exist; the absence of that primitive is why the value seam was skipped. + +### Proposed design + +**1. Sink primitive — status-only batch write (keep value).** + +```csharp +// IOpcUaAddressSpaceSink.cs (additive, default-implemented) +/// One status-only write within a batch: +/// the node's current VALUE is retained; only StatusCode + SourceTimestamp change. +public readonly record struct StatusWrite(string NodeId, uint StatusCode, DateTime SourceTimestampUtc, AddressSpaceRealm Realm); + +/// Batched status-only write — the comms-loss annotation for value variables (03/S15). A sink +/// that holds the node's current value MUST retain it (the SDK sink does). This DEFAULT cannot — it holds +/// no value — so it degrades to with a null value, which is the honest statement +/// for a value-less sink and keeps every fake compiling. Same per-item isolation contract as +/// ; an unknown/unmaterialised NodeId is a no-op, not a create (a status has +/// nothing to create a node from). +void WriteStatuses(IReadOnlyList statuses) +{ + ArgumentNullException.ThrowIfNull(statuses); + for (var i = 0; i < statuses.Count; i++) + { + var s = statuses[i]; + WriteValue(s.NodeId, null, s.StatusCode, s.SourceTimestampUtc, s.Realm); + } +} +``` + +- `OtOpcUaNodeManager.WriteStatuses` (new, beside `WriteValues :577`): same outside-the-lock pre-pass + 512-per-lock chunking; per item `if (_variables.TryGetValue(key, out var v)) { v.StatusCode = new StatusCode(s.StatusCode); v.Timestamp = s.SourceTimestampUtc; v.ClearChangeMasks(SystemContext, false); }` — **never `CreateVariable`**; a missing node is skipped (counted in a Debug tally, not a failure — the index can name a node the address space dropped on a surgical remove). Same `ValueWriteBatchException` aggregate on thrown items. +- `SdkAddressSpaceSink.WriteStatuses` → `_nodeManager.WriteStatuses`. `DeferredAddressSpaceSink.WriteStatuses` → `_inner.WriteStatuses` (the reflection guard turns red until this exists — run it first). + +**2. Publish-actor message.** `OpcUaPublishActor.AttributeStatusBatchUpdate(IReadOnlyList Items)` (beside `AttributeValueBatchUpdate :57`), handled by `HandleAttributeStatusBatchUpdate` — a copy of `HandleAttributeBatchUpdate :348-368` calling `_sink.WriteStatuses`, metered `OpcUaSinkWrite kind:"status"`, same partial-failure Warning. + +**3. Host fan-out — new method, called from the existing trigger.** + +```csharp +// DriverHostActor.cs — NEW method (R3-03 boundary: HandleRestartDriver / the watchdog are untouched) +/// 03/S15 — the value-node half of the comms-loss fan-out. Stamps BadNoCommunication (value +/// retained) on every raw NodeId AND every referencing UNS NodeId the disconnected driver owns, as ONE +/// status batch through the same publish actor the value fan-out uses. Nothing is written on reconnect: +/// recovery is the driver's own re-publish (poll forceRaise / push initial push); see TrackCommsLatch. +private void FanCommsLossToValueNodes(string driverInstanceId) +{ + if (_opcUaPublishActor is null) return; + if (!_valueRefsByDriver.TryGetValue(driverInstanceId, out var entries) || entries.Count == 0) return; + var ts = DateTime.UtcNow; + var writes = new List(entries.Count); + foreach (var (_, nodes) in entries) + foreach (var n in nodes) + writes.Add(new StatusWrite(n.NodeId, OpcUaStatus.BadNoCommunication, ts, n.Realm)); + _opcUaPublishActor.Tell(new OpcUaPublishActor.AttributeStatusBatchUpdate(writes)); + TrackCommsLatchSet(driverInstanceId, entries); // recovery audit, step 4 + _log.Info("DriverHost {Node}: driver {Driver} disconnected — {Count} value node(s) marked BadNoCommunication", …); +} +``` + +Wired by **one added line** in `OnDriverConnectivityChanged` next to the existing mux call (`:1253`): `if (!msg.Connected) { FanCommsLossToDependencyMux(id); FanCommsLossToValueNodes(id); } else TrackCommsLatchArmAudit(id);`. Ungated by redundancy role (same reason as the condition annotation: a Secondary keeps its address space honest for failover). **Every entry into `Reconnecting` reaches it** — watchdog, failed initial connect, Force Reconnect — because all of them run `Reconnecting()`'s `ConnectivityChanged(false)` Tell. (The one path that does NOT is a child being *stopped*: `PostStop` detaches silently. Task T11 makes `DriverInstanceActor.PostStop` Tell `ConnectivityChanged(false)` to its parent so an operator Restart (S16) and a supervisor respawn latch Bad too; after R3-03's re-subscribe the fresh child's initial push recovers them. On a deploy that *removes* the driver the index has already been rebuilt without it, so the Tell is a no-op.) + +**4. Recovery audit (the dedup-trap tripwire, not a Good write).** `_commsLatched: Dictionary>` (driver → RawPaths latched). Set in step 3; each item routed by `ForwardBatchToMux` removes its `FullReference` from the owning driver's set (one `TryGetValue` + `Remove` on the hot path — only while the set is non-empty, so steady state costs one dictionary miss per burst, keyed by driver not by item). On `ConnectivityChanged(true)` a single timer `comms-audit:{driver}` fires `CommsRecoveryAudit(driver)` after `CommsRecoveryAuditDelay` (30 s, ctor-injectable for tests): if the set is still non-empty ⇒ `_log.Warning("… {Count} ref(s) still BadNoCommunication {Delay} after reconnect — the driver did not re-publish them (missing initial push / dedup) …")` + meter `otopcua.driver.comms_latched_refs` (tag `driver_type`). A second disconnect before the audit re-arms it. This is what turns a future STAB-18-class regression into a log line instead of a silent stale-Bad. + +**Alternatives considered:** +- *Write `null` + `BadNoCommunication` through the existing `WriteValues`* — zero sink change, and it is what Modbus itself publishes per tag. Rejected as the primary design because the action item asks to keep the last value and because a Part 8 last-known-value is more useful to a SCADA consumer than a null; **retained as the fallback** if `WriteStatuses` is judged too wide during review — the host-side change is then a two-line difference. +- *Publish `Good` with the node's current value on reconnect* — rejected (asserts an unobserved quality; the existing remark at `:1310-1318` already rules it out for the mux). +- *Host-side last-value cache to re-publish on reconnect* — rejected: it recreates the independent buffer the v3 single-source design removed, and it would re-publish a value the device may no longer hold. +- *Status-only write keyed by the driver's `DriverState` rather than `ConnectivityChanged`* — rejected: `Degraded` means connected-with-some-bad-data (#527); the Reconnecting edge is the only honest "no communication". + +### Implementation steps + +1. `src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs` — `StatusWrite` + default `WriteStatuses`. `DeferredAddressSpaceSink.cs` — forward. +2. `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs` — `WriteStatuses` (+ `WriteStatusLocked`); `SdkAddressSpaceSink.cs` — forward. +3. `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs` — `AttributeStatusBatchUpdate` + handler. +4. `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs` — `FanCommsLossToValueNodes`, `_commsLatched` + `TrackCommsLatch*`, `CommsRecoveryAudit` message + `Receive`, one-line call in `OnDriverConnectivityChanged`; `Commons/Observability/OtOpcUaTelemetry.cs` — `DriverCommsLatchedRefs` counter. +5. `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs` — `PostStop`: `Context.Parent.Tell(new ConnectivityChanged(_driverInstanceId, Connected: false))` before the existing detach (T11; additive, not in the watchdog method). +6. Update the xmldoc at `DriverHostActor.cs:1223-1247` (the handler summary now covers values) and the remark at `:1310-1318` (recovery contract now names the audit + MQTT's initial push). + +### Tests + +- `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OtOpcUaNodeManagerWriteStatusesTests.cs` (new): materialise + `WriteValue(42, Good)`; `WriteStatuses(BadNoCommunication)` ⇒ node reads `Value == 42`, `StatusCode == 0x80310000`, timestamp updated; unknown NodeId ⇒ no node created, no throw; a throwing item does not drop the tail (`ValueWriteBatchException` split). +- `tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests` — goes RED on the new member until forwarded (the guard is the test; no new file). +- `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorStatusBatchTests.cs` (new): `AttributeStatusBatchUpdate` reaches `sink.WriteStatuses`; partial failure logs Warning and keeps the tally. +- `tests/Server/…Runtime.Tests/Drivers/DriverHostActorCommsLossValueNodesTests.cs` (new, publish-actor probe, `SeedV3Deployment` with 2 raw tags + 1 UNS ref like `DriverHostActorBatchFanOutTests`): `ConnectivityChanged("drv-1", false)` ⇒ ONE `AttributeStatusBatchUpdate` whose items are exactly {raw speed (Raw), raw temp (Raw), UNS mirror of speed (Uns)} with `BadNoCommunication`; **control:** a second driver's refs are absent; `ConnectivityChanged(true)` ⇒ no value/status message at all (`ExpectNoMsg`). +- `tests/Server/…Runtime.Tests/Drivers/DriverHostCommsLossRecoveryTests.cs` (new — the **contract + vacuity test**): real `DriverInstanceActor` under the host with a scripted `ISubscribable`+`IDriver` fake (`commsLossTimeout` = 200 ms, `HealthPollInterval` short, as `DriverInstanceActorCommsLossWatchdogTests` does): fake publishes `speed = 5 Good`, then reports `Degraded` with frozen `LastSuccessfulRead` ⇒ host emits the status batch (raw + UNS); fake's `InitializeAsync` then succeeds and its **`SubscribeAsync` performs an initial push of the IDENTICAL value `5 Good`** ⇒ assert an `AttributeValueBatchUpdate` carrying `5 / Good` for raw + UNS arrives **after** the status batch; assert no `comms_latched_refs` increment and no audit Warning. + - ⚠️ **Vacuity rule:** a recovery test whose fake pushes a *different* value (or whose assertion is only "some Good arrives") passes against a build with no force-republish anywhere and is **not** the contract test — call it out in the file header and do not ship it as the guard. The identical-value push is load-bearing. + - **Falsification control (same file, `[Fact]` named `…_control_silent_resubscribe_trips_the_audit`):** the same scenario with a fake whose `SubscribeAsync` pushes nothing (the pre-STAB-18 MQTT shape) ⇒ no Good batch arrives, the audit timer (injected 300 ms) fires, `otopcua.driver.comms_latched_refs` increments by **1** (the latch is keyed per RawPath — raw node + UNS mirror share one ref; the Warning reports refs, not nodes) and the Warning names the driver. This proves the contract test *can* fail and that the audit catches what it misses. +- `DriverInstanceActorTests` addition: stopping a Connected child Tells `ConnectivityChanged(false)` to its parent (T11). +- Existing: `DriverHostActorCommsLossTests` (mux half) and `DriverHostActorBatchFanOutTests` unchanged and green. + +### Effort + Risk + +**Effort:** M (sink primitive S, host S, recovery test M). **Risk:** medium — a new sink member (guarded by reflection), a new hot-path dictionary probe in `ForwardBatchToMux` (one lookup per burst while a latch exists; zero-cost when `_commsLatched` is empty — pin with a microbench-free assertion that the set is removed when it empties), and a client-visible behaviour change: **value nodes of a comms-lost driver now go Bad**, which is the point, but any consumer (ScadaBridge) that treated a Bad value as "drop the tag" will see more Bads. Called out in the docs task. + +--- + +## Finding 2 — 05/STAB-19: Calculation launders dependency quality to Good (High) + +### Restatement + +`DependencyConsumerMuxAdapter.OnDependencyValueChanged` forwards `statusCode: 0u` behind a comment that is false (the mux has carried the real status since #478/#519), and `CalculationDriver.OnDependencyValue` never reads its `statusCode` anyway. A comms-lost input arrives as *arrived, Good, null*: the warm-up gate is satisfied by a value that never arrived, `(double)ctx.GetTag("x").Value` NREs per trigger (a "script threw" storm), and a null-tolerant script publishes a **Good** output computed from a dead input — a slow input that dies leaves the calc output stale-Good indefinitely. `CalculationEvaluator.BuildReadCache` additionally stamps every dep Good with eval-time timestamps, so even a script that checks `.StatusCode` is lied to. + +### Verification + +Confirmed exact (table). Additional: the `_arrivedDeps` gate (`:442`) is a "has ever been pushed" set with no quality dimension; `MarkEvaluationFailed :569-577` publishes `BadInternalError` once per transition with the last value (R2-03 parity already in place) — so the *output* side already has the transition-once + force-republish-on-recovery idiom; only the *input* side is missing. `CalculationDriver.GetHealth` is Healthy while running (the watchdog never trips it; correct) — so the calc driver's own quality must come entirely from its inputs. Passthrough scripts (`:88-93`) bypass the read cache, so a passthrough of a Bad input currently returns the Bad input's **null value as Ok(null) Good** — the worst case of the laundering. + +### Root cause + +The adapter was written when the mux *did* strip quality; the contract moved under it (#478/#519) and the comment outlived the fact (theme 6). The calc driver then inherited the mux-strips-quality assumption in its storage shape (`Dictionary`). + +### Proposed design + +**Policy (chosen): hold on Bad, propagate worst status; evaluate through Uncertain, stamp the output with the worst input status.** Concretely, per calc tag with definition deps `D`: + +- Store `DataValueSnapshot` per dep (`_depValues : Dictionary`); `_arrivedDeps` stays (arrival ≠ usability). +- `worst = InputQuality.WorstOf(snapshots of D)` (shared helper, see below), where a never-pushed dep is `BadWaitingForInitialData`. +- **Readiness** = all of `D` arrived **and** `!IsBad(worst)` (alarm-engine parity: Uncertain is usable, Bad is not). +- On any dependency push: if not all arrived ⇒ do nothing (today's warm-up; the node stays at the materialiser's `BadWaitingForInitialData`). If all arrived and `IsBad(worst)` ⇒ **do not evaluate**; publish `(LastValue-or-null, worst)` **once per change of `worst`** (`TagState.LastPublishedStatus` replaces `LastPublishedBad`; a Bad→different-Bad change re-publishes so `BadNoCommunication` is not masked by an earlier `BadInternalError`, and vice versa). If usable ⇒ evaluate; on success publish `(value, worst)` (Good or the Uncertain sub-code), **bypassing value-dedup whenever `LastPublishedStatus != worst`** (generalises the existing `LastPublishedBad` bypass at `:551`) — this is the force-republish that makes an identical-value recovery land; on script failure keep `MarkEvaluationFailed` (`BadInternalError`, last value) unchanged. +- `ReadOne` (`:390-399`) returns `LastPublishedStatus` instead of the two-state mapping, so `IReadable` and the `SubscribeAsync` initial-data callback agree with what was published. +- `CalculationEvaluator.Evaluate` gains an overload taking `IReadOnlyDictionary`; `BuildReadCache` passes the snapshots **through** (real status, real source timestamps), and the passthrough fast path returns `Ok(snapshot.Value)` — its quality is then stamped by the caller as `worst`, which for a single-dep passthrough IS the input's status. The old `object?` overload stays as a thin adapter for the existing tests. + +**Why hold rather than evaluate-and-degrade:** the NRE storm (b) is caused by running the script against a null; "degrade the output" still runs it. The alarm engine made the same call for the same reason (`ScriptedAlarmEngine.cs:751-756`). **Why Bad rather than Uncertain:** `BadNoCommunication` on the output tells the operator *why* and is honest about age; Uncertain says "imprecise", which a two-hour-old value is not. **Why propagate the sub-code rather than a fixed `BadInternalError`:** the sub-code is the diagnostic (`BadNoCommunication` = go look at the PLC; `BadWaitingForInitialData` = the input never arrived; `BadInternalError` = the script). **Why evaluate through Uncertain:** the alarm engine treats Uncertain as ready; a calc over an Uncertain input is still computable, and stamping the output Uncertain keeps the contract without holding perfectly good arithmetic hostage. + +**Null guard:** with hold-on-Bad the evaluator is never handed a Bad/null-from-Bad dep, so the `(double)ctx.GetTag("x").Value` NRE class disappears without touching scripts. A *Good* null (a driver that legitimately publishes null) still reaches the script as today — not this finding's concern. + +**Shared helper (T1):** + +```csharp +// src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/InputQuality.cs (new) +/// The one worst-of-input rule every derived-value seam uses (scripted alarms, virtual tags, +/// Calculation). Severity is the OPC UA Part 4 top-two bits; among equal severity the FIRST seen wins +/// (stable, so a re-evaluation over the same inputs reports the same sub-code). +public static class InputQuality +{ + public static uint WorstOf(IEnumerable inputs); // null entry ⇒ BadWaitingForInitialData + public static bool IsUsable(uint statusCode) => (statusCode >> 30) < 2; // Good or Uncertain +} +``` + +`ScriptedAlarmEngine.WorstInputStatus :923-941` becomes a call to `WorstOf` (its `StatusNeverPushed` skip is preserved by the alarm engine passing `null` for never-pushed entries, or — simpler — by keeping its skip loop and delegating only the comparison; choose the smaller diff and keep `ScriptedAlarmEngineTests` green). + +**Adapter:** `DependencyConsumerMuxAdapter.OnDependencyValueChanged` forwards `msg.StatusCode`; delete the false comment; xmldoc states the contract. + +**Alternatives considered:** *evaluate on Bad inputs and publish Uncertain* — rejected (NRE storm persists; Uncertain is dishonest for comms loss). *Hold on Bad but keep the output's prior status* — that is the bug. *A per-script opt-in (`[TolerateBadInputs]`)* — defer; nobody has asked, and the alarm engine has no such knob. + +### Implementation steps + +1. `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/InputQuality.cs` (new) + `ScriptedAlarmEngine.cs:923-941` re-point. +2. `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyConsumerMuxAdapter.cs:70-73` — forward `msg.StatusCode`. +3. `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs` — `_depValues` snapshots; `PrepareEvaluation` readiness + worst; `RunEvaluation`/`MarkEvaluation*` status-keyed dedup; `TagState.LastPublishedStatus`; `ReadOne`; a new `MarkInputsBad` publishing `(last, worst)` once per status change; xmldoc at `:405-437` updated. +4. `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationEvaluator.cs` — snapshot overload + pass-through `BuildReadCache`; passthrough fast path on snapshots. +5. `docs/drivers/Calculation.md` — "Input quality" section (policy table). + +### Tests + +- `tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/InputQualityTests.cs` (new; the project exists): truth table Good/Uncertain/Bad/never-pushed, stable tie-break. +- `tests/Server/…Runtime.Tests/VirtualTags/DependencyConsumerMuxAdapterTests` (new or extend): a `DependencyValueChanged` with `BadNoCommunication` reaches the consumer's `statusCode` parameter verbatim (batch path too). +- `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationDriverInputQualityTests.cs` (new — the contract test; evaluator spy counting `Evaluate` calls): + - deps `a`,`b` Good ⇒ output Good (regression); then `a` arrives `BadNoCommunication`/null ⇒ **no evaluation**, exactly ONE publish `(lastValue, 0x80310000)`; a second Bad push for `a` ⇒ no publish (transition-once); **no script-log entry** (the storm is gone); + - recovery: `a` returns Good with the **identical** value ⇒ evaluation runs, output re-published Good **even though the computed value equals the pre-outage value** (the dedup bypass) — *this is the vacuity-sensitive assertion; a recovery with a different value is not the guard*; + - `a` Uncertain ⇒ evaluation runs, output stamped with `a`'s Uncertain code; back to Good ⇒ Good republished; + - cold start with one dep never pushed and the other Bad ⇒ no publish (node stays `BadWaitingForInitialData`); once both have arrived (one Bad) ⇒ publish `BadNoCommunication`; + - passthrough script over a Bad input ⇒ Bad output, never `Ok(null) Good`; + - `ReadAsync` after a Bad publish reports the same status the subscriber saw. +- `CalculationEvaluatorTests` addition: `ctx.GetTag("a").StatusCode` inside a script reads the real (Uncertain) code and the real source timestamp, not eval-time `Good`. +- Existing `CalculationDriverTests` / `CalculationDependencyFlowTests` green (the Good-only paths are byte-identical). + +### Effort + Risk + +**Effort:** S-M. **Risk:** low-medium — driver-local state-shape change; behaviour change only when an input is non-Good (today that path is wrong by construction). The one regression to watch: a script that *relied* on receiving a null for a Bad input (null-tolerant scripts) now holds instead of computing; the docs task names this. + +--- + +## Finding 3 — 05/STAB-18: MQTT has no push-model `forceRaise`; a Bad-latched ref recovers only when the plant value changes (High) + +### Restatement + +The host's comms-loss recovery contract is "the driver re-publishes everything on re-subscribe". MQTT's `SubscribeAsync` (plain and Sparkplug) records handles and pushes nothing; the plain-mode `Deliver` suppresses a message whose value+status equal the driver-owned cache, and that cache survives reconnects and redeploys. So (a) after a watchdog reconnect the retained-seed replay carries the identical value ⇒ suppressed ⇒ every mux ref latched `BadNoCommunication` by #481 — and, after S15, every value node — stays Bad until the plant value changes; (b) after an address-space rebuild, nodes reseeded `BadWaitingForInitialData` are never refilled for quiet topics/metrics (Sparkplug: a rebuild triggers no rebirth). + +### Verification + +Confirmed exact (table), plus: `DegradeTopic :664-690` already shows the "stale + Raise `BadCommunicationError`" idiom for a refused filter, so the codebase has the per-ref degrade shape — it is just never applied on a connection loss. `MqttDriver.GetHealth :425-443` maps connection `Reconnecting` to `DriverState.Reconnecting` (non-Healthy), so the #527 watchdog does trip for a dead broker after 90 s of no messages — but in the common case MQTTnet's own supervisor reconnects well inside 90 s and the host never learns anything; the driver is the only party that can mark the outage. + +### Root cause + +Push-model drivers inherit none of the poll engine's edge behaviours (initial push, fresh per-subscription last-value state); the MQTT manager re-implemented the *steady-state* dedup (`ValuesAreDifferent`, deliberately duplicated from the engine) without the *edge* behaviours that make that dedup safe. + +### Proposed design + +Two halves, both in the driver, no host change: + +**(A) Initial push on every `SubscribeAsync`** — both `MqttSubscriptionManager` (`:325-365`) and `SparkplugIngestor` (`:470-506`): after registering the handle, for each resolved def whose `Values.Read(def.Name)` is an **observed** snapshot (status ≠ `BadWaitingForInitialData`), `Raise(def.Name, snapshot)` attributed to the new handle. Mirrors `CalculationDriver.SubscribeAsync :356-371` and the poll engine's `forceRaise: true`. Fixes shape (b) (a rebuilt node gets its last observed value + status back immediately) and completes the recovery of shape (a) once (B) has staled the cache. Cost: one raise per subscribed ref per (re)subscribe — the same burst a poll driver already produces. + +**(B) Stale on connection loss** — `MqttConnection` gains `public event Action? ConnectionLost;`, fired from `OnDisconnectedAsync :932-948` on the `Connected → Reconnecting` edge only (it runs on MQTTnet's dispatcher; subscribers must be non-blocking — the manager's handler is dictionary work + `OnDataChange` invokes, the same work `Deliver` already does on that thread). `MqttSubscriptionManager.OnConnectionLost()` and `SparkplugIngestor.OnConnectionLost()`: for every authored ref whose cached snapshot is observed and not already Bad, `Values.Update(ref, snapshot with { StatusCode = BadNoCommunication, ServerTimestampUtc = now })` (**value retained**) and `Raise` it. After this, the reconnect's retained seed / NBIRTH-DBIRTH replay carries the same value with `Good` ⇒ `unchanged == false` ⇒ raised ⇒ recovery — no suppression-memory surgery needed. A non-retained topic stays `BadNoCommunication` with its last value until the plant publishes, which is the honest answer. `AttachTo` subscribes the handler beside `Reconnected` (`:232`); `DetachFrom`/dispose unsubscribes. + +**Sparkplug policy override, recorded:** `ResetSessionState`'s remark (`:583-588`) argues values are not staled on a session change because "the connection's own health surface already reports the outage". That is precisely the stale-Good shape this family is about — health is not quality, and an external client subscribed to the metric's node sees neither the health page nor the outage. The remark is rewritten to state the new rule and to keep the part that is still right: a *session reset* for a **config rebuild** (same live connection) does not stale — only `ConnectionLost` does. Deaths still stale as before (they already use `BadNoCommunication`, `:97-102`). + +**Dedup-trap test discipline for this seam:** the recovery test MUST replay the **identical** payload after the stale and assert a Good raise; a test that replays a different payload passes against today's code and is vacuous. The control is the inverse: without (B), replaying the identical payload after a host-style latch raises nothing (assert this in a `[Fact]` that documents it as the pre-fix behaviour by calling the manager's `Deliver` directly with an unchanged payload and no prior stale — it must stay suppressed, proving suppression still exists for the steady state). + +**Alternatives considered:** *Clear suppression memory on reconnect only* — rejected (non-retained topics keep stale-Good in the cache and in reads). *Re-raise cached snapshots on `Reconnected` as Good* — rejected (asserts Good for a value the session has not re-observed). *Poll-style periodic re-publish* — rejected (unconfigurable traffic class, the #523 lesson). *Stale on `ShutdownAsync`/teardown* — rejected: teardown also runs for a config reinit on a healthy broker and would flip every non-retained tag Bad on each redeploy. + +### Implementation steps + +1. `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttConnection.cs` — `ConnectionLost` event + fire in `OnDisconnectedAsync` on the Connected→Reconnecting edge; xmldoc states the dispatcher-thread constraint. +2. `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs` — `SubscribeAsync` initial push; `OnConnectionLost`; `AttachTo`/detach wiring; the suppression comment at `:752-761` gains "initial push + connection-loss stale are what make this safe". +3. `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs` — same two; rewrite the `ResetSessionState` remark. +4. `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/LastValueCache.cs` — optional `Snapshot()`/`ForEachObserved` enumerator so the stale pass does not need the authored table (keep it minimal). +5. `docs/drivers/Mqtt.md` — "Quality on connection loss and re-subscribe" section. + +### Tests + +(`tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests`; manager tests drive `Deliver`/`SubscribeAsync` directly as the existing suite does; connection tests use `MiniBroker`.) + +- `MqttSubscriptionManagerRecoveryTests.cs` (new): + - seed cache via `Deliver(payload 42)`; `SubscribeAsync([ref])` ⇒ `OnDataChange` raised once with `42 Good` attributed to the new handle (initial push); a second identical `Deliver(42)` ⇒ **not** raised (steady-state suppression intact — the control); + - `OnConnectionLost()` ⇒ raised `42 / BadNoCommunication` (value retained), cache reads the same; `Deliver(42)` (the retained replay) ⇒ raised `42 / Good` — **the identical-value recovery**; a ref never observed is not raised by the stale; + - unsubscribe + re-`SubscribeAsync` ⇒ initial push again (rebuild shape). +- `SparkplugIngestorRecoveryTests.cs` (new): DBIRTH seeds metric `m=7`; `SubscribeAsync` ⇒ initial push `7 Good`; `OnConnectionLost()` ⇒ `7 / BadNoCommunication`; reconnect path (`OnReconnectedAsync` + replayed DBIRTH with `m=7`) ⇒ `7 / Good` raised; DDEATH still stales as before (regression). +- `MqttConnectionTests` addition (MiniBroker): broker drops the session ⇒ `ConnectionLost` fires exactly once per Connected→Reconnecting edge, not on initial-connect failure, not after dispose. +- `MqttDriverSessionLifecycleTests` addition: end-to-end through `MqttDriver` — connect, deliver, kill broker ⇒ driver `OnDataChange` carries `BadNoCommunication`; broker back + retained replay of the identical payload ⇒ Good raised. + +### Effort + Risk + +**Effort:** M (two managers + transport event + four test files). **Risk:** medium — behaviour change on the MQTT data plane (tags now flip Bad on a broker drop, and re-subscribe produces a burst). The initial-push burst on a large Sparkplug group is bounded by the authored set, identical in size to a rebirth. The `ConnectionLost` handler runs on MQTTnet's dispatcher — keep it allocation-light and never await in it (pin with a test that the handler completes synchronously). + +--- + +## Finding 4 — 02/S14: VT path stamps every dependency Good (Medium) + +### Restatement + +The virtual-tag actor stores dependency *values* only, evaluates unconditionally, and the Roslyn evaluator's read cache fabricates `Good` + eval-time timestamps for every input — so a VT over a comms-lost input keeps publishing fresh-Good results, and a script author cannot detect it because `ctx.GetTag(path).StatusCode` reads the fabricated Good. R2-03 fixed "broken script looks Good"; "broken input looks Good" is the same lie through the other door. + +### Verification + +Confirmed exact (table). R2-03 decisions this plan **keeps**: Bad published once per transition with the last-known value; recovery bypasses value-dedup; the inputs-ready gate (`AllDependenciesArrived`) suppresses Bad on cold start/respawn; `EvaluationResult.StatusCode` rides verbatim through the host bridge and is historized for non-reassert results; `ReassertValue` re-emits the last published value/status after a deploy reset. R2-03 explicitly left the mux "untouched — quality flows child→parent→publish actor; the mux only fans values into children"; this plan does not change the mux either — it changes what the *child* does with the status the mux already delivers. + +### Root cause + +Same as STAB-19: the consumer's storage shape (`Dictionary`) predates the mux carrying status, and the doc at `:36` turned the omission into a stated behaviour. + +### Proposed design + +Apply **the same policy as Finding 2** (hold on Bad / propagate worst / evaluate through Uncertain), expressed in the VT actor's existing R2-03 vocabulary: + +- `_dependencies : Dictionary` (value + status + timestamps). `OnDependencyChanged`/`OnDependencyBatch` store snapshots. +- `Evaluate(ts)`: `worst = InputQuality.WorstOf(deps of _dependencyRefs)` **before** anything else. If `!AllDependenciesArrived()` ⇒ return (unchanged warm-up). If `IsBad(worst)` ⇒ `OnInputsBad(worst, ts)`: publish `EvaluationResult(last value or null, worst)` **once per change of the published status** (generalise `_lastPublishedBad: bool` → `_lastPublishedStatus: uint`), meter `VirtualTagEval outcome:"inputs-bad"`, **no script-log entry** (inputs being Bad is not a script fault; the script log stays the per-failure diagnostic R2-03 defined). Otherwise evaluate; on success publish `(value, worst)` with dedup keyed on **(value, status)** — `if (_lastPublishedStatus == worst && _hasLastValue && Equals(_lastValue, result.Value)) skip` — which is R2-03's "bypass while recovering from Bad" made general (it now also covers Uncertain→Good and Bad-subcode changes). Script failure keeps `OnEvaluationFailed` (`BadInternalError`) unchanged. +- `OnReassertValue` re-emits `_lastPublishedStatus` (so a deploy reset of a VT whose input is comms-lost re-asserts `BadNoCommunication`, not `Good`). +- `IVirtualTagEvaluator` gains `Evaluate(id, expr, IReadOnlyDictionary)` with a **default implementation** that projects to values and calls the existing overload (the 5 fakes compile unchanged; the actor calls the new overload). `RoslynVirtualTagEvaluator.BuildReadCache :165-178` passes snapshots through — real status, real source timestamps — so `ctx.GetTag(path).StatusCode` is honest for Uncertain inputs (Bad never reaches the script under hold-on-Bad). +- Fix the `DependencyValueChanged` doc (`:33-38`): it no longer says the VT engine ignores it. + +**Alternatives considered:** same set as Finding 2, same resolution. One VT-specific option — *publish the computed value with `Uncertain` on Bad inputs* (the report's option b) — rejected for the same NRE/honesty reasons, and because it would make the VT path disagree with the Calculation driver, whose scripts share `VirtualTagContext` and the Monaco editor; one policy for both keeps `docs/ScriptEditor.md` true for every `ctx.GetTag` call site. + +### Implementation steps + +1. `src/Core/ZB.MOM.WW.OtOpcUa.Commons/Engines/IVirtualTagEvaluator.cs` — snapshot overload (default-implemented). +2. `src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs` — override; `BuildReadCache` pass-through. +3. `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs` — snapshot map, worst-of gate, `_lastPublishedStatus`, `OnInputsBad`, dedup key, reassert, docs. +4. `docs/ScriptEditor.md` (`ctx.GetTag(...).StatusCode` is real; scripts are not run while an input is Bad; outputs inherit the worst input quality) + `docs/Uns.md` VT section pointer to the contract. + +### Tests + +- `tests/Server/…Runtime.Tests/VirtualTags/VirtualTagActorInputQualityTests.cs` (new — the contract test, fake evaluator counting calls): + - deps arrive Good ⇒ `EvaluationResult Good` (regression); `a` arrives `BadNoCommunication`/null ⇒ **no evaluator call**, ONE `EvaluationResult(lastValue, 0x80310000)`; repeat Bad ⇒ nothing; Bad with a *different* sub-code ⇒ one more result with the new code; + - recovery with the **identical** computed value ⇒ `EvaluationResult Good` published (the (value,status) dedup key) — *the vacuity-sensitive assertion*; a same-value recovery that is NOT republished is exactly the v3-gate "Bad forever" trap, so this test is the guard and must be run once against a build with the dedup key reverted to value-only to see it fail before merge; + - Uncertain input ⇒ evaluated, result stamped Uncertain; `ctx.GetTag("a").StatusCode` observed by a capturing fake evaluator equals the input's code (not 0); + - cold start: `b` never arrives, `a` Bad ⇒ no result (inputs-ready gate preserved); after a deploy `ReassertValue` re-emits the last **status** (Bad input ⇒ Bad reassert); + - R2-03 script-failure cases (`VirtualTagActorTests` transition tests) unchanged and green. +- `VirtualTagHostActorTests` addition: a `BadNoCommunication` result is bridged verbatim to `AttributeValueUpdate` and **historized** with that status (parity with S13's Bad); a reassert is not historized (unchanged). + +### Effort + Risk + +**Effort:** S-M. **Risk:** low — actor-local; the interface change is default-implemented; every Good-path assertion in the existing suites stays byte-identical. + +--- + +## Live gate (docker-dev) — required before merge + +Rig: `docker-dev/docker-compose.yml` (AdminUI `http://localhost:9200`, login disabled; OPC UA `opc.tcp://localhost:4840` central-1 / `:4841` central-2 — **rebuild both centrals**, `:9200` round-robins). Modbus fixture on the shared host: `ssh dohertj2@10.100.0.35 'cd /opt/otopcua-modbus && docker compose --profile standard up -d'` (container `otopcua-pymodbus-standard`, `10.100.0.35:5020`, reachable from the rig's containers). + +**Why this gate is non-vacuous:** HR[5] in the `standard` profile is **config-seeded to the constant 5** and restored to 5 on a container restart, so its value is identical before and after the outage — a Good re-publish of HR[5] can only come from a force-republish, never from a value change. HR[100] auto-increments and is the *positive control* (it would recover even with the dedup trap present; if HR[100] recovers and HR[5] does not, the trap is present). + +Authoring (AdminUI, or the headless `POST /api/deployments` + `X-Api-Key` recipe from the MTConnect memory): + +1. `/raw` → MAIN cluster → Folder `Plant` → Driver `Modbus` (`Modbus`, endpoint `10.100.0.35:5020`, unit 1, poll 1000 ms) → Device `sim` → tags `HR5` (holding register 5, UInt16) and `HR100` (holding register 100, UInt16). No deadband. +2. `/uns` → Area `A` / Line `L` / Equipment `E1` → Tags tab → "+ Add reference" → `Plant/Modbus/sim/HR5` (UNS mirror). +3. `/raw` → Driver `Calc` (`Calculation`) → tag `HR5x2` with script `return (int)ctx.GetTag("Plant/Modbus/sim/HR5").Value * 2;` (dependency on HR5). +4. `/uns` → on `E1` a VirtualTag `VT5` = `return (int)ctx.GetTag("Plant/Modbus/sim/HR5").Value + 1;`. +5. Deploy; confirm all four nodes read Good: `dotnet run --project src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI -- read -u opc.tcp://localhost:4840 -n "ns=;s=Plant/Modbus/sim/HR5"` (and the UNS `s=A/L/E1/HR5`, raw `s=Plant/Calc/…/HR5x2`, UNS `s=A/L/E1/VT5`). Use `browse -r -d 4` to get the exact ids/namespace indexes. + +Run: + +6. Start four `subscribe` sessions (or one with `-r` over `s=Plant` + one over `s=A`) at `-i 500`; note HR5 = 5 Good, HR100 counting. +7. `ssh dohertj2@10.100.0.35 'docker stop otopcua-pymodbus-standard'`. Expected, in order: within ~1–2 s HR5/HR100 (raw + UNS) go **Bad 0x80050000 (BadCommunicationError), value null** — Modbus's own per-tag degrade (pre-existing behaviour, confirms the publish path); the calc `HR5x2` and `VT5` go **Bad 0x80050000 with their last value** (10 / 6) — STAB-19 + S14 propagating the input's sub-code; within ≤ 90 s central-1's log shows `declaring comms lost and reconnecting` and the value nodes re-stamp **`BadNoCommunication` 0x80310000** (S15 — visible as a status change on the subscription), calc/VT follow with `BadNoCommunication`. `/hosts` shows the driver Reconnecting. Both the status and the fact that the *last value is retained* on the calc/VT nodes are the assertions. +8. Hold the outage > 2 min (two watchdog windows) to see no churn beyond the retry log lines (S17 is R3-03's; note but do not fail on a re-trip). +9. `ssh dohertj2@10.100.0.35 'docker start otopcua-pymodbus-standard'`. Expected within ~15 s (retry 10 s + first poll): **HR5 re-publishes `5 Good`** on raw AND UNS without its value having changed (the force-republish); `HR5x2` re-publishes `10 Good`, `VT5` `6 Good` — same values as before the outage. HR100 recovers too (positive control). No `comms_latched_refs` Warning in the log. +10. **Falsification leg (required):** temporarily set a `deadband` of 1000 on HR5 in `/raw` and redeploy; repeat 7→9. Modbus clears its deadband cache in `TeardownAsync`, so HR5 must *still* recover — if it does not, the driver-side cache survived the reconnect and the audit Warning `… still BadNoCommunication 30s after reconnect …` must appear in the log with count 1 and `otopcua.driver.comms_latched_refs` incremented. Either outcome is informative; a silent non-recovery with no Warning fails the gate. +11. **Operator restart leg (after R3-03 is merged):** `/hosts` → Restart driver on `Modbus` ⇒ nodes go `BadNoCommunication` (T11's PostStop Tell) then recover Good with unchanged values (R3-03's re-subscribe + the poll initial push). +12. **MQTT leg (`docker-dev/docker-compose.mqtt.yml` + the Mosquitto fixture):** author one retained JSON topic tag; `docker stop` the broker's network path (or `docker pause otopcua-mosquitto`) ⇒ tag `BadNoCommunication` within the MQTTnet keep-alive; `unpause` ⇒ retained replay with the identical payload ⇒ `Good` re-publish. Then redeploy (rebuild) with the broker quiet ⇒ the rebuilt node shows the cached value immediately (initial push), not `BadWaitingForInitialData`. + +Record outcomes + status-code captures in `archreview/plans/STATUS.md`. + +--- + +## Task breakdown + +Branch: `fix/archreview-r3-02-stale-good-family` off master, **after R3-03 merges** (zero textual overlap by construction; if R3-03 slips, land this first and R3-03 rebases — the only shared file regions are the `Receive` table in `DriverHostActor`, which both plans append to). Commits grouped per block; blocks B (MQTT), C (S15), D (Calculation), E (VT) touch disjoint files and may run in parallel worktrees **but merge in the order B → C, D/E anywhere** (resolved choice #4). + +Verification bar: every task has a deterministic unit guard; the recovery tasks (T5, T10, T13, T16) carry an explicit vacuity note and a named falsification variant; T19 is the rig gate. TDD throughout: write the failing test, see it FAIL, implement, see it PASS. + +--- + +### T1 — `InputQuality.WorstOf` shared helper + alarm-engine re-point + +**Classification:** standard (pure helper; alarm engine refactor behind its suite) +**Estimated implement time:** 5 min +**Parallelizable with:** T2–T11 (different files); T12–T16 depend on it +**Files:** `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/InputQuality.cs` (new), `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs:923-941`, `tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/InputQualityTests.cs` (new) + +1. Truth-table Theory (all-Good ⇒ Good; one Uncertain ⇒ that code; Bad beats Uncertain; null entry ⇒ `BadWaitingForInitialData`; equal severity ⇒ first seen; `IsUsable` Good/Uncertain true, Bad false). `dotnet test tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests --filter "FullyQualifiedName~InputQualityTests"` → **FAIL**. +2. Implement; re-point `WorstInputStatus` (keep its never-pushed skip). Filter + `ScriptedAlarmEngineTests` → **PASS**. +3. Commit: `feat(core): InputQuality.WorstOf — the one worst-of-input rule (archreview R3-02 contract)`. + +### T2 — MQTT plain-mode initial push on `SubscribeAsync` + +**Classification:** medium-risk (data-plane burst on subscribe) +**Estimated implement time:** 5 min +**Parallelizable with:** T1, T6–T16 +**Files:** `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs`, `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttSubscriptionManagerRecoveryTests.cs` (new) + +1. Tests: seeded cache + `SubscribeAsync` ⇒ one raise per observed ref with the cached snapshot on the new handle; never-observed ref ⇒ no raise; identical `Deliver` afterwards ⇒ suppressed (control). Filter `FullyQualifiedName~MqttSubscriptionManagerRecoveryTests` → **FAIL**. +2. Implement the push after handle registration. → **PASS**. Commit: `fix(mqtt): initial push of cached snapshots on (re)subscribe — the push-model forceRaise (05/STAB-18 shape b)`. + +### T3 — `MqttConnection.ConnectionLost` + plain-mode stale to `BadNoCommunication` + +**Classification:** HIGH-RISK (transport event on MQTTnet's dispatcher; data-plane Bad on broker drop) +**Estimated implement time:** 5 min + 5 min tests +**Parallelizable with:** T1, T6–T16; after T2 +**Files:** `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttConnection.cs`, `MqttSubscriptionManager.cs`, `MqttConnectionTests.cs`, `MqttSubscriptionManagerRecoveryTests.cs` + +1. Tests: MiniBroker drop ⇒ `ConnectionLost` once per Connected→Reconnecting edge (not on initial failure / after dispose); manager `OnConnectionLost()` ⇒ each observed ref raised `value-retained / BadNoCommunication`, cache agrees; then identical `Deliver` ⇒ raised **Good** (the recovery; identical payload is load-bearing — a different payload is vacuous). → **FAIL**. +2. Implement event + handler + `AttachTo` wiring; handler must not await (assert completes synchronously). → **PASS**. Commit: `fix(mqtt): stale observed refs BadNoCommunication on connection loss so the identical replay recovers (05/STAB-18 shape a)`. + +### T4 — Sparkplug ingestor: initial push + connection-loss stale + +**Classification:** medium-risk +**Estimated implement time:** 5 min +**Parallelizable with:** T1, T6–T16; after T3 (uses the event) +**Files:** `Sparkplug/SparkplugIngestor.cs`, `tests/…/SparkplugIngestorRecoveryTests.cs` (new) + +1. Tests: DBIRTH seeds; `SubscribeAsync` initial push; `OnConnectionLost` ⇒ `BadNoCommunication` value retained; reconnect + identical DBIRTH replay ⇒ Good; DDEATH regression. → **FAIL**. +2. Implement; rewrite the `ResetSessionState` remark (`:583-588`). → **PASS**. Commit: `fix(mqtt/sparkplug): initial push on subscribe + BadNoCommunication on connection loss`. + +### T5 — MQTT driver-level recovery test + `docs/drivers/Mqtt.md` + +**Classification:** standard (test + docs) +**Estimated implement time:** 5 min +**Parallelizable with:** T6–T16; after T4 +**Files:** `tests/…/MqttDriverSessionLifecycleTests.cs`, `docs/drivers/Mqtt.md` + +1. End-to-end through `MqttDriver` with MiniBroker: deliver → broker drop → `BadNoCommunication` raised → broker back + identical retained replay → Good raised. Vacuity note in the test header. → **PASS** (after T2–T4). +2. Docs section "Quality on connection loss and re-subscribe". Commit: `test+docs(mqtt): end-to-end Bad-then-identical-replay recovery`. + +### T6 — Sink primitive `WriteStatuses` (interface default + node manager + SDK sink + deferred forward) + +**Classification:** HIGH-RISK (new sink contract member; node-manager SDK mutation under `Lock`) +**Estimated implement time:** 5 min interface + 5 min node manager +**Parallelizable with:** T1–T5, T12–T16; T7–T10 depend on it +**Files:** `Commons/OpcUa/IOpcUaAddressSpaceSink.cs`, `Commons/OpcUa/DeferredAddressSpaceSink.cs`, `OpcUaServer/OtOpcUaNodeManager.cs`, `OpcUaServer/SdkAddressSpaceSink.cs`, `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OtOpcUaNodeManagerWriteStatusesTests.cs` (new) + +1. Add `StatusWrite` + default `WriteStatuses` to the interface; run `dotnet test tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests --filter "FullyQualifiedName~DeferredSinkForwardingReflectionTests"` → **FAIL** (guard sees an unforwarded member). Forward in `DeferredAddressSpaceSink` → **PASS**. +2. Node-manager tests (value retained, status+timestamp change, unknown id no-op, tail-isolation) → **FAIL**; implement `WriteStatuses`/`WriteStatusLocked` + `SdkAddressSpaceSink` forward → **PASS**; full `OpcUaServer.Tests`. +3. Commit: `feat(opcua): IOpcUaAddressSpaceSink.WriteStatuses — status-only batch write retaining the node value (03/S15)`. + +### T7 — `OpcUaPublishActor.AttributeStatusBatchUpdate` + +**Classification:** standard +**Estimated implement time:** 5 min +**Parallelizable with:** T1–T5, T12–T16; after T6 +**Files:** `Runtime/OpcUa/OpcUaPublishActor.cs`, `tests/Server/…Runtime.Tests/OpcUa/OpcUaPublishActorStatusBatchTests.cs` (new) + +1. Test: message ⇒ `sink.WriteStatuses` with the same items; partial failure ⇒ Warning + tally. → **FAIL**. Implement handler + `OpcUaSinkWrite kind:"status"`. → **PASS**. Commit: `feat(runtime): publish actor routes status-only batches to the sink`. + +### T8 — `DriverHostActor.FanCommsLossToValueNodes` (raw + UNS, one batch) + +**Classification:** HIGH-RISK (data-plane behaviour change: value nodes go Bad on comms loss) +**Estimated implement time:** 5 min +**Parallelizable with:** T1–T5, T12–T16; after T7. **R3-03 boundary:** new method + one-line call in `OnDriverConnectivityChanged`; `HandleRestartDriver` and the watchdog untouched. +**Files:** `Runtime/Drivers/DriverHostActor.cs`, `tests/Server/…Runtime.Tests/Drivers/DriverHostActorCommsLossValueNodesTests.cs` (new) + +1. Tests (publish probe, 2 raw tags + 1 UNS ref): disconnect ⇒ ONE `AttributeStatusBatchUpdate` = {raw×2 (Raw), UNS mirror (Uns)} all `BadNoCommunication`; second driver's refs absent (control); reconnect ⇒ `ExpectNoMsg`. → **FAIL**. +2. Implement + xmldoc updates at `:1223-1247`. → **PASS**; `DriverHostActorCommsLossTests` + `DriverHostActorBatchFanOutTests` unchanged green. Commit: `fix(runtime): comms loss marks raw + UNS value nodes BadNoCommunication, value retained (archreview 03/S15)`. + +### T9 — Recovery audit: `_commsLatched` + `CommsRecoveryAudit` timer + `otopcua.driver.comms_latched_refs` + +**Classification:** medium-risk (hot-path dictionary probe in `ForwardBatchToMux`) +**Estimated implement time:** 5 min +**Parallelizable with:** T1–T5, T12–T16; after T8 +**Files:** `DriverHostActor.cs`, `Commons/Observability/OtOpcUaTelemetry.cs`, `DriverHostActorCommsLossValueNodesTests.cs` + +1. Tests (audit delay injected 300 ms): disconnect then reconnect with a value batch covering ALL latched refs ⇒ no Warning, no meter; covering NONE ⇒ Warning names driver + count, meter increments, `_commsLatched` entry removed afterwards (assert via a second disconnect re-latching cleanly). → **FAIL**. +2. Implement (set on loss, prune per routed item only while non-empty, single timer per driver keyed `comms-audit:{id}`). → **PASS**. Commit: `feat(runtime): audit refs still BadNoCommunication after a reconnect (dedup-trap tripwire)`. + +### T10 — S15 end-to-end recovery contract test (real `DriverInstanceActor`, identical-value initial push) + falsification control + +**Classification:** high-risk verification (guards T8/T9 against the v3-gate dedup trap) +**Estimated implement time:** ~15 min (the one deliberately larger task — scripted fake driver + watchdog timing) +**Parallelizable with:** T1–T5, T12–T16; after T9 +**Files:** `tests/Server/…Runtime.Tests/Drivers/DriverHostCommsLossRecoveryTests.cs` (new) + +1. Scripted `IDriver+ISubscribable` fake (stable health; `LastSuccessfulRead` controllable; `SubscribeAsync` pushes a configured initial snapshot): publish `5 Good` → freeze read + report Degraded → host status batch observed → fake connect succeeds + initial push **`5 Good` (identical)** ⇒ `AttributeValueBatchUpdate` `5 Good` for raw + UNS arrives after the status batch; no audit Warning. +2. **Control `[Fact]`:** fake with a silent `SubscribeAsync` ⇒ no Good batch, audit Warning + meter. Header comment states the vacuity rule (a different-value push is not the guard). +3. Commit: `test(runtime): comms-loss → identical-value recovery contract + silent-resubscribe control (03/S15)`. + +### T11 — `DriverInstanceActor.PostStop` announces comms loss + +**Classification:** medium-risk (additive Tell in PostStop; interacts with R3-03's restart re-subscribe) +**Estimated implement time:** 5 min +**Parallelizable with:** all; after T8. **Not** in the watchdog method; **after R3-03 merges** so the restart leg of the live gate recovers. +**Files:** `Runtime/Drivers/DriverInstanceActor.cs` (PostStop), `DriverInstanceActorTests.cs` + +1. Test: stop a Connected child ⇒ parent probe receives `ConnectivityChanged(id, false)` before `Terminated`. → **FAIL**. Implement. → **PASS**. Commit: `fix(runtime): a stopping driver child announces comms loss so restart/respawn latches its nodes Bad`. + +### T12 — `DependencyConsumerMuxAdapter` forwards the real `StatusCode` + +**Classification:** standard (one-line fix + stale-comment removal, theme 6) +**Estimated implement time:** 5 min +**Parallelizable with:** T1–T11; T13 depends on it +**Files:** `Runtime/VirtualTags/DependencyConsumerMuxAdapter.cs:70-73`, `tests/Server/…Runtime.Tests/VirtualTags/DependencyConsumerMuxAdapterTests.cs` (new or extend) + +1. Test: single + batch delivery with `BadNoCommunication` reaches the consumer verbatim. → **FAIL**. Implement; delete the "Quality is stripped" comment. → **PASS**. Commit: `fix(runtime): mux adapter forwards the dependency's real status code (05/STAB-19)`. + +### T13 — `CalculationDriver` input quality: hold on Bad, propagate worst, status-keyed dedup + +**Classification:** HIGH-RISK (calc output semantics change on non-Good inputs) +**Estimated implement time:** 5 min per case — split across commits if needed +**Parallelizable with:** T2–T11, T15–T16; after T1 + T12 +**Files:** `Driver.Calculation/CalculationDriver.cs`, `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationDriverInputQualityTests.cs` (new) + +1. Tests per Finding 2 §Tests (no-eval on Bad; transition-once; **identical-value recovery republished** — vacuity-sensitive; Uncertain evaluated + stamped; cold-start gate; passthrough over Bad; `ReadAsync` parity). → **FAIL**. +2. Implement snapshots, `LastPublishedStatus`, `MarkInputsBad`, readiness, `ReadOne`. → **PASS**; `CalculationDriverTests` + `CalculationDependencyFlowTests` green. Commit: `fix(calculation): hold evaluation on Bad inputs and propagate the worst input status; status-keyed dedup (05/STAB-19)`. + +### T14 — `CalculationEvaluator` snapshot overload (honest `ctx.GetTag().StatusCode`/timestamps) + +**Classification:** standard +**Estimated implement time:** 5 min +**Parallelizable with:** T2–T11, T15–T16; after T13 +**Files:** `Driver.Calculation/CalculationEvaluator.cs`, `CalculationEvaluatorTests.cs` + +1. Test: script reading `ctx.GetTag("a").StatusCode` sees the Uncertain code + the real source timestamp. → **FAIL**. Implement overload + pass-through `BuildReadCache` + passthrough fast path. → **PASS**. Commit: `fix(calculation): read cache carries real input status + timestamps`. + +### T15 — `IVirtualTagEvaluator` snapshot overload + Roslyn `BuildReadCache` pass-through + +**Classification:** medium-risk (Commons interface; default-implemented so fakes compile) +**Estimated implement time:** 5 min +**Parallelizable with:** T2–T14; T16 depends on it +**Files:** `Commons/Engines/IVirtualTagEvaluator.cs`, `Host/Engines/RoslynVirtualTagEvaluator.cs:165-178`, Host engine tests + +1. Test: Roslyn evaluator given a snapshot map — script reads the real status/timestamp. → **FAIL**. Implement. → **PASS**; all 5 fakes compile untouched. Commit: `feat(engines): IVirtualTagEvaluator snapshot overload; Roslyn read cache stops fabricating Good (02/S14)`. + +### T16 — `VirtualTagActor` honours input quality (hold on Bad / worst-of / (value,status) dedup / reassert status) + +**Classification:** HIGH-RISK (VT output semantics on non-Good inputs; touches R2-03's transition machinery) +**Estimated implement time:** 5 min per case — split across commits +**Parallelizable with:** T2–T14; after T1 + T15 +**Files:** `Runtime/VirtualTags/VirtualTagActor.cs`, `tests/Server/…Runtime.Tests/VirtualTags/VirtualTagActorInputQualityTests.cs` (new), `VirtualTagHostActorTests.cs` (historize parity case) + +1. Tests per Finding 4 §Tests (no-eval on Bad; once per status change; **identical-value recovery republished** — run once against a value-only dedup key to see it FAIL; Uncertain evaluated + stamped; inputs-ready gate; reassert carries status; host bridge historizes the Bad-input result). → **FAIL**. +2. Implement; fix the `DependencyValueChanged` doc (`:33-38`). → **PASS**; `VirtualTagActorTests`/`VirtualTagHostActorTests` (R2-03) green. Commit: `fix(runtime): virtual tags hold on Bad inputs and inherit the worst input status (archreview 02/S14)`. + +### T17 — Docs: the contract stated once + per-seam pointers + CLAUDE.md touch + +**Classification:** standard (docs) +**Estimated implement time:** 5 min +**Parallelizable with:** all +**Files:** `docs/DriverLifecycle.md` (new §"Quality contract" quoting §The quality contract verbatim; the comms-loss fan-out section gains the value-node half + the audit meter), `docs/Raw.md` + `docs/Uns.md` (one paragraph each, link), `docs/ScriptEditor.md` (GetTag status is real; scripts do not run on Bad inputs), `docs/drivers/Calculation.md`, `docs/drivers/Mqtt.md` (if not done in T5), `CLAUDE.md` (one bullet under the comms-loss watchdog section pointing at the contract + the `comms_latched_refs` meter + the "value nodes now go Bad" consumer note) + +Commit: `docs: the stale-Good quality contract, stated once (R3-02)`. + +### T18 — Full-suite sweep + STATUS/report bookkeeping + +**Classification:** standard +**Estimated implement time:** 5 min (+ suite wall-clock) +**Parallelizable with:** nothing (after T1–T17) +**Files:** `archreview/plans/STATUS.md`, the four source reports' prior-finding tables + +`dotnet test ZB.MOM.WW.OtOpcUa.slnx` (unit legs); `Runtime.Tests` run 3× for the known flake profile (#500); record. Commit: `docs(archreview): R3-02 status`. + +### T19 — LIVE GATE: docker-dev comms-loss → identical-value recovery (Modbus const HR[5] + calc + VT + MQTT retained) — operator/rig task + +**Classification:** HIGH-RISK verification gate (merge-blocking) +**Estimated implement time:** ~45 min rig session +**Parallelizable with:** nothing (needs the built branch; restart leg needs R3-03 merged) +**Files:** none (findings recorded in STATUS.md) + +Run the twelve-step recipe in §Live gate. Pass criteria: (7) value nodes + calc + VT Bad with retained values and the `BadNoCommunication` re-stamp at the watchdog; (9) **HR[5] = 5 re-published Good on raw AND UNS with no value change**, calc/VT follow with unchanged values; (10) deadband leg recovers or is caught by the audit Warning; (12) MQTT identical retained replay recovers and a rebuild shows the cached value. Record status-code captures from `subscribe` output. + +--- + +### Sequencing summary + +``` +T1 ───────────────────────────────┐ +T2 → T3 → T4 → T5 (B: MQTT — MUST merge before C) +T6 → T7 → T8 → T9 → T10 (C: S15 host; T11 after T8 AND after R3-03 merges) +T1 + T12 → T13 → T14 (D: Calculation) +T1 + T15 → T16 (E: VT) +T17 anytime; T18 after all; T19 last +``` + +Overall effort: **~2 dev-days code+unit, plus the rig session**. The three HIGH-RISK data-plane slices (T3 MQTT stale, T8 value-node Bad, T13/T16 hold-on-Bad) each land behind their full suites; the two recovery contract tests (T10, T16's identical-value case) must each be seen to FAIL once against a reverted dedup before merge — that is the whole reason this family re-appeared. diff --git a/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json b/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json new file mode 100644 index 00000000..20d2dce8 --- /dev/null +++ b/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json @@ -0,0 +1,153 @@ +{ + "planPath": "archreview/plans/R3-02-stale-good-family-plan.md", + "lastUpdated": "2026-08-20", + "tasks": [ + { + "id": "T1", + "subject": "InputQuality.WorstOf shared helper (Core.Abstractions) + truth-table tests; re-point ScriptedAlarmEngine.WorstInputStatus", + "status": "pending", + "blockedBy": [] + }, + { + "id": "T2", + "subject": "MQTT plain-mode initial push of cached snapshots on SubscribeAsync — push-model forceRaise (05/STAB-18 shape b)", + "status": "pending", + "blockedBy": [] + }, + { + "id": "T3", + "subject": "MqttConnection.ConnectionLost event + plain-mode stale observed refs to BadNoCommunication (value retained) so the identical replay recovers (05/STAB-18 shape a)", + "status": "pending", + "blockedBy": [ + "T2" + ] + }, + { + "id": "T4", + "subject": "SparkplugIngestor: initial push on subscribe + connection-loss stale; rewrite the 'values are deliberately not staled' remark", + "status": "pending", + "blockedBy": [ + "T3" + ] + }, + { + "id": "T5", + "subject": "MQTT driver-level end-to-end recovery test (broker drop -> Bad -> identical retained replay -> Good) + docs/drivers/Mqtt.md section", + "status": "pending", + "blockedBy": [ + "T4" + ] + }, + { + "id": "T6", + "subject": "IOpcUaAddressSpaceSink.WriteStatuses (StatusWrite, default impl) + OtOpcUaNodeManager status-only batch retaining value + SdkAddressSpaceSink/DeferredAddressSpaceSink forwards (03/S15)", + "status": "pending", + "blockedBy": [] + }, + { + "id": "T7", + "subject": "OpcUaPublishActor.AttributeStatusBatchUpdate handler -> sink.WriteStatuses (metered, partial-failure Warning)", + "status": "pending", + "blockedBy": [ + "T6" + ] + }, + { + "id": "T8", + "subject": "DriverHostActor.FanCommsLossToValueNodes: one BadNoCommunication status batch over raw + UNS nodes on ConnectivityChanged(false); nothing on reconnect (new method — R3-03 boundary)", + "status": "pending", + "blockedBy": [ + "T7" + ] + }, + { + "id": "T9", + "subject": "Recovery audit: _commsLatched per driver, pruned by routed values, CommsRecoveryAudit timer + otopcua.driver.comms_latched_refs meter (dedup-trap tripwire)", + "status": "pending", + "blockedBy": [ + "T8" + ] + }, + { + "id": "T10", + "subject": "S15 end-to-end recovery contract test: real DriverInstanceActor + scripted fake with IDENTICAL-value initial push; silent-resubscribe falsification control", + "status": "pending", + "blockedBy": [ + "T9" + ] + }, + { + "id": "T11", + "subject": "DriverInstanceActor.PostStop Tells ConnectivityChanged(false) so operator restart / respawn latches nodes Bad (land after R3-03 merges)", + "status": "pending", + "blockedBy": [ + "T8" + ] + }, + { + "id": "T12", + "subject": "DependencyConsumerMuxAdapter forwards the real StatusCode; delete the false 'quality is stripped' comment (05/STAB-19)", + "status": "pending", + "blockedBy": [] + }, + { + "id": "T13", + "subject": "CalculationDriver input quality: snapshots per dep, hold on Bad, publish worst-of-input once per status change, status-keyed dedup bypass on recovery, ReadOne parity", + "status": "pending", + "blockedBy": [ + "T1", + "T12" + ] + }, + { + "id": "T14", + "subject": "CalculationEvaluator snapshot overload: read cache carries real input status + timestamps; passthrough fast path on snapshots", + "status": "pending", + "blockedBy": [ + "T13" + ] + }, + { + "id": "T15", + "subject": "IVirtualTagEvaluator snapshot overload (default-implemented) + RoslynVirtualTagEvaluator.BuildReadCache pass-through (02/S14)", + "status": "pending", + "blockedBy": [] + }, + { + "id": "T16", + "subject": "VirtualTagActor honours input quality: snapshot map, hold on Bad, worst-of status, (value,status) dedup key, reassert carries status; host-bridge historize parity test", + "status": "pending", + "blockedBy": [ + "T1", + "T15" + ] + }, + { + "id": "T17", + "subject": "Docs: the quality contract stated once (DriverLifecycle.md) + Raw/Uns/ScriptEditor/Calculation pointers + CLAUDE.md bullet", + "status": "pending", + "blockedBy": [] + }, + { + "id": "T18", + "subject": "Full-suite sweep (Runtime.Tests 3x for the #500 flake profile) + STATUS.md / source-report bookkeeping", + "status": "pending", + "blockedBy": [ + "T5", + "T10", + "T11", + "T14", + "T16", + "T17" + ] + }, + { + "id": "T19", + "subject": "LIVE GATE: docker-dev comms-loss -> identical-value recovery (Modbus constant HR[5] raw+UNS, calc, VT, deadband falsification leg, restart leg, MQTT retained replay)", + "status": "pending", + "blockedBy": [ + "T18" + ] + } + ] +}