From 899af63b2a5ec20744f0527914f942bc758524d9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 10 Jul 2026 04:05:23 -0400 Subject: [PATCH] =?UTF-8?q?docs(inbound-api):=20DB=20row=20is=20authoritat?= =?UTF-8?q?ive=20for=20method=20scripts=20=E2=80=94=20update=20broken-save?= =?UTF-8?q?=20warning=20+=20Direct=20SQL=20Warning=20for=20the=20revision-?= =?UTF-8?q?checked=20cache?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj --- docs/requirements/Component-InboundAPI.md | 4 ++-- .../ManagementActor.cs | 16 ++++++++++------ .../ManagementActorTests.cs | 5 +++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/docs/requirements/Component-InboundAPI.md b/docs/requirements/Component-InboundAPI.md index a89c7ffc..d79febb0 100644 --- a/docs/requirements/Component-InboundAPI.md +++ b/docs/requirements/Component-InboundAPI.md @@ -131,11 +131,11 @@ API method scripts are compiled at central startup — all method definitions ar - Updating a method via the CLI (`api-method update --id --code '...'`) or Management API triggers immediate recompilation (`CompileAndRegister`). The updated script takes effect on the next API call — no node restart is required. - Creating a new method after startup: if the method is created but not yet compiled, the first invocation triggers lazy (on-demand) compilation. -> **Compile failures on save are non-fatal but surfaced.** `CompileAndRegister` only replaces the cached delegate when the new script compiles; on failure it leaves the **previously registered handler (if any) in place** — a broken save never takes a working method offline. The change still persists, so the Create/Update Management result carries a top-level **`compileWarning`** (null when the script compiled) listing the Roslyn/trust-model errors. On **update**, the previous version keeps serving while the broken script is *not* live; on **create**, the method returns `"Script compilation failed"` until a compiling version is saved. This means a saved script that does not compile is **not silently a no-op** — the caller (CLI / Central UI) sees the warning, and the real diagnostics are also logged centrally. +> **Compile failures on save are non-fatal but surfaced, and the DB row is authoritative.** The persisted `ApiMethod.Script` row is the single source of truth: on every request `ExecuteAsync` revision-checks the cached delegate against the freshly-fetched script and recompiles in place when it differs, so a saved-but-broken script does **not** leave a previously registered handler silently serving. The change still persists, so the Create/Update Management result carries a top-level **`compileWarning`** (null when the script compiled) listing the Roslyn/trust-model errors. On both **create** and **update**, once a non-compiling script is saved the method returns a compilation error (`HTTP 500`) **consistently on every central node** until a compiling version is saved — there is no node-divergent "old delegate still works here" state. This means a saved script that does not compile is **not silently a no-op** — the caller (CLI / Central UI) sees the warning, and the real diagnostics are also logged centrally. ### Direct SQL Warning -> **Do not edit API method scripts via direct SQL.** The in-memory compiled script will not be updated until the next node restart. Always use the CLI, Management API, or Central UI to modify API method scripts. +> **Direct SQL edits to API method scripts are now picked up automatically.** Because the DB row is authoritative and `ExecuteAsync` revision-checks the cached delegate against the freshly-fetched `ApiMethod.Script` on every request, a script changed directly in the database takes effect on the **next call** — no node restart and no Management round-trip is required. A broken direct edit surfaces the same way as a broken save: the method returns a `HTTP 500` compilation error consistently on all central nodes until a compiling version is stored. Editing via the CLI, Management API, or Central UI is still preferred because those paths validate the script up front and return the `compileWarning`, whereas a direct SQL edit is only diagnosed on the next request (and in the central log). --- diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index b3164b9d..c78184f8 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -2634,9 +2634,11 @@ public class ManagementActor : ReceiveActor method.ReturnDefinition = cmd.ReturnDefinition; await repo.UpdateApiMethodAsync(method); await repo.SaveChangesAsync(); - // On a compile failure the previously registered handler keeps serving — the - // saved-but-broken script does NOT go live. Surface that as a warning rather - // than a silent success (the change still persists either way). + // The persisted DB row is authoritative: on a compile failure the method returns + // a compilation error on every request (ExecuteAsync revision-checks the cached + // handler against the fresh row), so the saved-but-broken script does NOT keep a + // working version serving. Surface that as a warning rather than a silent success + // (the change still persists either way). var compileWarning = TryCompileAndDescribe(sp, method, isUpdate: true); await AuditAsync(sp, user, "Update", "ApiMethod", method.Id.ToString(), method.Name, method); return ApiMethodResult(method, compileWarning); @@ -2657,8 +2659,10 @@ public class ManagementActor : ReceiveActor /// /// Recompiles and re-registers an inbound API method's script after a Create/Update, /// returning a non-null human-readable warning when the script did NOT compile. The - /// save is always persisted; on compile failure the previously registered handler (if - /// any) keeps serving requests — the broken script never replaces a working one (see + /// save is always persisted; the persisted DB row is authoritative, so on compile + /// failure the method returns a compilation error on every request (ExecuteAsync + /// revision-checks the cached handler against the fresh row) rather than a previously + /// registered handler silently continuing to serve (see /// InboundScriptExecutor.CompileAndRegister). Returns null when the /// script compiled, or when no executor is registered (e.g. a non-inbound host or a /// test double). @@ -2673,7 +2677,7 @@ public class ManagementActor : ReceiveActor var detail = string.Join("; ", errors); return isUpdate - ? $"Script saved but failed to compile; the previously registered version (if any) remains active and the new script is NOT live. Compilation errors: {detail}" + ? $"Script saved but failed to compile; the method will return a compilation error on every request until a compiling version is saved. Compilation errors: {detail}" : $"Script saved but failed to compile; the method will return a compilation error until a compiling version is saved. Compilation errors: {detail}"; } diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs index 02a35321..e70163ae 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs @@ -417,8 +417,9 @@ public class ManagementActorTests : TestKit, IDisposable { // The save still persists, but a script that does not compile must surface a // top-level compileWarning in the result rather than silently looking like a - // success — and the previously registered handler keeps serving (CompileAndRegister - // never replaces a working delegate with a broken one). + // success. The DB row is authoritative: ExecuteAsync revision-checks the cached + // handler against the fresh row, so the broken script returns a compilation error + // on every request/node rather than a previously registered handler still serving. var existing = new Commons.Entities.InboundApi.ApiMethod("MyMethod", "return 1;") { Id = 7,