From 8243afc0628fe2960fa0c112309843cf66fc2874 Mon Sep 17 00:00:00 2001 From: wangmiao0668000666 Date: Fri, 17 Jul 2026 16:39:05 +0800 Subject: [PATCH] fix(browser): honor AbortSignal in tab discovery poll (#109617) * fix(browser): honor AbortSignal in tab discovery poll * fix(browser): unify abort-aware tab discovery Apply the canonical abortable sleep to both discovery loops, reject late backend results after cancellation, and replace timing-sensitive proof with deterministic timer coverage. Co-authored-by: wangmiao0668000666 --------- Co-authored-by: Peter Steinberger --- .../src/browser/server-context.selection.ts | 10 +- ...r-context.tab-discovery-poll-abort.test.ts | 150 ++++++++++++++++++ .../src/browser/server-context.tab-ops.ts | 5 +- 3 files changed, 155 insertions(+), 10 deletions(-) create mode 100644 extensions/browser/src/browser/server-context.tab-discovery-poll-abort.test.ts diff --git a/extensions/browser/src/browser/server-context.selection.ts b/extensions/browser/src/browser/server-context.selection.ts index 37c8c5047cdd..f7f905db85f3 100644 --- a/extensions/browser/src/browser/server-context.selection.ts +++ b/extensions/browser/src/browser/server-context.selection.ts @@ -1,6 +1,7 @@ /** * Browser tab selection operations for default tab choice, focus, and close. */ +import { sleepWithAbort } from "openclaw/plugin-sdk/runtime-env"; import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; import { formatErrorMessage } from "../infra/errors.js"; import type { SsrFPolicy } from "../infra/net/ssrf.js"; @@ -64,12 +65,6 @@ function mergeOpenedTabSnapshot( return merged; } -function waitForTabDiscoveryPoll(): Promise { - return new Promise((resolve) => { - setTimeout(resolve, OPEN_TAB_DISCOVERY_POLL_MS); - }); -} - /** Builds tab selection/focus/close operations for one resolved browser profile. */ export function createProfileSelectionOps({ profile, @@ -100,6 +95,7 @@ export function createProfileSelectionOps({ const readTabs = async (): Promise => { try { const tabs = await listTabs(options); + options?.signal?.throwIfAborted(); sawSuccessfulList = true; if (tabs.length > 0) { lastNonEmptyTabs = tabs; @@ -155,7 +151,7 @@ export function createProfileSelectionOps({ ) { const deadline = Date.now() + OPEN_TAB_DISCOVERY_WINDOW_MS; while (Date.now() < deadline) { - await waitForTabDiscoveryPoll(); + await sleepWithAbort(OPEN_TAB_DISCOVERY_POLL_MS, options?.signal); listedTabs = await readTabs(); await openWhenConfirmedEmpty(listedTabs); unfilteredTabs = mergeOpenedTabSnapshot(listedTabs, openedTab); 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 new file mode 100644 index 000000000000..ab70e8f602d6 --- /dev/null +++ b/extensions/browser/src/browser/server-context.tab-discovery-poll-abort.test.ts @@ -0,0 +1,150 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { withBrowserFetchPreconnect } from "../../test-fetch.js"; +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 { createProfileSelectionOps } from "./server-context.selection.js"; +import type { ProfileRuntimeState } from "./server-context.types.js"; + +afterEach(() => { + globalThis.fetch = originalFetch; + vi.useRealTimers(); + vi.restoreAllMocks(); +}); + +async function flushUntil(predicate: () => boolean): Promise { + for (let attempt = 0; attempt < 20; attempt += 1) { + if (predicate()) { + return; + } + await Promise.resolve(); + } + throw new Error("condition did not settle"); +} + +function makeProfileRuntime(): ProfileRuntimeState { + return { + profile: { + name: "openclaw", + cdpPort: 18800, + cdpUrl: "http://127.0.0.1:18800", + cdpHost: "127.0.0.1", + cdpIsLoopback: true, + color: "#FF4500", + driver: "openclaw", + headless: true, + headlessSource: "config", + attachOnly: false, + }, + running: { pid: 1234, proc: { on: vi.fn() } }, + lastTargetId: null, + } as unknown as ProfileRuntimeState; +} + +describe("browser tab discovery poll abort", () => { + it("cancels the selection discovery timer", async () => { + vi.useFakeTimers(); + const runtime = makeProfileRuntime(); + const tabWithoutWsUrl = { + targetId: "PAGE", + title: "page", + url: "http://127.0.0.1:3001", + type: "page" as const, + }; + const listTabs = vi.fn(async () => [tabWithoutWsUrl]); + + const ops = createProfileSelectionOps({ + profile: runtime.profile, + runtime, + getCdpControlPolicy: () => undefined, + ensureBrowserAvailable: async () => {}, + listTabs, + openTab: async () => tabWithoutWsUrl, + }); + + const controller = new AbortController(); + const ensurePromise = ops.ensureTabAvailable(undefined, { signal: controller.signal }); + + await flushUntil(() => vi.getTimerCount() === 1); + controller.abort(); + + await expect(ensurePromise).rejects.toThrow(/aborted/i); + expect(vi.getTimerCount()).toBe(0); + }); + + it("rejects when an in-flight tab read succeeds after abort", async () => { + vi.useFakeTimers(); + const runtime = makeProfileRuntime(); + const tab = { + targetId: "PAGE", + title: "page", + url: "http://127.0.0.1:3001", + wsUrl: "ws://127.0.0.1/devtools/page/PAGE", + type: "page" as const, + }; + let resolveFirstRead!: (tabs: (typeof tab)[]) => void; + const firstRead = new Promise<(typeof tab)[]>((resolve) => { + resolveFirstRead = resolve; + }); + const listTabs = vi + .fn() + .mockImplementationOnce(async () => await firstRead) + .mockResolvedValue([tab]); + const ops = createProfileSelectionOps({ + profile: runtime.profile, + runtime, + getCdpControlPolicy: () => undefined, + ensureBrowserAvailable: async () => {}, + listTabs, + openTab: async () => tab, + }); + const controller = new AbortController(); + const ensurePromise = ops.ensureTabAvailable(undefined, { signal: controller.signal }); + + await flushUntil(() => listTabs.mock.calls.length === 1); + controller.abort(); + resolveFirstRead([tab]); + + await expect(ensurePromise).rejects.toThrow(/aborted/i); + expect(listTabs).toHaveBeenCalledTimes(1); + 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)}`); + } + 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); + }); +}); diff --git a/extensions/browser/src/browser/server-context.tab-ops.ts b/extensions/browser/src/browser/server-context.tab-ops.ts index 2564f1deae70..ec92ac9572fd 100644 --- a/extensions/browser/src/browser/server-context.tab-ops.ts +++ b/extensions/browser/src/browser/server-context.tab-ops.ts @@ -1,6 +1,7 @@ /** * Browser tab listing, opening, labeling, and alias management for one profile. */ +import { sleepWithAbort } from "openclaw/plugin-sdk/runtime-env"; import { resolveBrowserNavigationProxyMode } from "./browser-proxy-mode.js"; import { resolveCdpControlPolicy } from "./cdp-reachability-policy.js"; import { isSelectableCdpBrowserTarget } from "./cdp-target-filter.js"; @@ -319,9 +320,7 @@ export function createProfileTabOps({ profile, state, runtime }: TabOpsDeps): Pr { ...opts, label: normalizedLabel }, ); } - await new Promise((r) => { - setTimeout(r, OPEN_TAB_DISCOVERY_POLL_MS); - }); + 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