From 7816caf28428325b724b11f2c2820f07f2eeebb4 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sun, 11 Oct 2026 21:34:52 -0700 Subject: [PATCH 1/4] feat(sandbox): infra images keep their build contexts, and a sandbox that can't run an image's tar.gz builds it from that The run builds the gateway and service-db for this machine, linux/arm64 on Apple silicon, so a remote amd64 VM refused them after it was created, Modal's container gateway refused them as images in this machine's registry, and a run whose deploys were all remote didn't build them at all. - DockerImageArtifact.rebuilt_for(platform): an image with a tar.gz and a build context whose tar.gz a VM of that platform can't run. VmSandbox.load_docker_images builds those from the context for the VM's own platform and loads every other tar.gz, so an image stays byte-identical wherever it can. - Accepts.NAME_OR_CONTEXT takes any image with a build context, and Modal's prepare_image builds one that is only a context, named for a registry Modal can't reach, or built for another platform than Modal's. - Modal's container gateway checks and prepares its stored service-db images like its other images, so EnvironmentGatewayProvider.accepts loses its prepared argument. - The infra puts keep their build contexts. The gateway's narrows to gateway/, the website browser's Dockerfile carries PLAYWRIGHT_MCP_VERSION's default (a context carries no build args), and db-mcp drops its BuildKit-only cache mount, pins its clone, and fails if its sed patch stops applying. - With local stores, a run builds the infra for remote deploys too, and refuses it up front with no Docker here. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../artifact/artifacts/docker_image.py | 10 +++- src/agent_env/bundle/preflight.py | 27 ++++++--- src/agent_env/env/bootstrap.py | 34 ++++++------ .../env/envs/service_db/Dockerfile.db-mcp | 10 ++-- .../env/envs/website_browser/Dockerfile | 2 +- src/agent_env/env/gateway/Dockerfile | 4 +- .../env_providers/env_gateway_provider.py | 26 +++++---- .../sandbox_providers/modal_sandbox.py | 17 ++++-- .../providers/sandbox_providers/sandbox.py | 30 +++++----- .../sandbox_providers/sandbox_provider.py | 6 +- tst/integration/env/gateway/gateway_test.py | 4 +- .../env/task_step/task_steps_test.py | 2 +- .../test_docker_image_build_context.py | 17 ++++++ tst/unit/bundle/preflight_test.py | 55 +++++++++++++------ tst/unit/cli/build_platform_test.py | 4 +- tst/unit/env/bootstrap_test.py | 35 ++++++++++-- .../env_gateway_provider_test.py | 48 +++++++++++++--- .../modal_context_build_test.py | 19 ++++++- .../provider_capabilities_test.py | 6 +- .../sandbox_providers/vm_sandbox_test.py | 21 +++++++ 20 files changed, 272 insertions(+), 105 deletions(-) diff --git a/src/agent_env/artifact/artifacts/docker_image.py b/src/agent_env/artifact/artifacts/docker_image.py index 14af42d7..04356b8f 100644 --- a/src/agent_env/artifact/artifacts/docker_image.py +++ b/src/agent_env/artifact/artifacts/docker_image.py @@ -275,12 +275,18 @@ def context_only(self) -> bool: 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 rebuilt_for(self, platform: str | None) -> bool: + """Whether a VM sandbox of ``platform`` builds this image from its build context rather than load its tar.gz, + which is built for another platform: the image runs, though not byte for byte the one stored.""" + return bool(self.tar_gz_object_url and self.build_context_object_url) and self.platform_problem(platform) is not None + def load_problem(self, *, on_this_machine: bool, platform: str | None = None) -> str | None: """Why a VM sandbox can't get this image, or None when it can: a tar.gz is loaded, an image with none is pulled when its name names a registry, one that a VM ``on_this_machine`` or not reaches, and built from its build - context when it has only that; and a VM of ``platform`` runs only an image built for it.""" + context when it has only that; and a VM of ``platform`` runs only an image built for it, or one it rebuilds + from the build context beside a tar.gz of another platform.""" if self.tar_gz_object_url or self.context_only: - return self.platform_problem(platform) + return None if self.rebuilt_for(platform) else self.platform_problem(platform) if names_registry(self.image_name): return pull_problem(self.image_name, on_this_machine=on_this_machine) or self.platform_problem(platform) return (f"{self.id!r} v{self.version} has no tar.gz, and its image name {self.image_name!r} doesn't name a " diff --git a/src/agent_env/bundle/preflight.py b/src/agent_env/bundle/preflight.py index d99b8a6a..0fc8399c 100644 --- a/src/agent_env/bundle/preflight.py +++ b/src/agent_env/bundle/preflight.py @@ -16,9 +16,10 @@ 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 -done (``agent_env.env.bootstrap``), so they're named here, and refused here when they can't be built. +A gateway deploy 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. In local stores the run builds those once its writes are done +(``agent_env.env.bootstrap``), for this machine's platform and with their build contexts, which a VM or Modal running +another platform builds them from; so they're named here, and refused here when they can't be built. """ from __future__ import annotations @@ -40,6 +41,7 @@ default_env_id, infra_to_build, put_command, + stores_are_local, ) from agent_env.env.env import Env from agent_env.env.envs._deployment import provider_or_class @@ -154,11 +156,15 @@ def finish(self) -> Preflight: self.run_problems.extend(e.problems) contexts = self._contexts() self._default_agent() - if self.docker_users and (reason := docker_unreachable()): + reason = docker_unreachable() if self.docker_users or infra else None + if reason and self.docker_users: shown = ", ".join(self.docker_users[:_SHOWN_DOCKER_USERS]) more = len(self.docker_users) - _SHOWN_DOCKER_USERS self.run_problems.append(f"the local sandbox provider runs containers for {shown}" f"{f' and {more} more' if more > 0 else ''}, and {reason}") + elif reason: # infra the run builds for deploys elsewhere + self.run_problems.append(f"building the {', '.join(build.kind for build in infra)} env needs docker, and " + f"{reason}") problems = list(self.run_problems) for problem, wheres in self.problems.items(): first, *others = wheres @@ -191,10 +197,12 @@ def _env(self, where: str, step: DeployEnvTaskStep) -> None: if any(link.ON_THIS_MACHINE for link in taking): self.infra |= kinds self.docker_users.append(where) + elif taking and stores_are_local(): + self.infra |= kinds def _gateway_elsewhere(self, env_id: str, env_provider: type, kinds: set[str], link: SandboxProvider) -> list[str]: """Why ``link``, a provider not on this machine, can't run env ``env_id``'s gateway deploy: its sandboxes, or - the infra envs of ``kinds`` it runs, which agent-env builds only for the local provider.""" + the infra envs of ``kinds`` it runs.""" found = [] if problem := env_provider.sandbox_problem(link, websites=WEBSITE_BROWSER in kinds): found.append(f"deploys env {env_id!r} on the {_shown(link)} sandbox provider, which {problem}; run it on a VM " @@ -393,13 +401,14 @@ def _remote_infra(self, link: SandboxProvider, kinds: set[str]) -> list[str]: try: env = Env.get(env_id) except NotFoundError: - found.append(f"deploys on the {_shown(link)} sandbox provider, which needs the {kind} env {env_id!r}, and " - "the store doesn't hold it; agent-env builds it only for the local sandbox provider, so put " - f"it in a store that provider can reach (`{put_command(kind)}`)") + if not stores_are_local(): # in local stores the run builds it + found.append(f"deploys on the {_shown(link)} sandbox provider, which needs the {kind} env " + f"{env_id!r}, and the store doesn't hold it; agent-env builds infra envs only into local " + f"stores, so put it with `{put_command(kind)}`") continue artifacts = ([env.db_docker_image_artifact, env.db_web_docker_image_artifact, env.db_mcp_docker_image_artifact] if isinstance(env, ServiceDBEnv) else [env.docker_image_artifact]) - accepts = EnvironmentGatewayProvider.accepts(link, prepared=kind == GATEWAY) + accepts = EnvironmentGatewayProvider.accepts(link) found += [refusal for image in artifacts if (refusal := self._refusal(link, accepts, _Image(f"the {kind} env {env_id!r}'s image {image.id!r}", image), vms_load=True))] diff --git a/src/agent_env/env/bootstrap.py b/src/agent_env/env/bootstrap.py index fa54ddfb..00932c7e 100644 --- a/src/agent_env/env/bootstrap.py +++ b/src/agent_env/env/bootstrap.py @@ -3,6 +3,8 @@ (``default_gateway_env_id``, ``default_service_db_env_id``, ``default_website_browser_env_id``). The put commands build them one at a time; ``ensure_default_envs`` builds the ones a run needs, into local stores only. +Each image keeps its build context beside its tar.gz, so a VM or Modal running another platform than the one it was +built on builds it again from that. """ from __future__ import annotations @@ -17,11 +19,7 @@ from agent_env.env.env import Env from agent_env.env.envs import GatewayEnv, MCPServerEnv from agent_env.env.envs.service_db import ServiceDBEnv -from agent_env.env.envs.website_browser import ( - PLAYWRIGHT_MCP_VERSION, - WEBSITE_BROWSER_ENVIRONMENT_NAME, - WEBSITE_BROWSER_IMAGE_TAG, -) +from agent_env.env.envs.website_browser import WEBSITE_BROWSER_ENVIRONMENT_NAME, WEBSITE_BROWSER_IMAGE_TAG from agent_env.store.base import NotFoundError from agent_env.store.document_store import LocalSqliteDocumentStore from agent_env.store.ids import derive_id @@ -34,7 +32,7 @@ _ENV_PACKAGE = Path(__file__).parent GATEWAY_DOCKERFILE = _ENV_PACKAGE / "gateway" / "Dockerfile" -GATEWAY_CONTEXT = _ENV_PACKAGE +GATEWAY_CONTEXT = GATEWAY_DOCKERFILE.parent GATEWAY_IMAGE_TAG = "env-gateway" SERVICE_DB_DOCKERFILE = _ENV_PACKAGE / "envs" / "service_db" / "Dockerfile" SERVICE_DB_IMAGE_NAME = "agent-env-service-db" @@ -59,7 +57,8 @@ def put_gateway_env(env_id: str, *, platform: str | None, metadata: Mapping[str, build_image(GATEWAY_DOCKERFILE, GATEWAY_CONTEXT, GATEWAY_IMAGE_TAG, platform=platform) say("Creating DockerImageArtifact...") artifact = DockerImageArtifact.put(id=derive_id(env_id, "env_image"), description="Created from agent-env CLI", - image_name=GATEWAY_IMAGE_TAG) + image_name=GATEWAY_IMAGE_TAG, build_context_path=str(GATEWAY_CONTEXT), + dockerfile_path=GATEWAY_DOCKERFILE.name) say(f"Created artifact: id={artifact.id} version={artifact.version}") say("Creating GatewayEnv...") env = GatewayEnv.put(id=env_id, docker_image_artifact=artifact, @@ -84,7 +83,9 @@ def put_service_db_env(env_id: str, *, platform: str | None, metadata: Mapping[s say(f"Building {label} image from {dockerfile}...") build_image(dockerfile, dockerfile.parent, image, platform=platform) say(f"Creating {label} DockerImageArtifact...") - artifacts.append(DockerImageArtifact.put(id=artifact_id, description=description, image_name=image)) + artifacts.append(DockerImageArtifact.put(id=artifact_id, description=description, image_name=image, + build_context_path=str(dockerfile.parent), + dockerfile_path=dockerfile.name)) say(f"Created {label} DockerImageArtifact: id={artifacts[-1].id} version={artifacts[-1].version}") say("Creating ServiceDBEnv...") env = ServiceDBEnv.put( @@ -103,11 +104,12 @@ def put_website_browser_env(env_id: str, *, platform: str | None, metadata: Mapp """Build the website browser image for ``platform`` (this host's when None) and write it as the MCP server env ``env_id``.""" say("Building website browser Docker image...") - build_image(WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_CONTEXT, WEBSITE_BROWSER_IMAGE_TAG, platform=platform, - build_args={"PLAYWRIGHT_MCP_VERSION": PLAYWRIGHT_MCP_VERSION}) + build_image(WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_CONTEXT, WEBSITE_BROWSER_IMAGE_TAG, platform=platform) say("Creating DockerImageArtifact...") artifact = DockerImageArtifact.put(id=derive_id(env_id, "env_image"), description="Website browser MCP server", - image_name=WEBSITE_BROWSER_IMAGE_TAG) + image_name=WEBSITE_BROWSER_IMAGE_TAG, + build_context_path=str(WEBSITE_BROWSER_CONTEXT), + dockerfile_path=WEBSITE_BROWSER_DOCKERFILE.name) say(f"Created artifact: id={artifact.id} version={artifact.version}") say("Creating MCPServerEnv...") env = MCPServerEnv.put(id=env_id, docker_image_artifact=artifact, environment_name=WEBSITE_BROWSER_ENVIRONMENT_NAME, @@ -123,21 +125,19 @@ def _metadata(kind: str, dockerfile: Path, context: Path, extra: Mapping[str, st BUILD_INPUTS_KEY = "build_inputs_sha256" # the metadata key a put records build_inputs_digest under _BUILD_INPUTS = { - GATEWAY: lambda: _files_under(GATEWAY_DOCKERFILE.parent), # the gateway's Dockerfile copies only its own folder + GATEWAY: lambda: _files_under(GATEWAY_CONTEXT), SERVICE_DB: lambda: [SERVICE_DB_DOCKERFILE, DB_WEB_DOCKERFILE, DB_MCP_DOCKERFILE], # they copy nothing WEBSITE_BROWSER: lambda: [WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_CONTEXT / "entrypoint.sh"], } -_BUILD_ARGS = {WEBSITE_BROWSER: {"PLAYWRIGHT_MCP_VERSION": PLAYWRIGHT_MCP_VERSION}} def build_inputs_digest(kind: str) -> str: - """A digest of what agent-env builds ``kind``'s images from: the Dockerfiles it ships, the files they copy and the - build args. What a build fetches (base images, packages) isn't an input.""" + """A digest of what agent-env builds ``kind``'s images from: the Dockerfiles it ships and the files they copy, + which hold every build arg's value too, so a build from the build context alone builds the same thing. What a + build fetches (base images, packages) isn't an input.""" digest = hashlib.sha256() for path in sorted(_BUILD_INPUTS[kind]()): digest.update(path.relative_to(_ENV_PACKAGE).as_posix().encode() + b"\0" + path.read_bytes() + b"\0") - for name, value in sorted(_BUILD_ARGS.get(kind, {}).items()): - digest.update(f"{name}={value}\0".encode()) return digest.hexdigest() diff --git a/src/agent_env/env/envs/service_db/Dockerfile.db-mcp b/src/agent_env/env/envs/service_db/Dockerfile.db-mcp index 869f9c04..5b71e411 100644 --- a/src/agent_env/env/envs/service_db/Dockerfile.db-mcp +++ b/src/agent_env/env/envs/service_db/Dockerfile.db-mcp @@ -10,17 +10,19 @@ WORKDIR /app RUN apt-get update \ && apt-get install -y libpq-dev gcc git \ && rm -rf /var/lib/apt/lists/* -RUN git clone https://github.com/crystaldba/postgres-mcp.git /app +# Pinned, so every build, here or in a VM, builds the same source; move the pin on purpose. +RUN git clone https://github.com/crystaldba/postgres-mcp.git /app \ + && git -C /app checkout --quiet 15c8e33353546148acc2d8bd784551cf3905d1e2 # Patch: disable DNS rebinding protection in FastMCP constructor. # FastMCP auto-enables it when host="127.0.0.1" (the default), which rejects # requests arriving through reverse proxies with non-localhost Host headers. # postgres-mcp creates FastMCP("postgres-mcp") with defaults, then sets # mcp.settings.host later — but transport_security is already locked in. -RUN sed -i 's/mcp = FastMCP("postgres-mcp")/from mcp.server.transport_security import TransportSecuritySettings\nmcp = FastMCP("postgres-mcp", transport_security=TransportSecuritySettings(enable_dns_rebinding_protection=False))/' /app/src/postgres_mcp/server.py +RUN sed -i 's/mcp = FastMCP("postgres-mcp")/from mcp.server.transport_security import TransportSecuritySettings\nmcp = FastMCP("postgres-mcp", transport_security=TransportSecuritySettings(enable_dns_rebinding_protection=False))/' /app/src/postgres_mcp/server.py \ + && grep -q "enable_dns_rebinding_protection=False" /app/src/postgres_mcp/server.py -RUN --mount=type=cache,target=/root/.cache/uv \ - uv sync --frozen --no-dev +RUN uv sync --frozen --no-dev FROM public.ecr.aws/docker/library/python:3.12-slim-bookworm RUN groupadd -r app && useradd -r -g app app diff --git a/src/agent_env/env/envs/website_browser/Dockerfile b/src/agent_env/env/envs/website_browser/Dockerfile index 43b5b40c..ea1f7d49 100644 --- a/src/agent_env/env/envs/website_browser/Dockerfile +++ b/src/agent_env/env/envs/website_browser/Dockerfile @@ -28,7 +28,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ WORKDIR /app -ARG PLAYWRIGHT_MCP_VERSION +ARG PLAYWRIGHT_MCP_VERSION=0.0.41 ENV PLAYWRIGHT_MCP_VERSION=${PLAYWRIGHT_MCP_VERSION} # Pre-install the pinned version and Playwright's Chromium, which is built for linux/amd64 and linux/arm64 alike diff --git a/src/agent_env/env/gateway/Dockerfile b/src/agent_env/env/gateway/Dockerfile index dd79e8fc..73000f01 100644 --- a/src/agent_env/env/gateway/Dockerfile +++ b/src/agent_env/env/gateway/Dockerfile @@ -4,10 +4,10 @@ RUN apt-get update && apt-get install -y curl && rm -rf /var/lib/apt/lists/* WORKDIR /app -COPY gateway/requirements.txt . +COPY requirements.txt . RUN pip install --no-cache-dir -r requirements.txt -COPY gateway/ ./gateway/ +COPY . ./gateway/ ENV PYTHONPATH=/app 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 4adc83c3..206fea94 100644 --- a/src/agent_env/providers/env_providers/env_gateway_provider.py +++ b/src/agent_env/providers/env_providers/env_gateway_provider.py @@ -137,13 +137,12 @@ def in_vm(sandbox_provider: SandboxProvider) -> bool: return SANDBOX_MODE_VM in sandbox_provider.SUPPORTED_SANDBOX_MODES @classmethod - def accepts(cls, sandbox_provider: SandboxProvider, *, prepared: bool = True) -> Accepts: + def accepts(cls, sandbox_provider: SandboxProvider) -> Accepts: """The images a deploy on ``sandbox_provider`` runs: in a VM, any it loads; in containers, those the provider's - containers run, of the images the deploy prepares (its servers' and the gateway's), and the rest (its - service-db's) by name.""" + containers run, as it prepares each (its servers', the gateway's and its service-db's).""" if cls.in_vm(sandbox_provider): return Accepts.LOADABLE - return sandbox_provider.CONTAINER_ACCEPTS if prepared else Accepts.NAME + return sandbox_provider.CONTAINER_ACCEPTS @classmethod def sandbox_problem(cls, sandbox_provider: SandboxProvider, *, websites: bool) -> str | None: @@ -894,15 +893,20 @@ async def _deploy_via_containers( config = get_config() gateway_env = Env.get(config.default_gateway_env_id) - # Each image is checked, and prepared (one that is only a build context, built), before anything is created, so - # an image the provider can't run, or a failed build, leaves nothing. + # Each image is checked, and prepared (one the provider can't pull, built from its build context), before + # anything is created, so an image the provider can't run, or a failed build, leaves nothing. images = [gateway_env.docker_image_artifact, *mcp_server_images] - accepts = self.accepts(sandbox_provider) - problems = [problem for image in images - if (problem := accepts.problem(image, on_this_machine=sandbox_provider.ON_THIS_MACHINE))] + problems = [] if self._needs_local_postgres: # the servicedb and its sidecars, by the names the state provider picks - problems += [problem for name in self._state_provider.container_images() - if (problem := pull_problem(name, on_this_machine=sandbox_provider.ON_THIS_MACHINE))] + stored = {image.image_name: image for image in self._state_provider.store_images_to_load()} + for name in self._state_provider.container_images(): + if name in stored: + images.append(stored[name]) + elif problem := pull_problem(name, on_this_machine=sandbox_provider.ON_THIS_MACHINE): + problems.append(problem) + accepts = self.accepts(sandbox_provider) + problems += [problem for image in images + if (problem := accepts.problem(image, on_this_machine=sandbox_provider.ON_THIS_MACHINE))] if problems: raise ValueError(f"Can't deploy a gateway on {type(sandbox_provider).__name__}, which runs its images by " f"name: {'; '.join(problems)}") diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index af1ad617..9399bfa9 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -216,7 +216,8 @@ def __init__( # 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. + # The images prepare_image builds from their build contexts, by name, so create_container builds them rather + # than pull. self._context_images: dict[str, DockerImageArtifact] = {} async def _get_client(self) -> modal.Client: @@ -243,8 +244,12 @@ async def _get_app(self, app_name: str, app_tags: Optional[dict[str, str]] = Non return app 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: + """Build ``image`` from its build context when Modal can't run it by name: it's only a build context, its name + is in a registry Modal can't reach, or it's built for another platform than Modal's; unless this process has + already built its sources.""" + if image.build_context_object_url and ( + image.context_only or image.by_name_problem(on_this_machine=self.ON_THIS_MACHINE) + or image.platform_problem(self._sandbox_cls.platform)): self._context_images[image.image_name] = image await self._context_image(image) @@ -298,8 +303,8 @@ async def create_container( 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, built_id = await self._context_image(context_only) + if (prepared := self._context_images.get(image_name)) is not None: + image, built_id = await self._context_image(prepared) else: image = await self._registry_image(image_name) @@ -349,7 +354,7 @@ async def create(image: modal.Image) -> modal.Sandbox: 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) + image, built_id = await self._context_image(prepared, stale=built_id) sb = await create(image) except Exception as e: raise RuntimeError( diff --git a/src/agent_env/providers/sandbox_providers/sandbox.py b/src/agent_env/providers/sandbox_providers/sandbox.py index 2cb83a35..3016b919 100644 --- a/src/agent_env/providers/sandbox_providers/sandbox.py +++ b/src/agent_env/providers/sandbox_providers/sandbox.py @@ -294,32 +294,35 @@ async def wait_for_vm(self) -> None: async def load_docker_images(self, artifacts: list) -> None: """Put each DockerImageArtifact's image on the VM: those with a tar.gz are loaded from it, in parallel, those - with only a build context are built from it, and the rest are pulled by image name. An image this VM can't get, - such as one to pull from a registry on this machine when the VM isn't on it, or can't run, being built for - another platform than its own, is refused before anything is loaded.""" + with only a build context are built from it, and the rest are pulled by image name; one whose tar.gz is built + for another platform than the VM's is built from the build context beside it, for the VM's. An image this VM + can't get, such as one to pull from a registry on this machine when the VM isn't on it, or can't run, being + built for another platform with no build context, is refused before anything is loaded.""" if not artifacts: return if problems := self.load_problems(artifacts): raise RuntimeError(f"Can't load images: {'; '.join(problems)}") - if tarballs := [artifact for artifact in artifacts if artifact.tar_gz_object_url]: + built = [artifact for artifact in artifacts if artifact.context_only or artifact.rebuilt_for(self.platform)] + if tarballs := [artifact for artifact in artifacts + if artifact.tar_gz_object_url and not artifact.rebuilt_for(self.platform)]: await self._load_tarballs(tarballs) if pulled := [artifact.image_name for artifact in artifacts if not artifact.tar_gz_object_url and not artifact.context_only]: await self.pull_images(pulled) - if built := [artifact for artifact in artifacts if artifact.context_only]: + if built: await self.build_images(built) def load_problems(self, artifacts: list) -> list[str]: """Why this VM can't get or run each DockerImageArtifact it can't, as ``load_docker_images`` refuses them: one to pull from a registry on this machine when the VM isn't on it, one with no tar.gz whose name names no - registry, or one built for another platform than its own.""" + registry, or one built for another platform than its own with no build context beside it.""" return [problem for artifact in artifacts if (problem := artifact.load_problem(on_this_machine=self.ON_THIS_MACHINE, platform=self.platform))] async def build_images(self, artifacts: list) -> None: - """Build each context-only image from its build context, one build at a time, so heavy builds don't contend - for one VM; images with the same ``source_digest`` share a build. The build pulls its base images and packages - itself, so a network policy restricting egress must allow them; only the context's download is let through.""" + """Build each image from its build context, one build at a time, so heavy builds don't contend for one VM; + images with the same ``source_digest`` share a build. The build pulls its base images and packages itself, so + a network policy restricting egress must allow them; only the context's download is let through.""" builds: dict[str, list] = {} for artifact in artifacts: builds.setdefault(artifact.source_digest or artifact.build_context_object_url, []).append(artifact) @@ -327,13 +330,14 @@ async def build_images(self, artifacts: list) -> None: await self._build_image(images) async def _build_image(self, images: list) -> None: - """Download the context ``images`` share, extract it, and ``docker build`` it, tagged with each image's name. - The work folder is this build's own, since a host can run builds of one context at once: two of the local - provider's sandboxes, or two deploys onto one VM.""" + """Download the context ``images`` share, extract it, and ``docker build`` it, tagged with each image's name, + for the platform it records, or this VM's when its tar.gz is for another. The work folder is this build's own, + since a host can run builds of one context at once: two of the local provider's sandboxes, or two deploys onto + one VM.""" first = images[0] names = list(dict.fromkeys(image.image_name for image in images)) work = f"/tmp/agent-env-build-{uuid.uuid4().hex}" - platform = first.platform or DEFAULT_BUILD_PLATFORM + platform = self.platform if first.rebuilt_for(self.platform) else first.platform or DEFAULT_BUILD_PLATFORM tags = " ".join(f"-t {shlex.quote(name)}" for name in names) logger.info(f"Building {', '.join(names)} from its build context, for {platform}...") await self.exec_script(f"rm -rf {work} && mkdir -p {work}/context") diff --git a/src/agent_env/providers/sandbox_providers/sandbox_provider.py b/src/agent_env/providers/sandbox_providers/sandbox_provider.py index 32a71655..bb6dc46f 100644 --- a/src/agent_env/providers/sandbox_providers/sandbox_provider.py +++ b/src/agent_env/providers/sandbox_providers/sandbox_provider.py @@ -42,15 +42,15 @@ class Accepts(Enum): LOADABLE = "loadable" # any a VM can get: a tar.gz it loads, a registry name it pulls, a build context it builds NAME = "name" # one it starts by its name, which an image that is only a build context gets only from a build - NAME_OR_CONTEXT = "name_or_context" # one it starts by its name, or a build context it builds first (prepare_image) + NAME_OR_CONTEXT = "name_or_context" # one it starts by its name, or builds first from its build context (prepare_image) def problem(self, image: DockerImageArtifact, *, on_this_machine: bool) -> str | None: """Why a provider that accepts these, whose sandboxes run ``on_this_machine`` or elsewhere, can't run ``image``, or None when it can: one elsewhere can't pull from a registry on this machine.""" if self is Accepts.LOADABLE: return image.load_problem(on_this_machine=on_this_machine) - if self is Accepts.NAME_OR_CONTEXT and image.context_only: - return None # the provider builds it + if self is Accepts.NAME_OR_CONTEXT and image.build_context_object_url: + return None # the provider builds what it can't pull return image.by_name_problem(on_this_machine=on_this_machine) diff --git a/tst/integration/env/gateway/gateway_test.py b/tst/integration/env/gateway/gateway_test.py index 3b77d160..d1a79517 100644 --- a/tst/integration/env/gateway/gateway_test.py +++ b/tst/integration/env/gateway/gateway_test.py @@ -31,7 +31,6 @@ ) from agent_env.env.envs.service_db import ServiceDBEnv from agent_env.env.envs.website_browser import ( - PLAYWRIGHT_MCP_VERSION, WEBSITE_BROWSER_IMAGE_TAG, WEBSITE_BROWSER_ENVIRONMENT_NAME, ) @@ -64,7 +63,7 @@ class DockerImageConfig: name="gateway", dockerfile=ENV_DIR / "gateway" / "Dockerfile", tag="env-gateway", - context=ENV_DIR, + context=ENV_DIR / "gateway", service_name="gateway", ) @@ -229,7 +228,6 @@ def website_browser_env() -> MCPServerEnv: logger.info(f"Building {img.name} Docker image...") result = subprocess.run( ["docker", "build", "--platform", "linux/amd64", - "--build-arg", f"PLAYWRIGHT_MCP_VERSION={PLAYWRIGHT_MCP_VERSION}", "-f", str(img.dockerfile), "-t", img.tag, str(img.context)], capture_output=True, text=True, diff --git a/tst/integration/env/task_step/task_steps_test.py b/tst/integration/env/task_step/task_steps_test.py index 9a2ac0c9..25a63d9a 100644 --- a/tst/integration/env/task_step/task_steps_test.py +++ b/tst/integration/env/task_step/task_steps_test.py @@ -64,7 +64,7 @@ class DockerImageConfig: name="gateway", dockerfile=ENV_DIR / "gateway" / "Dockerfile", tag="env-gateway", - context=ENV_DIR, + context=ENV_DIR / "gateway", service_name="gateway", ) diff --git a/tst/unit/artifact/test_docker_image_build_context.py b/tst/unit/artifact/test_docker_image_build_context.py index 5662cc9b..cdf93344 100644 --- a/tst/unit/artifact/test_docker_image_build_context.py +++ b/tst/unit/artifact/test_docker_image_build_context.py @@ -175,3 +175,20 @@ def test_a_put_records_the_platform_as_docker_build_names_it(local_stores, conte platform="linux/arm64") assert image.platform == "linux/arm64" and image.source_digest == named.source_digest + + +@pytest.mark.parametrize("context_url, platform, rebuilt, problem", [ + ("s3://bucket/ctx.tar.gz", "linux/amd64", True, None), + ("s3://bucket/ctx.tar.gz", "linux/arm64", False, None), + ("s3://bucket/ctx.tar.gz", None, False, None), + (None, "linux/amd64", False, "'img' v1 is a linux/arm64 image, and the sandbox runs linux/amd64"), +], ids=["context-another-platform", "context-same-platform", "vm-runs-any", "no-context"]) +def test_a_vm_of_another_platform_builds_an_image_from_the_context_beside_its_tar_gz(context_url, platform, rebuilt, + problem): + image = DockerImageArtifact(id="img", version=1, description="d", image_name="localhost:5000/img:v1", + tar_gz_object_url="file:///state/img.tar.gz", build_context_object_url=context_url, + platform="linux/arm64") + + assert image.rebuilt_for(platform) is rebuilt + assert (image.load_problem(on_this_machine=False, platform=platform) or "").startswith(problem or "") + assert (image.load_problem(on_this_machine=False, platform=platform) is None) is (problem is None) diff --git a/tst/unit/bundle/preflight_test.py b/tst/unit/bundle/preflight_test.py index cae607f2..91238c89 100644 --- a/tst/unit/bundle/preflight_test.py +++ b/tst/unit/bundle/preflight_test.py @@ -367,23 +367,27 @@ def test_a_lone_server_whose_image_is_only_a_build_context_is_refused_on_any_pro 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): +def _built_here(id): + """An image as the infra bootstrap builds it: in this machine's registry, for its platform, with its tar.gz and + its build context.""" + with namespace_routing(): + return get_artifact_store().put_document(DockerImageArtifact( + id=id, description=id, image_name=f"localhost:5000/{id}:v1", tar_gz_s3_url="file:///tmp/img-v1.tar.gz", + build_context_object_url="file:///tmp/ctx.tar.gz", dockerfile_path="Dockerfile", platform="linux/arm64", + source_digest="sha256:" + "2" * 64)) + + +@pytest.mark.parametrize("image", [_context_image, _built_here], ids=["only-a-build-context", "built-here"]) +def test_modal_builds_the_gateway_and_service_db_images_it_cant_pull_from_their_build_contexts(bundle_dir, image): with namespace_routing(): MCPServerEnv.put(id="crm", docker_image_artifact=_image("crm-image", REMOTE), environment_name="crm") + GatewayEnv.put(id="default", docker_image_artifact=image("gateway-default")) + ServiceDBEnv.put(id="default-db", db_docker_image_artifact=image("service-db-default-db"), + db_web_docker_image_artifact=image("db-web-default-db"), + db_mcp_docker_image_artifact=image("db-mcp-default-db")) _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"] + assert [entry.name for entry in dry_run_bundle(bundle_dir, sandbox="modal").runs] == ["t"] # Images built for another platform than the sandboxes run, which a VM refuses as it loads them @@ -684,13 +688,32 @@ def test_missing_infra_in_stores_that_arent_local_is_refused_naming_its_put_comm ] -def test_a_gateway_deploy_on_another_provider_needs_infra_it_can_reach(bundle_dir): +def test_a_gateway_deploy_on_another_provider_has_its_missing_infra_built_in_local_stores(bundle_dir, monkeypatch): + """Built for this machine, with the build context a VM of another platform builds it from.""" _env("crm", REMOTE) _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) + assert [str(build) for build in dry_run_bundle(bundle_dir, sandbox="modal_vm").infra] == [ + "service-db env 'default-db' (missing)", "gateway env 'default' (missing)"] + monkeypatch.setattr("agent_env.bundle.preflight.stores_are_local", lambda: False) (problem, *_) = _problems(lambda: dry_run_bundle(bundle_dir, sandbox="modal_vm")) - assert problem.startswith("tasks/t.json: step 'env': deploys on the 'modal_vm' sandbox provider, which needs the " - "gateway env 'default', and the store doesn't hold it") + assert problem == ("tasks/t.json: step 'env': deploys on the 'modal_vm' sandbox provider, which needs the gateway " + "env 'default', and the store doesn't hold it; agent-env builds infra envs only into local " + "stores, so put it with `agent-env env gateway put --id default`") + + +def test_infra_built_for_deploys_elsewhere_needs_docker_here(bundle_dir, monkeypatch): + _env("crm", REMOTE) + _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) + monkeypatch.setattr(preflight_module, "docker_unreachable", lambda: "docker isn't on PATH") + + assert _problems(lambda: dry_run_bundle(bundle_dir, sandbox="modal_vm")) == [ + "building the service-db, gateway env needs docker, and docker isn't on PATH"] + + +def test_a_gateway_deploy_on_another_provider_needs_infra_it_can_reach(bundle_dir): + _env("crm", REMOTE) + _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) _infra(PULLED_HERE) problems = _problems(lambda: dry_run_bundle(bundle_dir, sandbox="modal_vm")) assert problems[0] == ("tasks/t.json: step 'env': deploys the gateway env 'default''s image 'gateway-default' on the " diff --git a/tst/unit/cli/build_platform_test.py b/tst/unit/cli/build_platform_test.py index ae15625a..8b365d18 100644 --- a/tst/unit/cli/build_platform_test.py +++ b/tst/unit/cli/build_platform_test.py @@ -8,7 +8,7 @@ from click.testing import CliRunner from agent_env.cli import cli -from agent_env.env.envs.website_browser import PLAYWRIGHT_MCP_VERSION, WEBSITE_BROWSER_IMAGE_TAG +from agent_env.env.envs.website_browser import WEBSITE_BROWSER_IMAGE_TAG from agent_env.utils.docker_build import DockerBuildError gateway = importlib.import_module("agent_env.cli.env.gateway") # the packages export the click commands @@ -50,7 +50,7 @@ def _put_commands(tmp_path): service_db.SERVICE_DB_IMAGE_NAME), None), "website-browser": ("agent_env.env.bootstrap", ["env", "website-browser", "put", "--id", "x"], (website_browser.WEBSITE_BROWSER_DOCKERFILE, website_browser.WEBSITE_BROWSER_CONTEXT, - WEBSITE_BROWSER_IMAGE_TAG), {"PLAYWRIGHT_MCP_VERSION": PLAYWRIGHT_MCP_VERSION}), + WEBSITE_BROWSER_IMAGE_TAG), None), } diff --git a/tst/unit/env/bootstrap_test.py b/tst/unit/env/bootstrap_test.py index 60d49fa4..18bf996a 100644 --- a/tst/unit/env/bootstrap_test.py +++ b/tst/unit/env/bootstrap_test.py @@ -2,6 +2,7 @@ stores only, for this host's platform, once each however many runs ask.""" import importlib +from pathlib import Path import pytest from click.testing import CliRunner @@ -12,6 +13,7 @@ from agent_env.env import GatewayEnv from agent_env.env import bootstrap from agent_env.env.bootstrap import GATEWAY, SERVICE_DB, WEBSITE_BROWSER, InfraError, ensure_default_envs +from agent_env.env.envs.website_browser import PLAYWRIGHT_MCP_VERSION @pytest.fixture @@ -35,6 +37,29 @@ def write(env_id, *, platform): return calls +@pytest.mark.parametrize("put, contexts", [ + (bootstrap.put_gateway_env, [(bootstrap.GATEWAY_CONTEXT, "Dockerfile")]), + (bootstrap.put_service_db_env, [(bootstrap.SERVICE_DB_DOCKERFILE.parent, name) + for name in ("Dockerfile", "Dockerfile.db-web", "Dockerfile.db-mcp")]), + (bootstrap.put_website_browser_env, [(bootstrap.WEBSITE_BROWSER_CONTEXT, "Dockerfile")]), +], ids=[GATEWAY, SERVICE_DB, WEBSITE_BROWSER]) +def test_each_infra_image_keeps_the_build_context_it_is_built_from(local_stores, monkeypatch, put, contexts): + """So a VM or Modal running another platform than this machine builds it again from that.""" + kept = [] + + def keep(id, *, description, image_name, build_context_path=None, dockerfile_path=None, platform=None): + kept.append((Path(build_context_path), dockerfile_path)) + return get_artifact_store().put_document(DockerImageArtifact( + id=id, description=description, image_name=image_name, tar_gz_s3_url="file:///x.tar.gz")) + + monkeypatch.setattr(bootstrap, "build_image", lambda *args, **kwargs: None) + monkeypatch.setattr(bootstrap.DockerImageArtifact, "put", keep) + + put("x", platform=None) + + assert kept == contexts + + def test_missing_infra_is_built_for_this_host_in_order_and_once(puts): said = [] @@ -168,12 +193,14 @@ def test_a_put_records_what_the_env_was_built_from_whatever_metadata_it_was_give assert env.metadata["owner"] == "me" -def test_the_build_inputs_are_the_shipped_files_and_build_args_less_python_caches(tmp_path, monkeypatch): +def test_the_build_inputs_are_the_shipped_files_less_python_caches(tmp_path): (tmp_path / "__pycache__").mkdir() (tmp_path / "__pycache__/gateway.cpython-312.pyc").write_text("compiled") (tmp_path / "gateway.py").write_text("served") - before = bootstrap.build_inputs_digest(WEBSITE_BROWSER) - monkeypatch.setitem(bootstrap._BUILD_ARGS, WEBSITE_BROWSER, {"PLAYWRIGHT_MCP_VERSION": "9.9.9"}) assert bootstrap._files_under(tmp_path) == [tmp_path / "gateway.py"] - assert bootstrap.build_inputs_digest(WEBSITE_BROWSER) != before + + +def test_the_website_browser_dockerfile_installs_the_playwright_mcp_version_agent_env_names(): + """A build from its build context alone, which carries no build args, installs that version too.""" + assert f"ARG PLAYWRIGHT_MCP_VERSION={PLAYWRIGHT_MCP_VERSION}\n" in bootstrap.WEBSITE_BROWSER_DOCKERFILE.read_text() 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 79e483bb..bcb5dc50 100644 --- a/tst/unit/providers/env_providers/env_gateway_provider_test.py +++ b/tst/unit/providers/env_providers/env_gateway_provider_test.py @@ -108,20 +108,18 @@ async def write_file_from_text(self, content: str, destination_path: str) -> Non self.write_calls.append((destination_path, content)) -@pytest.mark.parametrize("facts, in_vm, accepts, by_name_otherwise", [ - ({"SUPPORTED_SANDBOX_MODES": VMS_AND_CONTAINERS, "CONTAINER_ACCEPTS": Accepts.NAME}, True, Accepts.LOADABLE, Accepts.LOADABLE), - ({"SUPPORTED_SANDBOX_MODES": CONTAINERS, "CONTAINER_ACCEPTS": Accepts.NAME_OR_CONTEXT}, False, Accepts.NAME_OR_CONTEXT, Accepts.NAME), - ({"SUPPORTED_SANDBOX_MODES": CONTAINERS, "CONTAINER_ACCEPTS": Accepts.NAME}, False, Accepts.NAME, Accepts.NAME), +@pytest.mark.parametrize("facts, in_vm, accepts", [ + ({"SUPPORTED_SANDBOX_MODES": VMS_AND_CONTAINERS, "CONTAINER_ACCEPTS": Accepts.NAME}, True, Accepts.LOADABLE), + ({"SUPPORTED_SANDBOX_MODES": CONTAINERS, "CONTAINER_ACCEPTS": Accepts.NAME_OR_CONTEXT}, False, Accepts.NAME_OR_CONTEXT), + ({"SUPPORTED_SANDBOX_MODES": CONTAINERS, "CONTAINER_ACCEPTS": Accepts.NAME}, False, Accepts.NAME), ], ids=["vm", "containers-that-build", "containers-by-name"]) -def test_a_gateway_runs_in_a_vm_where_its_provider_creates_them_and_else_in_containers(facts, in_vm, accepts, - by_name_otherwise): - """In containers, the images the deploy prepares (its servers' and the gateway's) run as the provider's containers - run them, and the rest (its service-db's) by name.""" +def test_a_gateway_runs_in_a_vm_where_its_provider_creates_them_and_else_in_containers(facts, in_vm, accepts): + """In containers, every image the deploy prepares (its servers', the gateway's and its service-db's) runs as the + provider's containers run them.""" provider = MagicMock(**facts) assert EnvironmentGatewayProvider.in_vm(provider) is in_vm assert EnvironmentGatewayProvider.accepts(provider) is accepts - assert EnvironmentGatewayProvider.accepts(provider, prepared=False) is by_name_otherwise @pytest.mark.parametrize("facts, websites, problem", [ @@ -242,6 +240,38 @@ async def test_container_mode_refuses_a_servicedb_image_in_this_machines_registr ) +_SERVICEDB_BUILT_HERE = DockerImageArtifact(id="default-db__db_image", version=1, description="d", + image_name="localhost:5000/local/servicedb:v1", + tar_gz_object_url="file:///state/servicedb.tar.gz", + build_context_object_url="file:///state/ctx.tar.gz", platform="linux/arm64") + + +@pytest.mark.asyncio +async def test_container_mode_prepares_the_servicedb_images_it_starts_as_it_does_the_gateways(): + """As the infra bootstrap writes them: in this machine's registry, which Modal can't pull from, with the build + contexts Modal builds them from.""" + provider = ModalSandboxProvider() + gp = EnvironmentGatewayProvider() + gp._state_provider = MagicMock(spec=LocalPostgresStateProvider) + gp._state_provider.container_images.return_value = [_SERVICEDB_BUILT_HERE.image_name] + gp._state_provider.store_images_to_load.return_value = [_SERVICEDB_BUILT_HERE] + prepared = [] + + async def prepare(image, *, attribution=None): + prepared.append(image) + + with patch.object(provider, "prepare_image", side_effect=prepare), \ + patch.object(provider, "create_container", side_effect=RuntimeError("stop")), \ + patch("agent_env.env.env.Env.get", return_value=MagicMock(docker_image_artifact=_image("agent-gateway"))): + with pytest.raises(RuntimeError, match="stop"): + await gp._deploy_via_containers( + sandbox_provider=provider, mcp_servers=[MCPServerConfig(image="mcp-slack", environment_name="slack")], + mcp_server_images=[_image("mcp-slack")], gateway_port=18765, website_configs=None, + gateway_mode=MagicMock(value="performance"), ttl_seconds=60, disk_size_gb=10, + ) + assert [image.id for image in prepared] == ["agent-gateway", "mcp-slack", "default-db__db_image"] + + @pytest.mark.asyncio @pytest.mark.parametrize("unloadable", ["gateway", "server", "sidecar"]) async def test_a_vm_gateway_refuses_an_image_its_vm_cant_get_before_creating_the_vm(unloadable): 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 7a0f38a6..8a315c66 100644 --- a/tst/unit/providers/sandbox_providers/modal_context_build_test.py +++ b/tst/unit/providers/sandbox_providers/modal_context_build_test.py @@ -166,13 +166,30 @@ def test_everything_else_is_left_as_written(tmp_path, line): @pytest.mark.asyncio -async def test_an_image_that_is_not_only_a_build_context_is_not_built(local_stores, builds): +async def test_an_image_with_no_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 == [] +@pytest.mark.asyncio +@pytest.mark.parametrize("image_name, platform, built", [ + ("localhost:5000/local/gateway:v1", "linux/amd64", True), + ("registry.example/gateway:v1", "linux/arm64", True), + ("registry.example/gateway:v1", "linux/amd64", False), +], ids=["in-this-machines-registry", "for-another-platform", "pulled"]) +async def test_an_image_with_a_build_context_beside_its_tar_gz_is_built_when_modal_cant_run_it_by_name( + local_stores, context, builds, image_name, platform, built): + """As the infra bootstrap writes its images: a tar.gz, which Modal can't load, and the context it was built from.""" + image = _context_image("gateway-image", context).model_copy(update={ + "image_name": image_name, "tar_gz_object_url": "file:///state/gateway.tar.gz", "platform": platform}) + + await _provider().prepare_image(image) + + assert len(builds.calls) == int(built) + + def _fake_create(): """A patched ``modal.Sandbox._experimental_create`` that stops right after recording its call.""" create = MagicMock() diff --git a/tst/unit/providers/sandbox_providers/provider_capabilities_test.py b/tst/unit/providers/sandbox_providers/provider_capabilities_test.py index 3870748e..73326044 100644 --- a/tst/unit/providers/sandbox_providers/provider_capabilities_test.py +++ b/tst/unit/providers/sandbox_providers/provider_capabilities_test.py @@ -164,6 +164,7 @@ async def test_a_provider_whose_containers_are_vms_of_their_own_puts_none_on_a_p image_name="localhost:5000/local/img:v2") WITH_TAR_GZ = DockerImageArtifact(id="img", version=2, description="d", image_name="localhost:5000/local/img:v2", tar_gz_object_url="file:///state/img.tar.gz") +WITH_TAR_GZ_AND_CONTEXT = WITH_TAR_GZ.model_copy(update={"build_context_object_url": "file:///state/ctx.tar.gz"}) HERE = "is in a registry on this machine, which a sandbox elsewhere can't pull from" @@ -181,9 +182,12 @@ def test_an_image_is_refused_by_the_form_its_provider_accepts(image, accepts, pr (IN_THIS_MACHINES_REGISTRY, LOADABLE, HERE), (IN_THIS_MACHINES_REGISTRY, NAME, HERE), (IN_THIS_MACHINES_REGISTRY, NAME_OR_CONTEXT, HERE), (WITH_TAR_GZ, LOADABLE, None), (WITH_TAR_GZ, NAME, HERE), (WITH_TAR_GZ, NAME_OR_CONTEXT, HERE), + (WITH_TAR_GZ_AND_CONTEXT, LOADABLE, None), (WITH_TAR_GZ_AND_CONTEXT, NAME, HERE), + (WITH_TAR_GZ_AND_CONTEXT, NAME_OR_CONTEXT, None), ]) def test_a_sandbox_elsewhere_cant_pull_from_this_machines_registry_and_one_on_it_can(image, accepts, problem): - """A VM loads an image's tar.gz, which agent-env pushes to it, and pulls one only when it has none.""" + """A VM loads an image's tar.gz, which agent-env pushes to it, and pulls one only when it has none; a provider that + builds images builds one it can't pull from its build context.""" found = accepts.problem(image, on_this_machine=False) assert (found is None) if problem is None else (problem in found) assert accepts.problem(image, on_this_machine=True) is None diff --git a/tst/unit/providers/sandbox_providers/vm_sandbox_test.py b/tst/unit/providers/sandbox_providers/vm_sandbox_test.py index edc937be..367829e3 100644 --- a/tst/unit/providers/sandbox_providers/vm_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/vm_sandbox_test.py @@ -244,6 +244,27 @@ async def test_a_vm_refuses_a_tar_gz_for_a_platform_it_doesnt_run_before_anythin assert "docker load" in anything.scripts[0] +@pytest.mark.asyncio +async def test_a_vm_builds_an_image_from_the_build_context_beside_a_tar_gz_for_another_platform(signing_store): + """As the infra bootstrap writes its images on Apple silicon: built for the VM's platform, the image runs, though + not byte for byte the one stored; a VM of the tar.gz's platform loads the tar.gz.""" + arm64 = DockerImageArtifact(id="gw", version=1, description="d", image_name="localhost:5000/gw:v1", + platform="linux/arm64", tar_gz_object_url="s3://bucket/gw.tar.gz", + build_context_object_url="s3://bucket/gw/build-context.tar.gz", + dockerfile_path="Dockerfile", source_digest="sha256:" + "3" * 64) + + amd = _RecordingVmSandbox() + await amd.load_docker_images([arm64]) + assert not any("docker load" in script for script in amd.scripts) + assert any("docker build --platform linux/amd64 -f Dockerfile -t localhost:5000/gw:v1 ." in script + for script in amd.scripts) + + arm = _RecordingVmSandbox(images_stdout="localhost:5000/gw:v1\n") + arm.platform = "linux/arm64" + await arm.load_docker_images([arm64]) + assert "docker load" in arm.scripts[0] and not any("docker build" in script for script in arm.scripts) + + @pytest.mark.asyncio async def test_a_login_is_minted_once_per_registry(): minted = [] From f7249d1cd59a6e00ec3c9162b5c6a9672a555699 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sun, 11 Oct 2026 21:47:37 -0700 Subject: [PATCH 2/4] fix(preflight): infra the run rebuilds isn't checked as it stands; Modal drops a prepared image it then pulls Stock infra in local stores built before its images kept their build contexts is in this machine's registry, so on Modal preflight refused it and never scheduled the rebuild that would fix it; infra the run builds anew (missing, or built from other inputs) is no longer checked. And a Modal provider that prepared an image under a name, then a pullable one under the same name, built the first: prepare_image now drops the earlier entry when it pulls. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/bundle/preflight.py | 13 +++++++------ .../providers/sandbox_providers/modal_sandbox.py | 4 +++- tst/unit/bundle/preflight_test.py | 11 +++++++++++ .../sandbox_providers/modal_context_build_test.py | 13 +++++++++++++ 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/agent_env/bundle/preflight.py b/src/agent_env/bundle/preflight.py index 0fc8399c..2da18515 100644 --- a/src/agent_env/bundle/preflight.py +++ b/src/agent_env/bundle/preflight.py @@ -394,17 +394,18 @@ def _refusal(self, link: SandboxProvider, form: Accepts, image: _Image, *, vms_l f"on a provider {builds}") def _remote_infra(self, link: SandboxProvider, kinds: set[str]) -> list[str]: - """Why ``link``, a provider not on this machine, can't run the infra envs of ``kinds``.""" + """Why ``link``, a provider not on this machine, can't run the infra envs of ``kinds``. One the run builds, in + local stores missing or built from other inputs, isn't checked: it's built anew, with its build context.""" + built = {build.kind for build in infra_to_build(kinds)} if stores_are_local() else set() found = [] - for kind in sorted(kinds): + for kind in sorted(kinds - built): env_id = default_env_id(kind) try: env = Env.get(env_id) except NotFoundError: - if not stores_are_local(): # in local stores the run builds it - found.append(f"deploys on the {_shown(link)} sandbox provider, which needs the {kind} env " - f"{env_id!r}, and the store doesn't hold it; agent-env builds infra envs only into local " - f"stores, so put it with `{put_command(kind)}`") + found.append(f"deploys on the {_shown(link)} sandbox provider, which needs the {kind} env {env_id!r}, " + f"and the store doesn't hold it; agent-env builds infra envs only into local stores, so put " + f"it with `{put_command(kind)}`") continue artifacts = ([env.db_docker_image_artifact, env.db_web_docker_image_artifact, env.db_mcp_docker_image_artifact] if isinstance(env, ServiceDBEnv) else [env.docker_image_artifact]) diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index 9399bfa9..a485dc48 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -246,12 +246,14 @@ async def _get_app(self, app_name: str, app_tags: Optional[dict[str, str]] = Non async def prepare_image(self, image: DockerImageArtifact, *, attribution: Optional[Attribution] = None) -> None: """Build ``image`` from its build context when Modal can't run it by name: it's only a build context, its name is in a registry Modal can't reach, or it's built for another platform than Modal's; unless this process has - already built its sources.""" + already built its sources. One it pulls drops an image prepared earlier under its name.""" if image.build_context_object_url and ( image.context_only or image.by_name_problem(on_this_machine=self.ON_THIS_MACHINE) or image.platform_problem(self._sandbox_cls.platform)): self._context_images[image.image_name] = image await self._context_image(image) + else: + self._context_images.pop(image.image_name, None) async def _context_image(self, image: DockerImageArtifact, *, stale: str | None = None) -> tuple[modal.Image, str]: diff --git a/tst/unit/bundle/preflight_test.py b/tst/unit/bundle/preflight_test.py index 91238c89..8076c19f 100644 --- a/tst/unit/bundle/preflight_test.py +++ b/tst/unit/bundle/preflight_test.py @@ -702,6 +702,17 @@ def test_a_gateway_deploy_on_another_provider_has_its_missing_infra_built_in_loc "stores, so put it with `agent-env env gateway put --id default`") +def test_stock_infra_built_from_other_inputs_is_rebuilt_for_modal_rather_than_refused(bundle_dir): + """Infra built before its images kept their build contexts is in this machine's registry, which Modal can't pull + from; built anew, it carries the contexts Modal builds it from.""" + _env("crm", REMOTE) + _infra(PULLED_HERE, built_from="an earlier release's") + _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) + + assert [str(build) for build in dry_run_bundle(bundle_dir, sandbox="modal").infra] == [ + "service-db env 'default-db' (its build inputs changed)", "gateway env 'default' (its build inputs changed)"] + + def test_infra_built_for_deploys_elsewhere_needs_docker_here(bundle_dir, monkeypatch): _env("crm", REMOTE) _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) 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 8a315c66..af052a45 100644 --- a/tst/unit/providers/sandbox_providers/modal_context_build_test.py +++ b/tst/unit/providers/sandbox_providers/modal_context_build_test.py @@ -190,6 +190,19 @@ async def test_an_image_with_a_build_context_beside_its_tar_gz_is_built_when_mod assert len(builds.calls) == int(built) +@pytest.mark.asyncio +async def test_an_image_modal_pulls_drops_one_prepared_earlier_under_its_name(local_stores, context, builds): + provider = _provider() + built = _context_image("gateway-image", context).model_copy(update={ + "image_name": "registry.example/gateway:v1", "tar_gz_object_url": "file:///state/gateway.tar.gz", + "platform": "linux/arm64"}) + await provider.prepare_image(built) + + await provider.prepare_image(built.model_copy(update={"platform": "linux/amd64"})) + + assert provider._context_images == {} + + def _fake_create(): """A patched ``modal.Sandbox._experimental_create`` that stops right after recording its call.""" create = MagicMock() From 1460dcdd9c5ac90a4b93aaf5128a4a5d11336dcc Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sun, 11 Oct 2026 22:49:05 -0700 Subject: [PATCH 3/4] refactor(bootstrap): one helper builds and stores each infra image; build inputs are its build contexts - `_build_and_put` builds an infra image from its Dockerfile's folder and stores that folder as its build context, so the context a sandbox rebuilds from can't drift from what was built here. - `build_inputs_digest` hashes those build contexts (`BuildContext.source_digest`) instead of a hand-kept file list, so a Dockerfile that copies a new file is rebuilt without anyone updating the list. - The website browser's Dockerfile `ARG` is its one Playwright MCP version; the unread Python constant goes. - Preflight and `ensure_default_envs` share one message for infra that needs docker. - Modal's `prepare_image` drops a check `by_name_problem` already makes. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/agent_env/bundle/preflight.py | 4 +- src/agent_env/env/bootstrap.py | 114 ++++++++---------- .../env/envs/website_browser/Dockerfile | 1 + .../env/envs/website_browser/__init__.py | 5 - .../sandbox_providers/modal_sandbox.py | 5 +- tst/unit/env/bootstrap_test.py | 37 +++--- 6 files changed, 75 insertions(+), 91 deletions(-) diff --git a/src/agent_env/bundle/preflight.py b/src/agent_env/bundle/preflight.py index 2da18515..dc282c0c 100644 --- a/src/agent_env/bundle/preflight.py +++ b/src/agent_env/bundle/preflight.py @@ -40,6 +40,7 @@ InfraError, default_env_id, infra_to_build, + needs_docker, put_command, stores_are_local, ) @@ -163,8 +164,7 @@ def finish(self) -> Preflight: self.run_problems.append(f"the local sandbox provider runs containers for {shown}" f"{f' and {more} more' if more > 0 else ''}, and {reason}") elif reason: # infra the run builds for deploys elsewhere - self.run_problems.append(f"building the {', '.join(build.kind for build in infra)} env needs docker, and " - f"{reason}") + self.run_problems.append(needs_docker(infra, reason)) problems = list(self.run_problems) for problem, wheres in self.problems.items(): first, *others = wheres diff --git a/src/agent_env/env/bootstrap.py b/src/agent_env/env/bootstrap.py index 00932c7e..02263c5a 100644 --- a/src/agent_env/env/bootstrap.py +++ b/src/agent_env/env/bootstrap.py @@ -27,6 +27,7 @@ from agent_env.store.local_state import holding_locks from agent_env.store.object_store import LocalFilesystemObjectStore from agent_env.store.routing import configured_store +from agent_env.utils.build_context import BuildContext from agent_env.utils.build_metadata import detect_env_metadata from agent_env.utils.docker_build import build_image, docker_unreachable @@ -53,16 +54,10 @@ def _quiet(line: str) -> None: def put_gateway_env(env_id: str, *, platform: str | None, metadata: Mapping[str, str] | None = None, say: Callable[[str], None] = _quiet) -> GatewayEnv: """Build the gateway image for ``platform`` (this host's when None) and write it as the gateway env ``env_id``.""" - say("Building gateway Docker image...") - build_image(GATEWAY_DOCKERFILE, GATEWAY_CONTEXT, GATEWAY_IMAGE_TAG, platform=platform) - say("Creating DockerImageArtifact...") - artifact = DockerImageArtifact.put(id=derive_id(env_id, "env_image"), description="Created from agent-env CLI", - image_name=GATEWAY_IMAGE_TAG, build_context_path=str(GATEWAY_CONTEXT), - dockerfile_path=GATEWAY_DOCKERFILE.name) - say(f"Created artifact: id={artifact.id} version={artifact.version}") + artifact = _build_and_put("gateway", derive_id(env_id, "env_image"), "Created from agent-env CLI", + GATEWAY_DOCKERFILE, GATEWAY_IMAGE_TAG, platform=platform, say=say) say("Creating GatewayEnv...") - env = GatewayEnv.put(id=env_id, docker_image_artifact=artifact, - metadata=_metadata(GATEWAY, GATEWAY_DOCKERFILE, GATEWAY_CONTEXT, metadata)) + env = GatewayEnv.put(id=env_id, docker_image_artifact=artifact, metadata=_metadata(GATEWAY, metadata)) say(f"Created GatewayEnv: id={env.id} version={env.version}") return env @@ -71,30 +66,17 @@ def put_service_db_env(env_id: str, *, platform: str | None, metadata: Mapping[s say: Callable[[str], None] = _quiet) -> ServiceDBEnv: """Build the service-db, db-web and db-mcp images for ``platform`` (this host's when None) and write them as the service-db env ``env_id``.""" - artifacts = [] - for label, dockerfile, image, artifact_id, description in ( - ("ServiceDB", SERVICE_DB_DOCKERFILE, SERVICE_DB_IMAGE_NAME, derive_id(env_id, "db_image"), - f"ServiceDB PostgreSQL image for {env_id}"), - ("db-web", DB_WEB_DOCKERFILE, DB_WEB_IMAGE_NAME, derive_id(env_id, "db_web_image"), - "db-web lightweight web UI for database inspection"), - ("db-mcp", DB_MCP_DOCKERFILE, DB_MCP_IMAGE_NAME, derive_id(env_id, "db_mcp_image"), - "db-mcp PostgreSQL MCP server for direct DB access"), - ): - say(f"Building {label} image from {dockerfile}...") - build_image(dockerfile, dockerfile.parent, image, platform=platform) - say(f"Creating {label} DockerImageArtifact...") - artifacts.append(DockerImageArtifact.put(id=artifact_id, description=description, image_name=image, - build_context_path=str(dockerfile.parent), - dockerfile_path=dockerfile.name)) - say(f"Created {label} DockerImageArtifact: id={artifacts[-1].id} version={artifacts[-1].version}") + db = _build_and_put("ServiceDB", derive_id(env_id, "db_image"), f"ServiceDB PostgreSQL image for {env_id}", + SERVICE_DB_DOCKERFILE, SERVICE_DB_IMAGE_NAME, platform=platform, say=say) + db_web = _build_and_put("db-web", derive_id(env_id, "db_web_image"), + "db-web lightweight web UI for database inspection", DB_WEB_DOCKERFILE, DB_WEB_IMAGE_NAME, + platform=platform, say=say) + db_mcp = _build_and_put("db-mcp", derive_id(env_id, "db_mcp_image"), + "db-mcp PostgreSQL MCP server for direct DB access", DB_MCP_DOCKERFILE, DB_MCP_IMAGE_NAME, + platform=platform, say=say) say("Creating ServiceDBEnv...") - env = ServiceDBEnv.put( - id=env_id, - db_docker_image_artifact=artifacts[0], - db_web_docker_image_artifact=artifacts[1], - db_mcp_docker_image_artifact=artifacts[2], - metadata=_metadata(SERVICE_DB, SERVICE_DB_DOCKERFILE, SERVICE_DB_DOCKERFILE.parent, metadata), - ) + env = ServiceDBEnv.put(id=env_id, db_docker_image_artifact=db, db_web_docker_image_artifact=db_web, + db_mcp_docker_image_artifact=db_mcp, metadata=_metadata(SERVICE_DB, metadata)) say(f"Created ServiceDBEnv: id={env.id} version={env.version}") return env @@ -103,49 +85,53 @@ def put_website_browser_env(env_id: str, *, platform: str | None, metadata: Mapp say: Callable[[str], None] = _quiet) -> MCPServerEnv: """Build the website browser image for ``platform`` (this host's when None) and write it as the MCP server env ``env_id``.""" - say("Building website browser Docker image...") - build_image(WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_CONTEXT, WEBSITE_BROWSER_IMAGE_TAG, platform=platform) - say("Creating DockerImageArtifact...") - artifact = DockerImageArtifact.put(id=derive_id(env_id, "env_image"), description="Website browser MCP server", - image_name=WEBSITE_BROWSER_IMAGE_TAG, - build_context_path=str(WEBSITE_BROWSER_CONTEXT), - dockerfile_path=WEBSITE_BROWSER_DOCKERFILE.name) - say(f"Created artifact: id={artifact.id} version={artifact.version}") + artifact = _build_and_put("website browser", derive_id(env_id, "env_image"), "Website browser MCP server", + WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_IMAGE_TAG, platform=platform, say=say) say("Creating MCPServerEnv...") env = MCPServerEnv.put(id=env_id, docker_image_artifact=artifact, environment_name=WEBSITE_BROWSER_ENVIRONMENT_NAME, - metadata=_metadata(WEBSITE_BROWSER, WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_CONTEXT, - metadata)) + metadata=_metadata(WEBSITE_BROWSER, metadata)) say(f"Created MCPServerEnv: id={env.id} version={env.version} environment_name={env.environment_name}") return env -def _metadata(kind: str, dockerfile: Path, context: Path, extra: Mapping[str, str] | None) -> dict[str, str]: - return {**detect_env_metadata(dockerfile, context), **(extra or {}), BUILD_INPUTS_KEY: build_inputs_digest(kind)} +def _build_and_put(label: str, artifact_id: str, description: str, dockerfile: Path, image_name: str, *, + platform: str | None, say: Callable[[str], None]) -> DockerImageArtifact: + """Build ``dockerfile``'s image for ``platform`` from the folder it's in, and write it with that folder as its build + context, which a sandbox that can't run its tar.gz builds it from.""" + say(f"Building {label} image from {dockerfile}...") + build_image(dockerfile, dockerfile.parent, image_name, platform=platform) + say(f"Creating {label} DockerImageArtifact...") + artifact = DockerImageArtifact.put(id=artifact_id, description=description, image_name=image_name, + build_context_path=str(dockerfile.parent), dockerfile_path=dockerfile.name) + say(f"Created {label} DockerImageArtifact: id={artifact.id} version={artifact.version}") + return artifact + + +def _metadata(kind: str, extra: Mapping[str, str] | None) -> dict[str, str]: + dockerfile = _DOCKERFILES[kind][0] + return {**detect_env_metadata(dockerfile, dockerfile.parent), **(extra or {}), + BUILD_INPUTS_KEY: build_inputs_digest(kind)} BUILD_INPUTS_KEY = "build_inputs_sha256" # the metadata key a put records build_inputs_digest under -_BUILD_INPUTS = { - GATEWAY: lambda: _files_under(GATEWAY_CONTEXT), - SERVICE_DB: lambda: [SERVICE_DB_DOCKERFILE, DB_WEB_DOCKERFILE, DB_MCP_DOCKERFILE], # they copy nothing - WEBSITE_BROWSER: lambda: [WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_CONTEXT / "entrypoint.sh"], +# Each kind's Dockerfiles, each built from the folder it's in; the first is the one its env records. +_DOCKERFILES = { + GATEWAY: [GATEWAY_DOCKERFILE], + SERVICE_DB: [SERVICE_DB_DOCKERFILE, DB_WEB_DOCKERFILE, DB_MCP_DOCKERFILE], + WEBSITE_BROWSER: [WEBSITE_BROWSER_DOCKERFILE], } def build_inputs_digest(kind: str) -> str: - """A digest of what agent-env builds ``kind``'s images from: the Dockerfiles it ships and the files they copy, - which hold every build arg's value too, so a build from the build context alone builds the same thing. What a - build fetches (base images, packages) isn't an input.""" + """A digest of the build contexts agent-env builds ``kind``'s images from, as each put stores them: the Dockerfiles + it ships and the files beside them. They hold every build arg's value too, so a build from a build context alone + builds the same thing. What a build fetches (base images, packages) isn't an input.""" digest = hashlib.sha256() - for path in sorted(_BUILD_INPUTS[kind]()): - digest.update(path.relative_to(_ENV_PACKAGE).as_posix().encode() + b"\0" + path.read_bytes() + b"\0") + for dockerfile in _DOCKERFILES[kind]: + digest.update(BuildContext.of(dockerfile.parent, dockerfile).source_digest(None).encode() + b"\0") return digest.hexdigest() -def _files_under(folder: Path) -> list[Path]: - return [path for path in folder.rglob("*") - if path.is_file() and "__pycache__" not in path.parts and path.suffix != ".pyc"] - - _PUTS = {GATEWAY: put_gateway_env, SERVICE_DB: put_service_db_env, WEBSITE_BROWSER: put_website_browser_env} @@ -208,6 +194,11 @@ def infra_to_build(kinds: Iterable[str]) -> list[InfraBuild]: return builds +def needs_docker(builds: Iterable[InfraBuild], reason: str) -> str: + """Why ``builds`` can't be built here, given why docker can't be used (``docker_unreachable``).""" + return f"building the {', '.join(build.kind for build in builds)} env needs docker, and {reason}" + + def ensure_default_envs(kinds: Iterable[str], *, say: Callable[[str], None] = _quiet) -> list[InfraBuild]: """Build the infra envs of ``kinds`` that ``infra_to_build`` names, for this host's platform, and return them. Each id is locked while it's checked and built, so concurrent runs build it once. Raises InfraError before building @@ -219,7 +210,7 @@ def ensure_default_envs(kinds: Iterable[str], *, say: Callable[[str], None] = _q with holding_locks(ids, on_wait=lambda: say("waiting for another agent-env run to finish building the infra envs")): builds = infra_to_build(kinds) if builds and (reason := docker_unreachable()): - raise InfraError([f"building the {', '.join(build.kind for build in builds)} env needs docker, and {reason}"]) + raise InfraError([needs_docker(builds, reason)]) for build in builds: say(f"{build}: building, which can take minutes") _PUTS[build.kind](build.id, platform=None) @@ -234,14 +225,11 @@ def stores_are_local() -> bool: and isinstance(configured_store(config.get_image_store()), LocalRegistryImageStore)) -_STOCK_DOCKERFILES = {GATEWAY: GATEWAY_DOCKERFILE, SERVICE_DB: SERVICE_DB_DOCKERFILE, WEBSITE_BROWSER: WEBSITE_BROWSER_DOCKERFILE} - - def _built_from_stock(env: Env, kind: str) -> bool: """Whether ``env`` was built from the Dockerfile agent-env ships for ``kind``, in this install or another one, as the put commands record it; an env someone built from their own Dockerfile is theirs to rebuild.""" recorded = (env.metadata or {}).get("dockerfile_path") - shipped = _STOCK_DOCKERFILES[kind].relative_to(_ENV_PACKAGE.parent.parent).parts # ("agent_env", "env", ...) + shipped = _DOCKERFILES[kind][0].relative_to(_ENV_PACKAGE.parent.parent).parts # ("agent_env", "env", ...) return recorded is not None and Path(recorded).parts[-len(shipped):] == shipped diff --git a/src/agent_env/env/envs/website_browser/Dockerfile b/src/agent_env/env/envs/website_browser/Dockerfile index ea1f7d49..83b24f35 100644 --- a/src/agent_env/env/envs/website_browser/Dockerfile +++ b/src/agent_env/env/envs/website_browser/Dockerfile @@ -28,6 +28,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ WORKDIR /app +# The pinned @playwright/mcp version; update it here when upgrading the Playwright MCP server. ARG PLAYWRIGHT_MCP_VERSION=0.0.41 ENV PLAYWRIGHT_MCP_VERSION=${PLAYWRIGHT_MCP_VERSION} diff --git a/src/agent_env/env/envs/website_browser/__init__.py b/src/agent_env/env/envs/website_browser/__init__.py index b0c40000..ef99d7b1 100644 --- a/src/agent_env/env/envs/website_browser/__init__.py +++ b/src/agent_env/env/envs/website_browser/__init__.py @@ -1,9 +1,5 @@ """Website browser (Playwright) MCP server constants.""" -# Pinned version of @playwright/mcp to install in the Docker image. -# Update this when upgrading the Playwright MCP server. -PLAYWRIGHT_MCP_VERSION = "0.0.41" - # Docker image tag for the website browser server image. WEBSITE_BROWSER_IMAGE_TAG = "mcp-website-browser" @@ -11,7 +7,6 @@ WEBSITE_BROWSER_ENVIRONMENT_NAME = "website-browser" __all__ = [ - "PLAYWRIGHT_MCP_VERSION", "WEBSITE_BROWSER_IMAGE_TAG", "WEBSITE_BROWSER_ENVIRONMENT_NAME", ] diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index a485dc48..9c5bee77 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -247,9 +247,8 @@ async def prepare_image(self, image: DockerImageArtifact, *, attribution: Option """Build ``image`` from its build context when Modal can't run it by name: it's only a build context, its name is in a registry Modal can't reach, or it's built for another platform than Modal's; unless this process has already built its sources. One it pulls drops an image prepared earlier under its name.""" - if image.build_context_object_url and ( - image.context_only or image.by_name_problem(on_this_machine=self.ON_THIS_MACHINE) - or image.platform_problem(self._sandbox_cls.platform)): + if image.build_context_object_url and (image.by_name_problem(on_this_machine=self.ON_THIS_MACHINE) + or image.platform_problem(self._sandbox_cls.platform)): self._context_images[image.image_name] = image await self._context_image(image) else: diff --git a/tst/unit/env/bootstrap_test.py b/tst/unit/env/bootstrap_test.py index 18bf996a..d21136dc 100644 --- a/tst/unit/env/bootstrap_test.py +++ b/tst/unit/env/bootstrap_test.py @@ -13,7 +13,6 @@ from agent_env.env import GatewayEnv from agent_env.env import bootstrap from agent_env.env.bootstrap import GATEWAY, SERVICE_DB, WEBSITE_BROWSER, InfraError, ensure_default_envs -from agent_env.env.envs.website_browser import PLAYWRIGHT_MCP_VERSION @pytest.fixture @@ -26,7 +25,7 @@ def write(env_id, *, platform): calls.append((kind, env_id, platform)) artifact = get_artifact_store().put_document(DockerImageArtifact( id=f"{kind}-{env_id}", description=kind, image_name=f"{kind}:v1", tar_gz_s3_url="file:///x.tar.gz")) - shipped = bootstrap._STOCK_DOCKERFILES[kind].relative_to(bootstrap._ENV_PACKAGE.parent.parent) + shipped = bootstrap._DOCKERFILES[kind][0].relative_to(bootstrap._ENV_PACKAGE.parent.parent) GatewayEnv.put(id=env_id, docker_image_artifact=artifact, metadata={ "dockerfile_path": f"/another/install/site-packages/{shipped}", bootstrap.BUILD_INPUTS_KEY: bootstrap.build_inputs_digest(kind)}) @@ -37,14 +36,15 @@ def write(env_id, *, platform): return calls -@pytest.mark.parametrize("put, contexts", [ - (bootstrap.put_gateway_env, [(bootstrap.GATEWAY_CONTEXT, "Dockerfile")]), - (bootstrap.put_service_db_env, [(bootstrap.SERVICE_DB_DOCKERFILE.parent, name) - for name in ("Dockerfile", "Dockerfile.db-web", "Dockerfile.db-mcp")]), - (bootstrap.put_website_browser_env, [(bootstrap.WEBSITE_BROWSER_CONTEXT, "Dockerfile")]), +@pytest.mark.parametrize("kind, put, contexts", [ + (GATEWAY, bootstrap.put_gateway_env, [(bootstrap.GATEWAY_CONTEXT, "Dockerfile")]), + (SERVICE_DB, bootstrap.put_service_db_env, [(bootstrap.SERVICE_DB_DOCKERFILE.parent, name) + for name in ("Dockerfile", "Dockerfile.db-web", "Dockerfile.db-mcp")]), + (WEBSITE_BROWSER, bootstrap.put_website_browser_env, [(bootstrap.WEBSITE_BROWSER_CONTEXT, "Dockerfile")]), ], ids=[GATEWAY, SERVICE_DB, WEBSITE_BROWSER]) -def test_each_infra_image_keeps_the_build_context_it_is_built_from(local_stores, monkeypatch, put, contexts): - """So a VM or Modal running another platform than this machine builds it again from that.""" +def test_each_infra_image_keeps_the_build_context_it_is_built_from(local_stores, monkeypatch, kind, put, contexts): + """So a VM or Modal running another platform than this machine builds it again from that; and those contexts are + what its recorded build inputs cover.""" kept = [] def keep(id, *, description, image_name, build_context_path=None, dockerfile_path=None, platform=None): @@ -58,6 +58,7 @@ def keep(id, *, description, image_name, build_context_path=None, dockerfile_pat put("x", platform=None) assert kept == contexts + assert [folder / name for folder, name in kept] == bootstrap._DOCKERFILES[kind] def test_missing_infra_is_built_for_this_host_in_order_and_once(puts): @@ -193,14 +194,14 @@ def test_a_put_records_what_the_env_was_built_from_whatever_metadata_it_was_give assert env.metadata["owner"] == "me" -def test_the_build_inputs_are_the_shipped_files_less_python_caches(tmp_path): - (tmp_path / "__pycache__").mkdir() - (tmp_path / "__pycache__/gateway.cpython-312.pyc").write_text("compiled") +def test_the_build_inputs_are_every_file_in_the_build_context_less_python_caches(tmp_path, monkeypatch): + (tmp_path / "Dockerfile").write_text("FROM python:3.12\nCOPY . /app\n") (tmp_path / "gateway.py").write_text("served") + monkeypatch.setitem(bootstrap._DOCKERFILES, GATEWAY, [tmp_path / "Dockerfile"]) + before = bootstrap.build_inputs_digest(GATEWAY) - assert bootstrap._files_under(tmp_path) == [tmp_path / "gateway.py"] - - -def test_the_website_browser_dockerfile_installs_the_playwright_mcp_version_agent_env_names(): - """A build from its build context alone, which carries no build args, installs that version too.""" - assert f"ARG PLAYWRIGHT_MCP_VERSION={PLAYWRIGHT_MCP_VERSION}\n" in bootstrap.WEBSITE_BROWSER_DOCKERFILE.read_text() + (tmp_path / "__pycache__").mkdir() + (tmp_path / "__pycache__/gateway.cpython-312.pyc").write_text("compiled") + assert bootstrap.build_inputs_digest(GATEWAY) == before + (tmp_path / "gateway.py").write_text("served, changed") + assert bootstrap.build_inputs_digest(GATEWAY) != before From 585691fdcc45041106df24d1d6364de08b525f87 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sun, 11 Oct 2026 23:41:12 -0700 Subject: [PATCH 4/4] test: a VM builds images from their build contexts on real Docker; infra Dockerfiles build on Docker's legacy builder - Slow integration tier, local backend: the local sandbox, given this machine's platform as a remote VM records its own, builds an image that is only a build context, and rebuilds one whose tar.gz records another platform from the context beside it (its tar.gz points nowhere, so loading it would fail the deploy). Each serves its card through the gateway. The first integration coverage of a sandbox building from a build context. - Unit: every shipped infra Dockerfile builds from its context alone on Docker's legacy builder, which a VM with no buildx uses: no BuildKit-only syntax, no build arg without a default. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../env/envs/build_context_local_test.py | 98 +++++++++++++++++++ tst/unit/env/bootstrap_test.py | 40 ++++++++ 2 files changed, 138 insertions(+) create mode 100644 tst/integration/env/envs/build_context_local_test.py diff --git a/tst/integration/env/envs/build_context_local_test.py b/tst/integration/env/envs/build_context_local_test.py new file mode 100644 index 00000000..8b6cffde --- /dev/null +++ b/tst/integration/env/envs/build_context_local_test.py @@ -0,0 +1,98 @@ +"""Images a VM sandbox builds from their build contexts, on the local backend's, which runs this machine's Docker: one +that is only a build context, and one whose tar.gz is built for another platform than the VM's, which the VM builds from +the context beside it instead of loading. Each deploys an MCP server env through the gateway and reads its server's +card. Requires a docker daemon; spins up a throwaway ``registry:2`` and skips if it can't start. +""" + +import logging +import shutil +import uuid +from pathlib import Path + +import pytest +from agentenv_protocol import client as protocol_v1 + +from agent_env.artifact import DockerImageArtifact +from agent_env.artifact.store import get_artifact_store +from agent_env.env import legacy_protocol +from agent_env.env.envs.mcp_server import MCPServerEnv +from agent_env.providers.sandbox_providers import sandbox +from agent_env.providers.sandbox_providers.local_sandbox import LocalSandbox +from agent_env.utils.docker_build import normalize_platform +from tst.integration.env.envs.server_env_local_test import local_stack # noqa: F401 (fixture) +from tst.integration.store.local_parallel_deploy_test import _docker, _seed_bootstrap_envs + +pytestmark = [pytest.mark.integration, pytest.mark.int_test_slow] + +_REPO = Path(__file__).resolve().parents[4] +_OTHER_PLATFORM = {"linux/amd64": "linux/arm64", "linux/arm64": "linux/amd64"} + + +@pytest.fixture +def vm_platform(monkeypatch, caplog): + """The local sandbox records no platform, its Docker being this machine's; given this machine's, as a remote VM + records its own, it rebuilds an image recorded for another one as a remote VM does.""" + platform = normalize_platform(f"linux/{_docker('info', '--format', '{{.Architecture}}').stdout.strip()}") + if platform not in _OTHER_PLATFORM: + pytest.skip(f"no other platform to record for a {platform} host") + monkeypatch.setattr(LocalSandbox, "platform", platform) + caplog.set_level(logging.INFO, logger=sandbox.__name__) + return platform + + +@pytest.mark.asyncio +async def test_an_image_that_is_only_a_build_context_is_built_on_the_vm(local_stack, vm_platform, tmp_path, caplog): + _seed_bootstrap_envs() + uid = uuid.uuid4().hex[:8] + image = DockerImageArtifact.put_context(f"ctx-items-{uid}", description="build context e2e", + context_path=str(_items_context(tmp_path / "items")), + dockerfile_path="Dockerfile", platform=vm_platform) + + await _deploys_and_answers(image, uid) + + assert f"Building {image.image_name} from its build context, for {vm_platform}..." in caplog.messages + + +@pytest.mark.asyncio +async def test_an_image_whose_tar_gz_is_for_another_platform_is_built_from_the_context_beside_it( + local_stack, vm_platform, tmp_path, caplog): + _seed_bootstrap_envs() + uid = uuid.uuid4().hex[:8] + context = _items_context(tmp_path / "items") + tag = f"ctx-items-{uid}" + build = _docker("build", "-t", tag, str(context)) + assert build.returncode == 0, f"items build failed: {build.stderr[-2000:]}" + stored = DockerImageArtifact.put(id=tag, description="build context e2e", image_name=tag, + build_context_path=str(context), dockerfile_path="Dockerfile") + # As a machine of the other platform would have written it, with a tar.gz a VM that loaded it would fail on. + image = get_artifact_store().put_document(stored.model_copy(update={ + "id": f"{tag}-other", "version": 0, "platform": _OTHER_PLATFORM[vm_platform], + "tar_gz_object_url": "file:///nonexistent/items.tar.gz"})) + assert image.rebuilt_for(vm_platform) + + await _deploys_and_answers(image, uid) + + assert f"Building {image.image_name} from its build context, for {vm_platform}..." in caplog.messages + + +async def _deploys_and_answers(image: DockerImageArtifact, uid: str) -> None: + """Deploy an items server env of ``image`` through the gateway on the local sandbox, read its card, and close it.""" + env = MCPServerEnv.put(id=f"ctx-items-env-{uid}", docker_image_artifact=image, environment_name="items") + deployed = await env.deploy(sandbox_type="local", ttl_seconds=900) + work_dir = env._sandbox.work_dir + try: + base_url = await legacy_protocol.v1_base_url(deployed, deployed.gateway_url, "items") + assert (await protocol_v1.get_card(base_url))["name"] == "items" + await env.close() + finally: + _docker("compose", "--project-directory", str(work_dir), "down", "-v", "--remove-orphans") + + +def _items_context(root: Path) -> Path: + """The in-memory items server's build context (card 'items'), with the vendored agentenv_protocol.""" + data = _REPO / "tst" / "data" / "agentenv_mcp" + shutil.copytree(_REPO / "packages" / "agentenv-protocol" / "src" / "agentenv_protocol", root / "agentenv_protocol", + ignore=shutil.ignore_patterns("__pycache__")) + for name in ("server.py", "Dockerfile", "seed.json"): + shutil.copy(data / name, root / name) + return root diff --git a/tst/unit/env/bootstrap_test.py b/tst/unit/env/bootstrap_test.py index d21136dc..4f557f91 100644 --- a/tst/unit/env/bootstrap_test.py +++ b/tst/unit/env/bootstrap_test.py @@ -2,6 +2,7 @@ stores only, for this host's platform, once each however many runs ask.""" import importlib +import re from pathlib import Path import pytest @@ -205,3 +206,42 @@ def test_the_build_inputs_are_every_file_in_the_build_context_less_python_caches assert bootstrap.build_inputs_digest(GATEWAY) == before (tmp_path / "gateway.py").write_text("served, changed") assert bootstrap.build_inputs_digest(GATEWAY) != before + + +# What Docker's legacy builder refuses, by what it is. +_BUILDKIT_ONLY = { + "a syntax directive": r"^\s*#\s*syntax\s*=", + "RUN --mount, --network or --security": r"^\s*RUN\s+(--\S+\s+)*--(mount|network|security)\b", + "COPY or ADD --chmod, --link, --parents, --exclude, --checksum or --keep-git-dir": + r"^\s*(COPY|ADD)\s+(--\S+\s+)*--(chmod|link|parents|exclude|checksum|keep-git-dir)\b", + "a heredoc": r"<<-?\s*[\"']?\w+", + "a build arg with no default": r"^\s*ARG\s+\w+\s*$", +} + + +def _legacy_build_problems(dockerfile: str) -> list[str]: + return [what for what, pattern in _BUILDKIT_ONLY.items() + if re.search(pattern, dockerfile, re.IGNORECASE | re.MULTILINE)] + + +@pytest.mark.parametrize("dockerfile", [dockerfile for dockerfiles in bootstrap._DOCKERFILES.values() + for dockerfile in dockerfiles], + ids=lambda dockerfile: dockerfile.relative_to(bootstrap._ENV_PACKAGE).as_posix()) +def test_each_infra_image_builds_from_its_context_alone_on_dockers_legacy_builder(dockerfile): + """A sandbox rebuilds an infra image from its build context, which carries no build args, and a VM with no buildx, + as modal_vm's has none, builds with Docker's legacy builder.""" + assert _legacy_build_problems(dockerfile.read_text()) == [] + + +@pytest.mark.parametrize("line, problem", [ + ("# syntax=docker/dockerfile:1", "a syntax directive"), + ("RUN --mount=type=cache,target=/root/.npm npm ci", "RUN --mount, --network or --security"), + ("copy --chown=1:1 --chmod=755 entrypoint.sh /app/", + "COPY or ADD --chmod, --link, --parents, --exclude, --checksum or --keep-git-dir"), + ("RUN <