Skip to content

fix: suppress Windows console flashes from browse cold-start child spawns#1864

Open
mvanhorn wants to merge 3 commits into
garrytan:mainfrom
mvanhorn:fix/1784-windows-bun-spawnsync-console-flash
Open

fix: suppress Windows console flashes from browse cold-start child spawns#1864
mvanhorn wants to merge 3 commits into
garrytan:mainfrom
mvanhorn:fix/1784-windows-bun-spawnsync-console-flash

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

@mvanhorn mvanhorn commented Jun 4, 2026

Summary

Suppresses the focus-stealing console windows that flash on every cold start of browse on Windows by routing the console-subsystem child spawns through a hidden path. execFileSync('icacls', ...) in browse/src/file-permissions.ts gets windowsHide: true, and the Bun.spawnSync launcher and git-root calls in browse/src/cli.ts and browse/src/config.ts fall back to Node's child_process with windowsHide: true on Windows, since Bun.spawnSync does not honor windowsHide on bun 1.3.11.

Why this matters

Issue #1784 reports that on Windows 11 every cold start of browse flashes two or more focus-stealing conhost.exe windows that interrupt typing in other apps. A Win32_ProcessStartTrace traced the flashes to console-subsystem children spawned without an effective windowsHide: the node detach launcher and git rev-parse calls go through Bun.spawnSync, which ignores windowsHide on bun 1.3.11, and the icacls.exe calls use execFileSync without the flag. The fix adds windowsHide: true to the execFileSync calls in file-permissions.ts and, on Windows only, routes the launcher and git lookups in cli.ts and config.ts through Node's child_process (which honors the flag), keeping Bun.spawnSync on POSIX behind the existing platform check.

Testing

browse/test/cli-windows-hide.test.ts is a static regression test asserting the cold-start spawn sites carry windowsHide or use the hidden-spawn helper on Windows, and fails if a console-subsystem spawn site reintroduces a non-hidden Windows spawn. POSIX continues to use Bun.spawnSync unchanged. Full suite runs in CI.

Fixes #1784

@trunk-io
Copy link
Copy Markdown

trunk-io Bot commented Jun 4, 2026

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@buddy7599bot
Copy link
Copy Markdown

Flagging an overlap so this and your #1863 don't collide: both fix the same defect class — Windows console flashes from browse cold-start spawns (#1784 here, #1835 there) — and both add a new browse/test/cli-windows-hide.test.ts, so they'll conflict on that file once either merges.

They also cover disjoint spawn sites:

So neither PR is complete in isolation — the full fix is the union (outer launcher + inner detached spawn + taskkill + git + icacls). Suggest folding #1863's launcherCode change into this PR (the broader of the two), keeping a single cli-windows-hide.test.ts, and closing #1863 as superseded. Worth extending this PR's test to also assert windowsHide:true appears inside the launcherCode string, so the merged version pins both halves.

@kaiwulff
Copy link
Copy Markdown

kaiwulff commented Jun 4, 2026

Thanks for picking this up — the Bun.spawnSync→Node child_process approach here matches what we've been running as a local patch on Windows since #1784, so this is great to see upstreamed.

One gap worth folding in while you're here: browse/src/find-browse.ts has the same uncovered call that config.ts does. At the current tip its getGitRoot() is:

function getGitRoot(): string | null {
  ...
  const proc = Bun.spawnSync(['git', 'rev-parse', '--show-toplevel'], { ... });

That's the identical git rev-parse via Bun.spawnSync this PR fixes in config.ts, but find-browse.ts isn't in the changeset (only cli.ts, config.ts, file-permissions.ts). In our Win32_ProcessStartTrace for #1784, getGitRoot() showed up as a flicker source from both config.ts and find-browse.ts, and find-browse compiles to its own console-subsystem binary — so this one will keep flashing after the PR lands. Applying the same Windows-only Node-child_process + windowsHide: true treatment to find-browse.ts's getGitRoot() (and extending the cli-windows-hide.test.ts regression to cover that spawn site) should close it.

@mvanhorn
Copy link
Copy Markdown
Contributor Author

mvanhorn commented Jun 4, 2026

Good to hear it matches what you've been running locally since #1784, that's solid real-world validation. On the overlap flag: #1863 and this PR are complementary rather than competing. #1863 adds windowsHide to the long-lived detached server spawn; this one covers the short-lived cold-start child spawns that flash a console per invocation. Either can land first; no shared lines.

@mvanhorn
Copy link
Copy Markdown
Contributor Author

mvanhorn commented Jun 4, 2026

Correcting my earlier comment: the two PRs did overlap after all, both touched the startServer launcher block and both created cli-windows-hide.test.ts. Rather than leave a guaranteed merge conflict, #1863's changes are folded in here (commits 4d30746 and 77ebc14): windowsHide on the inner detached server spawn, the osascript raise helper, and the POSIX server spawn, plus its every-detached-spawn-carries-windowsHide test invariant. #1863 is closed; this PR is now the complete console-flash fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: browse cold-start flashes focus-stealing console windows (Bun.spawnSync ignores windowsHide)

3 participants