diff --git a/docs/comparison.md b/docs/comparison.md index 832e883d..1c45697a 100644 --- a/docs/comparison.md +++ b/docs/comparison.md @@ -73,7 +73,7 @@ Repl Toolkit is a command-surface framework — not just a CLI parser. It builds | WebSocket session hosting | ❌ | ❌ | ✅ `Repl.WebSocket` | | Telnet session hosting | ❌ | ❌ | ✅ `Repl.Telnet` | | Terminal metadata negotiation | ❌ | ❌ | ✅ NAWS, TTYPE, DTTERM | -| Per-session DI & state | ❌ | ❌ | ✅ `IReplSessionState` | +| Per-session DI & state | ❌ | ❌ | ✅ DI scope per session + `IReplSessionState` | | Window size detection | ❌ | ✅ `Console` only | ✅ Local + remote | | Transport-agnostic host | ❌ | ❌ | ✅ `StreamedReplHost` | diff --git a/src/Repl.Defaults/ReplApp.cs b/src/Repl.Defaults/ReplApp.cs index 530e3d4c..044cf191 100644 --- a/src/Repl.Defaults/ReplApp.cs +++ b/src/Repl.Defaults/ReplApp.cs @@ -17,8 +17,10 @@ public sealed class ReplApp : IReplApp private readonly IServiceCollection _services; // Lazily built provider shared between MapModule() and Run(). - // Ensures modules resolved via DI share the same service instances - // as handler parameters resolved at runtime. + // Singleton services resolved by modules are the same instances handler parameters + // see at runtime; Scoped services intentionally differ — handlers resolve them from + // the per-session scope opened by Run* (see RunInSessionScopeAsync), while module + // resolution at registration time happens outside any session. private ServiceProvider? _sharedProvider; // Extension packages (e.g. Repl.Spectre) park per-app configuration here so it stays @@ -252,18 +254,28 @@ public async ValueTask RunAsync( var runOptions = options ?? new ReplRunOptions(); if (runOptions.HostedServiceLifecycle is HostedServiceLifecycleMode.None or HostedServiceLifecycleMode.Guest) { - return await _core.RunWithServicesAsync(args, services, cancellationToken).ConfigureAwait(false); + return await RunInSessionScopeAsync( + services, + runOptions.SessionScope, + (sessionServices, ct) => _core.RunWithServicesAsync(args, sessionServices, ct), + cancellationToken).ConfigureAwait(false); } var started = Array.Empty(); var exitCode = 0; try { + // Hosted services are app-level, not session-level: they start from the + // unscoped provider so their lifetime is not tied to this session's scope. started = [.. await HostedServiceLifecycleCoordinator.StartAsync(services, cancellationToken) .ConfigureAwait(false), ]; - exitCode = await _core.RunWithServicesAsync(args, services, cancellationToken).ConfigureAwait(false); + exitCode = await RunInSessionScopeAsync( + services, + runOptions.SessionScope, + (sessionServices, ct) => _core.RunWithServicesAsync(args, sessionServices, ct), + cancellationToken).ConfigureAwait(false); } catch (HostedServiceLifecycleException ex) { @@ -366,9 +378,38 @@ public async ValueTask RunAsync( using (ReplSessionIO.SetSession(host.Output, host.Input, runOptions.AnsiSupport, sessionHost?.SessionId)) { ApplyTerminalOverrides(runOptions); - var sessionProvider = CreateSessionOverlay(services); - return await _core.RunWithServicesAsync(args, sessionProvider, cancellationToken) - .ConfigureAwait(false); + // The session overlay composes on top of the SESSION scope, so overlay lookups + // hitting the external provider observe the session's Scoped instances. + return await RunInSessionScopeAsync( + services, + runOptions.SessionScope, + (sessionServices, ct) => _core.RunWithServicesAsync(args, CreateSessionOverlay(sessionServices), ct), + cancellationToken).ConfigureAwait(false); + } + } + + // One Run* call is one session: it owns a DI scope so Scoped services resolve per + // session (the documented lifetime) and scoped disposables are released when the + // session ends. SessionScopeBehavior.CallerOwned opts out for providers that already + // represent the session's scope (e.g. a Blazor circuit scope, or Repl.Testing which + // runs each command of one logical session as a separate one-shot call); providers + // without scope support run unscoped as before. + private static async ValueTask RunInSessionScopeAsync( + IServiceProvider services, + SessionScopeBehavior sessionScope, + Func> run, + CancellationToken cancellationToken) + { + if (sessionScope == SessionScopeBehavior.CallerOwned + || services.GetService(typeof(IServiceScopeFactory)) is not IServiceScopeFactory scopeFactory) + { + return await run(services, cancellationToken).ConfigureAwait(false); + } + + var scope = scopeFactory.CreateAsyncScope(); + await using (scope.ConfigureAwait(false)) + { + return await run(scope.ServiceProvider, cancellationToken).ConfigureAwait(false); } } diff --git a/src/Repl.Defaults/ReplRunOptions.cs b/src/Repl.Defaults/ReplRunOptions.cs index c54fa813..07658145 100644 --- a/src/Repl.Defaults/ReplRunOptions.cs +++ b/src/Repl.Defaults/ReplRunOptions.cs @@ -10,6 +10,11 @@ public sealed record ReplRunOptions /// public HostedServiceLifecycleMode HostedServiceLifecycle { get; init; } = HostedServiceLifecycleMode.None; + /// + /// Gets or sets how this run manages the session's dependency-injection scope. + /// + public SessionScopeBehavior SessionScope { get; init; } = SessionScopeBehavior.PerRun; + /// /// Gets or sets the ANSI support mode for the session. /// diff --git a/src/Repl.Defaults/SessionScopeBehavior.cs b/src/Repl.Defaults/SessionScopeBehavior.cs new file mode 100644 index 00000000..1dc6c310 --- /dev/null +++ b/src/Repl.Defaults/SessionScopeBehavior.cs @@ -0,0 +1,20 @@ +namespace Repl; + +/// +/// How a Run* call manages the session's dependency-injection scope. +/// +public enum SessionScopeBehavior +{ + /// + /// The run opens one DI scope for the session (the default): Scoped services resolve + /// per session and scoped disposables are released when the session ends. + /// + PerRun = 0, + + /// + /// The caller's service provider already represents the session's scope (for example a + /// Blazor circuit or per-request scope, or a session owner spanning several one-shot + /// runs): the run resolves from it directly and never opens a nested scope. + /// + CallerOwned = 1, +} diff --git a/src/Repl.IntegrationTests/Given_SessionScopedServices.cs b/src/Repl.IntegrationTests/Given_SessionScopedServices.cs new file mode 100644 index 00000000..6f605751 --- /dev/null +++ b/src/Repl.IntegrationTests/Given_SessionScopedServices.cs @@ -0,0 +1,142 @@ +using Microsoft.Extensions.DependencyInjection; +using Repl.Testing; + +namespace Repl.IntegrationTests; + +[TestClass] +[DoNotParallelize] +public sealed class Given_SessionScopedServices +{ + private sealed class ScopedProbe + { + public Guid Id { get; } = Guid.NewGuid(); + } + + private sealed class TransientProbe + { + public Guid Id { get; } = Guid.NewGuid(); + } + + private sealed class DisposableProbe(List disposed) : IDisposable + { + public Guid Id { get; } = Guid.NewGuid(); + + public void Dispose() => disposed.Add(Id); + } + + private static ReplTestHost CreateProbeHost(List? disposedTracker = null) => + ReplTestHost.Create(() => + { + var app = ReplApp.Create(services => + { + services.AddScoped(); + services.AddTransient(); + services.AddScoped(_ => new DisposableProbe(disposedTracker ?? [])); + }).UseDefaultInteractive(); + app.Map("scoped", (ScopedProbe probe) => probe.Id.ToString()); + app.Map("transient", (TransientProbe probe) => probe.Id.ToString()); + app.Map("disposable", (DisposableProbe probe) => probe.Id.ToString()); + return app; + }); + + private static async Task RunAndCaptureAsync(ReplSessionHandle session, string command) + { + var result = await session.RunCommandAsync($"{command} --no-logo").ConfigureAwait(false); + result.ExitCode.Should().Be(0, "command output was: {0}", result.OutputText); + return result.OutputText.Trim(); + } + + [TestMethod] + [Description("Guards the core per-run session scope on ONE shared app: two Run* calls against the same ReplApp (the exact shape of two hosted connections sharing one app) must resolve two distinct Scoped instances from the app's own root provider — this exercises the scope-creation branch itself, independently of Repl.Testing's session plumbing.")] + public void When_TwoRunsShareOneApp_Then_ScopedInstancesDiffer() + { + var sut = ReplApp.Create(services => services.AddScoped()); + sut.Map("scoped", (ScopedProbe probe) => probe.Id.ToString()); + + var first = ConsoleCaptureHelper.Capture(() => sut.Run(["scoped", "--no-logo"])); + var second = ConsoleCaptureHelper.Capture(() => sut.Run(["scoped", "--no-logo"])); + + first.ExitCode.Should().Be(0); + second.ExitCode.Should().Be(0); + first.Text.Trim().Should().NotBe(second.Text.Trim()); + } + + [TestMethod] + [Description("Guards the CallerOwned opt-out: a caller whose provider already represents the session scope (Blazor circuit, per-request scope, multi-run session owners) must keep its own Scoped instances across runs — the entry points must not open a nested sibling scope that would hide them.")] + public async Task When_CallerOwnsTheSessionScope_Then_RunsShareTheCallerScopedInstances() + { + var sut = ReplApp.Create(services => services.AddScoped()); + sut.Map("scoped", (ScopedProbe probe) => probe.Id.ToString()); + var scopeFactory = (IServiceScopeFactory)sut.Services.GetService(typeof(IServiceScopeFactory))!; + var callerScope = scopeFactory.CreateAsyncScope(); + await using (callerScope.ConfigureAwait(false)) + { + var options = new ReplRunOptions { SessionScope = SessionScopeBehavior.CallerOwned }; + + var first = ConsoleCaptureHelper.Capture( + () => sut.Run(["scoped", "--no-logo"], callerScope.ServiceProvider, options)); + var second = ConsoleCaptureHelper.Capture( + () => sut.Run(["scoped", "--no-logo"], callerScope.ServiceProvider, options)); + + first.ExitCode.Should().Be(0); + first.Text.Trim().Should().Be(second.Text.Trim()); + } + } + + [TestMethod] + [Description("Guards the documented Scoped lifetime for hosted sessions: two distinct hosted sessions must resolve two distinct Scoped instances — sharing one instance across sessions leaks per-user state (auth context, carts) between concurrent clients.")] + public async Task When_TwoHostedSessionsResolveScopedService_Then_InstancesDiffer() + { + await using var host = CreateProbeHost(); + await using var sessionA = await host.OpenSessionAsync(); + await using var sessionB = await host.OpenSessionAsync(); + + var idA = await RunAndCaptureAsync(sessionA, "scoped"); + var idB = await RunAndCaptureAsync(sessionB, "scoped"); + + idA.Should().NotBe(idB); + } + + [TestMethod] + [Description("Guards the session-scope boundary from the other side: two commands within the SAME hosted session share the same Scoped instance — the scope is per session, not per command invocation.")] + public async Task When_SameSessionRunsTwoCommands_Then_ScopedInstanceIsShared() + { + await using var host = CreateProbeHost(); + await using var session = await host.OpenSessionAsync(); + + var first = await RunAndCaptureAsync(session, "scoped"); + var second = await RunAndCaptureAsync(session, "scoped"); + + first.Should().Be(second); + } + + [TestMethod] + [Description("Guards Transient semantics under the session scope: every resolution yields a fresh instance, including across commands of the same session.")] + public async Task When_TransientResolvedInTwoCommands_Then_InstancesDiffer() + { + await using var host = CreateProbeHost(); + await using var session = await host.OpenSessionAsync(); + + var first = await RunAndCaptureAsync(session, "transient"); + var second = await RunAndCaptureAsync(session, "transient"); + + first.Should().NotBe(second); + } + + [TestMethod] + [Description("Guards scoped-disposable lifetime: a Scoped IDisposable resolved during a hosted session is disposed when that session ends, not deferred to app shutdown — session resources (connections, per-user stores) must not accumulate for the host process lifetime.")] + public async Task When_SessionEnds_Then_ScopedDisposablesAreDisposed() + { + var disposed = new List(); + await using var host = CreateProbeHost(disposed); + + string id; + { + await using var session = await host.OpenSessionAsync(); + id = await RunAndCaptureAsync(session, "disposable"); + disposed.Should().NotContain(Guid.Parse(id)); + } + + disposed.Should().Contain(Guid.Parse(id)); + } +} diff --git a/src/Repl.Testing/ReplSessionHandle.cs b/src/Repl.Testing/ReplSessionHandle.cs index e7461b8d..282e8ada 100644 --- a/src/Repl.Testing/ReplSessionHandle.cs +++ b/src/Repl.Testing/ReplSessionHandle.cs @@ -140,23 +140,31 @@ public SessionSnapshot GetSnapshot() return SessionSnapshot.Empty(SessionId); } - public ValueTask DisposeAsync() + public async ValueTask DisposeAsync() { if (_disposed) { - return ValueTask.CompletedTask; + return; } _disposed = true; _owner.RemoveSession(SessionId); ReplSessionIO.RemoveSession(SessionId); - if (_services is IDisposable disposable) + // Take the command gate before disposing the session scope so an in-flight command + // (e.g. an orphaned timed-out run) cannot race scoped-service disposal. + await _commandGate.WaitAsync().ConfigureAwait(false); + switch (_services) { - disposable.Dispose(); + case IAsyncDisposable asyncDisposable: + // Releases the session's DI scope, disposing its Scoped services. + await asyncDisposable.DisposeAsync().ConfigureAwait(false); + break; + case IDisposable disposable: + disposable.Dispose(); + break; } _commandGate.Dispose(); - return ValueTask.CompletedTask; } internal static ValueTask StartAsync( @@ -175,14 +183,52 @@ internal static ValueTask StartAsync( var sessionId = $"session-{Guid.NewGuid():N}"; ReplSessionIO.EnsureSession(sessionId); var app = appFactory(); - var runOptions = descriptor.BuildRunOptions(options); - var serviceCollection = new ServiceCollection(); - serviceCollection.AddSingleton(); - var services = serviceCollection.BuildServiceProvider(); + // The handle owns the session's DI scope (each command is a separate one-shot run), + // so every run opts out of per-run scoping. + var runOptions = descriptor.BuildRunOptions(options) with { SessionScope = SessionScopeBehavior.CallerOwned }; + var services = SessionScopedTestServices.Create(app); var handle = new ReplSessionHandle(owner, app, options, services, runOptions, descriptor.Answers, sessionId); return ValueTask.FromResult(handle); } + // One logical test session spans several one-shot Run* calls, so the handle owns the + // session's DI scope itself: the provider is built over a scope of the APP's services + // (user registrations become visible to session commands, Scoped instances are shared + // across the session's commands) and every run uses SessionScopeBehavior.CallerOwned so + // the Run* entry points do not open a fresh scope — and fresh Scoped instances — per + // command. Note: the IReplSessionState mask below only intercepts direct GetService + // lookups; a user service constructor-injecting IReplSessionState inside the scope gets + // the app-registered implementation instead (pre-existing test-host limitation). + private sealed class SessionScopedTestServices : IServiceProvider, IAsyncDisposable + { + private readonly AsyncServiceScope? _scope; + private readonly IServiceProvider _inner; + private readonly InMemorySessionState _sessionState = new(); + + private SessionScopedTestServices(AsyncServiceScope? scope, IServiceProvider inner) + { + _scope = scope; + _inner = inner; + } + + public static SessionScopedTestServices Create(ReplApp app) + { + var root = app.Services; + if (root.GetService(typeof(IServiceScopeFactory)) is IServiceScopeFactory scopeFactory) + { + var scope = scopeFactory.CreateAsyncScope(); + return new SessionScopedTestServices(scope, scope.ServiceProvider); + } + + return new SessionScopedTestServices(scope: null, root); + } + + public object? GetService(Type serviceType) => + serviceType == typeof(IReplSessionState) ? _sessionState : _inner.GetService(serviceType); + + public ValueTask DisposeAsync() => _scope?.DisposeAsync() ?? ValueTask.CompletedTask; + } + private static string[] BuildArgsWithAnswers( List baseTokens, IReadOnlyDictionary? sessionAnswers,