[erikdarlingdata]

Claude posting for Erik Darling.

Fixes #5917.

Part of #5632, Phase B wave 3.

Why

Anything an agent changes must be easy to see and easy to take back. This gives an operator a list of agent changes (calls, refusals, and calls with no known result), a one-click undo that is safe when someone else has since changed the setting, and the switch that turns agent changes off.

What changes

1. AgentChangeUndo (Darling/PerformanceMonitor.Darling.Storage/AgentChangeUndo.cs). The list (calls with their result, refusals, the switch), UndoAsync, SetEnabledAsync, CanDecideAsync, and the alerts muted under a mute change. An undo runs in one transaction: it compares the target's current agent-settable fields with the journal's after image and, only if equal, restores the before image and journals action_undone with actor person:<login>; otherwise it refuses "changed since". Per kind: a mute rule (config_mute_rules), a custom alert rule (custom_alert_rules; a delete is undone with OVERRIDING SYSTEM VALUE, so the rule keeps its Custom:<id>; a name clash refuses name_taken; an update restore bumps version), a threshold override (compared per field). Equality never looks at updated_at, updated_by, version or the config_version beacon. Window 365 days. A call with neither an action_done nor an action_failed row is "result unknown" and has no undo. A second click finds already_undone (an advisory lock on the call, because the journal is append-only and the admin role has no UPDATE on it).
2. Viewer tab. "Agent changes" on the Investigations window, in its own AgentChangesTab.xaml/.cs, AgentChangesViewModel.cs (view rows and the ViewerDataService partial). Calls, refusals, "result unknown", Undo, the switch. The Viewer reads as the store admin; a read-only seat gets the list and no write controls.
3. Web. "Agent changes" tab on the Admin page, in its own wwwroot/js/pages/admin-agent-changes.js (admin.js gets the import, the tab entry and the builder entry). Routes in DarlingWebEndpoints.cs: GET /api/agent-changes, GET /api/agent-changes/{id}/muted-alerts, PUT /api/agent-changes/{id}/undo, PUT /api/agent-changes/switch. Decides only under the admin store login; under the viewer role it answers 403 with the exact text from the issue, and the list body carries can_decide so the page copy says it is list-only. Journal text goes through InvestigationFieldMasking.JournalDetail at the web's level.
4. Tests and guide. AgentChangeUndoLiveTests journals rows by hand in the tools' shapes; AgentChangeToolUndoLiveTests drives the real tools. Guide text in its own README section "Agent Changes: List, Undo and the Switch".

No migration, no Lite write path, nothing under deprecated/, no MCP tool for the switch, CHANGELOG.md not edited.

Image contract

One record per kind describes the journaled image, in Darling/PerformanceMonitor.Darling.Storage/AgentChangeImages.cs. The tool that writes it and the undo that reads it use the same type (#3684):

• MuteRuleImage (AgentChangeImages.cs:44) for kind = "mute". Column fields carry [ImageColumn(type)]; the column name is the [JsonPropertyName]. id, match_summary are wire-only.
• CustomAlertRuleImage (AgentChangeImages.cs:66) for kind = "custom_rule".
• ThresholdOverrideImage (AgentChangeImages.cs:82) for kind = "threshold_override".
• AgentChangeKinds (AgentChangeImages.cs:36) holds the three kind strings; the tools and the undo both use it.
• The undo's column lists are read off the records by reflection (ImageColumns, AgentChangeImages.cs:90), so no second field list exists.

The tools' call sites: DarlingMcpAlertTools.BuildMuteRulePayload (DarlingMcpAlertTools.cs:915) now returns a MuteRuleImage (still the mute_rule wire object the read tools return); DarlingWebEndpoints.BuildFullRuleNode builds a CustomAlertRuleImage (still the single-rule wire shape); DarlingMcpThresholdOverrideTools journals a ThresholdOverrideImage for both its before and after; DarlingMcpAgentActions passes MuteRuleImage.Kind and CustomAlertRuleImage.Kind to the gate.

Seam with #5934 and #5933

What the tools journal (the gate writes action_called.detail = {tool, kind, target:{id, server_id, database}, claim_no, evidence, before} and action_done.detail = {called_id, after}; after is the tool's own answer):

kind	before	after	target.id
mute	the rule as MuteRuleImage, null on create	the tool's answer: {status, mute_rule: MuteRuleImage}; a delete answers {status: "deleted", rule_id}	the rule id; null on create (the id is in the after image)
custom_rule	the rule as CustomAlertRuleImage, null on create	the rule itself on create and update; {status: "deleted", rule_id} on delete	the rule id; null on create
threshold_override	{value} of the previous override, null when there was none	{value}, null when cleared	the threshold key (with server_id and database in the target)

What changed on each side:

• Tools (Level 2: agents change mute rules and custom alert rules, journaled (#5915) #5934, Per-server threshold overrides: loader, PostgreSQL counts, set_alert_threshold_override (#5916) #5933): the images are now the shared records instead of an anonymous object (mute, rule) and an anonymous {value} (override). The JSON they produce is unchanged field for field, so the read tools, the web routes and the tool answers are the same. The override tool already journaled both images; the mute and rule tools already journaled a before image and the rule in the answer, so no tool needed a second journal row or a new image field.
• Undo (this PR): adapted to those shapes. kind is mute, custom_rule or threshold_override (the PR had rule and override). The after image is read out of the tool's answer (mute_rule for a mute, the node itself for a rule, status = deleted means no after image). Created is before null (undo deletes by the id in the after image); deleted is after null (undo inserts the before image back, a rule with its own id and a mute with its own id and created time); updated restores the before image. The undo still runs on its own connection and transaction, journals action_undone as person:<login> with the undone row's id, and refuses with a sentence when the row no longer matches the after image.
• Two defects the seam exposed, fixed here: (1) a mute create that repeated an existing rule answers already_exists and the gate journals it as done; undoing it would have deleted the rule the agent never created. The undo now refuses any call whose answer says unchanged or already_exists ("That call changed nothing"). (2) A tool's .NET time has 100 ns ticks and PostgreSQL keeps microseconds, so a freshly created mute never matched its own after image ("changed since"). Times now compare within one microsecond.
• The undo compares and writes only the fields a journaled image names, so a partial image restores only what it names.

Not blocked: all four paths (mute, custom rule, override created or updated or deleted) are determined by the shapes above. Nothing for the coordinator on that.

Merge of dev

git merge origin/dev into the PR branch: conflicts in DarlingWebEndpoints.cs (route mapping: kept MapThresholdOverrides and MapAgentChanges, both) and admin.js (kept all eight tabs, Agent changes last, in TABS and in BUILDERS; the threshold overrides section and the agent changes context both stay). Auto-merged and checked: InvestigationsWindow.xaml and .xaml.cs (Practice tab and Agent changes tab), Darling/README.md (both sections, no duplicated heading), McpServiceParameterDiSeatCensusTests and ReadAdmissionCoverageCensusTests (every entry from both sides; the four agent-changes web routes are in the read-admission census). node --check passes on admin.js and admin-agent-changes.js.

Review round 1: findings and fixes

1. CI red, AlertNotebookTableCatalogTests.EveryFieldKeyInTheCatalog_IsAFieldTheReadsOwnPayloadBuilds. The test now checks a read's catalog keys against the shared image record's [JsonPropertyName] names by reflection when the read's rows are an image type (get_mute_rules -> MuteRuleImage); every other read is checked as before. Darling/Darling.Tests/AlertNotebookTableCatalogTests.cs; the test passes.
2. muted_alerts was a count off a LIMIT page, plus an N+1 of 1000-row fetches. The list now counts with MutedAlertsCountSql (SELECT count(*), the same predicate as MutedAlertsSql, no LIMIT): one row per mute change instead of up to 1000. The /api/agent-changes/{id}/muted-alerts route still pages and reports alerts_returned. Pinned by AgentChangeUndoLiveTests.AMuteRow_CountsThePopulationOfMutedAlerts_NotThePageTheAlertsRouteReturns (1005 muted alerts seeded: the row says 1005, ReadMutedAlertsAsync(…, 2) returns 2). It fails on the old code (reported 1000).
3. The refusals list admitted Level 1 request refusals. RefusalsSql now also requires detail->>'tool' IS NOT NULL, which a gate refusal always carries. TheList_CarriesCallsAndRefusals_AndRefusesABadLogin now seeds a claim refusal (null detail) and a submit refusal (detail without tool) beside the gate refusal and asserts the list holds only the gate one. It fails on the old code.
4. Doc nit. AgentChangeUndo's XML docs name mute, custom_rule and threshold_override, and the mute field list now has external_source and client_app_pattern.

The CHANGELOG entry still holds: it says nothing about counts or which refusals show.

Test plan

• AgentChangeUndoLiveTests (11, live): unchanged mute restored and journaled by person:erik; changed since refused and left alone; agent-created mute deleted; mute links to alerts muted since; deleted rule returns with its own id, name clash refused; updated rule restored with version moved; override compared per field (restore, refuse, delete); result unknown or failed has no undo; 365-day window; switching off makes the gate refuse actions_off, on restores it.
• AgentChangesWebLiveTests (2, live, real viewer and admin grant replay): viewer role lists, can_decide false, undo and switch answer 403 with the exact sentence and change nothing; admin role undoes, a second click is 409, unknown id 404, switch off, bad body 400; "unknown" in the list body.
• AgentChangesViewerTests (4): the "result unknown" display and when Undo is offered.
• Targeted: AgentChange*, AgentActionGate*, AgentLevel2Store*, InvestigationsWindowStaTests, ViewerGridTimeColumnSortMemberTests, AdminPage*, AdminReportFeedback*, every *Census* and *Ratchet* class, WebExceptionTextCensusTests, DocCommentHygiene: 691 run, then the one failure (WebExceptionTextCensusTests, it flagged result.Message) fixed by renaming the property to Words; rerun green (70). Full suite not run (laptop); CI decides.
• Not run: a browser check of the new web tab (no playwright-cli session was used). The JS passes node --check and the Admin page tests pass.

Census edits are additive (ReadAdmissionCoverageCensusTests four route rows; WebRouteStatementCensusTests one fixed-writer row; ViewerGridTimeColumnSortMemberTests two rows). No new census or source-text test.

• Review round 1 fixes: AlertNotebookTableCatalogTests, every AgentChange* class, every *Census*, *Ratchet* and *Pin* class on the throwaway PostgreSQL rig (DARLING_TEST_PG set): 1168 run, 0 failed, 2 skipped. The two new behaviour pins fail on the pre-fix AgentChangeUndo.cs (2 of 12 in AgentChangeUndoLiveTests) and pass after. Build: 0 warnings, 0 errors.

Test plan (seam lane)

• AgentChangeToolUndoLiveTests (13, live on the throwaway PostgreSQL rig, the real tools through the gate, then the undo, then the table row): mute create then undo deletes it; create repeated (already_exists) then undo refuses and keeps the rule; mute update then undo restores the before row; mute turned off then undo turns it on; mute delete then undo brings it back with its id and created time; a person's edit after the agent then undo refuses changed_since; custom rule create then undo deletes; rule disabled then undo re-enables (version moves forward); rule delete then undo re-inserts it with its id and definition; override set over a previous one then undo restores the previous; override set with none then undo deletes; override cleared then undo puts it back; a person's change then undo refuses.
• Existing live classes moved to the real shapes: AgentChangeUndoLiveTests journals custom_rule and threshold_override kinds and wraps the after image the way the tools do; AgentChangesWebLiveTests journals a mute answer with mute_rule.
• Three source-text pins that read the old anonymous payload (MuteRulesPageTests.EveryRuleFieldThePageReads_IsOneTheToolReturns, MuteRuleExternalSourceSurfaceTests, MuteRuleClientAppTests) now read the field names off MuteRuleImage by reflection; their rules (the page reads only fields the tool returns; the payload carries external_source and client_app_pattern) still hold. No census, roster or source-text test was added.
• Builds: Darling.Tests (which builds the Service and Storage) and the Viewer, 0 Warning(s), 0 Error(s).
• One run, 2863 tests, 0 failed, 2 skipped (live skips): *AgentChange*, *AgentAction*, *AgentLevel2*, *ThresholdOverride*, *Investigation* (the Investigations window and the Viewer tests), *Admin* (the admin page tests and the Node harness), *Census*, *Ratchet*, *Pin*, *CustomAlert*, *MuteRule*, *Guard*, with DARLING_TEST_PG on the rig (port 55936, stopped afterwards). SlowClassCensusTests passed in that run (no ci-select timeout).
• node --check on admin.js and admin-agent-changes.js.
• Not run: the full suite on this laptop (CI decides the rest) and a browser check of the tab.

UI smoke

tools/ui-smoke baselines were not re-rendered. The admin-servers baseline shifts by one more tab button (the Admin page now has eight tabs: Servers, Notification Routes, Alert Settings, Report Feedback, Practice Scores, Pending Notes, Threshold Overrides, Agent changes).

CHANGELOG entry

SECTION: Added
ENTRY: - Agent changes list with one-click undo and an off switch ([#5936]) - The Viewer's Investigations window and the web Admin page have an Agent changes tab: every change an agent made, what it was refused, and calls with no known result shown as "result unknown". Undo puts a mute rule, custom alert rule or threshold override back the way it was before the agent's change, but only if nobody has changed it since (otherwise it says "changed since" and leaves it); a deleted rule comes back with its own Custom id. A switch turns every agent change off at once. The web decides only under the admin store login; on managed and compose stores it lists only.
REF: [#5936]: #5936

What the coordinator should double-check

• The shared image records against the two tools: AgentChangeImages.cs. The tools' JSON is unchanged field for field; the read-tool and web tests that run over the mute payload and the rule node passed.
• "Alerts muted under it" is linked by the rule's server and metric after the change (the alert log has no rule id), so it is the alerts the rule could have muted.
• Merge of dev: see "Merge of dev" above; redo if dev moves.

Left for a successor

Nothing from the four parts. A browser check of the web tab is the one unchecked item.

[claude]

muted_alerts is a count off a page, presented as the population's count.

MutedAlertsAsync(connection, changes[i], 1000, …) runs MutedAlertsSql, which ends LIMIT $4 (line 157), and the count is .Count of the rows it fetched. A mute rule that suppressed 4,000 alerts reports exactly 1000, and both surfaces present that as the fact: the web body puts it on muted_alerts (DarlingWebEndpoints.cs, the list body), the page prints String(c.muted_alerts) + " alerts" (admin-agent-changes.js), and the Viewer prints n + " alerts muted since" (AgentChangesViewModel.cs). There is no *_returned naming and no truncation flag to say the number is a page.

This is the .github/REVIEW-TRAPS.md shape verbatim: "A count taken off the page and named as the population's: non_default_count = rows.Count(!IsDefault) over the rows fetched reported the limit as a fact about the snapshot (#3679). The population's count belongs on the statement above the LIMIT; the page's count is *_returned." The sibling route gets it right — /api/agent-changes/{id}/muted-alerts answers alerts_returned.

Two things ride on the same line:

1. The count should come from a count(*) statement, not from fetching rows. A separate SELECT count(*) FROM config.config_alert_log AS a WHERE a.muted AND a.alert_time >= $1 AND (…) gives the true number and transfers one row.
2. It is an N+1 with a 1000-row body. The loop runs one query per mute-kind Done row, up to AgentChangesLimit = 200 of them, each materialising up to 1,000 config_alert_log rows only to call .Count — so one load of the Agent changes tab can pull 200,000 rows it throws away. The Viewer pays it too: InvestigationsWindow.LoadAllAsync calls AgentChangesView.LoadAsync() on every open of the window.

AMuteRow_LinksToTheAlertsMutedSinceTheChange asserts row.MutedAlerts == 1 against four seeded alerts, so nothing pins the cap.

[claude]

The refusals list is every agent refusal in the journal, not every refused agent change.

event = 'refused' AND actor LIKE 'agent:%' admits far more than the Level 2 gate's refusals. InvestigationRequestStore.RecordRefusalAsync writes the same refused event with the same agent:<id> actor for Level 1 request work:

• InvestigationRequestStore.cs:528-529 — claim refusals, reason = "claim:" + reason (claim:not_found, claim:lease_held, …), detail null.
• InvestigationRequestStore.cs:632 — report-submit refusals, reason = "submit:" + reason.
• InvestigationRequestStore.cs:734-735 — the generic recorder, reason = operation + ":" + reason; its callers in DarlingMcpInvestigationTools.cs:354,361,395,492,505,520 are all claim and submit.

Those rows land in the same list, and both surfaces label them as refused changes: the web tab draws them under el("h3", { text: "Refusals" }) with emptyStrip("No agent change was refused.") (admin-agent-changes.js), and the Viewer grid is AutomationProperties.Name="Refused agent changes" (AgentChangesTab.xaml). On a fleet running Level 1 only — no Level 2 route anywhere, so no agent change is even possible — the Agent changes tab shows an empty changes grid above a Refusals grid full of claim:unsupported_format and submit:too_large rows with a blank Tool column, which reads as "agents keep being refused changes here."

The discriminator is already in the SELECT list: a gate refusal always carries detail.tool (InvestigationRequestStore.AgentAction.cs:346 and DarlingAgentActionGate.cs:69), and the Level 1 recorders pass null detail or a detail with no tool. Requiring it narrows the list to what the tab says it holds:

Suggested change

	internal const string RefusalsSql = @"
	SELECT journal_id, event_time, request_id, server_id, actor, reason, detail->>'tool'
	FROM config.investigation_journal
	WHERE event = 'refused' AND actor LIKE 'agent:%' AND event_time >= $1
	ORDER BY event_time DESC, journal_id DESC
	LIMIT $2";
	internal const string RefusalsSql = @"
	SELECT journal_id, event_time, request_id, server_id, actor, reason, detail->>'tool'
	FROM config.investigation_journal
	WHERE event = 'refused' AND actor LIKE 'agent:%' AND detail->>'tool' IS NOT NULL AND event_time >= $1
	ORDER BY event_time DESC, journal_id DESC
	LIMIT $2";

TheList_CarriesCallsAndRefusals_AndRefusesABadLogin seeds one refused row that does carry detail.tool and asserts Assert.Single(read.Refusals), so it passes either way and pins nothing here.

[claude]

Non-blocking (doc only): this names the pre-rename kind words. The PR body says it plainly — "kind is mute, custom_rule or threshold_override (the PR had rule and override)" — and AgentChangeKinds (AgentChangeImages.cs) is the authority, so a reader who trusts this line will grep for a kind that no tool ever journals.

Suggested change

	/// <summary>The kind word the tool journaled (<c>mute</c>, <c>rule</c> or <c>override</c>).</summary>
	/// <summary>The kind word the tool journaled (<c>mute</c>, <c>custom_rule</c> or <c>threshold_override</c>).</summary>

The same two stale words survive in the class summary below (lines 82-84: "rule: custom_alert_rules" and "override: config_threshold_overrides"), and the <para> there also still says fields enabled, expires_at_utc, … without external_source and client_app_pattern, which MuteRuleImage does carry as [ImageColumn]s.

[claude]

Blind-read notes: the small things, none of them blocking

Posted separately from the two findings so the verdict stays readable. Everything here I'd take or leave.

The window is actually backed by the store. Worth saying out loud because it is the kind of thing that usually isn't: AgentChangeUndo.Window is 365 days and DarlingRetention.InvestigationRetentionDays is also 365 (DarlingRetention.cs:261, the pass at :1033-1034 slices config.investigation_journal on event_time). So "Undo works for 365 days" is a promise the journal can keep, and the expired arm and retention close at the same horizon rather than the UI offering an undo whose journal row was swept last month.

ReadMutedAlertsAsync is missing the guard its sibling has. The list path only computes the count for a mute whose result is Done (AgentChangeUndo.cs:170), but ReadMutedAlertsAsync (:217) gates on row.Kind != AgentChangeKinds.Mute alone. So GET /api/agent-changes/{id}/muted-alerts will list "alerts muted since the change" for a call that action_failed, or one with no result row at all — the same call the list itself labels "result unknown" and refuses to undo because what it changed is unknown. Nothing reaches that route from either UI today (neither admin-agent-changes.js nor AgentChangesViewModel.cs calls it), so it is a loose end rather than a visible defect.

Two dead handoffs in the web tab. agentUndoOutcome computes reread on every arm (admin-agent-changes.js), and neither caller reads it — undo() and flip() both end in an unconditional ctx.repaint(). And agentChangesContext hands over getSession (admin.js:1793) which the new file never touches; it imports apiGet/apiWrite from util.js directly. Either wire reread up (a 403 under the viewer role is the one answer where re-reading the list buys nothing) or drop both.

created_at is not restored for a re-inserted custom rule. MuteRuleImage.CreatedAtUtc carries [ImageColumn(Time)], so an undone mute delete comes back with its original creation time, and AMuteTheAgentDeleted_ComesBackWithItsIdAndItsCreatedTime pins exactly that. CustomAlertRuleImage.CreatedAt carries no [ImageColumn], so InsertAsync lets the column default to now() and the restored rule reads as created at undo time. Defensible — the class doc's reason for excluding created_at is that it is not the agent's to set — but it is an asymmetry between two kinds the README describes in one breath ("A rule the agent deleted comes back with its own Custom:<id>"), and ARuleTheAgentDeleted_ComesBackWithItsIdAndDefinition checks id|name|description|definition only.

The degenerate-image boundary. Equal skips any field the after image does not name (:490-494), which is the documented "a partial image compares only what it names" rule and is right. Its floor is that an after image of {} skips every field and returns true for any current row — so a call journaled with before: null, after: {} would take the after is not null && before is null arm and DELETE the target having compared nothing. Symmetrically, a before of {} builds UPDATE <table> SET WHERE … and throws a syntax error out of UndoAsync rather than refusing. Neither shape is producible by the three shipped tools (the override tool always emits {value}, the mute tools emit the full payload or status: "deleted", the rule tools emit the full node), so this is a note for whoever adds the fourth kind, not a bug here. A before/after.Count == 0 -> not_undoable arm beside the existing before is null && after is null check at :356 would close it cheaply.

ListSql's LATERALs are NULL-unsafe where FirstDetailAsync is not. Both laterals correlate with x.request_id = c.request_id (:126, :132), which matches nothing when the action_called row has a NULL request_id, while the undo's own lookup uses request_id IS NOT DISTINCT FROM $2 (:627). An action_called row with no request would therefore list as "result unknown" forever even after it finished. Unreachable today — the gate requires a parsed agent_action, so every call carries a request — and ix_investigation_journal_request is partial on request_id IS NOT NULL anyway, so the equality is also the indexable form. Flagging the inconsistency, not asking for a change.

ui-smoke: the un-re-rendered baseline will not red the workflow. The PR body flags the shift and leaves the baseline alone, which is the right call: shots.mjs pushes a problems entry only for a missing baseline (:210) or a size change (:219), the viewport is fixed at 1280x800 with no fullPage (:93), and a pixel difference prints a NOTICE and is advisory per #5931 (:133-134, :230). An eighth tab button inside a fixed viewport changes pixels, not dimensions.

[claude]

Verdict: CHANGES REQUESTED

Two findings, both inline on AgentChangeUndo.cs; non-blocking notes are in a separate top-level comment.

Claims

• VERIFIED — undo is one transaction that compares current fields to the after image, restores only if equal, journals action_undone as person:<login>, else refuses "changed since" — AgentChangeUndo.cs:291, :365, :390, :399, :646.
• VERIFIED — a deleted rule is re-inserted with OVERRIDING SYSTEM VALUE, a name clash refuses name_taken, an update restore bumps version — :597, :382-384, :568; legal because custom_alert_rules.id is GENERATED ALWAYS AS IDENTITY (PgMigrations.cs:1933).
• VERIFIED — equality never looks at updated_at, updated_by, version — those CustomAlertRuleImage properties carry no [ImageColumn], and ImageColumns.Of<T>() keeps only attributed ones, so they are absent from the SELECT, the compare and the SET list.
• VERIFIED — "Window 365 days", and the journal can keep it — :93, :317-318; DarlingRetention.cs:261 retains the journal 365 days, slice at :1033-1034.
• VERIFIED — a call with no action_done/action_failed row is "result unknown" with no undo — :202, :48, :325-326.
• VERIFIED — a second click finds already_undone via an advisory lock because the journal is append-only — :299 then :322-323; READ COMMITTED makes the post-lock read see the committed row, as :297-298 claims.
• VERIFIED — "The JSON they produce is unchanged field for field" — MuteRuleImage's fifteen [JsonPropertyName]s and their order match the deleted anonymous initialiser in BuildMuteRulePayload one for one; CustomAlertRuleImage matches the deleted BuildFullRuleNode the same way.
• VERIFIED — the already_exists hole was real and is fixed — refusal at :345-346; DarlingMcpAlertTools.cs:2004-2012 emits the status and DarlingMcpAgentActions.cs:171 accepted it as success.
• VERIFIED — times now compare within one microsecond — :512-515, the same tolerance as the mute tools' own SameExpiry (DarlingMcpAlertTools.cs:1997-2000).
• VERIFIED — a read-only Viewer seat gets the list and no write controls — AgentChangesTab.xaml.cs Initialize collapses UndoPanel and SwitchButton; SwitchCommandAsync re-checks _readOnly.
• VERIFIED — "No migration, no Lite write path" — no PgMigrations.cs hunk, no Lite file among the 24 changed paths; the store is Darling-only (PgMigrations.cs:1424).
• VERIFIED — "Census edits are additive … No new census or source-text test" — four route rows, one FixedWriters row, two grid rows; the three MuteRule* pins swap source Assert.Contains for reflection over MuteRuleImage.
• VERIFIED — the counts the body gives — 11 / 13 / 2 [Fact]s in the live classes, 4 cases in AgentChangesViewerTests, eight entries in TABS.
• UNVERIFIED — the viewer role answering 403 and changing nothing — needs DARLING_TEST_PG and ViewerGrantReplay; which statement first raises 42501 (SELECT … FOR UPDATE at :461, the write, or the journal INSERT) is not readable from the tree.
• UNVERIFIED — "2863 tests, 0 failed, 2 skipped … port 55936" and "691 run" — every live class is Assert.SkipWhen on DARLING_TEST_PG, so the undo, web and tool-undo surfaces are unexercised here.
• UNVERIFIED — "0 Warning(s), 0 Error(s)" and "node --check passes" — no build or node run from a read-only tree.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Storage/AgentChangeUndo.cs:171 — .Count over a LIMIT 1000 page shown as the alerts the rule muted, and an N+1 that can materialise 200,000 discarded rows per load — UNPINNED (AMuteRow_LinksToTheAlertsMutedSinceTheChange asserts == 1 over four seeded alerts).
2. Darling/PerformanceMonitor.Darling.Storage/AgentChangeUndo.cs:143 — event = 'refused' AND actor LIKE 'agent:%' admits Level 1 claim:* / submit:* refusals, surfaced as "Refused agent changes" — UNPINNED (TheList_CarriesCallsAndRefusals_AndRefusesABadLogin seeds a row that does carry detail.tool, so it passes either way).
3. Darling/PerformanceMonitor.Darling.Storage/AgentChangeUndo.cs:365 — if (!Equal(spec, current, after)) is the whole safety property; every write below runs only because it returned true, and it skips any field the after image does not name — pinned by AgentChangeToolUndoLiveTests.AMuteAPersonEditedAfterTheAgent_IsKept_AndTheUndoSaysSo and AnOverrideAPersonChangedAfterTheAgent_IsKept, both live and skipped without a rig.

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." — does not apply; the diff is the fix: the specs read their columns off the image records by reflection (:101-103) and the tools write those same records (DarlingMcpAlertTools.cs:915, BuildFullRuleNode, DarlingMcpThresholdOverrideTools.cs:137,152).
• "A count taken off the page and named as the population's … The population's count belongs on the statement above the LIMIT; the page's count is *_returned (#3679)." — applies, at :171 against the LIMIT $4 at :157. The sibling route gets it right (alerts_returned), which makes the bare muted_alerts the outlier.
• "Truncation inferred from a full page: truncated = rows.Count >= limit … the flag is fetched.Count > limit (#3679)." — does not apply: AgentChangesBody emits {enabled, can_decide, changes, refusals} and no truncation flag.
• "hours_back, limit, top, days_back are refused when out of range, never clamped — a clamp is a silent lie about the window (#3699)." — does not apply: the clamps at :162 and :217 are unreachable from input; both web routes pass constants (AgentChangesLimit = 200 and a literal 200), the Viewer a literal 200, and neither route declares a limit parameter.
• "A JSON number above 2^53 is rounded by every JavaScript client … Emit 64-bit ids as strings (#3487)." — does not apply: called_id is investigation_journal.journal_id, an identity starting at 1 (InvestigationTables.cs:176), and the family's shape is a number — NoteDecisionBody emits ["note_id"] the same way (DarlingWebEndpoints.cs:1051). request_id is correctly a string.
• "Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit (#3594)." — does not apply: this stands on investigation_journal and config_service.agent_actions_enabled, PostgreSQL-only; V191 says so (PgMigrations.cs:1424), and Lite has no agent-action gate.
• "Ruling (#5833): tests are load-bearing. … never answer a trap with a test that reads source text or asserts how code is spelled." — does not apply as a violation: the diff removes source reading (MuteRulesPageTests drops ReadRepoFile(… "DarlingMcpAlertTools.cs") and its BuildMuteRulePayload window), adds no census class, and the census files gain rows only. The two reflection asserts remain spelling pins, but pre-existing ones kept green.

Divergence (blind read vs PR body)

1. The muted-alerts caveat is in the wrong dimension. The coordinator note says the link is "by the rule's server and metric … the alerts the rule could have muted" — honest about which alerts, silent about how many. The number is .Count of a 1000-row page (:171 / :157) printed as a fact ("N alerts", "N alerts muted since"). That caveat leads a reader to check the predicate and stop, which is the blind spot #3679 names.
2. The refusals list reaches wider than the body describes. The body says "refusals" and the README says "the code it was told (for example actions_off)", a gate code. The query has no discriminator beyond event and actor, and RecordRefusalAsync writes the same pair for Level 1 claim and submit refusals (InvestigationRequestStore.cs:528, :632, :734). "Refused agent changes" therefore holds rows that are not agent changes, on a fleet where no agent change is possible.

Elsewhere the body matched the blind read closely, including both seam defects it claims to have fixed — each was a real hole.

Reviewer checklist

• With DARLING_TEST_PG, run AgentChangesWebLiveTests and confirm which statement raises 42501 under the replayed viewer grant — if it is SELECT … FOR UPDATE (:461), the route answers 403 before changed_since could be computed.
• Seed one mute matching more than 1,000 muted config_alert_log rows, open the tab in both surfaces, and check the number shown and the page timing.
• On a Level 1-only store (no route at agent_level = 2, so no action_called row), read the Refusals grid: do claim:* / submit:* rows appear under "Refused agent changes"?
• Browser check of the new Admin tab — the body's one unchecked item. Exercise the Undo column's render under can_decide: true and the 403 strip under the viewer role.
• Open the Viewer's Investigations window against a store with agent changes: confirm the new LoadAsync, now awaited last in LoadAllAsync, does not stall the window.

[claude]

Follow-up review (commits 6c2c6190b..ab9e6ba7d, scope: the three "review round 1" fixes).

1. muted_alerts population count (AgentChangeUndo.cs:163-169,244-251). New MutedAlertsCountSql is a bare count(*) with the same predicate as MutedAlertsSql and no LIMIT, replacing (await MutedAlertsAsync(..., 1000, ...)).Count. This is exactly the repo's documented trap ("A count taken off the page and named as the population's... the population's count belongs on the statement above the LIMIT", #3679) — and it was a live instance of it: AgentChangesViewModel.MutedDisplay and the web's muted_alerts field both surface this number directly to a human as "alerts muted since", so the undercount on >1000 alerts was user-visible, not just internal. Fixed correctly; MutedAlertsCountSql has no LIMIT and the cast via Math.Min(..., int.MaxValue) avoids an OverflowException on a pathological count. Pinned by AMuteRow_CountsThePopulationOfMutedAlerts_NotThePageTheAlertsRouteReturns (1005 seeded rows, old code would report 1000).

2. Refusals list excludes Level-1 refusals (AgentChangeUndo.cs:140-147). Added AND detail->>'tool' IS NOT NULL to RefusalsSql. Verified this is a sound discriminator: every gate-path refusal goes through InvestigationRequestStore.AgentAction.JournalRefusedAsync, which always writes {"tool": tool} (InvestigationRequestStore.AgentAction.cs:347); every Level-1 refusal goes through InvestigationRequestStore.RecordRefusalAsync (claim/submit), whose callers in both Lite/Mcp/McpInvestigationTools.cs and Darling/.../DarlingMcpInvestigationTools.cs pass null or a {fields:...}/{...} detail that never carries a "tool" key. So the new predicate can't accidentally swallow a real gate refusal. Pinned by the extended TheList_CarriesCallsAndRefusals_AndRefusesABadLogin.

3. Catalog test fallback for image-typed reads (AlertNotebookTableCatalogTests.cs:329-342,380). get_mute_rules now builds its payload via MuteRuleImage (prior commit in this PR), so its field names no longer appear as literal key = .../AS key text in the tool file; the test's BuiltKeyPattern text-scan would otherwise false-positive-fail. The added s_imageTypedReads/ImageWireNames reflects over [JsonPropertyName] on the actual record. Checked against read-fields.js's get_mute_rules catalog entry (id, enabled, server_name, metric_name, expires_at_utc, reason) — all six are MuteRuleImage property names, and GetMuteRules (DarlingMcpAlertTools.cs:899) really does serialize list.Select(BuildMuteRulePayload) under the mute_rules key the catalog's rowsKey names. This is a maintenance fix to an existing census (not a new source-reading test), keeping it accurate under the refactor rather than adding a new one under the #5833 ruling.

One minor, non-blocking nit: the PR body's item 4 says the doc fix added "external_source and client_app_pattern" to the mute field list, but client_app_pattern was already present before this follow-up (git show 6c2c6190b:.../AgentChangeUndo.cs); only external_source and the rule→custom_rule/override→threshold_override renames are new here. Doesn't affect any code.

No issues found in the three commits. Lite has no equivalent agent-action-gate journal (action_called/no RunAgentActionAsync), so this remains Darling-only by design, not a parity gap.

[claude]

Verdict: LGTM

Claims

• VERIFIED — "muted_alerts was a count off a LIMIT page, plus an N+1 ... now counts with MutedAlertsCountSql ... no LIMIT" — AgentChangeUndo.cs:163-169 has no LIMIT; old call site used MutedAlertsAsync(..., 1000, ...)).Count (diff hunk at AgentChangeUndo.cs:183).
• VERIFIED — "The refusals list admitted Level 1 request refusals ... RefusalsSql now also requires detail->>'tool' IS NOT NULL, which a gate refusal always carries" — confirmed JournalRefusedAsync (InvestigationRequestStore.AgentAction.cs:347) always sets {"tool": tool}, while every RecordRefusalAsync caller (Lite and Darling Mcp*InvestigationTools.cs) passes null or a {fields:...} detail with no tool key.
• VERIFIED — "The test now checks a read's catalog keys against the shared image record's [JsonPropertyName] names by reflection when the read's rows are an image type (get_mute_rules -> MuteRuleImage)" — AlertNotebookTableCatalogTests.cs:332-342, and read-fields.js's get_mute_rules catalog columns all match MuteRuleImage's JsonPropertyNames.
• VERIFIED — "Pinned by AgentChangeUndoLiveTests.AMuteRow_CountsThePopulationOfMutedAlerts_NotThePageTheAlertsRouteReturns (1005 muted alerts seeded: the row says 1005, ReadMutedAlertsAsync(…, 2) returns 2)" — test present and asserts exactly that (AgentChangeUndoLiveTests.cs:114-129).
• UNVERIFIED — "It fails on the old code (reported 1000)" / "It fails on the old code" (refusals) — requires actually running the tests against the pre-fix SQL; not checkable from a static tree, though the logic (bare LIMIT 1000 fetch, missing detail->>'tool' filter) supports it.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Storage/AgentChangeUndo.cs:145 — the refusal-filter predicate; too broad swallows real gate refusals, too narrow lets Level-1 noise back in — pinned by AgentChangeUndoLiveTests.TheList_CarriesCallsAndRefusals_AndRefusesABadLogin
2. Darling/PerformanceMonitor.Darling.Storage/AgentChangeUndo.cs:244-251 — the count-vs-page semantics for muted_alerts, user-visible via AgentChangesViewModel.MutedDisplay and the web muted_alerts field — pinned by AgentChangeUndoLiveTests.AMuteRow_CountsThePopulationOfMutedAlerts_NotThePageTheAlertsRouteReturns
3. Darling/Darling.Tests/AlertNotebookTableCatalogTests.cs:336-342 — ImageWireNames reflection fallback; a wrong mapping here would silently stop catching a real catalog/payload mismatch for get_mute_rules — pinned by EveryFieldKeyInTheCatalog_IsAFieldTheReadsOwnPayloadBuilds (fails if a catalog key isn't found by any path)

Traps near this diff

• "A count taken off the page and named as the population's... The population's count belongs on the statement above the LIMIT; the page's count is *_returned." — applies, and is the defect this follow-up fixes: AgentChangeUndo.cs:244-251 now counts above the LIMIT and the paged route (DarlingWebEndpoints.cs:1382) is named alerts_returned.
• "Ruling (#5833): tests are load-bearing. ... never add one ... that reads source text or asserts how code is spelled ... A trap is answered with a behaviour test" — does not apply as a new violation: AlertNotebookTableCatalogTests.cs is a pre-existing source-reading census (predates this follow-up), and the new commit only patches it (plus a reflection fallback) to stay accurate after an unrelated refactor moved field names out of literal text — no new census/pin/roster was added.
• "Lite and Darling ship in parity... a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding" — does not apply: Lite has no action_called/RunAgentActionAsync gate journal at all (grep for action_called, AgentActionHooks under Lite/ returns nothing), so AgentChangeUndo is Darling-only by design, not a parity gap.

Divergence (blind read vs PR body)

Minor, non-blocking: the PR body's item 4 ("the mute field list now has external_source and client_app_pattern") implies both were newly added by this doc fix, but client_app_pattern was already present before this follow-up range (confirmed against the pre-image at 6c2c6190b); only external_source and the rule→custom_rule/override→threshold_override kind renames are new here. Doesn't touch any code path.

Reviewer checklist

• Run AgentChangeUndoLiveTests and AlertNotebookTableCatalogTests against a live PostgreSQL rig to confirm the two new pins actually fail on the pre-fix SQL/test code as the PR body claims, and pass after.

Stale: the round-1 findings were fixed in ab9e6ba and round 2 approved that commit.