fix(runtime): Primary verdict published from host start, cache boot never adopts malformed bytes, strict drain-gate predicate, HostAbortedException passthrough, seed only Up members

Review R1: - M1 (#609): DriverHostActor.StopChild releases the instance's resilience state before evicting the hub. - m2 (#597): DriverHostActor publishes ShouldServiceAsPrimary() to IDriverPrimaryRoleSource at start and on cluster membership changes (deduped), so the native-ack gate denies in a pair's boot window. - m4 (#601): ApplyCachedArtifact returns false on empty/malformed bytes; the cached revision is not adopted, Direct mode goes Stale (no RUNNING FROM CACHE), FetchAndCache lands Steady-with-no-revision and a re-dispatch of the same revision really applies. - m3 (#603): LocalDbReplicationPolicy splits into CapturesChanges (PeerAddress||ApiKey, registration) and SharesRowsWithPeer (ApiKey && (PeerAddress||SyncListenPort>0), alarm drain gate). - m5 (#618): Program.cs catch-all filters HostAbortedException (design-time tooling). - m6 (Cluster-002): ClusterRoleInfo seeds only Up/WeaklyUp members. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
2026-10-08 07:47:34 -04:00
parent 83e9ffd27a
commit e3dcb180c3
17 changed files with 453 additions and 65 deletions
@@ -102,8 +102,14 @@ public sealed class DriverHostActorBootFromCacheTests : RuntimeActorTestBase

        // Barrier: diagnostics are served only after PreStart's boot work ran, so the absence below is
        // asserted after the boot path had its chance to Tell (positive control is the valid-bytes test).
-        AskDiagnostics(actor);
+        var snapshot = AskDiagnostics(actor);
        publishProbe.ExpectNoMsg(TimeSpan.FromMilliseconds(500));
+
+        // Review R1 m4: nothing was applied, so the node is NOT running from cache and has NOT adopted the
+        // cached revision (Stale with no configuration, retrying the ConfigDb) — the valid-bytes test above
+        // is the control that both flip when the bytes are usable.
+        snapshot.RunningFromCache.ShouldBeFalse();
+        snapshot.CurrentRevision.ShouldBeNull();
    }

    [Fact]
@@ -111,6 +111,39 @@ public sealed class DriverHostActorFetchAndCacheBootstrapTests : RuntimeActorTes
        fetcher.Calls.ShouldBe(1);
    }

+    /// <summary>
+    ///     #601 / review R1 m4 — a cache row whose bytes are malformed applied nothing, so its revision must not
+    ///     become current: the boot lands Steady-with-no-revision (as for an empty cache), and a re-dispatch of
+    ///     the SAME revision really applies (fetches) instead of being acked by the "already at this rev"
+    ///     short-circuit. <see cref="WithCachedArtifact_RestoresServedState_WithoutReadingCentralSql"/> is the
+    ///     control: valid bytes do adopt the revision.
+    /// </summary>
+    [Fact]
+    public void WithMalformedCachedBytes_TheRevisionIsNotAdopted_AndARedispatchOfItActuallyApplies()
+    {
+        var cached = new CachedDeploymentArtifact(
+            DeploymentId.NewId().ToString(), CachedRev.Value,
+            System.Text.Encoding.UTF8.GetBytes("{\"DriverInstances\": ["), DateTimeOffset.UtcNow.AddHours(-1));
+        var fetcher = new RecordingFetcher(Artifact());
+        var coordinator = CreateTestProbe();
+
+        var actor = Sys.ActorOf(DriverHostActor.Props(
+            new ThrowingDbFactory(), TestNode, coordinator.Ref,
+            localRoles: new HashSet<string> { "driver" },
+            deploymentArtifactCache: new StubArtifactCache(cached),
+            fetchAndCacheMode: true,
+            artifactFetcher: fetcher));
+
+        var snapshot = AskDiagnostics(actor);
+        snapshot.CurrentRevision.ShouldBeNull();
+        snapshot.RunningFromCache.ShouldBeFalse();
+
+        actor.Tell(new DispatchDeployment(DeploymentId.Parse(cached.DeploymentId), CachedRev, CorrelationId.NewId()));
+        coordinator.ExpectMsg<ApplyAck>(TimeSpan.FromSeconds(5)).Outcome.ShouldBe(ApplyAckOutcome.Applied);
+        fetcher.Calls.ShouldBe(1);
+        AskDiagnostics(actor).CurrentRevision.ShouldBe(CachedRev);
+    }
+
    private NodeDiagnosticsSnapshot AskDiagnostics(IActorRef actor)
    {
        var probe = CreateTestProbe();
@@ -0,0 +1,109 @@
+using Akka.Actor;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Commons.Messages.Redundancy;
+using ZB.MOM.WW.OtOpcUa.Commons.Types;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+using ZB.MOM.WW.OtOpcUa.Runtime.Drivers;
+using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness;
+
+namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers;
+
+/// <summary>
+///     Review R1 m2 (Gitea #597) — the Primary data-plane verdict published to <see cref="IDriverPrimaryRoleSource"/>
+///     (read by the native-ack gate and the Sparkplug primary-host claim) mirrors the host's own live gate from the
+///     moment the host starts, not only from the first redundancy snapshot.
+/// </summary>
+/// <remarks>
+///     <c>RedundancyRoleView</c> is seeded with the single-node verdict (allow). Before this fix nothing published to
+///     it until a snapshot arrived, so on a pair both nodes accepted a native ack through the whole boot window while
+///     <c>DriverHostActor</c> — which evaluates the driver member count live — denied the upstream route on the
+///     non-Primary. Assertions are on what was PUBLISHED: the seed already reads <c>true</c>, so a current-value
+///     assertion of the single-node case would pass with no publish at all.
+/// </remarks>
+public sealed class DriverHostActorPrimaryRoleSourceTests : RuntimeActorTestBase
+{
+    private static readonly NodeId TestNode = NodeId.Parse("driver-primary-source-test");
+    private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(5);
+
+    /// <summary>On a pair, an unknown role publishes DENY at start — before any snapshot. The later Primary snapshot
+    /// is the positive control: the source is wired, and the deny was the verdict, not a missing publish.</summary>
+    [Fact]
+    public void A_paired_host_publishes_deny_before_the_first_snapshot()
+    {
+        var source = new RecordingPrimaryRoleSource();
+        var host = SpawnHost(source, () => 2);
+
+        AwaitAssert(() => source.Published.ShouldBe([false]), duration: Timeout);
+
+        host.Tell(Snapshot(RedundancyRole.Primary));
+        AwaitAssert(() => source.Published.ShouldBe([false, true]), duration: Timeout);
+    }
+
+    /// <summary>A single-driver node keeps the allow posture — it is published, not merely seeded.</summary>
+    [Fact]
+    public void A_single_node_host_publishes_allow_at_start()
+    {
+        var source = new RecordingPrimaryRoleSource();
+        SpawnHost(source, () => 1);
+
+        AwaitAssert(() => source.Published.ShouldBe([true]), duration: Timeout);
+    }
+
+    /// <summary>A driver peer coming Up while the role is still unknown flips the published verdict to deny, with no
+    /// snapshot involved — the membership re-publish path.</summary>
+    [Fact]
+    public void A_peer_joining_before_any_snapshot_flips_the_published_verdict_to_deny()
+    {
+        var source = new RecordingPrimaryRoleSource();
+        var members = 1;
+        var host = SpawnHost(source, () => Volatile.Read(ref members));
+        AwaitAssert(() => source.Published.ShouldBe([true]), duration: Timeout);
+
+        Volatile.Write(ref members, 2);
+        host.Tell(DriverHostActor.RepublishPrimaryVerdict.Instance);
+
+        AwaitAssert(() => source.Published.ShouldBe([true, false]), duration: Timeout);
+    }
+
+    private IActorRef SpawnHost(IDriverPrimaryRoleSource source, Func<int> driverMemberCount) =>
+        Sys.ActorOf(DriverHostActor.Props(
+            NewInMemoryDbFactory(),
+            TestNode,
+            ackRouter: null,
+            driverMemberCountProvider: driverMemberCount,
+            primaryRoleSource: source));
+
+    private static RedundancyStateChanged Snapshot(RedundancyRole role) =>
+        new(
+            [
+                new NodeRedundancyState(TestNode, role,
+                    IsClusterLeader: role == RedundancyRole.Primary,
+                    IsDriverPrimary: role == RedundancyRole.Primary,
+                    AsOfUtc: DateTime.UtcNow),
+            ],
+            CorrelationId.NewId());
+
+    /// <summary>Remembers every published verdict, so tests assert on the actor's action, not on the seed.</summary>
+    private sealed class RecordingPrimaryRoleSource : IDriverPrimaryRoleSource
+    {
+        private readonly List<bool> _published = [];
+        private readonly Lock _gate = new();
+
+        /// <summary>Every value published so far, in order.</summary>
+        public IReadOnlyList<bool> Published
+        {
+            get { lock (_gate) return _published.ToArray(); }
+        }
+
+        public bool ShouldActAsPrimary
+        {
+            get { lock (_gate) return _published.Count == 0 || _published[^1]; }
+        }
+
+        public void PublishPrimaryRole(bool shouldActAsPrimary)
+        {
+            lock (_gate) _published.Add(shouldActAsPrimary);
+        }
+    }
+}
@@ -109,7 +109,8 @@ public sealed class AlarmHistorianRegistrationTests
    /// <remarks>
    ///     <para>
    ///         Standing down is only safe because a peer holds the same rows and will send them
-    ///         instead. Without <c>LocalDb:Replication:PeerAddress</c> there is no such peer: the rows
+    ///         instead. Without an authenticated sync path (<c>ApiKey</c> plus a dial or a listener —
+    ///         <c>LocalDbReplicationPolicy.SharesRowsWithPeer</c>) there is no such peer: the rows
    ///         are here and nowhere else, so any deferral means nobody ever delivers them.
    ///     </para>
    ///     <para>
@@ -124,14 +125,18 @@ public sealed class AlarmHistorianRegistrationTests
    // Neither key: an unpaired node. Its rows exist nowhere else.
    [InlineData(null, null, null, true)]
    // The dialing half of a pair — gated, because its partner holds the same rows.
-    [InlineData("http://peer:9001", null, null, false)]
+    [InlineData("http://peer:9001", null, "k", false)]
    // The LISTENING half. Same shared queue, but it never dials, so a PeerAddress-only test would
-    // wrongly class it as unpaired and leave it permanently ungated. It is identified by the shared
-    // ApiKey (the same predicate LocalDbSetup registers tables on), not by its listen port.
+    // wrongly class it as unpaired and leave it permanently ungated. Key + listen port = a peer.
    [InlineData(null, "9001", "k", false)]
    // A listen port with NO key: the fail-closed passive endpoint rejects every inbound stream, so the
    // queue is shared with nobody and the node must keep draining (#603: this used to be gated).
    [InlineData(null, "9001", null, true)]
+    // A dial with NO key: the peer's fail-closed endpoint rejects it — shares nothing, keeps draining.
+    [InlineData("http://peer:9001", null, null, true)]
+    // A key with NEITHER path (review R1 m3: e.g. a fleet-wide secret on an unpaired node) — no listener,
+    // no dial, so no peer holds these rows. Registered for capture, but must keep draining.
+    [InlineData(null, null, "k", true)]
    public async Task Only_an_unreplicated_node_ignores_the_role_view(
        string? peerAddress, string? syncListenPort, string? apiKey, bool expectDrain)
    {
@@ -4,29 +4,64 @@ using Xunit;

 namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Historian;

-/// <summary>#603: the single "is this node's LocalDb replicated" predicate shared by table registration and the alarm drain gate.</summary>
+/// <summary>
+///     #603 + review R1 m3: the two "is this node's LocalDb replicated" predicates — permissive for table
+///     registration (<see cref="LocalDbReplicationPolicy.CapturesChanges"/>), strict for the alarm drain gate
+///     (<see cref="LocalDbReplicationPolicy.SharesRowsWithPeer"/>).
+/// </summary>
 public sealed class LocalDbReplicationPolicyTests
 {
+    private const string Peer = "LocalDb:Replication:PeerAddress";
+    private const string Key = "LocalDb:Replication:ApiKey";
+    private const string Port = "LocalDb:SyncListenPort";
+
    private static IConfiguration Config(params (string Key, string? Value)[] pairs) =>
        new ConfigurationBuilder()
            .AddInMemoryCollection(pairs.Select(p => new KeyValuePair<string, string?>(p.Key, p.Value)))
            .Build();

-    /// <summary>PeerAddress OR ApiKey counts; the listen port alone and blank values do not.</summary>
+    /// <summary>Registration: PeerAddress OR ApiKey counts; the listen port alone and blank values do not.</summary>
    [Theory]
-    [InlineData("LocalDb:Replication:PeerAddress", "http://peer:9001", true)]
-    [InlineData("LocalDb:Replication:ApiKey", "k", true)]
-    [InlineData("LocalDb:SyncListenPort", "9001", false)]
-    [InlineData("LocalDb:Replication:ApiKey", "  ", false)]
-    public void IsConfigured_follows_the_PeerAddress_or_ApiKey_rule(string key, string value, bool expected)
+    [InlineData(Peer, "http://peer:9001", true)]
+    [InlineData(Key, "k", true)]
+    [InlineData(Port, "9001", false)]
+    [InlineData(Key, "  ", false)]
+    public void CapturesChanges_follows_the_PeerAddress_or_ApiKey_rule(string key, string value, bool expected)
    {
-        LocalDbReplicationPolicy.IsConfigured(Config((key, value))).ShouldBe(expected);
+        LocalDbReplicationPolicy.CapturesChanges(Config((key, value))).ShouldBe(expected);
    }

-    /// <summary>No replication keys at all is an unpaired node.</summary>
-    [Fact]
-    public void IsConfigured_is_false_for_an_empty_configuration()
+    /// <summary>Drain gate: ApiKey AND (PeerAddress OR SyncListenPort &gt; 0).</summary>
+    [Theory]
+    [InlineData("http://peer:9001", "k", null, true)]   // dialling half of a pair
+    [InlineData(null, "k", "9001", true)]               // listening half of a pair
+    [InlineData("http://peer:9001", "k", "9001", true)] // both
+    [InlineData(null, "k", null, false)]                // key, no path: shares nothing (review R1 m3)
+    [InlineData(null, "k", "0", false)]                 // port 0 = no listener
+    [InlineData(null, "k", "nope", false)]              // unparseable port = no listener
+    [InlineData("http://peer:9001", null, "9001", false)] // paths, no key: both auth directions fail closed
+    [InlineData(null, "  ", "9001", false)]             // blank key
+    public void SharesRowsWithPeer_needs_a_key_and_a_sync_path(string? peer, string? key, string? port, bool expected)
    {
-        LocalDbReplicationPolicy.IsConfigured(Config()).ShouldBeFalse();
+        LocalDbReplicationPolicy.SharesRowsWithPeer(Config((Peer, peer), (Key, key), (Port, port))).ShouldBe(expected);
+    }
+
+    /// <summary>Every node the gate treats as sharing rows also captures — never a gated node with no capture.</summary>
+    [Theory]
+    [InlineData("http://peer:9001", "k", null)]
+    [InlineData(null, "k", "9001")]
+    public void A_node_sharing_rows_always_captures(string? peer, string? key, string? port)
+    {
+        var config = Config((Peer, peer), (Key, key), (Port, port));
+        LocalDbReplicationPolicy.SharesRowsWithPeer(config).ShouldBeTrue();
+        LocalDbReplicationPolicy.CapturesChanges(config).ShouldBeTrue();
+    }
+
+    /// <summary>No replication keys at all is an unpaired node under both predicates.</summary>
+    [Fact]
+    public void Both_predicates_are_false_for_an_empty_configuration()
+    {
+        LocalDbReplicationPolicy.CapturesChanges(Config()).ShouldBeFalse();
+        LocalDbReplicationPolicy.SharesRowsWithPeer(Config()).ShouldBeFalse();
    }
 }