mute_analysis_finding stops writing the hash as the path and stops registering the same mute twice, remove_server can remove a server that never connected, and eight MCP descriptions stop saying what the code does not do (partial #3653 A15/A16)

[erikdarlingdata]

Two MCP writes stop acting on lies, and eight descriptions stop telling them

This is the A15/A16 "Description drift + envelope vocabulary" item of #3653, PARTIAL: the two behaviour bugs and the description lies land here; the truncation/severity vocabulary collapse and the four StampedUnderCollectionTime tools are deliberately NOT here (see "What this does NOT do"). #3653 stays open.

The item is one shape twice over. A write that stored something other than what its name says and reported success either way (mute_analysis_finding wrote the hash into the path column and registered a second row for a second identical mute; remove_server resolved the name against a table it does not delete from, so a server that had never connected could not be removed by the tool that added it). And a set of sentences a caller trusts that the code beneath them did not honour — an edition-aware audit with no edition branch, a "15-second ring buffer" that writes once a minute, a silent Take(10) whose ranking basis switched between a stopwatch and a guess, a bare impact beside paste-ready DDL, an AG badge that stays green over a lagging secondary with nothing in the description saying so, a "percentiles" catalogue line over a rates payload, and one tool whose rows came back in two different orders on the two SKUs.

The behaviour bugs

1. mute_analysis_finding — idempotent per (scope, hash), and the registry row names the chain. Both SKUs. The MCP entry point knows only the hash, and the pre-fix code wrote StoryPath = story_path_hash — a row that claims to name a diagnostic chain and names a checksum. And a second mute of the same hash in the same scope inserted a second row and reported registered: true again; the readers were never confused (they fold by hash, the viewer unmutes the smallest mute_id), but the caller was.

PgFindingStore.MuteStorySql / Lite FindingStore.MuteStorySql are now ONE guarded statement, byte-identical on PostgreSQL and DuckDB: INSERT … SELECT $1, $2, $3, COALESCE($4, (newest retained finding's story_path for the hash), $3), $5, $6 WHERE NOT EXISTS (same hash, same scope) RETURNING story_path. The path is the caller's when it holds one (the viewer does), else resolved from analysis_findings by hash (the hash is a function of the path — InferenceEngine.ComputeHash — so any row carrying it names the same chain), else the hash itself as a placeholder, because the column is NOT NULL and relaxing it is a rung this PR does not take; the result reports that case as StoryPath: null so the tool says "unknown" instead of echoing the checksum as a path. Scope is COALESCE(server_id, 0) = COALESCE($2, 0), so NULL and the legacy 0 all-servers rows fold together the way every reader already treats them. RETURNING distinguishes a written row from a pre-existing one without a second read.

The store method returns MuteWriteResult(MuteRegistration Registration, string? StoryPath) — Registered / AlreadyMuted / Failed (Darling's store swallows-and-logs; Lite's throws, so it never returns Failed). The tool payload gains already_muted, story_path, and a fourth status "already_muted" (registered false, nothing written, a different reason is NOT recorded — said in the note). MuteFindingAsync on both AnalysisServices returns the result; the Darling viewer's mute button compiles unchanged (it awaits and never read the bool).

Why a guarded INSERT and not a unique index or the write lock: no unique key exists over (scope, hash) and adding one is a rung; two callers muting the same pattern in the same statement's flight can still both land, and that duplicate is exactly the pre-fix state the readers tolerate. Lite's FindingStoreTests.TheWritePathsTakeAReadLockOnPurpose_AndSayWhy pin now states this in words (the NOT EXISTS is a courtesy to the caller's envelope, not an invariant the lock has to defend), so the guard is never mistaken for a reason to take the write lock.

2. remove_server — resolves against the definitions it deletes from. Darling-only tool. The connected-servers registry (servers) is written by the worker on a server's FIRST successful connection. A server that add_servers defined a minute ago with a wrong password, or on an unreachable host, had a definition and no registry row — and the tool, resolving against the registry, answered not_found for the very server its sibling had just added; the only undo was psql. It now loads config_monitored_servers (DefinitionsForRemovalSql: the #2218 identity columns, rebuilt into the storage name through the same ServerIdHelper.BuildStorageName the worker uses, plus an EXISTS against servers), applies #3615's exact-then-unique-partial rule unchanged (ResolveForRemoval and its six pins are untouched), and DELETEs by the definition's own server_id. Payload: matched_in: "config_monitored_servers", ever_connected (per candidate on ambiguous, on the row on removed), display_name, and a note saying whether history exists under the id. A name the registry resolves but no definition does — the darling.json-defined shape — returns not_found with matched_in: "servers" and a message that names the real reason, instead of the old "registered but has no definition" guess. No is_enabled filter: a disabled definition is still one an operator may remove.

The descriptions — before → after, one line each

• audit_config (both SKUs): "accounting for edition (Standard vs Enterprise)" → the edition is reported for context and no check branches on it (MAXDOP is topology-based, the rest resource-based). Darling's adds: SQL Server only; a PostgreSQL target is refused with not_collected and pointed at get_pg_server_config, get_pg_logging_audit (new next_tools row) and the CONFIG_PG_* facts. Instruction rows on both SKUs drop "Edition-aware".
• get_cpu_utilization note (Lite): "1-minute averages of 15-second ring buffer samples" → one RING_BUFFER_SCHEDULER_MONITOR record per minute on-prem / MI / RDS, one sys.dm_db_resource_stats row per 15 s on Azure SQL DB, samples_in_bucket is the measured count. The Darling twin line (DarlingMcpDataTools.cs:94) is byte-identical and NOT changed here — that file belongs to the wwwroot lane tonight; it is a one-line port (see report).
• analyze_*_plan (shared McpPlanAnalysisFormatter, so both SKUs and the frozen Dashboard twin by construction): the silent Take(10) → per statement operators_cap: 10, operators_returned, total_operators, truncated (observed against the whole population, never inferred), operators_ranked_by: "actual_elapsed_ms" | "cost_percent (optimizer estimate)". missing_indexes[]: bare impact → impact + impact_basis (the showplan MissingIndexGroup/@Impact: estimated, statement-scoped, not additive); create_statement removed and replaced by the three column lists (already there) plus a note that the optimizer's suggestion is a hint, not a design. The plan viewer and the alert remediation text still render the DMV's DDL for a human reading one plan; the MCP surface is the one an agent may act on unread. Descriptions of the four plan tools on both SKUs and both instruction tables say the new shape; Lite's "Missing indexes with CREATE statements" line is gone.
• get_ag_health (Darling; Lite has no twin): "Severities are computed server-side" → …and restate the DMVs' OWN verdicts only: lag and queue depth are NOT banded (a lagging async secondary under HEALTHY stays healthy — read the numbers), lag reads 0 while suspended (read it with is_suspended), and each group's collection_time is that server's newest AG collection — a server whose AGs were dropped keeps returning its last non-empty snapshot, so an old collection_time is that case. Same two sentences added to the instruction paragraph.
• DarlingWebEndpoints.cs catalogue: get_query_duration_trend "Query-duration percentiles over time." → "Query elapsed ms/sec + executions/sec over time (per-interval rates, not percentiles)." (the payload has been rates since The duration-trend trio routes by retention tier and discloses its source like get_query_trend already does, so a 7-day request no longer returns 4 days labelled quiet (#3541 A2) #3590/The MCP trend reader's tier ladder and hourly SQL were pinned EQUAL to the Storage definitions #3666 moved them to, which fails only after two copies drift — they are now aliases; and the Query Store duration chart, the one Performance Trends chart left untitled, says what served it and where the rollup's floor cut the window's head (#3653) #3684; Lite has no web catalogue).
• get_resource_semaphore (Darling reader): ORDER BY resource_semaphore_id, pool_id on the latest-snapshot AND window reads → pool_id, resource_semaphore_id, Lite's order and the order the pool-lens reads in both SKUs already use. Both SKUs always took the same newest snapshot (the MAX(collection_time) probe never drifted); only the row order did.
• mute_analysis_finding / remove_server descriptions and instruction rows carry the new keys and statuses above.

Claims in the item found already true or over-stated

• Darling's audit_config already refused a PostgreSQL target with not_collected (landed in the PostgreSQL-target analysis engine: design and v1 build #3542 lane); this PR adds the get_pg_logging_audit pointer and makes the DESCRIPTION say so. no_config_data remains the SQL Server no-facts answer.
• The top_operators basis switch existed; the cut did not publish itself. The missing_indexes DDL and unlabeled impact were as described.
• get_resource_semaphore: both SKUs took the latest snapshot the same way; the drift was ORDER BY alone (two statements, not one).

What this does NOT do

• The 7 truncation dialects / 5 severity vocabularies collapse — every tool file; three lanes are in those files tonight and the payload-census lane owns the roster it would collapse against.
• The four StampedUnderCollectionTime tools (get_database_sizes, get_running_jobs, get_server_properties, get_session_stats) — the wwwroot lane owns them with server-tabs.js.
• DarlingMcpDataTools.cs:94 (Darling's "15-second ring buffer" twin) — same lane's file; one-line port pending.
• No rung, no unique index on analysis_muted, no change to how the viewer or the alert remediation render missing-index DDL, no change to ResolveForRemoval or the read resolver.

Blast radius

• mute_analysis_finding payload: +already_muted, +story_path, status gains "already_muted"; registered is now FALSE for a repeat mute (it used to be true). PgFindingStore.MuteStoryAsync → Task<MuteWriteResult> (was Task<bool>), storyPath nullable; Lite FindingStore.MuteStoryAsync → Task<MuteWriteResult> (was Task). DarlingAnalysisService.MuteFindingAsync / Lite AnalysisService.MuteFindingAsync return the result. All existing await callers compile unchanged.
• remove_server payload: +matched_in, +ever_connected, +display_name, +note; ambiguous.candidates[] gains ever_connected; a not_found may carry matched_in: "servers" + candidates. A never-connected definition is now removable (the point).
• Plan envelope (both SKUs + Dashboard): −create_statement, +impact_basis, +note per missing index; +operators_ranked_by/operators_cap/operators_returned/total_operators/truncated per statement.
• get_resource_semaphore on Darling: same rows, pool-first order.

Tests

Every new pin was EXECUTED here before commit — Lite in-process on DuckDB 1.5.5 (a throwaway net10.0-windows harness Compile-Including the classes, WindowsDesktop stripped from the runtimeconfig), Darling through a Darling.Tests-named net10.0 harness against a throwaway timescale/timescaledb:2.28.1-pg18 migrated to ladder 131 with DARLING_TEST_PG set. The one thing NOT executed here is the remove_server half of AddServers_…_ThenRemoveServer_AgainstDevPostgres, because its add half needs DPAPI; the same six shapes were executed through a harness-only rig test (not committed) and CI is the first execution of the committed form.

New / extended, both compile-verified (0 warnings) and first executed in CI:

• Lite.Tests McpMuteReportsWhatItMatchedTests (+4): same hash twice → already_muted, one row, first reason kept; idempotence per scope incl. a planted legacy server_id = 0 row counting as global; path rule (caller's → retained finding's → hash placeholder reported as unknown); description tokens.
• Lite.Tests FindingStoreTests.TheWritePathsTakeAReadLockOnPurpose_AndSayWhy: anchors moved to the new signature / ExecuteScalarAsync; pins the "not an invariant the lock has to defend" sentence at the mute site.
• Lite.Tests McpDescriptionTruthPinTests (new, 3): audit_config no edition claim + no edition branch in the body + no "Edition-aware" in instructions; get_cpu_utilization note names both cadences; plan-tool descriptions name the cut and "no CREATE INDEX text".
• Darling.Tests DarlingMcpToolsTests: live mute round-trip asserts story_path = the planted chain on the wire and in the row, the second mute → already_muted with count 1, the unmatched hash → story_path: null with the hash placeholder in the row; +AuditConfig_Description_DoesNotClaimEditionAwareness_AndNamesThePostgresRefusal; the mute description pin gains the new tokens.
• Darling.Tests DarlingMcpServerAdminToolsTests: DefinitionsForRemovalSql_… dialect pin; the description pin gains the definitions/ever_connected/matched_in tokens; the live round-trip now registers ONLY the SQL host in servers and removes the never-connected Windows host and the never-connected host:AppDb definition, asserts three ambiguous candidates with ever_connected per row, and the matched_in: "servers" not_found on the darling.json shape.
• Darling.Tests McpPlanAnalysisEnvelopeTests (new, 3): synthetic 12-RelOp estimated plan → operators_returned 10 / total_operators 13 / truncated true / ranked_by cost_percent, rows actually in that order; 3-RelOp plan → not truncated; missing index → labelled impact, three column lists, note, no create_statement, no DDL text anywhere in the row.
• Darling.Tests DarlingMcpMemoryGrantToolsTests.TheSameToolName_OrdersTheSemaphoreRowsTheSameWay_OnBothSkus: both Darling consts and Lite's source hold one ORDER BY (pool first), with the drifted spelling excluded by a regex proven against literals (the DISTINCT ON subquery legitimately leads with the semaphore on both SKUs and is excluded by lookahead).

Rosters checked: AsOfWindowAnchorTests (no DateTime.UtcNow added to any tool body), RepoFileAdoptionTests.s_lfReaders (the new readers are raw, single-line anchors), FindingStoreTests lock-site count unchanged at 7 (the path resolution lives inside the one statement), PgTargetMcpSurfaceTests.AuditConfig_AnswersNotCollectedForAPostgresTarget_BeforeItReadsAnyFact (all tokens still present).

Partial #3653 — no Closes.

Which component(s) does this affect?

• Lite
• Darling
• Lite Tests
• Darling Tests
• SQL collection scripts
• Documentation
• Full Dashboard (deprecated) — inherits the shared plan-envelope change by construction; no Dashboard file edited
• CLI Installer (deprecated)

How was this tested?

Compile-only on macOS for the net10.0-windows test projects (0 warnings on Darling.Tests; Lite.Tests carries dev's one pre-existing xUnit2000 warning at LogTailOverlapThresholdPinTests.cs:95); Lite pins executed in-process on DuckDB (32/32), Darling pins + the live mute round-trip executed against a throwaway PG18/TimescaleDB rig (80/81 — the one red is the DPAPI-only add half of the admin live test, a macOS harness artifact). Rig and harnesses torn down. No SQL Server touched.

Checklist

• I have read the contributing guide
• My code builds with zero warnings (dotnet build -c Release)
• I have tested my changes against at least one SQL Server version (N/A — store-side writes and payload descriptions; no monitored-server path changed)
• I have not introduced any hardcoded credentials or server names