Fix deadlock in PipeTarget.Merge when a sub-target reads past end-of-stream

[Copilot]

SimplexStream (used internally by PipeTarget.Merge) can only deliver its end-of-stream signal once. Per the Stream contract, every read at end-of-stream must return 0, but a second read after the first EOF read blocks forever on a semaphore that's never released again. StreamReader-based targets (ToStringBuilder, ToDelegate, and transitively ExecuteBufferedAsync) can trigger this legitimately when a multibyte UTF-8 sequence causes an extra probe read, hanging ExecuteAsync entirely.

Fix

• Added an _isCompleted latch to SimplexStream, set when the closing (empty) buffer is written.
• Once latched, ReadAsync short-circuits to return 0 immediately for any subsequent read, without waiting on the read semaphore.
• The EOF-consuming read no longer releases the write lock, since no further writes are expected once the stream is closed.

Tests

Added regression tests in PipingSpecs:

• Minimal contract-level repro: a PipeTarget.Merge sub-target that reads once past end-of-stream.
• Real-world repro: PipeTarget.ToStringBuilder merged with another target, fed exactly 1024 bytes ending in a multibyte UTF-8 character (matches StreamReader's internal buffer size, the exact trigger condition from the report).

var overReadingTarget = PipeTarget.Create(async (stream, cancellationToken) =>
{
    var buffer = new byte[1024];
    while (await stream.ReadAsync(buffer, cancellationToken) > 0) { }

    // Must return 0 again per the Stream contract — previously hung forever
    await stream.ReadAsync(buffer, cancellationToken);
});

var merged = PipeTarget.Merge(overReadingTarget, PipeTarget.ToStream(Stream.Null));
await merged.CopyFromAsync(origin);

• Fixes PipeTarget.Merge deadlocks when a sub-target reads past end-of-stream (SimplexStream delivers EOF only once) #346

[Tyrrrz]

@copilot added some comments.

Also, was this a regression or a bug that stayed dormant for a while?

[Copilot]

@copilot added some comments.

Also, was this a regression or a bug that stayed dormant for a while?

It's a dormant bug, not a regression. The one-shot EOF semaphore design has been there since SimplexStream (originally HalfDuplexStream) was introduced in commit 8b8db05 back in 2020 (#75) — every write/read cycle only releases _readLock once per write, including the closing empty-buffer write. It stayed hidden because it only surfaces when a sub-target legally reads once past end-of-stream, which StreamReader only does under the specific multibyte/buffer-size-multiple condition described in the issue.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.80%. Comparing base (357f6f7) to head (22e2840).

Additional details and impacted files

@@            Coverage Diff             @@
##            prime     #347      +/-   ##
==========================================
+ Coverage   92.77%   92.80%   +0.03%     
==========================================
  Files          26       26              
  Lines        1162     1167       +5     
  Branches       73       76       +3     
==========================================
+ Hits         1078     1083       +5     
  Misses         64       64              
  Partials       20       20              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR fixes a deadlock in PipeTarget.Merge caused by SimplexStream only signaling end-of-stream once, which can legitimately trigger a second EOF read (e.g., via StreamReader) and hang ExecuteAsync.

Changes:

• Added an end-of-stream completion latch in SimplexStream so reads past EOF reliably return 0 without blocking.
• Adjusted EOF handling so the write lock is not released after the closing (empty) buffer is consumed.
• Added regression tests covering both a minimal over-read scenario and a StreamReader/UTF-8 boundary case.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
CliWrap/Utils/SimplexStream.cs	Latches completion and adjusts semaphore/lock behavior to avoid post-EOF blocking.
CliWrap.Tests/PipingSpecs.cs	Adds regression tests that reproduce the hang and validate the fix under realistic conditions.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.