From 66654f192b5d01d8673de71bc882a2585ee33193 Mon Sep 17 00:00:00 2001 From: graycyrus Date: Tue, 28 Jul 2026 23:46:32 +0530 Subject: [PATCH 1/4] =?UTF-8?q?feat(flows):=20browser=20companion=20core?= =?UTF-8?q?=20wiring=20=E2=80=94=20Chrome=20automation=20via=20slug:"brows?= =?UTF-8?q?er"=20(Part=201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- Cargo.lock | 11 +- src/core/runtime/builder.rs | 12 + src/core/runtime/services.rs | 37 ++ src/openhuman/browser_companion/mod.rs | 38 ++ src/openhuman/browser_companion/ops.rs | 497 ++++++++++++++++++ src/openhuman/browser_companion/store.rs | 88 ++++ src/openhuman/browser_companion/types.rs | 65 +++ .../config/schema/browser_companion.rs | 82 +++ src/openhuman/config/schema/mod.rs | 2 + src/openhuman/config/schema/types.rs | 11 + src/openhuman/flows/builder_tools.rs | 13 +- src/openhuman/flows/builder_tools_tests.rs | 24 + src/openhuman/flows/node_contracts.rs | 20 + src/openhuman/flows/ops.rs | 378 ++++++++++++- src/openhuman/flows/ops_tests.rs | 144 +++++ src/openhuman/flows/schemas.rs | 13 +- src/openhuman/mod.rs | 2 + src/openhuman/tinyflows/caps.rs | 48 ++ vendor/tinyflows | 2 +- 19 files changed, 1482 insertions(+), 5 deletions(-) create mode 100644 src/openhuman/browser_companion/mod.rs create mode 100644 src/openhuman/browser_companion/ops.rs create mode 100644 src/openhuman/browser_companion/store.rs create mode 100644 src/openhuman/browser_companion/types.rs create mode 100644 src/openhuman/config/schema/browser_companion.rs diff --git a/Cargo.lock b/Cargo.lock index 16dc21b540..f9fc1478ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -417,6 +417,7 @@ dependencies = [ "tower", "tower-layer", "tower-service", + "tracing", ] [[package]] @@ -435,6 +436,7 @@ dependencies = [ "sync_wrapper", "tower-layer", "tower-service", + "tracing", ] [[package]] @@ -7299,14 +7301,19 @@ name = "tinyflows" version = "0.5.1" dependencies = [ "async-trait", + "axum", "futures-timer", + "futures-util", + "getrandom 0.3.4", "jaq-core", "jaq-json", "jaq-std", + "reqwest", "serde", "serde_json", "thiserror 2.0.18", "tinyagents", + "tokio", "tracing", ] @@ -7317,7 +7324,7 @@ source = "git+https://github.com/tinyhumansai/sdk.git?rev=3ee4123ba3b7a76c5f167d dependencies = [ "base64 0.22.1", "percent-encoding", - "reqwest 0.12.28", + "reqwest", "serde", "serde_json", "thiserror 2.0.18", @@ -7642,6 +7649,7 @@ dependencies = [ "tokio", "tower-layer", "tower-service", + "tracing", ] [[package]] @@ -7680,6 +7688,7 @@ version = "0.1.44" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "63e71662fa4b2a2c3a26f570f037eb95bb1f85397f3cd8076caed2f026a6d100" dependencies = [ + "log", "pin-project-lite", "tracing-attributes", "tracing-core", diff --git a/src/core/runtime/builder.rs b/src/core/runtime/builder.rs index 6910dc2e8a..848165f88b 100644 --- a/src/core/runtime/builder.rs +++ b/src/core/runtime/builder.rs @@ -63,6 +63,10 @@ pub struct ServiceSet { pub memory_sync: bool, /// Orchestration relay-mailbox drain supervisor. pub orchestration: bool, + /// Browser Companion relay (TinyFlows Chrome extension WebSocket server). + /// Runtime-gated in turn by `config.browser_companion.enabled`. Only + /// meaningful when the `flows` Cargo feature is on — a no-op elsewhere. + pub companion_relay: bool, } impl ServiceSet { @@ -82,6 +86,7 @@ impl ServiceSet { integrations: true, memory_sync: true, orchestration: true, + companion_relay: true, } } @@ -102,6 +107,7 @@ impl ServiceSet { integrations: false, memory_sync: false, orchestration: false, + companion_relay: false, } } @@ -122,6 +128,7 @@ impl ServiceSet { integrations: false, memory_sync: false, orchestration: false, + companion_relay: false, } } @@ -152,6 +159,7 @@ impl ServiceSet { integrations: false, memory_sync: true, orchestration: false, + companion_relay: false, } } } @@ -722,6 +730,10 @@ impl CoreRuntime { if self.services.channels { services::spawn_channels_service(); } + #[cfg(feature = "flows")] + if self.services.companion_relay { + services::spawn_companion_relay_service(); + } } } diff --git a/src/core/runtime/services.rs b/src/core/runtime/services.rs index 2db9d80e79..72e2eb6799 100644 --- a/src/core/runtime/services.rs +++ b/src/core/runtime/services.rs @@ -217,6 +217,43 @@ pub fn spawn_channels_service() { log::debug!("[channels] channels feature disabled at compile time — not spawning listeners"); } +/// Browser Companion relay (TinyFlows Chrome extension WebSocket server). +/// +/// Entirely gated behind the `flows` Cargo feature (the `browser_companion` +/// domain is itself `#[cfg(feature = "flows")]`, so this function only +/// exists to be called when the feature is on — the call site in +/// [`crate::core::runtime::builder::CoreBuilder::start_selected_services`] +/// carries the matching `#[cfg]`). Runtime-gated in turn by +/// `config.browser_companion.enabled` inside `start_companion_server` +/// itself, mirroring `spawn_cron_service`'s config-gate pattern. +#[cfg(feature = "flows")] +pub fn spawn_companion_relay_service() { + tokio::spawn(async { + log::debug!("[browser_companion] spawn_companion_relay_service: loading config"); + match crate::openhuman::config::Config::load_or_init().await { + Ok(config) => { + if !config.browser_companion.enabled { + log::debug!( + "[browser_companion] spawn_companion_relay_service: disabled via config; skipping" + ); + return; + } + log::info!("[browser_companion] spawn_companion_relay_service: starting relay"); + if let Err(error) = + crate::openhuman::browser_companion::start_companion_server(&config).await + { + log::error!( + "[browser_companion] spawn_companion_relay_service: start_companion_server failed: {error}" + ); + } + } + Err(err) => { + log::warn!("[core] config load failed, skipping browser_companion relay: {err}"); + } + } + }); +} + /// Which bootstrap jobs a given [`ServiceSet`] enables — the single source of /// truth for the flag→job mapping. /// diff --git a/src/openhuman/browser_companion/mod.rs b/src/openhuman/browser_companion/mod.rs new file mode 100644 index 0000000000..525eeb8053 --- /dev/null +++ b/src/openhuman/browser_companion/mod.rs @@ -0,0 +1,38 @@ +//! Browser Companion: owns the lifecycle + pairing of the TinyFlows +//! `CompanionServer` — a loopback WebSocket relay the Chrome extension +//! connects to so native workflow runs can drive/observe the user's browser. +//! +//! **Increment 1 scope** (this module): server lifecycle (start/stop), +//! pairing (pair/unpair/rotate secret), and status reporting. +//! +//! **Increment 2** added [`bind_run`]/[`unbind_run`] (Stage C3 — flows +//! wiring): `src/openhuman/flows/ops.rs` calls these around a real +//! `flows_run`/`flows_run_detached` execution whose graph has a +//! `tool_call { slug: "browser" }` node, binding the run's `thread_id` to the +//! caller-selected shared tab so its `slug:"browser"` calls (routed through +//! [`browser_relay`] wrapped in `tinyflows::browser::RoutingToolInvoker`) are +//! authorized. No RPC controllers (`browser_companion.*`) yet — that still +//! lands in a later stage. +//! +//! Entirely gated behind the existing `flows` Cargo feature — this domain +//! rides the same `tinyflows` dependency as `openhuman::flows` / +//! `openhuman::tinyflows`, adds no new feature, and is compiled out +//! wholesale (leaf-gate style, see `AGENTS.md`'s `flows` gate section) when +//! `flows` is off. The one exception is [`crate::openhuman::config::schema::BrowserCompanionConfig`] +//! (`src/openhuman/config/schema/browser_companion.rs`), which stays +//! ungated as inert config data — matching the `MeetConfig` precedent. +//! +//! Spawned at boot by `core::runtime::services::spawn_companion_relay_service`, +//! selected by `ServiceSet::companion_relay`. + +mod ops; +mod store; +mod types; + +pub use ops::{ + bind_run, browser_relay, companion_status, is_extension_connected, pair, rotate_secret, + start_companion_server, stop_companion_server, unbind_run, unpair, +}; +pub use types::{BrowserCompanionStatus, PairingInfo, SharedTabView}; + +pub(crate) const LOG_PREFIX: &str = "[browser_companion]"; diff --git a/src/openhuman/browser_companion/ops.rs b/src/openhuman/browser_companion/ops.rs new file mode 100644 index 0000000000..aeb2f1a989 --- /dev/null +++ b/src/openhuman/browser_companion/ops.rs @@ -0,0 +1,497 @@ +//! Business logic for the Browser Companion domain: owns the lifecycle of the +//! TinyFlows `CompanionServer` (loopback WebSocket relay to the Chrome +//! extension) and its pairing secret. +//! +//! Increment 1 scope: lifecycle + pairing only. No RPC controllers and no +//! flows wiring yet — those land in later stages. See +//! `src/openhuman/browser_companion/mod.rs` for the module overview. + +use std::sync::{Arc, Mutex, OnceLock}; + +use tinyflows::browser::BrowserRelay; +use tinyflows::companion::{CompanionServer, CompanionServerConfig, RelayPolicy}; + +use crate::openhuman::browser_companion::store::resolve_secret_store; +use crate::openhuman::browser_companion::types::{BrowserCompanionStatus, PairingInfo}; +use crate::openhuman::browser_companion::LOG_PREFIX; +use crate::openhuman::config::Config; +use crate::openhuman::tinyflows::build_capabilities; + +/// Namespace passed to [`build_capabilities`] for every non-browser effect +/// (state store, http, code, agent) the companion server's native workflow +/// runs might use in a later increment. +const CAPS_STATE_NAMESPACE: &str = "browser-companion"; + +/// In-memory lifecycle state for the companion relay: at most one instance +/// runs per process. +struct CompanionRuntime { + server: Option, + task: Option>, +} + +impl CompanionRuntime { + const fn empty() -> Self { + Self { + server: None, + task: None, + } + } +} + +static RUNTIME: OnceLock> = OnceLock::new(); + +fn runtime() -> &'static Mutex { + RUNTIME.get_or_init(|| Mutex::new(CompanionRuntime::empty())) +} + +fn workflows_dir(config: &Config) -> std::path::PathBuf { + config + .workspace_dir + .join("browser_companion") + .join("workflows") +} + +fn relay_url(port: u16) -> String { + format!("ws://127.0.0.1:{port}/v1/extension") +} + +/// Starts the companion relay if `config.browser_companion.enabled` and no +/// instance is already running. No-op (with a log line) otherwise. +pub async fn start_companion_server(config: &Config) -> anyhow::Result<()> { + log::debug!("{LOG_PREFIX} start_companion_server: entry"); + + if !config.browser_companion.enabled { + log::debug!("{LOG_PREFIX} start_companion_server: disabled via config; skipping"); + return Ok(()); + } + + start_with_extension_id(config, config.browser_companion.extension_id.clone()).await +} + +/// Core start path shared by [`start_companion_server`] (uses the persisted +/// `extension_id`) and [`pair`] (uses the freshly supplied one). Does **not** +/// re-check `config.browser_companion.enabled` — callers that want the +/// enabled-gate must go through [`start_companion_server`]. +async fn start_with_extension_id(config: &Config, extension_id: String) -> anyhow::Result<()> { + { + let guard = runtime() + .lock() + .expect("browser_companion runtime poisoned"); + if guard.server.is_some() { + log::debug!("{LOG_PREFIX} start_with_extension_id: already running; no-op"); + return Ok(()); + } + } + + let port = config.browser_companion.port; + log::info!( + "{LOG_PREFIX} start_with_extension_id: starting relay port={port} extension_id_set={}", + !extension_id.is_empty() + ); + + let policy = RelayPolicy::loopback(port); + let workflows_dir = workflows_dir(config); + std::fs::create_dir_all(&workflows_dir).map_err(|error| { + log::warn!( + "{LOG_PREFIX} start_companion_server: failed to create workflows dir {}: {error}", + workflows_dir.display() + ); + anyhow::anyhow!("failed to create browser_companion workflows dir: {error}") + })?; + + let secret_store = resolve_secret_store(config)?; + let pairing_secret = secret_store.load_or_create().map_err(|error| { + log::warn!("{LOG_PREFIX} start_companion_server: secret load_or_create failed: {error}"); + anyhow::anyhow!("failed to load or create pairing secret: {error}") + })?; + + let capabilities = build_capabilities(Arc::new(config.clone()), CAPS_STATE_NAMESPACE); + + let server_config = CompanionServerConfig { + policy, + extension_id, + pairing_secret, + workflows_dir, + capabilities, + }; + + let server = CompanionServer::new(server_config).map_err(|error| { + log::warn!("{LOG_PREFIX} start_companion_server: CompanionServer::new failed: {error}"); + anyhow::anyhow!("failed to construct companion server: {error}") + })?; + + let bind_addr = server.bind_addr(); + let serving = server.clone(); + let task = tokio::spawn(async move { + log::info!("{LOG_PREFIX} companion relay serving on {bind_addr}"); + if let Err(error) = serving.serve().await { + log::error!("{LOG_PREFIX} companion relay listener exited with error: {error}"); + } else { + log::info!("{LOG_PREFIX} companion relay stopped cleanly"); + } + }); + + let mut guard = runtime() + .lock() + .expect("browser_companion runtime poisoned"); + guard.server = Some(server); + guard.task = Some(task); + log::info!("{LOG_PREFIX} start_with_extension_id: relay started bind_addr={bind_addr}"); + Ok(()) +} + +/// Stops the companion relay if running. No-op (with a log line) otherwise. +pub async fn stop_companion_server() { + log::debug!("{LOG_PREFIX} stop_companion_server: entry"); + let (server, task) = { + let mut guard = runtime() + .lock() + .expect("browser_companion runtime poisoned"); + (guard.server.take(), guard.task.take()) + }; + + if server.is_none() { + log::debug!("{LOG_PREFIX} stop_companion_server: not running; no-op"); + return; + } + + if let Some(task) = task { + task.abort(); + log::info!("{LOG_PREFIX} stop_companion_server: relay task aborted"); + } +} + +/// Returns a handle usable to route `slug:"browser"` tool calls to the +/// paired extension, for later flows wiring (Stage C3). `None` when the +/// relay is not running. +pub fn browser_relay() -> Option> { + let guard = runtime() + .lock() + .expect("browser_companion runtime poisoned"); + guard.server.as_ref().map(CompanionServer::browser_relay) +} + +/// Binds a workflow run to an explicitly-shared browser tab so that run's +/// `slug:"browser"` tool calls (routed through the handle from +/// [`browser_relay`]) are authorized against that tab. Mirrors +/// `tinyflows::companion::CompanionServer::bind_run`, keeping the server +/// handle itself encapsulated in this domain (Stage C3 — flows wiring). +/// +/// Returns an error when the relay is not currently running, or when the +/// underlying `CompanionServer::bind_run` call rejects the binding (e.g. the +/// tab isn't one the extension has explicitly shared — `tab_not_shared`). +pub fn bind_run(run_id: &str, tab_id: u64) -> anyhow::Result<()> { + log::debug!("{LOG_PREFIX} bind_run: entry run_id={run_id} tab_id={tab_id}"); + let guard = runtime() + .lock() + .expect("browser_companion runtime poisoned"); + let Some(server) = guard.server.as_ref() else { + log::warn!("{LOG_PREFIX} bind_run: relay not running; cannot bind run_id={run_id}"); + return Err(anyhow::anyhow!( + "browser companion relay is not running; cannot bind run '{run_id}' to tab {tab_id}" + )); + }; + server + .bind_run(run_id.to_string(), tab_id) + .map_err(|error| { + log::warn!( + "{LOG_PREFIX} bind_run: CompanionServer::bind_run failed run_id={run_id} \ + tab_id={tab_id}: {error}" + ); + anyhow::anyhow!("failed to bind run '{run_id}' to browser tab {tab_id}: {error}") + })?; + log::info!("{LOG_PREFIX} bind_run: bound run_id={run_id} tab_id={tab_id}"); + Ok(()) +} + +/// Releases a run→tab binding after an external run settles. No-op (with a +/// debug log) if the relay isn't running or nothing was bound for `run_id` — +/// idempotent, mirroring `tinyflows::companion::CompanionServer::unbind_run`. +pub fn unbind_run(run_id: &str) { + log::debug!("{LOG_PREFIX} unbind_run: entry run_id={run_id}"); + let guard = runtime() + .lock() + .expect("browser_companion runtime poisoned"); + let Some(server) = guard.server.as_ref() else { + log::debug!("{LOG_PREFIX} unbind_run: relay not running; no-op"); + return; + }; + server.unbind_run(run_id); + log::debug!("{LOG_PREFIX} unbind_run: unbound run_id={run_id} (no-op if it wasn't bound)"); +} + +/// Whether a paired extension currently holds an authenticated relay +/// session. Always `false` when the relay is not running. +pub fn is_extension_connected() -> bool { + let guard = runtime() + .lock() + .expect("browser_companion runtime poisoned"); + guard + .server + .as_ref() + .map(CompanionServer::is_extension_connected) + .unwrap_or(false) +} + +/// Current lifecycle + pairing snapshot. +pub fn companion_status(config: &Config) -> BrowserCompanionStatus { + let guard = runtime() + .lock() + .expect("browser_companion runtime poisoned"); + let running = guard.server.is_some(); + let extension_connected = guard + .server + .as_ref() + .map(CompanionServer::is_extension_connected) + .unwrap_or(false); + let shared_tabs = guard + .server + .as_ref() + .map(|server| server.shared_tabs().into_iter().map(Into::into).collect()) + .unwrap_or_default(); + + let paired_extension_id = if config.browser_companion.extension_id.is_empty() { + None + } else { + Some(config.browser_companion.extension_id.clone()) + }; + let relay_url = running.then(|| relay_url(config.browser_companion.port)); + + log::debug!( + "{LOG_PREFIX} companion_status: running={running} extension_connected={extension_connected} shared_tab_count={}", + { + let count: &Vec<_> = &shared_tabs; + count.len() + } + ); + + BrowserCompanionStatus { + running, + extension_connected, + paired_extension_id, + relay_url, + shared_tabs, + } +} + +/// Pairs a new extension id: restarts the relay bound to `extension_id` +/// (rather than whatever is currently persisted in `config`) and returns the +/// relay URL + current pairing secret. +/// +/// `config.browser_companion.extension_id` is **not** mutated here — the +/// caller of a future RPC handler is responsible for persisting the new +/// `extension_id` via `config.update_*` (see `TODO(stage-E)`). This +/// increment only needs the relay itself to come up bound to the new id, so +/// `extension_id` is threaded straight into [`start_with_extension_id`] +/// instead of round-tripping through a mutated config copy. +pub async fn pair(config: &Config, extension_id: String) -> anyhow::Result { + log::info!( + "{LOG_PREFIX} pair: entry extension_id_len={}", + extension_id.len() + ); + // TODO(stage-E): persist `browser_companion.extension_id` via a + // `config.update_*` RPC once the RPC surface for this domain lands. + + stop_companion_server().await; + start_with_extension_id(config, extension_id).await?; + + let secret_store = resolve_secret_store(config)?; + let pairing_secret = secret_store.load_or_create().map_err(|error| { + log::warn!("{LOG_PREFIX} pair: secret load_or_create failed: {error}"); + anyhow::anyhow!("failed to load pairing secret after pairing: {error}") + })?; + + log::info!("{LOG_PREFIX} pair: relay restarted with new extension_id"); + Ok(PairingInfo { + relay_url: relay_url(config.browser_companion.port), + pairing_secret: pairing_secret.expose().to_string(), + }) +} + +/// Clears the pairing (rotates the secret, invalidating the old one) and +/// stops the relay. +/// +/// Same in-memory-only caveat as [`pair`] applies to persisting a cleared +/// `extension_id` — see `TODO(stage-E)`. +pub async fn unpair(config: &Config) -> anyhow::Result<()> { + log::info!("{LOG_PREFIX} unpair: entry"); + // TODO(stage-E): persist the cleared `extension_id` via `config.update_*`. + + let secret_store = resolve_secret_store(config)?; + secret_store.rotate().map_err(|error| { + log::warn!("{LOG_PREFIX} unpair: secret rotate failed: {error}"); + anyhow::anyhow!("failed to rotate pairing secret during unpair: {error}") + })?; + + stop_companion_server().await; + log::info!("{LOG_PREFIX} unpair: relay stopped and secret rotated"); + Ok(()) +} + +/// Rotates the pairing secret and restarts the relay (if it was running) so +/// the new secret takes effect, returning it. +pub async fn rotate_secret(config: &Config) -> anyhow::Result { + log::info!("{LOG_PREFIX} rotate_secret: entry"); + let secret_store = resolve_secret_store(config)?; + let pairing_secret = secret_store.rotate().map_err(|error| { + log::warn!("{LOG_PREFIX} rotate_secret: secret rotate failed: {error}"); + anyhow::anyhow!("failed to rotate pairing secret: {error}") + })?; + + let was_running = { + let guard = runtime() + .lock() + .expect("browser_companion runtime poisoned"); + guard.server.is_some() + }; + if was_running { + stop_companion_server().await; + start_companion_server(config).await?; + log::info!("{LOG_PREFIX} rotate_secret: relay restarted with rotated secret"); + } + + Ok(PairingInfo { + relay_url: relay_url(config.browser_companion.port), + pairing_secret: pairing_secret.expose().to_string(), + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn test_config(workspace_dir: std::path::PathBuf) -> Config { + Config { + workspace_dir, + ..Config::default() + } + } + + #[test] + fn status_reports_no_paired_extension_for_default_config() { + // Deliberately does not assert `status.running`: the lifecycle test + // below shares the process-wide runtime static and may be running + // concurrently. `paired_extension_id` is derived purely from + // `config`, so it's deterministic regardless of runtime state. + let tmp = tempfile::tempdir().expect("tempdir"); + let config = test_config(tmp.path().to_path_buf()); + let status = companion_status(&config); + assert_eq!(status.paired_extension_id, None); + assert!(!config.browser_companion.enabled); + } + + #[test] + fn companion_status_reports_paired_extension_id_from_config() { + let tmp = tempfile::tempdir().expect("tempdir"); + let mut config = test_config(tmp.path().to_path_buf()); + config.browser_companion.extension_id = "abcdefghijklmnopabcdefghijklmnop".to_string(); + let status = companion_status(&config); + assert_eq!( + status.paired_extension_id, + Some("abcdefghijklmnopabcdefghijklmnop".to_string()) + ); + } + + #[test] + fn relay_url_formats_loopback_websocket_url() { + assert_eq!(relay_url(32189), "ws://127.0.0.1:32189/v1/extension"); + } + + #[tokio::test] + async fn start_is_noop_when_disabled_in_config() { + let tmp = tempfile::tempdir().expect("tempdir"); + let config = test_config(tmp.path().to_path_buf()); + assert!(!config.browser_companion.enabled); + // Returns before touching the shared runtime static at all, so this + // is safe to run alongside any other test in this file. + let result = start_companion_server(&config).await; + assert!(result.is_ok()); + } + + #[tokio::test] + async fn browser_relay_and_is_extension_connected_never_panic() { + // Read-only accessors must not panic regardless of whether some + // other test in this file has a relay running concurrently — they + // are not asserted against a specific state here. + let _ = browser_relay(); + let _ = is_extension_connected(); + } + + #[test] + fn bind_run_errs_when_relay_not_running() { + // Deliberately does not touch the shared runtime static (no start/stop + // call), so this is safe to run concurrently with any other test in + // this file — `bind_run` only reads whether a server is present. + // + // NOTE: cannot assert `.is_err()` unconditionally here because this + // process-wide static may already have a server running from another + // concurrently-executing test in this file (e.g. + // `start_then_status_running_then_stop`). Instead assert the + // documented CONTRACT: when no server is running, `bind_run` errs + // with a message naming the run id. + if browser_relay().is_none() { + let err = bind_run("test-run-not-running", 7) + .expect_err("bind_run must error when the relay is not running"); + assert!(err.to_string().contains("test-run-not-running"), "{err}"); + } + } + + #[test] + fn unbind_run_is_a_noop_when_relay_not_running_or_run_unknown() { + // Never panics regardless of runtime state; always a safe no-op for + // an unknown/unbound run id. + unbind_run("test-run-never-bound"); + } + + /// Full lifecycle: start (bound to an OS-assigned ephemeral port via + /// port 0, so this can never collide with a real or another test's + /// port) → status reports running → stop → status reports not running. + /// + /// This is the *only* test in this file that calls + /// `start_companion_server`/`stop_companion_server` with `enabled: true`, + /// so it cannot race against another test over the shared process-wide + /// runtime static. + #[tokio::test] + async fn start_then_status_running_then_stop() { + let tmp = tempfile::tempdir().expect("tempdir"); + let mut config = test_config(tmp.path().to_path_buf()); + config.browser_companion.enabled = true; + // Port 0: let the OS assign a free ephemeral port. `CompanionServer::new` + // only validates the policy and builds in-memory state — the actual + // `TcpListener::bind` happens inside the spawned `serve()` task, so + // this test's synchronous assertions don't depend on the bind + // actually completing. + config.browser_companion.port = 0; + // Must be exactly 32 chars in a-p to pass `Authenticator::new`'s + // Chrome-extension-id format check. + config.browser_companion.extension_id = "abcdefghijklmnopabcdefghijklmnop".to_string(); + + start_companion_server(&config) + .await + .expect("start_companion_server should succeed with a valid config"); + + let status = companion_status(&config); + assert!(status.running, "relay should report running after start"); + assert!( + !status.extension_connected, + "no extension has connected yet" + ); + assert!(status.shared_tabs.is_empty()); + + // bind_run/unbind_run while the relay is running (still the only test + // in this file exercising the shared runtime static with a real + // server up, so no race against another test's start/stop). No tab + // has been shared by any extension in this test, so binding must fail + // closed with a `tab_not_shared`-shaped error rather than silently + // succeeding. + let err = bind_run("test-run-1", 99).expect_err("no tab is shared in this test"); + assert!(err.to_string().contains("test-run-1"), "{err}"); + // Idempotent no-op even though nothing was ever actually bound. + unbind_run("test-run-1"); + + stop_companion_server().await; + + let status = companion_status(&config); + assert!(!status.running, "relay should report stopped after stop"); + } +} diff --git a/src/openhuman/browser_companion/store.rs b/src/openhuman/browser_companion/store.rs new file mode 100644 index 0000000000..7049aecc93 --- /dev/null +++ b/src/openhuman/browser_companion/store.rs @@ -0,0 +1,88 @@ +//! Persistence for the Browser Companion domain. +//! +//! Thin by design for increment 1: the only durable state owned outright by +//! this domain is the pairing secret file. Everything else +//! (`enabled` / `port` / `extension_id`) rides the existing `Config` +//! persistence path (`[browser_companion]` in `config.toml`) — this domain +//! reads the live `Config` passed in by its callers rather than building a +//! bespoke store for it. + +use std::path::PathBuf; + +use tinyflows::companion::SecretStore; + +use crate::openhuman::browser_companion::LOG_PREFIX; +use crate::openhuman::config::Config; + +/// Resolves the pairing-secret file path: +/// `{workspace_dir}/browser_companion/relay.secret`, creating the parent +/// directory if needed. +/// +/// The secret itself is never logged; only paths and outcomes are. +pub(crate) fn resolve_secret_store(config: &Config) -> std::io::Result { + let dir = secret_dir(config); + log::debug!("{LOG_PREFIX} resolving secret store dir={}", dir.display()); + std::fs::create_dir_all(&dir)?; + let path = dir.join("relay.secret"); + Ok(SecretStore::new(path)) +} + +/// The directory the pairing secret lives in, without touching the +/// filesystem — split out so tests can assert on the path shape without a +/// real workspace dir. +pub(crate) fn secret_dir(config: &Config) -> PathBuf { + config.workspace_dir.join("browser_companion") +} + +#[cfg(test)] +mod tests { + use super::*; + + fn test_config(workspace_dir: PathBuf) -> Config { + Config { + workspace_dir, + ..Config::default() + } + } + + #[test] + fn secret_dir_is_scoped_under_workspace() { + let config = test_config(PathBuf::from("/tmp/does-not-exist-oh-test-workspace")); + let dir = secret_dir(&config); + assert_eq!( + dir, + PathBuf::from("/tmp/does-not-exist-oh-test-workspace/browser_companion") + ); + } + + #[test] + fn resolve_secret_store_creates_dir_and_scoped_path() { + let tmp = tempfile::tempdir().expect("tempdir"); + let config = test_config(tmp.path().to_path_buf()); + + let store = resolve_secret_store(&config).expect("resolve secret store"); + assert_eq!(store.path(), secret_dir(&config).join("relay.secret")); + assert!(secret_dir(&config).is_dir()); + } + + #[cfg(unix)] + #[test] + fn load_or_create_persists_secret_with_owner_only_perms() { + use std::os::unix::fs::PermissionsExt; + + let tmp = tempfile::tempdir().expect("tempdir"); + let config = test_config(tmp.path().to_path_buf()); + let store = resolve_secret_store(&config).expect("resolve secret store"); + + let secret = store.load_or_create().expect("load_or_create"); + assert!(!secret.expose().is_empty()); + + let metadata = std::fs::metadata(store.path()).expect("secret file metadata"); + let mode = metadata.permissions().mode() & 0o777; + assert_eq!(mode, 0o600, "pairing secret file must be owner-only (0600)"); + + // Loading again returns the same secret rather than regenerating it. + let reloaded = store.load_or_create().expect("reload secret"); + assert_eq!(secret.expose(), reloaded.expose()); + } +} diff --git a/src/openhuman/browser_companion/types.rs b/src/openhuman/browser_companion/types.rs new file mode 100644 index 0000000000..9b074c1002 --- /dev/null +++ b/src/openhuman/browser_companion/types.rs @@ -0,0 +1,65 @@ +//! Serde-facing types for the Browser Companion domain. +//! +//! These types are the public shape returned to (eventual, later-increment) +//! RPC callers. They deliberately do not leak the vendored +//! `tinyflows::companion` types across the domain boundary — see +//! [`SharedTabView`] vs. `tinyflows::companion::SharedTab`. + +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +/// One browser tab the paired extension has explicitly shared with the +/// companion relay. Mapped from `tinyflows::companion::SharedTab`, dropping +/// the `generation` counter (an internal relay-freshness detail, not +/// meaningful to callers of this domain). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +pub struct SharedTabView { + pub id: u64, + pub window_id: u64, + pub url: String, + pub title: String, +} + +// No `#[cfg(feature = "flows")]` needed here: this whole module is only +// compiled when `browser_companion` itself is compiled, which is already +// gated behind `feature = "flows"` at the `pub mod browser_companion;` +// declaration in `src/openhuman/mod.rs`. +impl From for SharedTabView { + fn from(tab: tinyflows::companion::SharedTab) -> Self { + Self { + id: tab.id, + window_id: tab.window_id, + url: tab.url, + title: tab.title, + } + } +} + +/// Current lifecycle + pairing snapshot of the Browser Companion relay. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +pub struct BrowserCompanionStatus { + /// Whether the loopback `CompanionServer` is currently running. + pub running: bool, + /// Whether a paired extension currently holds an authenticated relay + /// session. Always `false` when `running` is `false`. + pub extension_connected: bool, + /// The extension id the relay is configured to accept, if any. + pub paired_extension_id: Option, + /// The `ws://127.0.0.1:/v1/extension` URL the extension connects + /// to, present only while the relay is running. + pub relay_url: Option, + /// Tabs the paired extension has explicitly shared with the companion. + /// Empty when not running or nothing is shared. + pub shared_tabs: Vec, +} + +/// Result of a pair / rotate-secret operation: what the extension needs to +/// complete pairing. Callers must treat `pairing_secret` as sensitive — +/// it authenticates the WebSocket upgrade. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +pub struct PairingInfo { + /// The `ws://127.0.0.1:/v1/extension` URL to connect to. + pub relay_url: String, + /// The freshly (re)generated pairing secret, exposed exactly once here. + pub pairing_secret: String, +} diff --git a/src/openhuman/config/schema/browser_companion.rs b/src/openhuman/config/schema/browser_companion.rs new file mode 100644 index 0000000000..54b48fb74c --- /dev/null +++ b/src/openhuman/config/schema/browser_companion.rs @@ -0,0 +1,82 @@ +//! Browser Companion (TinyFlows Chrome extension relay) integration settings. +//! +//! This is inert config data only — always compiled, regardless of the +//! `flows` Cargo feature — matching the type-carve-out convention documented +//! in `AGENTS.md` (config sections stay ungated; only the domain's +//! *behaviour* in `openhuman::browser_companion` is gated). See +//! `MeetConfig` (`meet.rs`) for the sibling precedent: the `meet` domain is +//! gated behind the `meet` feature, but `MeetConfig` itself is not. +//! +//! See tinyhumansai/openhuman browser companion integration, increment 1. + +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[serde(default)] +pub struct BrowserCompanionConfig { + /// Master switch: when `true`, the desktop core spawns the loopback + /// `CompanionServer` (TinyFlows Chrome extension relay) at boot. + #[serde(default)] + pub enabled: bool, + + /// Loopback TCP port the companion relay binds + /// (`RelayPolicy::loopback(port)`). + #[serde(default = "default_port")] + pub port: u16, + + /// Exact Chrome extension id allowed by the WebSocket origin check. + /// Empty until the user pairs an extension. + #[serde(default)] + pub extension_id: String, +} + +fn default_port() -> u16 { + 32189 +} + +impl Default for BrowserCompanionConfig { + fn default() -> Self { + Self { + enabled: false, + port: default_port(), + extension_id: String::new(), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn default_is_disabled_with_expected_port() { + let cfg = BrowserCompanionConfig::default(); + assert!(!cfg.enabled); + assert_eq!(cfg.port, 32189); + assert!(cfg.extension_id.is_empty()); + } + + #[test] + fn deserialize_missing_fields_uses_defaults() { + let cfg: BrowserCompanionConfig = serde_json::from_value(json!({})).unwrap(); + assert!(!cfg.enabled); + assert_eq!(cfg.port, 32189); + assert!(cfg.extension_id.is_empty()); + } + + #[test] + fn round_trip_preserves_all_fields() { + let original = BrowserCompanionConfig { + enabled: true, + port: 40000, + extension_id: "abcdefghijklmnopabcdefghijklmnop".to_string(), + }; + let s = serde_json::to_string(&original).unwrap(); + let back: BrowserCompanionConfig = serde_json::from_str(&s).unwrap(); + assert!(back.enabled); + assert_eq!(back.port, 40000); + assert_eq!(back.extension_id, original.extension_id); + } +} diff --git a/src/openhuman/config/schema/mod.rs b/src/openhuman/config/schema/mod.rs index 808886d7b3..bbe959783b 100644 --- a/src/openhuman/config/schema/mod.rs +++ b/src/openhuman/config/schema/mod.rs @@ -13,6 +13,7 @@ pub mod subconscious; pub use subconscious::{MedullaLocalConfig, SubconsciousConfig, SubconsciousEngine}; mod agent; mod autonomy; +mod browser_companion; mod capability_providers; mod channels; mod context; @@ -58,6 +59,7 @@ pub use agent::{ OrchestratorModelConfig, RequiredOutputContract, TeamModelConfig, }; pub use autonomy::AutonomyConfig; +pub use browser_companion::BrowserCompanionConfig; pub use capability_providers::{CapabilityProviderConfig, CapabilityProviderTrustState}; pub use channels::{ AuditConfig, ChannelsConfig, DingTalkConfig, DiscordConfig, EmailConfig, IMessageConfig, diff --git a/src/openhuman/config/schema/types.rs b/src/openhuman/config/schema/types.rs index 2be9538c34..21f284310f 100644 --- a/src/openhuman/config/schema/types.rs +++ b/src/openhuman/config/schema/types.rs @@ -463,6 +463,16 @@ pub struct Config { #[serde(default)] pub meet: MeetConfig, + /// Browser Companion (TinyFlows Chrome extension relay) integration + /// settings — loopback port, pairing extension id, and the master + /// enable switch. See + /// [`crate::openhuman::config::schema::BrowserCompanionConfig`]. Always + /// present in the schema regardless of the `flows` Cargo feature (inert + /// config data); the domain behaviour that reads it lives in + /// `openhuman::browser_companion`, gated behind `feature = "flows"`. + #[serde(default)] + pub browser_companion: BrowserCompanionConfig, + /// Whether the user has completed the **React UI** onboarding flow. /// /// Set by `OnboardingOverlay.tsx::handleDone` and the multi-step @@ -817,6 +827,7 @@ impl Default for Config { update: UpdateConfig::default(), dictation: DictationConfig::default(), meet: MeetConfig::default(), + browser_companion: BrowserCompanionConfig::default(), onboarding_completed: false, chat_onboarding_completed: false, model_registry: Vec::new(), diff --git a/src/openhuman/flows/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index 2f756f5a0e..d918394fb3 100644 --- a/src/openhuman/flows/builder_tools.rs +++ b/src/openhuman/flows/builder_tools.rs @@ -1286,13 +1286,24 @@ impl Tool for ListConnectableToolkitsTool { use crate::openhuman::memory_sync::composio::providers::agent_ready_toolkits; tracing::debug!(target: "flows", "[flows] list_connectable_toolkits: listing toolkits + connected state (read-only)"); let connected = ops::connected_toolkits(&self.config).await; - let toolkits: Vec = agent_ready_toolkits() + let mut toolkits: Vec = agent_ready_toolkits() .into_iter() .map(|tk| { let tk_lc = tk.to_ascii_lowercase(); json!({ "toolkit": tk_lc, "connected": connected.contains(&tk_lc) }) }) .collect(); + // Browser Companion (Stage C3): a built-in Chrome-automation "toolkit" + // — not Composio-backed, so it's appended here rather than coming + // from `agent_ready_toolkits()`. `type: "builtin"` distinguishes it + // from the Composio entries above so the workflow_builder agent + // doesn't try to `composio_connect` it. + #[cfg(feature = "flows")] + toolkits.push(json!({ + "toolkit": "browser", + "connected": crate::openhuman::browser_companion::is_extension_connected(), + "type": "builtin", + })); Ok(ToolResult::success(serde_json::to_string_pretty( &json!({ "toolkits": toolkits }), )?)) diff --git a/src/openhuman/flows/builder_tools_tests.rs b/src/openhuman/flows/builder_tools_tests.rs index af765479a3..0a1c31ea02 100644 --- a/src/openhuman/flows/builder_tools_tests.rs +++ b/src/openhuman/flows/builder_tools_tests.rs @@ -205,6 +205,30 @@ fn list_flow_connections_json_surfaces_platform_user_id() { assert!(json["platform_user_id"].is_null()); } +#[tokio::test] +async fn list_connectable_toolkits_includes_builtin_browser_entry() { + // Stage C3: the workflow_builder agent must be able to discover `browser` + // as an available (built-in, not Composio) toolkit alongside the + // Composio-backed ones from `agent_ready_toolkits()`. + let tmp = TempDir::new().unwrap(); + let tool = ListConnectableToolkitsTool::new(test_config(&tmp)); + assert_eq!(tool.permission_level(), PermissionLevel::None); + assert!(!tool.external_effect()); + + let result = tool.execute(json!({})).await.unwrap(); + assert!(!result.is_error, "{}", result.output()); + let parsed: Value = serde_json::from_str(&result.output()).unwrap(); + let toolkits = parsed["toolkits"].as_array().unwrap(); + let browser = toolkits + .iter() + .find(|t| t["toolkit"] == "browser") + .expect("browser entry must be present"); + assert_eq!(browser["type"], "builtin"); + // Fresh test config: the companion relay is disabled by default, so no + // extension can be connected. + assert_eq!(browser["connected"], false); +} + // ── search_tool_catalog / get_tool_contract ───────────────────────────────── // The live-catalog cache is process-global (`LIVE_CATALOG_CACHE`) — every // test below seeds the exact toolkit(s)/contract(s) it needs via diff --git a/src/openhuman/flows/node_contracts.rs b/src/openhuman/flows/node_contracts.rs index e2e14ebd88..d35ae2bdc4 100644 --- a/src/openhuman/flows/node_contracts.rs +++ b/src/openhuman/flows/node_contracts.rs @@ -60,6 +60,16 @@ fn apply_host_overlay(contract: NodeKindContract) -> NodeKindContract { bind downstream as =nodes..item.json.data., NOT .item.json.. To \ split_out over its result list, use get_tool_contract's primary_array_path \ prefixed with `json.` (e.g. \"json.data.messages\").", + ) + .with_note( + "config.slug = \"browser\" is a THIRD, built-in slug family — Chrome browser \ + automation via the Browser Companion — distinct from both a Composio action and \ + an oh: native tool. It needs NO config.connection_ref (it authorizes against a \ + per-run shared browser tab instead). config.args must be { \"action\": , ...action-specific fields }. \ + Requires the user to have paired the Chrome extension and shared a tab — a run \ + fails cleanly before executing if the companion isn't ready.", ), "http_request" => contract.with_note( "config.connection_ref is an http_cred: credential for authentication.", @@ -156,6 +166,16 @@ mod tests { assert!(notes.contains("get_tool_contract"), "{notes}"); } + #[test] + fn tool_call_overlay_documents_the_builtin_browser_slug() { + let c = node_kind_contract("tool_call").unwrap(); + let notes = c.notes.join("\n"); + assert!(notes.contains("browser"), "{notes}"); + assert!(notes.contains("Browser Companion"), "{notes}"); + assert!(notes.contains("connection_ref"), "{notes}"); + assert!(notes.contains("snapshot"), "{notes}"); + } + #[test] fn agent_overlay_adds_input_context_guidance() { let c = node_kind_contract("agent").unwrap(); diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index b427980492..6a867a92f9 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -788,6 +788,18 @@ pub(crate) async fn build_builder_proposal( payload["inference_message"] = json!(message); } } + // Browser Companion advisory (Stage C3, mirrors B45's `inference_status` / + // `inference_message` exactly): only present when the graph has at least + // one `tool_call { slug: "browser" }` node; a graph with no browser step + // omits both fields entirely. Authoring always succeeds regardless of + // `browser_status` — see `evaluate_browser_readiness`'s doc; the hard gate + // is run time only (`validate_browser_readiness` in `run_flow_body`). + if let Some(evaluation) = evaluate_browser_readiness(config, graph) { + payload["browser_status"] = json!(evaluation.status); + if let Some(message) = evaluation.message { + payload["browser_message"] = json!(message); + } + } if let Some(instruction) = instruction { payload["instruction"] = json!(instruction); } @@ -2299,6 +2311,160 @@ pub(crate) async fn validate_inference_readiness( } } +// ───────────────────────────────────────────────────────────────────────────── +// Browser Companion readiness gate (Stage C3 — mirrors the B45 +// inference-readiness pattern immediately above) +// ───────────────────────────────────────────────────────────────────────────── +// +// A `tool_call { config.slug: "browser" }` node routes to the Chrome +// companion (`src/openhuman/browser_companion/`) instead of Composio. Same +// two-layer posture as B45: +// +// - **Author time** (`build_builder_proposal`) — ADVISORY ONLY, via +// `evaluate_browser_readiness`: `browser_status`/`browser_message` ride +// along on the proposal payload so the UI can nudge "install/pair the +// extension" without ever blocking authoring. +// - **Run time** (`run_flow_body`) — HARD gate, via +// `validate_browser_readiness`: a real run (never `dry_run_workflow`) +// requires the companion running, an extension connected, AND a +// `browser_tab_id` for THIS run before the engine executes. + +/// The 16 browser actions `tinyflows::browser::BrowserAction` accepts +/// (`#[serde(tag = "action", rename_all = "snake_case")]`) — kept as a plain +/// list here (rather than parsing through the real enum) so this author-time +/// check has no dependency on `tinyflows::browser`'s exact deserialization +/// error shape, only on the wire vocabulary. +pub(crate) const BROWSER_ACTIONS: [&str; 16] = [ + "open", + "snapshot", + "click", + "fill", + "type", + "get_text", + "get_title", + "get_url", + "screenshot", + "wait", + "press", + "hover", + "scroll", + "is_visible", + "close", + "find", +]; + +/// Whether `graph` has at least one `tool_call` node whose `config.slug` is +/// the built-in Chrome-automation slug `"browser"` (not a Composio action). +pub(crate) fn graph_has_browser_node(graph: &WorkflowGraph) -> bool { + graph.nodes.iter().any(|node| { + node.kind == NodeKind::ToolCall + && node.config.get("slug").and_then(Value::as_str) == Some("browser") + }) +} + +/// Outcome of [`evaluate_browser_readiness`] for a graph that has at least +/// one `tool_call { slug: "browser" }` node. +struct BrowserReadinessEvaluation { + /// `"ready"` or `"not_ready"` — the fixed vocabulary shared with the + /// proposal payload's `browser_status` field. + status: &'static str, + /// User-actionable prose; `None` only when `status == "ready"`. + message: Option, +} + +/// Evaluate Browser Companion author-time readiness for `graph`. Returns +/// `None` when the graph has no `tool_call { slug: "browser" }` node at all +/// — a graph with no browser step never pays this check's cost, and the +/// proposal payload omits both `browser_status`/`browser_message` entirely +/// rather than claiming a meaningless "ready" (same contract as +/// [`evaluate_inference_readiness`]). +/// +/// Unlike inference readiness this needs no network probe — companion +/// running / extension-connected are both in-process state +/// ([`crate::openhuman::browser_companion::companion_status`]) — so this +/// is synchronous. The per-run tab selection (`browser_tab_id`) is NOT +/// evaluated here: a tab is chosen per-run, not per-authored-graph, so it is +/// only checked by [`validate_browser_readiness`] at run time. +fn evaluate_browser_readiness( + config: &Config, + graph: &WorkflowGraph, +) -> Option { + if !graph_has_browser_node(graph) { + return None; + } + let status = crate::openhuman::browser_companion::companion_status(config); + if !status.running { + return Some(BrowserReadinessEvaluation { + status: "not_ready", + message: Some( + "This flow uses the Chrome browser companion, which isn't running. Enable it in \ + Settings > Browser Companion." + .to_string(), + ), + }); + } + if !status.extension_connected { + return Some(BrowserReadinessEvaluation { + status: "not_ready", + message: Some( + "This flow uses the Chrome browser companion, but no Chrome extension is \ + connected yet. Install & pair the Chrome extension in Settings > Browser \ + Companion, then share a tab." + .to_string(), + ), + }); + } + Some(BrowserReadinessEvaluation { + status: "ready", + message: None, + }) +} + +/// The Browser Companion run-time HARD gate: empty when `graph` has no +/// `tool_call { slug: "browser" }` node, or when it does and the companion is +/// running, an extension is connected, AND `browser_tab_id` names the tab for +/// this run. Otherwise one specific, actionable error per missing +/// precondition — mirrors [`validate_inference_readiness`]'s shape exactly so +/// `run_flow_body` can gate on it identically. +/// +/// **No longer a builder gate.** Same posture as B45: a graph is never +/// rejected at author time for browser-companion readiness (see +/// [`evaluate_browser_readiness`]'s advisory-only use in +/// `build_builder_proposal`) — only a REAL run (never `dry_run_workflow`) +/// hard-fails here, before the tinyflows engine executes. +pub(crate) fn validate_browser_readiness( + config: &Config, + graph: &WorkflowGraph, + browser_tab_id: Option, +) -> Vec { + if !graph_has_browser_node(graph) { + return Vec::new(); + } + let status = crate::openhuman::browser_companion::companion_status(config); + if !status.running { + return vec![ + "This flow uses the Chrome browser companion, which is not running. Enable it in \ + Settings > Browser Companion." + .to_string(), + ]; + } + if !status.extension_connected { + return vec![ + "This flow uses the Chrome browser companion, but no Chrome extension is currently \ + connected. Install & pair the extension in Settings > Browser Companion." + .to_string(), + ]; + } + if browser_tab_id.is_none() { + return vec![ + "This flow uses the Chrome browser companion, but no browser tab was selected for \ + this run. Share a tab with the companion and pass its tab id when running this flow." + .to_string(), + ]; + } + Vec::new() +} + // ───────────────────────────────────────────────────────────────────────────── // Tool-contract enforcement gate (systemic tool-contract fix, Part 2) // ───────────────────────────────────────────────────────────────────────────── @@ -2366,6 +2532,49 @@ pub(crate) async fn validate_tool_contracts(config: &Config, graph: &WorkflowGra let Some(slug) = node.config.get("slug").and_then(Value::as_str) else { continue; }; + // Built-in Chrome-automation slug (Browser Companion, Stage C3) — NOT + // a Composio action, so it never hits the live catalog below. Its + // contract is `config.args.action` being one of the 16 supported + // browser actions, checked statically here instead. + if slug == "browser" { + let action = node + .config + .get("args") + .and_then(|a| a.get("action")) + .and_then(Value::as_str); + match action { + Some(a) if BROWSER_ACTIONS.contains(&a) => {} + Some(a) => { + tracing::warn!( + target: "flows", + node = %node.id, + action = %a, + "[flows] tool-contract check: browser action is not one of the supported \ + actions — rejecting" + ); + errors.push(format!( + "Node '{}': tool_call `browser` action `{a}` is not one of the supported \ + browser actions ({}).", + node.id, + BROWSER_ACTIONS.join(", ") + )); + } + None => { + tracing::warn!( + target: "flows", + node = %node.id, + "[flows] tool-contract check: browser tool_call missing config.args.action \ + — rejecting" + ); + errors.push(format!( + "Node '{}': tool_call `browser` requires `config.args.action`, one of: {}.", + node.id, + BROWSER_ACTIONS.join(", ") + )); + } + } + continue; + } // `=`-derived slugs resolve from upstream/trigger data at runtime — // nothing to check statically. Native `oh:` tools have no Composio // contract. @@ -2619,6 +2828,13 @@ fn validate_connection_refs_against( let Some(slug) = node.config.get("slug").and_then(Value::as_str) else { continue; }; + // Built-in Chrome-automation slug (Browser Companion, Stage C3) — + // carries no `connection_ref` at all (it authorizes against a + // per-run shared tab instead, checked by `validate_browser_readiness` + // at run time), so it has nothing for this gate to check. + if slug == "browser" { + continue; + } // `=`-derived slugs resolve at runtime; native `oh:` tools have no // Composio connection to name. if slug.starts_with('=') || slug.starts_with("oh:") { @@ -4307,6 +4523,33 @@ pub async fn flows_run( flow_id: &str, input: Value, trigger: FlowRunTrigger, +) -> Result, String> { + flows_run_with_browser_tab(config, flow_id, input, trigger, None).await +} + +/// Same as [`flows_run`], plus an optional `browser_tab_id`: the browser tab +/// (explicitly shared with the Browser Companion — see +/// `src/openhuman/browser_companion/`) this run's `tool_call { slug: "browser" }` +/// node(s), if any, are authorized against. `None` when the graph has no +/// browser node, or when the caller doesn't select one (in which case a +/// browser-node graph fails the run-time hard gate in [`run_flow_body`] with +/// a "no browser tab was selected" error rather than silently running +/// unrouted). +/// +/// Split out as its own function (rather than adding the parameter directly +/// to [`flows_run`]) so every existing call site of the public, +/// long-standing `flows_run` signature — the automatic trigger dispatch in +/// `flows::bus`, the agent `run_flow` tool's `flows_run_detached`, and the +/// large existing `ops_tests.rs` suite — is unaffected. Today only the +/// `flows.run` RPC handler (`schemas::handle_run`) threads a real +/// `browser_tab_id` through, parsed from the RPC's own `browser_tab_id` input +/// field. +pub async fn flows_run_with_browser_tab( + config: &Config, + flow_id: &str, + input: Value, + trigger: FlowRunTrigger, + browser_tab_id: Option, ) -> Result, String> { // Prep synchronously (validate + compile-check + mint the run id), insert // the initial `running` row, and announce it, then hand off to the shared @@ -4333,6 +4576,7 @@ pub async fn flows_run( no_actionable_nodes, cancel_token, run_guard, + browser_tab_id, ) .await } @@ -4388,6 +4632,10 @@ pub async fn flows_run_detached( let flow_id_owned = flow_id.to_string(); let body_thread_id = thread_id.clone(); tokio::spawn(async move { + // Agent-initiated runs don't (yet) select a browser tab — a graph + // with a `tool_call { slug: "browser" }` node fails the run-time hard + // gate below with a "no browser tab was selected" error rather than + // running unrouted. See `flows_run_with_browser_tab`'s doc. if let Err(e) = run_flow_body( config_arc, flow, @@ -4398,6 +4646,7 @@ pub async fn flows_run_detached( no_actionable_nodes, cancel_token, run_guard, + None, ) .await { @@ -4592,6 +4841,34 @@ impl Drop for RunRowFinalizer { } } +/// RAII guard (Stage C3) that releases a Browser Companion run→tab binding +/// ([`crate::openhuman::browser_companion::unbind_run`]) on `Drop` — +/// covering every exit path of `run_flow_body` (success, error, timeout, +/// cancellation mid-`tokio::select!`) without an explicit unbind call +/// sprinkled at each return, mirroring how [`RunRowFinalizer`] guarantees a +/// terminal write on drop. `None` is a no-op guard: nothing was bound (no +/// browser node in this run's graph, or the companion/tab weren't ready — +/// the run-time hard gate would already have failed the run in that case). +struct BrowserRunUnbindGuard(Option); + +impl BrowserRunUnbindGuard { + fn none() -> Self { + Self(None) + } + + fn bound(thread_id: &str) -> Self { + Self(Some(thread_id.to_string())) + } +} + +impl Drop for BrowserRunUnbindGuard { + fn drop(&mut self) { + if let Some(thread_id) = &self.0 { + crate::openhuman::browser_companion::unbind_run(thread_id); + } + } +} + /// Executes an already-prepared, already-`running`-row-inserted flow run to a /// terminal state, finalizing the `flow_runs` row on every exit path. /// @@ -4613,6 +4890,7 @@ impl Drop for RunRowFinalizer { /// status. Registering before the `run_id` is observable makes the cancel /// always take the signalled branch instead. `_run_guard` is held for the whole /// body and deregisters on any exit, including the early returns below. +#[allow(clippy::too_many_arguments)] async fn run_flow_body( config_arc: Arc, flow: Flow, @@ -4623,6 +4901,7 @@ async fn run_flow_body( no_actionable_nodes: bool, cancel_token: tokio_util::sync::CancellationToken, _run_guard: run_registry::RunGuard, + browser_tab_id: Option, ) -> Result, String> { let config: &Config = config_arc.as_ref(); let flow_id: &str = flow_id.as_str(); @@ -4675,6 +4954,42 @@ async fn run_flow_body( return Err(msg); } + // Browser Companion run-time hard gate (Stage C3 — mirrors the B45 + // inference-readiness preflight immediately above, same "fail cleanly + // before the engine executes" placement). `validate_browser_readiness` is + // a no-op `Vec` for a graph with no `tool_call { slug: "browser" }` node. + let browser_errors = validate_browser_readiness(config, &flow.graph, browser_tab_id); + if !browser_errors.is_empty() { + let detail = browser_errors.join(" "); + let msg = + format!("This flow's browser step needs the Chrome companion ready to run. {detail}"); + tracing::warn!( + target: "flows", + flow_id, + "[flows] run_flow_body: browser-companion readiness preflight failed — finalizing \ + run as failed without invoking the engine: {msg}" + ); + if let Err(rec_err) = store::record_run(config, flow_id, "failed") { + tracing::warn!( + target: "flows", + flow_id, + error = %rec_err, + "[flows] run_flow_body: failed to record failed run (browser-companion preflight)" + ); + } + let observed = current_persisted_steps(config, &thread_id); + finish_flow_run_row( + config, + &thread_id, + flow_id, + "failed", + &observed, + &[], + Some(&msg), + ); + return Err(msg); + } + // Recompile to execute — the entry point already compile-checked to fail // fast before the running row existed. A failure *now* (after the row was // inserted) must finalize the row as failed, never orphan it. @@ -4698,10 +5013,58 @@ async fn run_flow_body( }; // Scope the state store per-flow so two flows never collide on a state key. - let caps = crate::openhuman::tinyflows::build_capabilities( + let mut caps = crate::openhuman::tinyflows::build_capabilities( config_arc.clone(), format!("flow:{flow_id}"), ); + // Browser Companion routing (Stage C3). This is one of the TWO real + // `build_capabilities` execution sites in this file (the other is + // `flows_resume`'s, further below) — `dry_run_workflow` is a sandbox and + // does not route through `run_flow_body` at all, so there is no mock path + // here that must NOT be wrapped. The run-time hard gate above already + // proved (when the graph has a browser node) that the companion is + // running, an extension is connected, and `browser_tab_id` is set; + // `bind_run` is the remaining authoritative check that THIS SPECIFIC tab + // is actually shared — a real `tab_not_shared`-shaped error surfaces here + // if it isn't. + let mut _browser_unbind_guard = BrowserRunUnbindGuard::none(); + if graph_has_browser_node(&flow.graph) { + if let (Some(relay), Some(tab_id)) = ( + crate::openhuman::browser_companion::browser_relay(), + browser_tab_id, + ) { + if let Err(e) = crate::openhuman::browser_companion::bind_run(&thread_id, tab_id) { + let msg = e.to_string(); + tracing::warn!( + target: "flows", + flow_id, + error = %msg, + "[flows] run_flow_body: browser bind_run failed after start row inserted" + ); + let observed = current_persisted_steps(config, &thread_id); + finish_flow_run_row( + config, + &thread_id, + flow_id, + "failed", + &observed, + &[], + Some(&msg), + ); + return Err(msg); + } + _browser_unbind_guard = BrowserRunUnbindGuard::bound(&thread_id); + let browser = std::sync::Arc::new(tinyflows::browser::ChromeToolInvoker::new( + relay, + thread_id.clone(), + tab_id, + )); + caps.tools = std::sync::Arc::new(tinyflows::browser::RoutingToolInvoker::new( + browser, + caps.tools.clone(), + )); + } + } let checkpointer = match crate::openhuman::tinyflows::open_flow_checkpointer(config) { Ok(checkpointer) => checkpointer, Err(e) => { @@ -5013,6 +5376,19 @@ pub async fn flows_resume( } let compiled = tinyflows::compiler::compile(&flow.graph).map_err(|e| e.to_string())?; let config_arc = Arc::new(config.clone()); + // NOT wrapped with `RoutingToolInvoker` (Stage C3 deviation from the + // brief, which asked for both `build_capabilities` sites): this is a + // second REAL execution path (resuming a paused human-in-the-loop + // checkpoint), not a mock/dry-run — but unlike `run_flow_body`, + // `flows_resume` has no `browser_tab_id` input in this increment (see + // `flows_run_with_browser_tab`'s doc — only `flows.run` threads one + // through), so there is no tab to rebind here. A resumed run whose graph + // has a `tool_call { slug: "browser" }` node still fails CLOSED: with no + // `RoutingToolInvoker` installed, `OpenHumanTools::invoke`'s explicit + // `slug == "browser"` early-out (`src/openhuman/tinyflows/caps.rs`) is + // the fallback that rejects it with an actionable message instead of + // running unrouted. Threading `browser_tab_id` through `flows_resume` + // too is follow-up, not required for this increment. let caps = crate::openhuman::tinyflows::build_capabilities(config_arc, format!("flow:{flow_id}")); let checkpointer = diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index b0b5016b8b..0dc733ba3a 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -4099,6 +4099,64 @@ async fn validate_tool_contracts_passes_a_fully_wired_real_slug() { assert!(errors.is_empty(), "{errors:?}"); } +// ── validate_tool_contracts: built-in `browser` slug (Stage C3) ───────────── +// The `browser` slug is NOT a Composio action — no live-catalog fetch, no +// curation check. Its contract is `config.args.action` being one of the 16 +// supported browser actions. + +fn browser_tool_call_graph(action_args: Value) -> WorkflowGraph { + graph(json!({ + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Manual" }, + { "id": "act", "kind": "tool_call", "name": "Act", + "config": { "slug": "browser", "args": action_args } } + ], + "edges": [ { "from_node": "t", "to_node": "act" } ] + })) +} + +#[tokio::test] +async fn validate_tool_contracts_rejects_a_browser_node_missing_action() { + let config = Config::default(); + let g = browser_tool_call_graph(json!({})); + let errors = validate_tool_contracts(&config, &g).await; + assert_eq!(errors.len(), 1, "{errors:?}"); + assert!(errors[0].contains("act"), "{}", errors[0]); + assert!(errors[0].contains("config.args.action"), "{}", errors[0]); +} + +#[tokio::test] +async fn validate_tool_contracts_rejects_a_browser_node_with_an_unknown_action() { + let config = Config::default(); + let g = browser_tool_call_graph(json!({ "action": "teleport" })); + let errors = validate_tool_contracts(&config, &g).await; + assert_eq!(errors.len(), 1, "{errors:?}"); + assert!(errors[0].contains("teleport"), "{}", errors[0]); + assert!(errors[0].contains("act"), "{}", errors[0]); +} + +#[tokio::test] +async fn validate_tool_contracts_passes_every_valid_browser_action() { + let config = Config::default(); + for action in BROWSER_ACTIONS { + let g = browser_tool_call_graph(json!({ "action": action })); + let errors = validate_tool_contracts(&config, &g).await; + assert!(errors.is_empty(), "action `{action}`: {errors:?}"); + } +} + +#[tokio::test] +async fn validate_tool_contracts_never_hits_the_live_catalog_for_browser() { + // No `seed_live_catalog_cache` call for "browser" — if the check somehow + // treated it as a Composio toolkit it would either panic on an unseeded + // cache lookup or best-effort-skip (never actually validating the + // action), rather than rejecting the unknown action as it must. + let config = Config::default(); + let g = browser_tool_call_graph(json!({ "action": "not_a_real_action" })); + let errors = validate_tool_contracts(&config, &g).await; + assert_eq!(errors.len(), 1, "{errors:?}"); +} + // ── validate_connection_refs (WS3) ────────────────────────────────────────── // // The transcript bug: the user's connections were twitter → @@ -4234,6 +4292,18 @@ fn connection_refs_skip_oh_and_refless_and_expression_nodes() { ); } +#[test] +fn connection_refs_skip_the_builtin_browser_slug() { + // Stage C3: a `browser` tool_call node carries no `connection_ref` at all + // (it authorizes against a per-run shared tab instead) — even a stray + // `connection_ref` on one must not be checked against the Composio + // connection list. + let g = ws3_tool_call_graph("browser", Some("composio:twitter:whatever")); + assert!(validate_connection_refs_against(&g, Some(&ws3_transcript_connections())).is_empty()); + // Also skipped when connections are unavailable (fail-open path). + assert!(validate_connection_refs_against(&g, None).is_empty()); +} + #[test] fn connection_refs_fail_open_on_unavailable_connections_but_keep_mismatch() { // Connections unavailable (None): the id-existence check is SKIPPED — a @@ -4256,6 +4326,80 @@ fn connection_refs_fail_open_on_unavailable_connections_but_keep_mismatch() { assert!(errors[0].contains("tiktok"), "{}", errors[0]); } +// ── Browser Companion readiness gate (Stage C3, mirrors B45) ──────────────── +// +// `validate_browser_readiness` reads the process-wide Browser Companion +// runtime singleton (`browser_companion::ops`'s own tests document why it +// can't be a hard per-test guarantee — only one test in the whole binary +// starts the relay with `enabled: true`). These tests are written to hold +// deterministically regardless of that shared state where possible, and skip +// (rather than flake) the one case that truly needs the relay to be down. + +#[test] +fn graph_has_browser_node_detects_and_excludes_correctly() { + let with_browser = browser_tool_call_graph(json!({ "action": "snapshot" })); + assert!(graph_has_browser_node(&with_browser)); + + let without_browser = graph(json!({ + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Manual" }, + { "id": "post", "kind": "tool_call", "name": "Post", + "config": { "slug": "SLACK_SEND_MESSAGE", "args": {} } } + ], + "edges": [ { "from_node": "t", "to_node": "post" } ] + })); + assert!(!graph_has_browser_node(&without_browser)); + + let trigger_only = graph(trigger_only_graph()); + assert!(!graph_has_browser_node(&trigger_only)); +} + +#[test] +fn validate_browser_readiness_is_a_noop_for_a_graph_without_a_browser_node() { + let config = Config::default(); + let g = graph(json!({ + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Manual" }, + { "id": "post", "kind": "tool_call", "name": "Post", + "config": { "slug": "SLACK_SEND_MESSAGE", "args": {} } } + ], + "edges": [ { "from_node": "t", "to_node": "post" } ] + })); + // Regardless of the process-wide companion runtime state, a graph with no + // browser node never pays this check's cost. + assert!(validate_browser_readiness(&config, &g, None).is_empty()); + assert!(validate_browser_readiness(&config, &g, Some(1)).is_empty()); +} + +#[test] +fn validate_browser_readiness_rejects_a_browser_graph_with_no_tab_selected() { + let config = Config::default(); + let g = browser_tool_call_graph(json!({ "action": "snapshot" })); + // Deterministic regardless of the shared companion-runtime singleton's + // state: whichever of the three preconditions (running / connected / tab + // selected) is unmet, `browser_tab_id: None` guarantees the "no tab + // selected" one always is — so this must always reject with exactly one + // error. + let errors = validate_browser_readiness(&config, &g, None); + assert_eq!(errors.len(), 1, "{errors:?}"); +} + +#[test] +fn validate_browser_readiness_rejects_when_the_companion_is_not_running() { + // Only meaningful to assert when the shared companion-runtime singleton + // is known NOT to be running (the default/common case) — skip rather + // than flake if some other test in this binary happens to have it up + // concurrently. + if crate::openhuman::browser_companion::browser_relay().is_some() { + return; + } + let config = Config::default(); + let g = browser_tool_call_graph(json!({ "action": "snapshot" })); + let errors = validate_browser_readiness(&config, &g, Some(1)); + assert_eq!(errors.len(), 1, "{errors:?}"); + assert!(errors[0].contains("not running"), "{}", errors[0]); +} + // ── validate_required_arg_resolvability (issue B18) ───────────────────────── // // `validate_tool_contracts`'s `missing_required_args` only proves an arg is diff --git a/src/openhuman/flows/schemas.rs b/src/openhuman/flows/schemas.rs index fc50b29133..cfcf140005 100644 --- a/src/openhuman/flows/schemas.rs +++ b/src/openhuman/flows/schemas.rs @@ -686,6 +686,15 @@ pub fn schemas(function: &str) -> ControllerSchema { comment: "Trigger payload seeded into the run; defaults to null.", required: false, }, + FieldSchema { + name: "browser_tab_id", + ty: TypeSchema::Option(Box::new(TypeSchema::U64)), + comment: "Chrome tab id (from the Browser Companion's shared-tab list) this \ + run's `tool_call { slug: \"browser\" }` node(s), if any, are \ + authorized against. Required for a graph with a browser node; \ + ignored otherwise.", + required: false, + }, ], outputs: vec![FieldSchema { name: "result", @@ -1413,12 +1422,14 @@ fn handle_run(params: Map) -> ControllerFuture { let config = config_rpc::load_config_with_timeout().await?; let id = read_required::(¶ms, "id")?; let input = params.get("input").cloned().unwrap_or(Value::Null); + let browser_tab_id = params.get("browser_tab_id").and_then(Value::as_u64); to_json( - ops::flows_run( + ops::flows_run_with_browser_tab( &config, id.trim(), input, crate::openhuman::flows::FlowRunTrigger::Rpc, + browser_tab_id, ) .await?, ) diff --git a/src/openhuman/mod.rs b/src/openhuman/mod.rs index 5b30de78b0..f8c04816a8 100644 --- a/src/openhuman/mod.rs +++ b/src/openhuman/mod.rs @@ -31,6 +31,8 @@ pub mod artifacts; #[cfg(feature = "voice")] pub mod audio_toolkit; pub mod billing; +#[cfg(feature = "flows")] +pub mod browser_companion; pub mod channels; pub mod composio; pub mod config; diff --git a/src/openhuman/tinyflows/caps.rs b/src/openhuman/tinyflows/caps.rs index 47ab333520..f8fcbb8b15 100644 --- a/src/openhuman/tinyflows/caps.rs +++ b/src/openhuman/tinyflows/caps.rs @@ -2810,6 +2810,26 @@ impl ToolInvoker for PreflightToolInvoker { #[async_trait] impl ToolInvoker for OpenHumanTools { async fn invoke(&self, slug: &str, args: Value, conn: Option<&str>) -> Result { + // Browser Companion fallback (Stage C3): `slug:"browser"` is a + // built-in Chrome-automation slug, never a Composio action nor a + // native `oh:` tool. `OpenHumanTools` reaching this branch for it at + // all means `src/openhuman/flows/ops.rs::run_flow_body` did NOT + // install a `tinyflows::browser::RoutingToolInvoker` around this + // run's tools — the companion isn't running, no extension is + // connected, or no tab was shared/selected for this run (the + // run-time hard gate — `validate_browser_readiness` — should already + // have failed such a run before the engine ever started, but this is + // the guaranteed backstop). Reject explicitly here rather than + // falling through to the Composio branch below, which would fail + // with a confusing "unknown toolkit" instead of this actionable + // message. + if slug == "browser" { + return Err(EngineError::Capability( + "the Chrome browser companion is not running or no tab was shared — enable it \ + in Settings > Browser Companion and share a tab" + .to_string(), + )); + } // Native OpenHuman tool path (the "Tool" node): `oh:`. Bypasses // the Composio curation gate (it isn't a Composio slug) but still runs // through the autonomy-tier + approval gates, then dispatches to the @@ -4537,6 +4557,34 @@ mod tests { } } + // ── Browser Companion fallback (Stage C3) ─────────────────────────────── + + /// `slug:"browser"` reaching `OpenHumanTools::invoke` directly (i.e. no + /// `RoutingToolInvoker` was installed around it — the companion off, or + /// no tab shared/selected for this run) must fail with the dedicated, + /// actionable "not running or no tab was shared" message rather than + /// falling through to the Composio branch and failing with a confusing + /// "unknown toolkit" error instead. + #[tokio::test] + async fn browser_slug_without_a_routing_invoker_fails_with_actionable_message() { + use crate::openhuman::security::AutonomyLevel; + + let tools = OpenHumanTools { + config: Arc::new(Config::default()), + security: Arc::new(policy(AutonomyLevel::Full)), + }; + let err = tools + .invoke("browser", json!({"action": "snapshot"}), None) + .await + .expect_err("browser slug with no RoutingToolInvoker installed must fail"); + if let EngineError::Capability(msg) = err { + assert!(msg.contains("browser companion"), "{msg}"); + assert!(msg.contains("Settings > Browser Companion"), "{msg}"); + } else { + panic!("expected EngineError::Capability"); + } + } + /// End-to-end at the adapter: a Composio `tool_call` node under a /// read-only tier is refused BEFORE it ever reaches the curation gate or /// any Composio dispatch — closes the compound bypass where the Composio diff --git a/vendor/tinyflows b/vendor/tinyflows index fb24363aea..bef4df648f 160000 --- a/vendor/tinyflows +++ b/vendor/tinyflows @@ -1 +1 @@ -Subproject commit fb24363aea921f957958bc8f4aeb5b0a244e41c7 +Subproject commit bef4df648f940307c96370688dd25f6aa2e2e9a8 From 440456cb1ec8f56f84649c6ba3b3f4dce6f13aa3 Mon Sep 17 00:00:00 2001 From: graycyrus Date: Wed, 29 Jul 2026 00:37:13 +0530 Subject: [PATCH 2/4] =?UTF-8?q?fix(browser-companion):=20address=20review?= =?UTF-8?q?=20=E2=80=94=20listener/state=20sync,=20active-id=20retention,?= =?UTF-8?q?=20coverage,=20CI=20gates?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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). --- .github/workflows/ci-lite.yml | 1 + app/src-tauri/Cargo.lock | 24 ++++ src/core/runtime/builder.rs | 23 ++++ src/openhuman/browser_companion/ops.rs | 159 ++++++++++++++++++----- src/openhuman/browser_companion/types.rs | 28 ++++ 5 files changed, 199 insertions(+), 36 deletions(-) diff --git a/.github/workflows/ci-lite.yml b/.github/workflows/ci-lite.yml index 10951bc942..fa8013b988 100644 --- a/.github/workflows/ci-lite.yml +++ b/.github/workflows/ci-lite.yml @@ -456,6 +456,7 @@ jobs: core/cli_tests.rs core/jsonrpc_tests.rs core/legacy_aliases.rs + core/runtime/builder.rs core/runtime/services.rs openhuman/agent/harness/builtin_definitions.rs openhuman/agent/harness/definition_tests.rs diff --git a/app/src-tauri/Cargo.lock b/app/src-tauri/Cargo.lock index cee8aea064..2a47f5cf74 100644 --- a/app/src-tauri/Cargo.lock +++ b/app/src-tauri/Cargo.lock @@ -581,6 +581,7 @@ dependencies = [ "tower", "tower-layer", "tower-service", + "tracing", ] [[package]] @@ -599,6 +600,7 @@ dependencies = [ "sync_wrapper", "tower-layer", "tower-service", + "tracing", ] [[package]] @@ -5601,6 +5603,7 @@ dependencies = [ "tinychannels", "tinycortex", "tinyflows", + "tinyhumans-sdk", "tinyjuice", "tinyplace", "tokio", @@ -9014,17 +9017,36 @@ name = "tinyflows" version = "0.5.1" dependencies = [ "async-trait", + "axum", "futures-timer", + "futures-util", + "getrandom 0.3.4", "jaq-core", "jaq-json", "jaq-std", + "reqwest 0.12.28", "serde", "serde_json", "thiserror 2.0.18", "tinyagents", + "tokio", "tracing", ] +[[package]] +name = "tinyhumans-sdk" +version = "0.1.0" +source = "git+https://github.com/tinyhumansai/sdk.git?rev=3ee4123ba3b7a76c5f167d7bc2c72fca86671292#3ee4123ba3b7a76c5f167d7bc2c72fca86671292" +dependencies = [ + "base64 0.22.1", + "percent-encoding", + "reqwest 0.12.28", + "serde", + "serde_json", + "thiserror 2.0.18", + "url", +] + [[package]] name = "tinyjuice" version = "0.2.1" @@ -9342,6 +9364,7 @@ dependencies = [ "tokio", "tower-layer", "tower-service", + "tracing", ] [[package]] @@ -9380,6 +9403,7 @@ version = "0.1.44" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "63e71662fa4b2a2c3a26f570f037eb95bb1f85397f3cd8076caed2f026a6d100" dependencies = [ + "log", "pin-project-lite", "tracing-attributes", "tracing-core", diff --git a/src/core/runtime/builder.rs b/src/core/runtime/builder.rs index 848165f88b..f5676bd107 100644 --- a/src/core/runtime/builder.rs +++ b/src/core/runtime/builder.rs @@ -923,4 +923,27 @@ mod tests { assert!(!headless.memory_sync); assert!(!headless.orchestration); } + + #[test] + fn companion_relay_service_is_desktop_only() { + // The browser-companion relay binds a loopback WebSocket for the Chrome + // extension and must run ONLY on the desktop host — never in the + // headless API, the bare `none()` set, or the embedded runtime. + assert!( + ServiceSet::desktop().companion_relay, + "desktop() must enable companion_relay" + ); + assert!( + !ServiceSet::headless_api().companion_relay, + "headless_api() must not enable companion_relay" + ); + assert!( + !ServiceSet::none().companion_relay, + "none() must not enable companion_relay" + ); + assert!( + !ServiceSet::embedded().companion_relay, + "embedded() must not enable companion_relay" + ); + } } diff --git a/src/openhuman/browser_companion/ops.rs b/src/openhuman/browser_companion/ops.rs index aeb2f1a989..cc353b0a6b 100644 --- a/src/openhuman/browser_companion/ops.rs +++ b/src/openhuman/browser_companion/ops.rs @@ -27,6 +27,11 @@ const CAPS_STATE_NAMESPACE: &str = "browser-companion"; struct CompanionRuntime { server: Option, task: Option>, + /// The extension id the running relay was actually started with. This is + /// the authoritative source for status + restart — NOT `Config`, which may + /// hold a stale/default value after an unpersisted `pair` (Stage E persists + /// it; until then only the runtime knows the live id). + active_extension_id: Option, } impl CompanionRuntime { @@ -34,6 +39,32 @@ impl CompanionRuntime { Self { server: None, task: None, + active_extension_id: None, + } + } + + /// True only when a server is stored AND its listener task is still alive. + /// A task that already finished means `serve()` exited (e.g. the internal + /// bind failed) — so a stored `server` alone must NOT be reported running. + fn is_running(&self) -> bool { + self.server.is_some() && self.task.as_ref().is_some_and(|task| !task.is_finished()) + } + + /// Drops a server whose listener task has already exited, so a later start + /// is not blocked by dead state left behind by a failed `serve()`. + fn reap_if_dead(&mut self) { + if self.server.is_some() + && self + .task + .as_ref() + .is_some_and(tokio::task::JoinHandle::is_finished) + { + log::warn!( + "{LOG_PREFIX} reap_if_dead: listener task exited unexpectedly; clearing stale runtime state" + ); + self.server = None; + self.task = None; + self.active_extension_id = None; } } } @@ -74,10 +105,13 @@ pub async fn start_companion_server(config: &Config) -> anyhow::Result<()> { /// enabled-gate must go through [`start_companion_server`]. async fn start_with_extension_id(config: &Config, extension_id: String) -> anyhow::Result<()> { { - let guard = runtime() + let mut guard = runtime() .lock() .expect("browser_companion runtime poisoned"); - if guard.server.is_some() { + // A previous `serve()` may have exited (bind failure); clear that dead + // state so it does not masquerade as "already running" and block us. + guard.reap_if_dead(); + if guard.is_running() { log::debug!("{LOG_PREFIX} start_with_extension_id: already running; no-op"); return Ok(()); } @@ -107,6 +141,10 @@ async fn start_with_extension_id(config: &Config, extension_id: String) -> anyho let capabilities = build_capabilities(Arc::new(config.clone()), CAPS_STATE_NAMESPACE); + // Keep the id we started with as the authoritative active id (Config may + // not yet hold it — `pair` doesn't persist until Stage E). + let active_extension_id = extension_id.clone(); + let server_config = CompanionServerConfig { policy, extension_id, @@ -136,6 +174,7 @@ async fn start_with_extension_id(config: &Config, extension_id: String) -> anyho .expect("browser_companion runtime poisoned"); guard.server = Some(server); guard.task = Some(task); + guard.active_extension_id = Some(active_extension_id); log::info!("{LOG_PREFIX} start_with_extension_id: relay started bind_addr={bind_addr}"); Ok(()) } @@ -147,6 +186,7 @@ pub async fn stop_companion_server() { let mut guard = runtime() .lock() .expect("browser_companion runtime poisoned"); + guard.active_extension_id = None; (guard.server.take(), guard.task.take()) }; @@ -157,7 +197,11 @@ pub async fn stop_companion_server() { if let Some(task) = task { task.abort(); - log::info!("{LOG_PREFIX} stop_companion_server: relay task aborted"); + // Await the aborted task so its `TcpListener` is actually dropped before + // we return. Without this, an immediate pair/rotate restart can lose the + // race to re-bind the same loopback port (`Address already in use`). + let _ = task.await; + log::info!("{LOG_PREFIX} stop_companion_server: relay task aborted and joined"); } } @@ -235,34 +279,35 @@ pub fn is_extension_connected() -> bool { /// Current lifecycle + pairing snapshot. pub fn companion_status(config: &Config) -> BrowserCompanionStatus { - let guard = runtime() + let mut guard = runtime() .lock() .expect("browser_companion runtime poisoned"); - let running = guard.server.is_some(); + // Don't report a relay whose listener task already died as running. + guard.reap_if_dead(); + let running = guard.is_running(); let extension_connected = guard .server .as_ref() .map(CompanionServer::is_extension_connected) .unwrap_or(false); - let shared_tabs = guard + let shared_tabs: Vec<_> = guard .server .as_ref() .map(|server| server.shared_tabs().into_iter().map(Into::into).collect()) .unwrap_or_default(); - let paired_extension_id = if config.browser_companion.extension_id.is_empty() { - None - } else { + // Prefer the id the running relay was actually started with (the live + // truth); fall back to Config only when nothing is running (Stage E will + // persist the paired id so it survives a restart). + let paired_extension_id = guard.active_extension_id.clone().or_else(|| { Some(config.browser_companion.extension_id.clone()) - }; + .filter(|extension_id| !extension_id.is_empty()) + }); let relay_url = running.then(|| relay_url(config.browser_companion.port)); log::debug!( "{LOG_PREFIX} companion_status: running={running} extension_connected={extension_connected} shared_tab_count={}", - { - let count: &Vec<_> = &shared_tabs; - count.len() - } + shared_tabs.len() ); BrowserCompanionStatus { @@ -278,12 +323,12 @@ pub fn companion_status(config: &Config) -> BrowserCompanionStatus { /// (rather than whatever is currently persisted in `config`) and returns the /// relay URL + current pairing secret. /// -/// `config.browser_companion.extension_id` is **not** mutated here — the -/// caller of a future RPC handler is responsible for persisting the new -/// `extension_id` via `config.update_*` (see `TODO(stage-E)`). This -/// increment only needs the relay itself to come up bound to the new id, so -/// `extension_id` is threaded straight into [`start_with_extension_id`] -/// instead of round-tripping through a mutated config copy. +/// `config.browser_companion.extension_id` is **not** mutated here (Stage E +/// persists it via `config.update_*` — see `TODO(stage-E)`). The live id is +/// instead retained in `CompanionRuntime::active_extension_id`, which +/// [`companion_status`] and [`rotate_secret`] read as the authoritative +/// source — so status and secret-rotation stay correct across an unpersisted +/// pair, not just until the next process restart. pub async fn pair(config: &Config, extension_id: String) -> anyhow::Result { log::info!( "{LOG_PREFIX} pair: entry extension_id_len={}", @@ -338,15 +383,22 @@ pub async fn rotate_secret(config: &Config) -> anyhow::Result { anyhow::anyhow!("failed to rotate pairing secret: {error}") })?; - let was_running = { - let guard = runtime() + // Capture the live extension id BEFORE stopping so the restart rebinds to + // the same id — NOT `config.extension_id`, which may be stale/default after + // an unpersisted `pair`. Restarting via `start_companion_server` would also + // wrongly re-check `config.enabled` and no-op after such a pair. + let active_extension_id = { + let mut guard = runtime() .lock() .expect("browser_companion runtime poisoned"); - guard.server.is_some() + guard.reap_if_dead(); + guard + .is_running() + .then(|| guard.active_extension_id.clone().unwrap_or_default()) }; - if was_running { + if let Some(extension_id) = active_extension_id { stop_companion_server().await; - start_companion_server(config).await?; + start_with_extension_id(config, extension_id).await?; log::info!("{LOG_PREFIX} rotate_secret: relay restarted with rotated secret"); } @@ -369,15 +421,16 @@ mod tests { #[test] fn status_reports_no_paired_extension_for_default_config() { - // Deliberately does not assert `status.running`: the lifecycle test - // below shares the process-wide runtime static and may be running - // concurrently. `paired_extension_id` is derived purely from - // `config`, so it's deterministic regardless of runtime state. let tmp = tempfile::tempdir().expect("tempdir"); let config = test_config(tmp.path().to_path_buf()); - let status = companion_status(&config); - assert_eq!(status.paired_extension_id, None); assert!(!config.browser_companion.enabled); + // `paired_extension_id` now falls back to `config` ONLY when the shared + // runtime static is idle (it prefers the running relay's live active + // id). Guard on idle so the lifecycle test (which may run concurrently + // with a relay up) can't make this flake. + if browser_relay().is_none() { + assert_eq!(companion_status(&config).paired_extension_id, None); + } } #[test] @@ -385,11 +438,14 @@ mod tests { let tmp = tempfile::tempdir().expect("tempdir"); let mut config = test_config(tmp.path().to_path_buf()); config.browser_companion.extension_id = "abcdefghijklmnopabcdefghijklmnop".to_string(); - let status = companion_status(&config); - assert_eq!( - status.paired_extension_id, - Some("abcdefghijklmnopabcdefghijklmnop".to_string()) - ); + // Same idle-guard rationale as above: the config fallback is only the + // reported value when no relay (with its own active id) is running. + if browser_relay().is_none() { + assert_eq!( + companion_status(&config).paired_extension_id, + Some("abcdefghijklmnopabcdefghijklmnop".to_string()) + ); + } } #[test] @@ -477,6 +533,37 @@ mod tests { "no extension has connected yet" ); assert!(status.shared_tabs.is_empty()); + // The running relay's active id is reported from the runtime (the live + // authoritative source), matching what we started with. + assert_eq!( + status.paired_extension_id.as_deref(), + Some("abcdefghijklmnopabcdefghijklmnop") + ); + + // Re-pair with a DIFFERENT id: status must reflect the new id even + // though `config` still holds the old one and is never mutated — the + // runtime's active id is authoritative. Regression for the lost-id bug. + let new_id = "ponmlkjihgfedcbaponmlkjihgfedcba"; + let info = pair(&config, new_id.to_string()) + .await + .expect("pair should restart the relay with the new id"); + assert!(info.relay_url.starts_with("ws://127.0.0.1:")); + let paired = companion_status(&config); + assert!(paired.running, "relay should still be running after pair"); + assert_eq!(paired.paired_extension_id.as_deref(), Some(new_id)); + + // Rotate the secret: the relay must stay running (restarted with the + // active id, not the disabled/stale config) and return a fresh secret. + // Regression for "rotate stops but never restarts after an unpersisted + // pair". + let rotated = rotate_secret(&config) + .await + .expect("rotate_secret should restart the running relay"); + assert!(!rotated.pairing_secret.is_empty()); + assert!( + companion_status(&config).running, + "relay should still be running after rotate_secret" + ); // bind_run/unbind_run while the relay is running (still the only test // in this file exercising the shared runtime static with a real diff --git a/src/openhuman/browser_companion/types.rs b/src/openhuman/browser_companion/types.rs index 9b074c1002..db90436896 100644 --- a/src/openhuman/browser_companion/types.rs +++ b/src/openhuman/browser_companion/types.rs @@ -63,3 +63,31 @@ pub struct PairingInfo { /// The freshly (re)generated pairing secret, exposed exactly once here. pub pairing_secret: String, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn shared_tab_view_preserves_public_fields_and_drops_generation() { + let tab = tinyflows::companion::SharedTab { + id: 42, + window_id: 7, + url: "https://example.com/checkout".to_string(), + title: "Checkout".to_string(), + // `generation` is a relay-internal freshness counter that must NOT + // surface to callers — the mapping deliberately drops it. + generation: 99, + }; + + let view: SharedTabView = tab.into(); + + assert_eq!(view.id, 42); + assert_eq!(view.window_id, 7); + assert_eq!(view.url, "https://example.com/checkout"); + assert_eq!(view.title, "Checkout"); + // `SharedTabView` structurally has no `generation` field, so there is + // nothing internal to leak — asserting the four public fields above is + // the whole contract. + } +} From 1941695a1e86145b27f49cb429e5eef2dd6f3c07 Mon Sep 17 00:00:00 2001 From: graycyrus Date: Wed, 29 Jul 2026 00:44:45 +0530 Subject: [PATCH 3/4] fix(browser-companion): gate all public accessors on listener liveness 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. --- src/openhuman/browser_companion/ops.rs | 69 ++++++++++++-------------- 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/src/openhuman/browser_companion/ops.rs b/src/openhuman/browser_companion/ops.rs index cc353b0a6b..69b643e1ca 100644 --- a/src/openhuman/browser_companion/ops.rs +++ b/src/openhuman/browser_companion/ops.rs @@ -75,6 +75,22 @@ fn runtime() -> &'static Mutex { RUNTIME.get_or_init(|| Mutex::new(CompanionRuntime::empty())) } +/// Runs `f` against the live server, but ONLY when the relay is genuinely +/// running — first reaping any listener task that has already exited. Returns +/// `None` when the relay is not running (never started, stopped, or its +/// `serve()` died), so every public runtime observation honors listener +/// liveness rather than reading a stale `server` handle directly. +fn with_live_server(f: impl FnOnce(&CompanionServer) -> T) -> Option { + let mut guard = runtime() + .lock() + .expect("browser_companion runtime poisoned"); + guard.reap_if_dead(); + if !guard.is_running() { + return None; + } + guard.server.as_ref().map(f) +} + fn workflows_dir(config: &Config) -> std::path::PathBuf { config .workspace_dir @@ -209,10 +225,7 @@ pub async fn stop_companion_server() { /// paired extension, for later flows wiring (Stage C3). `None` when the /// relay is not running. pub fn browser_relay() -> Option> { - let guard = runtime() - .lock() - .expect("browser_companion runtime poisoned"); - guard.server.as_ref().map(CompanionServer::browser_relay) + with_live_server(CompanionServer::browser_relay) } /// Binds a workflow run to an explicitly-shared browser tab so that run's @@ -226,24 +239,20 @@ pub fn browser_relay() -> Option> { /// tab isn't one the extension has explicitly shared — `tab_not_shared`). pub fn bind_run(run_id: &str, tab_id: u64) -> anyhow::Result<()> { log::debug!("{LOG_PREFIX} bind_run: entry run_id={run_id} tab_id={tab_id}"); - let guard = runtime() - .lock() - .expect("browser_companion runtime poisoned"); - let Some(server) = guard.server.as_ref() else { + let Some(result) = with_live_server(|server| server.bind_run(run_id.to_string(), tab_id)) + else { log::warn!("{LOG_PREFIX} bind_run: relay not running; cannot bind run_id={run_id}"); return Err(anyhow::anyhow!( "browser companion relay is not running; cannot bind run '{run_id}' to tab {tab_id}" )); }; - server - .bind_run(run_id.to_string(), tab_id) - .map_err(|error| { - log::warn!( - "{LOG_PREFIX} bind_run: CompanionServer::bind_run failed run_id={run_id} \ + result.map_err(|error| { + log::warn!( + "{LOG_PREFIX} bind_run: CompanionServer::bind_run failed run_id={run_id} \ tab_id={tab_id}: {error}" - ); - anyhow::anyhow!("failed to bind run '{run_id}' to browser tab {tab_id}: {error}") - })?; + ); + anyhow::anyhow!("failed to bind run '{run_id}' to browser tab {tab_id}: {error}") + })?; log::info!("{LOG_PREFIX} bind_run: bound run_id={run_id} tab_id={tab_id}"); Ok(()) } @@ -253,28 +262,17 @@ pub fn bind_run(run_id: &str, tab_id: u64) -> anyhow::Result<()> { /// idempotent, mirroring `tinyflows::companion::CompanionServer::unbind_run`. pub fn unbind_run(run_id: &str) { log::debug!("{LOG_PREFIX} unbind_run: entry run_id={run_id}"); - let guard = runtime() - .lock() - .expect("browser_companion runtime poisoned"); - let Some(server) = guard.server.as_ref() else { + if with_live_server(|server| server.unbind_run(run_id)).is_none() { log::debug!("{LOG_PREFIX} unbind_run: relay not running; no-op"); return; - }; - server.unbind_run(run_id); + } log::debug!("{LOG_PREFIX} unbind_run: unbound run_id={run_id} (no-op if it wasn't bound)"); } /// Whether a paired extension currently holds an authenticated relay /// session. Always `false` when the relay is not running. pub fn is_extension_connected() -> bool { - let guard = runtime() - .lock() - .expect("browser_companion runtime poisoned"); - guard - .server - .as_ref() - .map(CompanionServer::is_extension_connected) - .unwrap_or(false) + with_live_server(CompanionServer::is_extension_connected).unwrap_or(false) } /// Current lifecycle + pairing snapshot. @@ -285,14 +283,13 @@ pub fn companion_status(config: &Config) -> BrowserCompanionStatus { // Don't report a relay whose listener task already died as running. guard.reap_if_dead(); let running = guard.is_running(); - let extension_connected = guard - .server - .as_ref() + // Only observe the server when it's genuinely running — consistent with the + // public accessors, which all gate on liveness via `with_live_server`. + let live_server = running.then(|| guard.server.as_ref()).flatten(); + let extension_connected = live_server .map(CompanionServer::is_extension_connected) .unwrap_or(false); - let shared_tabs: Vec<_> = guard - .server - .as_ref() + let shared_tabs: Vec<_> = live_server .map(|server| server.shared_tabs().into_iter().map(Into::into).collect()) .unwrap_or_default(); From e3c9366ea057f4e339d769a95c0ec424fec0ddbb Mon Sep 17 00:00:00 2001 From: graycyrus Date: Wed, 29 Jul 2026 01:43:52 +0530 Subject: [PATCH 4/4] chore(browser-companion): avoid literal cfg attribute in a comment tripping the feature-gate-smoke allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/openhuman/browser_companion/types.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/openhuman/browser_companion/types.rs b/src/openhuman/browser_companion/types.rs index db90436896..8fab1e07b7 100644 --- a/src/openhuman/browser_companion/types.rs +++ b/src/openhuman/browser_companion/types.rs @@ -20,10 +20,12 @@ pub struct SharedTabView { pub title: String, } -// No `#[cfg(feature = "flows")]` needed here: this whole module is only -// compiled when `browser_companion` itself is compiled, which is already -// gated behind `feature = "flows"` at the `pub mod browser_companion;` -// declaration in `src/openhuman/mod.rs`. +// No per-item feature gate is needed here: this whole module is only compiled +// when `browser_companion` itself is compiled, which is already gated behind the +// flows feature at the `pub mod browser_companion;` declaration in +// `src/openhuman/mod.rs`. (The literal gate attribute is intentionally NOT +// written above — the feature-gate-smoke lane greps sources for it to build its +// gated-test allowlist, and this file does not actually gate any test.) impl From for SharedTabView { fn from(tab: tinyflows::companion::SharedTab) -> Self { Self {