Conversation
…ug:"browser" (Part 1) New desktop-only `browser_companion` domain owning the tinyflows CompanionServer lifecycle + pairing (loopback WS relay, 0600 secret), an opt-in ServiceSet::companion_relay service, and first-class routing of `slug:"browser"` flow tool_calls to the paired Chrome tab via RoutingToolInvoker. Author-advisory / run-hard readiness gate mirrors the inference-readiness (B45) posture. Rides the flows Cargo feature; compiled out cleanly when flows is off. Depends on tinyflows Stage B (CompanionServer relay/bind handles) + v0.6 release.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a feature-gated Browser Companion relay with pairing persistence, browser toolkit contracts, readiness validation, and tab-scoped flow execution. Desktop services start the relay when enabled, while browser runs bind to shared tabs and clean up bindings afterward. ChangesBrowser Companion configuration and lifecycle
Browser flow integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant FlowSchema
participant FlowOps
participant CompanionRelay
participant ChromeExtension
Client->>FlowSchema: Submit flow with browser_tab_id
FlowSchema->>FlowOps: Start tab-scoped flow
FlowOps->>CompanionRelay: Bind run to shared tab
FlowOps->>ChromeExtension: Route browser actions
CompanionRelay-->>FlowOps: Unbind run
FlowOps-->>Client: Return result or readiness error
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
coderabbitai
Bot
added
bug
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@src/core/runtime/builder.rs`:
- Line 89: Add regression tests for companion-relay selection: in
src/core/runtime/builder.rs at lines 89, 110, 131, and 162, assert that
ServiceSet::desktop() enables companion_relay while headless_api(), none(), and
embedded() disable it; at lines 733-736, verify the selected flag reaches relay
startup only when flows is enabled; in src/core/runtime/services.rs lines
230-255, test both enabled and disabled configuration gates without starting a
real relay.
In `@src/openhuman/browser_companion/ops.rs`:
- Around line 125-140: Update start_with_extension_id and stop_companion_server
to synchronize runtime state with the spawned listener task: report bind
failures before returning success, clear server/task state on unexpected task
exit, and await the aborted task during shutdown so immediate restarts occur
only after teardown completes.
- Around line 287-307: Update pair and the related CompanionRuntime state to
retain the supplied extension_id as the active ID after start_with_extension_id
succeeds. Make companion_status report this active runtime ID and make
rotate_secret use it when restarting, including when the configured ID is
disabled or default, while preserving the existing pairing flow.
In `@src/openhuman/browser_companion/types.rs`:
- Around line 27-36: Add tests for the SharedTab to SharedTabView conversion
implemented by From<tinyflows::companion::SharedTab> for SharedTabView, using an
inline #[cfg(test)] module or sibling Rust test file. Construct a representative
SharedTab and assert id, window_id, url, and title are preserved while
relay-internal state is not exposed in the resulting view.
In `@vendor/tinyflows`:
- Line 1: Update the vendor/tinyflows gitlink to the exact commit referenced by
the v0.6.0 release tag, ensuring the submodule resolves to that tagged revision
and not another commit.
🪄 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: 3c12f436-72de-4539-9806-927c188dad5f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
src/core/runtime/builder.rssrc/core/runtime/services.rssrc/openhuman/browser_companion/mod.rssrc/openhuman/browser_companion/ops.rssrc/openhuman/browser_companion/store.rssrc/openhuman/browser_companion/types.rssrc/openhuman/config/schema/browser_companion.rssrc/openhuman/config/schema/mod.rssrc/openhuman/config/schema/types.rssrc/openhuman/flows/builder_tools.rssrc/openhuman/flows/builder_tools_tests.rssrc/openhuman/flows/node_contracts.rssrc/openhuman/flows/ops.rssrc/openhuman/flows/ops_tests.rssrc/openhuman/flows/schemas.rssrc/openhuman/mod.rssrc/openhuman/tinyflows/caps.rsvendor/tinyflows
…id retention, coverage, CI gates - ops.rs: reap dead listener state + is_running() (bind failure no longer reports running / blocks restart); stop awaits the aborted task so the loopback port is freed before an immediate pair/rotate restart; retain the live extension id in CompanionRuntime so status + rotate_secret use it (not stale Config) — fixes pair-then-rotate leaving the relay down. - Tests: ServiceSet companion_relay desktop-only; SharedTab->SharedTabView mapping; lifecycle extended to cover pair(new id) + rotate_secret restart. - CI: add core/runtime/builder.rs to the feature-gate-smoke allowlist; regenerate app/src-tauri/Cargo.lock for the new tinyflows transitive deps (--locked).
Route browser_relay/is_extension_connected/bind_run/unbind_run and companion_status's server observations through a with_live_server() helper that reaps a dead listener task and returns None unless is_running(). Closes the CodeRabbit follow-up: these APIs no longer read or operate on a stale server handle whose serve() has already exited.
# Conflicts: # app/src-tauri/Cargo.lock
…ipping the feature-gate-smoke allowlist The explanatory note in types.rs contained a literal `#[cfg(feature = "flows")]`, which the rust-feature-gate-smoke lane greps for when building its gated-test allowlist — so adding a test to this file falsely flagged it as a new gated-test module. Reworded to not embed the attribute; the file gates no test.
|
Maintainer pass (merge-readiness sweep). I picked this up to rebase and get it green, but stopped before touching the branch: the blocker you named in the description has hardened rather than cleared, and resolving the conflicts would mean guessing. Findings below, with the relocation map so the eventual rebase is cheap. The vendored dependency is the hard blockerThe PR body says this "must not merge until tinyflows cuts v0.6.0 and this submodule points at the release." That release never happened, and the branch it was cut from is gone:
So there is no pin that both satisfies this PR and is legal for
This needs the tinyflows companion work merged to the tinyflows mainline and released before the OpenHuman side can move at all. That is a decision for you and whoever owns tinyflows, not something a rebase can resolve — which is why I have left the branch untouched. The source-side conflicts are mechanical, but only after the aboveMerging current
Two things worth knowing before you rebase: StatusMarking this blocked on an upstream decision in the maintainer sweep rather than stale. Nothing here is a criticism of the change — the wiring reads well and the staged plan is clear; it is waiting on a tinyflows release that has not been cut. If the companion work is not going to land upstream, that is worth saying out loud so this draft can be closed rather than carried. No commits, no pushes, no force-push to this branch. |
|
@graycyrus — I picked this up to rebase it onto current The blocker: tinyflows deleted the companionThis PR is wired to
So this is not a merge conflict. The upstream capability the PR wires in was removed a month ago. Why the submodule pointer has no correct resolution
Between your merge base and now, tinyflows went from #79's own summary points at the alternative: "Browser automation remains available as a host-managed capability through the existing tool invocation interface." That reads like the upstream position is that the relay belongs in the host, not in tinyflows — which would be a real redesign of Part 1, not a rebase. Your call, and possibly one to take with @senamakel first. Everything else is mechanical — here is the map, if you revive itI ran the rebase far enough to enumerate the conflicts. Eleven files conflict; ten are pure relocation from the kernelization (#5328), and I confirmed every anchor function you patch still exists on
Other moves:
One placement decision is yours, not mine. What I did and did not do
Happy to do the ten mechanical relocations the moment the tinyflows question has an answer — it is maybe an hour of careful work, but it is wasted until there is a |
3 participants
Summary
tool_callnode withslug:"browser"drives the user's real, signed-in Chrome tab (16 CDP actions: open/click/fill/type/screenshot/find/…). This is Part 1 — core wiring of a staged feature (plan below); no user-facing surface yet (RPC + Settings pairing UI land in Part 2).browser_companiondomain owns the tinyflowsCompanionServerlifecycle (loopback WS relay + pairing secret,0600), exposed via a new opt-inServiceSet::companion_relayservice gated onconfig.browser_companion.enabled.slug:"browser"is now a built-in flow tool_call: routed to Chrome viaRoutingToolInvokerat run time, taught to the author-time curation gates (no Composio connection needed), with an author-advisory / run-hard readiness posture mirroring the existing inference-readiness (B45) pattern.flowsCargo feature (no new compile gate); compiled out cleanly whenflowsis off.Problem
Flows can orchestrate Composio/LLM/HTTP effects but cannot act on a live web page the user is signed into. tinyflows shipped a purpose-built Chrome companion (branch
feat/chrome-extension) whose own doc states "OpenHuman host wiring is intentionally separate follow-up work." This PR is that wiring.Solution
Data path: flow run →
build_capabilities()→ wrapOpenHumanToolsinRoutingToolInvoker(ChromeToolInvoker(relay, run_id=thread_id, tab_id))→ abrowsernode executes over the loopback relay to the shared tab; every other slug/connection_refflows through untouched.src/openhuman/browser_companion/(mod/types/ops/store): start/stop/pair/unpair/rotate-secret/status overtinyflows::companion::CompanionServer; secret at{workspace}/browser_companion/relay.secret.ServiceSet::companion_relay(desktop()=true,headless_api()/none()=false— never in cloud, matching the security model).spawn_companion_relay_service()no-ops unless enabled.build_capabilitiessite inrun_flow_body(the dry-run/mock path does not route through it, so nothing to mis-wrap). Binds run→tab via a Stage-BCompanionServer::bind_runand releases it with an RAII drop guard.validate_tool_contractsvalidates thebrowseraction against the 16-action set instead of the Composio catalog;validate_connection_refsskips browser; a run-time hard gatevalidate_browser_readiness(running + extension connected +browser_tab_id) fails a real run cleanly before the engine executes; the author path stays advisory (no hard gate inrun_builder_gates— see the B45 design note atflows/ops.rs).OpenHumanTools::invokereturns a clear "companion not running / no tab" error as a fallback.list_connectable_toolkitssurfaces{toolkit:"browser", connected, type:"builtin"}for the workflow_builder agent.tinyflows dependency (branch-tip, release-later — deliberate)
The vendored
vendor/tinyflowssubmodule is bumped to thefeat/chrome-extensiontip + a small additive API (browser_relay/is_extension_connected/shared_tabs/bind_run/unbind_runonCompanionServer, needed by an embedding host) — see companion tinyflows PR. This OpenHuman PR is a DRAFT and must not merge until tinyflows cuts v0.6.0 and this submodule points at the release. Version-compat is clean (both sides on tinyagents 2.1).Submission Checklist
invokefallback).invokefallback) are unit-tested. The few live socket-routing lines inrun_flow_bodyaren't unit-coverable (they need a live extension); the JSON-RPC E2E lands with the RPC surface in Part 2.N/A: new feature, matrix row added with the RPC surface in Part 2.## Related.feat/chrome-extensionalready vendors (axum 0.8 / tokio full, additive).N/A: no user-facing surface yet (Part 2).N/A: net-new feature; staged plan tracked in my_docs/browser_companion_integration/PLAN.md(gitignored).Impact
browser_companion.enabled=false); the relay never binds in headless/cloud. Security model preserved end-to-end: 127.0.0.1-pinned listener, exactchrome-extension://<id>origin + token auth, only explicitly-shared tabs, all extension JS bundled locally.flowsis off (domain compiled out) or when the companion is disabled (browser nodes fail closed with a clear message).Related
browser_companion_*RPC namespace + Settings pairing UI + extension-initiated runs (inbound control channel →flows_run); Part 3 — extension resource shipping in the Tauri bundle +workflow_builderprompt.md; persistextension_idvia config (TODO(stage-E) markers inops.rs).oh/chrome-ext-relay) + tinyflows v0.6.0 release.