diff --git a/crates/browser/src/manager.rs b/crates/browser/src/manager.rs index 6ae2fc48e7..ac975ee23c 100644 --- a/crates/browser/src/manager.rs +++ b/crates/browser/src/manager.rs @@ -109,6 +109,12 @@ impl BrowserManager { "executing browser action" ); + // Whether to attach a screenshot of the page state if this action + // fails. Computed before `request.action` is moved into the dispatcher. + // (`request.browser` is `Copy`, so it stays usable in the error arm.) + let want_failure_shot = + self.config.screenshot_each_step && request.action.is_state_changing(); + let start = Instant::now(); let timeout_duration = Duration::from_millis(request.timeout_ms); @@ -140,11 +146,48 @@ impl BrowserManager { ) .increment(1); - BrowserResponse::error( - request.session_id.unwrap_or_default(), + let mut resp = BrowserResponse::error( + request.session_id.clone().unwrap_or_default(), e.to_string(), duration_ms, - ) + ); + + // Best-effort: capture the page state at the moment of + // failure so the step timeline shows *why* it failed, + // not just the error text. Skipped for dead-connection + // errors (the session is gone; a screenshot would spin + // up a blank one) and bounded so a wedged page can't + // hang the error path. + if want_failure_shot + && !e.is_connection_error() + && let Some(sid) = + request.session_id.as_deref().filter(|s| !s.is_empty()) + { + match timeout( + Duration::from_secs(15), + self.screenshot(Some(sid), false, None, sandbox, request.browser), + ) + .await + { + Ok(Ok((_, shot))) => { + if let (Some(data), Some(scale)) = + (shot.screenshot, shot.screenshot_scale) + { + resp = resp.with_screenshot(data, scale); + } + }, + Ok(Err(err)) => debug!( + session_id = sid, + error = %err, + "failure-state screenshot failed; returning error without screenshot" + ), + Err(_) => { + debug!(session_id = sid, "failure-state screenshot timed out") + }, + } + } + + resp }, } }, @@ -214,9 +257,18 @@ impl BrowserManager { // Navigate has its own retry-with-fresh-session logic, so handle it // separately to avoid double-cleanup. if let BrowserAction::Navigate { ref url } = action { - return self.navigate(session_id, url, sandbox, browser).await; + let (sid, response) = self.navigate(session_id, url, sandbox, browser).await?; + let response = if self.config.screenshot_each_step { + self.attach_step_screenshot(&sid, response, sandbox, browser) + .await + } else { + response + }; + return Ok((sid, response)); } + // Capture before `action` is consumed by the match below. + let wants_step_screenshot = self.config.screenshot_each_step && action.is_state_changing(); let action_name = action.to_string(); let result = match action { @@ -254,12 +306,60 @@ impl BrowserManager { }; // Detect stale connections for all non-Navigate actions - match result { + let (sid, response) = match result { Err(ref e) if e.is_connection_error() => { let sid = session_id.unwrap_or("unknown"); - Err(self.cleanup_stale_session(sid, &action_name).await) + return Err(self.cleanup_stale_session(sid, &action_name).await); + }, + other => other?, + }; + + let response = if wants_step_screenshot { + self.attach_step_screenshot(&sid, response, sandbox, browser) + .await + } else { + response + }; + + Ok((sid, response)) + } + + /// Capture a screenshot after a *successful* state-changing action and + /// attach it to the action's response so chat clients can render a per-step + /// timeline. (The failure case is handled in `handle_request`, which has the + /// error response in hand.) + /// + /// Best-effort: if the response already carries a screenshot it is returned + /// unchanged, and any capture error (e.g. renderless browser kinds whose + /// `supports_screenshots()` is false) is ignored so the action never fails + /// because the auto-screenshot failed. + async fn attach_step_screenshot( + &self, + sid: &str, + response: BrowserResponse, + sandbox: bool, + browser: Option, + ) -> BrowserResponse { + if response.screenshot.is_some() { + return response; + } + + match self + .screenshot(Some(sid), false, None, sandbox, browser) + .await + { + Ok((_, shot)) => match (shot.screenshot, shot.screenshot_scale) { + (Some(data), Some(scale)) => response.with_screenshot(data, scale), + _ => response, + }, + Err(e) => { + debug!( + session_id = sid, + error = %e, + "auto step screenshot failed; returning response without screenshot" + ); + response }, - other => other, } } diff --git a/crates/browser/src/types.rs b/crates/browser/src/types.rs index 2d7949e741..4aba4c7405 100644 --- a/crates/browser/src/types.rs +++ b/crates/browser/src/types.rs @@ -78,6 +78,27 @@ fn default_wait_timeout_ms() -> u64 { 30000 } +impl BrowserAction { + /// Whether this action can change the rendered page and is therefore worth + /// auto-capturing a screenshot for. Read-only/meta actions (`Screenshot`, + /// `Snapshot`, `GetUrl`, `GetTitle`, `Close`) return `false`. + #[must_use] + pub fn is_state_changing(&self) -> bool { + matches!( + self, + Self::Navigate { .. } + | Self::Click { .. } + | Self::Type { .. } + | Self::Scroll { .. } + | Self::Evaluate { .. } + | Self::Wait { .. } + | Self::Back + | Self::Forward + | Self::Refresh + ) + } +} + /// Known Chromium-family browser engines we can launch. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -479,6 +500,11 @@ pub struct BrowserConfig { pub host_data_dir: Option, /// Browserless API compatibility mode (`v1` or `v2`). pub browserless_api_version: BrowserlessApiVersion, + /// Automatically capture a screenshot after each state-changing browser + /// action — on both success and failure — and attach it to that action's + /// response. Opt-in (default `false`). Ignored for renderless browser kinds + /// that cannot take screenshots. + pub screenshot_each_step: bool, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] @@ -532,6 +558,7 @@ impl Default for BrowserConfig { container_host: "127.0.0.1".to_string(), host_data_dir: None, browserless_api_version: BrowserlessApiVersion::V1, + screenshot_each_step: false, } } } @@ -582,6 +609,7 @@ impl From<&moltis_config::schema::BrowserConfig> for BrowserConfig { moltis_config::schema::BrowserlessApiVersion::V1 => BrowserlessApiVersion::V1, moltis_config::schema::BrowserlessApiVersion::V2 => BrowserlessApiVersion::V2, }, + screenshot_each_step: cfg.screenshot_each_step, } } } @@ -700,6 +728,66 @@ mod tests { assert!(config.resolved_profile_dir().is_none()); } + #[test] + fn state_changing_actions_request_step_screenshots() { + // State-changing actions opt in to auto-screenshots. + let state_changing = [ + BrowserAction::Navigate { + url: "https://example.com".into(), + }, + BrowserAction::Click { ref_: 1 }, + BrowserAction::Type { + ref_: 1, + text: "hi".into(), + }, + BrowserAction::Scroll { + ref_: None, + x: 0, + y: 100, + }, + BrowserAction::Evaluate { code: "1".into() }, + BrowserAction::Wait { + selector: None, + ref_: None, + timeout_ms: 0, + }, + BrowserAction::Back, + BrowserAction::Forward, + BrowserAction::Refresh, + ]; + for action in &state_changing { + assert!( + action.is_state_changing(), + "{action} should be state-changing" + ); + } + + // Read-only / meta actions are excluded (Screenshot already carries one). + let read_only = [ + BrowserAction::Screenshot { + full_page: false, + highlight_ref: None, + }, + BrowserAction::Snapshot, + BrowserAction::GetUrl, + BrowserAction::GetTitle, + BrowserAction::Close, + ]; + for action in &read_only { + assert!( + !action.is_state_changing(), + "{action} should not be state-changing" + ); + } + } + + #[test] + fn screenshot_each_step_defaults_off() { + // Opt-in: upgrading installs must not silently gain a screenshot + // round-trip after every browser action. + assert!(!BrowserConfig::default().screenshot_each_step); + } + #[test] fn resolved_profile_dir_uses_custom_path() { let config = BrowserConfig { diff --git a/crates/config/src/schema/tools.rs b/crates/config/src/schema/tools.rs index 17d2205a6e..ea0f392405 100644 --- a/crates/config/src/schema/tools.rs +++ b/crates/config/src/schema/tools.rs @@ -494,6 +494,14 @@ pub struct BrowserConfig { /// - "v2": try Browserless v2 paths (`/chrome`, `/chromium`) when needed. #[serde(default = "default_browserless_api_version")] pub browserless_api_version: BrowserlessApiVersion, + /// Automatically capture a screenshot after each state-changing browser + /// action (navigate, click, type, scroll, etc.) — on both success and + /// failure — and attach it to that action's result, so chat clients can + /// show a per-step screenshot timeline. Opt-in (default `false`) to avoid + /// adding a screenshot round-trip to every action on upgrade. Has no effect + /// for renderless browsers that cannot take screenshots. + #[serde(default = "default_screenshot_each_step")] + pub screenshot_each_step: bool, } #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] @@ -524,6 +532,10 @@ const fn default_browserless_api_version() -> BrowserlessApiVersion { BrowserlessApiVersion::V1 } +const fn default_screenshot_each_step() -> bool { + false +} + impl Default for BrowserConfig { fn default() -> Self { Self { @@ -548,6 +560,7 @@ impl Default for BrowserConfig { profile_dir: None, container_host: default_container_host(), browserless_api_version: default_browserless_api_version(), + screenshot_each_step: default_screenshot_each_step(), } } } diff --git a/crates/config/src/template.rs b/crates/config/src/template.rs index 8320fd737f..3081d16700 100644 --- a/crates/config/src/template.rs +++ b/crates/config/src/template.rs @@ -506,6 +506,7 @@ port = {port} # Port number (auto-generated for this i # device_scale_factor = 2.0 # HiDPI/Retina scaling # max_instances = 3 # Maximum concurrent browser instances # idle_timeout_secs = 300 # Close idle browsers after this many seconds +# screenshot_each_step = false # Set true to auto-capture a screenshot after each action, incl. failures (per-step timeline) # sandbox = false # Run browser in container for isolation # allowed_domains = [] # Domain restrictions (empty = all allowed) # chrome_path = "/path/to/chrome" # Custom Chrome binary path diff --git a/crates/config/src/validate/schema_map.rs b/crates/config/src/validate/schema_map.rs index b5db4d5f99..73ef35b3f0 100644 --- a/crates/config/src/validate/schema_map.rs +++ b/crates/config/src/validate/schema_map.rs @@ -189,6 +189,7 @@ pub(super) fn build_schema_map() -> KnownKeys { ("profile_dir", Leaf), ("container_host", Leaf), ("browserless_api_version", Leaf), + ("screenshot_each_step", Leaf), ])) }; diff --git a/docs/src/browser-automation.md b/docs/src/browser-automation.md index 228eff6782..d211aa74f3 100644 --- a/docs/src/browser-automation.md +++ b/docs/src/browser-automation.md @@ -51,6 +51,7 @@ headless = true # Run without visible window (default) viewport_width = 2560 # Default viewport width viewport_height = 1440 # Default viewport height device_scale_factor = 2.0 # HiDPI/Retina scaling (1.0 = standard, 2.0 = Retina) +screenshot_each_step = false # Opt-in: auto-capture a screenshot after each action, incl. failures (per-step timeline) # Pool management max_instances = 0 # 0 = unlimited (limited by memory), >0 = hard limit diff --git a/docs/src/configuration-reference.md b/docs/src/configuration-reference.md index e3b4cb0146..e0d60acdc6 100644 --- a/docs/src/configuration-reference.md +++ b/docs/src/configuration-reference.md @@ -491,6 +491,7 @@ Default `tool_overrides` entries: | `profile_dir` | optional string | `null` | Custom path for persistent Chrome profile directory. Implies `persist_profile = true`. | | `container_host` | string | `"127.0.0.1"` | Hostname/IP to connect to the browser container from the host. Use `"host.docker.internal"` when Moltis runs inside Docker. | | `browserless_api_version` | enum: `"v1"`, `"v2"` | `"v1"` | Browserless API compatibility mode for websocket endpoints. | +| `screenshot_each_step` | bool | `false` | Opt-in: auto-capture a screenshot after each state-changing action (navigate, click, type, scroll, …) — on both success and failure — and attach it to that action's result, enabling a per-step screenshot timeline in chat clients. Ignored by renderless browsers. | ---