fix: add windowsHide and explicit stdio pipes to prevent pwsh.exe stdout loss#389
fix: add windowsHide and explicit stdio pipes to prevent pwsh.exe stdout loss#389ludoplex wants to merge 1 commit intowonderwhy-er:mainfrom
Conversation
…out loss When defaultShell is pwsh.exe (PowerShell 7), child processes spawned by start_process fail to capture stdout because .NET 8 runtime allocates a new console via AllocConsole() instead of inheriting Node.js stdio pipes. Add windowsHide: true to suppress new console allocation and explicitly set stdio to [pipe, pipe, pipe] so child output routes back through Node.js regardless of the shell runtime. Fixes wonderwhy-er#378 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughThe change modifies Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
bot
added
the
size:XS
Sequence DiagramThis PR updates process spawning so shell sessions always use explicit stdio pipes and hidden windows on Windows. The main effect is that commands run through pwsh now return stdout to the manager instead of losing output to a separate console window. sequenceDiagram
participant Client
participant TerminalManager
participant ShellProcess
participant ExternalProgram
Client->>TerminalManager: start_process with command
TerminalManager->>ShellProcess: Spawn with piped stdio and windows hidden
ShellProcess->>ExternalProgram: Execute requested command
ExternalProgram-->>ShellProcess: Emit stdout
ShellProcess-->>TerminalManager: Forward stdout through pipe
Client->>TerminalManager: read_process_output
TerminalManager-->>Client: Return captured output lines
Generated by CodeAnt AI |
There was a problem hiding this comment.
Pull request overview
This PR updates the process-spawning logic in TerminalManager to improve stdout/stderr capture reliability on Windows when the configured default shell is PowerShell 7 (pwsh.exe), addressing the reported “external executable produces no captured output” behavior.
Changes:
- Adds
windowsHide: trueto thespawn()options to avoid showing/creating a visible console window when launching shells on Windows. - Explicitly sets
stdio: ['pipe', 'pipe', 'pipe']so child processes consistently use Node-managed pipes for stdout/stderr capture.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| spawnConfig = getShellSpawnArgs(shellToUse, enhancedCommand); | ||
| spawnOptions = { | ||
| stdio: ['pipe', 'pipe', 'pipe'] as const, | ||
| windowsHide: true, // Prevent .NET-based shells (pwsh.exe) from allocating a new console |
There was a problem hiding this comment.
The inline comment about windowsHide: true is likely overstating what Node guarantees. In Node.js, windowsHide hides the console window (and may set CREATE_NO_WINDOW), but it doesn’t strictly “prevent” a .NET runtime from calling AllocConsole(). Consider rephrasing to reflect the actual behavior (hide the window / avoid showing a new console) so future debugging isn’t misled.
| windowsHide: true, // Prevent .NET-based shells (pwsh.exe) from allocating a new console | |
| windowsHide: true, // Request that Windows hide the console window (avoid showing a new console for .NET-based shells like pwsh.exe) |
| spawnOptions = { | ||
| stdio: ['pipe', 'pipe', 'pipe'] as const, | ||
| windowsHide: true, // Prevent .NET-based shells (pwsh.exe) from allocating a new console | ||
| env: { |
There was a problem hiding this comment.
This change is addressing a Windows+pwsh-specific stdout capture regression, but there’s no automated test exercising “pwsh + external executable stdout” (e.g., pwsh -Command "python -c ..." or git --version"). Adding a Windows-gated regression test (skipping if pwsh isn’t present) would help prevent reintroducing the issue.
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/terminal-manager.ts (1)
160-161: Extract the shared spawn defaults and update the stale note.
stdio,windowsHide, andenvare now part of the Windows capture fix, but they're duplicated across both branches and the note at Lines 142-143 still says no special stdio handling is needed. Pulling the shared options into one base object and updating that note will make this less likely to drift later.♻️ Suggested cleanup
- // Note: No special stdio options needed here, Node.js handles pipes by default + // Explicit pipes and `windowsHide` are part of the Windows `pwsh.exe` capture fix. + + const baseSpawnOptions = { + stdio: ['pipe', 'pipe', 'pipe'] as const, + windowsHide: true, + env: { + ...process.env, + TERM: 'xterm-256color' + } + }; if (typeof shellToUse === 'string') { // Use shell-specific configuration with login flags where appropriate spawnConfig = getShellSpawnArgs(shellToUse, enhancedCommand); - spawnOptions = { - stdio: ['pipe', 'pipe', 'pipe'] as const, - windowsHide: true, // Prevent .NET-based shells (pwsh.exe) from allocating a new console - env: { - ...process.env, - TERM: 'xterm-256color' // Better terminal compatibility - } - }; + spawnOptions = { ...baseSpawnOptions }; // Add shell option if needed (for unknown shells) if (spawnConfig.useShellOption) { spawnOptions.shell = spawnConfig.useShellOption; } @@ - spawnOptions = { - shell: shellToUse, - stdio: ['pipe', 'pipe', 'pipe'] as const, - windowsHide: true, - env: { - ...process.env, - TERM: 'xterm-256color' - } - }; + spawnOptions = { ...baseSpawnOptions, shell: shellToUse }; }Also applies to: 181-182
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/terminal-manager.ts` around lines 160 - 161, Extract the duplicated spawn settings (stdio, windowsHide, env) into a single shared constant (e.g., DEFAULT_SPAWN_OPTIONS or baseSpawnOptions) and use that object in both places where child processes are spawned instead of repeating the array/object inline; replace the inline { stdio: ['pipe','pipe','pipe'] as const, windowsHide: true, env: ... } occurrences with a spread/merge off the shared constant so per-call overrides remain possible, and update the nearby stale comment about stdio handling to reflect that special stdio/windowsHide/env options are now intentionally applied for the Windows capture fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/terminal-manager.ts`:
- Around line 160-161: Extract the duplicated spawn settings (stdio,
windowsHide, env) into a single shared constant (e.g., DEFAULT_SPAWN_OPTIONS or
baseSpawnOptions) and use that object in both places where child processes are
spawned instead of repeating the array/object inline; replace the inline {
stdio: ['pipe','pipe','pipe'] as const, windowsHide: true, env: ... }
occurrences with a spread/merge off the shared constant so per-call overrides
remain possible, and update the nearby stale comment about stdio handling to
reflect that special stdio/windowsHide/env options are now intentionally applied
for the Windows capture fix.
User description
Summary
windowsHide: truetospawn()options to prevent .NET 8 runtime (used by pwsh.exe) from allocating a new console viaAllocConsole()stdio: ['pipe', 'pipe', 'pipe']to ensure child process output routes through Node.js pipes regardless of shell runtimeRoot cause
When
defaultShellispwsh.exe(PowerShell 7), the .NET 8 runtime allocates its own console instead of inheriting Node.js stdio pipes. This causes external executables (Python, Git, Node.js) to emit stdout to a visible desktop window rather than back through the captured pipe, resulting in zero output lines forstart_processandread_process_output.Test plan
start_processwithpwsh.exeas defaultShell captures stdout frompython -c "print('hello')"start_processwithpwsh.execaptures stdout fromgit --versionstart_processwithpowershell.exestill works as before (regression check)start_processwithcmd.exestill works as before (regression check)Fixes #378
Generated with Claude Code
CodeAnt-AI Description
Keep command output visible when using PowerShell on Windows
What Changed
Impact
✅ Fewer missing command outputs on Windows✅ Clearer terminal logs from external tools✅ No extra console window for PowerShell commands💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit