Joseph Doherty
549c656489
A deploy that triggers a FULL address-space rebuild clears `_alarmConditions` and re-materialises every condition node fresh — inactive, acked, confirmed, unshelved. The engine reloads its persisted state and re-derives Active from the predicate, so it ends up correct; but there is no transition to report, so `LoadAsync` yields `EmissionKind.None` and `OnEngineEmission` drops it. Nothing writes the node. Live-reproduced on the docker-dev rig against the pre-fix image: an alarm that had fired and cleared but was still UNACKNOWLEDGED came back from a rebuild reporting Acknowledged with Retain=false — so it disappeared from ConditionRefresh entirely, while the engine still held Unacknowledged. An operator's outstanding alarm silently vanishes from the alarm list on deploy. `ScriptedAlarmHostActor.ReassertConditionNodes` closes it. After each load it reads the new `ScriptedAlarmEngine.GetProjections()` and sends one `AlarmStateUpdate` per alarm NOT in the Part 9 no-event position. Load-bearing properties: - Node-only. It sends `AlarmStateUpdate` and nothing else — never the `alerts` topic, never the telemetry hub. `alerts` is the historization path, so a row per alarm per deploy would append a duplicate historian/AVEVA record every time anyone deploys. This is why it reads a projection rather than re-emitting: `ScriptedAlarmProjection` carries no `EmissionKind`, so there is nothing for the emission machinery — or a future refactor — to mistake for a transition. The issue's sketch said `GetState(alarmId)` + `LoadedAlarmIds`; that alone would have clobbered the node's severity and message, so the projection also carries the definition's severity, the resolved message template, and the last-observed worst-input quality. - Only non-normal alarms. One in the no-event position already matches what the materialise built, so a never-fired alarm stays byte-identical to a deploy without this pass (including its Message, which the materialise seeds with the display name), and an all-normal deploy writes nothing at all. - Timestamp is the condition's own `LastTransitionUtc`, not the deploy instant. - No duplicate Part 9 events: `WriteAlarmCondition`'s delta-gate compares against the node's CURRENT state, so a re-assert onto a freshly materialised node is a genuine delta (fires once — correct, the rebuild reset what clients see) while one onto a surgically-preserved node is suppressed. Tests: 6 new host tests + 6 new engine tests. Falsifiability checked by disabling the call — 4 of the 6 host tests go red; the other 2 are absence guards against an over-broad fix and pass either way by design. Live gate (docker-dev, central-2, pre-fix image vs. rebuilt image): - pre-fix: condition absent from ConditionRefresh after a rebuild; - post-fix: present, Unacknowledged, Retain=True, stamped with its own LastTransitionUtc rather than the restart instant; - /alerts stayed empty across two re-asserts, with the panel proven live by a real ACTIVATED transition immediately afterwards — no duplicate history.
124 lines
10 KiB
Markdown
124 lines
10 KiB
Markdown
# v3 Batch 4 — dual-namespace address space + raw-only runtime binding (= v3.0)
|
||
|
||
Implements `docs/plans/2026-07-15-v3-batch4-address-space-plan.md` (B4-WP1–WP5), executed via the
|
||
`/v3-batch 4` coordinator: contracts-first fan-out, worktree-isolated Opus agents per wave, a
|
||
code-reviewer pass + green build/test after each wave, then the non-negotiable **7-leg v3.0 live gate**
|
||
on docker-dev + Client.CLI. **This batch lights up the address space** — v3.0.
|
||
|
||
## What landed
|
||
|
||
- **Contracts** (`a1a56e22`) — two namespace URIs (`…/otopcua/raw`, `…/otopcua/uns`, replacing the single
|
||
`…/ns`), `V3NodeIds` (raw = `s=<RawPath>`, UNS = `s=<Area>/<Line>/<Equipment>/<EffectiveName>`), and the
|
||
`AddressSpaceRealm` (`Raw | Uns`) discriminator carried at every sink seam.
|
||
- **Wave A** (2 agents ∥):
|
||
- **WP1** — composer/planner un-darkened: emit the **Raw** subtree (Folder→Driver→Device→TagGroup→Tag,
|
||
tags as `(realm=Raw, s=RawPath)`) + the **UNS** subtree (each `UnsTagReference` a Variable
|
||
`(realm=Uns, s=Area/Line/Equipment/EffectiveName)` carrying its backing RawPath + an `Organizes`
|
||
UNS→Raw ref); native-alarm plans at the **raw** tag (ConditionId=RawPath) carrying referencing-equipment
|
||
paths; per-realm planner diff (raw rename = remove+add Raw + re-point UNS); reactivated the Batch-1-skipped
|
||
`EquipmentNamespaceMaterializationTests`.
|
||
- **WP2** — node manager registers both namespaces; `AddressSpaceRealm` on every node-naming sink method
|
||
(maps keyed by ns-qualified NodeId so a Raw + UNS node sharing a bare `s=` id stay distinct; historized
|
||
UNS node registers the **same** tagname, mux single by RawPath); everything **forwarded through
|
||
`DeferredAddressSpaceSink`** + reflection guard extended (the forwarding trap). **M1 fix**: new
|
||
`AddReference` sink seam wiring the `Organizes` UNS→Raw edge bidirectionally (idempotent, missing-endpoint
|
||
no-op, forwarded + reflection-covered).
|
||
- **Wave B** (1 agent — the delicate one):
|
||
- **WP3** — applier materializes both realms + wires the Organizes edge; `DriverHostActor` dual-NodeId
|
||
registration + single-source fan-out (drift-free — one publish → raw NodeId AND every referencing UNS
|
||
NodeId, identical value/quality/timestamp); write routing + failed-write revert through both NodeIds;
|
||
`EquipmentNodeIds` retired; transitional realm defaults removed from the sink surface. **Review fixes**:
|
||
**H1** realm-qualified `(realm, bareId)` write-routing key (a raw RawPath that coincidentally equals a UNS
|
||
path can no longer misroute a write); **M1** coherent-dormant discovery-injection guard; **M2** dual-node
|
||
self-correction tests; **L1** node-manager routing defaults removed (~180 call sites); byte-parity
|
||
round-trip test.
|
||
- **Wave C** (2 agents ∥):
|
||
- **WP4** — multi-notifier native alarms: one condition at the raw tag, `AddNotifier`-fanned to the raw
|
||
device folder + every referencing equipment folder, **one `ReportEvent`** (exactly one Server-object copy —
|
||
proven by an over-the-wire event-delivery test), teardown symmetry (`RemoveNotifier(bidirectional:true)` on
|
||
rebuild/condition-removal/subtree-removal + reconcile), ack/confirm/shelve still on ConditionId=RawPath;
|
||
`AlarmTransitionEvent` + `/alerts` gain the referencing-equipment list. **Review fixes**: M1 event-delivery
|
||
test, M2 resolution-failure meter + composer↔applier path-agreement test, M3 reconcile, L1/L3.
|
||
- **WP5** — over-the-wire dual-namespace integration tests (`DualNamespaceAddressSpaceTests`) + 2-node
|
||
harness materialization round-trip; docs (`CLAUDE.md`, `docs/Uns.md`, `docs/ScriptEditor.md`,
|
||
`docs/Historian.md`, `docs/ScriptedAlarms.md`, `docs/AlarmTracking.md`).
|
||
- **Coordinator seam-fix** — threaded `ReferencingEquipmentPaths` into `DriverHostActor.ForwardNativeAlarm`'s
|
||
`AlarmTransitionEvent` so `/alerts` chips populate in production (WP3-owned file WP4 couldn't edit).
|
||
- **Live-gate bug fix** — the gate caught a genuine prod-inert defect: **Equipment VirtualTags never resolved
|
||
live** (a redeploy resets the VT's UNS node to `BadWaitingForInitialData`, but a surviving unchanged-plan
|
||
`VirtualTagActor` keeps its value-dedup state and suppresses the re-publish → the reset node stays Bad
|
||
forever with a static dependency). Fix: `ReassertValue` on apply re-emits the last value after the node
|
||
reset. Regression test fail-before/pass-after; live-verified Good. (Reviewer follow-ups: reassert skips
|
||
historian re-record (M1); scripted-alarm redeploy recovery (M2); comment (LOW-3).)
|
||
|
||
## Reviewer verdict per wave
|
||
|
||
No HIGH survived any wave. Wave A: forwarding trap verified solid (DispatchProxy guard catches unforwarded
|
||
methods), collision-free ns-qualified keying, planner raw-rename-repoints-UNS holds; M1 (Organizes seam) fixed.
|
||
Wave B: **H1** write-route realm collision fixed with a regression test; M1/M2/L1 fixed. Wave C: single-ReportEvent
|
||
+ teardown symmetry + forwarding trap all confirmed; M1/M2/M3/L1/L3 fixed. VT-fix review: ordering confirmed real,
|
||
dedup/02-S13 semantics intact; M1/M2/LOW-3 closed.
|
||
|
||
## v3.0 live gate (docker-dev `:9200` / `opc.tcp://:4840`+`:4841`, both centrals rebuilt on Batch-4 code)
|
||
|
||
1. **Browse both namespaces** — `ns=2` Raw tree (`pymodbus/plc/HR200`, `s=<RawPath>`) + `ns=3` UNS tree
|
||
(`gatearea/gateline/gateequip/MainPressure`, effective names); both materialize `failed=0`.
|
||
2. **Single-source fan-out** — `MainPressure` (UNS) = `HR200` (Raw) = **1234**, identical value AND timestamp;
|
||
Calculation tag `Cval` computes (=1235); the `{{equip}}/MainPressure` VirtualTag reads Good (post-fix).
|
||
3. **Writes** — write **4242** via the UNS NodeId → read-back **4242** via the raw NodeId; write **777** via the
|
||
raw NodeId; `opc-readonly` rejected **0x801F0000** on the UNS NodeId (role gating symmetric). Failed-write
|
||
dual-node revert is unit-covered (`exception_injector` fixture not provisioned on this rig).
|
||
4. **History** — HistoryRead via the raw NodeId and the UNS NodeId both return **GoodNoData** under the same
|
||
historian tagname (Null source — no gateway on docker-dev); provisioning tally `dispatched=1 … failed=0`.
|
||
5. **Alarms (multi-notifier)** — the native alarm materialized as a Part 9 **AlarmCondition** at the raw tag
|
||
(ConditionId = RawPath); the **equipment folder** accepts an alarm subscription + ConditionRefresh, proving
|
||
it is a live event-notifier wired to the raw condition; the Server object likewise. The transition delivery
|
||
(exactly-one Server copy + delivery at both notifiers + ack) is covered by the over-the-wire
|
||
`NativeAlarmMultiNotifierEventDeliveryTests` — docker-dev has no `IAlarmSource` driver (only Galaxy is one)
|
||
so a live transition can't be tripped on the rig.
|
||
6. **Rename cascade** — renamed `HR200`→`HR200X` + deployed: old raw NodeId gone, new present; the UNS
|
||
reference `MainPressure` follows automatically; the `{{equip}}` VirtualTag keeps working and tracks the
|
||
renamed tag live (3610→3611 Good); the **absolute** RawPath script literal (`Cval`) now fails **Bad
|
||
0x80000000**. (Caveat: a raw-tag rename needs a node recreate/restart to hot-rebind the renamed tag's live
|
||
driver value — the pre-existing deploy-THEN-recreate pattern for structural edits.)
|
||
7. **Redundancy** — ServiceLevel split **central-1 = 250 / central-2 = 240** (both Good) — the Primary/Secondary
|
||
differentiation is intact with the second namespace present; single-emit-per-condition is unit-covered (the
|
||
alarm-emit gate is Primary-only).
|
||
|
||
## Tests
|
||
|
||
Full solution builds **0 errors**. Commons 306, Configuration 121, OpcUaServer 375/4-skip (+ OpcUaServer.IntegrationTests
|
||
dual-namespace + event-delivery), Runtime 377 (+ VT reassert regression), AdminUI 659, Host.IntegrationTests green
|
||
(the pre-existing Batch-2 `Calculation`-probe stale test fixed here). The remaining Runtime skips are legacy
|
||
`EquipmentTags`-model dark tests + the dormant discovery-injection scenarios (accurate skip reasons).
|
||
|
||
## Documented follow-ups (non-blocking)
|
||
|
||
- **Scripted-alarm redeploy recovery — FIXED (issue #487), see `docs/ScriptedAlarms.md` §"Post-(re)load
|
||
condition-node re-assert".** Landed as the proposed shape below: `ScriptedAlarmHostActor.ReassertConditionNodes`
|
||
in `OnAlarmsLoaded`, node-only, never the `alerts` topic. Two deviations from the sketch, both deliberate:
|
||
it reads a new `ScriptedAlarmEngine.GetProjections()` rather than `GetState(alarmId)` + `LoadedAlarmIds`
|
||
(a projection also carries the definition's severity, the resolved message template, and the last-observed
|
||
worst-input quality — `GetState` alone would have clobbered severity/message on the node); and it skips
|
||
alarms sitting in the Part 9 no-event position, so a deploy where nothing is in alarm writes no condition
|
||
nodes at all. The original deferral text follows for context.
|
||
The scripted-alarm Part 9 condition node has the same redeploy-reset race the VT `ReassertValue` fix closed:
|
||
`RebuildAddressSpace` clears `_alarmConditions` and `MaterialiseScriptedAlarms` recreates each condition
|
||
fresh/normal, but `ScriptedAlarmEngine.LoadAsync` reloads from the persisted state and yields
|
||
`EmissionKind.None` for a still-active condition (dropped by `OnEngineEmission`), so an **active scripted
|
||
alarm with static dependencies under-reports as normal after a full-rebuild deploy until its next real
|
||
transition** (the known "snapshot-is-one-shot / restart-to-re-deliver" gotcha — NOT a Batch-4 regression).
|
||
Deferred because the fix is a Core-engine emission-contract change carrying **double-alarm-history risk**
|
||
(a naive re-emit would append a duplicate AVEVA/historian row per deploy — the alarm analogue of the VT M1
|
||
historian issue). Proposed shape: a host-driven, node-only `AlarmStateUpdate` re-assert in `OnAlarmsLoaded`
|
||
(same safe post-materialise ordering as the VT fix), reusing `_engine.GetState(alarmId)` + `LoadedAlarmIds`,
|
||
**never touching the `alerts` topic**.
|
||
- Raw-tag rename hot-rebind of the renamed tag's live value without a recreate (today: deploy-THEN-recreate). Filed as issue #489.
|
||
- Absolute-path Monaco completion → RawPath (from Batch 3; the `{{equip}}/` completion works). Filed as issue #490.
|
||
- Cross-repo (post-merge): ScadaBridge Data-Connection-Layer NodeId/namespace cutover (raw `s=<RawPath>` /
|
||
UNS `s=<Area>/<Line>/<Equipment>/<EffectiveName>`, retired `EquipmentNodeIds`) — tracked in the
|
||
ScadaBridge repo as its issue #14 (its #20 landed the `nsu=` binding + native-alarm routing). The umbrella
|
||
index `../scadaproj/CLAUDE.md` OtOpcUa entry is **DONE** (scadaproj `ede5275`).
|
||
|
||
https://claude.ai/code/session_01LVneM3eh1UtJxEisFXgmox
|