fix(adminui): bound the diagnostics marker payload on both the endpoint and the circuit (D-a review r1)
I-1 - a non-C# paste produced thousands of Roslyn markers. The Problems-list report sent them all as one JS->.NET message, which could exceed the 1 MB hub limit and kill the circuit. The JS now sends at most MaxReportedMarkers (200) markers, errors first, each message cut to 300 chars, plus the total. ScriptProblemReport.FromEditor clamps again on arrival, and the Problems list shows "showing N of Total" / "+N more". The diagnostics endpoint also bounds its response: Distinct, errors first, MaxDiagnosticMarkers (1000), messages cut to 1000 chars. An error is always kept, so editor-accepts == publish-accepts. M-1 - drop the per-instance compilation lineage; analyses derive from BaseCompilation. M-2 - the VirtualTag modal clears its problems (and editor ref) on close and on a script reload. M-5 - an over-limit text is announced at blur (OnSyncRefused), not only at Save. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
@@ -77,19 +77,14 @@ public sealed class ScriptAnalysisService
|
||||
return compilation;
|
||||
}
|
||||
|
||||
// This scope's current compilation (one per scoped service). Each analysis swaps its tree in with
|
||||
// ReplaceSyntaxTree; a lost race between two concurrent calls on one scope is harmless because every
|
||||
// compilation in the lineage is a complete, valid single-tree compilation.
|
||||
private CSharpCompilation _compilation = BaseCompilation;
|
||||
|
||||
// userSource is appended after Preamble, then the method/class are closed.
|
||||
private (SyntaxTree Tree, CSharpCompilation Compilation, SemanticModel Model, int PreambleLength) Analyze(
|
||||
// userSource is appended after Preamble, then the method/class are closed. Every analysis derives straight
|
||||
// from the shared BaseCompilation: the analysis endpoints are minimal-API handlers, so a scoped service lives
|
||||
// for one HTTP request and a per-instance lineage would never be more than one step long.
|
||||
private static (SyntaxTree Tree, CSharpCompilation Compilation, SemanticModel Model, int PreambleLength) Analyze(
|
||||
string userSource, CancellationToken ct)
|
||||
{
|
||||
var tree = WrapAndParse(userSource, ct);
|
||||
var current = _compilation;
|
||||
var compilation = current.ReplaceSyntaxTree(current.SyntaxTrees.Single(), tree);
|
||||
_compilation = compilation;
|
||||
var compilation = BaseCompilation.ReplaceSyntaxTree(BaseCompilation.SyntaxTrees[0], tree);
|
||||
return (tree, compilation, compilation.GetSemanticModel(tree), Preamble.Length);
|
||||
}
|
||||
|
||||
@@ -159,7 +154,7 @@ public sealed class ScriptAnalysisService
|
||||
foreach (var inv in SetVirtualTagCalls(tree.GetRoot(ct), preambleLength))
|
||||
markers.Add(ToMarker(tree.GetMappedLineSpan(inv.Span), 4, SetVirtualTagInertMessage, "OTSCRIPT_SETVTAG_INERT"));
|
||||
|
||||
return new DiagnoseResponse(markers.Distinct().ToList());
|
||||
return new DiagnoseResponse(Bound(markers));
|
||||
}
|
||||
catch (OperationCanceledException) when (ct.IsCancellationRequested)
|
||||
{
|
||||
@@ -216,7 +211,7 @@ public sealed class ScriptAnalysisService
|
||||
$"reference named '{refName}'. Add a matching reference or correct the path.",
|
||||
"OTSCRIPT_EQUIPREF"));
|
||||
}
|
||||
return new DiagnoseResponse(markers.Distinct().ToList());
|
||||
return new DiagnoseResponse(Bound(markers));
|
||||
}
|
||||
catch (OperationCanceledException) when (ct.IsCancellationRequested)
|
||||
{
|
||||
@@ -229,6 +224,18 @@ public sealed class ScriptAnalysisService
|
||||
}
|
||||
}
|
||||
|
||||
// Bounds what the diagnostics endpoint returns (and so what Monaco renders and the Problems list is fed from):
|
||||
// de-duplicated, errors first then by position, at most MaxDiagnosticMarkers, each message cut to
|
||||
// MaxDiagnosticMessageChars. Ordering before the cut keeps an error whenever one exists, so the editor still
|
||||
// rejects exactly what publish rejects even for a pathological (e.g. non-C#) paste.
|
||||
private static IReadOnlyList<DiagnosticMarker> Bound(IEnumerable<DiagnosticMarker> markers)
|
||||
=> ScriptProblems.Order(markers.Distinct())
|
||||
.Take(ScriptEditorLimits.MaxDiagnosticMarkers)
|
||||
.Select(m => m.Message.Length <= ScriptEditorLimits.MaxDiagnosticMessageChars
|
||||
? m
|
||||
: m with { Message = ScriptProblems.Truncate(m.Message, ScriptEditorLimits.MaxDiagnosticMessageChars) })
|
||||
.ToList();
|
||||
|
||||
private const string SetVirtualTagInertMessage =
|
||||
"ctx.SetVirtualTag(...) has no effect at runtime: cross-tag writes are dropped in single-tag mode (the " +
|
||||
"cascade engine is not wired into the host). The script compiles and publishes, but this write will not " +
|
||||
|
||||
@@ -16,4 +16,26 @@ public static class ScriptEditorLimits
|
||||
|
||||
/// <summary>The largest serialized editor value the JS will send, leaving 64 KB of frame headroom.</summary>
|
||||
public const int MaxEditorSyncBytes = (int)HubMaximumReceiveMessageSize - 64 * 1024;
|
||||
|
||||
/// <summary>
|
||||
/// The most diagnostic markers the editor reports to .NET (for the Problems list) in one circuit message.
|
||||
/// A non-C# paste produces several Roslyn errors per line; uncapped, a few thousand markers exceed the hub
|
||||
/// limit and tear the circuit down. The JS sends the first N in display order (errors first) plus the total.
|
||||
/// Monaco's own in-editor squiggles are local and stay uncapped (up to <see cref="MaxDiagnosticMarkers"/>).
|
||||
/// </summary>
|
||||
public const int MaxReportedMarkers = 200;
|
||||
|
||||
/// <summary>Each reported marker's message is truncated to this many characters before it crosses the circuit.</summary>
|
||||
public const int MaxReportedMessageChars = 300;
|
||||
|
||||
/// <summary>
|
||||
/// The most markers the diagnostics endpoint returns (errors first). Bounds the analysis response and the
|
||||
/// marker set Monaco renders for pathological input; an error is always kept when one exists, so the
|
||||
/// editor still rejects exactly what publish rejects.
|
||||
/// </summary>
|
||||
public const int MaxDiagnosticMarkers = 1000;
|
||||
|
||||
/// <summary>Each diagnostics-endpoint marker message is truncated to this many characters (a Roslyn message
|
||||
/// can quote an arbitrarily long pasted token).</summary>
|
||||
public const int MaxDiagnosticMessageChars = 1000;
|
||||
}
|
||||
|
||||
@@ -50,5 +50,54 @@ public static class ScriptProblems
|
||||
return string.Join(", ", parts);
|
||||
}
|
||||
|
||||
/// <summary>A one-line count for a (possibly capped) report, e.g. <c>"200 errors (showing 200 of 4321)"</c>.</summary>
|
||||
/// <param name="report">The report the editor sent.</param>
|
||||
/// <returns>The summary of the shown markers, with the shown/total suffix when some were left out.</returns>
|
||||
public static string Summary(ScriptProblemReport report)
|
||||
{
|
||||
var summary = Summary(report.Markers.ToList());
|
||||
return report.Omitted > 0 ? $"{summary} (showing {report.Markers.Count} of {report.Total})" : summary;
|
||||
}
|
||||
|
||||
/// <summary>Cuts <paramref name="text"/> to at most <paramref name="maxChars"/> characters, ending in an ellipsis.</summary>
|
||||
/// <param name="text">The text.</param>
|
||||
/// <param name="maxChars">The maximum length, ellipsis included (at least 1).</param>
|
||||
/// <returns>The text, unchanged when it already fits.</returns>
|
||||
public static string Truncate(string text, int maxChars)
|
||||
=> text.Length <= maxChars ? text : string.Concat(text.AsSpan(0, Math.Max(0, maxChars - 1)), "…");
|
||||
|
||||
private static string Plural(int n, string noun) => n == 1 ? $"1 {noun}" : $"{n} {noun}s";
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// What the editor reported for the Problems list: at most <see cref="ScriptEditorLimits.MaxReportedMarkers"/>
|
||||
/// markers in display order (errors first) plus the <see cref="Total"/> the diagnostics pass produced. The JS caps
|
||||
/// the report before it crosses the circuit — one hub message, bounded by the 1 MB receive limit — and
|
||||
/// <see cref="FromEditor"/> clamps it again on arrival.
|
||||
/// </summary>
|
||||
/// <param name="Markers">The markers shown.</param>
|
||||
/// <param name="Total">How many markers the diagnostics pass produced (≥ <c>Markers.Count</c>).</param>
|
||||
public sealed record ScriptProblemReport(IReadOnlyList<DiagnosticMarker> Markers, int Total)
|
||||
{
|
||||
/// <summary>No problems.</summary>
|
||||
public static readonly ScriptProblemReport Empty = new(Array.Empty<DiagnosticMarker>(), 0);
|
||||
|
||||
/// <summary>How many markers were left out of <see cref="Markers"/>.</summary>
|
||||
public int Omitted => Math.Max(0, Total - Markers.Count);
|
||||
|
||||
/// <summary>Builds a report from what the editor JS sent, re-applying the circuit caps defensively.</summary>
|
||||
/// <param name="markers">The markers sent (already capped by the JS).</param>
|
||||
/// <param name="total">The total the JS counted.</param>
|
||||
/// <returns>The clamped report.</returns>
|
||||
public static ScriptProblemReport FromEditor(IReadOnlyList<DiagnosticMarker>? markers, int total)
|
||||
{
|
||||
if (markers is null || markers.Count == 0) return total <= 0 ? Empty : new(Array.Empty<DiagnosticMarker>(), total);
|
||||
var shown = ScriptProblems.Order(markers)
|
||||
.Take(ScriptEditorLimits.MaxReportedMarkers)
|
||||
.Select(m => m.Message.Length <= ScriptEditorLimits.MaxReportedMessageChars
|
||||
? m
|
||||
: m with { Message = ScriptProblems.Truncate(m.Message, ScriptEditorLimits.MaxReportedMessageChars) })
|
||||
.ToList();
|
||||
return new ScriptProblemReport(shown, Math.Max(total, markers.Count));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user