Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions docs/docker.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,106 @@ volumes:

The `shm_size` and `seccomp` settings are needed for Chromium to run properly in a container.

### External Browser

Run `chromedp/headless-shell` as a separate container and point Spacebot at it via
`connect_url`. This decouples the browser lifecycle from the main process and avoids
bundling Chromium into the Spacebot image.

Workers spawned by the same agent share one Chrome process (each gets its own tab). A
Workers pointing at the same `connect_url` share one Chrome process (each gets its own tab). A
Chrome crash kills all tabs connected to that browser.
Comment on lines +158 to +160

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a paste/duplication here (extra leading A + repeated sentence). Suggest:

Suggested change
Workers spawned by the same agent share one Chrome process (each gets its own tab). A
Workers pointing at the same `connect_url` share one Chrome process (each gets its own tab). A
Chrome crash kills all tabs connected to that browser.
Workers pointing at the same `connect_url` share one Chrome process (each gets its own tab). A Chrome crash kills all tabs connected to that browser.


#### Spacebot on host, browser in Docker

When Spacebot runs as a binary directly on the host, expose port 9222 so the host process
can reach the container:

```yaml
# docker-compose.yml
services:
browser:
image: chromedp/headless-shell:latest
container_name: browser
ports:
- "127.0.0.1:9222:9222"
shm_size: 1gb
restart: unless-stopped
```

Test whether the browser is reachable from the host:

```bash
curl http://localhost:9222/json/version
```

Then configure Spacebot via config:

```toml
[defaults.browser]
connect_url = "http://localhost:9222"
```
Comment thread
hotzen marked this conversation as resolved.

#### Per-agent dedicated sandboxes

Use a `config.toml` to route each agent to its own container:

```toml
[defaults.browser]
connect_url = "http://browser-main:9222"

[[agents]]
id = "research"
[agents.browser]
connect_url = "http://browser-research:9222"

[[agents]]
id = "internal"
[agents.browser]
enabled = false
```

```yaml
services:
spacebot:
image: ghcr.io/spacedriveapp/spacebot:slim
volumes:
- spacebot-data:/data
- ./config.toml:/data/config.toml:ro
networks:
- spacebot-net

browser-main:
image: chromedp/headless-shell:latest
networks:
- spacebot-net
shm_size: 512mb
restart: unless-stopped

browser-research:
image: chromedp/headless-shell:latest
networks:
- spacebot-net
shm_size: 1gb
restart: unless-stopped

networks:
spacebot-net:

volumes:
spacebot-data:
```

#### `connect_url`

Accepted formats:
- `http://host:9222` — auto-discovers the WebSocket URL via `/json/version` (preferred)
- `ws://host:9222/devtools/browser/<id>` — direct WebSocket URL

An empty string is treated as unset and falls back to the embedded launch path.

If the browser container crashes or the WebSocket drops, the next browser operation returns a clear `"external browser connection lost"` error rather than an opaque protocol failure.

## Building the Image

From the spacebot repo root:
Expand Down
37 changes: 37 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,8 @@ pub struct BrowserConfig {
pub executable_path: Option<String>,
/// Directory for storing screenshots and other browser artifacts.
pub screenshot_dir: Option<PathBuf>,
/// CDP URL of an external browser to connect to instead of launching one locally.
pub connect_url: Option<String>,
Comment thread
hotzen marked this conversation as resolved.
/// Directory for caching a fetcher-downloaded Chromium binary.
/// Populated from `{instance_dir}/chrome_cache` during config resolution.
pub chrome_cache_dir: PathBuf,
Expand All @@ -933,6 +935,7 @@ impl Default for BrowserConfig {
evaluate_enabled: false,
executable_path: None,
screenshot_dir: None,
connect_url: None,
chrome_cache_dir: PathBuf::from("chrome_cache"),
}
}
Expand Down Expand Up @@ -3040,6 +3043,7 @@ struct TomlBrowserConfig {
evaluate_enabled: Option<bool>,
executable_path: Option<String>,
screenshot_dir: Option<String>,
connect_url: Option<String>,
}

#[derive(Deserialize)]
Expand Down Expand Up @@ -4896,6 +4900,7 @@ impl Config {
.map(PathBuf::from)
.or_else(|| base.screenshot_dir.clone()),
chrome_cache_dir: chrome_cache_dir.clone(),
connect_url: b.connect_url.or_else(|| base.connect_url.clone()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Normalize connect_url before merging defaults/overrides.

Line 4848 and Line 5046-5048 preserve empty/whitespace values as Some, which can force a failing external-connect path instead of a clean fallback.

🧹 Proposed fix
+fn normalize_connect_url(value: Option<String>) -> Option<String> {
+    value
+        .map(|url| url.trim().to_string())
+        .filter(|url| !url.is_empty())
+}
@@
-                            connect_url: b.connect_url.or_else(|| base.connect_url.clone()),
+                            connect_url: normalize_connect_url(b.connect_url)
+                                .or_else(|| base.connect_url.clone()),
@@
-                        connect_url: b
-                            .connect_url
-                            .or_else(|| defaults.browser.connect_url.clone()),
+                        connect_url: normalize_connect_url(b.connect_url)
+                            .or_else(|| defaults.browser.connect_url.clone()),

Also applies to: 5046-5048

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/config.rs` at line 4848, When merging connect_url defaults/overrides (the
expression using b.connect_url.or_else(|| base.connect_url.clone())), normalize
both b.connect_url and base.connect_url by trimming whitespace and treating
empty strings as None before merging so that empty/"   " values don't win over a
valid fallback; update the merge logic to map and filter each Option<String> (or
equivalent type) to None if trimmed().is_empty() and only then perform
or_else/or logic for connect_url (apply same normalization where connect_url is
handled at the other location referenced by lines ~5046-5048).

}
})
.unwrap_or_else(|| BrowserConfig {
Expand Down Expand Up @@ -5093,6 +5098,9 @@ impl Config {
.map(PathBuf::from)
.or_else(|| defaults.browser.screenshot_dir.clone()),
chrome_cache_dir: defaults.browser.chrome_cache_dir.clone(),
connect_url: b
.connect_url
.or_else(|| defaults.browser.connect_url.clone()),
}),
mcp: match a.mcp {
Some(mcp_servers) => Some(
Expand Down Expand Up @@ -5618,6 +5626,13 @@ impl Config {
});
}

warn_browser_config("defaults", &defaults.browser);
for agent in &agents {
if let Some(browser) = &agent.browser {
warn_browser_config(&agent.id, browser);
}
}

Ok(Config {
instance_dir,
llm,
Expand Down Expand Up @@ -5891,6 +5906,28 @@ impl std::fmt::Debug for RuntimeConfig {
}
}

/// Warn at config load time about `BrowserConfig` fields that have no effect when
/// `connect_url` is set.
fn warn_browser_config(context: &str, config: &BrowserConfig) {
let Some(url) = config.connect_url.as_deref().filter(|u| !u.is_empty()) else {
return;
};
if config.executable_path.is_some() {
tracing::warn!(
context,
connect_url = url,
"connect_url is set; executable_path has no effect"
);
}
if !config.headless {
tracing::warn!(
context,
connect_url = url,
"connect_url is set; headless flag has no effect"
);
Comment thread
hotzen marked this conversation as resolved.
}
Comment on lines +5915 to +5928

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: logging the full connect_url here can leak credentials if someone uses ws://user:pass@... or query tokens. I'd rather log a boolean (or a redacted host).

Suggested change
if config.executable_path.is_some() {
tracing::warn!(
context,
connect_url = url,
"connect_url is set; executable_path has no effect"
);
}
if !config.headless {
tracing::warn!(
context,
connect_url = url,
"connect_url is set; headless flag has no effect"
);
}
if config.executable_path.is_some() {
tracing::warn!(
context,
connect_url_set = true,
"connect_url is set; executable_path has no effect"
);
}
if !config.headless {
tracing::warn!(
context,
connect_url_set = true,
"connect_url is set; headless flag has no effect"
);
}

}

/// Watches config, prompt, identity, and skill files for changes and triggers
/// hot reload on the corresponding RuntimeConfig.
///
Expand Down
117 changes: 112 additions & 5 deletions src/tools/browser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use std::collections::HashMap;
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
use std::path::{Path, PathBuf};
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use tokio::sync::Mutex;
use tokio::task::JoinHandle;

Expand Down Expand Up @@ -149,6 +150,11 @@ struct BrowserState {
/// Per-launch temp directory for Chrome's user data. Cleaned up on drop to
/// prevent stale singleton locks from blocking subsequent launches.
user_data_dir: Option<PathBuf>,
/// True when connected to an external browser process rather than a locally launched one.
connected: bool,
/// Shared flag set to `false` by the handler task when the WebSocket connection drops.
/// Only meaningful when `connected` is true.
connection_alive: Arc<AtomicBool>,
}

impl Drop for BrowserState {
Expand Down Expand Up @@ -180,6 +186,11 @@ impl std::fmt::Debug for BrowserState {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("BrowserState")
.field("has_browser", &self.browser.is_some())
.field("connected", &self.connected)
.field(
"connection_alive",
&self.connection_alive.load(Ordering::Relaxed),
)
.field("pages", &self.pages.len())
.field("active_target", &self.active_target)
.field("element_refs", &self.element_refs.len())
Expand Down Expand Up @@ -210,6 +221,8 @@ impl BrowserTool {
element_refs: HashMap::new(),
next_ref: 0,
user_data_dir: None,
connected: false,
connection_alive: Arc::new(AtomicBool::new(false)),
})),
config,
screenshot_dir,
Expand Down Expand Up @@ -505,6 +518,58 @@ impl BrowserTool {
}
}

let is_connect = self
.config
.connect_url
.as_deref()
.is_some_and(|url| !url.is_empty());

if is_connect {
self.connect_external().await
} else {
self.launch_local().await
}
}

async fn connect_external(&self) -> Result<BrowserOutput, BrowserError> {
let connect_url = self.config.connect_url.as_deref().unwrap();

tracing::info!(connect_url, "connecting to external browser");

let (browser, mut handler) = Browser::connect(connect_url).await.map_err(|error| {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs mention allowing http://host:9222 by auto-discovering the WebSocket URL via /json/version. If chromiumoxide::Browser::connect doesn't already do that, it might be worth resolving webSocketDebuggerUrl here (or tightening the docs to only claim ws://... support).

BrowserError::new(format!(
"failed to connect to browser at {connect_url}: {error}"
))
})?;

let connection_alive = Arc::new(AtomicBool::new(true));
let alive_flag = connection_alive.clone();
let handler_task = tokio::spawn(async move {
while handler.next().await.is_some() {}
alive_flag.store(false, Ordering::Release);
});

let mut state = self.state.lock().await;

// Guard against a concurrent launch that won the race.
if state.browser.is_some() {
drop(browser);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the external CDP connection drops, launch can still short-circuit on state.browser.is_some(), which makes the recovery message ("reconnect with launch") hard to follow. Consider treating connected && !connection_alive as "not running" and resetting state before returning success.

Suggested change
drop(browser);
{
let mut state = self.state.lock().await;
if state.browser.is_some() {
if state.connected && !state.connection_alive.load(Ordering::Acquire) {
self.reset_state(&mut state).await;
} else {
return Ok(BrowserOutput::success("Browser already running"));
}
}
}

handler_task.abort();
return Ok(BrowserOutput::success("Browser already running"));
}

state.browser = Some(browser);
state._handler_task = Some(handler_task);
state.connected = true;
state.connection_alive = connection_alive;

tracing::info!(connect_url, "connected to external browser");
Ok(BrowserOutput::success(format!(
"Connected to external browser at {connect_url}"
)))
}

async fn launch_local(&self) -> Result<BrowserOutput, BrowserError> {
// Resolve the Chrome executable path (may download ~150MB on first use):
// 1. Explicit config override
// 2. CHROME / CHROME_PATH env vars
Expand Down Expand Up @@ -551,13 +616,15 @@ impl BrowserTool {
// Another call launched while we were downloading/starting. Clean up
// the browser we just created and return success.
drop(browser);
handler_task.abort();
let _ = std::fs::remove_dir_all(&user_data_dir);
return Ok(BrowserOutput::success("Browser already running"));
}

state.browser = Some(browser);
state._handler_task = Some(handler_task);
state.user_data_dir = Some(user_data_dir);
state.connected = false;

tracing::info!("browser launched");
Ok(BrowserOutput::success("Browser launched successfully"))
Expand Down Expand Up @@ -1013,17 +1080,48 @@ impl BrowserTool {
async fn handle_close(&self) -> Result<BrowserOutput, BrowserError> {
let mut state = self.state.lock().await;

if state.connected {
self.disconnect(&mut state).await
} else {
self.close(&mut state).await
}
}

async fn disconnect(&self, state: &mut BrowserState) -> Result<BrowserOutput, BrowserError> {
// Close all pages we opened so they don't linger as orphan tabs in the container.
for (id, page) in state.pages.drain() {
if let Err(error) = page.close().await {
tracing::debug!(target_id = %id, %error, "failed to close page during disconnect");
}
}
// Drop without Browser.close — that CDP command would terminate the external process.
state.browser.take();
self.reset_state(state).await;
tracing::info!("external browser disconnected");
Ok(BrowserOutput::success("Browser disconnected"))
}

async fn close(&self, state: &mut BrowserState) -> Result<BrowserOutput, BrowserError> {
if let Some(mut browser) = state.browser.take()
&& let Err(error) = browser.close().await
{
tracing::warn!(%error, "browser close returned error");
tracing::warn!(%error, "embedded browser close returned error");
}
self.reset_state(state).await;
tracing::info!("embedded browser closed");
Ok(BrowserOutput::success("Browser closed"))
}

async fn reset_state(&self, state: &mut BrowserState) {
state.pages.clear();
state.active_target = None;
state.element_refs.clear();
state.next_ref = 0;
state._handler_task = None;
if let Some(task) = state._handler_task.take() {
task.abort();
}
state.connected = false;
state.connection_alive = Arc::new(AtomicBool::new(false));

// Clean up the per-launch user data dir to free disk space.
if let Some(dir) = state.user_data_dir.take()
Expand All @@ -1035,9 +1133,6 @@ impl BrowserTool {
"failed to clean up browser user data dir"
);
}

tracing::info!("browser closed");
Ok(BrowserOutput::success("Browser closed"))
}

/// Get the active page, or create a first one if the browser has no pages yet.
Expand All @@ -1046,6 +1141,12 @@ impl BrowserTool {
state: &'a mut BrowserState,
url: Option<&str>,
) -> Result<&'a chromiumoxide::Page, BrowserError> {
if state.connected && !state.connection_alive.load(Ordering::Acquire) {
return Err(BrowserError::new(
"external browser connection lost — reconnect with launch",
));
Comment on lines +1144 to +1147

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: right now launch will still report "already running" if the external WebSocket dropped, so the "reconnect with launch" instruction is a bit misleading. Maybe point folks at close + launch (and keep the message consistent in require_active_page too).

Suggested change
if state.connected && !state.connection_alive.load(Ordering::Acquire) {
return Err(BrowserError::new(
"external browser connection lost — reconnect with launch",
));
if state.connected && !state.connection_alive.load(Ordering::Acquire) {
return Err(BrowserError::new(
"external browser connection lost — call close, then launch to reconnect",
));
}

}

if let Some(target) = state.active_target.as_ref()
&& state.pages.contains_key(target)
{
Expand Down Expand Up @@ -1075,6 +1176,12 @@ impl BrowserTool {
&self,
state: &'a BrowserState,
) -> Result<&'a chromiumoxide::Page, BrowserError> {
if state.connected && !state.connection_alive.load(Ordering::Acquire) {
return Err(BrowserError::new(
"external browser connection lost — reconnect with launch",
));
}

let target = state
.active_target
.as_ref()
Expand Down