Make browser tools safe for hosted sessions#166
Open
yyy900 wants to merge 10 commits into
Open
Conversation
Playwright's sync API binds to the thread that started it. host() runs each agent turn on an arbitrary threadpool thread, so a hosted agent's second turn hit 'Cannot switch to a different thread', close() failed the same way while still nulling out state, and the orphaned Chromium kept the profile locked. Route every public BrowserAutomation method through a single worker thread: playwright starts on it and every later call executes there, whatever thread the server picked.
…ave_state) seed_state=<path> injects an exported storage_state JSON's cookies into the persistent context once, right after it is created and before any navigation, so a deployed agent starts already signed in. launch_persistent_context() can't take storage_state=, so the cookies are applied with add_cookies. save_state() exports the current session to that portable JSON. Unset seed_state => no injection, behaviour unchanged.
… params click_element_by_selector already clicks through Playwright's input layer (trusted, isTrusted=true) — the only thing it couldn't do was reach a cross-origin iframe. Add frame_url_contains/frame_name to give it that, and delete click_in_frame, which otherwise duplicated the selector-click protocol. Drop _human_mouse_move (its only caller); pre-click motion realism lives in the opt-in human_jitter plugin, not the click tool. Removes now-unused random/time imports.
Each chat session drives its own tab in the shared (single-login) context so concurrent sessions don't navigate over each other; the bind_browser_session plugin routes calls by session_id. Tabs are reclaimed by idle-TTL and a max-tabs backstop so a long-lived host doesn't leak. Re-encode screenshots over ~600KB as JPEG so a single frame stays under the relay's 1MB cap.
- wait_for_manual_login returns immediately without an interactive TTY instead of blocking input() on the single shared browser worker thread (that thread serializes every session, so blocking it would freeze them all on a host) - bound the per-session restore-URL map (LRU, _max_url_memory) so it cannot grow unbounded by session id on a long-lived shared host - omit :port from BROWSER_PROXY server string when the URL has no port - reframe iframe targeting, BROWSER_PROXY, and cursor-motion docs around legitimate use; drop CAPTCHA/anti-detection framing - document save_state/seed_state output as a secret (gitignore, secret store) - tests for the no-TTY guard and the bounded restore-URL map
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abf5294099
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Context-level methods that must NOT auto-create a tab before they run: | ||
| # open_browser creates the tab itself after launching the context; close/save_state | ||
| # operate on the whole context, and a fresh tab right before teardown is wrong. | ||
| _NO_TAB_METHODS = {"open_browser", "close", "save_state"} |
There was a problem hiding this comment.
When a hosted BrowserAutomation already has a live tab for session A and session B calls the exposed open_browser tool first, keeping open_browser in _NO_TAB_METHODS means the wrapper does not create B's tab before the liveness check. _browser_is_usable() then evaluates self.page for B as None, so open_browser() treats the live shared context as stale and calls close(), tearing down A's tab/context. Let open_browser ensure the current session page or check context liveness before deciding the browser is stale.
Useful? React with 👍 / 👎.
Comment on lines
+389
to
+391
| cookies = json.loads(Path(self._seed_state).read_text()).get("cookies", []) | ||
| if cookies: | ||
| self.browser.add_cookies(cookies) |
There was a problem hiding this comment.
For a state file produced by the new save_state(), Playwright includes origins with localStorage as well as cookies, but seed_state only reads and imports the cookies here. When a site keeps its auth token or required session data in localStorage, the hosted browser will start unauthenticated even though the exported state was supplied. Either replay the saved origins/localStorage into the context or make the export/import explicitly cookie-only.
Useful? React with 👍 / 👎.
A shared BrowserAutomation drives one tab per session. Three teardown paths could let one session destroy every other session's tabs: - a brand-new session's open_browser read its own (None) page as 'context unusable' and tore the shared context down — now it ensures its tab BEFORE judging usability - open_browser(force=True) and close() always tore the whole context down — now a BOUND session recycles/closes only its OWN tab; the full teardown (extracted to _teardown()) is reserved for an unbound caller (single-session CLI / context-manager exit) The else-branch still does a FULL _teardown() when the context is genuinely dead. Tests: a second session reuses the live context; force/close from a bound session leave other sessions intact; an autouse fixture resets the session threadlocal per test.
wu-changxing
pushed a commit
that referenced
this pull request
Jul 7, 2026
…loses #168) (#186) * browser: per-session tabs and hosted-session safety Serialize BrowserAutomation's public calls through one Playwright-owning worker thread so a shared instance is safe to call from any host() thread, and give each session (chat panel) its own tab in the shared login context so concurrent sessions stop navigating over one another. Reclaim idle tabs by TTL and an LRU backstop, restoring a reclaimed session's last URL on next use. Adds portable login state (save_state/seed_state), BROWSER_PROXY egress, frame-aware selector clicks, a bounded screenshot payload, and the opt-in human_jitter and bind_browser_session plugins. Ported from #166 onto current main. * browser: enable per-session isolation on the hosted co-ai agent The per-session tab machinery only activates when bind_browser_session runs before each tool, but no agent registered it — so on a co ai deploy every chat panel still shared one tab, the exact bug the machinery fixes. Register the plugin on create_coding_agent (the hosted browser agent) and assert it stays wired. * browser: replace Playwright with Patchright for driver-level stealth Patchright is a stealth-patched, API-compatible drop-in for Playwright. It removes the Runtime.enable / Console.enable CDP leaks and the navigator.webdriver flag at the driver level — below where Chrome flags and page init scripts operate, which is exactly what modern bot-detection (Cloudflare, DataDome, Kasada) reads. - swap the import; the sync API is identical so only the import line changes - drop the navigator.webdriver add_init_script shim (Patchright hides it, and init-script injection is itself a timing-detectable tell) - trim browser_config.py from the 53-flag browser-use mirror to only run-environment flags; Patchright owns anti-detection now, and its docs warn that over-configuring launch args can defeat the patches. Removes the standing browser-use sync burden. - rename PLAYWRIGHT_AVAILABLE -> BROWSER_AVAILABLE (it no longer gates Playwright) - update pyproject, the browser template requirements, and the co-ai Dockerfile (patchright install --with-deps chrome) Verified live: BrowserAutomation launches real Chrome via patchright, navigates, and navigator.webdriver reads false with no init script. Standalone raw-Playwright demos under examples/ are left as-is (not the SDK browser path). * docs(browser): update install to patchright The built-in browser tools ship Patchright now; the doc's pip/install commands pointed users at Playwright. * browser: instance-owned session binding + documented tab TTL/cap kwargs Cleanups on the per-session tab machinery, mechanism unchanged (the opt-in bind_browser_session plugin stays the way a hosted agent enables isolation on a shared browser): - The session binding moves off a module-level threading.local onto the BrowserAutomation instance (per instance, per thread). No shared module state between browser instances, and the test suites drop their autouse reset fixtures — a fresh instance starts unbound. - The plugin's handler is renamed _bind_browser_session so it no longer shadows the browser method (_bind_session) it calls; docstrings spell out the handler-vs-method split and who registers the plugin. - tab_idle_ttl / max_tabs become documented constructor kwargs so memory-tight deployments stop poking private attributes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * cli: add hosted-browser template — production-shaped site-automation agent The shape proven by the live LinkedIn agent, packaged as 'co init/create --template hosted-browser': one shared Patchright browser (persistent login profile, headed under Xvfb) served to many sessions via a host(create_agent) factory with bind_browser_session giving each session its own tab; site workflows as .co/skills; credentials and 2FA through ask_user; CAPTCHA = report-and-stop; and the memory bounds that keep a small deploy box alive (tab TTL/cap kwargs, a browser idle reaper class). Local one-shot mode (python agent.py "task") keeps wait_for_manual_login for first login; the hosted path removes it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * browser: replace _NO_TAB_METHODS name set with @_no_auto_tab markers The name set was stringly-typed and far from the methods it exempted: rename open_browser/close/save_state and the set silently stops matching, so the method starts auto-creating tabs with no error. The marker sits on the method itself and survives renames. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * cli/docs: finish the playwright -> patchright / browser rename Two kinds of stragglers left behind by earlier renames: - The interactive create menu still offered a 'playwright' template that has no templates/playwright/ directory (renamed to 'browser' long ago), so picking it always died with "Template not found". The menu now lists the real templates (browser, hosted-browser) and the --template help text / module notes on init/create stop advertising 'playwright'. - User-facing install instructions for the SDK browser path still said 'pip install playwright && playwright install chromium' while the code and its error message moved to patchright + chrome. Docs now match. Raw-Playwright tutorial examples (custom stateful tools in docs/concepts, best-practices) and the sync_playwright/PLAYWRIGHT_BROWSERS_PATH API names stay: patchright exports Playwright's names on purpose. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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
co browserCLI mode intact_browser_is_usable()safe for daemon-side liveness checks from non-browser threadsbind_browser_sessionso hosted sessions do not fight over one pagehuman_jitterVerification
Note: the daemon tests require AF_UNIX socket bind permissions and were run outside the sandbox.