From df7423e34e84f28b38fb00531f150850e73f6b4b Mon Sep 17 00:00:00 2001 From: Alain Prasquier Date: Tue, 26 May 2026 20:07:22 +0300 Subject: [PATCH 1/6] Clean up Supervaizer lifespan shutdown --- docs/CHANGELOG.md | 8 +++++++ src/supervaizer/server.py | 24 ++++++++++++++++---- tests/test_server.py | 47 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 8bcd04b..7f1f0d7 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -12,6 +12,14 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Changed + +- **FastAPI lifespan cleanup** — Controller shutdown now cancels and awaits the scheduled-step background loop, then closes the shared async HTTP event client. + +### Tests + +- `tests/test_server.py` — scheduler task cancellation and HTTP client cleanup during FastAPI lifespan shutdown. + ## [1.1.1] - 2026-05-20 ### Changed diff --git a/src/supervaizer/server.py b/src/supervaizer/server.py index 6097eb5..e091c85 100644 --- a/src/supervaizer/server.py +++ b/src/supervaizer/server.py @@ -16,7 +16,7 @@ import time import uuid from collections.abc import AsyncIterator, Callable -from contextlib import asynccontextmanager +from contextlib import asynccontextmanager, suppress from datetime import datetime # <-- REMOVED: Path (no longer needed) from hashlib import sha256 from typing import Any, ClassVar, TypeVar, cast @@ -37,6 +37,7 @@ 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 @@ -520,9 +521,24 @@ def __init__( openapi_url = "/openapi.json" @asynccontextmanager - async def _lifespan(app: FastAPI) -> AsyncIterator[None]: - asyncio.create_task(_run_scheduled_step_loop(self)) - yield + async def _lifespan(_app: FastAPI) -> AsyncIterator[None]: + # Keep a task handle so shutdown can stop the scheduler cleanly. + scheduled_step_task = asyncio.create_task( + _run_scheduled_step_loop(self), + name="supervaizer-scheduled-step-loop", + ) + try: + yield + finally: + # Let the scheduler observe cancellation instead of leaving a + # pending task attached to the event loop. + 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() app = FastAPI( lifespan=_lifespan, diff --git a/tests/test_server.py b/tests/test_server.py index 6185ce2..8983006 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -10,6 +10,7 @@ # If a copy of the MPL was not distributed with this file, you can obtain one at # https://mozilla.org/MPL/2.0/. +import asyncio import base64 import json import os @@ -371,6 +372,52 @@ def test_server_generated_api_key_is_exported_for_reload( assert os.environ["SUPERVAIZER_API_KEY"] == server.api_key +@pytest.mark.asyncio +async def test_server_lifespan_cleans_up_background_resources( + agent_fixture: Agent, + monkeypatch: pytest.MonkeyPatch, + mocker: Any, +) -> None: + monkeypatch.setenv("SUPERVAIZER_LOCAL_MODE", "false") + started = asyncio.Event() + cancelled = asyncio.Event() + loop_servers: list[Server] = [] + + async def fake_scheduled_step_loop(server: Server) -> None: + loop_servers.append(server) + started.set() + try: + await asyncio.Event().wait() + finally: + cancelled.set() + + close_httpx_client = mocker.patch( + "supervaizer.server.close_httpx_client", + new=mocker.AsyncMock(), + ) + monkeypatch.setattr( + "supervaizer.server._run_scheduled_step_loop", + fake_scheduled_step_loop, + ) + + 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): + await asyncio.wait_for(started.wait(), timeout=1) + assert loop_servers == [server] + + await asyncio.wait_for(cancelled.wait(), timeout=1) + close_httpx_client.assert_awaited_once_with() + + def test_server_decrypt(server_fixture: Server) -> None: unencrypted_parameters = str({"KEY": "VALUE"}) encrypted_parameters = server_fixture.encrypt(unencrypted_parameters) From 52da06b531d15fbccba823bc0e8f78ba21a8539e Mon Sep 17 00:00:00 2001 From: Alain Prasquier Date: Tue, 26 May 2026 20:10:52 +0300 Subject: [PATCH 2/6] Document Supervaizer import placement rule --- AGENTS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 2f9a9ad..ec7cb2b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,6 +5,7 @@ This is the canonical agent guide for the Supervaizer controller repo. Supervaiz ## Working Rules - Prefer simple, typed Python changes that match existing FastAPI/Pydantic patterns. +- Do not import from inside functions, methods, or local scopes unless it is absolutely required to avoid a concrete circular import, optional dependency, or startup-cost problem. Prefer module-level imports by default, and document the reason when a local import is unavoidable. - **No guessing / no implicit fallbacks:** when protocol versions, workspace identity, action/resource contracts, authentication, or transport configuration are missing or inconsistent, fail with a clear error that names the missing configuration. Do not infer another context, broaden scope, or silently fall back. - Use `just` recipes from this repo for local commands. - Use `uv` for Python environment and package operations. @@ -80,7 +81,7 @@ Ask. Refusing to act is always safer than taking an action that bypasses these r # GitNexus — Code Intelligence -This project is indexed by GitNexus as **supervaizer** (6117 symbols, 11434 relationships, 278 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. +This project is indexed by GitNexus as **supervaizer** (6192 symbols, 11554 relationships, 281 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. > If any GitNexus tool warns the index is stale, run `npx gitnexus analyze` in terminal first. From 0febbad724889e9ee5a76111f2d2006bf0ac0612 Mon Sep 17 00:00:00 2001 From: Alain Prasquier Date: Tue, 26 May 2026 20:28:33 +0300 Subject: [PATCH 3/6] Fix lifespan cleanup test determinism --- tests/test_server.py | 51 ++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index 8983006..31f8943 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -379,25 +379,47 @@ async def test_server_lifespan_cleans_up_background_resources( mocker: Any, ) -> None: monkeypatch.setenv("SUPERVAIZER_LOCAL_MODE", "false") - started = asyncio.Event() - cancelled = asyncio.Event() loop_servers: list[Server] = [] + created_task_names: list[str | None] = [] - async def fake_scheduled_step_loop(server: Server) -> None: - loop_servers.append(server) - started.set() - try: - await asyncio.Event().wait() - finally: - cancelled.set() + class FakeScheduledStepTask: + def __init__(self) -> None: + self.cancelled = False + self.awaited = False + + def cancel(self) -> None: + self.cancelled = True + + def __await__(self) -> Any: + async def _cancelled() -> None: + self.awaited = True + raise asyncio.CancelledError + + return _cancelled().__await__() + + scheduled_step_task = FakeScheduledStepTask() + + def fake_create_task( + coro: object, *, name: str | None = None, **_kwargs: Any + ) -> FakeScheduledStepTask: + if hasattr(coro, "close"): + coro.close() + created_task_names.append(name) + return scheduled_step_task close_httpx_client = mocker.patch( "supervaizer.server.close_httpx_client", new=mocker.AsyncMock(), ) monkeypatch.setattr( - "supervaizer.server._run_scheduled_step_loop", - fake_scheduled_step_loop, + Server.__init__.__globals__["asyncio"], + "create_task", + fake_create_task, + ) + monkeypatch.setitem( + Server.__init__.__globals__, + "close_httpx_client", + close_httpx_client, ) server = Server( @@ -411,10 +433,11 @@ async def fake_scheduled_step_loop(server: Server) -> None: ) async with server.app.router.lifespan_context(server.app): - await asyncio.wait_for(started.wait(), timeout=1) - assert loop_servers == [server] + assert created_task_names == ["supervaizer-scheduled-step-loop"] + assert scheduled_step_task.cancelled is False - await asyncio.wait_for(cancelled.wait(), timeout=1) + assert scheduled_step_task.cancelled is True + assert scheduled_step_task.awaited is True close_httpx_client.assert_awaited_once_with() From 3b37f99e7facf1456b112eae8732fd6bbc40ee05 Mon Sep 17 00:00:00 2001 From: Alain Prasquier Date: Tue, 26 May 2026 20:45:21 +0300 Subject: [PATCH 4/6] Address lifespan shutdown review feedback --- docs/CHANGELOG.md | 4 +- src/supervaizer/server.py | 23 ++++++---- tests/test_server.py | 88 +++++++++++++++++++++++++++++++++++---- 3 files changed, 95 insertions(+), 20 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7f1f0d7..b670e27 100644 --- a/docs/CHANGELOG.md +++ b/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 diff --git a/src/supervaizer/server.py b/src/supervaizer/server.py index e091c85..a6f3fea 100644 --- a/src/supervaizer/server.py +++ b/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, diff --git a/tests/test_server.py b/tests/test_server.py index 31f8943..f5b6e50 100644 --- a/tests/test_server.py +++ b/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: From 2c0c1853269b92b52bc33d592dba3866b4431155 Mon Sep 17 00:00:00 2001 From: Alain Prasquier Date: Tue, 26 May 2026 21:24:47 +0300 Subject: [PATCH 5/6] =?UTF-8?q?=E2=9C=A8feat:=20rename=20test=20and=20asse?= =?UTF-8?q?rt=20scheduled=20step=20task?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/test_server.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index f5b6e50..4a0f4d8 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -374,14 +374,14 @@ def test_server_generated_api_key_is_exported_for_reload( @pytest.mark.asyncio -async def test_server_lifespan_cleans_up_background_resources( +async def test_server_lifespan_cancels_scheduled_step_task( agent_fixture: Agent, monkeypatch: pytest.MonkeyPatch, - mocker: Any, ) -> None: monkeypatch.setenv("SUPERVAIZER_LOCAL_MODE", "false") - loop_servers: list[Server] = [] created_task_names: list[str | None] = [] + waited_tasks: list[set[object]] = [] + waited_timeouts: list[float | None] = [] class FakeScheduledStepTask: def __init__(self) -> None: From 697562db8b4d508ef2d2a49c3ea948e637c27074 Mon Sep 17 00:00:00 2001 From: Alain Prasquier Date: Tue, 26 May 2026 23:10:26 +0300 Subject: [PATCH 6/6] =?UTF-8?q?=E2=9C=A8=20feat:=20update=20GitNexus=20ind?= =?UTF-8?q?ex=20counts=20in=20AGENTS.md?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index ec7cb2b..b50034b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -81,7 +81,7 @@ Ask. Refusing to act is always safer than taking an action that bypasses these r # GitNexus — Code Intelligence -This project is indexed by GitNexus as **supervaizer** (6192 symbols, 11554 relationships, 281 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. +This project is indexed by GitNexus as **supervaizer** (6225 symbols, 11375 relationships, 276 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. > If any GitNexus tool warns the index is stale, run `npx gitnexus analyze` in terminal first.