fix(shell): stop shell_execute hang on background children

benchmarks/Netclaw.Benchmarks/CaptureBenchmarks.cs

@@ -29,7 +29,7 @@ public class CaptureBenchmarks
     public async Task<int> Capture_then_inline_window()
     {
         var reader = new SyntheticCharReader(TotalChars);
-        var (captured, _) = await BoundedOutputReader.DrainToWindowAsync(reader, CaptureMax, CancellationToken.None);
+        var (captured, _, _) = await BoundedOutputReader.DrainToWindowAsync(reader, CaptureMax, CancellationToken.None);
         var inline = BoundedOutputReader.Window(captured, InlineBudget);
         return inline.Length;
     }

benchmarks/Netclaw.Benchmarks/ShellDrainBenchmarks.cs

@@ -53,7 +53,7 @@ public async Task<int> ReadToEnd_ThenTruncate()
     public async Task<int> BoundedDrain()
     {
         var reader = new SyntheticCharReader(TotalChars);
-        var (text, _) = await BoundedOutputReader.DrainToWindowAsync(reader, Cap, CancellationToken.None);
+        var (text, _, _) = await BoundedOutputReader.DrainToWindowAsync(reader, Cap, CancellationToken.None);
         return text.Length;
     }
 }

src/Netclaw.Actors.Tests/Tools/BoundedOutputReaderTests.cs

@@ -16,26 +16,29 @@ public class BoundedOutputReaderTests
     public async Task DrainToWindow_short_output_returned_verbatim()
     {
         var input = "hello world";
-        var (text, truncated) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(input), 100, CancellationToken.None);
+        var (text, truncated, cancelled) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(input), 100, CancellationToken.None);
         Assert.Equal(input, text);
         Assert.False(truncated);
+        Assert.False(cancelled);
     }
 
     [Fact]
     public async Task DrainToWindow_empty_input_returns_empty()
     {
-        var (text, truncated) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(""), 100, CancellationToken.None);
+        var (text, truncated, cancelled) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(""), 100, CancellationToken.None);
         Assert.Equal("", text);
         Assert.False(truncated);
+        Assert.False(cancelled);
     }
 
     [Fact]
     public async Task DrainToWindow_output_exactly_at_cap_not_truncated()
     {
         var input = new string('a', 100);
-        var (text, truncated) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(input), 100, CancellationToken.None);
+        var (text, truncated, cancelled) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(input), 100, CancellationToken.None);
         Assert.Equal(input, text);
         Assert.False(truncated);
+        Assert.False(cancelled);
     }
 
     [Fact]
@@ -47,9 +50,10 @@ public async Task DrainToWindow_long_output_truncated_with_head_and_tail()
         var tail = new string('T', 100);
         var input = head + middle + tail;
 
-        var (text, truncated) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(input), 200, CancellationToken.None);
+        var (text, truncated, cancelled) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(input), 200, CancellationToken.None);
 
         Assert.True(truncated);
+        Assert.False(cancelled); // budget cut, not a cancelled/grace cut
         Assert.StartsWith(new string('H', 100), text);  // head preserved
         Assert.EndsWith(new string('T', 100), text);    // tail preserved
         Assert.Contains("...", text);                    // separator present
@@ -61,9 +65,10 @@ public async Task DrainToWindow_head_and_tail_split_evenly()
     {
         // budget=10 → headCap=5, tailCap=5
         var input = "AAAAAXXXXXXBBBBB"; // 16 chars: 5 head, 6 overflow discard, 5 tail
-        var (text, truncated) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(input), 10, CancellationToken.None);
+        var (text, truncated, cancelled) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(input), 10, CancellationToken.None);
 
         Assert.True(truncated);
+        Assert.False(cancelled);
         Assert.StartsWith("AAAAA", text);
         Assert.EndsWith("BBBBB", text);
     }
@@ -72,9 +77,10 @@ public async Task DrainToWindow_head_and_tail_split_evenly()
     public async Task DrainToWindow_disabled_cap_returns_full_output()
     {
         var input = new string('x', 10_000);
-        var (text, truncated) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(input), 0, CancellationToken.None);
+        var (text, truncated, cancelled) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(input), 0, CancellationToken.None);
         Assert.Equal(input, text);
         Assert.False(truncated);
+        Assert.False(cancelled);
     }
 
     [Fact]
@@ -86,12 +92,29 @@ public async Task DrainToWindow_tail_ring_wraps_across_small_chunks()
         // budget=10 → headCap=5 ("ABCDE"), tailCap=5; last 5 of "FGHIJKLMNO" = "KLMNO".
         var reader = new ChunkedReader("ABCDEFGHIJKLMNO", chunkSize: 3);
 
-        var (text, truncated) = await BoundedOutputReader.DrainToWindowAsync(reader, 10, CancellationToken.None);
+        var (text, truncated, cancelled) = await BoundedOutputReader.DrainToWindowAsync(reader, 10, CancellationToken.None);
 
         Assert.True(truncated);
+        Assert.False(cancelled);
         Assert.Equal($"ABCDE{Environment.NewLine}...{Environment.NewLine}KLMNO", text);
     }
 
+    [Fact]
+    public async Task DrainToWindow_cancelled_before_eof_reports_cancelled_true()
+    {
+        // A source that never reaches EOF, like a pipe that a background child
+        // still holds open. The token must end the read before the source does.
+        // The return value must show this cut, not report a clean read, so a
+        // caller can flag a capture that might be partial.
+        using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(50));
+
+        var (text, truncated, cancelled) = await BoundedOutputReader.DrainToWindowAsync(
+            new NeverEndingReader(), 100, cts.Token);
+
+        Assert.True(cancelled);
+        Assert.Equal("", text);
+    }
+
     // ── Window (pure string head+tail) ──
 
     [Fact]
@@ -160,7 +183,7 @@ public async Task Accumulator_matches_drain_output()
         var input = new string('H', 100) + new string('M', 5000) + new string('T', 100);
         const int budget = 200;
 
-        var (drainText, drainTruncated) = await BoundedOutputReader.DrainToWindowAsync(
+        var (drainText, drainTruncated, _) = await BoundedOutputReader.DrainToWindowAsync(
             new StringReader(input), budget, CancellationToken.None);
 
         var acc = new BoundedOutputAccumulator(budget);
@@ -190,4 +213,16 @@ public override ValueTask<int> ReadAsync(Memory<char> buffer, CancellationToken
             return ValueTask.FromResult(n);
         }
     }
+
+    // Stands in for a pipe that a background child still holds open: it never
+    // returns and never reaches EOF. Only the caller's token can end a read.
+    private sealed class NeverEndingReader : TextReader
+    {
+        [SlopwatchSuppress("SW004", "Infinite delay is the test fixture, not a timing guess: it stands in for a pipe that never reaches EOF, and only the caller's token can end the read.")]
+        public override async ValueTask<int> ReadAsync(Memory<char> buffer, CancellationToken cancellationToken = default)
+        {
+            await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken);
+            return 0; // unreachable: Task.Delay throws once cancellationToken fires
+        }
+    }
 }

src/Netclaw.Actors.Tests/Tools/ShellToolStreamingTests.cs

@@ -17,6 +17,8 @@ public class ShellToolStreamingTests
     private static readonly ShellExecutionEnvironment ShellEnvironment = TestShellEnvironment.Current;
     private readonly ShellTool _tool = CreateTool();
 
+    public static bool IsPosix => !OperatingSystem.IsWindows();
+
     private static ShellTool CreateTool(ToolConfig? config = null)
     {
         var commandPolicy = new ShellCommandPolicy(ShellEnvironment);
@@ -82,6 +84,9 @@ public async Task Echo_emits_activity_with_output_chunk_then_completion()
         Assert.NotNull(completion);
         Assert.Contains("Exit code: 0", completion.Result);
         Assert.Contains("hello", completion.Result);
+        // A normal command reaches EOF cleanly. The result must not carry a
+        // grace-cut marker that tells the agent the capture is incomplete.
+        Assert.DoesNotContain("background process", completion.Result);
 
         // At least one activity item should carry the output
         Assert.NotEmpty(activities);
@@ -138,6 +143,37 @@ public async Task Cancellation_kills_process_and_returns_timeout()
         Assert.Contains("timed out after", completion.Result);
     }
 
+    [SlopwatchSuppress("SW001", "Reproduces a backgrounded child holding the pipe open; the case needs POSIX `&` semantics.")]
+    [Fact(SkipUnless = nameof(IsPosix), Skip = "Requires POSIX background-job (`&`) semantics.")]
+    public async Task Direct_process_exit_with_backgrounded_child_holding_pipe_open_streams_promptly()
+    {
+        // Same reproduction as the non-streaming test: the direct bash
+        // process exits at once, but the backgrounded sleep inherits
+        // stdout/stderr and holds the pipe write end open for its own life
+        // span. The streaming path must complete once bash exits.
+        var args = ToolInput.Create("Command", "sleep 20 & exit 0");
+        var context = TestToolExecutionContext.CreateBound("test/thread", Path.GetTempPath(), new TestToolExecutionContextOptions
+        {
+            Audience = TrustAudience.Personal,
+            ExecutionTimeout = new ToolExecutionTimeout(TimeSpan.FromSeconds(90))
+        });
+
+        var stopwatch = System.Diagnostics.Stopwatch.StartNew();
+        var (_, completion) = await CollectStreamAsync(_tool, args, context, ct: TestContext.Current.CancellationToken);
+        stopwatch.Stop();
+
+        Assert.NotNull(completion);
+        Assert.Contains("Exit code: 0", completion.Result);
+        Assert.True(
+            stopwatch.Elapsed < TimeSpan.FromSeconds(5),
+            $"The tool must return soon after the direct process exits. It took {stopwatch.Elapsed}.");
+
+        // The grace window cut the drain before EOF. The backgrounded sleep
+        // process still holds the pipe open. The result must show this cut,
+        // not a capture that looks complete.
+        Assert.Contains("background process", completion.Result);
+    }
+
     [Fact]
     public async Task Output_clamping_preserved_in_completion_result()
     {

src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs

@@ -18,9 +18,17 @@ public class ShellToolTests
     private readonly ShellTool _tool = CreateTool();
 
     public static bool IsWindows => OperatingSystem.IsWindows();
-
     public static bool IsPosix => !OperatingSystem.IsWindows();
 
+    [Fact]
+    public void Constructor_preserves_three_parameter_binary_signature()
+    {
+        var constructor = typeof(ShellTool).GetConstructor(
+            [typeof(ToolConfig), typeof(ToolPathPolicy), typeof(ShellCommandPolicy)]);
+
+        Assert.NotNull(constructor);
+    }
+
     private static ShellTool CreateTool(ToolConfig? config = null)
     {
         var commandPolicy = new ShellCommandPolicy(ShellEnvironment);
@@ -98,6 +106,9 @@ public async Task Execute_echo_returns_output()
 
         Assert.Contains("hello", result);
         Assert.Contains("Exit code: 0", result);
+        // A normal command reaches EOF cleanly. The result must not carry a
+        // grace-cut marker that tells the agent the capture is incomplete.
+        Assert.DoesNotContain("background process", result);
     }
 
     [SlopwatchSuppress("SW001", "This native fallback test requires Windows PowerShell 5.1.")]
@@ -154,6 +165,38 @@ public async Task Timeout_kills_long_running_process()
         Assert.Contains("timed out", result);
     }
 
+    [SlopwatchSuppress("SW001", "Reproduces a backgrounded child holding the pipe open; the case needs POSIX `&` semantics.")]
+    [Fact(SkipUnless = nameof(IsPosix), Skip = "Requires POSIX background-job (`&`) semantics.")]
+    public async Task Direct_process_exit_with_backgrounded_child_holding_pipe_open_returns_promptly()
+    {
+        // The direct bash process exits at once. The backgrounded sleep
+        // inherits stdout/stderr and holds the pipe write end open for its
+        // own life span — the same shape as a self-daemonizing process, for
+        // example nginx. The tool must return once bash exits. It must not
+        // wait for the still-running child.
+        var tool = CreateTool();
+        var args = ToolInput.Create("Command", "sleep 20 & exit 0");
+        var context = TestToolExecutionContext.CreateBound("test/thread", Path.GetTempPath(), new TestToolExecutionContextOptions
+        {
+            Audience = TrustAudience.Personal,
+            ExecutionTimeout = new ToolExecutionTimeout(TimeSpan.FromSeconds(90))
+        });
+
+        var stopwatch = System.Diagnostics.Stopwatch.StartNew();
+        var result = await tool.ExecuteAsync(args, context, TestContext.Current.CancellationToken);
+        stopwatch.Stop();
+
+        Assert.Contains("Exit code: 0", result);
+        Assert.True(
+            stopwatch.Elapsed < TimeSpan.FromSeconds(5),
+            $"The tool must return soon after the direct process exits. It took {stopwatch.Elapsed}.");
+
+        // The grace window cut the drain before EOF. The backgrounded sleep
+        // process still holds the pipe open. The result must show this cut,
+        // not a capture that looks complete.
+        Assert.Contains("background process", result);
+    }
+
     [Fact]
     public async Task Caller_cancellation_kills_child_process_tree_and_returns_gracefully()
     {

src/Netclaw.Actors/Tools/BoundedOutputReader.cs

@@ -30,19 +30,43 @@ internal static class BoundedOutputReader
     /// <paramref name="budget"/> chars. Chars beyond the budget are discarded but
     /// the source continues to be read so a still-running child never deadlocks on
     /// a full pipe buffer. A non-positive <paramref name="budget"/> disables the
-    /// cap (reads the whole stream). Returns the captured text and whether it was
-    /// truncated.
+    /// cap (reads the whole stream). Returns the captured text, whether the budget
+    /// truncated it, and whether <paramref name="ct"/> cancelled the read before
+    /// the source reached EOF.
     /// </summary>
-    public static async Task<(string Text, bool Truncated)> DrainToWindowAsync(
+    /// <remarks>
+    /// A cancelled <paramref name="ct"/> stops the read and returns whatever was
+    /// captured so far, instead of throwing. Callers pass a real, boundable token
+    /// (not <see cref="CancellationToken.None"/>): a process pipe reaches EOF only
+    /// when every process holding its write end closes it, and a forked or
+    /// backgrounded grandchild (a daemon, a `cmd &amp;` job) can hold that write
+    /// end open long after the direct child process has exited. Without a way to
+    /// stop the read, the drain would hang for the grandchild's full life span.
+    /// When <paramref name="ct"/> cancels the read, unread data may still wait
+    /// behind it. A caller that cares about a silent partial capture must check
+    /// the returned <c>Cancelled</c> flag. The <c>Truncated</c> flag alone is not
+    /// enough: it reports only the budget cut.
+    /// </remarks>
+    public static async Task<(string Text, bool Truncated, bool Cancelled)> DrainToWindowAsync(
         TextReader reader, int budget, CancellationToken ct)
     {
         if (budget <= 0)
         {
-            var all = await reader.ReadToEndAsync(ct);
-            return (all, false);
+            try
+            {
+                var all = await reader.ReadToEndAsync(ct);
+                return (all, false, false);
+            }
+            catch (OperationCanceledException) when (ct.IsCancellationRequested)
+            {
+                // ReadToEndAsync has no partial-read API, so a cancellation here
+                // returns nothing captured rather than hanging past the bound.
+                return (string.Empty, true, true);
+            }
         }
 
         var acc = new BoundedOutputAccumulator(budget);
+        var cancelled = false;
 
         var buf = ArrayPool<char>.Shared.Rent(4096);
         try
@@ -51,12 +75,19 @@ internal static class BoundedOutputReader
             while ((read = await reader.ReadAsync(buf.AsMemory(), ct)) > 0)
                 acc.Append(buf.AsSpan(0, read));
         }
+        catch (OperationCanceledException) when (ct.IsCancellationRequested)
+        {
+            // The source never reached EOF within the caller's bound. Return
+            // whatever was captured instead of discarding it or hanging.
+            cancelled = true;
+        }
         finally
         {
             ArrayPool<char>.Shared.Return(buf, clearArray: true);
         }
 
-        return acc.Finish();
+        var (text, truncated) = acc.Finish();
+        return (text, truncated, cancelled);
     }
 
     /// <summary>