Skip to content

fix browse lifecycle on Windows#1983

Draft
garrett-fox wants to merge 1 commit into
garrytan:mainfrom
garrett-fox:codex/windows-browse-lifecycle
Draft

fix browse lifecycle on Windows#1983
garrett-fox wants to merge 1 commit into
garrytan:mainfrom
garrett-fox:codex/windows-browse-lifecycle

Conversation

@garrett-fox

Copy link
Copy Markdown

Summary

Fixes the Windows browse lifecycle path so status/stop do not accidentally start or leave behind a headless daemon, and hides gstack-owned helper subprocess windows on Windows.

What changed

  • make browse status passive by reading the state file and querying a new authenticated /status endpoint instead of calling ensureServer()
  • make browse stop operate on an existing state file without autostarting a daemon
  • add windowsHide: true to gstack-owned Windows subprocess launches, including taskkill, Node daemon launch, Bun polyfill subprocesses, and cookie-import helpers
  • shorten the default Windows headless idle timeout to 5 minutes while keeping non-Windows at 30 minutes
  • add focused CLI lifecycle tests that verify passive status behavior without booting the full browser integration suite

Why

On Windows, browse commands can surface short-lived PowerShell/console windows while agents are active. Separately, browse status previously went through the normal server setup path, so checking whether a daemon existed could create one. That made stale or stray daemon investigation harder and could keep the chrome-headless-shell process cycling after a session appeared to be done.

This addresses the behavior reported in #1835 and #1784.

Validation

  • bun test browse/test/cli-lifecycle.test.ts browse/test/cli-setsid-daemonize.test.ts browse/test/cookie-import-browser.test.ts
  • git diff --cached --check

I also smoke-tested the patched local install on Windows before extracting this PR: browse status with no daemon stayed stopped, browse goto https://example.com launched successfully, and browse stop removed the daemon and child headless Chromium processes.

Notes

This PR only changes tracked gstack source. I intentionally did not include a direct local patch to node_modules/playwright-core; if Playwright's internal process launcher still contributes visible windows after this, that should be handled as a separate build-time patch or upstream Playwright issue.

@trunk-io

trunk-io Bot commented Jun 12, 2026

Copy link
Copy Markdown

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

@jbetala7

Copy link
Copy Markdown
Contributor

Heads-up on a concrete overlap with #1998 (also targeting the #1952 Windows console-window flashes).

Both PRs independently make the same two edits in browse/src/cli.ts (lines 326 and 328 on current main):

  • add windowsHide: true to the detached Node daemon spawn ({detached:true,stdio:['ignore','ignore','ignore'],...})
  • add windowsHide: true to the Bun.spawnSync(['node', '-e', launcherCode], { stdio: [...] }) launcher

So whichever lands first, the other conflicts on those exact lines and one set of edits becomes redundant.

The rest of this PR is unique and doesn't overlap #1998: the passive browse status/stop path (state file + authenticated /status endpoint instead of ensureServer()), the new /status route in server.ts, windowsHide on taskkill, and the shorter Windows idle timeout. #1998's unique parts are the --headless=new switch and the terminal-agent-control.ts agent-spawn windowsHide.

Suggestion to avoid duplicate merges: let one PR own the shared cli.ts spawn windowsHide edits and narrow the other to its unique surface, or note the dependency so the second can rebase cleanly. Flagging for sequencing, not blocking.

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.

2 participants