feat(browser-use): embedded APP-053 host plane + Composer slash command - #204
Merged
Merged
Conversation
Wire Atmos in-app browser into Browser Use: loopback control plane on Desktop Electron (guest CDP/DOM), crates/browser-use with cua+embedded backends, atmos browser-use CLI, system skill, and / Browser Use slash in Welcome + Terminal composer.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedToo many files! This PR contains 213 files, which is 113 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (213)
You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR adds Browser Use through CUA and embedded Electron backends, CLI commands, browser control endpoints, web slash commands, skill documentation, and action chrome. It also updates macOS overlay, Dock, branding, browser-window, and Desktop Use permission behavior. ChangesBrowser Use feature
macOS overlay and window behavior
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
apps/web/src/features/terminal/components/TerminalAgentInputOverlay.tsx (1)
45-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the shared helper out of the Welcome feature.
TerminalAgentInputOverlayimports Browser Use helpers fromfeatures/welcome, andWelcomePageuses the same helpers.slash-browser-use.tsbelongs to both feature surfaces.Move the helper and its test to a shared composer or slash-command module. Update both imports.
As per coding guidelines, use feature-local ownership for libraries that belong to only one feature.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/features/terminal/components/TerminalAgentInputOverlay.tsx` around lines 45 - 50, Move slash-browser-use.ts and its test from the Welcome feature into an appropriate shared composer or slash-command module, then update TerminalAgentInputOverlay and WelcomePage to import BROWSER_USE_SLASH_COMMAND_ID, buildBrowserUseSlashCommand, matchesBrowserUseSlashQuery, and resolveBrowserUseSkillRef from the new location. Preserve the helper behavior and remove the old feature-owned imports.Source: Coding guidelines
crates/browser-use/src/types.rs (1)
15-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the alias variants into serde aliases.
Externalbehaves the same asCua, andAtmosbehaves the same asEmbedded.parsenever produces the alias variants, so the extra states exist only for deserialization. Everymatchon this enum must now handle four arms for two behaviors, and one missed arm changes behavior silently (seeexecuteinlib.rslines 23-26).♻️ Proposed refactor
pub enum BrowserBackendKind { /// System Chromium via managed control engine (CUA tools). Default. #[default] + #[serde(alias = "external", alias = "chrome", alias = "chromium")] Cua, - /// Alias for Cua. - External, /// Atmos in-app browser (APP-053 webview + host CDP control plane). + #[serde(alias = "atmos", alias = "webview", alias = "app")] Embedded, - /// Alias for Embedded. - Atmos, } impl BrowserBackendKind { @@ pub fn as_str(self) -> &'static str { match self { - Self::Cua | Self::External => "cua", - Self::Embedded | Self::Atmos => "embedded", + Self::Cua => "cua", + Self::Embedded => "embedded", } }This change removes the public variants
ExternalandAtmos. Updatelib.rsexecuteand any CLI match arms accordingly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/browser-use/src/types.rs` around lines 15 - 42, Remove the redundant External and Atmos variants from BrowserBackendKind, and add serde aliases for those input names on Cua and Embedded respectively. Update BrowserBackendKind::parse, as_str, lib.rs execute, and all CLI or other enum matches to use only Cua and Embedded while preserving existing deserialization and string parsing behavior.crates/browser-use/src/backends/mod.rs (1)
4-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe exported backend structs are unusable without the trait.
lib.rsre-exportsCuaExternalBackendandEmbeddedBackend, butBrowserBackendstays inside the privatemod backends. External callers can name the types and cannot callexecuteon them. Choose one surface: exportBrowserBackendalongside the structs, or keep the structs internal and expose onlyexecute(req).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/browser-use/src/backends/mod.rs` around lines 4 - 11, Make the backend API usable by external callers by exporting BrowserBackend from the backends module alongside CuaExternalBackend and EmbeddedBackend, and ensure the crate root re-exports the trait with those structs so callers can invoke execute. Keep the existing trait method signature unchanged.crates/browser-use/Cargo.toml (1)
1-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign managed dependencies with the workspace.
workspace.dependenciesis already defined, and the root manifest includescrates/*. Use workspace inheritance forserde;tempfileis the remaining local dependency needing a workspace entry. Do not adddirsto this crate unless it is actually used.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/browser-use/Cargo.toml` around lines 1 - 13, Update the browser-use manifest to inherit serde from workspace.dependencies, add tempfile to the workspace dependency table and inherit it here, and remove dirs unless the crate’s source actually uses it. Preserve the existing dependency features and avoid retaining redundant local version declarations.apps/desktop-electron/src/browser/browser-use-control.ts (1)
58-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse structured lifecycle logging.
This new lifecycle log writes to
console.log. Route it through the project's structured debug logging infrastructure so the event is written as JSON lines under./logs/debug/.As per coding guidelines, “Use the project's structured debug logging infrastructure when instrumenting lifecycle flows.” <coding_guidelines>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop-electron/src/browser/browser-use-control.ts` around lines 58 - 60, Replace the direct console.log in the browser control startup lifecycle flow with the project’s structured debug logger, preserving the existing control-plane URL message and ensuring it is emitted through the JSON-lines debug logging infrastructure under ./logs/debug/.Source: Coding guidelines
apps/desktop-electron/src/browser/browser-use-control.structural.test.ts (1)
7-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd behavioral control-plane tests.
These assertions only verify that source files contain selected strings. They can pass when server startup, routing, request validation, browser actions, or cleanup are broken.
Start the control plane with a mock
BrowserSurfaceManager. Test/v1/prepare,/v1/state, invalid methods, malformed and oversized bodies, authorization, navigation cache invalidation, andstop()cleanup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop-electron/src/browser/browser-use-control.structural.test.ts` around lines 7 - 27, Replace the string-presence checks in the structural test with behavioral tests that start the control plane using a mocked BrowserSurfaceManager and exercise its HTTP contract. Cover successful /v1/prepare and /v1/state flows, invalid methods, malformed and oversized request bodies, authorization failures, navigation cache invalidation, and stop() cleanup, asserting responses and manager interactions for each case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cli/src/output.rs`:
- Around line 56-59: Update the output-kind mapping for Commands::BrowserUse so
it uses a dedicated human-readable Browser Use kind instead of Self::Json by
default. Add that command kind and implement its rendering for host-operation
results, while preserving Self::Json when the user explicitly passes --json.
In `@apps/desktop-electron/src/browser/browser-use-control.ts`:
- Around line 192-223: Move the debugger cleanup in the mouse action method into
a finally block so dbg.detach() runs whenever this method attached the debugger,
including when either sendCommand call throws. Preserve the existing fallback
behavior and only detach when attachedHere is true.
- Around line 142-179: Update the inner snapshot script in snapshot() to exclude
credential-bearing controls before building elements: omit
input[type="password"] and other credential-bearing fields from the returned
element data, including their values, so the SessionCache exposed through
/v1/state cannot contain sensitive form values. Preserve normal metadata
collection for non-sensitive controls and update the element selection or
mapping logic rather than relying on downstream consumers.
- Around line 49-63: Update the server startup flow around the createServer
callback and server.listen call to attach an error listener before listening; on
listen failure, clear this.server and report the startup error through the
existing ./logs/debug/ mechanism. Preserve the loopback binding, port
assignment, metadata writing, and success logging behavior.
- Around line 399-406: After successful navigation completes in the block
containing guest.loadURL(navUrl) or this.manager.navigate(targetId, navUrl),
invalidate the cached snapshot associated with targetId to prevent subsequent
operations from using stale coordinates or element indices from the previous
page. Ensure the cache is cleared before sending the 200 response so that any
follow-up operations get a fresh snapshot from the new document.
In `@apps/web/src/features/welcome/lib/slash-browser-use.ts`:
- Around line 41-56: Update resolveBrowserUseSkillRef to include the matched
SkillInfo.status in its returned reference, including the fallback behavior as
appropriate. In the /browser selection flow, use that status to prevent
inserting the skill reference when it is disabled, unless the prompt resolver
already rejects disabled skill chips; preserve enabled-skill insertion and
existing fallback resolution.
In `@crates/browser-use/src/backends/embedded.rs`:
- Around line 81-103: The embedded client request flow around stream reading
must enforce a connection timeout and cap the identity-delimited response body.
Configure the connection attempt with an explicit timeout, and replace unbounded
read_to_end in the response parsing path with a bounded read that rejects bodies
exceeding the configured maximum while preserving existing HTTP and JSON parsing
behavior.
In `@crates/browser-use/src/engine_client.rs`:
- Around line 71-81: Update call_tool_on to execute the engine child with piped
stdio and enforce a bounded wall-clock timeout, killing the child and returning
a clear timeout error if it hangs. In call_tool, after the existing 2-second
socket poll expires, return an explicit “engine did not start” error instead of
proceeding to expose the raw engine failure.
In `@specs/APP/APP-053_desktop-browser-webview/TECH.md`:
- Line 10: Update the “Browser Use (embedded)” row to remove the incorrect
APP-052 §5.2 reference and make the embedded Browser Use contract
self-contained, while preserving the `atmos browser-use --backend embedded`
command and CDP/DOM via guest WebContents details.
---
Nitpick comments:
In `@apps/desktop-electron/src/browser/browser-use-control.structural.test.ts`:
- Around line 7-27: Replace the string-presence checks in the structural test
with behavioral tests that start the control plane using a mocked
BrowserSurfaceManager and exercise its HTTP contract. Cover successful
/v1/prepare and /v1/state flows, invalid methods, malformed and oversized
request bodies, authorization failures, navigation cache invalidation, and
stop() cleanup, asserting responses and manager interactions for each case.
In `@apps/desktop-electron/src/browser/browser-use-control.ts`:
- Around line 58-60: Replace the direct console.log in the browser control
startup lifecycle flow with the project’s structured debug logger, preserving
the existing control-plane URL message and ensuring it is emitted through the
JSON-lines debug logging infrastructure under ./logs/debug/.
In `@apps/web/src/features/terminal/components/TerminalAgentInputOverlay.tsx`:
- Around line 45-50: Move slash-browser-use.ts and its test from the Welcome
feature into an appropriate shared composer or slash-command module, then update
TerminalAgentInputOverlay and WelcomePage to import
BROWSER_USE_SLASH_COMMAND_ID, buildBrowserUseSlashCommand,
matchesBrowserUseSlashQuery, and resolveBrowserUseSkillRef from the new
location. Preserve the helper behavior and remove the old feature-owned imports.
In `@crates/browser-use/Cargo.toml`:
- Around line 1-13: Update the browser-use manifest to inherit serde from
workspace.dependencies, add tempfile to the workspace dependency table and
inherit it here, and remove dirs unless the crate’s source actually uses it.
Preserve the existing dependency features and avoid retaining redundant local
version declarations.
In `@crates/browser-use/src/backends/mod.rs`:
- Around line 4-11: Make the backend API usable by external callers by exporting
BrowserBackend from the backends module alongside CuaExternalBackend and
EmbeddedBackend, and ensure the crate root re-exports the trait with those
structs so callers can invoke execute. Keep the existing trait method signature
unchanged.
In `@crates/browser-use/src/types.rs`:
- Around line 15-42: Remove the redundant External and Atmos variants from
BrowserBackendKind, and add serde aliases for those input names on Cua and
Embedded respectively. Update BrowserBackendKind::parse, as_str, lib.rs execute,
and all CLI or other enum matches to use only Cua and Embedded while preserving
existing deserialization and string parsing behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bc2e913d-0cdd-42b8-ae5f-0d4dd4d6aa77
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
apps/cli/Cargo.tomlapps/cli/src/commands/browser_use.rsapps/cli/src/commands/mod.rsapps/cli/src/main.rsapps/cli/src/output.rsapps/desktop-electron/src/app-state.tsapps/desktop-electron/src/browser/browser-use-control.structural.test.tsapps/desktop-electron/src/browser/browser-use-control.tsapps/desktop-electron/src/browser/surface-manager.tsapps/desktop-electron/src/main.tsapps/web/messages/en.jsonapps/web/messages/zh.jsonapps/web/src/features/terminal/components/TerminalAgentInputOverlay.tsxapps/web/src/features/welcome/components/WelcomePage.tsxapps/web/src/features/welcome/hooks/use-welcome-slash-search.tsapps/web/src/features/welcome/lib/__tests__/slash-browser-use.test.tsapps/web/src/features/welcome/lib/slash-browser-use.tscrates/browser-use/Cargo.tomlcrates/browser-use/src/backends/cua.rscrates/browser-use/src/backends/embedded.rscrates/browser-use/src/backends/mod.rscrates/browser-use/src/engine_client.rscrates/browser-use/src/lib.rscrates/browser-use/src/types.rscrates/infra/src/utils/system_skill_sync.rsskills/atmos-browser-use/SKILL.mdskills/system-skills-manifest.jsonspecs/APP/APP-053_desktop-browser-webview/TECH.md
E2E report (expired)This report has been superseded by a newer CI - E2E run.
|
- Handle control-plane listen errors; detach CDP debugger on failure - Redact password field values and invalidate snapshots after navigate - Bound embedded HTTP connect/read; timeout control-engine calls - Skip /browser slash insert when the Browser Use skill is disabled
E2E report (expired)This report has been superseded by a newer CI - E2E run.
|
AppShot boot warm created a panel/always-on-top overlay and packaged builds called dock.setIcon, which left a zero-width Dock entry so Atmos looked icon-less. Skip overlay warm on darwin, drop panel type, setIcon only in dev, and recycle Dock after all-workspaces capture play.
…n key Add a browser-only macOS traffic light y for the h-8 tab strip without changing main/agent-chat chrome, and rename desktopNative → desktop so the element picker tooltip resolves.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/desktop-electron/src/windows/mac-dock.test.ts (1)
40-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest the hard Dock refresh branch.
The
falsepath now callsforceMacDockTileRefresh(), notensureMacDockVisible(). This test only counts window calls, so it passes if the hard path stops callingapp.dock.hide()andapp.dock.show().Mock
electron.app.dock, assert the false path performshide()beforeshow(), and update the stale comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop-electron/src/windows/mac-dock.test.ts` around lines 40 - 51, Update the macOS test around setOverlayVisibleOnAllWorkspaces to mock electron.app.dock and verify the false path invokes dock.hide() before dock.show(). Replace the stale ensureMacDockVisible comment with one describing forceMacDockTileRefresh, while preserving the existing window side-effect assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop-electron/src/appshot/capture-animation.ts`:
- Around line 310-314: Update playCaptureAnimation and its finally cleanup to
associate setOverlayVisibleOnAllWorkspaces ownership with each playback
generation, so stale cleanup cannot disable visibility enabled by a newer
overlapping call. Only clear the shared all-workspaces setting when the
completing generation still owns it, and add a test covering overlapping
playCaptureAnimation calls.
In `@crates/browser-use/src/engine_client.rs`:
- Around line 96-120: Update the control-engine execution flow around child
spawn and the timeout wait so stdout and stderr are drained concurrently while
the process runs, preventing pipe-buffer deadlocks; join both reader threads
after normal exit or timeout cleanup and propagate their captured buffers into
the existing result handling. Add a regression test that runs a child producing
output larger than a pipe buffer and verifies the call completes successfully.
---
Nitpick comments:
In `@apps/desktop-electron/src/windows/mac-dock.test.ts`:
- Around line 40-51: Update the macOS test around
setOverlayVisibleOnAllWorkspaces to mock electron.app.dock and verify the false
path invokes dock.hide() before dock.show(). Replace the stale
ensureMacDockVisible comment with one describing forceMacDockTileRefresh, while
preserving the existing window side-effect assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 851adb8d-d3ba-49b5-8106-0cee609fc9e4
📒 Files selected for processing (16)
apps/desktop-electron/src/appshot/capture-animation.tsapps/desktop-electron/src/branding.tsapps/desktop-electron/src/browser/browser-use-control.tsapps/desktop-electron/src/main.tsapps/desktop-electron/src/windows/mac-chrome.tsapps/desktop-electron/src/windows/mac-dock.test.tsapps/desktop-electron/src/windows/mac-dock.tsapps/desktop-electron/src/windows/secondary.tsapps/web/messages/en.jsonapps/web/messages/zh.jsonapps/web/src/features/terminal/components/TerminalAgentInputOverlay.tsxapps/web/src/features/welcome/components/WelcomePage.tsxapps/web/src/features/welcome/lib/__tests__/slash-browser-use.test.tsapps/web/src/features/welcome/lib/slash-browser-use.tscrates/browser-use/src/backends/embedded.rscrates/browser-use/src/engine_client.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/browser-use/src/backends/embedded.rs
- apps/web/src/features/terminal/components/TerminalAgentInputOverlay.tsx
- apps/web/messages/zh.json
- apps/web/src/features/welcome/components/WelcomePage.tsx
- apps/desktop-electron/src/browser/browser-use-control.ts
E2E report (expired)This report has been superseded by a newer CI - E2E run.
|
Absorb PR #202 (Desktop Use host engine, AppShot host capture, slash/skill) into the browser-use branch while keeping the real embedded host plane. Resolution: - CUA browser-use routes through desktop_use::host / DesktopUseManager - Embedded backend remains APP-053 control.json loopback (not stub) - CLI type --ref optional for embedded, required on CUA via validation - Both /desktop-use and /browser-use slash commands + system skills - Packaged dock.setIcon still skipped (Dock zero-width fix)
E2E report (expired)This report has been superseded by a newer CI - E2E run.
|
Wire session cursor/operation border for CUA and embedded browser-use clicks (and type-with-ref), harden chrome spawn against missing CLI, fix desktop-use skill status for typecheck, and add use-local-cli just recipe.
Prevent stale playCaptureAnimation cleanup from clearing all-workspaces visibility owned by a newer generation. Sanitize control-plane 500 bodies to message-only (no stack) for CodeQL stack-trace exposure.
cargo fmt --check failed on cua/chrome. Also stop sending Error-derived strings on the browser-use control plane (CodeQL stack-trace exposure); log full errors server-side only.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/browser-use/src/lib.rs (1)
52-69: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMake the
ATMOS_BROWSER_USE_HOMEoverride panic-safe and serialized.Line 55 changes a process-wide environment variable. If
executeor the assertions on lines 62-66 panic in this test, lines 67-69 do not run, and later tests may read a deleted temporary directory. Run this test in parallel mode, acquire the test lock, and restore the priorATMOS_BROWSER_USE_HOMEvalue on drop rather than unconditionallyremove_var.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/browser-use/src/lib.rs` around lines 52 - 69, The test around execute must serialize access to the process-wide ATMOS_BROWSER_USE_HOME variable and restore its previous value during unwinding. Update the test to acquire the existing test lock, install a scoped environment override whose drop handler restores the prior value instead of unconditionally calling remove_var, and keep the override active through execute and the assertions.
🧹 Nitpick comments (2)
apps/desktop-electron/src/browser/browser-use-control.ts (2)
338-338: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLocalize the Chrome status labels.
"Clicking page"and"Typing in page"are visible in the Desktop Use overlay. Replace these literals with the desktop i18n lookup. Add matching translations to every desktop locale. Updateapps/desktop-electron/src/browser/browser-use-control.structural.test.tsto assert the new contract.As per coding guidelines, avoid hardcoded user-facing copy and use existing i18n lookup patterns.
Also applies to: 405-405
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop-electron/src/browser/browser-use-control.ts` at line 338, Replace the hardcoded status labels passed by showChromeForRef in the click and typing paths with the existing desktop i18n lookup pattern, using distinct translation keys for “Clicking page” and “Typing in page”. Add those keys to every desktop locale, then update browser-use-control.structural.test.ts to assert the localized lookup contract.Source: Coding guidelines
49-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the structured debug logger for Browser Use diagnostics.
Replace these
console.warnandconsole.errorcalls with the project structured debug logger. Record operation context as fields. Keep error details out of user-facing responses.As per coding guidelines, use the project's structured debug logging infrastructure when instrumenting lifecycle flows or diagnosing difficult bugs; logs are written as JSON lines under
./logs/debug/.Also applies to: 317-329, 569-576
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop-electron/src/browser/browser-use-control.ts` around lines 49 - 63, Update the error logging in spawnDetachedQuiet and the additionally referenced browser-use lifecycle paths to use the project’s structured debug logger instead of console.warn or console.error. Record the operation and relevant command/error context as structured fields, while keeping these diagnostics internal and excluding error details from user-facing responses.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/browser-use/src/chrome.rs`:
- Line 6: Run cargo fmt --all across the repository and commit the resulting
formatting changes, including the affected imports and the code near the
referenced ranges in chrome.rs, so cargo fmt --all -- --check passes.
---
Outside diff comments:
In `@crates/browser-use/src/lib.rs`:
- Around line 52-69: The test around execute must serialize access to the
process-wide ATMOS_BROWSER_USE_HOME variable and restore its previous value
during unwinding. Update the test to acquire the existing test lock, install a
scoped environment override whose drop handler restores the prior value instead
of unconditionally calling remove_var, and keep the override active through
execute and the assertions.
---
Nitpick comments:
In `@apps/desktop-electron/src/browser/browser-use-control.ts`:
- Line 338: Replace the hardcoded status labels passed by showChromeForRef in
the click and typing paths with the existing desktop i18n lookup pattern, using
distinct translation keys for “Clicking page” and “Typing in page”. Add those
keys to every desktop locale, then update browser-use-control.structural.test.ts
to assert the localized lookup contract.
- Around line 49-63: Update the error logging in spawnDetachedQuiet and the
additionally referenced browser-use lifecycle paths to use the project’s
structured debug logger instead of console.warn or console.error. Record the
operation and relevant command/error context as structured fields, while keeping
these diagnostics internal and excluding error details from user-facing
responses.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: da4d3845-fc1b-4b21-9826-9f1405471226
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
apps/cli/AGENTS.mdapps/desktop-electron/src/appshot/capture-animation.tsapps/desktop-electron/src/branding.tsapps/desktop-electron/src/browser/browser-use-control.structural.test.tsapps/desktop-electron/src/browser/browser-use-control.tsapps/desktop-electron/src/browser/surface-manager.tsapps/web/messages/en.jsonapps/web/messages/zh.jsonapps/web/src/features/terminal/components/TerminalAgentInputOverlay.tsxapps/web/src/features/welcome/components/PromptComposer.tsxapps/web/src/features/welcome/components/SlashCommandPopover.tsxapps/web/src/features/welcome/components/WelcomePage.tsxapps/web/src/features/welcome/components/__tests__/slash-command-icons.test.tsapps/web/src/features/welcome/lib/slash-desktop-use.tscrates/browser-use/Cargo.tomlcrates/browser-use/src/backends/cua.rscrates/browser-use/src/backends/mod.rscrates/browser-use/src/chrome.rscrates/browser-use/src/lib.rscrates/browser-use/src/types.rsjustfilepackages/ui/src/components/icons/browser-use-icon-static.tsx
💤 Files with no reviewable changes (2)
- crates/browser-use/src/backends/mod.rs
- crates/browser-use/src/types.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- crates/browser-use/Cargo.toml
- apps/web/src/features/terminal/components/TerminalAgentInputOverlay.tsx
- apps/web/src/features/welcome/components/WelcomePage.tsx
- apps/desktop-electron/src/appshot/capture-animation.ts
- apps/desktop-electron/src/branding.ts
- crates/browser-use/src/backends/cua.rs
E2E report (expired)This report has been superseded by a newer CI - E2E run.
|
…d from window_id Pixel click/screenshot previously failed with opaque engine errors (px_capture_unavailable / missing pid). Preflight doctor when Screen Recording is denied, map capture failures to permissions_required, and resolve pid from list_windows when only --window-id is given.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/desktop-use/src/control.rs (1)
258-270: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReturn
permissions_requiredfor local Screen Recording failures.When the engine is unavailable, known Screen Recording failures get a grant instruction but retain
capture_failed. Clients cannot route this fallback through the same permission recovery flow as the preflight and engine paths. Seterror_codetopermissions_requiredwhen the same error predicate matches. Add a regression test for this fallback classification.Proposed fix
let cap = capture(CaptureRequest { out_path: req.out_path.clone(), include_base64: req.out_path.is_none(), }); + let screen_recording_failure = matches!( + cap.error.as_deref(), + Some(e) if e.contains("screencapture") || e.contains("could not create image") + ); DriveResult { @@ - error_code: if cap.ok { + error_code: if cap.ok { None + } else if screen_recording_failure { + Some("permissions_required".into()) } else { Some("capture_failed".into()) },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop-use/src/control.rs` around lines 258 - 270, Update the capture response classification around the existing `cap.error` predicate so local Screen Recording failures containing “screencapture” or “could not create image” return `error_code` as `permissions_required`, while other failures retain `capture_failed` and successful captures remain unchanged. Add a regression test covering this fallback classification.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/desktop-use/src/control.rs`:
- Around line 258-270: Update the capture response classification around the
existing `cap.error` predicate so local Screen Recording failures containing
“screencapture” or “could not create image” return `error_code` as
`permissions_required`, while other failures retain `capture_failed` and
successful captures remain unchanged. Add a regression test covering this
fallback classification.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f021bb3c-9436-4ba0-b755-66a992e735f0
📒 Files selected for processing (6)
apps/desktop-electron/src/browser/browser-use-control.tscrates/browser-use/src/backends/cua.rscrates/browser-use/src/chrome.rscrates/desktop-use/src/control.rscrates/desktop-use/src/highlight.rscrates/desktop-use/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/browser-use/src/chrome.rs
- crates/browser-use/src/backends/cua.rs
- apps/desktop-electron/src/browser/browser-use-control.ts
Nest Stop/Uninstall under Control engine, put Permissions second, and move Operation border into a Visual feedback group. Each group uses SettingsGroupCard with an icon and smart default expand/collapse.
Add group titles/descriptions for engine, permissions, and visibility, plus engine status row labels and settings-search border entry.
Assert collapsible group order/icons and extend settings search for stop/uninstall plus operation border under visual feedback.
Make agent status "{Agent} - {operation}" under the pointer only, hide
window borders when covered, convert screen points for window-scoped
actions, support type --x/--y, and auto-reset after desktop escalate so
window tools are not stuck on window_scope_disabled.
Move the macOS highlight overlay into a native prebuilt helper, add the Accessibility grant overlay preload, and wire readiness/permissions UI for Desktop Use control flows.
Raise task-complete and permission attention latches in core-service, expose them over API/WS, and teach the web attention/hooks stores to clear latches when panes are focused or sessions are dismissed.
Add shared center-tab title presentation, improve TUI mouse/scrollback handling, and add a setting to show or hide detected agent names in terminal and tab titles.
Add per-surface running-indicator styles (unicode spinners + AIcss Orbs), settings UI with live placement previews, and wire left sidebar, center tabs, terminal panel, and footer to the saved preferences.
Delegate BorderBeam to the published package and tighten dialog defaults.
Update workspace surface policies/hotkeys, attention filter shortcuts, appshot/welcome/desktop-use copy, and related e2e coverage.
Strip Grok-style realtime prefixes from center-tab titles and only clear native OSC topics on real shell CMD_END (9999), not reattach inject.
Persist cursor style/blink via function settings, expose them in Terminal settings, and reset the store on Computer connection changes.
Keep shell history for idle and alt-screen apps, wire cursor appearance into xterm, and clear OSC topics only on real shell CMD_END.
Opaque light-surface panels, confine maximized mosaic keep-alive, and dismiss body-portal terminal chrome when the surface is inactive.
Unify agent status marks across project/workspace/kanban/search and add managed PR lifecycle icons with center-tab open for PR and checks.
Pause inactive glyph animation, allow explicit Orb size, and avoid settings sync rewriting unchanged placement ids (preview flicker).
Refine history list presentation and expand coverage for selection and empty states.
Use system-style permission icons, simplify grant copy, and tighten the readiness dialog flow.
Regenerate Desktop Use host icns and web notification icon from the same art pack so brand surfaces stay in lockstep.
Fix commit textarea height so streaming text scrolls inside the frame, and drop redundant Refresh label on icon-only tabs.
Document sentence-case English UI labels and note that legacy icon regen also refreshes Desktop Use host and notification assets.
…use-embedded # Conflicts: # apps/cli/src/commands/desktop_use.rs # apps/desktop-electron/src/browser/surface-manager.ts # crates/desktop-use/src/control.rs
E2E report: ✅ Passed0 passed · 0 failed · 0 flaky · 0 skipped · 2m 56s · 100% pass rate Run
Overview
All selected E2E suites passed. |
This branch was successfully deployed
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
2 participants
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.
Summary
Wire Atmos Browser Use so agents can control:
<webview>, partitionpersist:atmos-browser) via a Desktop Electron loopback control plane (~/.atmos/browser-use/control.json) — guest DOM / CDP actions, not user-Chromebrowser_prepare.--backend cua) via a thin control-engine client against~/.atmos/desktop-usewhen the Desktop Use engine is installed.Also adds:
crates/browser-use(cua+embeddedbackends)atmos browser-use prepare|state|click|type|navigateatmos-browser-use+ manifest / skill syncBase branch is latest main (includes merged APP-053 / #203). Independent of open PR #202.
Related Issue
Type of Change
Validation
just lintjust testjust fmtcargo test -p browser-usecargo clippy -p browser-use -p atmos --all-targets -- -D warningsbun test apps/web/src/features/welcome/lib/__tests__/slash-browser-use.test.tsbun test apps/desktop-electron/src/browser/browser-use-control.structural.test.tsChecklist
skills/atmos-browser-use, APP-053 TECH cross-link)Manual test plan
atmos browser-use --json prepare --backend embeddedstate(list) →state --target-id <session>→click/type/navigate/shows Browser Use and insertsatmos-browser-useskill chipembedded_browser_host_unavailable)Summary by CodeRabbit