refactor(browser-control): consolidate to a single default browser path - #1779
Merged
bobleer merged 3 commits intoJul 26, 2026
Merged
Conversation
added 2 commits
July 26, 2026 18:40
The audit found seven overlapping browser-control paths; the model oscillating between two full automation stacks (agent-browser CLI vs ControlHub CDP, each with its own @en ref namespace, browser instance and login state) was the largest source of failed browser operations. - Make agent-browser default-off in every mode so the ControlHub browser domain is the single default path; explicit /skill invocation and user overrides still opt in (extend the default-hidden whitelist so explicit invocation keeps working) - Fix iframe click coordinates: element_center now accumulates ancestor frame offsets along the frameElement chain; cross-origin frames surface a structured error advising snapshot + re-target; missing coords error instead of clicking (0,0) - Remove the dead frame/frame_main actions and the unread active_frame session state they wrote; they never affected execution context - Emit structured error codes with recovery instructions at the source (element not found -> NOT_FOUND "take a new snapshot", dead session -> WRONG_TAB "reconnect or switch_page", timeouts -> TIMEOUT) instead of relying on phrase-matching classification; phrase matching stays as fallback only - State non-observability where control leaves the CDP stack: open_builtin marks the panel display-only, open_url/open_file explain the opened window cannot be observed or controlled and route to the right tool, WebFetch description routes login/JS pages to the browser domain - Record the product decision that ai.computer_use_enabled and browser control are independent switches (browser control stays available when computer use is off) in the refactor plan
…on safe An end-to-end review of the consolidated browser path (walking a real task: open -> log in -> find -> click -> observe -> read) found the advertised workflow could not actually be executed by a model. Results the model never saw: - The tool pipeline prefers `result_for_assistant`, and every browser action passed only a one-line count there, so `snapshot` returned literally "Snapshot: 42 interactive elements" and the @en refs the documented connect -> snapshot -> click flow depends on never reached the model. Same for list_pages/tab_query (no page ids, so switch_page was unusable), get_html, cookies, network/console/errors and meta.capabilities. Each now returns the payload the next decision needs; screenshot returns a real image attachment instead of a byte count and no longer duplicates base64 in the data field. Actions that silently did the wrong thing: - snapshot renumbered @en from 1 without clearing the previous pass's data-cdp-ref attributes, so after scrolling (viewport-filtered numbering) a stale attribute earlier in document order won the querySelector and click acted on the wrong element while reporting success. Refs are now cleared across the document, open shadow roots and same-origin frames before renumbering. - click/hover dispatched mouse events at a point without hit-testing it; an occluded target reported success. The center point is now verified with elementFromPoint and a covered target is refused with a recoverable error. - press_key sent no text/windowsVirtualKeyCode, so Enter never triggered native form submission. - browser.fetch wrapped CORS/transport failures in a "Fetched <url>" success line; they are now structured errors that explain the same-origin constraint, and HTTP status is surfaced in the summary while the response body is preserved. Frontmost-app detection (regression from the guard activated in GCWing#1777): - On Windows the foreground "name" is the window *title*, and the Chromium check matched bare substrings, so any window titled "Search" matched "arc" and had all desktop input blocked. Foreground apps now carry a process_name (executable basename on Windows, process name on macOS) and matching is whole-identity, falling back to token matching on the display name only when no identity is available. - The guard covers the app_*/interactive_*/visual_* variants that could previously bypass it, exempts the alt+tab/command+tab focus switch so a non-browser task can escape, and short-circuits on explicit non-browser selectors instead of querying the host. Also: select/wait accept @en refs, stale refs report STALE_REF instead of a generic not-found, unresolvable cross-origin iframes are explained instead of retried forever, prompts no longer promise the user's login state (connect uses BitFun's managed profile), open_builtin is routed as the display-only path, prompts stop pointing unconditionally at the default-off ComputerUse tool, and the agent-browser skill no longer argues against itself when explicitly invoked.
Collaborator
Author
Update: end-to-end review passAfter the consolidation commit I ran an adversarial end-to-end review of the resulting path — four reviewers (task flow, prompts/agent UX, diff correctness, capability gating), each finding independently verified against the code before being accepted. 33 findings survived verification; the fixes are in Two of them explain why browser automation failed in practice regardless of which stack was used: The
Also fixed, and worth calling out because it is a regression from #1777: on Windows the foreground app The rest: Verification: Known remaining (not in this PR): a guarded desktop action still queries the frontmost app twice per call (guard + result augmentation); caching it within a call needs plumbing the snapshot through and is better done with the per-session host-state work in phase 3 of |
The occlusion hit-test compared document.elementFromPoint against the target with contains(), but elementFromPoint stops at a shadow host and host.contains(shadowChild) is false — so every element inside an open shadow root (which resolve/snapshot deliberately support) was reported as occluded by its own host and refused. Descend through open shadow roots' elementFromPoint before comparing.
1688mengdie
pushed a commit
to 1688mengdie/BitFun
that referenced
this pull request
Jul 26, 2026
…th (GCWing#1779) * refactor(browser-control): consolidate to a single default browser path The audit found seven overlapping browser-control paths; the model oscillating between two full automation stacks (agent-browser CLI vs ControlHub CDP, each with its own @en ref namespace, browser instance and login state) was the largest source of failed browser operations. - Make agent-browser default-off in every mode so the ControlHub browser domain is the single default path; explicit /skill invocation and user overrides still opt in (extend the default-hidden whitelist so explicit invocation keeps working) - Fix iframe click coordinates: element_center now accumulates ancestor frame offsets along the frameElement chain; cross-origin frames surface a structured error advising snapshot + re-target; missing coords error instead of clicking (0,0) - Remove the dead frame/frame_main actions and the unread active_frame session state they wrote; they never affected execution context - Emit structured error codes with recovery instructions at the source (element not found -> NOT_FOUND "take a new snapshot", dead session -> WRONG_TAB "reconnect or switch_page", timeouts -> TIMEOUT) instead of relying on phrase-matching classification; phrase matching stays as fallback only - State non-observability where control leaves the CDP stack: open_builtin marks the panel display-only, open_url/open_file explain the opened window cannot be observed or controlled and route to the right tool, WebFetch description routes login/JS pages to the browser domain - Record the product decision that ai.computer_use_enabled and browser control are independent switches (browser control stays available when computer use is off) in the refactor plan * fix(browser-control): make browser results actionable and app detection safe An end-to-end review of the consolidated browser path (walking a real task: open -> log in -> find -> click -> observe -> read) found the advertised workflow could not actually be executed by a model. Results the model never saw: - The tool pipeline prefers `result_for_assistant`, and every browser action passed only a one-line count there, so `snapshot` returned literally "Snapshot: 42 interactive elements" and the @en refs the documented connect -> snapshot -> click flow depends on never reached the model. Same for list_pages/tab_query (no page ids, so switch_page was unusable), get_html, cookies, network/console/errors and meta.capabilities. Each now returns the payload the next decision needs; screenshot returns a real image attachment instead of a byte count and no longer duplicates base64 in the data field. Actions that silently did the wrong thing: - snapshot renumbered @en from 1 without clearing the previous pass's data-cdp-ref attributes, so after scrolling (viewport-filtered numbering) a stale attribute earlier in document order won the querySelector and click acted on the wrong element while reporting success. Refs are now cleared across the document, open shadow roots and same-origin frames before renumbering. - click/hover dispatched mouse events at a point without hit-testing it; an occluded target reported success. The center point is now verified with elementFromPoint and a covered target is refused with a recoverable error. - press_key sent no text/windowsVirtualKeyCode, so Enter never triggered native form submission. - browser.fetch wrapped CORS/transport failures in a "Fetched <url>" success line; they are now structured errors that explain the same-origin constraint, and HTTP status is surfaced in the summary while the response body is preserved. Frontmost-app detection (regression from the guard activated in GCWing#1777): - On Windows the foreground "name" is the window *title*, and the Chromium check matched bare substrings, so any window titled "Search" matched "arc" and had all desktop input blocked. Foreground apps now carry a process_name (executable basename on Windows, process name on macOS) and matching is whole-identity, falling back to token matching on the display name only when no identity is available. - The guard covers the app_*/interactive_*/visual_* variants that could previously bypass it, exempts the alt+tab/command+tab focus switch so a non-browser task can escape, and short-circuits on explicit non-browser selectors instead of querying the host. Also: select/wait accept @en refs, stale refs report STALE_REF instead of a generic not-found, unresolvable cross-origin iframes are explained instead of retried forever, prompts no longer promise the user's login state (connect uses BitFun's managed profile), open_builtin is routed as the display-only path, prompts stop pointing unconditionally at the default-off ComputerUse tool, and the agent-browser skill no longer argues against itself when explicitly invoked. * fix(browser-control): pierce shadow roots in the click hit-test The occlusion hit-test compared document.elementFromPoint against the target with contains(), but elementFromPoint stops at a shadow host and host.contains(shadowChild) is false — so every element inside an open shadow root (which resolve/snapshot deliberately support) was reported as occluded by its own host and refused. Descend through open shadow roots' elementFromPoint before comparing. --------- Co-authored-by: bowen628 <bowen628@noreply.gitcode.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
Follow-up to #1777. The audit identified seven overlapping browser-control paths as the main reason the agent performs poorly at browser operations: the model oscillates between two full automation stacks (the agent-browser CLI skill and the ControlHub CDP domain), each with its own
@eNref namespace, its own browser instance, and mutually invisible login state — refs from one stack fail in the other, and recovery guidance pointed in both directions.This PR makes the ControlHub browser domain the single default path and fixes the correctness issues that remained on it.
Changes
Single default stack
DISABLE_COMPUTER_USErule added toAGENTIC_POLICY/TEAM_POLICY). Both opt-in paths are preserved and contract-tested: user overrides, and explicit/skill agent-browserinvocation (the default-hidden whitelist now includes thecomputer-usegroup — without this, explicit invocation would have broken).Interaction correctness on the ControlHub path
element_centeraccumulates ancestor frame offsets (rect + border) along theframeElementchain, so clicks inside same-origin iframes land on the element instead of iframe-local coordinates. Cross-origin frames return a structured error advising a fresh snapshot and re-target; missing coordinates now error instead of silently clicking (0,0). All element-targeting actions (click,hover,fill, …) benefit.frame/frame_mainactions: they wroteactive_framestate that nothing ever read, so the model believed it had switched frames when nothing changed. The unread session state is deleted too.Recoverable errors
NOT_FOUND("take a new snapshot and use a fresh @en ref"), dead session/WebSocket →WRONG_TAB("call browser.connect or switch_page"), CDP/wait timeouts →TIMEOUT. The phrase-matching classifier inmap_dispatch_errorremains as fallback only.Honest observability at path boundaries
open_builtinnow states the built-in panel is display-only and not observable by the agent, routing reading/interaction toconnect+snapshot.open_url/open_fileresults explain the opened window cannot be observed or controlled by BitFun and route to the browser domain / desktop actions respectively — previously the model would open a URL and then try to drive the browser with desktop clicks.Plan update
ai.computer_use_enabledand browser control are independent switches: disabling computer use must not hide or disable the ControlHub browser domain; phase 6 will add a separateai.browser_control_enabled(default on) plus abrowser_controlpermission intent.Verification
cargo check --workspacepasses (verified with pipefail; includesbitfun-desktop)cargo test -p bitfun-core browser: 27 passed (new iframe/structured-error/open_builtin tests included)cargo test -p bitfun-core control_hub computer_use skills: 263+ passedcargo test -p bitfun-agent-runtime(incl.skill_contracts): all passed, with new contract tests for default-off in every mode and explicit-invocation opt-innode scripts/core-boundaries/checker.mjspasses