feat: CAMOFOX_USER_DATA_DIR env routes launch to persistent context#4793
Closed
DaveDev42 wants to merge 3 commits into
Closed
feat: CAMOFOX_USER_DATA_DIR env routes launch to persistent context#4793DaveDev42 wants to merge 3 commits into
DaveDev42 wants to merge 3 commits into
Conversation
When CAMOFOX_USER_DATA_DIR is set to a /tmp/firefox-borrow-<uuid>/ path, the wrapper switches from firefox.launch(options) to firefox.launchPersistentContext(userDataDir, options) so the launch reuses an existing Firefox profile (typically an rsync clone of the user's daily profile created by an external 'firefox-borrow' shell). Env unset → exact pre-patch behaviour, no regression risk. Persistent launches return a BrowserContext, not a Browser. shimNewContext aliases .newContext() to return the persistent context itself (Playwright forbids multiple contexts on a persistent launch), and stubs .contexts() for downstream iteration. Agent code asking for isolated contexts in borrow mode gets the default context back with a one-shot warning.
camofox-browser's wrapper calls browser.isConnected() at three sites (session getter ~L1086, /health route ~L2404, root route ~L5207), and Playwright's BrowserContext has no such method. Without a shim the first /tabs request 500s with `browser.isConnected is not a function`, and /health perpetually reports browserConnected:false even when the persistent context is healthy. Proxy isConnected() through browser.browser().isConnected() — Browser- Context.browser() returns the underlying Browser instance, which has isConnected. Fall back to `true` if browser() is unavailable, since context calls throw on stale browsers anyway (optimistic is safe; the real failure surfaces at next operation). Also bumps the shim's coverage from 2 methods (newContext, contexts) to 3, matching the actual Browser-only API surface the wrapper relies on. Other Browser methods (close, on, removeListener, version, etc.) are either present on BrowserContext or not called by the wrapper.
Contributor
|
Closing as superseded by the newer persistent-profile design in #6525, which is already labeled |
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
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
Adds support for a
CAMOFOX_USER_DATA_DIRenv var. When set to a pathmatching
/tmp/firefox-borrow-*, the wrapper switches fromfirefox.launch(options)tofirefox.launchPersistentContext(userDataDir, options).When unset (default), the wrapper behaves exactly like before — same
firefox.launch(options)call, same ephemeral profile. Regression riskon the default path: zero.
Motivation
Lets an external orchestrator clone a Firefox profile into a temp dir
and have camofox-browser launch against it, so agents can act on sites
the user is already logged into (cookies, OAuth tokens, etc.) without
the wrapper handling credentials itself. We use this from a hermes-agent
skill on top of an
rsyncclone of the user's daily Firefox profile.Changes (3 commits)
feat: env-var branch (
62332d1)lib/launch-branch.jswith a purechooseLaunch(env)decisionfunction. Validates that the env value is absolute and starts with
the
/tmp/firefox-borrow-prefix (defense in depth — the consumingshell also validates this, but a wrapper-side check protects
against a maliciously edited plist).
server.js's launch block branches onchooseLaunch(process.env).The else path is the original
firefox.launch(options)lineunchanged.
shimNewContext(browser)helper aliases.newContext()on apersistent context to return the context itself (Playwright forbids
multiple contexts on a persistent launch), with a one-shot warning
logged. Also stubs
.contexts()to return[self]so downstreamiteration works.
test/launch-branch.test.jscover the decisionfunction (
node --test; no extra dev deps).chore: clarify shimNewContext options-ignored semantics (
0892bbd)passed to
newContext({viewport, permissions, …})are dropped inpersistent mode. Found while reviewing the shim before merge.
fix: shim browser.isConnected() in persistent mode (
8ca7001)BrowserContexthas noisConnected(); the wrapper calls it atthree sites (session getter, /health route, root route). Without
the shim the first
/tabsrequest 500s withbrowser.isConnected is not a functionand/healthperpetuallyreports
browserConnected:false. Proxies throughbrowser.browser().isConnected()when available, falls back totrue(the context throws on use if the underlying browser isgone — optimistic is safe, the real failure surfaces at next op).
Test plan
node --test test/launch-branch.test.js— 5/5 pass for thebranch decision (env unset, empty, valid path, paths outside the
/tmp/firefox-borrow-prefix, relative path).firefox.launchPersistentContextis invoked, agent sees priorcookies, env-unset path produces the same launch as before, and
/tabs//healthwork in both modes.Backwards compatibility
Zero regression risk on the default (env-unset) path — the original
firefox.launch(options)line runs unchanged.When
CAMOFOX_USER_DATA_DIRis set, the wrapper enters asingle-context mode. Agents requesting an isolated
newContext({…})get the default context back with a logged warning — sufficient for
single-site agent work, not appropriate for multi-tenant scenarios.
The warning is one-shot per browser instance to avoid log spam.