Repository navigation
feat(cef): in-process CDP DevTools transport (foundation) - #3498
Merged
senamakel merged 13 commits intoJun 9, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughMigrates CDP attachment to a dual-mode system: per-account in-process Webview transports discovered via a CdpRegistry, and a legacy WebSocket fallback. Refactors CdpConn to route calls/events to either backend and updates app setup, session, target attach, scanner, and account lifecycle wiring. ChangesIn-Process CDP Transport and Dual-Mode Connection
Sequence DiagramsequenceDiagram
participant Session as Session
participant CdpConn
participant Transport as WebviewCdpTransport
participant Webview as Webview<CEF>
participant Handler as on_dev_tools_protocol
participant Pending as pending map
Session->>CdpConn: call(method, params, sessionId?)
CdpConn->>Transport: call(...) (in-process)
Transport->>Pending: insert(id, oneshot)
Transport->>Webview: send_dev_tools_message(JSON)
Webview-->>Handler: dev tools frame
alt frame has id
Handler->>Pending: complete oneshot with result/error
else event frame
Handler->>Transport: broadcast EventFrame (method, params, sessionId)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
Bot
added
feature
![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: 4
🤖 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 `@app/src-tauri/src/cdp/in_process.rs`:
- Around line 278-337: Two concurrent calls to install_for_webview can both pass
the initial registry.by_label check and register duplicate on_dev_tools_protocol
observers with separate WebviewCdpTransport instances (different
next_id/pending), causing response routing races; make the install atomic by
inserting (or reserving) the transport into the CdpRegistry before registering
the observer and returning any existing entry if the insert fails. Modify
install_for_webview to: (1) create the WebviewCdpTransport (label, webview
clone, pending, events_tx) but do NOT register the observer yet; (2) attempt an
atomic "insert-or-get" on the CdpRegistry (replace registry.by_label +
registry.insert combo) so that if registry returns an existing transport you
immediately return it and drop the newly created transport; (3) only when the
insert succeeds (you are the single owner) call webview.on_dev_tools_protocol
and register the observer on that transport; use the existing symbols
install_for_webview, CdpRegistry, registry.by_label/registry.insert (or
implement an insert_if_absent/entry API on CdpRegistry) and WebviewCdpTransport
to locate and change the logic.
In `@app/src-tauri/src/cdp/mod.rs`:
- Around line 8-15: The doc comment overstates removal of the loopback DevTools
TCP listener; update the text to accurately reflect that the preferred CDP
transport is now in-process (see in_process and Webview::send_dev_tools_message
/ Webview::on_dev_tools_protocol) but the legacy TCP/WebSocket path driven by
CDP_PORT remains supported for unmigrated scanners; remove the absolute claim
that the `--remote-debugging-port=19222` flag was dropped and instead note the
security implications and that the legacy path still exists until scanners are
migrated.
In `@app/src-tauri/src/cdp/target.rs`:
- Around line 100-110: conn_for_account currently returns an error immediately
when CdpRegistry::by_account(account_id) is None; change it to call
install_for_account(app, account_id) on that cache miss, then re-check
registry.by_account and only error if the transport is still missing. Locate
conn_for_account and the registry lookup (registry.by_account), invoke
install_for_account with the same app and account_id, handle its Result (treat
non-fatal install failures by continuing to re-check), and then construct the
CdpConn via CdpConn::new(transport) if found, otherwise return the original "no
cdp transport" error.
In `@app/src-tauri/src/lib.rs`:
- Around line 2452-2457: The top comment in app/src-tauri/src/lib.rs that states
"We do NOT pass `--remote-debugging-port`" contradicts later code and the
migration note; update that comment to reflect the migration state or remove it.
Locate the paragraph referencing `--remote-debugging-port` and either delete it
or change the wording to something like "We are MIGRATING AWAY from
`--remote-debugging-port` — it is still passed temporarily during migration" so
it aligns with the later comment and the actual flag usage.
🪄 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: 49da2e7d-df1c-462a-9448-71f7c20dd6cb
📒 Files selected for processing (8)
app/src-tauri/src/cdp/conn.rsapp/src-tauri/src/cdp/in_process.rsapp/src-tauri/src/cdp/mod.rsapp/src-tauri/src/cdp/session.rsapp/src-tauri/src/cdp/target.rsapp/src-tauri/src/lib.rsapp/src-tauri/src/slack_scanner/mod.rsapp/src-tauri/src/webview_accounts/mod.rs
Wraps Webview::send_dev_tools_message + on_dev_tools_protocol into a request/response API matching the existing CdpConn::call surface. Per-webview transport multiplexes pending request ids and fans out events through a tokio broadcast. CdpRegistry stores transports keyed by webview label so generic <R: Runtime> callers can look them up without holding a typed Webview<Cef>. CEF_APP_HANDLE OnceLock lets install_for_account reach the concrete Webview<Cef> the Tauri-CEF API requires without an unsafe runtime-generic transmute. Unit-tested: inbound response routing, error frames, event broadcast, orphan responses.
CdpConn::new(Arc<WebviewCdpTransport>) wraps the new in-process transport. CdpConn::open_ws preserves the legacy tungstenite path so the per-scanner duplicates (whatsapp, slack, telegram, wechat, discord, meet_video) keep working until they migrate. Same call / pump_events surface for both backends.
conn_for_account(app, account_id) looks up the pre-installed WebviewCdpTransport from the per-app CdpRegistry and wraps it in a CdpConn. connect_and_attach_matching_in_process drives the Target.getTargets walk + attachToTarget over the in-process channel. Legacy browser_ws_url + connect_and_attach_matching kept for callers that still attach via the TCP loopback DevTools port.
Exposes CdpRegistry, install_for_account, install_for_webview, set_cef_app_handle, WebviewCdpTransport, EventFrame from the new in_process module. CDP_HOST / CDP_PORT kept inline because the four per-scanner CdpConn duplicates still need them.
run_session_cycle now picks up the in-process WebviewCdpTransport via conn_for_account instead of resolving the loopback DevTools WebSocket URL. Same Target.attachToTarget flatten attach, Page.enable, Browser.grantPermissions flow downstream — only the transport layer changes.
Two-line wiring so the new in-process CDP transport can install observers on per-account webviews: - app.manage(cdp::CdpRegistry::default()) - cdp::set_cef_app_handle(app.handle().clone()) in Builder::setup The setup-callback path is the canonical way to capture a Webview<Cef>-typed handle without an unsafe transmute from Webview<R>. --remote-debugging-port=19222 stays passed to CEF for now because the per-scanner CdpConn duplicates (whatsapp / slack / telegram / wechat / discord / meet_video) still attach over the loopback DevTools port.
… teardown After parent_window.add_child returns, install the in-process CDP transport for the new account-keyed webview. teardown_account_scanners forgets the transport so a reopen installs a fresh observer instead of re-using the dead one tied to the closed webview. Install failure is logged at warn (not fatal) so a transient cold-boot race against the scanner registry doesn't block the open.
The canonical CdpConn now has two constructors: - CdpConn::new(Arc<WebviewCdpTransport>) for the in-process channel - CdpConn::open_ws(ws_url) for the legacy loopback DevTools WebSocket slack_scanner's pin-resolution probe stays on the legacy path until the scanner is migrated.
Hold the registry mutex across the existence check, observer registration, and insert so two concurrent install_for_webview callers for the same webview label cannot both register on_dev_tools_protocol observers. CEF gives no un-register hook, so a double-registration would split inbound frames between two independent next_id counters and permanently break response routing.
conn_for_account previously hard-failed when the CdpRegistry had no entry for the account, but the account-open path in webview_accounts::open treats install_for_account errors as warnings and keeps the webview alive. After one such non-fatal install failure every later session retry would return the same "no cdp transport" error forever. Retry the idempotent install on cache miss so a transient failure recovers on the next call.
… transports The previous module doc claimed the loopback DevTools port was already gone. In reality the per-scanner CdpConn duplicates (discord, whatsapp, slack, telegram, wechat, meet_video) still use the TCP path, and lib.rs still passes --remote-debugging-port=19222 to keep that path alive. Reword to call out both transports explicitly and note the flag will be dropped once the scanners migrate.
The block above the CefCommandLineArg list claimed "We do NOT pass --remote-debugging-port" while a few lines below it the flag is still passed with value 19222 to keep the per-scanner TCP CdpConn duplicates alive during migration. Reword to match the actual code state — the flag is still set, the in-process channel is the destination for the ongoing migration, and the flag will be dropped once the remaining scanners cut over.
Apply rustfmt to in_process.rs (struct field indentation, get_or_create signature inlining, lock-chain compaction) and mod.rs (re-export ordering). No behavior change — pure formatting to satisfy `pnpm rust:format:check` in CI.
oxoxDev
force-pushed
the
feat/cef-in-process-devtools
branch
from
June 8, 2026 18:44
3fbf8f9 to
c5ef526
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src-tauri/src/cdp/target.rs (1)
3-9:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign the module rustdoc with the still-supported legacy transport.
Lines 6-7 say this module has “no more
browser_ws_url()HTTP discovery and no remote attach”, butbrowser_ws_url()andconnect_and_attach_matching()are still part of the public legacy path for unmigrated scanners. That comment describes the end state, not the current behavior.📝 Suggested wording
-//! that used to live in this module has been simplified — there is no -//! more `browser_ws_url()` HTTP discovery and no remote attach. The -//! remaining helpers (`Target.getTargets` walk, `Target.attachToTarget` -//! flatten-attach, detach) still apply because the page itself may -//! contain iframes / workers that the scanners care about. +//! that used to live in this module has been simplified. New code paths +//! use the per-account in-process channel, while the legacy +//! `browser_ws_url()` / remote-attach helpers remain for scanners that +//! still depend on the loopback WebSocket path. The remaining helpers +//! (`Target.getTargets` walk, `Target.attachToTarget` flatten-attach, +//! detach) still apply because the page itself may contain iframes / +//! workers that the scanners care about.🤖 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 `@app/src-tauri/src/cdp/target.rs` around lines 3 - 9, Update the module-level rustdoc to accurately state that legacy HTTP discovery and remote attach are still supported for unmigrated scanners by referencing the existing public functions; replace the sentence claiming “no more `browser_ws_url()` HTTP discovery and no remote attach” with wording that notes those legacy paths (e.g., `browser_ws_url()` and `connect_and_attach_matching()`) remain available alongside the simplified in-process flow, and keep the existing notes about `Target.getTargets` and `Target.attachToTarget` behavior for pages with iframes/workers; ensure the new phrasing mentions the legacy transport by name so readers can find `browser_ws_url()` and `connect_and_attach_matching()`.
🤖 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 `@app/src-tauri/src/cdp/target.rs`:
- Around line 123-129: The rustdoc for the attach sequence incorrectly implies
that dropping CdpConn will detach the in-process session; update the comment for
the function that describes the attach flow (references: CdpRegistry,
Target.getTargets, CdpConn, detach_session, Target.detachFromTarget) to clearly
state that the session id is managed outside of CdpConn, that CdpConn does not
implement Drop-based cleanup (no automatic Target.detachFromTarget), and that
callers MUST call detach_session (or explicitly call Target.detachFromTarget) to
avoid leaving the session alive in CEF. Ensure the wording removes any
suggestion that dropping CdpConn suffices and explicitly documents the required
teardown step.
---
Duplicate comments:
In `@app/src-tauri/src/cdp/target.rs`:
- Around line 3-9: Update the module-level rustdoc to accurately state that
legacy HTTP discovery and remote attach are still supported for unmigrated
scanners by referencing the existing public functions; replace the sentence
claiming “no more `browser_ws_url()` HTTP discovery and no remote attach” with
wording that notes those legacy paths (e.g., `browser_ws_url()` and
`connect_and_attach_matching()`) remain available alongside the simplified
in-process flow, and keep the existing notes about `Target.getTargets` and
`Target.attachToTarget` behavior for pages with iframes/workers; ensure the new
phrasing mentions the legacy transport by name so readers can find
`browser_ws_url()` and `connect_and_attach_matching()`.
🪄 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: a0fd6c58-4b1a-40c5-a113-bee125275f23
📒 Files selected for processing (8)
app/src-tauri/src/cdp/conn.rsapp/src-tauri/src/cdp/in_process.rsapp/src-tauri/src/cdp/mod.rsapp/src-tauri/src/cdp/session.rsapp/src-tauri/src/cdp/target.rsapp/src-tauri/src/lib.rsapp/src-tauri/src/slack_scanner/mod.rsapp/src-tauri/src/webview_accounts/mod.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- app/src-tauri/src/cdp/mod.rs
- app/src-tauri/src/cdp/conn.rs
- app/src-tauri/src/webview_accounts/mod.rs
- app/src-tauri/src/slack_scanner/mod.rs
- app/src-tauri/src/cdp/session.rs
- app/src-tauri/src/cdp/in_process.rs
Comment on lines
+123
to
+129
| /// Full short-lived attach sequence on the account's webview via the | ||
| /// in-process channel: look up the [`CdpRegistry`] transport for the | ||
| /// given account, find the matching page target via | ||
| /// `Target.getTargets`, attach with `flatten: true`. Caller gets a | ||
| /// ready `CdpConn` + session id. Caller MUST `detach_session` (or drop | ||
| /// the `CdpConn`) when done so the session id doesn't linger inside | ||
| /// CEF. |
Contributor
There was a problem hiding this comment.
Don't imply that dropping CdpConn detaches an in-process session.
On this path the session id lives outside CdpConn, and CdpConn has no Drop-based Target.detachFromTarget cleanup. The current rustdoc tells callers they can skip detach_session, which would leave the attached target session alive in CEF.
📝 Suggested wording
-/// ready `CdpConn` + session id. Caller MUST `detach_session` (or drop
-/// the `CdpConn`) when done so the session id doesn't linger inside
-/// CEF.
+/// ready `CdpConn` + session id. Caller MUST call `detach_session`
+/// when done; dropping `CdpConn` alone does not detach the target
+/// session from CEF.🤖 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 `@app/src-tauri/src/cdp/target.rs` around lines 123 - 129, The rustdoc for the
attach sequence incorrectly implies that dropping CdpConn will detach the
in-process session; update the comment for the function that describes the
attach flow (references: CdpRegistry, Target.getTargets, CdpConn,
detach_session, Target.detachFromTarget) to clearly state that the session id is
managed outside of CdpConn, that CdpConn does not implement Drop-based cleanup
(no automatic Target.detachFromTarget), and that callers MUST call
detach_session (or explicitly call Target.detachFromTarget) to avoid leaving the
session alive in CEF. Ensure the wording removes any suggestion that dropping
CdpConn suffices and explicitly documents the required teardown step.
Member
Author
|
@coderabbitai review — all four prior comments addressed in 97e240e / 8d3b607 / 9b72c78 / cef6dca; rebased onto upstream/main; CI green. |
Contributor
|
✅ Action performedReview finished.
|
12 tasks done
Merged
6 tasks done
Merged
7 tasks done
senamakel
pushed a commit
to nocstah/openhuman
that referenced
this pull request
Sep 11, 2026
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
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
cdp::in_processmodule routes Chrome DevTools Protocol traffic for embedded CEF webviews throughWebview::send_dev_tools_message+on_dev_tools_protocol(the Tauri-CEF in-process channel) instead of a loopback TCP WebSocket.CdpConnis now dual-backend (Backend::InProcess/Backend::LegacyWs). New code paths attach via the in-process channel; the loopback WS path is kept for the per-scannerCdpConnduplicates that have not migrated yet.cdp/session.rs) is the first caller migrated — everywebview_accounts::opennow installs an in-process transport onadd_child, and the session opener picks it up viacdp::conn_for_account.--remote-debugging-port=19222continues to be passed to CEF for now because the per-scanner WS-basedCdpConnduplicates (whatsapp,slack,telegram,wechat,discord,meet_video/inject.rs) still attach over the loopback DevTools port. Follow-up PR per scanner; once all six migrate, the flag drops.Problem
--remote-debugging-portloopback HTTP+WS endpoint. The endpoint has no per-connection authentication and is reachable by any same-UID local process — surfaced via a reporter PoC that attached puppeteer to127.0.0.1:19222, walkedTarget.getTargets, and read embedded WhatsApp message content.send_dev_tools_message+ anon_dev_tools_protocolobserver. That channel lives inside the process — there is no listener for an external attacker to attach to. Five-filecdp/module + four duplicatedCdpConnshims kept us on the TCP path because the existing dispatch was written againsttokio-tungstenitebefore the in-process API was available.Solution
cdp::in_process(new) —WebviewCdpTransportholds the pending-id table + tokio broadcast for events. One transport per webview, installed atwebview_accounts::opentime viacdp::install_for_account(account_id). TheOnceLock<AppHandle<tauri::Cef>>populated inBuilder::<Cef>::setuplets the install path reach the concreteWebview<Cef>typed handle (send_dev_tools_messageis gated on it) without an unsafe runtime-generic transmute.cdp::CdpRegistry— managed viaapp.state()so generic<R: Runtime>callers (scanners, the per-account session opener) can look up an installed transport by account id without needing a typedWebview<Cef>themselves. Install is atomic per webview label: registry mutex is held across the existence check, observer registration, and insert so two concurrent installs for the same label can never both attach observers (CEF gives no un-register hook).cdp::CdpConn— dual-backend wrapper.CdpConn::new(Arc<WebviewCdpTransport>)for the in-process path;CdpConn::open_ws(url)keeps the legacy tungstenite WebSocket constructor for callers that haven't migrated. Both expose the samecall(method, params, session_id)/pump_events(session_id, on_event)surface, so callers swap transport without changing their dispatch logic.cdp/session.rs— migrated.run_session_cycleresolvesconn_for_account(app, account_id)instead ofbrowser_ws_url(); on a registry cache missconn_for_accountretries the idempotent install so a transient non-fatal install failure duringwebview_accounts::opendoesn't permanently lock the account out of CDP. The downstreamTarget.attachToTarget+Page.enable+Browser.grantPermissionsflow is unchanged.teardown_account_scannersevicts the transport from the registry so account reopen installs a fresh observer instead of re-using one tied to the closed webview.The TCP loopback debug port stays open for now — see follow-up below.
Submission Checklist
cdp::in_process::testscovers inbound-response routing, inbound-error frames, event broadcast, orphan responses.open_ws,browser_ws_url,connect_and_attach_matching) are exercised by existing scanner tests. (Original item: Diff coverage ≥ 80%.)## Related.)docs/RELEASE-MANUAL-SMOKE.md.)## Relatedbelow. (Original item: Linked issue closed viaCloses #NNNin the## Relatedsection.)Impact
send_dev_tools_messagewhich posts onto the CEF main thread. Equivalent to existingPage.navigate/Browser.grantPermissionscalls that already cross that boundary; no measurable regression expected for the per-account session opener (one attach + one navigate + one grant per account, all already on the main thread today).spawn_blockingwraps the send to keep tokio workers unparked.whatsapp/slack/telegram/wechat/discord/meet_video/inject.rs) continue to attach via the loopback DevTools WS path —browser_ws_url,CdpConn::open_ws, and the legacyconnect_and_attach_matchingare preserved.Related
whatsapp_scanner::CdpConnto in-process transportslack_scanner(probe + main attach)telegram_scanner::CdpConnwechat_scanner(singleconnect_and_attach_matchingcallsite)discord_scanner::CdpConn(MITM event stream)meet_video/inject.rs(singleconnect_and_attach_matchingcallsite)--remote-debugging-port=19222fromlib.rs, delete legacycdp::CdpConn::open_ws/cdp::browser_ws_url/cdp::connect_and_attach_matching/cdp::CDP_HOST/cdp::CDP_PORTAI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
app/src/**source touched. (Original item:pnpm --filter openhuman-app format:check.)app/src/**/*.tstouched. (Original item:pnpm typecheck.)cargo test --manifest-path app/src-tauri/Cargo.toml --lib cdp::in_process→ 4/4 pass.cargo fmt --check --manifest-path app/src-tauri/Cargo.tomlclean andcargo check --manifest-path app/src-tauri/Cargo.tomlclean.Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
CdpConn::open_ws,cdp::browser_ws_url,cdp::connect_and_attach_matchingretained verbatim for un-migrated scanners.--remote-debugging-port=19222still passed to CEF until every scanner migrates.run_session_cyclekeeps the same target-by-fragment match (target_matches_account_url), the sameTarget.attachToTarget {flatten: true}shape, the samePage.enableordering, the sameBrowser.grantPermissionsorigin-scoped grants per provider.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Refactor
Bug Fixes