mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 15:40:44 +00:00
fix(browser): propagate click aborts through agent act routes
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
committed by
Peter Steinberger
parent
adc05f090a
commit
cd82b94333
@@ -341,6 +341,28 @@ describe("chrome MCP page parsing", () => {
|
||||
expect(tabs).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("does not dispatch a click when the signal is already aborted", async () => {
|
||||
const session = createFakeSession();
|
||||
const callTool = vi.fn(async (_call: ToolCall) => {
|
||||
throw new Error("callTool should not run");
|
||||
});
|
||||
session.client.callTool = callTool as typeof session.client.callTool;
|
||||
setChromeMcpSessionFactoryForTest(async () => session);
|
||||
const ctrl = new AbortController();
|
||||
ctrl.abort(new Error("aborted before click"));
|
||||
|
||||
await expect(
|
||||
clickChromeMcpElement({
|
||||
profileName: "chrome-live",
|
||||
targetId: "1",
|
||||
uid: "btn-1",
|
||||
signal: ctrl.signal,
|
||||
}),
|
||||
).rejects.toThrow(/aborted before click/i);
|
||||
|
||||
expect(callTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("creates a fresh session when userDataDir changes for the same profile", async () => {
|
||||
const createdSessions: ChromeMcpSession[] = [];
|
||||
const closeMocks: Array<ReturnType<typeof vi.fn>> = [];
|
||||
|
||||
@@ -505,6 +505,7 @@ export async function clickViaPlaywright(opts: {
|
||||
delayMs?: number;
|
||||
timeoutMs?: number;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
signal?: AbortSignal;
|
||||
}): Promise<void> {
|
||||
const resolved = requireRefOrSelector(opts.ref, opts.selector);
|
||||
const page = await getRestoredPageForTarget(opts);
|
||||
@@ -514,6 +515,36 @@ export async function clickViaPlaywright(opts: {
|
||||
: page.locator(resolved.selector!);
|
||||
const timeout = resolveInteractionTimeoutMs(opts.timeoutMs);
|
||||
const previousUrl = page.url();
|
||||
const signal = opts.signal;
|
||||
let abortListener: (() => void) | undefined;
|
||||
let abortReject: ((reason: unknown) => void) | undefined;
|
||||
let abortPromise: Promise<never> | undefined;
|
||||
if (signal) {
|
||||
abortPromise = new Promise((_, reject) => {
|
||||
abortReject = reject;
|
||||
});
|
||||
void abortPromise.catch(() => {});
|
||||
const disconnect = () => {
|
||||
void forceDisconnectPlaywrightForTarget({
|
||||
cdpUrl: opts.cdpUrl,
|
||||
targetId: opts.targetId,
|
||||
reason: "click aborted",
|
||||
}).catch(() => {});
|
||||
};
|
||||
if (signal.aborted) {
|
||||
disconnect();
|
||||
throw signal.reason ?? new Error("aborted");
|
||||
}
|
||||
abortListener = () => {
|
||||
disconnect();
|
||||
abortReject?.(signal.reason ?? new Error("aborted"));
|
||||
};
|
||||
signal.addEventListener("abort", abortListener, { once: true });
|
||||
if (signal.aborted) {
|
||||
abortListener();
|
||||
throw signal.reason ?? new Error("aborted");
|
||||
}
|
||||
}
|
||||
try {
|
||||
await assertInteractionNavigationCompletedSafely({
|
||||
action: async () => {
|
||||
@@ -523,22 +554,28 @@ export async function clickViaPlaywright(opts: {
|
||||
ACT_MAX_CLICK_DELAY_MS,
|
||||
);
|
||||
if (delayMs > 0) {
|
||||
await locator.hover({ timeout });
|
||||
await awaitEvalWithAbort(locator.hover({ timeout }), abortPromise);
|
||||
await new Promise((resolve) => setTimeout(resolve, delayMs));
|
||||
}
|
||||
if (opts.doubleClick) {
|
||||
await locator.dblclick({
|
||||
await awaitEvalWithAbort(
|
||||
locator.dblclick({
|
||||
timeout,
|
||||
button: opts.button,
|
||||
modifiers: opts.modifiers,
|
||||
}),
|
||||
abortPromise,
|
||||
);
|
||||
return;
|
||||
}
|
||||
await awaitEvalWithAbort(
|
||||
locator.click({
|
||||
timeout,
|
||||
button: opts.button,
|
||||
modifiers: opts.modifiers,
|
||||
});
|
||||
return;
|
||||
}
|
||||
await locator.click({
|
||||
timeout,
|
||||
button: opts.button,
|
||||
modifiers: opts.modifiers,
|
||||
});
|
||||
}),
|
||||
abortPromise,
|
||||
);
|
||||
},
|
||||
cdpUrl: opts.cdpUrl,
|
||||
page,
|
||||
@@ -548,6 +585,10 @@ export async function clickViaPlaywright(opts: {
|
||||
});
|
||||
} catch (err) {
|
||||
throw toAIFriendlyError(err, label);
|
||||
} finally {
|
||||
if (signal && abortListener) {
|
||||
signal.removeEventListener("abort", abortListener);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -419,6 +419,8 @@ export function registerBrowserAgentActRoutes(
|
||||
targetId: tab.targetId,
|
||||
uid: action.ref!,
|
||||
doubleClick: action.doubleClick ?? false,
|
||||
timeoutMs: action.timeoutMs,
|
||||
signal: req.signal,
|
||||
}),
|
||||
guard: existingSessionNavigationGuard,
|
||||
});
|
||||
|
||||
@@ -8,6 +8,7 @@ import { createBrowserRouteApp, createBrowserRouteResponse } from "./test-helper
|
||||
const routeState = existingSessionRouteState;
|
||||
|
||||
const chromeMcpMocks = vi.hoisted(() => ({
|
||||
clickChromeMcpElement: vi.fn(async () => {}),
|
||||
evaluateChromeMcpScript: vi.fn(
|
||||
async (_params: { profileName: string; targetId: string; fn: string }) => true,
|
||||
),
|
||||
@@ -28,7 +29,7 @@ const navigationGuardMocks = vi.hoisted(() => ({
|
||||
}));
|
||||
|
||||
vi.mock("../chrome-mcp.js", () => ({
|
||||
clickChromeMcpElement: vi.fn(async () => {}),
|
||||
clickChromeMcpElement: chromeMcpMocks.clickChromeMcpElement,
|
||||
closeChromeMcpTab: vi.fn(async () => {}),
|
||||
dragChromeMcpElement: vi.fn(async () => {}),
|
||||
evaluateChromeMcpScript: chromeMcpMocks.evaluateChromeMcpScript,
|
||||
@@ -106,6 +107,7 @@ describe("existing-session browser routes", () => {
|
||||
beforeEach(() => {
|
||||
routeState.profileCtx.ensureTabAvailable.mockClear();
|
||||
routeState.profileCtx.listTabs.mockClear();
|
||||
chromeMcpMocks.clickChromeMcpElement.mockClear();
|
||||
chromeMcpMocks.evaluateChromeMcpScript.mockReset();
|
||||
chromeMcpMocks.navigateChromeMcpPage.mockClear();
|
||||
chromeMcpMocks.takeChromeMcpScreenshot.mockClear();
|
||||
@@ -262,4 +264,31 @@ describe("existing-session browser routes", () => {
|
||||
fn: "() => window.location.href",
|
||||
});
|
||||
});
|
||||
|
||||
it("forwards click timeoutMs to the existing-session click executor", async () => {
|
||||
const handler = getActPostHandler();
|
||||
const response = createBrowserRouteResponse();
|
||||
const ctrl = new AbortController();
|
||||
|
||||
await handler?.(
|
||||
{
|
||||
params: {},
|
||||
query: {},
|
||||
body: { kind: "click", ref: "btn-1", timeoutMs: 1234 },
|
||||
signal: ctrl.signal,
|
||||
},
|
||||
response.res,
|
||||
);
|
||||
|
||||
expect(response.statusCode).toBe(200);
|
||||
expect(chromeMcpMocks.clickChromeMcpElement).toHaveBeenCalledWith({
|
||||
profileName: "chrome-live",
|
||||
userDataDir: undefined,
|
||||
targetId: "7",
|
||||
uid: "btn-1",
|
||||
doubleClick: false,
|
||||
timeoutMs: 1234,
|
||||
signal: ctrl.signal,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user