From f52bc5535fbddf90a69a5dc4c26dd031ba1560b2 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 12 Oct 2026 08:54:01 -0700 Subject: [PATCH 1/4] fix(gateway): a container deploy cancelled mid-create leaves no containers running - The container gateway records each MCP server and sidecar container as it's created, not after the whole batch returns, so the close a cancelled or failed deploy gets terminates the ones already made. - Modal's create_container lets a create request it was cancelled during run to its end, and terminates the sandbox Modal made all the same, so a cancel that lands mid-request leaves nothing running either. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../env_providers/env_gateway_provider.py | 10 ++- .../sandbox_providers/modal_sandbox.py | 18 +++- .../env_gateway_provider_test.py | 88 ++++++++++++------- .../sandbox_providers/modal_sandbox_test.py | 28 ++++++ 4 files changed, 106 insertions(+), 38 deletions(-) diff --git a/src/agent_env/providers/env_providers/env_gateway_provider.py b/src/agent_env/providers/env_providers/env_gateway_provider.py index 206fea94..4f0509ec 100644 --- a/src/agent_env/providers/env_providers/env_gateway_provider.py +++ b/src/agent_env/providers/env_providers/env_gateway_provider.py @@ -935,7 +935,8 @@ async def _deploy_via_containers( self._state_provider.sidecar_specs(instance=state_instance) if self._needs_local_postgres else [] ) - # Step 2: MCP server sandboxes (HTTPS), provisioned in parallel. + # Step 2: MCP server sandboxes (HTTPS), provisioned in parallel. Each is recorded as it's created, so a + # deploy that fails or is cancelled mid-batch closes the ones already made. mcp_coros = [ self._provision_mcp(deploy, cfg, art, state_instance) for cfg, art in zip(mcp_servers, mcp_server_images) @@ -954,12 +955,10 @@ async def _deploy_via_containers( continue if i < len(mcp_coros): environment_name, mcp_sb = r - self._container_sandboxes.append(mcp_sb) self._environment_sandboxes[environment_name] = mcp_sb mcp_results.append((environment_name, mcp_sb)) else: spec = sidecar_specs[i - len(mcp_coros)] - self._container_sandboxes.append(r) if spec.name == PGWEB_SERVICE_NAME: pgweb_sb = r self._pgweb_sandbox = r @@ -1106,7 +1105,7 @@ async def _get_container_id(self, sandbox: VmSandbox, compose_service: str) -> s async def _provision_sidecar(self, deploy: _ContainerDeploy, spec) -> Sandbox: logger.info(f"Provisioning {spec.name} from {spec.image}...") - return await deploy.sandbox_provider.create_container( + sb = await deploy.sandbox_provider.create_container( image_name=spec.image, port=spec.port, env=spec.env, @@ -1116,6 +1115,8 @@ async def _provision_sidecar(self, deploy: _ContainerDeploy, spec) -> Sandbox: attribution=deploy.attribution, private_network=True, ) + self._container_sandboxes.append(sb) + return sb @property def _needs_local_postgres(self) -> bool: @@ -1217,6 +1218,7 @@ async def _provision_mcp( attribution=deploy.attribution, private_network=True, ) + self._container_sandboxes.append(sb) return cfg.environment_name, sb async def _deploy_gateway( diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index 9c5bee77..af051681 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -348,15 +348,25 @@ async def create(image: modal.Image) -> modal.Sandbox: *(command or []), i6pn=i6pn, image=image, **create_kwargs ) - try: + async def create_rebuilding_once() -> modal.Sandbox: try: - sb = await create(image) + return await create(image) except modal.exception.NotFoundError: if built_id is None: raise # Modal no longer holds the image this process built from the context, so it's built again, once. - image, built_id = await self._context_image(prepared, stale=built_id) - sb = await create(image) + rebuilt, _ = await self._context_image(prepared, stale=built_id) + return await create(rebuilt) + + # Modal may create the sandbox even when this create is cancelled, so the request runs to its end, and what it + # created is then terminated: a cancelled create leaves nothing running. + creating = asyncio.ensure_future(create_rebuilding_once()) + try: + sb = await asyncio.shield(creating) + except asyncio.CancelledError: + with contextlib.suppress(Exception, asyncio.CancelledError): + await (await creating).terminate.aio() + raise except Exception as e: raise RuntimeError( f"Modal sandbox create failed [{fmt_exc(e)}]: {call_context}" diff --git a/tst/unit/providers/env_providers/env_gateway_provider_test.py b/tst/unit/providers/env_providers/env_gateway_provider_test.py index bcb5dc50..bc0145aa 100644 --- a/tst/unit/providers/env_providers/env_gateway_provider_test.py +++ b/tst/unit/providers/env_providers/env_gateway_provider_test.py @@ -574,29 +574,16 @@ async def test_wait_for_tunnel_polls_through_connection_errors(monkeypatch): card, seen = await _probe(monkeypatch, httpx.ConnectError("refused"), lambda: httpx.Response(200, json=_CARD)) assert card == _CARD and len(seen) == 2 -async def _run_container_deploy(*, cpu=None, memory_mb=None) -> list[dict]: - """Run _deploy_via_containers with all I/O mocked, returning the list of - create_container kwargs (one per provisioned component: db, mcp, pgweb, - db-mcp, gateway). ECR-style db images are used so the pgweb/db-mcp aux - services are provisioned too.""" +@contextlib.contextmanager +def _slack_in_containers(gp: EnvironmentGatewayProvider, provider: ModalSandboxProvider, create_container): + """``gp`` ready to deploy the slack server on ``provider`` in containers, every I/O but ``create_container`` + mocked. ECR-style db images, so the pgweb and db-mcp sidecars are provisioned too. Yields the deploy's arguments.""" from agent_env.env.gateway import GatewayMode - provider = ModalSandboxProvider() - gp = EnvironmentGatewayProvider() gp._state_provider = LocalPostgresStateProvider() # normally set by create_gateway - create_calls: list[dict] = [] - - async def fake_create_container(**kwargs): - create_calls.append(kwargs) - port = kwargs["port"] - image = kwargs["image_name"] - if kwargs.get("is_tcp"): - return _FakeSandbox(f"db-{port}", port, f"tcp://db.modal.host:{port}") - return _FakeSandbox(f"sb-{image}-{port}", port, f"https://{image}.modal.host") - # The servicedb images are in the image store, so the containers run them. get_config().set_image_store(EcrImageStore(registry_host="123.dkr.ecr.us-west-2.amazonaws.com")) - with patch.object(provider, "create_container", side_effect=fake_create_container): + with patch.object(provider, "create_container", side_effect=create_container): gp._init_service_db_via_exec = AsyncMock() gp._wait_for_tunnel = AsyncMock(return_value=_CARD) with patch("agent_env.env.env.Env.get") as env_get, \ @@ -613,22 +600,63 @@ async def fake_create_container(**kwargs): cfg.default_service_db_env_id = "db-id" get_cfg.return_value = cfg env_get.side_effect = lambda env_id, *a, **k: gateway_env if env_id == "gw-id" else db_env + yield dict(mcp_servers=[MCPServerConfig(image="mcp-slack", environment_name="slack")], + mcp_server_images=[_image("mcp-slack")], gateway_port=18765, website_configs=None, + gateway_mode=GatewayMode.PERFORMANCE, ttl_seconds=60, disk_size_gb=10) - await gp._deploy_via_containers( - sandbox_provider=provider, - mcp_servers=[MCPServerConfig(image="mcp-slack", environment_name="slack")], - mcp_server_images=[_image("mcp-slack")], - gateway_port=18765, - website_configs=None, - gateway_mode=GatewayMode.PERFORMANCE, - ttl_seconds=60, - disk_size_gb=10, - cpu=cpu, - memory_mb=memory_mb, - ) + +async def _run_container_deploy(*, cpu=None, memory_mb=None) -> list[dict]: + """Run _deploy_via_containers with all I/O mocked, returning the list of + create_container kwargs (one per provisioned component: db, mcp, pgweb, + db-mcp, gateway).""" + provider = ModalSandboxProvider() + gp = EnvironmentGatewayProvider() + create_calls: list[dict] = [] + + async def fake_create_container(**kwargs): + create_calls.append(kwargs) + port = kwargs["port"] + image = kwargs["image_name"] + if kwargs.get("is_tcp"): + return _FakeSandbox(f"db-{port}", port, f"tcp://db.modal.host:{port}") + return _FakeSandbox(f"sb-{image}-{port}", port, f"https://{image}.modal.host") + + with _slack_in_containers(gp, provider, fake_create_container) as args: + await gp._deploy_via_containers(sandbox_provider=provider, cpu=cpu, memory_mb=memory_mb, **args) return create_calls +@pytest.mark.asyncio +async def test_a_deploy_cancelled_while_its_containers_are_created_closes_those_already_made(): + """Each container is recorded as it's created, so the close a cancelled deploy gets terminates the ones made + before the cancel, not only those a finished batch would have recorded.""" + provider = ModalSandboxProvider() + gp = EnvironmentGatewayProvider() + created: list[_FakeSandbox] = [] + made, never = asyncio.Event(), asyncio.Event() + + async def create_container(**kwargs): + if kwargs["image_name"].endswith("db-mcp"): + await never.wait() # still being created when the deploy is cancelled + name = kwargs["image_name"].rsplit("/", 1)[-1] + sandbox = _FakeSandbox(name, kwargs["port"], f"https://{name}.modal.host") + created.append(sandbox) + if len(created) == 3: # the servicedb, the slack server and pgweb + made.set() + return sandbox + + with _slack_in_containers(gp, provider, create_container) as args: + deploy = asyncio.create_task( + gp._run(provider, "slack", lambda p: gp._deploy_via_containers(sandbox_provider=p, **args))) + await asyncio.wait_for(made.wait(), timeout=10) + deploy.cancel() + with pytest.raises(asyncio.CancelledError): + await deploy + + assert [sandbox.sandbox_id for sandbox in created] == ["postgres", "mcp-slack", "pgweb"] + assert [sandbox.terminated for sandbox in created] == [True, True, True] + + @pytest.mark.asyncio async def test_deploy_level_size_tunes_the_gateway_only(): """cpu/memory_mb are the gateway's; the per-service sandboxes have their own diff --git a/tst/unit/providers/sandbox_providers/modal_sandbox_test.py b/tst/unit/providers/sandbox_providers/modal_sandbox_test.py index cc19a6b9..63871816 100644 --- a/tst/unit/providers/sandbox_providers/modal_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/modal_sandbox_test.py @@ -107,6 +107,34 @@ async def test_create_container_terminates_a_sandbox_whose_create_is_cancelled(_ fake_sb.terminate.aio.assert_awaited_once() +@pytest.mark.asyncio +async def test_create_container_terminates_a_sandbox_modal_creates_after_its_create_is_cancelled(_patched_provider): + """Cancelled while Modal is still creating the sandbox, as a deploy cancelled mid-batch is: Modal creates it all the + same, and it would run until its timeout, so the create waits for it and terminates it.""" + fake_sb = MagicMock() + fake_sb.terminate.aio = AsyncMock() + asked, release = asyncio.Event(), asyncio.Event() + + async def experimental_create(*args, **kwargs): + asked.set() + await release.wait() + return fake_sb + + with patch("agent_env.providers.sandbox_providers.modal_sandbox.modal.Sandbox._experimental_create") as mock_create: + mock_create.aio = AsyncMock(side_effect=experimental_create) + create = asyncio.create_task( + _patched_provider.create_container(image_name="registry.example/mcp-server-example:v1", port=18765, env={})) + await asyncio.wait_for(asked.wait(), timeout=5) + create.cancel() + release.set() + + with pytest.raises(asyncio.CancelledError): + await create + + fake_sb.terminate.aio.assert_awaited_once() + fake_sb.wait_until_ready.aio.assert_not_called() + + @pytest.mark.asyncio async def test_create_container_includes_cause_message_in_wrapped_text(_patched_provider): class _RemoteErrorWithMessage(Exception): From 25c7e5a2b401d69cab7d773f43b4694a64fcb598 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 12 Oct 2026 09:27:00 -0700 Subject: [PATCH 2/4] fix(modal): name a sandbox a cancelled create can't terminate When Modal returns the sandbox a cancelled create asked for and terminating it fails, the error is no longer dropped: a warning names the sandbox, which runs until its timeout, and the error. Waiting for the create and terminating it are separate steps, so a failed create stays quiet as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../sandbox_providers/modal_sandbox.py | 11 ++++++-- .../sandbox_providers/modal_sandbox_test.py | 27 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index af051681..d57dfa24 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -364,8 +364,15 @@ async def create_rebuilding_once() -> modal.Sandbox: try: sb = await asyncio.shield(creating) except asyncio.CancelledError: - with contextlib.suppress(Exception, asyncio.CancelledError): - await (await creating).terminate.aio() + created = None + with contextlib.suppress(Exception, asyncio.CancelledError): # the create failed, or a second cancel came + created = await creating + if created is not None: + try: + await created.terminate.aio() + except Exception as e: + logger.warning(f"Could not terminate Modal sandbox {created.object_id}, whose create was cancelled; " + f"it runs until its timeout: {fmt_exc(e)}") raise except Exception as e: raise RuntimeError( diff --git a/tst/unit/providers/sandbox_providers/modal_sandbox_test.py b/tst/unit/providers/sandbox_providers/modal_sandbox_test.py index 63871816..d7b2e3a6 100644 --- a/tst/unit/providers/sandbox_providers/modal_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/modal_sandbox_test.py @@ -135,6 +135,33 @@ async def experimental_create(*args, **kwargs): fake_sb.wait_until_ready.aio.assert_not_called() +@pytest.mark.asyncio +async def test_a_sandbox_a_cancelled_create_cant_terminate_is_named_in_a_warning(_patched_provider, caplog): + """It runs until its timeout, so whoever reads the log can find it.""" + fake_sb = MagicMock(object_id="sb-STILLRUNNING") + fake_sb.terminate.aio = AsyncMock(side_effect=RuntimeError("Modal is unavailable")) + asked, release = asyncio.Event(), asyncio.Event() + + async def experimental_create(*args, **kwargs): + asked.set() + await release.wait() + return fake_sb + + with patch("agent_env.providers.sandbox_providers.modal_sandbox.modal.Sandbox._experimental_create") as mock_create: + mock_create.aio = AsyncMock(side_effect=experimental_create) + create = asyncio.create_task( + _patched_provider.create_container(image_name="registry.example/mcp-server-example:v1", port=18765, env={})) + await asyncio.wait_for(asked.wait(), timeout=5) + create.cancel() + release.set() + + with pytest.raises(asyncio.CancelledError): + await create + + assert any("sb-STILLRUNNING" in r.getMessage() and "Modal is unavailable" in r.getMessage() + for r in caplog.records if r.levelname == "WARNING") + + @pytest.mark.asyncio async def test_create_container_includes_cause_message_in_wrapped_text(_patched_provider): class _RemoteErrorWithMessage(Exception): From 5197ea33c49764c9d8a8f53f52a4bfc94ef9ba93 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 12 Oct 2026 13:37:44 -0700 Subject: [PATCH 3/4] fix(modal): a cancelled create waits for its request 30 s at most, and terminates what it creates later A cancel that lands while Modal's create request is in flight waited for the request with no bound, so a request that hung held up a chain's provisioning deadline, a Ctrl-C or an activity's cancel. It now waits 30 s at most (the request takes a few seconds as a rule); past that, or on a second cancel, the cancel goes on, a warning says so, and the sandbox the request creates is terminated when it ends. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../sandbox_providers/modal_sandbox.py | 44 ++++++++++++++---- .../sandbox_providers/modal_sandbox_test.py | 45 +++++++++++++++++++ 2 files changed, 80 insertions(+), 9 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index d57dfa24..ad913045 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -53,6 +53,30 @@ def _resolve_app_base_name(app_name: Optional[str]) -> str: # chunks divisible by 3 so concatenated base64 has padding only at EOF (mid-stream # '=' would make `base64 -d` truncate the file). _STDIN_CHUNK_BYTES = 8 * 1024 * 1024 - ((8 * 1024 * 1024) % 3) +# How long a cancelled create waits for its request to Modal to end, a few seconds as a rule; past it the cancel goes +# on, and the sandbox the request creates is terminated when it ends. +_CANCELLED_CREATE_WAIT_SECONDS = 30 +# Terminations started after their create's cancel had gone on, kept so they run to their end. +_LATE_TERMINATIONS: set[asyncio.Task] = set() + + +async def _terminate_created(sb: modal.Sandbox) -> None: + """Terminate ``sb``, which a cancelled create made. A failure is logged, naming it, since it runs until its + timeout.""" + try: + await sb.terminate.aio() + except Exception as e: + logger.warning(f"Could not terminate Modal sandbox {sb.object_id}, whose create was cancelled; it runs until " + f"its timeout: {fmt_exc(e)}") + + +def _terminate_when_created(creating: asyncio.Future) -> None: + """Terminate the sandbox a create that outlasted its cancel's wait made, if it made one.""" + if creating.cancelled() or creating.exception() is not None: + return + task = creating.get_loop().create_task(_terminate_created(creating.result())) + _LATE_TERMINATIONS.add(task) + task.add_done_callback(_LATE_TERMINATIONS.discard) def _tunnel_url(t) -> str: @@ -358,21 +382,23 @@ async def create_rebuilding_once() -> modal.Sandbox: rebuilt, _ = await self._context_image(prepared, stale=built_id) return await create(rebuilt) - # Modal may create the sandbox even when this create is cancelled, so the request runs to its end, and what it - # created is then terminated: a cancelled create leaves nothing running. + # Modal may create the sandbox even when this create is cancelled, so the request runs on, and what it creates + # is terminated: a cancelled create leaves nothing running. The cancel waits for it a while only, so a request + # that hangs can't hold up a chain's deadline or a Ctrl-C. creating = asyncio.ensure_future(create_rebuilding_once()) try: sb = await asyncio.shield(creating) except asyncio.CancelledError: created = None - with contextlib.suppress(Exception, asyncio.CancelledError): # the create failed, or a second cancel came - created = await creating + try: + created = await asyncio.wait_for(asyncio.shield(creating), _CANCELLED_CREATE_WAIT_SECONDS) + except (Exception, asyncio.CancelledError): # it failed, outlasted the wait, or a second cancel came + if not creating.done(): + creating.add_done_callback(_terminate_when_created) + logger.warning(f"A cancelled Modal sandbox create is still waiting on Modal; the sandbox it " + f"creates, if any, is terminated when it ends: {call_context}") if created is not None: - try: - await created.terminate.aio() - except Exception as e: - logger.warning(f"Could not terminate Modal sandbox {created.object_id}, whose create was cancelled; " - f"it runs until its timeout: {fmt_exc(e)}") + await _terminate_created(created) raise except Exception as e: raise RuntimeError( diff --git a/tst/unit/providers/sandbox_providers/modal_sandbox_test.py b/tst/unit/providers/sandbox_providers/modal_sandbox_test.py index d7b2e3a6..9133b7be 100644 --- a/tst/unit/providers/sandbox_providers/modal_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/modal_sandbox_test.py @@ -8,6 +8,7 @@ import pytest from agent_env.config import get_config +from agent_env.providers.sandbox_providers import modal_sandbox from agent_env.providers.sandbox_providers.modal_sandbox import ModalSandbox, ModalSandboxProvider from agent_env.store import set_object_store from tst.unit.store.fakes import ConfiguredObjectStore @@ -162,6 +163,50 @@ async def experimental_create(*args, **kwargs): for r in caplog.records if r.levelname == "WARNING") +async def _cancel_a_create_that_hangs(provider, *, cancels: int) -> tuple[MagicMock, asyncio.Event]: + """Start a create whose request to Modal hangs until the returned event is set, cancel it ``cancels`` times, and + check it raises CancelledError without the request having ended.""" + fake_sb = MagicMock(object_id="sb-LATE") + fake_sb.terminate.aio = AsyncMock() + asked, release = asyncio.Event(), asyncio.Event() + + async def experimental_create(*args, **kwargs): + asked.set() + await release.wait() + return fake_sb + + with patch("agent_env.providers.sandbox_providers.modal_sandbox.modal.Sandbox._experimental_create") as mock_create: + mock_create.aio = AsyncMock(side_effect=experimental_create) + create = asyncio.create_task( + provider.create_container(image_name="registry.example/mcp-server-example:v1", port=18765, env={})) + await asyncio.wait_for(asked.wait(), timeout=5) + for _ in range(cancels): + create.cancel() + await asyncio.sleep(0) + with pytest.raises(asyncio.CancelledError): + await asyncio.wait_for(create, timeout=5) + fake_sb.terminate.aio.assert_not_called() + return fake_sb, release + + +@pytest.mark.asyncio +@pytest.mark.parametrize("cancels, wait_seconds", [(1, 0.05), (2, 30)], ids=["outlasts-the-wait", "cancelled-again"]) +async def test_a_cancelled_create_stops_waiting_on_a_request_that_hangs_and_terminates_what_it_creates_later( + _patched_provider, monkeypatch, caplog, cancels, wait_seconds): + """So a hung request can't hold up a chain's deadline or a Ctrl-C, nor leave a sandbox behind when it ends.""" + monkeypatch.setattr(modal_sandbox, "_CANCELLED_CREATE_WAIT_SECONDS", wait_seconds) + + fake_sb, release = await _cancel_a_create_that_hangs(_patched_provider, cancels=cancels) + release.set() + for _ in range(50): + if fake_sb.terminate.aio.await_count: + break + await asyncio.sleep(0.01) + + fake_sb.terminate.aio.assert_awaited_once() + assert any("still waiting on Modal" in r.getMessage() for r in caplog.records if r.levelname == "WARNING") + + @pytest.mark.asyncio async def test_create_container_includes_cause_message_in_wrapped_text(_patched_provider): class _RemoteErrorWithMessage(Exception): From 2adc73ff767e09a10dce9e0fc36e13acc4844470 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 12 Oct 2026 13:48:55 -0700 Subject: [PATCH 4/4] Terminate a sandbox created just as a cancelled create's wait gives up, and name a create the process abandons The wait can time out or be cancelled in the same step Modal answers, which left the create done but its sandbox running: the late-termination callback was only registered while the create was still pending. It is now always registered, and runs at once on a create that has already ended. A create still pending when the process exits is cancelled by asyncio.run, so a sandbox Modal makes for it runs until its timeout. The callback now logs that with the create's details, so the sandbox can be found. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../sandbox_providers/modal_sandbox.py | 16 +++++-- .../sandbox_providers/modal_sandbox_test.py | 45 +++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index ad913045..eaf4a72b 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -5,6 +5,7 @@ import asyncio import base64 import contextlib +import functools import hashlib import logging import os @@ -70,9 +71,15 @@ async def _terminate_created(sb: modal.Sandbox) -> None: f"its timeout: {fmt_exc(e)}") -def _terminate_when_created(creating: asyncio.Future) -> None: - """Terminate the sandbox a create that outlasted its cancel's wait made, if it made one.""" - if creating.cancelled() or creating.exception() is not None: +def _terminate_when_created(creating: asyncio.Future, *, call_context: str) -> None: + """Terminate the sandbox a create that outlasted its cancel's wait made, if it made one. A create cancelled before + it ended, as a process that exits cancels it, may have made one all the same, which runs until its timeout, so a + warning names the call.""" + if creating.cancelled(): + logger.warning(f"A cancelled Modal sandbox create was stopped before Modal answered; a sandbox Modal made for " + f"it runs until its timeout: {call_context}") + return + if creating.exception() is not None: return task = creating.get_loop().create_task(_terminate_created(creating.result())) _LATE_TERMINATIONS.add(task) @@ -393,8 +400,9 @@ async def create_rebuilding_once() -> modal.Sandbox: try: created = await asyncio.wait_for(asyncio.shield(creating), _CANCELLED_CREATE_WAIT_SECONDS) except (Exception, asyncio.CancelledError): # it failed, outlasted the wait, or a second cancel came + # A create that ended just as the wait gave up runs the callback at once + creating.add_done_callback(functools.partial(_terminate_when_created, call_context=call_context)) if not creating.done(): - creating.add_done_callback(_terminate_when_created) logger.warning(f"A cancelled Modal sandbox create is still waiting on Modal; the sandbox it " f"creates, if any, is terminated when it ends: {call_context}") if created is not None: diff --git a/tst/unit/providers/sandbox_providers/modal_sandbox_test.py b/tst/unit/providers/sandbox_providers/modal_sandbox_test.py index 9133b7be..56c6f967 100644 --- a/tst/unit/providers/sandbox_providers/modal_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/modal_sandbox_test.py @@ -207,6 +207,51 @@ async def test_a_cancelled_create_stops_waiting_on_a_request_that_hangs_and_term assert any("still waiting on Modal" in r.getMessage() for r in caplog.records if r.levelname == "WARNING") +@pytest.mark.asyncio +async def test_a_sandbox_created_just_as_a_cancelled_creates_wait_gives_up_is_terminated(_patched_provider, monkeypatch): + """The wait can give up in the same step Modal answers, which leaves the create done but its sandbox running.""" + fake_sb = MagicMock(object_id="sb-JUSTINTIME") + fake_sb.terminate.aio = AsyncMock() + asked, release = asyncio.Event(), asyncio.Event() + + async def experimental_create(*args, **kwargs): + asked.set() + await release.wait() + return fake_sb + + async def gives_up_as_the_create_ends(awaitable, timeout): + await awaitable + raise TimeoutError + + with patch("agent_env.providers.sandbox_providers.modal_sandbox.modal.Sandbox._experimental_create") as mock_create: + mock_create.aio = AsyncMock(side_effect=experimental_create) + create = asyncio.create_task( + _patched_provider.create_container(image_name="registry.example/mcp-server-example:v1", port=18765, env={})) + await asked.wait() + monkeypatch.setattr(modal_sandbox.asyncio, "wait_for", gives_up_as_the_create_ends) + create.cancel() + release.set() + with pytest.raises(asyncio.CancelledError): + await create + for _ in range(50): + if fake_sb.terminate.aio.await_count: + break + await asyncio.sleep(0.01) + + fake_sb.terminate.aio.assert_awaited_once() + + +def test_a_process_that_exits_while_a_cancelled_create_hangs_names_the_create_in_a_warning( + _patched_provider, monkeypatch, caplog): + """Exiting cancels the create, so what Modal makes for it runs until its timeout; the warning lets it be found.""" + monkeypatch.setattr(modal_sandbox, "_CANCELLED_CREATE_WAIT_SECONDS", 0.05) + + asyncio.run(_cancel_a_create_that_hangs(_patched_provider, cancels=1)) + + assert any("stopped before Modal answered" in r.getMessage() and "mcp-server-example:v1" in r.getMessage() + for r in caplog.records if r.levelname == "WARNING") + + @pytest.mark.asyncio async def test_create_container_includes_cause_message_in_wrapped_text(_patched_provider): class _RemoteErrorWithMessage(Exception):