From 9c48c15cd84a9b3881fbddd3d4d474b34906516a Mon Sep 17 00:00:00 2001 From: Copilot App <223556219+Copilot@users.noreply.github.com> Date: Sat, 22 Aug 2026 06:46:11 -0700 Subject: [PATCH] fix(setup): preserve trailing command output while bounding inherited-pipe waits CommandRunner previously always applied a short bounded grace to the stdout/stderr drain after the child exited, which could truncate trailing output on high-volume commands. Await full output-close on the normal path so complete output is preserved, and apply the bounded grace only on timeout cleanup or when a caller opts in via allowInheritedPipeHandleEscape for WSL inspection commands whose descendants can hold inherited pipe handles open. Opt the wsl.exe --list/--verbose inspection callers into the bounded escape (CleanupStaleDistroStep, CreateWslInstanceStep, ExistingConfigDetector, StartGatewayStep). Add regression tests: high-volume stdout/stderr drain with trailing markers on the default path, a bounded-timeout test, and update the pipe-holder test to use the opt-in. Fixes #1194 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../CleanupStaleDistroStep.cs | 7 +- src/OpenClaw.SetupEngine/CommandRunner.cs | 23 ++++--- .../CreateWslInstanceStep.cs | 24 +++++-- .../ExistingConfigDetector.cs | 6 +- src/OpenClaw.SetupEngine/PreflightWslStep.cs | 12 ++-- src/OpenClaw.SetupEngine/StartGatewayStep.cs | 14 +++- .../CommandRunnerTests.cs | 65 ++++++++++++++++++- .../LocalAiGatewayUninstallTests.cs | 3 +- .../SetupStepsTests.cs | 3 +- 9 files changed, 134 insertions(+), 23 deletions(-) diff --git a/src/OpenClaw.SetupEngine/CleanupStaleDistroStep.cs b/src/OpenClaw.SetupEngine/CleanupStaleDistroStep.cs index 9dd911e80..3584852e5 100644 --- a/src/OpenClaw.SetupEngine/CleanupStaleDistroStep.cs +++ b/src/OpenClaw.SetupEngine/CleanupStaleDistroStep.cs @@ -24,7 +24,12 @@ public override async Task ExecuteAsync(SetupContext ctx, Cancellati if (!DistroInstallPathPolicy.TryGetManagedInstallPath(ctx.LocalDataDir, distro, out var wslDir, out var pathError)) return StepResult.Terminal(pathError); - var list = await ctx.Commands.RunAsync(WslConstants.WslExePath, ["--list", "--quiet"], TimeSpan.FromSeconds(15), ct: ct); + var list = await ctx.Commands.RunAsync( + WslConstants.WslExePath, + ["--list", "--quiet"], + TimeSpan.FromSeconds(15), + ct: ct, + allowInheritedPipeHandleEscape: true); if (list.ExitCode != 0) return StepResult.Ok("WSL not available or no distros - nothing to clean"); diff --git a/src/OpenClaw.SetupEngine/CommandRunner.cs b/src/OpenClaw.SetupEngine/CommandRunner.cs index 1f1eaae16..b6d9fc037 100644 --- a/src/OpenClaw.SetupEngine/CommandRunner.cs +++ b/src/OpenClaw.SetupEngine/CommandRunner.cs @@ -17,7 +17,8 @@ Task RunAsync( string? workingDirectory = null, string? stdinInput = null, CancellationToken ct = default, - Stream? stdinStream = null); + Stream? stdinStream = null, + bool allowInheritedPipeHandleEscape = false); /// /// Run a command inside a WSL distro. @@ -86,7 +87,8 @@ public async Task RunAsync( string? workingDirectory = null, string? stdinInput = null, CancellationToken ct = default, - Stream? stdinStream = null) + Stream? stdinStream = null, + bool allowInheritedPipeHandleEscape = false) { ArgumentNullException.ThrowIfNull(executable); ArgumentNullException.ThrowIfNull(arguments); @@ -192,12 +194,17 @@ public async Task RunAsync( throw; } - // A surviving descendant can keep inherited pipe handles open after the child - // exits. Preserve output already in flight without charging the command's full - // timeout to an EOF that may never arrive. - await Task.WhenAny( - Task.WhenAll(stdoutClosed.Task, stderrClosed.Task), - Task.Delay(s_outputDrainGrace)); + var outputClosed = Task.WhenAll(stdoutClosed.Task, stderrClosed.Task); + if (timedOut || allowInheritedPipeHandleEscape) + { + // Some WSL inspection commands can leave a descendant holding inherited + // pipe handles. Bound only those known waits, plus timeout cleanup. + await Task.WhenAny(outputClosed, Task.Delay(s_outputDrainGrace)); + } + else + { + await outputClosed; + } sw.Stop(); var result = new CommandResult( diff --git a/src/OpenClaw.SetupEngine/CreateWslInstanceStep.cs b/src/OpenClaw.SetupEngine/CreateWslInstanceStep.cs index 717b1641f..65e8c923b 100644 --- a/src/OpenClaw.SetupEngine/CreateWslInstanceStep.cs +++ b/src/OpenClaw.SetupEngine/CreateWslInstanceStep.cs @@ -41,7 +41,12 @@ public override async Task ExecuteAsync(SetupContext ctx, Cancellati ctx.Logger.Info($"Creating clean app-owned WSL distro '{distro}' from '{baseDistro}' at '{installPath}'"); - var existing = await ctx.Commands.RunAsync(WslConstants.WslExePath, ["--list", "--quiet"], TimeSpan.FromSeconds(15), ct: ct); + var existing = await ctx.Commands.RunAsync( + WslConstants.WslExePath, + ["--list", "--quiet"], + TimeSpan.FromSeconds(15), + ct: ct, + allowInheritedPipeHandleEscape: true); if (existing.ExitCode != 0) return StepResult.Fail($"Failed to list WSL distros before creating '{distro}': {existing.Stderr}"); @@ -108,7 +113,12 @@ private static StepResult EnsureInstallPathReady(string installPath) private static async Task VerifyFreshDistro(SetupContext ctx, string distro, string installPath, CancellationToken ct) { - var list = await ctx.Commands.RunAsync(WslConstants.WslExePath, ["--list", "--quiet"], TimeSpan.FromSeconds(15), ct: ct); + var list = await ctx.Commands.RunAsync( + WslConstants.WslExePath, + ["--list", "--quiet"], + TimeSpan.FromSeconds(15), + ct: ct, + allowInheritedPipeHandleEscape: true); if (list.ExitCode != 0 || !WslInstallSupport.ContainsDistro(list.Stdout, distro)) { var environmentIssue = await PreflightWslStep.DetectEnvironmentIssueAsync(ctx, ct); @@ -120,7 +130,8 @@ private static async Task VerifyFreshDistro(SetupContext ctx, string WslConstants.WslExePath, ["--list", "--verbose"], DistroVersionVerificationTimeout, - ct: ct); + ct: ct, + allowInheritedPipeHandleEscape: true); if (verbose.ExitCode != 0 || !WslInstallSupport.TryGetDistroVersion(verbose.Stdout, distro, out var version)) return StepResult.Fail($"Fresh WSL install registered '{distro}', but setup could not verify it is WSL2."); @@ -164,7 +175,12 @@ private static async Task CleanupPartialInstall(SetupContext ctx, string { var cleanupErrors = new List(); var installPathExists = Directory.Exists(installPath) || File.Exists(installPath); - var list = await ctx.Commands.RunAsync(WslConstants.WslExePath, ["--list", "--quiet"], TimeSpan.FromSeconds(15), ct: ct); + var list = await ctx.Commands.RunAsync( + WslConstants.WslExePath, + ["--list", "--quiet"], + TimeSpan.FromSeconds(15), + ct: ct, + allowInheritedPipeHandleEscape: true); var registrationStateKnown = list.ExitCode == 0; var distroExists = registrationStateKnown && WslInstallSupport.ContainsDistro(list.Stdout, distro); var canDeleteInstallPath = registrationStateKnown && !distroExists; diff --git a/src/OpenClaw.SetupEngine/ExistingConfigDetector.cs b/src/OpenClaw.SetupEngine/ExistingConfigDetector.cs index b6daaea90..e3222f756 100644 --- a/src/OpenClaw.SetupEngine/ExistingConfigDetector.cs +++ b/src/OpenClaw.SetupEngine/ExistingConfigDetector.cs @@ -31,7 +31,11 @@ public static ExistingConfig Detect(string dataDir, string targetDistroName) var logger = new SetupLogger(filePath: null, LogLevel.Warn); var result = new CommandRunner(logger) - .RunAsync(WslConstants.WslExePath, ["--list", "--quiet"], TimeSpan.FromSeconds(5)) + .RunAsync( + WslConstants.WslExePath, + ["--list", "--quiet"], + TimeSpan.FromSeconds(5), + allowInheritedPipeHandleEscape: true) .GetAwaiter() .GetResult(); var hasDistro = InterpretDistroList(result, targetDistroName); diff --git a/src/OpenClaw.SetupEngine/PreflightWslStep.cs b/src/OpenClaw.SetupEngine/PreflightWslStep.cs index 83d777438..cdb24c0ed 100644 --- a/src/OpenClaw.SetupEngine/PreflightWslStep.cs +++ b/src/OpenClaw.SetupEngine/PreflightWslStep.cs @@ -48,7 +48,8 @@ public static async Task InspectAsync( WslConstants.WslExePath, ["--version"], TimeSpan.FromSeconds(5), - ct: ct); + ct: ct, + allowInheritedPipeHandleEscape: true); } catch (Exception ex) when (ex is not OperationCanceledException) { @@ -106,7 +107,8 @@ public static async Task InspectAsync( WslConstants.WslExePath, ["--status"], TimeSpan.FromSeconds(10), - ct: ct); + ct: ct, + allowInheritedPipeHandleEscape: true); } catch (Exception ex) when (ex is not OperationCanceledException) { @@ -186,7 +188,8 @@ public override async Task ExecuteAsync(SetupContext ctx, Cancellati WslConstants.WslExePath, ["--status"], TimeSpan.FromSeconds(10), - ct: ct); + ct: ct, + allowInheritedPipeHandleEscape: true); var combined = $"{status.Stdout}\n{status.Stderr}"; if (!WslInstallSupport.TryGetEnvironmentIssue(combined, out var message)) return null; @@ -230,7 +233,8 @@ internal static async Task InstallWslPlatformAsync(SetupContext ctx, WslConstants.WslExePath, ["--version"], TimeSpan.FromSeconds(5), - ct: ct); + ct: ct, + allowInheritedPipeHandleEscape: true); if (probe.ExitCode != 0 || WslViabilityInspector.LooksUnavailable(probe)) { return StepResult.Terminal( diff --git a/src/OpenClaw.SetupEngine/StartGatewayStep.cs b/src/OpenClaw.SetupEngine/StartGatewayStep.cs index c3bcad50b..31dbc2678 100644 --- a/src/OpenClaw.SetupEngine/StartGatewayStep.cs +++ b/src/OpenClaw.SetupEngine/StartGatewayStep.cs @@ -148,7 +148,12 @@ public override async Task RollbackAsync(SetupContext ctx, CancellationToken ct) var distro = ctx.DistroName!; // Check if distro is running before trying systemctl stop - var list = await ctx.Commands.RunAsync(WslConstants.WslExePath, ["--list", "--quiet"], TimeSpan.FromSeconds(15), ct: ct); + var list = await ctx.Commands.RunAsync( + WslConstants.WslExePath, + ["--list", "--quiet"], + TimeSpan.FromSeconds(15), + ct: ct, + allowInheritedPipeHandleEscape: true); if (!WslInstallSupport.ContainsDistro(list.Stdout, distro)) { ctx.Logger.Info("[Uninstall] Distro not registered — skipping gateway stop"); @@ -156,7 +161,12 @@ public override async Task RollbackAsync(SetupContext ctx, CancellationToken ct) } // Check distro state — only stop if Running - var verbose = await ctx.Commands.RunAsync(WslConstants.WslExePath, ["--list", "--verbose"], TimeSpan.FromSeconds(15), ct: ct); + var verbose = await ctx.Commands.RunAsync( + WslConstants.WslExePath, + ["--list", "--verbose"], + TimeSpan.FromSeconds(15), + ct: ct, + allowInheritedPipeHandleEscape: true); var isRunning = WslInstallSupport.Normalize(verbose.Stdout) .Split(['\r', '\n'], StringSplitOptions.RemoveEmptyEntries) .Any(line => line.Contains(distro, StringComparison.OrdinalIgnoreCase) diff --git a/tests/OpenClaw.SetupEngine.Tests/CommandRunnerTests.cs b/tests/OpenClaw.SetupEngine.Tests/CommandRunnerTests.cs index a2716a7d2..f8fa64e76 100644 --- a/tests/OpenClaw.SetupEngine.Tests/CommandRunnerTests.cs +++ b/tests/OpenClaw.SetupEngine.Tests/CommandRunnerTests.cs @@ -41,6 +41,23 @@ await Assert.ThrowsAnyAsync(() => runner.RunAsync( Assert.InRange(stopwatch.Elapsed, TimeSpan.Zero, TimeSpan.FromSeconds(5)); } + [Fact] + public async Task RunAsync_ProcessTimeoutRemainsBounded() + { + var runner = CreateRunner(); + var (executable, arguments) = SleepingCommand(); + var stopwatch = Stopwatch.StartNew(); + + var result = await runner.RunAsync( + executable, + arguments, + TimeSpan.FromMilliseconds(250)); + + Assert.True(result.TimedOut); + Assert.Equal(-1, result.ExitCode); + Assert.InRange(stopwatch.Elapsed, TimeSpan.Zero, TimeSpan.FromSeconds(5)); + } + [Fact] public async Task RunAsync_StreamStdinPreservesBinaryBytes() { @@ -87,13 +104,36 @@ public async Task RunAsync_ReturnsWhenDescendantKeepsOutputPipesOpen() var (executable, arguments) = ExitsLeavingPipeHolderCommand(); var stopwatch = Stopwatch.StartNew(); - var result = await runner.RunAsync(executable, arguments, TimeSpan.FromSeconds(30)); + var result = await runner.RunAsync( + executable, + arguments, + TimeSpan.FromSeconds(30), + allowInheritedPipeHandleEscape: true); Assert.False(result.TimedOut); Assert.Equal(0, result.ExitCode); Assert.InRange(stopwatch.Elapsed, TimeSpan.Zero, TimeSpan.FromSeconds(5)); } + [Fact] + public async Task RunAsync_DrainsHighVolumeStdoutAndStderrThroughTrailingMarkers() + { + const int lineCount = 8_000; + var (executable, arguments) = HighVolumeOutputCommand(lineCount); + + var result = await CreateRunner().RunAsync( + executable, + arguments, + TimeSpan.FromSeconds(30)); + + Assert.False(result.TimedOut); + Assert.Equal(0, result.ExitCode); + Assert.Equal(lineCount, CountLinesStartingWith(result.Stdout, "stdout-")); + Assert.Equal(lineCount, CountLinesStartingWith(result.Stderr, "stderr-")); + Assert.EndsWith($"STDOUT_MARKER{Environment.NewLine}", result.Stdout, StringComparison.Ordinal); + Assert.EndsWith($"STDERR_MARKER{Environment.NewLine}", result.Stderr, StringComparison.Ordinal); + } + private static CommandRunner CreateRunner() => new(new SetupLogger(filePath: null, LogLevel.Trace)); @@ -107,6 +147,29 @@ private static (string Executable, string[] Arguments) SleepingCommand() ? ("cmd.exe", ["/d", "/s", "/c", "ping 127.0.0.1 -n 30 >nul"]) : ("/bin/sh", ["-c", "sleep 30"]); + private static (string Executable, string[] Arguments) HighVolumeOutputCommand(int lineCount) + { + if (!OperatingSystem.IsWindows()) + { + var shellScript = + $"i=0; while [ $i -lt {lineCount} ]; do echo stdout-$i; echo stderr-$i >&2; i=$((i+1)); done; " + + "echo STDOUT_MARKER; echo STDERR_MARKER >&2"; + return ("/bin/sh", ["-c", shellScript]); + } + + var powerShellScript = + $"for ($i = 0; $i -lt {lineCount}; $i++) {{ " + + "[Console]::Out.WriteLine(\"stdout-$i\"); " + + "[Console]::Error.WriteLine(\"stderr-$i\") }; " + + "[Console]::Out.WriteLine('STDOUT_MARKER'); " + + "[Console]::Error.WriteLine('STDERR_MARKER')"; + return ("powershell.exe", ["-NoProfile", "-NonInteractive", "-Command", powerShellScript]); + } + + private static int CountLinesStartingWith(string value, string prefix) + => value.Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries) + .Count(line => line.StartsWith(prefix, StringComparison.Ordinal)); + private static (string Executable, string[] Arguments) CopyStdinCommand(string path) { if (!OperatingSystem.IsWindows()) diff --git a/tests/OpenClaw.SetupEngine.Tests/LocalAiGatewayUninstallTests.cs b/tests/OpenClaw.SetupEngine.Tests/LocalAiGatewayUninstallTests.cs index b1fdb81f7..c0e38674b 100644 --- a/tests/OpenClaw.SetupEngine.Tests/LocalAiGatewayUninstallTests.cs +++ b/tests/OpenClaw.SetupEngine.Tests/LocalAiGatewayUninstallTests.cs @@ -194,7 +194,8 @@ public Task RunAsync( string? workingDirectory = null, string? stdinInput = null, CancellationToken ct = default, - Stream? stdinStream = null) => throw new NotSupportedException(); + Stream? stdinStream = null, + bool allowInheritedPipeHandleEscape = false) => throw new NotSupportedException(); public Task RunInWslAsync( string distroName, diff --git a/tests/OpenClaw.SetupEngine.Tests/SetupStepsTests.cs b/tests/OpenClaw.SetupEngine.Tests/SetupStepsTests.cs index d0cee91ea..4a16b5c3a 100644 --- a/tests/OpenClaw.SetupEngine.Tests/SetupStepsTests.cs +++ b/tests/OpenClaw.SetupEngine.Tests/SetupStepsTests.cs @@ -4592,7 +4592,8 @@ public Task RunAsync( string? workingDirectory = null, string? stdinInput = null, CancellationToken ct = default, - Stream? stdinStream = null) + Stream? stdinStream = null, + bool allowInheritedPipeHandleEscape = false) { Calls.Add((executable, arguments)); TimedCalls.Add((executable, arguments, timeout));