Release v3.8.0 - #3519
Merged
Merged
Release v3.8.0#3519
Conversation
The 6 custom-alert MCP CRUD tools (create/get/list/update/delete/validate_custom_alert_rule)
tripped three cross-project inventory pins that live outside the tool's own files:
- Lite.Tests CrossAppMcpToolInventoryPinTests: bump the DarlingMcpInstructions census
(139->145 total, 53->59 unique-to-Darling; shared stays 86) and add the 6 tools to the
KnownLiteMissingMcpTools ratchet (Darling-only by architecture, like the Custom Views +
alert-tuning write tools).
- Darling.Tests DarlingWebEndpointsTests.ReadEndpoints: add the 6 tools to
DarlingWebEndpoints.ExcludedToolNames (no /api/read/{tool} 1:1 mirror, same disposition as
the Custom Views tools) and to the pinned exclusion array in ExcludedToolNames_AreTheNonReadSurfaceTools.
These are the pins documented to fail only under a full test run; verified locally with the
exact classes: CrossAppMcpToolInventoryPinTests 4/4 and DarlingWebEndpointsTests 32/32 green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WejwpgWF5Xfm3fAmoEbFz4
…-measures Add PostgreSQL measures to the compose catalog (#3285)
…-alert-tools Add MCP CRUD tools for custom-alert rules (#3285)
An alert's DetailText reached the history store, the Viewer, the MCP reader and the triage endpoint, and no delivery channel. The email template's detail section was gated on AlertContext.Details -- a different, structured field -- and a self-alert fires with context: null, so an email-only operator received a metric name, a value, a threshold and two timestamps with the remedy discarded. Reported on #3296. Email (both bodies), Teams, Slack, the generic channel and PagerDuty now carry it. The prose is resolved once per firing and suppressed when it is only a flattening of the structured context, so engine alerts render unchanged. Also: the #2813 Retention Held detail now names the service restart, which is the step that actually arms a held policy.
Mutation testing found the one uncovered hop: dropping the detail on the way to the email template alone passed every pin, because email has no interception point short of the protocol. That is the exact defect #3296 reported, so the loopback sink now speaks enough SMTP to capture the DATA and decode both transfer encodings, and the assertion is that the phrase appears TWICE -- once per MIME part.
Review raised that the prose detail bypasses the redaction that replaces copy-paste remediation T-SQL with a hint on every webhook channel. The rule's subject is the GENERATED payload -- IsCodeBlock bodies and RemediationAction, built by FactRemediation, multi-statement and in two shapes destructive. Prose is authored per alert, and where it names a command at all that command is the recovery action and the sentence is the alert; redacting it delivers "see email or the in-app dialog" to an operator whose only channel is the one being redacted. What made that a habit rather than a fact is that nothing stopped a generated command from being folded into the prose. FindingMessageFormatter.DetailText is finding metadata and the command rides only in the code-block item, so that is now asserted instead of assumed, and both docs say which surface the rule covers.
…w token Review found two fire sites that hand-author a detail text restating their own structured fields under different labels. The suppression gate compares the prose against the context's flattening, so it never fired for them and both alerts would now render the same facts twice on every channel. Per #2109 the structured fields are the canonical copy, so the restatement goes and the detail text is their flattening, as at the other eleven sites. The one thing each prose said that its fields did not -- no baseline yet, and that the query is running on the optimizer's plan -- is a field. The generic channel's new token was also in neither app's Settings help text, so nothing told an operator it exists. Both lists carry it now, and Lite's automation list picks up the triage_url it never got. The token set is one list the matcher is built from, and the help text is checked against that list rather than against a second copy of it: this is the second drift of the same kind.
Custom alert rules key history/mute/cooldown on the immutable metric_name "Custom:<rule_id>" (rename-safe), but the delivery layer rendered metric_name directly, so a human saw "Custom:42" instead of the rule's name in email subjects and Teams/Slack/PagerDuty titles. - Add an optional AlertOutcome.DisplayName. Built-in alerts leave it null and render exactly as before (byte-identical); the existing pinned webhook/email render suites stay green. - Thread it through the render sites (EmailSendCore subject + email body, WebhookAlertService Teams/Slack/PagerDuty titles/summaries), each falling back to metric_name when it is null/empty. ForMetric (severity), the email/webhook cooldown key, and the PagerDuty dedup_key all keep keying on metric_name. The generic webhook's "metric" field stays the immutable key (automation correlates on it). - CustomAlertEvaluator sets DisplayName to the rule name. - Persist via detail_text (no schema change): the rule name already rides detail_text, which get_alert_history and the viewer already return, so history/MCP/viewer resolve the name after the fact and it survives rename/delete. - Security guard: SanitizeDisplayText strips every line break/control char and length-caps the name+description before they enter detail_text, so a crafted name cannot forge a "Database:"/"Query:" label line that MuteRule.PopulateFromDetailText would parse into the viewer's mute-from-history pre-fill. Applied on both the fire and the resolve paths. New tests cover the newline-bearing spoof end to end. - Swap the em-dash separator in the fire detail_text for a plain " - ". Part of #3285. Addresses #3303. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WejwpgWF5Xfm3fAmoEbFz4
…rt-display-name Show the rule name in custom-alert notification titles (#3303)
…a.object A deadlock or blocked-process report whose statement sits inside a stored procedure showed SQL Server's own unresolved placeholder where the procedure name belongs. Nothing parsed it, so this was a resolution never attempted. Both surfaces shred <inputbuf> client-side, so the parse lives in C# and only the id-to-name lookup goes to the target, batched once per cycle through the definitions' supplemental-query seam. A pair the target cannot name is absent from the map and its raw placeholder survives.
…splay name #3308 threaded a custom rule's display name through the same five-channel seam this branch threads the alert's prose detail through, so every signature that gained a parameter from one side gained one from the other. Both are kept: detailText first (the positional form AlertIncidentRenderTests uses), displayName second. The generic channel takes the prose but not the display name — its payload has no title, and {{metric}} is the machine key an automation correlates on.
…s objects An incident's Involved Objects field is three-part, and it is built from the deadlock graph's own keylock/@objectname, which the engine writes three-part. The graph writes a procedure three-part too, in frame/@procname. A two-part Victim SQL would be the odd field out in its own message, which is what qualifying it was for. The database name is projected as its own column and required with the other two, so a partial answer still falls through to the raw placeholder.
Both said 'half' or 'schema-qualified' about a resolution that is now three parts.
The two fields the conflict was in are both string?, so losing one or transposing the
pair at any of the three hops between the deliverer and the payload builders is not a
compile error — and measured against HEAD, it is not a test failure either. Dropping
displayName in DarlingAlertDeliverer, and again in EmailSendCore's webhook fan-out,
both left every suite green: each side's tests cover its payload BUILDERS, and neither
could cover the hops with the other side's field also present.
This drives the whole Darling path once with both set and reads the bytes that left
the process. Bodies are identified by a channel-native marker rather than by arrival
order, and the generic channel's deliberate exemption — prose yes, display name no,
because {{metric}} is a machine key and that channel renders no title — is asserted
here instead of assumed.
The parse accepted anything whose prefix matched, while Resolve replaces the whole input — so trailing content would have been discarded rather than declined, and the doc comment's 'full shape' contract was not what the code enforced. The closing bracket and a padding-only tail are now part of the matched shape; a value truncated before the bracket still parses, because the ids are the thing being read. The padding predicate is used on both ends now, and named for that.
Expected State already renders "(no baseline yet)" whenever the baseline is missing — it has since #2109, and expectedText is computed from the same `pending` flag the extra Baseline field was conditioned on. So the field said the same thing under a second label, and shipped it to every channel now that the detail block is delivered. The three canonical fields stand alone again, and the pin counts occurrences rather than enumerating field names: a name list would pass while the fact arrived under a label nobody thought to enumerate, which is the failure being guarded.
A stored custom alert rule silently stops firing when its measure drifts out of the catalog (the incremental pg_* buildout) or when it compiles but can never fire (scope resolves to no monitored server, or its measure is always-NULL). Nobody "opens" an alert rule, so these gaps were invisible: an operator believed a rule was armed that never fires. - CustomAlertEvaluator re-parses every enabled rule against the LIVE catalog on each load/refresh (ClassifyRules) and collects the ones that no longer compile instead of only logging + skipping them. It also tracks a per-rule no-data-since timestamp (in memory) and exposes BuildHealthReportAsync, which flags "armed but never fires": a servers-scoped rule with no monitored match, or a rule with no data for longer than NoDataFlagWindow (20 min). Reuses SanitizeDisplayText from slice 1 for every rule name / error, so the report cannot re-introduce the mute-pre-fill spoof. - DarlingSelfAlertEvaluator gains a new fleet-global condition (EvaluateCustomRuleHealthAsync / ApplyCustomRuleHealthAsync) that raises ONE aggregated self-health alert for ALL unhealthy rules (a pg_* rename can break many at once), edge-triggered under the "Monitor Store" fleet label with a stable "Custom Alert Rules Unhealthy" metric and a "Custom Alert Rules Recovered" resolution. The detail lists rule id/name + reason (capped, "+N more"), NEVER the compiled SQL. - DarlingWorker runs the check once per fleet sweep (own 5-min cadence), above the connect gate. A rule-load failure returns null so the standing alert is HELD, never resolved on a transient store blip. - AlertMetricClassifier renders the count metric as a whole number; the resolution is state-only via IsResolution. Design decisions: the self-alert is fleet-global (mirrors Store Disk Pressure's once-per-sweep edge under StoreServerLabel), NOT per-server, so one rename yields one alert. N is a 20-minute DURATION rather than a raw sweep count, so it is independent of fleet size and per-rule cadence (a count would trip a large all-servers rule in a single sweep and flag a just-created rule before its first window filled). The no-data timestamp is deliberately in-memory (a restart restarts the window, no false alarm, no migration). The new self-alert wrapper's catch is added to the #3013 census as exempt (evidence handed in as a parameter; no store read here). Part of #3285. Addresses #3304. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WejwpgWF5Xfm3fAmoEbFz4
…rt-self-health Surface broken / never-firing custom alert rules as self-health (#3304)
Disabling or deleting a custom alert rule with an OPEN incident left it
open forever and orphaned its config.custom_alert_state row: delete
cascades the state away (ON DELETE CASCADE) without writing a recovery
row, and disable is skipped by the evaluator's enabled-only sweep so the
firing state just sits there.
- DELETE path: CustomAlertEvaluator.ResolveAndDeleteRuleAsync reads the
rule's firing subjects and writes ONE recovery row for each BEFORE
calling the store's delete, because that delete's FK cascade drops the
state that says which (rule, server) pairs were firing. The MCP
delete_custom_alert_rule tool routes through it (and its description no
longer claims an open incident is torn down without a resolve). Static,
so it works from the MCP surface that holds only the owner pool.
- DISABLE + out-of-scope: CustomAlertEvaluator.ReconcileStateAsync, run on
the fleet-global health cadence, force-resolves and cleans the state of
disabled rules and of servers that have left a rule's scope (or left
monitoring entirely). The pure ClassifyStateTeardown decides per row;
DarlingWorker builds the current fleet map and calls it after the health
check.
- CustomAlertStateStore gains ListFiringWithNamesAsync (delete path) and
ListAllWithRuleAsync (reconcile) + a CustomAlertStateRow record.
- WriteTeardownResolutionAsync is the shared resolve idiom (mirrors
DeliverResolveAsync: BuildResolutionRecord + AlertFiringLog.Resolved),
reusing SanitizeDisplayText so a crafted rule name cannot re-open the
mute-from-history spoof on the recovery row.
- Also corrects a stale slice-2 XML comment: EvaluateCustomAlertRuleHealth
says BuildHealthReportAsync returns null on a load error (the worker then
holds the standing alert), not "an empty report".
Delete/resolve coordination: resolve-BEFORE-delete, matching the store's
own DeleteAsync contract ("the resolve delivery must happen first"). The
recovery write and the delete are separate statements (the history write
opens its own connection), not one transaction: the resolve is best-effort
audit, failure-isolated, and a delete-after-resolve failure only leaves a
premature recovery row that the incident's normal resolve later supersedes
- no orphaned incident, no worse than today's behavior on any partial
failure. The evaluator's in-memory no-data map is cleaned lazily by the
slice-2 cache-refresh prune; the delivery-layer cooldown is keyed by the
now-gone metric_name and simply never recurs.
Part of #3285. Addresses #3305.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WejwpgWF5Xfm3fAmoEbFz4
…very-channels Deliver the alert's detail text on every channel
…rt-resolve-on-delete Force-resolve + reconcile custom-alert state on disable/delete (#3305)
FindingMessageFormatter is a third producer of (Context, DetailText) pairs. Its DetailText and its context's Diagnosis item render the same values under different labels, so the textual equality in AlertDetailText.ProseForDelivery cannot see the duplication and every channel prints those facts twice. The prose is suppressed at DELIVERY only. FindingAlert now carries the producer's own declaration, and the two senders route DeliveredProse to the channels while the alert-history row keeps DetailText in full - config_alert_log.detail_text is what the MCP reader, the triage endpoint and the Viewer's detail pane render, and the input AlertMuteContext.PopulateFromDetailText parses for the mute pre-fill.
… the row Occurrence counts rather than absences. Asserting only that the prose is gone would pass equally if the structured context had been dropped instead, which is the opposite defect and the more expensive one - the context carries the advice, the remediation T-SQL and the drill-down that the prose does not. Each count is paired with the same builder fed DetailText, the argument #3297 shipped, which must come back with the facts twice. Both directions, or the fix is indistinguishable from reverting #3297 for this seam: a producer that declares its prose independent still has it delivered, and the persisted detail text is a frozen literal on both SKUs. Driven through the real AnalysisNotificationService, not a hand-built FindingAlert - the declaration under test is the producer's.
… number #3313 is an open issue about webhook batch re-rendering. These comments were referencing a number that was never read, only constructed.
…laceholder Resolve the Proc [Database Id = N Object Id = M] placeholder to schema.object (#3307)
… baseline, and the tempdb rule the evidence demanded (#3475) The review on #3469 asked the question the merge had defaulted: the same-statement pileup detector grouped concurrent sessions by query_hash alone, and query_hash is not database-scoped - on a fleet of identical tenant schemas, textually-identical SQL in different databases shares a hash by construction - while a compiled-plan cache entry IS keyed per database, so the causal story the finding tells (one shared parameterized plan flipped, every caller of that one plan convoyed) is only strictly true within one. #3474 pulled the deciding evidence from the store's own snapshots before any code moved, and the evidence chose option A: - The measured incident's three convoy sessions all ran in ONE tenant database (the "ten tenants" are application-level tenants inside it, sharing exactly one plan), so per-database scoping preserves the motivating incident intact - its group, its 2.08 severity, and its notify margin are unchanged. - The conflation the review predicted was measured, not hypothetical: within the same hour, the same statement text ran 9.7 s / 7.4M reads in a DIFFERENT tenant database on the same server - one snapshot's timing away from joining the incident's pack under the server-wide key. - The DMV sometimes attributes a copy's database context to tempdb (a 16.5 s copy of the incident statement, the day before - the statement joins a #temp table and the request's context followed it), a shape neither option in the issue had named. The change, in the detector only (both SKUs' readers already project database_name and are untouched): - The group key is (DatabaseScope, StatementIdentity). Tempdb-attributed rows form their own (tempdb, statement) group - honest about what the DMV said, with reattribution deliberately not attempted: the snapshot carries no reliable session-to-database mapping at read time, and a guessed attribution would poison exactly the per-database baseline this change exists to keep clean. - The "normally sub-second" baseline reads only the same database's history, closing both pollution directions: another database's fast history cannot vouch for a statement that is new-and-slow where it actually runs, and another database's unrelated slow occurrence cannot suppress a real pileup - the direction that fails quiet. - The evidence pack's database is the group key's database, structurally, retiring the first-non-empty pick over the pack that was nondeterministic for multi-database packs - ComputeIncidentId folds on databaseName, so the pick's wobble destabilized the occurrence folding the detector was built around and the stable token #2138 wants. - The calibration carries over without a re-pull, by argument written into the derivation's doc comment: per-database scoping PARTITIONS the server-wide groups and can never grow one, so every gate is strictly harder to pass and the four-instants-in-four-server-days derivation stays conservative under the new predicate. Tests pin the kill-shot (three same-hash sessions in three databases are three groups, not a pileup), both baseline-isolation directions, the tempdb rules (own group, own history, no dilution of the tenant pack's arithmetic), fold determinism under row-order permutation, and the null-database fold to one scope. Closes #3474.
…ed degrade, and the deterministic name (#3476) * Carve the deliberate INFO reports out of IsWarning, so the history grids agree with the severity map (#3448 review) * Name the census window the digest actually reads: the trailing 24 hours, not the movers' baseline (#3448 review) * Hand get_sweep_reports' child reads the host's logger, so a degraded section leaves a trace (#3473 review) * Order the rollup's names read so a renamed server's newest name wins deterministically (#3473 review)
… already is (#3479) (#3480) The reporter sets the toolbar's Auto-refresh to 5m, restarts, and gets 1m back, every time. Root cause is that persistence never existed: the XAML hardcodes the combo at index 1 and the checkbox checked, and the Changed handlers moved only the in-memory DispatcherTimer. The pair was a control that offers a choice and discards it - the exact complaint #2640 fixed one control to the left, where the time range picker was write-only until PersistSelectedTimeRange taught it to remember. This is that fix, applied to its neighbors, through the same machinery: - One preference across all server tabs, not per-tab rows: like App.DefaultTimeRangeHours, the App-level statics are set FIRST so a second tab opened this session constructs on the values just chosen, and every tab at the next launch restores them. The reporter's mental model is "the app's refresh setting"; a toggle that meant something different per tab would surprise. - Save-on-change through App.WriteSetting, both keys in one write - they change from the same toolbar gesture, and two writes would be two chances for the file to hold half a preference. Failure is logged and never interrupts the refresh, same as the time range. - Stored as auto_refresh_interval_seconds, not a combo index, for the reason default_time_range_hours stores hours: an index is a fact about today's XAML. A stored value the combo does not offer restores the XAML default rather than a blank picker over a running timer. - The keys load in LoadDefaultTimeRange - the same class of setting, and the loader the SettingsSampleTests extraction already scopes - and are documented in settings.sample.json, which those tests enforce both ways. The restore rides the guard the handlers already had: _refreshTimer is built AFTER the constructor sets the restored values, so the Changed events the restore fires no-op on the null check - no save-echo of the value just loaded, no poking a timer that does not exist. The timer's interval then comes off the restored combo through the same UpdateAutoRefreshInterval a live change uses, and Start is gated on the restored checkbox, which until now could only ever construct checked. What used to be one private switch with a silent default arm is now the AutoRefreshSecondsForIndex/AutoRefreshIndexForSeconds pair - the one place the combo's items are given meaning, feeding the timer, the persisted value, and the restore alike. AutoRefreshMappingTests pins that pair to the source XAML: every item's label maps to its own seconds and round-trips to its own index (an interval that cannot round-trip resets on every launch, which is this issue), unoffered values restore the XAML default, and the App statics' initializers agree with the XAML attributes they replace on a fresh install. Closes #3479.
…hen it ran, and the render strips the legacy fabrication (#3478) (#3481) Two would_have_paged emissions existed and only one was gated: the presentation's top level honored the muted-mode contract while the engine's document composition wrote the key unconditionally, so every alerts-on sweep persisted would_have_paged: [] — a fabricated check, not an empty result — and both surfaces served it inside the embedded report. Caught live smoke-testing a nightly on a dogfood host. Fixed at both ends, each for its own reason (the author ruling on the issue). At storage: the document is now composed as an ordered member list (an anonymous type cannot omit a member per instance) and carries the ledger member only when the derivation ran — the muted document is byte-identical to the old shape, verified by diffing the engine's real output before and after, because stored documents are immutable and readers pin the muted shape. At presentation: BuildRunNode strips the key from an alerts-on sweep's embedded report — on the freshly parsed embed, never the stored row — because the store already holds the fabricated key on every alerts-on sweep since the feature landed, those documents live out their retention as-written, and the strip doubles as the belt against any future writer regression. The unparseable-payload arm rides untouched behind the type guard. Pinned: the engine's three-state document contract (absent on alerts-on, present-and-empty under mute, populated under mute with rows); the legacy-doc strip and the muted preserve at the run node, including the sweep_id fetch path's detail wrap; the MCP tool's shared-builder join so lane 3's shape pins provably cover both of its read paths; and the daily rollup's indifference — its ledger figures ride the store's rows, so all three document vintages extract identical facts.
…tory (#3482) (#3483) The Fleet Sweeps worklist (and get_sweep_reports' watch_items) rendered raw server_ids while every other table on the page showed display names: the shared builder never emitted the server field sweeps.js has preferred since lane 3, so the client's "server N" fallback was the only rendering. BuildWatchItemsNode now takes a names map and emits ["server"] exactly when it means something - omitted on the fleet-scope sentinel (a name for the fleet would be a fabrication; the fleet_scope flag is what a client renders from) and omitted when the id resolves to no retained verdict row, where the bare-id fallback is the honest degrade. The names come from a new whole-history store read, deliberately unwindowed: the worklist is current-STATE rows, and a carried item can outlive its server's presence in the fleet, so its name may exist only in sweeps older than any span a caller could honestly pick - and the population is bounded by the same base horizon that prunes the worklist itself. The statement carries the span sibling's #3476 dedup-and-determinism contract (newest sighting ordered last, last-write-wins reader), and the read is log-and-degrade: a failed lookup costs the names, never the worklist. Both surfaces share one gate-and-read helper (the ValidateWatchState sharing pattern), skipped when the worklist names nothing - the common case on a healthy fleet under the page's 60-second refresh.
…ity) for headless targets (#3484) The headless collector connected with Windows or SQL auth only; every Entra mode was refused on the "interactive MFA is nonsensical headless" reasoning, which swept in the two NON-interactive modes (Service Principal, Managed Identity) that fit a 24/7 service exactly. - Service principal reuses the existing credential shape: client id in username, client secret in encryptedPassword (DPAPI, like a SQL password) - no new config field, no store column, no migration. - Managed identity carries no secret (system- or user-assigned client id). - Connection-string mapping mirrors Lite's ApplyAuthentication. - Both onboarding paths honored: darling.json and the add_servers MCP tool. - Interactive Entra modes (MFA/device-code/default-credential) stay refused, now with a message that names the two supported modes. Tests: connection-string mapping (SP/MI Authentication mode + creds), config validation accept/refuse matrix, add_servers parse accept/reject. Viewer add-server dialog UI is a follow-up (#3485). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DRsdSKC8p1qCQ3q4hcMsM7
…ntra-service-principal # Conflicts: # CHANGELOG.md
…tra-service-principal Non-interactive Entra auth (Service Principal + Managed Identity) for headless targets (#3484)
… and the embedded report (#3487) (#3488) Every fleet-sweep payload emitted sweep_id and previous_sweep_id as JSON numbers, and a sweep id is ~70x past Number.MAX_SAFE_INTEGER - so every JS consumer's JSON.parse rounded it (~128-tick granularity at this magnitude, measured live: 639251940075451830 -> 639251940075451800), the Fleet Sweeps page fetched the rounded neighbor, and the detail pane answered 'never recorded' about a sweep the timeline was displaying. The input side (get_sweep_reports' string sweep_id, exact-parse-or-refuse) was designed against exactly this double; this closes the output side on the shared builders both surfaces ride: the run node's two header ids, the watch items' four *_sweep_id anchors, and the embedded stored report's two id fields - the last converted on the freshly parsed embed at render, the #3478 arm-B shape, never the stored row. Stored documents keep their numeric spelling by recorded decision at the engine's composition site (longs are exact .NET-side; the lie only happened at the JS boundary, and a mid-stream spelling change would make vintage sensitivity permanent). A first sweep's null previous_sweep_id stays null - an absence is not a spelling. sweeps.js needs no functional change (verified and stated at the fetch site): the concat, the strict active comparison, and the == null check all ride a string unchanged, and nothing on the page does arithmetic on an id. Co-authored-by: Erik Darling <erik@erikdarling.com>
…CP path's already do (#3490) The web dashboard host clears its app's logging providers on purpose — ASP.NET framework and per-request noise has no seat in the service log — but the fleet sweep routes' log-and-degrade store reads were logging through app.Logger, the cleared factory's logger, so every web-path degradation line wrote nowhere. A failed names read, a failed verdicts read, a failed ledger read: each degraded exactly as designed on the wire and left no trace anywhere an operator looks. Same gap class as the MCP path's, one host over: #3476 closed it there by registering the HOST SERVICE's logger (AddSingleton<ILogger>(_logger)), because the host's logger is the one wired to real providers. This is the web host's copy of that fix, at the wiring seam instead of DI because this host maps routes directly: DarlingWebHostService hands _logger to DarlingWebEndpoints.MapAll, MapAll threads it to DarlingFleetSweepEndpoints.Map, and every seat that read app.Logger — the timeline, both detail fetches, the worklist, and BuildDetailAsync's child reads — now takes the service logger by parameter. BuildDetailAsync stops taking the WebApplication entirely; app.Logger was the only thing it wanted from it. ClearProviders stays, and its comment now states the whole design in one place: framework noise deliberately silenced, app-level degradation lines deliberately routed to the service log via the endpoint seats. The two halves were always one decision; before this, only the silencing half was written down, and the routing half was a gap nobody had chosen. The /api/read mirror's get_sweep_reports entry comes along without widening the shared delegate: BuildReadDispatch grows an optional logger the one logging tool's entry captures by closure — ReadToolHandler stays three-seat, because a fourth parameter would touch every entry in the table for the one tool that logs. MapAll builds the dispatch with the host's logger, so the mirror traces into the same service log as both hosts' other paths; a dispatch built bare (the parity tests, the triage runner — neither maps that entry) keeps the pre-existing null, which is honest there: no service log is wired. Pinned both ways in the FleetSweep suites' source-scan idiom: the sweep wiring takes the ILogger seat and the endpoint file's CODE (comments and literals stripped, the CSharpSourceWalker separation) contains no app.Logger; the host clears providers AND hands _logger to MapAll beside them; the mirror entry passes the captured logger, never a hardcoded null, over an unwidened delegate.
…sus (#3489) * Correct the tempdb mechanics prose: the context never followed the temp object, and the plan-cache keying makes the own-bucket rule stronger, not apologetic The #3474 tempdb-wrinkle story shipped with the mechanics backwards: dm_exec_requests.database_id is the SESSION's execution context, and a query joining a #temp table does not move it. The measured row read tempdb because the session's context genuinely WAS tempdb, with the statement reaching the tenant tables by three-part names. The wrong telling made the (tempdb, statement) own-bucket rule sound like damage control - honesty about a DMV quirk - when the true mechanics make it correct plan-cache semantics: compiled-plan cache entries are keyed by the context database's dbid, so tempdb-context copies share a tempdb-keyed plan among themselves, the same one-shared-plan causal story every other group tells. A reattributed row would not just poison the tenant database's baseline; it would blame a plan that is not the one convoying. Behavior unchanged - three prose sites corrected: the DatabaseScope doc, the CHANGELOG entry, and the tempdb test's summary (the #3467 entry's #temp claim is about the plan flip and stands). * Hold the INFO carve-out lockstep by from-source census, so the claim beside IsInformational is finally true AlertMetricClassifier.IsInformational's doc promised that 'the lockstep test fails when a third deliberate INFO metric is declared there and not here' - and the test was an InlineData theory over the two known names, so a third INFO arm added to AlertSeverity.ForMetric while forgetting BOTH the classifier and the InlineData row failed nothing (#3476 review, promised on the inline thread). The fix goes the direction promised there: tighten the test to match the comment rather than the comment to match the test. A census beside the existing pins derives ForMetric's deliberate INFO arms from that file's SOURCE (string-literal switch arms, comments stripped, classified by the compiled method itself - an arm is deliberately INFO exactly when it renders identically to the unmapped fall-through, so the census cannot stale on a tier restyle) and IsInformational's names from ITS source (the 'is ... or ...' pattern - parsed rather than probed, because no probe set can enumerate a pattern's members, and the stale-entry direction is invisible to any probe derived from ForMetric), then asserts set-equality with a fix-in-hand message per direction, plus an invocation belt tying both parses to the compiled classifier. The McpToolTypeRegistrationTests shape from #3466, one assembly over. The InlineData theories stay: they pin behavior for the known arms; the census pins coverage. The overclaiming docs on IsInformational and the theory now describe the census that exists.
…for #3484), closes #3485 (#3491) * Offer Service Principal + Managed Identity in the Viewer's add-server dialogs (#3485) #3484 added non-interactive Entra auth to the headless onboarding paths; this completes it in the GUI. ServerStoreCredential.MapAuth now maps the two non-interactive Entra modes (the interactive modes still block at the whitelist), and both dialogs collect and store them: - Single AddServerDialog: Service Principal (client id + secret) and Managed Identity (optional user-assigned client id) resolve / prefill / test like SQL auth; the machine-bound-secret hint now covers service principal too. - Bulk AddMultipleServersDialog: one shared SP (client id + secret) or MI stamped on every pasted row - a single Entra app monitoring a fleet. The SP client secret is stored in the same DPAPI blob as a SQL password (no new field or column). The dialog XAML already had the SP/MI panels (a Lite port), so the change was opening the code gate. Tests: ServerStoreCredential map / RequiresSecret matrix updated (SP/MI supported, interactive unsupported); the machine-bound-hint source pin updated for the SP condition. Closes #3485. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DRsdSKC8p1qCQ3q4hcMsM7 * Migrate service principal + managed identity in the viewer->store projection too (#3485) CI caught two real gaps the Viewer change exposed: - ViewerServerMigration.TryProjectEntry accepted SP/MI (MapAuth is now non-null for them) but its secret branch was `!= Sql`, so a service-principal entry would project WITHOUT its client secret. Switched to RequiresSecret so SP resolves a secret like SQL and MI/integrated resolve none; generalized the skip message from SQL-specific to "requires a stored secret". - EntraDeviceCodeTests source-pin asserted MapAuth names no ServicePrincipal / ManagedIdentity (the old two-mode whitelist); updated to assert they ARE whitelisted now, keeping the device-code discriminator (no "Entra" substring appears in the arms, since SP/MI constants don't contain it). Tests: projection split into interactive-skip + SP (skips without a secret, projects with one) + MI (secret-less project); 36 Darling + 58 Lite green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DRsdSKC8p1qCQ3q4hcMsM7 * Address #3491 review: cross-auth secret reuse, MI profiles, MI client-id migration, stale docs (+ #3486 finding) The automated review found three real correctness bugs this PR introduced, now fixed: - Cross-auth-type secret reuse on Edit (AddServerDialog): the blank-secret "keep existing blob" fallback didn't check _existing.Auth matched the selected mode, so switching SQL<->ServicePrincipal and leaving the secret blank reused the wrong-type secret (old SQL password as an SP client secret, and vice versa). Each fallback is now gated on _existing.Auth matching the current mode. - Managed Identity credential profiles were unusable in both dialogs: the profile branch demanded a secret unconditionally, which an MI profile never stores. Gated the secret requirement on ServerStoreCredential.RequiresSecret. - User-assigned MI client id dropped in the viewer->store migration: the no-secret early-return didn't carry entry.ManagedIdentityClientId into Username, silently downgrading a user-assigned identity to system-assigned. Also fixed the review's doc nits (bulk dialog class doc + format-hint text) and a leftover #3486 finding: the SP secret-resolution branch's error strings still said "uses sql auth" (DarlingSecrets, DarlingServerConnector) - generalized to name a SQL password or a service-principal client secret. Tests: split the bulk/projection Azure tests into interactive-reject + SP/MI map-through; the MI projection test now sets ManagedIdentityClientId and asserts Username carries it. Full suites green (Darling 9586 / Lite 3905; the single Darling failure is an unrelated Npgsql cert-chain test this PR does not touch). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DRsdSKC8p1qCQ3q4hcMsM7 * Read the MI client id from the profile field that holds it The three profile-backed managed-identity branches read profile.Username for the user-assigned client id, but a ViewerCredentialProfile only ever populates Username for a SQL profile — an MI profile carries its optional user-assigned client id in ManagedIdentityClientId (Username is null). So a profile-backed user-assigned identity silently downgraded to system-assigned in the single-add dialog, the bulk dialog, and the store migration. Read profile.ManagedIdentityClientId at all three sites. Add a profile-backed case to the migration projection test (the entry-only path was covered; the profile path, where the bug lived, was not). Correct the doc comments that conflated the store row's single Username field (which does carry the id) with the profile's field layout (which splits identity across three fields). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DRsdSKC8p1qCQ3q4hcMsM7 --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…k budget, with stated omission (#3493) (#3494) Slack caps one text object at 3,000 characters and BuildSlackPayload rendered the alert's whole prose detail as ONE mrkdwn section, so the Collector Cost Digest failed delivery (HTTP 400 invalid_attachments) on exactly the stores busy enough to want it: both big-fleet stores' 20-mover digests failed on the first two live nights while the small store's 1-mover digest delivered, and one store's Fleet Sweep Rollup delivered through the SAME webhook 80ms after its digest failed. The ledger said failed with the transport error the whole time - the #3446 failed-arm working as designed. The fix is at the builder, so every long-prose producer inherits it: prose splits on line boundaries across as many sections as it needs (the AddSlackFieldSections precedent one level up), spends only what the payload's other blocks leave under Slack's 50-block message budget (details take precedence - the long-prose producers fire with no structured context, and the detail-heavy alerts carry short or suppressed prose), and a prose the budget cannot hold degrades by STATED omission: whole lines that fit, then one line naming the dropped count and quoting the first dropped line - the producers rank most-significant-first, so that quote is the headline of what the reader is not seeing - pointing at email / in-app Alert Details, where the full text has always lived. A single line past the cap hard-splits with a visible continuation marker. Small prose keeps the pre-#3493 rendering byte for byte, and the Teams builder is deliberately untouched: the same 20-mover digest renders ~7.2 KB of prose against the connector's ~28 KB cap. Co-authored-by: Erik Darling <erik@erikdarling.com>
…to-merge outruns (#3492) * A substantive review finding becomes a failed check, not a comment auto-merge outruns The #3470-#3473 train demonstrated the hole: claude[bot] left six review findings across one night's PRs (one real) and auto-merge outran every one of them, because review output was comments and the merge gate only ever read CI. The guard verified that the review POSTED, but a posted finding and a read finding are different observations, and nothing made a substantive finding cost anything. Now the review prompt carries a VERDICT PROTOCOL: every run ends by submitting exactly one formal GitHub review -- `gh pr review --request-changes` with a numbered summary when any finding is substantive (correctness, security, data integrity, contract violation), or a `--comment` LGTM review otherwise. Style nits, observations the review itself labels non-blocking, and tradeoffs the code or PR already documents as deliberate are explicitly NOT changes-requested, so the red mark keeps meaning something. Bash(gh pr review:*) joins allowedTools; permissions were already pull-requests: write, and nothing else widens. The guard grows the matching VERDICT ARM after its posted-something tally: fetch the PR's reviews, filter to the review bot, take the newest by submitted_at. CHANGES_REQUESTED fails the check, naming the review and the way out; anything else is clean. The semantics are NEWEST-VERDICT-WINS: a re-review after new commits reviews the new diff and submits a fresh verdict, and a clean fresh verdict clears an earlier changes-requested without --approve -- which the review token cannot issue without an org toggle this repo does not enable, and which nothing here needs. A human dismissing the review reads as cleared too (DISMISSED). Staleness needs no SHA bookkeeping: the concurrency groups already cancel both workflows on a new push, and the replacement guard waits for the replacement review run to complete, so the newest verdict at scan time is the current head's. The coupling moves as one commit because the guard's sentinel contract says it must: ALWAYS POST now appears in the prompt, which flips the guard's strict arms on by itself. Silence with the sentinel shipped stays a lost output (exit 1), and comments without the promised verdict review are the same broken-promise class -- and exactly the pre-verdict failure shape, findings living only in comments. Absent the sentinel (an older prompt, or a future one that drops the protocol) the verdict arm stands down, because runs that never promise fresh verdicts could never clear a stale changes-requested and the gate would wedge shut. The #2309 discipline is untouched: the new lookup routes through lookup(), so an API failure marks the review UNCONFIRMED and exits 0 -- a lookup failure is never a verdict. Verified with fixture JSON against the extracted step: newest CHANGES_REQUESTED fails naming the URL; an older changes-requested superseded by a newer COMMENTED clears; a verdict-scan API error is UNCONFIRMED exit 0; comments-without-verdict fails; no sentinel stands down; DISMISSED clears; human reviews are filtered out; pending (null submitted_at) reviews are skipped; both posted-nothing arms behave as before the obligation-read hoist. This PR edits claude-review.yml, so claude-code-action will refuse to review it (guard arm 1 warns and passes, as designed) -- a human must review it, and review stays refused repo-wide until the next release ships dev to main, which the guard header already documents as the cost of touching this file. * Name the guard's check what the enforcement requires: the job id was the check-run name, and the required context was a phantom Branch protection matches required status checks on the check-run name, which is the job's DISPLAY name - and the guard job had none, so it reported as its bare id 'verify' while the required context said 'Claude review guard'. Nothing could ever satisfy it: this PR's own first CI cycle sat all-green and BLOCKED on the phantom. The display name is now the enforcement contract, and the comment beside it says the two move together. --------- Co-authored-by: Erik Darling <erikdarlingdata@gmail.com>
…ebreak (#3496) (#3498) The alert pass's collection-signals read asked collection_log - a time-partitioned hypertable, 16.4M rows, 32 chunks, compressed history on the biggest production store class - for a server's recent runs with ORDER BY log_id DESC LIMIT $2. log_id is not the partition column and appears in no index, so the planner's only legal plan appended all 32 chunks - decompressing the columnar history - into a top-N heapsort of ~389,000 rows to return ten, twice per statement, every alert pass, every 30 seconds, per server. As the 90-day retention horizon filled, the cold excursions crossed the pass's 10-second command deadline (derived from a 1,744.9ms measured worst case): swallowed read failures climbed 13 -> 31 over twelve hours while the two same-class controls sat at 3 and 0. The statement carried its own control: its MAX(collection_time) arm - same table, same server_id predicate - resolved in 0.097ms, because its sort key is chunk-orderable, so ChunkAppend stopped at the newest chunk and 30 of 32 chunks reported never-executed. The recent-N subqueries now order by collection_time DESC, log_id DESC: semantics-identical (a server's id order and time order agree; the id stays as the deterministic tiebreak within one collection instant) and horizon-independent (ChunkAppend stops at the newest chunks however many the horizon accumulates, so extending retention cannot regress the read). The MAX arm is untouched - it is the measured control. Verified against a real TimescaleDB with compressed chunks: on a 37-chunk table (33 compressed, ~104K rows/server) the old shape executed every chunk and read the whole population; the new shape left 36 of 37 never executed and read 1,252 rows - the newest chunk's worth, an 82x collapse bounded by chunk width - with both orderings returning identical rows in identical order, ties included. The ordering is pinned at the source (the regression is silent on any store small enough to test against, which is how it shipped the first time), the gated live suite gains the ordering arm (newest-instant-first fill, highest ids winning inside a tied instant), and the 10s deadline's derivation comment now records this read as the first measured breach of its margin. Co-authored-by: Erik Darling <erik@erikdarling.com>
…res that had none (#3503) The screenshots were from February 2026 and had drifted: the hero was the deprecated Full Dashboard, and the "Lite Query Performance" shot predated the current tab layout. Recapture everything live while a HammerDB TPC-C workload ran on a monitored server, so the fleet is shown catching a real incident (one server Critical with blocking and deadlocks) rather than sitting idle. - Hero: the fleet overview flagging a Critical server, replacing the deprecated Full Dashboard shot. - New sections for features the README described but never illustrated: FinOps, Recommendations (advise-and-act), Fleet Sweeps, Blocking & deadlocks, and custom-view dashboards. - Drop the tiny alert-toast image (replaced by a richer Alert History view) and three unreferenced deprecated-Dashboard images. - Keep the Graphical Plan Viewer and MCP-analysis shots. FinOps and Recommendations are the desktop viewer; the rest are the Darling web dashboard. File names are ASCII, hyphenated, and space-free. Claude-Session: https://claude.ai/code/session_01RzwoqcD62jKyp6tzsWpZLz Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* The Add Server dialog can say PostgreSQL (#3499) The service carries PostgreSQL targets end to end -- the registry row has held engine and port since V70, the probe answers in that engine's version vocabulary (#3244), the collectors and the get_pg_* MCP surface serve the data -- and the desktop dialog that onboards servers offered no way to say the word. The viewer's row model, its upsert, and every one of its registry reads simply predated the columns: writes defaulted them ('sqlserver'/0) and reads never surfaced them, so an operator adding a PostgreSQL target got a SQL Server row whatever they meant, and the README's claim that the dialog is a PostgreSQL onboarding route described the intended state, not the shipped one. Reported externally with tag-pinned citations and an expected-behavior list precise enough to use as the acceptance spec -- and it was. The dialog grows an engine selector; SQL Server stays the checked default and that arm is pinned to render exactly the pre-selector form plus the selector row itself. The PostgreSQL arm exposes port (blank = 5432, the same omitted-means-default contract as MCP add_servers, so a default-port target derives the SAME identity whichever door onboarded it), keeps the shared Database/Encryption/Trust controls with a note mapping them onto sslmode, hides the SQL-Server-only controls by Visibility only -- hidden is not reset, which is what lets an edited row's stored values survive the round trip -- and pins auth to username/password, the one mode the PostgreSQL connect path honors, enforced again at save with the same tailored refusal the backend applies (#3486's discipline: name what IS supported). Engine and port ride Test Connection -- probing the exact connection the save will store, instead of reaching 5432-as-SQL-Server and vouching for a row that then fails every sweep -- and land in the registry write; port is omitted from args_json at its default so a SQL Server request's bytes are unchanged. The edit path loads a PostgreSQL row back recognizing every stored engine spelling (a darling.json seed persists its raw text), locks the engine radios -- the engine is part of what the server IS, and #2158 made an edit keep its identity, so a flip would interleave two engines' histories under one id -- and saves the stored engine string verbatim. The identity plumbing beneath gets the same completion. ComputeServerId takes engine and port with inert defaults (no pre-existing id moves; a PostgreSQL Add derives what add_servers would). The address-collision guard compares the full #2218 identity, with engine folded to a KIND in SQL rather than compared raw: the stored spelling is whatever onboarded the row, and because the upsert lands ON CONFLICT (server_id), a raw-string miss against a same-identity row spelled differently is not a duplicate but a silent clobber of the existing registration -- the exact write the guard exists to refuse. The bulk dialog's dedupe gate inherits the full-identity key for the same reason (a PostgreSQL row in the store no longer reads a pasted same-host SQL Server add as a duplicate); its engine picker is deliberately deferred, stated in the class doc: its paste grammar already spends the numeric field on the SQL Server port-rides-the-host convention, and bulk PostgreSQL onboarding stays with add_servers, a bulk tool by construction. Pinned in the #3491 idiom: the pure seams behaviourally (ParsePortText, IsPostgres parity with the service's own TargetEngine parse, row defaults, identity parity, args round-trip through MonitoredServer, dedupe-gate arms), the WPF shapes textually (the XAML default state IS the SQL Server form, the edit lock and prefill, the save gate, the test-connect threading). The viewer role's column grants already carried engine and port (V68), verified on both the shipped .sql and the managed-roles twin. * Adopt the shared repo-file reader the census requires: the dialog tests walked up on their own RepoFileAdoptionTests holds exactly what the #3489 review nit asked for - one declared reader, everyone else adopting it - and the new dialog tests declared a private walk-up instead. Adopted; the helper census is the reason a thirty-fifth copy cannot ship.
…nign, not RDS-only (#3501) (#3505) When the one-time `blocked process threshold (s)` bootstrap can't run, Lite and Darling logged "Cannot set blocked process threshold via sp_configure (may require platform config)". On an on-prem instance whose monitoring login lacks ALTER SETTINGS that reads as an RDS/Azure platform limitation rather than the permission issue it is, and it never says the failure is harmless. A user hit exactly that confusion after upgrading (#3501). Reword both emit sites to name the real cause (login lacks ALTER SETTINGS, or a platform where sp_configure is unavailable) and to state the benign consequence: blocking is still captured by the always-on DMV blocking snapshot; only the richer blocked-process-report XE stays off until the threshold is set. Raw exception detail moves to the end so its embedded newlines don't split the guidance. Behavior is unchanged - still INFO, still tolerated. Claude-Session: https://claude.ai/code/session_01RzwoqcD62jKyp6tzsWpZLz Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… pays for a sample (#3477) (#3509) * A per-collector database allow-list on the schedule row, so a fan-out pays for a sample (#3477) V125: config.config_collector_schedules gains a nullable databases text[] — empty/NULL is today's behavior, non-empty collects ONLY those databases, the fleet-wide row sets a default a server row overrides (NULL column falls through, an explicit empty array is the opt-back-out), and excludedDatabases still wins because the composed predicate is scoped-in AND NOT excluded, spliced engine-side in the exclusion filter's own parameter shape so both instruments share the engine's collation reality. One seam per fan-out family: the five enumeration-driven collectors compose the predicate through DatabaseScopeFilter.BuildEnumerationPredicate (a catalog-walking census pins that a sixth enumerator cannot skip it), and the per-database connection loop takes it inside both providers' BuildDatabaseListPlan. The worker resolves the scope live off the same schedule overrides the cadence gate reads; the Viewer's schedule editor gains a comma-separated Databases column that round-trips the null/empty distinction WYSIWYG. Verified against a live TimescaleDB pg17: ladder lands at 125, the three stored readings stay distinct, the beacon bumps on a scope write, and the on-engine enumeration returns scoped-in minus excluded with a never-named database staying out. * Split the scope column's live round-trip into its own serialized class, the hygiene census's preferred shape LivePostgresCollectionHygieneTests requires every class reaching the shared store to serialize or say why not - and the rung file's live arm writes config_service and config_collector_schedules on the shared store directly, so the own-store exemption would be a lie. The split is the shape the census itself recommends: the live test carries Collection(live-postgres) in its own file, and the rung file's eleven pure pins stop paying serialization for one live arm. * Keep the changelog sidecar out of the repo: it is the shepherd's buffer, not cargo * Mirror the scope clause into Lite's twin description, attributed: the byte-identity census is the pin the blast-radius map called absent BothSkusToolDescriptions_StayByteIdentical holds the two SKUs' get_collection_health descriptions equal to the byte, and the empty-enumeration honesty clause went into Darling's only - the lane's map read the clause as unpinned because the pin lives in the OTHER SKU's test project. Mirrored with the house attribution for Darling-only knobs, identical bytes both sides, proven by extraction before commit.
…er index (#3508) (#3510) The per-database body staged the three stats DMVs into #temps (the #1135 fix) but built the per-index DEFINITION metadata in the final SELECT as five correlated subqueries against the live catalog, evaluated per index row: key_columns and included_columns (FOR XML over sys.index_columns/sys.columns), the two FK EXISTS over sys.foreign_key_columns, and the compression TOP(1) over sys.partitions. On a wide schema that is ~5 catalog probes per index, and the cost scales with index count - the body-cost half of #3477 (a 54K-index database spends ~28 s here). Extend the existing staging to the definition catalog, the technique sp_IndexCleanup already uses: stage sys.index_columns (+ column names, once, with a supporting clustered index), the FK-backed/FK-referenced key-column sets, and the per-index lowest partition compression into #temps, then LEFT JOIN them in the final SELECT with no correlated subqueries. The emitted key/include strings and every flag are byte-identical, so the Stage-2 dedupe and FK-protection logic are unaffected - a plan-shape change only. IndexObjectStatsCollector is shared, so Lite and Darling both get it. Measured on a synthetic 56,001-index database (empty tables, so this is the definition-metadata cost in isolation): the body drops from ~4.1 s to ~2.0 s, and an EXCEPT-both-ways parity check returns zero rows in either direction at 12K and 56K indexes. A guard test pins that the definition metadata is staged, not correlated. Claude-Session: https://claude.ai/code/session_01RzwoqcD62jKyp6tzsWpZLz Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…eers.storeName (#3500) (#3506) Every fleet-level self-alert a Darling store emits -- Store Disk Pressure, Retention Held, the Collector Cost Digest, the Fleet Sweep Rollup, all twelve families and their resolution edges -- fired under the constant "Monitor Store". Unambiguous with exactly one store, and the product does not assume there is only one: the peers block exists precisely to describe a multi-store estate. The reporter is standing up one store per data centre, both feeding one Logic App, one Teams channel and one Azure DevOps board, and named both halves of the cost with precision: on the shared channel the alert cannot say which host it is about, and -- the half that bites -- the delivery fingerprint downstream automation dedups on is computed inside the product from the same constant identity on both stores, so the second store's disk pressure never opens its own work item; it lands on the first store's as a recurrence. One host's incident is silently lost, and no transport-level workaround can reach a fingerprint computed inside the product. The new file-only peers.storeName -- a short LABEL beside thisStoreCovers, which stays the sentence it is -- opts a store in. Set, it becomes the server label on every self-alert family through ONE seam: EffectiveStoreLabel resolves the configured name once at construction (the peers block is restart-only, so a Func here would claim a liveness the config cannot deliver), every fire site reads _storeLabel, and StoreKey() prefixes the label onto the family key beside it. The key half is load-bearing rather than cosmetic: a no-incident alert's dedup identity is {serverKey}:{metric} (DerivePagerDutyDedupKey's fallback), and the server NAME is not in it -- relabel the card without qualifying the key and two stores' work items keep colliding, the invisible half of the bug. The family key stays inside the qualified form because the per-object families (retention, compression, job cadence) rely on it to keep per-object cooldowns apart, and history storage is untouched either way: every non-numeric key already collapses into the write-only server_id 0 bucket (#3456), so qualifying re-keys nothing but the fingerprint -- which is the re-key the operator accepted by setting the field. Unset is byte-identical to today, pinned by comparing fired outcomes as whole records. That promise is also why the reporter's suggested machine-hostname fallback is deliberately NOT taken: it contradicts their own (correct, bolded) opt-in requirement by performing the fingerprint re-key on every existing install at upgrade, silently -- the exact harm they name. Only setting the field opts in, and a storeName spelled exactly as the constant resolves TO the constant: same bytes, no re-key, rather than an opt-in that changes nothing but the key shape. The couplings ride along rather than being discovered later. Mute rules match the alert row's server spelling, so an opted-in store's self-alert mutes must target the label -- documented at the field, in the sample, the README, the MCP tool descriptions and the CHANGELOG, and held by tests in both directions on the stale-mute alert's explicit-rule scan (the one mute path that does not go through the shared seam) plus a capture at the shared seam itself. The triage endpoint recognises BOTH spellings as fleet-level -- the label is by design not in the registry, and channels still hold links minted before the opt-in -- read through the ambient DarlingPeerDirectory publish the peers block already has, rather than a second config load. The label gets the peers block's credential-shaped-text refusal too, unconditional and before any early return, because it rides out on every delivery channel, a wider broadcast than the MCP-only siblings. And storeName alone does NOT summon the Fleet Coverage disclosure: it names the store on its own alerts, it does not declare a fleet split, so IsEmpty deliberately ignores it. A label that shadows a monitored server's display name is a stated non-goal to police: the registry is store-authoritative after seeding, so config validation cannot see it -- the field's comment and the docs say not to reuse one, and what it would cost (the two become indistinguishable on shared surfaces, including the triage page's fleet-level read). The resolution prose that hardcoded "Monitor Store:" now speaks through the label for the same reason the rows do, which is also what lets the census hold: the literal appears in exactly one string in the evaluator (the const), every fire site flows through the seam (held from source, so a thirteenth family cannot quietly hardcode the constant back in), and everywhere the label travels as a server name, the key beside it goes through StoreKey(). The reporter's "minimal viable fallback" (a display-only payload field leaving fingerprints untouched) is deliberately not in this change: the full opt-in subsumes its purpose, and a fifteenth webhook token has census reach across both apps' Settings help text. If the display-only field is still wanted beside the opt-in, it is its own small follow-up. Credit to the report: the estate shape, the fingerprint mechanics, the enumeration of the twelve families and the opt-in requirement arrived already worked out. The issue was the design document. Closes #3500. Co-authored-by: Erik Darling <erikdarlingdata@users.noreply.github.com>
…verdict dominance was being read as (#3502) (#3507) get_collection_health's per-collector fanout block published dominance - slowest_ms * items / run_ms, the slowest item against the MEAN item - and the tool description told readers to take the width-versus-concentration decision on it: near 1.0 the cost is width, around 2.0 or above one database dominates. That reading is numerically wrong at width, because the ratio's ceiling is items: at 72 databases, even a dominance of 10 is one database holding 14% of a pass. The #3477 reporter's evidence proved it with two instances scoring 3.58 (72 items) and 4.20 (15 items) - near-neighbour scores the guidance calls the same shape, whose slowest items' shares are 4.97% and 27.98%, 5.6x apart and wanting opposite remedies. The block now carries slowest_share_pct - slowest_ms / run_ms, which is dominance / items - the one-division-away number the decision actually turns on: low share means width and bounded parallelism is the lever, high share means one database dominates and a per-database schedule override or a stagger reaches exactly it. dominance stays for continuity as the evenness ratio - it is not wrong, it is just not a verdict. The share is DERIVED from dominance (share = dominance / items) on both SKUs' row models rather than recomputed from the columns, so the two figures can never drift apart or describe different runs, and it is null exactly when the fanout block is null - never a 0% that would read as "perfectly even" on a run that reported nothing. No schema change: the V80 columns already carry everything the share needs. Both SKUs move together because BothSkusToolDescriptions_StayByteIdentical holds the two descriptions to one contract: the corrected guidance routes the decision through the share and carries the two-instance counter-example as the permanent warning label, pinned alongside the field's wiring in CollectionLogFanoutRollupStoreTests (the anonymous-object emission no behavioural test reaches - the #3010 lesson). The V80 rung test keeps its two-shape arithmetic and now separates them by share; the new counter-example test pins the wide instance, the issue's dominance-of-10 limiting case, the share-saturates-at-100 endpoint, and the derivation itself, so a future edit that recomputes either figure from different columns fails loudly. The store round-trip asserts the share off real V80 columns beside the dominance it already read. Credit: the derivation, the counter-example, and the "publish the number the decision turns on" framing are the #3477 reporter's, verbatim from their evidence comment.
…e job (#3495, #3497) (#3511) * Name the active maintenance operation on the High CPU card (#3495) First night of live dogfood alerting: a High CPU page delivered honestly — 100% total CPU, correctly split SQL/other — and the cause still took two more instrument reads to establish: an RDS-managed BACKUP DATABASE ... WITH COMPRESSION seventeen minutes in (program RdsAdminService, ASYNC_IO_COMPLETION), visible the whole time in the active-session snapshot the service had already collected on the same sweep. Everything the triage needed was knowable by the alert pass at fire time; the card could not say the one sentence that closes it. When the High CPU condition fires, the fire branch now reads the same active-session surface the operator read by hand — the long-running-query alert's own store read, threshold zero, every noise opt-out OFF and an empty exclusion list, deliberately: excludeBackups exists to keep maintenance off THAT alert, and this probe wants exactly the population it removes — and appends one line per system-maintenance session to detail_text: Active maintenance: BACKUP DATABASE (RdsAdminService), 17m 14s elapsed, ASYNC_IO_COMPLETION BACKUP DATABASE/LOG, RESTORE DATABASE/LOG and ALTER INDEX are the recognized shapes, matched on the trimmed statement HEAD rather than contains-anywhere, so the annotation under-claims (a maintenance statement buried mid-batch costs only its line) rather than naming maintenance on a card where none runs. Concurrent sessions get one line each, longest first, capped at three with the omission stated — the #3494 discipline — and the longer prose rides the splitter that fix installed, so it costs Slack section blocks, never delivery. ANNOTATION, NEVER SUPPRESSION: the tiers stay, the page still fires, the line states what IS (kind, program, elapsed, wait) and never a verdict. A backup pinning CPU at 04:30 with the store idle is routine; the same pin at 14:30 under checkout load is a capacity finding with a named cause — that judgment belongs to the reader. Bounds and degrades: the probe runs only inside the cooldown-gated fire branch (one bounded read per delivered page, never per sweep; muted fires included so the history row matches), and a failed read logs at Warning, is counted on distinguishable from a condition gone blind — and leaves the card byte-identical to today, which the no-maintenance arm pins exactly. Fingerprint-inert by construction: High CPU carries no incident fingerprint and its cooldown keys on (server, metric), so a re-fire with a different elapsed is the same incident it always was. * Name the Agent job on the Long-Running Query card (#3497) Second night of live dogfood alerting: the nightly index-maintenance window opened and the Long-Running Query alert did its job — a dozen honest cards, each showing ALTER INDEX ... REORGANIZE past the threshold — and each leaving the reader to reconstruct "that is the maintenance job, running as scheduled, merely long" from the statement shape, the hour, and prior knowledge, twelve times, every night, because index maintenance recurs. The session data already identified the program as a SQL Agent job step; the card never surfaced WHICH job. Now a card whose session's program_name carries the standard 'SQLAgent - TSQL JobStep (Job 0x<hex> : Step <n>)' form gains one field, directly under the raw Program it explains: Running under Agent job: nightly index maintenance, step 3 (Reorganize fragmented indexes) The identity mechanics live in one shared place (AgentJobStepQuery), because a wrong job-id conversion here does not error — it resolves nothing, silently, on every card. The hex in program_name is sysjobs.job_id CONVERTed to binary(16): SQL Server's GUID layout, Data1–Data3 little-endian, the same conversion the collector's CDC filter already does server-side via TRY_CONVERT — and .NET's Guid(byte[]) constructor reads exactly that layout, so the parse does no byte swapping and the round trip is PINNED in AgentJobStepQueryTests (job_id AB6D9F63-3B01-4E15-9F34-B0A0F0B355A2 <-> 0x639F6DAB013B154E9F34B0A0F0B355A2) rather than trusted. Resolution is FIRE-time, deliberately, over capture-time: no schema rung on either store, no degrade baked into collected rows, nothing on quiet sweeps — the lookup sits inside the cooldown-gated fire branch, scoped to the sessions the card will show (the builder's display cap, now named), deduped, and answered in ONE msdb round trip (a VALUES join over sysjobs + sysjobsteps; INNER on sysjobs so a deleted job stays honestly unresolved, LEFT on sysjobsteps so a missing step still names the JOB). Both hosts wire the resolver on the recently-failed-job check's exact live-msdb seam and permission arm: a login without SELECT on msdb.dbo.sysjobs and sysjobsteps (expected for read-only monitoring accounts — SQLAgentReaderRole is not the remedy) degrades to an empty map, logged Info with the grant named, and the card renders the unresolved form — 'Running under Agent job: (name unresolved) Job 0x<hex>, step <n>' — which still states the fact the parse alone establishes and carries the raw marker an operator can match against the Program field by eye. A null resolver renders the pre-#3497 card byte-identically; both arms pinned. ANNOTATION, NEVER SUPPRESSION — the #3495 contract, sibling card: the thresholds stay, every card still fires, the annotation states what IS and never a verdict. And it is fingerprint-INERT by construction: the #1140 dedup key hashes (server, type, query_hash) and the annotation is a rendered field, so a card that re-fires with a different elapsed — or with the job name freshly resolved, or freshly unresolvable — folds into the same incident, which per-fingerprint delivery cooldowns and PagerDuty correlation key on; pinned through the engine, not merely stated. CHANGELOG entries for both siblings are buffered in CHANGELOG.pending.md per the parallel-lanes process; CHANGELOG.md itself is untouched. * Count the maintenance-annotation probe where every swallowed alerting read is counted The #3495 probe records its swallowed failure under its own name and clock - and the read-failure censuses hold the population by pinned count, per-file scope, and narrated addition. AlertEngine's fourteenth site joins the ledger the way the evaluator's eighth did on #3466: counted, because a fault folded into 'no maintenance ran' is the quiet-is-not-clean misreading wearing a card. * Give the resolver its own clock, and classify its catch: an msdb read on the monitored server, exempt by the failed-jobs precedent The census's deeper arms caught what the count fix alone could not: the LRQ block's two awaits shared one stopwatch (a resolver fault would have recorded the store read's elapsed on top of its own), and the resolver's catch was neither counted nor exempt. Restart between the awaits; the catch joins the exemption table keyed on its own log line - it reads the monitored server's msdb through the host resolver, not the store, the Recently-failed-job precedent one seam over, and the card's unresolved form keeps the gap visible where a count would double-book a read that is not the store's. * Restart the clock on the resolver's exit, not just its entry: the fault that must not double-bill is the NEXT one The census's pairwise walk flagged resolver-to-FireAsync; the earlier fix guarded store-read-to-resolver, the pair above it. Both boundaries now restart, and the faithful replication of the walk shows every consecutive pair in the long-running queries block carrying one.
…es, one PR, zero rebase conflicts (#3512) The parallel-lane pattern's second half: code PRs stopped touching CHANGELOG.md (every lane inserting at the Unreleased top guaranteed serial conflicts), each lane buffered its entry with the shepherd, and this PR seats them together - the collector database scope (#3477), both card annotations (#3495, #3497), the store self-alert label (#3500), and the fanout share (#3502) - with their reference links, in one conflict-free pass.
…w-up to #3499) (#3513) On the PostgreSQL engine arm the auth-type picker collapsed to a single radio still labeled "SQL Server Authentication" - force-checked but left visible, so a PostgreSQL target showed a one-choice picker mislabeled for the wrong engine (caught driving the live .447 nightly). Hide SqlAuthRadio on the PG arm too: it stays force-checked, and UpdateAuthPanels keys the username/password panel on IsChecked rather than Visibility, so the credentials still render directly under the Authentication header - which is exactly the "rather than leaving the operator a picker with one choice" the handler's own comment already claimed. Going back to SQL Server restores the radio. Test renamed and extended to pin the SqlAuthRadio visibility line. Claude-Session: https://claude.ai/code/session_01RzwoqcD62jKyp6tzsWpZLz Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…artup log line (#3514) (#3515) A LAN-exposed web dashboard loads its web.network.tls certificate once at service start and serves it for the process's life; its expiry was surfaced only by a startup-log warning within 30 days and the --status summary, both of which need someone to look. On a headless service that runs for months without a restart, neither re-fires as the clock crosses the window, so a lapsed certificate first showed up as the dashboard silently failing closed to loopback-only. A new host->worker seam, WebTlsCertificateState, carries the SERVED certificate's expiry out to the worker's alert sweep - the served certificate, not the file on disk (exposure is restart-only), and an already-expired certificate the host refused at start still surfaces its facts. A new DarlingSelfAlertEvaluator family re-reads that fixed expiry against the clock each sweep and raises a fleet self-alert through the existing delivery/cooldown/mute plumbing: Warning inside ExpiryWarningDays (the window the startup log uses), Critical once lapsed, one resolution when renewed past the window or TLS switched off - firing without a restart and re-stating daily. Wired into the triage endpoint; "Renewed" added to the resolution vocabulary and the metric registered state-only, both pinned by the census tests. Only fires when a certificate is actually served (loopback-only / unconfigured / unusable raise nothing). WebRuntimeState is deliberately single-direction (worker->host) and excludes web.network, so this is a new sibling object rather than an overload. Closes #3514. Claude-Session: https://claude.ai/code/session_01RzwoqcD62jKyp6tzsWpZLz Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…the expiry alert resolves (follow-up to #3514) (#3516) Adversarial review of #3514 caught a reachable bug: WebTlsCertificateState was write-once and StopServerAsync disposed the certificate without clearing it, so disabling the web dashboard at runtime (a no-restart op) left the worker firing the expiry alert forever - escalating to a Critical that claims the LAN dashboard is unreachable, about a dashboard the operator deliberately turned off - with no resolution short of a full service restart. Add WebTlsCertificateState.Clear() and call it wherever the host stops serving TLS: StopServerAsync (runtime disable), DisposeFailedStartAsync (failed start), and the SAN-catch loopback degrade. The lifetime-refusal path deliberately keeps the published facts so an expired-at-start certificate still fires its Critical. Rebind-safe: Stop->Start run back-to-back in one supervisor tick, re-publishing before the worker's hourly sweep observes the null. Also makes the resolve path reachable (review finding #2): the worker's report is now built by a pure static BuildWebTlsCertReport(snapshot) mapping a null/cleared snapshot to Configured=false, so a stop resolves the alert. Dropped the misleading "renewed past the window" test (it fed a report the seam cannot produce in-process - in-place renewal is restart-only), kept the now-reachable disable->resolve test, and added seam tests for Clear() and the report builder. CHANGELOG wording corrected to match. Claude-Session: https://claude.ai/code/session_01RzwoqcD62jKyp6tzsWpZLz Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* Cut 3.8.0: version bump to 3.8.0 and CHANGELOG archive Directory.Build.props 3.7.0 -> 3.8.0 (the single version source, #3222). CHANGELOG [Unreleased] renamed to [3.8.0] and its prose archived to docs/changelog/3.8.md via changelog_archive.py (split/census/verify/roundtrip all clean; census regenerated so ChangelogIndexAndArchiveTests passes). No [3.7.1] heading (that release was schema-identical to 3.7.0; adding it would break the migration-ladder fixture test). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RzwoqcD62jKyp6tzsWpZLz * Clear pre-existing xUnit-analyzer warnings in four test files (release zero-warning gate) The release-checklist step-4 bar is 0 warnings including test projects (they sit outside the WarningsAsErrors gate, so a Rebuild surfaces what incremental builds hide). Ten mechanical analyzer fixes, semantics preserved and each affected class re-run green: Assert.Equal(n,coll.Count) -> Assert.Single/Empty; Assert.True/False(regex.IsMatch) -> Assert.Matches/DoesNotMatch; and the xUnit2000 expected/actual swap so the constant is the expected argument. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RzwoqcD62jKyp6tzsWpZLz --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
erikdarlingdata
enabled auto-merge
September 17, 2026 21:22
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes dev to main for the v3.8.0 release. Full entry: docs/changelog/3.8.md (indexed in CHANGELOG.md).
Highlights this cycle: user-authored custom alert rules + PG compose measures (#3285), Fleet Sweep reports (#3466), web-dashboard TLS certificate expiry self-alert (#3514), non-interactive Entra auth — Service Principal / Managed Identity (#3484) and Device Code, per-collector database scope (#3477).
Head is dev (check-pr-branch gate). After merge: tag v3.8.0 (lightweight) and publish the GitHub release to trigger the signed build.
🤖 Generated with Claude Code
https://claude.ai/code/session_01RzwoqcD62jKyp6tzsWpZLz