fix(abcip): ALMA review round 2 - IsRestatement contract docs, ALMD no-InAlarm warning, un-ack retrigger

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe

CLAUDE.md

@@ -1139,10 +1139,11 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa
   condition (no prior told), Galaxy for an unacked active snapshot entry. **Those two also flag
   `AlarmEventArgs.IsRestatement` (#656)** — history-only: a flagged Retrigger on an Active+unacked condition with the
   same message AND severity, or a flagged Clear on an inactive one, writes no `/alerts` row (every Galaxy reconnect used
-  to duplicate them). Active+acked is the atomic **`AlarmTransitionKind.ActiveAcknowledged`** (Galaxy snapshot
+  to duplicate them); a flagged `Unspecified` on an already-active condition (AbCip ALMA governing-limit drop-out)
+  moves the node's text/severity, keeps active/acked and writes no row. Active+acked is the atomic **`AlarmTransitionKind.ActiveAcknowledged`** (Galaxy snapshot
   `ActiveAcked`, OpcUaClient inactive→acked-active) — never Acknowledge (kept a cold host's `Active=false`: the alarm was
   invisible) nor Raise+Acknowledge (an unacked intermediate). `NativeAlarmProjector` returns the rows to write; unflagged
-  sources (FOCAS/MTConnect/AbCip, live Galaxy) are unchanged. E2E: `Runtime.Tests/NativeAlarmRestatementEndToEndTests`.
+  sources (FOCAS/MTConnect, live Galaxy) are unchanged. E2E: `Runtime.Tests/NativeAlarmRestatementEndToEndTests`.
 - **Re-Initialize without teardown (#630):** FOCAS, AbLegacy, AbCip, TwinCAT, S7 and OpcUaClient `InitializeAsync`
   first release the prior device/session state (cancel, ≤ 5 s join, dispose — incl. the AbCip alarm projection;
   FOCAS's is PAUSED, its told record kept, #647) — the device-state half of `ShutdownAsync` only; the poll engine is kept and the actor re-subscribes

docs/AlarmTracking.md

@@ -69,10 +69,12 @@ same condition is wired as an **event notifier of every referencing equipment fo
   `Unspecified` kind projects no state change.
 - **Snapshot / first-sight re-statements (#656).** A source that RE-STATES upstream's current state — Galaxy's
   reconnect snapshot entries, OpcUaClient's first event per condition after a start or deploy respawn — flags
-  `AlarmEventArgs.IsRestatement` (optional, default `false`; FOCAS, MTConnect, AbCip and live Galaxy transitions never
-  set it). The flag governs **history only, never node state**: the projector still projects, but a flagged `Retrigger`
+  `AlarmEventArgs.IsRestatement` (optional, default `false`; FOCAS, MTConnect and live Galaxy transitions never
+  set it; AbCip ALMA sets it for the drop-out update below). The flag governs **history only, never node state**: the projector still projects, but a flagged `Retrigger`
   on a condition already Active+unacked with the **same message AND severity**, or a flagged `Clear` on an
-  already-inactive condition, writes no `/alerts` row. Before this every gateway reconnect wrote a duplicate
+  already-inactive condition, writes no `/alerts` row; so does a flagged `Unspecified` update on an already-active
+  condition, which moves the node's message/severity (one condition event, active/acked kept) — AbCip ALMA's
+  governing limit dropping out (HH→H), and OpcUaClient's first sight of an inactive condition after a respawn. Before this every gateway reconnect wrote a duplicate
   "Activated" (and "Cleared") row per alarm while clients saw no event. A **live** `Retrigger` is a genuine
   re-occurrence and keeps its row; a changed severity (an MTConnect Warning→Fault that keeps its text) does too.
   Residual: a Galaxy alarm that genuinely re-occurred during an outage and is still Active+unacked with identical

docs/drivers/AbCip.md

@@ -271,6 +271,8 @@ routes native transitions on) and `Kind` = `Raise` / `Clear`. Before Gitea #641
 **ALMD's active bit is `InAlarm`** (Rockwell's ALMD reference). Until hw4 the projection polled `InFaulted`, which
 is the *input-health* flag (Status.1, set when the program raises `InFault`), so a live ALMD would have raised on
 input faults and never on the alarm itself. Declare `InAlarm` (and `Severity`) on an ALMD tag.
+**Migration:** a tag that declared `InFaulted` but not `InAlarm` now never raises (the `InAlarm` read is
+`BadNodeIdUnknown`); the projection logs a once-per-tag Warning at subscribe — add `InAlarm` to the tag's members.
 
 ### ALMA (analog alarms)
 
@@ -294,7 +296,8 @@ layout — leave grouping off for ALMA tags (BOOL members are read member by mem
   instruction's `AckRequired` is false); active→none `Clear`; a limit **newly** entering while active `Retrigger`
   (a new unacknowledged occurrence) — or `ActiveAcknowledged` if nothing is pending ack, never
   Retrigger-then-Acknowledge; every declared `*Acked` bit going true while the condition was unacknowledged
-  `Acknowledge`; a limit dropping out (the governing limit or its severity changes, no new occurrence) an
+  `Acknowledge`; a Good `*Acked` read going false on a fully-acked condition with the same limits active
+  `Retrigger` (an occurrence the poll missed); a limit dropping out (the governing limit or its severity changes, no new occurrence) an
   `Unspecified` event flagged `IsRestatement`, which `NativeAlarmProjector` applies as a text/severity move with
   **no `/alerts` row**. A non-governing limit flickering emits nothing.
 - **"Acked" is judged over every declared limit, active or not.** In Logix an occurrence that returned to normal

src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IAlarmSource.cs

@@ -107,8 +107,12 @@ public sealed record AlarmAcknowledgeRequest(
 ///     watched happen: a reconnect snapshot entry (Galaxy) or the first event for a condition after a driver start or
 ///     deploy respawn (OpcUaClient). It governs <b>history only, never node state</b>: the host still projects the
 ///     state, but writes no <c>alerts</c> row when the re-statement changes nothing the condition shows (a Retrigger on
-///     an Active+unacked condition with the same message and severity; a Clear on an already-inactive condition).
-///     <c>false</c> (the default, every source that does not opt in) keeps every transition's row.
+///     an Active+unacked condition with the same message and severity; a Clear on an already-inactive condition) — or
+///     when a flagged <see cref="AlarmTransitionKind.Unspecified"/> update moves the message/severity of an
+///     already-active condition (node updated, one condition event, active/acked kept, no row). AbCip ALMA sets the
+///     flag for the last shape (the governing limit changed, no new occurrence); OpcUaClient's first sight of an
+///     inactive condition after a respawn can reach it too. <c>false</c> (the default, every source that does not
+///     opt in) keeps every transition's row.
 /// </param>
 public sealed record AlarmEventArgs(
     IAlarmSubscriptionHandle SubscriptionHandle,

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipAlarmProjection.cs

@@ -38,6 +38,7 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
     private readonly Dictionary<long, Subscription> _subs = new();
     private readonly Lock _subsLock = new();
     private long _nextId;
+    private readonly HashSet<string> _warnedNoInAlarm = new(StringComparer.Ordinal);
 
     /// <summary>Initializes a new instance of the <see cref="AbCipAlarmProjection"/> class.</summary>
     /// <param name="driver">The AB CIP driver instance.</param>
@@ -285,6 +286,8 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
         }
     }
 
+    private bool WarnOnce(string id) { lock (_warnedNoInAlarm) return _warnedNoInAlarm.Add(id); }
+
     private List<NodePlan> BuildPlans(IReadOnlyList<string> nodeIds)
     {
         var plans = new List<NodePlan>(nodeIds.Count);
@@ -293,6 +296,16 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
             AlmaPlan? alma = null;
             if (_driver.TryGetTagDefinition(id, out var def) && AbCipAlarmDetector.IsAlma(def))
                 alma = AlmaPlan.From(def);
+            else if (def is { DataType: AbCipDataType.Structure, Members: { } members }
+                && !members.Any(m => string.Equals(m.Name, "InAlarm", StringComparison.OrdinalIgnoreCase))
+                && WarnOnce(id))
+            {
+                // ALMD's active bit is InAlarm; a tag that declared members without it (e.g. a pre-hw4 config that
+                // declared InFaulted) reads BadNodeIdUnknown every tick and would stay silent forever.
+                _logger.LogWarning(
+                    "AbCip alarm ref {Ref} declares members but no 'InAlarm'; the ALMD projection polls InAlarm + Severity, " +
+                    "so this alarm will never raise. Add InAlarm (InFaulted is the input-health flag, not the alarm).", id);
+            }
             plans.Add(new NodePlan(alma));
         }
         return plans;
@@ -307,6 +320,8 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
     ///       <item>a limit NEWLY entering: Retrigger (a new unacknowledged occurrence) — or ActiveAcknowledged when
     ///         nothing is pending ack (AckRequired false), never Retrigger-then-Acknowledge (#656).</item>
     ///       <item>no new limit, every declared limit now acked while the condition was unacked: Acknowledge.</item>
+    ///       <item>no new limit, the condition was fully acked and a declared <c>*Acked</c> read (Good) is now false:
+    ///         Retrigger — an occurrence the poll missed.</item>
     ///       <item>no new limit, the governing limit or its severity changed (a drop-out): an
     ///         <c>Unspecified</c> + <c>IsRestatement</c> state update — the projector keeps active/acked and writes
     ///         NO alerts row; anything else here would render a false "Activated".</item>
@@ -386,6 +401,14 @@ internal sealed class AbCipAlarmProjection : IAsyncDisposable
         }
 
         _driver.AlarmAnalogRecord.Set(nodeId, mask, nowAcked, topId, topSev);
+        if (ackKnown && prior.Acked && !allAcked)
+        {
+            // Every limit was acked and now one is not, with the same limits active: an occurrence the poll missed (a
+            // limit left and re-entered inside one interval) or one a Bad *Acked read hid on the tick it entered.
+            // Either way the PLC says unacknowledged, so tell it — a new unacknowledged occurrence.
+            _driver.InvokeAlarmEvent(Build(AlarmTransitionKind.Retrigger, message, topSev));
+            return;
+        }
         if (nowAcked && !prior.Acked)
             _driver.InvokeAlarmEvent(Build(AlarmTransitionKind.Acknowledge, message, topSev));
         else if (topId != prior.Top || topSev != prior.TopSeverity)

src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs

@@ -52,8 +52,10 @@ public sealed class NativeAlarmProjector
     ///         <item>Gitea #656 — a <see cref="AlarmEventArgs.IsRestatement"/> event (a source re-stating upstream's
     ///         current state: a reconnect snapshot, first sight after a respawn) writes no row when it changes nothing
     ///         the condition shows: a Retrigger on an Active+unacked condition with the same message AND severity, or
-    ///         a Clear on an already-inactive condition. Unflagged sources (FOCAS, MTConnect, AbCip, live Galaxy
-    ///         transitions) are unaffected — a live Retrigger is a genuine re-occurrence and keeps its row.</item>
+    ///         a Clear on an already-inactive condition; and a flagged Unspecified update of an already-active
+    ///         condition moves its message/severity (node + one condition event) with active/acked kept and no row
+    ///         (AbCip ALMA's governing-limit change). Unflagged sources (FOCAS, MTConnect, live Galaxy transitions) are
+    ///         unaffected — a live Retrigger is a genuine re-occurrence and keeps its row.</item>
     ///     </list>
     /// </summary>
     /// <param name="nodeId">The materialised condition node's id (the projection's state key).</param>
@@ -110,6 +112,9 @@ public sealed class NativeAlarmProjector
                 break;
             // A message/severity-only update of an ALREADY-ACTIVE condition that the source flags as a re-statement
             // (AbCip ALMA: the governing limit changed but no new occurrence started, e.g. HH dropped back to H).
+            // OpcUaClient can reach it too: first sight after a respawn of an upstream-inactive condition is told
+            // Unspecified+flagged; with the host still Active the node stays Active exactly as before (a pre-existing
+            // gap) and the false "Activated" row it used to write is now gone.
             // The node's text/severity move; active/acked are kept; NO row (an "Activated" row would be false).
             case AlarmTransitionKind.Unspecified when e.IsRestatement && prev.Active:
                 (active, acked, historyKinds) = (true, prev.Acked, []);

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlarmProjectionTests.cs

@@ -196,6 +196,44 @@ public sealed class AbCipAlarmProjectionTests
         await drv.ShutdownAsync(CancellationToken.None);
     }
 
+    /// <summary>hw4 F2 — an ALMD that declared members but no InAlarm (a pre-hw4 InFaulted config) would read
+    /// BadNodeIdUnknown forever; it logs ONE Warning per tag instead of staying silent.</summary>
+    [Fact]
+    public async Task An_almd_without_a_declared_InAlarm_warns_once()
+    {
+        var legacy = new AbCipTagDefinition("Legacy", Device, "Legacy", AbCipDataType.Structure, Members:
+            [new AbCipStructureMember("InFaulted", AbCipDataType.DInt), new AbCipStructureMember("Acked", AbCipDataType.DInt),
+             new AbCipStructureMember("Severity", AbCipDataType.DInt)]);
+        var log = new CapturingLogger();
+        var opts = new AbCipDriverOptions
+        {
+            Devices = [new AbCipDeviceOptions(Device)],
+            RawTags = AbCipRawTags.From(legacy),
+            EnableAlarmProjection = true,
+            AlarmPollInterval = TimeSpan.FromMilliseconds(20),
+        };
+        var drv = new AbCipDriver(opts, "drv-1", new FakeAbCipTagFactory(), logger: log);
+        await drv.InitializeAsync("{}", CancellationToken.None);
+        var h1 = await drv.SubscribeAlarmsAsync(["Legacy"], CancellationToken.None);
+        var h2 = await drv.SubscribeAlarmsAsync(["Legacy"], CancellationToken.None);
+        await drv.UnsubscribeAlarmsAsync(h1, CancellationToken.None);
+        await drv.UnsubscribeAlarmsAsync(h2, CancellationToken.None);
+        await drv.ShutdownAsync(CancellationToken.None);
+        log.Warnings.Count(m => m.Contains("Legacy") && m.Contains("InAlarm")).ShouldBe(1);
+    }
+
+    private sealed class CapturingLogger : Microsoft.Extensions.Logging.ILogger<AbCipDriver>
+    {
+        public List<string> Warnings { get; } = [];
+        public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
+        public bool IsEnabled(Microsoft.Extensions.Logging.LogLevel logLevel) => true;
+        public void Log<TState>(Microsoft.Extensions.Logging.LogLevel logLevel, Microsoft.Extensions.Logging.EventId eventId,
+            TState state, Exception? exception, Func<TState, Exception?, string> formatter)
+        {
+            if (logLevel == Microsoft.Extensions.Logging.LogLevel.Warning) lock (Warnings) Warnings.Add(formatter(state, exception));
+        }
+    }
+
     /// <summary>Verifies that alarm clear event fires on 1-to-0 transition.</summary>
     [Fact]
     public async Task Clear_Event_Fires_On_1_to_0_Transition()

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipAlmaProjectionTests.cs

@@ -254,6 +254,25 @@ public sealed class AbCipAlmaProjectionTests
             "only the genuine PLC ack — the unacked HH excursion must keep the condition unacknowledged");
     }
 
+    /// <summary>F3 — fully acked, then a declared *Acked goes false with the SAME limits active (a limit that left and re-entered
+    /// between polls): the PLC says unacknowledged, so it is told as a Retrigger.</summary>
+    [Fact]
+    public async Task An_ack_falling_with_an_unchanged_active_set_retriggers()
+    {
+        var r = await StartAsync(x =>
+        {
+            x.Set("HInAlarm", true);
+            foreach (var a in new[] { "HHAcked", "HAcked", "LAcked", "LLAcked", "ROCPosAcked" }) x.Set(a, true);
+        });
+        await Next(r, x => x.Kind == AlarmTransitionKind.ActiveAcknowledged);
+        r.Set("HAcked", false);
+        var e = await Next(r, x => x.Kind == AlarmTransitionKind.Retrigger);
+        await Settled(r);
+        await r.StopAsync();
+        e.Message.ShouldBe($"ALMA {Tag} H alarm");
+        Snapshot(r).Count(x => x.Kind == AlarmTransitionKind.Retrigger).ShouldBe(1);
+    }
+
     /// <summary>An alarm already active AND acked when first seen is told atomically, never Raise-then-Acknowledge.</summary>
     [Fact]
     public async Task An_already_acked_active_alarm_is_active_acknowledged()