v1.43.3.0 fix(browse): headed-mode idle timer + onDisconnect target wrong BrowserManager for embedders

[garrytan]

Summary

Four module-level lifecycle handlers in browse/src/server.ts (idleCheckTick, parent watchdog, SIGTERM handler, onDisconnect default wire) read the module-level BrowserManager directly. Embedders (gbrowser's phoenix overlay) pass their own instance into buildFetchHandler({ browserManager: ... }), so the module-level instance never has launchHeaded() called on it. connectionMode stays 'launched' forever, headed-mode early-returns never fire, and after 30 minutes of HTTP idle the server self-terminates out from under the still-open overlay window.

Fix: introduce let activeBrowserManager: BrowserManager at module scope (symmetric with the existing let activeShutdown pattern). buildFetchHandler retargets it at cfg.browserManager and CHAINS cfg.browserManager.onDisconnect to activeShutdown — preserving any caller-installed handler instead of clobbering it.

3-commit bisect:

1. fix(browse): 6 edit sites in browse/src/server.ts (Edits 1, 2, 3, 4, 5-verify, 6) + __testInternals__ export.
2. test(browse): 5 new behavioral tests in browse/test/server-factory.test.ts + delete the broken string-grep test from browse/test/sidebar-ux.test.ts.
3. chore: VERSION + CHANGELOG for v1.43.3.0.

Test Coverage

CODE PATHS (new/changed)
[+] browse/src/server.ts
  ├── idleCheckTick() [NEW EXTRACTION]
  │   ├── headed early-return            → [★★★ T1: REGRESSION]
  │   ├── tunnel early-return            → [★★★ T4]
  │   └── threshold exceeded + shutdown  → [★★★ T2: paired defensive]
  ├── parent watchdog (line ~658)         → covered by T5 static guard
  ├── SIGTERM handler (line ~1212)         → covered by T5 static guard
  ├── buildFetchHandler binding (Edit 4)
  │   └── activeBrowserManager retarget   → covered via T1 setup path
  └── cfgBrowserManager.onDisconnect chain → [★★★ T3 — async .rejects.toThrow]

COVERAGE: 5/5 paths tested (100%)
QUALITY: ★★★:5  |  GAPS: 0

Tests run: 33/33 pass in browse/test/server-factory.test.ts; 50/50 across browser-manager-unit + server-factory.

Pre-Landing Review

• /plan-eng-review ran clean (0 arch / 0 quality / 0 perf issues; 2 test gaps surfaced via D1 + D2 and resolved with T3 + T4).
• /codex plan review caught 6 substantive issues the static eng review missed: Bun memoizes dynamic imports, initRegistry token-reuse throws, shutdown() is async (sync .toThrow() cannot catch the rejection), cfg.browserManager.onDisconnect is a public field callers may set (would have been silently clobbered by a bare reassign), the original plan missed the SIGTERM site at line 1186, and tests belonged in server-factory.test.ts not sidebar-ux.test.ts. All 6 verified against actual code and incorporated into the shipped plan. The chain semantics in Edit 4, the SIGTERM site in Edit 6, the __testInternals__ surface (including resetShutdownState + setLastActivity), the test relocation, and the .rejects.toThrow assertion all trace to Codex findings.

Plan Completion

All plan items DONE: 6 production edits applied (browse/src/server.ts:601, 658, 705, 1212, 1387, 1394), 5 behavioral tests added (browse/test/server-factory.test.ts:388–530), broken string-grep test deleted (browse/test/sidebar-ux.test.ts:1596), VERSION + CHANGELOG updated. Sanity-grep confirms exactly 3 module-level activeBrowserManager.getConnectionMode() reads (idleCheckTick, watchdog, SIGTERM) and 0 stray module-level browserManager.getConnectionMode() reads outside handleCommandInternalImpl response payloads (which report mode informationally and are out of scope for lifecycle).

Test plan

• bun test browse/test/server-factory.test.ts — 33/33 pass (5 new + 28 existing factory contract)
• bun test browse/test/browser-manager-unit.test.ts — 17/17 pass
• Static sanity-grep: exactly 3 module-level activeBrowserManager.getConnectionMode() sites (idleCheckTick, watchdog, SIGTERM); 0 stray module-level reads outside handleCommand response payloads
• Manual gbrowser overlay session: headed Chromium runs >30 min HTTP idle without server self-terminating
• Manual window-close on embedder-owned headed window fires cfgBrowserManager.onDisconnect chain (caller cb first, then gstack shutdown)

🤖 Generated with Claude Code

[github-actions]

E2E Evals: ✅ PASS

6/6 tests passed | $1.11 total cost | 12 parallel runners

Suite	Result	Status	Cost
e2e-browse	2/2	✅	$0.13
e2e-deploy	2/2	✅	$0.31
e2e-qa-workflow	1/1	✅	$0.65
llm-judge	1/1	✅	$0.02

---

12x ubicloud-standard-8 (Docker: pre-baked toolchain + deps) | wall clock ≈ slowest suite