From bfcad98b7683a95b1b89fb3a051d0dd1fa4e81bf Mon Sep 17 00:00:00 2001 From: ciregenz Date: Tue, 9 Jun 2026 02:05:26 -0700 Subject: [PATCH] [eric] browser: BrowserSaveData writes a big page dataset to one sandboxed file, ending the dispatch-per-chunk waste --- backend/apps/agents/browser/browser_agent.py | 38 ++++++++++ backend/apps/agents/browser/browser_save.py | 65 ++++++++++++++++ backend/apps/agents/browser/browser_schema.py | 43 ++++++++++- backend/tests/test_browser_agent_loop.py | 19 ++++- backend/tests/test_browser_save.py | 74 +++++++++++++++++++ 5 files changed, 236 insertions(+), 3 deletions(-) create mode 100644 backend/apps/agents/browser/browser_save.py create mode 100644 backend/tests/test_browser_save.py diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 36a4b9c6..fafb1a9a 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -63,6 +63,7 @@ from backend.apps.agents.browser import browser_batch_replay from backend.apps.agents.browser import browser_extract from backend.apps.agents.browser import browser_metrics from backend.apps.agents.browser import browser_playbook +from backend.apps.agents.browser import browser_save from backend.apps.agents.browser import browser_meta_playbook from backend.apps.agents.browser import browser_skills from backend.apps.agents.browser import browser_wait @@ -1319,6 +1320,43 @@ async def run_browser_agent( }) continue + # Bulk-data sink: write a page-assembled dataset straight to a file so a + # big list never has to squeeze through the (truncating) reply. The JS runs + # in the page, but Python picks the path, so the write stays sandboxed. + if tu.name == "BrowserSaveData": + st = time.time() + _expr = tu.input.get("expression") or "" + _fname = tu.input.get("filename") or "" + if not _expr: + sv_text, sv_ok = "BrowserSaveData needs a JS `expression` that returns the data (usually JSON.stringify(...)).", False + else: + _ev = await _cancellable(execute_browser_tool("BrowserEvaluate", {"expression": _expr}, browser_id, tab_id)) + if _ev is None: + cancelled = True + break + if isinstance(_ev, dict) and _ev.get("error"): + sv_text, sv_ok = f"Couldn't read the data to save: {_ev['error']}", False + else: + _cwd = None + if parent_session_id: + _ps = agent_manager.get_session(parent_session_id) + _cwd = getattr(_ps, "cwd", None) if _ps else None + sv_text = browser_save.save_page_data( + _cwd, parent_session_id or session_id, _fname, str((_ev or {}).get("text") or "")) + sv_ok = sv_text.startswith("Saved") + action_log.append({ + "tool": "BrowserSaveData", "input": {"filename": _fname}, + "result_summary": sv_text[:200], + "elapsed_ms": int((time.time() - st) * 1000), "ok": sv_ok, + }) + tool_results.append({"type": "tool_result", "tool_use_id": tu.id, "content": [{"type": "text", "text": sv_text}]}) + result_msg = Message(role="tool_result", content={"text": sv_text, "tool_name": tu.name, "elapsed_ms": int((time.time() - st) * 1000)}) + session.messages.append(result_msg) + await ws_manager.send_to_session(session_id, "agent:message", { + "session_id": session_id, "message": result_msg.model_dump(mode="json"), + }) + continue + # Intra-run batch replay: run a learned mechanical flow for many # inputs at machine speed, verify every step, gate sends, never # ghost. Reads/searches loop freely; irreversible steps refuse. diff --git a/backend/apps/agents/browser/browser_save.py b/backend/apps/agents/browser/browser_save.py new file mode 100644 index 00000000..a962dfbd --- /dev/null +++ b/backend/apps/agents/browser/browser_save.py @@ -0,0 +1,65 @@ +"""Persist page-scraped data straight to a workspace file. + +A big list (every comment, all N results) can't fit through the browser agent's +length-capped reply, so without this the agent burns a dispatch per 100-row chunk +just to funnel an array it already has back out. Here the page produces the +CONTENT (a JS expression) and Python owns the DESTINATION: a hostile or buggy page +can pick what to write but never WHERE, so the write stays sandboxed to one +workspace subdir. Returns a short receipt for the model, never the data itself. +""" +import json +import os + +_MAX_BYTES = 25 * 1024 * 1024 # a page can't realistically hold more scraped data +_ALLOWED_EXT = {".json", ".ndjson", ".csv", ".tsv", ".txt", ".md"} +_SUBDIR = "browser-data" # never the workspace root, so we can't clobber project files + + +def _dest_dir(cwd: str | None, session_id: str) -> str: + base = cwd if (cwd and os.path.isdir(cwd)) else os.path.join( + os.path.expanduser("~"), ".openswarm", "workspaces", session_id or "browser") + dest = os.path.join(base, _SUBDIR) + os.makedirs(dest, exist_ok=True) + return dest + + +def save_page_data(cwd: str | None, session_id: str, filename: str, content: str) -> str: + """Write `content` to a sandboxed data file; return a one-line receipt for the + model (the path + size, never the data). Every rejection is a plain message, + this never raises into the agent loop.""" + name = os.path.basename((filename or "").strip()) # strips any dir parts / .. / abs path + if not name: + return "Save failed: give a plain filename like results.json." + ext = os.path.splitext(name)[1].lower() + if ext not in _ALLOWED_EXT: + return (f"Save failed: '{ext or 'no extension'}' isn't allowed; this tool is for data, " + f"not code. Use one of: {', '.join(sorted(_ALLOWED_EXT))}.") + body = content or "" + if len(body.encode("utf-8", "ignore")) > _MAX_BYTES: + return f"Save failed: that's over the {_MAX_BYTES // (1024 * 1024)}MB cap; save fewer fields or rows." + + try: + dest_dir = _dest_dir(cwd, session_id) + dest_real = os.path.realpath(dest_dir) + full = os.path.realpath(os.path.join(dest_dir, name)) + # realpath + os.sep guard: defeats traversal, absolute paths, symlinks, AND a + # prefix-collision sibling (browser-data vs browser-data-evil). basename already + # neutralizes most of it; this is the belt to that suspenders. + if full != dest_real and not full.startswith(dest_real + os.sep): + return "Save failed: that filename escapes the workspace; use a plain name." + with open(full, "w", encoding="utf-8") as f: + f.write(body) + except Exception as e: + return f"Save failed: {type(e).__name__}." + + # best-effort item count so the receipt proves real data landed (not an empty array) + note = "" + try: + parsed = json.loads(body) + if isinstance(parsed, list): + note = f", {len(parsed)} items" + elif isinstance(parsed, dict): + note = f", {len(parsed)} keys" + except Exception: + pass + return f"Saved {len(body):,} chars{note} to {full}" diff --git a/backend/apps/agents/browser/browser_schema.py b/backend/apps/agents/browser/browser_schema.py index 44a66c35..ddbe1803 100644 --- a/backend/apps/agents/browser/browser_schema.py +++ b/backend/apps/agents/browser/browser_schema.py @@ -185,6 +185,40 @@ BROWSER_TOOLS_SCHEMA = [ "required": ["instruction"], }, }, + { + "name": "BrowserSaveData", + "description": ( + "Save a LARGE dataset you've assembled on the page straight to a file. " + "Use this for 'every comment / all N results / the full list' once you've " + "collected it into a page variable: trying to return hundreds of rows in your " + "reply TRUNCATES, so you'd otherwise waste turns chunking it. Give a JS " + "expression that returns the data as a string (almost always " + "JSON.stringify(window.__yourVar)) plus a filename; the whole dataset is " + "written to the workspace and you get back just the file path, not the data. " + "It only writes your own workspace file (never the web). Put the returned path " + "in your Done message so the user knows where it landed." + ), + "input_schema": { + "type": "object", + "properties": { + "expression": { + "type": "string", + "description": ( + "JS that returns the data as a string, e.g. " + "JSON.stringify(window.__rows) or a CSV string you build." + ), + }, + "filename": { + "type": "string", + "description": ( + "Plain data filename, e.g. results.json or comments.csv " + "(allowed: .json .ndjson .csv .tsv .txt .md)." + ), + }, + }, + "required": ["expression", "filename"], + }, + }, { "name": "BrowserGetConsole", "description": ( @@ -828,8 +862,13 @@ SYSTEM_PROMPT = ( "- Spanning MANY pages (get all N, every result)? Confirm the page shape ONCE, then COMMIT " "to the sweep: don't re-verify each page works. The site usually exposes far fewer than a " "round number asks (a '1000' is often ~15 pages); gather every page it does expose, then " - "Done with the full set and a one-line note on the real ceiling. Accumulate as you go so a " - "wrap-up nudge can always answer from what you already have.\n\n" + "Done with the full set and a one-line note on the real ceiling. Accumulate into a page " + "variable as you go (e.g. window.__rows) so nothing is lost between pages.\n" + "- A BIG result set (hundreds of rows) does NOT fit in your reply, it truncates. Do NOT " + "chunk it back through your messages 100 at a time. Once it's gathered into a page variable, " + "call BrowserSaveData('JSON.stringify(window.__rows)', 'results.json') ONCE: it writes the " + "whole thing to a file and hands you the path. Then Done, telling the user that path. That " + "is one step instead of a dozen.\n\n" "## When you genuinely cannot proceed\n" "Use RequestHumanIntervention for:\n" diff --git a/backend/tests/test_browser_agent_loop.py b/backend/tests/test_browser_agent_loop.py index 98d6372a..b9f1f662 100644 --- a/backend/tests/test_browser_agent_loop.py +++ b/backend/tests/test_browser_agent_loop.py @@ -307,7 +307,24 @@ def test_send_shortcut_does_not_arm_on_a_gather_task(monkeypatch): assert "went through" not in result["summary"] -def test_gather_pulling_new_data_each_turn_is_not_nudged_early(monkeypatch): +def test_browser_save_data_writes_a_file_and_returns_a_receipt(monkeypatch, tmp_path): + # BrowserSaveData should run the JS, write the result to a sandboxed file, and + # return a path receipt (NOT the data), so a big list lands in one step instead + # of a dozen reply-chunks. The mock's evaluate echoes its expression as the data. + import os as _os + monkeypatch.setattr(_os.path, "expanduser", lambda p: str(tmp_path)) # fallback workspace -> tmp + BH._browser_history.clear(); BH._domain_notes.clear() + primary = FakeLLM([ + Resp([_rp("save the rows"), _tu("BrowserSaveData", expression="JSON.stringify(window.__rows)", filename="rows.json")]), + Resp([_tu("Done", message="Saved the full set to rows.json.")]), + ]) + aux = FakeAux() + _install(monkeypatch, primary, aux) + result = asyncio.run(BA.run_browser_agent(task="get every row and save it", browser_id="b1", model="sonnet")) + # the file exists under the sandbox subdir, and the receipt (a tool_result) named a path + saved = list(tmp_path.glob("**/browser-data/rows.json")) + assert saved, "BrowserSaveData did not write the file" + assert result.get("done") is True # The Airbnb regression: a page-by-page gather (a fresh Extract returning NEW # listings every turn) must NOT trip the spin backstop, gathering is the work, # not spinning. Here 9 straight Extract turns each return distinct data; the run diff --git a/backend/tests/test_browser_save.py b/backend/tests/test_browser_save.py new file mode 100644 index 00000000..ac6dbf5b --- /dev/null +++ b/backend/tests/test_browser_save.py @@ -0,0 +1,74 @@ +"""Sandbox tests for BrowserSaveData's file sink. This is the security surface +(page-derived content, page-derived filename), so the path-confinement, size cap, +and extension allowlist are tested hard, including hostile filenames.""" +import json +import os + +from backend.apps.agents.browser.browser_save import save_page_data, _ALLOWED_EXT, _MAX_BYTES, _SUBDIR + + +def test_happy_path_writes_into_browser_data_subdir(tmp_path): + payload = json.dumps([{"n": "a"}, {"n": "b"}, {"n": "c"}]) + msg = save_page_data(str(tmp_path), "sid", "rows.json", payload) + assert msg.startswith("Saved") + assert "3 items" in msg + out = tmp_path / _SUBDIR / "rows.json" + assert out.is_file() + assert json.loads(out.read_text()) == json.loads(payload) + + +def test_dict_payload_reports_key_count(tmp_path): + msg = save_page_data(str(tmp_path), "sid", "obj.json", json.dumps({"a": 1, "b": 2})) + assert "2 keys" in msg + + +def test_traversal_filename_is_confined_not_escaped(tmp_path): + # a '../../evil.json' must NOT land outside the sandbox; basename flattens it + msg = save_page_data(str(tmp_path), "sid", "../../evil.json", "[]") + assert msg.startswith("Saved") + assert (tmp_path / _SUBDIR / "evil.json").is_file() + # nothing was written two levels up + assert not (tmp_path.parent.parent / "evil.json").exists() + + +def test_absolute_path_filename_is_confined(tmp_path): + msg = save_page_data(str(tmp_path), "sid", "/etc/evil.json", "[]") + assert msg.startswith("Saved") + assert (tmp_path / _SUBDIR / "evil.json").is_file() + assert not os.path.exists("/etc/evil.json") + + +def test_disallowed_extension_is_rejected(tmp_path): + for bad in ("hack.sh", "x.js", "y.py", "noext"): + msg = save_page_data(str(tmp_path), "sid", bad, "data") + assert msg.startswith("Save failed"), bad + # the allowed ones all pass + for good in sorted(_ALLOWED_EXT): + msg = save_page_data(str(tmp_path), "sid", f"file{good}", "x") + assert msg.startswith("Saved"), good + + +def test_empty_filename_is_rejected(tmp_path): + assert save_page_data(str(tmp_path), "sid", "", "data").startswith("Save failed") + assert save_page_data(str(tmp_path), "sid", " ", "data").startswith("Save failed") + + +def test_oversize_payload_is_rejected(tmp_path): + big = "x" * (_MAX_BYTES + 1) + msg = save_page_data(str(tmp_path), "sid", "big.txt", big) + assert msg.startswith("Save failed") + assert not (tmp_path / _SUBDIR / "big.txt").exists() + + +def test_falls_back_to_home_workspace_when_no_cwd(tmp_path, monkeypatch): + monkeypatch.setattr(os.path, "expanduser", lambda p: str(tmp_path)) + msg = save_page_data(None, "sess-xyz", "f.json", "[]") + assert msg.startswith("Saved") + assert (tmp_path / ".openswarm" / "workspaces" / "sess-xyz" / _SUBDIR / "f.json").is_file() + + +def test_non_json_content_still_saves_without_count(tmp_path): + msg = save_page_data(str(tmp_path), "sid", "notes.txt", "just some text") + assert msg.startswith("Saved") + assert "items" not in msg and "keys" not in msg + assert (tmp_path / _SUBDIR / "notes.txt").read_text() == "just some text"