fix(site-runtime): refuse an enable while a deploy pipeline owns the instance; revert is_enabled only for an invalid name (review 2026-10-21 follow-up L3)

docs/requirements/Component-SiteRuntime.md

@@ -181,7 +181,7 @@ flowchart TD
 
 ### Instance Lifecycle Commands
 - **Disable**: Stops the Instance Actor and its children. Retains the deployed configuration in SQLite so the instance can be re-enabled without redeployment.
-- **Enable**: Creates a new Instance Actor from the stored configuration (same as startup).
+- **Enable**: Creates a new Instance Actor from the stored configuration (same as startup). An enable that arrives while a deploy/redeploy/delete pipeline still **owns** the instance past its compile warm (predecessor still terminating, or awaiting its persistence/init join) is refused with `A deployment for instance … is in progress; retry the enable after it completes.` — it used to race the deploy into `CreateInstanceActor`, collide with the terminating predecessor's actor name, and then revert `is_enabled`, leaving the redeployed instance durably disabled. A failed enable reverts `is_enabled` only for a name Akka refuses outright, never for a uniqueness collision on a valid name, which is transient (review 2026-10-21 follow-up L3).
 - **Delete**: Stops the Instance Actor and its children, removes the deployed configuration from local SQLite. Does **not** clear store-and-forward messages.
 - **Ensure Started** (`EnsureInstanceStartedCommand`, site-internal — never sent by central): advisory, idempotent request to make sure an already-persisted, enabled instance is actually running. Raised by the startup reconcile self-heal; see the note under **Startup Behavior**. Unlike the three commands above it has **no reply** and applies no configuration — it only starts an Instance Actor for a config that is already fully stored, and no-ops when the actor is running, the stored row says disabled, the row is absent, or a deploy for that instance is in flight.
 - **Instance reconcile sweep** (internal timer, not a command): the periodic singleton-owned check that the running Instance Actors still agree with the durable config rows — starting an enabled instance that has none, and re-applying a stored config a running instance has fallen behind. See the note under **Startup Behavior**; the start half shares its implementation (and therefore its bookkeeping and site events) with **Ensure Started**, and the restart half goes through the ordinary redeploy path.

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

@@ -1799,6 +1799,18 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
         // Warm-then-gate: see TryBufferDuringDeployWarm.
         if (TryBufferDuringDeployWarm(instanceName, command, replyTo)) return;
 
+        // Review 2026-10-21 follow-up L3: a redeploy/delete past its warm still OWNS the instance
+        // (predecessor terminating, or awaiting its persistence/init join) and will create the
+        // actor itself. An enable racing it would collide with the terminating predecessor's
+        // actor name in CreateInstanceActor — refuse it plainly instead.
+        if (InstanceIsOwnedByADeployPipeline(instanceName))
+        {
+            replyTo.Tell(new InstanceLifecycleResponse(command.CommandId, instanceName, false,
+                $"A deployment for instance {instanceName} is in progress; retry the enable after it completes.",
+                DateTimeOffset.UtcNow));
+            return;
+        }
+
         var sender = replyTo;
 
         // Row 101 (initiation half) — an is_enabled UPDATE is about to start off-thread.
@@ -1853,15 +1865,23 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
             // Lane A L3: is_enabled=1 has already committed; put it back so the durable row
             // agrees with the runtime (and the sweep does not keep re-trying it). Row 101 bumps at
             // both ends, as for every other mutation of deployed_configurations.
-            BumpStoreWriteGeneration("enable revert dispatch");
-            var revert = _storage.SetInstanceEnabledAsync(instanceName, false);
-            revert.ContinueWith(t =>
+            //
+            // Follow-up L3: ONLY for a name Akka will never accept. A uniqueness collision (a
+            // valid name still held by a terminating actor) is transient — reverting it would
+            // leave an instance the operator enabled (or a redeploy just started) durably
+            // disabled; left enabled, the sweep starts it once the name is free.
+            if (ShouldRevertEnableAfterCreateFailure(instanceName))
             {
-                if (!t.IsCompletedSuccessfully)
-                    _logger.LogError(t.Exception?.GetBaseException(),
-                        "Failed to revert is_enabled for {Instance} after a failed enable", instanceName);
-            });
-            revert.ContinueWith(_ => new StoreWriteCompleted("enable revert")).PipeTo(Self);
+                BumpStoreWriteGeneration("enable revert dispatch");
+                var revert = _storage.SetInstanceEnabledAsync(instanceName, false);
+                revert.ContinueWith(t =>
+                {
+                    if (!t.IsCompletedSuccessfully)
+                        _logger.LogError(t.Exception?.GetBaseException(),
+                            "Failed to revert is_enabled for {Instance} after a failed enable", instanceName);
+                });
+                revert.ContinueWith(_ => new StoreWriteCompleted("enable revert")).PipeTo(Self);
+            }
             result.OriginalSender.Tell(new InstanceLifecycleResponse(
                 result.Command.CommandId, instanceName, false, createError, DateTimeOffset.UtcNow));
             return;
@@ -1877,6 +1897,17 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers
         _logger.LogInformation("Instance {Instance} enabled", instanceName);
     }
 
+    /// <summary>
+    /// Whether a failed enable should put <c>is_enabled</c> back to 0: only when the instance
+    /// name is one Akka refuses outright (it can never start, so leaving it enabled would only
+    /// make the sweep retry it forever). A collision on a VALID name is transient (review
+    /// 2026-10-21 follow-up L3).
+    /// </summary>
+    /// <param name="instanceName">The instance whose enable failed to create its actor.</param>
+    /// <returns><see langword="true"/> when the enable should be reverted.</returns>
+    internal static bool ShouldRevertEnableAfterCreateFailure(string instanceName) =>
+        !ActorPath.IsValidPathElement(instanceName);
+
     /// <summary>
     /// Reconcile self-heal (deferred register row 67): makes sure an instance whose config was
     /// (re)stored out-of-band is actually RUNNING.

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

@@ -230,4 +230,57 @@ public class DeploymentManagerActorNameTests : TestKit, IDisposable
             return Task.CompletedTask;
         }, TimeSpan.FromSeconds(10));
     }
+
+    /// <summary>
+    /// Follow-up L3: an enable that lands while a redeploy owns the instance (here: queued behind
+    /// the redeploy's compile warm, so it is dispatched the moment the predecessor actor starts
+    /// terminating) is refused with a clear "deployment in progress" reply. It used to race the
+    /// deploy into <c>CreateInstanceActor</c>, collide with the terminating predecessor's name,
+    /// and then REVERT is_enabled — leaving the freshly redeployed instance disabled.
+    /// </summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task Enable_DuringRedeploy_IsRefused_AndDoesNotDisableTheInstance()
+    {
+        var dm = CreateBootedDeploymentManager();
+        Assert.Equal(DeploymentStatus.Success, Deploy(dm, "Pump1").Status);
+
+        dm.Tell(new DeployInstanceCommand(
+            $"dep-{Guid.NewGuid():N}", "Pump1", "rev-2", ConfigJson("Pump1", "S2"),
+            "tester", DateTimeOffset.UtcNow));
+        dm.Tell(new EnableInstanceCommand("cmd-en", "Pump1", DateTimeOffset.UtcNow));
+
+        InstanceLifecycleResponse? enable = null;
+        DeploymentStatusResponse? deploy = null;
+        for (var i = 0; i < 2; i++)
+        {
+            var msg = ExpectMsg<object>(TimeSpan.FromSeconds(20));
+            if (msg is InstanceLifecycleResponse l) enable = l;
+            if (msg is DeploymentStatusResponse d) deploy = d;
+        }
+
+        Assert.NotNull(enable);
+        Assert.NotNull(deploy);
+        Assert.Equal(DeploymentStatus.Success, deploy!.Status);
+        Assert.False(enable!.Success);
+        Assert.Contains("deployment", enable.ErrorMessage, StringComparison.OrdinalIgnoreCase);
+        Assert.Contains("in progress", enable.ErrorMessage, StringComparison.OrdinalIgnoreCase);
+
+        // Nothing reverts the redeployed instance to disabled.
+        await Task.Delay(500);
+        Assert.True((await _storage.GetDeployedConfigMetadataAsync("Pump1"))!.IsEnabled);
+    }
+
+    /// <summary>
+    /// Follow-up L3: only a name Akka will NEVER accept reverts is_enabled after a failed enable;
+    /// a uniqueness collision (a valid name still held by a terminating actor) is transient and
+    /// must not flip the durable row to disabled.
+    /// </summary>
+    [Theory]
+    [InlineData("a b", true)]
+    [InlineData("Pump1", false)]
+    public void EnableCreateFailure_RevertsOnlyForAnInvalidName(string instanceName, bool expectRevert)
+    {
+        Assert.Equal(expectRevert, DeploymentManagerActor.ShouldRevertEnableAfterCreateFailure(instanceName));
+    }
 }