semantic-kernel icon indicating copy to clipboard operation
semantic-kernel copied to clipboard

.Net Bug: Adding a plugin to a `Kernel` instance does not retrieve dependencies from `Kernel.Services` by default.

Open crickman opened this issue 11 months ago • 3 comments

Describe the bug A customer experienced and reported this issue. Adding a plugin via AddFromType uses the kernel's service-collection/provider by default in the builder case, but not when invoked from an existing kernel instance (e.g. kernel.Plugins.AddFromType<T>(...)).

Upon inspection, there's a serviceProvider parameter on AddFromType.

When serviceProvider is default (null), should Kernel.Services be the default? If not, curious on thinking around this.

If no service-provider is, in fact, the desired default, then perhaps clarifying the pattern when DI is required in samples and documentation may be helpful.

Example

Can run as Concepts sample

using Microsoft.Extensions.DependencyInjection;
using Microsoft.SemanticKernel;

namespace Plugins;

public class ImportPluginWithDependency(ITestOutputHelper output) : BaseTest(output)
{
    /// <summary>
    /// Initialize plugin and its dependency using a builder.
    /// </summary>
    [Fact]
    public async Task UseBuilderAsync()
    {
        string referenceState = Guid.NewGuid().ToString();
        IKernelBuilder builder = CreateKernelBuilderWithDependency(referenceState);

        builder.Plugins.AddFromType<MyPlugin>();

        Kernel kernel = builder.Build();

        await this.InvokePluginAsync(kernel, referenceState);
    }

    /// <summary>
    /// Initialize plugin dependency using a builder and
    /// then add plugin to kernel with default parameters.
    /// </summary>
    [Fact]
    public async Task UseKernelIntuitiveAsync()
    {
        string referenceState = Guid.NewGuid().ToString();
        IKernelBuilder builder = CreateKernelBuilderWithDependency(referenceState);

        Kernel kernel = builder.Build();

        kernel.Plugins.AddFromType<MyPlugin>();

        await this.InvokePluginAsync(kernel, referenceState);
    }

    /// <summary>
    /// Initialize plugin dependency using a builder and
    /// then add plugin to kernel explicitly passing in kernel.Services.
    /// </summary>
    [Fact]
    public async Task UseKernelExpicitAsync()
    {
        string referenceState = Guid.NewGuid().ToString();
        IKernelBuilder builder = CreateKernelBuilderWithDependency(referenceState);

        Kernel kernel = builder.Build();

        kernel.Plugins.AddFromType<MyPlugin>(serviceProvider: kernel.Services);

        await this.InvokePluginAsync(kernel, referenceState);
    }

    private static IKernelBuilder CreateKernelBuilderWithDependency(string referenceState)
    {
        IKernelBuilder builder = Kernel.CreateBuilder();
        builder.Services.AddSingleton(new MyDependency(referenceState));
        return builder;
    }

    private async Task InvokePluginAsync(Kernel kernel, string referenceState)
    {
        KernelArguments arguments = new() { { "input", "test" } };
        FunctionResult result = await kernel.InvokeAsync(kernel.Plugins[nameof(MyPlugin)].Single(), arguments);
        PluginResult pluginResult = result.GetValue<PluginResult>() ?? throw new Exception("Whoops!");

        Console.WriteLine($"Plugin response: {pluginResult.Input} {pluginResult.State} =? {referenceState}");
    }

    private class MyDependency
    {
        public MyDependency(string state)
        {
            this.State = state;
        }

        public string State { get; }
    }

    private record PluginResult(string Input, string State);

    private class MyPlugin(MyDependency dependency)
    {
        [KernelFunction]
        public PluginResult MyOperation(string input)
        {
            return new(input, dependency.State);
        }
    }
}

crickman avatar Jan 22 '25 18:01 crickman

I think it was an intentional design decision made a long time ago, before SK v1 was released, not to couple the KernelPluginCollection that the kernel uses to keep its plugins with the kernel itself, in order to allow scenarios where the same collection can be reused among a few kernels if needed.

For example, if we update the KernelPluginCollection to have a reference to the kernel, this scenario may work:

ServiceCollection services = new();
services.AddSingleton(new MyDependency());

ServiceProvider serviceProvider = services.BuildServiceProvider()

var kernel1 = new Kernel(serviceProvider);
kernel1.Plugins.AddFromType<MyPlugin>();   // The `AddFromType` method uses kernel services to resolve MyPlugin dependencies.

var kernel2 = new Kernel(serviceProvider);
kernel2 .Plugins.AddFromType<MyPlugin>();  // The `AddFromType` method uses kernel services to resolve MyPlugin dependencies.

However, this one won't work and can be confusing because of the inconsistent behavior of the AddFromType method, which in some cases can resolve plugin dependencies and in others cannot:

KernelPluginCollection plugins = new();
plugins.AddFromType<MyPlugin>();

var kernel1 = new Kernel(plugins: plugins);
kernel1.Plugins.AddFromType<MyPlugin1>(); // The `AddFromType` can't use the kernel to resolve the plugin dependencies because the collection was created without it.

var kernel2 = new Kernel(plugins: plugins);
kernel2.Plugins.AddFromType<MyPlugin2>(); // The `AddFromType` can't use the kernel to resolve the plugin dependencies because the collection was created without it.

I'll check DI samples and update them if's not there.

CC: @stephentoub

SergeyMenshykh avatar Jan 23 '25 21:01 SergeyMenshykh

Agree its confusing. I question the priority of the "re-use" scenario.

Once a collection is associated with a Kernel instance, its understable that a developer will intuitively consider them bound.

I see that KernelPluginCollection is a constructor parameter. Unclear why IEnumerable<KernelPlugin> wouldn't have sufficed.

crickman avatar Feb 13 '25 01:02 crickman

This issue is stale because it has been open for 90 days with no activity.

github-actions[bot] avatar May 18 '25 02:05 github-actions[bot]

This issue was closed because it has been inactive for 14 days since being marked as stale.

github-actions[bot] avatar Jun 01 '25 02:06 github-actions[bot]