From 32bc2e0ea1868b345b5f5cfcf767f64c1a650965 Mon Sep 17 00:00:00 2001 From: Bernhard Wittmann Date: Mon, 24 Aug 2026 06:33:59 +0000 Subject: [PATCH] feat: Remember allowed Browser Use hosts (no-changelog) (#36744) --- .../src/approvedHosts.test.ts | 105 +++++++ .../src/approvedHosts.ts | 40 +++ .../src/background.silentConnect.test.ts | 295 ++++++++++++++++++ .../src/background.test.ts | 42 ++- .../mcp-browser-extension/src/background.ts | 62 +++- .../src/relayAllowlist.test.ts | 22 +- .../src/relayAllowlist.ts | 42 +-- .../@n8n/mcp-browser-extension/src/types.ts | 6 + .../@n8n/mcp-browser-extension/src/ui/App.vue | 49 +-- .../mcp-browser-extension/src/ui/app.test.ts | 75 +++++ .../src/ui/components/RememberedHosts.vue | 79 +++++ .../src/ui/components/rememberedHosts.test.ts | 35 +++ .../src/ui/composables/useConnection.test.ts | 188 ++++++++++- .../src/ui/composables/useConnection.ts | 43 ++- .../mcp-browser-extension/src/ui/tokens.scss | 16 + .../mcp-browser-extension/vitest.config.mts | 8 +- .../frontend/@n8n/i18n/src/locales/en.json | 1 + .../useInstanceAiInputMenuItems.test.ts | 20 +- .../components/InstanceAiCredentialSetup.vue | 41 +-- .../modals/BrowserUseConnectStep.vue | 57 +++- .../modals/BrowserUseSetupContent.vue | 22 +- .../__tests__/BrowserUseConnectStep.test.ts | 77 ++++- .../__tests__/BrowserUseSetupModal.test.ts | 43 +-- .../__tests__/useBrowserUseConnection.test.ts | 220 +++++++++++++ .../composables/useBrowserUseConnection.ts | 127 ++++++++ .../useExtensionDirectConnect.test.ts | 45 ++- .../composables/useExtensionDirectConnect.ts | 71 ++++- .../useInstanceAiInputMenuItems.ts | 12 +- 28 files changed, 1636 insertions(+), 207 deletions(-) create mode 100644 packages/@n8n/mcp-browser-extension/src/approvedHosts.test.ts create mode 100644 packages/@n8n/mcp-browser-extension/src/approvedHosts.ts create mode 100644 packages/@n8n/mcp-browser-extension/src/background.silentConnect.test.ts create mode 100644 packages/@n8n/mcp-browser-extension/src/ui/app.test.ts create mode 100644 packages/@n8n/mcp-browser-extension/src/ui/components/RememberedHosts.vue create mode 100644 packages/@n8n/mcp-browser-extension/src/ui/components/rememberedHosts.test.ts create mode 100644 packages/frontend/editor-ui/src/features/ai/instanceAi/composables/__tests__/useBrowserUseConnection.test.ts create mode 100644 packages/frontend/editor-ui/src/features/ai/instanceAi/composables/useBrowserUseConnection.ts diff --git a/packages/@n8n/mcp-browser-extension/src/approvedHosts.test.ts b/packages/@n8n/mcp-browser-extension/src/approvedHosts.test.ts new file mode 100644 index 00000000000..23f13b4c983 --- /dev/null +++ b/packages/@n8n/mcp-browser-extension/src/approvedHosts.test.ts @@ -0,0 +1,105 @@ +import { + forgetApprovedHost, + isHostApproved, + listApprovedHosts, + rememberHost, +} from './approvedHosts'; + +const KEY = 'approvedRelayHosts'; + +const storage = { + get: vi.fn(), + set: vi.fn().mockResolvedValue(undefined), +}; + +Object.assign(globalThis, { chrome: { storage: { local: storage } } }); + +/** Seed what `chrome.storage.local.get` reports for the approved-hosts key. */ +function seed(value: unknown): void { + storage.get.mockResolvedValue({ [KEY]: value }); +} + +beforeEach(() => { + vi.clearAllMocks(); + seed(undefined); + storage.set.mockResolvedValue(undefined); +}); + +describe('isHostApproved', () => { + it('is true for a stored host', async () => { + seed(['acme.app.n8n.cloud']); + expect(await isHostApproved('wss://acme.app.n8n.cloud/browser-use/x?token=y')).toBe(true); + }); + + it('is false when nothing is stored', async () => { + expect(await isHostApproved('wss://acme.app.n8n.cloud/x')).toBe(false); + }); + + it('treats the same hostname on a different port as a different instance', async () => { + seed(['localhost:5678']); + expect(await isHostApproved('ws://localhost:5678/x')).toBe(true); + expect(await isHostApproved('ws://localhost:5679/x')).toBe(false); + }); + + it('is false for an unparseable URL', async () => { + seed(['acme.app.n8n.cloud']); + expect(await isHostApproved('not a url')).toBe(false); + expect(await isHostApproved(null)).toBe(false); + }); + + it('ignores a malformed stored value', async () => { + seed({ notAnArray: true }); + expect(await isHostApproved('wss://acme.app.n8n.cloud/x')).toBe(false); + }); + + it('ignores non-string entries', async () => { + seed([42, 'acme.app.n8n.cloud']); + expect(await isHostApproved('wss://acme.app.n8n.cloud/x')).toBe(true); + }); +}); + +describe('rememberHost', () => { + it('appends the host and port', async () => { + seed(['other.app.n8n.cloud']); + await rememberHost('ws://localhost:5678/x'); + expect(storage.set).toHaveBeenCalledWith({ + [KEY]: ['other.app.n8n.cloud', 'localhost:5678'], + }); + }); + + it('does not duplicate an already-stored host', async () => { + seed(['acme.app.n8n.cloud']); + await rememberHost('wss://acme.app.n8n.cloud/x'); + expect(storage.set).not.toHaveBeenCalled(); + }); + + it('is a no-op for an unparseable URL', async () => { + await rememberHost('not a url'); + expect(storage.set).not.toHaveBeenCalled(); + }); +}); + +describe('forgetApprovedHost', () => { + it('removes only the named host', async () => { + seed(['acme.app.n8n.cloud', 'localhost:5678']); + await forgetApprovedHost('localhost:5678'); + expect(storage.set).toHaveBeenCalledWith({ [KEY]: ['acme.app.n8n.cloud'] }); + }); + + it('is a no-op when the host was never stored', async () => { + seed(['acme.app.n8n.cloud']); + await forgetApprovedHost('localhost:5678'); + expect(storage.set).not.toHaveBeenCalled(); + }); +}); + +describe('listApprovedHosts', () => { + it('returns every stored host so they can be managed', async () => { + seed(['acme.app.n8n.cloud', 'localhost:5678']); + expect(await listApprovedHosts()).toEqual(['acme.app.n8n.cloud', 'localhost:5678']); + }); + + it('is empty when nothing was ever remembered', async () => { + expect(await listApprovedHosts()).toEqual([]); + }); +}); diff --git a/packages/@n8n/mcp-browser-extension/src/approvedHosts.ts b/packages/@n8n/mcp-browser-extension/src/approvedHosts.ts new file mode 100644 index 00000000000..1e4080f58da --- /dev/null +++ b/packages/@n8n/mcp-browser-extension/src/approvedHosts.ts @@ -0,0 +1,40 @@ +/** + * Hosts the user chose to keep approved, so repeat sessions skip the connect + * confirmation. `local`, not `session`, so the choice survives a browser restart. This + * never widens what may be connected to — `relayAllowlist` still gates every relay URL. + */ + +import { getRelayHostKey } from './relayAllowlist'; + +const APPROVED_HOSTS_KEY = 'approvedRelayHosts'; + +export async function listApprovedHosts(): Promise { + const stored = await chrome.storage.local.get(APPROVED_HOSTS_KEY); + const value: unknown = stored[APPROVED_HOSTS_KEY]; + if (!Array.isArray(value)) return []; + return value.filter((entry): entry is string => typeof entry === 'string'); +} + +export async function isHostApproved(relayUrl: string | null | undefined): Promise { + const host = getRelayHostKey(relayUrl); + if (!host) return false; + return (await listApprovedHosts()).includes(host); +} + +/** Returns the resulting list, so callers refresh their view without a second read. */ +export async function rememberHost(relayUrl: string | null | undefined): Promise { + const host = getRelayHostKey(relayUrl); + const hosts = await listApprovedHosts(); + if (!host || hosts.includes(host)) return hosts; + const next = [...hosts, host]; + await chrome.storage.local.set({ [APPROVED_HOSTS_KEY]: next }); + return next; +} + +export async function forgetApprovedHost(host: string): Promise { + const hosts = await listApprovedHosts(); + if (!hosts.includes(host)) return hosts; + const next = hosts.filter((entry) => entry !== host); + await chrome.storage.local.set({ [APPROVED_HOSTS_KEY]: next }); + return next; +} diff --git a/packages/@n8n/mcp-browser-extension/src/background.silentConnect.test.ts b/packages/@n8n/mcp-browser-extension/src/background.silentConnect.test.ts new file mode 100644 index 00000000000..eaae36a01f8 --- /dev/null +++ b/packages/@n8n/mcp-browser-extension/src/background.silentConnect.test.ts @@ -0,0 +1,295 @@ +// Separate from `background.test.ts` because it mocks `RelayConnection` to observe what +// the handshake is handed, which that file's cases must not see. + +type ExternalMessageHandler = ( + message: unknown, + sender: chrome.runtime.MessageSender, + sendResponse: (response: unknown) => void, +) => unknown; + +const ALLOWED_ORIGIN = 'https://acme.app.n8n.cloud'; +const RELAY_URL = 'wss://acme.app.n8n.cloud/browser-use/extension/abc?token=bu_x'; + +const { registerSelectedTabs } = vi.hoisted(() => ({ + registerSelectedTabs: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('./relayConnection', () => ({ + isEligibleTab: () => true, + // eslint-disable-next-line @typescript-eslint/naming-convention -- must match the real export + RelayConnection: class { + onclose: (() => void) | null = null; + + ontabcreated: (() => void) | null = null; + + registerSelectedTabs = registerSelectedTabs; + + close = vi.fn(); + + getControlledIds = () => []; + + isAgentCreatedTab = () => false; + + isControlledTab = () => false; + }, +})); + +type InternalMessageHandler = ( + message: unknown, + sender: chrome.runtime.MessageSender, + sendResponse: (response: unknown) => void, +) => unknown; + +const externalMessageListeners: ExternalMessageHandler[] = []; +const internalMessageListeners: InternalMessageHandler[] = []; + +const chromeMock = { + runtime: { + getURL: (path: string) => `chrome-extension://testextensionid/${path}`, + getManifest: () => ({ version: '0.0.6' }), + sendMessage: vi.fn().mockResolvedValue(undefined), + onMessage: { + addListener: vi.fn((fn: InternalMessageHandler) => internalMessageListeners.push(fn)), + }, + onMessageExternal: { + addListener: vi.fn((fn: ExternalMessageHandler) => externalMessageListeners.push(fn)), + }, + }, + tabs: { + query: vi.fn().mockResolvedValue([]), + get: vi.fn().mockResolvedValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + remove: vi.fn().mockResolvedValue(undefined), + onCreated: { addListener: vi.fn() }, + onRemoved: { addListener: vi.fn() }, + onUpdated: { addListener: vi.fn() }, + }, + windows: { + update: vi.fn().mockResolvedValue(undefined), + create: vi.fn().mockResolvedValue({ tabs: [{ id: 99 }] }), + getLastFocused: vi.fn().mockResolvedValue({ left: 0, top: 0, width: 1920, height: 1080 }), + }, + storage: { + session: { + set: vi.fn().mockResolvedValue(undefined), + get: vi.fn().mockResolvedValue({}), + remove: vi.fn().mockResolvedValue(undefined), + }, + local: { + get: vi.fn().mockResolvedValue({ approvedRelayHosts: ['acme.app.n8n.cloud'] }), + set: vi.fn().mockResolvedValue(undefined), + }, + }, + webNavigation: { onCreatedNavigationTarget: { addListener: vi.fn() } }, + action: { + setBadgeText: vi.fn(), + setBadgeBackgroundColor: vi.fn(), + setPopup: vi.fn(), + onClicked: { addListener: vi.fn() }, + }, +}; + +Object.assign(globalThis, { chrome: chromeMock }); + +/** Relay socket whose handshake the test completes by hand, to interleave two connects. */ +function stubDeferredRelaySocket(): { openAll: () => void } { + const opens: Array<() => void> = []; + vi.stubGlobal( + 'WebSocket', + class { + onopen: (() => void) | null = null; + + onerror: ((event: unknown) => void) | null = null; + + constructor(public url: string) { + opens.push(() => this.onopen?.()); + } + + close(): void {} + }, + ); + return { + openAll: () => { + // Reverse, so the connect that started first is the last to finish. + for (const open of [...opens].reverse()) open(); + }, + }; +} + +/** Relay socket double, so the handshake resolves without a network. */ +function stubRelaySocket(opens: boolean): void { + vi.stubGlobal( + 'WebSocket', + class { + onopen: (() => void) | null = null; + + onerror: ((event: unknown) => void) | null = null; + + constructor(public url: string) { + setTimeout(() => (opens ? this.onopen?.() : this.onerror?.({})), 0); + } + + close(): void {} + }, + ); +} + +const flush = async () => await new Promise((resolve) => setTimeout(resolve, 0)); + +/** Drive a message from an extension view, which is how the connect page asks to connect. */ +async function sendFromExtensionView(message: unknown): Promise { + for (const fn of internalMessageListeners) { + fn(message, {} as chrome.runtime.MessageSender, () => {}); + } + await flush(); +} + +/** Same as `sendFromExtensionView`, but hands back what the background answered. */ +async function sendFromExtensionViewWithResponse(message: unknown): Promise { + let response: unknown; + for (const fn of internalMessageListeners) { + fn(message, {} as chrome.runtime.MessageSender, (r) => (response = r)); + } + await flush(); + return response; +} + +/** Returns a getter, since a response can be held open until the flow settles. */ +async function sendFromPage( + type: 'connect' | 'connectResult', + origin = ALLOWED_ORIGIN, + relayUrl = RELAY_URL, +): Promise<() => unknown> { + const holder: { value?: unknown } = {}; + for (const fn of externalMessageListeners) { + fn({ type, relayUrl }, { origin } as chrome.runtime.MessageSender, (r) => (holder.value = r)); + } + await flush(); + return () => holder.value; +} + +beforeAll(async () => { + await import('./background'); +}); + +// The throttle keys off Date.now(); step past the window before each case. +let nowMs = 0; +beforeEach(() => { + nowMs += 60_000; + vi.spyOn(Date, 'now').mockImplementation(() => nowMs); + registerSelectedTabs.mockClear(); + chromeMock.windows.create.mockClear(); + chromeMock.action.setPopup.mockClear(); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('silent connect to an approved host', () => { + it('attaches no existing tabs, so a prompt-free reconnect cannot reach them', async () => { + stubRelaySocket(true); + + await sendFromPage('connect'); + await flush(); + + expect(registerSelectedTabs).toHaveBeenCalledWith([]); + expect(chromeMock.windows.create).not.toHaveBeenCalled(); + }); + + it('leaves the extension icon usable — there is no pending page to focus', async () => { + stubRelaySocket(true); + + await sendFromPage('connect'); + await flush(); + + expect(chromeMock.action.setPopup).not.toHaveBeenCalledWith({ popup: '' }); + }); + + it('reports the connection once the handshake lands', async () => { + stubRelaySocket(true); + + await sendFromPage('connect'); + await flush(); + + expect((await sendFromPage('connectResult'))()).toEqual({ connected: true }); + }); + + it('still confirms by hand when a different instance asks', async () => { + stubRelaySocket(true); + + // Another allowed origin cannot spend an approval granted to this relay's own page. + const response = await sendFromPage('connect', 'https://other.app.n8n.cloud'); + await flush(); + + expect(response()).toEqual({ accepted: true, confirmationRequired: true }); + expect(chromeMock.windows.create).toHaveBeenCalled(); + expect(registerSelectedTabs).not.toHaveBeenCalled(); + }); + + it("leaves another instance's pending flow alone when this one lands", async () => { + stubRelaySocket(true); + const otherOrigin = 'https://other.app.n8n.cloud'; + const otherRelay = 'wss://other.app.n8n.cloud/browser-use/extension/zzz?token=bu_y'; + + // An unapproved instance is mid-confirmation: its popup is open and its page is + // holding a `connectResult` open, waiting to hear how that went. + await sendFromPage('connect', otherOrigin, otherRelay); + const otherResult = await sendFromPage('connectResult', otherOrigin, otherRelay); + expect(otherResult()).toBeUndefined(); + + // Meanwhile a connect page approves a different relay, which reaches connectToRelay + // without going through the supersede at the top of the external handler. + await sendFromExtensionView({ type: 'connect', relayUrl: RELAY_URL, selectedTabIds: [] }); + await flush(); + + // Settling here would tell that page its connect failed while its popup is still up. + expect(otherResult()).toBeUndefined(); + }); + + it('does not let a slower earlier handshake replace the live session', async () => { + const socket = stubDeferredRelaySocket(); + + // Two connects overlap; the first one's socket is the last to open. + await sendFromExtensionView({ type: 'connect', relayUrl: RELAY_URL, selectedTabIds: [] }); + const newerRelay = 'wss://acme.app.n8n.cloud/browser-use/extension/newer?token=bu_z'; + await sendFromExtensionView({ type: 'connect', relayUrl: newerRelay, selectedTabIds: [] }); + + socket.openAll(); + await flush(); + + const status = await sendFromExtensionViewWithResponse({ type: 'getStatus' }); + expect(status).toMatchObject({ connected: true, relayUrl: newerRelay }); + }); + + it('honours a disconnect issued while the handshake is still opening', async () => { + const socket = stubDeferredRelaySocket(); + + await sendFromExtensionView({ type: 'connect', relayUrl: RELAY_URL, selectedTabIds: [] }); + await sendFromExtensionView({ type: 'disconnect' }); + + socket.openAll(); + await flush(); + + // The user asked for no connection; a slow handshake must not deliver one anyway. + expect(await sendFromExtensionViewWithResponse({ type: 'getStatus' })).toMatchObject({ + connected: false, + }); + }); + + it('reports failure promptly instead of leaving the page waiting', async () => { + stubRelaySocket(false); + + expect((await sendFromPage('connect'))()).toEqual({ + accepted: true, + confirmationRequired: false, + }); + + // Settled by the failed handshake rather than the page's own timeout. + const result = await sendFromPage('connectResult'); + await flush(); + expect(result()).toEqual({ connected: false }); + }); +}); + +export {}; diff --git a/packages/@n8n/mcp-browser-extension/src/background.test.ts b/packages/@n8n/mcp-browser-extension/src/background.test.ts index 0331557c7a5..c4530d934f1 100644 --- a/packages/@n8n/mcp-browser-extension/src/background.test.ts +++ b/packages/@n8n/mcp-browser-extension/src/background.test.ts @@ -102,6 +102,19 @@ async function simulateTabRemoved(tabId: number): Promise { await flush(); } +/** Stands in for the relay socket so no test opens a real connection. */ +class FailingWebSocket { + onopen: (() => void) | null = null; + + onerror: ((event: unknown) => void) | null = null; + + constructor(public url: string) { + setTimeout(() => this.onerror?.({}), 0); + } + + close(): void {} +} + /** Flush pending microtasks/macrotasks so the listener's async IIFE settles. */ const flush = async () => await new Promise((resolve) => setTimeout(resolve, 0)); @@ -117,6 +130,11 @@ beforeEach(() => { vi.clearAllMocks(); }); +// In an `afterEach` so a failed assertion can't leak the stub into later cases. +afterEach(() => { + vi.unstubAllGlobals(); +}); + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -202,6 +220,7 @@ describe('external messages (direct connect flow)', () => { nowMs += 60_000; vi.spyOn(Date, 'now').mockImplementation(() => nowMs); chromeMock.tabs.query.mockResolvedValue([]); + chromeMock.storage.local.get.mockResolvedValue({}); chromeMock.windows.create.mockResolvedValue({ tabs: [{ id: POPUP_TAB_ID }] }); chromeMock.windows.getLastFocused.mockResolvedValue({ left: 0, @@ -241,7 +260,7 @@ describe('external messages (direct connect flow)', () => { left: 690, top: 190, }); - expect(response()).toEqual({ accepted: true }); + expect(response()).toEqual({ accepted: true, confirmationRequired: true }); }); it('reuses an already-open connect page instead of opening a new popup', async () => { @@ -261,7 +280,24 @@ describe('external messages (direct connect flow)', () => { type: 'relayUrlReady', relayUrl: RELAY_URL, }); - expect(response()).toEqual({ accepted: true }); + expect(response()).toEqual({ accepted: true, confirmationRequired: true }); + }); + + it('skips the confirmation popup for a previously approved host', async () => { + chromeMock.storage.local.get.mockResolvedValue({ + approvedRelayHosts: ['acme.app.n8n.cloud'], + }); + vi.stubGlobal('WebSocket', FailingWebSocket); + + const response = await simulateExternalMessage( + { type: 'connect', relayUrl: RELAY_URL }, + ALLOWED_ORIGIN, + ); + + // No confirmation was shown, so the page must not tell the user to look for one. + expect(response()).toEqual({ accepted: true, confirmationRequired: false }); + expect(chromeMock.windows.create).not.toHaveBeenCalled(); + expect(chromeMock.tabs.update).not.toHaveBeenCalled(); }); it('rejects a relay URL that is not a recognized n8n instance', async () => { @@ -297,7 +333,7 @@ describe('external messages (direct connect flow)', () => { ALLOWED_ORIGIN, ); - expect(first()).toEqual({ accepted: true }); + expect(first()).toEqual({ accepted: true, confirmationRequired: true }); expect(second()).toEqual({ accepted: false }); expect(chromeMock.windows.create).toHaveBeenCalledTimes(1); }); diff --git a/packages/@n8n/mcp-browser-extension/src/background.ts b/packages/@n8n/mcp-browser-extension/src/background.ts index 8b7bcc1129b..b4e59b4985d 100644 --- a/packages/@n8n/mcp-browser-extension/src/background.ts +++ b/packages/@n8n/mcp-browser-extension/src/background.ts @@ -5,8 +5,9 @@ * and tracks tab lifecycle for agent-created tabs only. */ +import { isHostApproved } from './approvedHosts'; import { createLogger } from './logger'; -import { isAllowedPageOrigin, isAllowedRelayUrl } from './relayAllowlist'; +import { getRelayHostKey, isAllowedPageOrigin, isAllowedRelayUrl } from './relayAllowlist'; import { RelayConnection, isEligibleTab } from './relayConnection'; import type { ExtensionMessage, @@ -23,6 +24,9 @@ interface ConnectionState { } let activeConnection: ConnectionState | null = null; +// Bumped per connect request. A handshake is slow enough that a newer one can finish while +// an older is still opening, so the older must not commit itself over the live session. +let connectGeneration = 0; /** A query param rather than a header because `WebSocket` cannot set request headers. */ export function buildRelayWsUrl(relayUrl: string, version: string): string { @@ -244,10 +248,13 @@ chrome.runtime.onMessageExternal.addListener( log.debug('external message received:', message.type, 'from', sender.origin); if (message.type === 'connect') { - void handleExternalConnect(message.relayUrl).then(sendResponse, (error: unknown) => { - log.warn('external connect failed:', error); - sendResponse({ accepted: false }); - }); + void handleExternalConnect(message.relayUrl, sender.origin).then( + sendResponse, + (error: unknown) => { + log.warn('external connect failed:', error); + sendResponse({ accepted: false }); + }, + ); return true; } @@ -264,7 +271,10 @@ chrome.runtime.onMessageExternal.addListener( }, ); -async function handleExternalConnect(relayUrl: string): Promise { +async function handleExternalConnect( + relayUrl: string, + senderOrigin: string | undefined, +): Promise { if (!isAllowedRelayUrl(relayUrl)) { log.warn('refusing external connect to disallowed relay:', relayUrl); return { accepted: false }; @@ -279,11 +289,26 @@ async function handleExternalConnect(relayUrl: string): Promise { + // A newer request may already own the pending flow; only settle our own. + if (!result.success && pendingConnectFlow === flow) settleConnectFlow(false); + }); + return { accepted: true, confirmationRequired: false }; + } + const existing = await deliverRelayUrl(relayUrl); const tabId = existing?.id ?? (await openConnectPopup(relayUrl)); pendingConnectFlow = { relayUrl, tabId, notify: null }; setDrawerEnabled(false); - return { accepted: true }; + return { accepted: true, confirmationRequired: true }; } async function openConnectPopup(relayUrl: string): Promise { @@ -427,8 +452,10 @@ async function connectToRelay( return { success: false, error: 'Refusing to connect: not a recognized n8n instance.' }; } - // Clean up existing connection + // Clean up existing connection, then claim a generation — `disconnect` advances it, so + // taking ours first would make this attempt invalidate itself. disconnect(); + const generation = ++connectGeneration; try { const ws = new WebSocket(buildRelayWsUrl(relayUrl, chrome.runtime.getManifest().version)); @@ -461,16 +488,25 @@ async function connectToRelay( throw error; } + if (generation !== connectGeneration) { + log.debug('discarding superseded relay connection:', relayUrl); + relay.close('superseded'); + return { success: false, error: 'Superseded by a newer connection request.' }; + } + activeConnection = { relay, relayUrl }; relay.onclose = () => { log.debug('relay connection closed'); + // A superseded relay closing must not clear the session that replaced it. + if (activeConnection?.relay !== relay) return; activeConnection = null; updateBadge(0); broadcastStatusChange(); }; relay.ontabcreated = () => { + if (activeConnection?.relay !== relay) return; broadcastStatusChange(); updateBadge(relay.getControlledIds().length); }; @@ -479,7 +515,9 @@ async function connectToRelay( log.debug('connected, controlling', tabCount, 'tabs'); updateBadge(tabCount); broadcastStatusChange(); - settleConnectFlow(pendingConnectFlow?.relayUrl === relayUrl); + // Only our own flow: a newer request may already own the pending one, and settling it + // here would fail a page whose confirmation is still on screen. + if (pendingConnectFlow?.relayUrl === relayUrl) settleConnectFlow(true); return { success: true }; } catch (error) { log.error('connectToRelay failed:', error); @@ -491,6 +529,9 @@ async function connectToRelay( } function disconnect(): void { + // Outside the guard below: a handshake that has not committed yet leaves + // `activeConnection` null, and it must still be invalidated by a teardown. + connectGeneration++; if (activeConnection) { log.debug('disconnecting'); activeConnection.relay.close('extension_disconnected'); @@ -514,7 +555,8 @@ function broadcastStatusChange(): void { // --------------------------------------------------------------------------- function updateBadge(tabCount: number): void { - const text = tabCount > 0 ? String(tabCount) : ''; + // A prompt-free connect shows no UI at all, so mark the icon even before any tab attaches. + const text = tabCount > 0 ? String(tabCount) : activeConnection ? '•' : ''; void chrome.action.setBadgeText({ text }); void chrome.action.setBadgeBackgroundColor({ color: tabCount > 0 ? '#4CAF50' : '#999' }); } diff --git a/packages/@n8n/mcp-browser-extension/src/relayAllowlist.test.ts b/packages/@n8n/mcp-browser-extension/src/relayAllowlist.test.ts index dfb3d19ac95..6f13d5955d5 100644 --- a/packages/@n8n/mcp-browser-extension/src/relayAllowlist.test.ts +++ b/packages/@n8n/mcp-browser-extension/src/relayAllowlist.test.ts @@ -1,5 +1,5 @@ import { - getRelayHost, + getRelayHostKey, isAllowedPageOrigin, isAllowedRelayUrl, isLocalhostRelay, @@ -19,6 +19,8 @@ describe('isAllowedRelayUrl', () => { it('allows localhost relays for local development', () => { expect(isAllowedRelayUrl('ws://localhost:5680/browser-use/cdp/s')).toBe(true); + // Plaintext is loopback-only, so a stored approval can't be spent over one. + expect(isAllowedRelayUrl('ws://acme.app.n8n.cloud/x')).toBe(false); expect(isAllowedRelayUrl('ws://127.0.0.1:5680/x')).toBe(true); expect(isAllowedRelayUrl('ws://[::1]:5680/x')).toBe(true); }); @@ -85,14 +87,20 @@ describe('isLocalhostRelay', () => { }); }); -describe('getRelayHost', () => { - it('returns the hostname for a valid URL', () => { - expect(getRelayHost('wss://acme.app.n8n.cloud/x')).toBe('acme.app.n8n.cloud'); +describe('getRelayHostKey', () => { + it('keeps the port so two local instances stay distinct', () => { + expect(getRelayHostKey('ws://localhost:5678/x')).toBe('localhost:5678'); + expect(getRelayHostKey('ws://localhost:5679/x')).toBe('localhost:5679'); + }); + + it('omits the port when it is the default for the protocol', () => { + expect(getRelayHostKey('wss://acme.app.n8n.cloud/x')).toBe('acme.app.n8n.cloud'); + expect(getRelayHostKey('wss://acme.app.n8n.cloud:443/x')).toBe('acme.app.n8n.cloud'); }); it('returns null for malformed or empty input', () => { - expect(getRelayHost('not a url')).toBeNull(); - expect(getRelayHost(null)).toBeNull(); - expect(getRelayHost(undefined)).toBeNull(); + expect(getRelayHostKey('not a url')).toBeNull(); + expect(getRelayHostKey(null)).toBeNull(); + expect(getRelayHostKey(undefined)).toBeNull(); }); }); diff --git a/packages/@n8n/mcp-browser-extension/src/relayAllowlist.ts b/packages/@n8n/mcp-browser-extension/src/relayAllowlist.ts index a63b9ef580f..5ceef1d3d70 100644 --- a/packages/@n8n/mcp-browser-extension/src/relayAllowlist.ts +++ b/packages/@n8n/mcp-browser-extension/src/relayAllowlist.ts @@ -2,40 +2,44 @@ const N8N_CLOUD_SUFFIXES = ['.app.n8n.cloud', '.stage-app.n8n.cloud']; const LOCAL_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]']); -export function getRelayHost(url: string | null | undefined): string | null { +function parseRelayUrl(url: string | null | undefined): URL | null { if (!url) return null; try { - return new URL(url).hostname; + return new URL(url); } catch { return null; } } +function getHostname(url: string | null | undefined): string | null { + return parseRelayUrl(url)?.hostname ?? null; +} + +/** + * The identity of a relay everywhere it is stored or shown: hostname plus port, with the + * protocol's default port omitted. Two local instances stay distinct; cloud hosts read + * unchanged. + */ +export function getRelayHostKey(url: string | null | undefined): string | null { + return parseRelayUrl(url)?.host ?? null; +} + export function isLocalhostRelay(url: string | null | undefined): boolean { - const host = getRelayHost(url); + const host = getHostname(url); return host !== null && LOCAL_HOSTS.has(host); } export function isAllowedRelayUrl(url: string | null | undefined): boolean { - if (!url) return false; - let parsed: URL; - try { - parsed = new URL(url); - } catch { - return false; - } - if (parsed.protocol !== 'ws:' && parsed.protocol !== 'wss:') return false; - return isAllowedHost(parsed.hostname); + const parsed = parseRelayUrl(url); + if (!parsed) return false; + if (parsed.protocol === 'wss:') return isAllowedHost(parsed.hostname); + // Plaintext only where there is no network to intercept. + return parsed.protocol === 'ws:' && LOCAL_HOSTS.has(parsed.hostname); } export function isAllowedPageOrigin(origin: string | null | undefined): boolean { - if (!origin) return false; - let parsed: URL; - try { - parsed = new URL(origin); - } catch { - return false; - } + const parsed = parseRelayUrl(origin); + if (!parsed) return false; if (parsed.protocol === 'https:') return isAllowedHost(parsed.hostname); return parsed.protocol === 'http:' && LOCAL_HOSTS.has(parsed.hostname); } diff --git a/packages/@n8n/mcp-browser-extension/src/types.ts b/packages/@n8n/mcp-browser-extension/src/types.ts index acaeda8dfc0..2f4e169ae82 100644 --- a/packages/@n8n/mcp-browser-extension/src/types.ts +++ b/packages/@n8n/mcp-browser-extension/src/types.ts @@ -77,6 +77,12 @@ export type ExternalMessage = ExternalConnectMessage | ExternalConnectResultMess export interface ExternalConnectResponse { accepted: boolean; + /** + * False when the host was already allowed and no confirmation was shown, so the page can + * say "connecting" rather than point at a popup that never appears. Older extensions + * omit it — treat as true. + */ + confirmationRequired?: boolean; } export interface ExternalConnectResultResponse { diff --git a/packages/@n8n/mcp-browser-extension/src/ui/App.vue b/packages/@n8n/mcp-browser-extension/src/ui/App.vue index 07d5e004419..1c0c03d87b5 100644 --- a/packages/@n8n/mcp-browser-extension/src/ui/App.vue +++ b/packages/@n8n/mcp-browser-extension/src/ui/App.vue @@ -1,8 +1,9 @@ + + + + diff --git a/packages/@n8n/mcp-browser-extension/src/ui/components/rememberedHosts.test.ts b/packages/@n8n/mcp-browser-extension/src/ui/components/rememberedHosts.test.ts new file mode 100644 index 00000000000..ae4e98f8fb7 --- /dev/null +++ b/packages/@n8n/mcp-browser-extension/src/ui/components/rememberedHosts.test.ts @@ -0,0 +1,35 @@ +import { mount } from '@vue/test-utils'; + +import RememberedHosts from './RememberedHosts.vue'; + +function render(hosts: string[]) { + return mount(RememberedHosts, { props: { hosts } }); +} + +describe('RememberedHosts', () => { + it('renders nothing when no host has been allowed', () => { + expect(render([]).text()).toBe(''); + }); + + it('names every allowed host, so none is silently trusted', () => { + const wrapper = render(['acme.app.n8n.cloud', 'localhost:5678']); + + const names = wrapper.findAll('.host-name').map((el) => el.text()); + expect(names).toEqual(['acme.app.n8n.cloud', 'localhost:5678']); + }); + + it('revokes only the host whose control was used', async () => { + const wrapper = render(['acme.app.n8n.cloud', 'localhost:5678']); + + await wrapper.findAll('.host-remove')[1].trigger('click'); + + expect(wrapper.emitted('forget')).toEqual([['localhost:5678']]); + }); + + it('labels the control for pointer and screen reader alike', () => { + const button = render(['localhost:5678']).find('.host-remove'); + + expect(button.attributes('title')).toBe('Ask before connecting to localhost:5678'); + expect(button.attributes('aria-label')).toBe('Ask before connecting to localhost:5678'); + }); +}); diff --git a/packages/@n8n/mcp-browser-extension/src/ui/composables/useConnection.test.ts b/packages/@n8n/mcp-browser-extension/src/ui/composables/useConnection.test.ts index b654d8c6754..d103bf1bbec 100644 --- a/packages/@n8n/mcp-browser-extension/src/ui/composables/useConnection.test.ts +++ b/packages/@n8n/mcp-browser-extension/src/ui/composables/useConnection.test.ts @@ -62,6 +62,12 @@ const chromeMock = { windows: { getCurrent: vi.fn(), }, + storage: { + local: { + get: vi.fn(), + set: vi.fn(), + }, + }, }; Object.assign(globalThis, { chrome: chromeMock }); @@ -142,6 +148,8 @@ beforeEach(() => { chromeMock.tabs.get.mockImplementation(async (id: number) => await Promise.resolve(makeTab(id))); chromeMock.windows.getCurrent.mockResolvedValue({ type: 'normal' } as chrome.windows.Window); + chromeMock.storage.local.get.mockResolvedValue({}); + chromeMock.storage.local.set.mockResolvedValue(undefined); }); // --------------------------------------------------------------------------- @@ -197,7 +205,7 @@ describe('useConnection', () => { }); describe('connected instance', () => { - const RELAY_URL = 'wss://acme.app.n8n.cloud/relay'; + const RELAY_URL = 'wss://acme.app.n8n.cloud:8443/relay'; it('names the instance reported by the background on mount', async () => { chromeMock.runtime.sendMessage.mockImplementation(async (msg: { type: string }) => { @@ -210,7 +218,7 @@ describe('useConnection', () => { const { wrapper, result } = mountComposable(); await flush(); - expect(result().relayHost.value).toBe('acme.app.n8n.cloud'); + expect(result().relayHostKey.value).toBe('acme.app.n8n.cloud:8443'); wrapper.unmount(); }); @@ -218,12 +226,12 @@ describe('useConnection', () => { it('names the instance when a connection starts while the view is open', async () => { const { wrapper, result } = mountComposable(); await flush(); - expect(result().relayHost.value).toBeNull(); + expect(result().relayHostKey.value).toBeNull(); pushMessage({ type: 'statusChanged', connected: true, tabIds: [], relayUrl: RELAY_URL }); await flush(); - expect(result().relayHost.value).toBe('acme.app.n8n.cloud'); + expect(result().relayHostKey.value).toBe('acme.app.n8n.cloud:8443'); wrapper.unmount(); }); @@ -237,7 +245,7 @@ describe('useConnection', () => { await result().disconnect(); await flush(); - expect(result().relayHost.value).toBeNull(); + expect(result().relayHostKey.value).toBeNull(); wrapper.unmount(); }); @@ -516,6 +524,176 @@ describe('useConnection', () => { }); }); + describe('remembered instances', () => { + const APPROVED_KEY = 'approvedRelayHosts'; + const RELAY_URL = 'ws://localhost:1234/ext'; + + /** Background responses for a connect that succeeds against RELAY_URL. */ + function stubSuccessfulConnect(): void { + chromeMock.runtime.sendMessage.mockImplementation(async (msg: { type: string }) => { + if (msg.type === 'getRelayUrl') return RELAY_URL; + if (msg.type === 'getStatus') return { connected: false, tabIds: [] }; + if (msg.type === 'getTabs') return [makeTab(1)]; + if (msg.type === 'connect') return { success: true }; + if (msg.type === 'clearRelayUrl') return { success: true }; + return await Promise.resolve({}); + }); + } + + it('stores the host and port when the user opts in', async () => { + stubSuccessfulConnect(); + + const { wrapper, result } = mountComposable(); + await flush(); + + result().rememberInstance.value = true; + await result().connect(); + await flush(); + + expect(chromeMock.storage.local.set).toHaveBeenCalledWith({ + [APPROVED_KEY]: ['localhost:1234'], + }); + + wrapper.unmount(); + }); + + it('stores the host the user approved even if the live URL changes mid-handshake', async () => { + chromeMock.runtime.sendMessage.mockImplementation(async (msg: { type: string }) => { + if (msg.type === 'getRelayUrl') return RELAY_URL; + if (msg.type === 'getStatus') return { connected: false, tabIds: [] }; + if (msg.type === 'getTabs') return [makeTab(1)]; + if (msg.type === 'connect') { + // The session drops mid-handshake, which clears the live relay URL. + pushMessage({ type: 'statusChanged', connected: false }); + return { success: true }; + } + if (msg.type === 'clearRelayUrl') return { success: true }; + return await Promise.resolve({}); + }); + + const { wrapper, result } = mountComposable(); + await flush(); + + result().rememberInstance.value = true; + await result().connect(); + await flush(); + + expect(chromeMock.storage.local.set).toHaveBeenCalledWith({ + [APPROVED_KEY]: ['localhost:1234'], + }); + + wrapper.unmount(); + }); + + it('clears the opt-in when a different instance takes over the page', async () => { + const { wrapper, result } = mountComposable(); + await flush(); + + result().rememberInstance.value = true; + pushMessage({ type: 'relayUrlReady', relayUrl: 'ws://other.host:9999/ext' }); + await flush(); + + expect(result().rememberInstance.value).toBe(false); + + wrapper.unmount(); + }); + + it('stores nothing by default — auto-connect is opt-in', async () => { + stubSuccessfulConnect(); + + const { wrapper, result } = mountComposable(); + await flush(); + + expect(result().rememberInstance.value).toBe(false); + await result().connect(); + await flush(); + + expect(chromeMock.storage.local.set).not.toHaveBeenCalled(); + + wrapper.unmount(); + }); + + it('stores nothing when auto-connecting even if remember is somehow set', async () => { + window.history.replaceState( + {}, + '', + '/?autoConnect=1&mcpRelayUrl=' + encodeURIComponent(RELAY_URL), + ); + stubSuccessfulConnect(); + + const { wrapper, result } = mountComposable(); + await flush(); + + result().rememberInstance.value = true; + await result().connect(); + await flush(); + + expect(chromeMock.storage.local.set).not.toHaveBeenCalled(); + + wrapper.unmount(); + window.history.replaceState({}, '', '/'); + }); + + it('stores nothing when auto-connecting for the eval harness', async () => { + window.history.replaceState( + {}, + '', + '/?autoConnect=1&mcpRelayUrl=' + encodeURIComponent(RELAY_URL), + ); + stubSuccessfulConnect(); + + const { wrapper, result } = mountComposable(); + await flush(); + + expect(result().isAutoConnect.value).toBe(true); + expect(chromeMock.storage.local.set).not.toHaveBeenCalled(); + + wrapper.unmount(); + window.history.replaceState({}, '', '/'); + }); + + it('exposes every stored host so they can be managed while disconnected', async () => { + chromeMock.storage.local.get.mockResolvedValue({ + [APPROVED_KEY]: ['acme.app.n8n.cloud', 'localhost:1234'], + }); + + const { wrapper, result } = mountComposable(); + await flush(); + + expect(result().approvedHosts.value).toEqual(['acme.app.n8n.cloud', 'localhost:1234']); + + wrapper.unmount(); + }); + + it('drops a stored host without ending the session', async () => { + const stored = ['acme.app.n8n.cloud', 'localhost:1234']; + chromeMock.storage.local.get.mockImplementation( + async () => await Promise.resolve({ [APPROVED_KEY]: stored }), + ); + chromeMock.storage.local.set.mockImplementation(async (update: Record) => { + stored.splice(0, stored.length, ...update[APPROVED_KEY]); + return await Promise.resolve(undefined); + }); + chromeMock.runtime.sendMessage.mockImplementation(async (msg: { type: string }) => { + if (msg.type === 'getRelayUrl') return null; + if (msg.type === 'getStatus') return { connected: true, tabIds: [], relayUrl: RELAY_URL }; + if (msg.type === 'getTabs') return [makeTab(1)]; + return await Promise.resolve({}); + }); + + const { wrapper, result } = mountComposable(); + await flush(); + + await result().forgetHost('localhost:1234'); + + expect(result().approvedHosts.value).toEqual(['acme.app.n8n.cloud']); + expect(result().status.value).toBe('connected'); + expect(chromeMock.runtime.sendMessage).not.toHaveBeenCalledWith({ type: 'disconnect' }); + + wrapper.unmount(); + }); + }); + describe('disconnect', () => { it('clears status and controlled tabs', async () => { const { wrapper, result } = mountComposable(); diff --git a/packages/@n8n/mcp-browser-extension/src/ui/composables/useConnection.ts b/packages/@n8n/mcp-browser-extension/src/ui/composables/useConnection.ts index f5ce1eb9c65..2bbebace72b 100644 --- a/packages/@n8n/mcp-browser-extension/src/ui/composables/useConnection.ts +++ b/packages/@n8n/mcp-browser-extension/src/ui/composables/useConnection.ts @@ -1,7 +1,8 @@ import { ref, computed, reactive, onMounted, onUnmounted } from 'vue'; +import { forgetApprovedHost, listApprovedHosts, rememberHost } from '../../approvedHosts'; import { createLogger } from '../../logger'; -import { getRelayHost, isAllowedRelayUrl, isLocalhostRelay } from '../../relayAllowlist'; +import { getRelayHostKey, isAllowedRelayUrl, isLocalhostRelay } from '../../relayAllowlist'; import { isEligibleTab } from '../../relayConnection'; import type { ConnectionStatus, @@ -34,6 +35,13 @@ export function useConnection() { // attacker-controlled WSS endpoint. const isAutoConnect = ref(false); + // ── Remembered-instance consent ────────────────────────────────────────── + // Opt-in: a remembered instance reconnects with no prompt at all, so nobody should end + // up in that state without ticking the box themselves. + const rememberInstance = ref(false); + // Every stored host, so the drawer can revoke them without being connected. + const approvedHosts = ref([]); + // ── Single source of truth: reactive tab registry ───────────────────────── // Maps chromeTabId → tab object. Kept in sync by Chrome tab event listeners. const tabRegistry = reactive(new Map()); @@ -55,8 +63,10 @@ export function useConnection() { // ── Computeds ───────────────────────────────────────────────────────────── const hasRelayUrl = computed(() => !!relayUrl.value); - const relayHost = computed(() => getRelayHost(connectedRelayUrl.value ?? relayUrl.value)); const isRelayAllowed = computed(() => isAllowedRelayUrl(relayUrl.value)); + // The one identity the user ever sees, and the one that gets stored, so what they agree + // to always matches what the revoke list shows back. + const relayHostKey = computed(() => getRelayHostKey(connectedRelayUrl.value ?? relayUrl.value)); // ── Private helpers ─────────────────────────────────────────────────────── @@ -114,24 +124,31 @@ export function useConnection() { } if (!isAllowedRelayUrl(relayUrl.value)) { - errorMessage.value = `Can't connect to ${relayHost.value ?? 'this address'} — not a recognized n8n instance.`; + errorMessage.value = `Can't connect to ${relayHostKey.value ?? 'this address'} — not a recognized n8n instance.`; log.warn('connect: relay URL not allowed', relayUrl.value); return; } - log.debug('connect: relay URL =', relayUrl.value, 'selectedTabs:', selectedTabIds.size); + // Pinned before any await: a new request can replace `relayUrl` mid-handshake, and the + // approval must record the host the user was actually shown. + const approvedUrl = relayUrl.value; + log.debug('connect: relay URL =', approvedUrl, 'selectedTabs:', selectedTabIds.size); status.value = 'connecting'; errorMessage.value = ''; const raw: unknown = await chrome.runtime.sendMessage({ type: 'connect', - relayUrl: relayUrl.value, + relayUrl: approvedUrl, selectedTabIds: [...selectedTabIds], }); log.debug('connect response:', raw); if (isConnectResponse(raw) && raw.success) { status.value = 'connected'; + // The eval harness connects unattended — it must not write user-facing trust state. + if (rememberInstance.value && !isAutoConnect.value) { + approvedHosts.value = await rememberHost(approvedUrl); + } await chrome.runtime.sendMessage({ type: 'clearRelayUrl' }); // Fetch controlled IDs — controlledTabDetails computed auto-resolves from registry const statusResponse: unknown = await chrome.runtime.sendMessage({ type: 'getStatus' }); @@ -158,6 +175,12 @@ export function useConnection() { relayUrl.value = null; } + /** Drops a stored approval. Never touches the live session. */ + async function forgetHost(host: string): Promise { + log.debug('forgetHost', host); + approvedHosts.value = await forgetApprovedHost(host); + } + async function decline(): Promise { log.debug('decline'); await chrome.runtime.sendMessage({ type: 'clearRelayUrl' }); @@ -185,6 +208,8 @@ export function useConnection() { if (message.type === 'relayUrlReady' && message.relayUrl) { log.debug('relayUrlReady received:', message.relayUrl); relayUrl.value = message.relayUrl; + // A different instance is asking now, so its approval has to be given afresh. + rememberInstance.value = false; // Drop the now-stale connection params from the page URL. The live value lives in // relayUrl + session storage, so a manual reload reads the fresh URL, not the old token. window.history.replaceState(null, '', window.location.pathname); @@ -253,7 +278,8 @@ export function useConnection() { applyStatus(currentStatus); } - await initTabRegistry(); + const [storedHosts] = await Promise.all([listApprovedHosts(), initTabRegistry()]); + approvedHosts.value = storedHosts; if (!mounted) return; @@ -282,13 +308,16 @@ export function useConnection() { errorMessage, relayUrl, hasRelayUrl, - relayHost, isRelayAllowed, isAutoConnect, + relayHostKey, + rememberInstance, + approvedHosts, controlledTabs: controlledTabDetails, toggleTab, connect, decline, disconnect, + forgetHost, }; } diff --git a/packages/@n8n/mcp-browser-extension/src/ui/tokens.scss b/packages/@n8n/mcp-browser-extension/src/ui/tokens.scss index 28365d2708c..3ca56f16386 100644 --- a/packages/@n8n/mcp-browser-extension/src/ui/tokens.scss +++ b/packages/@n8n/mcp-browser-extension/src/ui/tokens.scss @@ -37,3 +37,19 @@ body { background-color: var(--color--background); color: var(--color--text--shade-1); } + +// Grouping box shared by the views and the allowed-instances list. +.panel { + display: flex; + flex-direction: column; + gap: var(--spacing--lg); + background: var(--background--surface); + border: var(--border-width) var(--border-style) var(--color--foreground--tint-1); + border-radius: var(--radius--lg); + padding: var(--spacing--lg); + min-height: 0; +} + +.panel + .panel { + margin-top: var(--spacing--md); +} diff --git a/packages/@n8n/mcp-browser-extension/vitest.config.mts b/packages/@n8n/mcp-browser-extension/vitest.config.mts index 05a3312dde7..1132e1dcdc9 100644 --- a/packages/@n8n/mcp-browser-extension/vitest.config.mts +++ b/packages/@n8n/mcp-browser-extension/vitest.config.mts @@ -1,3 +1,9 @@ +import vue from '@vitejs/plugin-vue'; +import { defineConfig, mergeConfig } from 'vitest/config'; import { createVitestConfig } from '@n8n/vitest-config/frontend'; -export default createVitestConfig({ setupFiles: [] }); +// The UI is Vue SFCs, so the plugin has to be present for `.vue` imports to parse. +export default mergeConfig( + defineConfig({ plugins: [vue()] }), + createVitestConfig({ setupFiles: [] }), +); diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index bcc28d161fc..579e37092e7 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -6860,6 +6860,7 @@ "instanceAi.browserUse.step.connect.cta": "Open Browser Use extension", "instanceAi.browserUse.step.connect.extensionMissing": "We can't detect the extension. Install it above, or check that it's enabled in your browser.", "instanceAi.browserUse.directConnect.waiting": "Confirm the connection in the extension popup…", + "instanceAi.browserUse.directConnect.connecting": "Connecting to your browser…", "instanceAi.browserUse.directConnect.failed": "Problems connecting your browser?", "instanceAi.browserUse.directConnect.retry": "Try again", "instanceAi.browserUse.unsupportedBrowser": "Browser Use requires Google Chrome or another Chromium-based browser like Microsoft Edge or Brave. Open n8n in a supported browser to continue.", diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/useInstanceAiInputMenuItems.test.ts b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/useInstanceAiInputMenuItems.test.ts index 002f705e1b9..3d1a031fa43 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/useInstanceAiInputMenuItems.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/useInstanceAiInputMenuItems.test.ts @@ -5,13 +5,13 @@ import { useInstanceAiInputMenuItems, } from '../composables/useInstanceAiInputMenuItems'; import { - INSTANCE_AI_BROWSER_USE_SETUP_MODAL_KEY, INSTANCE_AI_COMPUTER_USE_SETUP_MODAL_KEY, INSTANCE_AI_TOOLS_CONNECTION_MODAL_KEY, } from '../constants'; const { browserUseTelemetry, + ensureBrowserConnected, computerUseTelemetry, featureFlags, mcpStore, @@ -20,6 +20,7 @@ const { uiStore, } = vi.hoisted(() => ({ browserUseTelemetry: { trackModalOpened: vi.fn() }, + ensureBrowserConnected: vi.fn(), computerUseTelemetry: { trackModalOpened: vi.fn() }, featureFlags: { browserUse: true, computerUse: true, mcp: true }, mcpStore: { @@ -70,6 +71,9 @@ vi.mock('@/experiments/instanceAiMcpConnections', () => ({ }), })); +vi.mock('../composables/useBrowserUseConnection', () => ({ + useBrowserUseConnection: () => ({ ensureConnected: ensureBrowserConnected }), +})); vi.mock('@/experiments/instanceAiBrowserUse', () => ({ useInstanceAiBrowserUseExperiment: () => ({ isFeatureEnabled: { @@ -200,7 +204,9 @@ describe('useInstanceAiInputMenuItems', () => { setDisconnected: () => { settingsStore.browserUseConnectionStatus = 'disconnected'; }, - modal: INSTANCE_AI_BROWSER_USE_SETUP_MODAL_KEY, + // Browser Use reconnects through the shared flow, which opens a modal only when + // the extension actually needs the user. + modal: null, title: 'instanceAi.inputMenu.browser.connectedTitle', }, ])( @@ -216,7 +222,12 @@ describe('useInstanceAiInputMenuItems', () => { expect(disconnectedConnectionCount.value).toBe(1); await findItem(menuItems.value, `${id}-reconnect`)?.data?.action?.(); - expect(uiStore.openModal).toHaveBeenCalledWith(modal); + if (modal) { + expect(uiStore.openModal).toHaveBeenCalledWith(modal); + } else { + expect(ensureBrowserConnected).toHaveBeenCalledWith('input_menu'); + expect(uiStore.openModal).not.toHaveBeenCalled(); + } }, ); @@ -282,7 +293,6 @@ describe('useInstanceAiInputMenuItems', () => { }); expect(mcpStore.disconnect).toHaveBeenCalledWith('1'); expect(settingsStore.disconnectComputerUse).toHaveBeenCalledOnce(); - expect(browserUseTelemetry.trackModalOpened).toHaveBeenCalledWith('input_menu'); - expect(uiStore.openModal).toHaveBeenCalledWith(INSTANCE_AI_BROWSER_USE_SETUP_MODAL_KEY); + expect(ensureBrowserConnected).toHaveBeenCalledWith('input_menu'); }); }); diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiCredentialSetup.vue b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiCredentialSetup.vue index dea467ae0d4..35e0639240c 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiCredentialSetup.vue +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiCredentialSetup.vue @@ -1,5 +1,4 @@ diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/modals/BrowserUseConnectStep.vue b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/modals/BrowserUseConnectStep.vue index 6c900ed5fa1..6769af7a628 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/modals/BrowserUseConnectStep.vue +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/modals/BrowserUseConnectStep.vue @@ -1,10 +1,13 @@