Merge review R1 fixes: removed driver's resilience tracker released; SDK timed-unshelve no-op (engine owns expiry); commented re-ack stored as AddComment; native-ack Primary verdict published from start; drain gate SharesRowsWithPeer; malformed cache bytes don't adopt the revision; HostAbortedException passthrough; ClusterRoleInfo seed filtered

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
2026-10-08 07:47:57 -04:00
parent b7a48e2b93 e3dcb180c3
commit b99aea46b2
28 changed files with 691 additions and 136 deletions
@@ -263,14 +263,18 @@ public sealed class ConditionMethodEventTests

    /// <summary>
    ///     A scripted TimedShelve expires: the engine's expiry comes back as an Unshelved re-projection, which
-    ///     reports exactly one unshelve event after the one shelve event.
+    ///     reports exactly one unshelve event after the one shelve event — and the SDK's own expiry timer routes
+    ///     nothing (review R1 M2).
    /// </summary>
    /// <remarks>
    ///     The expiry is the engine's (computed from the command's <c>UnshelveAtUtc</c>). Since Gitea #597 the node
-    ///     is shelved for the real duration, so the SDK's own timer fires at the same instant — but it only routes
-    ///     an Unshelve back to the engine (ignored by this stand-in, a no-op on an unshelved engine alarm) and
-    ///     changes no node state, so it adds no event. The stand-in engine here schedules its expiry from the real
-    ///     <c>UnshelveAtUtc</c> the method handler computed.
+    ///     is shelved for the real duration, so the SDK's own timer fires at that instant. It used to route an
+    ///     operator-style <c>Unshelve</c> by <c>system</c>, which — because the real engine only checks expiry every
+    ///     5 s — pre-empted the engine's <c>AutoUnshelve</c> on every expiry. This stand-in engine therefore
+    ///     (a) expires LATE, like the real one, so the SDK timer demonstrably fires first; and (b) ACTS on a routed
+    ///     <c>Unshelve</c> as the real engine would, recording who unshelved it. The wrapped
+    ///     <c>OnTimedUnshelve</c> is the positive control: the SDK timer did fire, and still nothing was routed and
+    ///     the unshelve on record is the engine's own.
    /// </remarks>
    [Fact]
    public async Task A_scripted_timed_shelve_expires_with_exactly_one_unshelve_event()
@@ -283,21 +287,47 @@ public sealed class ConditionMethodEventTests
            rig.MaterialiseScripted();
            var current = PreState("TimedShelve", native: false) with { Message = "base" };
            Task? expiry = null;
+            var routed = new System.Collections.Concurrent.ConcurrentQueue<string>();
+            string? unshelvedBy = null;
+            var engineLag = TimeSpan.FromMilliseconds(1500); // the real engine's 5 s shelving-check granularity
            rig.Nm.AlarmCommandRouter = cmd =>
            {
+                routed.Enqueue(cmd.Operation);
+                if (cmd.Operation == "Unshelve")
+                {
+                    // What the real engine does with a routed Unshelve: an operator-style unshelve.
+                    unshelvedBy ??= $"Unshelve:{cmd.User}";
+                    current = current with { Shelving = AlarmShelvingKind.Unshelved, UnshelveAtUtc = null };
+                    rig.ProjectLater(native: false, current);
+                    return;
+                }
                if (cmd.Operation != "TimedShelve") return;
-                current = current with { Shelving = AlarmShelvingKind.Timed };
+                // The real engine's projection carries its expiry, so the re-projection keeps the SDK timer armed
+                // for the real remaining time (without it, it would re-arm for MaxTimeShelved and never fire here).
+                current = current with { Shelving = AlarmShelvingKind.Timed, UnshelveAtUtc = cmd.UnshelveAtUtc };
                rig.ProjectLater(native: false, current);
                var unshelveAt = cmd.UnshelveAtUtc!.Value;
                expiry = Task.Run(async () =>
                {
-                    var wait = unshelveAt - DateTime.UtcNow;
+                    var wait = unshelveAt + engineLag - DateTime.UtcNow;
                    if (wait > TimeSpan.Zero) await Task.Delay(wait);
-                    current = current with { Shelving = AlarmShelvingKind.Unshelved };
+                    if (current.Shelving != AlarmShelvingKind.Timed) return; // ApplyShelvingCheck: Timed only
+                    unshelvedBy ??= "AutoUnshelve:system";
+                    current = current with { Shelving = AlarmShelvingKind.Unshelved, UnshelveAtUtc = null };
                    rig.Project(native: false, current);
                });
            };

+            // Count the SDK's own expiry-timer firings without changing what the production handler does.
+            var condition = rig.Nm.TryGetAlarmCondition(ScriptedAlarmId)!;
+            var sdkTimerFired = 0;
+            var production = condition.OnTimedUnshelve!;
+            condition.OnTimedUnshelve = (context, c) =>
+            {
+                Interlocked.Increment(ref sdkTimerFired);
+                return production(context, c);
+            };
+
            using var session = await rig.ConnectAsync(anonymous: false, ct);
            var events = new RecordedEvents();
            var subscription = await RecordedEvents.SubscribeAsync(session, events, ct);
@@ -315,6 +345,10 @@ public sealed class ConditionMethodEventTests
            rig.Project(false, current = current with { Message = "sentinel" });
            await RecordedEvents.WaitUntilAsync(() => events.Has(item, "sentinel"));

+            Volatile.Read(ref sdkTimerFired).ShouldBeGreaterThanOrEqualTo(1, "the SDK's timed-unshelve timer fired");
+            routed.ShouldNotContain("Unshelve", "the SDK timer must not route an operator-style Unshelve");
+            unshelvedBy.ShouldBe("AutoUnshelve:system", "the engine's own expiry owns the unshelve");
+
            var shelving = events.ConditionEvents(item, "base").Select(e => e[RecordedEvents.ShelvingField].Value).ToList();
            shelving.ShouldBe(
                [ObjectIds.ShelvedStateMachineType_Unshelved, ObjectIds.ShelvedStateMachineType_TimedShelved,
@@ -268,19 +268,15 @@ public sealed class AlarmCommandRouterTests : IDisposable
        await host.DisposeAsync();
    }

-    /// <summary>OnTimedUnshelve fires with the SDK's system context (no session, no user identity) —
-    /// the real SDK path when a TimedShelve duration expires. The gate must NOT veto: the result must be
-    /// Good, the router must be invoked exactly once with Operation == "Unshelve", and no UnshelveAtUtc is
-    /// carried.
-    /// <para>
-    /// Regression (OpcUaServer-007): the routed command's <see cref="AlarmCommand.User"/> must be a
-    /// NON-EMPTY system user (<c>"system"</c>). The engine's <c>Part9StateMachine.ApplyUnshelve</c> rejects
-    /// a null/whitespace user with <c>ArgumentException("User required.")</c>; that exception is swallowed
-    /// downstream, so an empty user made the timed auto-unshelve SILENTLY no-op — a TimedShelve would never
-    /// auto-expire. Asserting a non-empty user here pins the value the state machine requires.
-    /// </para></summary>
+    /// <summary>OnTimedUnshelve fires with the SDK's system context (no session, no user identity) when the
+    /// SDK's own TimedShelve timer expires. It must return Good and route NOTHING (review R1 M2, Gitea #597):
+    /// the scripted engine owns timed-shelve expiry (its shelving check records an "AutoUnshelve" by
+    /// "system"), and since #597 arms the SDK timer for the real remaining time, an "Unshelve" routed from
+    /// here pre-empted that check on every expiry, from both pair nodes, as an operator-style unshelve.
+    /// <para>Positive control: on the same condition and router, a client Unshelve (OnShelve with
+    /// <c>shelving:false</c>) DOES route — so the empty capture is not a disconnected router.</para></summary>
    [Fact]
-    public async Task OnTimedUnshelve_with_system_context_returns_good_and_routes_unshelve()
+    public async Task OnTimedUnshelve_with_system_context_returns_good_and_routes_nothing()
    {
        var (host, server) = await BootAsync();
        var nm = server.NodeManager!;
@@ -299,14 +295,13 @@ public sealed class AlarmCommandRouterTests : IDisposable
        var result = condition.OnTimedUnshelve!(ctx, condition);

        result.ShouldBe(ServiceResult.Good);
-        captured.Count.ShouldBe(1);
-        var cmd = captured[0];
-        cmd.AlarmId.ShouldBe("alm-tu");    // == ScriptedAlarmId / condition NodeId identifier
-        cmd.Operation.ShouldBe("Unshelve");
-        // System-initiated (no client principal), but the user MUST be non-empty so ApplyUnshelve accepts it.
-        cmd.User.ShouldNotBeNullOrWhiteSpace();
-        cmd.User.ShouldBe("system");
-        cmd.UnshelveAtUtc.ShouldBeNull();
+        captured.ShouldBeEmpty();
+
+        // Positive control: the router is live for a genuine operator Unshelve on the same condition.
+        var operatorCtx = SessionContext(server, "grace", OpcUaDataPlaneRoles.AlarmAck);
+        condition.OnShelve!(operatorCtx, condition, shelving: false, oneShot: false, shelvingTime: 0.0)
+            .ShouldBe(ServiceResult.Good);
+        captured.Select(c => c.Operation).ShouldBe(["Unshelve"]);

        await host.DisposeAsync();
    }
@@ -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);
+        }
+    }
+}
@@ -227,6 +227,77 @@ public sealed class DriverHostActorUnreadableArtifactTests : RuntimeActorTestBas
        replayed.ShouldContain("other-node-driver");
    }

+    /// <summary>
+    ///     #609 (review R1 M1) — the Resilience half of the eviction must SURVIVE the next resilience-publisher
+    ///     tick. <c>DriverResilienceStatusPublisherService</c> re-emits <c>tracker.Snapshot()</c> into the hub
+    ///     every interval, so evicting only the hub cache was undone within one tick: the host must also
+    ///     release the instance's tracker state. The tick below is the publisher's own body
+    ///     (<c>BuildMessages</c> → <c>hub.Emit</c>) minus the DPS publish, run against the REAL factory +
+    ///     tracker the Host wires. The neighbour instance is the positive control: it is still emitted by
+    ///     the same tick, so an absent drv-1 is not an empty tick.
+    /// </summary>
+    [Fact]
+    public void Removing_a_driver_instance_releases_its_resilience_state_so_the_next_publisher_tick_omits_it()
+    {
+        var db = NewInMemoryDbFactory();
+        var factory = new SubscribingDriverFactory("Modbus");
+        var hub = new ZB.MOM.WW.OtOpcUa.Runtime.Telemetry.TelemetryLocalHub();
+        var tracker = new ZB.MOM.WW.OtOpcUa.Core.Resilience.DriverResilienceStatusTracker();
+        var invokerFactory = new ZB.MOM.WW.OtOpcUa.Core.Resilience.DriverCapabilityInvokerFactory(
+            new ZB.MOM.WW.OtOpcUa.Core.Resilience.DriverResiliencePipelineBuilder(statusTracker: tracker),
+            tracker,
+            _ => ZB.MOM.WW.OtOpcUa.Core.Abstractions.DriverTier.A);
+        var coordinator = CreateTestProbe();
+        var actor = Sys.ActorOf(DriverHostActor.Props(
+            db, TestNode, coordinator.Ref,
+            driverFactory: factory,
+            localRoles: new HashSet<string> { "driver" },
+            invokerFactory: invokerFactory,
+            telemetryHub: hub));
+
+        actor.Tell(new DispatchDeployment(SeedV3Deployment(db, RevA), RevA, CorrelationId.NewId()));
+        coordinator.ExpectMsg<ApplyAck>(Timeout).Outcome.ShouldBe(ApplyAckOutcome.Applied);
+
+        // Resilience counters for the running driver and for an instance this host does not own.
+        tracker.RecordBreakerOpen("drv-1", "dev1", DateTime.UtcNow);
+        tracker.RecordFailure("neighbour-drv", "dev9", DateTime.UtcNow);
+        PublisherTick(tracker, hub);
+        ReplayedResilienceIds(hub).ShouldContain("drv-1"); // control: drv-1 is primed before the removal
+
+        actor.Tell(new DispatchDeployment(SeedDriverlessDeployment(db, RevB), RevB, CorrelationId.NewId()));
+        coordinator.ExpectMsg<ApplyAck>(Timeout).Outcome.ShouldBe(ApplyAckOutcome.Applied);
+        AwaitAssert(
+            () => AskDiagnostics(actor).Drivers.Select(d => d.Name).ShouldNotContain("drv-1"),
+            duration: Timeout);
+
+        // The publisher's next tick, after the removal.
+        PublisherTick(tracker, hub);
+
+        var replayed = ReplayedResilienceIds(hub);
+        replayed.ShouldNotContain("drv-1");
+        replayed.ShouldContain("neighbour-drv");
+
+        static void PublisherTick(
+            ZB.MOM.WW.OtOpcUa.Core.Resilience.DriverResilienceStatusTracker t,
+            ZB.MOM.WW.OtOpcUa.Runtime.Telemetry.TelemetryLocalHub h)
+        {
+            var now = DateTime.UtcNow;
+            foreach (var e in t.Snapshot())
+                h.Emit(new ZB.MOM.WW.OtOpcUa.Runtime.Telemetry.TelemetryItem.Resilience(new(
+                    e.DriverInstanceId, e.HostName, e.Snapshot.IsBreakerOpen, e.Snapshot.ConsecutiveFailures,
+                    e.Snapshot.CurrentInFlight, e.Snapshot.LastBreakerOpenUtc, e.Snapshot.LastSampledUtc, now)));
+        }
+
+        static List<string> ReplayedResilienceIds(ZB.MOM.WW.OtOpcUa.Runtime.Telemetry.TelemetryLocalHub h)
+        {
+            using var sub = h.Subscribe(64);
+            var ids = new List<string>();
+            while (sub.Reader.TryRead(out var i))
+                if (i is ZB.MOM.WW.OtOpcUa.Runtime.Telemetry.TelemetryItem.Resilience r) ids.Add(r.E.DriverInstanceId);
+            return ids;
+        }
+    }
+
    /// <summary>Spawns the host with the subscribing factory, dispatches <paramref name="deploymentId"/> and
    /// waits for the Applied ACK so the child + the initial SubscribeBulk pass are in place.</summary>
    private (IActorRef Actor, Akka.TestKit.TestProbe Coordinator) SpawnHostAndApply(
@@ -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();
    }
 }