From 0a5d03da951dd02c604fc30c5e90a68f8c43079d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 08:19:30 -0400 Subject: [PATCH] docs: apply CLAUDE-FACTS to CLAUDE.md, fix AlarmAck LDAP wording, retitle KNOWN LIMITATION 2 (#624) Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- CLAUDE.md | 115 ++++++++++++++++++++++++++-------- docker-dev/docker-compose.yml | 8 ++- 2 files changed, 93 insertions(+), 30 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3d65298b..63feeb58 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -186,10 +186,13 @@ is still default-closed; what changed is which deltas it whitelists. bounded resync). A **rebuild** still advances even with failures. - **Any surgical `false`/throw still ratchets to `SafeRebuild()`** — that one-way fallback is what makes every whitelist above safe. -- ⚠️ **Known gap, no longer neutral:** `AnnounceAddedNodes` is Uns-only, so surgically-added - **raw** containers/tags are materialised but never announced — previously invisible (the - rebuild forced a re-browse anyway), now genuinely undiscoverable until a client re-browses. - Added UNS reference variables ARE announced. +- **Announcements:** `AnnounceAddedNodes` announces surgically-added **Raw** containers/tags + (`ComputeRawAddAnnouncements`, #599 — the former "raw adds are undiscoverable until a re-browse" gap is + closed) as well as added UNS reference variables. +- **Array writability rides the same whitelist:** `ArrayWritable` joins the surgical raw-tag attributes and + `UnsReferenceVariable` carries `BackingArrayWritable` (see "Equipment-node writes" under Authentication). Removing + an equipment that has references removes the references before the equipment subtree, and + `RemoveEquipmentSubtree` severs the inverse `Organizes` edges. See `docs/plans/2026-08-15-surgical-deploys.md`, `docs/Uns.md` and `docs/Raw.md`. @@ -268,10 +271,12 @@ deploy) — not a wall-clock timer. Removed with the feature: `TagSetDrift`/`Tag `DriverInstanceActor`'s drift timer, and `ITagDiscovery.AuthoredDiscoveryRefs` / `DiscoveryStreamIncludesAuthoredTags`. -⚠️ **`IHostConnectivityProbe` is still unconsumed.** `GetHostStatuses()` has no production call site and -nothing ever writes a `DriverHostStatus` row (the table was re-created in the v3 initial migration, so -confirm intent before deleting). The separable `OnHostStatusChanged` event is the useful half. Tracked -as Gitea **#521**. +**`IHostConnectivityProbe` is consumed (Gitea #521, closed).** `DriverInstanceActor` subscribes +`OnHostStatusChanged`, pulls `GetHostStatuses()` fresh, and the host set rides `DriverHealthChanged` to the +AdminUI `/hosts` page — no SQL: the `DriverHostStatus` table is dropped, and so is +`DriverInstanceResilienceStatus` (#524; the live data goes over the `driver-resilience-status` telemetry +channel only). **Multi-host dispatch is wired too:** `DriverInstanceActor` resolves the breaker/host key +per call via `IPerCallHostResolver` (Modbus, FOCAS, AbLegacy, TwinCAT, AbCip). ### Comms-loss detection: `GetHealth()` is load-bearing (Gitea #527, closed) @@ -315,9 +320,12 @@ non-`Healthy` state **and** its `LastSuccessfulRead` has been frozen for 90 s. T quality contract" and implemented by `Core.Abstractions.InputQuality` (`WorstOf`/`IsUsable`). Three things changed with it, all client-visible: - ⚠️ **Value nodes of a comms-lost driver now go Bad.** `DriverHostActor.FanCommsLossToValueNodes` - stamps `BadNoCommunication` on the raw NodeId **and every referencing UNS NodeId**, **value retained**, - via the new status-only sink primitive `IOpcUaAddressSpaceSink.WriteStatuses` (the host holds no value - cache, so only the node can keep the value). #481 had wired comms loss to the mux and to Part 9 + stamps `BadNoCommunication` on the raw NodeId **and every referencing UNS NodeId**, **value retained + server-side**, via the new status-only sink primitive `IOpcUaAddressSpaceSink.WriteStatuses` (the host + holds no value cache, so only the node can keep the value). ⚠️ Retention is **not client-observable**: + the SDK's `NodeState.ReadAttribute` nulls the value of any Bad-status read/sample, so clients see + `Value = null` + the Bad status; the retained value is what the next Good re-publish and the + `(value, status)` dedup compare against. #481 had wired comms loss to the mux and to Part 9 conditions and stopped one seam short of the external data plane. A consumer that treats Bad as "drop the tag" (ScadaBridge) will see more Bads — that is the fix working. - **Nothing Good is written on reconnect** — recovery is the driver's own re-publish (the poll engine's @@ -500,13 +508,16 @@ incident (`docs/deployments/wonder-app-vd03-makino-z-34184.md`). - **`RoleParser.GossipedRolesHave`** (case-insensitive) is defence in depth at the handful of production sites that read LIVE GOSSIP directly (`Member.Roles`) rather than through `EffectiveRoles` — a peer running an older, pre-R3-04 build can still gossip a miscased plain role - during a rolling upgrade. ⚠️ **Known residual, not closed by R3-04:** - `RedundancyStateActor.SelectOldestUpMemberOfRole`/`SelectDriverPrimary` delegate to - `ZB.MOM.WW.Health.Akka`'s `ClusterActiveNode.OldestUpMember`, which matches via Akka's OWN ordinal - `Member.HasRole` — not this repo's to patch, the same class of "not ours to change" carve-out as - `ClusterSingletonOptions.Role`. A pair where every member is on a fixed build can no longer gossip - a miscased role at all (normalization happens before gossip), so this only bites a genuinely mixed - fixed/pre-fix rolling-upgrade window. + during a rolling upgrade. ⚠️ **Accepted residual (#620), not closed by R3-04:** + `RedundancyStateActor`'s election delegates to `ZB.MOM.WW.Health.Akka`'s `ClusterActiveNode.OldestUpMember`, + which matches via Akka's OWN ordinal `Member.HasRole`. Deliberately **not** patched — neither in the + library nor with a local case-insensitive pre-filter, which would split the Primary election from + `ClusterSingletonManager` placement (also ordinal). It only bites a mixed fixed/pre-fix rolling-upgrade + window; finish the upgrade. Full rationale: `docs/Redundancy.md` ("Accepted residual (#620)"). +- **`Program.cs` wraps the composition root** (#618): a start-up failure logs Fatal, writes `FATAL:` to + stderr (the bootstrap logger is still silent before `AddZbSerilog`), flushes, and `Environment.Exit(1)`s + — leftover Akka/Kestrel threads would otherwise keep the process alive. `HostAbortedException` + (design-time tooling) passes through untouched. - **`RoleAuthorityGuardTests`** (`tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/`) is the enforcement: a source scan failing on any production file outside `EffectiveRoles.cs`/`RoleParser.cs` that reads `OTOPCUA_ROLES`, `Cluster:Roles` as code, or declares a bare `"admin"`/`"driver"`/`"dev"` literal — @@ -621,6 +632,9 @@ fail-closed). Things worth knowing before touching it: and the node keeps serving last-known-good. It never tears down to an empty address space. At boot a `FetchAndCache` node restores served state from the LocalDb pointer with **no** central-SQL read; an empty/unreadable cache lands Steady-with-no-revision (the first dispatch fetches), **never Stale**. + **Malformed non-empty bytes are "no answer" too** — on cache boot, the `FetchAndCache` boot reconcile and + the subscription push alike (the lenient swallow leg is closed). A cache boot from malformed bytes does + **not** adopt the revision (`Direct` ⇒ Stale; `FetchAndCache` ⇒ no revision, the re-dispatch fetches). - **Both pair nodes fetch independently** — no Primary gating of the fetch (same shape as both reading SQL today; `StoreAsync` is idempotent and the cache replicates). - **The dedicated h2c listener coexists with the LocalDb-sync listener.** A fused admin+driver node with @@ -681,7 +695,8 @@ touching it: - **Reconnect story:** central runs one reconnecting dialer per driver node (discovered from `ClusterNode.Host`/`GrpcPort`, refreshed every `TelemetryDial:ContactRefreshSeconds`), immediate first retry then ~5s fixed backoff, generation-stamped so a superseded stream's late error/event - is ignored, and it never permanently gives up (observability, not data plane). The node-local hub + is ignored, and it never permanently gives up (observability, not data plane). `TelemetryLocalHub.Evict` drops a removed driver's snapshots (`StopChild`, via `plan.ToStop`; central-side + stores are not evicted). The node-local hub replays last-value snapshots for `driver-health`/`driver-resilience-status` on every (re)subscribe so those stores re-prime immediately after a reconnect; `alerts`/`script-logs` are append logs and tolerate the gap. The `/alerts`/`/script-log` pill reflects aggregate stream health (live when ≥1 @@ -751,7 +766,8 @@ things are worth knowing: message size. Every comment and doc row that called it "row-count-only against gRPC's 4 MB cap" was corrected with the bump; do not restore that rationale. - **Registration FOLLOWS the replication config — it is no longer unconditional (2026-08-18).** - `LocalDbSetup.OnReady` reads one predicate, `LocalDbSetup.ReplicationIsConfigured`: + `LocalDbSetup.OnReady` reads one predicate, `LocalDbReplicationPolicy.CapturesChanges` + (via `LocalDbSetup.ReplicationIsConfigured`): **`LocalDb:Replication:PeerAddress` OR `LocalDb:Replication:ApiKey` is non-blank.** The **OR is not belt-and-braces** — only the dialling half of a pair sets `PeerAddress`, so keying on it alone would strip capture from every *passive* node (site-a-2 carries `ApiKey` + @@ -759,6 +775,10 @@ things are worth knowing: silently. ⚠️ **`SyncListenPort` deliberately does NOT join the OR:** the passive endpoint is fail-closed on the key, so a port without an `ApiKey` rejects every inbound stream and has nothing to capture for. + - ⚠️ **The alarm DRAIN gate uses a different predicate** — `LocalDbReplicationPolicy.SharesRowsWithPeer`: + `ApiKey` **and** (`PeerAddress` **or** `LocalDb:SyncListenPort > 0`), i.e. "a peer actually holds these + rows". Capture must be wider than the drain gate (a dialling-only node still captures); a node with no + peer must still drain, so the two predicates are deliberately not the same. - **Configured ⇒ `RegisterReplicated(table, baselineExistingRows: true)`**, per table, in the same load-bearing DDL→register order. Baselining is what makes enabling replication on a node that has been running *without* it actually converge: capture is CDC and the snapshot streamer @@ -952,6 +972,21 @@ Two things to know before relying on any of this: **Browse** (dropped when none — the builders' Messages name the node, so a narrowed copy gets a neutral one), an audit write only with `Read` on its source node plus the SDK's audit/encryption guard. Any other event type fails closed. Auditing is off by default (never configured in this repo). +- **ns=0 standard nodes stay ungated by design, even with `Security:Acl:Enabled`** (#617) — `ServerStatus`, + `NamespaceArray`, `ServiceLevel` etc. are served by the SDK's core node manager outside our authorizer and + clients read them to connect and to judge redundancy health; they carry no plant data. Every raw/UNS node, + event subscription and HistoryRead is still gated. `docs/security.md`. +- **Equipment-node writes: `EvaluateEquipmentWriteStructure` is the ONLY type check.** `OnWriteValue` bypasses + the SDK's own type check on every writable equipment node, so `OtOpcUaNodeManager.EvaluateEquipmentWriteStructure` + (plus `EvaluateEquipmentWriteRange`) is the whole gate. Scalars are deliberately lenient (drivers + range-check; ScadaBridge sends Double to a Float node). **Array writes** (#610): exact element-type match + (`Byte[]` ≈ ByteString) else `BadTypeMismatch` with nothing sent, wrong length ⇒ `BadTypeMismatch`, an array + to a scalar node is refused, `IndexRange` writes ⇒ `BadWriteNotSupported`, `ValueRank > 1` out of scope. An + array node is writable only when the driver can write the array (`DriverArrayWriteSupport`: Modbus bounded + by `maxRegistersPerWrite`/coil cap, S7 non-Bool/non-wide types, OpcUaClient, Galaxy signed types **with a + declared `arrayLength`** — an undeclared length stays read-only because a short whole-array write would + resize the Galaxy attribute); `arrayLength >= 1` is required, and the node's `Writable && (!IsArray || + ArrayWritable)` is projected onto the raw node and every UNS mirror. - **Anonymous is a normal grantable principal**, not a bypass. `OpcUa:AllowAnonymous` (default `true`, preserving today's behaviour) controls *admission*; when admitted the session authenticates as the reserved principal `anonymous` and gets roles from `NodeAcl` rows naming it, arriving at the gates as @@ -1043,11 +1078,24 @@ Refusals are not comms faults: an Unsupported refusal (`BadNotSupported`) and an mismatch** (`BadTypeMismatch`, where libplctag returned Good garbage for a same-width mis-authored type) both keep the tag handle, leave driver health alone (no `Degraded`, nothing for the comms-loss watchdog) and warn once per tag — only genuine transport failures evict and degrade. `DeviceSessionPool.Shared` logs through the -host logger factory (wired in `DriverFactoryBootstrap`). libplctag is confined to `Libplctag*.cs` adapters by +host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT factories' `Register(registry, ILoggerFactory?)` take one too, #602 CONV-3). The AbCip `browsePageDelayMs` knob has an AdminUI field (`AbCipDriverForm` "Browse page delay (ms)"). libplctag is confined to `Libplctag*.cs` adapters by `LibplctagConfinementGuardTests`; do not reference it elsewhere. See `docs/drivers/AbCip.md` §Transport, `docs/drivers/AbLegacy.md` §Transport + §Wire evidence, `docs/Configuration.md` §`Drivers:AbCip`, and `docs/plans/2026-10-07-native-ab-transport-gate-results.md`. +## Other driver notes + +- **S7:** connect attempts back off 1 s doubling to 30 s (one attempt per window, not one per call/poll/probe + tick); a write-side access-denied (PUT/GET disabled) latches a **sticky Faulted** that outranks later read + passes; driver-side array writes cover non-Bool narrow/`Int32`/`Float32` types only — Bool arrays are + refused (a one-byte-per-element encode would clobber neighbour bits). +- **MQTT:** Plain `writeTopic` writes publish at QoS 1 (Sparkplug NCMD/DCMD stay QoS 0); the Sparkplug + primary-host claim backs off 2 s → 30 s; writes encode the birth's own datatype (unsupported birth types + `BadNotSupported`). +- **FOCAS / AbLegacy:** background loops are joined (≤ 5 s) on shutdown. +- **Driver removal (#609):** removing a driver releases its capability-invoker pipelines and resilience + tracker, *then* evicts its telemetry snapshots. + ## Library Preferences - **Logging**: Serilog with rolling daily file sink @@ -1070,6 +1118,8 @@ dotnet run --project src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI -- read -u opc.tcp: dotnet run --project src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI -- subscribe -u opc.tcp://localhost:4840 -n "ns=2;s=SomeNode" -i 500 ``` +**AdminUI interactive pages must wrap their body in ``** — `MainLayout` is static, so it cannot catch an interactive page's render failure. + Address pickers in AdminUI support live browse for OpcUaClient and Galaxy drivers — see `docs/plans/2026-05-28-driver-browsers-design.md`. The AdminUI's global **UNS** page (`/uns`) is the single surface for managing the unified namespace fleet-wide (Area → Line → Equipment → Tag/VirtualTag), replacing the old per-cluster UNS/Equipment/Tags tabs. See `docs/Uns.md`. @@ -1095,11 +1145,12 @@ exactly. The backend lives in endpoints under `/api/script-analysis/*`, gated by the `ConfigEditor` policy (Administrator or Designer) via `RequireAuthorization` in `ScriptAnalysisEndpoints.MapScriptAnalysisEndpoints`). +`Core.VirtualTags` is now just `DependencyGraph`, `ITagUpstreamSource` and `IHistoryWriter` — the dormant `VirtualTagEngine`/Source/Definition and `TimerTriggerScheduler` were deleted. See `docs/ScriptEditor.md` for the full guide. Scripts may use the `{{equip}}` token for equipment-relative tag paths that resolve per-equipment at deploy time — see the "Equipment-relative tag paths" section in `docs/ScriptEditor.md`. ## Scripted Alarm Ack/Shelve -Inbound operator acknowledge/shelve for scripted alarms is fully implemented. Two surfaces converge on the `alarm-commands` DPS topic: (1) OPC UA Part 9 condition methods (Acknowledge / Confirm / AddComment / OneShotShelve / TimedShelve / Unshelve) wired in `OtOpcUaNodeManager`, gated on the `AlarmAck` LDAP role via `RoleCarryingUserIdentity`; (2) AdminUI `/alerts` per-row buttons routed through the `AdminOperationsActor` singleton (gated by `DriverOperator` policy). `ScriptedAlarmHostActor` dispatches from the topic to the engine. Client.CLI supports `ack`, `confirm`, `shelve` commands. See `docs/ScriptedAlarms.md` §"Inbound operator ack/shelve" and `docs/AlarmTracking.md`. +Inbound operator acknowledge/shelve for scripted alarms is fully implemented. Two surfaces converge on the `alarm-commands` DPS topic: (1) OPC UA Part 9 condition methods (Acknowledge / Confirm / AddComment / OneShotShelve / TimedShelve / Unshelve) wired in `OtOpcUaNodeManager`, gated on the `AlarmAck` role — conferred by the session's API-key principal's `NodeAcl` grants (or the `anonymous` principal's), carried by `RoleCarryingUserIdentity`; LDAP is AdminUI-only; (2) AdminUI `/alerts` per-row buttons routed through the `AdminOperationsActor` singleton (gated by `DriverOperator` policy). `ScriptedAlarmHostActor` dispatches from the topic to the engine. Client.CLI supports `ack`, `confirm`, `shelve` commands. See `docs/ScriptedAlarms.md` §"Inbound operator ack/shelve" and `docs/AlarmTracking.md`. ⚠️ **One condition event per accepted method call is OURS, not the SDK's (Gitea #587).** The SDK's own report is gated on `AutoReportStateChanges` (left `false`), so before #587 a client Acknowledge emitted **no** @@ -1116,6 +1167,15 @@ Confirmed/Unshelved constants own those fields); a native ack re-asserts `Confir SDK's ack unconfirms any confirm-capable condition and ScadaBridge maps that field. Pinned over the wire by `ConditionMethodEventTests` (exactly-once after a sentinel, folder + `i=2253`). +Behaviours that were wrong or missing and are now fixed — keep them: the SDK's `OnTimedUnshelve` is a +**no-op** (the engine's `AutoUnshelve` owns timed-shelve expiry, and `UnshelveTime` reports the real +remaining time from `AlarmConditionSnapshot.UnshelveAtUtc`); a **commented** re-ack is stored as an +`AddComment` (durable, replicated) and reports one event, an uncommented re-ack stays silent; a **native +ack is refused `BadInvalidState` off-Primary** via `NativeAlarmAckPrimaryGate` (fed by +`IDriverPrimaryRoleSource`; `DriverHostActor` publishes its Primary verdict at host start and on member +changes, so the boot window before the first redundancy snapshot is closed — the same fix covers the MQTT +Sparkplug primary-host claim). A native re-ack is still re-sent to the driver (the retry path, by design). + ## Historian / HistoryRead **Backend: HistorianGateway (sole historian backend).** As of the gateway-integration cutover, the @@ -1196,7 +1256,7 @@ only the recorder + outbox knobs: ### Tag auto-provisioning (`IHistorianProvisioning` EnsureTags hook) `AddressSpaceApplier.Apply()` fires a **non-blocking, fire-and-forget** `IHistorianProvisioning.EnsureTagsAsync` -hook for added historized value tags — the gateway-backed `GatewayTagProvisioner` calls the gateway's +hook for added historized value tags — and, since #600.1, for **changed** raw tags that flip to historized or change their resolved historian name or DataType — the gateway-backed `GatewayTagProvisioner` calls the gateway's `EnsureTags` so a brand-new historized tag exists in the historian before the recorder's `WriteLiveValues` land. The hook is wrapped so a faulted/synchronously-throwing provisioner can **never** block or fail a deploy. Non-numeric (`String`/`DateTime`/`Reference`) data types are skipped (not provisioned); the @@ -1263,7 +1323,7 @@ dotnet test --filter "Category=LiveIntegration" The live suite **skips cleanly** when these env vars are absent (safe to run offline on macOS). It is the gate the operator runs on the VPN before trusting the cutover. -### KNOWN LIMITATION 2 — continuous-historization value-capture wired; live end-to-end verification still pending (issue #491) +### Continuous-historization value capture — wired and live-gated (was "KNOWN LIMITATION 2"; #491 CLOSED 2026-08-04) The `ContinuousHistorizationRecorder` is fully wired (actor + FasterLog outbox + gateway value-writer + meters). It is spawned with an **empty *initial* ref set** (`Array.Empty()` in @@ -1331,6 +1391,7 @@ tag); evidence in the same plan doc. Two things to know before touching either.* (NOT alongside the alarm `IRedundancyRoleView`, which only exists when that section is enabled — the #511 registration accident). -Also re-confirmed live: **`EnsureTags` provisioning fires only for newly-ADDED historized tags** — -flipping an existing tag's `historianTagname` produces no provisioning at all -(`2026-06-27-historian-gateway-integration-issues.md` Issue 5). +Re-confirmed live in 2026-06 and **since fixed (#600.1)**: `EnsureTags` provisioning used to fire only for +newly-ADDED historized tags, so flipping an existing tag's `historianTagname` (or historization, or DataType) +provisioned nothing (`2026-06-27-historian-gateway-integration-issues.md` Issue 5). It now also fires for those +changed raw tags. diff --git a/docker-dev/docker-compose.yml b/docker-dev/docker-compose.yml index e1d5542f..395601a4 100644 --- a/docker-dev/docker-compose.yml +++ b/docker-dev/docker-compose.yml @@ -104,9 +104,11 @@ # mesh gRPC call on a login-enabled admin node) survived seven phases of prior live gates. To drive # this rig production-faithfully: # OTOPCUA_DISABLE_LOGIN=false docker compose -f docker-dev/docker-compose.yml up -d central-1 central-2 -# Sign in as `alice` / `password` (or `bob`) against the shared GLAuth (10.100.0.35:3893, -# dc=zb,dc=local) — the same LDAP users every other node in this file already authenticates OPC UA -# sessions against. Restore the default with `docker compose ... up -d central-1 central-2` (no env +# Sign in as `admin` / `password` (OtOpcUa-Admins, AdminUI Administrator) or `multi-role` / `password` +# (member of every group) against the shared GLAuth (10.100.0.35:3893, dc=zb,dc=local). The old +# `alice`/`bob` users no longer exist there. NOTE: the serviceaccount bind password below is the +# pre-rotation value (rotated out-of-band, SEC-36, scadaproj/infra/glauth/config.toml); a real-login run +# needs the current one supplied via the environment. The data plane no longer uses LDAP at all (API keys). Restore the default with `docker compose ... up -d central-1 central-2` (no env # var). See docs/security.md "Mesh gRPC ingress is AllowAnonymous by design" for what this actually # exercises. #