diff --git a/crates/botloftd/src/browser/events.rs b/crates/botloftd/src/browser/events.rs index 3577092a..aed1bf40 100644 --- a/crates/botloftd/src/browser/events.rs +++ b/crates/botloftd/src/browser/events.rs @@ -16,7 +16,7 @@ use super::titles::retitles; impl Session { /// Sets up a new tab before it runs, and makes it the active one. - async fn attached(&self, params: &Value) { + async fn attached(&self, params: &Value, hooks: &Hooks) { let info = ¶ms["targetInfo"]; if info["type"] != "page" { return; @@ -46,6 +46,9 @@ impl Session { let extra = (tabs.list.len() > MAX_TABS).then(|| tabs.list[0].target.clone()); (previous, tabs.watching, extra, tabs.viewport) }; + // The app hears of the tab now: the setup below lets the page run, + // and a bot's tool may read it before the setup is done. + (hooks.changed)(self.lock().info()); let setup = [ ("Page.enable", json!({})), ("Network.enable", json!({})), @@ -123,7 +126,7 @@ impl Session { let params = &event.params; let Some(session) = event.session.as_deref() else { match event.method.as_str() { - "Target.attachedToTarget" => self.attached(params).await, + "Target.attachedToTarget" => self.attached(params, hooks).await, "Target.detachedFromTarget" => self.detached(params).await, "Target.targetInfoChanged" => self.info_changed(¶ms["targetInfo"]), "Browser.downloadWillBegin" => { @@ -277,24 +280,29 @@ pub(super) async fn pump( ) { let mut last = PageInfo::default(); loop { - tokio::select! { + let handled = tokio::select! { event = events.recv() => { let Some(event) = event else { break }; session.handle(&event, &hooks).await; - session.changed.notify_waiters(); if let Some(page) = event.session.filter(|_| retitles(&event.method)) { let session = Arc::clone(&session); tokio::spawn(async move { session.retitle(&page).await }); } + true } // The owner moved between tabs: no event says so. - () = session.moved.notified() => {} - } + () = session.moved.notified() => false, + }; let info = session.lock().info(); if info != last { (hooks.changed)(info.clone()); last = info; } + // Only now: a tool that waits for this event answers after the + // app's state shows it, so the tab list is never behind the bot. + if handled { + session.changed.notify_waiters(); + } } debug!("browser: closed"); session.close(); diff --git a/crates/botloftd/tests/browser.rs b/crates/botloftd/tests/browser.rs index 9b37d21a..9b69d4e4 100644 --- a/crates/botloftd/tests/browser.rs +++ b/crates/botloftd/tests/browser.rs @@ -182,13 +182,29 @@ async fn new_tabs_and_dialogs_reach_the_bot_and_the_owner_watches() { .await .expect("new tab"); assert!(tab.contains("Thanks, Tab"), "{tab}"); - let open = b.app.call("browser.list", Value::Null).await.expect("list"); - // The new tab is the active one, after the tab that opened it. - let tabs = open[0]["tabs"].as_array().expect("tabs"); + // The new tab is the active one, after the tab that opened it. The app + // may hear of its address a moment after the bot read the page: the + // events that bring it wait while the new tab is set up. + let deadline = Instant::now() + Duration::from_secs(5); + let tabs = loop { + let open = b.app.call("browser.list", Value::Null).await.expect("list"); + let tabs = open[0]["tabs"].as_array().expect("tabs").clone(); + let shown = tabs.len() == 2 + && tabs[1]["url"] + .as_str() + .is_some_and(|url| url.contains("/done")); + if shown || Instant::now() > deadline { + break tabs; + } + tokio::time::sleep(Duration::from_millis(50)).await; + }; assert_eq!(tabs.len(), 2, "{tabs:?}"); assert_eq!(tabs[0]["active"], false); assert_eq!(tabs[1]["active"], true); - assert!(tabs[1]["url"].as_str().expect("url").contains("/done")); + assert!( + tabs[1]["url"].as_str().expect("url").contains("/done"), + "{tabs:?}" + ); let picture = b.mcp.tool_result("browser_screenshot", json!({})).await; assert_eq!(picture["content"][0]["type"], "image");