From dc2a9fbb8d8ac9e43c2ec862888a892fe0d1bef3 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Wed, 26 Aug 2026 13:22:25 -0700 Subject: [PATCH] [eric] browser: a failed dispatch says whether anything changed, so a caller can branch instead of guessing (ENG-402) --- backend/apps/agents/browser/browser_agent.py | 8 ++- backend/apps/agents/browser/browser_loop.py | 12 ++-- .../apps/agents/browser/effect_disposition.py | 51 ++++++++++++++++ backend/tests/test_effect_disposition.py | 58 +++++++++++++++++++ 4 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 backend/apps/agents/browser/effect_disposition.py create mode 100644 backend/tests/test_effect_disposition.py diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 489f7149..410d1f5d 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_history import ( refusal_shaped_summary, PAGE_STATE_MARKER, ) +from backend.apps.agents.browser.effect_disposition import disposition_line, effect_disposition from backend.apps.agents.browser.browser_loop import ( LOOP_DETECTION_EXCLUDED_TOOLS, LOOP_HARD_CAP, @@ -3261,7 +3262,12 @@ async def run_browser_agent( summary=summary) final_status = "completed" if honest else "error" if not honest: - summary = f"I was not able to complete this task ({dishonest_reason})." + # The caller has to be able to DECIDE, and prose does not let it: "may not have gone + # out" leaves a parent choosing between stalling and a double-write (ENG-402). Say which + # of the three states this is, and what is safe to do next. + p_effect = effect_disposition(action_log, send_confirmed=send_confirmed) + summary = (f"I was not able to complete this task ({dishonest_reason}). " + f"{disposition_line(p_effect)}") logger.warning( f"[browser-agent {session_id}] completion gate caught a ghost: " f"model declared done but {dishonest_reason}; reporting as error" diff --git a/backend/apps/agents/browser/browser_loop.py b/backend/apps/agents/browser/browser_loop.py index 565b558a..1e368da0 100644 --- a/backend/apps/agents/browser/browser_loop.py +++ b/backend/apps/agents/browser/browser_loop.py @@ -222,7 +222,7 @@ def stagnation_exhausted(streak: int) -> bool: # --- completion honesty gate ---------------------------------------------- A model that ends its turn is NOT proof the goal happened. The worst ghost we measured: multi-minute runs where every tool errored, still reported "completed". This deterministic gate reality-checks the run before we let the status say "done", so a fake success is reported as the failure it actually is. # State-changing tools: a task that needed to DO something must land one of these. -P_PRODUCTIVE_TOOLS = { +STATE_CHANGING_TOOLS = { "BrowserClick", "BrowserClickIndex", "BrowserType", "BrowserNavigate", "BrowserPressKey", "BrowserScroll", "BrowserBatch", "BrowserActVerified", # Enumerated against the live dispatcher 2026-08-13 (ENG-297): these seven change state and were @@ -232,7 +232,7 @@ P_PRODUCTIVE_TOOLS = { "BrowserUploadFile", "BrowserSaveData", "BrowserRepeatFlow", } # Read/extract tools: a look-only task's evidence is that a read returned content. -P_READ_TOOLS = { +READ_ONLY_TOOLS = { "BrowserGetText", "BrowserGetElements", "BrowserListInteractives", "BrowserListRoutes", "BrowserReplayRoute", "BrowserScreenshot", "BrowserEvaluate", } @@ -426,14 +426,14 @@ def outcome_facts(action_log: list[dict]) -> dict: which is the failure mode this exists to remove. """ log = action_log or [] - mutations = [a for a in log if a.get("tool") in P_PRODUCTIVE_TOOLS] + mutations = [a for a in log if a.get("tool") in STATE_CHANGING_TOOLS] return { "calls": len(log), "mutations_attempted": len(mutations), "mutations_succeeded": len([a for a in mutations if a.get("ok")]), "reads_with_content": len([ a for a in log - if a.get("tool") in P_READ_TOOLS and a.get("ok") + if a.get("tool") in READ_ONLY_TOOLS and a.get("ok") and str(a.get("result_summary") or "").strip() ]), } @@ -484,7 +484,7 @@ def completion_is_honest( "check the page before trusting this") if not action_log: return False, "declared done without taking a single action" - actions = [a for a in action_log if a.get("tool") in P_PRODUCTIVE_TOOLS] + actions = [a for a in action_log if a.get("tool") in STATE_CHANGING_TOOLS] actions_ok = [a for a in actions if a.get("ok")] # Prestage seeds two reads into the log before the model ever runs. They are real page content, # so a model that ANSWERS from them did honest work (a read task needs no further tools). But a @@ -494,7 +494,7 @@ def completion_is_honest( p_answered = bool(str(summary or "").strip()) reads_ok = [ a for a in action_log - if a.get("tool") in P_READ_TOOLS and a.get("ok") + if a.get("tool") in READ_ONLY_TOOLS and a.get("ok") and (p_answered or not a.get("seeded")) and str(a.get("result_summary") or "").strip() ] diff --git a/backend/apps/agents/browser/effect_disposition.py b/backend/apps/agents/browser/effect_disposition.py new file mode 100644 index 00000000..c499700c --- /dev/null +++ b/backend/apps/agents/browser/effect_disposition.py @@ -0,0 +1,51 @@ +"""Did the run change anything out there? Answered as a value the caller can branch on. + +Haik, production 1.7.9, on a playlist edit: "Zero-confidence outcomes on a write path is the worst +possible spot to leave an agent in: I'm forced to choose between doing nothing and risking a +double-write." Our honesty gate was right to refuse to claim the send happened; what it gave back +was prose, so a parent agent could not tell "definitely did not happen" from "cannot tell" and had +no basis to decide between stalling and retrying (ENG-402). + +Fail-safe by construction: the read-only set is an ALLOWLIST, and anything not on it is assumed to +have changed something. A browser tool added tomorrow reads as UNKNOWN rather than as harmless. +""" + +from typing import Dict, List, Literal + +from typeguard import typechecked + +from backend.apps.agents.browser.browser_loop import READ_ONLY_TOOLS + +Disposition = Literal["applied", "none", "unknown"] + +NOTHING_HAPPENED = "none" +MAY_HAVE_HAPPENED = "unknown" +CONFIRMED = "applied" + + +@typechecked +def effect_disposition(action_log: List[Dict], send_confirmed: bool = False) -> Disposition: + """Whether anything left this machine, given what the run actually called. + + "applied" needs positive proof, not a tool that returned ok: a click reporting success is not + evidence the write landed, which is the precise gap that told a user "Done, I sent it for you" + while the post never arrived. + """ + p_changing = [a for a in action_log or [] if str(a.get("tool") or "") not in READ_ONLY_TOOLS] + if not p_changing: + return NOTHING_HAPPENED + if send_confirmed: + return CONFIRMED + return MAY_HAVE_HAPPENED + + +@typechecked +def disposition_line(disposition: Disposition) -> str: + """One sentence telling the caller what it is safe to do next.""" + if disposition == NOTHING_HAPPENED: + return ("Nothing was changed on the page, so this is safe to retry.") + if disposition == CONFIRMED: + return ("The change was confirmed on the page, so do not repeat it.") + return ("A change was attempted and could not be confirmed, so this may or may not have gone " + "through. Read the page and check before retrying; repeating it blind risks doing it " + "twice.") diff --git a/backend/tests/test_effect_disposition.py b/backend/tests/test_effect_disposition.py new file mode 100644 index 00000000..44a3a917 --- /dev/null +++ b/backend/tests/test_effect_disposition.py @@ -0,0 +1,58 @@ +"""A failed browser dispatch must resolve to a state the caller can branch on. + +Haik, production 1.7.9, on a Spotify playlist edit: "Zero-confidence outcomes on a write path is +the worst possible spot to leave an agent in: I'm forced to choose between doing nothing and +risking a double-write." The honesty gate was right to refuse to claim the send happened. What it +returned was prose, so a parent agent could not tell "definitely did not happen" from "cannot +tell", and a naive retry duplicates tracks (ENG-402). +""" + +from backend.apps.agents.browser.browser_loop import STATE_CHANGING_TOOLS, READ_ONLY_TOOLS +from backend.apps.agents.browser.effect_disposition import ( + CONFIRMED, MAY_HAVE_HAPPENED, NOTHING_HAPPENED, disposition_line, effect_disposition, +) + + +def p_log(*tools): + return [{"tool": t, "ok": True} for t in tools] + + +def test_a_run_that_only_looked_around_changed_nothing(): + assert effect_disposition([]) == NOTHING_HAPPENED + assert effect_disposition(p_log(*sorted(READ_ONLY_TOOLS))) == NOTHING_HAPPENED + + +def test_an_attempted_write_with_no_proof_is_unknown_not_failed(): + # This is the Spotify case: every click returned ok and the send was never confirmed. + assert effect_disposition(p_log("BrowserType", "BrowserClick")) == MAY_HAVE_HAPPENED + + +def test_ok_on_a_click_is_not_proof_the_write_landed(): + # A click reporting success is how a user got told "Done, I sent it for you" on a post that + # never arrived. Only the run's own confirmation signal earns "applied". + assert effect_disposition([{"tool": "BrowserClick", "ok": True}]) != CONFIRMED + assert effect_disposition([{"tool": "BrowserClick", "ok": True}], send_confirmed=True) == CONFIRMED + + +def test_an_unrecognised_tool_is_assumed_to_have_changed_something(): + # The read-only set is an ALLOWLIST. A browser tool added tomorrow must read as UNKNOWN, not as + # harmless: ENG-297 shipped seven state-changing tools that no classifier knew about. + assert effect_disposition(p_log("BrowserSomethingShippedNextWeek")) == MAY_HAVE_HAPPENED + for tool in STATE_CHANGING_TOOLS: + assert effect_disposition(p_log(tool)) == MAY_HAVE_HAPPENED, tool + + +def test_every_disposition_tells_the_caller_what_to_do_next(): + assert "safe to retry" in disposition_line(NOTHING_HAPPENED) + assert "do not repeat" in disposition_line(CONFIRMED) + unknown = disposition_line(MAY_HAVE_HAPPENED) + assert "check before retrying" in unknown and "twice" in unknown + assert "may or may not" in unknown, "the ambiguity has to be stated, not implied" + + +def test_the_failed_run_carries_the_disposition_home(): + src = open("backend/apps/agents/browser/browser_agent.py").read() + i = src.index("I was not able to complete this task") + body = src[i - 400:i + 300] + assert "effect_disposition(action_log" in body + assert "disposition_line(p_effect)" in body, "the sentence must reach the caller's summary"