fix(control-hub): make browser.wait actually wait

`browser.wait` read only `duration_ms`, so the very plausible
`{ "ms": 1800000 }` was dropped and the call fell through to a branch that
returned `{ success: true }` instantly. An agent asked to pause 30 minutes
got "Wait completed" back in milliseconds and moved straight on. The
action documented no parameters at all, so the model had to guess the key,
and even a correct guess was silently capped at 30 seconds.

- Accept the spellings models emit: `duration_ms` / `ms` / `wait_ms` /
  `sleep_ms`, plus `seconds` / `secs` variants, numeric strings included.
- Reject a `wait` carrying neither duration nor condition with
  INVALID_PARAMS instead of reporting a success that never waited.
- Raise the cap to 60 minutes, in step with AgentWait's MAX_TIMEOUT_MS, and
  report `ms` / `requested_ms` / `clamped` so a shortened wait says so.
- Race the sleep against the turn's cancellation token, and stop
  `call_impl` from folding Cancelled into an `ok: false` envelope — a stop
  during a long pause must not wait out the pause, nor look like a tool
  error the model tries to recover from.
- Serve duration waits before session resolution: a pure pause touches no
  page, and agents pace themselves long before they open a browser.
- Resolve `{ condition, timeout_ms }`: the condition always wins and any
  duration bounds it, rather than sleeping and never looking at the page.
  Condition waits keep their previous 15s default, now configurable.
- Document all of it in the tool description, and point repeating schedules
  at the Cron tool, which ends the turn instead of pinning it open.

Author: bobleer

src/crates/assembly/core/src/agentic/tools/browser_control/actions.rs

@@ -7,6 +7,16 @@ use serde_json::{json, Value};
 use std::collections::BTreeMap;
 use tokio::sync::broadcast;
 
+/// Upper bound for an explicit `wait` duration. Pacing waits ("check again in
+/// 30 minutes") are a legitimate agent pattern, so the ceiling is generous;
+/// it exists only so a nonsense duration cannot wedge the session forever.
+/// Kept in step with `AgentWaitTool::MAX_TIMEOUT_MS`.
+pub const MAX_WAIT_MS: u64 = 60 * 60 * 1_000;
+
+/// How long a `wait { condition }` runs before giving up when the caller does
+/// not say. Matches the previous hard-coded lifecycle and selector budgets.
+pub const DEFAULT_CONDITION_TIMEOUT_MS: u64 = 15_000;
+
 /// Result of waiting for a CDP `Page.lifecycleEvent`.
 enum LifecycleOutcome {
     /// One of the requested lifecycle names fired in time. Carries the name
@@ -1054,17 +1064,35 @@ impl<'a> BrowserActions<'a> {
     }
 
     /// Wait for a duration or a condition.
+    ///
+    /// Callers that can observe cancellation should sleep themselves rather
+    /// than routing a plain duration through here — see ControlHub's
+    /// `browser.wait`, which owns the cancellable, session-free duration path.
+    ///
+    /// `condition_timeout_ms` bounds the condition wait; it defaults to
+    /// [`DEFAULT_CONDITION_TIMEOUT_MS`] and is ignored for duration waits.
     pub async fn wait(
         &self,
         duration_ms: Option<u64>,
         condition: Option<&str>,
+        condition_timeout_ms: Option<u64>,
     ) -> BitFunResult<Value> {
         if let Some(ms) = duration_ms {
-            let clamped = ms.min(30_000);
+            let clamped = ms.min(MAX_WAIT_MS);
             tokio::time::sleep(std::time::Duration::from_millis(clamped)).await;
-            return Ok(json!({ "success": true, "action": "wait", "ms": clamped }));
+            return Ok(json!({
+                "success": true,
+                "action": "wait",
+                "ms": clamped,
+                "requested_ms": ms,
+                "clamped": clamped != ms,
+            }));
         }
         if let Some(cond) = condition {
+            let timeout_ms = condition_timeout_ms
+                .filter(|ms| *ms > 0)
+                .unwrap_or(DEFAULT_CONDITION_TIMEOUT_MS)
+                .min(MAX_WAIT_MS);
             match cond {
                 "networkidle" | "load" | "domcontentloaded" => {
                     // Phase 1: replace the previous "sleep 2s and hope" with
@@ -1085,7 +1113,7 @@ impl<'a> BrowserActions<'a> {
                         "domcontentloaded" => &["DOMContentLoaded", "load"],
                         _ => &["load"],
                     };
-                    let outcome = wait_for_lifecycle(&mut events, None, wanted, 15_000).await;
+                    let outcome = wait_for_lifecycle(&mut events, None, wanted, timeout_ms).await;
                     let (success, lifecycle_event, timed_out) = match outcome {
                         LifecycleOutcome::Reached(n) => (true, Some(n), false),
                         LifecycleOutcome::Timeout => (false, None, true),
@@ -1097,23 +1125,39 @@ impl<'a> BrowserActions<'a> {
                         "condition": cond,
                         "lifecycle_event": lifecycle_event,
                         "timed_out": timed_out,
+                        "timeout_ms": timeout_ms,
                     }));
                 }
                 selector => {
+                    const POLL_INTERVAL_MS: u64 = 500;
                     let js = Self::element_exists_js(selector);
-                    for _ in 0..30 {
+                    let deadline =
+                        tokio::time::Instant::now() + std::time::Duration::from_millis(timeout_ms);
+                    loop {
                         let result = self.evaluate(&js).await?;
                         let found = result
                             .get("result")
                             .and_then(|r| r.get("value"))
                             .and_then(|v| v.as_bool())
                             .unwrap_or(false);
                         if found {
-                            return Ok(
-                                json!({ "success": true, "action": "wait", "condition": cond }),
-                            );
+                            return Ok(json!({
+                                "success": true,
+                                "action": "wait",
+                                "condition": cond,
+                                "timeout_ms": timeout_ms,
+                            }));
                         }
-                        tokio::time::sleep(std::time::Duration::from_millis(500)).await;
+                        let remaining = deadline
+                            .saturating_duration_since(tokio::time::Instant::now())
+                            .as_millis() as u64;
+                        if remaining == 0 {
+                            break;
+                        }
+                        tokio::time::sleep(std::time::Duration::from_millis(
+                            remaining.min(POLL_INTERVAL_MS),
+                        ))
+                        .await;
                     }
                     return Err(structured_error(
                         ErrorCode::Timeout,
@@ -1123,7 +1167,18 @@ impl<'a> BrowserActions<'a> {
                 }
             }
         }
-        Ok(json!({ "success": true, "action": "wait" }))
+        // No duration and no condition: there is nothing to wait for. Reporting
+        // success here used to make a mis-keyed duration (`ms` instead of
+        // `duration_ms`) look like a completed wait that in fact returned
+        // instantly, so the agent silently skipped its pause.
+        Err(structured_error(
+            ErrorCode::InvalidParams,
+            "wait requires a duration or a condition",
+            &[
+                "Pass `duration_ms` (alias `ms`) to pause, e.g. { \"duration_ms\": 1800000 } for 30 minutes",
+                "Or pass `condition`: 'load' | 'domcontentloaded' | 'networkidle' | a CSS/@ref selector",
+            ],
+        ))
     }
 
     // ── Capture ────────────────────────────────────────────────────────