diff --git a/packages/sdk/src/mcp/automate.test.ts b/packages/sdk/src/mcp/automate.test.ts index 1723a4a..b0289ee 100644 --- a/packages/sdk/src/mcp/automate.test.ts +++ b/packages/sdk/src/mcp/automate.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; import type { AgentSnapshot } from '@tronbrowser/browser-core'; -import { automateTools, fetchPage, markdownExpression, parseFetchArgs, wallReason, engineStatus } from './automate.js'; +import { automateTools, exclusive, fetchPage, markdownExpression, parseFetchArgs, wallReason, engineStatus } from './automate.js'; import { ObscuraClient, type ObscuraProcess } from './obscura.js'; import { McpBrowserSession, type McpPage } from './session.js'; import { createMcpServer } from './server.js'; @@ -110,6 +110,44 @@ describe('wallReason', () => { }); }); +describe('fetchPage concurrency', () => { + it('concurrent fetches each get their own page, never the tab another fetch navigated to', async () => { + // One shared tab, like the real Obscura: navigate sets it after a delay, markdown reads whatever it is now. + let tab = ''; + const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + const obscura = { + available: () => true, + call: async (name: string, a: Record) => { + if (name === 'browser_navigate') { + await sleep(String(a.url).includes('slow') ? 30 : 5); + tab = String(a.url); + return { content: [{ type: 'text', text: 'ok' }] }; + } + await sleep(10); + return { content: [{ type: 'text', text: `# Page at ${tab}\n\n${LONG}` }] }; + }, + } as unknown as ObscuraClient; + const { session } = fakePage(); + const urls = ['https://a.example/slow', 'https://b.example/', 'https://c.example/']; + const out = await Promise.all(urls.map((url) => fetchPage({ obscura, session }, parseFetchArgs({ url })))); + out.forEach((r, i) => { + expect(r.outcome.engine).toBe('obscura'); + expect(r.text).toContain(`# Page at ${urls[i]}`); + }); + }); + + it('a failed fetch does not block the next one', async () => { + const order: string[] = []; + await expect(exclusive(fakePage().session, async () => { order.push('a'); throw new Error('boom'); })).rejects.toThrow('boom'); + expect(await exclusive(fakePage().session, async () => (order.push('b'), 'ok'))).toBe('ok'); + const key = {}; + const first = exclusive(key, async () => { throw new Error('x'); }); + const second = exclusive(key, async () => 'after'); + await expect(first).rejects.toThrow('x'); + expect(await second).toBe('after'); + }); +}); + describe('fetchPage routing', () => { it('uses Obscura when it answers with real content', async () => { const { client, calls } = fakeObscura({ browser_markdown: { text: '# Example\n\n' + LONG } }); diff --git a/packages/sdk/src/mcp/automate.ts b/packages/sdk/src/mcp/automate.ts index 98926af..c7325b0 100644 --- a/packages/sdk/src/mcp/automate.ts +++ b/packages/sdk/src/mcp/automate.ts @@ -154,7 +154,26 @@ export function markdownExpression(): string { })()`; } +/** + * One fetch at a time per browser. Each engine drives ONE shared tab: navigate, then read. Two + * fetches in flight interleave (A navigates, B navigates, A reads B's page), and every caller got + * the last page's text while the status line still named its own URL. So navigate+read is one + * critical section per engine, keyed by the browser object. Obscura and Chromium are separate + * locks, so an Obscura read and a Chromium fallback can still overlap. + */ +const tails = new WeakMap>(); +export function exclusive(key: object, run: () => Promise): Promise { + const before = tails.get(key) ?? Promise.resolve(); + const mine = before.then(run, run); + tails.set(key, mine.catch(() => undefined)); + return mine; +} + async function viaObscura(obscura: ObscuraClient, args: FetchArgs): Promise { + return exclusive(obscura, () => obscuraOnce(obscura, args)); +} + +async function obscuraOnce(obscura: ObscuraClient, args: FetchArgs): Promise { const nav = await obscura.call('browser_navigate', { url: args.url, waitUntil: 'load' }, args.budgetMs); if (nav.isError) throw new Error(resultText(nav) || 'navigate failed'); let result; @@ -231,8 +250,7 @@ export async function fetchPage(deps: AutomateDeps, args: FetchArgs): Promise<{ if (args.engine === 'obscura') throw new Error(fellBack); } - const page = await deps.session.getPage(); - const text = await viaChromium(page, args); + const text = await exclusive(deps.session, async () => viaChromium(await deps.session.getPage(), args)); return finish('chromium', text, fellBack); }