Skip to content

feat(browser): drive Electron desktop apps over CDP — hermes browser attach + session registry + escalation nudge (#89270) - #89340

Closed
kshitijk4poor wants to merge 3 commits into
NousResearch:mainfrom
kshitijk4poor:feat/electron-cdp-attach
Closed

feat(browser): drive Electron desktop apps over CDP — hermes browser attach + session registry + escalation nudge (#89270)#89340
kshitijk4poor wants to merge 3 commits into
NousResearch:mainfrom
kshitijk4poor:feat/electron-cdp-attach

Conversation

@kshitijk4poor

@kshitijk4poor kshitijk4poor commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Implements the full plan from #89357 in one PR — all three layers plus the CLI affordance, E2E-verified against a live Electron app.

Real-world impact (WHO / WHEN / WHY)

WHO: anyone asking the agent to drive a desktop Electron app — "organize my Obsidian vault", "post this in Slack", "toggle this VS Code setting".
WHEN: today the model reaches for computer_use, background input gets dropped by the unfocused Chromium renderer (suspected_noop), and the only escalation is foreground — a visible focus steal that disrupts the user mid-work.
WHY: a CDP attach gives exact DOM-level control with zero focus steal. The transport already worked (BROWSER_CDP_URL → browser_exec), but it was global-only (attaching an app hijacked ALL web browsing) and nothing made it discoverable at planning time, task-match time, or the failure point.

What this adds

1. hermes browser attach / list / detach (new CLI command; hermes_cli/browser_attach.py + hermes_cli/subcommands/browser.py)

  • Scans running processes for Electron main processes (resources/app.asar signature — packaged, unpacked, and macOS bundle layouts; --type= children excluded), probes advertised debug ports for live CDP.
  • Registers the endpoint under a named browser_exec session in $HERMES_HOME/browser-sessions.json (written via the existing utils.atomic_json_write).
  • For a detected app with no exposed port: offers the quit-and-relaunch with --remote-debugging-port=<free port ≠ 9222> itself, waiting on the existing dual-stack readiness probe.
  • Consent model: CDP into a live app exposes everything it can access (DMs, vault contents, tokens) — same power class as cua-driver's gated existing_profile attach. The agent never opens debug ports; this command is user-invoked and its relaunch confirmation is the consent moment. The skill instructs the model to ask the user to run it, never to relaunch apps via terminal.

2. Named-session registry resolution in browser_exec (tools/browser_use_cli.py)

  • A session registered by the attach command pins THAT session to the app's endpoint; the default session (and any other name) keeps browsing the web normally. Precedence: explicit BU_CDP_* env > session registry > global /browser connect override > cloud provider.
  • Electron CDP quirk, found E2E: Electron rejects Target.createTarget (Not supported), and the harness's named-daemon path fatally calls exactly that to mint its dedicated tab. App sessions therefore run the harness's default-name path (attach-to-existing-page — correct for an app, whose window IS the page), isolated per session via BH_HOME (the harness's own home var — no new HERMES_* env surface).

3. Failure-point nudge (tools/computer_use/tool.py, tools/computer_use/cua_backend.py)

  • _enrich_escalation() gains a second additive branch mirroring the existing typed-page one: when the driver recommends foreground and the acting pid's executable carries the Electron signature, the escalation payload adds alternative: "cdp_attach" + a hint naming hermes browser attach and the skill. The driver's recommended is never changed; detection runs only on the escalation path (zero cost on success); typed-page branch takes precedence for real browser windows.
  • The cua backend threads the acting pid into escalation meta additively (only when an escalation is present, never overwriting a driver-supplied pid).
  • This is reaction-only, per the shipped ladder doctrine ("never predict from the app being Electron") — many Electron controls DO accept background AX input, and nothing here changes rung ordering.

4. Bundled skill drive-electron-apps + one schema sentence

  • Skill: detection recipe, per-platform relaunch (incl. the single-instance-lock pitfall), target picking from /json/list, React native-value-setter / Radix one-eval / Input.insertText patterns, the window.app tip, consent rules, and when computer_use remains the right tool.
  • Schema: one sentence on the browser_exec header pointing at the attach flow. Note: the header was A/B benchmarked and pinned (Aug 2026, comment at tools/browser_use_cli.py:768) — this addition is outside that benchmark's coverage, flagging per that comment's intent. The description is computed at tool-definition time, so it remains byte-stable for the life of a conversation (no cache break).

Verification (all real output, macOS)

  • Live E2E: hermes browser attach obsidian against running Obsidian 1.10.6 → relaunch-free attach to its exposed port; browser_exec(code=…, session='obsidian') returned the real DOM title and full window.app access (vault name, file count, active file). Slack/Discord/Bitwarden/ChatGPT correctly detected as Electron; --no-relaunch refusal path, interactive detach/list, and a --session "My Obsidian" slug round-trip all exercised live.
  • Isolation proof: unnamed sessions and unregistered named sessions leave the env untouched (no BU_CDP_*, no BH_HOME); explicit BU_CDP_URL env still wins over the registry.
  • Tests: 161 passed — the full existing test_browser_use_cli.py + test_computer_use_delivery_ladder.py + browser-connect suites, plus new: registry round-trip/precedence, Electron detection across the three layouts, cmdline parsing, escalation-branch contract (driver verdict never overridden, typed-page precedence, silent skip without pid).
  • Mutation checks: disabling the escalation branch fails its 2 guard tests; disabling the registry consult fails 3 precedence tests; both restored green.
  • _BUILTIN_SUBCOMMANDS ↔ argparse parity contract test green with the new browser command; ruff clean.

Review gates run

/simplify-code (3 parallel reviewers on the full diff): reuse HIGH (hand-rolled atomic write → utils.atomic_json_write) and quality HIGH (third inline .app-name derivation → module's own app_display_name) both verified and folded as the second commit; efficiency reviewer dismissed its candidates with call-frequency evidence (registry read is opt-in per named session, consistent with existing per-call config reads in the same function). hermes-pr-review Phase 2 checks (dispatch-site completeness — static + dynamic schema paths both carry the note; wiring parity; E2E re-run post-cleanup) on the full final diff.

Closes the Phase 1–3 checkboxes of #89357.

Driving desktop Electron apps (Obsidian, Slack, VS Code, ...) with
computer_use background input frequently fails: Chromium drops synthetic
pointer events into occluded, unfocused renderers, and the only shipped
escalation is foreground (visible focus steal). A CDP attach gives exact
DOM-level control with zero focus steal — the transport already worked
via BROWSER_CDP_URL, but nothing made it discoverable or per-session.

Closes the gap at three layers (cheapest first), per the plan on NousResearch#89270:

1. hermes browser attach|list|detach (new CLI command): scans running
   processes for Electron main processes (resources/app.asar signature,
   no --type= child marker), probes advertised debug ports, and registers
   the endpoint under a named browser_exec session in
   $HERMES_HOME/browser-sessions.json. For a detected app without an
   exposed port it offers the quit-and-relaunch with
   --remote-debugging-port itself — user-invoked, so the confirmation
   prompt is the consent moment (same doctrine as cua-driver's
   grant_existing_profile gate; the agent never opens debug ports).

2. browser_exec named-session registry resolution: a session registered
   by the attach command pins THAT session to the app's CDP endpoint
   while the default session keeps browsing the web. App sessions run
   the harness's default-name daemon isolated via a per-session BH_HOME:
   Electron rejects Target.createTarget ('Not supported'), which the
   harness's named-daemon dedicated-tab path calls fatally on attach —
   the default-name path attaches to the existing page instead (the app
   window IS the page). Proven E2E against live Obsidian 1.10.6.

3. Failure-point nudge: _enrich_escalation gains an Electron branch —
   when the driver recommends foreground and the acting pid's executable
   carries the Electron signature, the escalation payload adds
   alternative:'cdp_attach' pointing at hermes browser attach and the
   new drive-electron-apps skill. Reaction-only per the ladder doctrine
   (never predicts from app identity); the pid is threaded into
   escalation meta additively by the cua backend, and detection runs
   only on the escalation path (zero cost on success).

Plus the bundled drive-electron-apps skill (detection, relaunch recipe,
target picking, React value-setter / Radix one-eval patterns, consent
rules) and one sentence on the browser_exec schema header so the model
considers the route at planning time (flagged: outside the Aug 2026
header A/B benchmark's coverage).

Tests: registry round-trip + precedence (session > global override,
explicit env wins, unnamed/unregistered sessions untouched), Electron
detection (packaged/unpacked/macOS bundle layouts), cmdline parsing,
escalation branch (driver verdict never overridden, typed-page branch
precedence, silent skip without pid), _is_electron_pid guards. 127
passed including the full existing browser_use_cli + delivery-ladder
suites; CLI wiring, live scan, attach, and DOM read verified E2E on
macOS against running Obsidian/Slack/Discord.

Refs NousResearch#89270
- _write_registry: use the existing utils.atomic_json_write (temp file +
  fsync + os.replace, parent mkdir) instead of a hand-rolled tmp+replace
  (reuse reviewer, verified against utils.py:346 and its existing
  hermes_cli callers).
- _terminate_app: derive the macOS app name via the module's own
  app_display_name() instead of a third inline .app-suffix derivation
  (quality reviewer).
- attach CLI: slug user-supplied --session names too, so a name with
  spaces/uppercase matches browser_exec's session grammar instead of
  failing at first use (found during gate testing: registered sessions
  were unreachable if the name didn't match _SESSION_RE).

Efficiency reviewer: no material findings (registry read is one
open+json.load per named-session call, consistent with the existing
per-call config reads in the same function; scan/relaunch paths are
user-invoked CLI only). Mutation checks: escalation-branch guard tests
and registry-precedence tests both fail when their production code is
disabled, pass restored. 41 browser-connect + attach tests green.

@andrexibiza andrexibiza left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 8d4bb8c0bc7e1ba3e6df500c5fb3396490200328 against base 9664e386f67965ec8bec5cf3db9d411f2c2b6cc0, including the CLI attach/relaunch path, registry resolution, browser_exec daemon isolation, computer-use escalation enrichment, bundled skill, tests, #89270's consent/ladder contract, and the adjacent shared-CDP ownership work in #86879.

The direction is strong: per-session registry resolution avoids hijacking the default web browser, the escalation branch is genuinely reaction-only, and the typed-page alternative still wins. I do not think this head is ready to merge yet, because three ownership/authority invariants are still weaker than the PR/issue claims.

1. Blocking: the "user consent" boundary is prompt text, not enforced authority

#89270 explicitly classifies this as the same power class as cua-driver existing_profile attachment and says the agent must never open the debug port itself. The implementation does not enforce that distinction. hermes browser attach is an ordinary CLI subcommand, and the only relaunch authorization is input("Quit and relaunch ... [y/N]"). A model with terminal can invoke that same command and feed stdin (printf 'y\n' | hermes browser attach ... / PTY equivalent), causing _terminate_app() + _spawn_with_debug_port() itself. The bundled skill saying "USER runs this, not you" is useful guidance, but it is not an authority boundary.

This repository already learned this exact lesson in CuaTypedBrowserRoute.prepare: computer_use.grant_existing_profile has a host-side floor because prompt/approval bypass must not silently turn into permission to expose a live browser profile. This PR claims equivalent sensitivity (Slack DMs, Bitwarden/ChatGPT, localStorage tokens) but provides only a conversational rule.

Please make the execution layer enforce a real grant for opening/relaunching an app with CDP. The interactive prompt can remain UX, but it cannot be the sole authorization proof. Reuse the existing existing-profile grant model or add an equivalent browser-attach capability whose validation the model cannot satisfy merely by writing y to stdin. Add a regression that invokes the attach path from a non-user/automated caller and proves no process is terminated and no debug port is opened without that grant.

2. Blocking: the registry binds a session to a port, not to the app/process that was authorized

save_session_endpoint() persists only {cdp_url, app, attached_at}. Later resolve_session_endpoint() returns that URL verbatim, and _resolve_backend_cdp() converts it into BU_CDP_URL / BU_CDP_WS without revalidating the owner of the listener.

The skill even notes that the registry outlives the process, but assumes the failure mode is "endpoint is dead." A stale port is not necessarily dead: after the authorized Electron app exits, that loopback port can be reused by another Chromium/Electron process. The named session would then silently attach to a different app than the one the user authorized. app is display metadata only; it is never checked.

This is the same identity-collapse class #86879 is fixing for shared CDP targets: an endpoint is routing information, not durable ownership proof. Capture ownership evidence when the attach is minted (at minimum executable identity + PID/kernel start fingerprint for the listening process, or an equivalent opaque attachment generation), and revalidate before browser_exec adopts the registry entry. If the process/generation changed or cannot be proven, fail closed and require a fresh attach. Add a port-reuse regression: authorize app A, retire it, reuse the port for B, and prove session A cannot drive B.

3. Blocking contract regression: app-attached session=<name> no longer provides the named-session isolation browser_exec already promises

The browser_exec header still says named sessions are for "parallel tasks that must not share tabs." For registered app sessions, however, _resolve_backend_cdp() deliberately pop("BU_NAME") and runs the harness's default-name attach-to-first-page path. BH_HOME gives each session a separate socket/log/pid directory, but it does not give it a separate CDP target.

So two different registered session names that point at the same Electron endpoint can launch two isolated daemons that both attach to the same first real page. Multi-window Electron apps have the same ambiguity. That recreates the cross-task target-sharing class #86879 is eliminating—just on browser_exec's Electron path. The PR's tests prove env/daemon-home isolation, but not target isolation.

Electron rejecting Target.createTarget means you cannot solve this by minting a blank tab, but the contract still needs an exact existing-target lease: select/store an opaque target id for the app session and fail closed if it disappears, or explicitly narrow the model-facing contract so app-attached names are not advertised as isolated concurrent sessions. Please add a two-session/same-endpoint witness that proves the sessions cannot mutate the same page accidentally.

Interlocks / topology

  • Preserve #89270 / @kshitij-eliza as the design/provenance root; this PR is implementing its phases, not replacing that authorship.
  • #86879 / @the3asic is adjacent rather than duplicate: it owns exact target/session lifecycle for the high-level shared-CDP path. Its core invariant—endpoint sharing does not imply target ownership—is directly applicable here and should be reused rather than re-solved inconsistently.
  • The reaction-only escalation behavior and typed-browser precedence looked correct in this head; I found no reason to block those pieces independently.

Exact-head GitHub status currently exposes no commit statuses/checks, so I am treating CI as not yet evidenced, not green. The PR's local 161 passed + live Obsidian E2E are useful positive evidence, but they do not exercise the three authority/ownership cases above.

Re-review gate: enforce attach authority below prompt text, bind registry entries to a verifiable app/process generation, and preserve real target isolation (or narrow the advertised session contract) for multiple app-attached names.

The consolidated 3-reviewer batch carried findings beyond the two HIGHs
already folded; each verified against the code before applying:

- scan/relaunch probes: use discover_local_cdp_url (dual-stack) instead
  of a hand-rolled IPv4-only probe — an IPv4-loopback squatter can push
  a relaunched app's debug listener onto [::1] only, and the IPv4 probe
  would falsely report 'did not come back up' (reuse reviewer; this is
  the exact failure mode browser_connect documents).
- _terminate_app: give a successful AppleEvent quit a 5s grace window
  before SIGTERM — terminating immediately defeated the graceful quit
  the osascript step exists for (quality reviewer).
- cua_backend._action: decide the escalation-pid injection BEFORE
  building the ActionResult instead of mutating res.meta afterwards,
  which silently relied on _action_result_from aliasing the meta dict
  (quality reviewer; verified: same observable payload, no aliasing
  contract).
- _resolve_backend_cdp: extract _set_cdp_env — the registry branch had
  added a third copy of the BU_CDP_URL/WS export idiom (quality
  reviewer).
- attach CLI: show the numbered picker whenever a filter matches
  multiple apps ('code' → Code + Code - Insiders) instead of silently
  taking the first (quality reviewer).
- session_slug: strip leading underscores too (browser_exec's grammar
  requires an alnum first char — a '_private-app' slug registered fine
  but was unreachable); new contract test locks slug ∈ _SESSION_RE for
  adversarial names (quality reviewer).
- scan_electron_apps: seen_exes dict→set (stored pids never read).
- app_display_name: implement on top of _bundle_path so the module has
  ONE .app scanner (quality reviewer).

36 attach+escalation tests green (incl. the new grammar contract test),
delivery-ladder suite green, live browser_exec against Obsidian re-run
green after the pid-injection restructure. ruff clean.
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard comp/tools Tool registry, model_tools, toolsets tool/browser Browser automation (CDP, Playwright) labels Aug 18, 2026
@kshitijk4poor

Copy link
Copy Markdown
Collaborator Author

Note: two earlier bookkeeping items on this work went out from a secondary account (kshitij-eliza) by mistake — the original tracking issue #89270 and one status comment. #89270 is closed and re-filed as #89357 under this account; the PR body now points at #89357. Commit authorship on this branch was always correct (kshitij 82637225+kshitijk4poor@users.noreply.github.com). Prior status update for the record: commit 7a9824c folded the remaining 3-reviewer batch findings (dual-stack CDP probes in scan/relaunch, AppleEvent-quit grace window before SIGTERM, escalation-pid decided before ActionResult construction, _set_cdp_env extraction, ambiguous-filter picker, session_slug grammar fix + contract test); 36 attach/escalation tests + delivery ladder green, live Obsidian E2E re-run green.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator Author

Superseded by #89372 (same branch content, re-opened for account-hygiene cleanup so every artifact — issue, PR, comments — is authored by kshitijk4poor). Tracking: #89357.

@kshitijk4poor
kshitijk4poor deleted the feat/electron-cdp-attach branch August 18, 2026 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have tool/browser Browser automation (CDP, Playwright) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants