From 614aade61022bcd097292e9825d554421be9fd3b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 08:45:40 -0400 Subject: [PATCH] fix(centralui): diagnose cache keyed on editor context; surface blocking-wait advisory as Monaco warning (review 2026-10-21 07/C-1, lane F) --- .../ScriptAnalysis/ScriptAnalysisService.cs | 47 ++++++++++++++++++- .../ScriptAnalysisServiceTests.cs | 47 +++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs index abc8c92c..90536b56 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/ScriptAnalysis/ScriptAnalysisService.cs @@ -136,7 +136,7 @@ public class ScriptAnalysisService if (string.IsNullOrEmpty(request.Code)) return new DiagnoseResponse(Array.Empty()); - var cacheKey = "diag:" + (int)request.Kind + ":" + HashCode(request.Code); + var cacheKey = "diag:" + (int)request.Kind + ":" + HashCode(request.Code) + ":" + ContextHash(request); if (_cache.TryGetValue(cacheKey, out DiagnoseResponse? cached) && cached is not null) return cached; @@ -171,9 +171,54 @@ public class ScriptAnalysisService markers.AddRange(FindUnknownChildren(tree, request.Children)); } + markers.AddRange(FindBlockingWaits(request.Code, GlobalsTypeFor(request.Kind))); + return Cache(cacheKey, new DiagnoseResponse(markers)); } + private static readonly System.Text.RegularExpressions.Regex AdvisoryLine = + new(@"\(line (\d+)\)", System.Text.RegularExpressions.RegexOptions.Compiled); + + /// + /// Surfaces advisories (a script blocking on a + /// Task with .Result/.Wait()/GetAwaiter().GetResult()) as warning markers + /// on the offending line. Advisory only; the analyzer failing never costs the other diagnostics. + /// + private static IEnumerable FindBlockingWaits(string code, Type globalsType) + { + IReadOnlyList advisories; + try { advisories = SharedTrust.BlockingWaitAnalyzer.Find(code, globalsType); } + catch (Exception) { yield break; } + + var lines = code.Split('\n'); + foreach (var advisory in advisories) + { + var m = AdvisoryLine.Match(advisory); + var line = m.Success && int.TryParse(m.Groups[1].Value, out var n) ? n : 1; + line = Math.Clamp(line, 1, lines.Length); + var endColumn = Math.Max(2, lines[line - 1].TrimEnd('\r').Length + 1); + yield return new DiagnosticMarker(4, line, 1, line, endColumn, advisory, "SCADA_BLOCKING_WAIT"); + } + } + + /// + /// Hash of the request context the context-dependent walkers read (declared parameters, + /// self attributes, children, parent, sibling scripts) so two requests with identical code but + /// different context never share a cached answer (review 2026-10-21 07/C-1). + /// + private static string ContextHash(DiagnoseRequest request) + { + var json = JsonSerializer.Serialize(new + { + request.DeclaredParameters, + request.SiblingScripts, + request.SelfAttributes, + request.Children, + request.Parent, + }); + return HashCode(json); + } + private const int SandboxMaxTimeoutSeconds = 10; private const int SandboxDefaultTimeoutSeconds = 5; private const int SandboxMaxConsoleChars = 32_000; diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/ScriptAnalysis/ScriptAnalysisServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/ScriptAnalysis/ScriptAnalysisServiceTests.cs index 2b9b9b1c..52a5a677 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/ScriptAnalysis/ScriptAnalysisServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/ScriptAnalysis/ScriptAnalysisServiceTests.cs @@ -201,6 +201,53 @@ public class ScriptAnalysisServiceTests Assert.DoesNotContain(resp.Markers, m => m.Code == "SCADA003"); } + /// Review 2026-10-21 07/C-1: identical code with a changed declared-parameter set must not be served the stale cached marker. + [Fact] + public void Diagnose_CacheKey_IncludesDeclaredParameters() + { + const string code = "var x = Parameters[\"x\"];"; + var before = _svc.Diagnose(new DiagnoseRequest(code, DeclaredParameters: new[] { "name" })); + var after = _svc.Diagnose(new DiagnoseRequest(code, DeclaredParameters: new[] { "name", "x" })); + + Assert.Contains(before.Markers, m => m.Code == "SCADA003"); + Assert.DoesNotContain(after.Markers, m => m.Code == "SCADA003"); + } + + /// Review 2026-10-21 07/C-1: byte-identical scripts on templates with different attribute sets do not share an answer. + [Fact] + public void Diagnose_CacheKey_IncludesSelfAttributes() + { + const string code = "var v = Attributes[\"Speed\"];"; + var without = _svc.Diagnose(new DiagnoseRequest(code, SelfAttributes: new[] { new AttributeShape("Other", "String") })); + var with = _svc.Diagnose(new DiagnoseRequest(code, SelfAttributes: new[] { new AttributeShape("Speed", "Double") })); + + Assert.NotEqual( + without.Markers.Select(m => m.Message).ToList(), + with.Markers.Select(m => m.Message).ToList()); + } + + /// Lane F follow-up: a blocking Task wait surfaces as a Monaco warning marker on its line. + [Fact] + public void BlockingWait_ReportsWarningMarkerOnItsLine() + { + var resp = _svc.Diagnose(new DiagnoseRequest( + "var t = System.Threading.Tasks.Task.FromResult(1);\nvar r = t.Result;\nreturn r;")); + + var marker = Assert.Single(resp.Markers, m => m.Code == "SCADA_BLOCKING_WAIT"); + Assert.Equal(4, marker.Severity); + Assert.Equal(2, marker.StartLineNumber); + Assert.Contains(".Result", marker.Message); + } + + /// A script that only awaits gets no blocking-wait marker. + [Fact] + public void AwaitedTask_NoBlockingWaitMarker() + { + var resp = _svc.Diagnose(new DiagnoseRequest( + "var t = System.Threading.Tasks.Task.FromResult(1);\nreturn await t;")); + Assert.DoesNotContain(resp.Markers, m => m.Code == "SCADA_BLOCKING_WAIT"); + } + // ── Completions ─────────────────────────────────────────────────────── /// Verifies that completion inside a Parameters string literal returns the declared parameter names.