Preserve Nuplane observer failure diagnostics

.specify/feature.json

@@ -1,3 +1,3 @@
 {
-  "feature_directory": "specs/023-live-nuplane-options"
+  "feature_directory": "specs/024-nuplane-failure-diagnostics"
 }

AGENTS.md

@@ -162,10 +162,11 @@ When asked to review or address PR review comments, think critically about each
 - C# 14 / .NET 10; source projects multi-target `net8.0;net9.0;net10.0` per repository conventions + Existing `Microsoft.Extensions.Configuration`, `Microsoft.Extensions.DependencyInjection`, `System.Text.Json`; no new third-party packages (014-polymorphic-feature-config)
 - N/A; configuration provider inputs only (014-polymorphic-feature-config)
 - C# 14; source packages multi-target `net8.0;net9.0;net10.0` + Existing `Microsoft.Extensions.DependencyInjection`, `Microsoft.Extensions.Logging`, and `System.Collections.Immutable`; no new package dependencies (018-shell-generation-build-leases)
+- C# 14 / .NET 10 SDK; source library targets `net8.0;net9.0;net10.0` + Existing Nuplane abstractions, Microsoft.Extensions.Options, and a direct `Microsoft.Extensions.Logging.Abstractions` dependency; test-only reference to Nuplane's real observer-dispatcher package (024-nuplane-failure-diagnostics)
 
 ## Recent Changes
 - 011-map-shell-config: Planned map-based shell configuration under `CShells:Shells`
 
 <!-- SPECKIT START -->
-For the active work unit, read `specs/023-live-nuplane-options/plan.md`.
+For the active work unit, read `specs/024-nuplane-failure-diagnostics/plan.md`.
 <!-- SPECKIT END -->

Directory.Packages.props

@@ -44,6 +44,7 @@
     <!-- Others -->
     <PackageVersion Include="Nuplane.Abstractions" Version="0.0.11-preview.99" />
     <PackageVersion Include="Nuplane.Loading.Abstractions" Version="0.0.11-preview.99" />
+    <PackageVersion Include="Nuplane" Version="0.0.11-preview.99" />
     <PackageVersion Include="FluentStorage" Version="6.0.0" />
     <PackageVersion Include="JetBrains.Annotations" Version="2025.2.4" />
     <!-- Testing -->

specs/024-nuplane-failure-diagnostics/checklists/requirements.md

@@ -0,0 +1,35 @@
+# Specification Quality Checklist: Nuplane Observer Failure Diagnostics
+
+**Purpose**: Validate specification completeness and quality before planning
+**Created**: 2026-10-09
+**Feature**: [spec.md](../spec.md)
+
+## Content Quality
+
+- [x] No implementation details beyond observable logging and propagation behavior
+- [x] Focused on host-maintainer value and operational diagnosis
+- [x] User scenarios are written for package-integration maintainers
+- [x] All mandatory sections are completed
+
+## Requirement Completeness
+
+- [x] No `[NEEDS CLARIFICATION]` markers remain
+- [x] Requirements are testable and unambiguous
+- [x] Success criteria are measurable
+- [x] Success criteria describe observable outcomes
+- [x] Acceptance scenarios are defined for the ordinary failure and exclusion paths
+- [x] Edge cases cover eligibility, cancellation, fatal exceptions, and existing result semantics
+- [x] Scope is bounded to Nuplane observer diagnostics
+- [x] Dependencies and assumptions are identified
+
+## Feature Readiness
+
+- [x] Functional requirements have observable acceptance criteria
+- [x] The primary user journey is covered by the P1 story
+- [x] Success criteria map to injected failure and observer-dispatch evidence
+- [x] No unsupported feature or release claim is made
+
+## Notes
+
+- The accepted issue body resolves the implementation and failure-classification choices; no owner clarification is needed.
+- Build-time freshness and returned per-shell reload results are explicitly excluded from the new exception diagnostics.

specs/024-nuplane-failure-diagnostics/contracts/observer-diagnostic-contract.md

@@ -0,0 +1,26 @@
+# Observer Diagnostic Contract
+
+This document describes an observable behavior of the existing optional adapter. It introduces no public type, method, option, or Nuplane API.
+
+## Ordinary admitted-operation failure
+
+For a nonfatal exception thrown by catalog refresh, active-registry access, reload invocation, or the configured reload-results callback during an enabled, eligible reconciliation delivery, the adapter emits exactly one `Error` record before propagating the failure.
+
+The record carries:
+
+- the exact original exception object as the logging exception;
+- structured `CorrelationId`, copied from `PackageChangeSet.CorrelationId`;
+- structured `Operation`, identifying the observer callback as `OnPackagesReconciledAsync`.
+
+The adapter rethrows that same exception instance. It does not format and substitute the exception, wrap it, swallow it, or clear pending work. Nuplane's actual `ObserverEventDispatcher` remains responsible for its existing Warning and continuation to later observers.
+
+## Exclusions and unchanged behavior
+
+- Do not Error-log a thrown `OperationCanceledException` when the callback token is requested. An uncanceled `OperationCanceledException` is an ordinary operation failure.
+- Do not Error-log `OutOfMemoryException`, `StackOverflowException`, `AccessViolationException`, `AppDomainUnloadedException`, or `BadImageFormatException`.
+- Do not extend the new diagnostic boundary to build-time freshness in `BeginAsync`.
+- A returned `ReloadResult.Error` is not a thrown failure and is not newly Error-logged; preserve the existing result callback and pending-reload behavior.
+- If the host has no logging provider, construction and operation remain valid through the null logger fallback.
+- Exception-preserving diagnostics assume registered logging providers do not throw from `ILogger.Log`; logger-provider failures are outside this adapter contract.
+
+No particular log message wording or event ID is a public compatibility promise. The exception object, structured correlation, observer-operation name, Error level, one-record count, and exception propagation are the contract.

specs/024-nuplane-failure-diagnostics/data-model.md

@@ -0,0 +1,12 @@
+# Data Model: Nuplane Observer Failure Diagnostics
+
+This change adds no persisted data, public model, coordinator state, or new identifier.
+
+| Existing input | Use in diagnostics | Boundary |
+|---|---|---|
+| `PackageChangeSet.CorrelationId` | Structured `CorrelationId` value on the adapter's Error record | Copied from the current reconciliation delivery; Nuplane uses the same value on its observer warning. |
+| Thrown `Exception` | The exception argument on one Error record and the exception rethrown from the observer | The object is not wrapped, translated, or replaced. |
+| Observer operation | Fixed structured `Operation` value `OnPackagesReconciledAsync` | Identifies the event boundary; the original exception remains the evidence for the specific failing call. No stage tracker is added. |
+| Callback cancellation token | Determines whether a thrown `OperationCanceledException` represents caller-requested cancellation | Suppression applies only when the token is requested and the thrown exception is `OperationCanceledException`. |
+
+Failure classification is limited to the existing observer callback. An `OperationCanceledException` with an uncanceled token is an ordinary failure. `OutOfMemoryException`, `StackOverflowException`, `AccessViolationException`, `AppDomainUnloadedException`, and `BadImageFormatException` remain unlogged fatal exceptions. A returned `ReloadResult.Error` is data for the configured result callback, not a thrown operation failure.

specs/024-nuplane-failure-diagnostics/plan.md

@@ -0,0 +1,68 @@
+# Implementation Plan: Nuplane Observer Failure Diagnostics
+
+**Branch**: `024-nuplane-failure-diagnostics` | **Date**: 2026-10-09 | **Spec**: [spec.md](spec.md)
+**Input**: Feature specification from `specs/024-nuplane-failure-diagnostics/spec.md`
+
+## Summary
+
+Make admitted Nuplane observer operation failures visible as one structured Error record carrying the original exception and reconciliation correlation, then preserve existing exception propagation and pending-work retry. Add an optional typed logger to the private coordinator, use `NullLogger` when no provider is registered, and resolve the logger in the existing explicit coordinator-holder factory. Leave build-time freshness, public APIs, coordinator state, and Nuplane's observer isolation unchanged.
+
+## Technical Context
+
+**Language/Version**: C# 14 / .NET 10 SDK; source library targets `net8.0;net9.0;net10.0`
+**Primary Dependencies**: Existing Nuplane abstractions, Microsoft.Extensions.Options, and a direct `Microsoft.Extensions.Logging.Abstractions` dependency; test-only reference to Nuplane's real observer-dispatcher package
+**Storage**: N/A
+**Testing**: xUnit integration tests; source project Release builds for all three target frameworks; public PackageReference consumer qualification for all three runtime versions
+**Target Platform**: .NET 8, .NET 9, and .NET 10
+**Project Type**: Multi-target .NET library and integration-test project
+**Performance Goals**: No additional work on ineligible callbacks or `BeginAsync`; one Error logging call only on an admitted operation failure
+**Constraints**: Preserve the original exception instance, existing epoch acknowledgements/retry, cancellation and fatal-exception exclusions, logger-optional composition, and observer continuation through Nuplane's dispatcher
+**Scale/Scope**: The existing private coordinator and its package/test qualification only; no new public API, public options, timers, state machine, host policy, or package-store mutation
+
+## Constitution Check
+
+**Pre-design gate**
+
+- **I. Abstraction-First Architecture**: Pass. This adds no public consumer contract and no abstraction project. The logging dependency belongs only to the optional implementation package.
+- **III. Modern C# Style**: Pass. Keep nullable annotations, file-scoped namespaces, idiomatic `var`, and existing source multi-targets.
+- **IV. Explicit Error Handling**: Pass. Failures are Error-logged and rethrown unchanged; no coordinator swallowing or exception wrapping is introduced.
+- **V. Test Coverage**: Pass. Add focused xUnit cases for the three failure sources, exclusions, DI, and the actual observer dispatcher.
+- **VI. Simplicity & Minimalism**: Pass. Reuse the existing coordinator, holder, logging abstractions, and options; do not add a logger abstraction or helper layer.
+- **VII. Lifecycle & Concurrency Contracts**: Pass. Keep the existing `finally` gate release and epoch acknowledgement paths; do not alter Nuplane fan-out isolation. Nuplane's dispatcher remains the boundary that catches the rethrown observer exception and continues to later observers.
+
+No constitution exception or unresolved gate remains.
+
+**Post-design gate**: The design adds only a private logger dependency and a catch/log/rethrow boundary around already-admitted observer work. It introduces no locks, async work, state transitions, public contract, or provider ownership. All gates remain satisfied.
+
+## Design and Research
+
+- [Research decisions](research.md) records source-backed choices and rejected alternatives.
+- [Failure diagnostic contract](contracts/observer-diagnostic-contract.md) records the externally observable log/exception boundary without defining a new API.
+- [Data model](data-model.md) describes the existing reconciliation correlation; no durable entities or coordinator state are added.
+- [Quickstart and validation](quickstart.md) gives the composition shape and focused verification sequence.
+
+## Project Structure
+
+```text
+Directory.Packages.props                              # central test-only Nuplane runtime pin at 0.0.11-preview.99
+src/CShells.Nuplane/
+├── CShells.Nuplane.csproj                         # direct logging abstractions dependency
+├── CShellsNuplaneBuilderExtensions.cs             # manual holder factory supplies optional logger
+├── Internal/
+│   └── NuplaneRefreshCoordinator.cs               # failure classification, one Error, unchanged rethrow
+└── README.md                                      # optional logging behavior and diagnostic boundary
+
+tests/CShells.Tests/CShells.Tests.csproj            # test-only Nuplane dispatcher package reference
+tests/CShells.Tests/Integration/Nuplane/
+├── NuplaneRefreshCoordinatorTests.cs              # refresh failure, identity, exclusion, retry
+├── NuplaneReloadResultsTests.cs                   # registry/reload and callback failures, identity, retry
+├── NuplaneCompositionTests.cs                      # DI logger wiring and no-provider composition
+├── NuplaneObserverDiagnosticsTests.cs              # actual Nuplane dispatcher + following observer
+└── NuplaneTestLoggerProvider.cs                     # shared structured log capture for integration tests
+```
+
+**Structure Decision**: Keep production changes in the existing optional adapter. Add a trailing optional `ILogger<NuplaneRefreshCoordinator>? logger = null` constructor parameter and use `NullLogger<NuplaneRefreshCoordinator>.Instance` when absent; pass `serviceProvider.GetService<ILogger<NuplaneRefreshCoordinator>>()` through the existing holder factory. Add Nuplane `0.0.11-preview.99` only to the net10 integration-test project, with a central test pin, so acceptance can exercise the actual `ObserverEventDispatcher` and `ReconciliationLogger`; do not add a runtime dependency from the adapter package or use a test-only dispatch shim.
+
+## Complexity Tracking
+
+No constitution violations or additional architectural components are planned.

specs/024-nuplane-failure-diagnostics/quickstart.md

@@ -0,0 +1,29 @@
+# Quickstart: Nuplane Observer Failure Diagnostics
+
+The diagnostic behavior is automatic when the optional Nuplane adapter is selected and the host has a logging provider. No new adapter option or public logging API is required.
+
+```csharp
+using CShells.DependencyInjection;
+using CShells.Nuplane;
+using Microsoft.Extensions.DependencyInjection;
+
+var services = new ServiceCollection();
+services.AddLogging();
+services.AddCShells(shells => shells.WithNuplaneFeatureDiscovery());
+```
+
+If the host does not register logging, the adapter uses its null logger fallback and retains the existing exception behavior.
+
+## Focused validation
+
+Run from the repository root through the standard `dotnet` build-slot wrapper:
+
+```sh
+dotnet test tests/CShells.Tests/CShells.Tests.csproj -c Release --filter 'FullyQualifiedName~Nuplane'
+dotnet build src/CShells.Nuplane/CShells.Nuplane.csproj -c Release -f net8.0
+dotnet build src/CShells.Nuplane/CShells.Nuplane.csproj -c Release -f net9.0
+dotnet build src/CShells.Nuplane/CShells.Nuplane.csproj -c Release -f net10.0
+dotnet test tests/CShells.Tests/CShells.Tests.csproj -c Release
+```
+
+Before issue closure, pass the required organization PR/review and main-branch CI gates, then use the separate PackageReference-only diagnostic consumer against the resulting owner-published corrective preview on actual .NET 8, .NET 9, and .NET 10 runtimes. Audit all ten CShells archives against successful owner workflow metadata and public-feed bytes first. The pre-correction preview counterexample is not correction proof. No manual package publication, stable-pin change, or stable-release qualification belongs to this work unit.

specs/024-nuplane-failure-diagnostics/research.md

@@ -0,0 +1,34 @@
+# Research: Nuplane Observer Failure Diagnostics
+
+## Decisions
+
+### 1. Use the existing typed-logger convention and optional fallback
+
+- **Decision**: Give the private `NuplaneRefreshCoordinator` an optional `ILogger<NuplaneRefreshCoordinator>` and use `NullLogger<NuplaneRefreshCoordinator>.Instance` when absent. Resolve the optional logger in the existing `CoordinatorHolder` service factory and pass it explicitly to the coordinator. Add a direct `Microsoft.Extensions.Logging.Abstractions` package reference to `CShells.Nuplane.csproj`.
+- **Rationale**: Other CShells services use this optional typed-logger pattern. The adapter is optional and is also composed in service collections without logging. `Directory.Packages.props` already pins `Microsoft.Extensions.Logging.Abstractions` for all three target-framework groups (9.0.11 for net8/net9 and 10.0.0 for net10), so no new central version is required.
+- **Alternatives considered**: A required logger would break no-provider composition; adding an adapter-specific logger abstraction would add a layer with no current consumer need.
+
+### 2. Keep the diagnostic boundary after existing admission and policy validation
+
+- **Decision**: Preserve argument guards, the caller-token precheck, ineligible-completion return, and option/trigger validation before the new Error boundary. Do not change `BeginAsync`. For admitted observer work, log one Error with the original exception and `changeSet.CorrelationId`, then use a bare rethrow so the same instance reaches Nuplane's dispatcher or a direct caller. Do not change gate `finally` blocks or acknowledge epochs on failure.
+- **Rationale**: `PackageChangeSet.CorrelationId` is Nuplane's documented reconciliation-cycle identifier, and `ObserverEventDispatcher` already places that same value on its warning. The accepted Foundation contract requires the original exception and this correlation; the coordinator's internal freshness epoch is not the reconciliation ID.
+- **Alternatives considered**: Logging inside `BeginAsync` would extend diagnostics to a distinct build-time operation. Wrapping or swallowing the exception would break upstream exception-identity tests and observer isolation semantics.
+
+### 3. Preserve the established cancellation and fatal-exception filter
+
+- **Decision**: Do not Error-log an `OperationCanceledException` only when the callback token is requested. Treat an uncanceled `OperationCanceledException` as an ordinary failure. Do not Error-log `OutOfMemoryException`, `StackOverflowException`, `AccessViolationException`, `AppDomainUnloadedException`, or `BadImageFormatException`. Other admitted nonfatal exceptions are logged and rethrown.
+- **Rationale**: This is the concrete exclusion behavior used by Foundation's existing observer integration. It distinguishes caller cancellation from an operation that merely happens to throw an `OperationCanceledException`.
+- **Alternatives considered**: Excluding every `OperationCanceledException` would hide non-cancellation failures; excluding every exception whenever the token is signaled would also hide unrelated faults.
+
+### 4. Verify both the coordinator boundary and Nuplane's real fan-out
+
+- **Decision**: Use focused coordinator tests for refresh, registry/reload, and callback failures, and a DI-backed test that resolves the public Nuplane `ObserverEventDispatcher` / `ReconciliationLogger` path with a following observer. Add `Nuplane` `0.0.11-preview.99` only to the net10 test project with a central pin; do not add it to the shipping adapter graph. Keep a no-logging-provider composition test.
+- **Rationale**: The actual published dispatcher catches observer exceptions, records its existing Warning with correlation and message only, and continues through later observers. An independently reviewed package probe showed the current adapter has no Error entry and the dispatcher warning lacks the exception object. The fix must add the adapter Error without replacing this established fan-out behavior.
+- **Alternatives considered**: Direct observer invocation alone cannot prove the actual dispatcher continues after the adapter's rethrow. A test-only dispatch shim cannot prove the public runtime path.
+
+## Source Findings
+
+- The coordinator currently owns refresh/reload epoch gates and preserves failed work for later eligible deliveries. Keep this state machine unchanged.
+- `CShellsNuplaneBuilderExtensions` creates the private coordinator manually inside the `CoordinatorHolder` factory; this is the point to pass a logger resolved with optional `GetService`.
+- The test project already references the adapter project. It currently sees only Nuplane abstractions transitively; the real `ObserverEventDispatcher` is in the Nuplane runtime package. The external public-package probe provides a concrete registration/dispatch pattern. No new test project or public contract is needed.
+- Returned `ReloadResult.Error` values flow to the existing options callback and retain their current meaning; only thrown operation failures enter this diagnostics boundary.