You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Skills: support DI catalog registration and request-scoped service access #1941
WithSkills(IMcpSkillCatalog) accepts an existing catalog instance shared across requests; there is no generic or factory registration overload.
McpSkillRequestContext exposes request and caller information, but not a service provider or McpServer.
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.
@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, and Services and Server on the catalog context. Here's what's left from my design, so I'd keep this open until it's in:
Options pattern: Pass configure to services.Configure<McpSkillsOptions> so it can be bound from configuration and validated at startup, ideally before Skills ships since it changes when configure runs.
Scoped by default: My design used scoped, because with ScopeRequests = false on stdio or stateful HTTP, the root keeps every disposable transient catalog until shutdown, while a scoped one throws under scope validation.
No-provider error: Name the fix in the message, like passing the service provider to McpServer.Create.
Docs, what custom handlers can't do: Say that they stamp their own resultType and get no MRTR, so ElicitAsync and SampleAsync on context.Server have no 2026-07-28 path, and none at all in stateless HTTP.
Docs, ScopeRequests = false: Stdio and stateful HTTP use the root, and legacy SSE shares the GET request's scope across concurrent requests, so a scoped DbContext is shared.
Docs, catalogs: Show RoleSkillCatalog registered as a singleton or an instance so its cache lasts, add IDbContextFactory<T> to the singleton guidance, and update the "raw request handlers" remarks.
Database example: An EF Core catalog with tenant filtering and paging, as a docs snippet plus a test over a fake store, or a runnable sample if you'd prefer.
Tests: For ValidateOnBuild reporting a missing catalog dependency, an abstract TCatalog, ScopeRequests = false on stdio and legacy SSE, and the no-provider error.
I can take these in a follow-up PR after #1969 merges.
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 🤖