From 0d66e318a14ef322dcd8849501c6e9bea537436b Mon Sep 17 00:00:00 2001 From: ciregenz Date: Sun, 7 Jun 2026 15:38:56 -0700 Subject: [PATCH] [eric] browser: only the Send ends a skill's replay prefix, not the composer opener --- backend/apps/agents/browser/browser_agent.py | 22 ++++++++--------- .../agents/browser/browser_batch_replay.py | 15 ++++++++++++ backend/apps/agents/browser/browser_skills.py | 13 ++++++---- backend/tests/test_browser_skills.py | 24 ++++++++++++++++--- 4 files changed, 54 insertions(+), 20 deletions(-) diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 9a8ee351..7b3b73d4 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -43,18 +43,16 @@ from backend.apps.agents.browser.browser_loop import ( ) from backend.apps.agents.browser.browser_validator import adjudicate_stuck -# Send-skill PREFIX replay (replay learned steps UP TO the gated Send, never the -# Send). Settle-before-step + detour-pruning landed, but a live proof (r93/r94) -# showed two upstream blockers still keep it 0-for-N, so it stays parked: -# 1. is_send_step flags the composer OPENER ("Message") as irreversible, so a -# clean skill has no safe prefix to replay (unsafe_i=0). Openers are -# reversible; only the Send is not. Fixing this (shared with the send-guard) -# is the real unlock. -# 2. the recorder sometimes captures a brittle long entity-card name that does -# not match at replay time, failing the prefix and quarantining the skill. -# The mechanism itself FIRES and is safe (clean fallback, send still verified); -# flip back on once #1 lands. -_PREFIX_REPLAY_ENABLED = False +# Send-skill PREFIX replay: replay the learned steps UP TO the irreversible Send +# mechanically, then hand the gated Send to the live model (the Send is NEVER +# replayed). Unlocked by first_unsafe_step now using is_replay_boundary, a +# composer OPENER ("Message"/"DM" click) is reversible and no longer ends the +# prefix (the r93/r94 blocker), so a clean skill replays [open composer ...] and +# only the real Send crosses to the live agent. A still-brittle recorded name +# self-heals: the prefix step fails, the skill quarantines, and the run falls +# back to the full agent (send still verified). Both fixes that parked it +# (settle-before-step, detour-pruning) already landed. +_PREFIX_REPLAY_ENABLED = True # Single actions the model could have folded into one BrowserBatch turn; # reads, waits, and the batch tools themselves don't count toward the streak. diff --git a/backend/apps/agents/browser/browser_batch_replay.py b/backend/apps/agents/browser/browser_batch_replay.py index b1d7066d..7ec54662 100644 --- a/backend/apps/agents/browser/browser_batch_replay.py +++ b/backend/apps/agents/browser/browser_batch_replay.py @@ -115,6 +115,21 @@ _LIVE_IRREVERSIBLE_RE = re.compile( ) +def is_replay_boundary(step: dict) -> bool: + """The genuinely irreversible step where a learned skill's mechanical replay + must STOP and hand to the live agent. Same as is_send_step EXCEPT a composer + OPENER ('Message'/'DM' click) is reversible and NOT a boundary: the prefix can + mechanically open the composer, and only the real Send (and composer typing) + crosses to the live model. Uses the same opener-excluded wordlist the live + send-guard already trusts, so a recorded Send still stops the prefix.""" + action = step.get("action") + if action == "click" and _LIVE_IRREVERSIBLE_RE.search(str(step.get("name") or "")): + return True + if action == "type" and _COMPOSE_SEL_RE.search(str(step.get("selector") or "")): + return True + return False + + def live_batch_guard(actions, seen_lines, composer_pending: bool = False) -> str: """Reason string if a live BrowserBatch carries an irreversible step, else ''. diff --git a/backend/apps/agents/browser/browser_skills.py b/backend/apps/agents/browser/browser_skills.py index d07315f9..99af2c1f 100644 --- a/backend/apps/agents/browser/browser_skills.py +++ b/backend/apps/agents/browser/browser_skills.py @@ -342,9 +342,12 @@ def replay_settle_target(step: dict) -> str | None: def first_unsafe_step(steps: list[dict]) -> tuple[int, str]: - """Index of the first outward-facing step (click Send/Submit/Pay, type into - a composer), -1 if none. Reuses the batch replayer's wordlist so there is - exactly one definition of "irreversible".""" + """Index of the first GENUINELY irreversible step (click Send/Submit/Pay, type + into a composer), -1 if none. This is the prefix-replay/batch boundary, so a + composer OPENER ('Message'/'DM' click) is NOT it: opening the box is + reversible and replays fine, only the real Send crosses to the live agent. + Uses is_replay_boundary (the opener-excluded wordlist) for exactly one + definition of the boundary; is_send_step stays conservative for the live guard.""" from backend.apps.agents.browser import browser_batch_replay for i, s in enumerate(steps): tool = s.get("tool", "") @@ -359,7 +362,7 @@ def first_unsafe_step(steps: list[dict]) -> tuple[int, str]: probe = {"action": "click", "name": name} elif tool == "BrowserType": probe = {"action": "type", "selector": p.get("selector") or ""} - if probe and browser_batch_replay.is_send_step(probe): + if probe and browser_batch_replay.is_replay_boundary(probe): what = probe.get("name") or probe.get("selector") return i, f"step {i+1} looks irreversible/outward-facing ({what!r})" return -1, "" @@ -761,7 +764,7 @@ def render_route_hint(skill: dict, task: str, score: float) -> tuple[str, list[t if s.get("tool") in ("BrowserClickByName", "BrowserClick"): p = s.get("params", {}) or {} name = p.get("name") or p.get("selector") or "" - if len(name) <= 40 and browser_batch_replay.is_send_step({"action": "click", "name": name}): + if len(name) <= 40 and browser_batch_replay.is_replay_boundary({"action": "click", "name": name}): mark = " [IRREVERSIBLE: do this SOLO with `expect` proof, never in a batch]" lines.append(f"{i + 1}. {_hint_step_line(s, values)}{mark}") trust = "proven by a verified rerun" if skill.get("state") == _TRUSTED else "from one verified success" diff --git a/backend/tests/test_browser_skills.py b/backend/tests/test_browser_skills.py index b39acb82..ec52d408 100644 --- a/backend/tests/test_browser_skills.py +++ b/backend/tests/test_browser_skills.py @@ -538,9 +538,27 @@ def test_long_card_blob_click_names_are_not_send_steps(): {"tool": "BrowserClickByName", "params": {"name": "Send"}}, ] i, why = first_unsafe_step(flow) - # the blob at step 1 must NOT be flagged; the conservative cut lands on the - # short "Message" composer-opener (shared wordlist), keeping sends live - assert i == 2, f"expected the Message click flagged, got {i}: {why}" + # the 100ch blob at step 1 isn't flagged (len guard); the composer OPENER + # "Message" at step 2 isn't either (reversible, it just opens the box); the + # boundary is the real "Send" at step 3, so the prefix can open the composer. + assert i == 3, f"expected the Send click flagged, got {i}: {why}" + + +def test_composer_opener_is_not_the_replay_boundary(): + from backend.apps.agents.browser.browser_skills import first_unsafe_step, replay_safety + # clicking "Message"/"DM" just OPENS the composer (reversible); the boundary + # is the real Send, so the open-the-composer steps can mechanically replay. + flow = [ + {"tool": "BrowserClickByName", "params": {"role": "link", "name": "Message"}}, + {"tool": "BrowserClickByName", "params": {"role": "textbox", "name": "Write a message…"}}, + {"tool": "BrowserClickByName", "params": {"role": "button", "name": "Send"}}, + ] + i, why = first_unsafe_step(flow) + assert i == 2 and "irreversible" in why # the Send (step 3), not Message + # a skill that only OPENS a composer (no send) is fully safe to replay + opener_only = flow[:2] + assert first_unsafe_step(opener_only) == (-1, "") + assert replay_safety(opener_only) == (True, "") def test_distill_maps_batched_click_index_to_click_by_name():