fix(centralui): diagnose cache keyed on editor context; surface blocking-wait advisory as Monaco warning (review 2026-10-21 07/C-1, lane F)

2026-10-22 08:45:40 -04:00
parent 5c8f6ff7aa
commit 614aade610
2 changed files with 93 additions and 1 deletions
@@ -136,7 +136,7 @@ public class ScriptAnalysisService
        if (string.IsNullOrEmpty(request.Code))
            return new DiagnoseResponse(Array.Empty<DiagnosticMarker>());

-        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);
+
+    /// <summary>
+    /// Surfaces <see cref="SharedTrust.BlockingWaitAnalyzer"/> advisories (a script blocking on a
+    /// Task with <c>.Result</c>/<c>.Wait()</c>/<c>GetAwaiter().GetResult()</c>) as warning markers
+    /// on the offending line. Advisory only; the analyzer failing never costs the other diagnostics.
+    /// </summary>
+    private static IEnumerable<DiagnosticMarker> FindBlockingWaits(string code, Type globalsType)
+    {
+        IReadOnlyList<string> 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");
+        }
+    }
+
+    /// <summary>
+    /// 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).
+    /// </summary>
+    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;
@@ -201,6 +201,53 @@ public class ScriptAnalysisServiceTests
        Assert.DoesNotContain(resp.Markers, m => m.Code == "SCADA003");
    }

+    /// <summary>Review 2026-10-21 07/C-1: identical code with a changed declared-parameter set must not be served the stale cached marker.</summary>
+    [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");
+    }
+
+    /// <summary>Review 2026-10-21 07/C-1: byte-identical scripts on templates with different attribute sets do not share an answer.</summary>
+    [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());
+    }
+
+    /// <summary>Lane F follow-up: a blocking Task wait surfaces as a Monaco warning marker on its line.</summary>
+    [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);
+    }
+
+    /// <summary>A script that only awaits gets no blocking-wait marker.</summary>
+    [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 ───────────────────────────────────────────────────────

    /// <summary>Verifies that completion inside a Parameters string literal returns the declared parameter names.</summary>