Skip to content

refactor(computer-use): phase-0 quick wins from computer-use/browser-control audit - #1777

Merged
bobleer merged 3 commits into
GCWing:mainfrom
bobleer:refactor/computer-use-phase0
Jul 26, 2026
Merged

refactor(computer-use): phase-0 quick wins from computer-use/browser-control audit#1777
bobleer merged 3 commits into
GCWing:mainfrom
bobleer:refactor/computer-use-phase0

Conversation

@bobleer

@bobleer bobleer commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Background

A full audit of the computer-use / browser-control stack (internal code review plus benchmarking against trycua/cua, OpenAI Codex CLI, Anthropic computer-use-demo, browser-use, and playwright-mcp) surfaced 33 issues (6 critical): 7 overlapping browser-control paths, a single god-tool with 40 actions / 7 click dialects / 4 coordinate systems, and safety boundaries that were never enforced. The full diagnosis and a six-phase refactor roadmap are added in docs/plans/computer-use-refactor-plan.md.

This PR lands the phase-0 quick wins: correctness and safety fixes only, no architectural changes.

Changes

Safety / correctness

  • Enforce the browser/desktop boundary guard: desktop_action_targets_browser was only wired to an unreachable fallthrough in handle_desktop; it now runs at ComputerUse dispatch, rejecting physical input actions while a Chromium-family browser is frontmost and pointing the model at the ControlHub browser domain. Firefox/Safari are removed from the browser detection table — CDP does not support them, so blocking desktop input there locked users out in both directions.
  • Headless mislabelling stopgap: browser.connect { mode: "headless" } now verifies via the CDP /json/version handshake that the endpoint really is a headless browser; previously it could attach to the user's real logged-in browser and label the session "Headless test browser".
  • Connect-failure hints no longer tell users to expose --remote-debugging-port on their everyday browser; they point at the BitFun-managed profile instead.
  • Debug screenshot persistence is now off by default (opt-in via BITFUN_COMPUTER_USE_DEBUG_SCREENSHOTS=1) and keeps only the 20 most recent files; previously every screenshot was unconditionally written into the user's workspace.
  • scroll_x/scroll_y now go through the same display-bounds check as other pointer actions.
  • Frontend: the slash-command path now respects the ComputerUse disabled gate (it previously bypassed the mode-dropdown check), and the toggle description no longer over-promises (browser control is not yet gated by this switch — now stated honestly).

Model execution quality

  • Remove all ghost tool references (ComputerUseMouseStep/MousePrecise/MouseClick): several model-visible prompts and error messages still told the model to call these long-deleted tools, causing failure loops.
  • Trim visual-only actions (build_interactive_view/interactive_click/build_visual_mark_view/visual_click) from the text-only ComputerUse schema and reject them at runtime with a clear error.
  • Unify browser routing guidance across the mode prompts and the agent-browser skill into a single ladder (WebFetch → ControlHub browser domain → ComputerUse desktop), removing the contradictory instructions that made the model oscillate between two @eN ref namespaces.

Cleanup

  • Delete zero-reference dead code: computer_use_input.rs, computer_use_result.rs, computer_use_verification.rs (plus its core-boundaries rule) and AIFeaturesConfig.tsx.
  • Consolidate the three copy-pasted ai.computer_use_enabled subscriptions into a shared useComputerUseEnabled() hook with a consistent initial value of false (they previously disagreed, causing a misleading first render).

Verification

  • cargo check --workspace passes
  • cargo test -p bitfun-core computer_use: 22 passed (includes new guard-reachability and text-only schema drift tests)
  • cargo test -p bitfun-core control_hub: 29 passed (includes 3 new headless validation tests)
  • cargo test -p bitfun-agent-tools: 103 passed
  • node scripts/core-boundaries/checker.mjs passes
  • web-ui tsc --noEmit and eslint pass

Follow-up

Phases 1–6 from docs/plans/computer-use-refactor-plan.md (remaining dead-code cleanup, 40→~15 action-space consolidation, per-session host state, provider-native computer-use tool mapping, single browser stack, config/permission/peer deny-table single source of truth) will follow as separate PRs.

bowen628 added 3 commits July 26, 2026 17:44
- Enforce the browser/desktop boundary guard in ComputerUse dispatch
  (previously unreachable dead code); narrow browser detection to
  Chromium-family so Firefox/Safari users are not locked out
- Remove ghost tool references (ComputerUseMouseStep/MousePrecise/
  MouseClick) from model-visible prompts, errors and MiniApp deny list
- Validate CDP /json/version before labelling a headless session; stop
  hinting users to expose a debug port on their everyday browser
- Trim visual-only actions from the text-only ComputerUse schema and
  reject them at runtime with a clear error
- Gate debug screenshot persistence behind an env flag with retention
- Guard scroll_x/scroll_y with the display-bounds check like other
  pointer actions
- Delete dead compat shims (computer_use_input/result/verification,
  AIFeaturesConfig) and their boundary-check rules
- Unify ai.computer_use_enabled subscription into a shared hook with a
  consistent initial value; close the slash-command bypass of the
  ComputerUse mode gate; correct over-promising toggle copy
- Align browser routing guidance across mode prompts and the
  agent-browser skill (ControlHub-first ladder)
@bobleer
bobleer merged commit fb1fe40 into GCWing:main Jul 26, 2026
5 checks passed
bobleer pushed a commit to bobleer/BitFun that referenced this pull request Jul 26, 2026
…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.
bobleer added a commit that referenced this pull request Jul 26, 2026
…th (#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 #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>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant