Skip to content

Commit c02fc02

Browse files
committed
fix: connect-existing with remote/containerized Firefox
When using --connect-existing to connect to Firefox running on a different host (e.g., via SSH tunnel or in a separate container), several issues prevented it from working: 1. The --marionette-host CLI arg was parsed but never passed to the options object, so geckodriver always connected to 127.0.0.1. 2. selenium-manager was invoked with --browser firefox, causing it to download a 307MB Firefox binary that connect-existing never uses. Changed to --driver geckodriver to fetch only the driver. 3. No cleanup on disconnect: the Marionette session was never released when the MCP client disconnected, so Firefox refused new connections until restarted. Added SIGTERM/SIGINT/stdin handlers that call firefox.close() before exit. Raw stdin listeners are needed because StdioServerTransport does not fire onclose on stdin EOF. 4. kill() was synchronous and skipped DELETE /session, leaving stale sessions. Made it async and added session cleanup. 5. BiDi was unavailable in connect-existing mode. Added WebSocket support via the webSocketUrl session capability, with URL rewriting for remote hosts. BiDi subscription failure is non-fatal so Classic WebDriver continues working.
1 parent 66a8240 commit c02fc02

4 files changed

Lines changed: 131 additions & 18 deletions

File tree

src/cli.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,11 @@ export const cliOptions = {
110110
description: 'Marionette port to connect to when using --connect-existing (default: 2828)',
111111
default: Number(process.env.MARIONETTE_PORT ?? '2828'),
112112
},
113+
marionetteHost: {
114+
type: 'string',
115+
description: 'Marionette host to connect to when using --connect-existing (default: 127.0.0.1). Also used as the BiDi WebSocket connect address when different from 127.0.0.1.',
116+
default: process.env.MARIONETTE_HOST ?? '127.0.0.1',
117+
},
113118
env: {
114119
type: 'array',
115120
description:

src/firefox/core.ts

Lines changed: 102 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { spawn, type ChildProcess } from 'node:child_process';
88
import { mkdirSync, openSync, closeSync } from 'node:fs';
99
import { homedir } from 'node:os';
1010
import { join } from 'node:path';
11+
import WebSocket from 'ws';
1112
import type { FirefoxLaunchOptions } from './types.js';
1213
import { log, logDebug } from '../utils/logger.js';
1314

@@ -129,21 +130,26 @@ class GeckodriverHttpDriver implements IDriver {
129130
private baseUrl: string;
130131
private sessionId: string;
131132
private gdProcess: ChildProcess;
133+
private webSocketUrl: string | null;
134+
private bidiConnection: IBiDi | null = null;
132135

133-
constructor(baseUrl: string, sessionId: string, gdProcess: ChildProcess) {
136+
constructor(baseUrl: string, sessionId: string, gdProcess: ChildProcess, webSocketUrl: string | null) {
134137
this.baseUrl = baseUrl;
135138
this.sessionId = sessionId;
136139
this.gdProcess = gdProcess;
140+
this.webSocketUrl = webSocketUrl;
137141
}
138142

139-
static async connect(marionettePort: number): Promise<GeckodriverHttpDriver> {
143+
static async connect(marionettePort: number, marionetteHost = '127.0.0.1'): Promise<GeckodriverHttpDriver> {
140144
// Find geckodriver binary via selenium-manager
141145
const path = await import('node:path');
142146
const { execFileSync } = await import('node:child_process');
143147

144148
let geckodriverPath: string;
145149
try {
146-
// selenium-manager ships with selenium-webdriver and resolves/downloads geckodriver
150+
// selenium-manager ships with selenium-webdriver and resolves/downloads geckodriver.
151+
// Use --driver instead of --browser to skip downloading Firefox, which is
152+
// already running externally in connect-existing mode.
147153
const { createRequire } = await import('node:module');
148154
const require = createRequire(import.meta.url);
149155
const swPkg = require.resolve('selenium-webdriver/package.json');
@@ -157,7 +163,7 @@ class GeckodriverHttpDriver implements IDriver {
157163
const ext = process.platform === 'win32' ? '.exe' : '';
158164
const smBin = path.join(swDir, 'bin', platform, `selenium-manager${ext}`);
159165
const result = JSON.parse(
160-
execFileSync(smBin, ['--browser', 'firefox', '--output', 'json'], { encoding: 'utf-8' })
166+
execFileSync(smBin, ['--driver', 'geckodriver', '--output', 'json'], { encoding: 'utf-8' })
161167
);
162168
geckodriverPath = result.result.driver_path;
163169
} catch {
@@ -175,7 +181,7 @@ class GeckodriverHttpDriver implements IDriver {
175181
// Use --port=0 to let the OS assign a free port atomically (geckodriver ≥0.34.0)
176182
const gd = spawn(
177183
geckodriverPath,
178-
['--connect-existing', '--marionette-port', String(marionettePort), '--port', '0'],
184+
['--connect-existing', '--marionette-host', marionetteHost, '--marionette-port', String(marionettePort), '--port', '0'],
179185
{ stdio: ['ignore', 'pipe', 'pipe'] }
180186
);
181187

@@ -206,11 +212,11 @@ class GeckodriverHttpDriver implements IDriver {
206212

207213
const baseUrl = `http://127.0.0.1:${port}`;
208214

209-
// Create a WebDriver session
215+
// Create a WebDriver session with BiDi opt-in
210216
const resp = await fetch(`${baseUrl}/session`, {
211217
method: 'POST',
212218
headers: { 'Content-Type': 'application/json' },
213-
body: JSON.stringify({ capabilities: { alwaysMatch: {} } }),
219+
body: JSON.stringify({ capabilities: { alwaysMatch: { webSocketUrl: true } } }),
214220
});
215221
const json = (await resp.json()) as {
216222
value: { sessionId: string; capabilities: Record<string, unknown> };
@@ -219,7 +225,21 @@ class GeckodriverHttpDriver implements IDriver {
219225
throw new Error(`Failed to create session: ${JSON.stringify(json)}`);
220226
}
221227

222-
return new GeckodriverHttpDriver(baseUrl, json.value.sessionId, gd);
228+
let wsUrl = json.value.capabilities.webSocketUrl as string | undefined;
229+
logDebug(`Session capabilities webSocketUrl: ${wsUrl ?? 'not present'}, marionetteHost: ${marionetteHost}`);
230+
if (wsUrl && marionetteHost !== '127.0.0.1') {
231+
// Rewrite the URL to connect through the remote host / tunnel.
232+
const parsed = new URL(wsUrl);
233+
parsed.hostname = marionetteHost;
234+
wsUrl = parsed.toString();
235+
}
236+
if (wsUrl) {
237+
logDebug(`BiDi WebSocket URL: ${wsUrl}`);
238+
} else {
239+
logDebug('BiDi WebSocket URL not available (Firefox may not support it or Remote Agent is not running)');
240+
}
241+
242+
return new GeckodriverHttpDriver(baseUrl, json.value.sessionId, gd, wsUrl ?? null);
223243
}
224244

225245
private async cmd(method: string, path: string, body?: unknown): Promise<unknown> {
@@ -422,6 +442,10 @@ class GeckodriverHttpDriver implements IDriver {
422442
}
423443

424444
async quit(): Promise<void> {
445+
if (this.bidiConnection) {
446+
(this.bidiConnection.socket as unknown as WebSocket).close();
447+
this.bidiConnection = null;
448+
}
425449
try {
426450
await this.cmd('DELETE', '');
427451
} catch {
@@ -430,13 +454,75 @@ class GeckodriverHttpDriver implements IDriver {
430454
this.gdProcess.kill();
431455
}
432456

433-
/** Kill the geckodriver process without closing Firefox */
434-
kill(): void {
457+
/** Kill the geckodriver process without closing Firefox.
458+
* Deletes the session first so Marionette accepts new connections. */
459+
async kill(): Promise<void> {
460+
if (this.bidiConnection) {
461+
(this.bidiConnection.socket as unknown as WebSocket).close();
462+
this.bidiConnection = null;
463+
}
464+
try {
465+
await this.cmd('DELETE', '');
466+
} catch {
467+
// ignore
468+
}
435469
this.gdProcess.kill();
436470
}
437471

438-
getBidi(): Promise<IBiDi> {
439-
throw new Error('BiDi not available in connect-existing mode');
472+
/**
473+
* Return a BiDi handle. Opens a WebSocket to Firefox's Remote Agent on
474+
* first call, using the webSocketUrl returned in the session capabilities.
475+
*/
476+
async getBidi(): Promise<IBiDi> {
477+
if (this.bidiConnection) return this.bidiConnection;
478+
if (!this.webSocketUrl) {
479+
throw new Error(
480+
'BiDi is not available: no webSocketUrl in session capabilities. ' +
481+
'Ensure Firefox was started with --remote-debugging-port.'
482+
);
483+
}
484+
485+
const ws = new WebSocket(this.webSocketUrl);
486+
await new Promise<void>((resolve, reject) => {
487+
ws.on('open', resolve);
488+
ws.on('error', (e: any) => {
489+
const msg = e?.message || e?.error?.message || e?.error || e?.type || JSON.stringify(e) || String(e);
490+
reject(new Error(`BiDi WS to ${this.webSocketUrl}: ${msg}`));
491+
});
492+
});
493+
494+
let cmdId = 0;
495+
const subscribe = async (event: string, contexts?: string[]): Promise<void> => {
496+
const msg: Record<string, unknown> = {
497+
id: ++cmdId,
498+
method: 'session.subscribe',
499+
params: { events: [event] },
500+
};
501+
if (contexts) msg.params = { events: [event], contexts };
502+
ws.send(JSON.stringify(msg));
503+
await new Promise<void>((resolve, reject) => {
504+
const timeout = setTimeout(() => reject(new Error(`BiDi subscribe timeout for ${event}`)), 5000);
505+
const onMsg = (data: WebSocket.Data) => {
506+
try {
507+
const payload = JSON.parse(data.toString());
508+
if (payload.id === cmdId) {
509+
clearTimeout(timeout);
510+
ws.off('message', onMsg);
511+
if (payload.error) {
512+
reject(new Error(`BiDi subscribe error: ${payload.error}`));
513+
} else {
514+
resolve();
515+
}
516+
}
517+
} catch { /* ignore parse errors from event messages */ }
518+
};
519+
ws.on('message', onMsg);
520+
});
521+
logDebug(`BiDi subscribed to ${event}`);
522+
};
523+
524+
this.bidiConnection = { subscribe, socket: ws as unknown as IBiDiSocket } as any;
525+
return this.bidiConnection;
440526
}
441527
}
442528

@@ -503,7 +589,8 @@ export class FirefoxCore {
503589
// We bypass selenium-webdriver because its BiDi auto-upgrade hangs
504590
// when used with geckodriver's --connect-existing mode.
505591
const port = this.options.marionettePort ?? 2828;
506-
this.driver = await GeckodriverHttpDriver.connect(port);
592+
const host = this.options.marionetteHost ?? '127.0.0.1';
593+
this.driver = await GeckodriverHttpDriver.connect(port, host);
507594
} else {
508595
// Set up output file for capturing Firefox stdout/stderr
509596
if (this.options.logFile) {
@@ -640,7 +727,7 @@ export class FirefoxCore {
640727
*/
641728
reset(): void {
642729
if (this.driver && this.options.connectExisting && 'kill' in this.driver) {
643-
(this.driver as { kill(): void }).kill();
730+
(this.driver as { kill(): Promise<void> }).kill();
644731
}
645732
this.driver = null;
646733
this.currentContextId = null;
@@ -762,7 +849,7 @@ export class FirefoxCore {
762849
async close(): Promise<void> {
763850
if (this.driver) {
764851
if (this.options.connectExisting && 'kill' in this.driver) {
765-
(this.driver as { kill(): void }).kill();
852+
await (this.driver as { kill(): Promise<void> }).kill();
766853
} else if ('quit' in this.driver) {
767854
await (this.driver as { quit(): Promise<void> }).quit();
768855
}

src/firefox/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ export interface FirefoxLaunchOptions {
6060
acceptInsecureCerts?: boolean | undefined;
6161
connectExisting?: boolean | undefined;
6262
marionettePort?: number | undefined;
63+
marionetteHost?: string | undefined;
6364
env?: Record<string, string> | undefined;
6465
logFile?: string | undefined;
6566
/** Firefox preferences to set at startup via moz:firefoxOptions */

src/index.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,11 @@ export async function getFirefox(): Promise<FirefoxDevTools> {
9898
if (firefox) {
9999
const isConnected = await firefox.isConnected();
100100
if (!isConnected) {
101-
log('Firefox connection lost - browser was closed or disconnected');
101+
log('Firefox connection lost, reconnecting...');
102102
resetFirefox();
103-
throw new FirefoxDisconnectedError('Browser was closed');
103+
} else {
104+
return firefox;
104105
}
105-
return firefox;
106106
}
107107

108108
// No existing instance - create new connection
@@ -142,6 +142,7 @@ export async function getFirefox(): Promise<FirefoxDevTools> {
142142
acceptInsecureCerts: args.acceptInsecureCerts,
143143
connectExisting: args.connectExisting,
144144
marionettePort: args.marionettePort,
145+
marionetteHost: args.marionetteHost,
145146
env: envVars,
146147
logFile: args.outputFile ?? undefined,
147148
prefs,
@@ -358,6 +359,25 @@ async function main() {
358359

359360
log('Firefox DevTools MCP server running on stdio');
360361
log('Ready to accept tool requests');
362+
363+
// Clean up the Marionette session so Firefox accepts new connections.
364+
// Without this, the session stays locked after the MCP client disconnects.
365+
const cleanup = async () => {
366+
if (firefox) {
367+
try {
368+
await firefox.close();
369+
} catch {
370+
// ignore
371+
}
372+
}
373+
await server.close();
374+
process.exit(0);
375+
};
376+
process.on('SIGTERM', cleanup);
377+
process.on('SIGINT', cleanup);
378+
// StdioServerTransport does not fire onclose on stdin EOF.
379+
process.stdin.on('end', cleanup);
380+
process.stdin.on('close', cleanup);
361381
}
362382

363383
// Only run main() if this file is executed directly (not imported)

0 commit comments

Comments
 (0)