feat(shell): stream shell_execute output to fix hard 90s timeout#1360
Merged
Aaronontheweb merged 6 commits intoJun 8, 2026
Merged
Conversation
netclaw-dev#1356, netclaw-dev#1046) ShellTool was non-streaming — the StreamingToolWatchdog received no activity items, so its inactivity budget acted as a hard wall-clock cap (90s default) rather than a silence detector. Override ExecuteStreamAsync to emit stdout/stderr chunks as ToolActivityUpdate items via a channel-based pattern. Each chunk resets the watchdog timer, so the 90s budget now measures silence between output, not total execution time. Chatty commands run indefinitely; silent commands use the existing _timeout_seconds mechanism (up to 600s). Extract BoundedOutputAccumulator from BoundedOutputReader so the streaming path can feed pipe chunks to both the activity channel and the head+tail window simultaneously without duplicating ring buffer logic.
…eout message - Guard WaitForExitAsync with process.HasExited so a token that fires between pipe-close and exit-status assembles valid output instead of discarding it as a spurious timeout. - Add outer catch-all in ExecuteStreamCoreAsync so unexpected exceptions surface as a tool-result error instead of silently faulting the fire-and-forget task. - Compute effectiveTimeoutSeconds in the streaming path and include the duration in the timeout message, matching the non-streaming path.
Slopwatch SW003 flagged the empty catch (InvalidOperationException) block in KillAndDrainAsync. Add a Debug.WriteLine matching the non-streaming path's pattern for the already-exited-before-kill case.
ShellTool uses cmd.exe on Windows, not bash. Replace bash-only syntax (for/do/done, >&2, sleep, seq) with platform-aware commands so tests pass on both the Linux and Windows CI runners.
effectiveTimeoutSeconds was computed but only used in the error message. Add a CancellationTokenSource.CancelAfter linked with the caller's ct so _timeout_seconds acts as an absolute wall-clock cap while the watchdog's inactivity budget handles silence detection independently.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
shell_executeasToolActivityUpdateitems so each output chunk resets theStreamingToolWatchdoginactivity timer. The 90s budget now measures silence between output, not total execution time. Fixes Tool execution framework hard 90s timeout ignores _timeout_seconds parameter #1356, closes Phase F of Opt-in streaming for shell_execute and web_fetch tools (streaming tool-calls Phase F) #1046.BoundedOutputAccumulatorfromBoundedOutputReaderso the streaming path can feed pipe chunks to both the activity channel and the head+tail capture simultaneously.Design
ExecuteStreamAsync→ thin channel-relay iterator (works around C# yield-in-catch restriction)ExecuteStreamCoreAsync→ non-iterator core: validates, starts process, drains pipes via two background tasks, relaysToolActivityUpdateitems, assembles finalToolCompletedUpdateDrainPipeToChannelAsync→ reads pipe withCancellationToken.None(pipe-deadlock safety), feeds chunks to accumulator AND coalesces into activity items at 500ms intervalsExecuteAsyncis unchanged — still used by tests and the base-class streaming adapterWhat changed
ShellTool.csExecuteStreamAsync,ExecuteStreamCoreAsync,DrainPipeToChannelAsync,KillAndDrainAsyncBoundedOutputReader.csBoundedOutputAccumulator; refactoredDrainToWindowAsyncto use itShellToolStreamingTests.csBoundedOutputReaderTests.csTest plan
dotnet slopwatch analyze— 0 new violations./scripts/Add-FileHeaders.ps1 -Verify— all headers presentecho hello) — verify activity items + completionCloses #1356
Relates to #1046