refactor: Reduce ExecutionOrchestrator constructor dependencies (15 → 8)

[thomhurst]

Summary

• Extract 3 single-responsibility components from ExecutionOrchestrator (15→8 dependencies):
  • IPipelineOutputCoordinator: Coordinates all output operations (printing, logging, exception flushing) - wraps 5 dependencies
  • IIgnoredModuleResultRegistrar: Handles registration of skipped results for ignored modules - wraps 4 dependencies
  • IPipelineSummaryFactory: Factory for creating PipelineSummary instances - wraps 3 dependencies

Benefits

• Improved testability: Each component can be unit tested in isolation
• Better maintainability: Smaller, focused classes are easier to understand
• Enhanced extensibility: Components can be replaced or extended via DI
• Clearer intent: Each dependency group has a meaningful name

Dependency Reduction

Before (15 deps)	After (8 deps)
IPipelineInitializer	IPipelineInitializer
IModuleDisposeExecutor	IModuleDisposeExecutor
IPrintModuleOutputExecutor	IPipelineExecutor
IPrintProgressExecutor	IPipelineOutputCoordinator (new)
IPipelineExecutor	IIgnoredModuleResultRegistrar (new)
IConsolePrinter	IPipelineSummaryFactory (new)
IAfterPipelineLogger	EngineCancellationToken
EngineCancellationToken	ILogger
ILogger
IExceptionBuffer
IModuleResultRegistry
IModuleResultRepository
IPipelineContextProvider
IMetricsCollector
IParallelLimitProvider

Test plan

• Build succeeds with no errors
• All unit tests pass (389 passed, 0 failures, 4 skipped)
• Code follows existing patterns from refactor: Apply SRP to ModuleExecutor and ModuleScheduler #1447

Closes #1425

🤖 Generated with Claude Code

[github-actions]

Summary

Extracts 3 single-responsibility components from ExecutionOrchestrator to reduce constructor dependencies from 15 to 8.

Critical Issues

None found ✅

Suggestions

1. Potential resource leak in PipelineOutputScope (PipelineOutputCoordinator.cs:71-75)

The nested PipelineOutputScope class disposes _printModuleOutputExecutor and _printProgressExecutor, but these instances are also stored as fields in the parent PipelineOutputCoordinator class. If InitializeAsync() is called multiple times, you could have multiple scopes trying to dispose the same shared instances.

Current code:

private sealed class PipelineOutputScope : IPipelineOutputScope
{
    private readonly IPrintModuleOutputExecutor _printModuleOutputExecutor;
    private readonly IPrintProgressExecutor _printProgressExecutor;

    public PipelineOutputScope(
        IPrintModuleOutputExecutor printModuleOutputExecutor,
        IPrintProgressExecutor printProgressExecutor)
    {
        _printModuleOutputExecutor = printModuleOutputExecutor;
        _printProgressExecutor = printProgressExecutor;
    }

    public async ValueTask DisposeAsync()
    {
        _printModuleOutputExecutor.Dispose();
        await _printProgressExecutor.DisposeAsync().ConfigureAwait(false);
    }
}

Concern: Looking at ExecutionOrchestrator.cs:171, the scope is properly used with await using, but the coordinator fields remain. Consider whether:

• InitializeAsync() should enforce single-call semantics (throw if called twice)
• Or the scope should dispose of new instances rather than shared coordinator fields

2. Type check using GetType() != typeof() (IgnoredModuleResultRegistrar.cs:47)

if (_resultRepository.GetType() != typeof(NoOpModuleResultRepository))

This is fragile - it breaks if someone creates a derived class. Consider:

if (_resultRepository is not NoOpModuleResultRepository)

This pattern would also work better with DI if you wanted to register a "null object" as a specific marker type.

Verdict

✅ APPROVE - Well-structured refactoring following established patterns from #1447. The dependency reduction is meaningful and the new abstractions have clear responsibilities.