From c0c15b4c9c2f7f97fae77ff52c9e675ee9020727 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Tue, 28 Jul 2026 12:02:50 -0700 Subject: [PATCH] [eric] browser: every tier emits the same auditable trace, so the bubble never expands to nothing --- backend/apps/agents/browser/browser_trace.py | 160 ++++++++++++++++++ .../agents/manager/run_browser_fast_path.py | 22 ++- backend/tests/test_browser_trace.py | 127 ++++++++++++++ 3 files changed, 307 insertions(+), 2 deletions(-) create mode 100644 backend/apps/agents/browser/browser_trace.py create mode 100644 backend/tests/test_browser_trace.py diff --git a/backend/apps/agents/browser/browser_trace.py b/backend/apps/agents/browser/browser_trace.py new file mode 100644 index 00000000..35b4c551 --- /dev/null +++ b/backend/apps/agents/browser/browser_trace.py @@ -0,0 +1,160 @@ +"""One auditable record of what the browser actually did, whatever tier did it. + +The user-facing promise is that a browser task is never "just trust me": the chat shows a Browser +Agent bubble you can expand to see the pages visited, what was clicked and typed, and the receipt +that proves a write landed. That promise held only on the sub-agent path, because the panel that +renders it reads from CHILD SESSIONS. The fast path creates no child session and closed its bubble +with a tool_result of literally "done", so on the tier that now handles most tasks the bubble +expanded to nothing at all. + +So the trace stops being a side effect of how the work was routed. Whichever tier ran builds the +same record here, and the bubble shows the same thing every time. + +Pure formatting: no I/O, no side effects, nothing that can fail a run. +""" + +import json +from typing import Any, Dict, List, Optional + +from pydantic import BaseModel, ConfigDict +from typeguard import typechecked + +# Enough to see what happened without turning the panel into a log file. A run that exceeds this +# says so rather than silently showing a prefix, because a trace you cannot trust to be complete is +# worse than no trace. +MAX_STEPS = 40 +MAX_ARG_CHARS = 90 + +# Tools whose arguments are the interesting part (where it went, what it typed) versus ones whose +# name already says everything (a screenshot is a screenshot). +P_ARG_KEYS = ("url", "text", "expression", "instruction", "target_text", "index", "name", "key") + + +class BrowserTrace(BaseModel): + """What to show under the bubble. Shaped so the renderer never parses prose.""" + + model_config = ConfigDict(validate_assignment=True) + + tier: str = "" # which path did the work, in plain words + pages: List[str] = [] # URLs actually visited, in order, deduped + steps: List[str] = [] # one line per action, already human-readable + steps_omitted: int = 0 + receipt: str = "" # the proof a write landed, when there was one + note: str = "" # anything the user should know about coverage + + +@typechecked +def p_arg_summary(inp: Any) -> str: + """The part of a tool's input worth showing, short enough to scan.""" + if not isinstance(inp, dict) or not inp: + return "" + for k in P_ARG_KEYS: + v = inp.get(k) + if v not in (None, "", []): + s = str(v).replace("\n", " ").strip() + return s[:MAX_ARG_CHARS] + ("..." if len(s) > MAX_ARG_CHARS else "") + s = json.dumps(inp)[:MAX_ARG_CHARS] + return s + + +@typechecked +def p_step_line(entry: Dict[str, Any]) -> str: + tool = str(entry.get("tool") or "?") + arg = p_arg_summary(entry.get("input")) + ms = entry.get("elapsed_ms") + ok = entry.get("ok") + tail = f" [{int(ms)}ms]" if isinstance(ms, (int, float)) and ms else "" + mark = "" if ok in (None, True) else " (failed)" + return f"{tool}({arg}){tail}{mark}" if arg else f"{tool}{tail}{mark}" + + +@typechecked +def p_pages_from(action_log: List[Dict[str, Any]]) -> List[str]: + """Every URL the run actually landed on, in order, without repeats. This is the spine of the + trace: it answers "where did it go" before "what did it do there".""" + out: List[str] = [] + for e in action_log: + inp = e.get("input") + url = str(inp.get("url") or "") if isinstance(inp, dict) else "" + if url.startswith(("http://", "https://")) and (not out or out[-1] != url): + out.append(url) + return out + + +@typechecked +def build_trace(tier: str, action_logs: List[List[Dict[str, Any]]], + receipt: str = "", note: str = "", entry_url: str = "") -> BrowserTrace: + """Fold every dispatch a run made into one record. Takes a LIST of logs because a fast-path run + can dispatch more than once (a recovery, a send probe) and the user should see all of it, not + just whichever attempt happened to be last. + + `entry_url` matters more than it looks: a cold run creates the card ALREADY pointed at its + target, so no BrowserNavigate is ever issued and harvesting URLs from the log alone leaves the + trace unable to answer "where did it go" at all.""" + merged: List[Dict[str, Any]] = [] + for log in action_logs: + merged.extend(e for e in (log or []) if isinstance(e, dict)) + pages = p_pages_from(merged) + if entry_url.startswith(("http://", "https://")) and entry_url not in pages[:1]: + pages = [entry_url] + pages + shown = merged[-MAX_STEPS:] + return BrowserTrace( + tier=tier, + pages=pages, + steps=[p_step_line(e) for e in shown], + steps_omitted=max(0, len(merged) - len(shown)), + receipt=receipt, + note=note, + ) + + +@typechecked +def trace_payload(trace: BrowserTrace) -> Dict[str, object]: + """The tool_result content the bubble renders. Kept as data rather than a rendered string so the + panel can lay it out, and so a future surface (an export, a report) does not have to re-parse + English.""" + return {"browser_trace": trace.model_dump(mode="json")} + + +@typechecked +def trace_text(trace: BrowserTrace) -> str: + """A plain-text fallback for anywhere that can only show a string.""" + lines: List[str] = [] + if trace.tier: + lines.append(f"Handled by: {trace.tier}") + if trace.pages: + lines.append("Pages: " + " -> ".join(trace.pages[:6])) + if trace.steps_omitted: + lines.append(f"... {trace.steps_omitted} earlier steps omitted ...") + lines.extend(f"{i}. {s}" for i, s in enumerate(trace.steps, trace.steps_omitted + 1)) + if trace.receipt: + lines.append(f"Verified: {trace.receipt}") + if trace.note: + lines.append(trace.note) + return "\n".join(lines) or "No browser actions were recorded." + + +@typechecked +def tier_label(fp_path: str, used_browser: bool) -> str: + """Plain words for the routing string the logs use, because 'read->browser' means nothing to + the person reading their own chat.""" + if not used_browser: + return "read the page directly, no browser needed" + if fp_path.startswith("read"): + return "opened the page in a browser and read it" + return "drove the browser" + + +@typechecked +def receipt_from(result: Optional[Dict[str, Any]]) -> str: + """The two-sided receipt, when the run produced one. This is the line that separates 'it says it + posted' from 'it posted', so it gets its own field rather than being buried in the steps.""" + if not isinstance(result, dict): + return "" + for key in ("receipt", "sent_receipt", "delivery"): + v = result.get(key) + if isinstance(v, str) and v.strip(): + return v.strip()[:300] + if v is True: + return "delivery confirmed on the page" + return "" diff --git a/backend/apps/agents/manager/run_browser_fast_path.py b/backend/apps/agents/manager/run_browser_fast_path.py index 5e24a2cc..d2cdc82f 100644 --- a/backend/apps/agents/manager/run_browser_fast_path.py +++ b/backend/apps/agents/manager/run_browser_fast_path.py @@ -40,6 +40,8 @@ async def run_browser_fast_path( # The fast-path skips the orchestrator, so the UI never gets the BrowserAgent tool-call that draws the "Browser Agent" bubble. Emit a synthetic tool_call/ tool_result pair (same shape + mcp__ name the orchestrator uses) so the bubble shows here too. None until we actually dispatch a browser (a pure READ answer has no browser, so no bubble). p_browser_tool = "mcp__openswarm-browser-agent__CreateBrowserAgent" p_bubble_tid: Optional[str] = None + p_action_logs: List[List[Dict[str, object]]] = [] + p_last_result: Dict[str, object] = {} try: from backend.apps.agents.browser.browser_agent import run_browser_agents from backend.apps.agents.browser import browser_fast_path @@ -76,7 +78,11 @@ async def run_browser_fast_path( parent_session_id=session_id, ) r = results[0] if results else {} - return r if isinstance(r, dict) else {"summary": str(r or ""), "action_log": []} + r = r if isinstance(r, dict) else {"summary": str(r or ""), "action_log": []} + # Keep EVERY dispatch's actions, not just the last: a run that needed a recovery or a + # send probe did that work on the user's behalf and the trace has to show it. + p_action_logs.append(list(r.get("action_log") or [])) + return r @typechecked def p_summary(r: Dict[str, object]) -> str: @@ -91,6 +97,7 @@ async def run_browser_fast_path( await ws_manager.send_to_session(session_id, "agent:message", { "session_id": session_id, "message": p_tc.model_dump(mode="json")}) first = await p_dispatch(browser_fast_path.compose_task(prompt, brief)) + p_last_result = first text = p_summary(first) if browser_fast_path.dispatch_failed(first): # Retry only transient failures; a dead dashboard fails the retry identically, so skip it and tell the user instead. @@ -132,8 +139,19 @@ async def run_browser_fast_path( ) # Close the synthetic bubble (always, even if the dispatch threw) so it never hangs as "running"; the bubble pairs this result with its call positionally. if p_bubble_tid: + # The bubble carries the same auditable record the sub-agent path shows. It used to close + # with the literal string "done", so expanding it on this tier revealed nothing. + from backend.apps.agents.browser import browser_trace + p_trace = browser_trace.build_trace( + tier=browser_trace.tier_label(p_fp_path, used_browser=True), + action_logs=p_action_logs, + receipt=browser_trace.receipt_from(p_last_result), + entry_url=p_entry or "", + ) p_tr = Message(role="tool_result", branch_id=session.active_branch_id, - content={"tool_use_id": p_bubble_tid, "tool": p_browser_tool, "text": "done"}) + content={"tool_use_id": p_bubble_tid, "tool": p_browser_tool, + "text": browser_trace.trace_text(p_trace), + **browser_trace.trace_payload(p_trace)}) session.messages.append(p_tr) await ws_manager.send_to_session(session_id, "agent:message", { "session_id": session_id, "message": p_tr.model_dump(mode="json")}) diff --git a/backend/tests/test_browser_trace.py b/backend/tests/test_browser_trace.py new file mode 100644 index 00000000..1b273a85 --- /dev/null +++ b/backend/tests/test_browser_trace.py @@ -0,0 +1,127 @@ +"""The browser trace: every tier owes the user the same auditable record. + +The bug this exists for: the expandable Browser Agent panel renders from CHILD SESSIONS, which only +the sub-agent path creates. The fast path closed its bubble with a tool_result of literally "done", +so on the tier that handles most tasks there was nothing to expand. "Trust me, I did it" is exactly +what a browser agent must never say. +""" +from backend.apps.agents.browser import browser_trace as bt + +NAV = {"tool": "BrowserNavigate", "input": {"url": "https://x.com/compose/post"}, + "elapsed_ms": 820, "ok": True} +TYPE = {"tool": "BrowserType", "input": {"text": "hello from my automation"}, "elapsed_ms": 140, "ok": True} +CLICK = {"tool": "BrowserClickByName", "input": {"name": "Post"}, "elapsed_ms": 310, "ok": True} + + +def test_the_trace_says_where_it_went_and_what_it_did(): + t = bt.build_trace("drove the browser", [[NAV, TYPE, CLICK]]) + assert t.pages == ["https://x.com/compose/post"] + assert len(t.steps) == 3 + text = bt.trace_text(t) + assert "x.com/compose/post" in text + assert "hello from my automation" in text, "what was typed is the whole point of an audit" + assert "Post" in text + + +def test_the_receipt_is_its_own_field_not_buried_in_the_steps(): + """The receipt is what separates 'it says it posted' from 'it posted', so it must be + structurally distinguishable, not a line the user has to spot among forty.""" + t = bt.build_trace("drove the browser", [[NAV]], receipt="composer cleared, post is on your profile") + assert t.receipt.startswith("composer cleared") + assert "Verified: composer cleared" in bt.trace_text(t) + + +def test_every_dispatch_shows_up_not_just_the_last(): + """A fast-path run can dispatch more than once (a recovery, a send probe). That work happened on + the user's behalf, so hiding all but the final attempt would misrepresent what was done.""" + t = bt.build_trace("drove the browser", [[NAV], [TYPE, CLICK]]) + assert len(t.steps) == 3 + + +def test_a_long_run_says_what_it_omitted_instead_of_silently_truncating(): + """A trace the user cannot tell is partial is worse than no trace, because they would read it as + the whole story.""" + t = bt.build_trace("drove the browser", [[NAV] * (bt.MAX_STEPS + 12)]) + assert len(t.steps) == bt.MAX_STEPS + assert t.steps_omitted == 12 + assert "12 earlier steps omitted" in bt.trace_text(t) + + +def test_pages_read_as_a_journey_not_a_log(): + """Consecutive repeats collapse (a reload is not a new place) but a genuine return does not, so + the list reads as where it went, in order. Revisiting after going elsewhere is real movement and + must survive.""" + home = {"tool": "BrowserNavigate", "input": {"url": "https://x.com/home"}, "ok": True} + t = bt.build_trace("x", [[NAV, TYPE, NAV, home, home]]) + assert t.pages == ["https://x.com/compose/post", "https://x.com/home"] + + back = bt.build_trace("x", [[NAV, home, NAV]]) + assert back.pages == ["https://x.com/compose/post", "https://x.com/home", "https://x.com/compose/post"] + + +def test_the_landing_page_shows_even_when_nothing_navigated(): + """Measured live: a cold run creates the card ALREADY pointed at its target, so no + BrowserNavigate is ever issued and a log-only trace could not say where the agent went. The + first thing anyone wants from an audit is the destination, so the entry URL is carried in.""" + reads = [{"tool": "BrowserGetText", "input": {}}, {"tool": "BrowserListInteractives", "input": {}}] + t = bt.build_trace("drove the browser", [reads], entry_url="https://claude.ai/") + assert t.pages == ["https://claude.ai/"] + assert "claude.ai" in bt.trace_text(t) + + +def test_the_landing_page_is_not_duplicated_when_it_did_navigate(): + t = bt.build_trace("x", [[NAV]], entry_url="https://x.com/compose/post") + assert t.pages == ["https://x.com/compose/post"] + + +def test_a_junk_entry_url_is_ignored_rather_than_shown(): + t = bt.build_trace("x", [[NAV]], entry_url="not a url") + assert t.pages == ["https://x.com/compose/post"] + + +def test_a_failed_step_is_marked_not_hidden(): + """A run that limped to its answer must not read as a clean one.""" + bad = {"tool": "BrowserClickByName", "input": {"name": "Post"}, "ok": False} + assert "(failed)" in bt.trace_text(bt.build_trace("x", [[bad]])) + + +def test_no_actions_is_stated_plainly_rather_than_rendering_blank(): + """The old failure mode was an empty panel, which reads as a broken UI rather than as a run that + genuinely did nothing in a browser.""" + assert bt.trace_text(bt.build_trace("", [], "")) == "No browser actions were recorded." + assert bt.trace_text(bt.build_trace("", [[]], "")) == "No browser actions were recorded." + + +def test_tier_is_described_in_words_a_user_understands(): + """'read->browser' is a routing string from a log line, not something anyone should be shown.""" + assert bt.tier_label("read", used_browser=False) == "read the page directly, no browser needed" + assert "browser" in bt.tier_label("read->browser", used_browser=True) + assert "->" not in bt.tier_label("read->browser", used_browser=True) + + +def test_the_payload_is_data_so_the_panel_never_parses_prose(): + t = bt.build_trace("drove the browser", [[NAV, TYPE]], receipt="delivered") + payload = bt.trace_payload(t)["browser_trace"] + assert payload["pages"] == ["https://x.com/compose/post"] + assert payload["receipt"] == "delivered" + assert isinstance(payload["steps"], list) + + +def test_malformed_entries_never_break_the_trace(): + """action_log comes from a live run; a half-written entry must degrade to a readable line rather + than take down the record of everything that DID happen.""" + junk = [{}, {"tool": None}, {"tool": "X", "input": "not-a-dict"}, {"input": {"url": 5}}] + text = bt.trace_text(bt.build_trace("x", [junk])) + assert text and "Traceback" not in text + + +def test_the_fast_path_actually_emits_a_trace(): + """INVARIANT: the whole point is that the tier which handles most tasks stops closing its bubble + with the string "done". Pinned by source because the emission sits inside a long async flow.""" + import inspect + + from backend.apps.agents.manager import run_browser_fast_path as fp + + src = inspect.getsource(fp.run_browser_fast_path) + assert '"text": "done"' not in src, 'the placeholder result is back; the bubble expands to nothing again' + assert "browser_trace.trace_payload" in src, "the bubble must carry the structured trace"