diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 8950eb7a..68b74420 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -1148,21 +1148,23 @@ async def run_browser_agent( # Removal tasks never replay a skill (see the record gate): a delete is a destructive one-shot, # not a replayable nav prefix, so a stale delete-"skill" of scrolls must not hijack it. p_task_is_removal = is_removal_task(skill_key_task) + # Computed here (pure, task-only) so the skill gate, prestage, and every dispatch tier below + # share one verdict. `task` here is prompt + an aux-written routing brief (compose_task), and + # the brief's prose can read informational and wrongly disarm a real send (facebook: the brief + # tripped the info-ask gate while the user's own "start a post" is plainly an action). The + # user's words are authoritative, so a send stands if EITHER the composed task OR the raw + # prompt says action. + task_is_send = not (deliverable_is_informational("", task) + and deliverable_is_informational("", user_prompt or task)) p_early_host = browser_skills.host_of(initial_url or current_url or next(iter(re.findall(r"https?://\S+", task)), "")) - p_skip_prestage_for_skill = bool(p_early_host and not p_task_is_removal - and browser_skills.find_skill(p_early_host, skill_key_task)) + p_skip_prestage_for_skill = browser_skills.replay_owns_nav( + p_early_host, bool(browser_skills.find_skill(p_early_host, skill_key_task)), + p_task_is_removal, task_is_send) if p_skip_prestage_for_skill: logger.info(f"[browser-skills] skill exists for {p_early_host}; skipping prestage (replay owns the nav)") from backend.apps.agents.browser import browser_prestage from backend.apps.agents.browser import browser_plan_dispatch - # Computed here (pure, task-only) so prestage + every dispatch tier below shares one verdict. - # `task` here is prompt + an aux-written routing brief (compose_task), and the brief's prose - # can read informational and wrongly disarm a real send (facebook: the brief tripped the - # info-ask gate while the user's own "start a post" is plainly an action). The user's words - # are authoritative, so a send stands if EITHER the composed task OR the raw prompt says action. - task_is_send = not (deliverable_is_informational("", task) - and deliverable_is_informational("", user_prompt or task)) if (browser_prestage.prestage_enabled() and not app_mode and not cancel_event.is_set() and not p_skip_prestage_for_skill): try: diff --git a/backend/apps/agents/browser/browser_skills.py b/backend/apps/agents/browser/browser_skills.py index af336e29..73cf369d 100644 --- a/backend/apps/agents/browser/browser_skills.py +++ b/backend/apps/agents/browser/browser_skills.py @@ -798,6 +798,23 @@ def hint_step_adopted(step_key: tuple, action_log: list[dict]) -> bool: return False +def replay_owns_nav(host: str, has_skill: bool, task_is_removal: bool, task_is_send: bool) -> bool: + """Should a learned skill's replay take over navigation, letting the caller skip prestage? + + Yes for a READ: the replayed prefix does the same navigation prestage would aux-drive, faster. + + No for a SEND, and this is the part that was wrong. Prestage is also what hands the send-script + its composer perception; skip it and the entire fill/click/receipt tail is unreachable, so the + model falls back to burning 4-5 turns. Measured live on x.com with a learned skill present: + 5/5 writes went the slow way at 41-146s (median ~57s) with the receipt never speaking, against + 19.4s with the script armed. Prestage on an already-loaded page costs ~2-5s. + + No for a removal either: a delete is a destructive one-shot, not a replayable nav prefix, so a + stale delete-"skill" made of scrolls must never hijack it. + """ + return bool(host and has_skill and not task_is_removal and not task_is_send) + + def mark_replay_succeeded(host: str, task: str) -> None: """A replay ran end to end. Count it and, if the skill was still on probation, PROMOTE it to trusted (the verify gate just passed).""" diff --git a/backend/tests/test_skill_replay_vs_send_script.py b/backend/tests/test_skill_replay_vs_send_script.py new file mode 100644 index 00000000..d3f142c7 --- /dev/null +++ b/backend/tests/test_skill_replay_vs_send_script.py @@ -0,0 +1,67 @@ +"""A learned skill must not disarm the fast write path. + +Measured live on x.com 2026-07-28. A skill had been learned for the host, so every write took the +"skill exists ... skipping prestage (replay owns the nav)" branch. Prestage is also what hands the +send-script its composer perception, so with it skipped the whole fill/click/receipt tail was +unreachable and the model fell back to 4-5 turns: + + with a skill (replay owns nav) 5/5 writes, 41-146s, median ~57s, receipt never spoke + with the send-script armed 19.4s, receipt correct + +The skip was a real optimisation for READS, where the replayed prefix genuinely replaces prestage's +navigation. It just was never true for sends. Prestage on an already-loaded page measured 1.9-5.0s, +so a send trades a few seconds to save tens. + +This is the second time this exact interaction bit: the removal case was already carved out because +a stale delete-"skill" of scrolls could hijack a destructive one-shot. Same shape, so it is worth a +pure predicate with a test rather than a condition buried in a 2000-line function. +""" +from backend.apps.agents.browser import browser_skills as bs + + +def test_a_read_with_a_learned_skill_lets_replay_own_the_nav(): + """The optimisation this branch exists for must survive.""" + assert bs.replay_owns_nav("x.com", has_skill=True, task_is_removal=False, task_is_send=False) + + +def test_a_send_keeps_its_prestage_even_when_a_skill_exists(): + """The regression: skipping here costs the send-script its composer perception.""" + assert not bs.replay_owns_nav("x.com", has_skill=True, task_is_removal=False, task_is_send=True) + + +def test_a_removal_never_replays_a_skill(): + """A delete is a destructive one-shot, not a replayable nav prefix.""" + assert not bs.replay_owns_nav("x.com", has_skill=True, task_is_removal=True, task_is_send=False) + + +def test_no_skill_means_nothing_to_replay(): + assert not bs.replay_owns_nav("x.com", has_skill=False, task_is_removal=False, task_is_send=False) + + +def test_no_host_means_nothing_to_replay(): + """host_of returns "" for a task with no resolvable URL; that must not read as a skill hit.""" + assert not bs.replay_owns_nav("", has_skill=True, task_is_removal=False, task_is_send=False) + + +def test_a_send_that_is_also_a_removal_still_stands_down(): + """is_removal_task and task_is_send both fire on "delete the post that says X" (the classifier + keys on the verb). Either one alone must be enough to keep replay out.""" + assert not bs.replay_owns_nav("x.com", has_skill=True, task_is_removal=True, task_is_send=True) + + +def test_the_predicate_returns_a_real_bool(): + """It feeds an `if`; a truthy string or None would still work by accident and then stop working + the moment someone logs or serialises it.""" + for send in (True, False): + got = bs.replay_owns_nav("x.com", True, False, send) + assert got is True or got is False + + +def test_the_agent_uses_the_predicate_rather_than_reinventing_the_condition(): + """The whole point of extracting it. If someone re-inlines the check, these tests would keep + passing while the live path regressed, which is exactly how this bug survived the first time.""" + src = (bs.__file__).replace("browser_skills.py", "browser_agent.py") + with open(src) as f: + text = f.read() + assert "browser_skills.replay_owns_nav(" in text + assert "p_skip_prestage_for_skill = bool(" not in text, "the condition was re-inlined"