fix: connect-existing with remote/containerized Firefox#50
Merged
freema merged 1 commit intomozilla:mainfrom Mar 29, 2026
Merged
fix: connect-existing with remote/containerized Firefox#50freema merged 1 commit intomozilla:mainfrom
freema merged 1 commit intomozilla:mainfrom
Conversation
MayCXC
force-pushed
the
connect-existing-bidi-v2
branch
from
1b0f461 to
fcd5855
Compare
MayCXC
force-pushed
the
connect-existing-bidi-v2
branch
2 times, most recently
from
ae0f07a to
5f6b39f
Compare
MayCXC
changed the title
MayCXC
force-pushed
the
connect-existing-bidi-v2
branch
6 times, most recently
from
478f62d to
4740414
Compare
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.
MayCXC
force-pushed
the
connect-existing-bidi-v2
branch
from
4740414 to
c02fc02
Compare
Contributor
Author
|
@juliandescottes @freema iterated on this a bit, it is ready now. |
Collaborator
|
Hey @MayCXC, this looks great, thanks! I'll merge this into main. I'll add tests and do some manual testing, then cut a new release. Small note: |
freema
added a commit
that referenced
this pull request
Mar 29, 2026
- Fix prettier formatting (long lines, missing line breaks) - Fix curly brace requirement for if statements - Fix no-base-to-string: use String() instead of .toString() - Fix no-floating-promises: void async kill() in reset() - Fix no-misused-promises: wrap async cleanup in void handler - Remove unused FirefoxDisconnectedError import - Fix IBiDi type to include subscribe method - Fix type narrowing for bidiConnection return Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
freema
added a commit
that referenced
this pull request
Mar 29, 2026
Tests cover: - GeckodriverHttpDriver BiDi: getBidi(), subscribe, caching, error handling - Session cleanup: kill() and quit() send DELETE /session - kill() resilience when DELETE fails - marionetteHost option passthrough - Reconnect: reset() clears driver state Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes several issues that prevented
--connect-existingfrom working with Firefox running on a remote host or in a separate container (e.g., via SSH tunnel).Bug fixes
--marionette-hostCLI arg was silently ignored. The option was parsed and the type had the field, but it was never passed to the options object. Geckodriver always connected to127.0.0.1.selenium-manager downloaded a 307MB Firefox binary. In connect-existing mode Firefox is already running. Changed
--browser firefoxto--driver geckodriverso selenium-manager only downloads the driver (~5MB).No cleanup on disconnect. No lifecycle handling existed. The Marionette session was never released when the MCP client disconnected, so Firefox refused new connections until restarted. Added SIGTERM/SIGINT/stdin handlers that clean up Firefox and close the server before exiting, following the pattern from the MCP SDK examples.
StdioServerTransportdoes not currently fireoncloseon stdin EOF (PR #1814), so raw stdin listeners are included as a workaround.kill()skipped session cleanup. Was synchronous and did not sendDELETE /session. Made async with session cleanup so Marionette accepts new connections.Seamless reconnect after Firefox restart.
getFirefox()now detects a lost connection and reconnects transparently instead of throwing an error on the first call.New features
--marionette-hostparameter. Controls which host geckodriver connects to for Marionette and where the BiDi WebSocket URL is rewritten to point. SupportsMARIONETTE_HOSTenv var.BiDi support for connect-existing. Opens a WebSocket to Firefox's Remote Agent using the
webSocketUrlsession capability, enabling console and network event monitoring.Test plan
list_pagesworks with--connect-existing --marionette-host host.internallist_console_messagesreturns BiDi dataSupersedes #51.