diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 68b74420..a7cfb10e 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -1173,6 +1173,7 @@ async def run_browser_agent( task, browser_id, tab_id, current_url, browser_settings, get_api_type(model), execute_browser_tool, perceive_only=(not task_is_send and browser_plan_dispatch.plan_dispatch_enabled()), + task_is_send=task_is_send, ), timeout=browser_prestage.TOTAL_TIMEOUT_S + 10, ) diff --git a/backend/apps/agents/browser/browser_prestage.py b/backend/apps/agents/browser/browser_prestage.py index 61129f24..6f5127c6 100644 --- a/backend/apps/agents/browser/browser_prestage.py +++ b/backend/apps/agents/browser/browser_prestage.py @@ -198,6 +198,7 @@ async def run_prestage( primary_api: str | None, execute_tool: ToolRunner, perceive_only: bool = False, + task_is_send: bool = False, ) -> tuple[str, str, list[dict]]: """(perception_block, current_url, action_records); ('', start_url, []) means nothing staged and the caller proceeds exactly as before. @@ -285,7 +286,8 @@ async def run_prestage( # left on, so a run can inherit a composer some earlier run opened and look like a win it # never earned; without this line there is no way to tell those apart after the fact. logger.info(f"[browser-prestage] start url={(start_url or '(none)')[:120]}") - p_compose_url = "" if perceive_only else (compose_entry.compose_entry_for(task, start_url) or "") + p_compose_url = "" if perceive_only else ( + compose_entry.compose_entry_for(task, start_url, task_is_send) or "") if p_compose_url: if await open_composer_directly(p_compose_url): staged_complete = True diff --git a/backend/apps/agents/browser/compose_entry.py b/backend/apps/agents/browser/compose_entry.py index f4431424..9edd7230 100644 --- a/backend/apps/agents/browser/compose_entry.py +++ b/backend/apps/agents/browser/compose_entry.py @@ -148,12 +148,19 @@ def enabled() -> bool: @typechecked -def compose_entry_for(task: str, start_url: str) -> Optional[str]: +def compose_entry_for(task: str, start_url: str, task_is_send: bool) -> Optional[str]: """The URL to open to reach this site's composer, or None to leave navigation alone. `start_url` is where the card already is; a host named in the task counts too, since a run that - begins on a blank tab still says "go to x.com and post ...".""" - if not enabled() or not wants_top_level_compose(task): + begins on a blank tab still says "go to x.com and post ...". + + `task_is_send` is the caller's already-computed write verdict and is REQUIRED, not defaulted, + because forgetting it is silently destructive: a quote is not proof of a write, and + `find the reddit post that says "..."` reads as a create to any regex short enough to be + readable (`post` is a noun there). Four such phrasings each resolved to reddit's SUBMIT page in + a probe, which would derail a plain read. The verdict the send script itself gates on is the + right authority, so this asks for it rather than growing a second opinion that can drift.""" + if not enabled() or not task_is_send or not wants_top_level_compose(task): return None # Which site to open comes from the user's words; a brief naming some other site must not # redirect the post. diff --git a/backend/tests/test_compose_entry.py b/backend/tests/test_compose_entry.py index 91699783..03a98e7e 100644 --- a/backend/tests/test_compose_entry.py +++ b/backend/tests/test_compose_entry.py @@ -13,6 +13,14 @@ import pytest from backend.apps.agents.browser import compose_entry as ce +def entry(task, start, is_send=True): + """Call the tier the way the browser agent does. `is_send` is its already-computed write + verdict; almost every case here is a write, so it defaults on and the read cases pass it + explicitly.""" + return ce.compose_entry_for(task, start, is_send) + + + # --- it fires where it should ------------------------------------------------------------------ @pytest.mark.parametrize("start,task,want_host", [ @@ -22,18 +30,18 @@ from backend.apps.agents.browser import compose_entry as ce ("https://mail.google.com/mail/u/0/", 'compose an email saying "hello"', "mail.google.com"), ]) def test_a_top_level_create_gets_the_sites_compose_url(start, task, want_host): - got = ce.compose_entry_for(task, start) + got = entry(task, start, True) assert got and ce.registrable_host(got) == want_host def test_the_host_can_come_from_the_task_when_the_tab_is_blank(): """A run that starts on about:blank still says where it is going.""" - got = ce.compose_entry_for('go to x.com and post "hello there"', "about:blank") + got = entry('go to x.com and post "hello there"', "about:blank", True) assert got == "https://x.com/compose/post" def test_subdomains_resolve_to_the_parent_sites_composer(): - assert ce.compose_entry_for('post "hello there"', "https://old.reddit.com/r/test") is not None + assert entry('post "hello there"', "https://old.reddit.com/r/test") is not None def test_www_and_mobile_prefixes_are_the_same_site(): @@ -53,50 +61,66 @@ def test_www_and_mobile_prefixes_are_the_same_site(): def test_answering_something_keeps_its_own_target(task): """A reply belongs on the thread in front of the user. Hijacking it to the site composer publishes a private answer to everyone, which is the worst failure this file guards.""" - assert ce.compose_entry_for(task, "https://x.com/home") is None + assert entry(task, "https://x.com/home") is None def test_the_refusals_above_are_caused_by_the_respond_word(): """Positive control. Every refusal test would also pass if the tier were simply broken and always returned None, so prove the same sentence DOES fire once the answering word is gone.""" - assert ce.compose_entry_for('reply with "hello there"', "https://x.com/home") is None - assert ce.compose_entry_for('post "hello there"', "https://x.com/home") is not None + assert entry('reply with "hello there"', "https://x.com/home") is None + assert entry('post "hello there"', "https://x.com/home") is not None def test_a_permalink_in_the_task_outranks_the_generic_composer(): task = 'post "hello there" on https://x.com/someone/status/12345' - assert ce.compose_entry_for(task, "https://x.com/home") is None + assert entry(task, "https://x.com/home") is None def test_a_bare_host_url_in_the_task_is_not_a_deeper_target(): - assert ce.compose_entry_for('go to https://x.com/ and post "hello there"', "about:blank") is not None + assert entry('go to https://x.com/ and post "hello there"', "about:blank") is not None def test_a_query_or_fragment_also_counts_as_a_chosen_target(): for url in ("https://www.reddit.com/?feed=home", "https://www.reddit.com/#top"): - assert ce.compose_entry_for(f'post "hello there" at {url}', "https://www.reddit.com/") is None + assert entry(f'post "hello there" at {url}', "https://www.reddit.com/") is None def test_a_read_task_never_navigates(): - assert ce.compose_entry_for("what is the top post on reddit", "https://www.reddit.com/") is None + """Two independent reasons, and the tier needs only one of them.""" + assert entry("what is the top post on reddit", "https://www.reddit.com/") is None + assert entry("what is the top post on reddit", "https://www.reddit.com/", False) is None + + +@pytest.mark.parametrize("task", [ + 'find the reddit post that says "hello there"', + 'check if my tweet "hello there" got any likes', + 'what does the post "hello there" say', + 'summarize the linkedin post about "quarterly results"', +]) +def test_a_quoted_READ_never_opens_a_composer(task): + """The bug this argument exists for. Each of these quotes something and contains a word that + reads as a create ("post" as a NOUN, "tweet" as a noun), and each one resolved to reddit's + SUBMIT page before the caller's verdict was required. Navigating a read to a compose form + derails the task and leaves the user staring at a half-open post box.""" + assert entry(task, "https://www.reddit.com/", False) is None def test_an_unknown_site_is_left_alone(): - assert ce.compose_entry_for('post "hello there"', "https://example.com/") is None + assert entry('post "hello there"', "https://example.com/") is None def test_a_lookalike_domain_is_not_the_real_one(): """`endswith("reddit.com")` matches `notreddit.com`. That would navigate a post intended for a site the user named to a completely different one.""" for host in ("notreddit.com", "fake-x.com", "linkedin.com.evil.test"): - assert ce.compose_entry_for('post "hello there"', f"https://{host}/") is None + assert entry('post "hello there"', f"https://{host}/") is None def test_already_on_the_compose_surface_does_not_remount_it(): """Re-navigating throws away a composer that is already open, and on a modal that means the user's half-typed state too.""" - assert ce.compose_entry_for('post "hello there"', "https://x.com/compose/post") is None - assert ce.compose_entry_for('start a post "hello there"', + assert entry('post "hello there"', "https://x.com/compose/post") is None + assert entry('start a post "hello there"', "https://www.linkedin.com/feed/?shareActive=true") is None @@ -115,20 +139,20 @@ def test_a_brief_that_quotes_something_does_not_disarm_the_tier(): (which passed the bare prompt) stayed green.""" task = p_dispatched('Go to x.com and post this tweet, exactly: "coverage probe 1"', 'Open the composer and type the text. Click "Post" to publish.') - assert ce.compose_entry_for(task, "") == "https://x.com/compose/post" + assert entry(task, "") == "https://x.com/compose/post" def test_a_brief_using_answering_words_does_not_disarm_the_tier(): task = p_dispatched('post "hello there" on x.com', 'If a dialog appears, respond to it and reply to any prompt.') - assert ce.compose_entry_for(task, "") is not None + assert entry(task, "") is not None def test_a_brief_naming_another_site_cannot_redirect_the_post(): """Which site to open is the user's call. A brief is a model's guess and must not move a post to a different service.""" task = p_dispatched('post "hello there" on x.com', 'You may find this on reddit.com instead.') - assert ce.compose_entry_for(task, "") == "https://x.com/compose/post" + assert entry(task, "") == "https://x.com/compose/post" def test_only_a_permalink_the_USER_named_vetoes(): @@ -137,10 +161,10 @@ def test_only_a_permalink_the_USER_named_vetoes(): post into a reply, because the words that would say so are the user's and are still read.""" brief_only = p_dispatched('post "hello there" on x.com', 'The target appears to be https://x.com/someone/status/12345') - assert ce.compose_entry_for(brief_only, "") == "https://x.com/compose/post" + assert entry(brief_only, "") == "https://x.com/compose/post" user_named = p_dispatched('post "hello there" on https://x.com/someone/status/12345', 'Open the composer.') - assert ce.compose_entry_for(user_named, "") is None + assert entry(user_named, "") is None # --- host parsing can't be sloppy --------------------------------------------------------------- @@ -158,4 +182,4 @@ def test_ports_and_credentials_are_dropped(): def test_garbage_never_raises(): for bad in ("", " ", "not a url", "://", "https://"): assert isinstance(ce.registrable_host(bad), str) - assert ce.compose_entry_for('post "hello there"', bad) is None + assert entry('post "hello there"', bad) is None