MCP write tools report what happened: every server you add lands in a counter, a partial name cannot delete the wrong server, an omitted field is never a clear, and a mute that matched nothing says so (#3541 A14)

[erikdarlingdata]

Every MCP write on this surface now reports what it DID, not what it was asked

This is the A14 "Write contracts" item of the payload-contract campaign (#3541), plus the one-line /api/read/get_alert_history rider parked on the issue by the A3 lane. PARTIAL for #3541 — A14 ticked, the rider done; the rest of the checklist is other lanes.

The rule under test is the contract's ninth: writes report what happened. Five write tools across both SKUs failed it in five different ways, and the common shape is worth naming because it is the shape every fix here takes: each tool returned the outcome it intended — muted, failed: 0, removed, the stored row — and never checked that outcome against the input it was handed. An agent that cannot see the code reads those payloads as facts.

The five defects and what the tools say now

1. add_servers: a collided server was counted in no counter. #2280 added a fifth per-row status, collides (the probe reached a database another registration already covers; the entry is refused). The summary was three ad-hoc Count(...) filters written before #2280, naming four statuses. A batch of three with one collision summarised as {added: 2, skipped: 0, failed: 0} — a clean run, with the collided server silently unmonitored. Now: {requested, added, skipped, collided, failed, results}, where every per-row status is mapped to exactly one counter by CounterOfStatus, the four counters sum to requested by construction, and the aggregator throws on a status the map does not know rather than dropping it. Statuses became constants (AddStatus) so a sixth cannot be introduced as a bare literal. collided is its own counter, not a kind of failed, because the remedy differs: a failed entry is retried after fixing the connection; a collided one must not be retried as sent.

2. The instruction table still denied Entra. #3484 accepted ServicePrincipal and ManagedIdentity; the add_servers row in DarlingMcpInstructions went on saying "Entra/MFA/Service-Principal/Managed-Identity auth is invalid (Windows/SQL only)" for a release. The row now matches the parser exactly — the two non-interactive modes accepted, the interactive trio (MFA / device-code / default-credential) invalid — and picks up engine / port and the new counters. A pin derives the accepted set from ParseRequest and holds the table to it.

3. remove_server: a partial name could delete the wrong server. It resolved through the read resolver, whose partial match is first-wins ordered by storage name. For a read that is a convenience — the payload names the server it landed on and the caller re-asks. On a DELETE the payload is the damage: a fragment two siblings contain removed whichever sorted first. The write now reads the same registry rows but with its own rule: exact match (storage or display name) first; a partial match honored only when unique; anything else returns {status: "ambiguous", matched_by, candidates: [{server, display_name}]} and deletes nothing. Exact matches are collected, not first-taken — display_name is not unique, so two registrations sharing one are ambiguous too. Partial matching survives because the description has promised it since the tool shipped and a unique partial is something the tool can honor; what it cannot honor is a choice. Success now carries matched_by: "exact" | "partial".

4. update_custom_view cleared an omitted description; update_custom_alert_rule kept it. Two tools, one field, opposite rules — and neither offered a way to clear at all (the rule tool's parameter said "this cannot clear it"). Over MCP an omitted argument and an explicit null both arrive as C# null, so null must mean unchanged (a caller sending {enabled: false} to pause a rule is not asking to lose its description), which leaves the empty string as the only in-band clear. One helper, ResolveOptionalText (omitted → current, empty/whitespace → NULL, text → text), now sits behind both tools; the view tool reads the row first so it has a current value to keep. Both descriptions and both parameter descriptions state the vocabulary and name each other.

5. mute_analysis_finding returned muted for any hash — both SKUs. A mistyped hash, a hash from another store, a hash whose INSERT Darling's store swallowed and logged: all {status: "muted"}. The registry is a pattern registry (no row references a finding; the filter phase consults it by hash on every pass), so a hash matching nothing today is a legitimate registration — the pattern may return after retention purged its history — and the most likely shape of a typo. So the count is a disclosure, not a gate: the response now carries registered and matched_now (stored findings in the mute's scope carrying that hash, counted after the write so both are read against one moment), with status: "muted" when ≥ 1 and "muted_unmatched" when 0, plus a note saying what to check. On Darling, PgFindingStore.MuteStoryAsync now returns whether the row landed (the swallow-and-log stays for the viewer's button; the caller learns which of the two things happened), and a failed write surfaces as status: "error" instead of "muted". A blank hash is refused as invalid on both SKUs. Both FindingStores gain CountStoredFindingsAsync(int? serverId, string hash), a plain equality on the existing idx_analysis_findings_hash (plus server_id when scoped) — sub-second by shape on the retained population, and off the alert path regardless.

Rider. /api/read/get_alert_history did not pass or advertise include_dismissed (added to the MCP tool by #3594), so a web/API caller received dismissed_excluded_count — "N rows were hidden" — with no wire key to un-hide them. The dispatch passes it by name through QueryBool like every other optional boolean, and the catalog entry advertises it as an optional bool defaulting to false.

Why these shapes, and what was rejected

• A map the aggregator refuses to serialize without, over a fourth Count(...) filter: the defect was a status added after the counters were written. A filter list has to be remembered; a map with a throw and a reflection census does not.
• Unique-partial over exact-only for remove_server: exact-only would break the documented contract and refuse names the tool can honor unambiguously. The contract rule is "refuse what you cannot honor," and a unique fragment is honorable.
• Register-and-disclose over refuse for an unmatched mute: refusing would block a legitimate pre-registration and buy nothing (a junk row from a typo can never match). The status vocabulary makes the miss un-missable without lying about the write.
• Empty string = clear, on both tools, over "match the alert-rule tool exactly" (which could not clear): a field that can be set but never cleared is a contract that cannot be honored on one side. Adding the clear to both is the one-vocabulary fix.
• ResolveOptionalText lives on the alert-rule tool (the newer contract's home) and the view tool calls it; a census pins both call sites to the one method.

Blast radius

• add_servers envelope gains requested and collided; existing keys unchanged. Per-row status strings unchanged (collides stays collides — a public contract already).
• remove_server gains matched_by on success and a new ambiguous status. A caller relying on first-wins partial deletion now gets a refusal — which is the point.
• mute_analysis_finding gains registered, matched_now, note; status is muted_unmatched where it used to say muted for a hash with no stored finding. The existing Darling live test's all-servers case asserted muted for exactly that situation; it now asserts the truthful status.
• PgFindingStore.MuteStoryAsync → Task<bool>, DarlingAnalysisService.MuteFindingAsync → Task<bool>. Existing await callers (the viewer's MuteFindingAsync) compile unchanged. Lite's twins keep Task because Lite's store throws instead of swallowing; the caller-visible contract is the same.
• update_custom_view performs one primary-key SELECT before its UPDATE. No store SQL changed on either tool; no schema, no rungs.
• The read resolver (DarlingServerResolver) is untouched; every read tool resolves exactly as before.

What this does NOT do

• Does not touch the frozen deprecated/Dashboard twin of mute_analysis_finding (same defect exists there — reported to the coordinator as out-of-lane per the file boundary).
• Does not rename the collides status or the skipped counter.
• Does not de-duplicate mute registrations (muting the same hash twice still writes two rows, as before).
• Does not change how remove_server resolves against the registry (a server added but never connected is still absent from servers, as before).

Test plan

All new tests are compile-verified here (zero warnings, all four projects) and first executed in CI — the test projects are net10.0-windows. Because CI is the first run, every pure-logic and source-text pin was additionally executed through a throwaway net10.0 console harness against the built assemblies, and every live path through a throwaway PG18/TimescaleDB container (Darling) and a real DuckDB file (Lite): all green, containers and harnesses torn down. That harness caught one shipping defect before commit: the count's scope test was serverId > 0, and a server_id is an FNV hash cast to int, so half of all real ids are negative — a > 0 test counted those servers' scoped mutes fleet-wide. It is now is not (null or 0), and the Lite test's second server id is negative on purpose.

New, ungated (Darling.Tests):

• EveryAddStatus_HasExactlyOneSummaryCounter_AndNoCounterIsUnnamed — status set == map key set, every counter named by the envelope, collides → collided.
• Aggregate_CountsEveryResultOnce_AndTheCountersSumToRequested — all five statuses × 2; counters, sum, order.
• Aggregate_RefusesAStatusNoCounterAccountsFor — unknown status throws, names it.
• EveryServerResultConstruction_NamesItsStatusThroughAddStatus — source census: no bare status literal at any result site.
• AddServersDescription_NamesEveryCounter_TheSumRule_AndTheCollidesStatus; InstructionsTable_MatchesTheAuthModesTheParserAccepts (accepted set derived from ParseRequest; both denial spellings absent).
• ResolveForRemoval_* (6): exact storage/display, ambiguous fragment → both, unique partial, exact beats partial, shared display name ambiguous, no match; RemoveServerDescription_PromisesTheAmbiguousRefusal.
• ResolveOptionalText_OmittedKeeps_EmptyClears_TextReplaces (truth table); BothUpdateTools_RouteDescriptionThroughTheSharedVocabulary (cross-tool source census; helper declared once); BothUpdateTools_DescribeTheSameDescriptionVocabulary.
• MuteAnalysisFinding_Description_NamesRegistered_MatchedNow_AndTheUnmatchedStatus.
• WebRead_GetAlertHistory_AdvertisesAndDispatchesIncludeDismissed — catalog param (bool, optional, default false) + dispatch source pin.

New, Lite.Tests (DuckDB, runs in CI): McpMuteReportsWhatItMatchedTests — matched (2) → muted; never-seen → muted_unmatched, still registered; scope (server vs fleet, negative server id); blank hash invalid + no row; CountStoredFindingsAsync scoping incl. null and 0; description tokens.

Extended, live (DARLING_TEST_PG): server-admin round-trip adds the collision batch (requested 3 / added 1 / skipped 1 / collided 1 / failed 0, store agrees), ambiguous fragment refusal with both candidates and nothing deleted, unique partial removal, exact removal with matched_by; custom-view round-trip adds omitted-keeps (v3) and empty-clears (v4); custom-alert-rule round-trip adds omitted-keeps, empty-clears, stays-cleared; MCP-tools round-trip asserts matched_now: 2 on the planted chain, muted_unmatched on a never-seen hash (row present), and the truthful all-servers status.

Two existing assertions changed: the all-servers mute assertion (muted → muted_unmatched, because no stored row carries that hash — the old assertion was asserting the lie), and one version literal in the alert-rule round-trip shifted for the two inserted steps.

Which component(s) does this affect?

• Lite
• Darling
• Lite Tests
• Darling Tests
• SQL collection scripts
• Documentation
• Full Dashboard (deprecated)
• CLI Installer (deprecated)

How was this tested?

Compile-only on macOS (all four projects, zero warnings); pure-logic, source-text and live paths executed through throwaway harnesses against the built assemblies, a throwaway PG18/TimescaleDB container and a real DuckDB file (see Test plan). No SQL Server is touched by any of these tools' code paths beyond the stubbed probe seam. CI is the first execution of the test projects themselves.

Checklist

• I have read the contributing guide
• My code builds with zero warnings (dotnet build -c Debug)
• I have tested my changes against at least one SQL Server version (N/A — store-side write contracts; the probe is a stubbed seam)
• I have not introduced any hardcoded credentials or server names

[claude]

LGTM — reviewed the five write-contract fixes (add_servers counters, remove_server ambiguity handling, update_custom_view/update_custom_alert_rule description vocabulary, mute_analysis_finding disclosure, and the get_alert_history web rider) for correctness, Lite/Darling parity, security, and performance. All parameterized SQL, no secret leakage, counter/status maps are exhaustive and throw on drift, and the mute/description fixes are correctly mirrored across both SKUs where both SKUs have the surface (fleet-admin and custom-view/alert tools are intentionally Darling-only). No issues found.

[claude]

LGTM — reviewed the add_servers counter/collides mapping, remove_server exact/unique-partial resolution, mute_analysis_finding registered/matched_now disclosure, and the shared ResolveOptionalText vocabulary across both Lite and Darling. Logic is correct, edge cases (negative server_id, ambiguous names, omitted-vs-empty description) are handled, and the two apps stay in parity.