diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 41afe3db..bdf43c0e 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -34,6 +34,7 @@ from backend.apps.agents.browser.browser_loop import ( advance_stagnation, card_is_unavailable, completion_is_honest, + replay_recheck_is_safe, stagnation_exhausted, ) from backend.apps.agents.browser.browser_validator import adjudicate_stuck @@ -425,39 +426,39 @@ async def run_browser_agent( # i.e. faster than a human. Robust by construction: clicks re-resolve by # (role,name), every step is verified, and ANY miss aborts to the full LLM # agent below (which re-records), so a changed page can never ghost-succeed. - replay_host = browser_skills.host_of(initial_url) if initial_url else "" - if not replay_host and current_url: - replay_host = browser_skills.host_of(current_url) # live URL of an existing card - if not replay_host: - m = re.search(r"https?://\S+", task) - if m: - replay_host = browser_skills.host_of(m.group(0)) - skill = browser_skills.find_skill(replay_host, skill_key_task) if replay_host else None - # Fill any parameter slots from THIS task's quoted values (so one learned - # skill serves "do the same thing with a different input"). If a slot can't - # be filled, concrete_steps is None and we run the full agent instead. - concrete_steps = browser_skills.rehydrate(skill, skill_key_task) if skill else None - if skill and not concrete_steps: - logger.info(f"[browser-skills] skill matched on {replay_host} but slots unfillable from task; running full agent") - skill = None replay_attempted = False - if skill and concrete_steps: + + async def _try_replay(host: str, turns_spent: int) -> dict | None: + """Run a learned skill for the stable task key on `host` with zero LLM + calls. Returns a completed-result dict on full success, or None to fall + through to the LLM agent (no skill, unfillable slots, cancel, or any step + miss). Updates skill trust on every attempt. Used at dispatch AND, when + the card started on the wrong host, again after the first navigation + lands us somewhere a skill exists (the deferred re-check).""" + nonlocal final_screenshot, last_seen_url, replay_attempted + if not host: + return None + sk_obj = browser_skills.find_skill(host, skill_key_task) + steps = browser_skills.rehydrate(sk_obj, skill_key_task) if sk_obj else None + if sk_obj and not steps: + logger.info(f"[browser-skills] skill matched on {host} but slots unfillable from task; running full agent") + return None + if not (sk_obj and steps): + return None replay_attempted = True - logger.info(f"[browser-skills] REPLAY attempt: {len(concrete_steps)} steps on {replay_host}") - replay_log: list[dict] = [] - replay_ok = True - for step in concrete_steps: + logger.info(f"[browser-skills] REPLAY attempt: {len(steps)} steps on {host} (after {turns_spent} LLM turn(s))") + rlog: list[dict] = [] + ok = True + for step in steps: if cancel_event.is_set(): - replay_ok = False - break + return None st = time.time() res = await _cancellable(execute_browser_tool(step["tool"], step.get("params", {}), browser_id, tab_id)) if res is None: - replay_ok = False - break + return None el_ms = int((time.time() - st) * 1000) step_ok = "error" not in res - replay_log.append({ + rlog.append({ "tool": step["tool"], "input": step.get("params", {}), "result_summary": str(res.get("text", res.get("error", "")))[:200], "elapsed_ms": el_ms, "ok": step_ok, @@ -469,18 +470,18 @@ async def run_browser_agent( ) if not step_ok: logger.info(f"[browser-skills] replay step failed ({step['tool']}: {res.get('error')}), falling back to full agent") - replay_ok = False + ok = False break if res.get("url"): last_seen_url = res["url"] - if replay_ok and replay_log: - browser_skills.mark_replay_succeeded(replay_host, skill_key_task) + if ok and rlog: + browser_skills.mark_replay_succeeded(host, skill_key_task) summary = browser_metrics.record_task( session_id, browser_id, task, "completed", metrics_started_at, - 0, replay_log, session.tokens, + turns_spent, rlog, session.tokens, path="replay", task_sig=browser_skills._sig(skill_key_task), ) - logger.info(f"[browser-skills] REPLAY SUCCEEDED in {summary['total_ms']}ms with 0 LLM calls") + logger.info(f"[browser-skills] REPLAY SUCCEEDED in {summary['total_ms']}ms ({turns_spent} LLM turn(s))") try: ss = await execute_browser_tool("BrowserScreenshot", {}, browser_id, tab_id) if ss.get("image"): @@ -495,17 +496,34 @@ async def run_browser_agent( }) return { "session_id": session_id, "browser_id": browser_id, - "summary": f"Completed via learned skill replay ({len(concrete_steps)} steps, no LLM).", - "action_log": replay_log, "final_screenshot": final_screenshot, + "summary": f"Completed via learned skill replay ({len(steps)} steps, no LLM).", + "action_log": rlog, "final_screenshot": final_screenshot, "replayed": True, } - # Replay didn't fully succeed. Update the skill's trust BEFORE falling - # through: an unproven skill that failed gets quarantined (never replayed - # again -> pure-LLM baseline), a proven one tolerates a transient miss. - # The full agent below then re-records edit-aware (new steps -> new rev). + # Replay didn't fully succeed. Update the skill's trust: an unproven skill + # that failed gets quarantined (never replayed again -> pure-LLM baseline), + # a proven one tolerates a transient miss. The full agent re-records edit- + # aware (new steps -> new rev). if not cancel_event.is_set(): - verdict = browser_skills.mark_replay_failed(replay_host, skill_key_task) + verdict = browser_skills.mark_replay_failed(host, skill_key_task) logger.info(f"[browser-skills] replay fell back to full agent (trust verdict: {verdict})") + return None + + replay_host = browser_skills.host_of(initial_url) if initial_url else "" + if not replay_host and current_url: + replay_host = browser_skills.host_of(current_url) # live URL of an existing card + if not replay_host: + m = re.search(r"https?://\S+", task) + if m: + replay_host = browser_skills.host_of(m.group(0)) + # The card might have started on the WRONG host (the orchestrator often opens + # a fresh card on google and only navigates to the target later); if so, this + # dispatch check misses and the deferred re-check inside the loop catches it + # after the first navigation. + replay_rechecked = False + _dispatch_replay = await _try_replay(replay_host, 0) + if _dispatch_replay is not None: + return _dispatch_replay text_parts = [] # initialized before loop so post-loop summary (line ~1294) has a default # Circuit breaker for ReportProgress violations. Some models get stuck @@ -863,6 +881,20 @@ async def run_browser_agent( last_seen_url = result["url"] card_gone_streak = card_gone_streak + 1 if card_is_unavailable(result) else 0 + # Deferred replay re-check: the orchestrator often opens a fresh + # card on the wrong host, so the dispatch-time replay missed. Once + # a navigation lands us on a host that DOES have a matching skill, + # and nothing has dirtied the page yet, switch to replay (still + # verified per-step, still trust-gated). Fires at most once. + if (not replay_rechecked and tu.name == "BrowserNavigate" + and replay_recheck_is_safe(action_log)): + cur_host = browser_skills.host_of(last_seen_url) + if cur_host and cur_host != replay_host: + replay_rechecked = True + _deferred = await _try_replay(cur_host, turn + 1) + if _deferred is not None: + return _deferred + if tu.name == "BrowserScreenshot" and result.get("image"): final_screenshot = result["image"] diff --git a/backend/apps/agents/browser/browser_loop.py b/backend/apps/agents/browser/browser_loop.py index efb88177..360ec7ca 100644 --- a/backend/apps/agents/browser/browser_loop.py +++ b/backend/apps/agents/browser/browser_loop.py @@ -213,6 +213,22 @@ def card_is_unavailable(result: dict) -> bool: return any(m in err for m in _CARD_GONE_MARKERS) +# Actions that DIRTY the page so replay-from-here is no longer equivalent to a +# clean dispatch. Navigation and reads don't dirty anything (they just get us to +# the page), so the deferred replay re-check is allowed after only those. +_REPLAY_DIRTYING_TOOLS = { + "BrowserType", "BrowserClick", "BrowserClickIndex", + "BrowserPressKey", "BrowserScroll", "BrowserBatch", +} + + +def replay_recheck_is_safe(action_log: list[dict]) -> bool: + """True if nothing in the run so far has mutated page state, so switching to + a learned-skill replay now is equivalent to replaying from a clean dispatch + (the agent only navigated / looked around to get to the right host).""" + return not any(a.get("tool") in _REPLAY_DIRTYING_TOOLS for a in action_log) + + def completion_is_honest(action_log: list[dict]) -> tuple[bool, str]: """Reality-check a run the model declared done. Returns (honest, reason). diff --git a/backend/tests/test_browser_agent_loop.py b/backend/tests/test_browser_agent_loop.py index 5938567e..419c103f 100644 --- a/backend/tests/test_browser_agent_loop.py +++ b/backend/tests/test_browser_agent_loop.py @@ -269,6 +269,83 @@ def test_replay_falls_back_to_full_agent_when_a_step_fails(monkeypatch): assert len(primary.calls) > 0, "fell back to the full LLM agent" +def test_deferred_replay_fires_after_navigating_to_the_right_host(monkeypatch): + # The #30 fix: the orchestrator opens a fresh card on the WRONG host (google), + # so the dispatch-time replay check misses. Once the agent navigates to the + # host that DOES have a skill, and nothing has dirtied the page yet, the + # deferred re-check must switch to replay instead of grinding the LLM loop. + import backend.apps.agents.browser.browser_skills as SK + SK.clear() + BH._browser_history.clear() + SK.record_skill("docs.google.com", "click the Submit button", [ + {"tool": "BrowserClickIndex", "input": {}, "ok": True, + "clicked_role": "button", "clicked_name": "Submit"}, + ]) + # turn 0 navigates to the doc; the re-check should preempt everything after. + primary = FakeLLM([ + Resp([_rp("go to the doc"), _tu("BrowserNavigate", url=DOC_URL)]), + Resp([_rp("now click"), _tu("BrowserClick", selector=".submit")]), + Resp([Blk("text", "done")], stop_reason="end_turn"), + ]) + sent = _install(monkeypatch, primary, FakeAux()) + GOOGLE = "https://www.google.com/" + orig = BA.ws_manager.send_browser_command + + async def _cmd(request_id, action, browser_id, params, tab_id=""): + # perception + reads report GOOGLE (so the DISPATCH replay misses there), + # navigation + clicks report the doc host (so the re-check matches) + if action in ("list_interactives", "get_text"): + return {"text": "stuff", "url": GOOGLE} + return await orig(request_id, action, browser_id, params, tab_id) + monkeypatch.setattr(BA.ws_manager, "send_browser_command", _cmd, raising=False) + + # NO initial_url -> dispatch perceives google -> dispatch replay misses. + r = asyncio.run(BA.run_browser_agent( + task="Please click the Submit button", browser_id="b1", model="sonnet", + )) + assert r.get("replayed") is True, "deferred re-check must replay after the navigation" + assert any(c["action"] == "click_by_name" for c in sent), "replay re-resolved by name" + assert len(primary.calls) == 1, "only the navigate turn ran; the re-check preempted the rest" + # and the deferred replay still promotes the skill through the trust gate + assert SK.find_skill("docs.google.com", "click the Submit button")["state"] == SK._TRUSTED + + +def test_deferred_replay_does_not_fire_after_the_page_was_dirtied(monkeypatch): + # Safety guard: if the agent already typed/clicked before reaching the right + # host, replaying from here is NOT equivalent to a clean dispatch (the page + # state is dirty), so the re-check must stay disabled and the LLM finishes. + import backend.apps.agents.browser.browser_skills as SK + SK.clear() + BH._browser_history.clear() + SK.record_skill("docs.google.com", "click the Submit button", [ + {"tool": "BrowserClickIndex", "input": {}, "ok": True, + "clicked_role": "button", "clicked_name": "Submit"}, + ]) + # turn 0 TYPES (dirties the page), THEN turn 1 navigates to the doc host. + primary = FakeLLM([ + Resp([_rp("type first"), _tu("BrowserType", selector="#x", text="hi")]), + Resp([_rp("now go"), _tu("BrowserNavigate", url=DOC_URL)]), + Resp([Blk("text", "All done.")], stop_reason="end_turn"), + ]) + sent = _install(monkeypatch, primary, FakeAux()) + GOOGLE = "https://www.google.com/" + orig = BA.ws_manager.send_browser_command + + async def _cmd(request_id, action, browser_id, params, tab_id=""): + if action in ("list_interactives", "get_text"): + return {"text": "stuff", "url": GOOGLE} + return await orig(request_id, action, browser_id, params, tab_id) + monkeypatch.setattr(BA.ws_manager, "send_browser_command", _cmd, raising=False) + + r = asyncio.run(BA.run_browser_agent( + task="Please click the Submit button", browser_id="b1", model="sonnet", + )) + # a dirtied page must NOT trigger the deferred replay; the LLM ran to the end + assert not r.get("replayed"), "must not replay from a dirtied page state" + assert not any(c["action"] == "click_by_name" for c in sent) + assert len(primary.calls) >= 3, "the LLM loop finished normally" + + def test_replay_resolves_host_from_live_page_when_no_initial_url(monkeypatch): # The real-flow fix: the parent often delegates to an EXISTING browser card # with no initial_url (and the backend doesn't track where that card