From b161f19efe6e81722b0e40b3f6b54490b3c32e48 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Thu, 13 Aug 2026 11:19:50 -0700 Subject: [PATCH] [eric] browser: reading cannot complete a task that asked for a change (ENG-297) --- backend/apps/agents/browser/browser_agent.py | 4 +- backend/apps/agents/browser/browser_loop.py | 52 ++++++++++++ .../tests/test_browser_completion_honesty.py | 82 +++++++++++++++++++ 3 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 backend/tests/test_browser_completion_honesty.py diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 6022a836..7e27dc79 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -46,6 +46,7 @@ from backend.apps.agents.browser.browser_loop import ( completion_is_honest, deliverable_is_informational, interstitial_dismiss_target, + is_mutation_task, is_publish_task, is_removal_task, recoverable_tool_error, @@ -3116,7 +3117,8 @@ async def run_browser_agent( else: honest, dishonest_reason = completion_is_honest( action_log, publish_task=is_publish_task(skill_key_task), - send_confirmed=send_confirmed) + send_confirmed=send_confirmed, + mutation_task=is_mutation_task(skill_key_task)) final_status = "completed" if honest else "error" if not honest: summary = f"I was not able to complete this task ({dishonest_reason})." diff --git a/backend/apps/agents/browser/browser_loop.py b/backend/apps/agents/browser/browser_loop.py index add4c23c..05a7257f 100644 --- a/backend/apps/agents/browser/browser_loop.py +++ b/backend/apps/agents/browser/browser_loop.py @@ -225,6 +225,11 @@ def stagnation_exhausted(streak: int) -> bool: P_PRODUCTIVE_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 + # all missing, so a run whose only actions were a delete, an upload or an API write counted as + # having taken NO action at all, and "every state-changing action failed" could never fire on it. + "BrowserClickByName", "BrowserClickPoint", "BrowserDeleteItem", "BrowserApiWrite", + "BrowserUploadFile", "BrowserSaveData", "BrowserRepeatFlow", } # Read/extract tools: a look-only task's evidence is that a read returned content. P_READ_TOOLS = { @@ -338,6 +343,43 @@ def is_publish_task(task: str) -> bool: return not deliverable_is_informational("", task) +# Verbs that CHANGE the page, as opposed to leaving something in the world (is_publish_task) or +# merely acting on it. A run that reads perfectly and edits nothing has not done any of these. +P_MUTATION_INTENT_RE = re.compile( + r"\b(edit|delete|remove|change|update|rename|replace|deploy|redeploy|install|" + r"uninstall|enable|disable|toggle|upload|clear|save)\b", re.I) + +# A question ABOUT state, which can never be an instruction to change it. Kept local to +# is_mutation_task rather than widening P_INFO_ASK_RE, because that one also decides what the skill +# store records and this needs no say there. "can you delete X" is a request, not a question, so it +# is deliberately not matched. +P_STATE_QUESTION_RE = re.compile( + r"^(what|which|where|who|whose|when|why|is|are|does|do|did|was|were|can i|could i|" + r"how (?:many|much|do|does))\b", re.I) + + +def is_mutation_task(task: str) -> bool: + """A task whose deliverable is a CHANGED page, so reading cannot satisfy it. + + Measured 2026-08-13, 6 dispatches at a Monaco editor: a run with 3 successful reads and zero + edits returned "Task completed." and the honesty gate agreed, because any successful read + counted as evidence once a run took no productive action. That rule is correct for "what is on + this page" and wrong for "change this page", and nothing distinguished them. + + Same fail-safe direction as is_publish_task: the verbs are ordinary words in informational asks + ("what does the delete button say"), so an info ask is excluded and an explicit read-only + directive settles it. Worst case we skip the gate on a real edit; we never call a good read a + failure, which is the error that would make the gate untrustworthy. + """ + if not P_MUTATION_INTENT_RE.search(task or ""): + return False + if browser_send_parse.is_readonly(task or ""): + return False + if P_STATE_QUESTION_RE.match((task or "").strip()): + return False + return not deliverable_is_informational("", task) + + def deliverable_is_informational(summary: str, task: str = "") -> bool: """True if the run's final answer is GATHERED CONTENT (a list/report the model extracted or judged), not a short action confirmation. A deterministic replay @@ -367,6 +409,7 @@ def deliverable_is_informational(summary: str, task: str = "") -> bool: def completion_is_honest( action_log: list[dict], publish_task: bool = False, send_confirmed: bool = False, + mutation_task: bool = False, ) -> tuple[bool, str]: """Reality-check a run the model declared done. Returns (honest, reason). @@ -399,6 +442,15 @@ def completion_is_honest( ] if actions and not actions_ok: return False, "every state-changing action failed" + # A task that asked for a CHANGE cannot be satisfied by reading. BrowserEvaluate counts here + # even though it is filed as a read, because on an edit task running JS IS how the edit happens + # (the one honest dispatch in the ENG-297 run did exactly that, ~12 times); refusing it would + # fail the only agent that did the work, which is the false positive that discredits the gate. + if mutation_task and not actions_ok and not any( + a.get("tool") == "BrowserEvaluate" and a.get("ok") for a in action_log + ): + return False, ("the task asked for a change but no state-changing action succeeded; " + "nothing on the page was edited") if not actions and not reads_ok: return False, "only looked around: no action taken and no content read back" return True, "" diff --git a/backend/tests/test_browser_completion_honesty.py b/backend/tests/test_browser_completion_honesty.py new file mode 100644 index 00000000..365bd3d0 --- /dev/null +++ b/backend/tests/test_browser_completion_honesty.py @@ -0,0 +1,82 @@ +"""A completion claim must be backed by evidence appropriate to what the task asked for (ENG-297). + +Run: + backend/.venv/bin/python -m pytest backend/tests/test_browser_completion_honesty.py -v +""" + +from backend.apps.agents.browser.browser_loop import ( + completion_is_honest, + is_mutation_task, +) + + +def p_ok(tool, summary=""): + return {"tool": tool, "ok": True, "result_summary": summary} + + +# --- ENG-297: a read-only run may not satisfy a task that demanded a change. --- +# +# Measured 2026-08-13, 6 dispatches at a Monaco editor (Google Apps Script), task "delete two name +# references and redeploy". Dispatch 5 returned "Task completed." on 3 read-only calls and zero +# edits; dispatch 2 returned instructions to open devtools by hand, also on 2 read-only calls. Both +# passed the honesty gate, because `completion_is_honest` accepts ANY successful read as evidence +# once the run took no productive action. That is right for "what is on this page" and wrong for +# "change this page", and the gate had no way to tell them apart. +# +# The caller then relayed a fabricated result to the user as a security incident. So the cost of +# this hole is not a wasted run, it is a lie with our name on it. + + +def test_a_mutation_task_is_recognised_as_one(): + for task in [ + "delete the two name references from the script and redeploy it", + "edit line 3 and save", + "rename the project", + "remove that comment", + ]: + assert is_mutation_task(task), f"not recognised as a change: {task!r}" + + +def test_an_informational_ask_is_not_a_mutation_task(): + # The cry-wolf direction. These verbs appear in read-only asks constantly, and a false positive + # here turns a correct read into a reported failure, which is worse than the bug being fixed. + for task in [ + "what does the delete button say", + "is there an edit option on this page", + "read the first comment", + "find the save button and tell me where it is", + ]: + assert not is_mutation_task(task), f"an informational ask scored as a change: {task!r}" + + +def test_reads_alone_cannot_complete_a_mutation_task(): + """The exact dispatch-5 ghost: 3 successful reads, zero edits, 'Task completed.'""" + log = [ + p_ok("BrowserGetText", "function doPost() {"), + p_ok("BrowserListInteractives", "12 buttons"), + p_ok("BrowserScreenshot", "captured"), + ] + honest, reason = completion_is_honest(log, mutation_task=True) + assert not honest, "a run that changed nothing was allowed to claim it completed a change" + assert "no state-changing action" in reason, reason + + +def test_the_same_log_is_still_honest_for_a_read_only_task(): + """Both directions: the fix must not turn every look-only run into a failure.""" + log = [ + p_ok("BrowserGetText", "function doPost() {"), + p_ok("BrowserListInteractives", "12 buttons"), + ] + honest, reason = completion_is_honest(log, mutation_task=False) + assert honest and reason == "", reason + + +def test_a_real_edit_via_browserevaluate_still_completes(): + """Dispatch 1 did the work with BrowserEvaluate ~12 times. Flagging it would be the false + positive that makes the gate untrustworthy, so an Evaluate counts as a change on a change task.""" + log = [ + p_ok("BrowserGetText", "function doPost() {"), + p_ok("BrowserEvaluate", "editor.setValue applied"), + ] + honest, reason = completion_is_honest(log, mutation_task=True) + assert honest and reason == "", reason