mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-08-17 18:25:42 +02:00
[eric] browser: make a read wait until the page stops growing, not just until it looks long enough
This commit is contained in:
@@ -130,7 +130,7 @@ p_bridge_known_absent: set[str] = set()
|
||||
# Sites whose sign-in we already borrowed into the partition. Only SUCCESSFUL borrows are recorded,
|
||||
# so a site the user signs into later still gets picked up without a restart; the re-probe that
|
||||
# costs is a keychain-free row count, so re-asking is cheap.
|
||||
p_signin_borrowed: set[str] = set()
|
||||
signin_borrowed: set[str] = set()
|
||||
|
||||
|
||||
def parse_bridge_result(result: dict) -> object:
|
||||
@@ -255,7 +255,7 @@ def p_summarize_action(tool_name: str, tool_input: dict) -> str:
|
||||
return p_summ_step(stype, ti) if stype else ""
|
||||
|
||||
|
||||
async def p_borrow_signin_before_nav(url: str, browser_id: str) -> None:
|
||||
async def borrow_signin_before_nav(url: str, browser_id: str) -> None:
|
||||
"""Load the user's own sign-in for a site BEFORE its first page load, so the very first request
|
||||
already carries it.
|
||||
|
||||
@@ -270,11 +270,11 @@ async def p_borrow_signin_before_nav(url: str, browser_id: str) -> None:
|
||||
if not browser_session_import.is_enabled(load_settings()):
|
||||
return
|
||||
domain = browser_session_import.site_domain(url)
|
||||
if not domain or domain in p_signin_borrowed:
|
||||
if not domain or domain in signin_borrowed:
|
||||
return
|
||||
if not browser_session_import.has_importable_session(domain):
|
||||
return
|
||||
p_signin_borrowed.add(domain)
|
||||
signin_borrowed.add(domain)
|
||||
await browser_session_import.import_signin(domain, browser_id)
|
||||
except Exception as exc:
|
||||
logger.info(f"[session-import] pre-nav borrow skipped: {type(exc).__name__}")
|
||||
@@ -285,7 +285,7 @@ async def execute_browser_tool(
|
||||
) -> dict:
|
||||
"""Execute a browser tool via ws_manager directly (no MCP/HTTP round-trip)."""
|
||||
if tool_name == "BrowserNavigate":
|
||||
await p_borrow_signin_before_nav(str((tool_input or {}).get("url") or ""), browser_id)
|
||||
await borrow_signin_before_nav(str((tool_input or {}).get("url") or ""), browser_id)
|
||||
# One greppable line naming the actual buttons/keys/selectors this call drives,
|
||||
# so a run reads as "key:ArrowRight x5" rather than an opaque tool name. Fires
|
||||
# for action tools only (reads stay quiet) and ungated so web runs get it too.
|
||||
@@ -844,11 +844,11 @@ async def try_borrow_signin(domain: str, browser_id: str, tab_id: str, url: str)
|
||||
return False
|
||||
# Already borrowed at the door and STILL standing at a wall, so the session did not take.
|
||||
# Re-importing the same values would change nothing; this one needs a human.
|
||||
if domain in p_signin_borrowed:
|
||||
if domain in signin_borrowed:
|
||||
return False
|
||||
if not browser_session_import.has_importable_session(domain):
|
||||
return False
|
||||
p_signin_borrowed.add(domain)
|
||||
signin_borrowed.add(domain)
|
||||
result = await browser_session_import.import_signin(domain, browser_id)
|
||||
if not result.ok:
|
||||
return False
|
||||
@@ -3187,7 +3187,7 @@ async def run_browser_agents(
|
||||
# instant it mounts, with no BrowserNavigate to hook, so this is the only moment the
|
||||
# very first request can already carry the session. Outside the pick lock: it can touch
|
||||
# the keychain, and holding a lock across that would serialize every card creation.
|
||||
await p_borrow_signin_before_nav(host_src, "")
|
||||
await borrow_signin_before_nav(host_src, "")
|
||||
async with p_card_pick_lock:
|
||||
browser_id = find_reusable_card(dashboard_id, host_src, parent_session_id)
|
||||
if browser_id:
|
||||
|
||||
@@ -23,8 +23,16 @@ P_TEXT_TIMEOUT_S = 8.0
|
||||
P_AUX_TIMEOUT_S = 12.0
|
||||
# Prestage's click often lands here while the SPA is still hydrating (measured: a
|
||||
# LinkedIn profile read 184 chars right after the click); wait out the render, bounded.
|
||||
P_THIN_RETRIES = 3
|
||||
P_THIN_SETTLE_S = 1.2
|
||||
# Crossing the char floor is NOT the same as being finished rendering: a hydrating SPA clears 500
|
||||
# chars on nav and footer chrome long before the content lands. Taking that first passing read hands
|
||||
# the aux a half-drawn page, and it answers confidently from what IS there, so nothing declines and
|
||||
# the INSUFFICIENT retry below never fires. A confident wrong answer is the one outcome worse than
|
||||
# just running the loop, so the page has to prove it stopped growing: two reads in a row within this
|
||||
# much of each other. Costs one extra read plus one short settle on a path that already takes ~7-10s.
|
||||
P_STABLE_GROWTH = 0.05
|
||||
P_STABLE_SETTLE_S = 0.4
|
||||
MAX_READS = 4
|
||||
# A long-enough-but-still-rendering page reads as INSUFFICIENT (measured: profile
|
||||
# passed 500 chars with the headline section missing); one settle + re-read + re-ask.
|
||||
P_INSUFFICIENT_RETRIES = 1
|
||||
@@ -71,15 +79,22 @@ async def run_read_script(
|
||||
from backend.apps.agents.core.aux_llm import safe_resp_text
|
||||
|
||||
async def p_page_text() -> tuple:
|
||||
for attempt in range(P_THIN_RETRIES):
|
||||
"""Page text, but only once two consecutive reads agree it has stopped growing."""
|
||||
prev = -1
|
||||
text, url = "", ""
|
||||
for attempt in range(MAX_READS):
|
||||
r = await asyncio.wait_for(
|
||||
execute_tool("BrowserGetText", {}, browser_id, tab_id), timeout=P_TEXT_TIMEOUT_S)
|
||||
text = str(r.get("text") or "") if isinstance(r, dict) and "error" not in r else ""
|
||||
url = str(r.get("url") or "") if isinstance(r, dict) else ""
|
||||
if len(text) >= P_MIN_PAGE_CHARS:
|
||||
if len(text) >= P_MIN_PAGE_CHARS and 0 <= prev <= len(text) <= prev * (1 + P_STABLE_GROWTH):
|
||||
return text, url
|
||||
await asyncio.sleep(P_THIN_SETTLE_S)
|
||||
return "", ""
|
||||
# Still thin waits longer than merely still-growing: one is a page that has not
|
||||
# started, the other is one about to finish.
|
||||
thin = len(text) < P_MIN_PAGE_CHARS
|
||||
prev = len(text)
|
||||
await asyncio.sleep(P_THIN_SETTLE_S if thin else P_STABLE_SETTLE_S)
|
||||
return (text, url) if len(text) >= P_MIN_PAGE_CHARS else ("", "")
|
||||
|
||||
for ask in range(1 + P_INSUFFICIENT_RETRIES):
|
||||
page, p_live_url = await p_page_text()
|
||||
|
||||
@@ -14,8 +14,14 @@ class Resp:
|
||||
|
||||
|
||||
class Aux:
|
||||
def __init__(self, text): self.txt = text; self.messages = self; self.calls = 0
|
||||
async def create(self, **kw): self.calls += 1; return Resp(self.txt)
|
||||
def __init__(self, text):
|
||||
self.txt = text; self.messages = self; self.calls = 0; self.last_page = ""
|
||||
|
||||
async def create(self, **kw):
|
||||
self.calls += 1
|
||||
# Keep what the aux was actually shown: which page reached it IS the hydration contract.
|
||||
self.last_page = str(kw.get("messages", [{}])[0].get("content", ""))
|
||||
return Resp(self.txt)
|
||||
|
||||
|
||||
def tool_returning(text):
|
||||
@@ -28,6 +34,65 @@ def tool_returning(text):
|
||||
PAGE = "Tyler Chen\nHe/Him · 1st\nSomething Here\nIrvine, California\nEntrepreneurs First\n" + ("filler " * 200)
|
||||
|
||||
|
||||
def tool_returning_sequence(pages):
|
||||
"""A page that changes between reads, like an SPA finishing its render."""
|
||||
seen = []
|
||||
|
||||
async def run_tool(name, params, browser_id, tab_id):
|
||||
assert name == "BrowserGetText"
|
||||
seen.append(len(seen))
|
||||
return {"text": pages[min(len(seen) - 1, len(pages) - 1)]}
|
||||
run_tool.seen = seen
|
||||
return run_tool
|
||||
|
||||
|
||||
# Chrome that clears the 500-char floor while the actual content is still missing. This is the
|
||||
# shape that made the bug invisible: it is long enough to look like a real page.
|
||||
CHROME_ONLY = ("Home Feed My Network Jobs Messaging Notifications Me Work "
|
||||
"Skip to main content Keyboard shortcuts Close jump menu " * 12)
|
||||
HYDRATED = CHROME_ONLY + "\nTyler Chen\nSomething Here\nEntrepreneurs First\n" + ("filler " * 200)
|
||||
|
||||
|
||||
def test_waits_for_the_page_to_stop_growing(monkeypatch):
|
||||
"""The false-clean: a hydrating SPA crosses the char floor on nav and footer chrome long before
|
||||
the content lands. Answering from that first passing read produces a CONFIDENT WRONG answer,
|
||||
because the aux reports what it can see and nothing declines, so the INSUFFICIENT retry never
|
||||
fires. Two reads have to agree the page stopped growing before the aux sees anything."""
|
||||
monkeypatch.setattr(rs, "P_THIN_SETTLE_S", 0)
|
||||
monkeypatch.setattr(rs, "P_STABLE_SETTLE_S", 0)
|
||||
aux = Aux("His title is \"Something Here\".")
|
||||
tool = tool_returning_sequence([CHROME_ONLY, HYDRATED, HYDRATED])
|
||||
out = asyncio.run(rs.run_read_script(aux, "m", "find tyler chen's title", "b1", "t1", tool))
|
||||
|
||||
assert out == "His title is \"Something Here\"."
|
||||
assert len(tool.seen) >= 3, "must re-read until two reads agree, not answer off the first"
|
||||
sent = aux.last_page
|
||||
assert "Tyler Chen" in sent, "the aux must be handed the HYDRATED page, not the chrome-only one"
|
||||
|
||||
|
||||
def test_a_settled_page_still_answers_without_extra_waiting(monkeypatch):
|
||||
"""The guard must not turn every read into a slow read: a page that is already done answers as
|
||||
soon as two reads agree, which is immediately."""
|
||||
monkeypatch.setattr(rs, "P_THIN_SETTLE_S", 0)
|
||||
monkeypatch.setattr(rs, "P_STABLE_SETTLE_S", 0)
|
||||
aux = Aux("answer")
|
||||
tool = tool_returning_sequence([HYDRATED, HYDRATED])
|
||||
assert asyncio.run(rs.run_read_script(aux, "m", "q", "b1", "t1", tool)) == "answer"
|
||||
assert len(tool.seen) == 2, "a settled page costs exactly one confirming re-read"
|
||||
|
||||
|
||||
def test_a_page_that_never_settles_still_answers_from_the_last_read(monkeypatch):
|
||||
"""Something that keeps streaming forever (a live feed) must not fail closed to the loop just
|
||||
for being busy; after the read budget we use the fullest page we got."""
|
||||
monkeypatch.setattr(rs, "P_THIN_SETTLE_S", 0)
|
||||
monkeypatch.setattr(rs, "P_STABLE_SETTLE_S", 0)
|
||||
grows = [HYDRATED + ("more " * 200 * i) for i in range(1, 8)]
|
||||
aux = Aux("answer")
|
||||
tool = tool_returning_sequence(grows)
|
||||
assert asyncio.run(rs.run_read_script(aux, "m", "q", "b1", "t1", tool)) == "answer"
|
||||
assert len(tool.seen) == rs.MAX_READS
|
||||
|
||||
|
||||
def test_flag_gate(monkeypatch):
|
||||
monkeypatch.delenv("OSW_READ_SCRIPT", raising=False)
|
||||
assert rs.read_script_enabled() is False
|
||||
|
||||
@@ -189,7 +189,7 @@ async def test_borrow_happens_at_the_door_not_only_at_the_wall(monkeypatch):
|
||||
from backend.apps.agents.browser import browser_agent
|
||||
|
||||
seen = []
|
||||
browser_agent.p_signin_borrowed.discard("x.com")
|
||||
browser_agent.signin_borrowed.discard("x.com")
|
||||
monkeypatch.setattr(browser_agent.browser_session_import, "is_enabled", lambda s: True)
|
||||
monkeypatch.setattr(browser_agent.browser_session_import, "has_importable_session", lambda d: True)
|
||||
|
||||
@@ -198,13 +198,13 @@ async def test_borrow_happens_at_the_door_not_only_at_the_wall(monkeypatch):
|
||||
return si.SessionImportResult(outcome="imported", domain=domain, entries_applied=3)
|
||||
|
||||
monkeypatch.setattr(browser_agent.browser_session_import, "import_signin", fake_import)
|
||||
await browser_agent.p_borrow_signin_before_nav("https://x.com/compose/post", "b1")
|
||||
await browser_agent.borrow_signin_before_nav("https://x.com/compose/post", "b1")
|
||||
assert seen == ["x.com"], "navigating to a site must borrow its sign-in first"
|
||||
|
||||
# Second navigate to the same site must not re-read the user's browser.
|
||||
await browser_agent.p_borrow_signin_before_nav("https://x.com/home", "b1")
|
||||
await browser_agent.borrow_signin_before_nav("https://x.com/home", "b1")
|
||||
assert seen == ["x.com"], "a borrowed site must not be re-imported on every navigate"
|
||||
browser_agent.p_signin_borrowed.discard("x.com")
|
||||
browser_agent.signin_borrowed.discard("x.com")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -212,11 +212,11 @@ async def test_pre_nav_borrow_respects_the_opt_in(monkeypatch):
|
||||
"""The door is the busiest path in the whole agent, so the gate has to hold there too."""
|
||||
from backend.apps.agents.browser import browser_agent
|
||||
|
||||
browser_agent.p_signin_borrowed.discard("x.com")
|
||||
browser_agent.signin_borrowed.discard("x.com")
|
||||
monkeypatch.setattr(browser_agent.browser_session_import, "is_enabled", lambda s: False)
|
||||
monkeypatch.setattr(browser_agent.browser_session_import, "has_importable_session",
|
||||
lambda d: pytest.fail("must not probe the user's browser while opted out"))
|
||||
await browser_agent.p_borrow_signin_before_nav("https://x.com/home", "b1")
|
||||
await browser_agent.borrow_signin_before_nav("https://x.com/home", "b1")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -226,14 +226,14 @@ async def test_wall_handoff_asks_a_human_once_the_door_borrow_did_not_take(monke
|
||||
silently returning True here would skip the prompt and strand the run."""
|
||||
from backend.apps.agents.browser import browser_agent
|
||||
|
||||
browser_agent.p_signin_borrowed.add("acme.example")
|
||||
browser_agent.signin_borrowed.add("acme.example")
|
||||
monkeypatch.setattr(browser_agent.browser_session_import, "is_enabled", lambda s: True)
|
||||
monkeypatch.setattr(browser_agent.browser_session_import, "import_signin",
|
||||
lambda d, b: pytest.fail("must not re-import the same values"))
|
||||
try:
|
||||
assert await browser_agent.try_borrow_signin("acme.example", "b1", "", "") is False
|
||||
finally:
|
||||
browser_agent.p_signin_borrowed.discard("acme.example")
|
||||
browser_agent.signin_borrowed.discard("acme.example")
|
||||
|
||||
|
||||
def test_import_timeout_outlasts_the_hidden_window_warm():
|
||||
|
||||
Reference in New Issue
Block a user