fix(management): compile an API method once per save — CompileForPublish replaces the validate pre-pass (review 2026-10-21 05/M1)

docs/requirements/Component-InboundAPI.md

@@ -175,7 +175,7 @@ The `sbk_<keyId>_<secret>` design already supports zero-downtime rotation — a
 
 ## Script Compilation & Hot-Reload
 
-API method scripts are pre-compiled in the **background after startup** by `InboundMethodWarmupService` (a hosted service registered after `AkkaHostedService`, so it never delays the cluster join or `/health/*`): it waits for `ApplicationStarted`, then — only once this node is the active central node (`IActiveNodeGate`; a standby pays nothing until a failover promotes it) — loads all method definitions and compiles them into in-memory delegates. Lazy compile + the per-request revision check guarantee correctness regardless. Compiles are **single-flight per method name + script revision**: N concurrent first callers share one compile (and one non-collectible loaded assembly). `InboundScriptExecutor.CompileAndRegister(ApiMethod, out IReadOnlyList<string> errors)` compiles once and returns the diagnostics, so a save needs no separate validate pass. `ApiMethodCache` prunes expired entries on a miss once large (scopes are exact method names — pinned by test — so growth is bounded).
+API method scripts are pre-compiled in the **background after startup** by `InboundMethodWarmupService` (a hosted service registered after `AkkaHostedService`, so it never delays the cluster join or `/health/*`): it waits for `ApplicationStarted`, then — only once this node is the active central node (`IActiveNodeGate`; a standby pays nothing until a failover promotes it) — loads all method definitions and compiles them into in-memory delegates. Lazy compile + the per-request revision check guarantee correctness regardless. Compiles are **single-flight per method name + script revision**: N concurrent first callers share one compile (and one non-collectible loaded assembly). `InboundScriptExecutor.CompileAndRegister(ApiMethod, out IReadOnlyList<string> errors)` compiles once and returns the diagnostics, so a save needs no separate validate pass; the management Create/Update path uses its sibling `CompileForPublish(method, out errors, out publish)`, which compiles once for the `compileWarning` and hands back a `publish` action queued as the post-commit registration — one compile (one pinned assembly) per save even inside a configuration transaction (review 2026-10-21 05/M1). `ApiMethodCache` prunes expired entries on a miss once large (scopes are exact method names — pinned by test — so growth is bounded).
 
 ### Update Workflow
 

docs/requirements/Component-ManagementService.md

@@ -213,7 +213,7 @@ The two-person authorization workflow for writes through the MxAccess Gateway. B
 ### Inbound API Methods
 
 - **ListApiMethods** / **GetApiMethod**: Query inbound API method definitions.
-- **CreateApiMethod** / **UpdateApiMethod** / **DeleteApiMethod**: Manage inbound API methods.
+- **CreateApiMethod** / **UpdateApiMethod** / **DeleteApiMethod**: Manage inbound API methods. Create/Update compile the script **once** (`InboundScriptExecutor.CompileForPublish`): the diagnostics become the result's `compileWarning` and the handler registration is queued as a post-commit action reusing that compile — each compile pins a non-collectible assembly, and the old validate-then-register pair compiled twice per save (review 2026-10-21 05/M1).
 
 ### Health
 

src/ZB.MOM.WW.ScadaBridge.InboundAPI/InboundScriptExecutor.cs

@@ -194,6 +194,31 @@ public class InboundScriptExecutor
     /// <returns>True if successfully compiled and registered, false otherwise.</returns>
     public bool CompileAndRegister(ApiMethod method, out IReadOnlyList<string> errors)
     {
+        var compiled = CompileForPublish(method, out errors, out var publish);
+        publish();
+        return compiled;
+    }
+
+    /// <summary>
+    /// Compiles an API method script ONCE and returns the diagnostics now, together with a
+    /// <paramref name="publish"/> action that applies the outcome to the handler cache later
+    /// without recompiling (review 2026-10-21 05/M1). A transactional caller (the management
+    /// Create/Update path) derives its "saved but did not compile" warning before commit and
+    /// queues <paramref name="publish"/> as a post-commit action; each compile pins a
+    /// non-collectible script assembly, so compiling separately for the warning and for the
+    /// registration doubled that cost per save. On success <paramref name="publish"/>
+    /// registers the handler for the compiled script text and clears any known-bad record;
+    /// on failure it records the known-bad method and leaves any previously registered
+    /// handler in place (see <see cref="CompileAndRegister(ApiMethod, out IReadOnlyList{string})"/>).
+    /// </summary>
+    /// <param name="method">The API method to compile.</param>
+    /// <param name="errors">The compilation/trust-model error messages; empty on success.</param>
+    /// <param name="publish">Applies the compile outcome to the handler cache; idempotent.</param>
+    /// <returns><c>true</c> when the script compiled; otherwise <c>false</c>.</returns>
+    public bool CompileForPublish(ApiMethod method, out IReadOnlyList<string> errors, out Action publish)
+    {
+        var name = method.Name;
+        var script = method.Script;
         var (handler, compileErrors) = CompileSingleFlight(method);
         if (handler == null)
         {
@@ -202,15 +227,19 @@ public class InboundScriptExecutor
             // through the capped TryRecordBadMethod helper so the cache
             // cannot grow without bound under a flood of unique method names.
             errors = compileErrors;
-            TryRecordBadMethod(method.Name, method.Script ?? string.Empty);
+            publish = () => TryRecordBadMethod(name, script ?? string.Empty);
             return false;
         }
 
         // The method definition was (re)compiled successfully — drop any stale
         // failure record so a fixed script is no longer treated as bad.
         errors = Array.Empty<string>();
-        _knownBadMethods.TryRemove(method.Name, out _);
-        return Register(method, handler);
+        publish = () =>
+        {
+            _knownBadMethods.TryRemove(name, out _);
+            _scriptHandlers[name] = new CachedHandler(script, handler);
+        };
+        return true;
     }
 
     /// <summary>
@@ -235,12 +264,6 @@ public class InboundScriptExecutor
         }
     }
 
-    private bool Register(ApiMethod method, Func<InboundScriptContext, Task<object?>> handler)
-    {
-        _scriptHandlers[method.Name] = new CachedHandler(method.Script, handler);
-        return true;
-    }
-
     /// <summary>
     /// Compiles a single API method script into an executable handler. Returns
     /// <c>null</c> when the script is missing, fails to compile, or violates the

src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs

@@ -3667,12 +3667,16 @@ public class ManagementActor : ReceiveActor
         var executor = sp.GetService<InboundAPI.InboundScriptExecutor>();
         if (executor is null)
             return null;
-        var compiled = executor.ValidateCompilation(method, out var errors);
+        // ONE compile (review 2026-10-21 05/M1): each compile pins a non-collectible script
+        // assembly, so the diagnostics for the warning and the handler registration come
+        // from the same compile. Registration still waits for the commit when the command
+        // runs inside a configuration transaction.
+        var compiled = executor.CompileForPublish(method, out var errors, out var publish);
         var actions = sp.GetService<IConfigurationCommitActions>();
         if (actions is not null)
-            actions.Add(() => { executor.CompileAndRegister(method); return Task.CompletedTask; });
+            actions.Add(() => { publish(); return Task.CompletedTask; });
         else
-            executor.CompileAndRegister(method);
+            publish();
         if (compiled)
             return null;
 

tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ApiMethodSingleCompileTests.cs

@@ -0,0 +1,119 @@
+using Akka.Actor;
+using Akka.TestKit.Xunit2;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
+using NSubstitute;
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
+using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management;
+using ZB.MOM.WW.ScadaBridge.InboundAPI;
+
+namespace ZB.MOM.WW.ScadaBridge.ManagementService.Tests;
+
+/// <summary>
+/// Review 2026-10-21 05/M1: an API-method save used to compile the script twice (a
+/// <c>ValidateCompilation</c> pre-pass, then <c>CompileAndRegister</c>), and each compile
+/// pins a non-collectible assembly. A save must compile exactly once — including when the
+/// registration is deferred until after the configuration transaction commits.
+/// </summary>
+public class ApiMethodSingleCompileTests : TestKit
+{
+    private readonly ServiceCollection _services = new ServiceCollection().AddTestConfigurationTransaction();
+    private readonly IInboundApiRepository _apiRepo = Substitute.For<IInboundApiRepository>();
+    private readonly CompileCountingLogger _logger = new();
+    private readonly CollectingCommitActions _commitActions = new();
+
+    /// <summary>Initializes the harness with a real executor whose compiles are counted.</summary>
+    public ApiMethodSingleCompileTests()
+    {
+        _services.AddScoped(_ => _apiRepo);
+        _services.AddScoped(_ => Substitute.For<IAuditService>());
+        _services.AddSingleton(new InboundScriptExecutor(_logger, Substitute.For<IServiceProvider>()));
+        _apiRepo.SaveChangesAsync(Arg.Any<CancellationToken>()).Returns(1);
+    }
+
+    private IActorRef CreateActor() =>
+        Sys.ActorOf(Props.Create(() => new ManagementActor(
+            _services.BuildServiceProvider(), NullLogger<ManagementActor>.Instance)));
+
+    private static ManagementEnvelope Envelope(object command) =>
+        new(new AuthenticatedUser("designer", "Designer", new[] { "Designer" }, Array.Empty<string>()),
+            command, Guid.NewGuid().ToString("N"));
+
+    /// <summary>A compiling create without deferred commit actions compiles once.</summary>
+    [Fact]
+    public void Create_CompilingScript_CompilesOnce()
+    {
+        CreateActor().Tell(Envelope(new CreateApiMethodCommand("Once", "return 1;", 30, null, null)));
+
+        var response = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(30));
+        Assert.Equal(1, _logger.CompileCount);
+        Assert.DoesNotContain("failed to compile", response.JsonData);
+    }
+
+    /// <summary>
+    /// With the registration deferred to post-commit, the warning is still derived before the
+    /// reply and the queued publish reuses the compiled result instead of recompiling.
+    /// </summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task Update_DeferredRegistration_CompilesOnce_AndStillWarns()
+    {
+        _services.AddScoped<IConfigurationCommitActions>(_ => _commitActions);
+        var existing = new ApiMethod("M", "return 1;") { Id = 7, TimeoutSeconds = 10 };
+        _apiRepo.GetApiMethodByIdAsync(7, Arg.Any<CancellationToken>()).Returns(existing);
+
+        CreateActor().Tell(Envelope(new UpdateApiMethodCommand(7, "return doesNotExist;", 10, null, null)));
+
+        var response = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(30));
+        Assert.Contains("failed to compile", response.JsonData);
+        foreach (var action in _commitActions.Actions)
+            await action();
+        Assert.NotEmpty(_commitActions.Actions);
+        Assert.Equal(0, _logger.CompileCount); // the broken script never reaches "compiled"
+        Assert.Equal(1, _logger.CompileFailureCount);
+    }
+
+    /// <summary>A compiling update with deferred registration compiles once across reply + publish.</summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task Update_DeferredRegistration_CompilingScript_CompilesOnce()
+    {
+        _services.AddScoped<IConfigurationCommitActions>(_ => _commitActions);
+        var existing = new ApiMethod("M2", "return 1;") { Id = 8, TimeoutSeconds = 10 };
+        _apiRepo.GetApiMethodByIdAsync(8, Arg.Any<CancellationToken>()).Returns(existing);
+
+        CreateActor().Tell(Envelope(new UpdateApiMethodCommand(8, "return 2;", 10, null, null)));
+
+        ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(30));
+        foreach (var action in _commitActions.Actions)
+            await action();
+        Assert.Equal(1, _logger.CompileCount);
+    }
+
+    private sealed class CollectingCommitActions : IConfigurationCommitActions
+    {
+        public List<Func<Task>> Actions { get; } = new();
+        public void Add(Func<Task> action) => Actions.Add(action);
+    }
+
+    private sealed class CompileCountingLogger : ILogger<InboundScriptExecutor>
+    {
+        private int _count;
+        private int _failures;
+        public int CompileCount => _count;
+        public int CompileFailureCount => _failures;
+        public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
+        public bool IsEnabled(LogLevel logLevel) => true;
+        public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
+        {
+            var text = formatter(state, exception);
+            if (text.EndsWith("script compiled", StringComparison.Ordinal))
+                Interlocked.Increment(ref _count);
+            else if (text.Contains("script compilation failed", StringComparison.Ordinal))
+                Interlocked.Increment(ref _failures);
+        }
+    }
+}