fix: clean up tabs owned by each CLI invocation

SKILL.md

@@ -93,6 +93,15 @@ Cloud profile cookie sync reference: https://github.com/browser-use/browser-harn
 - Login walls: stop and ask. Exception: use available SSO automatically when Chrome is already signed in; still stop for passwords, MFA, consent, or ambiguous account choice.
 - Raw CDP is available with `cdp("Domain.method", ...)`.
 
+## Tab Lifecycle
+
+- Tabs created by `new_tab()` are closed automatically when the CLI invocation ends, including after an error.
+- If `new_tab()` reuses an existing blank page, that page is restored to `about:blank` instead of being closed.
+- To keep new tabs for a later invocation, call `keep_opened_tabs()` or set `BH_KEEP_TABS=1`. The later workflow is responsible for closing them explicitly.
+- Preservation applies to tabs created by the invocation. A pre-existing blank tab borrowed by `new_tab()` is still restored to `about:blank`.
+- Existing tabs and tabs opened by another process are never automatically closed.
+- Popups and pages opened indirectly with `window.open()` or `target=_blank` are not process-owned and are left open.
+
 ## Interaction Skills
 
 If you get stuck on a browser mechanic, check https://github.com/browser-use/browser-harness/tree/main/interaction-skills.

src/browser_harness/helpers.py

@@ -255,6 +255,11 @@ def capture_screenshot(path=None, full=False, max_dim=None):
 
 
 # --- tabs ---
+_OPENED_TABS = set()
+_REUSED_BLANK_TABS = {}
+_KEEP_OPENED_TABS = False
+
+
 def _is_agent_startup_placeholder(title, url):
     url = str(url or "")
     return str(title or "").startswith("Starting agent ") and (
@@ -314,16 +319,142 @@ def new_tab(url="about:blank"):
             cur = current_tab()
             cur_url = cur.get("url") or ""
             if cur_url in ("", "about:blank") or cur_url.startswith("about:blank#"):
+                target_id = cur.get("targetId") or cur.get("target_id")
+                _REUSED_BLANK_TABS.setdefault(target_id, cur_url or "about:blank")
                 goto_url(url)
-                return cur.get("targetId") or cur.get("target_id")
+                return target_id
         except Exception:
             pass
     tid = cdp("Target.createTarget", url="about:blank")["targetId"]
+    _OPENED_TABS.add(tid)
     switch_tab(tid)
     if url != "about:blank":
         goto_url(url)
     return tid
 
+
+def opened_tabs():
+    """Return target IDs created by new_tab() in this CLI process."""
+    return list(_OPENED_TABS)
+
+
+def keep_opened_tabs(keep=True):
+    """Opt out of automatic cleanup for tabs owned by this CLI process."""
+    global _KEEP_OPENED_TABS
+    _KEEP_OPENED_TABS = keep
+
+
+def _restore_reused_blank_tabs():
+    restored = []
+    failures = []
+    for target_id, original_url in list(_REUSED_BLANK_TABS.items()):
+        try:
+            switch_tab(target_id)
+            goto_url(original_url or "about:blank")
+            restored.append(target_id)
+            _REUSED_BLANK_TABS.pop(target_id, None)
+        except Exception as exc:
+            failures.append((target_id, exc))
+    return restored, failures
+
+
+def _move_to_keeper(target_ids):
+    """Move off an owned target before it closes; return protected IDs and failures."""
+    try:
+        current_id = current_tab()["targetId"]
+    except Exception as exc:
+        # Without the active target ID, any owned target could be the daemon's
+        # current session. Fail closed instead of risking a stale attachment.
+        return set(target_ids), [("current target", exc)]
+    if current_id not in target_ids:
+        return set(), []
+
+    keeper_id = None
+    try:
+        keeper_id = cdp("Target.createTarget", url="about:blank")["targetId"]
+        switch_tab(keeper_id)
+        # The keeper becomes the daemon's neutral anchor. It is intentionally
+        # not owned by this invocation and can be reused by the next new_tab().
+    except Exception as exc:
+        failures = [("keeper handoff", exc)]
+        if keeper_id:
+            try:
+                result = cdp("Target.closeTarget", targetId=keeper_id)
+                if not result.get("success", True):
+                    raise RuntimeError("Target.closeTarget returned false")
+            except Exception as close_exc:
+                # Retain ownership so a later invocation can retry cleanup.
+                _OPENED_TABS.add(keeper_id)
+                failures.append((keeper_id, close_exc))
+        # Keeping one page is better than leaving the daemon attached to a dead
+        # target. Other owned tabs can still be cleaned up safely.
+        return {current_id}, failures
+    return set(), []
+
+
+def _cleanup_error_message(failures):
+    details = ", ".join(f"{target}: {error}" for target, error in failures)
+    return f"tab cleanup incomplete ({details})"
+
+
+def close_opened_tabs(force=False):
+    """Close created tabs and restore blank tabs reused by this CLI process."""
+    if _KEEP_OPENED_TABS and not force:
+        # Keeping means hands off every tab this invocation touched, not just
+        # ones it created via Target.createTarget. new_tab() usually reuses
+        # the current blank tab instead of creating a new one (the daemon is
+        # commonly parked on a blank keeper between invocations), so most
+        # "kept" tabs in practice are reused-blank ones. Restoring them to
+        # blank here would silently defeat keep_opened_tabs() for exactly the
+        # common case a caller relies on it for.
+        _OPENED_TABS.clear()
+        _REUSED_BLANK_TABS.clear()
+        return []
+
+    _, failures = _restore_reused_blank_tabs()
+
+    target_ids = set(_OPENED_TABS)
+    if not target_ids:
+        if failures:
+            raise RuntimeError(_cleanup_error_message(failures))
+        return []
+
+    protected_ids, handoff_failures = _move_to_keeper(target_ids)
+    failures.extend(handoff_failures)
+    target_ids.difference_update(protected_ids)
+
+    closed = []
+    for target_id in list(target_ids):
+        last_error = None
+        for _ in range(2):
+            try:
+                result = cdp("Target.closeTarget", targetId=target_id)
+                if result.get("success", True):
+                    closed.append(target_id)
+                    last_error = None
+                    break
+                last_error = RuntimeError("Target.closeTarget returned false")
+            except Exception as exc:
+                last_error = exc
+        if last_error is None:
+            _OPENED_TABS.discard(target_id)
+        else:
+            failures.append((target_id, last_error))
+
+    if closed:
+        deadline = time.time() + 2.0
+        while time.time() < deadline:
+            try:
+                remaining = {tab["targetId"] for tab in list_tabs(include_chrome=True)}
+            except Exception:
+                break
+            if not remaining.intersection(closed):
+                break
+            time.sleep(0.05)
+    if failures:
+        raise RuntimeError(_cleanup_error_message(failures))
+    return closed
+
 def close_tab(target=None):
     """Close a tab. If `target` is omitted, closes the currently attached tab.
     Accepts a raw targetId string or a dict from list_tabs()/current_tab()."""

src/browser_harness/run.py

@@ -28,6 +28,7 @@
     sync_local_profile,
 )
 from . import auth, telemetry
+from . import helpers as helper_module
 from .helpers import *
 
 HELP = """Browser Harness
@@ -343,7 +344,21 @@ def _run(args):
             start_remote_daemon(NAME)
         ensure_daemon()
     _install_helper_trace()
-    exec(code, globals())
+    try:
+        exec(code, globals())
+    finally:
+        keep_tabs = os.environ.get("BH_KEEP_TABS", "").lower()
+        try:
+            if keep_tabs in {"1", "true", "yes"}:
+                helper_module.keep_opened_tabs()
+            helper_module.close_opened_tabs()
+        except Exception as exc:
+            # Cleanup must never replace the browser task's result or exception.
+            print(f"Warning: automatic tab cleanup failed: {exc}", file=sys.stderr)
+        finally:
+            # main() can be exercised repeatedly in one Python process by tests
+            # and embedders, so keep the opt-out scoped to one invocation.
+            helper_module.keep_opened_tabs(False)
 
 
 if __name__ == "__main__":

tests/unit/test_run.py

@@ -7,6 +7,17 @@
 from browser_harness import run
 
 
+@pytest.fixture(autouse=True)
+def reset_tab_ownership():
+    run.helper_module._OPENED_TABS.clear()
+    run.helper_module._REUSED_BLANK_TABS.clear()
+    run.helper_module.keep_opened_tabs(False)
+    yield
+    run.helper_module._OPENED_TABS.clear()
+    run.helper_module._REUSED_BLANK_TABS.clear()
+    run.helper_module.keep_opened_tabs(False)
+
+
 def test_stdin_executes_code():
     stdout = StringIO()
     fake_stdin = StringIO("print('hello from stdin')")
@@ -21,6 +32,129 @@ def test_stdin_executes_code():
     assert stdout.getvalue().strip() == "hello from stdin"
 
 
+def test_stdin_closes_owned_tabs_after_success():
+    with patch.object(sys, "argv", ["browser-harness"]), \
+         patch("browser_harness.run.ensure_daemon"), \
+         patch("browser_harness.run.print_update_banner"), \
+         patch("browser_harness.run.helper_module.close_opened_tabs") as cleanup, \
+         patch("sys.stdin", StringIO("x = 1")):
+        run.main()
+
+    cleanup.assert_called_once_with()
+
+
+def test_stdin_closes_owned_tabs_after_error():
+    with patch.object(sys, "argv", ["browser-harness"]), \
+         patch("browser_harness.run.ensure_daemon"), \
+         patch("browser_harness.run.print_update_banner"), \
+         patch("browser_harness.run.helper_module.close_opened_tabs") as cleanup, \
+         patch("sys.stdin", StringIO("raise RuntimeError('boom')")), \
+         pytest.raises(RuntimeError, match="boom"):
+        run.main()
+
+    cleanup.assert_called_once_with()
+
+
+def test_cleanup_failure_does_not_mask_task_error():
+    stderr = StringIO()
+    with patch.object(sys, "argv", ["browser-harness"]), \
+         patch("browser_harness.run.ensure_daemon"), \
+         patch("browser_harness.run.print_update_banner"), \
+         patch(
+             "browser_harness.run.helper_module.close_opened_tabs",
+             side_effect=RuntimeError("cleanup failed"),
+         ), \
+         patch("sys.stdin", StringIO("raise RuntimeError('task failed')")), \
+         patch("sys.stderr", stderr), \
+         pytest.raises(RuntimeError, match="task failed"):
+        run.main()
+
+    assert "automatic tab cleanup failed: cleanup failed" in stderr.getvalue()
+
+
+def test_real_cleanup_runs_after_task_error():
+    closed = []
+
+    def fake_cdp(method, **kwargs):
+        if method == "Target.closeTarget":
+            closed.append(kwargs["targetId"])
+        return {"success": True}
+
+    with patch.object(sys, "argv", ["browser-harness"]), \
+         patch("browser_harness.run.ensure_daemon"), \
+         patch("browser_harness.run.print_update_banner"), \
+         patch.object(run.helper_module, "_OPENED_TABS", {"owned"}), \
+         patch.object(run.helper_module, "_REUSED_BLANK_TABS", {}), \
+         patch.object(run.helper_module, "_KEEP_OPENED_TABS", False), \
+         patch("browser_harness.helpers.current_tab", return_value={"targetId": "survivor"}), \
+         patch("browser_harness.helpers.cdp", side_effect=fake_cdp), \
+         patch("browser_harness.helpers.list_tabs", return_value=[]), \
+         patch("sys.stdin", StringIO("raise RuntimeError('task failed')")), \
+         pytest.raises(RuntimeError, match="task failed"):
+        run.main()
+
+    assert closed == ["owned"]
+
+
+def test_keep_opened_tabs_is_scoped_to_one_main_invocation():
+    with patch.object(sys, "argv", ["browser-harness"]), \
+         patch("browser_harness.run.ensure_daemon"), \
+         patch("browser_harness.run.print_update_banner"), \
+         patch("sys.stdin", StringIO("keep_opened_tabs()")):
+        run.main()
+
+    assert run.helper_module._KEEP_OPENED_TABS is False
+
+
+@pytest.mark.parametrize("value", ["1", "true", "TRUE", "yes", "YES"])
+def test_bh_keep_tabs_skips_automatic_cleanup(monkeypatch, value):
+    monkeypatch.setenv("BH_KEEP_TABS", value)
+    with patch.object(sys, "argv", ["browser-harness"]), \
+         patch("browser_harness.run.ensure_daemon"), \
+         patch("browser_harness.run.print_update_banner"), \
+         patch("browser_harness.run.helper_module.keep_opened_tabs") as keep, \
+         patch("browser_harness.run.helper_module.close_opened_tabs") as cleanup, \
+         patch("sys.stdin", StringIO("x = 1")):
+        run.main()
+
+    keep.assert_any_call()
+    keep.assert_called_with(False)
+    cleanup.assert_called_once_with()
+
+
+def test_cloud_admin_code_still_runs_tab_finalizer():
+    with patch.object(sys, "argv", ["browser-harness"]), \
+         patch("browser_harness.run.print_update_banner"), \
+         patch("browser_harness.run.start_remote_daemon"), \
+         patch("browser_harness.run.helper_module.close_opened_tabs") as cleanup, \
+         patch("sys.stdin", StringIO("start_remote_daemon('profile')")):
+        run.main()
+
+    cleanup.assert_called_once_with()
+
+
+def test_bh_keep_tabs_releases_owned_and_borrowed_blank_tabs_untouched(monkeypatch):
+    # Regression: BH_KEEP_TABS must protect reused-blank tabs too, since
+    # new_tab() usually reuses the current blank tab instead of creating a
+    # new one, so most "kept" tabs in practice go through that path.
+    monkeypatch.setenv("BH_KEEP_TABS", "1")
+    run.helper_module._OPENED_TABS.add("created")
+    run.helper_module._REUSED_BLANK_TABS["blank"] = "about:blank"
+
+    with patch.object(sys, "argv", ["browser-harness"]), \
+         patch("browser_harness.run.ensure_daemon"), \
+         patch("browser_harness.run.print_update_banner"), \
+         patch("browser_harness.helpers.switch_tab") as switch, \
+         patch("browser_harness.helpers.goto_url") as restore, \
+         patch("sys.stdin", StringIO("x = 1")):
+        run.main()
+
+    switch.assert_not_called()
+    restore.assert_not_called()
+    assert run.helper_module.opened_tabs() == []
+    assert run.helper_module._REUSED_BLANK_TABS == {}
+
+
 def test_c_flag_is_rejected():
     with patch.object(sys, "argv", ["browser-harness", "-c", "print('old path')"]), \
          patch("sys.stdin", StringIO("print('ignored')")):