fix(triggers): bound expression evaluation, align AlarmActor error handling, dedupe config parsing

src/ScadaLink.SiteRuntime/Actors/AlarmActor.cs

@@ -51,8 +51,8 @@ public class AlarmActor : ReceiveActor
     private readonly Script<object?>? _onTriggerCompiledScript;
 
     // Expression trigger: compiled expression + the attribute snapshot it
-    // evaluates against. The compiled expression is also held on the
+    // evaluates against. This field is the single home for the compiled
-    // ExpressionEvalConfig; this field caches it for the hot path.
+    // expression on the hot path.
     private readonly Script<object?>? _compiledTriggerExpression;
     private readonly Dictionary<string, object?> _attributeSnapshot = new();
 
@@ -369,17 +369,39 @@ public class AlarmActor : ReceiveActor
     /// Evaluates the compiled trigger expression against the current attribute
     /// snapshot, returning the resulting bool. This bool feeds the existing
     /// binary Normal↔Active state path — the alarm is active while true. A
-    /// throwing or non-bool expression is treated as false; the exception
+    /// throwing, non-bool, or timed-out expression is treated as false (logged
-    /// propagates to the caller's catch, which logs it and continues.
+    /// as an alarm error) so that the state machine still runs — an Active
+    /// alarm correctly clears if the expression starts throwing.
     /// </summary>
     private bool EvaluateExpression()
     {
         if (_compiledTriggerExpression == null) return false;
 
+        try
+        {
             var globals = new TriggerExpressionGlobals(_attributeSnapshot);
-        var state = _compiledTriggerExpression.RunAsync(globals).GetAwaiter().GetResult();
+            // Bound evaluation with a short timeout. The CancellationToken
+            // covers cooperative/async cases; a pathological CPU-bound
+            // expression is not fully interruptible. Acceptable because
+            // trigger expressions are authored by trusted Design-role users
+            // and are compile-checked pre-deployment.
+            using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(2));
+            var state = _compiledTriggerExpression
+                .RunAsync(globals, cancellationToken: cts.Token)
+                .GetAwaiter().GetResult();
             return state.ReturnValue is bool b && b;
         }
+        catch (Exception ex)
+        {
+            // OperationCanceledException (timeout) falls through here too,
+            // and is correctly treated as false.
+            _healthCollector?.IncrementAlarmError();
+            _logger.LogError(ex,
+                "Alarm {Alarm} trigger expression evaluation failed on {Instance}; treated as false",
+                _alarmName, _instanceName);
+            return false;
+        }
+    }
 
     /// <summary>
     /// HiLo level evaluator: returns the most-severe matching band for the
@@ -518,12 +540,12 @@ public class AlarmActor : ReceiveActor
                     HiHiMessage: TryReadString(root, "hiHiMessage")),
 
                 // Expression triggers have no single monitored attribute; they
-                // evaluate the compiled expression (passed into the actor) over
+                // evaluate the compiled expression (passed into the actor and
-                // the full attribute snapshot. MonitoredAttributeName is unused.
+                // cached in _compiledTriggerExpression) over the full attribute
+                // snapshot. MonitoredAttributeName is unused.
                 AlarmTriggerType.Expression => new ExpressionEvalConfig(
                     "",
-                    TryReadString(root, "expression") ?? "",
+                    TriggerExpressionGlobals.ExtractExpression(triggerConfigJson) ?? ""),
-                    _compiledTriggerExpression),
 
                 _ => new ValueMatchEvalConfig(attr, null)
             };
@@ -590,13 +612,13 @@ internal record RateOfChangeEvalConfig(
 /// <summary>
 /// Expression evaluation config: a read-only boolean C# expression evaluated
 /// over the full attribute snapshot. Has no single monitored attribute
-/// (<see cref="AlarmEvalConfig.MonitoredAttributeName"/> is empty); the
+/// (<see cref="AlarmEvalConfig.MonitoredAttributeName"/> is empty). The
-/// compiled expression is passed into the actor and cached here.
+/// compiled expression itself lives on the actor's <c>_compiledTriggerExpression</c>
+/// field, the single source for the hot path.
 /// </summary>
 internal record ExpressionEvalConfig(
     string MonitoredAttributeName,
-    string Expression,
+    string Expression) : AlarmEvalConfig(MonitoredAttributeName);
-    Script<object?>? CompiledExpression) : AlarmEvalConfig(MonitoredAttributeName);
 
 /// <summary>
 /// HiLo evaluation config: any subset of the four setpoints may be set; null

src/ScadaLink.SiteRuntime/Actors/InstanceActor.cs

@@ -1,4 +1,5 @@
 using Akka.Actor;
+using Microsoft.CodeAnalysis.Scripting;
 using Microsoft.Extensions.Logging;
 using ScadaLink.Commons.Messages.DataConnection;
 using ScadaLink.Commons.Messages.DebugView;
@@ -540,7 +541,7 @@ public class InstanceActor : ReceiveActor
         // Create Alarm Actors
         foreach (var alarm in _configuration.Alarms)
         {
-            Microsoft.CodeAnalysis.Scripting.Script<object?>? onTriggerScript = null;
+            Script<object?>? onTriggerScript = null;
 
             // Compile on-trigger script if defined
             if (!string.IsNullOrEmpty(alarm.OnTriggerScriptCanonicalName))
@@ -599,29 +600,14 @@ public class InstanceActor : ReceiveActor
     /// expression, or a compilation failure (logged) — in which case the
     /// trigger is inert and the actor still starts.
     /// </summary>
-    private Microsoft.CodeAnalysis.Scripting.Script<object?>? CompileTriggerExpression(
+    private Script<object?>? CompileTriggerExpression(
         string? triggerType, string? triggerConfigJson, string compileName)
     {
         if (!string.Equals(triggerType, "Expression", StringComparison.OrdinalIgnoreCase))
             return null;
-        if (string.IsNullOrEmpty(triggerConfigJson))
-            return null;
 
-        string? expression;
+        var expression = TriggerExpressionGlobals.ExtractExpression(triggerConfigJson);
-        try
+        if (expression == null)
-        {
-            var doc = JsonSerializer.Deserialize<JsonElement>(triggerConfigJson);
-            expression = doc.TryGetProperty("expression", out var e) ? e.GetString() : null;
-        }
-        catch (Exception ex)
-        {
-            _logger.LogWarning(ex,
-                "Failed to read trigger expression config for {Name} on {Instance}",
-                compileName, _instanceUniqueName);
-            return null;
-        }
-
-        if (string.IsNullOrWhiteSpace(expression))
             return null;
 
         var result = _compilationService.CompileTriggerExpression(compileName, expression);

src/ScadaLink.SiteRuntime/Actors/ScriptActor.cs

@@ -210,11 +210,21 @@ public class ScriptActor : ReceiveActor, IWithTimers
         try
         {
             var globals = new TriggerExpressionGlobals(_attributeSnapshot);
-            var state = _compiledTriggerExpression.RunAsync(globals).GetAwaiter().GetResult();
+            // Bound evaluation with a short timeout. The CancellationToken
+            // covers cooperative/async cases; a pathological CPU-bound
+            // expression is not fully interruptible. Acceptable because
+            // trigger expressions are authored by trusted Design-role users
+            // and are compile-checked pre-deployment.
+            using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(2));
+            var state = _compiledTriggerExpression
+                .RunAsync(globals, cancellationToken: cts.Token)
+                .GetAwaiter().GetResult();
             result = state.ReturnValue is bool b && b;
         }
         catch (Exception ex)
         {
+            // OperationCanceledException (timeout) falls through here too,
+            // and is correctly treated as false.
             LogExpressionError(ex);
             result = false;
         }
@@ -346,16 +356,8 @@ public class ScriptActor : ReceiveActor, IWithTimers
 
     private static ExpressionTriggerConfig? ParseExpressionTrigger(string? json)
     {
-        if (string.IsNullOrEmpty(json)) return null;
+        var expr = TriggerExpressionGlobals.ExtractExpression(json);
-        try
+        return expr == null ? null : new ExpressionTriggerConfig(expr);
-        {
-            var doc = JsonDocument.Parse(json);
-            var expr = doc.RootElement.TryGetProperty("expression", out var e)
-                ? e.GetString()
-                : null;
-            return string.IsNullOrWhiteSpace(expr) ? null : new ExpressionTriggerConfig(expr);
-        }
-        catch { return null; }
     }
 
     private static IntervalTriggerConfig? ParseIntervalTrigger(string? json)

src/ScadaLink.SiteRuntime/Scripts/TriggerExpressionGlobals.cs

@@ -1,3 +1,5 @@
+using System.Text.Json;
+
 namespace ScadaLink.SiteRuntime.Scripts;
 
 /// <summary>
@@ -11,6 +13,29 @@ namespace ScadaLink.SiteRuntime.Scripts;
 /// </summary>
 public sealed class TriggerExpressionGlobals
 {
+    /// <summary>
+    /// Extracts the <c>"expression"</c> field from an Expression-trigger config
+    /// JSON document. Returns <c>null</c> for a missing, blank, or malformed
+    /// config — the single parsing idiom shared by InstanceActor, ScriptActor,
+    /// and AlarmActor.
+    /// </summary>
+    public static string? ExtractExpression(string? triggerConfigJson)
+    {
+        if (string.IsNullOrEmpty(triggerConfigJson)) return null;
+        try
+        {
+            using var doc = JsonDocument.Parse(triggerConfigJson);
+            var expr = doc.RootElement.TryGetProperty("expression", out var e)
+                ? e.GetString()
+                : null;
+            return string.IsNullOrWhiteSpace(expr) ? null : expr;
+        }
+        catch (JsonException)
+        {
+            return null;
+        }
+    }
+
     private readonly IReadOnlyDictionary<string, object?> _snapshot;
 
     public TriggerExpressionGlobals(IReadOnlyDictionary<string, object?> snapshot)