From 5dabafecdc94a9f5f0139c85cbd4b1ccf1ce9cba Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 15:15:11 -0400 Subject: [PATCH] fix(calc): log a clamped per-driver run-guard value; document the Scripting ranges Review m-2. A Calculation driver's own maxAllocBytesPerRun / quarantineAfterConsecutiveTimeouts used to be clamped silently by ScriptRunGuard. ResolveScripting now sanitizes the layered options itself and returns the adjustments, and the factory logs one Warning per clamped value, naming the driver. docs/ScriptEditor.md "Runtime safety" gains the key table: defaults, the 1 MB allocation floor, the 1000 caps, NaN and disable semantics, and the per-driver override. docs/Configuration.md has no Scripting section, so these keys have no other home. Refs #731 #734 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- docs/ScriptEditor.md | 15 +++++++++++++ .../CalculationDriverFactoryExtensions.cs | 21 +++++++++++++------ ...culationScriptingOptionsResolutionTests.cs | 21 +++++++++++++++---- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/docs/ScriptEditor.md b/docs/ScriptEditor.md index 25fd73e2..e039653c 100644 --- a/docs/ScriptEditor.md +++ b/docs/ScriptEditor.md @@ -209,6 +209,21 @@ scripted-alarm predicates) runs scripts through the shared `ScriptRunGuard`: `Scripting:MaxAllocBytesPerRun` (64 MB) is quarantined. This is sampled after the run returns, so a run that OOMs the process first is not stopped — that needs an out-of-process runner. +- **Configuration keys and their ranges.** All three knobs are node-wide (the + `Scripting` appsettings section, shared by virtual tags, Calculation tags and + scripted alarms) and are clamped when bound, with one startup Warning per + clamped value: + + | Key | Default | Range after the clamp | + |---|---|---| + | `Scripting:MaxAllocBytesPerRun` | `67108864` (64 MB) | `0` (or negative) disables the budget; a positive value below `1048576` (1 MB) becomes 1 MB | + | `Scripting:QuarantineAfterConsecutiveTimeouts` | `3` | `1`–`1000` | + | `Scripting:StuckOrphanQuarantineFactor` | `10` | `0` (or negative) disables the rule; above `1000` becomes `1000`; `NaN` becomes the default | + + A Calculation driver may override `maxAllocBytesPerRun` and + `quarantineAfterConsecutiveTimeouts` in its own driver config. Those keys are + layered on the node values and clamped to the same ranges, and an out-of-range + value logs a Warning naming the driver. The stuck factor has no per-driver key. - **Lifting.** A quarantine lifts when the script source changes or on redeploy, never by itself. - **Script-log rate limit.** The cluster-wide script-log fan-out admits 20 entries diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriverFactoryExtensions.cs index ce7817cf..2c417de1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriverFactoryExtensions.cs @@ -86,7 +86,14 @@ public static class CalculationDriverFactoryExtensions ?? throw new InvalidOperationException( $"Calculation driver config for '{driverInstanceId}' deserialised to null"); - var scripting = ResolveScripting(dto.MaxAllocBytesPerRun, dto.QuarantineAfterConsecutiveTimeouts, nodeScripting); + var scripting = ResolveScripting( + dto.MaxAllocBytesPerRun, dto.QuarantineAfterConsecutiveTimeouts, nodeScripting, out var adjustments); + var logger = loggerFactory?.CreateLogger(); + // An authored value is never changed silently: the run guard would clamp it anyway, without a word. + foreach (var adjustment in adjustments) + logger?.LogWarning( + "Calculation driver {DriverInstanceId}: its run-guard config was out of range and is clamped: {Adjustment}", + driverInstanceId, adjustment); var options = new CalculationDriverOptions { @@ -96,9 +103,7 @@ public static class CalculationDriverFactoryExtensions Scripting = scripting, }; - return new CalculationDriver( - options, driverInstanceId, scriptRoot, - logger: loggerFactory?.CreateLogger()); + return new CalculationDriver(options, driverInstanceId, scriptRoot, logger: logger); } /// @@ -109,10 +114,14 @@ public static class CalculationDriverFactoryExtensions /// The driver config's maxAllocBytesPerRun, or null. /// The driver config's quarantineAfterConsecutiveTimeouts, or null. /// The node-wide options, or null. + /// The clamps applied to the driver's OWN keys (the node options were already + /// clamped and logged at bind), for the caller to log. Empty when nothing was out of range. /// The options to hand the driver, or null. internal static ScriptingOptions? ResolveScripting( - long? maxAllocBytesPerRun, int? quarantineAfterConsecutiveTimeouts, ScriptingOptions? nodeScripting) + long? maxAllocBytesPerRun, int? quarantineAfterConsecutiveTimeouts, ScriptingOptions? nodeScripting, + out IReadOnlyList adjustments) { + adjustments = []; if (maxAllocBytesPerRun is null && quarantineAfterConsecutiveTimeouts is null) return nodeScripting; var resolved = new ScriptingOptions(); @@ -124,7 +133,7 @@ public static class CalculationDriverFactoryExtensions } if (maxAllocBytesPerRun is { } alloc) resolved.MaxAllocBytesPerRun = alloc; if (quarantineAfterConsecutiveTimeouts is { } n) resolved.QuarantineAfterConsecutiveTimeouts = n; - return resolved; + return resolved.Sanitized(out adjustments); } /// The merged-config DTO the calc factory binds from. The Calculation driver has no diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationScriptingOptionsResolutionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationScriptingOptionsResolutionTests.cs index 3b187df6..5d2a86b7 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationScriptingOptionsResolutionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationScriptingOptionsResolutionTests.cs @@ -21,12 +21,12 @@ public sealed class CalculationScriptingOptionsResolutionTests [Fact] public void No_driver_keys_and_no_node_options_resolves_null_so_the_guard_uses_its_defaults() => - CalculationDriverFactoryExtensions.ResolveScripting(null, null, null).ShouldBeNull(); + CalculationDriverFactoryExtensions.ResolveScripting(null, null, null, out _).ShouldBeNull(); [Fact] public void No_driver_keys_inherits_every_node_knob() { - var resolved = CalculationDriverFactoryExtensions.ResolveScripting(null, null, Node).ShouldNotBeNull(); + var resolved = CalculationDriverFactoryExtensions.ResolveScripting(null, null, Node, out _).ShouldNotBeNull(); resolved.MaxAllocBytesPerRun.ShouldBe(Node.MaxAllocBytesPerRun); resolved.QuarantineAfterConsecutiveTimeouts.ShouldBe(7); @@ -36,19 +36,32 @@ public sealed class CalculationScriptingOptionsResolutionTests [Fact] public void A_driver_key_overrides_only_its_own_knob_and_the_node_object_is_not_mutated() { - var resolved = CalculationDriverFactoryExtensions.ResolveScripting(null, 2, Node).ShouldNotBeNull(); + var resolved = CalculationDriverFactoryExtensions.ResolveScripting(null, 2, Node, out var adjustments).ShouldNotBeNull(); resolved.QuarantineAfterConsecutiveTimeouts.ShouldBe(2); resolved.MaxAllocBytesPerRun.ShouldBe(Node.MaxAllocBytesPerRun, "the unauthored key keeps the node value"); resolved.StuckOrphanQuarantineFactor.ShouldBe(4, "the stuck factor has no driver key; it is node-wide"); resolved.ShouldNotBeSameAs(Node); Node.QuarantineAfterConsecutiveTimeouts.ShouldBe(7); + adjustments.ShouldBeEmpty(); + } + + /// Review m-2: an out-of-range value authored in the driver's own config is clamped AND reported, so + /// the factory can log it rather than the guard changing it silently. + [Fact] + public void An_out_of_range_driver_key_is_clamped_and_reported() + { + var resolved = CalculationDriverFactoryExtensions.ResolveScripting(1024L, 0, Node, out var adjustments).ShouldNotBeNull(); + + resolved.MaxAllocBytesPerRun.ShouldBe(ScriptingOptions.MinMaxAllocBytesPerRun); + resolved.QuarantineAfterConsecutiveTimeouts.ShouldBe(1); + adjustments.Count.ShouldBe(2); } [Fact] public void A_driver_key_with_no_node_options_layers_on_the_compiled_defaults() { - var resolved = CalculationDriverFactoryExtensions.ResolveScripting(0L, null, null).ShouldNotBeNull(); + var resolved = CalculationDriverFactoryExtensions.ResolveScripting(0L, null, null, out _).ShouldNotBeNull(); resolved.MaxAllocBytesPerRun.ShouldBe(0); resolved.QuarantineAfterConsecutiveTimeouts.ShouldBe(ScriptingOptions.DefaultQuarantineAfterConsecutiveTimeouts);