Merge pull request #74 from yllibed/dev/cdb/issue-70-session-scopes

fix(di): open one DI scope per session so Scoped services match the documented lifetime

Author: carldebilly

docs/architecture.md

@@ -115,6 +115,11 @@ The toolkit provides two application entry points for different scenarios.
 - Wraps `CoreReplApp` with `Microsoft.Extensions.DependencyInjection`
 - Provides `UseDefaultInteractive()`, `UseCliProfile()`, and other composition profiles
 - Lazily builds a shared `ServiceProvider` for module resolution and handler injection
+- Opens one DI scope per session: each `Run*` call is a session, so `Scoped` services resolve per
+  session and their disposables are released when it ends. `ReplRunOptions.SessionScope` set to
+  `SessionScopeBehavior.CallerOwned` opts out when the caller's provider already *is* the session scope
+  (a Blazor circuit, an ASP.NET request scope, a session owner spanning several one-shot runs).
+  Hosted services start outside that scope, because their lifetime is the application's
 - Best for: standalone CLI/REPL applications with service layers
 
 **`InvalidateRouting()`** — call this when module presence conditions may have changed at runtime (e.g., feature flags toggled, dynamic module discovery). Increments the routing cache version so the next execution re-evaluates all module presence predicates.

docs/best-practices.md

@@ -107,6 +107,52 @@ services.AddSingleton<ITenantClient>(sp =>
 
 Note: DI singleton factories are resolved lazily, so the values are available after global option parsing completes. However, singleton factories capture values once — in interactive mode, global options can change between commands. If your service needs to see updated values per command, inject `IGlobalOptionsAccessor` directly and read values at call time instead of capturing them in a factory. See [Commands — Accessing global options](commands.md#accessing-global-options-outside-handlers).
 
+## Pick the lifetime that matches the boundary
+
+| Lifetime | Resolves once per | Use it for |
+|---|---|---|
+| `Singleton` | application | caches, clients, anything shared by every session |
+| `Scoped` | **session** — one `Run*` call: a CLI invocation, an interactive run, a Telnet or WebSocket connection; under MCP, one client connection\* | per-user state: auth context, a cart, a unit of work |
+| `Transient` | resolution | cheap stateless helpers |
+
+A `Scoped` disposable is disposed when its session ends, so session resources do not accumulate for the
+life of a long-running host.
+
+\* **Except a server built from a reused `BuildMcpServerOptions()` result** — the shape ASP.NET Core and
+other custom-transport hosts use. That path has no per-connection context, so it has no per-connection DI
+scope either: one `Scoped` instance is shared by every client it serves, disposed only when the process
+ends. A `Scoped` auth-context or cart registered there leaks between clients. See the [known
+limitation](mcp-conformance.md#deliberate-gaps) and [Isolation boundaries](mcp-transports.md#what-is-isolated-and-at-which-boundary).
+
+Two concurrency notes that follow from the unit being the whole connection rather than one call: several
+MCP tool calls can run at once on one connection, so a `Scoped` instance can be entered concurrently —
+make it thread-safe, or serialize access to it yourself, the way a `DbContext` (a poor fit for concurrent
+entry) would need. And on `mcp serve`, that `Scoped` instance is exactly what carries state from one tool
+call to the next within a connection — the earlier calls in this section describing `Scoped` as
+resolving "once per session" are what makes that possible.
+
+The trap worth naming: a **singleton** that injects a `Scoped` service captures whichever scope first
+resolved it, and keeps it for the life of the application — silently, because the provider is built
+without `ValidateScopes`. Inject the scoped service into the handler instead, or register the holder
+`Scoped` too.
+
+The same trap catches a **module's own constructor**. `MapModule<TModule>()` resolves `TModule` once,
+at mapping time — before any session exists — the same way a singleton would. A module constructor
+that takes a `Scoped` auth-context or cart captures it exactly as the singleton case above does, and
+every session's handlers then share that one instance. The [module example above](#structure-commands-with-modules)
+avoids this because its handlers take `IContactStore` as a **handler parameter**, resolved fresh per
+invocation — not as a constructor dependency. Keep any `Scoped` service out of a module's constructor;
+inject it into the handler that needs it instead.
+
+When your caller's provider already represents the session — a Blazor circuit, an ASP.NET request scope,
+or a session owner running several one-shot calls — set
+`ReplRunOptions.SessionScope = SessionScopeBehavior.CallerOwned` so the run resolves from it directly
+instead of opening a second scope beside it. `IServiceScopeFactory` is itself a singleton, so that second
+scope is not nested inside the caller's — it is a SIBLING rooted at the application root, and it does not
+hide the caller's scoped instances so much as duplicate them: a second unit of work running alongside the
+live one. `SessionScopeBehavior` governs the `Run*` family only; MCP's connection-scoped DI is unconditional
+and has no equivalent opt-out.
+
 ## Group related options with `[ReplOptionsGroup]`
 
 When a command has many options, group them into a class instead of listing them all as handler parameters. This keeps handlers clean and makes option sets reusable across commands.

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` |
 

docs/glossary.md

@@ -56,7 +56,8 @@ Interface for packaging reusable command groups.
 
 ### IReplSessionState
 
-Per-session state container for interactive and hosted sessions.
+Per-session state container for interactive and hosted sessions. Registered `Scoped`, so each
+session gets its own — under MCP, each client connection.
 
 ### Literal segment
 

docs/mcp-conformance.md

@@ -129,6 +129,7 @@ returned by a creation tool and passed back as an argument, rather than implicit
 | --- | --- | --- |
 | No per-caller command graph | The one variance `2026-07-28` permits is by the authorization presented on the request. Repl has no request-authorization concept yet, so it advertises one graph to everyone. | [#97](https://github.com/yllibed/repl/issues/97) |
 | `*/list_changed` is advertised on the reusable-options path but never fires there | The SDK forces the flag true for any non-null collection, and the pre-built catalog always supplies one. | [#94](https://github.com/yllibed/repl/issues/94) |
+| A `Scoped` DI registration is shared by every client on the reusable-options path | That path has one handler-lifetime context serving all connections, so it has no per-connection scope to give `Scoped` — one instance lives for the process, same as the native roots cache and soft roots on the same path. A `Scoped` auth-context or cart registered there leaks between clients. See [best-practices.md](best-practices.md#pick-the-lifetime-that-matches-the-boundary). | — |
 | A multi-connection custom transport sees considerations this page does not solve | `mcp serve` is one connection per process; a host that multiplexes connections over one options instance owns the isolation questions that follow. See [Transports](mcp-transports.md). | — |
 
 ## Extensions and SEPs

docs/mcp-transports.md

@@ -102,10 +102,38 @@ every request rather than once per connection. So the boundaries are not all the
 |---|---|
 | Request | Client capabilities, the requested log level, the destination for sampling, elicitation and progress, and — on a reused `BuildMcpServerOptions()` result — native roots, resolved once per request and never cached across connections |
 | Invocation | I/O capture — each tool call gets its own capture scope, not one per connection |
-| Connection (`mcp serve` only) | The MCP session object, the native roots cache, soft roots, and session-aware routing state |
-
-A server created from a reused `BuildMcpServerOptions()` result has everything above except the
-connection row; see the known limitation above. That matters especially when using dynamic tools,
-roots, or session-specific modules.
+| Connection (`mcp serve` only) | The MCP session object, the native roots cache, soft roots, session-aware routing state, `IReplSessionState`, and **the DI scope**: a `Scoped` registration resolves once per connection, and its disposables are released when the connection ends |
+
+The DI scope is the connection's, for the same reason it is the session's on every other transport: a
+`Scoped` service is where per-user state lives, and state that resets between two calls of one client
+is not per-user state. It is also what keeps discovery and execution honest — both resolve a module
+presence predicate's services from the same scope, so a command that was advertised can be called.
+
+The MCP SDK opens a scope of its own per request (`McpServerOptions.ScopeRequests`, default `true`) and
+exposes it as `RequestContext.Services`. Repl does not run commands in it: that scope descends from the
+application root and is a sibling of the connection's, so adopting it would give the catalog and the
+command two different instances of the same service. `BuildDynamicServerOptions` — the `mcp serve` path
+that owns a real connection context — sets `ScopeRequests = false`, so the SDK does not open that
+sibling scope there to begin with.
+
+A server created from a reused `BuildMcpServerOptions()` result — which is how an ASP.NET Core or other
+custom-transport host reaches Repl — has everything above **except the connection row**: see the known
+limitation above. There is no per-connection context to give a DI scope to either, so `Scoped` resolves
+once from the application root and is shared by every client the process serves, for as long as it runs.
+That is true under stateless HTTP hosting too, even though the SDK itself builds one short-lived server
+per request there: Repl's own scope is a property of the *context* it built at construction, not of the
+SDK's per-request server, so it does not follow the SDK's request boundary.
+
+**Calling `BuildMcpServerOptions()` again for each connection does not isolate `Scoped` either.** The
+`ReplApp` overload always passes that app's one `Services` root (`app.Services`, cached for the app's
+whole lifetime), so two calls against the same `ReplApp` still share the same root — and therefore the
+same `Scoped` instances — no matter how often the method runs. Genuine per-connection isolation on this
+path requires the host to supply a **different** `IServiceProvider` per connection, through the
+`ICoreReplApp.BuildMcpServerOptions(configure, services)` overload: build a scope yourself (for example
+`app.Services.GetRequiredService<IServiceScopeFactory>().CreateAsyncScope()`), pass its
+`ServiceProvider` in for that connection, and dispose the scope yourself when the connection ends — Repl
+does not manage that scope's lifetime on this path, the same way it does not manage the connection
+itself. This is the "multiplexes connections over one options instance" case flagged as a caller
+responsibility below, made concrete.
 
 For those higher-level patterns, see [mcp-advanced.md](mcp-advanced.md).

src/Repl.Core/Session/InMemoryReplSessionState.cs

@@ -1,8 +1,14 @@
+using System.Collections.Concurrent;
+
 namespace Repl;
 
 internal sealed class InMemoryReplSessionState : IReplSessionState
 {
-	private readonly Dictionary<string, object?> _values = new(StringComparer.OrdinalIgnoreCase);
+	// Registered Scoped, so sessions no longer share an instance — but concurrent calls WITHIN one
+	// session still do: several MCP tool calls run at once on one connection, and a singleton that
+	// captured a scope hands its instance to every later caller. An unsynchronised Dictionary does
+	// not merely interleave under that, it corrupts its bucket table.
+	private readonly ConcurrentDictionary<string, object?> _values = new(StringComparer.OrdinalIgnoreCase);
 
 	public bool TryGet<T>(string key, out T? value)
 	{
@@ -33,7 +39,7 @@ public void Set<T>(string key, T value)
 	public bool Remove(string key)
 	{
 		ArgumentException.ThrowIfNullOrWhiteSpace(key);
-		return _values.Remove(key);
+		return _values.TryRemove(key, out _);
 	}
 
 	public void Clear() => _values.Clear();