diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 4ff86a04f..d9e8082cd 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -91,7 +91,7 @@ The server writes `.gstack/browse.json` (atomic write via tmp + rename, mode 0o6 { "pid": 12345, "port": 34567, "token": "uuid-v4", "startedAt": "...", "binaryVersion": "abc123" } ``` -The CLI reads this file to find the server. If the file is missing or the daemon process is dead, the CLI spawns a new server. A process that is alive but not answering `/health` is busy, not dead: the CLI probes for a bounded ~8s, then reports busy with a nonzero exit — only an explicit `--force-restart` kills a live daemon. Process liveness uses signal-0 (`isProcessAlive`, EPERM counts as alive) on every platform, with the health check (GET /health) as the responsiveness signal. Daemon stdout/stderr persists to `/.gstack/browse-daemon.log`. +The CLI reads this file to find the server. If the file is missing or the daemon process is dead, the CLI spawns a new server. A process that is alive but not answering `/health` is busy, not dead: the CLI probes for a bounded ~8s, then reports busy with a nonzero exit — only an explicit `--force-restart` kills a live daemon. Process liveness uses signal-0 (`isProcessAlive`, EPERM counts as alive) on every platform, with the health check (GET /health) as the responsiveness signal. Its `status` covers the daemon and its Chromium connection only; the active tab's main thread is reported separately (`activeTab.responsive`, a probe bounded to 1s), so one page stuck in a long script never makes the CLI refuse commands. Daemon stdout/stderr persists to `/.gstack/browse-daemon.log`. ### Port selection diff --git a/BROWSER.md b/BROWSER.md index 396bb6e9b..3047da8f5 100644 --- a/BROWSER.md +++ b/BROWSER.md @@ -626,6 +626,14 @@ Storage stays intact by default. With explicit approval on a Chromium target, `- | `tab-each [args...]` | Fan out a command across every open tab; returns JSON | | `frame ` | Switch to iframe context (or back to main); clears refs | +**A busy tab is not a busy daemon.** `/health` `status` is daemon liveness +(Chromium connected); the active tab's main thread is reported separately as +`activeTab: { id, responsive }`, probed for at most 1s. While one page is stuck +in a long script, `status`, `tabs`, `tab `, `newtab`, `closetab`, +`state save` and `stop` keep working, and `status` and `tabs` mark the tab as +not responding. A command that needs the stuck tab times out after 30s and +names it, so you can switch away or close it without `--force-restart`. + ### Extraction | Command | Description | diff --git a/CHANGELOG.md b/CHANGELOG.md index 74a29736d..b6fc8e044 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## [1.91.155.0] - 2026-10-19 + +**A busy page no longer locks up the browse daemon.** When one tab kept its main thread busy (a heavy dev bundle, a big grid render, a runaway script), every `browse` command, including `status`, `tabs`, `tab `, `closetab` and `stop`, failed with "Daemon busy" after about 9.6 s, and the only way out was `--force-restart`, which threw away cookies, tabs and logins. Fixes #3247. From #3250 by @AndreIntelas. + +### Fixed +- **`/health` `status` reports daemon liveness (Chromium connected)**, and the active tab's responsiveness is reported separately as `activeTab: {id, responsive}`. The CLI no longer gates every command on a 2 s `page.evaluate` race against the active tab. +- **`tabs` and `state save` bound their per-tab reads**, so a stuck tab shows as `(not responding)` instead of hanging the command, and one pending probe is shared per stuck tab rather than one per poll. A busy tab keeps its last good storage in saved state. +- **A command that really needs the stuck tab times out with an error naming that tab** and how to get out (switch tabs or close it). The CLI's timeout branch now matches Bun's `TimeoutError`; the old `AbortError` check never fired. + +### Changed +- **Gate integrity:** `browse/test/terminal-agent.test.ts` adds `activeTab` to the exact `/health` keys and updates its stub for the now-synchronous `isHealthy` and the new `probeActiveTab`; its no-token assertions are unchanged. `browse/test/busy-active-tab.test.ts` and `browse/test/command-timeout-report.test.ts` are new. No threshold, trial count or case kind changed. ## [1.91.154.0] - 2026-10-19 **/cso can verify Rails repairs in a read-only checkout, its retained Git history no longer brings back credential files the snapshot leaves out, a repair whose external assertions fail is now reported as `ASSERTION_FAILED` instead of a misleading `INCOMPATIBLE_INPUT`, and a security assertion can accept several safe rejection statuses while still requiring the secret to be absent.** The helper keeps `.env`, `master.key`, keys and agent configuration out of the readable tree, but `history.txt` captured `git log -p` in full, so a committed `.env` came back through its diff whenever the value wasn't credential-shaped enough for the redactor. Paid qualification run 39186328569 caught this when the private corpus's planted `.env` value showed up in the judge's helper state. diff --git a/VERSION b/VERSION index 7836617d0..29c1e5ddc 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.154.0 +1.91.155.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 4222e853e..4e3b32732 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.154.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.155.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 338c6dd6b..7ad480813 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -290,6 +290,44 @@ 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; + +/** + * Budget for saveState()'s per-tab storage read. Longer than the probe: a tab + * that misses it falls back to the last storage it answered with, which may + * be stale, so only a tab that is really stuck takes that path. + */ +const TAB_STORAGE_READ_TIMEOUT_MS = 5_000; + +type TabStorage = { localStorage: Record; sessionStorage: Record }; +type TabReadKind = 'probe' | 'title' | 'storage'; + +function urlOrigin(url: string): string { + return URL.canParse(url) ? new URL(url).origin : ''; +} + +/** + * 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 @@ -300,7 +338,7 @@ export interface BrowserState { pages: Array<{ url: string; isActive: boolean; - storage: { localStorage: Record; sessionStorage: Record } | null; + storage: TabStorage | null; /** * HTML content loaded via load-html (setContent), replayed after context recreation. * In-memory only — never persisted to disk (HTML may contain secrets or customer data). @@ -360,6 +398,13 @@ export class BrowserManager { private extensionContext = false; /** Pending and recorded navigation-guard verdicts per page (D2). */ private navigationGuards = new WeakMap>; blocked: string | null }>(); + /** One pending read per page and kind (#3247). A read that outlives its + * budget stays queued in the renderer until the page frees up, so a stuck + * tab reuses it instead of collecting a new one on every /health poll, + * `tabs` call or persist tick. */ + private pendingTabReads = new WeakMap>>(); + /** The last storage each page answered with, and the origin it came from. */ + private lastTabStorage = new WeakMap(); // Our own 'disconnected' listeners per browser. Detach only these: removing // every 'disconnected' listener also strips Playwright's internal one that // resolves browser.close(), so each close waited the full race timeout (and @@ -1090,20 +1135,43 @@ 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 probe = this.sharedTabRead(page, 'probe', () => page.evaluate('1').then(() => true, () => false)); + return { id, responsive: (await withinTabBudget(probe, timeoutMs)) === true }; + } + + /** Join the page's pending read of this kind, or start one. `read` must not + * reject. */ + private sharedTabRead(page: Page, kind: TabReadKind, read: () => Promise): Promise { + let reads = this.pendingTabReads.get(page); + if (!reads) { + reads = new Map(); + this.pendingTabReads.set(page, reads); } + const pending = reads.get(kind); + if (pending) return pending as Promise; + const started = read().finally(() => reads.delete(kind)); + reads.set(kind, started); + return started; } // ─── Tab Management ──────────────────────────────────────── @@ -1301,17 +1369,23 @@ export class BrowserManager { return released; } - async getTabListWithTitles(): Promise> { - const tabs: Array<{ id: number; url: string; title: string; active: boolean }> = []; - for (const [id, page] of this.pages) { - tabs.push({ + /** + * Every tab with its title. Titles are read in parallel, each bounded by + * TAB_PROBE_TIMEOUT_MS: a tab whose renderer is stuck comes back with + * `busy: true` and an empty title instead of hanging the list (#3247). + */ + async getTabListWithTitles(): Promise> { + return Promise.all([...this.pages].map(async ([id, page]) => { + const title = await withinTabBudget( + this.sharedTabRead(page, 'title', () => page.title().catch(() => '')), TAB_PROBE_TIMEOUT_MS); + return { id, url: page.url(), - title: await page.title().catch(() => ''), + title: title ?? '', active: id === this.activeTabId, - }); - } - return tabs; + busy: title === undefined, + }; + })); } // ─── Session Access ──────────────────────────────────────── @@ -1715,7 +1789,9 @@ export class BrowserManager { // ─── State Save/Restore (shared by recreateContext + handoff) ─ /** * Capture browser state: cookies, localStorage, sessionStorage, URLs, active tab. - * Skips pages that fail storage reads (e.g., already closed). + * Skips the storage of pages that fail the read (e.g., already closed). + * A page whose renderer is stuck past TAB_STORAGE_READ_TIMEOUT_MS does not + * hold up `state save` (#3247); see readTabStorage() for what it gets. */ async saveState(): Promise { if (!this.context) throw new Error('Browser not launched'); @@ -1725,13 +1801,7 @@ export class BrowserManager { for (const [id, page] of this.pages) { const url = page.url(); - let storage = null; - try { - storage = await page.evaluate(() => ({ - localStorage: { ...localStorage }, - sessionStorage: { ...sessionStorage }, - })); - } catch {} + const storage = await this.readTabStorage(page, url); // Capture load-html content so a later context recreation (viewport --scale) // can replay it via setTabContent. Never persisted to disk. @@ -1753,6 +1823,28 @@ export class BrowserManager { return { cookies, pages }; } + /** + * One tab's localStorage/sessionStorage for saveState(). A failed read + * (closed page, opaque origin) is null. A read that times out returns the + * last storage this page gave for the same origin: a periodic + * BROWSE_PERSIST_STATE snapshot must not replace good storage on disk with + * null just because the page is busy. + */ + private async readTabStorage(page: Page, url: string): Promise { + const origin = urlOrigin(url); + const read = await withinTabBudget(this.sharedTabRead(page, 'storage', () => page.evaluate(() => ({ + localStorage: { ...localStorage }, + sessionStorage: { ...sessionStorage }, + })).catch(() => null)), TAB_STORAGE_READ_TIMEOUT_MS); + if (read === undefined) { + const last = this.lastTabStorage.get(page); + return last?.origin === origin ? last.storage : null; + } + if (read) this.lastTabStorage.set(page, { origin, storage: read }); + else this.lastTabStorage.delete(page); + return read; + } + /** * Restore browser state into the current context: cookies, pages, storage. * Navigates to saved URLs, restores storage, wires page events. diff --git a/browse/src/cli.ts b/browse/src/cli.ts index d774d23d5..556947f0d 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -215,39 +215,50 @@ 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; +} + +/** The active tab's id when /health reports its main thread as not + * responding (#3247), else null — including daemons that predate the field. */ +async function fetchBusyActiveTab(port: number): Promise { + const activeTab = (await fetchDaemonHealth(port))?.activeTab as { id?: unknown; responsive?: unknown } | null | undefined; + return activeTab?.responsive === false && typeof activeTab.id === 'number' ? activeTab.id : null; +} + // ─── Process Management ───────────────────────────────────────── async function killServer(pid: number): Promise { if (!isProcessAlive(pid)) return; @@ -420,7 +431,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; } } @@ -1034,6 +1045,32 @@ export function extractTabId(args: string[]): { tabId: number | undefined; args: return { tabId, args: stripped }; } +/** Did this fetch fail because its AbortSignal fired? Bun rejects an + * AbortSignal.timeout() fetch with TimeoutError, not AbortError — matching + * only AbortError left the timeout reports below unreachable. */ +export function isFetchTimeout(err: unknown): boolean { + const name = (err as { name?: unknown } | null)?.name; + return name === 'AbortError' || name === 'TimeoutError'; +} + +/** + * What to print when a command's reply did not arrive within `timeoutMs`. + * #1781: a timeout on a heavy page usually means busy, not dead, so a live + * daemon is never restarted here. #3247: when /health names the active tab + * as not responding, say which tab and the commands that leave it without + * losing the session. + */ +export async function commandTimeoutReport(state: { pid: number; port: number } | null, timeoutMs: number): Promise { + const after = `[browse] Command timed out after ${timeoutMs / 1000}s`; + if (!state?.pid || !isProcessAlive(state.pid)) return [after]; + const busyTab = await fetchBusyActiveTab(state.port); + if (busyTab === null) return [`${after} (server still alive — busy, not restarting). Retry, or raise load.`]; + return [ + `${after}: tab ${busyTab} is not responding (its main thread is busy).`, + `[browse] The daemon, its other tabs and cookies still work: switch with 'browse tab ', close this one with 'browse closetab ${busyTab}', or retry once the page settles.`, + ]; +} + // ─── Command Dispatch ────────────────────────────────────────── export async function sendCommand(state: ServerState, command: string, args: string[], retries = 0): Promise { // Precedence: CLI --tab-id flag > BROWSE_TAB env var. @@ -1085,19 +1122,14 @@ export async function sendCommand(state: ServerState, command: string, args: str process.exit(1); } } catch (err: any) { - if (command === 'cookie-import-browser' && (['AbortError', 'TimeoutError'].includes(err.name) + if (command === 'cookie-import-browser' && (isFetchTimeout(err) || ['ECONNREFUSED', 'ECONNRESET'].includes(err.code) || err.message?.includes('fetch failed'))) { throw new Error('Cookie import response was lost or timed out. It may have partially completed; inspect the destination before retrying manually. The command was not replayed.'); } - if (err.name === 'AbortError') { - // #1781: a 30s timeout on a heavy page usually means busy, not dead. - // Don't kill a live server (that's what triggered the crash-loop) — report - // and exit so the user can retry rather than losing their (headed) window. - const ts = readState(); - const alive = ts?.pid ? isProcessAlive(ts.pid) : false; - console.error(alive - ? '[browse] Command timed out after 30s (server still alive — busy, not restarting). Retry, or raise load.' - : '[browse] Command timed out after 30s'); + if (isFetchTimeout(err)) { + // Report and exit so the user can retry rather than losing their + // (headed) window — killing a live server triggered the crash-loop. + for (const line of await commandTimeoutReport(readState(), timeoutMs)) console.error(line); process.exit(1); } // Connection error — server may have crashed, OR may just be busy. diff --git a/browse/src/meta-commands.ts b/browse/src/meta-commands.ts index 8cc9e2e6c..af49910a9 100644 --- a/browse/src/meta-commands.ts +++ b/browse/src/meta-commands.ts @@ -265,7 +265,7 @@ export async function handleMetaCommand( case 'tabs': { const tabs = await bm.getTabListWithTitles(); return tabs.map(t => - `${t.active ? '→ ' : ' '}[${t.id}] ${t.title || '(untitled)'} — ${t.url}` + `${t.active ? '→ ' : ' '}[${t.id}] ${t.busy ? '(not responding)' : t.title || '(untitled)'} — ${t.url}` ).join('\n'); } @@ -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/busy-active-tab.test.ts b/browse/test/busy-active-tab.test.ts new file mode 100644 index 000000000..04a15861b --- /dev/null +++ b/browse/test/busy-active-tab.test.ts @@ -0,0 +1,132 @@ +/** + * #3247: one busy tab must not make the daemon look dead. A renderer stuck in + * a long script never answers page.evaluate() or page.title(). /health folded + * that into its `status`, so the CLI refused every command (status, tabs, + * tab, newtab, stop) with "Daemon busy" until --force-restart threw the + * session away, and `tabs` and `state save` hung on the tab as well. + * + * A real Chromium tab is held in a busy loop while a second tab stays idle; + * each surface must answer within its bound and name the busy tab, without + * piling up reads on the stuck renderer or dropping its persisted storage. + */ +import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import type { Page } from 'playwright'; +import { BrowserManager, TAB_PROBE_TIMEOUT_MS } from '../src/browser-manager'; +import { HEALTH_REQUEST_TIMEOUT_MS } from '../src/cli'; +import { handleMetaCommand } from '../src/meta-commands'; +import { persistSessionState } from '../src/session-persist'; +import { callRoute, stubRouteContext } from './route-test-harness'; +import { startTestServer } from './test-server'; + +// Outlasts every check below; closing the busy tab at the end stops it sooner. +const BUSY_MS = 30_000; + +let testServer: ReturnType; +let bm: BrowserManager; +let busyId = 0; +let idleId = 0; +let busyPage: Page; +let profileDir = ''; +const originalProfile = process.env.CHROMIUM_PROFILE; + +beforeAll(async () => { + // Per-file profile, set in a hook (see commands.test.ts for why). + profileDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-busy-tab-profile-')); + process.env.CHROMIUM_PROFILE = profileDir; + testServer = startTestServer(0); + bm = new BrowserManager(); + await bm.launch(); + busyId = bm.getActiveTabId(); + await bm.getPage().goto(`${testServer.url}/basic.html`); + idleId = await bm.newTab(`${testServer.url}/basic.html`); + bm.switchTab(busyId, { bringToFront: false }); + busyPage = bm.getPage(); + // The good snapshot a BROWSE_PERSIST_STATE tick takes before the page hangs. + await busyPage.evaluate(() => localStorage.setItem('probe', 'kept')); + await bm.saveState(); + await busyPage.evaluate((ms) => { + setTimeout(() => { const end = Date.now() + ms; while (Date.now() < end) { /* busy */ } }, 50); + }, BUSY_MS); + await Bun.sleep(300); // let the loop start +}, 30_000); + +afterAll(async () => { + try { testServer.server.stop(true); } catch {} + try { await Promise.race([bm?.close(), Bun.sleep(3000)]); } catch {} + if (originalProfile === undefined) delete process.env.CHROMIUM_PROFILE; + else process.env.CHROMIUM_PROFILE = originalProfile; + if (profileDir) fs.rmSync(profileDir, { recursive: true, force: true }); +}); + +describe('busy active tab (#3247)', () => { + test('/health says the daemon is alive and names the busy tab before the CLI gives up', async () => { + // Equal budgets were the bug: the CLI aborted just before the reply landed. + expect(TAB_PROBE_TIMEOUT_MS).toBeLessThan(HEALTH_REQUEST_TIMEOUT_MS); + const ctx = stubRouteContext({ + browserManager: bm, + terminal: { readPort: () => null, grantToken: async () => true, restartSession: async () => true }, + }); + const started = Date.now(); + const body = await (await callRoute('GET', '/health', ctx)).json(); + expect(Date.now() - started).toBeLessThan(HEALTH_REQUEST_TIMEOUT_MS); + expect(body.status).toBe('healthy'); + expect(body.activeTab).toEqual({ id: busyId, responsive: false }); + }); + + test('tabs lists every tab without waiting on the busy one', async () => { + const started = Date.now(); + const out = await handleMetaCommand('tabs', [], bm, async () => {}); + expect(Date.now() - started).toBeLessThan(HEALTH_REQUEST_TIMEOUT_MS); + expect(out).toContain(`[${busyId}] (not responding)`); + expect(out).toContain(`[${idleId}] Test Page - Basic`); + }); + + test('status stays healthy and names the busy tab with a way out', async () => { + const out = await handleMetaCommand('status', [], bm, async () => {}); + expect(out).toContain('Status: healthy'); + expect(out).toContain(`Active tab ${busyId}`); + expect(out).toContain(`closetab ${busyId}`); + }); + + test('a stuck tab gets one pending probe, not one per poll', async () => { + let evaluates = 0; + const evaluate = busyPage.evaluate.bind(busyPage); + Object.assign(busyPage, { evaluate: (...args: Parameters) => { evaluates++; return evaluate(...args); } }); + try { + for (let i = 0; i < 3; i++) expect(await bm.probeActiveTab()).toEqual({ id: busyId, responsive: false }); + } finally { + delete (busyPage as { evaluate?: unknown }).evaluate; // back to the prototype method + } + // The /health check above may already hold the pending probe. + expect(evaluates).toBeLessThanOrEqual(1); + }, 10_000); + + test('a persisted snapshot taken while the tab is busy keeps its last good storage', async () => { + const file = path.join(profileDir, 'session-state.json'); + await persistSessionState(bm, file); + const saved = JSON.parse(fs.readFileSync(file, 'utf8')); + expect(saved.pages).toHaveLength(2); + expect(saved.pages.find((p: { isActive: boolean }) => p.isActive).storage.localStorage).toEqual({ probe: 'kept' }); + expect(saved.pages.find((p: { isActive: boolean }) => !p.isActive).storage).not.toBeNull(); + }, 15_000); + + test('the idle tab still answers', async () => { + bm.switchTab(idleId, { bringToFront: false }); + try { + expect(await bm.probeActiveTab()).toEqual({ id: idleId, responsive: true }); + expect(await bm.getPage().title()).toBe('Test Page - Basic'); + } finally { + bm.switchTab(busyId, { bringToFront: false }); + } + }); + + test('closetab closes the busy tab and the session carries on', async () => { + const out = await handleMetaCommand('closetab', [String(busyId)], bm, async () => {}); + expect(out).toContain(`Closed tab ${busyId}`); + expect(bm.getTabCount()).toBe(1); + expect(await bm.probeActiveTab()).toEqual({ id: idleId, responsive: true }); + }, 15_000); +}); diff --git a/browse/test/command-timeout-report.test.ts b/browse/test/command-timeout-report.test.ts new file mode 100644 index 000000000..0505c0737 --- /dev/null +++ b/browse/test/command-timeout-report.test.ts @@ -0,0 +1,66 @@ +/** + * What the CLI prints when a command's reply never arrives (#1781, #3247). + * + * Bun rejects an AbortSignal.timeout() fetch with TimeoutError, so the old + * AbortError-only check skipped the busy report and printed "The operation + * timed out." instead. When the daemon's /health names its active tab as not + * responding, the report names that tab and the commands that leave it + * without a restart; a dead daemon gets the plain line. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import { commandTimeoutReport, isFetchTimeout } from '../src/cli'; + +let servers: Array> = []; +afterEach(() => { for (const s of servers) s.stop(true); servers = []; }); + +/** A fake daemon whose /health answers `body`. */ +function fakeHealth(body: unknown): number { + const srv = Bun.serve({ port: 0, hostname: '127.0.0.1', fetch: () => Response.json(body) }); + servers.push(srv); + if (!srv.port) throw new Error('fake /health: no port'); + return srv.port; +} + +/** A pid that no longer runs: a child that has already exited. */ +function deadPid(): number { + return Bun.spawnSync([process.execPath, '-e', '']).pid; +} + +describe('isFetchTimeout', () => { + test('a real fetch whose AbortSignal.timeout fires counts as a timeout', async () => { + const silent = Bun.serve({ port: 0, hostname: '127.0.0.1', fetch: () => new Promise(() => {}) }); + servers.push(silent); + const err = await fetch(`http://127.0.0.1:${silent.port}/`, { signal: AbortSignal.timeout(50) }) + .then(() => null, (e: unknown) => e); + expect(isFetchTimeout(err)).toBe(true); + expect(isFetchTimeout(new TypeError('fetch failed'))).toBe(false); + }); +}); + +describe('commandTimeoutReport', () => { + test('names the busy tab and the way out, never a restart', async () => { + const port = fakeHealth({ status: 'healthy', activeTab: { id: 7, responsive: false } }); + const out = (await commandTimeoutReport({ pid: process.pid, port }, 30_000)).join('\n'); + expect(out).toContain('tab 7'); + expect(out).toContain('closetab 7'); + expect(out).not.toContain('--force-restart'); + }); + + test('a live daemon with a responsive or unreported tab gets the busy-not-restarting line', async () => { + for (const body of [ + { status: 'healthy', activeTab: { id: 1, responsive: true } }, + { status: 'healthy' }, // daemon from before activeTab existed + ]) { + const out = (await commandTimeoutReport({ pid: process.pid, port: fakeHealth(body) }, 30_000)).join('\n'); + expect(out).toContain('still alive'); + expect(out).not.toContain('closetab'); + } + }); + + test('a dead daemon gets only the timeout line', async () => { + const port = fakeHealth({ status: 'healthy', activeTab: { id: 7, responsive: false } }); + const out = await commandTimeoutReport({ pid: deadPid(), port }, 30_000); + expect(out).toHaveLength(1); + expect(out[0]).toContain('timed out after 30s'); + }); +}); 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'); }); diff --git a/docs/REMOTE_BROWSER_ACCESS.md b/docs/REMOTE_BROWSER_ACCESS.md index c5565ba0e..902df6efb 100644 --- a/docs/REMOTE_BROWSER_ACCESS.md +++ b/docs/REMOTE_BROWSER_ACCESS.md @@ -81,7 +81,9 @@ Response: (plain text result of the command) ``` #### GET /health -Server status. No auth required. Returns status, tabs, mode, uptime. +Server status. No auth required. Returns status, activeTab, tabs, mode, uptime. +`status` is daemon liveness; `activeTab` (`{ id, responsive }`, or null with no +page) says whether the active tab's main thread answered within 1s. Never carries a token — extension token bootstrap is `POST /extension-token` (local listener only, validates the pinned `chrome-extension://` Origin and a loopback Host; 403 otherwise). Not reachable over the tunnel (404). diff --git a/package.json b/package.json index e11883f58..de7b7c9d4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.154", + "version": "1.91.155", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module",