Skip to content

Darling MCP: a /core endpoint serves the tools analysis points to (#3898 D7) - #4121

Merged
erikdarlingdata merged 5 commits into
devfrom
feature/3898-p4-core-profile
Sep 24, 2026
Merged

erikdarlingdata merged 5 commits into
devfrom
feature/3898-p4-core-profile

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

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. /core serves only the tools closed over next_tools (about 80, per the ruling). Lite gets no profiles under D6. This PR touches Darling only.

What changes

The closure

DarlingCoreToolProfile.Closure is 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 a next_tools recommendation 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_tools payloads. None of it is hand-copied:

  • ToolRecommendations.AllToolNames. A new property on the existing class in DarlingMcpTools.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_clutter names a fixed six tools in its own next_tools, not keyed by fact. I named the six tools as constants and pointed both the array and the existing NextTools() 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 /core ships. Three new tests in DarlingCoreToolProfileTests.cs cover it. ClosureIsPinned pins the exact 74-name list. ClosureCountIsInsideTheRuledBand pins the band itself. EveryClosureNameIsARegisteredDarlingTool checks every closure name against the real tool set the host registers, through McpToolsListBudgetTests.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 as get_blocking (see [McpServerTool(Name = "get_blocking")] in DarlingMcpBlockingTools.cs). Eight entries in the SQL-Server fact-key table in DarlingMcpTools.cs had recommended the wrong name since they were written. Every blocking-related finding's next_tools pointed callers at a tool Darling has never had, and the call always failed. This PR fixes all eight to get_blocking.

Serving a subset per path

DarlingMcpHostService.cs's .WithHttpTransport(...) now also sets ConfigureSessionOptions. The SDK invokes that callback once per request in stateless mode, with that request's HttpContext. When the path is /core (checked by DarlingCoreToolProfile.IsCorePath, segment-matched, so /corex does not match), the callback replaces the session's McpServerOptions.ToolCollection with DarlingCoreToolProfile.FilterToolCollection(...). That method returns a fresh McpServerPrimitiveCollection<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 ### mcp section, describing /core.

Security

Same middleware, unconditionally. /core is mapped on the same _app as /, after the same _app.Use chain: 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 /core is refused at exactly the gate a request to / would be refused at. That refusal happens before either MapMcp call runs, so it also happens before the /core tool filter runs.

HostHeaderGuardTests.cs gets two new source-order tests that parse the real host source to prove this:

  • McpHost_RunsTheGuardBeforeMapMcpCore: the Host guard precedes MapMcp("/core").
  • McpHost_RunsTheNetworkModeGateBeforeBothMapMcpCalls: the bearer-token and CIDR block precedes both MapMcp calls.

That second test is the proof the brief asked for, that a request to /core without 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/call for a tool outside the profile fails on /core. It does not merely go missing from tools/list. The SDK's own tools/call dispatch checks McpServerOptions.ToolCollection before it falls back to any handler. Darling registers no fallback CallToolHandler anywhere; 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_ItDoesNotJustHideThem proves the removal directly. It runs the real served tool set, from McpToolsListBudgetTests.BuildServedTools() (widened from private to internal for this reuse), through FilterToolCollection. 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. /core adds 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

  • Build PerformanceMonitor.Darling.Service and Darling.Tests: 0 Warning(s), 0 Error(s).
  • Targeted runs while iterating: DarlingCoreToolProfileTests (6 tests), HostHeaderGuardTests (12 tests), McpToolTypeRegistrationTests, McpToolsListBudgetTests. All green together, 51 total in one combined run.
  • Full Darling.Tests suite, once, after merging origin/dev. Result: 13399 total, 0 failed, 0 errors, 637 skipped (PG-rig-gated, no DARLING_TEST_PG set), 1 not run. No DarlingInstallLocationTests failures seen.
  • Lite suite: not run. This PR does not touch PerformanceMonitor.Common or any Lite file. Lane-p4a owns McpSchemaCompat.cs; wave E lanes own the tool-description files. Lite is unaffected.
  • Live end-to-end HTTP request against a bound /core port: 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 review, round 1 (opus security-reviewer): no Critical, High or Medium findings. It confirmed the four checks the brief asked for:
    • Auth parity: the Host, token and CIDR gates run for every path.
    • A real subset: each request gets fresh options, and Darling registers no fallback call handler. The unknown-argument guard reads the filtered set.
    • Path matching: /CORE and /core/ match /core. /core/x, //core and /core%2F match no endpoint.
    • No new exposure.
  • It raised three Low items. This PR fixes all three:
    1. The subset holds only in Stateless mode. A stateful session is found by its id alone. A session opened on / can then call any tool on /core. A new test, McpHost_KeepsTheTransportStatelessWhileCoreIsMapped, pins options.Stateless = true in the same block as ConfigureSessionOptions.
    2. If a middleware branches on the path, the source-order tests prove nothing, because endpoints run last. A new test, McpHost_HasNoPathConditionalMiddlewareBeforeTheMcpMappings, looks between Build() and MapMcp("/core"). If Request.Path, UseWhen(, MapWhen(, .Map( or UsePathBase( appears there, it fails.
    3. Nothing kept write tools out of /core. A new test, ClosureHoldsNoWriteTool, checks every tool name that starts with a write verb: create_, update_, delete_, set_, add_, remove_ or mute_. If the closure holds one, it fails. It also checks that such tools exist, so it cannot pass by accident.
  • The review also noted that /core served the shared instructions, which count and describe all 159 tools. A /core session 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_AndKeepTheFullText covers it.
  • The changelog entry was committed at docs/changelog/buffer/, which is not a repo path. It moved to the local buffer, and this PR no longer adds that file.
  • Checked the closure: 74 tools, inside the 70-90 band, and all reads. The eight next_tools fixes are right. Darling has no get_blocked_process_reports. Its tool is get_blocking.
  • After the fixes: build succeeded with 0 warnings. Targeted classes passed: 242 (1 skipped, live PostgreSQL), then 54 more for the next_tools and PostgreSQL-target classes.

Handoff

Nothing is parked. The closure count, 74, sits inside the ruled band, so /core ships 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

erikdarlingdata and others added 4 commits September 23, 2026 23:16
 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
erikdarlingdata marked this pull request as ready for review September 24, 2026 03:38
@erikdarlingdata
erikdarlingdata enabled auto-merge (squash) September 24, 2026 03:38
@erikdarlingdata
erikdarlingdata merged commit a13dbba into dev Sep 24, 2026
16 of 18 checks passed
@erikdarlingdata
erikdarlingdata deleted the feature/3898-p4-core-profile branch September 24, 2026 04:25
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant