Stop Robot Advice from crashing the app on a deep plan (#430)

[erikdarlingdata]

Fixes #430.

What does this PR do?

System.Text.Json defaults MaxDepth to 64 and throws past it. An operator tree nests once per operator and each level costs two JSON levels (the object, then the Children array), so a plan roughly 30 operators deep exhausts the default. The throw came out of an Avalonia click handler, which Avalonia does not guard — so the process died with no dialog and nothing the user could act on.

The reporter's stack named the cause precisely:

System.Text.Json.JsonException: A possible object cycle was detected. This can either be due to a
cycle or if the object depth is larger than the maximum allowed depth of 64.
Path: $.Statements.OperatorTree.Children.Children...(30 deep)...NodeId
   at PlanViewer.App.Controls.QuerySessionControl.RobotAdvice_Click

Thirty consecutive Children, and that distinction decides the fix. The message offers two causes and they want opposite remedies — raising the ceiling on a genuine cycle just moves the crash. The serialized type is OperatorResult, which has Children and no parent link, so it is a tree and the depth reading is the right one.

Worth recording for whoever hits this next: the internal model it is mapped from, PlanViewer.Core.Models.PlanNode, does carry a Parent back-reference that the parser populates (ShowPlanParser.RelOp.cs:140). Serializing that type directly would be a real cycle needing [JsonIgnore], not a bigger number. Nothing does today, and the new file says so.

Four places serialize this object and none of them could tell when they got it wrong, so the ceiling is now one shared constant:

site	consequence before
QuerySessionControl.RobotAdvice_Click	process crash (the reported bug)
MainWindow.PlanViewer.cs robot advice	process crash — a second entry point that builds the payload independently
McpHelpers.JsonOptions	MCP tool returns an error where the caller expected a plan
AnalyzeCommand JSON + compact options	analyze --json fails on a large plan

1024 is about 500 nested operators against the ~30 that used to fail. It is deliberately paired with a caller-side catch at both UI sites: a serialization limit should never be able to take the app down, so the headroom is not the only thing standing between a deep plan and a crash.

Which component(s) does this affect?

• Desktop App (PlanViewer.App)
• Core Library (PlanViewer.Core)
• CLI Tool (PlanViewer.Cli)
• SSMS Extension (PlanViewer.Ssms)
• Tests
• Documentation

How was this tested?

AnalysisJsonDepthTests (6 cases), including a reproduction — a 60-operator chain that throws on the options the call sites used to build inline and serializes on the shared ones. Without that case the fix would be unfalsifiable: a passing serialize proves nothing if nothing ever failed. It also asserts every level actually made it into the output rather than the serializer stopping quietly partway, and that a tree past the ceiling still throws rather than emitting truncated JSON that reads like a complete plan.

• 137/137 in PlanViewer.Core.Tests, macOS (arm64)
• PlanViewer.App and PlanViewer.Cli build with zero warnings

Synthetic depth chains rather than a plan file — the reporter's plan was too large to anonymize, and depth is the whole variable, so constructing it directly tests the boundary precisely at 64 and at the new ceiling.

One note on the local run. The full suite reports all 137 passing in 54 ms and then the test host livelocks on shutdown at ~100% CPU; dotnet test prints Catastrophic failure: Test process crashed with exit code 143, which is the SIGTERM I sent it. A 3-second sample shows SuspendEE, inject_activation, CheckActivationSafePoint and thread_get_state/thread_set_state churn inside nested _sigtramp — the known CoreCLR GC-suspension livelock on macOS ARM64, unrelated to this change (sample itself hangs until the host is killed, since it needs the same thread suspension). Mentioning it because it makes a green local run look like a crash, and because #427's watchdog is the thing that catches it.

Checklist

• I have read the contributing guide
• My code builds with zero warnings (dotnet build -c Debug)
• All tests pass (dotnet test)
• I have not introduced any hardcoded credentials or server names

🤖 Generated with Claude Code

[claude]

Reviewed. The core fix (shared AnalysisJson.MaxDepth/AnalysisJson.Indented, the four updated call sites, the catch guards on the two Avalonia click handlers, and the depth tests) is correct and matches the described root cause — OperatorResult is genuinely a tree (no back-reference), so raising MaxDepth is the right fix and not a cycle being papered over.

Two more writers of the same AnalysisResult/OperatorTree graph exist in the repo but weren't brought in line, so the depth bug the PR fixes survives in both:

1. src/PlanViewer.Cli/Commands/QueryStoreCommand.cs:13-22 — this file has its own JsonOptions/CompactJsonOptions (a separate copy of the fields AnalyzeCommand.cs has, which were updated). They're passed into PlanAnalysisRunner.WriteResultFilesAsync at line 429 to serialize the same OperatorTree. Without MaxDepth = AnalysisJson.MaxDepth, planviewer querystore against a live server that has a plan ~30+ operators deep will still throw the depth JsonException there. It's caught by the per-plan catch (line 443) so the process won't crash, but that plan comes back as ERROR: ... in the batch output/summary instead of being analyzed — the exact class of failure this PR set out to close for analyze --json.

2. src/PlanViewer.Web/Services/PlanShareService.cs:49 and :88 — ShareAsync serializes AnalysisResult with default JsonSerializerOptions (implicit MaxDepth of 64), and LoadAsync deserializes it the same way. Both are reachable from the Blazor plan-share feature; a plan deep enough to trip [BUG] Robot Advice crashes the app #430 still can't be shared or re-loaded. ShareAsync's call is wrapped in try/catch in Index.razor so it won't crash the WASM app, but the feature stays broken for exactly the plans this PR targets.

Neither is covered by the new AnalysisJsonDepthTests, since those only exercise AnalysisJson.Indented directly rather than every call site that should be using it.

No other issues found — no new warnings, no untrusted-input/T-SQL concerns (this is pure JSON serialization plumbing), and the SSMS extension/versioning conventions aren't touched by this PR.