From d30f5803a71bf7ab57230a840c1e180b5a8f7959 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Mon, 13 Apr 2026 12:56:22 -0400 Subject: [PATCH 1/3] fix(browser): harden browser MCP websocket startup - fall back across globalThis.WebSocket, undici, and ws for installed runtimes - normalize socket event handling for both WHATWG and ws implementations - add an installed-copy regression test for missing ws in Bun global layouts --- lib/mcp/ccs-browser-server.cjs | 146 ++++++++++++++---- .../unit/hooks/ccs-browser-mcp-server.test.ts | 63 +++++++- 2 files changed, 176 insertions(+), 33 deletions(-) diff --git a/lib/mcp/ccs-browser-server.cjs b/lib/mcp/ccs-browser-server.cjs index 3182c874..70a0b654 100755 --- a/lib/mcp/ccs-browser-server.cjs +++ b/lib/mcp/ccs-browser-server.cjs @@ -1,6 +1,34 @@ #!/usr/bin/env node -const { WebSocket } = require('ws'); +function loadWebSocketImplementation() { + if (typeof globalThis.WebSocket === 'function') { + return globalThis.WebSocket; + } + + try { + const { WebSocket } = require('undici'); + if (typeof WebSocket === 'function') { + return WebSocket; + } + } catch { + // Fall through to the legacy ws dependency when available. + } + + try { + const { WebSocket } = require('ws'); + if (typeof WebSocket === 'function') { + return WebSocket; + } + } catch { + // Surface a dedicated error below if no implementation is available. + } + + throw new Error( + 'Browser MCP could not find a WebSocket implementation. Tried globalThis.WebSocket, undici, and ws.' + ); +} + +const WebSocket = loadWebSocketImplementation(); const PROTOCOL_VERSION = '2024-11-05'; const SERVER_NAME = 'ccs-browser'; @@ -29,6 +57,66 @@ const NAVIGATION_POLL_INTERVAL_MS = 100; let inputBuffer = Buffer.alloc(0); let requestCounter = 0; +function addSocketListener(socket, eventName, handler) { + if (typeof socket.addEventListener === 'function') { + socket.addEventListener(eventName, handler); + return; + } + + if (typeof socket.on === 'function') { + socket.on(eventName, handler); + } +} + +async function getSocketMessageText(message) { + const data = message && typeof message === 'object' && 'data' in message ? message.data : message; + + if (typeof data === 'string') { + return data; + } + + if (Buffer.isBuffer(data)) { + return data.toString('utf8'); + } + + if (data instanceof ArrayBuffer) { + return Buffer.from(data).toString('utf8'); + } + + if (ArrayBuffer.isView(data)) { + return Buffer.from(data.buffer, data.byteOffset, data.byteLength).toString('utf8'); + } + + if (data && typeof data.text === 'function') { + return await data.text(); + } + + return String(data); +} + +function closeSocket(socket) { + if (typeof socket.close === 'function') { + socket.close(); + } +} + +function abortSocket(socket) { + if (typeof socket.terminate === 'function') { + socket.terminate(); + return; + } + + closeSocket(socket); +} + +function toSocketError(error) { + if (error instanceof Error) { + return error; + } + + return new Error('Browser MCP lost the DevTools websocket connection.'); +} + function shouldExposeTools() { return Boolean(process.env.CCS_BROWSER_DEVTOOLS_HTTP_URL); } @@ -298,7 +386,7 @@ async function sendCdpCommand(page, method, params = {}) { const timer = setTimeout(() => { if (!settled) { settled = true; - ws.terminate(); + abortSocket(ws); reject(new Error('Browser MCP timed out waiting for a DevTools response.')); } }, CDP_TIMEOUT_MS); @@ -309,10 +397,10 @@ async function sendCdpCommand(page, method, params = {}) { } clearTimeout(timer); settled = true; - reject(error); + reject(toSocketError(error)); } - ws.on('open', () => { + addSocketListener(ws, 'open', () => { ws.send( JSON.stringify({ id: requestId, @@ -322,39 +410,43 @@ async function sendCdpCommand(page, method, params = {}) { ); }); - ws.on('message', (data) => { - if (settled) { - return; - } + addSocketListener(ws, 'message', (data) => { + void (async () => { + const raw = await getSocketMessageText(data); - let message; - try { - message = JSON.parse(data.toString()); - } catch { - return; - } + if (settled) { + return; + } - if (message.id !== requestId) { - return; - } + let message; + try { + message = JSON.parse(raw); + } catch { + return; + } - clearTimeout(timer); - settled = true; - ws.close(); + if (message.id !== requestId) { + return; + } - if (message.error) { - reject(new Error(message.error.message || 'DevTools request failed.')); - return; - } + clearTimeout(timer); + settled = true; + closeSocket(ws); - resolve(message.result || null); + if (message.error) { + reject(new Error(message.error.message || 'DevTools request failed.')); + return; + } + + resolve(message.result || null); + })().catch(settleError); }); - ws.on('error', (error) => { + addSocketListener(ws, 'error', (error) => { settleError(error); }); - ws.on('close', () => { + addSocketListener(ws, 'close', () => { if (settled) { return; } diff --git a/tests/unit/hooks/ccs-browser-mcp-server.test.ts b/tests/unit/hooks/ccs-browser-mcp-server.test.ts index 58cd0db8..a2ce77f6 100644 --- a/tests/unit/hooks/ccs-browser-mcp-server.test.ts +++ b/tests/unit/hooks/ccs-browser-mcp-server.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it } from 'bun:test'; import { spawn } from 'child_process'; -import { mkdtempSync, rmSync } from 'node:fs'; +import { cpSync, mkdirSync, mkdtempSync, rmSync } from 'node:fs'; import * as http from 'node:http'; import { WebSocketServer } from 'ws'; import { tmpdir } from 'node:os'; @@ -50,7 +50,12 @@ type MockPageState = { >; }; -const serverPath = join(process.cwd(), 'lib', 'mcp', 'ccs-browser-server.cjs'); +const bundledServerPath = join(process.cwd(), 'lib', 'mcp', 'ccs-browser-server.cjs'); + +type RunMcpRequestsOptions = { + serverPath?: string; + childEnv?: NodeJS.ProcessEnv; +}; function encodeMessage(message: unknown): string { return `${JSON.stringify(message)}\n`; @@ -162,7 +167,9 @@ function createMockBrowser(pagesInput: MockPageState[]) { }); } - async function start() { + async function start(options: RunMcpRequestsOptions = {}) { + const entryServerPath = options.serverPath || bundledServerPath; + const childEnv = options.childEnv || {}; tempDir = mkdtempSync(join(tmpdir(), 'ccs-browser-mcp-server-')); const port = await new Promise((resolve, reject) => { @@ -404,10 +411,11 @@ function createMockBrowser(pagesInput: MockPageState[]) { }); }); - const child = spawn('node', [serverPath], { + const child = spawn('node', [entryServerPath], { cwd: tempDir, env: { ...process.env, + ...childEnv, CCS_BROWSER_DEVTOOLS_HTTP_URL: `http://127.0.0.1:${port}`, }, stdio: ['pipe', 'pipe', 'pipe'], @@ -438,9 +446,13 @@ function createMockBrowser(pagesInput: MockPageState[]) { return { start, stop }; } -async function runMcpRequests(pages: MockPageState[], requests: JsonRpcMessage[]) { +async function runMcpRequests( + pages: MockPageState[], + requests: JsonRpcMessage[], + options: RunMcpRequestsOptions = {} +) { const browser = createMockBrowser(pages); - const child = await browser.start(); + const child = await browser.start(options); try { const responsesPromise = collectResponses(child, requests.length + 1); @@ -510,6 +522,45 @@ describe('ccs-browser MCP server', () => { } }); + it('works from an installed copy when ws is absent but undici is available via NODE_PATH', async () => { + const installDir = mkdtempSync(join(tmpdir(), 'ccs-browser-installed-copy-')); + const installedServerPath = join(installDir, 'ccs-browser-server.cjs'); + const nodeModulesDir = join(installDir, 'node_modules'); + + try { + cpSync(bundledServerPath, installedServerPath); + mkdirSync(nodeModulesDir, { recursive: true }); + cpSync(join(process.cwd(), 'node_modules', 'undici'), join(nodeModulesDir, 'undici'), { + recursive: true, + }); + + const responses = await runMcpRequests( + [{ id: 'page-1', title: 'Installed Copy', currentUrl: 'https://example.com/' }], + [ + { + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { name: 'browser_get_url_and_title', arguments: {} }, + }, + ], + { + serverPath: installedServerPath, + childEnv: { + NODE_PATH: nodeModulesDir, + }, + } + ); + + const response = responses.find((message) => message.id === 2); + expect((response?.result as { isError?: boolean }).isError).not.toBe(true); + expect(getResponseText(response)).toContain('title: Installed Copy'); + expect(getResponseText(response)).toContain('url: https://example.com/'); + } finally { + rmSync(installDir, { recursive: true, force: true }); + } + }); + it('navigates successfully after readiness polling', async () => { const responses = await runMcpRequests( [ From 3103f3f293b9b09114e32762e92c75770f6dcaf0 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Mon, 13 Apr 2026 13:10:05 -0400 Subject: [PATCH 2/3] test(hooks): harden cross-platform MCP coverage - replace the Windows-brittle chmod read-error setup with a directory-based failure case - force the browser installed-copy regression through the non-global WebSocket path --- tests/e2e/image-analyzer-hook.e2e.test.ts | 12 ++++-------- .../unit/hooks/ccs-browser-mcp-server.test.ts | 19 ++++++++++--------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/tests/e2e/image-analyzer-hook.e2e.test.ts b/tests/e2e/image-analyzer-hook.e2e.test.ts index 332f95da..32607646 100644 --- a/tests/e2e/image-analyzer-hook.e2e.test.ts +++ b/tests/e2e/image-analyzer-hook.e2e.test.ts @@ -871,12 +871,10 @@ describe('Image Analyzer Hook', () => { }); it('should output valid JSON structure on file read error', async () => { - // Create and immediately delete file to trigger error + // Use a directory with an analyzable extension so readFileSync fails + // consistently across POSIX and Windows. const errorPath = path.join(TEST_DIR, 'error-test.png'); - createTestPng(errorPath); - - // Make file unreadable (simulate permission error) - fs.chmodSync(errorPath, 0o000); + fs.mkdirSync(errorPath); const result = await invokeHookAsync( { @@ -886,9 +884,7 @@ describe('Image Analyzer Hook', () => { { CCS_IMAGE_ANALYSIS_ENABLED: '1', CCS_PROFILE_TYPE: 'cliproxy' } ); - // Restore permissions and cleanup - fs.chmodSync(errorPath, 0o644); - fs.unlinkSync(errorPath); + fs.rmSync(errorPath, { recursive: true, force: true }); // Should output error in JSON format expect(result.code).toBe(2); diff --git a/tests/unit/hooks/ccs-browser-mcp-server.test.ts b/tests/unit/hooks/ccs-browser-mcp-server.test.ts index a2ce77f6..6cab5f48 100644 --- a/tests/unit/hooks/ccs-browser-mcp-server.test.ts +++ b/tests/unit/hooks/ccs-browser-mcp-server.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it } from 'bun:test'; import { spawn } from 'child_process'; -import { cpSync, mkdirSync, mkdtempSync, rmSync } from 'node:fs'; +import { cpSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import * as http from 'node:http'; import { WebSocketServer } from 'ws'; import { tmpdir } from 'node:os'; @@ -522,17 +522,18 @@ describe('ccs-browser MCP server', () => { } }); - it('works from an installed copy when ws is absent but undici is available via NODE_PATH', async () => { + it('works from an installed copy when global WebSocket is unavailable and NODE_PATH supplies package dependencies', async () => { const installDir = mkdtempSync(join(tmpdir(), 'ccs-browser-installed-copy-')); const installedServerPath = join(installDir, 'ccs-browser-server.cjs'); - const nodeModulesDir = join(installDir, 'node_modules'); + const bootstrapServerPath = join(installDir, 'bootstrap.cjs'); try { cpSync(bundledServerPath, installedServerPath); - mkdirSync(nodeModulesDir, { recursive: true }); - cpSync(join(process.cwd(), 'node_modules', 'undici'), join(nodeModulesDir, 'undici'), { - recursive: true, - }); + writeFileSync( + bootstrapServerPath, + 'delete globalThis.WebSocket;\nrequire("./ccs-browser-server.cjs");\n', + 'utf8' + ); const responses = await runMcpRequests( [{ id: 'page-1', title: 'Installed Copy', currentUrl: 'https://example.com/' }], @@ -545,9 +546,9 @@ describe('ccs-browser MCP server', () => { }, ], { - serverPath: installedServerPath, + serverPath: bootstrapServerPath, childEnv: { - NODE_PATH: nodeModulesDir, + NODE_PATH: join(process.cwd(), 'node_modules'), }, } ); From 394d7d94fc77144c25cdd84195805fbec2966233 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Mon, 13 Apr 2026 15:09:18 -0400 Subject: [PATCH 3/3] test(update): isolate current-install command deps - load a fresh update-command module instance per test via query-string import - prevent Bun mock.module leakage from command-router tests from masking handleUpdateCommand in CI --- .../update-command-current-install.test.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/unit/commands/update-command-current-install.test.ts b/tests/unit/commands/update-command-current-install.test.ts index c1437f64..0d9efd12 100644 --- a/tests/unit/commands/update-command-current-install.test.ts +++ b/tests/unit/commands/update-command-current-install.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it } from 'bun:test'; -import { handleUpdateCommand, type UpdateCommandDeps } from '../../../src/commands/update-command'; +import type { UpdateCommandDeps } from '../../../src/commands/update-command'; import type { UpdateResult } from '../../../src/utils/update-checker'; let logLines: string[] = []; @@ -99,6 +99,13 @@ function createDeps(overrides: Partial = {}): UpdateCommandDe }; } +async function loadHandleUpdateCommand() { + const mod = await import( + `../../../src/commands/update-command?test=${Date.now()}-${Math.random()}` + ); + return mod.handleUpdateCommand; +} + beforeEach(() => { logLines = []; spawnCalls = []; @@ -118,6 +125,7 @@ beforeEach(() => { describe('update-command current install handling', () => { it('updates through the current install manager and prefix', async () => { + const handleUpdateCommand = await loadHandleUpdateCommand(); await handleUpdateCommand({ beta: true }, createDeps()); const installCall = spawnCalls.find((call) => call.args.includes('install')); @@ -129,6 +137,7 @@ describe('update-command current install handling', () => { }); it('fails when another manager updated elsewhere but the current binary stayed stale', async () => { + const handleUpdateCommand = await loadHandleUpdateCommand(); scenario = { beforeState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, afterState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, @@ -144,6 +153,7 @@ describe('update-command current install handling', () => { }); it('keeps force mode under exact target-version verification', async () => { + const handleUpdateCommand = await loadHandleUpdateCommand(); scenario = { beforeState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, afterState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, @@ -156,6 +166,7 @@ describe('update-command current install handling', () => { }); it('warns but succeeds when target resolution says no update and the current install stays unchanged', async () => { + const handleUpdateCommand = await loadHandleUpdateCommand(); scenario = { beforeState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, afterState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, @@ -169,6 +180,7 @@ describe('update-command current install handling', () => { }); it('warns but succeeds when target version resolution fails and the current install stays unchanged', async () => { + const handleUpdateCommand = await loadHandleUpdateCommand(); scenario = { beforeState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, afterState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, @@ -182,6 +194,7 @@ describe('update-command current install handling', () => { }); it('uses the injected version in the no-update message', async () => { + const handleUpdateCommand = await loadHandleUpdateCommand(); updateCheckResult = { status: 'no_update' }; await handleUpdateCommand( @@ -196,6 +209,7 @@ describe('update-command current install handling', () => { }); it('accepts a newer installed version when the dist-tag moves during update', async () => { + const handleUpdateCommand = await loadHandleUpdateCommand(); scenario = { beforeState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, afterState: { version: '7.67.1-dev.0', packageJsonMtimeMs: 200, scriptMtimeMs: 200 }, @@ -208,6 +222,7 @@ describe('update-command current install handling', () => { }); it('accepts force reinstall when the version stays the same but the current install files change', async () => { + const handleUpdateCommand = await loadHandleUpdateCommand(); scenario = { beforeState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 100, scriptMtimeMs: 100 }, afterState: { version: '7.67.0-dev.5', packageJsonMtimeMs: 200, scriptMtimeMs: 200 }, @@ -229,6 +244,7 @@ describe('update-command current install handling', () => { ])( 'routes updates through the current %s install and env', async (manager, expectedArg, envKey, envValue) => { + const handleUpdateCommand = await loadHandleUpdateCommand(); currentInstallOverride = { ...installDescriptor(), manager: manager as 'bun' | 'yarn' | 'pnpm',