feat(inbound-api): surface method compile errors to operators — api-method validate/recompile + compile state on get (ScadaBridge#75)

The data plane answers a broken inbound method with the generic "Script
compilation failed for this method"; the Roslyn diagnostic reached only the
central log, and the known-bad cache could not be observed or reset per method.

- InboundScriptExecutor keeps the diagnostics with each known-bad record and
  exposes GetCompileState(name, currentScript): IsCompiled / IsKnownBad /
  LastCompileError, judged against the current script text (a record for older
  text is reported as not compiled yet).
- Management: ValidateApiMethodCommand (compile the stored script, return
  ApiMethodCompileResult, register nothing, cache untouched) and
  RecompileApiMethodCommand (CompileAndRegister without re-sending the entity,
  drops the node's ApiMethodCache row, audited "Recompile"). Both Designer, like
  UpdateApiMethod; dispatched through the shared ApiMethodCompileOperations.
  GetApiMethod keeps the entity fields top-level and adds isCompiled,
  isKnownBad, lastCompileError and compileStateNode. Compile state is per
  central node and every result names the node.
- CLI: api-method validate --id, api-method recompile --id.
- Central UI: the API-method edit page shows the saved script's compile state
  and last error, with Validate and Recompile actions (same operations, same
  Designer gate, fresh DI scope).
- RequiredRoleMatrixTests entries; ManagementSurfaceParityTests covers the new
  CLI leaves; ApiMethodCompileDiagnosticsTests + an executor CompileState test.
- Docs: CLI README, Component-CLI/ManagementService/InboundAPI, components/*.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
2026-11-09 15:28:41 -05:00
parent 65ffa1dad4
commit 3dd492a77f
18 changed files with 542 additions and 15 deletions
+1 -1
View File
@@ -118,7 +118,7 @@ scadabridge instance deploy --id 7
| `flow-debug` | `run-method`, `run-script`, `run-value-change`, `replay` | Deployer; replay also accepts OperationalAudit roles |
| `shared-script` | `list`, `get`, `create`, `update`, `delete` | `Designer` |
| `db-connection` | `list`, `get`, `create`, `update`, `delete` | `Designer` |
| `api-method` | `list`, `get`, `create`, `update`, `delete` | `Designer` |
| `api-method` | `list`, `get`, `create`, `update`, `delete`, `validate`, `recompile` | `Designer` |
| `bundle` | `export`, `preview`, `import` | Designer export; Administrator preview/import |
| `audit` | `query`, `export`, `tree`, `backfill-source-node`, `verify-chain` | Audit roles; Administrator backfill; verify-chain is a local placeholder |
| `audit-config` | `query` (config-change audit trail; was `audit-log` pre-M8) | `Admin` |
+1 -1
View File
@@ -256,7 +256,7 @@ The in-memory handler cache still holds the previous compiled delegate. If the u
### A method is stuck in the known-bad-methods cache
If a previously broken script is fixed but `ExecuteAsync` still returns `"Script compilation failed for this method"`, the method name is in `_knownBadMethods`. `CompileAndRegister` clears the bad-method entry on a successful compile; calling it (via the Management API or CLI `api-method update`) after the fix is applied resets the cache and makes the corrected script active immediately.
If a previously broken script is fixed but `ExecuteAsync` still returns `"Script compilation failed for this method"`, the method name is in `_knownBadMethods`. A record only short-circuits while the stored script text is the one that failed, so a changed script always recompiles; a stale record therefore means the script text is unchanged and something outside it made the compile fail. `api-method get --id <N>` shows the node's `isKnownBad` / `lastCompileError` (the real diagnostics; the data plane keeps its generic message), `api-method validate --id <N>` compiles the stored script and returns the diagnostics without touching the cache, and `api-method recompile --id <N>` re-runs `CompileAndRegister` — clearing the entry on success — without re-sending the entity (#75). All three are per central node; the Central UI's API-method edit page exposes the same state and actions.
### Routed calls time out but the site is reachable
+1 -1
View File
@@ -177,7 +177,7 @@ There is **no** out-of-process actor path. The actor was advertised via `Cluster
| Notification lists / SMTP | `ListNotificationLists`, `GetNotificationList`, `CreateNotificationList`, `UpdateNotificationList`, `DeleteNotificationList`, `ListSmtpConfigs`, `UpdateSmtpConfig` | Designer (mutations) |
| Shared scripts | `ListSharedScripts`, `GetSharedScript`, `CreateSharedScript`, `UpdateSharedScript`, `DeleteSharedScript` | Designer (mutations) |
| Database connections | `ListDatabaseConnections`, `GetDatabaseConnection`, `CreateDatabaseConnectionDef`, `UpdateDatabaseConnectionDef`, `DeleteDatabaseConnectionDef` | Designer (mutations) |
| Inbound API methods | `ListApiMethods`, `GetApiMethod`, `CreateApiMethod`, `UpdateApiMethod`, `DeleteApiMethod` | Designer (mutations) |
| Inbound API methods | `ListApiMethods`, `GetApiMethod`, `CreateApiMethod`, `UpdateApiMethod`, `DeleteApiMethod`, `ValidateApiMethod`, `RecompileApiMethod` | Designer (mutations, validate, recompile) |
| Security | `ListRoleMappings`, `CreateRoleMapping`, `UpdateRoleMapping`, `DeleteRoleMapping`, `ListApiKeys`, `CreateApiKey`, `UpdateApiKey`, `DeleteApiKey`, `SetApiKeyMethods`, `ListScopeRules`, `AddScopeRule`, `DeleteScopeRule` | Administrator |
| Deployments | `MgmtDeployArtifacts`, `QueryDeployments`, `GetDeploymentDiff`, `GetPendingUpdates` | Deployer |
| Health | `GetHealthSummary`, `GetSiteHealth` | Any authenticated user |
+2
View File
@@ -409,6 +409,8 @@ scadabridge api-method get --id <id>
scadabridge api-method create --name <name> --script <code> [--timeout <seconds>] [--parameters <json>] [--return-def <json>]
scadabridge api-method update --id <id> --script <code> [--timeout <seconds>] [--parameters <json>] [--return-def <json>]
scadabridge api-method delete --id <id>
scadabridge api-method validate --id <id>
scadabridge api-method recompile --id <id>
```
### Bundle Commands (Transport #24)
@@ -181,6 +181,7 @@ API method scripts are pre-compiled in the **background after startup** by `Inbo
- Updating a method via the CLI (`api-method update --id <N> --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.
- **Operator diagnostics (#75).** The data plane answers a broken method with the generic `Script compilation failed for this method` (the Roslyn text can leak code to an API-key caller). Operators get the real diagnostics through the role-gated management surface instead: `GetApiMethod` carries the node's `IsCompiled` / `IsKnownBad` / `LastCompileError`, `ValidateApiMethod` compiles the stored script and returns the errors without registering anything, and `RecompileApiMethod` re-runs `CompileAndRegister` for one method without a full-replace update (CLI `api-method get|validate|recompile`, Central UI API-method edit page). Compile state is per central node.
> **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.
@@ -220,7 +220,9 @@ The two-person authorization workflow for writes through the MxAccess Gateway. B
### Inbound API Methods
- **ListApiMethods** / **GetApiMethod**: Query inbound API method definitions.
- **ListApiMethods** / **GetApiMethod**: Query inbound API method definitions. `GetApiMethod` also returns the answering central node's compile state for the stored script — `IsCompiled`, `IsKnownBad`, `LastCompileError` (the real diagnostics behind the data plane's generic `Script compilation failed for this method`) and `CompileStateNode` (#75).
- **ValidateApiMethod** (Designer): Compile the stored script on the receiving node and return `ApiMethodCompileResult` (`Success`, `Errors`, `Node`). Nothing is registered; the known-bad cache is untouched (#75).
- **RecompileApiMethod** (Designer): Re-run `CompileAndRegister` on the stored script without re-sending the entity, clearing a stale known-bad record on success, and drop the node's cached row. Returns `ApiMethodCompileResult`; audited as `Recompile`. Node-local, like every inbound compile (#75).
- **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
@@ -23,6 +23,8 @@ public static class ApiMethodCommands
command.Add(BuildCreate(urlOption, formatOption, usernameOption, passwordOption));
command.Add(BuildUpdate(urlOption, formatOption, usernameOption, passwordOption));
command.Add(BuildDelete(urlOption, formatOption, usernameOption, passwordOption));
command.Add(BuildValidate(urlOption, formatOption, usernameOption, passwordOption));
command.Add(BuildRecompile(urlOption, formatOption, usernameOption, passwordOption));
return command;
}
@@ -52,6 +54,40 @@ public static class ApiMethodCommands
return cmd;
}
private static Command BuildValidate(Option<string> urlOption, Option<string> formatOption, Option<string> usernameOption, Option<string> passwordOption)
{
var idOption = new Option<int>("--id") { Description = "API method ID", Required = true };
var cmd = new Command("validate")
{
Description = "Compile an API method's stored script on the central node and show the compile errors (nothing is registered)"
};
cmd.Add(idOption);
cmd.SetAction(async (ParseResult result) =>
{
var id = result.GetValue(idOption);
return await CommandHelpers.ExecuteCommandAsync(
result, urlOption, formatOption, usernameOption, passwordOption, new ValidateApiMethodCommand(id));
});
return cmd;
}
private static Command BuildRecompile(Option<string> urlOption, Option<string> formatOption, Option<string> usernameOption, Option<string> passwordOption)
{
var idOption = new Option<int>("--id") { Description = "API method ID", Required = true };
var cmd = new Command("recompile")
{
Description = "Recompile and re-register an API method's stored script on the central node, clearing a stale compile failure"
};
cmd.Add(idOption);
cmd.SetAction(async (ParseResult result) =>
{
var id = result.GetValue(idOption);
return await CommandHelpers.ExecuteCommandAsync(
result, urlOption, formatOption, usernameOption, passwordOption, new RecompileApiMethodCommand(id));
});
return cmd;
}
private static Command BuildCreate(Option<string> urlOption, Option<string> formatOption, Option<string> usernameOption, Option<string> passwordOption)
{
var nameOption = new Option<string>("--name") { Description = "Method name", Required = true };
+38 -1
View File
@@ -2342,7 +2342,13 @@ scadabridge --url <url> api-method list
#### `api-method get`
Get a single inbound API method by ID.
Get a single inbound API method by ID. Besides the method's fields the reply carries the
answering central node's compile state for the stored script: `isCompiled`, `isKnownBad`
(the script failed to compile and requests fast-fail with the generic
`Script compilation failed for this method`), `lastCompileError` (the real compiler /
trust-model diagnostics when `isKnownBad`) and `compileStateNode`. Each central node
compiles independently, so the state is per node; the fields are null on a host with no
inbound API executor.
```sh
scadabridge --url <url> api-method get --id <int>
@@ -2352,6 +2358,37 @@ scadabridge --url <url> api-method get --id <int>
|--------|----------|-------------|
| `--id` | yes | API method ID |
#### `api-method validate`
Compile the method's **stored** script on the central node and return
`{ apiMethodId, name, success, errors, node }` — the same compile the data plane runs, with
the diagnostics the data plane deliberately hides from API-key callers. Nothing is
registered and the known-bad cache is untouched. Requires `Designer`.
```sh
scadabridge --url <url> api-method validate --id <int>
```
| Option | Required | Description |
|--------|----------|-------------|
| `--id` | yes | API method ID |
#### `api-method recompile`
Re-run `CompileAndRegister` on the method's stored script on the central node, without
re-sending the entity: a success replaces the cached handler and clears a stale compile
failure (for example one caused by something outside the script that has since been
fixed); a failure records the new diagnostics. Returns the same shape as `validate` and is
audited as `Recompile`. Affects the answering node only. Requires `Designer`.
```sh
scadabridge --url <url> api-method recompile --id <int>
```
| Option | Required | Description |
|--------|----------|-------------|
| `--id` | yes | API method ID |
#### `api-method create`
Create a new inbound API method.
@@ -9,6 +9,9 @@
@using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi
@using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories
@using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Security
@using ZB.MOM.WW.ScadaBridge.Commons.Types.InboundApi
@using ZB.MOM.WW.ScadaBridge.ManagementService
@using Microsoft.Extensions.DependencyInjection
@using ScriptAnalysis = ZB.MOM.WW.ScadaBridge.CentralUI.ScriptAnalysis
@attribute [Authorize(Policy = AuthorizationPolicies.RequireDesign)]
@inject IInboundApiRepository InboundApiRepository
@@ -123,6 +126,65 @@
</div>
</fieldset>
@if (_existing != null)
{
@* ScadaBridge#75: the data plane answers a broken method with a generic
"Script compilation failed for this method"; operators read the real
diagnostics here (same operations as CLI api-method get/validate/recompile). *@
<fieldset class="mb-4" data-test="api-method-compile">
<legend class="h6">Compilation (saved script)</legend>
<div class="d-flex flex-wrap gap-2 align-items-center mb-2">
@if (_compileStatus is null)
{
<span class="badge bg-secondary" data-test="api-method-compile-status">Unavailable on this node</span>
}
else if (_compileStatus.IsKnownBad)
{
<span class="badge bg-danger" data-test="api-method-compile-status">Compile failed</span>
}
else if (_compileStatus.IsCompiled)
{
<span class="badge bg-success" data-test="api-method-compile-status">Compiled</span>
}
else
{
<span class="badge bg-secondary" data-test="api-method-compile-status">Not compiled yet</span>
}
@if (_compileStatus?.Node is { } node)
{
<span class="text-muted small">on node @node</span>
}
<button class="btn btn-outline-secondary btn-sm" data-test="api-method-validate"
@onclick="ValidateSavedScriptAsync" disabled="@_compiling">Validate</button>
<button class="btn btn-outline-secondary btn-sm" data-test="api-method-recompile"
@onclick="RecompileSavedScriptAsync" disabled="@_compiling">Recompile</button>
</div>
@if (_compileStatus?.LastCompileError is { } lastError)
{
<pre class="border border-danger-subtle rounded p-2 small mb-2 font-monospace text-danger" style="white-space: pre-wrap;">@lastError</pre>
}
@if (_compileResult != null)
{
<div class="small @(_compileResult.Success ? "text-success" : "text-danger")" data-test="api-method-compile-result">
@_compileAction: @(_compileResult.Success ? "the saved script compiles." : "the saved script does not compile.")
</div>
@if (_compileResult.Errors.Count > 0)
{
<ul class="small text-danger mt-1 mb-0">
@foreach (var error in _compileResult.Errors)
{
<li>@error</li>
}
</ul>
}
}
<div class="form-text">
Checks the script as saved, not unsaved edits. Recompile re-registers it on this
central node, clearing a stale compile failure.
</div>
</fieldset>
}
@if (_formError != null)
{
<div class="text-danger small mb-2">@_formError</div>
@@ -245,6 +307,10 @@
private HashSet<string> _initialKeyIds = new(StringComparer.Ordinal);
private bool _showTestRun;
private ApiMethodCompileStatus? _compileStatus;
private ApiMethodCompileResult? _compileResult;
private string? _compileAction;
private bool _compiling;
private bool _running;
private Dictionary<string, object?> _paramValues = new();
private ScriptAnalysis.SandboxRunResult? _runResult;
@@ -277,6 +343,7 @@
var keysForMethod = await ApiKeyAdmin.GetKeysForMethodAsync(_existing.Name);
_initialKeyIds = new HashSet<string>(keysForMethod, StringComparer.Ordinal);
_selectedKeyIds = new HashSet<string>(_initialKeyIds, StringComparer.Ordinal);
_compileStatus = ApiMethodCompileOperations.GetStatus(MutationServices, _existing);
}
}
catch (Exception ex) { _formError = ex.Message; }
@@ -284,6 +351,35 @@
_loading = false;
}
private Task ValidateSavedScriptAsync() =>
RunCompileActionAsync("Validate", (sp, _) => ApiMethodCompileOperations.ValidateAsync(sp, _existing!.Id));
private Task RecompileSavedScriptAsync() =>
RunCompileActionAsync("Recompile", (sp, user) => ApiMethodCompileOperations.RecompileAsync(sp, _existing!.Id, user));
/// <summary>
/// Runs a compile action under the same Designer gate as the management commands, in a
/// fresh DI scope (the recompile writes an audit row), then refreshes the status badge.
/// </summary>
private async Task RunCompileActionAsync(
string action, Func<IServiceProvider, string, Task<ApiMethodCompileResult>> operation)
{
if (_existing is null) return;
_formError = null;
_compiling = true;
try
{
var user = await MutationAuth.RequireRoleAsync(Roles.Designer);
await using var scope = MutationServices.GetRequiredService<IServiceScopeFactory>().CreateAsyncScope();
_compileResult = await operation(scope.ServiceProvider, user);
_compileAction = action;
var saved = await scope.ServiceProvider.GetRequiredService<IInboundApiRepository>().GetApiMethodByIdAsync(_existing.Id);
_compileStatus = saved is null ? null : ApiMethodCompileOperations.GetStatus(scope.ServiceProvider, saved);
}
catch (Exception ex) { _formError = ex.Message; }
finally { _compiling = false; }
}
private void ToggleKey(string keyId, bool isChecked)
{
if (isChecked) _selectedKeyIds.Add(keyId);
@@ -7,3 +7,18 @@ public record CreateApiMethodCommand(string Name, string Script, int TimeoutSeco
/// <param name="TraceAllCalls">Flow Debugger (design 2026-09-05 §5.2): <c>null</c> leaves the stored value unchanged so an older CLI never resets it.</param>
public record UpdateApiMethodCommand(int ApiMethodId, string Script, int TimeoutSeconds, string? ParameterDefinitions, string? ReturnDefinition, bool? TraceAllCalls = null);
public record DeleteApiMethodCommand(int ApiMethodId);
/// <summary>
/// Compiles the method's stored script on the receiving central node and returns the
/// diagnostics, without registering the handler or touching the known-bad cache
/// (ScadaBridge#75). Operators see the real compile error here; the data plane keeps
/// returning its generic message.
/// </summary>
/// <param name="ApiMethodId">The API method to validate.</param>
public record ValidateApiMethodCommand(int ApiMethodId);
/// <summary>
/// Re-runs <c>CompileAndRegister</c> for the method's stored script on the receiving central
/// node, replacing its cached handler or known-bad record, without re-sending the entity
/// (ScadaBridge#75). Returns the diagnostics.
/// </summary>
/// <param name="ApiMethodId">The API method to recompile.</param>
public record RecompileApiMethodCommand(int ApiMethodId);
@@ -0,0 +1,25 @@
namespace ZB.MOM.WW.ScadaBridge.Commons.Types.InboundApi;
/// <summary>
/// Result of <c>ValidateApiMethodCommand</c> / <c>RecompileApiMethodCommand</c>
/// (ScadaBridge#75): the outcome of compiling an inbound API method's stored script on one
/// central node, with the compiler / trust-model diagnostics an operator needs.
/// </summary>
/// <param name="ApiMethodId">The API method id.</param>
/// <param name="Name">The API method name.</param>
/// <param name="Success">True when the script compiled.</param>
/// <param name="Errors">The compile / trust-model errors; empty on success.</param>
/// <param name="Node">The central node that compiled it (compile state is per node), when known.</param>
public sealed record ApiMethodCompileResult(
int ApiMethodId, string Name, bool Success, IReadOnlyList<string> Errors, string? Node);
/// <summary>
/// One central node's runtime compile state for an inbound API method (ScadaBridge#75),
/// judged against the method's current stored script.
/// </summary>
/// <param name="IsCompiled">A handler compiled from the current script is cached on this node.</param>
/// <param name="IsKnownBad">The current script failed to compile on this node; requests fast-fail.</param>
/// <param name="LastCompileError">The diagnostics of that failure, when <paramref name="IsKnownBad"/>.</param>
/// <param name="Node">The central node the state was read from, when known.</param>
public sealed record ApiMethodCompileStatus(
bool IsCompiled, bool IsKnownBad, string? LastCompileError, string? Node);
@@ -0,0 +1,10 @@
namespace ZB.MOM.WW.ScadaBridge.InboundAPI;
/// <summary>
/// This node's compile state for one inbound API method, as reported by
/// <see cref="InboundScriptExecutor.GetCompileState"/> (ScadaBridge#75).
/// </summary>
/// <param name="IsCompiled">A handler compiled from the current script text is cached.</param>
/// <param name="IsKnownBad">The current script text failed to compile; requests fast-fail without recompiling.</param>
/// <param name="LastCompileError">The joined diagnostics of that failure, when <paramref name="IsKnownBad"/>.</param>
public sealed record ApiMethodCompileState(bool IsCompiled, bool IsKnownBad, string? LastCompileError);
@@ -57,7 +57,12 @@ public class InboundScriptExecutor
// dropped — the cache is just a fast-fail optimisation; the per-request DB
// lookup remains the correctness path.
private const int KnownBadMethodsCap = 1000;
private readonly ConcurrentDictionary<string, string> _knownBadMethods = new();
private readonly ConcurrentDictionary<string, KnownBadMethod> _knownBadMethods = new();
// A known-bad record: the exact script text that failed and the diagnostics it failed
// with, so an operator can read them through the management surface (ScadaBridge#75)
// instead of the central log.
private sealed record KnownBadMethod(string Script, IReadOnlyList<string> Errors);
/// <summary>Diagnostic helper — whether a compiled handler is cached for the method.</summary>
internal bool HasHandler(string methodName) => _scriptHandlers.ContainsKey(methodName);
@@ -69,6 +74,31 @@ public class InboundScriptExecutor
/// </summary>
internal int KnownBadMethodCount => _knownBadMethods.Count;
/// <summary>
/// This node's compile state for <paramref name="methodName"/>, judged against the
/// method's CURRENT script text (ScadaBridge#75). A handler or failure record left over
/// from different script text describes an older revision, so it is reported as not yet
/// compiled — the next request (or a recompile) compiles the current text. The state is
/// per central node: each node compiles and caches independently.
/// </summary>
/// <param name="methodName">The method name.</param>
/// <param name="currentScript">The method's current (persisted) script text.</param>
/// <returns>The compile state of the method on this node.</returns>
public ApiMethodCompileState GetCompileState(string methodName, string? currentScript)
{
var script = currentScript ?? string.Empty;
if (_knownBadMethods.TryGetValue(methodName, out var bad)
&& string.Equals(bad.Script, script, StringComparison.Ordinal))
{
return new ApiMethodCompileState(IsCompiled: false, IsKnownBad: true, string.Join("; ", bad.Errors));
}
// A null Script is the RegisterHandler test seam, which serves regardless of text.
var compiled = _scriptHandlers.TryGetValue(methodName, out var cached)
&& (cached.Script is null || string.Equals(cached.Script, script, StringComparison.Ordinal));
return new ApiMethodCompileState(IsCompiled: compiled, IsKnownBad: false, LastCompileError: null);
}
/// <summary>
/// Records <paramref name="methodName"/> → <paramref name="script"/> in the
/// known-bad-methods cache so the CURRENT failing script text short-circuits the
@@ -79,18 +109,19 @@ public class InboundScriptExecutor
/// (paying the cheap recompile next time rather than leaking memory under a
/// unique-name flood). Records are cleared on a successful (re)compile.
/// </summary>
private void TryRecordBadMethod(string methodName, string script)
private void TryRecordBadMethod(string methodName, string script, IReadOnlyList<string> errors)
{
var record = new KnownBadMethod(script, errors);
// Overwrite an existing record (newer bad script wins) without counting
// against the cap; only a genuinely new method name is gated by the cap.
if (_knownBadMethods.ContainsKey(methodName))
{
_knownBadMethods[methodName] = script;
_knownBadMethods[methodName] = record;
return;
}
if (_knownBadMethods.Count >= KnownBadMethodsCap)
return;
_knownBadMethods.TryAdd(methodName, script);
_knownBadMethods.TryAdd(methodName, record);
}
private readonly IServiceProvider _serviceProvider;
@@ -227,7 +258,7 @@ public class InboundScriptExecutor
// through the capped TryRecordBadMethod helper so the cache
// cannot grow without bound under a flood of unique method names.
errors = compileErrors;
publish = () => TryRecordBadMethod(name, script ?? string.Empty);
publish = () => TryRecordBadMethod(name, script ?? string.Empty, compileErrors);
return false;
}
@@ -562,20 +593,20 @@ public class InboundScriptExecutor
if (cached == null)
{
// Fast-fail only when the CURRENT script text is the one already known bad.
if (_knownBadMethods.TryGetValue(method.Name, out var badScript)
&& string.Equals(badScript, method.Script, StringComparison.Ordinal))
if (_knownBadMethods.TryGetValue(method.Name, out var knownBad)
&& string.Equals(knownBad.Script, method.Script, StringComparison.Ordinal))
return new InboundScriptResult(false, null, "Script compilation failed for this method", "SCRIPT_COMPILE_FAILED");
// Lazy compile (handles methods created after startup, and stale/known-bad
// recompiles). Compile outside the cache so a failed compile is not stored,
// then add so concurrent first-callers share a single handler instance.
var (compiled, _) = CompileSingleFlight(method);
var (compiled, compileErrors) = CompileSingleFlight(method);
if (compiled == null)
{
// Cache the failure so the next request short-circuits above.
// Routed through TryRecordBadMethod so the
// cache is bounded under a flood of unique method names.
TryRecordBadMethod(method.Name, method.Script ?? string.Empty);
TryRecordBadMethod(method.Name, method.Script ?? string.Empty, compileErrors);
return new InboundScriptResult(false, null, "Script compilation failed for this method", "SCRIPT_COMPILE_FAILED");
}
@@ -0,0 +1,85 @@
using Microsoft.Extensions.DependencyInjection;
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.Types.InboundApi;
using ZB.MOM.WW.ScadaBridge.InboundAPI;
namespace ZB.MOM.WW.ScadaBridge.ManagementService;
/// <summary>
/// Operator access to inbound API method compilation (ScadaBridge#75): the real compile
/// diagnostics, this node's compile state, and a recompile that does not need the entity
/// re-sent. Shared by the <see cref="ManagementActor"/> handlers (CLI / Management API) and
/// the Central UI, which call it under the same role gate, so every surface reports the same
/// thing. Compile state is per central node — each node compiles and caches independently —
/// so every result names the node it came from.
/// </summary>
public static class ApiMethodCompileOperations
{
/// <summary>
/// Compiles the method's stored script and returns the diagnostics. Nothing is registered
/// and the known-bad cache is untouched, so a validate never changes what requests see.
/// </summary>
/// <param name="sp">The scoped service provider.</param>
/// <param name="apiMethodId">The API method id.</param>
/// <returns>The compile outcome and its diagnostics.</returns>
public static async Task<ApiMethodCompileResult> ValidateAsync(IServiceProvider sp, int apiMethodId)
{
var method = await LoadAsync(sp, apiMethodId);
var valid = RequireExecutor(sp).ValidateCompilation(method, out var errors);
return new ApiMethodCompileResult(method.Id, method.Name, valid, errors, NodeName(sp));
}
/// <summary>
/// Re-runs <c>CompileAndRegister</c> on the method's stored script: a success replaces the
/// cached handler and clears the known-bad record, a failure records the new diagnostics
/// (a previously working handler is never replaced by a failed compile, but the stored row
/// stays authoritative, see <see cref="InboundScriptExecutor.CompileAndRegister(ApiMethod, out IReadOnlyList{string})"/>).
/// The node's cached row is dropped too, so the next request reads the stored row.
/// Audited as <c>Recompile</c>.
/// </summary>
/// <param name="sp">The scoped service provider.</param>
/// <param name="apiMethodId">The API method id.</param>
/// <param name="user">The acting user, for the audit row.</param>
/// <returns>The compile outcome and its diagnostics.</returns>
public static async Task<ApiMethodCompileResult> RecompileAsync(IServiceProvider sp, int apiMethodId, string user)
{
var method = await LoadAsync(sp, apiMethodId);
var compiled = RequireExecutor(sp).CompileAndRegister(method, out var errors);
sp.GetService<ApiMethodCache>()?.Invalidate(method.Name);
var result = new ApiMethodCompileResult(method.Id, method.Name, compiled, errors, NodeName(sp));
await sp.GetRequiredService<IAuditService>().LogAsync(
user, "Recompile", "ApiMethod", method.Id.ToString(), method.Name,
new { method.Id, method.Name, Compiled = compiled, ErrorCount = errors.Count, result.Node });
await sp.GetRequiredService<IInboundApiRepository>().SaveChangesAsync();
return result;
}
/// <summary>
/// This node's compile state for <paramref name="method"/>, or <c>null</c> on a host that
/// runs no inbound API executor.
/// </summary>
/// <param name="sp">The service provider.</param>
/// <param name="method">The API method, with its current stored script.</param>
/// <returns>The compile state, or <c>null</c>.</returns>
public static ApiMethodCompileStatus? GetStatus(IServiceProvider sp, ApiMethod method)
{
var executor = sp.GetService<InboundScriptExecutor>();
if (executor is null)
return null;
var state = executor.GetCompileState(method.Name, method.Script);
return new ApiMethodCompileStatus(state.IsCompiled, state.IsKnownBad, state.LastCompileError, NodeName(sp));
}
private static async Task<ApiMethod> LoadAsync(IServiceProvider sp, int apiMethodId) =>
await sp.GetRequiredService<IInboundApiRepository>().GetApiMethodByIdAsync(apiMethodId)
?? throw new ManagementCommandException($"ApiMethod with ID {apiMethodId} not found.");
private static InboundScriptExecutor RequireExecutor(IServiceProvider sp) =>
sp.GetService<InboundScriptExecutor>()
?? throw new ManagementCommandException("The inbound API script executor is not available on this node.");
private static string? NodeName(IServiceProvider sp) => sp.GetService<INodeIdentityProvider>()?.NodeName;
}
@@ -328,6 +328,9 @@ public class ManagementActor : ReceiveActor
or CreateSharedSchemaCommand or UpdateSharedSchemaCommand or DeleteSharedSchemaCommand
or CreateDatabaseConnectionDefCommand or UpdateDatabaseConnectionDefCommand or DeleteDatabaseConnectionDefCommand
or CreateApiMethodCommand or UpdateApiMethodCommand or DeleteApiMethodCommand
// ScadaBridge#75: operator compile diagnostics + targeted recompile, the same
// authority as UpdateApiMethod (which recompiles as a side effect).
or ValidateApiMethodCommand or RecompileApiMethodCommand
or CreateTemplateFolderCommand or RenameTemplateFolderCommand
or MoveTemplateFolderCommand or ReorderTemplateFolderCommand or DeleteTemplateFolderCommand
or MoveTemplateToFolderCommand
@@ -526,6 +529,8 @@ public class ManagementActor : ReceiveActor
CreateApiMethodCommand cmd => await HandleCreateApiMethod(sp, cmd, user.Username),
UpdateApiMethodCommand cmd => await HandleUpdateApiMethod(sp, cmd, user.Username),
DeleteApiMethodCommand cmd => await HandleDeleteApiMethod(sp, cmd, user.Username),
ValidateApiMethodCommand cmd => await ApiMethodCompileOperations.ValidateAsync(sp, cmd.ApiMethodId),
RecompileApiMethodCommand cmd => await ApiMethodCompileOperations.RecompileAsync(sp, cmd.ApiMethodId, user.Username),
// Security
ListRoleMappingsCommand => await HandleListRoleMappings(sp),
@@ -3802,7 +3807,26 @@ public class ManagementActor : ReceiveActor
private static async Task<object?> HandleGetApiMethod(IServiceProvider sp, GetApiMethodCommand cmd)
{
var repo = sp.GetRequiredService<IInboundApiRepository>();
return await repo.GetApiMethodByIdAsync(cmd.ApiMethodId);
var method = await repo.GetApiMethodByIdAsync(cmd.ApiMethodId);
if (method is null)
return null;
// ScadaBridge#75: the entity fields stay top-level (additive), plus this node's
// compile state so an operator can see why a method fails without firing it.
var status = ApiMethodCompileOperations.GetStatus(sp, method);
return new
{
method.Id,
method.Name,
method.Script,
method.ParameterDefinitions,
method.ReturnDefinition,
method.TimeoutSeconds,
method.TraceAllCalls,
status?.IsCompiled,
status?.IsKnownBad,
status?.LastCompileError,
CompileStateNode = status?.Node,
};
}
private static async Task<object?> HandleCreateApiMethod(IServiceProvider sp, CreateApiMethodCommand cmd, string user)
@@ -712,6 +712,31 @@ public class InboundScriptExecutorTests
Assert.True(_executor.CompileAndRegister(good));
}
/// <summary>
/// ScadaBridge#75: a failure on the data-plane (lazy-compile) path is observable per method
/// with its real diagnostic while the caller only gets the generic message; the state is
/// judged against the current script text and clears on a successful compile.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task CompileState_ReportsTheLazyCompileFailureWithItsDiagnostic()
{
var method = new ApiMethod("diag", "return doesNotExist;") { Id = 1, TimeoutSeconds = 10 };
Assert.Equal(new ApiMethodCompileState(false, false, null), _executor.GetCompileState("diag", method.Script));
var result = await _executor.ExecuteAsync(method, new Dictionary<string, object?>(), _route, TimeSpan.FromSeconds(10));
Assert.Equal("Script compilation failed for this method", result.ErrorMessage);
var state = _executor.GetCompileState("diag", method.Script);
Assert.True(state.IsKnownBad);
Assert.Contains("doesNotExist", state.LastCompileError);
// A different script text is a different revision: the record no longer describes it.
Assert.False(_executor.GetCompileState("diag", "return 1;").IsKnownBad);
Assert.True(_executor.CompileAndRegister(new ApiMethod("diag", "return 1;") { Id = 1, TimeoutSeconds = 10 }));
Assert.Equal(new ApiMethodCompileState(true, false, null), _executor.GetCompileState("diag", "return 1;"));
}
// --- _knownBadMethods must be bounded so a spam attack of
// unique method names cannot grow the cache without bound. ---
@@ -0,0 +1,135 @@
using System.Text.Json;
using Akka.Actor;
using Akka.TestKit.Xunit2;
using Microsoft.Extensions.DependencyInjection;
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>
/// ScadaBridge#75: an inbound method whose script fails to compile answers callers with a
/// generic message and its real diagnostic used to reach only the central log. The
/// management surface now returns the diagnostics (<c>ValidateApiMethod</c>), shows the
/// node's compile state on <c>GetApiMethod</c>, and can clear a stale failure without a
/// full-replace update (<c>RecompileApiMethod</c>).
/// </summary>
public class ApiMethodCompileDiagnosticsTests : TestKit
{
private const string BrokenScript = "return doesNotExist;";
private readonly ServiceCollection _services = new ServiceCollection().AddTestConfigurationTransaction();
private readonly IInboundApiRepository _apiRepo = Substitute.For<IInboundApiRepository>();
private readonly IAuditService _audit = Substitute.For<IAuditService>();
private readonly InboundScriptExecutor _executor =
new(NullLogger<InboundScriptExecutor>.Instance, Substitute.For<IServiceProvider>());
private readonly ApiMethod _method = new("Broken", BrokenScript) { Id = 5, TimeoutSeconds = 10 };
/// <summary>Initializes the harness with a real executor and a stubbed repository.</summary>
public ApiMethodCompileDiagnosticsTests()
{
_services.AddScoped(_ => _apiRepo);
_services.AddScoped(_ => _audit);
_services.AddSingleton(_executor);
_apiRepo.GetApiMethodByIdAsync(5, Arg.Any<CancellationToken>()).Returns(_method);
_apiRepo.SaveChangesAsync(Arg.Any<CancellationToken>()).Returns(1);
}
private JsonElement Send(object command, string role = "Designer")
{
var actor = Sys.ActorOf(Props.Create(() => new ManagementActor(
_services.BuildServiceProvider(), NullLogger<ManagementActor>.Instance)));
actor.Tell(new ManagementEnvelope(
new AuthenticatedUser("designer", "Designer", new[] { role }, Array.Empty<string>()),
command, Guid.NewGuid().ToString("N")));
var response = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(30));
return JsonDocument.Parse(response.JsonData).RootElement.Clone();
}
/// <summary>
/// After the data plane recorded a compile failure, GetApiMethod reports it with the real
/// compiler diagnostic, alongside the unchanged entity fields.
/// </summary>
[Fact]
public void Get_AfterACompileFailure_ReportsKnownBadWithTheDiagnostic()
{
Assert.False(_executor.CompileAndRegister(_method));
var result = Send(new GetApiMethodCommand(5), role: "Viewer");
Assert.Equal("Broken", result.GetProperty("name").GetString());
Assert.Equal(BrokenScript, result.GetProperty("script").GetString());
Assert.True(result.GetProperty("isKnownBad").GetBoolean());
Assert.False(result.GetProperty("isCompiled").GetBoolean());
Assert.Contains("doesNotExist", result.GetProperty("lastCompileError").GetString());
}
/// <summary>Validate returns the diagnostics and leaves the handler cache untouched.</summary>
[Fact]
public void Validate_ReturnsDiagnostics_WithoutRecordingAFailure()
{
var result = Send(new ValidateApiMethodCommand(5));
Assert.False(result.GetProperty("success").GetBoolean());
Assert.Contains(result.GetProperty("errors").EnumerateArray(),
e => e.GetString()!.Contains("doesNotExist", StringComparison.Ordinal));
var state = _executor.GetCompileState("Broken", BrokenScript);
Assert.False(state.IsKnownBad);
Assert.False(state.IsCompiled);
}
/// <summary>
/// A failure recorded for unchanged script text is cleared by a recompile once the cause
/// outside the script is gone — here modelled by the stored row now compiling — without
/// re-sending the entity. The recompile is audited.
/// </summary>
[Fact]
public void Recompile_ClearsAStaleFailure_AndIsAudited()
{
Assert.False(_executor.CompileAndRegister(_method));
_method.Script = "return 1;";
var result = Send(new RecompileApiMethodCommand(5));
Assert.True(result.GetProperty("success").GetBoolean());
Assert.Empty(result.GetProperty("errors").EnumerateArray());
var state = _executor.GetCompileState("Broken", "return 1;");
Assert.True(state.IsCompiled);
Assert.False(state.IsKnownBad);
_audit.Received(1).LogAsync("designer", "Recompile", "ApiMethod", "5", "Broken",
Arg.Any<object?>(), Arg.Any<CancellationToken>());
}
/// <summary>A recompile of a still-broken script records the new diagnostics.</summary>
[Fact]
public void Recompile_StillBroken_RecordsTheDiagnostics()
{
var result = Send(new RecompileApiMethodCommand(5));
Assert.False(result.GetProperty("success").GetBoolean());
var state = _executor.GetCompileState("Broken", BrokenScript);
Assert.True(state.IsKnownBad);
Assert.Contains("doesNotExist", state.LastCompileError);
}
/// <summary>Validate and recompile are Designer operations, like UpdateApiMethod.</summary>
/// <param name="command">The command name to send.</param>
[Theory]
[InlineData("Validate")]
[InlineData("Recompile")]
public void ValidateAndRecompile_RequireDesigner(string command)
{
var actor = Sys.ActorOf(Props.Create(() => new ManagementActor(
_services.BuildServiceProvider(), NullLogger<ManagementActor>.Instance)));
object message = command == "Validate" ? new ValidateApiMethodCommand(5) : new RecompileApiMethodCommand(5);
actor.Tell(new ManagementEnvelope(
new AuthenticatedUser("viewer", "Viewer", new[] { "Viewer" }, Array.Empty<string>()),
message, Guid.NewGuid().ToString("N")));
ExpectMsg<ManagementUnauthorized>(TimeSpan.FromSeconds(10));
}
}
@@ -116,6 +116,9 @@ public class RequiredRoleMatrixTests
["CreateApiMethod"] = [Roles.Designer],
["UpdateApiMethod"] = [Roles.Designer],
["DeleteApiMethod"] = [Roles.Designer],
// ScadaBridge#75: compile diagnostics + targeted recompile — UpdateApiMethod's authority.
["ValidateApiMethod"] = [Roles.Designer],
["RecompileApiMethod"] = [Roles.Designer],
["CreateTemplateFolder"] = [Roles.Designer],
["RenameTemplateFolder"] = [Roles.Designer],
["MoveTemplateFolder"] = [Roles.Designer],