fix(alarms): #647 review — Raise on an active condition is a re-statement; FOCAS outage quality + health

MAJOR-1: a deploy that changes a driver's config respawns it, and the fresh instance's empty told record re-sent Raise for every still-active alarm, un-acking acknowledged conditions and writing a duplicate "Activated" alerts row. NativeAlarmProjector now treats Raise on an already-active condition as a re-statement (keeps Acked; ForwardNativeAlarm skips the alerts row); only Retrigger (or inactive->active) un-acks. Sources whose upstream ack state is authoritative send Retrigger for an unacked re-statement: OpcUaClient on first sight (no prior told), Galaxy for an unacked active snapshot entry. Runtime E2E drives a FRESH FOCAS driver after a redeploy and asserts the ack survives. MAJOR-2: a failed FOCAS alarm read annotates each subscribed condition BadNoCommunication once per outage (#626 quality-only path) with one Warning (MINOR-6), lifted to Good on recovery; failed alarm reads degrade driver health with LastSuccessfulRead frozen (successful ones advance it), so an alarm-only driver trips the #527 comms-loss watchdog. MINOR-3 CLAUDE.md #630 bullet; MINOR-4 split line; MINOR-5 a non-empty cnc_rdalmmsg2 payload that fits neither stride throws; MINOR-7 value-tag + parsed-address tables are built and swapped, never cleared in place under the retained poll engine. Docs corrected (FOCAS.md, AlarmTracking.md, CLAUDE.md). Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
2026-10-08 20:18:03 -04:00
parent 482bafdfac
commit 89961af6e6
17 changed files with 516 additions and 66 deletions
@@ -257,7 +257,7 @@ public sealed class OpcUaClientDriverWireTests

    /// <summary>
    ///     #619 — ConditionRefresh over the wire: a condition already active+unacked BEFORE the driver subscribes is
-    ///     reported (Raise, keyed by the RawPath) right after subscribe, not only on its next transition. A second
+    ///     reported (Retrigger since #647, keyed by the RawPath) right after subscribe, not only on its next transition. A second
    ///     subscribe of the same set refreshes again and must emit nothing (no re-raise); a positive control then
    ///     proves the feed is still live by acknowledging upstream and seeing exactly the Acknowledge arrive.
    /// </summary>
@@ -293,9 +293,11 @@ public sealed class OpcUaClientDriverWireTests
        AlarmEventArgs[] Seen() { lock (seen) return [.. seen]; }

        var first = await drv.SubscribeAlarmsAsync([AlarmRef], ct);
-        await RecordedEvents.WaitUntilAsync(() => Seen().Any(e => e.Kind == AlarmTransitionKind.Raise));
+        // First sight of an active+UNACKED upstream condition is a Retrigger (#647 review): the host keeps an ack
+        // across a Raise on an already-active condition, so only a Retrigger forces the upstream's unack.
+        await RecordedEvents.WaitUntilAsync(() => Seen().Any(e => e.Kind == AlarmTransitionKind.Retrigger));

-        var raise = Seen().Single(e => e.Kind == AlarmTransitionKind.Raise);
+        var raise = Seen().Single(e => e.Kind == AlarmTransitionKind.Retrigger);
        raise.ConditionId.ShouldBe(AlarmRef);
        raise.Message.ShouldBe("already hot");
        raise.OpcUaSeverity.ShouldBe(800);
@@ -1,10 +1,12 @@
 using System.Text.Json;
 using Akka.Actor;
+using Akka.Cluster.Tools.PublishSubscribe;
 using Akka.TestKit;
 using Microsoft.EntityFrameworkCore;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Commons.Engines;
+using ZB.MOM.WW.OtOpcUa.Commons.Messages.Alerts;
 using ZB.MOM.WW.OtOpcUa.Commons.Messages.Deploy;
 using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
 using ZB.MOM.WW.OtOpcUa.Commons.Types;
@@ -15,6 +17,7 @@ using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
 using ZB.MOM.WW.OtOpcUa.Driver.FOCAS;
 using ZB.MOM.WW.OtOpcUa.Runtime.Drivers;
 using ZB.MOM.WW.OtOpcUa.Runtime.OpcUa;
+using ZB.MOM.WW.OtOpcUa.Runtime.ScriptedAlarms;
 using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness;

 namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers;
@@ -44,8 +47,8 @@ public sealed class FocasNativeAlarmEndToEndTests : RuntimeActorTestBase
    {
        var db = NewInMemoryDbFactory();
        var blob = BuildArtifact();
-        var deploymentId = Seed(db, blob);
-        var (host, publish) = SpawnHostAndApply(db, deploymentId);
+        var deploymentId = Seed(db, blob, Rev);
+        var (host, publish, _) = SpawnHostAndApply(db, deploymentId);

        // The raw tags exactly as the driver receives them: the artifact's merged DriverConfig RawTags array.
        var rawTags = RawTagsFor(blob, DriverId);
@@ -89,6 +92,76 @@ public sealed class FocasNativeAlarmEndToEndTests : RuntimeActorTestBase
        }
    }

+    /// <summary>
+    ///     Gitea #647 review (MAJOR-1) — a deploy that changes ANY of the driver's config respawns it, and the fresh
+    ///     instance has no record of what the server was told. Its first poll re-reads the still-active alarm and
+    ///     sends Raise; the host must treat that as a re-statement: the operator's acknowledgement survives and no
+    ///     second "Activated" alerts row is written. Driven through the real host with a FRESH driver instance.
+    /// </summary>
+    [Fact]
+    public async Task A_respawned_driver_re_reading_an_acknowledged_active_alarm_keeps_the_ack()
+    {
+        var db = NewInMemoryDbFactory();
+        var blob = BuildArtifact();
+        var alerts = CreateTestProbe();
+        DistributedPubSub.Get(Sys).Mediator.Tell(new Subscribe(ScriptedAlarmHostActor.AlertsTopic, alerts.Ref), alerts.Ref);
+        alerts.ExpectMsg<SubscribeAck>(TimeSpan.FromSeconds(5));
+        var (host, publish, coordinator) = SpawnHostAndApply(db, Seed(db, blob, Rev));
+        var rawTags = RawTagsFor(blob, DriverId);
+        var cnc = new AlarmOnlyFocasClient();
+        cnc.SetAlarms(new FocasActiveAlarm(401, FocasAlarmType.Servo, 1, "SERVO V-READY OFF"));
+
+        var first = await StartDriverAsync(rawTags, cnc, host);
+        try
+        {
+            publish.FishForMessage<OpcUaPublishActor.AlarmStateUpdate>(_ => true, TimeSpan.FromSeconds(5))
+                .State.Acknowledged.ShouldBeFalse();
+            alerts.ExpectMsg<AlarmTransitionEvent>(TimeSpan.FromSeconds(5));
+        }
+        finally
+        {
+            await first.ShutdownAsync(CancellationToken.None);
+        }
+
+        // The operator acknowledges, then a deploy changes the driver's config => stop + respawn.
+        host.Tell(new DriverHostActor.RouteNativeAlarmAck(ExpectedRawPath, "seen", "op"));
+        var revB = RevisionHash.Parse(new string('e', 64));
+        host.Tell(new DispatchDeployment(Seed(db, blob, revB), revB, CorrelationId.NewId()));
+        coordinator.FishForMessage<ApplyAck>(a => a.Outcome == ApplyAckOutcome.Applied, TimeSpan.FromSeconds(5));
+        publish.FishForMessage<OpcUaPublishActor.RebuildAddressSpace>(_ => true, TimeSpan.FromSeconds(5));
+
+        var respawned = await StartDriverAsync(rawTags, cnc, host);
+        try
+        {
+            var restated = publish.FishForMessage<OpcUaPublishActor.AlarmStateUpdate>(_ => true, TimeSpan.FromSeconds(5));
+            restated.AlarmNodeId.ShouldBe(ExpectedRawPath);
+            restated.State.Active.ShouldBeTrue();
+            restated.State.Acknowledged.ShouldBeTrue("a respawned driver's re-statement must not un-acknowledge");
+            alerts.ExpectNoMsg(TimeSpan.FromMilliseconds(500));
+        }
+        finally
+        {
+            await respawned.ShutdownAsync(CancellationToken.None);
+        }
+    }
+
+    private static async Task<FocasDriver> StartDriverAsync(List<RawTagEntry> rawTags, IFocasClient cnc, IActorRef host)
+    {
+        var driver = new FocasDriver(new FocasDriverOptions
+        {
+            Devices = [new FocasDeviceOptions(Host, DeviceName: "m1")],
+            RawTags = rawTags,
+            Probe = new FocasProbeOptions { Enabled = false },
+            AlarmProjection = new FocasAlarmProjectionOptions { PollInterval = TimeSpan.FromMilliseconds(20) },
+        }, DriverId, new SingleClientFactory(cnc));
+        driver.OnAlarmEvent += (_, e) => host.Tell(new DriverInstanceActor.AttributeAlarmPublished(DriverId, e));
+        await driver.InitializeAsync("{}", CancellationToken.None);
+#pragma warning disable OTOPCUA0001 // the routing is under test, not resilience wrapping
+        await driver.SubscribeAlarmsAsync([.. rawTags.Select(t => t.RawPath)], CancellationToken.None);
+#pragma warning restore OTOPCUA0001
+        return driver;
+    }
+
    /// <summary>The seeded artifact's FOCAS driver raw tags, decoded through the production artifact reader.</summary>
    private static List<RawTagEntry> RawTagsFor(byte[] blob, string driverInstanceId)
    {
@@ -133,14 +206,14 @@ public sealed class FocasNativeAlarmEndToEndTests : RuntimeActorTestBase
        },
    });

-    private static DeploymentId Seed(IDbContextFactory<OtOpcUaConfigDbContext> db, byte[] blob)
+    private static DeploymentId Seed(IDbContextFactory<OtOpcUaConfigDbContext> db, byte[] blob, RevisionHash rev)
    {
        var id = DeploymentId.NewId();
        using var ctx = db.CreateDbContext();
        ctx.Deployments.Add(new Deployment
        {
            DeploymentId = id.Value,
-            RevisionHash = Rev.Value,
+            RevisionHash = rev.Value,
            Status = DeploymentStatus.Sealed,
            CreatedBy = "test",
            SealedAtUtc = DateTime.UtcNow,
@@ -150,7 +223,7 @@ public sealed class FocasNativeAlarmEndToEndTests : RuntimeActorTestBase
        return id;
    }

-    private (IActorRef Actor, TestProbe Publish) SpawnHostAndApply(
+    private (IActorRef Actor, TestProbe Publish, TestProbe Coordinator) SpawnHostAndApply(
        IDbContextFactory<OtOpcUaConfigDbContext> db, DeploymentId deploymentId)
    {
        var coordinator = CreateTestProbe();
@@ -166,7 +239,7 @@ public sealed class FocasNativeAlarmEndToEndTests : RuntimeActorTestBase
        actor.Tell(new DispatchDeployment(deploymentId, Rev, CorrelationId.NewId()));
        coordinator.ExpectMsg<ApplyAck>(TimeSpan.FromSeconds(5)).Outcome.ShouldBe(ApplyAckOutcome.Applied);
        publish.ExpectMsg<OpcUaPublishActor.RebuildAddressSpace>(TimeSpan.FromSeconds(5));
-        return (actor, publish);
+        return (actor, publish, coordinator);
    }

    private sealed class SingleClientFactory(IFocasClient client) : IFocasClientFactory
@@ -224,6 +224,29 @@ public class NativeAlarmProjectorTests
        snap.StatusCode.ShouldBe(OpcUaStatus.BadNoCommunication);
    }

+    /// <summary>
+    ///     Gitea #647 review — a Raise on a condition that is ALREADY active is a re-statement (a respawned driver
+    ///     re-reading a still-active alarm it never told the server about): it keeps the acknowledgement and is
+    ///     flagged so the host writes no duplicate "Activated" row. Only a Retrigger (or inactive → active) un-acks.
+    /// </summary>
+    [Fact]
+    public void A_Raise_on_an_already_active_condition_is_a_restatement_that_keeps_the_ack()
+    {
+        var sut = new NativeAlarmProjector();
+        sut.Project("n1", Evt(AlarmTransitionKind.Raise), out var first).Active.ShouldBeTrue();
+        first.ShouldBeFalse("inactive → active is a real transition");
+        sut.MarkAcknowledged("n1");
+
+        var restated = sut.Project("n1", Evt(AlarmTransitionKind.Raise, msg: "again"), out var restatement);
+        restatement.ShouldBeTrue();
+        restated.Active.ShouldBeTrue();
+        restated.Acknowledged.ShouldBeTrue("a re-statement must not un-acknowledge");
+
+        var retriggered = sut.Project("n1", Evt(AlarmTransitionKind.Retrigger), out var retrigger);
+        retrigger.ShouldBeFalse();
+        retriggered.Acknowledged.ShouldBeFalse("a Retrigger is a new statement and un-acks");
+    }
+
    private static AlarmEventArgs Evt(
        AlarmTransitionKind kind,
        AlarmSeverity sev = AlarmSeverity.High,