Repository navigation
Level 2: agents change mute rules and custom alert rules, journaled (#5915) - #5934
Conversation
…5915) The mute and custom alert rule tools join the agent arm through the one gate, with the mute and rule_scope kind rules, attribution on the rows, and Level 2 offered in the brief and the Viewer route editor.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| [McpServerTool(Name = "update_mute_rule"), Description( | ||
| "Edits an existing alert mute rule IN PLACE by its id — changes shared alert configuration. PARTIAL " + | ||
| "Level 2 (investigations arm): needs agent_action; changes only the request's server. Edits an existing alert mute rule IN PLACE by its id — changes shared alert configuration. PARTIAL " + |
There was a problem hiding this comment.
update_mute_rule's served head now breaks the 620-character cap.
McpToolGuideTests.EveryConvertedHead_StaysAtOrUnder620Characters (Darling/Darling.Tests/McpToolGuideTests.cs:51) fails Assert.True for every tool whose description carries <<GUIDE>> and whose served head (pointer included) exceeds 620. This description is converted (marker at DarlingMcpAlertTools.cs:1565), so it is in scope.
Measured from the tree (lines 1559–1564 are the head; each source line is 8 spaces + " + content + " + + + CRLF, so content = line length − 14):
| line | length | content |
|---|---|---|
| 1559 | 198 | 184 |
| 1560 | 115 | 101 |
| 1561 | 113 | 99 |
| 1562 | 111 | 97 |
| 1563 | 110 | 96 |
| 1564 | 74 | 60 |
Head = 637, plus McpToolGuide.GuidePointer (31) = 668 > 620. The 84-character "Level 2 (investigations arm): needs agent_action; changes only the request's server. " prefix is what pushes it over: without it the head was 553 and served 584.
update_custom_alert_rule is over too (682 — separate comment). create_mute_rule lands at 613 and set_mute_rule_enabled at 568, so those pass, but create_mute_rule now has 7 characters of headroom.
The test plan lists McpToolsListBudgetTests (whose cap is the looser 1,000) but not McpToolGuideTests, which is where the 620 target lives — that is why the rig run came back green.
|
|
||
| [McpServerTool(Name = "update_custom_alert_rule"), Description( | ||
| "Partial update of an existing rule: send only the fields to change (name, description, definition, enabled); an " + | ||
| "Level 2 (investigations arm): needs agent_action; changes only the request's server. Partial update of an existing rule: send only the fields to change (name, description, definition, enabled); an " + |
There was a problem hiding this comment.
update_custom_alert_rule's served head now breaks the 620-character cap too.
Same failure as update_mute_rule: this description is converted (<<GUIDE>> marker at line 201), so McpToolGuideTests.EveryConvertedHead_StaysAtOrUnder620Characters applies.
Lines 195–200 are the head; content = line length − 14 (8 spaces + two quotes + + + CRLF):
| line | length | content |
|---|---|---|
| 195 | 211 | 197 |
| 196 | 123 | 109 |
| 197 | 125 | 111 |
| 198 | 128 | 114 |
| 199 | 126 | 112 |
| 200 | 23 | 9 |
Split does description[..at].TrimEnd(), so the trailing space on "its cap. " comes off: head = 651, plus the 31-character guide pointer = 682 > 620. Without the 84-character Level 2 prefix the head was 567 and served 598.
Two tools over the cap, so this is one red test, not two — but the fix has to shorten both heads (or move the Level 2 sentence into the <<GUIDE>> tail, where there is 24,000 characters of room and no cap pressure).
| internal static readonly IReadOnlySet<string> AgentActionTools = new HashSet<string>(StringComparer.Ordinal) | ||
| { | ||
| "create_mute_rule", | ||
| "update_mute_rule", | ||
| "delete_mute_rule", | ||
| "set_mute_rule_enabled", | ||
| "create_custom_alert_rule", | ||
| "update_custom_alert_rule", | ||
| "delete_custom_alert_rule", | ||
| }; |
There was a problem hiding this comment.
Filling this set breaks an existing assertion that no write tool is served under the investigations arm.
Darling/Darling.Tests/DarlingMcpInvestigationToolsTests.cs:120 applies the investigations arm to the real served collection and then asserts, at line 141:
DarlingMcpWritePolicy.Apply(all, new DarlingMcpWritePolicy.Effective(Writes: false, ServerAdmin: false, Investigations: true));
var byName = all.ToDictionary(t => t.ProtocolTool.Name, t => t.ProtocolTool.Annotations!, StringComparer.Ordinal);
...
Assert.DoesNotContain(byName.Keys, DarlingMcpWritePolicy.IsWrite);That held while AgentActionTools was empty: Offers reached if (IsWrite(toolName)) return Writes; and dropped every write. With the seven names here, Offers now short-circuits at if (Investigations && IsAgentAction(toolName, agentActionTools)) return true; (line 177), so byName.Keys contains create_mute_rule and its six siblings — all of them in WriteTools — and Assert.DoesNotContain fails.
The assertion is a secondary line in a test named for annotations, which is probably why it was missed; it is not in the test plan's run list either. It needs to become "no write outside AgentActionTools", in the same shape ToolCensus already uses at DarlingMcpInstructions.cs:92. Worth checking the annotation half of that test at the same time: the seven are destructiveHint: true and readOnlyHint: false under the investigations arm, which is correct but newly reachable on that arm.
| if (before is null) | ||
| return notFound!; | ||
| if (body is null) | ||
| return DarlingMcpAlertTools.Outcome("invalid", "changes_json must be a JSON object of the mute-rule fields to change."); | ||
| /* The reason carries the attribution suffix: what the agent sent, else the stored reason, then the suffix. */ | ||
| string? current = before.Reason; | ||
| if (body.TryGetPropertyValue("reason", out var sent) && sent is JsonValue v && v.TryGetValue(out string? text)) | ||
| current = text; | ||
| body["reason"] = WithSuffix(current, action!); |
There was a problem hiding this comment.
The attribution suffix accumulates: a second agent update stores … [agent:a request g] [agent:a request g].
current falls back to before.Reason when the body sends no reason, and WithSuffix appends unconditionally (reason.Trim() + " " + ReasonSuffix(action), line 112) with no check that the reason already ends with that suffix.
Two ways this fires:
- A second agent change on the same rule that touches another field —
update_mute_rule(rule_id, "{\"expires_at_utc\":\"2026-11-01T00:00:00Z\"}").before.Reasonis already"known batch [agent:agent-a request 7f3…]", so the stored reason becomes"known batch [agent:agent-a request 7f3…] [agent:agent-a request 7f3…]". - The read-modify-write an agent is most likely to do:
get_mute_rulesreturnsreasonwith the suffix, the agent edits one word and passes the whole string back, and the suffix doubles even though it sentreasonexplicitly.
The Viewer's mute list and get_mute_rules both show that field, and InvestigationRules.MaxAgentJournalRows only bounds changes per request — a long-lived mute touched across several requests keeps growing. AnUpdate_OnTheRequestsServer_KeepsTheSuffix_AndARefusedUpdateChangesNothing does exactly one successful update, so it does not see this.
The README's own wording is singular: "The reason ends with [agent:<id> request <uuid>]". Strip a trailing [agent:… request …] (or return reason unchanged when it already ends with this action's suffix) before appending.
| async (_, _, _) => | ||
| { | ||
| var answer = await DarlingMcpAlertTools.CreateMuteRuleOver(new PgMuteRuleStore(postgres), server_name, metric_name, | ||
| database_pattern, query_text_pattern, wait_type_pattern, job_name_pattern, WithSuffix(reason, action!), expires_at, | ||
| server_id, id => DarlingMcpAlertTools.MonitoredServerDisplayNameAsync(postgres, id), client_app_pattern, | ||
| external_source); | ||
| RequireSuccess(answer); | ||
| return answer; | ||
| }, ct); |
There was a problem hiding this comment.
A change that succeeded is journaled action_failed, not left as "result unknown".
All seven write hooks discard the connection and transaction the gate hands them (async (_, _, _) => here, and the same at lines 258, 278, 299, 357, 397, 418) and write on postgres instead, so the change commits on its own connection and is durable the moment the hook returns. Then RunAgentActionAsync inserts action_done and commits — and if either step fails, the catch-all at InvestigationRequestStore.AgentAction.cs:211-222 opens a fresh connection and writes:
await JournalReturningAsync(..., "action_failed", actor, "error",
new JsonObject { ["called_id"] = calledId, ["error"] = ex.Message }...);
await transaction.CommitAsync(CancellationToken.None);
throw;So the journal says the mute rule was not created while the rule is in force — the opposite of a missing result. The PR body's "Decisions" section says the failure mode is "the action_called row stands with no result, which the gate already reads as 'result unknown'"; the tree says action_failed. That is the audit trail the README sells as the Level 2 guarantee ("A change journals action_called … and then action_done … or action_failed"), so a reader cannot tell a refused change from a committed one.
Two smaller consequences of the same discard:
- The body also says "the write runs inside the gate's re-check transaction (row held
FOR SHARE)". It does not — only checks 3, 5 and 6 run there; the write is on a separate connection, so theFOR SHARElock does not cover it and a concurrent submit/release can land between the re-check and the write. AgentToolOutcomeException(not_found,conflict,invalidfromRequireSuccess/CustomRuleAnswer) also travels through that catch-all, which journalsreason: "error"withex.Message— the constant string"agent tool outcome". The actual refusal the agent was handed never reaches the journal.
A narrower version of the same gap: StillOnServerAsync (line 209) re-reads only ServerId, not MetricName, so a stored mute re-pointed at a protected metric between the kind rule and the write is still enabled; the custom-rule paths have no re-read at all before store.UpdateAsync / ResolveAndDeleteRuleAsync.
| /// <summary>The Level 2 tools a brief names (#5915): mutes and custom alert rules, on the request's server only.</summary> | ||
| internal const string Level2Tools = | ||
| "create_mute_rule, update_mute_rule, delete_mute_rule, set_mute_rule_enabled, create_custom_alert_rule, update_custom_alert_rule, delete_custom_alert_rule"; | ||
|
|
There was a problem hiding this comment.
Trap #3684: this is a second copy of AgentActionTools, and nothing pins the two equal.
Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy.
Level2Tools hand-writes the same seven names DarlingMcpWritePolicy.AgentActionTools holds (DarlingMcpWritePolicy.cs:117-126), and AgentLevel2ToolPolicyTests.ActionTools is a third copy. The test roster is at least checked against the real served set (TheInvestigationsArm_ServesExactlyTheSevenActionTools_OfTheWrites), so it fails loudly on drift; this constant is checked by nothing. ALevelTwoBrief_NamesTheTools_… only asserts the string contains create_mute_rule, so six of the seven could go missing silently.
The copy is structural — Common cannot reference the Darling service assembly — but the drift is already scheduled: the class doc on AgentActionTools says "the threshold override tools add to it (#5916)". When #5916 lands, a Level 2 brief will under-name the tools the agent is actually served, and the agent reads allowed_actions as its contract.
The seam that exists for this is InvestigationBriefTools (already a composer parameter, already supplied per SKU as DarlingInvestigationBriefTools.Tools): carry the action-tool names through it from the one Darling set, instead of a literal here. Failing that, a behaviour test in Darling.Tests that the brief's allowed_actions names every member of AgentActionTools would at least fail on the defect.
| internal static async Task<AgentActionRefusal?> RuleScopeRefusalAsync(NpgsqlConnection connection, NpgsqlTransaction transaction, | ||
| string definitionJson, int requestServerId, string which, CancellationToken ct) | ||
| { | ||
| var (definition, _) = CustomAlertRuleDefinition.TryParse(definitionJson); | ||
| string? storageName; | ||
| await using (var command = new NpgsqlCommand("SELECT server_name FROM servers WHERE server_id = $1", connection, transaction)) | ||
| { | ||
| command.Parameters.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.Integer, Value = requestServerId }); | ||
| storageName = await command.ExecuteScalarAsync(ct) as string; | ||
| } | ||
|
|
||
| var inScope = definition is not null && storageName is not null | ||
| && definition.ScopeMode == CustomAlertScopeMode.Servers | ||
| && definition.ScopeServers.Count == 1 | ||
| && string.Equals(definition.ScopeServers[0], storageName, StringComparison.OrdinalIgnoreCase); | ||
| return inScope |
There was a problem hiding this comment.
Non-blocking: a definition that does not parse is refused as rule_scope, not invalid.
TryParse failing leaves definition null, and inScope is then false, so the agent is told "The rule's scope (after) must be exactly the request's server: scope.mode 'servers' with that one server name…" for a body that was malformed JSON or failed validation. The write hook does have the right answer (throw new AgentToolOutcomeException(Outcome("invalid", error …)), line 356) but the kind rule runs first, so it is unreachable on a create. An agent that follows the refusal will keep rewriting a scope that was never the problem.
Separating the two reads cleanly: refuse invalid when definition is null, and rule_scope only when it parsed and the scope is wrong.
Also here: RequestServerTargetAsync (line 309) builds its command with postgres.CreateCommand and then calls ExecuteReaderAsync() / ReadAsync() with no cancellation token and no CommandTimeout, unlike MonitoredServerDisplayNameAsync next door, which sets McpCommandDeadlines.ReadSeconds. The ct the tool was handed is in scope.
| /// <summary> | ||
| /// The mute rule an <c>update_mute_rule</c> body would leave stored: <paramref name="before"/> cloned with the body's | ||
| /// changes applied, through the same parser the tool runs (#5915). Null with an <paramref name="error"/> when the body | ||
| /// is invalid. The Level 2 kind rule reads the after image from this, so it cannot disagree with the write. | ||
| /// </summary> | ||
| internal static MuteRule? MuteRuleAfterUpdate(MuteRule before, JsonObject body, out string? error) |
There was a problem hiding this comment.
Non-blocking: the new method was inserted between BuildMuteRuleUpdate and its doc comment, so it now carries two <summary> elements.
The long block above (lines 1730–1749, "turns a partial update body … into appliers over a MuteRule clone", the explicit-null rule, the four non-editable keys) documented BuildMuteRuleUpdate. MuteRuleAfterUpdate landed between them, so that method now has two <summary> tags in its doc — valid XML, no diagnostic, but the generated docs describe MuteRuleAfterUpdate with BuildMuteRuleUpdate's contract — and BuildMuteRuleUpdate at line 1767 is left undocumented.
Moving the new method (with its own summary) below BuildMuteRuleUpdate restores both.
| if (!writesServed) | ||
| off.Add($"the write tools are off, so the {writeCount + adminCount} that change monitoring configuration are not served " + | ||
| off.Add($"the write tools are off, so the {writeCount + adminCount - agentActionServed} that change monitoring configuration are not served " + | ||
| $"(`{DarlingMcpWritePolicy.WritesSetting}` with a token adds {writeCount}, `{DarlingMcpWritePolicy.ServerAdminSetting}` " + | ||
| $"{adminCount} more)"); | ||
| $"{adminCount} more)" + (agentActionServed > 0 | ||
| ? $"; the {agentActionServed} mute and custom-rule tools are served only to an agent with an `agent_action` at Level 2" | ||
| : string.Empty)); | ||
| else if (!adminServed) |
There was a problem hiding this comment.
Non-blocking: the parenthetical in the same sentence is now off by seven on the investigations arm.
The head count is corrected (writeCount + adminCount - agentActionServed), but the clause that follows still reads "mcp.allowWrites with a token adds {writeCount}" — and writeCount counts all of them, including the seven already served. Turning mcp.allowWrites on from this state adds 15, not 22, and (ruling R2, Resolve at DarlingMcpWritePolicy.cs:197) also removes the two investigation tools, so the sentence overstates the gain and omits the loss. An agent reading it to decide what to ask the operator for gets the wrong number.
writeCount - agentActionServed in the parenthetical would match the head of the sentence.
There was a problem hiding this comment.
Verdict: CHANGES REQUESTED
Two existing tests go red, neither in the test plan's run list; one correctness bug in the attribution suffix; and the "Decisions" account of the journal-failure mode is the opposite of what the tree does. The security shape of the agent arm reads correctly. Details in the inline comments.
Claims
- VERIFIED — The seven tools are in
AgentActionToolsand served under the investigations arm —DarlingMcpWritePolicy.cs:117-126, short-circuited inOffersat:177 - VERIFIED —
other_servercovers a stored rule with noserver_id—DarlingMcpAgentActions.cs:799-802plus gate check 6 atInvestigationRequestStore.AgentAction.cs:290 - VERIFIED —
self_monitor_metricis the family plus every*Metricconst except the nine, by reflection —DarlingMcpAgentActions.cs:658-683. Narrower than claimed: the predicate isf.IsLiteral, so a future alert declaredstatic readonly, or named without theMetricsuffix, is not protected without an edit here - VERIFIED —
rule_scopejudged on before and after by storage name —:387-388, the same name the evaluator uses (CustomAlertRuleDefinition.cs:104-106) - VERIFIED — Brief reads the stamped
autonomy_level, Level 1 byte-for-byte as before —InvestigationBriefComposer.cs:127with the default atInvestigationBriefSources.cs:19; the column issmallint(InvestigationTables.cs:91), soGetInt16(12)atPgInvestigationBriefSourceReader.cs:219matches its ordinal - VERIFIED — Seat and logger are DI services a client cannot supply —
DarlingMcpHostService.cs:1288, pinned atAgentLevel2MuteRuleToolTests.cs:92-93 - VERIFIED — Route editor offers an enabled Act item and tooltip —
NotificationRouteEditDialog.xaml.cs:73,.xaml:121; the save path already admitted 2 (ViewerDataService.NotificationRoutes.cs:270) - VERIFIED — No new tool names; Lite untouched; mute rules have no tag-scope column — no
[McpServerTool(Name = …)]added, no file underLite/, no tag field inPerformanceMonitor.Notifications/MuteRule.cs - VERIFIED — The census no longer reads the seven as "writes on", and the writes-arm tests are unaffected —
DarlingMcpInstructions.cs:92-94,:105-107; the writes-arm tests build fromEffective.Off/ writes-only, whereagentActionServedis 0 - UNVERIFIED — "the catalog is unchanged (182)" — needs runtime reflection over the registered tool classes
- UNVERIFIED — "Ran … Total 410, Failed 0. Build 0 warnings." — needs the rig and a build; the run list omits the two classes holding both failures below
- UNVERIFIED — "
Darling web UI smokeis red on dev for an unrelated reason (#5929)" — needs the CI history - REFUTED — "a failed
action_doneleaves theaction_calledrow with no result … 'result unknown'" — the catch-all atInvestigationRequestStore.AgentAction.cs:211-222journalsaction_failed, denying a change that is live - REFUTED — "the write runs inside the gate's re-check transaction (row held
FOR SHARE)" — all seven hooks discard the supplied connection and transaction (DarlingMcpAgentActions.cs:185, 258, 278, 299, 357, 397, 418) and write on the data source
Riskiest lines
Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpWritePolicy.cs:117— the one line making sevenWriteToolsreachable by a client holding only the investigations token — pinned byTheInvestigationsArm_ServesExactlyTheSevenActionTools_OfTheWrites, and what turnsDarlingMcpInvestigationToolsTests.cs:141redDarling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.cs:1185(and:1425,:1486,:1600,DarlingMcpCustomAlertTools.cs:165,:228,:297) —IsGatedis false for a null seat, so a served path reached without the seat writes ungated; only the source-text DI pin guards it — UNPINNEDDarling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAgentActions.cs:246—WithSuffix(current, action!)appends to whatever is stored, so a second change doubles the suffix — 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 —
InvestigationBriefComposer.Level2Tools(:51-53) copiesAgentActionTools(DarlingMcpWritePolicy.cs:117-126), nothing pins them equal, and that set's doc says #5916 will extend it - "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 — Level 2 is Darling-only by the existing ruling: Lite's store refuses to write it (
Lite/Services/DuckDbNotificationRouteStore.cs:610) and Lite registers none of the seven (Lite/Mcp/McpAlertTools.cs:709serves only theget_mute_rulesread) - "Ruling (#5833): tests are load-bearing … never answer a trap with a test that reads source text" — does not apply —
AgentLevel2ToolLiveTestsdrives the real tool methods against a scratch store and asserts rows and journal events (AgentLevel2MuteRuleToolTests.cs:275-533) - "A default interface member compiles a parity gap away (#3640)" — does not apply — no interface member added; the new optional parameters sit on
statictool methods and fail closed withoutInvestigations(DarlingAgentActionGate.cs:62-64) - "
hours_back,limit,top,days_backare refused when out of range, never clamped" — does not apply — the seven gained one string parameter; no window or page argument changed - "A JSON number above 2^53 is rounded by every JavaScript client (#3487)" — does not apply —
rule_idis echoed as a number atDarlingMcpAgentActions.cs:424, matching the existing shape, over abigserialfrom 1
Divergence (blind read vs PR body)
- "Total 410, Failed 0" is scoped to classes that do not contain the failures.
McpToolGuideTests.EveryConvertedHead_StaysAtOrUnder620Charactersfails:update_mute_rule's served head is 637 + 31 = 668 andupdate_custom_alert_rule's is 651 + 31 = 682, both over 620, because the 84-character Level 2 prefix landed on heads already at 553 and 567. The body listsMcpToolsListBudgetTests, whose cap is the looser 1,000, notMcpToolGuideTests, where the 620 target lives. SeparatelyDarlingMcpInvestigationToolsTests.cs:141asserts exactly the invariant this PR changes, and that class is not in the run list either — "the writes-arm tools/list tests pass unchanged" is true as written, which is why the gap stayed invisible: the test that breaks is an investigations-arm test. - The journal-failure mode is stated backwards, and the write does not run in the gate's transaction at all (see the two REFUTED claims).
- The suffix is described as singular and implemented as cumulative — a second update, or an agent that round-trips the
reasonit read fromget_mute_rules, stores it twice.
Also in the diff but not in the body: Level2Tools in Common is a third copy of the agent-action list, which the body's own #5916 note will break.
Reviewer checklist
- Run
Darling.Testsfiltered toMcpToolGuideTestsandDarlingMcpInvestigationToolsTests— both should be red on this head - Build and confirm "0 warnings" still holds, in particular that the doubled
<summary>atDarlingMcpAlertTools.cs:1749-1754raises no diagnostic - On the rig: two successive successful
update_mute_rulecalls under one claim (the second sending onlyexpires_at_utc), then readconfig_mute_rules.reason— the suffix should appear twice - On the rig: fail the
action_doneinsert and read what the journal says about a mute rule that is present inconfig_mute_rules - Drive a real MCP client at the investigations arm:
tools/listshould advertiseagent_actionbut neitheragentSeatnorlogger, against the live DI container - Click through the route editor: pick Act, save, reopen, confirm it reads back as Act with the limits boxes enabled
The agent writes now run on the gate's connection and transaction, so the re-check, the change and the action_done row are one commit. Heads under the 620 cap, one definition of the action tool names, one attribution suffix per reason, an unparsed definition refused as agent_action_invalid, and the tests that pinned the old shapes updated.
Claude posting for Erik Darling. All eight round-1 findings and the Guard red are answered in the body's Review round 1 section (head 4ea910e); the next round reviews that head.
…e and custom-rule tools)
Claude posting for Erik Darling.
Fixes #5915. Part of #5632 (Phase B, wave 3).
Why
At Level 2 an agent working a live claim can now mute one metric on the request's server and create, change, disable or delete a custom alert rule scoped to that server. Every change goes through the one gate from #5925, carries a reason and evidence, is attributed on the row and is journaled.
What changes
create_mute_rule,update_mute_rule,delete_mute_rule,set_mute_rule_enabledare inDarlingMcpWritePolicy.AgentActionTools. Under the investigations arm they takeagent_actionand callDarlingAgentActionGate.RunAsync(two phases:action_calledcommitted first, then ONE transaction that re-checks, writes on the gate's own connection and insertsaction_done; a failure rolls the change back and is journaledaction_failed). Under the writes arm they are the same tools and ignoreagent_action. The write is the same code the writes arm runs (CreateMuteRuleOver,UpdateMuteRuleCore,DeleteMuteRuleCore,SetMuteRuleEnabledCore).mute_needs_metric(noserver_idor nometric_name, on the after image of create, update and enable);self_monitor_metric(family self-monitor, or any*Metricconstant ofDarlingSelfAlertEvaluatorexcept the five AG metrics, Agent Not Running and the three report digests; read by reflection so a new constant is protected without an edit);other_serverwhen the mute'sserver_id, or the stored rule'sserver_idfor update, delete and enable, is not the request's (a stored rule with noserver_idcounts as another server's). Disable and delete need only the before image on the server.[agent:<id> request <uuid>]. A mute's scope is exactly the request'sserver_id. Mute rules have no tag-scope column, so "tag scope null" holds structurally; the agent arm has no parameter that could set one.create_custom_alert_rule,update_custom_alert_rule,delete_custom_alert_rule.rule_scoperefuses unless the definition's scope ismode: serverswith exactly the request's one server (by storage name), checked on the before image and the after image. Disable and delete are allowed.updated_byisagent:<id>. No view tool is in the agent arm.autonomy_level(newInvestigationBriefRequest.AutonomyLevel, default 1, read by the Postgres reader): a Level 2 brief names the seven tools inallowed_actionsand says "change first, report last: writes refuse after submit"; a Level 1 brief is byte-for-byte as before. The Viewer route editor now has an enabled "Act (Level 2...)" item and tooltip.action_proposalsstays refused (pinned). Web: the route table on the web is read-only (noagent_leveleditor exists there; it already prints "Act (n/h, n/day)"), so there was no web editor control to add.Darling/README.md("Level 2: agents change mute rules and custom alert rules"). Each tool description starts with a short agent-arm note ("Agent arm: needs agent_action, request's server only.").Also changed: the instructions census (
DarlingMcpInstructions.ToolCensus) no longer reads the seven agent tools as "writes on"; under the investigations arm it says the write tools are off and counts the seven as served to an agent.New DI seat
DarlingAgentActionSeat(holds the served policy; registered inConfigureMcpServices, so a client can never supply it). Pin inMcpServiceParameterDiSeatCensusTestsupdated for the new type.tools/list numbers
No new tool names: the catalog is unchanged (182). Under the writes arm the served set is unchanged. Under the investigations arm tools/list gains exactly the seven names, each carrying one new schema parameter,
agent_action(the seat and the logger are DI services, never served to a client). The writes-arm tools/list tests pass unchanged.Test plan
Darling/Darling.Tests/AgentLevel2MuteRuleToolTests.cs:AgentLevel2ToolPolicyTests(served set is exactly the seven writes; update_alert_settings, set_notification_route_enabled, delete_notification_route, mute_analysis_finding, Custom Views, edit_server, tag tools not offered; no served action tool has analerts_enabled,self_alertsor exclusion parameter; every self alert*Metricconstant protected except the nine; brief text at Level 1 and 2; action_proposals refused; census text) andAgentLevel2ToolLiveTests(live Postgres, through the real tool methods: attributed create;mute_needs_metricx3;other_serverfor a create, update, delete and enable;self_monitor_metricon five protected names and on update/enable; allowed uncovered names;rule_scopex5 on create and before/after on update and delete; gate codesagent_action_required,claim_not_live,actions_off,not_level_2through the tool; writes arm unchanged).McpServiceParameterDiSeatCensusTests,DarlingMcpWritePolicyTests,DarlingMcpInstructionsBudgetTests,McpToolsListBudgetTests,ViewerNotificationRouteActTests(updated: the Act item is now enabled),AgentActionGateTests,AgentActionGateLiveTests,AgentLevel2StoreTests,InvestigationBriefComposerTests,DarlingMcpAlertToolsTests,DarlingMcpCustomAlertToolsTests,McpUnknownArgumentGuardTests,McpPayloadContractCensusTests,DocCommentHygiene,ReadAdmissionCoverageCensusTests: Total 410, Failed 0. Build 0 warnings.Lite.Testsnot run: Lite is untouched, and the composer's Level 1 output is unchanged.Darling web UI smokeis red on dev 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 web page changed here, so none of the four baselines is affected.Decisions and things to check
claim_not_live,not_level_2,other_server, with the request row heldFOR SHARE) and theaction_donerow are one transaction. The write hooks now receive the gate's connection and transaction and the stores run on them (PgMuteRuleStoreandCustomAlertRuleStoretake an ambient connection and transaction, through the newPgStoreScope; nothing commits but the gate). So a change is in force if and only if itsaction_donerow exists. The one step that is not inside it is the custom rule delete's best-effort resolution of open incidents, which writes on its own connections before the delete, exactly as the web delete does. A write that fails (a duplicate name, a stale version, a rule re-scoped since it was read) rolls back and the gate journalsaction_failedon a fresh connection; the row now carries the answer the agent was handed, not a constant.rule_scopeis what judges the rule. For mute tools the target is the mute'sserver_id, so a mute on another server answersother_serverfrom the gate itself.mute_needs_metricon a create with noserver_idor no metric is answered before the live checks and journaled as a refusal (once per request, tool, code and hour).update_custom_alert_rulemay setenabledeither way on a rule in scope; no other enable flag is reachable.deprecated/. Sibling PRs (Per-server threshold overrides in Darling #5916, Agent changes list with one-click undo and the switch #5917, People can give feedback on investigation reports: store, web tab, false-positive counts (#5913) #5926) will touch the same README, the DI pin list and the census texts; my additions are self-contained so the merges are additive.CHANGELOG entry
SECTION: Added
ENTRY: - Agents at Level 2 can change mute rules and custom alert rules, journaled ([#5934]) - An alert route set to Act (Level 2) now lets an agent with a live claim mute one metric on the request's server and create, change, disable or delete a custom alert rule scoped to that server. Every change needs a reason and evidence, is stamped with the agent, and is journaled with the before and after rule; the service's own alerts cannot be muted and writes stop once the report is submitted. The Viewer route editor offers the Act level.
REF: [#5934]: #5934
Review round 1
The CI red and the review's findings, each with what changed.
CI red:
DarlingMcpAlertToolsSurfaceAndSqlTests.ParamContract_MatchesContractfor the four mute write tools. The optionalagent_actionparameter is last on each tool; the contract pin inDarlingMcpAlertToolsTests.csnow lists it forcreate_mute_rule,delete_mute_rule,set_mute_rule_enabledandupdate_mute_rule. A behaviour pin, no new census.Three more tests pinned the old shapes and failed locally, and are fixed with the same change:
MuteRulesPageTests.EveryRuleFieldThePageReads_IsOneTheToolReturnsandMuteRuleClientAppTests.TheMcpToolsNameTheClientAppParameter_InCreateAndTheUpdateFieldList(both searched the tool source for a signature text that changed:BuildMuteRulePayloadis nowinternal, and the create tool's list no longer ends atexternal_source), andDarlingMcpInvestigationToolsTests.ResolveForStart_WritesOff_InvestigationsOn_WithAToken_ServesThem(assertedcreate_mute_ruleis not offered under the investigations arm).update_mute_rulehead over 620. Fixed inDarlingMcpAlertTools.cs. The arm note is now "Agent arm: needs agent_action, request's server only." (54 characters, was 84) on all seven tools, and the head drops ", whatever changes" and "(nothing to write)". The head keeps every phraseMcpToolGuideHeadsAlertingTestspins.McpToolGuideTests.EveryConvertedHead_StaysAtOrUnder620Characterspasses.update_custom_alert_rulehead over 620. Fixed inDarlingMcpCustomAlertTools.cs: the same shorter note, "a rule" is "an existing rule" again, and the validation sentence is cut to "An invalid definition changes nothing." (the guide tail already says the rest). The head still namesupdate_custom_view(pinned byMcpToolGuideHeadsCustomAlertTests).DarlingMcpInvestigationToolsTests.Annotations_ClaimAndSubmit_...now pins the exact seven served writes (literal list), thatupdate_alert_settings,mute_analysis_finding, the route tools, the Custom View tools, the server tag tools and the server admin tools are not served, and that the seven are not read-only and are flagged destructive.ResolveForStart_WritesOff_InvestigationsOn_WithAToken_ServesThemgot the same correction (the seven offered, the others not).DarlingMcpAgentActions.WithSuffixstrips any trailing[agent:<id> request <uuid>](this agent's, another's, or several stacked) before it appends the current one. Tests:WithSuffix_PutsExactlyOneAttributionSuffix_OnTheReason(null, blank, already suffixed, another agent's, doubled, a bracket mid-text) and the liveASecondUpdate_NeverStacksTheAttributionSuffix_WhetherTheReasonIsOmittedOrSentBackWhole(an update with no reason, then the read-modify-write that sends the whole suffixed reason back).action_failed/ the write was not in the gate's transaction. The seven write hooks now take the gate's connection and transaction.PgMuteRuleStoreandCustomAlertRuleStoregot an internal constructor over an ambient connection and transaction (newPgStoreScope: owns nothing, commits nothing, rolls nothing back);CustomAlertEvaluator.ResolveAndDeleteRuleAsynchas an overload over a given store. The re-checks moved into the transaction and tightened: the mute recheck locks the stored rowFOR UPDATEand comparesserver_idANDmetric_namewith the before image (StillTheSameMuteAsync, so a mute re-pointed at a protected metric is refused asconflict); the custom update and delete re-read the stored definitionFOR UPDATEand re-run therule_scoperule on it (StillInScopeAsync).AgentToolOutcomeExceptionnow carries the answer as its message, so theaction_failedrow says why. The "journal atomicity" paragraph under Decisions is rewritten. Tests (live):AChangeThatSucceeds_IsJournaledActionDone_WithItsAfterImage_AndNeverActionFailed,AWriteThatFails_RollsBackWithTheGatesTransaction_AndJournalsActionFailed_WithTheRealRefusal,TheMuteRecheck_InTheGatesTransaction_RefusesARuleRePointedSinceItWasRead,TheCustomRuleRecheck_InTheGatesTransaction_RefusesARuleRescopedSinceItWasRead. The review's narrower point (StillOnServerAsyncre-read onlyServerId; the custom paths had no re-read) is covered by the last two.InvestigationRules.AgentActionToolNamesinPerformanceMonitor.Common.DarlingMcpWritePolicy.AgentActionToolsis built from it,InvestigationBriefComposerjoins it intoallowed_actions, andLevel2Toolsis deleted. A tool added to the list (Per-server threshold overrides in Darling #5916) is served and named with no second edit. The tests keep their literal rosters on purpose: they are the behaviour pin of what is served (TheInvestigationsArm_ServesExactlyTheSevenActionTools_OfTheWritesand the new literal in item 3), not a copy of production state.rule_scope. The after image of a create or update whose definition does not parse is now refused asagent_action_invalid, naming thedefinitionfield and the parse error, with no word about scope (RuleScopeRefusalAsync). A stored rule (the before image) that does not parse staysrule_scope, since the agent cannot fix it. Test:ACustomRuleDefinitionThatDoesNotParse_IsRefused_AgentActionInvalid_NamingTheField_NotRuleScope(create and update). Also from the same comment:RequestServerTargetAsyncnow setsMcpCommandDeadlines.ReadSecondsand passes the tool'sct.<summary>elements onMuteRuleAfterUpdate. TheBuildMuteRuleUpdatedoc block moved back aboveBuildMuteRuleUpdate; each method has one summary.DarlingMcpInstructions.ToolCensusnow saysmcp.allowWriteswith a token addswriteCount - agentActionServed, matching the head of the sentence. It does not say the writes arm also drops the two investigation tools (the review's second half): that is a wording change to a budgeted text and it is left as is.README: the Level 2 section states the one-transaction rule, the suffix rule and the
agent_action_invalidrefusal.Test run (rig PostgreSQL on a throwaway cluster in UTC, stopped afterwards):
AgentLevel2*,AgentActionGate*,AgentLevel2StoreTests,DarlingMcpAlertTools*,McpToolGuide*,DarlingMcpInvestigationTools*,*ToolsList*,DarlingMcpCustomAlertTools*,DarlingMcpCustomViewTools*,InvestigationBriefComposer*,DarlingMcpInstructions*,DarlingMcpWritePolicy*,*MuteRule*,*CustomAlert*,*Investigation*,*Viewer*,*Notification*,McpUnknownArgumentGuard*,DocCommentHygiene, every*Census*and*Ratchet*class: Total 5446, Failed 0, Skipped 1. Build 0 warnings. Full suite not run (laptop rule);Lite.Testsnot run (Lite untouched).CI fix (after round 1)
Claude posting for Erik Darling.
Darling PG shard 4 was red on five tests. Two files of pins, one source file of commands.
Darling/Darling.Tests/AgentActionGateTests.cs(AgentActionPolicyTests, three failures). They pinned yesterday's truth: no write tool is offered under the agent arm. This PR offers the Level 2 tools there, so:TodaysActionSet_LeavesEveryWriteOffUnderTheAgentArm(create_mute_rule,update_custom_alert_rule) is nowEveryWrite_IsOffUnderTheAgentArm_UnlessItIsALevel2ActionTool(line 69). Still a Theory over write tools; the expected value isActionToolNames.Contains(tool), whereActionToolNames(line 35) is built fromInvestigationRules.AgentActionToolNames, never a copied list. Every write outside that set (update_alert_settings,edit_server, and now alsodelete_mute_ruleas an in-set row) stays pinned to its set membership, so a write that is not offered under the agent arm stays pinned off.BothArms_AreTodaysBehaviouris nowBothArms_OfferALevel2ActionToolUnderTheAgentArm_AndEveryOtherWriteOnlyUnderTheWritesArm(line 78); thecreate_mute_rulerow's agent-arm expectation istrue.update_alert_settingsstaysfalseon the agent arm.McpReadCommandTimeoutTests(two failures). Both scans found real misses inDarlingMcpAgentActions.cs:EveryMcpAndWebReadCommand_SetsAnExplicitDeadline: theservers.server_namelookup inRuleScopeRefusalAsyncset noCommandTimeout, so it inherited Npgsql's 30 s. A real defect, not a scan quirk.EveryCommandInScope_IsBuiltAgainstTheStore_NotAMonitoredTarget: the three commands on the gate's connection and transaction (StillTheSameMuteAsync,StillInScopeAsync, the server_name lookup) usednew NpgsqlCommand(sql, connection, transaction). The scan only acceptspostgres.CreateCommandor aconnectionopened frompostgres.OpenConnectionAsyncin the same file, and the gate (a different file) opens that connection, so the receiver was unrecognised.PgMuteRuleStore.LockServerAndMetricAsync(PgMuteRuleStore.cs:163),CustomAlertRuleStore.LockDefinitionAsync(CustomAlertRuleStore.cs:281) andCustomAlertRuleStore.StorageServerNameAsync(CustomAlertRuleStore.cs:290). Each sets an explicitCommandTimeout(the stores' own existing deadline constants).DarlingMcpAgentActions.cs(lines 231, 357, 396) now calls them on the sameconnectionandtransaction, so theFOR UPDATErechecks stay in the gate's transaction (round 1's blocker fix is intact) and the answers (not_found, scope refusal) are unchanged.Verified locally (0 warnings):
AgentActionPolicyTests,McpReadCommandTimeoutTests, and every class matching*AgentAction*,*AgentLevel2*,*MuteRule*,*CustomAlertRule*,*InvestigationTool*,*McpServiceParameter*,*Census*(includingSlowClassCensusTests),*Ratchet*,*Pin*, with the throwaway PostgreSQL rig set (the Level 2 live classes ran): Total 1604, Failed 0, Skipped 2. The full suite was not run; CI decides the rest. The rig is stopped.Merge of dev (#5937)
Claude posting for Erik Darling.
Merged origin/dev (practice requests, #5937) into this branch as one merge commit (2ca6a4c). Two files conflicted.
McpServiceParameterDiSeatCensusTests.cs: both sides added a served DI seat. The pinned list now holds both,DarlingAgentActionSeat(this PR) andInvestigationReadPolicy(Practice requests with a score per agent #5918), so the pin went from 12 to 14 distinct types. Both comment sentences are kept.PgInvestigationBriefSourceReader.cs: both sides added a column toRequestSqland both read it at ordinal 12 (this PRautonomy_level, devpractice_of), so a plain keep-both would have read a smallint as a Guid. The select list is now..., r.first_dedup_key, r.autonomy_level, r.practice_of;autonomy_levelstays at ordinal 12 andpractice_ofmoves to ordinal 13. Dev'sEarlierReportsSqlandPriorLinksSqlpractice filters (LivePracticeRequestsSql) are untouched.DarlingMcpHostService.csregisters both seats,README.mdhas no duplicated heading, and the composer and sources keep both theAutonomyLevel(level 2) andPracticebranches.Verified:
dotnet buildofDarling.Tests(builds the service and storage): 0 warnings, 0 errors. Darling.Tests classesMcpServiceParameterDiSeatCensusTests,InvestigationBrief*,PgInvestigationBriefSourceReader*(live),InvestigationPractice*,AgentAction*,AgentLevel2*(live), and every*Census*,*Ratchet*and*Pin*class, on a throwaway PostgreSQL rig (stopped after): Total 1291, Failed 0, Skipped 2 (the twoDarlingPgRuntimeVersionPinTestscases gated onDARLING_TEST_PGRUNTIME). SlowClassCensusTests did not time out. No full suite; CI decides the rest. No new tests added.