[erikdarlingdata]

Claude posting for Erik Darling.

Fixes #6394.

What was missing

An MCP-only admin could not manage webhook signing keys. The Viewer makes, rotates and removes the key of the default generic webhook and of each route's own generic webhook, and update_notification_route told the caller the generic URL stays until the key is "removed in the Viewer", a dead end over MCP.

What changes

Two tools in a new file, Mcp/DarlingMcpAlertTools.SigningKeys.cs, both with an optional route_id (none is the default destination's key, a number is that route's own); generate_webhook_signing_key also takes replace_pending_rotation (see Round 1 review):

• generate_webhook_signing_key: makes the key in the service (WebhookSigning.NewKey), seals it for the destination's saved generic URL and the settings row's generic proxy with the same binding the Viewer uses, and stores it. The service reads the saved URL, proxy and changeover window on its own store connection (StoreOwnerSource), not through a function the mcp role can call. When a key is already stored it rotates: the current key moves to the previous slot and keeps signing for 7 days (WebhookSigning.RotationOverlapDays, the Viewer's window). The answer carries the new key once (shown_once: true), the window end as previous_key_valid_until, and a plain sentence saying the key is shown only this once, the receiver needs it, and the old key keeps working until that time. It never carries a previous or stored key. While the 7 day changeover of an earlier generate is running, a further generate is refused unless the call passes replace_pending_rotation: true, and the key is returned only over TLS or to a loopback caller (see Round 1 review).
• remove_webhook_signing_key: clears the key, the previous key and the window (the Viewer's three fields) and says whether a key was there (removed or unchanged).

Both are admin writes: in DarlingMcpWritePolicy.WriteTools (served only under mcp.allowWrites with a token), in NeverAgentTools, in the web endpoints and plain-text.js action rosters, answered through the shared Outcome envelope (invalid, not_found, refused), and logged with the caller identity the other MCP route writes use (ReadCaller.WriteIdentity), never a key. update_notification_route's text now names remove_webhook_signing_key.

The functions and the grant

Two SECURITY DEFINER functions in config, EXECUTE to mcp only (REVOKE ALL ... FROM PUBLIC), pinned search_path, defined once in DarlingManagedRoles.BuildWebhookSigningKeyFunctionsSql and copied byte for byte (with {config} replaced) into Darling/tools/provision-roles.sql. Grants are applied by provisioning on every start, so there is no migration.

• set_webhook_signing_key(route_id, sealed_key, stored_url, proxy): stores only a key in the sealed shape (sealed:v1:, 16 character key id, base64), so no caller can store a key as text; refuses a missing route (DL404) or a destination with no saved sealed URL (DL400); does the Viewer's rotation UPDATE verbatim, guarded on the URL and proxy still being what the key was sealed for (the route form reads the settings proxy FOR SHARE, as the Viewer's SQL does); returns the window end.
• clear_webhook_signing_key(route_id): refuses a missing route (DL404); clears the three fields; returns whether a key was there.

mcp gets no UPDATE or SELECT on any key column (still 42501).

Docs and counts

README (new bullet, plus a paragraph that the answer carries a credential so the request and answer need TLS or loopback), docs/mcp-server.md (the alert tuning row names the two tools; the count lines are dev's rounded wording, no exact count added), docs/editions.md (no change from dev), the MCP instructions census constant (259 and 147, the real numbers served, which tests pin), plain-text.js, the web endpoints list, and the alert tool surface test roster. The open tool PRs each change the census constant too, so whichever merges later needs its numbers bumped.

Test plan

• McpWebhookSigningKeyLiveTests (5 tests, live, on a throwaway PostgreSQL from pg-runtime.zip, as the provisioned mcp role through the managed batch): a generate on a route and on the default destination stores a sealed key that DarlingWebhookSecrets.Open opens and WebhookSigning.Sign signs with, returns the key once, and a second generate moves the first key to the previous slot with a window about RotationOverlap out and no answer carries the first key; remove clears all three fields and a second remove says unchanged; an unknown route (both tools), a route with no webhook URL, the default destination with no URL, and route_id 0 are refused and nothing is written; mcp gets 42501 on a direct UPDATE of each key column and on SELECT of signing_key; the function refuses a plain (unsealed) key with DL400. All pass.
• Revert-proved: with the route form's rotation UPDATE changed to drop the previous key, GenerateOnARoute_... fails at the previous-key assertion; restored, it passes.
• Targeted run, 0 failures: 1182 tests across McpWebhookSigningKey*, DarlingManagedRoles*, DarlingMcpAlertTools*, McpRoute*, DarlingMcpWritePolicy*, DarlingMcpInstructions*, McpToolGuide*, DarlingMcpNamedToken*, McpAlertAdmin*, ToolUndo*, WebEndpoint* and every *Census* and *Ratchet* class; and 227 (17 skipped, they need other rigs) across the provision-roles drift, collision and ACL classes, ComposeStoreRolesLiveTests, PasswordKeyGrantParityTests, PgClusterObjectCensusTests, WebPlainTextTests, ViewerSigningKeyTests and the rung classes that read provision-roles.sql. The first census run caught a catch block returning Outcome directly; it now goes through a helper like the other route tools.
• Build: 0 errors; the 8 warnings are in files this PR does not touch.
• Full suite and pre-push.sh not run (the brief leaves them to CI); a draft skips the main test job.
• Merged dev at 2a8550f: conflicts in DarlingManagedRoles.cs, provision-roles.sql, DarlingWebEndpoints.cs, DarlingMcpWritePolicy.cs, DarlingMcpInstructions.cs, plain-text.js, Darling/README.md, docs/editions.md and docs/mcp-server.md resolved by keeping both sides (dev's rounded counts taken in the docs; census constant set to 259 and 147); 857 tests pass with 0 skipped on a throwaway PostgreSQL (the live signing key, managed roles, provision-roles, compose, level 2, rung and census classes), and 849 across the instructions, tools-list budget, write policy, web endpoint, plain text, census and ratchet classes pass (9 skipped, they need a rig).

Considered, not added

• A function-level lock held across the read and the write: set_webhook_signing_key already guards the write on the URL and proxy still matching, so a Settings save in between makes it refuse with "changed while the key was made" and write nothing, the Viewer's own behavior.
• A separate "route has no generic webhook" refusal in remove when a key is somehow still stored: remove clears it anyway (a destination with no webhook and no key is refused); the Viewer's clear does the same.
• A rate limit or a confirmation on generate: a repeat call rotates the earlier key out after the window, which the tool text says; no concrete trigger beyond that.

CHANGELOG entry

SECTION: Added
ENTRY:

• MCP can generate and remove webhook signing keys ([Darling MCP: generate and remove webhook signing keys (global and per route) #6394]) - Darling's MCP now has generate_webhook_signing_key and remove_webhook_signing_key, for the default generic webhook or one route's (served only with mcp.allowWrites and a token). A generate makes the key in the service, rotates an existing key into the previous slot for 7 days, and shows the new key once, only over TLS or to a caller on the service's machine; a second generate inside the 7 day changeover needs replace_pending_rotation: true; a remove clears the key, the previous key and the window. update_notification_route now points to the new tool instead of the Viewer.
  REF:
  [Darling MCP: generate and remove webhook signing keys (global and per route) #6394]: MCP tools to generate and remove webhook signing keys, default and per route (#6394) #6655

Round 1 review

Claude posting for Erik Darling.

All three items are in one commit, 3d05445.

1. The tool reads the destination as the service; the read function is gone. Fixed (3d05445). generate_webhook_signing_key now reads generic_url, generic_proxy and the changeover window through the service's own store connection (StoreOwnerSource, as the plan force tools do). The unsealed-URL refusal is unchanged. config.read_webhook_signing_target, its REVOKE and its GRANT are removed from DarlingManagedRoles.cs and provision-roles.sql (kept identical), and both copies run DROP FUNCTION IF EXISTS config.read_webhook_signing_target(integer) so a store that ran an earlier build of this branch loses it. The comments, the README bullet and the PR body lines that named it are corrected; the set function still writes only when the URL and proxy are still those the key was sealed for. Tests (McpWebhookSigningKeyLiveTests, live): TheMcpLoginHasNoFunctionThatReturnsTheDestination_AndAnEarlierBuildsReadFunctionIsDropped (creates the old function and grants it to mcp, runs the function block, the function is gone, mcp gets 42883, generate still works on a route and on the default row) and AnUnsealedSavedUrl_IsRefusedWithTheSaveItFirstSentence_AndWritesNothing (route and default row, plain stored URL, refused with the existing sentence, no key column written).
2. A second generate inside the changeover needs a flag. Fixed (3d05445). In the MCP tool only (the Viewer is unchanged): when the destination's previous_signing_key_until is in the future, generate answers refused with "A key made on is still in its 7-day changeover. Making another now stops the key the receiver holds. Pass replace_pending_rotation: true to do it anyway." unless the call passes replace_pending_rotation: true. The tool description and the answer now say the key is shown once and must be passed on exactly as given. Test: ASecondGenerateInsideTheChangeover_IsRefusedUnlessTheCallerAsksToReplaceIt: the first generate (no key to rotate) and the second (starts the window) both succeed, the third is refused with the keys unchanged, the third with the flag rotates, the default row behaves the same, and a window that has passed is not pending. Revert-proved: with the refusal condition disabled the test fails; restored, it passes. The first-ever generate has no window, so it is never refused.
3. The key is returned only over TLS or to a local caller. Fixed (3d05445). The tool takes IHttpContextAccessor; generate is refused unless Request.IsHttps or Connection.RemoteIpAddress is a loopback address (IPv4-mapped loopback included); no HTTP context (an in-process caller) is allowed. A request with no remote address over plain HTTP is refused. The refusal names mcp.network.tls and writes nothing, and it runs before the store is touched. Remove is unchanged. The README paragraph now says a TLS-ending proxy on the same machine reaches the service from loopback and works. Tests: McpWebhookSigningKeyTransportTests (unit, fake DefaultHttpContext): plain HTTP from 203.0.113.9, 10.1.2.3, 2001:db8::1 and from no address is refused with the store connection never opened; HTTPS from a remote address, and HTTP from 127.0.0.1, ::1 and ::ffff:127.0.0.1, are served; no context is served.

Considered, not added: two generate calls at the same instant could both pass the pending check before either writes. Both would still rotate (the set function writes under row locks), so the cost is the same as one flagged call; no concrete trigger beyond a client issuing two calls at once.

Run on a throwaway PostgreSQL (stopped after): McpWebhookSigningKey* 17 of 17; 925 tests in the signing key, alert tool, managed roles, provision-roles drift and collision, write policy, instructions, payload contract, route, notification channel, *Census*, *Ratchet* and BYO classes, 0 failed; 416 more in the MCP host, tool, guide and doc classes, 0 failed. Build: 0 errors; the 8 warnings are in files this change does not touch. Not run: the full suite and pre-push.sh (CI decides).

Review round 2

Claude posting for Erik Darling. Fixed in bfaf635:

• PlanForceNoWritePathTests.OnlyTheForceToolClass_TakesTheStoreOwnerSource lists DarlingMcpAlertTools.GenerateWebhookSigningKey with the Darling MCP: generate and remove webhook signing keys (global and per route) #6394 reason inline: the new key is sealed for the destination's stored sealed generic URL, which the mcp role cannot read, so the tool reads that URL as the store owner. The round 1 targeted runs did not include that class, so the pin would have failed in CI.
• BuildWebhookSigningKeyFunctionsSql takes the rotation window from WebhookSigning.RotationOverlapDays, not a literal 7, so the store function, the Viewer and the MCP message use one value. The generated SQL is unchanged (7 days), so it still matches provision-roles.sql.

Considered, not added: the reviewer's note that a null IHttpContextAccessor reads as an in-process caller. It needs the MCP host to stop registering the accessor, and McpToolServiceParameterResolutionTests.EveryToolServiceParameter_ResolvesFromTheHostsRealServiceCollection already fails on that.

Also corrected: the summary line that said both tools take one argument; generate_webhook_signing_key also takes replace_pending_rotation.

No local build (laptop at its heat limit); CI decides.

CI fixes

Claude posting for Erik Darling.

CI on head bfaf635 failed in two groups of tests. Both are test-side only; the product is unchanged. Fix commit a3db30d (merged with dev, pushed head f7f5404).

1. In-process MCP host tests (McpWholeNumberArgumentTests, and McpUnknownArgumentGuardTests and McpArgumentTypeTests, which build their host through the same StartHostAsync). generate_webhook_signing_key takes IHttpContextAccessor? httpContextAccessor = null, and the host's inert-instance map had no entry for that interface, so McpInProcessHost.StartAsync threw "No inert instance for the abstract service type". Added IHttpContextAccessor -> new HttpContextAccessor() (its HttpContext is null) to InertInstanceFor in McpWholeNumberArgumentTests.cs, the one map all three classes use. The throw in McpInProcessHost is kept.
2. McpReadCommandTimeoutTests.EveryCommandInScope_IsBuiltAgainstTheStore_NotAMonitoredTarget. The tool's owner.CreateCommand( (DarlingMcpAlertTools.SigningKeys.cs:134) was an unrecognised receiver. owner there is ownerSource?.Read(), the store owner pool from StoreOwnerSource; it is the only owner.CreateCommand in the service project and in the Compose folder the test also scans. Added owner to the word-boundaried receiver alternation in s_storeReceiver and one sentence to its summary. The class prose said "all 126" store commands twice; reworded to "each"/"every" so there is no exact count.

Run locally after the merge: *McpUnknownArgumentGuard*, *McpArgumentType*, *McpWholeNumberArgument*, *McpReadCommandTimeout*, *SigningKey*, *Census*, *Ratchet*: Total 671, Failed 0, Skipped 14. An earlier run with *McpInProcess*, *PlanForceNoWritePath* too: one failure, StatementCollectionCensusTests.ABlockingRingBufferOfAboutFourMegabytes_IsJudgedInTimeLinearInItsSize, a timing test under a CPU-capped laptop; it passes alone (47/47). The three failed CI legs (3, 6, 10 of 14) showed only the two groups above. No pre-push gate or full suite was run, per the brief.

The Darling.Tests build reports one CA1068 warning in PerformanceMonitor.Notifications/WebhookAlertService.cs (TrySendGenericAlertAsync). That file is byte-identical to dev (from #6720) and this PR does not touch it.

Dev merge

Merged origin/dev 9fab3c9 into this branch (head 6b5f572). Only DarlingMcpInstructions.cs conflicted: CatalogCensus set to 262 tools, 112 shared with Lite, 150 unique to Darling (merge base 257/145, this PR +2, dev +3). Tests with a local rig, 0 skipped: instructions, write policy, named token, tools-list budget, payload contract, plain text, filter reads, custom views, managed roles, provision roles, Census, Ratchet, McpWholeNumberArgumentTests, McpReadCommandTimeoutTests: Total 835, Failed 0, Skipped 0.
Merged origin/dev cb20aea again (head fc1e6df). Only DarlingMcpInstructions.cs conflicted: CatalogCensus set to 263 tools, 112 shared with Lite, 151 unique to Darling (dev 261/112/149, this PR +2). Tests with a local rig, 0 skipped: the same classes as above: Total 827, Failed 0 on re-run (one timing-linearity test, StatementCollectionCensusTests, failed once under CPU load and passed alone, 47 of 47).

[claude]

This is the seventh [McpServerTool] method to take a StoreOwnerSource, and the pin that rosters them is not updated.

Darling/Darling.Tests/PlanForceNoWritePathTests.cs:166-187 asserts the exact set of [McpServerTool]-attributed methods anywhere in the PerformanceMonitor.Darling.Service assembly (typeof(PlanForceBot).Assembly, PlanForceBot.cs:50) whose parameters include typeof(StoreOwnerSource):

DarlingMcpPlanForceTools.EvictCachedPlan / ForceQueryStorePlan / KillSession / UnforceQueryStorePlan
DarlingMcpPlanUndoTools.UndoPlanChange          (#6230)
DarlingMcpServerActionTools.SetServerAction     (#6269)

DarlingMcpAlertTools is in that same assembly, GenerateWebhookSigningKey carries the attribute, and StoreOwnerSource? is typeof(StoreOwnerSource) for ParameterType, so takers now contains DarlingMcpAlertTools.GenerateWebhookSigningKey and the Assert.Equal fails. The PR's targeted-run globs (McpWebhookSigningKey*, DarlingManagedRoles*, DarlingMcpAlertTools*, McpRoute*, DarlingMcpWritePolicy*, DarlingMcpInstructions*, McpToolGuide*, DarlingMcpNamedToken*, McpAlertAdmin*, ToolUndo*, WebEndpoint*, *Census*, *Ratchet*) match no class named PlanForceNoWritePathTests, which is why a green targeted run and a red CI are both consistent with the body.

It is more than a stale list. That pin is the written record of a #5632-step-7 decision — "no other MCP tool method takes it, so a tool method cannot read or write the store as the owner" — and this change deliberately widens it so generate_webhook_signing_key can read a column mcp is denied (the sealed generic_url). The two earlier widenings each added their line with the reason inline (/* #6230: ... */, /* #6269: ... */). Add the entry the same way, naming #6394 and why the owner seat is needed here (the key must be sealed for the stored sealed URL text, which mcp has no SELECT on), so the next reader sees a decision rather than a list that drifted.

[claude]

Non-blocking, and I am not asking for a change here — a note on which way this arm fails.

context is null means two different things at this seat. One is the documented one: an in-process caller with no HTTP request. The other is "the IHttpContextAccessor service parameter was not resolved", and that one reads as serve the key. GenerateWebhookSigningKey gets the accessor from DI (DarlingMcpHostService.ConfigureMcpServices → services.AddHttpContextAccessor(), DarlingMcpHostService.cs:1296), so if that seat ever moves behind a condition the SDK serves the parameter as a client argument instead, it binds to null, and mcp.network.tls's guard disappears silently rather than refusing. That is the shape #4214's Low 4 was filed about (PostgresConfig), and McpToolServiceParameterResolutionTests.EveryToolServiceParameter_ResolvesFromTheHostsRealServiceCollection is the thing standing between that and production — worth knowing that this guard, unlike the others, is load-bearing for a credential on the wire.

A shape that fails closed instead: take the accessor (not HttpContext) through to the refusal and treat "no accessor" as "unknown transport, refuse", keeping "accessor present, HttpContext null" as the in-process case. The transport tests would need one more case and nothing else changes.

[claude]

Non-blocking nit. The window is a literal 7 here (both branches, and both copies of the file), while the Viewer binds the same window as a parameter from the constant — ViewerDataService.SigningKeys.cs:41 / :169 pass WebhookSigning.RotationOverlapDays as $4 — and this tool's own refusal sentence interpolates that constant (DarlingMcpAlertTools.SigningKeys.cs:163-164). So a change to RotationOverlapDays moves the Viewer's writes and the MCP message and leaves the store function writing 7 days.

This builder is already an interpolated string ({config}), so make_interval(days => {WebhookSigning.RotationOverlapDays}) would make the one definition the source for both — and provision-roles.sql keeps its literal, since it is generated text.

What makes it a nit rather than the #3684 "two copies pinned equal" trap: the drift is caught, by behaviour rather than by a pin — McpWebhookSigningKeyLiveTests.GenerateOnTheDefaultDestination_... asserts PreviousUntilUtc > now + (RotationOverlapDays - 1) and the route test asserts until within 5 minutes of now + RotationOverlap, so a constant raised past 7 fails both. Both are gated on DARLING_TEST_PG, so whether that is caught in CI depends on the rig.

[claude]

Verdict: CHANGES REQUESTED

Claims

• VERIFIED — the two functions are SECURITY DEFINER, EXECUTE to mcp only, pinned search_path, and copied byte for byte into the BYO script — DarlingManagedRoles.cs:3165-3256 vs provision-roles.sql:1901-1991 match modulo {config}→config; grants at :1819-1820 and :1992-1993.
• VERIFIED — "the route form reads the settings proxy FOR SHARE, as the Viewer's SQL does" — DarlingManagedRoles.cs:3211 is the same predicate as ViewerDataService.SigningKeys.cs:56.
• VERIFIED — "the same binding the Viewer uses" — SigningKeys.cs:169 matches ViewerWebhookSigningKeys.cs:117-118 argument for argument, and that is what the sender opens with (DarlingWebhookSecrets.cs:112, :222).
• VERIFIED — the roster additions — DarlingMcpWritePolicy.cs:95-96 and :185-186, DarlingWebEndpoints.cs:173-174 (excluded from the web mirror, so no web route can return the key), plain-text.js:69/:72, which WebPlainTextTests.cs:391-392 holds to the registry both ways.
• VERIFIED — "logged ... never a key" — SigningKeys.cs:916-918 logs tool, identity and destination only; McpToolLatencyFilter.cs:466-473 keeps only argument names for a write and never the result.
• VERIFIED — the TLS/loopback refusal runs before the store is touched — SigningKeys.cs:106 precedes the owner read at :134 and the write at :170.
• VERIFIED — "mcp gets no UPDATE or SELECT on any key column" — both key columns are SecretColumns on both tables (DarlingManagedRoles.cs:256, :277), and the only new privileges in the diff are the two EXECUTE grants.
• VERIFIED — "update_notification_route's text now names remove_webhook_signing_key" — RouteAdmin.cs:193.
• VERIFIED — the census constant is pinned by a derived test, not a hand-kept number — DarlingMcpInstructionsBudgetTests.ServedCensus_EverythingOn_IsTheCatalogCensus; 259 − 112 = 147 agrees with DarlingMcpInstructions.cs:928-929.
• UNVERIFIED — "Build: 0 errors" and the targeted runs (1182 / 925 / 416 tests, 0 failures) — needs a build and a run; see Divergence for the class those globs miss.
• UNVERIFIED — every McpWebhookSigningKeyLiveTests result — the class is Assert.SkipWhen-gated on DARLING_TEST_PG and provisions cluster-wide roles, so it needs a throwaway PostgreSQL.
• UNVERIFIED — the two revert-proofs (rotation UPDATE, changeover refusal) — needs the mutation runs on that rig.
• UNVERIFIED — that the served count really is 259 (overlap 112) — derived at runtime from the registered tools.
• REFUTED — "Two tools ... both with one optional argument route_id" — generate_webhook_signing_key takes two client arguments, route_id and replace_pending_rotation (SigningKeys.cs:58-59). A stale summary line; the Round 1 section documents the second argument in full.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.SigningKeys.cs:169 — the four-field seal binding. Wrong in any field and the key stores, shows, and then will not open at send time, where DarlingWebhookSecrets.cs:113-119 turns that destination's generic channel OFF rather than sending unsigned: alerts stop silently. — covered by McpWebhookSigningKeyLiveTests.GenerateOnARoute_StoresASealedKeyTheSenderOpensAndSignsWith_AndShowsItOnce, which opens the stored key through the sender's own path and signs with it (needs DARLING_TEST_PG)
2. Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.SigningKeys.cs:85 — context is null || context.Request.IsHttps returns "serve", so this line alone decides whether a fresh credential crosses a plain-HTTP segment. — covered by McpWebhookSigningKeyTransportTests.PlainHttpFromANonLoopbackAddress_IsRefused_AndTheStoreIsNeverTouched and TlsOrLoopback_IsServed_PastTheTransportCheck for the HTTP cases; the "accessor absent from DI" case is not covered — inline note left
3. Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpAlertTools.SigningKeys.cs:60 — the StoreOwnerSource parameter hands this tool the owner connection, which reads every column mcp is denied. — UNCOVERED: the only thing watching that seat is a roster pin (PlanForceNoWritePathTests.OnlyTheForceToolClass_TakesTheStoreOwnerSource), which is not coverage and currently fails

Traps near this diff

• "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" — does not apply — Lite/Mcp/McpAlertTools.cs:451, :613, :696 are the only notification tool names Lite registers (read, set-enabled, delete); it has no route/channel authoring writes to keep in step, the asymmetry #6392 established, and Lite's own keys are made Viewer-side in Lite/Services/LiteWebhookSigningKeys.cs:97.
• "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — applies, non-blocking — the function text is duplicated into provision-roles.sql with nothing holding the two bodies together (ProvisionRolesAclDriftTests compares only table grants, default privileges and role settings), and the 7-day window is a literal at DarlingManagedRoles.cs:3202/:3209 beside WebhookSigning.RotationOverlapDays at SigningKeys.cs:163-164. A nit because the drift fails a behaviour assertion rather than a spelling pin — but only on a DARLING_TEST_PG rig. Inline note on the line.
• "Ruling (#5833): ... never answer a trap with a test that reads source text or asserts how code is spelled (... a trait roster ...)" — applies to the fix, not to the diff — the repair is the missing line and its reason in an existing pin, not a new roster, and this review asks for no spelling test.
• "hours_back, limit, top, days_back are refused when out of range, never clamped" — does not apply — the only numeric input is route_id, refused below 1 at SigningKeys.cs:907-908, never clamped.
• "Truncation inferred from a full page" / "a total_* assigned from a .Count" — do not apply — neither answer is a page or carries a count (SigningKeys.cs:829-842, :876-892).
• "A JSON number above 2^53 is rounded by every JavaScript client" — does not apply — the only numbers are route_id (an int) and an ISO timestamp.

Divergence (blind read vs PR body)

One divergence, and it is the finding.

The body presents the StoreOwnerSource seat as a plain mechanism choice, and the test plan cannot have covered the guard that seat has. The body says generate "reads the saved URL, proxy and changeover window on its own store connection (StoreOwnerSource)". In this repository that seat is pinned: PlanForceNoWritePathTests.OnlyTheForceToolClass_TakesTheStoreOwnerSource (PlanForceNoWritePathTests.cs:166-187) asserts the exact set of [McpServerTool] methods in the PerformanceMonitor.Darling.Service assembly that take one. DarlingMcpAlertTools is in that assembly and GenerateWebhookSigningKey is a seventh taker, so the Assert.Equal fails — and every glob named in the test plan misses that class by name, which is why "0 failures" and a red CI are both true of this branch. The substance, not just the line: the pin records the #5632-step-7 decision that the owner data source reaches one tool class only, and #6230 and #6269 each widened it with the reason written inline. #6394 widens it again and should do the same, naming why the owner seat is needed here (the key is sealed for the stored sealed generic_url, which mcp has no SELECT on). Detail in the inline comment.

Two smaller disagreements, neither blocking: "both with one optional argument route_id" is refuted by replace_pending_rotation, and "7 days (WebhookSigning.RotationOverlapDays, the Viewer's window)" is true of the value but not of the mechanism — the store function writes a literal 7 while the Viewer binds the constant.

Everything else matched the tree on first reading, the three Round 1 items included: the read function is gone from both copies and both run the DROP FUNCTION IF EXISTS, the changeover refusal is MCP-only and leaves the Viewer alone, and the transport check precedes any store access.

Reviewer checklist

• Run the whole Darling.Tests assembly, not the targeted globs — specifically PlanForceNoWritePathTests.OnlyTheForceToolClass_TakesTheStoreOwnerSource, which no glob in the test plan reaches and which this diff makes fail.
• Run McpWebhookSigningKeyLiveTests with DARLING_TEST_PG on a cluster with no existing admin/viewer/mcp roles (it skips itself otherwise, and its window assertions are the only pin on the literal 7).
• On a real store: generate a key for one route, confirm the service hot-reloads off the trg_bump_notification_routes beacon, then post a real alert and confirm the receiver verifies with the returned key — and that the previous key still verifies inside the 7-day window.
• Drive the live MCP endpoint over plain HTTP from another machine and confirm the refusal names mcp.network.tls and wrote nothing; then confirm a TLS-terminating proxy on the service's own box is served from loopback.
• Run provision-roles.sql end to end against a self-managed store that already holds an earlier build's read_webhook_signing_target.

Claude posting for Erik Darling. Addressed in bfaf635: the owner-source pin lists generate_webhook_signing_key with its reason, the rotation window comes from the constant, and the stale summary line is corrected.

[claude]

Verdict: LGTM

Claims

• VERIFIED — "PlanForceNoWritePathTests.OnlyTheForceToolClass_TakesTheStoreOwnerSource lists DarlingMcpAlertTools.GenerateWebhookSigningKey with the #6394 reason inline" — PlanForceNoWritePathTests.cs:178-179 adds the entry and an inline /* #6394: ... */ comment; GenerateWebhookSigningKey does take a StoreOwnerSource? ownerSource parameter (DarlingMcpAlertTools.SigningKeys.cs:60), so the census now matches the real code and the ordinal-ordered array stays correctly sorted (DarlingMcpAlertTools < DarlingMcpPlanForceTools < ...PlanUndoTools < ...ServerActionTools).
• VERIFIED — "BuildWebhookSigningKeyFunctionsSql takes the rotation window from WebhookSigning.RotationOverlapDays, not a literal 7" — DarlingManagedRoles.cs:3202 and :3209 now interpolate PerformanceMonitor.Notifications.WebhookSigning.RotationOverlapDays instead of the literal 7.
• VERIFIED — "The generated SQL is unchanged (7 days), so it still matches provision-roles.sql" — RotationOverlapDays is public const int = 7 (WebhookSigning.cs:122), so the interpolated C# constant and the still-literal 7 in Darling/tools/provision-roles.sql:1937,1944 produce byte-identical SQL text.
• VERIFIED — "Considered, not added: ... McpToolServiceParameterResolutionTests.EveryToolServiceParameter_ResolvesFromTheHostsRealServiceCollection already fails on that" — the test exists and builds the real DI collection the host registers, resolving every [McpServerTool] service parameter type from it (McpToolServiceParameterResolutionTests.cs:40-60); if IHttpContextAccessor stopped being registered this would fail, matching the claim's mechanism.
• UNVERIFIED — "The round 1 targeted runs did not include that class, so the pin would have failed in CI" — a statement about which test globs a prior CI run selected; not checkable from a read-only tree.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Service/DarlingManagedRoles.cs:3202 (and :3209) — the rotation window moved from a literal to the shared constant; a wrong substitution here would silently change how long the previous signing key keeps verifying. — covered by McpWebhookSigningKeyLiveTests.GenerateOnARoute_StoresASealedKeyTheSenderOpensAndSignsWith_AndShowsItOnce, which asserts the stored previous_key_valid_until lands within 5 minutes of DateTime.UtcNow + WebhookSigning.RotationOverlap (needs DARLING_TEST_PG)
2. Darling/Darling.Tests/PlanForceNoWritePathTests.cs:179 — the roster entry that is itself the only guard on the StoreOwnerSource seat reaching GenerateWebhookSigningKey and no other unlisted tool. — this line is the fix; per the established #5632/#6230/#6269 pattern it is the accepted enforcement for this seat, not something a second test re-covers
3. Darling/tools/provision-roles.sql:1937,1944 (unchanged by this commit) — still a bare literal 7, so the C# side now names its source of truth while the SQL copy does not; today's values agree only because nothing has moved the constant yet. — UNCOVERED: ProvisionRolesAclDriftTests compares grants/defaults/role settings, not this interval, so a future change to RotationOverlapDays alone would not be caught by any pin on this file (flagged non-blocking in the Round 1 review already; this commit narrows but does not close it)

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, non-blocking — DarlingManagedRoles.cs:3202/3209 now derives from WebhookSigning.RotationOverlapDays, but Darling/tools/provision-roles.sql:1937,1944 still carries the bare literal 7 with nothing tying the two together; same nit the Round 1 review already raised, not made worse here.
• "Ruling (#5833): ... never answer a trap with a test that reads source text or asserts how code is spelled (... a trait roster ...)" — applies to the fix, not to the diff — the change is a missing line (with its #6394 reason) in an existing reflection-based census of real [McpServerTool] method signatures, not a new source-text pin.
• "Lite and Darling ship in parity (CONTRIBUTING's two-store rule)" — does not apply — webhook signing keys remain Darling-only (Lite/Mcp/McpAlertTools.cs registers no signing-key tools), an asymmetry already established before this follow-up and untouched by it.

Divergence (blind read vs PR body)

None — the Round 2 note in the PR body matches the diff on every checkable point: it names exactly the two fixes present (the roster entry with its #6394 reason, and the literal-to-constant change), and correctly notes the generated SQL text is unchanged.

Reviewer checklist

• Run Darling.Tests (at least PlanForceNoWritePathTests and McpWebhookSigningKeyLiveTests with DARLING_TEST_PG) to confirm the previously-failing roster assertion now passes and the rotation window still lands at 7 days end to end.