fix: headed browser auto-shutdown + disconnect cleanup (v0.18.1.0)#1025
Merged
fix: headed browser auto-shutdown + disconnect cleanup (v0.18.1.0)#1025
Conversation
The parent-process watchdog in server.ts polls the spawning CLI's PID every 15s and self-terminates if it is gone. The connect command in cli.ts exits with process.exit(0) immediately after launching the server, so the watchdog would reliably kill the headed browser within ~15s. This contradicted the idle timer's own design: server.ts:745 explicitly skips headed mode because "the user is looking at the browser. Never auto-die." The watchdog had no such exemption. Two-layer fix: 1. CLI layer: connect handler always sets BROWSE_PARENT_PID=0 (was only pass-through for pair-agent subprocesses). The user owns the headed browser lifecycle; cleanup happens via browser disconnect event or $B disconnect. 2. CLI layer: startServer() honors caller's BROWSE_PARENT_PID=0 in the headless spawn path too. Lets CI, non-interactive shells, and Claude Code Bash calls opt into persistent servers across short-lived CLI invocations. 3. Server layer: defense-in-depth. Watchdog now also skips when BROWSE_HEADED=1, so even if a future launcher forgets PID=0, headed browsers won't die. Adds log lines when the watchdog is disabled so lifecycle debugging is easier. Four community contributors diagnosed variants of this bug independently. Thanks for the clear analyses and reproductions. Closes #1020 (rocke2020) Closes #1018 (sanghyuk-seo-nexcube) Closes #1012 (rodbland2021) Closes #986 (jbetala7) Closes #1006 Closes #943 Co-Authored-By: rocke2020 <noreply@github.com> Co-Authored-By: sanghyuk-seo-nexcube <noreply@github.com> Co-Authored-By: rodbland2021 <noreply@github.com> Co-Authored-By: jbetala7 <noreply@github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When the user closed the headed browser window, the disconnect handler in browser-manager.ts called process.exit(2) directly, bypassing the server's shutdown() function entirely. That meant: - sidebar-agent daemon kept polling a dead server - session state wasn't saved - Chromium profile locks (SingletonLock, SingletonSocket, SingletonCookie) weren't cleaned — causing "profile in use" errors on next $B connect - state file at .gstack/browse.json was left stale Now the disconnect handler calls onDisconnect(), which server.ts wires up to shutdown(2). Full cleanup runs first, then the process exits with code 2 — preserving the existing semantic that distinguishes user-close (exit 2) from crashes (exit 1). shutdown() now accepts an optional exitCode parameter (default 0) so the SIGTERM/SIGINT paths and the disconnect path can share cleanup code while preserving their distinct exit codes. Surfaced by Codex during /plan-eng-review of the watchdog fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 23 tests in this file all shell out to gstack-config + gstack-relink (bash scripts doing subprocess work). Under parallel bun test load, those subprocess spawns contend with other test suites and each test can drift ~200ms past Bun's 5s default timeout, causing 5+ flaky timeouts per run in the gate-tier ship gate. Wrap the `test` import to default the per-test timeout to 15s. Explicit per-test timeouts (third arg) still win, so individual tests can lower it if needed. No behavior change — only gives subprocess-heavy tests more headroom under parallel load. Noticed by /ship pre-flight test run. Unrelated to the main PR fix but blocking the gate, so fixing as a separate commit per the test ownership protocol. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
E2E Evals: ✅ PASS8/8 tests passed | $1.08 total cost | 12 parallel runners
12x ubicloud-standard-2 (Docker: pre-baked toolchain + deps) | wall clock ≈ slowest suite |
Node's signal listeners receive the signal name ('SIGTERM' / 'SIGINT')
as the first argument. When shutdown() started accepting an optional
exitCode parameter in the prior disconnect-cleanup commit, the bare
`process.on('SIGTERM', shutdown)` registration started silently calling
shutdown('SIGTERM'). The string passed through to process.exit(), Node
coerced it to NaN, and the process exited with code 1 instead of 0.
Wrap both listeners so they call shutdown() with no args — signal name
never leaks into the exitCode slot. Surfaced by /ship's adversarial
subagent.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The disconnect handler calls this.onDisconnect() without awaiting it, but server.ts wires the callback to shutdown(2) — which is async. If that promise rejects, the rejection drops on the floor as an unhandled rejection, the browser is already disconnected, and the server keeps running indefinitely with no browser attached. Add a sync try/catch for throws and a .catch() chain for promise rejections. Both fall back to process.exit(2) so a dead browser never leaves a live server. Also widen the callback type from `() => void` to `() => void | Promise<void>` to match the actual runtime shape of the wired shutdown(2) call. Surfaced by /ship's adversarial subagent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The strict string compare `process.env.BROWSE_PARENT_PID === '0'` meant any stray newline or whitespace (common from shell `export` in a pipe or heredoc) would fail the check and re-enable the watchdog against the caller's intent. Switch to parseInt + === 0, matching the server's own parseInt at server.ts:760. Handles '0', '0\n', ' 0 ', and unset correctly; non-numeric values (parseInt returns NaN, NaN === 0 is false) fail safe — watchdog stays active, which is the safe default for unexpected input. Surfaced by /ship's adversarial subagent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit wrapped bun:test's `test` to bump the per-test timeout default to 15s but cast the wrapper `as typeof _bunTest` without copying the sub-properties (`.only`, `.skip`, `.each`, `.todo`, `.failing`, `.if`) from the original. The cast was a lie: the wrapper was a plain function, not the full callable with those chained properties attached. The file doesn't use any of them today, but a future test.only or test.skip would fail with a cryptic "undefined is not a function." Object.assign the original _bunTest's properties onto the wrapper so sub-APIs chain correctly forever. Surfaced by /ship's adversarial subagent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
End-to-end tests in browse/test/watchdog.test.ts that prove the three invariants v0.18.1.0 depends on. Each test spawns the real server.ts (not a mock), so any future change that breaks the watchdog logic fails here — the thing /ship's adversarial review flagged as missing. 1. BROWSE_PARENT_PID=0 disables the watchdog Spawns server with PID=0, reads stdout, confirms the "watchdog disabled (BROWSE_PARENT_PID=0)" log line appears and "Parent process ... exited" does NOT. ~2s. 2. BROWSE_HEADED=1 disables the watchdog (server-side guard) Spawns server with BROWSE_HEADED=1 and a bogus parent PID (999999). Proves BROWSE_HEADED takes precedence over a present PID — if the server-side defense-in-depth regresses, the watchdog would try to poll 999999 and fire on the "dead parent." ~2s. 3. Default headless mode: watchdog fires when parent dies The regression guard for the original orphan-prevention behavior. Spawns a real `sleep 60` parent and a server watching its PID, then kills the parent and waits up to 25s for the server to exit. The watchdog polls every 15s so first tick is 0-15s after death, plus shutdown() cleanup. ~18s. Total runtime: ~21s for all 3 tests. They catch the class of bug this branch exists to fix: "does the process live or die when it should?" Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dc40c2f to
2dc674f
Compare
garrytan
added a commit
that referenced
this pull request
Apr 16, 2026
Catches up commit 822e843 from main: - fix: headed browser auto-shutdown + disconnect cleanup (v0.18.1.0) (#1025) Version collision resolution: Both main (822e843) and my branch (6d879cd) bumped to v0.18.1.0. Per CLAUDE.md's CHANGELOG rule ("Merging main does NOT mean adopting main's version. Your branch still needs its OWN version bump on top"), I preserved main's v0.18.1.0 entry (headed browser fix) as-is and bumped this branch to v0.18.2.0. VERSION + package.json updated to match. CHANGELOG.md conflict resolved: main's v0.18.1.0 entry stays (it landed first); my context-rot-defense entry moves to v0.18.2.0 at the top. TODOS.md auto-merged cleanly (main added a new Browse TODO about scoping sidebar-agent kill to session PID — that stays). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
garrytan
added a commit
that referenced
this pull request
Apr 17, 2026
Resolves overlap between main #1025 (headed browser auto-shutdown fix) and this wave's #994 (server persists across Bash calls) + our SIGTERM mode-gate follow-up. Both touch the parent-PID watchdog and SIGTERM handlers. CHANGELOG / VERSION: - main shipped v0.18.1.0 with #1025 while this wave was open. Bumped this branch to v0.18.2.0 with its own entry on top, per CLAUDE.md branch-scoped CHANGELOG rules. Both entries preserved. - package.json synced to v0.18.2.0. browse/src/server.ts (parent-PID watchdog): - Combined main's outer env-var gate (`BROWSE_HEADED=1` skips the watchdog entirely) with our inner mode check (if watchdog runs and parent dies, decision tree by mode: active picker > headed/tunnel > normal). - Defense-in-depth: env var skips the setInterval cost, runtime mode check catches anything the env var missed. browse/src/server.ts (SIGTERM handler): - Combined main's lambda wrap (avoids signal name being passed as exitCode → NaN → exit 1) with our mode-aware handler (active picker > headed/tunnel shutdown > normal ignore). - Also wrapped SIGINT in a lambda — main's #1025 fix applied to that handler too, but our wave didn't have that fix. Now SIGINT exits with code 0 reliably. browse/test/watchdog.test.ts: - Inverted test 3's expectation: pre-#994 the server died on parent death; post-#994 the server STAYS ALIVE (the whole point — multi-step QA across Claude Code Bash calls now works). Test now verifies isProcessAlive(serverPid) is true after a 20-second observation window with the parent killed. - Updated docstring to document all 4 watchdog invariants (env-var disable, headed-mode disable, headless persists, tunnel shuts down). - Tunnel mode coverage left as documentation rather than test — tunnelActive is a runtime variable set by /pair-agent, not an env var, so faking it would require invasive test-only hooks. Verification: full bun test passes (EXIT=0). Watchdog test: 3 pass, 0 fail (22.7s including the 20s observation window). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
garrytan
added a commit
that referenced
this pull request
Apr 17, 2026
Resolves overlap between main #1025 (headed browser auto-shutdown fix) and this wave's #994 (server persists across Bash calls) + our SIGTERM mode-gate follow-up. Both touch the parent-PID watchdog and SIGTERM handlers. Mechanical resolution only — test inversion follows in next commit. CHANGELOG / VERSION: - main shipped v0.18.1.0 with #1025 while this wave was open. Bumped this branch to v0.18.2.0 with its own entry on top, per CLAUDE.md branch-scoped CHANGELOG rules. Both entries preserved, contiguous version sequence. - package.json synced to v0.18.2.0. browse/src/server.ts (parent-PID watchdog): - Combined main's outer env-var gate (BROWSE_HEADED=1 skips the watchdog entirely) with our inner mode check (when watchdog runs and parent dies, decision tree by mode: active picker > headed/tunnel > normal). - Defense-in-depth: env var skips the setInterval cost, runtime mode check catches anything the env var missed. browse/src/server.ts (SIGTERM/SIGINT handlers): - Combined main's lambda wrap (avoids signal name being passed as exitCode → NaN → exit 1) with our mode-aware SIGTERM handler. Active picker check short-circuits regardless of mode. - Wrapped SIGINT in a lambda too — main's #1025 fix applied to that handler, but our wave didn't have it. Now SIGINT exits with code 0 reliably. Note: browse/test/watchdog.test.ts test 3 (added by main) currently FAILS because main's "watchdog kills on parent death" expectation contradicts post-#994 "server stays alive on parent death." Inverted in the next commit.
garrytan
added a commit
that referenced
this pull request
Apr 17, 2026
main #1025 added browse/test/watchdog.test.ts with test 3 expecting the old "watchdog kills server when parent dies" behavior. The merge with this branch's #994 inverted that semantic — the server now STAYS ALIVE on parent death in normal headless mode (multi-step QA across Claude Code Bash calls depends on this). Changes: - Renamed test 3 from "watchdog fires when parent dies" to "server STAYS ALIVE when parent dies (#994)". - Replaced 25s shutdown poll with 20s observation window asserting the server remains alive after the watchdog tick. - Updated docstring to document all 3 watchdog invariants (env-var disable, headed-mode disable, headless persists) and note tunnel-mode coverage gap. Verification: bun test browse/test/watchdog.test.ts → 3 pass, 0 fail (22.7s). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
garrytan
added a commit
that referenced
this pull request
Apr 17, 2026
* fix: extend tilde-in-assignment fix to design resolver + 4 skill templates PR #993 fixed the Claude Code permission prompt for `scripts/resolvers/browse.ts` and `gstack-upgrade/SKILL.md.tmpl`. Same bug lives in three more places that weren't on the contributor's branch: - `scripts/resolvers/design.ts` (3 spots: D=, B=, and _DESIGN_DIR=) - `design-shotgun/SKILL.md.tmpl` (_DESIGN_DIR=) - `plan-design-review/SKILL.md.tmpl` (_DESIGN_DIR=) - `design-consultation/SKILL.md.tmpl` (_DESIGN_DIR=) - `design-review/SKILL.md.tmpl` (REPORT_DIR=) Replaces bare `~/` with quoted `"$HOME/..."` in the source-of-truth files, then regenerates. `grep -rEn '^[A-Za-z_]+=~/' --include="SKILL.md" .` now returns zero hits across all hosts (claude, codex, cursor, gbrain, hermes). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(openclaw): make native skills codex-friendly (#864) Normalizes YAML frontmatter on the 4 hand-authored OpenClaw skills so stricter parsers like Codex can load them. Codex CLI was rejecting these files with "mapping values are not allowed in this context" on colons inside unquoted description scalars. - Drops non-standard `version` and `metadata` fields - Rewrites descriptions into simple "Use when..." form (no inline colons) - Adds a regression test enforcing strict frontmatter (name + description only) Verified live: Codex CLI now loads the skills without errors. Observed during /codex outside-voice run on the eval-community-prs plan review — Codex stderr tripped on these exact files, which was real-world confirmation the fix is needed. Dropped the connect-chrome changes from the original PR (the symlink removal is out of scope for this fix; keeping connect-chrome -> open-gstack-browser). Co-Authored-By: Cathryn Lavery <cathrynlavery@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(browse): server persists across Claude Code Bash calls The browse server was dying between Bash tool invocations in Claude Code because: 1. SIGTERM: The Claude Code sandbox sends SIGTERM to all child processes when a Bash command completes. The server received this and called shutdown(), deleting the state file and exiting. 2. Parent watchdog: The server polls BROWSE_PARENT_PID every 15s. When the parent Bash shell exits (killed by sandbox), the watchdog detected it and called shutdown(). Both mechanisms made it impossible to use the browse tool across multiple Bash calls — every new `$B` invocation started a fresh server with no cookies, no page state, and no tabs. Fix: - SIGTERM handler: log and ignore instead of shutdown. Explicit shutdown is still available via the /stop command or SIGINT (Ctrl+C). - Parent watchdog: log once and continue instead of shutdown. The existing idle timeout (30 min) handles eventual cleanup. The /stop command and SIGINT still work for intentional shutdown. Windows behavior is unchanged (uses taskkill /F which bypasses signal handlers). Tested: browse server survives across 5+ separate Bash tool calls in Claude Code, maintaining cookies, page state, and navigation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(browse): gate #994 SIGTERM-ignore to normal mode only PR #994 made browse persist across Claude Code Bash calls by ignoring SIGTERM and parent-PID death, relying on the 30-min idle timeout for eventual cleanup. Codex outside-voice review caught that the idle timeout doesn't apply in two modes: headed mode (/open-gstack-browser) and tunnel mode (/pair-agent). Both early-return from idleCheckInterval. Combined with #994's ignore-SIGTERM, those sessions would leak forever after the user disconnects — a real resource leak on shared machines where multiple /pair-agent sessions come and go. Fix: gate SIGTERM-ignore and parent-PID-watchdog-ignore to normal (headless) mode only. Headed + tunnel modes respect both signals and shutdown cleanly. Idle timeout behavior unchanged. Also documents the deliberate contract change for future contributors — don't re-add global SIGTERM shutdown thinking it's missing; it's intentionally scoped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: keep cookie picker alive after cli exits Fixes #985 * fix: add opencode setup support * feat(browse): add Windows browser path detection and DPAPI cookie decryption - Extend BrowserPlatform to include win32 - Add windowsDataDir to BrowserInfo; populate for Chrome, Edge, Brave, Chromium - getBaseDir('win32') → ~/AppData/Local - findBrowserMatch checks Network/Cookies first on Windows (Chrome 80+) - Add getWindowsAesKey() reading os_crypt.encrypted_key from Local State JSON - Add dpapiDecrypt() via PowerShell ProtectedData.Unprotect (stdin/stdout) - decryptCookieValue branches on platform: AES-256-GCM (Windows) vs AES-128-CBC (mac/linux) - Fix hardcoded /tmp → TEMP_DIR from platform.ts in openDbFromCopy Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(browse): Windows cookie import — profile discovery, v20 detection, CDP fallback Three bugs fixed in cookie-import-browser.ts: - listProfiles() and findInstalledBrowsers() now check Network/Cookies on Windows (Chrome 80+ moved cookies from profile/Cookies to profile/Network/Cookies) - openDb() always uses copy-then-read on Windows (Chrome holds exclusive locks) - decryptCookieValue() detects v20 App-Bound Encryption with specific error code Added CDP-based extraction fallback (importCookiesViaCdp) for v20 cookies: - Launches Chrome headless with --remote-debugging-port on the real profile - Extracts cookies via Network.getAllCookies over CDP WebSocket - Requires Chrome to be closed (v20 keys are path-bound to user-data-dir) - Both cookie picker UI and CLI direct-import paths auto-fall back to CDP Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(browse): document CDP debug port security + log Chrome version on v20 fallback Follow-up to #892 per Codex outside-voice review. Two small additions to the Windows v20 App-Bound Encryption CDP fallback: 1. Inline comment documenting the deliberate security posture of the --remote-debugging-port. Chrome binds it to 127.0.0.1 by default, so the threat model is local-user-only (which is no worse than baseline — local attackers can already read the cookie DB). Random port 9222-9321 is for collision avoidance, not security. Chrome is always killed in finally. 2. One-time Chrome version log on CDP entry via /json/version. When Chrome inevitably changes v20 key format or /json/list shape in a future major version, logs will show exactly which version users are hitting. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: v0.18.1.0 — community wave (6 PRs + hardening) VERSION bump + users-first CHANGELOG entry for the wave: - #993 tilde-in-assignment fix (byliu-labs) - #994 browse server persists across Bash calls (joelgreen) - #996 cookie picker alive after cli exits (voidborne-d) - #864 OpenClaw skills codex-friendly (cathrynlavery) - #982 OpenCode native setup (breakneo) - #892 Windows cookie import + DPAPI + v20 CDP fallback (msr-hickory) Plus 3 follow-up hardening commits we own: - Extended tilde fix to design resolver + 4 more skill templates - Gated #994 SIGTERM-ignore to normal mode only (headed/tunnel preserve shutdown) - Documented CDP debug port security + log Chrome version on v20 fallback Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: review pass — package.json version, import dedup, error context, stale help Findings from /review on the wave PR: - [P1] package.json version was 0.18.0.1 but VERSION is 0.18.1.0, failing test/gen-skill-docs.test.ts:177 "package.json version matches VERSION file". Bumped package.json to 0.18.1.0. - [P2] Duplicate import of cookie-picker-routes in browse/src/server.ts (handleCookiePickerRoute at line 20 + hasActivePicker at line 792). Merged into single import at top. - [P2] cookie-import-browser.ts:494 generic rethrow loses underlying error. Now preserves the message so "ENOENT" vs "JSON parse error" vs "permission denied" are distinguishable in user output. - [P3] setup:46 "Missing value for --host" error message listed an incomplete set of hosts (missing factory, openclaw, hermes, gbrain). Aligned with the "Unknown value" error on line 94. Kept as-is (not real issues): - cookie-import-browser.ts:869 empty catch on Chrome version fetch is the correct pattern for best-effort diagnostics (per slop-scan philosophy in CLAUDE.md — fire-and-forget failures shouldn't throw). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(watchdog): invert test 3 to match merged #994 behavior main #1025 added browse/test/watchdog.test.ts with test 3 expecting the old "watchdog kills server when parent dies" behavior. The merge with this branch's #994 inverted that semantic — the server now STAYS ALIVE on parent death in normal headless mode (multi-step QA across Claude Code Bash calls depends on this). Changes: - Renamed test 3 from "watchdog fires when parent dies" to "server STAYS ALIVE when parent dies (#994)". - Replaced 25s shutdown poll with 20s observation window asserting the server remains alive after the watchdog tick. - Updated docstring to document all 3 watchdog invariants (env-var disable, headed-mode disable, headless persists) and note tunnel-mode coverage gap. Verification: bun test browse/test/watchdog.test.ts → 3 pass, 0 fail (22.7s). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ci): switch apt mirror to Hetzner to bypass Ubicloud → archive.ubuntu.com timeouts Both build attempts of `.github/docker/Dockerfile.ci` failed at `apt-get update` with persistent connection timeouts to archive.ubuntu.com:80 and security.ubuntu.com:80 — 90+ seconds of "connection timed out" against every Ubuntu IP. Not a transient blip; this PR doesn't touch the Dockerfile, and a re-run reproduced the same failure across all 9 mirror IPs. Root cause: Ubicloud runners (Hetzner FSN1-DC21 per runner output) have unreliable HTTP-port-80 routing to Ubuntu's official archive endpoints. Fix: - Rewrite /etc/apt/sources.list.d/ubuntu.sources (deb822 format in 24.04) to use https://mirror.hetzner.com/ubuntu/packages instead. Hetzner's mirror is publicly accessible from any cloud (not Hetzner-only despite the name) and route-local for Ubicloud's actual host. Solves both reliability and latency. - Add a 3-attempt retry loop around both `apt-get update` calls as belt-and-suspenders. Even Hetzner's mirror can have brief blips, and the retry costs nothing when the first attempt succeeds. Verification: the workflow will rebuild on push. Local `docker build` not practical for a 12-step image with bun + claude + playwright deps + a 10-min cold install. Trusting CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ci): use HTTP for Hetzner apt mirror (base image lacks ca-certificates) Previous commit switched to https://mirror.hetzner.com/... which proved the mirror is reachable and routes correctly (no more 90s timeouts), but exposed a chicken-and-egg: ubuntu:24.04 ships without ca-certificates, and that's exactly the package we're installing. Result: "No system certificates available. Try installing ca-certificates." Fix: use http:// for the Hetzner mirror. Apt's security model verifies package integrity via GPG-signed Release files, not TLS, so HTTP here is no weaker than the upstream defaults (Ubuntu's official sources also default to HTTP for the same reason). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Cathryn Lavery <cathrynlavery@users.noreply.github.com> Co-authored-by: Joel Green <thejoelgreen@gmail.com> Co-authored-by: d 🔹 <258577966+voidborne-d@users.noreply.github.com> Co-authored-by: Break <breakneo@gmail.com> Co-authored-by: Michael Spitzer-Rubenstein <msr.ext@hickory.ai>
JerkyJesse
added a commit
to JerkyJesse/cavestack
that referenced
this pull request
Apr 18, 2026
Ports gstack garrytan#994 (joelgreen) + garrytan#1020 gating + SIGINT wrapper fix (garrytan#1025). The browse server was dying between Claude Code Bash tool invocations because: 1. SIGTERM: Claude Code sandbox sends SIGTERM to child processes when a Bash command completes. Server called shutdown() and deleted state file. 2. Parent watchdog: 15s BROWSE_PARENT_PID poll shut down on parent death. Both made every new $B call start a fresh server with no cookies or state. Fix (scoped to normal headless mode): - SIGTERM handler: in headless mode, log and ignore. Idle timeout (30min) handles eventual cleanup. Headed/tunnel modes respect SIGTERM and shutdown cleanly (idle timeout early-returns in those modes, so ignoring would leak). - Parent watchdog: same three-way resolution. Active cookie picker always stays alive to avoid stranding picker UI with "Failed to fetch". - SIGINT (Ctrl+C) always shuts down — intentional stop. - Outer IS_HEADED_WATCHDOG gate skips watchdog entirely in headed mode. - shutdown() called via arrow wrapper so signal name arg doesn't become NaN exitCode (fixes legacy exit-code-1 bug). Imports hasActivePicker() from cookie-picker-routes (added in prior commit). /open-cavestack-browser path used in place of /open-gstack-browser in inline docs. Upstream: garrytan/gstack@1211b6b Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
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.
Summary
Headed browser:
/open-gstack-browserand$B connectno longer auto-shutdown after ~15s. Root cause: a parent-process watchdog inserver.tspolling the CLI's PID every 15s, self-terminating when the CLI exited (which happens immediately afterconnect).BROWSE_PARENT_PID=0for headed launches, (2) server-side defense-in-depth skips the watchdog whenBROWSE_HEADED=1.shutdown()for full cleanup (sidebar-agent, session state, profile locks, state file) instead of bypassing it withprocess.exit(2).Headless opt-in:
BROWSE_PARENT_PID=0from the caller's environment is now honored in the headless spawn path, letting CI / Claude Code Bash calls / multi-step workflows share a persistent server across short-lived CLI invocations.Regression fixes (surfaced by /ship's adversarial review):
SIGTERM/SIGINTsignal handlers were silently passing the signal name as the newexitCodearg, makingprocess.exit('SIGTERM')coerce to NaN and exit with code 1. Fixed to always exit 0 for signal shutdown.onDisconnectcallback now has sync try/catch + async.catch()fallback toprocess.exit(2)so a rejecting shutdown can never leave a live server attached to a dead browser.BROWSE_PARENT_PIDparsing usesparseIntto match the server, so0\n(common from shellexport) is honored.Test infrastructure:
test/relink.test.ts: 23 tests that shell out togstack-config+gstack-relinkwere flaking under parallel bun test load (5s default timeout too tight). Wrappedtestto default to 15s, preserving.only/.skip/.eachsub-APIs viaObject.assign.Test Coverage
E2E regression tests added (
browse/test/watchdog.test.ts): 3 tests, ~21s total runtime, spawn the realserver.ts(not mocks).These catch the class of bug this branch exists to fix: "does the process live or die when it should?"
Pre-Landing Review
pkillblast radius)Plan Completion
All plan items DONE. Bisect-friendly commit structure:
4b48f811— headed browser watchdog fix (cli.ts + server.ts)dc3df796— disconnect cleanup via shutdown(2) (server.ts + browser-manager.ts)3c7dc78d— pre-existing relink test flakiness80e13c78— adversarial review fixes2a7eec4c— VERSION + CHANGELOG + TODOdc40c2f6— watchdog regression testsCommunity PRs closed
Four contributors independently diagnosed variants of the same bug. Thanks to:
Closes #1020, #1018, #1012, #986, #1006, #943.
Test plan
bun test browse/test/watchdog.test.ts— 3/3 pass in 21sbun testfull suite — exit 0, no failuresbun run build)$B connect→ wait 60s → browser stays alive$B disconnect→ clean shutdown🤖 Generated with Claude Code