diff --git a/backend/apps/agents/agent_manager.py b/backend/apps/agents/agent_manager.py index 1f393930..16551a02 100644 --- a/backend/apps/agents/agent_manager.py +++ b/backend/apps/agents/agent_manager.py @@ -3413,7 +3413,7 @@ class AgentManager: "session_id": session_id, "message": _tc.model_dump(mode="json")}) first = await _dispatch(browser_fast_path.compose_task(prompt, brief)) text = _summary(first) - if browser_fast_path.dispatch_failed(text): + if browser_fast_path.dispatch_failed(first): # Retry only transient failures; a dead dashboard fails the # retry identically, so skip it and tell the user instead. if not ws_manager.global_connections: diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 9734d55d..fba6304b 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -844,10 +844,11 @@ async def run_browser_agent( }) return { "session_id": session_id, "browser_id": browser_id, - # Same OUTCOME shape the LLM path emits, so the parent translates it - # into a plain human confirmation instead of leaking the replay - # mechanics ('skill replay / N steps / no LLM') to the user. - "summary": f"OUTCOME: DONE - {task}", + # Clean human confirmation, never the replay mechanics ('skill + # replay / N steps / no LLM'); `done` is the structured success + # signal the parent reads instead of grepping for a tag. + "summary": "Done, I took care of that for you.", + "done": True, "action_log": rlog, "final_screenshot": final_screenshot, "replayed": True, } @@ -923,6 +924,12 @@ async def run_browser_agent( text_parts = [] # initialized before loop so post-loop summary (line ~1294) has a default rp_violations = 0 # turns the model acted without ReportProgress (now accepted + reminded, not rejected) + # The model finishes by calling the Done tool; `message` is the clean human + # reply, `success` whether the goal was met. Falls back to terminal text on + # the rare run that stops without calling Done. + done_called = False + done_message = "" + done_success = True # Completion detection: once an irreversible SEND has confirmed, the goal is # met. The model otherwise stalls re-verifying what the confirm already proved # (measured: send done at turn ~11, then ~12 wasted perception turns). We drive @@ -1121,12 +1128,13 @@ async def run_browser_agent( f"[browser-agent {session_id}] ending: {perception_stall} pure-perception " f"turns (send_confirmed={send_confirmed}); not letting it spin further" ) - # if the send registered, hand the parent a real DONE; otherwise just + # if the send registered, hand the parent a real done; otherwise just # end the spin and let the honesty gate decide from the action log if send_confirmed: # plain confirmation; the raw action-log proof (indices, coords) # is machine-speak, so we do NOT splice it into the user's line - text_parts = ["OUTCOME: DONE - the task is complete and was confirmed on the page."] + done_called = True + done_message = "All set, your message went through and it's showing in the conversation now." break else: perception_stall = 0 @@ -1321,6 +1329,21 @@ async def run_browser_agent( }) continue + # Handle Done: the model's typed-field finish. The `message` is + # the clean human reply (no OUTCOME tag, no UI mechanics), so it + # goes to the user as-is. Add its tool_result here (the post-loop + # integrity backfill pairs the rest + appends), set the flag, and + # break; the outer loop exits on done_called right after. + if tu.name == "Done": + done_called = True + done_message = (tu.input.get("message") or "").strip() + done_success = tu.input.get("success", True) is not False + tool_results.append({ + "type": "tool_result", "tool_use_id": tu.id, + "content": [{"type": "text", "text": "ok"}], + }) + break + # Handle RequestHumanIntervention; pause and wait for user if tu.name == "RequestHumanIntervention": problem = tu.input.get("problem", "") @@ -1482,8 +1505,8 @@ async def run_browser_agent( if _send_click: send_confirmed = True result["text"] = (f"{result.get('text') or ''}\n\n[task complete] The send " - "went through (the composer cleared). Don't re-check it. Give your " - "final answer now, your OUTCOME line.") + "went through (the composer cleared). Don't re-check it. Finish now by " + "calling Done with your reply to the user.") action_log.append({ "tool": tu.name, @@ -1815,6 +1838,9 @@ async def run_browser_agent( }) messages.append({"role": "user", "content": tool_results}) + if done_called: + break + if cancelled: break @@ -1857,8 +1883,14 @@ async def run_browser_agent( "final_screenshot": final_screenshot, } - summary_parts = text_parts if text_parts else ["Task completed."] - summary = "\n".join(summary_parts) + # The model finishes through Done, so its `message` IS the user's reply + # (clean, conversational, no tag). The rare run that stops without calling + # Done falls back to its own terminal text, which is plain prose since no + # prompt asks for a machine tag anymore. + if done_called: + summary = done_message or "Done." + else: + summary = "\n".join(text_parts).strip() if text_parts else "Task completed." if not final_screenshot: try: @@ -2005,6 +2037,10 @@ async def run_browser_agent( "session_id": session_id, "browser_id": browser_id, "summary": summary, + # structured success signal the parent reads (replaces grepping the + # summary for a tag): true only if the run is honest AND, when the + # model called Done, it reported success. No-Done runs lean on honest. + "done": honest and (done_success if done_called else True), # surface the honest failure to the parent so it doesn't treat a # did-nothing run as a success it can build on **({} if honest else {"error": summary}), diff --git a/backend/apps/agents/browser/browser_fast_path.py b/backend/apps/agents/browser/browser_fast_path.py index 38419609..5f359315 100644 --- a/backend/apps/agents/browser/browser_fast_path.py +++ b/backend/apps/agents/browser/browser_fast_path.py @@ -4,8 +4,8 @@ Browser fast path: skip the orchestrator for plainly browser-only requests. The orchestrator LLM is ~2/3 of the token bill on a single-browser task and adds two model turns of latency, all to decide "delegate this to a browser agent" and then restate the agent's own outcome. When the request is clearly -just browsing, dispatch the browser sub-agent directly and let its OUTCOME -line be the reply. +just browsing, dispatch the browser sub-agent directly and let its Done +message (a clean human reply already) be the reply. Three gates, all conservative; any miss falls through to the orchestrator: 1. eligibility: first message of an agent session on a dashboard, no @@ -128,13 +128,12 @@ def compose_task(prompt: str, brief: str) -> str: ) -def dispatch_failed(summary: str) -> bool: - """Fail-closed: only an explicit DONE line or a skill replay counts as - success; framework failures ('I was not able to complete this task (the - browser became unresponsive...)') carry no OUTCOME line at all, and the +def dispatch_failed(result: dict) -> bool: + """Fail-closed: a real completion sets done=True (the sub-agent called Done + with success, and the honesty gate agreed). Anything else, a hung/errored + dispatch or the model reporting it couldn't, means recovery should run. The recovery task's verify-first wording makes a rare redundant retry safe.""" - s = (summary or "").strip() - return not ("OUTCOME: DONE" in s.upper() or "learned skill replay" in s) + return not (isinstance(result, dict) and result.get("done", False)) NO_DASHBOARD_REPLY = ( diff --git a/backend/apps/agents/browser/browser_schema.py b/backend/apps/agents/browser/browser_schema.py index 68caa4de..93d456ee 100644 --- a/backend/apps/agents/browser/browser_schema.py +++ b/backend/apps/agents/browser/browser_schema.py @@ -11,16 +11,16 @@ purpose because it is one cohesive data blob, not multiple responsibilities. # ~28% and roughly halved narration turns. MERGE_VERIFY (a confirmed `expect` is the # proof, skip the re-check) drops a wasted round-trip at the end. _THINK_SHORTER = ( - "Do NOT also write a free-text sentence next to your action tools: your ReportProgress " + "Do NOT write a free-text sentence next to your action tools: your ReportProgress " "fields ARE your thinking, and a separate prose explanation just repeats them and slows " - "the turn (it is shown to the user twice). The ONLY time to write a plain message is your " - "FINAL turn, when the task is done and you call no action tool: that message is your " - "answer to the user (the OUTCOME line). Every other turn: ReportProgress + tools, no prose.\n" + "the turn. Don't narrate to the user as you go either. When the task is done you finish by " + "calling the Done tool (never by typing a sentence); every other turn is ReportProgress + " + "tools, no prose.\n" ) _MERGE_VERIFY = ( "When that `expect` CONFIRMS (the result says 'Confirmed: ...'), that IS your " - "verification: go STRAIGHT to your final OUTCOME line and cite it. Do NOT spend an " + "verification: go STRAIGHT to calling Done. Do NOT spend an " "extra screenshot or read turn to re-check what the confirmation already proved, that " "is a wasted round-trip. Only take a separate verification step when `expect` came " "back 'NOT confirmed' or you forgot to pass one.\n" @@ -88,6 +88,39 @@ BROWSER_TOOLS_SCHEMA = [ "required": ["working_memory", "next_goal"], }, }, + { + "name": "Done", + "description": ( + "Call this the moment the task is finished (or you've hit a wall you " + "can't get past) to deliver your final reply to the user. This ends the " + "run. Do NOT type a sentence to finish, always finish by calling Done." + ), + "input_schema": { + "type": "object", + "properties": { + "message": { + "type": "string", + "description": ( + "What the user reads, so write it like a quick text to a friend: " + "what you did and the proof a person actually cares about (the name, " + "the time, what's now on screen). One or two plain sentences. Use ZERO " + "interface words, no 'button', 'box', 'textbox', 'composer', 'field', " + "'element', index numbers, or coordinates, and don't mechanically repeat " + "the task back. If you couldn't finish, say what's missing in that same " + "plain voice and set success to false." + ), + }, + "success": { + "type": "boolean", + "description": ( + "true if you accomplished what the user asked, false if you couldn't " + "(login wall, missing info, something blocked you). Default true." + ), + }, + }, + "required": ["message"], + }, + }, { "name": "BrowserScreenshot", "description": ( @@ -792,16 +825,13 @@ SYSTEM_PROMPT = ( "- Anything genuinely ambiguous about user intent\n" "Don't use it for normal tool failures; try a different approach first.\n\n" - "Complete the task autonomously. Your FINAL message is ONE line and ONLY that line: " - "'OUTCOME: DONE - ' (or 'OUTCOME: NOT DONE - '). Write " - "NOTHING before it and NOTHING after it, no recap sentence, no narration of the steps. " - "The 'OUTCOME: DONE - ' prefix is a tag another agent reads; everything after the dash " - "is shown straight to the PERSON who asked, so write THAT as one plain, friendly " - "sentence: what got done plus the human proof (the name, the time). Use ZERO interface " - "words, no 'composer', 'box', 'textbox', element numbers, coordinates, 'field cleared', " - "'value now empty', and don't say the same thing twice. Say it the way a helpful " - "assistant would tell a friend. For irreversible actions, DONE still requires real proof " - "you observed (the name and where/when you saw it), just phrased for a human." + "Complete the task autonomously. When you're finished, end the run by calling the Done " + "tool, never by typing a sentence. Put your reply to the user in Done's `message`, " + "written like a normal chat reply: what got done plus the human proof (the name, the " + "time, what's now on screen), in one or two plain sentences with zero interface words. " + "Set `success` false if you couldn't finish. For irreversible actions, only report " + "success with real proof you actually observed (the name and where/when you saw it), " + "just phrased for a person, not for a machine." ) MAX_TURNS = 40 diff --git a/backend/apps/agents/manager/prompt/prompt_context.py b/backend/apps/agents/manager/prompt/prompt_context.py index 5585846e..16afb6c7 100644 --- a/backend/apps/agents/manager/prompt/prompt_context.py +++ b/backend/apps/agents/manager/prompt/prompt_context.py @@ -139,17 +139,14 @@ def _build_browser_context(dashboard_id: str | None, selected_browser_ids: list[ "and faster than spawning one agent per item with BrowserAgents, use parallel " "BrowserAgents only for genuinely DIFFERENT tasks, not for the same flow repeated.", "", - "**Trust the agent's OUTCOME line for your DECISION; TRANSLATE it for the user.** " - "Every browser agent result ends with 'OUTCOME: DONE - ' or 'OUTCOME: NOT " - "DONE - '. That line is internal plumbing for YOU, never show it to the user. " - "DONE with proof means complete: confirm it to the user the way a helpful person " - "would, one short natural sentence saying what got done plus the human-meaningful " - "proof from the result (the name, the time, the title). NEVER echo the literal " - "'OUTCOME:' tag or the agent's UI mechanics (composer, textbox, element indices, " - "'value now empty'), those mean nothing to a user; and do NOT dispatch a " - "verification agent. NOT DONE means re-dispatch with a sharper task (start from " - "what the agent reported), not a duplicate. Long restatements of what the agent " - "already said just slow the user down.", + "**The browser agent hands back a plain summary; relay it, don't re-narrate.** " + "It already writes its result like a normal chat reply (what got done plus the " + "human proof: the name, the time, the title), with no UI mechanics. When it " + "succeeded, just confirm that to the user in one short natural sentence, reusing " + "its words; don't pad it, don't dispatch a verification agent. If it reports it " + "couldn't finish, re-dispatch with a sharper task (start from what it reported), " + "not a duplicate. Long restatements of what the agent already said just slow the " + "user down.", ] if browser_cards and selected_browser_ids: diff --git a/backend/tests/test_browser_agent_loop.py b/backend/tests/test_browser_agent_loop.py index af8b1ebf..27384085 100644 --- a/backend/tests/test_browser_agent_loop.py +++ b/backend/tests/test_browser_agent_loop.py @@ -230,7 +230,41 @@ def test_confirmed_send_ends_the_run_instead_of_stalling(monkeypatch): # consuming all 8 scripted stall turns assert any(c["action"] == "click_index" and c["params"].get("index") == 99 for c in sent) assert primary.turn <= 4, f"run stalled {primary.turn} turns after a confirmed send" - assert result["summary"].startswith("OUTCOME: DONE") or "DONE" in result["summary"] + # structured success + a clean human summary, never the internal tag + assert result.get("done") is True + assert "OUTCOME" not in result["summary"] + assert result["summary"].strip() + + +def test_done_tool_delivers_a_clean_human_summary(monkeypatch): + # Canonical finish: the model calls Done(message); that message is the user's + # reply verbatim (no OUTCOME tag, no UI mechanics) and `done` is True. + BH._browser_history.clear(); BH._domain_notes.clear() + primary = FakeLLM([ + Resp([_rp("open profile + send"), _tu("BrowserClickIndex", index=5, expect="Sent")]), + Resp([_tu("Done", message="Sent your message to Tyler, it's in the thread now.")]), + ]) + aux = FakeAux() + _install(monkeypatch, primary, aux) + result = asyncio.run(BA.run_browser_agent(task="text Tyler hello", browser_id="b1", model="sonnet")) + assert result["summary"] == "Sent your message to Tyler, it's in the thread now." + assert result.get("done") is True + assert "OUTCOME" not in result["summary"] + + +def test_done_tool_success_false_marks_not_done(monkeypatch): + # Done(success=false) is the honest "couldn't finish": done is False so the + # fast path knows to recover, and the message still reads like a person wrote it. + BH._browser_history.clear(); BH._domain_notes.clear() + primary = FakeLLM([ + Resp([_rp("look for thread"), _tu("BrowserClickIndex", index=3)]), + Resp([_tu("Done", message="I hit a login wall, so I couldn't open the chat.", success=False)]), + ]) + aux = FakeAux() + _install(monkeypatch, primary, aux) + result = asyncio.run(BA.run_browser_agent(task="text Tyler hello", browser_id="b1", model="sonnet")) + assert result.get("done") is False + assert "login wall" in result["summary"] def test_early_perception_is_not_cut_short_before_any_action(monkeypatch): diff --git a/backend/tests/test_browser_fast_path.py b/backend/tests/test_browser_fast_path.py index bb0952db..500b156a 100644 --- a/backend/tests/test_browser_fast_path.py +++ b/backend/tests/test_browser_fast_path.py @@ -62,16 +62,16 @@ def test_compose_task_keeps_user_words_first(): def test_dispatch_failure_detection_is_fail_closed(): - assert dispatch_failed("") - assert dispatch_failed("Error: browser card was deleted") - assert dispatch_failed("Could not find the thread. OUTCOME: NOT DONE - login wall") - assert dispatch_failed( - "I was not able to complete this task (the browser became unresponsive " - "(the tab hung or was closed); it needs a fresh browser to continue)." - ) - assert dispatch_failed("Found the page and clicked around a bit.") - assert not dispatch_failed("Sent it. OUTCOME: DONE - bubble visible at 12:05 PM") - assert not dispatch_failed("Completed via learned skill replay (3 steps, no LLM).") + # The result dict's structured `done` is the signal now (set true only when + # the sub-agent called Done with success AND the honesty gate agreed). + assert dispatch_failed({}) + assert dispatch_failed(None) + assert dispatch_failed({"summary": "Error: browser card was deleted"}) + assert dispatch_failed({"summary": "couldn't find the thread", "done": False}) + assert dispatch_failed({"summary": "the browser became unresponsive", "error": "x"}) + assert dispatch_failed({"summary": "clicked around a bit"}) # no done -> failed + assert not dispatch_failed({"summary": "Sent it, it's in the thread now.", "done": True}) + assert not dispatch_failed({"summary": "Done, I took care of that for you.", "done": True}) def test_recovery_task_verifies_before_repeating(): @@ -107,7 +107,7 @@ def test_dispatch_refused_when_no_dashboard_connected(monkeypatch): results = asyncio.run(run_browser_agents(tasks=[{"task": "go to example.com"}], model="sonnet")) assert len(results) == 1 assert results[0]["summary"].startswith("Error: no dashboard window is connected") - assert dispatch_failed(results[0]["summary"]) + assert dispatch_failed(results[0]) def test_send_probe_verdict_parsing_order_and_fail_closed():