fix: close_tab() recovers session state when closing the currently attached tab

[mvanhorn]

Summary

close_tab() now switches the harness to a surviving real tab (or a fresh about:blank) before closing the currently attached tab, so the daemon session never points at a dead target.

Why this matters

Before this change, the no-arg form of close_tab() issued Target.closeTarget against the attached targetId without updating the daemon session that switch_tab() previously registered. After close_tab() the daemon still believed the now-dead targetId/sessionId was active, so the next call into js(), page_info(), current_tab(), or _mark_tab() landed on a dead session and failed until the caller manually switch_tab()'d.

Repro:

new_tab("https://example.com")
close_tab()
page_info()  # fails: not attached to live tab

Maintainer @sauravpanda filed #379 with a clear cause + Option (a) fix sketch.

Changes

• close_tab(target=None) now detects whether the target equals the currently attached tab. If so, it picks a surviving real tab via list_tabs(include_chrome=False) and switch_tab()s to it before closing; if no other real tab exists, it opens a fresh about:blank first. The fast path for explicit non-current targets is unchanged.
• Error path: if Target.closeTarget fails after the switch, the original session is restored rather than left half-migrated.

Testing

pytest tests/unit/test_helpers.py — 22/22 pass. Added 4 new test cases:

• Happy path: two tabs, no-arg close, current tab + daemon session land on the survivor.
• Single-tab fallback: about:blank created, session points at it, page_info() succeeds.
• Explicit non-current close: current tab + session unchanged.
• Error path: simulated close failure restores original session intact.

Fixes #379

---

Summary by cubic

Fixes a bug where closing the currently attached tab left the session pointing at a dead target. Now close_tab() switches to a live tab (or creates about:blank) before closing and cleans up on failure so follow-up calls work. Fixes #379.

• Bug Fixes
  • When closing the current tab, switch to a surviving real tab first; if none, open about:blank.
  • If closing a non-current tab, leave the current session unchanged.
  • If close fails after switching, restore the original session and best‑effort close any fallback tab; treat Target.closeTarget responses with success: false as failures.
  • Added unit tests covering survivor switch, blank fallback, non-current close, and failure rollback/cleanup.

^{Written for commit f33ea09. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found across 2 files

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[mvanhorn]

Good catch by cubic, the leak was real. Fixed in f33ea09: the single-tab fallback tab id is tracked and best-effort closed when Target.closeTarget fails, before re-raising, and success: false responses are now treated as failures too. Added fake-CDP tests for both failure modes; 24 unit tests pass.

[cubic-dev-ai]

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/browser_harness/helpers.py">

<violation number="1" location="src/browser_harness/helpers.py:364">
P2: The rollback path in `close_tab()` uses a bare `raise` in the `except` handler, which makes it fragile if `switch_tab(cur)` raises a second exception. If `_close_target` fails because the tab is already gone, `switch_tab(cur)` will raise its own error, mask the original close error, and skip the fallback cleanup entirely. Consider capturing the original exception and making the rollback best-effort so both the original error and cleanup run reliably.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The rollback path in close_tab() uses a bare raise in the except handler, which makes it fragile if switch_tab(cur) raises a second exception. If _close_target fails because the tab is already gone, switch_tab(cur) will raise its own error, mask the original close error, and skip the fallback cleanup entirely. Consider capturing the original exception and making the rollback best-effort so both the original error and cleanup run reliably.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/browser_harness/helpers.py, line 364:

<comment>The rollback path in `close_tab()` uses a bare `raise` in the `except` handler, which makes it fragile if `switch_tab(cur)` raises a second exception. If `_close_target` fails because the tab is already gone, `switch_tab(cur)` will raise its own error, mask the original close error, and skip the fallback cleanup entirely. Consider capturing the original exception and making the rollback best-effort so both the original error and cleanup run reliably.</comment>

<file context>
@@ -339,21 +345,27 @@ def close_tab(target=None):
+        _close_target(target_id)
     except Exception:
         switch_tab(cur)
+        if fallback_id is not None:
+            try:
+                _close_target(fallback_id)
</file context>