Merge fix/486: fail the apply when the deployment artifact could not be read

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

docs/plans/2026-07-21-issue-485-live-gate.md

@@ -61,3 +61,16 @@ not a data-plane one: the node keeps serving its last good configuration through
 ACK `Failed` and leave `_currentRevision` alone when the artifact could not be read, so the normal deploy
 retry heals it. Tracked separately rather than folded in here, because it changes deploy ACK semantics
 fleet-wide.
+
+## Addendum — issue #486 fix, live-gated (2026-07-21)
+
+The finding above is now fixed (`DriverHostActor.ApplyAndAck` fails the apply when `ReconcileDrivers`
+returns null) and gated on the same rig with the same induction.
+
+| # | Check | Result | Evidence |
+|---|---|---|---|
+| 8 | Regression — a normal deploy still ACKs Applied fleet-wide | ✅ | The change alters ACK semantics, so this is the risk that matters. A genuine deploy recorded `Status = Applied` on **all six** nodes. |
+| 9 | An unreadable artifact now ACKs Failed, with a reason | ✅ | central-2 recorded `Status = Failed` + `the deployment artifact could not be read (ConfigDb unreachable, or the row is missing/empty); nothing was applied`. The five nodes that read the real artifact recorded Applied — so the fleet view now distinguishes them. Pre-fix, all six claimed Applied. |
+| 10 | The revision is NOT advanced | ✅ | `apply of a855dd9e… FAILED — … Staying on revision 5bc0d9f3…` — the **previous** revision, not the dispatched `583122b7…`. This is what stops `HandleDispatchFromSteady` from short-circuiting a retry of that revision. |
+| 11 | The node keeps serving throughout (#485 still holds) | ✅ | The #485 guard fired first (`keeping the 5 running driver(s)`), and `ns=2;s=pymodbus/plc/HR200X` read **59603 @ 07:27:28**, live and current, after the Failed apply. Failing the apply reports the truth; it does not tear anything down. |
+| 12 | The node recovers on the next real deploy | ✅ | The following deployment applied on central-2 (`children=5`) and recorded Applied on all six nodes. |

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

@@ -1579,6 +1579,29 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
         try
         {
             var appliedBlob = ReconcileDrivers(deploymentId);
+
+            // Issue #486 — a null blob means the artifact could not be read (unreachable ConfigDb, or the
+            // empty bytes a missing row yields), so NOTHING was applied. Reporting Applied here used to
+            // advance _currentRevision too, and HandleDispatchFromSteady short-circuits on a revision
+            // match — so the node claimed a configuration it never applied AND could never be healed by
+            // re-dispatching that revision. Failing the apply keeps the revision where it was, which is
+            // what makes the retry land instead of being waved through. The node keeps serving its
+            // last-known-good address space, drivers and subscriptions throughout (issue #485) — this is
+            // about telling the truth, not about tearing anything down.
+            if (appliedBlob is null)
+            {
+                const string reason =
+                    "the deployment artifact could not be read (ConfigDb unreachable, or the row is missing/empty); nothing was applied";
+                UpsertNodeDeploymentState(deploymentId, NodeDeploymentStatus.Failed, reason);
+                SendAck(deploymentId, ApplyAckOutcome.Failed, reason, correlation);
+                OtOpcUaTelemetry.DeploymentApplied.Add(1, new KeyValuePair<string, object?>("outcome", "reject"));
+                span?.SetStatus(ActivityStatusCode.Error, reason);
+                _log.Error(
+                    "DriverHost {Node}: apply of {Id} FAILED — {Reason}. Staying on revision {Rev} and continuing to serve it; re-dispatch once the ConfigDb is readable",
+                    _localNode, deploymentId, reason, _currentRevision);
+                return;
+            }
+
             _currentRevision = revision;
             UpsertNodeDeploymentState(deploymentId, NodeDeploymentStatus.Applied, failureReason: null);
             SendAck(deploymentId, ApplyAckOutcome.Applied, failureReason: null, correlation);

tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorArtifactCacheTests.cs

@@ -129,10 +129,12 @@ public sealed class DriverHostActorArtifactCacheTests : RuntimeActorTestBase
 
         actor.Tell(new DispatchDeployment(deploymentId, RevA, CorrelationId.NewId()));
 
-        // The apply still succeeds — an empty artifact is a legitimate no-op deployment.
-        coordinator.ExpectMsg<ApplyAck>(TimeSpan.FromSeconds(5)).Outcome.ShouldBe(ApplyAckOutcome.Applied);
+        // Since #486 the apply FAILS rather than reporting a success it did not achieve: empty bytes are
+        // what an unreadable artifact looks like, not a legitimate no-op deployment. (A configuration that
+        // genuinely declares nothing still serialises to a JSON document with empty arrays.)
+        coordinator.ExpectMsg<ApplyAck>(TimeSpan.FromSeconds(5)).Outcome.ShouldBe(ApplyAckOutcome.Failed);
 
-        // But nothing is cached from it.
+        // The point of this test is unchanged, and now holds for two independent reasons: nothing is cached.
         Thread.Sleep(500);
         cache.Stores.ShouldBeEmpty();
     }

tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorTests.cs

@@ -262,8 +262,25 @@ public sealed class DriverHostActorTests : RuntimeActorTestBase
             Status = status,
             CreatedBy = "test",
             SealedAtUtc = status == DeploymentStatus.Sealed ? DateTime.UtcNow : null,
+            // A REAL (if empty-of-content) artifact. These tests are about the apply/ACK/state machine,
+            // not about the artifact, and used to omit the blob entirely — but since #486 an unreadable
+            // artifact (which a missing blob is indistinguishable from) correctly FAILS the apply, so an
+            // artifact-less row would no longer exercise the success path they are asserting on. A
+            // configuration that declares no drivers is the genuine "nothing to run" deployment.
+            ArtifactBlob = EmptyButRealArtifact,
         });
         ctx.SaveChanges();
         return id;
     }
+
+    /// <summary>A well-formed artifact declaring no drivers — what the composer emits for a configuration
+    /// with nothing in it, as distinct from bytes that could not be read.</summary>
+    private static readonly byte[] EmptyButRealArtifact = JsonSerializer.SerializeToUtf8Bytes(new
+    {
+        RawFolders = Array.Empty<object>(),
+        DriverInstances = Array.Empty<object>(),
+        Devices = Array.Empty<object>(),
+        TagGroups = Array.Empty<object>(),
+        Tags = Array.Empty<object>(),
+    });
 }

tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorUnreadableArtifactTests.cs

@@ -120,6 +120,57 @@ public sealed class DriverHostActorUnreadableArtifactTests : RuntimeActorTestBas
             duration: Timeout);
     }
 
+    /// <summary>
+    ///     Issue #486 — an apply that read no artifact applied nothing, so it must NOT be reported as
+    ///     Applied. Reporting success also advanced <c>_currentRevision</c>, and because
+    ///     <c>HandleDispatchFromSteady</c> short-circuits on a revision match, the node could never be
+    ///     healed by re-dispatching that revision.
+    /// </summary>
+    [Fact]
+    public void Dispatch_whose_artifact_reads_back_empty_acks_Failed_and_leaves_the_revision_alone()
+    {
+        var db = NewInMemoryDbFactory();
+        var factory = new SubscribingDriverFactory("Modbus");
+        var (actor, coordinator) = SpawnHostAndApply(db, SeedV3Deployment(db, RevA), factory);
+        AskDiagnostics(actor).CurrentRevision.ShouldBe(RevA);
+
+        actor.Tell(new DispatchDeployment(SeedUnreadableDeployment(db, RevB), RevB, CorrelationId.NewId()));
+        var ack = coordinator.ExpectMsg<ApplyAck>(Timeout);
+
+        ack.Outcome.ShouldBe(ApplyAckOutcome.Failed);
+        ack.FailureReason.ShouldNotBeNullOrWhiteSpace();
+
+        var snapshot = AskDiagnostics(actor);
+        snapshot.CurrentRevision.ShouldBe(RevA);                     // never applied ⇒ never claimed
+        snapshot.Drivers.Select(d => d.Name).ShouldContain("drv-1"); // …and #485 still holds
+    }
+
+    /// <summary>Issue #486 — the point of failing the apply: the SAME revision can be dispatched again and
+    /// now actually applies, instead of being waved through by the "already at this rev" short-circuit. The
+    /// recovered artifact adds a second driver, so a short-circuited ACK (which has no side effects) cannot
+    /// satisfy this test — drv-2 only appears if the artifact was genuinely re-read and reconciled.</summary>
+    [Fact]
+    public void A_failed_apply_is_retried_not_short_circuited_once_the_artifact_is_readable_again()
+    {
+        var db = NewInMemoryDbFactory();
+        var factory = new SubscribingDriverFactory("Modbus");
+        var (actor, coordinator) = SpawnHostAndApply(db, SeedV3Deployment(db, RevA), factory);
+
+        var deploymentId = SeedUnreadableDeployment(db, RevB);
+        actor.Tell(new DispatchDeployment(deploymentId, RevB, CorrelationId.NewId()));
+        coordinator.ExpectMsg<ApplyAck>(Timeout).Outcome.ShouldBe(ApplyAckOutcome.Failed);
+
+        // The ConfigDb recovers: the row now carries a real artifact, under the SAME revision.
+        SetArtifact(db, deploymentId, TwoDriverArtifact());
+        actor.Tell(new DispatchDeployment(deploymentId, RevB, CorrelationId.NewId()));
+
+        coordinator.ExpectMsg<ApplyAck>(Timeout).Outcome.ShouldBe(ApplyAckOutcome.Applied);
+        AwaitAssert(
+            () => AskDiagnostics(actor).Drivers.Select(d => d.Name).ShouldContain("drv-2"),
+            duration: Timeout);
+        AskDiagnostics(actor).CurrentRevision.ShouldBe(RevB);
+    }
+
     /// <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(
@@ -161,6 +212,50 @@ public sealed class DriverHostActorUnreadableArtifactTests : RuntimeActorTestBas
             },
         }));
 
+    /// <summary>A readable artifact carrying drv-1 AND drv-2, so an apply that genuinely happens is
+    /// distinguishable from a short-circuited ACK (which spawns nothing).</summary>
+    private static byte[] TwoDriverArtifact() => JsonSerializer.SerializeToUtf8Bytes(new
+    {
+        RawFolders = new[] { new { RawFolderId = "rf-plant", ParentRawFolderId = (string?)null, Name = "Plant", ClusterId = "c1" } },
+        DriverInstances = new[]
+        {
+            new { DriverInstanceId = "drv-1", RawFolderId = "rf-plant", Name = "Modbus", DriverType = "Modbus", DriverConfig = "{}", ClusterId = "c1", Enabled = true },
+            new { DriverInstanceId = "drv-2", RawFolderId = "rf-plant", Name = "Modbus2", DriverType = "Modbus", DriverConfig = "{}", ClusterId = "c1", Enabled = true },
+        },
+        Devices = new[] { new { DeviceId = "dev-1", DriverInstanceId = "drv-1", Name = "dev1", DeviceConfig = "{}" } },
+        TagGroups = Array.Empty<object>(),
+        Tags = new[]
+        {
+            new { TagId = "t-speed", DeviceId = "dev-1", TagGroupId = (string?)null, Name = "speed", DataType = "Double", AccessLevel = 1, TagConfig = "{}" },
+        },
+    });
+
+    /// <summary>Replaces a seeded deployment's artifact in place — the ConfigDb becoming readable again
+    /// without the revision changing.</summary>
+    private static void SetArtifact(
+        IDbContextFactory<OtOpcUaConfigDbContext> db, DeploymentId deploymentId, byte[] artifact)
+    {
+        // ArtifactBlob is init-only, so the row is replaced rather than mutated: delete and re-add under
+        // the SAME id + revision. Separate SaveChanges calls so the in-memory provider does not see a
+        // delete and an insert of the same key in one change set.
+        using var ctx = db.CreateDbContext();
+        var row = ctx.Deployments.Single(d => d.DeploymentId == deploymentId.Value);
+        var rev = row.RevisionHash;
+        ctx.Deployments.Remove(row);
+        ctx.SaveChanges();
+
+        ctx.Deployments.Add(new Deployment
+        {
+            DeploymentId = deploymentId.Value,
+            RevisionHash = rev,
+            Status = DeploymentStatus.Sealed,
+            CreatedBy = "test",
+            SealedAtUtc = DateTime.UtcNow,
+            ArtifactBlob = artifact,
+        });
+        ctx.SaveChanges();
+    }
+
     /// <summary>Seeds a Sealed deployment whose <c>ArtifactBlob</c> is zero-length — byte-for-byte what the
     /// host's <c>?? Array.Empty&lt;byte&gt;()</c> hands downstream when the row cannot be found.</summary>
     private static DeploymentId SeedUnreadableDeployment(IDbContextFactory<OtOpcUaConfigDbContext> db, RevisionHash rev) =>