From 322d2553545c15795da7271a6f70122bc36aa561 Mon Sep 17 00:00:00 2001 From: Andrei Vershinin Date: Sat, 17 Oct 2026 14:07:19 +0000 Subject: [PATCH] fix(browse): /health status is daemon liveness; active tab reported apart (#3247) /health answered `status` from a page.evaluate('1') race on the active tab, so one page stuck in a long script made the whole daemon read as unhealthy. The CLI gates every command on that answer, and its 2000 ms fetch timeout equalled the server's 2000 ms race, so it could not even read the reply: status, tabs, tab, newtab and stop all failed with "Daemon busy" until --force-restart threw the session away. - BrowserManager.isHealthy() is liveness only (Chromium connected). - probeActiveTab() reports { id, responsive }, bounded by TAB_PROBE_TIMEOUT_MS (1s); /health returns it as `activeTab`. - `status` adds a line naming a not-responding active tab and the commands that leave it. - The CLI reads /health through one helper whose timeout, HEALTH_REQUEST_TIMEOUT_MS (2s), stays above the probe. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Wb6Wntntmwau19fSuSbuaL --- browse/src/browser-manager.ts | 60 +++++++++++++++++++++++------- browse/src/cli.ts | 50 +++++++++++++------------ browse/src/meta-commands.ts | 5 +++ browse/src/routes/core.ts | 8 +++- browse/test/terminal-agent.test.ts | 7 +++- 5 files changed, 90 insertions(+), 40 deletions(-) diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 338c6dd6b..419299859 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -290,6 +290,30 @@ export async function handleChromiumDisconnect(browser: Browser | null): Promise process.exit(1); } +/** + * How long a tab-independent read waits on one tab's renderer (#3247). A page + * stuck in a long script never answers page.evaluate() or page.title(); before + * this bound, one such tab made /health call the whole daemon unhealthy and + * hung `tabs`. Kept well under the CLI's /health request timeout + * (HEALTH_REQUEST_TIMEOUT_MS in cli.ts) so the CLI reads the answer instead of + * timing out on it. + */ +export const TAB_PROBE_TIMEOUT_MS = 1_000; + +/** + * Resolve with `work`'s value, or with undefined once `ms` passes. The losing + * read settles on its own when the renderer frees up, so callers pass a promise + * that cannot reject. + */ +function withinTabBudget(work: Promise, ms: number): Promise { + let timer: ReturnType | undefined; + const deadline = new Promise((resolve) => { + timer = setTimeout(() => resolve(undefined), ms); + (timer as { unref?: () => void }).unref?.(); + }); + return Promise.race([work, deadline]).finally(() => clearTimeout(timer)); +} + export type { RefEntry }; // Re-export TabSession for consumers @@ -1090,20 +1114,28 @@ export class BrowserManager { this.closeOwnedDisplay(); } - /** Health check — verifies Chromium is connected AND responsive */ - async isHealthy(): Promise { - if (!this.browser || !this.browser.isConnected()) return false; - try { - const page = this.pages.get(this.activeTabId); - if (!page) return true; // connected but no pages — still healthy - await Promise.race([ - page.evaluate('1'), - new Promise((_, reject) => setTimeout(() => reject(new Error('timeout')), 2000)), - ]); - return true; - } catch { - return false; - } + /** + * Daemon liveness for /health: Chromium is connected. The active tab is NOT + * part of it (#3247): a page stuck in a long script is one busy tab, not a + * dead daemon, and the CLI gates every command on this answer — including + * the tabs, tab, closetab and stop commands that get out of that state. + * probeActiveTab() reports the tab on its own. + */ + isHealthy(): boolean { + return Boolean(this.browser?.isConnected()); + } + + /** + * Does the active tab's main thread answer within `timeoutMs`? Null when + * there is no active page. A rejected evaluate (crashed or closing target) + * also counts as not responsive. + */ + async probeActiveTab(timeoutMs = TAB_PROBE_TIMEOUT_MS): Promise<{ id: number; responsive: boolean } | null> { + const id = this.activeTabId; + const page = this.pages.get(id); + if (!page) return null; + const answered = await withinTabBudget(page.evaluate('1').then(() => true, () => false), timeoutMs); + return { id, responsive: answered === true }; } // ─── Tab Management ──────────────────────────────────────── diff --git a/browse/src/cli.ts b/browse/src/cli.ts index d774d23d5..120f90f76 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -215,39 +215,43 @@ function readState(): ServerState | null { // isProcessAlive is imported from ./error-handling -/** - * HTTP health check — definitive proof the server is alive and responsive. - * Used in all polling loops instead of isProcessAlive() (which is slow on Windows). - */ -export async function isServerHealthy(port: number, timeoutMs = 2000): Promise { - try { - const resp = await fetch(`http://127.0.0.1:${port}/health`, { - signal: AbortSignal.timeout(timeoutMs), - }); - if (!resp.ok) return false; - const health = await resp.json() as any; - return health.status === 'healthy'; - } catch { - return false; - } -} +/** Client timeout for one GET /health. Must stay above the daemon's active-tab + * probe (TAB_PROBE_TIMEOUT_MS in browser-manager.ts): when the two were equal, + * the CLI aborted just before a busy tab's reply arrived and could not tell a + * busy tab from a hung daemon (#3247). */ +export const HEALTH_REQUEST_TIMEOUT_MS = 2000; -/** Best-effort tab count via GET /health (no auth, bounded). Returns null - * when the daemon doesn't answer in time or predates the `tabs` field — - * callers degrade to a countless phrasing, never block on this. */ -async function fetchDaemonTabCount(port: number, timeoutMs = 2000): Promise { +/** GET /health (no auth, bounded). Null when the daemon doesn't answer in + * time or the reply is not a 200. */ +async function fetchDaemonHealth(port: number, timeoutMs = HEALTH_REQUEST_TIMEOUT_MS): Promise | null> { try { const resp = await fetch(`http://127.0.0.1:${port}/health`, { signal: AbortSignal.timeout(timeoutMs), }); if (!resp.ok) return null; - const health = await resp.json() as any; - return typeof health.tabs === 'number' ? health.tabs : null; + return await resp.json() as Record; } catch { return null; } } +/** + * HTTP health check — definitive proof the server is alive and responsive. + * Used in all polling loops instead of isProcessAlive() (which is slow on Windows). + * `status` is daemon liveness only; a busy active tab does not fail it (#3247). + */ +export async function isServerHealthy(port: number, timeoutMs = HEALTH_REQUEST_TIMEOUT_MS): Promise { + return (await fetchDaemonHealth(port, timeoutMs))?.status === 'healthy'; +} + +/** Best-effort tab count via GET /health. Returns null when the daemon + * doesn't answer in time or predates the `tabs` field — callers degrade to a + * countless phrasing, never block on this. */ +async function fetchDaemonTabCount(port: number): Promise { + const tabs = (await fetchDaemonHealth(port))?.tabs; + return typeof tabs === 'number' ? tabs : null; +} + // ─── Process Management ───────────────────────────────────────── async function killServer(pid: number): Promise { if (!isProcessAlive(pid)) return; @@ -420,7 +424,7 @@ async function probeHealthWithBackoff( await Bun.sleep(intervalMs); const remainingMs = deadline - Date.now(); if (remainingMs <= 0) return false; - if (await isServerHealthy(port, Math.min(2000, remainingMs))) return true; + if (await isServerHealthy(port, Math.min(HEALTH_REQUEST_TIMEOUT_MS, remainingMs))) return true; } } diff --git a/browse/src/meta-commands.ts b/browse/src/meta-commands.ts index 8cc9e2e6c..d79ee91b4 100644 --- a/browse/src/meta-commands.ts +++ b/browse/src/meta-commands.ts @@ -404,12 +404,17 @@ export async function handleMetaCommand( case 'status': { const tabs = bm.getTabCount(); const mode = bm.getConnectionMode(); + const activeTab = await bm.probeActiveTab(); return [ `Status: healthy`, `Mode: ${mode}`, `URL: ${bm.getCurrentUrl()}`, `Tabs: ${tabs}`, `PID: ${process.pid}`, + // #3247: name the busy tab and the ways out that keep the session. + ...(activeTab && !activeTab.responsive + ? [`Active tab ${activeTab.id}: not responding. Switch with: browse tab , or close it: browse closetab ${activeTab.id}`] + : []), ].join('\n'); } diff --git a/browse/src/routes/core.ts b/browse/src/routes/core.ts index 491fc5f76..611448594 100644 --- a/browse/src/routes/core.ts +++ b/browse/src/routes/core.ts @@ -77,13 +77,19 @@ export const coreRoutes: RouteEntry[] = [ // Health check — no auth required, does NOT reset idle timer. NEVER carries // a token in any mode: token bootstrap is POST /extension-token and shell // auth is POST /pty-session. Liveness/status only. + // + // `status` is daemon liveness (Chromium connected); the CLI gates every + // command on it. `activeTab.responsive` is the active tab's main thread, + // probed for at most TAB_PROBE_TIMEOUT_MS and reported beside it: one busy + // page must not read as a dead daemon (#3247). { method: '*', path: '/health', auth: 'none', surfaces: ['local'], handler: async (_req, _r, ctx) => { const { browserManager } = ctx; - const healthy = await browserManager.isHealthy(); + const healthy = browserManager.isHealthy(); return json({ status: healthy ? 'healthy' : 'unhealthy', + activeTab: healthy ? await browserManager.probeActiveTab() : null, mode: browserManager.getConnectionMode(), uptime: Math.floor((Date.now() - ctx.startTime) / 1000), tabs: browserManager.getTabCount(), diff --git a/browse/test/terminal-agent.test.ts b/browse/test/terminal-agent.test.ts index f066d1a23..c5b247ac1 100644 --- a/browse/test/terminal-agent.test.ts +++ b/browse/test/terminal-agent.test.ts @@ -94,13 +94,16 @@ describe('/health does NOT surface ptyToken', () => { test('/health response body does not include ptyToken', async () => { // It's allowed to include `terminalPort` (a port number, not auth). const ctx = stubRouteContext({ - browserManager: { isHealthy: async () => true, getConnectionMode: () => 'launched', getTabCount: () => 1 } as any, + browserManager: { + isHealthy: () => true, probeActiveTab: async () => ({ id: 1, responsive: true }), + getConnectionMode: () => 'launched', getTabCount: () => 1, + } as any, terminal: { readPort: () => 4242, grantToken: async () => true, restartSession: async () => true }, }); const text = await (await callRoute('GET', '/health', ctx)).text(); const body = JSON.parse(text); expect(body.terminalPort).toBe(4242); - expect(Object.keys(body).sort()).toEqual(['mode', 'status', 'tabs', 'terminalPort', 'uptime']); + expect(Object.keys(body).sort()).toEqual(['activeTab', 'mode', 'status', 'tabs', 'terminalPort', 'uptime']); expect(text).not.toContain('ptyToken'); expect(text).not.toContain('gstack_pty'); });