fix: keepalive process inspection can hang after PowerShell exits - #1168
Open
SebTardif wants to merge 1 commit into
Open
fix: keepalive process inspection can hang after PowerShell exits#1168SebTardif wants to merge 1 commit into
SebTardif wants to merge 1 commit into
Conversation
WaitForExit returns when PowerShell exits, but ReadToEnd only finishes after the write end of stdout closes. A descendant that inherited the pipe left GetResult hanging past the 5s timeout. Drain the leftover budget (same pattern as the MXC probe) and reuse the helper from setup and WSL keepalive copies. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 availability
|
Codex review: needs real behavior proof before merge. Reviewed August 16, 2026, 7:14 PM ET / 23:14 UTC. ClawSweeper reviewWhat this changesThis PR centralizes Windows process command-line inspection and bounds redirected-output draining so setup and WSL keepalive return an indeterminate result rather than hanging when a descendant retains stdout. Regression provenancePossible regression — suspected (reviewed change). No predecessor PR is attributed. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 4 items remain Keep open. Current main, including released v2026.7.1, still performs an unbounded redirected-output read after PowerShell exits, so the reported hang remains. The patch is a focused repair, but it needs real Windows proof and the repository-required validation before merge. Likely related people: Scott Hanselman (high), who introduced the shared inspection helper; Barbara Kudiess (medium), from adjacent managed-local gateway recovery work. Priority: P2 Review scores
Verification
How this fits togetherGateway setup rollback and the tray WSL keepalive service inspect local process command lines to decide whether a process belongs to the managed gateway. The inspection result feeds cleanup and recovery decisions, so a stuck PowerShell output pipe can stall those user-visible flows. flowchart LR
A[Setup rollback and tray keepalive] --> B[Process command-line inspection]
B --> C[PowerShell CIM query]
C --> D{Child exit and output drain}
D -->|Completed| E[Command-line result]
D -->|Pipe stays open| F[Indeterminate result]
E --> G[Keepalive cleanup or recovery]
F --> G
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the shared bounded-drain helper, then provide a redacted Windows runtime transcript for the retained-pipe case and complete the mandatory build and full Shared and Tray test subset. Do we have a high-confidence way to reproduce the issue? No. Current main clearly has the unbounded post-exit read, and the submitted test models it, but no current-head real Windows PowerShell/CIM retained-pipe reproduction is supplied. Is this the best way to solve the issue? Yes for the code direction. Consolidating the duplicate readers under the existing Connection-layer owner and using a bounded drain matches the established MXC pattern; merge confidence still requires Windows runtime proof. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against fc9add75eda7. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
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.
What Problem This Solves
Fixes an issue where users starting or repairing a local Windows gateway could see the tray or setup wizard hang after PowerShell finished listing a process command line. The hang shows up when keepalive cleanup or port ownership inspects a
wsl.exe/ gateway PID and a child still holds the redirected stdout pipe.Why This Change Was Made
The inspection already waited 5s for PowerShell to exit, then called unbounded
GetResult()on stdout.WaitForExitreturns when the child exits, butReadToEndonly finishes after the write end of the pipe closes. A descendant that inherited stdout could keep the pipe open and stall keepalive or setup past the timeout. The leftover time budget is now used to drain (or abandon) the read, matching the existing MXC probe helper. Setup and WSL keepalive now call the same helper instead of keeping their own copies.User Impact
Local gateway keepalive and setup process inspection return
null(indeterminate command line) instead of hanging when stdout never closes. Successful inspections are unchanged.Evidence
Terminal output from the patched helper. A child that already exited plus a read that never completes now returns null inside the leftover drain budget instead of blocking:
Related prior work: MXC probe already documents this pipe-inheritance hang in
MxcAvailability.cs. Open PR #1166 bounds MXC executor drain only and does not touchGetProcessCommandLine. Introduced in #1053 (bc210e04).Change Type
Scope
winnodeValidation
Red: compiling the hang detector against unbounded
GetResult()leavesAwaitRedirectedOutputblocked after child exit. Green: leftover drain returns null in ~400ms.Real behavior proof
Behavior or issue addressed: Local gateway keepalive and setup can hang after PowerShell exits if a descendant still holds redirected stdout while the inspector waits on
GetResult().Real environment tested: macOS 15, .NET 10.0.400, worktree
/tmp/wn-cmdline-drainonfix/bound-process-cmdline-drain.Exact steps or command run after this patch:
dotnet test ./tests/OpenClaw.Connection.Tests/OpenClaw.Connection.Tests.csproj --filter FullyQualifiedName~WindowsTcpListenerSnapshotTests --nologoEvidence after fix: terminal output from the patched helper. After child exit, a never-completing stdout Task returns null inside the leftover drain budget:
Observed result after fix:
AwaitRedirectedOutputreturns null instead of blocking. Invalid PIDs still return null. WSL keepalive still exposesGetProcessCommandLine(contract check passed).What was not tested: Live Windows CIM/PowerShell against a real descendant-held pipe. This checkout is macOS, so the hang detector uses a local
/bin/shchild plus a never-completing Task.Security Impact
No)No)No)No)No)Yes, explain the risk and mitigation:Compatibility and Migration
Yes)No)No)Review Conversations