Merge review R6 fixes: numeric array writes converted element-wise (checked) to the node's element type, refusing out-of-range/fractional/NaN; array-to-scalar refusal scoped to Modbus/S7; Primary-verdict pre-join window documented + real MemberUp subscription test; passive LocalDb pair half drain-gated on its mesh partner

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-08 08:43:59 -04:00
37 changed files with 764 additions and 112 deletions
+11
View File
@@ -103,6 +103,17 @@ Two postures are deliberate:
a key with no dial and no listener (e.g. a fleet-wide secret on an unpaired
node) captures changes but shares them with nobody, and gating it would park
its events behind a Primary that does not hold them (review R1 m3).
- **A gated node stands down only for the peer that holds its rows — and BOTH
halves of a pair know it.** The dialling half identifies its peer by the host
of `LocalDb:Replication:PeerAddress`. The passive half (`ApiKey` +
`SyncListenPort`, no `PeerAddress`) identifies it as its **per-cluster mesh
partner** (the other member of its `cluster-{ClusterId}` role). Before review
R6 MINOR-4 the passive half knew no peer at all, so as Secondary it kept
draining beside its Primary partner and the historian received every alarm
event twice. ⚠️ A passive half with **no** cluster-scoped role (a legacy,
pre-Phase-6 node) still cannot identify its partner and keeps draining — it
cannot prove which Primary holds its rows, and deferring to the wrong one
would strand them.
- **A gate that throws means "not now", never permission.** Draining on a gate
fault would put a pair into dual delivery exactly when its redundancy signal is
sick.
+5
View File
@@ -357,6 +357,11 @@ is client-visible.
> pair (role still unknown, a driver peer Up) the gate already denies, exactly
> as the route does (review R1 m2 — previously the view sat on its single-node
> "allow" seed until the first snapshot, reopening the flap for that window).
> It mirrors, it does not deny from start: until this node has **joined** and
> sees its peer Up, its member count is 0 and both the published verdict and the
> live route **allow** (seconds normally; up to the bootstrap guard's 25 s
> partner probe). The two agree there, so the flap cannot occur, but a native
> ack in that pre-join window is serviced (review R6 MINOR-2).
> **Best-effort during reconnect.** If a native ack arrives while the owning
> driver is `Connecting`/`Reconnecting`, it is dropped with a debug log and **not**
+2 -2
View File
@@ -526,13 +526,13 @@ Three data-plane surfaces are **Primary-gated** so a warm standby never touches
Before archreview 03/S4 the unknown-role case defaulted to *allow* unconditionally, opening a dual-primary window: a freshly-booted secondary on a multi-node cluster serviced shared-device writes and emitted duplicate fleet-wide alerts for up to the ~10 s `RedundancyStateActor` heartbeat (and indefinitely if the snapshot's NodeId never matched this node). Membership is authoritative much earlier than DPS delivery, so an unknown role on a **multi-driver** cluster now default-**denies**. The driver member count is read live from `Cluster.State.Members` (Up members with the `driver` role); a non-cluster ActorRefProvider yields 0 ⇒ single-node posture. If a snapshot arrives that never mentions this node while a driver peer exists (the 03/S5 identity-mismatch shape), `DriverHostActor` logs a one-time Warning so the silent mismatch is diagnosable.
**What a client sees during the boot window (multi-node, role unknown):** an inbound write is **rejected** (not queued — OT actuation seconds late is a surprise). The reject rides the sanctioned optimistic-write self-correction surface: the node reverts to its prior value with a transient Bad-quality blip and a Part 8 `AuditWriteUpdateEvent`; the client's synchronous write call still returned Good (exactly as any failed device write behaves since the #5 write-outcome self-correction). The rejection reason distinguishes the boot window (`"not primary (role unknown)"`) from a steady-state Secondary (`"not primary"`). Acks are dropped (Warning-logged when role-unknown) — and since review R1 m2 a native OPC UA Acknowledge is refused up front (`BadInvalidState`) in the same window: `DriverHostActor` publishes its Primary verdict to `IDriverPrimaryRoleSource` (the native-ack gate, and the MQTT Sparkplug primary-host claim) once at start and again on every cluster membership change, not only on redundancy snapshots, so the published verdict denies as soon as a driver peer is Up rather than sitting on the single-node "allow" seed until the first snapshot. The alerts emit is skipped (the Primary peer publishes the single fleet copy). The window closes on the first delivered snapshot.
**What a client sees during the boot window (multi-node, role unknown):** an inbound write is **rejected** (not queued — OT actuation seconds late is a surprise). The reject rides the sanctioned optimistic-write self-correction surface: the node reverts to its prior value with a transient Bad-quality blip and a Part 8 `AuditWriteUpdateEvent`; the client's synchronous write call still returned Good (exactly as any failed device write behaves since the #5 write-outcome self-correction). The rejection reason distinguishes the boot window (`"not primary (role unknown)"`) from a steady-state Secondary (`"not primary"`). Acks are dropped (Warning-logged when role-unknown) — and since review R1 m2 a native OPC UA Acknowledge is refused up front (`BadInvalidState`) in the same window: `DriverHostActor` publishes its Primary verdict to `IDriverPrimaryRoleSource` (the native-ack gate, and the MQTT Sparkplug primary-host claim) once at start and again on every cluster membership change, not only on redundancy snapshots, so the published verdict denies as soon as a driver peer is Up rather than sitting on the single-node "allow" seed until the first snapshot. ⚠️ "As soon as a driver peer is Up" means **in this node's own cluster view**, i.e. after it has joined: the host is spawned before the join, when the view holds no members, and a member count of 0 takes the single-node *allow* posture — for the published verdict and the live gate alike. So a node cold-starting into a pair whose peer is already Primary allows until its own join completes (seconds normally; up to `Cluster:BootstrapGuard:PartnerProbeSeconds`, 25 s, under the bootstrap guard), and in that window the MQTT Sparkplug primary-host claim can be held on both nodes. Native ack is not exposed (the view and the route agree, so there is no acked → unacked revert). Denying until self is Up was not adopted: it would also deny writes/acks during a single-node cluster's own join (review R6 MINOR-2). The alerts emit is skipped (the Primary peer publishes the single fleet copy). The window closes on the first delivered snapshot.
Under warm/hot redundancy both cluster nodes run `ScriptedAlarmHostActor` and evaluate scripted alarms, keeping each node's address space and engine state warm for instant failover. To avoid duplicate rows on `/alerts` and duplicate historian writes, only the Primary node publishes externally:
- **`alerts` topic emission** — `ScriptedAlarmHostActor` and `DriverHostActor.ForwardNativeAlarm` subscribe to the `redundancy-state` DPS topic and cache the local node's `RedundancyRole`, then gate the cluster `alerts` publish through `PrimaryGatePolicy` (table above). The OPC UA condition-node write and inbound ack/shelve command processing remain **ungated** on both nodes so the secondary is always ready to serve clients after a failover.
- **`HistorianAdapterActor` historization** — likewise Primary-gated so alarm historization is exactly-once across all alarm sources. The actor subscribes to the `alerts` DPS topic and translates each `AlarmTransitionEvent` → `AlarmHistorianEvent` before enqueuing it on the sink; scripted alarms therefore historize exactly once regardless of cluster size.
- **The alarm sink's DRAIN** — a second, independent gate on the same decision. The enqueue gate above is not sufficient any more: since Phase 2 the store-and-forward buffer lives in the replicated LocalDb, so the Secondary holds a full copy of the Primary's queued rows and an ungated drain there would re-deliver every event continuously. `LocalDbStoreAndForwardSink` takes a `drainGate` fed from `IRedundancyRoleView` — a singleton `DriverHostActor` publishes its `PrimaryGatePolicy` verdict to, because the drain runs on a timer and cannot receive `RedundancyStateChanged` itself. A gated-off node reports `HistorianDrainState.NotPrimary`. The gate applies only to a node whose queue a peer actually holds — `LocalDbReplicationPolicy.SharesRowsWithPeer`: `LocalDb:Replication:ApiKey` AND (`PeerAddress` OR `LocalDb:SyncListenPort` > 0); any other node always drains (review R1 m3). Delivery is **at-least-once across a failover** by design: rows the old Primary delivered just before dying may not have replicated their deletes yet. See [AlarmHistorian.md](AlarmHistorian.md).
- **The alarm sink's DRAIN** — a second, independent gate on the same decision. The enqueue gate above is not sufficient any more: since Phase 2 the store-and-forward buffer lives in the replicated LocalDb, so the Secondary holds a full copy of the Primary's queued rows and an ungated drain there would re-deliver every event continuously. `LocalDbStoreAndForwardSink` takes a `drainGate` fed from `IRedundancyRoleView` — a singleton `DriverHostActor` publishes its `PrimaryGatePolicy` verdict to, because the drain runs on a timer and cannot receive `RedundancyStateChanged` itself. A gated-off node reports `HistorianDrainState.NotPrimary`. The gate applies only to a node whose queue a peer actually holds — `LocalDbReplicationPolicy.SharesRowsWithPeer`: `LocalDb:Replication:ApiKey` AND (`PeerAddress` OR `LocalDb:SyncListenPort` > 0); any other node always drains (review R1 m3). Within a pair BOTH halves stand down for their partner: the dialling half knows it by the `PeerAddress` host, the passive half (no `PeerAddress`) as its per-cluster mesh partner (review R6 MINOR-4 — before that the passive half never stood down, so both halves drained and every event reached the historian twice; a passive half with no `cluster-{ClusterId}` role still cannot identify its partner and keeps draining). Delivery is **at-least-once across a failover** by design: rows the old Primary delivered just before dying may not have replicated their deletes yet. See [AlarmHistorian.md](AlarmHistorian.md).
- **Continuous-historization value CAPTURE** (Gitea #537) — a third gate on the same role rule, and the
one that gates the *opposite* half of its pipeline. Both pair nodes serve the same address space and see
+31 -12
View File
@@ -343,17 +343,34 @@ What a client sees on a writable array node:
- **Whole-array writes only.** The value must be a 1-D array of exactly `arrayLength` elements
(an unbounded `arrayLength`/`ArrayDimensions = [0]` takes any length — reachable only on
OpcUaClient); anything else is `BadTypeMismatch`, refused inline before dispatch.
- **The element type must match the node's DataType exactly** — an `Int32[]` to an `Int16` array,
a `UInt32[]` to an `Int32` array or a `Double[]` to a `Float` array is `BadTypeMismatch`, refused
inline (the one equivalence is a `Byte` array, which arrives as a ByteString). This is the
check the OPC UA SDK would normally make itself: it skips its own type check on any node with an
`OnWriteValue` handler, which every writable equipment node has, so the node manager makes it
instead. Without it a `UInt32[]` reached Galaxy's `int[]` encoder by CLR array covariance and was
written bit-reinterpreted (4,000,000,000 → −294,967,296) with a Good result. **Scalar** writes keep
their looser rule on purpose — any numeric to any numeric node — because consumers (ScadaBridge)
write CLR-typed values such as a `Double` to a `Float` node, and every driver's scalar encoder
converts with a range check, so an out-of-range scalar fails the device write and is reverted.
- An **array value to a scalar node** is likewise `BadTypeMismatch`, refused inline.
- **The element type must match the node's DataType, or be a numeric type that converts to it
exactly.** A matching element type (or a `Byte` array, which arrives as a ByteString) is dispatched
as written. A **numeric** array of another numeric element type is **converted, checked, element by
element** into the node's element type, and the converted array is what the node stores and the
driver receives — an `Int32[]` written to an `Int16` array reaches the driver as `short[]`. The whole
write is refused `BadTypeMismatch` inline, nothing dispatched, if **any** element is out of the
target's range, fractional for an integer node (`1.5` → `Int16` refused; `2.0` → `2`), NaN/±∞ for an
integer node, or finite but beyond `Float` range for a `Float` node. Every **non-numeric** mismatch
(`Boolean[]` → `Int32`, `Int32[]` → `String`, a `Variant[]`) is refused. This is the check the OPC UA
SDK would normally make itself: it skips its own type check on any node with an `OnWriteValue`
handler, which every writable equipment node has, so the node manager makes it instead. Without it a
`UInt32[]` reached Galaxy's `int[]` encoder by CLR array covariance and was written bit-reinterpreted
(4,000,000,000 → −294,967,296) with a Good result — that write is still refused (4,000,000,000
overflows `Int32`), while an in-range `UInt32[]` converts. The conversion is required, not a
nicety: ScadaBridge writes the CLR list it decoded from a `List` attribute, and its element types are
only Boolean/Int32/Float/Double/String/DateTime, so an exact-match rule left every `Int16`/`UInt16`/
`Byte`/`UInt32`/`Int64`/`UInt64` array node (Modbus's default `Int16` included) and a `Double` list to
a `Float` array unwritable from it. **Scalar** writes keep their looser rule — any numeric to any
numeric node, unconverted — because every driver's scalar encoder converts with a range check, so an
out-of-range scalar fails the device write and is reverted.
- An **array value to a scalar node** is refused `BadTypeMismatch` inline **only on Modbus and S7**,
whose scalar encoders cannot take one (`DriverArrayWriteSupport.RefusesArrayValueOnScalarTag`). Every
other driver keeps the pre-#610 behaviour — the write is dispatched and the driver decides. That
matters for browse-committed tags: the `/raw` commit sets the DataType but never `isArray`, so an
upstream array variable (OpcUaClient) or a Galaxy array attribute committed from the browser is a
**scalar** node into which the driver publishes an array, and writing that array back still works
(OpcUaClient forwards it to the upstream server, which arbitrates the shape; Galaxy encodes it). Set
`isArray` + `arrayLength` on such a tag to get the array-node checks above.
- A partial **IndexRange** write is
`BadWriteNotSupported` — every array-capable driver writes the whole block, and the server
does not hold the device array authoritatively enough to read-modify-write it.
@@ -362,7 +379,9 @@ What a client sees on a writable array node:
write-outcome self-correction, unchanged).
- Flipping a tag's array-write capability (a driver-type or element-type edit on an existing
array tag) is a **surgical** in-place writability update, not a rebuild: `ArrayWritable` is on
the classifier's raw-tag whitelist, and the mirrors repair through `BackingArrayWritable`.
the classifier's raw-tag whitelist, and the mirrors repair through `BackingArrayWritable`. A
change to the array-on-scalar refusal (`ScalarRefusesArrayValue` — only a driver-type edit can flip
it) is deliberately NOT whitelisted and rebuilds.
> ⚠️ **Upgrade note — array tags become client-writable without a deploy.** Before #610 every
> array node was read-only regardless of its authored `AccessLevel`. On the first boot after
+1 -1
View File
@@ -116,4 +116,4 @@ The session-less alarm feed (`GatewayGalaxyAlarmFeed`) and alarm acknowledger (`
- **Duplicate `attributeRef` (#605 / 06-S-13)**: two authored RawPaths dialling the same Galaxy attribute are both dialled, but the reverse map is first-wins, so value fan-out and alarm `ConditionId` surface under the first RawPath only. The loser is logged at Warning and recorded in the skipped-tag ledger with the winner path in the reason.
- **Subscribe-once reads are bounded (#605 / 06-S-3)**: `ReadAsync` on the production path (SubscribeBulk, first event, Unsubscribe) carries an internal 30 s deadline in addition to the caller token; slots that never publish come back `BadTimeout`, so `CancellationToken.None` can no longer hang on a silent tag.
- **Supervisory advise is retried after a thrown failure (#605 / 06-S-12)**: the writer marks an item handle advised before the call; a thrown advise (not just a non-OK status) now unmarks it, fails the write closed and re-advises on the next write.
- **Array writes (#610)**: a Galaxy array tag is client-writable only for the `MxValueEncoder` array types (OPC UA `Boolean`/`Int32`/`Int64`/`Float`/`Double`/`String`/`DateTime`) **and only with a declared `arrayLength`** — with none the node is unbounded, and a short whole-array write would *resize* the attribute. The element type must match the node exactly (`BadTypeMismatch` inline otherwise), and `MxValueEncoder` itself refuses `uint[]`/`ulong[]`: CLR array covariance let them match the `int[]`/`long[]` arms and be written bit-reinterpreted. Encoder-level only — Galaxy array writes have **no live gate** yet. See [Uns.md §Array writes](../Uns.md#array-writes-610), including the upgrade note.
- **Array writes (#610)**: a Galaxy array tag is client-writable only for the `MxValueEncoder` array types (OPC UA `Boolean`/`Int32`/`Int64`/`Float`/`Double`/`String`/`DateTime`) **and only with a declared `arrayLength`** — with none the node is unbounded, and a short whole-array write would *resize* the attribute. The element type must match the node or be a numeric type that converts to it exactly — a numeric array of another element type is converted, checked, to the node's type in the node manager (an `Int32` list to an `Int64` array arrives as `long[]`), and any unrepresentable element refuses the write `BadTypeMismatch` inline. An array written to a **scalar** Galaxy node (a browse-committed array attribute without `isArray`) is still dispatched, as before #610. Separately, `MxValueEncoder` itself refuses `uint[]`/`ulong[]`: CLR array covariance let them match the `int[]`/`long[]` arms and be written bit-reinterpreted. Encoder-level only — Galaxy array writes have **no live gate** yet. See [Uns.md §Array writes](../Uns.md#array-writes-610), including the upgrade note.
+8 -3
View File
@@ -363,9 +363,14 @@ Hence: bind the claim to a connection, and rebuild the connection on role change
| Role unknown, no driver peer (single node) | open | `ONLINE` |
The verdict comes from `PrimaryGatePolicy.ShouldServiceAsPrimary` via `IDriverPrimaryRoleSource`
(`Core.Abstractions`), published by `DriverHostActor` on every redundancy snapshot and **sampled on a
1-second timer** rather than evented — a process-singleton raising events at per-instance drivers
outlives every driver it holds a reference to.
(`Core.Abstractions`), published by `DriverHostActor` at host start, on every cluster membership change and on
every redundancy snapshot, and **sampled on a 1-second timer** rather than evented — a process-singleton
raising events at per-instance drivers outlives every driver it holds a reference to.
> ⚠️ **"Driver peer present" is this node's own cluster view.** Before the node has joined, it sees no
> members, which is the single-node row: a node cold-starting into a pair whose peer is already Primary
> can open the claim until its join completes (seconds; up to the bootstrap guard's 25 s partner probe),
> so both nodes may briefly hold it (review R6 MINOR-2).
> **Accepted trade: a brief edge-visible `OFFLINE` during failover**, while the old Primary stands down
> and the new one connects and republishes. That is a genuine state transition — for that window there
@@ -94,7 +94,10 @@ Replication has two knobs, both under `LocalDb`:
> the passive node has no `PeerAddress`; `SyncListenPort` is deliberately not part of it, since a
> port with no key is fail-closed and syncs with nobody). The alarm-historian drain gate uses the
> stricter `ApiKey` AND (`PeerAddress` OR `SyncListenPort > 0`) — so set the key **and** the port/peer
> together, or the node keeps draining its own queue. Consequences at enablement time:
> together, or the node keeps draining its own queue. Once gated, the passive half stands down for
> its **per-cluster mesh partner** (it has no `PeerAddress` to name one) — so it must carry the pair's
> `cluster-{ClusterId}` role, or as Secondary it keeps draining beside its Primary and the historian
> gets every alarm event twice (review R6 MINOR-4). Consequences at enablement time:
>
> - A configured node registers with **`baselineExistingRows: true`**, so the rows **already in the
> file** are seeded into the row-version ledger at the LWW floor (HLC `0`) and actually travel.