From bb4de6a00d61d2642eaedbb71679586ae4f1e0ae Mon Sep 17 00:00:00 2001 From: Matthew Nanas <37747855+matthewnanas@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:35:04 -0800 Subject: [PATCH 1/8] Require the rollout flag before personal-browser mode. A leftover extension upload id was enough to change hosted launches and disable reusable Contexts. Co-authored-by: Cursor --- .env.example | 2 +- docs/onepassword.md | 4 ++-- server/onepassword/browser-policy.ts | 5 ++++- tests/onepassword-availability.test.ts | 6 +++++- tests/onepassword-callback.test.ts | 1 + 5 files changed, 13 insertions(+), 5 deletions(-) diff --git a/.env.example b/.env.example index cf777a8..09f3a92 100644 --- a/.env.example +++ b/.env.example @@ -35,7 +35,7 @@ ONEPASSWORD_CLIENT_ID= ONEPASSWORD_CLIENT_SECRET= # Generate once with openssl rand -base64 32; keep stable and backed up. ONEPASSWORD_TOKEN_ENCRYPTION_KEY= -# Leave unset until hosted extension support is fixed. Setting this disables reusable Contexts. +# Leave unset until hosted extension support is fixed. Personal-browser mode also requires ONEPASSWORD_ENABLED=true. BROWSERBASE_1PASSWORD_EXTENSION_ID= # Optional Linq messaging. Keep tokens and your personal number in .env.local. LINQ_API_KEY= diff --git a/docs/onepassword.md b/docs/onepassword.md index d2df52e..a663d8e 100644 --- a/docs/onepassword.md +++ b/docs/onepassword.md @@ -28,14 +28,14 @@ Before changing the flag to `true`, the platform and SDK must support loading bo | `ONEPASSWORD_TOKEN_ENCRYPTION_KEY` | Stable 32-byte base64 key; generate with `openssl rand -base64 32`. Back it up with the database and do not rotate by overwriting it. | | `BROWSERBASE_API_KEY` | Browserbase access. | | `BROWSERBASE_PROJECT_ID` | Optional when the project is inferred from the API key. | - | `BROWSERBASE_1PASSWORD_EXTENSION_ID` | Browserbase upload ID for the supported Agentic Autofill extension. This enables personal-browser mode. | + | `BROWSERBASE_1PASSWORD_EXTENSION_ID` | Browserbase upload ID for the supported Agentic Autofill extension. Personal-browser mode also requires `ONEPASSWORD_ENABLED=true`. | 3. Register the OAuth client through a 1Password Business account. The connecting test user needs a separate US Individual or Families account and the supported desktop app/extension. Follow the current [Browserbase setup guide](https://docs.browserbase.com/integrations/1password/agentic-autofill) for partner access and extension packaging. A regular service-account token is not a replacement for this client or extension. 4. Keep the public callback and `/eve/v1/connections/.../callback/...` paths reachable through Vercel protection without adding query parameters to the registered redirect. The callbacks use server-issued, short-lived capabilities; do not make unrelated application routes public. 5. Keep Linq signature verification, receiving-line configuration, and the sender allowlist enabled. Groups are deliberately rejected. The web operator login does not establish an individual iMessage identity and cannot select someone else's personal account. 6. Start new conversations for the rollout. Remove `BROWSERBASE_CONTEXT_ID` when personal-browser mode is enabled. Bell disables reusable Context creation, selection, saving, and cookie export in this mode. Existing contexts are not deleted, and their cookies are not migrated into personal sessions. Disable the old `ONEPASSWORD_OAUTH_STATE_SECRET`; the new record encryption key replaces its use. -Setting the extension upload ID changes browser launch configuration. Stagehand 4 also needs its own extension. Bell requires Stagehand initialization and the 1Password runtime API to succeed in the same session; failures stop the operation and do not fall back to reading passwords through the SDK. The local synthetic test verifies coexistence in Chrome. Verify the actual hosted extension combination before enabling this on a public deployment; an upload ID alone is not proof of compatibility. +Personal-browser mode requires `ONEPASSWORD_ENABLED=true` and the extension upload ID together. That pair changes browser launch configuration. Stagehand 4 also needs its own extension. Bell requires Stagehand initialization and the 1Password runtime API to succeed in the same session; failures stop the operation and do not fall back to reading passwords through the SDK. The local synthetic test verifies coexistence in Chrome. Verify the actual hosted extension combination before enabling this on a public deployment; an upload ID alone is not proof of compatibility. ## User flow diff --git a/server/onepassword/browser-policy.ts b/server/onepassword/browser-policy.ts index ada2408..1ef4782 100644 --- a/server/onepassword/browser-policy.ts +++ b/server/onepassword/browser-policy.ts @@ -1,5 +1,6 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { Pool, type PoolClient } from "pg"; +import { personalConnectionsEnabled } from "./config"; const activeLock = new AsyncLocalStorage<{ client: PoolClient; sessionId: string }>(); export async function markSecretFill(blocked: boolean): Promise { const lock = activeLock.getStore(); @@ -7,7 +8,9 @@ export async function markSecretFill(blocked: boolean): Promise { if (blocked) await lock.client.query("INSERT INTO bell_onepassword_browser_guards(session_id) VALUES($1) ON CONFLICT DO NOTHING", [lock.sessionId]); else await lock.client.query("DELETE FROM bell_onepassword_browser_guards WHERE session_id=$1", [lock.sessionId]); } -export function personalBrowserEnabled(): boolean { return Boolean(process.env.BROWSERBASE_1PASSWORD_EXTENSION_ID?.trim()); } +export function personalBrowserEnabled(): boolean { + return personalConnectionsEnabled() && Boolean(process.env.BROWSERBASE_1PASSWORD_EXTENSION_ID?.trim()); +} export function requireReusableContexts(): void { if (personalBrowserEnabled()) throw new Error("Reusable browser Contexts are disabled for personal 1Password. Start a new private conversation instead."); } diff --git a/tests/onepassword-availability.test.ts b/tests/onepassword-availability.test.ts index 4477012..c8b0c97 100644 --- a/tests/onepassword-availability.test.ts +++ b/tests/onepassword-availability.test.ts @@ -52,12 +52,16 @@ describe("personal 1Password rollout", () => { expect(response.status).toBe(200); expect(capture).toHaveBeenCalledOnce(); }); - it("preserves ordinary browsing and private-context protections", () => { + it("keeps ordinary browsing until the rollout flag and extension id are both set", () => { const options = browserbaseLaunchOptions("test-key"); expect(options.extensionId).toBeUndefined(); expect(options.browserSettings).toMatchObject({ logSession: false, recordSession: false }); expect(requireReusableContexts).not.toThrow(); vi.stubEnv("BROWSERBASE_1PASSWORD_EXTENSION_ID", "previous-private-extension"); + expect(browserbaseLaunchOptions("test-key").extensionId).toBeUndefined(); + expect(requireReusableContexts).not.toThrow(); + vi.stubEnv("ONEPASSWORD_ENABLED", "true"); + expect(browserbaseLaunchOptions("test-key").extensionId).toBe("previous-private-extension"); expect(requireReusableContexts).toThrow(); }); }); diff --git a/tests/onepassword-callback.test.ts b/tests/onepassword-callback.test.ts index 5ef61c2..d2fa089 100644 --- a/tests/onepassword-callback.test.ts +++ b/tests/onepassword-callback.test.ts @@ -40,6 +40,7 @@ describe("personal 1Password boundaries", () => { expect(capture).not.toHaveBeenCalled(); }); it("uses private non-recorded browsers and rejects reusable Contexts in personal mode", () => { + vi.stubEnv("ONEPASSWORD_ENABLED", "true"); vi.stubEnv("BROWSERBASE_1PASSWORD_EXTENSION_ID", "uploaded-extension"); vi.stubEnv("BROWSERBASE_CONTEXT_ID", ""); const options = browserbaseLaunchOptions("test-key"); From 661b546a41027f59a4753002985587885dab92cc Mon Sep 17 00:00:00 2001 From: Matthew Nanas <37747855+matthewnanas@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:38:26 -0800 Subject: [PATCH 2/8] Share one advisory-lock helper across database callers. Link, 1Password, and the browser locks each owned a slightly different unlock path, so a failed lock could return a broken connection to the pool. Co-authored-by: Cursor --- server/browser/privacy-lock.ts | 24 +++--------- server/link/store.ts | 28 +++----------- server/monitors/store.ts | 5 +-- server/onepassword/browser-policy.ts | 24 ++++-------- server/onepassword/oauth.ts | 6 +-- server/onepassword/store.ts | 15 ++------ server/postgres.ts | 35 ++++++++++++++++++ tests/postgres.test.ts | 55 ++++++++++++++++++++++++++++ 8 files changed, 115 insertions(+), 77 deletions(-) create mode 100644 server/postgres.ts create mode 100644 tests/postgres.test.ts diff --git a/server/browser/privacy-lock.ts b/server/browser/privacy-lock.ts index f53ca7d..e90320f 100644 --- a/server/browser/privacy-lock.ts +++ b/server/browser/privacy-lock.ts @@ -1,6 +1,7 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { Pool } from "pg"; import { isLinkSandbox } from "../link/sandbox"; +import { createPool, withAdvisoryLock } from "../postgres"; const current = new AsyncLocalStorage<() => Promise>(); const sandboxBrowsers = new Map; quarantined: boolean }>(); @@ -29,27 +30,12 @@ export async function withBrowserPrivacyLock(sessionId: string, operation: () } if (!database && !process.env.LINK_CLIENT_ID?.trim()) return operation(); if (!database && !process.env.DATABASE_URL) throw new Error("Link checkout requires DATABASE_URL."); - if (!database && !pool) { - pool = new Pool({ connectionString: process.env.DATABASE_URL, max: 5, connectionTimeoutMillis: 10_000, idleTimeoutMillis: 30_000, allowExitOnIdle: true }); - pool.on("error", () => console.error("Browser privacy database connection failed.")); - } - const client = await (database ?? pool!).connect(); - let locked = false, destroy = false; - const key = `bell-browser-privacy:${sessionId}`; - try { - await client.query("SET statement_timeout='30s'"); - await client.query("SELECT pg_advisory_lock(hashtextextended($1,0))", [key]); - locked = true; + if (!database && !pool) pool = createPool(process.env.DATABASE_URL!, "Browser privacy", { allowExitOnIdle: true }); + return withAdvisoryLock(database ?? pool!, `bell-browser-privacy:${sessionId}`, async (client) => { if ((await client.query("SELECT 1 FROM bell_browser_privacy_guards WHERE session_id=$1", [sessionId])).rowCount) throw new Error("Payment details may remain in this browser. Use human handoff to inspect the existing purchase; do not retry it. Start a new conversation for further browser automation."); - return await current.run(async () => { + return current.run(async () => { await client.query("INSERT INTO bell_browser_privacy_guards(session_id) VALUES($1) ON CONFLICT DO NOTHING", [sessionId]); }, operation); - } finally { - if (locked) { - try { await client.query("SELECT pg_advisory_unlock(hashtextextended($1,0))", [key]); } - catch { destroy = true; } - } else destroy = true; - client.release(destroy); - } + }); } diff --git a/server/link/store.ts b/server/link/store.ts index 4702906..806b0de 100644 --- a/server/link/store.ts +++ b/server/link/store.ts @@ -1,4 +1,5 @@ import { Pool, type PoolClient } from "pg"; +import { createPool, withAdvisoryLock } from "../postgres"; export interface LinkOwner { issuer: string; id: string } export interface Grant { @@ -102,30 +103,13 @@ export class LinkStore { } async withOwner(owner: LinkOwner, operation: (store: OwnerStore) => Promise): Promise { - const client = await this.pool.connect(); - let locked = false; - let destroy = false; - try { - // A session lock spans independently committed writes around remote OAuth calls. - // In-progress markers must survive process failure; a transaction rollback would - // otherwise allow a single-use authorization code or refresh token to be retried. - await client.query("SET statement_timeout = '30s'"); - await client.query("SELECT pg_advisory_lock(hashtextextended($1, 0))", [ownerKey(owner)]); - locked = true; - return await operation(new OwnerStore(client, owner)); - } finally { - if (locked) { - try { - await client.query("SELECT pg_advisory_unlock(hashtextextended($1, 0))", [ownerKey(owner)]); - } catch { destroy = true; } - } else { destroy = true; } - client.release(destroy); - } + // A session lock spans independently committed writes around remote OAuth calls. + // In-progress markers must survive process failure; a transaction rollback would + // otherwise allow a single-use authorization code or refresh token to be retried. + return withAdvisoryLock(this.pool, ownerKey(owner), (client) => operation(new OwnerStore(client, owner))); } } export function createLinkStore(databaseUrl: string): LinkStore { - const pool = new Pool({ connectionString: databaseUrl, max: 5, connectionTimeoutMillis: 10_000, idleTimeoutMillis: 30_000 }); - pool.on("error", () => console.error("Link database connection failed.")); - return new LinkStore(pool); + return new LinkStore(createPool(databaseUrl, "Link")); } diff --git a/server/monitors/store.ts b/server/monitors/store.ts index d1db9dc..ecb1790 100644 --- a/server/monitors/store.ts +++ b/server/monitors/store.ts @@ -1,5 +1,6 @@ import { randomUUID } from "node:crypto"; import { Pool, type PoolClient } from "pg"; +import { createPool } from "../postgres"; import { z } from "zod"; import { type Destination, type Owner, required } from "./config"; @@ -154,9 +155,7 @@ export class MonitorStore { let singleton: MonitorStore | undefined; export function monitorStore(): MonitorStore { if (!singleton) { - const pool = new Pool({ connectionString: required("DATABASE_URL"), max: 5, connectionTimeoutMillis: 3_000, statement_timeout: 5_000 }); - pool.on("error", () => console.error("Monitor database connection failed.")); - singleton = new MonitorStore(pool); + singleton = new MonitorStore(createPool(required("DATABASE_URL"), "Monitor", { connectionTimeoutMillis: 3_000, statement_timeout: 5_000 })); } return singleton; } diff --git a/server/onepassword/browser-policy.ts b/server/onepassword/browser-policy.ts index 1ef4782..d74a068 100644 --- a/server/onepassword/browser-policy.ts +++ b/server/onepassword/browser-policy.ts @@ -1,6 +1,7 @@ import { AsyncLocalStorage } from "node:async_hooks"; -import { Pool, type PoolClient } from "pg"; +import { type PoolClient } from "pg"; import { personalConnectionsEnabled } from "./config"; +import { createPool, withAdvisoryLock } from "../postgres"; const activeLock = new AsyncLocalStorage<{ client: PoolClient; sessionId: string }>(); export async function markSecretFill(blocked: boolean): Promise { const lock = activeLock.getStore(); @@ -14,24 +15,13 @@ export function personalBrowserEnabled(): boolean { export function requireReusableContexts(): void { if (personalBrowserEnabled()) throw new Error("Reusable browser Contexts are disabled for personal 1Password. Start a new private conversation instead."); } -let pool: Pool | undefined; +let pool: ReturnType | undefined; export async function withPersonalBrowserLock(sessionId: string, operation: () => Promise): Promise { if (!personalBrowserEnabled()) return operation(); if (!process.env.DATABASE_URL) throw new Error("Personal 1Password requires DATABASE_URL."); - if (!pool) { - pool = new Pool({ connectionString: process.env.DATABASE_URL, max: 5, connectionTimeoutMillis: 10_000, idleTimeoutMillis: 30_000, allowExitOnIdle: true }); - pool.on("error", () => console.error("Private browser database connection failed.")); - } - const client = await pool.connect(); - let destroy = false; - try { - await client.query("SET statement_timeout='30s'"); - await client.query("SELECT pg_advisory_lock(hashtextextended($1,0))", [`bell-private-browser:${sessionId}`]); + pool ??= createPool(process.env.DATABASE_URL, "Private browser", { allowExitOnIdle: true }); + return withAdvisoryLock(pool, `bell-private-browser:${sessionId}`, async (client) => { if ((await client.query("SELECT 1 FROM bell_onepassword_browser_guards WHERE session_id=$1", [sessionId])).rowCount) throw new Error("A private login did not finish safely. Start a new conversation."); - return await activeLock.run({ client, sessionId }, operation); - } finally { - try { await client.query("SELECT pg_advisory_unlock(hashtextextended($1,0))", [`bell-private-browser:${sessionId}`]); } - catch { destroy = true; } - client.release(destroy); - } + return activeLock.run({ client, sessionId }, operation); + }); } diff --git a/server/onepassword/oauth.ts b/server/onepassword/oauth.ts index eef33ac..3740e00 100644 --- a/server/onepassword/oauth.ts +++ b/server/onepassword/oauth.ts @@ -1,8 +1,8 @@ import { randomBytes, randomUUID } from "node:crypto"; -import { Pool } from "pg"; import { ConnectionAuthorizationRequiredError, type ConnectionPrincipal } from "eve/connections"; import { z } from "zod"; import { hash } from "../encrypted-record"; +import { createPool } from "../postgres"; import { configuration, failed, frameworkCallback, integrationIdentity, ownerOf, scopes, type Config } from "./config"; import { OnePasswordStore, type ConnectionState, type OAuthAttempt, type RecordStore, type Tokens } from "./store"; @@ -183,9 +183,7 @@ let singleton: OnePasswordOAuth | undefined; export function getOnePassword(): OnePasswordOAuth { if (!singleton) { const config = configuration(); - const pool = new Pool({ connectionString: config.databaseUrl, max: 5, connectionTimeoutMillis: 10_000, idleTimeoutMillis: 30_000 }); - pool.on("error", () => console.error("1Password database connection failed.")); - singleton = new OnePasswordOAuth(config, new OnePasswordStore(pool, config.encryptionKey)); + singleton = new OnePasswordOAuth(config, new OnePasswordStore(createPool(config.databaseUrl, "1Password"), config.encryptionKey)); } return singleton; } diff --git a/server/onepassword/store.ts b/server/onepassword/store.ts index 8217fca..9e47dd6 100644 --- a/server/onepassword/store.ts +++ b/server/onepassword/store.ts @@ -1,4 +1,5 @@ import { Pool, type PoolClient } from "pg"; +import { withAdvisoryLock } from "../postgres"; import { seal, unseal } from "../encrypted-record"; import { failed } from "./config"; @@ -50,17 +51,7 @@ export class OnePasswordStore { return (await this.pool.query<{ owner: string }>("SELECT owner FROM bell_onepassword_tickets WHERE hash=$1 AND expires_at>now()", [hash])).rows[0]?.owner; } async withOwner(owner: string, operation: (db: RecordStore) => Promise): Promise { - const client = await this.pool.connect(); - let destroy = false; - try { - await client.query("SET statement_timeout='30s'"); - // The session lock spans committed crash markers around single-use token exchanges. - await client.query("SELECT pg_advisory_lock(hashtextextended($1,0))", [`onepassword:${owner}`]); - return await operation(new RecordStore(client, owner, this.key)); - } finally { - try { await client.query("SELECT pg_advisory_unlock(hashtextextended($1,0))", [`onepassword:${owner}`]); } - catch { destroy = true; } - client.release(destroy); - } + // The session lock spans committed crash markers around single-use token exchanges. + return withAdvisoryLock(this.pool, `onepassword:${owner}`, (client) => operation(new RecordStore(client, owner, this.key))); } } diff --git a/server/postgres.ts b/server/postgres.ts new file mode 100644 index 0000000..7531e15 --- /dev/null +++ b/server/postgres.ts @@ -0,0 +1,35 @@ +import { Pool, type PoolClient, type PoolConfig } from "pg"; + +export function createPool(connectionString: string, label: string, options: PoolConfig = {}): Pool { + const pool = new Pool({ + connectionString, + max: 5, + connectionTimeoutMillis: 10_000, + idleTimeoutMillis: 30_000, + ...options, + }); + pool.on("error", () => console.error(`${label} database connection failed.`)); + return pool; +} + +/** Hold a session advisory lock. A failed lock or unlock destroys the client instead of returning it to the pool. */ +export async function withAdvisoryLock(pool: Pool, key: string, operation: (client: PoolClient) => Promise): Promise { + const client = await pool.connect(); + let locked = false; + let destroy = false; + try { + await client.query("SET statement_timeout = '30s'"); + await client.query("SELECT pg_advisory_lock(hashtextextended($1, 0))", [key]); + locked = true; + return await operation(client); + } finally { + if (locked) { + try { + await client.query("SELECT pg_advisory_unlock(hashtextextended($1, 0))", [key]); + } catch { + destroy = true; + } + } else destroy = true; + client.release(destroy); + } +} diff --git a/tests/postgres.test.ts b/tests/postgres.test.ts new file mode 100644 index 0000000..1ed3ecf --- /dev/null +++ b/tests/postgres.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it, vi } from "vitest"; +import type { Pool, PoolClient } from "pg"; +import { withAdvisoryLock } from "../server/postgres"; + +function client(queries: Array<(sql: string) => unknown> = []) { + const query = vi.fn(async (sql: string) => { + const next = queries.shift(); + if (next) return next(sql); + return { rows: [], rowCount: 0 }; + }); + const release = vi.fn(); + return { query, release } as unknown as PoolClient & { query: ReturnType; release: ReturnType }; +} + +function poolFor(connection: PoolClient): Pool { + return { connect: async () => connection } as unknown as Pool; +} + +describe("advisory lock", () => { + it("runs the operation between the lock and unlock, then returns the client", async () => { + const connection = client(); + const result = await withAdvisoryLock(poolFor(connection), "owner", async () => "done"); + expect(result).toBe("done"); + expect(connection.query.mock.calls.map((call) => call[0])).toEqual([ + "SET statement_timeout = '30s'", + "SELECT pg_advisory_lock(hashtextextended($1, 0))", + "SELECT pg_advisory_unlock(hashtextextended($1, 0))", + ]); + expect(connection.query.mock.calls[1][1]).toEqual(["owner"]); + expect(connection.release).toHaveBeenCalledWith(false); + }); + + it("destroys the client when the lock is never acquired", async () => { + const connection = client([ + () => ({ rows: [] }), + () => { throw new Error("lock failed"); }, + ]); + await expect(withAdvisoryLock(poolFor(connection), "owner", async () => "done")).rejects.toThrow("lock failed"); + expect(connection.release).toHaveBeenCalledWith(true); + expect(connection.query.mock.calls.map((call) => call[0])).toEqual([ + "SET statement_timeout = '30s'", + "SELECT pg_advisory_lock(hashtextextended($1, 0))", + ]); + }); + + it("destroys the client when unlock fails after the operation", async () => { + const connection = client([ + () => ({ rows: [] }), + () => ({ rows: [] }), + () => { throw new Error("unlock failed"); }, + ]); + await expect(withAdvisoryLock(poolFor(connection), "owner", async () => "done")).resolves.toBe("done"); + expect(connection.release).toHaveBeenCalledWith(true); + }); +}); From 8723f6513c4bc85bbdec4776cd97eefe956266d1 Mon Sep 17 00:00:00 2001 From: Matthew Nanas <37747855+matthewnanas@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:40:17 -0800 Subject: [PATCH 3/8] Move watch SQL onto the monitor store. The service was a second writer for status, inbox, and scan updates, including a create-failure path that could commit only one of its two statements. Co-authored-by: Cursor --- server/monitors/service.ts | 31 +++++++++++---------------- server/monitors/store.ts | 43 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/server/monitors/service.ts b/server/monitors/service.ts index fd22445..3bb6b9d 100644 --- a/server/monitors/service.ts +++ b/server/monitors/service.ts @@ -33,8 +33,7 @@ export class MonitorService { await this.store.bind(watch.id, remote.monitor_id, remote.status === "cancelled", remote.frequency); } catch (error) { const rejected = error instanceof ProviderError && [400,401,403,422].includes(error.status); - await this.store.pool.query("UPDATE bell_monitors SET status=$2 WHERE id=$1 AND status='dispatched'", [watch.id, rejected ? "failed" : "uncertain"]); - await this.store.pool.query("UPDATE bell_monitor_jobs SET next_at=now() WHERE id=$1", [`reconcile:${watch.id}`]); + await this.store.markDispatchUnresolved(watch.id, rejected ? "failed" : "uncertain"); } } const result = await this.store.get(watch.id); @@ -79,10 +78,7 @@ export class MonitorService { const parsed = new Webhook(required("PARALLEL_WEBHOOK_SECRET")).verify(body, headers); const event = callbackSchema.parse(parsed), id = headers["webhook-id"]; if (!id || id.length > 256) throw new Error("Invalid webhook identity"); - await this.store.transaction(async client => { - const inserted = await client.query("INSERT INTO bell_monitor_inbox(id,payload) VALUES ($1,$2) ON CONFLICT DO NOTHING RETURNING id", [id, event]); - if (inserted.rowCount) await this.store.enqueue("inbox", `inbox:${id}`, null, { id }, client); - }); + await this.store.acceptInbox(id, event); } async process() { const deadline = Date.now() + this.budget; @@ -108,13 +104,13 @@ export class MonitorService { /** Resolves to a saved payload when the work budget ran out mid-scan, so the next run resumes from there. */ private async run(job: Job, deadline: number): Promise | undefined> { if (job.kind === "inbox") { - const payload = (await this.store.pool.query("SELECT payload FROM bell_monitor_inbox WHERE id=$1", [job.payload.id])).rows[0]?.payload; + const payload = await this.store.inboxPayload(job.payload.id); const event = callbackSchema.parse(payload); - let watch = (await this.store.pool.query("SELECT * FROM bell_monitors WHERE remote_id=$1", [event.data.monitor_id])).rows[0]; + let watch = await this.store.findByRemoteId(event.data.monitor_id); if (!watch) { const id = event.data.metadata?.bell_watch_id; if (!id || !z.string().uuid().safeParse(id).success) return; - watch = (await this.store.pool.query("SELECT * FROM bell_monitors WHERE id=$1 AND status IN ('dispatched','uncertain','cancel_pending')", [id])).rows[0]; + watch = await this.store.findPendingIdentity(id); if (!watch) return; // Early callbacks wait for API-backed reconciliation; metadata is not ownership proof. throw new Error("Monitor identity not reconciled yet"); @@ -137,13 +133,13 @@ export class MonitorService { cursor = result.next_cursor ?? undefined; if (!cursor) break; } - await this.store.pool.query("UPDATE bell_monitor_jobs SET payload=$3 WHERE id=$1 AND lease_token=$2", [job.id, job.lease_token, cursor ? { cursor } : {}]); + await this.store.saveLeasePayload(job.id, job.lease_token, cursor ? { cursor } : {}); throw new Error("Creation remains uncertain"); } if (job.kind === "cancel") { if (!watch.remote_id) throw new Error("Creation unresolved"); await this.provider.cancel(watch.remote_id); - await this.store.pool.query("UPDATE bell_monitors SET status='cancelled' WHERE id=$1", [watch.id]); return; + await this.store.markCancelled(watch.id); return; } if (watch.stopped || watch.status !== "active") return; if (job.kind === "update") { @@ -184,7 +180,7 @@ export class MonitorService { return []; }); const ids = events.flatMap(event => event.event_type === "event_stream" ? [event.event_id] : []); - const known = new Set((await this.store.pool.query<{ event_id: string }>("SELECT event_id FROM bell_monitor_events WHERE watch_id=$1 AND event_id=ANY($2::text[])", [watch.id, ids])).rows.map(row => row.event_id)); + const known = await this.store.knownEventIds(watch.id, ids); for (const event of events) { const identity = event.event_type === "event_stream" ? event.event_id : `${event.event_type}:${event.timestamp}`; anchor ??= identity; @@ -195,12 +191,12 @@ export class MonitorService { const next = result.next_cursor ?? undefined; if (!next) { if (invalidEvents) { - await this.store.pool.query("UPDATE bell_monitors SET recovery_gap=true WHERE id=$1", [watch.id]); + await this.store.markRecoveryGap(watch.id); throw new InvalidMonitorEvent("Invalid monitor event in recovery scan"); } if (!group) { const gap = watch.scan_anchor ? !previousAnchorSeen : runs.size >= 100; - await this.store.pool.query("UPDATE bell_monitors SET last_scan_at=$2,scan_anchor=COALESCE($3,scan_anchor),recovery_gap=recovery_gap OR $4 WHERE id=$1", [watch.id, started, anchor ?? null, gap]); + await this.store.completeScan(watch.id, started, anchor ?? null, gap); } return; } @@ -210,7 +206,7 @@ export class MonitorService { if (cursor && cursor !== saved.cursor && saved.resumes < resumeLimit) { return { group, cursor, started, anchor, anchor_seen: previousAnchorSeen, runs: [...runs], invalid: invalidEvents, resumes: saved.resumes + 1 }; } - await this.store.pool.query("UPDATE bell_monitors SET recovery_gap=true WHERE id=$1", [watch.id]); + await this.store.markRecoveryGap(watch.id); throw new Error("Recovery scan exceeded work budget"); } } @@ -223,10 +219,7 @@ export class MonitorService { const urls = [...new Set(event.output.basis.flatMap(b => b.citations.map(c => c.url).filter((u): u is string => !!u)))].slice(0, 3); const text = `Watch update: ${watch.query.slice(0, 120)}\n${content.slice(0, 1200)}${content.length > 1200 ? "…" : ""}\n${urls.join("\n")}`; const key = `bell-watch-${hash(`${watch.id}:${event.event_id}`)}`; - await this.store.transaction(async client => { - const inserted = await client.query("INSERT INTO bell_monitor_events (watch_id,event_id,payload) VALUES ($1,$2,$3) ON CONFLICT DO NOTHING RETURNING event_id", [watch.id, event.event_id, event]); - if (inserted.rowCount) await this.store.enqueue("send", key, watch.id, { text, key, eventId: event.event_id }, client); - }); + await this.store.recordEvent(watch.id, event.event_id, event, key, { text, key, eventId: event.event_id }); } } export function monitors() { return new MonitorService(monitorStore()); } diff --git a/server/monitors/store.ts b/server/monitors/store.ts index ecb1790..7618a35 100644 --- a/server/monitors/store.ts +++ b/server/monitors/store.ts @@ -151,6 +151,49 @@ export class MonitorStore { async health(id: string, health: string, timestamp: string) { await this.pool.query("UPDATE bell_monitors SET health=$2,health_at=$3 WHERE id=$1 AND (health_at IS NULL OR health_at<$3)", [id, health, timestamp]); } + async markDispatchUnresolved(id: string, status: "failed" | "uncertain") { + await this.transaction(async client => { + await client.query("UPDATE bell_monitors SET status=$2 WHERE id=$1 AND status='dispatched'", [id, status]); + await client.query("UPDATE bell_monitor_jobs SET next_at=now() WHERE id=$1", [`reconcile:${id}`]); + }); + } + async acceptInbox(id: string, event: unknown) { + await this.transaction(async client => { + const inserted = await client.query("INSERT INTO bell_monitor_inbox(id,payload) VALUES ($1,$2) ON CONFLICT DO NOTHING RETURNING id", [id, event]); + if (inserted.rowCount) await this.enqueue("inbox", `inbox:${id}`, null, { id }, client); + }); + } + async inboxPayload(id: unknown): Promise { + return (await this.pool.query("SELECT payload FROM bell_monitor_inbox WHERE id=$1", [id])).rows[0]?.payload; + } + async findByRemoteId(remoteId: string): Promise { + return (await this.pool.query("SELECT * FROM bell_monitors WHERE remote_id=$1", [remoteId])).rows[0]; + } + async findPendingIdentity(id: string): Promise { + return (await this.pool.query("SELECT * FROM bell_monitors WHERE id=$1 AND status IN ('dispatched','uncertain','cancel_pending')", [id])).rows[0]; + } + async saveLeasePayload(id: string, leaseToken: string, payload: Record) { + await this.pool.query("UPDATE bell_monitor_jobs SET payload=$3 WHERE id=$1 AND lease_token=$2", [id, leaseToken, payload]); + } + async markCancelled(id: string) { + await this.pool.query("UPDATE bell_monitors SET status='cancelled' WHERE id=$1", [id]); + } + async knownEventIds(watchId: string, ids: string[]): Promise> { + const rows = (await this.pool.query<{ event_id: string }>("SELECT event_id FROM bell_monitor_events WHERE watch_id=$1 AND event_id=ANY($2::text[])", [watchId, ids])).rows; + return new Set(rows.map(row => row.event_id)); + } + async markRecoveryGap(id: string) { + await this.pool.query("UPDATE bell_monitors SET recovery_gap=true WHERE id=$1", [id]); + } + async completeScan(id: string, started: string, anchor: string | null, gap: boolean) { + await this.pool.query("UPDATE bell_monitors SET last_scan_at=$2,scan_anchor=COALESCE($3,scan_anchor),recovery_gap=recovery_gap OR $4 WHERE id=$1", [id, started, anchor, gap]); + } + async recordEvent(watchId: string, eventId: string, event: unknown, jobId: string, payload: Record) { + await this.transaction(async client => { + const inserted = await client.query("INSERT INTO bell_monitor_events (watch_id,event_id,payload) VALUES ($1,$2,$3) ON CONFLICT DO NOTHING RETURNING event_id", [watchId, eventId, event]); + if (inserted.rowCount) await this.enqueue("send", jobId, watchId, payload, client); + }); + } } let singleton: MonitorStore | undefined; export function monitorStore(): MonitorStore { From 691d1e6ddc79d4c54f28db8368eeba78723cea77 Mon Sep 17 00:00:00 2001 From: Matthew Nanas <37747855+matthewnanas@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:43:55 -0800 Subject: [PATCH 4/8] Record CAPTCHA transitions through one callback. The session and the durable task state each parsed the timeout and kept a second copy of the transition, and the runtime polled every 50ms to join them. Co-authored-by: Cursor --- agent/lib/browser-runtime.ts | 30 ++------------- agent/lib/task-state.ts | 15 ++------ lib/browser/captcha.ts | 11 ++++++ server/browser/browser-session.ts | 57 +++++++--------------------- tests/browser/captcha-live.test.ts | 7 +++- tests/browser/recaptcha-real.test.ts | 7 +++- 6 files changed, 43 insertions(+), 84 deletions(-) diff --git a/agent/lib/browser-runtime.ts b/agent/lib/browser-runtime.ts index 6f9839e..ef36cc3 100644 --- a/agent/lib/browser-runtime.ts +++ b/agent/lib/browser-runtime.ts @@ -92,23 +92,9 @@ async function runBrowserOperationUnlocked(sessionId: string, operation: Brow const runtime = getRuntime(sessionId); scheduleIdleClose(sessionId, runtime); const traceStart = runtime.state.traces.length; - let settled = false; - const pending = operation(runtime.browser).finally(() => { - settled = true; - }); - while (!settled) { - await Promise.race([ - pending.then( - () => undefined, - () => undefined, - ), - new Promise((resolve) => setTimeout(resolve, 50)), - ]); - commitCaptchaQueue(runtime); - } let result: T; try { - result = await pending; + result = await operation(runtime.browser); } catch (error) { if (!(error instanceof CaptchaHandoffError)) throw error; result = { @@ -117,7 +103,6 @@ async function runBrowserOperationUnlocked(sessionId: string, operation: Brow action: "Complete the challenge in the existing Live View, then resume this task.", } as T; } - commitCaptchaQueue(runtime); scheduleIdleClose(sessionId, runtime); return { result, @@ -134,16 +119,6 @@ async function runBrowserOperationUnlocked(sessionId: string, operation: Brow }; } -function commitCaptchaQueue(runtime: BrowserRuntime): void { - const transitions = runtime.browser.drainCaptchaTransitions(); - if (!transitions.length) return; - taskState.update((current) => { - let next = current; - for (const transition of transitions) next = applyCaptchaTransition(next, transition); - return updateTaskBrowser(next, runtime.state.browser); - }); -} - export async function saveBrowserRuntime(sessionId: string): Promise { const runtime = runtimes.get(sessionId); if (runtime) @@ -328,6 +303,9 @@ function createRuntimeBrowser( contextId, proxyLocation, captcha: taskState.get().captcha, + onCaptchaTransition: (transition) => { + taskState.update((current) => updateTaskBrowser(applyCaptchaTransition(current, transition), state.browser)); + }, ensureContext: personalBrowserEnabled() ? undefined : async (currentContextId) => { const context = await ensureAgentContext(sessionId, currentContextId); state.browser.contextId = context.id; diff --git a/agent/lib/task-state.ts b/agent/lib/task-state.ts index f90ed56..024efd9 100644 --- a/agent/lib/task-state.ts +++ b/agent/lib/task-state.ts @@ -1,13 +1,16 @@ import { defineState } from "eve/context"; +import { captchaTimeoutMs, type CaptchaTransition } from "../../lib/browser/captcha"; import type { BrowserProxyLocation, BrowserState } from "../../server/browser/types.js"; +export { captchaTimeoutMs, type CaptchaTransition }; + export type TaskStatus = "idle" | "running" | "solving_captcha" | "waiting_for_user" | "complete" | "failed" | "canceled"; export type CaptchaStatus = "idle" | "solving" | "solved" | "errored" | "timed_out"; export interface CaptchaJournalEntry { id: string; - type: "started" | "finished" | "errored" | "timed_out"; + type: CaptchaTransition["type"]; at: string; } export interface DurableCaptchaRecord { @@ -20,12 +23,6 @@ export interface DurableCaptchaRecord { page?: { origin: string; path: string }; journal: CaptchaJournalEntry[]; } -export interface CaptchaTransition { - id: string; - type: CaptchaJournalEntry["type"]; - at: string; - page?: { origin: string; path: string }; -} export const CAPTCHA_JOURNAL_LIMIT = 32; export type BrowserLifecycle = "not_started" | "starting" | "active" | "disconnected" | "expired" | "closed" | "error"; @@ -219,10 +216,6 @@ export function reconcileCaptchaAfterRestart( page: captcha.page, }); } -export function captchaTimeoutMs(): number { - const value = Number(process.env.BELL_CAPTCHA_TIMEOUT_MS ?? 30_000); - return Number.isFinite(value) && value > 0 ? value : 30_000; -} function captchaBlocksGeneric(record: DurableTaskRecord): boolean { return ( record.status === "waiting_for_user" || diff --git a/lib/browser/captcha.ts b/lib/browser/captcha.ts index a5c4f21..fbd911c 100644 --- a/lib/browser/captcha.ts +++ b/lib/browser/captcha.ts @@ -4,6 +4,17 @@ export const CAPTCHA_EVENT_NAMES = [ "browserbase-solving-errored", ] as const; export type CaptchaSignal = "started" | "finished" | "errored"; +export interface CaptchaTransition { + id: string; + type: CaptchaSignal | "timed_out"; + at: string; + page?: { origin: string; path: string }; +} + +export function captchaTimeoutMs(): number { + const value = Number(process.env.BELL_CAPTCHA_TIMEOUT_MS ?? 30_000); + return Number.isFinite(value) && value > 0 ? value : 30_000; +} export function parseBrowserbaseCaptchaEvent(value: unknown): CaptchaSignal | undefined { if (!value || typeof value !== "object") return; diff --git a/server/browser/browser-session.ts b/server/browser/browser-session.ts index bf4b94c..9ef8b5f 100644 --- a/server/browser/browser-session.ts +++ b/server/browser/browser-session.ts @@ -10,16 +10,10 @@ import { type Page, } from "@browserbasehq/stagehand"; -import { parseBrowserbaseCaptchaEvent, safeCaptchaPage, type CaptchaSignal } from "../../lib/browser/captcha"; +import { captchaTimeoutMs, parseBrowserbaseCaptchaEvent, safeCaptchaPage, type CaptchaSignal, type CaptchaTransition } from "../../lib/browser/captcha"; import { addTrace } from "./trace.js"; import type { BrowserProxyLocation, BrowserState, ConversationState, RunAction } from "./types.js"; -export interface BrowserCaptchaTransition { - id: string; - type: "started" | "finished" | "errored" | "timed_out"; - at: string; - page?: { origin: string; path: string }; -} export class CaptchaHandoffError extends Error { readonly code = "captcha_handoff"; constructor(readonly reason: "errored" | "timed_out") { @@ -52,7 +46,6 @@ export class BrowserSession { private readonly blockedOrigins = new Set(); private readonly liveSourceOrigins = new Set(); private readonly captchaSubscriptions = new Map(); - private readonly captchaTransitions: BrowserCaptchaTransition[] = []; private captchaStatus: "idle" | "solving" | "solved" | "errored" | "timed_out" = "idle"; private captchaStartedAt?: string; private captchaNeedsObservation = false; @@ -66,6 +59,7 @@ export class BrowserSession { currentContextId?: string, ) => Promise<{ id: string; status: "draft" | "saved" }>; captcha?: { status: "idle" | "solving" | "solved" | "errored" | "timed_out"; startedAt?: string }; + onCaptchaTransition?: (transition: CaptchaTransition) => void; } = {}, ) { this.captchaStatus = options.captcha?.status ?? "idle"; @@ -678,11 +672,13 @@ export class BrowserSession { } } - drainCaptchaTransitions(): BrowserCaptchaTransition[] { - const page = safeCaptchaPage(this.state.browser.url); - return this.captchaTransitions - .splice(0) - .map((event) => (event.page ? event : { ...event, page })); + private emitCaptcha(type: CaptchaTransition["type"], at: string): void { + this.options.onCaptchaTransition?.({ + id: crypto.randomUUID(), + type, + at, + page: safeCaptchaPage(this.state.browser.url), + }); } private async ensureCaptchaSubscriptions(): Promise { @@ -704,31 +700,16 @@ export class BrowserSession { if (this.captchaStatus === "solving") return; this.captchaStatus = "solving"; this.captchaStartedAt = at; - this.captchaTransitions.push({ - id: crypto.randomUUID(), - type: "started", - at, - page, - }); + this.emitCaptcha("started", at); addTrace(this.state, "browser", "captcha.started", "Browserbase is solving a CAPTCHA.", page); } else if (signal === "finished" && this.captchaStatus === "solving") { this.captchaStatus = "solved"; this.captchaNeedsObservation = true; - this.captchaTransitions.push({ - id: crypto.randomUUID(), - type: "finished", - at, - page, - }); + this.emitCaptcha("finished", at); addTrace(this.state, "browser", "captcha.finished", "Browserbase solved the CAPTCHA.", page); } else if (signal === "errored" && this.captchaStatus === "solving") { this.captchaStatus = "errored"; - this.captchaTransitions.push({ - id: crypto.randomUUID(), - type: "errored", - at, - page, - }); + this.emitCaptcha("errored", at); addTrace( this.state, "error", @@ -745,19 +726,14 @@ export class BrowserSession { const started = Date.parse(this.captchaStartedAt ?? new Date().toISOString()); while ( this.captchaStatus === "solving" && - Date.now() - started < configuredCaptchaTimeoutMs() + Date.now() - started < captchaTimeoutMs() ) await new Promise((resolve) => setTimeout(resolve, 25)); if (this.captchaStatus === "solving") { const at = new Date().toISOString(); this.captchaStatus = "timed_out"; const page = safeCaptchaPage(this.state.browser.url); - this.captchaTransitions.push({ - id: crypto.randomUUID(), - type: "timed_out", - at, - page, - }); + this.emitCaptcha("timed_out", at); addTrace( this.state, "error", @@ -879,11 +855,6 @@ function safeActivityUrl(value: string): string { } } -function configuredCaptchaTimeoutMs(): number { - const value = Number(process.env.BELL_CAPTCHA_TIMEOUT_MS ?? 30_000); - return Number.isFinite(value) && value > 0 ? value : 30_000; -} - function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } diff --git a/tests/browser/captcha-live.test.ts b/tests/browser/captcha-live.test.ts index 8440793..5e1d61b 100644 --- a/tests/browser/captcha-live.test.ts +++ b/tests/browser/captcha-live.test.ts @@ -1,6 +1,7 @@ import { chromium, type Browser } from "playwright-core"; import { afterEach, describe, expect, it, vi } from "vitest"; +import type { CaptchaTransition } from "../../lib/browser/captcha"; import { BrowserSession } from "../../server/browser/browser-session.js"; import type { ConversationState } from "../../server/browser/types.js"; @@ -15,7 +16,10 @@ describe("opt-in hosted CAPTCHA marker fixture", () => { traces: [], browser: { provider: "not-started", status: "idle" }, }; - const session = new BrowserSession(state); + const transitions: CaptchaTransition[] = []; + const session = new BrowserSession(state, { + onCaptchaTransition: (transition) => transitions.push(transition), + }); let fixtureBrowser: Browser | undefined; try { const fixture = "https://bell-fixture.invalid/captcha"; @@ -34,7 +38,6 @@ describe("opt-in hosted CAPTCHA marker fixture", () => { expect(state.traces.map((event) => event.name)).toEqual( expect.arrayContaining(["captcha.started", "captcha.finished", "captcha.observed"]), ); - const transitions = session.drainCaptchaTransitions(); expect(transitions.map((event) => event.type)).toEqual(["started", "finished"]); expect(JSON.stringify({ transitions, traces: state.traces })).not.toMatch( /devtools|debuggerFullscreenUrl|wss=/i, diff --git a/tests/browser/recaptcha-real.test.ts b/tests/browser/recaptcha-real.test.ts index ea7a4fc..54cb4b9 100644 --- a/tests/browser/recaptcha-real.test.ts +++ b/tests/browser/recaptcha-real.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; +import type { CaptchaTransition } from "../../lib/browser/captcha"; import { BrowserSession } from "../../server/browser/browser-session.js"; import type { ConversationState } from "../../server/browser/types.js"; @@ -14,13 +15,15 @@ describe("opt-in real Browserbase CAPTCHA solve", () => { traces: [], browser: { provider: "not-started", status: "idle" }, }, - session = new BrowserSession(state); + transitions: CaptchaTransition[] = [], + session = new BrowserSession(state, { + onCaptchaTransition: (transition) => transitions.push(transition), + }); try { await session.run([ { action: "goto", url: "https://google.com/recaptcha/api2/demo" }, { action: "wait", milliseconds: 2_000 }, ]); - const transitions = session.drainCaptchaTransitions(); expect(transitions.map((event) => event.type)).toEqual(["started", "finished"]); await session.run([ From fe596ce75fe22214dd633091eef5f4da8c30c91f Mon Sep 17 00:00:00 2001 From: Matthew Nanas <37747855+matthewnanas@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:45:49 -0800 Subject: [PATCH 5/8] Share one browser operation lock across page work. Co-authored-by: Cursor --- server/browser/browser-session.ts | 145 ++++++++++++++---------------- 1 file changed, 68 insertions(+), 77 deletions(-) diff --git a/server/browser/browser-session.ts b/server/browser/browser-session.ts index 9ef8b5f..7d8cda7 100644 --- a/server/browser/browser-session.ts +++ b/server/browser/browser-session.ts @@ -171,11 +171,8 @@ export class BrowserSession { privateResult = false, ): Promise { signal.throwIfAborted(); - const release = await this.acquireOperation(); - try { + return this.withPage(async () => { signal.throwIfAborted(); - await this.start(); - await this.gateCaptcha(); if (!this.browserbase || !this.remoteSessionId || !this.browser) throw new Error("This integration requires a hosted Browserbase session."); const page = @@ -189,9 +186,7 @@ export class BrowserSession { await this.gateCaptcha(); } return result; - } finally { - release(); - } + }); } async snapshot(): Promise<{ @@ -201,10 +196,7 @@ export class BrowserSession { pageStatus: "live" | "blocked"; recovery: BrowserRecoveryReport; }> { - const release = await this.acquireOperation(); - try { - await this.start(); - await this.gateCaptcha(); + return this.withPage(async () => { const started = performance.now(); const result = await this.requireStagehand().experimentalBatch(async ({ page }) => { const snapshot = await page.snapshot({ includeIframes: true }); @@ -256,9 +248,7 @@ export class BrowserSession { pageStatus: assessment.blocked ? "blocked" : "live", recovery: this.recoveryReport(), }; - } finally { - release(); - } + }); } async run(actions: RunAction[]): Promise<{ completed: number; url: string }> { @@ -272,10 +262,7 @@ export class BrowserSession { ) throw new Error("Browser navigation requires an HTTP(S) URL without embedded credentials."); } - const release = await this.acquireOperation(); - try { - await this.start(); - await this.gateCaptcha(); + return this.withPage(async () => { const started = performance.now(); const resolvedActions = actions.map((item) => "target" in item && item.target @@ -333,9 +320,7 @@ export class BrowserSession { formatRunCode(resolvedActions), ); return { completed: actions.length, url }; - } finally { - release(); - } + }); } async secureLogin(input: { @@ -359,12 +344,10 @@ export class BrowserSession { input.totpTarget, input.otpSubmitTarget, input.successTarget]; if (selectors.some((selector) => selector && (/^\s*(?:xpath=|\/|\()/i.test(selector) || selector.includes(">>")))) throw new Error("Secure login requires CSS selectors in the top-level page; frame selectors are not allowed."); - const release = await this.acquireOperation(); - try { - await this.start(); - await this.gateCaptcha(); - const started = performance.now(); - const result = await this.requireStagehand().experimentalBatch( + return this.withPage(async () => { + try { + const started = performance.now(); + const result = await this.requireStagehand().experimentalBatch( async ({ page }, values) => { const checkDestination = async () => { const destination = new URL(await page.url()); @@ -405,43 +388,42 @@ export class BrowserSession { }, input, { timeout: 45_000 }, - ); - await this.gateCaptcha(); - this.updateBrowserState(result.url, result.title); - const host = new URL(result.url).hostname; - const status = - input.totpTarget && !input.totp - ? "waiting_for_user" - : result.success - ? "complete" - : "unverified"; - addTrace( - this.state, - "tool", - "vault.login", - status === "complete" - ? `Verified login state on ${host}.` - : status === "waiting_for_user" - ? `Login on ${host} is waiting for a human challenge.` - : `Filled the login on ${host}, but success is not verified yet.`, - { host, status, secretValuesRedacted: true }, - Math.round(performance.now() - started), - ); - return { status, host, url: result.url }; - } catch (error) { - if (error instanceof CaptchaHandoffError) throw error; - addTrace( - this.state, - "error", - "vault.login", - "The secure login attempt failed. Secret values were redacted.", - ); - throw new Error( - "Secure login failed. Inspect the current page and field selectors, then retry.", - ); - } finally { - release(); - } + ); + await this.gateCaptcha(); + this.updateBrowserState(result.url, result.title); + const host = new URL(result.url).hostname; + const status = + input.totpTarget && !input.totp + ? "waiting_for_user" + : result.success + ? "complete" + : "unverified"; + addTrace( + this.state, + "tool", + "vault.login", + status === "complete" + ? `Verified login state on ${host}.` + : status === "waiting_for_user" + ? `Login on ${host} is waiting for a human challenge.` + : `Filled the login on ${host}, but success is not verified yet.`, + { host, status, secretValuesRedacted: true }, + Math.round(performance.now() - started), + ); + return { status, host, url: result.url }; + } catch (error) { + if (error instanceof CaptchaHandoffError) throw error; + addTrace( + this.state, + "error", + "vault.login", + "The secure login attempt failed. Secret values were redacted.", + ); + throw new Error( + "Secure login failed. Inspect the current page and field selectors, then retry.", + ); + } + }); } async screenshot(): Promise<{ @@ -449,10 +431,7 @@ export class BrowserSession { data: string; url: string; }> { - const release = await this.acquireOperation(); - try { - await this.start(); - await this.gateCaptcha(); + return this.withPage(async () => { const started = performance.now(); const result = await this.requireStagehand().experimentalBatch(async ({ page }) => ({ data: await page.screenshot({ type: "png" }), @@ -473,9 +452,7 @@ export class BrowserSession { Math.round(performance.now() - started), ); return { mediaType: "image/png", data: buffer.toString("base64"), url }; - } finally { - release(); - } + }); } async close(): Promise { @@ -503,9 +480,7 @@ export class BrowserSession { } async importCookies(cookies: CookieParam[], url?: string): Promise { - const release = await this.acquireOperation(); - try { - await this.start(); + return this.withOperation(async () => { if (cookies.length) await this.browser!.context.addCookies(cookies); if (url) { const page = @@ -526,9 +501,7 @@ export class BrowserSession { url: url ? safeCaptchaPage(url) : undefined, }, ); - } finally { - release(); - } + }); } recoveryReport(): BrowserRecoveryReport { @@ -554,6 +527,24 @@ export class BrowserSession { return this.stagehand; } + private async withPage(operation: () => Promise): Promise { + return this.withOperation(async () => { + await this.gateCaptcha(); + return operation(); + }); + } + + /** Serialize browser work. Cookie import starts the session without the CAPTCHA gate. */ + private async withOperation(operation: () => Promise): Promise { + const release = await this.acquireOperation(); + try { + await this.start(); + return await operation(); + } finally { + release(); + } + } + private async acquireOperation(): Promise<() => void> { let release!: () => void; const gate = new Promise((resolve) => { From 8f3b01e8d75967bba93d777803810b5f2135bc92 Mon Sep 17 00:00:00 2001 From: Matthew Nanas <37747855+matthewnanas@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:51:48 -0800 Subject: [PATCH 6/8] Move CAPTCHA gating and hosted launch out of the browser session. Co-authored-by: Cursor --- server/browser/browser-session.ts | 362 ++++++--------------------- server/browser/browserbase-launch.ts | 169 +++++++++++++ server/browser/captcha-gate.ts | 154 ++++++++++++ tests/browser/captcha-live.test.ts | 4 +- 4 files changed, 408 insertions(+), 281 deletions(-) create mode 100644 server/browser/browserbase-launch.ts create mode 100644 server/browser/captcha-gate.ts diff --git a/server/browser/browser-session.ts b/server/browser/browser-session.ts index 7d8cda7..77c78d0 100644 --- a/server/browser/browser-session.ts +++ b/server/browser/browser-session.ts @@ -3,27 +3,24 @@ import Browserbase from "@browserbasehq/sdk"; import { localBrowser, Stagehand, - browserbase as stagehandBrowserbase, - type CDPSubscription, type CookieParam, type StagehandBrowser, type Page, } from "@browserbasehq/stagehand"; -import { captchaTimeoutMs, parseBrowserbaseCaptchaEvent, safeCaptchaPage, type CaptchaSignal, type CaptchaTransition } from "../../lib/browser/captcha"; +import { safeCaptchaPage, type CaptchaTransition } from "../../lib/browser/captcha"; +import { + browserProvider, + launchHostedBrowser, + releaseHostedSession, + type HostedBrowserHost, +} from "./browserbase-launch"; +import { CaptchaGate, CaptchaHandoffError } from "./captcha-gate"; import { addTrace } from "./trace.js"; import type { BrowserProxyLocation, BrowserState, ConversationState, RunAction } from "./types.js"; -export class CaptchaHandoffError extends Error { - readonly code = "captcha_handoff"; - constructor(readonly reason: "errored" | "timed_out") { - super( - reason === "errored" - ? "CAPTCHA solving errored. Continue in Live View, then resume this task." - : "CAPTCHA solving timed out. Continue in Live View, then resume this task.", - ); - } -} +export { CaptchaHandoffError } from "./captcha-gate"; +export { browserProvider, browserbaseLaunchOptions } from "./browserbase-launch"; export interface BrowserRecoveryReport { visitedOrigins: string[]; @@ -45,10 +42,7 @@ export class BrowserSession { private readonly visitedOrigins = new Set(); private readonly blockedOrigins = new Set(); private readonly liveSourceOrigins = new Set(); - private readonly captchaSubscriptions = new Map(); - private captchaStatus: "idle" | "solving" | "solved" | "errored" | "timed_out" = "idle"; - private captchaStartedAt?: string; - private captchaNeedsObservation = false; + private readonly captchaGate: CaptchaGate; constructor( private readonly state: ConversationState, @@ -62,14 +56,23 @@ export class BrowserSession { onCaptchaTransition?: (transition: CaptchaTransition) => void; } = {}, ) { - this.captchaStatus = options.captcha?.status ?? "idle"; - this.captchaStartedAt = options.captcha?.startedAt; + this.captchaGate = new CaptchaGate( + { + state, + browser: () => this.browser, + onCaptchaTransition: options.onCaptchaTransition, + observePage: () => this.observeCaptchaPage(), + applyPage: (xpathMap, url, title) => { + this.xpathMap = xpathMap; + this.updateBrowserState(url, title); + }, + }, + options.captcha, + ); } resumeAfterHumanInput(): void { - if (this.captchaStatus !== "errored" && this.captchaStatus !== "timed_out") return; - this.captchaStatus = "idle"; - this.captchaNeedsObservation = true; + this.captchaGate.resumeAfterHumanInput(); } async start(): Promise { @@ -108,7 +111,7 @@ export class BrowserSession { this.options.contextId = currentContextId; contextStatus = "saved"; } - await this.startBrowserbase(apiKey); + await launchHostedBrowser(this.hostedBrowser(), apiKey); } else { this.browser = await localBrowser.launch({ headless: process.env.BELL_HEADLESS !== "false", @@ -127,7 +130,7 @@ export class BrowserSession { proxyLocation: requested === "browserbase" ? this.options.proxyLocation : undefined, sessionId: this.remoteSessionId, }; - await this.ensureCaptchaSubscriptions(); + await this.captchaGate.ensureSubscriptions(); if (requested === "browserbase" && contextStatus) { addTrace( this.state, @@ -458,14 +461,14 @@ export class BrowserSession { async close(): Promise { this.closeRequested = true; await this.startPromise?.catch(() => undefined); - await this.clearCaptchaSubscriptions(); + await this.captchaGate.clearSubscriptions(); const stagehand = this.stagehand; const browser = this.browser; this.stagehand = undefined; this.browser = undefined; await stagehand?.close().catch(() => undefined); await browser?.close().catch(() => undefined); - await this.releaseRemoteSession(); + await releaseHostedSession(this.hostedBrowser()); if (this.state.browser.status !== "error") this.state.browser.status = "idle"; } @@ -560,238 +563,83 @@ export class BrowserSession { return release; } - private async startBrowserbase(apiKey: string): Promise { - this.browserbase = new Browserbase({ apiKey }); - const persistedSessionId = this.loadPersistedSessionId(); - - if (persistedSessionId) { - for (let attempt = 1; attempt <= 2; attempt += 1) { - try { - const prior = await this.browserbase.sessions.retrieve(persistedSessionId); - if (prior.userMetadata?.networkLogging !== "false" || prior.userMetadata?.sessionRecording !== "false") - throw new Error("Session logging and recording settings cannot be verified."); - if (personalBrowserEnabled() && (prior.userMetadata?.onepasswordExtension !== process.env.BROWSERBASE_1PASSWORD_EXTENSION_ID || prior.userMetadata?.privateContext !== "true")) - throw new Error("The personal 1Password browser settings cannot be verified."); - const browser = await stagehandBrowserbase.connect({ - apiKey, - sessionId: persistedSessionId, - }); - await this.attachStagehand(browser); - this.remoteSessionId = persistedSessionId; - addTrace( - this.state, - "browser", - "browser.reattach", - "Reattached Stagehand v4 to the saved Browserbase session.", - { sessionId: persistedSessionId, attempt }, - ); - return; - } catch { - await this.stagehand?.close().catch(() => undefined); - await this.browser?.close().catch(() => undefined); - this.stagehand = undefined; - this.browser = undefined; - if (attempt < 2) await new Promise((resolve) => setTimeout(resolve, 500)); - } - } - - this.remoteSessionId = persistedSessionId; - await this.releaseRemoteSession(); - addTrace( - this.state, - "system", - "browser.replace", - "The saved Browserbase session expired. Bell is starting a replacement with the same Context.", - { - contextPreserved: Boolean(this.options.contextId ?? process.env.BROWSERBASE_CONTEXT_ID), - }, - ); - } - - let lastError: unknown; - for (let attempt = 1; attempt <= 3; attempt += 1) { - let launched: StagehandBrowser | undefined; - try { - launched = await stagehandBrowserbase.launch( - browserbaseLaunchOptions(apiKey, this.options.contextId, this.options.proxyLocation), - ); - await this.attachStagehand(launched); - if (!launched.sessionId) - throw new Error("Stagehand did not return a Browserbase session ID."); - this.remoteSessionId = launched.sessionId; - this.persistSessionId(launched.sessionId); - return; - } catch (error) { - lastError = error; - await this.stagehand?.close().catch(() => undefined); - await launched?.close().catch(() => undefined); - this.stagehand = undefined; - this.browser = undefined; - if (launched?.sessionId) { - this.remoteSessionId = launched.sessionId; - await this.releaseRemoteSession(); - } - if (attempt < 3) { - addTrace( - this.state, - "system", - "browser.start.retry", - `The Browserbase connection failed. Bell will retry with a new verified, proxied session (${attempt}/3).`, - { attempt, proxies: true, verified: true }, - ); - await new Promise((resolve) => setTimeout(resolve, attempt * 500)); - } - } - } - - throw lastError instanceof Error ? lastError : new Error(String(lastError)); - } - private async attachStagehand(browser: StagehandBrowser): Promise { try { const stagehand = await Stagehand.create({ browser, logging: { level: "off" }, }); - await this.clearCaptchaSubscriptions(); + await this.captchaGate.clearSubscriptions(); this.browser = browser; this.stagehand = stagehand; - await this.ensureCaptchaSubscriptions(); + await this.captchaGate.ensureSubscriptions(); } catch (error) { await browser.close().catch(() => undefined); throw error; } } - private emitCaptcha(type: CaptchaTransition["type"], at: string): void { - this.options.onCaptchaTransition?.({ - id: crypto.randomUUID(), - type, - at, - page: safeCaptchaPage(this.state.browser.url), - }); - } - - private async ensureCaptchaSubscriptions(): Promise { - if (!this.browser) return; - for (const page of await this.browser.context.pages()) { - if (this.captchaSubscriptions.has(page.pageId)) continue; - const subscription = await page.on("console", (event) => { - const signal = parseBrowserbaseCaptchaEvent(event); - if (signal) this.recordCaptchaSignal(signal); - }); - this.captchaSubscriptions.set(page.pageId, subscription); - } - } - - private recordCaptchaSignal(signal: CaptchaSignal): void { - const at = new Date().toISOString(); - const page = safeCaptchaPage(this.state.browser.url); - if (signal === "started") { - if (this.captchaStatus === "solving") return; - this.captchaStatus = "solving"; - this.captchaStartedAt = at; - this.emitCaptcha("started", at); - addTrace(this.state, "browser", "captcha.started", "Browserbase is solving a CAPTCHA.", page); - } else if (signal === "finished" && this.captchaStatus === "solving") { - this.captchaStatus = "solved"; - this.captchaNeedsObservation = true; - this.emitCaptcha("finished", at); - addTrace(this.state, "browser", "captcha.finished", "Browserbase solved the CAPTCHA.", page); - } else if (signal === "errored" && this.captchaStatus === "solving") { - this.captchaStatus = "errored"; - this.emitCaptcha("errored", at); - addTrace( - this.state, - "error", - "captcha.errored", - "Browserbase CAPTCHA solving errored; human input is required.", - page, - ); - } - } - private async gateCaptcha(): Promise { - await this.ensureCaptchaSubscriptions(); - if (this.captchaStatus === "solving") { - const started = Date.parse(this.captchaStartedAt ?? new Date().toISOString()); - while ( - this.captchaStatus === "solving" && - Date.now() - started < captchaTimeoutMs() - ) - await new Promise((resolve) => setTimeout(resolve, 25)); - if (this.captchaStatus === "solving") { - const at = new Date().toISOString(); - this.captchaStatus = "timed_out"; - const page = safeCaptchaPage(this.state.browser.url); - this.emitCaptcha("timed_out", at); - addTrace( - this.state, - "error", - "captcha.timed_out", - "CAPTCHA solving timed out; human input is required.", - page, - ); - } - } - if (this.captchaStatus === "errored" || this.captchaStatus === "timed_out") - throw new CaptchaHandoffError(this.captchaStatus); - if (this.captchaNeedsObservation) { - this.captchaNeedsObservation = false; - const result = await this.requireStagehand().experimentalBatch(async ({ page }) => ({ - snapshot: await page.snapshot({ includeIframes: true }), - url: await page.url(), - title: await page.title(), - })); - this.xpathMap = result.snapshot.xpathMap; - this.updateBrowserState(result.url, result.title); - addTrace( - this.state, - "browser", - "captcha.observed", - `Read the post-CAPTCHA page after the current gate.`, - { - page: safeCaptchaPage(result.url), - treePreview: result.snapshot.formattedTree.slice(0, 800), - }, - ); - } + await this.captchaGate.gate(); } - private async clearCaptchaSubscriptions(): Promise { - const subscriptions = [...this.captchaSubscriptions.values()]; - this.captchaSubscriptions.clear(); - await Promise.all( - subscriptions.map((subscription) => subscription.unsubscribe().catch(() => undefined)), - ); + private observeCaptchaPage() { + return this.requireStagehand().experimentalBatch(async ({ page }) => ({ + snapshot: await page.snapshot({ includeIframes: true }), + url: await page.url(), + title: await page.title(), + })); } - private async releaseRemoteSession(): Promise { - const sessionId = this.remoteSessionId ?? this.loadPersistedSessionId(); - if (!sessionId) return; - this.browserbase ??= new Browserbase({ apiKey: process.env.BROWSERBASE_API_KEY }); - try { - await this.browserbase.sessions.update(sessionId, { status: "REQUEST_RELEASE" }); - } catch (error) { - // An already removed session is released. Keep the ID on every other - // failure so cleanup can retry instead of silently orphaning the browser. - if (!(error instanceof Browserbase.APIError) || error.status !== 404) - throw new Error("The hosted browser could not be released. Retry cleanup."); - } - this.remoteSessionId = undefined; - this.clearPersistedSessionId(); - } - - private loadPersistedSessionId(): string | undefined { - return this.state.browser.sessionId; + private hostedBrowser(): HostedBrowserHost { + const readBrowserbase = () => this.browserbase; + const writeBrowserbase = (client: Browserbase | undefined) => { + this.browserbase = client; + }; + const readRemoteSessionId = () => this.remoteSessionId; + const writeRemoteSessionId = (sessionId: string | undefined) => { + this.remoteSessionId = sessionId; + }; + const readContextId = () => this.options.contextId; + const readProxyLocation = () => this.options.proxyLocation; + return { + state: this.state, + get contextId() { + return readContextId(); + }, + get proxyLocation() { + return readProxyLocation(); + }, + get browserbase() { + return readBrowserbase(); + }, + set browserbase(client) { + writeBrowserbase(client); + }, + get remoteSessionId() { + return readRemoteSessionId(); + }, + set remoteSessionId(sessionId) { + writeRemoteSessionId(sessionId); + }, + attach: (browser) => this.attachStagehand(browser), + discardAttached: () => this.discardAttached(), + discardLaunch: (browser) => this.discardLaunch(browser), + }; } - private persistSessionId(sessionId: string): void { - this.state.browser.sessionId = sessionId; + private async discardAttached(): Promise { + await this.stagehand?.close().catch(() => undefined); + await this.browser?.close().catch(() => undefined); + this.stagehand = undefined; + this.browser = undefined; } - private clearPersistedSessionId(): void { - delete this.state.browser.sessionId; + private async discardLaunch(browser: StagehandBrowser | undefined): Promise { + await this.stagehand?.close().catch(() => undefined); + await browser?.close().catch(() => undefined); + this.stagehand = undefined; + this.browser = undefined; } private updateBrowserState(url: string, title: string): BrowserState { @@ -850,50 +698,6 @@ function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } -export function browserProvider(): "local" | "browserbase" { - if (process.env.STAGEHAND_BROWSER === "local") return "local"; - if (process.env.STAGEHAND_BROWSER === "browserbase") return "browserbase"; - return process.env.BROWSERBASE_API_KEY ? "browserbase" : "local"; -} - -export function browserbaseLaunchOptions( - apiKey: string, - durableContextId?: string, - proxyLocation?: BrowserProxyLocation, -) { - const contextId = durableContextId ?? process.env.BROWSERBASE_CONTEXT_ID; - const projectId = process.env.BROWSERBASE_PROJECT_ID; - if (personalBrowserEnabled() && contextId) throw new Error("Personal 1Password cannot use a reusable Context."); - return { - apiKey, - ...(personalBrowserEnabled() ? { extensionId: process.env.BROWSERBASE_1PASSWORD_EXTENSION_ID!.trim() } : {}), - ...(projectId ? { projectId } : {}), - proxies: proxyLocation - ? [ - { - type: "browserbase" as const, - geolocation: proxyLocation, - }, - ] - : true, - keepAlive: true, - browserSettings: { - verified: true, - logSession: false, - recordSession: false, - solveCaptchas: true, - ...(contextId ? { context: { id: contextId, persist: true } } : {}), - }, - userMetadata: { - product: "bell", - transport: "stagehand-v4-eve", - networkLogging: "false", - sessionRecording: "false", - ...(personalBrowserEnabled() ? { onepasswordExtension: process.env.BROWSERBASE_1PASSWORD_EXTENSION_ID!.trim(), privateContext: "true" } : {}), - }, - }; -} - export function assessSnapshot(tree: string, url: string): { blocked: boolean; signal?: string } { const text = `${url}\n${tree}`.toLowerCase(); if ( diff --git a/server/browser/browserbase-launch.ts b/server/browser/browserbase-launch.ts new file mode 100644 index 0000000..3fd0daf --- /dev/null +++ b/server/browser/browserbase-launch.ts @@ -0,0 +1,169 @@ +import Browserbase from "@browserbasehq/sdk"; +import { browserbase as stagehandBrowserbase, type StagehandBrowser } from "@browserbasehq/stagehand"; + +import { personalBrowserEnabled } from "../onepassword/browser-policy"; +import { addTrace } from "./trace"; +import type { BrowserProxyLocation, ConversationState } from "./types"; + +/** The session fields a hosted launch is allowed to change. */ +export interface HostedBrowserHost { + readonly state: ConversationState; + readonly contextId?: string; + readonly proxyLocation?: BrowserProxyLocation; + browserbase?: Browserbase; + remoteSessionId?: string; + attach(browser: StagehandBrowser): Promise; + discardAttached(): Promise; + discardLaunch(browser: StagehandBrowser | undefined): Promise; +} + +export function browserProvider(): "local" | "browserbase" { + if (process.env.STAGEHAND_BROWSER === "local") return "local"; + if (process.env.STAGEHAND_BROWSER === "browserbase") return "browserbase"; + return process.env.BROWSERBASE_API_KEY ? "browserbase" : "local"; +} + +export function browserbaseLaunchOptions( + apiKey: string, + durableContextId?: string, + proxyLocation?: BrowserProxyLocation, +) { + const contextId = durableContextId ?? process.env.BROWSERBASE_CONTEXT_ID; + const projectId = process.env.BROWSERBASE_PROJECT_ID; + if (personalBrowserEnabled() && contextId) throw new Error("Personal 1Password cannot use a reusable Context."); + return { + apiKey, + ...(personalBrowserEnabled() ? { extensionId: process.env.BROWSERBASE_1PASSWORD_EXTENSION_ID!.trim() } : {}), + ...(projectId ? { projectId } : {}), + proxies: proxyLocation + ? [ + { + type: "browserbase" as const, + geolocation: proxyLocation, + }, + ] + : true, + keepAlive: true, + browserSettings: { + verified: true, + logSession: false, + recordSession: false, + solveCaptchas: true, + ...(contextId ? { context: { id: contextId, persist: true } } : {}), + }, + userMetadata: { + product: "bell", + transport: "stagehand-v4-eve", + networkLogging: "false", + sessionRecording: "false", + ...(personalBrowserEnabled() + ? { + onepasswordExtension: process.env.BROWSERBASE_1PASSWORD_EXTENSION_ID!.trim(), + privateContext: "true", + } + : {}), + }, + }; +} + +export async function launchHostedBrowser(host: HostedBrowserHost, apiKey: string): Promise { + const client = new Browserbase({ apiKey }); + host.browserbase = client; + const persistedSessionId = host.state.browser.sessionId; + + if (persistedSessionId) { + for (let attempt = 1; attempt <= 2; attempt += 1) { + try { + const prior = await client.sessions.retrieve(persistedSessionId); + if (prior.userMetadata?.networkLogging !== "false" || prior.userMetadata?.sessionRecording !== "false") + throw new Error("Session logging and recording settings cannot be verified."); + if ( + personalBrowserEnabled() && + (prior.userMetadata?.onepasswordExtension !== process.env.BROWSERBASE_1PASSWORD_EXTENSION_ID || + prior.userMetadata?.privateContext !== "true") + ) + throw new Error("The personal 1Password browser settings cannot be verified."); + const browser = await stagehandBrowserbase.connect({ + apiKey, + sessionId: persistedSessionId, + }); + await host.attach(browser); + host.remoteSessionId = persistedSessionId; + addTrace( + host.state, + "browser", + "browser.reattach", + "Reattached Stagehand v4 to the saved Browserbase session.", + { sessionId: persistedSessionId, attempt }, + ); + return; + } catch { + await host.discardAttached(); + if (attempt < 2) await new Promise((resolve) => setTimeout(resolve, 500)); + } + } + + host.remoteSessionId = persistedSessionId; + await releaseHostedSession(host); + addTrace( + host.state, + "system", + "browser.replace", + "The saved Browserbase session expired. Bell is starting a replacement with the same Context.", + { + contextPreserved: Boolean(host.contextId ?? process.env.BROWSERBASE_CONTEXT_ID), + }, + ); + } + + let lastError: unknown; + for (let attempt = 1; attempt <= 3; attempt += 1) { + let launched: StagehandBrowser | undefined; + try { + launched = await stagehandBrowserbase.launch( + browserbaseLaunchOptions(apiKey, host.contextId, host.proxyLocation), + ); + await host.attach(launched); + if (!launched.sessionId) throw new Error("Stagehand did not return a Browserbase session ID."); + host.remoteSessionId = launched.sessionId; + host.state.browser.sessionId = launched.sessionId; + return; + } catch (error) { + lastError = error; + await host.discardLaunch(launched); + if (launched?.sessionId) { + host.remoteSessionId = launched.sessionId; + await releaseHostedSession(host); + } + if (attempt < 3) { + addTrace( + host.state, + "system", + "browser.start.retry", + `The Browserbase connection failed. Bell will retry with a new verified, proxied session (${attempt}/3).`, + { attempt, proxies: true, verified: true }, + ); + await new Promise((resolve) => setTimeout(resolve, attempt * 500)); + } + } + } + + throw lastError instanceof Error ? lastError : new Error(String(lastError)); +} + +export async function releaseHostedSession(host: HostedBrowserHost): Promise { + const sessionId = host.remoteSessionId ?? host.state.browser.sessionId; + if (!sessionId) return; + const client = host.browserbase ?? new Browserbase({ apiKey: process.env.BROWSERBASE_API_KEY }); + host.browserbase = client; + try { + await client.sessions.update(sessionId, { status: "REQUEST_RELEASE" }); + } catch (error) { + // An already removed session is released. Keep the ID on every other + // failure so cleanup can retry instead of silently orphaning the browser. + if (!(error instanceof Browserbase.APIError) || error.status !== 404) + throw new Error("The hosted browser could not be released. Retry cleanup."); + } + host.remoteSessionId = undefined; + delete host.state.browser.sessionId; +} diff --git a/server/browser/captcha-gate.ts b/server/browser/captcha-gate.ts new file mode 100644 index 0000000..496ff6a --- /dev/null +++ b/server/browser/captcha-gate.ts @@ -0,0 +1,154 @@ +import type { CDPSubscription, StagehandBrowser } from "@browserbasehq/stagehand"; + +import { + captchaTimeoutMs, + parseBrowserbaseCaptchaEvent, + safeCaptchaPage, + type CaptchaSignal, + type CaptchaTransition, +} from "../../lib/browser/captcha"; +import type { ConversationState } from "./types"; +import { addTrace } from "./trace"; + +export class CaptchaHandoffError extends Error { + readonly code = "captcha_handoff"; + constructor(readonly reason: "errored" | "timed_out") { + super( + reason === "errored" + ? "CAPTCHA solving errored. Continue in Live View, then resume this task." + : "CAPTCHA solving timed out. Continue in Live View, then resume this task.", + ); + } +} + +export interface CaptchaObservation { + snapshot: { formattedTree: string; xpathMap: Record }; + url: string; + title: string; +} + +interface CaptchaGateHost { + state: ConversationState; + browser(): StagehandBrowser | undefined; + onCaptchaTransition?: (transition: CaptchaTransition) => void; + observePage(): Promise; + applyPage(xpathMap: Record, url: string, title: string): void; +} + +/** Tracks Browserbase CAPTCHA console events and blocks page work until a solve finishes or hands off. */ +export class CaptchaGate { + readonly subscriptions = new Map(); + private status: "idle" | "solving" | "solved" | "errored" | "timed_out"; + private startedAt?: string; + private needsObservation = false; + + constructor( + private readonly host: CaptchaGateHost, + captcha?: { status: CaptchaGate["status"]; startedAt?: string }, + ) { + this.status = captcha?.status ?? "idle"; + this.startedAt = captcha?.startedAt; + } + + resumeAfterHumanInput(): void { + if (this.status !== "errored" && this.status !== "timed_out") return; + this.status = "idle"; + this.needsObservation = true; + } + + async ensureSubscriptions(): Promise { + const browser = this.host.browser(); + if (!browser) return; + for (const page of await browser.context.pages()) { + if (this.subscriptions.has(page.pageId)) continue; + const subscription = await page.on("console", (event) => { + const signal = parseBrowserbaseCaptchaEvent(event); + if (signal) this.recordSignal(signal); + }); + this.subscriptions.set(page.pageId, subscription); + } + } + + async clearSubscriptions(): Promise { + const subscriptions = [...this.subscriptions.values()]; + this.subscriptions.clear(); + await Promise.all( + subscriptions.map((subscription) => subscription.unsubscribe().catch(() => undefined)), + ); + } + + async gate(): Promise { + await this.ensureSubscriptions(); + if (this.status === "solving") { + const started = Date.parse(this.startedAt ?? new Date().toISOString()); + while (this.status === "solving" && Date.now() - started < captchaTimeoutMs()) + await new Promise((resolve) => setTimeout(resolve, 25)); + if (this.status === "solving") { + const at = new Date().toISOString(); + this.status = "timed_out"; + const page = safeCaptchaPage(this.host.state.browser.url); + this.emit("timed_out", at); + addTrace( + this.host.state, + "error", + "captcha.timed_out", + "CAPTCHA solving timed out; human input is required.", + page, + ); + } + } + if (this.status === "errored" || this.status === "timed_out") + throw new CaptchaHandoffError(this.status); + if (this.needsObservation) { + this.needsObservation = false; + const result = await this.host.observePage(); + this.host.applyPage(result.snapshot.xpathMap, result.url, result.title); + addTrace( + this.host.state, + "browser", + "captcha.observed", + "Read the post-CAPTCHA page after the current gate.", + { + page: safeCaptchaPage(result.url), + treePreview: result.snapshot.formattedTree.slice(0, 800), + }, + ); + } + } + + private emit(type: CaptchaTransition["type"], at: string): void { + this.host.onCaptchaTransition?.({ + id: crypto.randomUUID(), + type, + at, + page: safeCaptchaPage(this.host.state.browser.url), + }); + } + + private recordSignal(signal: CaptchaSignal): void { + const at = new Date().toISOString(); + const page = safeCaptchaPage(this.host.state.browser.url); + if (signal === "started") { + if (this.status === "solving") return; + this.status = "solving"; + this.startedAt = at; + this.emit("started", at); + addTrace(this.host.state, "browser", "captcha.started", "Browserbase is solving a CAPTCHA.", page); + } else if (signal === "finished" && this.status === "solving") { + this.status = "solved"; + this.needsObservation = true; + this.emit("finished", at); + addTrace(this.host.state, "browser", "captcha.finished", "Browserbase solved the CAPTCHA.", page); + } else if (signal === "errored" && this.status === "solving") { + this.status = "errored"; + this.emit("errored", at); + addTrace( + this.host.state, + "error", + "captcha.errored", + "Browserbase CAPTCHA solving errored; human input is required.", + page, + ); + } + } +} diff --git a/tests/browser/captcha-live.test.ts b/tests/browser/captcha-live.test.ts index 5e1d61b..84c1b97 100644 --- a/tests/browser/captcha-live.test.ts +++ b/tests/browser/captcha-live.test.ts @@ -49,8 +49,8 @@ describe("opt-in hosted CAPTCHA marker fixture", () => { await session.close(); } expect( - (session as unknown as { captchaSubscriptions: Map }) - .captchaSubscriptions.size, + (session as unknown as { captchaGate: { subscriptions: Map } }) + .captchaGate.subscriptions.size, ).toBe(0); } }, From 26f5fd3d8053d96efbba8fb9b7be8553de0ba876 Mon Sep 17 00:00:00 2001 From: Matthew Nanas <37747855+matthewnanas@users.noreply.github.com> Date: Tue, 3 Nov 2026 12:03:37 -0800 Subject: [PATCH 7/8] Fix review regressions in CAPTCHA, cancellation, and 1Password guards. - Queue CAPTCHA transitions and commit them inside the running operation. - Keep the private-browser guard and reattach checks independent of the rollout flag. - Re-check cancellation after taking the operation lock, before starting the browser. - Keep secure-login start and CAPTCHA failures redacted. - Restore the monitor pool's 10s idle timeout and drop an unused re-export. Co-Authored-By: Claude Opus 5.5 --- agent/lib/browser-runtime.ts | 37 +++++++++++++++++++++++-- agent/lib/task-state.ts | 2 -- server/browser/browser-session.ts | 20 +++++++++---- server/browser/browserbase-launch.ts | 13 +++++---- server/browser/captcha-gate.ts | 1 - server/link/store.ts | 3 -- server/monitors/store.ts | 2 +- server/onepassword/browser-policy.ts | 7 +++-- server/onepassword/store.ts | 1 - server/postgres.ts | 1 - tests/browser/browser-recording.test.ts | 33 ++++++++++++++++++++++ tests/browser/browser-security.test.ts | 8 ++++++ tests/browser/captcha-recovery.test.ts | 25 +++++++++++++++++ tests/browser/hosted-page.test.ts | 24 ++++++++++++++++ tests/onepassword-availability.test.ts | 8 +++++- 15 files changed, 159 insertions(+), 26 deletions(-) diff --git a/agent/lib/browser-runtime.ts b/agent/lib/browser-runtime.ts index ef36cc3..57e8ae6 100644 --- a/agent/lib/browser-runtime.ts +++ b/agent/lib/browser-runtime.ts @@ -1,3 +1,4 @@ +import type { CaptchaTransition } from "../../lib/browser/captcha"; import { personalBrowserEnabled, withPersonalBrowserLock } from "../../server/onepassword/browser-policy"; import { createAgentContext, @@ -27,6 +28,7 @@ type BrowserOperation = (browser: BrowserSession) => Promise; interface BrowserRuntime { browser: BrowserSession; state: ConversationState; + captchaTransitions: CaptchaTransition[]; idleTimer?: ReturnType; closing?: Promise; } @@ -65,11 +67,14 @@ function getRuntime(sessionId: string): BrowserRuntime { title: durable.browser.title, }, }; + const captchaTransitions: CaptchaTransition[] = []; const runtime = { state, + captchaTransitions, browser: createRuntimeBrowser( sessionId, state, + captchaTransitions, durable.browser.contextId, durable.browser.proxyLocation, ), @@ -92,9 +97,23 @@ async function runBrowserOperationUnlocked(sessionId: string, operation: Brow const runtime = getRuntime(sessionId); scheduleIdleClose(sessionId, runtime); const traceStart = runtime.state.traces.length; + let settled = false; + const pending = operation(runtime.browser).finally(() => { + settled = true; + }); + while (!settled) { + await Promise.race([ + pending.then( + () => undefined, + () => undefined, + ), + new Promise((resolve) => setTimeout(resolve, 50)), + ]); + commitCaptchaQueue(runtime); + } let result: T; try { - result = await operation(runtime.browser); + result = await pending; } catch (error) { if (!(error instanceof CaptchaHandoffError)) throw error; result = { @@ -103,6 +122,7 @@ async function runBrowserOperationUnlocked(sessionId: string, operation: Brow action: "Complete the challenge in the existing Live View, then resume this task.", } as T; } + commitCaptchaQueue(runtime); scheduleIdleClose(sessionId, runtime); return { result, @@ -213,6 +233,7 @@ export async function createBrowserContext( runtime.browser = createRuntimeBrowser( sessionId, runtime.state, + runtime.captchaTransitions, context.id, runtime.state.browser.proxyLocation, ); @@ -245,6 +266,7 @@ export async function selectBrowserContext( runtime.browser = createRuntimeBrowser( sessionId, runtime.state, + runtime.captchaTransitions, contextId, runtime.state.browser.proxyLocation, ); @@ -293,9 +315,20 @@ export async function saveBrowserContext( }; } +function commitCaptchaQueue(runtime: BrowserRuntime): void { + if (!runtime.captchaTransitions.length) return; + const transitions = runtime.captchaTransitions.splice(0); + taskState.update((current) => { + let next = current; + for (const transition of transitions) next = applyCaptchaTransition(next, transition); + return updateTaskBrowser(next, runtime.state.browser); + }); +} + function createRuntimeBrowser( sessionId: string, state: ConversationState, + captchaTransitions: CaptchaTransition[], contextId?: string, proxyLocation?: BrowserProxyLocation, ): BrowserSession { @@ -304,7 +337,7 @@ function createRuntimeBrowser( proxyLocation, captcha: taskState.get().captcha, onCaptchaTransition: (transition) => { - taskState.update((current) => updateTaskBrowser(applyCaptchaTransition(current, transition), state.browser)); + captchaTransitions.push(transition); }, ensureContext: personalBrowserEnabled() ? undefined : async (currentContextId) => { const context = await ensureAgentContext(sessionId, currentContextId); diff --git a/agent/lib/task-state.ts b/agent/lib/task-state.ts index 024efd9..79232e5 100644 --- a/agent/lib/task-state.ts +++ b/agent/lib/task-state.ts @@ -3,8 +3,6 @@ import { defineState } from "eve/context"; import { captchaTimeoutMs, type CaptchaTransition } from "../../lib/browser/captcha"; import type { BrowserProxyLocation, BrowserState } from "../../server/browser/types.js"; -export { captchaTimeoutMs, type CaptchaTransition }; - export type TaskStatus = "idle" | "running" | "solving_captcha" | "waiting_for_user" | "complete" | "failed" | "canceled"; export type CaptchaStatus = "idle" | "solving" | "solved" | "errored" | "timed_out"; diff --git a/server/browser/browser-session.ts b/server/browser/browser-session.ts index 77c78d0..8afb03f 100644 --- a/server/browser/browser-session.ts +++ b/server/browser/browser-session.ts @@ -189,7 +189,7 @@ export class BrowserSession { await this.gateCaptcha(); } return result; - }); + }, signal); } async snapshot(): Promise<{ @@ -347,8 +347,10 @@ export class BrowserSession { input.totpTarget, input.otpSubmitTarget, input.successTarget]; if (selectors.some((selector) => selector && (/^\s*(?:xpath=|\/|\()/i.test(selector) || selector.includes(">>")))) throw new Error("Secure login requires CSS selectors in the top-level page; frame selectors are not allowed."); - return this.withPage(async () => { + return this.withLock(async () => { try { + await this.start(); + await this.gateCaptcha(); const started = performance.now(); const result = await this.requireStagehand().experimentalBatch( async ({ page }, values) => { @@ -530,18 +532,24 @@ export class BrowserSession { return this.stagehand; } - private async withPage(operation: () => Promise): Promise { + private async withPage(operation: () => Promise, signal?: AbortSignal): Promise { return this.withOperation(async () => { await this.gateCaptcha(); return operation(); + }, signal); + } + + private async withOperation(operation: () => Promise, signal?: AbortSignal): Promise { + return this.withLock(async () => { + signal?.throwIfAborted(); + await this.start(); + return operation(); }); } - /** Serialize browser work. Cookie import starts the session without the CAPTCHA gate. */ - private async withOperation(operation: () => Promise): Promise { + private async withLock(operation: () => Promise): Promise { const release = await this.acquireOperation(); try { - await this.start(); return await operation(); } finally { release(); diff --git a/server/browser/browserbase-launch.ts b/server/browser/browserbase-launch.ts index 3fd0daf..53a10b4 100644 --- a/server/browser/browserbase-launch.ts +++ b/server/browser/browserbase-launch.ts @@ -5,7 +5,6 @@ import { personalBrowserEnabled } from "../onepassword/browser-policy"; import { addTrace } from "./trace"; import type { BrowserProxyLocation, ConversationState } from "./types"; -/** The session fields a hosted launch is allowed to change. */ export interface HostedBrowserHost { readonly state: ConversationState; readonly contextId?: string; @@ -77,10 +76,13 @@ export async function launchHostedBrowser(host: HostedBrowserHost, apiKey: strin const prior = await client.sessions.retrieve(persistedSessionId); if (prior.userMetadata?.networkLogging !== "false" || prior.userMetadata?.sessionRecording !== "false") throw new Error("Session logging and recording settings cannot be verified."); + const personalSession = + prior.userMetadata?.privateContext === "true" || Boolean(prior.userMetadata?.onepasswordExtension); if ( - personalBrowserEnabled() && - (prior.userMetadata?.onepasswordExtension !== process.env.BROWSERBASE_1PASSWORD_EXTENSION_ID || - prior.userMetadata?.privateContext !== "true") + personalBrowserEnabled() + ? prior.userMetadata?.onepasswordExtension !== process.env.BROWSERBASE_1PASSWORD_EXTENSION_ID || + prior.userMetadata?.privateContext !== "true" + : personalSession ) throw new Error("The personal 1Password browser settings cannot be verified."); const browser = await stagehandBrowserbase.connect({ @@ -159,8 +161,7 @@ export async function releaseHostedSession(host: HostedBrowserHost): Promise, url: string, title: string): void; } -/** Tracks Browserbase CAPTCHA console events and blocks page work until a solve finishes or hands off. */ export class CaptchaGate { readonly subscriptions = new Map(); private status: "idle" | "solving" | "solved" | "errored" | "timed_out"; diff --git a/server/link/store.ts b/server/link/store.ts index 806b0de..33ebef7 100644 --- a/server/link/store.ts +++ b/server/link/store.ts @@ -103,9 +103,6 @@ export class LinkStore { } async withOwner(owner: LinkOwner, operation: (store: OwnerStore) => Promise): Promise { - // A session lock spans independently committed writes around remote OAuth calls. - // In-progress markers must survive process failure; a transaction rollback would - // otherwise allow a single-use authorization code or refresh token to be retried. return withAdvisoryLock(this.pool, ownerKey(owner), (client) => operation(new OwnerStore(client, owner))); } } diff --git a/server/monitors/store.ts b/server/monitors/store.ts index 7618a35..dde66d9 100644 --- a/server/monitors/store.ts +++ b/server/monitors/store.ts @@ -198,7 +198,7 @@ export class MonitorStore { let singleton: MonitorStore | undefined; export function monitorStore(): MonitorStore { if (!singleton) { - singleton = new MonitorStore(createPool(required("DATABASE_URL"), "Monitor", { connectionTimeoutMillis: 3_000, statement_timeout: 5_000 })); + singleton = new MonitorStore(createPool(required("DATABASE_URL"), "Monitor", { connectionTimeoutMillis: 3_000, idleTimeoutMillis: 10_000, statement_timeout: 5_000 })); } return singleton; } diff --git a/server/onepassword/browser-policy.ts b/server/onepassword/browser-policy.ts index d74a068..1c19784 100644 --- a/server/onepassword/browser-policy.ts +++ b/server/onepassword/browser-policy.ts @@ -9,15 +9,18 @@ export async function markSecretFill(blocked: boolean): Promise { if (blocked) await lock.client.query("INSERT INTO bell_onepassword_browser_guards(session_id) VALUES($1) ON CONFLICT DO NOTHING", [lock.sessionId]); else await lock.client.query("DELETE FROM bell_onepassword_browser_guards WHERE session_id=$1", [lock.sessionId]); } +function extensionConfigured(): boolean { + return Boolean(process.env.BROWSERBASE_1PASSWORD_EXTENSION_ID?.trim()); +} export function personalBrowserEnabled(): boolean { - return personalConnectionsEnabled() && Boolean(process.env.BROWSERBASE_1PASSWORD_EXTENSION_ID?.trim()); + return personalConnectionsEnabled() && extensionConfigured(); } export function requireReusableContexts(): void { if (personalBrowserEnabled()) throw new Error("Reusable browser Contexts are disabled for personal 1Password. Start a new private conversation instead."); } let pool: ReturnType | undefined; export async function withPersonalBrowserLock(sessionId: string, operation: () => Promise): Promise { - if (!personalBrowserEnabled()) return operation(); + if (!extensionConfigured()) return operation(); if (!process.env.DATABASE_URL) throw new Error("Personal 1Password requires DATABASE_URL."); pool ??= createPool(process.env.DATABASE_URL, "Private browser", { allowExitOnIdle: true }); return withAdvisoryLock(pool, `bell-private-browser:${sessionId}`, async (client) => { diff --git a/server/onepassword/store.ts b/server/onepassword/store.ts index 9e47dd6..6d6c578 100644 --- a/server/onepassword/store.ts +++ b/server/onepassword/store.ts @@ -51,7 +51,6 @@ export class OnePasswordStore { return (await this.pool.query<{ owner: string }>("SELECT owner FROM bell_onepassword_tickets WHERE hash=$1 AND expires_at>now()", [hash])).rows[0]?.owner; } async withOwner(owner: string, operation: (db: RecordStore) => Promise): Promise { - // The session lock spans committed crash markers around single-use token exchanges. return withAdvisoryLock(this.pool, `onepassword:${owner}`, (client) => operation(new RecordStore(client, owner, this.key))); } } diff --git a/server/postgres.ts b/server/postgres.ts index 7531e15..be68883 100644 --- a/server/postgres.ts +++ b/server/postgres.ts @@ -12,7 +12,6 @@ export function createPool(connectionString: string, label: string, options: Poo return pool; } -/** Hold a session advisory lock. A failed lock or unlock destroys the client instead of returning it to the pool. */ export async function withAdvisoryLock(pool: Pool, key: string, operation: (client: PoolClient) => Promise): Promise { const client = await pool.connect(); let locked = false; diff --git a/tests/browser/browser-recording.test.ts b/tests/browser/browser-recording.test.ts index 455b7aa..17b057b 100644 --- a/tests/browser/browser-recording.test.ts +++ b/tests/browser/browser-recording.test.ts @@ -61,6 +61,39 @@ describe("hosted recording privacy", () => { await browser.close(); }); + it("replaces a personal browser when the rollout flag is off", async () => { + vi.useFakeTimers(); + vi.stubEnv("ONEPASSWORD_ENABLED", "false"); + vi.stubEnv("BROWSERBASE_1PASSWORD_EXTENSION_ID", "synthetic-extension"); + mocks.retrieve.mockResolvedValue({ + userMetadata: { + networkLogging: "false", + sessionRecording: "false", + privateContext: "true", + onepasswordExtension: "synthetic-extension", + }, + }); + const remote = { + sessionId: "replacement-browser", closed: false, close: vi.fn(async () => {}), + context: { pages: async () => [] }, + }; + vi.spyOn(browserbase, "launch").mockResolvedValue(remote as never); + const connect = vi.spyOn(browserbase, "connect"); + vi.spyOn(Stagehand, "create").mockResolvedValue({ close: async () => {} } as never); + const state: ConversationState = { + id: "personal-flag-off", traces: [], + browser: { provider: "browserbase", status: "idle", sessionId: "previous-browser" }, + }; + const browser = new BrowserSession(state); + const started = browser.start(); + await vi.runAllTimersAsync(); + await started; + expect(connect).not.toHaveBeenCalled(); + expect(mocks.update).toHaveBeenCalledWith("previous-browser", { status: "REQUEST_RELEASE" }); + expect(state.browser.sessionId).toBe("replacement-browser"); + await browser.close(); + }); + it.each(unsafeMetadata)("refuses handoff to unsafe privacy markers: %j", async userMetadata => { mocks.retrieve.mockResolvedValue({ userMetadata }); const token = issueBrowserHandoffToken({ eveSessionId: "eve-session", browserSessionId: "browser-session" }, { secret }); diff --git a/tests/browser/browser-security.test.ts b/tests/browser/browser-security.test.ts index 6b25ebd..7378fba 100644 --- a/tests/browser/browser-security.test.ts +++ b/tests/browser/browser-security.test.ts @@ -109,6 +109,14 @@ describe("vault login credential boundary", () => { } }); + it("redacts a browser start failure before the fill begins", async () => { + const { session, state } = fixture(); + vi.spyOn(session, "start").mockRejectedValue(new Error(login.password)); + await expect(session.secureLogin(login)).rejects.toThrow("Secure login failed"); + expect(JSON.stringify(state)).not.toContain(login.password); + expect(state.traces).toContainEqual(expect.objectContaining({ name: "vault.login", kind: "error" })); + }); + it("redacts failures and releases the operation lock for the next attempt", async () => { const { session, state, fill } = fixture(); fill.mockRejectedValueOnce(new Error(login.password)); diff --git a/tests/browser/captcha-recovery.test.ts b/tests/browser/captcha-recovery.test.ts index 5188695..9f3e3e1 100644 --- a/tests/browser/captcha-recovery.test.ts +++ b/tests/browser/captcha-recovery.test.ts @@ -69,6 +69,31 @@ describe("resuming a browser after CAPTCHA handoff", () => { expect(taskState.get()).toMatchObject({ status: "waiting_for_user", captcha: { status: "errored" } }); })); + it("records a listener event on the turn that is running", () => + contextStorage.run(new ContextContainer(), async () => { + const id = "captcha-active-turn"; + const other = new ContextContainer(); + taskState.update(current => ({ ...current, sessionId: id })); + try { + await runBrowserOperation(id, async browser => { + const notify = (browser as unknown as { + options: { onCaptchaTransition?: (transition: { id: string; type: "started" | "errored"; at: string }) => void }; + }).options.onCaptchaTransition; + const at = new Date().toISOString(); + contextStorage.run(other, () => { + notify?.({ id: "challenge-start", type: "started", at }); + notify?.({ id: "challenge-error", type: "errored", at }); + }); + }); + expect(taskState.get()).toMatchObject({ status: "waiting_for_user", captcha: { status: "errored" } }); + contextStorage.run(other, () => { + expect(taskState.get().captcha).toBeUndefined(); + }); + } finally { + await closeBrowserRuntime(id); + } + })); + it("preserves CAPTCHA handoff errors from secure login", async () => { const browser = new BrowserSession({ id: "login-handoff", traces: [], browser: { provider: "not-started", status: "idle" }, diff --git a/tests/browser/hosted-page.test.ts b/tests/browser/hosted-page.test.ts index 5b60c3d..372b280 100644 --- a/tests/browser/hosted-page.test.ts +++ b/tests/browser/hosted-page.test.ts @@ -59,6 +59,30 @@ describe("shared hosted-page integration boundary", () => { expect((session as unknown as { gateCaptcha: unknown }).gateCaptcha).toHaveBeenCalledTimes(1); expect(state.browser.url).toBeUndefined(); }); + it("does not launch a browser for a call cancelled while waiting for the lock", async () => { + const { session } = fixture(); + let releaseStart!: () => void; + const started = new Promise((resolve) => { + releaseStart = resolve; + }); + let entered!: () => void; + const startEntered = new Promise((resolve) => { + entered = resolve; + }); + const start = vi.spyOn(session, "start").mockImplementation(async () => { + entered(); + await started; + }); + const first = session.withHostedPage(new AbortController().signal, async () => "first"); + await startEntered; + const controller = new AbortController(); + const second = session.withHostedPage(controller.signal, async () => "second"); + controller.abort(); + releaseStart(); + await expect(first).resolves.toBe("first"); + await expect(second).rejects.toMatchObject({ name: "AbortError" }); + expect(start).toHaveBeenCalledTimes(1); + }); it("does no work for an already-cancelled call", async () => { const { session } = fixture(); const operation = vi.fn(); diff --git a/tests/onepassword-availability.test.ts b/tests/onepassword-availability.test.ts index c8b0c97..361b0c4 100644 --- a/tests/onepassword-availability.test.ts +++ b/tests/onepassword-availability.test.ts @@ -8,7 +8,7 @@ import status from "../agent/tools/onepassword_status"; import disconnect from "../agent/tools/onepassword_disconnect"; import { onePasswordGet, onePasswordPost } from "../server/onepassword/http"; import { browserbaseLaunchOptions } from "../server/browser/browser-session"; -import { requireReusableContexts } from "../server/onepassword/browser-policy"; +import { requireReusableContexts, withPersonalBrowserLock } from "../server/onepassword/browser-policy"; describe("personal 1Password rollout", () => { let ctx: ToolContext; @@ -64,4 +64,10 @@ describe("personal 1Password rollout", () => { expect(browserbaseLaunchOptions("test-key").extensionId).toBe("previous-private-extension"); expect(requireReusableContexts).toThrow(); }); + it("still checks the private-browser guard when the rollout flag is off", async () => { + vi.stubEnv("ONEPASSWORD_ENABLED", "false"); + vi.stubEnv("BROWSERBASE_1PASSWORD_EXTENSION_ID", "previous-private-extension"); + vi.stubEnv("DATABASE_URL", ""); + await expect(withPersonalBrowserLock("conversation", async () => "ran")).rejects.toThrow("DATABASE_URL"); + }); }); From 8505f716a705331b5c59a8c6adebe8baf3b4df13 Mon Sep 17 00:00:00 2001 From: Matthew Nanas <37747855+matthewnanas@users.noreply.github.com> Date: Tue, 3 Nov 2026 12:08:50 -0800 Subject: [PATCH 8/8] Merge the hosted-browser discard helpers and pass the CAPTCHA page into emit. Co-Authored-By: Claude Opus 5.5 --- server/browser/browser-session.ts | 12 ++---------- server/browser/browserbase-launch.ts | 8 ++++---- server/browser/captcha-gate.ts | 17 ++++++----------- 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/server/browser/browser-session.ts b/server/browser/browser-session.ts index 8afb03f..5d1bf4a 100644 --- a/server/browser/browser-session.ts +++ b/server/browser/browser-session.ts @@ -631,19 +631,11 @@ export class BrowserSession { writeRemoteSessionId(sessionId); }, attach: (browser) => this.attachStagehand(browser), - discardAttached: () => this.discardAttached(), - discardLaunch: (browser) => this.discardLaunch(browser), + discard: (browser) => this.discard(browser), }; } - private async discardAttached(): Promise { - await this.stagehand?.close().catch(() => undefined); - await this.browser?.close().catch(() => undefined); - this.stagehand = undefined; - this.browser = undefined; - } - - private async discardLaunch(browser: StagehandBrowser | undefined): Promise { + private async discard(browser = this.browser): Promise { await this.stagehand?.close().catch(() => undefined); await browser?.close().catch(() => undefined); this.stagehand = undefined; diff --git a/server/browser/browserbase-launch.ts b/server/browser/browserbase-launch.ts index 53a10b4..81a8ae6 100644 --- a/server/browser/browserbase-launch.ts +++ b/server/browser/browserbase-launch.ts @@ -12,8 +12,8 @@ export interface HostedBrowserHost { browserbase?: Browserbase; remoteSessionId?: string; attach(browser: StagehandBrowser): Promise; - discardAttached(): Promise; - discardLaunch(browser: StagehandBrowser | undefined): Promise; + /** Close Stagehand and the given browser (the attached one by default) and clear both. */ + discard(browser?: StagehandBrowser): Promise; } export function browserProvider(): "local" | "browserbase" { @@ -100,7 +100,7 @@ export async function launchHostedBrowser(host: HostedBrowserHost, apiKey: strin ); return; } catch { - await host.discardAttached(); + await host.discard(); if (attempt < 2) await new Promise((resolve) => setTimeout(resolve, 500)); } } @@ -132,7 +132,7 @@ export async function launchHostedBrowser(host: HostedBrowserHost, apiKey: strin return; } catch (error) { lastError = error; - await host.discardLaunch(launched); + await host.discard(launched); if (launched?.sessionId) { host.remoteSessionId = launched.sessionId; await releaseHostedSession(host); diff --git a/server/browser/captcha-gate.ts b/server/browser/captcha-gate.ts index 392565a..ecab9a5 100644 --- a/server/browser/captcha-gate.ts +++ b/server/browser/captcha-gate.ts @@ -86,7 +86,7 @@ export class CaptchaGate { const at = new Date().toISOString(); this.status = "timed_out"; const page = safeCaptchaPage(this.host.state.browser.url); - this.emit("timed_out", at); + this.emit("timed_out", at, page); addTrace( this.host.state, "error", @@ -115,13 +115,8 @@ export class CaptchaGate { } } - private emit(type: CaptchaTransition["type"], at: string): void { - this.host.onCaptchaTransition?.({ - id: crypto.randomUUID(), - type, - at, - page: safeCaptchaPage(this.host.state.browser.url), - }); + private emit(type: CaptchaTransition["type"], at: string, page: CaptchaTransition["page"]): void { + this.host.onCaptchaTransition?.({ id: crypto.randomUUID(), type, at, page }); } private recordSignal(signal: CaptchaSignal): void { @@ -131,16 +126,16 @@ export class CaptchaGate { if (this.status === "solving") return; this.status = "solving"; this.startedAt = at; - this.emit("started", at); + this.emit("started", at, page); addTrace(this.host.state, "browser", "captcha.started", "Browserbase is solving a CAPTCHA.", page); } else if (signal === "finished" && this.status === "solving") { this.status = "solved"; this.needsObservation = true; - this.emit("finished", at); + this.emit("finished", at, page); addTrace(this.host.state, "browser", "captcha.finished", "Browserbase solved the CAPTCHA.", page); } else if (signal === "errored" && this.status === "solving") { this.status = "errored"; - this.emit("errored", at); + this.emit("errored", at, page); addTrace( this.host.state, "error",