From 6363790b1f51359d3e77ae2742c25890fed9349e Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Tue, 16 Jun 2026 05:30:37 +0530 Subject: [PATCH] [merge 2/5] feat(channels/web): per-runtime approval-surface bridge for E2E tests (#3471) (#3666) Co-authored-by: Claude Fable 5 --- .../channels/providers/web/event_bus.rs | 66 +++++++++++++++++++ src/openhuman/channels/providers/web/mod.rs | 5 ++ 2 files changed, 71 insertions(+) diff --git a/src/openhuman/channels/providers/web/event_bus.rs b/src/openhuman/channels/providers/web/event_bus.rs index 43ed69684..902860003 100644 --- a/src/openhuman/channels/providers/web/event_bus.rs +++ b/src/openhuman/channels/providers/web/event_bus.rs @@ -179,6 +179,36 @@ impl EventHandler for ArtifactSurfaceSubscriber { } } +/// Create a **fresh** approval-surface subscription on the **current** tokio runtime. +/// +/// Unlike [`register_approval_surface_subscriber`], which is guarded by a process-level +/// [`OnceLock`] and intended for production use, this function subscribes unconditionally +/// and returns the [`SubscriptionHandle`] to the caller. +/// +/// The caller **must keep the returned handle alive** for the duration of the subscription. +/// Dropping it aborts the background task and silently stops bridging events. +/// +/// Primary use-case: integration tests that spin up a fresh tokio runtime per test. +/// The OnceLock-guarded singleton is tied to the runtime it was first registered on; when +/// that runtime drops, the task is cancelled and subsequent tests in the same process can no +/// longer receive `approval_request` SSE events. Calling this function once per test and +/// storing the handle on a local variable ensures the bridge runs on — and lives for exactly +/// as long as — the current test's runtime. +/// +/// Compiled only in debug builds (`#[cfg(debug_assertions)]`) so this OnceLock-bypassing +/// helper can never be linked into a release binary, where a second live subscriber would +/// surface every `ApprovalRequested` event twice. Production always uses the singleton +/// [`register_approval_surface_subscriber`]. +#[cfg(debug_assertions)] +#[doc(hidden)] +pub fn fresh_approval_surface_subscription() -> Option { + tracing::trace!( + "[web-channel] fresh_approval_surface_subscription — debug-only OnceLock bypass, \ + registering a per-runtime approval-surface bridge for tests" + ); + crate::core::event_bus::subscribe_global(Arc::new(ApprovalSurfaceSubscriber)) +} + struct ApprovalSurfaceSubscriber; #[async_trait] @@ -230,3 +260,39 @@ impl EventHandler for ApprovalSurfaceSubscriber { } } } + +#[cfg(test)] +mod tests { + use super::*; + + /// `fresh_approval_surface_subscription` returns `Some` when the global event bus has + /// been initialised and `None` otherwise (bus not started). It must never return `None` + /// after `init_global` has been called — the production path always initialises the bus + /// before the web channel starts handling requests. + #[tokio::test] + async fn fresh_approval_surface_subscription_returns_some_when_bus_is_ready() { + crate::core::event_bus::init_global(crate::core::event_bus::DEFAULT_CAPACITY); + let handle = fresh_approval_surface_subscription(); + assert!( + handle.is_some(), + "fresh_approval_surface_subscription() must return Some when the global event bus \ + is initialised" + ); + } + + /// Calling `fresh_approval_surface_subscription` multiple times returns independent + /// handles. Each is backed by its own background task so multiple callers can bridge + /// independently (e.g. multiple integration tests running sequentially in the same + /// process, each on their own tokio runtime). + #[tokio::test] + async fn fresh_approval_surface_subscription_is_not_a_singleton() { + crate::core::event_bus::init_global(crate::core::event_bus::DEFAULT_CAPACITY); + let h1 = fresh_approval_surface_subscription(); + let h2 = fresh_approval_surface_subscription(); + assert!(h1.is_some(), "first subscription handle must be Some"); + assert!(h2.is_some(), "second subscription handle must be Some"); + // Both handles are alive — drop explicitly to show they're independent. + drop(h1); + drop(h2); + } +} diff --git a/src/openhuman/channels/providers/web/mod.rs b/src/openhuman/channels/providers/web/mod.rs index bc4064c5c..22e81f9c6 100644 --- a/src/openhuman/channels/providers/web/mod.rs +++ b/src/openhuman/channels/providers/web/mod.rs @@ -26,6 +26,11 @@ pub use event_bus::{ register_artifact_surface_subscriber, subscribe_web_channel_events, }; +// Test-only: OnceLock-bypassing approval bridge for per-runtime integration tests. +// Compiled only in debug builds so it cannot be linked into a release binary. +#[cfg(debug_assertions)] +pub use event_bus::fresh_approval_surface_subscription; + // Public API — operations #[cfg(any(test, debug_assertions))] pub use ops::parallel_in_flight_entries_for_test;