diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index c072b68e..ad2493fb 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -127,6 +127,11 @@ P_BRIDGE_READY_WAIT_MS = 8000 P_BRIDGE_POLL_INTERVAL_MS = 400 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() + def parse_bridge_result(result: dict) -> object: """Decode the JSON string an app-bridge evaluate returns (it always returns @@ -250,10 +255,37 @@ 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: + """Load the user's own sign-in for a site BEFORE its first page load, so the very first request + already carries it. + + Borrowing only at a detected wall was too late in two ways: a task the model answers in one turn + calls Done, which breaks the loop before the handoff block ever runs, so short tasks never got it + at all; and even a long task had to render a logged-out page, notice, and throw it away. Doing it + at the door costs the user nothing and skips both. Silent and best-effort; the wall handoff is + still there as the backstop when this finds nothing.""" + from backend.apps.settings.settings import load_settings + + try: + 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: + return + if not browser_session_import.has_importable_session(domain): + return + p_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__}") + + async def execute_browser_tool( tool_name: str, tool_input: dict, browser_id: str, tab_id: str = "", ) -> 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) # 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. @@ -810,8 +842,13 @@ async def try_borrow_signin(domain: str, browser_id: str, tab_id: str, url: str) try: if not browser_session_import.is_enabled(load_settings()): 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: + return False if not browser_session_import.has_importable_session(domain): return False + p_signin_borrowed.add(domain) result = await browser_session_import.import_signin(domain, browser_id) if not result.ok: return False @@ -3146,6 +3183,11 @@ async def run_browser_agents( if not browser_id and dashboard_id: # the url param is often empty with the target buried in the task text; a url there still names the host we must not duplicate host_src = url or entry_url or next(iter(re.findall(r"https?://[^\s)\"'<>]+", task_text)), "") + # Borrow the user's sign-in BEFORE any card exists. A new card loads its entry URL the + # 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, "") async with p_card_pick_lock: browser_id = find_reusable_card(dashboard_id, host_src, parent_session_id) if browser_id: diff --git a/backend/tests/test_browser_session_import.py b/backend/tests/test_browser_session_import.py index b6dcd278..3b078f3e 100644 --- a/backend/tests/test_browser_session_import.py +++ b/backend/tests/test_browser_session_import.py @@ -178,6 +178,61 @@ async def test_a_broken_borrow_can_never_break_the_run(monkeypatch): assert await browser_agent.try_borrow_signin("acme.example", "b1", "", "") is False +@pytest.mark.asyncio +async def test_borrow_happens_at_the_door_not_only_at_the_wall(monkeypatch): + """Borrowing only at a detected wall was too late: a task the model answers in one turn calls + Done, which breaks the loop BEFORE the handoff runs, so short tasks never got the session at + all. Navigating must carry it.""" + from backend.apps.agents.browser import browser_agent + + seen = [] + browser_agent.p_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) + + async def fake_import(domain, browser_id): + seen.append(domain) + 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") + 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") + assert seen == ["x.com"], "a borrowed site must not be re-imported on every navigate" + browser_agent.p_signin_borrowed.discard("x.com") + + +@pytest.mark.asyncio +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") + 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") + + +@pytest.mark.asyncio +async def test_wall_handoff_asks_a_human_once_the_door_borrow_did_not_take(monkeypatch): + """If we already borrowed at the door and are STILL at a wall, the session did not work. + Re-importing identical values would change nothing, so this case belongs to the human, and + 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") + 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") + + def test_agent_checks_the_opt_in_before_reading_anything(): """INVARIANT: the borrow helper must consult the setting FIRST. Pinned by source because the ordering is the whole consent story, and an innocent-looking reorder would start reading the