feat!: modernize host setup API with ASP.NET Core minimal API pattern

[thomhurst]

Summary

• Implements a complete redesign of the pipeline configuration API to follow ASP.NET Core's WebApplicationBuilder pattern
• Replaces callback-based PipelineHostBuilder with new PipelineBuilder that exposes Services, Configuration, and Options directly
• Provides a cleaner, more intuitive developer experience aligned with modern .NET patterns

Breaking Changes

The entire host setup API has been replaced:

Before (callback-based):

await PipelineHostBuilder.Create()
    .ConfigureServices((context, services) => { 
        services.AddModule<MyModule>(); 
    })
    .ConfigurePipelineOptions((context, options) => { 
        options.DefaultRetryCount = 3;
    })
    .ExecutePipelineAsync();

After (direct property access):

var builder = Pipeline.CreateBuilder();
builder.Services.AddModule<MyModule>();
builder.Options.DefaultRetryCount = 3;
await builder.Build().RunAsync();

Changes

New API

• Pipeline.CreateBuilder() - Static factory method (like WebApplication.CreateBuilder())
• PipelineBuilder - Builder with direct property access:
  • .Services - IServiceCollection for DI registration
  • .Configuration - ConfigurationManager for configuration sources
  • .Options - PipelineOptions for pipeline settings
  • .Environment - IHostEnvironment for environment info
• IPipeline - Interface for built pipelines (replaces IPipelineHost)
• PipelineBuilderExtensions - Convenience methods like AddModule<T>(), ConfigureServices(), etc.

Removed

• PipelineHostBuilder class
• PipelineHost class
• IPipelineHost interface
• HostExtensions class
• All callback-based configuration methods

Test Plan

• Build passes with 0 errors
• All 892 unit tests pass (6 skipped)
• Updated Build pipeline (ModularPipelines.Build) works correctly
• Updated test helpers work correctly

Closes #2065

🤖 Generated with Claude Code

[thomhurst]

Summary

Redesigns the pipeline configuration API from callback-based PipelineHostBuilder to a direct property access pattern (PipelineBuilder) following ASP.NET Core's WebApplicationBuilder pattern.

Critical Issues

1. CRITICAL: Finalizer calling async disposal synchronously

In PipelineImpl.cs:

~PipelineImpl()
{
    DisposeAsync().AsTask().GetAwaiter().GetResult();
}

Problem: Calling .GetAwaiter().GetResult() in a finalizer can cause deadlocks on thread pool starvation, especially if DisposeAsync has continuations that need to marshal back to specific synchronization contexts. Finalizers run on the finalizer thread and blocking it can cause serious runtime issues.

Recommendation: Follow the standard pattern - finalizers should only dispose unmanaged resources. Remove the finalizer entirely since this class only manages managed resources (IHost, AsyncServiceScope), or implement IDisposable with a proper dispose pattern that tracks whether async disposal has occurred.

Previous implementation (PipelineHost.cs):

~PipelineHost()
{
    Dispose();  // Called non-async Dispose()
}

public void Dispose()
{
    #pragma warning disable CS4014
    DisposeAsync();  // Fire and forget - also problematic but different
    #pragma warning restore CS4014
}

The old implementation was also problematic (fire-and-forget), but the new one is worse (blocking). Best practice: remove finalizer for managed-only resources.

Reference: https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-disposeasync#implement-both-dispose-and-async-dispose-patterns

---

2. CRITICAL: Build() synchronously blocks on async initialization

In PipelineBuilder.cs:

public IPipeline Build()
{
    return BuildPipelineAsync().GetAwaiter().GetResult();
}

Problem: BuildPipelineAsync() calls host.Services.InitializeAsync(), which can deadlock in certain synchronization contexts (e.g., WPF, WinForms, legacy ASP.NET). While ModularPipelines is primarily CLI-focused, this creates a footgun for library consumers.

Recommendation: Either:

• Document prominently that Build() should only be used in console applications or async-safe contexts
• Rename to BuildUnsafe() to signal danger
• Always prefer BuildAsync() in documentation examples

The example in Program.cs uses Build() synchronously, which sets a bad precedent:

var pipeline = builder.Build();  // Synchronous
await pipeline.RunAsync();

Should be:

var pipeline = await builder.BuildAsync();  // Preferred
await pipeline.RunAsync();

Suggestions

1. Inconsistent naming: BuildAsync() vs BuildPipelineAsync()

BuildAsync() is the public API but internally calls BuildPipelineAsync(). Consider renaming the internal method to CreatePipelineInternalAsync() or similar to avoid confusion about which is the "real" build method.

2. Consider exposing PipelineBuilderOptions

The PipelineBuilderOptions class exists but isn't clearly documented in the builder API. If it's intended for public consumption, consider adding examples of when to use it vs direct property access.

Previous Review Status

No previous comments found.

Verdict

⚠️ REQUEST CHANGES - Critical issues found (finalizer blocking on async, sync Build() method)

[thomhurst]

Summary

This PR redesigns the pipeline configuration API from a callback-based pattern to direct property access, following ASP.NET Core WebApplicationBuilder pattern.

Critical Issues

None found

Observations

Positive Changes

1. Cleaner API with direct property access
2. Simplified Program.cs with reduced nesting
3. Better discoverability through IDE autocomplete
4. Proper separation of concerns

Minor Observations

1. Collection expression usage requires C# 12
2. args parameter now required - verify optional handling
3. Module dependency validation moved - verify still exists
4. All 892 tests pass - good coverage
5. Extension methods provide backward compatibility

Questions

1. Verify resource disposal handles service scope disposables
2. CLAUDE.md:62 references old PipelineHostBuilder

Verdict

APPROVE - Well-executed API modernization with clear breaking change documentation