From 3c1d8d1785ff239b13a8c7656a618b044ea7d4f7 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 8 Oct 2026 22:37:55 +0200 Subject: [PATCH 1/4] docs: specify live Nuplane integration options --- .specify/feature.json | 2 +- AGENTS.md | 2 +- .../checklists/requirements.md | 32 +++++++ .../contracts/options-policy.md | 35 +++++++ specs/023-live-nuplane-options/data-model.md | 26 +++++ specs/023-live-nuplane-options/plan.md | 94 +++++++++++++++++++ specs/023-live-nuplane-options/quickstart.md | 45 +++++++++ specs/023-live-nuplane-options/research.md | 59 ++++++++++++ specs/023-live-nuplane-options/spec.md | 92 ++++++++++++++++++ specs/023-live-nuplane-options/tasks.md | 67 +++++++++++++ 10 files changed, 452 insertions(+), 2 deletions(-) create mode 100644 specs/023-live-nuplane-options/checklists/requirements.md create mode 100644 specs/023-live-nuplane-options/contracts/options-policy.md create mode 100644 specs/023-live-nuplane-options/data-model.md create mode 100644 specs/023-live-nuplane-options/plan.md create mode 100644 specs/023-live-nuplane-options/quickstart.md create mode 100644 specs/023-live-nuplane-options/research.md create mode 100644 specs/023-live-nuplane-options/spec.md create mode 100644 specs/023-live-nuplane-options/tasks.md diff --git a/.specify/feature.json b/.specify/feature.json index 28fbc34..4baf4f0 100644 --- a/.specify/feature.json +++ b/.specify/feature.json @@ -1,3 +1,3 @@ { - "feature_directory": "specs/022-settled-readiness" + "feature_directory": "specs/023-live-nuplane-options" } diff --git a/AGENTS.md b/AGENTS.md index 700481a..4b6a0c8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -167,5 +167,5 @@ When asked to review or address PR review comments, think critically about each - 011-map-shell-config: Planned map-based shell configuration under `CShells:Shells` -For the active work unit, read `specs/022-settled-readiness/plan.md`. +For the active work unit, read `specs/023-live-nuplane-options/plan.md`. diff --git a/specs/023-live-nuplane-options/checklists/requirements.md b/specs/023-live-nuplane-options/checklists/requirements.md new file mode 100644 index 0000000..99a847d --- /dev/null +++ b/specs/023-live-nuplane-options/checklists/requirements.md @@ -0,0 +1,32 @@ +# Specification Quality Checklist: Apply Live Nuplane Integration Options + +**Purpose**: Validate specification completeness and quality before planning + +**Created**: 2026-10-08 + +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details in the user journeys or outcome requirements +- [x] Focused on host operator and integration-consumer value +- [x] Written in plain language with all mandatory specification sections complete + +## Requirement Completeness + +- [x] No unresolved clarification markers remain +- [x] Requirements and acceptance scenarios are testable and unambiguous +- [x] Success criteria are measurable and tied to observable behavior +- [x] Edge cases cover cancellation, in-flight policy changes, pending work, and invalid options +- [x] Scope, assumptions, compatibility, and external adoption boundaries are explicit + +## Feature Readiness + +- [x] Every functional requirement is covered by an acceptance scenario or edge case +- [x] The primary scenario can be tested independently through eligible completion deliveries +- [x] The specification distinguishes behavior preserved from runtime behavior being added +- [x] No Foundation, core-package, or pruning work is included in this feature + +## Notes + +The separately reviewed live-options requirements gate resolves the runtime option source and compatibility decisions. Its probe is evidence of the existing snapshot gap, not acceptance of an implementation. No `.specify/extensions.yml` hooks are installed. diff --git a/specs/023-live-nuplane-options/contracts/options-policy.md b/specs/023-live-nuplane-options/contracts/options-policy.md new file mode 100644 index 0000000..1da40c4 --- /dev/null +++ b/specs/023-live-nuplane-options/contracts/options-policy.md @@ -0,0 +1,35 @@ +# Contract: Per-delivery Nuplane integration options + +## Scope + +This is the behavioral contract for the existing optional `CShells.Nuplane` adapter. It adds no public API or configuration key. See [`NuplaneIntegrationOptions`](../../../src/CShells.Nuplane/NuplaneIntegrationOptions.cs) and the [adapter guide](../../../src/CShells.Nuplane/README.md) for public names and host examples. + +## Supported options source + +Supported configuration includes `WithNuplaneFeatureDiscovery(Action)`, standard options configuration/binding, and dependency-aware `services.AddOptions().Configure(...)`. A host with a custom options provider may register `IOptionsMonitor` explicitly. Replacing only `IOptions` is unsupported; the adapter does not combine monitor and static values or define precedence between them. + +## Delivery boundary + +For a delivered callback, null arguments, an already-canceled token, and ineligible empty work are checked first. Eligibility means at least one successfully applied package or at least one committed removal. These paths do not read the options monitor or mutate epochs or access the shell registry. + +For an eligible callback, read `IOptionsMonitor.CurrentValue` once. Before any epoch mutation, registry access, or await: + +1. copy `Enabled`, `RefreshTrigger`, `AutoReload`, and the `OnReloadResults` delegate reference to one private immutable operation value; +2. verify `RefreshTrigger` is a defined enum member and reject an undefined value with an actionable configuration error; +3. allow options-monitor and options-validation failures to propagate before coordinator side effects; no new wrapper or last-known-good fallback is required. + +All policy decisions and result reporting for the admitted delivery use these copied values. Changes after capture apply to later deliveries only. The synchronous field copy is not an atomic multi-property transaction against concurrent mutation of one options object. + +## Field behavior + +- `Enabled = false`: do not record new observer-requested work; retain existing freshness and reload epochs. Build-participant refreshes for previously recorded work are independent of this observer setting. +- `RefreshTrigger = ChangedOrPending`: preserve existing change and pending-freshness behavior. +- `RefreshTrigger = EveryEligibleCompletion`: preserve existing refresh behavior for every eligible delivery. +- `AutoReload = false`: freshness work may proceed, but do not attempt or acknowledge pending reload work. A later eligible delivery with reload enabled may retry retained work without rescanning an already-fresh catalog unless a newer package change requires refresh. +- `OnReloadResults`: invoke the callback captured by that delivery with the existing stable read-only copy of raw registry results. Preserve existing result evaluation, partial-error, cancellation, reentry, and retry behavior. + +The build participant does not read these observer options. It consumes already-recorded freshness before the catalog read and does not activate, reload, or report a shell. + +## Non-goals and limits + +The contract does not add an options-change listener, timer, background retry loop, new public flag, new package dependency, core-package policy, host composition, stable pin, package-readability guarantee, physical unloading, pruning, or Foundation host readiness policy. Those remain with their existing owners and acceptance gates. diff --git a/specs/023-live-nuplane-options/data-model.md b/specs/023-live-nuplane-options/data-model.md new file mode 100644 index 0000000..d8221a9 --- /dev/null +++ b/specs/023-live-nuplane-options/data-model.md @@ -0,0 +1,26 @@ +# Data Model: Apply Live Nuplane Integration Options + +## Captured integration policy + +An ephemeral value created once for an eligible, non-canceled package-reconciliation completion. It contains copies of: + +| Value | Source | Lifetime and use | +|---|---|---| +| Enabled | Current `NuplaneIntegrationOptions.Enabled` | Determines whether the delivery requests observer work. Disabled delivery adds no new epochs and leaves previous epochs unchanged. | +| Refresh trigger | Current `NuplaneIntegrationOptions.RefreshTrigger` | Selects the existing refresh behavior. Undefined enum values fail before coordinator side effects. | +| Automatic reload | Current `NuplaneIntegrationOptions.AutoReload` | Determines whether eligible work requests/attempts shell promotion. When false, already-pending promotion remains pending. | +| Reload-results callback | Current `NuplaneIntegrationOptions.OnReloadResults` delegate reference | The same captured callback receives results for this delivery. A later replacement applies to a later delivery. | + +The copy is made after null, cancellation, and empty-eligibility checks, and before epoch mutation, registry access, or awaits. Reading the current options once and copying each property does not promise atomicity against arbitrary concurrent mutation of the same options object during the copy. + +## Existing coordinator state + +The coordinator already tracks requested and committed catalog-freshness epochs and requested and committed reload epochs, including the catalog epoch associated with a reload request. Those counters, their synchronization, retry rules, and commit points remain unchanged. The captured policy is operation-local and is not retained as a second state machine. + +## Relationships + +- An eligible observer delivery may advance the existing catalog epoch according to its captured policy and package change set. +- A successful catalog refresh acknowledges only the freshness epoch captured by that refresh. +- Automatic reload is gated by the captured policy and current pending epochs; a successful reload acknowledges only work handled by that attempt. +- A shell-generation build participant consumes previously recorded catalog freshness without reading observer options or requesting activation/reporting. +- A settings change after one delivery captures its policy has no relationship to that in-flight value; it is observed by the next eligible delivery. diff --git a/specs/023-live-nuplane-options/plan.md b/specs/023-live-nuplane-options/plan.md new file mode 100644 index 0000000..6cf023e --- /dev/null +++ b/specs/023-live-nuplane-options/plan.md @@ -0,0 +1,94 @@ +# Implementation Plan: Apply Live Nuplane Integration Options + +**Branch**: `023-live-nuplane-options` | **Date**: 2026-10-08 | **Spec**: [spec.md](spec.md) + +**Input**: Feature specification from `/specs/023-live-nuplane-options/spec.md` + +## Summary + +Make the optional CShells.Nuplane coordinator consume current supported option values for each eligible package-reconciliation completion. Capture and copy one small immutable per-delivery policy before epoch mutation or asynchronous work, then carry those copied values through freshness, automatic reload, and reload-result reporting. Preserve current epochs, build participation, registration aliases, eligibility, reentry, and settlement behavior. + +## Technical Context + +**Language/Version**: C# 14; source packages target net8.0, net9.0, net10.0 + +**Primary Dependencies**: Existing `Microsoft.Extensions.Options`, `Microsoft.Extensions.Configuration`, and CShells/Nuplane abstractions; no new package + +**Storage**: Existing in-memory refresh and promotion epochs; no new persisted state + +**Testing**: xUnit in `tests/CShells.Tests`, net10.0; external public-package consumer on actual net8/net9/net10 after publication + +**Target Platform**: Existing supported .NET runtimes + +**Project Type**: Multi-package library; this work is confined to `src/CShells.Nuplane` + +**Performance Goals**: No new listener, background timer, queue, or per-delivery allocation beyond a small immutable value snapshot; no separate performance target + +**Constraints**: Snapshot current options once after callback validation/cancellation/eligibility checks and before epoch mutation or awaits; do not promise an atomic multi-property read against concurrent mutation of one options object; preserve current package-observer ordering and root/child ownership + +**Scale/Scope**: One internal adapter policy change, deterministic tests, adapter documentation, and required publication qualification; no CShells-core, Nuplane-core, Foundation host, or stable-package changes + +## Constitution Check + +- **I. Abstraction-first**: No consumer-extensibility API is added. Existing public `NuplaneIntegrationOptions` remains the contract; implementation remains in the optional adapter. +- **II. Feature modularity**: No feature composition, assembly discovery, or shell policy changes are introduced. +- **III. Modern C#**: Preserve nullable, language, package-target, and repository style conventions. +- **IV. Explicit errors**: An undefined `RefreshTrigger` gets an actionable configuration error before state/registry work. Monitor and options-validation failures propagate before that work without a new wrapping-error contract. +- **V. Test coverage**: Add deterministic adapter regressions for changes across deliveries, changes during an admitted operation, pending/deferred work, invalid settings, empty/canceled callbacks, and existing configuration paths. The repository requires tests; it does not declare test-first/TDD cadence for this feature. +- **VI. Simplicity**: Use one immutable private policy value and one `IOptionsMonitor` read per eligible callback. Add no public abstraction, second configuration source, background listener, timer, general settings framework, or new package dependency. +- **VII. Lifecycle and concurrency**: Keep epoch locks/gates and async sequencing as-is. Options are copied before awaits; no user callback or async operation is introduced while holding a new lock. + +Pre-design check passes. Recheck after design: no principle violation or exception is needed. + +## Design Decisions + +1. Resolve `IOptionsMonitor` in the root-owned coordinator and update the coordinator factory in `CShellsNuplaneBuilderExtensions.cs` to supply that monitor. Keep existing `Configure(Action)`, standard bind, and dependency-aware `Configure` paths. +2. For an eligible, non-canceled callback, read `CurrentValue` exactly once and synchronously copy `Enabled`, `RefreshTrigger`, `AutoReload`, and `OnReloadResults` into a private immutable value before mutating epochs or resolving the registry. Empty/failed-only and already-canceled callbacks return or cancel before reading options. +3. Validate that the copied refresh trigger is a defined enum member before any epoch or registry work and report an actionable configuration error for an invalid value. Let options-monitor and options-validation failures propagate before side effects; do not add last-known-good fallback or promise to wrap arbitrary provider errors. +4. Use the copied values throughout that callback, including callback invocation and result interpretation. Changes after capture apply to later deliveries and do not cancel or rewrite current work. +5. Keep existing epoch state authoritative. Disabled observer work creates no new epochs and leaves old ones intact. Automatic reload off may refresh catalog state but does not attempt or acknowledge pending reload work. A later enabled eligible delivery retries pending reload work without rescanning a fresh catalog unless a newer source change requires it. +6. Keep `BeginAsync` independent of observer options and retain existing catalog refresh-before-catalog-read ordering. Do not add an options-change listener or settings dependency to the build participant. +7. Treat direct replacement of `IOptions` alone as unsupported. Standard options configuration or explicit `IOptionsMonitor` registration is supported; do not add dual-source precedence or compatibility shims. + +See [research.md](research.md), [data-model.md](data-model.md), and [contracts/options-policy.md](contracts/options-policy.md) for source evidence and exact policy boundaries. + +## Design and Test Cadence + +The source repository's active constitution requires tests but does not impose a test-first cadence. Implement deterministic regression cases with the adapter change, then run the focused suite. Gates must be controlled with `TaskCompletionSource` and bounded joins, not sleeps. Existing tests that instantiate the internal coordinator with `IOptions` need a small shared test monitor or service-provider resolution through the real options system; do not retain production `IOptions` as a compatibility path. + +The critical snapshot proof holds catalog refresh after policy capture, changes values on the held options object and replaces the callback, then releases refresh. The admitted delivery must use its copied policy/callback; a subsequent eligible delivery must use the new values. Additional gates prove a later enabled delivery retries retained reload work without rescanning. Failed-only and pre-canceled callbacks must make zero monitor reads. Invalid enum/current-options failures must leave epoch/catalog/registry counters unchanged. + +## Project Structure + +```text +specs/023-live-nuplane-options/ + spec.md, plan.md, research.md, data-model.md, quickstart.md, tasks.md + checklists/requirements.md + contracts/options-policy.md +src/CShells.Nuplane/ + Internal/NuplaneRefreshCoordinator.cs + CShellsNuplaneBuilderExtensions.cs + README.md +tests/CShells.Tests/Integration/Nuplane/ + NuplaneRefreshCoordinatorTests.cs + NuplaneCompositionTests.cs + NuplaneCoordinatorTestHarness.cs or a narrowly scoped options-monitor helper +``` + +**Structure Decision**: Keep the implementation in the existing optional adapter and use existing Nuplane integration test fixtures. Avoid new projects, public options abstractions, core changes, or duplicate test harnesses. + +## Verification + +Run, once on the final implementation head: + +1. Focused `NuplaneRefreshCoordinatorTests`, `NuplaneCompositionTests`, and `NuplaneReloadResultsTests`. +2. Full `tests/CShells.Tests/CShells.Tests.csproj` suite. +3. Release builds of `src/CShells.Nuplane/CShells.Nuplane.csproj` for net8.0, net9.0, and net10.0. +4. A compiled freeze-options mutation that restores the old captured-settings behavior; the live-options regression must fail under the mutation and pass after byte-identical restoration. +5. Required owner-repository PR CI and main checks, then a complete ten-package actual-public-Feedz archive audit and a fresh-cache PackageReference-only options-change consumer on actual net8/net9/net10 runtimes. Record exact version, source commit, archive hashes, loaded DLL identities, commands, and outputs. Do not count local or private package preflight as public package evidence. + +Final stable release and Foundation host adoption/pins/locks are separate gates owned outside this feature. + +## Complexity Tracking + +No constitution exceptions or added complexity are proposed. diff --git a/specs/023-live-nuplane-options/quickstart.md b/specs/023-live-nuplane-options/quickstart.md new file mode 100644 index 0000000..a5eb38f --- /dev/null +++ b/specs/023-live-nuplane-options/quickstart.md @@ -0,0 +1,45 @@ +# Validation: Apply Live Nuplane Integration Options + +This guide defines the proof to run on the final implementation head. No implementation or acceptance test is claimed by this planning artifact. + +## Deterministic owner tests + +Run the focused Nuplane adapter suites through the repository's shared `dotnet` wrapper: + +```bash +dotnet test tests/CShells.Tests/CShells.Tests.csproj -c Release \ + --filter 'FullyQualifiedName~NuplaneRefreshCoordinatorTests|FullyQualifiedName~NuplaneCompositionTests|FullyQualifiedName~NuplaneReloadResultsTests' +``` + +The focused cases must prove: + +- Standard binding/configuration changes are observed by the next eligible callback without rebuilding the host provider. +- A refresh barrier captures all four policy values before awaits. Mutating the held source object and replacing `OnReloadResults` while that operation is blocked does not alter it; the next delivery sees the new values and callback. +- `Enabled = false` admits no new observer epochs but a build consumes older pending freshness without consulting live options. +- `AutoReload = false` retains failed/deferred reload work while refreshing eligible freshness. A later enabled delivery retries without a redundant catalog scan when that snapshot is already fresh. +- Failed-only, empty, and already-canceled deliveries make no monitor read, epoch mutation, registry access, or catalog refresh. +- An undefined refresh trigger produces an actionable error before epoch mutation and registry access; options-monitor and options-validation exceptions propagate before those side effects. Follow each rejected policy/read with an unchanged valid delivery or build and prove that no work was recorded. +- Existing fluent configuration, dependency-aware configuration, refresh triggers, result callbacks, alias identity, ordering, build reentry, and lifecycle behavior remain covered. + +Use `TaskCompletionSource` gates and bounded waits. Release gates and join every owned operation in cleanup; no sleeps are evidence of ordering. + +## Owner suite and target builds + +```bash +dotnet test tests/CShells.Tests/CShells.Tests.csproj -c Release --no-restore +dotnet build src/CShells.Nuplane/CShells.Nuplane.csproj -c Release +``` + +Confirm the adapter build covers `net8.0`, `net9.0`, and `net10.0`, with no new warnings or errors. Capture the exact head and command results in the issue or validation record. + +## Causal mutation + +In an isolated copy of the final tree, restore the prior construction-time `IOptions.Value` capture (or equivalent immutable startup-only behavior). Build and execute the dynamic options regression. It must fail because the second eligible delivery does not apply changed settings. Restore the production source byte-for-byte and execute the regression successfully. A compile failure does not count as the negative result. + +## Public-package acceptance + +After normal owner-repository merge and package publication, audit all ten actual public Feedz archives for exact source commit/version, family dependencies, archive identity, and `net8.0`/`net9.0`/`net10.0` assets. Then run a fresh-cache, outside-checkout PackageReference-only consumer with `CShells` and `CShells.Abstractions` plus the optional adapter as required by the public composition. Run on actual .NET 8, 9, and 10 runtimes; verify archive hashes, NuGet source metadata, loaded assembly identities, and the dynamic-options cases. Local builds, private packs, or a probe that only reproduces the old snapshot gap do not satisfy this acceptance. + +## Acceptance boundary + +Record each exact source/package identity and results. This feature does not complete stable release, Foundation pin/lock updates, Foundation Host/Workbench adoption, `#2164` readability acceptance, or `#2362` unloading/pruning work. diff --git a/specs/023-live-nuplane-options/research.md b/specs/023-live-nuplane-options/research.md new file mode 100644 index 0000000..afb08e0 --- /dev/null +++ b/specs/023-live-nuplane-options/research.md @@ -0,0 +1,59 @@ +# Research: Apply Live Nuplane Integration Options + +## Current behavior and reproduced gap + +`CShellsNuplaneBuilderExtensions.WithNuplaneFeatureDiscovery` registers one root-owned coordinator as both `INuplaneObserver` and `IShellGenerationBuildParticipant`, with the coordinator appended after existing Nuplane observers. Its registration resolves `IOptions`. `NuplaneRefreshCoordinator` reads `.Value` once in its constructor and stores that mutable options object for its lifetime. All later observer deliveries consult this captured object, so an ordinary configuration reload can update `IOptionsMonitor.CurrentValue` while the singleton coordinator keeps using the earlier values. + +The evidence is source-grounded in: + +- `src/CShells.Nuplane/CShellsNuplaneBuilderExtensions.cs`: existing options configuration and root coordinator aliases. +- `src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs`: constructor snapshot and per-delivery eligibility, epoch, refresh, and reload decisions. +- `src/CShells.Nuplane/NuplaneIntegrationOptions.cs`: the four mutable policy properties and their current defaults. +- `src/CShells.Nuplane/README.md`: generic defaults, host-profile examples, observer eligibility, refresh/reload behavior, and callback contract. +- `tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs` and `NuplaneCompositionTests.cs`: current epoch, cold-build, disabled, configuration, alias, and host-profile regression patterns. + +An outside-checkout public-package probe ran against actual CShells.Nuplane `0.0.30-preview.167` (source `94eab66fea5a5d33ceb8e660c0c841a23db9d96f`) and Nuplane abstractions `0.0.11-preview.99` (source `eb2cf6c2ee1f79dc2c45fb83cc415bbe4856d0d4`). It bound real options to a reloadable configuration provider, resolved the adapter's registered public observer, and delivered two eligible updates while a public fake catalog and registry counted work. After changing `AutoReload` from false to true, `IOptionsMonitor.CurrentValue` reported true and the second delivery refreshed the fake catalog, but no registry reload occurred. Root-local evidence is retained at `/Users/sipke/.codex-workspaces/artifacts/modular-hosting-2500/foundation-adoption-readiness/options-snapshot-probe/`; it is not part of this repository. This is evidence of the captured-snapshot gap, not a fix, Foundation host/store test, or runtime-adoption result. + +## Decisions + +### D1. Use the existing standard options monitor + +**Decision**: The root coordinator depends on `IOptionsMonitor` and reads its `CurrentValue` once for each eligible, non-canceled reconciliation delivery. + +**Rationale**: This is the existing Microsoft options mechanism for current values in singleton consumers. It makes the next delivery see provider reloads without adding a package, public flag, background listener, or second configuration source. The existing options package reference already supplies the abstraction. + +**Alternatives rejected**: + +- Keep `IOptions` and recreate the host: violates the per-delivery configuration behavior Foundation currently relies on. +- Subscribe to `IOptionsMonitor.OnChange` and cache a mutable settings object: adds a second lifetime/notification state machine and risks a callback changing an operation already in progress. +- Add a new CShells or Nuplane settings API: duplicates standard host configuration and expands public surface without a need. + +### D2. Copy a small policy at the delivery boundary + +**Decision**: After argument, cancellation, and eligibility checks, copy `Enabled`, `RefreshTrigger`, `AutoReload`, and `OnReloadResults` into one private immutable per-delivery value before epoch changes, registry access, or awaits. Use those copies for the whole operation, including invoking the captured reporting callback. + +**Rationale**: A single current-options read ensures the operation does not intentionally reread mutable settings at later phases. Copying only the four values keeps the boundary small and makes the lifetime rule testable. A deterministic barrier can change the source object after capture and prove that the admitted delivery remains unchanged while a later delivery observes the new values. + +**Limit**: Reading `CurrentValue` once is not a synchronization transaction over arbitrary callers mutating the same options instance concurrently while its four fields are copied. The contract guarantees that changes after capture do not alter the captured value; it makes no cross-property atomicity promise during that synchronous copy. + +### D3. Preserve existing epoch semantics + +**Decision**: Keep the catalog and reload epochs, locks, gates, commit timing, and retry rules. Disabled observer work records no new request but leaves existing epochs. Automatic reload disabled allows eligible freshness work while leaving pending reload work unattempted and unacknowledged. A later enabled eligible delivery can retry retained work; a fresh catalog is reused unless a newer source change requests another scan. `BeginAsync` remains independent of options and consumes already-recorded freshness before the catalog read. + +**Rationale**: Existing source and tests make the epoch separation, build participation, and reentry boundaries explicit. The desired change is how each observer delivery receives policy, not a replacement state machine. Future Foundation/Workbench host defaults select their own profiles without changing generic defaults. + +### D4. Fail invalid policy before coordinator side effects + +**Decision**: Validate the captured `RefreshTrigger` before epoch mutation, registry resolution/access, or asynchronous work. An undefined enum receives an actionable configuration error. Options-monitor and options-validation exceptions propagate before state/registry work; the adapter does not promise a new wrapper for arbitrary provider failures. No last-known-good policy cache is added. + +**Rationale**: Silently continuing with stale policy would hide invalid operator input. Failing before coordinator work leaves epochs and registry access unmodified and is directly testable with counters, while preserving the original type/message of monitor and validation failures. + +### D5. Keep the options-source compatibility boundary explicit + +**Decision**: Continue supporting `WithNuplaneFeatureDiscovery(Action)`, standard options binding/configuration, and dependency-aware options configuration. Direct replacement of only `IOptions` is explicitly unsupported; callers with a custom options source must provide `IOptionsMonitor`. + +**Rationale**: Standard options providers feed both `IOptions` and `IOptionsMonitor`. The current supported adapter examples already use the fluent callback and `services.AddOptions().Configure(...)`. A hidden fallback that combines `IOptions` and monitor values would create undefined precedence and retain the stale snapshot defect. + +## Resolved uncertainty + +The runtime option source, timing boundary, old-work behavior, error behavior, supported registration path, and ownership scope are settled by the reviewed Task #161 requirements gate and the audited public-package probe. No further owner choice is required before implementation planning. The gate and probe are root-local artifacts, not repository files. The probe's fake registry/catalog and single runtime are bounded diagnosis; the feature's actual acceptance still requires deterministic owner tests, full owner CI, all supported target builds, compiled mutation proof, and post-publication PackageReference consumers on actual net8/net9/net10. diff --git a/specs/023-live-nuplane-options/spec.md b/specs/023-live-nuplane-options/spec.md new file mode 100644 index 0000000..86476b3 --- /dev/null +++ b/specs/023-live-nuplane-options/spec.md @@ -0,0 +1,92 @@ +# Feature Specification: Apply Live Nuplane Integration Options + +**Feature Branch**: `023-live-nuplane-options` + +**Created**: 2026-10-08 + +**Status**: Draft + +**Input**: User description: Make runtime changes to the optional Nuplane/CShells integration settings take effect for later package-reconciliation deliveries without restarting the host, while preserving existing refresh, reload, and callback behavior. + +## User Scenarios & Testing + +### User Story 1 - Apply updated integration settings on the next package change (Priority: P1) + +A host operator changes Nuplane integration settings while the host is running. On the next eligible package-reconciliation delivery, the integration uses the settings that are current for that delivery. Work already admitted keeps the settings it captured when it began, and the next delivery observes later changes. + +**Why this priority**: Hosts need the optional integration to preserve their existing per-delivery configuration behavior even though the integration coordinator is long-lived. Requiring a host restart to change reload or reporting policy would make the adapter behave differently from the host configuration contract it replaces. + +**Independent Test**: Configure automatic reload off, deliver an eligible package change, then change the setting to on and deliver another eligible change. The second delivery refreshes the catalog and requests an active-shell reload. Separately hold one admitted delivery at a deterministic barrier while changing its settings, then prove that it completes with its captured policy and that the following delivery uses the new policy. + +**Acceptance Scenarios**: + +1. **Given** a running integration and automatic reload disabled, **When** the host changes automatic reload to enabled and a later eligible package update or removal is delivered, **Then** the later delivery applies the enabled policy without rebuilding the host service provider. +2. **Given** an eligible delivery paused at a deterministic operation barrier, **When** the host changes settings and replaces the reload-results callback, **Then** the paused delivery uses the values and callback it captured, and a later delivery observes the changed settings and callback. +3. **Given** standard options configuration or dependency-aware options configuration, **When** a supported setting changes through the configured options source, **Then** the next eligible delivery observes the change. + +### User Story 2 - Preserve deferred work and configuration errors (Priority: P2) + +A host can pause automatic reload or disable new observer work without losing work already recorded. When the host later enables the applicable behavior and delivers another eligible change, previously pending work can be retried. Invalid policy values fail clearly before the callback changes package epochs or queries the shell registry. + +**Why this priority**: Runtime configurability must not discard deferred freshness or promotion work, and a bad value must not leave partially advanced coordinator state. + +**Independent Test**: Arrange pending promotion work, disable automatic reload while allowing freshness, then enable it on a later eligible delivery and verify the pending promotion is retried without an unnecessary second catalog refresh. Arrange previously recorded freshness, disable new observer work, and verify a shell-generation build can consume that freshness. Verify invalid values and options-monitor failures leave epoch and registry-call counts unchanged. + +**Acceptance Scenarios**: + +1. **Given** a failed or deferred promotion remains pending, **When** automatic reload is disabled for a later eligible delivery, **Then** freshness may still advance but the pending promotion is neither attempted nor cleared; a later eligible delivery with automatic reload enabled retries it without rescanning an already-fresh catalog. +2. **Given** freshness work was recorded before observer work was disabled, **When** a shell generation begins while observer work remains disabled, **Then** the build consumes the recorded freshness before reading the catalog, without requiring or reading current observer options. +3. **Given** an undefined refresh policy or a failing current-options read, **When** an eligible callback arrives, **Then** the undefined policy produces an actionable configuration error and the current-options failure is surfaced before changing epochs or accessing the shell registry. +4. **Given** a callback is canceled or has no successfully applied packages and no committed removals, **When** it is delivered, **Then** it does no options read or epoch, catalog, or registry work. + +## Edge Cases + +- A configuration change during an admitted operation applies only to a later delivery; it does not cancel or rewrite the admitted operation. +- Capturing policy means reading the monitor's current options once and copying the four supported values synchronously before operation awaits. No atomic transaction is promised if a caller concurrently mutates the same options instance while those individual properties are being copied. +- `Enabled = false` admits no new observer work and does not erase previously recorded freshness or promotion epochs. +- `AutoReload = false` permits configured catalog freshness work but neither attempts nor acknowledges pending promotion work. +- A valid options update that changes only the reporting callback still applies to the next admitted reload operation; an operation already in progress uses the callback it captured. +- An options-monitor exception or undefined refresh-trigger value is surfaced before coordinator state or registry interaction changes. +- Direct replacement of `IOptions` alone is unsupported; use standard options configuration/binding or supply the monitor contract. +- `BeginAsync` remains independent of live observer settings. It consumes recorded catalog freshness before the candidate reads the catalog and does not activate, reload, or report a shell. + +## Requirements + +### Functional Requirements + +- **FR-001**: The optional integration MUST observe supported runtime setting changes on the next eligible package-reconciliation delivery without recreating the host service provider. +- **FR-002**: Each eligible delivery MUST use one captured set of the enabled flag, refresh policy, automatic-reload flag, and reload-results callback for all decisions and reporting in that operation. +- **FR-003**: Changes made after a delivery has captured its settings MUST affect later deliveries only; they MUST NOT cancel or change the admitted operation. +- **FR-004**: A delivery canceled before admission or with no successfully applied package and no committed removal MUST NOT read current options, mutate work epochs, access the shell registry, or refresh the catalog. +- **FR-005**: When observer work is disabled, the integration MUST record no new observer-requested work and MUST preserve previously recorded freshness and promotion work. +- **FR-006**: When automatic reload is disabled, eligible freshness work MAY proceed, but pending promotion work MUST remain unattempted and pending until a later enabled eligible delivery can retry it. +- **FR-007**: A later retry of pending promotion work MUST reuse successfully refreshed catalog state when no newer source change requires another refresh. +- **FR-008**: Shell-generation catalog freshness work MUST remain independent of current observer settings and MUST be consumed before a candidate reads the catalog. +- **FR-009**: An undefined refresh policy MUST produce an actionable configuration error before any epoch change or shell-registry access. Options-monitor and options-validation failures MUST propagate before those side effects without requiring a new wrapping exception. +- **FR-010**: Standard options configuration, configuration binding, and dependency-aware configuration MUST remain supported. Replacing only the `IOptions` service MUST be explicitly documented as unsupported. +- **FR-011**: The integration MUST retain its existing callback eligibility, refresh trigger, reload-result, alias, and provider-ownership behavior apart from observing the current captured options for each eligible delivery. +- **FR-012**: The change MUST remain confined to the optional CShells.Nuplane adapter and its tests/documentation; it MUST NOT add a CShells-core or Nuplane-core options dependency, listener, timer, public flag, or second coordinator. + +### Key Entities + +- **Eligible reconciliation delivery**: A delivered package-reconciliation completion with at least one successfully applied package or one committed removal, unless canceled before admission. +- **Captured integration policy**: The four supported setting values copied for one admitted delivery and used consistently until that operation finishes. +- **Pending freshness and promotion work**: Existing coordinator epochs representing catalog refresh or shell reload work not yet successfully acknowledged. They remain authoritative and are not replaced by new persistent state. + +## Assumptions + +- Supported runtime changes arrive through the standard options monitor and its configured providers. The adapter does not provide a new settings source or dual-source precedence. +- Individual policy fields are copied synchronously from the current options object. If a caller mutates one object concurrently during that copy, cross-property atomicity is outside this contract; deterministic tests change options after capture. +- Existing callback eligibility, epoch sequencing, catalog commit participation, shell-registry access, and reload outcome semantics remain as defined by the current optional adapter. +- Final stable package adoption and Foundation host qualification are separate downstream gates; this feature does not change Foundation pins, locks, or host composition. + +## Success Criteria + +### Measurable Outcomes + +- **SC-001**: A supported options change is reflected by the first later eligible completion, with no service-provider recreation. +- **SC-002**: A deterministic operation-barrier test proves that a setting or callback update during an admitted operation does not alter that operation and is observed by the following delivery. +- **SC-003**: Tests prove that disabling observer work or automatic reload preserves already-pending work and that a later enabled delivery resumes it without unnecessary catalog refresh. +- **SC-004**: Tests prove that canceled and empty deliveries do not read current settings or touch coordinator epochs, registry, or catalog. +- **SC-005**: Undefined refresh-trigger values fail with an actionable diagnostic before epoch mutation or registry access; monitor and options-validation exceptions propagate before those side effects. +- **SC-006**: Existing adapter tests and all required source-package, public-package, and runtime gates pass without adding a package dependency or changing core hosting behavior. diff --git a/specs/023-live-nuplane-options/tasks.md b/specs/023-live-nuplane-options/tasks.md new file mode 100644 index 0000000..4499dea --- /dev/null +++ b/specs/023-live-nuplane-options/tasks.md @@ -0,0 +1,67 @@ +# Tasks: Apply Live Nuplane Integration Options + +**Input**: [spec.md](spec.md), [plan.md](plan.md), [research.md](research.md), [data-model.md](data-model.md), [contract](contracts/options-policy.md), [quickstart](quickstart.md) + +**Prerequisites**: All documents above are present and reviewed. + +**Tests**: Required by CShells constitution V. The constitution does not require test-first/TDD ordering; add deterministic tests with each implementation slice and verify them before merge. + +**Task setup**: This checkout does not contain `setup-tasks.sh`. Tasks were generated from the repository's task template and verified through `check-prerequisites.sh --json --require-tasks --include-tasks`. + +## Phase 1: Setup + +**Purpose**: Provide a focused monitor test double shared by coordinator tests. + +- [ ] T001 Add a minimal `IOptionsMonitor` test helper with current-value read counting, current-value replacement, injected read failure, and the required no-op `OnChange` implementation in `tests/CShells.Tests/Integration/Nuplane/NuplaneCoordinatorTestHarness.cs`. + +## Phase 2: Foundational Options Wiring + +**Purpose**: Make the existing root coordinator and direct test construction resolve the standard monitor contract before implementing per-delivery behavior. + +- [ ] T002 Change the root coordinator factory in `src/CShells.Nuplane/CShellsNuplaneBuilderExtensions.cs` and the constructor in `src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs` to use `IOptionsMonitor`, then migrate direct coordinator construction in `tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs` and `tests/CShells.Tests/Integration/Nuplane/NuplaneReloadResultsTests.cs` to the shared helper while preserving current test behavior. + +## Phase 3: User Story 1 - Apply updated settings on the next delivery (P1) + +**Goal**: A root-owned singleton adapter coordinator uses the current supported options for each eligible callback and keeps one operation's choices consistent through its lifetime. + +**Independent Test**: An outside or DI-composed observer receives one eligible event with reload off, configuration is reloaded to turn reload on, and a later event reloads the active shell without rebuilding the provider. A gate proves changes during an admitted operation apply only to the next event. + +- [ ] T003 [US1] Update `src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs` to read `CurrentValue` once after null/cancellation/eligibility checks, validate the enum, copy all four policy values before epochs/registry/awaits, and use only that private immutable snapshot through refresh, reload, and result reporting. +- [ ] T004 [US1] Add deterministic tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs` for false-to-true automatic reload across deliveries and for a gated refresh where mutating the captured settings object and replacing `OnReloadResults` does not alter the current operation but the next delivery sees the new values. +- [ ] T005 [US1] Add or extend a DI integration test in `tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs` to prove configuration reload reaches the adapter through standard options binding, while retaining fluent and dependency-aware configuration coverage. + +## Phase 4: User Story 2 - Preserve pending work and fail bad policy before side effects (P2) + +**Goal**: Disabled or paused behavior does not lose already-recorded work, and configuration errors cannot partially mutate coordinator state. + +**Independent Test**: Gate a freshness/reload attempt, toggle enabled/reload policy, and assert existing epochs survive and can be retried without unnecessary catalog work. Use monitor, registry, and catalog counters to prove empty/canceled/error paths remain side-effect free. + +- [ ] T006 [US2] Extend `tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs` to prove disabling observer work records no new epochs while a build consumes prior freshness without reading options, and automatic-reload-off retains pending promotion work for a later enabled delivery without rescanning fresh catalog state. +- [ ] T007 [US2] Add monitor-read, catalog, and registry counter tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs` for failed-only, empty, pre-canceled, undefined refresh-trigger, throwing monitor, and options-validation failures; follow rejected invalid reads with a valid unchanged delivery/build and prove no work was recorded. +- [ ] T008 [US2] Update `src/CShells.Nuplane/README.md` to document next-eligible-delivery settings, per-operation callback capture, preserved pending work, standard configuration paths, and direct `IOptions`-only replacement as unsupported. + +## Phase 5: Verification and Publication Qualification + +**Purpose**: Prove the behavioral contract, catch restoration of the stale snapshot, and validate the actual delivered packages. + +- [ ] T009 Run the focused Nuplane tests, full `tests/CShells.Tests/CShells.Tests.csproj` suite, and Release build of `src/CShells.Nuplane/CShells.Nuplane.csproj` for net8.0/net9.0/net10.0; then perform a compiled frozen-options mutation that fails the dynamic options test and verify byte-identical restoration passes, recording exact-head evidence in `specs/023-live-nuplane-options/quickstart.md`. +- [ ] T010 After normal owner-repository PR merge and main checks, audit all ten public package archives and run an outside-checkout, fresh-cache PackageReference-only dynamic-options consumer on actual .NET 8, 9, and 10 runtimes; record source/package identity, cache source, hashes, loaded DLL assets, exact commands, and scenario results in `specs/023-live-nuplane-options/quickstart.md` and the owning issue. + +## Dependencies and Execution Order + +### Phase Dependencies + +- **Setup (Phase 1)**: T001 provides the shared test monitor. +- **Foundational (Phase 2)**: T002 depends on T001 and updates existing direct coordinator tests for the internal constructor change. +- **US1 (Phase 3)**: T003 and T004 depend on T002; T005 validates real options binding and existing DI composition. +- **US2 (Phase 4)**: T006 and T007 depend on T003; T008 may be reviewed alongside implementation but must describe the delivered behavior. +- **Verification (Phase 5)**: T009 follows implementation and documentation. T010 requires passing local/hosted gates, normal main publication, and an audited actual public package set. + +### User Story Dependencies + +- **US1 (P1)**: Independent after the shared test-monitor setup; delivers live options on subsequent eligible callbacks. +- **US2 (P2)**: Uses the same captured policy and existing epochs from US1; its tests can be completed after T003. + +## Implementation Strategy + +The MVP is US1: replace startup-only option consumption with a one-read immutable per-delivery policy and prove later deliveries observe reloaded settings. Complete US2 before acceptance so disabled, pending, and invalid-setting behavior remains safe. Tests must use deterministic gates and bounded cleanup. Public package qualification is a separate final acceptance gate; preview or private-package results do not close it. From 8cb10fba9e58c68040da993a9e634fad02f4a372 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 8 Oct 2026 22:45:43 +0200 Subject: [PATCH 2/4] fix: apply live Nuplane options per completion --- specs/023-live-nuplane-options/plan.md | 3 +- specs/023-live-nuplane-options/quickstart.md | 2 +- specs/023-live-nuplane-options/tasks.md | 6 +- .../CShellsNuplaneBuilderExtensions.cs | 2 +- .../Internal/NuplaneRefreshCoordinator.cs | 50 +++- src/CShells.Nuplane/README.md | 4 +- .../Nuplane/NuplaneCompositionTests.cs | 38 ++- .../Nuplane/NuplaneCoordinatorTestHarness.cs | 84 ++++++ .../Nuplane/NuplaneLiveOptionsTests.cs | 262 ++++++++++++++++++ .../Nuplane/NuplaneRefreshCoordinatorTests.cs | 68 +---- .../Nuplane/NuplaneReloadResultsTests.cs | 3 +- 11 files changed, 433 insertions(+), 89 deletions(-) create mode 100644 tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs diff --git a/specs/023-live-nuplane-options/plan.md b/specs/023-live-nuplane-options/plan.md index 6cf023e..617c93d 100644 --- a/specs/023-live-nuplane-options/plan.md +++ b/specs/023-live-nuplane-options/plan.md @@ -72,6 +72,7 @@ src/CShells.Nuplane/ tests/CShells.Tests/Integration/Nuplane/ NuplaneRefreshCoordinatorTests.cs NuplaneCompositionTests.cs + NuplaneLiveOptionsTests.cs NuplaneCoordinatorTestHarness.cs or a narrowly scoped options-monitor helper ``` @@ -81,7 +82,7 @@ tests/CShells.Tests/Integration/Nuplane/ Run, once on the final implementation head: -1. Focused `NuplaneRefreshCoordinatorTests`, `NuplaneCompositionTests`, and `NuplaneReloadResultsTests`. +1. Focused `NuplaneRefreshCoordinatorTests`, `NuplaneCompositionTests`, `NuplaneLiveOptionsTests`, and `NuplaneReloadResultsTests`. 2. Full `tests/CShells.Tests/CShells.Tests.csproj` suite. 3. Release builds of `src/CShells.Nuplane/CShells.Nuplane.csproj` for net8.0, net9.0, and net10.0. 4. A compiled freeze-options mutation that restores the old captured-settings behavior; the live-options regression must fail under the mutation and pass after byte-identical restoration. diff --git a/specs/023-live-nuplane-options/quickstart.md b/specs/023-live-nuplane-options/quickstart.md index a5eb38f..56ce64e 100644 --- a/specs/023-live-nuplane-options/quickstart.md +++ b/specs/023-live-nuplane-options/quickstart.md @@ -8,7 +8,7 @@ Run the focused Nuplane adapter suites through the repository's shared `dotnet` ```bash dotnet test tests/CShells.Tests/CShells.Tests.csproj -c Release \ - --filter 'FullyQualifiedName~NuplaneRefreshCoordinatorTests|FullyQualifiedName~NuplaneCompositionTests|FullyQualifiedName~NuplaneReloadResultsTests' + --filter 'FullyQualifiedName~NuplaneRefreshCoordinatorTests|FullyQualifiedName~NuplaneCompositionTests|FullyQualifiedName~NuplaneLiveOptionsTests|FullyQualifiedName~NuplaneReloadResultsTests' ``` The focused cases must prove: diff --git a/specs/023-live-nuplane-options/tasks.md b/specs/023-live-nuplane-options/tasks.md index 4499dea..7413ab1 100644 --- a/specs/023-live-nuplane-options/tasks.md +++ b/specs/023-live-nuplane-options/tasks.md @@ -27,7 +27,7 @@ **Independent Test**: An outside or DI-composed observer receives one eligible event with reload off, configuration is reloaded to turn reload on, and a later event reloads the active shell without rebuilding the provider. A gate proves changes during an admitted operation apply only to the next event. - [ ] T003 [US1] Update `src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs` to read `CurrentValue` once after null/cancellation/eligibility checks, validate the enum, copy all four policy values before epochs/registry/awaits, and use only that private immutable snapshot through refresh, reload, and result reporting. -- [ ] T004 [US1] Add deterministic tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs` for false-to-true automatic reload across deliveries and for a gated refresh where mutating the captured settings object and replacing `OnReloadResults` does not alter the current operation but the next delivery sees the new values. +- [ ] T004 [US1] Add deterministic tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs` for false-to-true automatic reload across deliveries and for a gated refresh where mutating the captured settings object and replacing `OnReloadResults` does not alter the current operation but the next delivery sees the new values. - [ ] T005 [US1] Add or extend a DI integration test in `tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs` to prove configuration reload reaches the adapter through standard options binding, while retaining fluent and dependency-aware configuration coverage. ## Phase 4: User Story 2 - Preserve pending work and fail bad policy before side effects (P2) @@ -36,8 +36,8 @@ **Independent Test**: Gate a freshness/reload attempt, toggle enabled/reload policy, and assert existing epochs survive and can be retried without unnecessary catalog work. Use monitor, registry, and catalog counters to prove empty/canceled/error paths remain side-effect free. -- [ ] T006 [US2] Extend `tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs` to prove disabling observer work records no new epochs while a build consumes prior freshness without reading options, and automatic-reload-off retains pending promotion work for a later enabled delivery without rescanning fresh catalog state. -- [ ] T007 [US2] Add monitor-read, catalog, and registry counter tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs` for failed-only, empty, pre-canceled, undefined refresh-trigger, throwing monitor, and options-validation failures; follow rejected invalid reads with a valid unchanged delivery/build and prove no work was recorded. +- [ ] T006 [US2] Add tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs` to prove disabling observer work records no new epochs while a build consumes prior freshness without reading options, and automatic-reload-off retains pending promotion work for a later enabled delivery without rescanning fresh catalog state. +- [ ] T007 [US2] Add monitor-read, catalog, and registry counter tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs` for failed-only, empty, pre-canceled, undefined refresh-trigger, throwing monitor, and options-validation failures; follow rejected invalid reads with a valid unchanged delivery/build and prove no work was recorded. - [ ] T008 [US2] Update `src/CShells.Nuplane/README.md` to document next-eligible-delivery settings, per-operation callback capture, preserved pending work, standard configuration paths, and direct `IOptions`-only replacement as unsupported. ## Phase 5: Verification and Publication Qualification diff --git a/src/CShells.Nuplane/CShellsNuplaneBuilderExtensions.cs b/src/CShells.Nuplane/CShellsNuplaneBuilderExtensions.cs index 9f6bb42..0b66e5b 100644 --- a/src/CShells.Nuplane/CShellsNuplaneBuilderExtensions.cs +++ b/src/CShells.Nuplane/CShellsNuplaneBuilderExtensions.cs @@ -43,7 +43,7 @@ public static CShellsBuilder WithNuplaneFeatureDiscovery( services.AddSingleton(serviceProvider => new CoordinatorHolder( new NuplaneRefreshCoordinator( serviceProvider.GetRequiredService(), - serviceProvider.GetRequiredService>(), + serviceProvider.GetRequiredService>(), () => serviceProvider.GetRequiredService()))); services.AddSingleton(serviceProvider => serviceProvider.GetRequiredService().Coordinator); services.AddSingleton(serviceProvider => serviceProvider.GetRequiredService().Coordinator); diff --git a/src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs b/src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs index 0020c31..444b647 100644 --- a/src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs +++ b/src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs @@ -8,11 +8,11 @@ namespace CShells.Nuplane.Internal; /// Coordinates deferred catalog freshness and optional automatic shell reload. internal sealed class NuplaneRefreshCoordinator( IRuntimeFeatureCatalog featureCatalog, - IOptions options, + IOptionsMonitor options, Func shellRegistryFactory) : INuplaneObserver, IShellGenerationBuildParticipant { private readonly IRuntimeFeatureCatalog catalog = featureCatalog ?? throw new ArgumentNullException(nameof(featureCatalog)); - private readonly NuplaneIntegrationOptions integrationOptions = options?.Value ?? throw new ArgumentNullException(nameof(options)); + private readonly IOptionsMonitor optionsMonitor = options ?? throw new ArgumentNullException(nameof(options)); private readonly Func registryFactory = shellRegistryFactory ?? throw new ArgumentNullException(nameof(shellRegistryFactory)); private readonly object epochStateLock = new(); private readonly SemaphoreSlim refreshGate = new(1, 1); @@ -39,11 +39,15 @@ public async Task OnPackagesReconciledAsync( ArgumentNullException.ThrowIfNull(appliedPackages); cancellationToken.ThrowIfCancellationRequested(); - if (!integrationOptions.Enabled || (appliedPackages.Count == 0 && changeSet.Removed.Count == 0)) + if (appliedPackages.Count == 0 && changeSet.Removed.Count == 0) + return; + + var policy = CapturePolicy(); + if (!policy.Enabled) return; var hasSourceChanges = changeSet.Added.Count > 0 || changeSet.Updated.Count > 0 || changeSet.Removed.Count > 0; - var forceRefresh = integrationOptions.RefreshTrigger == NuplaneRefreshTrigger.EveryEligibleCompletion; + var forceRefresh = policy.RefreshTrigger == NuplaneRefreshTrigger.EveryEligibleCompletion; long catalogRequestAtDelivery; bool callbackRequestsCatalogWork; @@ -61,7 +65,7 @@ public async Task OnPackagesReconciledAsync( if (registry.GetActiveShells().Count == 0) return; - if (integrationOptions.AutoReload && callbackRequestsCatalogWork) + if (policy.AutoReload && callbackRequestsCatalogWork) { lock (epochStateLock) { @@ -71,7 +75,7 @@ public async Task OnPackagesReconciledAsync( } await RefreshPendingCatalogAsync(cancellationToken).ConfigureAwait(false); - await TryReloadPendingAsync(registry, cancellationToken).ConfigureAwait(false); + await TryReloadPendingAsync(registry, policy, cancellationToken).ConfigureAwait(false); } public async ValueTask BeginAsync( @@ -109,9 +113,31 @@ private async ValueTask RefreshPendingCatalogAsync(CancellationToken cancellatio } } - private async ValueTask TryReloadPendingAsync(IShellRegistry registry, CancellationToken cancellationToken) + private IntegrationPolicy CapturePolicy() { - if (!integrationOptions.AutoReload) + var current = optionsMonitor.CurrentValue ?? throw new InvalidOperationException( + $"The options monitor returned null for {nameof(NuplaneIntegrationOptions)}."); + var policy = new IntegrationPolicy( + current.Enabled, + current.RefreshTrigger, + current.AutoReload, + current.OnReloadResults); + + if (!Enum.IsDefined(policy.RefreshTrigger)) + throw new ArgumentOutOfRangeException( + nameof(NuplaneIntegrationOptions.RefreshTrigger), + policy.RefreshTrigger, + $"Unsupported {nameof(NuplaneRefreshTrigger)} value. Use {NuplaneRefreshTrigger.ChangedOrPending} or {NuplaneRefreshTrigger.EveryEligibleCompletion}."); + + return policy; + } + + private async ValueTask TryReloadPendingAsync( + IShellRegistry registry, + IntegrationPolicy policy, + CancellationToken cancellationToken) + { + if (!policy.AutoReload) return; await reloadGate.WaitAsync(cancellationToken).ConfigureAwait(false); @@ -138,7 +164,7 @@ private async ValueTask TryReloadPendingAsync(IShellRegistry registry, Cancellat allSucceeded = false; } - if (integrationOptions.OnReloadResults is { } callback) + if (policy.OnReloadResults is { } callback) await callback(stableResults, cancellationToken).ConfigureAwait(false); cancellationToken.ThrowIfCancellationRequested(); @@ -154,4 +180,10 @@ private async ValueTask TryReloadPendingAsync(IShellRegistry registry, Cancellat reloadGate.Release(); } } + + private readonly record struct IntegrationPolicy( + bool Enabled, + NuplaneRefreshTrigger RefreshTrigger, + bool AutoReload, + Func, CancellationToken, ValueTask>? OnReloadResults); } diff --git a/src/CShells.Nuplane/README.md b/src/CShells.Nuplane/README.md index 634fbfb..9667e50 100644 --- a/src/CShells.Nuplane/README.md +++ b/src/CShells.Nuplane/README.md @@ -63,7 +63,9 @@ 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. -`Enabled = false` suppresses observer-driven refresh and reload. It does not disable the assembly provider's normal query when CShells initializes the feature catalog. Reloading is independent: with `AutoReload = false`, a refresh updates catalog metadata while active shells keep their current generation. +`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. + +The adapter reads the current options once for each eligible reconciliation delivery and copies the four policy values before recording work. Standard `IOptionsMonitor` configuration therefore applies on the next eligible delivery, including values bound from a reloaded `IConfiguration`; changes do not start a background refresh or reload by themselves. The current delivery keeps the policy it captured even if configuration changes while its refresh is running. Build participation does not read options. Use `Configure`, `Bind`, or dependency-aware `AddOptions` configuration so the registered monitor owns the live values. A custom `IOptionsMonitor` registration is supported; replacing only `IOptions` is unsupported. ## Refresh cold shells before construction diff --git a/tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs b/tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs index cc14361..b3bd7f3 100644 --- a/tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs +++ b/tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs @@ -109,21 +109,35 @@ public async Task FailedPromotionAfterColdRefreshReusesTheCommittedCatalogOnTheN Assert.Equal(1, packageCatalog.QueryCount); } - [Fact] - public void StandardDependencyAwareOptionsConfigurationRemainsAvailable() + [Fact(DisplayName = "Dependency-aware options configuration reaches the registered observer")] + public async Task DependencyAwareConfiguration_RegisteredWithTheObserver_InvokesConfiguredCallback() { - Func, CancellationToken, ValueTask> callback = (_, _) => ValueTask.CompletedTask; - var services = new ServiceCollection(); - services.AddSingleton(new CallbackDependency(callback)); - services.AddOptions() - .Configure((options, dependency) => options.OnReloadResults = dependency.Callback); - services.AddSingleton(new FakePackageAssemblyCatalog()); - services.AddCShells(shells => shells.WithNuplaneFeatureDiscovery()); - using var provider = services.BuildServiceProvider(); + var callbackCount = 0; + var dependencyCallback = (IReadOnlyList _, CancellationToken _) => + { + Interlocked.Increment(ref callbackCount); + return ValueTask.CompletedTask; + }; + await using var host = NuplaneTestHost.Build( + services => + { + services.AddSingleton(new CallbackDependency(dependencyCallback)); + services.AddOptions() + .Configure((options, dependency) => + { + options.AutoReload = true; + options.OnReloadResults = dependency.Callback; + }); + services.AddSingleton(new FakePackageAssemblyCatalog()); + }, + shells => shells.WithNuplaneFeatureDiscovery().AddShell("dependency-options", _ => { })); - var options = provider.GetRequiredService>().Value; + var configured = host.GetRequiredService>().Value; + Assert.Same(dependencyCallback, configured.OnReloadResults); + await host.GetRequiredService().ActivateAsync("dependency-options"); + await NuplaneCoordinatorTestCases.NotifyAsync(host.GetRequiredService()); - Assert.Same(callback, options.OnReloadResults); + Assert.Equal(1, callbackCount); } [Theory] diff --git a/tests/CShells.Tests/Integration/Nuplane/NuplaneCoordinatorTestHarness.cs b/tests/CShells.Tests/Integration/Nuplane/NuplaneCoordinatorTestHarness.cs index c14e553..b1c7aa2 100644 --- a/tests/CShells.Tests/Integration/Nuplane/NuplaneCoordinatorTestHarness.cs +++ b/tests/CShells.Tests/Integration/Nuplane/NuplaneCoordinatorTestHarness.cs @@ -1,10 +1,48 @@ using System.Reflection; using CShells.Features; using CShells.Lifecycle; +using Microsoft.Extensions.Options; using Nuplane.Abstractions; +using System.Runtime.ExceptionServices; namespace CShells.Tests.Integration.Nuplane; +internal sealed class TestOptionsMonitor(TOptions currentValue) : IOptionsMonitor + where TOptions : class +{ + private TOptions _currentValue = currentValue; + private Exception? _readException; + private int _readCount; + + public int ReadCount => Volatile.Read(ref _readCount); + + public TOptions CurrentValue => Get(Options.DefaultName); + + public TOptions Get(string? name) + { + Interlocked.Increment(ref _readCount); + if (Volatile.Read(ref _readException) is { } exception) + System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(exception).Throw(); + + return Volatile.Read(ref _currentValue); + } + + public IDisposable? OnChange(Action listener) => NoOpDisposable.Instance; + + public void Replace(TOptions options) => Volatile.Write(ref _currentValue, options); + + public void ThrowOnRead(Exception? exception) => Volatile.Write(ref _readException, exception); + + private sealed class NoOpDisposable : IDisposable + { + public static NoOpDisposable Instance { get; } = new(); + + public void Dispose() + { + } + } +} + internal sealed class TestRuntimeFeatureCatalog : IRuntimeFeatureCatalog { public int RefreshCount { get; private set; } @@ -98,4 +136,50 @@ public static Task NotifyAsync( return observer.OnPackagesReconciledAsync(changeSet, appliedPackages, cancellationToken); } + + public static async Task RunWithGateCleanupAsync(Func exercise, Action releaseGates, params Func[] inFlightTasks) + { + Exception? primaryFailure = null; + try + { + await exercise(); + } + catch (Exception exception) + { + primaryFailure = exception; + } + finally + { + releaseGates(); + } + + var cleanupFailures = new List(); + foreach (var getTask in inFlightTasks) + { + if (getTask() is not { } task) + continue; + + try + { + await task.WaitAsync(TimeSpan.FromSeconds(5)); + } + catch (Exception exception) + { + cleanupFailures.Add(exception); + } + } + + if (primaryFailure is not null) + { + if (cleanupFailures.Count > 0) + throw new AggregateException("The gated test failed and cleanup also failed.", [primaryFailure, .. cleanupFailures]); + + ExceptionDispatchInfo.Capture(primaryFailure).Throw(); + } + + if (cleanupFailures.Count == 1) + ExceptionDispatchInfo.Capture(cleanupFailures[0]).Throw(); + if (cleanupFailures.Count > 1) + throw new AggregateException("Gated test cleanup failed.", cleanupFailures); + } } diff --git a/tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs b/tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs new file mode 100644 index 0000000..dc572fc --- /dev/null +++ b/tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs @@ -0,0 +1,262 @@ +using CShells.Lifecycle; +using CShells.Nuplane; +using CShells.Nuplane.Internal; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Nuplane.Abstractions; +using Nuplane.Loading; + +namespace CShells.Tests.Integration.Nuplane; + +public sealed class NuplaneLiveOptionsTests +{ + [Fact(DisplayName = "An eligible delivery uses its captured options while later deliveries see replacements")] + public async Task DeliveryPolicy_CapturedBeforeGatedRefresh_RemainsStableForThatDelivery() + { + var refreshStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseRefresh = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var oldCallbackCount = 0; + var newCallbackCount = 0; + var original = new NuplaneIntegrationOptions + { + RefreshTrigger = NuplaneRefreshTrigger.EveryEligibleCompletion, + AutoReload = true, + OnReloadResults = (_, _) => + { + Interlocked.Increment(ref oldCallbackCount); + return ValueTask.CompletedTask; + } + }; + var monitor = new TestOptionsMonitor(original); + var catalog = new TestRuntimeFeatureCatalog + { + RefreshHandler = async (number, _) => + { + if (number != 1) + return; + + refreshStarted.TrySetResult(); + await releaseRefresh.Task; + } + }; + var registry = TestShellRegistry.Create(out var registryState); + var coordinator = new NuplaneRefreshCoordinator(catalog, monitor, () => registry); + Task? firstDelivery = null; + + await NuplaneCoordinatorTestCases.RunWithGateCleanupAsync( + async () => + { + firstDelivery = NuplaneCoordinatorTestCases.NotifyAsync(coordinator); + await refreshStarted.Task.WaitAsync(TimeSpan.FromSeconds(5)); + + original.Enabled = false; + original.RefreshTrigger = (NuplaneRefreshTrigger)99; + original.AutoReload = false; + original.OnReloadResults = (_, _) => ValueTask.FromException(new InvalidOperationException("The mutated callback must not run.")); + monitor.Replace(new NuplaneIntegrationOptions + { + RefreshTrigger = NuplaneRefreshTrigger.ChangedOrPending, + AutoReload = false, + OnReloadResults = (_, _) => + { + Interlocked.Increment(ref newCallbackCount); + return ValueTask.CompletedTask; + } + }); + + releaseRefresh.TrySetResult(); + await firstDelivery.WaitAsync(TimeSpan.FromSeconds(5)); + + Assert.Equal(1, catalog.RefreshCount); + Assert.Equal(1, registryState.ReloadCount); + Assert.Equal(1, oldCallbackCount); + Assert.Equal(0, newCallbackCount); + + await NuplaneCoordinatorTestCases.NotifyAsync(coordinator, sourceChanged: false); + Assert.Equal(1, catalog.RefreshCount); + Assert.Equal(1, registryState.ReloadCount); + Assert.Equal(0, newCallbackCount); + + monitor.Replace(new NuplaneIntegrationOptions + { + RefreshTrigger = NuplaneRefreshTrigger.ChangedOrPending, + AutoReload = true, + OnReloadResults = (_, _) => + { + Interlocked.Increment(ref newCallbackCount); + return ValueTask.CompletedTask; + } + }); + await NuplaneCoordinatorTestCases.NotifyAsync(coordinator); + + Assert.Equal(2, catalog.RefreshCount); + Assert.Equal(2, registryState.ReloadCount); + Assert.Equal(1, newCallbackCount); + }, + () => releaseRefresh.TrySetResult(), + () => firstDelivery); + } + + [Fact(DisplayName = "Disabled deliveries preserve pending build freshness and add no new epochs")] + public async Task DisabledDelivery_PendingBuildFreshness_IsPreservedWithoutNewEpochs() + { + var monitor = new TestOptionsMonitor(new NuplaneIntegrationOptions()); + var catalog = new TestRuntimeFeatureCatalog(); + var registry = TestShellRegistry.Create(out var registryState); + registryState.HasActiveShell = false; + var coordinator = new NuplaneRefreshCoordinator(catalog, monitor, () => registry); + var context = new ShellGenerationBuildContext(ShellDescriptor.Create("deferred", 1), new ShellId("deferred")); + + await NuplaneCoordinatorTestCases.NotifyAsync(coordinator); + Assert.Equal(1, monitor.ReadCount); + monitor.Replace(new NuplaneIntegrationOptions { Enabled = false }); + await NuplaneCoordinatorTestCases.NotifyAsync(coordinator); + Assert.Equal(2, monitor.ReadCount); + + await using var pendingLease = await coordinator.BeginAsync(context); + Assert.Equal(1, catalog.RefreshCount); + + await NuplaneCoordinatorTestCases.NotifyAsync(coordinator); + Assert.Equal(3, monitor.ReadCount); + await using var unchangedLease = await coordinator.BeginAsync(context); + + Assert.Equal(1, catalog.RefreshCount); + Assert.Equal(3, monitor.ReadCount); + Assert.Equal(1, registryState.ActiveShellReadCount); + Assert.Equal(0, registryState.ReloadCount); + } + + [Fact(DisplayName = "Turning automatic reload off preserves an existing failed reload for a later retry")] + public async Task AutoReload_OffAfterFailedReload_PreservesRetryWithoutRescan() + { + var firstReloadFailure = new InvalidOperationException("First reload did not promote the shell."); + var monitor = new TestOptionsMonitor(new NuplaneIntegrationOptions { AutoReload = true }); + var catalog = new TestRuntimeFeatureCatalog(); + var registry = TestShellRegistry.Create(out var registryState); + registryState.ReloadHandler = (number, _) => Task.FromResult>( + number == 1 + ? [new ReloadResult("test", null, null, firstReloadFailure)] + : [new ReloadResult("test", null, null, null)]); + var coordinator = new NuplaneRefreshCoordinator(catalog, monitor, () => registry); + + await NuplaneCoordinatorTestCases.NotifyAsync(coordinator); + monitor.Replace(new NuplaneIntegrationOptions { AutoReload = false }); + await NuplaneCoordinatorTestCases.NotifyAsync(coordinator, sourceChanged: false); + + Assert.Equal(1, catalog.RefreshCount); + Assert.Equal(1, registryState.ReloadCount); + + monitor.Replace(new NuplaneIntegrationOptions { AutoReload = true }); + await NuplaneCoordinatorTestCases.NotifyAsync(coordinator, sourceChanged: false); + + Assert.Equal(1, catalog.RefreshCount); + Assert.Equal(2, registryState.ReloadCount); + } + + [Fact(DisplayName = "Empty, failed-only, and pre-cancelled callbacks never read the options monitor")] + public async Task IneligibleDelivery_EmptyFailedOnlyOrCancelled_DoesNotReadOptions() + { + var monitor = new TestOptionsMonitor(new NuplaneIntegrationOptions()); + var catalog = new TestRuntimeFeatureCatalog(); + var registry = TestShellRegistry.Create(out var registryState); + var coordinator = new NuplaneRefreshCoordinator(catalog, monitor, () => registry); + var empty = FakePackageAssemblyCatalog.ChangeSet(); + var failedOnly = FakePackageAssemblyCatalog.ChangeSet(added: [FakePackageAssemblyCatalog.Resolved("failed-only")]); + + await coordinator.OnPackagesReconciledAsync(empty, [], CancellationToken.None); + await coordinator.OnPackagesReconciledAsync(failedOnly, [], CancellationToken.None); + using var canceled = new CancellationTokenSource(); + canceled.Cancel(); + await Assert.ThrowsAnyAsync(() => + NuplaneCoordinatorTestCases.NotifyAsync(coordinator, cancellationToken: canceled.Token)); + + Assert.Equal(0, monitor.ReadCount); + Assert.Equal(0, catalog.RefreshCount); + Assert.Equal(0, registryState.ActiveShellReadCount); + Assert.Equal(0, registryState.ReloadCount); + } + + [Fact(DisplayName = "Options validation and invalid triggers fail before recording catalog work")] + public async Task OptionsValidationOrInvalidTrigger_FailsBeforeCatalogWorkIsRecorded() + { + var monitorReadFailure = new InvalidOperationException("Options monitor failed."); + var validationFailure = new OptionsValidationException( + Options.DefaultName, + typeof(NuplaneIntegrationOptions), + ["The configured options were rejected."]); + var monitor = new TestOptionsMonitor(new NuplaneIntegrationOptions()); + var catalog = new TestRuntimeFeatureCatalog(); + var registry = TestShellRegistry.Create(out var registryState); + var coordinator = new NuplaneRefreshCoordinator(catalog, monitor, () => registry); + monitor.ThrowOnRead(monitorReadFailure); + + var monitorThrown = await Assert.ThrowsAsync(() => NuplaneCoordinatorTestCases.NotifyAsync(coordinator)); + Assert.Same(monitorReadFailure, monitorThrown); + Assert.Equal(0, catalog.RefreshCount); + + monitor.ThrowOnRead(null); + await NuplaneCoordinatorTestCases.NotifyAsync(coordinator, sourceChanged: false); + Assert.Equal(0, catalog.RefreshCount); + Assert.Equal(1, registryState.ActiveShellReadCount); + Assert.Equal(0, registryState.ReloadCount); + + monitor.ThrowOnRead(validationFailure); + var validationThrown = await Assert.ThrowsAsync(() => NuplaneCoordinatorTestCases.NotifyAsync(coordinator)); + Assert.Same(validationFailure, validationThrown); + monitor.ThrowOnRead(null); + await NuplaneCoordinatorTestCases.NotifyAsync(coordinator, sourceChanged: false); + Assert.Equal(0, catalog.RefreshCount); + Assert.Equal(2, registryState.ActiveShellReadCount); + + monitor.Replace(new NuplaneIntegrationOptions { RefreshTrigger = (NuplaneRefreshTrigger)99 }); + await Assert.ThrowsAsync(() => NuplaneCoordinatorTestCases.NotifyAsync(coordinator)); + monitor.Replace(new NuplaneIntegrationOptions()); + await NuplaneCoordinatorTestCases.NotifyAsync(coordinator, sourceChanged: false); + + Assert.Equal(0, catalog.RefreshCount); + Assert.Equal(3, registryState.ActiveShellReadCount); + Assert.Equal(0, registryState.ReloadCount); + } + + [Fact(DisplayName = "Configuration reload changes the policy used by the registered Nuplane observer")] + public async Task ConfigurationReload_UpdatesRegisteredObserverPolicy_OnLaterDelivery() + { + var packageCatalog = new FakePackageAssemblyCatalog + { + Packages = [FakePackageAssemblyCatalog.CreatePackage("features", typeof(NuplaneCompositionTests.DiscoveredPackageFeature).Assembly)] + }; + var data = new Dictionary + { + ["NuplaneIntegration:Enabled"] = "true", + ["NuplaneIntegration:RefreshTrigger"] = nameof(NuplaneRefreshTrigger.ChangedOrPending), + ["NuplaneIntegration:AutoReload"] = "false" + }; + using var configurationRoot = (ConfigurationRoot)new ConfigurationBuilder().AddInMemoryCollection(data).Build(); + IConfiguration configuration = configurationRoot; + + await using var host = NuplaneTestHost.Build( + services => + { + services.AddSingleton(configuration); + services.AddOptions().Bind(configuration.GetSection("NuplaneIntegration")); + services.AddSingleton(packageCatalog); + }, + shells => shells.WithNuplaneFeatureDiscovery() + .AddShell("live-options", shell => shell.WithFeature("NuplaneDiscovered"))); + + var registry = host.GetRequiredService(); + var initialShell = await registry.ActivateAsync("live-options"); + var observer = host.GetRequiredService(); + await NuplaneCoordinatorTestCases.NotifyAsync(observer); + Assert.Same(initialShell, registry.GetActive("live-options")); + + var memoryProvider = Assert.IsType(configurationRoot.Providers.Single()); + memoryProvider.Set("NuplaneIntegration:AutoReload", "true"); + configurationRoot.Reload(); + await NuplaneCoordinatorTestCases.NotifyAsync(observer); + + Assert.NotSame(initialShell, registry.GetActive("live-options")); + Assert.Equal(3, packageCatalog.QueryCount); + } +} diff --git a/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs b/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs index 99fd527..8a3ea36 100644 --- a/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs +++ b/tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs @@ -2,10 +2,8 @@ using CShells.Nuplane; using CShells.Nuplane.Internal; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; using Nuplane.Abstractions; using Nuplane.Loading; -using System.Runtime.ExceptionServices; namespace CShells.Tests.Integration.Nuplane; @@ -98,7 +96,7 @@ public async Task DisabledObserverDoesNotPreventBuildFromConsumingPreviouslyPend var catalog = new TestRuntimeFeatureCatalog(); var registry = TestShellRegistry.Create(out var registryState); registryState.HasActiveShell = false; - var coordinator = new NuplaneRefreshCoordinator(catalog, Options.Create(options), () => registry); + var coordinator = new NuplaneRefreshCoordinator(catalog, new TestOptionsMonitor(options), () => registry); await NuplaneCoordinatorTestCases.NotifyAsync(coordinator); options.Enabled = false; @@ -147,11 +145,11 @@ public async Task NewerEventDuringRefreshRemainsPendingAndIsRefreshedAfterGateOp if (count == 2) secondActiveRead.TrySetResult(); }; - var coordinator = new NuplaneRefreshCoordinator(catalog, Options.Create(new NuplaneIntegrationOptions()), () => registry); + var coordinator = new NuplaneRefreshCoordinator(catalog, new TestOptionsMonitor(new NuplaneIntegrationOptions()), () => registry); Task? first = null; Task? second = null; - await RunWithGateCleanupAsync( + await NuplaneCoordinatorTestCases.RunWithGateCleanupAsync( async () => { first = NuplaneCoordinatorTestCases.NotifyAsync(coordinator); @@ -177,7 +175,7 @@ public async Task FailedRefreshRetainsFreshnessForAnUnchangedEligibleRetry() : Task.CompletedTask }; var registry = TestShellRegistry.Create(out _); - var coordinator = new NuplaneRefreshCoordinator(catalog, Options.Create(new NuplaneIntegrationOptions()), () => registry); + var coordinator = new NuplaneRefreshCoordinator(catalog, new TestOptionsMonitor(new NuplaneIntegrationOptions()), () => registry); var exception = await Assert.ThrowsAsync(() => NuplaneCoordinatorTestCases.NotifyAsync(coordinator)); Assert.Equal("refresh failed", exception.Message); @@ -198,7 +196,7 @@ public async Task FailedBuildRefreshRetainsFreshnessForTheNextBuildAttempt() }; var registry = TestShellRegistry.Create(out var registryState); registryState.HasActiveShell = false; - var coordinator = new NuplaneRefreshCoordinator(catalog, Options.Create(new NuplaneIntegrationOptions()), () => registry); + var coordinator = new NuplaneRefreshCoordinator(catalog, new TestOptionsMonitor(new NuplaneIntegrationOptions()), () => registry); await NuplaneCoordinatorTestCases.NotifyAsync(coordinator); var context = new ShellGenerationBuildContext(ShellDescriptor.Create("retry-build", 1), new ShellId("retry-build")); @@ -222,7 +220,7 @@ public async Task CancellationAfterRefreshReturnsDoesNotAcknowledgeCatalogEpoch( } }; var registry = TestShellRegistry.Create(out _); - var coordinator = new NuplaneRefreshCoordinator(catalog, Options.Create(new NuplaneIntegrationOptions()), () => registry); + var coordinator = new NuplaneRefreshCoordinator(catalog, new TestOptionsMonitor(new NuplaneIntegrationOptions()), () => registry); await Assert.ThrowsAnyAsync(() => NuplaneCoordinatorTestCases.NotifyAsync(coordinator, cancellationToken: cancellation.Token)); await NuplaneCoordinatorTestCases.NotifyAsync(coordinator, sourceChanged: false); @@ -289,12 +287,12 @@ public async Task NewSourceEpochArrivingDuringAutomaticReloadIsNotAcknowledgedBy }; var coordinator = new NuplaneRefreshCoordinator( catalog, - Options.Create(new NuplaneIntegrationOptions { AutoReload = true }), + new TestOptionsMonitor(new NuplaneIntegrationOptions { AutoReload = true }), () => registry); Task? first = null; Task? second = null; - await RunWithGateCleanupAsync( + await NuplaneCoordinatorTestCases.RunWithGateCleanupAsync( async () => { first = NuplaneCoordinatorTestCases.NotifyAsync(coordinator); @@ -316,56 +314,8 @@ private static (NuplaneRefreshCoordinator Coordinator, TestRuntimeFeatureCatalog { var catalog = new TestRuntimeFeatureCatalog(); var registry = TestShellRegistry.Create(out var registryState); - var coordinator = new NuplaneRefreshCoordinator(catalog, Options.Create(options ?? new NuplaneIntegrationOptions()), () => registry); + var coordinator = new NuplaneRefreshCoordinator(catalog, new TestOptionsMonitor(options ?? new NuplaneIntegrationOptions()), () => registry); return (coordinator, catalog, registryState); } - private static async Task RunWithGateCleanupAsync( - Func exercise, - Action releaseGates, - params Func[] inFlightTasks) - { - Exception? primaryFailure = null; - try - { - await exercise(); - } - catch (Exception exception) - { - primaryFailure = exception; - } - finally - { - releaseGates(); - } - - var cleanupFailures = new List(); - foreach (var getTask in inFlightTasks) - { - if (getTask() is not { } task) - continue; - - try - { - await task.WaitAsync(TimeSpan.FromSeconds(5)); - } - catch (Exception exception) - { - cleanupFailures.Add(exception); - } - } - - if (primaryFailure is not null) - { - if (cleanupFailures.Count > 0) - throw new AggregateException("The gated test failed and cleanup also failed.", [primaryFailure, .. cleanupFailures]); - - ExceptionDispatchInfo.Capture(primaryFailure).Throw(); - } - - if (cleanupFailures.Count == 1) - ExceptionDispatchInfo.Capture(cleanupFailures[0]).Throw(); - if (cleanupFailures.Count > 1) - throw new AggregateException("Gated test cleanup failed.", cleanupFailures); - } } diff --git a/tests/CShells.Tests/Integration/Nuplane/NuplaneReloadResultsTests.cs b/tests/CShells.Tests/Integration/Nuplane/NuplaneReloadResultsTests.cs index 647ca86..8ea9ef7 100644 --- a/tests/CShells.Tests/Integration/Nuplane/NuplaneReloadResultsTests.cs +++ b/tests/CShells.Tests/Integration/Nuplane/NuplaneReloadResultsTests.cs @@ -2,7 +2,6 @@ using CShells.Nuplane; using CShells.Nuplane.Internal; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; using Nuplane.Abstractions; using Nuplane.Loading; @@ -180,7 +179,7 @@ private static (NuplaneRefreshCoordinator Coordinator, TestRuntimeFeatureCatalog { var catalog = new TestRuntimeFeatureCatalog(); var registry = TestShellRegistry.Create(out var registryState); - var coordinator = new NuplaneRefreshCoordinator(catalog, Options.Create(options), () => registry); + var coordinator = new NuplaneRefreshCoordinator(catalog, new TestOptionsMonitor(options), () => registry); return (coordinator, catalog, registry, registryState); } } From c65d18a3d55d2d26fddc60485d4ff52e906f40fe Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 8 Oct 2026 23:01:00 +0200 Subject: [PATCH 3/4] docs: record live options source qualification --- specs/023-live-nuplane-options/quickstart.md | 16 +++++++++++++++- specs/023-live-nuplane-options/tasks.md | 18 +++++++++--------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/specs/023-live-nuplane-options/quickstart.md b/specs/023-live-nuplane-options/quickstart.md index 56ce64e..7ac77cf 100644 --- a/specs/023-live-nuplane-options/quickstart.md +++ b/specs/023-live-nuplane-options/quickstart.md @@ -1,6 +1,6 @@ # Validation: Apply Live Nuplane Integration Options -This guide defines the proof to run on the final implementation head. No implementation or acceptance test is claimed by this planning artifact. +This guide defines the source and publication gates for Task #161. The source checks below passed; actual public-package acceptance remains pending. ## Deterministic owner tests @@ -43,3 +43,17 @@ After normal owner-repository merge and package publication, audit all ten actua ## Acceptance boundary Record each exact source/package identity and results. This feature does not complete stable release, Foundation pin/lock updates, Foundation Host/Workbench adoption, `#2164` readability acceptance, or `#2362` unloading/pruning work. + +## Source qualification record (2026-10-08) + +Product/test head: `8cb10fba9e58c68040da993a9e634fad02f4a372`, based on `353f9e37db0607d9fb1540e139bce02aecf14e16`. Subsequent validation-record changes contain documentation only. Root and independent source review found no material blocker after correcting disabled/pending counter assertions, cleanup ownership, and real configuration reload coverage. + +| Gate | Exact command/result | +| --- | --- | +| Focused adapter tests | `dotnet test tests/CShells.Tests/CShells.Tests.csproj -c Release --no-restore --filter 'FullyQualifiedName~CShells.Tests.Integration.Nuplane' --logger trx` — 34 passed, zero failed/skipped. | +| Full library suite | `dotnet test tests/CShells.Tests/CShells.Tests.csproj -c Release --no-build --no-restore --logger trx` — 795 passed, zero failed/skipped. | +| Adapter Release targets | `dotnet build src/CShells.Nuplane/CShells.Nuplane.csproj -c Release --no-restore` — net8.0, net9.0, net10.0 passed, zero warnings/errors. | +| Compiled causal mutation | Isolated exact-head worktree; replace monitor field with construction-time `CurrentValue` capture and use it in policy capture. Run `dotnet test tests/CShells.Tests/CShells.Tests.csproj -c Release --filter FullyQualifiedName~ConfigurationReload_UpdatesRegisteredObserverPolicy_OnLaterDelivery --logger trx` — one executed, one failed at `Assert.NotSame` because the shell was not reloaded. Exit 1; no compilation or timeout failure. | +| Byte-identical restoration | Restore coordinator SHA-256 `6cfb3cb0cc725994b508cb9838f75bb849f05156eba574b9623475c351106092`; repeat the mutation command with `--no-restore` — one executed/passed, zero failed/skipped, exit 0. | + +TRX, build logs, both mutation sources, command arrays, hashes, and root/independent review records are retained with the program delivery artifacts. Hosted owner-repository CI, normal merge/main publication, all ten public archives, and the outside-checkout three-runtime consumer remain required before T010 and issue closure. Copilot was not requested; Greptile is optional under the maintainer's D15 decision. diff --git a/specs/023-live-nuplane-options/tasks.md b/specs/023-live-nuplane-options/tasks.md index 7413ab1..e4265c4 100644 --- a/specs/023-live-nuplane-options/tasks.md +++ b/specs/023-live-nuplane-options/tasks.md @@ -12,13 +12,13 @@ **Purpose**: Provide a focused monitor test double shared by coordinator tests. -- [ ] T001 Add a minimal `IOptionsMonitor` test helper with current-value read counting, current-value replacement, injected read failure, and the required no-op `OnChange` implementation in `tests/CShells.Tests/Integration/Nuplane/NuplaneCoordinatorTestHarness.cs`. +- [x] T001 Add a minimal `IOptionsMonitor` test helper with current-value read counting, current-value replacement, injected read failure, and the required no-op `OnChange` implementation in `tests/CShells.Tests/Integration/Nuplane/NuplaneCoordinatorTestHarness.cs`. ## Phase 2: Foundational Options Wiring **Purpose**: Make the existing root coordinator and direct test construction resolve the standard monitor contract before implementing per-delivery behavior. -- [ ] T002 Change the root coordinator factory in `src/CShells.Nuplane/CShellsNuplaneBuilderExtensions.cs` and the constructor in `src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs` to use `IOptionsMonitor`, then migrate direct coordinator construction in `tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs` and `tests/CShells.Tests/Integration/Nuplane/NuplaneReloadResultsTests.cs` to the shared helper while preserving current test behavior. +- [x] T002 Change the root coordinator factory in `src/CShells.Nuplane/CShellsNuplaneBuilderExtensions.cs` and the constructor in `src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs` to use `IOptionsMonitor`, then migrate direct coordinator construction in `tests/CShells.Tests/Integration/Nuplane/NuplaneRefreshCoordinatorTests.cs` and `tests/CShells.Tests/Integration/Nuplane/NuplaneReloadResultsTests.cs` to the shared helper while preserving current test behavior. ## Phase 3: User Story 1 - Apply updated settings on the next delivery (P1) @@ -26,9 +26,9 @@ **Independent Test**: An outside or DI-composed observer receives one eligible event with reload off, configuration is reloaded to turn reload on, and a later event reloads the active shell without rebuilding the provider. A gate proves changes during an admitted operation apply only to the next event. -- [ ] T003 [US1] Update `src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs` to read `CurrentValue` once after null/cancellation/eligibility checks, validate the enum, copy all four policy values before epochs/registry/awaits, and use only that private immutable snapshot through refresh, reload, and result reporting. -- [ ] T004 [US1] Add deterministic tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs` for false-to-true automatic reload across deliveries and for a gated refresh where mutating the captured settings object and replacing `OnReloadResults` does not alter the current operation but the next delivery sees the new values. -- [ ] T005 [US1] Add or extend a DI integration test in `tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs` to prove configuration reload reaches the adapter through standard options binding, while retaining fluent and dependency-aware configuration coverage. +- [x] T003 [US1] Update `src/CShells.Nuplane/Internal/NuplaneRefreshCoordinator.cs` to read `CurrentValue` once after null/cancellation/eligibility checks, validate the enum, copy all four policy values before epochs/registry/awaits, and use only that private immutable snapshot through refresh, reload, and result reporting. +- [x] T004 [US1] Add deterministic tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs` for false-to-true automatic reload across deliveries and for a gated refresh where mutating the captured settings object and replacing `OnReloadResults` does not alter the current operation but the next delivery sees the new values. +- [x] T005 [US1] Add or extend a DI integration test in `tests/CShells.Tests/Integration/Nuplane/NuplaneCompositionTests.cs` to prove configuration reload reaches the adapter through standard options binding, while retaining fluent and dependency-aware configuration coverage. ## Phase 4: User Story 2 - Preserve pending work and fail bad policy before side effects (P2) @@ -36,15 +36,15 @@ **Independent Test**: Gate a freshness/reload attempt, toggle enabled/reload policy, and assert existing epochs survive and can be retried without unnecessary catalog work. Use monitor, registry, and catalog counters to prove empty/canceled/error paths remain side-effect free. -- [ ] T006 [US2] Add tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs` to prove disabling observer work records no new epochs while a build consumes prior freshness without reading options, and automatic-reload-off retains pending promotion work for a later enabled delivery without rescanning fresh catalog state. -- [ ] T007 [US2] Add monitor-read, catalog, and registry counter tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs` for failed-only, empty, pre-canceled, undefined refresh-trigger, throwing monitor, and options-validation failures; follow rejected invalid reads with a valid unchanged delivery/build and prove no work was recorded. -- [ ] T008 [US2] Update `src/CShells.Nuplane/README.md` to document next-eligible-delivery settings, per-operation callback capture, preserved pending work, standard configuration paths, and direct `IOptions`-only replacement as unsupported. +- [x] T006 [US2] Add tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs` to prove disabling observer work records no new epochs while a build consumes prior freshness without reading options, and automatic-reload-off retains pending promotion work for a later enabled delivery without rescanning fresh catalog state. +- [x] T007 [US2] Add monitor-read, catalog, and registry counter tests in `tests/CShells.Tests/Integration/Nuplane/NuplaneLiveOptionsTests.cs` for failed-only, empty, pre-canceled, undefined refresh-trigger, throwing monitor, and options-validation failures; follow rejected invalid reads with a valid unchanged delivery/build and prove no work was recorded. +- [x] T008 [US2] Update `src/CShells.Nuplane/README.md` to document next-eligible-delivery settings, per-operation callback capture, preserved pending work, standard configuration paths, and direct `IOptions`-only replacement as unsupported. ## Phase 5: Verification and Publication Qualification **Purpose**: Prove the behavioral contract, catch restoration of the stale snapshot, and validate the actual delivered packages. -- [ ] T009 Run the focused Nuplane tests, full `tests/CShells.Tests/CShells.Tests.csproj` suite, and Release build of `src/CShells.Nuplane/CShells.Nuplane.csproj` for net8.0/net9.0/net10.0; then perform a compiled frozen-options mutation that fails the dynamic options test and verify byte-identical restoration passes, recording exact-head evidence in `specs/023-live-nuplane-options/quickstart.md`. +- [x] T009 Run the focused Nuplane tests, full `tests/CShells.Tests/CShells.Tests.csproj` suite, and Release build of `src/CShells.Nuplane/CShells.Nuplane.csproj` for net8.0/net9.0/net10.0; then perform a compiled frozen-options mutation that fails the dynamic options test and verify byte-identical restoration passes, recording exact-head evidence in `specs/023-live-nuplane-options/quickstart.md`. - [ ] T010 After normal owner-repository PR merge and main checks, audit all ten public package archives and run an outside-checkout, fresh-cache PackageReference-only dynamic-options consumer on actual .NET 8, 9, and 10 runtimes; record source/package identity, cache source, hashes, loaded DLL assets, exact commands, and scenario results in `specs/023-live-nuplane-options/quickstart.md` and the owning issue. ## Dependencies and Execution Order From d2f1000d7e2a87b8731a6a187a8288320ee91f75 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 8 Oct 2026 23:07:31 +0200 Subject: [PATCH 4/4] test: hold drain open during concurrent identity checks --- specs/023-live-nuplane-options/quickstart.md | 4 + .../Lifecycle/ShellDrainPropertyTests.cs | 90 +++++++++++++++---- 2 files changed, 78 insertions(+), 16 deletions(-) diff --git a/specs/023-live-nuplane-options/quickstart.md b/specs/023-live-nuplane-options/quickstart.md index 7ac77cf..62699f5 100644 --- a/specs/023-live-nuplane-options/quickstart.md +++ b/specs/023-live-nuplane-options/quickstart.md @@ -57,3 +57,7 @@ Product/test head: `8cb10fba9e58c68040da993a9e634fad02f4a372`, based on `353f9e3 | Byte-identical restoration | Restore coordinator SHA-256 `6cfb3cb0cc725994b508cb9838f75bb849f05156eba574b9623475c351106092`; repeat the mutation command with `--no-restore` — one executed/passed, zero failed/skipped, exit 0. | TRX, build logs, both mutation sources, command arrays, hashes, and root/independent review records are retained with the program delivery artifacts. Hosted owner-repository CI, normal merge/main publication, all ten public archives, and the outside-checkout three-runtime consumer remain required before T010 and issue closure. Copilot was not requested; Greptile is optional under the maintainer's D15 decision. + +### Hosted CI ordering correction + +PR #162's first CI run (`37843878579`) built successfully but failed one existing lifecycle test (794/795 library tests; 31/31 end-to-end). `Drain_SameInstance_AcrossConcurrentDrainAsyncCalls` issued synchronous calls without holding the drain open; the first operation could finish and release its pointer before later calls. The corrected test uses the existing blocking drain feature, releases 16 callers through an async barrier, and compares the same in-flight operation before releasing the drain. Cleanup joins all owned calls and distinct drain handles and preserves assertion and cleanup failures. No lifecycle product code changed. Root and independent delta review passed; `dotnet test tests/CShells.Tests/CShells.Tests.csproj -c Release --no-restore --filter FullyQualifiedName~ShellDrainPropertyTests --logger trx` passed all five tests, with zero failures/skips. Corrected-head hosted CI remains required. diff --git a/tests/CShells.Tests/Integration/Lifecycle/ShellDrainPropertyTests.cs b/tests/CShells.Tests/Integration/Lifecycle/ShellDrainPropertyTests.cs index 8b13e8d..12d77de 100644 --- a/tests/CShells.Tests/Integration/Lifecycle/ShellDrainPropertyTests.cs +++ b/tests/CShells.Tests/Integration/Lifecycle/ShellDrainPropertyTests.cs @@ -1,3 +1,4 @@ +using System.Runtime.ExceptionServices; using CShells.DependencyInjection; using CShells.Features; using CShells.Lifecycle; @@ -108,28 +109,85 @@ public async Task Drain_SameInstance_AsRegistryDrainAsyncReturn() [Fact(DisplayName = "Drain is the same instance across concurrent DrainAsync calls (publish-once CAS)")] public async Task Drain_SameInstance_AcrossConcurrentDrainAsyncCalls() { - await using var host = ShellRegistryActivateTests.BuildHost(cshells => cshells - .WithAssemblies() - .AddShell("epsilon", _ => { })); + var gate = new BlockingDrainGate(); + await using var host = ShellRegistryActivateTests.BuildHost(cshells => + { + cshells.Services.AddSingleton(gate); + cshells + .WithAssemblyContaining() + .AddShell("epsilon", shell => shell.WithFeature()); + }); var registry = host.GetRequiredService(); var shell = await registry.ActivateAsync("epsilon"); - // Race 16 concurrent DrainAsync calls. The first to win the CAS publishes its - // DrainOperation onto the shell; every subsequent caller observes the same instance. - var tasks = Enumerable - .Range(0, 16) - .Select(_ => registry.DrainAsync(shell)) + var start = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var calls = Enumerable.Range(0, 16) + .Select(_ => StartDrainCallAsync()) .ToArray(); - var results = await Task.WhenAll(tasks); + var operations = new HashSet(ReferenceEqualityComparer.Instance); + Exception? primaryFailure = null; - var first = results[0]; - Assert.All(results, r => Assert.Same(first, r)); + async Task StartDrainCallAsync() + { + await start.Task; + return await registry.DrainAsync(shell); + } + + try + { + start.TrySetResult(); + await gate.Started.WaitAsync(TimeSpan.FromSeconds(5)); + var results = await Task.WhenAll(calls).WaitAsync(TimeSpan.FromSeconds(5)); + + var first = results[0]; + Assert.All(results, operation => Assert.Same(first, operation)); + } + catch (Exception exception) + { + primaryFailure = exception; + } + finally + { + start.TrySetResult(); + gate.Release(); + } + + var cleanupFailures = new List(); + foreach (var call in calls) + { + try + { + var operation = await call.WaitAsync(TimeSpan.FromSeconds(5)); + operations.Add(operation); + } + catch (Exception exception) + { + cleanupFailures.Add(exception); + } + } + + foreach (var operation in operations) + { + try + { + await operation.WaitAsync().WaitAsync(TimeSpan.FromSeconds(5)); + } + catch (Exception exception) + { + cleanupFailures.Add(exception); + } + } + + if (primaryFailure is not null) + { + if (cleanupFailures.Count > 0) + throw new AggregateException("The drain identity assertion and cleanup both failed.", [primaryFailure, .. cleanupFailures]); + + ExceptionDispatchInfo.Capture(primaryFailure).Throw(); + } - // Don't assert shell.Drain here — with no handlers the drain can complete and dispose - // the shell before Task.WhenAll returns. The CAS contract is - // already proven by Assert.All above; shell.Drain identity is covered by - // Drain_SameInstance_AsRegistryDrainAsyncReturn. - await first.WaitAsync().WaitAsync(TimeSpan.FromSeconds(5)); + if (cleanupFailures.Count > 0) + throw new AggregateException("Concurrent drain cleanup failed.", cleanupFailures); } public sealed class BlockingDrainFeature : IShellFeature