From 30a3145d3a4658077ef92d8b182b3565bd3b7fcc Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Thu, 8 Oct 2026 23:19:16 -0700 Subject: [PATCH 01/10] fix(modal): terminate a sandbox whose create is cancelled A chain's deadline cancels a create still waiting on its sandbox, and the CancelledError skipped the cleanup that terminates it, so the sandbox ran until its timeout. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../sandbox_providers/modal_sandbox.py | 4 +++- .../sandbox_providers/modal_sandbox_test.py | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index ea14c39c..5ee459fb 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -368,11 +368,13 @@ async def create_container( sandbox.vnc_url = f"{tunnel_urls[vnc_port]}/vnc.html" logger.info(f"Modal sandbox vnc_url: {sandbox.vnc_url}") return sandbox - except Exception as e: + except BaseException as e: # a cancelled create, at a chain's deadline say, terminates its sandbox too try: await sb.terminate.aio() except Exception: pass + if not isinstance(e, Exception): + raise raise RuntimeError( f"Modal sandbox post-create failed [{_fmt_exc(e)}]: " f"sb_id={sb.object_id} {call_context}" diff --git a/tst/unit/providers/sandbox_providers/modal_sandbox_test.py b/tst/unit/providers/sandbox_providers/modal_sandbox_test.py index 2a3ffb4e..6f20e6d6 100644 --- a/tst/unit/providers/sandbox_providers/modal_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/modal_sandbox_test.py @@ -2,6 +2,7 @@ from __future__ import annotations +import asyncio from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -90,6 +91,23 @@ async def test_create_container_wraps_post_create_failure_and_terminates(_patche fake_sb.terminate.aio.assert_awaited_once() +@pytest.mark.asyncio +async def test_create_container_terminates_a_sandbox_whose_create_is_cancelled(_patched_provider): + """A chain's deadline cancels a create still waiting on its sandbox, which would otherwise run until its timeout.""" + fake_sb = MagicMock() + fake_sb.object_id = "sb-FAKEFAKEFAKEFAKEFAKE" + fake_sb.wait_until_ready.aio = AsyncMock(side_effect=asyncio.CancelledError) + fake_sb.terminate.aio = AsyncMock() + + with patch("agent_env.providers.sandbox_providers.modal_sandbox.modal.Sandbox._experimental_create") as mock_create: + mock_create.aio = AsyncMock(return_value=fake_sb) + + with pytest.raises(asyncio.CancelledError): + await _patched_provider.create_container(image_name="registry.example/mcp-server-example:v1", port=18765, env={}) + + fake_sb.terminate.aio.assert_awaited_once() + + @pytest.mark.asyncio async def test_create_container_includes_cause_message_in_wrapped_text(_patched_provider): class _RemoteErrorWithMessage(Exception): From 63812464bfb74f4002f3f1a3ae9fcf404c82c99b Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Thu, 8 Oct 2026 23:19:25 -0700 Subject: [PATCH 02/10] feat(sandbox): a prepare_image hook, run outside a chain's deadline SandboxProvider.prepare_image(image) readies an image before a sandbox is created from its name; it does nothing by default. A chain remembers the image and prepares each provider just before trying it, outside the deadline on creating the sandbox, so a slow build doesn't count against it, and a failed prepare falls through to the next provider like a failed create. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../chained_sandbox_provider.py | 12 +++++ .../sandbox_providers/sandbox_provider.py | 6 +++ .../sandbox_provider_test.py | 54 +++++++++++++++++++ 3 files changed, 72 insertions(+) diff --git a/src/agent_env/providers/sandbox_providers/chained_sandbox_provider.py b/src/agent_env/providers/sandbox_providers/chained_sandbox_provider.py index a668e7a1..201d204e 100644 --- a/src/agent_env/providers/sandbox_providers/chained_sandbox_provider.py +++ b/src/agent_env/providers/sandbox_providers/chained_sandbox_provider.py @@ -2,11 +2,15 @@ import asyncio import logging +from typing import TYPE_CHECKING from agent_env.attribution import Attribution from agent_env.providers.sandbox_providers.sandbox import NetworkPolicy, NetworkPolicyUnsupportedError, Sandbox from agent_env.providers.sandbox_providers.sandbox_provider import SandboxProvider +if TYPE_CHECKING: + from agent_env.artifact.artifacts.docker_image import DockerImageArtifact + logger = logging.getLogger(__name__) _PROVISION_DEADLINE_SECONDS = 180 @@ -17,6 +21,7 @@ def __init__(self, providers: list[SandboxProvider]): if not providers: raise ValueError("ChainedSandboxProvider requires at least one provider") self._providers = providers + self._images: dict[str, DockerImageArtifact] = {} @property def providers(self) -> tuple[SandboxProvider, ...]: @@ -47,6 +52,10 @@ def filter_sandbox_providers( def supports_network_policy(self, policy: NetworkPolicy) -> bool: return any(p.supports_network_policy(policy) for p in self._providers) + async def prepare_image(self, image: DockerImageArtifact) -> None: + """Remember ``image``, so each provider is prepared for it just before it's tried, and only that one.""" + self._images[image.image_name] = image + async def create_sandbox( self, *, @@ -54,10 +63,13 @@ async def create_sandbox( **kwargs, ) -> Sandbox: sandbox_providers = self.filter_sandbox_providers(self._providers, kwargs.get("network_policy")) + image = self._images.get(kwargs.get("image_name")) errors: list[tuple[str, Exception]] = [] for p in sandbox_providers: name = type(p).__name__ try: + if image is not None: + await p.prepare_image(image) # outside the deadline, which bounds creating the sandbox, not a build async with asyncio.timeout(_PROVISION_DEADLINE_SECONDS): sandbox = await p.create_sandbox(attribution=attribution, **kwargs) except Exception as e: diff --git a/src/agent_env/providers/sandbox_providers/sandbox_provider.py b/src/agent_env/providers/sandbox_providers/sandbox_provider.py index b0a8fdd0..20e5a20d 100644 --- a/src/agent_env/providers/sandbox_providers/sandbox_provider.py +++ b/src/agent_env/providers/sandbox_providers/sandbox_provider.py @@ -23,6 +23,7 @@ ) if TYPE_CHECKING: + from agent_env.artifact.artifacts.docker_image import DockerImageArtifact from agent_env.config.runtime import Config logger = logging.getLogger(__name__) @@ -108,6 +109,11 @@ async def create_container( pass raise + async def prepare_image(self, image: DockerImageArtifact) -> None: + """Get ready to run ``image`` by its name, before create_sandbox or create_container is asked for it and outside + any deadline on creating one. Does nothing here; a provider that builds a context-only image itself builds + it.""" + async def _start_container(self, sandbox: VmSandbox, *, image_name: str, port: int, env: dict[str, str]) -> None: """Run the pulled ``image_name`` as the sandbox's container, publishing ``port``.""" args = self._container_args(sandbox, image_name=image_name, port=port, env=env) diff --git a/tst/unit/providers/sandbox_providers/sandbox_provider_test.py b/tst/unit/providers/sandbox_providers/sandbox_provider_test.py index f674bc7d..1ddfa8c3 100644 --- a/tst/unit/providers/sandbox_providers/sandbox_provider_test.py +++ b/tst/unit/providers/sandbox_providers/sandbox_provider_test.py @@ -6,6 +6,9 @@ import pytest +import agent_env.providers.sandbox_providers.chained_sandbox_provider as chained_module + +from agent_env.artifact.artifacts.docker_image import DockerImageArtifact from agent_env.config import get_config from agent_env.config.loader import load_impl from agent_env.providers.sandbox_providers.chained_sandbox_provider import ChainedSandboxProvider @@ -280,6 +283,57 @@ async def test_chain_create_sandbox_all_fail_raises_with_all_names(): assert "second boom" in msg +_CONTEXT_ONLY = DockerImageArtifact(id="solver-image", version=2, description="d", image_name="local/solver-0123456789ab:v2", + build_context_object_url="s3://bucket/ctx.tar.gz") + + +class _PreparingProvider(_StubProvider): + """A stub whose prepare_image takes ``prepare_seconds``, or raises ``prepare_raises``, recording each image.""" + + def __init__(self, *, prepare_seconds: float = 0, prepare_raises: Exception | None = None, **kwargs): + super().__init__(**kwargs) + self.prepare_seconds, self.prepare_raises, self.prepared = prepare_seconds, prepare_raises, [] + + async def prepare_image(self, image): + self.prepared.append(image.image_name) + await asyncio.sleep(self.prepare_seconds) + if self.prepare_raises: + raise self.prepare_raises + + +async def _chain_create(chain: ChainedSandboxProvider) -> Sandbox: + await chain.prepare_image(_CONTEXT_ONLY) + return await chain.create_sandbox(image_name=_CONTEXT_ONLY.image_name, port=8000, env={}) + + +@pytest.mark.asyncio +async def test_chain_prepares_a_provider_outside_the_deadline_on_creating_its_sandbox(monkeypatch): + monkeypatch.setattr(chained_module, "_PROVISION_DEADLINE_SECONDS", 0.05) + builds = _PreparingProvider(prepare_seconds=0.2) + + assert await _chain_create(ChainedSandboxProvider([builds])) is builds._sandbox + assert builds.prepared == [_CONTEXT_ONLY.image_name] + + +@pytest.mark.asyncio +async def test_chain_prepares_only_the_providers_it_tries_and_falls_through_a_failed_prepare(): + fails = _PreparingProvider(prepare_raises=RuntimeError("the build failed")) + second = _PreparingProvider(sandbox=_FakeSandbox("second")) + third = _PreparingProvider() + + assert (await _chain_create(ChainedSandboxProvider([fails, second, third]))).sandbox_id == "second" + assert (fails.create_sandbox_calls, fails.prepared) == (0, [_CONTEXT_ONLY.image_name]) + assert (second.prepared, third.prepared) == ([_CONTEXT_ONLY.image_name], []) + + +@pytest.mark.asyncio +async def test_chain_prepares_nothing_for_an_image_it_wasnt_told_of(): + provider = _PreparingProvider() + + await ChainedSandboxProvider([provider]).create_sandbox(image_name="registry.example/solver:v2", port=8000, env={}) + assert provider.prepared == [] + + @pytest.mark.asyncio async def test_chain_get_sandbox_returns_first_success(): second_sandbox = _FakeSandbox("second") From 949c484664dfb1e5841d08b80ab6f2bff2159961 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Thu, 8 Oct 2026 23:19:37 -0700 Subject: [PATCH 03/10] feat(images): unpack a build context, and name the ignore file a build reads build_context.extract unpacks a context's tar.gz: its files, folders and links, leaving out a link that leads out of the folder, which nothing in the build can use, and refusing what no context holds, such as a hard link, a device or a name leading out. ignore_file, now public, names the ignore file docker build reads, so a build elsewhere applies the same one. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/utils/build_context.py | 32 +++++++++++++-- tst/unit/utils/build_context_test.py | 59 +++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/src/agent_env/utils/build_context.py b/src/agent_env/utils/build_context.py index 3f2513b8..c00ad88f 100644 --- a/src/agent_env/utils/build_context.py +++ b/src/agent_env/utils/build_context.py @@ -52,14 +52,14 @@ def of(cls, root: Path, dockerfile: Path | None = None) -> BuildContext: if not root.is_dir(): raise ValueError(f"build context {root} is not a folder") top = Path(os.path.realpath(root)) - ignore_file = _ignore_file(top, dockerfile) + ignores = ignore_file(top, dockerfile) # Sent whatever the ignore file says, and through any link to them, so a build finds them in the context. - sent = [path for path in (dockerfile, ignore_file) if path is not None] + sent = [path for path in (dockerfile, ignores) if path is not None] kept = {name for path in sent for name in (_inside(top, path), _inside(top, Path(os.path.realpath(path)))) if name} named = None if dockerfile is not None and _inside(top, Path(os.path.realpath(dockerfile))): named = _inside(top, dockerfile) - matcher = FilePatternMatcher.from_file(ignore_file) if ignore_file else None + matcher = FilePatternMatcher.from_file(ignores) if ignores else None walk = _Walk(top, matcher, kept) walk.run() if walk.problems: @@ -99,6 +99,14 @@ def write(self, out: Path) -> None: raise RuntimeError(f"{entry.name} changed while its build context was being written") +def extract(archive: Path, out: Path) -> None: + """Unpack a build context's tar.gz into the folder ``out``: its files, folders and links. A link that leads out + of ``out`` is left out, as nothing there is the build's. Raises ValueError on anything else, which no context + written by ``BuildContext.write`` holds: a hard link, a device, a name that leads out of ``out``.""" + with tarfile.open(archive, "r:gz") as tar: + tar.extractall(out, filter=_context_member) + + class _Walk: """Lists a context folder in sorted order.""" @@ -198,7 +206,23 @@ def hexdigest(self) -> str: return self._digest.hexdigest() -def _ignore_file(top: Path, dockerfile: Path | None) -> Path | None: +def _context_member(member: tarfile.TarInfo, out: str) -> tarfile.TarInfo | None: + """``member`` as ``extract`` writes it, None to leave it out.""" + if not (member.isreg() or member.isdir() or member.issym()): + raise ValueError(f"{member.name!r} in the build context is neither a file, a folder nor a link") + try: + member = tarfile.tar_filter(member, out) + except tarfile.FilterError as e: + raise ValueError(f"{member.name!r} in the build context: {e}") from e + if member.issym(): + top = os.path.realpath(out) + target = os.path.realpath(os.path.join(top, os.path.dirname(member.name), member.linkname)) + if os.path.commonpath([top, target]) != top: + return None + return member + + +def ignore_file(top: Path, dockerfile: Path | None) -> Path | None: """The ignore file ``docker build`` reads: ``.dockerignore`` beside the Dockerfile, else the context's ``.dockerignore``.""" candidates = [dockerfile.with_name(f"{dockerfile.name}.dockerignore")] if dockerfile is not None else [] diff --git a/tst/unit/utils/build_context_test.py b/tst/unit/utils/build_context_test.py index 2669d837..ea5b91e6 100644 --- a/tst/unit/utils/build_context_test.py +++ b/tst/unit/utils/build_context_test.py @@ -9,7 +9,7 @@ import pytest -from agent_env.utils.build_context import BuildContext +from agent_env.utils.build_context import BuildContext, extract, ignore_file def _tree(root: Path, files: dict[str, str], executable: tuple[str, ...] = ()) -> Path: @@ -180,6 +180,63 @@ def test_the_written_tar_extracts_to_the_context(tmp_path): assert (out / "data" / "empty" / ".keep").exists() +def test_extract_unpacks_files_folders_and_links_inside_and_leaves_out_links_leading_out(tmp_path): + root = _tree(tmp_path / "ctx", SERVER, executable=("app/run.sh",)) + (root / "lib").symlink_to("app", target_is_directory=True) + (root / "app" / "up").symlink_to("../data") + (root / ".venv-python").symlink_to("/usr/bin/python3") + (root / "app" / "escape").symlink_to("../../outside") + (root / "app" / "loop").symlink_to(root, target_is_directory=True) + archive = tmp_path / "ctx.tar.gz" + BuildContext.of(root, root / "Dockerfile").write(archive) + + extract(archive, tmp_path / "out") + + out = tmp_path / "out" + assert (out / "app" / "main.py").read_text() == "print(1)\n" and os.access(out / "app" / "run.sh", os.X_OK) + assert (out / "data" / "empty").is_dir() + assert (os.readlink(out / "lib"), os.readlink(out / "app" / "up")) == ("app", "../data") + assert not any(os.path.lexists(out / name) for name in (".venv-python", "app/escape", "app/loop")) + + +def _archive(tmp_path: Path, *members: tuple[tarfile.TarInfo, bytes]) -> Path: + archive = tmp_path / "odd.tar.gz" + with tarfile.open(archive, "w:gz") as tar: + for info, data in members: + info.size = len(data) + tar.addfile(info, io.BytesIO(data)) + return archive + + +def _member(name: str, kind: bytes = tarfile.REGTYPE, linkname: str = "") -> tarfile.TarInfo: + info = tarfile.TarInfo(name) + info.type, info.linkname = kind, linkname + return info + + +@pytest.mark.parametrize("member, problem", [ + (_member("hard", tarfile.LNKTYPE, "Dockerfile"), "is neither a file, a folder nor a link"), + (_member("pipe", tarfile.FIFOTYPE), "is neither a file, a folder nor a link"), + (_member("../outside"), "would be extracted to"), +], ids=["hard link", "fifo", "name leading out"]) +def test_extract_refuses_what_no_build_context_holds(tmp_path, member, problem): + archive = _archive(tmp_path, (_member("Dockerfile"), b"FROM scratch\n"), (member, b"")) + + with pytest.raises(ValueError, match=problem): + extract(archive, tmp_path / "out") + + +def test_ignore_file_is_the_dockerfiles_own_else_the_contexts(tmp_path): + root = _tree(tmp_path / "ctx", {**SERVER, "docker/app.Dockerfile": "FROM python:3.12\n"}) + assert ignore_file(root, root / "Dockerfile") is None + + (root / ".dockerignore").write_text("*.log\n") + assert ignore_file(root, root / "docker" / "app.Dockerfile") == root / ".dockerignore" + + (root / "docker" / "app.Dockerfile.dockerignore").write_text("data\n") + assert ignore_file(root, root / "docker" / "app.Dockerfile") == root / "docker" / "app.Dockerfile.dockerignore" + + def test_a_dockerfile_that_is_a_link_keeps_its_own_name_and_brings_its_target(tmp_path): root = _tree(tmp_path / "ctx", {**SERVER, "docker/real.Dockerfile": "FROM python:3.12\n", ".dockerignore": "docker\n", "Dockerfile.dockerignore": "app\n", "docker/real.Dockerfile.dockerignore": "data\n"}) From a099be611242a33d0e73c3a16700e164d282de68 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Thu, 8 Oct 2026 23:20:02 -0700 Subject: [PATCH 04/10] feat(modal): build an image that is only a build context, with from_dockerfile ModalSandboxProvider.prepare_image downloads a context-only image's build context, unpacks it, and builds it with modal.Image.from_dockerfile and the context's own ignore file, then create_container runs the built image instead of pulling its name. The built image's id is kept per Modal token, app and source digest for the life of the process, and one build of each runs at a time, so deploys of the same sources share a build; a failed build is built again by the next deploy, and create_container builds an image it was told of on a miss. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../sandbox_providers/modal_sandbox.py | 116 ++++++++++--- .../modal_context_build_test.py | 162 ++++++++++++++++++ .../modal_create_signature_test.py | 22 +++ 3 files changed, 274 insertions(+), 26 deletions(-) create mode 100644 tst/unit/providers/sandbox_providers/modal_context_build_test.py diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index 5ee459fb..fd5ea2bb 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -10,7 +10,10 @@ import os import re import shlex -from typing import AsyncIterator, ClassVar, Optional +import tempfile +import weakref +from pathlib import Path +from typing import TYPE_CHECKING, AsyncIterator, ClassVar, Optional import httpx import modal @@ -23,9 +26,20 @@ SandboxProvider, apply_default_attribution, ) +from agent_env.utils.build_context import extract, ignore_file + +if TYPE_CHECKING: + from agent_env.artifact.artifacts.docker_image import DockerImageArtifact logger = logging.getLogger(__name__) +# The id of each image built from a build context, by Modal token, app and source digest, for every provider in the +# process to reuse; one build of each runs at a time, so deploys of one image at once share its build. +_built_image_ids: dict[tuple[str, str, str], str] = {} +_build_locks: weakref.WeakValueDictionary[tuple[asyncio.AbstractEventLoop, tuple[str, str, str]], asyncio.Lock] = ( + weakref.WeakValueDictionary() +) + DEFAULT_APP_NAME = "agent-env" # Lets a dedicated service (e.g. the sandbox proxy service) attribute its Modal apps under its own # base name instead of the shared default. Resolved per-construction, so it's read @@ -206,12 +220,16 @@ def __init__( self._gpu = gpu # Looked up on demand and cached by name. self._apps: dict[str, modal.App] = {} + self._token_id = "" + # The context-only images prepare_image was given, by name, so create_container builds them rather than pull. + self._context_images: dict[str, DockerImageArtifact] = {} async def _get_client(self) -> modal.Client: if self._client is None: from agent_env.config import get_config token_id, token_secret = get_config().get_modal_credentials() self._client = await modal.Client.from_credentials.aio(token_id, token_secret) + self._token_id = token_id return self._client async def _get_app(self, app_name: str, app_tags: Optional[dict[str, str]] = None) -> modal.App: @@ -229,6 +247,42 @@ async def _get_app(self, app_name: str, app_tags: Optional[dict[str, str]] = Non self._apps[app_name] = app return app + async def prepare_image(self, image: DockerImageArtifact) -> None: + """Build ``image`` when it's only a build context, unless this process has already built its sources.""" + if image.context_only: + self._context_images[image.image_name] = image + await self._context_image(image) + + async def _context_image(self, image: DockerImageArtifact) -> modal.Image: + """The Modal image built from ``image``'s build context: built now unless one built from the same sources is.""" + client = await self._get_client() + app = await self._get_app(self._app_name, _attribution_tags({})) + key = (self._token_id, self._app_name, image.source_digest or image.build_context_object_url) + async with _build_locks.setdefault((asyncio.get_running_loop(), key), asyncio.Lock()): + if (image_id := _built_image_ids.get(key)) is None: + image_id = _built_image_ids[key] = await self._build(image, app) + return modal.Image.from_id(image_id, client=client) + + async def _build(self, image: DockerImageArtifact, app: modal.App) -> str: + """Build ``image`` from its build context, with the context's own ignore file; returns the image's id.""" + logger.info(f"Building {image.image_name} on Modal from its build context...") + with tempfile.TemporaryDirectory(prefix="agent-env-build-") as work: + context = Path(work) / "context" + try: + await asyncio.to_thread(_fetch_context, image.build_context_object_url, Path(work) / "context.tar.gz", + context) + dockerfile = context / (image.dockerfile_path or "Dockerfile") + ignores = ignore_file(context, dockerfile) + built = modal.Image.from_dockerfile( + dockerfile, context_dir=context, ignore=ignores.read_text("utf8").splitlines() if ignores else [], + ) + await built.build.aio(app) + except Exception as e: + raise RuntimeError(f"Building {image.image_name} on Modal from its build context failed: " + f"{_fmt_exc(e)}") from e + logger.info(f"Built {image.image_name} on Modal as {built.object_id}") + return built.object_id + async def create_container( self, *, @@ -248,8 +302,6 @@ async def create_container( expose_externally: bool = True, vnc_port: Optional[int] = None, ) -> Sandbox: - from agent_env.config import get_config - effective = self.effective_network_policy(network_policy) attribution = dict(attribution or {}) @@ -260,30 +312,10 @@ async def create_container( app_name = self._app_name app = await self._get_app(app_name, app_tags) - image_store = get_config().get_image_store_at(image_name) - from agent_env.store.image_store import ( - EcrCredentials, - OciRegistryImageStore, - registry_host_from_ref, - ) - - uses_ecr_pull_secret = ( - self._ecr_pull_secret_name is not None - and isinstance(image_store, OciRegistryImageStore) - and isinstance(image_store.credentials, EcrCredentials) - and registry_host_from_ref(image_name) == image_store.registry_host - ) - auth = None if uses_ecr_pull_secret else await asyncio.to_thread(image_store.auth, image_name) - if uses_ecr_pull_secret: - image = modal.Image.from_aws_ecr(image_name, secret=modal.Secret.from_name( - self._ecr_pull_secret_name, required_keys=ECR_READER_SECRET_KEYS)) - elif auth is not None: - image = modal.Image.from_registry(image_name, secret=modal.Secret.from_dict({ - "REGISTRY_USERNAME": auth.username, - "REGISTRY_PASSWORD": auth.password, - })) + if (context_only := self._context_images.get(image_name)) is not None: + image = await self._context_image(context_only) else: - image = modal.Image.from_registry(image_name) + image = await self._registry_image(image_name) logger.info( f"Creating Modal sandbox from image {image_name} " @@ -401,6 +433,38 @@ async def get_sandbox(self, sandbox_id: str) -> Sandbox: i6pn_address = await _resolve_i6pn_address(sb) return self._sandbox_cls(sb, tunnel_urls, i6pn_address=i6pn_address) + async def _registry_image(self, image_name: str) -> modal.Image: + """``image_name`` pulled from its registry, with the image store's credentials for it, if any.""" + image_store = get_config().get_image_store_at(image_name) + from agent_env.store.image_store import ( + EcrCredentials, + OciRegistryImageStore, + registry_host_from_ref, + ) + + uses_ecr_pull_secret = ( + self._ecr_pull_secret_name is not None + and isinstance(image_store, OciRegistryImageStore) + and isinstance(image_store.credentials, EcrCredentials) + and registry_host_from_ref(image_name) == image_store.registry_host + ) + auth = None if uses_ecr_pull_secret else await asyncio.to_thread(image_store.auth, image_name) + if uses_ecr_pull_secret: + return modal.Image.from_aws_ecr(image_name, secret=modal.Secret.from_name( + self._ecr_pull_secret_name, required_keys=ECR_READER_SECRET_KEYS)) + if auth is not None: + return modal.Image.from_registry(image_name, secret=modal.Secret.from_dict({ + "REGISTRY_USERNAME": auth.username, + "REGISTRY_PASSWORD": auth.password, + })) + return modal.Image.from_registry(image_name) + + +def _fetch_context(object_url: str, archive: Path, out: Path) -> None: + """Download the build context at ``object_url`` to ``archive`` and unpack it into ``out``.""" + get_config().get_object_store_at(object_url).download_to_file(object_url, str(archive)) + extract(archive, out) + def _attribution_tags(attribution: Attribution) -> dict[str, str]: """``attribution`` as Modal tags, any keys: unset ones filled from config.toml diff --git a/tst/unit/providers/sandbox_providers/modal_context_build_test.py b/tst/unit/providers/sandbox_providers/modal_context_build_test.py new file mode 100644 index 00000000..9b30920a --- /dev/null +++ b/tst/unit/providers/sandbox_providers/modal_context_build_test.py @@ -0,0 +1,162 @@ +"""Modal builds an image that is only a build context from its context, unpacked on this machine, with the context's own +ignore file: once per process for the same sources, before its container, and again on the next deploy after a failure. +Modal's build and sandbox calls are faked; the context is a real one on the local stores.""" + +from __future__ import annotations + +import asyncio +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import modal +import pytest + +from agent_env.artifact.artifacts.docker_image import DockerImageArtifact +from agent_env.providers.sandbox_providers import modal_sandbox +from agent_env.providers.sandbox_providers.modal_sandbox import ModalSandboxProvider + +SERVER = {"Dockerfile": "FROM python:3.12\nCOPY . /app\n", "src/app.py": "print(1)\n", ".dockerignore": "*.log\n", + "debug.log": "noise"} + + +class _Builds: + """Stands in for ``modal.Image.from_dockerfile``: each image it returns records its context when built.""" + + def __init__(self, fail: Exception | None = None): + self.fail, self.calls, self.contexts = fail, [], [] + + def __call__(self, path, *, context_dir, ignore): + self.calls.append((Path(path), Path(context_dir), ignore)) + image = MagicMock() + + async def build(app): + self.contexts.append(sorted(p.relative_to(context_dir).as_posix() for p in Path(context_dir).rglob("*"))) + await asyncio.sleep(0.01) # long enough for a second deploy to wait on the first + if self.fail: + raise self.fail + image.object_id = f"im-{len(self.contexts)}" + + image.build.aio = AsyncMock(side_effect=build) + return image + + +@pytest.fixture(autouse=True) +def _no_built_images(monkeypatch): + monkeypatch.setattr(modal_sandbox, "_built_image_ids", {}) + + +@pytest.fixture +def context(tmp_path): + root = tmp_path / "ctx" + for name, text in SERVER.items(): + (root / name).parent.mkdir(parents=True, exist_ok=True) + (root / name).write_text(text) + return root + + +@pytest.fixture +def builds(monkeypatch): + builds = _Builds() + monkeypatch.setattr(modal.Image, "from_dockerfile", builds) + monkeypatch.setattr(modal.Image, "from_id", lambda image_id, client=None: ("from_id", image_id)) + return builds + + +def _provider() -> ModalSandboxProvider: + provider = ModalSandboxProvider(app_name="agent-env-test") + provider._get_client = AsyncMock(return_value=MagicMock()) + provider._get_app = AsyncMock(return_value="app") + return provider + + +def _context_image(id: str, context: Path) -> DockerImageArtifact: + return DockerImageArtifact.put_context(id, description="d", context_path=str(context), dockerfile_path="Dockerfile") + + +@pytest.mark.asyncio +async def test_an_image_is_built_from_its_unpacked_context_with_the_contexts_ignore_file(local_stores, context, builds): + image = _context_image("solver-image", context) + + await _provider().prepare_image(image) + + [(dockerfile, context_dir, ignore)] = builds.calls + assert dockerfile == context_dir / "Dockerfile" + assert builds.contexts == [[".dockerignore", "Dockerfile", "src", "src/app.py"]] + assert ignore == ["*.log"] + assert not context_dir.exists() # unpacked only for the build + + +@pytest.mark.asyncio +async def test_a_context_with_no_ignore_file_is_built_ignoring_nothing(local_stores, context, builds): + (context / ".dockerignore").unlink() + + await _provider().prepare_image(_context_image("solver-image", context)) + + [(_, _, ignore)] = builds.calls + assert ignore == [] + + +@pytest.mark.asyncio +async def test_deploys_of_the_same_sources_at_once_share_one_build_and_later_ones_reuse_it(local_stores, context, builds): + first, second = _context_image("solver-image", context), _context_image("judge-image", context) + assert first.source_digest == second.source_digest + + await asyncio.gather(_provider().prepare_image(first), _provider().prepare_image(second)) + await _provider().prepare_image(first) + + assert len(builds.calls) == 1 + + +@pytest.mark.asyncio +async def test_a_failed_build_names_the_image_and_the_next_deploy_builds_it_again(local_stores, context, builds): + image = _context_image("solver-image", context) + builds.fail = RuntimeError("step 2/2 COPY failed") + + with pytest.raises(RuntimeError, match=rf"Building {image.image_name} on Modal from its build context failed: " + r"builtins.RuntimeError: step 2/2 COPY failed"): + await _provider().prepare_image(image) + builds.fail = None + await _provider().prepare_image(image) + + assert len(builds.calls) == 2 + + +@pytest.mark.asyncio +async def test_an_image_that_is_not_only_a_build_context_is_not_built(local_stores, builds): + await _provider().prepare_image(DockerImageArtifact(id="solver-image", description="d", + image_name="registry.example/solver:v2")) + + assert builds.calls == [] + + +def _fake_create(): + """A patched ``modal.Sandbox._experimental_create`` that stops right after recording its call.""" + create = MagicMock() + create.aio = AsyncMock(side_effect=RuntimeError("stop")) + return patch.object(modal_sandbox.modal.Sandbox, "_experimental_create", create) + + +@pytest.mark.asyncio +async def test_a_container_runs_the_built_image_and_builds_it_on_a_miss(local_stores, context, builds): + image = _context_image("solver-image", context) + provider = _provider() + await provider.prepare_image(image) + modal_sandbox._built_image_ids.clear() + + with _fake_create() as create, pytest.raises(RuntimeError, match="Modal sandbox create failed"): + await provider.create_container(image_name=image.image_name, port=8000, env={}) + + assert create.aio.call_args.kwargs["image"] == ("from_id", "im-2") + assert len(builds.calls) == 2 + + +@pytest.mark.asyncio +async def test_a_container_of_an_image_it_wasnt_told_of_is_pulled(local_stores, builds, monkeypatch): + pulled = MagicMock() + monkeypatch.setattr(modal.Image, "from_registry", lambda name, **kwargs: (pulled, name)) + + with _fake_create() as create, pytest.raises(RuntimeError, match="Modal sandbox create failed"): + await _provider().create_container(image_name="registry.example/solver:v2", port=8000, env={}) + + assert create.aio.call_args.kwargs["image"] == (pulled, "registry.example/solver:v2") + assert builds.calls == [] diff --git a/tst/unit/providers/sandbox_providers/modal_create_signature_test.py b/tst/unit/providers/sandbox_providers/modal_create_signature_test.py index c41b28e5..480a43d6 100644 --- a/tst/unit/providers/sandbox_providers/modal_create_signature_test.py +++ b/tst/unit/providers/sandbox_providers/modal_create_signature_test.py @@ -11,8 +11,10 @@ import modal import pytest +from agent_env.artifact.artifacts.docker_image import DockerImageArtifact from agent_env.attribution import PIPELINE_STEP_KEY from agent_env.config import get_config, reset_config +from agent_env.providers.sandbox_providers import modal_sandbox from agent_env.providers.sandbox_providers.modal_sandbox import ModalSandboxProvider from agent_env.providers.sandbox_providers.modal_vm_sandbox import ModalVmSandboxProvider from agent_env.providers.sandbox_providers.sandbox import NetworkMode, NetworkPolicy @@ -65,3 +67,23 @@ async def test_the_installed_modal_accepts_every_create_argument(name): call = fake.aio.call_args assert call is not None, f"{name} never reached modal.Sandbox.{method}" signature.bind(*call.args, **call.kwargs) + + +@pytest.mark.asyncio +async def test_the_installed_modal_accepts_every_argument_of_a_build_from_a_context(local_stores, tmp_path, monkeypatch): + (tmp_path / "Dockerfile").write_text("FROM python:3.12\n") + image = DockerImageArtifact.put_context("solver-image", description="d", context_path=str(tmp_path), + dockerfile_path="Dockerfile") + monkeypatch.setattr(modal_sandbox, "_built_image_ids", {}) + from_dockerfile, from_id = inspect.signature(modal.Image.from_dockerfile), inspect.signature(modal.Image.from_id) + built = MagicMock(object_id="im-1") + built.build.aio = AsyncMock() + fake_from_dockerfile, fake_from_id = MagicMock(return_value=built), MagicMock() + provider = _provider(ModalSandboxProvider) + + with patch.object(modal.Image, "from_dockerfile", fake_from_dockerfile), patch.object(modal.Image, "from_id", fake_from_id): + await provider.prepare_image(image) + + from_dockerfile.bind(*fake_from_dockerfile.call_args.args, **fake_from_dockerfile.call_args.kwargs) + inspect.signature(modal.Image.build.aio).bind(built, *built.build.aio.call_args.args, **built.build.aio.call_args.kwargs) + from_id.bind(modal.Image, *fake_from_id.call_args.args, **fake_from_id.call_args.kwargs) # its cls, then the call From 4b369cf3e14208a7bc6beb53608ac9eebdc38994 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Thu, 8 Oct 2026 23:20:09 -0700 Subject: [PATCH 05/10] feat(images): deploy an image that is only a build context on Modal Modal now builds such an image, so the guards that refused it there are lifted: an agent on Modal or on a chain holding it, Modal's gateway and its MCP servers, and a lone server on Modal each prepare the image before creating anything, and the gateway builds all of its images before its first container. A container on the local provider, a lone server on any other provider, and an agent attached to an existing container sandbox still refuse it, and preflight agrees, naming a provider that builds it. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- src/agent_env/a2a_agent/a2a_agent.py | 5 +- .../artifact/artifacts/docker_image.py | 7 +- src/agent_env/bundle/ledger.py | 4 +- src/agent_env/bundle/materialize.py | 15 +-- src/agent_env/bundle/preflight.py | 49 ++++++---- src/agent_env/bundle/run.py | 2 +- .../env_providers/env_gateway_provider.py | 6 +- .../env_providers/env_server_provider.py | 9 +- .../a2a_agent/context_only_deploy_test.py | 55 ++++++++++- .../test_docker_image_build_context.py | 4 +- tst/unit/bundle/materialize_test.py | 4 +- tst/unit/bundle/preflight_test.py | 97 ++++++++++--------- .../env_gateway_provider_test.py | 9 +- .../env_providers/env_server_provider_test.py | 36 ++++++- 15 files changed, 204 insertions(+), 100 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 878b7712..0430de79 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -91,7 +91,7 @@ and deserialized through a registry. | Package | Contents | |---|---| | `env/` | `Env` base class: a subclass declares `type`, implements `from_dict` and `async deploy(...) -> DeployedEnv`, and may override `reset` and `load_file_artifact_universe`. Built-ins in `env/envs/`: `MCPServerEnv` (`mcp_server`), `MultiEnv` (`multi`, MCP servers plus websites), `ServiceDBEnv` (`service_db`, PostgreSQL), `WebsiteEnv` (`website`), `GatewayEnv` (`gateway_server`). `env/gateway/gateway.py` is the env-side gateway server, container-only code. | -| `artifact/` | Immutable versioned artifacts; every `put()` creates a new version. The document goes to the document store, binary payloads to the object store. Built-ins: `CliArtifact` (`cli`), `FileArtifact` (`file`), `FileArtifactUniverse` (`file_artifact_universe`), `DockerImageArtifact` (`docker_image`), `EnvironmentArtifact` (`environment`; legacy `service`), `EnvironmentUniverseArtifact` (`environment_universe`; legacy `service_universe`), `SkillArtifact` (`skill`). A `DockerImageArtifact` holds a tar.gz of its image, or, written by `put_ref` (`--image-ref` on `env mcp-server put` and `a2a-agent put`), only a registry reference with its tag pinned to a digest, which sandboxes pull, or, written by `put_context`, only its build context, which a VM sandbox builds before running it (`VmSandbox.load_docker_images`); anything that runs images by name (a container on the local or Modal provider, a lone server) refuses such an image. A build context (`utils/build_context.py`) is what `docker build` would send, `.dockerignore` applied, written so equal files give equal bytes and an equal `source_digest`. | +| `artifact/` | Immutable versioned artifacts; every `put()` creates a new version. The document goes to the document store, binary payloads to the object store. Built-ins: `CliArtifact` (`cli`), `FileArtifact` (`file`), `FileArtifactUniverse` (`file_artifact_universe`), `DockerImageArtifact` (`docker_image`), `EnvironmentArtifact` (`environment`; legacy `service`), `EnvironmentUniverseArtifact` (`environment_universe`; legacy `service_universe`), `SkillArtifact` (`skill`). A `DockerImageArtifact` holds a tar.gz of its image, or, written by `put_ref` (`--image-ref` on `env mcp-server put` and `a2a-agent put`), only a registry reference with its tag pinned to a digest, which sandboxes pull, or, written by `put_context`, only its build context, which a VM sandbox builds before running it (`VmSandbox.load_docker_images`) and Modal builds before creating its container (`ModalSandboxProvider.prepare_image`, outside a fallback chain's deadline); anything else that runs images by name (a container on the local provider, a lone server on any provider but Modal) refuses such an image. A build context (`utils/build_context.py`) is what `docker build` would send, `.dockerignore` applied, written so equal files give equal bytes and an equal `source_digest`. | | `task/`, `task_step/` | A `Task` holds its `TaskStep`s inline; `Task.run()` executes them as a DAG. `depends_on` (None means all prior steps) gates a step, independent steps run concurrently, `fail_task_on_error` makes a failure fatal or tolerated, `retry_config` rolls a failed span back through the step journal and re-dispatches it. Built-in steps live in `task_step/task_steps/` (`deploy_env`, `deploy_agent`, `prompt_agent`, the verifiers under `verifiers/`, and more). | | `store/` | Four store ABCs with local and cloud implementations: `DocumentStore` (SQLite, MongoDB), `ObjectStore` (filesystem, S3, Cloud Storage), `ImageStore` (local OCI registry, ECR), `SecretStore` (env vars or file, AWS Secrets Manager, Google Cloud Secret Manager). `VersionedEntityStore` implements the shared versioned get/put logic, `QueryBuilder` is the immutable chained query API, `store/base.py` holds the error types. A new backend must pass the conformance kits in `tst/store/`. | | `config/` | The `Config` singleton (`get_config`, `configure`, `reset_config`) in `config/runtime.py`, file discovery in `config/loader.py`, and `load_impl`, which resolves `module:Class` pointers. `agent_env.store` re-exports the config names for compatibility. | diff --git a/src/agent_env/a2a_agent/a2a_agent.py b/src/agent_env/a2a_agent/a2a_agent.py index 1b575c36..dde14ffd 100644 --- a/src/agent_env/a2a_agent/a2a_agent.py +++ b/src/agent_env/a2a_agent/a2a_agent.py @@ -34,7 +34,6 @@ start_trusting, ) from agent_env.providers.sandbox_providers.chained_sandbox_provider import ChainedSandboxProvider -from agent_env.providers.sandbox_providers.modal_sandbox import ModalSandboxProvider from agent_env.providers.sandbox_providers.sandbox_provider import all_sandbox_container_env, all_sandbox_url_rewrites from agent_env.attribution import Attribution @@ -438,13 +437,15 @@ async def deploy( raise ValueError(f"Can't deploy agent {self.id!r} on the container sandbox {sandbox.sandbox_id!r}, which " f"runs its own image, never the agent's: {problem}") links = provider.providers if isinstance(provider, ChainedSandboxProvider) else [provider] if provider else [] - if by_name := [link for link in links if isinstance(link, (LocalSandboxProvider, ModalSandboxProvider))]: + if by_name := [link for link in links if isinstance(link, LocalSandboxProvider)]: raise ValueError(f"Can't deploy agent {self.id!r} with {type(by_name[0]).__name__}, which runs an " f"agent's image by name: {problem}") try: if provider is not None: logger.info(f"Provisioning sandbox for A2A agent '{self.id}' via {type(provider).__name__}...") + if self.docker_image_artifact.context_only: + await provider.prepare_image(self.docker_image_artifact) self._sandbox = await provider.create_sandbox( image_name=image_name, port=a2a_port, env=merged_env, cpu=cpu, memory=memory, disk_size_gb=disk_size_gb, timeout=ttl_seconds, diff --git a/src/agent_env/artifact/artifacts/docker_image.py b/src/agent_env/artifact/artifacts/docker_image.py index 9cf2edba..6064a0cc 100644 --- a/src/agent_env/artifact/artifacts/docker_image.py +++ b/src/agent_env/artifact/artifacts/docker_image.py @@ -271,7 +271,7 @@ def load(self) -> bytes: @property def context_only(self) -> bool: """Whether only the image's build context is stored: no tar.gz, and an image name that names no registry, the - tag a VM sandbox gives the image when it builds it.""" + tag the image's build gives it.""" return bool(self.build_context_object_url) and not self.tar_gz_object_url and not names_registry(self.image_name) def load_problem(self) -> str | None: @@ -284,9 +284,10 @@ def load_problem(self) -> str | None: def by_name_problem(self) -> str | None: """Why a sandbox that runs an image by pulling its name can't run this one, or None when it can: a context-only - image's name is a tag only a VM sandbox's build gives it, and pulled, it would be looked up on Docker Hub.""" + image's name is a tag only its build gives it, a VM sandbox's or Modal's, and pulled, it would be looked up on + Docker Hub.""" if self.context_only: - return f"{self.id!r} v{self.version} is only a build context, which only a VM sandbox builds" + return f"{self.id!r} v{self.version} is only a build context, which has to be built, not pulled" return None @classmethod diff --git a/src/agent_env/bundle/ledger.py b/src/agent_env/bundle/ledger.py index cf64d38d..3effdfac 100644 --- a/src/agent_env/bundle/ledger.py +++ b/src/agent_env/bundle/ledger.py @@ -42,7 +42,7 @@ SCHEME = 1 _CONTEXT = "build context" -_FORMS = {None: "an image built on this machine", _CONTEXT: "a build context, which its VMs build"} +_FORMS = {None: "an image built on this machine", _CONTEXT: "a build context, built where it runs"} _COLLECTIONS = {"env": ENVS_COLLECTION, "agent": A2A_AGENTS_COLLECTION, "artifact": ARTIFACTS_COLLECTION, "task": TASKS_COLLECTION, "eval": EVALS_COLLECTION} @@ -53,7 +53,7 @@ class Digest: value: str inputs: dict[str, Any] # type, config, files {key: sha256}, needs and store_refs {" ": version}, and - # for an image written as its build context, its form and the platform its VMs build it for + # for an image written as its build context, its form and the platform it's built for @dataclass(frozen=True) diff --git a/src/agent_env/bundle/materialize.py b/src/agent_env/bundle/materialize.py index 0aa3ddb0..5e7ca0b5 100644 --- a/src/agent_env/bundle/materialize.py +++ b/src/agent_env/bundle/materialize.py @@ -4,11 +4,12 @@ that can't be written whole. It needs the CLI's namespace routing, which sends ``@local`` writes to the ``@local`` namespace's store. Holding the bundle's lock, it writes the entities in the plan's order, an image built from an entry's Dockerfile just before the entry: with ``docker build`` on this machine, or, for one -whose deploys build it in their VMs, as its build context alone. Then it builds and preflights every task -before writing any of them, and writes the evals last, since they name the tasks. Each write goes through the ledger, so one whose inputs haven't changed reuses the version the bundle -last wrote. A reused task keeps whatever its steps took from config when it was first written, such as a -rubrics verifier's default judge model. A dry run takes the same path, refusals, checks and preflights -included, without the lock or a single write. +whose deploys build it where they run, as its build context alone. Then it builds and preflights every task +before writing any of them, and writes the evals last, since they name the tasks. Each write goes through the +ledger, so one whose inputs haven't changed reuses the version the bundle last wrote. A reused task keeps +whatever its steps took from config when it was first written, such as a rubrics verifier's default judge +model. A dry run takes the same path, refusals, checks and preflights included, without the lock or a single +write. """ from __future__ import annotations @@ -83,7 +84,7 @@ def materialize( one stays: they're in the ledger, so the next run reuses them. ``on_wait`` is called when another run holds a lock this one needs, ``on_build`` before an image is built, and ``on_write`` after each write, reused or not. An image the bundle builds whose id is in ``contexts`` is written as its build context - alone, for the VM sandboxes that deploy it to build (``preflight_run`` decides which). + alone, for the providers that deploy it to build (``preflight_run`` decides which). ``dry_run`` checks and preflights what a run would, and writes nothing: no entity, ledger row or lock. Each write gets the version it would reuse or the store's next, which another run can take first. A step @@ -159,7 +160,7 @@ def _staged(plan: Plan, image: BuiltImage) -> Iterator[Path]: def _write_build_context(plan: Plan, write: Write) -> int: """Write the image an entry's Dockerfile describes as a docker_image artifact holding its build context alone, - which each VM sandbox that deploys it builds.""" + which each provider that deploys it builds.""" image = write.source with _staged(plan, image) as context: try: diff --git a/src/agent_env/bundle/preflight.py b/src/agent_env/bundle/preflight.py index 42372b78..5b2603e4 100644 --- a/src/agent_env/bundle/preflight.py +++ b/src/agent_env/bundle/preflight.py @@ -10,8 +10,9 @@ checked from its planned env.toml. An image the bundle builds is written in the form its deploys can run: built on this machine when they deploy on the -local provider, and as its build context alone when they deploy on providers whose VMs build it. One version is one of -the two, so a run deploying it both ways is refused, as is one deploying it on a provider that runs an image by name. +local provider, and as its build context alone when they deploy on providers that build it, in a VM or, as Modal does, +themselves. One version is one of the two, so a run deploying it both ways is refused, as is one deploying it on a +provider that runs an image by name. A gateway deploy on the local provider runs on infra envs the store may not hold yet: the gateway, the service-db its local Postgres state runs from, and the website browser it adds for websites. The run builds those once its writes are @@ -193,9 +194,10 @@ def _env(self, where: str, step: DeployEnvTaskStep) -> None: if GATEWAY in kinds and (loaders := [link for link in _links(provider) if not isinstance(link, ModalSandboxProvider)]): self._loadable(where, loaders, images) # a gateway VM loads its images; Modal's runs each by name - # A lone server runs in a container from its image's name on any provider; a gateway does on Modal. - by_name = ([link for link in _links(provider) if isinstance(link, ModalSandboxProvider)] if GATEWAY in kinds - else _links(provider)) + # A lone server runs in a container from its image's name, though Modal's builds one that's only a build context; + # a gateway VM loads its images, and Modal's gateway builds each. + by_name = [] if GATEWAY in kinds else [link for link in _links(provider) + if not isinstance(link, ModalSandboxProvider)] self._by_name(where, by_name, images) self._use(where, _links(provider), by_name, images) if _local_link(provider): @@ -305,13 +307,15 @@ def _agent_deploy(self, where: str, provider: SandboxProvider, agent_id: str | N if agent_id is None: self.default_agent_users.append((where, unnamed)) agent_id = get_config().get_default_a2a_agent_id() - by_name = [link for link in _links(provider) if isinstance(link, (LocalSandboxProvider, ModalSandboxProvider))] - loaders = [link for link in _links(provider) if link not in by_name] # by_name run an agent in a container + containers = [link for link in _links(provider) + if isinstance(link, (LocalSandboxProvider, ModalSandboxProvider))] + loaders = [link for link in _links(provider) if link not in containers] # containers run an agent's image remote = _remote_links(provider) if (image := self._agent_image(agent_id, version)) is None: return if loaders: self._loadable(where, loaders, [image]) + by_name = [link for link in containers if isinstance(link, LocalSandboxProvider)] # Modal's builds it self._by_name(where, by_name, [image]) self._use(where, _links(provider), by_name, [image]) if remote: @@ -358,12 +362,13 @@ def _loadable(self, where: str, loaders: list[SandboxProvider], images: list[_Im def _by_name(self, where: str, links: list[SandboxProvider], images: list[_Image]) -> None: """Refuse each of ``images`` a sandbox on ``links`` can't run: it pulls an image by its name, and a context-only - image's name is a tag only a VM sandbox's build gives it.""" + image's name is a tag only its build gives it.""" + on_modal = bool(links) and isinstance(links[0], ModalSandboxProvider) + builds = "whose VMs build it, such as --sandbox modal_vm" if on_modal else "that builds it, such as --sandbox modal" for image in images: if links and image.by_name: self._problem(where, f"deploys {image.what} on the {_shown(links[0])} sandbox provider, which runs it " - f"by name: {image.by_name}; run it on a provider whose VMs build it, such as " - "--sandbox modal_vm") + f"by name: {image.by_name}; run it on a provider {builds}") def _reachable(self, where: str, remote: list[SandboxProvider], images: list[_Image]) -> None: for image in images: @@ -386,7 +391,7 @@ def _remote_infra(self, where: str, provider: SandboxProvider, kinds: set[str]) images = [_image(f"the {kind} env {env_id!r}'s image {image.id!r}", image) for image in artifacts] if not isinstance(provider, ModalSandboxProvider): # whose containers run by name, or are swapped out self._loadable(where, [provider], images) - else: + elif kind != GATEWAY: # whose image Modal builds when it's only a build context self._by_name(where, [provider], images) self._reachable(where, [provider], images) @@ -397,8 +402,8 @@ def _use(self, where: str, links: list[SandboxProvider], by_name: list[SandboxPr self.uses.setdefault(image.built, []).append((where, image.what, links, by_name)) def _contexts(self) -> frozenset[str]: - """The images the bundle builds that it writes as build contexts: those every deploy runs on providers whose - VMs build them. One its deploys run only on the local provider, or that none runs, is built on this machine.""" + """The images the bundle builds that it writes as build contexts: those every deploy runs on providers that + build them. One its deploys run only on the local provider, or that none runs, is built on this machine.""" contexts = set() for image_id, uses in self.uses.items(): built = self.built[image_id] @@ -408,12 +413,12 @@ def _contexts(self) -> frozenset[str]: for link in links: if isinstance(link, LocalSandboxProvider): local.append((where, what, links, link)) - elif link in by_name or not _creates_vms(link): - how = "runs it by name" if link in by_name else "creates no VM to build it in" + elif link in by_name or not _builds(link): + how = "runs it by name" if link in by_name else "can't build it from its build context" self._problem(where, f"deploys {what} on the {_shown(link)} sandbox provider, which {how}, and " - f"the bundle builds it from {source}, which a sandbox runs only once built " - "on this machine or in its VM; run it with --sandbox local, or on a " - "provider whose VMs build it, such as --sandbox modal_vm") + f"the bundle builds it from {source}, which a sandbox runs only once it's " + "built on this machine or by the provider it runs on; run it with " + "--sandbox local, or on a provider that builds it, such as --sandbox modal") else: vms.append((where, what, links, link)) if local and vms: @@ -421,7 +426,7 @@ def _contexts(self) -> frozenset[str]: on = (f"{_named(links)} sandbox provider" if elsewhere == where else f"{_shown(link)} sandbox provider, and {elsewhere} on the {_shown(vm)} one") self._problem(where, f"deploys {what} on the {on}: the bundle builds it from {source} on this machine " - "for the local provider, and as a build context for one whose VMs build it, and a " + "for the local provider, and as a build context for one that builds it, and a " "run writes it one way; run its deploys on one kind, such as --sandbox local or " "--sandbox modal_vm") elif vms: @@ -489,6 +494,12 @@ def _creates_vms(provider: SandboxProvider) -> bool: return type(provider).create_vm is not SandboxProvider.create_vm +def _builds(provider: SandboxProvider) -> bool: + """Whether ``provider`` builds an image that's only a build context: in a VM it creates, or itself before running + it (``prepare_image``), as Modal does.""" + return _creates_vms(provider) or type(provider).prepare_image is not SandboxProvider.prepare_image + + def _shown(provider: SandboxProvider) -> str: return _named(_links(provider)) diff --git a/src/agent_env/bundle/run.py b/src/agent_env/bundle/run.py index 4c8ad1c2..d36e5b2e 100644 --- a/src/agent_env/bundle/run.py +++ b/src/agent_env/bundle/run.py @@ -162,7 +162,7 @@ def images(self) -> tuple[str, ...]: plan, contexts = self.materialization.plan, self.materialization.contexts return tuple( f"{_label(plan, done.write)} v{done.version}: " - + (f"a build context, which each VM deploying it builds for {DEFAULT_BUILD_PLATFORM}" + + (f"a build context, which each deploy builds where it runs, for {DEFAULT_BUILD_PLATFORM}" if done.write.id in contexts else "built on this machine") for done in self.materialization.writes if isinstance(done.write.source, BuiltImage) ) 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 a842203c..c61542f0 100644 --- a/src/agent_env/providers/env_providers/env_gateway_provider.py +++ b/src/agent_env/providers/env_providers/env_gateway_provider.py @@ -836,9 +836,9 @@ async def _deploy_via_containers( config = get_config() gateway_env = Env.get(config.default_gateway_env_id) - if problems := [problem for image in [gateway_env.docker_image_artifact, *mcp_server_images] - if (problem := image.by_name_problem())]: - raise ValueError(f"Can't deploy on Modal, whose gateway runs each image by name: {'; '.join(problems)}") + # Images that are only a build context are built before anything is created, so a failed build leaves nothing. + images = [gateway_env.docker_image_artifact, *mcp_server_images] + await asyncio.gather(*(sandbox_provider.prepare_image(image) for image in images if image.context_only)) i6pn_kwargs = {"i6pn": True, "region": config.modal_default_region} if isinstance(sandbox_provider, ModalSandboxProvider) else {} deploy = _ContainerDeploy( diff --git a/src/agent_env/providers/env_providers/env_server_provider.py b/src/agent_env/providers/env_providers/env_server_provider.py index 2742e323..a3cba294 100644 --- a/src/agent_env/providers/env_providers/env_server_provider.py +++ b/src/agent_env/providers/env_providers/env_server_provider.py @@ -14,6 +14,7 @@ from agent_env.env.gateway import AGENT_ENV_GATEWAY_MCP_PORT from agent_env.env.gateway.constants import WELL_KNOWN_PATH from agent_env.providers.env_providers.env_provider import _SandboxEnvironmentProvider, _size_kwargs, _tool_names +from agent_env.providers.sandbox_providers.modal_sandbox import ModalSandboxProvider from agent_env.providers.sandbox_providers.sandbox_provider import SandboxProvider if TYPE_CHECKING: @@ -67,8 +68,12 @@ async def _deploy_server( from agent_env.env.envs.mcp_server import MCPServerEnv name = env.environment_name - if problem := env.docker_image_artifact.by_name_problem(): - raise ValueError(f"Can't deploy {name!r} as a server, which runs its image by name: {problem}") + image = env.docker_image_artifact + if isinstance(sandbox_provider, ModalSandboxProvider): # which builds an image that's only a build context + await sandbox_provider.prepare_image(image) + elif problem := image.by_name_problem(): + raise ValueError(f"Can't deploy {name!r} as a server on {type(sandbox_provider).__name__}, which runs its " + f"image by name: {problem}") server = await sandbox_provider.create_container( image_name=env.docker_image_artifact.image_name, port=AGENT_ENV_GATEWAY_MCP_PORT, diff --git a/tst/unit/a2a_agent/context_only_deploy_test.py b/tst/unit/a2a_agent/context_only_deploy_test.py index c8558c0f..b9771cc3 100644 --- a/tst/unit/a2a_agent/context_only_deploy_test.py +++ b/tst/unit/a2a_agent/context_only_deploy_test.py @@ -1,5 +1,5 @@ -"""An agent whose image is only a build context deploys only where a VM builds it: a provider that runs an agent in a -container by its image's name refuses it before creating anything.""" +"""An agent whose image is only a build context deploys only where it's built, in a VM or by Modal: a provider that runs an +agent in a container by its image's name refuses it before creating anything, and Modal is told of it first.""" import pytest @@ -8,6 +8,7 @@ from agent_env.providers.sandbox_providers import sandbox_provider from agent_env.providers.sandbox_providers.chained_sandbox_provider import ChainedSandboxProvider from agent_env.providers.sandbox_providers.local_sandbox import LocalSandboxProvider +from agent_env.providers.sandbox_providers.modal_sandbox import ModalSandboxProvider from agent_env.providers.sandbox_providers.sandbox_provider import SandboxProvider @@ -25,6 +26,21 @@ async def create_sandbox(self, **_): raise _Created +class _ModalProvider(ModalSandboxProvider): + """Modal with its build and its container faked, recording the order they're asked for.""" + + def __init__(self): + super().__init__() + self.steps = [] + + async def prepare_image(self, image): + self.steps.append(("prepare", image.image_name)) + + async def create_container(self, **kwargs): + self.steps.append(("create", kwargs["image_name"])) + raise _Created + + IMAGE = DockerImageArtifact(id="solver-image", version=2, description="d", image_name="local/solver-0123456789ab:v2", build_context_object_url="s3://bucket/ctx.tar.gz") @@ -42,7 +58,8 @@ async def test_a_provider_that_runs_the_agent_by_name_refuses_it(monkeypatch, lo monkeypatch.setattr(sandbox_provider, "build_sandbox_provider", lambda name: provider) with pytest.raises(ValueError, match="Can't deploy agent 'solver' with _LocalProvider, which runs an agent's image by " - "name: 'solver-image' v2 is only a build context, which only a VM sandbox builds"): + "name: 'solver-image' v2 is only a build context, which has to be built, not " + "pulled"): await A2AAgent(id="solver", version=1, docker_image_artifact=IMAGE).deploy(sandbox_type="any") @@ -54,6 +71,38 @@ async def test_a_vm_provider_is_asked_for_a_sandbox(monkeypatch, local_stores, l await A2AAgent(id="solver", version=1, docker_image_artifact=IMAGE).deploy(sandbox_type="any") +@pytest.mark.asyncio +async def test_modal_is_told_of_the_image_before_it_is_asked_for_a_sandbox(monkeypatch, local_stores, litellm): + modal = _ModalProvider() + monkeypatch.setattr(sandbox_provider, "build_sandbox_provider", lambda name: modal) + + with pytest.raises(_Created): + await A2AAgent(id="solver", version=1, docker_image_artifact=IMAGE).deploy(sandbox_type="any") + assert modal.steps == [("prepare", IMAGE.image_name), ("create", IMAGE.image_name)] + + +@pytest.mark.asyncio +async def test_a_chain_tells_modal_of_the_image_once_it_tries_modal(monkeypatch, local_stores, litellm): + modal = _ModalProvider() + monkeypatch.setattr(sandbox_provider, "build_sandbox_provider", + lambda name: ChainedSandboxProvider([_VmProvider(), modal])) + + with pytest.raises(RuntimeError, match="All 2 providers failed"): + await A2AAgent(id="solver", version=1, docker_image_artifact=IMAGE).deploy(sandbox_type="any") + assert modal.steps == [("prepare", IMAGE.image_name), ("create", IMAGE.image_name)] + + +@pytest.mark.asyncio +async def test_an_image_that_is_not_only_a_build_context_is_not_prepared(monkeypatch, local_stores, litellm): + modal = _ModalProvider() + monkeypatch.setattr(sandbox_provider, "build_sandbox_provider", lambda name: modal) + pulled = DockerImageArtifact(id="solver-image", version=2, description="d", image_name="registry.example/solver:v2") + + with pytest.raises(_Created): + await A2AAgent(id="solver", version=1, docker_image_artifact=pulled).deploy(sandbox_type="any") + assert modal.steps == [("create", "registry.example/solver:v2")] + + class _ContainerSandbox: sandbox_id = "sb-container" mode = "container" diff --git a/tst/unit/artifact/test_docker_image_build_context.py b/tst/unit/artifact/test_docker_image_build_context.py index 4a2787b2..a39ae469 100644 --- a/tst/unit/artifact/test_docker_image_build_context.py +++ b/tst/unit/artifact/test_docker_image_build_context.py @@ -66,11 +66,11 @@ def test_equal_sources_give_equal_digests_and_bytes(local_stores, context): assert three.version == 2 -def test_a_context_only_image_is_built_by_a_vm_and_refused_where_run_by_name(local_stores, context): +def test_a_context_only_image_is_built_and_refused_where_run_by_name(local_stores, context): image = DockerImageArtifact.put_context("a", description="d", context_path=str(context), dockerfile_path="Dockerfile") assert image.context_only and image.load_problem() is None - assert image.by_name_problem() == "'a' v1 is only a build context, which only a VM sandbox builds" + assert image.by_name_problem() == "'a' v1 is only a build context, which has to be built, not pulled" with pytest.raises(ValueError, match="has no tar.gz; its image is built from its build context"): image.load() diff --git a/tst/unit/bundle/materialize_test.py b/tst/unit/bundle/materialize_test.py index 3f62b41f..1064a72a 100644 --- a/tst/unit/bundle/materialize_test.py +++ b/tst/unit/bundle/materialize_test.py @@ -431,11 +431,11 @@ def test_an_image_switched_between_built_here_and_a_build_context_is_rewritten_w as_context = _summary(materialize(plan_of(bundle_dir), contexts=frozenset({image}))) built_again = _summary(_run(bundle_dir)) - assert as_context[image] == (2, False, ("now written as a build context, which its VMs build, not an image built " + assert as_context[image] == (2, False, ("now written as a build context, built where it runs, not an image built " "on this machine",)) assert as_context[f"{ROOT}/solver"] == (2, False, (f"artifact {image} is written anew (v1 → v2)",)) assert built_again[image] == (3, False, ("now written as an image built on this machine, not a build context, " - "which its VMs build",)) + "built where it runs",)) assert len(builds) == 2 diff --git a/tst/unit/bundle/preflight_test.py b/tst/unit/bundle/preflight_test.py index c7e7f6c2..c26e8932 100644 --- a/tst/unit/bundle/preflight_test.py +++ b/tst/unit/bundle/preflight_test.py @@ -138,34 +138,24 @@ def test_a_remote_image_runs_anywhere_and_a_local_one_on_the_local_provider(bund BUILT = "@local/~/triage/solver__agent_image" +CONTEXT = "a build context, which each deploy builds where it runs, for linux/amd64" + + @pytest.mark.parametrize("sandbox, form", [ - ("local", "built on this machine"), - ("modal_vm", "a build context, which each VM deploying it builds for linux/amd64"), + ("local", "built on this machine"), ("modal_vm", CONTEXT), ("modal", CONTEXT), ("modal_vm,modal", CONTEXT), ]) -def test_an_agent_the_bundle_builds_is_built_here_for_the_local_provider_and_a_build_context_for_vms( +def test_an_agent_the_bundle_builds_is_built_here_for_the_local_provider_and_a_build_context_for_those_that_build_it( bundle_dir, docker_on_path, sandbox, form): + """A VM builds it, and Modal builds it before creating the container that runs it.""" layout(bundle_dir, {"agents/solver/Dockerfile": "FROM scratch\n"}) _task(bundle_dir, [AGENT]) dry = dry_run_bundle(bundle_dir, sandbox=sandbox) - assert dry.materialization.contexts == ({BUILT} if sandbox == "modal_vm" else set()) + assert dry.materialization.contexts == (set() if sandbox == "local" else {BUILT}) assert dry.images() == (f"agents/solver (Dockerfile image) v1: {form}",) -@pytest.mark.parametrize("sandbox", ["modal", "modal_vm,modal"]) -def test_an_agent_the_bundle_builds_is_refused_on_a_provider_that_runs_it_by_name(bundle_dir, sandbox): - layout(bundle_dir, {"agents/solver/Dockerfile": "FROM scratch\n"}) - _task(bundle_dir, [AGENT]) - - assert _problems(lambda: dry_run_bundle(bundle_dir, sandbox=sandbox)) == [ - "tasks/t.json: step 'agent': deploys agent '@local/~/triage/solver''s image on the 'modal' sandbox provider, " - "which runs it by name, and the bundle builds it from agents/solver/Dockerfile, which a sandbox runs only once " - "built on this machine or in its VM; run it with --sandbox local, or on a provider whose VMs build it, such as " - "--sandbox modal_vm", - ] - - class _ContainersOnly(SandboxProvider): """A plugin's provider that runs an agent's image in a container of its own, and creates no VM.""" @@ -173,8 +163,7 @@ async def create_sandbox(self, **kwargs): raise AssertionError("preflight creates nothing") -def test_an_agent_the_bundle_builds_is_refused_on_a_provider_that_creates_no_vm_to_build_it_in(bundle_dir, - monkeypatch): +def test_an_agent_the_bundle_builds_is_refused_on_a_provider_that_cant_build_it(bundle_dir, monkeypatch): registry = Config.sandbox_registry monkeypatch.setattr(Config, "sandbox_registry", lambda self: {**registry(self), "containers": {"impl": _ContainersOnly}}) layout(bundle_dir, {"agents/solver/Dockerfile": "FROM scratch\n"}) @@ -182,14 +171,14 @@ def test_an_agent_the_bundle_builds_is_refused_on_a_provider_that_creates_no_vm_ assert _problems(lambda: dry_run_bundle(bundle_dir, sandbox="containers")) == [ "tasks/t.json: step 'agent': deploys agent '@local/~/triage/solver''s image on the 'containers' sandbox " - "provider, which creates no VM to build it in, and the bundle builds it from agents/solver/Dockerfile, which a " - "sandbox runs only once built on this machine or in its VM; run it with --sandbox local, or on a provider whose " - "VMs build it, such as --sandbox modal_vm", + "provider, which can't build it from its build context, and the bundle builds it from agents/solver/Dockerfile, " + "which a sandbox runs only once it's built on this machine or by the provider it runs on; run it with --sandbox " + "local, or on a provider that builds it, such as --sandbox modal", ] MIXED = ("the bundle builds it from agents/solver/Dockerfile on this machine for the local provider, and as a build " - "context for one whose VMs build it, and a run writes it one way; run its deploys on one kind, such as " + "context for one that builds it, and a run writes it one way; run its deploys on one kind, such as " "--sandbox local or --sandbox modal_vm") @@ -269,7 +258,7 @@ def test_an_env_image_with_no_tarball_and_no_registry_is_refused_where_a_gateway assert [entry.name for entry in dry_run_bundle(bundle_dir, sandbox="modal").runs] == ["t"] -# Images with only a build context, which a VM sandbox builds and nothing else can run +# Images with only a build context, which a VM sandbox or Modal builds and nothing else can run def _context_image(id): @@ -279,12 +268,13 @@ def _context_image(id): dockerfile_path="Dockerfile", platform="linux/amd64", source_digest="sha256:" + "1" * 64)) -BY_NAME = ("on the {sandbox!r} sandbox provider, which runs it by name: '{id}' v1 is only a build context, which only a " - "VM sandbox builds; run it on a provider whose VMs build it, such as --sandbox modal_vm") +BY_NAME = ("on the {sandbox!r} sandbox provider, which runs it by name: '{id}' v1 is only a build context, which has to " + "be built, not pulled; run it on a provider that builds it, such as --sandbox modal") -@pytest.mark.parametrize("sandbox, refused", [("modal_vm", False), ("local", True), ("modal", True), ("modal_vm,local", True)]) -def test_an_agent_image_that_is_only_a_build_context_runs_where_a_vm_builds_it(bundle_dir, sandbox, refused): +@pytest.mark.parametrize("sandbox, refused", [("modal_vm", False), ("modal", False), ("modal_vm,modal", False), + ("local", True), ("modal_vm,local", True)]) +def test_an_agent_image_that_is_only_a_build_context_runs_where_a_vm_or_modal_builds_it(bundle_dir, sandbox, refused): with namespace_routing(): A2AAgent.put(id="solver", docker_image_artifact=_context_image("solver-image")) _task(bundle_dir, [AGENT]) @@ -298,19 +288,18 @@ def test_an_agent_image_that_is_only_a_build_context_runs_where_a_vm_builds_it(b assert [entry.name for entry in dry_run_bundle(bundle_dir, sandbox=sandbox).runs] == ["t"] -def test_an_env_image_that_is_only_a_build_context_runs_on_a_gateway_vm_and_not_on_modals_containers(bundle_dir): +def test_an_env_image_that_is_only_a_build_context_runs_on_a_gateway_vm_and_on_modals_containers(bundle_dir): with namespace_routing(): MCPServerEnv.put(id="crm", docker_image_artifact=_context_image("crm-image"), environment_name="crm") _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) assert [entry.name for entry in dry_run_bundle(bundle_dir, sandbox="local").runs] == ["t"] _infra(REMOTE) - assert _problems(lambda: dry_run_bundle(bundle_dir, sandbox="modal")) == [ - "tasks/t.json: step 'env': deploys env 'crm''s image 'crm-image' " + BY_NAME.format(sandbox="modal", id="crm-image")] + assert [entry.name for entry in dry_run_bundle(bundle_dir, sandbox="modal").runs] == ["t"] @pytest.mark.parametrize("sandbox", ["local", "modal_vm"]) -def test_a_lone_server_whose_image_is_only_a_build_context_is_refused_on_any_provider(bundle_dir, sandbox): +def test_a_lone_server_whose_image_is_only_a_build_context_is_refused_on_any_provider_but_modal(bundle_dir, sandbox): with namespace_routing(): MCPServerEnv.put(id="crm", docker_image_artifact=_context_image("crm-image"), environment_name="crm", env_provider_type="server") @@ -318,6 +307,26 @@ def test_a_lone_server_whose_image_is_only_a_build_context_is_refused_on_any_pro assert _problems(lambda: dry_run_bundle(bundle_dir, sandbox=sandbox)) == [ "tasks/t.json: step 'env': deploys env 'crm''s image 'crm-image' " + BY_NAME.format(sandbox=sandbox, id="crm-image")] + assert [entry.name for entry in dry_run_bundle(bundle_dir, sandbox="modal").runs] == ["t"] + + +def test_modal_builds_a_gateway_image_that_is_only_a_build_context_but_not_a_service_db_one(bundle_dir): + with namespace_routing(): + MCPServerEnv.put(id="crm", docker_image_artifact=_image("crm-image", REMOTE), environment_name="crm") + _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) + _infra(REMOTE) + with namespace_routing(): + GatewayEnv.put(id="default", docker_image_artifact=_context_image("gateway-default")) + assert [entry.name for entry in dry_run_bundle(bundle_dir, sandbox="modal").runs] == ["t"] + + with namespace_routing(): + ServiceDBEnv.put(id="default-db", db_docker_image_artifact=_context_image("service-db-default-db"), + db_web_docker_image_artifact=_image("db-web-default-db", REMOTE), + db_mcp_docker_image_artifact=_image("db-mcp-default-db", REMOTE)) + assert _problems(lambda: dry_run_bundle(bundle_dir, sandbox="modal")) == [ + "tasks/t.json: step 'env': deploys the service-db env 'default-db''s image 'service-db-default-db' on the 'modal' " + "sandbox provider, which runs it by name: 'service-db-default-db' v2 is only a build context, which has to be " + "built, not pulled; run it on a provider whose VMs build it, such as --sandbox modal_vm"] # What a provider can create @@ -589,28 +598,28 @@ def _env_folder(root, name, toml="", kind="mcp_server"): f"envs/{name}/env.toml": f'type = "{kind}"\n{toml}'}) -def test_an_image_the_bundle_builds_for_an_env_is_a_build_context_on_a_gateway_vm(bundle_dir): - _env_folder(bundle_dir, "crm") +@pytest.mark.parametrize("sandbox, toml", [("modal_vm", ""), ("modal", ""), ("modal", 'env_provider_type = "server"\n')]) +def test_an_image_the_bundle_builds_for_an_env_is_a_build_context_where_its_server_is_built(bundle_dir, sandbox, toml): + """A gateway VM builds it, and Modal builds it for its gateway's container or a lone server's.""" + _env_folder(bundle_dir, "crm", toml) _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) _infra(REMOTE) - dry = dry_run_bundle(bundle_dir, sandbox="modal_vm") + dry = dry_run_bundle(bundle_dir, sandbox=sandbox) assert dry.materialization.contexts == {f"{BUNDLE}/crm__env_image"} -@pytest.mark.parametrize("sandbox, toml", [("modal", ""), ("modal_vm", 'env_provider_type = "server"\n')]) -def test_an_image_the_bundle_builds_for_an_env_is_refused_where_its_server_runs_by_name(bundle_dir, sandbox, toml): - """Modal's gateway runs each server in a container from its image's name, and a lone server does on any provider.""" - _env_folder(bundle_dir, "crm", toml) +def test_an_image_the_bundle_builds_for_a_lone_server_is_refused_where_the_server_runs_by_name(bundle_dir): + """A lone server runs in a container from its image's name everywhere but on Modal.""" + _env_folder(bundle_dir, "crm", 'env_provider_type = "server"\n') _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) - _infra(REMOTE) - assert _problems(lambda: dry_run_bundle(bundle_dir, sandbox=sandbox)) == [ - f"tasks/t.json: step 'env': deploys env '{BUNDLE}/crm''s image '{BUNDLE}/crm__env_image' on the {sandbox!r} " + assert _problems(lambda: dry_run_bundle(bundle_dir, sandbox="modal_vm")) == [ + f"tasks/t.json: step 'env': deploys env '{BUNDLE}/crm''s image '{BUNDLE}/crm__env_image' on the 'modal_vm' " "sandbox provider, which runs it by name, and the bundle builds it from envs/crm/Dockerfile, which a sandbox " - "runs only once built on this machine or in its VM; run it with --sandbox local, or on a provider whose VMs " - "build it, such as --sandbox modal_vm", + "runs only once it's built on this machine or by the provider it runs on; run it with --sandbox local, or on a " + "provider that builds it, such as --sandbox modal", ] 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 55804337..658e9a9c 100644 --- a/tst/unit/providers/env_providers/env_gateway_provider_test.py +++ b/tst/unit/providers/env_providers/env_gateway_provider_test.py @@ -125,19 +125,20 @@ async def test_container_mode_rejects_websites(): @pytest.mark.parametrize("gateway_image, server_image", [(_CONTEXT_ONLY, _image("mcp-slack")), (_image("agent-gateway"), _CONTEXT_ONLY)], ids=["gateway", "server"]) -async def test_container_mode_refuses_a_context_only_image_before_creating_anything(gateway_image, server_image): +async def test_container_mode_builds_a_context_only_image_before_creating_anything(gateway_image, server_image): provider = ModalSandboxProvider() gp = EnvironmentGatewayProvider() gp._build_local_store = AsyncMock() - with patch.object(provider, "create_container", side_effect=AssertionError("created a container")), \ + with patch.object(provider, "prepare_image", side_effect=RuntimeError("the build failed")) as prepare, \ + patch.object(provider, "create_container", side_effect=AssertionError("created a container")), \ patch("agent_env.env.env.Env.get", return_value=MagicMock(docker_image_artifact=gateway_image)): - with pytest.raises(ValueError, match="Can't deploy on Modal, whose gateway runs each image by name: 'slack-image' v2 " - "is only a build context"): + with pytest.raises(RuntimeError, match="the build failed"): await gp._deploy_via_containers( sandbox_provider=provider, mcp_servers=[MCPServerConfig(image=server_image.image_name, environment_name="slack")], mcp_server_images=[server_image], gateway_port=18765, website_configs=None, gateway_mode=MagicMock(value="performance"), ttl_seconds=60, disk_size_gb=10, ) + prepare.assert_awaited_once_with(_CONTEXT_ONLY) gp._build_local_store.assert_not_awaited() diff --git a/tst/unit/providers/env_providers/env_server_provider_test.py b/tst/unit/providers/env_providers/env_server_provider_test.py index c16c21d0..ca6f6142 100644 --- a/tst/unit/providers/env_providers/env_server_provider_test.py +++ b/tst/unit/providers/env_providers/env_server_provider_test.py @@ -20,6 +20,7 @@ from agent_env.providers.env_providers.env_provider import build_env_provider, record_class_for from agent_env.providers.env_providers.env_gateway_provider import EnvironmentGatewayProvider from agent_env.providers.env_providers.env_server_provider import EnvironmentServerProvider +from agent_env.providers.sandbox_providers.modal_sandbox import ModalSandboxProvider from agent_env.providers.sandbox_providers.sandbox_provider import SandboxProvider _SERVER_CARD = {"name": "slack", "additionalInterfaces": [{"url": "/custom-mcp", "transport": "mcp"}]} @@ -61,17 +62,42 @@ async def test_a_deploy_starts_only_the_server_on_sqlite_with_its_sizing(): assert call["env"] == {"ENVIRONMENT_NAME": "slack", "MCP_HOST": "0.0.0.0", "DATABASE_URL": "sqlite:////tmp/slack.sqlite"} +_CONTEXT_ONLY = DockerImageArtifact(id="slack-image", version=3, description="d", image_name="local/slack-0123456789ab:v3", + build_context_object_url="s3://bucket/ctx.tar.gz") + + @pytest.mark.asyncio async def test_a_server_whose_image_is_only_a_build_context_is_refused_before_any_container(): - image = DockerImageArtifact(id="slack-image", version=3, description="d", image_name="local/slack-0123456789ab:v3", - build_context_object_url="s3://bucket/ctx.tar.gz") - run = await _run_server_deploy(env=SimpleNamespace(**{**vars(_ENV), "docker_image_artifact": image})) + run = await _run_server_deploy(env=SimpleNamespace(**{**vars(_ENV), "docker_image_artifact": _CONTEXT_ONLY})) - assert "Can't deploy 'slack' as a server, which runs its image by name: 'slack-image' v3 is only a build context" in str( - run.result) + assert ("Can't deploy 'slack' as a server on _FakeSandboxProvider, which runs its image by name: 'slack-image' v3 is " + "only a build context") in str(run.result) assert run.calls == [] +class _ModalProvider(ModalSandboxProvider): + """Modal with its build and its container faked, recording the order they're asked for.""" + + def __init__(self): + super().__init__() + self.steps = [] + + async def prepare_image(self, image): + self.steps.append(("prepare", image.image_name)) + + async def create_container(self, **kwargs): + self.steps.append(("create", kwargs["image_name"])) + raise RuntimeError("created") + + +@pytest.mark.asyncio +async def test_modal_builds_a_server_whose_image_is_only_a_build_context_before_its_container(): + modal = _ModalProvider() + await _run_server_deploy(modal, env=SimpleNamespace(**{**vars(_ENV), "docker_image_artifact": _CONTEXT_ONLY})) + + assert modal.steps == [("prepare", "local/slack-0123456789ab:v3"), ("create", "local/slack-0123456789ab:v3")] + + @pytest.mark.asyncio @pytest.mark.parametrize("card, tools, error", [ (None, ["slack_list"], "did not serve its env card at https://mcp-slack.fake.host/.well-known/agent-env.json in time; an env deployed without a gateway must serve its own card"), From a745a8fb4d7690c4508f69ac599b96573aa37796 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Thu, 8 Oct 2026 23:55:53 -0700 Subject: [PATCH 06/10] build: require Python 3.11.4, whose tarfile has the extraction filters a build context is unpacked with Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- CONTRIBUTING.md | 2 +- README.md | 2 +- pyproject.toml | 2 +- uv.lock | 8 +++----- 5 files changed, 7 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0430de79..a74b2b6a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -14,7 +14,7 @@ environments; agent-env depends on it. ## Setup -Python 3.11 or newer. The repo convention is a `.venv` at the root, which the Makefile and CI use. +Python 3.11.4 or newer. The repo convention is a `.venv` at the root, which the Makefile and CI use. ```bash uv sync --extra dev && . .venv/bin/activate diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0e0a5460..f54e2c3d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,7 +16,7 @@ you agree that it is licensed under the same terms. ## Development setup -You need Python 3.11 or newer, [uv](https://docs.astral.sh/uv/) and, for the integration tiers, +You need Python 3.11.4 or newer, [uv](https://docs.astral.sh/uv/) and, for the integration tiers, Docker. ```bash diff --git a/README.md b/README.md index 64adcc2d..67aa102e 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ AgentEnv Framework is a Python SDK and CLI for building, deploying and running a ## Install -Both packages are on PyPI. You need Python 3.11 or newer and, to run environments locally, a running Docker daemon. With [uv](https://docs.astral.sh/uv/), install the `agent-env` command as a tool: +Both packages are on PyPI. You need Python 3.11.4 or newer and, to run environments locally, a running Docker daemon. With [uv](https://docs.astral.sh/uv/), install the `agent-env` command as a tool: ```bash uv tool install agentenv-framework diff --git a/pyproject.toml b/pyproject.toml index b2159e35..df852d65 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ readme = "README.md" license = "Apache-2.0" license-files = ["LICENSE", "NOTICE", "THIRD_PARTY_NOTICES.md"] authors = [{ name = "Scale AI" }] -requires-python = ">=3.11" +requires-python = ">=3.11.4" keywords = ["reinforcement-learning", "rl-environments", "ai-agents", "llm-agents", "agent-evaluation", "mcp", "model-context-protocol", "a2a", "sandbox"] classifiers = [ "Development Status :: 4 - Beta", diff --git a/uv.lock b/uv.lock index 645a37e6..af9e9320 100644 --- a/uv.lock +++ b/uv.lock @@ -1,15 +1,13 @@ version = 1 revision = 3 -requires-python = ">=3.11" +requires-python = ">=3.11.4" resolution-markers = [ "python_full_version >= '3.14' and sys_platform == 'win32'", "python_full_version >= '3.14' and sys_platform != 'win32'", "python_full_version == '3.13.*' and sys_platform == 'win32'", + "python_full_version < '3.13' and sys_platform == 'win32'", "python_full_version == '3.13.*' and sys_platform != 'win32'", - "python_full_version == '3.12.*' and sys_platform == 'win32'", - "python_full_version == '3.12.*' and sys_platform != 'win32'", - "python_full_version < '3.12' and sys_platform == 'win32'", - "python_full_version < '3.12' and sys_platform != 'win32'", + "python_full_version < '3.13' and sys_platform != 'win32'", ] [manifest] From 6fb059b6189d39f5b5bb48cac74aa180f85c5d48 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Fri, 9 Oct 2026 08:30:08 -0700 Subject: [PATCH 07/10] fix(modal): build again an image Modal no longer holds, once, rather than failing every deploy of it Co-Authored-By: Claude Opus 5.5 (1M context) --- .../sandbox_providers/modal_sandbox.py | 38 +++++++++++++------ .../modal_context_build_test.py | 35 +++++++++++++++++ 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index fd5ea2bb..9af1ef66 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -253,15 +253,18 @@ async def prepare_image(self, image: DockerImageArtifact) -> None: self._context_images[image.image_name] = image await self._context_image(image) - async def _context_image(self, image: DockerImageArtifact) -> modal.Image: - """The Modal image built from ``image``'s build context: built now unless one built from the same sources is.""" + async def _context_image(self, image: DockerImageArtifact, *, + stale: str | None = None) -> tuple[modal.Image, str]: + """The Modal image built from ``image``'s build context, and its id: built now unless one built from the same + sources is, other than ``stale``, an id Modal no longer holds.""" client = await self._get_client() app = await self._get_app(self._app_name, _attribution_tags({})) key = (self._token_id, self._app_name, image.source_digest or image.build_context_object_url) async with _build_locks.setdefault((asyncio.get_running_loop(), key), asyncio.Lock()): - if (image_id := _built_image_ids.get(key)) is None: + image_id = _built_image_ids.get(key) + if image_id is None or image_id == stale: image_id = _built_image_ids[key] = await self._build(image, app) - return modal.Image.from_id(image_id, client=client) + return modal.Image.from_id(image_id, client=client), image_id async def _build(self, image: DockerImageArtifact, app: modal.App) -> str: """Build ``image`` from its build context, with the context's own ignore file; returns the image's id.""" @@ -312,8 +315,9 @@ async def create_container( app_name = self._app_name app = await self._get_app(app_name, app_tags) + built_id = None if (context_only := self._context_images.get(image_name)) is not None: - image = await self._context_image(context_only) + image, built_id = await self._context_image(context_only) else: image = await self._registry_image(image_name) @@ -345,7 +349,7 @@ async def create_container( # ``i6pn`` is threaded per-factory below (V2 only); everything else is shared. create_kwargs = dict( - app=app, image=image, cpu=cpu, memory=memory, timeout=timeout, + app=app, cpu=cpu, memory=memory, timeout=timeout, env=env or {}, client=client, **port_kwargs, tags=sandbox_tags or None, readiness_probe=modal.Probe.with_exec("true"), @@ -354,18 +358,28 @@ async def create_container( if region is not None: create_kwargs["region"] = region - try: + async def create(image: modal.Image) -> modal.Sandbox: if self._gpu: - sb = await modal.Sandbox.create.aio( + return await modal.Sandbox.create.aio( *(command or []), gpu=self._gpu, experimental_options={"enable_docker_in_gvisor": True}, + image=image, **create_kwargs, ) - else: - sb = await modal.Sandbox._experimental_create.aio( - *(command or []), i6pn=i6pn, **create_kwargs - ) + return await modal.Sandbox._experimental_create.aio( + *(command or []), i6pn=i6pn, image=image, **create_kwargs + ) + + try: + try: + sb = 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(context_only, stale=built_id) + sb = await create(image) except Exception as e: raise RuntimeError( f"Modal sandbox create failed [{_fmt_exc(e)}]: {call_context}" diff --git a/tst/unit/providers/sandbox_providers/modal_context_build_test.py b/tst/unit/providers/sandbox_providers/modal_context_build_test.py index 9b30920a..88da350e 100644 --- a/tst/unit/providers/sandbox_providers/modal_context_build_test.py +++ b/tst/unit/providers/sandbox_providers/modal_context_build_test.py @@ -150,6 +150,41 @@ async def test_a_container_runs_the_built_image_and_builds_it_on_a_miss(local_st assert len(builds.calls) == 2 +@pytest.mark.asyncio +async def test_a_built_image_modal_no_longer_holds_is_built_again_once_for_every_deploy_waiting_on_it(local_stores, + context, builds): + image = _context_image("solver-image", context) + provider = _provider() + await provider.prepare_image(image) + created = [] + + async def create(*args, image, **kwargs): + created.append(image) + if image == ("from_id", "im-1"): + raise modal.exception.NotFoundError("No Image with ID 'im-1' found") + raise RuntimeError("stop") + + with _fake_create() as fake: + fake.aio.side_effect = create + for result in await asyncio.gather(*(provider.create_container(image_name=image.image_name, port=8000, env={}) + for _ in range(2)), return_exceptions=True): + assert isinstance(result, RuntimeError) and "stop" in str(result) + + assert len(builds.calls) == 2 + assert created.count(("from_id", "im-2")) == 2 + + +@pytest.mark.asyncio +async def test_a_missing_image_it_was_not_built_from_a_context_is_not_rebuilt(local_stores, builds, monkeypatch): + monkeypatch.setattr(modal.Image, "from_registry", lambda name, **kwargs: ("pulled", name)) + + with _fake_create() as fake, pytest.raises(RuntimeError, match="No Image"): + fake.aio.side_effect = modal.exception.NotFoundError("No Image with ID 'im-9' found") + await _provider().create_container(image_name="registry.example/solver:v2", port=8000, env={}) + + assert builds.calls == [] and fake.aio.call_count == 1 + + @pytest.mark.asyncio async def test_a_container_of_an_image_it_wasnt_told_of_is_pulled(local_stores, builds, monkeypatch): pulled = MagicMock() From 20afa97e3889e5a25334b4d68e1bb48d0888b984 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Fri, 9 Oct 2026 11:11:39 -0700 Subject: [PATCH 08/10] feat(sandbox): prepare_image takes the deploy's attribution, as creating its sandbox does Every caller and a chain pass it, so a provider can attribute the build to the deploy it's for. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/a2a_agent/a2a_agent.py | 2 +- .../env_providers/env_gateway_provider.py | 3 ++- .../env_providers/env_server_provider.py | 2 +- .../chained_sandbox_provider.py | 5 +++-- .../sandbox_providers/modal_sandbox.py | 2 +- .../sandbox_providers/sandbox_provider.py | 6 +++--- tst/unit/a2a_agent/context_only_deploy_test.py | 11 ++++++++--- .../env_providers/env_gateway_provider_test.py | 3 ++- .../env_providers/env_server_provider_test.py | 9 ++++++--- .../sandbox_providers/sandbox_provider_test.py | 18 ++++++++++++++++-- 10 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/agent_env/a2a_agent/a2a_agent.py b/src/agent_env/a2a_agent/a2a_agent.py index dde14ffd..25f4b1b7 100644 --- a/src/agent_env/a2a_agent/a2a_agent.py +++ b/src/agent_env/a2a_agent/a2a_agent.py @@ -445,7 +445,7 @@ async def deploy( if provider is not None: logger.info(f"Provisioning sandbox for A2A agent '{self.id}' via {type(provider).__name__}...") if self.docker_image_artifact.context_only: - await provider.prepare_image(self.docker_image_artifact) + await provider.prepare_image(self.docker_image_artifact, attribution=attribution) self._sandbox = await provider.create_sandbox( image_name=image_name, port=a2a_port, env=merged_env, cpu=cpu, memory=memory, disk_size_gb=disk_size_gb, timeout=ttl_seconds, 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 c61542f0..f2a79e2a 100644 --- a/src/agent_env/providers/env_providers/env_gateway_provider.py +++ b/src/agent_env/providers/env_providers/env_gateway_provider.py @@ -838,7 +838,8 @@ async def _deploy_via_containers( gateway_env = Env.get(config.default_gateway_env_id) # Images that are only a build context are built before anything is created, so a failed build leaves nothing. images = [gateway_env.docker_image_artifact, *mcp_server_images] - await asyncio.gather(*(sandbox_provider.prepare_image(image) for image in images if image.context_only)) + await asyncio.gather(*(sandbox_provider.prepare_image(image, attribution=attribution) + for image in images if image.context_only)) i6pn_kwargs = {"i6pn": True, "region": config.modal_default_region} if isinstance(sandbox_provider, ModalSandboxProvider) else {} deploy = _ContainerDeploy( diff --git a/src/agent_env/providers/env_providers/env_server_provider.py b/src/agent_env/providers/env_providers/env_server_provider.py index a3cba294..587ccb96 100644 --- a/src/agent_env/providers/env_providers/env_server_provider.py +++ b/src/agent_env/providers/env_providers/env_server_provider.py @@ -70,7 +70,7 @@ async def _deploy_server( name = env.environment_name image = env.docker_image_artifact if isinstance(sandbox_provider, ModalSandboxProvider): # which builds an image that's only a build context - await sandbox_provider.prepare_image(image) + await sandbox_provider.prepare_image(image, attribution=attribution) elif problem := image.by_name_problem(): raise ValueError(f"Can't deploy {name!r} as a server on {type(sandbox_provider).__name__}, which runs its " f"image by name: {problem}") diff --git a/src/agent_env/providers/sandbox_providers/chained_sandbox_provider.py b/src/agent_env/providers/sandbox_providers/chained_sandbox_provider.py index 201d204e..09b9931d 100644 --- a/src/agent_env/providers/sandbox_providers/chained_sandbox_provider.py +++ b/src/agent_env/providers/sandbox_providers/chained_sandbox_provider.py @@ -52,7 +52,7 @@ def filter_sandbox_providers( def supports_network_policy(self, policy: NetworkPolicy) -> bool: return any(p.supports_network_policy(policy) for p in self._providers) - async def prepare_image(self, image: DockerImageArtifact) -> None: + async def prepare_image(self, image: DockerImageArtifact, *, attribution: Attribution | None = None) -> None: """Remember ``image``, so each provider is prepared for it just before it's tried, and only that one.""" self._images[image.image_name] = image @@ -69,7 +69,8 @@ async def create_sandbox( name = type(p).__name__ try: if image is not None: - await p.prepare_image(image) # outside the deadline, which bounds creating the sandbox, not a build + # outside the deadline, which bounds creating the sandbox, not a build + await p.prepare_image(image, attribution=attribution) async with asyncio.timeout(_PROVISION_DEADLINE_SECONDS): sandbox = await p.create_sandbox(attribution=attribution, **kwargs) except Exception as e: diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index 9af1ef66..0f6b6933 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -247,7 +247,7 @@ async def _get_app(self, app_name: str, app_tags: Optional[dict[str, str]] = Non self._apps[app_name] = app return app - async def prepare_image(self, image: DockerImageArtifact) -> None: + async def prepare_image(self, image: DockerImageArtifact, *, attribution: Optional[Attribution] = None) -> None: """Build ``image`` when it's only a build context, unless this process has already built its sources.""" if image.context_only: self._context_images[image.image_name] = image diff --git a/src/agent_env/providers/sandbox_providers/sandbox_provider.py b/src/agent_env/providers/sandbox_providers/sandbox_provider.py index 20e5a20d..d1d5355b 100644 --- a/src/agent_env/providers/sandbox_providers/sandbox_provider.py +++ b/src/agent_env/providers/sandbox_providers/sandbox_provider.py @@ -109,10 +109,10 @@ async def create_container( pass raise - async def prepare_image(self, image: DockerImageArtifact) -> None: + async def prepare_image(self, image: DockerImageArtifact, *, attribution: Optional[Attribution] = None) -> None: """Get ready to run ``image`` by its name, before create_sandbox or create_container is asked for it and outside - any deadline on creating one. Does nothing here; a provider that builds a context-only image itself builds - it.""" + any deadline on creating one, for the deploy ``attribution`` describes, as it describes the sandbox. Does + nothing here; a provider that builds a context-only image itself builds it.""" async def _start_container(self, sandbox: VmSandbox, *, image_name: str, port: int, env: dict[str, str]) -> None: """Run the pulled ``image_name`` as the sandbox's container, publishing ``port``.""" diff --git a/tst/unit/a2a_agent/context_only_deploy_test.py b/tst/unit/a2a_agent/context_only_deploy_test.py index b9771cc3..ec02e9f6 100644 --- a/tst/unit/a2a_agent/context_only_deploy_test.py +++ b/tst/unit/a2a_agent/context_only_deploy_test.py @@ -31,13 +31,15 @@ class _ModalProvider(ModalSandboxProvider): def __init__(self): super().__init__() - self.steps = [] + self.steps, self.attributions = [], [] - async def prepare_image(self, image): + async def prepare_image(self, image, *, attribution=None): self.steps.append(("prepare", image.image_name)) + self.attributions.append(attribution) async def create_container(self, **kwargs): self.steps.append(("create", kwargs["image_name"])) + self.attributions.append(kwargs["attribution"]) raise _Created @@ -77,8 +79,11 @@ async def test_modal_is_told_of_the_image_before_it_is_asked_for_a_sandbox(monke monkeypatch.setattr(sandbox_provider, "build_sandbox_provider", lambda name: modal) with pytest.raises(_Created): - await A2AAgent(id="solver", version=1, docker_image_artifact=IMAGE).deploy(sandbox_type="any") + await A2AAgent(id="solver", version=1, docker_image_artifact=IMAGE).deploy( + sandbox_type="any", attribution={"project_id": "0123456789abcdef01234567"}) assert modal.steps == [("prepare", IMAGE.image_name), ("create", IMAGE.image_name)] + prepared, created = modal.attributions + assert prepared == created and prepared["project_id"] == "0123456789abcdef01234567" @pytest.mark.asyncio 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 658e9a9c..260fa75c 100644 --- a/tst/unit/providers/env_providers/env_gateway_provider_test.py +++ b/tst/unit/providers/env_providers/env_gateway_provider_test.py @@ -137,8 +137,9 @@ async def test_container_mode_builds_a_context_only_image_before_creating_anythi sandbox_provider=provider, mcp_servers=[MCPServerConfig(image=server_image.image_name, environment_name="slack")], mcp_server_images=[server_image], gateway_port=18765, website_configs=None, gateway_mode=MagicMock(value="performance"), ttl_seconds=60, disk_size_gb=10, + attribution={"project_id": "0123456789abcdef01234567"}, ) - prepare.assert_awaited_once_with(_CONTEXT_ONLY) + prepare.assert_awaited_once_with(_CONTEXT_ONLY, attribution={"project_id": "0123456789abcdef01234567"}) gp._build_local_store.assert_not_awaited() diff --git a/tst/unit/providers/env_providers/env_server_provider_test.py b/tst/unit/providers/env_providers/env_server_provider_test.py index ca6f6142..451332ee 100644 --- a/tst/unit/providers/env_providers/env_server_provider_test.py +++ b/tst/unit/providers/env_providers/env_server_provider_test.py @@ -80,10 +80,11 @@ class _ModalProvider(ModalSandboxProvider): def __init__(self): super().__init__() - self.steps = [] + self.steps, self.attributions = [], [] - async def prepare_image(self, image): + async def prepare_image(self, image, *, attribution=None): self.steps.append(("prepare", image.image_name)) + self.attributions.append(attribution) async def create_container(self, **kwargs): self.steps.append(("create", kwargs["image_name"])) @@ -93,9 +94,11 @@ async def create_container(self, **kwargs): @pytest.mark.asyncio async def test_modal_builds_a_server_whose_image_is_only_a_build_context_before_its_container(): modal = _ModalProvider() - await _run_server_deploy(modal, env=SimpleNamespace(**{**vars(_ENV), "docker_image_artifact": _CONTEXT_ONLY})) + await _run_server_deploy(modal, env=SimpleNamespace(**{**vars(_ENV), "docker_image_artifact": _CONTEXT_ONLY}), + attribution={"project_id": "0123456789abcdef01234567"}) assert modal.steps == [("prepare", "local/slack-0123456789ab:v3"), ("create", "local/slack-0123456789ab:v3")] + assert modal.attributions == [{"project_id": "0123456789abcdef01234567"}] @pytest.mark.asyncio diff --git a/tst/unit/providers/sandbox_providers/sandbox_provider_test.py b/tst/unit/providers/sandbox_providers/sandbox_provider_test.py index 1ddfa8c3..f7f73923 100644 --- a/tst/unit/providers/sandbox_providers/sandbox_provider_test.py +++ b/tst/unit/providers/sandbox_providers/sandbox_provider_test.py @@ -292,10 +292,12 @@ class _PreparingProvider(_StubProvider): def __init__(self, *, prepare_seconds: float = 0, prepare_raises: Exception | None = None, **kwargs): super().__init__(**kwargs) - self.prepare_seconds, self.prepare_raises, self.prepared = prepare_seconds, prepare_raises, [] + self.prepare_seconds, self.prepare_raises, self.prepared, self.attributions = ( + prepare_seconds, prepare_raises, [], []) - async def prepare_image(self, image): + async def prepare_image(self, image, *, attribution=None): self.prepared.append(image.image_name) + self.attributions.append(attribution) await asyncio.sleep(self.prepare_seconds) if self.prepare_raises: raise self.prepare_raises @@ -326,6 +328,18 @@ async def test_chain_prepares_only_the_providers_it_tries_and_falls_through_a_fa assert (second.prepared, third.prepared) == ([_CONTEXT_ONLY.image_name], []) +@pytest.mark.asyncio +async def test_chain_prepares_each_provider_for_the_deploy_its_sandbox_is_for(): + provider = _PreparingProvider() + chain = ChainedSandboxProvider([provider]) + + await chain.prepare_image(_CONTEXT_ONLY) + await chain.create_sandbox(image_name=_CONTEXT_ONLY.image_name, port=8000, env={}, + attribution={"project_id": "0123456789abcdef01234567"}) + + assert provider.attributions == [{"project_id": "0123456789abcdef01234567"}] + + @pytest.mark.asyncio async def test_chain_prepares_nothing_for_an_image_it_wasnt_told_of(): provider = _PreparingProvider() From 7ee9b24fa038407bad7c5df440b9c83dee6c8b24 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Fri, 9 Oct 2026 11:11:40 -0700 Subject: [PATCH 09/10] fix(modal): write the Dockerfile forms Modal refuses as what docker build makes of them, and name the build log on failure Modal's builder fetches only a URL for ADD, and its context parser can't read a JSON-form COPY. The build reads a copy of the Dockerfile, beside the context so a COPY of the context still copies it as written, in which a JSON-form COPY or ADD whose paths hold no whitespace is in shell form, and an ADD of files and folders in the context, none of them a tar archive, is the COPY it is. A failed build names the Modal build log that has its cause. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../sandbox_providers/modal_sandbox.py | 71 ++++++++++++++++++- .../modal_context_build_test.py | 47 +++++++++++- 2 files changed, 114 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 0f6b6933..e8202d4c 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -5,11 +5,14 @@ import asyncio import base64 import contextlib +import glob import hashlib +import json import logging import os import re import shlex +import tarfile import tempfile import weakref from pathlib import Path @@ -276,13 +279,18 @@ async def _build(self, image: DockerImageArtifact, app: modal.App) -> str: context) dockerfile = context / (image.dockerfile_path or "Dockerfile") ignores = ignore_file(context, dockerfile) + # Beside the context, not in it, so a COPY of the context still copies the Dockerfile as written. + for_modal = Path(work) / "Dockerfile" + for_modal.write_text(modal_dockerfile(dockerfile.read_text("utf8"), context), "utf8") built = modal.Image.from_dockerfile( - dockerfile, context_dir=context, ignore=ignores.read_text("utf8").splitlines() if ignores else [], + for_modal, context_dir=context, ignore=ignores.read_text("utf8").splitlines() if ignores else [], ) await built.build.aio(app) except Exception as e: + log = (f"; Modal's build log for {e.image_id} has the cause" + if isinstance(e, modal.exception.ImageBuildError) and getattr(e, "image_id", None) else "") raise RuntimeError(f"Building {image.image_name} on Modal from its build context failed: " - f"{_fmt_exc(e)}") from e + f"{_fmt_exc(e)}{log}") from e logger.info(f"Built {image.image_name} on Modal as {built.object_id}") return built.object_id @@ -474,6 +482,65 @@ async def _registry_image(self, image_name: str) -> modal.Image: return modal.Image.from_registry(image_name) +_COPY_OR_ADD = re.compile(r"\s*(COPY|ADD)\s+(.*)", re.IGNORECASE | re.DOTALL) +# The flags COPY takes as ADD does; ADD's own (--checksum, --keep-git-dir, --unpack) keep an ADD as it is. +_COPY_FLAGS = ("--chown=", "--chmod=", "--link") + + +def modal_dockerfile(text: str, context: Path) -> str: + """``text``, a Dockerfile, with the two forms Modal's build refuses written as what ``docker build`` makes of them: + a JSON-form COPY or ADD whose paths hold no whitespace in shell form, which Modal's context parser reads, and an + ADD of files and folders in ``context``, none of them a tar archive, as the COPY it is, since Modal's ADD only + fetches a URL. Every other line is left as written.""" + lines, instruction = [], [] + for line in text.splitlines(keepends=True): + instruction.append(line) + if not line.rstrip("\r\n").endswith("\\"): + lines.append(_for_modal("".join(instruction), context)) + instruction = [] + return "".join(lines + instruction) + + +def _for_modal(instruction: str, context: Path) -> str: + match = _COPY_OR_ADD.fullmatch(re.sub(r"\\\r?\n", " ", instruction).strip()) + if match is None: + return instruction + keyword, rest = match.group(1).upper(), match.group(2).strip() + flags = [] + while rest.startswith("--"): + flag, _, rest = rest.partition(" ") + flags.append(flag) + rest = rest.strip() + if rest.startswith("["): + try: + args = json.loads(rest) + except ValueError: + return instruction + if not (isinstance(args, list) and len(args) >= 2 + and all(isinstance(arg, str) and arg and not any(c.isspace() for c in arg) for arg in args)): + return instruction + elif keyword == "ADD" and not rest.startswith("<<") and not any(c in rest for c in "\"'"): + args = rest.split() + else: + return instruction + if keyword == "ADD": + if len(args) < 2 or not all(flag.startswith(_COPY_FLAGS) for flag in flags) \ + or not all(_local_and_not_an_archive(source, context) for source in args[:-1]): + return instruction + keyword = "COPY" + return " ".join([keyword, *flags, *args]) + "\n" + + +def _local_and_not_an_archive(source: str, context: Path) -> bool: + """Whether ``source``, an ADD source, names files or folders in ``context``, none a tar archive ADD would unpack.""" + if "://" in source or source.startswith("git@"): + return False + matches = glob.glob(source.lstrip("/"), root_dir=context) if glob.has_magic(source) else [source.lstrip("/")] + paths = [context / match for match in matches] + return bool(paths) and all(path.exists() for path in paths) \ + and not any(path.is_file() and tarfile.is_tarfile(path) for path in paths) + + def _fetch_context(object_url: str, archive: Path, out: Path) -> None: """Download the build context at ``object_url`` to ``archive`` and unpack it into ``out``.""" get_config().get_object_store_at(object_url).download_to_file(object_url, str(archive)) diff --git a/tst/unit/providers/sandbox_providers/modal_context_build_test.py b/tst/unit/providers/sandbox_providers/modal_context_build_test.py index 88da350e..19221d9b 100644 --- a/tst/unit/providers/sandbox_providers/modal_context_build_test.py +++ b/tst/unit/providers/sandbox_providers/modal_context_build_test.py @@ -5,6 +5,7 @@ from __future__ import annotations import asyncio +import tarfile from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch @@ -23,7 +24,7 @@ class _Builds: """Stands in for ``modal.Image.from_dockerfile``: each image it returns records its context when built.""" def __init__(self, fail: Exception | None = None): - self.fail, self.calls, self.contexts = fail, [], [] + self.fail, self.calls, self.contexts, self.dockerfiles = fail, [], [], [] def __call__(self, path, *, context_dir, ignore): self.calls.append((Path(path), Path(context_dir), ignore)) @@ -31,6 +32,7 @@ def __call__(self, path, *, context_dir, ignore): async def build(app): self.contexts.append(sorted(p.relative_to(context_dir).as_posix() for p in Path(context_dir).rglob("*"))) + self.dockerfiles.append(Path(path).read_text()) await asyncio.sleep(0.01) # long enough for a second deploy to wait on the first if self.fail: raise self.fail @@ -80,7 +82,8 @@ async def test_an_image_is_built_from_its_unpacked_context_with_the_contexts_ign await _provider().prepare_image(image) [(dockerfile, context_dir, ignore)] = builds.calls - assert dockerfile == context_dir / "Dockerfile" + assert dockerfile == context_dir.parent / "Dockerfile" # beside the context, as Modal reads it + assert builds.dockerfiles == [SERVER["Dockerfile"]] assert builds.contexts == [[".dockerignore", "Dockerfile", "src", "src/app.py"]] assert ignore == ["*.log"] assert not context_dir.exists() # unpacked only for the build @@ -121,6 +124,46 @@ async def test_a_failed_build_names_the_image_and_the_next_deploy_builds_it_agai assert len(builds.calls) == 2 +@pytest.mark.asyncio +async def test_a_failed_modal_build_points_at_its_build_log(local_stores, context, builds): + builds.fail = modal.exception.ImageBuildError("Image build for im-abc failed.", "im-abc") + + with pytest.raises(RuntimeError, match="Image build for im-abc failed.; Modal's build log for im-abc has the cause"): + await _provider().prepare_image(_context_image("solver-image", context)) + + +@pytest.mark.parametrize("line, built", [ + ('COPY ["a.txt", "/x/a.txt"]', "COPY a.txt /x/a.txt"), + ('copy --chown=1:1 ["a.txt", "sub", "/y/"]', "COPY --chown=1:1 a.txt sub /y/"), + ("ADD a.txt /added/", "COPY a.txt /added/"), + ("ADD sub \\\n /added/sub/", "COPY sub /added/sub/"), + ("ADD *.txt /globbed/", "COPY *.txt /globbed/"), + ('ADD ["a.txt", "/j/"]', "COPY a.txt /j/"), + ("ADD --chmod=755 a.txt /c", "COPY --chmod=755 a.txt /c"), +], ids=["json-copy", "json-copy-flags", "add-file", "add-folder-continued", "add-glob", "json-add", "add-chmod"]) +def test_what_modal_cant_take_is_written_as_its_docker_equivalent(tmp_path, line, built): + (tmp_path / "a.txt").write_text("a") + (tmp_path / "sub").mkdir() + + assert modal_sandbox.modal_dockerfile(f"FROM python\n{line}\nRUN true\n", tmp_path) == \ + f"FROM python\n{built}\nRUN true\n" + + +@pytest.mark.parametrize("line", [ + 'COPY ["with space.txt", "/z/"]', "ADD archive.tar.gz /unpacked/", "ADD https://example.com/f /f", + "ADD --checksum=sha256:abc a.txt /c", "ADD missing.txt /m", "COPY --from=build /out /out", "COPY a.txt /x/", + 'ADD "a.txt" /q/', "ADD < Date: Fri, 9 Oct 2026 11:12:58 -0700 Subject: [PATCH 10/10] fix(modal): drop the build-log pointer, since Modal's own error names the command that shows the log Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/providers/sandbox_providers/modal_sandbox.py | 4 +--- .../sandbox_providers/modal_context_build_test.py | 7 ++++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index e8202d4c..118cfbf7 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -287,10 +287,8 @@ async def _build(self, image: DockerImageArtifact, app: modal.App) -> str: ) await built.build.aio(app) except Exception as e: - log = (f"; Modal's build log for {e.image_id} has the cause" - if isinstance(e, modal.exception.ImageBuildError) and getattr(e, "image_id", None) else "") raise RuntimeError(f"Building {image.image_name} on Modal from its build context failed: " - f"{_fmt_exc(e)}{log}") from e + f"{_fmt_exc(e)}") from e logger.info(f"Built {image.image_name} on Modal as {built.object_id}") return built.object_id diff --git a/tst/unit/providers/sandbox_providers/modal_context_build_test.py b/tst/unit/providers/sandbox_providers/modal_context_build_test.py index 19221d9b..afcc5b85 100644 --- a/tst/unit/providers/sandbox_providers/modal_context_build_test.py +++ b/tst/unit/providers/sandbox_providers/modal_context_build_test.py @@ -125,10 +125,11 @@ async def test_a_failed_build_names_the_image_and_the_next_deploy_builds_it_agai @pytest.mark.asyncio -async def test_a_failed_modal_build_points_at_its_build_log(local_stores, context, builds): - builds.fail = modal.exception.ImageBuildError("Image build for im-abc failed.", "im-abc") +async def test_a_failed_modal_build_keeps_modals_pointer_to_its_build_log(local_stores, context, builds): + builds.fail = modal.exception.ImageBuildError( + "Image build for im-abc failed.\nView the build logs:\n modal image logs im-abc", "im-abc") - with pytest.raises(RuntimeError, match="Image build for im-abc failed.; Modal's build log for im-abc has the cause"): + with pytest.raises(RuntimeError, match="(?s)Image build for im-abc failed.*modal image logs im-abc"): await _provider().prepare_image(_context_image("solver-image", context))