fix(client): clean up provider when session startup fails

src/openenv/core/env_client.py

@@ -415,7 +415,11 @@ def _start_provider_if_needed(self) -> None:
         self._set_base_url(base_url)
 
     def _create_session_client(self) -> "EnvClient[Any, Any, Any]":
-        self._start_provider_if_needed()
+        try:
+            self._start_provider_if_needed()
+        except Exception:
+            self._stop_provider_best_effort()
+            raise
         if self._base_url is None:
             raise RuntimeError("EnvClient has no base URL.")
 

tests/test_core/test_generic_client.py

@@ -134,7 +134,10 @@ def stop_container(self):
             await client.connect()
 
     @pytest.mark.asyncio
-    async def test_new_session_reuses_provider_server(self, mock_provider):
+    @pytest.mark.parametrize("connect_parent", [False, True])
+    async def test_new_session_reuses_provider_server(
+        self, mock_provider, connect_parent
+    ):
         """Child sessions connect to the same server without owning the provider."""
         websockets = []
 
@@ -145,21 +148,52 @@ async def fake_ws_connect(*args, **kwargs):
 
         with patch("openenv.core.env_client.ws_connect", side_effect=fake_ws_connect):
             client = GenericEnvClient(provider=mock_provider)
-            await client.connect()
+            if connect_parent:
+                await client.connect()
             session = await client.new_session()
 
             assert isinstance(session, GenericEnvClient)
             assert session is not client
             assert session._provider is None
             assert session._base_url == "http://localhost:8000"
             assert session._ws_url == "ws://localhost:8000/ws"
-            assert len(websockets) == 2
+            assert len(websockets) == 1 + int(connect_parent)
             mock_provider.start_container.assert_called_once_with()
 
             await client.close()
 
         mock_provider.stop_container.assert_called_once_with()
 
+    @pytest.mark.asyncio
+    @pytest.mark.parametrize(
+        "start_method,stop_method",
+        [("start_container", "stop_container"), ("start", "stop")],
+    )
+    @pytest.mark.parametrize("cleanup_fails", [False, True])
+    async def test_new_session_stops_provider_when_readiness_fails(
+        self, start_method, stop_method, cleanup_fails
+    ):
+        provider = Mock(spec=[start_method, stop_method, "wait_for_ready"])
+        start = getattr(provider, start_method)
+        stop = getattr(provider, stop_method)
+        start.return_value = "http://localhost:8000"
+        error = TimeoutError("environment never became ready")
+        provider.wait_for_ready.side_effect = error
+        if cleanup_fails:
+            stop.side_effect = RuntimeError("cleanup failed")
+        client = GenericEnvClient(provider=provider)
+
+        with patch("openenv.core.env_client.ws_connect", AsyncMock()) as connect:
+            with pytest.raises(TimeoutError) as exc_info:
+                await client.new_session()
+
+        assert exc_info.value is error
+        start.assert_called_once_with()
+        stop.assert_called_once_with()
+        connect.assert_not_called()
+        assert client.base_url is None
+        assert client._child_clients == []
+
     @pytest.mark.asyncio
     async def test_close_stops_provider_when_child_close_raises(self, mock_provider):
         """Parent cleanup continues even when a child session close fails."""