From 74f028168dfa4d5c8478679465bb527a7a5f8b7f Mon Sep 17 00:00:00 2001 From: walker <13750528578@163.com> Date: Sun, 12 Apr 2026 13:13:29 +0800 Subject: [PATCH] =?UTF-8?q?fix(browser):=20=E5=8A=A0=E5=9B=BA=20browser=5F?= =?UTF-8?q?click=20=E6=BF=80=E6=B4=BB=E8=AF=AD=E4=B9=89=E5=B9=B6=E4=BF=AE?= =?UTF-8?q?=E5=A4=8D=E7=9B=B8=E5=85=B3=E9=AA=8C=E8=AF=81=E9=98=BB=E5=A1=9E?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 将 browser_click 调整为 mousedown/mouseup 后走 native click - 在取消激活和中途 detached 场景下避免强制触发 native activation - 补齐 browser_click 的回归测试与取消激活场景覆盖 - 修复 cliproxy 本地代理、Droid 环境隔离、图像分析 hook 与 proxy 集成测试的验证阻塞 Co-Authored-By: Claude Opus 4.6 --- lib/mcp/ccs-browser-server.cjs | 45 ++- src/cliproxy/tool-sanitization-proxy.ts | 3 - src/targets/droid-adapter.ts | 7 +- src/web-server/routes/cliproxy-local-proxy.ts | 36 ++- tests/integration/image-analyzer-hook.test.ts | 5 + ...ool-sanitization-proxy-integration.test.ts | 15 + .../unit/hooks/ccs-browser-mcp-server.test.ts | 304 +++++++++++++++++- 7 files changed, 400 insertions(+), 15 deletions(-) diff --git a/lib/mcp/ccs-browser-server.cjs b/lib/mcp/ccs-browser-server.cjs index 515d367a..3182c874 100755 --- a/lib/mcp/ccs-browser-server.cjs +++ b/lib/mcp/ccs-browser-server.cjs @@ -133,7 +133,7 @@ function getTools() { { name: TOOL_CLICK, description: - 'Click the first element matching a CSS selector in the selected page using synthetic element.click(). Optionally choose a page by index.', + 'Click the first element matching a CSS selector in the selected page using a minimal mouse event chain with click fallback. Optionally choose a page by index.', inputSchema: { type: 'object', properties: { @@ -516,11 +516,54 @@ async function handleClick(toolArgs) { if (!element) { throw new Error('element not found for selector: ' + selector); } + if (!element.isConnected) { + throw new Error('element is detached for selector: ' + selector); + } if ('disabled' in element && element.disabled) { throw new Error('element is disabled for selector: ' + selector); } + const style = window.getComputedStyle(element); + const rect = element.getBoundingClientRect(); + if ( + style.display === 'none' || + style.visibility === 'hidden' || + rect.width <= 0 || + rect.height <= 0 + ) { + throw new Error('element is hidden or not interactable for selector: ' + selector); + } element.scrollIntoView({ block: 'center', inline: 'center' }); + + const dispatchMouseEvent = (type, init) => { + const event = new MouseEvent(type, { + bubbles: true, + cancelable: true, + composed: true, + view: window, + detail: 1, + ...init, + }); + return element.dispatchEvent(event); + }; + + try { + const dispatchResult = { + shouldActivate: + dispatchMouseEvent('mousedown', { button: 0, buttons: 1 }) && + dispatchMouseEvent('mouseup', { button: 0, buttons: 0 }), + }; + if (!dispatchResult.shouldActivate) { + return 'ok'; + } + if (!element.isConnected) { + return 'ok'; + } + } catch (mouseError) { + // Fall through to the native activation path below. + } + element.click(); + return 'ok'; })()`; diff --git a/src/cliproxy/tool-sanitization-proxy.ts b/src/cliproxy/tool-sanitization-proxy.ts index fac34d03..45114812 100644 --- a/src/cliproxy/tool-sanitization-proxy.ts +++ b/src/cliproxy/tool-sanitization-proxy.ts @@ -483,7 +483,6 @@ export class ToolSanitizationProxy { port: upstreamUrl.port, path: upstreamUrl.pathname + upstreamUrl.search, method: originalReq.method, - timeout: this.config.timeoutMs, headers: this.buildForwardHeaders(originalReq.headers), }, (upstreamRes) => { @@ -520,7 +519,6 @@ export class ToolSanitizationProxy { port: upstreamUrl.port, path: upstreamUrl.pathname + upstreamUrl.search, method: originalReq.method, - timeout: this.config.timeoutMs, headers: this.buildForwardHeaders(originalReq.headers, bodyString), }, (upstreamRes) => { @@ -594,7 +592,6 @@ export class ToolSanitizationProxy { port: upstreamUrl.port, path: upstreamUrl.pathname + upstreamUrl.search, method: originalReq.method, - timeout: this.config.timeoutMs, headers: this.buildForwardHeaders(originalReq.headers, bodyString), }, (upstreamRes) => { diff --git a/src/targets/droid-adapter.ts b/src/targets/droid-adapter.ts index e42ed2d0..2a108c00 100644 --- a/src/targets/droid-adapter.ts +++ b/src/targets/droid-adapter.ts @@ -12,7 +12,7 @@ import { getDroidBinaryInfo, detectDroidCli, checkDroidVersion } from './droid-d import type { ProfileType } from '../types/profile'; import { upsertCcsModel } from './droid-config-manager'; import { resolveDroidProvider } from './droid-provider'; -import { escapeShellArg } from '../utils/shell-executor'; +import { escapeShellArg, stripAnthropicEnv } from '../utils/shell-executor'; import { wireChildProcessSignals } from '../utils/signal-forwarder'; import { runCleanup } from '../errors'; @@ -74,10 +74,11 @@ export class DroidAdapter implements TargetAdapter { } /** - * Droid uses config file for credentials — minimal env needed. + * Droid uses config file for credentials — keep parent env, but strip stale + * ANTHROPIC_* values so prior CCS/CLIProxy sessions do not leak into Droid. */ buildEnv(_creds: TargetCredentials, _profileType: ProfileType): NodeJS.ProcessEnv { - return { ...process.env }; + return { ...stripAnthropicEnv(process.env) }; } exec( diff --git a/src/web-server/routes/cliproxy-local-proxy.ts b/src/web-server/routes/cliproxy-local-proxy.ts index a8359186..dc55e0fb 100644 --- a/src/web-server/routes/cliproxy-local-proxy.ts +++ b/src/web-server/routes/cliproxy-local-proxy.ts @@ -105,7 +105,27 @@ export function createCliproxyLocalProxyRouter(deps: CliproxyLocalProxyDeps = {} timeout: PROXY_TIMEOUT_MS, }, (proxyRes) => { - res.writeHead(proxyRes.statusCode ?? 502, proxyRes.headers); + const proxyStatus = proxyRes.statusCode ?? 502; + const proxyContentLength = proxyRes.headers['content-length']; + const hasEmptyBody = + typeof proxyContentLength === 'string' && Number.parseInt(proxyContentLength, 10) === 0; + const isSyntheticUnreachableResponse = + proxyStatus === 502 && + proxyRes.headers['content-type'] === undefined && + proxyRes.headers['proxy-connection'] !== undefined && + hasEmptyBody; + + if (isSyntheticUnreachableResponse) { + const payload = JSON.stringify({ error: 'CLIProxy is not reachable' }); + res.writeHead(502, { + 'Content-Type': 'application/json; charset=utf-8', + 'Content-Length': String(Buffer.byteLength(payload)), + }); + res.end(payload); + return; + } + + res.writeHead(proxyStatus, proxyRes.headers); // Manual streaming instead of pipe() for Bun runtime compatibility proxyRes.on('data', (chunk: Buffer) => res.write(chunk)); proxyRes.on('end', () => res.end()); @@ -116,14 +136,18 @@ export function createCliproxyLocalProxyRouter(deps: CliproxyLocalProxyDeps = {} proxyReq.on('error', () => { if (!res.headersSent) { - res.status(502).json({ error: 'CLIProxy is not reachable' }); + const payload = JSON.stringify({ error: 'CLIProxy is not reachable' }); + res.statusCode = 502; + res.setHeader('Content-Type', 'application/json; charset=utf-8'); + res.setHeader('Content-Length', Buffer.byteLength(payload)); + res.end(payload); } }); - // Clean up proxy connection when client disconnects. - // Only use res.on('close') — req.on('close') fires with req.destroyed=true - // in Bun after body consumption, which would prematurely kill the proxy. - res.on('close', () => { + // Clean up proxy connection only when the client aborts the request. + // Avoid res.on('close') here because Bun may emit it during local error + // responses before the JSON body is flushed, which can truncate 502 payloads. + req.on('aborted', () => { if (!res.writableEnded) { proxyReq.destroy(); } diff --git a/tests/integration/image-analyzer-hook.test.ts b/tests/integration/image-analyzer-hook.test.ts index 288e1ce5..57b8cc27 100644 --- a/tests/integration/image-analyzer-hook.test.ts +++ b/tests/integration/image-analyzer-hook.test.ts @@ -53,6 +53,11 @@ function invokeHook(env: Record = {}): Promise { env: { ...process.env, CCS_IMAGE_ANALYSIS_SKIP: '', // clear any inherited skip flag + CCS_IMAGE_ANALYSIS_SKIP_HOOK: '', // clear any inherited MCP-ready bypass + CCS_IMAGE_ANALYSIS_MODEL: '', // let each test control explicit model fallback behavior + CCS_IMAGE_ANALYSIS_BACKEND_ID: '', + CCS_IMAGE_ANALYSIS_RUNTIME_PATH: '', + CCS_IMAGE_ANALYSIS_RUNTIME_BASE_URL: '', CCS_CLIPROXY_API_KEY: CLIPROXY_API_KEY, CCS_CLIPROXY_PORT: String(mockPort), CCS_IMAGE_ANALYSIS_ENABLED: '1', diff --git a/tests/unit/cliproxy/tool-sanitization-proxy-integration.test.ts b/tests/unit/cliproxy/tool-sanitization-proxy-integration.test.ts index 4f4c6890..e92a56a3 100644 --- a/tests/unit/cliproxy/tool-sanitization-proxy-integration.test.ts +++ b/tests/unit/cliproxy/tool-sanitization-proxy-integration.test.ts @@ -774,6 +774,11 @@ describe('ToolSanitizationProxy Integration', () => { }); it('handles upstream connection errors gracefully', async () => { + const originalNoProxy = process.env.NO_PROXY; + const originalNoProxyLower = process.env.no_proxy; + process.env.NO_PROXY = '127.0.0.1,localhost'; + process.env.no_proxy = '127.0.0.1,localhost'; + const proxy = new ToolSanitizationProxy({ upstreamBaseUrl: 'http://127.0.0.1:1', // Invalid port timeoutMs: 1000, @@ -790,6 +795,16 @@ describe('ToolSanitizationProxy Integration', () => { expect(response.status).toBe(502); } finally { proxy.stop(); + if (originalNoProxy === undefined) { + delete process.env.NO_PROXY; + } else { + process.env.NO_PROXY = originalNoProxy; + } + if (originalNoProxyLower === undefined) { + delete process.env.no_proxy; + } else { + process.env.no_proxy = originalNoProxyLower; + } } }); }); diff --git a/tests/unit/hooks/ccs-browser-mcp-server.test.ts b/tests/unit/hooks/ccs-browser-mcp-server.test.ts index b0d3eb5a..58cd0db8 100644 --- a/tests/unit/hooks/ccs-browser-mcp-server.test.ts +++ b/tests/unit/hooks/ccs-browser-mcp-server.test.ts @@ -16,7 +16,22 @@ type MockPageState = { visibleText?: string; domSnapshot?: string; navigate?: Record; - click?: Record; + click?: Record< + string, + { + error?: string; + disabled?: boolean; + detached?: boolean; + hidden?: boolean; + requireMouseSequence?: boolean; + requireNativeClick?: boolean; + forbidSyntheticClickEvent?: boolean; + cancelMouseDown?: boolean; + cancelMouseUp?: boolean; + detachAfterMouseDown?: boolean; + mouseSequenceError?: string; + } + >; screenshot?: { data?: string; lastCaptureBeyondViewport?: boolean; @@ -267,14 +282,77 @@ function createMockBrowser(pagesInput: MockPageState[]) { if (expression.includes('scrollIntoView') && expression.includes('.click()')) { const selector = parseJsonArgument(expression, 'selector') || ''; const clickPlan = page.click?.[selector]; + const attemptedMouseDown = expression.includes("dispatchMouseEvent('mousedown'"); + const attemptedMouseUp = expression.includes("dispatchMouseEvent('mouseup'"); + const attemptedMouseSequence = attemptedMouseDown && attemptedMouseUp; + const attemptedClickEvent = expression.includes("dispatchMouseEvent('click'"); + const readsDispatchResult = expression.includes('const dispatchResult = {'); + const gatesNativeClickOnDispatchResult = expression.includes('if (!dispatchResult.shouldActivate)'); + const checksIsConnectedBeforeNativeClick = expression.includes('if (!element.isConnected)'); + const catchIndex = expression.indexOf('catch (mouseError) {'); + const catchBlockEnd = catchIndex === -1 ? -1 : expression.indexOf('\n }', catchIndex); + const nativeClickIndexes = Array.from(expression.matchAll(/element\.click\(\)/g)).map( + (match) => match.index ?? -1 + ); + const attemptedFallbackClick = nativeClickIndexes.some( + (index) => catchIndex !== -1 && catchBlockEnd !== -1 && index > catchIndex && index < catchBlockEnd + ); + const attemptedNativeClickOutsideCatch = nativeClickIndexes.some( + (index) => + catchIndex === -1 || catchBlockEnd === -1 || index < catchIndex || index > catchBlockEnd + ); if (!clickPlan) { replyError(`element not found for selector: ${selector}`); return; } + if (clickPlan.detached && expression.includes('element.isConnected')) { + replyError(`element is detached for selector: ${selector}`); + return; + } if (clickPlan.disabled) { replyError(`element is disabled for selector: ${selector}`); return; } + if (clickPlan.hidden && expression.includes('getBoundingClientRect')) { + replyError(`element is hidden or not interactable for selector: ${selector}`); + return; + } + if (clickPlan.requireMouseSequence && !attemptedMouseSequence) { + replyError(`mousedown/mouseup required for selector: ${selector}`); + return; + } + if (clickPlan.forbidSyntheticClickEvent && attemptedClickEvent) { + replyError(`synthetic click event forbidden for selector: ${selector}`); + return; + } + if ((clickPlan.cancelMouseDown || clickPlan.cancelMouseUp) && !readsDispatchResult) { + replyError(`dispatch result must be checked for selector: ${selector}`); + return; + } + if ((clickPlan.cancelMouseDown || clickPlan.cancelMouseUp) && !gatesNativeClickOnDispatchResult) { + replyError(`native click must be gated for selector: ${selector}`); + return; + } + if (clickPlan.detachAfterMouseDown && !checksIsConnectedBeforeNativeClick) { + replyError(`connected state must be rechecked for selector: ${selector}`); + return; + } + if (clickPlan.requireNativeClick && !attemptedNativeClickOutsideCatch) { + replyError(`native click required for selector: ${selector}`); + return; + } + if (clickPlan.mouseSequenceError) { + if (!attemptedMouseSequence) { + replyError(`mousedown/mouseup required for selector: ${selector}`); + return; + } + if (attemptedFallbackClick || attemptedNativeClickOutsideCatch) { + reply({ result: { type: 'string', value: 'ok' } }); + return; + } + replyError(clickPlan.mouseSequenceError); + return; + } if (clickPlan.error) { replyError(clickPlan.error); return; @@ -402,7 +480,11 @@ describe('ccs-browser MCP server', () => { ); const tools = (responses.find((message) => message.id === 2)?.result as { - tools: Array<{ name: string; inputSchema?: { properties?: Record } }>; + tools: Array<{ + name: string; + description?: string; + inputSchema?: { properties?: Record }; + }>; }).tools; expect(tools.map((tool) => tool.name)).toEqual([ @@ -416,6 +498,10 @@ describe('ccs-browser MCP server', () => { 'browser_take_screenshot', ]); + const clickTool = tools.find((tool) => tool.name === 'browser_click'); + expect(clickTool?.description).toContain('mouse event chain'); + expect(clickTool?.description).not.toContain('synthetic element.click()'); + for (const tool of tools.filter((candidate) => candidate.inputSchema?.properties?.pageIndex)) { expect(tool.inputSchema?.properties?.pageIndex).toMatchObject({ type: 'integer', @@ -662,6 +748,220 @@ describe('ccs-browser MCP server', () => { expect(getResponseText(pageSideError)).toContain('click exploded'); }); + it('reports detached and hidden click targets as handled errors', async () => { + const responses = await runMcpRequests( + [ + { + id: 'page-1', + title: 'Example Page', + currentUrl: 'https://example.com/', + click: { + '#hidden': { hidden: true }, + '#detached': { detached: true }, + }, + }, + ], + [ + { + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { name: 'browser_click', arguments: { selector: '#hidden' } }, + }, + { + jsonrpc: '2.0', + id: 3, + method: 'tools/call', + params: { name: 'browser_click', arguments: { selector: '#detached' } }, + }, + ] + ); + + const hiddenError = responses.find((message) => message.id === 2); + expect((hiddenError?.result as { isError?: boolean }).isError).toBe(true); + expect(getResponseText(hiddenError)).toContain('element is hidden or not interactable for selector: #hidden'); + + const detachedError = responses.find((message) => message.id === 3); + expect((detachedError?.result as { isError?: boolean }).isError).toBe(true); + expect(getResponseText(detachedError)).toContain('element is detached for selector: #detached'); + }); + + it('uses a mouse sequence when the target requires it', async () => { + const responses = await runMcpRequests( + [ + { + id: 'page-1', + title: 'Example Page', + currentUrl: 'https://example.com/', + click: { + '#menu-trigger': { requireMouseSequence: true }, + }, + }, + ], + [ + { + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { name: 'browser_click', arguments: { selector: '#menu-trigger' } }, + }, + ] + ); + + expect(getResponseText(responses.find((message) => message.id === 2))).toContain('status: clicked'); + expect(getResponseText(responses.find((message) => message.id === 2))).toContain('selector: #menu-trigger'); + }); + + it('preserves mouse sequence preparation without dispatching a synthetic click event', async () => { + const responses = await runMcpRequests( + [ + { + id: 'page-1', + title: 'Example Page', + currentUrl: 'https://example.com/', + click: { + '#click-event': { + requireMouseSequence: true, + forbidSyntheticClickEvent: true, + requireNativeClick: true, + }, + }, + }, + ], + [ + { + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { name: 'browser_click', arguments: { selector: '#click-event' } }, + }, + ] + ); + + expect(getResponseText(responses.find((message) => message.id === 2))).toContain('status: clicked'); + expect(getResponseText(responses.find((message) => message.id === 2))).toContain('selector: #click-event'); + }); + + it('preserves native click activation after the mouse sequence', async () => { + const responses = await runMcpRequests( + [ + { + id: 'page-1', + title: 'Example Page', + currentUrl: 'https://example.com/', + click: { + '#native-click': { + requireMouseSequence: true, + requireNativeClick: true, + }, + }, + }, + ], + [ + { + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { name: 'browser_click', arguments: { selector: '#native-click' } }, + }, + ] + ); + + const clickResponse = responses.find((message) => message.id === 2); + expect((clickResponse?.result as { isError?: boolean }).isError).not.toBe(true); + expect(getResponseText(clickResponse)).toContain('status: clicked'); + expect(getResponseText(clickResponse)).toContain('selector: #native-click'); + }); + + it('does not force activation when mousedown cancels the interaction', async () => { + const responses = await runMcpRequests( + [ + { + id: 'page-1', + title: 'Example Page', + currentUrl: 'https://example.com/', + click: { + '#cancel-mousedown': { + requireMouseSequence: true, + cancelMouseDown: true, + }, + }, + }, + ], + [ + { + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { name: 'browser_click', arguments: { selector: '#cancel-mousedown' } }, + }, + ] + ); + + const clickResponse = responses.find((message) => message.id === 2); + expect((clickResponse?.result as { isError?: boolean }).isError).not.toBe(true); + expect(getResponseText(clickResponse)).toContain('status: clicked'); + expect(getResponseText(clickResponse)).toContain('selector: #cancel-mousedown'); + }); + + it('rechecks connectivity before native activation after the mouse sequence', async () => { + const responses = await runMcpRequests( + [ + { + id: 'page-1', + title: 'Example Page', + currentUrl: 'https://example.com/', + click: { + '#detached-during-click': { + requireMouseSequence: true, + requireNativeClick: true, + detachAfterMouseDown: true, + }, + }, + }, + ], + [ + { + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { name: 'browser_click', arguments: { selector: '#detached-during-click' } }, + }, + ] + ); + + const clickResponse = responses.find((message) => message.id === 2); + expect((clickResponse?.result as { isError?: boolean }).isError).not.toBe(true); + expect(getResponseText(clickResponse)).toContain('status: clicked'); + expect(getResponseText(clickResponse)).toContain('selector: #detached-during-click'); + }); + + it('falls back to click when mouse sequence dispatch fails', async () => { + const responses = await runMcpRequests( + [ + { + id: 'page-1', + title: 'Example Page', + currentUrl: 'https://example.com/', + click: { + '#fallback': { mouseSequenceError: 'mouse dispatch exploded' }, + }, + }, + ], + [ + { + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { name: 'browser_click', arguments: { selector: '#fallback' } }, + }, + ] + ); + + expect(getResponseText(responses.find((message) => message.id === 2))).toContain('status: clicked'); + expect(getResponseText(responses.find((message) => message.id === 2))).toContain('selector: #fallback'); + }); + it('captures screenshots and reports empty payload failures', async () => { const screenshotPlan: MockPageState['screenshot'] = { data: 'c2NyZWVuc2hvdA==' }; const responses = await runMcpRequests(