feat: added cloud_browser feature#2
feat: added cloud_browser feature#2cubic-dev-local[bot] wants to merge 8 commits intoeval-pr-3139-target-1762452059715from
Conversation
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Agent Task Evaluation Results: 0/2 (0%)View detailed results
Check the evaluate-tasks job for detailed task execution logs. |
![cubic-dev-local[bot]](https://avatars.githubusercontent.com/u/189440881?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 10 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:64">
API token retrieval logic duplicates within CloudBrowserClient. This authentication pattern should be centralized into a single helper method (e.g., `_get_api_token`) within the `CloudBrowserClient` class to ensure consistency and easier maintenance of authentication logic.</violation>
</file>
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:847">
When stopping a cloud session we call `cleanup_cloud_client()` but leave `browser_profile.cdp_url` populated. Because `reset()` only clears the CDP URL for local sessions, the stale cloud URL persists and the next start skips provisioning, trying to reuse a terminated session. Please clear the CDP URL after cleaning up the cloud client so a fresh browser is provisioned on restart.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| url = f'{self.api_base_url}/api/v2/browsers' | ||
|
|
||
| # Try to get API key from environment variable first, then auth config | ||
| api_token = os.getenv('BROWSER_USE_API_KEY') |
There was a problem hiding this comment.
API token retrieval logic duplicates within CloudBrowserClient. This authentication pattern should be centralized into a single helper method (e.g., _get_api_token) within the CloudBrowserClient class to ensure consistency and easier maintenance of authentication logic.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 64:
<comment>API token retrieval logic duplicates within CloudBrowserClient. This authentication pattern should be centralized into a single helper method (e.g., `_get_api_token`) within the `CloudBrowserClient` class to ensure consistency and easier maintenance of authentication logic.</comment>
<file context>
@@ -0,0 +1,286 @@
+ url = f'{self.api_base_url}/api/v2/browsers'
+
+ # Try to get API key from environment variable first, then auth config
+ api_token = os.getenv('BROWSER_USE_API_KEY')
+
+ if not api_token:
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Duplicate Detection Agent
| try: | ||
| from browser_use.browser.cloud import cleanup_cloud_client | ||
|
|
||
| await cleanup_cloud_client() |
There was a problem hiding this comment.
When stopping a cloud session we call cleanup_cloud_client() but leave browser_profile.cdp_url populated. Because reset() only clears the CDP URL for local sessions, the stale cloud URL persists and the next start skips provisioning, trying to reuse a terminated session. Please clear the CDP URL after cleaning up the cloud client so a fresh browser is provisioned on restart.
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 847:
<comment>When stopping a cloud session we call `cleanup_cloud_client()` but leave `browser_profile.cdp_url` populated. Because `reset()` only clears the CDP URL for local sessions, the stale cloud URL persists and the next start skips provisioning, trying to reuse a terminated session. Please clear the CDP URL after cleaning up the cloud client so a fresh browser is provisioned on restart.</comment>
<file context>
@@ -813,6 +839,16 @@ async def on_BrowserStopEvent(self, event: BrowserStopEvent) -> None:
+ try:
+ from browser_use.browser.cloud import cleanup_cloud_client
+
+ await cleanup_cloud_client()
+ self.logger.info('🌤️ Cloud browser session cleaned up')
+ except Exception as e:
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 10 files
Prompt for AI agents (all 6 issues)
Understand the root cause of the following 6 issues and fix them.
<file name="browser_use/dom/playground/extraction.py">
<violation number="1" location="browser_use/dom/playground/extraction.py:179">
`tiktoken.encoding_for_model` cannot resolve `'gpt-4.1-mini'` with the current tiktoken version, raising a KeyError and aborting the extraction loop. Consider using a supported encoding (e.g. `get_encoding(...)`) or adding fallback logic so the script continues to run.
• **Exa queries**: "tiktoken encoding_for_model gpt-4.1-mini latest version support", ""gpt-4.1-mini" tiktoken encoding_for_model", "tiktoken model mapping list includes gpt-4.1"</violation>
</file>
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:18">
Sensitive CDP URL from cloud flow will be logged/propagated, exposing tokenized WebSocket endpoint (session hijack risk).</violation>
<violation number="2" location="browser_use/browser/session.py:287">
When both `use_cloud` and the legacy `cloud_browser` are provided, this assignment overwrites the caller's explicit `use_cloud` value with the legacy one, so an intentional `use_cloud=False` can be flipped to True. Only copy the legacy flag when the new field is absent to avoid breaking explicit configuration.</violation>
</file>
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:64">
The method for managing the cloud browser API key is inconsistent with the established configuration pattern for other credentials in the project. This new feature introduces a separate configuration file (`cloud_auth.json`) and also checks for an environment variable, fragmenting the configuration and making it harder to manage.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: functionalImpact.*</violation>
<violation number="2" location="browser_use/browser/cloud.py:227">
The use of a global `_cloud_client` with a mutable `current_session_id` is not safe for concurrency. If multiple cloud sessions are initiated in parallel, session IDs will be overwritten, causing all but the last session to be orphaned in the cloud without proper cleanup.</violation>
</file>
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:253">
The test awaits the patched mock directly, so it never calls the real cloud-browser code and cannot catch regressions. Invoke the actual function (patching the symbol the code under test resolves) instead of the mock so the integration is exercised.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| # pyperclip.copy(text_to_save) | ||
|
|
||
| encoding = tiktoken.encoding_for_model('gpt-4o') | ||
| encoding = tiktoken.encoding_for_model('gpt-4.1-mini') |
There was a problem hiding this comment.
tiktoken.encoding_for_model cannot resolve 'gpt-4.1-mini' with the current tiktoken version, raising a KeyError and aborting the extraction loop. Consider using a supported encoding (e.g. get_encoding(...)) or adding fallback logic so the script continues to run.
• Exa queries: "tiktoken encoding_for_model gpt-4.1-mini latest version support", ""gpt-4.1-mini" tiktoken encoding_for_model", "tiktoken model mapping list includes gpt-4.1"
Prompt for AI agents
Address the following comment on browser_use/dom/playground/extraction.py at line 179:
<comment>`tiktoken.encoding_for_model` cannot resolve `'gpt-4.1-mini'` with the current tiktoken version, raising a KeyError and aborting the extraction loop. Consider using a supported encoding (e.g. `get_encoding(...)`) or adding fallback logic so the script continues to run.
• **Exa queries**: "tiktoken encoding_for_model gpt-4.1-mini latest version support", ""gpt-4.1-mini" tiktoken encoding_for_model", "tiktoken model mapping list includes gpt-4.1"</comment>
<file context>
@@ -176,7 +176,7 @@ def get_website_list_for_prompt() -> str:
# pyperclip.copy(text_to_save)
- encoding = tiktoken.encoding_for_model('gpt-4o')
+ encoding = tiktoken.encoding_for_model('gpt-4.1-mini')
token_count = len(encoding.encode(text_to_save))
print(f'Token count: {token_count}')
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| from pydantic import BaseModel, ConfigDict, Field, PrivateAttr | ||
| from uuid_extensions import uuid7str | ||
|
|
||
| from browser_use.browser.cloud import CloudBrowserAuthError, CloudBrowserError, get_cloud_browser_cdp_url |
There was a problem hiding this comment.
Sensitive CDP URL from cloud flow will be logged/propagated, exposing tokenized WebSocket endpoint (session hijack risk).
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 18:
<comment>Sensitive CDP URL from cloud flow will be logged/propagated, exposing tokenized WebSocket endpoint (session hijack risk).</comment>
<file context>
@@ -15,6 +15,8 @@
from pydantic import BaseModel, ConfigDict, Field, PrivateAttr
from uuid_extensions import uuid7str
+from browser_use.browser.cloud import CloudBrowserAuthError, CloudBrowserError, get_cloud_browser_cdp_url
+
# CDP logging is now handled by setup_logging() in logging_config.py
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Security Vulnerabilities Agent
|
|
||
| # Handle backward compatibility: map cloud_browser to use_cloud | ||
| if 'cloud_browser' in profile_kwargs: | ||
| profile_kwargs['use_cloud'] = profile_kwargs.pop('cloud_browser') |
There was a problem hiding this comment.
When both use_cloud and the legacy cloud_browser are provided, this assignment overwrites the caller's explicit use_cloud value with the legacy one, so an intentional use_cloud=False can be flipped to True. Only copy the legacy flag when the new field is absent to avoid breaking explicit configuration.
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 287:
<comment>When both `use_cloud` and the legacy `cloud_browser` are provided, this assignment overwrites the caller's explicit `use_cloud` value with the legacy one, so an intentional `use_cloud=False` can be flipped to True. Only copy the legacy flag when the new field is absent to avoid breaking explicit configuration.</comment>
<file context>
@@ -278,6 +282,10 @@ def __init__(
+ # Handle backward compatibility: map cloud_browser to use_cloud
+ if 'cloud_browser' in profile_kwargs:
+ profile_kwargs['use_cloud'] = profile_kwargs.pop('cloud_browser')
+
# if is_local is False but executable_path is provided, set is_local to True
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| profile_kwargs['use_cloud'] = profile_kwargs.pop('cloud_browser') | |
| profile_kwargs.setdefault('use_cloud', profile_kwargs.pop('cloud_browser')) |
|
|
||
|
|
||
| # Global client instance | ||
| _cloud_client: CloudBrowserClient | None = None |
There was a problem hiding this comment.
The use of a global _cloud_client with a mutable current_session_id is not safe for concurrency. If multiple cloud sessions are initiated in parallel, session IDs will be overwritten, causing all but the last session to be orphaned in the cloud without proper cleanup.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 227:
<comment>The use of a global `_cloud_client` with a mutable `current_session_id` is not safe for concurrency. If multiple cloud sessions are initiated in parallel, session IDs will be overwritten, causing all but the last session to be orphaned in the cloud without proper cleanup.</comment>
<file context>
@@ -0,0 +1,286 @@
+
+
+# Global client instance
+_cloud_client: CloudBrowserClient | None = None
+
+
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
| url = f'{self.api_base_url}/api/v2/browsers' | ||
|
|
||
| # Try to get API key from environment variable first, then auth config | ||
| api_token = os.getenv('BROWSER_USE_API_KEY') |
There was a problem hiding this comment.
The method for managing the cloud browser API key is inconsistent with the established configuration pattern for other credentials in the project. This new feature introduces a separate configuration file (cloud_auth.json) and also checks for an environment variable, fragmenting the configuration and making it harder to manage.
DEV MODE: This violation would have been filtered out by screening filters. Failing filters: functionalImpact.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 64:
<comment>The method for managing the cloud browser API key is inconsistent with the established configuration pattern for other credentials in the project. This new feature introduces a separate configuration file (`cloud_auth.json`) and also checks for an environment variable, fragmenting the configuration and making it harder to manage.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: functionalImpact.*</comment>
<file context>
@@ -0,0 +1,286 @@
+ url = f'{self.api_base_url}/api/v2/browsers'
+
+ # Try to get API key from environment variable first, then auth config
+ api_token = os.getenv('BROWSER_USE_API_KEY')
+
+ if not api_token:
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: System Design Agent
| with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_get_cdp_url: | ||
| mock_get_cdp_url.return_value = 'wss://test.proxy.daytona.works' | ||
|
|
||
| cdp_url = await mock_get_cdp_url() |
There was a problem hiding this comment.
The test awaits the patched mock directly, so it never calls the real cloud-browser code and cannot catch regressions. Invoke the actual function (patching the symbol the code under test resolves) instead of the mock so the integration is exercised.
Prompt for AI agents
Address the following comment on tests/ci/test_cloud_browser.py at line 253:
<comment>The test awaits the patched mock directly, so it never calls the real cloud-browser code and cannot catch regressions. Invoke the actual function (patching the symbol the code under test resolves) instead of the mock so the integration is exercised.</comment>
<file context>
@@ -0,0 +1,333 @@
+ with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_get_cdp_url:
+ mock_get_cdp_url.return_value = 'wss://test.proxy.daytona.works'
+
+ cdp_url = await mock_get_cdp_url()
+ assert cdp_url == 'wss://test.proxy.daytona.works'
+ mock_get_cdp_url.assert_called_once()
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
There was a problem hiding this comment.
4 issues found across 10 files
Prompt for AI agents (all 4 issues)
Understand the root cause of the following 4 issues and fix them.
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:64">
API token retrieval logic is duplicated within the `CloudBrowserClient` class. This critical authentication pattern should be extracted into a shared helper method.</violation>
<violation number="2" location="browser_use/browser/cloud.py:227">
The use of a global singleton for `CloudBrowserClient` introduces a race condition when multiple cloud-based `BrowserSession` instances are used concurrently, leading to incorrect session cleanup and potential resource leaks. The client's lifecycle should be tied to the `BrowserSession` instance itself, not managed globally.</violation>
</file>
<file name="browser_use/dom/playground/extraction.py">
<violation number="1" location="browser_use/dom/playground/extraction.py:179">
`tiktoken.encoding_for_model('gpt-4.1-mini')` raises a `KeyError` because this model name is not mapped in tiktoken, so the playground will crash when this line executes. Please keep using the supported model identifier (e.g., `gpt-4o`) until tiktoken exposes a mapping for the new model.</violation>
</file>
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:847">
After cleaning up the cloud session we need to unset the stored CDP URL; otherwise the next start skips provisioning and tries to reuse a terminated cloud connection.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| url = f'{self.api_base_url}/api/v2/browsers' | ||
|
|
||
| # Try to get API key from environment variable first, then auth config | ||
| api_token = os.getenv('BROWSER_USE_API_KEY') |
There was a problem hiding this comment.
API token retrieval logic is duplicated within the CloudBrowserClient class. This critical authentication pattern should be extracted into a shared helper method.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 64:
<comment>API token retrieval logic is duplicated within the `CloudBrowserClient` class. This critical authentication pattern should be extracted into a shared helper method.</comment>
<file context>
@@ -0,0 +1,286 @@
+ url = f'{self.api_base_url}/api/v2/browsers'
+
+ # Try to get API key from environment variable first, then auth config
+ api_token = os.getenv('BROWSER_USE_API_KEY')
+
+ if not api_token:
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: Duplicate Detection Agent
| # pyperclip.copy(text_to_save) | ||
|
|
||
| encoding = tiktoken.encoding_for_model('gpt-4o') | ||
| encoding = tiktoken.encoding_for_model('gpt-4.1-mini') |
There was a problem hiding this comment.
tiktoken.encoding_for_model('gpt-4.1-mini') raises a KeyError because this model name is not mapped in tiktoken, so the playground will crash when this line executes. Please keep using the supported model identifier (e.g., gpt-4o) until tiktoken exposes a mapping for the new model.
Prompt for AI agents
Address the following comment on browser_use/dom/playground/extraction.py at line 179:
<comment>`tiktoken.encoding_for_model('gpt-4.1-mini')` raises a `KeyError` because this model name is not mapped in tiktoken, so the playground will crash when this line executes. Please keep using the supported model identifier (e.g., `gpt-4o`) until tiktoken exposes a mapping for the new model.</comment>
<file context>
@@ -176,7 +176,7 @@ def get_website_list_for_prompt() -> str:
# pyperclip.copy(text_to_save)
- encoding = tiktoken.encoding_for_model('gpt-4o')
+ encoding = tiktoken.encoding_for_model('gpt-4.1-mini')
token_count = len(encoding.encode(text_to_save))
print(f'Token count: {token_count}')
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
|
|
||
|
|
||
| # Global client instance | ||
| _cloud_client: CloudBrowserClient | None = None |
There was a problem hiding this comment.
The use of a global singleton for CloudBrowserClient introduces a race condition when multiple cloud-based BrowserSession instances are used concurrently, leading to incorrect session cleanup and potential resource leaks. The client's lifecycle should be tied to the BrowserSession instance itself, not managed globally.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 227:
<comment>The use of a global singleton for `CloudBrowserClient` introduces a race condition when multiple cloud-based `BrowserSession` instances are used concurrently, leading to incorrect session cleanup and potential resource leaks. The client's lifecycle should be tied to the `BrowserSession` instance itself, not managed globally.</comment>
<file context>
@@ -0,0 +1,286 @@
+
+
+# Global client instance
+_cloud_client: CloudBrowserClient | None = None
+
+
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
| try: | ||
| from browser_use.browser.cloud import cleanup_cloud_client | ||
|
|
||
| await cleanup_cloud_client() |
There was a problem hiding this comment.
After cleaning up the cloud session we need to unset the stored CDP URL; otherwise the next start skips provisioning and tries to reuse a terminated cloud connection.
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 847:
<comment>After cleaning up the cloud session we need to unset the stored CDP URL; otherwise the next start skips provisioning and tries to reuse a terminated cloud connection.</comment>
<file context>
@@ -813,6 +839,16 @@ async def on_BrowserStopEvent(self, event: BrowserStopEvent) -> None:
+ try:
+ from browser_use.browser.cloud import cleanup_cloud_client
+
+ await cleanup_cloud_client()
+ self.logger.info('🌤️ Cloud browser session cleaned up')
+ except Exception as e:
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
There was a problem hiding this comment.
4 issues found across 10 files
Prompt for AI agents (all 4 issues)
Understand the root cause of the following 4 issues and fix them.
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:64">
API token retrieval logic duplicates itself within the CloudBrowserClient class in browser_use/browser/cloud.py. This critical authentication pattern should be centralized into a shared helper method.</violation>
<violation number="2" location="browser_use/browser/cloud.py:247">
Returning only the CDP URL drops the session id, so creating multiple cloud sessions before stopping leaks earlier sessions because they can no longer be stopped.</violation>
</file>
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:294">
This mocked auth error message never contains the substring the assertion expects, so the test always fails. Update the side effect to reuse the real no-token error string emitted by CloudBrowserClient.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The test calls the imported `get_cloud_browser_cdp_url()` directly, so the patched mock (and its `'No auth token'` message) is never triggered. The real function raises `CloudBrowserAuthError` with the message "No authentication token found. Please set BROWSER_USE_API_KEY environment variable..." (see browser_use/browser/cloud.py lines 76 & 160), so the substring assertion passes.</violation>
<violation number="2" location="tests/ci/test_cloud_browser.py:330">
Because `_cloud_client` persists from the previous test, this call reuses a stale client whose `patch` coroutine was never configured, so `stop_browser` raises `CloudBrowserError` rather than returning the mocked payload. Reset the global client (e.g., via `cleanup_cloud_client`) before invoking the helper.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: After the auth failure test runs, `get_cloud_browser_cdp_url()` catches the exception, invokes `await _cloud_client.close()`, and resets `_cloud_client = None` (browser_use/browser/cloud.py lines ~236-252). Consequently, `stop_cloud_browser_session()` instantiates a fresh `CloudBrowserClient` inside the patched `httpx.AsyncClient` block, so the stale-client scenario cannot occur.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| url = f'{self.api_base_url}/api/v2/browsers' | ||
|
|
||
| # Try to get API key from environment variable first, then auth config | ||
| api_token = os.getenv('BROWSER_USE_API_KEY') |
There was a problem hiding this comment.
API token retrieval logic duplicates itself within the CloudBrowserClient class in browser_use/browser/cloud.py. This critical authentication pattern should be centralized into a shared helper method.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 64:
<comment>API token retrieval logic duplicates itself within the CloudBrowserClient class in browser_use/browser/cloud.py. This critical authentication pattern should be centralized into a shared helper method.</comment>
<file context>
@@ -0,0 +1,286 @@
+ url = f'{self.api_base_url}/api/v2/browsers'
+
+ # Try to get API key from environment variable first, then auth config
+ api_token = os.getenv('BROWSER_USE_API_KEY')
+
+ if not api_token:
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Duplicate Detection Agent
| mock_client.patch.return_value = mock_response | ||
| mock_client_class.return_value = mock_client | ||
|
|
||
| result = await stop_cloud_browser_session('test-browser-id') |
There was a problem hiding this comment.
Because _cloud_client persists from the previous test, this call reuses a stale client whose patch coroutine was never configured, so stop_browser raises CloudBrowserError rather than returning the mocked payload. Reset the global client (e.g., via cleanup_cloud_client) before invoking the helper.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: After the auth failure test runs, get_cloud_browser_cdp_url() catches the exception, invokes await _cloud_client.close(), and resets _cloud_client = None (browser_use/browser/cloud.py lines ~236-252). Consequently, stop_cloud_browser_session() instantiates a fresh CloudBrowserClient inside the patched httpx.AsyncClient block, so the stale-client scenario cannot occur.
Prompt for AI agents
Address the following comment on tests/ci/test_cloud_browser.py at line 330:
<comment>Because `_cloud_client` persists from the previous test, this call reuses a stale client whose `patch` coroutine was never configured, so `stop_browser` raises `CloudBrowserError` rather than returning the mocked payload. Reset the global client (e.g., via `cleanup_cloud_client`) before invoking the helper.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: After the auth failure test runs, `get_cloud_browser_cdp_url()` catches the exception, invokes `await _cloud_client.close()`, and resets `_cloud_client = None` (browser_use/browser/cloud.py lines ~236-252). Consequently, `stop_cloud_browser_session()` instantiates a fresh `CloudBrowserClient` inside the patched `httpx.AsyncClient` block, so the stale-client scenario cannot occur.</comment>
<file context>
@@ -0,0 +1,333 @@
+ mock_client.patch.return_value = mock_response
+ mock_client_class.return_value = mock_client
+
+ result = await stop_cloud_browser_session('test-browser-id')
+
+ assert result.id == 'test-browser-id'
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
|
|
||
| # Test that cloud browser client raises error without fallback | ||
| with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_cloud_cdp: | ||
| mock_cloud_cdp.side_effect = CloudBrowserAuthError('No auth token') |
There was a problem hiding this comment.
This mocked auth error message never contains the substring the assertion expects, so the test always fails. Update the side effect to reuse the real no-token error string emitted by CloudBrowserClient.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: The test calls the imported get_cloud_browser_cdp_url() directly, so the patched mock (and its 'No auth token' message) is never triggered. The real function raises CloudBrowserAuthError with the message "No authentication token found. Please set BROWSER_USE_API_KEY environment variable..." (see browser_use/browser/cloud.py lines 76 & 160), so the substring assertion passes.
Prompt for AI agents
Address the following comment on tests/ci/test_cloud_browser.py at line 294:
<comment>This mocked auth error message never contains the substring the assertion expects, so the test always fails. Update the side effect to reuse the real no-token error string emitted by CloudBrowserClient.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The test calls the imported `get_cloud_browser_cdp_url()` directly, so the patched mock (and its `'No auth token'` message) is never triggered. The real function raises `CloudBrowserAuthError` with the message "No authentication token found. Please set BROWSER_USE_API_KEY environment variable..." (see browser_use/browser/cloud.py lines 76 & 160), so the substring assertion passes.</comment>
<file context>
@@ -0,0 +1,333 @@
+
+ # Test that cloud browser client raises error without fallback
+ with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_cloud_cdp:
+ mock_cloud_cdp.side_effect = CloudBrowserAuthError('No auth token')
+
+ # Verify that the cloud browser client raises the expected error
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| mock_cloud_cdp.side_effect = CloudBrowserAuthError('No auth token') | |
| mock_cloud_cdp.side_effect = CloudBrowserAuthError('No authentication token found. Please set BROWSER_USE_API_KEY environment variable to authenticate with the cloud service.') |
|
|
||
| try: | ||
| browser_response = await _cloud_client.create_browser() | ||
| return browser_response.cdpUrl |
There was a problem hiding this comment.
Returning only the CDP URL drops the session id, so creating multiple cloud sessions before stopping leaks earlier sessions because they can no longer be stopped.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 247:
<comment>Returning only the CDP URL drops the session id, so creating multiple cloud sessions before stopping leaks earlier sessions because they can no longer be stopped.</comment>
<file context>
@@ -0,0 +1,286 @@
+
+ try:
+ browser_response = await _cloud_client.create_browser()
+ return browser_response.cdpUrl
+ except Exception:
+ # Clean up client on error
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Functional Bugs Agent
There was a problem hiding this comment.
4 issues found across 10 files
Prompt for AI agents (all 4 issues)
Understand the root cause of the following 4 issues and fix them.
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:847">
After cleaning up the cloud browser we leave `browser_profile.cdp_url` pointing at the expired CDP endpoint, so the next start skips provisioning and tries to reconnect to a dead websocket. Please clear the CDP URL when shutting down cloud sessions.</violation>
</file>
<file name="browser_use/dom/playground/extraction.py">
<violation number="1" location="browser_use/dom/playground/extraction.py:179">
`tiktoken.encoding_for_model('gpt-4.1-mini')` raises a `KeyError` because tiktoken does not yet map that model to an encoding, so this change will crash before printing the token count. Please keep using a supported model mapping or switch to `get_encoding` with a valid tokenizer.</violation>
</file>
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:227">
The use of a global stateful client for managing cloud browsers is not thread-safe and will cause race conditions. If multiple cloud `BrowserSession`s are active concurrently, stopping one will terminate another's browser instance, and will also cause resource leaks.</violation>
</file>
<file name="browser_use/browser/profile.py">
<violation number="1" location="browser_use/browser/profile.py:555">
The default behavior of opting into a paid cloud service implicitly based on an environment variable is a poor design choice. It can lead to unexpected behavior and costs for users. Opt-in for paid services should always be explicit (e.g., `default=False`).</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| try: | ||
| from browser_use.browser.cloud import cleanup_cloud_client | ||
|
|
||
| await cleanup_cloud_client() |
There was a problem hiding this comment.
After cleaning up the cloud browser we leave browser_profile.cdp_url pointing at the expired CDP endpoint, so the next start skips provisioning and tries to reconnect to a dead websocket. Please clear the CDP URL when shutting down cloud sessions.
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 847:
<comment>After cleaning up the cloud browser we leave `browser_profile.cdp_url` pointing at the expired CDP endpoint, so the next start skips provisioning and tries to reconnect to a dead websocket. Please clear the CDP URL when shutting down cloud sessions.</comment>
<file context>
@@ -813,6 +839,16 @@ async def on_BrowserStopEvent(self, event: BrowserStopEvent) -> None:
+ try:
+ from browser_use.browser.cloud import cleanup_cloud_client
+
+ await cleanup_cloud_client()
+ self.logger.info('🌤️ Cloud browser session cleaned up')
+ except Exception as e:
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| # pyperclip.copy(text_to_save) | ||
|
|
||
| encoding = tiktoken.encoding_for_model('gpt-4o') | ||
| encoding = tiktoken.encoding_for_model('gpt-4.1-mini') |
There was a problem hiding this comment.
tiktoken.encoding_for_model('gpt-4.1-mini') raises a KeyError because tiktoken does not yet map that model to an encoding, so this change will crash before printing the token count. Please keep using a supported model mapping or switch to get_encoding with a valid tokenizer.
Prompt for AI agents
Address the following comment on browser_use/dom/playground/extraction.py at line 179:
<comment>`tiktoken.encoding_for_model('gpt-4.1-mini')` raises a `KeyError` because tiktoken does not yet map that model to an encoding, so this change will crash before printing the token count. Please keep using a supported model mapping or switch to `get_encoding` with a valid tokenizer.</comment>
<file context>
@@ -176,7 +176,7 @@ def get_website_list_for_prompt() -> str:
# pyperclip.copy(text_to_save)
- encoding = tiktoken.encoding_for_model('gpt-4o')
+ encoding = tiktoken.encoding_for_model('gpt-4.1-mini')
token_count = len(encoding.encode(text_to_save))
print(f'Token count: {token_count}')
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
|
|
||
|
|
||
| # Global client instance | ||
| _cloud_client: CloudBrowserClient | None = None |
There was a problem hiding this comment.
The use of a global stateful client for managing cloud browsers is not thread-safe and will cause race conditions. If multiple cloud BrowserSessions are active concurrently, stopping one will terminate another's browser instance, and will also cause resource leaks.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 227:
<comment>The use of a global stateful client for managing cloud browsers is not thread-safe and will cause race conditions. If multiple cloud `BrowserSession`s are active concurrently, stopping one will terminate another's browser instance, and will also cause resource leaks.</comment>
<file context>
@@ -0,0 +1,286 @@
+
+
+# Global client instance
+_cloud_client: CloudBrowserClient | None = None
+
+
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
| is_local: bool = Field(default=False, description='Whether this is a local browser instance') | ||
| # label: str = 'default' | ||
| use_cloud: bool = Field( | ||
| default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')), |
There was a problem hiding this comment.
The default behavior of opting into a paid cloud service implicitly based on an environment variable is a poor design choice. It can lead to unexpected behavior and costs for users. Opt-in for paid services should always be explicit (e.g., default=False).
Prompt for AI agents
Address the following comment on browser_use/browser/profile.py at line 555:
<comment>The default behavior of opting into a paid cloud service implicitly based on an environment variable is a poor design choice. It can lead to unexpected behavior and costs for users. Opt-in for paid services should always be explicit (e.g., `default=False`).</comment>
<file context>
@@ -550,7 +551,15 @@ class BrowserProfile(BrowserConnectArgs, BrowserLaunchPersistentContextArgs, Bro
is_local: bool = Field(default=False, description='Whether this is a local browser instance')
- # label: str = 'default'
+ use_cloud: bool = Field(
+ default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')),
+ description='Use browser-use cloud browser service instead of local browser',
+ )
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: System Design Agent
There was a problem hiding this comment.
5 issues found across 10 files
Prompt for AI agents (all 5 issues)
Understand the root cause of the following 5 issues and fix them.
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:75">
API token retrieval and validation logic is duplicated between `create_browser` and `stop_browser` in `browser_use/browser/cloud.py`. This impacts consistent authentication behavior and maintainability.</violation>
<violation number="2" location="browser_use/browser/cloud.py:112">
Sensitive data exposure: cdpUrl logged at debug allows session hijacking via logs.</violation>
<violation number="3" location="browser_use/browser/cloud.py:227">
The use of a global `_cloud_client` instance to manage stateful cloud browser sessions is a significant architectural flaw. This will cause race conditions and incorrect behavior when multiple cloud browser sessions are used concurrently, as they will all share and overwrite the same client state (`current_session_id`). The client's lifecycle should be tied to the `BrowserSession`.</violation>
</file>
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:298">
This await still calls the original `get_cloud_browser_cdp_url` function, so the preceding patch never takes effect and the test exercises the real implementation rather than the mocked auth failure scenario.</violation>
</file>
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:286">
The alias handling always overwrites an explicitly provided `use_cloud` value with `cloud_browser`, so passing both options can flip the cloud setting unexpectedly. Please only map the alias when `use_cloud` isn't already supplied and still drop the legacy key.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| pass | ||
|
|
||
| if not api_token: | ||
| raise CloudBrowserAuthError( |
There was a problem hiding this comment.
API token retrieval and validation logic is duplicated between create_browser and stop_browser in browser_use/browser/cloud.py. This impacts consistent authentication behavior and maintainability.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 75:
<comment>API token retrieval and validation logic is duplicated between `create_browser` and `stop_browser` in `browser_use/browser/cloud.py`. This impacts consistent authentication behavior and maintainability.</comment>
<file context>
@@ -0,0 +1,286 @@
+ pass
+
+ if not api_token:
+ raise CloudBrowserAuthError(
+ 'No authentication token found. Please set BROWSER_USE_API_KEY environment variable to authenticate with the cloud service.'
+ )
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: Duplicate Detection Agent
|
|
||
| # Verify that the cloud browser client raises the expected error | ||
| with pytest.raises(CloudBrowserAuthError) as exc_info: | ||
| await get_cloud_browser_cdp_url() |
There was a problem hiding this comment.
This await still calls the original get_cloud_browser_cdp_url function, so the preceding patch never takes effect and the test exercises the real implementation rather than the mocked auth failure scenario.
Prompt for AI agents
Address the following comment on tests/ci/test_cloud_browser.py at line 298:
<comment>This await still calls the original `get_cloud_browser_cdp_url` function, so the preceding patch never takes effect and the test exercises the real implementation rather than the mocked auth failure scenario.</comment>
<file context>
@@ -0,0 +1,333 @@
+
+ # Verify that the cloud browser client raises the expected error
+ with pytest.raises(CloudBrowserAuthError) as exc_info:
+ await get_cloud_browser_cdp_url()
+
+ assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value)
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| self.current_session_id = browser_response.id | ||
|
|
||
| logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}') | ||
| logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}') |
There was a problem hiding this comment.
Sensitive data exposure: cdpUrl logged at debug allows session hijacking via logs.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 112:
<comment>Sensitive data exposure: cdpUrl logged at debug allows session hijacking via logs.</comment>
<file context>
@@ -0,0 +1,286 @@
+ self.current_session_id = browser_response.id
+
+ logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}')
+ logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}')
+ logger.debug(f'🌤️ Live URL: {browser_response.liveUrl}')
+
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Security Vulnerabilities Agent
| logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}') | |
| logger.debug('🌤️ CDP URL obtained') |
|
|
||
|
|
||
| # Global client instance | ||
| _cloud_client: CloudBrowserClient | None = None |
There was a problem hiding this comment.
The use of a global _cloud_client instance to manage stateful cloud browser sessions is a significant architectural flaw. This will cause race conditions and incorrect behavior when multiple cloud browser sessions are used concurrently, as they will all share and overwrite the same client state (current_session_id). The client's lifecycle should be tied to the BrowserSession.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 227:
<comment>The use of a global `_cloud_client` instance to manage stateful cloud browser sessions is a significant architectural flaw. This will cause race conditions and incorrect behavior when multiple cloud browser sessions are used concurrently, as they will all share and overwrite the same client state (`current_session_id`). The client's lifecycle should be tied to the `BrowserSession`.</comment>
<file context>
@@ -0,0 +1,286 @@
+
+
+# Global client instance
+_cloud_client: CloudBrowserClient | None = None
+
+
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
| profile_kwargs = {k: v for k, v in locals().items() if k not in ['self', 'browser_profile', 'id'] and v is not None} | ||
|
|
||
| # Handle backward compatibility: map cloud_browser to use_cloud | ||
| if 'cloud_browser' in profile_kwargs: |
There was a problem hiding this comment.
The alias handling always overwrites an explicitly provided use_cloud value with cloud_browser, so passing both options can flip the cloud setting unexpectedly. Please only map the alias when use_cloud isn't already supplied and still drop the legacy key.
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 286:
<comment>The alias handling always overwrites an explicitly provided `use_cloud` value with `cloud_browser`, so passing both options can flip the cloud setting unexpectedly. Please only map the alias when `use_cloud` isn't already supplied and still drop the legacy key.</comment>
<file context>
@@ -278,6 +282,10 @@ def __init__(
profile_kwargs = {k: v for k, v in locals().items() if k not in ['self', 'browser_profile', 'id'] and v is not None}
+ # Handle backward compatibility: map cloud_browser to use_cloud
+ if 'cloud_browser' in profile_kwargs:
+ profile_kwargs['use_cloud'] = profile_kwargs.pop('cloud_browser')
+
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
There was a problem hiding this comment.
7 issues found across 10 files
Prompt for AI agents (all 7 issues)
Understand the root cause of the following 7 issues and fix them.
<file name="README.md">
<violation number="1" location="README.md:89">
Please adjust the sentence to clearly instruct users to set the environment variable, e.g., “First set the `BROWSER_USE_API_KEY` environment variable. You can get your API key from here.” This improves readability and reduces confusion.</violation>
</file>
<file name="browser_use/dom/playground/extraction.py">
<violation number="1" location="browser_use/dom/playground/extraction.py:179">
Using tiktoken.encoding_for_model with unsupported model name "gpt-4.1-mini" raises KeyError, crashing the script before token counting.</violation>
</file>
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:109">
Storing the cloud session id on the shared client means only the newest session is tracked. If two BrowserSession instances start cloud browsers, the second call overwrites `current_session_id`, so the first session’s cleanup stops the wrong browser (and the other session leaks). Please return/propagate the session id so each caller cleans up its own instance instead of relying on shared state.</violation>
</file>
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:287">
`cloud_browser` should only backfill `use_cloud` when the new parameter is absent; otherwise the explicit `use_cloud` value gets overwritten when both are provided.</violation>
<violation number="2" location="browser_use/browser/session.py:514">
After cleaning up a cloud browser session, the stored `browser_profile.cdp_url` must be cleared so the next start provisions a new cloud session instead of reusing a dead CDP URL.</violation>
</file>
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:100">
This assertion expects the mocked CloudBrowserAuthError to mention `BROWSER_USE_API_KEY`, but the mock is configured with the message 'No auth token', so the test will always fail. Update the expected message to match the mocked error.</violation>
</file>
<file name="browser_use/browser/profile.py">
<violation number="1" location="browser_use/browser/profile.py:560">
Removing the cloud_browser field means passing cloud_browser=True is ignored (extra fields are dropped), so cloud sessions never enable the cloud browser.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| ) | ||
| ``` | ||
|
|
||
| First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com). |
There was a problem hiding this comment.
Please adjust the sentence to clearly instruct users to set the environment variable, e.g., “First set the BROWSER_USE_API_KEY environment variable. You can get your API key from here.” This improves readability and reduces confusion.
Prompt for AI agents
Address the following comment on README.md at line 89:
<comment>Please adjust the sentence to clearly instruct users to set the environment variable, e.g., “First set the `BROWSER_USE_API_KEY` environment variable. You can get your API key from here.” This improves readability and reduces confusion.</comment>
<file context>
@@ -74,6 +74,22 @@ OPENAI_API_KEY=
+)
+```
+
+First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com).
+
+For other settings, models, and more, check out the [Cloud documentation 📕](https://docs.cloud.browser-use.com).
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com). | |
| First set the `BROWSER_USE_API_KEY` environment variable. You can get your API key from [here](https://cloud.browser-use.com). |
| # pyperclip.copy(text_to_save) | ||
|
|
||
| encoding = tiktoken.encoding_for_model('gpt-4o') | ||
| encoding = tiktoken.encoding_for_model('gpt-4.1-mini') |
There was a problem hiding this comment.
Using tiktoken.encoding_for_model with unsupported model name "gpt-4.1-mini" raises KeyError, crashing the script before token counting.
Prompt for AI agents
Address the following comment on browser_use/dom/playground/extraction.py at line 179:
<comment>Using tiktoken.encoding_for_model with unsupported model name "gpt-4.1-mini" raises KeyError, crashing the script before token counting.</comment>
<file context>
@@ -176,7 +176,7 @@ def get_website_list_for_prompt() -> str:
# pyperclip.copy(text_to_save)
- encoding = tiktoken.encoding_for_model('gpt-4o')
+ encoding = tiktoken.encoding_for_model('gpt-4.1-mini')
token_count = len(encoding.encode(text_to_save))
print(f'Token count: {token_count}')
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Functional Bugs Agent
| browser_response = CloudBrowserResponse(**browser_data) | ||
|
|
||
| # Store session ID for cleanup | ||
| self.current_session_id = browser_response.id |
There was a problem hiding this comment.
Storing the cloud session id on the shared client means only the newest session is tracked. If two BrowserSession instances start cloud browsers, the second call overwrites current_session_id, so the first session’s cleanup stops the wrong browser (and the other session leaks). Please return/propagate the session id so each caller cleans up its own instance instead of relying on shared state.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 109:
<comment>Storing the cloud session id on the shared client means only the newest session is tracked. If two BrowserSession instances start cloud browsers, the second call overwrites `current_session_id`, so the first session’s cleanup stops the wrong browser (and the other session leaks). Please return/propagate the session id so each caller cleans up its own instance instead of relying on shared state.</comment>
<file context>
@@ -0,0 +1,286 @@
+ browser_response = CloudBrowserResponse(**browser_data)
+
+ # Store session ID for cleanup
+ self.current_session_id = browser_response.id
+
+ logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}')
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| # If no CDP URL, launch local browser or cloud browser | ||
| if not self.cdp_url: | ||
| if self.is_local: | ||
| if self.browser_profile.use_cloud: |
There was a problem hiding this comment.
After cleaning up a cloud browser session, the stored browser_profile.cdp_url must be cleared so the next start provisions a new cloud session instead of reusing a dead CDP URL.
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 514:
<comment>After cleaning up a cloud browser session, the stored `browser_profile.cdp_url` must be cleared so the next start provisions a new cloud session instead of reusing a dead CDP URL.</comment>
<file context>
@@ -496,9 +509,22 @@ async def on_BrowserStartEvent(self, event: BrowserStartEvent) -> dict[str, str]
+ # If no CDP URL, launch local browser or cloud browser
if not self.cdp_url:
- if self.is_local:
+ if self.browser_profile.use_cloud:
+ # Use cloud browser service
+ try:
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
|
|
||
| # Handle backward compatibility: map cloud_browser to use_cloud | ||
| if 'cloud_browser' in profile_kwargs: | ||
| profile_kwargs['use_cloud'] = profile_kwargs.pop('cloud_browser') |
There was a problem hiding this comment.
cloud_browser should only backfill use_cloud when the new parameter is absent; otherwise the explicit use_cloud value gets overwritten when both are provided.
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 287:
<comment>`cloud_browser` should only backfill `use_cloud` when the new parameter is absent; otherwise the explicit `use_cloud` value gets overwritten when both are provided.</comment>
<file context>
@@ -278,6 +282,10 @@ def __init__(
+ # Handle backward compatibility: map cloud_browser to use_cloud
+ if 'cloud_browser' in profile_kwargs:
+ profile_kwargs['use_cloud'] = profile_kwargs.pop('cloud_browser')
+
# if is_local is False but executable_path is provided, set is_local to True
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| profile_kwargs['use_cloud'] = profile_kwargs.pop('cloud_browser') | |
| profile_kwargs.setdefault('use_cloud', profile_kwargs.pop('cloud_browser')) |
| with pytest.raises(CloudBrowserAuthError) as exc_info: | ||
| await client.create_browser() | ||
|
|
||
| assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value) |
There was a problem hiding this comment.
This assertion expects the mocked CloudBrowserAuthError to mention BROWSER_USE_API_KEY, but the mock is configured with the message 'No auth token', so the test will always fail. Update the expected message to match the mocked error.
Prompt for AI agents
Address the following comment on tests/ci/test_cloud_browser.py at line 100:
<comment>This assertion expects the mocked CloudBrowserAuthError to mention `BROWSER_USE_API_KEY`, but the mock is configured with the message 'No auth token', so the test will always fail. Update the expected message to match the mocked error.</comment>
<file context>
@@ -0,0 +1,333 @@
+ with pytest.raises(CloudBrowserAuthError) as exc_info:
+ await client.create_browser()
+
+ assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value)
+
+ async def test_create_browser_http_401(self, mock_auth_config):
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value) | |
| assert 'No auth token' in str(exc_info.value) |
| ) | ||
|
|
||
| @property | ||
| def cloud_browser(self) -> bool: |
There was a problem hiding this comment.
Removing the cloud_browser field means passing cloud_browser=True is ignored (extra fields are dropped), so cloud sessions never enable the cloud browser.
Prompt for AI agents
Address the following comment on browser_use/browser/profile.py at line 560:
<comment>Removing the cloud_browser field means passing cloud_browser=True is ignored (extra fields are dropped), so cloud sessions never enable the cloud browser.</comment>
<file context>
@@ -550,7 +551,15 @@ class BrowserProfile(BrowserConnectArgs, BrowserLaunchPersistentContextArgs, Bro
+ )
+
+ @property
+ def cloud_browser(self) -> bool:
+ """Alias for use_cloud field for compatibility."""
+ return self.use_cloud
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Functional Bugs Agent
There was a problem hiding this comment.
6 issues found across 10 files
Prompt for AI agents (all 6 issues)
Understand the root cause of the following 6 issues and fix them.
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:64">
Authentication token retrieval logic is duplicated within `CloudBrowserClient.create_browser()` and `CloudBrowserClient.stop_browser()`. This critical logic should be centralized to ensure consistent authentication and easier maintenance.</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:89">
The new sentence has a grammatical error (`Set` is capitalized mid-sentence and `the` is missing before the environment variable), which reduces clarity. Please rewrite it for proper grammar.</violation>
</file>
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:847">
The use of a stateful global client for managing cloud browser sessions introduces a critical architectural flaw. This design leads to race conditions and incorrect behavior when more than one cloud browser session is used concurrently, as stopping one session will incorrectly terminate another.</violation>
</file>
<file name="examples/browser/cloud_browser.py">
<violation number="1" location="examples/browser/cloud_browser.py:19">
The top-level `async def main():` line is indented with a leading tab, which causes `IndentationError: unexpected indent` when the script runs. Please remove the indentation so the function definition starts at column 0.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Inspected the file and confirmed the `async def main():` line starts at column 0 with no leading indentation. The reported tab character is not present in the actual code, so there is no syntax error or IndentationError risk. This violation is factually incorrect.</violation>
</file>
<file name="browser_use/dom/playground/extraction.py">
<violation number="1" location="browser_use/dom/playground/extraction.py:179">
Invalid model name passed to tiktoken.encoding_for_model causes a KeyError at runtime.</violation>
</file>
<file name="browser_use/browser/profile.py">
<violation number="1" location="browser_use/browser/profile.py:560">
Renaming `cloud_browser` to `use_cloud` needs a real alias. With `extra='ignore'`, `BrowserProfile(cloud_browser=True)` now silently drops the kwarg, so previous API usage no longer enables the cloud browser.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| url = f'{self.api_base_url}/api/v2/browsers' | ||
|
|
||
| # Try to get API key from environment variable first, then auth config | ||
| api_token = os.getenv('BROWSER_USE_API_KEY') |
There was a problem hiding this comment.
Authentication token retrieval logic is duplicated within CloudBrowserClient.create_browser() and CloudBrowserClient.stop_browser(). This critical logic should be centralized to ensure consistent authentication and easier maintenance.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 64:
<comment>Authentication token retrieval logic is duplicated within `CloudBrowserClient.create_browser()` and `CloudBrowserClient.stop_browser()`. This critical logic should be centralized to ensure consistent authentication and easier maintenance.</comment>
<file context>
@@ -0,0 +1,286 @@
+ url = f'{self.api_base_url}/api/v2/browsers'
+
+ # Try to get API key from environment variable first, then auth config
+ api_token = os.getenv('BROWSER_USE_API_KEY')
+
+ if not api_token:
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Duplicate Detection Agent
| ) | ||
| ``` | ||
|
|
||
| First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com). |
There was a problem hiding this comment.
The new sentence has a grammatical error (Set is capitalized mid-sentence and the is missing before the environment variable), which reduces clarity. Please rewrite it for proper grammar.
Prompt for AI agents
Address the following comment on README.md at line 89:
<comment>The new sentence has a grammatical error (`Set` is capitalized mid-sentence and `the` is missing before the environment variable), which reduces clarity. Please rewrite it for proper grammar.</comment>
<file context>
@@ -74,6 +74,22 @@ OPENAI_API_KEY=
+)
+```
+
+First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com).
+
+For other settings, models, and more, check out the [Cloud documentation 📕](https://docs.cloud.browser-use.com).
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com). | |
| First set the BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com). |
| try: | ||
| from browser_use.browser.cloud import cleanup_cloud_client | ||
|
|
||
| await cleanup_cloud_client() |
There was a problem hiding this comment.
The use of a stateful global client for managing cloud browser sessions introduces a critical architectural flaw. This design leads to race conditions and incorrect behavior when more than one cloud browser session is used concurrently, as stopping one session will incorrectly terminate another.
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 847:
<comment>The use of a stateful global client for managing cloud browser sessions introduces a critical architectural flaw. This design leads to race conditions and incorrect behavior when more than one cloud browser session is used concurrently, as stopping one session will incorrectly terminate another.</comment>
<file context>
@@ -813,6 +839,16 @@ async def on_BrowserStopEvent(self, event: BrowserStopEvent) -> None:
+ try:
+ from browser_use.browser.cloud import cleanup_cloud_client
+
+ await cleanup_cloud_client()
+ self.logger.info('🌤️ Cloud browser session cleaned up')
+ except Exception as e:
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
| from browser_use import Agent, Browser, ChatOpenAI | ||
|
|
||
|
|
||
| async def main(): |
There was a problem hiding this comment.
The top-level async def main(): line is indented with a leading tab, which causes IndentationError: unexpected indent when the script runs. Please remove the indentation so the function definition starts at column 0.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Inspected the file and confirmed the async def main(): line starts at column 0 with no leading indentation. The reported tab character is not present in the actual code, so there is no syntax error or IndentationError risk. This violation is factually incorrect.
Prompt for AI agents
Address the following comment on examples/browser/cloud_browser.py at line 19:
<comment>The top-level `async def main():` line is indented with a leading tab, which causes `IndentationError: unexpected indent` when the script runs. Please remove the indentation so the function definition starts at column 0.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Inspected the file and confirmed the `async def main():` line starts at column 0 with no leading indentation. The reported tab character is not present in the actual code, so there is no syntax error or IndentationError risk. This violation is factually incorrect.</comment>
<file context>
@@ -0,0 +1,41 @@
+from browser_use import Agent, Browser, ChatOpenAI
+
+
+async def main():
+ """Basic cloud browser example."""
+
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| async def main(): | |
| async def main(): |
| # pyperclip.copy(text_to_save) | ||
|
|
||
| encoding = tiktoken.encoding_for_model('gpt-4o') | ||
| encoding = tiktoken.encoding_for_model('gpt-4.1-mini') |
There was a problem hiding this comment.
Invalid model name passed to tiktoken.encoding_for_model causes a KeyError at runtime.
Prompt for AI agents
Address the following comment on browser_use/dom/playground/extraction.py at line 179:
<comment>Invalid model name passed to tiktoken.encoding_for_model causes a KeyError at runtime.</comment>
<file context>
@@ -176,7 +176,7 @@ def get_website_list_for_prompt() -> str:
# pyperclip.copy(text_to_save)
- encoding = tiktoken.encoding_for_model('gpt-4o')
+ encoding = tiktoken.encoding_for_model('gpt-4.1-mini')
token_count = len(encoding.encode(text_to_save))
print(f'Token count: {token_count}')
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Functional Bugs Agent
| ) | ||
|
|
||
| @property | ||
| def cloud_browser(self) -> bool: |
There was a problem hiding this comment.
Renaming cloud_browser to use_cloud needs a real alias. With extra='ignore', BrowserProfile(cloud_browser=True) now silently drops the kwarg, so previous API usage no longer enables the cloud browser.
Prompt for AI agents
Address the following comment on browser_use/browser/profile.py at line 560:
<comment>Renaming `cloud_browser` to `use_cloud` needs a real alias. With `extra='ignore'`, `BrowserProfile(cloud_browser=True)` now silently drops the kwarg, so previous API usage no longer enables the cloud browser.</comment>
<file context>
@@ -550,7 +551,15 @@ class BrowserProfile(BrowserConnectArgs, BrowserLaunchPersistentContextArgs, Bro
+ )
+
+ @property
+ def cloud_browser(self) -> bool:
+ """Alias for use_cloud field for compatibility."""
+ return self.use_cloud
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
There was a problem hiding this comment.
7 issues found across 10 files
Prompt for AI agents (all 7 issues)
Understand the root cause of the following 7 issues and fix them.
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:64">
API token retrieval logic duplicates in `CloudBrowserClient.create_browser()` and `CloudBrowserClient.stop_browser()`. This authentication pattern already exists with `CloudAuthConfig.load_from_file()` in `browser_use/sync/auth.py` and should be centralized into a single helper function.</violation>
<violation number="2" location="browser_use/browser/cloud.py:112">
Logging the CDP URL exposes the authenticated remote debugging endpoint for the live cloud browser session; anyone with log access could hijack the browser. Please drop or redact this log line so the URL never leaves process memory.</violation>
</file>
<file name="docs/customize/browser/remote.mdx">
<violation number="1" location="docs/customize/browser/remote.mdx:37">
Missing comma between arguments makes example code a SyntaxError.</violation>
</file>
<file name="browser_use/dom/playground/extraction.py">
<violation number="1" location="browser_use/dom/playground/extraction.py:179">
'gpt-4.1-mini' is not a known tiktoken alias, so encoding_for_model raises KeyError and breaks the loop.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The claim that tiktoken lacks an alias for "gpt-4.1-mini" could not be confirmed. Documentation and repository context do not show that the alias is absent, and the project now consistently uses this model name elsewhere, suggesting current tooling likely supports it. Without concrete evidence of a KeyError, the report is insufficiently certain to keep.</violation>
</file>
<file name="browser_use/browser/profile.py">
<violation number="1" location="browser_use/browser/profile.py:560">
Declaring `cloud_browser` as a read-only property breaks the documented compatibility claim. With `extra='ignore'` configured, passing `cloud_browser=True` to `BrowserProfile(...)` is now discarded, and assignments like `profile.cloud_browser = True` raise `AttributeError`. Please provide a real alias (e.g., validation alias or setter) so existing code can still configure the option via `cloud_browser`.</violation>
</file>
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:847">
Stopping a cloud browser leaves `browser_profile.cdp_url` pointing at the terminated session, so the next `start()` sees a populated CDP URL, skips provisioning, and then fails when it tries to connect to the now-deleted browser. Please clear the CDP URL after cleanup so a fresh cloud session is created on restart.</violation>
</file>
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:294">
The mocked cloud auth error message doesn't match the expected text, so this test will always fail when it asserts for 'BROWSER_USE_API_KEY environment variable'.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The test imports `get_cloud_browser_cdp_url` directly (`from browser_use.browser.cloud import get_cloud_browser_cdp_url`). When it calls `await get_cloud_browser_cdp_url()`, it invokes the real function, not the patched mock. With no auth token configured, the real implementation raises `CloudBrowserAuthError` with the message “No authentication token found. Please set BROWSER_USE_API_KEY environment variable…”, which already contains the asserted substring. Therefore the assertion succeeds and the reported failure does not occur.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| url = f'{self.api_base_url}/api/v2/browsers' | ||
|
|
||
| # Try to get API key from environment variable first, then auth config | ||
| api_token = os.getenv('BROWSER_USE_API_KEY') |
There was a problem hiding this comment.
API token retrieval logic duplicates in CloudBrowserClient.create_browser() and CloudBrowserClient.stop_browser(). This authentication pattern already exists with CloudAuthConfig.load_from_file() in browser_use/sync/auth.py and should be centralized into a single helper function.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 64:
<comment>API token retrieval logic duplicates in `CloudBrowserClient.create_browser()` and `CloudBrowserClient.stop_browser()`. This authentication pattern already exists with `CloudAuthConfig.load_from_file()` in `browser_use/sync/auth.py` and should be centralized into a single helper function.</comment>
<file context>
@@ -0,0 +1,286 @@
+ url = f'{self.api_base_url}/api/v2/browsers'
+
+ # Try to get API key from environment variable first, then auth config
+ api_token = os.getenv('BROWSER_USE_API_KEY')
+
+ if not api_token:
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: Duplicate Detection Agent
| # Use Browser-Use cloud browser service | ||
| browser = Browser( | ||
| cloud_browser=True # Automatically provisions a cloud browser | ||
| ) |
There was a problem hiding this comment.
Missing comma between arguments makes example code a SyntaxError.
Prompt for AI agents
Address the following comment on docs/customize/browser/remote.mdx at line 37:
<comment>Missing comma between arguments makes example code a SyntaxError.</comment>
<file context>
@@ -23,8 +23,39 @@ agent = Agent(
+# Use Browser-Use cloud browser service
+browser = Browser(
+ cloud_browser=True # Automatically provisions a cloud browser
+)
+
+agent = Agent(
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Functional Bugs Agent
| ) | |
| ), |
| # pyperclip.copy(text_to_save) | ||
|
|
||
| encoding = tiktoken.encoding_for_model('gpt-4o') | ||
| encoding = tiktoken.encoding_for_model('gpt-4.1-mini') |
There was a problem hiding this comment.
'gpt-4.1-mini' is not a known tiktoken alias, so encoding_for_model raises KeyError and breaks the loop.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: The claim that tiktoken lacks an alias for "gpt-4.1-mini" could not be confirmed. Documentation and repository context do not show that the alias is absent, and the project now consistently uses this model name elsewhere, suggesting current tooling likely supports it. Without concrete evidence of a KeyError, the report is insufficiently certain to keep.
Prompt for AI agents
Address the following comment on browser_use/dom/playground/extraction.py at line 179:
<comment>'gpt-4.1-mini' is not a known tiktoken alias, so encoding_for_model raises KeyError and breaks the loop.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The claim that tiktoken lacks an alias for "gpt-4.1-mini" could not be confirmed. Documentation and repository context do not show that the alias is absent, and the project now consistently uses this model name elsewhere, suggesting current tooling likely supports it. Without concrete evidence of a KeyError, the report is insufficiently certain to keep.</comment>
<file context>
@@ -176,7 +176,7 @@ def get_website_list_for_prompt() -> str:
# pyperclip.copy(text_to_save)
- encoding = tiktoken.encoding_for_model('gpt-4o')
+ encoding = tiktoken.encoding_for_model('gpt-4.1-mini')
token_count = len(encoding.encode(text_to_save))
print(f'Token count: {token_count}')
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Functional Bugs Agent
| ) | ||
|
|
||
| @property | ||
| def cloud_browser(self) -> bool: |
There was a problem hiding this comment.
Declaring cloud_browser as a read-only property breaks the documented compatibility claim. With extra='ignore' configured, passing cloud_browser=True to BrowserProfile(...) is now discarded, and assignments like profile.cloud_browser = True raise AttributeError. Please provide a real alias (e.g., validation alias or setter) so existing code can still configure the option via cloud_browser.
Prompt for AI agents
Address the following comment on browser_use/browser/profile.py at line 560:
<comment>Declaring `cloud_browser` as a read-only property breaks the documented compatibility claim. With `extra='ignore'` configured, passing `cloud_browser=True` to `BrowserProfile(...)` is now discarded, and assignments like `profile.cloud_browser = True` raise `AttributeError`. Please provide a real alias (e.g., validation alias or setter) so existing code can still configure the option via `cloud_browser`.</comment>
<file context>
@@ -550,7 +551,15 @@ class BrowserProfile(BrowserConnectArgs, BrowserLaunchPersistentContextArgs, Bro
+ )
+
+ @property
+ def cloud_browser(self) -> bool:
+ """Alias for use_cloud field for compatibility."""
+ return self.use_cloud
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
| self.current_session_id = browser_response.id | ||
|
|
||
| logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}') | ||
| logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}') |
There was a problem hiding this comment.
Logging the CDP URL exposes the authenticated remote debugging endpoint for the live cloud browser session; anyone with log access could hijack the browser. Please drop or redact this log line so the URL never leaves process memory.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 112:
<comment>Logging the CDP URL exposes the authenticated remote debugging endpoint for the live cloud browser session; anyone with log access could hijack the browser. Please drop or redact this log line so the URL never leaves process memory.</comment>
<file context>
@@ -0,0 +1,286 @@
+ self.current_session_id = browser_response.id
+
+ logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}')
+ logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}')
+ logger.debug(f'🌤️ Live URL: {browser_response.liveUrl}')
+
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
| logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}') | |
| logger.debug('🌤️ Cloud browser created successfully (CDP URL hidden)') |
| try: | ||
| from browser_use.browser.cloud import cleanup_cloud_client | ||
|
|
||
| await cleanup_cloud_client() |
There was a problem hiding this comment.
Stopping a cloud browser leaves browser_profile.cdp_url pointing at the terminated session, so the next start() sees a populated CDP URL, skips provisioning, and then fails when it tries to connect to the now-deleted browser. Please clear the CDP URL after cleanup so a fresh cloud session is created on restart.
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 847:
<comment>Stopping a cloud browser leaves `browser_profile.cdp_url` pointing at the terminated session, so the next `start()` sees a populated CDP URL, skips provisioning, and then fails when it tries to connect to the now-deleted browser. Please clear the CDP URL after cleanup so a fresh cloud session is created on restart.</comment>
<file context>
@@ -813,6 +839,16 @@ async def on_BrowserStopEvent(self, event: BrowserStopEvent) -> None:
+ try:
+ from browser_use.browser.cloud import cleanup_cloud_client
+
+ await cleanup_cloud_client()
+ self.logger.info('🌤️ Cloud browser session cleaned up')
+ except Exception as e:
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
|
|
||
| # Test that cloud browser client raises error without fallback | ||
| with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_cloud_cdp: | ||
| mock_cloud_cdp.side_effect = CloudBrowserAuthError('No auth token') |
There was a problem hiding this comment.
The mocked cloud auth error message doesn't match the expected text, so this test will always fail when it asserts for 'BROWSER_USE_API_KEY environment variable'.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: The test imports get_cloud_browser_cdp_url directly (from browser_use.browser.cloud import get_cloud_browser_cdp_url). When it calls await get_cloud_browser_cdp_url(), it invokes the real function, not the patched mock. With no auth token configured, the real implementation raises CloudBrowserAuthError with the message “No authentication token found. Please set BROWSER_USE_API_KEY environment variable…”, which already contains the asserted substring. Therefore the assertion succeeds and the reported failure does not occur.
Prompt for AI agents
Address the following comment on tests/ci/test_cloud_browser.py at line 294:
<comment>The mocked cloud auth error message doesn't match the expected text, so this test will always fail when it asserts for 'BROWSER_USE_API_KEY environment variable'.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The test imports `get_cloud_browser_cdp_url` directly (`from browser_use.browser.cloud import get_cloud_browser_cdp_url`). When it calls `await get_cloud_browser_cdp_url()`, it invokes the real function, not the patched mock. With no auth token configured, the real implementation raises `CloudBrowserAuthError` with the message “No authentication token found. Please set BROWSER_USE_API_KEY environment variable…”, which already contains the asserted substring. Therefore the assertion succeeds and the reported failure does not occur.</comment>
<file context>
@@ -0,0 +1,333 @@
+
+ # Test that cloud browser client raises error without fallback
+ with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_cloud_cdp:
+ mock_cloud_cdp.side_effect = CloudBrowserAuthError('No auth token')
+
+ # Verify that the cloud browser client raises the expected error
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
There was a problem hiding this comment.
5 issues found across 10 files
Prompt for AI agents (all 5 issues)
Understand the root cause of the following 5 issues and fix them.
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:286">
Allow `cloud_browser` to populate `use_cloud` only when the new `use_cloud` flag is absent so the explicit flag keeps precedence.</violation>
<violation number="2" location="browser_use/browser/session.py:519">
Stopping a cloud browser leaves an old CDP URL in the profile, so the next start reuses a dead connection and fails.</violation>
</file>
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:253">
This test only exercises the patched mock (`mock_get_cdp_url`) instead of calling the real `get_cloud_browser_cdp_url`, so the BrowserSession cloud integration path remains untested.</violation>
</file>
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:46">
Hard-coding the cloud API base URL here ignores the existing `BROWSER_USE_CLOUD_API_URL` configuration, so anyone pointing the client at staging or mocked endpoints cannot use the new cloud browser feature. Please default to the configurable URL instead of a constant.</violation>
<violation number="2" location="browser_use/browser/cloud.py:281">
The use of a stateful global client for managing cloud browser sessions will cause incorrect session termination when multiple cloud browsers are used concurrently. The session lifecycle management should be handled within each `BrowserSession` instance, not through a global singleton.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| profile_kwargs = {k: v for k, v in locals().items() if k not in ['self', 'browser_profile', 'id'] and v is not None} | ||
|
|
||
| # Handle backward compatibility: map cloud_browser to use_cloud | ||
| if 'cloud_browser' in profile_kwargs: |
There was a problem hiding this comment.
Allow cloud_browser to populate use_cloud only when the new use_cloud flag is absent so the explicit flag keeps precedence.
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 286:
<comment>Allow `cloud_browser` to populate `use_cloud` only when the new `use_cloud` flag is absent so the explicit flag keeps precedence.</comment>
<file context>
@@ -278,6 +282,10 @@ def __init__(
profile_kwargs = {k: v for k, v in locals().items() if k not in ['self', 'browser_profile', 'id'] and v is not None}
+ # Handle backward compatibility: map cloud_browser to use_cloud
+ if 'cloud_browser' in profile_kwargs:
+ profile_kwargs['use_cloud'] = profile_kwargs.pop('cloud_browser')
+
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| try: | ||
| cloud_cdp_url = await get_cloud_browser_cdp_url() | ||
| self.browser_profile.cdp_url = cloud_cdp_url | ||
| self.browser_profile.is_local = False |
There was a problem hiding this comment.
Stopping a cloud browser leaves an old CDP URL in the profile, so the next start reuses a dead connection and fails.
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 519:
<comment>Stopping a cloud browser leaves an old CDP URL in the profile, so the next start reuses a dead connection and fails.</comment>
<file context>
@@ -496,9 +509,22 @@ async def on_BrowserStartEvent(self, event: BrowserStartEvent) -> dict[str, str]
+ try:
+ cloud_cdp_url = await get_cloud_browser_cdp_url()
+ self.browser_profile.cdp_url = cloud_cdp_url
+ self.browser_profile.is_local = False
+ self.logger.info('🌤️ Successfully connected to cloud browser service')
+ except CloudBrowserAuthError:
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Functional Bugs Agent
| with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_get_cdp_url: | ||
| mock_get_cdp_url.return_value = 'wss://test.proxy.daytona.works' | ||
|
|
||
| cdp_url = await mock_get_cdp_url() |
There was a problem hiding this comment.
This test only exercises the patched mock (mock_get_cdp_url) instead of calling the real get_cloud_browser_cdp_url, so the BrowserSession cloud integration path remains untested.
Prompt for AI agents
Address the following comment on tests/ci/test_cloud_browser.py at line 253:
<comment>This test only exercises the patched mock (`mock_get_cdp_url`) instead of calling the real `get_cloud_browser_cdp_url`, so the BrowserSession cloud integration path remains untested.</comment>
<file context>
@@ -0,0 +1,333 @@
+ with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_get_cdp_url:
+ mock_get_cdp_url.return_value = 'wss://test.proxy.daytona.works'
+
+ cdp_url = await mock_get_cdp_url()
+ assert cdp_url == 'wss://test.proxy.daytona.works'
+ mock_get_cdp_url.assert_called_once()
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| raise | ||
|
|
||
|
|
||
| async def cleanup_cloud_client(): |
There was a problem hiding this comment.
The use of a stateful global client for managing cloud browser sessions will cause incorrect session termination when multiple cloud browsers are used concurrently. The session lifecycle management should be handled within each BrowserSession instance, not through a global singleton.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 281:
<comment>The use of a stateful global client for managing cloud browser sessions will cause incorrect session termination when multiple cloud browsers are used concurrently. The session lifecycle management should be handled within each `BrowserSession` instance, not through a global singleton.</comment>
<file context>
@@ -0,0 +1,286 @@
+ raise
+
+
+async def cleanup_cloud_client():
+ """Clean up the global cloud client."""
+ global _cloud_client
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
| class CloudBrowserClient: | ||
| """Client for browser-use cloud browser service.""" | ||
|
|
||
| def __init__(self, api_base_url: str = 'https://api.browser-use.com'): |
There was a problem hiding this comment.
Hard-coding the cloud API base URL here ignores the existing BROWSER_USE_CLOUD_API_URL configuration, so anyone pointing the client at staging or mocked endpoints cannot use the new cloud browser feature. Please default to the configurable URL instead of a constant.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 46:
<comment>Hard-coding the cloud API base URL here ignores the existing `BROWSER_USE_CLOUD_API_URL` configuration, so anyone pointing the client at staging or mocked endpoints cannot use the new cloud browser feature. Please default to the configurable URL instead of a constant.</comment>
<file context>
@@ -0,0 +1,286 @@
+class CloudBrowserClient:
+ """Client for browser-use cloud browser service."""
+
+ def __init__(self, api_base_url: str = 'https://api.browser-use.com'):
+ self.api_base_url = api_base_url
+ self.client = httpx.AsyncClient(timeout=30.0)
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
There was a problem hiding this comment.
4 issues found across 10 files
Prompt for AI agents (all 4 issues)
Understand the root cause of the following 4 issues and fix them.
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:64">
Authentication token retrieval logic is duplicated within CloudBrowserClient.create_browser() and CloudBrowserClient.stop_browser(). This logic should be extracted into a shared helper method.</violation>
<violation number="2" location="browser_use/browser/cloud.py:112">
Sensitive data exposure: CDP URL is logged, enabling unauthorized remote control if logs are accessed.</violation>
</file>
<file name="browser_use/browser/profile.py">
<violation number="1" location="browser_use/browser/profile.py:554">
Renaming the field to `use_cloud` without declaring an alias means configs that still pass `cloud_browser` are now ignored (extra fields are dropped), so the cloud flag silently falls back to the environment-default. Please add a validation alias so legacy inputs keep working.</violation>
</file>
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:294">
This test configures the mocked CloudBrowserAuthError to use the message 'No auth token' but then asserts that the raised message contains 'BROWSER_USE_API_KEY environment variable', so the assertion will always fail. Update the mocked exception message to match the expected text.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| url = f'{self.api_base_url}/api/v2/browsers' | ||
|
|
||
| # Try to get API key from environment variable first, then auth config | ||
| api_token = os.getenv('BROWSER_USE_API_KEY') |
There was a problem hiding this comment.
Authentication token retrieval logic is duplicated within CloudBrowserClient.create_browser() and CloudBrowserClient.stop_browser(). This logic should be extracted into a shared helper method.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 64:
<comment>Authentication token retrieval logic is duplicated within CloudBrowserClient.create_browser() and CloudBrowserClient.stop_browser(). This logic should be extracted into a shared helper method.</comment>
<file context>
@@ -0,0 +1,286 @@
+ url = f'{self.api_base_url}/api/v2/browsers'
+
+ # Try to get API key from environment variable first, then auth config
+ api_token = os.getenv('BROWSER_USE_API_KEY')
+
+ if not api_token:
</file context>
[internal] Confidence score: 9.5/10
[internal] Posted by: Duplicate Detection Agent
| self.current_session_id = browser_response.id | ||
|
|
||
| logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}') | ||
| logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}') |
There was a problem hiding this comment.
Sensitive data exposure: CDP URL is logged, enabling unauthorized remote control if logs are accessed.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 112:
<comment>Sensitive data exposure: CDP URL is logged, enabling unauthorized remote control if logs are accessed.</comment>
<file context>
@@ -0,0 +1,286 @@
+ self.current_session_id = browser_response.id
+
+ logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}')
+ logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}')
+ logger.debug(f'🌤️ Live URL: {browser_response.liveUrl}')
+
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Security Vulnerabilities Agent
| logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}') | |
| logger.debug('🌤️ CDP URL obtained') |
| cdp_url: str | None = Field(default=None, description='CDP URL for connecting to existing browser instance') | ||
| is_local: bool = Field(default=False, description='Whether this is a local browser instance') | ||
| # label: str = 'default' | ||
| use_cloud: bool = Field( |
There was a problem hiding this comment.
Renaming the field to use_cloud without declaring an alias means configs that still pass cloud_browser are now ignored (extra fields are dropped), so the cloud flag silently falls back to the environment-default. Please add a validation alias so legacy inputs keep working.
Prompt for AI agents
Address the following comment on browser_use/browser/profile.py at line 554:
<comment>Renaming the field to `use_cloud` without declaring an alias means configs that still pass `cloud_browser` are now ignored (extra fields are dropped), so the cloud flag silently falls back to the environment-default. Please add a validation alias so legacy inputs keep working.</comment>
<file context>
@@ -550,7 +551,15 @@ class BrowserProfile(BrowserConnectArgs, BrowserLaunchPersistentContextArgs, Bro
cdp_url: str | None = Field(default=None, description='CDP URL for connecting to existing browser instance')
is_local: bool = Field(default=False, description='Whether this is a local browser instance')
- # label: str = 'default'
+ use_cloud: bool = Field(
+ default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')),
+ description='Use browser-use cloud browser service instead of local browser',
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
|
|
||
| # Test that cloud browser client raises error without fallback | ||
| with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_cloud_cdp: | ||
| mock_cloud_cdp.side_effect = CloudBrowserAuthError('No auth token') |
There was a problem hiding this comment.
This test configures the mocked CloudBrowserAuthError to use the message 'No auth token' but then asserts that the raised message contains 'BROWSER_USE_API_KEY environment variable', so the assertion will always fail. Update the mocked exception message to match the expected text.
Prompt for AI agents
Address the following comment on tests/ci/test_cloud_browser.py at line 294:
<comment>This test configures the mocked CloudBrowserAuthError to use the message 'No auth token' but then asserts that the raised message contains 'BROWSER_USE_API_KEY environment variable', so the assertion will always fail. Update the mocked exception message to match the expected text.</comment>
<file context>
@@ -0,0 +1,333 @@
+
+ # Test that cloud browser client raises error without fallback
+ with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_cloud_cdp:
+ mock_cloud_cdp.side_effect = CloudBrowserAuthError('No auth token')
+
+ # Verify that the cloud browser client raises the expected error
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
There was a problem hiding this comment.
3 issues found across 10 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="README.md">
<violation number="1" location="README.md:89">
The new sentence has incorrect capitalization and is missing an article, which makes the documentation harder to read. Please rewrite it for proper grammar.</violation>
</file>
<file name="browser_use/browser/profile.py">
<violation number="1" location="browser_use/browser/profile.py:554">
Initializing `BrowserProfile` with the legacy `cloud_browser` parameter is now ignored because the new `use_cloud` field lacks an alias, so extra='ignore' drops it and leaves cloud sessions disabled. Please add a validation alias that maps `cloud_browser` to `use_cloud` to preserve backward compatibility.</violation>
</file>
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:227">
The use of a global singleton (`_cloud_client`) for managing cloud browser sessions is a critical architectural flaw. This design prevents running multiple concurrent cloud sessions, as creating a new session overwrites the state of the previous one, leading to incorrect session termination and orphaned cloud resources.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| ) | ||
| ``` | ||
|
|
||
| First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com). |
There was a problem hiding this comment.
The new sentence has incorrect capitalization and is missing an article, which makes the documentation harder to read. Please rewrite it for proper grammar.
Prompt for AI agents
Address the following comment on README.md at line 89:
<comment>The new sentence has incorrect capitalization and is missing an article, which makes the documentation harder to read. Please rewrite it for proper grammar.</comment>
<file context>
@@ -74,6 +74,22 @@ OPENAI_API_KEY=
+)
+```
+
+First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com).
+
+For other settings, models, and more, check out the [Cloud documentation 📕](https://docs.cloud.browser-use.com).
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com). | |
| First set the `BROWSER_USE_API_KEY` environment variable. You can get your API key from [here](https://cloud.browser-use.com). |
| cdp_url: str | None = Field(default=None, description='CDP URL for connecting to existing browser instance') | ||
| is_local: bool = Field(default=False, description='Whether this is a local browser instance') | ||
| # label: str = 'default' | ||
| use_cloud: bool = Field( |
There was a problem hiding this comment.
Initializing BrowserProfile with the legacy cloud_browser parameter is now ignored because the new use_cloud field lacks an alias, so extra='ignore' drops it and leaves cloud sessions disabled. Please add a validation alias that maps cloud_browser to use_cloud to preserve backward compatibility.
Prompt for AI agents
Address the following comment on browser_use/browser/profile.py at line 554:
<comment>Initializing `BrowserProfile` with the legacy `cloud_browser` parameter is now ignored because the new `use_cloud` field lacks an alias, so extra='ignore' drops it and leaves cloud sessions disabled. Please add a validation alias that maps `cloud_browser` to `use_cloud` to preserve backward compatibility.</comment>
<file context>
@@ -550,7 +551,15 @@ class BrowserProfile(BrowserConnectArgs, BrowserLaunchPersistentContextArgs, Bro
cdp_url: str | None = Field(default=None, description='CDP URL for connecting to existing browser instance')
is_local: bool = Field(default=False, description='Whether this is a local browser instance')
- # label: str = 'default'
+ use_cloud: bool = Field(
+ default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')),
+ description='Use browser-use cloud browser service instead of local browser',
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
|
|
||
|
|
||
| # Global client instance | ||
| _cloud_client: CloudBrowserClient | None = None |
There was a problem hiding this comment.
The use of a global singleton (_cloud_client) for managing cloud browser sessions is a critical architectural flaw. This design prevents running multiple concurrent cloud sessions, as creating a new session overwrites the state of the previous one, leading to incorrect session termination and orphaned cloud resources.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 227:
<comment>The use of a global singleton (`_cloud_client`) for managing cloud browser sessions is a critical architectural flaw. This design prevents running multiple concurrent cloud sessions, as creating a new session overwrites the state of the previous one, leading to incorrect session termination and orphaned cloud resources.</comment>
<file context>
@@ -0,0 +1,286 @@
+
+
+# Global client instance
+_cloud_client: CloudBrowserClient | None = None
+
+
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
There was a problem hiding this comment.
6 issues found across 10 files
Prompt for AI agents (all 6 issues)
Understand the root cause of the following 6 issues and fix them.
<file name="docs/customize/browser/remote.mdx">
<violation number="1" location="docs/customize/browser/remote.mdx:37">
Missing comma between Browser arguments makes example code invalid Python.</violation>
</file>
<file name="browser_use/config.py">
<violation number="1" location="browser_use/config.py:290">
Arbitrary file overwrite via env-controlled BROWSER_USE_CONFIG_PATH; JSON parse failures trigger overwrite with open(..., 'w'), enabling destructive writes to sensitive files.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The environment variable is intended to let the process owner choose a custom config location; it does not introduce a new privilege escalation path because overwriting a file still requires the process to already have write permission to that target. This behavior is consistent with prior releases and is not a demonstrable vulnerability. Additionally, the plaintext API key entry already existed before this change—the only modification here is the model name—so there is no new secret-handling issue introduced in this diff.</violation>
</file>
<file name="browser_use/browser/profile.py">
<violation number="1" location="browser_use/browser/profile.py:554">
`use_cloud` ignores the incoming `cloud_browser` kwarg: BrowserProfile has `extra='ignore'`, so `BrowserProfile(cloud_browser=True)` gets dropped and the new property has no setter. Add an alias so legacy constructors still set the flag.</violation>
</file>
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:847">
Cloud sessions keep using a stale CDP URL after cleanup, so restarting the session fails</violation>
</file>
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:112">
Avoid logging the CDP endpoint; it exposes the remote-debugging URL and would let anyone with log access hijack the session.</violation>
<violation number="2" location="browser_use/browser/cloud.py:113">
Avoid logging the live-view URL; publishing it to logs shares a sensitive link that grants access to the running cloud browser session.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| # Use Browser-Use cloud browser service | ||
| browser = Browser( | ||
| cloud_browser=True # Automatically provisions a cloud browser | ||
| ) |
There was a problem hiding this comment.
Missing comma between Browser arguments makes example code invalid Python.
Prompt for AI agents
Address the following comment on docs/customize/browser/remote.mdx at line 37:
<comment>Missing comma between Browser arguments makes example code invalid Python.</comment>
<file context>
@@ -23,8 +23,39 @@ agent = Agent(
+# Use Browser-Use cloud browser service
+browser = Browser(
+ cloud_browser=True # Automatically provisions a cloud browser
+)
+
+agent = Agent(
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Functional Bugs Agent
| ) | |
| ), |
|
|
||
| # Create default LLM entry | ||
| new_config.llm[llm_id] = LLMEntry(id=llm_id, default=True, model='gpt-4o', api_key='your-openai-api-key-here') | ||
| new_config.llm[llm_id] = LLMEntry(id=llm_id, default=True, model='gpt-4.1-mini', api_key='your-openai-api-key-here') |
There was a problem hiding this comment.
Arbitrary file overwrite via env-controlled BROWSER_USE_CONFIG_PATH; JSON parse failures trigger overwrite with open(..., 'w'), enabling destructive writes to sensitive files.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: The environment variable is intended to let the process owner choose a custom config location; it does not introduce a new privilege escalation path because overwriting a file still requires the process to already have write permission to that target. This behavior is consistent with prior releases and is not a demonstrable vulnerability. Additionally, the plaintext API key entry already existed before this change—the only modification here is the model name—so there is no new secret-handling issue introduced in this diff.
Prompt for AI agents
Address the following comment on browser_use/config.py at line 290:
<comment>Arbitrary file overwrite via env-controlled BROWSER_USE_CONFIG_PATH; JSON parse failures trigger overwrite with open(..., 'w'), enabling destructive writes to sensitive files.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The environment variable is intended to let the process owner choose a custom config location; it does not introduce a new privilege escalation path because overwriting a file still requires the process to already have write permission to that target. This behavior is consistent with prior releases and is not a demonstrable vulnerability. Additionally, the plaintext API key entry already existed before this change—the only modification here is the model name—so there is no new secret-handling issue introduced in this diff.</comment>
<file context>
@@ -287,7 +287,7 @@ def create_default_config() -> DBStyleConfigJSON:
# Create default LLM entry
- new_config.llm[llm_id] = LLMEntry(id=llm_id, default=True, model='gpt-4o', api_key='your-openai-api-key-here')
+ new_config.llm[llm_id] = LLMEntry(id=llm_id, default=True, model='gpt-4.1-mini', api_key='your-openai-api-key-here')
# Create default agent entry
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Security Vulnerabilities Agent
| cdp_url: str | None = Field(default=None, description='CDP URL for connecting to existing browser instance') | ||
| is_local: bool = Field(default=False, description='Whether this is a local browser instance') | ||
| # label: str = 'default' | ||
| use_cloud: bool = Field( |
There was a problem hiding this comment.
use_cloud ignores the incoming cloud_browser kwarg: BrowserProfile has extra='ignore', so BrowserProfile(cloud_browser=True) gets dropped and the new property has no setter. Add an alias so legacy constructors still set the flag.
Prompt for AI agents
Address the following comment on browser_use/browser/profile.py at line 554:
<comment>`use_cloud` ignores the incoming `cloud_browser` kwarg: BrowserProfile has `extra='ignore'`, so `BrowserProfile(cloud_browser=True)` gets dropped and the new property has no setter. Add an alias so legacy constructors still set the flag.</comment>
<file context>
@@ -550,7 +551,15 @@ class BrowserProfile(BrowserConnectArgs, BrowserLaunchPersistentContextArgs, Bro
cdp_url: str | None = Field(default=None, description='CDP URL for connecting to existing browser instance')
is_local: bool = Field(default=False, description='Whether this is a local browser instance')
- # label: str = 'default'
+ use_cloud: bool = Field(
+ default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')),
+ description='Use browser-use cloud browser service instead of local browser',
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| try: | ||
| from browser_use.browser.cloud import cleanup_cloud_client | ||
|
|
||
| await cleanup_cloud_client() |
There was a problem hiding this comment.
Cloud sessions keep using a stale CDP URL after cleanup, so restarting the session fails
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 847:
<comment>Cloud sessions keep using a stale CDP URL after cleanup, so restarting the session fails</comment>
<file context>
@@ -813,6 +839,16 @@ async def on_BrowserStopEvent(self, event: BrowserStopEvent) -> None:
+ try:
+ from browser_use.browser.cloud import cleanup_cloud_client
+
+ await cleanup_cloud_client()
+ self.logger.info('🌤️ Cloud browser session cleaned up')
+ except Exception as e:
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Functional Bugs Agent
|
|
||
| logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}') | ||
| logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}') | ||
| logger.debug(f'🌤️ Live URL: {browser_response.liveUrl}') |
There was a problem hiding this comment.
Avoid logging the live-view URL; publishing it to logs shares a sensitive link that grants access to the running cloud browser session.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 113:
<comment>Avoid logging the live-view URL; publishing it to logs shares a sensitive link that grants access to the running cloud browser session.</comment>
<file context>
@@ -0,0 +1,286 @@
+
+ logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}')
+ logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}')
+ logger.debug(f'🌤️ Live URL: {browser_response.liveUrl}')
+
+ return browser_response
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| logger.debug(f'🌤️ Live URL: {browser_response.liveUrl}') | |
| logger.debug('🌤️ Cloud browser live view ready') |
| self.current_session_id = browser_response.id | ||
|
|
||
| logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}') | ||
| logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}') |
There was a problem hiding this comment.
Avoid logging the CDP endpoint; it exposes the remote-debugging URL and would let anyone with log access hijack the session.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 112:
<comment>Avoid logging the CDP endpoint; it exposes the remote-debugging URL and would let anyone with log access hijack the session.</comment>
<file context>
@@ -0,0 +1,286 @@
+ self.current_session_id = browser_response.id
+
+ logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}')
+ logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}')
+ logger.debug(f'🌤️ Live URL: {browser_response.liveUrl}')
+
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}') | |
| logger.debug('🌤️ Cloud browser connection established') |
There was a problem hiding this comment.
5 issues found across 10 files
Prompt for AI agents (all 5 issues)
Understand the root cause of the following 5 issues and fix them.
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:64">
API token retrieval logic is duplicated in `CloudBrowserClient.create_browser()` and `CloudBrowserClient.stop_browser()`. This pattern also exists in `browser_use/sync/auth.py`.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The two methods in CloudBrowserClient do repeat the simple API-token lookup, but this is a small, straightforward block that does not pose a significant maintenance risk. Moreover, the explanation incorrectly claims the same logic exists in browser_use/sync/auth.py, which does not perform the environment-variable fallback. Given the low impact and partially inaccurate evidence, this should not be reported.</violation>
<violation number="2" location="browser_use/browser/cloud.py:227">
The introduction of a global `_cloud_client` singleton to manage cloud browser sessions introduces a critical architectural flaw. The client instance stores `current_session_id`, which is state specific to a single browser session. If multiple `BrowserSession` instances with `use_cloud=True` are created concurrently, they will all share and overwrite this single global client and its `current_session_id`. This leads to race conditions where stopping one session can terminate the browser of another session, causing unpredictable behavior and orphaned cloud resources. The `CloudBrowserClient` should be instantiated per `BrowserSession` to ensure state isolation.</violation>
</file>
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:847">
Cleaning up a cloud session leaves `browser_profile.cdp_url` pointing to the terminated instance, so the next start sees a CDP URL, skips provisioning, and then fails connecting to a dead CDP socket. Please clear `browser_profile.cdp_url` when cleaning up cloud sessions so a fresh browser can be created.</violation>
</file>
<file name="browser_use/browser/profile.py">
<violation number="1" location="browser_use/browser/profile.py:556">
Existing callers that pass `cloud_browser=...` to `BrowserProfile` now silently lose that configuration because extra fields are ignored and no alias maps the legacy name to `use_cloud`. Please add a validation alias so the legacy parameter still sets `use_cloud`.</violation>
</file>
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:298">
This test awaits the real `get_cloud_browser_cdp_url` coroutine even though it patches the cloud module. Because the symbol was imported directly, the patch never takes effect, so the test can reach the live cloud API whenever credentials are present. Please patch the local name (or call through the module) to keep the test isolated.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| url = f'{self.api_base_url}/api/v2/browsers' | ||
|
|
||
| # Try to get API key from environment variable first, then auth config | ||
| api_token = os.getenv('BROWSER_USE_API_KEY') |
There was a problem hiding this comment.
API token retrieval logic is duplicated in CloudBrowserClient.create_browser() and CloudBrowserClient.stop_browser(). This pattern also exists in browser_use/sync/auth.py.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: The two methods in CloudBrowserClient do repeat the simple API-token lookup, but this is a small, straightforward block that does not pose a significant maintenance risk. Moreover, the explanation incorrectly claims the same logic exists in browser_use/sync/auth.py, which does not perform the environment-variable fallback. Given the low impact and partially inaccurate evidence, this should not be reported.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 64:
<comment>API token retrieval logic is duplicated in `CloudBrowserClient.create_browser()` and `CloudBrowserClient.stop_browser()`. This pattern also exists in `browser_use/sync/auth.py`.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The two methods in CloudBrowserClient do repeat the simple API-token lookup, but this is a small, straightforward block that does not pose a significant maintenance risk. Moreover, the explanation incorrectly claims the same logic exists in browser_use/sync/auth.py, which does not perform the environment-variable fallback. Given the low impact and partially inaccurate evidence, this should not be reported.</comment>
<file context>
@@ -0,0 +1,286 @@
+ url = f'{self.api_base_url}/api/v2/browsers'
+
+ # Try to get API key from environment variable first, then auth config
+ api_token = os.getenv('BROWSER_USE_API_KEY')
+
+ if not api_token:
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Duplicate Detection Agent
| try: | ||
| from browser_use.browser.cloud import cleanup_cloud_client | ||
|
|
||
| await cleanup_cloud_client() |
There was a problem hiding this comment.
Cleaning up a cloud session leaves browser_profile.cdp_url pointing to the terminated instance, so the next start sees a CDP URL, skips provisioning, and then fails connecting to a dead CDP socket. Please clear browser_profile.cdp_url when cleaning up cloud sessions so a fresh browser can be created.
Prompt for AI agents
Address the following comment on browser_use/browser/session.py at line 847:
<comment>Cleaning up a cloud session leaves `browser_profile.cdp_url` pointing to the terminated instance, so the next start sees a CDP URL, skips provisioning, and then fails connecting to a dead CDP socket. Please clear `browser_profile.cdp_url` when cleaning up cloud sessions so a fresh browser can be created.</comment>
<file context>
@@ -813,6 +839,16 @@ async def on_BrowserStopEvent(self, event: BrowserStopEvent) -> None:
+ try:
+ from browser_use.browser.cloud import cleanup_cloud_client
+
+ await cleanup_cloud_client()
+ self.logger.info('🌤️ Cloud browser session cleaned up')
+ except Exception as e:
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| # label: str = 'default' | ||
| use_cloud: bool = Field( | ||
| default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')), | ||
| description='Use browser-use cloud browser service instead of local browser', |
There was a problem hiding this comment.
Existing callers that pass cloud_browser=... to BrowserProfile now silently lose that configuration because extra fields are ignored and no alias maps the legacy name to use_cloud. Please add a validation alias so the legacy parameter still sets use_cloud.
Prompt for AI agents
Address the following comment on browser_use/browser/profile.py at line 556:
<comment>Existing callers that pass `cloud_browser=...` to `BrowserProfile` now silently lose that configuration because extra fields are ignored and no alias maps the legacy name to `use_cloud`. Please add a validation alias so the legacy parameter still sets `use_cloud`.</comment>
<file context>
@@ -550,7 +551,15 @@ class BrowserProfile(BrowserConnectArgs, BrowserLaunchPersistentContextArgs, Bro
- # label: str = 'default'
+ use_cloud: bool = Field(
+ default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')),
+ description='Use browser-use cloud browser service instead of local browser',
+ )
+
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
|
|
||
|
|
||
| # Global client instance | ||
| _cloud_client: CloudBrowserClient | None = None |
There was a problem hiding this comment.
The introduction of a global _cloud_client singleton to manage cloud browser sessions introduces a critical architectural flaw. The client instance stores current_session_id, which is state specific to a single browser session. If multiple BrowserSession instances with use_cloud=True are created concurrently, they will all share and overwrite this single global client and its current_session_id. This leads to race conditions where stopping one session can terminate the browser of another session, causing unpredictable behavior and orphaned cloud resources. The CloudBrowserClient should be instantiated per BrowserSession to ensure state isolation.
Prompt for AI agents
Address the following comment on browser_use/browser/cloud.py at line 227:
<comment>The introduction of a global `_cloud_client` singleton to manage cloud browser sessions introduces a critical architectural flaw. The client instance stores `current_session_id`, which is state specific to a single browser session. If multiple `BrowserSession` instances with `use_cloud=True` are created concurrently, they will all share and overwrite this single global client and its `current_session_id`. This leads to race conditions where stopping one session can terminate the browser of another session, causing unpredictable behavior and orphaned cloud resources. The `CloudBrowserClient` should be instantiated per `BrowserSession` to ensure state isolation.</comment>
<file context>
@@ -0,0 +1,286 @@
+
+
+# Global client instance
+_cloud_client: CloudBrowserClient | None = None
+
+
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: System Design Agent
|
|
||
| # Verify that the cloud browser client raises the expected error | ||
| with pytest.raises(CloudBrowserAuthError) as exc_info: | ||
| await get_cloud_browser_cdp_url() |
There was a problem hiding this comment.
This test awaits the real get_cloud_browser_cdp_url coroutine even though it patches the cloud module. Because the symbol was imported directly, the patch never takes effect, so the test can reach the live cloud API whenever credentials are present. Please patch the local name (or call through the module) to keep the test isolated.
Prompt for AI agents
Address the following comment on tests/ci/test_cloud_browser.py at line 298:
<comment>This test awaits the real `get_cloud_browser_cdp_url` coroutine even though it patches the cloud module. Because the symbol was imported directly, the patch never takes effect, so the test can reach the live cloud API whenever credentials are present. Please patch the local name (or call through the module) to keep the test isolated.</comment>
<file context>
@@ -0,0 +1,333 @@
+
+ # Verify that the cloud browser client raises the expected error
+ with pytest.raises(CloudBrowserAuthError) as exc_info:
+ await get_cloud_browser_cdp_url()
+
+ assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value)
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
There was a problem hiding this comment.
7 issues found across 10 files
Prompt for AI agents (all 7 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="README.md">
<violation number="1" location="README.md:89">
P3: Grammar issue: "Set" should be lowercase "set". Consider rephrasing to "First, set the BROWSER_USE_API_KEY environment variable."</violation>
</file>
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:100">
P1: Test assertion will always fail: the mock raises `CloudBrowserAuthError('No auth token')` but the assertion expects the error message to contain `'BROWSER_USE_API_KEY environment variable'`. The error message from the mock does not match the expected substring.</violation>
</file>
<file name="browser_use/browser/profile.py">
<violation number="1" location="browser_use/browser/profile.py:554">
P1: The `cloud_browser` property is read-only and cannot be used as a constructor argument. Since the model has `extra='ignore'`, passing `BrowserProfile(cloud_browser=True)` will silently ignore the argument. Add a `validation_alias` to `use_cloud` to support both names as constructor parameters.</violation>
</file>
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:521">
P2: Exception re-raised without chaining loses the original traceback. Use `raise ... from e` to preserve the exception chain for easier debugging.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Catching a specific exception and raising a new instance with a clearer, user-focused message is a common pattern. While it drops the original stack trace, this is an intentional trade-off and not a clear defect with high impact, so it shouldn’t be reported as a violation.</violation>
<violation number="2" location="browser_use/browser/session.py:525">
P2: Exception re-raised without chaining loses the original traceback. Use `raise ... from e` instead of embedding `{e}` in the message string.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The handler re-raises a CloudBrowserError with additional context about the failure, which is a valid pattern. The absence of explicit exception chaining is at most a minor debugging consideration and does not rise to a high-impact issue worth flagging.</violation>
<violation number="3" location="browser_use/browser/session.py:845">
P3: Inconsistent import pattern: `cleanup_cloud_client` should be added to the top-level import from `browser_use.browser.cloud` rather than importing locally inside the function.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: functionalImpact, objectivity.*
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Using a local import for `cleanup_cloud_client` is a stylistic choice (and may avoid unnecessary imports or circular dependencies). It has no functional impact, so it shouldn’t be treated as a violation.</violation>
</file>
<file name="browser_use/dom/playground/extraction.py">
<violation number="1" location="browser_use/dom/playground/extraction.py:179">
P1: `tiktoken.encoding_for_model('gpt-4.1-mini')` will raise a `KeyError` because GPT-4.1 models are not yet supported in tiktoken. Use a supported model like `'gpt-4o'` or use the encoding directly with `tiktoken.get_encoding('o200k_base')`.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Cannot independently confirm that tiktoken.encoding_for_model lacks support for 'gpt-4.1-mini'; available documentation searched does not verify the claimed KeyError, so the issue remains unproven.
• **Exa queries**: "tiktoken encoding_for_model gpt-4.1 support latest version KeyError", "tiktoken encoding_for_model gpt-4.1 KeyError latest version", "tiktoken encoding_for_model "gpt-4.1-mini" KeyError"</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| ) | ||
| ``` | ||
|
|
||
| First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com). |
There was a problem hiding this comment.
P3: Grammar issue: "Set" should be lowercase "set". Consider rephrasing to "First, set the BROWSER_USE_API_KEY environment variable."
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 89:
<comment>Grammar issue: "Set" should be lowercase "set". Consider rephrasing to "First, set the BROWSER_USE_API_KEY environment variable."</comment>
<file context>
@@ -74,6 +74,22 @@ OPENAI_API_KEY=
+)
+```
+
+First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com).
+
+For other settings, models, and more, check out the [Cloud documentation 📕](https://docs.cloud.browser-use.com).
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com). | |
| First, set the `BROWSER_USE_API_KEY` environment variable. You can get your API key from [here](https://cloud.browser-use.com). |
| with pytest.raises(CloudBrowserAuthError) as exc_info: | ||
| await client.create_browser() | ||
|
|
||
| assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value) |
There was a problem hiding this comment.
P1: Test assertion will always fail: the mock raises CloudBrowserAuthError('No auth token') but the assertion expects the error message to contain 'BROWSER_USE_API_KEY environment variable'. The error message from the mock does not match the expected substring.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/ci/test_cloud_browser.py, line 100:
<comment>Test assertion will always fail: the mock raises `CloudBrowserAuthError('No auth token')` but the assertion expects the error message to contain `'BROWSER_USE_API_KEY environment variable'`. The error message from the mock does not match the expected substring.</comment>
<file context>
@@ -0,0 +1,333 @@
+ with pytest.raises(CloudBrowserAuthError) as exc_info:
+ await client.create_browser()
+
+ assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value)
+
+ async def test_create_browser_http_401(self, mock_auth_config):
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value) | |
| assert 'No auth token' in str(exc_info.value) |
| use_cloud: bool = Field( | ||
| default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')), | ||
| description='Use browser-use cloud browser service instead of local browser', | ||
| ) |
There was a problem hiding this comment.
P1: The cloud_browser property is read-only and cannot be used as a constructor argument. Since the model has extra='ignore', passing BrowserProfile(cloud_browser=True) will silently ignore the argument. Add a validation_alias to use_cloud to support both names as constructor parameters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/profile.py, line 554:
<comment>The `cloud_browser` property is read-only and cannot be used as a constructor argument. Since the model has `extra='ignore'`, passing `BrowserProfile(cloud_browser=True)` will silently ignore the argument. Add a `validation_alias` to `use_cloud` to support both names as constructor parameters.</comment>
<file context>
@@ -550,7 +551,15 @@ class BrowserProfile(BrowserConnectArgs, BrowserLaunchPersistentContextArgs, Bro
cdp_url: str | None = Field(default=None, description='CDP URL for connecting to existing browser instance')
is_local: bool = Field(default=False, description='Whether this is a local browser instance')
- # label: str = 'default'
+ use_cloud: bool = Field(
+ default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')),
+ description='Use browser-use cloud browser service instead of local browser',
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| use_cloud: bool = Field( | |
| default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')), | |
| description='Use browser-use cloud browser service instead of local browser', | |
| ) | |
| use_cloud: bool = Field( | |
| default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')), | |
| description='Use browser-use cloud browser service instead of local browser', | |
| validation_alias=AliasChoices('cloud_browser', 'use_cloud'), | |
| ) |
| # Clean up cloud browser session if using cloud browser | ||
| if self.browser_profile.use_cloud: | ||
| try: | ||
| from browser_use.browser.cloud import cleanup_cloud_client |
There was a problem hiding this comment.
P3: Inconsistent import pattern: cleanup_cloud_client should be added to the top-level import from browser_use.browser.cloud rather than importing locally inside the function.
DEV MODE: This violation would have been filtered out by screening filters. Failing filters: functionalImpact, objectivity.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Using a local import for cleanup_cloud_client is a stylistic choice (and may avoid unnecessary imports or circular dependencies). It has no functional impact, so it shouldn’t be treated as a violation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/session.py, line 845:
<comment>Inconsistent import pattern: `cleanup_cloud_client` should be added to the top-level import from `browser_use.browser.cloud` rather than importing locally inside the function.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: functionalImpact, objectivity.*
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Using a local import for `cleanup_cloud_client` is a stylistic choice (and may avoid unnecessary imports or circular dependencies). It has no functional impact, so it shouldn’t be treated as a violation.</comment>
<file context>
@@ -813,6 +839,16 @@ async def on_BrowserStopEvent(self, event: BrowserStopEvent) -> None:
+ # Clean up cloud browser session if using cloud browser
+ if self.browser_profile.use_cloud:
+ try:
+ from browser_use.browser.cloud import cleanup_cloud_client
+
+ await cleanup_cloud_client()
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
| except CloudBrowserError as e: | ||
| raise CloudBrowserError(f'Failed to create cloud browser: {e}') |
There was a problem hiding this comment.
P2: Exception re-raised without chaining loses the original traceback. Use raise ... from e instead of embedding {e} in the message string.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: The handler re-raises a CloudBrowserError with additional context about the failure, which is a valid pattern. The absence of explicit exception chaining is at most a minor debugging consideration and does not rise to a high-impact issue worth flagging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/session.py, line 525:
<comment>Exception re-raised without chaining loses the original traceback. Use `raise ... from e` instead of embedding `{e}` in the message string.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: The handler re-raises a CloudBrowserError with additional context about the failure, which is a valid pattern. The absence of explicit exception chaining is at most a minor debugging consideration and does not rise to a high-impact issue worth flagging.</comment>
<file context>
@@ -496,9 +509,22 @@ async def on_BrowserStartEvent(self, event: BrowserStartEvent) -> dict[str, str]
+ raise CloudBrowserAuthError(
+ 'Authentication failed for cloud browser service. Set BROWSER_USE_API_KEY environment variable'
+ )
+ except CloudBrowserError as e:
+ raise CloudBrowserError(f'Failed to create cloud browser: {e}')
+ elif self.is_local:
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| except CloudBrowserError as e: | |
| raise CloudBrowserError(f'Failed to create cloud browser: {e}') | |
| except CloudBrowserError as e: | |
| raise CloudBrowserError('Failed to create cloud browser') from e |
| except CloudBrowserAuthError: | ||
| raise CloudBrowserAuthError( | ||
| 'Authentication failed for cloud browser service. Set BROWSER_USE_API_KEY environment variable' | ||
| ) |
There was a problem hiding this comment.
P2: Exception re-raised without chaining loses the original traceback. Use raise ... from e to preserve the exception chain for easier debugging.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Catching a specific exception and raising a new instance with a clearer, user-focused message is a common pattern. While it drops the original stack trace, this is an intentional trade-off and not a clear defect with high impact, so it shouldn’t be reported as a violation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/session.py, line 521:
<comment>Exception re-raised without chaining loses the original traceback. Use `raise ... from e` to preserve the exception chain for easier debugging.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Catching a specific exception and raising a new instance with a clearer, user-focused message is a common pattern. While it drops the original stack trace, this is an intentional trade-off and not a clear defect with high impact, so it shouldn’t be reported as a violation.</comment>
<file context>
@@ -496,9 +509,22 @@ async def on_BrowserStartEvent(self, event: BrowserStartEvent) -> dict[str, str]
+ self.browser_profile.cdp_url = cloud_cdp_url
+ self.browser_profile.is_local = False
+ self.logger.info('🌤️ Successfully connected to cloud browser service')
+ except CloudBrowserAuthError:
+ raise CloudBrowserAuthError(
+ 'Authentication failed for cloud browser service. Set BROWSER_USE_API_KEY environment variable'
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
| except CloudBrowserAuthError: | |
| raise CloudBrowserAuthError( | |
| 'Authentication failed for cloud browser service. Set BROWSER_USE_API_KEY environment variable' | |
| ) | |
| except CloudBrowserAuthError as e: | |
| raise CloudBrowserAuthError( | |
| 'Authentication failed for cloud browser service. Set BROWSER_USE_API_KEY environment variable' | |
| ) from e |
| # pyperclip.copy(text_to_save) | ||
|
|
||
| encoding = tiktoken.encoding_for_model('gpt-4o') | ||
| encoding = tiktoken.encoding_for_model('gpt-4.1-mini') |
There was a problem hiding this comment.
P1: tiktoken.encoding_for_model('gpt-4.1-mini') will raise a KeyError because GPT-4.1 models are not yet supported in tiktoken. Use a supported model like 'gpt-4o' or use the encoding directly with tiktoken.get_encoding('o200k_base').
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: Cannot independently confirm that tiktoken.encoding_for_model lacks support for 'gpt-4.1-mini'; available documentation searched does not verify the claimed KeyError, so the issue remains unproven.
• Exa queries: "tiktoken encoding_for_model gpt-4.1 support latest version KeyError", "tiktoken encoding_for_model gpt-4.1 KeyError latest version", "tiktoken encoding_for_model "gpt-4.1-mini" KeyError"
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/dom/playground/extraction.py, line 179:
<comment>`tiktoken.encoding_for_model('gpt-4.1-mini')` will raise a `KeyError` because GPT-4.1 models are not yet supported in tiktoken. Use a supported model like `'gpt-4o'` or use the encoding directly with `tiktoken.get_encoding('o200k_base')`.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: Cannot independently confirm that tiktoken.encoding_for_model lacks support for 'gpt-4.1-mini'; available documentation searched does not verify the claimed KeyError, so the issue remains unproven.
• **Exa queries**: "tiktoken encoding_for_model gpt-4.1 support latest version KeyError", "tiktoken encoding_for_model gpt-4.1 KeyError latest version", "tiktoken encoding_for_model "gpt-4.1-mini" KeyError"</comment>
<file context>
@@ -176,7 +176,7 @@ def get_website_list_for_prompt() -> str:
# pyperclip.copy(text_to_save)
- encoding = tiktoken.encoding_for_model('gpt-4o')
+ encoding = tiktoken.encoding_for_model('gpt-4.1-mini')
token_count = len(encoding.encode(text_to_save))
print(f'Token count: {token_count}')
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
|
@cubic-dev-ai review this |
@gpurich I have started the AI code review. It will take a few minutes to complete. |
1 similar comment
@gpurich I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
8 issues found across 10 files
Confidence score: 3/5
browser_use/browser/profile.pynow ignores thecloud_browserargument, silently breaking backward compatibility and leavingBrowserProfile(cloud_browser=True)unusable.tests/ci/test_cloud_browser.pyasserts for the wrong error message and directly calls the mock, so the suite will fail while also not exercising real code paths, andbrowser_use/browser/session.pyswallows originalCloudBrowserAuthErrorcontext, hindering debugging.- Pay close attention to
browser_use/browser/profile.py,tests/ci/test_cloud_browser.py,browser_use/browser/session.py- breaking compatibility, failing test expectations, and exception trace preservation.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:100">
P1: Assertion will always fail: the mock raises `CloudBrowserAuthError('No auth token')` but the assertion expects the message to contain `'BROWSER_USE_API_KEY environment variable'`. Either update the mock's error message to match or fix the assertion.</violation>
<violation number="2" location="tests/ci/test_cloud_browser.py:253">
P2: This test calls the mock object directly (`await mock_get_cdp_url()`) instead of calling code that would invoke the mocked function. It tests that the mock returns the configured value, not any actual functionality.</violation>
</file>
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:521">
P2: Catching `CloudBrowserAuthError` without `as e` loses the original error message and traceback. The original exception may contain more specific information. Consider using `raise` to re-raise the original exception.</violation>
<violation number="2" location="browser_use/browser/session.py:526">
P2: Re-raising `CloudBrowserError` without `from e` breaks the exception chain and loses the original traceback. Use `raise CloudBrowserError(...) from e` to preserve the exception chain.</violation>
</file>
<file name="browser_use/browser/profile.py">
<violation number="1" location="browser_use/browser/profile.py:554">
P1: Read-only property `cloud_browser` breaks backward compatibility for `BrowserProfile(cloud_browser=True)`. The parameter will be silently ignored due to `extra='ignore'` in model_config, causing `use_cloud` to remain at its default value instead of being set to True. Users expecting a cloud browser won't get one.</violation>
</file>
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:19">
P2: CloudBrowserResponse lacks `model_config`. Consider adding `model_config = ConfigDict(extra='forbid')` to reject unexpected API response fields, following the pattern used in `TabInfo` and other project models.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: This is a consistency/style suggestion rather than a bug. Looking at the same views.py file, PageInfo also lacks model_config, showing the pattern is not universally applied in the project. More importantly, for an API response model like CloudBrowserResponse, using `extra='forbid'` could be counterproductive: if the cloud API adds new fields in future versions, the client would fail to parse responses instead of gracefully ignoring unknown fields. This is a forward compatibility consideration that makes the current approach actually appropriate for this use case.</violation>
<violation number="2" location="browser_use/browser/cloud.py:24">
P3: Redundant Field aliases where alias equals field name. Pydantic uses field names by default, so `Field(alias='liveUrl')` on a field named `liveUrl` is unnecessary. Either remove the aliases or use snake_case field names with camelCase aliases for consistency.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: functionalImpact.*
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: This is a pure style/nitpick issue with zero functional impact. While technically correct that the aliases are redundant (Pydantic uses field names by default), the code works identically with or without these aliases. This is the kind of low-impact formatting/style suggestion that should not be flagged according to the review guidelines.</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:89">
P3: Incorrect capitalization: 'Set' should be lowercase in 'First set'</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| with pytest.raises(CloudBrowserAuthError) as exc_info: | ||
| await client.create_browser() | ||
|
|
||
| assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value) |
There was a problem hiding this comment.
P1: Assertion will always fail: the mock raises CloudBrowserAuthError('No auth token') but the assertion expects the message to contain 'BROWSER_USE_API_KEY environment variable'. Either update the mock's error message to match or fix the assertion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/ci/test_cloud_browser.py, line 100:
<comment>Assertion will always fail: the mock raises `CloudBrowserAuthError('No auth token')` but the assertion expects the message to contain `'BROWSER_USE_API_KEY environment variable'`. Either update the mock's error message to match or fix the assertion.</comment>
<file context>
@@ -0,0 +1,333 @@
+ with pytest.raises(CloudBrowserAuthError) as exc_info:
+ await client.create_browser()
+
+ assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value)
+
+ async def test_create_browser_http_401(self, mock_auth_config):
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| cdp_url: str | None = Field(default=None, description='CDP URL for connecting to existing browser instance') | ||
| is_local: bool = Field(default=False, description='Whether this is a local browser instance') | ||
| # label: str = 'default' | ||
| use_cloud: bool = Field( |
There was a problem hiding this comment.
P1: Read-only property cloud_browser breaks backward compatibility for BrowserProfile(cloud_browser=True). The parameter will be silently ignored due to extra='ignore' in model_config, causing use_cloud to remain at its default value instead of being set to True. Users expecting a cloud browser won't get one.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/profile.py, line 554:
<comment>Read-only property `cloud_browser` breaks backward compatibility for `BrowserProfile(cloud_browser=True)`. The parameter will be silently ignored due to `extra='ignore'` in model_config, causing `use_cloud` to remain at its default value instead of being set to True. Users expecting a cloud browser won't get one.</comment>
<file context>
@@ -550,7 +551,15 @@ class BrowserProfile(BrowserConnectArgs, BrowserLaunchPersistentContextArgs, Bro
cdp_url: str | None = Field(default=None, description='CDP URL for connecting to existing browser instance')
is_local: bool = Field(default=False, description='Whether this is a local browser instance')
- # label: str = 'default'
+ use_cloud: bool = Field(
+ default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')),
+ description='Use browser-use cloud browser service instead of local browser',
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
Fix confidence: 9/10
| use_cloud: bool = Field( | |
| use_cloud: bool = Field( | |
| default_factory=lambda: bool(os.getenv('BROWSER_USE_API_KEY')), | |
| description='Use browser-use cloud browser service instead of local browser', | |
| alias='cloud_browser', | |
| ) |
| with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_get_cdp_url: | ||
| mock_get_cdp_url.return_value = 'wss://test.proxy.daytona.works' | ||
|
|
||
| cdp_url = await mock_get_cdp_url() |
There was a problem hiding this comment.
P2: This test calls the mock object directly (await mock_get_cdp_url()) instead of calling code that would invoke the mocked function. It tests that the mock returns the configured value, not any actual functionality.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/ci/test_cloud_browser.py, line 253:
<comment>This test calls the mock object directly (`await mock_get_cdp_url()`) instead of calling code that would invoke the mocked function. It tests that the mock returns the configured value, not any actual functionality.</comment>
<file context>
@@ -0,0 +1,333 @@
+ with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_get_cdp_url:
+ mock_get_cdp_url.return_value = 'wss://test.proxy.daytona.works'
+
+ cdp_url = await mock_get_cdp_url()
+ assert cdp_url == 'wss://test.proxy.daytona.works'
+ mock_get_cdp_url.assert_called_once()
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| 'Authentication failed for cloud browser service. Set BROWSER_USE_API_KEY environment variable' | ||
| ) | ||
| except CloudBrowserError as e: | ||
| raise CloudBrowserError(f'Failed to create cloud browser: {e}') |
There was a problem hiding this comment.
P2: Re-raising CloudBrowserError without from e breaks the exception chain and loses the original traceback. Use raise CloudBrowserError(...) from e to preserve the exception chain.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/session.py, line 526:
<comment>Re-raising `CloudBrowserError` without `from e` breaks the exception chain and loses the original traceback. Use `raise CloudBrowserError(...) from e` to preserve the exception chain.</comment>
<file context>
@@ -496,9 +509,22 @@ async def on_BrowserStartEvent(self, event: BrowserStartEvent) -> dict[str, str]
+ 'Authentication failed for cloud browser service. Set BROWSER_USE_API_KEY environment variable'
+ )
+ except CloudBrowserError as e:
+ raise CloudBrowserError(f'Failed to create cloud browser: {e}')
+ elif self.is_local:
# Launch local browser using event-driven approach
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: General AI Review Agent
Fix confidence: 9/10
| raise CloudBrowserError(f'Failed to create cloud browser: {e}') | |
| raise CloudBrowserError(f'Failed to create cloud browser: {e}') from e |
| except CloudBrowserAuthError: | ||
| raise CloudBrowserAuthError( | ||
| 'Authentication failed for cloud browser service. Set BROWSER_USE_API_KEY environment variable' | ||
| ) |
There was a problem hiding this comment.
P2: Catching CloudBrowserAuthError without as e loses the original error message and traceback. The original exception may contain more specific information. Consider using raise to re-raise the original exception.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/session.py, line 521:
<comment>Catching `CloudBrowserAuthError` without `as e` loses the original error message and traceback. The original exception may contain more specific information. Consider using `raise` to re-raise the original exception.</comment>
<file context>
@@ -496,9 +509,22 @@ async def on_BrowserStartEvent(self, event: BrowserStartEvent) -> dict[str, str]
+ self.browser_profile.cdp_url = cloud_cdp_url
+ self.browser_profile.is_local = False
+ self.logger.info('🌤️ Successfully connected to cloud browser service')
+ except CloudBrowserAuthError:
+ raise CloudBrowserAuthError(
+ 'Authentication failed for cloud browser service. Set BROWSER_USE_API_KEY environment variable'
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
Fix confidence: 8/10
| except CloudBrowserAuthError: | |
| raise CloudBrowserAuthError( | |
| 'Authentication failed for cloud browser service. Set BROWSER_USE_API_KEY environment variable' | |
| ) | |
| except CloudBrowserAuthError: | |
| raise |
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class CloudBrowserResponse(BaseModel): |
There was a problem hiding this comment.
P2: CloudBrowserResponse lacks model_config. Consider adding model_config = ConfigDict(extra='forbid') to reject unexpected API response fields, following the pattern used in TabInfo and other project models.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: This is a consistency/style suggestion rather than a bug. Looking at the same views.py file, PageInfo also lacks model_config, showing the pattern is not universally applied in the project. More importantly, for an API response model like CloudBrowserResponse, using extra='forbid' could be counterproductive: if the cloud API adds new fields in future versions, the client would fail to parse responses instead of gracefully ignoring unknown fields. This is a forward compatibility consideration that makes the current approach actually appropriate for this use case.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/cloud.py, line 19:
<comment>CloudBrowserResponse lacks `model_config`. Consider adding `model_config = ConfigDict(extra='forbid')` to reject unexpected API response fields, following the pattern used in `TabInfo` and other project models.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: This is a consistency/style suggestion rather than a bug. Looking at the same views.py file, PageInfo also lacks model_config, showing the pattern is not universally applied in the project. More importantly, for an API response model like CloudBrowserResponse, using `extra='forbid'` could be counterproductive: if the cloud API adds new fields in future versions, the client would fail to parse responses instead of gracefully ignoring unknown fields. This is a forward compatibility consideration that makes the current approach actually appropriate for this use case.</comment>
<file context>
@@ -0,0 +1,286 @@
+logger = logging.getLogger(__name__)
+
+
+class CloudBrowserResponse(BaseModel):
+ """Response from cloud browser API."""
+
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: General AI Review Agent
|
|
||
| id: str | ||
| status: str | ||
| liveUrl: str = Field(alias='liveUrl') |
There was a problem hiding this comment.
P3: Redundant Field aliases where alias equals field name. Pydantic uses field names by default, so Field(alias='liveUrl') on a field named liveUrl is unnecessary. Either remove the aliases or use snake_case field names with camelCase aliases for consistency.
DEV MODE: This violation would have been filtered out by screening filters. Failing filters: functionalImpact.
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• GPT-5: This is a pure style/nitpick issue with zero functional impact. While technically correct that the aliases are redundant (Pydantic uses field names by default), the code works identically with or without these aliases. This is the kind of low-impact formatting/style suggestion that should not be flagged according to the review guidelines.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/cloud.py, line 24:
<comment>Redundant Field aliases where alias equals field name. Pydantic uses field names by default, so `Field(alias='liveUrl')` on a field named `liveUrl` is unnecessary. Either remove the aliases or use snake_case field names with camelCase aliases for consistency.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: functionalImpact.*
DEV MODE: This violation would have been filtered out by GPT-5.
Reasoning:
• **GPT-5**: This is a pure style/nitpick issue with zero functional impact. While technically correct that the aliases are redundant (Pydantic uses field names by default), the code works identically with or without these aliases. This is the kind of low-impact formatting/style suggestion that should not be flagged according to the review guidelines.</comment>
<file context>
@@ -0,0 +1,286 @@
+
+ id: str
+ status: str
+ liveUrl: str = Field(alias='liveUrl')
+ cdpUrl: str = Field(alias='cdpUrl')
+ timeoutAt: str = Field(alias='timeoutAt')
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: General AI Review Agent
| ) | ||
| ``` | ||
|
|
||
| First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com). |
There was a problem hiding this comment.
P3: Incorrect capitalization: 'Set' should be lowercase in 'First set'
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 89:
<comment>Incorrect capitalization: 'Set' should be lowercase in 'First set'</comment>
<file context>
@@ -74,6 +74,22 @@ OPENAI_API_KEY=
+)
+```
+
+First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com).
+
+For other settings, models, and more, check out the [Cloud documentation 📕](https://docs.cloud.browser-use.com).
</file context>
[internal] Confidence score: 10/10
[internal] Posted by: General AI Review Agent
Fix confidence: 10/10
| First Set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com). | |
| First set BROWSER_USE_API_KEY environment variable. You can get your API key from [here](https://cloud.browser-use.com). |
There was a problem hiding this comment.
6 issues found across 10 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="browser_use/dom/playground/extraction.py">
<violation number="1" location="browser_use/dom/playground/extraction.py:179">
P2: [Orchestrator] Invalid model name 'gpt-4.1-mini' will cause tiktoken.encoding_for_model() to raise KeyError. OpenAI has no model with this name - likely a typo for 'gpt-4o-mini'.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: futureIssue.*
• **Exa queries**: "tiktoken encoding_for_model supported model names"</violation>
</file>
<file name="browser_use/browser/cloud.py">
<violation number="1" location="browser_use/browser/cloud.py:109">
P2: [Orchestrator] Concurrent cloud browser sessions cannot be tracked independently. The global `_cloud_client` singleton stores only the most recent `current_session_id`, so when multiple `BrowserSession` instances use cloud browsers, earlier sessions lose their session ID reference and cannot properly stop their cloud browsers, causing resource leaks.</violation>
<violation number="2" location="browser_use/browser/cloud.py:112">
P2: [Orchestrator] Sensitive CDP and Live URLs containing session identifiers are logged at DEBUG level. These URLs effectively act as credentials for accessing cloud browser sessions. Consider logging only a masked portion (e.g., session ID prefix) or removing these debug statements to prevent potential session hijacking via log exposure.</violation>
</file>
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:848">
P1: [Orchestrator] Cloud browser cleanup does not clear the CDP URL. After `cleanup_cloud_client()` tears down the remote browser, the stale `cdp_url` is never cleared because the condition `if self.is_local:` evaluates to False for cloud sessions. On subsequent `start()` calls, the session attempts to reuse the dead WebSocket endpoint instead of provisioning a new cloud browser.</violation>
</file>
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:253">
P2: [Orchestrator] Test calls mock directly instead of production code - `await mock_get_cdp_url()` just invokes the mock object, not any code that uses `get_cloud_browser_cdp_url`. This test provides false coverage and will pass even if the integration is broken.</violation>
<violation number="2" location="tests/ci/test_cloud_browser.py:293">
P2: [Orchestrator] Mock is ineffective: patching `browser_use.browser.cloud.get_cloud_browser_cdp_url` doesn't affect the already-imported `get_cloud_browser_cdp_url` in this test module. The test runs the real implementation instead of the mock, passing only coincidentally because the real error also contains 'BROWSER_USE_API_KEY'. Fix by patching at the test module's namespace or calling the mock directly.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| from browser_use.browser.cloud import cleanup_cloud_client | ||
|
|
||
| await cleanup_cloud_client() | ||
| self.logger.info('🌤️ Cloud browser session cleaned up') |
There was a problem hiding this comment.
P1: [Orchestrator] Cloud browser cleanup does not clear the CDP URL. After cleanup_cloud_client() tears down the remote browser, the stale cdp_url is never cleared because the condition if self.is_local: evaluates to False for cloud sessions. On subsequent start() calls, the session attempts to reuse the dead WebSocket endpoint instead of provisioning a new cloud browser.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/session.py, line 848:
<comment>[Orchestrator] Cloud browser cleanup does not clear the CDP URL. After `cleanup_cloud_client()` tears down the remote browser, the stale `cdp_url` is never cleared because the condition `if self.is_local:` evaluates to False for cloud sessions. On subsequent `start()` calls, the session attempts to reuse the dead WebSocket endpoint instead of provisioning a new cloud browser.</comment>
<file context>
@@ -813,6 +839,16 @@ async def on_BrowserStopEvent(self, event: BrowserStopEvent) -> None:
+ from browser_use.browser.cloud import cleanup_cloud_client
+
+ await cleanup_cloud_client()
+ self.logger.info('🌤️ Cloud browser session cleaned up')
+ except Exception as e:
+ self.logger.debug(f'Failed to cleanup cloud browser session: {e}')
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Orchestrator AI Review Agent
Fix confidence: 9/10
| self.logger.info('🌤️ Cloud browser session cleaned up') | |
| self.logger.info('🌤️ Cloud browser session cleaned up') | |
| self.browser_profile.cdp_url = None |
| # pyperclip.copy(text_to_save) | ||
|
|
||
| encoding = tiktoken.encoding_for_model('gpt-4o') | ||
| encoding = tiktoken.encoding_for_model('gpt-4.1-mini') |
There was a problem hiding this comment.
P2: [Orchestrator] Invalid model name 'gpt-4.1-mini' will cause tiktoken.encoding_for_model() to raise KeyError. OpenAI has no model with this name - likely a typo for 'gpt-4o-mini'.
DEV MODE: This violation would have been filtered out by screening filters. Failing filters: futureIssue.
• Exa queries: "tiktoken encoding_for_model supported model names"
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/dom/playground/extraction.py, line 179:
<comment>[Orchestrator] Invalid model name 'gpt-4.1-mini' will cause tiktoken.encoding_for_model() to raise KeyError. OpenAI has no model with this name - likely a typo for 'gpt-4o-mini'.
*DEV MODE: This violation would have been filtered out by screening filters. Failing filters: futureIssue.*
• **Exa queries**: "tiktoken encoding_for_model supported model names"</comment>
<file context>
@@ -176,7 +176,7 @@ def get_website_list_for_prompt() -> str:
# pyperclip.copy(text_to_save)
- encoding = tiktoken.encoding_for_model('gpt-4o')
+ encoding = tiktoken.encoding_for_model('gpt-4.1-mini')
token_count = len(encoding.encode(text_to_save))
print(f'Token count: {token_count}')
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Orchestrator AI Review Agent
Fix confidence: 9/10
| encoding = tiktoken.encoding_for_model('gpt-4.1-mini') | |
| encoding = tiktoken.encoding_for_model('gpt-4o-mini') |
| @@ -0,0 +1,286 @@ | |||
| """Cloud browser service integration for browser-use. | |||
There was a problem hiding this comment.
P2: [Orchestrator] Concurrent cloud browser sessions cannot be tracked independently. The global _cloud_client singleton stores only the most recent current_session_id, so when multiple BrowserSession instances use cloud browsers, earlier sessions lose their session ID reference and cannot properly stop their cloud browsers, causing resource leaks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/cloud.py, line 109:
<comment>[Orchestrator] Concurrent cloud browser sessions cannot be tracked independently. The global `_cloud_client` singleton stores only the most recent `current_session_id`, so when multiple `BrowserSession` instances use cloud browsers, earlier sessions lose their session ID reference and cannot properly stop their cloud browsers, causing resource leaks.</comment>
<file context>
@@ -0,0 +1,286 @@
+ browser_response = CloudBrowserResponse(**browser_data)
+
+ # Store session ID for cleanup
+ self.current_session_id = browser_response.id
+
+ logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}')
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Orchestrator AI Review Agent
| self.current_session_id = browser_response.id | ||
|
|
||
| logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}') | ||
| logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}') |
There was a problem hiding this comment.
P2: [Orchestrator] Sensitive CDP and Live URLs containing session identifiers are logged at DEBUG level. These URLs effectively act as credentials for accessing cloud browser sessions. Consider logging only a masked portion (e.g., session ID prefix) or removing these debug statements to prevent potential session hijacking via log exposure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/cloud.py, line 112:
<comment>[Orchestrator] Sensitive CDP and Live URLs containing session identifiers are logged at DEBUG level. These URLs effectively act as credentials for accessing cloud browser sessions. Consider logging only a masked portion (e.g., session ID prefix) or removing these debug statements to prevent potential session hijacking via log exposure.</comment>
<file context>
@@ -0,0 +1,286 @@
+ self.current_session_id = browser_response.id
+
+ logger.info(f'🌤️ Cloud browser created successfully: {browser_response.id}')
+ logger.debug(f'🌤️ CDP URL: {browser_response.cdpUrl}')
+ logger.debug(f'🌤️ Live URL: {browser_response.liveUrl}')
+
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Orchestrator AI Review Agent
| profile = BrowserProfile(use_cloud=True) | ||
|
|
||
| # Test that cloud browser client raises error without fallback | ||
| with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_cloud_cdp: |
There was a problem hiding this comment.
P2: [Orchestrator] Mock is ineffective: patching browser_use.browser.cloud.get_cloud_browser_cdp_url doesn't affect the already-imported get_cloud_browser_cdp_url in this test module. The test runs the real implementation instead of the mock, passing only coincidentally because the real error also contains 'BROWSER_USE_API_KEY'. Fix by patching at the test module's namespace or calling the mock directly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/ci/test_cloud_browser.py, line 293:
<comment>[Orchestrator] Mock is ineffective: patching `browser_use.browser.cloud.get_cloud_browser_cdp_url` doesn't affect the already-imported `get_cloud_browser_cdp_url` in this test module. The test runs the real implementation instead of the mock, passing only coincidentally because the real error also contains 'BROWSER_USE_API_KEY'. Fix by patching at the test module's namespace or calling the mock directly.</comment>
<file context>
@@ -0,0 +1,333 @@
+ profile = BrowserProfile(use_cloud=True)
+
+ # Test that cloud browser client raises error without fallback
+ with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_cloud_cdp:
+ mock_cloud_cdp.side_effect = CloudBrowserAuthError('No auth token')
+
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Orchestrator AI Review Agent
| with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_get_cdp_url: | ||
| mock_get_cdp_url.return_value = 'wss://test.proxy.daytona.works' | ||
|
|
||
| cdp_url = await mock_get_cdp_url() |
There was a problem hiding this comment.
P2: [Orchestrator] Test calls mock directly instead of production code - await mock_get_cdp_url() just invokes the mock object, not any code that uses get_cloud_browser_cdp_url. This test provides false coverage and will pass even if the integration is broken.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/ci/test_cloud_browser.py, line 253:
<comment>[Orchestrator] Test calls mock directly instead of production code - `await mock_get_cdp_url()` just invokes the mock object, not any code that uses `get_cloud_browser_cdp_url`. This test provides false coverage and will pass even if the integration is broken.</comment>
<file context>
@@ -0,0 +1,333 @@
+ with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_get_cdp_url:
+ mock_get_cdp_url.return_value = 'wss://test.proxy.daytona.works'
+
+ cdp_url = await mock_get_cdp_url()
+ assert cdp_url == 'wss://test.proxy.daytona.works'
+ mock_get_cdp_url.assert_called_once()
</file context>
[internal] Confidence score: 9/10
[internal] Posted by: Orchestrator AI Review Agent
|
@cubic-dev-local-sanxroz review this |
|
@cubic-dev-local-sanxroz review this |
@sanxroz I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 10 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:298">
P2: [Orchestrator] Mock is applied to the module attribute, but the test calls the locally imported get_cloud_browser_cdp_url, so the real function runs and the mock never takes effect, making the test brittle and not exercising the intended error path.</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.
|
|
||
| # Verify that the cloud browser client raises the expected error | ||
| with pytest.raises(CloudBrowserAuthError) as exc_info: | ||
| await get_cloud_browser_cdp_url() |
There was a problem hiding this comment.
P2: [Orchestrator] Mock is applied to the module attribute, but the test calls the locally imported get_cloud_browser_cdp_url, so the real function runs and the mock never takes effect, making the test brittle and not exercising the intended error path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/ci/test_cloud_browser.py, line 298:
<comment>[Orchestrator] Mock is applied to the module attribute, but the test calls the locally imported get_cloud_browser_cdp_url, so the real function runs and the mock never takes effect, making the test brittle and not exercising the intended error path.</comment>
<file context>
@@ -0,0 +1,333 @@
+
+ # Verify that the cloud browser client raises the expected error
+ with pytest.raises(CloudBrowserAuthError) as exc_info:
+ await get_cloud_browser_cdp_url()
+
+ assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value)
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Orchestrator AI Review Agent
|
@cubic-dev-local-sanxroz review this |
1 similar comment
|
@cubic-dev-local-sanxroz review this |
@sanxroz I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 10 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:514">
P2: [Orchestrator] Cloud browser stop cleans up the remote session but leaves the cached CDP URL intact, so the next start skips provisioning and tries to reconnect to a stopped cloud session.</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.
| # If no CDP URL, launch local browser or cloud browser | ||
| if not self.cdp_url: | ||
| if self.is_local: | ||
| if self.browser_profile.use_cloud: |
There was a problem hiding this comment.
P2: [Orchestrator] Cloud browser stop cleans up the remote session but leaves the cached CDP URL intact, so the next start skips provisioning and tries to reconnect to a stopped cloud session.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/session.py, line 514:
<comment>[Orchestrator] Cloud browser stop cleans up the remote session but leaves the cached CDP URL intact, so the next start skips provisioning and tries to reconnect to a stopped cloud session.</comment>
<file context>
@@ -496,9 +509,22 @@ async def on_BrowserStartEvent(self, event: BrowserStartEvent) -> dict[str, str]
+ # If no CDP URL, launch local browser or cloud browser
if not self.cdp_url:
- if self.is_local:
+ if self.browser_profile.use_cloud:
+ # Use cloud browser service
+ try:
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: Orchestrator AI Review Agent
|
@cubic-dev-local-sanxroz review this |
@sanxroz I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 10 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="browser_use/browser/session.py">
<violation number="1" location="browser_use/browser/session.py:847">
P2: [Orchestrator] cleanup_cloud_client() operates on a global cloud client that only tracks a single current session. Calling it on every cloud session stop can stop another active cloud session if multiple BrowserSession instances run concurrently.</violation>
</file>
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:293">
P2: [Orchestrator] Patch targets the original module but the test calls the locally imported function, so the mock isn’t applied; the real networked function can run depending on env vars, making the test nondeterministic.</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.
| try: | ||
| from browser_use.browser.cloud import cleanup_cloud_client | ||
|
|
||
| await cleanup_cloud_client() |
There was a problem hiding this comment.
P2: [Orchestrator] cleanup_cloud_client() operates on a global cloud client that only tracks a single current session. Calling it on every cloud session stop can stop another active cloud session if multiple BrowserSession instances run concurrently.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At browser_use/browser/session.py, line 847:
<comment>[Orchestrator] cleanup_cloud_client() operates on a global cloud client that only tracks a single current session. Calling it on every cloud session stop can stop another active cloud session if multiple BrowserSession instances run concurrently.</comment>
<file context>
@@ -813,6 +839,16 @@ async def on_BrowserStopEvent(self, event: BrowserStopEvent) -> None:
+ try:
+ from browser_use.browser.cloud import cleanup_cloud_client
+
+ await cleanup_cloud_client()
+ self.logger.info('🌤️ Cloud browser session cleaned up')
+ except Exception as e:
</file context>
[internal] Confidence score: 7/10
[internal] Posted by: Orchestrator AI Review Agent
| profile = BrowserProfile(use_cloud=True) | ||
|
|
||
| # Test that cloud browser client raises error without fallback | ||
| with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_cloud_cdp: |
There was a problem hiding this comment.
P2: [Orchestrator] Patch targets the original module but the test calls the locally imported function, so the mock isn’t applied; the real networked function can run depending on env vars, making the test nondeterministic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/ci/test_cloud_browser.py, line 293:
<comment>[Orchestrator] Patch targets the original module but the test calls the locally imported function, so the mock isn’t applied; the real networked function can run depending on env vars, making the test nondeterministic.</comment>
<file context>
@@ -0,0 +1,333 @@
+ profile = BrowserProfile(use_cloud=True)
+
+ # Test that cloud browser client raises error without fallback
+ with patch('browser_use.browser.cloud.get_cloud_browser_cdp_url') as mock_cloud_cdp:
+ mock_cloud_cdp.side_effect = CloudBrowserAuthError('No auth token')
+
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Orchestrator AI Review Agent
|
@cubic-dev-local-sanxroz review this |
@sanxroz I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-local-sanxroz review this |
@sanxroz I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-local-sanxroz review this |
@sanxroz I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-local-sanxroz review this |
@sanxroz I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-local-sanxroz review this |
@sanxroz I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-local-sanxroz review this |
@sanxroz I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 10 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="tests/ci/test_cloud_browser.py">
<violation number="1" location="tests/ci/test_cloud_browser.py:100">
P2: [Orchestrator] Test configures the mock to raise 'No auth token' but asserts a different error message, so the test will fail once the mock is invoked.</violation>
<violation number="2" location="tests/ci/test_cloud_browser.py:298">
P2: [Orchestrator] Patched get_cloud_browser_cdp_url is not used; the test calls the already-imported local name, so the mock’s side_effect never triggers and the real function may run.</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.
| with pytest.raises(CloudBrowserAuthError) as exc_info: | ||
| await client.create_browser() | ||
|
|
||
| assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value) |
There was a problem hiding this comment.
P2: [Orchestrator] Test configures the mock to raise 'No auth token' but asserts a different error message, so the test will fail once the mock is invoked.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/ci/test_cloud_browser.py, line 100:
<comment>[Orchestrator] Test configures the mock to raise 'No auth token' but asserts a different error message, so the test will fail once the mock is invoked.</comment>
<file context>
@@ -0,0 +1,333 @@
+ with pytest.raises(CloudBrowserAuthError) as exc_info:
+ await client.create_browser()
+
+ assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value)
+
+ async def test_create_browser_http_401(self, mock_auth_config):
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Orchestrator AI Review Agent
|
|
||
| # Verify that the cloud browser client raises the expected error | ||
| with pytest.raises(CloudBrowserAuthError) as exc_info: | ||
| await get_cloud_browser_cdp_url() |
There was a problem hiding this comment.
P2: [Orchestrator] Patched get_cloud_browser_cdp_url is not used; the test calls the already-imported local name, so the mock’s side_effect never triggers and the real function may run.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/ci/test_cloud_browser.py, line 298:
<comment>[Orchestrator] Patched get_cloud_browser_cdp_url is not used; the test calls the already-imported local name, so the mock’s side_effect never triggers and the real function may run.</comment>
<file context>
@@ -0,0 +1,333 @@
+
+ # Verify that the cloud browser client raises the expected error
+ with pytest.raises(CloudBrowserAuthError) as exc_info:
+ await get_cloud_browser_cdp_url()
+
+ assert 'BROWSER_USE_API_KEY environment variable' in str(exc_info.value)
</file context>
[internal] Confidence score: 8/10
[internal] Posted by: Orchestrator AI Review Agent
|
@cubic-dev-local-sanxroz review this |
@sanxroz I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-local-sanxroz review this |
@sanxroz I have started the AI code review. It will take a few minutes to complete. |
3 participants
Summary by cubic
Add cloud_browser support to run agents on the Browser-Use Cloud. It provisions a cloud browser on demand, connects via CDP, and cleans up sessions automatically.
New Features
Migration