From b83d1ed19c81a404e7ede6e080e5751619e9a51d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 8 Jul 2026 23:33:48 -0400 Subject: [PATCH] =?UTF-8?q?fix(site-runtime):=20site-side=20compile=20fail?= =?UTF-8?q?ure=20now=20rejects=20the=20deployment=20per=20spec=20(S3)=20?= =?UTF-8?q?=E2=80=94=20synchronous=20validation=20gate=20before=20actor=20?= =?UTF-8?q?creation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validation runs on the actor thread (not off-thread as the plan sketched): piping the verdict back to self reorders concurrent deploys relative to each other and to delete/disable, breaking the redeploy-supersede FIFO ordering (SR020/SR029). A deploy is an infrequent admin command, so a brief synchronous Roslyn compile is acceptable. --- .../Actors/DeploymentManagerActor.cs | 40 +++++++++++++++++++ .../Actors/InstanceActor.cs | 4 +- .../Actors/DeploymentManagerActorTests.cs | 34 ++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs index 9b9cf6ff..b2c8d54c 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/DeploymentManagerActor.cs @@ -35,6 +35,10 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers { private readonly SiteStorageService _storage; private readonly ScriptCompilationService _compilationService; + // Pure, off-thread deploy-time compile gate. A site-side compile failure + // must reject the deployment (spec: no partial state applied), so every + // DeployInstanceCommand is validated before any actor/persistence work. + private readonly DeployCompileValidator _deployCompileValidator; private readonly SharedScriptLibrary _sharedScriptLibrary; private readonly SiteStreamManager? _streamManager; private readonly SiteRuntimeOptions _options; @@ -125,6 +129,7 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers { _storage = storage; _compilationService = compilationService; + _deployCompileValidator = new DeployCompileValidator(compilationService); _sharedScriptLibrary = sharedScriptLibrary; _streamManager = streamManager; _options = options; @@ -387,6 +392,41 @@ public class DeploymentManagerActor : ReceiveActor, IWithTimers /// apply still replies to the right actor. /// private void HandleDeploy(DeployInstanceCommand command, IActorRef replyTo) + { + // Site-side compile gate (S3): a compile failure must reject the + // deployment with NO partial state applied (no Instance Actor, no + // persisted config) — the design spec's contract. Validation runs + // synchronously on the actor thread: it is a pure prefix step of the + // deploy handler, so the existing redeploy-supersede / delete-during- + // redeploy ordering (which depends on strict mailbox FIFO) is preserved + // exactly. It is NOT run off-thread — piping the verdict back to self + // reorders concurrent deploys relative to each other and to + // delete/disable commands, breaking that ordering. A deploy is an + // infrequent admin command, so briefly holding the singleton for a pure + // Roslyn compile is acceptable; the central deployer already Asks and + // waits for the DeploymentStatusResponse. + var errors = _deployCompileValidator.Validate(command.FlattenedConfigurationJson); + if (errors.Count > 0) + { + var instanceName = command.InstanceUniqueName; + var joined = string.Join(" | ", errors); + LogDeploymentEvent("Error", instanceName, + $"Instance {instanceName} deploy rejected — site compile validation failed (deploymentId={command.DeploymentId})", + joined); + replyTo.Tell(new DeploymentStatusResponse( + command.DeploymentId, instanceName, DeploymentStatus.Failed, + "Script compilation failed on site: " + joined, DateTimeOffset.UtcNow)); + return; + } + + ProceedWithDeploy(command, replyTo); + } + + /// + /// Core deploy application (redeploy-supersede + fresh apply), reached only + /// after the synchronous compile gate in passes. + /// + private void ProceedWithDeploy(DeployInstanceCommand command, IActorRef replyTo) { var instanceName = command.InstanceUniqueName; _logger.LogInformation( diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs index f0fa95cb..0e79f3e2 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs @@ -1336,7 +1336,9 @@ public class InstanceActor : ReceiveActor /// Creates child Script Actors and Alarm Actors from the flattened configuration. /// Script Actors spawned per script definition. /// Alarm Actors spawned per alarm definition, as peers to Script Actors. - /// Compilation errors reject entire instance deployment (logged but actor still starts). + /// Compile failures are rejected at deploy time by the Deployment Manager's + /// validation gate; a failure reaching this point (startup load of a legacy + /// config) is logged and the offending script/alarm skipped. /// /// Each child is seeded from a private point-in-time snapshot /// of _attributes, NOT the live dictionary. The snapshot is taken here on diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorTests.cs index 1c6925a4..1f82cf81 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentManagerActorTests.cs @@ -232,6 +232,40 @@ public class DeploymentManagerActorTests : TestKit, IDisposable Assert.Equal("NewPump", response.InstanceUniqueName); } + // ── S3: site-side compile failure rejects the deployment (no partial state) ── + + private static string MakeConfigWithBrokenScriptJson(string instanceName) => + JsonSerializer.Serialize(new FlattenedConfiguration + { + InstanceUniqueName = instanceName, + Attributes = [new ResolvedAttribute { CanonicalName = "TestAttr", Value = "42", DataType = "Int32" }], + Scripts = [new ResolvedScript { CanonicalName = "S1", Code = "this is not C# ~~~" }] + }); + + [Fact] + public async Task Deploy_WithNonCompilingScript_ReportsFailed_AndCreatesNoInstanceActor() + { + var actor = CreateDeploymentManager(); + await Task.Delay(500); // wait for empty startup + + actor.Tell(new DeployInstanceCommand( + "dep-bad", "bad-instance", "r1", + MakeConfigWithBrokenScriptJson("bad-instance"), "admin", DateTimeOffset.UtcNow)); + + var resp = ExpectMsg(TimeSpan.FromSeconds(10)); + Assert.Equal(DeploymentStatus.Failed, resp.Status); + Assert.NotNull(resp.ErrorMessage); + Assert.Contains("compil", resp.ErrorMessage!, StringComparison.OrdinalIgnoreCase); + + // No partial state applied: instance actor must not exist … + Sys.ActorSelection(actor.Path / "bad-instance").Tell(new Identify(1)); + Assert.Null(ExpectMsg().Subject); + + // … and no config must have been persisted. + var configs = await _storage.GetAllDeployedConfigsAsync(); + Assert.DoesNotContain(configs, c => c.InstanceUniqueName == "bad-instance"); + } + // ── M1.6: site event log `deployment` category ───────────────────────── [Fact]