Repository navigation
Per-server threshold overrides: loader, PostgreSQL counts, set_alert_threshold_override (#5916) - #5933
Conversation
…ion, set_alert_threshold_override, get_alert_settings server view (#5916)
…ssification pins (#5916)
…in only (#5916 part 4) Viewer: ThresholdOverridesWindow with ThresholdOverridesViewModel (key and bounds checked before any write), opened from Settings, Alert rules. Web: GET and PUT /api/threshold-overrides and an Admin 'Threshold Overrides' tab; the write is the tool's own body, 403 naming the admin login under the viewer role. The key table moved to PerformanceMonitor.Alerting and the store statements to Darling.Storage so the Viewer, the web route and the tool share them.
| /// </summary> | ||
| internal static AgentActionRefusal? AgentKindRule(DarlingThresholdKeys.Entry entry, int value) | ||
| { | ||
| var unreachable = value == int.MaxValue |
There was a problem hiding this comment.
A Level 2 agent can still turn an alert off per server; the README and this tool's own description say it cannot.
unreachable is value == int.MaxValue or value == 100 on exactly two keys. Every other way to make a bar unreachable passes:
| key | bounds (DarlingThresholdKeys.cs) |
value that passes | effect on that server |
|---|---|---|---|
blocking.count_threshold |
floor…int.MaxValue |
2147483646 |
Blocking Detected can never fire |
deadlocks.count_threshold |
floor…int.MaxValue |
2147483646 |
Deadlocks Detected can never fire |
blocking.pg_count_threshold / deadlocks.pg_count_threshold |
floor…int.MaxValue |
2147483646 |
the two PostgreSQL gates can never fire |
pvs.threshold_percent |
0…100 | 100 |
PVS can never breach |
file_growth.volume_percent |
0…100 | 100 |
File Growth volume arm can never breach |
low_disk.threshold_percent + low_disk.threshold_gb |
0…100 / 0…max | 0 + 0 |
Low Disk can never fire |
long_running_query.threshold_minutes |
1…int.MaxValue |
2147483646 |
no query is ever long-running |
So the two sentences this PR ships are not true:
Darling/README.md(new section): "the agent … cannot set a value that would turn the alert off".- the tool
Descriptionat line 157: "To silence an alert, mute it: an override moves a threshold and never turns an alert off."
The PR body's "What the coordinator should double-check" is honest that the rule is "deliberately narrow", and which values count as "off" really is a product call — but the shipped docs state the strong version as a guarantee, and an agent reading the tool description is being told a bound that is not enforced. Either tighten the rule (e.g. refuse the key's Max and the degenerate end of each percent/floor key under the agent arm, not just int.MaxValue/100 on two keys) or soften both sentences to say what is actually checked.
ThresholdOverrideTests.TheToolCannotLowerAnAgentsBarToNothing_AndLeavesOrdinaryValuesAlone pins only the two shapes the rule already catches, so nothing here fails on the gap.
| private async void Save_Click(object sender, RoutedEventArgs e) | ||
| { | ||
| var server = ServerCombo.SelectedItem as ThresholdOverrideServerChoice; | ||
| var outcome = await _model.SaveAsync(server?.ServerId, KeyCombo.SelectedItem as string, ValueBox.Text, DatabaseBox.Text); | ||
| SetStatus(outcome.Message, !outcome.Ok); | ||
| if (outcome.Ok) | ||
| { | ||
| await LoadAsync(); | ||
| } | ||
| } | ||
|
|
||
| private async void Remove_Click(object sender, RoutedEventArgs e) | ||
| { | ||
| var outcome = await _model.RemoveAsync(OverridesGrid.SelectedItem as ThresholdOverrideViewRow); | ||
| SetStatus(outcome.Message, !outcome.Ok); | ||
| if (outcome.Ok) | ||
| { | ||
| await LoadAsync(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Both handlers are unguarded async void: any store failure that is not SQLSTATE 42501 crashes the Viewer.
ViewerDataService.SaveThresholdOverrideAsync / RemoveThresholdOverrideAsync (ViewerDataService.ThresholdOverrides.cs:32, :49) catch exactly one thing:
catch (PostgresException ex) when (ex.SqlState == InsufficientPrivilegeSqlState)and ThresholdOverridesViewModel.SaveAsync / RemoveAsync catch exactly ViewerReadOnlyException. So a dropped connection, a command timeout, a lock timeout, a serialization failure, or the foreign-key violation a server with no config.config_monitored_servers row would raise all propagate out of these two async void handlers. An exception from async void is posted to the dispatcher, and App.OnDispatcherUnhandledException (App.xaml.cs:188) marks only the Hardcodet tray-tooltip race Handled — everything else is logged and left unhandled, which takes the process down.
LoadAsync just above (line 71) already has the right shape (catch (Exception ex) when (ex is not OperationCanceledException) → SetStatus(..., true)), and NotificationRoutesWindow — the window this one's XAML says it mirrors — wraps every async void handler: AddRoute_Click:103, EditRoute_Click:157, ToggleRoute_Click:175, DeleteRoute_Click:201, each catch (Exception ex) { ShowError(...); }.
This also makes the class doc comment at line 11 ("The store's own refusals … surface as a sentence, never as an exception to the window") and the PR body's "a store refusal becomes a sentence" true only for the 42501 case.
| private async void Save_Click(object sender, RoutedEventArgs e) | |
| { | |
| var server = ServerCombo.SelectedItem as ThresholdOverrideServerChoice; | |
| var outcome = await _model.SaveAsync(server?.ServerId, KeyCombo.SelectedItem as string, ValueBox.Text, DatabaseBox.Text); | |
| SetStatus(outcome.Message, !outcome.Ok); | |
| if (outcome.Ok) | |
| { | |
| await LoadAsync(); | |
| } | |
| } | |
| private async void Remove_Click(object sender, RoutedEventArgs e) | |
| { | |
| var outcome = await _model.RemoveAsync(OverridesGrid.SelectedItem as ThresholdOverrideViewRow); | |
| SetStatus(outcome.Message, !outcome.Ok); | |
| if (outcome.Ok) | |
| { | |
| await LoadAsync(); | |
| } | |
| } | |
| private async void Save_Click(object sender, RoutedEventArgs e) | |
| { | |
| try | |
| { | |
| var server = ServerCombo.SelectedItem as ThresholdOverrideServerChoice; | |
| var outcome = await _model.SaveAsync(server?.ServerId, KeyCombo.SelectedItem as string, ValueBox.Text, DatabaseBox.Text); | |
| SetStatus(outcome.Message, !outcome.Ok); | |
| if (outcome.Ok) | |
| { | |
| await LoadAsync(); | |
| } | |
| } | |
| catch (Exception ex) when (ex is not OperationCanceledException) | |
| { | |
| SetStatus("Could not save the override: " + ex.Message, true); | |
| } | |
| } | |
| private async void Remove_Click(object sender, RoutedEventArgs e) | |
| { | |
| try | |
| { | |
| var outcome = await _model.RemoveAsync(OverridesGrid.SelectedItem as ThresholdOverrideViewRow); | |
| SetStatus(outcome.Message, !outcome.Ok); | |
| if (outcome.Ok) | |
| { | |
| await LoadAsync(); | |
| } | |
| } | |
| catch (Exception ex) when (ex is not OperationCanceledException) | |
| { | |
| SetStatus("Could not remove the override: " + ex.Message, true); | |
| } | |
| } |
| }; | ||
| } | ||
|
|
||
| private static int GlobalValue(DarlingAlertReader.AlertSettingsReadRow s, string wire) => wire switch |
There was a problem hiding this comment.
Trap #3684 — "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted. Alias, do not copy." This is a third copy of the wire → global-value mapping, and it has already drifted.
Three mappings now exist for the same fact:
AlertThresholdKeys.GlobalValue(this IAlertEngineSettings, AlertThresholdKey)(PerformanceMonitor.Alerting/AlertThresholdKey.cs:156) — the one the engine reads through, and whose doc says "The engine reads a tunable member only through this switch, so a census test can hold that it never reads one aroundIAlertThresholdOverrides.Resolve".- this
GlobalValue(AlertSettingsReadRow, string wire)switch. DarlingThresholdKeys.Entry.Column(wire →config_alert_settingscolumn), whichThresholdOverrideStore.ListAsyncuses for the grids'global_value.
The drift is already observable: this switch reads the raw row, while BuildAlertSettingsPayload reports the same two pairs clamped, on purpose —
// DarlingMcpAlertTools.cs:468
warning_per_scheduler = RunnableQueueEvaluator.ClampBars(s.RunnableQueueWarningPerScheduler, s.RunnableQueueCriticalPerScheduler).Warning,with the comment "Reported through RunnableQueueEvaluator.ClampBars, the way the engine reads them, so the payload never shows a pair the engine would rewrite" (same at :477 for client_timeouts).
server_view now shows exactly such a pair, in the same response. Reachable with nothing but this PR's own tool, since each bar's bounds are checked independently (1 … int.MaxValue) and there is no cross-field rule on the override path, while update_alert_settings does enforce one (DarlingMcpAlertTools.cs:2347, "The cross-field rule (Critical never below Warning) is checked after the loop … and against the stored row inside the write transaction for a lone bar"):
- global
runnable_queue= (warning 8, critical 16). set_alert_threshold_override server_name=srv-7 key=runnable_queue.warning_per_scheduler value=20— accepted.get_alert_settings server_name=srv-7→runnable_queue.warning_per_schedulereffective_value20,runnable_queue.critical_per_schedulereffective_value16, while the engine evaluates (20, 20) becauseRunnableQueueEvaluator.ClampBarsruns on every evaluation (whichIAlertThresholdOverrides.cs:22-24names as the reason the pairs "need no care here").
So the source/effective_value contract — "the value in effect for this server" — is wrong for those four bars whenever an override inverts the pair. Routing this switch (and Entry.Column) through the one AlertThresholdKeys.GlobalValue definition, and applying the same clamp the payload above applies, removes both the duplicate and the divergence. Nothing currently fails: GetAlertSettings_WithServerName_ShowsTheEffectiveValueAndWhereItComesFrom exercises cpu.threshold_percent and blocking.count_threshold only.
| /// after a read that succeeded, and swallows its own write failures.</para> | ||
| /// </summary> | ||
| /// <summary> | ||
| /// Reloads the threshold overrides (#5916). A failed read keeps the overrides already in force and is counted, as the |
There was a problem hiding this comment.
Non-blocking: the new method was inserted between LoadMuteRulesAsync's doc block and LoadMuteRulesAsync itself, so the mute-rule prose now documents LoadThresholdOverridesAsync.
The member at line 6774 carries two <summary> elements — the four paragraphs ending "…LoadAsync's expiry purge runs only after a read that succeeded, and swallows its own write failures" (which is about MuteRuleService, config_mute_rules and #3013's surface), then the threshold-override one. LoadMuteRulesAsync at line 6797 is left with no doc comment at all.
Move the new <summary> below the mute-rule block's closing </summary> → above your method, and leave the original block attached to LoadMuteRulesAsync.
| case "refused": | ||
| case "invalid": | ||
| return (400, new JsonObject | ||
| { |
There was a problem hiding this comment.
Non-blocking: precondition has two causes and this collapses them to the wrong sentence for one of them.
SetAsync answers precondition when HoldsGrantAsync is false or when the store raises 42501. On a bring-your-own store that has migrated to V191 but has not re-run provision-roles.sql step 3j, the web login genuinely is the admin role and still has no grant on the single new table — and this route tells the operator the opposite: "needs a store login with the admin role … set postgres.webConnectionString to an admin login."
The README section this PR adds promises the other message for exactly that case: "Re-run Darling/tools/provision-roles.sql once after upgrading; until then the tool answers precondition and names the script." ListBodyAsync only reaches DarlingMcpThresholdOverrideTools.PreconditionMessage when the SELECT also 42501s, so an admin login holding SELECT but not INSERT/UPDATE/DELETE gets a clean list, can_edit: false, note: null, and the Admin page's OVERRIDES_NOTE_READ_ONLY strip — three surfaces all naming the wrong remedy.
Carrying answer["message"] through on the precondition arm (or appending PreconditionMessage to RefusedMessage) would name the script where the README says it will.
Smaller notes (none of these are blockers)1. "a fifth tab" is the sixth. The body's 2. 3. A server removed from config but not yet mirrored can FK-fail instead of refusing. 4. 5. |
There was a problem hiding this comment.
Verdict: CHANGES REQUESTED
Two blockers inline: the Level 2 kind rule does not enforce what the README and the tool description promise, and the Viewer window's two async void handlers can end the process. A third finding (a duplicated global-value mapping that has already drifted) plus five smaller notes are posted separately.
Claims
- VERIFIED — loader implements
IAlertThresholdOverridesfrom the table, one in-memory snapshot, no sweep-path I/O, loads at start and each control-plane reload, a failed load keeps the snapshot —DarlingThresholdOverrides.cs:54-97;DarlingWorker.cs:3376,:6703, catch at:6786 - VERIFIED — a row that is not tunable / not an integer / out of bounds / database on a server-only key is skipped and logged, never applied —
Accept:1113-1122;ThresholdOverrideTests.ARowTheGlobalSettingWouldRefuse_IsNotApplied - VERIFIED — a database override wins for that database, a server override covers the server's other databases —
ResolveWire:1140-1142;ThresholdOverrideTests.AnOverride_MovesItsServer_AndLeavesTheOther_AndADatabaseOverrideAppliesToThatDatabaseOnly - VERIFIED —
DarlingThresholdKeysis every #5910 key plus the two PostgreSQL counts, wire-named, withupdate_alert_settings' per-field bounds — 23 entries atDarlingThresholdKeys.cs:35-57, pinned asEnum.GetValues<AlertThresholdKey>().Length + 2; all 23Columnnames exist onconfig_alert_settings - VERIFIED —
BuildAlertEnginepasses the loader (DarlingWorker.cs:7660) and the two PostgreSQL count checks resolve throughResolveWirewithsnapshot.ServerKey(:8517,:8727), which isruntime.ServerId.ToString(InvariantCulture)(:7739) — the same identity asconfig_threshold_overrides.server_id - VERIFIED — Lite passes nothing and is untouched — no
Lite/file in the diff;IAlertThresholdOverrides.cs:17-18 - VERIFIED — in
WriteToolsandAgentActionToolsso both arms offer it (DarlingMcpWritePolicy.cs:79,:118-121,Offers:168-177); writes arm ignoresagent_action, investigations arm requires it, gates to the request's server (InvestigationRequestStore.AgentAction.cs:290-292) and recordsagent:<id>+ the request id (journal:182/:204) - VERIFIED — the four refusal classes and
preconditionnamingprovision-roles.sql—DarlingThresholdKeys.cs:2900-2929,HoldsGrantAsync:213 - VERIFIED —
get_alert_settingsgains an optionalserver_nameandserver_view, and the web/api/readpasses it —DarlingMcpAlertTools.cs:305,:330-342;DarlingWebEndpoints.cs:6644; both existing callers still compile - VERIFIED — the web half: 200/400/415/403-with-
RefusedMessage(ChangeAsync:1390-1405), every body throughDarlingWebStatementSweep.JsonText, read-only seats refused byDarlingWebSeat.IsRequestAllowed:110-123, the form gated on bothcan_editflags,ThresholdOverrideStoreshared by tool/route/Viewer,DarlingThresholdKeyspublic in Alerting (...Viewer.csproj:52), both routes added toReadAdmissionCoverageCensusTests, no new census - UNVERIFIED — every rig and run count (22/25, 11, 18, 11, 3; "3300 run, 0 failed"; "2888 run"; "1140 Failed: 0") and "Build 0 warnings" — needs
dotnet buildand a store withDARLING_TEST_PG - UNVERIFIED — "
McpToolsListBudgetTestspasses", "default 163 to 164, catalog 188 to 189" — the catalog figure is machine-pinned atDarlingMcpWritePolicyTests.cs:242but needs a build; the default-served figure is nowhere in the tree - UNVERIFIED — "
node --checkpasses" onadmin.js— not executed here - UNVERIFIED — "the window was built but not opened by hand" — needs a desktop session, which is where blocker 2 surfaces
- REFUTED — "
admin.jsgains a fifth tab" (## Part 4, repeated in## UI smoke) —TABSatadmin.js:660has six entries; the body's own merge note says "Six tabs" - REFUTED — "a store refusal becomes a sentence" /
ThresholdOverridesWindow.xaml.cs:11"never as an exception to the window" — only for SQLSTATE 42501:ViewerDataService.ThresholdOverrides.cs:32/:49catch that one state, the view-model onlyViewerReadOnlyException, andSave_Click/Remove_Clickare bareasync void, so anything else reachesApp.OnDispatcherUnhandledException:188, which marks only the tray-tooltip raceHandled - REFUTED — the README's "the agent … cannot set a value that would turn the alert off" and the description's "never turns an alert off" —
AgentKindRule:211refusesint.MaxValueand100on two keys only;blocking.count_threshold = 2147483646,pvs.threshold_percent = 100,file_growth.volume_percent = 100,low_disk.threshold_percent = 0all pass and make their alert unreachable
Riskiest lines
Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpThresholdOverrideTools.cs:211— the only thing between a Level 2 agent and a per-server alert switch-off, enumerating two shapes out of many — pinned byThresholdOverrideTests.TheToolCannotLowerAnAgentsBarToNothing_AndLeavesOrdinaryValuesAlonefor the two it catches; the bypasses UNPINNEDDarling/PerformanceMonitor.Darling.Viewer/ThresholdOverridesWindow.xaml.cs:94and:105— unguardedasync void— UNPINNED (ThresholdOverridesViewModelTestsdrives the view-model, never the window)Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs:8517and:8727— the two PostgreSQL bars now come from the snapshot on the sweep path — resolution pinned byThresholdOverrideTests.EachPostgreSqlCountThreshold_FollowsItsServersOverride, the loader byThresholdOverrideLiveTests.WritesArm_SetsUpdatesAndClears_IgnoresAgentAction_AndTheLoaderReadsIt, the call sites UNPINNED
Traps near this diff
- "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — applies —
GlobalValue:308is a second wire→global-value switch besideAlertThresholdKeys.GlobalValue(AlertThresholdKey.cs:156),Entry.Columna third; already drifted, since this one reads the raw row whileDarlingMcpAlertTools.cs:468/:477report the same bars clamped "so the payload never shows a pair the engine would rewrite" - "Lite and Darling ship in parity: a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit" — does not apply — the seam is optional, not defaulted (
IAlertThresholdOverrides.cs:17-18), and the README's section ends "Lite keeps global thresholds only and has no override path" - "A default interface member compiles a parity gap away (#5640)" — does not apply — both new interfaces declare bodiless members (
ThresholdOverridesViewModel.cs:2392-2396,IAlertThresholdOverrides.cs:36) - "
hours_back,limit,top,days_backare refused when out of range, never clamped" — does not apply —ValidateValue:2921-2925refuses andSetAsyncreturns before any statement (:146-147), pinned byThresholdOverrideLiveTests.ARefusedKeyOrValue_LeavesTheTableEmpty - "Truncation inferred from a full page" / "A
total_*assigned from a.Count(#3699)" — do not apply — neither new read is paged:ThresholdOverrideStore.ListSqlandBuildServerViewAsynccarry noLIMIT, and no payload reports a total ortruncated - "A JSON number above 2^53 is rounded by every JavaScript client (#3487)" — does not apply — the only id on the wire is
request_id, a uuid string (DarlingWebThresholdOverrides.cs:1316);server_idisint32 - "Ruling (#5833): tests are load-bearing … never answer a trap with a test that reads source text" — does not apply — the new classes are behaviour tests (recording fake gateway, live store, Node harness over the shipped
admin.js); both census touches are entries in existing rosters
Divergence (blind read vs PR body)
All three in the same direction: the body and the shipped docs state a guarantee one notch stronger than the code enforces.
- Kind rule reach. The body is candid ("deliberately narrow … a product call"), which is why this is divergence and not misstatement — but the README and the tool
Descriptionstate the strong version, and an agent reads the description. - "a store refusal becomes a sentence." True of the view-model, not of the window; the failure mode is not a bad sentence but the Viewer exiting.
- One definition, implemented twice. "The one key table" holds for the write path — one registry, one store, three surfaces, which is the good shape here. The read path did not follow:
server_view's global value is a fresh switch that reads raw where the enclosing payload reads clamped, so one response can carry two numbers for one global threshold.
"No behaviour change to the tool" holds for the store extraction; Entry.Column and the grids' global_value are new behaviour beside an existing tool, described elsewhere in the body, so minor.
Reviewer checklist
- Open the Viewer's Threshold Overrides window against a store you can break mid-save (kill the backend, or force a 1 s command timeout) and see whether Save and Remove show a sentence or end the process
- Run the three new live classes with
DARLING_TEST_PGset, confirm the 3 + 11 counts, thendotnet buildfor "0 warnings" - Confirm
DarlingMcpWritePolicyTestsagrees with "189 tools / 94 Darling-only" andMcpToolsListBudgetTestsstill passes - On a BYO store at V191 without
provision-roles.sqlre-run, compare the sentence the tool, the listnoteand the Admin tab each give - As a Level 2 agent, try
key=blocking.count_threshold value=2147483646and decide whether that outcome is the product intent
Claude posting for Erik Darling. Both blockers, the duplicated mapping, the five notes and the CI reds are answered in the body's Review round 1 section (head 99f61f1); the next round reviews that head.
Claude posting for Erik Darling.
Fixes #5916.
Part of #5632, Phase B wave 3. This lands all four parts of the issue: the loader and engine sites, the tool and view, the README section, and the two grids (part 4, see
## Part 4: the two grids).Why
Some alerts fire often on one server and not on others. Darling can now hold a threshold for one server (or one database) that differs from the global value, and a person or a Level 2 agent can set it. The global row is untouched.
What changes
DarlingThresholdOverrides(new,Darling/PerformanceMonitor.Darling.Service/DarlingThresholdOverrides.cs) implementsIAlertThresholdOverridesfromconfig.config_threshold_overrides. It holds one immutable in-memory snapshot (no I/O on the sweep path), loads at start and on every control-plane reload (the table is on theconfig_versionbeacon), and a failed load keeps the snapshot already in force. A stored row that is not a tunable key, not an integer, out of bounds, or a database name on a server-only key is skipped and logged, never applied. A database override wins for that database; a server override covers the server's other databases.DarlingThresholdKeysis the one key table: every The alert engine takes per-server and per-database threshold overrides #5910 registry key plus the two PostgreSQL count thresholds, named asget_alert_settingsnames them (cpu.threshold_percent,blocking.pg_count_threshold, ...), with the same boundsupdate_alert_settingsand the Viewer enforce. PostgreSQL predictor constants have no entry.DarlingWorker.BuildAlertEnginepasses the loader toAlertEngine. The two PostgreSQL count checks (EvaluatePgDeadlocksAsync,EvaluatePgBlockingAsync) resolve their bar throughResolveWirewith the snapshot's server key. Lite passes nothing and is untouched (no Lite write path).set_alert_threshold_override(server_name,key,value,database_name,clear,set_by,agent_action), inMcp/DarlingMcpThresholdOverrideTools.cs. It is inWriteToolsandAgentActionTools, so both arms offer it. Writes arm: an ordinary write,agent_actionignored, rowset_byis theset_byargument (defaultmcp). Investigations arm:agent_actionrequired, the change goes throughDarlingAgentActionGate(request's server only, journaledaction_calledthenaction_done), the row recordsagent:<agent_id>and the request id, and a kind rule refuses the value that turns the alert off (each key's top of range, or zero for the fourlow_disk.*keys; use a mute). Details under## Review round 1. Refused with{status: refused, reason}: key outside the registry (not_in_registry),self_alerts.*/ enable / cooldown / route keys (not_tunable), out of bounds, a database name on a server-only key. Without the mcp role's grant (bring-your-own store beforeprovision-roles.sqlis re-run) it answers{status: precondition}naming the script.get_alert_settingstakes an optionalserver_nameand addsserver_view: each threshold's global value, effective value and source (globalorserver_override), plus the server's database overrides. The web/api/read/get_alert_settingspasses the sameserver_name.Darling/README.md, section "Per-server threshold overrides".Test plan
ThresholdOverrideTests(no store, 22 cases): refused key classes (outside the registry,self_alerts.*, enable, cooldown, route, empty), out-of-bounds values, database scope only on database-scoped keys, an override moves its server and leaves another alone, a database override applies to that database only, bad stored rows are not applied, the loader moves one server's alert throughAlertEngineand leaves the other's, each PostgreSQL count threshold follows its server's override throughRollingCountAlertGate, the agent kind rule, and the tool offered by the right arms.ThresholdOverrideLiveTests(11 cases, live PostgreSQL rig): writes arm sets, updates and clears and ignores a garbageagent_action, the loader reads what the tool wrote (both PostgreSQL keys and a database row), each refused class leaves the table empty, investigations arm requiresagent_action/ records the agent and request / refuses another server / refuses a disabling value, a role with no grant getspreconditionnamingprovision-roles.sql,get_alert_settingswithserver_nameshows effective value and source. 11 of 11 passed on a throwaway rig; the full run of both classes passed (33 of 33 for*ThresholdOverrideTests*, which also matches the T1 class).*Mcp*,*ToolsList*,*WebEndpoints*,*AdminPage*,*Census*,*Ratchet*classes: 2888 run, 0 failed after the updates (6 pins moved: tool counts 188 to 189 and 93 to 94 Darling-only in the instructions census,get_alert_settingsnow takesserver_name,DarlingMcpServedPolicyadded to the DI-seat pin,set_alert_threshold_overrideadded to the web excluded-tools pin, and the investigations-arm annotation check now allows the Level 2 action tools).McpToolsListBudgetTests(head, guide and parameter-description caps) passes.get_alert_settingsgained one parameter whose description is under the 200-character cap. Served counts: default 163 to 164, catalog 188 to 189 (the instructions census now reads "exposes 189 tools ... remaining 94 are unique to Darling"). I did not take a separate token measurement beyond the passing budget tests.## UI smoke. Not run (the smoke check is red on every PR for an unrelated reason, Darling web UI smoke is red on every PR since 06:00Z on 2026-10-16: the four page baselines no longer match dev #5929; no baseline touched).Part 4: the two grids
Admin login only. Both grids list every override with its server, database, key, value, the global value it replaces, who set it, the request id and when.
ThresholdOverridesWindow(own XAML and code-behind) opened from Settings, Alert rules, "Manage Threshold Overrides...", beside the global thresholds. Its logic isThresholdOverridesViewModel(no WPF): Save checks the server, the key (DarlingThresholdKeys.RefusalFor), a whole-number value and the bounds and database scope (ValidateValue) before it calls the store, so a refused entry never reaches a write; Remove needs a row. A read-only seat (connectAs = "viewer") gets the grid and no form, and a store refusal becomes a sentence (the read-only role's refusal in the view-model, any other store failure in the window's own Save and Remove handlers, after review round 1). The store seam isIThresholdOverrideGateway, whichViewerDataService.ThresholdOverrides.csimplements.GET /api/threshold-overrides(the list, the key table with bounds, the registry's server names,can_edit) andPUT /api/threshold-overrides(set, orclear: true), mapped inDarlingWebEndpoints.cs(WebRouteStatementCensusTestsallows no other mapper file; every body goes through the statement sweep). The write isDarlingMcpThresholdOverrideTools.SetAsyncon the writes arm, so the key registry, bounds, database scope and the statement are the tool's own;set_byis the seat's editor name shaped to the store's rule. Answers: 200okorcleared; 400 for a refused key, value, database, server or body; 415 withoutapplication/json; 403 with the admin-login sentence (DarlingWebThresholdOverrides.RefusedMessage) when the store login holds no write grant, which is theviewerrole on managed and compose stores (the tab is then list-only and its note says so, and the Change column and form are not drawn).admin.jsgains a sixth tab (the page has six), "Threshold Overrides" (buildOverrides, its own pure exportsoverrideRows,validateOverride,overrideOutcome,overrideColumns). The form and the Edit and Remove buttons show only when both the session says the seat can write and the list says the store login holds the write grant.DarlingThresholdKeysmoved from the Service project toPerformanceMonitor.Alerting(public; the Viewer references Alerting but not the Service) and gained each key'sconfig_alert_settingscolumn, used for the global value. The store statements (ReadValueAsync,UpsertAsync,DeleteAsync, the list and the global read) moved intoDarling.StorageasThresholdOverrideStore, which the tool, the web route and the Viewer all call. No behaviour change to the tool.ReadAdmissionCoverageCensusTestsextended with the two routes (GETas a definition read,PUTas a write), as Investigation notes: approve or reject pending agent notes in the viewer (#5903) #5908 and People can give feedback on investigation reports: store, web tab, false-positive counts (#5913) #5926 did. No new census.Part 4 tests
ThresholdOverrideWebLiveTests(3, live PostgreSQL rig, new): as the managedadminrole the list shows the override with its global value (80), servers and keys, and set, change, database row and clear are accepted and stamped; a bad key, self_alerts key, out-of-bounds, database on a server-only key, missing value, non-integer value, missing server, non-object body and wrong content type are refused (400 or 415) with the table unchanged; as the managedviewerrole the list works andcan_editis false, and three changes (set, change, clear) each answer 403 with the admin-login sentence and leave the stored value alone.ThresholdOverridesViewModelTests(18, no store, new): 14 refused entries (no server, key outside the registry, empty, self_alerts, enable, cooldown, below, above and negative bounds, non-numeric, empty and fractional values, database on a server-only key,long_running_job.multiplier21) each say why and make zero store calls; a good entry is written once per call with the login shaped to the store's rule; every key the form offers accepts its own min and max; Remove needs a row; a read-only seat gets a sentence.ThresholdOverrideAdminTabTests(11, Node, new): rows, form refusals, answer wording (a 403 carries the admin-login sentence through), and the Change column only for a seat that can write.shared_preload_libraries = 'timescaledb,pg_stat_statements'):ThresholdOverrideTests25,ThresholdOverrideLiveTests11, the three new classes above, plus*WebEndpoints*,*AdminPage*,*Admin*,*WebRoute*,*Census*,*Ratchet*,*Mcp*,*ToolsList*: 3300 run, 0 failed. Build 0 warnings. The first run caughtWebRouteStatementCensusTests(routes belong inDarlingWebEndpoints.cs, and a handler namedWriteAsyncreads as a direct response write); fixed. Rig stopped bypg_ctl stop.UI smoke
The web Admin page (
#/admin, the admin-servers baseline page) now draws the sixth tab, "Threshold Overrides", in its tab bar, so that page's rendering changes by one tab button. The smoke check was not run and no baseline undertools/ui-smoke/baselineswas touched (the check is red on every PR for #5929); the baseline for admin-servers may need the new tab link when #5929 is fixed.What the coordinator should double-check
## Review round 1). Check the two judgment calls there: the File Growth look-back window has no off end, and the Failed Job look-back's off end is its shortest window (1 minute).DarlingMcpServedPolicyis a new DI service parameter (the policy the host started with) so one tool can tell its two arms apart; Level 2: agents change mute rules and custom alert rules, journaled #5915 will need the same seat and may want to share it.get_alert_settingsuses (for examplecpu.threshold_percent), not the enum names.Review round 1
Claude posting for Erik Darling. Fixes for the review of 97f7990, one push. Targeted run on a throwaway UTC rig (timescaledb, pg_stat_statements):
*ThresholdOverride*,*AlertReadFailure*,*McpPayload*,*McpServiceParameterDiSeat*,*ComposeQueryStoreStampGuardLiveTests: Total: 175, Failed: 0. The wider run (*Mcp*,*Tool*,*Description*,*Instruction*,*Alert*,*Viewer*,*Web*,*Guard*,*Admin*,*Hygiene*,*Slow*, plus*Census*,*Ratchet*,*Pin*earlier) ran 11355 tests with one failure,ComposeQueryStoreStampGuardLiveTests.TheOneDayFleetPanel_..., which passes on its own re-run and touches nothing here. Lite.Tests:CrossAppMcpToolInventoryPinTests,CrossSkuSurfaceSourceTests,*Census*,*Pin*,*AlertReadFailure*,*McpToolsListBudget*: Total: 350, Failed: 0. Both projects build with 0 warnings.origin/devwas merged (clean). No full suite on the laptop; CI decides.DarlingMcpThresholdOverrideTools.csAgentKindRule,DarlingThresholdKeys.csEntry.OffEnd). One per-key definition: eachEntrycarries anOffEnd, and the rule refuses a value AT it for every key. No caps, no bands. Off ends: the top of the range for the counts, minutes, percent and size keys (cpuandtempdb100,pvs.threshold_percentandfile_growth.volume_percent100,long_running_job.multiplier20, the restint.MaxValue); zero for the fourlow_disk.*keys. Two judgment calls, flagged for you:file_growth.lookback_minuteshas NO off end (it is the window the growth rate is averaged over, so no value turns the alert off), andfailed_job.lookback_minuteshas its SHORTEST window (1) as the off end, not 1440, because a longer look-back finds more failed jobs, not fewer. A very high count below the top (blocking.count_threshold = 2147483646) is allowed; it still fires if that count is reached. The toolDescriptionand the README bullet ("Setting one") now say exactly this. Test:ThresholdOverrideLiveTests.AnAgent_IsRefusedEachKeysOffEndValue_AndStillMayUseOneBelowItcalls the tool as a Level 2 agent with a hand-written off-end value for each of the 22 keys that have one and expectsthreshold_disables_alerteach time, with the table still empty, then showsint.MaxValue - 1and the File Growth window at 1440 are accepted. The values are written in the test, not read from the registry, so a wrong registry value does not agree with itself.async void(ThresholdOverridesWindow.xaml.csSave_Click,Remove_Click). Both wrapped withcatch (Exception ex) when (ex is not OperationCanceledException)thenSetStatus(..., true), asLoadAsyncandNotificationRoutesWindowdo. The "never as an exception to the window" sentence is in theThresholdOverridesViewModel.csclass comment (line 59), not the window's; it now says the view-model turns a read-only refusal into a sentence and the window's handlers catch the rest. Not unit tested: the window is WPF code-behind and the lane has no desktop session, so I did not open it. The earlier claim "a store refusal becomes a sentence" is true for the window only now.DarlingMcpThresholdOverrideTools.csGlobalValueAsEngineReadsItandEvaluated;DarlingMcpAlertTools.cspayload).server_viewand the enclosing payload now read the same function, and the pair clamp (RunnableQueueEvaluator.ClampBars,AttentionEvaluator.ClampBars) is applied to the effective value too, so an override that inverts a warning/critical pair shows the pair the engine evaluates. Test:ThresholdOverrideLiveTests.ServerView_ShowsTheWarningAndCriticalPairTheEngineEvaluates_NotTheStoredOne. Not aliased toAlertThresholdKeys.GlobalValueitself: that switch takes anIAlertEngineSettings(54 members) and the tool holds anAlertSettingsReadRow, so an adapter implementing the interface would be a new copy. The row-side switch now exists once and both consumers call it.Entry.Columnstays: it names a SQL column, not a value mapping, and the grid shows the stored global value (valid by construction). Coordinator: say if you want the adapter anyway.## Part 4and## UI smoke. The review adds that the baseline gains two tab links against the pre-People can give feedback on investigation reports: store, web tab, false-positive counts (#5913) #5926 baseline; this PR's own share is one tab button.ThresholdOverrideStore.csListSql,GlobalRowSql):ASadded.DarlingMcpThresholdOverrideTools.cs, theForeignKeyViolationcatch): the tool answers{status: refused, reason: server_not_registered}with a sentence, and the web route turns that into its normal 400 with the message. This catch and the existingpreconditioncatch return a status word, notFormatError, soMcpPayloadContractCensusTests.AdHocErrorReturnshas one deliberate entry for this tool, with the reason in its comment.Replaceskips silently (DarlingThresholdOverrides.cs):Replacenow logs a skipped row through the sameLogSkippedasLoadAsync.previous_overridetruncates (DarlingMcpThresholdOverrideTools.cs): the stored double is reported as it is.DarlingWorker.cs:LoadThresholdOverridesAsyncnow sits afterLoadMuteRulesAsync, so the mute-rule doc block is attached to its own method again.DarlingWebThresholdOverrides.csRefusedMessage: the 403 sentence now also says that an admin login on a store that has not hadprovision-roles.sqlre-run needs the script (the route cannot tell the two causes apart). The Admin tab's note "an override moves a threshold and never turns an alert off" (admin.js) now reads "To silence an alert, mute it.", since a person is not bound by the kind rule.CrossAppMcpToolInventoryPinTests: the new tool's attribute usedName = ToolName, which the parity scan cannot read; it is now the literal"set_alert_threshold_override"(a source fix, not a pin edit).CrossSkuSurfaceSourceTests.TheFleetScopedInventory_MatchesWhatTheCounterActuallyRecords: 14 to 15 (the threshold-override reload records a fleet-scoped read failure), with the name added toAlertReadFailureCounter.FleetScopedReads(shared prose, so both SKUs'get_collection_healthdescriptions now name it) and to the test's asserts. Running the Darling classes that read the same sources turned up two more pins the same change shifted, updated with reasons:AlertReadFailureSurfaceTests(CountedSites43 to 44,LoadThresholdOverridesAsyncadded to the scanned worker members,WorkerCountedSites12 to 13) andMcpPayloadContractCensusTests(the ad-hoc roster entry above).UI smoke (round 1)
tools/ui-smokebaselines were not re-rendered. The admin-servers baseline shifts by one tab button, "Threshold Overrides". Not run.CHANGELOG entry
SECTION: Added
ENTRY: - Per-server alert threshold overrides ([#5933]) - Darling can hold a threshold for one server, or one database on it, that differs from the global value, so an alert that is noisy on one server can be tuned without changing every other server. It covers the integer alert thresholds and the PostgreSQL deadlock and blocking counts; set one with
set_alert_threshold_override, and see the effective value and its source withget_alert_settingsandserver_name. The desktop Viewer (Settings, Alert rules) and the web Admin page (Threshold Overrides tab) show a grid of the overrides with the global value beside each; both refuse a bad key or bound, and the web accepts a change only under an admin store login (a managed or compose store connects the web dashboard read-only, so there it is a list). A Level 2 agent may change an override only for its own request's server and can never set a threshold to the value that turns its alert off (the top of the key's range, or zero for Low Disk; to silence an alert, mute it), and a bring-your-own store needsprovision-roles.sqlre-run once to grant the table.REF: [#5933]: #5933
Dev 67835f0 (#5926, the Report Feedback tab) was merged into this branch. The only conflict was
wwwroot/js/pages/admin.js: the header comment now says "Six tabs" and lists Servers, Notification Routes, Alert Settings, Report Feedback, Pending Notes and Threshold Overrides, and the builder map carries all six (feedback: buildFeedbackfrom dev andoverrides: buildOverridesfrom this PR);node --checkpasses.ReadAdmissionCoverageCensusTests.cspins both PRs' routes, andDarlingWebEndpoints.csandDarling/README.mdauto-merged with both PRs' routes and sections present. Darling.Tests built with 0 warnings. On a throwaway PG rig (stopped afterward), the classes*AdminPage*,*Admin*,*WebEndpoints*,*WebRoute*,ThresholdOverride*,*Census*and*Ratchet*ran with Total: 1140, Failed: 0, Skipped: 0. The Node harness tests for admin.js (ThresholdOverrideAdminTabTests,AdminReportFeedbackBehaviourTestsand the other*Admin*classes) are in that run. No full suite ran locally; CI decides.Merge of dev (#5937)
Claude posting for Erik Darling.
dev (#5937, practice requests) was merged into this branch in one merge commit. Two files conflicted, both resolved by keeping both sides:
McpServiceParameterDiSeatCensusTests.cs: both sides added a served DI seat (this PRDarlingMcpServedPolicy, Practice requests with a score per agent (#5918) #5937InvestigationReadPolicy, added by Practice requests with a score per agent #5918). The expected list now holds both, in sorted order, and the explanatory comment carries both reasons. The pin's rule (every service parameter type is named) is unchanged.wwwroot/js/pages/admin.js:TABSis now Servers, Notification Routes, Alert Settings, Report Feedback, Practice Scores, Pending Notes, Threshold Overrides (this PR's tab after Practice requests with a score per agent (#5918) #5937's);BUILDERShaspracticeandoverrides; both render blocks kept. The header comment now says "Seven tabs" and lists Practice Scores, which Practice requests with a score per agent (#5918) #5937 had left out.Auto-merged files checked:
ReadAdmissionCoverageCensusTests.cshas both routes,DarlingWebEndpoints.cs,DarlingMcpHostService.csandDarling/README.mdhave both sides with no duplicate heading.No other pin moved: the tools/list budget, route census and read-admission census passed unchanged, so no count was edited.
tools/ui-smokebaselines were not re-rendered. The admin-servers baseline now shifts by two tab buttons (Practice Scores and Threshold Overrides over the dev baseline), so it will differ until re-rendered.Test run on the merged tree (throwaway PostgreSQL rig from
pg-runtime.zip, stopped afterwards):dotnet buildof Darling.Tests and of the Darling Viewer: 0 warnings, 0 errors.node --check admin.js: OK.*Census*(includes SlowClassCensusTests, McpServiceParameterDiSeatCensusTests, ReadAdmissionCoverageCensusTests),*Ratchet*,*Pin*, McpToolsListBudgetTests, DarlingMcpWritePolicyTests,*ThresholdOverride*(live and Node): Total 1235, Failed 0, Skipped 2 (the twoDarlingPgRuntimeVersionPinTests_Gatedtests, which needDARLING_TEST_PGRUNTIME).AdminPage*,AdminServerEdit*,AwsRoleEdit*,ThresholdOverrideAdminTabTests) plusInvestigationPractice*andReportFeedback*: Total 437, Failed 0.Merge of dev (#5934)
Claude posting for Erik Darling.
Merged origin/dev (Level 2 mute and custom-rule tools, #5934) into this branch as one merge commit.
Conflicts and how each was resolved:
DarlingMcpWritePolicy.cs: took dev'sAgentActionTools = new HashSet<string>(InvestigationRules.AgentActionToolNames). To keep this PR's tool in the agent arm,set_alert_threshold_overrideis now the eighth name inInvestigationRules.AgentActionToolNames(Common), the one definition the served set and the brief composer'sallowed_actionsboth read, so it is served and named with no second list. The write-tool list (writes arm) already held every tool from both sides.DarlingMcpInvestigationToolsTests.cs: kept dev's behaviour pin of the served writes under the investigations arm and addedset_alert_threshold_overrideto its literal (seven to eight). Dropped this PR's weakerDoesNotContainline, which the literal supersedes.McpServiceParameterDiSeatCensusTests.cs: served-DI-seat list now holdsDarlingAgentActionSeat(Level 2: agents change mute rules and custom alert rules, journaled (#5915) #5934),InvestigationReadPolicy(Practice requests with a score per agent (#5918) #5937) andDarlingMcpServedPolicy(this PR), sorted; the comment carries all three reasons.Darling/README.md: both sections kept (Level 2 section, then Per-server threshold overrides), no duplicated heading.Pins that moved (the union made
set_alert_threshold_overridean agent action tool):AgentLevel2ToolPolicyTests(inAgentLevel2MuteRuleToolTests.cs): served agent-arm writes 7 to 8; the instructions census now expectsthe {writes + admin - 8} ... not servedandthe 8 mute, custom-rule and threshold override tools are served only to an agent with an agent_action at Level 2(was 7, "mute and custom-rule"). The instructions text inDarlingMcpInstructions.cswas reworded to name the threshold override tools, since they are now counted in that number. Two tests renamed Seven to Eight.McpToolsListBudgetTests,DarlingMcpWritePolicyTests,AlertReadFailureSurfaceTests,McpPayloadContractCensusTests, LiteCrossAppMcpToolInventoryPinTestsandCrossSkuSurfaceSourceTestspass unchanged. No new census, roster or source-text test added.Test results (laptop, throwaway PostgreSQL rig stopped afterward):
*AgentLevel2*,*Level2*,*AgentAction*,*Instructions*,*Investigation*,*ThresholdOverride*,*MuteRule*, everyCensus,RatchetandPinclass (includingSlowClassCensusTests, no timeout), plus the five named classes: Total 1800, Failed 0, Skipped 0.CrossAppMcpToolInventoryPinTests,CrossSkuSurfaceSourceTests, everyCensus,RatchetandPinclass: Total 345, Failed 0.