diff --git a/src/agent_env/cli/__init__.py b/src/agent_env/cli/__init__.py index 31d72126..e39688d4 100644 --- a/src/agent_env/cli/__init__.py +++ b/src/agent_env/cli/__init__.py @@ -5,6 +5,7 @@ from agent_env.plugins._cli import load_cli_plugins, load_cli_root_options from agent_env.store.base import NotFoundError from agent_env.store.routing import namespace_routing +from agent_env.utils.docker_build import DockerBuildError from .a2a_agent import a2a_agent from .artifact import artifact @@ -17,11 +18,11 @@ from .up import up -# Errors that mean "not allowed" or "not there" rather than "agent-env is broken". Spelled out -# because there is no domain base class to catch: ConfigError subclasses ValueError (so does -# pydantic's ValidationError), while NotFoundError subclasses Exception. A TypeError or a +# Errors that mean "not allowed", "not there" or "your build failed" rather than "agent-env is broken". +# Spelled out because there is no domain base class to catch: ConfigError subclasses ValueError (so does +# pydantic's ValidationError), while NotFoundError and DockerBuildError don't. A TypeError or a # retry-exhausted DuplicateKeyError is a defect, and a defect keeps its traceback. -_USER_FACING_ERRORS = (ValueError, NotFoundError) +_USER_FACING_ERRORS = (ValueError, NotFoundError, DockerBuildError) class _UserErrorsAreNotCrashes(click.Group): diff --git a/src/agent_env/cli/a2a_agent/put.py b/src/agent_env/cli/a2a_agent/put.py index 1900f585..bc6777e9 100644 --- a/src/agent_env/cli/a2a_agent/put.py +++ b/src/agent_env/cli/a2a_agent/put.py @@ -1,4 +1,3 @@ -import subprocess import sys from pathlib import Path @@ -6,7 +5,8 @@ from agent_env.a2a_agent import A2AAgent from agent_env.artifact import DockerImageArtifact -from agent_env.cli.utils import build_platform_option, detect_env_metadata, docker_build_platform_args, skips_local_validation +from agent_env.cli.utils import build_platform_option, detect_env_metadata, skips_local_validation +from agent_env.utils.docker_build import build_image @click.command() @@ -44,15 +44,7 @@ def put(agent_id: str, dockerfile: str, context_path: str | None, env_var_pairs: image_tag = f"a2a-agent-{agent_id}" click.echo(f"Building Docker image...") - result = subprocess.run( - ["docker", "build", *docker_build_platform_args(build_platform), - "-f", str(dockerfile_path), "-t", image_tag, str(context)], - capture_output=True, - text=True, - ) - if result.returncode != 0: - click.echo(f"Docker build failed: {result.stderr}", err=True) - sys.exit(1) + build_image(dockerfile_path, context, image_tag, platform=build_platform) click.echo(f"Creating DockerImageArtifact...") artifact = DockerImageArtifact.put( diff --git a/src/agent_env/cli/env/gateway.py b/src/agent_env/cli/env/gateway.py index e4a37c4f..c9f06356 100644 --- a/src/agent_env/cli/env/gateway.py +++ b/src/agent_env/cli/env/gateway.py @@ -1,12 +1,12 @@ -import subprocess import sys from pathlib import Path import click from agent_env.artifact import DockerImageArtifact -from agent_env.cli.utils import build_platform_option, detect_env_metadata, docker_build_platform_args +from agent_env.cli.utils import build_platform_option, detect_env_metadata from agent_env.env import GatewayEnv +from agent_env.utils.docker_build import build_image _PACKAGE_ROOT = Path(__file__).parent.parent.parent GATEWAY_DOCKERFILE = _PACKAGE_ROOT / "env" / "gateway" / "Dockerfile" @@ -28,15 +28,7 @@ def put(env_id: str, metadata_pairs: tuple[str, ...], build_platform: str): """Build and upload a gateway environment.""" click.echo(f"Building gateway Docker image...") - result = subprocess.run( - ["docker", "build", *docker_build_platform_args(build_platform), - "-f", str(GATEWAY_DOCKERFILE), "-t", GATEWAY_IMAGE_TAG, str(GATEWAY_CONTEXT)], - capture_output=True, - text=True, - ) - if result.returncode != 0: - click.echo(f"Docker build failed: {result.stderr}", err=True) - sys.exit(1) + build_image(GATEWAY_DOCKERFILE, GATEWAY_CONTEXT, GATEWAY_IMAGE_TAG, platform=build_platform) click.echo(f"Creating DockerImageArtifact...") artifact = DockerImageArtifact.put( diff --git a/src/agent_env/cli/env/mcp_server.py b/src/agent_env/cli/env/mcp_server.py index e1844526..8030855c 100644 --- a/src/agent_env/cli/env/mcp_server.py +++ b/src/agent_env/cli/env/mcp_server.py @@ -1,6 +1,5 @@ import asyncio import os -import subprocess import sys from pathlib import Path @@ -8,17 +7,16 @@ from agent_env.artifact import DockerImageArtifact, EnvironmentArtifact from agent_env.cli.utils import ( - DEFAULT_BUILD_PLATFORM, deployed_env_from_instance, build_platform_option, detect_env_metadata, - docker_build_platform_args, env_provider_type_option, environment_name_options, resolve_environment_name, skips_local_validation, ) from agent_env.utils.card_naming import card_name_from_github, card_name_from_source +from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM, build_image from agent_env.env import Env, MCPServerEnv @@ -194,15 +192,7 @@ def _on_progress(step: str, message: str, percent: int) -> None: image_tag = f"mcp-server-{env_id}" click.echo(f"Building MCP server Docker image...") - result = subprocess.run( - ["docker", "build", *docker_build_platform_args(build_platform), - "-f", str(dockerfile_path), "-t", image_tag, str(context)], - capture_output=True, - text=True, - ) - if result.returncode != 0: - click.echo(f"Docker build failed: {result.stderr}", err=True) - sys.exit(1) + build_image(dockerfile_path, context, image_tag, platform=build_platform) click.echo(f"Creating DockerImageArtifact...") artifact = DockerImageArtifact.put( diff --git a/src/agent_env/cli/env/service_db.py b/src/agent_env/cli/env/service_db.py index 4d0cec95..121e0590 100644 --- a/src/agent_env/cli/env/service_db.py +++ b/src/agent_env/cli/env/service_db.py @@ -1,14 +1,14 @@ """CLI commands for ServiceDBEnv.""" -import subprocess from pathlib import Path import click from agent_env.artifact import DockerImageArtifact -from agent_env.cli.utils import build_platform_option, detect_env_metadata, docker_build_platform_args +from agent_env.cli.utils import build_platform_option, detect_env_metadata from agent_env.env.envs.service_db import ServiceDBEnv from agent_env.config import get_config +from agent_env.utils.docker_build import build_image # Path to ServiceDB Dockerfile SERVICE_DB_DOCKERFILE = Path(__file__).parent.parent.parent / "env" / "envs" / "service_db" / "Dockerfile" @@ -37,17 +37,7 @@ def put(env_id: str, metadata_pairs: tuple[str, ...], build_platform: str): click.echo(f"Building ServiceDB image from {SERVICE_DB_DOCKERFILE}...") # Build Docker image - result = subprocess.run( - ["docker", "build", *docker_build_platform_args(build_platform), - "-f", str(SERVICE_DB_DOCKERFILE), - "-t", SERVICE_DB_IMAGE_NAME, - str(SERVICE_DB_DOCKERFILE.parent)], - capture_output=True, - text=True, - ) - if result.returncode != 0: - click.echo(f"Docker build failed: {result.stderr}", err=True) - raise click.Abort() + build_image(SERVICE_DB_DOCKERFILE, SERVICE_DB_DOCKERFILE.parent, SERVICE_DB_IMAGE_NAME, platform=build_platform) click.echo("Docker build successful") # Create DB DockerImageArtifact @@ -61,17 +51,7 @@ def put(env_id: str, metadata_pairs: tuple[str, ...], build_platform: str): # Build db-web image (build instead of pull to avoid docker save manifest issues on Apple Silicon) click.echo(f"Building db-web image from {DB_WEB_DOCKERFILE}...") - result = subprocess.run( - ["docker", "build", *docker_build_platform_args(build_platform), - "-f", str(DB_WEB_DOCKERFILE), - "-t", DB_WEB_IMAGE_NAME, - str(DB_WEB_DOCKERFILE.parent)], - capture_output=True, - text=True, - ) - if result.returncode != 0: - click.echo(f"db-web build failed: {result.stderr}", err=True) - raise click.Abort() + build_image(DB_WEB_DOCKERFILE, DB_WEB_DOCKERFILE.parent, DB_WEB_IMAGE_NAME, platform=build_platform) click.echo("db-web build successful") # Create db-web DockerImageArtifact @@ -85,17 +65,7 @@ def put(env_id: str, metadata_pairs: tuple[str, ...], build_platform: str): # Build db-mcp image (PostgreSQL MCP server for direct DB access) click.echo(f"Building db-mcp image from {DB_MCP_DOCKERFILE}...") - result = subprocess.run( - ["docker", "build", *docker_build_platform_args(build_platform), - "-f", str(DB_MCP_DOCKERFILE), - "-t", DB_MCP_IMAGE_NAME, - str(DB_MCP_DOCKERFILE.parent)], - capture_output=True, - text=True, - ) - if result.returncode != 0: - click.echo(f"db-mcp build failed: {result.stderr}", err=True) - raise click.Abort() + build_image(DB_MCP_DOCKERFILE, DB_MCP_DOCKERFILE.parent, DB_MCP_IMAGE_NAME, platform=build_platform) click.echo("db-mcp build successful") # Create db-mcp DockerImageArtifact diff --git a/src/agent_env/cli/env/website.py b/src/agent_env/cli/env/website.py index 1fd5be10..49bb4d1c 100644 --- a/src/agent_env/cli/env/website.py +++ b/src/agent_env/cli/env/website.py @@ -1,6 +1,5 @@ import asyncio import os -import subprocess import sys from pathlib import Path @@ -8,17 +7,16 @@ from agent_env.artifact import DockerImageArtifact, EnvironmentArtifact from agent_env.cli.utils import ( - DEFAULT_BUILD_PLATFORM, deployed_env_from_instance, build_platform_option, detect_env_metadata, - docker_build_platform_args, env_provider_type_option, environment_name_options, resolve_environment_name, skips_local_validation, ) from agent_env.utils.card_naming import card_name_from_github, card_name_from_source +from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM, build_image from agent_env.env import Env from agent_env.env.envs.website import WebsiteEnv from agent_env.providers import get_env_sandbox_provider @@ -152,15 +150,7 @@ def _frontend_progress(step: str, message: str, percent: int) -> None: click.echo(f"Derived environment_name={environment_name!r} from the environment card.") click.echo(f"Building website backend Docker image...") - result = subprocess.run( - ["docker", "build", *docker_build_platform_args(build_platform), - "-f", str(backend_dockerfile_path), "-t", backend_tag, str(backend_ctx)], - capture_output=True, - text=True, - ) - if result.returncode != 0: - click.echo(f"Backend Docker build failed: {result.stderr}", err=True) - sys.exit(1) + build_image(backend_dockerfile_path, backend_ctx, backend_tag, platform=build_platform) click.echo(f"Creating backend DockerImageArtifact...") backend_artifact = DockerImageArtifact.put( @@ -175,15 +165,7 @@ def _frontend_progress(step: str, message: str, percent: int) -> None: frontend_tag = f"website-frontend-{env_id}" click.echo(f"Building website frontend Docker image...") - result = subprocess.run( - ["docker", "build", *docker_build_platform_args(build_platform), - "-f", str(frontend_dockerfile_path), "-t", frontend_tag, str(frontend_ctx)], - capture_output=True, - text=True, - ) - if result.returncode != 0: - click.echo(f"Frontend Docker build failed: {result.stderr}", err=True) - sys.exit(1) + build_image(frontend_dockerfile_path, frontend_ctx, frontend_tag, platform=build_platform) click.echo(f"Creating frontend DockerImageArtifact...") frontend_artifact = DockerImageArtifact.put( diff --git a/src/agent_env/cli/env/website_browser.py b/src/agent_env/cli/env/website_browser.py index 84339d21..59f74c46 100644 --- a/src/agent_env/cli/env/website_browser.py +++ b/src/agent_env/cli/env/website_browser.py @@ -1,17 +1,17 @@ -import subprocess import sys from pathlib import Path import click from agent_env.artifact import DockerImageArtifact -from agent_env.cli.utils import build_platform_option, detect_env_metadata, docker_build_platform_args +from agent_env.cli.utils import build_platform_option, detect_env_metadata from agent_env.env import MCPServerEnv from agent_env.env.envs.website_browser import ( PLAYWRIGHT_MCP_VERSION, WEBSITE_BROWSER_IMAGE_TAG, WEBSITE_BROWSER_ENVIRONMENT_NAME, ) +from agent_env.utils.docker_build import build_image _PACKAGE_ROOT = Path(__file__).parent.parent.parent WEBSITE_BROWSER_DOCKERFILE = _PACKAGE_ROOT / "env" / "envs" / "website_browser" / "Dockerfile" @@ -36,16 +36,8 @@ def put(env_id: str, metadata_pairs: tuple[str, ...], build_platform: str): env_id = get_config().default_website_browser_env_id click.echo("Building website browser Docker image...") - result = subprocess.run( - ["docker", "build", *docker_build_platform_args(build_platform), - "--build-arg", f"PLAYWRIGHT_MCP_VERSION={PLAYWRIGHT_MCP_VERSION}", - "-f", str(WEBSITE_BROWSER_DOCKERFILE), "-t", WEBSITE_BROWSER_IMAGE_TAG, str(WEBSITE_BROWSER_CONTEXT)], - capture_output=True, - text=True, - ) - if result.returncode != 0: - click.echo(f"Docker build failed: {result.stderr}", err=True) - sys.exit(1) + build_image(WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_CONTEXT, WEBSITE_BROWSER_IMAGE_TAG, platform=build_platform, + build_args={"PLAYWRIGHT_MCP_VERSION": PLAYWRIGHT_MCP_VERSION}) click.echo("Creating DockerImageArtifact...") artifact = DockerImageArtifact.put( diff --git a/src/agent_env/cli/utils.py b/src/agent_env/cli/utils.py index c6c06f10..47aeffc4 100644 --- a/src/agent_env/cli/utils.py +++ b/src/agent_env/cli/utils.py @@ -10,8 +10,7 @@ from agent_env.providers.env_providers.env_server_provider import EnvironmentServerProvider from agent_env.store.base import NotFoundError from agent_env.store.ids import is_local_id - -DEFAULT_BUILD_PLATFORM = "linux/amd64" +from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM def build_platform_option(f): @@ -35,11 +34,6 @@ def build_platform_option(f): )(f) -def docker_build_platform_args(platform: str | None) -> list[str]: - """['--platform', ] when a platform is set, else [] (host-native build).""" - return ["--platform", platform] if platform else [] - - def env_provider_type_option(help: str, env_type: str = "mcp_server"): """Shared `--env-provider-type` option (default `gateway`), refused at parse time unless an installed provider has that type, and for an env other than one MCP server, unless that provider deploys more than one.""" diff --git a/src/agent_env/utils/docker_build.py b/src/agent_env/utils/docker_build.py new file mode 100644 index 00000000..44123687 --- /dev/null +++ b/src/agent_env/utils/docker_build.py @@ -0,0 +1,29 @@ +"""Build a Docker image on this machine, the one way every put command and bundle writer does it.""" + +from __future__ import annotations + +import subprocess +from collections.abc import Mapping +from pathlib import Path + +DEFAULT_BUILD_PLATFORM = "linux/amd64" # the remote sandbox VMs + + +class DockerBuildError(RuntimeError): + """A local ``docker build`` that failed or couldn't start; the message carries docker's output.""" + + +def build_image(dockerfile: Path, context: Path, tag: str, *, platform: str | None, + build_args: Mapping[str, str] | None = None) -> None: + """Build ``context`` with ``dockerfile`` into the local image ``tag``, for ``platform``, or for this host's + platform when it's None or empty. Raises DockerBuildError, with the build's output, when it fails.""" + argv = ["docker", "build", *(["--platform", platform] if platform else [])] + for name, value in (build_args or {}).items(): + argv += ["--build-arg", f"{name}={value}"] + argv += ["-f", str(dockerfile), "-t", tag, str(context)] + try: + result = subprocess.run(argv, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, errors="replace") + except FileNotFoundError as e: + raise DockerBuildError(f"docker build of {tag} failed: docker is not on PATH") from e + if result.returncode != 0: + raise DockerBuildError(f"docker build of {tag} failed (exit {result.returncode}):\n{result.stdout.rstrip()}") diff --git a/tst/unit/cli/build_platform_test.py b/tst/unit/cli/build_platform_test.py index 50b6e7b3..bbbc4cd3 100644 --- a/tst/unit/cli/build_platform_test.py +++ b/tst/unit/cli/build_platform_test.py @@ -1,30 +1,98 @@ -"""Tests for the shared docker build --platform CLI option.""" +"""Each put command builds through ``build_image`` with its ``--platform``, and a build that fails is reported +as one line, not a traceback.""" +import importlib +from unittest.mock import call, patch + +import pytest from click.testing import CliRunner -from agent_env.cli.utils import DEFAULT_BUILD_PLATFORM, docker_build_platform_args +from agent_env.cli import cli +from agent_env.env.envs.website_browser import PLAYWRIGHT_MCP_VERSION, 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 +service_db = importlib.import_module("agent_env.cli.env.service_db") +website_browser = importlib.import_module("agent_env.cli.env.website_browser") -def test_default_platform_preserves_amd64(): - # Backward compatibility: unchanged callers still build amd64. - assert DEFAULT_BUILD_PLATFORM == "linux/amd64" - assert docker_build_platform_args(DEFAULT_BUILD_PLATFORM) == ["--platform", "linux/amd64"] +def test_platform_option_wired_on_mcp_server_put(): + result = CliRunner().invoke(cli, ["env", "mcp-server", "put", "--help"]) + assert result.exit_code == 0 + assert "--platform" in result.output + assert "linux/amd64" in result.output # default shown -def test_explicit_arm64_platform(): - assert docker_build_platform_args("linux/arm64") == ["--platform", "linux/arm64"] +def _dockerfile(folder, name="Dockerfile"): + folder.mkdir(exist_ok=True) + path = folder / name + path.write_text("FROM scratch\n") + return path -def test_empty_platform_builds_host_native(): - # Empty / None omits the flag entirely so docker uses the host platform. - assert docker_build_platform_args("") == [] - assert docker_build_platform_args(None) == [] +def _put_commands(tmp_path): + """Each put command, and the first build it makes: its Dockerfile, context and tag, and its build args.""" + dockerfile = _dockerfile(tmp_path) + backend, frontend = _dockerfile(tmp_path, "backend.Dockerfile"), _dockerfile(tmp_path, "frontend.Dockerfile") + return { + "a2a-agent": ("agent_env.cli.a2a_agent.put", ["a2a-agent", "put", "--id", "x", "--dockerfile", str(dockerfile)], + (dockerfile, tmp_path, "a2a-agent-x"), None), + "mcp-server": ("agent_env.cli.env.mcp_server", + ["env", "mcp-server", "put", "--id", "x", "--environment-name", "items", "--dockerfile", + str(dockerfile)], (dockerfile, tmp_path, "mcp-server-x"), None), + "website": ("agent_env.cli.env.website", + ["env", "website", "put", "--id", "x", "--environment-name", "shop", "--backend-dockerfile", str(backend), + "--frontend-dockerfile", str(frontend)], (backend, tmp_path, "website-backend-x"), None), + "gateway": ("agent_env.cli.env.gateway", ["env", "gateway", "put", "--id", "x"], + (gateway.GATEWAY_DOCKERFILE, gateway.GATEWAY_CONTEXT, gateway.GATEWAY_IMAGE_TAG), None), + "service-db": ("agent_env.cli.env.service_db", ["env", "service-db", "put", "--id", "x"], + (service_db.SERVICE_DB_DOCKERFILE, service_db.SERVICE_DB_DOCKERFILE.parent, + service_db.SERVICE_DB_IMAGE_NAME), None), + "website-browser": ("agent_env.cli.env.website_browser", ["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}), + } -def test_platform_option_wired_on_mcp_server_put(): - from agent_env.cli.env.mcp_server import put as mcp_server_put - result = CliRunner().invoke(mcp_server_put, ["--help"]) - assert result.exit_code == 0 - assert "--platform" in result.output - assert "linux/amd64" in result.output # default shown +@pytest.mark.parametrize("name", ["a2a-agent", "mcp-server", "website", "gateway", "service-db", "website-browser"]) +def test_each_put_builds_through_build_image_and_a_failed_build_is_one_line(tmp_path, name): + module, argv, (dockerfile, context, tag), build_args = _put_commands(tmp_path)[name] + + with patch(f"{module}.build_image", side_effect=DockerBuildError(f"docker build of {tag} failed (exit 1):\nboom")) \ + as build: + result = CliRunner().invoke(cli, [*argv, "--platform", "linux/arm64"]) + + expected = {"platform": "linux/arm64", **({"build_args": build_args} if build_args else {})} + assert build.call_args.args == (dockerfile, context, tag) + assert build.call_args.kwargs == expected + assert result.exit_code == 1 + assert result.output.endswith(f"Error: docker build of {tag} failed (exit 1):\nboom\n") + assert "Traceback" not in result.output + + +def _multi_build_puts(tmp_path): + """The puts that build more than one image: the env type each writes, and every build it makes.""" + backend, frontend = _dockerfile(tmp_path / "backend"), _dockerfile(tmp_path / "frontend") + return { + "service-db": ("agent_env.cli.env.service_db", "ServiceDBEnv", ["env", "service-db", "put", "--id", "x"], [ + (service_db.SERVICE_DB_DOCKERFILE, service_db.SERVICE_DB_DOCKERFILE.parent, service_db.SERVICE_DB_IMAGE_NAME), + (service_db.DB_WEB_DOCKERFILE, service_db.DB_WEB_DOCKERFILE.parent, service_db.DB_WEB_IMAGE_NAME), + (service_db.DB_MCP_DOCKERFILE, service_db.DB_MCP_DOCKERFILE.parent, service_db.DB_MCP_IMAGE_NAME), + ]), + "website": ("agent_env.cli.env.website", "WebsiteEnv", + ["env", "website", "put", "--id", "x", "--environment-name", "shop", "--skip-validation", + "--backend-dockerfile", str(backend), "--frontend-dockerfile", str(frontend)], + [(backend, backend.parent, "website-backend-x"), (frontend, frontend.parent, "website-frontend-x")]), + } + + +@pytest.mark.parametrize("name", ["service-db", "website"]) +def test_a_put_that_builds_several_images_builds_every_one_for_its_platform(tmp_path, name): + module, env_type, argv, builds = _multi_build_puts(tmp_path)[name] + + with patch(f"{module}.build_image") as build, patch(f"{module}.DockerImageArtifact.put"), \ + patch(f"{module}.{env_type}.put"): + result = CliRunner().invoke(cli, [*argv, "--platform", "linux/arm64"]) + + assert result.exit_code == 0, result.output + assert build.call_args_list == [call(*args, platform="linux/arm64") for args in builds] diff --git a/tst/unit/cli/env_provider_type_test.py b/tst/unit/cli/env_provider_type_test.py index fbf2b7bf..23a57446 100644 --- a/tst/unit/cli/env_provider_type_test.py +++ b/tst/unit/cli/env_provider_type_test.py @@ -24,7 +24,7 @@ def test_the_github_build_puts_the_declared_type(flag, put_with): def test_an_unknown_type_is_refused_before_anything_is_built(tmp_path): - with patch("agent_env.cli.env.mcp_server.subprocess.run") as build, patch("agent_env.cli.env.mcp_server.MCPServerEnv.put") as put: + with patch("agent_env.cli.env.mcp_server.build_image") as build, patch("agent_env.cli.env.mcp_server.MCPServerEnv.put") as put: result = CliRunner().invoke(cli, [*_PUT, "--dockerfile", str(_dockerfile(tmp_path)), "--env-provider-type", "vm"]) assert result.exit_code == 2 and "Invalid value for '--env-provider-type'" in result.output assert (build.called, put.called) == (False, False) @@ -40,7 +40,7 @@ def _capture(**kwargs): captured.update(kwargs) return _put_env(kwargs) - with patch("agent_env.cli.env.mcp_server.subprocess.run", return_value=MagicMock(returncode=0, stdout="", stderr="")), \ + with patch("agent_env.cli.env.mcp_server.build_image"), \ patch("agent_env.cli.env.mcp_server.DockerImageArtifact.put", return_value=MagicMock(id="a", version=1)), \ patch("agent_env.cli.env.mcp_server.detect_env_metadata", return_value={}), \ patch("agent_env.cli.env.mcp_server.MCPServerEnv.put", side_effect=_capture): diff --git a/tst/unit/cli/env_put_validation_test.py b/tst/unit/cli/env_put_validation_test.py index 3dfae881..fc45b897 100644 --- a/tst/unit/cli/env_put_validation_test.py +++ b/tst/unit/cli/env_put_validation_test.py @@ -69,7 +69,7 @@ def _mcp_put(tmp_path, build: str, *flags: str): return result, gate dockerfile = tmp_path / "Dockerfile" dockerfile.write_text("FROM scratch\n") - with patch("agent_env.cli.env.mcp_server.subprocess.run", return_value=MagicMock(returncode=0, stdout="", stderr="")), \ + with patch("agent_env.cli.env.mcp_server.build_image"), \ patch("agent_env.cli.env.mcp_server.DockerImageArtifact.put", return_value=MagicMock(id="a", version=1)), \ patch("agent_env.cli.env.mcp_server.detect_env_metadata", return_value={}), \ patch("agent_env.cli.env.mcp_server.MCPServerEnv.put", return_value=env): diff --git a/tst/unit/cli/environment_name_alias_test.py b/tst/unit/cli/environment_name_alias_test.py index 49ab33e5..9a598ac2 100644 --- a/tst/unit/cli/environment_name_alias_test.py +++ b/tst/unit/cli/environment_name_alias_test.py @@ -116,7 +116,7 @@ def _capture(**kwargs): raise SystemExit(0) with patch("agent_env.cli.env.mcp_server.card_name_from_source", return_value="items") as derive, \ - patch("agent_env.cli.env.mcp_server.subprocess.run", return_value=MagicMock(returncode=0, stdout="", stderr="")), \ + patch("agent_env.cli.env.mcp_server.build_image"), \ patch("agent_env.cli.env.mcp_server.DockerImageArtifact.put", return_value=MagicMock(id="a", version=1)), \ patch("agent_env.cli.env.mcp_server.detect_env_metadata", return_value={}), \ patch("agent_env.cli.env.mcp_server.MCPServerEnv.put", side_effect=_capture): @@ -141,7 +141,7 @@ def _capture(**kwargs): raise SystemExit(0) with patch("agent_env.cli.env.website.card_name_from_source", return_value="webitems") as derive, \ - patch("agent_env.cli.env.website.subprocess.run", return_value=MagicMock(returncode=0, stdout="", stderr="")), \ + patch("agent_env.cli.env.website.build_image"), \ patch("agent_env.cli.env.website.DockerImageArtifact.put", return_value=MagicMock(id="a", version=1)), \ patch("agent_env.cli.env.website.detect_env_metadata", return_value={}), \ patch("agent_env.cli.env.website.WebsiteEnv.put", side_effect=_capture): diff --git a/tst/unit/cli/sandbox_url_relocation_test.py b/tst/unit/cli/sandbox_url_relocation_test.py index d293c89d..64ed8408 100644 --- a/tst/unit/cli/sandbox_url_relocation_test.py +++ b/tst/unit/cli/sandbox_url_relocation_test.py @@ -11,6 +11,8 @@ from click.testing import CliRunner from unittest.mock import MagicMock, patch +from agent_env.utils.docker_build import DockerBuildError + def test_old_cli_path_no_longer_re_exports_the_helper(): # The compatibility re-export is retired: its consumer, a sandbox proxy @@ -95,8 +97,7 @@ def test_service_db_put_falls_back_to_the_config_default(): cfg = _config(default_service_db_env_id="svc-db-from-config") with patch("agent_env.cli.env.service_db.get_config", return_value=cfg), \ patch("agent_env.cli.env.service_db.DockerImageArtifact") as artifact, \ - patch("agent_env.cli.env.service_db.subprocess.run") as run: - run.return_value = MagicMock(returncode=0) + patch("agent_env.cli.env.service_db.build_image"): artifact.put.side_effect = RuntimeError("stop once the id is recorded") res = CliRunner().invoke(service_db, ["put"]) @@ -108,8 +109,8 @@ def test_service_db_put_explicit_id_wins_and_skips_the_config_read(): from agent_env.cli.env.service_db import service_db with patch("agent_env.cli.env.service_db.get_config") as get_config, \ - patch("agent_env.cli.env.service_db.subprocess.run") as run: - run.return_value = MagicMock(returncode=1, stderr="stop here") + patch("agent_env.cli.env.service_db.build_image") as run: + run.side_effect = DockerBuildError("stop here") CliRunner().invoke(service_db, ["put", "--id", "explicit-env"]) get_config.assert_not_called() @@ -121,8 +122,7 @@ def test_website_browser_put_falls_back_to_the_config_default(): cfg = _config(default_website_browser_env_id="wb-from-config") with patch("agent_env.config.get_config", return_value=cfg), \ patch("agent_env.cli.env.website_browser.DockerImageArtifact") as artifact, \ - patch("agent_env.cli.env.website_browser.subprocess.run") as run: - run.return_value = MagicMock(returncode=0) + patch("agent_env.cli.env.website_browser.build_image"): artifact.put.side_effect = RuntimeError("stop once the id is recorded") res = CliRunner().invoke(website_browser, ["put"]) diff --git a/tst/unit/env/envs/test_mcp_server_plugin_provider.py b/tst/unit/env/envs/test_mcp_server_plugin_provider.py index 6bad8a4a..553c615a 100644 --- a/tst/unit/env/envs/test_mcp_server_plugin_provider.py +++ b/tst/unit/env/envs/test_mcp_server_plugin_provider.py @@ -338,7 +338,7 @@ def _put(tmp_path, *flags): dockerfile = tmp_path / "Dockerfile" dockerfile.write_text("FROM scratch\n") put = MagicMock(return_value=MagicMock(id="x", version=1, environment_name="email")) - with patch("agent_env.cli.env.mcp_server.subprocess.run", return_value=MagicMock(returncode=0, stdout="", stderr="")), \ + with patch("agent_env.cli.env.mcp_server.build_image"), \ patch("agent_env.cli.env.mcp_server.DockerImageArtifact.put", return_value=MagicMock(id="a", version=1)), \ patch("agent_env.cli.env.mcp_server.detect_env_metadata", return_value={}), \ patch("agent_env.cli.env.mcp_server.MCPServerEnv.put", put): diff --git a/tst/unit/utils/docker_build_test.py b/tst/unit/utils/docker_build_test.py new file mode 100644 index 00000000..ad06d64d --- /dev/null +++ b/tst/unit/utils/docker_build_test.py @@ -0,0 +1,86 @@ +"""``build_image``: the argv it runs, what a failed or impossible build raises, and that importing it pulls in +neither the CLI nor the providers, so library code can build too.""" + +import os +import subprocess +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from agent_env.utils import docker_build +from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM, DockerBuildError, build_image + + +@pytest.fixture +def runs(monkeypatch): + calls = [] + + def run(argv, **kwargs): + calls.append((argv, kwargs)) + return SimpleNamespace(returncode=0, stdout="") + + monkeypatch.setattr(docker_build.subprocess, "run", run) + return calls + + +def test_the_default_platform_is_the_remote_sandboxes(): + assert DEFAULT_BUILD_PLATFORM == "linux/amd64" + + +@pytest.mark.parametrize("platform, flags", [ + ("linux/amd64", ["--platform", "linux/amd64"]), + ("linux/arm64", ["--platform", "linux/arm64"]), + ("", []), + (None, []), +], ids=["amd64", "arm64", "empty-is-host-native", "none-is-host-native"]) +def test_it_builds_the_context_with_the_dockerfile_into_the_tag(runs, platform, flags): + build_image(Path("/ctx/Dockerfile"), Path("/ctx"), "my-image", platform=platform, build_args={"VERSION": "1.2"}) + + (argv, kwargs), = runs + assert argv == ["docker", "build", *flags, "--build-arg", "VERSION=1.2", "-f", "/ctx/Dockerfile", "-t", "my-image", + "/ctx"] + assert kwargs["stderr"] is subprocess.STDOUT + + +def test_a_failed_build_raises_with_its_output(monkeypatch): + output = "#5 [2/3] RUN pip install nope\n#5 ERROR: process did not complete\n" + monkeypatch.setattr(docker_build.subprocess, "run", lambda argv, **_: SimpleNamespace(returncode=1, stdout=output)) + + with pytest.raises(DockerBuildError) as failed: + build_image(Path("Dockerfile"), Path("."), "my-image", platform=None) + + assert str(failed.value) == f"docker build of my-image failed (exit 1):\n{output.rstrip()}" + + +def test_the_error_carries_both_of_docker_s_streams_with_undecodable_bytes_replaced(tmp_path, monkeypatch): + docker = tmp_path / "docker" + docker.write_text("#!/bin/sh\nprintf 'step 1\\n'\nprintf 'bad \\377 byte\\n' >&2\nexit 3\n") + docker.chmod(0o755) + monkeypatch.setenv("PATH", f"{tmp_path}{os.pathsep}{os.environ['PATH']}") + + with pytest.raises(DockerBuildError) as failed: + build_image(Path("Dockerfile"), Path("."), "my-image", platform=None) + + assert str(failed.value) == "docker build of my-image failed (exit 3):\nstep 1\nbad � byte" + + +def test_a_missing_docker_raises_rather_than_crashing(monkeypatch): + def run(argv, **_): + raise FileNotFoundError(2, "No such file or directory", "docker") + + monkeypatch.setattr(docker_build.subprocess, "run", run) + + with pytest.raises(DockerBuildError, match="docker build of my-image failed: docker is not on PATH"): + build_image(Path("Dockerfile"), Path("."), "my-image", platform=None) + + +def test_importing_it_pulls_neither_cli_nor_providers(): + code = ( + "import sys; import agent_env.utils.docker_build; " + "assert 'agent_env.cli' not in sys.modules, 'cli imported'; " + "assert 'agent_env.providers' not in sys.modules, 'providers imported'" + ) + proc = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True) + assert proc.returncode == 0, proc.stderr