fix: attach() closes executor on failure; guard session adoption

docs/superpowers/plans/2026-07-07-wabot-modernization.md

@@ -1126,7 +1126,6 @@ git commit -m "feat: ManagedService spawns detached driver services that outlive
 `tests/unit/test_reattach.py`. Constructing a `Remote` performs **no HTTP until the first command** (verified), so adoption can be unit-tested without a server; the dead-session path monkeypatches the probe:
 
 ```python
-import pytest
 from selenium.common.exceptions import WebDriverException
 
 from wabot.hosts import ReattachingRemote, attach, build_options
@@ -1134,6 +1133,8 @@ from wabot.hosts import ReattachingRemote, attach, build_options
 
 class TestReattachingRemote:
     def test_adopts_session_id_without_creating_a_session(self):
+        # constructing against a closed port (127.0.0.1:1) proves no HTTP happens
+        # until the first command
         driver = ReattachingRemote(
             "http://127.0.0.1:1", "saved-session-id", options=build_options("chromium")
         )
@@ -1168,6 +1169,19 @@ class TestAttach:
     def test_returns_none_when_server_is_unreachable(self):
         # no server on port 1: connection refused is immediate; attach must not raise
         assert attach("http://127.0.0.1:1", "sid", "chromium") is None
+
+    def test_dead_session_closes_executor(self, monkeypatch):
+        from selenium.webdriver.remote.remote_connection import RemoteConnection
+
+        closed = []
+        monkeypatch.setattr(RemoteConnection, "close", lambda self: closed.append(1))
+
+        def boom(self):
+            raise WebDriverException("invalid session id")
+
+        monkeypatch.setattr(ReattachingRemote, "current_url", property(boom))
+        assert attach("http://127.0.0.1:1", "sid", "chromium") is None
+        assert closed == [1]  # executor was closed, not leaked
 ```
 
 - [ ] **Step 2: Run tests to verify they fail**
@@ -1182,8 +1196,14 @@ Append to `src/wabot/hosts.py`; add these imports to the top block:
 ```python
 from selenium.common.exceptions import WebDriverException
 from selenium.webdriver.remote.webdriver import WebDriver as RemoteWebDriver
+from urllib3.exceptions import HTTPError as _Urllib3HTTPError
 ```
 
+(The `urllib3` import is the resolved form of the note below: selenium's HTTP
+layer surfaces `test_returns_none_when_server_is_unreachable`'s connection
+refusal as a `urllib3.exceptions.MaxRetryError`, which is **not** an `OSError`,
+so it must be caught explicitly. This was confirmed against selenium 4.45.)
+
 Appended code:
 
 ```python
@@ -1201,6 +1221,11 @@ class ReattachingRemote(RemoteWebDriver):
         # calling start_session, so stash the id under a private name.
         self._reattach_session_id = session_id
         super().__init__(command_executor=command_executor, options=options)
+        if self.session_id != session_id:
+            raise RuntimeError(
+                "session adoption failed: selenium's start_session contract "
+                "changed; pin selenium <5 or update ReattachingRemote"
+            )
 
     def start_session(self, capabilities: dict) -> None:
         # Skip Command.NEW_SESSION entirely; adopt the saved session.
@@ -1216,31 +1241,28 @@ def attach(executor_url: str, session_id: str, browser: str):
         return driver
     except WebDriverException as ex:
         LOGGER.warning("saved session %s is dead: %s", session_id, ex)
+        driver.command_executor.close()  # do not leak the keep-alive socket
         return None
-    except OSError as ex:  # server itself unreachable (connection refused, timeout)
+    except (OSError, _Urllib3HTTPError) as ex:  # server unreachable (connection refused, timeout)
         LOGGER.warning("no server at %s: %s", executor_url, ex)
+        driver.command_executor.close()  # do not leak the keep-alive socket
         return None
 ```
 
-Note: selenium's HTTP layer (urllib3) can surface connection failures as
-`urllib3.exceptions.HTTPError` subclasses that are **not** `OSError`. If
-`test_returns_none_when_server_is_unreachable` fails with an unhandled
-`MaxRetryError`, add it explicitly:
-
-```python
-from urllib3.exceptions import HTTPError as _Urllib3HTTPError
-```
-
-and change the last except clause to `except (OSError, _Urllib3HTTPError) as ex:`.
-Whichever variant makes the test pass is the correct final code.
+`driver.quit()` cannot be used to release the executor on the failure paths:
+on a dead session it issues a DELETE that re-raises. Closing the executor
+directly (`driver.command_executor.close()`) releases urllib3's pooled
+keep-alive connection without another round-trip. The `session_id` tripwire in
+`__init__` raises (not `assert`, so it survives `python -O`) to turn a future
+silent adoption failure into a loud one at construction time.
 
 - [ ] **Step 4: Run tests to verify they pass**
 
 Run: `uv run pytest tests/unit/test_reattach.py -v`
-Expected: 5 passed
+Expected: 6 passed
 
 Run: `uv run pytest`
-Expected: all unit tests so far pass (pacing 7, sessions 14, hosts 19, reattach 5 = 45)
+Expected: all unit tests so far pass (pacing 7, sessions 14, hosts 19, reattach 6 = 46)
 
 - [ ] **Step 5: Commit**
 
@@ -2765,7 +2787,7 @@ Run: `uv run pytest tests/unit/test_api.py -v`
 Expected: 11 passed
 
 Run: `uv run pytest`
-Expected: full unit suite passes (106 tests: pacing 7, sessions 14, hosts 19, reattach 5, fields 11, page 23, screenshot 3, browser 13, api 11)
+Expected: full unit suite passes (107 tests: pacing 7, sessions 14, hosts 19, reattach 6, fields 11, page 23, screenshot 3, browser 13, api 11)
 
 - [ ] **Step 5: Commit**
 

src/wabot/hosts.py

@@ -236,6 +236,11 @@ class ReattachingRemote(RemoteWebDriver):
         # calling start_session, so stash the id under a private name.
         self._reattach_session_id = session_id
         super().__init__(command_executor=command_executor, options=options)
+        if self.session_id != session_id:
+            raise RuntimeError(
+                "session adoption failed: selenium's start_session contract "
+                "changed; pin selenium <5 or update ReattachingRemote"
+            )
 
     def start_session(self, capabilities: dict) -> None:
         # Skip Command.NEW_SESSION entirely; adopt the saved session.
@@ -251,7 +256,9 @@ def attach(executor_url: str, session_id: str, browser: str):
         return driver
     except WebDriverException as ex:
         LOGGER.warning("saved session %s is dead: %s", session_id, ex)
+        driver.command_executor.close()
         return None
     except (OSError, _Urllib3HTTPError) as ex:  # server unreachable (connection refused, timeout)
         LOGGER.warning("no server at %s: %s", executor_url, ex)
+        driver.command_executor.close()
         return None

tests/unit/test_reattach.py

@@ -5,6 +5,8 @@ from wabot.hosts import ReattachingRemote, attach, build_options
 
 class TestReattachingRemote:
     def test_adopts_session_id_without_creating_a_session(self):
+        # constructing against a closed port (127.0.0.1:1) proves no HTTP happens
+        # until the first command
         driver = ReattachingRemote(
             "http://127.0.0.1:1", "saved-session-id", options=build_options("chromium")
         )
@@ -39,3 +41,16 @@ class TestAttach:
     def test_returns_none_when_server_is_unreachable(self):
         # no server on port 1: connection refused is immediate; attach must not raise
         assert attach("http://127.0.0.1:1", "sid", "chromium") is None
+
+    def test_dead_session_closes_executor(self, monkeypatch):
+        from selenium.webdriver.remote.remote_connection import RemoteConnection
+
+        closed = []
+        monkeypatch.setattr(RemoteConnection, "close", lambda self: closed.append(1))
+
+        def boom(self):
+            raise WebDriverException("invalid session id")
+
+        monkeypatch.setattr(ReattachingRemote, "current_url", property(boom))
+        assert attach("http://127.0.0.1:1", "sid", "chromium") is None
+        assert closed == [1]  # executor was closed, not leaked