diff --git a/packages/@n8n/mcp-browser-extension/src/__tests__/relayConnection.test.ts b/packages/@n8n/mcp-browser-extension/src/__tests__/relayConnection.test.ts index dce228f1b7c..bedb6dc9524 100644 --- a/packages/@n8n/mcp-browser-extension/src/__tests__/relayConnection.test.ts +++ b/packages/@n8n/mcp-browser-extension/src/__tests__/relayConnection.test.ts @@ -1,69 +1,14 @@ import { RelayConnection } from '../relayConnection'; // --------------------------------------------------------------------------- -// Mocks for chrome.debugger API +// Chrome API mock — single object, access mocks via chrome.debugger.attach etc. // --------------------------------------------------------------------------- -const mockAttach = jest.fn().mockResolvedValue(undefined); -const mockDetach = jest.fn().mockResolvedValue(undefined); - -/** Deterministic CDP targetId for a given chromeTabId. */ function targetIdForTab(chromeTabId: number): string { return `TARGET_${chromeTabId}`; } -/** - * Mock sendCommand: returns Target.getTargetInfo result when called with - * that method (used for agent-created tabs), otherwise returns a generic result. - */ -const mockSendCommand = jest.fn( - async (debuggee: { tabId: number }, method: string, _params?: object) => { - if (method === 'Target.getTargetInfo') { - return await Promise.resolve({ - targetInfo: { targetId: targetIdForTab(debuggee.tabId) }, - }); - } - return await Promise.resolve({}); - }, -); - -/** - * Mock getTargets: returns TargetInfo[] for all known tabs. - * Tests should configure this to return entries for the tabs they register. - */ -const mockGetTargets = jest.fn().mockResolvedValue([]); - -const eventListeners: Array<(...args: unknown[]) => void> = []; -const detachListeners: Array<(...args: unknown[]) => void> = []; - -const mockAddEventListener = jest.fn((fn: (...args: unknown[]) => void) => { - eventListeners.push(fn); -}); -const mockRemoveEventListener = jest.fn((fn: (...args: unknown[]) => void) => { - const idx = eventListeners.indexOf(fn); - if (idx >= 0) eventListeners.splice(idx, 1); -}); -const mockAddDetachListener = jest.fn((fn: (...args: unknown[]) => void) => { - detachListeners.push(fn); -}); -const mockRemoveDetachListener = jest.fn((fn: (...args: unknown[]) => void) => { - const idx = detachListeners.indexOf(fn); - if (idx >= 0) detachListeners.splice(idx, 1); -}); - -const mockTabsGet = jest - .fn() - .mockResolvedValue({ id: 42, title: 'Test Tab', url: 'https://example.com' }); - -/** Helper to create a mock TargetInfo entry for getTargets. */ -function mockTarget(chromeTabId: number): { - id: string; - tabId: number; - type: string; - title: string; - url: string; - attached: boolean; -} { +function mockTarget(chromeTabId: number) { return { id: targetIdForTab(chromeTabId), tabId: chromeTabId, @@ -74,36 +19,53 @@ function mockTarget(chromeTabId: number): { }; } -Object.assign(globalThis, { - chrome: { - debugger: { - attach: mockAttach, - detach: mockDetach, - sendCommand: mockSendCommand, - getTargets: mockGetTargets, - onEvent: { - addListener: mockAddEventListener, - removeListener: mockRemoveEventListener, - }, - onDetach: { - addListener: mockAddDetachListener, - removeListener: mockRemoveDetachListener, - }, - }, - tabs: { - create: jest.fn().mockResolvedValue({ id: 999, title: 'New Tab', url: 'about:blank' }), - remove: jest.fn().mockResolvedValue(undefined), - get: mockTabsGet, - query: jest.fn().mockResolvedValue([]), - }, - storage: { - local: { - get: jest.fn().mockResolvedValue({}), - set: jest.fn().mockResolvedValue(undefined), - }, +const chrome = { + debugger: { + attach: jest.fn().mockResolvedValue(undefined), + detach: jest.fn().mockResolvedValue(undefined), + sendCommand: jest.fn< + Promise, + [debuggee: { tabId: number }, method: string, params?: object] + >(async (debuggee, method) => { + if (method === 'Target.getTargetInfo') { + return await Promise.resolve({ targetInfo: { targetId: targetIdForTab(debuggee.tabId) } }); + } + return await Promise.resolve({}); + }), + getTargets: jest.fn().mockResolvedValue([]), + onEvent: { addListener: jest.fn(), removeListener: jest.fn() }, + onDetach: { addListener: jest.fn(), removeListener: jest.fn() }, + }, + tabs: { + create: jest.fn().mockResolvedValue({ id: 999, title: 'New Tab', url: 'about:blank' }), + remove: jest.fn().mockResolvedValue(undefined), + get: jest.fn().mockResolvedValue({ id: 42, title: 'Test Tab', url: 'https://example.com' }), + query: jest.fn().mockResolvedValue([]), + }, + storage: { + local: { + get: jest.fn().mockResolvedValue({}), + set: jest.fn().mockResolvedValue(undefined), }, }, -}); +}; +Object.assign(globalThis, { chrome }); + +/** Fire the debugger event listener registered by RelayConnection's constructor. */ +function fireDebuggerEvent(source: object, method: string, params?: object) { + const listener = chrome.debugger.onEvent.addListener.mock.calls[0]?.[0] as + | ((...args: unknown[]) => void) + | undefined; + listener?.(source, method, params); +} + +/** Fire the debugger detach listener registered by RelayConnection's constructor. */ +function fireDebuggerDetach(source: object, reason: string) { + const listener = chrome.debugger.onDetach.addListener.mock.calls[0]?.[0] as + | ((...args: unknown[]) => void) + | undefined; + listener?.(source, reason); +} // --------------------------------------------------------------------------- // Minimal WebSocket stub @@ -153,36 +115,23 @@ describe('RelayConnection', () => { beforeEach(() => { jest.clearAllMocks(); - eventListeners.length = 0; - detachListeners.length = 0; - - // Reset mockSendCommand to the default implementation - mockSendCommand.mockImplementation( - async (debuggee: { tabId: number }, method: string, _params?: object) => { - if (method === 'Target.getTargetInfo') { - return await Promise.resolve({ - targetInfo: { targetId: targetIdForTab(debuggee.tabId) }, - }); - } - return await Promise.resolve({}); - }, - ); - - // Default: return empty targets (tests set up their own) - mockGetTargets.mockResolvedValue([]); ws = new MockWebSocket(); relay = new RelayConnection(ws as unknown as WebSocket); }); it('should register chrome.debugger listeners on construction', () => { - expect(mockAddEventListener).toHaveBeenCalledTimes(1); - expect(mockAddDetachListener).toHaveBeenCalledTimes(1); + expect(chrome.debugger.onEvent.addListener).toHaveBeenCalledTimes(1); + expect(chrome.debugger.onDetach.addListener).toHaveBeenCalledTimes(1); }); describe('registerSelectedTabs', () => { it('should resolve CDP targetIds via getTargets without attaching', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(1), mockTarget(2), mockTarget(3)]); + chrome.debugger.getTargets.mockResolvedValueOnce([ + mockTarget(1), + mockTarget(2), + mockTarget(3), + ]); await relay.registerSelectedTabs([1, 2, 3]); const ids = relay.getControlledIds(); @@ -194,15 +143,15 @@ describe('RelayConnection', () => { ]); // Should NOT attach any debuggers (lazy) - expect(mockAttach).not.toHaveBeenCalled(); + expect(chrome.debugger.attach).not.toHaveBeenCalled(); // Should have called getTargets once - expect(mockGetTargets).toHaveBeenCalledTimes(1); + expect(chrome.debugger.getTargets).toHaveBeenCalledTimes(1); }); it('should set the first tab as primary (route commands without id)', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(10), mockTarget(20)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(10), mockTarget(20)]); await relay.registerSelectedTabs([10, 20]); - mockSendCommand.mockResolvedValueOnce({ data: 'ok' }); + chrome.debugger.sendCommand.mockResolvedValueOnce({ data: 'ok' }); const message = JSON.stringify({ id: 1, @@ -213,15 +162,15 @@ describe('RelayConnection', () => { await tick(); // Should lazy-attach and send command to chromeTabId 10 (first registered) - expect(mockAttach).toHaveBeenCalledWith({ tabId: 10 }, '1.3'); - expect(mockSendCommand).toHaveBeenCalledWith({ tabId: 10 }, 'Runtime.evaluate', { + expect(chrome.debugger.attach).toHaveBeenCalledWith({ tabId: 10 }, '1.3'); + expect(chrome.debugger.sendCommand).toHaveBeenCalledWith({ tabId: 10 }, 'Runtime.evaluate', { expression: '1', }); }); it('should skip tabs not found in getTargets', async () => { // Only return targets for tabs 1 and 3, not 2 - mockGetTargets.mockResolvedValueOnce([mockTarget(1), mockTarget(3)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(1), mockTarget(3)]); await relay.registerSelectedTabs([1, 2, 3]); const ids = relay.getControlledIds(); @@ -235,7 +184,7 @@ describe('RelayConnection', () => { describe('addTab / removeTab', () => { it('should add a tab with CDP targetId without attaching', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(42)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); await relay.addTab(42, 'Test', 'https://test.com'); expect(relay.getControlledIds()).toHaveLength(1); @@ -245,7 +194,7 @@ describe('RelayConnection', () => { }); // Should NOT attach (lazy) - expect(mockAttach).not.toHaveBeenCalled(); + expect(chrome.debugger.attach).not.toHaveBeenCalled(); const sent = JSON.parse(ws.sent[0]); expect(sent.method).toBe('tabOpened'); @@ -256,7 +205,7 @@ describe('RelayConnection', () => { }); it('should not add duplicate tabs', async () => { - mockGetTargets.mockResolvedValue([mockTarget(42)]); + chrome.debugger.getTargets.mockResolvedValue([mockTarget(42)]); await relay.addTab(42, 'Test', 'https://test.com'); await relay.addTab(42, 'Test', 'https://test.com'); @@ -265,7 +214,7 @@ describe('RelayConnection', () => { }); it('should remove a tab and send tabClosed event', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(42)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); await relay.addTab(42, 'Test', 'https://test.com'); ws.sent.length = 0; @@ -278,14 +227,14 @@ describe('RelayConnection', () => { }); it('should not detach debugger for unattached tabs', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(42)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); await relay.addTab(42, 'Test', 'https://test.com'); relay.removeTab(42); - expect(mockDetach).not.toHaveBeenCalled(); + expect(chrome.debugger.detach).not.toHaveBeenCalled(); }); it('should close connection when last tab is removed', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(42)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); await relay.addTab(42, 'Test', 'https://test.com'); const onclose = jest.fn(); relay.onclose = onclose; @@ -296,7 +245,7 @@ describe('RelayConnection', () => { }); it('should keep connection when one of multiple tabs is removed', async () => { - mockGetTargets.mockResolvedValue([mockTarget(42), mockTarget(43)]); + chrome.debugger.getTargets.mockResolvedValue([mockTarget(42), mockTarget(43)]); await relay.addTab(42, 'A', 'https://a.com'); await relay.addTab(43, 'B', 'https://b.com'); @@ -306,7 +255,7 @@ describe('RelayConnection', () => { }); it('should silently skip addTab when target not found', async () => { - mockGetTargets.mockResolvedValueOnce([]); // No targets + chrome.debugger.getTargets.mockResolvedValueOnce([]); // No targets await relay.addTab(42, 'Test', 'https://test.com'); expect(relay.getControlledIds()).toHaveLength(0); expect(ws.sent).toHaveLength(0); @@ -315,11 +264,11 @@ describe('RelayConnection', () => { describe('listRegisteredTabs', () => { it('should return tab metadata with CDP targetIds', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(42)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); await relay.registerSelectedTabs([42]); ws.sent.length = 0; - mockTabsGet.mockResolvedValueOnce({ + chrome.tabs.get.mockResolvedValueOnce({ id: 42, title: 'Example', url: 'https://example.com', @@ -328,7 +277,7 @@ describe('RelayConnection', () => { ws.onmessage?.({ data: JSON.stringify({ id: 1, method: 'listRegisteredTabs' }) }); await tick(); - expect(mockAttach).not.toHaveBeenCalled(); + expect(chrome.debugger.attach).not.toHaveBeenCalled(); expect(ws.sent).toHaveLength(1); const response = JSON.parse(ws.sent[0]); expect(response.id).toBe(1); @@ -343,11 +292,11 @@ describe('RelayConnection', () => { describe('forwardCDPCommand (lazy attach)', () => { it('should lazy-attach debugger on first CDP command', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(123)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(123)]); await relay.registerSelectedTabs([123]); const tabId = relay.getControlledIds()[0].targetId; ws.sent.length = 0; - mockSendCommand.mockResolvedValueOnce({ data: 'test-result' }); + chrome.debugger.sendCommand.mockResolvedValueOnce({ data: 'test-result' }); ws.onmessage?.({ data: JSON.stringify({ @@ -358,19 +307,19 @@ describe('RelayConnection', () => { }); await tick(); - expect(mockAttach).toHaveBeenCalledTimes(1); - expect(mockAttach).toHaveBeenCalledWith({ tabId: 123 }, '1.3'); - expect(mockSendCommand).toHaveBeenCalledWith({ tabId: 123 }, 'Runtime.evaluate', { + expect(chrome.debugger.attach).toHaveBeenCalledTimes(1); + expect(chrome.debugger.attach).toHaveBeenCalledWith({ tabId: 123 }, '1.3'); + expect(chrome.debugger.sendCommand).toHaveBeenCalledWith({ tabId: 123 }, 'Runtime.evaluate', { expression: '1+1', }); }); it('should not re-attach on subsequent commands', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(123)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(123)]); await relay.registerSelectedTabs([123]); const tabId = relay.getControlledIds()[0].targetId; ws.sent.length = 0; - mockSendCommand.mockResolvedValue({ data: 'result' }); + chrome.debugger.sendCommand.mockResolvedValueOnce({ data: 'result' }); ws.onmessage?.({ data: JSON.stringify({ @@ -390,16 +339,16 @@ describe('RelayConnection', () => { }); await tick(); - expect(mockAttach).toHaveBeenCalledTimes(1); - expect(mockSendCommand).toHaveBeenCalledTimes(2); + expect(chrome.debugger.attach).toHaveBeenCalledTimes(1); + expect(chrome.debugger.sendCommand).toHaveBeenCalledTimes(2); }); it('should route CDP commands to specific tab by CDP targetId', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(10), mockTarget(20)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(10), mockTarget(20)]); await relay.registerSelectedTabs([10, 20]); const ids = relay.getControlledIds(); ws.sent.length = 0; - mockSendCommand.mockResolvedValueOnce({ data: 'result' }); + chrome.debugger.sendCommand.mockResolvedValueOnce({ data: 'result' }); // Send to second tab (chromeTabId 20) ws.onmessage?.({ @@ -415,8 +364,8 @@ describe('RelayConnection', () => { }); await tick(); - expect(mockAttach).toHaveBeenCalledWith({ tabId: 20 }, '1.3'); - expect(mockSendCommand).toHaveBeenCalledWith({ tabId: 20 }, 'Page.navigate', { + expect(chrome.debugger.attach).toHaveBeenCalledWith({ tabId: 20 }, '1.3'); + expect(chrome.debugger.sendCommand).toHaveBeenCalledWith({ tabId: 20 }, 'Page.navigate', { url: 'https://example.com', }); }); @@ -438,7 +387,7 @@ describe('RelayConnection', () => { describe('isAgentCreatedTab', () => { it('should return false for non-agent tabs', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(42)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); await relay.registerSelectedTabs([42]); expect(relay.isAgentCreatedTab(42)).toBe(false); }); @@ -462,7 +411,7 @@ describe('RelayConnection', () => { expect(relay.isAgentCreatedTab(999)).toBe(true); // Agent-created tabs ARE eagerly attached - expect(mockAttach).toHaveBeenCalledWith({ tabId: 999 }, '1.3'); + expect(chrome.debugger.attach).toHaveBeenCalledWith({ tabId: 999 }, '1.3'); // Response should have CDP targetId, not chromeTabId const response = JSON.parse(ws.sent[0]); @@ -481,10 +430,10 @@ describe('RelayConnection', () => { }); it('should only detach attached debuggees on close', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(42), mockTarget(43)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42), mockTarget(43)]); await relay.registerSelectedTabs([42, 43]); const ids = relay.getControlledIds(); - mockSendCommand.mockResolvedValueOnce({}); + chrome.debugger.sendCommand.mockResolvedValueOnce({}); // Attach only one tab via CDP command ws.onmessage?.({ @@ -500,20 +449,19 @@ describe('RelayConnection', () => { relay.close('test'); expect(ws.closed).toBe(true); - expect(mockRemoveEventListener).toHaveBeenCalledTimes(1); - expect(mockRemoveDetachListener).toHaveBeenCalledTimes(1); + expect(chrome.debugger.onEvent.removeListener).toHaveBeenCalledTimes(1); + expect(chrome.debugger.onDetach.removeListener).toHaveBeenCalledTimes(1); // Only detach the one that was attached (chromeTabId 42) - expect(mockDetach).toHaveBeenCalledTimes(1); - expect(mockDetach).toHaveBeenCalledWith({ tabId: 42 }); + expect(chrome.debugger.detach).toHaveBeenCalledTimes(1); + expect(chrome.debugger.detach).toHaveBeenCalledWith({ tabId: 42 }); }); it('should forward debugger events with CDP targetId', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(42)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); await relay.addTab(42, 'Test', 'https://test.com'); ws.sent.length = 0; - const listener = eventListeners[0]; - listener({ tabId: 42 }, 'Page.loadEventFired', { timestamp: 123 }); + fireDebuggerEvent({ tabId: 42 }, 'Page.loadEventFired', { timestamp: 123 }); expect(ws.sent).toHaveLength(1); const event = JSON.parse(ws.sent[0]); @@ -524,24 +472,22 @@ describe('RelayConnection', () => { }); it('should ignore debugger events for uncontrolled tabs', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(42)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); await relay.addTab(42, 'Test', 'https://test.com'); ws.sent.length = 0; - const listener = eventListeners[0]; - listener({ tabId: 99 }, 'Page.loadEventFired', {}); + fireDebuggerEvent({ tabId: 99 }, 'Page.loadEventFired', {}); expect(ws.sent).toHaveLength(0); }); it('should remove tab on debugger detach and close if no tabs left', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(42)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); await relay.addTab(42, 'Test', 'https://test.com'); const onclose = jest.fn(); relay.onclose = onclose; - const detachListener = detachListeners[0]; - detachListener({ tabId: 42 }, 'target_closed'); + fireDebuggerDetach({ tabId: 42 }, 'target_closed'); expect(relay.getControlledIds()).toEqual([]); expect(ws.closed).toBe(true); @@ -549,12 +495,11 @@ describe('RelayConnection', () => { }); it('should keep connection alive when one of multiple tabs detaches', async () => { - mockGetTargets.mockResolvedValue([mockTarget(42), mockTarget(43)]); + chrome.debugger.getTargets.mockResolvedValue([mockTarget(42), mockTarget(43)]); await relay.addTab(42, 'A', 'https://a.com'); await relay.addTab(43, 'B', 'https://b.com'); - const detachListener = detachListeners[0]; - detachListener({ tabId: 42 }, 'target_closed'); + fireDebuggerDetach({ tabId: 42 }, 'target_closed'); expect(relay.getControlledIds()).toHaveLength(1); expect(ws.closed).toBe(false); @@ -577,7 +522,7 @@ describe('RelayConnection', () => { }); it('should reject closeTab when tab closing is disabled', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(42)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); await relay.addTab(42, 'Test', 'https://test.com'); const addedId = relay.getControlledIds()[0].targetId; relay.setSettings({ allowTabCreation: true, allowTabClosing: false }); @@ -598,7 +543,7 @@ describe('RelayConnection', () => { describe('spawned tab helpers', () => { it('isControlledTab returns true for registered tabs', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(10), mockTarget(20)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(10), mockTarget(20)]); await relay.registerSelectedTabs([10, 20]); expect(relay.isControlledTab(10)).toBe(true); expect(relay.isControlledTab(20)).toBe(true); @@ -606,7 +551,7 @@ describe('RelayConnection', () => { }); it('isControlledTab returns true for dynamically added tabs', async () => { - mockGetTargets.mockResolvedValueOnce([mockTarget(42)]); + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); await relay.addTab(42, 'Test', 'https://test.com'); expect(relay.isControlledTab(42)).toBe(true); }); @@ -626,7 +571,7 @@ describe('RelayConnection', () => { }); it('markAsAgentCreated tabs are cleaned up on removeTab', async () => { - mockGetTargets.mockResolvedValue([mockTarget(50), mockTarget(51)]); + chrome.debugger.getTargets.mockResolvedValue([mockTarget(50), mockTarget(51)]); await relay.addTab(50, 'Spawned', 'https://spawned.com'); relay.markAsAgentCreated(50); expect(relay.isAgentCreatedTab(50)).toBe(true); @@ -636,4 +581,265 @@ describe('RelayConnection', () => { expect(relay.isAgentCreatedTab(50)).toBe(false); }); }); + + // Helper: register a tab and force lazy-attach by sending a CDP command + async function registerAndAttach(tabId: number) { + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(tabId)]); + await relay.registerSelectedTabs([tabId]); + ws.onmessage?.({ + data: JSON.stringify({ + id: 99, + method: 'forwardCDPCommand', + params: { method: 'Runtime.evaluate', params: {} }, + }), + }); + await tick(); + ws.sent.length = 0; + chrome.debugger.sendCommand.mockClear(); + } + + describe('restricted child target filtering', () => { + it('should filter chrome-extension:// child targets and detach them', async () => { + await registerAndAttach(42); + + fireDebuggerEvent({ tabId: 42 }, 'Target.attachedToTarget', { + sessionId: 's1', + targetInfo: { url: 'chrome-extension://abc/page.html' }, + }); + + // Should not forward to relay + expect(ws.sent).toHaveLength(0); + // Should detach the restricted child + expect(chrome.debugger.sendCommand).toHaveBeenCalledWith( + { tabId: 42 }, + 'Target.detachFromTarget', + { sessionId: 's1' }, + ); + }); + + it('should filter chrome:// child targets', async () => { + await registerAndAttach(42); + + fireDebuggerEvent({ tabId: 42 }, 'Target.attachedToTarget', { + sessionId: 's2', + targetInfo: { url: 'chrome://settings' }, + }); + + expect(ws.sent).toHaveLength(0); + expect(chrome.debugger.sendCommand).toHaveBeenCalledWith( + { tabId: 42 }, + 'Target.detachFromTarget', + { sessionId: 's2' }, + ); + }); + + it('should forward normal child targets', async () => { + await registerAndAttach(42); + + fireDebuggerEvent({ tabId: 42 }, 'Target.attachedToTarget', { + sessionId: 's3', + targetInfo: { url: 'https://example.com/iframe' }, + }); + + expect(ws.sent).toHaveLength(1); + const event = JSON.parse(ws.sent[0]); + expect(event.method).toBe('forwardCDPEvent'); + expect(event.params.method).toBe('Target.attachedToTarget'); + }); + }); + + describe('Target.setAutoAttach caching', () => { + it('should broadcast setAutoAttach to all attached tabs', async () => { + // Register and attach two tabs + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(10), mockTarget(20)]); + await relay.registerSelectedTabs([10, 20]); + + // Attach both via CDP commands + ws.onmessage?.({ + data: JSON.stringify({ + id: 1, + method: 'forwardCDPCommand', + params: { method: 'Runtime.evaluate', params: {}, id: targetIdForTab(10) }, + }), + }); + await tick(); + ws.onmessage?.({ + data: JSON.stringify({ + id: 2, + method: 'forwardCDPCommand', + params: { method: 'Runtime.evaluate', params: {}, id: targetIdForTab(20) }, + }), + }); + await tick(); + chrome.debugger.sendCommand.mockClear(); + + // Send root-level Target.setAutoAttach (no id param) + const autoAttachParams = { autoAttach: true, waitForDebuggerOnStart: false, flatten: true }; + ws.onmessage?.({ + data: JSON.stringify({ + id: 3, + method: 'forwardCDPCommand', + params: { method: 'Target.setAutoAttach', params: autoAttachParams }, + }), + }); + await tick(); + + // Should be sent to both attached tabs + const setAutoAttachCalls = chrome.debugger.sendCommand.mock.calls.filter( + (c: unknown[]) => c[1] === 'Target.setAutoAttach', + ); + expect(setAutoAttachCalls).toHaveLength(2); + expect(setAutoAttachCalls[0][0]).toEqual({ tabId: 10 }); + expect(setAutoAttachCalls[1][0]).toEqual({ tabId: 20 }); + }); + + it('should reapply cached autoAttach when lazily attaching a new tab', async () => { + // Register one tab and send root-level setAutoAttach + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(10)]); + await relay.registerSelectedTabs([10]); + + ws.onmessage?.({ + data: JSON.stringify({ + id: 1, + method: 'forwardCDPCommand', + params: { method: 'Runtime.evaluate', params: {} }, + }), + }); + await tick(); + + const autoAttachParams = { autoAttach: true, waitForDebuggerOnStart: false, flatten: true }; + ws.onmessage?.({ + data: JSON.stringify({ + id: 2, + method: 'forwardCDPCommand', + params: { method: 'Target.setAutoAttach', params: autoAttachParams }, + }), + }); + await tick(); + + // Now add a second tab and trigger lazy attach + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(20)]); + await relay.addTab(20, 'New Tab', 'https://new.com'); + chrome.debugger.sendCommand.mockClear(); + + ws.onmessage?.({ + data: JSON.stringify({ + id: 3, + method: 'forwardCDPCommand', + params: { method: 'DOM.getDocument', params: {}, id: targetIdForTab(20) }, + }), + }); + await tick(); + + // After attach, setAutoAttach should be reapplied to the new tab + const setAutoAttachCalls = chrome.debugger.sendCommand.mock.calls.filter( + (c: unknown[]) => c[1] === 'Target.setAutoAttach', + ); + expect(setAutoAttachCalls).toHaveLength(1); + expect(setAutoAttachCalls[0][0]).toEqual({ tabId: 20 }); + expect(setAutoAttachCalls[0][2]).toEqual(autoAttachParams); + }); + }); + + describe('Runtime.enable workaround', () => { + it('should wait for executionContextCreated before resolving Runtime.enable', async () => { + await registerAndAttach(42); + + // Send Runtime.enable — this registers a one-shot onEvent listener and + // waits for executionContextCreated (with auxData.isDefault === true) before resolving. + ws.onmessage?.({ + data: JSON.stringify({ + id: 1, + method: 'forwardCDPCommand', + params: { method: 'Runtime.enable', params: {} }, + }), + }); + + // Response should not be sent yet — waiting for the context event. + await tick(); + expect(ws.sent.some((s) => JSON.parse(s).id === 1)).toBe(false); + + // Fire the executionContextCreated event via the one-shot listener. + // It is the last listener registered (after the constructor's permanent listener). + const allListeners = chrome.debugger.onEvent.addListener.mock.calls; + const oneShotListener = allListeners[allListeners.length - 1]?.[0] as + | ((...args: unknown[]) => void) + | undefined; + oneShotListener?.({ tabId: 42 }, 'Runtime.executionContextCreated', { + context: { auxData: { isDefault: true } }, + }); + + await tick(); + + const methods = chrome.debugger.sendCommand.mock.calls.map((c: unknown[]) => c[1]); + // Runtime.disable must NOT be called — we use event-based waiting now + expect(methods).not.toContain('Runtime.disable'); + expect(methods).toContain('Runtime.enable'); + // Response was sent after context confirmed + expect(ws.sent.some((s) => JSON.parse(s).id === 1)).toBe(true); + }); + + it('should resolve Runtime.enable after timeout if executionContextCreated never fires', async () => { + await registerAndAttach(42); + + // Install fake timers before sending the message so the 3 000 ms + // timeout inside contextReady is created under fake time control. + jest.useFakeTimers(); + try { + ws.onmessage?.({ + data: JSON.stringify({ + id: 2, + method: 'forwardCDPCommand', + params: { method: 'Runtime.enable', params: {} }, + }), + }); + // Drain microtasks (sendCommand resolves immediately), then advance + // past the 3 000 ms fallback timeout. + await Promise.resolve(); + await jest.advanceTimersByTimeAsync(3_100); + } finally { + jest.useRealTimers(); + } + + const methods = chrome.debugger.sendCommand.mock.calls.map((c: unknown[]) => c[1]); + expect(methods).not.toContain('Runtime.disable'); + expect(methods).toContain('Runtime.enable'); + }); + }); + + describe('typed close reasons', () => { + it('should close with extension_disconnected when last tab is removed', async () => { + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); + await relay.addTab(42, 'Test', 'https://test.com'); + + relay.removeTab(42); + expect(ws.closeReason).toBe('extension_disconnected'); + }); + + it('should close with debugger_detached when last tab debugger detaches', async () => { + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); + await relay.addTab(42, 'Test', 'https://test.com'); + + fireDebuggerDetach({ tabId: 42 }, 'target_closed'); + expect(ws.closeReason).toBe('debugger_detached'); + }); + + it('should close with extension_disconnected when last tab is closed via closeTab', async () => { + chrome.debugger.getTargets.mockResolvedValueOnce([mockTarget(42)]); + await relay.addTab(42, 'Test', 'https://test.com'); + relay.setSettings({ allowTabCreation: true, allowTabClosing: true }); + ws.sent.length = 0; + + ws.onmessage?.({ + data: JSON.stringify({ + id: 1, + method: 'closeTab', + params: { id: targetIdForTab(42) }, + }), + }); + await tick(); + + expect(ws.closeReason).toBe('extension_disconnected'); + }); + }); }); diff --git a/packages/@n8n/mcp-browser-extension/src/background.ts b/packages/@n8n/mcp-browser-extension/src/background.ts index 5568d9378ff..2fd1c0bd8ad 100644 --- a/packages/@n8n/mcp-browser-extension/src/background.ts +++ b/packages/@n8n/mcp-browser-extension/src/background.ts @@ -343,7 +343,7 @@ async function connectToRelay( const settings = await loadSettings(); relay.setSettings(settings); } catch (error) { - relay.close('Setup failed'); + relay.close('network_error'); throw error; } @@ -373,7 +373,7 @@ async function connectToRelay( function disconnect(): void { if (activeConnection) { log.debug('disconnecting'); - activeConnection.relay.close('User disconnected'); + activeConnection.relay.close('extension_disconnected'); activeConnection = null; updateBadge(0); } diff --git a/packages/@n8n/mcp-browser-extension/src/relayConnection.ts b/packages/@n8n/mcp-browser-extension/src/relayConnection.ts index 32643f238fc..44241b931ca 100644 --- a/packages/@n8n/mcp-browser-extension/src/relayConnection.ts +++ b/packages/@n8n/mcp-browser-extension/src/relayConnection.ts @@ -62,6 +62,14 @@ const ATTACH_TIMEOUT_MS = 5_000; // RelayConnection // --------------------------------------------------------------------------- +/** URL prefixes that indicate restricted child targets (extensions, internal pages). */ +function isRestrictedUrl(url: string | undefined): boolean { + if (!url) return false; + if (url.startsWith('chrome-extension://')) return true; + const restrictedPrefixes = ['chrome://', 'devtools://', 'edge://']; + return restrictedPrefixes.some((prefix) => url.startsWith(prefix)); +} + export class RelayConnection { /** Primary map: CDP targetId → Chrome tab state */ private readonly tabs = new Map(); @@ -73,6 +81,8 @@ export class RelayConnection { private readonly agentCreatedChromeTabIds = new Set(); /** The primary tab ID (first registered), used as default target */ private primaryId: string | undefined; + /** Cached Target.setAutoAttach params — reapplied to newly attached tabs for iframe support. */ + private autoAttachParams: object | null = null; private readonly ws: WebSocket; private readonly eventListener: ( @@ -169,7 +179,7 @@ export class RelayConnection { this.sendMessage({ method: 'tabClosed', params: { id } }); if (this.tabs.size === 0) { - this.close('All tabs closed'); + this.close('extension_disconnected'); } } @@ -310,6 +320,19 @@ export class RelayConnection { ]); entry.attached = true; log.debug(`ensureAttached: attached ${id}`); + + // Reapply cached auto-attach so new tabs report iframes immediately + if (this.autoAttachParams) { + try { + await chrome.debugger.sendCommand( + { tabId: entry.chromeTabId }, + 'Target.setAutoAttach', + this.autoAttachParams, + ); + } catch (e) { + log.debug('Failed to apply auto-attach after attach:', e); + } + } })(); this.pendingAttaches.set(id, promise); @@ -329,6 +352,27 @@ export class RelayConnection { const id = this.chromeTabIdToId.get(source.tabId); if (!id) return; + // Filter restricted child targets from auto-attach (extension pages, chrome://, etc.). + // Without this, Chrome's debugger API throws "Cannot access a chrome-extension:// URL + // of a different extension" when the relay tries to send commands to these targets. + if (method === 'Target.attachedToTarget') { + const targetParams = params as + | { sessionId?: string; targetInfo?: { url?: string } } + | undefined; + if (isRestrictedUrl(targetParams?.targetInfo?.url)) { + log.debug('filtering restricted child target:', targetParams?.targetInfo?.url); + // Detach from the restricted child — sent on the parent tab session, not the child + if (targetParams?.sessionId) { + chrome.debugger + .sendCommand({ tabId: source.tabId }, 'Target.detachFromTarget', { + sessionId: targetParams.sessionId, + }) + .catch((e) => log.debug('failed to detach restricted target:', e)); + } + return; + } + } + this.sendMessage({ method: 'forwardCDPEvent', params: { method, params, id }, @@ -356,7 +400,7 @@ export class RelayConnection { this.sendMessage({ method: 'tabClosed', params: { id } }); if (this.tabs.size === 0) { - this.close(`Debugger detached: ${reason}`); + this.close('debugger_detached'); } } @@ -434,6 +478,25 @@ export class RelayConnection { private async handleForwardCDPCommand(params: Record): Promise { const { method, params: cmdParams, id: rawId } = params; + + // Root-level Target.setAutoAttach: cache params and apply to ALL attached tabs. + // This ensures Chrome emits Target.attachedToTarget for cross-origin iframes. + if (method === 'Target.setAutoAttach' && !rawId) { + this.autoAttachParams = (cmdParams as object) ?? null; + const promises: Array> = []; + for (const [, entry] of this.tabs) { + if (!entry.attached) continue; + promises.push( + chrome.debugger + .sendCommand({ tabId: entry.chromeTabId }, 'Target.setAutoAttach', cmdParams as object) + .then(() => {}) + .catch((e) => log.debug('setAutoAttach failed:', e)), + ); + } + await Promise.all(promises); + return {}; + } + const { id, entry } = this.resolveTab(rawId as string | undefined); log.debug(`CDP: ${method as string} → targetId=${id} (chromeTabId=${entry.chromeTabId})`); @@ -443,6 +506,43 @@ export class RelayConnection { const debuggee = { tabId: entry.chromeTabId }; + // Wait for the main-frame execution context before returning from Runtime.enable. + // Without this, Playwright may reference execution contexts that don't exist yet, + // causing "Cannot find context" errors. + // Mirrors Playwriter's approach (playwriter/src/cdp-relay.ts Runtime.enable case): + // wait for Runtime.executionContextCreated with auxData.isDefault === true rather + // than using a fixed delay. auxData.isDefault identifies the top-level frame context + // (the page's window object) as opposed to iframes, workers, or injected worlds. + if (method === 'Runtime.enable') { + const contextReady = new Promise((resolve) => { + const timeout = setTimeout(() => { + log.debug('Runtime.enable: timed out waiting for executionContextCreated, proceeding'); + chrome.debugger.onEvent.removeListener(handler); + resolve(); + }, 3_000); + function handler(src: chrome.debugger.Debuggee, evt: string, evtParams?: object): void { + if (src.tabId !== entry.chromeTabId) return; + if (evt !== 'Runtime.executionContextCreated') return; + const auxData = ( + evtParams as { context?: { auxData?: { isDefault?: boolean } } } | undefined + )?.context?.auxData; + if (auxData?.isDefault !== true) return; + clearTimeout(timeout); + chrome.debugger.onEvent.removeListener(handler); + resolve(); + } + chrome.debugger.onEvent.addListener(handler); + }); + + const result = await chrome.debugger.sendCommand( + debuggee, + method as string, + cmdParams as object | undefined, + ); + await contextReady; + return result; + } + const result = await Promise.race([ chrome.debugger.sendCommand(debuggee, method as string, cmdParams as object | undefined), new Promise((_resolve, reject) => { @@ -479,6 +579,21 @@ export class RelayConnection { this.chromeTabIdToId.set(tab.id, targetId); this.agentCreatedChromeTabIds.add(tab.id); + // Apply cached auto-attach params so the new tab reports iframes immediately. + // ensureAttached() does this for lazily-attached tabs; we must do it here too + // for eagerly-attached agent-created tabs. + if (this.autoAttachParams) { + try { + await chrome.debugger.sendCommand( + { tabId: tab.id }, + 'Target.setAutoAttach', + this.autoAttachParams, + ); + } catch (e) { + log.debug('Failed to apply auto-attach after eager attach:', e); + } + } + log.debug(`createTab: targetId=${targetId} chromeTabId=${tab.id} url=${tab.url ?? url ?? ''}`); return { @@ -523,7 +638,7 @@ export class RelayConnection { } if (this.tabs.size === 0) { - this.close('All tabs closed'); + this.close('extension_disconnected'); } return { closed: true, id }; diff --git a/packages/@n8n/mcp-browser/package.json b/packages/@n8n/mcp-browser/package.json index 94bb09f312e..0b392b0d1fc 100644 --- a/packages/@n8n/mcp-browser/package.json +++ b/packages/@n8n/mcp-browser/package.json @@ -1,6 +1,6 @@ { "name": "@n8n/mcp-browser", - "version": "0.1.0", + "version": "0.2.0", "description": "Browser automation MCP tools built on Playwright, WebDriver BiDi, and safaridriver", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/packages/@n8n/mcp-browser/src/__tests__/cdp-relay.test.ts b/packages/@n8n/mcp-browser/src/__tests__/cdp-relay.test.ts new file mode 100644 index 00000000000..ef9e8f0ff8a --- /dev/null +++ b/packages/@n8n/mcp-browser/src/__tests__/cdp-relay.test.ts @@ -0,0 +1,198 @@ +import { WebSocket } from 'ws'; + +import { CDPRelayServer } from '../cdp-relay'; +import type { ExtensionRequest } from '../cdp-relay-protocol'; +import { ExtensionNotConnectedError } from '../errors'; +import { configureLogger } from '../logger'; + +configureLogger({ level: 'silent' }); + +let relay: CDPRelayServer; +let port: number; + +beforeEach(async () => { + relay = new CDPRelayServer({ connectionTimeoutMs: 2_000 }); + port = await relay.listen(); +}); + +afterEach(() => { + relay.stop(); +}); + +function connectExtension(): WebSocket { + return new WebSocket(relay.extensionEndpoint(port)); +} + +function connectPlaywright(): WebSocket { + return new WebSocket(relay.cdpEndpoint(port)); +} + +async function waitForOpen(ws: WebSocket): Promise { + return await new Promise((resolve, reject) => { + ws.on('open', resolve); + ws.on('error', reject); + }); +} + +function parseWsData(data: unknown): string { + return Buffer.isBuffer(data) ? data.toString('utf8') : String(data); +} + +/** + * Create a fake extension that auto-responds to relay commands. + * Override `handlers` to customize responses. + */ +function createFakeExtension(ws: WebSocket) { + const handlers: Record unknown> = { + listRegisteredTabs: () => ({ + tabs: [{ id: 'tab-1', title: 'Test Page', url: 'https://example.com' }], + }), + forwardCDPCommand: () => ({}), + attachTab: () => ({}), + listTabs: () => ({ + tabs: [{ id: 'tab-1', title: 'Test Page', url: 'https://example.com' }], + }), + }; + + ws.on('message', (data) => { + try { + const msg = JSON.parse(parseWsData(data)) as ExtensionRequest; + const handler = handlers[msg.method]; + if (handler) { + ws.send(JSON.stringify({ id: msg.id, result: handler(msg.params) })); + } + } catch { + // ignore malformed + } + }); + + return { handlers }; +} + +describe('CDPRelayServer', () => { + it('should resolve waitForExtension when extension connects', async () => { + const ext = connectExtension(); + await waitForOpen(ext); + await expect(relay.waitForExtension()).resolves.toBeUndefined(); + ext.close(); + }); + + it('should reject waitForExtension after timeout', async () => { + jest.useFakeTimers(); + relay.stop(); + relay = new CDPRelayServer({ connectionTimeoutMs: 2_000 }); + port = await relay.listen(); + + // Capture the promise before advancing timers + const promise = relay.waitForExtension().catch((e: unknown) => e); + await jest.advanceTimersByTimeAsync(2_100); + + const error = await promise; + expect(error).toBeInstanceOf(ExtensionNotConnectedError); + jest.useRealTimers(); + }); + + it('should report disconnect reason when extension closes with explicit reason', async () => { + const ext = connectExtension(); + await waitForOpen(ext); + await relay.waitForExtension(); + + const reason = await new Promise((resolve) => { + relay.onExtensionDisconnect = (r) => resolve(r); + ext.close(1000, 'browser_closed'); + }); + + expect(reason).toBe('browser_closed'); + }); + + it('should list tabs from extension', async () => { + const ext = connectExtension(); + await waitForOpen(ext); + createFakeExtension(ext); + await relay.waitForExtension(); + + const tabs = await relay.listTabs(); + expect(tabs).toEqual([{ id: 'tab-1', title: 'Test Page', url: 'https://example.com' }]); + + ext.close(); + }); + + it('should forward CDP commands to extension and return response', async () => { + const ext = connectExtension(); + await waitForOpen(ext); + createFakeExtension(ext); + await relay.waitForExtension(); + + const pw = connectPlaywright(); + await waitForOpen(pw); + + // Send Browser.getVersion (handled by relay itself, no extension roundtrip) + const response = await new Promise<{ id: number; result: { product: string } }>((resolve) => { + pw.on('message', (data) => { + try { + resolve(JSON.parse(parseWsData(data)) as { id: number; result: { product: string } }); + } catch { + // ignore malformed + } + }); + pw.send(JSON.stringify({ id: 1, method: 'Browser.getVersion' })); + }); + + expect(response.id).toBe(1); + expect(response.result.product).toContain('Chrome'); + + pw.close(); + ext.close(); + }); + + it('should disconnect extension after heartbeat timeout', async () => { + // Enable fake timers before creating relay so setInterval is captured + jest.useFakeTimers(); + + relay.stop(); + relay = new CDPRelayServer({ connectionTimeoutMs: 2_000 }); + port = await relay.listen(); + + new WebSocket(relay.extensionEndpoint(port), { autoPong: false }); + + // Let WebSocket handshake complete through the event loop + await jest.advanceTimersByTimeAsync(100); + await relay.waitForExtension(); + + const disconnectPromise = new Promise((resolve) => { + relay.onExtensionDisconnect = (r) => resolve(r); + }); + + // Advance past heartbeat interval (5s) + timeout (15s) + await jest.advanceTimersByTimeAsync(20_000); + + const reason = await disconnectPromise; + expect(reason).toBe('heartbeat_timeout'); + + // Restore real timers before afterEach cleanup (ws.close uses setTimeout) + jest.useRealTimers(); + }); + + it('should allow extension to reconnect within grace window', async () => { + const ext1 = connectExtension(); + await waitForOpen(ext1); + createFakeExtension(ext1); + await relay.waitForExtension(); + + // Disconnect first extension and wait for close to propagate + await new Promise((resolve) => { + ext1.on('close', () => resolve()); + ext1.close(1000, 'network_error'); + }); + + const ext2 = connectExtension(); + await waitForOpen(ext2); + createFakeExtension(ext2); + + // Relay should still work with new extension + const tabs = await relay.listTabs(); + expect(tabs).toEqual([{ id: 'tab-1', title: 'Test Page', url: 'https://example.com' }]); + + ext2.close(); + }); +}); diff --git a/packages/@n8n/mcp-browser/src/adapters/playwright.ts b/packages/@n8n/mcp-browser/src/adapters/playwright.ts index d466e80215e..5c53f2b444b 100644 --- a/packages/@n8n/mcp-browser/src/adapters/playwright.ts +++ b/packages/@n8n/mcp-browser/src/adapters/playwright.ts @@ -12,7 +12,12 @@ import type { import { chromium } from 'playwright-core'; import { CDPRelayServer } from '../cdp-relay'; -import { BrowserExecutableNotFoundError, PageNotFoundError, StaleRefError } from '../errors'; +import { + BrowserExecutableNotFoundError, + PageNotFoundError, + StaleRefError, + type ConnectionLostReason, +} from '../errors'; import { createLogger } from '../logger'; import type { ClickOptions, @@ -92,6 +97,9 @@ export class PlaywrightAdapter { /** Pending activation: set by ensurePage(), consumed by context.on('page'). */ private pendingActivation?: { id: string; resolve: (page: Page) => void }; + /** Called when the browser connection is unexpectedly lost. */ + onDisconnect?: (reason: ConnectionLostReason) => void; + constructor(config: ResolvedConfig) { this.resolvedConfig = config; } @@ -165,6 +173,18 @@ export class PlaywrightAdapter { } }); + // Detect unexpected disconnection from the browser (process crash, etc.) + this.browser.on('disconnected', () => { + log.debug('browser disconnected event'); + this.onDisconnect?.('browser_closed'); + }); + + // Detect extension disconnection via the relay (already a typed reason) + this.relay.onExtensionDisconnect = (reason) => { + log.debug('relay: extension disconnected, reason:', reason); + this.onDisconnect?.(reason); + }; + log.debug('launch complete, context ready for lazy activation'); } diff --git a/packages/@n8n/mcp-browser/src/cdp-relay.ts b/packages/@n8n/mcp-browser/src/cdp-relay.ts index 203e1b6ed3c..f10a76c6fa5 100644 --- a/packages/@n8n/mcp-browser/src/cdp-relay.ts +++ b/packages/@n8n/mcp-browser/src/cdp-relay.ts @@ -9,6 +9,7 @@ */ import { randomUUID } from 'node:crypto'; +import { EventEmitter } from 'node:events'; import http from 'node:http'; import type net from 'node:net'; import { WebSocketServer, WebSocket } from 'ws'; @@ -21,11 +22,31 @@ import type { ExtensionEvents, ExtensionResponse, } from './cdp-relay-protocol'; -import { ExtensionNotConnectedError, type ExtensionNotConnectedPhase } from './errors'; +import { + ConnectionLostError, + ExtensionNotConnectedError, + type ConnectionLostReason, + type ExtensionNotConnectedPhase, +} from './errors'; import { createLogger } from './logger'; const log = createLogger('relay'); +// --------------------------------------------------------------------------- +// Restricted target filtering +// --------------------------------------------------------------------------- + +/** Targets that should not be exposed to Playwright (extension pages, service workers, etc.). */ +function isRestrictedTarget(targetInfo: { type?: string; url?: string }): boolean { + const { type, url } = targetInfo; + // Only allow page and iframe targets + if (type && type !== 'page' && type !== 'iframe') return true; + if (!url) return false; + return ['chrome://', 'chrome-extension://', 'devtools://', 'edge://'].some((prefix) => + url.startsWith(prefix), + ); +} + // --------------------------------------------------------------------------- // CDPRelayServer // --------------------------------------------------------------------------- @@ -65,8 +86,18 @@ export class CDPRelayServer { private extensionConnectedReject?: (error: Error) => void; private extensionConnectedPromise: Promise; + /** Called when the extension disconnects with a typed reason. */ + onExtensionDisconnect?: (reason: ConnectionLostReason) => void; + private readonly connectionTimeoutMs: number; + /** Grace period allowing the extension to reconnect before tearing down Playwright. */ + private readonly RECONNECT_WINDOW_MS = 15_000; + private reconnectTimer: ReturnType | undefined; + + /** Internal event bus for CDP events (used to synchronize Runtime.enable). */ + private readonly cdpEvents = new EventEmitter(); + constructor(options?: CDPRelayServerOptions) { this.connectionTimeoutMs = options?.connectionTimeoutMs ?? 15_000; @@ -142,6 +173,10 @@ export class CDPRelayServer { /** Shut down the relay, closing all connections. */ stop(): void { log.debug('stopping relay server'); + if (this.reconnectTimer) { + clearTimeout(this.reconnectTimer); + this.reconnectTimer = undefined; + } this.closePlaywrightConnection('Server stopped'); this.closeExtensionConnection('Server stopped'); this.wss.close(); @@ -333,16 +368,20 @@ export class CDPRelayServer { return {}; case 'Target.setAutoAttach': { - // Child session auto-attach: ack without forwarding - if (sessionId) return {}; + // Child session auto-attach: forward to extension so Chrome attaches to iframes + if (sessionId) { + return await this.forwardToExtension(method, params, sessionId); + } + + // Root-level: forward to extension (applies to all attached tabs) AND list tabs for cache + log.debug('Target.setAutoAttach: forwarding to extension and listing tabs'); + await this.forwardToExtension(method, params, sessionId); - log.debug('Target.setAutoAttach: listing tabs from extension'); const { tabs } = (await this.extensionConn!.send('listRegisteredTabs', {})) as { tabs: Array<{ id: string; title: string; url: string }>; }; log.debug('listRegisteredTabs result:', tabs.length, 'tabs'); - // Cache metadata — don't activate (lazy) for (const tab of tabs) { this.tabCache.set(tab.id, { title: tab.title, url: tab.url }); this.primaryTabId ??= tab.id; @@ -350,6 +389,50 @@ export class CDPRelayServer { return {}; } + case 'Runtime.enable': { + if (!sessionId) { + return await this.forwardToExtension(method, params, sessionId); + } + + // Wait for Chrome to emit executionContextCreated (default context) before + // returning, so Playwright's context cache is populated. Without this there's + // a race where Playwright tries to use the context before it's announced. + const contextCreatedPromise = new Promise((resolve) => { + const handler = (event: { + method: string; + params: unknown; + sessionId: string; + }) => { + if ( + event.method === 'Runtime.executionContextCreated' && + event.sessionId === sessionId + ) { + const ctxParams = event.params as { + context?: { auxData?: { isDefault?: boolean } }; + }; + if (ctxParams?.context?.auxData?.isDefault === true) { + clearTimeout(timer); + this.cdpEvents.off('cdp:event', handler); + resolve(); + } + } + }; + const timer = setTimeout(() => { + this.cdpEvents.off('cdp:event', handler); + log.debug( + 'Runtime.enable: timed out waiting for executionContextCreated, session:', + sessionId, + ); + resolve(); + }, 3_000); + this.cdpEvents.on('cdp:event', handler); + }); + + const result = await this.forwardToExtension(method, params, sessionId); + await contextCreatedPromise; + return result; + } + case 'Target.createTarget': { const createParams = (params ?? {}) as Record; const url = (createParams.url as string) ?? undefined; @@ -396,7 +479,7 @@ export class CDPRelayServer { params: unknown, sessionId: string | undefined, ): Promise { - if (!this.extensionConn) throw new Error('Extension not connected'); + if (!this.extensionConn) throw new ConnectionLostError('extension_disconnected'); // sessionId IS the CDP targetId — pass it directly log.debug('→ EXT: forwardCDPCommand', method, sessionId ? 'id=' + sessionId : '(primary)'); @@ -419,7 +502,7 @@ export class CDPRelayServer { /** Create a new tab via the extension. */ private async createTab(url?: string): Promise<{ id: string; title: string; url: string }> { - if (!this.extensionConn) throw new Error('Extension not connected'); + if (!this.extensionConn) throw new ConnectionLostError('extension_disconnected'); log.debug('createTab: requesting from extension, url =', url); const result = (await this.extensionConn.send('createTab', { url })) as { @@ -456,7 +539,7 @@ export class CDPRelayServer { /** Close a tab via the extension. */ async closeTab(id: string): Promise { - if (!this.extensionConn) throw new Error('Extension not connected'); + if (!this.extensionConn) throw new ConnectionLostError('extension_disconnected'); await this.extensionConn.send('closeTab', { id }); @@ -510,7 +593,11 @@ export class CDPRelayServer { if (this.playwrightWs?.readyState === WebSocket.OPEN) { const json = JSON.stringify(message); log.debug('→ PW:', json.length > 200 ? json.slice(0, 200) + '…' : json); - this.playwrightWs.send(json); + try { + this.playwrightWs.send(json); + } catch { + log.debug('sendToPlaywright: send failed (connection closing)'); + } } else { log.debug('sendToPlaywright: no Playwright connection'); } @@ -521,21 +608,66 @@ export class CDPRelayServer { // ========================================================================= private handleExtensionConnection(ws: WebSocket): void { + // If reconnecting during the grace window, cancel the teardown timer + if (this.reconnectTimer) { + clearTimeout(this.reconnectTimer); + this.reconnectTimer = undefined; + log.debug('extension reconnected within grace window'); + } + if (this.extensionConn) { - log.debug('rejected duplicate extension connection'); - ws.close(1000, 'Another extension already connected'); - return; + // Accept the new connection as a replacement (e.g. reconnect after transient drop). + // Clear the handler first so the async close event from the old connection doesn't + // null out extensionConn or start a spurious reconnect timer after we've already + // installed the replacement. + log.debug('replacing existing extension connection'); + this.extensionConn.onclose = undefined; + this.extensionConn.close('Replaced by new connection'); } log.debug('extension connected'); this.extensionConn = new ExtensionConnection(ws); this.extensionConn.onclose = () => { - log.debug('extension disconnected'); - this.resetState(); - this.closePlaywrightConnection('Extension disconnected'); + const rawReason = this.extensionConn?.closeReason; + log.debug('extension disconnected, reason:', rawReason ?? '(none)'); + this.onExtensionDisconnect?.(asConnectionLostReason(rawReason)); + + // Clear extension ref but KEEP tabCache, activatedTabs, and Playwright connection. + // Pending requests are already rejected by ExtensionConnection.handleClose. + this.extensionConn = null; + + // Start a grace period: if the extension reconnects in time, we rebind + // without tearing down Playwright. If not, we close everything. + this.reconnectTimer = setTimeout(() => { + this.reconnectTimer = undefined; + log.debug('reconnection window expired, closing Playwright'); + this.resetState(); + this.closePlaywrightConnection('Extension did not reconnect'); + }, this.RECONNECT_WINDOW_MS); + + // Re-create the promise so waitForExtension() works for the next connection + this.extensionConnectedPromise = new Promise((resolve, reject) => { + this.extensionConnectedResolve = resolve; + this.extensionConnectedReject = reject; + }); + this.extensionConnectedPromise.catch(() => {}); }; + this.setupExtensionEventHandlers(); + + // Re-sync tab state from extension after reconnect (if Playwright is still alive) + if (this.playwrightWs) { + void this.resyncTabsFromExtension(); + } + + this.extensionConnectedResolve?.(); + } + + /** Wire up event handlers for the current extension connection. */ + private setupExtensionEventHandlers(): void { + if (!this.extensionConn) return; + this.extensionConn.onmessage = ( method: M, params: ExtensionEvents[M]['params'], @@ -547,6 +679,54 @@ export class CDPRelayServer { // Use the CDP targetId as Playwright's sessionId const sessionId = eventParams.id ?? this.primaryTabId; + // --- Restricted target filtering (second line of defense after extension) --- + if (eventParams.method === 'Target.attachedToTarget') { + const attachParams = eventParams.params as + | { + sessionId?: string; + targetInfo?: { type?: string; url?: string }; + waitingForDebugger?: boolean; + } + | undefined; + if (attachParams?.targetInfo && isRestrictedTarget(attachParams.targetInfo)) { + log.debug('filtering restricted target:', attachParams.targetInfo.url); + // Resume if waiting for debugger to prevent hanging navigations + if (attachParams.waitingForDebugger && attachParams.sessionId && this.extensionConn) { + this.extensionConn + .send('forwardCDPCommand', { + id: sessionId, + method: 'Runtime.runIfWaitingForDebugger', + params: {}, + }) + .catch((e) => log.debug('failed to resume restricted target:', e)); + } + return; + } + } + + // --- Target crash cleanup --- + if (eventParams.method === 'Target.targetCrashed') { + const crashParams = eventParams.params as { targetId?: string } | undefined; + const targetId = crashParams?.targetId; + if (targetId && this.tabCache.has(targetId)) { + log.debug('Target.targetCrashed: removing crashed tab:', targetId); + this.tabCache.delete(targetId); + const wasActivated = this.activatedTabs.delete(targetId); + if (targetId === this.primaryTabId) { + const remaining = [...this.tabCache.keys()]; + this.primaryTabId = remaining.length > 0 ? remaining[0] : undefined; + } + if (wasActivated) { + this.sendToPlaywright({ + method: 'Target.targetCrashed', + params: { targetId }, + sessionId: targetId, + }); + } + } + return; + } + // Keep cached metadata fresh on navigation if (eventParams.method === 'Page.frameNavigated' && sessionId) { const frame = (eventParams.params as Record | undefined)?.frame as @@ -566,6 +746,13 @@ export class CDPRelayServer { method: eventParams.method, params: eventParams.params, }); + + // Emit internally so Runtime.enable can wait for executionContextCreated + this.cdpEvents.emit('cdp:event', { + method: eventParams.method, + params: eventParams.params, + sessionId, + }); } else if (method === 'tabOpened') { const p = params as ExtensionEvents['tabOpened']['params']; this.handleTabOpened(p.id, p.title, p.url); @@ -574,8 +761,40 @@ export class CDPRelayServer { this.handleTabClosed(p.id); } }; + } - this.extensionConnectedResolve?.(); + /** After reconnect, sync tab state from extension and clean up stale entries. */ + private async resyncTabsFromExtension(): Promise { + if (!this.extensionConn) return; + try { + const { tabs } = (await this.extensionConn.send('listRegisteredTabs', {})) as { + tabs: Array<{ id: string; title: string; url: string }>; + }; + const currentIds = new Set(tabs.map((t) => t.id)); + + // Update cache for tabs that still exist + for (const tab of tabs) { + this.tabCache.set(tab.id, { title: tab.title, url: tab.url }); + this.primaryTabId ??= tab.id; + } + + // Remove stale entries for tabs that no longer exist in extension + for (const id of [...this.tabCache.keys()]) { + if (!currentIds.has(id)) { + this.tabCache.delete(id); + if (this.activatedTabs.delete(id)) { + this.sendToPlaywright({ + method: 'Target.detachedFromTarget', + params: { sessionId: id }, + }); + } + } + } + + log.debug('resyncTabsFromExtension: synced', tabs.length, 'tabs'); + } catch (e) { + log.error('resyncTabsFromExtension failed:', e); + } } private closeExtensionConnection(reason: string): void { @@ -612,6 +831,9 @@ export class CDPRelayServer { // ExtensionConnection — wraps the WebSocket to the extension // --------------------------------------------------------------------------- +const HEARTBEAT_INTERVAL_MS = 5_000; +const HEARTBEAT_TIMEOUT_MS = 15_000; + class ExtensionConnection { private readonly ws: WebSocket; private readonly callbacks = new Map< @@ -619,6 +841,11 @@ class ExtensionConnection { { resolve: (value: unknown) => void; reject: (reason: Error) => void } >(); private lastId = 0; + private lastPongAt = Date.now(); + private heartbeatInterval: ReturnType | undefined; + + /** The reason string from the WebSocket close frame, if any. */ + closeReason: string | undefined; onmessage?: ( method: M, @@ -629,14 +856,22 @@ class ExtensionConnection { constructor(ws: WebSocket) { this.ws = ws; this.ws.on('message', (data) => this.handleMessage(data)); - this.ws.on('close', () => { - log.debug('ExtensionConnection WebSocket closed'); + this.ws.on('close', (_code: number, reason: Buffer) => { + const reasonStr = reason.toString(); + log.debug('ExtensionConnection WebSocket closed:', reasonStr || '(no reason)'); + // Only update closeReason if not already set (e.g. by heartbeat timeout) + this.closeReason ??= reasonStr || undefined; this.handleClose(); }); this.ws.on('error', (error) => { log.debug('ExtensionConnection WebSocket error:', error); this.handleClose(); }); + this.ws.on('pong', () => { + this.lastPongAt = Date.now(); + }); + + this.startHeartbeat(); } async send( @@ -645,13 +880,17 @@ class ExtensionConnection { timeoutMs = 30_000, ): Promise { if (this.ws.readyState !== WebSocket.OPEN) { - throw new Error(`WebSocket not open (state=${this.ws.readyState})`); + throw new ConnectionLostError('network_error'); } const id = ++this.lastId; const payload = JSON.stringify({ id, method, params }); log.debug('→ EXT:', method, 'id=' + String(id)); - this.ws.send(payload); + try { + this.ws.send(payload); + } catch { + throw new ConnectionLostError('network_error'); + } return await new Promise((resolve, reject) => { const timer = setTimeout(() => { @@ -666,11 +905,7 @@ class ExtensionConnection { 'pending:', this.callbacks.size, ); - reject( - new Error( - `Extension command '${String(method)}' (id=${id}) timed out after ${timeoutMs}ms`, - ), - ); + reject(new ConnectionLostError('heartbeat_timeout')); }, timeoutMs); this.callbacks.set(id, { @@ -687,11 +922,39 @@ class ExtensionConnection { } close(reason: string): void { + this.stopHeartbeat(); if (this.ws.readyState === WebSocket.OPEN) { this.ws.close(1000, reason); } } + private startHeartbeat(): void { + this.heartbeatInterval = setInterval(() => { + if (this.ws.readyState !== WebSocket.OPEN) { + this.stopHeartbeat(); + return; + } + + const elapsed = Date.now() - this.lastPongAt; + if (elapsed > HEARTBEAT_TIMEOUT_MS) { + log.debug('heartbeat timeout: no pong for', elapsed, 'ms'); + this.closeReason = 'heartbeat_timeout'; + this.stopHeartbeat(); + this.ws.terminate(); + return; + } + + this.ws.ping(); + }, HEARTBEAT_INTERVAL_MS); + } + + private stopHeartbeat(): void { + if (this.heartbeatInterval) { + clearInterval(this.heartbeatInterval); + this.heartbeatInterval = undefined; + } + } + private handleMessage(data: unknown): void { let parsed: ExtensionResponse; try { @@ -722,14 +985,34 @@ class ExtensionConnection { } private handleClose(): void { + this.stopHeartbeat(); const pendingCount = this.callbacks.size; if (pendingCount > 0) { log.debug('ExtensionConnection closed with', pendingCount, 'pending callbacks'); } + const reason = asConnectionLostReason(this.closeReason); for (const pending of this.callbacks.values()) { - pending.reject(new Error('WebSocket closed')); + pending.reject(new ConnectionLostError(reason)); } this.callbacks.clear(); this.onclose?.(); } } + +// --------------------------------------------------------------------------- +// Close reason validation +// --------------------------------------------------------------------------- + +const VALID_REASONS = new Set([ + 'browser_closed', + 'extension_disconnected', + 'debugger_detached', + 'network_error', + 'heartbeat_timeout', +]); + +/** Validate a raw close reason string as a typed `ConnectionLostReason`. */ +function asConnectionLostReason(raw: string | undefined): ConnectionLostReason { + if (raw && VALID_REASONS.has(raw as ConnectionLostReason)) return raw as ConnectionLostReason; + return 'network_error'; +} diff --git a/packages/@n8n/mcp-browser/src/connection.ts b/packages/@n8n/mcp-browser/src/connection.ts index a7c87de17e3..4529dbd9f3a 100644 --- a/packages/@n8n/mcp-browser/src/connection.ts +++ b/packages/@n8n/mcp-browser/src/connection.ts @@ -1,5 +1,12 @@ import { getDefaultDiscovery, getInstallInstructions } from './browser-discovery'; -import { AlreadyConnectedError, BrowserNotAvailableError, NotConnectedError } from './errors'; +import { + AlreadyConnectedError, + BrowserNotAvailableError, + ConnectionLostError, + NotConnectedError, + type ConnectionLostReason, +} from './errors'; +import { createLogger } from './logger'; import type { BrowserName, Config, @@ -11,8 +18,11 @@ import type { } from './types'; import { configSchema } from './types'; +const log = createLogger('connection'); + export class BrowserConnection { private state: ConnectionState | null = null; + private disconnectReason: ConnectionLostReason | undefined; private readonly config: ResolvedConfig; constructor(userConfig?: Partial) { @@ -68,6 +78,15 @@ export class BrowserConnection { }; const adapter = await this.createAdapter(); + + // Listen for unexpected disconnections so we can invalidate state immediately + adapter.onDisconnect = (reason) => { + if (!this.state) return; // already disconnected + log.debug('unexpected disconnect, reason:', reason); + this.disconnectReason = reason; + this.state = null; + }; + await adapter.launch(connectConfig); // Two-tier model: listTabs() returns metadata from the relay (no @@ -90,6 +109,7 @@ export class BrowserConnection { const { adapter } = this.state; this.state = null; + this.disconnectReason = undefined; try { await adapter.close(); @@ -99,7 +119,12 @@ export class BrowserConnection { } getConnection(): ConnectionState { - if (!this.state) throw new NotConnectedError(); + if (!this.state) { + if (this.disconnectReason) { + throw new ConnectionLostError(this.disconnectReason); + } + throw new NotConnectedError(); + } return this.state; } diff --git a/packages/@n8n/mcp-browser/src/errors.ts b/packages/@n8n/mcp-browser/src/errors.ts index 140d4760b63..e018fe28271 100644 --- a/packages/@n8n/mcp-browser/src/errors.ts +++ b/packages/@n8n/mcp-browser/src/errors.ts @@ -69,6 +69,30 @@ export class BrowserNotAvailableError extends McpBrowserError { } } +export type ConnectionLostReason = + | 'browser_closed' + | 'extension_disconnected' + | 'debugger_detached' + | 'network_error' + | 'heartbeat_timeout'; + +const connectionLostMessages: Record = { + browser_closed: 'The browser was closed', + extension_disconnected: 'The browser extension disconnected', + debugger_detached: 'The Chrome debugger was detached (banner dismissed or DevTools closed)', + network_error: 'The connection to the browser extension was lost', + heartbeat_timeout: 'The browser extension stopped responding', +}; + +export class ConnectionLostError extends McpBrowserError { + constructor(readonly reason: ConnectionLostReason) { + super( + `Browser connection lost: ${connectionLostMessages[reason]}`, + 'Call browser_connect to reconnect.', + ); + } +} + export class BrowserExecutableNotFoundError extends McpBrowserError { constructor(readonly browser: string) { super( diff --git a/packages/@n8n/mcp-browser/src/tools/helpers.ts b/packages/@n8n/mcp-browser/src/tools/helpers.ts index 49f1c70908a..6302b626371 100644 --- a/packages/@n8n/mcp-browser/src/tools/helpers.ts +++ b/packages/@n8n/mcp-browser/src/tools/helpers.ts @@ -1,6 +1,7 @@ import type { z } from 'zod'; import type { BrowserConnection } from '../connection'; +import { ConnectionLostError } from '../errors'; import { createLogger } from '../logger'; import type { AffectedResource, @@ -112,6 +113,16 @@ export function createConnectedTool< return result; } catch (error) { + // Playwright throws TargetClosedError when browser/page dies mid-operation. + // Re-throw as our typed error so the AI gets a clear message + hint. + if (error instanceof Error && error.name === 'TargetClosedError') { + return await buildErrorResponse( + new ConnectionLostError('browser_closed'), + connection, + args, + options ?? {}, + ); + } return await buildErrorResponse(error, connection, args, options ?? {}); } },