diff --git a/src/agent_env/a2a_agent/validator.py b/src/agent_env/a2a_agent/validator.py index 58b344ca..05f07ec0 100644 --- a/src/agent_env/a2a_agent/validator.py +++ b/src/agent_env/a2a_agent/validator.py @@ -54,6 +54,7 @@ VIDEO_PROBE_MP4_B64, VIDEO_PROBE_PROMPT, ) +from agent_env.task_step.task_steps.sandbox_utils.sandbox_utils import find_agent_container if TYPE_CHECKING: from agent_env.a2a_agent.a2a_agent import A2AAgent @@ -686,7 +687,7 @@ def record(*, supported, advertised, save_ok, apply_ok, roundtrip_ok, note=""): if apply_agent.sandbox_type else get_agent_sandbox_provider()) sandbox = await provider.get_sandbox(apply_agent.sandbox_id) if sandbox.mode == SANDBOX_MODE_VM: - container = await A2AAgentValidator._discover_agent_container(sandbox) + container = await find_agent_container(sandbox) args = ("sudo", "docker", "exec", container, "cat", marker_path) else: args = ("cat", marker_path) @@ -706,22 +707,6 @@ def record(*, supported, advertised, save_ok, apply_ok, roundtrip_ok, note=""): record(supported=roundtrip_ok, advertised=advertised, save_ok=save_ok, apply_ok=True, roundtrip_ok=roundtrip_ok) - @staticmethod - async def _discover_agent_container(sandbox) -> str: - """Find the agent container on a VM sandbox (mirrors collect_artifacts / - verify_sandbox): prefer 'agent-api', else the first 'a2a-agent-*'.""" - exit_code, stdout, stderr = await sandbox.exec_with_output( - "sudo", "docker", "ps", "--format", "{{.Names}}") - if exit_code != 0: - raise RuntimeError(f"docker ps failed: {stderr[:200]}") - running = [n.strip() for n in stdout.splitlines() if n.strip()] - if sandbox.container_name in running: - return sandbox.container_name - fallback = [n for n in running if n.startswith("a2a-agent-")] - if not fallback: - raise RuntimeError(f"no agent container found; running: {running}") - return fallback[0] - @staticmethod def _upload_install_test_image_fixture(agent: "A2AAgent"): """Upload a minimal Dockerfile as a FileArtifactUniverse so the install diff --git a/src/agent_env/env/envs/mcp_server.py b/src/agent_env/env/envs/mcp_server.py index c7db1441..c80a7aa9 100644 --- a/src/agent_env/env/envs/mcp_server.py +++ b/src/agent_env/env/envs/mcp_server.py @@ -222,7 +222,7 @@ async def _copy_artifact_into_container(self, file_artifact) -> str: await self._sandbox.load_s3_file(file_artifact.object_url, vm_temp_path) container_id = await self._env_provider._get_container_id(self._sandbox, self.environment_name) await self._sandbox.exec_script(f"docker exec {container_id} mkdir -p /data") - await self._sandbox.exec_script(f"docker cp {vm_temp_path} {container_id}:{container_path}") + await self._sandbox.docker_cp(vm_temp_path, f"{container_id}:{container_path}") await self._sandbox.exec_script(f"rm -f {vm_temp_path}") return container_path diff --git a/src/agent_env/providers/sandbox_providers/sandbox.py b/src/agent_env/providers/sandbox_providers/sandbox.py index fd2e9391..dd259aa3 100644 --- a/src/agent_env/providers/sandbox_providers/sandbox.py +++ b/src/agent_env/providers/sandbox_providers/sandbox.py @@ -326,11 +326,22 @@ async def _remove_vm_temp_file(self, *vm_paths: str) -> None: except Exception as e: logger.warning(f"Best-effort cleanup of {', '.join(vm_paths)} failed (ignored): {e}") + async def docker_cp(self, source: str, destination: str, *, remove_source: bool = False) -> None: + """``docker cp source destination``, one side ``container:path``. The paths go as arguments, not + script text, so a sandbox that maps its paths (the local one maps /app) maps only the host side. + ``remove_source`` deletes the copied host file in the same exec.""" + script = 'docker cp "$1" "$2"' + (' && rm -f "$1"' if remove_source else "") + exit_code, stdout, stderr = await self.exec_with_output("sudo", "bash", "-c", script, "docker-cp", source, destination) + if exit_code != 0: + raise RuntimeError( + f"docker cp {source} {destination} failed (exit {exit_code}):\nstdout: {stdout[-1500:]}\nstderr: {stderr[-1500:]}" + ) + async def _copy_into_container(self, vm_path: str, destination_path: str) -> None: parent = os.path.dirname(destination_path) if parent: await self.exec_script(f"docker exec -u 0 {shlex.quote(self.container_name)} mkdir -p {shlex.quote(parent)}") - await self.exec_script(f"docker cp {shlex.quote(vm_path)} {self.container_name}:{shlex.quote(destination_path)}") + await self.docker_cp(vm_path, f"{self.container_name}:{destination_path}") @staticmethod def _staging_path(kind: str, destination_path: str) -> str: diff --git a/src/agent_env/task_step/task_steps/collect_artifacts.py b/src/agent_env/task_step/task_steps/collect_artifacts.py index e8fbd3f9..15f4e250 100644 --- a/src/agent_env/task_step/task_steps/collect_artifacts.py +++ b/src/agent_env/task_step/task_steps/collect_artifacts.py @@ -84,6 +84,7 @@ from agent_env.task_step.context import TaskStepContext from agent_env.entity_refs import EntityRef from agent_env.task_step.task_step import TaskStep, TaskStepDependency +from agent_env.task_step.task_steps.sandbox_utils.sandbox_utils import find_agent_container from agent_env.task_step.thread_work import finish_on_thread logger = logging.getLogger(__name__) @@ -443,39 +444,6 @@ async def _resolve_live_sandbox(self, provider, sandbox_id: str): f"a re-run-from-step needs the original live sandbox; re-run the full task." ) from err - async def _discover_container(self, sandbox) -> str: - """Find the agent container running on the VM. - - The A2A agent deploy hardcodes the container name to 'agent-api' - (see agent_env/a2a_agent/a2a_agent.py). We also accept any - 'a2a-agent-*' container as a fallback in case the naming scheme - evolves. - """ - exit_code, stdout, stderr = await sandbox.exec_with_output( - "sudo", "docker", "ps", - "--format", "{{.Names}}", - ) - if exit_code != 0: - raise RuntimeError(f"Failed to list containers: {stderr[:300]}") - running = [n.strip() for n in stdout.splitlines() if n.strip()] - # Prefer this sandbox's own container name, fall back to the a2a-agent-* prefix. - if sandbox.container_name in running: - return sandbox.container_name - if getattr(sandbox, "owns_container", False): - # Its Docker host is shared, so any other agent container there is another run's. - raise RuntimeError( - f"Agent container {sandbox.container_name!r} is not running. Running containers: {running}." - ) - fallback = [n for n in running if n.startswith("a2a-agent-")] - if fallback: - if len(fallback) > 1: - logger.warning(f"Multiple a2a-agent-* containers found; using first: {fallback}") - return fallback[0] - raise RuntimeError( - f"No agent container found on the VM (looked for 'agent-api' or 'a2a-agent-*'). " - f"Running containers: {running}. Has deploy_agent been run in this task?" - ) - async def _get_file_size(self, sandbox, container: Optional[str], source_path: str) -> int: """Get file size on the agent's filesystem. Returns -1 only if the file is genuinely absent; a `stat` that fails for any other reason RAISES so a failed size check is a @@ -809,7 +777,7 @@ async def _collect_via_agent_container(self, context, store, artifact_id, versio sandbox = await self._resolve_live_sandbox(provider, agent.sandbox_id) logger.info(f"Connected to sandbox {agent.sandbox_id} (mode={sandbox.mode})") - container = await self._discover_container(sandbox) if sandbox.mode == SANDBOX_MODE_VM else None + container = await find_agent_container(sandbox) if sandbox.mode == SANDBOX_MODE_VM else None if container: logger.info(f"Using agent container: {container}") @@ -847,7 +815,7 @@ async def _collect_via_vm_host(self, context, store, artifact_id, version): async def _collect_via_sandbox_container(self, context, store, artifact_id, version): """Collect from a plain container started by ``run_docker_container``. - The agent path finds its container by discovery, and ``_discover_container`` only accepts + The agent path finds its container by discovery, and ``find_agent_container`` only accepts ``agent-api`` / ``a2a-agent-*`` names — so a task that deploys a sandbox and runs an ordinary image had no way to get its files out. Here the (sandbox, container) pair is named explicitly and resolved exactly as ``load_artifact`` resolves it. diff --git a/src/agent_env/task_step/task_steps/load_artifact.py b/src/agent_env/task_step/task_steps/load_artifact.py index c4d1ac6c..cb3cfc1c 100644 --- a/src/agent_env/task_step/task_steps/load_artifact.py +++ b/src/agent_env/task_step/task_steps/load_artifact.py @@ -662,12 +662,9 @@ async def _stage_environment_payload_into_container( ) else: await sandbox.exec_script( - f"sudo docker exec -u 0 {shlex.quote(container_name)} mkdir -p {shlex.quote(destination)}" - ) - await sandbox.exec_script( - f"sudo docker cp {shlex.quote(vm_stage)}/. " - f"{shlex.quote(container_name)}:{shlex.quote(destination)}" + f"docker exec -u 0 {shlex.quote(container_name)} mkdir -p {shlex.quote(destination)}" ) + await sandbox.docker_cp(f"{vm_stage}/.", f"{container_name}:{destination}") finally: try: await sandbox.exec_script(f"rm -rf {shlex.quote(vm_payload)} {shlex.quote(vm_stage)}") @@ -747,7 +744,7 @@ async def _load_universe_into_container(sandbox, container_name: str, universe, return [] await sandbox.exec_script( - f"sudo docker exec {shlex.quote(container_name)} mkdir -p {shlex.quote(destination)}" + f"docker exec {shlex.quote(container_name)} mkdir -p {shlex.quote(destination)}" ) loaded: list[str] = [] total = len(file_artifacts) @@ -764,12 +761,8 @@ async def _load_universe_into_container(sandbox, container_name: str, universe, await sandbox.load_s3_file(fa.object_url, vm_temp) if parent and parent != destination: await sandbox.exec_script( - f"sudo docker exec {shlex.quote(container_name)} mkdir -p {shlex.quote(parent)}" + f"docker exec {shlex.quote(container_name)} mkdir -p {shlex.quote(parent)}" ) - await sandbox.exec_script( - f"sudo docker cp {shlex.quote(vm_temp)} " - f"{shlex.quote(container_name)}:{shlex.quote(dest_path)} && " - f"rm -f {shlex.quote(vm_temp)}" - ) + await sandbox.docker_cp(vm_temp, f"{container_name}:{dest_path}", remove_source=True) loaded.append(filename) return loaded diff --git a/src/agent_env/task_step/task_steps/sandbox_utils/sandbox_utils.py b/src/agent_env/task_step/task_steps/sandbox_utils/sandbox_utils.py index 975f8498..10149ee4 100644 --- a/src/agent_env/task_step/task_steps/sandbox_utils/sandbox_utils.py +++ b/src/agent_env/task_step/task_steps/sandbox_utils/sandbox_utils.py @@ -77,3 +77,28 @@ async def fetch_container_logs(agent, tail: int = 500) -> Optional[str]: sandbox_id, e, exc_info=True, ) return None + + +async def find_agent_container(sandbox) -> str: + """The agent container running on a VM sandbox: the sandbox's own ``container_name``, else the + first ``a2a-agent-*``. A sandbox that owns its container never takes another: its Docker host + is shared, so any other agent container there is another run's.""" + exit_code, stdout, stderr = await sandbox.exec_with_output("sudo", "docker", "ps", "--format", "{{.Names}}") + if exit_code != 0: + raise RuntimeError(f"Failed to list containers: {stderr[:300]}") + running = [n.strip() for n in stdout.splitlines() if n.strip()] + if sandbox.container_name in running: + return sandbox.container_name + if getattr(sandbox, "owns_container", False): + raise RuntimeError( + f"Agent container {sandbox.container_name!r} is not running. Running containers: {running}." + ) + fallback = [n for n in running if n.startswith("a2a-agent-")] + if fallback: + if len(fallback) > 1: + logger.warning(f"Multiple a2a-agent-* containers found; using first: {fallback}") + return fallback[0] + raise RuntimeError( + f"No agent container found on the VM (looked for {sandbox.container_name!r} or 'a2a-agent-*'). " + f"Running containers: {running}. Has deploy_agent been run in this task?" + ) diff --git a/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py b/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py index f3764a09..16f3eba7 100644 --- a/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py +++ b/src/agent_env/task_step/task_steps/verifiers/run_container_unit_tests_verifier.py @@ -262,7 +262,7 @@ async def execute(self, context: TaskStepContext) -> TaskStepContext: f"{setup_cmd.splitlines()[0][:140]}" ) await sandbox.exec_script( - f"sudo docker exec -u {shlex.quote(self.user)} {env_flags} " + f"docker exec -u {shlex.quote(self.user)} {env_flags} " f"{shlex.quote(self.container_name)} bash -c {shlex.quote(setup_cmd)}" ) @@ -272,7 +272,7 @@ async def execute(self, context: TaskStepContext) -> TaskStepContext: f"in container '{self.container_name}': {command[:160]}" ) wrapped = ( - f"sudo docker exec -u {shlex.quote(self.user)} {env_flags} " + f"docker exec -u {shlex.quote(self.user)} {env_flags} " f"{shlex.quote(self.container_name)} " f"timeout --kill-after=10 {self.timeout_sec} bash -c {shlex.quote(command)}" ) @@ -428,10 +428,7 @@ def _upload_text_artifact(text: str, artifact_id: str, description: str, s3_url: async def _extract_file(self, sandbox, path_in_container: str) -> str: """`docker cp` a file out of the container, read it from the VM, return text.""" vm_temp = f"/tmp/_verifier_out_{uuid.uuid4().hex[:8]}" - await sandbox.exec_script( - f"sudo docker cp {shlex.quote(self.container_name)}:{shlex.quote(path_in_container)} " - f"{shlex.quote(vm_temp)}" - ) + await sandbox.docker_cp(f"{self.container_name}:{path_in_container}", vm_temp) try: exit_code, stdout, stderr = await sandbox.exec_with_output("sudo", "cat", vm_temp) if exit_code != 0: diff --git a/src/agent_env/task_step/task_steps/verifiers/verify_sandbox.py b/src/agent_env/task_step/task_steps/verifiers/verify_sandbox.py index 7bb7c3a2..aeaa7d38 100644 --- a/src/agent_env/task_step/task_steps/verifiers/verify_sandbox.py +++ b/src/agent_env/task_step/task_steps/verifiers/verify_sandbox.py @@ -36,6 +36,7 @@ ) from agent_env.task_step.context import TaskStepContext from agent_env.task_step.task_step import TaskStep, TaskStepDependency +from agent_env.task_step.task_steps.sandbox_utils.sandbox_utils import find_agent_container from agent_env.task_step.task_steps.verifiers.scoring import ScoreAggregator, aggregate_score logger = logging.getLogger(__name__) @@ -120,7 +121,7 @@ async def execute(self, context: TaskStepContext) -> TaskStepContext: sandbox = await (self._agent_sandbox(context) if on_agent else self._deployed_sandbox(context)) logger.info(f"Connected to sandbox {sandbox.sandbox_id} (mode={sandbox.mode})") container = ( - await self._discover_container(sandbox) + await find_agent_container(sandbox) if on_agent and sandbox.mode == SANDBOX_MODE_VM else None ) @@ -287,27 +288,3 @@ async def _eval_shell( exit_code, _, stderr = await self._exec(sandbox, args) return _outcome(exit_code == 0, f"exit={exit_code}; stderr={stderr[:200]}") - async def _discover_container(self, sandbox) -> str: - """Find the agent container on a VM sandbox. Mirrors collect_artifacts._discover_container.""" - exit_code, stdout, stderr = await sandbox.exec_with_output( - "sudo", "docker", "ps", "--format", "{{.Names}}", - ) - if exit_code != 0: - raise RuntimeError(f"Failed to list containers: {stderr[:300]}") - running = [n.strip() for n in stdout.splitlines() if n.strip()] - if sandbox.container_name in running: - return sandbox.container_name - if getattr(sandbox, "owns_container", False): - # Its Docker host is shared, so any other agent container there is another run's. - raise RuntimeError( - f"Agent container {sandbox.container_name!r} is not running. Running containers: {running}." - ) - fallback = [n for n in running if n.startswith("a2a-agent-")] - if fallback: - if len(fallback) > 1: - logger.warning(f"Multiple a2a-agent-* containers found; using first: {fallback}") - return fallback[0] - raise RuntimeError( - f"No agent container found on the VM (looked for 'agent-api' or 'a2a-agent-*'). " - f"Running containers: {running}." - ) diff --git a/tst/integration/task_step/test_verify_sandbox_local.py b/tst/integration/task_step/test_verify_sandbox_local.py index 264d27b0..5131b26c 100644 --- a/tst/integration/task_step/test_verify_sandbox_local.py +++ b/tst/integration/task_step/test_verify_sandbox_local.py @@ -5,22 +5,31 @@ an agent's, so they need Docker too, but no model. """ +import io +import json import shutil import subprocess import uuid +import zipfile import pytest import pytest_asyncio from agent_env.artifact import FileArtifact, FileArtifactUniverse +from agent_env.artifact.artifacts.environment import EnvironmentArtifact +from agent_env.a2a_agent.a2a_agent import DeployedA2AAgent +from agent_env.a2a_agent.store import get_a2a_agent_instance_store from agent_env.artifact.store import reset_artifact_store from agent_env.config import configure, get_config, reset_config from agent_env.providers.sandbox_providers.local_sandbox import LocalSandbox, LocalSandboxProvider from agent_env.task import Task -from agent_env.task_step.context import DeployedAgent, TaskStepContext +from agent_env.task_step.context import DeployedAgent, DeployedSandbox, TaskStepContext from agent_env.task_step.task_steps.collect_artifacts import CollectArtifactsTaskStep from agent_env.task_step.task_steps.deploy_sandbox import DeploySandboxTaskStep from agent_env.task_step.task_steps.load_artifact import LoadArtifactTaskStep +from agent_env.task_step.task_steps.verifiers.run_container_unit_tests_verifier import ( + RunContainerUnitTestsVerifierTaskStep, +) from agent_env.task_step.task_steps.verifiers.verify_sandbox import VerifySandboxTaskStep from tst.util.capabilities import missing_capability_reason @@ -168,6 +177,81 @@ async def test_collect_artifacts_reads_a_local_agents_file_from_its_container(lo assert get_config().get_object_store().get(url) == f"{token}\n".encode() +@pytest.mark.asyncio +async def test_load_artifact_puts_a_universe_in_a_local_agents_container(local_agent, tmp_path): + sandbox, context = local_agent + instance = get_a2a_agent_instance_store().create_instance(DeployedA2AAgent( + agent_id="agent", agent_version=1, a2a_url=sandbox.tunnel_urls[80], sandbox_id=sandbox.sandbox_id, + agent_card={}, sandbox_type="local", + ), 600) + context.deployed_agents[0].instance_id = instance.instance_id + universe = _greeting_universe(tmp_path / "greeting", uuid.uuid4().hex[:8]) + step = LoadArtifactTaskStep( + id="load", version=None, artifact_id=universe.id, agent_name="agent", destination_path="/app/greeting", + ) + + await step.execute(context) + + shown = subprocess.run(["docker", "exec", sandbox.container_name, "cat", "/app/greeting/hello.txt"], + capture_output=True, text=True) + assert (shown.returncode, shown.stdout) == (0, "hello, world\n") + assert list(sandbox.work_dir.iterdir()) == [sandbox.work_dir / ".agent-container-mode"] + + +@pytest.mark.asyncio +async def test_load_artifact_stages_an_environment_payload_in_a_local_container(local_agent, tmp_path): + sandbox, context = local_agent + _as_deployed_container(sandbox, context) + payload = io.BytesIO() + with zipfile.ZipFile(payload, "w") as zf: + zf.writestr("root/docs/readme.md", "root doc\n") + zf.writestr("data.json", json.dumps({"files": [{"path": "notes/a.txt", "content": "from data.json\n"}]})) + (tmp_path / "payload.zip").write_bytes(payload.getvalue()) + suffix = uuid.uuid4().hex[:8] + artifact = EnvironmentArtifact.put( + id=f"payload-{suffix}", environment_name="filesystem", + file_artifact=FileArtifact.put(id=f"payload-{suffix}-zip", description="payload", file_path=str(tmp_path / "payload.zip")), + ) + step = LoadArtifactTaskStep( + id="load", version=None, artifact_id=artifact.id, sandbox_name="box", container_name=sandbox.container_name, + destination_path="/app/my files", + ) + + await step.execute(context) + + for path, text in (("/app/my files/docs/readme.md", "root doc\n"), ("/app/my files/notes/a.txt", "from data.json\n")): + shown = subprocess.run(["docker", "exec", sandbox.container_name, "cat", path], capture_output=True, text=True) + assert (shown.returncode, shown.stdout) == (0, text) + + +@pytest.mark.asyncio +async def test_the_unit_tests_verifier_runs_in_a_local_container(local_agent): + sandbox, context = local_agent + _as_deployed_container(sandbox, context) + _write_in_container(sandbox, "/app/data/hello.txt", "hello") + step = RunContainerUnitTestsVerifierTaskStep( + id="verify", version=None, sandbox_name="box", container_name=sandbox.container_name, + setup_commands=["mkdir -p /logs && touch /logs/setup-ran"], + command="test -f /logs/setup-ran && grep -q hello /app/data/hello.txt && echo '{\"reward\": 1}' > /logs/reward.json", + result_paths=["/logs/reward.json"], + ) + + ctx = await step.execute(context) + + entry = ctx.metadata["verifications"]["verify"] + assert (entry["exit_code"], entry["extracted_files"]) == (0, {"/logs/reward.json": {"reward": 1}}) + + +def _as_deployed_container(sandbox, context): + """Record the agent's container as a ``run_docker_container`` container on the deployed sandbox ``box``.""" + context.deployed_sandboxes.append( + DeployedSandbox(sandbox_name="box", sandbox_id=sandbox.sandbox_id, sandbox_mode="vm", sandbox_type="local"), + ) + context.metadata.setdefault("deployed_docker_containers", []).append( + {"container_name": sandbox.container_name, "sandbox_name": "box", "sandbox_id": sandbox.sandbox_id}, + ) + + def _write_in_container(sandbox, path, text): """Write ``text`` at ``path`` inside the sandbox's container, as the agent would.""" script = f"mkdir -p $(dirname {path}) && echo {text} > {path}" diff --git a/tst/unit/a2a_agent/test_validator_changelog_objects.py b/tst/unit/a2a_agent/test_validator_changelog_objects.py index bb715ea1..e348e61a 100644 --- a/tst/unit/a2a_agent/test_validator_changelog_objects.py +++ b/tst/unit/a2a_agent/test_validator_changelog_objects.py @@ -149,3 +149,42 @@ async def test_validator_records_a_capture_it_cannot_send_instead_of_raising(sto assert not requests assert verification["apply"] is False assert verification["note"].startswith("apply request failed: ") + + +@pytest.mark.asyncio +async def test_the_marker_is_never_read_from_another_runs_container(store, requests, monkeypatch): + """A reattached local agent owns its container; with that container gone, the check fails rather than + reading the marker out of another run's agent on the same Docker host.""" + calls: list[tuple] = [] + + class _ReattachedLocalAgent: + mode = "vm" + container_name = "agent-local-apply1" + owns_container = True + + async def exec_with_output(self, *args): + calls.append(args) + if args[:3] == ("sudo", "docker", "ps"): + return 0, "a2a-agent-other\n", "" + return 0, "marker-token", "" + + class _Provider: + async def get_sandbox(self, sandbox_id): + return _ReattachedLocalAgent() + + async def close(self): + return None + + monkeypatch.setattr(provider_mod, "build_sandbox_provider", lambda spec: _Provider()) + for name in ("000000.tar", "000001.tar"): + store.put(f"changelog/run-1/{name}", b"increment") + context = _context( + {"object_url": store.object_url("changelog/run-1"), "transfer_mode": "objects"}, + ["increments"], + ) + + verification = await _validate(context) + + assert verification["roundtrip"] is False + assert "'agent-local-apply1' is not running" in verification["note"] + assert calls == [("sudo", "docker", "ps", "--format", "{{.Names}}")] diff --git a/tst/unit/providers/sandbox_providers/local_sandbox_test.py b/tst/unit/providers/sandbox_providers/local_sandbox_test.py index 534827cc..d6e22faa 100644 --- a/tst/unit/providers/sandbox_providers/local_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/local_sandbox_test.py @@ -686,6 +686,17 @@ async def test_exec_leaves_a_docker_exec_script_naming_the_containers_app_alone( assert spawned == [("bash", "-c", script)] +@pytest.mark.asyncio +@pytest.mark.parametrize("source, destination, ran", [ + ("/app/_artifact_staging/x", "c:/app/x", ("/tmp/agent-env-work/_artifact_staging/x", "c:/app/x")), + ("c:/app/out.txt", "/app/out.txt", ("c:/app/out.txt", "/tmp/agent-env-work/out.txt")), +]) +async def test_docker_cp_points_only_the_host_side_at_the_work_dir(spawned, source, destination, ran): + await LocalSandbox(work_dir=Path("/tmp/agent-env-work")).docker_cp(source, destination) + + assert spawned == [("bash", "-c", 'docker cp "$1" "$2"', "docker-cp", *ran)] + + @pytest.mark.parametrize("script", ["ls ~/app/x", "cat ${HOME}/app/x", "cat $(pwd)/app/x", "cat $APP/app/x"]) def test_rewrite_app_script_keeps_app_after_an_expansion(script): assert LocalSandbox(work_dir=Path("/tmp/agent-env-work"))._rewrite_app_script(script) == script diff --git a/tst/unit/providers/sandbox_providers/sandbox_provider_test.py b/tst/unit/providers/sandbox_providers/sandbox_provider_test.py index a8b20137..baea3428 100644 --- a/tst/unit/providers/sandbox_providers/sandbox_provider_test.py +++ b/tst/unit/providers/sandbox_providers/sandbox_provider_test.py @@ -15,6 +15,7 @@ from agent_env.providers.sandbox_providers.sandbox import Sandbox, VmSandbox from agent_env.providers.sandbox_providers.sandbox_provider import _BUILTIN_SANDBOX_PROVIDERS, SandboxProvider, build_sandbox_provider from agent_env.store import ImageStore, RegistryAuth +from tst.util.exec_scripts import script_run class _FakeSandbox(Sandbox): @@ -313,6 +314,10 @@ async def exec_script(self, script: str, *, max_retries: int = 0) -> str: self.scripts.append(script) return "" + async def exec_with_output(self, *args): + self.scripts.append(script_run(args)) + return 0, "", "" + @pytest.mark.asyncio async def test_load_s3_file_curl_retries_dns_failures(): diff --git a/tst/unit/providers/sandbox_providers/vm_sandbox_test.py b/tst/unit/providers/sandbox_providers/vm_sandbox_test.py index 775125b1..75ae6d81 100644 --- a/tst/unit/providers/sandbox_providers/vm_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/vm_sandbox_test.py @@ -10,6 +10,7 @@ from agent_env.providers.sandbox_providers.sandbox import VmSandbox from agent_env.store import set_object_store from agent_env.config import reset_config +from tst.util.exec_scripts import script_run class _SigningStore: @@ -49,7 +50,7 @@ async def exec_with_output(self, *args): if args[:2] == ("sudo", "docker") and "images" in args: return 0, self._images_stdout, "" if args[:2] == ("sudo", "bash"): - self.scripts.append(args[-1]) + self.scripts.append(script_run(args)) return 0, "", "" async def write_file_from_text(self, content, destination_path): # pragma: no cover @@ -109,7 +110,7 @@ async def exec(self, *command): # pragma: no cover async def exec_with_output(self, *args): if args[:3] == ("sudo", "bash", "-c"): - self.scripts.append(args[-1]) + self.scripts.append(script_run(args)) return 0, "", "" @@ -181,7 +182,7 @@ async def exec(self, *command): # pragma: no cover async def exec_with_output(self, *args): if args[:3] == ("sudo", "bash", "-c"): - script = args[-1] + script = script_run(args) self.scripts.append(script) if script.startswith(self._fail_on): return -1, "", "" @@ -386,3 +387,38 @@ async def test_write_host_file_writes_on_the_host_in_bounded_chunks(): b64_path = "/tmp/agentenv_run_code/input.json.b64" assert _b64_from_chunk_scripts(sandbox.scripts, b64_path) == base64.b64encode(data).decode() assert sandbox.scripts[-1] == f"base64 -d {b64_path} > /tmp/agentenv_run_code/input.json && rm -f {b64_path}" + + +class _ArgsRecorder(VmSandbox): + def __init__(self, exit_code: int = 0): + self.calls: list[tuple[str, ...]] = [] + self._exit_code = exit_code + + async def terminate(self) -> None: # pragma: no cover + pass + + async def exec(self, *command): # pragma: no cover + return None + + async def exec_with_output(self, *args): + self.calls.append(args) + return self._exit_code, "", "no such container" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("remove_source, script", [ + (False, 'docker cp "$1" "$2"'), + (True, 'docker cp "$1" "$2" && rm -f "$1"'), +]) +async def test_docker_cp_passes_its_paths_as_arguments(remove_source, script): + sandbox = _ArgsRecorder() + + await sandbox.docker_cp("/tmp/a b", "c:/app/x", remove_source=remove_source) + + assert sandbox.calls == [("sudo", "bash", "-c", script, "docker-cp", "/tmp/a b", "c:/app/x")] + + +@pytest.mark.asyncio +async def test_a_failed_docker_cp_raises_with_its_stderr(): + with pytest.raises(RuntimeError, match="(?s)exit 1.*no such container"): + await _ArgsRecorder(exit_code=1).docker_cp("/tmp/a", "c:/x") diff --git a/tst/unit/task_step/test_collect_artifacts.py b/tst/unit/task_step/test_collect_artifacts.py index f6676791..ff8861fc 100644 --- a/tst/unit/task_step/test_collect_artifacts.py +++ b/tst/unit/task_step/test_collect_artifacts.py @@ -21,6 +21,7 @@ from agent_env.task_step.context import DeployedAgent, PromptResponse, TaskStepContext from agent_env.providers.sandbox_providers.local_sandbox import LocalSandbox, LocalSandboxProvider from agent_env.task_step.task_steps.collect_artifacts import CollectArtifactsTaskStep, _exec_args, _is_url_entry +from agent_env.task_step.task_steps.sandbox_utils.sandbox_utils import find_agent_container from agent_env.env.env import DeployedEnv, DeployedGatewayEnv, EnvCapabilityUnsupported from agent_env.env.gateway.constants import EXT_STEP_URI, GATEWAY_EXTENSIONS, WELL_KNOWN_PATH from tst.unit.event_loop_probe import on_event_loop @@ -547,7 +548,7 @@ def test_a_reattached_local_agent_is_read_inside_its_own_container(self, tmp_pat sandbox, calls = _reattached_local_agent(tmp_path, monkeypatch, running="a2a-agent-other\nagent-local-agent1\n") step = CollectArtifactsTaskStep(id="collect", version=None, agent_name="solver") - container = _run(step._discover_container(sandbox)) + container = _run(find_agent_container(sandbox)) _run(step._list_base_directory(sandbox, container)) assert calls == [ @@ -561,7 +562,7 @@ def test_a_reattached_local_agent_never_borrows_another_runs_container(self, tmp sandbox, calls = _reattached_local_agent(tmp_path, monkeypatch, running="a2a-agent-other\n") with pytest.raises(RuntimeError, match="'agent-local-agent1' is not running"): - _run(CollectArtifactsTaskStep(id="collect", version=None)._discover_container(sandbox)) + _run(find_agent_container(sandbox)) assert calls == [("sudo", "docker", "ps", "--format", "{{.Names}}")] @@ -716,7 +717,7 @@ class TestSandboxContainerPath: """`container_name` + `sandbox_name`: collecting from a plain run_docker_container container. Before this path existed, a task built from deploy_sandbox -> run_docker_container had no way - to get files out: `_discover_container` only matches `agent-api` / `a2a-agent-*`, so the agent + to get files out: `find_agent_container` only matches `agent-api` / `a2a-agent-*`, so the agent path found nothing and the CUA path refuses a non-CUA env. """ diff --git a/tst/unit/task_step/test_load_artifact_file.py b/tst/unit/task_step/test_load_artifact_file.py index 32cac102..83fe8a63 100644 --- a/tst/unit/task_step/test_load_artifact_file.py +++ b/tst/unit/task_step/test_load_artifact_file.py @@ -36,6 +36,9 @@ async def exec_script(self, script, **kw): async def exec(self, *command): self.commands.append(" ".join(command)) + async def docker_cp(self, source, destination, *, remove_source=False): + self.commands.append(f"docker cp {source} {destination}") + async def load_s3_file(self, s3_url, destination_path): self.pulls.append((s3_url, destination_path)) diff --git a/tst/unit/task_step/test_sandbox_utils_agent_container.py b/tst/unit/task_step/test_sandbox_utils_agent_container.py new file mode 100644 index 00000000..db42e622 --- /dev/null +++ b/tst/unit/task_step/test_sandbox_utils_agent_container.py @@ -0,0 +1,40 @@ +"""``find_agent_container``: which container on a VM sandbox is the agent's.""" + +from __future__ import annotations + +import asyncio +from types import SimpleNamespace + +import pytest + +from agent_env.task_step.task_steps.sandbox_utils.sandbox_utils import find_agent_container + + +def _sandbox(running: str, *, owns_container: bool = False, exit_code: int = 0): + async def exec_with_output(*args): + assert args == ("sudo", "docker", "ps", "--format", "{{.Names}}") + return exit_code, running, "boom" + + return SimpleNamespace(container_name="agent-local-1", owns_container=owns_container, exec_with_output=exec_with_output) + + +def test_the_sandboxs_own_container_wins(): + assert asyncio.run(find_agent_container(_sandbox("a2a-agent-x\nagent-local-1\n"))) == "agent-local-1" + + +def test_a_sandbox_that_owns_its_container_never_takes_another(): + with pytest.raises(RuntimeError, match="'agent-local-1' is not running"): + asyncio.run(find_agent_container(_sandbox("a2a-agent-x\n", owns_container=True))) + + +def test_a_vm_the_run_does_not_own_falls_back_to_an_a2a_agent_container(): + assert asyncio.run(find_agent_container(_sandbox("db\na2a-agent-x\na2a-agent-y\n"))) == "a2a-agent-x" + + +@pytest.mark.parametrize("running, exit_code, message", [ + ("db\n", 0, "No agent container found"), + ("", 1, "Failed to list containers: boom"), +]) +def test_no_agent_container_is_an_error(running, exit_code, message): + with pytest.raises(RuntimeError, match=message): + asyncio.run(find_agent_container(_sandbox(running, exit_code=exit_code))) diff --git a/tst/util/exec_scripts.py b/tst/util/exec_scripts.py new file mode 100644 index 00000000..bb54f28b --- /dev/null +++ b/tst/util/exec_scripts.py @@ -0,0 +1,9 @@ +"""What a recording fake sandbox saw a ``sudo bash -c`` exec run.""" + + +def script_run(args: tuple[str, ...]) -> str: + """The script of ``("sudo", "bash", "-c", script, name, *params)``, each ``"$n"`` replaced by its param.""" + script = args[3] + for n, param in enumerate(args[5:], 1): + script = script.replace(f'"${n}"', param) + return script