Fix Windows console-window flash on browser/runtime auto-launch#471
Conversation
Add windowsHide:true to every spawn() that launches a browser via cmd.exe (shell:true on win32) or starts the Docker/Podman engine. Without it, Node's spawn defaults windowsHide to false, so the intermediate console-subsystem process briefly flashes a visible window before handing off. win32-only option, no behavior change elsewhere.
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✨ 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 |
PR Summary by QodoHide Windows console flash when auto-launching browser/runtime
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
300 rules✅ Skills:
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| // On win32 this runs through `cmd.exe /c start <url>` (shell: true); without | ||
| // windowsHide, that intermediate cmd.exe briefly flashes a visible console | ||
| // window before handing off to the actual browser process. | ||
| spawn(cmd, [url], { | ||
| stdio: 'ignore', | ||
| detached: true, | ||
| shell: process.platform === 'win32', | ||
| windowsHide: true, | ||
| }).unref(); |
There was a problem hiding this comment.
1. Duplicated openbrowser/openurl spawn logic 📜 Skill insight ⚙ Maintainability
The PR repeats near-identical spawn(..., { shell: win32, windowsHide: true, ... }) browser-launch
logic (and explanatory comments) across multiple files, increasing maintenance overhead and drift
risk. This violates the DRY compliance requirement to refactor duplicated logic into a shared
helper/module.
Agent Prompt
## Issue description
The same `openBrowser`/`openUrl` spawn configuration (including `shell: process.platform === 'win32'` and `windowsHide: true`) is duplicated across multiple files, which makes future changes error-prone and inconsistent.
## Issue Context
This PR already touches all copies of the helper, which is a good opportunity to consolidate into a single shared utility (e.g., `src/utils/open-browser.{js,ts}`) and import it where needed.
## Fix Focus Areas
- src/hooks/auto-launch.js[56-64]
- src/observability/dashboard/server.js[128-135]
- src/integrations/pi/rstack-sdlc.ts[102-109]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
✅ Fixed — exported openBrowser (auto-launch.js) / openUrl (rstack-sdlc.ts) with an injectable platform/spawnImpl (mirrors sandbox.js's existing spawnImpl convention), and added tests/windows-console-flash-470.test.js pinning windowsHide:true + per-platform shell selection for both, plus sandbox.js's startContainerRuntime. dashboard/server.js's identical copy is covered by code-review parity rather than a direct import test — that module calls server.listen() unguarded at the top level (pre-existing, out of scope here), so importing it for the export would start a real HTTP server as a side effect.
Qodo flagged that the windowsHide:true change shipped with no tests. Export openBrowser (auto-launch.js, dashboard/server.js) and openUrl (rstack-sdlc.ts) with an injectable platform/spawnImpl (mirrors sandbox.js's existing spawnImpl convention) so the fix is testable without actually spawning a browser. Pins windowsHide:true + shell selection per-platform for auto-launch.js, rstack-sdlc.ts, and sandbox.js's startContainerRuntime. dashboard/server.js's identical openBrowser copy isn't covered by a direct import test: that module calls server.listen() unguarded at the top level (pre-existing, out of scope here), so importing it for the export would start a real HTTP server as a side effect. Covered by code-review parity with the other two call sites instead.
Qodo Fix Summary — Round 1Reviewed and addressed Qodo review issues: ✅ Fixed Issues
⏭️ Deferred Issues
|
openUrl (rstack-sdlc.ts) intentionally early-returns when process.env.CI is set, to avoid popping a browser during automated runs. GitHub Actions always sets CI=true, so the two openUrl tests added in the last commit never actually reached spawnImpl there, while passing locally (CI unset) — caught by PR #471's real CI run, not by local verification. Clear process.env.CI for the duration of those two tests (restored after) so they exercise the real spawn path in CI too, and add a dedicated test pinning the CI no-op behavior itself so it stays covered rather than just being an env quirk the other tests dodge. Verified locally both with and without CI=true set.
2 participants
Summary
rstack-agents hub, Pi session-start) and the Docker/Podmandoctor --start-runtimeengine auto-start briefly flash a visiblecmd.execonsole window.spawn(cmd, args, { shell: true })on win32 runs throughcmd.exe /c ...;child_process.spawndefaultswindowsHidetofalse, so that intermediate console-subsystem process is visible for a split second before it hands off and exits.openBrowser/openUrlhelper existed in three files (src/hooks/auto-launch.js,src/observability/dashboard/server.js,src/integrations/pi/rstack-sdlc.ts), plus the same gap instartContainerRuntime(src/core/harness/sandbox.js), which spawnscmd.exe/podman.exedirectly on win32.windowsHide: trueon all four spawn calls. win32-only option — no behavior change on macOS/Linux, and no change to what actually launches (browser/engine still start identically), only the transient console window is suppressed.Test plan
npm run lint— clean on the four touched filesnpm run typecheck— 0 errorsnpx tsx --test tests/integrations-init.test.js tests/sandbox-doctor-452.test.js tests/sandbox-execution-452.test.js tests/harness-observability.test.js— same 54 pass / 1 pre-existing unrelated fail (Windows path-separator flake insandbox-execution-452.test.js, confirmed present identically without this change viagit stash)windowsHide) and fix are directly confirmed against Node's documentedchild_process.spawnbehavior