From e28d05f357dc6084fcb204c42673e2e3bdd1e686 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sat, 7 Nov 2026 09:25:07 -0800 Subject: [PATCH 01/12] feat(computer): Windows disks and their bases live in the image store, as Linux disks do A composed Windows disk is pushed to the env's image store under its key and recorded pinned; a compose of the same inputs on any machine reuses it, and a deploy pulls it. A base is pushed under a tag of its recipe, with the key to its admin, so a machine without it pulls it to compose a layer instead of installing Windows, and fetches no ISO to find it. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/computers/README.md | 17 +-- .../env/envs/computer/utils/composer.py | 55 ++++---- .../env/envs/computer/utils/windows_disk.py | 119 +++++++++++++----- tst/unit/env/envs/test_computer_windows.py | 80 ++++++++---- 4 files changed, 183 insertions(+), 88 deletions(-) diff --git a/src/agent_env/computers/README.md b/src/agent_env/computers/README.md index c5da91ac8..f0d2390c8 100644 --- a/src/agent_env/computers/README.md +++ b/src/agent_env/computers/README.md @@ -150,19 +150,20 @@ the desktop signs in as at every boot; `setup.ps1` runs once at `agentadmin`'s f setup that fails is read back over ssh at once (`setup-failed`), then turns off what would get in an agent's way (updates, the lock screen, notifications, Widgets, Copilot, first-run pages, the page file and the firewall, behind QEMU's network, which reaches the guest at the ports the launcher forwards alone), installs the pinned Python with Tk -and `agentenv-session`, and switches the sign-in to `user`. Setup takes about 13 minutes. The installed disk is kept -in this machine's Docker as the base's image, `disk/base.img` in a layer of its own under an empty overlay of it, with -the date its 90-day evaluation expires; a compose installs a base afresh once fewer than 7 days are left, and every -input of an install (the ISO's sha256 and the recipe) is part of its key, so a newer ISO installs a new base. Name it -as `windows-11`, or by its id, `@local/agentenv-framework/computers/windows-11`. +and `agentenv-session`, and switches the sign-in to `user`. Setup takes about 13 minutes. The installed disk, trimmed, +becomes the base's image, `disk/base.img` in a layer of its own under an empty overlay of it, with the date its 90-day +evaluation expires, the ISO's sha256 and the key to `agentadmin`; it's pushed to the env's image store, as Linux disks +are, under a tag of its recipe, so another machine composing on it pulls it rather than installing Windows, and fetches +no ISO. A compose installs a base afresh once fewer than 7 days of its evaluation are left, from the ISO Microsoft's +link serves then. Name it as `windows-11`, or by its id, `@local/agentenv-framework/computers/windows-11`. A Windows computer env is composed on the same machine (`env/envs/computer/utils/windows_disk.py`): the base booted on an overlay kept in a folder of its own, each layer installed over ssh as `agentadmin` by a PowerShell script composing writes, as its `[install.windows]` says (its files under `C:\agentenv\drivers\`, its downloads checked by sha256, its venv on the pinned Python, its PowerShell run scripts, elevated, and its services), then shut down and -packed onto the base's image, so every computer composed on a base shares one copy of it. A composed disk stays in this -machine's Docker, as a VM bundle stays on its Mac, under a tag of its key, which a compose of the same inputs reuses -while it's there; gate 3 probes it at full, and one that fails is removed. At the session user's sign-in, +packed onto the base's image, so every computer composed on a base shares one copy of it, and pushed to the env's image +store under its key, pinned, as a Linux disk is; a compose of the same inputs on any machine reuses it, and a deploy +pulls it. Gate 3 probes it at full. At the session user's sign-in, `agentenv-session`, run windowless by `pythonw`, reads the deploy's environment from the NoCloud seed, which the launcher attaches as a removable USB drive, since Windows mounts a FAT filesystem with no partition table on removable media alone, and starts each layer's services in that session, again whenever one exits. A Windows computer boots diff --git a/src/agent_env/env/envs/computer/utils/composer.py b/src/agent_env/env/envs/computer/utils/composer.py index 6caa6d94c..5ba61a48f 100644 --- a/src/agent_env/env/envs/computer/utils/composer.py +++ b/src/agent_env/env/envs/computer/utils/composer.py @@ -176,7 +176,7 @@ def left(form: str, arch: str, record: dict | None) -> bool: if status == "unverified": return bool(boot.native_links(provider, arch, form, env.stored_id, env.computer.os.type)) if form == "disk" and env.computer.os.type == "windows": - return not windows_disk.held(record["image"]) or windows_disk.expiring(record.get("expires")) + return windows_disk.expiring(record.get("expires")) if form != "vm_bundle": return False links = boot.native_links(provider, arch, form, env.stored_id) @@ -336,11 +336,11 @@ def _composed_vm_bundle(env: ComputerEnv, base: ComputerArtifact, driver: Comput def _composed_windows_disk(env: ComputerEnv, base: ComputerArtifact, driver: ComputerDriverEnv, arch: str, provider: SandboxProvider, progress: _Progress) -> dict: - """The Windows disk of ``arch`` composed with this machine's Docker and KVM (``windows_disk``), which keeps it: its - base installed from Microsoft's media first unless held, then the driver installed over ssh as a layer on it and - packed under a tag of its key; or one of that key reused, held here already. Then probed at full (gate 3), and - removed when that fails, so no deploy here boots it. All of it holds a lock of its tag, which keeps two composes of - one key from breaking each other: the other waits, then reuses it.""" + """The Windows disk of ``arch``: one of its key the env's image store holds, reused; else composed with this + machine's Docker and KVM (``windows_disk``), on its base, this machine's, the store's or installed from Microsoft's + media, the driver installed over ssh as a layer on it, packed and pushed to the store under its key, as Linux disks + are. Then probed at full (gate 3). All of it holds a lock of its key, which keeps two composes of one key on this + machine from breaking each other: the other waits, then reuses it.""" pair = f"disk/{arch}" with (_attempting(env, "disk", arch) as attempt, tempfile.TemporaryDirectory(prefix="agentenv-compose-") as work): @@ -369,33 +369,32 @@ def say(line: str) -> None: progress.say(f"{pair}: {line}") attempt.stage = "base" - iso = windows_disk.media(say) + store = get_config().get_image_store_for(env.stored_id) + store.ensure_repository(REPOSITORY) + registry = windows_disk.Registry(store, REPOSITORY) disk_gb, user = base.forms.disk.min_disk_gb, base.session.user - made = windows_disk.base(_key(iso=windows_disk.sha256(iso), recipe=_digest(windows_disk.RECIPE), - disk_gb=disk_gb, user=user, form="windows-base", format=FORMAT), - iso, disk_gb, user, say) + made = windows_disk.base(_key(recipe=_digest(windows_disk.RECIPE), disk_gb=disk_gb, user=user, + form="windows-base", format=FORMAT), + lambda: windows_disk.media(say), disk_gb, user, say, registry) script = windows_disk.write(build, [part], settings, {"base": made.image, "layers": [part.folder]}) - key = attempt.key = _key(base=windows_disk.image_id(made.image), provision=script, context=_digest(context), - build_settings=settings, form="disk", arch=arch, format=FORMAT) - image = f"agentenv-windows:{key[7:23]}" + key = attempt.key = _key(base=made.image, base_expires=made.expires.isoformat(timespec="seconds"), + provision=script, context=_digest(context), build_settings=settings, form="disk", + arch=arch, format=FORMAT) + tag, expires = key.removeprefix("sha256:"), made.expires.isoformat(timespec="seconds") attempt.stage = "build" - with holding_locks([f"windows-disk:{image}"], lambda: progress.at( - pair, "waiting", f"waiting for the compose or probe of {image} under way on this machine")): - if windows_disk.held(image): - progress.at(pair, "reusing", "reusing the disk of the same key, composed on this machine earlier") - result = {"status": "unverified", "key": key, "image": image, "reused": True, - "expires": made.expires.isoformat(timespec="seconds"), "built_at": now()} + with holding_locks([f"windows-disk:{key}"], lambda: progress.at( + pair, "waiting", "waiting for the compose of the same disk under way on this machine")): + if (pinned := registry.served(tag)) is not None: + progress.at(pair, "reusing", "reusing the disk of the same key, composed earlier") + result = {"status": "unverified", "key": key, "image": pinned, "reused": True, "expires": expires, + "built_at": now()} else: progress.at(pair, "composing", "composing on this machine's Docker") - windows_disk.compose(made, build, image, {KEY_LABEL: key, windows_disk.EXPIRES_LABEL: - made.expires.isoformat(timespec="seconds")}, say) - result = {"status": "unverified", "key": key, "image": image, "reused": False, - "builder": "LocalSandboxProvider", "expires": made.expires.isoformat(timespec="seconds"), - "built_at": now()} - probed = _probed(env, driver, "disk", arch, result, provider, progress) - if probed["status"] == "failed": - windows_disk.remove(image) - return probed + pinned = windows_disk.compose(made, build, f"agentenv-windows:{key[7:23]}", + {KEY_LABEL: key, windows_disk.EXPIRES_LABEL: expires}, say, registry, tag) + result = {"status": "unverified", "key": key, "image": pinned, "reused": False, + "builder": "LocalSandboxProvider", "expires": expires, "built_at": now()} + return _probed(env, driver, "disk", arch, result, provider, progress) return attempt.result diff --git a/src/agent_env/env/envs/computer/utils/windows_disk.py b/src/agent_env/env/envs/computer/utils/windows_disk.py index c598c8850..bf0af4d1a 100644 --- a/src/agent_env/env/envs/computer/utils/windows_disk.py +++ b/src/agent_env/env/envs/computer/utils/windows_disk.py @@ -26,15 +26,18 @@ from collections.abc import Callable, Iterator from dataclasses import dataclass from datetime import datetime, timedelta, timezone -from pathlib import Path, PureWindowsPath +from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Any from agent_env.artifact.artifacts.computer import DISK_BASE_FILE, DISK_FILE, DISK_OWNER +from agent_env.artifact.artifacts.docker_image import docker_login, push_local_image from agent_env.config.paths import cache_root from agent_env.env.envs.computer.utils.install import WindowsInstall -from agent_env.providers.sandbox_providers.host_images import SOURCES_LABEL, present +from agent_env.providers.sandbox_providers.host_images import present from agent_env.providers.sandbox_providers.local_docker.cli import docker from agent_env.providers.sandbox_providers.local_docker.disk import KVM, launcher, machine_image +from agent_env.store.image_store.image_contents import labels +from agent_env.store.image_store.registry_api import NoSuchImageError, pin_digest from agent_env.store.local_state import holding_locks from agent_env.utils.docker.build import build_image @@ -51,6 +54,11 @@ #: The fewest days of its evaluation a base may have left, as a deploy needs some. RENEW_DAYS = 7 EXPIRES_LABEL, BUILD_LABEL, KEY_LABEL = "dev.agentenv.windows.expires", "dev.agentenv.windows.build", "dev.agentenv.key" +#: The sha256 of the ISO a base was installed from. +ISO_LABEL = "dev.agentenv.windows.iso" +#: Where a base's image keeps the key to its admin, which composing a layer on it takes over ssh. The image is in the +#: user's own image store, and the guest's ssh is reached only on this machine's loopback while a layer is composed. +ADMIN_KEY = PurePosixPath("/disk/admin.key") #: How long an install, and a compose, may take, and how long the session user's first sign-in is left to settle. INSTALL_TIMEOUT_S, COMPOSE_TIMEOUT_S, SETTLE_S = 3600, 3600, 60 #: How long a guest may take to power off once told. @@ -67,13 +75,45 @@ "once; it's installed for evaluation under Microsoft's terms and runs 90 days from each install") +@dataclass(frozen=True) +class Registry: + """The env's image store as Windows disks are kept in it: by tag in ``repository``, pinned by digest, as Linux + disks are.""" + + store: Any + repository: str + + def served(self, tag: str) -> str | None: + """``tag``'s ref, pinned to the digest the store serves for it now; None when it serves none.""" + ref = self.store.image_ref(self.repository, tag) + try: + return pin_digest(ref, self.store.auth(ref)) + except NoSuchImageError: + return None + + def labels(self, pinned: str) -> dict[str, str]: + return labels(pinned, "amd64", self.store.auth(pinned)) + + def push(self, local: str, tag: str) -> str: + """``local`` pushed as ``tag``; its ref there, pinned.""" + ref = self.store.image_ref(self.repository, tag) + push_local_image(local, ref, self.store) + return pin_digest(ref, self.store.auth(ref)) + + def pull(self, pinned: str) -> None: + docker_login(self.store.auth(pinned)) + docker("pull", "-q", pinned) + + @dataclass(frozen=True) class Base: - """A base on this machine: its image's tag, the folder of its admin's key, and when its evaluation expires.""" + """A base: its image's tag on this machine, the folder of its admin's key, when its evaluation expires, and, for one + the image store holds that this machine doesn't yet, its ref there, pulled when a layer is composed on it.""" image: str keys: Path expires: datetime + pinned: str | None = None @dataclass(frozen=True) @@ -122,29 +162,62 @@ def sha256(path: Path) -> str: return hashed.hexdigest() -def base(key: str, iso: Path, disk_gb: int, user: str, say: Callable[[str], None]) -> Base: - """The base of ``key`` on this machine, installed from ``iso`` first when there's none, its key is gone, or its - evaluation has fewer than ``RENEW_DAYS`` left; one install of a key runs at once, and another waits for it.""" +def base(key: str, iso: Callable[[], Path], disk_gb: int, user: str, say: Callable[[str], None], + registry: Registry) -> Base: + """The base of ``key``: this machine's; else the one the image store holds for ``key``, pulled, with the key to its + admin it carries; else Windows installed from the ISO ``iso`` gives, and pushed there. One with fewer than + ``RENEW_DAYS`` of its evaluation left is installed afresh. One install of a key runs at once on a machine, and + another waits for it.""" image, keys = f"agentenv-windows-base:{key[7:23]}", cache_root() / "windows" / "bases" / key[7:23] + tag = f"windows-base-{key[7:23]}" with holding_locks([f"windows-base:{key}"], lambda: say("waiting for the install of the same base under way")): if present(image) and (keys / "id").exists(): expires = datetime.fromisoformat(_label(image, EXPIRES_LABEL)) - if expires - datetime.now(timezone.utc) >= timedelta(days=RENEW_DAYS): + if not expiring(expires.isoformat()): return Base(image, keys, expires) say(f"its base's evaluation expires {expires:%Y-%m-%d}; installing it afresh") + elif (pinned := registry.served(tag)) is not None and not expiring( + stored := registry.labels(pinned).get(EXPIRES_LABEL)): + return Base(image, keys, datetime.fromisoformat(stored), pinned) shutil.rmtree(keys, ignore_errors=True) keys.mkdir(parents=True) subprocess.run(["ssh-keygen", "-q", "-t", "ed25519", "-N", "", "-C", f"agentenv {image}", "-f", str(keys / "id")], check=True) + media = iso() with _working(f"base-{key[7:23]}") as work: - facts = _installed(work, iso, keys, disk_gb, user, say) + facts = _installed(work, media, keys, disk_gb, user, say) say("packing the base") expires = datetime.now(timezone.utc) + timedelta(minutes=facts["grace_minutes"]) + shutil.copy(keys / "id", work / ADMIN_KEY.name) _pack(work, image, {KEY_LABEL: key, EXPIRES_LABEL: expires.isoformat(timespec="seconds"), - BUILD_LABEL: facts["build"]}, base_image=None) + BUILD_LABEL: facts["build"], ISO_LABEL: sha256(media)}, base_image=None) + say("pushing the base to the image store") + registry.push(image, tag) return Base(image, keys, expires) +def _here(base_: Base, registry: Registry, say: Callable[[str], None]) -> None: + """``base_`` on this machine, with the key to its admin: pulled from the image store when it's the store's.""" + if base_.pinned is None or (present(base_.image) and (base_.keys / "id").exists()): + return + say(f"pulling its base from the image store, {base_.pinned}") + registry.pull(base_.pinned) + docker("tag", base_.pinned, base_.image) + _take_admin_key(base_.image, base_.keys) + + +def _take_admin_key(image: str, keys: Path) -> None: + """The key to the admin of the base ``image`` holds, written to ``keys``, as composing a layer on it needs.""" + shutil.rmtree(keys, ignore_errors=True) + keys.mkdir(parents=True) + container = docker("create", image, "none") + try: + docker("cp", f"{container}:{ADMIN_KEY}", str(keys / "id")) + finally: + subprocess.run(["docker", "rm", container], capture_output=True) + (keys / "id").chmod(0o600) + + def _installed(work: Path, iso: Path, keys: Path, disk_gb: int, user: str, say: Callable[[str], None]) -> dict: """Windows installed from ``iso`` onto ``work/disk.img`` by the launcher, answered as the recipe says: then booted until its session user has signed in and settled, and shut down; what it reports of itself.""" @@ -187,9 +260,12 @@ def _installed(work: Path, iso: Path, keys: Path, disk_gb: int, user: str, say: return facts -def compose(base_: Base, build: Path, image: str, labels: dict[str, str], say: Callable[[str], None]) -> None: +def compose(base_: Base, build: Path, image: str, labels: dict[str, str], say: Callable[[str], None], + registry: Registry, tag: str) -> str: """``image``, a disk composed on ``base_``: the base booted on an overlay kept in a folder of this machine, ``build`` - copied in and its ``provision.ps1`` run as the admin over ssh, then shut down and packed onto the base's image.""" + copied in and its ``provision.ps1`` run as the admin over ssh, then shut down, packed onto the base's image and + pushed to the image store as ``tag``; its ref there, pinned. A base the store holds is pulled first.""" + _here(base_, registry, say) with _working(f"disk-{image.rpartition(':')[2]}") as work: out = work / "out" out.mkdir() @@ -221,11 +297,8 @@ def compose(base_: Base, build: Path, image: str, labels: dict[str, str], say: C (out / "guest").write_text("windows") say("packing the disk") _pack(out, image, labels, base_image=base_.image) - - -def held(image: str) -> bool: - """Whether this machine's Docker holds ``image``.""" - return present(image) + say("pushing the disk to the image store") + return registry.push(image, tag) def expiring(expires: str | None) -> bool: @@ -234,16 +307,6 @@ def expiring(expires: str | None) -> bool: return expires is None or datetime.fromisoformat(expires) - datetime.now(timezone.utc) < timedelta(days=RENEW_DAYS) -def image_id(image: str) -> str: - """The id of ``image`` in this machine's Docker, which a base installed afresh under its tag changes.""" - return docker("image", "inspect", "-f", "{{.Id}}", image) - - -def remove(image: str) -> None: - """``image`` removed from this machine's Docker, so no deploy here boots it.""" - subprocess.run(["docker", "image", "rm", "-f", image], capture_output=True) - - def write(build: Path, parts: list[Part], build_settings: dict[str, Any], image: dict) -> str: """Write ``build``'s ``provision.ps1``, which installs ``parts`` from ``build/context``, their services for agentenv-session, ``build_settings`` as every service's ``AGENTENV_SETTING_`` and ``image`` as what the disk @@ -398,14 +461,14 @@ def _pack(folder: Path, image: str, labels: dict[str, str], base_image: str | No check=True, capture_output=True) (folder / "guest").write_text("windows") dockerfile = (f"FROM scratch\nCOPY {owner} base.img /{DISK_BASE_FILE}\n" + f"COPY --chmod=600 {ADMIN_KEY.name} {ADMIN_KEY}\n" f"COPY {owner} disk.img guest /{Path(DISK_FILE).parent}/\n") else: dockerfile = f"FROM {base_image}\nCOPY {owner} disk.img guest /{Path(DISK_FILE).parent}/\n" for name in ("answers", "out"): shutil.rmtree(folder / name, ignore_errors=True) (folder / "Dockerfile").write_text(dockerfile) - # Labelled as built here, so a deploy boots this machine's image and pulls none of its name. - build_image(folder / "Dockerfile", folder, image, platform="linux/amd64", labels={**labels, SOURCES_LABEL: image}) + build_image(folder / "Dockerfile", folder, image, platform="linux/amd64", labels=labels) @contextlib.contextmanager diff --git a/tst/unit/env/envs/test_computer_windows.py b/tst/unit/env/envs/test_computer_windows.py index 04b263a7a..aaf6196b2 100644 --- a/tst/unit/env/envs/test_computer_windows.py +++ b/tst/unit/env/envs/test_computer_windows.py @@ -4,6 +4,7 @@ the env's computer is a disk of the base's form. A Windows base is amd64, disk-only and started by agentenv-session.""" import json +from types import SimpleNamespace from datetime import datetime, timedelta, timezone from pathlib import Path, PureWindowsPath @@ -43,37 +44,49 @@ def disk_image_platforms(self): return self.platforms +class FakeRegistry: + """The env's image store as Windows disks are kept in it: what it serves, by tag, pinned.""" + + def __init__(self): + self.held = {} + + def served(self, tag): + return self.held.get(tag) + + def push(self, local, tag): + self.held[tag] = f"registry.test/agentenv-computers:{tag}@sha256:{len(self.held):064x}" + return self.held[tag] + + class FakeDocker: - """windows_disk on this machine's Docker: bases installed and disks composed, as images it holds.""" + """windows_disk on one machine's Docker: bases installed and disks composed and pushed to the image store.""" - def __init__(self, tmp_path: Path): - self.tmp, self.images, self.installs, self.composed = tmp_path, set(), [], [] + def __init__(self, tmp_path: Path, registry: FakeRegistry): + self.tmp, self.registry, self.bases, self.installs, self.composed = tmp_path, registry, {}, [], [] - def base(self, key, iso, disk_gb, user, say): - image = f"agentenv-windows-base:{key[7:23]}" - if image not in self.images: - self.installs.append((key, iso, disk_gb, user)) - self.images.add(image) - return windows_disk.Base(image, self.tmp / "keys", datetime.now(timezone.utc) + timedelta(days=89)) + def base(self, key, iso, disk_gb, user, say, registry): + if key not in self.bases: + self.installs.append((key, iso(), disk_gb, user)) + self.bases[key] = datetime.now(timezone.utc) + timedelta(days=89) + return windows_disk.Base(f"agentenv-windows-base:{key[7:23]}", self.tmp / "keys", self.bases[key]) - def compose(self, base, build, image, labels, say): + def compose(self, base, build, image, labels, say, registry, tag): self.composed.append((base.image, (build / "provision.ps1").read_bytes().decode(), sorted(p.relative_to(build).as_posix() for p in build.rglob("*") if p.is_file()), labels)) - self.images.add(image) + return registry.push(image, tag) @pytest.fixture def docker(local_stores, cli_routing, monkeypatch, tmp_path): - fake = FakeDocker(tmp_path) + fake = FakeDocker(tmp_path, FakeRegistry()) iso = tmp_path / "windows.iso" iso.write_bytes(b"iso") monkeypatch.setattr(windows_disk, "media", lambda say: iso) - monkeypatch.setattr(windows_disk, "sha256", lambda path: "ab" * 32) monkeypatch.setattr(windows_disk, "base", fake.base) monkeypatch.setattr(windows_disk, "compose", fake.compose) - monkeypatch.setattr(windows_disk, "held", lambda image: image in fake.images) - monkeypatch.setattr(windows_disk, "image_id", lambda image: f"sha256:{image}") - monkeypatch.setattr(windows_disk, "remove", fake.images.discard) + monkeypatch.setattr(windows_disk, "Registry", lambda store, repository: fake.registry) + monkeypatch.setattr(composer, "get_config", lambda: SimpleNamespace( + get_image_store_for=lambda stored_id: SimpleNamespace(ensure_repository=lambda repository: None))) monkeypatch.setattr(composer, "_probed", lambda env, driver, form, arch, result, provider, progress: { **result, "status": "ready"}) return fake @@ -118,7 +131,8 @@ def test_a_windows_env_composes_a_thin_disk_on_its_base_here_the_driver_installe env = _put(_driver(tmp_path / "desk"), FakeLinux(), theme="dark") built = env.metadata["composed"]["disk"]["amd64"] - assert built["status"] == "ready" and built["image"].startswith("agentenv-windows:") and not built["reused"] + assert built["status"] == "ready" and not built["reused"] + assert built["image"] == docker.registry.held[built["key"].removeprefix("sha256:")] (installed,) = docker.installs assert installed[1:] == (tmp_path / "windows.iso", 64, "user") ((base, script, files, labels),) = docker.composed @@ -143,18 +157,16 @@ def test_a_windows_env_composes_a_thin_disk_on_its_base_here_the_driver_installe assert computer.forms.disk.image is None and env.metadata["computer"]["disks"] == {"amd64": built["image"]} -def test_a_compose_of_the_same_inputs_reuses_the_disk_this_machine_holds_and_composes_one_it_lost(docker, tmp_path): +def test_a_compose_of_the_same_inputs_reuses_the_disk_the_image_store_holds(docker, tmp_path): driver = _driver(tmp_path / "desk") first = _put(driver, FakeLinux()) image = first.metadata["composed"]["disk"]["amd64"]["image"] again = _put(driver, FakeLinux()) - assert again.metadata["composed"]["disk"]["amd64"]["reused"] and len(docker.composed) == 1 - assert composer.pending(again, FakeLinux()) == [] - docker.images.discard(image) - assert composer.pending(again, FakeLinux()) == [("disk", "amd64")] - assert len(docker.installs) == 1 + built = again.metadata["composed"]["disk"]["amd64"] + assert built["reused"] and built["image"] == image and len(docker.composed) == 1 and len(docker.installs) == 1 + assert composer.pending(again, FakeLinux()) == [] def test_a_machine_that_boots_no_windows_disk_composes_none(docker, tmp_path): @@ -229,11 +241,31 @@ def test_a_base_is_reused_until_its_evaluation_nears_its_end_then_installed_afre monkeypatch.setattr(windows_disk, "_installed", lambda *args: installed.append(args) or { "grace_minutes": 90 * 24 * 60, "build": "26200.6584"}) monkeypatch.setattr(windows_disk, "_pack", lambda folder, image, labels, base_image: None) + monkeypatch.setattr(windows_disk, "sha256", lambda path: "ab" * 32) + registry = FakeRegistry() - base = windows_disk.base(key, tmp_path / "windows.iso", 64, "user", lambda line: None) + base = windows_disk.base(key, lambda: tmp_path / "windows.iso", 64, "user", lambda line: None, registry) assert len(installed) == installs and base.image == f"agentenv-windows-base:{key[7:23]}" assert (base.expires - datetime.now(timezone.utc)).days == (days_left - 1 if not installs else 89) + assert list(registry.held) == ([f"windows-base-{key[7:23]}"] if installs else []) + + +def test_a_machine_without_the_base_takes_the_image_stores_unless_its_evaluation_nears_its_end(monkeypatch, tmp_path): + """Found without the ISO, which only an install fetches; pulled only when a layer is composed on it.""" + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path)) + key = "sha256:" + "34" * 32 + registry = FakeRegistry() + pinned = registry.push("agentenv-windows-base:x", f"windows-base-{key[7:23]}") + expires = datetime.now(timezone.utc) + timedelta(days=60) + registry.labels = lambda ref: {windows_disk.EXPIRES_LABEL: expires.isoformat(timespec="seconds")} + monkeypatch.setattr(windows_disk, "present", lambda image: False) + fetched = [] + + base = windows_disk.base(key, lambda: fetched.append("iso"), 64, "user", lambda line: None, registry) + + assert base.pinned == pinned and base.image == f"agentenv-windows-base:{key[7:23]}" and fetched == [] + assert (base.expires - datetime.now(timezone.utc)).days == 59 def test_a_windows_computers_display_is_1920x1080_for_now(local_stores, cli_routing): From 40b65ff597f43f9aab91bc983945703e62c6242a Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sat, 7 Nov 2026 09:33:15 -0800 Subject: [PATCH 02/12] feat(computer): boot Windows disks on modal_vm A thin disk is carried whole: its base, exported once for each base, its layer and its guest file, compressed and mounted beside each other; the launcher expands the base beside the disk. Windows gets a longer ready timeout. Co-Authored-By: Claude Opus 5.5 --- .../computers/disk/linux/launcher/launch | 5 ++ src/agent_env/env/envs/computer/utils/boot.py | 10 ++- .../sandbox_providers/local_docker/disk.py | 68 +++++++++++++++---- .../sandbox_providers/modal_vm_sandbox.py | 21 ++++-- tst/unit/env/envs/test_computer_env.py | 2 +- tst/unit/env/envs/test_disk_recipe.py | 57 +++++++++++++--- .../modal_create_signature_test.py | 3 +- .../modal_vm_sandbox_test.py | 30 ++++++-- 8 files changed, 157 insertions(+), 39 deletions(-) diff --git a/src/agent_env/computers/disk/linux/launcher/launch b/src/agent_env/computers/disk/linux/launcher/launch index d27420a6b..d3acc6516 100755 --- a/src/agent_env/computers/disk/linux/launcher/launch +++ b/src/agent_env/computers/disk/linux/launcher/launch @@ -8,6 +8,8 @@ # and, under KVM, Hyper-V enlightenments. # VM_DISK the disk, /disk/disk.img by default, which may be backed by another beside it; one ending .zst is # expanded onto this machine's disk first +# VM_BASE the base of a thin VM_DISK ending .zst, compressed too, which a provider carries beside it: expanded +# beside the expanded disk, as the base.img it's backed by # VM_ENV_ the computer's variable NAME, its value base64'd, for its NoCloud seed (CIDATA; see seed) # VM_PORTS guest TCP ports, each served on the same port here by socat, whose listen backlog takes a burst of # connections, in front of QEMU's forward on 127.0.0.1 at the port plus 10000, whose backlog is 1; @@ -111,6 +113,9 @@ trap 'kill "$(cat "$control/qemu.pid" 2>/dev/null)" 2>/dev/null; exit 143' TERM if [[ "$disk" == *.zst ]]; then zstd -q -d -f "$disk" -o "$state/disk.img" disk=$state/disk.img + if [ -n "${VM_BASE:-}" ]; then + zstd -q -d -f "$VM_BASE" -o "$state/base.img" + fi fi boot() { diff --git a/src/agent_env/env/envs/computer/utils/boot.py b/src/agent_env/env/envs/computer/utils/boot.py index d8fe7cf1e..50d918483 100644 --- a/src/agent_env/env/envs/computer/utils/boot.py +++ b/src/agent_env/env/envs/computer/utils/boot.py @@ -33,6 +33,9 @@ #: How long a computer may take from its container starting to its driver saying it's ready. READY_TIMEOUT_S = 180.0 +#: How long a Windows computer may take: its first boot from a composed disk signs its session user in, and a provider +#: that carries its disk compressed expands the base first. +WINDOWS_READY_TIMEOUT_S = 600.0 #: How much longer than the slowest starting app's own readiness timeout a computer may take, for the rest of it first. APP_READY_MARGIN_S = 30.0 #: How long one computer-ready status may take: the driver's own checks, then every app's server at once. @@ -192,9 +195,10 @@ async def discard(sandbox: Sandbox) -> None: def ready_timeout(apps: list[ComputerAppEnv], os_type: str) -> float: - """How long a computer of ``os_type`` and ``apps`` may take to be ready: ``READY_TIMEOUT_S``, or longer when an app - that starts with it there gives itself longer.""" - return max([READY_TIMEOUT_S, *(launch.ready.timeout_s + APP_READY_MARGIN_S for app in apps + """How long a computer of ``os_type`` and ``apps`` may take to be ready: ``READY_TIMEOUT_S``, Windows' + ``WINDOWS_READY_TIMEOUT_S``, or longer when an app that starts with it there gives itself longer.""" + least = WINDOWS_READY_TIMEOUT_S if os_type == "windows" else READY_TIMEOUT_S + return max([least, *(launch.ready.timeout_s + APP_READY_MARGIN_S for app in apps if (launch := app.launch(os_type)) and launch.at == "boot" and launch.ready)]) diff --git a/src/agent_env/providers/sandbox_providers/local_docker/disk.py b/src/agent_env/providers/sandbox_providers/local_docker/disk.py index 746825943..87d5402a5 100644 --- a/src/agent_env/providers/sandbox_providers/local_docker/disk.py +++ b/src/agent_env/providers/sandbox_providers/local_docker/disk.py @@ -8,6 +8,7 @@ import os import subprocess import tempfile +from dataclasses import dataclass from pathlib import Path, PurePosixPath from agent_env.artifact.artifacts.computer import DISK_BASE_FILE, DISK_FILE, DISK_GUEST_FILE @@ -17,7 +18,7 @@ from agent_env.env.published_images import LAUNCHER_IMAGE, published_pinned from agent_env.providers.sandbox_providers.disk_launcher import launcher_variables from agent_env.providers.sandbox_providers.host_images import host_platform, present -from agent_env.providers.sandbox_providers.local_docker.cli import docker +from agent_env.providers.sandbox_providers.local_docker.cli import DockerFailed, docker from agent_env.store.local_state import holding_locks from agent_env.utils.docker.build import build_image @@ -62,37 +63,74 @@ def launcher_args(ports: list[int], env: dict[str, str], cpu: float, memory: int for arg in ("-e", f"{name}={value}"))] -def exported_disk(image: str, platform: str) -> Path: - """``image``'s ``DISK_FILE`` compressed with zstd in this machine's cache, exported from its image store by this - machine's Docker unless an export of it is there already: once for each composed disk, by its ref, whose tag is its - composer's key. ``platform`` is the disk's own, which this machine's Docker may not be.""" +@dataclass(frozen=True) +class Exported: + """A disk exported for a provider elsewhere to carry: its ``DISK_FILE``, compressed; for a thin disk, its base, + compressed, exported once for each base; and its guest file, when it names one.""" + + disk: Path + base: Path | None = None + guest: Path | None = None + + +def exported_disk(image: str, platform: str) -> Exported: + """``image``'s disk compressed with zstd in this machine's cache, exported from its image store by this machine's + Docker unless an export of it is there already: once for each composed disk, by its ref, whose tag is its composer's + key; a thin disk's base once for each base, by its layer, which every disk composed on it shares. ``platform`` is + the disk's own, which this machine's Docker may not be.""" path = cache_root() / "disks" / f"{hashlib.sha256(image.encode()).hexdigest()[:32]}.img.zst" with holding_locks([f"disk-export:{path.name}"]): if not path.exists(): _export(image, platform, path) - return path + base, guest = path.with_name(path.name + ".base"), path.with_name(path.name + ".guest") + return Exported(path, path.with_name(base.read_text()) if base.exists() else None, guest if guest.exists() else None) def _export(image: str, platform: str, path: Path) -> None: """Signed in to the image store first, as a boot's pull is, since ``docker create`` pulls ``image`` when it isn't - here; for ``platform``, as the image's index holds that one alone, which a pull for this machine's wouldn't find.""" + here; for ``platform``, as the image's index holds that one alone, which a pull for this machine's wouldn't find. + The disk is placed last, so one that's there was exported whole, with its base and guest file.""" path.parent.mkdir(parents=True, exist_ok=True) docker_login(get_config().get_image_store().auth(image)) container = docker("create", "--platform", platform, image, "disk") try: with tempfile.TemporaryDirectory(prefix="export-", dir=path.parent) as work: - raw, compressed = Path(work) / "disk.img", Path(work) / path.name - docker("cp", f"{container}:/{DISK_FILE}", str(raw)) - with raw.open("rb") as source, compressed.open("wb") as out: - done = subprocess.run(["docker", "run", "--rm", "-i", "--entrypoint", "zstd", launcher(), "-q", "-3", - "-T0", "-c"], stdin=source, stdout=out, stderr=subprocess.PIPE, text=False) - if done.returncode: - raise RuntimeError(f"compressing {image}'s disk failed: {done.stderr.decode().strip()}") - os.replace(compressed, path) + if (guest := _copied(container, DISK_GUEST_FILE, Path(work) / "guest")) is not None: + guest.replace(path.with_name(path.name + ".guest")) + layers = docker("image", "inspect", "-f", "{{len .RootFS.Layers}} {{index .RootFS.Layers 0}}", image) + if int(layers.split()[0] if layers else 1) > 1: + base = path.with_name(f"base-{hashlib.sha256(layers.split()[1].encode()).hexdigest()[:32]}.img.zst") + with holding_locks([f"disk-export:{base.name}"]): + if not base.exists(): + _compressed(container, DISK_BASE_FILE, base, Path(work), image) + path.with_name(path.name + ".base").write_text(base.name) + _compressed(container, DISK_FILE, path, Path(work), image) finally: subprocess.run(["docker", "rm", container], capture_output=True) +def _copied(container: str, file: str, to: Path) -> Path | None: + """``file`` copied out of ``container`` to ``to``; None when the container has no such file.""" + try: + docker("cp", f"{container}:/{file}", str(to)) + except DockerFailed: + return None + return to + + +def _compressed(container: str, file: str, path: Path, work: Path, image: str) -> None: + """``file`` of ``container`` compressed with zstd to ``path``, through ``work``.""" + raw, compressed = work / PurePosixPath(file).name, work / path.name + docker("cp", f"{container}:/{file}", str(raw)) + with raw.open("rb") as source, compressed.open("wb") as out: + done = subprocess.run(["docker", "run", "--rm", "-i", "--entrypoint", "zstd", launcher(), "-q", "-3", "-T0", "-c"], + stdin=source, stdout=out, stderr=subprocess.PIPE, text=False) + if done.returncode: + raise RuntimeError(f"compressing {image}'s {file} failed: {done.stderr.decode().strip()}") + raw.unlink() + os.replace(compressed, path) + + def launcher() -> str: if (pinned := published_pinned(LAUNCHER_IMAGE)) is not None: if not present(pinned): diff --git a/src/agent_env/providers/sandbox_providers/modal_vm_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_vm_sandbox.py index 10057a1b0..f0a60428c 100644 --- a/src/agent_env/providers/sandbox_providers/modal_vm_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_vm_sandbox.py @@ -16,7 +16,7 @@ With a computer's disk form as its image, ``create_vm`` makes a VM sandbox with nested virtualization whose own image is the QEMU launcher, with no Docker in it, booting the disk mounted at create from its export on this machine -(``local_docker.disk.exported_disk``); ``reset_vm`` boots it afresh in the same sandbox. ``[sandbox.providers.modal_vm]`` +(``local_docker.disk.exported_disk``), with a thin disk's base and guest file beside it; ``reset_vm`` boots it afresh in the same sandbox. ``[sandbox.providers.modal_vm]`` takes ``region``, where every sandbox it makes goes, and ``idle_timeout``, in seconds, after which an idle one ends. """ @@ -114,7 +114,8 @@ ''' _KEEPALIVE_CMD = ("sh", "-c", _MAIN_SCRIPT) #: Where a computer's disk is mounted on the launcher's image, compressed; the launcher expands it onto the sandbox's disk. -_MOUNTED_DISK = "/disk/disk.img.zst" +#: A thin disk's base is mounted beside it, compressed too, and expanded beside it; its guest file, beside both. +_MOUNTED_DISK, _MOUNTED_BASE, _MOUNTED_GUEST = "/disk/disk.img.zst", "/disk/base.img.zst", "/disk/guest" #: The memory a disk's sandbox has beyond its VM's: QEMU's own, and the sandbox's. _DISK_HOST_MEMORY_MB = 2048 #: How long a disk's sandbox stays up with no connection open and no command running, when the config sets no @@ -300,8 +301,9 @@ def supports_network_policy(cls, policy: NetworkPolicy) -> bool: @classmethod def disk_image_platforms(cls) -> frozenset[str]: - """Modal's VM sandboxes are amd64, and run QEMU with KVM under nested virtualization.""" - return frozenset({"linux/amd64"}) + """Modal's VM sandboxes are amd64, and run QEMU with KVM under nested virtualization: a Linux disk, or a Windows + one, thin on its base.""" + return frozenset({"linux/amd64", "windows/amd64"}) def __init__(self, app_name: Optional[str] = None, environment_name: Optional[str] = None, region: Optional[str] = None, idle_timeout: Optional[int] = None): @@ -364,11 +366,16 @@ async def create_vm( attribution=attribution, network_policy=network_policy, setup_for_gateway=setup_for_gateway, idle_timeout=self._idle_timeout, experimental_options={"vm_runtime": True}) - (platform,) = self.disk_image_platforms() - disk = await asyncio.to_thread(exported_disk, image, platform) + exported = await asyncio.to_thread(exported_disk, image, "linux/amd64") variables = {**launcher_variables(ports, env or {}, cpu, memory), "VM_DISK": _MOUNTED_DISK} + launcher = (await _launcher()).add_local_file(exported.disk, _MOUNTED_DISK, copy=False) + if exported.base is not None: + launcher = launcher.add_local_file(exported.base, _MOUNTED_BASE, copy=False) + variables["VM_BASE"] = _MOUNTED_BASE + if exported.guest is not None: + launcher = launcher.add_local_file(exported.guest, _MOUNTED_GUEST, copy=False) return await self._created( - (LAUNCH,), (await _launcher()).add_local_file(disk, _MOUNTED_DISK, copy=False), cpu=cpu, + (LAUNCH,), launcher, cpu=cpu, memory=memory + _DISK_HOST_MEMORY_MB, timeout=timeout, ports=ports, attribution=attribution, network_policy=network_policy, setup_for_gateway=False, runtime="vm", experimental_options={"enable_nested_virt": True}, secrets=[modal.Secret.from_dict(variables)], diff --git a/tst/unit/env/envs/test_computer_env.py b/tst/unit/env/envs/test_computer_env.py index a8a363820..6a0b8f50f 100644 --- a/tst/unit/env/envs/test_computer_env.py +++ b/tst/unit/env/envs/test_computer_env.py @@ -1480,7 +1480,7 @@ def test_an_app_that_requires_nothing_still_gets_a_venv_for_its_own_server(base, build["dockerfile"]) assert build["confs"]["agentenv-app-mail.conf"] == MAIL_SERVER assert boot.ready_timeout([app], "linux") == (270.0 if at == "boot" else boot.READY_TIMEOUT_S) - assert boot.ready_timeout([app], "windows") == boot.READY_TIMEOUT_S + assert boot.ready_timeout([app], "windows") == boot.WINDOWS_READY_TIMEOUT_S def test_an_apps_packages_downloads_scripts_and_steps_are_fetched_at_put_and_installed_offline(base, driver, docker, diff --git a/tst/unit/env/envs/test_disk_recipe.py b/tst/unit/env/envs/test_disk_recipe.py index 213a255e0..b816e051c 100644 --- a/tst/unit/env/envs/test_disk_recipe.py +++ b/tst/unit/env/envs/test_disk_recipe.py @@ -267,13 +267,16 @@ def test_the_launcher_fails_rather_than_emulate_where_kvm_is_required_and_cant_r assert emulated.wait(10) == 0 and "-machine virt,accel=tcg" in _logged(calls, "qemu-system-aarch64")[0] -def test_the_launcher_expands_a_compressed_disk_onto_its_state_folder_and_boots_that(launcher, tmp_path): +@pytest.mark.parametrize("base", [None, "/disk/base.img.zst"], ids=["whole", "thin"]) +def test_the_launcher_expands_a_compressed_disk_onto_its_state_folder_and_boots_that(launcher, tmp_path, base): + """A thin disk's base, carried beside it, is expanded beside it, as the base.img it's backed by.""" launch, calls = launcher - done = launch(VM_DISK="/disk/disk.img.zst", QEMU_RUNS_S="0.1") + done = launch(VM_DISK="/disk/disk.img.zst", QEMU_RUNS_S="0.1", **({"VM_BASE": base} if base else {})) assert done.wait(10) == 0 - assert _logged(calls, "zstd") == [f"zstd -q -d -f /disk/disk.img.zst -o {tmp_path}/state/disk.img"] + assert _logged(calls, "zstd") == [f"zstd -q -d -f /disk/disk.img.zst -o {tmp_path}/state/disk.img"] + ( + [f"zstd -q -d -f {base} -o {tmp_path}/state/base.img"] if base else []) assert f"-b {tmp_path}/state/disk.img -F qcow2" in _logged(calls, "qemu-img")[0] @@ -568,9 +571,11 @@ def test_a_disk_is_exported_compressed_into_the_cache_once_for_each_ref(tmp_path def docker(*args): ran.append(args) + if args[0] == "cp" and args[1].endswith("/disk/guest"): + raise DockerFailed("Could not find the file /disk/guest in container container1", 1) if args[0] == "cp": Path(args[2]).write_bytes(b"disk of " + args[1].encode()) - return "container1" if args[0] == "create" else "" + return "container1" if args[0] == "create" else "1 sha256:linux" if args[0] == "image" else "" def run(argv, stdin=None, stdout=None, **kwargs): if argv[:2] == ["docker", "run"]: @@ -584,10 +589,46 @@ def run(argv, stdin=None, stdout=None, **kwargs): exported = [local_disk.exported_disk(image, "linux/amd64") for image in (first, first, first.replace("1", "2"))] - assert exported[0] == exported[1] != exported[2] and exported[0].parent == tmp_path / "cache" / "agentenv" / "disks" - assert exported[0].read_bytes() == b"zstd:disk of container1:/disk/disk.img" and exported[0].name.endswith(".img.zst") - assert [args[0] for args in ran] == ["login", "create", "cp", "login", "create", "cp"] + assert exported[0] == exported[1] != exported[2] and exported[0].disk.parent == tmp_path / "cache" / "agentenv" / "disks" + assert exported[0].disk.read_bytes() == b"zstd:disk of container1:/disk/disk.img" + assert exported[0].disk.name.endswith(".img.zst") and exported[0].base is exported[0].guest is None + assert [args[0] for args in ran] == ["login", "create", "cp", "image", "cp"] * 2 assert ran[0] == ("login", f"credentials for {first}") and ran[1][1:] == ("--platform", "linux/amd64", first, "disk") assert compressed[0] == ["docker", "run", "--rm", "-i", "--entrypoint", "zstd", "agentenv-qemu-launcher:src-1", "-q", "-3", "-T0", "-c"] and len(compressed) == 2 - assert sorted(p.name for p in exported[0].parent.iterdir()) == sorted({exported[0].name, exported[2].name}) + assert sorted(p.name for p in exported[0].disk.parent.iterdir()) == sorted({exported[0].disk.name, + exported[2].disk.name}) + + +def test_a_thin_disk_is_exported_with_its_guest_file_and_its_base_once_for_every_disk_on_that_base(tmp_path, + monkeypatch): + """A Windows disk is a layer on its base, whose first layer every disk composed on it shares.""" + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache")) + monkeypatch.setattr(local_disk, "launcher", lambda: "agentenv-qemu-launcher:src-1") + monkeypatch.setattr(local_disk, "get_config", lambda: SimpleNamespace(get_image_store=lambda: SimpleNamespace( + auth=lambda ref: None))) + monkeypatch.setattr(local_disk, "docker_login", lambda auth: None) + copied = [] + + def docker(*args): + if args[0] == "cp": + copied.append(args[1]) + Path(args[2]).write_bytes(b"windows" if args[1].endswith("guest") else b"copy of " + args[1].encode()) + return "container1" if args[0] == "create" else "3 sha256:base" if args[0] == "image" else "" + + def run(argv, stdin=None, stdout=None, **kwargs): + stdout.write(b"zstd:" + stdin.read()) + return subprocess.CompletedProcess(argv, 0, b"", b"") + + monkeypatch.setattr(local_disk, "docker", docker) + monkeypatch.setattr(local_disk.subprocess, "run", run) + first = "localhost:5000/agentenv-computers:" + "1" * 64 + "@sha256:" + "a" * 64 + + one, two = (local_disk.exported_disk(image, "windows/amd64") for image in (first, first.replace("1", "2"))) + + assert one.disk != two.disk and one.base == two.base and one.base.parent == one.disk.parent + assert one.base.read_bytes() == b"zstd:copy of container1:/disk/base.img" + assert one.disk.read_bytes() == b"zstd:copy of container1:/disk/disk.img" and one.guest.read_text() == "windows" + assert copied == ["container1:/disk/guest", "container1:/disk/base.img", "container1:/disk/disk.img", + "container1:/disk/guest", "container1:/disk/disk.img"] + assert local_disk.exported_disk(first, "windows/amd64") == one and len(copied) == 5 diff --git a/tst/unit/providers/sandbox_providers/modal_create_signature_test.py b/tst/unit/providers/sandbox_providers/modal_create_signature_test.py index 509857c4e..9995a4188 100644 --- a/tst/unit/providers/sandbox_providers/modal_create_signature_test.py +++ b/tst/unit/providers/sandbox_providers/modal_create_signature_test.py @@ -16,6 +16,7 @@ from agent_env.config import get_config, reset_config from agent_env.providers.sandbox_providers import modal_image_build, modal_vm_sandbox from agent_env.providers.sandbox_providers.modal_sandbox import ModalSandboxProvider +from agent_env.providers.sandbox_providers.local_docker.disk import Exported from agent_env.providers.sandbox_providers.modal_vm_sandbox import ModalVmSandboxProvider from agent_env.providers.sandbox_providers.sandbox import NetworkMode, NetworkPolicy from tst.unit.store.fakes import FakeImageStore @@ -62,7 +63,7 @@ def _provider(cls, **kwargs): def _exported_disk(tmp_path, monkeypatch): """A disk's export on this machine, and the launcher's image, without Docker or a registry.""" (tmp_path / "disk.img.zst").write_bytes(b"disk") - monkeypatch.setattr(modal_vm_sandbox, "exported_disk", lambda image, platform: tmp_path / "disk.img.zst") + monkeypatch.setattr(modal_vm_sandbox, "exported_disk", lambda image, platform: Exported(tmp_path / "disk.img.zst")) monkeypatch.setattr(modal_vm_sandbox, "published_digest", lambda image: "sha256:" + "b" * 64) diff --git a/tst/unit/providers/sandbox_providers/modal_vm_sandbox_test.py b/tst/unit/providers/sandbox_providers/modal_vm_sandbox_test.py index 00abfa117..b138eba36 100644 --- a/tst/unit/providers/sandbox_providers/modal_vm_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/modal_vm_sandbox_test.py @@ -421,6 +421,7 @@ async def test_terminate_sandbox_terminates_a_vm_by_id_and_counts_one_modal_no_l from types import SimpleNamespace # noqa: E402 from agent_env.providers.sandbox_providers.disk_launcher import RESET, launcher_variables # noqa: E402 +from agent_env.providers.sandbox_providers.local_docker.disk import Exported # noqa: E402 DISK_IMAGE = "localhost:5000/agentenv-computers:" + "1" * 64 + "@sha256:" + "a" * 64 @@ -429,7 +430,8 @@ async def test_terminate_sandbox_terminates_a_vm_by_id_and_counts_one_modal_no_l def modal_fake(monkeypatch): """Modal's create, with each call's command and arguments, a sandbox's tunnel, the disk's export and the launcher's image recorded; and each secret's variables.""" - made = SimpleNamespace(creates=[], secrets=[], exported=[], launcher=MagicMock(name="launcher")) + made = SimpleNamespace(creates=[], secrets=[], exported=[], launcher=MagicMock(name="launcher"), + export=Exported(Path("/cache/disk.img.zst"))) async def create(*command, **kwargs): made.creates.append((command, kwargs)) @@ -443,7 +445,7 @@ async def create(*command, **kwargs): monkeypatch.setattr(mvs.modal.Sandbox, "_experimental_create", SimpleNamespace(aio=create)) monkeypatch.setattr(mvs.modal.Secret, "from_dict", lambda variables: made.secrets.append(variables) or variables) monkeypatch.setattr(mvs, "exported_disk", lambda image, platform: made.exported.append((image, platform)) - or Path("/cache/disk.img.zst")) + or made.export) monkeypatch.setattr(mvs, "_launcher", AsyncMock(return_value=made.launcher)) monkeypatch.setattr(mvs, "_log_sandbox_started", AsyncMock()) return made @@ -456,8 +458,8 @@ def _vm_provider(**config) -> ModalVmSandboxProvider: return provider -def test_modal_vm_boots_amd64_disks(): - assert ModalVmSandboxProvider.disk_image_platforms() == frozenset({"linux/amd64"}) +def test_modal_vm_boots_amd64_disks_linux_and_windows(): + assert ModalVmSandboxProvider.disk_image_platforms() == frozenset({"linux/amd64", "windows/amd64"}) @pytest.mark.asyncio @@ -483,6 +485,26 @@ async def test_a_disk_boots_in_a_vm_sandbox_whose_image_is_the_launcher_with_the assert sandbox.tunnel_urls == {18765: "https://sb-1-18765.w.modal.host"} and sandbox.type == "modal_vm" +@pytest.mark.asyncio +async def test_a_thin_disk_boots_with_its_base_and_guest_file_mounted_beside_it_and_its_base_expanded_there(modal_fake): + modal_fake.export = Exported(Path("/cache/disk.img.zst"), Path("/cache/base-1.img.zst"), + Path("/cache/disk.img.zst.guest")) + + await _vm_provider().create_vm(image=DISK_IMAGE, env={}, exposed_ports=[18765], cpu=2, memory=4096) + + ((_, kwargs),) = modal_fake.creates + disk = modal_fake.launcher.add_local_file + base = disk.return_value.add_local_file + guest = base.return_value.add_local_file + assert [mount.call_args for mount in (disk, base, guest)] == [ + ((Path("/cache/disk.img.zst"), "/disk/disk.img.zst"), {"copy": False}), + ((Path("/cache/base-1.img.zst"), "/disk/base.img.zst"), {"copy": False}), + ((Path("/cache/disk.img.zst.guest"), "/disk/guest"), {"copy": False})] + assert kwargs["image"] is guest.return_value + assert modal_fake.secrets == [{**launcher_variables([18765], {}, 2, 4096), "VM_DISK": "/disk/disk.img.zst", + "VM_BASE": "/disk/base.img.zst"}] + + @pytest.mark.asyncio async def test_a_configured_region_and_idle_timeout_hold_for_every_sandbox_the_provider_makes(modal_fake): """A computer's, an agent's and a gateway's alike, so they land together; neither is passed by default.""" From d5919b57670773e437f17e8e5149bde55ca866e1 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sat, 7 Nov 2026 09:34:19 -0800 Subject: [PATCH 03/12] test(computer): the thin export's fake Docker removes its container too Co-Authored-By: Claude Opus 5.5 --- tst/unit/env/envs/test_disk_recipe.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tst/unit/env/envs/test_disk_recipe.py b/tst/unit/env/envs/test_disk_recipe.py index b816e051c..98f0436b3 100644 --- a/tst/unit/env/envs/test_disk_recipe.py +++ b/tst/unit/env/envs/test_disk_recipe.py @@ -617,7 +617,8 @@ def docker(*args): return "container1" if args[0] == "create" else "3 sha256:base" if args[0] == "image" else "" def run(argv, stdin=None, stdout=None, **kwargs): - stdout.write(b"zstd:" + stdin.read()) + if argv[:2] == ["docker", "run"]: + stdout.write(b"zstd:" + stdin.read()) return subprocess.CompletedProcess(argv, 0, b"", b"") monkeypatch.setattr(local_disk, "docker", docker) From b9a7d4da0db08db22a642180756203cca028ae13 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sat, 7 Nov 2026 09:39:36 -0800 Subject: [PATCH 04/12] refactor(computer): name the disk image's platform on modal_vm; no fallback for a layer count docker always gives Co-Authored-By: Claude Opus 5.5 --- .../providers/sandbox_providers/local_docker/disk.py | 7 ++++--- .../providers/sandbox_providers/modal_vm_sandbox.py | 4 +++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/local_docker/disk.py b/src/agent_env/providers/sandbox_providers/local_docker/disk.py index 87d5402a5..c318917ff 100644 --- a/src/agent_env/providers/sandbox_providers/local_docker/disk.py +++ b/src/agent_env/providers/sandbox_providers/local_docker/disk.py @@ -97,9 +97,10 @@ def _export(image: str, platform: str, path: Path) -> None: with tempfile.TemporaryDirectory(prefix="export-", dir=path.parent) as work: if (guest := _copied(container, DISK_GUEST_FILE, Path(work) / "guest")) is not None: guest.replace(path.with_name(path.name + ".guest")) - layers = docker("image", "inspect", "-f", "{{len .RootFS.Layers}} {{index .RootFS.Layers 0}}", image) - if int(layers.split()[0] if layers else 1) > 1: - base = path.with_name(f"base-{hashlib.sha256(layers.split()[1].encode()).hexdigest()[:32]}.img.zst") + layers, first = docker("image", "inspect", "-f", "{{len .RootFS.Layers}} {{index .RootFS.Layers 0}}", + image).split() + if int(layers) > 1: + base = path.with_name(f"base-{hashlib.sha256(first.encode()).hexdigest()[:32]}.img.zst") with holding_locks([f"disk-export:{base.name}"]): if not base.exists(): _compressed(container, DISK_BASE_FILE, base, Path(work), image) diff --git a/src/agent_env/providers/sandbox_providers/modal_vm_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_vm_sandbox.py index f0a60428c..76737fd76 100644 --- a/src/agent_env/providers/sandbox_providers/modal_vm_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_vm_sandbox.py @@ -116,6 +116,8 @@ #: Where a computer's disk is mounted on the launcher's image, compressed; the launcher expands it onto the sandbox's disk. #: A thin disk's base is mounted beside it, compressed too, and expanded beside it; its guest file, beside both. _MOUNTED_DISK, _MOUNTED_BASE, _MOUNTED_GUEST = "/disk/disk.img.zst", "/disk/base.img.zst", "/disk/guest" +#: The platform of a disk's image in the image store, a Windows disk's too: its guest file names the OS it boots. +_DISK_IMAGE_PLATFORM = "linux/amd64" #: The memory a disk's sandbox has beyond its VM's: QEMU's own, and the sandbox's. _DISK_HOST_MEMORY_MB = 2048 #: How long a disk's sandbox stays up with no connection open and no command running, when the config sets no @@ -366,7 +368,7 @@ async def create_vm( attribution=attribution, network_policy=network_policy, setup_for_gateway=setup_for_gateway, idle_timeout=self._idle_timeout, experimental_options={"vm_runtime": True}) - exported = await asyncio.to_thread(exported_disk, image, "linux/amd64") + exported = await asyncio.to_thread(exported_disk, image, _DISK_IMAGE_PLATFORM) variables = {**launcher_variables(ports, env or {}, cpu, memory), "VM_DISK": _MOUNTED_DISK} launcher = (await _launcher()).add_local_file(exported.disk, _MOUNTED_DISK, copy=False) if exported.base is not None: From 13736726d9b61c612ca571573256241dced9bf52 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sat, 7 Nov 2026 09:55:48 -0800 Subject: [PATCH 05/12] fix(computer): pull a stored Windows base under its lock, and push a local base the image store lacks Co-Authored-By: Claude Opus 5.5 --- .../env/envs/computer/utils/windows_disk.py | 33 ++++++++---- tst/unit/env/envs/test_computer_windows.py | 50 ++++++++++++++++++- 2 files changed, 71 insertions(+), 12 deletions(-) diff --git a/src/agent_env/env/envs/computer/utils/windows_disk.py b/src/agent_env/env/envs/computer/utils/windows_disk.py index bf0af4d1a..e23bfd161 100644 --- a/src/agent_env/env/envs/computer/utils/windows_disk.py +++ b/src/agent_env/env/envs/computer/utils/windows_disk.py @@ -164,16 +164,19 @@ def sha256(path: Path) -> str: def base(key: str, iso: Callable[[], Path], disk_gb: int, user: str, say: Callable[[str], None], registry: Registry) -> Base: - """The base of ``key``: this machine's; else the one the image store holds for ``key``, pulled, with the key to its - admin it carries; else Windows installed from the ISO ``iso`` gives, and pushed there. One with fewer than - ``RENEW_DAYS`` of its evaluation left is installed afresh. One install of a key runs at once on a machine, and - another waits for it.""" + """The base of ``key``: this machine's, pushed to the image store unless it holds one of ``key``; else the one the + store holds, pulled, with the key to its admin it carries, when a layer is composed on it; else Windows installed + from the ISO ``iso`` gives, and pushed there. One with fewer than ``RENEW_DAYS`` of its evaluation left is installed + afresh. One install of a key runs at once on a machine, and another waits for it.""" image, keys = f"agentenv-windows-base:{key[7:23]}", cache_root() / "windows" / "bases" / key[7:23] tag = f"windows-base-{key[7:23]}" - with holding_locks([f"windows-base:{key}"], lambda: say("waiting for the install of the same base under way")): + with holding_locks([_base_lock(image)], lambda: say("waiting for the install of the same base under way")): if present(image) and (keys / "id").exists(): expires = datetime.fromisoformat(_label(image, EXPIRES_LABEL)) if not expiring(expires.isoformat()): + if registry.served(tag) is None: + say("pushing the base to the image store, which doesn't hold it") + registry.push(image, tag) return Base(image, keys, expires) say(f"its base's evaluation expires {expires:%Y-%m-%d}; installing it afresh") elif (pinned := registry.served(tag)) is not None and not expiring( @@ -197,13 +200,21 @@ def base(key: str, iso: Callable[[], Path], disk_gb: int, user: str, say: Callab def _here(base_: Base, registry: Registry, say: Callable[[str], None]) -> None: - """``base_`` on this machine, with the key to its admin: pulled from the image store when it's the store's.""" - if base_.pinned is None or (present(base_.image) and (base_.keys / "id").exists()): + """``base_`` on this machine, with the key to its admin: pulled from the image store when it's the store's, under + the base's lock, so a compose of another disk on it waits, then finds it.""" + if base_.pinned is None: return - say(f"pulling its base from the image store, {base_.pinned}") - registry.pull(base_.pinned) - docker("tag", base_.pinned, base_.image) - _take_admin_key(base_.image, base_.keys) + with holding_locks([_base_lock(base_.image)], lambda: say("waiting for the pull of the same base under way")): + if present(base_.image) and (base_.keys / "id").exists(): + return + say(f"pulling its base from the image store, {base_.pinned}") + registry.pull(base_.pinned) + docker("tag", base_.pinned, base_.image) + _take_admin_key(base_.image, base_.keys) + + +def _base_lock(image: str) -> str: + return f"windows-base:{image}" def _take_admin_key(image: str, keys: Path) -> None: diff --git a/tst/unit/env/envs/test_computer_windows.py b/tst/unit/env/envs/test_computer_windows.py index aaf6196b2..1cd051a9b 100644 --- a/tst/unit/env/envs/test_computer_windows.py +++ b/tst/unit/env/envs/test_computer_windows.py @@ -4,6 +4,8 @@ the env's computer is a disk of the base's form. A Windows base is amd64, disk-only and started by agentenv-session.""" import json +import threading +import time from types import SimpleNamespace from datetime import datetime, timedelta, timezone from pathlib import Path, PureWindowsPath @@ -228,6 +230,7 @@ def test_a_disk_whose_evaluation_nears_its_end_is_composed_again(docker, tmp_pat @pytest.mark.parametrize("days_left, installs", [(30, 0), (3, 1)]) def test_a_base_is_reused_until_its_evaluation_nears_its_end_then_installed_afresh(monkeypatch, tmp_path, days_left, installs): + """Either way the image store gets it, which a push that failed after an install, or another env's store, lacks.""" monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path)) key = "sha256:" + "12" * 32 keys = tmp_path / "agentenv" / "windows" / "bases" / key[7:23] @@ -248,7 +251,52 @@ def test_a_base_is_reused_until_its_evaluation_nears_its_end_then_installed_afre assert len(installed) == installs and base.image == f"agentenv-windows-base:{key[7:23]}" assert (base.expires - datetime.now(timezone.utc)).days == (days_left - 1 if not installs else 89) - assert list(registry.held) == ([f"windows-base-{key[7:23]}"] if installs else []) + assert list(registry.held) == [f"windows-base-{key[7:23]}"] + + +def test_a_base_this_machine_holds_goes_to_an_image_store_lacking_it_once(monkeypatch, tmp_path): + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path)) + key = "sha256:" + "56" * 32 + keys = tmp_path / "agentenv" / "windows" / "bases" / key[7:23] + keys.mkdir(parents=True) + (keys / "id").write_text("key") + expires = datetime.now(timezone.utc) + timedelta(days=30) + monkeypatch.setattr(windows_disk, "present", lambda image: True) + monkeypatch.setattr(windows_disk, "_label", lambda image, name: expires.isoformat(timespec="seconds")) + registry, pushed = FakeRegistry(), [] + push = registry.push + registry.push = lambda local, tag: pushed.append(local) or push(local, tag) + + for _ in range(2): + windows_disk.base(key, lambda: tmp_path / "windows.iso", 64, "user", lambda line: None, registry) + + assert pushed == [f"agentenv-windows-base:{key[7:23]}"] + + +def test_composes_of_two_disks_on_a_base_the_store_holds_pull_it_and_take_its_key_once(monkeypatch, tmp_path): + """The second waits for the first's pull, then finds the base here.""" + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path)) + keys = tmp_path / "keys" + made, pulls = threading.Event(), [] + + def pull(pinned): + pulls.append(pinned) + time.sleep(0.2) + + monkeypatch.setattr(windows_disk, "present", lambda image: made.is_set()) + monkeypatch.setattr(windows_disk, "docker", lambda *args: None) + monkeypatch.setattr(windows_disk, "_take_admin_key", lambda image, at: (at.mkdir(), (at / "id").write_text("key"), + made.set())) + registry = SimpleNamespace(pull=pull) + base = windows_disk.Base("agentenv-windows-base:1", keys, datetime.now(timezone.utc), "registry.test/base@sha256:1") + + composes = [threading.Thread(target=windows_disk._here, args=(base, registry, lambda line: None)) for _ in range(2)] + for compose in composes: + compose.start() + for compose in composes: + compose.join(10) + + assert pulls == ["registry.test/base@sha256:1"] and (keys / "id").read_text() == "key" def test_a_machine_without_the_base_takes_the_image_stores_unless_its_evaluation_nears_its_end(monkeypatch, tmp_path): From 8651111a6511ea825bc516e159ee756469ad152e Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sat, 7 Nov 2026 10:21:24 -0800 Subject: [PATCH 06/12] fix(computer): an export fails on a guest file docker can't copy, rather than cache the disk without it Co-Authored-By: Claude Opus 5.5 --- .../sandbox_providers/local_docker/disk.py | 9 +++++-- tst/unit/env/envs/test_disk_recipe.py | 24 ++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/local_docker/disk.py b/src/agent_env/providers/sandbox_providers/local_docker/disk.py index c318917ff..e1b7ce941 100644 --- a/src/agent_env/providers/sandbox_providers/local_docker/disk.py +++ b/src/agent_env/providers/sandbox_providers/local_docker/disk.py @@ -23,6 +23,8 @@ from agent_env.utils.docker.build import build_image KVM = "/dev/kvm" +#: What ``docker cp`` says of a path its container lacks: Docker 23 and later, then earlier. +_NO_SUCH_PATH = ("Could not find the file", "No such container:path") def kvm_platforms() -> frozenset[str]: @@ -111,10 +113,13 @@ def _export(image: str, platform: str, path: Path) -> None: def _copied(container: str, file: str, to: Path) -> Path | None: - """``file`` copied out of ``container`` to ``to``; None when the container has no such file.""" + """``file`` copied out of ``container`` to ``to``; None when the container has no such file. Any other failure is + raised, as an export cached without a guest file it has would boot it as the wrong OS.""" try: docker("cp", f"{container}:/{file}", str(to)) - except DockerFailed: + except DockerFailed as e: + if not any(missing in str(e) for missing in _NO_SUCH_PATH): + raise return None return to diff --git a/tst/unit/env/envs/test_disk_recipe.py b/tst/unit/env/envs/test_disk_recipe.py index 98f0436b3..3ce65e3ac 100644 --- a/tst/unit/env/envs/test_disk_recipe.py +++ b/tst/unit/env/envs/test_disk_recipe.py @@ -572,7 +572,8 @@ def test_a_disk_is_exported_compressed_into_the_cache_once_for_each_ref(tmp_path def docker(*args): ran.append(args) if args[0] == "cp" and args[1].endswith("/disk/guest"): - raise DockerFailed("Could not find the file /disk/guest in container container1", 1) + raise DockerFailed("docker cp container1:/disk/guest failed: Error response from daemon: Could not find the " + "file /disk/guest in container container1", 1) if args[0] == "cp": Path(args[2]).write_bytes(b"disk of " + args[1].encode()) return "container1" if args[0] == "create" else "1 sha256:linux" if args[0] == "image" else "" @@ -600,6 +601,27 @@ def run(argv, stdin=None, stdout=None, **kwargs): exported[2].disk.name}) +def test_an_export_that_cant_copy_a_disks_guest_file_fails_rather_than_cache_the_disk_without_it(tmp_path, + monkeypatch): + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache")) + monkeypatch.setattr(local_disk, "get_config", lambda: SimpleNamespace(get_image_store=lambda: SimpleNamespace( + auth=lambda ref: None))) + monkeypatch.setattr(local_disk, "docker_login", lambda auth: None) + + def docker(*args): + if args[0] == "cp": + raise DockerFailed("docker cp container1:/disk/guest failed: Error response from daemon: i/o timeout", 1) + return "container1" + + monkeypatch.setattr(local_disk, "docker", docker) + monkeypatch.setattr(local_disk.subprocess, "run", lambda argv, **kwargs: subprocess.CompletedProcess(argv, 0)) + image = "localhost:5000/agentenv-computers:" + "1" * 64 + "@sha256:" + "a" * 64 + + with pytest.raises(DockerFailed, match="i/o timeout"): + local_disk.exported_disk(image, "linux/amd64") + assert [path.name for path in (tmp_path / "cache" / "agentenv" / "disks").iterdir()] == [] + + def test_a_thin_disk_is_exported_with_its_guest_file_and_its_base_once_for_every_disk_on_that_base(tmp_path, monkeypatch): """A Windows disk is a layer on its base, whose first layer every disk composed on it shares.""" From 36e6bac28d2eeba0ae9f24bc518803bcf483314c Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sat, 7 Nov 2026 10:45:15 -0800 Subject: [PATCH 07/12] fix(computer): an export killed outright leaves no copy of a base behind Its work folder and container are named for the disk and cleared under its lock before it's exported again. Co-Authored-By: Claude Opus 5.5 --- .../sandbox_providers/local_docker/disk.py | 41 ++++++++----- tst/unit/env/envs/test_disk_recipe.py | 60 +++++++++++++++---- 2 files changed, 74 insertions(+), 27 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/local_docker/disk.py b/src/agent_env/providers/sandbox_providers/local_docker/disk.py index e1b7ce941..69bfa1a6e 100644 --- a/src/agent_env/providers/sandbox_providers/local_docker/disk.py +++ b/src/agent_env/providers/sandbox_providers/local_docker/disk.py @@ -6,6 +6,7 @@ import asyncio import hashlib import os +import shutil import subprocess import tempfile from dataclasses import dataclass @@ -91,25 +92,33 @@ def exported_disk(image: str, platform: str) -> Exported: def _export(image: str, platform: str, path: Path) -> None: """Signed in to the image store first, as a boot's pull is, since ``docker create`` pulls ``image`` when it isn't here; for ``platform``, as the image's index holds that one alone, which a pull for this machine's wouldn't find. - The disk is placed last, so one that's there was exported whole, with its base and guest file.""" - path.parent.mkdir(parents=True, exist_ok=True) + The disk is placed last, so one that's there was exported whole, with its base and guest file. Its work folder and + container are named for ``path`` and cleared first, under its lock, so an export killed outright leaves no copy of + a base behind once the disk is exported again.""" + work, container = path.with_name(path.name + ".work"), f"agentenv-export-{path.name.split('.')[0]}" + _cleared(work, container) + work.mkdir(parents=True) docker_login(get_config().get_image_store().auth(image)) - container = docker("create", "--platform", platform, image, "disk") + docker("create", "--name", container, "--platform", platform, image, "disk") try: - with tempfile.TemporaryDirectory(prefix="export-", dir=path.parent) as work: - if (guest := _copied(container, DISK_GUEST_FILE, Path(work) / "guest")) is not None: - guest.replace(path.with_name(path.name + ".guest")) - layers, first = docker("image", "inspect", "-f", "{{len .RootFS.Layers}} {{index .RootFS.Layers 0}}", - image).split() - if int(layers) > 1: - base = path.with_name(f"base-{hashlib.sha256(first.encode()).hexdigest()[:32]}.img.zst") - with holding_locks([f"disk-export:{base.name}"]): - if not base.exists(): - _compressed(container, DISK_BASE_FILE, base, Path(work), image) - path.with_name(path.name + ".base").write_text(base.name) - _compressed(container, DISK_FILE, path, Path(work), image) + if (guest := _copied(container, DISK_GUEST_FILE, work / "guest")) is not None: + guest.replace(path.with_name(path.name + ".guest")) + layers, first = docker("image", "inspect", "-f", "{{len .RootFS.Layers}} {{index .RootFS.Layers 0}}", + image).split() + if int(layers) > 1: + base = path.with_name(f"base-{hashlib.sha256(first.encode()).hexdigest()[:32]}.img.zst") + with holding_locks([f"disk-export:{base.name}"]): + if not base.exists(): + _compressed(container, DISK_BASE_FILE, base, work, image) + path.with_name(path.name + ".base").write_text(base.name) + _compressed(container, DISK_FILE, path, work, image) finally: - subprocess.run(["docker", "rm", container], capture_output=True) + _cleared(work, container) + + +def _cleared(work: Path, container: str) -> None: + subprocess.run(["docker", "rm", "-f", container], capture_output=True) + shutil.rmtree(work, ignore_errors=True) def _copied(container: str, file: str, to: Path) -> Path | None: diff --git a/tst/unit/env/envs/test_disk_recipe.py b/tst/unit/env/envs/test_disk_recipe.py index 3ce65e3ac..4116dc7bf 100644 --- a/tst/unit/env/envs/test_disk_recipe.py +++ b/tst/unit/env/envs/test_disk_recipe.py @@ -6,6 +6,7 @@ import asyncio import base64 +import hashlib import json import os import re @@ -575,8 +576,8 @@ def docker(*args): raise DockerFailed("docker cp container1:/disk/guest failed: Error response from daemon: Could not find the " "file /disk/guest in container container1", 1) if args[0] == "cp": - Path(args[2]).write_bytes(b"disk of " + args[1].encode()) - return "container1" if args[0] == "create" else "1 sha256:linux" if args[0] == "image" else "" + Path(args[2]).write_bytes(b"disk of " + args[1].partition(":")[2].encode()) + return "1 sha256:linux" if args[0] == "image" else "" def run(argv, stdin=None, stdout=None, **kwargs): if argv[:2] == ["docker", "run"]: @@ -591,16 +592,54 @@ def run(argv, stdin=None, stdout=None, **kwargs): exported = [local_disk.exported_disk(image, "linux/amd64") for image in (first, first, first.replace("1", "2"))] assert exported[0] == exported[1] != exported[2] and exported[0].disk.parent == tmp_path / "cache" / "agentenv" / "disks" - assert exported[0].disk.read_bytes() == b"zstd:disk of container1:/disk/disk.img" + assert exported[0].disk.read_bytes() == b"zstd:disk of /disk/disk.img" assert exported[0].disk.name.endswith(".img.zst") and exported[0].base is exported[0].guest is None assert [args[0] for args in ran] == ["login", "create", "cp", "image", "cp"] * 2 - assert ran[0] == ("login", f"credentials for {first}") and ran[1][1:] == ("--platform", "linux/amd64", first, "disk") + assert ran[0] == ("login", f"credentials for {first}") and ran[1][1:] == ( + "--name", f"agentenv-export-{exported[0].disk.name.split('.')[0]}", "--platform", "linux/amd64", first, "disk") assert compressed[0] == ["docker", "run", "--rm", "-i", "--entrypoint", "zstd", "agentenv-qemu-launcher:src-1", "-q", "-3", "-T0", "-c"] and len(compressed) == 2 assert sorted(p.name for p in exported[0].disk.parent.iterdir()) == sorted({exported[0].disk.name, exported[2].disk.name}) +def test_an_export_killed_outright_leaves_nothing_behind_once_the_disk_is_exported_again(tmp_path, monkeypatch): + """Its work folder, holding as much of a base as it had copied, and its container are named for the disk, and + cleared under the disk's lock first.""" + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache")) + monkeypatch.setattr(local_disk, "launcher", lambda: "agentenv-qemu-launcher:src-1") + monkeypatch.setattr(local_disk, "get_config", lambda: SimpleNamespace(get_image_store=lambda: SimpleNamespace( + auth=lambda ref: None))) + monkeypatch.setattr(local_disk, "docker_login", lambda auth: None) + image = "localhost:5000/agentenv-computers:" + "1" * 64 + "@sha256:" + "a" * 64 + disks = tmp_path / "cache" / "agentenv" / "disks" + left = disks / f"{hashlib.sha256(image.encode()).hexdigest()[:32]}.img.zst.work" + left.mkdir(parents=True) + (left / "base.img").write_bytes(b"half a base") + events = [] + + def docker(*args): + events.append(args[:2]) + if args[0] == "cp": + assert not (left / "base.img").exists() + Path(args[2]).write_bytes(b"disk") + return "1 sha256:linux" if args[0] == "image" else "" + + def run(argv, stdin=None, stdout=None, **kwargs): + events.append(tuple(argv[1:3])) + if argv[:2] == ["docker", "run"]: + stdout.write(stdin.read()) + return subprocess.CompletedProcess(argv, 0, b"", b"") + + monkeypatch.setattr(local_disk, "docker", docker) + monkeypatch.setattr(local_disk.subprocess, "run", run) + + exported = local_disk.exported_disk(image, "linux/amd64") + + assert events[:2] == [("rm", "-f"), ("create", "--name")] and events[-1] == ("rm", "-f") + assert sorted(path.name for path in disks.iterdir()) == [exported.disk.name, exported.disk.name + ".guest"] + + def test_an_export_that_cant_copy_a_disks_guest_file_fails_rather_than_cache_the_disk_without_it(tmp_path, monkeypatch): monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache")) @@ -634,9 +673,9 @@ def test_a_thin_disk_is_exported_with_its_guest_file_and_its_base_once_for_every def docker(*args): if args[0] == "cp": - copied.append(args[1]) - Path(args[2]).write_bytes(b"windows" if args[1].endswith("guest") else b"copy of " + args[1].encode()) - return "container1" if args[0] == "create" else "3 sha256:base" if args[0] == "image" else "" + copied.append(args[1].partition(":")[2]) + Path(args[2]).write_bytes(b"windows" if args[1].endswith("guest") else b"copy of " + copied[-1].encode()) + return "3 sha256:base" if args[0] == "image" else "" def run(argv, stdin=None, stdout=None, **kwargs): if argv[:2] == ["docker", "run"]: @@ -650,8 +689,7 @@ def run(argv, stdin=None, stdout=None, **kwargs): one, two = (local_disk.exported_disk(image, "windows/amd64") for image in (first, first.replace("1", "2"))) assert one.disk != two.disk and one.base == two.base and one.base.parent == one.disk.parent - assert one.base.read_bytes() == b"zstd:copy of container1:/disk/base.img" - assert one.disk.read_bytes() == b"zstd:copy of container1:/disk/disk.img" and one.guest.read_text() == "windows" - assert copied == ["container1:/disk/guest", "container1:/disk/base.img", "container1:/disk/disk.img", - "container1:/disk/guest", "container1:/disk/disk.img"] + assert one.base.read_bytes() == b"zstd:copy of /disk/base.img" + assert one.disk.read_bytes() == b"zstd:copy of /disk/disk.img" and one.guest.read_text() == "windows" + assert copied == ["/disk/guest", "/disk/base.img", "/disk/disk.img", "/disk/guest", "/disk/disk.img"] assert local_disk.exported_disk(first, "windows/amd64") == one and len(copied) == 5 From 56a97d09dfecc14c793ab10a6ba55efda784f4d0 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sat, 7 Nov 2026 11:01:00 -0800 Subject: [PATCH 08/12] fix(computer): name an export's container by its work folder's whole path, so two users' exports never clear each other's Co-Authored-By: Claude Opus 5.5 --- .../sandbox_providers/local_docker/disk.py | 6 ++-- tst/unit/env/envs/test_disk_recipe.py | 35 +++++++++++++++++-- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/agent_env/providers/sandbox_providers/local_docker/disk.py b/src/agent_env/providers/sandbox_providers/local_docker/disk.py index 69bfa1a6e..523a00718 100644 --- a/src/agent_env/providers/sandbox_providers/local_docker/disk.py +++ b/src/agent_env/providers/sandbox_providers/local_docker/disk.py @@ -94,8 +94,10 @@ def _export(image: str, platform: str, path: Path) -> None: here; for ``platform``, as the image's index holds that one alone, which a pull for this machine's wouldn't find. The disk is placed last, so one that's there was exported whole, with its base and guest file. Its work folder and container are named for ``path`` and cleared first, under its lock, so an export killed outright leaves no copy of - a base behind once the disk is exported again.""" - work, container = path.with_name(path.name + ".work"), f"agentenv-export-{path.name.split('.')[0]}" + a base behind once the disk is exported again; the container by the folder's whole path, as another user's export + of the same disk through this machine's Docker holds a lock of its own.""" + work = path.with_name(path.name + ".work") + container = f"agentenv-export-{hashlib.sha256(str(work).encode()).hexdigest()[:24]}" _cleared(work, container) work.mkdir(parents=True) docker_login(get_config().get_image_store().auth(image)) diff --git a/tst/unit/env/envs/test_disk_recipe.py b/tst/unit/env/envs/test_disk_recipe.py index 4116dc7bf..20f5d7276 100644 --- a/tst/unit/env/envs/test_disk_recipe.py +++ b/tst/unit/env/envs/test_disk_recipe.py @@ -595,8 +595,8 @@ def run(argv, stdin=None, stdout=None, **kwargs): assert exported[0].disk.read_bytes() == b"zstd:disk of /disk/disk.img" assert exported[0].disk.name.endswith(".img.zst") and exported[0].base is exported[0].guest is None assert [args[0] for args in ran] == ["login", "create", "cp", "image", "cp"] * 2 - assert ran[0] == ("login", f"credentials for {first}") and ran[1][1:] == ( - "--name", f"agentenv-export-{exported[0].disk.name.split('.')[0]}", "--platform", "linux/amd64", first, "disk") + assert ran[0] == ("login", f"credentials for {first}") and ran[1][1] == "--name" + assert ran[1][2].startswith("agentenv-export-") and ran[1][3:] == ("--platform", "linux/amd64", first, "disk") assert compressed[0] == ["docker", "run", "--rm", "-i", "--entrypoint", "zstd", "agentenv-qemu-launcher:src-1", "-q", "-3", "-T0", "-c"] and len(compressed) == 2 assert sorted(p.name for p in exported[0].disk.parent.iterdir()) == sorted({exported[0].disk.name, @@ -640,6 +640,37 @@ def run(argv, stdin=None, stdout=None, **kwargs): assert sorted(path.name for path in disks.iterdir()) == [exported.disk.name, exported.disk.name + ".guest"] +def test_two_users_exports_of_one_disk_through_one_docker_name_their_containers_apart(tmp_path, monkeypatch): + """Each holds a lock of its own, so neither clears the other's container.""" + monkeypatch.setattr(local_disk, "launcher", lambda: "agentenv-qemu-launcher:src-1") + monkeypatch.setattr(local_disk, "get_config", lambda: SimpleNamespace(get_image_store=lambda: SimpleNamespace( + auth=lambda ref: None))) + monkeypatch.setattr(local_disk, "docker_login", lambda auth: None) + created = [] + + def docker(*args): + if args[0] == "create": + created.append(args[2]) + if args[0] == "cp": + Path(args[2]).write_bytes(b"disk") + return "1 sha256:linux" if args[0] == "image" else "" + + def run(argv, stdin=None, stdout=None, **kwargs): + if argv[:2] == ["docker", "run"]: + stdout.write(stdin.read()) + return subprocess.CompletedProcess(argv, 0, b"", b"") + + monkeypatch.setattr(local_disk, "docker", docker) + monkeypatch.setattr(local_disk.subprocess, "run", run) + image = "localhost:5000/agentenv-computers:" + "1" * 64 + "@sha256:" + "a" * 64 + + for user in ("one", "two"): + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / user / "cache")) + local_disk.exported_disk(image, "linux/amd64") + + assert len(set(created)) == 2 + + def test_an_export_that_cant_copy_a_disks_guest_file_fails_rather_than_cache_the_disk_without_it(tmp_path, monkeypatch): monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache")) From fa5aefb910c994a694459d02130680b4fd03d24e Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sat, 7 Nov 2026 15:59:40 -0800 Subject: [PATCH 09/12] refactor(computer): one store lookup for Linux and Windows disks, Windows names derived from a key in one place - pin_if_served beside pin_digest replaces the composer's _served and Registry's own copy of it; _image_store gets the env's store with its repository for _stored and the Windows compose alike. - base_names and disk_names name a base or disk from its key, here and in the store; compose takes the key rather than both names. - labels is imported as image_labels, no longer shadowed by compose's parameter and Registry's method. - The ISO label nothing read, and the sha256 helper only it used, are gone; Registry's store is an ImageStore. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/computers/README.md | 2 +- .../env/envs/computer/utils/composer.py | 40 +++++----- .../env/envs/computer/utils/windows_disk.py | 76 ++++++++++--------- .../store/image_store/registry_api.py | 8 ++ tst/unit/env/envs/conftest.py | 2 + tst/unit/env/envs/test_computer_env.py | 2 +- tst/unit/env/envs/test_computer_windows.py | 5 +- tst/unit/store/registry_api_test.py | 12 +++ 8 files changed, 83 insertions(+), 64 deletions(-) diff --git a/src/agent_env/computers/README.md b/src/agent_env/computers/README.md index f0d2390c8..7874544b8 100644 --- a/src/agent_env/computers/README.md +++ b/src/agent_env/computers/README.md @@ -152,7 +152,7 @@ setup that fails is read back over ssh at once (`setup-failed`), then turns off QEMU's network, which reaches the guest at the ports the launcher forwards alone), installs the pinned Python with Tk and `agentenv-session`, and switches the sign-in to `user`. Setup takes about 13 minutes. The installed disk, trimmed, becomes the base's image, `disk/base.img` in a layer of its own under an empty overlay of it, with the date its 90-day -evaluation expires, the ISO's sha256 and the key to `agentadmin`; it's pushed to the env's image store, as Linux disks +evaluation expires and the key to `agentadmin`; it's pushed to the env's image store, as Linux disks are, under a tag of its recipe, so another machine composing on it pulls it rather than installing Windows, and fetches no ISO. A compose installs a base afresh once fewer than 7 days of its evaluation are left, from the ISO Microsoft's link serves then. Name it as `windows-11`, or by its id, `@local/agentenv-framework/computers/windows-11`. diff --git a/src/agent_env/env/envs/computer/utils/composer.py b/src/agent_env/env/envs/computer/utils/composer.py index 5ba61a48f..021d672be 100644 --- a/src/agent_env/env/envs/computer/utils/composer.py +++ b/src/agent_env/env/envs/computer/utils/composer.py @@ -54,7 +54,8 @@ from agent_env.store.ids import derive_id from agent_env.store.image_store.image_contents import image_config, labels from agent_env.store.image_store.oci_registry_credentials import names_registry -from agent_env.store.image_store.registry_api import NoSuchImageError, pin_digest, write_index +from agent_env.store.image_store.image_store import ImageStore +from agent_env.store.image_store.registry_api import pin_digest, pin_if_served, write_index from agent_env.store.local_state import holding_locks from agent_env.utils.docker.build import DockerBuildError @@ -369,29 +370,26 @@ def say(line: str) -> None: progress.say(f"{pair}: {line}") attempt.stage = "base" - store = get_config().get_image_store_for(env.stored_id) - store.ensure_repository(REPOSITORY) - registry = windows_disk.Registry(store, REPOSITORY) + registry = windows_disk.Registry(_image_store(env), REPOSITORY) disk_gb, user = base.forms.disk.min_disk_gb, base.session.user made = windows_disk.base(_key(recipe=_digest(windows_disk.RECIPE), disk_gb=disk_gb, user=user, form="windows-base", format=FORMAT), lambda: windows_disk.media(say), disk_gb, user, say, registry) script = windows_disk.write(build, [part], settings, {"base": made.image, "layers": [part.folder]}) - key = attempt.key = _key(base=made.image, base_expires=made.expires.isoformat(timespec="seconds"), - provision=script, context=_digest(context), build_settings=settings, form="disk", - arch=arch, format=FORMAT) - tag, expires = key.removeprefix("sha256:"), made.expires.isoformat(timespec="seconds") + expires = made.expires.isoformat(timespec="seconds") + key = attempt.key = _key(base=made.image, base_expires=expires, provision=script, context=_digest(context), + build_settings=settings, form="disk", arch=arch, format=FORMAT) attempt.stage = "build" with holding_locks([f"windows-disk:{key}"], lambda: progress.at( pair, "waiting", "waiting for the compose of the same disk under way on this machine")): - if (pinned := registry.served(tag)) is not None: + if (pinned := registry.served(windows_disk.disk_names(key).tag)) is not None: progress.at(pair, "reusing", "reusing the disk of the same key, composed earlier") result = {"status": "unverified", "key": key, "image": pinned, "reused": True, "expires": expires, "built_at": now()} else: progress.at(pair, "composing", "composing on this machine's Docker") - pinned = windows_disk.compose(made, build, f"agentenv-windows:{key[7:23]}", - {KEY_LABEL: key, windows_disk.EXPIRES_LABEL: expires}, say, registry, tag) + pinned = windows_disk.compose(made, build, key, + {KEY_LABEL: key, windows_disk.EXPIRES_LABEL: expires}, say, registry) result = {"status": "unverified", "key": key, "image": pinned, "reused": False, "builder": "LocalSandboxProvider", "expires": expires, "built_at": now()} return _probed(env, driver, "disk", arch, result, provider, progress) @@ -653,13 +651,19 @@ def _key(**inputs: Any) -> str: return "sha256:" + hashlib.sha256(json.dumps(inputs, sort_keys=True).encode()).hexdigest() -def _stored(env: ComputerEnv, key: str) -> tuple[Any, str, str | None]: +def _stored(env: ComputerEnv, key: str) -> tuple[ImageStore, str, str | None]: """The env's image store, the ref there of the image of ``key``, and that ref pinned when the store serves one already, for any env that built it.""" + store = _image_store(env) + ref = store.image_ref(REPOSITORY, key.removeprefix("sha256:")) + return store, ref, pin_if_served(ref, store.auth(ref)) + + +def _image_store(env: ComputerEnv) -> ImageStore: + """The env's image store, with the repository its computers' images are kept in.""" store = get_config().get_image_store_for(env.stored_id) store.ensure_repository(REPOSITORY) - ref = store.image_ref(REPOSITORY, key.removeprefix("sha256:")) - return store, ref, _served(ref, store) + return store def _base_disk(base: ComputerArtifact, arch: str) -> tuple[str | None, str]: @@ -911,14 +915,6 @@ def _wants(layers: list[_Layer], protocol: bool) -> list[packages.Want]: return wants -def _served(ref: str, store) -> str | None: - """``ref`` pinned to the digest the image store serves for it, or None when it serves none.""" - try: - return pin_digest(ref, store.auth(ref)) - except NoSuchImageError: - return None - - def _computer(env: ComputerEnv, base: ComputerArtifact, built: dict[str, str], disks: dict[str, str], bundles: dict[str, str]) -> dict | None: """Write the env's computer over the images ``built``, by arch, joined into one index in the env's image store, signed diff --git a/src/agent_env/env/envs/computer/utils/windows_disk.py b/src/agent_env/env/envs/computer/utils/windows_disk.py index e23bfd161..652ea7f0a 100644 --- a/src/agent_env/env/envs/computer/utils/windows_disk.py +++ b/src/agent_env/env/envs/computer/utils/windows_disk.py @@ -12,7 +12,6 @@ import asyncio import base64 import contextlib -import hashlib import io import json import re @@ -27,7 +26,7 @@ from dataclasses import dataclass from datetime import datetime, timedelta, timezone from pathlib import Path, PurePosixPath, PureWindowsPath -from typing import Any +from typing import Any, NamedTuple from agent_env.artifact.artifacts.computer import DISK_BASE_FILE, DISK_FILE, DISK_OWNER from agent_env.artifact.artifacts.docker_image import docker_login, push_local_image @@ -36,8 +35,9 @@ from agent_env.providers.sandbox_providers.host_images import present from agent_env.providers.sandbox_providers.local_docker.cli import docker from agent_env.providers.sandbox_providers.local_docker.disk import KVM, launcher, machine_image -from agent_env.store.image_store.image_contents import labels -from agent_env.store.image_store.registry_api import NoSuchImageError, pin_digest +from agent_env.store.image_store.image_contents import labels as image_labels +from agent_env.store.image_store.image_store import ImageStore +from agent_env.store.image_store.registry_api import pin_digest, pin_if_served from agent_env.store.local_state import holding_locks from agent_env.utils.docker.build import build_image @@ -54,8 +54,6 @@ #: The fewest days of its evaluation a base may have left, as a deploy needs some. RENEW_DAYS = 7 EXPIRES_LABEL, BUILD_LABEL, KEY_LABEL = "dev.agentenv.windows.expires", "dev.agentenv.windows.build", "dev.agentenv.key" -#: The sha256 of the ISO a base was installed from. -ISO_LABEL = "dev.agentenv.windows.iso" #: Where a base's image keeps the key to its admin, which composing a layer on it takes over ssh. The image is in the #: user's own image store, and the guest's ssh is reached only on this machine's loopback while a layer is composed. ADMIN_KEY = PurePosixPath("/disk/admin.key") @@ -80,19 +78,16 @@ class Registry: """The env's image store as Windows disks are kept in it: by tag in ``repository``, pinned by digest, as Linux disks are.""" - store: Any + store: ImageStore repository: str def served(self, tag: str) -> str | None: """``tag``'s ref, pinned to the digest the store serves for it now; None when it serves none.""" ref = self.store.image_ref(self.repository, tag) - try: - return pin_digest(ref, self.store.auth(ref)) - except NoSuchImageError: - return None + return pin_if_served(ref, self.store.auth(ref)) def labels(self, pinned: str) -> dict[str, str]: - return labels(pinned, "amd64", self.store.auth(pinned)) + return image_labels(pinned, "amd64", self.store.auth(pinned)) def push(self, local: str, tag: str) -> str: """``local`` pushed as ``tag``; its ref there, pinned.""" @@ -105,6 +100,26 @@ def pull(self, pinned: str) -> None: docker("pull", "-q", pinned) +class Names(NamedTuple): + """What a base or a disk is called: ``local`` in this machine's Docker, ``tag`` in the image store.""" + + local: str + tag: str + + +def base_names(key: str) -> Names: + return Names(f"agentenv-windows-base:{_short(key)}", f"windows-base-{_short(key)}") + + +def disk_names(key: str) -> Names: + """A disk's names, its tag in the image store being its key, as a Linux disk's is.""" + return Names(f"agentenv-windows:{_short(key)}", key.removeprefix("sha256:")) + + +def _short(key: str) -> str: + return key.removeprefix("sha256:")[:16] + + @dataclass(frozen=True) class Base: """A base: its image's tag on this machine, the folder of its admin's key, when its evaluation expires, and, for one @@ -148,28 +163,13 @@ def media(say: Callable[[str], None]) -> Path: return iso -def sha256(path: Path) -> str: - """``path``'s sha256, kept beside it for its size and mtime, since an ISO takes a while to read.""" - stat, kept = path.stat(), path.with_name(path.name + ".sha256") - stamp = f"{stat.st_size} {stat.st_mtime_ns}" - if kept.exists() and (lines := kept.read_text().split("\n"))[0] == stamp: - return lines[1] - hashed = hashlib.sha256() - with path.open("rb") as source: - while chunk := source.read(1 << 20): - hashed.update(chunk) - kept.write_text(f"{stamp}\n{hashed.hexdigest()}\n") - return hashed.hexdigest() - - def base(key: str, iso: Callable[[], Path], disk_gb: int, user: str, say: Callable[[str], None], registry: Registry) -> Base: """The base of ``key``: this machine's, pushed to the image store unless it holds one of ``key``; else the one the store holds, pulled, with the key to its admin it carries, when a layer is composed on it; else Windows installed from the ISO ``iso`` gives, and pushed there. One with fewer than ``RENEW_DAYS`` of its evaluation left is installed afresh. One install of a key runs at once on a machine, and another waits for it.""" - image, keys = f"agentenv-windows-base:{key[7:23]}", cache_root() / "windows" / "bases" / key[7:23] - tag = f"windows-base-{key[7:23]}" + (image, tag), keys = base_names(key), cache_root() / "windows" / "bases" / _short(key) with holding_locks([_base_lock(image)], lambda: say("waiting for the install of the same base under way")): if present(image) and (keys / "id").exists(): expires = datetime.fromisoformat(_label(image, EXPIRES_LABEL)) @@ -187,13 +187,13 @@ def base(key: str, iso: Callable[[], Path], disk_gb: int, user: str, say: Callab subprocess.run(["ssh-keygen", "-q", "-t", "ed25519", "-N", "", "-C", f"agentenv {image}", "-f", str(keys / "id")], check=True) media = iso() - with _working(f"base-{key[7:23]}") as work: + with _working(f"base-{_short(key)}") as work: facts = _installed(work, media, keys, disk_gb, user, say) say("packing the base") expires = datetime.now(timezone.utc) + timedelta(minutes=facts["grace_minutes"]) shutil.copy(keys / "id", work / ADMIN_KEY.name) _pack(work, image, {KEY_LABEL: key, EXPIRES_LABEL: expires.isoformat(timespec="seconds"), - BUILD_LABEL: facts["build"], ISO_LABEL: sha256(media)}, base_image=None) + BUILD_LABEL: facts["build"]}, base_image=None) say("pushing the base to the image store") registry.push(image, tag) return Base(image, keys, expires) @@ -271,16 +271,18 @@ def _installed(work: Path, iso: Path, keys: Path, disk_gb: int, user: str, say: return facts -def compose(base_: Base, build: Path, image: str, labels: dict[str, str], say: Callable[[str], None], - registry: Registry, tag: str) -> str: - """``image``, a disk composed on ``base_``: the base booted on an overlay kept in a folder of this machine, ``build`` - copied in and its ``provision.ps1`` run as the admin over ssh, then shut down, packed onto the base's image and - pushed to the image store as ``tag``; its ref there, pinned. A base the store holds is pulled first.""" +def compose(base_: Base, build: Path, key: str, labels: dict[str, str], say: Callable[[str], None], + registry: Registry) -> str: + """The disk of ``key`` composed on ``base_``: the base booted on an overlay kept in a folder of this machine, + ``build`` copied in and its ``provision.ps1`` run as the admin over ssh, then shut down, packed onto the base's + image with ``labels`` and pushed to the image store; its ref there, pinned. A base the store holds is pulled + first.""" + image, tag = disk_names(key) _here(base_, registry, say) - with _working(f"disk-{image.rpartition(':')[2]}") as work: + with _working(f"disk-{_short(key)}") as work: out = work / "out" out.mkdir() - name = f"agentenv-windows-compose-{image.rpartition(':')[2]}" + name = f"agentenv-windows-compose-{_short(key)}" say(f"booting its base in {name}") machine = asyncio.run(machine_image(base_.image)) container = _launched(name, {"VM_KEEP": "/out/disk.img"}, ["-v", f"{out}:/out"], machine) diff --git a/src/agent_env/store/image_store/registry_api.py b/src/agent_env/store/image_store/registry_api.py index 61ff38bdb..4544cfed8 100644 --- a/src/agent_env/store/image_store/registry_api.py +++ b/src/agent_env/store/image_store/registry_api.py @@ -68,6 +68,14 @@ def pin_digest(ref: str, auth: RegistryAuth | None, *, timeout: float = _TIMEOUT return ref +def pin_if_served(ref: str, auth: RegistryAuth | None) -> str | None: + """``ref`` pinned as ``pin_digest`` pins it, or None when its registry serves no such image.""" + try: + return pin_digest(ref, auth) + except NoSuchImageError: + return None + + def pinned_from(pinned: str, ref: str) -> bool: """Whether ``pinned``, a ref as ``pin_digest`` returns it, pins ``ref``: ``ref`` itself when it gives a digest, else the same name and tag (``latest`` when none is given) with a digest.""" diff --git a/tst/unit/env/envs/conftest.py b/tst/unit/env/envs/conftest.py index 3f561951f..4096f25b0 100644 --- a/tst/unit/env/envs/conftest.py +++ b/tst/unit/env/envs/conftest.py @@ -10,6 +10,7 @@ from agent_env.artifact.store import get_artifact_store from agent_env.env.envs.computer.utils import builder, composer, probing from agent_env.providers.sandbox_providers.local_docker import provider as local_sandbox +from agent_env.store.image_store import registry_api from agent_env.store.image_store.image_store import OciRegistryImageStore from agent_env.store.image_store.local_registry_image_store import LocalRegistryImageStore from agent_env.store.object_store.local.store import LocalFilesystemObjectStore @@ -27,6 +28,7 @@ def docker(local_stores, cli_routing, monkeypatch): monkeypatch.setattr(builder, "docker_login", lambda auth: None) monkeypatch.setattr(composer, "write_index", fake.joined) monkeypatch.setattr(composer, "pin_digest", registry.pin_digest) + monkeypatch.setattr(registry_api, "pin_digest", registry.pin_digest) monkeypatch.setattr(docker_image, "pin_digest", registry.pin_digest) monkeypatch.setattr(builder, "host_platform", lambda: "linux/arm64") monkeypatch.setattr(local_sandbox, "host_platform", lambda: "linux/arm64") diff --git a/tst/unit/env/envs/test_computer_env.py b/tst/unit/env/envs/test_computer_env.py index a8a363820..9789c25bf 100644 --- a/tst/unit/env/envs/test_computer_env.py +++ b/tst/unit/env/envs/test_computer_env.py @@ -630,7 +630,7 @@ def test_builds_of_one_env_at_once_keep_each_others_pairs_and_a_failure_doesnt_r composer.compose(first, [("container", "amd64")]) composer.compose(second, [("container", "arm64")]) - monkeypatch.setattr(composer, "_served", lambda ref, store: None) + monkeypatch.setattr(composer, "pin_if_served", lambda ref, auth: None) docker.fail["linux/arm64"] = "boom" composer.compose(third, [("container", "arm64")]) diff --git a/tst/unit/env/envs/test_computer_windows.py b/tst/unit/env/envs/test_computer_windows.py index 1cd051a9b..a2b584be9 100644 --- a/tst/unit/env/envs/test_computer_windows.py +++ b/tst/unit/env/envs/test_computer_windows.py @@ -72,10 +72,10 @@ def base(self, key, iso, disk_gb, user, say, registry): self.bases[key] = datetime.now(timezone.utc) + timedelta(days=89) return windows_disk.Base(f"agentenv-windows-base:{key[7:23]}", self.tmp / "keys", self.bases[key]) - def compose(self, base, build, image, labels, say, registry, tag): + def compose(self, base, build, key, labels, say, registry): self.composed.append((base.image, (build / "provision.ps1").read_bytes().decode(), sorted(p.relative_to(build).as_posix() for p in build.rglob("*") if p.is_file()), labels)) - return registry.push(image, tag) + return registry.push(*windows_disk.disk_names(key)) @pytest.fixture @@ -244,7 +244,6 @@ def test_a_base_is_reused_until_its_evaluation_nears_its_end_then_installed_afre monkeypatch.setattr(windows_disk, "_installed", lambda *args: installed.append(args) or { "grace_minutes": 90 * 24 * 60, "build": "26200.6584"}) monkeypatch.setattr(windows_disk, "_pack", lambda folder, image, labels, base_image: None) - monkeypatch.setattr(windows_disk, "sha256", lambda path: "ab" * 32) registry = FakeRegistry() base = windows_disk.base(key, lambda: tmp_path / "windows.iso", 64, "user", lambda line: None, registry) diff --git a/tst/unit/store/registry_api_test.py b/tst/unit/store/registry_api_test.py index 10bd8fcde..aec0d54fe 100644 --- a/tst/unit/store/registry_api_test.py +++ b/tst/unit/store/registry_api_test.py @@ -14,6 +14,7 @@ NoSuchImageError, RegistryRefusedError, pin_digest, + pin_if_served, pinned_from, read_blob, read_manifest, @@ -187,6 +188,17 @@ def test_a_missing_image_and_a_refusal_each_have_their_own_error(fake_registry, assert type(raised.value) is (kind or ValueError) +@pytest.mark.parametrize("status, kind", [(404, None), (401, RegistryRefusedError), (500, ValueError)]) +def test_pin_if_served_is_none_for_a_missing_image_and_raises_anything_else(fake_registry, status, kind): + fake_registry(lambda request: httpx.Response(status)) + + if kind is None: + assert pin_if_served("ghcr.io/org/tool:v9", None) is None + else: + with pytest.raises(kind): + pin_if_served("ghcr.io/org/tool:v9", None) + + def test_an_unreachable_registry_is_named(fake_registry): def down(request): raise httpx.ConnectError("connection refused") From 8a460053de34004a0f3767e37cea1189ef27a47d Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sat, 7 Nov 2026 16:49:53 -0800 Subject: [PATCH 10/12] docs(computer): Windows computers boot on modal_vm, their base carried beside the disk, under your own licence Co-Authored-By: Claude Opus 5.5 --- src/agent_env/computers/README.md | 17 ++++++++++------- .../providers/sandbox_providers/AGENTS.md | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/agent_env/computers/README.md b/src/agent_env/computers/README.md index 90e25095f..8d7853de4 100644 --- a/src/agent_env/computers/README.md +++ b/src/agent_env/computers/README.md @@ -38,10 +38,12 @@ before reset in place started is refused a reset, to be redeployed. It boots on Docker can pass it `/dev/kvm`, and on `modal_vm`, amd64 only, whose sandbox runs the launcher as its image under nested virtualization: the provider exports the disk from the image store with this machine's Docker, compressed, once for each composed disk, into the per-user cache, and mounts it on the sandbox at create, which Modal uploads once for each -content and the launcher expands onto the sandbox's disk. Such a sandbox is ended once it has been idle, with no -connection open, for the provider's `idle_timeout`, 30 minutes unless its config sets another, as a running QEMU isn't -activity. A gate 3 probe that ends with the connection to the driver dropped, refused or timed out runs again on a fresh -boot, up to three in all, and the record keeps each attempt; one with a failed check is recorded as it is. +content and the launcher expands onto the sandbox's disk; a thin disk, a Windows one, carries its base too, exported +once for each base and expanded beside it there, so another computer on that base adds its layer alone. Such a sandbox +is ended once it has been idle, with no connection open, for the provider's `idle_timeout`, 30 minutes unless its config +sets another, as a running QEMU isn't activity. A gate 3 probe that ends with the connection to the driver dropped, +refused or timed out runs again on a fresh boot, up to three in all, and the record keeps each attempt; one with a +failed check is recorded as it is. A put, `env build` and the release compose every form; a run composes only the forms its sandbox's deploys boot: on each link of a chain, the first form it boots natively, as a deploy picks, so local Docker composes containers, and a sandbox @@ -170,10 +172,11 @@ pulls it. Gate 3 probes it at full. At the session user's sign-in, `agentenv-session`, run windowless by `pythonw`, reads the deploy's environment from the NoCloud seed, which the launcher attaches as a removable USB drive, since Windows mounts a FAT filesystem with no partition table on removable media alone, and starts each layer's services in that session, again whenever one exits. A Windows computer boots -with 4 vCPUs and 8 GB, on local Docker with KVM; it has computer-exec and data/add, through the protocol's Windows +with 4 vCPUs and 8 GB, on local Docker with KVM and on `modal_vm`, and may take 10 minutes to be ready, for its first +sign-in and, on `modal_vm`, its base expanded first; it has computer-exec and data/add, through the protocol's Windows defaults, so the probe runs its fixture there as on Linux, with Python one-liners for the commands it would give a -shell. A Windows computer composes its driver alone for now, no app, and boots on no Modal VM sandbox yet, whose export -carries a disk's top layer alone. +shell. On `modal_vm` it runs in your own Modal workspace, under the evaluation's terms or a licence of your own; +whether they allow it there is yours to check. A Windows computer composes its driver alone for now, no app. This bundle has no tasks of its own: other bundles pull its computers by alias, and `agent-env run computers` has nothing to run. diff --git a/src/agent_env/providers/sandbox_providers/AGENTS.md b/src/agent_env/providers/sandbox_providers/AGENTS.md index cd7d788b1..5c669d3d8 100644 --- a/src/agent_env/providers/sandbox_providers/AGENTS.md +++ b/src/agent_env/providers/sandbox_providers/AGENTS.md @@ -57,7 +57,7 @@ with `exposed_ports=[port]` and returns the VM, and the deploy then loads and st | `create_container(...)` | inherited | containers that run an env or agent image. Override only when the platform boots images natively | | `reset_container(sandbox, *, image_name, port, env)` | inherited | a computer env's reset, which replaces the container `create_container` ran with a fresh one on the same sandbox and port. The default works on a VM backend; one whose sandbox is the container itself raises `NotImplementedError` | | `native_platforms()` | no | a computer env's deploy and gate 3, which boot a build only on a provider that runs its arch without emulation. The default is `linux/amd64`; local Docker returns this machine's | -| `disk_image_platforms()` | no | the same for a computer's disk form: the platforms whose disks `create_vm` boots without emulation. The default is none; local Docker returns this machine's where it can pass `/dev/kvm` to a container, and `modal_vm` `linux/amd64`, booting the launcher as its sandbox's image under nested virtualization | +| `disk_image_platforms()` | no | the same for a computer's disk form: the platforms whose disks `create_vm` boots without emulation. The default is none; local Docker returns this machine's where it can pass `/dev/kvm` to a container, and `modal_vm` `linux/amd64` and `windows/amd64`, booting the launcher as its sandbox's image under nested virtualization, a Windows disk's base carried beside it | | `vm_bundle_platforms()` | no | the same for a computer's vm_bundle form, a whole saved VM (`format = "lume"`): the platforms, as `/`, whose VM bundles `create_vm` boots. The default is none; `lume` returns `macos/arm64` on an Apple Silicon Mac. A provider that names one implements the VM-bundle methods below too | | `reset_vm(sandbox, *, image, env)` | no | a computer env's reset of a disk `create_vm` booted: boot it afresh on the same sandbox and ports. The default raises `NotImplementedError`; local Docker and `modal_vm` run the launcher's `launch reset` in the sandbox hosting it, with `env` on its stdin | | `has_vm_bundle(image)`, `delete_vm_bundle(image)`, `open_builder(image, name, build)`, `keep_builder(builder)`, `ssh_password` | when `vm_bundle_platforms()` names a platform | a macOS computer's compose, and a deploy on a Mac that doesn't hold the env's VM bundle: `open_builder` boots a clone of the base `image`, named `name`, with the folder `build` (named `agentenv-build`) shared read-write, and its sandbox's `exec_with_output` runs over ssh as the session user, whose sudo reads `ssh_password` from that folder; `keep_builder` shuts it down from inside and keeps it, returning `local:`; `has_vm_bundle` says whether this machine holds a `local:` VM bundle, and `delete_vm_bundle` deletes one, as a compose does one that failed its probe. Here `create_vm` boots `image`, `local:` or a registry reference pinned by digest, with `env` as the protocol's seed in a folder the VM shares, and `reset_vm` clones it afresh. See `lume/provider.py` | From 55e2ac64a223a44d0ff031d7d521e2d3c81c0484 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sat, 7 Nov 2026 16:59:04 -0800 Subject: [PATCH 11/12] refactor(computer): one thin-disk rule, the export's cache named in one place, and a disk form's own ready timeout - _layout gives a disk's id and its base's first layer, for the machine image and the export alike. - _Cached names an export's files: the disk, its guest file, the file naming its base's export, its work folder. - modal_vm mounts the disk, base and guest file where a disk's own image holds them (DISK_FILE and its siblings). - A disk form says how long a boot of it may take to be ready (ready_timeout_s); windows-11 says 600 s, which replaces the os_type branch in boot.ready_timeout, now given the env and the form it boots. - The export tests share one fake Docker; _compressed no longer takes the image for its message. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/artifact/artifacts/computer.py | 3 + src/agent_env/computers/README.md | 11 +- .../artifacts/windows-11/artifact.toml | 1 + src/agent_env/env/envs/computer/utils/boot.py | 18 +- .../env/envs/computer/utils/probing.py | 2 +- .../env_providers/env_computer_provider.py | 6 +- .../sandbox_providers/local_docker/disk.py | 112 +++++++---- .../sandbox_providers/modal_vm_sandbox.py | 13 +- tst/unit/env/envs/test_computer_env.py | 10 +- tst/unit/env/envs/test_computer_windows.py | 12 +- tst/unit/env/envs/test_disk_recipe.py | 181 +++++++----------- 11 files changed, 188 insertions(+), 181 deletions(-) diff --git a/src/agent_env/artifact/artifacts/computer.py b/src/agent_env/artifact/artifacts/computer.py index 879511ec6..6fe1dc62d 100644 --- a/src/agent_env/artifact/artifacts/computer.py +++ b/src/agent_env/artifact/artifacts/computer.py @@ -146,6 +146,9 @@ class DiskForm(BaseModel): input_channel: Literal["nocloud"] firmware: list[Literal["efi"]] = Field(default=["efi"], min_length=1) min_disk_gb: int = Field(ge=1, description="Its virtual size, which a deploy's disk is at least") + ready_timeout_s: float | None = Field(default=None, gt=0, exclude_if=lambda value: value is None, description=( + "How long a boot of it may take to be ready, for a computer agent-env's default isn't long enough for; left " + "out when unset, so a disk form without it is stored as it was before it existed")) class VmBundleForm(BaseModel): diff --git a/src/agent_env/computers/README.md b/src/agent_env/computers/README.md index 8d7853de4..0ab0a76c9 100644 --- a/src/agent_env/computers/README.md +++ b/src/agent_env/computers/README.md @@ -172,11 +172,12 @@ pulls it. Gate 3 probes it at full. At the session user's sign-in, `agentenv-session`, run windowless by `pythonw`, reads the deploy's environment from the NoCloud seed, which the launcher attaches as a removable USB drive, since Windows mounts a FAT filesystem with no partition table on removable media alone, and starts each layer's services in that session, again whenever one exits. A Windows computer boots -with 4 vCPUs and 8 GB, on local Docker with KVM and on `modal_vm`, and may take 10 minutes to be ready, for its first -sign-in and, on `modal_vm`, its base expanded first; it has computer-exec and data/add, through the protocol's Windows -defaults, so the probe runs its fixture there as on Linux, with Python one-liners for the commands it would give a -shell. On `modal_vm` it runs in your own Modal workspace, under the evaluation's terms or a licence of your own; -whether they allow it there is yours to check. A Windows computer composes its driver alone for now, no app. +with 4 vCPUs and 8 GB, on local Docker with KVM and on `modal_vm`, and may take 10 minutes to be ready, as its disk +form's `ready_timeout_s` says, for its first sign-in and, on `modal_vm`, its base expanded first; it has computer-exec +and data/add, through the protocol's Windows defaults, so the probe runs its fixture there as on Linux, with Python +one-liners for the commands it would give a shell. On `modal_vm` it runs in your own Modal workspace, under the +evaluation's terms or a licence of your own; whether they allow it there is yours to check. A Windows computer composes +its driver alone for now, no app. This bundle has no tasks of its own: other bundles pull its computers by alias, and `agent-env run computers` has nothing to run. diff --git a/src/agent_env/computers/artifacts/windows-11/artifact.toml b/src/agent_env/computers/artifacts/windows-11/artifact.toml index 06d92f9ed..988906eee 100644 --- a/src/agent_env/computers/artifacts/windows-11/artifact.toml +++ b/src/agent_env/computers/artifacts/windows-11/artifact.toml @@ -9,6 +9,7 @@ init = "agentenv-session" input_channel = "nocloud" firmware = ["efi"] min_disk_gb = 64 +ready_timeout_s = 600 [config.resolution] default = "1920x1080" diff --git a/src/agent_env/env/envs/computer/utils/boot.py b/src/agent_env/env/envs/computer/utils/boot.py index 50d918483..2ca24de96 100644 --- a/src/agent_env/env/envs/computer/utils/boot.py +++ b/src/agent_env/env/envs/computer/utils/boot.py @@ -28,14 +28,12 @@ from agent_env.artifact.artifacts.computer import ComputerArtifact from agent_env.attribution import Attribution from agent_env.env.envs.computer.app_env import ComputerAppEnv + from agent_env.env.envs.computer.env import ComputerEnv from agent_env.providers.sandbox_providers.sandbox import NetworkPolicy, Sandbox from agent_env.providers.sandbox_providers.sandbox_provider import SandboxProvider #: How long a computer may take from its container starting to its driver saying it's ready. READY_TIMEOUT_S = 180.0 -#: How long a Windows computer may take: its first boot from a composed disk signs its session user in, and a provider -#: that carries its disk compressed expands the base first. -WINDOWS_READY_TIMEOUT_S = 600.0 #: How much longer than the slowest starting app's own readiness timeout a computer may take, for the rest of it first. APP_READY_MARGIN_S = 30.0 #: How long one computer-ready status may take: the driver's own checks, then every app's server at once. @@ -194,12 +192,14 @@ async def discard(sandbox: Sandbox) -> None: await asyncio.to_thread(remove_local_work_dir, sandbox.sandbox_id) -def ready_timeout(apps: list[ComputerAppEnv], os_type: str) -> float: - """How long a computer of ``os_type`` and ``apps`` may take to be ready: ``READY_TIMEOUT_S``, Windows' - ``WINDOWS_READY_TIMEOUT_S``, or longer when an app that starts with it there gives itself longer.""" - least = WINDOWS_READY_TIMEOUT_S if os_type == "windows" else READY_TIMEOUT_S - return max([least, *(launch.ready.timeout_s + APP_READY_MARGIN_S for app in apps - if (launch := app.launch(os_type)) and launch.at == "boot" and launch.ready)]) +def ready_timeout(env: ComputerEnv, form: Optional[str]) -> float: + """How long ``env``'s computer booted in ``form`` may take to be ready: as long as its disk form says + (``ready_timeout_s``) when it boots that, else ``READY_TIMEOUT_S``, or longer when an app that starts with it there + gives itself longer.""" + disk, os_type = env.computer.forms.disk, env.computer.os.type + least = (disk.ready_timeout_s if form == "disk" and disk is not None else None) or READY_TIMEOUT_S + return max([least, *(launch.ready.timeout_s + APP_READY_MARGIN_S for app in env.composed_apps() + if (launch := app.launch(os_type)) and launch.at == "boot" and launch.ready)]) async def ready(url: str, timeout: float, token: Optional[str] = None, *, answering: bool = False) -> dict: diff --git a/src/agent_env/env/envs/computer/utils/probing.py b/src/agent_env/env/envs/computer/utils/probing.py index 3989a4b10..a111fb0af 100644 --- a/src/agent_env/env/envs/computer/utils/probing.py +++ b/src/agent_env/env/envs/computer/utils/probing.py @@ -165,7 +165,7 @@ async def _probe(links: list[SandboxProvider], env: ComputerEnv, driver: Compute try: booted = await boot.boot(link, image=image, port=driver.port, environment=environment, token=token, network_policy=None, timeout=_PROBE_TTL_S, attribution=None, - ready_s=boot.ready_timeout(env.composed_apps(), env.computer.os.type), form=form, + ready_s=boot.ready_timeout(env, form), form=form, os_type=env.computer.os.type) except Exception as e: problems.append(f"{type(link).__name__}: {e}") diff --git a/src/agent_env/providers/env_providers/env_computer_provider.py b/src/agent_env/providers/env_providers/env_computer_provider.py index bf4897315..564d13624 100644 --- a/src/agent_env/providers/env_providers/env_computer_provider.py +++ b/src/agent_env/providers/env_providers/env_computer_provider.py @@ -142,7 +142,7 @@ async def _boot(self, env: ComputerEnv, builds: list[tuple[str, str, dict]], par sandbox_provider, image=build["image"], port=env.driver.port, network_policy=policy, token=token, environment=_environment(env, computer, values, params), timeout=ttl_seconds, attribution=attribution, disk_size_gb=disk_size_gb, cpu=cpu, memory_mb=memory_mb, - ready_s=ready_timeout(env.composed_apps(), env.computer.os.type), form=form, held=bool(params.site_routing), + ready_s=ready_timeout(env, form), form=form, held=bool(params.site_routing), os_type=env.computer.os.type) self._container_sandboxes.append(booted.sandbox) self._environment_sandboxes[name] = booted.sandbox @@ -189,7 +189,7 @@ async def reboot(self, env: ComputerEnv, deployed: DeployedSandboxEnv) -> None: env=environment) except NotImplementedError as e: # a reset the env supports, which this provider can't carry out raise RuntimeError(f"'{env.id}' can't be reset on {type(sandbox_provider).__name__}: {e}") from None - card = await ready(deployed.environment_url, ready_timeout(env.composed_apps(), env.computer.os.type), + card = await ready(deployed.environment_url, ready_timeout(env, record.get("form")), deployed.control_token, answering=bool(params.site_routing)) _require_name(env, card) valid_for_s = (record.get("sites") or {}).get("valid_for_s", 0) @@ -227,7 +227,7 @@ async def _launch_held_apps(env: ComputerEnv, record: DeployedSandboxEnv, url: s answers then, as its boot's card may list an app by its name alone. Each is launched as the child its card lists, as a computer named for its app, a browser alone, has that name too.""" os_type = env.computer.os.type - timeout = ready_timeout(env.composed_apps(), os_type) + timeout = ready_timeout(env, record.metadata["computer"].get("form")) held = [app for app in env.composed_apps() if (launch := app.launch(os_type)) is not None and launch.at == "boot"] launchers: dict[str, protocol_v1.ChildEnv] = {} if held: diff --git a/src/agent_env/providers/sandbox_providers/local_docker/disk.py b/src/agent_env/providers/sandbox_providers/local_docker/disk.py index 523a00718..146607dad 100644 --- a/src/agent_env/providers/sandbox_providers/local_docker/disk.py +++ b/src/agent_env/providers/sandbox_providers/local_docker/disk.py @@ -39,27 +39,39 @@ def kvm_platforms() -> frozenset[str]: async def machine_image(disk: str) -> str: """An image of the launcher holding ``disk``'s files for this machine's platform, built unless held: one for each - image ``disk`` names, by its id, so a disk built again under its tag gets a machine of its own. A disk of more than - one layer is a thin one on ``DISK_BASE_FILE``, which gets a step of its own, so every machine on that base shares - one copy of it.""" + image ``disk`` names, by its id, so a disk built again under its tag gets a machine of its own. A thin disk's base + gets a step of its own, so every machine on that base shares one copy of it.""" source = await asyncio.to_thread(launcher) - identity, layers = (await asyncio.to_thread(docker, "image", "inspect", "-f", "{{.Id}} {{len .RootFS.Layers}}", - disk)).split() - tag = "agentenv-vm:" + hashlib.sha256(f"{source} {disk} {identity}".encode()).hexdigest()[:16] + layout = await asyncio.to_thread(_layout, disk) + tag = "agentenv-vm:" + hashlib.sha256(f"{source} {disk} {layout.identity}".encode()).hexdigest()[:16] if not await asyncio.to_thread(present, tag): - layers = int(layers) - await asyncio.to_thread(_build, _machine_dockerfile(source, disk, layers), tag) + await asyncio.to_thread(_build, _machine_dockerfile(source, disk, thin=layout.base is not None), tag) return tag -def _machine_dockerfile(launcher: str, disk: str, layers: int) -> str: +def _machine_dockerfile(launcher: str, disk: str, thin: bool) -> str: folder = f"/{PurePosixPath(DISK_FILE).parent}/" - if layers == 1: + if not thin: return f"FROM {launcher}\nCOPY --from={disk} {folder} {folder}\n" return (f"FROM {launcher}\nCOPY --from={disk} /{DISK_BASE_FILE} /{DISK_BASE_FILE}\n" f"COPY --from={disk} /{DISK_FILE} /{DISK_GUEST_FILE} {folder}\n") +@dataclass(frozen=True) +class _Layout: + """A disk image's id and, for a thin disk, one of more than one layer on ``DISK_BASE_FILE``, its ``base``: the + digest of its first layer, the base's own, which every disk composed on that base shares; None for a whole one.""" + + identity: str + base: str | None + + +def _layout(disk: str) -> _Layout: + identity, layers, first = docker("image", "inspect", "-f", + "{{.Id}} {{len .RootFS.Layers}} {{index .RootFS.Layers 0}}", disk).split() + return _Layout(identity, first if int(layers) > 1 else None) + + def launcher_args(ports: list[int], env: dict[str, str], cpu: float, memory: int) -> list[str]: """The ``docker run`` arguments that pass the launcher KVM and its ``launcher_variables``.""" return ["--device", KVM, *(arg for name, value in launcher_variables(ports, env, cpu, memory).items() @@ -81,41 +93,69 @@ def exported_disk(image: str, platform: str) -> Exported: Docker unless an export of it is there already: once for each composed disk, by its ref, whose tag is its composer's key; a thin disk's base once for each base, by its layer, which every disk composed on it shares. ``platform`` is the disk's own, which this machine's Docker may not be.""" - path = cache_root() / "disks" / f"{hashlib.sha256(image.encode()).hexdigest()[:32]}.img.zst" - with holding_locks([f"disk-export:{path.name}"]): - if not path.exists(): - _export(image, platform, path) - base, guest = path.with_name(path.name + ".base"), path.with_name(path.name + ".guest") - return Exported(path, path.with_name(base.read_text()) if base.exists() else None, guest if guest.exists() else None) + cached = _Cached(cache_root() / "disks" / f"{hashlib.sha256(image.encode()).hexdigest()[:32]}.img.zst") + with holding_locks([f"disk-export:{cached.disk.name}"]): + if not cached.disk.exists(): + _export(image, platform, cached) + return cached.exported() + + +@dataclass(frozen=True) +class _Cached: + """A disk's export in this machine's cache, named for its image's ref: the ``disk``, compressed, and beside it its + guest file, a file naming its base's export, which every disk on that base shares, and the folder an export of it + works in.""" + + disk: Path + + @property + def guest(self) -> Path: + return self._beside("guest") + + @property + def base_link(self) -> Path: + return self._beside("base") + + @property + def work(self) -> Path: + return self._beside("work") + + def base_of(self, layer: str) -> Path: + """The export of the base whose first layer is ``layer``.""" + return self.disk.with_name(f"base-{hashlib.sha256(layer.encode()).hexdigest()[:32]}.img.zst") + + def exported(self) -> Exported: + base = self.disk.with_name(self.base_link.read_text()) if self.base_link.exists() else None + return Exported(self.disk, base, self.guest if self.guest.exists() else None) + + def _beside(self, what: str) -> Path: + return self.disk.with_name(f"{self.disk.name}.{what}") -def _export(image: str, platform: str, path: Path) -> None: +def _export(image: str, platform: str, cached: _Cached) -> None: """Signed in to the image store first, as a boot's pull is, since ``docker create`` pulls ``image`` when it isn't here; for ``platform``, as the image's index holds that one alone, which a pull for this machine's wouldn't find. The disk is placed last, so one that's there was exported whole, with its base and guest file. Its work folder and - container are named for ``path`` and cleared first, under its lock, so an export killed outright leaves no copy of + container are named for the export and cleared first, under its lock, so an export killed outright leaves no copy of a base behind once the disk is exported again; the container by the folder's whole path, as another user's export of the same disk through this machine's Docker holds a lock of its own.""" - work = path.with_name(path.name + ".work") - container = f"agentenv-export-{hashlib.sha256(str(work).encode()).hexdigest()[:24]}" - _cleared(work, container) - work.mkdir(parents=True) + container = f"agentenv-export-{hashlib.sha256(str(cached.work).encode()).hexdigest()[:24]}" + _cleared(cached.work, container) + cached.work.mkdir(parents=True) docker_login(get_config().get_image_store().auth(image)) docker("create", "--name", container, "--platform", platform, image, "disk") try: - if (guest := _copied(container, DISK_GUEST_FILE, work / "guest")) is not None: - guest.replace(path.with_name(path.name + ".guest")) - layers, first = docker("image", "inspect", "-f", "{{len .RootFS.Layers}} {{index .RootFS.Layers 0}}", - image).split() - if int(layers) > 1: - base = path.with_name(f"base-{hashlib.sha256(first.encode()).hexdigest()[:32]}.img.zst") + if (guest := _copied(container, DISK_GUEST_FILE, cached.work / "guest")) is not None: + guest.replace(cached.guest) + if (layer := _layout(image).base) is not None: + base = cached.base_of(layer) with holding_locks([f"disk-export:{base.name}"]): if not base.exists(): - _compressed(container, DISK_BASE_FILE, base, work, image) - path.with_name(path.name + ".base").write_text(base.name) - _compressed(container, DISK_FILE, path, work, image) + _compressed(container, DISK_BASE_FILE, base, cached.work) + cached.base_link.write_text(base.name) + _compressed(container, DISK_FILE, cached.disk, cached.work) finally: - _cleared(work, container) + _cleared(cached.work, container) def _cleared(work: Path, container: str) -> None: @@ -135,15 +175,15 @@ def _copied(container: str, file: str, to: Path) -> Path | None: return to -def _compressed(container: str, file: str, path: Path, work: Path, image: str) -> None: +def _compressed(container: str, file: str, path: Path, work: Path) -> None: """``file`` of ``container`` compressed with zstd to ``path``, through ``work``.""" raw, compressed = work / PurePosixPath(file).name, work / path.name docker("cp", f"{container}:/{file}", str(raw)) with raw.open("rb") as source, compressed.open("wb") as out: - done = subprocess.run(["docker", "run", "--rm", "-i", "--entrypoint", "zstd", launcher(), "-q", "-3", "-T0", "-c"], - stdin=source, stdout=out, stderr=subprocess.PIPE, text=False) + done = subprocess.run(["docker", "run", "--rm", "-i", "--entrypoint", "zstd", launcher(), "-q", "-3", "-T0", + "-c"], stdin=source, stdout=out, stderr=subprocess.PIPE, text=False) if done.returncode: - raise RuntimeError(f"compressing {image}'s {file} failed: {done.stderr.decode().strip()}") + raise RuntimeError(f"compressing a disk's {file} failed: {done.stderr.decode().strip()}") raw.unlink() os.replace(compressed, path) diff --git a/src/agent_env/providers/sandbox_providers/modal_vm_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_vm_sandbox.py index 76737fd76..c0b31491e 100644 --- a/src/agent_env/providers/sandbox_providers/modal_vm_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_vm_sandbox.py @@ -16,8 +16,9 @@ With a computer's disk form as its image, ``create_vm`` makes a VM sandbox with nested virtualization whose own image is the QEMU launcher, with no Docker in it, booting the disk mounted at create from its export on this machine -(``local_docker.disk.exported_disk``), with a thin disk's base and guest file beside it; ``reset_vm`` boots it afresh in the same sandbox. ``[sandbox.providers.modal_vm]`` -takes ``region``, where every sandbox it makes goes, and ``idle_timeout``, in seconds, after which an idle one ends. +(``local_docker.disk.exported_disk``), with a thin disk's base and guest file beside it; ``reset_vm`` boots it +afresh in the same sandbox. ``[sandbox.providers.modal_vm]`` takes ``region``, where every sandbox it makes goes, and +``idle_timeout``, in seconds, after which an idle one ends. """ from __future__ import annotations @@ -41,6 +42,7 @@ _tunnel_url, _modal_network_kwargs, ) +from agent_env.artifact.artifacts.computer import DISK_BASE_FILE, DISK_FILE, DISK_GUEST_FILE from agent_env.attribution import Attribution from agent_env.config import get_config from agent_env.env.published_images import LAUNCHER_IMAGE, published_digest @@ -113,9 +115,10 @@ while true; do sleep 3600; done ''' _KEEPALIVE_CMD = ("sh", "-c", _MAIN_SCRIPT) -#: Where a computer's disk is mounted on the launcher's image, compressed; the launcher expands it onto the sandbox's disk. -#: A thin disk's base is mounted beside it, compressed too, and expanded beside it; its guest file, beside both. -_MOUNTED_DISK, _MOUNTED_BASE, _MOUNTED_GUEST = "/disk/disk.img.zst", "/disk/base.img.zst", "/disk/guest" +#: Where a computer's disk, compressed, a thin disk's base, compressed too, and its guest file are mounted on the +#: launcher's image: where a disk's own image holds them, as the launcher reads the guest file beside the disk. It +#: expands the disk and the base onto the sandbox's disk. +_MOUNTED_DISK, _MOUNTED_BASE, _MOUNTED_GUEST = f"/{DISK_FILE}.zst", f"/{DISK_BASE_FILE}.zst", f"/{DISK_GUEST_FILE}" #: The platform of a disk's image in the image store, a Windows disk's too: its guest file names the OS it boots. _DISK_IMAGE_PLATFORM = "linux/amd64" #: The memory a disk's sandbox has beyond its VM's: QEMU's own, and the sandbox's. diff --git a/tst/unit/env/envs/test_computer_env.py b/tst/unit/env/envs/test_computer_env.py index 16debc7cc..373e6d1eb 100644 --- a/tst/unit/env/envs/test_computer_env.py +++ b/tst/unit/env/envs/test_computer_env.py @@ -1043,7 +1043,8 @@ async def test_a_computer_routing_sites_launches_its_held_boot_apps_once_listed_ order = [] apps = [SimpleNamespace(environment_name=name, launch=lambda os_type, at=at: SimpleNamespace(at=at)) for name, at in [("mail", "boot"), ("music", "demand"), ("notes", "boot")]] - env = SimpleNamespace(computer=SimpleNamespace(os=SimpleNamespace(type="linux")), composed_apps=lambda: apps) + env = SimpleNamespace(computer=SimpleNamespace(os=SimpleNamespace(type="linux"), forms=SimpleNamespace(disk=None)), + composed_apps=lambda: apps) def app_card(name): return {"name": name, "url": f"/svc/{name}/agentenv", "capabilities": {"extensions": [ @@ -1066,7 +1067,8 @@ async def ready(url, timeout, token): monkeypatch.setattr(env_computer_provider, "listed", listed) monkeypatch.setattr(env_computer_provider, "ready", ready) monkeypatch.setattr(env_computer_provider, "ready_timeout", lambda apps, os_type: 30.0) - record = SimpleNamespace(invoke=invoke, control_token="t0ken", environment_card={"name": "booted"}) + record = SimpleNamespace(invoke=invoke, control_token="t0ken", environment_card={"name": "booted"}, + metadata={"computer": {"form": "container"}}) await env_computer_provider._launch_held_apps(env, record, "http://computer") assert order == [("listed", "http://computer", ["mail", "notes"]), (COMPUTER_APP, "launch", "mail", True, "mail"), @@ -1577,8 +1579,8 @@ def test_an_app_that_requires_nothing_still_gets_a_venv_for_its_own_server(base, assert "/opt/agentenv/apps/mail/venv/bin/pip install --no-index --no-deps /tmp/agentenv-packages/wheels/apps-mail/*.whl" in ( build["dockerfile"]) assert build["confs"]["agentenv-app-mail.conf"] == MAIL_SERVER - assert boot.ready_timeout([app], "linux") == (270.0 if at == "boot" else boot.READY_TIMEOUT_S) - assert boot.ready_timeout([app], "windows") == boot.WINDOWS_READY_TIMEOUT_S + assert boot.ready_timeout(SimpleNamespace(computer=base, composed_apps=lambda: [app]), "container") == ( + 270.0 if at == "boot" else boot.READY_TIMEOUT_S) def test_the_locks_debs_and_wheels_are_mounted_into_the_runs_that_install_them_never_copied_into_a_layer( diff --git a/tst/unit/env/envs/test_computer_windows.py b/tst/unit/env/envs/test_computer_windows.py index a2b584be9..4a4c8c234 100644 --- a/tst/unit/env/envs/test_computer_windows.py +++ b/tst/unit/env/envs/test_computer_windows.py @@ -6,6 +6,7 @@ import json import threading import time +import tomllib from types import SimpleNamespace from datetime import datetime, timedelta, timezone from pathlib import Path, PureWindowsPath @@ -21,7 +22,7 @@ from agent_env.env.env import Env from agent_env.env.envs.computer.driver_env import ComputerDriverEnv from agent_env.env.envs.computer.env import ComputerEnv -from agent_env.env.envs.computer.utils import composer, gate, windows_disk +from agent_env.env.envs.computer.utils import boot, composer, gate, windows_disk from agent_env.env.envs.computer.utils.composer import Composing from agent_env.env.envs.computer.utils.install import WindowsInstall, WindowsService from agent_env.providers.sandbox_providers.sandbox_provider import SandboxProvider @@ -315,6 +316,15 @@ def test_a_machine_without_the_base_takes_the_image_stores_unless_its_evaluation assert (base.expires - datetime.now(timezone.utc)).days == 59 +def test_a_windows_computer_is_waited_on_as_long_as_its_disk_form_says(local_stores, cli_routing): + """The shipped base says 10 minutes; a base that says nothing gets agent-env's default.""" + shipped = tomllib.loads((windows_disk.RECIPE.parents[1] / "artifacts" / "windows-11" / "artifact.toml").read_text()) + declared = _base(forms={"disk": shipped["forms"]["disk"]}) + + assert boot.ready_timeout(SimpleNamespace(computer=declared, composed_apps=list), "disk") == 600 + assert boot.ready_timeout(SimpleNamespace(computer=_base(), composed_apps=list), "disk") == boot.READY_TIMEOUT_S + + def test_a_windows_computers_display_is_1920x1080_for_now(local_stores, cli_routing): base = _base() diff --git a/tst/unit/env/envs/test_disk_recipe.py b/tst/unit/env/envs/test_disk_recipe.py index 20f5d7276..f673c8252 100644 --- a/tst/unit/env/envs/test_disk_recipe.py +++ b/tst/unit/env/envs/test_disk_recipe.py @@ -536,7 +536,7 @@ async def test_a_machine_copies_a_thin_disks_base_in_a_step_of_its_own_so_machin built = [] monkeypatch.setattr(local_disk, "launcher", lambda: "launcher:1") monkeypatch.setattr(local_disk, "present", lambda tag: False) - monkeypatch.setattr(local_disk, "docker", lambda *args: f"sha256:1 {layers}") + monkeypatch.setattr(local_disk, "docker", lambda *args: f"sha256:1 {layers} sha256:first") monkeypatch.setattr(local_disk, "_build", lambda text, tag: built.append(text)) await local_disk.machine_image("disk:1") @@ -550,7 +550,7 @@ async def test_a_disk_built_again_under_its_tag_gets_a_machine_of_its_own(monkey identity = "sha256:1" monkeypatch.setattr(local_disk, "launcher", lambda: "launcher:1") monkeypatch.setattr(local_disk, "present", lambda tag: True) - monkeypatch.setattr(local_disk, "docker", lambda *args: f"{identity} 2") + monkeypatch.setattr(local_disk, "docker", lambda *args: f"{identity} 2 sha256:first") first = await local_disk.machine_image("disk:1") again = await local_disk.machine_image("disk:1") @@ -560,167 +560,114 @@ async def test_a_disk_built_again_under_its_tag_gets_a_machine_of_its_own(monkey assert first == again != rebuilt -def test_a_disk_is_exported_compressed_into_the_cache_once_for_each_ref(tmp_path, monkeypatch): - """By this machine's Docker, signed in to the image store as a boot's pull is: the disk copied out of a container of - its image, then compressed in the launcher.""" +EXPORTED = "localhost:5000/agentenv-computers:" + "1" * 64 + "@sha256:" + "a" * 64 + + +@pytest.fixture +def exporting(tmp_path, monkeypatch): + """This machine's Docker as an export sees it, signed in to a store whose credentials name the ref, every call kept + in ``log``, ``ran`` (docker's own) and ``runs`` (the processes it starts): the disk a whole one unless ``layers`` + says more, its first layer ``first``, and its guest file missing unless ``guest`` holds its text, or the error + copying it raises; zstd as a ``zstd:`` prefix.""" + fake = SimpleNamespace(layers=1, first="sha256:linux", guest=None, log=[], ran=[], runs=[], + cache=tmp_path / "cache" / "agentenv" / "disks") monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache")) monkeypatch.setattr(local_disk, "launcher", lambda: "agentenv-qemu-launcher:src-1") - ran, compressed = [], [] store = SimpleNamespace(auth=lambda ref: f"credentials for {ref}") monkeypatch.setattr(local_disk, "get_config", lambda: SimpleNamespace(get_image_store=lambda: store)) - monkeypatch.setattr(local_disk, "docker_login", lambda auth: ran.append(("login", auth))) + monkeypatch.setattr(local_disk, "docker_login", lambda auth: fake.ran.append(("login", auth))) def docker(*args): - ran.append(args) - if args[0] == "cp" and args[1].endswith("/disk/guest"): - raise DockerFailed("docker cp container1:/disk/guest failed: Error response from daemon: Could not find the " - "file /disk/guest in container container1", 1) + fake.ran.append(args) + fake.log.append(args[0]) if args[0] == "cp": - Path(args[2]).write_bytes(b"disk of " + args[1].partition(":")[2].encode()) - return "1 sha256:linux" if args[0] == "image" else "" + file = args[1].partition(":")[2] + if file.endswith("/guest") and not isinstance(fake.guest, str): + raise fake.guest or DockerFailed(f"docker cp {args[1]} failed: Error response from daemon: Could not " + f"find the file {file} in container", 1) + Path(args[2]).write_bytes(fake.guest.encode() if file.endswith("/guest") else f"copy of {file}".encode()) + return f"sha256:image {fake.layers} {fake.first}" if args[0] == "image" else "" def run(argv, stdin=None, stdout=None, **kwargs): + fake.runs.append(argv) + fake.log.append(argv[1]) if argv[:2] == ["docker", "run"]: - compressed.append(argv) stdout.write(b"zstd:" + stdin.read()) return subprocess.CompletedProcess(argv, 0, b"", b"") monkeypatch.setattr(local_disk, "docker", docker) monkeypatch.setattr(local_disk.subprocess, "run", run) - first = "localhost:5000/agentenv-computers:" + "1" * 64 + "@sha256:" + "a" * 64 + return fake - exported = [local_disk.exported_disk(image, "linux/amd64") for image in (first, first, first.replace("1", "2"))] - assert exported[0] == exported[1] != exported[2] and exported[0].disk.parent == tmp_path / "cache" / "agentenv" / "disks" - assert exported[0].disk.read_bytes() == b"zstd:disk of /disk/disk.img" +def test_a_disk_is_exported_compressed_into_the_cache_once_for_each_ref(exporting): + """By this machine's Docker, signed in to the image store as a boot's pull is: the disk copied out of a container of + its image, then compressed in the launcher.""" + exported = [local_disk.exported_disk(image, "linux/amd64") for image in (EXPORTED, EXPORTED, + EXPORTED.replace("1", "2"))] + + assert exported[0] == exported[1] != exported[2] and exported[0].disk.parent == exporting.cache + assert exported[0].disk.read_bytes() == b"zstd:copy of /disk/disk.img" assert exported[0].disk.name.endswith(".img.zst") and exported[0].base is exported[0].guest is None - assert [args[0] for args in ran] == ["login", "create", "cp", "image", "cp"] * 2 - assert ran[0] == ("login", f"credentials for {first}") and ran[1][1] == "--name" - assert ran[1][2].startswith("agentenv-export-") and ran[1][3:] == ("--platform", "linux/amd64", first, "disk") + assert [args[0] for args in exporting.ran] == ["login", "create", "cp", "image", "cp"] * 2 + assert exporting.ran[0] == ("login", f"credentials for {EXPORTED}") and exporting.ran[1][1] == "--name" + assert exporting.ran[1][2].startswith("agentenv-export-") + assert exporting.ran[1][3:] == ("--platform", "linux/amd64", EXPORTED, "disk") + compressed = [argv for argv in exporting.runs if argv[:2] == ["docker", "run"]] assert compressed[0] == ["docker", "run", "--rm", "-i", "--entrypoint", "zstd", "agentenv-qemu-launcher:src-1", "-q", "-3", "-T0", "-c"] and len(compressed) == 2 - assert sorted(p.name for p in exported[0].disk.parent.iterdir()) == sorted({exported[0].disk.name, - exported[2].disk.name}) + assert sorted(p.name for p in exporting.cache.iterdir()) == sorted({exported[0].disk.name, exported[2].disk.name}) -def test_an_export_killed_outright_leaves_nothing_behind_once_the_disk_is_exported_again(tmp_path, monkeypatch): +def test_an_export_killed_outright_leaves_nothing_behind_once_the_disk_is_exported_again(exporting, monkeypatch): """Its work folder, holding as much of a base as it had copied, and its container are named for the disk, and cleared under the disk's lock first.""" - monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache")) - monkeypatch.setattr(local_disk, "launcher", lambda: "agentenv-qemu-launcher:src-1") - monkeypatch.setattr(local_disk, "get_config", lambda: SimpleNamespace(get_image_store=lambda: SimpleNamespace( - auth=lambda ref: None))) - monkeypatch.setattr(local_disk, "docker_login", lambda auth: None) - image = "localhost:5000/agentenv-computers:" + "1" * 64 + "@sha256:" + "a" * 64 - disks = tmp_path / "cache" / "agentenv" / "disks" - left = disks / f"{hashlib.sha256(image.encode()).hexdigest()[:32]}.img.zst.work" + left = exporting.cache / f"{hashlib.sha256(EXPORTED.encode()).hexdigest()[:32]}.img.zst.work" left.mkdir(parents=True) (left / "base.img").write_bytes(b"half a base") - events = [] + docker = local_disk.docker - def docker(*args): - events.append(args[:2]) - if args[0] == "cp": - assert not (left / "base.img").exists() - Path(args[2]).write_bytes(b"disk") - return "1 sha256:linux" if args[0] == "image" else "" + def copying_with_no_half_base(*args): + assert args[0] != "cp" or not (left / "base.img").exists() + return docker(*args) - def run(argv, stdin=None, stdout=None, **kwargs): - events.append(tuple(argv[1:3])) - if argv[:2] == ["docker", "run"]: - stdout.write(stdin.read()) - return subprocess.CompletedProcess(argv, 0, b"", b"") - - monkeypatch.setattr(local_disk, "docker", docker) - monkeypatch.setattr(local_disk.subprocess, "run", run) + monkeypatch.setattr(local_disk, "docker", copying_with_no_half_base) - exported = local_disk.exported_disk(image, "linux/amd64") + exported = local_disk.exported_disk(EXPORTED, "linux/amd64") - assert events[:2] == [("rm", "-f"), ("create", "--name")] and events[-1] == ("rm", "-f") - assert sorted(path.name for path in disks.iterdir()) == [exported.disk.name, exported.disk.name + ".guest"] + assert exporting.log[:2] == ["rm", "create"] and exporting.log[-1] == "rm" + assert sorted(path.name for path in exporting.cache.iterdir()) == [exported.disk.name] -def test_two_users_exports_of_one_disk_through_one_docker_name_their_containers_apart(tmp_path, monkeypatch): +def test_two_users_exports_of_one_disk_through_one_docker_name_their_containers_apart(exporting, tmp_path, + monkeypatch): """Each holds a lock of its own, so neither clears the other's container.""" - monkeypatch.setattr(local_disk, "launcher", lambda: "agentenv-qemu-launcher:src-1") - monkeypatch.setattr(local_disk, "get_config", lambda: SimpleNamespace(get_image_store=lambda: SimpleNamespace( - auth=lambda ref: None))) - monkeypatch.setattr(local_disk, "docker_login", lambda auth: None) - created = [] - - def docker(*args): - if args[0] == "create": - created.append(args[2]) - if args[0] == "cp": - Path(args[2]).write_bytes(b"disk") - return "1 sha256:linux" if args[0] == "image" else "" - - def run(argv, stdin=None, stdout=None, **kwargs): - if argv[:2] == ["docker", "run"]: - stdout.write(stdin.read()) - return subprocess.CompletedProcess(argv, 0, b"", b"") - - monkeypatch.setattr(local_disk, "docker", docker) - monkeypatch.setattr(local_disk.subprocess, "run", run) - image = "localhost:5000/agentenv-computers:" + "1" * 64 + "@sha256:" + "a" * 64 - for user in ("one", "two"): monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / user / "cache")) - local_disk.exported_disk(image, "linux/amd64") - - assert len(set(created)) == 2 + local_disk.exported_disk(EXPORTED, "linux/amd64") + assert len({args[2] for args in exporting.ran if args[0] == "create"}) == 2 -def test_an_export_that_cant_copy_a_disks_guest_file_fails_rather_than_cache_the_disk_without_it(tmp_path, - monkeypatch): - monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache")) - monkeypatch.setattr(local_disk, "get_config", lambda: SimpleNamespace(get_image_store=lambda: SimpleNamespace( - auth=lambda ref: None))) - monkeypatch.setattr(local_disk, "docker_login", lambda auth: None) - - def docker(*args): - if args[0] == "cp": - raise DockerFailed("docker cp container1:/disk/guest failed: Error response from daemon: i/o timeout", 1) - return "container1" - monkeypatch.setattr(local_disk, "docker", docker) - monkeypatch.setattr(local_disk.subprocess, "run", lambda argv, **kwargs: subprocess.CompletedProcess(argv, 0)) - image = "localhost:5000/agentenv-computers:" + "1" * 64 + "@sha256:" + "a" * 64 +def test_an_export_that_cant_copy_a_disks_guest_file_fails_rather_than_cache_the_disk_without_it(exporting): + exporting.guest = DockerFailed("docker cp container1:/disk/guest failed: Error response from daemon: i/o timeout", + 1) with pytest.raises(DockerFailed, match="i/o timeout"): - local_disk.exported_disk(image, "linux/amd64") - assert [path.name for path in (tmp_path / "cache" / "agentenv" / "disks").iterdir()] == [] + local_disk.exported_disk(EXPORTED, "linux/amd64") + assert list(exporting.cache.iterdir()) == [] -def test_a_thin_disk_is_exported_with_its_guest_file_and_its_base_once_for_every_disk_on_that_base(tmp_path, - monkeypatch): +def test_a_thin_disk_is_exported_with_its_guest_file_and_its_base_once_for_every_disk_on_that_base(exporting): """A Windows disk is a layer on its base, whose first layer every disk composed on it shares.""" - monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache")) - monkeypatch.setattr(local_disk, "launcher", lambda: "agentenv-qemu-launcher:src-1") - monkeypatch.setattr(local_disk, "get_config", lambda: SimpleNamespace(get_image_store=lambda: SimpleNamespace( - auth=lambda ref: None))) - monkeypatch.setattr(local_disk, "docker_login", lambda auth: None) - copied = [] - - def docker(*args): - if args[0] == "cp": - copied.append(args[1].partition(":")[2]) - Path(args[2]).write_bytes(b"windows" if args[1].endswith("guest") else b"copy of " + copied[-1].encode()) - return "3 sha256:base" if args[0] == "image" else "" - - def run(argv, stdin=None, stdout=None, **kwargs): - if argv[:2] == ["docker", "run"]: - stdout.write(b"zstd:" + stdin.read()) - return subprocess.CompletedProcess(argv, 0, b"", b"") - - monkeypatch.setattr(local_disk, "docker", docker) - monkeypatch.setattr(local_disk.subprocess, "run", run) - first = "localhost:5000/agentenv-computers:" + "1" * 64 + "@sha256:" + "a" * 64 + exporting.layers, exporting.first, exporting.guest = 3, "sha256:base", "windows" - one, two = (local_disk.exported_disk(image, "windows/amd64") for image in (first, first.replace("1", "2"))) + one, two = (local_disk.exported_disk(image, "windows/amd64") for image in (EXPORTED, EXPORTED.replace("1", "2"))) + copied = [args[1].partition(":")[2] for args in exporting.ran if args[0] == "cp"] assert one.disk != two.disk and one.base == two.base and one.base.parent == one.disk.parent assert one.base.read_bytes() == b"zstd:copy of /disk/base.img" assert one.disk.read_bytes() == b"zstd:copy of /disk/disk.img" and one.guest.read_text() == "windows" assert copied == ["/disk/guest", "/disk/base.img", "/disk/disk.img", "/disk/guest", "/disk/disk.img"] - assert local_disk.exported_disk(first, "windows/amd64") == one and len(copied) == 5 + assert local_disk.exported_disk(EXPORTED, "windows/amd64") == one + assert len([args for args in exporting.ran if args[0] == "cp"]) == 5 From eb372a6b75f1018404686e2b4dbc1bd38ae8c96c Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sat, 7 Nov 2026 18:15:50 -0800 Subject: [PATCH 12/12] revert(computer): Windows keeps the default ready timeout, which every boot measured stays well within Its ready wait takes about 40 s on modal_vm, whose export and upload come before it, and about 30 s on local Docker. Drops the disk form's ready_timeout_s, the windows-11 value and the ready_timeout signature change. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/artifact/artifacts/computer.py | 3 --- src/agent_env/computers/README.md | 10 ++++------ .../computers/artifacts/windows-11/artifact.toml | 1 - src/agent_env/env/envs/computer/utils/boot.py | 14 +++++--------- src/agent_env/env/envs/computer/utils/probing.py | 2 +- .../env_providers/env_computer_provider.py | 6 +++--- tst/unit/env/envs/test_computer_env.py | 10 ++++------ tst/unit/env/envs/test_computer_windows.py | 12 +----------- 8 files changed, 18 insertions(+), 40 deletions(-) diff --git a/src/agent_env/artifact/artifacts/computer.py b/src/agent_env/artifact/artifacts/computer.py index 6fe1dc62d..879511ec6 100644 --- a/src/agent_env/artifact/artifacts/computer.py +++ b/src/agent_env/artifact/artifacts/computer.py @@ -146,9 +146,6 @@ class DiskForm(BaseModel): input_channel: Literal["nocloud"] firmware: list[Literal["efi"]] = Field(default=["efi"], min_length=1) min_disk_gb: int = Field(ge=1, description="Its virtual size, which a deploy's disk is at least") - ready_timeout_s: float | None = Field(default=None, gt=0, exclude_if=lambda value: value is None, description=( - "How long a boot of it may take to be ready, for a computer agent-env's default isn't long enough for; left " - "out when unset, so a disk form without it is stored as it was before it existed")) class VmBundleForm(BaseModel): diff --git a/src/agent_env/computers/README.md b/src/agent_env/computers/README.md index 0ab0a76c9..ce2575906 100644 --- a/src/agent_env/computers/README.md +++ b/src/agent_env/computers/README.md @@ -172,12 +172,10 @@ pulls it. Gate 3 probes it at full. At the session user's sign-in, `agentenv-session`, run windowless by `pythonw`, reads the deploy's environment from the NoCloud seed, which the launcher attaches as a removable USB drive, since Windows mounts a FAT filesystem with no partition table on removable media alone, and starts each layer's services in that session, again whenever one exits. A Windows computer boots -with 4 vCPUs and 8 GB, on local Docker with KVM and on `modal_vm`, and may take 10 minutes to be ready, as its disk -form's `ready_timeout_s` says, for its first sign-in and, on `modal_vm`, its base expanded first; it has computer-exec -and data/add, through the protocol's Windows defaults, so the probe runs its fixture there as on Linux, with Python -one-liners for the commands it would give a shell. On `modal_vm` it runs in your own Modal workspace, under the -evaluation's terms or a licence of your own; whether they allow it there is yours to check. A Windows computer composes -its driver alone for now, no app. +with 4 vCPUs and 8 GB, on local Docker with KVM and on `modal_vm`; it has computer-exec and data/add, through the +protocol's Windows defaults, so the probe runs its fixture there as on Linux, with Python one-liners for the commands it +would give a shell. On `modal_vm` it runs in your own Modal workspace, under the evaluation's terms or a licence of your +own; whether they allow it there is yours to check. A Windows computer composes its driver alone for now, no app. This bundle has no tasks of its own: other bundles pull its computers by alias, and `agent-env run computers` has nothing to run. diff --git a/src/agent_env/computers/artifacts/windows-11/artifact.toml b/src/agent_env/computers/artifacts/windows-11/artifact.toml index 988906eee..06d92f9ed 100644 --- a/src/agent_env/computers/artifacts/windows-11/artifact.toml +++ b/src/agent_env/computers/artifacts/windows-11/artifact.toml @@ -9,7 +9,6 @@ init = "agentenv-session" input_channel = "nocloud" firmware = ["efi"] min_disk_gb = 64 -ready_timeout_s = 600 [config.resolution] default = "1920x1080" diff --git a/src/agent_env/env/envs/computer/utils/boot.py b/src/agent_env/env/envs/computer/utils/boot.py index 2ca24de96..d8fe7cf1e 100644 --- a/src/agent_env/env/envs/computer/utils/boot.py +++ b/src/agent_env/env/envs/computer/utils/boot.py @@ -28,7 +28,6 @@ from agent_env.artifact.artifacts.computer import ComputerArtifact from agent_env.attribution import Attribution from agent_env.env.envs.computer.app_env import ComputerAppEnv - from agent_env.env.envs.computer.env import ComputerEnv from agent_env.providers.sandbox_providers.sandbox import NetworkPolicy, Sandbox from agent_env.providers.sandbox_providers.sandbox_provider import SandboxProvider @@ -192,14 +191,11 @@ async def discard(sandbox: Sandbox) -> None: await asyncio.to_thread(remove_local_work_dir, sandbox.sandbox_id) -def ready_timeout(env: ComputerEnv, form: Optional[str]) -> float: - """How long ``env``'s computer booted in ``form`` may take to be ready: as long as its disk form says - (``ready_timeout_s``) when it boots that, else ``READY_TIMEOUT_S``, or longer when an app that starts with it there - gives itself longer.""" - disk, os_type = env.computer.forms.disk, env.computer.os.type - least = (disk.ready_timeout_s if form == "disk" and disk is not None else None) or READY_TIMEOUT_S - return max([least, *(launch.ready.timeout_s + APP_READY_MARGIN_S for app in env.composed_apps() - if (launch := app.launch(os_type)) and launch.at == "boot" and launch.ready)]) +def ready_timeout(apps: list[ComputerAppEnv], os_type: str) -> float: + """How long a computer of ``os_type`` and ``apps`` may take to be ready: ``READY_TIMEOUT_S``, or longer when an app + that starts with it there gives itself longer.""" + return max([READY_TIMEOUT_S, *(launch.ready.timeout_s + APP_READY_MARGIN_S for app in apps + if (launch := app.launch(os_type)) and launch.at == "boot" and launch.ready)]) async def ready(url: str, timeout: float, token: Optional[str] = None, *, answering: bool = False) -> dict: diff --git a/src/agent_env/env/envs/computer/utils/probing.py b/src/agent_env/env/envs/computer/utils/probing.py index a111fb0af..3989a4b10 100644 --- a/src/agent_env/env/envs/computer/utils/probing.py +++ b/src/agent_env/env/envs/computer/utils/probing.py @@ -165,7 +165,7 @@ async def _probe(links: list[SandboxProvider], env: ComputerEnv, driver: Compute try: booted = await boot.boot(link, image=image, port=driver.port, environment=environment, token=token, network_policy=None, timeout=_PROBE_TTL_S, attribution=None, - ready_s=boot.ready_timeout(env, form), form=form, + ready_s=boot.ready_timeout(env.composed_apps(), env.computer.os.type), form=form, os_type=env.computer.os.type) except Exception as e: problems.append(f"{type(link).__name__}: {e}") diff --git a/src/agent_env/providers/env_providers/env_computer_provider.py b/src/agent_env/providers/env_providers/env_computer_provider.py index 564d13624..bf4897315 100644 --- a/src/agent_env/providers/env_providers/env_computer_provider.py +++ b/src/agent_env/providers/env_providers/env_computer_provider.py @@ -142,7 +142,7 @@ async def _boot(self, env: ComputerEnv, builds: list[tuple[str, str, dict]], par sandbox_provider, image=build["image"], port=env.driver.port, network_policy=policy, token=token, environment=_environment(env, computer, values, params), timeout=ttl_seconds, attribution=attribution, disk_size_gb=disk_size_gb, cpu=cpu, memory_mb=memory_mb, - ready_s=ready_timeout(env, form), form=form, held=bool(params.site_routing), + ready_s=ready_timeout(env.composed_apps(), env.computer.os.type), form=form, held=bool(params.site_routing), os_type=env.computer.os.type) self._container_sandboxes.append(booted.sandbox) self._environment_sandboxes[name] = booted.sandbox @@ -189,7 +189,7 @@ async def reboot(self, env: ComputerEnv, deployed: DeployedSandboxEnv) -> None: env=environment) except NotImplementedError as e: # a reset the env supports, which this provider can't carry out raise RuntimeError(f"'{env.id}' can't be reset on {type(sandbox_provider).__name__}: {e}") from None - card = await ready(deployed.environment_url, ready_timeout(env, record.get("form")), + card = await ready(deployed.environment_url, ready_timeout(env.composed_apps(), env.computer.os.type), deployed.control_token, answering=bool(params.site_routing)) _require_name(env, card) valid_for_s = (record.get("sites") or {}).get("valid_for_s", 0) @@ -227,7 +227,7 @@ async def _launch_held_apps(env: ComputerEnv, record: DeployedSandboxEnv, url: s answers then, as its boot's card may list an app by its name alone. Each is launched as the child its card lists, as a computer named for its app, a browser alone, has that name too.""" os_type = env.computer.os.type - timeout = ready_timeout(env, record.metadata["computer"].get("form")) + timeout = ready_timeout(env.composed_apps(), os_type) held = [app for app in env.composed_apps() if (launch := app.launch(os_type)) is not None and launch.at == "boot"] launchers: dict[str, protocol_v1.ChildEnv] = {} if held: diff --git a/tst/unit/env/envs/test_computer_env.py b/tst/unit/env/envs/test_computer_env.py index 373e6d1eb..0537cda7e 100644 --- a/tst/unit/env/envs/test_computer_env.py +++ b/tst/unit/env/envs/test_computer_env.py @@ -1043,8 +1043,7 @@ async def test_a_computer_routing_sites_launches_its_held_boot_apps_once_listed_ order = [] apps = [SimpleNamespace(environment_name=name, launch=lambda os_type, at=at: SimpleNamespace(at=at)) for name, at in [("mail", "boot"), ("music", "demand"), ("notes", "boot")]] - env = SimpleNamespace(computer=SimpleNamespace(os=SimpleNamespace(type="linux"), forms=SimpleNamespace(disk=None)), - composed_apps=lambda: apps) + env = SimpleNamespace(computer=SimpleNamespace(os=SimpleNamespace(type="linux")), composed_apps=lambda: apps) def app_card(name): return {"name": name, "url": f"/svc/{name}/agentenv", "capabilities": {"extensions": [ @@ -1067,8 +1066,7 @@ async def ready(url, timeout, token): monkeypatch.setattr(env_computer_provider, "listed", listed) monkeypatch.setattr(env_computer_provider, "ready", ready) monkeypatch.setattr(env_computer_provider, "ready_timeout", lambda apps, os_type: 30.0) - record = SimpleNamespace(invoke=invoke, control_token="t0ken", environment_card={"name": "booted"}, - metadata={"computer": {"form": "container"}}) + record = SimpleNamespace(invoke=invoke, control_token="t0ken", environment_card={"name": "booted"}) await env_computer_provider._launch_held_apps(env, record, "http://computer") assert order == [("listed", "http://computer", ["mail", "notes"]), (COMPUTER_APP, "launch", "mail", True, "mail"), @@ -1579,8 +1577,8 @@ def test_an_app_that_requires_nothing_still_gets_a_venv_for_its_own_server(base, assert "/opt/agentenv/apps/mail/venv/bin/pip install --no-index --no-deps /tmp/agentenv-packages/wheels/apps-mail/*.whl" in ( build["dockerfile"]) assert build["confs"]["agentenv-app-mail.conf"] == MAIL_SERVER - assert boot.ready_timeout(SimpleNamespace(computer=base, composed_apps=lambda: [app]), "container") == ( - 270.0 if at == "boot" else boot.READY_TIMEOUT_S) + assert boot.ready_timeout([app], "linux") == (270.0 if at == "boot" else boot.READY_TIMEOUT_S) + assert boot.ready_timeout([app], "windows") == boot.READY_TIMEOUT_S def test_the_locks_debs_and_wheels_are_mounted_into_the_runs_that_install_them_never_copied_into_a_layer( diff --git a/tst/unit/env/envs/test_computer_windows.py b/tst/unit/env/envs/test_computer_windows.py index 4a4c8c234..a2b584be9 100644 --- a/tst/unit/env/envs/test_computer_windows.py +++ b/tst/unit/env/envs/test_computer_windows.py @@ -6,7 +6,6 @@ import json import threading import time -import tomllib from types import SimpleNamespace from datetime import datetime, timedelta, timezone from pathlib import Path, PureWindowsPath @@ -22,7 +21,7 @@ from agent_env.env.env import Env from agent_env.env.envs.computer.driver_env import ComputerDriverEnv from agent_env.env.envs.computer.env import ComputerEnv -from agent_env.env.envs.computer.utils import boot, composer, gate, windows_disk +from agent_env.env.envs.computer.utils import composer, gate, windows_disk from agent_env.env.envs.computer.utils.composer import Composing from agent_env.env.envs.computer.utils.install import WindowsInstall, WindowsService from agent_env.providers.sandbox_providers.sandbox_provider import SandboxProvider @@ -316,15 +315,6 @@ def test_a_machine_without_the_base_takes_the_image_stores_unless_its_evaluation assert (base.expires - datetime.now(timezone.utc)).days == 59 -def test_a_windows_computer_is_waited_on_as_long_as_its_disk_form_says(local_stores, cli_routing): - """The shipped base says 10 minutes; a base that says nothing gets agent-env's default.""" - shipped = tomllib.loads((windows_disk.RECIPE.parents[1] / "artifacts" / "windows-11" / "artifact.toml").read_text()) - declared = _base(forms={"disk": shipped["forms"]["disk"]}) - - assert boot.ready_timeout(SimpleNamespace(computer=declared, composed_apps=list), "disk") == 600 - assert boot.ready_timeout(SimpleNamespace(computer=_base(), composed_apps=list), "disk") == boot.READY_TIMEOUT_S - - def test_a_windows_computers_display_is_1920x1080_for_now(local_stores, cli_routing): base = _base()