From 6719e67c12f5e6170c68b3e7f1495f85d194f24e Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Tue, 13 Oct 2026 00:45:23 +0200 Subject: [PATCH 1/3] Add Nuplane path-independent participation support --- Directory.Packages.props | 6 +- .../contracts/CShells.Nuplane.md | 6 +- .../Internal/NuplaneRefreshCoordinator.cs | 2 +- src/CShells.Nuplane/README.md | 4 +- src/Directory.Build.props | 4 +- .../DynamicFeature.cs | 5 + .../Nuplane/NuplaneCompositionTests.cs | 1 + .../Nuplane/NuplaneRefreshCoordinatorTests.cs | 122 ++++++++++++++++++ 8 files changed, 139 insertions(+), 11 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index cb89c1b..e1a6277 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -42,9 +42,9 @@ - - - + + + diff --git a/specs/021-nuplane-adapter/contracts/CShells.Nuplane.md b/specs/021-nuplane-adapter/contracts/CShells.Nuplane.md index b0345bd..9728ae2 100644 --- a/specs/021-nuplane-adapter/contracts/CShells.Nuplane.md +++ b/specs/021-nuplane-adapter/contracts/CShells.Nuplane.md @@ -2,7 +2,7 @@ ## Dependencies -The adapter package references `CShells`, `CShells.Abstractions`, `Nuplane.Abstractions`, and `Nuplane.Loading.Abstractions`. Nuplane dependencies use central interim version `0.0.11-preview.99`. The package does not reference Nuplane runtime/loading implementations or Elsa. +The adapter package references `CShells`, `CShells.Abstractions`, `Nuplane.Abstractions`, and `Nuplane.Loading.Abstractions`. This release candidate pins the Nuplane family to `0.0.12`; stable package availability is an external prerequisite for package restore and final qualification. The package does not reference Nuplane runtime/loading implementations or Elsa. ## Feature assembly provider @@ -75,9 +75,9 @@ The adapter does not format host-specific errors. A host callback may recognize ## Registration and ordering guarantees -- The adapter appends its own `INuplaneObserver` factory after the loader observer. Existing observer descriptors remain in place and are not replaced. +- The adapter appends its own `INuplaneObserver` factory after the loader observer. The coordinator implements `IPackagePathIndependentNuplaneObserver` because every lifecycle callback uses change metadata and the loaded-assembly catalog; it never opens or retains package paths. Existing observer descriptors remain in place and are not replaced. - Root observer and root `IShellGenerationBuildParticipant` resolve one coordinator instance. - Only the private, nonparticipant `CoordinatorHolder` is selected by `ShareSingletonWithShells()`. - The participant concrete and interface descriptors stay root-only. Shell observer aliases resolve the root coordinator through the shared holder. - Registry resolution is lazy inside observer work through a root `Func`; the coordinator constructor does not resolve the registry. -- The no-op build lease provides no assembly-readability or generation-lifetime protection. +- The no-op build lease provides no separate assembly-readability or generation-lifetime protection. Runtime catalog snapshots and shell providers retain loaded assemblies and feature types while needed; a serving or draining generation therefore keeps its collectible load context alive until the generation is disposed. Nuplane retains ownership of package paths and graph-use lifetime. diff --git a/src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs b/src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs index 89f389f..1276af1 100644 --- a/src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs +++ b/src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs @@ -12,7 +12,7 @@ internal sealed class NuplaneRefreshCoordinator( IRuntimeFeatureCatalog featureCatalog, IOptionsMonitor options, Func shellRegistryFactory, - ILogger? logger = null) : INuplaneObserver, IShellGenerationBuildParticipant + ILogger? logger = null) : IPackagePathIndependentNuplaneObserver, IShellGenerationBuildParticipant { private readonly IRuntimeFeatureCatalog catalog = featureCatalog ?? throw new ArgumentNullException(nameof(featureCatalog)); private readonly IOptionsMonitor optionsMonitor = options ?? throw new ArgumentNullException(nameof(options)); diff --git a/src/CShells.Nuplane/README.md b/src/CShells.Nuplane/README.md index 0e3a7cb..4edb67c 100644 --- a/src/CShells.Nuplane/README.md +++ b/src/CShells.Nuplane/README.md @@ -26,7 +26,7 @@ shells.WithNuplaneFeatureDiscovery(); `AddNuplane` accepts a configuration callback and returns `IServiceCollection`. Calling `WithNuplaneFeatureDiscovery` afterward appends the CShells adapter observer after Nuplane's autoloader and existing observer registrations. The adapter uses one root coordinator for both Nuplane observation and CShells build participation. Do not register another coordinator through `OnPackagesChanged()`. -The provider queries `IPackageAssemblyCatalog` and contributes each `PackageAssemblies.Assemblies` collection in package order. These are already-loaded `Assembly` instances. The adapter does not read `AssemblyReferences`, load packages, or activate them. Adding the package alone does not replace CShells' current feature sources; the host opts in by calling `WithNuplaneFeatureDiscovery`. +The provider queries `IPackageAssemblyCatalog` and contributes each `PackageAssemblies.Assemblies` collection in package order. These are already-loaded `Assembly` instances. The adapter does not read `AssemblyReferences`, load packages, or activate them. Its coordinator implements Nuplane's `IPackagePathIndependentNuplaneObserver` contract because its lifecycle callbacks never open or retain package paths. Adding the package alone does not replace CShells' current feature sources; the host opts in by calling `WithNuplaneFeatureDiscovery`. ## Choose refresh and reload behavior @@ -77,7 +77,7 @@ As with the framework's other logging consumers, exception-preserving diagnostic When a package completion arrives with no active shell, the adapter records catalog freshness and returns without scanning assemblies, activating a shell, or reloading. The next requested shell build consumes pending freshness before CShells initializes or reads the feature catalog. A successful refresh is committed even if later shell initialization fails, so the next build can reuse that fresh snapshot. -Refresh requests and acknowledgements use separate epochs. A failed or cancelled refresh remains pending. A source change arriving during refresh remains outstanding after the in-flight refresh commits its captured epoch. The build participant returns a no-op lease and does not provide assembly-readability, load-context, or generation-lifetime protection. +Refresh requests and acknowledgements use separate epochs. A failed or cancelled refresh remains pending. A source change arriving during refresh remains outstanding after the in-flight refresh commits its captured epoch. The build participant returns a no-op lease; it contributes no separate lifetime protection. The runtime catalog and each shell provider retain the discovered feature types they use. After package removal, a serving generation can continue resolving its existing feature types while it drains; its references keep the collectible load context alive until the generation is disposed. Nuplane remains responsible for package paths and any graph-use lifetime attached to that load context. ## Report host-specific reload failures diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 900cc5d..9e98b1e 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -13,7 +13,7 @@ $(NoWarn);CS1591 - 0.0.30 + 0.0.31 Sipke Schoorstra @@ -48,4 +48,4 @@ - \ No newline at end of file + diff --git a/tests/CShells.DynamicFeatureFixture/DynamicFeature.cs b/tests/CShells.DynamicFeatureFixture/DynamicFeature.cs index 2761bce..a6d46a6 100644 --- a/tests/CShells.DynamicFeatureFixture/DynamicFeature.cs +++ b/tests/CShells.DynamicFeatureFixture/DynamicFeature.cs @@ -16,6 +16,7 @@ public sealed class DynamicFeature : IWebShellFeature /// public void ConfigureServices(IServiceCollection services) { + services.AddSingleton(); } /// @@ -26,3 +27,7 @@ public void MapEndpoints(IEndpointRouteBuilder endpoints, IHostEnvironment? envi private static IResult HandleRequest() => Results.Ok("collectible-feature"); } + +public sealed class DynamicFeatureMarker +{ +} diff --git a/tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs b/tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs index 629ee35..40409bd 100644 --- a/tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs +++ b/tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs @@ -170,6 +170,7 @@ public async Task RootAndShellResolveTheSameCoordinatorRegardlessOfFirstAlias(bo } Assert.Same(observer, participant); + Assert.IsAssignableFrom(observer); var shell = await host.GetRequiredService().ActivateAsync("aliases"); Assert.Same(observer, shell.ServiceProvider.GetServices().Last()); Assert.Null(shell.ServiceProvider.GetService()); diff --git a/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs b/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs index e98b249..2955227 100644 --- a/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs +++ b/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs @@ -1,3 +1,7 @@ +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.Loader; +using CShells.Features; using CShells.Lifecycle; using CShells.Nuplane; using CShells.Nuplane.Internal; @@ -9,6 +13,17 @@ namespace CShells.Tests.Integration.Nuplane; public sealed class NuplaneRefreshCoordinatorTests { + [Fact] + public async Task RemovedPackageAssembly_RemainsAvailableToDrainingShellUntilItsScopeEnds() + { + var fixturePath = Path.Combine(AppContext.BaseDirectory, "CShells.DynamicFeatureFixture.dll"); + Assert.True(File.Exists(fixturePath), $"Dynamic feature fixture was not copied to '{fixturePath}'."); + + var (packageCatalog, loadContextReference) = CreateCollectiblePackageCatalog(fixturePath); + await RemovePackageWhileServingShellDrainsAsync(packageCatalog, loadContextReference); + await AssertCollectibleAsync(loadContextReference); + } + [Fact] public async Task ColdChangedWorkIsDeferredUntilBuildAndUnchangedCallbackDoesNotReloadFirstShell() { @@ -447,4 +462,111 @@ private static (NuplaneRefreshCoordinator Coordinator, TestRuntimeFeatureCatalog return (coordinator, catalog, registryState); } + [MethodImpl(MethodImplOptions.NoInlining)] + private static (FakePackageAssemblyCatalog Catalog, WeakReference LoadContextReference) CreateCollectiblePackageCatalog( + string fixturePath) + { + var loadContext = new CollectibleFeatureLoadContext(); + var loadContextReference = new WeakReference(loadContext, trackResurrection: true); + var assembly = loadContext.LoadFromAssemblyPath(fixturePath); + return (new FakePackageAssemblyCatalog + { + Packages = [FakePackageAssemblyCatalog.CreatePackage("dynamic-feature", assembly)] + }, loadContextReference); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static async Task RemovePackageWhileServingShellDrainsAsync( + FakePackageAssemblyCatalog packageCatalog, + WeakReference loadContextReference) + { + await using var host = NuplaneTestHost.Build( + services => services.AddSingleton(packageCatalog), + shells => shells.WithNuplaneFeatureDiscovery(options => options.AutoReload = true) + .AddShell("collectible-package", shell => shell.WithFeature("Dynamic"))); + var registry = host.GetRequiredService(); + var oldShell = await registry.ActivateAsync("collectible-package"); + AssertShellUsesDynamicFeature(oldShell); + + var oldScope = oldShell.BeginScope(); + try + { + packageCatalog.Packages = []; + await host.GetRequiredService().OnPackagesReconciledAsync( + FakePackageAssemblyCatalog.ChangeSet(removed: ["dynamic-feature"]), [], CancellationToken.None); + + var activeShell = registry.GetActive("collectible-package"); + Assert.NotNull(activeShell); + Assert.NotSame(oldShell, activeShell); + Assert.Equal(ShellLifecycleState.Active, activeShell!.State); + Assert.Equal(ShellLifecycleState.Draining, oldShell.State); + Assert.True(loadContextReference.IsAlive, + "The collectible package context must remain alive while its shell generation still serves an open scope."); + + var retainedFeature = AssertRetainedDynamicFeature(oldScope, activeShell); + Assert.Equal("DynamicFeature", retainedFeature.Name); + var markerType = retainedFeature.Assembly.GetType( + "CShells.DynamicFeatureFixture.DynamicFeatureMarker", throwOnError: true)!; + Assert.NotNull(oldScope.ServiceProvider.GetService(markerType)); + Assert.Null(activeShell.ServiceProvider.GetService(markerType)); + + var oldDrain = Assert.IsAssignableFrom(oldShell.Drain); + await oldScope.DisposeAsync(); + var drainResult = await oldDrain.WaitAsync().WaitAsync(TimeSpan.FromSeconds(5)); + Assert.Equal(DrainStatus.Completed, drainResult.Status); + Assert.Equal(ShellLifecycleState.Disposed, oldShell.State); + } + finally + { + await oldScope.DisposeAsync(); + } + + var loadContext = Assert.IsAssignableFrom(loadContextReference.Target); + loadContext.Unload(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void AssertShellUsesDynamicFeature(IShell shell) + { + var descriptors = shell.ServiceProvider.GetRequiredService>(); + Assert.Contains(descriptors, descriptor => descriptor.Id == "Dynamic"); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static Type AssertRetainedDynamicFeature(IShellScope oldScope, IShell activeShell) + { + var oldDescriptors = oldScope.ServiceProvider.GetRequiredService>(); + var retainedType = Assert.Single(oldDescriptors, descriptor => descriptor.Id == "Dynamic").StartupType; + Assert.NotNull(retainedType); + + var activeDescriptors = activeShell.ServiceProvider.GetRequiredService>(); + Assert.DoesNotContain(activeDescriptors, descriptor => descriptor.Id == "Dynamic"); + return retainedType!; + } + + private static async Task AssertCollectibleAsync(WeakReference loadContextReference) + { + for (var attempt = 0; attempt < 100 && loadContextReference.IsAlive; attempt++) + { + GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, blocking: true); + GC.WaitForPendingFinalizers(); + GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, blocking: true); + await Task.Yield(); + } + + Assert.False(loadContextReference.IsAlive, + "The removed package context remained rooted after the old shell generation drained and disposed."); + } + + private sealed class CollectibleFeatureLoadContext : AssemblyLoadContext + { + public CollectibleFeatureLoadContext() + : base($"CShells.Nuplane.DynamicFeature-{Guid.NewGuid():N}", isCollectible: true) + { + } + + protected override Assembly? Load(AssemblyName assemblyName) => + AssemblyLoadContext.Default.LoadFromAssemblyName(assemblyName); + } + } From d80dbb31fd56eb12024e77c75886b9498db8e05c Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Tue, 13 Oct 2026 01:14:31 +0200 Subject: [PATCH 2/3] Prove draining shell retains an unloading package context --- .../Nuplane/NuplaneRefreshCoordinatorTests.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs b/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs index 2955227..c1aa993 100644 --- a/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs +++ b/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs @@ -492,6 +492,7 @@ private static async Task RemovePackageWhileServingShellDrainsAsync( try { packageCatalog.Packages = []; + RequestCollectibleUnload(loadContextReference); await host.GetRequiredService().OnPackagesReconciledAsync( FakePackageAssemblyCatalog.ChangeSet(removed: ["dynamic-feature"]), [], CancellationToken.None); @@ -500,6 +501,7 @@ await host.GetRequiredService().OnPackagesReconciledAsync( Assert.NotSame(oldShell, activeShell); Assert.Equal(ShellLifecycleState.Active, activeShell!.State); Assert.Equal(ShellLifecycleState.Draining, oldShell.State); + CollectForUnload(); Assert.True(loadContextReference.IsAlive, "The collectible package context must remain alive while its shell generation still serves an open scope."); @@ -521,10 +523,12 @@ await host.GetRequiredService().OnPackagesReconciledAsync( await oldScope.DisposeAsync(); } - var loadContext = Assert.IsAssignableFrom(loadContextReference.Target); - loadContext.Unload(); } + [MethodImpl(MethodImplOptions.NoInlining)] + private static void RequestCollectibleUnload(WeakReference loadContextReference) + => Assert.IsAssignableFrom(loadContextReference.Target).Unload(); + [MethodImpl(MethodImplOptions.NoInlining)] private static void AssertShellUsesDynamicFeature(IShell shell) { @@ -548,9 +552,7 @@ private static async Task AssertCollectibleAsync(WeakReference loadContextRefere { for (var attempt = 0; attempt < 100 && loadContextReference.IsAlive; attempt++) { - GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, blocking: true); - GC.WaitForPendingFinalizers(); - GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, blocking: true); + CollectForUnload(); await Task.Yield(); } @@ -558,6 +560,13 @@ private static async Task AssertCollectibleAsync(WeakReference loadContextRefere "The removed package context remained rooted after the old shell generation drained and disposed."); } + private static void CollectForUnload() + { + GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, blocking: true); + GC.WaitForPendingFinalizers(); + GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, blocking: true); + } + private sealed class CollectibleFeatureLoadContext : AssemblyLoadContext { public CollectibleFeatureLoadContext() From 545d6e5a6cfc99d071de40bb8fffa5df1cfdc75f Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 15 Oct 2026 10:07:45 +0200 Subject: [PATCH 3/3] Refresh coordinated feature catalogs after Nuplane loading --- .../contracts/CShells.Nuplane.md | 2 +- .../Internal/NuplaneRefreshCoordinator.cs | 53 +++++++++++++++++-- .../NuplaneFeatureAssemblyProvider.cs | 5 +- src/CShells.Nuplane/README.md | 6 +-- .../Nuplane/NuplaneCompositionTests.cs | 3 +- .../Nuplane/NuplaneRefreshCoordinatorTests.cs | 52 ++++++++++++++++++ 6 files changed, 111 insertions(+), 10 deletions(-) diff --git a/specs/021-nuplane-adapter/contracts/CShells.Nuplane.md b/specs/021-nuplane-adapter/contracts/CShells.Nuplane.md index 9728ae2..156fb14 100644 --- a/specs/021-nuplane-adapter/contracts/CShells.Nuplane.md +++ b/specs/021-nuplane-adapter/contracts/CShells.Nuplane.md @@ -75,7 +75,7 @@ The adapter does not format host-specific errors. A host callback may recognize ## Registration and ordering guarantees -- The adapter appends its own `INuplaneObserver` factory after the loader observer. The coordinator implements `IPackagePathIndependentNuplaneObserver` because every lifecycle callback uses change metadata and the loaded-assembly catalog; it never opens or retains package paths. Existing observer descriptors remain in place and are not replaced. +- The adapter appends its own `INuplaneObserver` factory after the loader observer. The coordinator implements `IPostLoadingNuplaneObserver`. Its scoped callbacks are no-ops during coordinated Phase A; it refreshes the catalog and optionally reloads shells from the awaited post-loading callback, after root admission and store-lock release and after Nuplane loads leased graphs. On the ordinary non-coordinated route, `OnPackagesReconciledAsync` retains its existing behavior. The coordinator does not claim package-path independence because its refresh reads the active package catalog. - Root observer and root `IShellGenerationBuildParticipant` resolve one coordinator instance. - Only the private, nonparticipant `CoordinatorHolder` is selected by `ShareSingletonWithShells()`. - The participant concrete and interface descriptors stay root-only. Shell observer aliases resolve the root coordinator through the shared holder. diff --git a/src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs b/src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs index 1276af1..49f3bc4 100644 --- a/src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs +++ b/src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Nuplane.Abstractions; +using Nuplane.Abstractions.PackageStoreProtection; namespace CShells.Nuplane.Internal; @@ -12,7 +13,7 @@ internal sealed class NuplaneRefreshCoordinator( IRuntimeFeatureCatalog featureCatalog, IOptionsMonitor options, Func shellRegistryFactory, - ILogger? logger = null) : IPackagePathIndependentNuplaneObserver, IShellGenerationBuildParticipant + ILogger? logger = null) : IPostLoadingNuplaneObserver, IShellGenerationBuildParticipant { private readonly IRuntimeFeatureCatalog catalog = featureCatalog ?? throw new ArgumentNullException(nameof(featureCatalog)); private readonly IOptionsMonitor optionsMonitor = options ?? throw new ArgumentNullException(nameof(options)); @@ -34,10 +35,56 @@ internal sealed class NuplaneRefreshCoordinator( public Task OnPackageFailedAsync(string packageId, Exception exception, CancellationToken cancellationToken) => Task.CompletedTask; - public async Task OnPackagesReconciledAsync( + public Task OnPackagesReconciledAsync( PackageChangeSet changeSet, IReadOnlyList appliedPackages, CancellationToken cancellationToken) + => ProcessCompletionAsync( + changeSet, + appliedPackages, + nameof(INuplaneObserver.OnPackagesReconciledAsync), + cancellationToken); + + public Task OnPackagesChangingAsync( + PackageChangeSet changeSet, + PackageStoreOperationBorrow borrow, + CancellationToken cancellationToken) => Task.CompletedTask; + + public Task OnPackagesChangedAsync( + PackageChangeSet changeSet, + PackageStoreOperationBorrow borrow, + CancellationToken cancellationToken) => Task.CompletedTask; + + public Task OnPackageFailedAsync( + string packageId, + Exception exception, + PackageStoreOperationBorrow borrow, + CancellationToken cancellationToken) => Task.CompletedTask; + + /// + /// Participating reconciliation deliberately does no refresh while its root admission and store lock are held. + /// The post-loading callback runs after those locks are released and after Nuplane has loaded leased graphs. + /// + public Task OnPackagesReconciledAsync( + PackageChangeSet changeSet, + IReadOnlyList appliedPackages, + PackageStoreOperationBorrow borrow, + CancellationToken cancellationToken) => Task.CompletedTask; + + public Task OnPackageLoadingCompletedAsync( + PackageChangeSet changeSet, + IReadOnlyList appliedPackages, + CancellationToken cancellationToken) => ProcessCompletionAsync( + changeSet, + appliedPackages, + nameof(IPostLoadingNuplaneObserver.OnPackageLoadingCompletedAsync), + cancellationToken); + + private async Task ProcessCompletionAsync( + PackageChangeSet changeSet, + IReadOnlyList appliedPackages, + string operationName, + CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(changeSet); ArgumentNullException.ThrowIfNull(appliedPackages); @@ -88,7 +135,7 @@ public async Task OnPackagesReconciledAsync( loggerInstance.LogError( exception, "Nuplane reconciliation operation {Operation} failed for {CorrelationId}.", - nameof(INuplaneObserver.OnPackagesReconciledAsync), + operationName, changeSet.CorrelationId); throw; } diff --git a/src/CShells.Nuplane/NuplaneFeatureAssemblyProvider.cs b/src/CShells.Nuplane/NuplaneFeatureAssemblyProvider.cs index c6ead4c..4cf623e 100644 --- a/src/CShells.Nuplane/NuplaneFeatureAssemblyProvider.cs +++ b/src/CShells.Nuplane/NuplaneFeatureAssemblyProvider.cs @@ -8,8 +8,9 @@ namespace CShells.Nuplane; /// Contributes assemblies Nuplane has already loaded for active packages to CShells feature discovery. /// /// -/// The provider returns loaded instances in package-catalog order. It does not load -/// package files or use . +/// The provider returns loaded instances in package-catalog order. The catalog owns its +/// active/load-state and package-data lookups; this provider itself does not open package files or use +/// . /// public sealed class NuplaneFeatureAssemblyProvider(IPackageAssemblyCatalog packageAssemblyCatalog) : IFeatureAssemblyProvider { diff --git a/src/CShells.Nuplane/README.md b/src/CShells.Nuplane/README.md index 4edb67c..94796d6 100644 --- a/src/CShells.Nuplane/README.md +++ b/src/CShells.Nuplane/README.md @@ -26,7 +26,7 @@ shells.WithNuplaneFeatureDiscovery(); `AddNuplane` accepts a configuration callback and returns `IServiceCollection`. Calling `WithNuplaneFeatureDiscovery` afterward appends the CShells adapter observer after Nuplane's autoloader and existing observer registrations. The adapter uses one root coordinator for both Nuplane observation and CShells build participation. Do not register another coordinator through `OnPackagesChanged()`. -The provider queries `IPackageAssemblyCatalog` and contributes each `PackageAssemblies.Assemblies` collection in package order. These are already-loaded `Assembly` instances. The adapter does not read `AssemblyReferences`, load packages, or activate them. Its coordinator implements Nuplane's `IPackagePathIndependentNuplaneObserver` contract because its lifecycle callbacks never open or retain package paths. Adding the package alone does not replace CShells' current feature sources; the host opts in by calling `WithNuplaneFeatureDiscovery`. +The provider invokes `IPackageAssemblyCatalog` and contributes each returned `PackageAssemblies.Assemblies` collection in package order. Catalog queries may inspect active state and installed package data, and may materialize or load additional assemblies from active installs under Nuplane's coordinated admission; obtaining the returned `Assembly` instances is not a memory-only read. The CShells provider delegates those reads to the catalog and does not directly open package files or read `AssemblyReferences`, or independently manage package loading and activation. Its coordinator implements Nuplane's `IPostLoadingNuplaneObserver`: coordinated reconciliation callbacks do no catalog work while the root admission and store lock are held, then the post-loading callback refreshes the catalog and optionally reloads shells after Nuplane has loaded the leased graphs. Ordinary non-coordinated observer callbacks retain their existing refresh behavior. Adding the package alone does not replace CShells' current feature sources; the host opts in by calling `WithNuplaneFeatureDiscovery`. ## Choose refresh and reload behavior @@ -61,7 +61,7 @@ shells.WithNuplaneFeatureDiscovery(options => }); ``` -An eligible completion is a delivered `OnPackagesReconciledAsync` callback with at least one successfully applied package or a committed removal. `ChangedOrPending` scans after additions, updates, removals, or a previous failed/deferred refresh. An unchanged completion with a fresh catalog and no pending reload does no work. `EveryEligibleCompletion` scans each eligible delivery, including unchanged successful applications. +An eligible completion has at least one successfully applied package or a committed removal. In coordinated reconciliation, refresh and reload run from the post-loading callback after Nuplane finishes loading leased graphs; otherwise they run from `OnPackagesReconciledAsync`. `ChangedOrPending` scans after additions, updates, removals, or a previous failed/deferred refresh. An unchanged completion with a fresh catalog and no pending reload does no work. `EveryEligibleCompletion` scans each eligible delivery, including unchanged successful applications. `Enabled = false` suppresses new observer-driven refresh and reload requests. It does not disable the assembly provider's normal query when CShells initializes the feature catalog, and it does not discard freshness already pending for a later build. Reloading is independent: with `AutoReload = false`, a refresh updates catalog metadata while active shells keep their current generation. Disabling automatic reload pauses an already-pending failed reload; while disabled, new work does not create a reload request. A later eligible delivery with automatic reload enabled may retry retained failed reload work. @@ -69,7 +69,7 @@ The adapter reads the current options once for each eligible reconciliation deli ## Diagnose observer operation failures -When logging is registered, an ordinary nonfatal failure from admitted reconciliation work produces one adapter `Error` record with the original exception, the `PackageChangeSet.CorrelationId`, and the `OnPackagesReconciledAsync` operation name. The adapter then rethrows the same exception so Nuplane retains its observer warning and continuation behavior. Requested-token cancellation, the documented fatal exceptions, build-time refresh failures, and returned per-shell `ReloadResult.Error` values remain outside this diagnostic boundary. With no logging provider, the adapter uses `NullLogger` and preserves the same failure propagation. +When logging is registered, an ordinary nonfatal failure from an observer-driven refresh or reload produces one adapter `Error` record with the original exception, the `PackageChangeSet.CorrelationId`, and the operation name of the callback that performed the work (`OnPackagesReconciledAsync` or `OnPackageLoadingCompletedAsync`). The adapter then rethrows the same exception so Nuplane retains its observer warning and continuation behavior. Requested-token cancellation, the documented fatal exceptions, build-time refresh failures, and returned per-shell `ReloadResult.Error` values remain outside this diagnostic boundary. With no logging provider, the adapter uses `NullLogger` and preserves the same failure propagation. As with the framework's other logging consumers, exception-preserving diagnostics assume registered logging providers do not throw from `ILogger.Log`; logger-provider failures are outside this adapter contract. diff --git a/tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs b/tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs index 40409bd..54b3b46 100644 --- a/tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs +++ b/tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs @@ -170,7 +170,8 @@ public async Task RootAndShellResolveTheSameCoordinatorRegardlessOfFirstAlias(bo } Assert.Same(observer, participant); - Assert.IsAssignableFrom(observer); + Assert.IsAssignableFrom(observer); + Assert.False(observer is IPackagePathIndependentNuplaneObserver); var shell = await host.GetRequiredService().ActivateAsync("aliases"); Assert.Same(observer, shell.ServiceProvider.GetServices().Last()); Assert.Null(shell.ServiceProvider.GetService()); diff --git a/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs b/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs index c1aa993..a372210 100644 --- a/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs +++ b/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs @@ -6,7 +6,9 @@ using CShells.Nuplane; using CShells.Nuplane.Internal; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Nuplane.Abstractions; +using Nuplane.Abstractions.PackageStoreProtection; using Nuplane.Loading; namespace CShells.Tests.Integration.Nuplane; @@ -208,6 +210,56 @@ public async Task FailedRefreshRetainsFreshnessForAnUnchangedEligibleRetry() Assert.Single(loggerProvider.Snapshot()); } + [Fact] + public async Task CoordinatedRefreshWaitsForPostLoadingAndRetriesPendingRefresh() + { + var refreshFailure = new InvalidOperationException("first post-loading refresh failed"); + var loggerProvider = new NuplaneTestLoggerProvider(); + var catalog = new TestRuntimeFeatureCatalog + { + RefreshHandler = (number, _) => number == 1 + ? Task.FromException(refreshFailure) + : Task.CompletedTask + }; + var registry = TestShellRegistry.Create(out var registryState); + var coordinator = new NuplaneRefreshCoordinator( + catalog, + new TestOptionsMonitor(new NuplaneIntegrationOptions { AutoReload = true }), + () => registry, + loggerProvider.CreateTypedLogger()); + var postLoadingObserver = Assert.IsAssignableFrom(coordinator); + var applied = FakePackageAssemblyCatalog.Resolved("feature-package"); + var changed = new PackageChangeSet([applied], [], [], "post-loading-correlation", DateTimeOffset.UtcNow); + + await coordinator.OnPackagesChangingAsync(changed, null!, CancellationToken.None); + await coordinator.OnPackagesChangedAsync(changed, null!, CancellationToken.None); + await coordinator.OnPackageFailedAsync("failed-package", new InvalidOperationException("ignored"), null!, CancellationToken.None); + await coordinator.OnPackagesReconciledAsync(changed, [applied], null!, CancellationToken.None); + + Assert.Equal(0, catalog.RefreshCount); + Assert.Equal(0, registryState.ActiveShellReadCount); + Assert.Equal(0, registryState.ReloadCount); + + var firstFailure = await Assert.ThrowsAsync(() => + postLoadingObserver.OnPackageLoadingCompletedAsync(changed, [applied], CancellationToken.None)); + + Assert.Same(refreshFailure, firstFailure); + Assert.Equal(1, catalog.RefreshCount); + Assert.Equal(0, registryState.ReloadCount); + var error = Assert.Single(loggerProvider.Snapshot()); + Assert.Equal(LogLevel.Error, error.Level); + Assert.Same(refreshFailure, error.Exception); + Assert.Equal(nameof(IPostLoadingNuplaneObserver.OnPackageLoadingCompletedAsync), error.Properties["Operation"]); + Assert.Equal("post-loading-correlation", error.Properties["CorrelationId"]); + + var unchangedEligible = FakePackageAssemblyCatalog.ChangeSet(); + await postLoadingObserver.OnPackageLoadingCompletedAsync(unchangedEligible, [applied], CancellationToken.None); + + Assert.Equal(2, catalog.RefreshCount); + Assert.Equal(1, registryState.ReloadCount); + Assert.Single(loggerProvider.Snapshot()); + } + [Fact] public async Task BuildOnlyRefreshFailureRemainsUnloggedAndUnchanged() {