From 979552aa5ca09fe63459b2ebf7a7a33511eb1fcd Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 11 Nov 2026 17:14:37 +0800 Subject: [PATCH] fix(browser): retain initialization through cancelled startup (#10045) fix(browser-scenario): retain startup work through cancellation --- src/browser/PlaywrightScenarioBrowser.test.ts | 86 +++++++++++++++++++ src/browser/PlaywrightScenarioBrowser.ts | 5 ++ src/browser/PlaywrightScenarioSession.test.ts | 58 +++++++++++++ src/browser/PlaywrightScenarioSession.ts | 21 ++++- 4 files changed, 166 insertions(+), 4 deletions(-) diff --git a/src/browser/PlaywrightScenarioBrowser.test.ts b/src/browser/PlaywrightScenarioBrowser.test.ts index 21ef9806b7..20854534dc 100644 --- a/src/browser/PlaywrightScenarioBrowser.test.ts +++ b/src/browser/PlaywrightScenarioBrowser.test.ts @@ -187,6 +187,92 @@ const launchScenario = browserScenarioSchema.parse({ actions: [{ step_id: "settle", action: "wait_for_timeout", duration_ms: 1 }], }); +it("does not launch a browser for an already-cancelled opening", async () => { + const controller = new AbortController(); + const reason = { caller: "cancelled before browser acquisition" }; + controller.abort(reason); + const launch = vi.fn(async () => { + throw new Error("unexpected launch after cancellation"); + }); + await expect( + openPlaywrightScenarioBrowser( + launchScenario, + {}, + { + signal: controller.signal, + launcher: { + launchPersistentContext: launch, + connectOverCDP: async () => { + throw new Error("Unexpected connection"); + }, + }, + }, + ), + ).rejects.toMatchObject({ reason: "cancelled", cause: reason }); + expect(launch).not.toHaveBeenCalled(); +}); + +it.each([false, true])( + "admits a late launch only while its caller is active (cancelled: %s)", + async (cancelled) => { + const controller = new AbortController(); + const reason = { caller: "cancelled during browser launch" }; + const started = deferred(); + const launched = deferred(); + let profile = ""; + const close = vi.fn(async () => undefined); + const newPage = vi.fn(async () => ({}) as Page); + const opening = openPlaywrightScenarioBrowser( + launchScenario, + {}, + { + signal: controller.signal, + launcher: { + launchPersistentContext: (directory) => { + profile = directory; + started.resolve(undefined); + return launched.promise; + }, + connectOverCDP: async () => { + throw new Error("Unexpected connection"); + }, + }, + }, + ).then( + (value) => ({ ok: true as const, value }), + (cause: unknown) => ({ ok: false as const, cause }), + ); + await started.promise; + if (cancelled) controller.abort(reason); + launched.resolve({ + close, + browser: () => ({}) as Browser, + pages: () => [], + newPage, + } as unknown as BrowserContext); + const result = await opening; + if (result.ok) { + try { + expect(cancelled).toBe(false); + expect(newPage).toHaveBeenCalledOnce(); + expect(close).not.toHaveBeenCalled(); + expect(result.value.profilePath).toBe(profile); + } finally { + await result.value.cleanup.close(); + } + } else { + expect(cancelled).toBe(true); + expect(result.cause).toMatchObject({ reason: "cancelled" }); + if (!(result.cause instanceof AnalysisError)) + throw new Error("Expected typed cancellation"); + expect(result.cause.cause).toBe(reason); + expect(newPage).not.toHaveBeenCalled(); + } + expect(close).toHaveBeenCalledOnce(); + await expect(access(profile)).rejects.toMatchObject({ code: "ENOENT" }); + }, +); + it.each([ { environment: {}, sandbox: true }, { environment: { REA_BROWSER_NO_SANDBOX: "false" }, sandbox: true }, diff --git a/src/browser/PlaywrightScenarioBrowser.ts b/src/browser/PlaywrightScenarioBrowser.ts index 9b525daffe..34201ad353 100644 --- a/src/browser/PlaywrightScenarioBrowser.ts +++ b/src/browser/PlaywrightScenarioBrowser.ts @@ -855,6 +855,7 @@ export const openPlaywrightScenarioBrowser = async ( readonly retainCleanup?: (lease: AnalysisCleanupLease) => void; } = {}, ): Promise => { + throwIfScenarioCancelled(options.signal); // Load Playwright only when a scenario opens; MCP startup stays off its module graph. const launcher = options.launcher ?? (await import("playwright-core")).chromium; @@ -869,6 +870,7 @@ export const openPlaywrightScenarioBrowser = async ( commandTimeoutMs, ); + throwIfScenarioCancelled(options.signal); const profilePath = await mkdtemp(join(tmpdir(), "rea-browser-scenario-")); let context: BrowserContext; const executablePath = scenario.browser.executable_path; @@ -877,6 +879,7 @@ export const openPlaywrightScenarioBrowser = async ( let launchStarted = false; try { await chmod(profilePath, 0o700); + throwIfScenarioCancelled(options.signal); launchStarted = true; context = await withScenarioCommandDeadline( () => @@ -950,10 +953,12 @@ export const openPlaywrightScenarioBrowser = async ( rm(profilePath, { recursive: true, force: true, maxRetries: 3 }), }); try { + throwIfScenarioCancelled(options.signal); const browser = context.browser(); if (browser === null) throw new BrowserObservationError(OPERATION, "protocol_error"); const page = context.pages()[0] ?? (await context.newPage()); + throwIfScenarioCancelled(options.signal); return { context, page, diff --git a/src/browser/PlaywrightScenarioSession.test.ts b/src/browser/PlaywrightScenarioSession.test.ts index 575bde0633..49d3447475 100644 --- a/src/browser/PlaywrightScenarioSession.test.ts +++ b/src/browser/PlaywrightScenarioSession.test.ts @@ -1185,6 +1185,64 @@ it("finishes initialization cleanup independently of request cancellation", asyn expect(retainCleanup).not.toHaveBeenCalled(); }); +it.each([false, true])( + "joins pending cookie setup before releasing an attached browser (late rejection: %s)", + async (rejectCookies) => { + const { context, cleanup, storage } = fixture(); + const controller = new AbortController(); + const reason = { stop: "during cookie setup" }; + const started = deferred(); + const cookies = deferred(); + context.addCookies.mockImplementationOnce(async () => { + started.resolve(); + await cookies.promise; + if (rejectCookies) + throw new Error("cookie setup rejected after cancellation"); + }); + const input = browserScenarioSchema.parse({ + ...connectedScenario("allow"), + storage: { + cookies: [ + { + name: "session", + http_only: true, + secure: true, + same_site: "Lax", + value: { source: "literal", value: "selected" }, + destination: { url: "https://storage.example.test/" }, + }, + ], + }, + }); + const provider = new PlaywrightBrowserScenarioProvider( + (scenario, options) => + PlaywrightScenarioSession.open(scenario, {}, options), + ); + const result = provider.captureScenario(input, { + signal: controller.signal, + }); + try { + await started.promise; + controller.abort(reason); + await new Promise((resolve) => setImmediate(resolve)); + expect(cleanup.close).not.toHaveBeenCalled(); + cookies.resolve(); + const outcome = await result; + if (outcome.ok) + throw new Error("Expected cancelled cookie initialization"); + expect(outcome.error._tag).toBe("AnalysisCancelledError"); + expect(outcome.error.cause).toBe(reason); + expect(outcome.error.cleanupIncomplete).toBe(false); + expect(cleanup.close).toHaveBeenCalledOnce(); + expect(storage?.initialize).not.toHaveBeenCalled(); + } finally { + cookies.resolve(); + await result; + await provider.close(); + } + }, +); + it("can retry cleanup of a browser that opened after request cancellation", async () => { const { opened } = fixture(); const closeBrowser = vi diff --git a/src/browser/PlaywrightScenarioSession.ts b/src/browser/PlaywrightScenarioSession.ts index 3771fb9c9a..5af4627e56 100644 --- a/src/browser/PlaywrightScenarioSession.ts +++ b/src/browser/PlaywrightScenarioSession.ts @@ -329,6 +329,7 @@ export class PlaywrightScenarioSession implements BrowserScenarioSessionPort { ): Promise { let events: PlaywrightScenarioEvents | undefined; let storage: PlaywrightScenarioStorage | undefined; + let pageInitialization: Promise | undefined; const serviceWorkers = scenario.browser.mode === "connect" && scenario.environment.service_workers === "block" @@ -341,10 +342,19 @@ export class PlaywrightScenarioSession implements BrowserScenarioSessionPort { : undefined; try { await withPlaywrightExecutionBoundary( - async () => { - await serviceWorkers?.initialize(); - await initializePage(opened.context, scenario, secrets); - }, + () => serviceWorkers?.initialize() ?? Promise.resolve(), + undefined, + options.signal, + ); + throwIfScenarioCancelled(options.signal); + const initializingPage = initializePage( + opened.context, + scenario, + secrets, + ); + pageInitialization = initializingPage; + await withPlaywrightExecutionBoundary( + () => initializingPage, undefined, options.signal, ); @@ -386,6 +396,9 @@ export class PlaywrightScenarioSession implements BrowserScenarioSessionPort { storage !== undefined && options.signal?.aborted === true; let policyStopsLoading = !storageStopsLoading; const cleanup = async () => { + // Cancelling the wait cannot revoke an already-sent cookie mutation. + // Join its acknowledgement before releasing the browser transport. + await pageInitialization?.catch(() => undefined); let storageFailure: { readonly cause: unknown } | undefined; let policyFailure: { readonly cause: unknown } | undefined; let browserFailure: { readonly cause: unknown } | undefined;