From acb3f0efb4fe40a84eb05a6a9938bb6504e90dbc Mon Sep 17 00:00:00 2001 From: Octopus Date: Fri, 3 Apr 2026 13:40:43 +0800 Subject: [PATCH 01/27] fix: detect browser process exit early in _wait_for_cdp_url (fixes #4471) When Chrome fails to start on headless Linux (e.g. due to missing sandbox capabilities, missing display, or missing system dependencies), the previous code would silently poll the CDP endpoint for the full 30-second timeout before raising a generic TimeoutError, giving no hint about the actual cause. This change adds an optional process parameter to _wait_for_cdp_url. When provided, the polling loop checks whether the browser process is still alive on each iteration and raises a descriptive RuntimeError immediately if it has exited - cutting the wait from 30s to <0.1s and surfacing a message that points users toward the likely fix (--no-sandbox, Xvfb, etc.). Also replaces the deprecated asyncio.get_event_loop() calls in this async method with asyncio.get_running_loop() per Python 3.10+ best practice. --- .../watchdogs/local_browser_watchdog.py | 39 ++++++++++++++++--- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/browser_use/browser/watchdogs/local_browser_watchdog.py b/browser_use/browser/watchdogs/local_browser_watchdog.py index 9f2c3d30c..17d2ced5c 100644 --- a/browser_use/browser/watchdogs/local_browser_watchdog.py +++ b/browser_use/browser/watchdogs/local_browser_watchdog.py @@ -157,7 +157,7 @@ class LocalBrowserWatchdog(BaseWatchdog): process = psutil.Process(subprocess.pid) # Wait for CDP to be ready and get the URL - cdp_url = await self._wait_for_cdp_url(debug_port) + cdp_url = await self._wait_for_cdp_url(debug_port, process=process) # Success! Clean up only the temp dirs we created but didn't use currently_used_dir = str(profile.user_data_dir) @@ -405,13 +405,42 @@ class LocalBrowserWatchdog(BaseWatchdog): return port @staticmethod - async def _wait_for_cdp_url(port: int, timeout: float = 30) -> str: - """Wait for the browser to start and return the CDP URL.""" + async def _wait_for_cdp_url(port: int, timeout: float = 30, process: psutil.Process | None = None) -> str: + """Wait for the browser to start and return the CDP URL. + + Args: + port: The local port Chrome is listening on for CDP. + timeout: Maximum seconds to wait before raising TimeoutError. + process: Optional psutil.Process for the browser subprocess. If provided, + the loop will fail fast with a descriptive error if the process + exits before CDP becomes available (e.g. due to missing sandbox + capabilities or a missing display on headless Linux). + """ import aiohttp - start_time = asyncio.get_event_loop().time() + start_time = asyncio.get_running_loop().time() + + while asyncio.get_running_loop().time() - start_time < timeout: + # Fail fast if the browser process has already exited + if process is not None: + try: + if not process.is_running(): + raise RuntimeError( + f'Browser process (PID {process.pid}) exited before CDP became available on port {port}. ' + 'This usually means Chrome failed to start — check that it is properly installed and ' + 'all required system dependencies are present (e.g. --no-sandbox may be needed in ' + 'Docker/headless environments, or a virtual display such as Xvfb on headless Linux).' + ) + except psutil.NoSuchProcess: + raise RuntimeError( + f'Browser process (PID {process.pid}) exited before CDP became available on port {port}. ' + 'This usually means Chrome failed to start — check that it is properly installed and ' + 'all required system dependencies are present (e.g. --no-sandbox may be needed in ' + 'Docker/headless environments, or a virtual display such as Xvfb on headless Linux).' + ) + except psutil.AccessDenied: + pass # Cannot check process status; continue polling CDP - while asyncio.get_event_loop().time() - start_time < timeout: try: async with aiohttp.ClientSession() as session: async with session.get(f'http://127.0.0.1:{port}/json/version') as resp: From 554ce8f99e83a9c23e910962dac75d720dc9c72f Mon Sep 17 00:00:00 2001 From: lisa0314 Date: Wed, 15 Jul 2026 10:07:05 +0800 Subject: [PATCH 02/27] Restore session handlers after event bus reset --- browser_use/browser/session.py | 8 +++++++- tests/ci/browser/test_session_start.py | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/browser_use/browser/session.py b/browser_use/browser/session.py index c22c487a3..3609c05dc 100644 --- a/browser_use/browser/session.py +++ b/browser_use/browser/session.py @@ -686,11 +686,15 @@ class BrowserSession(BaseModel): self.logger.info('✅ Browser session reset complete') def model_post_init(self, __context) -> None: - """Register event handlers after model initialization.""" + """Initialize runtime state and register event handlers.""" self._connection_lock = asyncio.Lock() # Initialize reconnect event as set (no reconnection pending) self._reconnect_event = asyncio.Event() self._reconnect_event.set() + self._register_session_event_handlers() + + def _register_session_event_handlers(self) -> None: + """Register BrowserSession handlers on the current event bus.""" # Check if handlers are already registered to prevent duplicates from browser_use.browser.watchdog_base import BaseWatchdog @@ -742,6 +746,7 @@ class BrowserSession(BaseModel): await self.reset() # Create fresh event bus self.event_bus = ResilientEventBus() + self._register_session_event_handlers() async def stop(self) -> None: """Stop the browser session without killing the browser process. @@ -767,6 +772,7 @@ class BrowserSession(BaseModel): await self.reset() # Create fresh event bus self.event_bus = ResilientEventBus() + self._register_session_event_handlers() async def close(self) -> None: """Alias for stop().""" diff --git a/tests/ci/browser/test_session_start.py b/tests/ci/browser/test_session_start.py index 4ccce869d..058b6a4ea 100644 --- a/tests/ci/browser/test_session_start.py +++ b/tests/ci/browser/test_session_start.py @@ -246,6 +246,26 @@ class TestBrowserSessionEventSystem: assert browser_session.event_bus.name.startswith('EventBus_') # Event bus name format may vary, just check it exists + @pytest.mark.parametrize('reset_method', ['stop', 'kill']) + async def test_session_handlers_registered_after_event_bus_reset(self, browser_session: BrowserSession, reset_method: str): + """Session handlers must be restored when stop() or kill() replaces the event bus.""" + initial_bus = browser_session.event_bus + initial_handlers = { + event_name: [getattr(handler, '__name__', str(handler)) for handler in handlers] + for event_name, handlers in initial_bus.handlers.items() + } + assert initial_handlers + assert 'BrowserStartEvent' in initial_handlers + assert 'BrowserStopEvent' in initial_handlers + + await getattr(browser_session, reset_method)() + + assert browser_session.event_bus is not initial_bus + assert { + event_name: [getattr(handler, '__name__', str(handler)) for handler in handlers] + for event_name, handlers in browser_session.event_bus.handlers.items() + } == initial_handlers + async def test_event_handlers_registration(self, browser_session: BrowserSession): """Test that event handlers are properly registered.""" # Attach all watchdogs to register their handlers From c45b850231411b9fae07e8ca9934da01c98c9663 Mon Sep 17 00:00:00 2001 From: Faseeh Date: Wed, 19 Aug 2026 22:46:24 +0500 Subject: [PATCH 03/27] fix(tools): report failed tab switches as errors instead of silent success switch() returned a non-error ActionResult on both of its failure paths (a stale/unknown tab_id, and a SwitchTabEvent that produced no result), so callers had no way to tell a failed switch from a real one. The false 'Switched to tab #...' claim was written into long_term_memory, so subsequent agent steps reasoned from a tab that was never actually reached. Raise BrowserError on both failure paths instead, following the same convention upload_file already uses in this file. The error message preserves the actual underlying cause instead of a generic string, so ActionResult.error carries actionable information back to the agent. Adds regression tests covering the failing tab_id case and confirming the happy path is unaffected. --- browser_use/tools/service.py | 28 ++++++++++++-------- tests/ci/browser/test_tabs.py | 48 +++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/browser_use/tools/service.py b/browser_use/tools/service.py index 2eb65e809..915690ff1 100644 --- a/browser_use/tools/service.py +++ b/browser_use/tools/service.py @@ -1014,18 +1014,24 @@ class Tools(Generic[Context]): event = browser_session.event_bus.dispatch(SwitchTabEvent(target_id=target_id)) await event new_target_id = await event.event_result(raise_if_any=False, raise_if_none=False) # Don't raise on errors - - if new_target_id: - memory = f'Switched to tab #{new_target_id[-4:]}' - else: - memory = f'Switched to tab #{params.tab_id}' - - logger.info(f'🔄 {memory}') - return ActionResult(extracted_content=memory, long_term_memory=memory) except Exception as e: - logger.warning(f'Tab switch may have failed: {e}') - memory = f'Attempted to switch to tab #{params.tab_id}' - return ActionResult(extracted_content=memory, long_term_memory=memory) + logger.warning(f'Tab switch failed: {e}') + # Preserve the concrete cause (e.g. a stale tab_id) instead of a generic + # message, so the agent gets actionable failure info in both memories. + memory = f'Failed to switch to tab #{params.tab_id}: {e}' + raise BrowserError(memory, short_term_memory=memory, long_term_memory=memory) + + # on_SwitchTabEvent returns the newly focused TargetID on every success path, so a + # missing result means the handler failed rather than having quietly succeeded + # (raise_if_any=False above only suppresses the exception, it doesn't mean success). + if not new_target_id: + memory = f'Failed to switch to tab #{params.tab_id}: tab switch produced no result' + logger.warning(memory) + raise BrowserError(memory, short_term_memory=memory, long_term_memory=memory) + + memory = f'Switched to tab #{new_target_id[-4:]}' + logger.info(f'🔄 {memory}') + return ActionResult(extracted_content=memory, long_term_memory=memory) @self.registry.action( 'Close a tab by tab_id. Tab IDs are shown in browser state tabs list (last 4 chars of target_id). Use to clean up tabs you no longer need.', diff --git a/tests/ci/browser/test_tabs.py b/tests/ci/browser/test_tabs.py index a0372b92a..e08633f6b 100644 --- a/tests/ci/browser/test_tabs.py +++ b/tests/ci/browser/test_tabs.py @@ -26,6 +26,7 @@ from pytest_httpserver import HTTPServer from browser_use.agent.service import Agent from browser_use.browser import BrowserSession from browser_use.browser.profile import BrowserProfile +from browser_use.tools.service import Tools from tests.ci.conftest import create_mock_llm @@ -669,3 +670,50 @@ class TestMultiTabOperations: assert 'Successfully' in final_result, 'Agent should report success' except TimeoutError: pytest.fail('Test timed out after 2 minutes - agent hung during multiple tab operations') + + +class TestSwitchTabFailureReporting: + """A failed `switch` must surface as ActionResult.error rather than a fake success. + + Previously both failure paths (a stale/unknown tab_id, and a missing SwitchTabEvent + result) returned a non-error ActionResult claiming the switch succeeded. The false + claim was written into long_term_memory, so subsequent steps reasoned from a tab the + agent never actually reached. + """ + + async def test_switch_to_nonexistent_tab_reports_error(self, browser_session): + tools = Tools() + ActionModel = tools.registry.create_action_model() + + tabs_before = await browser_session.get_tabs() + live_ids = {tab.target_id[-4:] for tab in tabs_before} + bogus_tab_id = 'zzzz' + assert bogus_tab_id not in live_ids + + result = await tools.act(ActionModel(switch={'tab_id': bogus_tab_id}), browser_session=browser_session) + + assert result.error is not None, 'a failed tab switch must set ActionResult.error' + assert bogus_tab_id in result.error + assert 'Switched to tab' not in (result.extracted_content or '') + assert 'Switched to tab' not in (result.long_term_memory or '') + + tabs_after = await browser_session.get_tabs() + assert {tab.target_id[-4:] for tab in tabs_after} == live_ids, 'no tab switch should have happened' + + async def test_switch_to_open_tab_still_succeeds(self, browser_session, base_url): + tools = Tools() + ActionModel = tools.registry.create_action_model() + + original_tab_id = (await browser_session.get_tabs())[0].target_id[-4:] + + open_result = await tools.act( + ActionModel(navigate={'url': f'{base_url}/page1', 'new_tab': True}), + browser_session=browser_session, + ) + assert open_result.error is None, f'opening a new tab should not error: {open_result.error}' + + result = await tools.act(ActionModel(switch={'tab_id': original_tab_id}), browser_session=browser_session) + + assert result.error is None, f'switching to a live tab must not error: {result.error}' + assert result.long_term_memory is not None + assert 'Switched to tab' in result.long_term_memory From c4e314072efd077a6033c98664b9cf772debf6c8 Mon Sep 17 00:00:00 2001 From: Faseeh Date: Wed, 19 Aug 2026 23:02:33 +0500 Subject: [PATCH 04/27] address review feedback: preserve real cause via raise_if_any, fix pyright - event_result(raise_if_any=True) so a SwitchTabEvent handler failure surfaces its actual cause through the except block instead of becoming a generic 'produced no result' message (only one handler is ever registered for SwitchTabEvent, so this is safe). - Test file used a dynamically-built ActionModel that pyright can't see the fields of; switched to a statically declared _TabActionModel subclass, matching the pattern used elsewhere in this file. --- browser_use/tools/service.py | 8 +++++--- tests/ci/browser/test_tabs.py | 22 +++++++++++++++++----- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/browser_use/tools/service.py b/browser_use/tools/service.py index 915690ff1..9898c7ccf 100644 --- a/browser_use/tools/service.py +++ b/browser_use/tools/service.py @@ -1013,7 +1013,9 @@ class Tools(Generic[Context]): event = browser_session.event_bus.dispatch(SwitchTabEvent(target_id=target_id)) await event - new_target_id = await event.event_result(raise_if_any=False, raise_if_none=False) # Don't raise on errors + # raise_if_any=True so a handler failure surfaces its real cause here instead + # of silently becoming a "produced no result" below. + new_target_id = await event.event_result(raise_if_any=True, raise_if_none=False) except Exception as e: logger.warning(f'Tab switch failed: {e}') # Preserve the concrete cause (e.g. a stale tab_id) instead of a generic @@ -1022,8 +1024,8 @@ class Tools(Generic[Context]): raise BrowserError(memory, short_term_memory=memory, long_term_memory=memory) # on_SwitchTabEvent returns the newly focused TargetID on every success path, so a - # missing result means the handler failed rather than having quietly succeeded - # (raise_if_any=False above only suppresses the exception, it doesn't mean success). + # missing result (with no exception raised) means the handler still failed rather + # than having quietly succeeded. if not new_target_id: memory = f'Failed to switch to tab #{params.tab_id}: tab switch produced no result' logger.warning(memory) diff --git a/tests/ci/browser/test_tabs.py b/tests/ci/browser/test_tabs.py index e08633f6b..a38124248 100644 --- a/tests/ci/browser/test_tabs.py +++ b/tests/ci/browser/test_tabs.py @@ -19,11 +19,13 @@ Usage: import asyncio import time +from typing import Any import pytest from pytest_httpserver import HTTPServer from browser_use.agent.service import Agent +from browser_use.agent.views import ActionModel from browser_use.browser import BrowserSession from browser_use.browser.profile import BrowserProfile from browser_use.tools.service import Tools @@ -672,6 +674,18 @@ class TestMultiTabOperations: pytest.fail('Test timed out after 2 minutes - agent hung during multiple tab operations') +class _TabActionModel(ActionModel): + """ActionModel with explicit slots for the tab actions driven directly via tools.act(). + + registry.create_action_model() builds its fields at runtime, so a statically declared + subclass is what keeps pyright able to check these call sites. act() dispatches on the + key returned by model_dump(exclude_unset=True), so the real registered actions still run. + """ + + switch: dict[str, Any] | None = None + navigate: dict[str, Any] | None = None + + class TestSwitchTabFailureReporting: """A failed `switch` must surface as ActionResult.error rather than a fake success. @@ -683,14 +697,13 @@ class TestSwitchTabFailureReporting: async def test_switch_to_nonexistent_tab_reports_error(self, browser_session): tools = Tools() - ActionModel = tools.registry.create_action_model() tabs_before = await browser_session.get_tabs() live_ids = {tab.target_id[-4:] for tab in tabs_before} bogus_tab_id = 'zzzz' assert bogus_tab_id not in live_ids - result = await tools.act(ActionModel(switch={'tab_id': bogus_tab_id}), browser_session=browser_session) + result = await tools.act(_TabActionModel(switch={'tab_id': bogus_tab_id}), browser_session=browser_session) assert result.error is not None, 'a failed tab switch must set ActionResult.error' assert bogus_tab_id in result.error @@ -702,17 +715,16 @@ class TestSwitchTabFailureReporting: async def test_switch_to_open_tab_still_succeeds(self, browser_session, base_url): tools = Tools() - ActionModel = tools.registry.create_action_model() original_tab_id = (await browser_session.get_tabs())[0].target_id[-4:] open_result = await tools.act( - ActionModel(navigate={'url': f'{base_url}/page1', 'new_tab': True}), + _TabActionModel(navigate={'url': f'{base_url}/page1', 'new_tab': True}), browser_session=browser_session, ) assert open_result.error is None, f'opening a new tab should not error: {open_result.error}' - result = await tools.act(ActionModel(switch={'tab_id': original_tab_id}), browser_session=browser_session) + result = await tools.act(_TabActionModel(switch={'tab_id': original_tab_id}), browser_session=browser_session) assert result.error is None, f'switching to a live tab must not error: {result.error}' assert result.long_term_memory is not None From cbc816abb37feedf05a7f67b8342ae3b96c1398e Mon Sep 17 00:00:00 2001 From: Faseeh Date: Thu, 20 Aug 2026 19:23:54 +0500 Subject: [PATCH 05/27] test: cover the 'switch event yields no result' failure path Reviewer feedback noted the second fixed failure branch (a SwitchTabEvent that completes without raising but yields no TargetID) had no dedicated coverage. Adds a test using the same event_bus.dispatch monkeypatch pattern already used elsewhere in this test suite. --- tests/ci/browser/test_tabs.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/ci/browser/test_tabs.py b/tests/ci/browser/test_tabs.py index a38124248..eb54c3bfc 100644 --- a/tests/ci/browser/test_tabs.py +++ b/tests/ci/browser/test_tabs.py @@ -27,6 +27,7 @@ from pytest_httpserver import HTTPServer from browser_use.agent.service import Agent from browser_use.agent.views import ActionModel from browser_use.browser import BrowserSession +from browser_use.browser.events import SwitchTabEvent from browser_use.browser.profile import BrowserProfile from browser_use.tools.service import Tools from tests.ci.conftest import create_mock_llm @@ -729,3 +730,36 @@ class TestSwitchTabFailureReporting: assert result.error is None, f'switching to a live tab must not error: {result.error}' assert result.long_term_memory is not None assert 'Switched to tab' in result.long_term_memory + + async def test_switch_reports_error_when_event_yields_no_result(self, browser_session, monkeypatch): + """A handler that completes without raising but yields no TargetID is still a failure. + + on_SwitchTabEvent returns a TargetID on every success path, so a missing result is + never a quiet success - this must be reported as an error even though nothing raised. + """ + tools = Tools() + tab_id = (await browser_session.get_tabs())[0].target_id[-4:] + + class NoResultEvent: + async def _wait(self): + return self + + def __await__(self): + return self._wait().__await__() + + async def event_result(self, **_kwargs): + return None + + original_dispatch = browser_session.event_bus.dispatch + monkeypatch.setattr( + browser_session.event_bus, + 'dispatch', + lambda event: NoResultEvent() if isinstance(event, SwitchTabEvent) else original_dispatch(event), + ) + + result = await tools.act(_TabActionModel(switch={'tab_id': tab_id}), browser_session=browser_session) + + assert result.error is not None, 'a switch that yields no result must set ActionResult.error' + assert 'produced no result' in result.error + assert 'Switched to tab' not in (result.extracted_content or '') + assert 'Switched to tab' not in (result.long_term_memory or '') From 0a6e427c6f1a9345833fe4f1f85b3fa1afecc0fa Mon Sep 17 00:00:00 2001 From: IENVYshanks Date: Fri, 28 Aug 2026 11:25:30 +0530 Subject: [PATCH 06/27] fix(browser): support literal plus in send_keys --- .../watchdogs/default_action_watchdog.py | 47 +++++----- .../ci/browser/test_send_keys_literal_plus.py | 87 +++++++++++++++++++ 2 files changed, 114 insertions(+), 20 deletions(-) create mode 100644 tests/ci/browser/test_send_keys_literal_plus.py diff --git a/browser_use/browser/watchdogs/default_action_watchdog.py b/browser_use/browser/watchdogs/default_action_watchdog.py index e38777e9d..805b5b43a 100644 --- a/browser_use/browser/watchdogs/default_action_watchdog.py +++ b/browser_use/browser/watchdogs/default_action_watchdog.py @@ -2504,31 +2504,35 @@ class DefaultActionWatchdog(BaseWatchdog): 'end': 'End', } - # Parse and normalize the key string keys = event.keys - if '+' in keys: - # Handle key combinations like "ctrl+a" - parts = keys.split('+') - normalized_parts = [] - for part in parts: - part_lower = part.strip().lower() - normalized = key_aliases.get(part_lower, part) - normalized_parts.append(normalized) - normalized_keys = '+'.join(normalized_parts) - else: - # Single key - keys_lower = keys.strip().lower() - normalized_keys = key_aliases.get(keys_lower, keys) + modifier_map = {'Alt': 1, 'Control': 2, 'Meta': 4, 'Shift': 8} + is_combination = False + modifiers = [] + main_key = None + if '+' in keys and keys != '+': + if keys.endswith('++'): + prefix = keys[:-2] + raw_modifiers = prefix.split('+') + if all(part.strip() for part in raw_modifiers): + normalized_modifiers = [key_aliases.get(part.strip().lower(), part) for part in raw_modifiers] + if all(modifier in modifier_map for modifier in normalized_modifiers): + is_combination = True + modifiers = normalized_modifiers + main_key = '+' + else: + prefix, suffix = keys.rsplit('+', 1) + raw_modifiers = prefix.split('+') + if suffix.strip() and all(part.strip() for part in raw_modifiers): + normalized_modifiers = [key_aliases.get(part.strip().lower(), part) for part in raw_modifiers] + if all(modifier in modifier_map for modifier in normalized_modifiers): + is_combination = True + modifiers = normalized_modifiers + main_key = key_aliases.get(suffix.strip().lower(), suffix) - # Handle key combinations like "Control+A" - if '+' in normalized_keys: - parts = normalized_keys.split('+') - modifiers = parts[:-1] - main_key = parts[-1] + if is_combination and main_key is not None: # Calculate modifier bitmask modifier_value = 0 - modifier_map = {'Alt': 1, 'Control': 2, 'Meta': 4, 'Shift': 8} for mod in modifiers: modifier_value |= modifier_map.get(mod, 0) @@ -2545,6 +2549,9 @@ class DefaultActionWatchdog(BaseWatchdog): for mod in reversed(modifiers): await self._dispatch_key_event(cdp_session, 'keyUp', mod) else: + keys_lower = keys.strip().lower() + normalized_keys = key_aliases.get(keys_lower, keys) + # Check if this is a text string or special key special_keys = { 'Enter', diff --git a/tests/ci/browser/test_send_keys_literal_plus.py b/tests/ci/browser/test_send_keys_literal_plus.py new file mode 100644 index 000000000..49c161cff --- /dev/null +++ b/tests/ci/browser/test_send_keys_literal_plus.py @@ -0,0 +1,87 @@ +import asyncio +from types import SimpleNamespace + +from browser_use.browser.events import SendKeysEvent +from browser_use.browser.watchdogs.default_action_watchdog import DefaultActionWatchdog + + +def make_watchdog(recorded_params, dispatched_keys): + class Input: + async def dispatchKeyEvent(self, params=None, session_id=None): + recorded_params.append(params or {}) + + cdp_session = SimpleNamespace( + cdp_client=SimpleNamespace(send=SimpleNamespace(Input=Input())), + session_id='session-1', + ) + + class BrowserSession: + async def get_or_create_cdp_session(self, focus=False): + return cdp_session + + async def dispatch_key_event(_session, event_type, key, modifiers=0): + dispatched_keys.append((event_type, key, modifiers)) + + watchdog = SimpleNamespace( + browser_session=BrowserSession(), + logger=SimpleNamespace(info=lambda *args, **kwargs: None), + _dispatch_key_event=dispatch_key_event, + ) + watchdog._get_char_modifiers_and_vk = DefaultActionWatchdog._get_char_modifiers_and_vk.__get__(watchdog) + watchdog._get_key_code_for_char = DefaultActionWatchdog._get_key_code_for_char.__get__(watchdog) + return watchdog + + +def test_send_keys_literal_plus_dispatches_char_event(): + recorded_params = [] + dispatched_keys = [] + watchdog = make_watchdog(recorded_params, dispatched_keys) + + asyncio.run(DefaultActionWatchdog.on_SendKeysEvent(watchdog, SendKeysEvent(keys='+'))) + + assert any(params.get('type') == 'char' and params.get('text') == '+' for params in recorded_params) + assert all(key for _, key, _ in dispatched_keys) + + +def test_send_keys_text_with_plus_dispatches_all_characters(): + recorded_params = [] + dispatched_keys = [] + watchdog = make_watchdog(recorded_params, dispatched_keys) + + asyncio.run(DefaultActionWatchdog.on_SendKeysEvent(watchdog, SendKeysEvent(keys='C++'))) + + assert [params['text'] for params in recorded_params if params.get('type') == 'char'] == ['C', '+', '+'] + + +def test_send_keys_control_plus_keeps_plus_as_main_key(): + recorded_params = [] + dispatched_keys = [] + watchdog = make_watchdog(recorded_params, dispatched_keys) + + asyncio.run(DefaultActionWatchdog.on_SendKeysEvent(watchdog, SendKeysEvent(keys='Control++'))) + + assert dispatched_keys == [ + ('keyDown', 'Control', 0), + ('keyDown', '+', 2), + ('keyUp', '+', 2), + ('keyUp', 'Control', 0), + ] + + +def test_send_keys_existing_shortcut_and_special_key_still_work(): + recorded_params = [] + dispatched_keys = [] + watchdog = make_watchdog(recorded_params, dispatched_keys) + + asyncio.run(DefaultActionWatchdog.on_SendKeysEvent(watchdog, SendKeysEvent(keys='Control+a'))) + assert dispatched_keys == [ + ('keyDown', 'Control', 0), + ('keyDown', 'a', 2), + ('keyUp', 'a', 2), + ('keyUp', 'Control', 0), + ] + + recorded_params.clear() + dispatched_keys.clear() + asyncio.run(DefaultActionWatchdog.on_SendKeysEvent(watchdog, SendKeysEvent(keys='Enter'))) + assert dispatched_keys == [('keyDown', 'Enter', 0), ('keyUp', 'Enter', 0)] From 81a472a3e2c67a443a03a9224765bd73416b191a Mon Sep 17 00:00:00 2001 From: r266-tech Date: Fri, 28 Aug 2026 09:07:07 +0000 Subject: [PATCH 07/27] fix(dom): expose image context for directly clickable images --- browser_use/dom/serializer/serializer.py | 57 +++++++++------ .../ci/test_image_only_dom_representation.py | 73 +++++++++++++++++++ 2 files changed, 108 insertions(+), 22 deletions(-) diff --git a/browser_use/dom/serializer/serializer.py b/browser_use/dom/serializer/serializer.py index 8b86a9c75..1c2f9df32 100644 --- a/browser_use/dom/serializer/serializer.py +++ b/browser_use/dom/serializer/serializer.py @@ -16,6 +16,7 @@ from browser_use.dom.views import ( ) DISABLED_ELEMENTS = {'style', 'script', 'head', 'meta', 'link', 'title'} +URL_C0_CONTROL_OR_SPACE = ''.join(chr(codepoint) for codepoint in range(0x21)) # SVG child elements to skip (decorative only, no interaction value) SVG_ELEMENTS = { @@ -57,6 +58,7 @@ class DOMTreeSerializer: DEFAULT_CONTAINMENT_THRESHOLD = 0.99 # 99% containment by default MAX_CHILD_IMAGE_CONTEXTS = 3 MAX_CHILD_IMAGE_DESCENDANTS = 100 + MAX_IMAGE_CONTEXT_ATTRIBUTE_LENGTH = 4096 def __init__( self, @@ -922,17 +924,47 @@ class DOMTreeSerializer: @staticmethod def _get_child_image_context(node: SimplifiedNode) -> str: - """Extract compact context from image descendants of an interactive element.""" + """Extract compact context from an interactive image and its descendants.""" image_context: list[str] = [] def normalize_src(src: str) -> str: - clean_src = src.strip() + if len(src) > DOMTreeSerializer.MAX_IMAGE_CONTEXT_ATTRIBUTE_LENGTH: + return '' + clean_src = src.strip(URL_C0_CONTROL_OR_SPACE).replace('\t', '').replace('\n', '').replace('\r', '') if clean_src.lower().startswith('data:'): return '' path_without_query = clean_src.split('?', 1)[0].split('#', 1)[0].rstrip('/') return path_without_query.rsplit('/', 1)[-1] + def add_image_context(original_node: EnhancedDOMTreeNode) -> None: + if original_node.node_type != NodeType.ELEMENT_NODE or original_node.tag_name != 'img': + return + + attributes = original_node.attributes or {} + parts = [] + + for attr_name, output_name in ( + ('alt', 'image_alt'), + ('title', 'image_title'), + ('aria-label', 'image_label'), + ): + raw_attr_value = str(attributes.get(attr_name) or '') + if len(raw_attr_value) > DOMTreeSerializer.MAX_IMAGE_CONTEXT_ATTRIBUTE_LENGTH: + continue + attr_value = raw_attr_value.strip() + if attr_value: + parts.append(f'{output_name}={cap_text_length(attr_value, 100)}') + + src = normalize_src(str(attributes.get('src') or '')) + if src: + parts.append(f'image_src={cap_text_length(src, 100)}') + + if parts: + image_context.append(' '.join(parts)) + + add_image_context(node.original_node) + child_iterators = [iter(node.children)] visited_descendants = 0 while ( @@ -946,26 +978,7 @@ class DOMTreeSerializer: child_iterators.pop() continue visited_descendants += 1 - original_node = current.original_node - if original_node.node_type == NodeType.ELEMENT_NODE and original_node.tag_name == 'img': - attributes = original_node.attributes or {} - parts = [] - - for attr_name, output_name in ( - ('alt', 'image_alt'), - ('title', 'image_title'), - ('aria-label', 'image_label'), - ): - attr_value = str(attributes.get(attr_name) or '').strip() - if attr_value: - parts.append(f'{output_name}={cap_text_length(attr_value, 100)}') - - src = normalize_src(str(attributes.get('src') or '')) - if src: - parts.append(f'image_src={cap_text_length(src, 100)}') - - if parts: - image_context.append(' '.join(parts)) + add_image_context(current.original_node) if current.children: child_iterators.append(iter(current.children)) diff --git a/tests/ci/test_image_only_dom_representation.py b/tests/ci/test_image_only_dom_representation.py index 1ff1d3fe9..3aa48030b 100644 --- a/tests/ci/test_image_only_dom_representation.py +++ b/tests/ci/test_image_only_dom_representation.py @@ -81,6 +81,79 @@ def test_image_only_interactive_parent_includes_child_image_context_in_llm_dom() assert 'acme-bank-primary-card.png' in llm_dom +def test_direct_interactive_image_includes_own_context_in_llm_dom(): + """A directly clickable image should expose its own sanitized source context.""" + image = _make_element_node( + 221, + 'img', + {'src': 'https://cdn.example.test/logos/acme-card.png?token=must-not-leak#preview'}, + x=18, + y=18, + ) + + llm_dom = DOMTreeSerializer.serialize_tree( + SimplifiedNode( + original_node=image, + children=[], + is_interactive=True, + selector_index=221, + ), + DEFAULT_INCLUDE_ATTRIBUTES, + ) + + assert '[221]