fix(address-space): no UNS value Variable for a reference to an alarm-bearing tag (#896)

A UnsTagReference to a raw tag that carries an "alarm" object produced a UNS value Variable that read BadWaitingForInitialData forever (G6 live gate, TestAlarm002Condition on testmachine-015). BuildUnsReferenceVariables now skips those references; the equipment gets the alarm only as the raw condition wired as an event notifier of its folder, which is unchanged (the reference still feeds RawTagPlan.ReferencingEquipmentPaths). Why condition-only and not "feed the mirror": - The raw side has no value node for such a tag: AddressSpaceApplier materialises an alarm tag as a condition OR a value variable, never both. There is no raw value for the UNS mirror to mirror. - DriverHostActor skips alarm tags when it builds the value fan-out and write routing maps (`continue` before the UNS registration), and the driver leaves them out of its value subscription ("subscribed to 5 refs" with 6 tags in G6). Feeding the mirror would mean subscribing a value the design says the tag does not have. - Docs already state condition-only: ScriptedAlarms.md ("instead of a value variable"), AlarmTracking.md ("only the condition, with no sibling value variable"), the v3 design (native alarm = one condition at the raw tag, surfaced at equipment via notifiers). - The composer tests that asserted the mirror were incidental: f4f3e17e (v3 B4-WP1) put the alarm on the fixture's Speed tag to test raw alarm attachment, and later tests reused Speed for writable / historian / array checks of the mirror. No commit or doc chose a value mirror for alarm tags. - ScadaBridge does not read it: its OtOpcUa alarm path is event subscriptions on the equipment/raw folders (v3 alarm B/C live gate), and a node that never had a value cannot be a working binding. G6 itself authored the alarm's Boolean as a separate value tag (TestAlarm002), which is the supported way to read it. Tests: the composer fixture moves the alarm onto its own HighTemp tag (Speed is now a plain historized value tag), with a new test asserting no UNS Variable for the alarm reference, with positive controls (the raw tag still carries the equipment path; the value references still emit). New real-SDK test Alarm_bearing_reference_surfaces_as_the_condition_only_against_real_SDK: no Variable at the UNS path, the Speed mirror present, one condition, the equipment folder wired as its notifier. Both new tests fail with the skip removed. Docs: Uns.md, ScriptedAlarms.md, G6 evidence note. Claude-Session: https://claude.ai/code/session_01GT67ACmpMR7b9LGRPLijMi
2026-11-12 23:55:49 -05:00
parent d9b2381235
commit 09b373e4db
6 changed files with 147 additions and 14 deletions
@@ -240,7 +240,9 @@ blob and is parsed byte-parity in both the compose (`AddressSpaceComposer`) and
 > never one-per-root. Teardown is symmetric (`RemoveNotifier(..., bidirectional:true)` on
 > rebuild / reference removal) so inverse-notifier entries never leak across redeploys.
 > Ack/confirm/shelve route on **`ConditionId = RawPath`** (never `SourceNodeId`), and the
-> `/alerts` row lists the referencing equipment paths. See
+> `/alerts` row lists the referencing equipment paths. The equipment gets the condition
+> **only**: no UNS value variable is created for a reference to an alarm-bearing tag (#896),
+> matching the raw side, which has no value variable either. See
 > [AlarmTracking.md](AlarmTracking.md) and the WP4 section of
 > `docs/plans/2026-07-15-v3-batch4-address-space-plan.md`.

@@ -169,6 +169,12 @@ source-timestamp — the two NodeIds never drift.
 - **Native alarms** materialize once at the raw tag (`ConditionId = RawPath`) and
  fan via SDK notifiers to the raw device folder AND every referencing equipment
  folder — see [`ScriptedAlarms.md`](ScriptedAlarms.md) / [`AlarmTracking.md`](AlarmTracking.md).
+  A reference to an **alarm-bearing** raw tag therefore creates **no UNS variable**
+  (#896): the raw side has only the condition and no value to fan, so the
+  reference's only effect is to wire the condition as an event notifier of the
+  equipment folder. A client reads the alarm's state from the condition's fields
+  (`ActiveState`, `AckedState`, …) at either folder, not from a value variable. The
+  reference still counts for effective-name uniqueness.

 Note the two distinct identity strings: the wire **NodeId** is the path
 `Area/Line/Equipment/EffectiveName`, while the **effective-name uniqueness key**
@@ -237,9 +237,11 @@ cleanly with it.
  never went active, and its ack would dial a reference the gateway refuses. Three unit tests were added to
  `GalaxyDriverRawPathKeyingTests`. The main one fails with the fix reverted, which was checked by reverting
  the fix and re-running it.
- **#896 (open).** The UNS mirror of an alarm-bearing raw tag gets a value Variable that stays
+- **#896 (fixed after this run).** The UNS mirror of an alarm-bearing raw tag gets a value Variable that stays
  `BadWaitingForInitialData` forever, against the documented "condition instead of value variable" contract.
  It was not fixed here because the composer test fixtures assert that mirror.
+  Fixed afterwards: the composer no longer emits a UNS Variable for a reference to an alarm-bearing tag, so the
+  equipment shows only the condition (the `<- #896` Variable in the Step 1 browse is gone).

 ## Cleanup

@@ -921,7 +921,8 @@ public static class AddressSpaceComposer
    ///     equipment path resolve, keyed <c>(realm=Uns, s=Area/Line/Equipment/EffectiveName)</c> and carrying
    ///     the backing RawPath (Organizes target + fan-out) plus the inherited DataType / writable. A
    ///     reference whose backing tag is unknown, whose RawPath is unresolvable, or whose UNS path segments
-    ///     are invalid is SKIPPED. Sorted by NodeId ordinal.
+    ///     are invalid is SKIPPED. A reference to an ALARM-BEARING tag is skipped too (#896): it surfaces at the
+    ///     equipment as the raw condition's event notifier, not as a value Variable. Sorted by NodeId ordinal.
    /// </summary>
    private static IReadOnlyList<UnsReferenceVariable> BuildUnsReferenceVariables(
        IReadOnlyList<UnsTagReference>? unsTagReferences,
@@ -965,6 +966,14 @@ public static class AddressSpaceComposer
            // doc-comment; 2026-08-15 surgical-deploys review). Parsed from the SAME TagConfigIntent the raw
            // tag plan uses, so the two sides can never disagree.
            var intent = TagConfigIntent.Parse(tag.TagConfig);
+            // #896: an alarm-bearing backing tag gets NO UNS value Variable. Its raw side is a Part 9 condition
+            // with no sibling value variable (AddressSpaceApplier.MaterialiseRawSubtree), the driver host keeps
+            // it out of the value fan-out and the value subscription (DriverHostActor skips Alarm tags), so a
+            // mirror Variable here could never receive a value and read BadWaitingForInitialData forever. The
+            // reference still reaches the equipment: it puts the equipment's path on the raw tag's
+            // ReferencingEquipmentPaths, and the applier wires the single condition as an event notifier of that
+            // equipment folder. Same TagConfigIntent the raw pass reads, so the two sides cannot disagree.
+            if (intent.Alarm is not null) continue;
            // Same derivation as BuildRawSubtree's ArrayWritable, off the same row, so a mirror is writable
            // exactly when its raw node is.
            var backingArrayWritable = intent.IsArray && arrayWritableFor(tag);
@@ -236,6 +236,84 @@ public sealed class AddressSpaceApplierHierarchyTests : IDisposable
        sdkServer.NodeManager!.VariableCount.ShouldBe(2);
    }

+    /// <summary>
+    ///     Gitea #896, against a real SDK node manager: an equipment that references a value tag AND an alarm-bearing
+    ///     tag gets a UNS Variable for the value tag only. The alarm tag is the single Part 9 condition at its RawPath,
+    ///     wired as an event notifier of the equipment folder, and no UNS Variable is created at
+    ///     <c>filling/line-1/station-1/HighTemp</c> (on the G6 live gate that Variable existed and read
+    ///     <c>BadWaitingForInitialData</c> forever, because nothing ever feeds it). The value-tag mirror and the
+    ///     condition's equipment notifier are the positive controls.
+    /// </summary>
+    [Fact]
+    public async Task Alarm_bearing_reference_surfaces_as_the_condition_only_against_real_SDK()
+    {
+        var applicationUri = $"urn:OtOpcUa.AlarmRefE2e:{Guid.NewGuid():N}";
+        var started = await OpcUaHostStart.StartAsync(
+            port => new OpcUaApplicationHost(
+                new OpcUaApplicationHostOptions
+                {
+                    ApplicationName = "OtOpcUa.AlarmRefE2e",
+                    ApplicationUri = applicationUri,
+                    OpcUaPort = port,
+                    PublicHostname = "localhost",
+                    PkiStoreRoot = _pkiRoot,
+                },
+                NullLogger<OpcUaApplicationHost>.Instance),
+            () => new OtOpcUaSdkServer(), Ct);
+        await using var host = started.Host;
+        var sdkServer = started.Server;
+        sdkServer.NodeManager.ShouldNotBeNull();
+        var nm = sdkServer.NodeManager!;
+
+        var folder = new RawFolder { RawFolderId = "rf-plant", ClusterId = "c1", Name = "Plant", ParentRawFolderId = null };
+        var driver = new DriverInstance { DriverInstanceId = "drv-modbus", ClusterId = "c1", Name = "Modbus", DriverType = "Modbus", DriverConfig = "{}", RawFolderId = "rf-plant" };
+        var device = new Device { DeviceId = "dev-1", DriverInstanceId = "drv-modbus", Name = "dev1", DeviceConfig = "{}" };
+        var speed = new Tag
+        {
+            TagId = "tag-speed", DeviceId = "dev-1", Name = "Speed", DataType = "Float",
+            AccessLevel = TagAccessLevel.Read, TagConfig = "{}",
+        };
+        var highTemp = new Tag
+        {
+            TagId = "tag-hightemp", DeviceId = "dev-1", Name = "HighTemp", DataType = "Boolean",
+            AccessLevel = TagAccessLevel.Read,
+            TagConfig = "{\"alarm\":{\"alarmType\":\"OffNormalAlarm\",\"severity\":700}}",
+        };
+        var area = new UnsArea { UnsAreaId = "nw-area-filling", ClusterId = "c1", Name = "filling" };
+        var line = new UnsLine { UnsLineId = "nw-line-1", UnsAreaId = "nw-area-filling", Name = "line-1" };
+        var equipment = new Equipment { EquipmentId = "eq-1", UnsLineId = "nw-line-1", Name = "station-1", MachineCode = "STATION_001" };
+
+        var composition = AddressSpaceComposer.Compose(
+            new[] { area }, new[] { line }, new[] { equipment }, new[] { driver },
+            Array.Empty<ScriptedAlarm>(),
+            unsTagReferences: new[]
+            {
+                new UnsTagReference { UnsTagReferenceId = "ref-speed", EquipmentId = "eq-1", TagId = "tag-speed" },
+                new UnsTagReference { UnsTagReferenceId = "ref-hightemp", EquipmentId = "eq-1", TagId = "tag-hightemp" },
+            },
+            tags: new[] { speed, highTemp },
+            rawFolders: new[] { folder }, devices: new[] { device });
+
+        var applier = new AddressSpaceApplier(new SdkAddressSpaceSink(nm), NullLogger<AddressSpaceApplier>.Instance);
+        applier.MaterialiseHierarchy(composition);
+        applier.MaterialiseRawSubtree(composition);
+        applier.MaterialiseUnsReferences(composition);
+
+        // No UNS Variable for the alarm-bearing reference, and none at the raw RawPath either.
+        nm.TryGetVariable("filling/line-1/station-1/HighTemp", AddressSpaceRealm.Uns).ShouldBeNull();
+        nm.TryGetVariable("Plant/Modbus/dev1/HighTemp", AddressSpaceRealm.Raw).ShouldBeNull();
+
+        // Positive control 1: the value tag still has both its raw node and its UNS mirror (and nothing else).
+        nm.TryGetVariable("Plant/Modbus/dev1/Speed", AddressSpaceRealm.Raw).ShouldNotBeNull();
+        nm.TryGetVariable("filling/line-1/station-1/Speed", AddressSpaceRealm.Uns).ShouldNotBeNull();
+        nm.VariableCount.ShouldBe(2);
+
+        // Positive control 2: the alarm reaches the equipment as the single raw condition wired to its folder.
+        nm.TryGetAlarmCondition("Plant/Modbus/dev1/HighTemp", AddressSpaceRealm.Raw).ShouldNotBeNull();
+        nm.AlarmConditionCount.ShouldBe(1);
+        nm.FolderNotifierCount("eq-1", AddressSpaceRealm.Uns).ShouldBe(1);
+    }
+
    /// <summary>OpcUaServer-001 — a UNS Area / Line rename-only deploy refreshes the EXISTING folder's
    /// DisplayName IN PLACE against a real SDK node manager (no rebuild, no node count change). Proves the
    /// full chain: planner emits a RenamedFolders delta → applier drives the surgical
@@ -17,7 +17,8 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests;
 public sealed class AddressSpaceComposerDualNamespaceTests
 {
    // A raw chain: RawFolder "Plant" → Driver "Modbus1" → Device "PLC-A" → Group "Motors" → tags.
-    // Speed sits under the group; Status directly under the device; Levels is an array tag.
+    // Speed sits under the group; Status directly under the device; Levels is an array tag; HighTemp is a
+    // native-alarm tag under the group. Speed, Status and HighTemp are referenced into equipment station-1.
    private static AddressSpaceComposition ComposeFullFixture()
    {
        var folder = new RawFolder { RawFolderId = "raw-plant", ClusterId = "c1", Name = "Plant" };
@@ -33,8 +34,13 @@ public sealed class AddressSpaceComposerDualNamespaceTests
        {
            TagId = "t-speed", DeviceId = "dev-1", TagGroupId = "g1", Name = "Speed",
            DataType = "Float", AccessLevel = TagAccessLevel.ReadWrite,
-            TagConfig = "{\"isHistorized\":true,\"historianTagname\":\"speed_hist\"," +
-                        "\"alarm\":{\"alarmType\":\"LimitAlarm\",\"severity\":700}}",
+            TagConfig = "{\"isHistorized\":true,\"historianTagname\":\"speed_hist\"}",
+        };
+        var highTemp = new Tag
+        {
+            TagId = "t-hightemp", DeviceId = "dev-1", TagGroupId = "g1", Name = "HighTemp",
+            DataType = "Boolean", AccessLevel = TagAccessLevel.Read,
+            TagConfig = "{\"alarm\":{\"alarmType\":\"LimitAlarm\",\"severity\":700}}",
        };
        var status = new Tag
        {
@@ -53,6 +59,7 @@ public sealed class AddressSpaceComposerDualNamespaceTests
        var equip = new Equipment { EquipmentId = "eq-1", UnsLineId = "line-1", Name = "station-1", MachineCode = "STATION_001" };

        var refSpeed = new UnsTagReference { UnsTagReferenceId = "ref-speed", EquipmentId = "eq-1", TagId = "t-speed" };
+        var refHighTemp = new UnsTagReference { UnsTagReferenceId = "ref-hightemp", EquipmentId = "eq-1", TagId = "t-hightemp" };
        var refStatus = new UnsTagReference
        {
            UnsTagReferenceId = "ref-status", EquipmentId = "eq-1", TagId = "t-status",
@@ -62,8 +69,8 @@ public sealed class AddressSpaceComposerDualNamespaceTests
        return AddressSpaceComposer.Compose(
            new[] { area }, new[] { line }, new[] { equip },
            new[] { driver }, Array.Empty<ScriptedAlarm>(),
-            unsTagReferences: new[] { refSpeed, refStatus },
-            tags: new[] { speed, status, levels },
+            unsTagReferences: new[] { refSpeed, refStatus, refHighTemp },
+            tags: new[] { speed, status, levels, highTemp },
            rawFolders: new[] { folder },
            devices: new[] { device },
            tagGroups: new[] { group });
@@ -136,11 +143,12 @@ public sealed class AddressSpaceComposerDualNamespaceTests
    {
        var c = ComposeFullFixture();

-        var speed = c.RawTags.Single(t => t.TagId == "t-speed");
-        speed.Alarm.ShouldNotBeNull();
-        speed.Alarm!.AlarmType.ShouldBe("LimitAlarm");
-        speed.Alarm.Severity.ShouldBe(700);
-        speed.ReferencingEquipmentPaths.ShouldBe(new[] { "filling/line-1/station-1" });
+        var highTemp = c.RawTags.Single(t => t.TagId == "t-hightemp");
+        highTemp.NodeId.ShouldBe("Plant/Modbus1/PLC-A/Motors/HighTemp"); // ConditionId = RawPath
+        highTemp.Alarm.ShouldNotBeNull();
+        highTemp.Alarm!.AlarmType.ShouldBe("LimitAlarm");
+        highTemp.Alarm.Severity.ShouldBe(700);
+        highTemp.ReferencingEquipmentPaths.ShouldBe(new[] { "filling/line-1/station-1" });

        // A non-alarm tag carries no condition + its own referencing-equipment list.
        var status = c.RawTags.Single(t => t.TagId == "t-status");
@@ -152,7 +160,35 @@ public sealed class AddressSpaceComposerDualNamespaceTests
        levels.ReferencingEquipmentPaths.ShouldBeEmpty();
    }

-    /// <summary>Each UnsTagReference emits a UNS Variable keyed
+    /// <summary>
+    ///     Gitea #896: a reference to an ALARM-BEARING raw tag emits NO UNS value Variable. The raw side of such a
+    ///     tag is a Part 9 condition with no sibling value variable, and the driver host keeps it out of the value
+    ///     fan-out, so a mirror Variable could never receive a value (it read <c>BadWaitingForInitialData</c>
+    ///     forever on the G6 live gate). The equipment still gets the alarm: the reference puts the equipment's
+    ///     path on the raw tag's <c>ReferencingEquipmentPaths</c>, which is what wires the condition as a notifier
+    ///     of the equipment folder. Positive controls: that path is present, and the value references in the same
+    ///     equipment still emit their Variables.
+    /// </summary>
+    [Fact]
+    public void A_reference_to_an_alarm_bearing_tag_emits_no_uns_variable_but_still_reaches_the_equipment()
+    {
+        var c = ComposeFullFixture();
+
+        c.UnsReferenceVariables.ShouldNotContain(v => v.UnsTagReferenceId == "ref-hightemp");
+        c.UnsReferenceVariables.ShouldNotContain(v => v.BackingRawPath == "Plant/Modbus1/PLC-A/Motors/HighTemp");
+        c.UnsReferenceVariables.ShouldNotContain(v => v.NodeId == "filling/line-1/station-1/HighTemp");
+
+        // Positive control 1: the condition still reaches station-1 through the raw tag's notifier fan-out.
+        var highTemp = c.RawTags.Single(t => t.TagId == "t-hightemp");
+        highTemp.Alarm.ShouldNotBeNull();
+        highTemp.ReferencingEquipmentPaths.ShouldBe(new[] { "filling/line-1/station-1" });
+
+        // Positive control 2: the value references in the same equipment still get their Variables.
+        c.UnsReferenceVariables.Select(v => v.UnsTagReferenceId).OrderBy(x => x, StringComparer.Ordinal)
+            .ShouldBe(new[] { "ref-speed", "ref-status" });
+    }
+
+    /// <summary>Each value-tag UnsTagReference emits a UNS Variable keyed
    /// <c>s=&lt;Area&gt;/&lt;Line&gt;/&lt;Equipment&gt;/&lt;EffectiveName&gt;</c> (realm=Uns), carrying its backing
    /// RawPath (the Organizes target + fan-out) and inheriting DataType + writable from the raw tag. Effective
    /// name = DisplayNameOverride else the backing raw tag's Name.</summary>