[eric] merge eric/browser-index-leak: stop the browser sub-agent relaying raw element indices to the user

backend/apps/agents/browser/browser_agent.py

@@ -49,6 +49,7 @@ from backend.apps.agents.browser.browser_loop import (
     stagnation_exhausted,
 )
 from backend.apps.agents.browser.browser_validator import adjudicate_stuck
+from backend.apps.agents.browser.humanize_element_rows import humanize_element_rows
 
 # Single actions the model could have folded into one BrowserBatch turn; reads, waits, and the batch tools themselves don't count toward the streak.
 P_BATCHABLE_ACTION_TOOLS = {
@@ -2462,8 +2463,13 @@ async def run_browser_agents(
 
     final = []
     for r in results:
-        if isinstance(r, Exception):
+        # gather(return_exceptions=True) hands back a cancelled child as a bare BaseException too, and that is not a result dict either.
+        if not isinstance(r, dict):
             final.append({"summary": f"Error: {str(r)}", "action_log": [], "final_screenshot": None})
-        else:
-            final.append(r)
+            continue
+        # Last stop before the sub-agent's own words reach a parent agent or, on the fast path, the user verbatim.
+        for p_key in ("summary", "error"):
+            if isinstance(r.get(p_key), str):
+                r[p_key] = humanize_element_rows(r[p_key])
+        final.append(r)
     return final

backend/apps/agents/browser/humanize_element_rows.py

@@ -0,0 +1,69 @@
+"""Rewrite the browser subsystem's internal element-index rows into plain prose.
+
+BrowserListInteractives hands the sub-agent rows like `[3]<button "Post">` so it can
+click things by number. That shape is machine input; a human reading a chat should
+never see it. Ask a sub-agent to "report the list verbatim" and it will happily copy
+those rows into its summary, which is the one string that crosses out of the browser
+subsystem into a person's transcript, so the summary gets laundered here.
+"""
+
+import re
+from typing import Dict, Match
+
+from typeguard import typechecked
+
+from backend.apps.agents.browser.browser_history import PAGE_STATE_MARKER
+
+# Exact inverse of the row the renderer builds in frontend/src/shared/browserCommandHandler.ts:
+# `[index]*<role "name" ctx="..." value="...">`. test_browser_index_leak.py pins that
+# template so a change over there can't quietly outrun this pattern.
+P_ROW_RE = re.compile(
+    r'\[\d+\]\*?<\s*(?P<role>[A-Za-z][A-Za-z0-9_-]*)\s+"(?P<name>[^"]*)"'
+    r'(?P<attrs>(?:\s+[a-z]+="[^"]*")*)\s*>'
+)
+P_ATTR_RE = re.compile(r'\s+(?P<key>[a-z]+)="(?P<val>[^"]*)"')
+# The asterisk legend and the truncation footer only mean anything next to indices, and
+# the footer names a tool the person reading it cannot call.
+P_LEGEND_RE = re.compile(r' *\(\* = new since your last look;[^)]*\)')
+P_TRUNCATION_RE = re.compile(r'(\d+ more not shown);[^.\n]*\.')
+P_MARKER_RE = re.compile(re.escape(PAGE_STATE_MARKER) + r'\n?')
+
+# ARIA role names a normal person would not recognize.
+P_ROLE_LABELS: Dict[str, str] = {
+    "combobox": "dropdown",
+    "textbox": "text field",
+    "searchbox": "search box",
+    "listbox": "list",
+    "menuitem": "menu item",
+    "menuitemcheckbox": "menu checkbox",
+    "menuitemradio": "menu option",
+    "spinbutton": "number field",
+    "treeitem": "tree item",
+}
+
+
+@typechecked
+def p_render_row(match: Match[str]) -> str:
+    role: str = match.group("role").lower()
+    name: str = match.group("name").strip()
+    attrs: Dict[str, str] = {
+        m.group("key"): m.group("val").strip()
+        for m in P_ATTR_RE.finditer(match.group("attrs") or "")
+    }
+    value: str = attrs.get("value", "")
+    role_label: str = P_ROLE_LABELS.get(role, role)
+    if name and value:
+        return f'"{name}" ({role_label}, currently "{value}")'
+    label: str = name or value
+    if not label:
+        return f"an unlabeled {role_label}"
+    return f'"{label}" ({role_label})'
+
+
+@typechecked
+def humanize_element_rows(text: str) -> str:
+    """Strip the click-by-index serialization out of browser-agent-authored prose."""
+    out: str = P_ROW_RE.sub(p_render_row, text)
+    out = P_LEGEND_RE.sub("", out)
+    out = P_TRUNCATION_RE.sub(r"\1.", out)
+    return P_MARKER_RE.sub("", out)

backend/tests/test_browser_index_leak.py

@@ -0,0 +1,95 @@
+"""The browser subsystem's click-by-index rows must never reach a human.
+
+`[1]<combobox "" value="-- pick --">` is what BrowserListInteractives hands the
+browser sub-agent so it can click by number. A user asked a normal agent to list a
+page's interactive elements and got those rows back verbatim in chat. The rows are
+produced in the renderer (frontend/src/shared/browserCommandHandler.ts) and ride into
+the transcript on the sub-agent's summary, so both the laundering and the renderer's
+template are pinned here.
+"""
+
+import asyncio
+import re
+from pathlib import Path
+
+from backend.apps.agents.browser import browser_agent as BA
+from backend.apps.agents.browser.browser_history import PAGE_STATE_MARKER
+from backend.apps.agents.browser.humanize_element_rows import humanize_element_rows
+
+# Verbatim from the bug report: six rows the agent pasted into a normal chat, on one line.
+LEAKED_REPLY = (
+    '[1]<combobox "" value="-- pick --"> [2]<button "Choose File"> '
+    '[3]<button "Shadow Button"> [4]<textbox "title"> [5]<button "Post"> '
+    '[6]<button "Frame Button">'
+)
+
+INDEX_ROW_RE = re.compile(r'\[\d+\]\*?<')
+
+
+def test_leaked_reply_loses_every_index_and_keeps_every_label():
+    out = humanize_element_rows(LEAKED_REPLY)
+    assert not INDEX_ROW_RE.search(out), out
+    assert "<" not in out and ">" not in out, out
+    for label in ("-- pick --", "Choose File", "Shadow Button", "title", "Post", "Frame Button"):
+        assert label in out, f"{label} was dropped: {out}"
+    assert "dropdown" in out and "text field" in out and "button" in out
+
+
+def test_multiline_listing_header_legend_and_footer_are_cleaned():
+    listing = (
+        "3 interactive elements (* = new since your last look; same number = same "
+        "element as before):\n"
+        '[1]<button "Like">\n'
+        '[2]*<textbox "Search" value="cats">\n'
+        '[3]<button "Message" ctx="Alice Smith">\n'
+        "... 25 more not shown; scroll or scope with BrowserGetElements to reach them."
+    )
+    out = humanize_element_rows(listing)
+    assert not INDEX_ROW_RE.search(out), out
+    assert "same number = same element as before" not in out
+    assert "BrowserGetElements" not in out
+    assert "25 more not shown." in out
+    assert out.startswith("3 interactive elements:")
+    assert '"Like" (button)' in out
+    assert '"Search" (text field, currently "cats")' in out
+
+
+def test_unlabeled_element_and_page_state_marker():
+    out = humanize_element_rows(f'{PAGE_STATE_MARKER}\n[7]<checkbox "">')
+    assert PAGE_STATE_MARKER not in out
+    assert out.strip() == "an unlabeled checkbox"
+
+
+def test_ordinary_prose_is_untouched():
+    prose = "I posted the reply as Alice at 10:43 PM. The list [1] had 3 items <b>ok</b>."
+    assert humanize_element_rows(prose) == prose
+
+
+def test_run_browser_agents_launders_summary_and_error(monkeypatch):
+    """The one funnel both delivery paths use: the MCP tool result the parent agent
+    reads, and the fast path that pipes the summary straight into the user's chat."""
+    async def p_fake_run_browser_agent(**kwargs):
+        return {"summary": LEAKED_REPLY, "error": LEAKED_REPLY,
+                "action_log": [], "final_screenshot": None}
+
+    monkeypatch.setattr(BA, "run_browser_agent", p_fake_run_browser_agent, raising=True)
+    monkeypatch.setattr(BA.ws_manager, "global_connections", [object()], raising=False)
+
+    results = asyncio.run(BA.run_browser_agents(
+        tasks=[{"task": "list the interactive elements", "browser_id": "b1"}],
+        model="sonnet",
+    ))
+
+    assert not INDEX_ROW_RE.search(results[0]["summary"]), results[0]["summary"]
+    assert not INDEX_ROW_RE.search(results[0]["error"]), results[0]["error"]
+    assert "Choose File" in results[0]["summary"]
+
+
+def test_renderer_row_template_still_matches_the_scrubber():
+    """Drift pin: the rows are built in TypeScript, so a Python-only change set can
+    silently stop matching. If this line moved, re-check humanize_element_rows."""
+    handler = Path(__file__).resolve().parents[2] / "frontend/src/shared/browserCommandHandler.ts"
+    source = handler.read_text(encoding="utf-8")
+    assert 'return `[${el.index}]${el.isNew ? \'*\' : \'\'}<${el.role} "${el.name}"${ctx}${val}>`;' in source
+    assert "const ctx = dup && el.context ? ` ctx=\"${el.context}\"` : '';" in source
+    assert "const val = el.value ? ` value=\"${el.value}\"` : '';" in source