fix: address code review issues in immediate output feature

- Fix pause functionality in ProgressSession: progress loop now properly
  checks _isPaused flag and waits on _resumeSignal when paused
- Remove dead code: MarkCompleted() and CompletedAtUtc from IModuleOutputBuffer
  and ModuleOutputBuffer (no longer needed with immediate flush)
- Improve error handling in ModuleLogger.Dispose: add 30s timeout to prevent
  potential deadlocks, add specific exception handling for AggregateException
  and OperationCanceledException

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Author: thomhurst

src/ModularPipelines/Console/IModuleOutputBuffer.cs

@@ -15,8 +15,8 @@ namespace ModularPipelines.Console;
 /// <b>Thread Safety:</b> All methods are thread-safe and can be called concurrently.
 /// </para>
 /// <para>
-/// <b>Flush Ordering:</b> Buffers are flushed in completion order (by CompletedAtUtc)
-/// to maintain logical output sequence.
+/// <b>Flush Behavior:</b> Buffers are flushed immediately when modules complete,
+/// via the OutputCoordinator which ensures ordered output.
 /// </para>
 /// </remarks>
 internal interface IModuleOutputBuffer
@@ -26,18 +26,6 @@ internal interface IModuleOutputBuffer
     /// </summary>
     Type ModuleType { get; }
 
-    /// <summary>
-    /// Gets when the module completed (for ordering during flush).
-    /// Null if not yet completed.
-    /// </summary>
-    DateTime? CompletedAtUtc { get; }
-
-    /// <summary>
-    /// Records completion time for flush ordering.
-    /// Called when the module finishes execution.
-    /// </summary>
-    void MarkCompleted();
-
     /// <summary>
     /// Adds a plain string line to the buffer.
     /// Used for Console.WriteLine interceptions.

src/ModularPipelines/Console/ModuleOutputBuffer.cs

@@ -31,9 +31,6 @@ internal class ModuleOutputBuffer : IModuleOutputBuffer
     /// <inheritdoc />
     public Type ModuleType { get; }
 
-    /// <inheritdoc />
-    public DateTime? CompletedAtUtc { get; private set; }
-
     /// <summary>
     /// Initializes a new buffer for the specified module type.
     /// </summary>
@@ -80,15 +77,6 @@ public void AddLogEvent(
         }
     }
 
-    /// <inheritdoc />
-    public void MarkCompleted()
-    {
-        lock (_lock)
-        {
-            CompletedAtUtc ??= DateTime.UtcNow;
-        }
-    }
-
     /// <inheritdoc />
     public void SetException(Exception exception)
     {
@@ -165,7 +153,7 @@ public void FlushTo(TextWriter console, IBuildSystemFormatter formatter, ILogger
 
     private string FormatHeader(Exception? exception)
     {
-        var duration = (CompletedAtUtc ?? DateTime.UtcNow) - _startTimeUtc;
+        var duration = DateTime.UtcNow - _startTimeUtc;
         var durationStr = duration.TotalSeconds >= 60
             ? $"{duration.TotalMinutes:F1}m"
             : $"{duration.TotalSeconds:F1}s";

src/ModularPipelines/Console/ProgressSession.cs

@@ -98,6 +98,27 @@ await AnsiConsole.Progress()
                     // Keep alive until all modules complete or cancellation
                     while (!ctx.IsFinished && !_cancellationToken.IsCancellationRequested)
                     {
+                        // Check if we should pause for module output
+                        TaskCompletionSource? resumeSignal = null;
+                        await _pauseLock.WaitAsync().ConfigureAwait(false);
+                        try
+                        {
+                            if (_isPaused)
+                            {
+                                resumeSignal = _resumeSignal;
+                            }
+                        }
+                        finally
+                        {
+                            _pauseLock.Release();
+                        }
+
+                        if (resumeSignal != null)
+                        {
+                            // Wait for resume signal
+                            await resumeSignal.Task.ConfigureAwait(false);
+                        }
+
                         await Task.Delay(100, CancellationToken.None).ConfigureAwait(false);
                     }
                 });

src/ModularPipelines/Logging/ModuleLogger.cs

@@ -144,15 +144,27 @@ public override void Dispose()
             }
 
             // Flush output immediately instead of just marking completed
-            // This blocks until the output is written
+            // This blocks until the output is written, with a timeout to prevent deadlocks
             try
             {
-                _outputCoordinator.EnqueueAndFlushAsync(_buffer).GetAwaiter().GetResult();
+                var flushTask = _outputCoordinator.EnqueueAndFlushAsync(_buffer);
+
+                // Wait with timeout to prevent potential deadlocks
+                // 30 seconds should be more than enough for any reasonable output
+                if (!flushTask.Wait(TimeSpan.FromSeconds(30)))
+                {
+                    // Timeout - output may be lost, but we can't block forever
+                    // This is a last resort safety measure
+                }
             }
-            catch
+            catch (AggregateException)
             {
                 // Best effort - don't fail disposal
             }
+            catch (OperationCanceledException)
+            {
+                // Cancellation during shutdown is expected
+            }
 
             GC.SuppressFinalize(this);
         }