From ae64bb569d9bcb3b32036da4cddb9211b64cde1f Mon Sep 17 00:00:00 2001 From: Jeremy Schoemaker Date: Tue, 8 Sep 2026 17:29:39 -0500 Subject: [PATCH 1/2] fix: bypass Selenium Manager only where it cannot run PR #170 made every platform pass an explicit geckodriver path to firefox.ServiceBuilder. That fixed aarch64 Linux, where Selenium Manager ships an x86-64 binary that cannot execute, but it also regressed browser resolution in 0.10.2 for everyone else. selenium-webdriver's Firefox Driver.createSession() calls getBinaryPaths() only when the supplied DriverService has no executable, and that one call resolves geckodriver *and* the Firefox binary it injects into moz:firefoxOptions.binary. Handing the service a path therefore also opts out of finding Firefox. On a machine with no system Firefox, 0.10.1 launched fine because Selenium Manager downloaded and selected one under ~/.cache/selenium/firefox/; 0.10.2 fails with "Unable to detect Firefox binary automatically" unless --firefox-path is given. This is the same mechanism the Android branch relies on deliberately, where skipping getBinaryPaths() is what stops a desktop binary being injected over androidPackage. Resolve geckodriver ourselves only where Selenium Manager genuinely cannot do the job: win32, where ServiceBuilder() invoked from the MCP hangs (Bug 2040849), and non-x64 Linux (Bug 2062055). Elsewhere leave the executable unset so Selenium Manager resolves both halves as it did in 0.10.1. The test added in #170 asserted an explicit path on every platform, so it encoded the regression as expected behaviour and could not have caught this. It is replaced with per-platform cases covering both sides, verified to fail against the 0.10.2 code and pass against this change. Reported and diagnosed by @mightykatun on #170. --- src/firefox/core.ts | 29 +++++++++++++++---- tests/firefox/core.test.ts | 59 +++++++++++++++++++++++++++++++------- 2 files changed, 72 insertions(+), 16 deletions(-) diff --git a/src/firefox/core.ts b/src/firefox/core.ts index 078a247..aa39202 100644 --- a/src/firefox/core.ts +++ b/src/firefox/core.ts @@ -364,11 +364,30 @@ export class FirefoxCore { } } - // Always resolve geckodriver ourselves rather than relying on selenium - // entirely. See Bug 2062055, 2040849. - const geckodriverPath = await findGeckodriver(); - logDebug(`Using geckodriver: ${geckodriverPath}`); - const serviceBuilder = new firefox.ServiceBuilder(geckodriverPath); + // Resolve geckodriver ourselves only where Selenium Manager cannot do the + // job, because giving the service an executable costs more than it looks. + // selenium-webdriver calls getBinaryPaths() only when the DriverService + // has none, and that single call resolves geckodriver *and* the Firefox + // binary it injects into moz:firefoxOptions.binary. Passing a path + // therefore also opts out of finding Firefox, which is exactly what the + // Android branch above relies on and what broke desktop users with no + // system Firefox in 0.10.2. + // win32: ServiceBuilder() invoked from the MCP hangs (Bug 2040849). + // non-x64 Linux: Selenium Manager ships an x86-64 binary that cannot + // execute on aarch64 (Bug 2062055). + // Everywhere else Selenium Manager works, so let it resolve both. + const mustResolveGeckodriver = + process.platform === 'win32' || (process.platform === 'linux' && process.arch !== 'x64'); + + let serviceBuilder; + if (mustResolveGeckodriver) { + const geckodriverPath = await findGeckodriver(); + logDebug(`Using geckodriver: ${geckodriverPath}`); + serviceBuilder = new firefox.ServiceBuilder(geckodriverPath); + } else { + logDebug('Letting Selenium Manager resolve geckodriver and Firefox'); + serviceBuilder = new firefox.ServiceBuilder(); + } if (this.logFilePath) { // Create the parent directory, as the generated-path branch above does. diff --git a/tests/firefox/core.test.ts b/tests/firefox/core.test.ts index 44fe2b2..563f096 100644 --- a/tests/firefox/core.test.ts +++ b/tests/firefox/core.test.ts @@ -4,7 +4,7 @@ import { join } from 'node:path'; import { MCP_PROFILE_DIR_NAME } from '@/firefox/profile.js'; -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { FirefoxCore } from '@/firefox/core.js'; import type { FirefoxLaunchOptions } from '@/firefox/types.js'; @@ -389,18 +389,55 @@ describe('FirefoxCore connect() profile handling', () => { ); }); - // Bug 2062055: geckodriver path should always be resolved before calling - // the ServiceBuilder. - it('should build the geckodriver service with an explicit binary path', async () => { - const { FirefoxCore } = await import('@/firefox/core.js'); + // Bug 2062055 and its follow-up. Giving the DriverService an executable also + // opts out of Selenium Manager resolving the Firefox binary, so the explicit + // path is only correct where Selenium Manager cannot run. + describe('geckodriver resolution', () => { + const platform = process.platform; + const arch = process.arch; - const core = new FirefoxCore({ headless: true }); - await core.connect(); + const setPlatform = (value: NodeJS.Platform, archValue: string) => { + Object.defineProperty(process, 'platform', { value, configurable: true }); + Object.defineProperty(process, 'arch', { value: archValue, configurable: true }); + }; + + afterEach(() => { + setPlatform(platform, arch); + }); + + it.each([ + ['win32', 'x64'], + ['linux', 'arm64'], + ] as const)( + 'resolves geckodriver explicitly on %s/%s, where Selenium Manager cannot', + async (osName, archName) => { + setPlatform(osName, archName); + const { FirefoxCore } = await import('@/firefox/core.js'); + + await new FirefoxCore({ headless: true }).connect(); + + expect(mockServiceBuilderCtor).toHaveBeenCalledTimes(1); + const [geckodriverPath] = mockServiceBuilderCtor.mock.calls[0] as [unknown]; + expect(typeof geckodriverPath).toBe('string'); + expect(String(geckodriverPath)).toContain('geckodriver'); + } + ); + + it.each([ + ['linux', 'x64'], + ['darwin', 'arm64'], + ] as const)( + 'leaves the service executable unset on %s/%s, so Selenium Manager still finds Firefox', + async (osName, archName) => { + setPlatform(osName, archName); + const { FirefoxCore } = await import('@/firefox/core.js'); - expect(mockServiceBuilderCtor).toHaveBeenCalledTimes(1); - const [geckodriverPath] = mockServiceBuilderCtor.mock.calls[0] as [unknown]; - expect(typeof geckodriverPath).toBe('string'); - expect(String(geckodriverPath)).toContain('geckodriver'); + await new FirefoxCore({ headless: true }).connect(); + + expect(mockServiceBuilderCtor).toHaveBeenCalledTimes(1); + expect(mockServiceBuilderCtor.mock.calls[0]).toHaveLength(0); + } + ); }); }); From a8b2b8394354a73a532b028382755be35ae7103f Mon Sep 17 00:00:00 2001 From: Julian Descottes Date: Thu, 10 Sep 2026 09:42:55 +0200 Subject: [PATCH 2/2] nit: use shorter comment for binary detection in core.ts --- src/firefox/core.ts | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/firefox/core.ts b/src/firefox/core.ts index aa39202..a54b0da 100644 --- a/src/firefox/core.ts +++ b/src/firefox/core.ts @@ -364,18 +364,10 @@ export class FirefoxCore { } } - // Resolve geckodriver ourselves only where Selenium Manager cannot do the - // job, because giving the service an executable costs more than it looks. - // selenium-webdriver calls getBinaryPaths() only when the DriverService - // has none, and that single call resolves geckodriver *and* the Firefox - // binary it injects into moz:firefoxOptions.binary. Passing a path - // therefore also opts out of finding Firefox, which is exactly what the - // Android branch above relies on and what broke desktop users with no - // system Firefox in 0.10.2. - // win32: ServiceBuilder() invoked from the MCP hangs (Bug 2040849). - // non-x64 Linux: Selenium Manager ships an x86-64 binary that cannot - // execute on aarch64 (Bug 2062055). - // Everywhere else Selenium Manager works, so let it resolve both. + // Giving the service an executable skips getBinaryPaths(), which resolves + // geckodriver *and* the Firefox binary injected into moz:firefoxOptions.binary. + // Only do that where Selenium Manager cannot run: win32 hangs when invoked from + // the MCP (Bug 2040849), non-x64 Linux ships an x86-64 binary (Bug 2062055). const mustResolveGeckodriver = process.platform === 'win32' || (process.platform === 'linux' && process.arch !== 'x64');