Repository navigation
Darling MCP: a /core endpoint serves the tools analysis points to (#3898 D7) - #4121
Merged
Merged
Conversation
D7) /core is a second MapMcp mapping, filtered in ConfigureSessionOptions to DarlingCoreToolProfile's closure: the four entry tools (list_servers, get_fleet_overview, analyze_server, get_tool_guide) plus every tool name a next_tools recommendation can point at, computed from the recommendation tables rather than hand-kept. / is unchanged. Along the way, fixed a real bug the closure computation surfaced: the SQL Server next_tools table recommended "get_blocked_process_reports", a tool Darling never registered (Darling's name is get_blocking) - so those findings' next_tools always 404'd. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQh8LczP1HGLXQYAx4oTZC
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQh8LczP1HGLXQYAx4oTZC
…th a note (#3898) The security review's three Low items, and its instructions note: - A test pins options.Stateless = true beside ConfigureSessionOptions. The subset holds only in Stateless mode. - A test fails if anything between Build() and MapMcp("/core") reads the path or maps a branch, so the order checks mean something. - A test fails if the /core closure holds a write-verb tool. - A /core session's instructions lead with a note saying it serves 74 tools and that the shared text describes the full set on /. The changelog entry leaves docs/changelog/buffer/, which is not a repo path. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQh8LczP1HGLXQYAx4oTZC
erikdarlingdata
marked this pull request as ready for review
September 24, 2026 03:38
erikdarlingdata
enabled auto-merge (squash)
September 24, 2026 03:38
erikdarlingdata
added a commit
that referenced
this pull request
Sep 24, 2026
* Add 48 buffered CHANGELOG entries in one splice Adds 48 entries and 48 link refs (#4057, #4077, #4079, #4081, #4082, #4083, #4084, #4085, #4086, #4087, #4088, #4089, #4090, #4091, #4092, #4093, #4095, #4096, #4099, #4100, #4101, #4103, #4105, #4106, #4107, #4108, #4109, #4110, #4111, #4113, #4114, #4115, #4116, #4117, #4118, #4119, #4120, #4121, #4122, #4123, #4124, #4125, #4126, #4127, #4131, #4133, #4136, #4141). Each PR's entry was buffered, and this lands every entry whose PR was merged on origin/dev when it ran. Entries found under a bare section line (none unless the old script ran again) move into the matching ### section, below the new entries. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQh8LczP1HGLXQYAx4oTZC * Changelog splice: add #4137's entry, which merged before the splice but was missed #4137 (pg_plan_capture reads csvlog) merged at 05:57Z with a CHANGELOG entry section in its body. It was neither spliced nor listed as needing no entry. Its entry goes under Fixed, right after #4136's, and #4136's last sentence now points to it instead of saying plan capture is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NUU29PuGg9TUFBsACgZg2K --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #3898.
Why
Erik's rulings D6/D7 on #3898 call for a URL-path profile on Darling's MCP host.
/keeps serving the full tool set, unchanged, for good./coreserves only the tools closed overnext_tools(about 80, per the ruling). Lite gets no profiles under D6. This PR touches Darling only.What changes
The closure
DarlingCoreToolProfile.Closureis a new file,Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingCoreToolProfile.cs. It starts from four entry tools:list_servers,get_fleet_overview,analyze_server,get_tool_guide. It unions in every tool name anext_toolsrecommendation can point at. It repeats that union in a real fixed-point loop until nothing new is added.The nameable set is read directly from the three production tables that build
next_toolspayloads. None of it is hand-copied:ToolRecommendations.AllToolNames. A new property on the existing class inDarlingMcpTools.cs. This is the SQL-Server-target fact-key table.PgTargetToolRecommendations.AllToolNames. A new property, same pattern. This is the PostgreSQL-target fact-key table.DarlingMcpQueryStoreClutterTools.FixedNextToolNames. A new array.get_query_store_clutternames a fixed six tools in its ownnext_tools, not keyed by fact. I named the six tools as constants and pointed both the array and the existingNextTools()method at those same constants, so the served payload and the closure computation cannot drift apart.Today the union converges after one pass. Nothing in the current tables keys a recommendation off a tool's own identity. The loop is still a real loop, not a single union dressed up as one, so a source that does chain in the future will still converge correctly.
The count: 74
That sits inside Erik's [70, 90] band, so
/coreships. Three new tests inDarlingCoreToolProfileTests.cscover it.ClosureIsPinnedpins the exact 74-name list.ClosureCountIsInsideTheRuledBandpins the band itself.EveryClosureNameIsARegisteredDarlingToolchecks every closure name against the real tool set the host registers, throughMcpToolsListBudgetTests.Measure().A real bug the closure computation found
Building the "every name is registered" check failed on
get_blocked_process_reports. That is Lite's name for the tool. Darling registers the same read asget_blocking(see[McpServerTool(Name = "get_blocking")]inDarlingMcpBlockingTools.cs). Eight entries in the SQL-Server fact-key table inDarlingMcpTools.cshad recommended the wrong name since they were written. Every blocking-related finding'snext_toolspointed callers at a tool Darling has never had, and the call always failed. This PR fixes all eight toget_blocking.Serving a subset per path
DarlingMcpHostService.cs's.WithHttpTransport(...)now also setsConfigureSessionOptions. The SDK invokes that callback once per request in stateless mode, with that request'sHttpContext. When the path is/core(checked byDarlingCoreToolProfile.IsCorePath, segment-matched, so/corexdoes not match), the callback replaces the session'sMcpServerOptions.ToolCollectionwithDarlingCoreToolProfile.FilterToolCollection(...). That method returns a freshMcpServerPrimitiveCollection<McpServerTool>holding only the closure's tools. Every other path, in practice just/, is untouched and keeps the full roughly 159-tool set._app.MapMcp("/core");is added right after the existing_app.MapMcp();.Docs
One plain sentence in
Darling/README.md's### mcpsection, describing/core.Security
Same middleware, unconditionally.
/coreis mapped on the same_appas/, after the same_app.Usechain: the Host-header rebinding guard in both modes, then in network mode the constant-time bearer-token check, then the loopback-exempt CIDR check. None of that chain reads the request path. A request to/coreis refused at exactly the gate a request to/would be refused at. That refusal happens before eitherMapMcpcall runs, so it also happens before the/coretool filter runs.HostHeaderGuardTests.csgets two new source-order tests that parse the real host source to prove this:McpHost_RunsTheGuardBeforeMapMcpCore: the Host guard precedesMapMcp("/core").McpHost_RunsTheNetworkModeGateBeforeBothMapMcpCalls: the bearer-token and CIDR block precedes bothMapMcpcalls.That second test is the proof the brief asked for, that a request to
/corewithout a token is rejected exactly like one to/. The proof is structural: same unconditioned middleware, not a live socket test. That matches how every other gate in this file is already tested; no live-HTTP harness exists yet for either Darling host.A real subset, not a listing filter. A
tools/callfor a tool outside the profile fails on/core. It does not merely go missing fromtools/list. The SDK's owntools/calldispatch checksMcpServerOptions.ToolCollectionbefore it falls back to any handler. Darling registers no fallbackCallToolHandleranywhere; I confirmed this with a repo-wide grep. So an excluded tool gets the SDK's own "unknown tool" error, identical to a made-up name.DarlingCoreToolProfileTests.FilterToolCollection_RemovesToolsOutsideTheClosure_ItDoesNotJustHideThemproves the removal directly. It runs the real served tool set, fromMcpToolsListBudgetTests.BuildServedTools()(widened fromprivatetointernalfor this reuse), throughFilterToolCollection. It asserts three things: the result's tool-name set is exactly the 74-name closure, its count is strictly smaller than the input's, and one specific excluded tool name is absent from the filtered collection.No new write surface, no new secret.
/coreadds no new tool, no new credential, and no new config knob. It is a narrower read of the same tool set/already serves, gated by config that already exists (mcp.network.*).Test plan
PerformanceMonitor.Darling.ServiceandDarling.Tests: 0 Warning(s), 0 Error(s).DarlingCoreToolProfileTests(6 tests),HostHeaderGuardTests(12 tests),McpToolTypeRegistrationTests,McpToolsListBudgetTests. All green together, 51 total in one combined run.Darling.Testssuite, once, after mergingorigin/dev. Result: 13399 total, 0 failed, 0 errors, 637 skipped (PG-rig-gated, noDARLING_TEST_PGset), 1 not run. NoDarlingInstallLocationTestsfailures seen.PerformanceMonitor.Commonor any Lite file. Lane-p4a ownsMcpSchemaCompat.cs; wave E lanes own the tool-description files. Lite is unaffected./coreport: not run. The security proof here is structural, matching this test file's existing style. No live-Kestrel HTTP harness exists yet for either Darling host.Coordinator verification
security-reviewer): no Critical, High or Medium findings. It confirmed the four checks the brief asked for:/COREand/core/match/core./core/x,//coreand/core%2Fmatch no endpoint./can then call any tool on/core. A new test,McpHost_KeepsTheTransportStatelessWhileCoreIsMapped, pinsoptions.Stateless = truein the same block asConfigureSessionOptions.McpHost_HasNoPathConditionalMiddlewareBeforeTheMcpMappings, looks betweenBuild()andMapMcp("/core"). IfRequest.Path,UseWhen(,MapWhen(,.Map(orUsePathBase(appears there, it fails./core. A new test,ClosureHoldsNoWriteTool, checks every tool name that starts with a write verb:create_,update_,delete_,set_,add_,remove_ormute_. If the closure holds one, it fails. It also checks that such tools exist, so it cannot pass by accident./coreserved the shared instructions, which count and describe all 159 tools. A/coresession now leads its instructions with a short note. The note says it serves 74 tools and that the text below describes the full set on/. It also says a tool it does not serve fails as unknown. The SDK (ModelContextProtocol.AspNetCore 2.2.0,StreamableHttpHandler) builds each request's options through the options factory before the callback runs. So the instructions are already set when the note is added, and nothing carries over between requests.CoreInstructions_LeadWithTheCoreNote_AndKeepTheFullTextcovers it.docs/changelog/buffer/, which is not a repo path. It moved to the local buffer, and this PR no longer adds that file.get_blocked_process_reports. Its tool isget_blocking.Handoff
Nothing is parked. The closure count, 74, sits inside the ruled band, so
/coreships in this PR rather than waiting.No live HTTP test exists for either Darling host. The source tests above cover what a source test can. #4128 tracks a live test.
🤖 Generated with Claude Code
https://claude.ai/code/session_01QQh8LczP1HGLXQYAx4oTZC