diff --git a/extensions/browser/src/browser/cdp.test.ts b/extensions/browser/src/browser/cdp.test.ts index 71314f5d608c..e98d7e926510 100644 --- a/extensions/browser/src/browser/cdp.test.ts +++ b/extensions/browser/src/browser/cdp.test.ts @@ -318,41 +318,65 @@ describe("cdp", () => { expect(methods).not.toContain("Runtime.evaluate"); }); - it("preserves caller cancellation after target creation while navigation is settling", async () => { - const controller = new AbortController(); - const reason = new Error("cancel after target creation"); - const wsPort = await startWsServerWithMessages((msg, socket) => { - if (msg.method === "Target.createTarget") { - socket.send(JSON.stringify({ id: msg.id, result: { targetId: "TARGET_CANCEL" } })); - return; - } - if (msg.method === "Page.getFrameTree") { - controller.abort(reason); - socket.send( - JSON.stringify({ - id: msg.id, - result: { - frameTree: { - frame: { loaderId: "LOADER_CANCEL", url: "https://example.com" }, + it.each([ + { abortAt: "creation", closeFails: false }, + { abortAt: "navigation", closeFails: false }, + { abortAt: "navigation", closeFails: true }, + ])( + "closes an unreturned target after $abortAt abort (close fails: $closeFails)", + async ({ abortAt, closeFails }) => { + const controller = new AbortController(); + const reason = new Error("cancel after target creation"); + const closedTargets: unknown[] = []; + const wsPort = await startWsServerWithMessages((msg, socket) => { + if (msg.method === "Target.createTarget") { + if (abortAt === "creation") { + controller.abort(reason); + } + socket.send(JSON.stringify({ id: msg.id, result: { targetId: "TARGET_CANCEL" } })); + return; + } + if (msg.method === "Target.closeTarget") { + closedTargets.push(msg.params?.targetId); + socket.send( + JSON.stringify({ + id: msg.id, + ...(closeFails + ? { error: { message: "close failed" } } + : { result: { success: true } }), + }), + ); + return; + } + if (msg.method === "Page.getFrameTree") { + controller.abort(reason); + socket.send( + JSON.stringify({ + id: msg.id, + result: { + frameTree: { + frame: { loaderId: "LOADER_CANCEL", url: "https://example.com" }, + }, }, - }, - }), - ); - } - }); - const httpPort = await startVersionHttpServer({ - webSocketDebuggerUrl: `ws://127.0.0.1:${wsPort}/devtools/browser/TEST`, - }); + }), + ); + } + }); + const httpPort = await startVersionHttpServer({ + webSocketDebuggerUrl: `ws://127.0.0.1:${wsPort}/devtools/browser/TEST`, + }); - await expect( - createTargetViaCdp({ - cdpUrl: `http://127.0.0.1:${httpPort}`, - url: "https://example.com", - signal: controller.signal, - waitForNavigationResult: true, - }), - ).rejects.toBe(reason); - }); + await expect( + createTargetViaCdp({ + cdpUrl: `http://127.0.0.1:${httpPort}`, + url: "https://example.com", + signal: controller.signal, + waitForNavigationResult: true, + }), + ).rejects.toBe(reason); + expect(closedTargets).toEqual(["TARGET_CANCEL"]); + }, + ); it("cancels hanging endpoint discovery without creating a target", async () => { const controller = new AbortController(); diff --git a/extensions/browser/src/browser/cdp.ts b/extensions/browser/src/browser/cdp.ts index 331d1e08badf..f1db064980c4 100644 --- a/extensions/browser/src/browser/cdp.ts +++ b/extensions/browser/src/browser/cdp.ts @@ -300,15 +300,22 @@ export async function createTargetViaCdp(opts: { if (!targetId) { throw new Error("CDP Target.createTarget returned no targetId"); } - opts.signal?.throwIfAborted(); - const finalUrl = await prepareCdpTargetSession( - send, - targetId, - opts.waitForNavigationResult ? opts.url : undefined, - opts.signal, - ); - opts.signal?.throwIfAborted(); - return finalUrl ? { targetId, finalUrl } : { targetId }; + try { + opts.signal?.throwIfAborted(); + const finalUrl = await prepareCdpTargetSession( + send, + targetId, + opts.waitForNavigationResult ? opts.url : undefined, + opts.signal, + ); + opts.signal?.throwIfAborted(); + return finalUrl ? { targetId, finalUrl } : { targetId }; + } catch (error) { + // The caller cannot compensate until it receives this id. Keep cleanup + // on the creating socket, independent of cancellation, before releasing it. + await send("Target.closeTarget", { targetId }).catch(() => {}); + throw error; + } }, { commandTimeoutMs: opts.timeouts?.httpTimeoutMs ?? 5000, diff --git a/extensions/browser/src/browser/server-context.tab-discovery-poll-abort.test.ts b/extensions/browser/src/browser/server-context.tab-discovery-poll-abort.test.ts index e967d0575966..25d0a1a450ef 100644 --- a/extensions/browser/src/browser/server-context.tab-discovery-poll-abort.test.ts +++ b/extensions/browser/src/browser/server-context.tab-discovery-poll-abort.test.ts @@ -4,11 +4,9 @@ import "../test-support/browser-security.mock.js"; import "./server-context.chrome-test-harness.js"; import * as cdpModule from "./cdp.js"; import { OPEN_TAB_DISCOVERY_POLL_MS } from "./server-context.constants.js"; -import { - createTestBrowserRouteContext, - makeState, - originalFetch, -} from "./server-context.remote-tab-ops.harness.js"; +import { createBrowserRouteContext } from "./server-context.js"; +import { beginProfileTransition, markBrowserRuntimeStopping } from "./server-context.lifecycle.js"; +import { makeState, originalFetch } from "./server-context.remote-tab-ops.harness.js"; import { createProfileSelectionOps } from "./server-context.selection.js"; import type { ProfileRuntimeState } from "./server-context.types.js"; @@ -116,35 +114,64 @@ describe("browser tab discovery poll abort", () => { expect(vi.getTimerCount()).toBe(0); }); - it("cancels the opened-target discovery timer", async () => { - vi.useFakeTimers(); - const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout"); - vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ - targetId: "PENDING", - finalUrl: "about:blank", - }); - const fetchMock = vi.fn(async (url: unknown) => { - if (!String(url).includes("/json/list")) { - throw new Error(`unexpected fetch: ${String(url)}`); + it.each([ + { closeFails: false, shutdown: false }, + { closeFails: true, shutdown: false }, + { closeFails: false, shutdown: true }, + ])( + "cancels opened-target discovery and closes the target (close fails: $closeFails, shutdown: $shutdown)", + async ({ closeFails, shutdown }) => { + vi.useFakeTimers(); + const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout"); + vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ + targetId: "PENDING", + finalUrl: "about:blank", + }); + const closeRequests: string[] = []; + const closeSignalsAborted: Array = []; + const fetchMock = vi.fn(async (url: unknown, init?: RequestInit) => { + if (String(url).includes("/json/close/")) { + closeRequests.push(String(url)); + closeSignalsAborted.push(init?.signal?.aborted); + if (closeFails) { + throw new Error("close request failed"); + } + return { ok: true } as Response; + } + if (!String(url).includes("/json/list")) { + throw new Error(`unexpected fetch: ${String(url)}`); + } + return { ok: true, json: async () => [] } as unknown as Response; + }); + globalThis.fetch = withBrowserFetchPreconnect(fetchMock); + const state = makeState("openclaw"); + const openclaw = createBrowserRouteContext({ getState: () => state }).forProfile("openclaw"); + const controller = new AbortController(); + const openPromise = openclaw.openTab("about:blank", { signal: controller.signal }); + + await vi.advanceTimersByTimeAsync(0); + expect(setTimeoutSpy.mock.calls.some((call) => call[1] === OPEN_TAB_DISCOVERY_POLL_MS)).toBe( + true, + ); + expect(vi.getTimerCount()).toBe(1); + let stopping: Promise | undefined; + if (shutdown) { + markBrowserRuntimeStopping(state); + stopping = beginProfileTransition({ + state, + runtime: state.profiles.get("openclaw")!, + reason: "runtime shutdown", + closeSharedAdapters: false, + }); + } else { + controller.abort(); } - return { ok: true, json: async () => [] } as unknown as Response; - }); - globalThis.fetch = withBrowserFetchPreconnect(fetchMock); - const state = makeState("openclaw"); - const openclaw = createTestBrowserRouteContext({ getState: () => state }).forProfile( - "openclaw", - ); - const controller = new AbortController(); - const openPromise = openclaw.openTab("about:blank", { signal: controller.signal }); - await vi.advanceTimersByTimeAsync(0); - expect(setTimeoutSpy.mock.calls.some((call) => call[1] === OPEN_TAB_DISCOVERY_POLL_MS)).toBe( - true, - ); - expect(vi.getTimerCount()).toBe(1); - controller.abort(); - - await expect(openPromise).rejects.toThrow(/aborted/i); - expect(vi.getTimerCount()).toBe(0); - }); + await expect(openPromise).rejects.toMatchObject({ name: "AbortError", message: "aborted" }); + expect(closeRequests).toEqual(["http://127.0.0.1:18800/json/close/PENDING"]); + expect(closeSignalsAborted).toEqual([false]); + await stopping; + expect(vi.getTimerCount()).toBe(0); + }, + ); }); diff --git a/extensions/browser/src/browser/server-context.tab-ops.ts b/extensions/browser/src/browser/server-context.tab-ops.ts index 04eb02f5693f..f6f77b9dad19 100644 --- a/extensions/browser/src/browser/server-context.tab-ops.ts +++ b/extensions/browser/src/browser/server-context.tab-ops.ts @@ -252,39 +252,16 @@ export function createProfileTabOps({ profile, state, runtime }: TabOpsDeps): Pr options?: BrowserOperationOptions, ): Promise => { const cdpTimeouts = getRemoteCdpActionTimeouts(); - let ownership: BrowserOpenResult["ownership"]; - try { - ownership = await resolveCdpTabOwnership({ + return { + ...tab, + ownership: await resolveCdpTabOwnership({ profileName: profile.name, cdpUrl: profile.cdpUrl, nativeTargetId: tab.targetId, signal: options?.signal, timeoutMs: cdpTimeouts?.httpTimeoutMs, ssrfPolicy: getCdpControlPolicy(), - }); - } catch (ownershipError) { - try { - // Ownership probing happens after target creation. Cleanup must not - // inherit a caller abort that would strand the new untracked page. - await fetchOk( - appendCdpPath(cdpHttpBase, `/json/close/${encodeURIComponent(tab.targetId)}`), - state().resolved.remoteCdpTimeoutMs, - undefined, - getCdpControlPolicy(), - ); - } catch (closeError) { - throw Object.assign( - new Error("Failed to resolve browser tab ownership and close the new target", { - cause: ownershipError, - }), - { errors: [ownershipError, closeError] }, - ); - } - throw ownershipError; - } - return { - ...tab, - ownership, + }), }; }; @@ -295,10 +272,12 @@ export function createProfileTabOps({ profile, state, runtime }: TabOpsDeps): Pr opts?.signal?.throwIfAborted(); const normalizedLabel = opts?.label === undefined ? undefined : normalizeTabLabel(opts.label); const ssrfPolicyOpts = getNavigationPolicy(); + const cdpPolicy = getCdpControlPolicy(); + // Runtime shutdown fences state() before draining this operation's cleanup. + const cleanupTimeoutMs = state().resolved.remoteCdpTimeoutMs; if (capabilities.usesChromeMcp) { await assertBrowserNavigationAllowed({ url, ...ssrfPolicyOpts }); - const cdpPolicy = getCdpControlPolicy(); assertChromeMcpCdpTransportAllowed(profile, cdpPolicy); const { openChromeMcpTab } = await getChromeMcpModule(); const cdpTimeouts = getRemoteCdpActionTimeouts(); @@ -312,185 +291,204 @@ export function createProfileTabOps({ profile, state, runtime }: TabOpsDeps): Pr return adoptValidatedTab(page, { ...opts, label: normalizedLabel }); } - if (capabilities.usesPersistentPlaywright) { - const mod = await getPwAiModule({ mode: "strict" }); - const createPageViaPlaywright = (mod as Partial | null)?.createPageViaPlaywright; - if (typeof createPageViaPlaywright === "function") { - const page = await createPageViaPlaywright({ - cdpUrl: profile.cdpUrl, - url, - cdpPolicy: getCdpControlPolicy(), - ...ssrfPolicyOpts, - }); - return adoptValidatedTab( - await withTabOwnership( - { - targetId: page.targetId, - title: page.title, - url: page.url, - type: page.type, - }, - opts, - ), - { ...opts, label: normalizedLabel }, + let createdTargetId: string | undefined; + try { + if (capabilities.usesPersistentPlaywright) { + const mod = await getPwAiModule({ mode: "strict" }); + const createPageViaPlaywright = (mod as Partial | null) + ?.createPageViaPlaywright; + if (typeof createPageViaPlaywright === "function") { + const page = await createPageViaPlaywright({ + cdpUrl: profile.cdpUrl, + url, + cdpPolicy, + ...ssrfPolicyOpts, + }); + createdTargetId = page.targetId; + return adoptValidatedTab( + await withTabOwnership( + { + targetId: page.targetId, + title: page.title, + url: page.url, + type: page.type, + }, + opts, + ), + { ...opts, label: normalizedLabel }, + ); + } + } + + if (requiresInspectableBrowserNavigationRedirectsForUrl(url, state().resolved.ssrfPolicy)) { + throw new InvalidBrowserNavigationUrlError( + "Navigation blocked: strict browser SSRF policy requires Playwright-backed redirect-hop inspection", ); } - } - if (requiresInspectableBrowserNavigationRedirectsForUrl(url, state().resolved.ssrfPolicy)) { - throw new InvalidBrowserNavigationUrlError( - "Navigation blocked: strict browser SSRF policy requires Playwright-backed redirect-hop inspection", - ); - } + await assertBrowserNavigationAllowed({ url, ...ssrfPolicyOpts }); + const cdpActionTimeouts = getRemoteCdpActionTimeouts(); + const createTargetOpts: Parameters[0] = { + cdpUrl: profile.cdpUrl, + url, + ssrfPolicy: cdpPolicy, + waitForNavigationResult: true, + }; + if (cdpActionTimeouts) { + createTargetOpts.timeouts = cdpActionTimeouts; + } + if (opts?.signal) { + createTargetOpts.signal = opts.signal; + } + const createdViaCdp = await createTargetViaCdp(createTargetOpts).catch(() => null); + createdTargetId = createdViaCdp?.targetId; + opts?.signal?.throwIfAborted(); - await assertBrowserNavigationAllowed({ url, ...ssrfPolicyOpts }); - const cdpActionTimeouts = getRemoteCdpActionTimeouts(); - const createTargetOpts: Parameters[0] = { - cdpUrl: profile.cdpUrl, - url, - ssrfPolicy: getCdpControlPolicy(), - waitForNavigationResult: true, - }; - if (cdpActionTimeouts) { - createTargetOpts.timeouts = cdpActionTimeouts; - } - if (opts?.signal) { - createTargetOpts.signal = opts.signal; - } - const createdViaCdp = await createTargetViaCdp(createTargetOpts).catch(() => null); - opts?.signal?.throwIfAborted(); - - if (createdViaCdp) { - if (!createdViaCdp.finalUrl) { - // The target exists, but its committed document is not authoritative. - // Preserve the explicit result without sticky, alias, or cleanup adoption. + if (createdViaCdp) { + if (!createdViaCdp.finalUrl) { + // The target exists, but its committed document is not authoritative. + // Preserve the explicit result without sticky, alias, or cleanup adoption. + return await withTabOwnership( + { + targetId: createdViaCdp.targetId, + title: "", + url, + type: "page", + }, + opts, + ); + } + await assertBrowserNavigationResultAllowed({ + url: createdViaCdp.finalUrl, + ...ssrfPolicyOpts, + }); + const deadline = Date.now() + OPEN_TAB_DISCOVERY_WINDOW_MS; + while (Date.now() < deadline) { + opts?.signal?.throwIfAborted(); + const tabs = await readTabs(opts).catch(() => [] as BrowserTab[]); + const found = tabs.find((t) => t.targetId === createdViaCdp.targetId); + if (found) { + await assertBrowserNavigationResultAllowed({ url: found.url, ...ssrfPolicyOpts }); + // The attached target owns the committed URL; /json/list supplies the + // remaining metadata and may briefly lag that exact document snapshot. + return adoptValidatedTab( + await withTabOwnership({ ...found, url: createdViaCdp.finalUrl }, opts), + { ...opts, label: normalizedLabel }, + ); + } + await sleepWithAbort(OPEN_TAB_DISCOVERY_POLL_MS, opts?.signal); + } + opts?.signal?.throwIfAborted(); + // Preserve the explicit target-id result for callers, but do not adopt an + // undiscovered target into sticky, alias, or managed-cleanup state. return await withTabOwnership( { targetId: createdViaCdp.targetId, title: "", - url, + url: createdViaCdp.finalUrl, type: "page", }, opts, ); } - await assertBrowserNavigationResultAllowed({ - url: createdViaCdp.finalUrl, - ...ssrfPolicyOpts, - }); - const deadline = Date.now() + OPEN_TAB_DISCOVERY_WINDOW_MS; - while (Date.now() < deadline) { - opts?.signal?.throwIfAborted(); - const tabs = await readTabs(opts).catch(() => [] as BrowserTab[]); - const found = tabs.find((t) => t.targetId === createdViaCdp.targetId); - if (found) { - await assertBrowserNavigationResultAllowed({ url: found.url, ...ssrfPolicyOpts }); - // The attached target owns the committed URL; /json/list supplies the - // remaining metadata and may briefly lag that exact document snapshot. - return adoptValidatedTab( - await withTabOwnership({ ...found, url: createdViaCdp.finalUrl }, opts), - { ...opts, label: normalizedLabel }, + + const encoded = encodeURIComponent(url); + const endpointUrl = new URL(appendCdpPath(cdpHttpBase, "/json/new")); + const endpoint = endpointUrl.search + ? (() => { + endpointUrl.searchParams.set("url", url); + return endpointUrl.toString(); + })() + : `${endpointUrl.toString()}?${encoded}`; + opts?.signal?.throwIfAborted(); + const created = await fetchJson( + endpoint, + cdpActionTimeouts?.httpTimeoutMs ?? CDP_JSON_NEW_TIMEOUT_MS, + { + method: "PUT", + }, + getCdpControlPolicy(), + ).catch(async (err: unknown) => { + if (String(err).includes("HTTP 405")) { + return await fetchJson( + endpoint, + cdpActionTimeouts?.httpTimeoutMs ?? CDP_JSON_NEW_TIMEOUT_MS, + undefined, + getCdpControlPolicy(), ); } - await sleepWithAbort(OPEN_TAB_DISCOVERY_POLL_MS, opts?.signal); - } - opts?.signal?.throwIfAborted(); - // Preserve the explicit target-id result for callers, but do not adopt an - // undiscovered target into sticky, alias, or managed-cleanup state. - return await withTabOwnership( - { - targetId: createdViaCdp.targetId, - title: "", - url: createdViaCdp.finalUrl, - type: "page", - }, - opts, - ); - } + throw err; + }); - const encoded = encodeURIComponent(url); - const endpointUrl = new URL(appendCdpPath(cdpHttpBase, "/json/new")); - const endpoint = endpointUrl.search - ? (() => { - endpointUrl.searchParams.set("url", url); - return endpointUrl.toString(); - })() - : `${endpointUrl.toString()}?${encoded}`; - opts?.signal?.throwIfAborted(); - const created = await fetchJson( - endpoint, - cdpActionTimeouts?.httpTimeoutMs ?? CDP_JSON_NEW_TIMEOUT_MS, - { - method: "PUT", - }, - getCdpControlPolicy(), - ).catch(async (err: unknown) => { - if (String(err).includes("HTTP 405")) { - return await fetchJson( - endpoint, - cdpActionTimeouts?.httpTimeoutMs ?? CDP_JSON_NEW_TIMEOUT_MS, - undefined, - getCdpControlPolicy(), + createdTargetId = created.id; + opts?.signal?.throwIfAborted(); + if (!created.id) { + throw new Error("Failed to open tab (missing id)"); + } + const resolvedUrl = created.url ?? url; + if (!isSelectableCdpBrowserTarget({ url: resolvedUrl, type: created.type })) { + throw new Error("Failed to open tab (non-selectable target)"); + } + await assertBrowserNavigationResultAllowed({ url: resolvedUrl, ...ssrfPolicyOpts }); + const wsUrl = normalizeWsUrl(created.webSocketDebuggerUrl, profile.cdpUrl); + const wsPin = wsUrl + ? await assertCdpEndpointAllowed(wsUrl, getCdpControlPolicy(), { + source: "discovered", + configuredUrl: profile.cdpUrl, + }) + : undefined; + const committedUrl = wsUrl + ? await waitForCdpCommittedNavigationUrl({ + wsUrl, + configuredCdpUrl: profile.cdpUrl, + cdpPolicy: getCdpControlPolicy(), + requestedUrl: url, + signal: opts?.signal, + timeouts: cdpActionTimeouts, + }) + : undefined; + opts?.signal?.throwIfAborted(); + if (!committedUrl) { + return await withTabOwnership( + { + targetId: created.id, + title: created.title ?? "", + url: resolvedUrl, + wsUrl, + ...(wsPin?.lookup ? { wsLookup: wsPin.lookup } : {}), + type: created.type, + }, + opts, ); } - throw err; - }); - - opts?.signal?.throwIfAborted(); - if (!created.id) { - throw new Error("Failed to open tab (missing id)"); - } - const resolvedUrl = created.url ?? url; - if (!isSelectableCdpBrowserTarget({ url: resolvedUrl, type: created.type })) { - throw new Error("Failed to open tab (non-selectable target)"); - } - await assertBrowserNavigationResultAllowed({ url: resolvedUrl, ...ssrfPolicyOpts }); - const wsUrl = normalizeWsUrl(created.webSocketDebuggerUrl, profile.cdpUrl); - const wsPin = wsUrl - ? await assertCdpEndpointAllowed(wsUrl, getCdpControlPolicy(), { - source: "discovered", - configuredUrl: profile.cdpUrl, - }) - : undefined; - const committedUrl = wsUrl - ? await waitForCdpCommittedNavigationUrl({ - wsUrl, - configuredCdpUrl: profile.cdpUrl, - cdpPolicy: getCdpControlPolicy(), - requestedUrl: url, - signal: opts?.signal, - timeouts: cdpActionTimeouts, - }) - : undefined; - opts?.signal?.throwIfAborted(); - if (!committedUrl) { - return await withTabOwnership( - { - targetId: created.id, - title: created.title ?? "", - url: resolvedUrl, - wsUrl, - ...(wsPin?.lookup ? { wsLookup: wsPin.lookup } : {}), - type: created.type, - }, - opts, + await assertBrowserNavigationResultAllowed({ url: committedUrl, ...ssrfPolicyOpts }); + return adoptValidatedTab( + await withTabOwnership( + { + targetId: created.id, + title: created.title ?? "", + url: committedUrl, + wsUrl, + ...(wsPin?.lookup ? { wsLookup: wsPin.lookup } : {}), + type: created.type, + }, + opts, + ), + { ...opts, label: normalizedLabel }, ); + } catch (openError) { + if (createdTargetId) { + // Creation owns the target until a successful handoff. Cleanup must not + // inherit the caller's abort or replace the original open failure. + await fetchOk( + appendCdpPath(cdpHttpBase, `/json/close/${encodeURIComponent(createdTargetId)}`), + cleanupTimeoutMs, + undefined, + cdpPolicy, + ).catch(() => {}); + } + throw openError; } - await assertBrowserNavigationResultAllowed({ url: committedUrl, ...ssrfPolicyOpts }); - return adoptValidatedTab( - await withTabOwnership( - { - targetId: created.id, - title: created.title ?? "", - url: committedUrl, - wsUrl, - ...(wsPin?.lookup ? { wsLookup: wsPin.lookup } : {}), - type: created.type, - }, - opts, - ), - { ...opts, label: normalizedLabel }, - ); }; const labelTab = async ( diff --git a/extensions/browser/src/browser/server-context.tab-ownership.test.ts b/extensions/browser/src/browser/server-context.tab-ownership.test.ts index 42fa8694a59c..d134ecac3e34 100644 --- a/extensions/browser/src/browser/server-context.tab-ownership.test.ts +++ b/extensions/browser/src/browser/server-context.tab-ownership.test.ts @@ -15,72 +15,78 @@ afterEach(() => { }); describe("browser tab ownership probes", () => { - it("propagates caller abort through the managed ownership version probe", async () => { - vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ - targetId: "CREATED", - finalUrl: "http://127.0.0.1:8080", - }); - let versionSignal: AbortSignal | undefined; - let closedCreatedTarget = false; - let markProbeStarted!: () => void; - const probeStarted = new Promise((resolve) => { - markProbeStarted = resolve; - }); - const fetchMock = vi.fn(async (url: unknown, init?: RequestInit) => { - const value = String(url); - if (value.includes("/json/list")) { - return { - ok: true, - json: async () => [ - { - id: "CREATED", - title: "New Tab", - url: "http://127.0.0.1:8080", - webSocketDebuggerUrl: "ws://127.0.0.1/devtools/page/CREATED", - type: "page", - }, - ], - } as unknown as Response; - } - if (value.includes("/json/version")) { - versionSignal = init?.signal ?? undefined; - markProbeStarted(); - return await new Promise((_resolve, reject) => { - init?.signal?.addEventListener( - "abort", - () => - reject( - init.signal?.reason instanceof Error - ? init.signal.reason - : new Error("managed ownership probe aborted"), - ), - { once: true }, - ); - }); - } - if (value.includes("/json/close/CREATED")) { - closedCreatedTarget = true; - return { ok: true } as Response; - } - throw new Error(`unexpected fetch: ${value}`); - }); - global.fetch = withBrowserFetchPreconnect(fetchMock); - const state = makeState("openclaw"); - const openclaw = createTestBrowserRouteContext({ getState: () => state }).forProfile( - "openclaw", - ); - const controller = new AbortController(); - const abortError = new Error("caller aborted managed ownership probe"); + it.each([false, true])( + "propagates caller abort through the managed ownership version probe (close fails: %s)", + async (closeFails) => { + vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ + targetId: "CREATED", + finalUrl: "http://127.0.0.1:8080", + }); + let versionSignal: AbortSignal | undefined; + const closeRequests: string[] = []; + let markProbeStarted!: () => void; + const probeStarted = new Promise((resolve) => { + markProbeStarted = resolve; + }); + const fetchMock = vi.fn(async (url: unknown, init?: RequestInit) => { + const value = String(url); + if (value.includes("/json/list")) { + return { + ok: true, + json: async () => [ + { + id: "CREATED", + title: "New Tab", + url: "http://127.0.0.1:8080", + webSocketDebuggerUrl: "ws://127.0.0.1/devtools/page/CREATED", + type: "page", + }, + ], + } as unknown as Response; + } + if (value.includes("/json/version")) { + versionSignal = init?.signal ?? undefined; + markProbeStarted(); + return await new Promise((_resolve, reject) => { + init?.signal?.addEventListener( + "abort", + () => + reject( + init.signal?.reason instanceof Error + ? init.signal.reason + : new Error("managed ownership probe aborted"), + ), + { once: true }, + ); + }); + } + if (value.includes("/json/close/CREATED")) { + closeRequests.push(value); + if (closeFails) { + throw new Error("close request failed"); + } + return { ok: true } as Response; + } + throw new Error(`unexpected fetch: ${value}`); + }); + global.fetch = withBrowserFetchPreconnect(fetchMock); + const state = makeState("openclaw"); + const openclaw = createTestBrowserRouteContext({ getState: () => state }).forProfile( + "openclaw", + ); + const controller = new AbortController(); + const abortError = new Error("caller aborted managed ownership probe"); - const opening = openclaw.openTab("http://127.0.0.1:8080", { - signal: controller.signal, - }); - await probeStarted; - controller.abort(abortError); - const propagatedImmediately = versionSignal?.aborted; + const opening = openclaw.openTab("http://127.0.0.1:8080", { + signal: controller.signal, + }); + await probeStarted; + controller.abort(abortError); + const propagatedImmediately = versionSignal?.aborted; - await expect(opening).rejects.toBe(abortError); - expect(propagatedImmediately).toBe(true); - expect(closedCreatedTarget).toBe(true); - }); + await expect(opening).rejects.toBe(abortError); + expect(propagatedImmediately).toBe(true); + expect(closeRequests).toEqual(["http://127.0.0.1:18800/json/close/CREATED"]); + }, + ); }); diff --git a/extensions/browser/src/browser/server-context.tab-selection-state.test.ts b/extensions/browser/src/browser/server-context.tab-selection-state.test.ts index 8608034829f3..795cc60e98be 100644 --- a/extensions/browser/src/browser/server-context.tab-selection-state.test.ts +++ b/extensions/browser/src/browser/server-context.tab-selection-state.test.ts @@ -148,6 +148,7 @@ describe("browser server-context tab selection state", () => { browserInstanceFingerprint: expect.stringMatching(/^sha256:/), }); expect(state.profiles.get("openclaw")?.lastTargetId).toBe("CREATED"); + expect(fetchCallUrls(fetchMock).some((url) => url.includes("/json/close/"))).toBe(false); expect(createTargetViaCdp).toHaveBeenCalledWith({ cdpUrl: "http://127.0.0.1:18800", url: "http://127.0.0.1:8080", @@ -221,7 +222,9 @@ describe("browser server-context tab selection state", () => { expect(profileState?.lastTargetId).not.toBe("BLOCKED"); expect(profileState?.tabAliases).toEqual(aliasesBefore); expect(profileState?.tabAliases?.byTargetId.BLOCKED).toBeUndefined(); - expect(fetchCallUrls(fetchMock).some((url) => url.includes("/json/close/BLOCKED"))).toBe(false); + expect(fetchCallUrls(fetchMock).filter((url) => url.includes("/json/close/"))).toEqual([ + "http://127.0.0.1:18800/json/close/BLOCKED", + ]); await expect(openclaw.ensureTabAvailable()).resolves.toEqual( expect.objectContaining({ targetId: "GOOD" }),