From c8b5871782731fa95f0ba810bdde8840b74639ba Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 19 May 2026 03:44:34 -0400 Subject: [PATCH] fix(notification-outbox): re-align Central UI sandbox Notify API with production The script-analysis sandbox Notify surface was stale after the Notification Outbox change: SandboxNotifyTarget.Send returned Task and there was no Status method, while production NotifyTarget.Send returns Task (a NotificationId) plus NotifyHelper.Status. A script that test-ran cleanly in the sandbox would not compile against the real site runtime. - Move the NotificationDeliveryStatus record from ScadaLink.SiteRuntime.Scripts into ScadaLink.Commons.Messages.Notification so both production and the CentralUI sandbox reference the exact same type (CentralUI does not, and should not, reference SiteRuntime). Production NotifyHelper.Status is otherwise untouched. - Rewrite SandboxNotifyHelper/SandboxNotifyTarget to be a signature-faithful no-op fake: Send returns Task (a fake NotificationId), Status returns Task. Production now enqueues into the site S&F engine, which has no central-side equivalent in the sandbox, so the fake no longer carries an INotificationDeliveryService. - Add script-analysis tests proving a script using the new Notify shape both diagnoses clean and runs in the sandbox. --- .../ScriptAnalysis/SandboxHostHelpers.cs | 52 ++++++++++++++----- .../ScriptAnalysis/SandboxScriptHost.cs | 12 ++--- .../ScriptAnalysis/ScriptAnalysisService.cs | 18 ++++--- .../Notification/NotificationMessages.cs | 15 ++++++ .../Scripts/ScriptRuntimeContext.cs | 15 ------ .../ScriptAnalysisServiceTests.cs | 36 +++++++++++++ 6 files changed, 106 insertions(+), 42 deletions(-) diff --git a/src/ScadaLink.CentralUI/ScriptAnalysis/SandboxHostHelpers.cs b/src/ScadaLink.CentralUI/ScriptAnalysis/SandboxHostHelpers.cs index 4167b30..0ba317e 100644 --- a/src/ScadaLink.CentralUI/ScriptAnalysis/SandboxHostHelpers.cs +++ b/src/ScadaLink.CentralUI/ScriptAnalysis/SandboxHostHelpers.cs @@ -1,5 +1,6 @@ using System.Data.Common; using ScadaLink.Commons.Interfaces.Services; +using ScadaLink.Commons.Messages.Notification; namespace ScadaLink.CentralUI.ScriptAnalysis; @@ -80,39 +81,62 @@ public class SandboxDatabaseHelper } } +/// +/// Sandbox mirror of ScadaLink.SiteRuntime.Scripts.NotifyHelper — the +/// Notify global. Signature-faithful to production so the same user code +/// (Notify.To(...).Send(...) / Notify.Status(...)) compiles +/// identically against both surfaces. +/// +/// In the Notification Outbox design production no longer delivers notification +/// email inline — Notify.Send enqueues into the site Store-and-Forward +/// Engine and returns a NotificationId. The sandbox has no S&F engine +/// and no central, so it is a pure no-op fake: Send returns a generated +/// fake id and Status returns a placeholder . +/// Nothing is delivered. +/// public class SandboxNotifyHelper { - private readonly INotificationDeliveryService? _service; private readonly string _instanceName; - public SandboxNotifyHelper(INotificationDeliveryService? service, string instanceName) + public SandboxNotifyHelper(string instanceName) { - _service = service; _instanceName = instanceName; } + /// Selects the notification list to send to. public SandboxNotifyTarget To(string listName) => - new(listName, _service, _instanceName); + new(listName, _instanceName); + + /// + /// Queries the delivery status of a previously-sent notification. The + /// sandbox never delivers, so this always reports the placeholder + /// Unknown status — it exists for signature fidelity with + /// NotifyHelper.Status. + /// + public Task Status(string notificationId) => + Task.FromResult(new NotificationDeliveryStatus("Unknown", 0, null, null)); } +/// +/// Sandbox mirror of ScadaLink.SiteRuntime.Scripts.NotifyTarget — the +/// target of Notify.To("listName"). +/// public class SandboxNotifyTarget { private readonly string _listName; - private readonly INotificationDeliveryService? _service; private readonly string _instanceName; - internal SandboxNotifyTarget(string listName, INotificationDeliveryService? service, string instanceName) + internal SandboxNotifyTarget(string listName, string instanceName) { _listName = listName; - _service = service; _instanceName = instanceName; } - public Task Send(string subject, string message, CancellationToken cancellationToken = default) - { - if (_service == null) - throw new ScriptSandboxException( - $"Notify.To(\"{_listName}\").Send(...) — notification service not configured for Test Run."); - return _service.SendAsync(_listName, subject, message, _instanceName, cancellationToken); - } + /// + /// Mirrors NotifyTarget.Send — returns a NotificationId. In + /// the sandbox nothing is enqueued or delivered; a fake id is returned so + /// the call type-checks identically to production. + /// + public Task Send(string subject, string message, CancellationToken cancellationToken = default) => + Task.FromResult(Guid.NewGuid().ToString("N")); } diff --git a/src/ScadaLink.CentralUI/ScriptAnalysis/SandboxScriptHost.cs b/src/ScadaLink.CentralUI/ScriptAnalysis/SandboxScriptHost.cs index e3cfc0b..d4027c4 100644 --- a/src/ScadaLink.CentralUI/ScriptAnalysis/SandboxScriptHost.cs +++ b/src/ScadaLink.CentralUI/ScriptAnalysis/SandboxScriptHost.cs @@ -13,9 +13,9 @@ namespace ScadaLink.CentralUI.ScriptAnalysis; /// instance. With no instance bound they throw ; /// with one bound (see ) they route to it. /// -/// ExternalSystem, Database, Notify, and -/// Scripts.CallShared run against central's real services and fire for -/// real — they do not depend on a bound instance. +/// ExternalSystem, Database, and Scripts.CallShared run +/// against central's real services and fire for real; Notify is a +/// signature-faithful no-op fake. None of them depend on a bound instance. /// public class SandboxScriptHost { @@ -58,8 +58,8 @@ public interface ISandboxInstanceGateway /// the Instance global. Attribute and sibling-script access needs a real /// deployed instance: with no gateway wired it throws; with one (a bound /// instance) it routes cross-site. ExternalSystem/Database/ -/// Notify/Scripts run against central's real services regardless -/// of binding. +/// Scripts run against central's real services regardless of binding; +/// Notify is a signature-faithful no-op fake. /// public class SandboxInstanceContext { @@ -80,7 +80,7 @@ public class SandboxInstanceContext _gateway = gateway; ExternalSystem = external ?? new SandboxExternalHelper(null, ""); Database = database ?? new SandboxDatabaseHelper(null, ""); - Notify = notify ?? new SandboxNotifyHelper(null, ""); + Notify = notify ?? new SandboxNotifyHelper(""); Scripts = scripts ?? new SandboxScriptCallHelper(null); } diff --git a/src/ScadaLink.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs b/src/ScadaLink.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs index 0606775..a046ec6 100644 --- a/src/ScadaLink.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs +++ b/src/ScadaLink.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs @@ -154,11 +154,12 @@ public class ScriptAnalysisService /// scripts against . /// Pure logic + the supplied Parameters always work. /// For the SandboxScriptHost surface, Attributes still throws while - /// External, Database, and Notify are wired to - /// central's real , - /// , and - /// — calls fire for real and - /// have production-equivalent side effects (HTTP, SQL, SMTP). + /// External and Database are wired to central's real + /// and — + /// calls fire for real and have production-equivalent side effects (HTTP, + /// SQL). Notify is a signature-faithful no-op fake (production + /// enqueues into the site Store-and-Forward Engine, which has no + /// central-side equivalent in the sandbox). /// CallShared compiles and executes the named shared script in the /// same sandbox, with a recursion limit of /// . CallScript still throws @@ -269,10 +270,13 @@ public class ScriptAnalysisService var externalClient = _services.GetService(); var databaseGateway = _services.GetService(); - var notifyService = _services.GetService(); var external = new SandboxExternalHelper(externalClient, instanceLabel); var database = new SandboxDatabaseHelper(databaseGateway, instanceLabel); - var notify = new SandboxNotifyHelper(notifyService, instanceLabel); + // The Notification Outbox sandbox Notify is a pure no-op fake — it + // mirrors production signatures so scripts compile identically, but it + // does not deliver (production now enqueues into the site S&F engine, + // which has no central-side equivalent here). + var notify = new SandboxNotifyHelper(instanceLabel); var compileCache = new Dictionary>(StringComparer.Ordinal); var compileCacheLock = new object(); diff --git a/src/ScadaLink.Commons/Messages/Notification/NotificationMessages.cs b/src/ScadaLink.Commons/Messages/Notification/NotificationMessages.cs index d5652ea..718e1b6 100644 --- a/src/ScadaLink.Commons/Messages/Notification/NotificationMessages.cs +++ b/src/ScadaLink.Commons/Messages/Notification/NotificationMessages.cs @@ -40,3 +40,18 @@ public record NotificationStatusResponse( int RetryCount, string? LastError, DateTimeOffset? DeliveredAt); + +/// +/// Notification Outbox: the delivery status of a notification, as returned to a +/// script by Notify.Status(id). +/// +/// is either a central status (Pending, Retrying, +/// Delivered, Parked, Discarded), the site-local Forwarding +/// state (the notification is still buffered at the site and has not yet been +/// forwarded/acked), or Unknown (no central row and not buffered locally). +/// +public record NotificationDeliveryStatus( + string Status, + int RetryCount, + string? LastError, + DateTimeOffset? DeliveredAt); diff --git a/src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs b/src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs index 1b6b593..273e59b 100644 --- a/src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs +++ b/src/ScadaLink.SiteRuntime/Scripts/ScriptRuntimeContext.cs @@ -529,18 +529,3 @@ public class ScriptRuntimeContext } } } - -/// -/// Notification Outbox: the delivery status of a notification, as returned to a -/// script by Notify.Status(id). -/// -/// is either a central status (Pending, Retrying, -/// Delivered, Parked, Discarded), the site-local Forwarding -/// state (the notification is still buffered at the site and has not yet been -/// forwarded/acked), or Unknown (no central row and not buffered locally). -/// -public record NotificationDeliveryStatus( - string Status, - int RetryCount, - string? LastError, - DateTimeOffset? DeliveredAt); diff --git a/tests/ScadaLink.CentralUI.Tests/ScriptAnalysis/ScriptAnalysisServiceTests.cs b/tests/ScadaLink.CentralUI.Tests/ScriptAnalysis/ScriptAnalysisServiceTests.cs index ecca4b1..0c75bb0 100644 --- a/tests/ScadaLink.CentralUI.Tests/ScriptAnalysis/ScriptAnalysisServiceTests.cs +++ b/tests/ScadaLink.CentralUI.Tests/ScriptAnalysis/ScriptAnalysisServiceTests.cs @@ -466,6 +466,42 @@ public class ScriptAnalysisServiceTests Assert.Equal("42", result.ReturnValueJson); } + [Fact] + public void NotifyOutboxShape_DiagnosesClean() + { + // Notification Outbox: the sandbox Notify surface must be + // signature-faithful to production NotifyHelper/NotifyTarget — + // Send returns Task (a NotificationId) and Status takes that + // id. A script using the new shape must compile clean in the sandbox, + // exactly as it would against the real site runtime. + var resp = _svc.Diagnose(new DiagnoseRequest( + "var id = await Notify.To(\"ops\").Send(\"subj\", \"body\"); " + + "var st = await Notify.Status(id); " + + "return st.Status;")); + + Assert.DoesNotContain(resp.Markers, m => m.Code.StartsWith("CS")); + Assert.DoesNotContain(resp.Markers, m => m.Code.StartsWith("SCADA")); + } + + [Fact] + public async Task RunInSandbox_NotifyOutboxShape_StillRuns() + { + // The new Notify shape must also run end-to-end in the no-op sandbox: + // Send yields a fake NotificationId, Status yields a placeholder + // NotificationDeliveryStatus. + var result = await _svc.RunInSandboxAsync( + new SandboxRunRequest( + "var id = await Notify.To(\"ops\").Send(\"subj\", \"body\"); " + + "var st = await Notify.Status(id); " + + "return st.Status;", + Parameters: null, + TimeoutSeconds: null), + CancellationToken.None); + + Assert.True(result.Success); + Assert.Equal("\"Unknown\"", result.ReturnValueJson); + } + [Fact] public async Task RunInSandbox_CapturesConsoleOutput() {