diff --git a/extensions/browser/src/attached-browser-tool-runtime.test.ts b/extensions/browser/src/attached-browser-tool-runtime.test.ts index 9c834387038c..cf7821704f51 100644 --- a/extensions/browser/src/attached-browser-tool-runtime.test.ts +++ b/extensions/browser/src/attached-browser-tool-runtime.test.ts @@ -7,6 +7,7 @@ const mocks = vi.hoisted(() => ({ createBrowserTool: vi.fn(), startBrowserBridgeServer: vi.fn(), stopBrowserBridgeServer: vi.fn(), + closePlaywrightBrowserConnection: vi.fn(), })); vi.mock("./browser-tool.js", () => ({ @@ -18,11 +19,16 @@ vi.mock("./browser/bridge-server.js", () => ({ stopBrowserBridgeServer: mocks.stopBrowserBridgeServer, })); +vi.mock("./browser/pw-session.js", () => ({ + closePlaywrightBrowserConnection: mocks.closePlaywrightBrowserConnection, +})); + import { createAttachedBrowserToolRuntime } from "./attached-browser-tool-runtime.js"; describe("attached Browser tool runtime", () => { beforeEach(() => { vi.clearAllMocks(); + mocks.closePlaywrightBrowserConnection.mockResolvedValue(undefined); mocks.createBrowserTool.mockReturnValue({ name: "browser" }); mocks.startBrowserBridgeServer.mockResolvedValue({ baseUrl: "http://127.0.0.1:18443", @@ -80,11 +86,13 @@ describe("attached Browser tool runtime", () => { const sourcePath = path.join(workspaceDir, "source.png"); await fs.writeFile(sourcePath, "screenshot-bytes"); const artifactPath = await toolParams.persistScreenshot({ sourcePath, type: "png" }); - expect(artifactPath).toMatch( + expect(artifactPath.replaceAll("\\", "/")).toMatch( /\.artifacts\/cloud-worker-browser\/screenshot-[a-f0-9]{16}\.png$/u, ); expect(await fs.readFile(artifactPath, "utf8")).toBe("screenshot-bytes"); - expect((await fs.stat(artifactPath)).mode & 0o777).toBe(0o600); + if (process.platform !== "win32") { + expect((await fs.stat(artifactPath)).mode & 0o777).toBe(0o600); + } const filesBeforeFailure = await fs.readdir(path.dirname(artifactPath)); await expect( toolParams.persistScreenshot({ @@ -97,10 +105,39 @@ describe("attached Browser tool runtime", () => { await runtime.dispose(); expect(mocks.stopBrowserBridgeServer).toHaveBeenCalledWith({ marker: "bridge-server" }); + expect(mocks.closePlaywrightBrowserConnection).toHaveBeenCalledWith({ + cdpUrl: "http://127.0.0.1:9222", + }); expect(ensureAttachTarget).toHaveBeenCalledOnce(); await fs.rm(workspaceDir, { recursive: true, force: true }); }); + it("retries exact Playwright CDP adapter disposal after a failed disconnect", async () => { + const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "attached-browser-workspace-")); + const runtime = await createAttachedBrowserToolRuntime({ + cdpUrl: "http://127.0.0.1:9222", + ensureAttachTarget: async () => {}, + workspaceDir, + }); + mocks.closePlaywrightBrowserConnection + .mockRejectedValueOnce(new Error("disconnect failed")) + .mockResolvedValueOnce(undefined); + + await expect(runtime.dispose()).rejects.toThrow("disconnect failed"); + await expect(runtime.dispose()).resolves.toBeUndefined(); + + expect(mocks.stopBrowserBridgeServer).toHaveBeenCalledTimes(2); + expect(mocks.closePlaywrightBrowserConnection).toHaveBeenCalledTimes(2); + expect(mocks.closePlaywrightBrowserConnection).toHaveBeenNthCalledWith(1, { + cdpUrl: "http://127.0.0.1:9222", + }); + expect(mocks.closePlaywrightBrowserConnection).toHaveBeenNthCalledWith(2, { + cdpUrl: "http://127.0.0.1:9222", + }); + + await fs.rm(workspaceDir, { recursive: true, force: true }); + }); + it.runIf(process.platform !== "win32")( "rejects screenshot artifact symlink escapes", async () => { diff --git a/extensions/browser/src/attached-browser-tool-runtime.ts b/extensions/browser/src/attached-browser-tool-runtime.ts index df4c375b7d37..ad19953fe35d 100644 --- a/extensions/browser/src/attached-browser-tool-runtime.ts +++ b/extensions/browser/src/attached-browser-tool-runtime.ts @@ -11,6 +11,7 @@ import { createBrowserTool } from "./browser-tool.js"; import type { AnyAgentTool } from "./browser-tool.runtime.js"; import { startBrowserBridgeServer, stopBrowserBridgeServer } from "./browser/bridge-server.js"; import { resolveBrowserConfig } from "./browser/config.js"; +import { closePlaywrightBrowserConnection } from "./browser/pw-session.js"; import { writeExternalFileWithinRoot } from "./sdk-security-runtime.js"; const ATTACHED_PROFILE_NAME = "worker"; @@ -113,6 +114,13 @@ export async function createAttachedBrowserToolRuntime( authToken: randomBytes(32).toString("base64url"), onEnsureAttachTarget: async () => await params.ensureAttachTarget(), }); + const dispose = async () => { + try { + await stopBrowserBridgeServer(bridge.server); + } finally { + await closePlaywrightBrowserConnection({ cdpUrl }); + } + }; try { const tool = createBrowserTool({ sandboxBridgeUrl: bridge.baseUrl, @@ -132,10 +140,10 @@ export async function createAttachedBrowserToolRuntime( }); return { tool, - dispose: async () => await stopBrowserBridgeServer(bridge.server), + dispose, }; } catch (error) { - await stopBrowserBridgeServer(bridge.server); + await dispose(); throw error; } }