diff --git a/src/agent_env/cli/a2a_agent/put.py b/src/agent_env/cli/a2a_agent/put.py index 09a60beb..a2786649 100644 --- a/src/agent_env/cli/a2a_agent/put.py +++ b/src/agent_env/cli/a2a_agent/put.py @@ -5,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 +from agent_env.cli.utils import build_platform_option, detect_env_metadata, refuse_unwritable_ids +from agent_env.store.ids import derive_id, image_repository from agent_env.utils.docker_build import build_image @@ -41,14 +42,16 @@ def put(agent_id: str, dockerfile: str, context_path: str | None, env_var_pairs: dockerfile_path = Path(dockerfile) context = Path(context_path) if context_path else dockerfile_path.parent - image_tag = f"a2a-agent-{agent_id}" + image_id = derive_id(agent_id, "agent_image") + image_tag = image_repository(image_id) + refuse_unwritable_ids(agent_id, image_id) click.echo(f"Building Docker image...") build_image(dockerfile_path, context, image_tag, platform=build_platform) click.echo(f"Creating DockerImageArtifact...") artifact = DockerImageArtifact.put( - id=f"a2a-agent-{agent_id}", + id=image_id, description=f"A2A agent image for {agent_id}", image_name=image_tag, build_context_path=str(context), diff --git a/src/agent_env/cli/env/mcp_server.py b/src/agent_env/cli/env/mcp_server.py index 00c3e74b..67572c1f 100644 --- a/src/agent_env/cli/env/mcp_server.py +++ b/src/agent_env/cli/env/mcp_server.py @@ -12,11 +12,13 @@ detect_env_metadata, env_provider_type_option, environment_name_options, + refuse_unwritable_ids, resolve_environment_name, ) 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 +from agent_env.store.ids import derive_id, image_repository @click.group(name="mcp-server") @@ -188,14 +190,16 @@ def _on_progress(step: str, message: str, percent: int) -> None: click.echo("Error: no @environment_card(name=...) found in the build source; pass --environment-name.", err=True) sys.exit(1) click.echo(f"Derived environment_name={environment_name!r} from the environment card.") - image_tag = f"mcp-server-{env_id}" + image_id = derive_id(env_id, "env_image") + image_tag = image_repository(image_id) + refuse_unwritable_ids(env_id, image_id) click.echo(f"Building MCP server Docker image...") build_image(dockerfile_path, context, image_tag, platform=build_platform) click.echo(f"Creating DockerImageArtifact...") artifact = DockerImageArtifact.put( - id=f"mcp-server-{env_id}", + id=image_id, description="Created from agent-env CLI", image_name=image_tag, build_context_path=str(context), diff --git a/src/agent_env/cli/env/website.py b/src/agent_env/cli/env/website.py index e7a165ed..dd2e6041 100644 --- a/src/agent_env/cli/env/website.py +++ b/src/agent_env/cli/env/website.py @@ -12,6 +12,7 @@ detect_env_metadata, env_provider_type_option, environment_name_options, + refuse_unwritable_ids, resolve_environment_name, ) from agent_env.utils.card_naming import card_name_from_github, card_name_from_source @@ -19,6 +20,7 @@ from agent_env.env import Env from agent_env.env.envs.website import WebsiteEnv from agent_env.providers import get_env_sandbox_provider +from agent_env.store.ids import derive_id, image_repository @@ -140,7 +142,9 @@ def _frontend_progress(step: str, message: str, percent: int) -> None: # Local Docker build path backend_dockerfile_path = Path(backend_dockerfile) backend_ctx = Path(backend_docker_context) if backend_docker_context else backend_dockerfile_path.parent - backend_tag = f"website-backend-{env_id}" + backend_id, frontend_id = derive_id(env_id, "backend_image"), derive_id(env_id, "frontend_image") + refuse_unwritable_ids(env_id, backend_id, frontend_id) + backend_tag = image_repository(backend_id) if environment_name is None: environment_name = card_name_from_source(str(backend_dockerfile_path), str(backend_ctx)) if not environment_name: @@ -153,7 +157,7 @@ def _frontend_progress(step: str, message: str, percent: int) -> None: click.echo(f"Creating backend DockerImageArtifact...") backend_artifact = DockerImageArtifact.put( - id=f"website-backend-{env_id}", + id=backend_id, description="Website backend image created from agent-env CLI", image_name=backend_tag, ) @@ -161,14 +165,14 @@ def _frontend_progress(step: str, message: str, percent: int) -> None: frontend_dockerfile_path = Path(frontend_dockerfile) frontend_ctx = Path(frontend_docker_context) if frontend_docker_context else frontend_dockerfile_path.parent - frontend_tag = f"website-frontend-{env_id}" + frontend_tag = image_repository(frontend_id) click.echo(f"Building website frontend Docker image...") build_image(frontend_dockerfile_path, frontend_ctx, frontend_tag, platform=build_platform) click.echo(f"Creating frontend DockerImageArtifact...") frontend_artifact = DockerImageArtifact.put( - id=f"website-frontend-{env_id}", + id=frontend_id, description="Website frontend image created from agent-env CLI", image_name=frontend_tag, ) diff --git a/src/agent_env/cli/utils.py b/src/agent_env/cli/utils.py index cd122b30..7df4225e 100644 --- a/src/agent_env/cli/utils.py +++ b/src/agent_env/cli/utils.py @@ -2,6 +2,7 @@ import click +from agent_env.config import get_config from agent_env.env import Env from agent_env.providers.env_providers.env_provider import _env_provider_class from agent_env.providers.env_providers.env_server_provider import EnvironmentServerProvider @@ -31,6 +32,17 @@ def build_platform_option(f): )(f) +def refuse_unwritable_ids(*ids: str) -> None: + """Refuse, before anything is built, an id the store a put writes it to wouldn't take, such as an @local id the + image's suffix makes too long.""" + store = get_config().get_document_store() + for entity_id in ids: + try: + store.check_id(entity_id) + except ValueError as e: + raise click.ClickException(str(e)) from None + + 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/env/bootstrap.py b/src/agent_env/env/bootstrap.py index f0883aaf..fa54ddfb 100644 --- a/src/agent_env/env/bootstrap.py +++ b/src/agent_env/env/bootstrap.py @@ -24,6 +24,7 @@ ) from agent_env.store.base import NotFoundError from agent_env.store.document_store import LocalSqliteDocumentStore +from agent_env.store.ids import derive_id from agent_env.store.image_store import LocalRegistryImageStore from agent_env.store.local_state import holding_locks from agent_env.store.object_store import LocalFilesystemObjectStore @@ -57,7 +58,7 @@ def put_gateway_env(env_id: str, *, platform: str | None, metadata: Mapping[str, say("Building gateway Docker image...") build_image(GATEWAY_DOCKERFILE, GATEWAY_CONTEXT, GATEWAY_IMAGE_TAG, platform=platform) say("Creating DockerImageArtifact...") - artifact = DockerImageArtifact.put(id=f"gateway-{env_id}", description="Created from agent-env CLI", + artifact = DockerImageArtifact.put(id=derive_id(env_id, "env_image"), description="Created from agent-env CLI", image_name=GATEWAY_IMAGE_TAG) say(f"Created artifact: id={artifact.id} version={artifact.version}") say("Creating GatewayEnv...") @@ -73,11 +74,11 @@ def put_service_db_env(env_id: str, *, platform: str | None, metadata: Mapping[s service-db env ``env_id``.""" artifacts = [] for label, dockerfile, image, artifact_id, description in ( - ("ServiceDB", SERVICE_DB_DOCKERFILE, SERVICE_DB_IMAGE_NAME, f"service-db-{env_id}", + ("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, f"db-web-{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, f"db-mcp-{env_id}", + ("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}...") @@ -105,7 +106,7 @@ def put_website_browser_env(env_id: str, *, platform: str | None, metadata: Mapp build_image(WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_CONTEXT, WEBSITE_BROWSER_IMAGE_TAG, platform=platform, build_args={"PLAYWRIGHT_MCP_VERSION": PLAYWRIGHT_MCP_VERSION}) say("Creating DockerImageArtifact...") - artifact = DockerImageArtifact.put(id=f"website-browser-{env_id}", description="Website browser MCP server", + artifact = DockerImageArtifact.put(id=derive_id(env_id, "env_image"), description="Website browser MCP server", image_name=WEBSITE_BROWSER_IMAGE_TAG) say(f"Created artifact: id={artifact.id} version={artifact.version}") say("Creating MCPServerEnv...") diff --git a/src/agent_env/env/envs/mcp_server.py b/src/agent_env/env/envs/mcp_server.py index c80a7aa9..ba64bfc8 100644 --- a/src/agent_env/env/envs/mcp_server.py +++ b/src/agent_env/env/envs/mcp_server.py @@ -350,7 +350,7 @@ async def put_from_github( """ refuse_local_github_build(id) docker_image_artifact = await DockerImageArtifact.put_from_github( - id=f"mcp-server-{id}", + id=derive_id(id, "env_image"), dockerfile_github_url=dockerfile_github_url, docker_context_github_url=docker_context_github_url, on_progress=on_progress, diff --git a/src/agent_env/env/envs/website.py b/src/agent_env/env/envs/website.py index f3e5430b..6cb80820 100644 --- a/src/agent_env/env/envs/website.py +++ b/src/agent_env/env/envs/website.py @@ -247,14 +247,14 @@ async def put_from_github( refuse_local_github_build(id) backend, frontend = await asyncio.gather( DockerImageArtifact.put_from_github( - id=f"website-backend-{id}", + id=derive_id(id, "backend_image"), dockerfile_github_url=backend_dockerfile_github_url, docker_context_github_url=backend_docker_context_github_url, on_progress=on_backend_progress, github_token=github_token, ), DockerImageArtifact.put_from_github( - id=f"website-frontend-{id}", + id=derive_id(id, "frontend_image"), dockerfile_github_url=frontend_dockerfile_github_url, docker_context_github_url=frontend_docker_context_github_url, on_progress=on_frontend_progress, diff --git a/tst/unit/cli/build_platform_test.py b/tst/unit/cli/build_platform_test.py index 6b7a0577..ae15625a 100644 --- a/tst/unit/cli/build_platform_test.py +++ b/tst/unit/cli/build_platform_test.py @@ -36,13 +36,13 @@ def _put_commands(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), + (dockerfile, tmp_path, "x__agent_image"), 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), + str(dockerfile)], (dockerfile, tmp_path, "x__env_image"), 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), + "--frontend-dockerfile", str(frontend)], (backend, tmp_path, "x__backend_image"), None), "gateway": ("agent_env.env.bootstrap", ["env", "gateway", "put", "--id", "x"], (gateway.GATEWAY_DOCKERFILE, gateway.GATEWAY_CONTEXT, gateway.GATEWAY_IMAGE_TAG), None), "service-db": ("agent_env.env.bootstrap", ["env", "service-db", "put", "--id", "x"], @@ -82,7 +82,7 @@ def _multi_build_puts(tmp_path): "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")]), + [(backend, backend.parent, "x__backend_image"), (frontend, frontend.parent, "x__frontend_image")]), } diff --git a/tst/unit/cli/sandbox_url_relocation_test.py b/tst/unit/cli/sandbox_url_relocation_test.py index fb208f7d..2b774333 100644 --- a/tst/unit/cli/sandbox_url_relocation_test.py +++ b/tst/unit/cli/sandbox_url_relocation_test.py @@ -101,7 +101,7 @@ def test_service_db_put_falls_back_to_the_config_default(): artifact.put.side_effect = RuntimeError("stop once the id is recorded") res = CliRunner().invoke(service_db, ["put"]) - assert artifact.put.call_args.kwargs["id"] == "service-db-svc-db-from-config" + assert artifact.put.call_args.kwargs["id"] == "svc-db-from-config__db_image" assert res.exit_code != 0 @@ -126,7 +126,7 @@ def test_website_browser_put_falls_back_to_the_config_default(): artifact.put.side_effect = RuntimeError("stop once the id is recorded") res = CliRunner().invoke(website_browser, ["put"]) - assert artifact.put.call_args.kwargs["id"] == "website-browser-wb-from-config" + assert artifact.put.call_args.kwargs["id"] == "wb-from-config__env_image" assert res.exit_code != 0 diff --git a/tst/unit/env/bootstrap_test.py b/tst/unit/env/bootstrap_test.py index f624448b..60d49fa4 100644 --- a/tst/unit/env/bootstrap_test.py +++ b/tst/unit/env/bootstrap_test.py @@ -83,7 +83,7 @@ def test_infra_that_records_no_shipped_dockerfile_is_left_as_it_is(puts): def test_an_env_built_from_someone_elses_dockerfile_is_theirs_to_rebuild(puts, monkeypatch): artifact = get_artifact_store().put_document(DockerImageArtifact( - id="website-browser-website-browser", description="b", image_name="b:v1", tar_gz_s3_url="file:///b.tar.gz")) + id="website-browser__env_image", description="b", image_name="b:v1", tar_gz_s3_url="file:///b.tar.gz")) GatewayEnv.put(id="website-browser", docker_image_artifact=artifact, metadata={"dockerfile_path": "/home/me/my-browser/Dockerfile", bootstrap.BUILD_INPUTS_KEY: "theirs"}) diff --git a/tst/unit/env/envs/test_put_from_github_token.py b/tst/unit/env/envs/test_put_from_github_token.py index aab58683..bb562d99 100644 --- a/tst/unit/env/envs/test_put_from_github_token.py +++ b/tst/unit/env/envs/test_put_from_github_token.py @@ -61,5 +61,5 @@ async def test_website_env_forwards_the_token_to_both_builds(recorded_builds): frontend_dockerfile_github_url="https://github.com/o/r/blob/main/fe/Dockerfile", github_token="ghs_y", ) - assert sorted(c["id"] for c in recorded_builds) == ["website-backend-w", "website-frontend-w"] + assert sorted(c["id"] for c in recorded_builds) == ["w__backend_image", "w__frontend_image"] assert {c["github_token"] for c in recorded_builds} == {"ghs_y"} diff --git a/tst/unit/store/local_id_acceptance_test.py b/tst/unit/store/local_id_acceptance_test.py index 3e0da684..8da1c7c4 100644 --- a/tst/unit/store/local_id_acceptance_test.py +++ b/tst/unit/store/local_id_acceptance_test.py @@ -28,7 +28,7 @@ ) from agent_env.store import Filter, LocalSqliteDocumentStore, VersionedEntityStore from agent_env.store.base import NotFoundError -from agent_env.store.ids import fs_safe, key_segment +from agent_env.store.ids import fs_safe, image_repository, key_segment from agent_env.store.routing import LocalNamespaceDocumentStore from agent_env.task import Task from tst.unit.store.fakes import FakeDocumentStore, FakeImageStore, SigningObjectStore, reattach_for_snapshot @@ -261,6 +261,61 @@ def test_under_the_cli_a_bare_id_derived_from_an_local_one_is_refused_before_any LocalSqliteDocumentStore(str(state_root() / "services.db")).check_id(derived) +LOCAL_AGENT = "@local/~/bundle/agents/a" + + +@pytest.mark.parametrize("module, argv, images", [ + ("agent_env.cli.a2a_agent.put", ["a2a-agent", "put", "--id", LOCAL_AGENT, "--skip-validation"], + [f"{LOCAL_AGENT}__agent_image"]), + ("agent_env.cli.env.mcp_server", ["env", "mcp-server", "put", "--id", LOCAL_ENV, "--environment-name", "items"], + [f"{LOCAL_ENV}__env_image"]), + ("agent_env.cli.env.website", ["env", "website", "put", "--id", LOCAL_ENV, "--environment-name", "shop", + "--skip-validation", "--backend-dockerfile", "{dockerfile}"], + [f"{LOCAL_ENV}__backend_image", f"{LOCAL_ENV}__frontend_image"]), +], ids=["a2a-agent", "mcp-server", "website"]) +def test_a_put_with_an_local_id_builds_and_writes_its_images_under_ids_derived_from_it( + local_stores, tmp_path, monkeypatch, module, argv, images, +): + dockerfile = tmp_path / "Dockerfile" + dockerfile.write_text("FROM scratch\n") + tags = [] + monkeypatch.setattr(importlib.import_module(module), "build_image", + lambda dockerfile, context, tag, **kwargs: tags.append(tag)) + tarball = local_stores.get_object_store().put("image.tar.gz", b"x") + monkeypatch.setattr(DockerImageArtifact, "put", classmethod(lambda cls, id, *, description, image_name, **kwargs: ( + cls.put_tar(id, description=description, image_name=image_name, tar_gz_s3_url=tarball)))) + argv = [arg.format(dockerfile=dockerfile) for arg in argv] + flag = "--frontend-dockerfile" if "--backend-dockerfile" in argv else "--dockerfile" + + result = CliRunner().invoke(cli, [*argv, flag, str(dockerfile)]) + + assert result.exit_code == 0, result.output + assert tags == [image_repository(image) for image in images] and all(tag.startswith("local/") for tag in tags) + saved = {d["id"]: d["image_name"] for d in _local().query("artifacts", Filter())} + assert saved == {image: image_repository(image) for image in images} + (owner,) = _local().query("a2a_agents" if module.endswith(".put") else "envs", Filter()) + assert sorted(ref["id"] for ref in owner.values() if isinstance(ref, dict) and "id" in ref) == images + assert not _documents().path.exists() or _documents().count("artifacts", Filter()) == 0 + + +@pytest.mark.parametrize("module, argv", [ + ("agent_env.cli.a2a_agent.put", ["a2a-agent", "put", "--skip-validation"]), + ("agent_env.cli.env.mcp_server", ["env", "mcp-server", "put", "--environment-name", "items"]), +], ids=["a2a-agent", "mcp-server"]) +def test_a_put_whose_image_id_would_be_too_long_is_refused_before_it_builds( + local_stores, tmp_path, monkeypatch, module, argv, +): + dockerfile = tmp_path / "Dockerfile" + dockerfile.write_text("FROM scratch\n") + monkeypatch.setattr(importlib.import_module(module), "build_image", lambda *args, **kwargs: pytest.fail("it built")) + longest = "@local/~/" + "x" * (4096 - len("@local/~/")) # as long as an @local id may be, so a suffix can't fit + + result = CliRunner().invoke(cli, [*argv, "--id", longest, "--dockerfile", str(dockerfile)]) + + assert result.exit_code == 1 and "Error:" in result.output, result.output + assert not _local().path.exists() or _local().count("artifacts", Filter()) == 0 + + class _Deployable: """An env or agent whose deploy records what a deployment does: a state instance naming no owner, and an instance of a bare infrastructure env."""