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
2026-10-22 15:15:11 -04:00
parent 4d8de20050
commit 5dabafecdc
3 changed files with 47 additions and 10 deletions
@@ -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
@@ -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<CalculationDriver>();
+        // 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<CalculationDriver>());
+        return new CalculationDriver(options, driverInstanceId, scriptRoot, logger: logger);
    }

    /// <summary>
@@ -109,10 +114,14 @@ public static class CalculationDriverFactoryExtensions
    /// <param name="maxAllocBytesPerRun">The driver config's <c>maxAllocBytesPerRun</c>, or null.</param>
    /// <param name="quarantineAfterConsecutiveTimeouts">The driver config's <c>quarantineAfterConsecutiveTimeouts</c>, or null.</param>
    /// <param name="nodeScripting">The node-wide options, or null.</param>
+    /// <param name="adjustments">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.</param>
    /// <returns>The options to hand the driver, or null.</returns>
    internal static ScriptingOptions? ResolveScripting(
-        long? maxAllocBytesPerRun, int? quarantineAfterConsecutiveTimeouts, ScriptingOptions? nodeScripting)
+        long? maxAllocBytesPerRun, int? quarantineAfterConsecutiveTimeouts, ScriptingOptions? nodeScripting,
+        out IReadOnlyList<string> 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);
    }

    /// <summary>The merged-config DTO the calc factory binds from. The Calculation driver has no
@@ -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();
+    }
+
+    /// <summary>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.</summary>
+    [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);