Address lifespan shutdown review feedback

Author: alain-sv

docs/CHANGELOG.md

@@ -14,11 +14,11 @@ All notable changes to this project will be documented in this file.
 
 ### Changed
 
-- **FastAPI lifespan cleanup** — Controller shutdown now cancels and awaits the scheduled-step background loop, then closes the shared async HTTP event client.
+- **FastAPI lifespan cleanup** — Controller shutdown now cancels the scheduled-step background loop and waits briefly for it to stop.
 
 ### Tests
 
-- `tests/test_server.py` — scheduler task cancellation and HTTP client cleanup during FastAPI lifespan shutdown.
+- `tests/test_server.py` — scheduler task cancellation and bounded shutdown waiting during FastAPI lifespan shutdown.
 
 ## [1.1.1] - 2026-05-20
 

src/supervaizer/server.py

@@ -37,7 +37,6 @@
 
 from supervaizer.__version__ import VERSION
 from supervaizer.account import Account
-from supervaizer.account_service import close_httpx_client
 from supervaizer.agent import (
     Agent,
 )  # <-- MODIFIED: removed AdminIPAllowlistMiddleware, create_admin_routes imports
@@ -77,6 +76,7 @@
 insp = inspect
 
 T = TypeVar("T")
+SCHEDULED_STEP_SHUTDOWN_TIMEOUT_SECONDS = 5.0
 
 # Additional imports for server persistence
 
@@ -530,15 +530,20 @@ async def _lifespan(_app: FastAPI) -> AsyncIterator[None]:
             try:
                 yield
             finally:
-                # Let the scheduler observe cancellation instead of leaving a
-                # pending task attached to the event loop.
+                # Give the scheduler a bounded chance to observe cancellation.
                 scheduled_step_task.cancel()
-                with suppress(asyncio.CancelledError):
-                    await scheduled_step_task
-
-                # The event client is process-wide; close it once the app has
-                # stopped accepting controller work.
-                await close_httpx_client()
+                done, pending = await asyncio.wait(
+                    {scheduled_step_task},
+                    timeout=SCHEDULED_STEP_SHUTDOWN_TIMEOUT_SECONDS,
+                )
+                if pending:
+                    log.warning(
+                        "[Scheduled step] Shutdown timed out while waiting for "
+                        "the scheduler task to stop"
+                    )
+                if done:
+                    with suppress(asyncio.CancelledError):
+                        await scheduled_step_task
 
         app = FastAPI(
             lifespan=_lifespan,

tests/test_server.py

@@ -25,6 +25,7 @@
 from fastapi.testclient import TestClient
 from rich import inspect
 
+import supervaizer.server as server_module
 from supervaizer import Server
 from supervaizer.__version__ import VERSION
 from supervaizer.agent import Agent
@@ -407,19 +408,22 @@ def fake_create_task(
         created_task_names.append(name)
         return scheduled_step_task
 
-    close_httpx_client = mocker.patch(
-        "supervaizer.server.close_httpx_client",
-        new=mocker.AsyncMock(),
-    )
+    async def fake_wait(
+        tasks: set[object], *, timeout: float | None = None
+    ) -> tuple[set[object], set[object]]:
+        waited_tasks.append(set(tasks))
+        waited_timeouts.append(timeout)
+        return set(tasks), set()
+
     monkeypatch.setattr(
         Server.__init__.__globals__["asyncio"],
         "create_task",
         fake_create_task,
     )
-    monkeypatch.setitem(
-        Server.__init__.__globals__,
-        "close_httpx_client",
-        close_httpx_client,
+    monkeypatch.setattr(
+        Server.__init__.__globals__["asyncio"],
+        "wait",
+        fake_wait,
     )
 
     server = Server(
@@ -438,7 +442,73 @@ def fake_create_task(
 
     assert scheduled_step_task.cancelled is True
     assert scheduled_step_task.awaited is True
-    close_httpx_client.assert_awaited_once_with()
+    assert waited_tasks == [{scheduled_step_task}]
+    assert waited_timeouts == [server_module.SCHEDULED_STEP_SHUTDOWN_TIMEOUT_SECONDS]
+
+
+@pytest.mark.asyncio
+async def test_server_lifespan_leaves_shutdown_after_scheduler_timeout(
+    agent_fixture: Agent,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setenv("SUPERVAIZER_LOCAL_MODE", "false")
+
+    class PendingScheduledStepTask:
+        def __init__(self) -> None:
+            self.cancelled = False
+            self.awaited = False
+
+        def cancel(self) -> None:
+            self.cancelled = True
+
+        def __await__(self) -> Any:
+            async def _unexpected() -> None:
+                self.awaited = True
+                raise AssertionError("pending scheduler task should not be awaited")
+
+            return _unexpected().__await__()
+
+    pending_task = PendingScheduledStepTask()
+
+    def fake_create_task(
+        coro: object, *, name: str | None = None, **_kwargs: Any
+    ) -> PendingScheduledStepTask:
+        if hasattr(coro, "close"):
+            coro.close()
+        return pending_task
+
+    async def fake_wait(
+        tasks: set[object], *, timeout: float | None = None
+    ) -> tuple[set[object], set[object]]:
+        assert timeout == server_module.SCHEDULED_STEP_SHUTDOWN_TIMEOUT_SECONDS
+        return set(), set(tasks)
+
+    monkeypatch.setattr(
+        Server.__init__.__globals__["asyncio"],
+        "create_task",
+        fake_create_task,
+    )
+    monkeypatch.setattr(
+        Server.__init__.__globals__["asyncio"],
+        "wait",
+        fake_wait,
+    )
+
+    server = Server(
+        agents=[agent_fixture],
+        supervisor_account=None,
+        admin_interface=False,
+        host="localhost",
+        port=8001,
+        environment="test",
+        api_key="test-key",
+    )
+
+    async with server.app.router.lifespan_context(server.app):
+        assert pending_task.cancelled is False
+
+    assert pending_task.cancelled is True
+    assert pending_task.awaited is False
 
 
 def test_server_decrypt(server_fixture: Server) -> None: