diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 14cb4ea6c..de3905227 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -462,7 +462,7 @@ Scheduled-task runtime note: - `browser_automation/` - Agentic browser control (stateful `navigate → observe → click/type` loop) via Playwright, distinct from the read-only `web_fetch`/`web_capture` tools. Tools: `browser_navigate`, `browser_snapshot`, `browser_click`, `browser_type`, `browser_get_text`, `browser_back`, `browser_screenshot`, `browser_close` (config `group: browser`). A process-local `BrowserSessionManager` owns one private, loop-affine Playwright event-loop thread (same pattern as the BoxLite provider) so a per-thread browser session survives across turns regardless of the caller's loop (Gateway / TUI / test). Each action returns a fresh page snapshot whose interactive elements are addressed by a stable numeric `[ref]` index (stamped as `data-df-ref` during snapshot), so the model acts on what it just observed instead of holding stale handles or guessing selectors. URLs are SSRF-screened via the shared `validate_public_http_url` (opt-out `allow_private_addresses` only for intentional internal targets). CDP attachment cannot install the request guard on an existing Chrome context, so `cdp_url` fails closed unless the operator explicitly sets `allow_unguarded_cdp: true` for a trusted local browser. Browser REST/Live access also requires an exact non-NULL thread owner, rather than the general legacy shared-thread policy, because retained pages may contain authenticated state. Session admission is a hard `max_sessions` cap: pinned Live/operation sessions are never evicted, and a new thread is rejected when no unpinned session can be closed; one Live viewer owns a session at a time. Optional dependency: `cd backend && uv sync --extra browser && uv run playwright install chromium`; `scripts/detect_uv_extras.py` preserves the extra when `config.yaml` enables `browser_navigate`, and Gateway startup fails fast if configured browser control cannot import Playwright. Tests: `tests/test_browser_automation.py` (mocked tools + a real-Chromium integration test guarded by `importorskip`); `tests/manual_browser_live_check.py` is a manual DeepSeek-driven end-to-end check (not collected by pytest). Live UI input dispatch is kept independent from JPEG capture: non-move actions start a rate-limited background refresh loop, so pointer, wheel, or keyboard input stays responsive while continuous gestures still produce frames throughout the interaction. -Additional providers also live here (`boxlite`, `brave`, `browserless`, `crawl4ai`, `ddg_search`, `e2b_sandbox`, `exa`, `fastcrw`, `groundroute`, `infoquest`, `searxng`, `serper`); see each subpackage for specifics. +Additional providers also live here (`boxlite`, `brave`, `browserless`, `crawl4ai`, `ddg_search`, `e2b_sandbox`, `exa`, `fastcrw`, `groundroute`, `infoquest`, `searxng`, `serper`); see each subpackage for specifics. E2B bootstrap is required. If it fails, the provider kills and closes the unusable remote sandbox. New sandbox creation raises an error. Warm-pool reclaim and remote discovery discard the sandbox and continue acquisition. E2B mounts remain optional. **ACP agent tools**: - `invoke_acp_agent` - Invokes external ACP-compatible agents from `config.yaml` diff --git a/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox_provider.py b/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox_provider.py index ee1dbd8fb..83d805ae2 100644 --- a/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox_provider.py +++ b/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox_provider.py @@ -299,10 +299,8 @@ class E2BSandboxProvider(SandboxProvider): return None self._refresh_remote_timeout(client) - try: - self._bootstrap_sandbox_paths(client) - except Exception as e: - logger.debug("bootstrap on warm-pool reclaim failed: %s", e) + if self._bootstrap_or_discard(client, target_id) is not None: + return None self._register_connected_sandbox(target_id, client, thread_id=thread_id, user_id=user_id) logger.info( "Reclaimed warm-pool e2b sandbox %s for user/thread %s/%s", @@ -415,10 +413,8 @@ class E2BSandboxProvider(SandboxProvider): return None self._refresh_remote_timeout(client) - try: - self._bootstrap_sandbox_paths(client) - except Exception as e: - logger.debug("bootstrap on remote discovery failed: %s", e) + if self._bootstrap_or_discard(client, target_id) is not None: + return None self._register_connected_sandbox(target_id, client, thread_id=thread_id, user_id=user_id) logger.info( "Discovered remote e2b sandbox %s for user/thread %s/%s (seed=%s)", @@ -475,14 +471,9 @@ class E2BSandboxProvider(SandboxProvider): # use the same /mnt/user-data prefix as LocalSandbox / AioSandbox — fail # with PermissionError because /mnt is owned by root in the e2b # template. See the path-mapping note in :class:`E2BSandbox`. - try: - self._bootstrap_sandbox_paths(client) - except Exception as e: - logger.warning( - "Failed to bootstrap virtual paths in e2b sandbox %s: %s", - sandbox_id, - e, - ) + error = self._bootstrap_or_discard(client, sandbox_id) + if error is not None: + raise RuntimeError(f"Failed to bootstrap e2b sandbox {sandbox_id}") from error # One-shot mount uploads. e2b has no host bind-mount, so we copy # files from ``host_path`` into ``container_path`` at sandbox start. @@ -612,6 +603,18 @@ class E2BSandboxProvider(SandboxProvider): logger.debug("e2b client close raised: %s", e) return + def _bootstrap_or_discard(self, client: E2BClientSandbox, sandbox_id: str) -> Exception | None: + """Bootstrap a sandbox or return its error after cleanup.""" + try: + self._bootstrap_sandbox_paths(client) + except Exception as e: + logger.exception("Failed to bootstrap e2b sandbox %s. Discarding the unusable sandbox.", sandbox_id) + if error := self._kill_client(client): + logger.warning("Failed to kill e2b sandbox %s after bootstrap failure: %s", sandbox_id, error) + self._safe_close_client(client) + return e + return None + def _bootstrap_sandbox_paths(self, client: E2BClientSandbox) -> None: """Materialise DeerFlow's virtual path layout inside the e2b VM. @@ -637,11 +640,9 @@ class E2BSandboxProvider(SandboxProvider): writes through the symlink target succeed. The e2b code-interpreter template puts ``user`` in the ``sudo`` group - with passwordless sudo, so the ``sudo`` calls below succeed without - interactive prompts. If the customer template removes that, the - commands fail loudly here and we fall back to silently relying on the - path remap inside ``E2BSandbox`` — agent shell commands will still - fail, but the read/write/list APIs continue to work. + with passwordless sudo. Custom templates must provide equivalent + permissions. Bootstrap failure makes the sandbox unusable. The + provider discards it instead of returning a partially functional VM. """ # Use the configured ``home_dir`` so a custom template can move HOME. home_dir = self._config["home_dir"].rstrip("/") or "/home/user" @@ -669,21 +670,13 @@ class E2BSandboxProvider(SandboxProvider): try: result = client.commands.run(bootstrap_script) except Exception as e: - logger.warning( - "e2b bootstrap script raised: %s (agent shell commands using /mnt/user-data may fail until the VM is recycled)", - e, - ) - return + raise RuntimeError("e2b bootstrap script raised") from e stdout = getattr(result, "stdout", "") or "" stderr = getattr(result, "stderr", "") or "" exit_code = getattr(result, "exit_code", 0) if exit_code not in (0, None) or "BOOTSTRAP_OK" not in stdout: - logger.warning( - "e2b bootstrap script exited with code=%s; stderr=%s", - exit_code, - stderr.strip(), - ) + raise RuntimeError(f"e2b bootstrap script failed with exit code {exit_code}; stderr={stderr.strip()}") def _apply_mounts(self, client: E2BClientSandbox) -> None: mounts = self._config.get("mounts") or [] diff --git a/backend/tests/test_e2b_sandbox_provider.py b/backend/tests/test_e2b_sandbox_provider.py index db0bb09bc..a8026d067 100644 --- a/backend/tests/test_e2b_sandbox_provider.py +++ b/backend/tests/test_e2b_sandbox_provider.py @@ -489,6 +489,29 @@ def test_reclaim_warm_pool_sandbox_handles_reconnect_exception(monkeypatch): assert "sb-broken" not in p._warm_pool +def test_acquire_discards_warm_sandbox_when_bootstrap_fails(monkeypatch): + p = _make_provider() + fake_cls = _install_fake_sdk(monkeypatch, p) + warm_client = FakeClient( + sandbox_id="sb-warm", + commands=FakeCommandsAPI( + [ + SimpleNamespace(stdout="ok", stderr="", exit_code=0), + SimpleNamespace(stdout="", stderr="permission denied", exit_code=1), + ] + ), + ) + fresh_client = FakeClient(sandbox_id="sb-fresh") + fake_cls.connect_factory = lambda _sid, **_kw: warm_client + fake_cls.create_factory = lambda **_kw: fresh_client + p._warm_pool["sb-warm"] = (p._stable_seed("t1", "u1"), 12345.0) + + assert p.acquire("t1", user_id="u1") == "sb-fresh" + assert warm_client.killed is True + assert warm_client.closed is True + assert p.get("sb-warm") is None + + def test_reclaim_warm_pool_sandbox_returns_none_on_seed_mismatch(monkeypatch): p = _make_provider() _install_fake_sdk(monkeypatch, p) @@ -566,6 +589,27 @@ def test_discover_remote_sandbox_skips_dead_candidate(monkeypatch): assert client.closed is True +def test_discover_remote_sandbox_discards_candidate_when_bootstrap_fails(monkeypatch): + p = _make_provider() + fake_cls = _install_fake_sdk(monkeypatch, p) + fake_cls.list_return = [_info("sb-broken", "u1", "t1")] + client = FakeClient( + sandbox_id="sb-broken", + commands=FakeCommandsAPI( + [ + SimpleNamespace(stdout="ok", stderr="", exit_code=0), + SimpleNamespace(stdout="", stderr="permission denied", exit_code=1), + ] + ), + ) + fake_cls.connect_factory = lambda _sid, **_kw: client + + assert p._discover_remote_sandbox("t1", user_id="u1") is None + assert client.killed is True + assert client.closed is True + assert ("u1", "t1") not in p._thread_sandboxes + + def test_kill_client_returns_exception_without_raising(): p = _make_provider() client = FakeClient() @@ -645,14 +689,69 @@ def test_bootstrap_sandbox_paths_emits_expected_script(): assert f"/home/user/{sub}" in script -def test_bootstrap_sandbox_paths_swallows_command_failure(): +def test_bootstrap_sandbox_paths_raises_on_command_failure(): p = _make_provider() def boom(_cmd: str) -> Any: raise RuntimeError("sudo not allowed") client = FakeClient(commands=FakeCommandsAPI([boom])) - p._bootstrap_sandbox_paths(client) + with pytest.raises(RuntimeError, match="bootstrap script raised"): + p._bootstrap_sandbox_paths(client) + + +def test_acquire_cleans_up_and_fails_when_bootstrap_fails(monkeypatch): + provider = _make_provider() + fake_cls = _install_fake_sdk(monkeypatch, provider) + client = FakeClient( + sandbox_id="bootstrap-failure", + commands=FakeCommandsAPI([SimpleNamespace(stdout="", stderr="permission denied", exit_code=1)]), + ) + fake_cls.create_factory = lambda **kwargs: client + + with pytest.raises(RuntimeError, match="bootstrap") as error: + provider.acquire("thread-1", user_id="user-1") + + assert error.value.__cause__ is not None + assert "permission denied" in str(error.value.__cause__) + assert client.killed is True + assert client.closed is True + assert provider.get("bootstrap-failure") is None + + +def test_acquire_rejects_falsey_bootstrap_error(monkeypatch): + class FalseyError(RuntimeError): + def __bool__(self) -> bool: + return False + + provider = _make_provider() + fake_cls = _install_fake_sdk(monkeypatch, provider) + client = FakeClient(sandbox_id="bootstrap-failure") + error = FalseyError("bootstrap failed") + fake_cls.create_factory = lambda **kwargs: client + provider._bootstrap_or_discard = MagicMock(return_value=error) + + with pytest.raises(RuntimeError, match="bootstrap") as caught: + provider.acquire("thread-1", user_id="user-1") + + assert caught.value.__cause__ is error + assert provider.get("bootstrap-failure") is None + + +def test_acquire_closes_client_when_bootstrap_cleanup_kill_fails(monkeypatch): + provider = _make_provider() + fake_cls = _install_fake_sdk(monkeypatch, provider) + client = FakeClient( + sandbox_id="bootstrap-failure", + commands=FakeCommandsAPI([SimpleNamespace(stdout="", stderr="permission denied", exit_code=1)]), + ) + client.kill = MagicMock(side_effect=RuntimeError("sandbox already gone")) + fake_cls.create_factory = lambda **kwargs: client + + with pytest.raises(RuntimeError, match="bootstrap"): + provider.acquire("thread-1", user_id="user-1") + + assert client.closed is True def test_release_unknown_sandbox_id_is_noop():