Lite's MCP reports every alert group Darling's does

[erikdarlingdata]

Closes #2394 — scope 1 only, read-only parity.

Lite's get_alert_settings reported four groups (cpu, blocking, deadlocks, smtp) where Darling's reports nineteen. The issue's framing holds up completely against the source: nothing about Lite's alerting is narrower. PerformanceMonitor.Alerting evaluates the same alerts on both SKUs against IAlertEngineSettings, and AppAlertEngineSettings already projects every one of Lite's App.Alert* statics onto that interface. So an operator could have tempdb-space, low-disk, PVS, file-growth, long-running-query, long-running-job, failed-job, database-state and analysis alerting all live, and an agent triaging that instance could not read whether any of it was even switched on. The gap was purely the MCP surface, and this closes it.

Thirteen groups are added — poison_wait, long_running_query, tempdb_space, low_disk, pvs, file_growth, long_running_job, failed_job, database_state, cooldown_minutes, excluded_databases, delivery, analysis — spelled Darling's way down to the last key, in Darling's order, because one MCP schema across both apps is what #1839 and #1911 were for.

The two spellings that are easy to get wrong were checked against Darling's emitted shape rather than guessed. low_disk nests the #1136 CRITICAL floors as critical_free_percent / critical_free_gb, dropping the "disk" that the App.AlertDiskCriticalFree* statics carry — naming them after the statics would have read as correct and been a fifth key-level mismatch. And delivery.mode is a value-level alignment like cpu.mode, though it is the one place ToString() is right rather than a mapping: AlertNotificationMode is the shared enum both SKUs run on and Darling's store holds literally a.DeliveryMode.ToString(), so the two cannot drift the way Lite's app-local CpuAlertMode could.

The one group not added

self_alerts — Darling's own store volume, collection staleness/failure counts, and store-job cadence. This is a genuine Darling-only concept rather than an omission of convenience. AppAlertEngineSettings returns shipped constants (10, 30, 10) for three of its four members precisely because a single-instance WPF app has no headless store volume and no fleet collection loop to self-monitor, and Lite has no member at all for the fourth, store_job_cadence_warn_percent. Reporting constants under names that read as knobs would tell an agent it can tune something Lite cannot, and an admitted gap beats an overstated capability. The omission is asserted as a decision, not left to look like an oversight.

smtp stays as Lite's one addition — Lite delivers its own email, where Darling manages delivery credentials outside the settings row and reports no smtp group at all.

Why there is no update_alert_settings

Out of scope, and the issue is right that it needs a design decision first. Darling's tools write config_alert_settings and the service picks the change up through StoreConfigProvider.ApplyToConfig — a live reload with no restart. Lite has no equivalent plane: its settings are App.Alert* statics that the Settings window writes to settings.json, and SaveAlertSettings reads its controls, not the statics — so an open Settings window would silently overwrite an MCP-applied change on the next Save. That wants a change-notification seam (the issue's scope 3), not a tool bolted on next to the hazard.

Tests

McpAlertSettingsKeyTests is extended, not rewritten — its existing assertions are all still there. It grows from pinning three renamed keys to pinning the whole shape, which is the lesson of this issue: the drift it was written to catch had already happened at a scale no per-key assertion would notice.

The new parity assertion derives Darling's shape from Darling's source (BuildAlertSettingsPayload) rather than transcribing it. A hand-copied list of nineteen groups is exactly the artifact that stays green on the day a twentieth arrives. Both directions are asserted, so a key Lite emits that Darling does not fails too — two spellings of one setting across the two apps is the same class of bug as a missing one — and smtp is pinned as the exact set of legitimate Lite-only additions, so a second one cannot appear without someone justifying it here.

The parser proves itself before it is trusted: it asserts the parse found a plausible number of groups and that known anchors (cpu, analysis, self_alerts, and cpu's three nested keys) are present, because a parse that quietly returned nothing would make every assertion built on it vacuously true.

Verification, and what it does and does not cover

Lite.Tests targets net10.0-windows, so it builds on macOS but cannot run; CI is the arbiter. McpAlertSettingsKeyTests is a runtime test, so rather than assert it passes, the logic was verified three ways against the real files:

1. The shipped parser loop, run for real. The DarlingPayloadShape scanning loop was copied verbatim into a throwaway net10.0 harness and run against the actual DarlingMcpAlertTools.cs and McpAlertTools.cs. It reads 19 groups from Darling with correct nested keys, and the parity comparison passes group-for-group and key-for-key.
2. It fails without the fix. The same comparison against dev's pre-fix McpAlertTools.cs reports 6 groups and 13 parity problems — every group this PR adds. A check that passed both before and after would be worse than none.
3. The source-to-wire link. The whole key-name argument rests on the C# identifier being the wire key, so that was verified rather than assumed: a harness reads the real McpHelpers.JsonOptions by reflection (it is internal) and confirms it carries no naming policy, that snake_case identifiers survive verbatim at both nesting levels, that List<string> serializes as an array under its identifier, and that AlertNotificationMode.Summary/PerEvent.ToString() are byte-for-byte the two values Darling's update_alert_settings accepts for delivery.mode.

What that does not cover: the App.Alert* static reads themselves, which need a WPF assembly and therefore CI.

Notes for review

No CHANGELOG entry, deliberately. #2395 is cutting [3.5.1] right now and dev has no [Unreleased] section, so an entry here would either collide with that diff or wrongly claim to be in a release this landed after. Happy to fold one into the next section once the cut lands.

One correction rode along: the class doc-comment claimed the payload is serialized "with a naming policy in McpHelpers.JsonOptions". There is no naming policy — the options object is new() { WriteIndented = false }. The reason the test is runtime rather than source-parsing is still sound (that object owns the mapping and could acquire a policy tomorrow, changing every key without this payload being touched), so the reasoning is kept and the false fact is fixed.

Two things worth separate issues, found while matching the shape — neither touched here:

• Darling's AlertSettingsReadRow reads notify_connection_down_at_startup, connection_refire_minutes, notify_ag_health, ag_lag_alert_seconds, ag_redo_queue_alert_kb and ag_disconnect_refire_minutes, and BuildAlertSettingsPayload reports none of them. The reader's own comment says it was widened so "an MCP client could not see the V33 connection opt-ins or the V35 Availability Group family at all" — the reader was widened and the payload was not, so neither SKU reports the AG family. Lite has the matching statics (App.NotifyAgHealth, AgLagAlertSeconds, AgRedoQueueAlertKb), so this is a both-SKUs gap, and it is Darling's payload that has to move first for parity to mean anything.
• Darling's get_alert_settings emits blocking.wait_threshold_seconds, but update_alert_settings' blocking group dispatches only on enabled and count_threshold — so feeding a full get_alert_settings payload back to it is rejected with Unknown field 'blocking.wait_threshold_seconds'. BuildAlertSettingsPayload's doc comment promises "the same field names update_alert_settings accepts on the way in, so a read → modify → write round-trips", and for that one key it does not.

🤖 Generated with Claude Code

[claude]

Review

Verified this line-by-line against Darling's source of truth (DarlingMcpAlertTools.cs / BuildAlertSettingsPayload) rather than trusting the diff at face value:

• Key-for-key parity: every group and nested key Lite now emits (poison_wait, long_running_query, tempdb_space, low_disk incl. critical_free_percent/critical_free_gb, pvs, file_growth, long_running_job, failed_job, database_state, cooldown_minutes, excluded_databases, delivery, analysis) matches Darling's spelling and document order exactly, with self_alerts correctly omitted and smtp correctly the sole Lite-only addition.
• All referenced App.Alert*/App.Analysis* statics exist in Lite/App.xaml.cs with the expected types (confirmed via grep, since this can't be built on this box — Lite.Tests targets net10.0-windows).
• The "already wired" claim checks out: Lite/Services/AppAlertEngineSettings.cs already forwards every one of these statics into IAlertEngineSettings, so this is genuinely a read-surface-only change, not new alerting behavior.
• No secrets leak: smtp still reports password_configured as a bool, never the credential itself — unchanged.
• Lite/Darling parity: this is a Lite-catches-up-to-Darling fix; Darling's side is untouched and was already correct, so no drift introduced in the other direction.

No correctness, security, or parity issues found. One non-blocking observation: DarlingPayloadShape()'s regex/brace-depth parser in the test file is intricate (comment-stripping + depth-tracked identifier scanning), but it validates itself before use (Assert.InRange, known-anchor checks) and the PR description documents it was run for real against both pre- and post-fix source, so I don't consider it a risk.

Nice work on the self-verification methodology described in the PR body (running the shipped parser against real files, confirming the pre-fix failure mode, and confirming JsonOptions carries no naming policy via reflection) — that's exactly the kind of proof this class of "two apps must agree on wire spelling" bug needs.