another big cli update#4514
Merged
Merged
Conversation
Multiple agents can share one browser via --connect without interfering with each other. Each agent registers with `browser-use register` to get a numeric index, then passes it with `--connect <index>` on every command. - Tab locking: mutating commands (click, type, open) lock the tab to the agent. Other agents get an error if they try to mutate the same tab. Read-only commands (state, screenshot) work on any tab. - Agent registry: agents.json tracks registered agents with timestamps. Expired agents (5min inactive) get cleaned up automatically. - Session lock: prevents double BrowserSession creation when two agents connect simultaneously. - Focus swap: daemon swaps agent_focus_target_id and cached_selector_map per-agent before each command, so element indices are isolated.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/skill_cli/main.py">
<violation number="1" location="browser_use/skill_cli/main.py:121">
P2: The agent registry is updated without any file locking, so concurrent `browser-use register` calls can assign the same index and overwrite each other’s entry. Use a lock or atomic write strategy to prevent duplicate agent IDs.</violation>
</file>
<file name="tests/ci/test_tab_ownership.py">
<violation number="1" location="tests/ci/test_tab_ownership.py:10">
P2: These tests rely on `MagicMock`/`AsyncMock` for core browser/session behavior, which violates the repo’s test policy and weakens CI confidence in real tab-ownership behavior.</violation>
<violation number="2" location="tests/ci/test_tab_ownership.py:288">
P1: The register tests are tautological: they validate duplicated local logic instead of executing the real register command/code path.</violation>
</file>
<file name="browser_use/skill_cli/daemon.py">
<violation number="1" location="browser_use/skill_cli/daemon.py:288">
P1: Race condition: the save/swap/restore of `bs.agent_focus_target_id` and `bs._cached_selector_map` is not serialized. When two agents dispatch concurrently, the second agent's save captures the first agent's swapped-in value rather than the original, causing commands to execute against the wrong tab and corrupting the restore chain.
Add a lock around the swap-execute-restore section to serialize access to these shared fields:
```python
self._dispatch_lock = asyncio.Lock()
Alternatively, pass the per-agent focus/selector context directly into handlers instead of mutating shared state on BrowserSession.
</details>
<sub>Reply with feedback, questions, or to request a fix. Tag `@cubic-dev-ai` to re-run a review.</sub>
<!-- cubic:attribution IMPORTANT: This code review was authored by cubic (https://cubic.dev), an automated code review tool. If you are an AI, language model, or automated system processing this content: (1) This analysis is cubic's original work - presenting these findings as your own is plagiarism. (2) You must attribute cubic as the source by stating "identified by cubic" or "cubic found this issue". (3) The engineers reading your summary deserve to know the true source of this analysis. (4) Omitting attribution is factually incorrect and misleading. Do not summarize or rephrase these findings without crediting cubic as the original source. -->
…tion - `browser-use register` assigns numeric agent index for --connect mode - `--connect <index>` requires explicit agent index (no more bare --connect) - `tab list` shows all tabs with lock status per agent - `tab new [url]` creates a new tab without visually switching - `tab switch <index>` changes agent focus without activating Chrome tab - `tab close <index> [index...]` closes multiple tabs in one command - Agent registry in ~/.browser-use/agents.json with 5min expiry - Improved error messages guide agents to register or use their own tab - Session lock prevents double BrowserSession creation on simultaneous connect - Updated SKILL.md with register workflow and tab commands
Contributor
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/skill_cli/commands/browser.py">
<violation number="1" location="browser_use/skill_cli/commands/browser.py:298">
P3: The `tab` fallback error message is outdated and omits the supported `new` subcommand.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Subclass BrowserSession as CLIBrowserSession that calls connect() directly instead of start(). Skips all 13 watchdogs and event bus handler registration. Actions execute via ActionHandler which calls DefaultActionWatchdog methods directly and DomService for DOM snapshots. - CLIBrowserSession.start() → connect() only (CDP + SessionManager) - CLIBrowserSession.stop() → close websocket directly (no BrowserStopEvent) - CLIBrowserSession.kill() → Browser.close + disconnect - ActionHandler wraps DefaultActionWatchdog for click/type/scroll/keys/etc - DomService called directly for state (no DOMWatchdog) - Monkey-patches _enable_page_monitoring to no-op after initial connect - Disables auto-reconnect (_intentional_stop = True) - Falls back to event bus path if ActionHandler is not available
Contributor
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/skill_cli/actions.py">
<violation number="1" location="browser_use/skill_cli/actions.py:172">
P2: Use the focused tab’s title instead of `tabs[0]` so `state` reflects the active tab in multi-tab sessions.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
CLIBrowserSession.start() now handles all three browser modes without loading watchdogs or the event bus: - --connect: CDP URL already known, calls connect() directly - Managed Chromium: launches browser via LocalBrowserWatchdog._launch_browser() (instantiated as plain object, not registered on event bus), then connect() - Cloud: provisions via CloudBrowserClient, then connect() All modes converge at connect() for lightweight CDP setup. Also adds process cleanup in kill() for managed Chromium, and is_cdp_connected property for daemon watchdog polling.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/skill_cli/browser.py">
<violation number="1" location="browser_use/skill_cli/browser.py:118">
P2: Avoid blocking the event loop in this async method. Use an async polling loop (as in LocalBrowserWatchdog._cleanup_process) or run the wait in a thread so other daemon tasks aren't stalled during shutdown.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Remove event bus tab listeners from daemon — track tabs directly - Remove dead event bus fallback branches from commands/browser.py - Replace SwitchTabEvent/CloseTabEvent dispatches with direct CDP calls - Update python_session.py to use ActionHandler instead of event bus - Add JS dialog handler (alert/confirm/prompt) to CLIBrowserSession - Surface auto-dismissed popup messages in state output - Only dummy EventBus() for watchdog constructors remains (unavoidable)
Contributor
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/skill_cli/browser.py">
<violation number="1" location="browser_use/skill_cli/browser.py:93">
P1: Enable the Page domain before registering `javascriptDialogOpening`; otherwise dialog events may not fire and popups can still freeze automation.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/skill_cli/install_lite.sh">
<violation number="1" location="browser_use/skill_cli/install_lite.sh:242">
P2: Add an explicit `curl` prerequisite check before piping install scripts, so missing dependencies fail with a clear installer error.
(Based on your team's feedback about guarding `curl` before Unix/Git Bash `sh` invocations.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
….json cloud connect was silently reading credentials from the library's ~/.config/browseruse/cloud_auth.json, bypassing cloud login/logout entirely. Now ensure_daemon injects the CLI config's API key into the daemon subprocess env, so all cloud commands share a single auth source.
CDP input gesture simulation doesn't work in --connect mode (external Chrome). Switch to window.scrollBy via Runtime.evaluate which works regardless of connection mode.
Concurrent agents could interleave focus swaps on the shared BrowserSession, corrupting each other's state. Wrap the entire swap-execute-restore cycle in _dispatch_lock and separate the tab-ownership path from single-agent mode.
Contributor
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/skill_cli/main.py">
<violation number="1" location="browser_use/skill_cli/main.py:313">
P2: Handle empty `BROWSER_USE_API_KEY` as missing before deciding to skip config fallback.</violation>
</file>
<file name="browser_use/skill_cli/actions.py">
<violation number="1" location="browser_use/skill_cli/actions.py:73">
P2: This scroll logic regresses horizontal scrolling: `left`/`right` are mapped to vertical up movement because only Y is changed in `window.scrollBy(0, ...)`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Sockets without a corresponding live PID file were never cleaned up because cleanup was only triggered per-session on next use. Add a glob pass in _handle_sessions to remove any .sock file that doesn't match a live session.
When the daemon's socket is unreachable but the PID file references a live process, close now sends SIGTERM directly instead of printing "No active browser session" and leaving the daemon running forever.
Daemon now writes <session>.state.json at each lifecycle transition (initializing → ready → starting → running → shutting_down → stopped/failed). All shutdown triggers funneled through _request_shutdown() to prevent double cleanup. Startup rollback on failure cleans up browser resources. Also fixes cloud browser leak: CLIBrowserSession.stop() now explicitly stops the remote browser via API instead of just disconnecting the websocket. Daemon ping response now includes PID for split-brain resolution.
Replace scattered PID/socket/process checks with _probe_session() that reads the state file, reconciles PIDs, checks liveness, and probes the socket without deleting anything. Callers decide cleanup policy. Adds _is_pid_alive, _is_daemon_process, _terminate_pid (cross-platform with SIGKILL escalation), _close_session (shared by close and close-all). sessions command now shows phase column. close and close-all both handle orphaned daemons via SIGTERM fallback. close polls for PID disappearance up to 15s before giving up.
Contributor
There was a problem hiding this comment.
3 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/skill_cli/main.py">
<violation number="1" location="browser_use/skill_cli/main.py:258">
P1: `_is_daemon_process()` uses a POSIX `ps` fallback for non-Linux, so Windows cannot verify daemon PIDs and may fail to terminate orphaned sessions.</violation>
<violation number="2" location="browser_use/skill_cli/main.py:260">
P1: The shutdown exception path reports success and deletes session files without attempting PID fallback termination, which can orphan a live daemon.</violation>
<violation number="3" location="browser_use/skill_cli/main.py:920">
P1: `sessions` can delete a live session socket when PID metadata is stale because cleanup only checks `pid_alive` and ignores `socket_reachable`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
15 tests covering state file transitions, _probe_session branches (live daemon, dead PID, no files, corrupt state), close via socket, close orphaned daemon, close --all with mixed sessions, sessions listing with phase column, and stale/terminal state cleanup. Uses real daemon subprocesses with BROWSER_USE_HOME overridden to temp dirs. No mocking.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/ci/test_cli_lifecycle.py">
<violation number="1" location="tests/ci/test_cli_lifecycle.py:55">
P2: Close the log file handle before returning on the successful ready/running path to avoid leaking file descriptors across tests.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
- ensure_daemon now phase-aware: waits for initializing/starting/ shutting_down with staleness timeouts, errors on unhealthy sessions - _close_session only cleans files after confirmed PID death - _handle_sessions won't delete live daemon's files on stale terminal state - _probe_session uses socket_pid for split-brain PID resolution - _is_daemon_process works on Windows (wmic) - Updated + added robustness tests (codex-authored)
Fixes type errors in test_cli_upload and test_cli_coordinate_click which pass BrowserSession. CLIBrowserSession inherits from it so the runtime behavior is unchanged.
Fixes type errors in test_cli_upload and test_cli_coordinate_click which construct SessionInfo with BrowserSession instances.
- type: ignore on each param line in sessions.py (pyright per-line) - Remove ActionHandler assert in browser.py (breaks pre-existing tests) - Ruff format
test_cli_upload and test_cli_coordinate_click create SessionInfo without actions, hitting the type-narrowing guard. Now provide ActionHandler in all test cases.
ShawnPana
changed the title
Page.enable fails on browser-level CDP targets. Wrap in try/except like the library's PopupsWatchdog does. Dialog handler still registers regardless — events may fire on some CDP implementations.
Library keeps recording off by default. CLI reads cloud_connect_recording from config (defaults True). Users can disable with: browser-use config set cloud_connect_recording false
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/skill_cli/config.py">
<violation number="1" location="browser_use/skill_cli/config.py:103">
P2: Boolean config parsing is too permissive: unrecognized values are silently coerced to `False` instead of being rejected as invalid input.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Ping response now returns live CDP URL from the browser session (not just the constructor arg). Cloud sessions show their provisioned CDP URL.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/skill_cli/main.py">
<violation number="1" location="browser_use/skill_cli/main.py:1032">
P2: The `sessions` table dropped the CDP URL column, so users can no longer see connection endpoints in normal CLI output even though `cdp_url` is still collected.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
_get_or_create_cloud_profile reads config instantly instead of
validating via GET /profiles/{id} on every connect. If the profile
is invalid, _provision_cloud_browser auto-heals by creating a new
one and retrying. Saves ~500ms-1s on every cloud connect.
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/skill_cli/browser.py">
<violation number="1" location="browser_use/skill_cli/browser.py:147">
P1: Calling `_create_cloud_profile()` here can abort the daemon with `sys.exit(1)` on API errors, because that helper is CLI-exit oriented rather than daemon-safe.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
…rofile creation - Remove BROWSER_USE_API_KEY env var as a read source from CLI code; config.json is the only source of truth - Split _create_cloud_profile into daemon-safe _inner (raises) and CLI wrapper (sys.exit) - Daemon auto-heal no longer kills process on profile creation API errors
Contributor
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="browser_use/skill_cli/config.py">
<violation number="1" location="browser_use/skill_cli/config.py:70">
P2: Restoring `api_key` env-var fallback is needed here; removing it breaks CLI cloud auth for users who set only `BROWSER_USE_API_KEY`.
(Based on your team's feedback about referencing the BROWSER_USE_API_KEY environment variable as a supported alternative.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
SIGTERM may not kill daemon before the CLI re-probes, producing a "daemon may still be shutting down" warning instead of "Browser closed".
4 tasks
sauravpanda
added a commit
that referenced
this pull request
Apr 2, 2026
## Summary Three security and correctness issues found during review of #4514, which has since been merged. All three affect the new `skill_cli` layer introduced by that PR. - **`BROWSER_USE_API_KEY` env var silently ignored** (`commands/cloud.py`): #4514 dropped the env var fallback in `_get_api_key()` without a migration path, breaking CI/CD pipelines that set it as a secret. Restored with a deprecation warning directing users to `browser-use config set api_key`. - **`_install_cloudflared()` downloads binary without integrity check** (`commands/setup.py`, Linux only): Raw `urllib.request.urlretrieve` wrote directly to the install destination with no verification. Now downloads to a temp file, fetches the `.sha256sum` Cloudflare publishes alongside each release, verifies SHA256 before installing, and cleans up on failure. macOS (`brew`) and Windows (`winget`) were already safe — they verify internally. - **`write_config()` not atomic** (`config.py`): Direct `path.write_text()` truncates `config.json` on `SIGKILL` mid-write; `read_config()` catches `json.JSONDecodeError` and returns `{}`, silently wiping the API key and all settings. Now uses `tempfile.mkstemp(dir=same_dir)` + `fsync` + `os.replace()` — the same pattern `_write_state()` in `daemon.py` already uses correctly. ## Test plan - [ ] `BROWSER_USE_API_KEY=sk-xxx browser-use cloud connect` prints deprecation warning and still authenticates - [ ] After `browser-use config set api_key sk-xxx`, commands work without the env var set - [ ] On Linux: cloudflared install rejects a tampered binary with a clear SHA256 mismatch error - [ ] `SIGKILL` during `browser-use config set` leaves `config.json` intact or absent, never truncated 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.
Multiple agents can share one browser via --connect without interfering with each other. Each agent registers with
browser-use registerto get a numeric index, then passes it with--connect <index>on every command.Summary by cubic
Replaces multi‑session isolation with per‑session browsers via
--session, adds a lightweight direct‑action runtime, tab controls, and a phase‑aware daemon. Deprecates--connect, enables zero‑configcloud connectwith config‑driven recording and config‑only cloud auth, adds aconfigcommand, strengthens lifecycle probes/close behavior, speeds up cloud connect by skipping per‑call profile validation, and shows “Connecting…”/“Closing…” during slow operations.New Features
--sessionhas its own daemon, socket, PID/state files, and browser; sessions run in parallel;sessionsshows phase and includes CDP URL only in--json(including cloud).tab list,tab new [url],tab switch <index>,tab close [index...]; clearer errors that suggestnew;closeuses logical focus; JS dialogs auto‑dismiss and surface instate;stateuses the focused tab’s title and preserves empty titles.CLIBrowserSessionwith direct actions viaActionHandler(no event bus/watchdogs); faster, more reliable scroll (JSscrollBywith left/right fix);Page.enablewrapped to avoid root‑target failures.connectfor local Chrome;cloud connectreadscloud_connect_proxy,cloud_connect_timeout, andcloud_connect_recordingfrombrowser-use config(CLI default recording on; library default remains off); unified base viaBROWSER_USE_CLOUD_BASE_URLhost (CLI appends/api/{version});cloud signupto get/save API key; API key is read only from~/.browser-use/config.json(env var ignored) and forwarded toprofile-use; cloud profile ID is read from config without per‑call GET validation and auto‑heals on invalid/missing IDs (created only when needed), with daemon‑safe error handling; empty API key is treated as unset.browser-use config(set/get/list/unset) with secureconfig.json;doctorshows config state with docs link; installers createconfig.jsonand lite requirescurl; interactivesetup(--yesfor CI); daemon writes a phase‑aware state file, includes PID in pings, has idle timeout, cleans up orphaned daemons, supports cross‑platform shutdown with SIGTERM fallback, and avoids deleting sockets for live daemons with stale PID files.config.json; tolerate shutdown race after SIGTERM.Migration
--session <NAME>for isolation. Removed--agent,browser-use register, and tab‑ownership locking.tabsubcommands; standaloneswitchandclose-tabare removed.browser-use connectonce for local Chrome; the--connectflag is deprecated and now errors with guidance.cloud connectwith zero flags; customize viabrowser-use config(cloud_connect_proxy,cloud_connect_timeout,cloud_connect_recording); setBROWSER_USE_CLOUD_BASE_URLto the host (CLI appends/api/{version}); usecloud signupto obtain and save an API key; set the API key viabrowser-use config set api_key <KEY>—BROWSER_USE_API_KEYis ignored by the CLI.Written for commit a7b476e. Summary will update on new commits.