fix(template): preserve per-script ExecutionTimeoutSeconds across UI edits; add alarm fallback tests (#9)

The UI script editor has no ExecutionTimeoutSeconds control (authoring deferred), so a body edit silently cleared a timeout set via Transport import. Round-trip the loaded value so UI edits preserve it. Add the missing AlarmExecutionActor null/<=0 fallback tests for symmetry with ScriptExecutionActor.
2026-06-15 14:49:37 -04:00
parent 3edef09f51
commit 3032faac0d
3 changed files with 108 additions and 0 deletions
@@ -331,6 +331,66 @@ public class TemplateServiceTests
        Assert.Contains("Name", result.Error);
    }

+    [Fact]
+    public async Task UpdateScript_UiEditPath_PreservesExistingExecutionTimeoutSeconds()
+    {
+        // M2.5 (#9): ExecutionTimeoutSeconds has no authoring control in the UI.
+        // A UI-style update (proposed.ExecutionTimeoutSeconds == null) must NOT
+        // overwrite a timeout previously set via Transport import.
+        //
+        // The fix is in TemplateEdit.razor: it round-trips the loaded value, so
+        // proposed.ExecutionTimeoutSeconds will equal the existing value, not null.
+        // This test proves that when the round-trip is working, the service
+        // preserves the timeout end-to-end.
+        var existing = new TemplateScript("OnStart", "return true;")
+        {
+            Id = 1,
+            TemplateId = 1,
+            ExecutionTimeoutSeconds = 30 // set via Transport import
+        };
+        _repoMock.Setup(r => r.GetTemplateScriptByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(existing);
+        var template = new Template("Pump") { Id = 1 };
+        _repoMock.Setup(r => r.GetTemplateByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(template);
+
+        // Simulate what the UI now does: round-trip the loaded ExecutionTimeoutSeconds.
+        var proposed = new TemplateScript("OnStart", "return false;")
+        {
+            ExecutionTimeoutSeconds = existing.ExecutionTimeoutSeconds // round-trip
+        };
+
+        var result = await _service.UpdateScriptAsync(1, proposed, "admin");
+
+        Assert.True(result.IsSuccess);
+        Assert.Equal(30, result.Value.ExecutionTimeoutSeconds);
+    }
+
+    [Fact]
+    public async Task UpdateScript_ExplicitNullTimeout_ClearsExecutionTimeoutSeconds()
+    {
+        // M2.5 (#9): a deliberate clear (e.g. via Transport or CLI setting null
+        // explicitly) must still work — the service must not guard against null.
+        var existing = new TemplateScript("OnStart", "return true;")
+        {
+            Id = 1,
+            TemplateId = 1,
+            ExecutionTimeoutSeconds = 30
+        };
+        _repoMock.Setup(r => r.GetTemplateScriptByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(existing);
+        var template = new Template("Pump") { Id = 1 };
+        _repoMock.Setup(r => r.GetTemplateByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(template);
+
+        // Explicit null — caller intentionally clears the timeout.
+        var proposed = new TemplateScript("OnStart", "return false;")
+        {
+            ExecutionTimeoutSeconds = null
+        };
+
+        var result = await _service.UpdateScriptAsync(1, proposed, "admin");
+
+        Assert.True(result.IsSuccess);
+        Assert.Null(result.Value.ExecutionTimeoutSeconds);
+    }
+
    // ========================================================================
    // WP-5: Shared Script CRUD (see SharedScriptServiceTests)
    // ========================================================================