diff --git a/extensions/fast-browser/browser_worker.py b/extensions/fast-browser/browser_worker.py index 6a43597..c8f4de6 100644 --- a/extensions/fast-browser/browser_worker.py +++ b/extensions/fast-browser/browser_worker.py @@ -53,6 +53,9 @@ NAVIGATION_TIMEOUT_SECONDS = 10.0 # timeout returns a structured result instead of racing the parent deadline. POPUP_WAIT_SECONDS = 4.75 EXPECTED_BROWSER_HARNESS_VERSION = "0.1.13" +_INTERNAL_TARGET_PREFIXES = ( + "chrome://", "chrome-untrusted://", "devtools://", "chrome-extension://", "about:", +) _TEXT_INPUT_TYPES = {"", "text", "search", "email", "url", "tel", "number", "password"} _FILL_ROLES = {"textbox", "searchbox", "spinbutton"} @@ -493,12 +496,12 @@ class BrowserWorker: self.backend.fill(candidate.backend_id, text) deadline = time.monotonic() + ACTION_TIMEOUT_SECONDS while time.monotonic() < deadline: - self._post_action_state(snapshot, same_document=True) + tab = self._post_action_state(snapshot, same_document=True) value_verified = self.backend.value_matches(candidate.backend_id, text) condition_verified = True if postcondition is not None: if postcondition["kind"] == "url": - condition_verified = self.backend.current_tab().get("url") == postcondition["value"] + condition_verified = tab.get("url") == postcondition["value"] else: condition_verified = self.backend.visible_text(postcondition["value"]) if value_verified and condition_verified: @@ -560,6 +563,7 @@ class RealBrowserBackend: self._daemon_ready = False self._process: subprocess.Popen | None = None self._attached_target: str | None = None + self._select_all_modifier: int | None = None self._configure_environment(cdp_url, self._sandbox) try: self._load_harness() @@ -703,45 +707,46 @@ class RealBrowserBackend: def switch_tab(self, target_id: str) -> None: if not isinstance(target_id, str) or not target_id: raise WorkerError("tab identity is unavailable") + # browser-harness 0.1.13 enables Page/DOM/Runtime/Network as part of + # its switch_tab set_session path. Re-enabling them here duplicates + # four CDP calls and can race the daemon's session setup. self._session_id = self._helpers.switch_tab(target_id, activate=False) self._attached_target = target_id - # The pinned helper does not enable these domains on every attach. Do - # so here once per target; drain_events is then a truthful source for - # the bounded diagnostics surface. - for method in ("Page.enable", "DOM.enable", "Runtime.enable", "Network.enable"): - try: - self._helpers.cdp(method) - except Exception: - # Diagnostics are best effort and never block a browser action. - pass def list_tabs(self) -> list[dict[str, Any]]: - tabs = self._helpers.list_tabs(include_chrome=False) - opener_by_target: dict[str, str] = {} - try: - raw = self._helpers.cdp("Target.getTargets") - for info in raw.get("targetInfos", []) if isinstance(raw, dict) else []: - if not isinstance(info, dict) or info.get("type") != "page": - continue - target = info.get("targetId") - opener = info.get("openerId") - if isinstance(target, str) and isinstance(opener, str) and opener: - opener_by_target[target] = opener - except Exception: - pass + # helpers.list_tabs() and this opener lookup used to issue two identical + # Target.getTargets requests. Keep its page/internal filtering locally + # while deriving opener metadata from the same target inventory. + raw = self._helpers.cdp("Target.getTargets") + target_infos = raw.get("targetInfos", []) if isinstance(raw, dict) else [] + if not isinstance(target_infos, list): + return [] result: list[dict[str, Any]] = [] - for tab in tabs: - if not isinstance(tab, dict) or not isinstance(tab.get("url"), str): + for info in target_infos: + if not isinstance(info, dict) or info.get("type") != "page": continue - target = tab.get("targetId") or tab.get("target_id") + url = info.get("url", "") + title = info.get("title", "") + if not isinstance(url, str) or not isinstance(title, str): + continue + # Match browser-harness helpers.list_tabs(include_chrome=False), + # including its startup-placeholder suppression. + if title.startswith("Starting agent ") and ( + url in ("", "about:blank") or url.startswith("about:blank#") + ): + continue + if url.startswith(_INTERNAL_TARGET_PREFIXES): + continue + target = info.get("targetId") if not isinstance(target, str) or not target: continue + opener = info.get("openerId") result.append({ "targetId": target, "target_id": target, - "url": tab.get("url", ""), - "title": tab.get("title", ""), - "openerTargetId": opener_by_target.get(target), + "url": url, + "title": title, + "openerTargetId": opener if isinstance(opener, str) and opener else None, }) return result @@ -948,9 +953,12 @@ class RealBrowserBackend: raise WorkerError("candidate could not be focused") # Select all without an arbitrary page script. Input.insertText is a # single bounded browser operation and is substantially faster than - # dispatching up to 8,000 individual key events. - user_agent = self._helpers.cdp("Browser.getVersion").get("userAgent", "") - modifiers = 4 if "Mac OS X" in user_agent or "Macintosh" in user_agent else 2 + # dispatching up to 8,000 individual key events. Browser.getVersion is + # stable for this backend, so cache the platform modifier for its life. + if getattr(self, "_select_all_modifier", None) is None: + user_agent = self._helpers.cdp("Browser.getVersion").get("userAgent", "") + self._select_all_modifier = 4 if "Mac OS X" in user_agent or "Macintosh" in user_agent else 2 + modifiers = self._select_all_modifier select_all = {"key": "a", "code": "KeyA", "modifiers": modifiers, "windowsVirtualKeyCode": 65, "nativeVirtualKeyCode": 65, "commands": ["SelectAll"]} diff --git a/extensions/fast-browser/test_browser_worker.py b/extensions/fast-browser/test_browser_worker.py index 3546d45..23c3849 100644 --- a/extensions/fast-browser/test_browser_worker.py +++ b/extensions/fast-browser/test_browser_worker.py @@ -721,6 +721,26 @@ class BridgeRegressionTests(unittest.TestCase): with self.assertRaisesRegex(WorkerError, "outside"): worker.act({"observationId": obs["observationId"], "candidateId": "c-0", "action": "fill", "text": "Ada"}) + def test_fill_url_postcondition_reuses_post_action_tab_state(self): + backend = FakeBackend() + original_fill = backend.fill + def fill_and_navigate(node, text): + original_fill(node, text) + backend.url = "http://fixture.test/done" + backend.fill = fill_and_navigate + current_tab = backend.current_tab + backend.current_tab = Mock(side_effect=current_tab) + worker = BrowserWorker(backend, ["http://fixture.test"]) + observation = worker.observe() + result = worker.act({ + "observationId": observation["observationId"], "candidateId": "c-0", + "action": "fill", "text": "Ada", + "postcondition": {"kind": "url", "value": "http://fixture.test/done"}, + }) + self.assertTrue(result["verified"]) + # observe, fresh pre-mutation revalidation, and one post-action read. + self.assertEqual(backend.current_tab.call_count, 3) + def test_oversized_input_and_sensitive_error_are_not_echoed(self): worker = BrowserWorker(FakeBackend(), ["http://fixture.test"]) out = io.BytesIO() @@ -738,6 +758,74 @@ class BridgeRegressionTests(unittest.TestCase): backend._load_harness() spawn.assert_not_called() + def test_real_backend_list_tabs_uses_one_inventory_with_helper_filtering_and_openers(self): + helper = Mock() + helper.cdp.return_value = {"targetInfos": [ + {"type": "page", "targetId": "page", "url": "http://fixture.test/form", "title": "Fixture", + "openerId": "opener"}, + {"type": "page", "targetId": "internal", "url": "chrome://settings", "title": "Settings"}, + {"type": "page", "targetId": "devtools", "url": "devtools://devtools", "title": "DevTools"}, + {"type": "page", "targetId": "placeholder", "url": "about:blank#startup", "title": "Starting agent 1"}, + {"type": "service_worker", "targetId": "worker", "url": "http://fixture.test/sw", "title": "Worker"}, + {"type": "page", "targetId": "plain", "url": "https://other.test/", "title": "Other", "openerId": ""}, + {"type": "page", "targetId": "malformed-opener", "url": "http://fixture.test/other", "title": "Other", + "openerId": 42}, + ]} + backend = RealBrowserBackend.__new__(RealBrowserBackend) + backend._helpers = helper + + tabs = backend.list_tabs() + + self.assertEqual([tab["targetId"] for tab in tabs], ["page", "plain", "malformed-opener"]) + self.assertEqual(tabs[0]["openerTargetId"], "opener") + self.assertIsNone(tabs[1]["openerTargetId"]) + self.assertIsNone(tabs[2]["openerTargetId"]) + helper.cdp.assert_called_once_with("Target.getTargets") + helper.list_tabs.assert_not_called() + + def test_real_backend_list_tabs_propagates_inventory_errors_without_fallback(self): + helper = Mock() + helper.cdp.side_effect = RuntimeError("inventory unavailable") + backend = RealBrowserBackend.__new__(RealBrowserBackend) + backend._helpers = helper + + with self.assertRaisesRegex(RuntimeError, "inventory unavailable"): + backend.list_tabs() + helper.cdp.assert_called_once_with("Target.getTargets") + helper.list_tabs.assert_not_called() + + def test_real_backend_switch_tab_does_not_reenable_harness_domains(self): + helper = Mock() + helper.switch_tab.return_value = "session-1" + backend = RealBrowserBackend.__new__(RealBrowserBackend) + backend._helpers = helper + + backend.switch_tab("target-1") + + self.assertEqual(backend._session_id, "session-1") + self.assertEqual(backend._attached_target, "target-1") + helper.switch_tab.assert_called_once_with("target-1", activate=False) + helper.cdp.assert_not_called() + + def test_real_backend_caches_select_all_modifier_for_backend_lifetime(self): + helper = Mock() + calls = [] + def cdp(method, **_kwargs): + calls.append(method) + return {"userAgent": "Macintosh"} if method == "Browser.getVersion" else {} + helper.cdp.side_effect = cdp + backend = RealBrowserBackend.__new__(RealBrowserBackend) + backend._helpers = helper + backend._call = Mock(return_value=True) + + backend.fill(1, "first") + backend.fill(1, "second") + + self.assertEqual(calls.count("Browser.getVersion"), 1) + keydowns = [call for call in helper.cdp.call_args_list + if call.args[0] == "Input.dispatchKeyEvent" and call.kwargs["type"] == "rawKeyDown"] + self.assertEqual([call.kwargs["modifiers"] for call in keydowns], [4, 4]) + def test_real_backend_unrestricted_policy_still_requires_one_web_tab(self): helper = Mock() helper.list_tabs.return_value = [