Skip to content

Resolving scoped service is not supported #198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gong626035906 opened this issue Apr 3, 2025 · 7 comments · Fixed by #276
Closed

Resolving scoped service is not supported #198

gong626035906 opened this issue Apr 3, 2025 · 7 comments · Fixed by #276
Assignees
Labels
bug Something isn't working

Comments

@gong626035906
Copy link

Describe the bug
When I try to inject a service with a Scoped lifetime in the Tool method, an error will occur. I wonder if all injections must be of the Singleton type?

To Reproduce
Steps to reproduce the behavior:

  1. Add the following code in Program.cs: builder.Services.AddDbContext<CoreDbContext>(); At this point, the lifetime of CoreDbContext is Scoped.
  2. Add the following parameter to the McpServerTool method:
Task<string> GetKnowledge(
        IMcpServer thisServer,
        CoreDbContext db,
        [Description("question")] string question)
{
}
  1. An exception will be thrown when the LLM calls the method.

Expected behavior
Inject CoreDbContext correctly

Logs

System.InvalidOperationException: Cannot resolve scoped service 'Zenative.Server.Database.CoreDbContext' from root provider.
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteValidator.ValidateResolution(ServiceCallSite callSite, IServiceScope scope, IServiceScope rootScope)
   at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetService(ServiceIdentifier serviceIdentifier, ServiceProviderEngineScope serviceProviderEngineScope)
   at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetService(Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.ConstructorMatcher.CreateInstance(IServiceProvider provider)
   at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.CreateInstance(IServiceProvider provider, Type instanceType, Object[] parameters)
   at Microsoft.Extensions.AI.TemporaryAIFunctionFactory.ReflectionAIFunction.InvokeCoreAsync(IEnumerable`1 arguments, CancellationToken cancellationToken)
@gong626035906 gong626035906 added the bug Something isn't working label Apr 3, 2025
@eiriktsarpalis
Copy link
Contributor

cc @stephentoub. Seems we might want to transfer this to dotnet/extensions?

@stephentoub
Copy link
Contributor

cc @stephentoub. Seems we might want to transfer this to dotnet/extensions?

What is the problem? Wouldn't this be more about which IServiceProvider is supplied?

@stephentoub
Copy link
Contributor

@gong626035906, can you please share a minimal repro of the issue?

@halter73
Copy link
Contributor

halter73 commented Apr 3, 2025

@stephentoub and I had a conversation about this in #201 just after it was merged.

resolving a scoped service without first creating a scope is bad practice, and our hosts now guard against it in dev environments. learn.microsoft.com/en-us/dotnet/core/compatibility/aspnet-core/9.0/hostbuilder-validation
...
Using scoped services in MCP tools is not currently supported for stdio scenarios (unless you disable validation) because there are no scopes. Without creating scopes, it's hard to see the point of scoped services anyway. You should be able to resolve request scoped services in tools invoked via MapMcp though. I'll add tests for this in my next PR.

@halter73
Copy link
Contributor

halter73 commented Apr 3, 2025

It's probably worth mentioning that you could do the following as a workaround for your tool that needs a DbContext.

Task<string> GetKnowledge(
        IMcpServer thisServer,
        IServiceScopeFactory serviceScopeFactory,
        [Description("question")] string question)
{
    await using scope = serviceScopeFactory.CreateAsyncScope();
    var db = scope.ServiceProvider.GetRequiredService<CoreDbContext>();

    // ...
}

This is arguably better than us creating a scope per tool invocation for you, because it gives you explicit control, but I could see us doing it implicitly being handy as well. This would be more in line with HTTP request middleware and SignalR Hub methods which both create a scope per invocation.

So, I retract my position on the PR that we shouldn't make any product changes.

Using the request services from the /sse streaming request as the tool scope also isn't great, because the scope would last the duration of the session which is probably too long for most scoped services like a DbContext. I'll work on creating a scope per tool invocation. It would still be nice to use the request service scope for stateless tool invocations, since that would be 1:1, but I'll cross that bridge when I get there.

@halter73 halter73 self-assigned this Apr 3, 2025
@gong626035906
Copy link
Author

I recognize the point. Thanks.

@stephentoub
Copy link
Contributor

I believe @halter73 is using this issue to track a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants