Closed
Description
Summary
Track dependency-injection registration and request-scoped service access for custom Skills catalogs, deferred from #1856 by agreement during review.
Review discussion: #1856 (comment)
Current behavior
WithSkills(IMcpSkillCatalog)accepts an existing catalog instance shared across requests; there is no generic or factory registration overload.McpSkillRequestContextexposes request and caller information, but not a service provider orMcpServer.- A database-backed catalog can create its own scope through
IServiceScopeFactory, but that scope is independent of the existing request scope. A shared catalog must not capture scoped services directly.
The Skills guide in #1856 documents these constraints. Proper DI integration was explicitly left out of that PR.
Follow-up scope
- Design generic and/or factory-based catalog registration, including the supported catalog lifetimes.
- Separately determine how catalogs can access services from the existing request scope, including any Core plumbing needed for raw request handlers.
- Define scope ownership, disposal (including async disposal), and behavior across supported transports without introducing captive scoped dependencies.
- Add focused tests and documentation for the chosen registration and lifetime model, including a database-backed catalog example.
Creating a separate scope per catalog call remains a valid interim approach; it must not be described as sharing the MCP request scope. This issue tracks the design and implementation, not a requirement to expand the initial Skills PR.
Note
This issue was generated by GitHub Copilot at the maintainer's request.
Metadata
Metadata
Assignees
Labels
No labels
Activity
tarekgh commentedon Oct 20, 2026
CC @girishkvs @PederHP
girishkvs commentedon Oct 20, 2026
@tarekgh Can you assign this issue to me? I already have a local draft from my #1856 work.
tarekgh commentedon Oct 20, 2026
Thanks @girishkvs for willing to help with this issue.
girishkvs commentedon Oct 30, 2026
@tarekgh @halter73 #1969 covers the DI part of this:
WithSkills<TCatalog>()and factory registration with lifetimes, catalogs resolved from the request's services, custom handlers on the built-in request scope, andServicesandServeron the catalog context. Here's what's left from my design, so I'd keep this open until it's in:configuretoservices.Configure<McpSkillsOptions>so it can be bound from configuration and validated at startup, ideally before Skills ships since it changes whenconfigureruns.ScopeRequests = falseon stdio or stateful HTTP, the root keeps every disposable transient catalog until shutdown, while a scoped one throws under scope validation.McpServer.Create.resultTypeand get no MRTR, soElicitAsyncandSampleAsynconcontext.Serverhave no 2026-07-28 path, and none at all in stateless HTTP.ScopeRequests = false: Stdio and stateful HTTP use the root, and legacy SSE shares the GET request's scope across concurrent requests, so a scopedDbContextis shared.RoleSkillCatalogregistered as a singleton or an instance so its cache lasts, addIDbContextFactory<T>to the singleton guidance, and update the "raw request handlers" remarks.ValidateOnBuildreporting a missing catalog dependency, an abstractTCatalog,ScopeRequests = falseon stdio and legacy SSE, and the no-provider error.I can take these in a follow-up PR after #1969 merges.
— girishkvs's Copilot 🤖