feat: add integrated browser - #277
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (6)
Summary by CodeRabbit
WalkthroughThis PR introduces an integrated browser backed by an Electron Sequence Diagram(s)sequenceDiagram
participant Renderer as Chat/BrowserPanel
participant Main as Electron Main
participant Manager as BrowserManager
participant Page as BrowserPage/Relay
participant AgentTool as browser_* tool
participant ControlServer as BrowserControlServer
Renderer->>Main: invoke browser operation
Main->>Manager: delegate request
Manager->>Page: create or reuse page
Page-->>Manager: browser state or result
Manager-->>Renderer: stateChanged event
AgentTool->>ControlServer: POST /v1/browser with bearer token
ControlServer->>Manager: execute(request, signal)
Manager->>Page: perform browser action
Page-->>Manager: action result
Manager-->>ControlServer: JSON result
ControlServer-->>AgentTool: response
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
electron/browser/playwright-relay.ts (1)
88-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
send()'s CDP special-case routing is not exercised by any test.
playwright-relay.test.tsfully mockschromium.connectOverCDP, so this method's routing (Browser.getVersion,Browser.setDownloadBehavior,Target.setAutoAttach/announcePage, scopedTarget.getTargetInfo, and the generic passthrough/error path) never actually runs in the test suite — only the higher-levelconnect()/emulateMediabehavior is verified. Since this is the core protocol-translation logic bridging Electron's debugger and Playwright's CDP client, consider adding a test that drivestransport.send(...)directly (e.g. by having the mockedconnectOverCDPinvoketransport.sendwith sample CDP requests) to assert the correctonmessagereplies for each branch.🤖 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 `@electron/browser/playwright-relay.ts` around lines 88 - 127, Add direct coverage for the private send() CDP routing by having the mocked chromium.connectOverCDP invoke transport.send with representative requests. Assert onmessage replies for Browser.getVersion, Browser.setDownloadBehavior, Target.setAutoAttach and announcePage, scoped Target.getTargetInfo, generic debugger passthrough, and debugger errors.
🤖 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 `@electron/browser/control-server.ts`:
- Line 14: Update BrowserControlServer’s response handling so both execute()
completion branches call respond() only while the response socket remains
connected, and remove the redundant request.once("aborted") listener while
preserving the existing response.once("close") abort behavior.
In `@electron/browser/policy.ts`:
- Around line 5-14: Update parseBrowserUrl so bare host:port inputs such as
localhost:5173 and example.com:8080 are prefixed with https:// instead of being
interpreted as custom schemes, while preserving explicit HTTP/HTTPS handling.
Add a regression test covering a local development address and verify it returns
an HTTP(S) URL.
In `@electron/chat/context-system.ts`:
- Around line 131-133: Update buildPermissionModeSystem to conditionally include
the integrated-browser guidance only when the effective browser state indicates
the browser is available and enabled. Pass that state into the prompt builder
from its caller, accounting for settings still loading, and omit the
browser-specific bullets when browserEnabled is false or unavailable.
In `@src/components/app-shell/use-browser-panel-state.ts`:
- Around line 38-64: Update the browserRequested event handler in the browser
panel state hook to set receivedStateEvent before opening the panel, so an
in-flight getState(activeSessionId) result cannot overwrite the requested open
state. Preserve the existing active-session filtering and panel-opening
behavior.
In `@src/routes/Chat/BrowserPanel.tsx`:
- Around line 59-65: Update the unmount cleanup in BrowserPanel’s effect to
report failures from browserService.invoke("hide", sessionId) through the
existing reportRendererHandledError mechanism instead of silently swallowing
them. Preserve the cleanup’s observer, resize-listener, and animation-frame
teardown while ensuring hide failures leave the same diagnostic trail as other
browserService invocations.
---
Nitpick comments:
In `@electron/browser/playwright-relay.ts`:
- Around line 88-127: Add direct coverage for the private send() CDP routing by
having the mocked chromium.connectOverCDP invoke transport.send with
representative requests. Assert onmessage replies for Browser.getVersion,
Browser.setDownloadBehavior, Target.setAutoAttach and announcePage, scoped
Target.getTargetInfo, generic debugger passthrough, and debugger errors.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cae769f7-d899-4692-9da3-3b27f93caa05
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (64)
AGENTS.mddocs/ai/README.mddocs/ai/integrated-browser-implementation.mddocs/architecture.mddocs/conventions.mddocs/integrated-browser.mddocs/openconnector-support-plan.mddocs/project-overview.mdelectron/agent/binaries.tselectron/agent/manager.tselectron/agent/tool-sources.test.tselectron/agent/tool-sources.tselectron/agent/workspace.test.tselectron/agent/workspace.tselectron/browser/common.tselectron/browser/control-server.test.tselectron/browser/control-server.tselectron/browser/node.test.tselectron/browser/node.tselectron/browser/page.test.tselectron/browser/page.tselectron/browser/playwright-relay.test.tselectron/browser/playwright-relay.tselectron/browser/policy.test.tselectron/browser/policy.tselectron/chat/common.tselectron/chat/context-system.test.tselectron/chat/context-system.tselectron/main.tselectron/settings/common.test.tselectron/settings/common.tselectron/settings/node.test.tselectron/settings/node.tselectron/settings/store.test.tselectron/settings/store.tspackage.jsonresources/wanta-skills/browser/SKILL.mdscripts/check-oo.tsscripts/skills.test.tsscripts/skills.tssrc/components/AppContext.tssrc/components/ai-elements/message.test.tssrc/components/app-shell/AppShell.tsxsrc/components/app-shell/AppShellMainTitlebar.tsxsrc/components/app-shell/AppShellRightPanel.tsxsrc/components/app-shell/app-shell-model.tssrc/components/app-shell/use-artifacts-panel-state.tssrc/components/app-shell/use-browser-download-notifications.tssrc/components/app-shell/use-browser-panel-state.tssrc/hooks/useAppSettings.tssrc/i18n/app-messages.en.tssrc/i18n/app-messages.zh.tssrc/main.tsxsrc/routes/Chat/BrowserPanel.tsxsrc/routes/Chat/ChatComposer.tsxsrc/routes/Chat/ContextMentionChips.tsxsrc/routes/Chat/composer-palette-items.test.tssrc/routes/Chat/composer-palette-items.tssrc/routes/Chat/context-mention-label.test.tssrc/routes/Chat/context-mention-label.tssrc/routes/Chat/useComposerPalette.tssrc/routes/Settings/index.tsxsrc/styles/app-shell.cssvite.config.ts
Summary
WebContentsViewandplaywright-core, with an authenticated loopback bridge and seven flat browser tools.Verification
corepack pnpm run ts-checkcorepack pnpm run lintcorepack pnpm run formatcorepack pnpm test— 276 files, 2,061 testscorepack pnpm run buildMachine-observable Electron 42 probes passed for the Playwright CDP relay, theme inheritance, and the light-only transparent-page fallback. Final human visual checks remain for the native browser panel, resize sash, long-title compression, and the referenced Zhihu page, following the repository instruction not to automate visual inspection.
Safety and Compatibility
playwright-coreadds no bundled browser binary. A macOS arm64 package comparison measured an increase of 10.999 MiB inapp.asarand 2.635 MiB in the final zip.