fix(site-runtime): deployment whose Instance Actor dies during init reports Failed, exactly-once reply (S6)

Success now requires BOTH persistence commit AND an InstanceActorInitialized
readiness signal from the actor's PreStart — persistence can commit before the
actor's async init has run or failed, so persistence alone must not report
Success. An actor that dies during init never signals readiness; the Terminated
fallback fails that deployment and rolls back the optimistic state. The
persisted-row rollback is deferred until the store commits so it cannot race the
optimistic write.

Deviation from plan Task 15: the plan's swallow-only guard did not handle the
persistence-first ordering (empirically the store commits before the Terminated
signal, so Success was reported for a dead actor). Added the readiness handshake
to make the join deterministic.

src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs

@@ -110,6 +110,30 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
     /// </summary>
     private readonly HashSet<string> _deployedInstanceNames = new();
 
+    /// <summary>
+    /// Deployments awaiting their persistence result, keyed by instance name (S6).
+    /// The entry is added when the Instance Actor is created optimistically and removed
+    /// by whichever of two paths fires first:
+    /// <list type="bullet">
+    /// <item><see cref="HandleDeployPersistenceResult"/> on a normal persistence outcome,
+    /// which then replies Success/Failed;</item>
+    /// <item>the <see cref="HandleTerminated"/> S6 fallback when the actor dies during
+    /// init, which replies Failed and rolls back the optimistic state.</item>
+    /// </list>
+    /// Removing the entry in exactly one of those paths guarantees the deployer receives
+    /// exactly one <see cref="DeploymentStatusResponse"/> regardless of ordering.
+    /// </summary>
+    private readonly Dictionary<string, InFlightDeploy> _inFlightDeployments = new();
+
+    /// <summary>
+    /// Instances whose deployment was failed by the S6 Terminated fallback WHILE their
+    /// optimistic <c>StoreDeployedConfigAsync</c> was still in flight. The persisted-row
+    /// rollback must run AFTER the store commits (the two are independent background
+    /// tasks with no ordering guarantee), so the removal is deferred here and executed by
+    /// <see cref="HandleDeployPersistenceResult"/> when the store's result finally lands.
+    /// </summary>
+    private readonly HashSet<string> _initFailedPendingRowRemoval = new();
+
     /// <summary>Akka timer scheduler injected by the framework via <see cref="IWithTimers"/>.</summary>
     public ITimerScheduler Timers { get; set; } = null!;
 
@@ -266,6 +290,10 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
         // Internal deploy persistence result
         Receive<DeployPersistenceResult>(HandleDeployPersistenceResult);
 
+        // Instance Actor init confirmation — the second half of the deploy-success
+        // join (S6). Success is only reported once both this and persistence land.
+        Receive<InstanceActorInitialized>(HandleInstanceActorInitialized);
+
         // Terminated signal — drains a buffered redeployment once the previous
         // Instance Actor has fully stopped.
         Receive<Terminated>(HandleTerminated);
@@ -567,6 +595,33 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
                 name);
             LogDeploymentEvent("Error", name,
                 $"Instance {name} terminated unexpectedly (init failure or crash) — removed from routing");
+
+            // S6: if the death happened while a deployment was still in flight, the
+            // actor died during init — fail that deployment (the persistence path must
+            // NOT later report Success) and roll back the optimistically-applied state
+            // so the site never advertises an instance it cannot durably recover.
+            if (_inFlightDeployments.Remove(name, out var inflight))
+            {
+                inflight.ReplyTo.Tell(new DeploymentStatusResponse(
+                    inflight.DeploymentId, name, DeploymentStatus.Failed,
+                    "Instance actor failed during initialization — see site event log",
+                    DateTimeOffset.UtcNow));
+
+                if (!inflight.IsRedeploy)
+                {
+                    _deployedInstanceNames.Remove(name);
+                    UpdateInstanceCounts();
+
+                    // Remove the optimistically-persisted config row so a restart does
+                    // not re-create the failing actor. If the store already committed
+                    // (Persisted), remove it now; otherwise defer the removal until the
+                    // in-flight store completes so the remove cannot race ahead of it.
+                    if (inflight.Persisted)
+                        RollbackPersistedConfig(name);
+                    else
+                        _initFailedPendingRowRemoval.Add(name);
+                }
+            }
         }
     }
 
@@ -665,6 +720,16 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
             _deployedInstanceNames.Add(instanceName);
         UpdateInstanceCounts();
 
+        // Record the in-flight deployment so the deployer is answered exactly once:
+        // Success only once BOTH persistence commits AND the actor confirms init, or
+        // Failed via the S6 Terminated fallback if the Instance Actor dies during init.
+        _inFlightDeployments[instanceName] = new InFlightDeploy
+        {
+            DeploymentId = command.DeploymentId,
+            ReplyTo = sender,
+            IsRedeploy = isRedeploy
+        };
+
         // Persist to SQLite and clear static overrides asynchronously
         Task.Run(async () =>
         {
@@ -704,29 +769,44 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
     }
 
     /// <summary>
-    /// Reports the deployment outcome to central only after the
-    /// persistence result is known. On a persistence failure the Instance Actor that was
-    /// created optimistically is stopped and the deployed-instance counter rolled back,
-    /// so the in-memory state stays consistent with durable storage, and central is told
-    /// the deployment <see cref="DeploymentStatus.Failed"/>.
+    /// Records the persistence half of the deploy-success join. On a persistence
+    /// failure the optimistically-created Instance Actor is stopped and the counter
+    /// rolled back and the deployer is told <see cref="DeploymentStatus.Failed"/>. On
+    /// success the in-flight entry is marked persisted and success is reported only once
+    /// the actor has ALSO confirmed init (<see cref="TryCompleteDeploy"/>) — persistence
+    /// can commit before the actor's async init has run or failed (S6).
     /// </summary>
     private void HandleDeployPersistenceResult(DeployPersistenceResult result)
     {
-        if (result.Success)
+        // If the S6 Terminated fallback already failed this deployment (the Instance
+        // Actor died during init), the in-flight entry is gone — swallow the now-stale
+        // persistence result so the deployer is answered exactly once.
+        if (!_inFlightDeployments.TryGetValue(result.InstanceName, out var inflight))
         {
-            // Operational `deployment` event — deploy succeeded.
-            LogDeploymentEvent("Info", result.InstanceName,
-                $"Instance {result.InstanceName} deployed (deploymentId={result.DeploymentId})");
+            // The fallback deferred the persisted-row rollback because the store was
+            // still in flight then. The store has now committed (Success), so remove
+            // the orphan row — guaranteed to run after the store, never racing it.
+            if (_initFailedPendingRowRemoval.Remove(result.InstanceName) && result.Success)
+                RollbackPersistedConfig(result.InstanceName);
 
-            result.OriginalSender.Tell(new DeploymentStatusResponse(
-                result.DeploymentId,
-                result.InstanceName,
-                DeploymentStatus.Success,
-                null,
-                DateTimeOffset.UtcNow));
+            _logger.LogDebug(
+                "Persistence result for {Instance} arrived after the deployment was already " +
+                "resolved (init-time termination) — ignoring (deploymentId={DeploymentId}, success={Success})",
+                result.InstanceName, result.DeploymentId, result.Success);
             return;
         }
 
+        if (result.Success)
+        {
+            // Persistence committed — record it and complete only if init also confirmed.
+            inflight.Persisted = true;
+            TryCompleteDeploy(result.InstanceName, inflight);
+            return;
+        }
+
+        // Persistence genuinely failed — fail now regardless of init state.
+        _inFlightDeployments.Remove(result.InstanceName);
+
         _logger.LogError(
             "Failed to persist deployment {DeploymentId} for {Instance}: {Error}",
             result.DeploymentId, result.InstanceName, result.Error);
@@ -736,15 +816,15 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
             $"Instance {result.InstanceName} deploy failed (deploymentId={result.DeploymentId})",
             result.Error);
 
-        // Persistence failed — undo the optimistic actor creation and counter bump so
-        // the site does not advertise an instance it cannot durably recover.
+        // Undo the optimistic actor creation and counter bump so the site does not
+        // advertise an instance it cannot durably recover.
         if (_instanceActors.Remove(result.InstanceName, out var orphan))
             Context.Stop(orphan);
         if (!result.IsRedeploy)
             _deployedInstanceNames.Remove(result.InstanceName);
         UpdateInstanceCounts();
 
-        result.OriginalSender.Tell(new DeploymentStatusResponse(
+        inflight.ReplyTo.Tell(new DeploymentStatusResponse(
             result.DeploymentId,
             result.InstanceName,
             DeploymentStatus.Failed,
@@ -752,6 +832,61 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
             DateTimeOffset.UtcNow));
     }
 
+    /// <summary>
+    /// Records the init half of the deploy-success join (S6): the Instance Actor
+    /// finished PreStart and sent <see cref="InstanceActorInitialized"/>. Success is
+    /// reported only once persistence has ALSO committed. Actors created outside a
+    /// deployment (startup, enable) have no in-flight entry and are ignored here.
+    /// </summary>
+    private void HandleInstanceActorInitialized(InstanceActorInitialized msg)
+    {
+        if (_inFlightDeployments.TryGetValue(msg.InstanceUniqueName, out var inflight))
+        {
+            inflight.Initialized = true;
+            TryCompleteDeploy(msg.InstanceUniqueName, inflight);
+        }
+    }
+
+    /// <summary>
+    /// Completes a deployment with <see cref="DeploymentStatus.Success"/> once BOTH the
+    /// persistence result and the Instance Actor init confirmation have landed (S6).
+    /// A no-op until both signals are present.
+    /// </summary>
+    private void TryCompleteDeploy(string instanceName, InFlightDeploy inflight)
+    {
+        if (!inflight.Persisted || !inflight.Initialized)
+            return;
+
+        _inFlightDeployments.Remove(instanceName);
+
+        // Operational `deployment` event — deploy succeeded.
+        LogDeploymentEvent("Info", instanceName,
+            $"Instance {instanceName} deployed (deploymentId={inflight.DeploymentId})");
+
+        inflight.ReplyTo.Tell(new DeploymentStatusResponse(
+            inflight.DeploymentId,
+            instanceName,
+            DeploymentStatus.Success,
+            null,
+            DateTimeOffset.UtcNow));
+    }
+
+    /// <summary>
+    /// Fire-and-forget removal of an optimistically-persisted deployed-config row after
+    /// an init failure (S6). Only ever called once the row's write has committed, so it
+    /// cannot race the store. A fault is logged, never thrown.
+    /// </summary>
+    private void RollbackPersistedConfig(string instanceName)
+    {
+        Task.Run(() => _storage.RemoveDeployedConfigAsync(instanceName)).ContinueWith(t =>
+        {
+            if (!t.IsCompletedSuccessfully)
+                _logger.LogError(t.Exception?.GetBaseException(),
+                    "Failed to roll back the persisted config for {Instance} after an init failure",
+                    instanceName);
+        });
+    }
+
     /// <summary>
     /// Disables an instance: stops the actor and marks as disabled in SQLite.
     /// </summary>
@@ -1827,6 +1962,31 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
     /// </summary>
     internal record PendingRedeploy(DeployInstanceCommand Command, IActorRef OriginalSender);
 
+    /// <summary>
+    /// Join state for a deployment awaiting confirmation. Deployment <c>Success</c> is
+    /// reported only once BOTH signals land: <see cref="Persisted"/> (the config committed
+    /// to SQLite) AND <see cref="Initialized"/> (the Instance Actor finished PreStart and
+    /// sent <see cref="Messages.InstanceActorInitialized"/>). Persistence can commit before
+    /// the actor's asynchronous init has run or failed, so persistence alone is not enough
+    /// to declare success (S6). An actor that dies during init never sets
+    /// <see cref="Initialized"/>; the S6 Terminated fallback fails that deployment instead.
+    /// <see cref="IsRedeploy"/> gates rollback so a failed init rolls back the
+    /// deployed-instance set and the persisted row only for a genuinely new deployment.
+    /// </summary>
+    internal sealed class InFlightDeploy
+    {
+        /// <summary>The deployment id echoed back in the status response.</summary>
+        public required string DeploymentId { get; init; }
+        /// <summary>The deployer to answer exactly once.</summary>
+        public required IActorRef ReplyTo { get; init; }
+        /// <summary>Whether this is a redeploy (an update) rather than a new deployment.</summary>
+        public required bool IsRedeploy { get; init; }
+        /// <summary>Set when the config has committed to SQLite.</summary>
+        public bool Persisted { get; set; }
+        /// <summary>Set when the Instance Actor has finished PreStart successfully.</summary>
+        public bool Initialized { get; set; }
+    }
+
     /// <summary>
     /// Notify-and-fetch: piped back to self when the deployment's flattened
     /// config has been fetched from central. Carries the original central sender so the

src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs

@@ -262,6 +262,12 @@ public class InstanceActor : ReceiveActor
 
         // Subscribe to DCL for data-sourced attributes
         SubscribeToDcl();
+
+        // Init succeeded — tell the Deployment Manager so it can confirm the
+        // deployment (S6). If any step above throws, PreStart aborts before this
+        // line and the actor is stopped with ActorInitializationException instead,
+        // so the DM never sees a readiness signal for an instance that failed to init.
+        Context.Parent.Tell(new InstanceActorInitialized(_instanceUniqueName));
     }
 
     /// <inheritdoc />

src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Messages/InstanceActorInitialized.cs

@@ -0,0 +1,19 @@
+namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Messages;
+
+/// <summary>
+/// Readiness signal an <c>InstanceActor</c> sends to its parent
+/// <c>DeploymentManagerActor</c> at the very end of a successful <c>PreStart</c> —
+/// after its child Script/Alarm/NativeAlarm actors have been created and its DCL
+/// subscriptions issued. If <c>PreStart</c> throws before this point (e.g. a child
+/// actor cannot be created), the message is never sent and the actor is stopped
+/// with an <c>ActorInitializationException</c> instead.
+///
+/// The Deployment Manager uses it to confirm a deployment only once the Instance
+/// Actor has actually initialized — SQLite persistence completing is necessary but
+/// NOT sufficient, because persistence can commit before the actor's asynchronous
+/// init has run (or failed). Reporting deployment <c>Success</c> is therefore gated
+/// on BOTH the persistence result AND this signal (S6); an actor that dies during
+/// init never sends it, so its deployment is failed by the Terminated fallback.
+/// </summary>
+/// <param name="InstanceUniqueName">The unique name of the initialized instance.</param>
+public sealed record InstanceActorInitialized(string InstanceUniqueName);

tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerRedeployTests.cs

@@ -360,4 +360,38 @@ public class DeploymentManagerRedeployTests : TestKit, IDisposable
                 r.Message.Contains("terminated unexpectedly", StringComparison.OrdinalIgnoreCase));
         }, TimeSpan.FromSeconds(10));
     }
+
+    [Fact]
+    public async Task Deploy_WhoseActorDiesDuringInit_ReportsFailed_NotSuccess()
+    {
+        // A deploy whose config compiles (passing the compile gate) but whose Instance
+        // Actor dies during init must report Failed — NOT the Success it used to report
+        // once SQLite persisted, leaving the deployer believing a dead instance is live
+        // (S6). The reply must be exactly-once.
+        var actor = CreateDeploymentManager();
+        await Task.Delay(500); // empty startup
+
+        actor.Tell(new DeployInstanceCommand(
+            "dep-dies", "dies-on-init", "r1",
+            CtorThrowingConfig("dies-on-init"), "admin", DateTimeOffset.UtcNow));
+
+        var resp = ExpectMsg<DeploymentStatusResponse>(TimeSpan.FromSeconds(10));
+        Assert.Equal(DeploymentStatus.Failed, resp.Status);
+
+        // Exactly one reply — no late Success from the persistence path.
+        ExpectNoMsg(TimeSpan.FromMilliseconds(500));
+
+        // The optimistically-persisted config row is rolled back so a restart does not
+        // re-create the failing actor. The rollback is fire-and-forget once the store
+        // commits, so poll for it rather than reading once.
+        List<DeployedInstance> configs = [];
+        for (var i = 0; i < 50; i++)
+        {
+            configs = await _storage.GetAllDeployedConfigsAsync();
+            if (configs.All(c => c.InstanceUniqueName != "dies-on-init"))
+                break;
+            await Task.Delay(100);
+        }
+        Assert.DoesNotContain(configs, c => c.InstanceUniqueName == "dies-on-init");
+    }
 }